Support disabling change ReviewDb table access entirely

We already had DisabledChangesReviewDbWrapper to throw an exception
indicating it's a programmer error to try to read from changes tables
directly. By design, it was still possible to punch through this wrapper
and get access to the ReviewDb table.

We need to go deeper if we really want to shut off the changes tables.
Add a new config option that adds another level of wrapping to ReviewDb,
which makes all operations into complete no-ops. When this option is
set, there is literally no way to write to or read from the underlying
database table using the ReviewDb API. The table could go away and
Gerrit would still function.

Change-Id: Ia23a51449217a50eeaecd9934626270739c096e0

Documentation/dev-note-db.txt

@@ -77,6 +77,16 @@ previous options, unless otherwise noted.
   Due to an implementation detail, writes to Changes or related tables still
   result in write calls to the database layer, but they are inside a transaction
   that is always rolled back.
+- `noteDb.changes.disableReviewDb=true`: All access to Changes or related tables
+  is disabled; reads return no results, and writes are no-ops. Assumes the state
+  of all changes in NoteDb is accurate, and so is only safe once all changes are
+  NoteDb primary. Otherwise, reading changes only from NoteDb might result in
+  inaccurate results, and writing to NoteDb would compound the problem. +
+  Thus it is up to an admin of a previously-ReviewDb site to ensure
+  MigratePrimaryStorage has been run for all changes. Note that the current
+  implementation of the `rebuild-note-db` program does not do this. +
+  In this phase, it would be possible to delete the Changes tables out from
+  under a running server with no effect.
 
 [[migration]]
 == Migration

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/DisallowReadFromChangesReviewDbWrapper.java

@@ -24,22 +24,22 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.ResultSet;
 
