Replace configurable NotesMigration with a stub implementation

Remove MutableNotesMigration, NotesMigration.Snapshot,
NotesMigrationState, and NoteDbMode. The only way to run tests is with
NoteDb enabled. (Tests passing GERRIT_NOTEDB in the environment or
gerrit.notedb as a system property will still continue to work; the
property is just ignored.)

The method bodies of NotesMigration now simply return constant values.
The next step will be to eliminate usages of these methods.

Change-Id: I808a7cc8b3dc4bedb072f3737c2df9758edbdea5

java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -115,7 +115,7 @@ import com.google.gerrit.server.index.group.GroupIndexer;
 import com.google.gerrit.server.notedb.AbstractChangeNotes;
 import com.google.gerrit.server.notedb.ChangeNoteUtil;
 import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.notedb.MutableNotesMigration;
+import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectConfig;
 import com.google.gerrit.server.project.testing.Util;
@@ -127,7 +127,6 @@ import com.google.gerrit.testing.ConfigSuite;
 import com.google.gerrit.testing.FakeEmailSender;
 import com.google.gerrit.testing.FakeEmailSender.Message;
 import com.google.gerrit.testing.FakeGroupAuditService;
-import com.google.gerrit.testing.NoteDbMode;
 import com.google.gerrit.testing.SshMode;
 import com.google.gson.Gson;
 import com.google.gwtorm.server.OrmException;
@@ -262,7 +261,7 @@ public abstract class AbstractDaemonTest {
   @Inject protected PluginConfigFactory pluginConfig;
   @Inject protected Revisions revisions;
   @Inject protected SystemGroupBackend systemGroupBackend;
-  @Inject protected MutableNotesMigration notesMigration;
+  @Inject protected NotesMigration notesMigration;
   @Inject protected ChangeNotes.Factory notesFactory;
   @Inject protected BatchAbandon batchAbandon;
   @Inject protected TestSshKeys sshKeys;
@@ -598,7 +597,6 @@ public abstract class AbstractDaemonTest {
       server.close();
       server = null;
     }
-    NoteDbMode.resetFromEnv(notesMigration);
   }
 
   protected void closeSsh() {

java/com/google/gerrit/acceptance/GerritServer.java

@@ -45,7 +45,6 @@ import com.google.gerrit.server.util.SystemLog;
 import com.google.gerrit.testing.FakeEmailSender;
 import com.google.gerrit.testing.FakeGroupAuditService;
 import com.google.gerrit.testing.InMemoryRepositoryManager;
-import com.google.gerrit.testing.NoteDbMode;
 import com.google.gerrit.testing.SshMode;
 import com.google.inject.AbstractModule;
 import com.google.inject.Injector;
@@ -482,8 +481,6 @@ public class GerritServer implements AutoCloseable {
     cfg.setInt("receive", null, "threadPoolSize", 1);
     cfg.setInt("index", null, "threads", 1);
     cfg.setBoolean("index", null, "reindexAfterRefUpdate", false);
-
-    NoteDbMode.newNotesMigrationFromEnv().setConfigValues(cfg);
   }
 
   private static Injector createTestInjector(Daemon daemon) throws Exception {

java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java

@@ -28,7 +28,6 @@ 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.schema.NotesMigrationSchemaFactory;
 import com.google.gerrit.server.schema.ReviewDbFactory;
 import com.google.gerrit.server.schema.ReviewDbSchemaModule;
@@ -75,7 +74,6 @@ class InMemoryTestingDatabaseModule extends LifecycleModule {
 
     bind(MetricMaker.class).to(DisabledMetricMaker.class);
 
-    install(new NotesMigration.Module());
     TypeLiteral<SchemaFactory<ReviewDb>> schemaFactory =
         new TypeLiteral<SchemaFactory<ReviewDb>>() {};
     bind(schemaFactory).to(NotesMigrationSchemaFactory.class);

java/com/google/gerrit/httpd/init/WebAppInitializer.java

@@ -78,7 +78,6 @@ import com.google.gerrit.server.mail.SignedTokenEmailTokenVerifier;
 import com.google.gerrit.server.mail.receive.MailReceiver;
 import com.google.gerrit.server.mail.send.SmtpEmailSender;
 import com.google.gerrit.server.mime.MimeUtil2Module;
-import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.patch.DiffExecutorModule;
 import com.google.gerrit.server.permissions.DefaultPermissionBackendModule;
 import com.google.gerrit.server.plugins.PluginGuiceEnvironment;
@@ -277,7 +276,6 @@ public class WebAppInitializer extends GuiceServletContextListener implements Fi
       modules.add(new GerritServerConfigModule());
     }
     modules.add(new DatabaseModule());
-    modules.add(new NotesMigration.Module());
     modules.add(new DropWizardMetricMaker.ApiModule());
     return Guice.createInjector(PRODUCTION, modules);
   }

java/com/google/gerrit/pgm/init/InitNoteDb.java

@@ -19,10 +19,8 @@ import static com.google.gerrit.server.notedb.NotesMigration.SECTION_NOTE_DB;
 
 import com.google.gerrit.pgm.init.api.InitStep;
 import com.google.gerrit.pgm.init.api.Section;
-import com.google.gerrit.server.notedb.NotesMigrationState;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
-import org.eclipse.jgit.lib.Config;
 
 /** Initialize the NoteDb in gerrit site. */
 @Singleton
@@ -41,10 +39,10 @@ class InitNoteDb implements InitStep {
   }
 
   private void initNoteDb() {
-    Config defaultConfig = new Config();
+    noteDbChanges.set("write", "false");
-    NotesMigrationState.FINAL.setConfigValues(defaultConfig);
+    noteDbChanges.set("read", "false");
-    for (String name : defaultConfig.getNames(SECTION_NOTE_DB, CHANGES.key())) {
+    noteDbChanges.set("primaryStorage", "note db");
-      noteDbChanges.set(name, defaultConfig.getString(SECTION_NOTE_DB, CHANGES.key(), name));
+    noteDbChanges.set("disableReviewDb", "true");
-    }
+    noteDbChanges.set("sequence", "true");
   }
 }

