Merge "Change the structure of Submission Id"

2019-12-10 14:51:15 +00:00
parent 22a190635c dd31db99c3
commit 1fa522e414
16 changed files with 95 additions and 58 deletions
--- 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
+"<id>-<topic>" of the change that triggered the submission.
+The callers must not rely on the format of the submission ID.
 |==================================

 [[change-input]]
--- a/java/com/google/gerrit/entities/SubmissionId.java
+++ 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;
+  }
+}
--- 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());
    }
--- 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;
--- 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);
--- 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<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));
--- 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<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);
      }
--- 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<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");
  }
--- 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<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)) {
--- 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<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,
--- 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)
--- 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
--- 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;
            }
          });
--- 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());
-  }
 }
--- 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());
-  }
 }
--- 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;
    }