Merge "Migrate AccountGroup owner Id to UUID"

gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDetail.java

@@ -26,7 +26,7 @@ public class GroupDetail {
   public AccountGroup group;
   public List<AccountGroupMember> members;
   public List<AccountGroupInclude> 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;
   }
 

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());
 

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;

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountGroup.java

@@ -51,7 +51,7 @@ public final class AccountGroup {
       StringKey<com.google.gwtorm.client.Key<?>> {
     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.
-   * <p>
-   * 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.
+   * <p>
+   * 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() {
+  public AccountGroup.UUID getOwnerGroupUUID() {
-    return ownerGroupId;
+    return ownerGroupUUID;
   }
 
-  public void setOwnerGroupId(final AccountGroup.Id id) {
+  public void setOwnerGroupUUID(final AccountGroup.UUID uuid) {
-    ownerGroupId = id;
+    ownerGroupUUID = uuid;
   }
 
   public Type getType() {

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) {
+  GroupControl(CurrentUser who, AccountGroup gc) {
-    groupCache = g;
     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 = group.getOwnerGroupUUID();
-      AccountGroup.UUID ownerUUID = g != null ? g.getGroupUUID() : null;
       isOwner = getCurrentUser().getEffectiveGroups().contains(ownerUUID)
              || getCurrentUser().getCapabilities().canAdministrateServer();
     }

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<GroupDetail> {
     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());

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);

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(

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<Schema_66> C = Schema_66.class;
+  public static final Class<Schema_67> C = Schema_67.class;
 
   public static class Module extends AbstractModule {
     @Override

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<AccountGroup.Id> adminGroupIds = getAdministrateServerGroups(db, config);
+      List<AccountGroup.UUID> 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<AccountGroup.Id> getAdministrateServerGroups(
+  private List<AccountGroup.UUID> getAdministrateServerGroups(
-      ReviewDb db, ProjectConfig cfg) throws OrmException {
+      ReviewDb db, ProjectConfig cfg) {
     List<PermissionRule> rules = cfg.getAccessSection(AccessSection.GLOBAL_CAPABILITIES)
        .getPermission(GlobalCapability.ADMINISTRATE_SERVER)
        .getRules();
 
-    List<AccountGroup.Id> groups =
+    List<AccountGroup.UUID> groups =
         Lists.newArrayListWithExpectedSize(rules.size());
     for (PermissionRule rule : rules) {
       if (rule.getAction() == Action.ALLOW) {
-        groups.add(db.accountGroups()
+        groups.add(rule.getGroup().getUUID());
-            .byUUID(rule.getGroup().getUUID())
-            .toList()
-            .get(0)
-            .getId());
       }
     }
     if (groups.isEmpty()) {
@@ -299,7 +295,7 @@ public class Schema_65 extends SchemaVersion {
   }
 
   private GroupReference getOrCreateGroupForIndividuals(ReviewDb db,
-      ProjectConfig config, List<AccountGroup.Id> adminGroupIds,
+      ProjectConfig config, List<AccountGroup.UUID> 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<AccountGroup.Id> adminGroupIds,
+      List<AccountGroup.UUID> adminGroupUUIDs,
       Map<Integer, ContributorAgreement> 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()

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<Schema_66> 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<AccountGroup.Id, AccountGroup.Id> 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<Integer, ContributorAgreement> 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<AccountGroup.Id> all =
+        Sets.newHashSet(Iterables.concat(idMap.keySet(), idMap.values()));
+    Map<AccountGroup.Id, AccountGroup> groups = Maps.newHashMap();
+    com.google.gwtorm.server.ResultSet<AccountGroup> rs =
+        db.accountGroups().get(all);
+    try {
+      for (AccountGroup group : rs) {
+        groups.put(group.getId(), group);
+      }
+    } finally {
+      rs.close();
+    }
+
+    // Update the ownerGroupUUID.
+    List<AccountGroup> toUpdate = Lists.newArrayListWithCapacity(idMap.size());
+    for (Entry<AccountGroup.Id, AccountGroup.Id> 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);
+  }
+}