java/com/google/gerrit/pgm/util/SiteProgram.java

@@ -25,7 +25,6 @@ import com.google.gerrit.server.config.GerritRuntime;
 import com.google.gerrit.server.config.GerritServerConfigModule;
 import com.google.gerrit.server.config.SitePath;
 import com.google.gerrit.server.git.GitRepositoryManagerModule;
-import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.schema.DatabaseModule;
 import com.google.gerrit.server.schema.ReviewDbSchemaModule;
 import com.google.gerrit.server.securestore.SecureStoreClassName;
@@ -123,7 +122,6 @@ public abstract class SiteProgram extends AbstractProgram {
     modules.add(new DatabaseModule());
     modules.add(new ReviewDbSchemaModule());
     modules.add(cfgInjector.getInstance(GitRepositoryManagerModule.class));
-    modules.add(new NotesMigration.Module());
 
     try {
       return Guice.createInjector(PRODUCTION, modules);

java/com/google/gerrit/server/notedb/MutableNotesMigration.java

@@ -1,95 +0,0 @@
-// Copyright (C) 2017 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 com.google.common.annotations.VisibleForTesting;
-import com.google.gerrit.server.config.GerritServerConfig;
-import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-import java.util.function.Function;
-import org.eclipse.jgit.lib.Config;
-
-/**
- * {@link NotesMigration} with additional methods for altering the migration state at runtime.
- *
- * <p>Almost all callers care only about inspecting the migration state, and for safety should not
- * have access to mutation methods, which must be used with extreme care. Those callers should
- * inject {@link NotesMigration}.
- *
- * <p>Some callers, namely the NoteDb migration pipeline and tests, do need to alter the migration
- * state at runtime, and those callers are expected to take the necessary precautions such as
- * keeping the in-memory and on-disk config state in sync. Those callers use this class.
- *
- * <p>Mutations to the {@link MutableNotesMigration} are guaranteed to be instantly visible to all
- * callers that use the non-mutable {@link NotesMigration}. The current implementation accomplishes
- * this by always binding {@link NotesMigration} to {@link MutableNotesMigration} in Guice, so there
- * is just one {@link NotesMigration} instance process-wide.
- */
-@Singleton
-public class MutableNotesMigration extends NotesMigration {
-  public static MutableNotesMigration newDisabled() {
-    return new MutableNotesMigration(new Config());
-  }
-
-  public static MutableNotesMigration fromConfig(Config cfg) {
-    return new MutableNotesMigration(cfg);
-  }
-
-  @Inject
-  MutableNotesMigration(@GerritServerConfig Config cfg) {
-    super(Snapshot.create(cfg));
-  }
-
-  public MutableNotesMigration setReadChanges(boolean readChanges) {
-    return set(b -> b.setReadChanges(readChanges));
-  }
-
-  public MutableNotesMigration setWriteChanges(boolean writeChanges) {
-    return set(b -> b.setWriteChanges(writeChanges));
-  }
-
-  public MutableNotesMigration setReadChangeSequence(boolean readChangeSequence) {
-    return set(b -> b.setReadChangeSequence(readChangeSequence));
-  }
-
-  public MutableNotesMigration setChangePrimaryStorage(PrimaryStorage changePrimaryStorage) {
-    return set(b -> b.setChangePrimaryStorage(changePrimaryStorage));
-  }
-
-  public MutableNotesMigration setDisableChangeReviewDb(boolean disableChangeReviewDb) {
-    return set(b -> b.setDisableChangeReviewDb(disableChangeReviewDb));
-  }
-
-  /**
-   * Set the in-memory values returned by this instance to match the given state.
-   *
-   * <p>This method is only intended for use by tests.
-   *
-   * <p>This <em>only</em> modifies the in-memory state; if this instance was initialized from a
-   * file-based config, the underlying storage is not updated. Callers are responsible for managing
-   * the underlying storage on their own.
-   */
-  @VisibleForTesting
-  public MutableNotesMigration setFrom(NotesMigrationState state) {
-    snapshot.set(state.snapshot());
-    return this;
-  }
-
-  private MutableNotesMigration set(Function<Snapshot.Builder, Snapshot.Builder> f) {
-    snapshot.updateAndGet(s -> f.apply(s.toBuilder()).build());
-    return this;
-  }
-}

java/com/google/gerrit/server/notedb/NotesMigration.java

@@ -14,44 +14,17 @@
 
 package com.google.gerrit.server.notedb;
 
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.gerrit.server.notedb.NoteDbTable.CHANGES;
-
-import com.google.auto.value.AutoValue;
 import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
-import com.google.inject.AbstractModule;
+import com.google.inject.Singleton;
-import java.util.concurrent.atomic.AtomicReference;
+import java.util.Objects;
-import org.eclipse.jgit.lib.Config;
 
 /**
  * Current low-level settings of the NoteDb migration for changes.
  *
- * <p>This class only describes the migration state of the {@link
+ * <p>This class is a stub and will be removed soon; NoteDb is the only mode.
- * com.google.gerrit.reviewdb.client.Change Change} entity group, since it is possible for a given
- * site to be in different states of the Change NoteDb migration process while staying at the same
- * ReviewDb schema version. It does <em>not</em> describe the migration state of non-Change tables;
- * those are automatically migrated using the ReviewDb schema migration process, so the NoteDb
- * migration state at a given ReviewDb schema cannot vary.
- *
- * <p>In many places, core Gerrit code should not directly care about the NoteDb migration state,
- * and should prefer high-level APIs like {@link com.google.gerrit.server.ApprovalsUtil
- * ApprovalsUtil} that don't require callers to inspect the migration state. The
- * <em>implementation</em> of those utilities does care about the state, and should query the {@code
- * NotesMigration} for the properties of the migration, for example, {@link #changePrimaryStorage()
- * where new changes should be stored}.
- *
- * <p>Core Gerrit code is mostly interested in one facet of the migration at a time (reading or
- * writing, say), but not all combinations of return values are supported or even make sense.
- *
- * <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.
- *
- * <p><strong>Note:</strong> Callers should not assume the values returned by {@code
- * NotesMigration}'s methods will not change in a running server.
  */
-public abstract class NotesMigration {
+@Singleton
+public class NotesMigration {
   public static final String SECTION_NOTE_DB = "noteDb";
   public static final String READ = "read";
   public static final String WRITE = "write";
@@ -59,80 +32,6 @@ public abstract class NotesMigration {
   public static final String PRIMARY_STORAGE = "primaryStorage";
   public static final String SEQUENCE = "sequence";
 
-  public static class Module extends AbstractModule {
-    @Override
-    public void configure() {
-      bind(MutableNotesMigration.class);
-      bind(NotesMigration.class).to(MutableNotesMigration.class);
-    }
-  }
-
-  @AutoValue
-  abstract static class Snapshot {
-    static Builder builder() {
-      // Default values are defined as what we would read from an empty config.
-      return create(new Config()).toBuilder();
-    }
-
-    static Snapshot create(Config cfg) {
-      return new AutoValue_NotesMigration_Snapshot.Builder()
-          .setWriteChanges(cfg.getBoolean(SECTION_NOTE_DB, CHANGES.key(), WRITE, false))
-          .setReadChanges(cfg.getBoolean(SECTION_NOTE_DB, CHANGES.key(), READ, false))
-          .setReadChangeSequence(cfg.getBoolean(SECTION_NOTE_DB, CHANGES.key(), SEQUENCE, false))
-          .setChangePrimaryStorage(
-              cfg.getEnum(
-                  SECTION_NOTE_DB, CHANGES.key(), PRIMARY_STORAGE, PrimaryStorage.REVIEW_DB))
-          .setDisableChangeReviewDb(
-              cfg.getBoolean(SECTION_NOTE_DB, CHANGES.key(), DISABLE_REVIEW_DB, false))
-          .build();
-    }
-
-    abstract boolean writeChanges();
-
-    abstract boolean readChanges();
-
-    abstract boolean readChangeSequence();
-
-    abstract PrimaryStorage changePrimaryStorage();
-
-    abstract boolean disableChangeReviewDb();
-
-    abstract Builder toBuilder();
-
-    void setConfigValues(Config cfg) {
-      cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), WRITE, writeChanges());
-      cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), READ, readChanges());
-      cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), SEQUENCE, readChangeSequence());
-      cfg.setEnum(SECTION_NOTE_DB, CHANGES.key(), PRIMARY_STORAGE, changePrimaryStorage());
-      cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), DISABLE_REVIEW_DB, disableChangeReviewDb());
-    }
-
-    @AutoValue.Builder
-    abstract static class Builder {
-      abstract Builder setWriteChanges(boolean writeChanges);
-
-      abstract Builder setReadChanges(boolean readChanges);
-
-      abstract Builder setReadChangeSequence(boolean readChangeSequence);
-
-      abstract Builder setChangePrimaryStorage(PrimaryStorage changePrimaryStorage);
-
-      abstract Builder setDisableChangeReviewDb(boolean disableChangeReviewDb);
-
-      abstract Snapshot autoBuild();
-
-      Snapshot build() {
-        Snapshot s = autoBuild();
-        checkArgument(
-            !(s.disableChangeReviewDb() && s.changePrimaryStorage() != PrimaryStorage.NOTE_DB),
-            "cannot disable ReviewDb for changes if default change primary storage is ReviewDb");
-        return s;
-      }
-    }
-  }
-
-  protected final AtomicReference<Snapshot> snapshot;
-
   /**
    * Read changes from NoteDb.
    *
@@ -144,7 +43,7 @@ public abstract class NotesMigration {
    * attempts to write will generate an error.
    */
   public final boolean readChanges() {
-    return snapshot.get().readChanges();
+    return true;
   }
 
   /**
@@ -162,7 +61,7 @@ public abstract class NotesMigration {
    * write will generate an error.
    */
   public final boolean rawWriteChangesSetting() {
-    return snapshot.get().writeChanges();
+    return true;
   }
 
   /**
@@ -172,12 +71,12 @@ public abstract class NotesMigration {
    * change IDs are read from ReviewDb's native sequences.
    */
   public final boolean readChangeSequence() {
-    return snapshot.get().readChangeSequence();
+    return true;
   }
 
   /** @return default primary storage for new changes. */
   public final PrimaryStorage changePrimaryStorage() {
-    return snapshot.get().changePrimaryStorage();
+    return PrimaryStorage.NOTE_DB;
   }
 
   /**
@@ -188,47 +87,26 @@ public abstract class NotesMigration {
    * Changes table.
    */
   public final boolean disableChangeReviewDb() {
-    return snapshot.get().disableChangeReviewDb();
+    return true;
   }
 
   public final boolean commitChangeWrites() {
-    // It may seem odd that readChanges() without writeChanges() means we should
+    return true;
-    // attempt to commit writes. However, this method is used by callers to know
-    // whether or not they should short-circuit and skip attempting to read or
-    // write NoteDb refs.
-    //
-    // It is possible for commitChangeWrites() to return true and
-    // failChangeWrites() to also return true, causing an error later in the
-    // same codepath. This specific condition is used by the auto-rebuilding
-    // path to rebuild a change and stage the results, but not commit them due
-    // to failChangeWrites().
-    return rawWriteChangesSetting() || readChanges();
   }
 
   public final boolean failChangeWrites() {
-    return !rawWriteChangesSetting() && readChanges();
+    return false;
-  }
-
-  public final void setConfigValues(Config cfg) {
-    snapshot.get().setConfigValues(cfg);
   }
 
   @Override
   public final boolean equals(Object o) {
-    return o instanceof NotesMigration
+    return o instanceof NotesMigration;
-        && snapshot.get().equals(((NotesMigration) o).snapshot.get());
   }
 
   @Override
   public final int hashCode() {
-    return snapshot.get().hashCode();
+    return Objects.hash();
   }
 
