opendev/gerrit
Code Issues Proposed changes

Merge "Allow group includes to be by UUID instead of group ID"

Browse Source
This commit is contained in:
Shawn Pearce
2013-01-10 13:01:20 -08:00
committed by Gerrit Code Review
parent 72276f39cb 2cf207f384
commit 97dbc3108a
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

@@ -127,15 +127,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

@@ -21,7 +21,7 @@ import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.common.errors.EmailException;
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;
@@ -60,6 +60,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;
@@ -477,10 +478,15 @@ public abstract class ChangeEmail extends NotificationEmail {
.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);
}
}