Let enclosed changes have different submission ids

In change I7fffa4d34 I added a submission id to all enclosed changes.
However, it seems that if two changes were stacked and submitted,
they would have different submission ids.
Here I ensured that enclosed changes called in ReceiveCommits would
get the same submission id.
I originally thought I also need to update ChangeInserter and
ConsistencyChecker, but it seems they don't have that problem.

Change-Id: I7b49fed64ec26957becc8380af7d43fdf4f50d1c
This commit is contained in:
Gal Paikin
2019-11-20 15:54:31 -08:00
parent 106387eb6a
commit 19d0c1a335
4 changed files with 49 additions and 4 deletions

View File

@@ -52,6 +52,7 @@ public class MergedByPushOp implements BatchUpdateOp {
MergedByPushOp create(
RequestScopePropagator requestScopePropagator,
PatchSet.Id psId,
@Assisted RequestId submissionId,
@Assisted("refName") String refName,
@Assisted("mergeResultRevId") String mergeResultRevId);
}
@@ -65,6 +66,7 @@ public class MergedByPushOp implements BatchUpdateOp {
private final ChangeMerged changeMerged;
private final PatchSet.Id psId;
private final RequestId submissionId;
private final String refName;
private final String mergeResultRevId;
@@ -84,6 +86,7 @@ public class MergedByPushOp implements BatchUpdateOp {
ChangeMerged changeMerged,
@Assisted RequestScopePropagator requestScopePropagator,
@Assisted PatchSet.Id psId,
@Assisted RequestId submissionId,
@Assisted("refName") String refName,
@Assisted("mergeResultRevId") String mergeResultRevId) {
this.patchSetInfoFactory = patchSetInfoFactory;
@@ -93,6 +96,7 @@ public class MergedByPushOp implements BatchUpdateOp {
this.sendEmailExecutor = sendEmailExecutor;
this.changeMerged = changeMerged;
this.requestScopePropagator = requestScopePropagator;
this.submissionId = submissionId;
this.psId = psId;
this.refName = refName;
this.mergeResultRevId = mergeResultRevId;
@@ -133,7 +137,6 @@ public class MergedByPushOp implements BatchUpdateOp {
}
change.setCurrentPatchSet(info);
change.setStatus(Change.Status.MERGED);
RequestId submissionId = new RequestId(change.getId().toString());
change.setSubmissionId(submissionId.toStringForStorage());
// we cannot reconstruct the submit records for when this change was
// submitted, this is why we must fix the status and other details.

View File

@@ -3281,6 +3281,7 @@ class ReceiveCommits {
int existingPatchSets = 0;
int newPatchSets = 0;
RequestId submissionId = null;
COMMIT:
for (RevCommit c; (c = rw.next()) != null; ) {
rw.parseBody(c);
@@ -3289,13 +3290,20 @@ class ReceiveCommits {
receivePackRefCache.tipsFromObjectId(c.copy(), RefNames.REFS_CHANGES)) {
PatchSet.Id psId = PatchSet.Id.fromRef(ref.getName());
Optional<ChangeNotes> notes = getChangeNotes(psId.changeId());
if (submissionId == null) {
submissionId = new RequestId(psId.changeId().toString());
}
if (notes.isPresent() && notes.get().getChange().getDest().equals(branch)) {
existingPatchSets++;
bu.addOp(notes.get().getChangeId(), setPrivateOpFactory.create(false, null));
bu.addOp(
psId.changeId(),
mergedByPushOpFactory.create(
requestScopePropagator, psId, refName, newTip.getId().getName()));
requestScopePropagator,
psId,
submissionId,
refName,
newTip.getId().getName()));
continue COMMIT;
}
}
@@ -3324,13 +3332,20 @@ class ReceiveCommits {
logger.atFine().log("Not closing %s because validation failed", id);
continue;
}
if (submissionId == null) {
submissionId = new RequestId(id.toString());
}
req.addOps(bu, null);
bu.addOp(id, setPrivateOpFactory.create(false, null));
bu.addOp(
id,
mergedByPushOpFactory
.create(
requestScopePropagator, req.psId, refName, newTip.getId().getName())
requestScopePropagator,
req.psId,
submissionId,
refName,
newTip.getId().getName())
.setPatchSetProvider(req.replaceOp::getPatchSet));
bu.addOp(id, new ChangeProgressOp(progress));
ids.add(id);

View File

@@ -61,6 +61,7 @@ import com.google.gerrit.server.extensions.events.CommentAdded;
import com.google.gerrit.server.extensions.events.RevisionCreated;
import com.google.gerrit.server.git.MergedByPushOp;
import com.google.gerrit.server.git.receive.ReceiveCommits.MagicBranchInput;
import com.google.gerrit.server.logging.RequestId;
import com.google.gerrit.server.mail.MailUtil.MailRecipients;
import com.google.gerrit.server.mail.send.ReplacePatchSetSender;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -236,7 +237,11 @@ public class ReplaceOp implements BatchUpdateOp {
if (mergedInto != null) {
mergedByPushOp =
mergedByPushOpFactory.create(
requestScopePropagator, patchSetId, mergedInto, mergeResultRevId);
requestScopePropagator,
patchSetId,
new RequestId(patchSetId.changeId().toString()),
mergedInto,
mergeResultRevId);
}
}

View File

@@ -3572,6 +3572,28 @@ public class ChangeIT extends AbstractDaemonTest {
assertPermitted(change, "Code-Review", 0, 1, 2);
}
@Test
public void checkSubmissionIdForAutoClosedChange() throws Exception {
PushOneCommit.Result first = createChange();
PushOneCommit.Result second = createChange();
PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
PushOneCommit.Result result = push.to("refs/heads/master");
result.assertOkStatus();
ChangeInfo firstChange = gApi.changes().id(first.getChangeId()).get();
assertThat(firstChange.status).isEqualTo(ChangeStatus.MERGED);
assertThat(firstChange.submissionId).isNotNull();
ChangeInfo secondChange = gApi.changes().id(second.getChangeId()).get();
assertThat(secondChange.status).isEqualTo(ChangeStatus.MERGED);
assertThat(secondChange.submissionId).isNotNull();
assertThat(secondChange.submissionId).isEqualTo(firstChange.submissionId);
assertThat(gApi.changes().id(second.getChangeId()).submittedTogether()).hasSize(2);
}
@Test
public void maxPermittedValueAllowed() throws Exception {
final int minPermittedValue = -2;