-  protected NotesMigration(Snapshot snapshot) {
+  public NotesMigration() {}
-    this.snapshot = new AtomicReference<>(snapshot);
-  }
-
-  final Snapshot snapshot() {
-    return snapshot.get();
-  }
 }

java/com/google/gerrit/server/notedb/NotesMigrationState.java

@@ -1,92 +0,0 @@
-// Copyright (C) 2017 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 com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
-import com.google.gerrit.server.notedb.NotesMigration.Snapshot;
-import java.util.Optional;
-import java.util.stream.Stream;
-import org.eclipse.jgit.lib.Config;
-
-/**
- * Possible high-level states of the NoteDb migration for changes.
- *
- * <p>This class describes the series of states required to migrate a site from ReviewDb-only to
- * NoteDb-only. This process has several steps, and covers only a small subset of the theoretically
- * possible combinations of {@link NotesMigration} return values.
- *
- * <p>These states are ordered: a one-way migration from ReviewDb to NoteDb will pass through states
- * in the order in which they are defined.
- */
-public enum NotesMigrationState {
-  REVIEW_DB(false, false, false, PrimaryStorage.REVIEW_DB, false),
-
-  WRITE(false, true, false, PrimaryStorage.REVIEW_DB, false),
-
-  READ_WRITE_NO_SEQUENCE(true, true, false, PrimaryStorage.REVIEW_DB, false),
-
-  READ_WRITE_WITH_SEQUENCE_REVIEW_DB_PRIMARY(true, true, true, PrimaryStorage.REVIEW_DB, false),
-
-  READ_WRITE_WITH_SEQUENCE_NOTE_DB_PRIMARY(true, true, true, PrimaryStorage.NOTE_DB, false),
-
-  NOTE_DB(true, true, true, PrimaryStorage.NOTE_DB, true);
-
-  public static final NotesMigrationState FINAL = NOTE_DB;
-
-  public static Optional<NotesMigrationState> forConfig(Config cfg) {
-    return forSnapshot(Snapshot.create(cfg));
-  }
-
-  public static Optional<NotesMigrationState> forNotesMigration(NotesMigration migration) {
-    return forSnapshot(migration.snapshot());
-  }
-
-  private static Optional<NotesMigrationState> forSnapshot(Snapshot s) {
-    return Stream.of(values()).filter(v -> v.snapshot.equals(s)).findFirst();
-  }
-
-  private final Snapshot snapshot;
-
-  NotesMigrationState(
-      // Arguments match abstract methods in NotesMigration.
-      boolean readChanges,
-      boolean rawWriteChangesSetting,
-      boolean readChangeSequence,
-      PrimaryStorage changePrimaryStorage,
-      boolean disableChangeReviewDb) {
-    this.snapshot =
-        Snapshot.builder()
-            .setReadChanges(readChanges)
-            .setWriteChanges(rawWriteChangesSetting)
-            .setReadChangeSequence(readChangeSequence)
-            .setChangePrimaryStorage(changePrimaryStorage)
-            .setDisableChangeReviewDb(disableChangeReviewDb)
-            .build();
-  }
-
-  public void setConfigValues(Config cfg) {
-    snapshot.setConfigValues(cfg);
-  }
-
-  public String toText() {
-    Config cfg = new Config();
-    setConfigValues(cfg);
-    return cfg.toText();
-  }
-
-  Snapshot snapshot() {
-    return snapshot;
-  }
-}

