diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index b407ec4cbf..7985b94fbb 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java @@ -95,6 +95,7 @@ import com.google.gerrit.server.mail.Address; import com.google.gerrit.server.mail.send.EmailHeader; 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.project.ChangeControl; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.Util; @@ -104,9 +105,9 @@ import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.testutil.ConfigSuite; import com.google.gerrit.testutil.FakeEmailSender; import com.google.gerrit.testutil.FakeEmailSender.Message; +import com.google.gerrit.testutil.NoteDbMode; import com.google.gerrit.testutil.SshMode; import com.google.gerrit.testutil.TempFileUtil; -import com.google.gerrit.testutil.TestNotesMigration; import com.google.gson.Gson; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.SchemaFactory; @@ -215,7 +216,7 @@ public abstract class AbstractDaemonTest { @Inject protected PluginConfigFactory pluginConfig; @Inject protected Revisions revisions; @Inject protected SystemGroupBackend systemGroupBackend; - @Inject protected TestNotesMigration notesMigration; + @Inject protected MutableNotesMigration notesMigration; @Inject protected ChangeNotes.Factory notesFactory; @Inject protected Abandon changeAbandoner; @@ -498,7 +499,7 @@ public abstract class AbstractDaemonTest { server.close(); server = null; } - notesMigration.resetFromEnv(); + NoteDbMode.resetFromEnv(notesMigration); } protected TestRepository.CommitBuilder commitBuilder() throws Exception { @@ -722,12 +723,12 @@ public abstract class AbstractDaemonTest { } protected Context disableDb() { - notesMigration.setFailOnLoad(true); + notesMigration.setFailOnLoadForTest(true); return atrScope.disableDb(); } protected void enableDb(Context preDisableContext) { - notesMigration.setFailOnLoad(false); + notesMigration.setFailOnLoadForTest(false); atrScope.set(preDisableContext); } diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java index d810f12869..926c3babf9 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java @@ -374,6 +374,8 @@ 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 { diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java index 551c26bb11..0f30fa2ddb 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java @@ -38,7 +38,6 @@ import com.google.gerrit.server.schema.SchemaVersion; import com.google.gerrit.testutil.InMemoryDatabase; import com.google.gerrit.testutil.InMemoryH2Type; import com.google.gerrit.testutil.InMemoryRepositoryManager; -import com.google.gerrit.testutil.TestNotesMigration; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmRuntimeException; import com.google.gwtorm.server.SchemaFactory; @@ -78,7 +77,7 @@ class InMemoryTestingDatabaseModule extends LifecycleModule { bind(MetricMaker.class).to(DisabledMetricMaker.class); bind(DataSourceType.class).to(InMemoryH2Type.class); - bind(NotesMigration.class).to(TestNotesMigration.class); + install(new NotesMigration.Module()); TypeLiteral> schemaFactory = new TypeLiteral>() {}; bind(schemaFactory).to(NotesMigrationSchemaFactory.class); diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java index 7ab686832b..03dcbca888 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java @@ -31,10 +31,10 @@ import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.notedb.ReviewerStateInternal; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.InternalChangeQuery; -import com.google.gerrit.testutil.TestNotesMigration; import com.google.gwtorm.server.OrmException; import com.google.inject.Provider; import com.google.inject.assistedinject.Assisted; @@ -144,7 +144,7 @@ public class PushOneCommit { private final ChangeNotes.Factory notesFactory; private final ApprovalsUtil approvalsUtil; private final Provider queryProvider; - private final TestNotesMigration notesMigration; + private final NotesMigration notesMigration; private final ReviewDb db; private final TestRepository testRepo; @@ -162,7 +162,7 @@ public class PushOneCommit { ChangeNotes.Factory notesFactory, ApprovalsUtil approvalsUtil, Provider queryProvider, - TestNotesMigration notesMigration, + NotesMigration notesMigration, @Assisted ReviewDb db, @Assisted PersonIdent i, @Assisted TestRepository testRepo) @@ -185,7 +185,7 @@ public class PushOneCommit { ChangeNotes.Factory notesFactory, ApprovalsUtil approvalsUtil, Provider queryProvider, - TestNotesMigration notesMigration, + NotesMigration notesMigration, @Assisted ReviewDb db, @Assisted PersonIdent i, @Assisted TestRepository testRepo, @@ -210,7 +210,7 @@ public class PushOneCommit { ChangeNotes.Factory notesFactory, ApprovalsUtil approvalsUtil, Provider queryProvider, - TestNotesMigration notesMigration, + NotesMigration notesMigration, @Assisted ReviewDb db, @Assisted PersonIdent i, @Assisted TestRepository testRepo, @@ -237,7 +237,7 @@ public class PushOneCommit { ChangeNotes.Factory notesFactory, ApprovalsUtil approvalsUtil, Provider queryProvider, - TestNotesMigration notesMigration, + NotesMigration notesMigration, @Assisted ReviewDb db, @Assisted PersonIdent i, @Assisted TestRepository testRepo, @@ -262,7 +262,7 @@ public class PushOneCommit { ChangeNotes.Factory notesFactory, ApprovalsUtil approvalsUtil, Provider queryProvider, - TestNotesMigration notesMigration, + NotesMigration notesMigration, @Assisted ReviewDb db, @Assisted PersonIdent i, @Assisted TestRepository testRepo, @@ -288,7 +288,7 @@ public class PushOneCommit { ChangeNotes.Factory notesFactory, ApprovalsUtil approvalsUtil, Provider queryProvider, - TestNotesMigration notesMigration, + NotesMigration notesMigration, ReviewDb db, PersonIdent i, TestRepository testRepo, diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/StandaloneNoteDbMigrationIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/StandaloneNoteDbMigrationIT.java index e7803db8c2..29a9a1f438 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/StandaloneNoteDbMigrationIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/pgm/StandaloneNoteDbMigrationIT.java @@ -32,7 +32,6 @@ import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.index.GerritIndexStatus; import com.google.gerrit.server.index.change.ChangeIndexCollection; import com.google.gerrit.server.index.change.ChangeSchemaDefinitions; -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; @@ -201,8 +200,7 @@ public class StandaloneNoteDbMigrationIT extends StandaloneSiteTest { private void assertNotesMigrationState(NotesMigrationState expected) throws Exception { gerritConfig.load(); - assertThat(NotesMigrationState.forNotesMigration(new ConfigNotesMigration(gerritConfig))) - .hasValue(expected); + assertThat(NotesMigrationState.forConfig(gerritConfig)).hasValue(expected); } private ReviewDb openUnderlyingReviewDb(ServerContext ctx) throws Exception { diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java index 0b004ec6e3..e8903c27a5 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java @@ -325,7 +325,7 @@ public class ChangeReviewersByEmailIT extends AbstractDaemonTest { input.state = state; gApi.changes().id(r.getChangeId()).addReviewer(input); - notesMigration.setFailOnLoad(true); + notesMigration.setFailOnLoadForTest(true); try { ChangeInfo info = Iterables.getOnlyElement( @@ -335,7 +335,7 @@ public class ChangeReviewersByEmailIT extends AbstractDaemonTest { .get()); assertThat(info.reviewers).isEqualTo(ImmutableMap.of(state, ImmutableList.of(acc))); } finally { - notesMigration.setFailOnLoad(false); + notesMigration.setFailOnLoadForTest(false); } } } 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 29b8ee7293..485bb1b5a3 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 @@ -38,7 +38,6 @@ import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.Sequences; 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; @@ -380,15 +379,14 @@ public class OnlineNoteDbMigrationIT extends AbstractDaemonTest { private void assertNotesMigrationState(NotesMigrationState expected) throws Exception { assertThat(NotesMigrationState.forNotesMigration(notesMigration)).hasValue(expected); gerritConfig.load(); - assertThat(NotesMigrationState.forNotesMigration(new ConfigNotesMigration(gerritConfig))) - .hasValue(expected); + assertThat(NotesMigrationState.forConfig(gerritConfig)).hasValue(expected); } private void setNotesMigrationState(NotesMigrationState state) throws Exception { gerritConfig.load(); - ConfigNotesMigration.setConfigValues(gerritConfig, state.migration()); + state.setConfigValues(gerritConfig); gerritConfig.save(); - notesMigration.setFrom(state.migration()); + notesMigration.setFrom(state); } @FunctionalInterface diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java index 669b610925..001c9880d5 100644 --- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java +++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java @@ -46,7 +46,7 @@ import com.google.gerrit.server.util.RequestContext; import com.google.gerrit.server.util.ThreadLocalRequestContext; import com.google.gerrit.testutil.InMemoryDatabase; import com.google.gerrit.testutil.InMemoryModule; -import com.google.gerrit.testutil.TestNotesMigration; +import com.google.gerrit.testutil.NoteDbMode; import com.google.inject.Guice; import com.google.inject.Inject; import com.google.inject.Injector; @@ -104,7 +104,8 @@ public class GerritPublicKeyCheckerTest { ImmutableList.of( Fingerprint.toString(keyB().getPublicKey().getFingerprint()), Fingerprint.toString(keyD().getPublicKey().getFingerprint()))); - Injector injector = Guice.createInjector(new InMemoryModule(cfg, new TestNotesMigration())); + Injector injector = + Guice.createInjector(new InMemoryModule(cfg, NoteDbMode.newNotesMigrationFromEnv())); lifecycle = new LifecycleManager(); lifecycle.add(injector); diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitExperimental.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitExperimental.java index 769cdb4ccf..c83f4e49f2 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitExperimental.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitExperimental.java @@ -14,15 +14,15 @@ package com.google.gerrit.pgm.init; -import static com.google.gerrit.server.notedb.ConfigNotesMigration.SECTION_NOTE_DB; import static com.google.gerrit.server.notedb.NoteDbTable.CHANGES; +import static com.google.gerrit.server.notedb.NotesMigration.SECTION_NOTE_DB; import com.google.gerrit.extensions.client.UiType; import com.google.gerrit.pgm.init.api.ConsoleUI; import com.google.gerrit.pgm.init.api.InitFlags; import com.google.gerrit.pgm.init.api.InitStep; import com.google.gerrit.pgm.init.api.Section; -import com.google.gerrit.server.notedb.ConfigNotesMigration; +import com.google.gerrit.server.notedb.NotesMigrationState; import com.google.inject.Inject; import java.util.Locale; import javax.inject.Singleton; @@ -66,7 +66,8 @@ class InitExperimental implements InitStep { return; } - Config defaultConfig = ConfigNotesMigration.allEnabledConfig(); + Config defaultConfig = new Config(); + NotesMigrationState.FINAL.setConfigValues(defaultConfig); for (String name : defaultConfig.getNames(SECTION_NOTE_DB, CHANGES.key())) { noteDbChanges.set(name, defaultConfig.getString(SECTION_NOTE_DB, CHANGES.key(), name)); } diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteProgram.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteProgram.java index 3b8da24ccc..b59e085858 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteProgram.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/SiteProgram.java @@ -28,7 +28,7 @@ import com.google.gerrit.server.config.GerritServerConfig; 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.ConfigNotesMigration; +import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.schema.DataSourceModule; import com.google.gerrit.server.schema.DataSourceProvider; import com.google.gerrit.server.schema.DataSourceType; @@ -182,7 +182,7 @@ public abstract class SiteProgram extends AbstractProgram { modules.add(new DatabaseModule()); modules.add(new SchemaModule()); modules.add(cfgInjector.getInstance(GitRepositoryManagerModule.class)); - modules.add(new ConfigNotesMigration.Module()); + modules.add(new NotesMigration.Module()); try { return Guice.createInjector(PRODUCTION, modules); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java index 91a93b0108..c16c284667 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java @@ -151,7 +151,7 @@ public abstract class AbstractChangeNotes { loadDefaults(); return self(); } - if (args.migration.failOnLoad()) { + if (args.migration.failOnLoadForTest()) { throw new OrmException("Reading from NoteDb is disabled"); } try (Timer1.Context timer = args.metrics.readLatency.start(CHANGES); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ConfigNotesMigration.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ConfigNotesMigration.java deleted file mode 100644 index b5ba8503ea..0000000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ConfigNotesMigration.java +++ /dev/null @@ -1,178 +0,0 @@ -// Copyright (C) 2013 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 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.config.GerritServerConfig; -import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage; -import com.google.inject.AbstractModule; -import com.google.inject.Inject; -import com.google.inject.Singleton; -import org.eclipse.jgit.lib.Config; - -/** - * Implement NoteDb migration stages using {@code gerrit.config}. - * - *

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. Changing options quite likely requires re-running {@code MigrateToNoteDb}. For - * these reasons, the options remain undocumented. - */ -@Singleton -public class ConfigNotesMigration extends NotesMigration { - public static class Module extends AbstractModule { - @Override - public void configure() { - bind(NotesMigration.class).to(ConfigNotesMigration.class); - } - } - - public static final String SECTION_NOTE_DB = "noteDb"; - - private static final String DISABLE_REVIEW_DB = "disableReviewDb"; - private static final String FUSE_UPDATES = "fuseUpdates"; - private static final String PRIMARY_STORAGE = "primaryStorage"; - private static final String READ = "read"; - private static final String SEQUENCE = "sequence"; - private static final String WRITE = "write"; - - public static Config allEnabledConfig() { - Config cfg = new Config(); - cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), WRITE, true); - cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), READ, true); - cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), SEQUENCE, true); - cfg.setString(SECTION_NOTE_DB, CHANGES.key(), PRIMARY_STORAGE, PrimaryStorage.NOTE_DB.name()); - cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), DISABLE_REVIEW_DB, true); - // TODO(dborowitz): Set to true when FileRepository supports it. - cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), FUSE_UPDATES, false); - return cfg; - } - - public static void setConfigValues(Config cfg, NotesMigration migration) { - cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), WRITE, migration.rawWriteChangesSetting()); - cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), READ, migration.readChanges()); - cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), SEQUENCE, migration.readChangeSequence()); - cfg.setEnum(SECTION_NOTE_DB, CHANGES.key(), PRIMARY_STORAGE, migration.changePrimaryStorage()); - cfg.setBoolean( - SECTION_NOTE_DB, CHANGES.key(), DISABLE_REVIEW_DB, migration.disableChangeReviewDb()); - cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), FUSE_UPDATES, migration.fuseUpdates()); - } - - public static String toText(NotesMigration migration) { - Config cfg = new Config(); - setConfigValues(cfg, migration); - return cfg.toText(); - } - - @AutoValue - abstract static class Snapshot { - static Snapshot create(Config cfg) { - boolean writeChanges = cfg.getBoolean(SECTION_NOTE_DB, CHANGES.key(), WRITE, false); - boolean readChanges = cfg.getBoolean(SECTION_NOTE_DB, CHANGES.key(), READ, false); - boolean readChangeSequence = cfg.getBoolean(SECTION_NOTE_DB, CHANGES.key(), SEQUENCE, false); - PrimaryStorage changePrimaryStorage = - cfg.getEnum(SECTION_NOTE_DB, CHANGES.key(), PRIMARY_STORAGE, PrimaryStorage.REVIEW_DB); - boolean disableChangeReviewDb = - cfg.getBoolean(SECTION_NOTE_DB, CHANGES.key(), DISABLE_REVIEW_DB, false); - boolean fuseUpdates = cfg.getBoolean(SECTION_NOTE_DB, CHANGES.key(), FUSE_UPDATES, false); - - checkArgument( - !(disableChangeReviewDb && changePrimaryStorage != PrimaryStorage.NOTE_DB), - "cannot disable ReviewDb for changes if default change primary storage is ReviewDb"); - - return new AutoValue_ConfigNotesMigration_Snapshot( - writeChanges, - readChanges, - readChangeSequence, - changePrimaryStorage, - disableChangeReviewDb, - fuseUpdates); - } - - abstract boolean writeChanges(); - - abstract boolean readChanges(); - - abstract boolean readChangeSequence(); - - abstract PrimaryStorage changePrimaryStorage(); - - abstract boolean disableChangeReviewDb(); - - abstract boolean fuseUpdates(); - } - - private volatile Snapshot snapshot; - - @Inject - public ConfigNotesMigration(@GerritServerConfig Config cfg) { - this.snapshot = Snapshot.create(cfg); - } - - @Override - public boolean rawWriteChangesSetting() { - return snapshot.writeChanges(); - } - - @Override - public boolean readChanges() { - return snapshot.readChanges(); - } - - @Override - public boolean readChangeSequence() { - return snapshot.readChangeSequence(); - } - - @Override - public PrimaryStorage changePrimaryStorage() { - return snapshot.changePrimaryStorage(); - } - - @Override - public boolean disableChangeReviewDb() { - return snapshot.disableChangeReviewDb(); - } - - @Override - public boolean fuseUpdates() { - return snapshot.fuseUpdates(); - } - - /** - * Set the in-memory values returned by this instance to match another instance. - * - *

This method is only intended for use by {@link - * com.google.gerrit.server.notedb.rebuild.NoteDbMigrator}. - * - *

This only 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. This method is synchronized to aid in such - * implementations. - * - * @see NotesMigration#setFrom(NotesMigration) - */ - @Override - public synchronized ConfigNotesMigration setFrom(NotesMigration other) { - Config cfg = new Config(); - setConfigValues(cfg, other); - snapshot = Snapshot.create(cfg); - return this; - } -} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/MutableNotesMigration.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/MutableNotesMigration.java new file mode 100644 index 0000000000..e16763536e --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/MutableNotesMigration.java @@ -0,0 +1,108 @@ +// 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.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. + * + *

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}. + * + *

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. + * + *

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)); + } + + public MutableNotesMigration setFuseUpdates(boolean fuseUpdates) { + return set(b -> b.setFuseUpdates(fuseUpdates)); + } + + public MutableNotesMigration setFailOnLoadForTest(boolean failOnLoadForTest) { + return set(b -> b.setFailOnLoadForTest(failOnLoadForTest)); + } + + /** + * Set the in-memory values returned by this instance to match the given state. + * + *

This method is only intended for use by {@link + * com.google.gerrit.server.notedb.rebuild.NoteDbMigrator}. + * + *

This only 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. + */ + public MutableNotesMigration setFrom(NotesMigrationState state) { + snapshot.set(state.snapshot()); + return this; + } + + /** @see #setFrom(NotesMigrationState) */ + public MutableNotesMigration setFrom(NotesMigration other) { + snapshot.set(other.snapshot.get()); + return this; + } + + private MutableNotesMigration set(Function f) { + snapshot.updateAndGet(s -> f.apply(s.toBuilder()).build()); + return this; + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java index 61bcf17a23..efd4e9e7f9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java @@ -1,4 +1,4 @@ -// Copyright (C) 2016 The Android Open Source Project +// 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. @@ -14,8 +14,14 @@ 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 java.util.Objects; +import com.google.inject.AbstractModule; +import java.util.concurrent.atomic.AtomicReference; +import org.eclipse.jgit.lib.Config; /** * Current low-level settings of the NoteDb migration for changes. @@ -47,6 +53,100 @@ import java.util.Objects; * NotesMigration}'s methods will not change in a running server. */ public abstract class NotesMigration { + public static final String SECTION_NOTE_DB = "noteDb"; + + private static final String DISABLE_REVIEW_DB = "disableReviewDb"; + private static final String FUSE_UPDATES = "fuseUpdates"; + private static final String PRIMARY_STORAGE = "primaryStorage"; + private static final String READ = "read"; + private static final String SEQUENCE = "sequence"; + private static final String WRITE = "write"; + + 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)) + .setFuseUpdates(cfg.getBoolean(SECTION_NOTE_DB, CHANGES.key(), FUSE_UPDATES, false)) + .setFailOnLoadForTest(false) // Only set in tests, can't be set via config. + .build(); + } + + abstract boolean writeChanges(); + + abstract boolean readChanges(); + + abstract boolean readChangeSequence(); + + abstract PrimaryStorage changePrimaryStorage(); + + abstract boolean disableChangeReviewDb(); + + abstract boolean fuseUpdates(); + + abstract boolean failOnLoadForTest(); + + 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()); + cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), FUSE_UPDATES, fuseUpdates()); + } + + @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 Builder setFuseUpdates(boolean fuseUpdates); + + abstract Builder setFailOnLoadForTest(boolean failOnLoadForTest); + + 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; + /** * Read changes from NoteDb. * @@ -57,7 +157,9 @@ public abstract class NotesMigration { *

If true and {@code writeChanges() = false}, changes can still be read from NoteDb, but any * attempts to write will generate an error. */ - public abstract boolean readChanges(); + public final boolean readChanges() { + return snapshot.get().readChanges(); + } /** * Write changes to NoteDb. @@ -73,7 +175,9 @@ public abstract class NotesMigration { * readChanges() = false}, writes to NoteDb are simply ignored; if {@code true}, any attempts to * write will generate an error. */ - public abstract boolean rawWriteChangesSetting(); + public final boolean rawWriteChangesSetting() { + return snapshot.get().writeChanges(); + } /** * Read sequential change ID numbers from NoteDb. @@ -81,10 +185,14 @@ public abstract class NotesMigration { *

If true, change IDs are read from {@code refs/sequences/changes} in All-Projects. If false, * change IDs are read from ReviewDb's native sequences. */ - public abstract boolean readChangeSequence(); + public final boolean readChangeSequence() { + return snapshot.get().readChangeSequence(); + } /** @return default primary storage for new changes. */ - public abstract PrimaryStorage changePrimaryStorage(); + public final PrimaryStorage changePrimaryStorage() { + return snapshot.get().changePrimaryStorage(); + } /** * Disable ReviewDb access for changes. @@ -93,7 +201,9 @@ public abstract class NotesMigration { * results; updates do nothing, as does opening, committing, or rolling back a transaction on the * Changes table. */ - public abstract boolean disableChangeReviewDb(); + public final boolean disableChangeReviewDb() { + return snapshot.get().disableChangeReviewDb(); + } /** * Fuse meta ref updates in the same batch as code updates. @@ -106,18 +216,8 @@ public abstract class NotesMigration { * *

Has no effect if {@link #disableChangeReviewDb()} is false. */ - public abstract boolean fuseUpdates(); - - /** - * Set the values returned by this instance to match another instance. - * - *

Optional operation: not all implementations support setting values after initialization. - * - * @param other other instance to copy values from. - * @return this. - */ - public NotesMigration setFrom(NotesMigration other) { - throw new UnsupportedOperationException(getClass().getSimpleName() + " is read-only"); + public final boolean fuseUpdates() { + return snapshot.get().fuseUpdates(); } /** @@ -125,8 +225,8 @@ public abstract class NotesMigration { * *

Used in conjunction with {@link #readChanges()} for tests. */ - public boolean failOnLoad() { - return false; + public boolean failOnLoadForTest() { + return snapshot.get().failOnLoadForTest(); } public final boolean commitChangeWrites() { @@ -151,30 +251,26 @@ public abstract class NotesMigration { return rawWriteChangesSetting() || readChanges(); } + public final void setConfigValues(Config cfg) { + snapshot.get().setConfigValues(cfg); + } + @Override public final boolean equals(Object o) { - if (!(o instanceof NotesMigration)) { - return false; - } - NotesMigration m = (NotesMigration) o; - return readChanges() == m.readChanges() - && rawWriteChangesSetting() == m.rawWriteChangesSetting() - && readChangeSequence() == m.readChangeSequence() - && changePrimaryStorage() == m.changePrimaryStorage() - && disableChangeReviewDb() == m.disableChangeReviewDb() - && fuseUpdates() == m.fuseUpdates() - && failOnLoad() == m.failOnLoad(); + return o instanceof NotesMigration + && snapshot.get().equals(((NotesMigration) o).snapshot.get()); } @Override public final int hashCode() { - return Objects.hash( - readChanges(), - rawWriteChangesSetting(), - readChangeSequence(), - changePrimaryStorage(), - disableChangeReviewDb(), - fuseUpdates(), - failOnLoad()); + return snapshot.get().hashCode(); + } + + protected NotesMigration(Snapshot snapshot) { + this.snapshot = new AtomicReference<>(snapshot); + } + + final Snapshot snapshot() { + return snapshot.get(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigrationState.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigrationState.java index 8c589d845d..2469574db8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigrationState.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigrationState.java @@ -15,8 +15,10 @@ 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. @@ -47,11 +49,22 @@ public enum NotesMigrationState { NOTE_DB(true, true, true, PrimaryStorage.NOTE_DB, true, true); - public static Optional forNotesMigration(NotesMigration migration) { - return Stream.of(values()).filter(s -> s.migration().equals(migration)).findFirst(); + // TODO(dborowitz): Replace with NOTE_DB when FileRepository fuses BatchRefUpdates. + public static final NotesMigrationState FINAL = NOTE_DB_UNFUSED; + + public static Optional forConfig(Config cfg) { + return forSnapshot(Snapshot.create(cfg)); } - private final NotesMigration migration; + public static Optional forNotesMigration(NotesMigration migration) { + return forSnapshot(migration.snapshot()); + } + + private static Optional 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. @@ -61,45 +74,28 @@ public enum NotesMigrationState { PrimaryStorage changePrimaryStorage, boolean disableChangeReviewDb, boolean fuseUpdates) { - this.migration = - new NotesMigration() { - @Override - public boolean readChanges() { - return readChanges; - } - - @Override - public boolean rawWriteChangesSetting() { - return rawWriteChangesSetting; - } - - @Override - public boolean readChangeSequence() { - return readChangeSequence; - } - - @Override - public PrimaryStorage changePrimaryStorage() { - return changePrimaryStorage; - } - - @Override - public boolean disableChangeReviewDb() { - return disableChangeReviewDb; - } - - @Override - public boolean fuseUpdates() { - return fuseUpdates; - } - }; + this.snapshot = + Snapshot.builder() + .setReadChanges(readChanges) + .setWriteChanges(rawWriteChangesSetting) + .setReadChangeSequence(readChangeSequence) + .setChangePrimaryStorage(changePrimaryStorage) + .setDisableChangeReviewDb(disableChangeReviewDb) + .setFuseUpdates(fuseUpdates) + .build(); } - public NotesMigration migration() { - return migration; + public void setConfigValues(Config cfg) { + snapshot.setConfigValues(cfg); } public String toText() { - return ConfigNotesMigration.toText(migration); + Config cfg = new Config(); + setConfigValues(cfg); + return cfg.toText(); + } + + Snapshot snapshot() { + return snapshot; } } 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 9e4e646df5..0a7629481c 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 @@ -18,7 +18,7 @@ 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.ConfigNotesMigration.SECTION_NOTE_DB; +import static com.google.gerrit.server.notedb.NotesMigration.SECTION_NOTE_DB; import static com.google.gerrit.server.notedb.NotesMigrationState.NOTE_DB_UNFUSED; import static com.google.gerrit.server.notedb.NotesMigrationState.READ_WRITE_NO_SEQUENCE; import static com.google.gerrit.server.notedb.NotesMigrationState.READ_WRITE_WITH_SEQUENCE_NOTE_DB_PRIMARY; @@ -54,9 +54,8 @@ import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.LockFailureException; import com.google.gerrit.server.git.WorkQueue; -import com.google.gerrit.server.notedb.ConfigNotesMigration; +import com.google.gerrit.server.notedb.MutableNotesMigration; import com.google.gerrit.server.notedb.NoteDbTable; -import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.notedb.NotesMigrationState; import com.google.gerrit.server.notedb.PrimaryStorageMigrator; import com.google.gerrit.server.notedb.RepoSequence; @@ -114,7 +113,7 @@ public class NoteDbMigrator implements AutoCloseable { private final ThreadLocalRequestContext requestContext; private final ChangeRebuilder rebuilder; private final WorkQueue workQueue; - private final NotesMigration globalNotesMigration; + private final MutableNotesMigration globalNotesMigration; private final PrimaryStorageMigrator primaryStorageMigrator; private int threads; @@ -138,7 +137,7 @@ public class NoteDbMigrator implements AutoCloseable { InternalUser.Factory userFactory, ChangeRebuilder rebuilder, WorkQueue workQueue, - NotesMigration globalNotesMigration, + MutableNotesMigration globalNotesMigration, PrimaryStorageMigrator primaryStorageMigrator) { this.cfg = cfg; this.sitePaths = sitePaths; @@ -327,7 +326,7 @@ public class NoteDbMigrator implements AutoCloseable { private final ThreadLocalRequestContext requestContext; private final InternalUser.Factory userFactory; private final ChangeRebuilder rebuilder; - private final NotesMigration globalNotesMigration; + private final MutableNotesMigration globalNotesMigration; private final PrimaryStorageMigrator primaryStorageMigrator; private final ListeningExecutorService executor; @@ -348,7 +347,7 @@ public class NoteDbMigrator implements AutoCloseable { ThreadLocalRequestContext requestContext, InternalUser.Factory userFactory, ChangeRebuilder rebuilder, - NotesMigration globalNotesMigration, + MutableNotesMigration globalNotesMigration, PrimaryStorageMigrator primaryStorageMigrator, ListeningExecutorService executor, ImmutableList projects, @@ -568,7 +567,7 @@ public class NoteDbMigrator implements AutoCloseable { private Optional loadState() throws IOException { try { gerritConfig.load(); - return NotesMigrationState.forNotesMigration(new ConfigNotesMigration(gerritConfig)); + return NotesMigrationState.forConfig(gerritConfig); } catch (ConfigInvalidException | IllegalArgumentException e) { log.warn("error reading NoteDb migration options from " + gerritConfig.getFile(), e); return Optional.empty(); @@ -601,12 +600,12 @@ public class NoteDbMigrator implements AutoCloseable { ? "But found this state:\n" + actualOldState.get().toText() : "But could not parse the current state")); } - ConfigNotesMigration.setConfigValues(gerritConfig, newState.migration()); + newState.setConfigValues(gerritConfig); additionalUpdates.accept(gerritConfig); gerritConfig.save(); // Only set in-memory state once it's been persisted to storage. - globalNotesMigration.setFrom(newState.migration()); + globalNotesMigration.setFrom(newState); return newState; } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java index f59063add8..f03fb37f86 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java @@ -56,7 +56,6 @@ import com.google.gerrit.testutil.FakeAccountCache; import com.google.gerrit.testutil.GerritBaseTests; import com.google.gerrit.testutil.InMemoryRepositoryManager; import com.google.gerrit.testutil.TestChanges; -import com.google.gerrit.testutil.TestNotesMigration; import com.google.gerrit.testutil.TestTimeUtil; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.SchemaFactory; @@ -98,8 +97,6 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests { private static final TimeZone TZ = TimeZone.getTimeZone("America/Los_Angeles"); - private static final NotesMigration MIGRATION = new TestNotesMigration().setAllEnabled(true); - protected Account.Id otherUserId; protected FakeAccountCache accountCache; protected IdentifiedUser changeOwner; @@ -154,7 +151,6 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests { install(NoteDbModule.forTest(testConfig)); bind(AllUsersName.class).toProvider(AllUsersNameProvider.class); bind(String.class).annotatedWith(GerritServerId.class).toInstance("gerrit"); - bind(NotesMigration.class).toInstance(MIGRATION); bind(GitRepositoryManager.class).toInstance(repoManager); bind(ProjectCache.class).toProvider(Providers.of(null)); bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(testConfig); @@ -177,6 +173,11 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests { bind(MetricMaker.class).to(DisabledMetricMaker.class); bind(ReviewDb.class).toProvider(Providers.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>() {}) .toInstance( diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaUpdaterTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaUpdaterTest.java index 74eb1d2a59..d9fc7e5a8b 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaUpdaterTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaUpdaterTest.java @@ -30,7 +30,7 @@ import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.group.SystemGroupBackend; -import com.google.gerrit.server.notedb.ConfigNotesMigration; +import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.testutil.InMemoryDatabase; import com.google.gerrit.testutil.InMemoryH2Type; import com.google.gerrit.testutil.InMemoryRepositoryManager; @@ -113,7 +113,7 @@ public class SchemaUpdaterTest { bind(DataSourceType.class).to(InMemoryH2Type.class); bind(SystemGroupBackend.class); - install(new ConfigNotesMigration.Module()); + install(new NotesMigration.Module()); } }) .getInstance(SchemaUpdater.class); diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/GerritServerTests.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/GerritServerTests.java index 8f37c20a98..b84b8ed24b 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/GerritServerTests.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/GerritServerTests.java @@ -14,6 +14,7 @@ package com.google.gerrit.testutil; +import com.google.gerrit.server.notedb.MutableNotesMigration; import org.eclipse.jgit.lib.Config; import org.junit.Rule; import org.junit.rules.TestRule; @@ -27,7 +28,7 @@ public class GerritServerTests extends GerritBaseTests { @ConfigSuite.Name private String configName; - protected TestNotesMigration notesMigration; + protected MutableNotesMigration notesMigration; @Rule public TestRule testRunner = @@ -49,10 +50,10 @@ public class GerritServerTests extends GerritBaseTests { }; public void beforeTest() throws Exception { - notesMigration = new TestNotesMigration(); + notesMigration = NoteDbMode.newNotesMigrationFromEnv(); } public void afterTest() { - notesMigration.resetFromEnv(); + NoteDbMode.resetFromEnv(notesMigration); } } diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java index 365ddab756..12c76ebc9c 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java @@ -59,6 +59,7 @@ import com.google.gerrit.server.index.group.GroupSchemaDefinitions; import com.google.gerrit.server.mail.SignedTokenEmailTokenVerifier; import com.google.gerrit.server.notedb.ChangeBundleReader; import com.google.gerrit.server.notedb.GwtormChangeBundleReader; +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.project.DefaultPermissionBackendModule; @@ -118,13 +119,13 @@ public class InMemoryModule extends FactoryModule { } private final Config cfg; - private final TestNotesMigration notesMigration; + private final MutableNotesMigration notesMigration; public InMemoryModule() { - this(newDefaultConfig(), new TestNotesMigration()); + this(newDefaultConfig(), NoteDbMode.newNotesMigrationFromEnv()); } - public InMemoryModule(Config cfg, TestNotesMigration notesMigration) { + public InMemoryModule(Config cfg, MutableNotesMigration notesMigration) { this.cfg = cfg; this.notesMigration = notesMigration; } @@ -176,7 +177,8 @@ 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(NotesMigration.class).toInstance(notesMigration); + bind(MutableNotesMigration.class).toInstance(notesMigration); + bind(NotesMigration.class).to(MutableNotesMigration.class); bind(ListeningExecutorService.class) .annotatedWith(ChangeUpdateExecutor.class) .toInstance(MoreExecutors.newDirectExecutorService()); diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryRepositoryManager.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryRepositoryManager.java index a90cbb97ea..0e7201a6d7 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryRepositoryManager.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryRepositoryManager.java @@ -32,7 +32,7 @@ import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; /** Repository manager that uses in-memory repositories. */ public class InMemoryRepositoryManager implements GitRepositoryManager { public static InMemoryRepository newRepository(Project.NameKey name) { - return new Repo(new TestNotesMigration(), name); + return new Repo(NoteDbMode.newNotesMigrationFromEnv(), name); } public static class Description extends DfsRepositoryDescription { @@ -82,7 +82,7 @@ public class InMemoryRepositoryManager implements GitRepositoryManager { private final Map repos; public InMemoryRepositoryManager() { - this(new TestNotesMigration()); + this(NoteDbMode.newNotesMigrationFromEnv()); } @Inject diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbChecker.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbChecker.java index ad876ce351..b5edb2516f 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbChecker.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbChecker.java @@ -29,6 +29,7 @@ import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.notedb.ChangeBundle; import com.google.gerrit.server.notedb.ChangeBundleReader; import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gerrit.server.notedb.MutableNotesMigration; import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder; import com.google.gwtorm.client.IntKey; import com.google.gwtorm.server.OrmException; @@ -52,7 +53,7 @@ public class NoteDbChecker { private final Provider dbProvider; private final GitRepositoryManager repoManager; - private final TestNotesMigration notesMigration; + private final MutableNotesMigration notesMigration; private final ChangeBundleReader bundleReader; private final ChangeNotes.Factory notesFactory; private final ChangeRebuilder changeRebuilder; @@ -62,7 +63,7 @@ public class NoteDbChecker { NoteDbChecker( Provider dbProvider, GitRepositoryManager repoManager, - TestNotesMigration notesMigration, + MutableNotesMigration notesMigration, ChangeBundleReader bundleReader, ChangeNotes.Factory notesFactory, ChangeRebuilder changeRebuilder, diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbMode.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbMode.java index 2bde775147..7c19191094 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbMode.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbMode.java @@ -18,7 +18,7 @@ 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.NotesMigration; +import com.google.gerrit.server.notedb.MutableNotesMigration; import com.google.gerrit.server.notedb.NotesMigrationState; public enum NoteDbMode { @@ -72,9 +72,19 @@ public enum NoteDbMode { return mode; } - final NotesMigration migration; + 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) { - migration = state.migration(); + this.state = state; } } diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestNotesMigration.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestNotesMigration.java deleted file mode 100644 index a89135898b..0000000000 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestNotesMigration.java +++ /dev/null @@ -1,128 +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.testutil; - -import static com.google.common.base.Preconditions.checkNotNull; - -import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage; -import com.google.gerrit.server.notedb.NotesMigration; -import com.google.inject.Singleton; - -/** {@link NotesMigration} with bits that can be flipped live for testing. */ -@Singleton -public class TestNotesMigration extends NotesMigration { - private volatile boolean readChanges; - private volatile boolean writeChanges; - private volatile boolean readChangeSequence; - private volatile PrimaryStorage changePrimaryStorage = PrimaryStorage.REVIEW_DB; - private volatile boolean disableChangeReviewDb; - private volatile boolean fuseUpdates; - private volatile boolean failOnLoad; - - public TestNotesMigration() { - resetFromEnv(); - } - - @Override - public boolean readChanges() { - return readChanges; - } - - @Override - public boolean readChangeSequence() { - return readChangeSequence; - } - - @Override - public PrimaryStorage changePrimaryStorage() { - return changePrimaryStorage; - } - - @Override - public boolean disableChangeReviewDb() { - return disableChangeReviewDb; - } - - @Override - public boolean fuseUpdates() { - return fuseUpdates; - } - - // Increase visbility from superclass, as tests may want to check whether - // NoteDb data is written in specific migration scenarios. - @Override - public boolean rawWriteChangesSetting() { - return writeChanges; - } - - @Override - public boolean failOnLoad() { - return failOnLoad; - } - - public TestNotesMigration setReadChanges(boolean readChanges) { - this.readChanges = readChanges; - return this; - } - - public TestNotesMigration setWriteChanges(boolean writeChanges) { - this.writeChanges = writeChanges; - return this; - } - - public TestNotesMigration setReadChangeSequence(boolean readChangeSequence) { - this.readChangeSequence = readChangeSequence; - return this; - } - - public TestNotesMigration setChangePrimaryStorage(PrimaryStorage changePrimaryStorage) { - this.changePrimaryStorage = checkNotNull(changePrimaryStorage); - return this; - } - - public TestNotesMigration setDisableChangeReviewDb(boolean disableChangeReviewDb) { - this.disableChangeReviewDb = disableChangeReviewDb; - return this; - } - - public TestNotesMigration setFuseUpdates(boolean fuseUpdates) { - this.fuseUpdates = fuseUpdates; - return this; - } - - public TestNotesMigration setFailOnLoad(boolean failOnLoad) { - this.failOnLoad = failOnLoad; - return this; - } - - public TestNotesMigration setAllEnabled(boolean enabled) { - return setReadChanges(enabled).setWriteChanges(enabled); - } - - public TestNotesMigration resetFromEnv() { - return setFrom(NoteDbMode.get().migration); - } - - @Override - public TestNotesMigration setFrom(NotesMigration other) { - setWriteChanges(other.rawWriteChangesSetting()); - setReadChanges(other.readChanges()); - setReadChangeSequence(other.readChangeSequence()); - setChangePrimaryStorage(other.changePrimaryStorage()); - setDisableChangeReviewDb(other.disableChangeReviewDb()); - setFuseUpdates(other.fuseUpdates()); - return this; - } -} diff --git a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java index 7d003a17fe..cb8860cce7 100644 --- a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java +++ b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java @@ -58,7 +58,7 @@ 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.ConfigNotesMigration; +import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.patch.DiffExecutorModule; import com.google.gerrit.server.plugins.PluginGuiceEnvironment; import com.google.gerrit.server.plugins.PluginModule; @@ -292,7 +292,7 @@ public class WebAppInitializer extends GuiceServletContextListener implements Fi modules.add(new GerritServerConfigModule()); } modules.add(new DatabaseModule()); - modules.add(new ConfigNotesMigration.Module()); + modules.add(new NotesMigration.Module()); modules.add(new DropWizardMetricMaker.ApiModule()); return Guice.createInjector(PRODUCTION, modules); }