Merge "Move ChangeRebuilder#rebuildProject to RebuildNoteDb"

gerrit-pgm/src/main/java/com/google/gerrit/pgm/RebuildNoteDb.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.pgm;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.gerrit.reviewdb.server.ReviewDbUtil.unwrapDb;
 import static com.google.gerrit.server.schema.DataSourceProvider.Context.MULTI_USER;
 
@@ -28,6 +29,7 @@ import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.ListeningExecutorService;
 import com.google.common.util.concurrent.MoreExecutors;
+import com.google.gerrit.common.FormatUtil;
 import com.google.gerrit.extensions.config.FactoryModule;
 import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
 import com.google.gerrit.extensions.registration.DynamicSet;
@@ -42,12 +44,16 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.change.ChangeResource;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.git.ChainedReceiveCommands;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.WorkQueue;
 import com.google.gerrit.server.index.DummyIndexModule;
 import com.google.gerrit.server.index.change.ReindexAfterUpdate;
+import com.google.gerrit.server.notedb.ChangeBundleReader;
+import com.google.gerrit.server.notedb.NoteDbUpdateManager;
 import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder;
+import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder.NoPatchSetsException;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
@@ -57,9 +63,12 @@ import org.eclipse.jgit.lib.BatchRefUpdate;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.RefDatabase;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.TextProgressMonitor;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.transport.ReceiveCommand;
 import org.kohsuke.args4j.Option;