java/com/google/gerrit/testing/GerritServerTests.java

@@ -14,7 +14,7 @@
 
 package com.google.gerrit.testing;
 
-import com.google.gerrit.server.notedb.MutableNotesMigration;
+import com.google.gerrit.server.notedb.NotesMigration;
 import org.eclipse.jgit.lib.Config;
 import org.junit.Rule;
 import org.junit.rules.TestRule;
@@ -28,7 +28,7 @@ public class GerritServerTests extends GerritBaseTests {
 
   @ConfigSuite.Name private String configName;
 
-  protected MutableNotesMigration notesMigration;
+  protected NotesMigration notesMigration;
 
   @Rule
   public TestRule testRunner =
@@ -39,21 +39,13 @@ public class GerritServerTests extends GerritBaseTests {
             @Override
             public void evaluate() throws Throwable {
               beforeTest();
-              try {
               base.evaluate();
-              } finally {
-                afterTest();
-              }
             }
           };
         }
       };
 
-  public void beforeTest() throws Exception {
+  public void beforeTest() {
-    notesMigration = NoteDbMode.newNotesMigrationFromEnv();
+    notesMigration = new NotesMigration();
-  }
-
-  public void afterTest() {
-    NoteDbMode.resetFromEnv(notesMigration);
   }
 }

