Move methods for scanning change refs into ChangeNotes.Factory

All remaining code that loads changes in batches from the database
should go into ChangeNotes.Factory.

Change-Id: Ib1d8c09f78e3f6d898378e91b501fa4d1ec154d5
Signed-off-by: Edwin Kempin <ekempin@google.com>

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

@@ -19,36 +19,25 @@ import static com.google.gerrit.server.git.SearchingChangeCacheImpl.ID_CACHE;
 import com.google.common.base.Function;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.RefNames;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.cache.CacheModule;
 import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.util.ManualRequestContext;
 import com.google.gerrit.server.util.OneOffRequestContext;
-import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Module;
 import com.google.inject.Singleton;
 import com.google.inject.TypeLiteral;
 import com.google.inject.name.Named;
 
-import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Collections;
-import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Map;
-import java.util.Set;
 import java.util.concurrent.ExecutionException;
 
 @Singleton
@@ -90,17 +79,14 @@ public class ScanningChangeCacheImpl implements ChangeCache {
 
   static class Loader extends CacheLoader<Project.NameKey, List<Change>> {
     private final GitRepositoryManager repoManager;
-    private final NotesMigration notesMigration;
     private final ChangeNotes.Factory notesFactory;
     private final OneOffRequestContext requestContext;
 
     @Inject
     Loader(GitRepositoryManager repoManager,
-        NotesMigration notesMigration,
         ChangeNotes.Factory notesFactory,
         OneOffRequestContext requestContext) {
       this.repoManager = repoManager;
-      this.notesMigration = notesMigration;
       this.notesFactory = notesFactory;
       this.requestContext = requestContext;
     }
@@ -109,66 +95,15 @@ public class ScanningChangeCacheImpl implements ChangeCache {
     public List<Change> load(Project.NameKey key) throws Exception {
       try (Repository repo = repoManager.openRepository(key);
           ManualRequestContext ctx = requestContext.open()) {
-        return scan(notesMigration, notesFactory, repo,
+        return Lists.transform(
-            ctx.getReviewDbProvider().get(), key);
+            notesFactory.scan(repo, ctx.getReviewDbProvider().get(), key),
+            new Function<ChangeNotes, Change>() {
+              @Override
+              public Change apply(ChangeNotes notes) {
+                return notes.getChange();
+              }
+            });
       }
     }
   }
-
-  public static List<Change> scan(NotesMigration notesMigration,
-      ChangeNotes.Factory notesFactory, Repository repo, ReviewDb db,
-      Project.NameKey project) throws OrmException, IOException {
-    if (!notesMigration.readChanges()) {
-      return scanDb(repo, db);
-    }
-
-    return scanChangesFromNotedb(notesFactory, repo, db, project);
-  }
-
-  public static List<Change> scanDb(Repository repo, ReviewDb db)
-      throws OrmException, IOException {
-    Map<String, Ref> refs =
-        repo.getRefDatabase().getRefs(RefNames.REFS_CHANGES);
-    Set<Change.Id> ids = new LinkedHashSet<>();
-    for (Ref r : refs.values()) {
-      Change.Id id = Change.Id.fromRef(r.getName());
-      if (id != null) {
-        ids.add(id);
-      }
-    }
-    List<Change> changes = new ArrayList<>(ids.size());
-    // A batch size of N may overload get(Iterable), so use something smaller,
-    // but still >1.
-    for (List<Change.Id> batch : Iterables.partition(ids, 30)) {
-      Iterables.addAll(changes, db.changes().get(batch));
-    }
-    return changes;
-  }
-
-  private static List<Change> scanChangesFromNotedb(
-      ChangeNotes.Factory notesFactory, Repository repo, ReviewDb db,
-      Project.NameKey project) throws OrmException, IOException {
-    List<ChangeNotes> changeNotes = scanNotedb(notesFactory, repo, db, project);
-    return Lists.transform(changeNotes, new Function<ChangeNotes, Change>() {
-      @Override
-      public Change apply(ChangeNotes notes) {
-        return notes.getChange();
-      }
-    });
-  }
-
-  public static List<ChangeNotes> scanNotedb(ChangeNotes.Factory notesFactory,
-      Repository repo, ReviewDb db, Project.NameKey project)
-          throws OrmException, IOException {
-    Map<String, Ref> refs =
-        repo.getRefDatabase().getRefs(RefNames.REFS_CHANGES);
-    List<ChangeNotes> changeNotes = new ArrayList<>(refs.size());
-    for (Ref r : refs.values()) {
-      Change.Id id = Change.Id.fromRef(r.getName());
-      if (id != null) {
-        changeNotes.add(notesFactory.create(db, project, id));
-      }
-    }
-    return changeNotes;
-  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java

@@ -37,9 +37,7 @@ import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.MergeUtil;
 import com.google.gerrit.server.git.MultiProgressMonitor;
 import com.google.gerrit.server.git.MultiProgressMonitor.Task;
-import com.google.gerrit.server.git.ScanningChangeCacheImpl;
 import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.patch.PatchListLoader;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gwtorm.server.SchemaFactory;
@@ -118,7 +116,6 @@ public class SiteIndexer {
   private final GitRepositoryManager repoManager;
   private final ListeningExecutorService executor;
   private final ChangeIndexer.Factory indexerFactory;
-  private final NotesMigration notesMigration;
   private final ChangeNotes.Factory notesFactory;
   private final ThreeWayMergeStrategy mergeStrategy;
 
@@ -133,7 +130,6 @@ public class SiteIndexer {
       GitRepositoryManager repoManager,
       @IndexExecutor(BATCH) ListeningExecutorService executor,
       ChangeIndexer.Factory indexerFactory,
-      NotesMigration notesMigration,
       ChangeNotes.Factory notesFactory,
       @GerritServerConfig Config config) {
     this.schemaFactory = schemaFactory;
@@ -141,7 +137,6 @@ public class SiteIndexer {
     this.repoManager = repoManager;
     this.executor = executor;
     this.indexerFactory = indexerFactory;
-    this.notesMigration = notesMigration;
     this.notesFactory = notesFactory;
     this.mergeStrategy = MergeUtil.getMergeStrategy(config);
   }
@@ -257,8 +252,8 @@ public class SiteIndexer {
         try (Repository repo = repoManager.openRepository(project);
             ReviewDb db = schemaFactory.open()) {
           Map<String, Ref> refs = repo.getRefDatabase().getRefs(ALL);
-          for (Change c : ScanningChangeCacheImpl.scan(notesMigration,
+          for (ChangeNotes cn : notesFactory.scan(repo, db, project)) {
-              notesFactory, repo, db, project)) {
+            Change c = cn.getChange();
             Ref r = refs.get(c.currentPatchSetId().toRefName());
             if (r != null) {
               byId.put(r.getObjectId(), changeDataFactory.create(db, c));

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

@@ -31,6 +31,7 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSetMultimap;
 import com.google.common.collect.ImmutableSortedMap;
 import com.google.common.collect.ImmutableSortedSet;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.Ordering;
 import com.google.common.primitives.Ints;
@@ -43,12 +44,12 @@ import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.reviewdb.server.ReviewDbUtil;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.ScanningChangeCacheImpl;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.query.change.ChangeData;
@@ -62,6 +63,7 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.CommitBuilder;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.notes.NoteMap;
 import org.eclipse.jgit.revwalk.RevWalk;
@@ -70,8 +72,11 @@ import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.sql.Timestamp;
+import java.util.ArrayList;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /** View of a single {@link Change} based on the log of its notes branch. */
 public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
@@ -217,7 +222,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
      * Instantiate ChangeNotes for a change that has been loaded by a batch read
      * from the database.
      */
-    public ChangeNotes createFromChangeOnlyWhenNotedbDisabled(Change change)
+    private ChangeNotes createFromChangeOnlyWhenNotedbDisabled(Change change)
         throws OrmException {
       checkState(!migration.readChanges(), "do not call"
           + " createFromChangeWhenNotedbDisabled when notedb is enabled");
@@ -231,8 +236,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
       if (migration.readChanges()) {
         for (Project.NameKey project : projectCache.all()) {
           try (Repository repo = repoManager.openRepository(project)) {
-            List<ChangeNotes> changes =
+            List<ChangeNotes> changes = scanNotedb(repo, db, project);
-                ScanningChangeCacheImpl.scanNotedb(this, repo, db, project);
             for (ChangeNotes cn : changes) {
               if (predicate.apply(cn)) {
                 m.put(project, cn);
@@ -250,6 +254,51 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
       }
       return ImmutableListMultimap.copyOf(m);
     }
+
+    public List<ChangeNotes> scan(Repository repo, ReviewDb db,
+        Project.NameKey project) throws OrmException, IOException {
+      if (!migration.readChanges()) {
+        return scanDb(repo, db);
+      }
+
+      return scanNotedb(repo, db, project);
+    }
+
+    private List<ChangeNotes> scanDb(Repository repo, ReviewDb db)
+        throws OrmException, IOException {
+      Map<String, Ref> refs =
+          repo.getRefDatabase().getRefs(RefNames.REFS_CHANGES);
+      Set<Change.Id> ids = new LinkedHashSet<>();
+      for (Ref r : refs.values()) {
+        Change.Id id = Change.Id.fromRef(r.getName());
+        if (id != null) {
+          ids.add(id);
+        }
+      }
+      List<ChangeNotes> notes = new ArrayList<>(ids.size());
+      // A batch size of N may overload get(Iterable), so use something smaller,
+      // but still >1.
+      for (List<Change.Id> batch : Iterables.partition(ids, 30)) {
+        for (Change change : db.changes().get(batch)) {
+          notes.add(createFromChangeOnlyWhenNotedbDisabled(change));
+        }
+      }
+      return notes;
+    }
+
+    private List<ChangeNotes> scanNotedb(Repository repo, ReviewDb db,
+        Project.NameKey project) throws OrmException, IOException {
+      Map<String, Ref> refs =
+          repo.getRefDatabase().getRefs(RefNames.REFS_CHANGES);
+      List<ChangeNotes> changeNotes = new ArrayList<>(refs.size());
+      for (Ref r : refs.values()) {
+        Change.Id id = Change.Id.fromRef(r.getName());
+        if (id != null) {
+          changeNotes.add(create(db, project, id));
+        }
+      }
+      return changeNotes;
+    }
   }
 
   private final Project.NameKey project;