Merge branch 'stable-2.14' into stable-2.15v2.15.1

* stable-2.14:
  Fix creating missing repository
  Fix project deletion logs
  Use logger built-in formatter
  Fix project creation logs

Change-Id: Idacfd3fa54d7ff5e974bc8dae41979d1f0d9b6b5

5 files changed, 90 insertions, 86 deletions

diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
index 55c7072..8b6b8fc 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
@@ -75,11 +75,9 @@ public class AutoReloadConfigDecorator implements ReplicationConfig {
           this.currentConfig = newConfig;
           this.currentConfigTs = lastModified;
           log.info(
-              "Configuration reloaded: "
-                  + currentConfig.getDestinations(FilterType.ALL).size()
-                  + " destinations, "
-                  + discarded
-                  + " replication events discarded");
+              "Configuration reloaded: {} destinations, {} replication events discarded",
+              currentConfig.getDestinations(FilterType.ALL).size(),
+              discarded);
         }
       }
     } catch (Exception e) {
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 926d36f..0cee37c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
@@ -136,7 +136,7 @@ public class Destination {
           builder.add(g.getUUID());
           addRecursiveParents(g.getUUID(), builder, groupIncludeCache);
         } else {
-          repLog.warn(String.format("Group \"%s\" not recognized, removing from authGroup", name));
+          repLog.warn("Group \"{}\" not recognized, removing from authGroup", name);
         }
       }
       remoteUser = replicationUserFactory.create(new ListGroupMembership(builder.build()));
