From 66d61a5fe1d4fc8c3c8c6e3d671eaa6dc6073e65 Mon Sep 17 00:00:00 2001
From: Dave Borowitz <dborowitz@google.com>
Date: Mon, 10 Jul 2017 13:25:13 -0400
Subject: [PATCH] MergeOp: Give slightly more helpful generic error messages

Change-Id: I482a0588e0ffc9bf22d741ec219c9a4cfb9d490f
---
 .../com/google/gerrit/server/git/MergeOp.java | 40 ++++++++++++++++++-
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
index 353cba2725..ff5c5ed214 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
@@ -63,6 +63,7 @@ import com.google.gerrit.server.git.strategy.SubmitStrategyFactory;
 import com.google.gerrit.server.git.strategy.SubmitStrategyListener;
 import com.google.gerrit.server.git.validators.MergeValidationException;
 import com.google.gerrit.server.git.validators.MergeValidators;
+import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.SubmitRuleOptions;
@@ -233,6 +234,7 @@ public class MergeOp implements AutoCloseable {
   private final Provider<MergeOpRepoManager> ormProvider;
   private final NotifyUtil notifyUtil;
   private final RetryHelper retryHelper;
+  private final NotesMigration notesMigration;
 
   private Timestamp ts;
   private RequestId submissionId;
@@ -260,7 +262,8 @@ public class MergeOp implements AutoCloseable {
       Provider<MergeOpRepoManager> ormProvider,
       NotifyUtil notifyUtil,
       TopicMetrics topicMetrics,
-      RetryHelper retryHelper) {
+      RetryHelper retryHelper,
+      NotesMigration notesMigration) {
     this.cmUtil = cmUtil;
     this.batchUpdateFactory = batchUpdateFactory;
     this.internalUserFactory = internalUserFactory;
@@ -273,6 +276,7 @@ public class MergeOp implements AutoCloseable {
     this.notifyUtil = notifyUtil;
     this.retryHelper = retryHelper;
     this.topicMetrics = topicMetrics;
+    this.notesMigration = notesMigration;
   }
 
   @Override
@@ -592,7 +596,7 @@ public class MergeOp implements AutoCloseable {
       if (e.getCause() instanceof IntegrationException) {
         msg = e.getCause().getMessage();
       } else {
-        msg = "Error submitting change" + (cs.size() != 1 ? "s" : "");
+        msg = genericMergeError(cs);
       }
       throw new IntegrationException(msg, e);
     }
@@ -892,6 +896,38 @@ public class MergeOp implements AutoCloseable {
     }
   }
 
+  private String genericMergeError(ChangeSet cs) {
+    int c = cs.size();
+    if (c == 1) {
+      return "Error submitting change";
+    }
+    int p;
+    try {
+      p = cs.projects().size();
+    } catch (OrmException e) {
+      log.debug("Error looking up projects for " + cs, e);
+      return "Error submitting changes";
+    }
+    if (p == 1) {
+      if (!notesMigration.fuseUpdates()) {
+        // No fused updates: any subset of changes might or might not have been submitted, so don't
+        // make any strong claims.
+        return "Error submitting changes";
+      }
+      // Fused updates: it's correct to say that none of the n changes were submitted.
+      return "Error submitting " + c + " changes";
+    }
+    // Multiple projects involved, but we don't know at this point what failed. At least give the
+    // user a heads up that some changes may be unsubmitted, even if the change screen they land on
+    // after the error message says that this particular change was submitted.
+    return "Error submitting some of the "
+        + c
+        + " changes to one or more of the "
+        + p
+        + " projects involved; some projects may have submitted successfully, but others may have"
+        + " failed";
+  }
+
   private void logDebug(String msg, Object... args) {
     if (log.isDebugEnabled()) {
       log.debug(submissionId + msg, args);