Remove support for groups in ReviewDb

Groups have been migrated to NoteDb. Hence we no longer need to be able
to read groups from ReviewDb.

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

java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -360,8 +360,8 @@ public abstract class AbstractDaemonTest {
     accountIndexer.index(accountId);
   }
 
-  private void reindexAllGroups() throws OrmException, IOException, ConfigInvalidException {
+  private void reindexAllGroups() throws IOException, ConfigInvalidException {
-    Iterable<GroupReference> allGroups = groups.getAllGroupReferences(db)::iterator;
+    Iterable<GroupReference> allGroups = groups.getAllGroupReferences()::iterator;
     for (GroupReference group : allGroups) {
       groupIndexer.index(group.getUUID());
     }

java/com/google/gerrit/acceptance/AccountCreator.java

@@ -23,7 +23,6 @@ import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.errors.NoSuchGroupException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.Sequences;
 import com.google.gerrit.server.ServerInitiated;
 import com.google.gerrit.server.account.AccountsUpdate;
@@ -35,7 +34,6 @@ import com.google.gerrit.server.group.db.GroupsUpdate;
 import com.google.gerrit.server.group.db.InternalGroupUpdate;
 import com.google.gerrit.server.ssh.SshKeyCache;
 import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
@@ -57,7 +55,6 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
 public class AccountCreator {
   private final Map<String, TestAccount> accounts;
 
-  private final SchemaFactory<ReviewDb> reviewDbProvider;
   private final Sequences sequences;
   private final Provider<AccountsUpdate> accountsUpdateProvider;
   private final VersionedAuthorizedKeys.Accessor authorizedKeys;
@@ -68,7 +65,6 @@ public class AccountCreator {
 
   @Inject
   AccountCreator(
-      SchemaFactory<ReviewDb> schema,
       Sequences sequences,
       @ServerInitiated Provider<AccountsUpdate> accountsUpdateProvider,
       VersionedAuthorizedKeys.Accessor authorizedKeys,
@@ -77,7 +73,6 @@ public class AccountCreator {
       SshKeyCache sshKeyCache,
       @SshEnabled boolean sshEnabled) {
     accounts = new HashMap<>();
-    reviewDbProvider = schema;
     this.sequences = sequences;
     this.accountsUpdateProvider = accountsUpdateProvider;
     this.authorizedKeys = authorizedKeys;
@@ -98,51 +93,49 @@ public class AccountCreator {
     if (account != null) {
       return account;
     }
-    try (ReviewDb db = reviewDbProvider.open()) {
+    Account.Id id = new Account.Id(sequences.nextAccountId());
-      Account.Id id = new Account.Id(sequences.nextAccountId());
 
-      List<ExternalId> extIds = new ArrayList<>(2);
+    List<ExternalId> extIds = new ArrayList<>(2);
-      String httpPass = null;
+    String httpPass = null;
-      if (username != null) {
+    if (username != null) {
-        httpPass = "http-pass";
+      httpPass = "http-pass";
-        extIds.add(ExternalId.createUsername(username, id, httpPass));
+      extIds.add(ExternalId.createUsername(username, id, httpPass));
-      }
-
-      if (email != null) {
-        extIds.add(ExternalId.createEmail(id, email));
-      }
-
-      accountsUpdateProvider
-          .get()
-          .insert(
-              "Create Test Account",
-              id,
-              u -> u.setFullName(fullName).setPreferredEmail(email).addExternalIds(extIds));
-
-      if (groupNames != null) {
-        for (String n : groupNames) {
-          AccountGroup.NameKey k = new AccountGroup.NameKey(n);
-          Optional<InternalGroup> group = groupCache.get(k);
-          if (!group.isPresent()) {
-            throw new NoSuchGroupException(n);
-          }
-          addGroupMember(db, group.get().getGroupUUID(), id);
-        }
-      }
-
-      KeyPair sshKey = null;
-      if (sshEnabled && username != null) {
-        sshKey = genSshKey();
-        authorizedKeys.addKey(id, publicKey(sshKey, email));
-        sshKeyCache.evict(username);
-      }
-
-      account = new TestAccount(id, username, email, fullName, sshKey, httpPass);
-      if (username != null) {
-        accounts.put(username, account);
-      }
-      return account;
     }
+
+    if (email != null) {
+      extIds.add(ExternalId.createEmail(id, email));
+    }
+
+    accountsUpdateProvider
+        .get()
+        .insert(
+            "Create Test Account",
+            id,
+            u -> u.setFullName(fullName).setPreferredEmail(email).addExternalIds(extIds));
+
+    if (groupNames != null) {
+      for (String n : groupNames) {
+        AccountGroup.NameKey k = new AccountGroup.NameKey(n);
+        Optional<InternalGroup> group = groupCache.get(k);
+        if (!group.isPresent()) {
+          throw new NoSuchGroupException(n);
+        }
+        addGroupMember(group.get().getGroupUUID(), id);
+      }
+    }
+
+    KeyPair sshKey = null;
+    if (sshEnabled && username != null) {
+      sshKey = genSshKey();
+      authorizedKeys.addKey(id, publicKey(sshKey, email));
+      sshKeyCache.evict(username);
+    }
+
+    account = new TestAccount(id, username, email, fullName, sshKey, httpPass);
+    if (username != null) {
+      accounts.put(username, account);
+    }
+    return account;
   }
 
   public TestAccount create(@Nullable String username, String group) throws Exception {
@@ -193,12 +186,12 @@ public class AccountCreator {
     return out.toString(US_ASCII.name()).trim();
   }
 
-  private void addGroupMember(ReviewDb db, AccountGroup.UUID groupUuid, Account.Id accountId)
+  private void addGroupMember(AccountGroup.UUID groupUuid, Account.Id accountId)
       throws OrmException, IOException, NoSuchGroupException, ConfigInvalidException {
     InternalGroupUpdate groupUpdate =
         InternalGroupUpdate.builder()
             .setMemberModification(memberIds -> Sets.union(memberIds, ImmutableSet.of(accountId)))
             .build();
-    groupsUpdateProvider.get().updateGroup(db, groupUuid, groupUpdate);
+    groupsUpdateProvider.get().updateGroup(groupUuid, groupUpdate);
   }
 }

java/com/google/gerrit/acceptance/GerritServer.java

@@ -36,7 +36,6 @@ import com.google.gerrit.server.util.OneOffRequestContext;
 import com.google.gerrit.server.util.SocketUtil;
 import com.google.gerrit.server.util.SystemLog;
 import com.google.gerrit.testing.FakeEmailSender;
-import com.google.gerrit.testing.GroupNoteDbMode;
 import com.google.gerrit.testing.InMemoryDatabase;
 import com.google.gerrit.testing.InMemoryRepositoryManager;
 import com.google.gerrit.testing.NoteDbChecker;
@@ -419,7 +418,6 @@ public class GerritServer implements AutoCloseable {
     cfg.setBoolean("index", null, "reindexAfterRefUpdate", false);
 
     NoteDbMode.newNotesMigrationFromEnv().setConfigValues(cfg);
-    GroupNoteDbMode.get().getGroupsMigration().setConfigValuesIfNotSetYet(cfg);
   }
 
   private static Injector createTestInjector(Daemon daemon) throws Exception {

java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java

@@ -29,7 +29,6 @@ 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;
@@ -93,7 +92,6 @@ 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,7 +71,6 @@ 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.permissions.DefaultPermissionBackendModule;
@@ -309,7 +308,6 @@ 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

@@ -16,11 +16,8 @@ package com.google.gerrit.pgm.init;
 
 import static com.google.common.base.Preconditions.checkArgument;
 
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Sets;
-import com.google.common.collect.Streams;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.common.errors.NoSuchGroupException;
@@ -28,8 +25,6 @@ import com.google.gerrit.pgm.init.api.AllUsersNameOnInitProvider;
 import com.google.gerrit.pgm.init.api.InitFlags;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.AccountGroupMember;
-import com.google.gerrit.reviewdb.client.AccountGroupName;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.GerritPersonIdentProvider;
@@ -41,17 +36,13 @@ import com.google.gerrit.server.group.InternalGroup;
 import com.google.gerrit.server.group.db.AuditLogFormatter;
 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;
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Path;
 import java.sql.Timestamp;
-import java.util.List;
 import java.util.stream.Stream;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.internal.storage.file.FileRepository;
@@ -74,14 +65,12 @@ public class GroupsOnInit {
   private final InitFlags flags;
   private final SitePaths site;
   private final String allUsers;
-  private final GroupsMigration groupsMigration;
 
   @Inject
   public GroupsOnInit(InitFlags flags, SitePaths site, AllUsersNameOnInitProvider allUsers) {
     this.flags = flags;
     this.site = site;
     this.allUsers = allUsers.get();
-    this.groupsMigration = new GroupsMigration(flags.cfg);
   }
 
   /**
@@ -97,15 +86,6 @@ public class GroupsOnInit {
    */
   public InternalGroup getExistingGroup(ReviewDb db, GroupReference groupReference)
       throws OrmException, NoSuchGroupException, IOException, ConfigInvalidException {
-    if (groupsMigration.readFromNoteDb()) {
-      return getExistingGroupFromNoteDb(groupReference);
-    }
-
-    return getExistingGroupFromReviewDb(db, groupReference);
-  }
-
-  private InternalGroup getExistingGroupFromNoteDb(GroupReference groupReference)
-      throws IOException, ConfigInvalidException, NoSuchGroupException {
     File allUsersRepoPath = getPathToAllUsersRepository();
     if (allUsersRepoPath != null) {
       try (Repository allUsersRepo = new FileRepository(allUsersRepoPath)) {
@@ -119,23 +99,6 @@ public class GroupsOnInit {
     throw new NoSuchGroupException(groupReference.getUUID());
   }
 
-  private static InternalGroup getExistingGroupFromReviewDb(
-      ReviewDb db, GroupReference groupReference) throws OrmException, NoSuchGroupException {
-    String groupName = groupReference.getName();
-    AccountGroupName accountGroupName =
-        db.accountGroupNames().get(new AccountGroup.NameKey(groupName));
-    if (accountGroupName == null) {
-      throw new NoSuchGroupException(groupName);
-    }
-
-    AccountGroup.Id groupId = accountGroupName.getId();
-    AccountGroup group = db.accountGroups().get(groupId);
-    if (group == null) {
-      throw new NoSuchGroupException(groupName);
-    }
-    return Groups.asInternalGroup(db, group);
-  }
-
   /**
    * Returns {@code GroupReference}s for all internal groups.
    *
@@ -147,18 +110,13 @@ public class GroupsOnInit {
    */
   public Stream<GroupReference> getAllGroupReferences(ReviewDb db)
       throws OrmException, IOException, ConfigInvalidException {
-    if (groupsMigration.readFromNoteDb()) {
+    File allUsersRepoPath = getPathToAllUsersRepository();
-      File allUsersRepoPath = getPathToAllUsersRepository();
+    if (allUsersRepoPath != null) {
-      if (allUsersRepoPath != null) {
+      try (Repository allUsersRepo = new FileRepository(allUsersRepoPath)) {
-        try (Repository allUsersRepo = new FileRepository(allUsersRepoPath)) {
+        return GroupNameNotes.loadAllGroups(allUsersRepo).stream();
-          return GroupNameNotes.loadAllGroups(allUsersRepo).stream();
-        }
       }
-      return Stream.empty();
     }
-
+    return Stream.empty();
-    return Streams.stream(db.accountGroups().all())
-        .map(group -> new GroupReference(group.getGroupUUID(), group.getName()));
   }
 
   /**
@@ -176,49 +134,6 @@ public class GroupsOnInit {
    */
   public void addGroupMember(ReviewDb db, AccountGroup.UUID groupUuid, Account account)
       throws OrmException, NoSuchGroupException, IOException, ConfigInvalidException {
-    addGroupMemberInReviewDb(db, groupUuid, account.getId());
-    if (!groupsMigration.writeToNoteDb()) {
-      return;
-    }
-    addGroupMemberInNoteDb(groupUuid, account);
-  }
-
-  private static void addGroupMemberInReviewDb(
-      ReviewDb db, AccountGroup.UUID groupUuid, Account.Id accountId)
-      throws OrmException, NoSuchGroupException {
-    AccountGroup group = getExistingGroup(db, groupUuid);
-    AccountGroup.Id groupId = group.getId();
-
-    if (isMember(db, groupId, accountId)) {
-      return;
-    }
-
-    db.accountGroupMembers()
-        .insert(
-            ImmutableList.of(
-                new AccountGroupMember(new AccountGroupMember.Key(accountId, groupId))));
-  }
-
-  private static AccountGroup getExistingGroup(ReviewDb db, AccountGroup.UUID groupUuid)
-      throws OrmException, NoSuchGroupException {
-    List<AccountGroup> accountGroups = db.accountGroups().byUUID(groupUuid).toList();
-    if (accountGroups.size() == 1) {
-      return Iterables.getOnlyElement(accountGroups);
-    } else if (accountGroups.isEmpty()) {
-      throw new NoSuchGroupException(groupUuid);
-    } else {
-      throw new OrmDuplicateKeyException("Duplicate group UUID " + groupUuid);
-    }
-  }
-
-  private static boolean isMember(ReviewDb db, AccountGroup.Id groupId, Account.Id accountId)
-      throws OrmException {
-    AccountGroupMember.Key key = new AccountGroupMember.Key(accountId, groupId);
-    return db.accountGroupMembers().get(key) != null;
-  }
-
-  private void addGroupMemberInNoteDb(AccountGroup.UUID groupUuid, Account account)
-      throws IOException, ConfigInvalidException, NoSuchGroupException {
     File allUsersRepoPath = getPathToAllUsersRepository();
     if (allUsersRepoPath != null) {
       try (Repository repository = new FileRepository(allUsersRepoPath)) {

java/com/google/gerrit/pgm/util/SiteProgram.java

@@ -28,7 +28,6 @@ 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;
@@ -184,7 +183,6 @@ 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/reviewdb/server/DisallowReadFromGroupsReviewDbWrapper.java

@@ -1,306 +0,0 @@
-// 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.reviewdb.server;
-
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.AccountGroupById;
-import com.google.gerrit.reviewdb.client.AccountGroupByIdAud;
-import com.google.gerrit.reviewdb.client.AccountGroupMember;
-import com.google.gerrit.reviewdb.client.AccountGroupMemberAudit;
-import com.google.gerrit.reviewdb.client.AccountGroupName;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.ResultSet;
-
-public class DisallowReadFromGroupsReviewDbWrapper extends ReviewDbWrapper {
-  private static final String MSG = "This table has been migrated to NoteDb";
-
-  private final Groups groups;
-  private final GroupNames groupNames;
-  private final GroupMembers groupMembers;
-  private final GroupMemberAudits groupMemberAudits;
-  private final ByIds byIds;
-  private final ByIdAudits byIdAudits;
-
-  public DisallowReadFromGroupsReviewDbWrapper(ReviewDb db) {
-    super(db);
-    groups = new Groups(delegate.accountGroups());
-    groupNames = new GroupNames(delegate.accountGroupNames());
-    groupMembers = new GroupMembers(delegate.accountGroupMembers());
-    groupMemberAudits = new GroupMemberAudits(delegate.accountGroupMembersAudit());
-    byIds = new ByIds(delegate.accountGroupById());
-    byIdAudits = new ByIdAudits(delegate.accountGroupByIdAud());
-  }
-
-  @Override
-  public AccountGroupAccess accountGroups() {
-    return groups;
-  }
-
-  @Override
-  public AccountGroupNameAccess accountGroupNames() {
-    return groupNames;
-  }
-
-  @Override
-  public AccountGroupMemberAccess accountGroupMembers() {
-    return groupMembers;
-  }
-
-  @Override
-  public AccountGroupMemberAuditAccess accountGroupMembersAudit() {
-    return groupMemberAudits;
-  }
-
-  @Override
-  public AccountGroupByIdAccess accountGroupById() {
-    return byIds;
-  }
-
-  @Override
-  public AccountGroupByIdAudAccess accountGroupByIdAud() {
-    return byIdAudits;
-  }
-
-  private static class Groups extends AccountGroupAccessWrapper {
-    protected Groups(AccountGroupAccess delegate) {
-      super(delegate);
-    }
-
-    @Override
-    public ResultSet<AccountGroup> iterateAllEntities() {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @SuppressWarnings("deprecation")
-    @Override
-    public com.google.common.util.concurrent.CheckedFuture<AccountGroup, OrmException> getAsync(
-        AccountGroup.Id key) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroup> get(Iterable<AccountGroup.Id> keys) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public AccountGroup get(AccountGroup.Id id) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroup> byUUID(AccountGroup.UUID uuid) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroup> all() {
-      throw new UnsupportedOperationException(MSG);
-    }
-  }
-
-  private static class GroupNames extends AccountGroupNameAccessWrapper {
-    protected GroupNames(AccountGroupNameAccess delegate) {
-      super(delegate);
-    }
-
-    @Override
-    public ResultSet<AccountGroupName> iterateAllEntities() {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @SuppressWarnings("deprecation")
-    @Override
-    public com.google.common.util.concurrent.CheckedFuture<AccountGroupName, OrmException> getAsync(
-        AccountGroup.NameKey key) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroupName> get(Iterable<AccountGroup.NameKey> keys) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public AccountGroupName get(AccountGroup.NameKey name) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroupName> all() {
-      throw new UnsupportedOperationException(MSG);
-    }
-  }
-
-  private static class GroupMembers extends AccountGroupMemberAccessWrapper {
-    protected GroupMembers(AccountGroupMemberAccess delegate) {
-      super(delegate);
-    }
-
-    @Override
-    public ResultSet<AccountGroupMember> iterateAllEntities() {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @SuppressWarnings("deprecation")
-    @Override
-    public com.google.common.util.concurrent.CheckedFuture<AccountGroupMember, OrmException>
-        getAsync(AccountGroupMember.Key key) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroupMember> get(Iterable<AccountGroupMember.Key> keys) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public AccountGroupMember get(AccountGroupMember.Key key) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroupMember> byAccount(Account.Id id) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroupMember> byGroup(AccountGroup.Id id) {
-      throw new UnsupportedOperationException(MSG);
-    }
-  }
-
-  private static class GroupMemberAudits extends AccountGroupMemberAuditAccessWrapper {
-    protected GroupMemberAudits(AccountGroupMemberAuditAccess delegate) {
-      super(delegate);
-    }
-
-    @Override
-    public ResultSet<AccountGroupMemberAudit> iterateAllEntities() {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @SuppressWarnings("deprecation")
-    @Override
-    public com.google.common.util.concurrent.CheckedFuture<AccountGroupMemberAudit, OrmException>
-        getAsync(AccountGroupMemberAudit.Key key) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroupMemberAudit> get(Iterable<AccountGroupMemberAudit.Key> keys) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public AccountGroupMemberAudit get(AccountGroupMemberAudit.Key key) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroupMemberAudit> byGroupAccount(
-        AccountGroup.Id groupId, Account.Id accountId) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroupMemberAudit> byGroup(AccountGroup.Id groupId) {
-      throw new UnsupportedOperationException(MSG);
-    }
-  }
-
-  private static class ByIds extends AccountGroupByIdAccessWrapper {
-    protected ByIds(AccountGroupByIdAccess delegate) {
-      super(delegate);
-    }
-
-    @Override
-    public ResultSet<AccountGroupById> iterateAllEntities() {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @SuppressWarnings("deprecation")
-    @Override
-    public com.google.common.util.concurrent.CheckedFuture<AccountGroupById, OrmException> getAsync(
-        AccountGroupById.Key key) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroupById> get(Iterable<AccountGroupById.Key> keys) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public AccountGroupById get(AccountGroupById.Key key) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroupById> byIncludeUUID(AccountGroup.UUID uuid) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroupById> byGroup(AccountGroup.Id id) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroupById> all() {
-      throw new UnsupportedOperationException(MSG);
-    }
-  }
-
-  private static class ByIdAudits extends AccountGroupByIdAudAccessWrapper {
-    protected ByIdAudits(AccountGroupByIdAudAccess delegate) {
-      super(delegate);
-    }
-
-    @Override
-    public ResultSet<AccountGroupByIdAud> iterateAllEntities() {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @SuppressWarnings("deprecation")
-    @Override
-    public com.google.common.util.concurrent.CheckedFuture<AccountGroupByIdAud, OrmException>
-        getAsync(AccountGroupByIdAud.Key key) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroupByIdAud> get(Iterable<AccountGroupByIdAud.Key> keys) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public AccountGroupByIdAud get(AccountGroupByIdAud.Key key) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroupByIdAud> byGroupInclude(
-        AccountGroup.Id groupId, AccountGroup.UUID incGroupUUID) {
-      throw new UnsupportedOperationException(MSG);
-    }
-
-    @Override
-    public ResultSet<AccountGroupByIdAud> byGroup(AccountGroup.Id groupId) {
-      throw new UnsupportedOperationException(MSG);
-    }
-  }
-}

java/com/google/gerrit/reviewdb/server/ReviewDbUtil.java

@@ -53,9 +53,6 @@ public class ReviewDbUtil {
     if (db instanceof DisallowReadFromChangesReviewDbWrapper) {
       return unwrapDb(((DisallowReadFromChangesReviewDbWrapper) db).unsafeGetDelegate());
     }
-    if (db instanceof DisallowReadFromGroupsReviewDbWrapper) {
-      return unwrapDb(((DisallowReadFromGroupsReviewDbWrapper) db).unsafeGetDelegate());
-    }
     return db;
   }
 

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

@@ -28,7 +28,6 @@ import com.google.gerrit.common.errors.NoSuchGroupException;
 import com.google.gerrit.extensions.client.AccountFieldName;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.Sequences;
 import com.google.gerrit.server.ServerInitiated;
@@ -43,7 +42,6 @@ import com.google.gerrit.server.group.db.InternalGroupUpdate;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.ssh.SshKeyCache;
 import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
@@ -65,7 +63,6 @@ import org.slf4j.LoggerFactory;
 public class AccountManager {
   private static final Logger log = LoggerFactory.getLogger(AccountManager.class);
 
-  private final SchemaFactory<ReviewDb> schema;
   private final Sequences sequences;
   private final Accounts accounts;
   private final Provider<AccountsUpdate> accountsUpdateProvider;
@@ -82,7 +79,6 @@ public class AccountManager {
 
   @Inject
   AccountManager(
-      SchemaFactory<ReviewDb> schema,
       Sequences sequences,
       @GerritServerConfig Config cfg,
       Accounts accounts,
@@ -95,7 +91,6 @@ public class AccountManager {
       ExternalIds externalIds,
       GroupsUpdate.Factory groupsUpdateFactory,
       SetInactiveFlag setInactiveFlag) {
-    this.schema = schema;
     this.sequences = sequences;
     this.accounts = accounts;
     this.accountsUpdateProvider = accountsUpdateProvider;
@@ -140,51 +135,49 @@ public class AccountManager {
       throw e;
     }
     try {
-      try (ReviewDb db = schema.open()) {
+      ExternalId id = externalIds.get(who.getExternalIdKey());
-        ExternalId id = externalIds.get(who.getExternalIdKey());
+      if (id == null) {
-        if (id == null) {
+        if (who.getUserName().isPresent()) {
-          if (who.getUserName().isPresent()) {
+          ExternalId.Key key = ExternalId.Key.create(SCHEME_USERNAME, who.getUserName().get());
-            ExternalId.Key key = ExternalId.Key.create(SCHEME_USERNAME, who.getUserName().get());
+          ExternalId existingId = externalIds.get(key);
-            ExternalId existingId = externalIds.get(key);
+          if (existingId != null) {
-            if (existingId != null) {
+            // An inconsistency is detected in the database, having a record for scheme "username:"
-              // An inconsistency is detected in the database, having a record for scheme "username:"
+            // but no record for scheme "gerrit:". Try to recover by linking
-              // but no record for scheme "gerrit:". Try to recover by linking
+            // "gerrit:" identity to the existing account.
-              // "gerrit:" identity to the existing account.
+            log.warn(
-              log.warn(
+                "User {} already has an account; link new identity to the existing account.",
-                  "User {} already has an account; link new identity to the existing account.",
+                who.getUserName());
-                  who.getUserName());
+            return link(existingId.accountId(), who);
-              return link(existingId.accountId(), who);
-            }
           }
-          // New account, automatically create and return.
-          log.info("External ID not found. Attempting to create new account.");
-          return create(db, who);
         }
-
+        // New account, automatically create and return.
-        Optional<AccountState> accountState = byIdCache.get(id.accountId());
+        log.info("External ID not found. Attempting to create new account.");
-        if (!accountState.isPresent()) {
+        return create(who);
-          log.error(
-              String.format(
-                  "Authentication with external ID %s failed. Account %s doesn't exist.",
-                  id.key().get(), id.accountId().get()));
-          throw new AccountException("Authentication error, account not found");
-        }
-
-        // Account exists
-        Optional<Account> act = updateAccountActiveStatus(who, accountState.get().getAccount());
-        if (!act.isPresent()) {
-          // The account was deleted since we checked for it last time. This should never happen
-          // since we don't support deletion of accounts.
-          throw new AccountException("Authentication error, account not found");
-        }
-        if (!act.get().isActive()) {
-          throw new AccountException("Authentication error, account inactive");
-        }
-
-        // return the identity to the caller.
-        update(who, id);
-        return new AuthResult(id.accountId(), who.getExternalIdKey(), false);
       }
+
+      Optional<AccountState> accountState = byIdCache.get(id.accountId());
+      if (!accountState.isPresent()) {
+        log.error(
+            String.format(
+                "Authentication with external ID %s failed. Account %s doesn't exist.",
+                id.key().get(), id.accountId().get()));
+        throw new AccountException("Authentication error, account not found");
+      }
+
+      // Account exists
+      Optional<Account> act = updateAccountActiveStatus(who, accountState.get().getAccount());
+      if (!act.isPresent()) {
+        // The account was deleted since we checked for it last time. This should never happen
+        // since we don't support deletion of accounts.
+        throw new AccountException("Authentication error, account not found");
+      }
+      if (!act.get().isActive()) {
+        throw new AccountException("Authentication error, account inactive");
+      }
+
+      // return the identity to the caller.
+      update(who, id);
+      return new AuthResult(id.accountId(), who.getExternalIdKey(), false);
     } catch (OrmException | ConfigInvalidException e) {
       throw new AccountException("Authentication error", e);
     }
@@ -289,7 +282,7 @@ public class AccountManager {
     }
   }
 
-  private AuthResult create(ReviewDb db, AuthRequest who)
+  private AuthResult create(AuthRequest who)
       throws OrmException, AccountException, IOException, ConfigInvalidException {
     Account.Id newId = new Account.Id(sequences.nextAccountId());
 
@@ -349,7 +342,7 @@ public class AccountManager {
               .getPermission(GlobalCapability.ADMINISTRATE_SERVER);
 
       AccountGroup.UUID adminGroupUuid = admin.getRules().get(0).getGroup().getUUID();
-      addGroupMember(db, adminGroupUuid, user);
+      addGroupMember(adminGroupUuid, user);
     }
 
     realm.onCreateAccount(who, accountState.getAccount());
@@ -369,7 +362,7 @@ public class AccountManager {
     return ExternalId.create(SCHEME_USERNAME, username, accountId);
   }
 
-  private void addGroupMember(ReviewDb db, AccountGroup.UUID groupUuid, IdentifiedUser user)
+  private void addGroupMember(AccountGroup.UUID groupUuid, IdentifiedUser user)
       throws OrmException, IOException, ConfigInvalidException, AccountException {
     // The user initiated this request by logging in. -> Attribute all modifications to that user.
     GroupsUpdate groupsUpdate = groupsUpdateFactory.create(user);
@@ -379,7 +372,7 @@ public class AccountManager {
                 memberIds -> Sets.union(memberIds, ImmutableSet.of(user.getAccountId())))
             .build();
     try {
-      groupsUpdate.updateGroup(db, groupUuid, groupUpdate);
+      groupsUpdate.updateGroup(groupUuid, groupUpdate);
     } catch (NoSuchGroupException e) {
       throw new AccountException(String.format("Group %s not found", groupUuid));
     }

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

@@ -17,12 +17,10 @@ package com.google.gerrit.server.account;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.cache.CacheModule;
 import com.google.gerrit.server.group.InternalGroup;
 import com.google.gerrit.server.group.db.Groups;
 import com.google.gerrit.server.query.group.InternalGroupQuery;
-import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
 import com.google.inject.Module;
 import com.google.inject.Provider;
@@ -166,20 +164,16 @@ public class GroupCacheImpl implements GroupCache {
   }
 
   static class ByUUIDLoader extends CacheLoader<String, Optional<InternalGroup>> {
-    private final SchemaFactory<ReviewDb> schema;
     private final Groups groups;
 
     @Inject
-    ByUUIDLoader(SchemaFactory<ReviewDb> sf, Groups groups) {
+    ByUUIDLoader(Groups groups) {
-      schema = sf;
       this.groups = groups;
     }
 
     @Override
     public Optional<InternalGroup> load(String uuid) throws Exception {
-      try (ReviewDb db = schema.open()) {
+      return groups.getGroup(new AccountGroup.UUID(uuid));
-        return groups.getGroup(db, new AccountGroup.UUID(uuid));
-      }
     }
   }
 }

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

@@ -23,13 +23,11 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.cache.CacheModule;
 import com.google.gerrit.server.group.InternalGroup;
 import com.google.gerrit.server.group.db.Groups;
 import com.google.gerrit.server.query.group.InternalGroupQuery;
 import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
 import com.google.inject.Module;
 import com.google.inject.Provider;
@@ -180,20 +178,16 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
   }
 
   static class AllExternalLoader extends CacheLoader<String, ImmutableList<AccountGroup.UUID>> {
-    private final SchemaFactory<ReviewDb> schema;
     private final Groups groups;
 
     @Inject
-    AllExternalLoader(SchemaFactory<ReviewDb> sf, Groups groups) {
+    AllExternalLoader(Groups groups) {
-      schema = sf;
       this.groups = groups;
     }
 
     @Override
     public ImmutableList<AccountGroup.UUID> load(String key) throws Exception {
-      try (ReviewDb db = schema.open()) {
+      return groups.getExternalGroups().collect(toImmutableList());
-        return groups.getExternalGroups(db).collect(toImmutableList());
-      }
     }
   }
 }

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

@@ -20,15 +20,12 @@ import com.google.common.collect.ImmutableList;
 import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.group.InternalGroup;
 import com.google.gerrit.server.group.InternalGroupDescription;
 import com.google.gerrit.server.group.db.Groups;
 import com.google.gerrit.server.group.db.GroupsNoteDbConsistencyChecker;
 import com.google.gerrit.server.project.ProjectState;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import java.io.IOException;
@@ -43,7 +40,6 @@ public class InternalGroupBackend implements GroupBackend {
   private final GroupControl.Factory groupControlFactory;
   private final GroupCache groupCache;
   private final Groups groups;
-  private final SchemaFactory<ReviewDb> schema;
   private final IncludingGroupMembership.Factory groupMembershipFactory;
 
   @Inject
@@ -51,12 +47,10 @@ public class InternalGroupBackend implements GroupBackend {
       GroupControl.Factory groupControlFactory,
       GroupCache groupCache,
       Groups groups,
-      SchemaFactory<ReviewDb> schema,
       IncludingGroupMembership.Factory groupMembershipFactory) {
     this.groupControlFactory = groupControlFactory;
     this.groupCache = groupCache;
     this.groups = groups;
-    this.schema = schema;
     this.groupMembershipFactory = groupMembershipFactory;
   }
 
@@ -77,13 +71,13 @@ public class InternalGroupBackend implements GroupBackend {
 
   @Override
   public Collection<GroupReference> suggest(String name, ProjectState project) {
-    try (ReviewDb db = schema.open()) {
+    try {
       return groups
-          .getAllGroupReferences(db)
+          .getAllGroupReferences()
           .filter(group -> startsWithIgnoreCase(group, name))
           .filter(this::isVisible)
           .collect(toList());
-    } catch (OrmException | IOException | ConfigInvalidException e) {
+    } catch (IOException | ConfigInvalidException e) {
       return ImmutableList.of();
     }
   }

java/com/google/gerrit/server/config/GerritGlobalModule.java

@@ -155,7 +155,6 @@ import com.google.gerrit.server.mail.send.ReplacePatchSetSender;
 import com.google.gerrit.server.mail.send.SetAssigneeSender;
 import com.google.gerrit.server.mime.FileTypeRegistry;
 import com.google.gerrit.server.mime.MimeUtilFileTypeRegistry;
-import com.google.gerrit.server.notedb.GroupsMigration;
 import com.google.gerrit.server.notedb.NoteDbModule;
 import com.google.gerrit.server.patch.PatchListCacheImpl;
 import com.google.gerrit.server.patch.PatchScriptFactory;
@@ -204,14 +203,11 @@ import org.eclipse.jgit.transport.PreUploadHook;
 public class GerritGlobalModule extends FactoryModule {
   private final Config cfg;
   private final AuthModule authModule;
-  private final GroupsMigration groupsMigration;
 
   @Inject
-  GerritGlobalModule(
+  GerritGlobalModule(@GerritServerConfig Config cfg, AuthModule authModule) {
-      @GerritServerConfig Config cfg, AuthModule authModule, GroupsMigration groupsMigration) {
     this.cfg = cfg;
     this.authModule = authModule;
-    this.groupsMigration = groupsMigration;
   }
 
   @Override
@@ -307,7 +303,6 @@ public class GerritGlobalModule extends FactoryModule {
     install(new com.google.gerrit.server.restapi.access.Module());
     install(new ConfigRestModule());
     install(new com.google.gerrit.server.restapi.change.Module());
-    install(new com.google.gerrit.server.group.Module(groupsMigration));
     install(new com.google.gerrit.server.restapi.account.Module());
     install(new com.google.gerrit.server.restapi.project.Module());
     install(new com.google.gerrit.server.restapi.group.Module());

java/com/google/gerrit/server/group/DbGroupAuditListener.java

@@ -1,265 +0,0 @@
-// Copyright (C) 2014 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.group;
-
-import static com.google.common.collect.ImmutableSet.toImmutableSet;
-import static com.google.gerrit.reviewdb.server.ReviewDbUtil.unwrapDb;
-
-import com.google.common.base.Joiner;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.AccountGroupByIdAud;
-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.account.AccountCache;
-import com.google.gerrit.server.account.AccountState;
-import com.google.gerrit.server.account.GroupCache;
-import com.google.gerrit.server.account.UniversalGroupBackend;
-import com.google.gerrit.server.audit.group.GroupAuditEvent;
-import com.google.gerrit.server.audit.group.GroupAuditListener;
-import com.google.gerrit.server.audit.group.GroupMemberAuditEvent;
-import com.google.gerrit.server.audit.group.GroupSubgroupAuditEvent;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.ResultSet;
-import com.google.gwtorm.server.SchemaFactory;
-import com.google.inject.Inject;
-import java.sql.Timestamp;
-import java.text.MessageFormat;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Optional;
-import org.slf4j.Logger;
-
-class DbGroupAuditListener implements GroupAuditListener {
-  private static final Logger log = org.slf4j.LoggerFactory.getLogger(DbGroupAuditListener.class);
-
-  private final SchemaFactory<ReviewDb> schema;
-  private final AccountCache accountCache;
-  private final GroupCache groupCache;
-  private final UniversalGroupBackend groupBackend;
-
-  @Inject
-  DbGroupAuditListener(
-      SchemaFactory<ReviewDb> schema,
-      AccountCache accountCache,
-      GroupCache groupCache,
-      UniversalGroupBackend groupBackend) {
-    this.schema = schema;
-    this.accountCache = accountCache;
-    this.groupCache = groupCache;
-    this.groupBackend = groupBackend;
-  }
-
-  @Override
-  public void onAddMembers(GroupMemberAuditEvent event) {
-    Optional<InternalGroup> updatedGroup = groupCache.get(event.getUpdatedGroup());
-    if (!updatedGroup.isPresent()) {
-      logFailToLoadUpdatedGroup(event);
-      return;
-    }
-
-    InternalGroup group = updatedGroup.get();
-    try (ReviewDb db = unwrapDb(schema.open())) {
-      db.accountGroupMembersAudit().insert(toAccountGroupMemberAudits(event, group.getId()));
-    } catch (OrmException e) {
-      logOrmException(
-          "Cannot log add accounts to group event performed by user", event, group.getName(), e);
-    }
-  }
-
-  @Override
-  public void onDeleteMembers(GroupMemberAuditEvent event) {
-    Optional<InternalGroup> updatedGroup = groupCache.get(event.getUpdatedGroup());
-    if (!updatedGroup.isPresent()) {
-      logFailToLoadUpdatedGroup(event);
-      return;
-    }
-
-    InternalGroup group = updatedGroup.get();
-    List<AccountGroupMemberAudit> auditInserts = new ArrayList<>();
-    List<AccountGroupMemberAudit> auditUpdates = new ArrayList<>();
-    try (ReviewDb db = unwrapDb(schema.open())) {
-      for (Account.Id accountId : event.getModifiedMembers()) {
-        AccountGroupMemberAudit audit = null;
-        ResultSet<AccountGroupMemberAudit> audits =
-            db.accountGroupMembersAudit().byGroupAccount(group.getId(), accountId);
-        for (AccountGroupMemberAudit a : audits) {
-          if (a.isActive()) {
-            audit = a;
-            break;
-          }
-        }
-
-        if (audit != null) {
-          audit.removed(event.getActor(), event.getTimestamp());
-          auditUpdates.add(audit);
-          continue;
-        }
-        AccountGroupMember.Key key = new AccountGroupMember.Key(accountId, group.getId());
-        audit =
-            new AccountGroupMemberAudit(
-                new AccountGroupMember(key), event.getActor(), event.getTimestamp());
-        audit.removedLegacy();
-        auditInserts.add(audit);
-      }
-      db.accountGroupMembersAudit().update(auditUpdates);
-      db.accountGroupMembersAudit().insert(auditInserts);
-    } catch (OrmException e) {
-      logOrmException(
-          "Cannot log delete accounts from group event performed by user",
-          event,
-          group.getName(),
-          e);
-    }
-  }
-
-  @Override
-  public void onAddSubgroups(GroupSubgroupAuditEvent event) {
-    Optional<InternalGroup> updatedGroup = groupCache.get(event.getUpdatedGroup());
-    if (!updatedGroup.isPresent()) {
-      logFailToLoadUpdatedGroup(event);
-      return;
-    }
-
-    InternalGroup group = updatedGroup.get();
-    try (ReviewDb db = unwrapDb(schema.open())) {
-      db.accountGroupByIdAud().insert(toAccountGroupByIdAudits(event, group.getId()));
-    } catch (OrmException e) {
-      logOrmException(
-          "Cannot log add groups to group event performed by user", event, group.getName(), e);
-    }
-  }
-
-  @Override
-  public void onDeleteSubgroups(GroupSubgroupAuditEvent event) {
-    Optional<InternalGroup> updatedGroup = groupCache.get(event.getUpdatedGroup());
-    if (!updatedGroup.isPresent()) {
-      logFailToLoadUpdatedGroup(event);
-      return;
-    }
-
-    InternalGroup group = updatedGroup.get();
-    List<AccountGroupByIdAud> auditUpdates = new ArrayList<>();
-    try (ReviewDb db = unwrapDb(schema.open())) {
-      for (AccountGroup.UUID uuid : event.getModifiedSubgroups()) {
-        AccountGroupByIdAud audit = null;
-        ResultSet<AccountGroupByIdAud> audits =
-            db.accountGroupByIdAud().byGroupInclude(updatedGroup.get().getId(), uuid);
-        for (AccountGroupByIdAud a : audits) {
-          if (a.isActive()) {
-            audit = a;
-            break;
-          }
-        }
-
-        if (audit != null) {
-          audit.removed(event.getActor(), event.getTimestamp());
-          auditUpdates.add(audit);
-        }
-      }
-      db.accountGroupByIdAud().update(auditUpdates);
-    } catch (OrmException e) {
-      logOrmException(
-          "Cannot log delete groups from group event performed by user", event, group.getName(), e);
-    }
-  }
-
-  private void logFailToLoadUpdatedGroup(GroupAuditEvent event) {
-    ImmutableList<String> descriptions = createEventDescriptions(event, "(fail to load group)");
-    String message =
-        createErrorMessage("Fail to load the updated group", event.getActor(), descriptions);
-    log.error(message);
-  }
-
-  private void logOrmException(
-      String header, GroupAuditEvent event, String updatedGroupName, OrmException e) {
-    ImmutableList<String> descriptions = createEventDescriptions(event, updatedGroupName);
-    String message = createErrorMessage(header, event.getActor(), descriptions);
-    log.error(message, e);
-  }
-
-  private ImmutableList<String> createEventDescriptions(
-      GroupAuditEvent event, String updatedGroupName) {
-    ImmutableList.Builder<String> builder = ImmutableList.builder();
-    if (event instanceof GroupMemberAuditEvent) {
-      GroupMemberAuditEvent memberAuditEvent = (GroupMemberAuditEvent) event;
-      for (Account.Id accountId : memberAuditEvent.getModifiedMembers()) {
-        String userName = getUserName(accountId).orElse("");
-        builder.add(
-            MessageFormat.format(
-                "account {0}/{1}, group {2}/{3}",
-                accountId, userName, event.getUpdatedGroup(), updatedGroupName));
-      }
-    } else if (event instanceof GroupSubgroupAuditEvent) {
-      GroupSubgroupAuditEvent subgroupAuditEvent = (GroupSubgroupAuditEvent) event;
-      for (AccountGroup.UUID groupUuid : subgroupAuditEvent.getModifiedSubgroups()) {
-        String groupName = groupBackend.get(groupUuid).getName();
-        builder.add(
-            MessageFormat.format(
-                "group {0}/{1}, group {2}/{3}",
-                groupUuid, groupName, subgroupAuditEvent.getUpdatedGroup(), updatedGroupName));
-      }
-    }
-
-    return builder.build();
-  }
-
-  private String createErrorMessage(
-      String header, Account.Id me, ImmutableList<String> descriptions) {
-    StringBuilder message = new StringBuilder(header);
-    message.append(" ");
-    message.append(me);
-    message.append("/");
-    message.append(getUserName(me).orElse(null));
-    message.append(": ");
-    message.append(Joiner.on("; ").join(descriptions));
-    return message.toString();
-  }
-
-  private Optional<String> getUserName(Account.Id accountId) {
-    return accountCache.get(accountId).map(AccountState::getUserName).orElse(Optional.empty());
-  }
-
-  private static ImmutableSet<AccountGroupMemberAudit> toAccountGroupMemberAudits(
-      GroupMemberAuditEvent event, AccountGroup.Id updatedGroupId) {
-    Timestamp timestamp = event.getTimestamp();
-    Account.Id actor = event.getActor();
-    return event
-        .getModifiedMembers()
-        .stream()
-        .map(
-            member ->
-                new AccountGroupMemberAudit(
-                    new AccountGroupMemberAudit.Key(member, updatedGroupId, timestamp), actor))
-        .collect(toImmutableSet());
-  }
-
-  private static ImmutableSet<AccountGroupByIdAud> toAccountGroupByIdAudits(
-      GroupSubgroupAuditEvent event, AccountGroup.Id updatedGroupId) {
-    Timestamp timestamp = event.getTimestamp();
-    Account.Id actor = event.getActor();
-    return event
-        .getModifiedSubgroups()
-        .stream()
-        .map(
-            subgroup ->
-                new AccountGroupByIdAud(
-                    new AccountGroupByIdAud.Key(updatedGroupId, subgroup, timestamp), actor))
-        .collect(toImmutableSet());
-  }
-}

java/com/google/gerrit/server/group/Module.java

@@ -1,24 +0,0 @@
-package com.google.gerrit.server.group;
-
-import com.google.gerrit.extensions.config.FactoryModule;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.server.audit.group.GroupAuditListener;
-import com.google.gerrit.server.notedb.GroupsMigration;
-
-public class Module extends FactoryModule {
-  private final GroupsMigration groupsMigration;
-
-  public Module(GroupsMigration groupsMigration) {
-    this.groupsMigration = groupsMigration;
-  }
-
-  @Override
-  protected void configure() {
-    if (!groupsMigration.disableGroupReviewDb()) {
-      // DbGroupAuditListener is used solely for the ReviewDb audit log. It does not respect
-      // ReviewDb wrappers that disable reads. Hence, we don't want to bind it if ReviewDb is
-      // disabled.
-      DynamicSet.bind(binder(), GroupAuditListener.class).to(DbGroupAuditListener.class);
-    }
-  }
-}

java/com/google/gerrit/server/group/SystemGroupBackend.java

@@ -24,7 +24,6 @@ import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.StartupCheck;
 import com.google.gerrit.server.StartupException;
@@ -34,8 +33,6 @@ import com.google.gerrit.server.account.ListGroupMembership;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.group.db.Groups;
 import com.google.gerrit.server.project.ProjectState;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import java.io.IOException;
@@ -191,13 +188,11 @@ public class SystemGroupBackend extends AbstractGroupBackend {
   public static class NameCheck implements StartupCheck {
     private final Config cfg;
     private final Groups groups;
-    private final SchemaFactory<ReviewDb> schema;
 
     @Inject
-    NameCheck(@GerritServerConfig Config cfg, Groups groups, SchemaFactory<ReviewDb> schema) {
+    NameCheck(@GerritServerConfig Config cfg, Groups groups) {
       this.cfg = cfg;
       this.groups = groups;
-      this.schema = schema;
     }
 
     @Override
@@ -216,14 +211,14 @@ public class SystemGroupBackend extends AbstractGroupBackend {
       }
 
       Optional<GroupReference> conflictingGroup;
-      try (ReviewDb db = schema.open()) {
+      try {
         conflictingGroup =
             groups
-                .getAllGroupReferences(db)
+                .getAllGroupReferences()
                 .filter(group -> hasConfiguredName(byLowerCaseConfiguredName, group))
                 .findAny();
 
-      } catch (OrmException | IOException | ConfigInvalidException ignored) {
+      } catch (IOException | ConfigInvalidException ignored) {
         return;
       }
 

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

@@ -14,28 +14,15 @@
 
 package com.google.gerrit.server.group.db;
 
-import static com.google.common.collect.ImmutableSet.toImmutableSet;
-
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Streams;
 import com.google.gerrit.common.data.GroupReference;
-import com.google.gerrit.common.errors.NoSuchGroupException;
-import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.AccountGroupById;
 import com.google.gerrit.reviewdb.client.AccountGroupByIdAud;
-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.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;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import java.io.IOException;
@@ -48,10 +35,10 @@ import org.eclipse.jgit.lib.Repository;
 /**
  * A database accessor for read calls related to groups.
  *
- * <p>All calls which read group related details from the database (either ReviewDb or NoteDb) are
+ * <p>All calls which read group related details from the database are gathered here. Other classes
- * gathered here. Other classes should always use this class instead of accessing the database
+ * should always use this class instead of accessing the database directly. There are a few
- * directly. There are a few exceptions though: schema classes, wrapper classes, and classes
+ * exceptions though: schema classes, wrapper classes, and classes executed during init. The latter
- * executed during init. The latter ones should use {@code GroupsOnInit} instead.
+ * ones should use {@code GroupsOnInit} instead.
  *
  * <p>Most callers should not need to read groups directly from the database; they should use the
  * {@link com.google.gerrit.server.account.GroupCache GroupCache} instead.
@@ -60,18 +47,13 @@ import org.eclipse.jgit.lib.Repository;
  */
 @Singleton
 public class Groups {
-  private final GroupsMigration groupsMigration;
   private final GitRepositoryManager repoManager;
   private final AllUsersName allUsersName;
   private final AuditLogReader auditLogReader;
 
   @Inject
   public Groups(
-      GroupsMigration groupsMigration,
+      GitRepositoryManager repoManager, AllUsersName allUsersName, AuditLogReader auditLogReader) {
-      GitRepositoryManager repoManager,
-      AllUsersName allUsersName,
-      AuditLogReader auditLogReader) {
-    this.groupsMigration = groupsMigration;
     this.repoManager = repoManager;
     this.allUsersName = allUsersName;
     this.auditLogReader = auditLogReader;
@@ -80,27 +62,16 @@ public class Groups {
   /**
    * Returns the {@code InternalGroup} for the specified UUID if it exists.
    *
-   * @param db the {@code ReviewDb} instance to use for lookups
    * @param groupUuid the UUID of the group
    * @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)
+  public Optional<InternalGroup> getGroup(AccountGroup.UUID groupUuid)
-      throws OrmException, IOException, ConfigInvalidException {
+      throws IOException, ConfigInvalidException {
-    if (groupsMigration.readFromNoteDb()) {
+    try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
-      try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
+      return getGroupFromNoteDb(allUsersRepo, groupUuid);
-        return getGroupFromNoteDb(allUsersRepo, groupUuid);
-      }
     }
-
-    Optional<AccountGroup> accountGroup = getGroupFromReviewDb(db, groupUuid);
-    if (!accountGroup.isPresent()) {
-      return Optional.empty();
-    }
-    return Optional.of(asInternalGroup(db, accountGroup.get()));
   }
 
   private static Optional<InternalGroup> getGroupFromNoteDb(
@@ -116,130 +87,31 @@ public class Groups {
     return loadedGroup;
   }
 
-  public static InternalGroup asInternalGroup(ReviewDb db, AccountGroup accountGroup)
-      throws OrmException {
-    ImmutableSet<Account.Id> members =
-        getMembersFromReviewDb(db, accountGroup.getId()).collect(toImmutableSet());
-    ImmutableSet<AccountGroup.UUID> subgroups =
-        getSubgroupsFromReviewDb(db, accountGroup.getId()).collect(toImmutableSet());
-    return InternalGroup.create(accountGroup, members, subgroups);
-  }
-
-  /**
-   * Returns the {@code AccountGroup} for the specified UUID.
-   *
-   * @param db the {@code ReviewDb} instance to use for lookups
-   * @param groupUuid the UUID of the group
-   * @return the {@code AccountGroup} which has the specified UUID
-   * @throws OrmDuplicateKeyException if multiple groups are found for the specified UUID
-   * @throws OrmException if the group couldn't be retrieved from ReviewDb
-   * @throws NoSuchGroupException if a group with such a UUID doesn't exist
-   */
-  static AccountGroup getExistingGroupFromReviewDb(ReviewDb db, AccountGroup.UUID groupUuid)
-      throws OrmException, NoSuchGroupException {
-    Optional<AccountGroup> group = getGroupFromReviewDb(db, groupUuid);
-    return group.orElseThrow(() -> new NoSuchGroupException(groupUuid));
-  }
-
-  /**
-   * Returns the {@code AccountGroup} for the specified UUID if it exists.
-   *
-   * @param db the {@code ReviewDb} instance to use for lookups
-   * @param groupUuid the UUID of the group
-   * @return the found {@code AccountGroup} 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
-   */
-  private static Optional<AccountGroup> getGroupFromReviewDb(
-      ReviewDb db, AccountGroup.UUID groupUuid) throws OrmException {
-    List<AccountGroup> accountGroups = db.accountGroups().byUUID(groupUuid).toList();
-    if (accountGroups.size() == 1) {
-      return Optional.of(Iterables.getOnlyElement(accountGroups));
-    } else if (accountGroups.isEmpty()) {
-      return Optional.empty();
-    } else {
-      throw new OrmDuplicateKeyException("Duplicate group UUID " + groupUuid);
-    }
-  }
-
   /**
    * Returns {@code GroupReference}s for all internal groups.
    *
-   * @param db the {@code ReviewDb} instance to use for lookups
    * @return a stream of the {@code GroupReference}s of all internal groups
-   * @throws OrmException if an error occurs while reading from ReviewDb
    * @throws IOException if an error occurs while reading from NoteDb
    * @throws ConfigInvalidException if the data in NoteDb is in an incorrect format
    */
-  public Stream<GroupReference> getAllGroupReferences(ReviewDb db)
+  public Stream<GroupReference> getAllGroupReferences() throws IOException, ConfigInvalidException {
-      throws OrmException, IOException, ConfigInvalidException {
+    try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
-    if (groupsMigration.readFromNoteDb()) {
+      return GroupNameNotes.loadAllGroups(allUsersRepo).stream();
-      try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
-        return GroupNameNotes.loadAllGroups(allUsersRepo).stream();
-      }
     }
-
-    return Streams.stream(db.accountGroups().all())
-        .map(group -> new GroupReference(group.getGroupUUID(), group.getName()));
-  }
-
-  /**
-   * Returns the members (accounts) of a group.
-   *
-   * <p><strong>Note</strong>: This method doesn't check whether the accounts exist!
-   *
-   * @param db the {@code ReviewDb} instance to use for lookups
-   * @param groupId the ID of the group
-   * @return a stream of the IDs of the members
-   * @throws OrmException if an error occurs while reading from ReviewDb
-   */
-  static Stream<Account.Id> getMembersFromReviewDb(ReviewDb db, AccountGroup.Id groupId)
-      throws OrmException {
-    ResultSet<AccountGroupMember> accountGroupMembers = db.accountGroupMembers().byGroup(groupId);
-    return Streams.stream(accountGroupMembers).map(AccountGroupMember::getAccountId);
-  }
-
-  /**
-   * Returns the subgroups of a group.
-   *
-   * <p>This parent group must be an internal group whereas the subgroups can either be internal or
-   * external groups.
-   *
-   * <p><strong>Note</strong>: This method doesn't check whether the subgroups exist!
-   *
-   * @param db the {@code ReviewDb} instance to use for lookups
-   * @param groupId the ID of the group
-   * @return a stream of the UUIDs of the subgroups
-   * @throws OrmException if an error occurs while reading from ReviewDb
-   */
-  static Stream<AccountGroup.UUID> getSubgroupsFromReviewDb(ReviewDb db, AccountGroup.Id groupId)
-      throws OrmException {
-    ResultSet<AccountGroupById> accountGroupByIds = db.accountGroupById().byGroup(groupId);
-    return Streams.stream(accountGroupByIds).map(AccountGroupById::getIncludeUUID).distinct();
   }
 
   /**
    * Returns all known external groups. External groups are 'known' when they are specified as a
    * subgroup of an internal group.
    *
-   * @param db the {@code ReviewDb} instance to use for lookups
    * @return a stream of the UUIDs of the known external groups
-   * @throws OrmException if an error occurs while reading from ReviewDb
    * @throws IOException if an error occurs while reading from NoteDb
    * @throws ConfigInvalidException if the data in NoteDb is in an incorrect format
    */
-  public Stream<AccountGroup.UUID> getExternalGroups(ReviewDb db)
+  public Stream<AccountGroup.UUID> getExternalGroups() throws IOException, ConfigInvalidException {
-      throws OrmException, IOException, ConfigInvalidException {
+    try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
-    if (groupsMigration.readFromNoteDb()) {
+      return getExternalGroupsFromNoteDb(allUsersRepo);
-      try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
-        return getExternalGroupsFromNoteDb(allUsersRepo);
-      }
     }
-
-    return Streams.stream(db.accountGroupById().all())
-        .map(AccountGroupById::getIncludeUUID)
-        .distinct()
-        .filter(groupUuid -> !AccountGroup.isInternalGroup(groupUuid));
   }
 
   private static Stream<AccountGroup.UUID> getExternalGroupsFromNoteDb(Repository allUsersRepo)
@@ -259,50 +131,28 @@ public class Groups {
   /**
    * Returns the membership audit records for a given group.
    *
-   * @param db the {@code ReviewDb} instance to use for lookups
    * @param repo All-Users repository.
    * @param groupUuid the UUID of the group
    * @return the audit records, in arbitrary order; empty if the group does not exist
-   * @throws OrmException if an error occurs while reading from ReviewDb
    * @throws IOException if an error occurs while reading from NoteDb
    * @throws ConfigInvalidException if the group couldn't be retrieved from NoteDb
    */
-  public List<AccountGroupMemberAudit> getMembersAudit(
+  public List<AccountGroupMemberAudit> getMembersAudit(Repository repo, AccountGroup.UUID groupUuid)
-      ReviewDb db, Repository repo, AccountGroup.UUID groupUuid)
+      throws IOException, ConfigInvalidException {
-      throws OrmException, IOException, ConfigInvalidException {
+    return auditLogReader.getMembersAudit(repo, groupUuid);
-    if (groupsMigration.readFromNoteDb()) {
-      return auditLogReader.getMembersAudit(repo, groupUuid);
-    }
-    Optional<AccountGroup> group = getGroupFromReviewDb(db, groupUuid);
-    if (!group.isPresent()) {
-      return ImmutableList.of();
-    }
-
-    return db.accountGroupMembersAudit().byGroup(group.get().getId()).toList();
   }
 
   /**
    * Returns the subgroup audit records for a given group.
    *
-   * @param db the {@code ReviewDb} instance to use for lookups
    * @param repo All-Users repository.
    * @param groupUuid the UUID of the group
    * @return the audit records, in arbitrary order; empty if the group does not exist
-   * @throws OrmException if an error occurs while reading from ReviewDb
    * @throws IOException if an error occurs while reading from NoteDb
    * @throws ConfigInvalidException if the group couldn't be retrieved from NoteDb
    */
-  public List<AccountGroupByIdAud> getSubgroupsAudit(
+  public List<AccountGroupByIdAud> getSubgroupsAudit(Repository repo, AccountGroup.UUID groupUuid)
-      ReviewDb db, Repository repo, AccountGroup.UUID groupUuid)
+      throws IOException, ConfigInvalidException {
-      throws OrmException, IOException, ConfigInvalidException {
+    return auditLogReader.getSubgroupsAudit(repo, groupUuid);
-    if (groupsMigration.readFromNoteDb()) {
-      return auditLogReader.getSubgroupsAudit(repo, groupUuid);
-    }
-    Optional<AccountGroup> group = getGroupFromReviewDb(db, groupUuid);
-    if (!group.isPresent()) {
-      return ImmutableList.of();
-    }
-
-    return db.accountGroupByIdAud().byGroup(group.get().getId()).toList();
   }
 }

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

@@ -26,7 +26,6 @@ import com.google.gerrit.server.account.GroupBackend;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.group.InternalGroup;
-import com.google.gerrit.server.notedb.GroupsMigration;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -52,7 +51,6 @@ public class GroupsConsistencyChecker {
   private final Accounts accounts;
   private final GitRepositoryManager repoManager;
   private final GroupsNoteDbConsistencyChecker globalChecker;
-  private final GroupsMigration groupsMigration;
 
   @Inject
   GroupsConsistencyChecker(
@@ -60,22 +58,16 @@ public class GroupsConsistencyChecker {
       GroupBackend groupBackend,
       Accounts accounts,
       GitRepositoryManager repositoryManager,
-      GroupsNoteDbConsistencyChecker globalChecker,
+      GroupsNoteDbConsistencyChecker globalChecker) {
-      GroupsMigration groupsMigration) {
     this.allUsersName = allUsersName;
     this.groupBackend = groupBackend;
     this.accounts = accounts;
     this.repoManager = repositoryManager;
     this.globalChecker = globalChecker;
-    this.groupsMigration = groupsMigration;
   }
 
   /** Checks that all internal group references exist, and that no groups have cycles. */
   public List<ConsistencyProblemInfo> check() throws IOException {
-    if (!groupsMigration.writeToNoteDb()) {
-      return new ArrayList<>();
-    }
-
     try (Repository repo = repoManager.openRepository(allUsersName)) {
       GroupsNoteDbConsistencyChecker.Result result = globalChecker.check(repo);
       if (!result.problems.isEmpty()) {

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

@@ -14,14 +14,10 @@
 
 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 com.google.auto.value.AutoValue;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.common.base.Throwables;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.google.gerrit.common.Nullable;
@@ -29,12 +25,7 @@ import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.errors.NoSuchGroupException;
 import com.google.gerrit.reviewdb.client.Account;
 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.client.AccountGroupName;
 import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.reviewdb.server.ReviewDbUtil;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountCache;
@@ -51,11 +42,9 @@ 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.index.group.GroupIndexer;
-import com.google.gerrit.server.notedb.GroupsMigration;
 import com.google.gerrit.server.update.RefUpdateUtil;
 import com.google.gerrit.server.update.RetryHelper;
 import com.google.gwtorm.server.OrmDuplicateKeyException;
-import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.assistedinject.Assisted;
@@ -75,10 +64,10 @@ import org.eclipse.jgit.lib.Repository;
 /**
  * A database accessor for write calls related to groups.
  *
- * <p>All calls which write group related details to the database (either ReviewDb or NoteDb) are
+ * <p>All calls which write group related details to the database are gathered here. Other classes
- * gathered here. Other classes should always use this class instead of accessing the database
+ * should always use this class instead of accessing the database directly. There are a few
- * directly. There are a few exceptions though: schema classes, wrapper classes, and classes
+ * exceptions though: schema classes, wrapper classes, and classes executed during init. The latter
- * executed during init. The latter ones should use {@code GroupsOnInit} instead.
+ * ones should use {@code GroupsOnInit} instead.
  *
  * <p>If not explicitly stated, all methods of this class refer to <em>internal</em> groups.
  */
@@ -109,7 +98,6 @@ public class GroupsUpdate {
   private final AuditLogFormatter auditLogFormatter;
   private final PersonIdent authorIdent;
   private final MetaDataUpdateFactory metaDataUpdateFactory;
-  private final GroupsMigration groupsMigration;
   private final GitReferenceUpdated gitRefUpdated;
   private final RetryHelper retryHelper;
 
@@ -127,7 +115,6 @@ public class GroupsUpdate {
       @GerritServerId String serverId,
       @GerritPersonIdent PersonIdent serverIdent,
       MetaDataUpdate.InternalFactory metaDataUpdateInternalFactory,
-      GroupsMigration groupsMigration,
       GitReferenceUpdated gitRefUpdated,
       RetryHelper retryHelper,
       @Assisted @Nullable IdentifiedUser currentUser) {
@@ -138,7 +125,6 @@ public class GroupsUpdate {
     this.indexer = indexer;
     this.auditService = auditService;
     this.renameGroupOpFactory = renameGroupOpFactory;
-    this.groupsMigration = groupsMigration;
     this.gitRefUpdated = gitRefUpdated;
     this.retryHelper = retryHelper;
     this.currentUser = currentUser;
@@ -184,37 +170,18 @@ public class GroupsUpdate {
   /**
    * Creates the specified group for the specified members (accounts).
    *
-   * @param db the {@code ReviewDb} instance to update
    * @param groupCreation an {@code InternalGroupCreation} which specifies all mandatory properties
    *     of the group
    * @param groupUpdate an {@code InternalGroupUpdate} which specifies optional properties of the
    *     group. If this {@code InternalGroupUpdate} updates a property which was already specified
    *     by the {@code InternalGroupCreation}, the value of this {@code InternalGroupUpdate} wins.
-   * @throws OrmException if an error occurs while reading/writing from/to ReviewDb
    * @throws OrmDuplicateKeyException if a group with the chosen name already exists
    * @throws IOException if indexing fails, or an error occurs while reading/writing from/to NoteDb
    * @return the created {@code InternalGroup}
    */
   public InternalGroup createGroup(
-      ReviewDb db, InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate)
+      InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate)
-      throws OrmException, IOException, ConfigInvalidException {
+      throws OrmDuplicateKeyException, IOException, ConfigInvalidException {
-    if (!groupsMigration.disableGroupReviewDb()) {
-      if (!groupUpdate.getUpdatedOn().isPresent()) {
-        // Set updatedOn to a specific value so that the same timestamp is used for ReviewDb and
-        // NoteDb.
-        groupUpdate = groupUpdate.toBuilder().setUpdatedOn(TimeUtil.nowTs()).build();
-      }
-
-      InternalGroup createdGroupInReviewDb =
-          createGroupInReviewDb(ReviewDbUtil.unwrapDb(db), groupCreation, groupUpdate);
-
-      if (!groupsMigration.writeToNoteDb()) {
-        updateCachesOnGroupCreation(createdGroupInReviewDb);
-        dispatchAuditEventsOnGroupCreation(createdGroupInReviewDb);
-        return createdGroupInReviewDb;
-      }
-    }
-
     InternalGroup createdGroup = createGroupInNoteDbWithRetry(groupCreation, groupUpdate);
     updateCachesOnGroupCreation(createdGroup);
     dispatchAuditEventsOnGroupCreation(createdGroup);
@@ -224,67 +191,32 @@ public class GroupsUpdate {
   /**
    * Updates the specified group.
    *
-   * @param db the {@code ReviewDb} instance to update
    * @param groupUuid the UUID of the group to update
    * @param groupUpdate an {@code InternalGroupUpdate} which indicates the desired updates on the
    *     group
-   * @throws OrmException if an error occurs while reading/writing from/to ReviewDb
+   * @throws OrmDuplicateKeyException if the new name of the group is used by another group
-   * @throws com.google.gwtorm.server.OrmDuplicateKeyException if the new name of the group is used
-   *     by another group
    * @throws IOException if indexing fails, or an error occurs while reading/writing from/to NoteDb
    * @throws NoSuchGroupException if the specified group doesn't exist
    */
-  public void updateGroup(ReviewDb db, AccountGroup.UUID groupUuid, InternalGroupUpdate groupUpdate)
+  public void updateGroup(AccountGroup.UUID groupUuid, InternalGroupUpdate groupUpdate)
-      throws OrmException, IOException, NoSuchGroupException, ConfigInvalidException {
+      throws OrmDuplicateKeyException, IOException, NoSuchGroupException, ConfigInvalidException {
     Optional<Timestamp> updatedOn = groupUpdate.getUpdatedOn();
     if (!updatedOn.isPresent()) {
-      // Set updatedOn to a specific value so that the same timestamp is used for ReviewDb and
-      // NoteDb. This timestamp is also used by audit events.
       updatedOn = Optional.of(TimeUtil.nowTs());
       groupUpdate = groupUpdate.toBuilder().setUpdatedOn(updatedOn.get()).build();
     }
 
-    UpdateResult result = updateGroupInDb(db, groupUuid, groupUpdate);
+    UpdateResult result = updateGroupInDb(groupUuid, groupUpdate);
     updateCachesOnGroupUpdate(result);
     dispatchAuditEventsOnGroupUpdate(result, updatedOn.get());
   }
 
   @VisibleForTesting
-  public UpdateResult updateGroupInDb(
+  public UpdateResult updateGroupInDb(AccountGroup.UUID groupUuid, InternalGroupUpdate groupUpdate)
-      ReviewDb db, AccountGroup.UUID groupUuid, InternalGroupUpdate groupUpdate)
+      throws OrmDuplicateKeyException, NoSuchGroupException, IOException, ConfigInvalidException {
-      throws OrmException, NoSuchGroupException, IOException, ConfigInvalidException {
-    UpdateResult reviewDbUpdateResult = null;
-    if (!groupsMigration.disableGroupReviewDb()) {
-      AccountGroup group = getExistingGroupFromReviewDb(ReviewDbUtil.unwrapDb(db), groupUuid);
-      reviewDbUpdateResult = updateGroupInReviewDb(ReviewDbUtil.unwrapDb(db), group, groupUpdate);
-
-      if (!groupsMigration.writeToNoteDb()) {
-        return reviewDbUpdateResult;
-      }
-    }
-
     Optional<UpdateResult> noteDbUpdateResult =
         updateGroupInNoteDbWithRetry(groupUuid, groupUpdate);
-    return noteDbUpdateResult.orElse(reviewDbUpdateResult);
+    return noteDbUpdateResult.orElse(null);
-  }
-
-  private InternalGroup createGroupInReviewDb(
-      ReviewDb db, InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate)
-      throws OrmException {
-
-    AccountGroupName gn = new AccountGroupName(groupCreation.getNameKey(), groupCreation.getId());
-    // first insert the group name to validate that the group name hasn't
-    // already been used to create another group
-    db.accountGroupNames().insert(ImmutableList.of(gn));
-
-    Timestamp createdOn = groupUpdate.getUpdatedOn().orElseGet(TimeUtil::nowTs);
-    AccountGroup group = createAccountGroup(groupCreation, createdOn);
-    UpdateResult updateResult = updateGroupInReviewDb(db, group, groupUpdate);
-    return InternalGroup.create(
-        group,
-        updateResult.getAddedMembers(),
-        updateResult.getAddedSubgroups(),
-        updateResult.getRefState());
   }
 
   public static AccountGroup createAccountGroup(
@@ -308,157 +240,9 @@ public class GroupsUpdate {
     groupUpdate.getVisibleToAll().ifPresent(group::setVisibleToAll);
   }
 
-  private UpdateResult updateGroupInReviewDb(
-      ReviewDb db, AccountGroup group, InternalGroupUpdate groupUpdate) throws OrmException {
-    AccountGroup.NameKey originalName = group.getNameKey();
-    applyUpdate(group, groupUpdate);
-    AccountGroup.NameKey updatedName = group.getNameKey();
-
-    // The name must be inserted first so that we stop early for already used names.
-    updateNameInReviewDb(db, group.getId(), originalName, updatedName);
-    db.accountGroups().upsert(ImmutableList.of(group));
-
-    ImmutableSet<Account.Id> originalMembers =
-        Groups.getMembersFromReviewDb(db, group.getId()).collect(toImmutableSet());
-    ImmutableSet<Account.Id> updatedMembers =
-        ImmutableSet.copyOf(groupUpdate.getMemberModification().apply(originalMembers));
-    ImmutableSet<AccountGroup.UUID> originalSubgroups =
-        Groups.getSubgroupsFromReviewDb(db, group.getId()).collect(toImmutableSet());
-    ImmutableSet<AccountGroup.UUID> updatedSubgroups =
-        ImmutableSet.copyOf(groupUpdate.getSubgroupModification().apply(originalSubgroups));
-
-    Set<Account.Id> addedMembers =
-        addGroupMembersInReviewDb(db, group.getId(), originalMembers, updatedMembers);
-    Set<Account.Id> deletedMembers =
-        deleteGroupMembersInReviewDb(db, group.getId(), originalMembers, updatedMembers);
-    Set<AccountGroup.UUID> addedSubgroups =
-        addSubgroupsInReviewDb(db, group.getId(), originalSubgroups, updatedSubgroups);
-    Set<AccountGroup.UUID> deletedSubgroups =
-        deleteSubgroupsInReviewDb(db, group.getId(), originalSubgroups, updatedSubgroups);
-
-    UpdateResult.Builder resultBuilder =
-        UpdateResult.builder()
-            .setGroupUuid(group.getGroupUUID())
-            .setGroupId(group.getId())
-            .setGroupName(group.getNameKey())
-            .setAddedMembers(addedMembers)
-            .setDeletedMembers(deletedMembers)
-            .setAddedSubgroups(addedSubgroups)
-            .setDeletedSubgroups(deletedSubgroups);
-    if (!Objects.equals(originalName, updatedName)) {
-      resultBuilder.setPreviousGroupName(originalName);
-    }
-    return resultBuilder.build();
-  }
-
-  private static void updateNameInReviewDb(
-      ReviewDb db,
-      AccountGroup.Id groupId,
-      AccountGroup.NameKey originalName,
-      AccountGroup.NameKey updatedName)
-      throws OrmException {
-    try {
-      AccountGroupName id = new AccountGroupName(updatedName, groupId);
-      db.accountGroupNames().insert(ImmutableList.of(id));
-    } catch (OrmException e) {
-      AccountGroupName other = db.accountGroupNames().get(updatedName);
-      if (other != null) {
-        // If we are using this identity, don't report the exception.
-        if (other.getId().equals(groupId)) {
-          return;
-        }
-      }
-      throw e;
-    }
-    db.accountGroupNames().deleteKeys(ImmutableList.of(originalName));
-  }
-
-  private static Set<Account.Id> addGroupMembersInReviewDb(
-      ReviewDb db,
-      AccountGroup.Id groupId,
-      ImmutableSet<Account.Id> originalMembers,
-      ImmutableSet<Account.Id> updatedMembers)
-      throws OrmException {
-    Set<Account.Id> accountIds = Sets.difference(updatedMembers, originalMembers);
-    if (accountIds.isEmpty()) {
-      return accountIds;
-    }
-
-    ImmutableSet<AccountGroupMember> newMembers = toAccountGroupMembers(groupId, accountIds);
-    db.accountGroupMembers().insert(newMembers);
-    return accountIds;
-  }
-
-  private static Set<Account.Id> deleteGroupMembersInReviewDb(
-      ReviewDb db,
-      AccountGroup.Id groupId,
-      ImmutableSet<Account.Id> originalMembers,
-      ImmutableSet<Account.Id> updatedMembers)
-      throws OrmException {
-    Set<Account.Id> accountIds = Sets.difference(originalMembers, updatedMembers);
-    if (accountIds.isEmpty()) {
-      return accountIds;
-    }
-
-    ImmutableSet<AccountGroupMember> membersToRemove = toAccountGroupMembers(groupId, accountIds);
-    db.accountGroupMembers().delete(membersToRemove);
-    return accountIds;
-  }
-
-  private static ImmutableSet<AccountGroupMember> toAccountGroupMembers(
-      AccountGroup.Id groupId, Set<Account.Id> accountIds) {
-    return accountIds
-        .stream()
-        .map(accountId -> new AccountGroupMember.Key(accountId, groupId))
-        .map(AccountGroupMember::new)
-        .collect(toImmutableSet());
-  }
-
-  private static Set<AccountGroup.UUID> addSubgroupsInReviewDb(
-      ReviewDb db,
-      AccountGroup.Id parentGroupId,
-      ImmutableSet<AccountGroup.UUID> originalSubgroups,
-      ImmutableSet<AccountGroup.UUID> updatedSubgroups)
-      throws OrmException {
-    Set<AccountGroup.UUID> subgroupUuids = Sets.difference(updatedSubgroups, originalSubgroups);
-    if (subgroupUuids.isEmpty()) {
-      return subgroupUuids;
-    }
-
-    ImmutableSet<AccountGroupById> newSubgroups = toAccountGroupByIds(parentGroupId, subgroupUuids);
-    db.accountGroupById().insert(newSubgroups);
-    return subgroupUuids;
-  }
-
-  private static Set<AccountGroup.UUID> deleteSubgroupsInReviewDb(
-      ReviewDb db,
-      AccountGroup.Id parentGroupId,
-      ImmutableSet<AccountGroup.UUID> originalSubgroups,
-      ImmutableSet<AccountGroup.UUID> updatedSubgroups)
-      throws OrmException {
-    Set<AccountGroup.UUID> subgroupUuids = Sets.difference(originalSubgroups, updatedSubgroups);
-    if (subgroupUuids.isEmpty()) {
-      return subgroupUuids;
-    }
-
-    ImmutableSet<AccountGroupById> subgroupsToRemove =
-        toAccountGroupByIds(parentGroupId, subgroupUuids);
-    db.accountGroupById().delete(subgroupsToRemove);
-    return subgroupUuids;
-  }
-
-  private static ImmutableSet<AccountGroupById> toAccountGroupByIds(
-      AccountGroup.Id parentGroupId, Set<AccountGroup.UUID> subgroupUuids) {
-    return subgroupUuids
-        .stream()
-        .map(subgroupUuid -> new AccountGroupById.Key(parentGroupId, subgroupUuid))
-        .map(AccountGroupById::new)
-        .collect(toImmutableSet());
-  }
-
   private InternalGroup createGroupInNoteDbWithRetry(
       InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate)
-      throws IOException, ConfigInvalidException, OrmException {
+      throws IOException, ConfigInvalidException, OrmDuplicateKeyException {
     try {
       return retryHelper.execute(
           RetryHelper.ActionType.GROUP_UPDATE,
@@ -514,14 +298,11 @@ public class GroupsUpdate {
 
   private Optional<UpdateResult> updateGroupInNoteDb(
       AccountGroup.UUID groupUuid, InternalGroupUpdate groupUpdate)
-      throws IOException, ConfigInvalidException, OrmDuplicateKeyException, NoSuchGroupException {
+      throws IOException, ConfigInvalidException, OrmDuplicateKeyException {
     try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
       GroupConfig groupConfig = GroupConfig.loadForGroup(allUsersRepo, groupUuid);
       groupConfig.setGroupUpdate(groupUpdate, auditLogFormatter);
       if (!groupConfig.getLoadedGroup().isPresent()) {
-        if (groupsMigration.readFromNoteDb()) {
-          throw new NoSuchGroupException(groupUuid);
-        }
         return Optional.empty();
       }
 

java/com/google/gerrit/server/index/group/AllGroupsIndexer.java

@@ -24,14 +24,11 @@ import com.google.common.util.concurrent.ListeningExecutorService;
 import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.index.SiteIndexer;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.account.GroupCache;
 import com.google.gerrit.server.group.InternalGroup;
 import com.google.gerrit.server.group.db.Groups;
 import com.google.gerrit.server.group.db.GroupsNoteDbConsistencyChecker;
 import com.google.gerrit.server.index.IndexExecutor;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import java.io.IOException;
@@ -52,18 +49,15 @@ import org.slf4j.LoggerFactory;
 public class AllGroupsIndexer extends SiteIndexer<AccountGroup.UUID, InternalGroup, GroupIndex> {
   private static final Logger log = LoggerFactory.getLogger(AllGroupsIndexer.class);
 
-  private final SchemaFactory<ReviewDb> schemaFactory;
   private final ListeningExecutorService executor;
   private final GroupCache groupCache;
   private final Groups groups;
 
   @Inject
   AllGroupsIndexer(
-      SchemaFactory<ReviewDb> schemaFactory,
       @IndexExecutor(BATCH) ListeningExecutorService executor,
       GroupCache groupCache,
       Groups groups) {
-    this.schemaFactory = schemaFactory;
     this.executor = executor;
     this.groupCache = groupCache;
     this.groups = groups;
@@ -77,7 +71,7 @@ public class AllGroupsIndexer extends SiteIndexer<AccountGroup.UUID, InternalGro
     List<AccountGroup.UUID> uuids;
     try {
       uuids = collectGroups(progress);
-    } catch (OrmException | IOException | ConfigInvalidException e) {
+    } catch (IOException | ConfigInvalidException e) {
       log.error("Error collecting groups", e);
       return new SiteIndexer.Result(sw, false, 0, 0);
     }
@@ -133,13 +127,10 @@ public class AllGroupsIndexer extends SiteIndexer<AccountGroup.UUID, InternalGro
   }
 
   private List<AccountGroup.UUID> collectGroups(ProgressMonitor progress)
-      throws OrmException, IOException, ConfigInvalidException {
+      throws IOException, ConfigInvalidException {
     progress.beginTask("Collecting groups", ProgressMonitor.UNKNOWN);
-    try (ReviewDb db = schemaFactory.open()) {
+    try {
-      return groups
+      return groups.getAllGroupReferences().map(GroupReference::getUUID).collect(toImmutableList());
-          .getAllGroupReferences(db)
-          .map(GroupReference::getUUID)
-          .collect(toImmutableList());
     } finally {
       progress.endTask();
     }

java/com/google/gerrit/server/index/group/StalenessChecker.java

@@ -21,7 +21,6 @@ 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.git.GitRepositoryManager;
-import com.google.gerrit.server.notedb.GroupsMigration;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import java.io.IOException;
@@ -47,27 +46,20 @@ public class StalenessChecker {
   private final GitRepositoryManager repoManager;
   private final IndexConfig indexConfig;
   private final AllUsersName allUsers;
-  private final GroupsMigration groupsMigration;
 
   @Inject
   StalenessChecker(
       GroupIndexCollection indexes,
       GitRepositoryManager repoManager,
       IndexConfig indexConfig,
-      AllUsersName allUsers,
+      AllUsersName allUsers) {
-      GroupsMigration groupsMigration) {
     this.indexes = indexes;
     this.repoManager = repoManager;
     this.indexConfig = indexConfig;
     this.allUsers = allUsers;
-    this.groupsMigration = groupsMigration;
   }
 
   public boolean isStale(AccountGroup.UUID uuid) throws IOException {
-    if (!groupsMigration.readFromNoteDb()) {
-      return false; // This class only treats staleness for groups in NoteDb.
-    }
-
     GroupIndex i = indexes.getSearchIndex();
     if (i == null) {
       return false; // No index; caller couldn't do anything if it is stale.

java/com/google/gerrit/server/notedb/GroupsMigration.java

@@ -1,79 +0,0 @@
-// 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.DISABLE_REVIEW_DB;
-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 java.util.Set;
-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;
-  private final boolean disableGroupReviewDb;
-
-  @Inject
-  public GroupsMigration(@GerritServerConfig Config cfg) {
-    // TODO(aliceks): Remove these flags and the ReviewDb code for groups.
-    this(
-        true,
-        cfg.getBoolean(SECTION_NOTE_DB, GROUPS.key(), READ, true),
-        cfg.getBoolean(SECTION_NOTE_DB, GROUPS.key(), DISABLE_REVIEW_DB, true));
-  }
-
-  public GroupsMigration(
-      boolean writeToNoteDb, boolean readFromNoteDb, boolean disableGroupReviewDb) {
-    this.writeToNoteDb = writeToNoteDb;
-    this.readFromNoteDb = readFromNoteDb;
-    this.disableGroupReviewDb = disableGroupReviewDb;
-  }
-
-  public boolean writeToNoteDb() {
-    return writeToNoteDb;
-  }
-
-  public boolean readFromNoteDb() {
-    return readFromNoteDb;
-  }
-
-  public boolean disableGroupReviewDb() {
-    return disableGroupReviewDb;
-  }
-
-  public void setConfigValuesIfNotSetYet(Config cfg) {
-    Set<String> subsections = cfg.getSubsections(SECTION_NOTE_DB);
-    if (!subsections.contains(GROUPS.key())) {
-      cfg.setBoolean(SECTION_NOTE_DB, GROUPS.key(), WRITE, writeToNoteDb());
-      cfg.setBoolean(SECTION_NOTE_DB, GROUPS.key(), READ, readFromNoteDb());
-      cfg.setBoolean(SECTION_NOTE_DB, GROUPS.key(), DISABLE_REVIEW_DB, disableGroupReviewDb());
-    }
-  }
-}

java/com/google/gerrit/server/restapi/account/CreateAccount.java

@@ -36,7 +36,6 @@ import com.google.gerrit.extensions.restapi.TopLevelResource;
 import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.Sequences;
 import com.google.gerrit.server.UserInitiated;
 import com.google.gerrit.server.account.AccountExternalIdCreator;
@@ -67,7 +66,6 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
     CreateAccount create(String username);
   }
 
-  private final ReviewDb db;
   private final Sequences seq;
   private final GroupsCollection groupsCollection;
   private final VersionedAuthorizedKeys.Accessor authorizedKeys;
@@ -81,7 +79,6 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
 
   @Inject
   CreateAccount(
-      ReviewDb db,
       Sequences seq,
       GroupsCollection groupsCollection,
       VersionedAuthorizedKeys.Accessor authorizedKeys,
@@ -92,7 +89,6 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
       @UserInitiated Provider<GroupsUpdate> groupsUpdate,
       OutgoingEmailValidator validator,
       @Assisted String username) {
-    this.db = db;
     this.seq = seq;
     this.groupsCollection = groupsCollection;
     this.authorizedKeys = authorizedKeys;
@@ -202,6 +198,6 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
         InternalGroupUpdate.builder()
             .setMemberModification(memberIds -> Sets.union(memberIds, ImmutableSet.of(accountId)))
             .build();
-    groupsUpdate.get().updateGroup(db, groupUuid, groupUpdate);
+    groupsUpdate.get().updateGroup(groupUuid, groupUpdate);
   }
 }

java/com/google/gerrit/server/restapi/group/AddMembers.java

@@ -29,7 +29,6 @@ import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.UserInitiated;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.AccountException;
@@ -92,7 +91,6 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
   private final AccountResolver accountResolver;
   private final AccountCache accountCache;
   private final AccountLoader.Factory infoFactory;
-  private final Provider<ReviewDb> db;
   private final Provider<GroupsUpdate> groupsUpdateProvider;
 
   @Inject
@@ -103,7 +101,6 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
       AccountResolver accountResolver,
       AccountCache accountCache,
       AccountLoader.Factory infoFactory,
-      Provider<ReviewDb> db,
       @UserInitiated Provider<GroupsUpdate> groupsUpdateProvider) {
     this.accountManager = accountManager;
     this.authType = authConfig.getAuthType();
@@ -111,7 +108,6 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
     this.accountResolver = accountResolver;
     this.accountCache = accountCache;
     this.infoFactory = infoFactory;
-    this.db = db;
     this.groupsUpdateProvider = groupsUpdateProvider;
   }
 
@@ -186,7 +182,7 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
         InternalGroupUpdate.builder()
             .setMemberModification(memberIds -> Sets.union(memberIds, newMemberIds))
             .build();
-    groupsUpdateProvider.get().updateGroup(db.get(), groupUuid, groupUpdate);
+    groupsUpdateProvider.get().updateGroup(groupUuid, groupUpdate);
   }
 
   private Optional<Account> createAccountByLdap(String user) throws IOException {

java/com/google/gerrit/server/restapi/group/AddSubgroups.java

@@ -28,7 +28,6 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.UserInitiated;
 import com.google.gerrit.server.account.GroupControl;
 import com.google.gerrit.server.group.GroupResource;
@@ -75,18 +74,15 @@ public class AddSubgroups implements RestModifyView<GroupResource, Input> {
   }
 
   private final GroupsCollection groupsCollection;
-  private final Provider<ReviewDb> db;
   private final Provider<GroupsUpdate> groupsUpdateProvider;
   private final GroupJson json;
 
   @Inject
   public AddSubgroups(
       GroupsCollection groupsCollection,
-      Provider<ReviewDb> db,
       @UserInitiated Provider<GroupsUpdate> groupsUpdateProvider,
       GroupJson json) {
     this.groupsCollection = groupsCollection;
-    this.db = db;
     this.groupsUpdateProvider = groupsUpdateProvider;
     this.json = json;
   }
@@ -128,7 +124,7 @@ public class AddSubgroups implements RestModifyView<GroupResource, Input> {
         InternalGroupUpdate.builder()
             .setSubgroupModification(subgroupUuids -> Sets.union(subgroupUuids, newSubgroupUuids))
             .build();
-    groupsUpdateProvider.get().updateGroup(db.get(), parentGroupUuid, groupUpdate);
+    groupsUpdateProvider.get().updateGroup(parentGroupUuid, groupUpdate);
   }
 
   static class PutSubgroup implements RestModifyView<GroupResource, Input> {

java/com/google/gerrit/server/restapi/group/CreateGroup.java

@@ -34,7 +34,6 @@ import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.extensions.restapi.Url;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.Sequences;
@@ -75,7 +74,6 @@ public class CreateGroup implements RestModifyView<TopLevelResource, GroupInput>
 
   private final Provider<IdentifiedUser> self;
   private final PersonIdent serverIdent;
-  private final ReviewDb db;
   private final Provider<GroupsUpdate> groupsUpdateProvider;
   private final GroupCache groupCache;
   private final GroupsCollection groups;
@@ -91,7 +89,6 @@ public class CreateGroup implements RestModifyView<TopLevelResource, GroupInput>
   CreateGroup(
       Provider<IdentifiedUser> self,
       @GerritPersonIdent PersonIdent serverIdent,
-      ReviewDb db,
       @UserInitiated Provider<GroupsUpdate> groupsUpdateProvider,
       GroupCache groupCache,
       GroupsCollection groups,
@@ -104,7 +101,6 @@ public class CreateGroup implements RestModifyView<TopLevelResource, GroupInput>
       Sequences sequences) {
     this.self = self;
     this.serverIdent = serverIdent;
-    this.db = db;
     this.groupsUpdateProvider = groupsUpdateProvider;
     this.groupCache = groupCache;
     this.groups = groups;
@@ -222,7 +218,7 @@ public class CreateGroup implements RestModifyView<TopLevelResource, GroupInput>
     groupUpdateBuilder.setMemberModification(
         members -> ImmutableSet.copyOf(createGroupArgs.initialMembers));
     try {
-      return groupsUpdateProvider.get().createGroup(db, groupCreation, groupUpdateBuilder.build());
+      return groupsUpdateProvider.get().createGroup(groupCreation, groupUpdateBuilder.build());
     } catch (OrmDuplicateKeyException e) {
       throw new ResourceConflictException(
           "group '" + createGroupArgs.getGroupName() + "' already exists");

java/com/google/gerrit/server/restapi/group/DeleteMembers.java

@@ -25,7 +25,6 @@ import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.UserInitiated;
 import com.google.gerrit.server.account.GroupControl;
 import com.google.gerrit.server.group.GroupResource;
@@ -46,16 +45,12 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
 @Singleton
 public class DeleteMembers implements RestModifyView<GroupResource, Input> {
   private final AccountsCollection accounts;
-  private final Provider<ReviewDb> db;
   private final Provider<GroupsUpdate> groupsUpdateProvider;
 
   @Inject
   DeleteMembers(
-      AccountsCollection accounts,
+      AccountsCollection accounts, @UserInitiated Provider<GroupsUpdate> groupsUpdateProvider) {
-      Provider<ReviewDb> db,
-      @UserInitiated Provider<GroupsUpdate> groupsUpdateProvider) {
     this.accounts = accounts;
-    this.db = db;
     this.groupsUpdateProvider = groupsUpdateProvider;
   }
 
@@ -93,7 +88,7 @@ public class DeleteMembers implements RestModifyView<GroupResource, Input> {
         InternalGroupUpdate.builder()
             .setMemberModification(memberIds -> Sets.difference(memberIds, accountIds))
             .build();
-    groupsUpdateProvider.get().updateGroup(db.get(), groupUuid, groupUpdate);
+    groupsUpdateProvider.get().updateGroup(groupUuid, groupUpdate);
   }
 
   @Singleton

java/com/google/gerrit/server/restapi/group/DeleteSubgroups.java

@@ -25,7 +25,6 @@ import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.UserInitiated;
 import com.google.gerrit.server.account.GroupControl;
 import com.google.gerrit.server.group.GroupResource;
@@ -45,16 +44,13 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
 @Singleton
 public class DeleteSubgroups implements RestModifyView<GroupResource, Input> {
   private final GroupsCollection groupsCollection;
-  private final Provider<ReviewDb> db;
   private final Provider<GroupsUpdate> groupsUpdateProvider;
 
   @Inject
   DeleteSubgroups(
       GroupsCollection groupsCollection,
-      Provider<ReviewDb> db,
       @UserInitiated Provider<GroupsUpdate> groupsUpdateProvider) {
     this.groupsCollection = groupsCollection;
-    this.db = db;
     this.groupsUpdateProvider = groupsUpdateProvider;
   }
 
@@ -96,7 +92,7 @@ public class DeleteSubgroups implements RestModifyView<GroupResource, Input> {
             .setSubgroupModification(
                 subgroupUuids -> Sets.difference(subgroupUuids, removedSubgroupUuids))
             .build();
-    groupsUpdateProvider.get().updateGroup(db.get(), parentGroupUuid, groupUpdate);
+    groupsUpdateProvider.get().updateGroup(parentGroupUuid, groupUpdate);
   }
 
   @Singleton

java/com/google/gerrit/server/restapi/group/GetAuditLog.java

@@ -26,7 +26,6 @@ import com.google.gerrit.extensions.restapi.Url;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.AccountGroupByIdAud;
 import com.google.gerrit.reviewdb.client.AccountGroupMemberAudit;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.account.AccountLoader;
 import com.google.gerrit.server.account.GroupBackend;
 import com.google.gerrit.server.account.GroupCache;
@@ -38,7 +37,6 @@ import com.google.gerrit.server.group.InternalGroupDescription;
 import com.google.gerrit.server.group.db.Groups;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
-import com.google.inject.Provider;
 import com.google.inject.Singleton;
 import java.io.IOException;
 import java.util.ArrayList;
@@ -50,7 +48,6 @@ import org.eclipse.jgit.lib.Repository;
 
 @Singleton
 public class GetAuditLog implements RestReadView<GroupResource> {
-  private final Provider<ReviewDb> db;
   private final AccountLoader.Factory accountLoaderFactory;
   private final AllUsersName allUsers;
   private final GroupCache groupCache;
@@ -61,7 +58,6 @@ public class GetAuditLog implements RestReadView<GroupResource> {
 
   @Inject
   public GetAuditLog(
-      Provider<ReviewDb> db,
       AccountLoader.Factory accountLoaderFactory,
       AllUsersName allUsers,
       GroupCache groupCache,
@@ -69,7 +65,6 @@ public class GetAuditLog implements RestReadView<GroupResource> {
       GroupBackend groupBackend,
       Groups groups,
       GitRepositoryManager repoManager) {
-    this.db = db;
     this.accountLoaderFactory = accountLoaderFactory;
     this.allUsers = allUsers;
     this.groupCache = groupCache;
@@ -95,7 +90,7 @@ public class GetAuditLog implements RestReadView<GroupResource> {
 
     try (Repository allUsersRepo = repoManager.openRepository(allUsers)) {
       for (AccountGroupMemberAudit auditEvent :
-          groups.getMembersAudit(db.get(), allUsersRepo, group.getGroupUUID())) {
+          groups.getMembersAudit(allUsersRepo, group.getGroupUUID())) {
         AccountInfo member = accountLoader.get(auditEvent.getMemberId());
 
         auditEvents.add(
@@ -110,7 +105,7 @@ public class GetAuditLog implements RestReadView<GroupResource> {
       }
 
       for (AccountGroupByIdAud auditEvent :
-          groups.getSubgroupsAudit(db.get(), allUsersRepo, group.getGroupUUID())) {
+          groups.getSubgroupsAudit(allUsersRepo, group.getGroupUUID())) {
         AccountGroup.UUID includedGroupUUID = auditEvent.getIncludeUUID();
         Optional<InternalGroup> includedGroup = groupCache.get(includedGroupUUID);
         GroupInfo member;

java/com/google/gerrit/server/restapi/group/ListGroups.java

@@ -33,7 +33,6 @@ import com.google.gerrit.extensions.restapi.TopLevelResource;
 import com.google.gerrit.extensions.restapi.Url;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountResource;
@@ -83,7 +82,6 @@ public class ListGroups implements RestReadView<TopLevelResource> {
   private final GroupBackend groupBackend;
   private final Groups groups;
   private final GroupsCollection groupsCollection;
-  private final Provider<ReviewDb> db;
 
   private EnumSet<ListGroupsOption> options = EnumSet.noneOf(ListGroupsOption.class);
   private boolean visibleToAll;
@@ -232,8 +230,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
       final GroupsCollection groupsCollection,
       GroupJson json,
       GroupBackend groupBackend,
-      Groups groups,
+      Groups groups) {
-      Provider<ReviewDb> db) {
     this.groupCache = groupCache;
     this.groupControlFactory = groupControlFactory;
     this.genericGroupControlFactory = genericGroupControlFactory;
@@ -244,7 +241,6 @@ public class ListGroups implements RestReadView<TopLevelResource> {
     this.groupBackend = groupBackend;
     this.groups = groups;
     this.groupsCollection = groupsCollection;
-    this.db = db;
   }
 
   public void setOptions(EnumSet<ListGroupsOption> options) {
@@ -316,8 +312,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
     return groupInfos;
   }
 
-  private Stream<GroupReference> getAllExistingGroups()
+  private Stream<GroupReference> getAllExistingGroups() throws IOException, ConfigInvalidException {
-      throws OrmException, IOException, ConfigInvalidException {
     if (!projects.isEmpty()) {
       return projects
           .stream()
@@ -325,7 +320,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
           .flatMap(Collection::stream)
           .distinct();
     }
-    return groups.getAllGroupReferences(db.get());
+    return groups.getAllGroupReferences();
   }
 
   private List<GroupInfo> suggestGroups() throws OrmException, BadRequestException {
@@ -388,7 +383,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
     Pattern pattern = getRegexPattern();
     Stream<? extends GroupDescription.Internal> foundGroups =
         groups
-            .getAllGroupReferences(db.get())
+            .getAllGroupReferences()
             .filter(group -> isRelevant(pattern, group))
             .map(this::loadGroup)
             .flatMap(Streams::stream)

java/com/google/gerrit/server/restapi/group/PutDescription.java

@@ -23,7 +23,6 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.UserInitiated;
 import com.google.gerrit.server.group.GroupResource;
 import com.google.gerrit.server.group.db.GroupsUpdate;
@@ -38,13 +37,10 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
 
 @Singleton
 public class PutDescription implements RestModifyView<GroupResource, DescriptionInput> {
-  private final Provider<ReviewDb> db;
   private final Provider<GroupsUpdate> groupsUpdateProvider;
 
   @Inject
-  PutDescription(
+  PutDescription(@UserInitiated Provider<GroupsUpdate> groupsUpdateProvider) {
-      Provider<ReviewDb> db, @UserInitiated Provider<GroupsUpdate> groupsUpdateProvider) {
-    this.db = db;
     this.groupsUpdateProvider = groupsUpdateProvider;
   }
 
@@ -69,7 +65,7 @@ public class PutDescription implements RestModifyView<GroupResource, Description
       InternalGroupUpdate groupUpdate =
           InternalGroupUpdate.builder().setDescription(newDescription).build();
       try {
-        groupsUpdateProvider.get().updateGroup(db.get(), groupUuid, groupUpdate);
+        groupsUpdateProvider.get().updateGroup(groupUuid, groupUpdate);
       } catch (NoSuchGroupException e) {
         throw new ResourceNotFoundException(String.format("Group %s not found", groupUuid));
       }

java/com/google/gerrit/server/restapi/group/PutName.java

@@ -24,13 +24,11 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.UserInitiated;
 import com.google.gerrit.server.group.GroupResource;
 import com.google.gerrit.server.group.db.GroupsUpdate;
 import com.google.gerrit.server.group.db.InternalGroupUpdate;
 import com.google.gwtorm.server.OrmDuplicateKeyException;
-import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
@@ -39,19 +37,17 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
 
 @Singleton
 public class PutName implements RestModifyView<GroupResource, NameInput> {
-  private final Provider<ReviewDb> db;
   private final Provider<GroupsUpdate> groupsUpdateProvider;
 
   @Inject
-  PutName(Provider<ReviewDb> db, @UserInitiated Provider<GroupsUpdate> groupsUpdateProvider) {
+  PutName(@UserInitiated Provider<GroupsUpdate> groupsUpdateProvider) {
-    this.db = db;
     this.groupsUpdateProvider = groupsUpdateProvider;
   }
 
   @Override
   public String apply(GroupResource rsrc, NameInput input)
       throws NotInternalGroupException, AuthException, BadRequestException,
-          ResourceConflictException, ResourceNotFoundException, OrmException, IOException,
+          ResourceConflictException, ResourceNotFoundException, IOException,
           ConfigInvalidException {
     GroupDescription.Internal internalGroup =
         rsrc.asInternalGroup().orElseThrow(NotInternalGroupException::new);
@@ -74,13 +70,13 @@ public class PutName implements RestModifyView<GroupResource, NameInput> {
   }
 
   private void renameGroup(GroupDescription.Internal group, String newName)
-      throws ResourceConflictException, ResourceNotFoundException, OrmException, IOException,
+      throws ResourceConflictException, ResourceNotFoundException, IOException,
           ConfigInvalidException {
     AccountGroup.UUID groupUuid = group.getGroupUUID();
     InternalGroupUpdate groupUpdate =
         InternalGroupUpdate.builder().setName(new AccountGroup.NameKey(newName)).build();
     try {
-      groupsUpdateProvider.get().updateGroup(db.get(), groupUuid, groupUpdate);
+      groupsUpdateProvider.get().updateGroup(groupUuid, groupUpdate);
     } catch (NoSuchGroupException e) {
       throw new ResourceNotFoundException(String.format("Group %s not found", groupUuid));
     } catch (OrmDuplicateKeyException e) {

java/com/google/gerrit/server/restapi/group/PutOptions.java

@@ -22,7 +22,6 @@ import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.UserInitiated;
 import com.google.gerrit.server.group.GroupResource;
 import com.google.gerrit.server.group.db.GroupsUpdate;
@@ -36,12 +35,10 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
 
 @Singleton
 public class PutOptions implements RestModifyView<GroupResource, GroupOptionsInfo> {
-  private final Provider<ReviewDb> db;
   private final Provider<GroupsUpdate> groupsUpdateProvider;
 
   @Inject
-  PutOptions(Provider<ReviewDb> db, @UserInitiated Provider<GroupsUpdate> groupsUpdateProvider) {
+  PutOptions(@UserInitiated Provider<GroupsUpdate> groupsUpdateProvider) {
-    this.db = db;
     this.groupsUpdateProvider = groupsUpdateProvider;
   }
 
@@ -67,7 +64,7 @@ public class PutOptions implements RestModifyView<GroupResource, GroupOptionsInf
       InternalGroupUpdate groupUpdate =
           InternalGroupUpdate.builder().setVisibleToAll(input.visibleToAll).build();
       try {
-        groupsUpdateProvider.get().updateGroup(db.get(), groupUuid, groupUpdate);
+        groupsUpdateProvider.get().updateGroup(groupUuid, groupUpdate);
       } catch (NoSuchGroupException e) {
         throw new ResourceNotFoundException(String.format("Group %s not found", groupUuid));
       }

java/com/google/gerrit/server/restapi/group/PutOwner.java

@@ -25,7 +25,6 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.UserInitiated;
 import com.google.gerrit.server.group.GroupResource;
 import com.google.gerrit.server.group.db.GroupsUpdate;
@@ -41,18 +40,15 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
 public class PutOwner implements RestModifyView<GroupResource, OwnerInput> {
   private final GroupsCollection groupsCollection;
   private final Provider<GroupsUpdate> groupsUpdateProvider;
-  private final Provider<ReviewDb> db;
   private final GroupJson json;
 
   @Inject
   PutOwner(
       GroupsCollection groupsCollection,
       @UserInitiated Provider<GroupsUpdate> groupsUpdateProvider,
-      Provider<ReviewDb> db,
       GroupJson json) {
     this.groupsCollection = groupsCollection;
     this.groupsUpdateProvider = groupsUpdateProvider;
-    this.db = db;
     this.json = json;
   }
 
@@ -77,7 +73,7 @@ public class PutOwner implements RestModifyView<GroupResource, OwnerInput> {
       InternalGroupUpdate groupUpdate =
           InternalGroupUpdate.builder().setOwnerGroupUUID(owner.getGroupUUID()).build();
       try {
-        groupsUpdateProvider.get().updateGroup(db.get(), groupUuid, groupUpdate);
+        groupsUpdateProvider.get().updateGroup(groupUuid, groupUpdate);
       } catch (NoSuchGroupException e) {
         throw new ResourceNotFoundException(String.format("Group %s not found", groupUuid));
       }

java/com/google/gerrit/server/schema/NoGroupsReviewDbWrapper.java

@@ -1,202 +0,0 @@
-// 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.schema;
-
-import com.google.common.collect.ImmutableList;
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.AccountGroupById;
-import com.google.gerrit.reviewdb.client.AccountGroupByIdAud;
-import com.google.gerrit.reviewdb.client.AccountGroupMember;
-import com.google.gerrit.reviewdb.client.AccountGroupMemberAudit;
-import com.google.gerrit.reviewdb.client.AccountGroupName;
-import com.google.gerrit.reviewdb.server.AccountGroupAccess;
-import com.google.gerrit.reviewdb.server.AccountGroupByIdAccess;
-import com.google.gerrit.reviewdb.server.AccountGroupByIdAudAccess;
-import com.google.gerrit.reviewdb.server.AccountGroupMemberAccess;
-import com.google.gerrit.reviewdb.server.AccountGroupMemberAuditAccess;
-import com.google.gerrit.reviewdb.server.AccountGroupNameAccess;
-import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.reviewdb.server.ReviewDbWrapper;
-import com.google.gwtorm.server.ListResultSet;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.ResultSet;
-
-/**
- * Wrapper for ReviewDb that never calls the underlying groups tables.
- *
- * <p>See {@link NotesMigrationSchemaFactory} for discussion.
- */
-public class NoGroupsReviewDbWrapper extends ReviewDbWrapper {
-  private static <T> ResultSet<T> empty() {
-    return new ListResultSet<>(ImmutableList.of());
-  }
-
-  private final AccountGroupAccess groups;
-  private final AccountGroupNameAccess groupNames;
-  private final AccountGroupMemberAccess members;
-  private final AccountGroupMemberAuditAccess memberAudits;
-  private final AccountGroupByIdAccess byIds;
-  private final AccountGroupByIdAudAccess byIdAudits;
-
-  protected NoGroupsReviewDbWrapper(ReviewDb db) {
-    super(db);
-    this.groups = new Groups(this, delegate);
-    this.groupNames = new GroupNames(this, delegate);
-    this.members = new Members(this, delegate);
-    this.memberAudits = new MemberAudits(this, delegate);
-    this.byIds = new ByIds(this, delegate);
-    this.byIdAudits = new ByIdAudits(this, delegate);
-  }
-
-  @Override
-  public AccountGroupAccess accountGroups() {
-    return groups;
-  }
-
-  @Override
-  public AccountGroupNameAccess accountGroupNames() {
-    return groupNames;
-  }
-
-  @Override
-  public AccountGroupMemberAccess accountGroupMembers() {
-    return members;
-  }
-
-  @Override
-  public AccountGroupMemberAuditAccess accountGroupMembersAudit() {
-    return memberAudits;
-  }
-
-  @Override
-  public AccountGroupByIdAudAccess accountGroupByIdAud() {
-    return byIdAudits;
-  }
-
-  @Override
-  public AccountGroupByIdAccess accountGroupById() {
-    return byIds;
-  }
-
-  private static class Groups extends AbstractDisabledAccess<AccountGroup, AccountGroup.Id>
-      implements AccountGroupAccess {
-    private Groups(ReviewDbWrapper wrapper, ReviewDb db) {
-      super(wrapper, db.accountGroups());
-    }
-
-    @Override
-    public ResultSet<AccountGroup> byUUID(AccountGroup.UUID uuid) throws OrmException {
-      return empty();
-    }
-
-    @Override
-    public ResultSet<AccountGroup> all() throws OrmException {
-      return empty();
-    }
-  }
-
-  private static class GroupNames
-      extends AbstractDisabledAccess<AccountGroupName, AccountGroup.NameKey>
-      implements AccountGroupNameAccess {
-    private GroupNames(ReviewDbWrapper wrapper, ReviewDb db) {
-      super(wrapper, db.accountGroupNames());
-    }
-
-    @Override
-    public ResultSet<AccountGroupName> all() throws OrmException {
-      return empty();
-    }
-  }
-
-  private static class Members
-      extends AbstractDisabledAccess<AccountGroupMember, AccountGroupMember.Key>
-      implements AccountGroupMemberAccess {
-    private Members(ReviewDbWrapper wrapper, ReviewDb db) {
-      super(wrapper, db.accountGroupMembers());
-    }
-
-    @Override
-    public ResultSet<AccountGroupMember> byAccount(Account.Id id) throws OrmException {
-      return empty();
-    }
-
-    @Override
-    public ResultSet<AccountGroupMember> byGroup(AccountGroup.Id id) throws OrmException {
-      return empty();
-    }
-  }
-
-  private static class MemberAudits
-      extends AbstractDisabledAccess<AccountGroupMemberAudit, AccountGroupMemberAudit.Key>
-      implements AccountGroupMemberAuditAccess {
-    private MemberAudits(ReviewDbWrapper wrapper, ReviewDb db) {
-      super(wrapper, db.accountGroupMembersAudit());
-    }
-
-    @Override
-    public ResultSet<AccountGroupMemberAudit> byGroupAccount(
-        AccountGroup.Id groupId, com.google.gerrit.reviewdb.client.Account.Id accountId)
-        throws OrmException {
-      return empty();
-    }
-
-    @Override
-    public ResultSet<AccountGroupMemberAudit> byGroup(AccountGroup.Id groupId) throws OrmException {
-      return empty();
-    }
-  }
-
-  private static class ByIds extends AbstractDisabledAccess<AccountGroupById, AccountGroupById.Key>
-      implements AccountGroupByIdAccess {
-    private ByIds(ReviewDbWrapper wrapper, ReviewDb db) {
-      super(wrapper, db.accountGroupById());
-    }
-
-    @Override
-    public ResultSet<AccountGroupById> byIncludeUUID(AccountGroup.UUID uuid) throws OrmException {
-      return empty();
-    }
-
-    @Override
-    public ResultSet<AccountGroupById> byGroup(AccountGroup.Id id) throws OrmException {
-      return empty();
-    }
-
-    @Override
-    public ResultSet<AccountGroupById> all() throws OrmException {
-      return empty();
-    }
-  }
-
-  private static class ByIdAudits
-      extends AbstractDisabledAccess<AccountGroupByIdAud, AccountGroupByIdAud.Key>
-      implements AccountGroupByIdAudAccess {
-    private ByIdAudits(ReviewDbWrapper wrapper, ReviewDb db) {
-      super(wrapper, db.accountGroupByIdAud());
-    }
-
-    @Override
-    public ResultSet<AccountGroupByIdAud> byGroupInclude(
-        AccountGroup.Id groupId, AccountGroup.UUID incGroupUUID) throws OrmException {
-      return empty();
-    }
-
-    @Override
-    public ResultSet<AccountGroupByIdAud> byGroup(AccountGroup.Id groupId) throws OrmException {
-      return empty();
-    }
-  }
-}

java/com/google/gerrit/server/schema/NotesMigrationSchemaFactory.java

@@ -15,9 +15,7 @@
 package com.google.gerrit.server.schema;
 
 import com.google.gerrit.reviewdb.server.DisallowReadFromChangesReviewDbWrapper;
-import com.google.gerrit.reviewdb.server.DisallowReadFromGroupsReviewDbWrapper;
 import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.notedb.GroupsMigration;
 import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.SchemaFactory;
@@ -28,16 +26,12 @@ import com.google.inject.Singleton;
 public class NotesMigrationSchemaFactory implements SchemaFactory<ReviewDb> {
   private final SchemaFactory<ReviewDb> delegate;
   private final NotesMigration migration;
-  private final GroupsMigration groupsMigration;
 
   @Inject
   NotesMigrationSchemaFactory(
-      @ReviewDbFactory SchemaFactory<ReviewDb> delegate,
+      @ReviewDbFactory SchemaFactory<ReviewDb> delegate, NotesMigration migration) {
-      NotesMigration migration,
-      GroupsMigration groupsMigration) {
     this.delegate = delegate;
     this.migration = migration;
-    this.groupsMigration = groupsMigration;
   }
 
   @Override
@@ -73,23 +67,12 @@ public class NotesMigrationSchemaFactory implements SchemaFactory<ReviewDb> {
       db = new NoChangesReviewDbWrapper(db);
     }
 
-    if (groupsMigration.readFromNoteDb() && groupsMigration.disableGroupReviewDb()) {
-      // Disable writes to group tables in ReviewDb (ReviewDb access for groups are No-Ops).
-      db = new NoGroupsReviewDbWrapper(db);
-    }
-
     // Second create the wrappers which can be removed by ReviewDbUtil#unwrapDb(ReviewDb).
     if (migration.readChanges()) {
       // If reading changes from NoteDb is configured, changes should not be read from ReviewDb.
       // Make sure that any attempt to read a change from ReviewDb anyway fails with an exception.
       db = new DisallowReadFromChangesReviewDbWrapper(db);
     }
-
-    if (groupsMigration.readFromNoteDb()) {
-      // If reading groups from NoteDb is configured, groups should not be read from ReviewDb.
-      // Make sure that any attempt to read a group from ReviewDb anyway fails with an exception.
-      db = new DisallowReadFromGroupsReviewDbWrapper(db);
-    }
     return db;
   }
 }

java/com/google/gerrit/server/schema/SchemaCreator.java

@@ -14,13 +14,11 @@
 
 package com.google.gerrit.server.schema;
 
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.metrics.MetricMaker;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.AccountGroupName;
 import com.google.gerrit.reviewdb.client.CurrentSchemaVersion;
 import com.google.gerrit.reviewdb.client.SystemConfig;
 import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -40,12 +38,10 @@ import com.google.gerrit.server.group.InternalGroup;
 import com.google.gerrit.server.group.db.AuditLogFormatter;
 import com.google.gerrit.server.group.db.GroupConfig;
 import com.google.gerrit.server.group.db.GroupNameNotes;
-import com.google.gerrit.server.group.db.GroupsUpdate;
 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;
@@ -73,7 +69,6 @@ public class SchemaCreator {
   private final PersonIdent serverUser;
   private final DataSourceType dataSourceType;
   private final GroupIndexCollection indexCollection;
-  private final GroupsMigration groupsMigration;
   private final String serverId;
 
   private final Config config;
@@ -91,7 +86,6 @@ public class SchemaCreator {
       @GerritPersonIdent PersonIdent au,
       DataSourceType dst,
       GroupIndexCollection ic,
-      GroupsMigration gm,
       @GerritServerId String serverId,
       @GerritServerConfig Config config,
       MetricMaker metricMaker,
@@ -106,7 +100,6 @@ public class SchemaCreator {
         au,
         dst,
         ic,
-        gm,
         serverId,
         config,
         metricMaker,
@@ -123,7 +116,6 @@ public class SchemaCreator {
       @GerritPersonIdent PersonIdent au,
       DataSourceType dst,
       GroupIndexCollection ic,
-      GroupsMigration gm,
       String serverId,
       Config config,
       MetricMaker metricMaker,
@@ -137,7 +129,6 @@ public class SchemaCreator {
     serverUser = au;
     dataSourceType = dst;
     indexCollection = ic;
-    groupsMigration = gm;
     this.serverId = serverId;
 
     this.config = config;
@@ -176,25 +167,24 @@ public class SchemaCreator {
             allUsersName,
             metricMaker);
     try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
-      createAdminsGroup(db, seqs, allUsersRepo, admins);
+      createAdminsGroup(seqs, allUsersRepo, admins);
-      createBatchUsersGroup(db, seqs, allUsersRepo, batchUsers, admins.getUUID());
+      createBatchUsersGroup(seqs, allUsersRepo, batchUsers, admins.getUUID());
     }
 
     dataSourceType.getIndexScript().run(db);
   }
 
   private void createAdminsGroup(
-      ReviewDb db, Sequences seqs, Repository allUsersRepo, GroupReference groupReference)
+      Sequences seqs, Repository allUsersRepo, GroupReference groupReference)
       throws OrmException, IOException, ConfigInvalidException {
     InternalGroupCreation groupCreation = getGroupCreation(seqs, groupReference);
     InternalGroupUpdate groupUpdate =
         InternalGroupUpdate.builder().setDescription("Gerrit Site Administrators").build();
 
-    createGroup(db, allUsersRepo, groupCreation, groupUpdate);
+    createGroup(allUsersRepo, groupCreation, groupUpdate);
   }
 
   private void createBatchUsersGroup(
-      ReviewDb db,
       Sequences seqs,
       Repository allUsersRepo,
       GroupReference groupReference,
@@ -207,35 +197,16 @@ public class SchemaCreator {
             .setOwnerGroupUUID(adminsGroupUuid)
             .build();
 
-    createGroup(db, allUsersRepo, groupCreation, groupUpdate);
+    createGroup(allUsersRepo, groupCreation, groupUpdate);
   }
 
   private void createGroup(
-      ReviewDb db,
+      Repository allUsersRepo, InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate)
-      Repository allUsersRepo,
-      InternalGroupCreation groupCreation,
-      InternalGroupUpdate groupUpdate)
       throws OrmException, ConfigInvalidException, IOException {
-    InternalGroup groupInReviewDb = createGroupInReviewDb(db, groupCreation, groupUpdate);
-
-    if (!groupsMigration.writeToNoteDb()) {
-      index(groupInReviewDb);
-      return;
-    }
-
     InternalGroup createdGroup = createGroupInNoteDb(allUsersRepo, groupCreation, groupUpdate);
     index(createdGroup);
   }
 
-  private static InternalGroup createGroupInReviewDb(
-      ReviewDb db, InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate)
-      throws OrmException {
-    AccountGroup group = GroupsUpdate.createAccountGroup(groupCreation, groupUpdate);
-    db.accountGroupNames().insert(ImmutableList.of(new AccountGroupName(group)));
-    db.accountGroups().insert(ImmutableList.of(group));
-    return InternalGroup.create(group, ImmutableSet.of(), ImmutableSet.of());
-  }
-
   private InternalGroup createGroupInNoteDb(
       Repository allUsersRepo, InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate)
       throws ConfigInvalidException, IOException, OrmDuplicateKeyException {

java/com/google/gerrit/sshd/commands/RenameGroupCommand.java

@@ -23,7 +23,6 @@ import com.google.gerrit.server.restapi.group.GroupsCollection;
 import com.google.gerrit.server.restapi.group.PutName;
 import com.google.gerrit.sshd.CommandMetaData;
 import com.google.gerrit.sshd.SshCommand;
-import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import java.io.IOException;
 import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -53,7 +52,7 @@ public class RenameGroupCommand extends SshCommand {
       NameInput input = new NameInput();
       input.name = newGroupName;
       putName.apply(rsrc, input);
-    } catch (RestApiException | OrmException | IOException | ConfigInvalidException e) {
+    } catch (RestApiException | IOException | ConfigInvalidException e) {
       throw die(e);
     }
   }

java/com/google/gerrit/testing/GroupNoteDbMode.java

@@ -1,78 +0,0 @@
-// 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.testing;
-
-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.GroupsMigration;
-
-public enum GroupNoteDbMode {
-  /** NoteDb is disabled, groups are only in ReviewDb */
-  OFF(new GroupsMigration(false, false, false)),
-
-  /** Writing new groups to NoteDb is enabled. */
-  WRITE(new GroupsMigration(true, false, false)),
-
-  /**
-   * Reading/writing groups from/to NoteDb is enabled. Trying to read groups from ReviewDb throws an
-   * exception.
-   */
-  READ_WRITE(new GroupsMigration(true, true, false)),
-
-  /**
-   * All group tables in ReviewDb are entirely disabled. Trying to read groups from ReviewDb throws
-   * an exception. Reading groups through an unwrapped ReviewDb instance writing groups to ReviewDb
-   * is a No-Op.
-   */
-  ON(new GroupsMigration(true, true, true));
-
-  private static final String ENV_VAR = "GERRIT_NOTEDB_GROUPS";
-  private static final String SYS_PROP = "gerrit.notedb.groups";
-
-  public static GroupNoteDbMode get() {
-    String value = System.getenv(ENV_VAR);
-    if (Strings.isNullOrEmpty(value)) {
-      value = System.getProperty(SYS_PROP);
-    }
-    if (Strings.isNullOrEmpty(value)) {
-      return OFF;
-    }
-    value = value.toUpperCase().replace("-", "_");
-    GroupNoteDbMode mode = Enums.getIfPresent(GroupNoteDbMode.class, value).orNull();
-    if (!Strings.isNullOrEmpty(System.getenv(ENV_VAR))) {
-      checkArgument(
-          mode != null, "Invalid value for env variable %s: %s", ENV_VAR, System.getenv(ENV_VAR));
-    } else {
-      checkArgument(
-          mode != null,
-          "Invalid value for system property %s: %s",
-          SYS_PROP,
-          System.getProperty(SYS_PROP));
-    }
-    return mode;
-  }
-
-  private final GroupsMigration groupsMigration;
-
-  private GroupNoteDbMode(GroupsMigration groupsMigration) {
-    this.groupsMigration = groupsMigration;
-  }
-
-  public GroupsMigration getGroupsMigration() {
-    return groupsMigration;
-  }
-}

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

@@ -15,11 +15,6 @@
 package com.google.gerrit.acceptance.api.group;
 
 import static com.google.common.truth.Truth.assertWithMessage;
-import static com.google.gerrit.server.notedb.NoteDbTable.GROUPS;
-import static com.google.gerrit.server.notedb.NotesMigration.DISABLE_REVIEW_DB;
-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 static com.google.gerrit.truth.ListSubject.assertThat;
 import static com.google.gerrit.truth.OptionalSubject.assertThat;
 
@@ -29,7 +24,6 @@ import com.google.gerrit.extensions.api.GerritApi;
 import com.google.gerrit.extensions.common.GroupInfo;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ServerInitiated;
 import com.google.gerrit.server.account.GroupCache;
 import com.google.gerrit.server.group.InternalGroup;
@@ -48,27 +42,15 @@ import java.io.IOException;
 import java.util.List;
 import java.util.Optional;
 import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Config;
 import org.junit.Rule;
 import org.junit.Test;
 
 public class GroupIndexerIT {
-  private static Config createPureNoteDbConfig() {
+  @Rule public InMemoryTestEnvironment testEnvironment = new InMemoryTestEnvironment();
-    Config config = new Config();
-    config.setBoolean(SECTION_NOTE_DB, GROUPS.key(), WRITE, true);
-    config.setBoolean(SECTION_NOTE_DB, GROUPS.key(), READ, true);
-    config.setBoolean(SECTION_NOTE_DB, GROUPS.key(), DISABLE_REVIEW_DB, true);
-    return config;
-  }
-
-  @Rule
-  public InMemoryTestEnvironment testEnvironment =
-      new InMemoryTestEnvironment(GroupIndexerIT::createPureNoteDbConfig);
 
   @Inject private GroupIndexer groupIndexer;
   @Inject private GerritApi gApi;
   @Inject private GroupCache groupCache;
-  @Inject private ReviewDb db;
   @Inject @ServerInitiated private GroupsUpdate groupsUpdate;
   @Inject private Provider<InternalGroupQuery> groupQueryProvider;
 
@@ -176,7 +158,7 @@ public class GroupIndexerIT {
   private void updateGroupWithoutCacheOrIndex(
       AccountGroup.UUID groupUuid, InternalGroupUpdate groupUpdate)
       throws OrmException, NoSuchGroupException, IOException, ConfigInvalidException {
-    groupsUpdate.updateGroupInDb(db, groupUuid, groupUpdate);
+    groupsUpdate.updateGroupInDb(groupUuid, groupUpdate);
   }
 
   private static OptionalSubject<InternalGroupSubject, InternalGroup> assertThatGroup(

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

@@ -15,9 +15,7 @@
 package com.google.gerrit.acceptance.api.group;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.TruthJUnit.assume;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
-import static com.google.gerrit.server.notedb.NoteDbTable.GROUPS;
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
@@ -32,11 +30,7 @@ import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.group.db.GroupConfig;
 import com.google.gerrit.server.group.db.GroupNameNotes;
 import com.google.gerrit.server.group.db.testing.GroupTestUtil;
-import com.google.gerrit.server.notedb.GroupsMigration;
-import com.google.gerrit.server.notedb.NotesMigration;
-import com.google.gerrit.testing.ConfigSuite;
 import java.util.List;
-import javax.inject.Inject;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.RefRename;
 import org.eclipse.jgit.lib.RefUpdate;
@@ -53,17 +47,6 @@ import org.junit.Test;
 @Sandboxed
 @NoHttpd
 public class GroupsConsistencyIT extends AbstractDaemonTest {
-
-  @ConfigSuite.Config
-  public static Config noteDbConfig() {
-    Config config = new Config();
-    config.setBoolean(NotesMigration.SECTION_NOTE_DB, GROUPS.key(), NotesMigration.WRITE, true);
-    config.setBoolean(NotesMigration.SECTION_NOTE_DB, GROUPS.key(), NotesMigration.READ, true);
-    return config;
-  }
-
-  @Inject private GroupsMigration groupsMigration;
-
   private GroupInfo gAdmin;
   private GroupInfo g1;
   private GroupInfo g2;
@@ -72,7 +55,6 @@ public class GroupsConsistencyIT extends AbstractDaemonTest {
 
   @Before
   public void basicSetup() throws Exception {
-    assume().that(groupsInNoteDb()).isTrue();
     allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
 
     String name1 = createGroup("g1");
@@ -87,10 +69,6 @@ public class GroupsConsistencyIT extends AbstractDaemonTest {
     this.gAdmin = gApi.groups().id("Administrators").detail();
   }
 
-  private boolean groupsInNoteDb() {
-    return groupsMigration.writeToNoteDb();
-  }
-
   @Test
   public void allGood() throws Exception {
     assertThat(check()).isEmpty();

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

@@ -16,18 +16,12 @@ package com.google.gerrit.acceptance.api.group;
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.TruthJUnit.assume;
 import static com.google.gerrit.acceptance.GitUtil.deleteRef;
 import static com.google.gerrit.acceptance.GitUtil.fetch;
 import static com.google.gerrit.acceptance.api.group.GroupAssert.assertGroupInfo;
 import static com.google.gerrit.acceptance.rest.account.AccountAssert.assertAccountInfos;
 import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
-import static com.google.gerrit.server.notedb.NoteDbTable.GROUPS;
-import static com.google.gerrit.server.notedb.NotesMigration.DISABLE_REVIEW_DB;
-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 static java.lang.annotation.ElementType.METHOD;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
 import static java.util.stream.Collectors.toList;
@@ -85,7 +79,6 @@ import com.google.gerrit.server.group.db.InternalGroupUpdate;
 import com.google.gerrit.server.index.group.GroupIndexer;
 import com.google.gerrit.server.index.group.StalenessChecker;
 import com.google.gerrit.server.util.MagicBranch;
-import com.google.gerrit.testing.ConfigSuite;
 import com.google.gerrit.testing.TestTimeUtil;
 import com.google.inject.Inject;
 import java.io.IOException;
@@ -119,21 +112,6 @@ import org.junit.Test;
 
 @NoHttpd
 public class GroupsIT extends AbstractDaemonTest {
-  @ConfigSuite.Config
-  public static Config noteDbConfig() {
-    Config config = new Config();
-    config.setBoolean(SECTION_NOTE_DB, GROUPS.key(), WRITE, true);
-    config.setBoolean(SECTION_NOTE_DB, GROUPS.key(), READ, true);
-    return config;
-  }
-
-  @ConfigSuite.Config
-  public static Config disableReviewDb() {
-    Config config = noteDbConfig();
-    config.setBoolean(SECTION_NOTE_DB, GROUPS.key(), DISABLE_REVIEW_DB, true);
-    return config;
-  }
-
   @Inject private Groups groups;
   @Inject @ServerInitiated private GroupsUpdate groupsUpdate;
   @Inject private GroupIncludeCache groupIncludeCache;
@@ -732,7 +710,7 @@ public class GroupsIT extends AbstractDaemonTest {
   @Test
   public void listAllGroups() throws Exception {
     List<String> expectedGroups =
-        groups.getAllGroupReferences(db).map(GroupReference::getName).sorted().collect(toList());
+        groups.getAllGroupReferences().map(GroupReference::getName).sorted().collect(toList());
     assertThat(expectedGroups.size()).isAtLeast(2);
     assertThat(gApi.groups().list().getAsMap().keySet())
         .containsExactlyElementsIn(expectedGroups)
@@ -908,8 +886,6 @@ public class GroupsIT extends AbstractDaemonTest {
   @Sandboxed
   @IgnoreGroupInconsistencies
   public void getAuditLogAfterDeletingASubgroup() throws Exception {
-    assume().that(readGroupsFromNoteDb()).isTrue();
-
     GroupInfo parentGroup = gApi.groups().create(name("parent-group")).get();
 
     // Creates a subgroup and adds it to "parent-group" as a subgroup.
@@ -973,7 +949,6 @@ public class GroupsIT extends AbstractDaemonTest {
 
   @Test
   public void pushToGroupBranchIsRejectedForAllUsersRepo() throws Exception {
-    assume().that(groupsInNoteDb()).isTrue(); // branch only exists when groups are in NoteDb
     assertPushToGroupBranch(
         allUsers, RefNames.refsGroups(adminGroupUuid()), "group update not allowed");
   }
@@ -989,7 +964,6 @@ public class GroupsIT extends AbstractDaemonTest {
 
   @Test
   public void pushToGroupNamesBranchIsRejectedForAllUsersRepo() throws Exception {
-    assume().that(groupsInNoteDb()).isTrue(); // branch only exists when groups are in NoteDb
     // refs/meta/group-names isn't usually available for fetch, so grant ACCESS_DATABASE
     allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
     assertPushToGroupBranch(allUsers, RefNames.REFS_GROUPNAMES, "group update not allowed");
@@ -1114,8 +1088,6 @@ public class GroupsIT extends AbstractDaemonTest {
   @Test
   @IgnoreGroupInconsistencies
   public void cannotCreateGroupNamesBranch() throws Exception {
-    assume().that(groupsInNoteDb()).isTrue();
-
     // Use ProjectResetter to restore the group names ref
     try (ProjectResetter resetter =
         projectResetter
@@ -1155,7 +1127,6 @@ public class GroupsIT extends AbstractDaemonTest {
 
   @Test
   public void cannotDeleteGroupBranch() throws Exception {
-    assume().that(groupsInNoteDb()).isTrue();
     testCannotDeleteGroupBranch(RefNames.REFS_GROUPS + "*", RefNames.refsGroups(adminGroupUuid()));
   }
 
@@ -1168,8 +1139,6 @@ public class GroupsIT extends AbstractDaemonTest {
 
   @Test
   public void cannotDeleteGroupNamesBranch() throws Exception {
-    assume().that(groupsInNoteDb()).isTrue();
-
     // refs/meta/group-names is only visible with ACCESS_DATABASE
     allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
 
@@ -1199,8 +1168,6 @@ public class GroupsIT extends AbstractDaemonTest {
   @Test
   @IgnoreGroupInconsistencies
   public void stalenessChecker() throws Exception {
-    assume().that(readGroupsFromNoteDb()).isTrue();
-
     // Newly created group is not stale
     GroupInfo groupInfo = gApi.groups().create(name("foo")).get();
     AccountGroup.UUID groupUuid = new AccountGroup.UUID(groupInfo.id);
@@ -1246,8 +1213,6 @@ public class GroupsIT extends AbstractDaemonTest {
   @Test
   @Sandboxed
   public void groupsOfUserCanBeListedInSlaveMode() throws Exception {
-    assume().that(readGroupsFromNoteDb()).isTrue();
-
     GroupInput groupInput = new GroupInput();
     groupInput.name = name("contributors");
     groupInput.members = ImmutableList.of(user.username);
@@ -1267,11 +1232,8 @@ public class GroupsIT extends AbstractDaemonTest {
   @GerritConfig(name = "index.autoReindexIfStale", value = "false")
   @IgnoreGroupInconsistencies
   public void reindexGroupsInSlaveMode() throws Exception {
-    assume().that(readGroupsFromNoteDb()).isTrue();
-    assume().that(cfg.getBoolean(SECTION_NOTE_DB, GROUPS.key(), DISABLE_REVIEW_DB, false)).isTrue();
-
     List<AccountGroup.UUID> expectedGroups =
-        groups.getAllGroupReferences(db).map(GroupReference::getUUID).collect(toList());
+        groups.getAllGroupReferences().map(GroupReference::getUUID).collect(toList());
     assertThat(expectedGroups.size()).isAtLeast(2);
 
     // Restart the server as slave, on startup of the slave all groups are indexed.
@@ -1303,7 +1265,7 @@ public class GroupsIT extends AbstractDaemonTest {
       // Update a group without updating the cache or index,
       // then run the reindexer -> only the updated group is reindexed.
       groupsUpdate.updateGroupInDb(
-          db, groupUuid, InternalGroupUpdate.builder().setDescription("bar").build());
+          groupUuid, InternalGroupUpdate.builder().setDescription("bar").build());
       slaveGroupIndexer.run();
       groupIndexedCounter.assertReindexOf(groupUuid);
 
@@ -1328,10 +1290,8 @@ public class GroupsIT extends AbstractDaemonTest {
   @GerritConfig(name = "index.autoReindexIfStale", value = "false")
   @IgnoreGroupInconsistencies
   public void disabledReindexGroupsOnStartupSlaveMode() throws Exception {
-    assume().that(readGroupsFromNoteDb()).isTrue();
-
     List<AccountGroup.UUID> expectedGroups =
-        groups.getAllGroupReferences(db).map(GroupReference::getUUID).collect(toList());
+        groups.getAllGroupReferences().map(GroupReference::getUUID).collect(toList());
     assertThat(expectedGroups.size()).isAtLeast(2);
 
     restartAsSlave();
@@ -1360,8 +1320,6 @@ public class GroupsIT extends AbstractDaemonTest {
 
   private void pushToGroupBranchForReviewAndSubmit(
       Project.NameKey project, String groupRef, String expectedError) throws Exception {
-    assume().that(groupsInNoteDb()).isTrue(); // branch only exists when groups are in NoteDb
-
     grantLabel(
         "Code-Review", -2, 2, project, RefNames.REFS_GROUPS + "*", false, REGISTERED_USERS, false);
     grant(project, RefNames.REFS_GROUPS + "*", Permission.SUBMIT, false, REGISTERED_USERS);
@@ -1484,14 +1442,6 @@ public class GroupsIT extends AbstractDaemonTest {
     }
   }
 
-  private boolean groupsInNoteDb() {
-    return cfg.getBoolean(SECTION_NOTE_DB, GROUPS.key(), WRITE, false);
-  }
-
-  private boolean readGroupsFromNoteDb() {
-    return groupsInNoteDb() && cfg.getBoolean(SECTION_NOTE_DB, GROUPS.key(), READ, false);
-  }
-
   @Target({METHOD})
   @Retention(RUNTIME)
   private @interface IgnoreGroupInconsistencies {}

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

@@ -15,17 +15,11 @@
 package com.google.gerrit.acceptance.api.group;
 
 import static com.google.common.truth.Truth8.assertThat;
-import static com.google.gerrit.server.notedb.NoteDbTable.GROUPS;
-import static com.google.gerrit.server.notedb.NotesMigration.DISABLE_REVIEW_DB;
-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.ImmutableSet;
 import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ServerInitiated;
 import com.google.gerrit.server.group.db.Groups;
 import com.google.gerrit.server.group.db.GroupsUpdate;
@@ -39,27 +33,14 @@ import java.io.IOException;
 import java.util.Set;
 import java.util.stream.Stream;
 import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Config;
 import org.junit.Rule;
 import org.junit.Test;
 
 public class GroupsUpdateIT {
-
+  @Rule public InMemoryTestEnvironment testEnvironment = new InMemoryTestEnvironment();
-  private static Config createPureNoteDbConfig() {
-    Config config = new Config();
-    config.setBoolean(SECTION_NOTE_DB, GROUPS.key(), WRITE, true);
-    config.setBoolean(SECTION_NOTE_DB, GROUPS.key(), READ, true);
-    config.setBoolean(SECTION_NOTE_DB, GROUPS.key(), DISABLE_REVIEW_DB, true);
-    return config;
-  }
-
-  @Rule
-  public InMemoryTestEnvironment testEnvironment =
-      new InMemoryTestEnvironment(GroupsUpdateIT::createPureNoteDbConfig);
 
   @Inject @ServerInitiated private Provider<GroupsUpdate> groupsUpdateProvider;
   @Inject private Groups groups;
-  @Inject private ReviewDb reviewDb;
 
   @Test
   public void groupCreationIsRetriedWhenFailedDueToConcurrentNameModification() throws Exception {
@@ -100,17 +81,16 @@ public class GroupsUpdateIT {
 
   private void createGroup(InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate)
       throws OrmException, IOException, ConfigInvalidException {
-    groupsUpdateProvider.get().createGroup(reviewDb, groupCreation, groupUpdate);
+    groupsUpdateProvider.get().createGroup(groupCreation, groupUpdate);
   }
 
   private void updateGroup(AccountGroup.UUID groupUuid, InternalGroupUpdate groupUpdate)
       throws Exception {
-    groupsUpdateProvider.get().updateGroup(reviewDb, groupUuid, groupUpdate);
+    groupsUpdateProvider.get().updateGroup(groupUuid, groupUpdate);
   }
 
-  private Stream<String> getAllGroupNames()
+  private Stream<String> getAllGroupNames() throws IOException, ConfigInvalidException {
-      throws OrmException, IOException, ConfigInvalidException {
+    return groups.getAllGroupReferences().map(GroupReference::getName);
-    return groups.getAllGroupReferences(reviewDb).map(GroupReference::getName);
   }
 
   private static InternalGroupCreation getGroupCreation(String groupName, String groupUuid) {
@@ -145,7 +125,7 @@ public class GroupsUpdateIT {
       InternalGroupCreation groupCreation = getGroupCreation(groupName, groupName + "-UUID");
       InternalGroupUpdate groupUpdate = InternalGroupUpdate.builder().build();
       try {
-        groupsUpdateProvider.get().createGroup(reviewDb, groupCreation, groupUpdate);
+        groupsUpdateProvider.get().createGroup(groupCreation, groupUpdate);
       } catch (OrmException | IOException | ConfigInvalidException e) {
         throw new IllegalStateException(e);
       }

javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java

@@ -359,7 +359,7 @@ public abstract class AbstractQueryGroupsTest extends GerritServerTests {
     AccountGroup.UUID groupUuid = new AccountGroup.UUID(group1.id);
     InternalGroupUpdate groupUpdate =
         InternalGroupUpdate.builder().setDescription(newDescription).build();
-    groupsUpdateProvider.get().updateGroupInDb(db, groupUuid, groupUpdate);
+    groupsUpdateProvider.get().updateGroupInDb(groupUuid, groupUpdate);
 
     assertQuery("description:" + group1.description, group1);
     assertQuery("description:" + newDescription);

javatests/com/google/gerrit/server/schema/SchemaUpdaterTest.java

@@ -33,7 +33,6 @@ import com.google.gerrit.server.config.GerritServerId;
 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;
@@ -119,7 +118,6 @@ public class SchemaUpdaterTest {
 
                     bind(SystemGroupBackend.class);
                     install(new NotesMigration.Module());
-                    install(new GroupsMigration.Module());
                     bind(MetricMaker.class).to(DisabledMetricMaker.class);
                   }
                 })

javatests/com/google/gerrit/server/schema/Schema_150_to_151_Test.java

@@ -18,17 +18,20 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.TruthJUnit.assume;
 
 import com.google.gerrit.common.TimeUtil;
-import com.google.gerrit.extensions.api.groups.GroupInput;
+import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.extensions.common.GroupInfo;
-import com.google.gerrit.extensions.restapi.TopLevelResource;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.AccountGroup.Id;
 import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.restapi.group.CreateGroup;
+import com.google.gerrit.reviewdb.server.ReviewDbWrapper;
+import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.Sequences;
+import com.google.gerrit.server.account.GroupUUID;
 import com.google.gerrit.testing.InMemoryTestEnvironment;
 import com.google.gerrit.testing.TestUpdateUI;
 import com.google.gwtorm.jdbc.JdbcSchema;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
@@ -37,6 +40,7 @@ import java.time.Instant;
 import java.time.LocalDateTime;
 import java.time.Month;
 import java.time.ZoneOffset;
+import org.eclipse.jgit.lib.PersonIdent;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
@@ -46,14 +50,22 @@ public class Schema_150_to_151_Test {
 
   @Rule public InMemoryTestEnvironment testEnv = new InMemoryTestEnvironment();
 
-  @Inject private CreateGroup.Factory createGroupFactory;
   @Inject private Schema_151 schema151;
   @Inject private ReviewDb db;
+  @Inject private IdentifiedUser currentUser;
+  @Inject private @GerritPersonIdent Provider<PersonIdent> serverIdent;
+  @Inject private Sequences seq;
 
   private Connection connection;
   private PreparedStatement createdOnRetrieval;
   private PreparedStatement createdOnUpdate;
   private PreparedStatement auditEntryDeletion;
+  private JdbcSchema jdbcSchema;
+
+  @Before
+  public void unwrapDb() {
+    jdbcSchema = ReviewDbWrapper.unwrapJbdcSchema(db);
+  }
 
   @Before
   public void setUp() throws Exception {
@@ -87,7 +99,7 @@ public class Schema_150_to_151_Test {
   @Test
   public void createdOnIsPopulatedForGroupsCreatedAfterAudit() throws Exception {
     Timestamp testStartTime = TimeUtil.nowTs();
-    AccountGroup.Id groupId = createGroup("Group for schema migration");
+    AccountGroup.Id groupId = createGroupInReviewDb("Group for schema migration");
     setCreatedOnToVeryOldTimestamp(groupId);
 
     schema151.migrateData(db, new TestUpdateUI());
@@ -98,7 +110,7 @@ public class Schema_150_to_151_Test {
 
   @Test
   public void createdOnIsPopulatedForGroupsCreatedBeforeAudit() throws Exception {
-    AccountGroup.Id groupId = createGroup("Ancient group for schema migration");
+    AccountGroup.Id groupId = createGroupInReviewDb("Ancient group for schema migration");
     setCreatedOnToVeryOldTimestamp(groupId);
     removeAuditEntriesFor(groupId);
 
@@ -108,12 +120,16 @@ public class Schema_150_to_151_Test {
     assertThat(createdOn).isEqualTo(AccountGroup.auditCreationInstantTs());
   }
 
-  private AccountGroup.Id createGroup(String name) throws Exception {
+  private AccountGroup.Id createGroupInReviewDb(String name) throws Exception {
-    GroupInput groupInput = new GroupInput();
+    AccountGroup group =
-    groupInput.name = name;
+        new AccountGroup(
-    GroupInfo groupInfo =
+            new AccountGroup.NameKey(name),
-        createGroupFactory.create(name).apply(TopLevelResource.INSTANCE, groupInput);
+            new AccountGroup.Id(seq.nextGroupId()),
-    return new Id(groupInfo.groupId);
+            GroupUUID.make(name, serverIdent.get()),
+            TimeUtil.nowTs());
+    storeInReviewDb(group);
+    addMembersInReviewDb(group.getId(), currentUser.getAccountId());
+    return group.getId();
   }
 
   private Timestamp getCreatedOn(Id groupId) throws Exception {
@@ -138,4 +154,69 @@ public class Schema_150_to_151_Test {
     auditEntryDeletion.setInt(1, groupId.get());
     auditEntryDeletion.executeUpdate();
   }
+
+  private void storeInReviewDb(AccountGroup... groups) throws Exception {
+    try (PreparedStatement stmt =
+        jdbcSchema
+            .getConnection()
+            .prepareStatement(
+                "INSERT INTO account_groups"
+                    + " (group_uuid,"
+                    + " group_id,"
+                    + " name,"
+                    + " description,"
+                    + " created_on,"
+                    + " owner_group_uuid,"
+                    + " visible_to_all) VALUES (?, ?, ?, ?, ?, ?, ?)")) {
+      for (AccountGroup group : groups) {
+        stmt.setString(1, group.getGroupUUID().get());
+        stmt.setInt(2, group.getId().get());
+        stmt.setString(3, group.getName());
+        stmt.setString(4, group.getDescription());
+        stmt.setTimestamp(5, group.getCreatedOn());
+        stmt.setString(6, group.getOwnerGroupUUID().get());
+        stmt.setString(7, group.isVisibleToAll() ? "Y" : "N");
+        stmt.addBatch();
+      }
+      stmt.executeBatch();
+    }
+  }
+
+  private void addMembersInReviewDb(AccountGroup.Id groupId, Account.Id... memberIds)
+      throws Exception {
+    try (PreparedStatement addMemberStmt =
+            jdbcSchema
+                .getConnection()
+                .prepareStatement(
+                    "INSERT INTO account_group_members"
+                        + " (group_id,"
+                        + " account_id) VALUES ("
+                        + groupId.get()
+                        + ", ?)");
+        PreparedStatement addMemberAuditStmt =
+            jdbcSchema
+                .getConnection()
+                .prepareStatement(
+                    "INSERT INTO account_group_members_audit"
+                        + " (group_id,"
+                        + " account_id,"
+                        + " added_by,"
+                        + " added_on) VALUES ("
+                        + groupId.get()
+                        + ", ?, "
+                        + currentUser.getAccountId().get()
+                        + ", ?)")) {
+      Timestamp addedOn = TimeUtil.nowTs();
+      for (Account.Id memberId : memberIds) {
+        addMemberStmt.setInt(1, memberId.get());
+        addMemberStmt.addBatch();
+
+        addMemberAuditStmt.setInt(1, memberId.get());
+        addMemberAuditStmt.setTimestamp(2, addedOn);
+        addMemberAuditStmt.addBatch();
+      }
+      addMemberStmt.executeBatch();
+      addMemberAuditStmt.executeBatch();
+    }
+  }
 }

javatests/com/google/gerrit/server/schema/Schema_166_to_167_WithGroupsInNoteDbTest.java

@@ -87,7 +87,6 @@ public class Schema_166_to_167_WithGroupsInNoteDbTest {
     // disableReviewDb == true)
     InternalGroup internalGroup =
         groupsUpdate.createGroup(
-            db,
             InternalGroupCreation.builder()
                 .setNameKey(new AccountGroup.NameKey("users"))
                 .setGroupUUID(new AccountGroup.UUID("users"))

javatests/com/google/gerrit/server/schema/Schema_166_to_167_WithGroupsInReviewDbTest.java

@@ -39,6 +39,7 @@ import com.google.gerrit.extensions.common.GroupAuditEventInfo.GroupMemberAuditE
 import com.google.gerrit.extensions.common.GroupAuditEventInfo.Type;
 import com.google.gerrit.extensions.common.GroupAuditEventInfo.UserMemberAuditEventInfo;
 import com.google.gerrit.extensions.common.GroupInfo;
+import com.google.gerrit.extensions.common.GroupOptionsInfo;
 import com.google.gerrit.extensions.registration.DynamicSet;
 import com.google.gerrit.extensions.restapi.IdString;
 import com.google.gerrit.extensions.restapi.RestApiException;
@@ -49,8 +50,10 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.reviewdb.server.ReviewDbWrapper;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.Sequences;
 import com.google.gerrit.server.account.GroupBackend;
 import com.google.gerrit.server.account.GroupMembership;
+import com.google.gerrit.server.account.GroupUUID;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.config.GerritServerId;
 import com.google.gerrit.server.config.GerritServerIdProvider;
@@ -89,10 +92,8 @@ import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevSort;
@@ -129,6 +130,7 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
   @Inject private GroupBundle.Factory groupBundleFactory;
   @Inject private GroupBackend groupBackend;
   @Inject private DynamicSet<GroupBackend> backends;
+  @Inject private Sequences seq;
 
   private JdbcSchema jdbcSchema;
 
@@ -169,6 +171,7 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
     groupInput.name = "verifiers";
     groupInput.description = "old";
     GroupInfo group1 = gApi.groups().create(groupInput).get();
+    storeInReviewDb(group1);
 
     // Update group only in ReviewDb
     AccountGroup group1InReviewDb = getFromReviewDb(new AccountGroup.Id(group1.groupId));
@@ -177,6 +180,7 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
 
     // Create a second group in NoteDb and ReviewDb
     GroupInfo group2 = gApi.groups().create("contributors").get();
+    storeInReviewDb(group2);
 
     executeSchemaMigration(schema167, group1, group2);
 
@@ -193,8 +197,9 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
 
   @Test
   public void adminGroupIsMigratedToNoteDb() throws Exception {
-    // Administrators group is automatically created for all Gerrit servers.
+    // Administrators group is automatically created for all Gerrit servers (NoteDb only).
     GroupInfo adminGroup = gApi.groups().id("Administrators").get();
+    storeInReviewDb(adminGroup);
 
     executeSchemaMigration(schema167, adminGroup);
 
@@ -206,8 +211,9 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
 
   @Test
   public void nonInteractiveUsersGroupIsMigratedToNoteDb() throws Exception {
-    // 'Non-Interactive Users' group is automatically created for all Gerrit servers.
+    // 'Non-Interactive Users' group is automatically created for all Gerrit servers (NoteDb only).
     GroupInfo nonInteractiveUsersGroup = gApi.groups().id("Non-Interactive Users").get();
+    storeInReviewDb(nonInteractiveUsersGroup);
 
     executeSchemaMigration(schema167, nonInteractiveUsersGroup);
 
@@ -219,6 +225,11 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
 
   @Test
   public void groupsAreConsistentAfterMigrationToNoteDb() throws Exception {
+    // Administrators group are automatically created for all Gerrit servers (NoteDb only).
+    GroupInfo adminGroup = gApi.groups().id("Administrators").get();
+    GroupInfo nonInteractiveUsersGroup = gApi.groups().id("Non-Interactive Users").get();
+    storeInReviewDb(adminGroup, nonInteractiveUsersGroup);
+
     AccountGroup group1 = newGroup().setName("verifiers").build();
     AccountGroup group2 = newGroup().setName("contributors").build();
     storeInReviewDb(group1, group2);
@@ -344,7 +355,7 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
     storeInReviewDb(group);
     Account.Id member1 = new Account.Id(23456);
     Account.Id member2 = new Account.Id(93483);
-    addMembersInReviewDb(group, member1, member2);
+    addMembersInReviewDb(group.getId(), member1, member2);
 
     executeSchemaMigration(schema167, group);
 
@@ -370,29 +381,26 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
   public void logFormatWithAccountsAndGerritGroups() throws Exception {
     AccountInfo user1 = createAccount("user1");
     AccountInfo user2 = createAccount("user2");
-    GroupInfo group1 = gApi.groups().create("group1").get();
+
-    GroupInfo group2 = gApi.groups().create("group2").get();
+    AccountGroup group1 = createInReviewDb("group1");
-    GroupInfo group3 = gApi.groups().create("group3").get();
+    AccountGroup group2 = createInReviewDb("group2");
+    AccountGroup group3 = createInReviewDb("group3");
 
     // Add some accounts
     try (TempClockStep step = TestTimeUtil.freezeClock()) {
-      gApi.groups()
+      addMembersInReviewDb(
-          .id(group1.id)
+          group1.getId(), new Account.Id(user1._accountId), new Account.Id(user2._accountId));
-          .addMembers(Integer.toString(user1._accountId), Integer.toString(user2._accountId));
     }
     TimeUtil.nowTs();
 
     // Add some Gerrit groups
     try (TempClockStep step = TestTimeUtil.freezeClock()) {
-      gApi.groups().id(group1.id).addGroups(group2.id, group3.id);
+      addSubgroupsInReviewDb(group1.getId(), group2.getGroupUUID(), group3.getGroupUUID());
     }
 
-    AccountGroup.UUID group1Uuid = new AccountGroup.UUID(group1.id);
-    deleteGroupRefs(group1Uuid);
-
     executeSchemaMigration(schema167, group1, group2, group3);
 
-    GroupBundle noteDbBundle = readGroupBundleFromNoteDb(group1Uuid);
+    GroupBundle noteDbBundle = readGroupBundleFromNoteDb(group1.getGroupUUID());
 
     ImmutableList<CommitInfo> log = log(group1);
     assertThat(log).hasSize(4);
@@ -438,14 +446,15 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
         .isEqualTo(
             "Update group\n"
                 + "\n"
-                + ("Add-group: " + group2.name + " <" + group2.id + ">\n")
+                + ("Add-group: " + group2.getName() + " <" + group2.getGroupUUID().get() + ">\n")
-                + ("Add-group: " + group3.name + " <" + group3.id + ">"));
+                + ("Add-group: " + group3.getName() + " <" + group3.getGroupUUID().get() + ">"));
     assertThat(log.get(3)).author().name().isEqualTo(currentUser.getName());
     assertThat(log.get(3)).author().email().isEqualTo(currentUser.getAccountId() + "@" + serverId);
     assertThat(log.get(3)).committer().hasSameDateAs(log.get(3).author);
 
     // Verify that audit log is correctly read by Gerrit
-    List<? extends GroupAuditEventInfo> auditEvents = gApi.groups().id(group1.id).auditLog();
+    List<? extends GroupAuditEventInfo> auditEvents =
+        gApi.groups().id(group1.getGroupUUID().get()).auditLog();
     assertThat(auditEvents).hasSize(5);
     AccountInfo currentUserInfo = gApi.accounts().id(currentUser.getAccountId().get()).get();
     assertMemberAuditEvent(
@@ -462,27 +471,22 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
         auditEvents.get(0),
         Type.ADD_GROUP,
         currentUser.getAccountId(),
-        group2,
+        toGroupInfo(group2),
-        group3);
+        toGroupInfo(group3));
   }
 
   @Test
   public void logFormatWithSystemGroups() throws Exception {
-    GroupInfo group = gApi.groups().create("group1").get();
+    AccountGroup group = createInReviewDb("group");
 
     try (TempClockStep step = TestTimeUtil.freezeClock()) {
-      gApi.groups()
+      addSubgroupsInReviewDb(
-          .id(group.id)
+          group.getId(), SystemGroupBackend.ANONYMOUS_USERS, SystemGroupBackend.REGISTERED_USERS);
-          .addGroups(
-              SystemGroupBackend.ANONYMOUS_USERS.get(), SystemGroupBackend.REGISTERED_USERS.get());
     }
 
-    AccountGroup.UUID groupUuid = new AccountGroup.UUID(group.id);
-    deleteGroupRefs(groupUuid);
-
     executeSchemaMigration(schema167, group);
 
-    GroupBundle noteDbBundle = readGroupBundleFromNoteDb(groupUuid);
+    GroupBundle noteDbBundle = readGroupBundleFromNoteDb(group.getGroupUUID());
 
     ImmutableList<CommitInfo> log = log(group);
     assertThat(log).hasSize(3);
@@ -523,7 +527,8 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
     assertThat(log.get(2)).committer().hasSameDateAs(log.get(2).author);
 
     // Verify that audit log is correctly read by Gerrit
-    List<? extends GroupAuditEventInfo> auditEvents = gApi.groups().id(group.id).auditLog();
+    List<? extends GroupAuditEventInfo> auditEvents =
+        gApi.groups().id(group.getGroupUUID().get()).auditLog();
     assertThat(auditEvents).hasSize(3);
     AccountInfo currentUserInfo = gApi.accounts().id(currentUser.getAccountId().get()).get();
     assertMemberAuditEvent(
@@ -539,20 +544,17 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
 
   @Test
   public void logFormatWithExternalGroup() throws Exception {
-    GroupInfo group = gApi.groups().create("group").get();
+    AccountGroup group = createInReviewDb("group");
 
     backends.add(new TestGroupBackend());
     AccountGroup.UUID subgroupUuid = TestGroupBackend.createUuuid("foo");
 
     assertThat(groupBackend.handles(subgroupUuid)).isTrue();
-    addSubgroupsInReviewDb(new AccountGroup.Id(group.groupId), subgroupUuid);
+    addSubgroupsInReviewDb(group.getId(), subgroupUuid);
-
-    AccountGroup.UUID groupUuid = new AccountGroup.UUID(group.id);
-    deleteGroupRefs(groupUuid);
 
     executeSchemaMigration(schema167, group);
 
-    GroupBundle noteDbBundle = readGroupBundleFromNoteDb(groupUuid);
+    GroupBundle noteDbBundle = readGroupBundleFromNoteDb(group.getGroupUUID());
 
     ImmutableList<CommitInfo> log = log(group);
     assertThat(log).hasSize(3);
@@ -599,7 +601,8 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
     assertThat(log.get(2)).committer().hasSameDateAs(log.get(2).author);
 
     // Verify that audit log is correctly read by Gerrit
-    List<? extends GroupAuditEventInfo> auditEvents = gApi.groups().id(group.id).auditLog();
+    List<? extends GroupAuditEventInfo> auditEvents =
+        gApi.groups().id(group.getGroupUUID().get()).auditLog();
     assertThat(auditEvents).hasSize(2);
     AccountInfo currentUserInfo = gApi.accounts().id(currentUser.getAccountId().get()).get();
     assertMemberAuditEvent(
@@ -613,19 +616,16 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
 
   @Test
   public void logFormatWithNonExistingExternalGroup() throws Exception {
-    GroupInfo group = gApi.groups().create("group").get();
+    AccountGroup group = createInReviewDb("group");
 
     AccountGroup.UUID subgroupUuid = new AccountGroup.UUID("notExisting:foo");
 
     assertThat(groupBackend.handles(subgroupUuid)).isFalse();
-    addSubgroupsInReviewDb(new AccountGroup.Id(group.groupId), subgroupUuid);
+    addSubgroupsInReviewDb(group.getId(), subgroupUuid);
-
-    AccountGroup.UUID groupUuid = new AccountGroup.UUID(group.id);
-    deleteGroupRefs(groupUuid);
 
     executeSchemaMigration(schema167, group);
 
-    GroupBundle noteDbBundle = readGroupBundleFromNoteDb(groupUuid);
+    GroupBundle noteDbBundle = readGroupBundleFromNoteDb(group.getGroupUUID());
 
     ImmutableList<CommitInfo> log = log(group);
     assertThat(log).hasSize(3);
@@ -665,7 +665,8 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
     assertThat(log.get(2)).committer().hasSameDateAs(log.get(2).author);
 
     // Verify that audit log is correctly read by Gerrit
-    List<? extends GroupAuditEventInfo> auditEvents = gApi.groups().id(group.id).auditLog();
+    List<? extends GroupAuditEventInfo> auditEvents =
+        gApi.groups().id(group.getGroupUUID().get()).auditLog();
     assertThat(auditEvents).hasSize(2);
     AccountInfo currentUserInfo = gApi.accounts().id(currentUser.getAccountId().get()).get();
     assertMemberAuditEvent(
@@ -681,6 +682,25 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
     return TestGroup.builder();
   }
 
+  private AccountGroup createInReviewDb(String groupName) throws Exception {
+    AccountGroup group =
+        new AccountGroup(
+            new AccountGroup.NameKey(groupName),
+            new AccountGroup.Id(seq.nextGroupId()),
+            GroupUUID.make(groupName, serverIdent),
+            TimeUtil.nowTs());
+    storeInReviewDb(group);
+    addMembersInReviewDb(group.getId(), currentUser.getAccountId());
+    return group;
+  }
+
+  private void storeInReviewDb(GroupInfo... groups) throws Exception {
+    storeInReviewDb(
+        Arrays.stream(groups)
+            .map(Schema_166_to_167_WithGroupsInReviewDbTest::toAccountGroup)
+            .toArray(AccountGroup[]::new));
+  }
+
   private void storeInReviewDb(AccountGroup... groups) throws Exception {
     try (PreparedStatement stmt =
         jdbcSchema
@@ -772,21 +792,41 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
     }
   }
 
-  private void addMembersInReviewDb(AccountGroup group, Account.Id... memberIds) throws Exception {
+  private void addMembersInReviewDb(AccountGroup.Id groupId, Account.Id... memberIds)
-    try (PreparedStatement stmt =
+      throws Exception {
-        jdbcSchema
+    try (PreparedStatement addMemberStmt =
-            .getConnection()
+            jdbcSchema
-            .prepareStatement(
+                .getConnection()
-                "INSERT INTO account_group_members"
+                .prepareStatement(
-                    + " (group_id,"
+                    "INSERT INTO account_group_members"
-                    + " account_id) VALUES ("
+                        + " (group_id,"
-                    + group.getId().get()
+                        + " account_id) VALUES ("
-                    + ", ?)")) {
+                        + groupId.get()
+                        + ", ?)");
+        PreparedStatement addMemberAuditStmt =
+            jdbcSchema
+                .getConnection()
+                .prepareStatement(
+                    "INSERT INTO account_group_members_audit"
+                        + " (group_id,"
+                        + " account_id,"
+                        + " added_by,"
+                        + " added_on) VALUES ("
+                        + groupId.get()
+                        + ", ?, "
+                        + currentUser.getAccountId().get()
+                        + ", ?)")) {
+      Timestamp addedOn = TimeUtil.nowTs();
       for (Account.Id memberId : memberIds) {
-        stmt.setInt(1, memberId.get());
+        addMemberStmt.setInt(1, memberId.get());
-        stmt.addBatch();
+        addMemberStmt.addBatch();
+
+        addMemberAuditStmt.setInt(1, memberId.get());
+        addMemberAuditStmt.setTimestamp(2, addedOn);
+        addMemberAuditStmt.addBatch();
       }
-      stmt.executeBatch();
+      addMemberStmt.executeBatch();
+      addMemberAuditStmt.executeBatch();
     }
   }
 
@@ -841,21 +881,6 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
     }
   }
 
-  private void deleteGroupRefs(AccountGroup.UUID groupUuid) throws Exception {
-    try (Repository repo = gitRepoManager.openRepository(allUsersName)) {
-      String refName = RefNames.refsGroups(groupUuid);
-      RefUpdate ru = repo.updateRef(refName);
-      ru.setForceUpdate(true);
-      Ref oldRef = repo.exactRef(refName);
-      if (oldRef == null) {
-        return;
-      }
-      ru.setExpectedOldObjectId(oldRef.getObjectId());
-      ru.setNewObjectId(ObjectId.zeroId());
-      assertThat(ru.delete()).isEqualTo(RefUpdate.Result.FORCED);
-    }
-  }
-
   private void executeSchemaMigration(SchemaVersion schema, AccountGroup... groupsToVerify)
       throws Exception {
     executeSchemaMigration(
@@ -892,12 +917,12 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
     assertThat(GroupBundle.compareWithAudits(expectedReviewDbBundle, noteDbBundle)).isEmpty();
   }
 
-  private ImmutableList<CommitInfo> log(GroupInfo g) throws Exception {
+  private ImmutableList<CommitInfo> log(AccountGroup group) throws Exception {
     ImmutableList.Builder<CommitInfo> result = ImmutableList.builder();
     List<Date> commitDates = new ArrayList<>();
     try (Repository allUsersRepo = gitRepoManager.openRepository(allUsersName);
         RevWalk rw = new RevWalk(allUsersRepo)) {
-      Ref ref = allUsersRepo.exactRef(RefNames.refsGroups(new AccountGroup.UUID(g.id)));
+      Ref ref = allUsersRepo.exactRef(RefNames.refsGroups(group.getGroupUUID()));
       if (ref != null) {
         rw.sort(RevSort.REVERSE);
         rw.setRetainBody(true);
@@ -1030,6 +1055,35 @@ public class Schema_166_to_167_WithGroupsInReviewDbTest {
     return groupInfo;
   }
 
+  private static AccountGroup toAccountGroup(GroupInfo info) {
+    AccountGroup group =
+        new AccountGroup(
+            new AccountGroup.NameKey(info.name),
+            new AccountGroup.Id(info.groupId),
+            new AccountGroup.UUID(info.id),
+            info.createdOn);
+    group.setDescription(info.description);
+    if (info.ownerId != null) {
+      group.setOwnerGroupUUID(new AccountGroup.UUID(info.ownerId));
+    }
+    group.setVisibleToAll(
+        info.options != null && info.options.visibleToAll != null && info.options.visibleToAll);
+    return group;
+  }
+
+  private static GroupInfo toGroupInfo(AccountGroup group) {
+    GroupInfo groupInfo = new GroupInfo();
+    groupInfo.id = group.getGroupUUID().get();
+    groupInfo.groupId = group.getId().get();
+    groupInfo.name = group.getName();
+    groupInfo.createdOn = group.getCreatedOn();
+    groupInfo.description = group.getDescription();
+    groupInfo.owner = group.getOwnerGroupUUID().get();
+    groupInfo.options = new GroupOptionsInfo();
+    groupInfo.options.visibleToAll = group.isVisibleToAll() ? true : null;
+    return groupInfo;
+  }
+
   private static class TestGroupBackend implements GroupBackend {
     static final String PREFIX = "testbackend:";