Remove @SecondaryKey from AccountGroup

Like our prior changes, we need to drop these @SecondaryKey
annotations for databases which don't support multiple keys
on a single entity.

For the external name attribute we simply change it to honor
a list of groups which match the external name.  This allows
an administrator to create multiple groups in Gerrit that use
the same underlying LDAP group for membership.  Its crazy to
do, but there isn't really any good reason to not allow it.

For the internal name attribute we create a new entity that
can be used to enforce uniqueness on the name attribute, and
connects the name to the group.

Change-Id: I933c38a6a4e2c3ed3d7d5a66cab04c2e7175e24f
Signed-off-by: Shawn O. Pearce <sop@google.com>

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountModule.java

@@ -16,7 +16,6 @@ package com.google.gerrit.httpd.rpc.account;
 
 import com.google.gerrit.httpd.rpc.RpcServletModule;
 import com.google.gerrit.httpd.rpc.UiRpcModule;
-import com.google.gerrit.server.account.ChangeUserName;
 import com.google.gerrit.server.config.FactoryModule;
 
 public class AccountModule extends RpcServletModule {
@@ -30,10 +29,12 @@ public class AccountModule extends RpcServletModule {
       @Override
       protected void configure() {
         factory(AgreementInfoFactory.Factory.class);
+        factory(CreateGroup.Factory.class);
         factory(DeleteExternalIds.Factory.class);
         factory(ExternalIdDetailFactory.Factory.class);
         factory(GroupDetailFactory.Factory.class);
         factory(MyGroupsFactory.Factory.class);
+        factory(RenameGroup.Factory.class);
       }
     });
     rpc(AccountSecurityImpl.class);

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/CreateGroup.java

@@ -0,0 +1,89 @@
+// Copyright (C) 2009 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.httpd.rpc.account;
+
+import com.google.gerrit.common.errors.NameAlreadyUsedException;
+import com.google.gerrit.httpd.rpc.Handler;
+import com.google.gerrit.reviewdb.Account;
+import com.google.gerrit.reviewdb.AccountGroup;
+import com.google.gerrit.reviewdb.AccountGroupMember;
+import com.google.gerrit.reviewdb.AccountGroupMemberAudit;
+import com.google.gerrit.reviewdb.AccountGroupName;
+import com.google.gerrit.reviewdb.ReviewDb;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gwtorm.client.OrmDuplicateKeyException;
+import com.google.gwtorm.client.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+
+import java.util.Collections;
+
+class CreateGroup extends Handler<AccountGroup.Id> {
+  interface Factory {
+    CreateGroup create(String newName);
+  }
+
+  private final ReviewDb db;
+  private final IdentifiedUser user;
+  private final AccountCache accountCache;
+
+  private final String name;
+
+  @Inject
+  CreateGroup(final ReviewDb db, final IdentifiedUser user,
+      final AccountCache accountCache,
+
+      @Assisted final String newName) {
+    this.db = db;
+    this.user = user;
+    this.accountCache = accountCache;
+
+    this.name = newName;
+  }
+
+  @Override
+  public AccountGroup.Id call() throws OrmException, NameAlreadyUsedException {
+    final AccountGroup.NameKey key = new AccountGroup.NameKey(name);
+
+    if (db.accountGroupNames().get(key) != null) {
+      throw new NameAlreadyUsedException();
+    }
+
+    final AccountGroup.Id id = new AccountGroup.Id(db.nextAccountGroupId());
+    final Account.Id me = user.getAccountId();
+
+    final AccountGroup group = new AccountGroup(key, id);
+    db.accountGroups().insert(Collections.singleton(group));
+
+    try {
+      final AccountGroupName n = new AccountGroupName(key, id);
+      db.accountGroupNames().insert(Collections.singleton(n));
+    } catch (OrmDuplicateKeyException dupeErr) {
+      db.accountGroups().delete(Collections.singleton(group));
+      throw new NameAlreadyUsedException();
+    }
+
+    AccountGroupMember member = new AccountGroupMember(//
+        new AccountGroupMember.Key(me, id));
+
+    db.accountGroupMembersAudit().insert(
+        Collections.singleton(new AccountGroupMemberAudit(member, me)));
+    db.accountGroupMembers().insert(Collections.singleton(member));
+
+    accountCache.evict(me);
+    return id;
+  }
+}

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java