@@ -67,6 +76,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
+import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -106,6 +116,9 @@ public class RebuildNoteDb extends SiteProgram {
   @Inject
   private GitRepositoryManager repoManager;
 
+  @Inject
+  private NoteDbUpdateManager.Factory updateManagerFactory;
+
   @Inject
   private NotesMigration notesMigration;
 
@@ -115,6 +128,9 @@ public class RebuildNoteDb extends SiteProgram {
   @Inject
   private WorkQueue workQueue;
 
+  @Inject
+  private ChangeBundleReader bundleReader;
+
   @Override
   public int run() throws Exception {
     mustHaveValidSite();
@@ -153,7 +169,7 @@ public class RebuildNoteDb extends SiteProgram {
               @Override
               public Boolean call() {
                 try (ReviewDb db = unwrapDb(schemaFactory.open())) {
-                  return rebuilder.rebuildProject(
+                  return rebuildProject(
                       db, changesByProject, project, allUsersRepo);
                 } catch (Exception e) {
                   log.error("Error rebuilding project " + project, e);
@@ -257,4 +273,37 @@ public class RebuildNoteDb extends SiteProgram {
       return ImmutableMultimap.copyOf(changesByProject);
     }
   }
+
+  private boolean rebuildProject(ReviewDb db,
+      ImmutableMultimap<Project.NameKey, Change.Id> allChanges,
+      Project.NameKey project, Repository allUsersRepo)
+      throws IOException, OrmException {
+    checkArgument(allChanges.containsKey(project));
+    boolean ok = true;
+    ProgressMonitor pm = new TextProgressMonitor(new PrintWriter(System.out));
+    pm.beginTask(
+        FormatUtil.elide(project.get(), 50), allChanges.get(project).size());
+    try (NoteDbUpdateManager manager = updateManagerFactory.create(project);
+        ObjectInserter allUsersInserter = allUsersRepo.newObjectInserter();
+        RevWalk allUsersRw = new RevWalk(allUsersInserter.newReader())) {
+      manager.setAllUsersRepo(allUsersRepo, allUsersRw, allUsersInserter,
+          new ChainedReceiveCommands(allUsersRepo));
+      for (Change.Id changeId : allChanges.get(project)) {
+        try {
+          rebuilder.buildUpdates(
+              manager, bundleReader.fromReviewDb(db, changeId));
+        } catch (NoPatchSetsException e) {
+          log.warn(e.getMessage());
+        } catch (Throwable t) {
+          log.error("Failed to rebuild change " + changeId, t);
+          ok = false;
+        }
+        pm.update(1);
+      }
+      manager.execute();
+    } finally {
+      pm.endTask();
+    }
+    return ok;
+  }
 }

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

@@ -16,11 +16,8 @@ package com.google.gerrit.server.notedb;
 
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
-import com.google.common.collect.ImmutableMultimap;
 import com.google.gerrit.extensions.config.FactoryModule;
 import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.Change.Id;
-import com.google.gerrit.reviewdb.client.Project.NameKey;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.notedb.NoteDbUpdateManager.Result;
 import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder;
@@ -29,7 +26,6 @@ import com.google.inject.TypeLiteral;
 import com.google.inject.name.Names;
 
 import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.lib.Repository;
 
 public class NoteDbModule extends FactoryModule {
   private final Config cfg;
@@ -78,13 +74,6 @@ public class NoteDbModule extends FactoryModule {
           return null;
         }
 
-        @Override
-        public boolean rebuildProject(ReviewDb db,
-            ImmutableMultimap<NameKey, Id> allChanges, NameKey project,
-            Repository allUsersRepo) {
-          return false;
-        }
-
         @Override
         public NoteDbUpdateManager stage(ReviewDb db, Change.Id changeId) {
           return null;
@@ -95,6 +84,12 @@ public class NoteDbModule extends FactoryModule {
             NoteDbUpdateManager manager) {
           return null;
         }
+
+        @Override
+        public void buildUpdates(NoteDbUpdateManager manager,
+            ChangeBundle bundle) {
+          // Do nothing.
+        }
       });
       bind(new TypeLiteral<Cache<ChangeNotesCache.Key, ChangeNotesState>>() {})
           .annotatedWith(Names.named(ChangeNotesCache.CACHE_NAME))

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

@@ -15,9 +15,7 @@
 package com.google.gerrit.server.notedb;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableMultimap;
 import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.notedb.NoteDbUpdateManager.Result;
 import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder;
@@ -29,7 +27,6 @@ import com.google.inject.Inject;
 import com.google.inject.Singleton;
 
 import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Repository;
 
 import java.io.IOException;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -82,23 +79,6 @@ public class TestChangeRebuilderWrapper extends ChangeRebuilder {
     return delegate.rebuild(manager, bundle);
   }
 
-  @Override
-  public boolean rebuildProject(ReviewDb db,
-      ImmutableMultimap<Project.NameKey, Change.Id> allChanges,
-      Project.NameKey project, Repository allUsersRepo)
-      throws NoSuchChangeException, IOException, OrmException,
-      ConfigInvalidException {
-    if (failNextUpdate.getAndSet(false)) {
-      throw new IOException("Update failed");
-    }
-    boolean result =
-        delegate.rebuildProject(db, allChanges, project, allUsersRepo);
-    if (stealNextUpdate.getAndSet(false)) {
-      throw new IOException("Update stolen");
-    }
-    return result;
-  }
-
   @Override
   public NoteDbUpdateManager stage(ReviewDb db, Change.Id changeId)
       throws NoSuchChangeException, IOException, OrmException {
@@ -119,4 +99,11 @@ public class TestChangeRebuilderWrapper extends ChangeRebuilder {
     }
     return result;
   }
+
+  @Override
+  public void buildUpdates(NoteDbUpdateManager manager, ChangeBundle bundle)
+      throws IOException, OrmException {
+    // Don't check for manual failure; that happens in execute().
+    delegate.buildUpdates(manager, bundle);
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilder.java

@@ -14,11 +14,9 @@
 
 package com.google.gerrit.server.notedb.rebuild;
 
-import com.google.common.collect.ImmutableMultimap;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.ListeningExecutorService;
 import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.notedb.ChangeBundle;
 import com.google.gerrit.server.notedb.NoteDbUpdateManager;
@@ -28,7 +26,6 @@ import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.SchemaFactory;
 
 import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Repository;
 
 import java.io.IOException;
 import java.util.concurrent.Callable;
@@ -69,11 +66,8 @@ public abstract class ChangeRebuilder {
       ChangeBundle bundle) throws NoSuchChangeException, IOException,
       OrmException, ConfigInvalidException;
 
-  public abstract boolean rebuildProject(ReviewDb db,
+  public abstract void buildUpdates(NoteDbUpdateManager manager,
-      ImmutableMultimap<Project.NameKey, Change.Id> allChanges,
+      ChangeBundle bundle) throws IOException, OrmException;
-      Project.NameKey project, Repository allUsersRepo)
-      throws NoSuchChangeException, IOException, OrmException,
-      ConfigInvalidException;
 
   public abstract NoteDbUpdateManager stage(ReviewDb db, Change.Id changeId)
       throws NoSuchChangeException, IOException, OrmException;

gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java

@@ -15,7 +15,6 @@
 package com.google.gerrit.server.notedb.rebuild;
 
 import static com.google.common.base.MoreObjects.firstNonNull;
-import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
 import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_HASHTAGS;
@@ -26,7 +25,6 @@ import static java.util.stream.Collectors.toList;
 import com.google.common.base.Optional;
 import com.google.common.base.Splitter;
 import com.google.common.collect.ArrayListMultimap;
-import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Multimap;
@@ -34,7 +32,6 @@ import com.google.common.collect.Ordering;
 import com.google.common.collect.Sets;
 import com.google.common.collect.Table;
 import com.google.common.primitives.Ints;
-import com.google.gerrit.common.FormatUtil;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
@@ -44,7 +41,6 @@ import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
 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.server.ReviewDb;
 import com.google.gerrit.reviewdb.server.ReviewDbUtil;
@@ -75,12 +71,8 @@ import com.google.inject.Inject;
 
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.PersonIdent;
-import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.lib.TextProgressMonitor;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.transport.ReceiveCommand;
@@ -88,7 +80,6 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
-import java.io.PrintWriter;
 import java.sql.Timestamp;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -166,6 +157,8 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
       throws NoSuchChangeException, IOException, OrmException,
       ConfigInvalidException {
     db = ReviewDbUtil.unwrapDb(db);
+    // Read change just to get project; this instance is then discarded so we
+    // can read a consistent ChangeBundle inside a transaction.
     Change change = db.changes().get(changeId);
     if (change == null) {
       throw new NoSuchChangeException(changeId);
@@ -260,40 +253,7 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
   }
 
   @Override
-  public boolean rebuildProject(ReviewDb db,
+  public void buildUpdates(NoteDbUpdateManager manager, ChangeBundle bundle)
-      ImmutableMultimap<Project.NameKey, Change.Id> allChanges,
-      Project.NameKey project, Repository allUsersRepo)
-      throws NoSuchChangeException, IOException, OrmException,
-      ConfigInvalidException {
-    checkArgument(allChanges.containsKey(project));
-    boolean ok = true;
-    ProgressMonitor pm = new TextProgressMonitor(new PrintWriter(System.out));
-    pm.beginTask(
-        FormatUtil.elide(project.get(), 50), allChanges.get(project).size());
-    try (NoteDbUpdateManager manager = updateManagerFactory.create(project);
-        ObjectInserter allUsersInserter = allUsersRepo.newObjectInserter();
-        RevWalk allUsersRw = new RevWalk(allUsersInserter.newReader())) {
-      manager.setAllUsersRepo(allUsersRepo, allUsersRw, allUsersInserter,
-          new ChainedReceiveCommands(allUsersRepo));
-      for (Change.Id changeId : allChanges.get(project)) {
-        try {
-          buildUpdates(manager, bundleReader.fromReviewDb(db, changeId));
-        } catch (NoPatchSetsException e) {
-          log.warn(e.getMessage());
-        } catch (Throwable t) {
-          log.error("Failed to rebuild change " + changeId, t);
-          ok = false;
-        }
-        pm.update(1);
-      }
-      manager.execute();
-    } finally {
-      pm.endTask();
-    }
-    return ok;
-  }
-
-  private void buildUpdates(NoteDbUpdateManager manager, ChangeBundle bundle)
       throws IOException, OrmException {
     manager.setCheckExpectedState(false);
     Change change = new Change(bundle.getChange());