Merge "GroupCache: Return ImmutableList from all()"

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/group/GroupsIT.java

@@ -45,12 +45,9 @@ import org.junit.Test;
 
 import java.sql.Timestamp;
 import java.util.Arrays;
-import java.util.Collections;
-import java.util.Comparator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
-import java.util.stream.StreamSupport;
 
 @NoHttpd
 public class GroupsIT extends AbstractDaemonTest {
@@ -399,12 +396,10 @@ public class GroupsIT extends AbstractDaemonTest {
 
   @Test
   public void testListAllGroups() throws Exception {
-    List<String> expectedGroups =
+    List<String> expectedGroups = groupCache.all().stream()
-        StreamSupport.stream(groupCache.all().spliterator(), false)
           .map(a -> a.getName())
+          .sorted()
           .collect(toList());
-    Collections.sort(expectedGroups, Comparator.naturalOrder());
-
     assertThat(expectedGroups.size()).isAtLeast(2);
     assertThat(gApi.groups().list().getAsMap().keySet())
         .containsExactlyElementsIn(expectedGroups).inOrder();

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

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.account;
 
+import com.google.common.collect.ImmutableList;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 
@@ -31,8 +32,8 @@ public interface GroupCache {
   @Nullable
   AccountGroup get(AccountGroup.UUID uuid);
 
-  /** @return sorted iteration of groups. */
+  /** @return sorted list of groups. */
-  Iterable<AccountGroup> all();
+  ImmutableList<AccountGroup> all();
 
   /** Notify the cache that a new group was constructed. */
   void onCreateGroup(AccountGroup.NameKey newGroupName);

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

@@ -16,6 +16,7 @@ package com.google.gerrit.server.account;
 
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
+import com.google.common.collect.ImmutableList;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.AccountGroupName;
 import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -32,7 +33,6 @@ import com.google.inject.name.Named;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.ExecutionException;
@@ -151,12 +151,12 @@ public class GroupCacheImpl implements GroupCache {
   }
 
   @Override
-  public Iterable<AccountGroup> all() {
+  public ImmutableList<AccountGroup> all() {
     try (ReviewDb db = schema.open()) {
-      return Collections.unmodifiableList(db.accountGroups().all().toList());
+      return ImmutableList.copyOf(db.accountGroups().all());
     } catch (OrmException e) {
       log.warn("Cannot list internal groups", e);
-      return Collections.emptyList();
+      return ImmutableList.of();
     }
   }
 

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

@@ -28,7 +28,6 @@ import com.google.inject.Singleton;
 import org.eclipse.jgit.lib.ObjectId;
 
 import java.util.Collection;
-import java.util.stream.StreamSupport;
 
 /** Implementation of GroupBackend for the internal group system. */
 @Singleton
@@ -68,7 +67,7 @@ public class InternalGroupBackend implements GroupBackend {
   @Override
   public Collection<GroupReference> suggest(final String name,
       final ProjectControl project) {
-    return StreamSupport.stream(groupCache.all().spliterator(), false)
+    return groupCache.all().stream()
         .filter(group ->
             // startsWithIgnoreCase && isVisible
             group.getName().regionMatches(true, 0, name, 0, name.length())

gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java

@@ -45,6 +45,7 @@ import com.google.inject.Provider;
 import org.kohsuke.args4j.Option;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
@@ -314,11 +315,11 @@ public class ListGroups implements RestReadView<TopLevelResource> {
     return groups;
   }
 
-  private List<AccountGroup> filterGroups(final Iterable<AccountGroup> groups) {
+  private List<AccountGroup> filterGroups(Collection<AccountGroup> groups) {
-    final List<AccountGroup> filteredGroups = new ArrayList<>();
+    List<AccountGroup> filteredGroups = new ArrayList<>(groups.size());
-    final boolean isAdmin =
+    boolean isAdmin =
         identifiedUser.get().getCapabilities().canAdministrateServer();
-    for (final AccountGroup group : groups) {
+    for (AccountGroup group : groups) {
       if (!Strings.isNullOrEmpty(matchSubstring)) {
         if (!group.getName().toLowerCase(Locale.US)
             .contains(matchSubstring.toLowerCase(Locale.US))) {
@@ -326,7 +327,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
         }
       }
       if (!isAdmin) {
-        final GroupControl c = groupControlFactory.controlFor(group);
+        GroupControl c = groupControlFactory.controlFor(group);
         if (!c.isVisible()) {
           continue;
         }