java/com/google/gerrit/testing/InMemoryModule.java

@@ -73,8 +73,6 @@ import com.google.gerrit.server.index.group.AllGroupsIndexer;
 import com.google.gerrit.server.index.group.GroupIndexCollection;
 import com.google.gerrit.server.index.group.GroupSchemaDefinitions;
 import com.google.gerrit.server.mail.SignedTokenEmailTokenVerifier;
-import com.google.gerrit.server.notedb.MutableNotesMigration;
-import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.patch.DiffExecutor;
 import com.google.gerrit.server.permissions.DefaultPermissionBackendModule;
 import com.google.gerrit.server.plugins.ServerInformationImpl;
@@ -135,15 +133,13 @@ public class InMemoryModule extends FactoryModule {
   }
 
   private final Config cfg;
-  private final MutableNotesMigration notesMigration;
 
   public InMemoryModule() {
-    this(newDefaultConfig(), NoteDbMode.newNotesMigrationFromEnv());
+    this(newDefaultConfig());
   }
 
-  public InMemoryModule(Config cfg, MutableNotesMigration notesMigration) {
+  public InMemoryModule(Config cfg) {
     this.cfg = cfg;
-    this.notesMigration = notesMigration;
   }
 
   public void inject(Object instance) {
@@ -191,8 +187,6 @@ public class InMemoryModule extends FactoryModule {
     bind(GitRepositoryManager.class).to(InMemoryRepositoryManager.class);
     bind(InMemoryRepositoryManager.class).in(SINGLETON);
     bind(TrackingFooters.class).toProvider(TrackingFootersProvider.class).in(SINGLETON);
-    bind(MutableNotesMigration.class).toInstance(notesMigration);
-    bind(NotesMigration.class).to(MutableNotesMigration.class);
     bind(ListeningExecutorService.class)
         .annotatedWith(ChangeUpdateExecutor.class)
         .toInstance(MoreExecutors.newDirectExecutorService());

java/com/google/gerrit/testing/InMemoryTestEnvironment.java

@@ -108,8 +108,7 @@ public final class InMemoryTestEnvironment implements MethodRule {
     Config cfg = configProvider.get();
     InMemoryModule.setDefaults(cfg);
 
-    Injector injector =
+    Injector injector = Guice.createInjector(new InMemoryModule(cfg));
-        Guice.createInjector(new InMemoryModule(cfg, NoteDbMode.newNotesMigrationFromEnv()));
     injector.injectMembers(this);
     lifecycle = new LifecycleManager();
     lifecycle.add(injector);

java/com/google/gerrit/testing/NoteDbMode.java

@@ -1,81 +0,0 @@
-// 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.
-// 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.testing;
-
-import static com.google.common.base.Preconditions.checkArgument;
-
-import com.google.common.base.Enums;
-import com.google.common.base.Strings;
-import com.google.gerrit.server.notedb.MutableNotesMigration;
-import com.google.gerrit.server.notedb.NotesMigrationState;
-
-public enum NoteDbMode {
-  /** NoteDb is disabled. */
-  OFF(NotesMigrationState.REVIEW_DB),
-
-  /** Writing data to NoteDb is enabled. */
-  WRITE(NotesMigrationState.WRITE),
-
-  /** Reading and writing all data to NoteDb is enabled. */
-  READ_WRITE(NotesMigrationState.READ_WRITE_WITH_SEQUENCE_REVIEW_DB_PRIMARY),
-
-  /** Changes are created with their primary storage as NoteDb. */
-  PRIMARY(NotesMigrationState.READ_WRITE_WITH_SEQUENCE_NOTE_DB_PRIMARY),
-
-  /** All change tables are entirely disabled, and code/meta ref updates are fused. */
-  ON(NotesMigrationState.NOTE_DB);
-
-  private static final String ENV_VAR = "GERRIT_NOTEDB";
-  private static final String SYS_PROP = "gerrit.notedb";
-
-  public static NoteDbMode get() {
-    String value = System.getenv(ENV_VAR);
-    if (Strings.isNullOrEmpty(value)) {
-      value = System.getProperty(SYS_PROP);
-    }
-    if (Strings.isNullOrEmpty(value)) {
-      return ON;
-    }
-    value = value.toUpperCase().replace("-", "_");
-    NoteDbMode mode = Enums.getIfPresent(NoteDbMode.class, value).orNull();
-    if (!Strings.isNullOrEmpty(System.getenv(ENV_VAR))) {
-      checkArgument(
-          mode != null, "Invalid value for env variable %s: %s", ENV_VAR, System.getenv(ENV_VAR));
-    } else {
-      checkArgument(
-          mode != null,
-          "Invalid value for system property %s: %s",
-          SYS_PROP,
-          System.getProperty(SYS_PROP));
-    }
-    return mode;
-  }
-
-  public static MutableNotesMigration newNotesMigrationFromEnv() {
-    MutableNotesMigration m = MutableNotesMigration.newDisabled();
-    resetFromEnv(m);
-    return m;
-  }
-
-  public static void resetFromEnv(MutableNotesMigration migration) {
-    migration.setFrom(get().state);
-  }
-
-  private final NotesMigrationState state;
-
-  private NoteDbMode(NotesMigrationState state) {
-    this.state = state;
-  }
-}

javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java

@@ -53,7 +53,6 @@ import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
 import com.google.gerrit.server.project.testing.Util;
 import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.testing.NoteDbMode;
 import com.google.inject.Inject;
 import java.io.IOException;
 import java.util.ArrayList;
@@ -665,9 +664,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
 
     List<String> expectedMetaRefs =
         new ArrayList<>(ImmutableList.of(mr.getPatchSetId().toRefName()));
-    if (NoteDbMode.get() != NoteDbMode.OFF) {
     expectedMetaRefs.add(changeRefPrefix(mr.getChange().getId()) + "meta");
-    }
 
     List<String> expectedAllRefs = new ArrayList<>(expectedNonMetaRefs);
     expectedAllRefs.addAll(expectedMetaRefs);

javatests/com/google/gerrit/elasticsearch/ElasticV5QueryAccountsTest.java

@@ -65,6 +65,6 @@ public class ElasticV5QueryAccountsTest extends AbstractQueryAccountsTest {
     String indicesPrefix = getSanitizedMethodName();
     ElasticTestUtils.configure(
         elasticsearchConfig, nodeInfo.port, indicesPrefix, ElasticVersion.V5_6);
-    return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
+    return Guice.createInjector(new InMemoryModule(elasticsearchConfig));
   }
 }

javatests/com/google/gerrit/elasticsearch/ElasticV5QueryChangesTest.java

@@ -65,6 +65,6 @@ public class ElasticV5QueryChangesTest extends AbstractQueryChangesTest {
     String indicesPrefix = getSanitizedMethodName();
     ElasticTestUtils.configure(
         elasticsearchConfig, nodeInfo.port, indicesPrefix, ElasticVersion.V5_6);
-    return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
+    return Guice.createInjector(new InMemoryModule(elasticsearchConfig));
   }
 }

javatests/com/google/gerrit/elasticsearch/ElasticV5QueryGroupsTest.java

@@ -65,6 +65,6 @@ public class ElasticV5QueryGroupsTest extends AbstractQueryGroupsTest {
     String indicesPrefix = getSanitizedMethodName();
     ElasticTestUtils.configure(
         elasticsearchConfig, nodeInfo.port, indicesPrefix, ElasticVersion.V5_6);
-    return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
+    return Guice.createInjector(new InMemoryModule(elasticsearchConfig));
   }
 }

javatests/com/google/gerrit/elasticsearch/ElasticV5QueryProjectsTest.java

@@ -65,6 +65,6 @@ public class ElasticV5QueryProjectsTest extends AbstractQueryProjectsTest {
     String indicesPrefix = getSanitizedMethodName();
     ElasticTestUtils.configure(
         elasticsearchConfig, nodeInfo.port, indicesPrefix, ElasticVersion.V5_6);
-    return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
+    return Guice.createInjector(new InMemoryModule(elasticsearchConfig));
   }
 }

