FetchOne: consider order of create/delete events when replicating deletions The previous logic for detecting which refs had to be deleted as a result of a replication queue refspecs to be replicated did not take into consideration the order of the create/delete events for the same ref. As a result, the multiple create+delete of the same ref was potentially causing missed replication events as it was only resulting into a deletion. Example: 1. create refs/foo/bar + delete refs/foo/bar => delete refs/foo/bar 2. delete refs/foo/bar + create refs/foo/bar => create refs/foo/bar The step 2. was resulting into a delete refs/foo/bar instead of a create. Make the extraction of the refs to delete more intelligent so that all operations are considered and properly calculated in order for getting the final results of the create + delete of the same ref. Bug: Issue 394760809 Change-Id: I83d3b38cbb8e3391ca7af2af8ddea245258c4413

diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchOne.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchOne.java
index e75c4c3..5e7fda2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchOne.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchOne.java

@@ -17,6 +17,7 @@
 import static com.googlesource.gerrit.plugins.replication.pull.PullReplicationLogger.repLog;
 import static java.util.concurrent.TimeUnit.NANOSECONDS;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Throwables;
 import com.google.common.collect.LinkedListMultimap;
 import com.google.common.collect.ListMultimap;
@@ -476,11 +477,7 @@
     try {
       List<FetchRefSpec> toFetch =
           fetchRefSpecs.stream().filter(rs -> rs.getSource() != null).toList();
-      Set<String> toDelete =
-          fetchRefSpecs.stream()
-              .filter(rs -> rs.getSource() == null)
-              .map(RefSpec::getDestination)
-              .collect(Collectors.toSet());
+      Set<String> toDelete = refsToDelete(fetchRefSpecs);
       updateStates(fetch.fetch(toFetch));
 
       // JGit doesn't support a fetch of <empty> to a ref (e.g. :refs/to/delete) therefore we have
@@ -512,6 +509,21 @@
     return fetchRefSpecs;
   }
 
+  @VisibleForTesting
+  static Set<String> refsToDelete(List<FetchRefSpec> fetchRefSpecs) {
+    final Set<String> refsToDelete = new HashSet<>();
+    fetchRefSpecs.forEach(
+        rs -> {
+          String refName = rs.refName();
+          if (rs.isDelete()) {
+            refsToDelete.add(refName);
+          } else {
+            refsToDelete.remove(refName);
+          }
+        });
+    return refsToDelete;
+  }
+
   /**
    * Return the list of refSpecs to fetch, possibly after having been filtered.
    *

diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchRefSpec.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchRefSpec.java
index f85f22a..e41e194 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchRefSpec.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchRefSpec.java

@@ -44,6 +44,10 @@
     return MoreObjects.firstNonNull(getSource(), getDestination());
   }
 
+  public boolean isDelete() {
+    return getSource() == null;
+  }
+
   @Override
   public String toString() {
     return getSource() == null ? "<<DELETED>>:" + getDestination() : super.toString();

diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchOneTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchOneTest.java
index 49e8d6f..73715d6 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchOneTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/FetchOneTest.java

@@ -15,6 +15,7 @@
 package com.googlesource.gerrit.plugins.replication.pull;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.googlesource.gerrit.plugins.replication.pull.FetchOne.refsToDelete;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.anyList;
 import static org.mockito.Mockito.argThat;
@@ -850,6 +851,36 @@
     assertThat(testMetricMaker.getTimer("replication_latency")).isGreaterThan(0);
   }
 
+  @Test
+  public void shouldSkipDeletionWhenDeleteAndCreateOfSameRef() {
+    List<FetchRefSpec> fetchRefSpecs =
+        List.of(
+            FetchRefSpec.fromRef(":refs/something/someref"),
+            FetchRefSpec.fromRef("refs/something/someref"));
+    assertThat(refsToDelete(fetchRefSpecs)).isEmpty();
+  }
+
+  @Test
+  public void shouldDeleteWhenCreateAndDeleteOfSameRef() {
+    List<FetchRefSpec> fetchRefSpecs =
+        List.of(
+            FetchRefSpec.fromRef("refs/something/someref"),
+            FetchRefSpec.fromRef(":refs/something/someref"));
+    assertThat(refsToDelete(fetchRefSpecs)).isEqualTo(Set.of("refs/something/someref"));
+  }
+
+  @Test
+  public void shouldNotDeleteWhenCreateRef() {
+    List<FetchRefSpec> fetchRefSpecs = List.of(FetchRefSpec.fromRef("refs/something/someref"));
+    assertThat(refsToDelete(fetchRefSpecs)).isEmpty();
+  }
+
+  @Test
+  public void shouldDeleteWhenDeleteRef() {
+    List<FetchRefSpec> fetchRefSpecs = List.of(FetchRefSpec.fromRef(":refs/something/someref"));
+    assertThat(refsToDelete(fetchRefSpecs)).isEqualTo(Set.of("refs/something/someref"));
+  }
+
   private void setupRequestScopeMock() {
     when(scoper.scope(any()))
         .thenAnswer(