Improved ProgressListener.setDescription() and added some more calls …

…in remoting commands Signed-off-by: Gabriel Roldan <[email protected]>
locationtech · Sep 12, 2018 · 2e0a92f · 2e0a92f
1 parent 2b615b0
commit 2e0a92f
Show file tree

Hide file tree

Showing 16 changed files with 56 additions and 38 deletions.
diff --git a/src/api/src/main/java/org/locationtech/geogig/model/Ref.java b/src/api/src/main/java/org/locationtech/geogig/model/Ref.java
@@ -265,7 +265,7 @@ public int compareTo(Ref o) {
      */
     @Override
     public String toString() {
-        return String.format("Ref[%s -> %s]", name, objectId);
+        return String.format("[%s -> %s]", name, objectId);
     }
 
     public static String append(String namespace, String child) {

diff --git a/src/api/src/main/java/org/locationtech/geogig/model/SymRef.java b/src/api/src/main/java/org/locationtech/geogig/model/SymRef.java
@@ -53,7 +53,7 @@ public String getTarget() {
 
     @Override
     public String toString() {
-        return String.format("SymRef[%s -> Ref[%s -> %s]]", getName(), target, getObjectId());
+        return String.format("%s -> [%s -> %s]", getName(), target, getObjectId());
     }
 
     /**

diff --git a/src/api/src/main/java/org/locationtech/geogig/repository/DefaultProgressListener.java b/src/api/src/main/java/org/locationtech/geogig/repository/DefaultProgressListener.java
@@ -62,14 +62,9 @@ public String getDescription() {
         return description;
     }
 
-    /**
-     * Sets the description of the task.
-     * 
-     * @param description the text to use for the description
-     */
     @Override
-    public void setDescription(String description) {
-        this.description = description;
+    public void setDescription(String format, Object... args) {
+        this.description = String.format(format, args);
     }
 
     /**

diff --git a/src/api/src/main/java/org/locationtech/geogig/repository/ProgressListener.java b/src/api/src/main/java/org/locationtech/geogig/repository/ProgressListener.java
@@ -44,9 +44,10 @@ public default String getProgressDescription() {
     /**
      * Sets the description of the current task being run
      * 
-     * @param description
+     * @param format description format like in {@link String#format}
+     * @param args format arguments like in {@link String#format}
      */
-    void setDescription(String description);
+    void setDescription(String format, Object... args);
 
     /**
      * Notifies this listener that the operation begins.
@@ -60,7 +61,7 @@ public default String getProgressDescription() {
      * 
      */
     void setProgress(float progress);
-    
+
     void incrementBy(float amount);
 
     /**

diff --git a/src/api/src/main/java/org/locationtech/geogig/repository/Remote.java b/src/api/src/main/java/org/locationtech/geogig/repository/Remote.java
@@ -229,7 +229,7 @@ public String getPassword() {
     }
 
     public @Override String toString() {
-        return String.format("%s", getName());
+        return String.format("%s [%s]", getName(), fetch);
     }
 
     /**

diff --git a/src/api/src/main/java/org/locationtech/geogig/repository/SubProgressListener.java b/src/api/src/main/java/org/locationtech/geogig/repository/SubProgressListener.java
@@ -77,14 +77,9 @@ public void setProgress(float progress) {
         parentProgressListener.setProgress(start + (amount * percent));
     }
 
-    /**
-     * Sets the description of the task.
-     * 
-     * @param description the text to use for the description
-     */
     @Override
-    public void setDescription(String description) {
-        parentProgressListener.setDescription(description);
+    public void setDescription(String format, Object... args) {
+        parentProgressListener.setDescription(format, args);
     }
 
     /**

diff --git a/src/api/src/test/java/org/locationtech/geogig/model/RefTest.java b/src/api/src/test/java/org/locationtech/geogig/model/RefTest.java
@@ -43,7 +43,7 @@ public void testConstructor() throws Exception {
     public void testToString() throws Exception {
         Ref testRef = new Ref(Ref.REFS_PREFIX + "commit1", oid);
 
-        assertEquals("Ref[" + testRef.getName() + " -> " + testRef.getObjectId().toString() + "]",
+        assertEquals("[" + testRef.getName() + " -> " + testRef.getObjectId().toString() + "]",
                 testRef.toString());
     }
 

diff --git a/src/api/src/test/java/org/locationtech/geogig/model/SymRefTest.java b/src/api/src/test/java/org/locationtech/geogig/model/SymRefTest.java
@@ -26,7 +26,7 @@ public void testSymRef() {
 
         String symRefString = symRef.toString();
 
-        assertEquals("SymRef[TestRef -> " + "Ref[" + testRef.getName() + " -> "
-                + testRef.getObjectId().toString() + "]]", symRefString);
+        assertEquals("TestRef -> " + "[" + testRef.getName() + " -> "
+                + testRef.getObjectId().toString() + "]", symRefString);
     }
 }
diff --git a/src/cli/core/src/main/java/org/locationtech/geogig/cli/GeogigCLI.java b/src/cli/core/src/main/java/org/locationtech/geogig/cli/GeogigCLI.java
@@ -798,11 +798,11 @@ public void started() {
                 }
 
                 @Override
-                public void setDescription(String s) {
+                public void setDescription(String s, Object... args) {
                     lastRun = platform.nanoTime();
                     try {
                         console.println();
-                        console.println(s);
+                        console.println(String.format(s, args));
                         console.flush();
                     } catch (IOException e) {
                         throw new RuntimeException(e);

diff --git a/src/core/src/main/java/org/locationtech/geogig/porcelain/MergeOp.java b/src/core/src/main/java/org/locationtech/geogig/porcelain/MergeOp.java
@@ -203,6 +203,7 @@ protected MergeReport _call() throws RuntimeException {
 
         // capture original values in case the operation is cancelled
         origHead = currHead.get();
+        getProgressListener().setDescription(String.format("Merge: merging %s onto %s", commits, origHead));
         if (origHead instanceof SymRef) {
             final String currentBranch = ((SymRef) origHead).getTarget();
             origCurrentBranch = command(RefParse.class).setName(currentBranch).call().get();
@@ -290,17 +291,18 @@ protected MergeReport _call() throws RuntimeException {
                     "Conflicted merge.\nCannot merge more than two commits when conflicts exist"
                             + " or features have been modified in several histories");
             for (ObjectId commitId : commits) {
-                progress.setDescription("Merging commit " + commitId);
-
                 if (headRef.getObjectId().isNull()) {
                     // Fast-forward
                     headRef = doFastForwardMerge(headRef, commitId, mergeStatusBuilder);
                     continue;
                 }
-
-                RevCommit headCommit = repository().getCommit(headRef.getObjectId());
+                final RevCommit headCommit = repository().getCommit(headRef.getObjectId());
                 final RevCommit targetCommit = repository().getCommit(commitId);
 
+                progress.setDescription(String.format("Merging commit %s onto %s",
+                        fmt(targetCommit), fmt(headCommit)));
+
+
                 Optional<ObjectId> ancestorCommit = command(FindCommonAncestor.class)
                         .setLeft(headCommit).setRight(targetCommit).call();
 
@@ -346,22 +348,32 @@ protected MergeReport _call() throws RuntimeException {
 
             progress.complete();
         }
-
         if (!mergeStatusBuilder.isChanged()) {
+            progress.setDescription("Merge: complete, nothing to merge.");
             throw new NothingToCommitException("The branch has already been merged.");
         }
         if (noFastForward) {
             mergeStatusBuilder.setFastFoward(false);
         }
+        progress.setDescription("Creating merge commit");
         RevCommit mergeCommit = commit(mergeStatusBuilder.isFastForward());
 
+        progress.setDescription("Merge: created merge commit " + mergeCommit);
         MergeReport result = new MergeReport(mergeCommit, Optional.fromNullable(mergeScenario),
                 oursId, pairs);
 
         return result;
 
     }
 
+    private String fmt(RevCommit c) {
+        String msg = c.getMessage();
+        if (msg.length() > 30) {
+            msg = msg.substring(0, 30) + "...";
+        }
+        return String.format("%s (%s)", c.getId().toString().substring(0, 8), msg);
+    }
+
     private MergeReport abort() {
         command(CleanRefsOp.class).call();
         conflictsDatabase().removeConflicts(null);
@@ -370,6 +382,9 @@ private MergeReport abort() {
 
     private Ref doFastForwardMerge(Ref headRef, ObjectId commitId,
             MergeStatusBuilder mergeStatusBuilder) {
+        getProgressListener().setDescription(String.format("Fast forward merging %s onto %s",
+                commitId.toString().substring(0, 8),
+                headRef.getObjectId().toString().substring(0, 8)));
         if (headRef instanceof SymRef) {
             final String currentBranch = ((SymRef) headRef).getTarget();
             command(UpdateRef.class).setName(currentBranch).setNewValue(commitId).call();

diff --git a/src/core/src/main/java/org/locationtech/geogig/repository/impl/GeogigTransaction.java b/src/core/src/main/java/org/locationtech/geogig/repository/impl/GeogigTransaction.java
@@ -167,8 +167,12 @@ public void commit(ProgressListener listener) throws ConflictsException {
     }
 
     public void commitSyncTransaction() throws ConflictsException {
+        commitSyncTransaction(DefaultProgressListener.NULL);
+    }
+
+    public void commitSyncTransaction(ProgressListener listener) throws ConflictsException {
         context.command(TransactionEnd.class).setAuthor(authorName.orNull(), authorEmail.orNull())
-                .setTransaction(this).setCancel(false).call();
+                .setTransaction(this).setCancel(false).setProgressListener(listener).call();
     }
 
     public void abort() {

diff --git a/src/core/src/test/java/org/locationtech/geogig/test/integration/LogOpTest.java b/src/core/src/test/java/org/locationtech/geogig/test/integration/LogOpTest.java
@@ -133,8 +133,8 @@ public void testComplex() throws Exception {
 
     protected static final ProgressListener SIMPLE_PROGRESS = new DefaultProgressListener() {
         public @Override
-        void setDescription(String msg) {
-            System.err.println(msg);
+        void setDescription(String msg, Object... args) {
+            System.err.printf(msg+"\n", args);
         }
     };
 

diff --git a/src/remoting/src/main/java/org/locationtech/geogig/remotes/PullOp.java b/src/remoting/src/main/java/org/locationtech/geogig/remotes/PullOp.java
@@ -219,14 +219,15 @@ protected PullResult _call() {
         }
 
         getProgressListener().started();
+        getProgressListener().setDescription("Pull: pulling " + remote);
 
         PullResult result = new PullResult();
         TransferSummary fetchResult = command(FetchOp.class)//
                 .addRemote(remote)//
                 .setDepth(depth.or(0))//
                 .setFullDepth(fullDepth)//
                 .setAllRemotes(all)//
-                .setProgressListener(subProgress(80.f))//
+                .setProgressListener(getProgressListener())//
                 .call();
 
         result.setFetchResult(fetchResult);
@@ -242,8 +243,10 @@ protected PullResult _call() {
             }
             final Ref localRemoteRef = localRemoteRefOpt.get();
             if (rebase) {
+                getProgressListener().setDescription("Pull: rebasing...");
                 command(RebaseOp.class).setUpstream(() -> localRemoteRef.getObjectId()).call();
             } else {
+                getProgressListener().setDescription("Pull: merging...");
                 String message = this.message;
                 if (noFastForward && Strings.isNullOrEmpty(message)) {
                     message = String.format("Pull changes from %s:%s onto %s", remote.getName(),
@@ -267,6 +270,7 @@ protected PullResult _call() {
             result.setNewRef(currentBranchFinalState);
         }
 
+        getProgressListener().setDescription("Pull: finished pulling " + remote);
         getProgressListener().complete();
 
         return result;

diff --git a/src/remoting/src/main/java/org/locationtech/geogig/remotes/pack/FetchOp.java b/src/remoting/src/main/java/org/locationtech/geogig/remotes/pack/FetchOp.java
@@ -46,6 +46,8 @@
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 
+import lombok.extern.slf4j.Slf4j;
+
 /**
  * Fetches named heads or tags from one or more other repositories, along with the objects necessary
  * to complete them.
@@ -74,6 +76,7 @@
  * repository under its {@code refs/heads/ or {@code refs/tags} namespaces that were created,
  * deleted, or updated.
  */
+@Slf4j
 public class FetchOp extends AbstractGeoGigOp<TransferSummary> {
 
     private FetchArgs.Builder argsBuilder = new FetchArgs.Builder();
@@ -95,7 +98,7 @@ public class FetchOp extends AbstractGeoGigOp<TransferSummary> {
             }
             try (IRemoteRepo remoteRepo = openRemote(remote)) {
                 Preconditions.checkState(remote.equals(remoteRepo.getInfo()));
-
+                progress.setDescription("Fetching " + remoteRepo.getInfo());
                 final List<LocalRemoteRef> localToRemoteRemoteRefs = resolveRemoteRefs(remoteRepo);
 
                 final PackRequest request = prepareRequest(localRepo, localToRemoteRemoteRefs);
@@ -119,6 +122,7 @@ public class FetchOp extends AbstractGeoGigOp<TransferSummary> {
                 remoteRemoteRefs = updateLocalRemoteRefs(remote, localToRemoteRemoteRefs,
                         args.prune);
                 result.addAll(remote.getFetchURL(), Lists.newArrayList(remoteRemoteRefs));
+                progress.setDescription("Fetched " + remoteRepo.getInfo());
             }
         }
 

diff --git a/src/remoting/src/main/java/org/locationtech/geogig/remotes/pack/PackImpl.java b/src/remoting/src/main/java/org/locationtech/geogig/remotes/pack/PackImpl.java
@@ -104,7 +104,7 @@ private RefDiff applyToPreOrder(PackProcessor target, RefRequest req, Deduplicat
 
         progress.started();
 
-        progress.setDescription("Applying changes of " + req.name);
+        progress.setDescription("Saving missing revision objects changes for " + req.name);
         ObjectReporter objectReport = new ObjectReporter(progress);
 
         // back up current progress indicator

diff --git a/...test/java/org/locationtech/geogig/test/integration/remoting/RemoteRepositoryTestCase.java b/...test/java/org/locationtech/geogig/test/integration/remoting/RemoteRepositoryTestCase.java
@@ -104,8 +104,8 @@
 public abstract class RemoteRepositoryTestCase {
 
     protected static final ProgressListener SIMPLE_PROGRESS = new DefaultProgressListener() {
-        public @Override void setDescription(String msg) {
-            System.err.println(msg);
+        public @Override void setDescription(String msg, Object... args) {
+            System.err.printf(msg + "\n", args);
         }
     };