PushOne: Improve format of log of references to be pushed

The references to be pushed are logged by just passing the list
of RemoteRefUpdate instances to the logger. This results in the
List's default implementation of toString() being called, which
renders each object in a comma-separated list.

Each object is rendered by RemoteRefUpdate's toString() method
which includes some fields that are not relevant here, or omits
some information that might be useful. For example:

  [RemoteRefUpdate[remoteName=refs/meta/config, NOT_ATTEMPTED,
  (null)...a7038eb8827cfd29cb3fca335e882f4d5ed09b62, srcRef=refs/meta/config,
  forceUpdate, message=null]

- The 'remoteName' is the name on the destination. In this case it's
  the same as 'srcRef' but could be different if replication has been
  configured to push to a different refname. It is potentially confusing
  to have them not logged next to each other.
- '(null)...a7038eb8827cfd29cb3fca335e882f4d5ed09b62' shows the old
  and new IDs; this is in the standard git ref update format aside
  from '(null)' which is shown instead of zeros.
- 'forceUpdate' is only included when true; this is the same for
  the 'fastForward' field.
- the update has a flag indicating if it's a delete, but this is
  not included in the output at all.
- 'message' is always null for udates by replication so it's not
  necessary to include in the log.

Add a custom method to format the refs for logging, with the information
presented in a clearer way. Continue to log it on a single line, comma-
separated. This might not be ideal for human readers, but makes it easier
when processing the logs with grep or any other tool.

With this change, the logged information looks like:

  RemoteRefUpdate{refSpec=refs/meta/config:refs/meta/config,
  status=NOT_ATTEMPTED, id=(null)..a7038eb8827cfd29cb3fca335e882f4d5ed09b62
  force=yes, delete=no, ffwd=no}

Now the irrelevant information (message) is removed, missing information
(delete) is added, and the source and destination refs are presented in
a single "refSpec" field.

Change-Id: Idd153daf44cd79a0920ea0b72c64ea58d0926f59

1 files changed, 31 insertions, 1 deletions

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 ee29162..4e5cca6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
@@ -16,7 +16,9 @@ package com.googlesource.gerrit.plugins.replication;
 
 import static com.googlesource.gerrit.plugins.replication.ReplicationQueue.repLog;
 import static java.util.concurrent.TimeUnit.NANOSECONDS;
+import static java.util.stream.Collectors.joining;
 
+import com.google.common.base.MoreObjects;
 import com.google.common.base.Throwables;
 import com.google.common.collect.LinkedListMultimap;
 import com.google.common.collect.ListMultimap;
@@ -62,6 +64,7 @@ import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.errors.TransportException;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.NullProgressMonitor;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.CredentialsProvider;
@@ -463,11 +466,38 @@ class PushOne implements ProjectRunnable, CanceledWhileRunning {
       return new PushResult();
     }
 
-    repLog.info("Push to {} references: {}", uri, todo);
+    repLog.info("Push to {} references: {}", uri, refUpdatesForLogging(todo));
 
     return tn.push(NullProgressMonitor.INSTANCE, todo);
   }
 
+  private static String refUpdatesForLogging(List<RemoteRefUpdate> refUpdates) {
+    return refUpdates.stream().map(PushOne::refUpdateForLogging).collect(joining(", "));
+  }
+
+  private static String refUpdateForLogging(RemoteRefUpdate update) {
+    String refSpec = String.format("%s:%s", update.getSrcRef(), update.getRemoteName());
+    String id =
+        String.format(
+            "%s..%s", objectIdToString(update.getExpectedOldObjectId()), update.getNewObjectId());
+    return MoreObjects.toStringHelper(RemoteRefUpdate.class)
+        .add("refSpec", refSpec)
+        .add("status", update.getStatus())
+        .add("id", id)
+        .add("force", booleanToString(update.isForceUpdate()))
+        .add("delete", booleanToString(update.isDelete()))
+        .add("ffwd", booleanToString(update.isFastForward()))
+        .toString();
+  }
+
+  private static String objectIdToString(ObjectId id) {
+    return id != null ? id.getName() : "(null)";
+  }
+
+  private static String booleanToString(boolean b) {
+    return b ? "yes" : "no";
+  }
+
   private List<RemoteRefUpdate> generateUpdates(Transport tn)
       throws IOException, PermissionBackendException {
     ProjectState projectState = projectCache.checkedGet(projectName);