Create change ref from ChangeInserter

Reduce some duplicated code. However, there are still a few special
snowflakes, so leave a flag to turn this off:
 - ReceiveCommits manages its own BatchRefUpdate. (It also doesn't
   even use PatchSetInserter for the new patch set case.)
 - ReviewProjectAccess updates the ref using VersionedMetaData, which
   I'm not going to touch for now.

Change-Id: I6f4f9fb8a6364627a737698d1001ad29ce8a981a

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

@@ -124,10 +124,10 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
             RefNames.REFS_CONFIG),
         TimeUtil.nowTs());
     try (RevWalk rw = new RevWalk(md.getRepository())) {
-      ChangeInserter ins = changeInserterFactory.create(
+      changeInserterFactory.create(md.getRepository(), rw, ctl, change, commit)
-              md.getRepository(), rw, ctl, change, commit)
+          .setValidatePolicy(CommitValidators.Policy.NONE)
-          .setValidatePolicy(CommitValidators.Policy.NONE);
+          .setUpdateRef(false) // Created by commitToNewRef.
-      ins.insert();
+          .insert();
     } catch (InvalidChangeOperationException e) {
       throw new IOException(e);
     }

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

@@ -276,26 +276,13 @@ public class ChangeUtil {
       ChangeInserter ins = changeInserterFactory.create(
             git, revWalk, refControl.getProjectControl(), change, revertCommit)
           .setValidatePolicy(CommitValidators.Policy.GERRIT);
-      ins.validate();
-
-      PatchSet ps = ins.getPatchSet();
-      RefUpdate ru = git.updateRef(ps.getRefName());
-      ru.setExpectedOldObjectId(ObjectId.zeroId());
-      ru.setNewObjectId(revertCommit);
-      ru.disableRefLog();
-      if (ru.update(revWalk) != RefUpdate.Result.NEW) {
-        throw new IOException(String.format(
-            "Failed to create ref %s in %s: %s", ps.getRefName(),
-            change.getDest().getParentKey().get(), ru.getResult()));
-      }
-
       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());