@@ -54,6 +54,9 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
   private final Realm accountRealm;
   private final GroupCache groupCache;
   private final GroupControl.Factory groupControlFactory;
+
+  private final CreateGroup.Factory createGroupFactory;
+  private final RenameGroup.Factory renameGroupFactory;
   private final GroupDetailFactory.Factory groupDetailFactory;
 
   @Inject
@@ -62,6 +65,8 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
       final AccountCache accountCache, final AccountResolver accountResolver,
       final Realm accountRealm, final GroupCache groupCache,
       final GroupControl.Factory groupControlFactory,
+      final CreateGroup.Factory createGroupFactory,
+      final RenameGroup.Factory renameGroupFactory,
       final GroupDetailFactory.Factory groupDetailFactory) {
     super(schema, currentUser);
     this.identifiedUser = currentUser;
@@ -70,6 +75,8 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
     this.accountRealm = accountRealm;
     this.groupCache = groupCache;
     this.groupControlFactory = groupControlFactory;
+    this.createGroupFactory = createGroupFactory;
+    this.renameGroupFactory = renameGroupFactory;
     this.groupDetailFactory = groupDetailFactory;
   }
 
@@ -112,37 +119,7 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
 
   public void createGroup(final String newName,
       final AsyncCallback<AccountGroup.Id> callback) {
-    run(callback, new Action<AccountGroup.Id>() {
+    createGroupFactory.create(newName).to(callback);
-      public AccountGroup.Id run(final ReviewDb db) throws OrmException,
-          Failure {
-        final AccountGroup.NameKey nameKey = new AccountGroup.NameKey(newName);
-        if (db.accountGroups().get(nameKey) != null) {
-          throw new Failure(new NameAlreadyUsedException());
-        }
-
-        final AccountGroup group =
-            new AccountGroup(nameKey, new AccountGroup.Id(db
-                .nextAccountGroupId()));
-        group.setNameKey(nameKey);
-        group.setType(AccountGroup.Type.INTERNAL);
-        group.setDescription("");
-
-        final Account.Id me = getAccountId();
-        final AccountGroupMember m =
-            new AccountGroupMember(
-                new AccountGroupMember.Key(me, group.getId()));
-
-        final Transaction txn = db.beginTransaction();
-        db.accountGroups().insert(Collections.singleton(group), txn);
-        db.accountGroupMembers().insert(Collections.singleton(m), txn);
-        db.accountGroupMembersAudit().insert(
-            Collections.singleton(new AccountGroupMemberAudit(m, me)), txn);
-        txn.commit();
-        accountCache.evict(m.getAccountId());
-
-        return group.getId();
-      }
-    });
   }
 
   public void groupDetail(final AccountGroup.Id groupId,
@@ -172,7 +149,7 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
         assertAmGroupOwner(db, group);
 
         final AccountGroup owner =
-            db.accountGroups().get(new AccountGroup.NameKey(newOwnerName));
+            groupCache.get(new AccountGroup.NameKey(newOwnerName));
         if (owner == null) {
           throw new Failure(new NoSuchEntityException());
         }
@@ -187,25 +164,7 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
 
   public void renameGroup(final AccountGroup.Id groupId, final String newName,
       final AsyncCallback<VoidResult> callback) {
-    run(callback, new Action<VoidResult>() {
+    renameGroupFactory.create(groupId, newName).to(callback);
-      public VoidResult run(final ReviewDb db) throws OrmException, Failure {
-        final AccountGroup group = db.accountGroups().get(groupId);
-        assertAmGroupOwner(db, group);
-
-        final AccountGroup.NameKey oldKey = group.getNameKey();
-        final AccountGroup.NameKey newKey = new AccountGroup.NameKey(newName);
-        if (!newKey.equals(oldKey)) {
-          if (db.accountGroups().get(newKey) != null) {
-            throw new Failure(new NameAlreadyUsedException());
-          }
-          group.setNameKey(newKey);
-          db.accountGroups().update(Collections.singleton(group));
-          groupCache.evict(group);
-          groupCache.evictAfterRename(oldKey);
-        }
-        return VoidResult.INSTANCE;
-      }
-    });
   }
 
   public void changeGroupType(final AccountGroup.Id groupId,

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/RenameGroup.java

@@ -0,0 +1,99 @@
+// Copyright (C) 2009 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.httpd.rpc.account;
+
+import com.google.gerrit.common.errors.NameAlreadyUsedException;
+import com.google.gerrit.httpd.rpc.Handler;
+import com.google.gerrit.reviewdb.AccountGroup;
+import com.google.gerrit.reviewdb.AccountGroupName;
+import com.google.gerrit.reviewdb.ReviewDb;
+import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.server.account.GroupControl;
+import com.google.gerrit.server.account.NoSuchGroupException;
+import com.google.gwtjsonrpc.client.VoidResult;
+import com.google.gwtorm.client.OrmDuplicateKeyException;
+import com.google.gwtorm.client.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+
+import java.util.Collections;
+
+class RenameGroup extends Handler<VoidResult> {
+  interface Factory {
+    RenameGroup create(AccountGroup.Id id, String newName);
+  }
+
+  private final ReviewDb db;
+  private final GroupCache groupCache;
+  private final GroupControl.Factory groupControlFactory;
+
+  private final AccountGroup.Id groupId;
+  private final String newName;
+
+  @Inject
+  RenameGroup(final ReviewDb db, final GroupCache groupCache,
+      final GroupControl.Factory groupControlFactory,
+
+      @Assisted final AccountGroup.Id groupId, @Assisted final String newName) {
+    this.db = db;
+    this.groupCache = groupCache;
+    this.groupControlFactory = groupControlFactory;
+
+    this.groupId = groupId;
+    this.newName = newName;
+  }
+
+  @Override
+  public VoidResult call() throws OrmException, NameAlreadyUsedException,
+      NoSuchGroupException {
+    final GroupControl ctl = groupControlFactory.validateFor(groupId);
+    final AccountGroup group = db.accountGroups().get(groupId);
+    if (group == null || !ctl.isOwner()) {
+      throw new NoSuchGroupException(groupId);
+    }
+
+    final AccountGroup.NameKey old = group.getNameKey();
+    final AccountGroup.NameKey key = new AccountGroup.NameKey(newName);
+
+    try {
+      final AccountGroupName id = new AccountGroupName(key, groupId);
+      db.accountGroupNames().insert(Collections.singleton(id));
+    } catch (OrmDuplicateKeyException dupeErr) {
+      // If we are using this identity, don't report the exception.
+      //
+      AccountGroupName other = db.accountGroupNames().get(key);
+      if (other != null && other.getId().equals(groupId)) {
+        return VoidResult.INSTANCE;
+      }
+
+      // Otherwise, someone else has this identity.
+      //
+      throw new NameAlreadyUsedException();
+    }
+
+    group.setNameKey(key);
+    db.accountGroups().update(Collections.singleton(group));
+
+    AccountGroupName priorName = db.accountGroupNames().get(old);
+    if (priorName != null) {
+      db.accountGroupNames().delete(Collections.singleton(priorName));
+    }
+
+    groupCache.evict(group);
+    groupCache.evictAfterRename(old);
+
+    return VoidResult.INSTANCE;
+  }
+}

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/AddProjectRight.java

@@ -23,6 +23,7 @@ import com.google.gerrit.reviewdb.ApprovalCategory;
 import com.google.gerrit.reviewdb.Project;
 import com.google.gerrit.reviewdb.ProjectRight;
 import com.google.gerrit.reviewdb.ReviewDb;
+import com.google.gerrit.server.account.GroupCache;
 import com.google.gerrit.server.account.NoSuchGroupException;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectCache;
@@ -43,6 +44,7 @@ class AddProjectRight extends Handler<ProjectDetail> {
   private final ProjectDetailFactory.Factory projectDetailFactory;
   private final ProjectControl.Factory projectControlFactory;
   private final ProjectCache projectCache;
+  private final GroupCache groupCache;
   private final ReviewDb db;
   private final ApprovalTypes approvalTypes;
 
@@ -55,8 +57,8 @@ class AddProjectRight extends Handler<ProjectDetail> {
   @Inject
   AddProjectRight(final ProjectDetailFactory.Factory projectDetailFactory,
       final ProjectControl.Factory projectControlFactory,
-      final ProjectCache projectCache, final ReviewDb db,
+      final ProjectCache projectCache, final GroupCache groupCache,
-      final ApprovalTypes approvalTypes,
+      final ReviewDb db, final ApprovalTypes approvalTypes,
 
       @Assisted final Project.NameKey projectName,
       @Assisted final ApprovalCategory.Id categoryId,
@@ -65,6 +67,7 @@ class AddProjectRight extends Handler<ProjectDetail> {
     this.projectDetailFactory = projectDetailFactory;
     this.projectControlFactory = projectControlFactory;
     this.projectCache = projectCache;
+    this.groupCache = groupCache;
     this.approvalTypes = approvalTypes;
     this.db = db;
 
@@ -104,7 +107,7 @@ class AddProjectRight extends Handler<ProjectDetail> {
           + " or range " + min + ".." + max);
     }
 
-    final AccountGroup group = db.accountGroups().get(groupName);
+    final AccountGroup group = groupCache.get(groupName);
     if (group == null) {
       throw new NoSuchGroupException(groupName);
     }

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupAccess.java

@@ -19,18 +19,15 @@ import com.google.gwtorm.client.OrmException;
 import com.google.gwtorm.client.PrimaryKey;
 import com.google.gwtorm.client.Query;
 import com.google.gwtorm.client.ResultSet;
-import com.google.gwtorm.client.SecondaryKey;
 
 public interface AccountGroupAccess extends
     Access<AccountGroup, AccountGroup.Id> {
   @PrimaryKey("groupId")
   AccountGroup get(AccountGroup.Id id) throws OrmException;
 
-  @SecondaryKey("name")
+  @Query("WHERE externalName = ?")
-  AccountGroup get(AccountGroup.NameKey name) throws OrmException;
+  ResultSet<AccountGroup> byExternalName(AccountGroup.ExternalNameKey name)
-
+      throws OrmException;
-  @SecondaryKey("externalName")
-  AccountGroup get(AccountGroup.ExternalNameKey name) throws OrmException;
 
   @Query("ORDER BY name")
   ResultSet<AccountGroup> all() throws OrmException;

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupName.java

@@ -0,0 +1,42 @@
+// Copyright (C) 2009 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;
+
+import com.google.gwtorm.client.Column;
+
+/** Unique name of an {@link AccountGroup}. */
+public class AccountGroupName {
+  @Column(id = 1)
+  protected AccountGroup.NameKey name;
+
+  @Column(id = 2)
+  protected AccountGroup.Id groupId;
+
+  protected AccountGroupName() {
+  }
+
+  public AccountGroupName(AccountGroup.NameKey name, AccountGroup.Id groupId) {
+    this.name = name;
+    this.groupId = groupId;
+  }
+
+  public AccountGroup.NameKey getNameKey() {
+    return name;
+  }
+
+  public AccountGroup.Id getId() {
+    return groupId;
+  }
+}

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupNameAccess.java

@@ -0,0 +1,26 @@
+// Copyright (C) 2009The 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;
+
+import com.google.gwtorm.client.Access;
+import com.google.gwtorm.client.OrmException;
+import com.google.gwtorm.client.PrimaryKey;
+
+
+public interface AccountGroupNameAccess extends
+    Access<AccountGroupName, AccountGroup.NameKey> {
+  @PrimaryKey("name")
+  AccountGroupName get(AccountGroup.NameKey name) throws OrmException;
+}

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ReviewDb.java

@@ -63,6 +63,9 @@ public interface ReviewDb extends Schema {
   @Relation
   AccountGroupAccess accountGroups();
 
+  @Relation
+  AccountGroupNameAccess accountGroupNames();
+
   @Relation
   AccountGroupMemberAccess accountGroupMembers();
 

gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_generic.sql

@@ -33,7 +33,6 @@ ON account_external_ids (email_address);
 
 -- *********************************************************************
 -- AccountGroupAccess
---    @SecondaryKey("name") covers:  all, suggestByName
 CREATE INDEX account_groups_ownedByGroup
 ON account_groups (owner_group_id);
 

gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_postgres.sql

@@ -72,7 +72,6 @@ ON account_external_ids (email_address);
 
 -- *********************************************************************
 -- AccountGroupAccess
---    @SecondaryKey("name") covers:  all, suggestByName
 CREATE INDEX account_groups_ownedByGroup
 ON account_groups (owner_group_id);
 

gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCache.java

@@ -16,15 +16,17 @@ package com.google.gerrit.server.account;
 
 import com.google.gerrit.reviewdb.AccountGroup;
 
+import java.util.Collection;
+
 /** Tracks group objects in memory for efficient access. */
 public interface GroupCache {
   public AccountGroup get(AccountGroup.Id groupId);
 
-  public AccountGroup get(AccountGroup.ExternalNameKey externalName);
+  public AccountGroup get(AccountGroup.NameKey name);
+
+  public Collection<AccountGroup> get(AccountGroup.ExternalNameKey externalName);
 
   public void evict(AccountGroup group);
 
   public void evictAfterRename(AccountGroup.NameKey oldName);
-
-  public AccountGroup lookup(String groupName);
 }

gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCacheImpl.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.account;
 
 import com.google.gerrit.reviewdb.AccountGroup;
+import com.google.gerrit.reviewdb.AccountGroupName;
 import com.google.gerrit.reviewdb.ReviewDb;
 import com.google.gerrit.server.cache.Cache;
 import com.google.gerrit.server.cache.CacheModule;
@@ -28,6 +29,8 @@ import com.google.inject.Singleton;
 import com.google.inject.TypeLiteral;
 import com.google.inject.name.Named;
 
+import java.util.Collection;
+
 /** Tracks group objects in memory for efficient access. */
 @Singleton
 public class GroupCacheImpl implements GroupCache {
@@ -50,7 +53,7 @@ public class GroupCacheImpl implements GroupCache {
   private final AccountGroup.Id administrators;
   private final SelfPopulatingCache<AccountGroup.Id, AccountGroup> byId;
   private final SelfPopulatingCache<AccountGroup.NameKey, AccountGroup> byName;
-  private final SelfPopulatingCache<AccountGroup.ExternalNameKey, AccountGroup> byExternalName;
+  private final SelfPopulatingCache<AccountGroup.ExternalNameKey, Collection<AccountGroup>> byExternalName;
 
   @Inject
   GroupCacheImpl(
@@ -85,11 +88,11 @@ public class GroupCacheImpl implements GroupCache {
         };
 
     byExternalName =
-        new SelfPopulatingCache<AccountGroup.ExternalNameKey, AccountGroup>(
+        new SelfPopulatingCache<AccountGroup.ExternalNameKey, Collection<AccountGroup>>(
             (Cache) rawAny) {
           @Override
-          public AccountGroup createEntry(final AccountGroup.ExternalNameKey key)
+          public Collection<AccountGroup> createEntry(
-              throws Exception {
+              final AccountGroup.ExternalNameKey key) throws Exception {
             return lookup(key);
           }
         };
@@ -119,21 +122,23 @@ public class GroupCacheImpl implements GroupCache {
     return g;
   }
 
-  private AccountGroup lookup(final AccountGroup.NameKey groupName)
+  private AccountGroup lookup(final AccountGroup.NameKey name)
       throws OrmException {
+    final AccountGroupName r;
     final ReviewDb db = schema.open();
     try {
-      return db.accountGroups().get(groupName);
+      r = db.accountGroupNames().get(name);
     } finally {
       db.close();
     }
+    return r != null ? get(r.getId()) : null;
   }
 
-  private AccountGroup lookup(final AccountGroup.ExternalNameKey externalName)
+  private Collection<AccountGroup> lookup(
-      throws OrmException {
+      final AccountGroup.ExternalNameKey name) throws OrmException {
     final ReviewDb db = schema.open();
     try {
-      return db.accountGroups().get(externalName);
+      return db.accountGroups().byExternalName(name).toList();
     } finally {
       db.close();
     }
@@ -153,11 +158,12 @@ public class GroupCacheImpl implements GroupCache {
     byName.remove(oldName);
   }
 
-  public AccountGroup lookup(final String groupName) {
+  public AccountGroup get(final AccountGroup.NameKey name) {
-    return byName.get(new AccountGroup.NameKey(groupName));
+    return byName.get(name);
   }
 
-  public AccountGroup get(final AccountGroup.ExternalNameKey externalName) {
+  public Collection<AccountGroup> get(
+      final AccountGroup.ExternalNameKey externalName) {
     return byExternalName.get(externalName);
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java

@@ -422,11 +422,11 @@ class LdapRealm implements Realm {
 
     final Set<AccountGroup.Id> actual = new HashSet<AccountGroup.Id>();
     for (String dn : groupDNs) {
-      final AccountGroup group;
+      for (AccountGroup group : groupCache
-
+          .get(new AccountGroup.ExternalNameKey(dn))) {
-      group = groupCache.get(new AccountGroup.ExternalNameKey(dn));
+        if (group.getType() == AccountGroup.Type.LDAP) {
-      if (group != null && group.getType() == AccountGroup.Type.LDAP) {
+          actual.add(group.getId());
-        actual.add(group.getId());
+        }
       }
     }
 

gerrit-server/src/main/java/com/google/gerrit/server/git/PushReplication.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.git;
 
 import com.google.gerrit.reviewdb.AccountGroup;
+import com.google.gerrit.reviewdb.AccountGroupName;
 import com.google.gerrit.reviewdb.Project;
 import com.google.gerrit.reviewdb.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
@@ -353,8 +354,8 @@ public class PushReplication implements ReplicationQueue {
         final ReviewDb db = dbfactory.open();
         try {
           for (String name : groupNames) {
-            AccountGroup group =
+            AccountGroupName group =
-                db.accountGroups().get(new AccountGroup.NameKey(name));
+                db.accountGroupNames().get(new AccountGroup.NameKey(name));
             if (group == null) {
               log.warn("Group \"" + name + "\" not in database,"
                   + " removing from authGroup");

gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java

@@ -32,7 +32,7 @@ import java.util.List;
 /** A version of the database schema. */
 public abstract class SchemaVersion {
   /** The current schema version. */
-  private static final Class<? extends SchemaVersion> C = Schema_22.class;
+  private static final Class<? extends SchemaVersion> C = Schema_23.class;
 
   public static class Module extends AbstractModule {
     @Override

gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_23.java

@@ -0,0 +1,66 @@
+// Copyright (C) 2009 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.gerrit.reviewdb.AccountGroup;
+import com.google.gerrit.reviewdb.AccountGroupName;
+import com.google.gerrit.reviewdb.ReviewDb;
+import com.google.gwtorm.client.OrmException;
+import com.google.gwtorm.client.Transaction;
+import com.google.gwtorm.jdbc.JdbcSchema;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.ArrayList;
+import java.util.Collection;
+
+class Schema_23 extends SchemaVersion {
+  @Inject
+  Schema_23(Provider<Schema_22> prior) {
+    super(prior);
+  }
+
+  @Override
+  protected void migrateData(ReviewDb db) throws OrmException, SQLException {
+    Collection<AccountGroupName> names = new ArrayList<AccountGroupName>();
+    Statement queryStmt = ((JdbcSchema) db).getConnection().createStatement();
+    try {
+      ResultSet results =
+          queryStmt.executeQuery("SELECT group_id, name FROM account_groups");
+      while (results.next()) {
+        final int id = results.getInt(1);
+        final String name = results.getString(2);
+
+        final AccountGroup.Id group = new AccountGroup.Id(id);
+        final AccountGroup.NameKey key = toKey(name);
+        names.add(new AccountGroupName(key, group));
+      }
+    } finally {
+      queryStmt.close();
+    }
+    if (!names.isEmpty()) {
+      Transaction t = db.beginTransaction();
+      db.accountGroupNames().insert(names, t);
+      t.commit();
+    }
+  }
+
+  private AccountGroup.NameKey toKey(final String name) {
+    return new AccountGroup.NameKey(name);
+  }
+}

gerrit-sshd/src/main/java/com/google/gerrit/sshd/args4j/AccountGroupIdHandler.java

@@ -42,7 +42,7 @@ public class AccountGroupIdHandler extends OptionHandler<AccountGroup.Id> {
   public final int parseArguments(final Parameters params)
       throws CmdLineException {
     final String n = params.getParameter(0);
-    final AccountGroup group = groupCache.lookup(n);
+    final AccountGroup group = groupCache.get(new AccountGroup.NameKey(n));
     if (group == null) {
       throw new CmdLineException(owner, "Group \"" + n + "\" does not exist");
     }