OnlineNoteDbMIgrationIT: Tests for subsets, failed preconditions
Prefer MigrationException to unchecked exceptions, for a more
consistent interface, and to make tests slightly easier to write.
Change-Id: I2b2b7ff2b15c5b528ad1ab799cf41e137453f982
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
index f7331b8..181967d 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
@@ -18,40 +18,89 @@
import static com.google.common.truth.Truth8.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.Sandboxed;
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.ReviewDbUtil;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.notedb.ConfigNotesMigration;
import com.google.gerrit.server.notedb.NoteDbChangeState;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.notedb.NoteDbChangeState.RefState;
import com.google.gerrit.server.notedb.NotesMigrationState;
+import com.google.gerrit.server.notedb.rebuild.MigrationException;
import com.google.gerrit.server.notedb.rebuild.NoteDbMigrator;
+import com.google.gerrit.server.schema.ReviewDbFactory;
import com.google.gerrit.testutil.NoteDbMode;
+import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import com.google.inject.Provider;
+import java.util.List;
+import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.util.FS;
import org.junit.Before;
import org.junit.Test;
@Sandboxed
@NoHttpd
public class OnlineNoteDbMigrationIT extends AbstractDaemonTest {
+ // Tests in this class are generally interested in the actual ReviewDb contents, but the shifting
+ // migration state may result in various kinds of wrappers showing up unexpectedly.
+ @Inject @ReviewDbFactory private SchemaFactory<ReviewDb> schemaFactory;
+
+ @Inject private SitePaths sitePaths;
@Inject private Provider<NoteDbMigrator.Builder> migratorBuilderProvider;
+ private FileBasedConfig gerritConfig;
+
@Before
- public void setUp() {
+ public void setUp() throws Exception {
assume().that(NoteDbMode.get()).isEqualTo(NoteDbMode.OFF);
+ gerritConfig = new FileBasedConfig(sitePaths.gerrit_config.toFile(), FS.detect());
assertNotesMigrationState(NotesMigrationState.REVIEW_DB);
}
@Test
+ public void preconditionsFail() throws Exception {
+ List<Change.Id> cs = ImmutableList.of(new Change.Id(1));
+ List<Project.NameKey> ps = ImmutableList.of(new Project.NameKey("p"));
+ assertMigrationException(
+ "Cannot rebuild without noteDb.changes.write=true", b -> b, NoteDbMigrator::rebuild);
+ assertMigrationException(
+ "Cannot set both changes and projects", b -> b.setChanges(cs).setProjects(ps), m -> {});
+ assertMigrationException(
+ "Cannot set changes or projects during auto-migration",
+ b -> b.setChanges(cs),
+ NoteDbMigrator::migrate);
+ assertMigrationException(
+ "Cannot set changes or projects during auto-migration",
+ b -> b.setProjects(ps),
+ NoteDbMigrator::migrate);
+
+ setNotesMigrationState(NotesMigrationState.READ_WRITE_WITH_SEQUENCE_REVIEW_DB_PRIMARY);
+ assertMigrationException(
+ "Migration has already progressed past the endpoint of the \"trial mode\" state",
+ b -> b.setTrialMode(true),
+ NoteDbMigrator::migrate);
+
+ setNotesMigrationState(NotesMigrationState.READ_WRITE_WITH_SEQUENCE_NOTE_DB_PRIMARY);
+ assertMigrationException(
+ "Cannot force rebuild changes; NoteDb is already the primary storage for some changes",
+ b -> b.setForceRebuild(true),
+ NoteDbMigrator::migrate);
+ }
+
+ @Test
public void rebuildOneChangeTrialMode() throws Exception {
PushOneCommit.Result r = createChange();
Change.Id id = r.getChange().getId();
@@ -68,15 +117,115 @@
metaId = ref.getObjectId();
}
- Change c = ReviewDbUtil.unwrapDb(db).changes().get(id);
- assertThat(c).isNotNull();
- NoteDbChangeState state = NoteDbChangeState.parse(c);
- assertThat(state).isNotNull();
- assertThat(state.getPrimaryStorage()).isEqualTo(PrimaryStorage.REVIEW_DB);
- assertThat(state.getRefState()).hasValue(RefState.create(metaId, ImmutableMap.of()));
+ try (ReviewDb db = schemaFactory.open()) {
+ Change c = db.changes().get(id);
+ assertThat(c).isNotNull();
+ NoteDbChangeState state = NoteDbChangeState.parse(c);
+ assertThat(state).isNotNull();
+ assertThat(state.getPrimaryStorage()).isEqualTo(PrimaryStorage.REVIEW_DB);
+ assertThat(state.getRefState()).hasValue(RefState.create(metaId, ImmutableMap.of()));
+ }
}
- private void assertNotesMigrationState(NotesMigrationState expected) {
+ @Test
+ public void rebuildSubsetOfChanges() throws Exception {
+ setNotesMigrationState(NotesMigrationState.WRITE);
+
+ PushOneCommit.Result r1 = createChange();
+ PushOneCommit.Result r2 = createChange();
+ Change.Id id1 = r1.getChange().getId();
+ Change.Id id2 = r2.getChange().getId();
+
+ String invalidState = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
+ try (ReviewDb db = schemaFactory.open()) {
+ Change c1 = db.changes().get(id1);
+ c1.setNoteDbState(invalidState);
+ Change c2 = db.changes().get(id2);
+ c2.setNoteDbState(invalidState);
+ db.changes().update(ImmutableList.of(c1, c2));
+ }
+
+ try (NoteDbMigrator migrator =
+ migratorBuilderProvider.get().setChanges(ImmutableList.of(id2)).build()) {
+ migrator.rebuild();
+ }
+
+ try (ReviewDb db = schemaFactory.open()) {
+ NoteDbChangeState s1 = NoteDbChangeState.parse(db.changes().get(id1));
+ assertThat(s1.getChangeMetaId().name()).isEqualTo(invalidState);
+
+ NoteDbChangeState s2 = NoteDbChangeState.parse(db.changes().get(id2));
+ assertThat(s2.getChangeMetaId().name()).isNotEqualTo(invalidState);
+ }
+ }
+
+ @Test
+ public void rebuildSubsetOfProjects() throws Exception {
+ setNotesMigrationState(NotesMigrationState.WRITE);
+
+ Project.NameKey p2 = createProject("project2");
+ TestRepository<?> tr2 = cloneProject(p2, admin);
+
+ PushOneCommit.Result r1 = createChange();
+ PushOneCommit.Result r2 = pushFactory.create(db, admin.getIdent(), tr2).to("refs/for/master");
+ Change.Id id1 = r1.getChange().getId();
+ Change.Id id2 = r2.getChange().getId();
+
+ String invalidState = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
+ try (ReviewDb db = schemaFactory.open()) {
+ Change c1 = db.changes().get(id1);
+ c1.setNoteDbState(invalidState);
+ Change c2 = db.changes().get(id2);
+ c2.setNoteDbState(invalidState);
+ db.changes().update(ImmutableList.of(c1, c2));
+ }
+
+ try (NoteDbMigrator migrator =
+ migratorBuilderProvider.get().setProjects(ImmutableList.of(p2)).build()) {
+ migrator.rebuild();
+ }
+
+ try (ReviewDb db = schemaFactory.open()) {
+ NoteDbChangeState s1 = NoteDbChangeState.parse(db.changes().get(id1));
+ assertThat(s1.getChangeMetaId().name()).isEqualTo(invalidState);
+
+ NoteDbChangeState s2 = NoteDbChangeState.parse(db.changes().get(id2));
+ assertThat(s2.getChangeMetaId().name()).isNotEqualTo(invalidState);
+ }
+ }
+
+ private void assertNotesMigrationState(NotesMigrationState expected) throws Exception {
assertThat(NotesMigrationState.forNotesMigration(notesMigration)).hasValue(expected);
+ gerritConfig.load();
+ assertThat(NotesMigrationState.forNotesMigration(new ConfigNotesMigration(gerritConfig)))
+ .hasValue(expected);
+ }
+
+ private void setNotesMigrationState(NotesMigrationState state) throws Exception {
+ gerritConfig.load();
+ ConfigNotesMigration.setConfigValues(gerritConfig, state.migration());
+ gerritConfig.save();
+ notesMigration.setFrom(state.migration());
+ }
+
+ @FunctionalInterface
+ interface PrepareBuilder {
+ NoteDbMigrator.Builder prepare(NoteDbMigrator.Builder b) throws Exception;
+ }
+
+ @FunctionalInterface
+ interface RunMigration {
+ void run(NoteDbMigrator m) throws Exception;
+ }
+
+ private void assertMigrationException(
+ String expectMessageContains, PrepareBuilder b, RunMigration m) throws Exception {
+ try {
+ try (NoteDbMigrator migrator = b.prepare(migratorBuilderProvider.get()).build()) {
+ m.run(migrator);
+ }
+ } catch (MigrationException e) {
+ assertThat(e).hasMessageThat().contains(expectMessageContains);
+ }
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/MigrationException.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/MigrationException.java
index 209fd7a..0587b80 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/MigrationException.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/MigrationException.java
@@ -17,7 +17,7 @@
import java.io.IOException;
/** Exception thrown by {@link NoteDbMigrator} when migration fails. */
-class MigrationException extends IOException {
+public class MigrationException extends IOException {
private static final long serialVersionUID = 1L;
MigrationException(String message) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
index 8d43e0b..92ddd42 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
@@ -16,7 +16,6 @@
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.reviewdb.server.ReviewDbUtil.unwrapDb;
import static com.google.gerrit.server.notedb.NotesMigrationState.NOTE_DB_UNFUSED;
import static com.google.gerrit.server.notedb.NotesMigrationState.READ_WRITE_NO_SEQUENCE;
@@ -197,7 +196,7 @@
return this;
}
- public NoteDbMigrator build() {
+ public NoteDbMigrator build() throws MigrationException {
return new NoteDbMigrator(
sitePaths,
schemaFactory,
@@ -236,17 +235,16 @@
ImmutableList<Change.Id> changes,
OutputStream progressOut,
boolean trial,
- boolean forceRebuild) {
+ boolean forceRebuild)
+ throws MigrationException {
+ if (!changes.isEmpty() && !projects.isEmpty()) {
+ throw new MigrationException("Cannot set both changes and projects");
+ }
+
this.schemaFactory = schemaFactory;
this.rebuilder = rebuilder;
this.globalNotesMigration = globalNotesMigration;
-
- boolean hasChanges = !changes.isEmpty();
- boolean hasProjects = !projects.isEmpty();
- checkArgument(!(hasChanges && hasProjects), "cannot set both changes and projects");
-
this.gerritConfig = new FileBasedConfig(sitePaths.gerrit_config.toFile(), FS.detect());
-
this.executor = executor;
this.projects = projects;
this.changes = changes;
@@ -261,9 +259,10 @@
}
public void migrate() throws OrmException, IOException {
- checkState(
- changes.isEmpty() && projects.isEmpty(),
- "cannot set changes or projects during auto-migration; call rebuild() instead");
+ if (!changes.isEmpty() || !projects.isEmpty()) {
+ throw new MigrationException(
+ "Cannot set changes or projects during auto-migration; call rebuild() instead");
+ }
Optional<NotesMigrationState> maybeState = loadState();
if (!maybeState.isPresent()) {
throw new MigrationException("Could not determine initial migration state");
@@ -384,9 +383,9 @@
}
public void rebuild() throws MigrationException, OrmException {
- checkState(
- globalNotesMigration.commitChangeWrites(),
- "cannot rebuild without noteDb.changes.write=true");
+ if (!globalNotesMigration.commitChangeWrites()) {
+ throw new MigrationException("Cannot rebuild without noteDb.changes.write=true");
+ }
boolean ok;
Stopwatch sw = Stopwatch.createStarted();
log.info("Rebuilding changes in NoteDb");
