Move PatchSet creation into ChangeInserter

This also fixes some probably-unnoticeable inconsistencies in
commit/change/patch set timestamps. Some callers generated new
timestamps for the patch set, and some didn't. Also, the calls to
ChangeUtil.updated(change) before the change was written had been
overriding the manually-set timestamp.

Change-Id: I76bbf9cbed0fd921cef3a2c2847fb03856c6bcb7

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

@@ -20,7 +20,6 @@ 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.PatchSetAncestor;
-import com.google.gerrit.reviewdb.client.PatchSetInfo;
 import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.client.TrackingId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -249,13 +248,9 @@ public class ChangeUtil {
           user.getAccountId(),
           changeToRevert.getDest());
       change.setTopic(changeToRevert.getTopic());
-
+      ChangeInserter ins =
-      PatchSet.Id id =
+          changeInserterFactory.create(refControl, change, revertCommit);
-          new PatchSet.Id(change.getId(), Change.INITIAL_PATCH_SET_ID);
+      PatchSet ps = ins.getPatchSet();
-      final PatchSet ps = new PatchSet(id);
-      ps.setCreatedOn(change.getCreatedOn());
-      ps.setUploader(change.getOwner());
-      ps.setRevision(new RevId(revertCommit.name()));
 
       CommitReceivedEvent commitReceivedEvent =
           new CommitReceivedEvent(new ReceiveCommand(ObjectId.zeroId(),
@@ -269,11 +264,6 @@ public class ChangeUtil {
         throw new InvalidChangeOperationException(e.getMessage());
       }
 
-      PatchSetInfo info = patchSetInfoFactory.get(revertCommit, ps.getId());
-      change.setCurrentPatchSet(info);
-      ChangeUtil.updated(change);
-
-
       final RefUpdate ru = git.updateRef(ps.getRefName());
       ru.setExpectedOldObjectId(ObjectId.zeroId());
       ru.setNewObjectId(revertCommit);
@@ -293,9 +283,7 @@ public class ChangeUtil {
       msgBuf.append("This patchset was reverted in change: " + change.getKey().get());
       cmsg.setMessage(msgBuf.toString());
 
-      changeInserterFactory.create(refControl, change, ps, revertCommit, info)
+      ins.setMessage(cmsg).insert();
-          .setMessage(cmsg)
-          .insert();
 
       final RevertedSender cm = revertedSenderFactory.create(change);
       cm.setFrom(user.getAccountId());

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

@@ -14,6 +14,8 @@
 
 package com.google.gerrit.server.change;
 
+import static com.google.gerrit.reviewdb.client.Change.INITIAL_PATCH_SET_ID;
+
 import com.google.gerrit.common.ChangeHooks;
 import com.google.gerrit.common.data.LabelTypes;
 import com.google.gerrit.reviewdb.client.Account;
@@ -21,11 +23,13 @@ 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.RevId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ApprovalsUtil;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.config.TrackingFooters;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.patch.PatchSetInfoFactory;
 import com.google.gerrit.server.project.RefControl;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -40,8 +44,7 @@ import java.util.Set;
 
 public class ChangeInserter {
   public static interface Factory {
-    ChangeInserter create(RefControl ctl, Change c, PatchSet ps, RevCommit rc,
+    ChangeInserter create(RefControl ctl, Change c, RevCommit rc);
-        PatchSetInfo psi);
   }
 
   private final Provider<ReviewDb> dbProvider;
@@ -58,18 +61,18 @@ public class ChangeInserter {
 
   private ChangeMessage changeMessage;
   private Set<Account.Id> reviewers;
+  private boolean draft;
 
   @Inject
   ChangeInserter(Provider<ReviewDb> dbProvider,
+      PatchSetInfoFactory patchSetInfoFactory,
       GitReferenceUpdated gitRefUpdated,
       ChangeHooks hooks,
       ApprovalsUtil approvalsUtil,
       TrackingFooters trackingFooters,
       @Assisted RefControl refControl,
       @Assisted Change change,
-      @Assisted PatchSet patchSet,
+      @Assisted RevCommit commit) {
-      @Assisted RevCommit commit,
-      @Assisted PatchSetInfo patchSetInfo) {
     this.dbProvider = dbProvider;
     this.gitRefUpdated = gitRefUpdated;
     this.hooks = hooks;
@@ -77,11 +80,22 @@ public class ChangeInserter {
     this.trackingFooters = trackingFooters;
     this.refControl = refControl;
     this.change = change;
-    this.patchSet = patchSet;
     this.commit = commit;
-    this.patchSetInfo = patchSetInfo;
 
     this.reviewers = Collections.emptySet();
+
+    patchSet =
+        new PatchSet(new PatchSet.Id(change.getId(), INITIAL_PATCH_SET_ID));
+    patchSet.setCreatedOn(change.getCreatedOn());
+    patchSet.setUploader(change.getOwner());
+    patchSet.setRevision(new RevId(commit.name()));
+    if (draft) {
+      change.setStatus(Change.Status.DRAFT);
+      patchSet.setDraft(true);
+    }
+    patchSetInfo = patchSetInfoFactory.get(commit, patchSet.getId());
+    change.setCurrentPatchSet(patchSetInfo);
+    ChangeUtil.computeSortKey(change);
   }
 
   public ChangeInserter setMessage(ChangeMessage changeMessage) {
@@ -94,6 +108,19 @@ public class ChangeInserter {
     return this;
   }
 
+  public ChangeInserter setDraft(boolean draft) {
+    this.draft = draft;
+    return this;
+  }
+
+  public PatchSet getPatchSet() {
+    return patchSet;
+  }
+
+  public PatchSetInfo getPatchSetInfo() {
+    return patchSetInfo;
+  }
+
   public void insert() throws OrmException {
     ReviewDb db = dbProvider.get();
     db.changes().beginTransaction(change.getId());

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

@@ -19,9 +19,7 @@ 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.ChangeUtil;
 import com.google.gerrit.server.GerritPersonIdent;
@@ -57,7 +55,6 @@ import org.eclipse.jgit.transport.ReceiveCommand;
 import org.eclipse.jgit.util.ChangeIdUtil;
 
 import java.io.IOException;
-import java.sql.Timestamp;
 import java.util.List;
 
 public class CherryPickChange {
@@ -214,11 +211,9 @@ public class CherryPickChange {
         new Change(changeKey, new Change.Id(db.nextChangeId()),
             currentUser.getAccountId(), new Branch.NameKey(project,
                 destRef.getName()));
-    PatchSet.Id id = new PatchSet.Id(change.getId(), Change.INITIAL_PATCH_SET_ID);
+    ChangeInserter ins =
-    PatchSet newPatchSet = new PatchSet(id);
+        changeInserterFactory.create(refControl, change, cherryPickCommit);
-    newPatchSet.setCreatedOn(new Timestamp(System.currentTimeMillis()));
+    PatchSet newPatchSet = ins.getPatchSet();
-    newPatchSet.setUploader(change.getOwner());
-    newPatchSet.setRevision(new RevId(cherryPickCommit.name()));
 
     CommitValidators commitValidators =
         commitValidatorsFactory.create(refControl, new NoSshInfo(), git);
@@ -244,16 +239,7 @@ public class CherryPickChange {
           change.getDest().getParentKey().get(), ru.getResult()));
     }
 
-    PatchSetInfo newPatchSetInfo =
+    ins.setMessage(buildChangeMessage(patchSetId, change)).insert();
-        patchSetInfoFactory.get(cherryPickCommit, newPatchSet.getId());
-    change.setCurrentPatchSet(newPatchSetInfo);
-    ChangeUtil.updated(change);
-
-    changeInserterFactory
-        .create(refControl, change, newPatchSet, cherryPickCommit,
-            newPatchSetInfo)
-        .setMessage(buildChangeMessage(patchSetId, change))
-        .insert();
 
     return change.getId();
   }

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

@@ -14,10 +14,10 @@
 
 package com.google.gerrit.server.git;
 
-import static com.google.gerrit.reviewdb.client.Change.INITIAL_PATCH_SET_ID;
 import static com.google.gerrit.server.git.MultiProgressMonitor.UNKNOWN;
 import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromApprovals;
 import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters;
+
 import static org.eclipse.jgit.lib.Constants.R_HEADS;
 import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED;
 import static org.eclipse.jgit.transport.ReceiveCommand.Result.OK;
@@ -1458,37 +1458,21 @@ public class ReceiveCommits {
   private class CreateRequest {
     final RevCommit commit;
     final Change change;
-    final PatchSet ps;
     final ReceiveCommand cmd;
-    private final RefControl ctl;
+    final ChangeInserter ins;
-    private final PatchSetInfo info;
     boolean created;
 
     CreateRequest(RefControl ctl, RevCommit c, Change.Key changeKey)
         throws OrmException {
-      this.ctl = ctl;
       commit = c;
-
       change = new Change(changeKey,
           new Change.Id(db.nextChangeId()),
           currentUser.getAccountId(),
           magicBranch.dest);
       change.setTopic(magicBranch.topic);
-
+      ins = changeInserterFactory.create(ctl, change, c);
-      ps = new PatchSet(new PatchSet.Id(change.getId(), INITIAL_PATCH_SET_ID));
+      cmd = new ReceiveCommand(ObjectId.zeroId(), c,
-      ps.setCreatedOn(change.getCreatedOn());
+          ins.getPatchSet().getRefName());
-      ps.setUploader(change.getOwner());
-      ps.setRevision(toRevId(c));
-
-      if (magicBranch.isDraft()) {
-        change.setStatus(Change.Status.DRAFT);
-        ps.setDraft(true);
-      }
-
-      info = patchSetInfoFactory.get(c, ps.getId());
-      change.setCurrentPatchSet(info);
-      ChangeUtil.updated(change);
-      cmd = new ReceiveCommand(ObjectId.zeroId(), c, ps.getRefName());
     }
 
     CheckedFuture<Void, OrmException> insertChange() throws IOException {
@@ -1519,6 +1503,7 @@ public class ReceiveCommits {
     }
 
     private void insertChange(ReviewDb db) throws OrmException {
+      final PatchSet ps = ins.getPatchSet();
       final Account.Id me = currentUser.getAccountId();
       final List<FooterLine> footerLines = commit.getFooterLines();
       final MailRecipients recipients = new MailRecipients();
@@ -1528,10 +1513,7 @@ public class ReceiveCommits {
       recipients.add(getRecipientsFromFooters(accountResolver, ps, footerLines));
       recipients.remove(me);
 
-      changeInserterFactory.create(ctl, change, ps, commit, info)
+      ins.setReviewers(recipients.getReviewers()).insert();
-          .setReviewers(recipients.getReviewers())
-          .insert();
-
       created = true;
 
       workQueue.getDefaultQueue()
@@ -1542,7 +1524,7 @@ public class ReceiveCommits {
             CreateChangeSender cm =
                 createChangeSenderFactory.create(change);
             cm.setFrom(me);
-            cm.setPatchSet(ps, info);
+            cm.setPatchSet(ps, ins.getPatchSetInfo());
             cm.addReviewers(recipients.getReviewers());
             cm.addExtraCC(recipients.getCcOnly());
             cm.send();