Remove ProjectControl from the group stack

This is a step towards using PermissionBackend everywheren and make
ProjectControl package-private.

Change-Id: Ic9f9392c446382b227abb506c0f0ae292c08963b
This commit is contained in:
Patrick Hiesel
2017-10-11 10:52:16 +02:00
parent 5015709198
commit 53b64df7ad
6 changed files with 27 additions and 43 deletions

View File

@@ -111,7 +111,7 @@ public class ReviewersUtil {
private final AccountQueryBuilder accountQueryBuilder; private final AccountQueryBuilder accountQueryBuilder;
private final Provider<AccountQueryProcessor> queryProvider; private final Provider<AccountQueryProcessor> queryProvider;
private final GroupBackend groupBackend; private final GroupBackend groupBackend;
private final GroupMembers.Factory groupMembersFactory; private final GroupMembers groupMembers;
private final Provider<CurrentUser> currentUser; private final Provider<CurrentUser> currentUser;
private final ReviewerRecommender reviewerRecommender; private final ReviewerRecommender reviewerRecommender;
private final Metrics metrics; private final Metrics metrics;
@@ -122,7 +122,7 @@ public class ReviewersUtil {
AccountQueryBuilder accountQueryBuilder, AccountQueryBuilder accountQueryBuilder,
Provider<AccountQueryProcessor> queryProvider, Provider<AccountQueryProcessor> queryProvider,
GroupBackend groupBackend, GroupBackend groupBackend,
GroupMembers.Factory groupMembersFactory, GroupMembers groupMembers,
Provider<CurrentUser> currentUser, Provider<CurrentUser> currentUser,
ReviewerRecommender reviewerRecommender, ReviewerRecommender reviewerRecommender,
Metrics metrics) { Metrics metrics) {
@@ -133,7 +133,7 @@ public class ReviewersUtil {
this.queryProvider = queryProvider; this.queryProvider = queryProvider;
this.currentUser = currentUser; this.currentUser = currentUser;
this.groupBackend = groupBackend; this.groupBackend = groupBackend;
this.groupMembersFactory = groupMembersFactory; this.groupMembers = groupMembers;
this.reviewerRecommender = reviewerRecommender; this.reviewerRecommender = reviewerRecommender;
this.metrics = metrics; this.metrics = metrics;
} }
@@ -303,10 +303,7 @@ public class ReviewersUtil {
} }
try { try {
Set<Account> members = Set<Account> members = groupMembers.listAccounts(group.getUUID(), project.getNameKey());
groupMembersFactory
.create(currentUser.get())
.listAccounts(group.getUUID(), project.getNameKey());
if (members.isEmpty()) { if (members.isEmpty()) {
return result; return result;

View File

@@ -21,15 +21,14 @@ import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.group.InternalGroup; import com.google.gerrit.server.group.InternalGroup;
import com.google.gerrit.server.group.InternalGroupDescription; import com.google.gerrit.server.group.InternalGroupDescription;
import com.google.gerrit.server.group.SystemGroupBackend; import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.io.IOException; import java.io.IOException;
import java.util.Collections; import java.util.Collections;
import java.util.HashSet; import java.util.HashSet;
@@ -37,28 +36,22 @@ import java.util.Optional;
import java.util.Set; import java.util.Set;
public class GroupMembers { public class GroupMembers {
public interface Factory {
GroupMembers create(CurrentUser currentUser);
}
private final GroupCache groupCache; private final GroupCache groupCache;
private final GroupControl.Factory groupControlFactory; private final GroupControl.Factory groupControlFactory;
private final AccountCache accountCache; private final AccountCache accountCache;
private final ProjectControl.GenericFactory projectControl; private final ProjectCache projectCache;
private final CurrentUser currentUser;
@Inject @Inject
GroupMembers( GroupMembers(
GroupCache groupCache, GroupCache groupCache,
GroupControl.Factory groupControlFactory, GroupControl.Factory groupControlFactory,
AccountCache accountCache, AccountCache accountCache,
ProjectControl.GenericFactory projectControl, ProjectCache projectCache) {
@Assisted CurrentUser currentUser) {
this.groupCache = groupCache; this.groupCache = groupCache;
this.groupControlFactory = groupControlFactory; this.groupControlFactory = groupControlFactory;
this.accountCache = accountCache; this.accountCache = accountCache;
this.projectControl = projectControl; this.projectCache = projectCache;
this.currentUser = currentUser;
} }
public Set<Account> listAccounts(AccountGroup.UUID groupUUID, Project.NameKey project) public Set<Account> listAccounts(AccountGroup.UUID groupUUID, Project.NameKey project)
@@ -88,11 +81,13 @@ public class GroupMembers {
return Collections.emptySet(); return Collections.emptySet();
} }
final Iterable<AccountGroup.UUID> ownerGroups = ProjectState projectState = projectCache.checkedGet(project);
projectControl.controlFor(project, currentUser).getProjectState().getAllOwners(); if (projectState == null) {
throw new NoSuchProjectException(project);
}
final HashSet<Account> projectOwners = new HashSet<>(); final HashSet<Account> projectOwners = new HashSet<>();
for (AccountGroup.UUID ownerGroup : ownerGroups) { for (AccountGroup.UUID ownerGroup : projectState.getAllOwners()) {
if (!seen.contains(ownerGroup)) { if (!seen.contains(ownerGroup)) {
projectOwners.addAll(listAccounts(ownerGroup, project, seen)); projectOwners.addAll(listAccounts(ownerGroup, project, seen));
} }

View File

@@ -34,7 +34,6 @@ import com.google.gerrit.server.group.ListGroups;
import com.google.gerrit.server.group.QueryGroups; import com.google.gerrit.server.group.QueryGroups;
import com.google.gerrit.server.permissions.GlobalPermission; import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.ProjectResource; import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.project.ProjectsCollection; import com.google.gerrit.server.project.ProjectsCollection;
import com.google.inject.Inject; import com.google.inject.Inject;
@@ -54,7 +53,6 @@ class GroupsImpl implements Groups {
private final PermissionBackend permissionBackend; private final PermissionBackend permissionBackend;
private final CreateGroup.Factory createGroup; private final CreateGroup.Factory createGroup;
private final GroupApiImpl.Factory api; private final GroupApiImpl.Factory api;
private final ProjectControl.GenericFactory projectControlFactory;
@Inject @Inject
GroupsImpl( GroupsImpl(
@@ -66,8 +64,7 @@ class GroupsImpl implements Groups {
Provider<CurrentUser> user, Provider<CurrentUser> user,
PermissionBackend permissionBackend, PermissionBackend permissionBackend,
CreateGroup.Factory createGroup, CreateGroup.Factory createGroup,
GroupApiImpl.Factory api, GroupApiImpl.Factory api) {
ProjectControl.GenericFactory projectControlFactory) {
this.accounts = accounts; this.accounts = accounts;
this.groups = groups; this.groups = groups;
this.projects = projects; this.projects = projects;
@@ -77,7 +74,6 @@ class GroupsImpl implements Groups {
this.permissionBackend = permissionBackend; this.permissionBackend = permissionBackend;
this.createGroup = createGroup; this.createGroup = createGroup;
this.api = api; this.api = api;
this.projectControlFactory = projectControlFactory;
} }
@Override @Override
@@ -125,7 +121,7 @@ class GroupsImpl implements Groups {
for (String project : req.getProjects()) { for (String project : req.getProjects()) {
try { try {
ProjectResource rsrc = projects.parse(tlr, IdString.fromDecoded(project)); ProjectResource rsrc = projects.parse(tlr, IdString.fromDecoded(project));
list.addProject(projectControlFactory.controlFor(rsrc.getNameKey(), rsrc.getUser())); list.addProject(rsrc.getProjectState());
} catch (Exception e) { } catch (Exception e) {
throw asRestApiException("Error looking up project " + project, e); throw asRestApiException("Error looking up project " + project, e);
} }

View File

@@ -91,7 +91,7 @@ public class PostReviewers
private final PermissionBackend permissionBackend; private final PermissionBackend permissionBackend;
private final GroupsCollection groupsCollection; private final GroupsCollection groupsCollection;
private final GroupMembers.Factory groupMembersFactory; private final GroupMembers groupMembers;
private final AccountLoader.Factory accountLoaderFactory; private final AccountLoader.Factory accountLoaderFactory;
private final Provider<ReviewDb> dbProvider; private final Provider<ReviewDb> dbProvider;
private final ChangeData.Factory changeDataFactory; private final ChangeData.Factory changeDataFactory;
@@ -111,7 +111,7 @@ public class PostReviewers
ReviewerResource.Factory reviewerFactory, ReviewerResource.Factory reviewerFactory,
PermissionBackend permissionBackend, PermissionBackend permissionBackend,
GroupsCollection groupsCollection, GroupsCollection groupsCollection,
GroupMembers.Factory groupMembersFactory, GroupMembers groupMembers,
AccountLoader.Factory accountLoaderFactory, AccountLoader.Factory accountLoaderFactory,
Provider<ReviewDb> db, Provider<ReviewDb> db,
ChangeData.Factory changeDataFactory, ChangeData.Factory changeDataFactory,
@@ -130,7 +130,7 @@ public class PostReviewers
this.reviewerFactory = reviewerFactory; this.reviewerFactory = reviewerFactory;
this.permissionBackend = permissionBackend; this.permissionBackend = permissionBackend;
this.groupsCollection = groupsCollection; this.groupsCollection = groupsCollection;
this.groupMembersFactory = groupMembersFactory; this.groupMembers = groupMembers;
this.accountLoaderFactory = accountLoaderFactory; this.accountLoaderFactory = accountLoaderFactory;
this.dbProvider = db; this.dbProvider = db;
this.changeDataFactory = changeDataFactory; this.changeDataFactory = changeDataFactory;
@@ -287,10 +287,7 @@ public class PostReviewers
Set<Account.Id> reviewers = new HashSet<>(); Set<Account.Id> reviewers = new HashSet<>();
Set<Account> members; Set<Account> members;
try { try {
members = members = groupMembers.listAccounts(group.getGroupUUID(), rsrc.getProject());
groupMembersFactory
.create(rsrc.getUser())
.listAccounts(group.getGroupUUID(), rsrc.getProject());
} catch (NoSuchGroupException e) { } catch (NoSuchGroupException e) {
return fail( return fail(
reviewer, reviewer,

View File

@@ -91,7 +91,6 @@ import com.google.gerrit.server.account.EmailExpander;
import com.google.gerrit.server.account.GroupCacheImpl; import com.google.gerrit.server.account.GroupCacheImpl;
import com.google.gerrit.server.account.GroupControl; import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.account.GroupIncludeCacheImpl; import com.google.gerrit.server.account.GroupIncludeCacheImpl;
import com.google.gerrit.server.account.GroupMembers;
import com.google.gerrit.server.account.VersionedAuthorizedKeys; import com.google.gerrit.server.account.VersionedAuthorizedKeys;
import com.google.gerrit.server.account.externalids.ExternalIdModule; import com.google.gerrit.server.account.externalids.ExternalIdModule;
import com.google.gerrit.server.api.accounts.AccountExternalIdCreator; import com.google.gerrit.server.api.accounts.AccountExternalIdCreator;
@@ -250,7 +249,6 @@ public class GerritGlobalModule extends FactoryModule {
factory(ChangeData.AssistedFactory.class); factory(ChangeData.AssistedFactory.class);
factory(ChangeJson.AssistedFactory.class); factory(ChangeJson.AssistedFactory.class);
factory(CreateChangeSender.Factory.class); factory(CreateChangeSender.Factory.class);
factory(GroupMembers.Factory.class);
factory(EmailMerge.Factory.class); factory(EmailMerge.Factory.class);
factory(MergedSender.Factory.class); factory(MergedSender.Factory.class);
factory(MergeUtil.Factory.class); factory(MergeUtil.Factory.class);

View File

@@ -67,7 +67,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
protected final GroupCache groupCache; protected final GroupCache groupCache;
private final List<ProjectControl> projects = new ArrayList<>(); private final List<ProjectState> projects = new ArrayList<>();
private final Set<AccountGroup.UUID> groupsToInspect = new HashSet<>(); private final Set<AccountGroup.UUID> groupsToInspect = new HashSet<>();
private final GroupControl.Factory groupControlFactory; private final GroupControl.Factory groupControlFactory;
private final GroupControl.GenericFactory genericGroupControlFactory; private final GroupControl.GenericFactory genericGroupControlFactory;
@@ -95,6 +95,10 @@ public class ListGroups implements RestReadView<TopLevelResource> {
usage = "projects for which the groups should be listed" usage = "projects for which the groups should be listed"
) )
public void addProject(ProjectControl project) { public void addProject(ProjectControl project) {
addProject(project.getProjectState());
}
public void addProject(ProjectState project) {
projects.add(project); projects.add(project);
} }
@@ -241,7 +245,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
return user; return user;
} }
public List<ProjectControl> getProjects() { public List<ProjectState> getProjects() {
return projects; return projects;
} }
@@ -298,7 +302,6 @@ public class ListGroups implements RestReadView<TopLevelResource> {
if (!projects.isEmpty()) { if (!projects.isEmpty()) {
return projects return projects
.stream() .stream()
.map(ProjectControl::getProjectState)
.map(ProjectState::getAllGroups) .map(ProjectState::getAllGroups)
.flatMap(Collection::stream) .flatMap(Collection::stream)
.map(GroupReference::getUUID) .map(GroupReference::getUUID)
@@ -318,9 +321,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
List<GroupReference> groupRefs = List<GroupReference> groupRefs =
Lists.newArrayList( Lists.newArrayList(
Iterables.limit( Iterables.limit(
groupBackend.suggest( groupBackend.suggest(suggest, projects.stream().findFirst().orElse(null)),
suggest,
projects.stream().findFirst().map(pc -> pc.getProjectState()).orElse(null)),
limit <= 0 ? 10 : Math.min(limit, 10))); limit <= 0 ? 10 : Math.min(limit, 10)));
List<GroupInfo> groupInfos = Lists.newArrayListWithCapacity(groupRefs.size()); List<GroupInfo> groupInfos = Lists.newArrayListWithCapacity(groupRefs.size());