opendev/gerrit
Code Issues Proposed changes

Change the structure of Submission Id

Browse Source 
Currently Submission Id is constructed by the change-id of the change
that triggered the submission, the timestamp, and a random alphanumeric
string. This is not pretty. Since we recently revealed the submission id
via ChangeInfo in Ic557e0c94 and via queries in I337f7f8ae, it's a good
idea to change the submission id to be more user friendly.

Submission Id is now constructed of "<numeric-id>-<topic>" or
"<numeric-id>" if topic doesn't exist for that change.

The change id is used because it makes the submission id unique (a
change can only be submitted once). The topic is used mostly since this
is currently one of the main use cases for Revert Submission.

Change-Id: Iec8b0db66adf9a7ff2809994acaedef09e4e885c
This commit is contained in:
Gal Paikin 2019-12-06 14:43:35 +01:00
parent c8a7bfff95
commit dd31db99c3
16 changed files with 95 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
Documentation/rest-api-changes.txt
View File

@ -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
"<id>-<topic>" of the change that triggered the submission.
The callers must not rely on the format of the submission ID.
|==================================
[[change-input]]

34
java/com/google/gerrit/entities/SubmissionId.java Normal file
View File

@ -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;
}
}

4
java/com/google/gerrit/server/change/ChangeInserter.java
View File

@ -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());
}

4
java/com/google/gerrit/server/change/ConsistencyChecker.java
View File

@ -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;

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

@ -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);

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

@ -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<ChangeNotes> 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));

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

@ -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<String> 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<String> groups;
private final Map<String, Short> 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);
}

10
java/com/google/gerrit/server/notedb/ChangeUpdate.java
View File

@ -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<SubmitRecord> submitRecords) {
public void merge(SubmissionId submissionId, Iterable<SubmitRecord> 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");
}

5
java/com/google/gerrit/server/submit/MergeOp.java
View File

@ -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<Change.Id, Change> 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)) {

8
java/com/google/gerrit/server/submit/SubmitStrategy.java
View File

@ -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<RevCommit> alreadyAccepted,
Set<CodeReviewCommit> 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<RevCommit> 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<RevCommit> alreadyAccepted,
@Assisted Set<CodeReviewCommit> incoming,
@Assisted RequestId submissionId,
@Assisted SubmissionId submissionId,
@Assisted SubmitType submitType,
@Assisted SubmitInput submitInput,
@Assisted SubmoduleOp submoduleOp,

4
java/com/google/gerrit/server/submit/SubmitStrategyFactory.java
View File

@ -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)

2
java/com/google/gerrit/server/submit/SubmitStrategyOp.java
View File

@ -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

6
javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
View File

@ -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;
}
});

20
javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
View File

@ -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());
}
}

14
javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
View File

@ -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());
}
}

4
javatests/com/google/gerrit/server/update/BatchUpdateTest.java
View File

@ -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;
}