javatests/com/google/gerrit/elasticsearch/ElasticV6QueryAccountsTest.java

@@ -64,6 +64,6 @@ public class ElasticV6QueryAccountsTest extends AbstractQueryAccountsTest {
     InMemoryModule.setDefaults(elasticsearchConfig);
     String indicesPrefix = getSanitizedMethodName();
     ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix);
-    return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
+    return Guice.createInjector(new InMemoryModule(elasticsearchConfig));
   }
 }

javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java

@@ -64,6 +64,6 @@ public class ElasticV6QueryChangesTest extends AbstractQueryChangesTest {
     InMemoryModule.setDefaults(elasticsearchConfig);
     String indicesPrefix = getSanitizedMethodName();
     ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix);
-    return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
+    return Guice.createInjector(new InMemoryModule(elasticsearchConfig));
   }
 }

javatests/com/google/gerrit/elasticsearch/ElasticV6QueryGroupsTest.java

@@ -64,6 +64,6 @@ public class ElasticV6QueryGroupsTest extends AbstractQueryGroupsTest {
     InMemoryModule.setDefaults(elasticsearchConfig);
     String indicesPrefix = getSanitizedMethodName();
     ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix);
-    return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
+    return Guice.createInjector(new InMemoryModule(elasticsearchConfig));
   }
 }

