Remove ChangeControl from ChangeUpdate and migrate callers

Change-Id: Id189e2ed5049a96adc15302aa475cf9e0289e3e5

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/NoteDbPrimaryIT.java

@@ -63,7 +63,6 @@ import com.google.gerrit.server.notedb.NoteDbChangeState;
 import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
 import com.google.gerrit.server.notedb.PrimaryStorageMigrator;
 import com.google.gerrit.server.notedb.TestChangeRebuilderWrapper;
-import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.update.RetryHelper;
 import com.google.gerrit.testutil.ConfigSuite;
 import com.google.gerrit.testutil.NoteDbMode;
@@ -101,7 +100,7 @@ public class NoteDbPrimaryIT extends AbstractDaemonTest {
   @Inject private ChangeBundleReader bundleReader;
   @Inject private CommentsUtil commentsUtil;
   @Inject private TestChangeRebuilderWrapper rebuilderWrapper;
-  @Inject private ChangeControl.GenericFactory changeControlFactory;
+  @Inject private ChangeNotes.Factory changeNotesFactory;
   @Inject private ChangeUpdate.Factory updateFactory;
   @Inject private InternalUser.Factory internalUserFactory;
   @Inject private RetryHelper retryHelper;
@@ -125,7 +124,7 @@ public class NoteDbPrimaryIT extends AbstractDaemonTest {
         allUsers,
         rebuilderWrapper,
         ensureRebuiltRetryer,
-        changeControlFactory,
+        changeNotesFactory,
         queryProvider,
         updateFactory,
         internalUserFactory,

gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java

@@ -26,7 +26,6 @@ import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.InternalUser;
-import com.google.gerrit.server.project.ChangeControl;
 import com.google.gwtorm.server.OrmException;
 import java.io.IOException;
 import java.sql.Timestamp;
@@ -62,7 +61,8 @@ public abstract class AbstractChangeUpdate {
   protected AbstractChangeUpdate(
       Config cfg,
       NotesMigration migration,
-      ChangeControl ctl,
+      ChangeNotes notes,
+      CurrentUser user,
       PersonIdent serverIdent,
       String anonymousCowardName,
       ChangeNoteUtil noteUtil,
@@ -71,12 +71,12 @@ public abstract class AbstractChangeUpdate {
     this.noteUtil = noteUtil;
     this.serverIdent = new PersonIdent(serverIdent, when);
     this.anonymousCowardName = anonymousCowardName;
-    this.notes = ctl.getNotes();
+    this.notes = notes;
     this.change = notes.getChange();
-    this.accountId = accountId(ctl.getUser());
+    this.accountId = accountId(user);
-    Account.Id realAccountId = accountId(ctl.getUser().getRealUser());
+    Account.Id realAccountId = accountId(user.getRealUser());
     this.realAccountId = realAccountId != null ? realAccountId : accountId;
-    this.authorIdent = ident(noteUtil, serverIdent, anonymousCowardName, ctl.getUser(), when);
+    this.authorIdent = ident(noteUtil, serverIdent, anonymousCowardName, user, when);
     this.when = when;
     this.readOnlySkewMs = NoteDbChangeState.getReadOnlySkew(cfg);
   }

gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java

@@ -59,12 +59,12 @@ import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.client.RobotComment;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.config.AnonymousCowardName;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.mail.Address;
-import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.util.LabelVote;
 import com.google.gerrit.server.util.RequestId;
@@ -108,9 +108,9 @@ import org.eclipse.jgit.revwalk.RevWalk;
  */
 public class ChangeUpdate extends AbstractChangeUpdate {
   public interface Factory {
-    ChangeUpdate create(ChangeControl ctl);
+    ChangeUpdate create(ChangeNotes notes, CurrentUser user);
 
-    ChangeUpdate create(ChangeControl ctl, Date when);
+    ChangeUpdate create(ChangeNotes notes, CurrentUser user, Date when);
 
     ChangeUpdate create(
         Change change,
@@ -121,7 +121,8 @@ public class ChangeUpdate extends AbstractChangeUpdate {
         Comparator<String> labelNameComparator);
 
     @VisibleForTesting
-    ChangeUpdate create(ChangeControl ctl, Date when, Comparator<String> labelNameComparator);
+    ChangeUpdate create(
+        ChangeNotes notes, CurrentUser user, Date when, Comparator<String> labelNameComparator);
   }
 
   private final AccountCache accountCache;
@@ -175,7 +176,8 @@ public class ChangeUpdate extends AbstractChangeUpdate {
       RobotCommentUpdate.Factory robotCommentUpdateFactory,
       DeleteCommentRewriter.Factory deleteCommentRewriterFactory,
       ProjectCache projectCache,
-      @Assisted ChangeControl ctl,
+      @Assisted ChangeNotes notes,
+      @Assisted CurrentUser user,
       ChangeNoteUtil noteUtil) {
     this(
         cfg,
@@ -188,7 +190,8 @@ public class ChangeUpdate extends AbstractChangeUpdate {
         robotCommentUpdateFactory,
         deleteCommentRewriterFactory,
         projectCache,
-        ctl,
+        notes,
+        user,
         serverIdent.getWhen(),
         noteUtil);
   }
@@ -205,7 +208,8 @@ public class ChangeUpdate extends AbstractChangeUpdate {
       RobotCommentUpdate.Factory robotCommentUpdateFactory,
       DeleteCommentRewriter.Factory deleteCommentRewriterFactory,
       ProjectCache projectCache,
-      @Assisted ChangeControl ctl,
+      @Assisted ChangeNotes notes,
+      @Assisted CurrentUser user,
       @Assisted Date when,
       ChangeNoteUtil noteUtil) {
     this(
@@ -218,16 +222,13 @@ public class ChangeUpdate extends AbstractChangeUpdate {
         draftUpdateFactory,
         robotCommentUpdateFactory,
         deleteCommentRewriterFactory,
-        ctl,
+        notes,
+        user,
         when,
-        projectCache.get(getProjectName(ctl)).getLabelTypes().nameComparator(),
+        projectCache.get(notes.getProjectName()).getLabelTypes().nameComparator(),
         noteUtil);
   }
 
-  private static Project.NameKey getProjectName(ChangeControl ctl) {
-    return ctl.getProject().getNameKey();
-  }
-
   private static Table<String, Account.Id, Optional<Short>> approvals(
       Comparator<String> nameComparator) {
     return TreeBasedTable.create(nameComparator, comparing(IntKey::get));
@@ -244,11 +245,12 @@ public class ChangeUpdate extends AbstractChangeUpdate {
       ChangeDraftUpdate.Factory draftUpdateFactory,
       RobotCommentUpdate.Factory robotCommentUpdateFactory,
       DeleteCommentRewriter.Factory deleteCommentRewriterFactory,
-      @Assisted ChangeControl ctl,
+      @Assisted ChangeNotes notes,
+      @Assisted CurrentUser user,
       @Assisted Date when,
       @Assisted Comparator<String> labelNameComparator,
       ChangeNoteUtil noteUtil) {
-    super(cfg, migration, ctl, serverIdent, anonymousCowardName, noteUtil, when);
+    super(cfg, migration, notes, user, serverIdent, anonymousCowardName, noteUtil, when);
     this.accountCache = accountCache;
     this.updateManagerFactory = updateManagerFactory;
     this.draftUpdateFactory = draftUpdateFactory;

gerrit-server/src/main/java/com/google/gerrit/server/notedb/PrimaryStorageMigrator.java

@@ -46,7 +46,6 @@ import com.google.gerrit.server.index.change.ChangeField;
 import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
 import com.google.gerrit.server.notedb.NoteDbChangeState.RefState;
 import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder;
-import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gerrit.server.update.BatchUpdate;
@@ -83,7 +82,7 @@ public class PrimaryStorageMigrator {
   private static final Logger log = LoggerFactory.getLogger(PrimaryStorageMigrator.class);
 
   private final AllUsersName allUsers;
-  private final ChangeControl.GenericFactory changeControlFactory;
+  private final ChangeNotes.Factory changeNotesFactory;
   private final ChangeRebuilder rebuilder;
   private final ChangeUpdate.Factory updateFactory;
   private final GitRepositoryManager repoManager;
@@ -103,7 +102,7 @@ public class PrimaryStorageMigrator {
       GitRepositoryManager repoManager,
       AllUsersName allUsers,
       ChangeRebuilder rebuilder,
-      ChangeControl.GenericFactory changeControlFactory,
+      ChangeNotes.Factory changeNotesFactory,
       Provider<InternalChangeQuery> queryProvider,
       ChangeUpdate.Factory updateFactory,
       InternalUser.Factory internalUserFactory,
@@ -115,7 +114,7 @@ public class PrimaryStorageMigrator {
         allUsers,
         rebuilder,
         null,
-        changeControlFactory,
+        changeNotesFactory,
         queryProvider,
         updateFactory,
         internalUserFactory,
@@ -130,7 +129,7 @@ public class PrimaryStorageMigrator {
       AllUsersName allUsers,
       ChangeRebuilder rebuilder,
       @Nullable Retryer<NoteDbChangeState> testEnsureRebuiltRetryer,
-      ChangeControl.GenericFactory changeControlFactory,
+      ChangeNotes.Factory changeNotesFactory,
       Provider<InternalChangeQuery> queryProvider,
       ChangeUpdate.Factory updateFactory,
       InternalUser.Factory internalUserFactory,
@@ -140,7 +139,7 @@ public class PrimaryStorageMigrator {
     this.allUsers = allUsers;
     this.rebuilder = rebuilder;
     this.testEnsureRebuiltRetryer = testEnsureRebuiltRetryer;
-    this.changeControlFactory = changeControlFactory;
+    this.changeNotesFactory = changeNotesFactory;
     this.queryProvider = queryProvider;
     this.updateFactory = updateFactory;
     this.internalUserFactory = internalUserFactory;
@@ -410,7 +409,7 @@ public class PrimaryStorageMigrator {
     Timestamp until = new Timestamp(now.getTime() + timeoutMs);
     ChangeUpdate update =
         updateFactory.create(
-            changeControlFactory.controlFor(db.get(), project, id, internalUserFactory.create()));
+            changeNotesFactory.createChecked(db.get(), project, id), internalUserFactory.create());
     update.setReadOnlyUntil(until);
     return update.commit();
   }

gerrit-server/src/main/java/com/google/gerrit/server/update/NoteDbBatchUpdate.java

@@ -35,7 +35,6 @@ import com.google.gerrit.server.index.change.ChangeIndexer;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.ChangeUpdate;
 import com.google.gerrit.server.notedb.NoteDbUpdateManager;
-import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.util.RequestId;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -215,13 +214,13 @@ class NoteDbBatchUpdate extends BatchUpdate {
   }
 
   private class ChangeContextImpl extends ContextImpl implements ChangeContext {
-    private final ChangeControl ctl;
+    private final ChangeNotes notes;
     private final Map<PatchSet.Id, ChangeUpdate> updates;
 
     private boolean deleted;
 
-    protected ChangeContextImpl(ChangeControl ctl) {
+    protected ChangeContextImpl(ChangeNotes notes) {
-      this.ctl = checkNotNull(ctl);
+      this.notes = checkNotNull(notes);
       updates = new TreeMap<>(comparing(PatchSet.Id::get));
     }
 
@@ -229,8 +228,8 @@ class NoteDbBatchUpdate extends BatchUpdate {
     public ChangeUpdate getUpdate(PatchSet.Id psId) {
       ChangeUpdate u = updates.get(psId);
       if (u == null) {
-        u = changeUpdateFactory.create(ctl, when);
+        u = changeUpdateFactory.create(notes, user, when);
-        if (newChanges.containsKey(ctl.getId())) {
+        if (newChanges.containsKey(notes.getChangeId())) {
           u.setAllowWriteToNewRef(true);
         }
         u.setPatchSetId(psId);
@@ -241,7 +240,7 @@ class NoteDbBatchUpdate extends BatchUpdate {
 
     @Override
     public ChangeNotes getNotes() {
-      return ctl.getNotes();
+      return notes;
     }
 
     @Override
@@ -264,7 +263,6 @@ class NoteDbBatchUpdate extends BatchUpdate {
   }
 
   private final ChangeNotes.Factory changeNotesFactory;
-  private final ChangeControl.GenericFactory changeControlFactory;
   private final ChangeUpdate.Factory changeUpdateFactory;
   private final NoteDbUpdateManager.Factory updateManagerFactory;
   private final ChangeIndexer indexer;
@@ -276,7 +274,6 @@ class NoteDbBatchUpdate extends BatchUpdate {
       GitRepositoryManager repoManager,
       @GerritPersonIdent PersonIdent serverIdent,
       ChangeNotes.Factory changeNotesFactory,
-      ChangeControl.GenericFactory changeControlFactory,
       ChangeUpdate.Factory changeUpdateFactory,
       NoteDbUpdateManager.Factory updateManagerFactory,
       ChangeIndexer indexer,
@@ -287,7 +284,6 @@ class NoteDbBatchUpdate extends BatchUpdate {
       @Assisted Timestamp when) {
     super(repoManager, serverIdent, project, user, when);
     this.changeNotesFactory = changeNotesFactory;
-    this.changeControlFactory = changeControlFactory;
     this.changeUpdateFactory = changeUpdateFactory;
     this.updateManagerFactory = updateManagerFactory;
     this.indexer = indexer;
@@ -445,8 +441,7 @@ class NoteDbBatchUpdate extends BatchUpdate {
       logDebug("Change {} is new", id);
     }
     ChangeNotes notes = changeNotesFactory.createForBatchUpdate(c, !isNew);
-    ChangeControl ctl = changeControlFactory.controlFor(notes, user);
+    return new ChangeContextImpl(notes);
-    return new ChangeContextImpl(ctl);
   }
 
   private void executePostOps() throws Exception {

gerrit-server/src/main/java/com/google/gerrit/server/update/ReviewDbBatchUpdate.java

@@ -56,7 +56,6 @@ import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
 import com.google.gerrit.server.notedb.NoteDbUpdateManager;
 import com.google.gerrit.server.notedb.NoteDbUpdateManager.MismatchedStateException;
 import com.google.gerrit.server.notedb.NotesMigration;
-import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.util.RequestId;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.SchemaFactory;
@@ -165,7 +164,7 @@ class ReviewDbBatchUpdate extends BatchUpdate {
   }
 
   private class ChangeContextImpl extends ContextImpl implements ChangeContext {
-    private final ChangeControl ctl;
+    private final ChangeNotes notes;
     private final Map<PatchSet.Id, ChangeUpdate> updates;
     private final ReviewDbWrapper dbWrapper;
     private final Repository threadLocalRepo;
@@ -175,8 +174,8 @@ class ReviewDbBatchUpdate extends BatchUpdate {
     private boolean bumpLastUpdatedOn = true;
 
     protected ChangeContextImpl(
-        ChangeControl ctl, ReviewDbWrapper dbWrapper, Repository repo, RevWalk rw) {
+        ChangeNotes notes, ReviewDbWrapper dbWrapper, Repository repo, RevWalk rw) {
-      this.ctl = ctl;
+      this.notes = checkNotNull(notes);
       this.dbWrapper = dbWrapper;
       this.threadLocalRepo = repo;
       this.threadLocalRevWalk = rw;
@@ -198,8 +197,8 @@ class ReviewDbBatchUpdate extends BatchUpdate {
     public ChangeUpdate getUpdate(PatchSet.Id psId) {
       ChangeUpdate u = updates.get(psId);
       if (u == null) {
-        u = changeUpdateFactory.create(ctl, when);
+        u = changeUpdateFactory.create(notes, user, when);
-        if (newChanges.containsKey(ctl.getId())) {
+        if (newChanges.containsKey(notes.getChangeId())) {
           u.setAllowWriteToNewRef(true);
         }
         u.setPatchSetId(psId);
@@ -210,8 +209,7 @@ class ReviewDbBatchUpdate extends BatchUpdate {
 
     @Override
     public ChangeNotes getNotes() {
-      checkNotNull(ctl);
+      return notes;
-      return ctl.getNotes();
     }
 
     @Override
@@ -308,7 +306,6 @@ class ReviewDbBatchUpdate extends BatchUpdate {
   }
 
   private final AllUsersName allUsers;
-  private final ChangeControl.GenericFactory changeControlFactory;
   private final ChangeIndexer indexer;
   private final ChangeNotes.Factory changeNotesFactory;
   private final ChangeUpdate.Factory changeUpdateFactory;
@@ -329,7 +326,6 @@ class ReviewDbBatchUpdate extends BatchUpdate {
   ReviewDbBatchUpdate(
       @GerritServerConfig Config cfg,
       AllUsersName allUsers,
-      ChangeControl.GenericFactory changeControlFactory,
       ChangeIndexer indexer,
       ChangeNotes.Factory changeNotesFactory,
       @ChangeUpdateExecutor ListeningExecutorService changeUpdateExector,
@@ -347,7 +343,6 @@ class ReviewDbBatchUpdate extends BatchUpdate {
       @Assisted Timestamp when) {
     super(repoManager, serverIdent, project, user, when);
     this.allUsers = allUsers;
-    this.changeControlFactory = changeControlFactory;
     this.changeNotesFactory = changeNotesFactory;
     this.changeUpdateExector = changeUpdateExector;
     this.changeUpdateFactory = changeUpdateFactory;
@@ -786,8 +781,7 @@ class ReviewDbBatchUpdate extends BatchUpdate {
         NoteDbChangeState.checkNotReadOnly(c, skewMs);
       }
       ChangeNotes notes = changeNotesFactory.createForBatchUpdate(c, !isNew);
-      ChangeControl ctl = changeControlFactory.controlFor(notes, user);
+      return new ChangeContextImpl(notes, new BatchUpdateReviewDb(db), repo, rw);
-      return new ChangeContextImpl(ctl, new BatchUpdateReviewDb(db), repo, rw);
     }
 
     private NoteDbUpdateManager stageNoteDbUpdate(ChangeContextImpl ctx, boolean deleted)

gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java

@@ -15,7 +15,6 @@
 package com.google.gerrit.testutil;
 
 import static com.google.common.base.MoreObjects.firstNonNull;
-import static org.easymock.EasyMock.expect;
 
 import com.google.common.collect.Ordering;
 import com.google.gerrit.common.TimeUtil;
@@ -33,12 +32,9 @@ import com.google.gerrit.server.notedb.AbstractChangeNotes;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.ChangeUpdate;
 import com.google.gerrit.server.notedb.NotesMigration;
-import com.google.gerrit.server.project.ChangeControl;
-import com.google.gwtorm.server.OrmException;
 import com.google.inject.Injector;
 import java.util.TimeZone;
 import java.util.concurrent.atomic.AtomicInteger;
-import org.easymock.EasyMock;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.PersonIdent;
@@ -95,7 +91,8 @@ public class TestChanges {
         injector
             .getInstance(ChangeUpdate.Factory.class)
             .create(
-                stubChangeControl(injector.getInstance(AbstractChangeNotes.Args.class), c, user),
+                new ChangeNotes(injector.getInstance(AbstractChangeNotes.Args.class), c).load(),
+                user,
                 TimeUtil.nowTs(),
                 Ordering.<String>natural());
 
@@ -129,19 +126,6 @@ public class TestChanges {
     }
   }
 
-  private static ChangeControl stubChangeControl(
-      AbstractChangeNotes.Args args, Change c, CurrentUser user) throws OrmException {
-    ChangeControl ctl = EasyMock.createMock(ChangeControl.class);
-    expect(ctl.getChange()).andStubReturn(c);
-    expect(ctl.getProject()).andStubReturn(new Project(c.getProject()));
-    expect(ctl.getUser()).andStubReturn(user);
-    ChangeNotes notes = new ChangeNotes(args, c).load();
-    expect(ctl.getNotes()).andStubReturn(notes);
-    expect(ctl.getId()).andStubReturn(c.getId());
-    EasyMock.replay(ctl);
-    return ctl;
-  }
-
   public static void incrementPatchSet(Change change) {
     PatchSet.Id curr = change.currentPatchSetId();
     PatchSetInfo ps =