diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 0fc733abfa..67ae65c9cc 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -6017,6 +6017,10 @@ When present, change has been marked Ready at some point in time. The numeric Change-Id of the change that this change reverts. |`submission_id` |optional| ID of the submission of this change. Only set if the status is `MERGED`. +This ID is equal to the numeric ID of the change that triggered the submission. +If the change that triggered the submission also has a topic, it will be +"-" of the change that triggered the submission. +The callers must not rely on the format of the submission ID. |================================== [[change-input]] diff --git a/java/com/google/gerrit/entities/SubmissionId.java b/java/com/google/gerrit/entities/SubmissionId.java new file mode 100644 index 0000000000..eb03a5a97b --- /dev/null +++ b/java/com/google/gerrit/entities/SubmissionId.java @@ -0,0 +1,34 @@ +// Copyright (C) 2019 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.entities; + +import org.eclipse.jgit.annotations.Nullable; + +public class SubmissionId { + private final String submissionId; + + public SubmissionId(Change.Id changeId, @Nullable String topic) { + submissionId = topic != null ? String.format("%s-%s", changeId, topic) : changeId.toString(); + } + + public SubmissionId(Change change) { + this(change.getId(), change.getTopic()); + } + + @Override + public String toString() { + return submissionId; + } +} diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java index 42633731d7..d1c27d5a28 100644 --- a/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/java/com/google/gerrit/server/change/ChangeInserter.java @@ -37,6 +37,7 @@ import com.google.gerrit.entities.ChangeMessage; import com.google.gerrit.entities.PatchSet; import com.google.gerrit.entities.PatchSetApproval; import com.google.gerrit.entities.PatchSetInfo; +import com.google.gerrit.entities.SubmissionId; import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.client.ReviewerState; import com.google.gerrit.extensions.restapi.ResourceConflictException; @@ -55,7 +56,6 @@ import com.google.gerrit.server.extensions.events.RevisionCreated; import com.google.gerrit.server.git.GroupCollector; import com.google.gerrit.server.git.validators.CommitValidationException; import com.google.gerrit.server.git.validators.CommitValidators; -import com.google.gerrit.server.logging.RequestId; import com.google.gerrit.server.mail.send.CreateChangeSender; import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.patch.PatchSetInfoFactory; @@ -396,7 +396,7 @@ public class ChangeInserter implements InsertChangeOp { * instead of setting the status directly? */ if (change.getStatus() == Change.Status.MERGED) { - update.fixStatusToMerged(new RequestId(ctx.getChange().getId().toString())); + update.fixStatusToMerged(new SubmissionId(change)); } else { update.setStatus(change.getStatus()); } diff --git a/java/com/google/gerrit/server/change/ConsistencyChecker.java b/java/com/google/gerrit/server/change/ConsistencyChecker.java index 19db5eece3..513f5ca95c 100644 --- a/java/com/google/gerrit/server/change/ConsistencyChecker.java +++ b/java/com/google/gerrit/server/change/ConsistencyChecker.java @@ -33,6 +33,7 @@ import com.google.gerrit.common.Nullable; import com.google.gerrit.entities.Change; import com.google.gerrit.entities.PatchSet; import com.google.gerrit.entities.Project; +import com.google.gerrit.entities.SubmissionId; import com.google.gerrit.exceptions.StorageException; import com.google.gerrit.extensions.api.changes.FixInput; import com.google.gerrit.extensions.common.ProblemInfo; @@ -44,7 +45,6 @@ import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.account.Accounts; import com.google.gerrit.server.git.GitRepositoryManager; -import com.google.gerrit.server.logging.RequestId; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.PatchSetState; import com.google.gerrit.server.patch.PatchSetInfoFactory; @@ -578,7 +578,7 @@ public class ConsistencyChecker { public boolean updateChange(ChangeContext ctx) { ctx.getChange().setStatus(Change.Status.MERGED); ctx.getUpdate(ctx.getChange().currentPatchSetId()) - .fixStatusToMerged(new RequestId(ctx.getChange().getId().toString())); + .fixStatusToMerged(new SubmissionId(ctx.getChange())); p.status = Status.FIXED; p.outcome = "Marked change as merged"; return true; diff --git a/java/com/google/gerrit/server/git/MergedByPushOp.java b/java/com/google/gerrit/server/git/MergedByPushOp.java index 9aebebf57d..01d5380fc7 100644 --- a/java/com/google/gerrit/server/git/MergedByPushOp.java +++ b/java/com/google/gerrit/server/git/MergedByPushOp.java @@ -22,11 +22,11 @@ import com.google.gerrit.entities.ChangeMessage; import com.google.gerrit.entities.LabelId; import com.google.gerrit.entities.PatchSet; import com.google.gerrit.entities.PatchSetInfo; +import com.google.gerrit.entities.SubmissionId; import com.google.gerrit.server.ChangeMessagesUtil; import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.config.SendEmailExecutor; import com.google.gerrit.server.extensions.events.ChangeMerged; -import com.google.gerrit.server.logging.RequestId; import com.google.gerrit.server.mail.send.MergedSender; import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.patch.PatchSetInfoFactory; @@ -52,7 +52,7 @@ public class MergedByPushOp implements BatchUpdateOp { MergedByPushOp create( RequestScopePropagator requestScopePropagator, PatchSet.Id psId, - @Assisted RequestId submissionId, + @Assisted SubmissionId submissionId, @Assisted("refName") String refName, @Assisted("mergeResultRevId") String mergeResultRevId); } @@ -66,7 +66,7 @@ public class MergedByPushOp implements BatchUpdateOp { private final ChangeMerged changeMerged; private final PatchSet.Id psId; - private final RequestId submissionId; + private final SubmissionId submissionId; private final String refName; private final String mergeResultRevId; @@ -86,7 +86,7 @@ public class MergedByPushOp implements BatchUpdateOp { ChangeMerged changeMerged, @Assisted RequestScopePropagator requestScopePropagator, @Assisted PatchSet.Id psId, - @Assisted RequestId submissionId, + @Assisted SubmissionId submissionId, @Assisted("refName") String refName, @Assisted("mergeResultRevId") String mergeResultRevId) { this.patchSetInfoFactory = patchSetInfoFactory; @@ -137,7 +137,7 @@ public class MergedByPushOp implements BatchUpdateOp { } change.setCurrentPatchSet(info); change.setStatus(Change.Status.MERGED); - change.setSubmissionId(submissionId.toStringForStorage()); + change.setSubmissionId(submissionId.toString()); // we cannot reconstruct the submit records for when this change was // submitted, this is why we must fix the status and other details. update.fixStatusToMerged(submissionId); diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index cec9e4e818..5155925ce2 100644 --- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java @@ -74,6 +74,7 @@ import com.google.gerrit.entities.PatchSet; import com.google.gerrit.entities.PatchSetInfo; import com.google.gerrit.entities.Project; import com.google.gerrit.entities.RefNames; +import com.google.gerrit.entities.SubmissionId; import com.google.gerrit.exceptions.StorageException; import com.google.gerrit.extensions.api.changes.HashtagsInput; import com.google.gerrit.extensions.api.changes.NotifyHandling; @@ -3023,7 +3024,8 @@ class ReceiveCommits { info, groups, magicBranch, - receivePack.getPushCertificate()) + receivePack.getPushCertificate(), + notes.getChange()) .setRequestScopePropagator(requestScopePropagator); bu.addOp(notes.getChangeId(), replaceOp); if (progress != null) { @@ -3281,7 +3283,7 @@ class ReceiveCommits { int existingPatchSets = 0; int newPatchSets = 0; - RequestId submissionId = null; + SubmissionId submissionId = null; COMMIT: for (RevCommit c; (c = rw.next()) != null; ) { rw.parseBody(c); @@ -3290,10 +3292,10 @@ class ReceiveCommits { receivePackRefCache.tipsFromObjectId(c.copy(), RefNames.REFS_CHANGES)) { PatchSet.Id psId = PatchSet.Id.fromRef(ref.getName()); Optional notes = getChangeNotes(psId.changeId()); - if (submissionId == null) { - submissionId = new RequestId(psId.changeId().toString()); - } if (notes.isPresent() && notes.get().getChange().getDest().equals(branch)) { + if (submissionId == null) { + submissionId = new SubmissionId(notes.get().getChange()); + } existingPatchSets++; bu.addOp(notes.get().getChangeId(), setPrivateOpFactory.create(false, null)); bu.addOp( @@ -3333,7 +3335,7 @@ class ReceiveCommits { continue; } if (submissionId == null) { - submissionId = new RequestId(id.toString()); + submissionId = new SubmissionId(req.notes.getChange()); } req.addOps(bu, null); bu.addOp(id, setPrivateOpFactory.create(false, null)); diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java index 6c0d5d374d..24154d605a 100644 --- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java +++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java @@ -36,6 +36,7 @@ import com.google.gerrit.entities.Comment; import com.google.gerrit.entities.PatchSet; import com.google.gerrit.entities.PatchSetApproval; import com.google.gerrit.entities.PatchSetInfo; +import com.google.gerrit.entities.SubmissionId; import com.google.gerrit.extensions.api.changes.AddReviewerInput; import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.client.ChangeKind; @@ -61,7 +62,6 @@ 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; @@ -110,7 +110,8 @@ public class ReplaceOp implements BatchUpdateOp { PatchSetInfo info, List groups, @Nullable MagicBranchInput magicBranch, - @Nullable PushCertificate pushCertificate); + @Nullable PushCertificate pushCertificate, + Change change); } private static final String CHANGE_IS_CLOSED = "change is closed"; @@ -143,6 +144,7 @@ public class ReplaceOp implements BatchUpdateOp { private final PatchSetInfo info; private final MagicBranchInput magicBranch; private final PushCertificate pushCertificate; + private final Change change; private List groups; private final Map approvals = new HashMap<>(); @@ -177,6 +179,7 @@ public class ReplaceOp implements BatchUpdateOp { ProjectCache projectCache, @SendEmailExecutor ExecutorService sendEmailExecutor, ReviewerAdder reviewerAdder, + Change change, @Assisted ProjectState projectState, @Assisted BranchNameKey dest, @Assisted boolean checkMergedInto, @@ -218,6 +221,7 @@ public class ReplaceOp implements BatchUpdateOp { this.groups = groups; this.magicBranch = magicBranch; this.pushCertificate = pushCertificate; + this.change = change; } @Override @@ -239,7 +243,7 @@ public class ReplaceOp implements BatchUpdateOp { mergedByPushOpFactory.create( requestScopePropagator, patchSetId, - new RequestId(patchSetId.changeId().toString()), + new SubmissionId(change), mergedInto, mergeResultRevId); } diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java index 02a4dcc59c..6a900c0c85 100644 --- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java +++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java @@ -55,11 +55,11 @@ import com.google.gerrit.entities.Change; import com.google.gerrit.entities.Comment; import com.google.gerrit.entities.Project; import com.google.gerrit.entities.RobotComment; +import com.google.gerrit.entities.SubmissionId; import com.google.gerrit.exceptions.StorageException; import com.google.gerrit.mail.Address; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.GerritPersonIdent; -import com.google.gerrit.server.logging.RequestId; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.util.LabelVote; import com.google.inject.assistedinject.Assisted; @@ -220,10 +220,10 @@ public class ChangeUpdate extends AbstractChangeUpdate { this.status = status; } - public void fixStatusToMerged(RequestId submissionId) { + public void fixStatusToMerged(SubmissionId submissionId) { checkArgument(submissionId != null, "submission id must be set for merged changes"); this.status = Change.Status.MERGED; - this.submissionId = submissionId.toStringForStorage(); + this.submissionId = submissionId.toString(); } public void putApproval(String label, short value) { @@ -242,9 +242,9 @@ public class ChangeUpdate extends AbstractChangeUpdate { approvals.put(label, reviewer, Optional.empty()); } - public void merge(RequestId submissionId, Iterable submitRecords) { + public void merge(SubmissionId submissionId, Iterable submitRecords) { this.status = Change.Status.MERGED; - this.submissionId = submissionId.toStringForStorage(); + this.submissionId = submissionId.toString(); this.submitRecords = ImmutableList.copyOf(submitRecords); checkArgument(!this.submitRecords.isEmpty(), "no submit records specified at submit time"); } diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java index 75ae62d605..8bdd98e2da 100644 --- a/java/com/google/gerrit/server/submit/MergeOp.java +++ b/java/com/google/gerrit/server/submit/MergeOp.java @@ -41,6 +41,7 @@ import com.google.gerrit.entities.Change; import com.google.gerrit.entities.ChangeMessage; import com.google.gerrit.entities.PatchSet; import com.google.gerrit.entities.Project; +import com.google.gerrit.entities.SubmissionId; import com.google.gerrit.exceptions.StorageException; import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.SubmitInput; @@ -237,7 +238,7 @@ public class MergeOp implements AutoCloseable { private final Map updatedChanges; private Timestamp ts; - private RequestId submissionId; + private SubmissionId submissionId; private IdentifiedUser caller; private MergeOpRepoManager orm; @@ -449,7 +450,7 @@ public class MergeOp implements AutoCloseable { this.dryrun = dryrun; this.caller = caller; this.ts = TimeUtil.nowTs(); - this.submissionId = new RequestId(change.getId().toString()); + this.submissionId = new SubmissionId(change); try (TraceContext traceContext = TraceContext.open().addTag(RequestId.Type.SUBMISSION_ID, submissionId)) { diff --git a/java/com/google/gerrit/server/submit/SubmitStrategy.java b/java/com/google/gerrit/server/submit/SubmitStrategy.java index 4c68e1b521..efb2f769c9 100644 --- a/java/com/google/gerrit/server/submit/SubmitStrategy.java +++ b/java/com/google/gerrit/server/submit/SubmitStrategy.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Sets; import com.google.gerrit.entities.BranchNameKey; import com.google.gerrit.entities.Change; +import com.google.gerrit.entities.SubmissionId; import com.google.gerrit.extensions.api.changes.SubmitInput; import com.google.gerrit.extensions.client.SubmitType; import com.google.gerrit.extensions.config.FactoryModule; @@ -42,7 +43,6 @@ import com.google.gerrit.server.git.MergeTip; import com.google.gerrit.server.git.MergeUtil; import com.google.gerrit.server.git.TagCache; import com.google.gerrit.server.git.validators.OnSubmitValidators; -import com.google.gerrit.server.logging.RequestId; import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectConfig; @@ -94,7 +94,7 @@ public abstract class SubmitStrategy { RevFlag canMergeFlag, Set alreadyAccepted, Set incoming, - RequestId submissionId, + SubmissionId submissionId, SubmitInput submitInput, SubmoduleOp submoduleOp, boolean dryrun); @@ -125,7 +125,7 @@ public abstract class SubmitStrategy { final MergeTip mergeTip; final RevFlag canMergeFlag; final Set alreadyAccepted; - final RequestId submissionId; + final SubmissionId submissionId; final SubmitType submitType; final SubmitInput submitInput; final SubmoduleOp submoduleOp; @@ -164,7 +164,7 @@ public abstract class SubmitStrategy { @Assisted RevFlag canMergeFlag, @Assisted Set alreadyAccepted, @Assisted Set incoming, - @Assisted RequestId submissionId, + @Assisted SubmissionId submissionId, @Assisted SubmitType submitType, @Assisted SubmitInput submitInput, @Assisted SubmoduleOp submoduleOp, diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyFactory.java b/java/com/google/gerrit/server/submit/SubmitStrategyFactory.java index cba572bcb7..a015665b6f 100644 --- a/java/com/google/gerrit/server/submit/SubmitStrategyFactory.java +++ b/java/com/google/gerrit/server/submit/SubmitStrategyFactory.java @@ -16,13 +16,13 @@ package com.google.gerrit.server.submit; import com.google.common.flogger.FluentLogger; import com.google.gerrit.entities.BranchNameKey; +import com.google.gerrit.entities.SubmissionId; import com.google.gerrit.extensions.api.changes.SubmitInput; import com.google.gerrit.extensions.client.SubmitType; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk; import com.google.gerrit.server.git.MergeTip; -import com.google.gerrit.server.logging.RequestId; import com.google.gerrit.server.submit.MergeOp.CommitStatus; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -52,7 +52,7 @@ public class SubmitStrategyFactory { IdentifiedUser caller, MergeTip mergeTip, CommitStatus commitStatus, - RequestId submissionId, + SubmissionId submissionId, SubmitInput submitInput, SubmoduleOp submoduleOp, boolean dryrun) diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java index b7bc58a34a..6be4a91bc3 100644 --- a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java +++ b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java @@ -453,7 +453,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp { Change c = ctx.getChange(); logger.atFine().log("Setting change %s merged", c.getId()); c.setStatus(Change.Status.MERGED); - c.setSubmissionId(args.submissionId.toStringForStorage()); + c.setSubmissionId(args.submissionId.toString()); // TODO(dborowitz): We need to be able to change the author of the message, // which is not the user from the update context. addMergedMessage was able diff --git a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java index 08719d36ce..ee5f1179d1 100644 --- a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java +++ b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java @@ -29,6 +29,7 @@ import com.google.gerrit.entities.Account; import com.google.gerrit.entities.Change; import com.google.gerrit.entities.PatchSet; import com.google.gerrit.entities.RefNames; +import com.google.gerrit.entities.SubmissionId; import com.google.gerrit.extensions.api.changes.FixInput; import com.google.gerrit.extensions.client.ChangeStatus; import com.google.gerrit.extensions.common.ChangeInfo; @@ -39,7 +40,6 @@ import com.google.gerrit.server.change.ChangeInserter; import com.google.gerrit.server.change.ConsistencyChecker; import com.google.gerrit.server.change.NotifyResolver; import com.google.gerrit.server.change.PatchSetInserter; -import com.google.gerrit.server.logging.RequestId; import com.google.gerrit.server.notedb.ChangeNoteUtil; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.Sequences; @@ -315,7 +315,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest { public boolean updateChange(ChangeContext ctx) { ctx.getChange().setStatus(Change.Status.MERGED); ctx.getUpdate(ctx.getChange().currentPatchSetId()) - .fixStatusToMerged(new RequestId(ctx.getChange().getId().toString())); + .fixStatusToMerged(new SubmissionId(ctx.getChange())); return true; } }); @@ -865,7 +865,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest { public boolean updateChange(ChangeContext ctx) { ctx.getChange().setStatus(Change.Status.MERGED); ctx.getUpdate(ctx.getChange().currentPatchSetId()) - .fixStatusToMerged(new RequestId(ctx.getChange().getId().toString())); + .fixStatusToMerged(new SubmissionId(ctx.getChange())); return true; } }); diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java index 145e914e7e..db0dec898c 100644 --- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java +++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java @@ -45,6 +45,7 @@ import com.google.gerrit.entities.Comment; import com.google.gerrit.entities.CommentRange; import com.google.gerrit.entities.PatchSet; import com.google.gerrit.entities.PatchSetApproval; +import com.google.gerrit.entities.SubmissionId; import com.google.gerrit.exceptions.StorageException; import com.google.gerrit.mail.Address; import com.google.gerrit.server.AssigneeStatusUpdate; @@ -52,7 +53,6 @@ import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.ReviewerSet; import com.google.gerrit.server.config.GerritServerId; -import com.google.gerrit.server.logging.RequestId; import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk; import com.google.gerrit.server.util.time.TimeUtil; import com.google.gerrit.testing.TestChanges; @@ -432,7 +432,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { @Test public void approvalsPostSubmit() throws Exception { Change c = newChange(); - RequestId submissionId = submissionId(c); + SubmissionId submissionId = new SubmissionId(c); ChangeUpdate update = newUpdate(c, changeOwner); update.putApproval("Code-Review", (short) 1); update.putApproval("Verified", (short) 1); @@ -467,7 +467,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { @Test public void approvalsDuringSubmit() throws Exception { Change c = newChange(); - RequestId submissionId = submissionId(c); + SubmissionId submissionId = new SubmissionId(c); ChangeUpdate update = newUpdate(c, changeOwner); update.putApproval("Code-Review", (short) 1); update.putApproval("Verified", (short) 1); @@ -604,7 +604,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { @Test public void submitRecords() throws Exception { Change c = newChange(); - RequestId submissionId = submissionId(c); + SubmissionId submissionId = new SubmissionId(c); ChangeUpdate update = newUpdate(c, changeOwner); update.setSubjectForCommit("Submit patch set 1"); @@ -640,13 +640,13 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { null, submitLabel("Verified", "OK", changeOwner.getAccountId()), submitLabel("Alternative-Code-Review", "NEED", null))); - assertThat(notes.getChange().getSubmissionId()).isEqualTo(submissionId.toStringForStorage()); + assertThat(notes.getChange().getSubmissionId()).isEqualTo(submissionId.toString()); } @Test public void latestSubmitRecordsOnly() throws Exception { Change c = newChange(); - RequestId submissionId = submissionId(c); + SubmissionId submissionId = new SubmissionId(c); ChangeUpdate update = newUpdate(c, changeOwner); update.setSubjectForCommit("Submit patch set 1"); update.merge( @@ -669,7 +669,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { assertThat(notes.getSubmitRecords()) .containsExactly( submitRecord("OK", null, submitLabel("Code-Review", "OK", changeOwner.getAccountId()))); - assertThat(notes.getChange().getSubmissionId()).isEqualTo(submissionId.toStringForStorage()); + assertThat(notes.getChange().getSubmissionId()).isEqualTo(submissionId.toString()); } @Test @@ -977,7 +977,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { // Finish off by merging the change. update = newUpdate(c, changeOwner); update.merge( - submissionId(c), + new SubmissionId(c), ImmutableList.of( submitRecord( "NOT_READY", @@ -3140,8 +3140,4 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { update.commit(); return tr.parseBody(commit); } - - private RequestId submissionId(Change c) { - return new RequestId(c.getId().toString()); - } } diff --git a/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java b/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java index 97781a4a34..5e25c131ef 100644 --- a/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java +++ b/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java @@ -21,9 +21,9 @@ import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER; import com.google.common.collect.ImmutableList; import com.google.gerrit.entities.Account; import com.google.gerrit.entities.Change; +import com.google.gerrit.entities.SubmissionId; import com.google.gerrit.mail.Address; import com.google.gerrit.server.CurrentUser; -import com.google.gerrit.server.logging.RequestId; import com.google.gerrit.server.util.time.TimeUtil; import com.google.gerrit.testing.ConfigSuite; import com.google.gerrit.testing.TestChanges; @@ -151,7 +151,7 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest { ChangeUpdate update = newUpdate(c, changeOwner); update.setSubjectForCommit("Submit patch set 1"); - RequestId submissionId = submissionId(c); + SubmissionId submissionId = new SubmissionId(c); update.merge( submissionId, ImmutableList.of( @@ -174,7 +174,7 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest { + "Patch-set: 1\n" + "Status: merged\n" + "Submission-id: " - + submissionId.toStringForStorage() + + submissionId.toString() + "\n" + "Submitted-with: NOT_READY\n" + "Submitted-with: OK: Verified: Gerrit User 1 <1@gerrit>\n" @@ -223,7 +223,7 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest { ChangeUpdate update = newUpdate(c, changeOwner); update.setSubjectForCommit("Submit patch set 1"); - RequestId submissionId = submissionId(c); + SubmissionId submissionId = new SubmissionId(c); update.merge( submissionId, ImmutableList.of(submitRecord("RULE_ERROR", "Problem with patch set:\n1"))); update.commit(); @@ -234,7 +234,7 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest { + "Patch-set: 1\n" + "Status: merged\n" + "Submission-id: " - + submissionId.toStringForStorage() + + submissionId.toString() + "\n" + "Submitted-with: RULE_ERROR Problem with patch set: 1\n", update.getResult()); @@ -427,8 +427,4 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest { RevCommit commit = parseCommit(commitId); assertThat(commit.getFullMessage()).isEqualTo(expected); } - - private RequestId submissionId(Change c) { - return new RequestId(c.getId().toString()); - } } diff --git a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java index 5860c48d60..c7ca887c1a 100644 --- a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java +++ b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java @@ -26,12 +26,12 @@ import com.google.gerrit.entities.Change; import com.google.gerrit.entities.PatchSet; import com.google.gerrit.entities.Project; import com.google.gerrit.entities.RefNames; +import com.google.gerrit.entities.SubmissionId; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.change.ChangeInserter; import com.google.gerrit.server.change.PatchSetInserter; import com.google.gerrit.server.git.GitRepositoryManager; -import com.google.gerrit.server.logging.RequestId; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.notedb.Sequences; @@ -332,7 +332,7 @@ public class BatchUpdateTest { cr.label = "Code-Review"; sr.labels = ImmutableList.of(cr); ChangeUpdate update = ctx.getUpdate(ctx.getChange().currentPatchSetId()); - update.merge(new RequestId(), ImmutableList.of(sr)); + update.merge(new SubmissionId(ctx.getChange()), ImmutableList.of(sr)); update.setChangeMessage("Submitted"); return true; }