Automatically rebuild out-of-date NoteDb changes

Changes are always written to ReviewDb before NoteDb, which implies
ReviewDb is the source of truth in case the NoteDb write fails. In
servers running in this mode, we want some way to fix up changes
automatically when we can detect they are out of date. Do this in
ChangeNotes immediately after opening the repo.

This implementation is slightly complicated for several reasons.
First, when auto-rebuilding stale changes, ChangeRebuilder needs to
construct ChangeNotes in order to write out its ChangeUpdates. To
prevent recursive auto-rebuilding, add a new ChangeNotes.Factory
method with an explicit flag to turn it off.

Second, injecting ChangeRebuilder into AbstractChangeNotes.Args
thickens the dependency stack considerably and requires some hacking
to make Guice work out. We need to bind ChangeRebuilder to an
unimplemented version for AbstractChangeNotesTest. And we need to put
a seam[1] in the dependency graph to work around a circular dependency
from ChangeRebuilder back to AbstractChangeNotes.Args.

[1] https://github.com/google/guice/wiki/CyclicDependencies#break-the-cycle-with-a-provider

Change-Id: I1fda0f158d268a28837c93ec10a8ef4877286f08

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java

@@ -25,21 +25,29 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.PatchLineCommentsUtil;
 import com.google.gerrit.server.change.Rebuild;
 import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.notedb.ChangeBundle;
 import com.google.gerrit.server.notedb.ChangeNoteUtil;
+import com.google.gerrit.server.notedb.NoteDbChangeState;
 import com.google.gerrit.server.schema.DisabledChangesReviewDbWrapper;
 import com.google.gerrit.testutil.NoteDbChecker;
 import com.google.gerrit.testutil.NoteDbMode;
+import com.google.gerrit.testutil.TestTimeUtil;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
+import java.util.Collections;
+import java.util.concurrent.TimeUnit;
+
 public class ChangeRebuilderIT extends AbstractDaemonTest {
   @Inject
   private AllUsersName allUsers;
@@ -53,12 +61,21 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
   @Inject
   private Provider<ReviewDb> dbProvider;
 
+  @Inject
+  private PatchLineCommentsUtil plcUtil;
+
   @Before
   public void setUp() {
     assume().that(NoteDbMode.readWrite()).isFalse();
+    TestTimeUtil.resetWithClockStep(1, TimeUnit.SECONDS);
     notesMigration.setAllEnabled(false);
   }
 
+  @After
+  public void tearDown() {
+    TestTimeUtil.useSystemTime();
+  }
+
   @Test
   public void changeFields() throws Exception {
     PushOneCommit.Result r = createChange();
@@ -188,6 +205,54 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
         + "," + user.getId() + "=" + userDraftsId.name());
   }
 