javatests/com/google/gerrit/elasticsearch/ElasticV6QueryProjectsTest.java

@@ -64,6 +64,6 @@ public class ElasticV6QueryProjectsTest extends AbstractQueryProjectsTest {
     InMemoryModule.setDefaults(elasticsearchConfig);
     String indicesPrefix = getSanitizedMethodName();
     ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix);
-    return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
+    return Guice.createInjector(new InMemoryModule(elasticsearchConfig));
   }
 }

javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java

@@ -64,6 +64,6 @@ public class ElasticV7QueryAccountsTest extends AbstractQueryAccountsTest {
     InMemoryModule.setDefaults(elasticsearchConfig);
     String indicesPrefix = getSanitizedMethodName();
     ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix);
-    return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
+    return Guice.createInjector(new InMemoryModule(elasticsearchConfig));
   }
 }

javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java

@@ -82,6 +82,6 @@ public class ElasticV7QueryChangesTest extends AbstractQueryChangesTest {
     InMemoryModule.setDefaults(elasticsearchConfig);
     String indicesPrefix = getSanitizedMethodName();
     ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix);
-    return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
+    return Guice.createInjector(new InMemoryModule(elasticsearchConfig));
   }
 }

javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java

@@ -64,6 +64,6 @@ public class ElasticV7QueryGroupsTest extends AbstractQueryGroupsTest {
     InMemoryModule.setDefaults(elasticsearchConfig);
     String indicesPrefix = getSanitizedMethodName();
     ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix);
-    return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
+    return Guice.createInjector(new InMemoryModule(elasticsearchConfig));
   }
 }

javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java

@@ -64,6 +64,6 @@ public class ElasticV7QueryProjectsTest extends AbstractQueryProjectsTest {
     InMemoryModule.setDefaults(elasticsearchConfig);
     String indicesPrefix = getSanitizedMethodName();
     ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix);
-    return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
+    return Guice.createInjector(new InMemoryModule(elasticsearchConfig));
   }
 }

javatests/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java

@@ -47,7 +47,6 @@ import com.google.gerrit.server.util.ThreadLocalRequestContext;
 import com.google.gerrit.testing.GerritBaseTests;
 import com.google.gerrit.testing.InMemoryDatabase;
 import com.google.gerrit.testing.InMemoryModule;
