Move creation of Change instance into ChangeInserter

Every caller of ChangeInserter had to do the creation of the Change
instance on its own. By letting ChangeInserter create the Change
instance we can remove some duplicate code. This way it is also
guranteed that the Change instance is consistent with the other input
parameters of ChangeInserter. When the Change instance was passed in
we had to check this explicitly.

Change-Id: Ie5a115988f967ae41cc0b57343ebe3fe78386a3d
Signed-off-by: Edwin Kempin <ekempin@google.com>

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java

@@ -14,7 +14,6 @@
 
 package com.google.gerrit.httpd.rpc.project;
 
-import com.google.gerrit.common.FooterConstants;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.AccessSection;
@@ -23,13 +22,11 @@ import com.google.gerrit.common.data.PermissionRule;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.Sequences;
 import com.google.gerrit.server.account.GroupBackend;
 import com.google.gerrit.server.change.ChangeInserter;
@@ -71,7 +68,6 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
 
   private final ReviewDb db;
   private final Sequences seq;
-  private final IdentifiedUser user;
   private final Provider<PostReviewers> reviewersProvider;
   private final ProjectCache projectCache;
   private final ChangesCollection changes;
@@ -82,7 +78,6 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
   ReviewProjectAccess(final ProjectControl.Factory projectControlFactory,
       GroupBackend groupBackend,
       MetaDataUpdate.User metaDataUpdateFactory, ReviewDb db,
-      IdentifiedUser user,
       Provider<PostReviewers> reviewersProvider,
       ProjectCache projectCache,
       AllProjectsNameProvider allProjects,
@@ -102,7 +97,6 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
         parentProjectName, message, false);
     this.db = db;
     this.seq = seq;
-    this.user = user;
     this.reviewersProvider = reviewersProvider;
     this.projectCache = projectCache;
     this.changes = changes;
@@ -123,23 +117,15 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
       return null;
     }
 
-    Change change = new Change(
-        getChangeId(commit),
-        changeId,
-        user.getAccountId(),
-        new Branch.NameKey(
-            config.getProject().getNameKey(),
-            RefNames.REFS_CONFIG),
-        TimeUtil.nowTs());
     try (RevWalk rw = new RevWalk(md.getRepository());
         ObjectInserter objInserter = md.getRepository().newObjectInserter();
         BatchUpdate bu = updateFactory.create(
-          db, change.getProject(), ctl.getUser(),
+          db, config.getProject().getNameKey(), ctl.getUser(),
-          change.getCreatedOn())) {
+          TimeUtil.nowTs())) {
       bu.setRepository(md.getRepository(), rw, objInserter);
       bu.insertChange(
           changeInserterFactory.create(
-                ctl.controlForRef(change.getDest().get()), change, commit)
+                ctl.controlForRef(RefNames.REFS_CONFIG), changeId, commit)
               .setValidatePolicy(CommitValidators.Policy.NONE)
               .setUpdateRef(false)); // Created by commitToNewRef.
       bu.execute();
@@ -160,14 +146,6 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
     return changeId;
   }
 
