ForwardedIndexChangeHandler: check SHA visibility before reindexing

Move the isChangeUpToDate() check before reindexIfStale() so that
staleness is detected upfront. Previously, reindexIfStale() was called
unconditionally and isChangeUpToDate() served as a post-condition: if
the JGit cache happened to refresh between the two calls, the post-check
would pass and we would accept a result where no actual reindexing took
place.

With this change, if the meta SHA is not yet visible, indexOnce()
returns false immediately and we proceed with the next (re)try.
reindexIfStale() is only called once the SHA is confirmed visible,
making the retry logic effective.

Bug: Issue 541924680
Change-Id: If546dbacbfc9635a5ae9ac10ec5848486ce5cd4c

src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandler.java

diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandler.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandler.java
index fd654ba..102ee77 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandler.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandler.java
@@ -84,18 +84,18 @@
         changeNotes = Optional.empty();
       }
       if (changeNotes.isPresent()) {
-        ChangeNotes notes = changeNotes.get();
-        reindex(notes);
-
-        if (checker.isChangeUpToDate(indexEvent)) {
-          log.atFine().log("Change %s successfully indexed", id);
-          return true;
+        if (!checker.isChangeUpToDate(indexEvent)) {
+          log.atFine().log(
+              "Change %s seems too old compared to the event timestamp (event-Ts=%s >>"
+                  + " change-Ts=%s)",
+              id, indexEvent, checker);
+          return false;
         }
 
-        log.atFine().log(
-            "Change %s seems too old compared to the event timestamp (event-Ts=%s >> change-Ts=%s)",
-            id, indexEvent, checker);
-        return false;
+        ChangeNotes notes = changeNotes.get();
+        reindex(notes);
+        log.atFine().log("Change %s successfully indexed", id);
+        return true;
       }
 
       log.atFine().log(

src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandlerTest.java

diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandlerTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandlerTest.java
index 6a3a348..446a1e9 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandlerTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandlerTest.java
@@ -20,8 +20,8 @@
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.mockito.Answers.RETURNS_DEEP_STUBS;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.atLeast;
 import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -100,11 +100,30 @@
   }
 
   @Test
-  public void changeIsStillIndexedEvenWhenOutdated() throws Exception {
+  public void changeIsNotReindexedWhenShaIsNeverVisible() throws Exception {
     setupChangeAccessRelatedMocks(CHANGE_EXISTS, CHANGE_OUTDATED);
     handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.of(new IndexEvent())).get(10, SECONDS);
-    verify(indexerMock, atLeast(1))
-        .reindexIfStale(any(Project.NameKey.class), any(Change.Id.class));
+    verify(indexerMock, never()).reindexIfStale(any(Project.NameKey.class), any(Change.Id.class));
+  }
+
+  @Test
+  public void changeIsEventuallyIndexedWhenShaBecomesVisible() throws Exception {
+    // First attempt: sha not visible yet (outdated); second attempt: sha visible (up-to-date).
+    when(changeCheckerFactoryMock.create(TEST_CHANGE_ID))
+        .thenReturn(changeCheckerAbsentMock)
+        .thenReturn(changeCheckerPresentMock);
+
+    when(changeCheckerAbsentMock.getChangeNotes()).thenReturn(Optional.of(changeNotes));
+    when(changeCheckerAbsentMock.isChangeUpToDate(any())).thenReturn(CHANGE_OUTDATED);
+
+    when(changeCheckerPresentMock.getChangeNotes()).thenReturn(Optional.of(changeNotes));
+    when(changeCheckerPresentMock.isChangeUpToDate(any())).thenReturn(CHANGE_UP_TO_DATE);
+
+    when(changeNotes.getChangeId()).thenReturn(id);
+    when(changeNotes.getProjectName()).thenReturn(projectName);
+
+    handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.of(new IndexEvent())).get(10, SECONDS);
+    verify(indexerMock, times(1)).reindexIfStale(any(Project.NameKey.class), any(Change.Id.class));
   }
 
   @Test