Merge changes from topic 'list-branches-cleanup'

* changes:
  ListBranches: Factor out method for listing all branches
  ListBranches: Handle HEAD and refs/meta/config in the Comparator
  ListBranches: Use FluentIterable for filter, start, and limit
  ListBranches: Remove unnecessary finals
  ListBranches: Don't ignore IOExceptions reading refs
  ListBranches: Be more precise about collection sizes
  ListBranches: Use Repository in try-with-resources

gerrit-server/src/main/java/com/google/gerrit/server/project/ListBranches.java

@@ -15,9 +15,9 @@
 package com.google.gerrit.server.project;
 
 import com.google.common.base.Predicate;
+import com.google.common.base.Strings;
+import com.google.common.collect.ComparisonChain;
 import com.google.common.collect.FluentIterable;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 import com.google.gerrit.extensions.common.ActionInfo;
 import com.google.gerrit.extensions.common.WebLinkInfo;
@@ -34,9 +34,6 @@ import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.inject.Inject;
 import com.google.inject.util.Providers;
 
-import dk.brics.automaton.RegExp;
-import dk.brics.automaton.RunAutomaton;
-
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.Ref;
@@ -45,6 +42,7 @@ import org.kohsuke.args4j.Option;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
@@ -53,6 +51,9 @@ import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
 
