Fix submit on push for same commit on multiple branches

If createNewCommitForAllInTarget = true is configured a commit might
identify more than one change. When trying to close changes on push
in ReceiveCommits this is not taken under consideration, instead it
tries to close the change of the first patch set that matches the
revision.
This could result in not closing changes even though patch sets
of the changes have been pushed into target.

Bug: Issue 7827
Change-Id: I085cc7129291194ffcf3b465b4db3111105c0efe

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -1275,4 +1275,10 @@ public abstract class AbstractDaemonTest {
     projectsToWatch.add(pwi);
     gApi.accounts().self().setWatchedProjects(projectsToWatch);
   }
+
+  protected void enableCreateNewChangeForAllNotInTarget() throws Exception {
+    ProjectConfig config = projectCache.checkedGet(project).getConfig();
+    config.getProject().setCreateNewChangeForAllNotInTarget(InheritableBoolean.TRUE);
+    saveProjectConfig(project, config);
+  }
 }

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java

@@ -1203,7 +1203,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
     PushOneCommit.Result r = push1.to("refs/for/master");
     r.assertOkStatus();
 
-    //abandon the change
+    // abandon the change
     String changeId = r.getChangeId();
     assertThat(info(changeId).status).isEqualTo(ChangeStatus.NEW);
     gApi.changes().id(changeId).abandon();
@@ -1552,10 +1552,4 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
       assertThat(refUpdate.getMessage()).contains(expectedMessage);
     }
   }
-
-  private void enableCreateNewChangeForAllNotInTarget() throws Exception {
-    ProjectConfig config = projectCache.checkedGet(project).getConfig();
-    config.getProject().setCreateNewChangeForAllNotInTarget(InheritableBoolean.TRUE);
-    saveProjectConfig(project, config);
-  }
 }

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java

@@ -33,6 +33,9 @@ import com.google.gerrit.server.ApprovalsUtil;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.inject.Inject;
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.api.errors.InvalidRemoteException;
+import org.eclipse.jgit.api.errors.TransportException;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
@@ -41,6 +44,7 @@ import org.eclipse.jgit.revwalk.RevObject;
 import org.eclipse.jgit.revwalk.RevTag;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.transport.RefSpec;
+import org.eclipse.jgit.transport.RemoteRefUpdate;
 import org.junit.Test;
 
 @NoHttpd
@@ -204,6 +208,46 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
     assertThat(cd.patchSet(psId).getRevision().get()).isEqualTo(c.name());
   }
 
+  @Test
+  public void mergeOnPushToBranchWithChangeMergedInOther() throws Exception {
+    enableCreateNewChangeForAllNotInTarget();
+    String master = "refs/heads/master";
+    String other = "refs/heads/other";
+    grant(Permission.PUSH, project, master);
+    grant(Permission.CREATE, project, other);
+    grant(Permission.PUSH, project, other);
+    RevCommit masterRev = getRemoteHead();
+    pushCommitTo(masterRev, other);
+    PushOneCommit.Result r = createChange();
+    r.assertOkStatus();
+    RevCommit commit = r.getCommit();
+    pushCommitTo(commit, master);
+    assertCommit(project, master);
+    ChangeData cd =
+        Iterables.getOnlyElement(queryProvider.get().byKey(new Change.Key(r.getChangeId())));
+    assertThat(cd.change().getStatus()).isEqualTo(Change.Status.MERGED);
+
+    RemoteRefUpdate.Status status = pushCommitTo(commit, "refs/for/other");
+    assertThat(status).isEqualTo(RemoteRefUpdate.Status.OK);
+
+    pushCommitTo(commit, other);
+    assertCommit(project, other);
+
+    for (ChangeData c : queryProvider.get().byKey(new Change.Key(r.getChangeId()))) {
+      if (c.change().getDest().get().equals(other)) {
+        assertThat(c.change().getStatus()).isEqualTo(Change.Status.MERGED);
+      }
+    }
+  }
+
+  private RemoteRefUpdate.Status pushCommitTo(RevCommit commit, String ref)
+      throws GitAPIException, InvalidRemoteException, TransportException {
+    return Iterables.getOnlyElement(
+            git().push().setRefSpecs(new RefSpec(commit.name() + ":" + ref)).call())
+        .getRemoteUpdate(ref)
+        .getStatus();
+  }
+
   @Test
   public void mergeOnPushToBranchWithNewPatchset() throws Exception {
     grant(Permission.PUSH, project, "refs/heads/master");

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -2816,17 +2816,20 @@ public class ReceiveCommits {
         rw.parseBody(c);
 
         for (Ref ref : byCommit.get(c.copy())) {
-          existingPatchSets++;
           PatchSet.Id psId = PatchSet.Id.fromRef(ref.getName());
-          bu.addOp(
-              psId.getParentKey(),
-              mergedByPushOpFactory.create(requestScopePropagator, psId, refName));
-          continue COMMIT;
+          Optional<ChangeData> cd = byLegacyId(psId.getParentKey());
+          if (cd.isPresent() && cd.get().change().getDest().equals(branch)) {
+            existingPatchSets++;
+            bu.addOp(
+                psId.getParentKey(),
+                mergedByPushOpFactory.create(requestScopePropagator, psId, refName));
+            continue COMMIT;
+          }
         }
 
         for (String changeId : c.getFooterLines(CHANGE_ID)) {
           if (byKey == null) {
-            byKey = openChangesByBranch(branch);
+            byKey = openChangesByKeyByBranch(branch);
           }
 
           ChangeNotes onto = byKey.get(new Change.Key(changeId.trim()));
@@ -2875,7 +2878,7 @@ public class ReceiveCommits {
     }
   }
 
-  private Map<Change.Key, ChangeNotes> openChangesByBranch(Branch.NameKey branch)
+  private Map<Change.Key, ChangeNotes> openChangesByKeyByBranch(Branch.NameKey branch)
       throws OrmException {
     Map<Change.Key, ChangeNotes> r = new HashMap<>();
     for (ChangeData cd : queryProvider.get().byBranchOpen(branch)) {
@@ -2884,6 +2887,14 @@ public class ReceiveCommits {
     return r;
   }
 
+  private Optional<ChangeData> byLegacyId(Change.Id legacyId) throws OrmException {
+    List<ChangeData> res = queryProvider.get().byLegacyChangeId(legacyId);
+    if (res.isEmpty()) {
+      return Optional.empty();
+    }
+    return Optional.of(res.get(0));
+  }
+
   private void reject(ReceiveCommand cmd, String why) {
     cmd.setResult(REJECTED_OTHER_REASON, why);
     commandProgress.update(1);