Fix updating changes that fail to merge

If a submitted change fails to merge due an error (e.g. inconsistent
data or a pre merge validation found an issue) the change state should
be set back to 'New', but it stays as 'Submitted, Merge Pending'. It
should also display a proper error message to the user doing the
submit and add a change message to the change that contains the error
message. Both is currently not happening and the user has no idea why
the change got stuck in 'Submitted, Merge Pending' state.

This change ensures that all changes which cannot be merged due to an
error are properly updated.

Conflicts:
	gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java

Change-Id: Idd8da6a4cd089d2a7b09cbb60342612f1c5256e9
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
(cherry picked from commit 851f7017fd8728616406c605bd5b906706894bc7)
Reviewed-by: Ismo Haataja <ismo.haataja@digia.com>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@theqtcompany.com>

1 files changed, 16 insertions, 0 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 f704434a54..c2921e7373 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
@@ -24,6 +24,7 @@ import com.google.common.base.Objects;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.ListMultimap;
+import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 import com.google.gerrit.common.ChangeHooks;
 import com.google.gerrit.common.data.Capable;
@@ -138,6 +139,7 @@ public class MergeOp {
   private final ListMultimap<SubmitType, CodeReviewCommit> toMerge;
   private final List<CodeReviewCommit> potentiallyStillSubmittable;
   private final Map<Change.Id, CodeReviewCommit> commits;
+  private final List<Change> toUpdate;
   private ReviewDb db;
   private Repository repo;
   private RevWalk rw;
@@ -194,6 +196,7 @@ public class MergeOp {
     toMerge = ArrayListMultimap.create();
     potentiallyStillSubmittable = new ArrayList<CodeReviewCommit>();
     commits = new HashMap<Change.Id, CodeReviewCommit>();
+    toUpdate = Lists.newArrayList();
   }
 
   public void verifyMergeability(Change change) throws NoSuchProjectException {
@@ -316,6 +319,8 @@ public class MergeOp {
         toMerge.putAll(toMergeNextTurn);
       }
 
+      updateChangeStatus(toUpdate);
+
       for (final CodeReviewCommit commit : potentiallyStillSubmittableOnNextRun) {
         final Capable capable = isSubmitStillPossible(commit);
         if (capable != Capable.OK) {
@@ -487,6 +492,7 @@ public class MergeOp {
       if (chg.currentPatchSetId() == null) {
         commits.put(changeId, CodeReviewCommit
             .error(CommitMergeStatus.NO_PATCH_SET));
+        toUpdate.add(chg);
         continue;
       }
 
@@ -500,6 +506,7 @@ public class MergeOp {
           || ps.getRevision().get() == null) {
         commits.put(changeId, CodeReviewCommit
             .error(CommitMergeStatus.NO_PATCH_SET));
+        toUpdate.add(chg);
         continue;
       }
 
@@ -510,6 +517,7 @@ public class MergeOp {
       } catch (IllegalArgumentException iae) {
         commits.put(changeId, CodeReviewCommit
             .error(CommitMergeStatus.NO_PATCH_SET));
+        toUpdate.add(chg);
         continue;
       }
 
@@ -525,6 +533,7 @@ public class MergeOp {
         //
         commits.put(changeId, CodeReviewCommit
             .error(CommitMergeStatus.REVISION_GONE));
+        toUpdate.add(chg);
         continue;
       }
 
@@ -535,6 +544,7 @@ public class MergeOp {
         log.error("Invalid commit " + id.name() + " on " + chg.getKey(), e);
         commits.put(changeId, CodeReviewCommit
             .error(CommitMergeStatus.REVISION_GONE));
+        toUpdate.add(chg);
         continue;
       }
 
@@ -548,6 +558,7 @@ public class MergeOp {
         } catch (Exception e) {
           commits.put(changeId, CodeReviewCommit
               .error(CommitMergeStatus.INVALID_PROJECT_CONFIGURATION));
+          toUpdate.add(chg);
           continue;
         }
         final Project.NameKey oldParent =
@@ -557,6 +568,7 @@ public class MergeOp {
           if (newParent != null) {
             commits.put(changeId, CodeReviewCommit
                 .error(CommitMergeStatus.INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT));
+            toUpdate.add(chg);
             continue;
           }
         } else {
@@ -565,6 +577,7 @@ public class MergeOp {
             if (psa == null) {
               commits.put(changeId, CodeReviewCommit
                   .error(CommitMergeStatus.SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN));
+              toUpdate.add(chg);
               continue;
             }
             final IdentifiedUser submitter =
@@ -572,12 +585,14 @@ public class MergeOp {
             if (!submitter.getCapabilities().canAdministrateServer()) {
               commits.put(changeId, CodeReviewCommit
                   .error(CommitMergeStatus.SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN));
+              toUpdate.add(chg);
               continue;
             }
 
             if (projectCache.get(newParent) == null) {
               commits.put(changeId, CodeReviewCommit
                   .error(CommitMergeStatus.INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND));
+              toUpdate.add(chg);
               continue;
             }
           }
@@ -613,6 +628,7 @@ public class MergeOp {
       if (submitType == null) {
         commits.put(changeId,
             CodeReviewCommit.error(CommitMergeStatus.NO_SUBMIT_TYPE));
+        toUpdate.add(chg);
         continue;
       }