opendev/gerrit
Code Issues Proposed changes

Remove access of groups related db tables from GroupDetailFactory

Browse Source 
Change-Id: I35ae4b83f6d8309e441181f645833c05936a179b
This commit is contained in:
Alice Kober-Sotzek 2017-07-25 14:54:54 +02:00
parent a3d9d3bb7c
commit 7a3589a802
5 changed files with 52 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -14,24 +14,24 @@
package com.google.gerrit.common.data;
import com.google.gerrit.reviewdb.client.AccountGroupById;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
import java.util.List;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import java.util.Set;
public class GroupDetail {
private List<AccountGroupMember> members;
private List<AccountGroupById> includes;
private Set<Account.Id> members;
private Set<AccountGroup.UUID> includes;
public GroupDetail(List<AccountGroupMember> members, List<AccountGroupById> includes) {
public GroupDetail(Set<Account.Id> members, Set<AccountGroup.UUID> includes) {
this.members = members;
this.includes = includes;
}
public List<AccountGroupMember> getMembers() {
public Set<Account.Id> getMembers() {
return members;
}
public List<AccountGroupById> getIncludes() {
public Set<AccountGroup.UUID> getIncludes() {
return includes;
}
}

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

@ -14,18 +14,18 @@
package com.google.gerrit.server.account;
import com.google.common.collect.ImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import com.google.common.collect.ImmutableSet;
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.AccountGroupById;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.group.Groups;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;
public class GroupDetailFactory implements Callable<GroupDetail> {
@ -35,15 +35,20 @@ public class GroupDetailFactory implements Callable<GroupDetail> {
private final ReviewDb db;
private final GroupControl.Factory groupControl;
private final Groups groups;
private final AccountGroup.Id groupId;
private GroupControl control;
@Inject
GroupDetailFactory(
ReviewDb db, GroupControl.Factory groupControl, @Assisted AccountGroup.Id groupId) {
ReviewDb db,
GroupControl.Factory groupControl,
Groups groups,
@Assisted AccountGroup.Id groupId) {
this.db = db;
this.groupControl = groupControl;
this.groups = groups;
this.groupId = groupId;
}
@ -51,30 +56,20 @@ public class GroupDetailFactory implements Callable<GroupDetail> {
@Override
public GroupDetail call() throws OrmException, NoSuchGroupException {
control = groupControl.validateFor(groupId);
List<AccountGroupMember> members = loadMembers();
List<AccountGroupById> includes = loadIncludes();
ImmutableSet<Account.Id> members = loadMembers();
ImmutableSet<AccountGroup.UUID> includes = loadIncludes();
return new GroupDetail(members, includes);
}
private List<AccountGroupMember> loadMembers() throws OrmException {
List<AccountGroupMember> members = new ArrayList<>();
for (AccountGroupMember m : db.accountGroupMembers().byGroup(groupId)) {
if (control.canSeeMember(m.getAccountId())) {
members.add(m);
}
}
return members;
private ImmutableSet<Account.Id> loadMembers() throws OrmException {
return groups.getMembers(db, groupId).filter(control::canSeeMember).collect(toImmutableSet());
}
private List<AccountGroupById> loadIncludes() throws OrmException {
private ImmutableSet<AccountGroup.UUID> loadIncludes() throws OrmException {
if (!control.canSeeGroup()) {
return ImmutableList.of();
return ImmutableSet.of();
}
List<AccountGroupById> groups = new ArrayList<>();
for (AccountGroupById m : db.accountGroupById().byGroup(groupId)) {
groups.add(m);
}
return groups;
return groups.getIncludes(db, groupId).collect(toImmutableSet());
}
}

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

@ -18,8 +18,6 @@ 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.AccountGroupById;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.group.SystemGroupBackend;
@ -104,11 +102,12 @@ public class GroupMembers {
final GroupDetail groupDetail = groupDetailFactory.create(group.getId()).call();
final Set<Account> members = new HashSet<>();
for (AccountGroupMember member : groupDetail.getMembers()) {
members.add(accountCache.get(member.getAccountId()).getAccount());
for (Account.Id memberId : groupDetail.getMembers()) {
members.add(accountCache.get(memberId).getAccount());
}
for (AccountGroupById groupInclude : groupDetail.getIncludes()) {
final AccountGroup includedGroup = groupCache.get(groupInclude.getIncludeUUID());
for (AccountGroup.UUID groupIncludeUuid : groupDetail.getIncludes()) {
AccountGroup includedGroup = groupCache.get(groupIncludeUuid);
if (includedGroup != null && !seen.contains(includedGroup.getGroupUUID())) {
members.addAll(listAccounts(includedGroup.getGroupUUID(), project, seen));
}

15
gerrit-server/src/main/java/com/google/gerrit/server/group/Groups.java
View File

@ -16,16 +16,20 @@ package com.google.gerrit.server.group;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupById;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
import com.google.gerrit.reviewdb.client.AccountGroupName;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gwtorm.server.OrmDuplicateKeyException;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.ResultSet;
import com.google.inject.Singleton;
import java.util.List;
import java.util.Optional;
import java.util.stream.Stream;
@Singleton
public class Groups {
@ -65,4 +69,15 @@ public class Groups {
AccountGroupMember.Key key = new AccountGroupMember.Key(accountId, group.getId());
return db.accountGroupMembers().get(key) != null;
}
public Stream<Account.Id> getMembers(ReviewDb db, AccountGroup.Id groupId) throws OrmException {
ResultSet<AccountGroupMember> accountGroupMembers = db.accountGroupMembers().byGroup(groupId);
return Streams.stream(accountGroupMembers).map(AccountGroupMember::getAccountId);
}
public Stream<AccountGroup.UUID> getIncludes(ReviewDb db, AccountGroup.Id groupId)
throws OrmException {
ResultSet<AccountGroupById> accountGroupByIds = db.accountGroupById().byGroup(groupId);
return Streams.stream(accountGroupByIds).map(AccountGroupById::getIncludeUUID);
}
}

14
gerrit-server/src/main/java/com/google/gerrit/server/group/ListMembers.java
View File

@ -22,8 +22,6 @@ import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupById;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.account.GroupDetailFactory;
@ -102,16 +100,16 @@ public class ListMembers implements RestReadView<GroupResource> {
return Collections.emptyMap();
}
for (AccountGroupMember m : groupDetail.getMembers()) {
if (!members.containsKey(m.getAccountId())) {
members.put(m.getAccountId(), accountLoader.get(m.getAccountId()));
for (Account.Id member : groupDetail.getMembers()) {
if (!members.containsKey(member)) {
members.put(member, accountLoader.get(member));
}
}
if (recursive) {
for (AccountGroupById includedGroup : groupDetail.getIncludes()) {
if (!seenGroups.contains(includedGroup.getIncludeUUID())) {
members.putAll(getMembers(includedGroup.getIncludeUUID(), seenGroups));
for (AccountGroup.UUID includedGroupUuid : groupDetail.getIncludes()) {
if (!seenGroups.contains(includedGroupUuid)) {
members.putAll(getMembers(includedGroupUuid, seenGroups));
}
}
}