+import dk.brics.automaton.RegExp;
+import dk.brics.automaton.RunAutomaton;
+
 public class ListBranches implements RestReadView<ProjectResource> {
   private final GitRepositoryManager repoManager;
   private final DynamicMap<RestView<BranchResource>> branchViews;
@@ -82,47 +83,39 @@ public class ListBranches implements RestReadView<ProjectResource> {
   @Override
   public List<BranchInfo> apply(ProjectResource rsrc)
       throws ResourceNotFoundException, IOException, BadRequestException {
-    List<BranchInfo> branches = Lists.newArrayList();
+    FluentIterable<BranchInfo> branches = allBranches(rsrc);
+    branches = filterBranches(branches);
+    if (start > 0) {
+      branches = branches.skip(start);
+    }
+    if (limit > 0) {
+      branches = branches.limit(limit);
+    }
+    return branches.toList();
+  }
 
-    BranchInfo headBranch = null;
+  private FluentIterable<BranchInfo> allBranches(ProjectResource rsrc)
-    BranchInfo configBranch = null;
+      throws IOException, ResourceNotFoundException {
-    final Set<String> targets = Sets.newHashSet();
+    List<Ref> refs;
-
+    try (Repository db = repoManager.openRepository(rsrc.getNameKey())) {
-    final Repository db;
+      Collection<Ref> heads =
-    try {
+          db.getRefDatabase().getRefs(Constants.R_HEADS).values();
-      db = repoManager.openRepository(rsrc.getNameKey());
+      refs = new ArrayList<>(heads.size() + 2);
+      refs.addAll(heads);
+      addRef(db, refs, Constants.HEAD);
+      addRef(db, refs, RefNames.REFS_CONFIG);
     } catch (RepositoryNotFoundException noGitRepository) {
       throw new ResourceNotFoundException();
     }
 
-    try {
+    Set<String> targets = Sets.newHashSetWithExpectedSize(1);
-      List<Ref> refs =
-          new ArrayList<>(db.getRefDatabase().getRefs(Constants.R_HEADS)
-              .values());
-
-        try {
-          Ref head = db.getRef(Constants.HEAD);
-          if (head != null) {
-            refs.add(head);
-          }
-        } catch (IOException e) {
-          // Ignore the failure reading HEAD.
-        }
-        try {
-          Ref config = db.getRef(RefNames.REFS_CONFIG);
-          if (config != null) {
-            refs.add(config);
-          }
-        } catch (IOException e) {
-          // Ignore the failure reading refs/meta/config.
-        }
-
     for (Ref ref : refs) {
       if (ref.isSymbolic()) {
         targets.add(ref.getTarget().getName());
       }
     }
 
+    List<BranchInfo> branches = new ArrayList<>(refs.size());
     for (Ref ref : refs) {
       if (ref.isSymbolic()) {
         // A symbolic reference to another branch, instead of
@@ -138,94 +131,95 @@ public class ListBranches implements RestReadView<ProjectResource> {
         }
 
         BranchInfo b = new BranchInfo(ref.getName(), target, false);
-
-          if (Constants.HEAD.equals(ref.getName())) {
-            headBranch = b;
-          } else {
-            b.setCanDelete(targetRefControl.canDelete());
         branches.add(b);
+
+        if (!Constants.HEAD.equals(ref.getName())) {
+          b.setCanDelete(targetRefControl.canDelete());
         }
         continue;
       }
 
-        final RefControl refControl = rsrc.getControl().controlForRef(ref.getName());
+      RefControl refControl = rsrc.getControl().controlForRef(ref.getName());
       if (refControl.isVisible()) {
-          if (RefNames.REFS_CONFIG.equals(ref.getName())) {
-            configBranch = createBranchInfo(ref, refControl, targets);
-          } else {
         branches.add(createBranchInfo(ref, refControl, targets));
       }
     }
-      }
+    Collections.sort(branches, new BranchComparator());
-    } finally {
+    return FluentIterable.from(branches);
-      db.close();
-    }
-    Collections.sort(branches, new Comparator<BranchInfo>() {
-      @Override
-      public int compare(final BranchInfo a, final BranchInfo b) {
-        return a.ref.compareTo(b.ref);
-      }
-    });
-    if (configBranch != null) {
-      branches.add(0, configBranch);
-    }
-    if (headBranch != null) {
-      branches.add(0, headBranch);
   }
 
-    List<BranchInfo> filteredBranches;
+  private static class BranchComparator implements Comparator<BranchInfo> {
-    if ((matchSubstring != null && !matchSubstring.isEmpty())
+    @Override
-        || (matchRegex != null && !matchRegex.isEmpty())) {
+    public int compare(BranchInfo a, BranchInfo b) {
-      filteredBranches = filterBranches(branches);
+      return ComparisonChain.start()
-    } else {
+          .compareTrueFirst(isHead(a), isHead(b))
-      filteredBranches = branches;
+          .compareTrueFirst(isConfig(a), isConfig(b))
-    }
+          .compare(a.ref, b.ref)
-    if (!filteredBranches.isEmpty()) {
+          .result();
-      int end = filteredBranches.size();
-      if (limit > 0 && start + limit < end) {
-        end = start + limit;
-      }
-      if (start <= end) {
-        filteredBranches = filteredBranches.subList(start, end);
-      } else {
-        filteredBranches = Collections.emptyList();
-      }
-    }
-    return filteredBranches;
     }
 
-  private List<BranchInfo> filterBranches(List<BranchInfo> branches)
+    private static boolean isHead(BranchInfo i) {
-      throws BadRequestException {
+      return Constants.HEAD.equals(i.ref);
-    if (matchSubstring != null) {
+    }
-      return Lists.newArrayList(Iterables.filter(branches,
+
-          new Predicate<BranchInfo>() {
+    private static boolean isConfig(BranchInfo i) {
-            @Override
+      return RefNames.REFS_CONFIG.equals(i.ref);
-            public boolean apply(BranchInfo in) {
-              if (!in.ref.startsWith(Constants.R_HEADS)){
-                return in.ref.toLowerCase(Locale.US).contains(
-                    matchSubstring.toLowerCase(Locale.US));
-              } else {
-                return in.ref.substring(Constants.R_HEADS.length())
-                    .toLowerCase(Locale.US)
-                    .contains(matchSubstring.toLowerCase(Locale.US));
     }
   }
-          }));
+
-    } else if (matchRegex != null) {
+  private static void addRef(Repository db, List<Ref> refs, String name)
-      if (matchRegex.startsWith("^")) {
+      throws IOException {
-        matchRegex = matchRegex.substring(1);
+    Ref ref = db.getRef(name);
-        if (matchRegex.endsWith("$") && !matchRegex.endsWith("\\$")) {
+    if (ref != null) {
-          matchRegex = matchRegex.substring(0, matchRegex.length() - 1);
+      refs.add(ref);
     }
   }
-      if (matchRegex.equals(".*")) {
+
+  private FluentIterable<BranchInfo> filterBranches(
+      FluentIterable<BranchInfo> branches) throws BadRequestException {
+    if (!Strings.isNullOrEmpty(matchSubstring)) {
+      branches = branches.filter(new SubstringPredicate(matchSubstring));
+    } else if (!Strings.isNullOrEmpty(matchRegex)) {
+      branches = branches.filter(new RegexPredicate(matchRegex));
+    }
     return branches;
   }
+
+  private static class SubstringPredicate implements Predicate<BranchInfo> {
+    private final String substring;
+
+    private SubstringPredicate(String substring) {
+      this.substring = substring.toLowerCase(Locale.US);
+    }
+
+    @Override
+    public boolean apply(BranchInfo in) {
+      String ref = in.ref;
+      if (ref.startsWith(Constants.R_HEADS)) {
+        ref = ref.substring(Constants.R_HEADS.length());
+      }
+      ref = ref.toLowerCase(Locale.US);
+      return ref.contains(substring);
+    }
+  }
+
+  private static class RegexPredicate implements Predicate<BranchInfo> {
+    private final RunAutomaton a;
+
+    private RegexPredicate(String regex) throws BadRequestException {
+      if (regex.startsWith("^")) {
+        regex = regex.substring(1);
+        if (regex.endsWith("$") && !regex.endsWith("\\$")) {
+          regex = regex.substring(0, regex.length() - 1);
+        }
+      }
       try {
-        final RunAutomaton a =
+        a = new RunAutomaton(new RegExp(regex).toAutomaton());
-            new RunAutomaton(new RegExp(matchRegex).toAutomaton());
+      } catch (IllegalArgumentException e) {
-        return Lists.newArrayList(Iterables.filter(
+        throw new BadRequestException(e.getMessage());
-            branches, new Predicate<BranchInfo>() {
+      }
+    }
+
     @Override
     public boolean apply(BranchInfo in) {
       if (!in.ref.startsWith(Constants.R_HEADS)){
@@ -234,12 +228,6 @@ public class ListBranches implements RestReadView<ProjectResource> {
         return a.run(in.ref.substring(Constants.R_HEADS.length()));
       }
     }
-            }));
-      } catch (IllegalArgumentException e) {
-        throw new BadRequestException(e.getMessage());
-      }
-    }
-    return branches;
   }
 
   private BranchInfo createBranchInfo(Ref ref, RefControl refControl,