Merge "MergeOp: Use a Provider instead of a Factory"

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

@@ -128,7 +128,7 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
   private final ChangeUpdate.Factory updateFactory;
   private final ApprovalsUtil approvalsUtil;
   private final ChangeMessagesUtil cmUtil;
-  private final MergeOp.Factory mergeOpFactory;
+  private final Provider<MergeOp> mergeOpProvider;
   private final ChangeIndexer indexer;
   private final LabelNormalizer labelNormalizer;
   private final AccountsCollection accounts;
@@ -149,7 +149,7 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
       ChangeUpdate.Factory updateFactory,
       ApprovalsUtil approvalsUtil,
       ChangeMessagesUtil cmUtil,
-      MergeOp.Factory mergeOpFactory,
+      Provider<MergeOp> mergeOpProvider,
       AccountsCollection accounts,
       ChangesCollection changes,
       ChangeIndexer indexer,
@@ -164,7 +164,7 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
     this.updateFactory = updateFactory;
     this.approvalsUtil = approvalsUtil;
     this.cmUtil = cmUtil;
-    this.mergeOpFactory = mergeOpFactory;
+    this.mergeOpProvider = mergeOpProvider;
     this.accounts = accounts;
     this.changes = changes;
     this.indexer = indexer;
@@ -215,7 +215,7 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
     ChangeSet submittedChanges = ChangeSet.create(submit(rsrc, caller, false));
 
     try {
-      mergeOpFactory.create(submittedChanges, caller).merge(true);
+      mergeOpProvider.get().merge(submittedChanges, caller, true);
       change = dbProvider.get().changes().get(change.getId());
     } catch (NoSuchChangeException e) {
       throw new OrmException("Submission failed", e);

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

@@ -74,7 +74,6 @@ import com.google.gerrit.server.change.MergeabilityCacheImpl;
 import com.google.gerrit.server.events.EventFactory;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.git.GitModule;
-import com.google.gerrit.server.git.MergeOp;
 import com.google.gerrit.server.git.MergeUtil;
 import com.google.gerrit.server.git.NotesBranchUtil;
 import com.google.gerrit.server.git.ReceivePackInitializer;
@@ -233,7 +232,6 @@ public class GerritGlobalModule extends FactoryModule {
     bind(Boolean.class).annotatedWith(DisableReverseDnsLookup.class)
         .toProvider(DisableReverseDnsLookupProvider.class).in(SINGLETON);
 
-    factory(MergeOp.Factory.class);
     factory(SubmoduleOp.Factory.class);
     bind(PatchSetInfoFactory.class);
     bind(IdentifiedUser.GenericFactory.class).in(SINGLETON);

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

@@ -82,7 +82,6 @@ import com.google.inject.Injector;
 import com.google.inject.OutOfScopeException;
 import com.google.inject.Provider;
 import com.google.inject.Provides;
-import com.google.inject.assistedinject.Assisted;
 import com.google.inject.servlet.RequestScoped;
 
 import com.jcraft.jsch.HostKey;
@@ -131,10 +130,6 @@ import java.util.concurrent.Callable;
  * be merged cleanly.
  */
 public class MergeOp {
-  public interface Factory {
-    MergeOp create(ChangeSet changes, IdentifiedUser caller);
-  }
-
   private static final Logger log = LoggerFactory.getLogger(MergeOp.class);
 
   private final AccountCache accountCache;
@@ -164,9 +159,7 @@ public class MergeOp {
   private final Map<Change.Id, CodeReviewCommit> commits;
   private final List<Change> toUpdate;
   private final PerThreadRequestScope.Scoper threadScoper;
-  private final ChangeSet changes;
+  private String logPrefix;
-  private final IdentifiedUser caller;
-  private final String logPrefix;
 
   private ProjectState destProject;
   private ReviewDb db;
@@ -203,9 +196,7 @@ public class MergeOp {
       SubmitStrategyFactory submitStrategyFactory,
       SubmoduleOp.Factory subOpFactory,
       TagCache tagCache,
-      WorkQueue workQueue,
+      WorkQueue workQueue) {
-      @Assisted ChangeSet changes,
-      @Assisted IdentifiedUser caller) {
     this.accountCache = accountCache;
     this.approvalsUtil = approvalsUtil;
     this.changeControlFactory = changeControlFactory;
@@ -229,11 +220,9 @@ public class MergeOp {
     this.subOpFactory = subOpFactory;
     this.tagCache = tagCache;
     this.workQueue = workQueue;
-    this.changes = changes;
-    this.caller = caller;
     commits = new HashMap<>();
     toUpdate = Lists.newArrayList();
-    logPrefix = String.format("[%s]: ", String.valueOf(changes.hashCode()));
+
 
     pendingRefUpdates = new HashMap<>();
     openBranches = new HashMap<>();
@@ -405,8 +394,9 @@ public class MergeOp {
 
   // For historic reasons we will first go into the submitted state
   // TODO(sbeller): remove this when we get rid of Change.Status.SUBMITTED
-  private void submitAllChanges(ChangeSet cs, boolean force)
+  private void submitAllChanges(ChangeSet cs, IdentifiedUser caller,
-      throws OrmException, ResourceConflictException, IOException {
+      boolean force) throws OrmException, ResourceConflictException,
+      IOException {
     for (Change.Id id : cs.ids()) {
       ChangeData cd = changeDataFactory.create(db, id);
       switch (cd.change().getStatus()) {
@@ -430,8 +420,10 @@ public class MergeOp {
     }
   }
 
-  public void merge(boolean checkPermissions) throws NoSuchChangeException,
+  public void merge(ChangeSet changes, IdentifiedUser caller,
+      boolean checkPermissions) throws NoSuchChangeException,
       OrmException, ResourceConflictException {
+    logPrefix = String.format("[%s]: ", String.valueOf(changes.hashCode()));
     logDebug("Beginning merge of {}", changes);
     try {
       openSchema();
@@ -441,12 +433,12 @@ public class MergeOp {
       if (checkPermissions) {
         logDebug("Submitting all calculated changes while "
             + "enforcing submit rules");
-        submitAllChanges(cs, false);
+        submitAllChanges(cs, caller, false);
         logDebug("Checking permissions");
         checkPermissions(cs);
       } else {
         logDebug("Submitting all calculated changes ignoring submit rules");
-        submitAllChanges(cs, true);
+        submitAllChanges(cs, caller, true);
       }
       try {
         integrateIntoHistory(cs);
@@ -466,7 +458,7 @@ public class MergeOp {
 
   private void integrateIntoHistory(ChangeSet cs)
       throws MergeException, NoSuchChangeException, ResourceConflictException {
-    logDebug("Beginning merge attempt on {}", changes);
+    logDebug("Beginning merge attempt on {}", cs);
     Map<Branch.NameKey, ListMultimap<SubmitType, Change>> toSubmit =
         new HashMap<>();
     try {

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

@@ -327,7 +327,7 @@ public class ReceiveCommits {
 
   private final SubmoduleOp.Factory subOpFactory;
   private final Provider<Submit> submitProvider;
-  private final MergeOp.Factory mergeFactory;
+  private final Provider<MergeOp> mergeOpProvider;
   private final DynamicMap<ProjectConfigEntry> pluginConfigEntries;
   private final NotesMigration notesMigration;
   private final ChangeEditUtil editUtil;
@@ -377,7 +377,7 @@ public class ReceiveCommits {
       @Assisted final Repository repo,
       final SubmoduleOp.Factory subOpFactory,
       final Provider<Submit> submitProvider,
-      final MergeOp.Factory mergeFactory,
+      final Provider<MergeOp> mergeOpProvider,
       final ChangeKindCache changeKindCache,
       final DynamicMap<ProjectConfigEntry> pluginConfigEntries,
       final NotesMigration notesMigration,
@@ -424,7 +424,7 @@ public class ReceiveCommits {
 
     this.subOpFactory = subOpFactory;
     this.submitProvider = submitProvider;
-    this.mergeFactory = mergeFactory;
+    this.mergeOpProvider = mergeOpProvider;
     this.pluginConfigEntries = pluginConfigEntries;
     this.notesMigration = notesMigration;
 
@@ -1780,8 +1780,8 @@ public class ReceiveCommits {
       throw new IOException(e);
     }
     try {
-      mergeFactory.create(ChangeSet.create(changes),
+      mergeOpProvider.get().merge(ChangeSet.create(changes),
-          (IdentifiedUser) changeCtl.getCurrentUser()).merge(false);
+          (IdentifiedUser) changeCtl.getCurrentUser(), false);
     } catch (NoSuchChangeException e) {
       throw new OrmException(e);
     }