Clean up ChangeInserter similar to PatchSetInserter

Remove the commitMessageNotForChange branch. CherryPickChange doesn't
even use this anymore. Don't require setting the ChangeMessage, just
the message string.

Unlike PatchSetInserter, ChangeInserter creates the PatchSet
immediately, because it doesn't have to scan refs to determine the
next patch set ID (it's always 1).

Change-Id: I166374c197413cad8d9036790d252154f9a0b9aa

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

@@ -23,7 +23,6 @@ import com.google.common.collect.Ordering;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 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.Project;
@@ -311,22 +310,18 @@ public class ChangeUtil {
             change.getDest().getParentKey().get(), ru.getResult()));
       }
 
-      ChangeMessage cmsg = new ChangeMessage(
-          new ChangeMessage.Key(changeId, messageUUID(db.get())),
-          user().getAccountId(), TimeUtil.nowTs(), patchSetId);
       StringBuilder msgBuf = new StringBuilder();
       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());
-      cmsg.setMessage(msgBuf.toString());
 
-      ins.setMessage(cmsg).insert();
+      ins.setMessage(msgBuf.toString()).insert();
 
       try {
         RevertedSender cm = revertedSenderFactory.create(change.getId());
         cm.setFrom(user().getAccountId());
-        cm.setChangeMessage(cmsg);
+        cm.setChangeMessage(ins.getChangeMessage());
         cm.send();
       } catch (Exception err) {
         log.error("Cannot send email for revert change " + change.getId(),

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

@@ -14,9 +14,10 @@
 
 package com.google.gerrit.server.change;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
 import static com.google.gerrit.reviewdb.client.Change.INITIAL_PATCH_SET_ID;
 
-import com.google.common.util.concurrent.CheckedFuture;
 import com.google.gerrit.common.ChangeHooks;
 import com.google.gerrit.common.data.LabelTypes;
 import com.google.gerrit.extensions.api.changes.HashtagsInput;
@@ -31,6 +32,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ApprovalsUtil;
 import com.google.gerrit.server.ChangeMessagesUtil;
 import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.git.GroupCollector;
@@ -79,12 +81,14 @@ public class ChangeInserter {
   private final WorkQueue workQueue;
 
   private final ProjectControl projectControl;
+  private final IdentifiedUser user;
   private final Change change;
   private final PatchSet patchSet;
   private final RevCommit commit;
   private final PatchSetInfo patchSetInfo;
 
-  private ChangeMessage changeMessage;
+  // Fields exposed as setters.
+  private String message;
   private Set<Account.Id> reviewers;
   private Set<Account.Id> extraCC;
   private Map<String, Short> approvals;
@@ -93,6 +97,9 @@ public class ChangeInserter {
   private boolean runHooks;
   private boolean sendMail;
 
+  // Fields set during the insertion process.
+  private ChangeMessage changeMessage;
+
   @Inject
   ChangeInserter(Provider<ReviewDb> dbProvider,
       ChangeUpdate.Factory updateFactory,
@@ -130,6 +137,7 @@ public class ChangeInserter {
     this.runHooks = true;
     this.sendMail = true;
 
+    user = checkUser(projectControl);
     patchSet =
         new PatchSet(new PatchSet.Id(change.getId(), INITIAL_PATCH_SET_ID));
     patchSet.setCreatedOn(change.getCreatedOn());
@@ -139,12 +147,18 @@ public class ChangeInserter {
     change.setCurrentPatchSet(patchSetInfo);
   }
 
+  private static IdentifiedUser checkUser(ProjectControl ctl) {
+    checkArgument(ctl.getCurrentUser().isIdentifiedUser(),
+        "only IdentifiedUser may create change");
+    return (IdentifiedUser) ctl.getCurrentUser();
+  }
+
   public Change getChange() {
     return change;
   }
 
-  public ChangeInserter setMessage(ChangeMessage changeMessage) {
+  public ChangeInserter setMessage(String message) {
-    this.changeMessage = changeMessage;
+    this.message = message;
     return this;
   }
 
@@ -202,6 +216,15 @@ public class ChangeInserter {
     return patchSetInfo;
   }
 
+  public ChangeMessage getChangeMessage() {
+    if (message == null) {
+      return null;
+    }
+    checkState(changeMessage != null,
+        "getChangeMessage() only valid after inserting change");
+    return changeMessage;
+  }
+
   public Change insert() throws OrmException, IOException {
     ReviewDb db = dbProvider.get();
     ChangeControl ctl = projectControl.controlFor(change);
@@ -221,7 +244,12 @@ public class ChangeInserter {
           patchSet, patchSetInfo, reviewers, Collections.<Account.Id> emptySet());
       approvalsUtil.addApprovals(db, update, labelTypes, patchSet, patchSetInfo,
           ctl, approvals);
-      if (messageIsForChange()) {
+      if (message != null) {
+        changeMessage =
+            new ChangeMessage(new ChangeMessage.Key(change.getId(),
+                ChangeUtil.messageUUID(db)), user.getAccountId(),
+                patchSet.getCreatedOn(), patchSet.getId());
+        changeMessage.setMessage(message);
         cmUtil.addChangeMessage(db, update, changeMessage);
       }
       db.commit();
@@ -241,11 +269,7 @@ public class ChangeInserter {
       }
     }
 
-    CheckedFuture<?, IOException> f = indexer.indexAsync(change.getId());
+    indexer.index(db, change);
-    if (!messageIsForChange()) {
-      commitMessageNotForChange();
-    }
-    f.checkedGet();
 
     if (sendMail) {
       Runnable sender = new Runnable() {
@@ -290,29 +314,4 @@ public class ChangeInserter {
 
     return change;
   }
-
-  private void commitMessageNotForChange() throws OrmException,
-      IOException {
-    ReviewDb db = dbProvider.get();
-    if (changeMessage != null) {
-      Change otherChange =
-          db.changes().get(changeMessage.getPatchSetId().getParentKey());
-      ChangeUtil.bumpRowVersionNotLastUpdatedOn(
-          changeMessage.getKey().getParentKey(), db);
-      ChangeControl otherControl = projectControl.controlFor(otherChange);
-      ChangeUpdate updateForOtherChange =
-          updateFactory.create(otherControl, change.getLastUpdatedOn());
-      cmUtil.addChangeMessage(db, updateForOtherChange, changeMessage);
-      updateForOtherChange.commit();
-    }
-  }
-
-  private boolean messageIsForChange() {
-    if (changeMessage == null) {
-      return false;
-    }
-    Change.Id id = change.getId();
-    Change.Id msgId = changeMessage.getKey().getParentKey();
-    return msgId.equals(id);
-  }
 }

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

@@ -32,7 +32,6 @@ 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.ChangeMessage;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
@@ -204,19 +203,11 @@ public class CreateChange implements
           new Branch.NameKey(project, refName),
           now);
 
-      ChangeInserter ins =
+      ChangeInserter ins = changeInserterFactory
-          changeInserterFactory.create(refControl.getProjectControl(),
+          .create(refControl.getProjectControl(), change, c);
-              change, c);
+      ins.setMessage(String.format("Uploaded patch set %s.",
-
-      ChangeMessage msg = new ChangeMessage(new ChangeMessage.Key(change.getId(),
-          ChangeUtil.messageUUID(db.get())),
-          me.getAccountId(),
-          ins.getPatchSet().getCreatedOn(),
-          ins.getPatchSet().getId());
-      msg.setMessage(String.format("Uploaded patch set %s.",
           ins.getPatchSet().getPatchSetId()));
 
-      ins.setMessage(msg);
       validateCommit(git, refControl, c, me, ins);
       updateRef(git, rw, c, change, ins.getPatchSet());
 

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

@@ -1726,19 +1726,12 @@ public class ReceiveCommits {
     CheckedFuture<Void, RestApiException> insertChange() throws IOException {
       rp.getRevWalk().parseBody(commit);
 
-      final Thread caller = Thread.currentThread();
       ListenableFuture<Void> future = changeUpdateExector.submit(
           requestScopePropagator.wrap(new Callable<Void>() {
         @Override
         public Void call() throws OrmException, IOException,
             ResourceConflictException {
-          if (caller == Thread.currentThread()) {
+          insertChangeImpl();
-            insertChange(db);
-          } else {
-            try (ReviewDb db = schemaFactory.open()) {
-              insertChange(db);
-            }
-          }
           synchronized (newProgress) {
             newProgress.update(1);
           }
@@ -1748,7 +1741,7 @@ public class ReceiveCommits {
       return Futures.makeChecked(future, INSERT_EXCEPTION);
     }
 
-    private void insertChange(ReviewDb db) throws OrmException, IOException,
+    private void insertChangeImpl() throws OrmException, IOException,
         ResourceConflictException {
       final PatchSet ps = ins.setGroups(groups).getPatchSet();
       final Account.Id me = currentUser.getAccountId();
@@ -1762,17 +1755,11 @@ public class ReceiveCommits {
       }
       recipients.add(getRecipientsFromFooters(accountResolver, ps, footerLines));
       recipients.remove(me);
-
-      ChangeMessage msg =
-          new ChangeMessage(new ChangeMessage.Key(change.getId(),
-              ChangeUtil.messageUUID(db)), me, ps.getCreatedOn(), ps.getId());
-      msg.setMessage("Uploaded patch set " + ps.getPatchSetId() + ".");
-
       ins
         .setReviewers(recipients.getReviewers())
         .setExtraCC(recipients.getCcOnly())
         .setApprovals(approvals)
-        .setMessage(msg)
+        .setMessage("Uploaded patch set " + ps.getPatchSetId() + ".")
         .setRequestScopePropagator(requestScopePropagator)
         .setSendMail(true)
         .insert();