NoteDbChecker: Refactor to support checking without rebuilding

Change-Id: Idd6a154b25dc7e54bb8105ab8e75b4c36560422c

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

@@ -285,7 +285,8 @@ public class GerritServer {
   void stop() throws Exception {
     try {
       if (NoteDbMode.get().equals(NoteDbMode.CHECK)) {
-        testInjector.getInstance(NoteDbChecker.class).checkAllChanges();
+        testInjector.getInstance(NoteDbChecker.class)
+            .rebuildAndCheckAllChanges();
       }
     } finally {
       daemon.getLifecycleManager().stop();

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

@@ -41,7 +41,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
     PushOneCommit.Result r = createChange();
     Change.Id id = r.getPatchSetId().getParentKey();
     gApi.changes().id(id.get()).topic(name("a-topic"));
-    checker.checkChanges(id);
+    checker.rebuildAndCheckChanges(id);
   }
 
   @Test
@@ -49,6 +49,6 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
     PushOneCommit.Result r = createChange();
     Change.Id id = r.getPatchSetId().getParentKey();
     r = amendChange(r.getChangeId());
-    checker.checkChanges(id);
+    checker.rebuildAndCheckChanges(id);
   }
 }

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

@@ -59,63 +59,107 @@ public class NoteDbChecker {
     this.plcUtil = plcUtil;
   }
 
-  public void checkAllChanges() throws Exception {
+  public void rebuildAndCheckAllChanges() throws Exception {
-    checkChanges(
+    rebuildAndCheckChanges(
         Iterables.transform(
             unwrapDb().changes().all(),
             ReviewDbUtil.changeIdFunction()));
   }
 
+  public void rebuildAndCheckChanges(Change.Id... changeIds) throws Exception {
+    rebuildAndCheckChanges(Arrays.asList(changeIds));
+  }
+
+  public void rebuildAndCheckChanges(Iterable<Change.Id> changeIds)
+      throws Exception {
+    ReviewDb db = unwrapDb();
+
+    List<ChangeBundle> allExpected = readExpected(changeIds);
+
+    boolean oldWrite = notesMigration.writeChanges();
+    boolean oldRead = notesMigration.readChanges();
+    try {
+      notesMigration.setWriteChanges(true);
+      notesMigration.setReadChanges(true);
+      List<String> msgs = new ArrayList<>();
+      for (ChangeBundle expected : allExpected) {
+        Change c = expected.getChange();
+        try {
+          changeRebuilder.rebuild(db, c.getId());
+        } catch (RepositoryNotFoundException e) {
+          msgs.add("Repository not found for change, cannot convert: " + c);
+        }
+      }
+
+      checkActual(allExpected, msgs);
+    } finally {
+      notesMigration.setReadChanges(oldRead);
+      notesMigration.setWriteChanges(oldWrite);
+    }
+  }
+
   public void checkChanges(Change.Id... changeIds) throws Exception {
     checkChanges(Arrays.asList(changeIds));
   }
 
   public void checkChanges(Iterable<Change.Id> changeIds) throws Exception {
+    checkActual(readExpected(changeIds), new ArrayList<String>());
+  }
+
+  private List<ChangeBundle> readExpected(Iterable<Change.Id> changeIds)
+      throws Exception {
     ReviewDb db = unwrapDb();
+    boolean old = notesMigration.readChanges();
+    try {
+      notesMigration.setReadChanges(false);
+      List<Change.Id> sortedIds =
+          ReviewDbUtil.intKeyOrdering().sortedCopy(changeIds);
+      List<ChangeBundle> expected = new ArrayList<>(sortedIds.size());
+      for (Change.Id id : sortedIds) {
+        expected.add(ChangeBundle.fromReviewDb(db, id));
+      }
+      return expected;
+    } finally {
+      notesMigration.setReadChanges(old);
+    }
+  }
 
-    notesMigration.setReadChanges(false);
+  private void checkActual(List<ChangeBundle> allExpected, List<String> msgs)
-    List<Change.Id> sortedIds =
+      throws Exception {
-        ReviewDbUtil.intKeyOrdering().sortedCopy(changeIds);
+    ReviewDb db = unwrapDb();
-    List<ChangeBundle> allExpected = new ArrayList<>(sortedIds.size());
+    boolean oldRead = notesMigration.readChanges();
-    for (Change.Id id : sortedIds) {
+    boolean oldWrite = notesMigration.writeChanges();
-      allExpected.add(ChangeBundle.fromReviewDb(db, id));
+    try {
-    }
+      notesMigration.setWriteChanges(true);
-
+      notesMigration.setReadChanges(true);
-    notesMigration.setWriteChanges(true);
+      for (ChangeBundle expected : allExpected) {
-    notesMigration.setReadChanges(true);
+        Change c = expected.getChange();
-    List<String> all = new ArrayList<>();
+        ChangeBundle actual;
-    for (ChangeBundle expected : allExpected) {
+        try {
-      Change c = expected.getChange();
+          actual = ChangeBundle.fromNotes(
-      try {
+              plcUtil, notesFactory.create(db, c.getProject(), c.getId()));
-        changeRebuilder.rebuild(db, c.getId());
+        } catch (Throwable t) {
-      } catch (RepositoryNotFoundException e) {
+          String msg = "Error converting change: " + c;
-        all.add("Repository not found for change, cannot convert: " + c);
+          msgs.add(msg);
+          log.error(msg, t);
+          continue;
+        }
+        List<String> diff = expected.differencesFrom(actual);
+        if (!diff.isEmpty()) {
+          msgs.add("Differences between ReviewDb and NoteDb for " + c + ":");
+          msgs.addAll(diff);
+          msgs.add("");
+        } else {
+          System.err.println(
+              "NoteDb conversion of change " + c.getId() + " successful");
+        }
       }
+    } finally {
+      notesMigration.setReadChanges(oldRead);
+      notesMigration.setWriteChanges(oldWrite);
     }
-    for (ChangeBundle expected : allExpected) {
+    if (!msgs.isEmpty()) {
-      Change c = expected.getChange();
+      throw new AssertionError(Joiner.on('\n').join(msgs));
-      ChangeBundle actual;
-      try {
-        actual = ChangeBundle.fromNotes(
-            plcUtil, notesFactory.create(db, c.getProject(), c.getId()));
-      } catch (Throwable t) {
-        String msg = "Error converting change: " + c;
-        all.add(msg);
-        log.error(msg, t);
-        continue;
-      }
-      List<String> diff = expected.differencesFrom(actual);
-      if (!diff.isEmpty()) {
-        all.add("Differences between ReviewDb and NoteDb for " + c + ":");
-        all.addAll(diff);
-        all.add("");
-      } else {
-        System.err.println(
-            "NoteDb conversion of change " + c.getId() + " successful");
-      }
-    }
-    if (!all.isEmpty()) {
-      throw new AssertionError(Joiner.on('\n').join(all));
     }
   }