Check parents of a merge commit before staging

Chech that parents of a merge commit are merged. If they are not, staging is not allowed.

Fixes: QTQAINFRA-3341
Change-Id: I3e6f48e76bfb55a4a07c90afc9289c8d1a7ff07c
Reviewed-by: Paul Wicking <[email protected]>

Author: vs-systencess

src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtStage.java

@@ -17,8 +17,10 @@
 import com.google.gerrit.extensions.webui.UiAction;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gerrit.reviewdb.client.Change.Status;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.ProjectUtil;
 import com.google.gerrit.server.account.AccountResolver;
@@ -33,19 +35,25 @@
 import com.google.gerrit.server.project.NoSuchRefException;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gerrit.server.submit.IntegrationException;
 import com.google.gerrit.server.submit.MergeOp;
 import com.google.gerrit.server.update.UpdateException;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 import com.google.inject.Singleton;
+
 import java.io.IOException;
+import java.util.List;
 import java.util.Map;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.RefUpdate.Result;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
 
 @Singleton
 public class QtStage
@@ -70,6 +78,7 @@ private Output(Change c) {
   private final GitReferenceUpdated referenceUpdated;
   private final QtCherryPickPatch qtCherryPickPatch;
   private final QtUtil qtUtil;
+  private final Provider<InternalChangeQuery> queryProvider;
 
   private final AccountResolver accountResolver;
   private final String label;
@@ -90,7 +99,8 @@ private Output(Change c) {
       ProjectCache projectCache,
       GitReferenceUpdated referenceUpdated,
       QtCherryPickPatch qtCherryPickPatch,
-      QtUtil qtUtil) {
+      QtUtil qtUtil,
+      Provider<InternalChangeQuery> queryProvider) {
 
     this.repoManager = repoManager;
     this.permissionBackend = permissionBackend;
@@ -105,6 +115,7 @@ private Output(Change c) {
     this.referenceUpdated = referenceUpdated;
     this.qtCherryPickPatch = qtCherryPickPatch;
     this.qtUtil = qtUtil;
+    this.queryProvider = queryProvider;
   }
 
   @Override
@@ -171,6 +182,8 @@ private Change changeToStaging(RevisionResource rsrc, IdentifiedUser submitter,
       if (sourceId == null)
         throw new NoSuchRefException("Invalid Revision: " + rsrc.getPatchSet().getRevision().get());
 
+      checkParents(git, rsrc);
+
       changeData = changeDataFactory.create(change);
       MergeOp.checkSubmitRule(changeData, false);
 
@@ -214,6 +227,36 @@ private Change changeToStaging(RevisionResource rsrc, IdentifiedUser submitter,
     }
   }
 
+  private void checkParents(RevisionResource resource) throws ResourceConflictException {
+    try (final Repository repository = repoManager.openRepository(resource.getProject())) {
+      checkParents(repository, resource);
+    } catch (IOException e) {
+      throw new ResourceConflictException("Can not read repository.", e);
+    }
+  }
+
+  private void checkParents(Repository repository, RevisionResource resource) throws ResourceConflictException {
+    try (final RevWalk rw = new RevWalk(repository)) {
+      final PatchSet ps = resource.getPatchSet();
+      final RevCommit rc = rw.parseCommit(ObjectId.fromString(ps.getRevision().get()));
+      if (rc.getParentCount() < 2) {
+          return;
+      }
+      for (final RevCommit parent : rc.getParents()) {
+        final List<ChangeData> changes =
+                queryProvider.get().enforceVisibility(true).byProjectCommit(resource.getProject(), parent);
+        for (ChangeData cd : changes) {
+          final Change change = cd.change();
+          if (change.getStatus() != Status.MERGED) {
+            throw new ResourceConflictException(String.format("Can not stage: Parent \"%s\" of a merged commit is not merged.", parent.name()));
+          }
+        }
+      }
+    } catch (IOException e) {
+      throw new ResourceConflictException("Can not read repository.", e);
+    }
+  }
+
   @Override
   public UiAction.Description getDescription(RevisionResource resource) {
     Change change = resource.getChange();
@@ -223,6 +266,12 @@ public UiAction.Description getDescription(RevisionResource resource) {
         || !resource.permissions().testOrFalse(ChangePermission.QT_STAGE)) {
       return null; // submit not visible
     }
+    try {
+        checkParents(resource);
+    } catch (ResourceConflictException e) {
+        logger.atWarning().log("Parent(s) check failed. %s", e.getMessage());
+        return null;
+    }
     try {
       if (!projectCache.checkedGet(resource.getProject()).statePermitsWrite()) {
         return null; // stage not visible

src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApproveIT.java

@@ -141,9 +141,16 @@ public void avoid_Double_Handling_Of_Change_On_Merge_Of_Merge() throws Exception
     final PushOneCommit.Result m1 = pushCommit("master", "m1-commitmsg", "m1-file", "m1-content");
     approve(m1.getChangeId());
 
+    // make another change on master branch
+    final PushOneCommit.Result m2 = pushCommit("master", "m2-commitmsg", "m2-file", "m2-content");
+    approve(m2.getChangeId());
+    QtStage(m2);
+    QtNewBuild("master", "merge-build-001");
+    QtApproveBuild("master", "merge-build-001");
+
     // merge feature branch into master
     final PushOneCommit mm = pushFactory.create(admin.newIdent(), testRepo);
-    mm.setParents(ImmutableList.of(f1.getCommit(), m1.getCommit()));
+    mm.setParents(ImmutableList.of(f1.getCommit(), m2.getCommit()));
     final PushOneCommit.Result m = mm.to("refs/for/master");
     m.assertOkStatus();
     approve(m.getChangeId());
@@ -154,12 +161,13 @@ public void avoid_Double_Handling_Of_Change_On_Merge_Of_Merge() throws Exception
     QtStage(m);
 
     // Create build and approve it
-    QtNewBuild("master", "merge-build-000");
-    QtApproveBuild("master", "merge-build-000");
+    QtNewBuild("master", "merge-build-002");
+    QtApproveBuild("master", "merge-build-002");
 
     final Changes changes = gApi.changes();
     assertThat(changes.id(project.get(), "feature", f1.getChangeId()).get(CURRENT_REVISION).status).isEqualTo(ChangeStatus.MERGED);
     assertThat(changes.id(project.get(), "master", m1.getChangeId()).get(CURRENT_REVISION).status).isEqualTo(ChangeStatus.MERGED);
+    assertThat(changes.id(project.get(), "master", m2.getChangeId()).get(CURRENT_REVISION).status).isEqualTo(ChangeStatus.MERGED);
     assertThat(changes.id(project.get(), "master", m.getChangeId()).get(CURRENT_REVISION).status).isEqualTo(ChangeStatus.MERGED);
   }
 

src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtStageIT.java

@@ -217,6 +217,22 @@ public void errorStage_Not_Reviewed() throws Exception {
     assertThat(response.getEntityContent()).contains("needs Code-Review");
   }
 
+  @Test
+  public void errorStage_Parent_Not_Merged() throws Exception {
+    RevCommit initialHead = getRemoteHead();
+
+    // make a change on feature branch without submit
+    final PushOneCommit.Result f1 = pushCommit("feature", "f1-commitmsg", "f1-file", "f1-content");
+
+    // merge feature branch into master
+    final PushOneCommit mm = pushFactory.create(admin.newIdent(), testRepo);
+    mm.setParents(ImmutableList.of(f1.getCommit(), initialHead));
+    final PushOneCommit.Result m = mm.to("refs/for/master");
+    approve(m.getChangeId());
+
+    qtStageExpectFail(m, initialHead, initialHead, HttpStatus.SC_CONFLICT);
+  }
+
   @Test
   public void errorAmend_Status_Staged() throws Exception {
     RevCommit initialHead = getRemoteHead();