@@ -215,7 +215,7 @@ public class Destination {
   public int shutdown() {
     int cnt = 0;
     if (pool != null) {
-      repLog.warn(String.format("Cancelling replication events"));
+      repLog.warn("Cancelling replication events");
 
       cnt = pool.shutdownNow().size();
       pool = null;
@@ -552,8 +552,7 @@ public class Destination {
         } else if (remoteNameStyle.equals("basenameOnly")) {
           name = FilenameUtils.getBaseName(name);
         } else if (!remoteNameStyle.equals("slash")) {
-          repLog.debug(
-              String.format("Unknown remoteNameStyle: %s, falling back to slash", remoteNameStyle));
+          repLog.debug("Unknown remoteNameStyle: {}, falling back to slash", remoteNameStyle);
         }
         String replacedPath =
             ReplicationQueue.replaceName(uri.getPath(), name, isSingleProjectMatch());
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 8521910..1efad4f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
@@ -211,10 +211,10 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning {
     if (ALL_REFS.equals(ref)) {
       delta.clear();
       pushAllRefs = true;
-      repLog.trace("Added all refs for replication to " + uri);
+      repLog.trace("Added all refs for replication to {}", uri);
     } else if (!pushAllRefs) {
       delta.add(ref);
-      repLog.trace("Added ref " + ref + " for replication to " + uri);
+      repLog.trace("Added ref {} for replication to {}", ref, uri);
     }
   }
 
@@ -298,13 +298,13 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning {
     if (!pool.requestRunway(this)) {
       if (!canceled) {
         repLog.info(
-            "Rescheduling replication to " + uri + " to avoid collision with an in-flight push.");
+            "Rescheduling replication to {} to avoid collision with an in-flight push.", uri);
         pool.reschedule(this, Destination.RetryReason.COLLISION);
       }
       return;
     }
 
-    repLog.info("Replication to " + uri + " started...");
+    repLog.info("Replication to {} started...", uri);
     Timer1.Context context = metrics.start(config.getName());
     try {
       long startedAt = context.getStartTime();
@@ -314,15 +314,11 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning {
       runImpl();
       long elapsed = NANOSECONDS.toMillis(context.stop());
       repLog.info(
-          "Replication to "
-              + uri
-              + " completed in "
-              + (elapsed)
-              + "ms, "
-              + (delay)
-              + "ms delay, "
-              + retryCount
-              + " retries");
+          "Replication to {} completed in {}ms, {}ms delay, {} retries",
+          uri,
+          elapsed,
+          delay,
+          retryCount);
     } catch (RepositoryNotFoundException e) {
       stateLog.error(
           "Cannot replicate " + projectName + "; Local repository error: " + e.getMessage(),
@@ -338,7 +334,7 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning {
           || msg.contains("Git repository not found")) {
         createRepository();
       } else {
-        repLog.error("Cannot replicate " + projectName + "; Remote repository error: " + msg);
+        repLog.error("Cannot replicate {}; Remote repository error: {}", projectName, msg);
       }
 
     } catch (NoRemoteRepositoryException e) {
@@ -348,12 +344,12 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning {
     } catch (TransportException e) {
       Throwable cause = e.getCause();
       if (cause instanceof JSchException && cause.getMessage().startsWith("UnknownHostKey:")) {
-        repLog.error("Cannot replicate to " + uri + ": " + cause.getMessage());
+        repLog.error("Cannot replicate to {}: {}", uri, cause.getMessage());
       } else if (e instanceof LockFailureException) {
         lockRetryCount++;
         // The LockFailureException message contains both URI and reason
         // for this failure.
-        repLog.error("Cannot replicate to " + e.getMessage());
+        repLog.error("Cannot replicate to {}: {}", uri, e.getMessage());
 
         // The remote push operation should be retried.
         if (lockRetryCount <= maxLockRetries) {
@@ -364,16 +360,16 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning {
           }
         } else {
           repLog.error(
-              "Giving up after "
-                  + lockRetryCount
-                  + " of this error during replication to "
-                  + e.getMessage());
+              "Giving up after {} occurrences of this error: {} during replication to {}",
+              lockRetryCount,
+              e.getMessage(),
+              uri);
         }
       } else {
         if (canceledWhileRunning.get()) {
           logCanceledWhileRunningException(e);
         } else {
-          repLog.error("Cannot replicate to " + uri, e);
+          repLog.error("Cannot replicate to {}", uri, e);
           // The remote push operation should be retried.
           pool.reschedule(this, Destination.RetryReason.TRANSPORT_ERROR);
         }
@@ -391,22 +387,22 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning {
   }
 
   private void logCanceledWhileRunningException(TransportException e) {
-    repLog.info("Cannot replicate to " + uri + ". It was canceled while running", e);
+    repLog.info("Cannot replicate to {}. It was canceled while running", uri, e);
   }
 
   private void createRepository() {
     if (pool.isCreateMissingRepos()) {
       try {
         Ref head = git.exactRef(Constants.HEAD);
-        if (replicationQueue.createProject(projectName, head != null ? head.getName() : null)) {
-          repLog.warn("Missing repository created; retry replication to " + uri);
+        if (replicationQueue.createProject(projectName, head != null ? getName(head) : null)) {
+          repLog.warn("Missing repository created; retry replication to {}", uri);
           pool.reschedule(this, Destination.RetryReason.REPOSITORY_MISSING);
         } else {
           repLog.warn(
-              "Missing repository could not be created when replicating "
-                  + uri
-                  + ". You can only create missing repositories locally, over SSH or when "
-                  + "using adminUrl in replication.config. See documentation for more information.");
+              "Missing repository could not be created when replicating {}. "
+                  + "You can only create missing repositories locally, over SSH or when "
+                  + "using adminUrl in replication.config. See documentation for more information.",
+              uri);
         }
       } catch (IOException ioe) {
         stateLog.error(
@@ -419,6 +415,14 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning {
     }
   }
 
+  private String getName(Ref ref) {
+    Ref target = ref;
+    while (target.isSymbolic()) {
+      target = target.getTarget();
+    }
+    return target.getName();
+  }
+
   private void runImpl() throws IOException, PermissionBackendException {
     PushResult res;
     try (Transport tn = Transport.open(git, uri)) {
@@ -441,7 +445,7 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning {
       return new PushResult();
     }
 
-    repLog.info("Push to " + uri + " references: " + todo);
+    repLog.info("Push to {} references: {}", uri, todo);
 
     return tn.push(NullProgressMonitor.INSTANCE, todo);
   }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java
index 82e68ed..bec4f20 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java
@@ -82,11 +82,11 @@ public class ReplicationFileBasedConfig implements ReplicationConfig {
   private List<Destination> allDestinations(DestinationFactory destinationFactory)
       throws ConfigInvalidException, IOException {
     if (!config.getFile().exists()) {
-      log.warn("Config file " + config.getFile() + " does not exist; not replicating");
+      log.warn("Config file {} does not exist; not replicating", config.getFile());
       return Collections.emptyList();
     }
     if (config.getFile().length() == 0) {
-      log.info("Config file " + config.getFile() + " is empty; not replicating");
+      log.info("Config file {} is empty; not replicating", config.getFile());
       return Collections.emptyList();
     }
 
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java
index 226c6fb..30aff44 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java
@@ -101,7 +101,7 @@ public class ReplicationQueue
     running = false;
     int discarded = config.shutdown();
     if (discarded > 0) {
-      repLog.warn(String.format("Canceled %d replication events during shutdown", discarded));
+      repLog.warn("Canceled {} replication events during shutdown", discarded);
     }
   }
 
@@ -194,7 +194,7 @@ public class ReplicationQueue
         try {
           uri = new URIish(url);
         } catch (URISyntaxException e) {
-          repLog.warn(String.format("adminURL '%s' is invalid: %s", url, e.getMessage()));
+          repLog.warn("adminURL '{}' is invalid: {}", url, e.getMessage());
           continue;
         }
 
@@ -202,13 +202,13 @@ public class ReplicationQueue
           String path =
               replaceName(uri.getPath(), projectName.get(), config.isSingleProjectMatch());
           if (path == null) {
-            repLog.warn(String.format("adminURL %s does not contain ${name}", uri));
+            repLog.warn("adminURL {} does not contain ${name}", uri);
             continue;
           }
 
           uri = uri.setPath(path);
           if (!isSSH(uri)) {
-            repLog.warn(String.format("adminURL '%s' is invalid: only SSH is supported", uri));
+            repLog.warn("adminURL '{}' is invalid: only SSH is supported", uri);
             continue;
           }
         }
@@ -238,17 +238,14 @@ public class ReplicationQueue
       gerritAdmin.createProject(replicateURI, projectName, head);
     } else if (!replicateURI.isRemote()) {
       createLocally(replicateURI, head);
-      repLog.info("Created local repository: " + replicateURI);
     } else if (isSSH(replicateURI)) {
       createRemoteSsh(replicateURI, head);
-      repLog.info("Created remote repository: " + replicateURI);
     } else {
       repLog.warn(
-          String.format(
-              "Cannot create new project on remote site %s."
-                  + " Only local paths and SSH URLs are supported"
-                  + " for remote repository creation",
-              replicateURI));
+          "Cannot create new project on remote site {}."
+              + " Only local paths and SSH URLs are supported"
+              + " for remote repository creation",
+          replicateURI);
       return false;
     }
     return true;
@@ -258,13 +255,14 @@ public class ReplicationQueue
     try (Repository repo = new FileRepository(uri.getPath())) {
       repo.create(true /* bare */);
 
-      if (head != null) {
+      if (head != null && head.startsWith(Constants.R_REFS)) {
         RefUpdate u = repo.updateRef(Constants.HEAD);
         u.disableRefLog();
         u.link(head);
       }
+      repLog.info("Created local repository: {}", uri);
     } catch (IOException e) {
-      repLog.error(String.format("Error creating local repository %s:\n", uri.getPath()), e);
+      repLog.error("Error creating local repository {}:\n", uri.getPath(), e);
     }
   }
 
@@ -277,14 +275,17 @@ public class ReplicationQueue
     OutputStream errStream = sshHelper.newErrorBufferStream();
     try {
       sshHelper.executeRemoteSsh(uri, cmd, errStream);
+      repLog.info("Created remote repository: {}", uri);
     } catch (IOException e) {
       repLog.error(
-          String.format(
-              "Error creating remote repository at %s:\n"
-                  + "  Exception: %s\n"
-                  + "  Command: %s\n"
-                  + "  Output: %s",
-              uri, e, cmd, errStream),
+          "Error creating remote repository at {}:\n"
+              + "  Exception: {}\n"
+              + "  Command: {}\n"
+              + "  Output: {}",
+          uri,
+          e,
+          cmd,
+          errStream,
           e);
     }
   }
@@ -295,25 +296,23 @@ public class ReplicationQueue
       repLog.info("Deleted remote repository: " + replicateURI);
     } else if (!replicateURI.isRemote()) {
       deleteLocally(replicateURI);
-      repLog.info("Deleted local repository: " + replicateURI);
     } else if (isSSH(replicateURI)) {
       deleteRemoteSsh(replicateURI);
-      repLog.info("Deleted remote repository: " + replicateURI);
     } else {
       repLog.warn(
-          String.format(
-              "Cannot delete project on remote site %s."
-                  + " Only local paths and SSH URLs are supported"
-                  + " for remote repository deletion",
-              replicateURI));
+          "Cannot delete project on remote site {}. "
+              + "Only local paths and SSH URLs are supported"
+              + " for remote repository deletion",
+          replicateURI);
     }
   }
 
   private static void deleteLocally(URIish uri) {
     try {
       recursivelyDelete(new File(uri.getPath()));
+      repLog.info("Deleted local repository: {}", uri);
     } catch (IOException e) {
-      repLog.error(String.format("Error deleting local repository %s:\n", uri.getPath()), e);
+      repLog.error("Error deleting local repository {}:\n", uri.getPath(), e);
     }
   }
 
@@ -341,14 +340,17 @@ public class ReplicationQueue
     OutputStream errStream = sshHelper.newErrorBufferStream();
     try {
       sshHelper.executeRemoteSsh(uri, cmd, errStream);
+      repLog.info("Deleted remote repository: {}", uri);
     } catch (IOException e) {
       repLog.error(
-          String.format(
-              "Error deleting remote repository at %s:\n"
-                  + "  Exception: %s\n"
-                  + "  Command: %s\n"
-                  + "  Output: %s",
-              uri, e, cmd, errStream),
+          "Error deleting remote repository at {}:\n"
+              + "  Exception: {}\n"
+              + "  Command: {}\n"
+              + "  Output: {}",
+          uri,
+          e,
+          cmd,
+          errStream,
           e);
     }
   }
@@ -362,11 +364,10 @@ public class ReplicationQueue
       updateHeadRemoteSsh(replicateURI, newHead);
     } else {
       repLog.warn(
-          String.format(
-              "Cannot update HEAD of project on remote site %s."
-                  + " Only local paths and SSH URLs are supported"
-                  + " for remote HEAD update.",
-              replicateURI));
+          "Cannot update HEAD of project on remote site {}."
+              + " Only local paths and SSH URLs are supported"
+              + " for remote HEAD update.",
+          replicateURI);
     }
   }
 
@@ -379,12 +380,15 @@ public class ReplicationQueue
       sshHelper.executeRemoteSsh(uri, cmd, errStream);
     } catch (IOException e) {
       repLog.error(
-          String.format(
-              "Error updating HEAD of remote repository at %s to %s:\n"
-                  + "  Exception: %s\n"
-                  + "  Command: %s\n"
-                  + "  Output: %s",
-              uri, newHead, e, cmd, errStream),
+          "Error updating HEAD of remote repository at {} to {}:\n"
+              + "  Exception: {}\n"
+              + "  Command: {}\n"
+              + "  Output: {}",
+          uri,
+          newHead,
+          e,
+          cmd,
+          errStream,
           e);
     }
   }
@@ -396,8 +400,7 @@ public class ReplicationQueue
         u.link(newHead);
       }
     } catch (IOException e) {
-      repLog.error(
-          String.format("Failed to update HEAD of repository %s to %s", uri.getPath(), newHead), e);
+      repLog.error("Failed to update HEAD of repository {} to {}", uri.getPath(), newHead, e);
     }
   }