Return ImmutableList from ResultSet

Change-Id: Ie3883f2a1a9601f6813ea789eb0cb27830bd924f

java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java

@@ -20,6 +20,7 @@ import static com.google.gson.FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.Lists;
@@ -38,6 +39,7 @@ import com.google.gerrit.index.QueryOptions;
 import com.google.gerrit.index.Schema;
 import com.google.gerrit.index.query.DataSource;
 import com.google.gerrit.index.query.FieldBundle;
+import com.google.gerrit.index.query.ListResultSet;
 import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.index.query.QueryParseException;
 import com.google.gerrit.index.query.ResultSet;
@@ -62,7 +64,6 @@ import java.net.URLEncoder;
 import java.sql.Timestamp;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -385,7 +386,6 @@ abstract class AbstractElasticIndex<K, V> implements Index<K, V> {
 
     private <T> ResultSet<T> readImpl(Function<JsonObject, T> mapper) throws OrmException {
       try {
-        List<T> results = Collections.emptyList();
         String uri = getURI(index, SEARCH);
         Response response =
             performRequest(HttpPost.METHOD_NAME, uri, search, Collections.emptyMap());
@@ -396,34 +396,19 @@ abstract class AbstractElasticIndex<K, V> implements Index<K, V> {
               new JsonParser().parse(content).getAsJsonObject().getAsJsonObject("hits");
           if (obj.get("hits") != null) {
             JsonArray json = obj.getAsJsonArray("hits");
-            results = Lists.newArrayListWithCapacity(json.size());
+            ImmutableList.Builder<T> results = ImmutableList.builderWithExpectedSize(json.size());
             for (int i = 0; i < json.size(); i++) {
               T mapperResult = mapper.apply(json.get(i).getAsJsonObject());
               if (mapperResult != null) {
                 results.add(mapperResult);
               }
             }
+            return new ListResultSet<>(results.build());
           }
         } else {
           logger.atSevere().log(statusLine.getReasonPhrase());
         }
-        final List<T> r = Collections.unmodifiableList(results);
+        return new ListResultSet<>(ImmutableList.of());
-        return new ResultSet<T>() {
-          @Override
-          public Iterator<T> iterator() {
-            return r.iterator();
-          }
-
-          @Override
-          public List<T> toList() {
-            return r;
-          }
-
-          @Override
-          public void close() {
-            // Do nothing.
-          }
-        };
       } catch (IOException e) {
         throw new OrmException(e);
       }

java/com/google/gerrit/index/Index.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.index;
 
+import com.google.common.collect.ImmutableList;
 import com.google.gerrit.index.query.DataSource;
 import com.google.gerrit.index.query.FieldBundle;
 import com.google.gerrit.index.query.IndexPredicate;
@@ -21,7 +22,6 @@ import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.index.query.QueryParseException;
 import com.google.gwtorm.server.OrmException;
 import java.io.IOException;
-import java.util.List;
 import java.util.Optional;
 
 /**
@@ -95,7 +95,7 @@ public interface Index<K, V> {
    */
   default Optional<V> get(K key, QueryOptions opts) throws IOException {
     opts = opts.withStart(0).withLimit(2);
-    List<V> results;
+    ImmutableList<V> results;
     try {
       results = getSource(keyPredicate(key), opts).read().toList();
     } catch (QueryParseException e) {
@@ -120,7 +120,7 @@ public interface Index<K, V> {
    */
   default Optional<FieldBundle> getRaw(K key, QueryOptions opts) throws IOException {
     opts = opts.withStart(0).withLimit(2);
-    List<FieldBundle> results;
+    ImmutableList<FieldBundle> results;
     try {
       results = getSource(keyPredicate(key), opts).readRaw().toList();
     } catch (QueryParseException e) {

java/com/google/gerrit/index/query/ListResultSet.java

@@ -14,15 +14,15 @@
 
 package com.google.gerrit.index.query;
 
-import java.util.ArrayList;
+import com.google.common.collect.ImmutableList;
 import java.util.Iterator;
 import java.util.List;
 
 public class ListResultSet<T> implements ResultSet<T> {
-  private List<T> items;
+  private ImmutableList<T> items;
 
-  public ListResultSet(final List<T> r) {
+  public ListResultSet(List<T> r) {
-    items = r;
+    items = ImmutableList.copyOf(r);
   }
 
   @Override
@@ -31,11 +31,11 @@ public class ListResultSet<T> implements ResultSet<T> {
   }
 
   @Override
-  public List<T> toList() {
+  public ImmutableList<T> toList() {
     if (items == null) {
       throw new IllegalStateException("Results already obtained");
     }
-    final List<T> r = new ArrayList<>(items);
+    ImmutableList<T> r = items;
     items = null;
     return r;
   }

java/com/google/gerrit/index/query/QueryProcessor.java

@@ -266,7 +266,7 @@ public abstract class QueryProcessor<T> {
 
       out = new ArrayList<>(cnt);
       for (int i = 0; i < cnt; i++) {
-        List<T> matchesList = matches.get(i).toList();
+        ImmutableList<T> matchesList = matches.get(i).toList();
         logger.atFine().log(
             "Matches[%d]:\n%s",
             i, lazy(() -> matchesList.stream().map(this::formatForLogging).collect(toSet())));

java/com/google/gerrit/index/query/QueryResult.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.index.query;
 
 import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableList;
 import com.google.gerrit.common.Nullable;
 import java.util.List;
 
@@ -22,15 +23,15 @@ import java.util.List;
 @AutoValue
 public abstract class QueryResult<T> {
   public static <T> QueryResult<T> create(
-      @Nullable String query, Predicate<T> predicate, int limit, List<T> entites) {
+      @Nullable String query, Predicate<T> predicate, int limit, List<T> entities) {
     boolean more;
-    if (entites.size() > limit) {
+    if (entities.size() > limit) {
       more = true;
-      entites = entites.subList(0, limit);
+      entities = entities.subList(0, limit);
     } else {
       more = false;
     }
-    return new AutoValue_QueryResult<>(query, predicate, entites, more);
+    return new AutoValue_QueryResult<>(query, predicate, ImmutableList.copyOf(entities), more);
   }
 
   /** @return the original query string, or null if the query was created programmatically. */
@@ -41,7 +42,7 @@ public abstract class QueryResult<T> {
   public abstract Predicate<T> predicate();
 
   /** @return the query results. */
-  public abstract List<T> entities();
+  public abstract ImmutableList<T> entities();
 
   /**
    * @return whether the query could be retried with a higher start/limit to produce more results.

java/com/google/gerrit/index/query/ResultSet.java

@@ -14,8 +14,8 @@
 
 package com.google.gerrit.index.query;
 
+import com.google.common.collect.ImmutableList;
 import java.util.Iterator;
-import java.util.List;
 
 /**
  * Result from any data store query function.
@@ -38,9 +38,9 @@ public interface ResultSet<T> extends Iterable<T> {
    * <p>Prior to returning {@link #close()} is invoked. This method must not be combined with {@link
    * #iterator()} on the same instance.
    *
-   * @return mutable list of the complete results.
+   * @return immutable list of the complete results.
    */
-  List<T> toList();
+  ImmutableList<T> toList();
 
   /**
    * Close the result, discarding any further results.

java/com/google/gerrit/lucene/AbstractLuceneIndex.java

@@ -20,6 +20,7 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS;
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.Sets;
 import com.google.common.flogger.FluentLogger;
@@ -37,6 +38,7 @@ import com.google.gerrit.index.Schema;
 import com.google.gerrit.index.Schema.Values;
 import com.google.gerrit.index.query.DataSource;
 import com.google.gerrit.index.query.FieldBundle;
+import com.google.gerrit.index.query.ListResultSet;
 import com.google.gerrit.index.query.ResultSet;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.index.IndexUtils;
@@ -45,10 +47,6 @@ import com.google.gerrit.server.logging.LoggingContextAwareScheduledExecutorServ
 import com.google.gwtorm.server.OrmException;
 import java.io.IOException;
 import java.sql.Timestamp;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.Callable;
@@ -494,32 +492,16 @@ public abstract class AbstractLuceneIndex<K, V> implements Index<K, V> {
         searcher = acquire();
         int realLimit = opts.start() + opts.limit();
         TopFieldDocs docs = searcher.search(query, realLimit, sort);
-        List<T> result = new ArrayList<>(docs.scoreDocs.length);
+        ImmutableList.Builder<T> b = ImmutableList.builderWithExpectedSize(docs.scoreDocs.length);
         for (int i = opts.start(); i < docs.scoreDocs.length; i++) {
           ScoreDoc sd = docs.scoreDocs[i];
           Document doc = searcher.doc(sd.doc, opts.fields());
           T mapperResult = mapper.apply(doc);
           if (mapperResult != null) {
-            result.add(mapperResult);
+            b.add(mapperResult);
           }
         }
-        final List<T> r = Collections.unmodifiableList(result);
+        return new ListResultSet<>(b.build());
-        return new ResultSet<T>() {
-          @Override
-          public Iterator<T> iterator() {
-            return r.iterator();
-          }
-
-          @Override
-          public List<T> toList() {
-            return r;
-          }
-
-          @Override
-          public void close() {
-            // Do nothing.
-          }
-        };
       } catch (IOException e) {
         throw new OrmException(e);
       } finally {

java/com/google/gerrit/lucene/LuceneChangeIndex.java

@@ -28,6 +28,7 @@ import com.google.common.base.Function;
 import com.google.common.base.Throwables;
 import com.google.common.collect.Collections2;
 import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.MultimapBuilder;
@@ -333,7 +334,8 @@ public class LuceneChangeIndex implements ChangeIndex {
       } catch (IOException e) {
         throw new OrmException(e);
       }
-      List<FieldBundle> fieldBundles = documents.stream().map(rawDocumentMapper).collect(toList());
+      ImmutableList<FieldBundle> fieldBundles =
+          documents.stream().map(rawDocumentMapper).collect(toImmutableList());
       return new ResultSet<FieldBundle>() {
         @Override
         public Iterator<FieldBundle> iterator() {
@@ -341,7 +343,7 @@ public class LuceneChangeIndex implements ChangeIndex {
         }
 
         @Override
-        public List<FieldBundle> toList() {
+        public ImmutableList<FieldBundle> toList() {
           return fieldBundles;
         }
 
@@ -401,15 +403,16 @@ public class LuceneChangeIndex implements ChangeIndex {
     }
 
     @Override
-    public List<ChangeData> toList() {
+    public ImmutableList<ChangeData> toList() {
       try {
         List<Document> docs = future.get();
-        List<ChangeData> result = new ArrayList<>(docs.size());
+        ImmutableList.Builder<ChangeData> result =
+            ImmutableList.builderWithExpectedSize(docs.size());
         String idFieldName = LEGACY_ID.getName();
         for (Document doc : docs) {
           result.add(toChangeData(fields(doc, fields), fields, idFieldName));
         }
-        return result;
+        return result.build();
       } catch (InterruptedException e) {
         close();
         throw new OrmRuntimeException(e);

java/com/google/gerrit/server/index/change/IndexedChangeQuery.java

@@ -19,6 +19,7 @@ import static com.google.gerrit.server.index.change.ChangeField.CHANGE;
 import static com.google.gerrit.server.index.change.ChangeField.PROJECT;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.gerrit.index.IndexConfig;
@@ -37,7 +38,6 @@ import com.google.gwtorm.server.OrmException;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -98,8 +98,8 @@ public class IndexedChangeQuery extends IndexedQuery<Change.Id, ChangeData>
       }
 
       @Override
-      public List<ChangeData> toList() {
+      public ImmutableList<ChangeData> toList() {
-        List<ChangeData> r = rs.toList();
+        ImmutableList<ChangeData> r = rs.toList();
         for (ChangeData cd : r) {
           fromSource.put(cd, currSource);
         }