-  private static Change.Key getChangeId(RevCommit commit) {
-    List<String> idList = commit.getFooterLines(FooterConstants.CHANGE_ID);
-    Change.Key changeKey = !idList.isEmpty()
-        ? new Change.Key(idList.get(idList.size() - 1).trim())
-        : new Change.Key("I" + commit.name());
-    return changeKey;
-  }
-
   private void addProjectOwnersAsReviewers(ChangeResource rsrc) {
     final String projectOwners =
         groupBackend.get(SystemGroupBackend.PROJECT_OWNERS).getName();

gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java

@@ -203,12 +203,12 @@ public class ChangeUtil {
       throws NoSuchChangeException, OrmException,
       MissingObjectException, IncorrectObjectTypeException, IOException,
       RestApiException, UpdateException {
-    Change.Id changeId = patchSetId.getParentKey();
+    Change.Id changeIdToRevert = patchSetId.getParentKey();
     PatchSet patch = db.get().patchSets().get(patchSetId);
     if (patch == null) {
-      throw new NoSuchChangeException(changeId);
+      throw new NoSuchChangeException(changeIdToRevert);
     }
-    Change changeToRevert = db.get().changes().get(changeId);
+    Change changeToRevert = db.get().changes().get(changeIdToRevert);
 
     Project.NameKey project = ctl.getChange().getProject();
     try (Repository git = gitManager.openRepository(project);
@@ -246,22 +246,17 @@ public class ChangeUtil {
 
       RevCommit revertCommit;
       ChangeInserter ins;
+      Change.Id changeId = new Change.Id(seq.nextChangeId());
       try (ObjectInserter oi = git.newObjectInserter()) {
         ObjectId id = oi.insert(revertCommitBuilder);
         oi.flush();
         revertCommit = revWalk.parseCommit(id);
 
         RefControl refControl = ctl.getRefControl();
-        Change change = new Change(
-            new Change.Key("I" + computedChangeId.name()),
-            new Change.Id(seq.nextChangeId()),
-            user.get().getAccountId(),
-            changeToRevert.getDest(),
-            TimeUtil.nowTs());
-        change.setTopic(changeToRevert.getTopic());
         ins = changeInserterFactory.create(
-              refControl, change, revertCommit)
+              refControl, changeId, revertCommit)
-            .setValidatePolicy(CommitValidators.Policy.GERRIT);
+            .setValidatePolicy(CommitValidators.Policy.GERRIT)
+            .setTopic(changeToRevert.getTopic());
 
         ChangeMessage changeMessage = new ChangeMessage(
             new ChangeMessage.Key(
@@ -271,7 +266,7 @@ public class ChangeUtil {
         msgBuf.append("Patch Set ").append(patchSetId.get()).append(": Reverted");
         msgBuf.append("\n\n");
         msgBuf.append("This patchset was reverted in change: ")
-              .append(change.getKey().get());
+              .append("I").append(computedChangeId.name());
         changeMessage.setMessage(msgBuf.toString());
         ChangeUpdate update = changeUpdateFactory.create(ctl, TimeUtil.nowTs());
         changeMessagesUtil.addChangeMessage(db.get(), update, changeMessage);
@@ -279,27 +274,26 @@ public class ChangeUtil {
 
         ins.setMessage("Uploaded patch set 1.");
         try (BatchUpdate bu = updateFactory.create(
-            db.get(), change.getProject(), refControl.getUser(),
+            db.get(), project, refControl.getUser(),
-            change.getCreatedOn())) {
+            TimeUtil.nowTs())) {
           bu.setRepository(git, revWalk, oi);
           bu.insertChange(ins);
           bu.execute();
         }
       }
 
-      Change.Id id = ins.getChange().getId();
       try {
-        RevertedSender cm = revertedSenderFactory.create(id);
+        RevertedSender cm = revertedSenderFactory.create(changeId);
         cm.setFrom(user.get().getAccountId());
         cm.setChangeMessage(ins.getChangeMessage());
         cm.send();
       } catch (Exception err) {
-        log.error("Cannot send email for revert change " + id, err);
+        log.error("Cannot send email for revert change " + changeId, err);
       }
 
-      return id;
+      return changeId;
     } catch (RepositoryNotFoundException e) {
-      throw new NoSuchChangeException(changeId, e);
+      throw new NoSuchChangeException(changeIdToRevert, e);
     }
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java

@@ -14,19 +14,22 @@
 
 package com.google.gerrit.server.change;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.gerrit.reviewdb.client.Change.INITIAL_PATCH_SET_ID;
 
+import com.google.common.base.MoreObjects;
 import com.google.gerrit.common.ChangeHooks;
+import com.google.gerrit.common.FooterConstants;
 import com.google.gerrit.common.data.LabelTypes;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetInfo;
+import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ApprovalsUtil;
@@ -58,17 +61,19 @@ import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.notes.NoteMap;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.transport.ReceiveCommand;
+import org.eclipse.jgit.util.ChangeIdUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
 public class ChangeInserter extends BatchUpdate.InsertChangeOp {
   public static interface Factory {
-    ChangeInserter create(RefControl ctl, Change c, RevCommit rc);
+    ChangeInserter create(RefControl ctl, Change.Id cid, RevCommit rc);
   }
 
   private static final Logger log =
@@ -85,9 +90,12 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
   private final RefControl refControl;
   private final IdentifiedUser user;
   private final PatchSet patchSet;
+  private final Change.Id changeId;
   private final RevCommit commit;
 
   // Fields exposed as setters.
+  private Change.Status status;
+  private String topic;
   private String message;
   private CommitValidators.Policy validatePolicy =
       CommitValidators.Policy.GERRIT;
@@ -113,15 +121,8 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
       WorkQueue workQueue,
       CommitValidators.Factory commitValidatorsFactory,
       @Assisted RefControl refControl,
-      @Assisted Change change,
+      @Assisted Change.Id changeId,
       @Assisted RevCommit commit) {
-    String projectName = refControl.getProjectControl().getProject().getName();
-    String refName = refControl.getRefName();
-    checkArgument(projectName.equals(change.getProject().get())
-          && refName.equals(change.getDest().get()),
-        "RefControl for %s,%s does not match change destination %s",
-        projectName, refName, change.getDest());
-
     this.patchSetInfoFactory = patchSetInfoFactory;
     this.hooks = hooks;
     this.approvalsUtil = approvalsUtil;
@@ -131,7 +132,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
     this.commitValidatorsFactory = commitValidatorsFactory;
 
     this.refControl = refControl;
-    this.change = change;
+    this.changeId = changeId;
     this.commit = commit;
     this.reviewers = Collections.emptySet();
     this.extraCC = Collections.emptySet();
@@ -142,14 +143,43 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
 
     user = refControl.getUser().asIdentifiedUser();
     patchSet =
-        new PatchSet(new PatchSet.Id(change.getId(), INITIAL_PATCH_SET_ID));
+        new PatchSet(new PatchSet.Id(changeId, INITIAL_PATCH_SET_ID));
-    patchSet.setCreatedOn(change.getCreatedOn());
-    patchSet.setUploader(change.getOwner());
     patchSet.setRevision(new RevId(commit.name()));
   }
 
   @Override
+  public Change createChange(Context ctx) throws IOException {
+    change = new Change(
+        getChangeKey(commit),
+        changeId,
+        ctx.getUser().getAccountId(),
+        new Branch.NameKey(
+            refControl.getProjectControl().getProject().getNameKey(),
+            refControl.getRefName()),
+        ctx.getWhen());
+    change.setStatus(MoreObjects.firstNonNull(status, Change.Status.NEW));
+    change.setTopic(topic);
+    patchSet.setCreatedOn(ctx.getWhen());
+    patchSet.setUploader(ctx.getUser().getAccountId());
+    return change;
+  }
+
+  private static Change.Key getChangeKey(RevCommit commit) throws IOException {
+    List<String> idList = commit.getFooterLines(FooterConstants.CHANGE_ID);
+    if (!idList.isEmpty()) {
+      return new Change.Key(idList.get(idList.size() - 1).trim());
+    }
+
+    ObjectId id = ChangeIdUtil.computeChangeId(commit.getTree(), commit,
+        commit.getAuthorIdent(), commit.getCommitterIdent(),
+        commit.getShortMessage());
+    StringBuilder changeId = new StringBuilder();
+    changeId.append("I").append(ObjectId.toString(id));
+    return new Change.Key(changeId.toString());
+  }
+
   public Change getChange() {
+    checkState(change != null, "getChange() only valid after creating change");
     return change;
   }
 
@@ -157,6 +187,16 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
     return user;
   }
 
+  public Project.NameKey getProject() {
+    return refControl.getProjectControl().getProject().getNameKey();
+  }
+
+  public ChangeInserter setTopic(String topic) {
+    checkState(change == null, "setTopic(String) only valid before creating change");
+    this.topic = topic;
+    return this;
+  }
+
   public ChangeInserter setMessage(String message) {
     this.message = message;
     return this;
@@ -178,8 +218,18 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
   }
 
   public ChangeInserter setDraft(boolean draft) {
-    change.setStatus(draft ? Change.Status.DRAFT : Change.Status.NEW);
+    checkState(change == null,
-    patchSet.setDraft(draft);
+        "setDraft(boolean) only valid before creating change");
+    return setStatus(draft ? Change.Status.DRAFT : Change.Status.NEW);
+  }
+
+  public ChangeInserter setStatus(Change.Status status) {
+    checkState(change == null,
+        "setStatus(Change.Status) only valid before creating change");
+    this.status = status;
+    if (Change.Status.DRAFT.equals(status)) {
+      patchSet.setDraft(true);
+    }
     return this;
   }
 
@@ -245,14 +295,15 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
     patchSetInfo = patchSetInfoFactory.get(
         ctx.getRevWalk(), commit, patchSet.getId());
     ctx.getChange().setCurrentPatchSet(patchSetInfo);
+
     ChangeUpdate update = ctx.getUpdate(patchSet.getId());
+    update.setTopic(change.getTopic());
 
     if (patchSet.getGroups() == null) {
       patchSet.setGroups(GroupCollector.getDefaultGroups(patchSet));
     }
     db.patchSets().insert(Collections.singleton(patchSet));
     ctx.saveChange();
-    update.setTopic(change.getTopic());
 
     /* TODO: fixStatus is used here because the tests
      * (byStatusClosed() in AbstractQueryChangesTest)

gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java

@@ -189,14 +189,14 @@ public class CherryPickChange {
           if (!Strings.isNullOrEmpty(change.getTopic())) {
             newTopic = change.getTopic() + "-" + newDest.getShortName();
           }
-          Change newChange = createNewChange(git, revWalk, oi, changeKey,
+          Change.Id newChangeId =
-              project, destRef, cherryPickCommit, refControl, identifiedUser,
+              createNewChange(git, revWalk, oi, project, cherryPickCommit,
-              newTopic, change.getDest());
+                  refControl, identifiedUser, newTopic, change.getDest());
 
           addMessageToSourceChange(change, patch.getId(), destinationBranch,
               cherryPickCommit, identifiedUser, refControl);
 
-          return newChange.getId();
+          return newChangeId;
         }
       } catch (MergeIdenticalTreeException | MergeConflictException e) {
         throw new IntegrationException("Cherry pick failed: " + e.getMessage());
@@ -231,29 +231,26 @@ public class CherryPickChange {
     return change.getId();
   }
 
-  private Change createNewChange(Repository git, RevWalk revWalk,
+  private Change.Id createNewChange(Repository git, RevWalk revWalk,
-      ObjectInserter oi, Change.Key changeKey, Project.NameKey project,
+      ObjectInserter oi, Project.NameKey project,
-      Ref destRef, CodeReviewCommit cherryPickCommit, RefControl refControl,
+      CodeReviewCommit cherryPickCommit, RefControl refControl,
       IdentifiedUser identifiedUser, String topic, Branch.NameKey sourceBranch)
-      throws RestApiException, UpdateException, OrmException {
+          throws RestApiException, UpdateException, OrmException, IOException {
-    Change change =
+    Change.Id changeId = new Change.Id(seq.nextChangeId());
-        new Change(changeKey, new Change.Id(seq.nextChangeId()),
-            identifiedUser.getAccountId(), new Branch.NameKey(project,
-                destRef.getName()), TimeUtil.nowTs());
-    change.setTopic(topic);
     ChangeInserter ins = changeInserterFactory.create(
-          refControl, change, cherryPickCommit)
+          refControl, changeId, cherryPickCommit)
-        .setValidatePolicy(CommitValidators.Policy.GERRIT);
+        .setValidatePolicy(CommitValidators.Policy.GERRIT)
+        .setTopic(topic);
 
     ins.setMessage(
         messageForDestinationChange(ins.getPatchSet().getId(), sourceBranch));
     try (BatchUpdate bu = batchUpdateFactory.create(
-        db.get(), change.getProject(), identifiedUser, TimeUtil.nowTs())) {
+        db.get(), project, identifiedUser, TimeUtil.nowTs())) {
       bu.setRepository(git, revWalk, oi);
       bu.insertChange(ins);
       bu.execute();
     }
-    return ins.getChange();
+    return changeId;
   }
 
   private void addMessageToSourceChange(Change change, PatchSet.Id patchSetId,

gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java

@@ -16,7 +16,6 @@ package com.google.gerrit.server.change;
 
 import com.google.common.base.Strings;
 import com.google.common.collect.Iterables;
-import com.google.gerrit.common.FooterConstants;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.Capable;
 import com.google.gerrit.extensions.client.ChangeStatus;
@@ -29,7 +28,6 @@ import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
 import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
-import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.Project;
@@ -196,15 +194,9 @@ public class CreateChange implements
       try (ObjectInserter oi = git.newObjectInserter()) {
         RevCommit c = newCommit(oi, rw, author, mergeTip, commitMessage);
 
-        Change change = new Change(
+        Change.Id changeId = new Change.Id(seq.nextChangeId());
-            getChangeId(id, c),
-            new Change.Id(seq.nextChangeId()),
-            me.getAccountId(),
-            new Branch.NameKey(project, refName),
-            now);
-
         ChangeInserter ins = changeInserterFactory
-            .create(refControl, change, c)
+            .create(refControl, changeId, c)
             .setValidatePolicy(CommitValidators.Policy.GERRIT);
         ins.setMessage(String.format("Uploaded patch set %s.",
             ins.getPatchSet().getPatchSetId()));
@@ -212,31 +204,22 @@ public class CreateChange implements
         if (topic != null) {
           topic = Strings.emptyToNull(topic.trim());
         }
-        change.setTopic(topic);
+        ins.setTopic(topic);
         ins.setDraft(input.status != null && input.status == ChangeStatus.DRAFT);
         ins.setGroups(groups);
         try (BatchUpdate bu = updateFactory.create(
-            db.get(), change.getProject(), me, now)) {
+            db.get(), project, me, now)) {
           bu.setRepository(git, rw, oi);
           bu.insertChange(ins);
           bu.execute();
         }
         ChangeJson json = jsonFactory.create(ChangeJson.NO_OPTIONS);
-        return Response.created(json.format(change.getId()));
+        return Response.created(json.format(changeId));
       }
 
     }
   }
 
-  private static Change.Key getChangeId(ObjectId id, RevCommit emptyCommit) {
-    List<String> idList = emptyCommit.getFooterLines(
-        FooterConstants.CHANGE_ID);
-    Change.Key changeKey = !idList.isEmpty()
-        ? new Change.Key(idList.get(idList.size() - 1).trim())
-        : new Change.Key("I" + id.name());
-    return changeKey;
-  }
-
   private static RevCommit newCommit(ObjectInserter oi, RevWalk rw,
       PersonIdent authorIdent, RevCommit mergeTip, String commitMessage)
       throws IOException {

gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java

@@ -231,7 +231,7 @@ public class BatchUpdate implements AutoCloseable {
   }
 
   public abstract static class InsertChangeOp extends Op {
-    public abstract Change getChange();
+    public abstract Change createChange(Context ctx) throws IOException;
   }
 
   private static class ChainedReceiveCommands {
@@ -481,8 +481,9 @@ public class BatchUpdate implements AutoCloseable {
     return this;
   }
 
-  public BatchUpdate insertChange(InsertChangeOp op) {
+  public BatchUpdate insertChange(InsertChangeOp op) throws IOException {
-    Change c = op.getChange();
+    Context ctx = new Context();
+    Change c = op.createChange(ctx);
     checkArgument(!newChanges.containsKey(c.getId()),
         "only one op allowed to create change %s", c.getId());
     newChanges.put(c.getId(), c);

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -712,7 +712,7 @@ public class ReceiveCommits {
         Iterables.filter(newChanges, new Predicate<CreateRequest>() {
           @Override
           public boolean apply(CreateRequest input) {
-            return input.created;
+            return input.change != null;
           }
         });
     if (!Iterables.isEmpty(created)) {
@@ -1583,10 +1583,9 @@ public class ReceiveCommits {
             + "to override please set the base manually");
         }
 
-        Change.Key changeKey = new Change.Key("I" + c.name());
         final List<String> idList = c.getFooterLines(CHANGE_ID);
         if (idList.isEmpty()) {
-          newChanges.add(new CreateRequest(magicBranch.ctl, c, changeKey));
+          newChanges.add(new CreateRequest(magicBranch.ctl, c));
           continue;
         }
 
@@ -1598,8 +1597,7 @@ public class ReceiveCommits {
           return;
         }
 
-        changeKey = new Change.Key(idStr);
+        pending.add(new ChangeLookup(c, new Change.Key(idStr)));
-        pending.add(new ChangeLookup(c, changeKey));
         if (maxBatchChanges != 0
             && pending.size() + newChanges.size() > maxBatchChanges) {
           reject(magicBranch.cmd,
@@ -1650,7 +1648,7 @@ public class ReceiveCommits {
 
           newChangeIds.add(p.changeKey);
         }
-        newChanges.add(new CreateRequest(magicBranch.ctl, p.commit, p.changeKey));
+        newChanges.add(new CreateRequest(magicBranch.ctl, p.commit));
       }
     } catch (IOException e) {
       // Should never happen, the core receive process would have
@@ -1729,21 +1727,17 @@ public class ReceiveCommits {
     final RevCommit commit;
     final ReceiveCommand cmd;
     final ChangeInserter ins;
+    Change.Id changeId;
     Change change;
-    boolean created;
     Collection<String> groups;
 
-    CreateRequest(RefControl ctl, RevCommit c, Change.Key changeKey)
+    CreateRequest(RefControl ctl, RevCommit c)
         throws OrmException {
       commit = c;
-      change = new Change(changeKey,
+      changeId = new Change.Id(seq.nextChangeId());
-          new Change.Id(seq.nextChangeId()),
+      ins = changeInserterFactory.create(ctl, changeId, c)
-          user.getAccountId(),
-          magicBranch.dest,
-          TimeUtil.nowTs());
-      change.setTopic(magicBranch.topic);
-      ins = changeInserterFactory.create(ctl, change, c)
           .setDraft(magicBranch.draft)
+          .setTopic(magicBranch.topic)
           // Changes already validated in validateNewCommits.
           .setValidatePolicy(CommitValidators.Policy.NONE);
       cmd = new ReceiveCommand(ObjectId.zeroId(), c,
@@ -1757,8 +1751,8 @@ public class ReceiveCommits {
       ListenableFuture<Void> future = changeUpdateExector.submit(
           requestScopePropagator.wrap(new Callable<Void>() {
         @Override
-        public Void call()
+        public Void call() throws IOException, OrmException,
-            throws OrmException, RestApiException, UpdateException {
+            RestApiException, UpdateException {
           if (caller == Thread.currentThread()) {
             insertChange(ReceiveCommits.this.db);
           } else {
@@ -1774,7 +1768,7 @@ public class ReceiveCommits {
     }
 
     private void insertChange(ReviewDb threadLocalDb)
-        throws OrmException, RestApiException, UpdateException {
+        throws IOException, OrmException, RestApiException, UpdateException {
       final PatchSet ps = ins.setGroups(groups).getPatchSet();
       final Account.Id me = user.getAccountId();
       final List<FooterLine> footerLines = commit.getFooterLines();
@@ -1790,7 +1784,7 @@ public class ReceiveCommits {
           approvals, Collections.<String, PatchSetApproval> emptyMap());
       try (ObjectInserter oi = repo.newObjectInserter();
           BatchUpdate bu = batchUpdateFactory.create(threadLocalDb,
-            change.getProject(), user, change.getCreatedOn())) {
+              magicBranch.dest.getParentKey(), user, TimeUtil.nowTs())) {
         bu.setRepository(repo, rp.getRevWalk(), oi);
         bu.insertChange(ins
             .setReviewers(recipients.getReviewers())
@@ -1802,12 +1796,12 @@ public class ReceiveCommits {
             .setUpdateRef(false));
         if (magicBranch != null) {
           bu.addOp(
-              ins.getChange().getId(),
+              changeId,
               hashtagsFactory.create(new HashtagsInput(magicBranch.hashtags))
                 .setRunHooks(false));
           if (!Strings.isNullOrEmpty(magicBranch.topic)) {
             bu.addOp(
-                ins.getChange().getId(),
+                changeId,
                 new BatchUpdate.Op() {
                   @Override
                   public void updateChange(ChangeContext ctx) throws Exception {
@@ -1818,7 +1812,6 @@ public class ReceiveCommits {
         }
         bu.execute();
       }
-      created = true;
       change = ins.getChange();
 
       if (magicBranch != null && magicBranch.submit) {

gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java

@@ -17,7 +17,6 @@ package com.google.gerrit.server.query.change;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.TruthJUnit.assume;
 import static com.google.gerrit.extensions.client.ListChangesOption.REVIEWED;
-import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.concurrent.TimeUnit.HOURS;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.concurrent.TimeUnit.MINUTES;
@@ -30,7 +29,6 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
-import com.google.common.hash.Hashing;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.extensions.api.GerritApi;
@@ -196,8 +194,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byId() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    Change change1 = insert(newChange(repo, null, null, null, null));
+    Change change1 = insert(newChange(repo));
-    Change change2 = insert(newChange(repo, null, null, null, null));
+    Change change2 = insert(newChange(repo));
 
     assertQuery("12345");
     assertQuery(change1.getId().get(), change1);
@@ -207,7 +205,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byKey() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    Change change = insert(newChange(repo, null, null, null, null));
+    Change change = insert(newChange(repo));
     String key = change.getKey().get();
 
     assertQuery("I0000000000000000000000000000000000000000");
@@ -220,7 +218,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byTriplet() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    Change change = insert(newChange(repo, null, null, null, "branch"));
+    Change change = insert(newChangeForBranch(repo, "branch"));
     String k = change.getKey().get();
 
     assertQuery("repo~branch~" + k, change);
@@ -243,14 +241,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byStatus() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    ChangeInserter ins1 = newChange(repo, null, null, null, null);
+    ChangeInserter ins1 = newChangeWithStatus(repo, Change.Status.NEW);
-    Change change1 = ins1.getChange();
+    Change change1 = insert(ins1);
-    change1.setStatus(Change.Status.NEW);
+    ChangeInserter ins2 = newChangeWithStatus(repo, Change.Status.MERGED);
-    insert(ins1);
+    Change change2 = insert(ins2);
-    ChangeInserter ins2 = newChange(repo, null, null, null, null);
-    Change change2 = ins2.getChange();
-    change2.setStatus(Change.Status.MERGED);
-    insert(ins2);
 
     assertQuery("status:new", change1);
     assertQuery("status:NEW", change1);
@@ -262,18 +256,11 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byStatusOpen() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    ChangeInserter ins1 = newChange(repo, null, null, null, null);
+    ChangeInserter ins1 = newChangeWithStatus(repo, Change.Status.NEW);
-    Change change1 = ins1.getChange();
+    Change change1 = insert(ins1);
-    change1.setStatus(Change.Status.NEW);
+    ChangeInserter ins2 = newChangeWithStatus(repo, Change.Status.DRAFT);
-    insert(ins1);
+    Change change2 = insert(ins2);
-    ChangeInserter ins2 = newChange(repo, null, null, null, null);
+    insert(newChangeWithStatus(repo, Change.Status.MERGED));
-    Change change2 = ins2.getChange();
-    change2.setStatus(Change.Status.DRAFT);
-    insert(ins2);
-    ChangeInserter ins3 = newChange(repo, null, null, null, null);
-    Change change3 = ins3.getChange();
-    change3.setStatus(Change.Status.MERGED);
-    insert(ins3);
 
     Change[] expected = new Change[] {change2, change1};
     assertQuery("status:open", expected);
@@ -292,14 +279,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byStatusDraft() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    ChangeInserter ins1 = newChange(repo, null, null, null, null);
+    insert(newChangeWithStatus(repo, Change.Status.NEW));
-    Change change1 = ins1.getChange();
+    ChangeInserter ins2 = newChangeWithStatus(repo, Change.Status.DRAFT);
-    change1.setStatus(Change.Status.NEW);
+    Change change2 = insert(ins2);
-    insert(ins1);
-    ChangeInserter ins2 = newChange(repo, null, null, null, null);
-    Change change2 = ins2.getChange();
-    change2.setStatus(Change.Status.DRAFT);
-    insert(ins2);
 
     Change[] expected = new Change[] {change2};
     assertQuery("status:draft", expected);
@@ -314,18 +296,11 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byStatusClosed() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    ChangeInserter ins1 = newChange(repo, null, null, null, null);
+    ChangeInserter ins1 = newChangeWithStatus(repo, Change.Status.MERGED);
-    Change change1 = ins1.getChange();
+    Change change1 = insert(ins1);
-    change1.setStatus(Change.Status.MERGED);
+    ChangeInserter ins2 = newChangeWithStatus(repo, Change.Status.ABANDONED);
-    insert(ins1);
+    Change change2 = insert(ins2);
-    ChangeInserter ins2 = newChange(repo, null, null, null, null);
+    insert(newChangeWithStatus(repo, Change.Status.NEW));
-    Change change2 = ins2.getChange();
-    change2.setStatus(Change.Status.ABANDONED);
-    insert(ins2);
-    ChangeInserter ins3 = newChange(repo, null, null, null, null);
-    Change change3 = ins3.getChange();
-    change3.setStatus(Change.Status.NEW);
-    insert(ins3);
 
     Change[] expected = new Change[] {change2, change1};
     assertQuery("status:closed", expected);
@@ -342,14 +317,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byStatusPrefix() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    ChangeInserter ins1 = newChange(repo, null, null, null, null);
+    ChangeInserter ins1 = newChangeWithStatus(repo, Change.Status.NEW);
-    Change change1 = ins1.getChange();
+    Change change1 = insert(ins1);
-    change1.setStatus(Change.Status.NEW);
+    insert(newChangeWithStatus(repo, Change.Status.MERGED));
-    insert(ins1);
-    ChangeInserter ins2 = newChange(repo, null, null, null, null);
-    Change change2 = ins2.getChange();
-    change2.setStatus(Change.Status.MERGED);
-    insert(ins2);
 
     assertQuery("status:n", change1);
     assertQuery("status:ne", change1);
@@ -364,7 +334,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byCommit() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    ChangeInserter ins = newChange(repo, null, null, null, null);
+    ChangeInserter ins = newChange(repo);
     insert(ins);
     String sha = ins.getPatchSet().getRevision().get();
 
@@ -378,10 +348,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byOwner() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    Change change1 = insert(newChange(repo, null, null, userId.get(), null));
+    Change change1 = insert(newChangeForUser(repo, userId));
-    int user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser"))
+    Account.Id user2 = accountManager.authenticate(
-        .getAccountId().get();
+        AuthRequest.forUser("anotheruser")).getAccountId();
-    Change change2 = insert(newChange(repo, null, null, user2, null));
+    Change change2 = insert(newChangeForUser(repo, user2));
 
     assertQuery("owner:" + userId.get(), change1);
     assertQuery("owner:" + user2, change2);
@@ -390,7 +360,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byAuthor() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    Change change1 = insert(newChange(repo, null, null, userId.get(), null));
+    Change change1 = insert(newChangeForUser(repo, userId));
 
     // By exact email address
     assertQuery("author:jauthor@example.com", change1);
@@ -417,7 +387,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byCommitter() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    Change change1 = insert(newChange(repo, null, null, userId.get(), null));
+    Change change1 = insert(newChangeForUser(repo, userId));
 
     // By exact email address
     assertQuery("committer:jcommitter@example.com", change1);
@@ -444,10 +414,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byOwnerIn() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    Change change1 = insert(newChange(repo, null, null, userId.get(), null));
+    Change change1 = insert(newChangeForUser(repo, userId));
-    int user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser"))
+    Account.Id user2 = accountManager.authenticate(
-        .getAccountId().get();
+        AuthRequest.forUser("anotheruser")).getAccountId();
-    Change change2 = insert(newChange(repo, null, null, user2, null));
+    Change change2 = insert(newChangeForUser(repo, user2));
 
     assertQuery("ownerin:Administrators", change1);
     assertQuery("ownerin:\"Registered Users\"", change2, change1);
@@ -457,8 +427,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   public void byProject() throws Exception {
     TestRepository<Repo> repo1 = createProject("repo1");
     TestRepository<Repo> repo2 = createProject("repo2");
-    Change change1 = insert(newChange(repo1, null, null, null, null));
+    Change change1 = insert(newChange(repo1));
-    Change change2 = insert(newChange(repo2, null, null, null, null));
+    Change change2 = insert(newChange(repo2));
 
     assertQuery("project:foo");
     assertQuery("project:repo");
@@ -470,8 +440,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   public void byProjectPrefix() throws Exception {
     TestRepository<Repo> repo1 = createProject("repo1");
     TestRepository<Repo> repo2 = createProject("repo2");
-    Change change1 = insert(newChange(repo1, null, null, null, null));
+    Change change1 = insert(newChange(repo1));
-    Change change2 = insert(newChange(repo2, null, null, null, null));
+    Change change2 = insert(newChange(repo2));
 
     assertQuery("projects:foo");
     assertQuery("projects:repo1", change1);
@@ -482,8 +452,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byBranchAndRef() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    Change change1 = insert(newChange(repo, null, null, null, "master"));
+    Change change1 = insert(newChangeForBranch(repo, "master"));
-    Change change2 = insert(newChange(repo, null, null, null, "branch"));
+    Change change2 = insert(newChangeForBranch(repo, "branch"));
 
     assertQuery("branch:foo");
     assertQuery("branch:master", change1);
@@ -500,27 +470,19 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byTopic() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    ChangeInserter ins1 = newChange(repo, null, null, null, null);
+    ChangeInserter ins1 = newChangeWithTopic(repo, "feature1");
-    Change change1 = ins1.getChange();
+    Change change1 = insert(ins1);
-    change1.setTopic("feature1");
-    insert(ins1);
 
-    ChangeInserter ins2 = newChange(repo, null, null, null, null);
+    ChangeInserter ins2 = newChangeWithTopic(repo, "feature2");
-    Change change2 = ins2.getChange();
+    Change change2 = insert(ins2);
-    change2.setTopic("feature2");
-    insert(ins2);
 
-    ChangeInserter ins3 = newChange(repo, null, null, null, null);
+    ChangeInserter ins3 = newChangeWithTopic(repo, "Cherrypick-feature2");
-    Change change3 = ins3.getChange();
+    Change change3 = insert(ins3);
-    change3.setTopic("Cherrypick-feature2");
-    insert(ins3);
 
-    ChangeInserter ins4 = newChange(repo, null, null, null, null);
+    ChangeInserter ins4 = newChangeWithTopic(repo, "feature2-fixup");
-    Change change4 = ins4.getChange();
+    Change change4 = insert(ins4);
-    change4.setTopic("feature2-fixup");
-    insert(ins4);
 
-    Change change5 = insert(newChange(repo, null, null, null, null));
+    Change change5 = insert(newChange(repo));
 
     assertQuery("intopic:foo");
     assertQuery("intopic:feature1", change1);
@@ -536,9 +498,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   public void byMessageExact() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
     RevCommit commit1 = repo.parseBody(repo.commit().message("one").create());
-    Change change1 = insert(newChange(repo, commit1, null, null, null));
+    Change change1 = insert(newChangeForCommit(repo, commit1));
     RevCommit commit2 = repo.parseBody(repo.commit().message("two").create());
-    Change change2 = insert(newChange(repo, commit2, null, null, null));
+    Change change2 = insert(newChangeForCommit(repo, commit2));
 
     assertQuery("message:foo");
     assertQuery("message:one", change1);
@@ -550,10 +512,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     TestRepository<Repo> repo = createProject("repo");
     RevCommit commit1 =
         repo.parseBody(repo.commit().message("12345 67890").create());
-    Change change1 = insert(newChange(repo, commit1, null, null, null));
+    Change change1 = insert(newChangeForCommit(repo, commit1));
     RevCommit commit2 =
         repo.parseBody(repo.commit().message("12346 67891").create());
-    Change change2 = insert(newChange(repo, commit2, null, null, null));
+    Change change2 = insert(newChangeForCommit(repo, commit2));
 
     assertQuery("message:1234");
     assertQuery("message:12345", change1);
@@ -564,7 +526,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   public void byLabel() throws Exception {
     accountManager.authenticate(AuthRequest.forUser("anotheruser"));
     TestRepository<Repo> repo = createProject("repo");
-    ChangeInserter ins = newChange(repo, null, null, null, null);
+    ChangeInserter ins = newChange(repo);
     Change change = insert(ins);
 
     gApi.changes().id(change.getId().get()).current()
@@ -627,7 +589,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     gApi.groups().id(g2).addMembers("user2");
 
     // create a change
-    ChangeInserter ins = newChange(repo, null, null, user1.get(), null);
+    ChangeInserter ins = newChangeForUser(repo, user1);
     Change change1 = insert(ins);
 
     // post a review with user1
@@ -650,7 +612,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     Change last = null;
     int n = 5;
     for (int i = 0; i < n; i++) {
-      last = insert(newChange(repo, null, null, null, null));
+      last = insert(newChange(repo));
     }
 
     for (int i = 1; i <= n + 2; i++) {
@@ -677,7 +639,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     TestRepository<Repo> repo = createProject("repo");
     List<Change> changes = Lists.newArrayList();
     for (int i = 0; i < 2; i++) {
-      changes.add(insert(newChange(repo, null, null, null, null)));
+      changes.add(insert(newChange(repo)));
     }
 
     assertQuery("status:new", changes.get(1), changes.get(0));
@@ -691,7 +653,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     TestRepository<Repo> repo = createProject("repo");
     List<Change> changes = Lists.newArrayList();
     for (int i = 0; i < 3; i++) {
-      changes.add(insert(newChange(repo, null, null, null, null)));
+      changes.add(insert(newChange(repo)));
     }
 
     assertQuery("status:new limit:2", changes.get(2), changes.get(1));
@@ -705,7 +667,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void maxPages() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    Change change = insert(newChange(repo, null, null, null, null));
+    Change change = insert(newChange(repo));
 
     QueryRequest query = newQuery("status:new").withLimit(10);
     assertQuery(query, change);
@@ -722,7 +684,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     List<ChangeInserter> inserters = Lists.newArrayList();
     List<Change> changes = Lists.newArrayList();
     for (int i = 0; i < 5; i++) {
-      inserters.add(newChange(repo, null, null, null, null));
+      inserters.add(newChange(repo));
       changes.add(insert(inserters.get(i)));
     }
 
@@ -744,9 +706,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   public void updatedOrderWithMinuteResolution() throws Exception {
     TestTimeUtil.resetWithClockStep(2, MINUTES);
     TestRepository<Repo> repo = createProject("repo");
-    ChangeInserter ins1 = newChange(repo, null, null, null, null);
+    ChangeInserter ins1 = newChange(repo);
     Change change1 = insert(ins1);
-    Change change2 = insert(newChange(repo, null, null, null, null));
+    Change change2 = insert(newChange(repo));
 
     assertThat(lastUpdatedMs(change1)).isLessThan(lastUpdatedMs(change2));
     assertQuery("status:new", change2, change1);
@@ -766,9 +728,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void updatedOrderWithSubMinuteResolution() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    ChangeInserter ins1 = newChange(repo, null, null, null, null);
+    ChangeInserter ins1 = newChange(repo);
     Change change1 = insert(ins1);
-    Change change2 = insert(newChange(repo, null, null, null, null));
+    Change change2 = insert(newChange(repo));
 
     assertThat(lastUpdatedMs(change1)).isLessThan(lastUpdatedMs(change2));
 
@@ -789,11 +751,11 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void filterOutMoreThanOnePageOfResults() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    Change change = insert(newChange(repo, null, null, userId.get(), null));
+    Change change = insert(newChangeForUser(repo, userId));
-    int user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser"))
+    Account.Id user2 = accountManager.authenticate(
-        .getAccountId().get();
+        AuthRequest.forUser("anotheruser")).getAccountId();
     for (int i = 0; i < 5; i++) {
-      insert(newChange(repo, null, null, user2, null));
+      insert(newChangeForUser(repo, user2));
     }
 
     assertQuery("status:new ownerin:Administrators", change);
@@ -803,10 +765,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void filterOutAllResults() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    int user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser"))
+    Account.Id user2 = accountManager.authenticate(
-        .getAccountId().get();
+        AuthRequest.forUser("anotheruser")).getAccountId();
     for (int i = 0; i < 5; i++) {
-      insert(newChange(repo, null, null, user2, null));
+      insert(newChangeForUser(repo, user2));
     }
 
     assertQuery("status:new ownerin:Administrators");
@@ -820,7 +782,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
         repo.commit().message("one")
         .add("dir/file1", "contents1").add("dir/file2", "contents2")
         .create());
-    Change change = insert(newChange(repo, commit, null, null, null));
+    Change change = insert(newChangeForCommit(repo, commit));
 
     assertQuery("file:file");
     assertQuery("file:dir", change);
@@ -837,7 +799,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
         repo.commit().message("one")
         .add("dir/file1", "contents1").add("dir/file2", "contents2")
         .create());
-    Change change = insert(newChange(repo, commit, null, null, null));
+    Change change = insert(newChangeForCommit(repo, commit));
 
     assertQuery("file:.*file.*");
     assertQuery("file:^file.*"); // Whole path only.
@@ -851,7 +813,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
         repo.commit().message("one")
         .add("dir/file1", "contents1").add("dir/file2", "contents2")
         .create());
-    Change change = insert(newChange(repo, commit, null, null, null));
+    Change change = insert(newChangeForCommit(repo, commit));
 
     assertQuery("path:file");
     assertQuery("path:dir");
@@ -868,7 +830,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
         repo.commit().message("one")
         .add("dir/file1", "contents1").add("dir/file2", "contents2")
         .create());
-    Change change = insert(newChange(repo, commit, null, null, null));
+    Change change = insert(newChangeForCommit(repo, commit));
 
     assertQuery("path:.*file.*");
     assertQuery("path:^dir.file.*", change);
@@ -877,7 +839,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byComment() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    ChangeInserter ins = newChange(repo, null, null, null, null);
+    ChangeInserter ins = newChange(repo);
     Change change = insert(ins);
 
     ReviewInput input = new ReviewInput();
@@ -899,8 +861,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     long thirtyHoursInMs = MILLISECONDS.convert(30, HOURS);
     TestTimeUtil.resetWithClockStep(thirtyHoursInMs, MILLISECONDS);
     TestRepository<Repo> repo = createProject("repo");
-    Change change1 = insert(newChange(repo, null, null, null, null));
+    Change change1 = insert(newChange(repo));
-    Change change2 = insert(newChange(repo, null, null, null, null));
+    Change change2 = insert(newChange(repo));
     // Queried by AgePredicate constructor.
     TestTimeUtil.setClockStep(0, MILLISECONDS);
     long now = TimeUtil.nowMs();
@@ -922,8 +884,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   public void byBefore() throws Exception {
     TestTimeUtil.resetWithClockStep(30, HOURS);
     TestRepository<Repo> repo = createProject("repo");
-    Change change1 = insert(newChange(repo, null, null, null, null));
+    Change change1 = insert(newChange(repo));
-    Change change2 = insert(newChange(repo, null, null, null, null));
+    Change change2 = insert(newChange(repo));
     TestTimeUtil.setClockStep(0, MILLISECONDS);
 
     assertQuery("before:2009-09-29");
@@ -942,8 +904,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   public void byAfter() throws Exception {
     TestTimeUtil.resetWithClockStep(30, HOURS);
     TestRepository<Repo> repo = createProject("repo");
-    Change change1 = insert(newChange(repo, null, null, null, null));
+    Change change1 = insert(newChange(repo));
-    Change change2 = insert(newChange(repo, null, null, null, null));
+    Change change2 = insert(newChange(repo));
     TestTimeUtil.setClockStep(0, MILLISECONDS);
 
     assertQuery("after:2009-10-03");
@@ -964,8 +926,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     RevCommit commit2 = repo.parseBody(
         repo.commit().parent(commit1).add("file1", "foo").create());
 
-    Change change1 = insert(newChange(repo, commit1, null, null, null));
+    Change change1 = insert(newChangeForCommit(repo, commit1));
-    Change change2 = insert(newChange(repo, commit2, null, null, null));
+    Change change2 = insert(newChangeForCommit(repo, commit2));
 
     assertQuery("added:>4");
     assertQuery("-added:<=4");
@@ -1014,8 +976,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
 
   private List<Change> setUpHashtagChanges() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    Change change1 = insert(newChange(repo, null, null, null, null));
+    Change change1 = insert(newChange(repo));
-    Change change2 = insert(newChange(repo, null, null, null, null));
+    Change change2 = insert(newChange(repo));
 
     HashtagsInput in = new HashtagsInput();
     in.add = ImmutableSet.of("foo");
@@ -1057,31 +1019,29 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   public void byDefault() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
 
-    Change change1 = insert(newChange(repo, null, null, null, null));
+    Change change1 = insert(newChange(repo));
 
     RevCommit commit2 = repo.parseBody(
         repo.commit().message("foosubject").create());
-    Change change2 = insert(newChange(repo, commit2, null, null, null));
+    Change change2 = insert(newChangeForCommit(repo, commit2));
 
     RevCommit commit3 = repo.parseBody(
         repo.commit()
         .add("Foo.java", "foo contents")
         .create());
-    Change change3 = insert(newChange(repo, commit3, null, null, null));
+    Change change3 = insert(newChangeForCommit(repo, commit3));
 
-    ChangeInserter ins4 = newChange(repo, null, null, null, null);
+    ChangeInserter ins4 = newChange(repo);
     Change change4 = insert(ins4);
     ReviewInput ri4 = new ReviewInput();
     ri4.message = "toplevel";
     ri4.labels = ImmutableMap.<String, Short> of("Code-Review", (short) 1);
     gApi.changes().id(change4.getId().get()).current().review(ri4);
 
-    ChangeInserter ins5 = newChange(repo, null, null, null, null);
+    ChangeInserter ins5 = newChangeWithTopic(repo, "feature5");
-    Change change5 = ins5.getChange();
+    Change change5 = insert(ins5);
-    change5.setTopic("feature5");
-    insert(ins5);
 
-    Change change6 = insert(newChange(repo, null, null, null, "branch6"));
+    Change change6 = insert(newChangeForBranch(repo, "branch6"));
 
     assertQuery(change1.getId().get(), change1);
     assertQuery(ChangeTriplet.format(change1), change1);
@@ -1102,11 +1062,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void implicitVisibleTo() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    Change change1 = insert(newChange(repo, null, null, userId.get(), null));
+    Change change1 = insert(newChangeForUser(repo, userId));
-    ChangeInserter ins2 = newChange(repo, null, null, userId.get(), null);
+    ChangeInserter ins2 =
-    Change change2 = ins2.getChange();
+        newChange(repo, null, userId, null, Change.Status.DRAFT, null);
-    change2.setStatus(Change.Status.DRAFT);
+    Change change2 = insert(ins2);
-    insert(ins2);
 
     String q = "project:repo";
     assertQuery(q, change2, change1);
@@ -1120,11 +1079,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void explicitVisibleTo() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    Change change1 = insert(newChange(repo, null, null, userId.get(), null));
+    Change change1 = insert(newChangeForUser(repo, userId));
-    ChangeInserter ins2 = newChange(repo, null, null, userId.get(), null);
+    ChangeInserter ins2 =
-    Change change2 = ins2.getChange();
+        newChange(repo, null, userId, null, Change.Status.DRAFT, null);
-    change2.setStatus(Change.Status.DRAFT);
+    Change change2 = insert(ins2);
-    insert(ins2);
 
     String q = "project:repo";
     assertQuery(q, change2, change1);
@@ -1139,8 +1097,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byCommentBy() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    Change change1 = insert(newChange(repo, null, null, null, null));
+    Change change1 = insert(newChange(repo));
-    Change change2 = insert(newChange(repo, null, null, null, null));
+    Change change2 = insert(newChange(repo));
 
     int user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser"))
         .getAccountId().get();
@@ -1165,8 +1123,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byDraftBy() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    Change change1 = insert(newChange(repo, null, null, null, null));
+    Change change1 = insert(newChange(repo));
-    Change change2 = insert(newChange(repo, null, null, null, null));
+    Change change2 = insert(newChange(repo));
 
     DraftInput in = new DraftInput();
     in.line = 1;
@@ -1190,11 +1148,11 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Test
   public void byFrom() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
-    Change change1 = insert(newChange(repo, null, null, null, null));
+    Change change1 = insert(newChange(repo));
 
-    int user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser"))
+    Account.Id user2 = accountManager.authenticate(
-        .getAccountId().get();
+        AuthRequest.forUser("anotheruser")).getAccountId();
-    ChangeInserter ins2 = newChange(repo, null, null, user2, null);
+    ChangeInserter ins2 = newChangeForUser(repo, user2);
     Change change2 = insert(ins2);
 
     ReviewInput input = new ReviewInput();
@@ -1231,10 +1189,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
         repo.commit()
             .add("file4", "contents4")
             .create());
-    Change change1 = insert(newChange(repo, commit1, null, null, null));
+    Change change1 = insert(newChangeForCommit(repo, commit1));
-    Change change2 = insert(newChange(repo, commit2, null, null, null));
+    Change change2 = insert(newChangeForCommit(repo, commit2));
-    Change change3 = insert(newChange(repo, commit3, null, null, null));
+    Change change3 = insert(newChangeForCommit(repo, commit3));
-    Change change4 = insert(newChange(repo, commit4, null, null, null));
+    Change change4 = insert(newChangeForCommit(repo, commit4));
 
     assertQuery("conflicts:" + change1.getId().get(), change3);
     assertQuery("conflicts:" + change2.getId().get());
@@ -1246,9 +1204,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   public void reviewedBy() throws Exception {
     TestTimeUtil.resetWithClockStep(2, MINUTES);
     TestRepository<Repo> repo = createProject("repo");
-    Change change1 = insert(newChange(repo, null, null, null, null));
+    Change change1 = insert(newChange(repo));
-    Change change2 = insert(newChange(repo, null, null, null, null));
+    Change change2 = insert(newChange(repo));
-    Change change3 = insert(newChange(repo, null, null, null, null));
+    Change change3 = insert(newChange(repo));
 
     gApi.changes()
       .id(change1.getId().get())
@@ -1303,7 +1261,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     List<Integer> expectedIds = new ArrayList<>(n);
     Branch.NameKey dest = null;
     for (int i = 0; i < n; i++) {
-      ChangeInserter ins = newChange(repo, null, null, null, null);
+      ChangeInserter ins = newChange(repo);
       insert(ins);
       if (dest == null) {
         dest = ins.getChange().getDest();
@@ -1333,7 +1291,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   public void prepopulatedFields() throws Exception {
     assume().that(notesMigration.enabled()).isFalse();
     TestRepository<Repo> repo = createProject("repo");
-    Change change = insert(newChange(repo, null, null, null, null));
+    Change change = insert(newChange(repo));
 
     db = new DisabledReviewDb();
     requestContext.setContext(newRequestContext(userId));
@@ -1362,7 +1320,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   public void prepopulateOnlyRequestedFields() throws Exception {
     assume().that(notesMigration.enabled()).isFalse();
     TestRepository<Repo> repo = createProject("repo");
-    Change change = insert(newChange(repo, null, null, null, null));
+    Change change = insert(newChange(repo));
 
     db = new DisabledReviewDb();
     requestContext.setContext(newRequestContext(userId));
@@ -1383,14 +1341,44 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     cd.currentApprovals();
   }
 
-  protected ChangeInserter newChange(
+  protected ChangeInserter newChange(TestRepository<Repo> repo)
-      TestRepository<Repo> repo,
+      throws Exception {
-      @Nullable RevCommit commit, @Nullable String key, @Nullable Integer owner,
+    return newChange(repo, null, null, null, null, null);
-      @Nullable String branch) throws Exception {
+  }
+
+  protected ChangeInserter newChangeForCommit(TestRepository<Repo> repo,
+      RevCommit commit) throws Exception {
+    return newChange(repo, commit, null, null, null, null);
+  }
+
+  protected ChangeInserter newChangeForUser(TestRepository<Repo> repo,
+      Account.Id owner) throws Exception {
+    return newChange(repo, null, owner, null, null, null);
+  }
+
+  protected ChangeInserter newChangeForBranch(TestRepository<Repo> repo,
+      String branch) throws Exception {
+    return newChange(repo, null, null, branch, null, null);
+  }
+
+  protected ChangeInserter newChangeWithStatus(TestRepository<Repo> repo,
+      Change.Status status) throws Exception {
+    return newChange(repo, null, null, null, status, null);
+  }
+
+  protected ChangeInserter newChangeWithTopic(TestRepository<Repo> repo,
+      String topic) throws Exception {
+    return newChange(repo, null, null, null, null, topic);
+  }
+
+  protected ChangeInserter newChange(TestRepository<Repo> repo,
+      @Nullable RevCommit commit, @Nullable Account.Id owner,
+      @Nullable String branch, @Nullable Change.Status status,
+      @Nullable String topic) throws Exception {
     if (commit == null) {
       commit = repo.parseBody(repo.commit().message("message").create());
     }
-    Account.Id ownerId = owner != null ? new Account.Id(owner) : userId;
+    Account.Id ownerId = owner != null ? owner : userId;
     branch = MoreObjects.firstNonNull(branch, "refs/heads/master");
     if (!branch.startsWith("refs/heads/")) {
       branch = "refs/heads/" + branch;
@@ -1399,30 +1387,19 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
         repo.getRepository().getDescription().getRepositoryName());
 
     Change.Id id = new Change.Id(seq.nextChangeId());
-    if (key == null) {
-      key = "I" + Hashing.sha1().newHasher()
-          .putInt(id.get())
-          .putString(project.get(), UTF_8)
-          .putString(commit.name(), UTF_8)
-          .putInt(ownerId.get())
-          .putString(branch, UTF_8)
-          .hash()
-          .toString();
-    }
-
-    Change change = new Change(new Change.Key(key), id, ownerId,
-        new Branch.NameKey(project, branch), TimeUtil.nowTs());
     IdentifiedUser user = userFactory.create(Providers.of(db), ownerId);
     RefControl refControl = projectControlFactory.controlFor(project, user)
-        .controlForRef(change.getDest());
+        .controlForRef(new Branch.NameKey(project, branch));
-    return changeFactory.create(refControl, change, commit)
+    ChangeInserter ins = changeFactory.create(refControl, id, commit)
-        .setValidatePolicy(CommitValidators.Policy.NONE);
+        .setValidatePolicy(CommitValidators.Policy.NONE)
+        .setStatus(status)
+        .setTopic(topic);
+    return ins;
   }
 
   protected Change insert(ChangeInserter ins) throws Exception {
-    try (BatchUpdate bu = updateFactory.create(
+    try (BatchUpdate bu = updateFactory.create(db, ins.getProject(),
-        db, ins.getChange().getProject(), ins.getUser(),
+        ins.getUser(), TimeUtil.nowTs())) {
-        ins.getChange().getCreatedOn())) {
       bu.insertChange(ins);
       bu.execute();
       return ins.getChange();

gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java

@@ -38,10 +38,10 @@ public class LuceneQueryChangesTest extends AbstractQueryChangesTest {
     TestRepository<Repo> repo = createProject("repo");
     RevCommit commit1 =
         repo.parseBody(repo.commit().message("foo_bar_foo").create());
-    Change change1 = insert(newChange(repo, commit1, null, null, null));
+    Change change1 = insert(newChangeForCommit(repo, commit1));
     RevCommit commit2 =
         repo.parseBody(repo.commit().message("one.two.three").create());
-    Change change2 = insert(newChange(repo, commit2, null, null, null));
+    Change change2 = insert(newChangeForCommit(repo, commit2));
 
     assertQuery("message:foo_ba");
     assertQuery("message:bar", change1);