-import com.google.gerrit.testing.NoteDbMode;
 import com.google.inject.Guice;
 import com.google.inject.Inject;
 import com.google.inject.Injector;
@@ -103,8 +102,7 @@ public class GerritPublicKeyCheckerTest extends GerritBaseTests {
         ImmutableList.of(
             Fingerprint.toString(keyB().getPublicKey().getFingerprint()),
             Fingerprint.toString(keyD().getPublicKey().getFingerprint())));
-    Injector injector =
+    Injector injector = Guice.createInjector(new InMemoryModule(cfg));
-        Guice.createInjector(new InMemoryModule(cfg, NoteDbMode.newNotesMigrationFromEnv()));
 
     lifecycle = new LifecycleManager();
     lifecycle.add(injector);

javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java

@@ -161,10 +161,6 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests {
                 bind(GitReferenceUpdated.class).toInstance(GitReferenceUpdated.DISABLED);
                 bind(MetricMaker.class).to(DisabledMetricMaker.class);
                 bind(ReviewDb.class).toProvider(Providers.<ReviewDb>of(null));
-                MutableNotesMigration migration = MutableNotesMigration.newDisabled();
-                migration.setFrom(NotesMigrationState.FINAL);
-                bind(MutableNotesMigration.class).toInstance(migration);
-                bind(NotesMigration.class).to(MutableNotesMigration.class);
 
                 // Tests don't support ReviewDb at all, but bindings are required via NoteDbModule.
                 bind(new TypeLiteral<SchemaFactory<ReviewDb>>() {})

javatests/com/google/gerrit/server/query/account/LuceneQueryAccountsTest.java

@@ -44,6 +44,6 @@ public class LuceneQueryAccountsTest extends AbstractQueryAccountsTest {
   protected Injector createInjector() {
     Config luceneConfig = new Config(config);
     InMemoryModule.setDefaults(luceneConfig);
-    return Guice.createInjector(new InMemoryModule(luceneConfig, notesMigration));
+    return Guice.createInjector(new InMemoryModule(luceneConfig));
   }
 }

javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java

@@ -49,7 +49,7 @@ public class LuceneQueryChangesTest extends AbstractQueryChangesTest {
   protected Injector createInjector() {
     Config luceneConfig = new Config(config);
     InMemoryModule.setDefaults(luceneConfig);
-    return Guice.createInjector(new InMemoryModule(luceneConfig, notesMigration));
+    return Guice.createInjector(new InMemoryModule(luceneConfig));
   }
 
   @Test

javatests/com/google/gerrit/server/query/group/LuceneQueryGroupsTest.java

@@ -43,6 +43,6 @@ public class LuceneQueryGroupsTest extends AbstractQueryGroupsTest {
   protected Injector createInjector() {
     Config luceneConfig = new Config(config);
     InMemoryModule.setDefaults(luceneConfig);
-    return Guice.createInjector(new InMemoryModule(luceneConfig, notesMigration));
+    return Guice.createInjector(new InMemoryModule(luceneConfig));
   }
 }

javatests/com/google/gerrit/server/query/project/LuceneQueryProjectsTest.java

@@ -45,6 +45,6 @@ public class LuceneQueryProjectsTest extends AbstractQueryProjectsTest {
   protected Injector createInjector() {
     Config luceneConfig = new Config(config);
     InMemoryModule.setDefaults(luceneConfig);
-    return Guice.createInjector(new InMemoryModule(luceneConfig, notesMigration));
+    return Guice.createInjector(new InMemoryModule(luceneConfig));
   }
 }

javatests/com/google/gerrit/server/schema/NoteDbSchemaUpdaterTest.java

@@ -29,9 +29,8 @@ import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.notedb.IntBlob;
-import com.google.gerrit.server.notedb.MutableNotesMigration;
 import com.google.gerrit.server.notedb.NoteDbSchemaVersionManager;
-import com.google.gerrit.server.notedb.NotesMigrationState;
+import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.notedb.RepoSequence;
 import com.google.gerrit.testing.GerritBaseTests;
 import com.google.gerrit.testing.InMemoryRepositoryManager;
@@ -117,8 +116,6 @@ public class NoteDbSchemaUpdaterTest extends GerritBaseTests {
       args = new NoteDbSchemaVersion.Arguments(repoManager, allProjectsName);
       NoteDbSchemaVersionManager versionManager =
           new NoteDbSchemaVersionManager(allProjectsName, repoManager);
-      MutableNotesMigration notesMigration = MutableNotesMigration.newDisabled();
-      notesMigration.setFrom(NotesMigrationState.NOTE_DB);
       updater =
           new NoteDbSchemaUpdater(
               cfg,
@@ -126,7 +123,7 @@ public class NoteDbSchemaUpdaterTest extends GerritBaseTests {
               allUsersName,
               repoManager,
               schemaCreator,
-              notesMigration,
+              new NotesMigration(),
               versionManager,
               args,
               ImmutableSortedMap.of(10, TestSchema_10.class, 11, TestSchema_11.class));