Extract an abstract base class for NotesMigration

We want to be able to provide a different implementation for testing
that does not require restarting the server in order to pick up
changes. This will allow us to test rebuilding on a running server,
which fits better with the way existing acceptance tests are set up.

Change-Id: Ie0e30d86db9f951995110a09696decb0fc885381

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -58,6 +58,7 @@ import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.index.ChangeIndexer;
 import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ConfigNotesMigration;
 import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.Util;
@@ -254,7 +255,7 @@ public abstract class AbstractDaemonTest {
       GerritServer.Description.forTestMethod(description, configName);
 
     if (isNoteDbTestEnabled()) {
-      NotesMigration.setAllEnabledConfig(baseConfig);
+      ConfigNotesMigration.setAllEnabledConfig(baseConfig);
     }
     baseConfig.setString("gerrit", null, "tempSiteDir",
         tempSiteDir.getRoot().getPath());

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java

@@ -27,7 +27,7 @@ import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.config.TrackingFooters;
 import com.google.gerrit.server.config.TrackingFootersProvider;
 import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.notedb.NotesMigration;
+import com.google.gerrit.server.notedb.ConfigNotesMigration;
 import com.google.gerrit.server.schema.DataSourceType;
 import com.google.gerrit.server.schema.NotesMigrationSchemaFactory;
 import com.google.gerrit.server.schema.ReviewDbFactory;
@@ -76,7 +76,7 @@ class InMemoryTestingDatabaseModule extends LifecycleModule {
     bind(MetricMaker.class).to(DisabledMetricMaker.class);
     bind(DataSourceType.class).to(InMemoryH2Type.class);
 
-    bind(NotesMigration.class);
+    install(new ConfigNotesMigration.Module());
     TypeLiteral<SchemaFactory<ReviewDb>> schemaFactory =
         new TypeLiteral<SchemaFactory<ReviewDb>>() {};
     bind(schemaFactory).to(NotesMigrationSchemaFactory.class);

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/RebuildNotedbIT.java

@@ -19,7 +19,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.io.Files;
 import com.google.gerrit.launcher.GerritLauncher;
-import com.google.gerrit.server.notedb.NotesMigration;
+import com.google.gerrit.server.notedb.ConfigNotesMigration;
 import com.google.gerrit.testutil.TempFileUtil;
 
 import org.junit.After;
@@ -46,7 +46,7 @@ public class RebuildNotedbIT {
   @Test
   public void rebuildEmptySite() throws Exception {
     initSite();
-    Files.append(NotesMigration.allEnabledConfig().toText(),
+    Files.append(ConfigNotesMigration.allEnabledConfig().toText(),
         new File(sitePath.toString(), "etc/gerrit.config"),
         UTF_8);
     runGerrit("RebuildNotedb", "-d", sitePath.toString(),

gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteProgram.java

@@ -29,6 +29,7 @@ import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.GerritServerConfigModule;
 import com.google.gerrit.server.config.SitePath;
 import com.google.gerrit.server.git.LocalDiskRepositoryManager;
+import com.google.gerrit.server.notedb.ConfigNotesMigration;
 import com.google.gerrit.server.schema.DataSourceModule;
 import com.google.gerrit.server.schema.DataSourceProvider;
 import com.google.gerrit.server.schema.DataSourceType;
@@ -170,6 +171,7 @@ public abstract class SiteProgram extends AbstractProgram {
     modules.add(new DatabaseModule());
     modules.add(new SchemaModule());
     modules.add(new LocalDiskRepositoryManager.Module());
+    modules.add(new ConfigNotesMigration.Module());
 
     try {
       return Guice.createInjector(PRODUCTION, modules);

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

@@ -37,9 +37,9 @@ 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.git.ScanningChangeCacheImpl;
 import com.google.gerrit.server.patch.PatchListLoader;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gwtorm.server.SchemaFactory;

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

@@ -0,0 +1,116 @@
+// Copyright (C) 2013 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.notedb;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+import org.eclipse.jgit.lib.Config;
+
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * Implement NoteDb migration stages using {@code gerrit.config}.
+ * <p>
+ * This class controls the state of the migration according to options in
+ * {@code gerrit.config}. In general, any changes to these options should only
+ * be made by adventurous administrators, who know what they're doing, on
+ * non-production data, for the purposes of testing the NoteDb implementation.
+ * Changing options quite likely requires re-running {@code RebuildNoteDb}. For
+ * these reasons, the options remain undocumented.
+ */
+@Singleton
+public class ConfigNotesMigration extends NotesMigration {
+  public static class Module extends AbstractModule {
+    @Override
+    public void configure() {
+      bind(NotesMigration.class).to(ConfigNotesMigration.class);
+    }
+  }
+
+  private static enum Table {
+    CHANGES;
+
+    private String key() {
+      return name().toLowerCase();
+    }
+  }
+
+  private static final String NOTEDB = "notedb";
+  private static final String READ = "read";
+  private static final String WRITE = "write";
+
+  private static void checkConfig(Config cfg) {
+    Set<String> keys = new HashSet<>();
+    for (Table t : Table.values()) {
+      keys.add(t.key());
+    }
+    for (String t : cfg.getSubsections(NOTEDB)) {
+      checkArgument(keys.contains(t.toLowerCase()),
+          "invalid notedb table: %s", t);
+      for (String key : cfg.getNames(NOTEDB, t)) {
+        String lk = key.toLowerCase();
+        checkArgument(lk.equals(WRITE) || lk.equals(READ),
+            "invalid notedb key: %s.%s", t, key);
+      }
+      boolean write = cfg.getBoolean(NOTEDB, t, WRITE, false);
+      boolean read = cfg.getBoolean(NOTEDB, t, READ, false);
+      checkArgument(!(read && !write),
+          "must have write enabled when read enabled: %s", t);
+    }
+  }
+
+  public static ConfigNotesMigration allEnabled() {
+    return new ConfigNotesMigration(allEnabledConfig());
+  }
+
+  public static Config allEnabledConfig() {
+    Config cfg = new Config();
+    setAllEnabledConfig(cfg);
+    return cfg;
+  }
+
+  public static void setAllEnabledConfig(Config cfg) {
+    for (Table t : Table.values()) {
+      cfg.setBoolean(NOTEDB, t.key(), WRITE, true);
+      cfg.setBoolean(NOTEDB, t.key(), READ, true);
+    }
+  }
+
+  private final boolean writeChanges;
+  private final boolean readChanges;
+
+  @Inject
+  ConfigNotesMigration(@GerritServerConfig Config cfg) {
+    checkConfig(cfg);
+    writeChanges = cfg.getBoolean(NOTEDB, Table.CHANGES.key(), WRITE, false);
+    readChanges = cfg.getBoolean(NOTEDB, Table.CHANGES.key(), READ, false);
+  }
+
+  @Override
+  public boolean writeChanges() {
+    return writeChanges;
+  }
+
+  @Override
+  public boolean readChanges() {
+    return readChanges;
+  }
+}

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

@@ -1,4 +1,4 @@
-// Copyright (C) 2013 The Android Open Source Project
+// Copyright (C) 2016 The Android Open Source Project
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -14,17 +14,6 @@
 
 package com.google.gerrit.server.notedb;
 
-import static com.google.common.base.Preconditions.checkArgument;
-
-import com.google.gerrit.server.config.GerritServerConfig;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-
-import org.eclipse.jgit.lib.Config;
-
-import java.util.HashSet;
-import java.util.Set;
-
 /**
  * Holds the current state of the NoteDb migration.
  * <p>
@@ -44,77 +33,13 @@ import java.util.Set;
  * Changing options quite likely requires re-running {@code RebuildNoteDb}. For
  * these reasons, the options remain undocumented.
  */
-@Singleton
-public class NotesMigration {
-  private static enum Table {
-    CHANGES;
+public abstract class NotesMigration {
+  public abstract boolean readChanges();
 
-    private String key() {
-      return name().toLowerCase();
-    }
-  }
-
-  private static final String NOTEDB = "notedb";
-  private static final String READ = "read";
-  private static final String WRITE = "write";
-
-  private static void checkConfig(Config cfg) {
-    Set<String> keys = new HashSet<>();
-    for (Table t : Table.values()) {
-      keys.add(t.key());
-    }
-    for (String t : cfg.getSubsections(NOTEDB)) {
-      checkArgument(keys.contains(t.toLowerCase()),
-          "invalid notedb table: %s", t);
-      for (String key : cfg.getNames(NOTEDB, t)) {
-        String lk = key.toLowerCase();
-        checkArgument(lk.equals(WRITE) || lk.equals(READ),
-            "invalid notedb key: %s.%s", t, key);
-      }
-      boolean write = cfg.getBoolean(NOTEDB, t, WRITE, false);
-      boolean read = cfg.getBoolean(NOTEDB, t, READ, false);
-      checkArgument(!(read && !write),
-          "must have write enabled when read enabled: %s", t);
-    }
-  }
-
-  public static NotesMigration allEnabled() {
-    return new NotesMigration(allEnabledConfig());
-  }
-
-  public static Config allEnabledConfig() {
-    Config cfg = new Config();
-    setAllEnabledConfig(cfg);
-    return cfg;
-  }
-
-  public static void setAllEnabledConfig(Config cfg) {
-    for (Table t : Table.values()) {
-      cfg.setBoolean(NOTEDB, t.key(), WRITE, true);
-      cfg.setBoolean(NOTEDB, t.key(), READ, true);
-    }
-  }
-
-  private final boolean writeChanges;
-  private final boolean readChanges;
-
-  @Inject
-  NotesMigration(@GerritServerConfig Config cfg) {
-    checkConfig(cfg);
-    writeChanges = cfg.getBoolean(NOTEDB, Table.CHANGES.key(), WRITE, false);
-    readChanges = cfg.getBoolean(NOTEDB, Table.CHANGES.key(), READ, false);
-  }
+  public abstract boolean writeChanges();
 
   public boolean enabled() {
     return writeChanges()
         || readChanges();
   }
-
-  public boolean writeChanges() {
-    return writeChanges;
-  }
-
-  public boolean readChanges() {
-    return readChanges;
-  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/schema/DatabaseModule.java

@@ -18,7 +18,6 @@ import static com.google.inject.Scopes.SINGLETON;
 
 import com.google.gerrit.extensions.config.FactoryModule;
 import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gwtorm.jdbc.Database;
 import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Key;
@@ -33,7 +32,6 @@ public class DatabaseModule extends FactoryModule {
     TypeLiteral<Database<ReviewDb>> database =
         new TypeLiteral<Database<ReviewDb>>() {};
 
-    bind(NotesMigration.class);
     bind(schemaFactory).to(NotesMigrationSchemaFactory.class);
     bind(Key.get(schemaFactory, ReviewDbFactory.class))
         .to(database)

gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java

@@ -77,7 +77,7 @@ public class AbstractChangeNotesTest extends GerritBaseTests {
   private static final TimeZone TZ =
       TimeZone.getTimeZone("America/Los_Angeles");
 
-  private static final NotesMigration MIGRATION = NotesMigration.allEnabled();
+  private static final NotesMigration MIGRATION = ConfigNotesMigration.allEnabled();
 
   protected Account.Id otherUserId;
   protected FakeAccountCache accountCache;

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

@@ -15,7 +15,7 @@
 package com.google.gerrit.testutil;
 
 import com.google.common.collect.ImmutableList;
-import com.google.gerrit.server.notedb.NotesMigration;
+import com.google.gerrit.server.notedb.ConfigNotesMigration;
 
 import org.eclipse.jgit.lib.Config;
 import org.junit.Rule;
@@ -60,7 +60,7 @@ public class GerritServerTests extends GerritBaseTests {
 
   public void beforeTest() throws Exception {
     if (isNoteDbTestEnabled()) {
-      NotesMigration.setAllEnabledConfig(config);
+      ConfigNotesMigration.setAllEnabledConfig(config);
     }
   }
 

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

@@ -50,6 +50,8 @@ import com.google.gerrit.server.git.SendEmailExecutor;
 import com.google.gerrit.server.index.ChangeSchemas;
 import com.google.gerrit.server.index.IndexModule.IndexType;
 import com.google.gerrit.server.mail.SignedTokenEmailTokenVerifier;
+import com.google.gerrit.server.notedb.ConfigNotesMigration;
+import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.patch.DiffExecutor;
 import com.google.gerrit.server.schema.DataSourceType;
 import com.google.gerrit.server.schema.SchemaCreator;
@@ -159,6 +161,7 @@ public class InMemoryModule extends FactoryModule {
     bind(InMemoryRepositoryManager.class).in(SINGLETON);
     bind(TrackingFooters.class).toProvider(TrackingFootersProvider.class)
         .in(SINGLETON);
+    bind(NotesMigration.class).to(ConfigNotesMigration.class);
 
     bind(DataSourceType.class)
       .to(InMemoryH2Type.class);

gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java

@@ -51,6 +51,7 @@ import com.google.gerrit.server.index.IndexModule.IndexType;
 import com.google.gerrit.server.mail.SignedTokenEmailTokenVerifier;
 import com.google.gerrit.server.mail.SmtpEmailSender;
 import com.google.gerrit.server.mime.MimeUtil2Module;
+import com.google.gerrit.server.notedb.ConfigNotesMigration;
 import com.google.gerrit.server.patch.DiffExecutorModule;
 import com.google.gerrit.server.plugins.PluginGuiceEnvironment;
 import com.google.gerrit.server.plugins.PluginRestApiModule;
@@ -284,6 +285,7 @@ public class WebAppInitializer extends GuiceServletContextListener
     }
     modules.add(new SchemaModule());
     modules.add(new LocalDiskRepositoryManager.Module());
+    modules.add(new ConfigNotesMigration.Module());
     modules.add(SchemaVersionCheck.module());
     modules.add(new AuthConfigModule());
     return dbInjector.createChildInjector(modules);