-public class DisabledChangesReviewDbWrapper extends ReviewDbWrapper {
+public class DisallowReadFromChangesReviewDbWrapper extends ReviewDbWrapper {
   private static final String MSG = "This table has been migrated to NoteDb";
 
-  private final DisabledChangeAccess changes;
-  private final DisabledPatchSetApprovalAccess patchSetApprovals;
-  private final DisabledChangeMessageAccess changeMessages;
-  private final DisabledPatchSetAccess patchSets;
-  private final DisabledPatchLineCommentAccess patchComments;
+  private final Changes changes;
+  private final PatchSetApprovals patchSetApprovals;
+  private final ChangeMessages changeMessages;
+  private final PatchSets patchSets;
+  private final PatchLineComments patchComments;
 
-  public DisabledChangesReviewDbWrapper(ReviewDb db) {
+  public DisallowReadFromChangesReviewDbWrapper(ReviewDb db) {
     super(db);
-    changes = new DisabledChangeAccess(delegate.changes());
-    patchSetApprovals = new DisabledPatchSetApprovalAccess(delegate.patchSetApprovals());
-    changeMessages = new DisabledChangeMessageAccess(delegate.changeMessages());
-    patchSets = new DisabledPatchSetAccess(delegate.patchSets());
-    patchComments = new DisabledPatchLineCommentAccess(delegate.patchComments());
+    changes = new Changes(delegate.changes());
+    patchSetApprovals = new PatchSetApprovals(delegate.patchSetApprovals());
+    changeMessages = new ChangeMessages(delegate.changeMessages());
+    patchSets = new PatchSets(delegate.patchSets());
+    patchComments = new PatchLineComments(delegate.patchComments());
   }
 
   public ReviewDb unsafeGetDelegate() {
@@ -71,9 +71,9 @@ public class DisabledChangesReviewDbWrapper extends ReviewDbWrapper {
     return patchComments;
   }
 
-  private static class DisabledChangeAccess extends ChangeAccessWrapper {
+  private static class Changes extends ChangeAccessWrapper {
 
-    protected DisabledChangeAccess(ChangeAccess delegate) {
+    protected Changes(ChangeAccess delegate) {
       super(delegate);
     }
 
@@ -103,8 +103,8 @@ public class DisabledChangesReviewDbWrapper extends ReviewDbWrapper {
     }
   }
 
-  private static class DisabledPatchSetApprovalAccess extends PatchSetApprovalAccessWrapper {
-    DisabledPatchSetApprovalAccess(PatchSetApprovalAccess delegate) {
+  private static class PatchSetApprovals extends PatchSetApprovalAccessWrapper {
+    PatchSetApprovals(PatchSetApprovalAccess delegate) {
       super(delegate);
     }
 
@@ -139,8 +139,8 @@ public class DisabledChangesReviewDbWrapper extends ReviewDbWrapper {
     }
   }
 
-  private static class DisabledChangeMessageAccess extends ChangeMessageAccessWrapper {
-    DisabledChangeMessageAccess(ChangeMessageAccess delegate) {
+  private static class ChangeMessages extends ChangeMessageAccessWrapper {
+    ChangeMessages(ChangeMessageAccess delegate) {
       super(delegate);
     }
 
@@ -180,8 +180,8 @@ public class DisabledChangesReviewDbWrapper extends ReviewDbWrapper {
     }
   }
 
-  private static class DisabledPatchSetAccess extends PatchSetAccessWrapper {
-    DisabledPatchSetAccess(PatchSetAccess delegate) {
+  private static class PatchSets extends PatchSetAccessWrapper {
+    PatchSets(PatchSetAccess delegate) {
       super(delegate);
     }
 
@@ -211,8 +211,8 @@ public class DisabledChangesReviewDbWrapper extends ReviewDbWrapper {
     }
   }
 
-  private static class DisabledPatchLineCommentAccess extends PatchLineCommentAccessWrapper {
-    DisabledPatchLineCommentAccess(PatchLineCommentAccess delegate) {
+  private static class PatchLineComments extends PatchLineCommentAccessWrapper {
+    PatchLineComments(PatchLineCommentAccess delegate) {
       super(delegate);
     }
 

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbUtil.java

@@ -42,8 +42,8 @@ public class ReviewDbUtil {
   }
 
   public static ReviewDb unwrapDb(ReviewDb db) {
-    if (db instanceof DisabledChangesReviewDbWrapper) {
-      return ((DisabledChangesReviewDbWrapper) db).unsafeGetDelegate();
+    if (db instanceof DisallowReadFromChangesReviewDbWrapper) {
+      return ((DisallowReadFromChangesReviewDbWrapper) db).unsafeGetDelegate();
     }
     return db;
   }

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

@@ -47,6 +47,7 @@ public class ConfigNotesMigration extends NotesMigration {
   private static final String NOTE_DB = "noteDb";
 
   // All of these names must be reflected in the allowed set in checkConfig.
+  private static final String DISABLE_REVIEW_DB = "disableReviewDb";
   private static final String PRIMARY_STORAGE = "primaryStorage";
   private static final String READ = "read";
   private static final String SEQUENCE = "sequence";
@@ -56,6 +57,7 @@ public class ConfigNotesMigration extends NotesMigration {
     Set<String> keys = ImmutableSet.of(CHANGES.key());
     Set<String> allowed =
         ImmutableSet.of(
+            DISABLE_REVIEW_DB.toLowerCase(),
             PRIMARY_STORAGE.toLowerCase(),
             READ.toLowerCase(),
             WRITE.toLowerCase(),
@@ -79,6 +81,7 @@ public class ConfigNotesMigration extends NotesMigration {
   private final boolean readChanges;
   private final boolean readChangeSequence;
   private final PrimaryStorage changePrimaryStorage;
+  private final boolean disableChangeReviewDb;
 
   @Inject
   ConfigNotesMigration(@GerritServerConfig Config cfg) {
@@ -95,6 +98,11 @@ public class ConfigNotesMigration extends NotesMigration {
 
     changePrimaryStorage =
         cfg.getEnum(NOTE_DB, CHANGES.key(), PRIMARY_STORAGE, PrimaryStorage.REVIEW_DB);
+    disableChangeReviewDb = cfg.getBoolean(NOTE_DB, CHANGES.key(), DISABLE_REVIEW_DB, false);
+
+    checkArgument(
+        !(disableChangeReviewDb && changePrimaryStorage != PrimaryStorage.NOTE_DB),
+        "cannot disable ReviewDb for changes if default change primary storage is ReviewDb");
   }
 
   @Override
@@ -116,4 +124,9 @@ public class ConfigNotesMigration extends NotesMigration {
   public PrimaryStorage changePrimaryStorage() {
     return changePrimaryStorage;
   }
+
+  @Override
+  public boolean disableChangeReviewDb() {
+    return disableChangeReviewDb;
+  }
 }

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

@@ -70,6 +70,15 @@ public abstract class NotesMigration {
   /** @return default primary storage for new changes. */
   public abstract PrimaryStorage changePrimaryStorage();
 
+  /**
+   * Disable ReviewDb access for changes.
+   *
+   * <p>When set, ReviewDb operations involving the Changes table become no-ops. Lookups return no
+   * results; updates do nothing, as does opening, committing, or rolling back a transaction on the
+   * Changes table.
+   */
+  public abstract boolean disableChangeReviewDb();
+
   /**
    * Whether to fail when reading any data from NoteDb.
    *

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

@@ -0,0 +1,350 @@
+// 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.schema;
+
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.util.concurrent.CheckedFuture;
+import com.google.common.util.concurrent.Futures;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.ChangeMessage;
+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.server.ChangeAccess;
+import com.google.gerrit.reviewdb.server.ChangeMessageAccess;
+import com.google.gerrit.reviewdb.server.PatchLineCommentAccess;
+import com.google.gerrit.reviewdb.server.PatchSetAccess;
+import com.google.gerrit.reviewdb.server.PatchSetApprovalAccess;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.reviewdb.server.ReviewDbWrapper;
+import com.google.gwtorm.client.Key;
+import com.google.gwtorm.server.Access;
+import com.google.gwtorm.server.AtomicUpdate;
+import com.google.gwtorm.server.ListResultSet;
+import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.ResultSet;
+import java.util.Map;
+import java.util.function.Function;
+
+/**
+ * Wrapper for ReviewDb that never calls the underlying change tables.
+ *
+ * <p>See {@link NotesMigrationSchemaFactory} for discussion.
+ */
+class NoChangesReviewDbWrapper extends ReviewDbWrapper {
+  private static <T> ResultSet<T> empty() {
+    return new ListResultSet<>(ImmutableList.of());
+  }
+
+  private static <T, K extends Key<?>> CheckedFuture<T, OrmException> emptyFuture() {
+    return Futures.immediateCheckedFuture(null);
+  }
+
+  private final ChangeAccess changes;
+  private final PatchSetApprovalAccess patchSetApprovals;
+  private final ChangeMessageAccess changeMessages;
+  private final PatchSetAccess patchSets;
+  private final PatchLineCommentAccess patchComments;
+
+  private boolean inTransaction;
+
+  NoChangesReviewDbWrapper(ReviewDb db) {
+    super(db);
+    changes = new Changes(this, delegate);
+    patchSetApprovals = new PatchSetApprovals(this, delegate);
+    changeMessages = new ChangeMessages(this, delegate);
+    patchSets = new PatchSets(this, delegate);
+    patchComments = new PatchLineComments(this, delegate);
+  }
+
+  @Override
+  public ChangeAccess changes() {
+    return changes;
+  }
+
+  @Override
+  public PatchSetApprovalAccess patchSetApprovals() {
+    return patchSetApprovals;
+  }
+
+  @Override
+  public ChangeMessageAccess changeMessages() {
+    return changeMessages;
+  }
+
+  @Override
+  public PatchSetAccess patchSets() {
+    return patchSets;
+  }
+
+  @Override
+  public PatchLineCommentAccess patchComments() {
+    return patchComments;
+  }
+
+  @Override
+  public void commit() throws OrmException {
+    if (!inTransaction) {
+      // This reads a little weird, we're not in a transaction, so why are we calling commit?
+      // Because we want to let the underlying ReviewDb do its normal thing in this case (which may
+      // be throwing an exception, or not, depending on implementation).
+      delegate.commit();
+    }
+  }
+
+  @Override
+  public void rollback() throws OrmException {
+    if (inTransaction) {
+      inTransaction = false;
+    } else {
+      // See comment in commit(): we want to let the underlying ReviewDb do its thing.
+      delegate.rollback();
+    }
+  }
+
+  private abstract static class AbstractDisabledAccess<T, K extends Key<?>>
+      implements Access<T, K> {
+    // Don't even hold a reference to delegate, so it's not possible to use it accidentally.
+    private final NoChangesReviewDbWrapper wrapper;
+    private final String relationName;
+    private final int relationId;
+    private final Function<T, K> primaryKey;
+    private final Function<Iterable<T>, Map<K, T>> toMap;
+
+    private AbstractDisabledAccess(NoChangesReviewDbWrapper wrapper, Access<T, K> delegate) {
+      this.wrapper = wrapper;
+      this.relationName = delegate.getRelationName();
+      this.relationId = delegate.getRelationID();
+      this.primaryKey = delegate::primaryKey;
+      this.toMap = delegate::toMap;
+    }
+
+    @Override
+    public final int getRelationID() {
+      return relationId;
+    }
+
+    @Override
+    public final String getRelationName() {
+      return relationName;
+    }
+
+    @Override
+    public final K primaryKey(T entity) {
+      return primaryKey.apply(entity);
+    }
+
+    @Override
+    public final Map<K, T> toMap(Iterable<T> iterable) {
+      return toMap.apply(iterable);
+    }
+
+    @Override
+    public final ResultSet<T> iterateAllEntities() {
+      return empty();
+    }
+
+    @Override
+    public final CheckedFuture<T, OrmException> getAsync(K key) {
+      return emptyFuture();
+    }
+
+    @Override
+    public final ResultSet<T> get(Iterable<K> keys) {
+      return empty();
+    }
+
+    @Override
+    public final void insert(Iterable<T> instances) {
+      // Do nothing.
+    }
+
+    @Override
+    public final void update(Iterable<T> instances) {
+      // Do nothing.
+    }
+
+    @Override
+    public final void upsert(Iterable<T> instances) {
+      // Do nothing.
+    }
+
+    @Override
+    public final void deleteKeys(Iterable<K> keys) {
+      // Do nothing.
+    }
+
+    @Override
+    public final void delete(Iterable<T> instances) {
+      // Do nothing.
+    }
+
+    @Override
+    public final void beginTransaction(K key) {
+      // Keep track of when we've started a transaction so that we can avoid calling commit/rollback
+      // on the underlying ReviewDb. This is just a simple arm's-length approach, and may produce
+      // slightly different results from a native ReviewDb in corner cases like:
+      //  * beginning transactions on different tables simultaneously
+      //  * doing work between commit and rollback
+      // These kinds of things are already misuses of ReviewDb, and shouldn't be happening in
+      // current code anyway.
+      checkState(!wrapper.inTransaction, "already in transaction");
+      wrapper.inTransaction = true;
+    }
+
+    @Override
+    public final T atomicUpdate(K key, AtomicUpdate<T> update) {
+      return null;
+    }
+
+    @Override
+    public final T get(K id) {
+      return null;
+    }
+  }
+
+  private static class Changes extends AbstractDisabledAccess<Change, Change.Id>
+      implements ChangeAccess {
+    private Changes(NoChangesReviewDbWrapper wrapper, ReviewDb db) {
+      super(wrapper, db.changes());
+    }
+
+    @Override
+    public ResultSet<Change> all() {
+      return empty();
+    }
+  }
+
+  private static class ChangeMessages
+      extends AbstractDisabledAccess<ChangeMessage, ChangeMessage.Key>
+      implements ChangeMessageAccess {
+    private ChangeMessages(NoChangesReviewDbWrapper wrapper, ReviewDb db) {
+      super(wrapper, db.changeMessages());
+    }
+
+    @Override
+    public ResultSet<ChangeMessage> byChange(Change.Id id) throws OrmException {
+      return empty();
+    }
+
+    @Override
+    public ResultSet<ChangeMessage> byPatchSet(PatchSet.Id id) throws OrmException {
+      return empty();
+    }
+
+    @Override
+    public ResultSet<ChangeMessage> all() throws OrmException {
+      return empty();
+    }
+  }
+
+  private static class PatchSets extends AbstractDisabledAccess<PatchSet, PatchSet.Id>
+      implements PatchSetAccess {
+    private PatchSets(NoChangesReviewDbWrapper wrapper, ReviewDb db) {
+      super(wrapper, db.patchSets());
+    }
+
+    @Override
+    public ResultSet<PatchSet> byChange(Change.Id id) {
+      return empty();
+    }
+
+    @Override
+    public ResultSet<PatchSet> all() {
+      return empty();
+    }
+  }
+
+  private static class PatchSetApprovals
+      extends AbstractDisabledAccess<PatchSetApproval, PatchSetApproval.Key>
+      implements PatchSetApprovalAccess {
+    private PatchSetApprovals(NoChangesReviewDbWrapper wrapper, ReviewDb db) {
+      super(wrapper, db.patchSetApprovals());
+    }
+
+    @Override
+    public ResultSet<PatchSetApproval> byChange(Change.Id id) {
+      return empty();
+    }
+
+    @Override
+    public ResultSet<PatchSetApproval> byPatchSet(PatchSet.Id id) {
+      return empty();
+    }
+
+    @Override
+    public ResultSet<PatchSetApproval> byPatchSetUser(PatchSet.Id patchSet, Account.Id account) {
+      return empty();
+    }
+
+    @Override
+    public ResultSet<PatchSetApproval> all() {
+      return empty();
+    }
+  }
+
+  private static class PatchLineComments
+      extends AbstractDisabledAccess<PatchLineComment, PatchLineComment.Key>
+      implements PatchLineCommentAccess {
+    private PatchLineComments(NoChangesReviewDbWrapper wrapper, ReviewDb db) {
+      super(wrapper, db.patchComments());
+    }
+
+    @Override
+    public ResultSet<PatchLineComment> byChange(Change.Id id) {
+      return empty();
+    }
+
+    @Override
+    public ResultSet<PatchLineComment> byPatchSet(PatchSet.Id id) {
+      return empty();
+    }
+
+    @Override
+    public ResultSet<PatchLineComment> publishedByChangeFile(Change.Id id, String file) {
+      return empty();
+    }
+
+    @Override
+    public ResultSet<PatchLineComment> publishedByPatchSet(PatchSet.Id patchset) {
+      return empty();
+    }
+
+    @Override
+    public ResultSet<PatchLineComment> draftByPatchSetAuthor(
+        PatchSet.Id patchset, Account.Id author) {
+      return empty();
+    }
+
+    @Override
+    public ResultSet<PatchLineComment> draftByChangeFileAuthor(
+        Change.Id id, String file, Account.Id author) {
+      return empty();
+    }
+
+    @Override
+    public ResultSet<PatchLineComment> draftByAuthor(Account.Id author) {
+      return empty();
+    }
+
+    @Override
+    public ResultSet<PatchLineComment> all() {
+      return empty();
+    }
+  }
+}

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

@@ -14,7 +14,7 @@
 
 package com.google.gerrit.server.schema;
 
-import com.google.gerrit.reviewdb.server.DisabledChangesReviewDbWrapper;
+import com.google.gerrit.reviewdb.server.DisallowReadFromChangesReviewDbWrapper;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gwtorm.server.OrmException;
@@ -40,6 +40,34 @@ public class NotesMigrationSchemaFactory implements SchemaFactory<ReviewDb> {
     if (!migration.readChanges()) {
       return db;
     }
-    return new DisabledChangesReviewDbWrapper(db);
+
+    // There are two levels at which this class disables access to Changes and related tables,
+    // corresponding to two phases of the NoteDb migration:
+    //
+    // 1. When changes are read from NoteDb but some changes might still have their primary storage
+    //    in ReviewDb, it is generally programmer error to read changes from ReviewDb. However,
+    //    since ReviewDb is still the primary storage for most or all changes, we still need to
+    //    support writing to ReviewDb. This behavior is accomplished by wrapping in a
+    //    DisallowReadFromChangesReviewDbWrapper.
+    //
+    //    Some codepaths might need to be able to read from ReviewDb if they really need to, because
+    //    they need to operate on the underlying source of truth, for example when reading a change
+    //    to determine its primary storage. To support this, ReviewDbUtil#unwrapDb can detect and
+    //    unwrap databases of this type.
+    //
+    // 2. After all changes have their primary storage in NoteDb, we can completely shut off access
+    //    to the change tables. At this point in the migration, we are by definition not using the
+    //    ReviewDb tables at all; we could even delete the tables at this point, and Gerrit would
+    //    continue to function.
+    //
+    //    This is accomplished by setting the delegate ReviewDb *underneath* DisallowReadFromChanges
+    //    to be a complete no-op, with NoChangesReviewDbWrapper. With this wrapper, all read
+    //    operations return no results, and write operations silently do nothing. This wrapper is
+    //    not a public class and nobody should ever attempt to unwrap it.
+
+    if (migration.disableChangeReviewDb()) {
+      db = new NoChangesReviewDbWrapper(db);
+    }
+    return new DisallowReadFromChangesReviewDbWrapper(db);
   }
 }

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

@@ -21,22 +21,25 @@ import com.google.common.base.Strings;
 
 public enum NoteDbMode {
   /** NoteDb is disabled. */
-  OFF,
+  OFF(false),
 
   /** Writing data to NoteDb is enabled. */
-  WRITE,
+  WRITE(false),
 
   /** Reading and writing all data to NoteDb is enabled. */
-  READ_WRITE,
+  READ_WRITE(true),
 
   /** Changes are created with their primary storage as NoteDb. */
-  PRIMARY,
+  PRIMARY(true),
+
+  /** All change tables are entirely disabled. */
+  DISABLE_CHANGE_REVIEW_DB(true),
 
   /**
    * Run tests with NoteDb disabled, then convert ReviewDb to NoteDb and check that the results
    * match.
    */
-  CHECK;
+  CHECK(false);
 
   private static final String ENV_VAR = "GERRIT_NOTEDB";
   private static final String SYS_PROP = "gerrit.notedb";
@@ -65,6 +68,12 @@ public enum NoteDbMode {
   }
 
   public static boolean readWrite() {
-    return get() == READ_WRITE || get() == PRIMARY;
+    return get().readWrite;
+  }
+
+  private final boolean readWrite;
+
+  private NoteDbMode(boolean readWrite) {
+    this.readWrite = readWrite;
   }
 }

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

@@ -26,6 +26,7 @@ public class TestNotesMigration extends NotesMigration {
   private volatile boolean readChanges;
   private volatile boolean writeChanges;
   private volatile PrimaryStorage changePrimaryStorage = PrimaryStorage.REVIEW_DB;
+  private volatile boolean disableChangeReviewDb;
   private volatile boolean failOnLoad;
 
   @Override
@@ -45,6 +46,11 @@ public class TestNotesMigration extends NotesMigration {
     return changePrimaryStorage;
   }
 
+  @Override
+  public boolean disableChangeReviewDb() {
+    return disableChangeReviewDb;
+  }
+
   // Increase visbility from superclass, as tests may want to check whether
   // NoteDb data is written in specific migration scenarios.
   @Override
@@ -72,6 +78,11 @@ public class TestNotesMigration extends NotesMigration {
     return this;
   }
 
+  public TestNotesMigration setDisableChangeReviewDb(boolean disableChangeReviewDb) {
+    this.disableChangeReviewDb = disableChangeReviewDb;
+    return this;
+  }
+
   public TestNotesMigration setFailOnLoad(boolean failOnLoad) {
     this.failOnLoad = failOnLoad;
     return this;
@@ -87,16 +98,25 @@ public class TestNotesMigration extends NotesMigration {
         setWriteChanges(true);
         setReadChanges(true);
         setChangePrimaryStorage(PrimaryStorage.REVIEW_DB);
+        setDisableChangeReviewDb(false);
         break;
       case WRITE:
         setWriteChanges(true);
         setReadChanges(false);
         setChangePrimaryStorage(PrimaryStorage.REVIEW_DB);
+        setDisableChangeReviewDb(false);
         break;
       case PRIMARY:
         setWriteChanges(true);
         setReadChanges(true);
         setChangePrimaryStorage(PrimaryStorage.NOTE_DB);
+        setDisableChangeReviewDb(false);
+        break;
+      case DISABLE_CHANGE_REVIEW_DB:
+        setWriteChanges(true);
+        setReadChanges(true);
+        setChangePrimaryStorage(PrimaryStorage.NOTE_DB);
+        setDisableChangeReviewDb(true);
         break;
       case CHECK:
       case OFF:
@@ -104,6 +124,7 @@ public class TestNotesMigration extends NotesMigration {
         setWriteChanges(false);
         setReadChanges(false);
         setChangePrimaryStorage(PrimaryStorage.REVIEW_DB);
+        setDisableChangeReviewDb(false);
         break;
     }
     return this;