Add class to encapsulate config parameters of groups to NoteDb migration

Have a single place to read the config parameters of the groups to
NoteDb migration. This makes it easier to add futher config parameters
(e.g. to disable group tables in ReviewDb).

Change-Id: I7805a04b7d619048e849c3401e9b452fa3cf1089
Signed-off-by: Edwin Kempin <ekempin@google.com>

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<ReviewDb>> schemaFactory =
         new TypeLiteral<SchemaFactory<ReviewDb>>() {};
     bind(schemaFactory).to(NotesMigrationSchemaFactory.class);

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);
   }

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 GroupsMigration groupsMigration;
-  private final boolean writeGroupsToNoteDb;
 
   @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);
+    this.groupsMigration = new GroupsMigration(flags.cfg);
-    // 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);
   }
 
   /**
@@ -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<GroupReference> 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);

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);

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<InternalGroup> 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<GroupReference> 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<AccountGroup.UUID> 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<AccountGroupMemberAudit> 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<AccountGroupByIdAud> 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");
     }

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;
     }
 

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

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;
+  }
+}

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
+    groupsMigration = gm;
-    // 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);
 
     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;
     }

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);
                   }
                 })