opendev/gerrit
Code Issues Proposed changes

Allow group includes to be by UUID instead of group ID

Browse Source 
Per discussion on-list, the behavior change in 2.5 to only
allow groups by ID is considered a regression for groups
that want to include external groups (like LDAP).

In order to permit smooth migration, we want to allow both
ID and UUIDs to co-exist for the time being.

Change-Id: I0dbdb15b9c62f2dbce64acbc34c515c7b8229c04
This commit is contained in:
Chad Horohoe
2012-12-13 15:00:04 -05:00
parent 57143be354
commit 2cf207f384
24 changed files with 329 additions and 192 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java
View File

@@ -123,15 +123,15 @@ public class GroupControl {
return canSeeMembers();
}
public boolean canAddGroup(AccountGroup.Id id) {
public boolean canAddGroup(AccountGroup.UUID uuid) {
return isOwner();
}
public boolean canRemoveGroup(AccountGroup.Id id) {
public boolean canRemoveGroup(AccountGroup.UUID uuid) {
return isOwner();
}
public boolean canSeeGroup(AccountGroup.Id id) {
public boolean canSeeGroup(AccountGroup.UUID uuid) {
return canSeeMembers();
}

22
gerrit-server/src/main/java/com/google/gerrit/server/account/GroupDetailFactory.java
View File

@@ -20,7 +20,7 @@ import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupInclude;
import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuid;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gwtorm.server.OrmException;
@@ -123,21 +123,21 @@ public class GroupDetailFactory implements Callable<GroupDetail> {
return members;
}
private List<AccountGroupInclude> loadIncludes() throws OrmException {
List<AccountGroupInclude> groups = new ArrayList<AccountGroupInclude>();
private List<AccountGroupIncludeByUuid> loadIncludes() throws OrmException {
List<AccountGroupIncludeByUuid> groups = new ArrayList<AccountGroupIncludeByUuid>();
for (final AccountGroupInclude m : db.accountGroupIncludes().byGroup(groupId)) {
if (control.canSeeGroup(m.getIncludeId())) {
gic.want(m.getIncludeId());
for (final AccountGroupIncludeByUuid m : db.accountGroupIncludesByUuid().byGroup(groupId)) {
if (control.canSeeGroup(m.getIncludeUUID())) {
gic.want(m.getIncludeUUID());
groups.add(m);
}
}
Collections.sort(groups, new Comparator<AccountGroupInclude>() {
public int compare(final AccountGroupInclude o1,
final AccountGroupInclude o2) {
final AccountGroup a = gic.get(o1.getIncludeId());
final AccountGroup b = gic.get(o2.getIncludeId());
Collections.sort(groups, new Comparator<AccountGroupIncludeByUuid>() {
public int compare(final AccountGroupIncludeByUuid o1,
final AccountGroupIncludeByUuid o2) {
final AccountGroup a = gic.get(o1.getIncludeUUID());
final AccountGroup b = gic.get(o2.getIncludeUUID());
return n(a).compareTo(n(b));
}

6
gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java
View File

@@ -19,7 +19,7 @@ import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupInclude;
import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuid;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gwtorm.server.SchemaFactory;
@@ -102,8 +102,8 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
}
Set<AccountGroup.Id> ids = Sets.newHashSet();
for (AccountGroupInclude agi : db.accountGroupIncludes()
.byInclude(group.get(0).getId())) {
for (AccountGroupIncludeByUuid agi : db.accountGroupIncludesByUuid()
.byIncludeUUID(group.get(0).getGroupUUID())) {
ids.add(agi.getGroupId());
}

25
gerrit-server/src/main/java/com/google/gerrit/server/account/GroupInfoCacheFactory.java
View File

@@ -31,35 +31,35 @@ public class GroupInfoCacheFactory {
}
private final GroupCache groupCache;
private final Map<AccountGroup.Id, AccountGroup> out;
private final Map<AccountGroup.UUID, AccountGroup> out;
@Inject
GroupInfoCacheFactory(final GroupCache groupCache) {
this.groupCache = groupCache;
this.out = new HashMap<AccountGroup.Id, AccountGroup>();
this.out = new HashMap<AccountGroup.UUID, AccountGroup>();
}
/**
* Indicate a group will be needed later on.
*
* @param id identity that will be needed in the future; may be null.
* @param uuid identity that will be needed in the future; may be null.
*/
public void want(final AccountGroup.Id id) {
if (id != null && !out.containsKey(id)) {
out.put(id, groupCache.get(id));
public void want(final AccountGroup.UUID uuid) {
if (uuid != null && !out.containsKey(uuid)) {
out.put(uuid, groupCache.get(uuid));
}
}
/** Indicate one or more groups will be needed later on. */
public void want(final Iterable<AccountGroup.Id> ids) {
for (final AccountGroup.Id id : ids) {
want(id);
public void want(final Iterable<AccountGroup.UUID> uuids) {
for (final AccountGroup.UUID uuid : uuids) {
want(uuid);
}
}
public AccountGroup get(final AccountGroup.Id id) {
want(id);
return out.get(id);
public AccountGroup get(final AccountGroup.UUID uuid) {
want(uuid);
return out.get(uuid);
}
/**
@@ -68,6 +68,7 @@ public class GroupInfoCacheFactory {
public GroupInfoCache create() {
final List<GroupInfo> r = new ArrayList<GroupInfo>(out.size());
for (final AccountGroup a : out.values()) {
if (a == null) continue;
r.add(new GroupInfo(a));
}
return new GroupInfoCache(r);

6
gerrit-server/src/main/java/com/google/gerrit/server/account/GroupMembers.java
View File

@@ -18,7 +18,7 @@ import com.google.gerrit.common.data.GroupDetail;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupInclude;
import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuid;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.IdentifiedUser;
@@ -111,9 +111,9 @@ public class GroupMembers {
}
}
if (groupDetail.includes != null) {
for (final AccountGroupInclude groupInclude : groupDetail.includes) {
for (final AccountGroupIncludeByUuid groupInclude : groupDetail.includes) {
final AccountGroup includedGroup =
groupCache.get(groupInclude.getIncludeId());
groupCache.get(groupInclude.getIncludeUUID());
if (!seen.contains(includedGroup.getGroupUUID())) {
members.addAll(listAccounts(includedGroup.getGroupUUID(), project, seen));
}

36
gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java
View File

@@ -18,8 +18,8 @@ import com.google.gerrit.common.errors.NameAlreadyUsedException;
import com.google.gerrit.common.errors.PermissionDeniedException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupInclude;
import com.google.gerrit.reviewdb.client.AccountGroupIncludeAudit;
import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuid;
import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuidAudit;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
import com.google.gerrit.reviewdb.client.AccountGroupMemberAudit;
import com.google.gerrit.reviewdb.client.AccountGroupName;
@@ -35,7 +35,6 @@ import org.eclipse.jgit.lib.PersonIdent;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
public class PerformCreateGroup {
@@ -89,7 +88,7 @@ public class PerformCreateGroup {
final String groupDescription, final boolean visibleToAll,
final AccountGroup.Id ownerGroupId,
final Collection<? extends Account.Id> initialMembers,
final Collection<? extends AccountGroup.Id> initialGroups)
final Collection<? extends AccountGroup.UUID> initialGroups)
throws OrmException, NameAlreadyUsedException, PermissionDeniedException {
if (!currentUser.getCapabilities().canCreateGroup()) {
throw new PermissionDeniedException(String.format(
@@ -160,26 +159,25 @@ public class PerformCreateGroup {
}
private void addGroups(final AccountGroup.Id groupId,
final Collection<? extends AccountGroup.Id> groups) throws OrmException {
final List<AccountGroupInclude> includeList =
new ArrayList<AccountGroupInclude>();
final List<AccountGroupIncludeAudit> includesAudit =
new ArrayList<AccountGroupIncludeAudit>();
for (AccountGroup.Id includeId : groups) {
final AccountGroupInclude groupInclude =
new AccountGroupInclude(new AccountGroupInclude.Key(groupId, includeId));
final Collection<? extends AccountGroup.UUID> groups) throws OrmException {
final List<AccountGroupIncludeByUuid> includeList =
new ArrayList<AccountGroupIncludeByUuid>();
final List<AccountGroupIncludeByUuidAudit> includesAudit =
new ArrayList<AccountGroupIncludeByUuidAudit>();
for (AccountGroup.UUID includeUUID : groups) {
final AccountGroupIncludeByUuid groupInclude =
new AccountGroupIncludeByUuid(new AccountGroupIncludeByUuid.Key(groupId, includeUUID));
includeList.add(groupInclude);
final AccountGroupIncludeAudit audit =
new AccountGroupIncludeAudit(groupInclude, currentUser.getAccountId());
final AccountGroupIncludeByUuidAudit audit =
new AccountGroupIncludeByUuidAudit(groupInclude, currentUser.getAccountId());
includesAudit.add(audit);
}
db.accountGroupIncludes().insert(includeList);
db.accountGroupIncludesAudit().insert(includesAudit);
db.accountGroupIncludesByUuid().insert(includeList);
db.accountGroupIncludesByUuidAudit().insert(includesAudit);
for (AccountGroup group : db.accountGroups().get(
new HashSet<AccountGroup.Id>(groups))) {
groupIncludeCache.evictInclude(group.getGroupUUID());
for (AccountGroup.UUID uuid : groups) {
groupIncludeCache.evictInclude(uuid);
}
}
}

14
gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
View File

@@ -20,7 +20,7 @@ import com.google.gerrit.common.data.GroupDescriptions;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupInclude;
import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuid;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
@@ -59,6 +59,7 @@ import java.text.MessageFormat;
import java.util.Collections;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Queue;
import java.util.Set;
import java.util.TreeSet;
@@ -476,10 +477,15 @@ public abstract class ChangeEmail extends OutgoingEmail {
.byGroup(next)) {
matching.accounts.add(m.getAccountId());
}
for (AccountGroupInclude m : args.db.get().accountGroupIncludes()
for (AccountGroupIncludeByUuid m : args.db.get().accountGroupIncludesByUuid()
.byGroup(next)) {
if (seen.add(m.getIncludeId())) {
scan.add(m.getIncludeId());
List<AccountGroup> incGroup = args.db.get().accountGroups().
byUUID(m.getIncludeUUID()).toList();
if (incGroup.size() == 1) {
AccountGroup.Id includeId = incGroup.get(0).getId();
if (seen.add(includeId)) {
scan.add(includeId);
}
}
}
}

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

@@ -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_73> C = Schema_73.class;
public static final Class<Schema_74> C = Schema_74.class;
public static class Module extends AbstractModule {
@Override

13
gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_57.java
View File

@@ -30,6 +30,7 @@ import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.LocalDiskRepositoryManager;
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gwtorm.jdbc.JdbcSchema;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -41,6 +42,8 @@ import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.eclipse.jgit.util.FS;
import java.io.IOException;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.util.Collections;
public class Schema_57 extends SchemaVersion {
@@ -95,6 +98,11 @@ public class Schema_57 extends SchemaVersion {
// Move the repository.*.createGroup to Create Project.
String[] createGroupList = cfg.getStringList("repository", "*", "createGroup");
// Prepare the account_group_includes query
PreparedStatement stmt = ((JdbcSchema) db).getConnection().
prepareStatement("SELECT * FROM account_group_includes WHERE group_id = ?");
for (String name : createGroupList) {
AccountGroup.NameKey key = new AccountGroup.NameKey(name);
AccountGroupName groupName = db.accountGroupNames().get(key);
@@ -117,9 +125,10 @@ public class Schema_57 extends SchemaVersion {
}
AccountGroup batch = db.accountGroups().get(sc.batchUsersGroupId);
stmt.setInt(0, sc.batchUsersGroupId.get());
if (batch != null
&& db.accountGroupMembers().byGroup(sc.batchUsersGroupId).toList().isEmpty()
&& db.accountGroupIncludes().byGroup(sc.batchUsersGroupId).toList().isEmpty()) {
&& stmt.executeQuery().first() != false) {
// If the batch user group is not used, delete it.
//
db.accountGroups().delete(Collections.singleton(batch));
@@ -136,6 +145,8 @@ public class Schema_57 extends SchemaVersion {
md.setMessage("Upgrade to Gerrit Code Review schema 57\n");
config.commit(md);
} catch (SQLException err) {
throw new OrmException( "Cannot read account_group_includes", err);
} finally {
git.close();
}

114
gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_74.java Normal file
View File

@@ -0,0 +1,114 @@
// 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.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuid;
import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuidAudit;
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.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
import java.util.HashMap;
/* Handles copying all entries from AccountGroupIncludes(Audit) to the new tables */
public class Schema_74 extends SchemaVersion {
@Inject
Schema_74(Provider<Schema_73> prior) {
super(prior);
}
@Override
protected void migrateData(final ReviewDb db, final UpdateUI ui)
throws SQLException, OrmException {
// Grab all the groups since we don't have the cache available
HashMap<AccountGroup.Id, AccountGroup.UUID> allGroups =
new HashMap<AccountGroup.Id, AccountGroup.UUID>();
for( AccountGroup ag : db.accountGroups().all() ) {
allGroups.put(ag.getId(), ag.getGroupUUID());
}
// Initialize some variables
Connection conn = ((JdbcSchema) db).getConnection();
ArrayList<AccountGroupIncludeByUuid> newIncludes =
new ArrayList<AccountGroupIncludeByUuid>();
ArrayList<AccountGroupIncludeByUuidAudit> newIncludeAudits =
new ArrayList<AccountGroupIncludeByUuidAudit>();
// Iterate over all entries in account_group_includes
Statement oldGroupIncludesStmt = conn.createStatement();
ResultSet oldGroupIncludes = oldGroupIncludesStmt.
executeQuery("SELECT * FROM account_group_includes");
while (oldGroupIncludes.next()) {
AccountGroup.Id oldGroupId =
new AccountGroup.Id(oldGroupIncludes.getInt("group_id"));
AccountGroup.Id oldIncludeId =
new AccountGroup.Id(oldGroupIncludes.getInt("include_id"));
AccountGroup.UUID uuidFromIncludeId = allGroups.get(oldIncludeId);
// If we've got an include, but the group no longer exists, don't bother converting
if (uuidFromIncludeId == null) {
ui.message("Skipping group_id = \"" + oldIncludeId.get() +
"\", not a current group");
continue;
}
// Create the new include entry
AccountGroupIncludeByUuid destIncludeEntry = new AccountGroupIncludeByUuid(
new AccountGroupIncludeByUuid.Key(oldGroupId, uuidFromIncludeId));
// Iterate over all the audits (for this group)
PreparedStatement oldAuditsQuery = conn.prepareStatement(
"SELECT * FROM account_group_includes_audit WHERE group_id=? AND include_id=?");
oldAuditsQuery.setInt(1, oldGroupId.get());
oldAuditsQuery.setInt(2, oldIncludeId.get());
ResultSet oldGroupIncludeAudits = oldAuditsQuery.executeQuery();
while (oldGroupIncludeAudits.next()) {
Account.Id addedBy = new Account.Id(oldGroupIncludeAudits.getInt("added_by"));
int removedBy = oldGroupIncludeAudits.getInt("removed_by");
// Create the new audit entry
AccountGroupIncludeByUuidAudit destAuditEntry =
new AccountGroupIncludeByUuidAudit(destIncludeEntry, addedBy,
oldGroupIncludeAudits.getTimestamp("added_on"));
// If this was a "removed on" entry, note that
if (removedBy > 0) {
destAuditEntry.removed(new Account.Id(removedBy),
oldGroupIncludeAudits.getTimestamp("removed_on"));
}
newIncludeAudits.add(destAuditEntry);
}
newIncludes.add(destIncludeEntry);
oldAuditsQuery.close();
oldGroupIncludeAudits.close();
}
oldGroupIncludes.close();
oldGroupIncludesStmt.close();
// Now insert all of the new entries to the database
db.accountGroupIncludesByUuid().insert(newIncludes);
db.accountGroupIncludesByUuidAudit().insert(newIncludeAudits);
}
}