+  @Test
+  public void rebuildAutomaticallyWhenChangeOutOfDate() throws Exception {
+    notesMigration.setAllEnabled(true);
+
+    PushOneCommit.Result r = createChange();
+    Change.Id id = r.getPatchSetId().getParentKey();
+    assertUpToDate(true, id);
+
+    // Make a ReviewDb change behind NoteDb's back and ensure it's detected.
+    notesMigration.setAllEnabled(false);
+    gApi.changes().id(id.get()).topic(name("a-topic"));
+    setInvalidNoteDbState(id);
+    assertUpToDate(false, id);
+
+    // On next NoteDb read, the change is transparently rebuilt.
+    notesMigration.setAllEnabled(true);
+    assertThat(gApi.changes().id(id.get()).info().topic)
+        .isEqualTo(name("a-topic"));
+    assertUpToDate(true, id);
+
+    // Check that the bundles are equal.
+    ChangeBundle actual = ChangeBundle.fromNotes(
+        plcUtil, notesFactory.create(dbProvider.get(), project, id));
+    ChangeBundle expected = ChangeBundle.fromReviewDb(unwrapDb(), id);
+    assertThat(actual.differencesFrom(expected)).isEmpty();
+  }
+
+  private void setInvalidNoteDbState(Change.Id id) throws Exception {
+    ReviewDb db = unwrapDb();
+    Change c = db.changes().get(id);
+    // In reality we would have NoteDb writes enabled, which would write a real
+    // state into this field. For tests however, we turn NoteDb writes off, so
+    // just use a dummy state to force ChangeNotes to view the notes as
+    // out-of-date.
+    c.setNoteDbState("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef");
+    db.changes().update(Collections.singleton(c));
+  }
+
+  private void assertUpToDate(boolean expected, Change.Id id) throws Exception {
+    try (Repository repo = repoManager.openMetadataRepository(project)) {
+      Change c = unwrapDb().changes().get(id);
+      assertThat(c).isNotNull();
+      assertThat(c.getNoteDbState()).isNotNull();
+      assertThat(NoteDbChangeState.parse(c).isChangeUpToDate(repo))
+          .isEqualTo(expected);
+    }
+  }
+
   private ObjectId getMetaRef(Project.NameKey p, String name) throws Exception {
     try (Repository repo = repoManager.openMetadataRepository(p)) {
       Ref ref = repo.exactRef(name);

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

@@ -16,14 +16,18 @@ package com.google.gerrit.server.notedb;
 
 import static com.google.gerrit.server.notedb.NoteDbTable.CHANGES;
 
+import com.google.auto.value.AutoValue;
 import com.google.common.annotations.VisibleForTesting;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.metrics.Timer1;
 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.config.AllUsersName;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
 import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -44,6 +48,11 @@ public abstract class AbstractChangeNotes<T> {
     final AllUsersName allUsers;
     final ChangeNoteUtil noteUtil;
     final NoteDbMetrics metrics;
+    final Provider<ReviewDb> db;
+
+    // Must be a Provider due to dependency cycle between ChangeRebuilder and
+    // Args via ChangeNotes.Factory.
+    final Provider<ChangeRebuilder> rebuilder;
 
     @Inject
     Args(
@@ -51,12 +60,32 @@ public abstract class AbstractChangeNotes<T> {
         NotesMigration migration,
         AllUsersName allUsers,
         ChangeNoteUtil noteUtil,
-        NoteDbMetrics metrics) {
+        NoteDbMetrics metrics,
+        Provider<ReviewDb> db,
+        Provider<ChangeRebuilder> rebuilder) {
       this.repoManager = repoManager;
       this.migration = migration;
       this.allUsers = allUsers;
       this.noteUtil = noteUtil;
       this.metrics = metrics;
+      this.db = db;
+      this.rebuilder = rebuilder;
+    }
+  }
+
+  @AutoValue
+  public static abstract class LoadHandle implements AutoCloseable {
+    public static LoadHandle create(RevWalk walk, ObjectId id) {
+      return new AutoValue_AbstractChangeNotes_LoadHandle(
+          walk, id != null ? id.copy() : null);
+    }
+
+    public abstract RevWalk walk();
+    @Nullable public abstract ObjectId id();
+
+    @Override
+    public void close() {
+      walk().close();
     }
   }
 
@@ -84,18 +113,16 @@ public abstract class AbstractChangeNotes<T> {
     if (loaded) {
       return self();
     }
-    if (!args.migration.enabled() || changeId == null) {
+    if (!args.migration.readChanges() || changeId == null) {
       loadDefaults();
       return self();
     }
     try (Timer1.Context timer = args.metrics.readLatency.start(CHANGES);
         Repository repo =
             args.repoManager.openMetadataRepository(getProjectName());
-        RevWalk walk = new RevWalk(repo)) {
+        LoadHandle handle = openHandle(repo)) {
-      Ref ref = repo.getRefDatabase().exactRef(getRefName());
+      revision = handle.id();
-      ObjectId id = ref != null ? ref.getObjectId() : null;
+      onLoad(handle);
-      revision = id != null ? walk.parseCommit(id).copy() : null;
-      onLoad(walk);
       loaded = true;
     } catch (ConfigInvalidException | IOException e) {
       throw new OrmException(e);
@@ -103,6 +130,13 @@ public abstract class AbstractChangeNotes<T> {
     return self();
   }
 
+  protected LoadHandle openHandle(Repository repo) throws IOException {
+    Ref ref = repo.getRefDatabase().exactRef(getRefName());
+    return LoadHandle.create(
+        new RevWalk(repo),
+        ref != null ? ref.getObjectId() : null);
+  }
+
   public T reload() throws OrmException {
     loaded = false;
     return load();
@@ -136,7 +170,7 @@ public abstract class AbstractChangeNotes<T> {
   protected abstract String getRefName();
 
   /** Set up the metadata, parsing any state from the loaded revision. */
-  protected abstract void onLoad(RevWalk walk)
+  protected abstract void onLoad(LoadHandle handle)
       throws IOException, ConfigInvalidException;
 
   @SuppressWarnings("unchecked")

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

@@ -195,6 +195,11 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
       return new ChangeNotes(args, change.getProject(), change).load();
     }
 
+    public ChangeNotes createWithAutoRebuildingDisabled(Change change)
+        throws OrmException {
+      return new ChangeNotes(args, change.getProject(), change, false).load();
+    }
+
     // TODO(ekempin): Remove when database backend is deleted
     /**
      * Instantiate ChangeNotes for a change that has been loaded by a batch read
@@ -364,6 +369,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
 
   private final Project.NameKey project;
   private final Change change;
+  private final boolean autoRebuild;
   private ImmutableSortedMap<PatchSet.Id, PatchSet> patchSets;
   private ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals;
   private ImmutableSetMultimap<ReviewerStateInternal, Account.Id> reviewers;
@@ -382,9 +388,15 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
 
   @VisibleForTesting
   public ChangeNotes(Args args, Project.NameKey project, Change change) {
+    this(args, project, change, true);
+  }
+
+  private ChangeNotes(Args args, Project.NameKey project, Change change,
+      boolean autoRebuild) {
     super(args, change != null ? change.getId() : null);
     this.project = project;
     this.change = change != null ? new Change(change) : null;
+    this.autoRebuild = autoRebuild;
   }
 
   public Change getChange() {
@@ -518,16 +530,16 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
   }
 
   @Override
-  protected void onLoad(RevWalk walk)
+  protected void onLoad(LoadHandle handle)
       throws IOException, ConfigInvalidException {
-    ObjectId rev = getRevision();
+    ObjectId rev = handle.id();
     if (rev == null) {
       loadDefaults();
       return;
     }
     try (ChangeNotesParser parser = new ChangeNotesParser(
-         project, change.getId(), rev, walk, args.repoManager, args.noteUtil,
+         project, change.getId(), rev, handle.walk(), args.repoManager,
-         args.metrics)) {
+         args.noteUtil, args.metrics)) {
       parser.parseAll();
 
       if (parser.status != null) {
@@ -590,4 +602,31 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
   public Project.NameKey getProjectName() {
     return project;
   }
+
+  @Override
+  protected LoadHandle openHandle(Repository repo) throws IOException {
+    if (autoRebuild) {
+      NoteDbChangeState state = NoteDbChangeState.parse(change);
+      if (state == null || !state.isChangeUpToDate(repo)) {
+        return rebuildAndOpen(repo);
+      }
+    }
+    return super.openHandle(repo);
+  }
+
+  private LoadHandle rebuildAndOpen(Repository repo) throws IOException {
+    try {
+      NoteDbChangeState newState =
+          args.rebuilder.get().rebuild(args.db.get(), getChangeId());
+      if (newState == null) {
+        return super.openHandle(repo); // May be null in tests.
+      }
+      repo.scanForRepoChanges();
+      return LoadHandle.create(new RevWalk(repo), newState.getChangeMetaId());
+    } catch (NoSuchChangeException e) {
+      return super.openHandle(repo);
+    } catch (OrmException | ConfigInvalidException e) {
+      throw new IOException(e);
+    }
+  }
 }

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

@@ -34,20 +34,19 @@ public abstract class ChangeRebuilder {
     this.schemaFactory = schemaFactory;
   }
 
-  public final ListenableFuture<?> rebuildAsync(
+  public final ListenableFuture<NoteDbChangeState> rebuildAsync(
       final Change.Id id, ListeningExecutorService executor) {
-    return executor.submit(new Callable<Void>() {
+    return executor.submit(new Callable<NoteDbChangeState>() {
-      @Override
+        @Override
-      public Void call() throws Exception {
+      public NoteDbChangeState call() throws Exception {
         try (ReviewDb db = schemaFactory.open()) {
-          rebuild(db, id);
+          return rebuild(db, id);
         }
-        return null;
       }
     });
   }
 
-  public abstract void rebuild(ReviewDb db, Change.Id changeId)
+  public abstract NoteDbChangeState rebuild(ReviewDb db, Change.Id changeId)
       throws NoSuchChangeException, IOException, OrmException,
       ConfigInvalidException;
 }

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

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.notedb;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
 import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_HASHTAGS;
@@ -48,6 +49,7 @@ import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.patch.PatchListCache;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.schema.DisabledChangesReviewDbWrapper;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
@@ -95,6 +97,7 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
   private final IdentifiedUser.GenericFactory userFactory;
   private final InternalUser.Factory internalUserFactory;
   private final PatchListCache patchListCache;
+  private final ChangeNotes.Factory notesFactory;
   private final ChangeUpdate.Factory updateFactory;
   private final ChangeDraftUpdate.Factory draftUpdateFactory;
   private final NoteDbUpdateManager.Factory updateManagerFactory;
@@ -107,6 +110,7 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
       IdentifiedUser.GenericFactory userFactory,
       InternalUser.Factory internalUserFactory,
       PatchListCache patchListCache,
+      ChangeNotes.Factory notesFactory,
       ChangeUpdate.Factory updateFactory,
       ChangeDraftUpdate.Factory draftUpdateFactory,
       NoteDbUpdateManager.Factory updateManagerFactory,
@@ -117,6 +121,7 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
     this.userFactory = userFactory;
     this.internalUserFactory = internalUserFactory;
     this.patchListCache = patchListCache;
+    this.notesFactory = notesFactory;
     this.updateFactory = updateFactory;
     this.draftUpdateFactory = draftUpdateFactory;
     this.updateManagerFactory = updateManagerFactory;
@@ -124,12 +129,13 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
   }
 
   @Override
-  public void rebuild(ReviewDb db, Change.Id changeId)
+  public NoteDbChangeState rebuild(ReviewDb db, Change.Id changeId)
       throws NoSuchChangeException, IOException, OrmException,
       ConfigInvalidException {
+    db = unwrapDb(db);
     Change change = db.changes().get(changeId);
     if (change == null) {
-      return;
+      throw new NoSuchChangeException(changeId);
     }
     NoteDbUpdateManager manager =
         updateManagerFactory.create(change.getProject());
@@ -200,27 +206,31 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
       }
       flushEventsToDraftUpdate(db, manager, plcel, change);
     }
-    execute(db, changeId, manager);
+    return execute(db, changeId, manager);
   }
 
-  private void execute(ReviewDb db, Change.Id changeId,
+  private NoteDbChangeState execute(ReviewDb db, Change.Id changeId,
       NoteDbUpdateManager manager)
       throws NoSuchChangeException, OrmException, IOException  {
+    NoteDbChangeState result;
     db.changes().beginTransaction(changeId);
     try {
       Change change = db.changes().get(changeId);
       if (change == null) {
         throw new NoSuchChangeException(changeId);
       }
-      NoteDbChangeState.applyDelta(
+      result = NoteDbChangeState.applyDelta(
           change,
           manager.stage().get(changeId));
+      checkNotNull(result,
+          "expected new NoteDbChangeState when rebuilding change %s", changeId);
       db.changes().update(Collections.singleton(change));
       db.commit();
     } finally {
       db.rollback();
     }
     manager.execute();
+    return result;
   }
 
   private void flushEventsToUpdate(ReviewDb db, NoteDbUpdateManager manager,
@@ -230,7 +240,9 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
       return;
     }
     ChangeUpdate update = updateFactory.create(
-        controlFactory.controlFor(db, change, events.getUser(db)),
+        controlFactory.controlFor(
+            notesFactory.createWithAutoRebuildingDisabled(change),
+            events.getUser(db)),
         events.getWhen());
     update.setAllowWriteToNewRef(true);
     update.setPatchSetId(events.getPatchSetId());
@@ -248,7 +260,9 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
       return;
     }
     ChangeDraftUpdate update = draftUpdateFactory.create(
-        controlFactory.controlFor(db, change, events.getUser(db)),
+        controlFactory.controlFor(
+            notesFactory.createWithAutoRebuildingDisabled(change),
+            events.getUser(db)),
         events.getWhen());
     update.setPatchSetId(events.getPatchSetId());
     for (PatchLineCommentEvent e : events) {
@@ -708,4 +722,11 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
       }
     }
   }
+
+  private ReviewDb unwrapDb(ReviewDb db) {
+    if (db instanceof DisabledChangesReviewDbWrapper) {
+      db = ((DisabledChangesReviewDbWrapper) db).unsafeGetDelegate();
+    }
+    return db;
+  }
 }

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

@@ -32,7 +32,6 @@ import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.notes.NoteMap;
 import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevWalk;
 
 import java.io.IOException;
 
@@ -87,16 +86,16 @@ public class DraftCommentNotes extends AbstractChangeNotes<DraftCommentNotes> {
   }
 
   @Override
-  protected void onLoad(RevWalk walk)
+  protected void onLoad(LoadHandle handle)
       throws IOException, ConfigInvalidException {
-    ObjectId rev = getRevision();
+    ObjectId rev = handle.id();
     if (rev == null) {
       loadDefaults();
       return;
     }
 
-    RevCommit tipCommit = walk.parseCommit(rev);
+    RevCommit tipCommit = handle.walk().parseCommit(rev);
-    ObjectReader reader = walk.getObjectReader();
+    ObjectReader reader = handle.walk().getObjectReader();
     revisionNoteMap = RevisionNoteMap.parse(
         args.noteUtil, getChangeId(), reader, NoteMap.read(reader, tipCommit),
         true);

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

@@ -71,7 +71,7 @@ public class NoteDbChangeState {
     abstract ImmutableMap<Account.Id, ObjectId> newDraftIds();
   }
 
-  static NoteDbChangeState parse(Change c) {
+  public static NoteDbChangeState parse(Change c) {
     return parse(c.getId(), c.getNoteDbState());
   }
 
@@ -98,16 +98,16 @@ public class NoteDbChangeState {
     return new NoteDbChangeState(id, changeMetaId, draftIds);
   }
 
-  public static void applyDelta(Change change, Delta delta) {
+  public static NoteDbChangeState applyDelta(Change change, Delta delta) {
     if (delta == null) {
-      return;
+      return null;
     }
     String oldStr = change.getNoteDbState();
     if (oldStr == null && !delta.newChangeMetaId().isPresent()) {
       // Neither an old nor a new meta ID was present, most likely because we
       // aren't writing a NoteDb graph at all for this change at this point. No
       // point in proceeding.
-      return;
+      return null;
     }
     NoteDbChangeState oldState = parse(change.getId(), oldStr);
 
@@ -116,7 +116,7 @@ public class NoteDbChangeState {
       changeMetaId = delta.newChangeMetaId().get();
       if (changeMetaId.equals(ObjectId.zeroId())) {
         change.setNoteDbState(null);
-        return;
+        return null;
       }
     } else {
       changeMetaId = oldState.changeMetaId;
@@ -134,7 +134,10 @@ public class NoteDbChangeState {
       }
     }
 
-    change.setNoteDbState(toString(changeMetaId, draftIds));
+    NoteDbChangeState state = new NoteDbChangeState(
+        change.getId(), changeMetaId, draftIds);
+    change.setNoteDbState(state.toString());
+    return state;
   }
 
   private static String toString(ObjectId changeMetaId,
@@ -185,7 +188,6 @@ public class NoteDbChangeState {
     return changeId;
   }
 
-  @VisibleForTesting
   ObjectId getChangeMetaId() {
     return changeMetaId;
   }

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

@@ -17,6 +17,7 @@ package com.google.gerrit.server.notedb;
 import com.google.gerrit.extensions.config.FactoryModule;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gwtorm.server.OrmException;
 
 public class NoteDbModule extends FactoryModule {
   private final boolean useTestBindings;
@@ -44,8 +45,9 @@ public class NoteDbModule extends FactoryModule {
     } else {
       bind(ChangeRebuilder.class).toInstance(new ChangeRebuilder(null) {
         @Override
-        public void rebuild(ReviewDb db, Change.Id changeId) {
+        public NoteDbChangeState rebuild(ReviewDb db, Change.Id changeId)
-          throw new UnsupportedOperationException();
+            throws OrmException {
+          return null;
         }
       });
     }

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

@@ -31,6 +31,7 @@ import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.IdentifiedUser;
@@ -171,6 +172,7 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests {
         bind(StarredChangesUtil.class)
             .toProvider(Providers.<StarredChangesUtil> of(null));
         bind(MetricMaker.class).to(DisabledMetricMaker.class);
+        bind(ReviewDb.class).toProvider(Providers.<ReviewDb> of(null));
       }
     });