-
+      ins.setMessage(msgBuf.toString())
-      ins.setMessage(msgBuf.toString()).insert();
+          .insert();
 
       try {
         RevertedSender cm = revertedSenderFactory.create(change.getId());

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

@@ -59,6 +59,7 @@ import com.google.inject.Provider;
 import com.google.inject.assistedinject.Assisted;
 
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.notes.NoteMap;
 import org.eclipse.jgit.revwalk.RevCommit;
@@ -114,10 +115,10 @@ public class ChangeInserter {
   private RequestScopePropagator requestScopePropagator;
   private boolean runHooks;
   private boolean sendMail;
+  private boolean updateRef;
 
   // Fields set during the insertion process.
   private ChangeMessage changeMessage;
-  private boolean validated; // TODO(dborowitz): Remove; see validate().
 
   @Inject
   ChangeInserter(Provider<ReviewDb> dbProvider,
@@ -162,6 +163,7 @@ public class ChangeInserter {
     this.hashtags = Collections.emptySet();
     this.runHooks = true;
     this.sendMail = true;
+    this.updateRef = true;
 
     user = checkUser(projectControl);
     patchSet =
@@ -243,6 +245,11 @@ public class ChangeInserter {
     return this;
   }
 
+  public ChangeInserter setUpdateRef(boolean updateRef) {
+    this.updateRef = updateRef;
+    return this;
+  }
+
   public PatchSetInfo getPatchSetInfo() {
     return patchSetInfo;
   }
@@ -260,6 +267,8 @@ public class ChangeInserter {
       throws OrmException, IOException, InvalidChangeOperationException {
     validate();
 
+    updateRef();
+
     ReviewDb db = dbProvider.get();
     ProjectControl projectControl = refControl.getProjectControl();
     ChangeControl ctl = projectControl.controlFor(change);
@@ -350,11 +359,23 @@ public class ChangeInserter {
     return change;
   }
 
-  // TODO(dborowitz): This is only public because callers expect validation to
+  private void updateRef() throws IOException {
-  // happen before updating any refs, and they are still updating refs manually.
+    if (!updateRef) {
-  // Make private once we have migrated ref updates into this class.
+      return;
-  public void validate() throws IOException, InvalidChangeOperationException {
+    }
-    if (validated || validatePolicy == CommitValidators.Policy.NONE) {
+    RefUpdate ru = git.updateRef(patchSet.getRefName());
+    ru.setExpectedOldObjectId(ObjectId.zeroId());
+    ru.setNewObjectId(commit);
+    ru.disableRefLog();
+    if (ru.update(revWalk) != RefUpdate.Result.NEW) {
+      throw new IOException(String.format(
+          "Failed to create ref %s in %s: %s", ru.getRef().getName(),
+          change.getDest().getParentKey().get(), ru.getResult()));
+    }
+  }
+
+  private void validate() throws IOException, InvalidChangeOperationException {
+    if (validatePolicy == CommitValidators.Policy.NONE) {
       return;
     }
     CommitValidators cv =
@@ -386,6 +407,5 @@ public class ChangeInserter {
     } catch (CommitValidationException e) {
       throw new InvalidChangeOperationException(e.getMessage());
     }
-    validated = true;
   }
 }

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

@@ -60,7 +60,6 @@ import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.util.ChangeIdUtil;
@@ -241,24 +240,9 @@ public class CherryPickChange {
             git, revWalk, refControl.getProjectControl(), change,
             cherryPickCommit)
         .setValidatePolicy(CommitValidators.Policy.GERRIT);
-    ins.setMessage(
+    return ins.setMessage(
-        messageForDestinationChange(ins.getPatchSet().getId(), sourceBranch));
+          messageForDestinationChange(ins.getPatchSet().getId(), sourceBranch))
-    ins.validate();
+        .insert();
-    PatchSet newPatchSet = ins.getPatchSet();
-
-    final RefUpdate ru = git.updateRef(newPatchSet.getRefName());
-    ru.setExpectedOldObjectId(ObjectId.zeroId());
-    ru.setNewObjectId(cherryPickCommit);
-    ru.disableRefLog();
-    if (ru.update(revWalk) != RefUpdate.Result.NEW) {
-      throw new IOException(String.format(
-          "Failed to create ref %s in %s: %s", newPatchSet.getRefName(),
-          change.getDest().getParentKey().get(), ru.getResult()));
-    }
-
-    ins.insert();
-
-    return change;
   }
 
   private void addMessageToSourceChange(Change change, PatchSet.Id patchSetId,

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

@@ -58,7 +58,6 @@ import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
@@ -201,10 +200,6 @@ public class CreateChange implements
           .setValidatePolicy(CommitValidators.Policy.GERRIT);
       ins.setMessage(String.format("Uploaded patch set %s.",
           ins.getPatchSet().getPatchSetId()));
-      ins.validate();
-
-      updateRef(git, rw, c, change, ins.getPatchSet());
-
       String topic = input.topic;
       if (topic != null) {
         topic = Strings.emptyToNull(topic.trim());
@@ -219,19 +214,6 @@ public class CreateChange implements
     }
   }
 
-  private static void updateRef(Repository git, RevWalk rw, RevCommit c,
-      Change change, PatchSet newPatchSet) throws IOException {
-    RefUpdate ru = git.updateRef(newPatchSet.getRefName());
-    ru.setExpectedOldObjectId(ObjectId.zeroId());
-    ru.setNewObjectId(c);
-    ru.disableRefLog();
-    if (ru.update(rw) != RefUpdate.Result.NEW) {
-      throw new IOException(String.format(
-          "Failed to create ref %s in %s: %s", newPatchSet.getRefName(),
-          change.getDest().getParentKey().get(), ru.getResult()));
-    }
-  }
-
   private static Change.Key getChangeId(ObjectId id, RevCommit emptyCommit) {
     List<String> idList = emptyCommit.getFooterLines(
         FooterConstants.CHANGE_ID);

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

@@ -1766,6 +1766,7 @@ public class ReceiveCommits {
         .setMessage("Uploaded patch set " + ps.getPatchSetId() + ".")
         .setRequestScopePropagator(requestScopePropagator)
         .setSendMail(true)
+        .setUpdateRef(false)
         .insert();
       created = true;