Filter excluded refs before scheduling tasks

excludedRefsPattern was only applied when building the push,
so matching refs were still queued and written to task storage
as no-op work. Check the pattern in Destination.wouldPushRef()
so those refs are skipped before a task is scheduled or persisted.

Change-Id: Iebfca8a5d025bef57894d67578668422f05a56ad
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
index b35a5d1..08a6c62 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
@@ -85,7 +85,6 @@
 import java.util.concurrent.locks.ReadWriteLock;
 import java.util.function.Function;
 import java.util.function.Supplier;
-import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.Ref;
@@ -821,6 +820,10 @@
     if (PushOne.ALL_REFS.equals(ref)) {
       return true;
     }
+    if (isRefExcluded(ref)) {
+      repLog.atFine().log("Skipping push of ref %s; it matches excludedRefsPattern", ref);
+      return false;
+    }
     for (RefSpec s : config.getRemoteConfig().getPushRefSpecs()) {
       if (s.matchSource(ref)) {
         return true;
@@ -975,8 +978,8 @@
     return config.replicateNoteDbMetaRefs();
   }
 
-  ImmutableList<Pattern> excludedRefsPattern() {
-    return config.excludedRefsPattern();
+  boolean isRefExcluded(String ref) {
+    return config.isRefExcluded(ref);
   }
 
   boolean storeRefLog() {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
index 24f12e3..07cb830 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
@@ -795,7 +795,7 @@
     return !(noPerms && RefNames.REFS_CONFIG.equals(ref))
         && !ref.startsWith(RefNames.REFS_CACHE_AUTOMERGE)
         && !(!pool.replicateNoteDbMetaRefs() && RefNames.isNoteDbMetaRef(ref))
-        && pool.excludedRefsPattern().stream().noneMatch(p -> p.matcher(ref).matches());
+        && !pool.isRefExcluded(ref);
   }
 
   private Map<String, Ref> listRemote(Transport tn)
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteConfiguration.java b/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteConfiguration.java
index 79bdbf3..108250b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteConfiguration.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteConfiguration.java
@@ -161,6 +161,16 @@
   }
 
   /**
+   * Whether a ref is excluded from replication by any of the {@link #excludedRefsPattern()}
+   *
+   * @param ref name of the ref to check
+   * @return true if the ref should not be replicated, false otherwise
+   */
+  default boolean isRefExcluded(String ref) {
+    return excludedRefsPattern().stream().anyMatch(p -> p.matcher(ref).matches());
+  }
+
+  /**
    * reflog storage flag for newly created repositories
    *
    * @return true if new repositories should store ref-updates in their reflog
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 211bbcb..76a1858 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -738,6 +738,9 @@
   `excludedRefsPattern` keys can be supplied, to specify multiple regular
   expressions to match against.
 
+  Excluded refs are filtered out before a replication task is scheduled, so they
+  do not appear in the replication queue or in the persisted task storage.
+
   Do not exclude any refs by default.
 
 remote.NAME.urlDistributionStrategy
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/PushOneTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/PushOneTest.java
index 6def78f..076a903 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/PushOneTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/PushOneTest.java
@@ -53,7 +53,6 @@
 import java.util.concurrent.Callable;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
-import java.util.regex.Pattern;
 import org.eclipse.jgit.errors.NotSupportedException;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.errors.TransportException;
@@ -297,9 +296,8 @@
 
   @Test
   public void skipPushingExcludedRefs() throws InterruptedException, IOException {
-    when(destinationMock.excludedRefsPattern())
-        .thenReturn(
-            ImmutableList.of(Pattern.compile("refs/foo/.*"), Pattern.compile("refs/bar/.*")));
+    when(destinationMock.isRefExcluded("refs/foo/test")).thenReturn(true);
+    when(destinationMock.isRefExcluded("refs/bar/test")).thenReturn(true);
     PushOne pushOne = Mockito.spy(createPushOne(null));
 
     Ref ref1 =
@@ -496,7 +494,6 @@
   private void setupDestinationMock() {
     destinationMock = mock(Destination.class);
     when(destinationMock.requestRunway(any())).thenReturn(RunwayStatus.allowed());
-    when(destinationMock.excludedRefsPattern()).thenReturn(ImmutableList.of());
   }
 
   private void setupPermissionBackedMock() {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationStorageIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationStorageIT.java
index bfcf5be..08a7ecb 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationStorageIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationStorageIT.java
@@ -131,6 +131,21 @@
   }
 
   @Test
+  public void shouldNotCreateReplicationTaskForRefMatchingExcludedRefsPattern() throws Exception {
+    createTestProject(project + "replica");
+    setExcludedRefsPattern("foo", "refs/heads/excluded.*");
+
+    scheduleFullSync("refs/heads/excluded-branch");
+
+    assertThat(listWaiting()).isEmpty();
+
+    String replicatedRef = "refs/heads/replicated-branch";
+    scheduleFullSync(replicatedRef);
+
+    assertThat(listWaitingReplicationTasks(Pattern.quote(replicatedRef))).hasSize(1);
+  }
+
+  @Test
   public void shouldFirePendingOnlyToIncompleteUri() throws Exception {
     String suffix1 = "replica1";
     String suffix2 = "replica2";
@@ -377,6 +392,20 @@
     assertThat(listWaitingReplicationTasks(branchToDelete)).hasSize(1);
   }
 
+  private void setExcludedRefsPattern(String remote, String pattern) throws Exception {
+    setReplicationDestination(remote, "replica", ALL_PROJECTS, Integer.MAX_VALUE);
+    config.setString("remote", remote, "excludedRefsPattern", pattern);
+    config.save();
+    reloadConfig();
+  }
+
+  private void scheduleFullSync(String ref) {
+    plugin
+        .getSysInjector()
+        .getInstance(ReplicationQueue.class)
+        .scheduleFullSync(project, null, ref, Set.of(), new ReplicationState(NO_OP), false);
+  }
+
   private boolean isTaskRescheduled(Queue queue, URIish uri) {
     PushOne pushOne = queue.pending.get(uri);
     return pushOne == null ? false : pushOne.isRetrying();