From b1dac0a73baa3f7e95341af49b97fa37cf8c0711 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Thu, 31 Dec 2009 10:26:26 -0800 Subject: [PATCH] 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 --- .../httpd/rpc/account/AccountModule.java | 3 +- .../gerrit/httpd/rpc/account/CreateGroup.java | 89 +++++++++++++++++ .../rpc/account/GroupAdminServiceImpl.java | 61 ++---------- .../gerrit/httpd/rpc/account/RenameGroup.java | 99 +++++++++++++++++++ .../httpd/rpc/project/AddProjectRight.java | 9 +- .../gerrit/reviewdb/AccountGroupAccess.java | 9 +- .../gerrit/reviewdb/AccountGroupName.java | 42 ++++++++ .../reviewdb/AccountGroupNameAccess.java | 26 +++++ .../com/google/gerrit/reviewdb/ReviewDb.java | 3 + .../google/gerrit/reviewdb/index_generic.sql | 1 - .../google/gerrit/reviewdb/index_postgres.sql | 1 - .../gerrit/server/account/GroupCache.java | 8 +- .../gerrit/server/account/GroupCacheImpl.java | 30 +++--- .../gerrit/server/auth/ldap/LdapRealm.java | 10 +- .../gerrit/server/git/PushReplication.java | 5 +- .../gerrit/server/schema/SchemaVersion.java | 2 +- .../gerrit/server/schema/Schema_23.java | 66 +++++++++++++ .../sshd/args4j/AccountGroupIdHandler.java | 2 +- 18 files changed, 379 insertions(+), 87 deletions(-) create mode 100644 gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/CreateGroup.java create mode 100644 gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/RenameGroup.java create mode 100644 gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupName.java create mode 100644 gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupNameAccess.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_23.java diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountModule.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountModule.java index 3f7c7a7c00..13dd9b4389 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountModule.java +++ b/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); diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/CreateGroup.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/CreateGroup.java new file mode 100644 index 0000000000..1525b27f1b --- /dev/null +++ b/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 { + 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; + } +} diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java index fa072c1156..e5449eadf2 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java +++ b/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 callback) { - run(callback, new Action() { - 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(); - } - }); + createGroupFactory.create(newName).to(callback); } 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 callback) { - run(callback, new Action() { - 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; - } - }); + renameGroupFactory.create(groupId, newName).to(callback); } public void changeGroupType(final AccountGroup.Id groupId, diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/RenameGroup.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/RenameGroup.java new file mode 100644 index 0000000000..63b5bceb10 --- /dev/null +++ b/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 { + 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; + } +} diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/AddProjectRight.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/AddProjectRight.java index 362eb11197..b12c92f489 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/AddProjectRight.java +++ b/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 { 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 { @Inject AddProjectRight(final ProjectDetailFactory.Factory projectDetailFactory, final ProjectControl.Factory projectControlFactory, - final ProjectCache projectCache, final ReviewDb db, - final ApprovalTypes approvalTypes, + final ProjectCache projectCache, final GroupCache groupCache, + final ReviewDb db, final ApprovalTypes approvalTypes, @Assisted final Project.NameKey projectName, @Assisted final ApprovalCategory.Id categoryId, @@ -65,6 +67,7 @@ class AddProjectRight extends Handler { 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 { + " or range " + min + ".." + max); } - final AccountGroup group = db.accountGroups().get(groupName); + final AccountGroup group = groupCache.get(groupName); if (group == null) { throw new NoSuchGroupException(groupName); } diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupAccess.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupAccess.java index 231ff0ab3b..14a2822221 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupAccess.java +++ b/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 { @PrimaryKey("groupId") AccountGroup get(AccountGroup.Id id) throws OrmException; - @SecondaryKey("name") - AccountGroup get(AccountGroup.NameKey name) throws OrmException; - - @SecondaryKey("externalName") - AccountGroup get(AccountGroup.ExternalNameKey name) throws OrmException; + @Query("WHERE externalName = ?") + ResultSet byExternalName(AccountGroup.ExternalNameKey name) + throws OrmException; @Query("ORDER BY name") ResultSet all() throws OrmException; diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupName.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupName.java new file mode 100644 index 0000000000..8c7ecaef8a --- /dev/null +++ b/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; + } +} diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupNameAccess.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupNameAccess.java new file mode 100644 index 0000000000..f49df45cb0 --- /dev/null +++ b/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 { + @PrimaryKey("name") + AccountGroupName get(AccountGroup.NameKey name) throws OrmException; +} diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ReviewDb.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ReviewDb.java index 35c30d9365..168112e4fe 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ReviewDb.java +++ b/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(); diff --git a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_generic.sql b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_generic.sql index cfdb23681e..ea6b9d9c80 100644 --- a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_generic.sql +++ b/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); diff --git a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_postgres.sql b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_postgres.sql index 455d96018d..a74e2f725c 100644 --- a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_postgres.sql +++ b/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); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCache.java index 880ae2cc49..978d9c2a90 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCache.java +++ b/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 get(AccountGroup.ExternalNameKey externalName); public void evict(AccountGroup group); public void evictAfterRename(AccountGroup.NameKey oldName); - - public AccountGroup lookup(String groupName); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCacheImpl.java index 74e3218453..9fedd806a2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCacheImpl.java +++ b/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 byId; private final SelfPopulatingCache byName; - private final SelfPopulatingCache byExternalName; + private final SelfPopulatingCache> byExternalName; @Inject GroupCacheImpl( @@ -85,11 +88,11 @@ public class GroupCacheImpl implements GroupCache { }; byExternalName = - new SelfPopulatingCache( + new SelfPopulatingCache>( (Cache) rawAny) { @Override - public AccountGroup createEntry(final AccountGroup.ExternalNameKey key) - throws Exception { + public Collection createEntry( + 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) - throws OrmException { + private Collection lookup( + 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) { - return byName.get(new AccountGroup.NameKey(groupName)); + public AccountGroup get(final AccountGroup.NameKey name) { + return byName.get(name); } - public AccountGroup get(final AccountGroup.ExternalNameKey externalName) { + public Collection get( + final AccountGroup.ExternalNameKey externalName) { return byExternalName.get(externalName); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java index 324f25431a..8b2444a858 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java @@ -422,11 +422,11 @@ class LdapRealm implements Realm { final Set actual = new HashSet(); for (String dn : groupDNs) { - final AccountGroup group; - - group = groupCache.get(new AccountGroup.ExternalNameKey(dn)); - if (group != null && group.getType() == AccountGroup.Type.LDAP) { - actual.add(group.getId()); + for (AccountGroup group : groupCache + .get(new AccountGroup.ExternalNameKey(dn))) { + if (group.getType() == AccountGroup.Type.LDAP) { + actual.add(group.getId()); + } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/PushReplication.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/PushReplication.java index 3981223bea..daec791ae8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/PushReplication.java +++ b/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 = - db.accountGroups().get(new AccountGroup.NameKey(name)); + AccountGroupName group = + db.accountGroupNames().get(new AccountGroup.NameKey(name)); if (group == null) { log.warn("Group \"" + name + "\" not in database," + " removing from authGroup"); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java index 1a3db8ec1e..1f04777d4c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java +++ b/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 C = Schema_22.class; + private static final Class C = Schema_23.class; public static class Module extends AbstractModule { @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_23.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_23.java new file mode 100644 index 0000000000..bb68bcff60 --- /dev/null +++ b/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 prior) { + super(prior); + } + + @Override + protected void migrateData(ReviewDb db) throws OrmException, SQLException { + Collection names = new ArrayList(); + 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); + } +} diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/args4j/AccountGroupIdHandler.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/args4j/AccountGroupIdHandler.java index 7966142b10..13ff1b10f0 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/args4j/AccountGroupIdHandler.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/args4j/AccountGroupIdHandler.java @@ -42,7 +42,7 @@ public class AccountGroupIdHandler extends OptionHandler { 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"); }