Add possibility to read groups from NoteDb

By default, groups are still read from ReviewDb. Reading from NoteDb
can be enabled by setting the config parameter 'readGroupsFromNoteDb'
in the section 'user' to 'true'.

To test the implementation of groups in ReviewDb as well as in NoteDb,
we execute all tests in GroupsIT twice: once with 'readGroupsFromNoteDb'
enabled and once without. Ideally, we would do so for all tests but
for the moment, we have to do that manually.

This change adds some TODOs which must be addressed before we can
submit this topic.

Change-Id: I24dfd1b7d0ee0a623b684816185bd59f7f89549d

java/com/google/gerrit/pgm/init/GroupsOnInit.java

@@ -92,6 +92,7 @@ public class GroupsOnInit {
    */
   public AccountGroup getExistingGroup(ReviewDb db, AccountGroup.NameKey groupName)
       throws OrmException, NoSuchGroupException {
+    // TODO(aliceks): Add implementation for NoteDb.
     AccountGroupName accountGroupName = db.accountGroupNames().get(groupName);
     if (accountGroupName == null) {
       throw new NoSuchGroupException(groupName.toString());

java/com/google/gerrit/server/account/GroupCacheImpl.java

@@ -150,18 +150,15 @@ public class GroupCacheImpl implements GroupCache {
 
   static class ByIdLoader extends CacheLoader<AccountGroup.Id, Optional<InternalGroup>> {
     private final SchemaFactory<ReviewDb> schema;
-    private final Groups groups;
     private final BooleanSupplier hasGroupIndex;
     private final Provider<InternalGroupQuery> groupQueryProvider;
 
     @Inject
     ByIdLoader(
         SchemaFactory<ReviewDb> schema,
-        Groups groups,
         GroupIndexCollection groupIndexCollection,
         Provider<InternalGroupQuery> groupQueryProvider) {
       this.schema = schema;
-      this.groups = groups;
       hasGroupIndex = () -> groupIndexCollection.getSearchIndex() != null;
       this.groupQueryProvider = groupQueryProvider;
     }
@@ -173,7 +170,7 @@ public class GroupCacheImpl implements GroupCache {
       }
 
       try (ReviewDb db = schema.open()) {
-        return groups.getGroup(db, key);
+        return Groups.getGroupFromReviewDb(db, key);
       }
     }
   }

java/com/google/gerrit/server/group/db/Groups.java

@@ -25,14 +25,22 @@ import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.AccountGroupById;
 import com.google.gerrit.reviewdb.client.AccountGroupMember;
 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.gwtorm.server.OrmDuplicateKeyException;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.ResultSet;
+import com.google.inject.Inject;
 import com.google.inject.Singleton;
+import java.io.IOException;
 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;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -50,6 +58,20 @@ import org.slf4j.LoggerFactory;
 public class Groups {
   private static final Logger log = LoggerFactory.getLogger(Groups.class);
 
+  private final boolean readFromNoteDb;
+  private final GitRepositoryManager repoManager;
+  private final AllUsersName allUsersName;
+
+  @Inject
+  public Groups(
+      @GerritServerConfig Config config,
+      GitRepositoryManager repoManager,
+      AllUsersName allUsersName) {
+    readFromNoteDb = config.getBoolean("user", null, "readGroupsFromNoteDb", false);
+    this.repoManager = repoManager;
+    this.allUsersName = allUsersName;
+  }
+
   /**
    * Returns the {@code AccountGroup} for the specified ID if it exists.
    *
@@ -58,7 +80,7 @@ public class Groups {
    * @return the found {@code AccountGroup} if it exists, or else an empty {@code Optional}
    * @throws OrmException if the group couldn't be retrieved from ReviewDb
    */
-  public Optional<InternalGroup> getGroup(ReviewDb db, AccountGroup.Id groupId)
+  public static Optional<InternalGroup> getGroupFromReviewDb(ReviewDb db, AccountGroup.Id groupId)
       throws OrmException {
     AccountGroup accountGroup = db.accountGroups().get(groupId);
     if (accountGroup == null) {
@@ -75,9 +97,17 @@ public class Groups {
    * @return the found {@code InternalGroup} if it exists, or else an empty {@code Optional}
    * @throws OrmDuplicateKeyException if multiple groups are found for the specified UUID
    * @throws OrmException if the group couldn't be retrieved from ReviewDb
+   * @throws IOException if the group couldn't be retrieved from NoteDb
+   * @throws ConfigInvalidException if the group couldn't be retrieved from NoteDb
    */
   public Optional<InternalGroup> getGroup(ReviewDb db, AccountGroup.UUID groupUuid)
-      throws OrmException {
+      throws OrmException, IOException, ConfigInvalidException {
+    if (readFromNoteDb) {
+      try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
+        return getGroupFromNoteDb(allUsersRepo, groupUuid);
+      }
+    }
+
     Optional<AccountGroup> accountGroup = getGroupFromReviewDb(db, groupUuid);
     if (!accountGroup.isPresent()) {
       return Optional.empty();
@@ -85,6 +115,13 @@ public class Groups {
     return Optional.of(asInternalGroup(db, accountGroup.get()));
   }
 
+  private static Optional<InternalGroup> getGroupFromNoteDb(
+      Repository allUsersRepository, AccountGroup.UUID groupUuid)
+      throws IOException, ConfigInvalidException {
+    GroupConfig groupConfig = GroupConfig.loadForGroup(allUsersRepository, groupUuid);
+    return groupConfig.getLoadedGroup();
+  }
+
   private static InternalGroup asInternalGroup(ReviewDb db, AccountGroup accountGroup)
       throws OrmException {
     ImmutableSet<Account.Id> members =
@@ -132,19 +169,21 @@ public class Groups {
   }
 
   public Stream<InternalGroup> getAll(ReviewDb db) throws OrmException {
+    // TODO(aliceks): Add code for NoteDb.
     return getAllUuids(db)
         .map(groupUuid -> getGroupIfPossible(db, groupUuid))
         .flatMap(Streams::stream);
   }
 
   public Stream<AccountGroup.UUID> getAllUuids(ReviewDb db) throws OrmException {
+    // TODO(aliceks): Add code for NoteDb.
     return Streams.stream(db.accountGroups().all()).map(AccountGroup::getGroupUUID);
   }
 
   private Optional<InternalGroup> getGroupIfPossible(ReviewDb db, AccountGroup.UUID groupUuid) {
     try {
       return getGroup(db, groupUuid);
-    } catch (OrmException e) {
+    } catch (OrmException | IOException | ConfigInvalidException e) {
       log.warn(String.format("Cannot look up group %s by uuid", groupUuid.get()), e);
     }
     return Optional.empty();
@@ -233,6 +272,7 @@ public class Groups {
    * @throws OrmException if an error occurs while reading from ReviewDb
    */
   public Stream<AccountGroup.UUID> getExternalGroups(ReviewDb db) throws OrmException {
+    // TODO(aliceks): Add code for NoteDb.
     return Streams.stream(db.accountGroupById().all())
         .map(AccountGroupById::getIncludeUUID)
         .distinct()

javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java

@@ -63,6 +63,7 @@ import com.google.gerrit.server.group.InternalGroup;
 import com.google.gerrit.server.group.SystemGroupBackend;
 import com.google.gerrit.server.group.db.Groups;
 import com.google.gerrit.server.util.MagicBranch;
+import com.google.gerrit.testing.ConfigSuite;
 import com.google.inject.Inject;
 import java.io.IOException;
 import java.sql.Timestamp;
@@ -89,6 +90,14 @@ import org.junit.Test;
 
 @NoHttpd
 public class GroupsIT extends AbstractDaemonTest {
+  @ConfigSuite.Config
+  public static Config noteDbConfig() {
+    Config config = new Config();
+    config.setBoolean("user", null, "writeGroupsToNoteDb", true);
+    config.setBoolean("user", null, "readGroupsFromNoteDb", true);
+    return config;
+  }
+
   @Inject private Groups groups;
   @Inject private GroupIncludeCache groupIncludeCache;
   @Inject private AllUsersName allUsers;