diff --git a/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java b/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java index 3f2cbda0a2..4b1211b828 100644 --- a/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java +++ b/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java @@ -28,6 +28,7 @@ import com.google.gerrit.server.config.TrackingFooters; import com.google.gerrit.server.config.TrackingFootersProvider; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.notedb.ChangeBundleReader; +import com.google.gerrit.server.notedb.GroupsMigration; import com.google.gerrit.server.notedb.GwtormChangeBundleReader; import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.schema.DataSourceType; @@ -78,6 +79,7 @@ class InMemoryTestingDatabaseModule extends LifecycleModule { bind(DataSourceType.class).to(InMemoryH2Type.class); install(new NotesMigration.Module()); + install(new GroupsMigration.Module()); TypeLiteral> schemaFactory = new TypeLiteral>() {}; bind(schemaFactory).to(NotesMigrationSchemaFactory.class); diff --git a/java/com/google/gerrit/httpd/init/WebAppInitializer.java b/java/com/google/gerrit/httpd/init/WebAppInitializer.java index b7ec467098..b5995a8c51 100644 --- a/java/com/google/gerrit/httpd/init/WebAppInitializer.java +++ b/java/com/google/gerrit/httpd/init/WebAppInitializer.java @@ -71,6 +71,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.GroupsMigration; import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.patch.DiffExecutorModule; import com.google.gerrit.server.plugins.PluginGuiceEnvironment; @@ -307,6 +308,7 @@ public class WebAppInitializer extends GuiceServletContextListener implements Fi } modules.add(new DatabaseModule()); modules.add(new NotesMigration.Module()); + modules.add(new GroupsMigration.Module()); modules.add(new DropWizardMetricMaker.ApiModule()); return Guice.createInjector(PRODUCTION, modules); } diff --git a/java/com/google/gerrit/pgm/init/GroupsOnInit.java b/java/com/google/gerrit/pgm/init/GroupsOnInit.java index acc98ce0a0..338524429a 100644 --- a/java/com/google/gerrit/pgm/init/GroupsOnInit.java +++ b/java/com/google/gerrit/pgm/init/GroupsOnInit.java @@ -15,10 +15,6 @@ package com.google.gerrit.pgm.init; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.gerrit.server.notedb.NoteDbTable.GROUPS; -import static com.google.gerrit.server.notedb.NotesMigration.READ; -import static com.google.gerrit.server.notedb.NotesMigration.SECTION_NOTE_DB; -import static com.google.gerrit.server.notedb.NotesMigration.WRITE; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -46,6 +42,7 @@ import com.google.gerrit.server.group.db.GroupConfig; import com.google.gerrit.server.group.db.GroupNameNotes; import com.google.gerrit.server.group.db.Groups; import com.google.gerrit.server.group.db.InternalGroupUpdate; +import com.google.gerrit.server.notedb.GroupsMigration; import com.google.gwtorm.server.OrmDuplicateKeyException; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -76,20 +73,14 @@ public class GroupsOnInit { private final InitFlags flags; private final SitePaths site; private final String allUsers; - private final boolean readFromNoteDb; - private final boolean writeGroupsToNoteDb; + private final GroupsMigration groupsMigration; @Inject public GroupsOnInit(InitFlags flags, SitePaths site, AllUsersNameOnInitProvider allUsers) { this.flags = flags; this.site = site; this.allUsers = allUsers.get(); - readFromNoteDb = flags.cfg.getBoolean(SECTION_NOTE_DB, GROUPS.key(), READ, false); - // TODO(aliceks): Remove this flag when all other necessary TODOs for writing groups to NoteDb - // have been addressed. - // Don't flip this flag in a production setting! We only added it to spread the implementation - // of groups in NoteDb among several changes which are gradually merged. - writeGroupsToNoteDb = flags.cfg.getBoolean(SECTION_NOTE_DB, GROUPS.key(), WRITE, false); + this.groupsMigration = new GroupsMigration(flags.cfg); } /** @@ -105,7 +96,7 @@ public class GroupsOnInit { */ public InternalGroup getExistingGroup(ReviewDb db, GroupReference groupReference) throws OrmException, NoSuchGroupException, IOException, ConfigInvalidException { - if (readFromNoteDb) { + if (groupsMigration.readFromNoteDb()) { return getExistingGroupFromNoteDb(groupReference); } @@ -155,7 +146,7 @@ public class GroupsOnInit { */ public Stream getAllGroupReferences(ReviewDb db) throws OrmException, IOException, ConfigInvalidException { - if (readFromNoteDb) { + if (groupsMigration.readFromNoteDb()) { File allUsersRepoPath = getPathToAllUsersRepository(); if (allUsersRepoPath != null) { try (Repository allUsersRepo = new FileRepository(allUsersRepoPath)) { @@ -185,7 +176,7 @@ public class GroupsOnInit { public void addGroupMember(ReviewDb db, AccountGroup.UUID groupUuid, Account account) throws OrmException, NoSuchGroupException, IOException, ConfigInvalidException { addGroupMemberInReviewDb(db, groupUuid, account.getId()); - if (!writeGroupsToNoteDb) { + if (!groupsMigration.writeToNoteDb()) { return; } addGroupMemberInNoteDb(groupUuid, account); diff --git a/java/com/google/gerrit/pgm/util/SiteProgram.java b/java/com/google/gerrit/pgm/util/SiteProgram.java index b59e085858..afabcf6628 100644 --- a/java/com/google/gerrit/pgm/util/SiteProgram.java +++ b/java/com/google/gerrit/pgm/util/SiteProgram.java @@ -28,6 +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.GroupsMigration; import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.schema.DataSourceModule; import com.google.gerrit.server.schema.DataSourceProvider; @@ -183,6 +184,7 @@ public abstract class SiteProgram extends AbstractProgram { modules.add(new SchemaModule()); modules.add(cfgInjector.getInstance(GitRepositoryManagerModule.class)); modules.add(new NotesMigration.Module()); + modules.add(new GroupsMigration.Module()); try { return Guice.createInjector(PRODUCTION, modules); diff --git a/java/com/google/gerrit/server/group/db/Groups.java b/java/com/google/gerrit/server/group/db/Groups.java index ea8f376726..e1bd4abefc 100644 --- a/java/com/google/gerrit/server/group/db/Groups.java +++ b/java/com/google/gerrit/server/group/db/Groups.java @@ -15,9 +15,6 @@ package com.google.gerrit.server.group.db; import static com.google.common.collect.ImmutableSet.toImmutableSet; -import static com.google.gerrit.server.notedb.NoteDbTable.GROUPS; -import static com.google.gerrit.server.notedb.NotesMigration.READ; -import static com.google.gerrit.server.notedb.NotesMigration.SECTION_NOTE_DB; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -33,9 +30,9 @@ import com.google.gerrit.reviewdb.client.AccountGroupMember; import com.google.gerrit.reviewdb.client.AccountGroupMemberAudit; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.config.AllUsersName; -import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.group.InternalGroup; +import com.google.gerrit.server.notedb.GroupsMigration; import com.google.gwtorm.server.OrmDuplicateKeyException; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.ResultSet; @@ -46,7 +43,6 @@ import java.util.List; import java.util.Optional; import java.util.stream.Stream; import org.eclipse.jgit.errors.ConfigInvalidException; -import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Repository; /** @@ -64,16 +60,16 @@ import org.eclipse.jgit.lib.Repository; */ @Singleton public class Groups { - private final boolean readFromNoteDb; + private final GroupsMigration groupsMigration; private final GitRepositoryManager repoManager; private final AllUsersName allUsersName; @Inject public Groups( - @GerritServerConfig Config config, + GroupsMigration groupsMigration, GitRepositoryManager repoManager, AllUsersName allUsersName) { - readFromNoteDb = config.getBoolean(SECTION_NOTE_DB, GROUPS.key(), READ, false); + this.groupsMigration = groupsMigration; this.repoManager = repoManager; this.allUsersName = allUsersName; } @@ -108,7 +104,7 @@ public class Groups { */ public Optional getGroup(ReviewDb db, AccountGroup.UUID groupUuid) throws OrmException, IOException, ConfigInvalidException { - if (readFromNoteDb) { + if (groupsMigration.readFromNoteDb()) { try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) { return getGroupFromNoteDb(allUsersRepo, groupUuid); } @@ -185,7 +181,7 @@ public class Groups { */ public Stream getAllGroupReferences(ReviewDb db) throws OrmException, IOException, ConfigInvalidException { - if (readFromNoteDb) { + if (groupsMigration.readFromNoteDb()) { try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) { return GroupNameNotes.loadAllGroupReferences(allUsersRepo).stream(); } @@ -281,7 +277,7 @@ public class Groups { */ public Stream getExternalGroups(ReviewDb db) throws OrmException, IOException, ConfigInvalidException { - if (readFromNoteDb) { + if (groupsMigration.readFromNoteDb()) { try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) { return getExternalGroupsFromNoteDb(allUsersRepo); } @@ -318,7 +314,7 @@ public class Groups { */ public List getMembersAudit(ReviewDb db, AccountGroup.UUID groupUuid) throws OrmException { - if (readFromNoteDb) { + if (groupsMigration.readFromNoteDb()) { // TODO(dborowitz): Implement. throw new OrmException("Audit logs not yet implemented in NoteDb"); } @@ -339,7 +335,7 @@ public class Groups { */ public List getSubgroupsAudit(ReviewDb db, AccountGroup.UUID groupUuid) throws OrmException { - if (readFromNoteDb) { + if (groupsMigration.readFromNoteDb()) { // TODO(dborowitz): Implement. throw new OrmException("Audit logs not yet implemented in NoteDb"); } diff --git a/java/com/google/gerrit/server/group/db/GroupsUpdate.java b/java/com/google/gerrit/server/group/db/GroupsUpdate.java index 62ba1c9919..1e4af412c3 100644 --- a/java/com/google/gerrit/server/group/db/GroupsUpdate.java +++ b/java/com/google/gerrit/server/group/db/GroupsUpdate.java @@ -16,9 +16,6 @@ package com.google.gerrit.server.group.db; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.gerrit.server.group.db.Groups.getExistingGroupFromReviewDb; -import static com.google.gerrit.server.notedb.NoteDbTable.GROUPS; -import static com.google.gerrit.server.notedb.NotesMigration.SECTION_NOTE_DB; -import static com.google.gerrit.server.notedb.NotesMigration.WRITE; import com.google.auto.value.AutoValue; import com.google.common.annotations.VisibleForTesting; @@ -51,6 +48,7 @@ import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.RenameGroupOp; import com.google.gerrit.server.group.InternalGroup; +import com.google.gerrit.server.notedb.GroupsMigration; import com.google.gerrit.server.update.RefUpdateUtil; import com.google.gwtorm.server.OrmDuplicateKeyException; import com.google.gwtorm.server.OrmException; @@ -107,8 +105,8 @@ public class GroupsUpdate { @Nullable private final IdentifiedUser currentUser; private final PersonIdent authorIdent; private final MetaDataUpdateFactory metaDataUpdateFactory; + private final GroupsMigration groupsMigration; private final GitReferenceUpdated gitRefUpdated; - private final boolean writeGroupsToNoteDb; private final boolean reviewDbUpdatesAreBlocked; @Inject @@ -124,6 +122,7 @@ public class GroupsUpdate { @GerritServerId String serverId, @GerritPersonIdent PersonIdent serverIdent, MetaDataUpdate.InternalFactory metaDataUpdateInternalFactory, + GroupsMigration groupsMigration, @GerritServerConfig Config config, GitReferenceUpdated gitRefUpdated, @Assisted @Nullable IdentifiedUser currentUser) { @@ -136,18 +135,13 @@ public class GroupsUpdate { this.anonymousCowardName = anonymousCowardName; this.renameGroupOpFactory = renameGroupOpFactory; this.serverId = serverId; + this.groupsMigration = groupsMigration; this.gitRefUpdated = gitRefUpdated; this.currentUser = currentUser; metaDataUpdateFactory = getMetaDataUpdateFactory( metaDataUpdateInternalFactory, currentUser, serverIdent, serverId, anonymousCowardName); authorIdent = getAuthorIdent(serverIdent, currentUser); - // TODO(aliceks): Remove this flag when all other necessary TODOs for writing groups to NoteDb - // have been addressed. - // Don't flip this flag in a production setting! We only added it to spread the implementation - // of groups in NoteDb among several changes which are gradually merged. - writeGroupsToNoteDb = config.getBoolean(SECTION_NOTE_DB, GROUPS.key(), WRITE, false); - reviewDbUpdatesAreBlocked = config.getBoolean("user", null, "blockReviewDbGroupUpdates", false); } @@ -212,7 +206,7 @@ public class GroupsUpdate { throws OrmException, IOException, ConfigInvalidException { InternalGroup createdGroupInReviewDb = createGroupInReviewDb(db, groupCreation, groupUpdate); - if (!writeGroupsToNoteDb) { + if (!groupsMigration.writeToNoteDb()) { updateCachesOnGroupCreation(createdGroupInReviewDb); return createdGroupInReviewDb; } @@ -249,7 +243,7 @@ public class GroupsUpdate { AccountGroup group = getExistingGroupFromReviewDb(db, groupUuid); UpdateResult reviewDbUpdateResult = updateGroupInReviewDb(db, group, groupUpdate); - if (!writeGroupsToNoteDb) { + if (!groupsMigration.writeToNoteDb()) { return reviewDbUpdateResult; } diff --git a/java/com/google/gerrit/server/index/group/StalenessChecker.java b/java/com/google/gerrit/server/index/group/StalenessChecker.java index 242e143681..418bb35dd3 100644 --- a/java/com/google/gerrit/server/index/group/StalenessChecker.java +++ b/java/com/google/gerrit/server/index/group/StalenessChecker.java @@ -14,23 +14,18 @@ package com.google.gerrit.server.index.group; -import static com.google.gerrit.server.notedb.NoteDbTable.GROUPS; -import static com.google.gerrit.server.notedb.NotesMigration.READ; -import static com.google.gerrit.server.notedb.NotesMigration.SECTION_NOTE_DB; - import com.google.common.collect.ImmutableSet; import com.google.gerrit.index.IndexConfig; import com.google.gerrit.index.query.FieldBundle; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.config.AllUsersName; -import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.notedb.GroupsMigration; import com.google.inject.Inject; import com.google.inject.Singleton; import java.io.IOException; import java.util.Optional; -import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; @@ -52,7 +47,7 @@ public class StalenessChecker { private final GitRepositoryManager repoManager; private final IndexConfig indexConfig; private final AllUsersName allUsers; - private final Config config; + private final GroupsMigration groupsMigration; @Inject StalenessChecker( @@ -60,16 +55,16 @@ public class StalenessChecker { GitRepositoryManager repoManager, IndexConfig indexConfig, AllUsersName allUsers, - @GerritServerConfig Config config) { + GroupsMigration groupsMigration) { this.indexes = indexes; this.repoManager = repoManager; this.indexConfig = indexConfig; this.allUsers = allUsers; - this.config = config; + this.groupsMigration = groupsMigration; } public boolean isStale(AccountGroup.UUID uuid) throws IOException { - if (!config.getBoolean(SECTION_NOTE_DB, GROUPS.key(), READ, false)) { + if (!groupsMigration.readFromNoteDb()) { return false; // This class only treats staleness for groups in NoteDb. } diff --git a/java/com/google/gerrit/server/notedb/GroupsMigration.java b/java/com/google/gerrit/server/notedb/GroupsMigration.java new file mode 100644 index 0000000000..66a0b44e48 --- /dev/null +++ b/java/com/google/gerrit/server/notedb/GroupsMigration.java @@ -0,0 +1,57 @@ +// 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 static com.google.gerrit.server.notedb.NoteDbTable.GROUPS; +import static com.google.gerrit.server.notedb.NotesMigration.READ; +import static com.google.gerrit.server.notedb.NotesMigration.SECTION_NOTE_DB; +import static com.google.gerrit.server.notedb.NotesMigration.WRITE; + +import com.google.gerrit.server.config.GerritServerConfig; +import com.google.inject.AbstractModule; +import com.google.inject.Inject; +import com.google.inject.Singleton; +import org.eclipse.jgit.lib.Config; + +@Singleton +public class GroupsMigration { + public static class Module extends AbstractModule { + @Override + public void configure() { + bind(GroupsMigration.class); + } + } + + private final boolean writeToNoteDb; + private final boolean readFromNoteDb; + + @Inject + public GroupsMigration(@GerritServerConfig Config cfg) { + // TODO(aliceks): Remove these flags when all other necessary TODOs for writing groups to + // NoteDb have been addressed. + // Don't flip these flags in a production setting! We only added them to spread the + // implementation of groups in NoteDb among several changes which are gradually merged. + this.writeToNoteDb = cfg.getBoolean(SECTION_NOTE_DB, GROUPS.key(), WRITE, false); + this.readFromNoteDb = cfg.getBoolean(SECTION_NOTE_DB, GROUPS.key(), READ, false); + } + + public boolean writeToNoteDb() { + return writeToNoteDb; + } + + public boolean readFromNoteDb() { + return readFromNoteDb; + } +} diff --git a/java/com/google/gerrit/server/schema/SchemaCreator.java b/java/com/google/gerrit/server/schema/SchemaCreator.java index c6b1ea8a5d..112fd55dc0 100644 --- a/java/com/google/gerrit/server/schema/SchemaCreator.java +++ b/java/com/google/gerrit/server/schema/SchemaCreator.java @@ -14,10 +14,6 @@ package com.google.gerrit.server.schema; -import static com.google.gerrit.server.notedb.NoteDbTable.GROUPS; -import static com.google.gerrit.server.notedb.NotesMigration.SECTION_NOTE_DB; -import static com.google.gerrit.server.notedb.NotesMigration.WRITE; - import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.gerrit.common.Nullable; @@ -49,6 +45,7 @@ import com.google.gerrit.server.group.db.InternalGroupCreation; import com.google.gerrit.server.group.db.InternalGroupUpdate; import com.google.gerrit.server.index.group.GroupIndex; import com.google.gerrit.server.index.group.GroupIndexCollection; +import com.google.gerrit.server.notedb.GroupsMigration; import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.update.RefUpdateUtil; import com.google.gwtorm.jdbc.JdbcExecutor; @@ -76,7 +73,7 @@ public class SchemaCreator { private final PersonIdent serverUser; private final DataSourceType dataSourceType; private final GroupIndexCollection indexCollection; - private final boolean writeGroupsToNoteDb; + private final GroupsMigration groupsMigration; private final Config config; private final MetricMaker metricMaker; @@ -93,6 +90,7 @@ public class SchemaCreator { @GerritPersonIdent PersonIdent au, DataSourceType dst, GroupIndexCollection ic, + GroupsMigration gm, @GerritServerConfig Config config, MetricMaker metricMaker, NotesMigration migration, @@ -106,6 +104,7 @@ public class SchemaCreator { au, dst, ic, + gm, config, metricMaker, migration, @@ -121,6 +120,7 @@ public class SchemaCreator { @GerritPersonIdent PersonIdent au, DataSourceType dst, GroupIndexCollection ic, + GroupsMigration gm, Config config, MetricMaker metricMaker, NotesMigration migration, @@ -133,11 +133,7 @@ public class SchemaCreator { serverUser = au; dataSourceType = dst; indexCollection = ic; - // TODO(aliceks): Remove this flag when all other necessary TODOs for writing groups to NoteDb - // have been addressed. - // Don't flip this flag in a production setting! We only added it to spread the implementation - // of groups in NoteDb among several changes which are gradually merged. - writeGroupsToNoteDb = config.getBoolean(SECTION_NOTE_DB, GROUPS.key(), WRITE, false); + groupsMigration = gm; this.config = config; this.allProjectsName = apName; @@ -217,7 +213,7 @@ public class SchemaCreator { throws OrmException, ConfigInvalidException, IOException { InternalGroup groupInReviewDb = createGroupInReviewDb(db, groupCreation, groupUpdate); - if (!writeGroupsToNoteDb) { + if (!groupsMigration.writeToNoteDb()) { index(groupInReviewDb); return; } diff --git a/javatests/com/google/gerrit/server/schema/SchemaUpdaterTest.java b/javatests/com/google/gerrit/server/schema/SchemaUpdaterTest.java index bdcee402ce..e4b0da5724 100644 --- a/javatests/com/google/gerrit/server/schema/SchemaUpdaterTest.java +++ b/javatests/com/google/gerrit/server/schema/SchemaUpdaterTest.java @@ -32,6 +32,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.GroupsMigration; import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.testing.InMemoryDatabase; import com.google.gerrit.testing.InMemoryH2Type; @@ -115,6 +116,7 @@ public class SchemaUpdaterTest { bind(SystemGroupBackend.class); install(new NotesMigration.Module()); + install(new GroupsMigration.Module()); bind(MetricMaker.class).to(DisabledMetricMaker.class); } })