Convert ChangeInserter to the builder pattern

Like PatchSetInserter, the goal is to cut down on proliferating
methods and reuse this code from more places. However, because of the
way it is currently used from ReceiveCommits, some things like commit
validation and ref updates will be harder to move into ChangeInserter.

Change-Id: I457c92ffeb7f5de1fa9aa8f0da21e4b2b3fed1b1

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

@@ -15,9 +15,7 @@
 package com.google.gerrit.server;
 
 import com.google.gerrit.common.ChangeHooks;
-import com.google.gerrit.common.data.LabelTypes;
 import com.google.gerrit.common.errors.EmailException;
-import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.gerrit.reviewdb.client.PatchSet;
@@ -196,7 +194,7 @@ public class ChangeUtil {
       ReviewDb db, RevertedSender.Factory revertedSenderFactory,
       ChangeHooks hooks, Repository git,
       PatchSetInfoFactory patchSetInfoFactory, PersonIdent myIdent,
-      ChangeInserter changeInserter)
+      ChangeInserter.Factory changeInserterFactory)
           throws NoSuchChangeException, EmailException,
       OrmException, MissingObjectException, IncorrectObjectTypeException,
       IOException, InvalidChangeOperationException {
@@ -295,9 +293,9 @@ public class ChangeUtil {
       msgBuf.append("This patchset was reverted in change: " + change.getKey().get());
       cmsg.setMessage(msgBuf.toString());
 
-      LabelTypes labelTypes = refControl.getProjectControl().getLabelTypes();
-      changeInserter.insertChange(db, change, cmsg, ps, revertCommit,
-          labelTypes, info, Collections.<Account.Id> emptySet());
+      changeInserterFactory.create(refControl, change, ps, revertCommit, info)
+          .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

@@ -26,8 +26,11 @@ 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.project.RefControl;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.assistedinject.Assisted;
 
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.revwalk.RevCommit;
@@ -36,40 +39,72 @@ import java.util.Collections;
 import java.util.Set;
 
 public class ChangeInserter {
+  public static interface Factory {
+    ChangeInserter create(RefControl ctl, Change c, PatchSet ps, RevCommit rc,
+        PatchSetInfo psi);
+  }
+
+  private final Provider<ReviewDb> dbProvider;
   private final GitReferenceUpdated gitRefUpdated;
   private final ChangeHooks hooks;
   private final ApprovalsUtil approvalsUtil;
   private final TrackingFooters trackingFooters;
 
+  private final RefControl refControl;
+  private final Change change;
+  private final PatchSet patchSet;
+  private final RevCommit commit;
+  private final PatchSetInfo patchSetInfo;
+
+  private ChangeMessage changeMessage;
+  private Set<Account.Id> reviewers;
+
   @Inject
-  public ChangeInserter(final GitReferenceUpdated gitRefUpdated,
-      ChangeHooks hooks, ApprovalsUtil approvalsUtil,
-      TrackingFooters trackingFooters) {
+  ChangeInserter(Provider<ReviewDb> dbProvider,
+      GitReferenceUpdated gitRefUpdated,
+      ChangeHooks hooks,
+      ApprovalsUtil approvalsUtil,
+      TrackingFooters trackingFooters,
+      @Assisted RefControl refControl,
+      @Assisted Change change,
+      @Assisted PatchSet patchSet,
+      @Assisted RevCommit commit,
+      @Assisted PatchSetInfo patchSetInfo) {
+    this.dbProvider = dbProvider;
     this.gitRefUpdated = gitRefUpdated;
     this.hooks = hooks;
     this.approvalsUtil = approvalsUtil;
     this.trackingFooters = trackingFooters;
+    this.refControl = refControl;
+    this.change = change;
+    this.patchSet = patchSet;
+    this.commit = commit;
+    this.patchSetInfo = patchSetInfo;
+
+    this.reviewers = Collections.emptySet();
   }
 
-  public void insertChange(ReviewDb db, Change change, PatchSet ps,
-      RevCommit commit, LabelTypes labelTypes, PatchSetInfo info,
-      Set<Account.Id> reviewers) throws OrmException {
-    insertChange(db, change, null, ps, commit, labelTypes, info, reviewers);
+  public ChangeInserter setMessage(ChangeMessage changeMessage) {
+    this.changeMessage = changeMessage;
+    return this;
   }
 
-  public void insertChange(ReviewDb db, Change change,
-      ChangeMessage changeMessage, PatchSet ps, RevCommit commit,
-      LabelTypes labelTypes, PatchSetInfo info, Set<Account.Id> reviewers)
-      throws OrmException {
+  public ChangeInserter setReviewers(Set<Account.Id> reviewers) {
+    this.reviewers = reviewers;
+    return this;
+  }
 
+  public void insert() throws OrmException {
+    ReviewDb db = dbProvider.get();
     db.changes().beginTransaction(change.getId());
     try {
-      ChangeUtil.insertAncestors(db, ps.getId(), commit);
-      db.patchSets().insert(Collections.singleton(ps));
+      ChangeUtil.insertAncestors(db, patchSet.getId(), commit);
+      db.patchSets().insert(Collections.singleton(patchSet));
       db.changes().insert(Collections.singleton(change));
       ChangeUtil.updateTrackingIds(db, change, trackingFooters, commit.getFooterLines());
-      approvalsUtil.addReviewers(db, labelTypes, change, ps, info, reviewers,
-          Collections.<Account.Id> emptySet());
+      LabelTypes labelTypes = refControl.getProjectControl().getLabelTypes();
+      approvalsUtil.addReviewers(db, labelTypes, change, patchSet, patchSetInfo,
+          reviewers, Collections.<Account.Id> emptySet());
       db.commit();
     } finally {
       db.rollback();
@@ -78,8 +113,8 @@ public class ChangeInserter {
       db.changeMessages().insert(Collections.singleton(changeMessage));
     }
 
-    gitRefUpdated.fire(change.getProject(), ps.getRefName(), ObjectId.zeroId(),
-        commit);
-    hooks.doPatchsetCreatedHook(change, ps, db);
+    gitRefUpdated.fire(change.getProject(), patchSet.getRefName(),
+        ObjectId.zeroId(), commit);
+    hooks.doPatchsetCreatedHook(change, patchSet, db);
   }
 }

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

@@ -14,9 +14,7 @@
 
 package com.google.gerrit.server.change;
 
-import com.google.gerrit.common.data.LabelTypes;
 import com.google.gerrit.common.errors.EmailException;
-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;
@@ -60,7 +58,6 @@ import org.eclipse.jgit.util.ChangeIdUtil;
 
 import java.io.IOException;
 import java.sql.Timestamp;
-import java.util.Collections;
 import java.util.List;
 
 public class CherryPickChange {
@@ -73,7 +70,7 @@ public class CherryPickChange {
   private final PersonIdent myIdent;
   private final IdentifiedUser currentUser;
   private final CommitValidators.Factory commitValidatorsFactory;
-  private final ChangeInserter changeInserter;
+  private final ChangeInserter.Factory changeInserterFactory;
   private final PatchSetInserter.Factory patchSetInserterFactory;
   final MergeUtil.Factory mergeUtilFactory;
 
@@ -82,7 +79,7 @@ public class CherryPickChange {
       final ReviewDb db, @GerritPersonIdent final PersonIdent myIdent,
       final GitRepositoryManager gitManager, final IdentifiedUser currentUser,
       final CommitValidators.Factory commitValidatorsFactory,
-      final ChangeInserter changeInserter,
+      final ChangeInserter.Factory changeInserterFactory,
       final PatchSetInserter.Factory patchSetInserterFactory,
       final MergeUtil.Factory mergeUtilFactory) {
     this.patchSetInfoFactory = patchSetInfoFactory;
@@ -91,7 +88,7 @@ public class CherryPickChange {
     this.myIdent = myIdent;
     this.currentUser = currentUser;
     this.commitValidatorsFactory = commitValidatorsFactory;
-    this.changeInserter = changeInserter;
+    this.changeInserterFactory = changeInserterFactory;
     this.patchSetInserterFactory = patchSetInserterFactory;
     this.mergeUtilFactory = mergeUtilFactory;
   }
@@ -247,18 +244,16 @@ public class CherryPickChange {
           change.getDest().getParentKey().get(), ru.getResult()));
     }
 
-    LabelTypes labelTypes =
-        refControl.getProjectControl().getLabelTypes();
-
     PatchSetInfo newPatchSetInfo =
         patchSetInfoFactory.get(cherryPickCommit, newPatchSet.getId());
     change.setCurrentPatchSet(newPatchSetInfo);
     ChangeUtil.updated(change);
 
-    changeInserter.insertChange(db, change,
-        buildChangeMessage(patchSetId, change), newPatchSet,
-        cherryPickCommit, labelTypes, newPatchSetInfo,
-        Collections.<Account.Id> emptySet());
+    changeInserterFactory
+        .create(refControl, change, newPatchSet, cherryPickCommit,
+            newPatchSetInfo)
+        .setMessage(buildChangeMessage(patchSetId, change))
+        .insert();
 
     return change.getId();
   }

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

@@ -93,6 +93,7 @@ public class Module extends RestApiModule {
         factory(ReviewerResource.Factory.class);
         factory(AccountInfo.Loader.Factory.class);
         factory(EmailReviewComments.Factory.class);
+        factory(ChangeInserter.Factory.class);
         factory(PatchSetInserter.Factory.class);
       }
     });

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

@@ -49,7 +49,7 @@ public class Revert implements RestModifyView<ChangeResource, Input> {
   private final GitRepositoryManager gitManager;
   private final PersonIdent myIdent;
   private final PatchSetInfoFactory patchSetInfoFactory;
-  private final ChangeInserter changeInserter;
+  private final ChangeInserter.Factory changeInserterFactory;
 
   public static class Input {
     public String message;
@@ -64,7 +64,7 @@ public class Revert implements RestModifyView<ChangeResource, Input> {
       GitRepositoryManager gitManager,
       final PatchSetInfoFactory patchSetInfoFactory,
       @GerritPersonIdent final PersonIdent myIdent,
-      final ChangeInserter changeInserter) {
+      final ChangeInserter.Factory changeInserterFactory) {
     this.hooks = hooks;
     this.revertedSenderFactory = revertedSenderFactory;
     this.commitValidatorsFactory = commitValidatorsFactory;
@@ -72,7 +72,7 @@ public class Revert implements RestModifyView<ChangeResource, Input> {
     this.json = json;
     this.gitManager = gitManager;
     this.myIdent = myIdent;
-    this.changeInserter = changeInserter;
+    this.changeInserterFactory = changeInserterFactory;
     this.patchSetInfoFactory = patchSetInfoFactory;
   }
 
@@ -97,7 +97,7 @@ public class Revert implements RestModifyView<ChangeResource, Input> {
               commitValidators,
               Strings.emptyToNull(input.message), dbProvider.get(),
               revertedSenderFactory, hooks, git, patchSetInfoFactory,
-              myIdent, changeInserter);
+              myIdent, changeInserterFactory);
 
       return json.format(revertedChangeId);
     } catch (InvalidChangeOperationException e) {

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java

@@ -65,7 +65,6 @@ import com.google.gerrit.server.auth.UniversalAuthBackend;
 import com.google.gerrit.server.auth.ldap.LdapModule;
 import com.google.gerrit.server.avatar.AvatarProvider;
 import com.google.gerrit.server.cache.CacheRemovalListener;
-import com.google.gerrit.server.change.ChangeInserter;
 import com.google.gerrit.server.events.EventFactory;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.git.ChangeCache;
@@ -222,7 +221,6 @@ public class GerritGlobalModule extends FactoryModule {
     bind(TransferConfig.class);
 
     bind(ApprovalsUtil.class);
-    bind(ChangeInserter.class);
     bind(ChangeMergeQueue.class).in(SINGLETON);
     bind(MergeQueue.class).to(ChangeMergeQueue.class).in(SINGLETON);
     factory(ReloadSubmitQueueOp.Factory.class);

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

@@ -259,7 +259,7 @@ public class ReceiveCommits {
   private final CommitValidators.Factory commitValidatorsFactory;
   private final TrackingFooters trackingFooters;
   private final TagCache tagCache;
-  private final ChangeInserter changeInserter;
+  private final ChangeInserter.Factory changeInserterFactory;
   private final WorkQueue workQueue;
   private final ListeningExecutorService changeUpdateExector;
   private final RequestScopePropagator requestScopePropagator;
@@ -315,7 +315,7 @@ public class ReceiveCommits {
       final GitRepositoryManager repoManager,
       final TagCache tagCache,
       final ChangeCache changeCache,
-      final ChangeInserter changeInserter,
+      final ChangeInserter.Factory changeInserterFactory,
       final CommitValidators.Factory commitValidatorsFactory,
       @CanonicalWebUrl @Nullable final String canonicalWebUrl,
       @GerritPersonIdent final PersonIdent gerritIdent,
@@ -348,7 +348,7 @@ public class ReceiveCommits {
     this.canonicalWebUrl = canonicalWebUrl;
     this.trackingFooters = trackingFooters;
     this.tagCache = tagCache;
-    this.changeInserter = changeInserter;
+    this.changeInserterFactory = changeInserterFactory;
     this.commitValidatorsFactory = commitValidatorsFactory;
     this.workQueue = workQueue;
     this.changeUpdateExector = changeUpdateExector;
@@ -1342,7 +1342,7 @@ public class ReceiveCommits {
         Change.Key changeKey = new Change.Key("I" + c.name());
         final List<String> idList = c.getFooterLines(CHANGE_ID);
         if (idList.isEmpty()) {
-          newChanges.add(new CreateRequest(c, changeKey));
+          newChanges.add(new CreateRequest(magicBranch.ctl, c, changeKey));
           continue;
         }
 
@@ -1392,7 +1392,7 @@ public class ReceiveCommits {
 
           newChangeIds.add(p.changeKey);
         }
-        newChanges.add(new CreateRequest(p.commit, p.changeKey));
+        newChanges.add(new CreateRequest(magicBranch.ctl, p.commit, p.changeKey));
       }
     } catch (IOException e) {
       // Should never happen, the core receive process would have
@@ -1460,10 +1460,13 @@ public class ReceiveCommits {
     final Change change;
     final PatchSet ps;
     final ReceiveCommand cmd;
+    private final RefControl ctl;
     private final PatchSetInfo info;
     boolean created;
 
-    CreateRequest(RevCommit c, Change.Key changeKey) throws OrmException {
+    CreateRequest(RefControl ctl, RevCommit c, Change.Key changeKey)
+        throws OrmException {
+      this.ctl = ctl;
       commit = c;
 
       change = new Change(changeKey,
@@ -1525,8 +1528,9 @@ public class ReceiveCommits {
       recipients.add(getRecipientsFromFooters(accountResolver, ps, footerLines));
       recipients.remove(me);
 
-      changeInserter.insertChange(db, change, ps, commit, labelTypes, info,
-          recipients.getReviewers());
+      changeInserterFactory.create(ctl, change, ps, commit, info)
+          .setReviewers(recipients.getReviewers())
+          .insert();
 
       created = true;