Merge branch 'stable-3.0' into stable-3.1

* stable-3.0:
  GerritRestApi: Use UTF_8 from StandardCharsets instead of Guava
  Fix NPE in PushResultProcessing
  Fix start --wait to track in-flight collisions and to not fail
  Improve URLmatching to match real Urls
  Convert PushResultProcessing to an interface

Change-Id: I623b58397d4ae9b9cc6c7038c992225eb33bc3b7
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 0d3939a..029dc83 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
@@ -649,32 +649,34 @@
 
   List<URIish> getURIs(Project.NameKey project, String urlMatch) {
     List<URIish> r = Lists.newArrayListWithCapacity(config.getRemoteConfig().getURIs().size());
-    for (URIish uri : config.getRemoteConfig().getURIs()) {
-      if (matches(uri, urlMatch)) {
-        String name = project.get();
-        if (needsUrlEncoding(uri)) {
-          name = encode(name);
-        }
-        String remoteNameStyle = config.getRemoteNameStyle();
-        if (remoteNameStyle.equals("dash")) {
-          name = name.replace("/", "-");
-        } else if (remoteNameStyle.equals("underscore")) {
-          name = name.replace("/", "_");
-        } else if (remoteNameStyle.equals("basenameOnly")) {
-          name = FilenameUtils.getBaseName(name);
-        } else if (!remoteNameStyle.equals("slash")) {
-          repLog.debug("Unknown remoteNameStyle: {}, falling back to slash", remoteNameStyle);
-        }
-        String replacedPath = replaceName(uri.getPath(), name, config.isSingleProjectMatch());
-        if (replacedPath != null) {
-          uri = uri.setPath(replacedPath);
-          r.add(uri);
-        }
+    for (URIish configUri : config.getRemoteConfig().getURIs()) {
+      URIish uri = getURI(configUri, project);
+      if (matches(configUri, urlMatch) || matches(uri, urlMatch)) {
+        r.add(uri);
       }
     }
     return r;
   }
 
+  URIish getURI(URIish template, Project.NameKey project) {
+    String name = project.get();
+    if (needsUrlEncoding(template)) {
+      name = encode(name);
+    }
+    String remoteNameStyle = config.getRemoteNameStyle();
+    if (remoteNameStyle.equals("dash")) {
+      name = name.replace("/", "-");
+    } else if (remoteNameStyle.equals("underscore")) {
+      name = name.replace("/", "_");
+    } else if (remoteNameStyle.equals("basenameOnly")) {
+      name = FilenameUtils.getBaseName(name);
+    } else if (!remoteNameStyle.equals("slash")) {
+      repLog.debug("Unknown remoteNameStyle: {}, falling back to slash", remoteNameStyle);
+    }
+    String replacedPath = replaceName(template.getPath(), name, isSingleProjectMatch());
+    return (replacedPath != null) ? template.setPath(replacedPath) : template;
+  }
+
   static boolean needsUrlEncoding(URIish uri) {
     return "http".equalsIgnoreCase(uri.getScheme())
         || "https".equalsIgnoreCase(uri.getScheme())
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/GerritRestApi.java b/src/main/java/com/googlesource/gerrit/plugins/replication/GerritRestApi.java
index f910a40..66130f9 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/GerritRestApi.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/GerritRestApi.java
@@ -16,8 +16,8 @@
 
 import static com.googlesource.gerrit.plugins.replication.GerritSshApi.GERRIT_ADMIN_PROTOCOL_PREFIX;
 import static com.googlesource.gerrit.plugins.replication.ReplicationQueue.repLog;
+import static java.nio.charset.StandardCharsets.UTF_8;
 
-import com.google.common.base.Charsets;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.extensions.restapi.Url;
 import com.google.inject.Inject;
@@ -93,8 +93,7 @@
     String url = String.format("%s/a/projects/%s/HEAD", toHttpUri(uri), Url.encode(project.get()));
     try {
       HttpPut req = new HttpPut(url);
-      req.setEntity(
-          new StringEntity(String.format("{\"ref\": \"%s\"}", newHead), Charsets.UTF_8.name()));
+      req.setEntity(new StringEntity(String.format("{\"ref\": \"%s\"}", newHead), UTF_8.name()));
       req.addHeader(new BasicHeader("Content-Type", "application/json"));
       httpClient.execute(req, new HttpResponseHandler(), getContext());
       return true;
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 883cb23..2cbd627 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
@@ -103,6 +103,7 @@
   private final Set<String> delta = Sets.newHashSetWithExpectedSize(4);
   private boolean pushAllRefs;
   private Repository git;
+  private boolean isCollision;
   private boolean retrying;
   private int retryCount;
   private final int maxRetries;
@@ -281,7 +282,7 @@
   }
 
   private void statesCleanUp() {
-    if (!stateMap.isEmpty() && !isRetrying()) {
+    if (!stateMap.isEmpty() && !isRetrying() && !isCollision) {
       for (Map.Entry<String, ReplicationState> entry : stateMap.entries()) {
         entry
             .getValue()
@@ -316,6 +317,7 @@
     //
     MDC.put(ID_MDC_KEY, HexFormat.fromInt(id));
     RunwayStatus status = pool.requestRunway(this);
+    isCollision = false;
     if (!status.isAllowed()) {
       if (status.isCanceled()) {
         repLog.info("PushOp for replication to {} was canceled and thus won't be rescheduled", uri);
@@ -325,6 +327,7 @@
             uri,
             HexFormat.fromInt(status.getInFlightPushId()));
         pool.reschedule(this, Destination.RetryReason.COLLISION);
+        isCollision = true;
       }
       return;
     }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/PushResultProcessing.java b/src/main/java/com/googlesource/gerrit/plugins/replication/PushResultProcessing.java
index ad68d42..92ba4be 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/PushResultProcessing.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/PushResultProcessing.java
@@ -25,36 +25,54 @@
 import org.eclipse.jgit.transport.RemoteRefUpdate;
 import org.eclipse.jgit.transport.URIish;
 
-public abstract class PushResultProcessing {
+public interface PushResultProcessing {
+  public static final PushResultProcessing NO_OP = new PushResultProcessing() {};
 
-  abstract void onRefReplicatedToOneNode(
+  /**
+   * Invoked when a ref has been replicated to one node.
+   *
+   * @param project
+   * @param ref
+   * @param uri
+   * @param status
+   * @param refStatus
+   */
+  default void onRefReplicatedToOneNode(
       String project,
       String ref,
       URIish uri,
       RefPushResult status,
-      RemoteRefUpdate.Status refStatus);
+      RemoteRefUpdate.Status refStatus) {}
 
-  abstract void onRefReplicatedToAllNodes(String project, String ref, int nodesCount);
+  /**
+   * Invoked when a ref has been replicated to all nodes.
+   *
+   * @param project
+   * @param ref
+   * @param nodesCount
+   */
+  default void onRefReplicatedToAllNodes(String project, String ref, int nodesCount) {}
 
-  abstract void onAllRefsReplicatedToAllNodes(int totalPushTasksCount);
+  /**
+   * Invoked when all refs have been replicated to all nodes.
+   *
+   * @param totalPushTasksCount
+   */
+  default void onAllRefsReplicatedToAllNodes(int totalPushTasksCount) {}
 
   /**
    * Write message to standard out.
    *
    * @param message message text.
    */
-  void writeStdOut(String message) {
-    // Default doing nothing
-  }
+  default void writeStdOut(String message) {}
 
   /**
    * Write message to standard error.
    *
    * @param message message text.
    */
-  void writeStdErr(String message) {
-    // Default doing nothing
-  }
+  default void writeStdErr(String message) {}
 
   static String resolveNodeName(URIish uri) {
     StringBuilder sb = new StringBuilder();
@@ -70,7 +88,7 @@
     return sb.toString();
   }
 
-  public static class CommandProcessing extends PushResultProcessing {
+  public static class CommandProcessing implements PushResultProcessing {
     private WeakReference<StartCommand> sshCommand;
     private AtomicBoolean hasError = new AtomicBoolean();
 
@@ -79,7 +97,7 @@
     }
 
     @Override
-    void onRefReplicatedToOneNode(
+    public void onRefReplicatedToOneNode(
         String project,
         String ref,
         URIish uri,
@@ -109,13 +127,13 @@
           break;
       }
       sb.append(" (");
-      sb.append(refStatus.toString());
+      sb.append(refStatus == null ? "unknown" : refStatus.toString());
       sb.append(")");
       writeStdOut(sb.toString());
     }
 
     @Override
-    void onRefReplicatedToAllNodes(String project, String ref, int nodesCount) {
+    public void onRefReplicatedToAllNodes(String project, String ref, int nodesCount) {
       StringBuilder sb = new StringBuilder();
       sb.append("Replication of ");
       sb.append(project);
@@ -128,7 +146,7 @@
     }
 
     @Override
-    void onAllRefsReplicatedToAllNodes(int totalPushTasksCount) {
+    public void onAllRefsReplicatedToAllNodes(int totalPushTasksCount) {
       if (totalPushTasksCount == 0) {
         return;
       }
@@ -141,7 +159,7 @@
     }
 
     @Override
-    void writeStdOut(String message) {
+    public void writeStdOut(String message) {
       StartCommand command = sshCommand.get();
       if (command != null) {
         command.writeStdOutSync(message);
@@ -149,7 +167,7 @@
     }
 
     @Override
-    void writeStdErr(String message) {
+    public void writeStdErr(String message) {
       StartCommand command = sshCommand.get();
       if (command != null) {
         command.writeStdErrSync(message);
@@ -157,7 +175,7 @@
     }
   }
 
-  public static class GitUpdateProcessing extends PushResultProcessing {
+  public static class GitUpdateProcessing implements PushResultProcessing {
     private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
     private final EventDispatcher dispatcher;
@@ -167,7 +185,7 @@
     }
 
     @Override
-    void onRefReplicatedToOneNode(
+    public void onRefReplicatedToOneNode(
         String project,
         String ref,
         URIish uri,
@@ -177,13 +195,10 @@
     }
 
     @Override
-    void onRefReplicatedToAllNodes(String project, String ref, int nodesCount) {
+    public void onRefReplicatedToAllNodes(String project, String ref, int nodesCount) {
       postEvent(new RefReplicationDoneEvent(project, ref, nodesCount));
     }
 
-    @Override
-    void onAllRefsReplicatedToAllNodes(int totalPushTasksCount) {}
-
     private void postEvent(RefEvent event) {
       try {
         dispatcher.postEvent(event);
diff --git a/src/main/resources/Documentation/cmd-start.md b/src/main/resources/Documentation/cmd-start.md
index 6af73af..8291421 100644
--- a/src/main/resources/Documentation/cmd-start.md
+++ b/src/main/resources/Documentation/cmd-start.md
@@ -97,10 +97,10 @@
 :	Schedule replication for all projects.
 
 `--url <PATTERN>`
-:	Replicate only to replication destinations whose URL contains
-	the substring `PATTERN`.  This can be useful to replicate
-	only to a previously down node, which has been brought back
-	online.
+:	Replicate only to replication destinations whose configuration
+	URL contains the substring `PATTERN`, or whose expanded project
+	URL contains `PATTERN`. This can be useful to replicate only to
+	a previously down node, which has been brought back online.
 
 EXAMPLES
 --------
@@ -136,6 +136,12 @@
   $ ssh -p @SSH_PORT@ @SSH_HOST@ @PLUGIN@ start --url slave1 ^(|.*/)vendor(|/.*)
 ```
 
+Replicate to only one specific destination URL:
+
+```
+  $ ssh -p @SSH_PORT@ @SSH_HOST@ @PLUGIN@ start --url https://example.com/tools/gerrit.git
+```
+
 SEE ALSO
 --------
 
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java
index 9cf5489..19948db 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationIT.java
@@ -16,6 +16,7 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static com.googlesource.gerrit.plugins.replication.PushResultProcessing.NO_OP;
 import static java.util.stream.Collectors.toList;
 
 import com.google.common.flogger.FluentLogger;
@@ -47,8 +48,6 @@
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.storage.file.FileBasedConfig;
-import org.eclipse.jgit.transport.RemoteRefUpdate;
-import org.eclipse.jgit.transport.URIish;
 import org.eclipse.jgit.util.FS;
 import org.junit.Test;
 
@@ -206,25 +205,7 @@
 
   @Test
   public void shouldCreateOneReplicationTaskWhenSchedulingRepoFullSync() throws Exception {
-    PushResultProcessing pushResultProcessing =
-        new PushResultProcessing() {
-
-          @Override
-          void onRefReplicatedToOneNode(
-              String project,
-              String ref,
-              URIish uri,
-              ReplicationState.RefPushResult status,
-              RemoteRefUpdate.Status refStatus) {}
-
-          @Override
-          void onRefReplicatedToAllNodes(String project, String ref, int nodesCount) {}
-
-          @Override
-          void onAllRefsReplicatedToAllNodes(int totalPushTasksCount) {}
-        };
-
-    createTestProject("replica");
+    createTestProject(project + "replica");
 
     setReplicationDestination("foo", "replica", ALL_PROJECTS);
     reloadConfig();
@@ -232,12 +213,55 @@
     plugin
         .getSysInjector()
         .getInstance(ReplicationQueue.class)
-        .scheduleFullSync(project, null, new ReplicationState(pushResultProcessing), true);
+        .scheduleFullSync(project, null, new ReplicationState(NO_OP), true);
 
     assertThat(listReplicationTasks(".*all.*")).hasSize(1);
   }
 
   @Test
+  public void shouldMatchTemplatedURL() throws Exception {
+    createTestProject(project + "replica");
+
+    setReplicationDestination("foo", "replica", ALL_PROJECTS);
+    reloadConfig();
+
+    String urlMatch = gitPath.resolve("${name}" + "replica" + ".git").toString();
+    String expectedURI = gitPath.resolve(project + "replica" + ".git").toString();
+
+    plugin
+        .getSysInjector()
+        .getInstance(ReplicationQueue.class)
+        .scheduleFullSync(project, urlMatch, new ReplicationState(NO_OP), true);
+
+    assertThat(listReplicationTasks(".*all.*")).hasSize(1);
+    for (ReplicationTasksStorage.ReplicateRefUpdate task : tasksStorage.list()) {
+      assertThat(task.uri).isEqualTo(expectedURI);
+    }
+  }
+
+  @Test
+  public void shouldMatchRealURL() throws Exception {
+    createTestProject(project + "replica");
+
+    setReplicationDestination("foo", "replica", ALL_PROJECTS);
+    reloadConfig();
+
+    String urlMatch = gitPath.resolve(project + "replica" + ".git").toString();
+    String expectedURI = urlMatch;
+
+    plugin
+        .getSysInjector()
+        .getInstance(ReplicationQueue.class)
+        .scheduleFullSync(project, urlMatch, new ReplicationState(NO_OP), true);
+
+    assertThat(listReplicationTasks(".*")).hasSize(1);
+    for (ReplicationTasksStorage.ReplicateRefUpdate task : tasksStorage.list()) {
+      assertThat(task.uri).isEqualTo(expectedURI);
+    }
+    assertThat(tasksStorage.list()).isNotEmpty();
+  }
+
+  @Test
   public void shouldReplicateHeadUpdate() throws Exception {
     setReplicationDestination("foo", "replica", ALL_PROJECTS);
     reloadConfig();