opendev/gerrit
Code Issues Proposed changes

change-merged: set correct newRev on submit by push

Browse Source 
Bug: Issue 11083
Change-Id: I12acf29549c35c93219867348c3691a24415e28c
This commit is contained in:
Sven Selberg 2019-06-27 16:39:59 +02:00 committed by Marco Miller
parent 0f6f21c916
commit 77df787ce5
6 changed files with 39 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
Documentation/cmd-stream-events.txt
View File

@ -112,7 +112,8 @@ patchSet:: link:json.html#patchSet[patchSet attribute]
submitter:: link:json.html#account[account attribute]
newRev:: The resulting revision of the merge.
newRev:: The state (revision) of the target branch after the operation that
closed the change was completed.
eventCreatedOn:: Time in seconds since the UNIX epoch when this event was
created.

4
java/com/google/gerrit/acceptance/EventRecorder.java
View File

@ -16,6 +16,7 @@ package com.google.gerrit.acceptance;
import static com.google.common.truth.Truth.assertThat;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.LinkedListMultimap;
@ -109,7 +110,8 @@ public class EventRecorder {
return events;
}
private ImmutableList<ChangeMergedEvent> getChangeMergedEvents(
@VisibleForTesting
public ImmutableList<ChangeMergedEvent> getChangeMergedEvents(
String project, String branch, int expectedSize) {
String key = refEventKey(ChangeMergedEvent.TYPE, project, branch);
if (expectedSize == 0) {

13
java/com/google/gerrit/server/git/MergedByPushOp.java
View File

@ -54,7 +54,10 @@ public class MergedByPushOp implements BatchUpdateOp {
public interface Factory {
MergedByPushOp create(
RequestScopePropagator requestScopePropagator, PatchSet.Id psId, String refName);
RequestScopePropagator requestScopePropagator,
PatchSet.Id psId,
@Assisted("refName") String refName,
@Assisted("mergeResultRevId") String mergeResultRevId);
}
private final RequestScopePropagator requestScopePropagator;
@ -67,6 +70,7 @@ public class MergedByPushOp implements BatchUpdateOp {
private final PatchSet.Id psId;
private final String refName;
private final String mergeResultRevId;
private Change change;
private boolean correctBranch;
@ -84,7 +88,8 @@ public class MergedByPushOp implements BatchUpdateOp {
ChangeMerged changeMerged,
@Assisted RequestScopePropagator requestScopePropagator,
@Assisted PatchSet.Id psId,
@Assisted String refName) {
@Assisted("refName") String refName,
@Assisted("mergeResultRevId") String mergeResultRevId) {
this.patchSetInfoFactory = patchSetInfoFactory;
this.cmUtil = cmUtil;
this.mergedSenderFactory = mergedSenderFactory;
@ -94,6 +99,7 @@ public class MergedByPushOp implements BatchUpdateOp {
this.requestScopePropagator = requestScopePropagator;
this.psId = psId;
this.refName = refName;
this.mergeResultRevId = mergeResultRevId;
}
public String getMergedIntoRef() {
@ -196,8 +202,7 @@ public class MergedByPushOp implements BatchUpdateOp {
}
}));
changeMerged.fire(
change, patchSet, ctx.getAccount(), patchSet.getRevision().get(), ctx.getWhen());
changeMerged.fire(change, patchSet, ctx.getAccount(), mergeResultRevId, ctx.getWhen());
}
private PatchSetInfo getPatchSetInfo(ChangeContext ctx) throws IOException, OrmException {

6
java/com/google/gerrit/server/git/receive/ReceiveCommits.java
View File

@ -2818,6 +2818,7 @@ class ReceiveCommits {
projectState,
notes.getChange().getDest(),
checkMergedInto,
checkMergedInto ? inputCommand.getNewId().name() : null,
priorPatchSet,
priorCommit,
psId,
@ -3117,7 +3118,8 @@ class ReceiveCommits {
existingPatchSets++;
bu.addOp(
psId.getParentKey(),
mergedByPushOpFactory.create(requestScopePropagator, psId, refName));
mergedByPushOpFactory.create(
requestScopePropagator, psId, refName, newTip.getId().getName()));
continue COMMIT;
}
}
@ -3150,7 +3152,7 @@ class ReceiveCommits {
bu.addOp(
id,
mergedByPushOpFactory
.create(requestScopePropagator, req.psId, refName)
.create(requestScopePropagator, req.psId, refName, newTip.getId().getName())
.setPatchSetProvider(req.replaceOp::getPatchSet));
bu.addOp(id, new ChangeProgressOp(progress));
ids.add(id);

7
java/com/google/gerrit/server/git/receive/ReplaceOp.java
View File

@ -103,6 +103,7 @@ public class ReplaceOp implements BatchUpdateOp {
ProjectState projectState,
Branch.NameKey dest,
boolean checkMergedInto,
@Nullable String mergeResultRevId,
@Assisted("priorPatchSetId") PatchSet.Id priorPatchSetId,
@Assisted("priorCommitId") ObjectId priorCommit,
@Assisted("patchSetId") PatchSet.Id patchSetId,
@ -136,6 +137,7 @@ public class ReplaceOp implements BatchUpdateOp {
private final ProjectState projectState;
private final Branch.NameKey dest;
private final boolean checkMergedInto;
private final String mergeResultRevId;
private final PatchSet.Id priorPatchSetId;
private final ObjectId priorCommitId;
private final PatchSet.Id patchSetId;
@ -181,6 +183,7 @@ public class ReplaceOp implements BatchUpdateOp {
@Assisted ProjectState projectState,
@Assisted Branch.NameKey dest,
@Assisted boolean checkMergedInto,
@Assisted @Nullable String mergeResultRevId,
@Assisted("priorPatchSetId") PatchSet.Id priorPatchSetId,
@Assisted("priorCommitId") ObjectId priorCommitId,
@Assisted("patchSetId") PatchSet.Id patchSetId,
@ -210,6 +213,7 @@ public class ReplaceOp implements BatchUpdateOp {
this.projectState = projectState;
this.dest = dest;
this.checkMergedInto = checkMergedInto;
this.mergeResultRevId = mergeResultRevId;
this.priorPatchSetId = priorPatchSetId;
this.priorCommitId = priorCommitId.copy();
this.patchSetId = patchSetId;
@ -236,7 +240,8 @@ public class ReplaceOp implements BatchUpdateOp {
String mergedInto = findMergedInto(ctx, dest.get(), commit);
if (mergedInto != null) {
mergedByPushOp =
mergedByPushOpFactory.create(requestScopePropagator, patchSetId, mergedInto);
mergedByPushOpFactory.create(
requestScopePropagator, patchSetId, mergedInto, mergeResultRevId);
}
}

15
javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
View File

@ -30,9 +30,11 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.events.ChangeMergedEvent;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import java.util.List;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.api.errors.InvalidRemoteException;
import org.eclipse.jgit.api.errors.TransportException;
@ -167,6 +169,19 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
assertThat(cd.patchSet(psId).getRevision().get()).isEqualTo(c.name());
}
@Test
public void correctNewRevOnMergeByPushToBranch() throws Exception {
grant(project, "refs/heads/master", Permission.PUSH);
push("refs/for/master", PushOneCommit.SUBJECT, "one.txt", "One");
PushOneCommit.Result r = push("refs/for/master", PushOneCommit.SUBJECT, "two.txt", "Two");
startEventRecorder();
git().push().setRefSpecs(new RefSpec(r.getCommit().name() + ":refs/heads/master")).call();
List<ChangeMergedEvent> changeMergedEvents =
eventRecorder.getChangeMergedEvents(project.get(), "refs/heads/master", 2);
assertThat(changeMergedEvents.get(0).newRev).isEqualTo(r.getPatchSet().getRevision().get());
assertThat(changeMergedEvents.get(1).newRev).isEqualTo(r.getPatchSet().getRevision().get());
}
@Test
public void mergeOnPushToBranchWithChangeMergedInOther() throws Exception {
enableCreateNewChangeForAllNotInTarget();