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 <edwin.kempin@sap.com>

gerrit-server/src/main/java/com/google/gerrit/server/account/VisibleGroups.java

@@ -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> identifiedUser;
-  private final GroupCache groupCache;
-  private final GroupControl.Factory groupControlFactory;
-
-  private boolean onlyVisibleToAll;
-  private AccountGroup.Type groupType;
-  private String match;
-
-  @Inject
-  VisibleGroups(final Provider<IdentifiedUser> 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<AccountGroup> get() {
-    return filterGroups(groupCache.all());
-  }
-
-  public List<AccountGroup> get(final Collection<ProjectControl> projects)
-      throws NoSuchGroupException {
-    Map<AccountGroup.UUID, AccountGroup> groups = Maps.newHashMap();
-    for (final ProjectControl projectControl : projects) {
-      final Set<GroupReference> 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<AccountGroup> filterGroups(final Iterable<AccountGroup> groups) {
-    final List<AccountGroup> 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;
-  }
-}

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);

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<TopLevelResource> {
 
-  private final VisibleGroups.Factory visibleGroupsFactory;
+  protected final GroupCache groupCache;
+
+  private final GroupControl.Factory groupControlFactory;
+  private final Provider<IdentifiedUser> identifiedUser;
   private final IdentifiedUser.GenericFactory userFactory;
   private final Provider<GetGroups> accountGetGroups;
 
@@ -68,10 +78,14 @@ public class ListGroups implements RestReadView<TopLevelResource> {
   private String matchSubstring;
 
   @Inject
-  protected ListGroups(final VisibleGroups.Factory visibleGroupsFactory,
+  protected ListGroups(final GroupCache groupCache,
+      final GroupControl.Factory groupControlFactory,
+      final Provider<IdentifiedUser> identifiedUser,
       final IdentifiedUser.GenericFactory userFactory,
-      Provider<GetGroups> accountGetGroups) {
-    this.visibleGroupsFactory = visibleGroupsFactory;
+      final Provider<GetGroups> 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<TopLevelResource> {
   }
 
   public List<GroupInfo> get() throws NoSuchGroupException {
-    List<GroupInfo> groups;
+    List<GroupInfo> 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<AccountGroup> groupList;
       if (!projects.isEmpty()) {
-        groupList = visibleGroups.get(projects);
+        Map<AccountGroup.UUID, AccountGroup> groups = Maps.newHashMap();
+        for (final ProjectControl projectControl : projects) {
+          final Set<GroupReference> 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<AccountGroup> filterGroups(final Iterable<AccountGroup> groups) {
+    final List<AccountGroup> 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;
   }
 }

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> identifiedUser,
         final IdentifiedUser.GenericFactory userFactory,
-        final Provider<GetGroups> accountGetGroups,
-        final GroupCache groupCache) {
-      super(visibleGroupsFactory, userFactory, accountGetGroups);
-      this.groupCache = groupCache;
+        final Provider<GetGroups> accountGetGroups) {
+      super(groupCache, groupControlFactory, identifiedUser, userFactory,
+          accountGetGroups);
     }
 
     void display(final PrintWriter out) throws NoSuchGroupException {