diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index 4fa45185e7..b85783218a 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/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); + } } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java index bec8e36c8f..a9b617f002 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java +++ b/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); - } } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java index 2f9d501698..a6851419db 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java +++ b/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"); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 3141de1c10..7c0b23203f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/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 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 openChangesByBranch(Branch.NameKey branch) + private Map openChangesByKeyByBranch(Branch.NameKey branch) throws OrmException { Map r = new HashMap<>(); for (ChangeData cd : queryProvider.get().byBranchOpen(branch)) { @@ -2884,6 +2887,14 @@ public class ReceiveCommits { return r; } + private Optional byLegacyId(Change.Id legacyId) throws OrmException { + List 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);