From 0f55837a284783c89daed63feab78b64d68eefd5 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Tue, 22 Jan 2013 16:10:02 +0100 Subject: [PATCH] Merge VisibleGroups into ListGroups ListGroups was the only user of VisibleGroups. Instead of forwarding calls from ListGroups to VisibleGroups move the code into ListGroups and get rid of VisibleGroups. Change-Id: Ie5dd6035487448486036a94f11f1e9237cc19012 Signed-off-by: Edwin Kempin --- .../gerrit/server/account/VisibleGroups.java | 116 ------------------ .../server/config/GerritGlobalModule.java | 2 - .../gerrit/server/group/ListGroups.java | 80 +++++++++--- .../sshd/commands/ListGroupsCommand.java | 15 ++- 4 files changed, 71 insertions(+), 142 deletions(-) delete mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/account/VisibleGroups.java diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/VisibleGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/VisibleGroups.java deleted file mode 100644 index b44fa2f4a2..0000000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/VisibleGroups.java +++ /dev/null @@ -1,116 +0,0 @@ -// Copyright (C) 2011 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.account; - -import com.google.common.base.Strings; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; -import com.google.gerrit.common.data.GroupReference; -import com.google.gerrit.common.errors.NoSuchGroupException; -import com.google.gerrit.reviewdb.client.AccountGroup; -import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.project.ProjectControl; -import com.google.inject.Inject; -import com.google.inject.Provider; - -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.Set; - -public class VisibleGroups { - - public interface Factory { - VisibleGroups create(); - } - - private final Provider identifiedUser; - private final GroupCache groupCache; - private final GroupControl.Factory groupControlFactory; - - private boolean onlyVisibleToAll; - private AccountGroup.Type groupType; - private String match; - - @Inject - VisibleGroups(final Provider currentUser, - final GroupCache groupCache, - final GroupControl.Factory groupControlFactory) { - this.identifiedUser = currentUser; - this.groupCache = groupCache; - this.groupControlFactory = groupControlFactory; - } - - public void setOnlyVisibleToAll(final boolean onlyVisibleToAll) { - this.onlyVisibleToAll = onlyVisibleToAll; - } - - public void setGroupType(final AccountGroup.Type groupType) { - this.groupType = groupType; - } - - public void setMatch(final String match) { - this.match = match; - } - - public List get() { - return filterGroups(groupCache.all()); - } - - public List get(final Collection projects) - throws NoSuchGroupException { - Map groups = Maps.newHashMap(); - for (final ProjectControl projectControl : projects) { - final Set groupsRefs = projectControl.getAllGroups(); - for (final GroupReference groupRef : groupsRefs) { - final AccountGroup group = groupCache.get(groupRef.getUUID()); - if (group == null) { - throw new NoSuchGroupException(groupRef.getUUID()); - } - groups.put(group.getGroupUUID(), group); - } - } - return filterGroups(groups.values()); - } - - private List filterGroups(final Iterable groups) { - final List filteredGroups = Lists.newArrayList(); - final boolean isAdmin = - identifiedUser.get().getCapabilities().canAdministrateServer(); - for (final AccountGroup group : groups) { - if (!Strings.isNullOrEmpty(match)) { - if (!group.getName().toLowerCase(Locale.US) - .contains(match.toLowerCase(Locale.US))) { - continue; - } - } - if (!isAdmin) { - final GroupControl c = groupControlFactory.controlFor(group); - if (!c.isVisible()) { - continue; - } - } - if ((onlyVisibleToAll && !group.isVisibleToAll()) - || (groupType != null && !groupType.equals(group.getType()))) { - continue; - } - filteredGroups.add(group); - } - Collections.sort(filteredGroups, new GroupComparator()); - return filteredGroups; - } -} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java index 8f80deeaba..000af332c9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java @@ -54,7 +54,6 @@ import com.google.gerrit.server.account.IncludingGroupMembership; import com.google.gerrit.server.account.InternalGroupBackend; import com.google.gerrit.server.account.Realm; import com.google.gerrit.server.account.UniversalGroupBackend; -import com.google.gerrit.server.account.VisibleGroups; import com.google.gerrit.server.auth.AuthBackend; import com.google.gerrit.server.auth.InternalAuthBackend; import com.google.gerrit.server.auth.UniversalAuthBackend; @@ -159,7 +158,6 @@ public class GerritGlobalModule extends FactoryModule { factory(CapabilityControl.Factory.class); factory(ChangeQueryBuilder.Factory.class); factory(GroupInfoCacheFactory.Factory.class); - factory(VisibleGroups.Factory.class); factory(GroupDetailFactory.Factory.class); factory(InternalUser.Factory.class); factory(ProjectNode.Factory.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java index fb369b3957..bc356bf6a1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java @@ -15,9 +15,11 @@ package com.google.gerrit.server.group; import com.google.common.base.Objects; +import com.google.common.base.Strings; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.gerrit.common.data.GroupDescriptions; +import com.google.gerrit.common.data.GroupReference; import com.google.gerrit.common.errors.NoSuchGroupException; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; @@ -30,7 +32,9 @@ import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.OutputFormat; import com.google.gerrit.server.account.AccountResource; import com.google.gerrit.server.account.GetGroups; -import com.google.gerrit.server.account.VisibleGroups; +import com.google.gerrit.server.account.GroupCache; +import com.google.gerrit.server.account.GroupComparator; +import com.google.gerrit.server.account.GroupControl; import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.util.Url; import com.google.gson.reflect.TypeToken; @@ -40,13 +44,19 @@ import com.google.inject.Provider; import org.kohsuke.args4j.Option; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.Locale; import java.util.Map; +import java.util.Set; /** List groups visible to the calling user. */ public class ListGroups implements RestReadView { - private final VisibleGroups.Factory visibleGroupsFactory; + protected final GroupCache groupCache; + + private final GroupControl.Factory groupControlFactory; + private final Provider identifiedUser; private final IdentifiedUser.GenericFactory userFactory; private final Provider accountGetGroups; @@ -68,10 +78,14 @@ public class ListGroups implements RestReadView { private String matchSubstring; @Inject - protected ListGroups(final VisibleGroups.Factory visibleGroupsFactory, + protected ListGroups(final GroupCache groupCache, + final GroupControl.Factory groupControlFactory, + final Provider identifiedUser, final IdentifiedUser.GenericFactory userFactory, - Provider accountGetGroups) { - this.visibleGroupsFactory = visibleGroupsFactory; + final Provider accountGetGroups) { + this.groupCache = groupCache; + this.groupControlFactory = groupControlFactory; + this.identifiedUser = identifiedUser; this.userFactory = userFactory; this.accountGetGroups = accountGetGroups; } @@ -99,26 +113,60 @@ public class ListGroups implements RestReadView { } public List get() throws NoSuchGroupException { - List groups; + List groupInfos; if (user != null) { - groups = accountGetGroups.get().apply( + groupInfos = accountGetGroups.get().apply( new AccountResource(userFactory.create(user))); } else { - VisibleGroups visibleGroups = visibleGroupsFactory.create(); - visibleGroups.setOnlyVisibleToAll(visibleToAll); - visibleGroups.setGroupType(groupType); - visibleGroups.setMatch(matchSubstring); List groupList; if (!projects.isEmpty()) { - groupList = visibleGroups.get(projects); + Map groups = Maps.newHashMap(); + for (final ProjectControl projectControl : projects) { + final Set groupsRefs = projectControl.getAllGroups(); + for (final GroupReference groupRef : groupsRefs) { + final AccountGroup group = groupCache.get(groupRef.getUUID()); + if (group == null) { + throw new NoSuchGroupException(groupRef.getUUID()); + } + groups.put(group.getGroupUUID(), group); + } + } + groupList = filterGroups(groups.values()); } else { - groupList = visibleGroups.get(); + groupList = filterGroups(groupCache.all()); } - groups = Lists.newArrayListWithCapacity(groupList.size()); + groupInfos = Lists.newArrayListWithCapacity(groupList.size()); for (AccountGroup group : groupList) { - groups.add(new GroupInfo(GroupDescriptions.forAccountGroup(group))); + groupInfos.add(new GroupInfo(GroupDescriptions.forAccountGroup(group))); } } - return groups; + return groupInfos; + } + + private List filterGroups(final Iterable groups) { + final List filteredGroups = Lists.newArrayList(); + final boolean isAdmin = + identifiedUser.get().getCapabilities().canAdministrateServer(); + for (final AccountGroup group : groups) { + if (!Strings.isNullOrEmpty(matchSubstring)) { + if (!group.getName().toLowerCase(Locale.US) + .contains(matchSubstring.toLowerCase(Locale.US))) { + continue; + } + } + if (!isAdmin) { + final GroupControl c = groupControlFactory.controlFor(group); + if (!c.isVisible()) { + continue; + } + } + if ((visibleToAll && !group.isVisibleToAll()) + || (groupType != null && !groupType.equals(group.getType()))) { + continue; + } + filteredGroups.add(group); + } + Collections.sort(filteredGroups, new GroupComparator()); + return filteredGroups; } } diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListGroupsCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListGroupsCommand.java index 015e0afc66..0e46a45d57 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListGroupsCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListGroupsCommand.java @@ -21,7 +21,7 @@ import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.GetGroups; import com.google.gerrit.server.account.GroupCache; -import com.google.gerrit.server.account.VisibleGroups; +import com.google.gerrit.server.account.GroupControl; import com.google.gerrit.server.group.GroupInfo; import com.google.gerrit.server.group.ListGroups; import com.google.gerrit.server.ioutil.ColumnFormatter; @@ -65,15 +65,14 @@ public class ListGroupsCommand extends BaseCommand { "owner group UUID, and whether the group is visible to all") private boolean verboseOutput; - private final GroupCache groupCache; - @Inject - MyListGroups(final VisibleGroups.Factory visibleGroupsFactory, + MyListGroups(final GroupCache groupCache, + final GroupControl.Factory groupControlFactory, + final Provider identifiedUser, final IdentifiedUser.GenericFactory userFactory, - final Provider accountGetGroups, - final GroupCache groupCache) { - super(visibleGroupsFactory, userFactory, accountGetGroups); - this.groupCache = groupCache; + final Provider accountGetGroups) { + super(groupCache, groupControlFactory, identifiedUser, userFactory, + accountGetGroups); } void display(final PrintWriter out) throws NoSuchGroupException {