Merge branch 'stable-3.8' into stable-3.9 * stable-3.8: Add acceptance test for change up-to-date checker Indexing retries: lower normal condition logs to debug Check for existence of change's target SHA1 for reindexing Change-Id: I93b55dc97518657f522e2fc030fd938ac329f0e5

diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexingHandlerWithRetries.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexingHandlerWithRetries.java
index f3d4e70..d135e39 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexingHandlerWithRetries.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexingHandlerWithRetries.java

@@ -64,7 +64,7 @@
   protected boolean rescheduleIndex(T id) {
     IndexingRetry retry = indexingRetryTaskMap.get(id);
     if (retry == null) {
-      log.info(
+      log.debug(
           "{} {} successfully indexed by different task, rescheduling isn't needed",
           indexName(),
           id);
@@ -110,7 +110,7 @@
     IndexingRetry retry = new IndexingRetry(event);
     if (indexingRetryTaskMap.put(id, retry) != null) {
       indexOnce.accept(id);
-      log.info(
+      log.debug(
           "Skipping indexing because there is already a running task for the specified id. Index name: {}, task id: {}",
           indexName(),
           id);

diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerImpl.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerImpl.java
index 7b90468..f3fa2e3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerImpl.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerImpl.java

@@ -26,11 +26,12 @@
 import com.google.gerrit.server.util.ManualRequestContext;
 import com.google.gerrit.server.util.OneOffRequestContext;
 import com.google.inject.Inject;
+import com.google.inject.Module;
 import com.google.inject.assistedinject.Assisted;
+import com.google.inject.assistedinject.FactoryModuleBuilder;
 import com.googlesource.gerrit.plugins.multisite.forwarder.events.ChangeIndexEvent;
 import java.io.IOException;
 import java.sql.Timestamp;
-import java.util.Objects;
 import java.util.Optional;
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.lib.Config;
@@ -57,6 +58,12 @@
     public ChangeChecker create(String changeId);
   }
 
+  public static Module module() {
+    return new FactoryModuleBuilder()
+        .implement(ChangeChecker.class, ChangeCheckerImpl.class)
+        .build(ChangeCheckerImpl.Factory.class);
+  }
+
   @Inject
   public ChangeCheckerImpl(
       GitRepositoryManager gitRepoMgr,
@@ -114,8 +121,7 @@
         .map(
             e ->
                 (computedChangeTs.get() > e.eventCreatedOn)
-                    || ((computedChangeTs.get() == e.eventCreatedOn)
-                        && (Objects.equals(getBranchTargetSha(), e.targetSha))))
+                    || ((computedChangeTs.get() == e.eventCreatedOn) && repositoryHas(e.targetSha)))
         .orElse(true);
   }
 
@@ -154,6 +160,15 @@
     }
   }
 
+  private boolean repositoryHas(String targetSha) {
+    try (Repository repo = gitRepoMgr.openRepository(changeNotes.get().getProjectName())) {
+      return repo.parseCommit(ObjectId.fromString(targetSha)) != null;
+    } catch (IOException e) {
+      log.warn("Unable to find SHA1 {} for change {}", targetSha, changeId, e);
+      return false;
+    }
+  }
+
   @Override
   public boolean isChangeConsistent() {
     Optional<ChangeNotes> notes = getChangeNotes();

diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/IndexModule.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/IndexModule.java
index 5f47371..0e7a54f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/IndexModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/IndexModule.java

@@ -21,7 +21,6 @@
 import com.google.gerrit.extensions.registration.DynamicSet;
 import com.google.gerrit.lifecycle.LifecycleModule;
 import com.google.gerrit.server.events.EventListener;
-import com.google.inject.assistedinject.FactoryModuleBuilder;
 import java.util.concurrent.Executor;
 import java.util.concurrent.ScheduledExecutorService;
 
@@ -43,9 +42,6 @@
     bind(ProjectChecker.class).to(ProjectCheckerImpl.class);
     bind(GroupChecker.class).to(GroupCheckerImpl.class);
 
-    install(
-        new FactoryModuleBuilder()
-            .implement(ChangeChecker.class, ChangeCheckerImpl.class)
-            .build(ChangeCheckerImpl.Factory.class));
+    install(ChangeCheckerImpl.module());
   }
 }

diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerIT.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerIT.java
new file mode 100644
index 0000000..d3c5e8d
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerIT.java

@@ -0,0 +1,114 @@
+// Copyright (C) 2024 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.multisite.index;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.inject.AbstractModule;
+import com.googlesource.gerrit.plugins.multisite.forwarder.events.ChangeIndexEvent;
+import java.util.Optional;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.junit.Test;
+
+@TestPlugin(
+    name = "multi-site",
+    sysModule = "com.googlesource.gerrit.plugins.multisite.index.ChangeCheckerIT$TestModule")
+public class ChangeCheckerIT extends LightweightPluginDaemonTest {
+  private static final String TEST_BRANCH_REFS_HEADS = RefNames.fullName("master");
+  private static final String NONEXISTENTSHA1 = "d3d192b8f704aec0c764cca427fdaca88db71513";
+  private ChangeCheckerImpl.Factory changeCheckerFactory;
+
+  public static class TestModule extends AbstractModule {
+    @Override
+    protected void configure() {
+      install(ChangeCheckerImpl.module());
+    }
+  }
+
+  @Override
+  public void setUpTestPlugin() throws Exception {
+    super.setUpTestPlugin();
+
+    changeCheckerFactory = plugin.getSysInjector().getInstance(ChangeCheckerImpl.Factory.class);
+  }
+
+  @Test
+  @GerritConfig(name = "gerrit.instanceId", value = "test-instance")
+  public void shouldBeUpToDateIfTargetBranchSHA1HasAdvanced() throws Exception {
+    RevCommit headCommit = createCommit();
+    int changeNum = newChangeNum();
+    String changeId = changeProjectAndNum(changeNum);
+    long changeCommitTs = changeCommitTs(changeNum);
+
+    ChangeIndexEvent indexChangeEvent = newIndexChangeEvent(changeNum);
+    indexChangeEvent.eventCreatedOn = changeCommitTs / 1000L;
+    indexChangeEvent.targetSha = headCommit.getId().getName();
+
+    RevCommit secondCommit = createCommit();
+    assertThat(secondCommit.getId().getName()).isNotEqualTo(indexChangeEvent.targetSha);
+    assertThat(changeCheckerFactory.create(changeId).isUpToDate(Optional.of(indexChangeEvent)))
+        .isTrue();
+  }
+
+  @Test
+  @GerritConfig(name = "gerrit.instanceId", value = "test-instance")
+  public void shouldNotBeUpToDateIfTargetSHA1Absent() throws Exception {
+    int changeNum = newChangeNum();
+    String changeId = changeProjectAndNum(changeNum);
+    long changeCommitTs = changeCommitTs(changeNum);
+
+    ChangeIndexEvent indexChangeEvent = newIndexChangeEvent(changeNum);
+    indexChangeEvent.eventCreatedOn = changeCommitTs / 1000L;
+    indexChangeEvent.targetSha = NONEXISTENTSHA1;
+
+    assertThat(changeCheckerFactory.create(changeId).isUpToDate(Optional.of(indexChangeEvent)))
+        .isFalse();
+  }
+
+  private ChangeIndexEvent newIndexChangeEvent(int changeNum) {
+    ChangeIndexEvent indexChangeEvent =
+        new ChangeIndexEvent(project.get(), changeNum, false, instanceId);
+    return indexChangeEvent;
+  }
+
+  private int newChangeNum() throws Exception {
+    PushOneCommit.Result changeRes = createChange();
+    changeRes.assertOkStatus();
+    int changeNum = changeRes.getChange().getId().get();
+    return changeNum;
+  }
+
+  private String changeProjectAndNum(int changeNum) {
+    String changeId = String.format("%s~%d", project.get(), changeNum);
+    return changeId;
+  }
+
+  private long changeCommitTs(int changeNum) throws RestApiException {
+    long changeCommitTs = gApi.changes().id(project.get(), changeNum).get().updated.getTime();
+    return changeCommitTs;
+  }
+
+  private RevCommit createCommit() throws Exception {
+    PushOneCommit.Result result = pushTo(TEST_BRANCH_REFS_HEADS);
+    result.assertOkStatus();
+    return result.getCommit();
+  }
+}