diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDetail.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDetail.java index 65723f7d1e..01c7985c0e 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDetail.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDetail.java @@ -26,7 +26,7 @@ public class GroupDetail { public AccountGroup group; public List members; public List includes; - public AccountGroup ownerGroup; + public GroupReference ownerGroup; public boolean canModify; public GroupDetail() { @@ -52,7 +52,7 @@ public class GroupDetail { includes = i; } - public void setOwnerGroup(AccountGroup g) { + public void setOwnerGroup(GroupReference g) { ownerGroup = g; } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupInfoScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupInfoScreen.java index eaf564f3d9..130241e236 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupInfoScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupInfoScreen.java @@ -446,7 +446,7 @@ public class AccountGroupInfoScreen extends AccountGroupScreen { if (groupDetail.ownerGroup != null) { ownerTxt.setText(groupDetail.ownerGroup.getName()); } else { - ownerTxt.setText(Util.M.deletedGroup(group.getOwnerGroupId().get())); + ownerTxt.setText(Util.M.deletedReference(group.getOwnerGroupUUID().get())); } descTxt.setText(group.getDescription()); 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 e90c2bffda..d9f327d9c8 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 @@ -145,13 +145,13 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements final AccountGroup group = db.accountGroups().get(groupId); assertAmGroupOwner(db, group); - final AccountGroup owner = + AccountGroup owner = groupCache.get(new AccountGroup.NameKey(newOwnerName)); if (owner == null) { throw new Failure(new NoSuchEntityException()); } - group.setOwnerGroupId(owner.getId()); + group.setOwnerGroupUUID(owner.getGroupUUID()); db.accountGroups().update(Collections.singleton(group)); groupCache.evict(group); return VoidResult.INSTANCE; diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountGroup.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountGroup.java index 8e2541ab44..1f244bb1a2 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountGroup.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountGroup.java @@ -51,7 +51,7 @@ public final class AccountGroup { StringKey> { private static final long serialVersionUID = 1L; - @Column(id = 1, length = 40) + @Column(id = 1) protected String uuid; protected UUID() { @@ -193,14 +193,6 @@ public final class AccountGroup { @Column(id = 2) protected Id groupId; - /** - * Identity of the group whose members can manage this group. - *

- * This can be a self-reference to indicate the group's members manage itself. - */ - @Column(id = 3) - protected Id ownerGroupId; - /** A textual description of the group's purpose. */ @Column(id = 4, length = Integer.MAX_VALUE, notNull = false) protected String description; @@ -220,6 +212,14 @@ public final class AccountGroup { @Column(id = 9) protected UUID groupUUID; + /** + * Identity of the group whose members can manage this group. + *

+ * This can be a self-reference to indicate the group's members manage itself. + */ + @Column(id = 10) + protected UUID ownerGroupUUID; + protected AccountGroup() { } @@ -227,9 +227,9 @@ public final class AccountGroup { final AccountGroup.Id newId, final AccountGroup.UUID uuid) { name = newName; groupId = newId; - ownerGroupId = groupId; visibleToAll = false; groupUUID = uuid; + ownerGroupUUID = groupUUID; setType(Type.INTERNAL); } @@ -257,12 +257,12 @@ public final class AccountGroup { description = d; } - public AccountGroup.Id getOwnerGroupId() { - return ownerGroupId; + public AccountGroup.UUID getOwnerGroupUUID() { + return ownerGroupUUID; } - public void setOwnerGroupId(final AccountGroup.Id id) { - ownerGroupId = id; + public void setOwnerGroupUUID(final AccountGroup.UUID uuid) { + ownerGroupUUID = uuid; } public Type getType() { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java index f7451a8488..ea388c7599 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java @@ -40,7 +40,7 @@ public class GroupControl { if (group == null) { throw new NoSuchGroupException(groupId); } - return new GroupControl(groupCache, user.get(), group); + return new GroupControl(user.get(), group); } public GroupControl controlFor(final AccountGroup.UUID groupId) @@ -49,11 +49,11 @@ public class GroupControl { if (group == null) { throw new NoSuchGroupException(groupId); } - return new GroupControl(groupCache, user.get(), group); + return new GroupControl(user.get(), group); } public GroupControl controlFor(final AccountGroup group) { - return new GroupControl(groupCache, user.get(), group); + return new GroupControl(user.get(), group); } public GroupControl validateFor(final AccountGroup.Id groupId) @@ -66,13 +66,11 @@ public class GroupControl { } } - private final GroupCache groupCache; private final CurrentUser user; private final AccountGroup group; private Boolean isOwner; - GroupControl(GroupCache g, CurrentUser who, AccountGroup gc) { - groupCache = g; + GroupControl(CurrentUser who, AccountGroup gc) { user = who; group = gc; } @@ -94,8 +92,7 @@ public class GroupControl { public boolean isOwner() { if (isOwner == null) { - AccountGroup g = groupCache.get(group.getOwnerGroupId()); - AccountGroup.UUID ownerUUID = g != null ? g.getGroupUUID() : null; + AccountGroup.UUID ownerUUID = group.getOwnerGroupUUID(); isOwner = getCurrentUser().getEffectiveGroups().contains(ownerUUID) || getCurrentUser().getCapabilities().canAdministrateServer(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupDetailFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupDetailFactory.java index 7f88134153..6bb31b386c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupDetailFactory.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupDetailFactory.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.account; import com.google.gerrit.common.data.GroupDetail; +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; @@ -66,7 +67,10 @@ public class GroupDetailFactory implements Callable { final AccountGroup group = control.getAccountGroup(); final GroupDetail detail = new GroupDetail(); detail.setGroup(group); - detail.setOwnerGroup(groupCache.get(group.getOwnerGroupId())); + AccountGroup ownerGroup = groupCache.get(group.getGroupUUID()); + if (ownerGroup != null) { + detail.setOwnerGroup(GroupReference.forGroup(ownerGroup)); + } switch (group.getType()) { case INTERNAL: detail.setMembers(loadMembers()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java index 5f94df21e2..1ff1a3e60f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java @@ -107,7 +107,10 @@ public class PerformCreateGroup { final AccountGroup group = new AccountGroup(nameKey, groupId, uuid); group.setVisibleToAll(visibleToAll); if (ownerGroupId != null) { - group.setOwnerGroupId(ownerGroupId); + AccountGroup ownerGroup = groupCache.get(ownerGroupId); + if (ownerGroup != null) { + group.setOwnerGroupUUID(ownerGroup.getGroupUUID()); + } } if (groupDescription != null) { group.setDescription(groupDescription); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaCreator.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaCreator.java index 4070248546..08d94af71d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaCreator.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaCreator.java @@ -161,7 +161,7 @@ public class SchemaCreator { anonymous = newGroup(c, "Anonymous Users", AccountGroup.ANONYMOUS_USERS); anonymous.setDescription("Any user, signed-in or not"); - anonymous.setOwnerGroupId(admin.getId()); + anonymous.setOwnerGroupUUID(admin.getGroupUUID()); anonymous.setType(AccountGroup.Type.SYSTEM); c.accountGroups().insert(Collections.singleton(anonymous)); c.accountGroupNames().insert( @@ -170,7 +170,7 @@ public class SchemaCreator { registered = newGroup(c, "Registered Users", AccountGroup.REGISTERED_USERS); registered.setDescription("Any signed-in user"); - registered.setOwnerGroupId(admin.getId()); + registered.setOwnerGroupUUID(admin.getGroupUUID()); registered.setType(AccountGroup.Type.SYSTEM); c.accountGroups().insert(Collections.singleton(registered)); c.accountGroupNames().insert( @@ -178,7 +178,7 @@ public class SchemaCreator { final AccountGroup batchUsers = newGroup(c, "Non-Interactive Users", null); batchUsers.setDescription("Users who perform batch actions on Gerrit"); - batchUsers.setOwnerGroupId(admin.getId()); + batchUsers.setOwnerGroupUUID(admin.getGroupUUID()); batchUsers.setType(AccountGroup.Type.INTERNAL); c.accountGroups().insert(Collections.singleton(batchUsers)); c.accountGroupNames().insert( @@ -186,7 +186,7 @@ public class SchemaCreator { owners = newGroup(c, "Project Owners", AccountGroup.PROJECT_OWNERS); owners.setDescription("Any owner of the project"); - owners.setOwnerGroupId(admin.getId()); + owners.setOwnerGroupUUID(admin.getGroupUUID()); owners.setType(AccountGroup.Type.SYSTEM); c.accountGroups().insert(Collections.singleton(owners)); c.accountGroupNames().insert( 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 4779e10dde..9c89c7343d 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. */ - public static final Class C = Schema_66.class; + public static final Class C = Schema_67.class; public static class Module extends AbstractModule { @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_65.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_65.java index a3c9770ad8..3383364e68 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_65.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_65.java @@ -101,15 +101,15 @@ public class Schema_65 extends SchemaVersion { ui.message("Moved contributor agreements to project.config"); // Create the auto verify groups. - List adminGroupIds = getAdministrateServerGroups(db, config); + List adminGroupUUIDs = getAdministrateServerGroups(db, config); for (ContributorAgreement agreement : agreements.values()) { if (agreement.getAutoVerify() != null) { - getOrCreateGroupForIndividuals(db, config, adminGroupIds, agreement); + getOrCreateGroupForIndividuals(db, config, adminGroupUUIDs, agreement); } } // Scan AccountAgreement - long minTime = addAccountAgreements(db, config, adminGroupIds, agreements); + long minTime = addAccountAgreements(db, config, adminGroupUUIDs, agreements); ProjectConfig base = ProjectConfig.read(md, null); for (ContributorAgreement agreement : agreements.values()) { @@ -257,14 +257,14 @@ public class Schema_65 extends SchemaVersion { } private AccountGroup createGroup(ReviewDb db, String groupName, - AccountGroup.Id adminGroupId, String description) + AccountGroup.UUID adminGroupUUID, String description) throws OrmException { final AccountGroup.Id groupId = new AccountGroup.Id(db.nextAccountGroupId()); final AccountGroup.NameKey nameKey = new AccountGroup.NameKey(groupName); final AccountGroup.UUID uuid = GroupUUID.make(groupName, serverUser); final AccountGroup group = new AccountGroup(nameKey, groupId, uuid); - group.setOwnerGroupId(adminGroupId); + group.setOwnerGroupUUID(adminGroupUUID); group.setDescription(description); final AccountGroupName gn = new AccountGroupName(group); // first insert the group name to validate that the group name hasn't @@ -274,21 +274,17 @@ public class Schema_65 extends SchemaVersion { return group; } - private List getAdministrateServerGroups( - ReviewDb db, ProjectConfig cfg) throws OrmException { + private List getAdministrateServerGroups( + ReviewDb db, ProjectConfig cfg) { List rules = cfg.getAccessSection(AccessSection.GLOBAL_CAPABILITIES) .getPermission(GlobalCapability.ADMINISTRATE_SERVER) .getRules(); - List groups = + List groups = Lists.newArrayListWithExpectedSize(rules.size()); for (PermissionRule rule : rules) { if (rule.getAction() == Action.ALLOW) { - groups.add(db.accountGroups() - .byUUID(rule.getGroup().getUUID()) - .toList() - .get(0) - .getId()); + groups.add(rule.getGroup().getUUID()); } } if (groups.isEmpty()) { @@ -299,7 +295,7 @@ public class Schema_65 extends SchemaVersion { } private GroupReference getOrCreateGroupForIndividuals(ReviewDb db, - ProjectConfig config, List adminGroupIds, + ProjectConfig config, List adminGroupUUIDs, ContributorAgreement agreement) throws OrmException { if (!agreement.getAccepted().isEmpty()) { @@ -317,12 +313,12 @@ public class Schema_65 extends SchemaVersion { "account group name exists but account group does not: " + name); } - if (!adminGroupIds.contains(ag.getOwnerGroupId())) { + if (!adminGroupUUIDs.contains(ag.getOwnerGroupUUID())) { throw new IllegalStateException( "individual group exists with non admin owner group: " + name); } } else { - ag = createGroup(db, name, adminGroupIds.get(0), + ag = createGroup(db, name, adminGroupUUIDs.get(0), String.format("Users who have accepted the %s CLA", agreement.getName())); } GroupReference group = config.resolve(ag); @@ -344,7 +340,7 @@ public class Schema_65 extends SchemaVersion { } private long addAccountAgreements(ReviewDb db, ProjectConfig config, - List adminGroupIds, + List adminGroupUUIDs, Map agreements) throws SQLException, OrmException { Statement stmt = ((JdbcSchema) db).getConnection().createStatement(); @@ -373,7 +369,7 @@ public class Schema_65 extends SchemaVersion { // Enter Agreement GroupReference individualGroup = - getOrCreateGroupForIndividuals(db, config, adminGroupIds, agreement); + getOrCreateGroupForIndividuals(db, config, adminGroupUUIDs, agreement); AccountGroup.Id groupId = db.accountGroups() .byUUID(individualGroup.getUUID()) .toList() diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_67.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_67.java new file mode 100644 index 0000000000..7c7b88096b --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_67.java @@ -0,0 +1,95 @@ +// Copyright (C) 2012 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.Iterables; +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; +import com.google.gerrit.common.data.ContributorAgreement; +import com.google.gerrit.reviewdb.client.AccountGroup; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gwtorm.jdbc.JdbcSchema; +import com.google.gwtorm.server.OrmException; +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.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; + +public class Schema_67 extends SchemaVersion { + + @Inject + Schema_67(Provider prior) { + super(prior); + } + + @Override + protected void migrateData(ReviewDb db, UpdateUI ui) + throws OrmException, SQLException { + ui.message("Update ownerGroupId to ownerGroupUUID"); + + // Scan all AccountGroup, and find the ones that need the owner_group_id + // migrated to owner_group_uuid. + Map idMap = Maps.newHashMap(); + Statement stmt = ((JdbcSchema) db).getConnection().createStatement(); + try { + ResultSet rs = stmt.executeQuery( + "SELECT group_id, owner_group_id FROM account_groups" + + " WHERE owner_group_uuid is NULL or owner_group_uuid =''"); + try { + Map agreements = Maps.newHashMap(); + while (rs.next()) { + AccountGroup.Id groupId = new AccountGroup.Id(rs.getInt(1)); + AccountGroup.Id ownerId = new AccountGroup.Id(rs.getInt(2)); + idMap.put(groupId, ownerId); + } + } finally { + rs.close(); + } + } finally { + stmt.close(); + } + + // Lookup up all groups by ID. + Set all = + Sets.newHashSet(Iterables.concat(idMap.keySet(), idMap.values())); + Map groups = Maps.newHashMap(); + com.google.gwtorm.server.ResultSet rs = + db.accountGroups().get(all); + try { + for (AccountGroup group : rs) { + groups.put(group.getId(), group); + } + } finally { + rs.close(); + } + + // Update the ownerGroupUUID. + List toUpdate = Lists.newArrayListWithCapacity(idMap.size()); + for (Entry entry : idMap.entrySet()) { + AccountGroup group = groups.get(entry.getKey()); + AccountGroup owner = groups.get(entry.getValue()); + group.setOwnerGroupUUID(owner.getGroupUUID()); + toUpdate.add(group); + } + + db.accountGroups().update(toUpdate); + } +}