Simplify getting the children of a BranchHierarchyNode

Avoid repeatedly querying the ref database; this may lead to
inconsistencies when branches are deleted. Instead let the
RepositoriesViewContentProvider deal with this directly and
use its cached map of refs, like it does for tags.

Change-Id: I31f1d87d01f532972af70f66c95b87590db8b0a4
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

2 files changed, 26 insertions, 74 deletions

diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesViewContentProvider.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesViewContentProvider.java
index 6514bdd850..0fb5171b3d 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesViewContentProvider.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesViewContentProvider.java
@@ -28,9 +28,11 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Objects;
 import java.util.Set;
 import java.util.WeakHashMap;
 
@@ -39,7 +41,6 @@ import org.eclipse.core.commands.State;
 import org.eclipse.core.resources.IWorkspaceRoot;
 import org.eclipse.core.resources.WorkspaceJob;
 import org.eclipse.core.runtime.CoreException;
-import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Path;
@@ -218,10 +219,10 @@ public class RepositoriesViewContentProvider implements ITreeContentProvider,
 		case REMOTETRACKING:
 			return getBranchChildren(node, repo, Constants.R_REMOTES);
 
-		case BRANCHHIERARCHY: {
-			return getBranchHierarchyChildren((BranchHierarchyNode) node, repo,
-					node);
-		}
+		case BRANCHHIERARCHY:
+			return getBranchHierarchyChildren(node, repo,
+					((BranchHierarchyNode) node).getObject()
+							.toPortableString());
 
 		case TAGS:
 			return getTagsChildren(node, repo);
@@ -392,9 +393,7 @@ public class RepositoriesViewContentProvider implements ITreeContentProvider,
 	private Object[] getBranchChildren(RepositoryTreeNode node, Repository repo,
 			String prefix) {
 		if (branchHierarchyMode) {
-			return getBranchHierarchyChildren(
-					new BranchHierarchyNode(node, repo, new Path(prefix)), repo,
-					node);
+			return getBranchHierarchyChildren(node, repo, prefix);
 		} else {
 			try {
 				return getRefs(repo, prefix).values().stream()
@@ -406,19 +405,27 @@ public class RepositoriesViewContentProvider implements ITreeContentProvider,
 		}
 	}
 
-	private Object[] getBranchHierarchyChildren(BranchHierarchyNode hierNode,
-			Repository repo, RepositoryTreeNode parentNode) {
+	private Object[] getBranchHierarchyChildren(RepositoryTreeNode node,
+			Repository repo, String prefix) {
 		try {
-			List<RepositoryTreeNode> children = new ArrayList<>();
-			for (IPath path : hierNode.getChildPaths()) {
-				children.add(new BranchHierarchyNode(parentNode, repo, path));
-			}
-			for (Ref ref : hierNode.getChildRefs()) {
-				children.add(new RefNode(parentNode, repo, ref));
-			}
-			return children.toArray();
+			Set<String> folderChildren = new HashSet<>();
+			return getRefs(repo, prefix).entrySet().stream()
+					.filter(e -> !e.getValue().isSymbolic()).map(e -> {
+						int i = e.getKey().indexOf('/', prefix.length());
+						if (i < 0) {
+							return new RefNode(node, repo, e.getValue());
+						} else {
+							String name = e.getKey().substring(prefix.length(),
+									i);
+							if (folderChildren.add(name)) {
+								return new BranchHierarchyNode(node, repo,
+										Path.fromPortableString(prefix + name));
+							}
+							return null;
+						}
+					}).filter(Objects::nonNull).toArray();
 		} catch (IOException e) {
-			return handleException(e, parentNode);
+			return handleException(e, node);
 		}
 	}
 
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/BranchHierarchyNode.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/BranchHierarchyNode.java
index ab4dd67805..d9ae7ed263 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/BranchHierarchyNode.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/BranchHierarchyNode.java
@@ -13,13 +13,10 @@
 package org.eclipse.egit.ui.internal.repository.tree;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
 import java.util.stream.Collectors;
 
 import org.eclipse.core.runtime.IPath;
-import org.eclipse.core.runtime.Path;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 
@@ -45,47 +42,6 @@ public class BranchHierarchyNode extends RepositoryTreeNode<IPath> {
 	}
 
 	/**
-	 * @return the child paths
-	 * @throws IOException
-	 */
-	public List<IPath> getChildPaths() throws IOException {
-		List<IPath> result = new ArrayList<>();
-		for (IPath myPath : getPathList()) {
-			if (getObject().isPrefixOf(myPath)) {
-				int segmentDiff = myPath.segmentCount()
-						- getObject().segmentCount();
-				if (segmentDiff > 1) {
-					IPath newPath = getObject().append(
-							myPath.segment(getObject().segmentCount()));
-					if (!result.contains(newPath))
-						result.add(newPath);
-				}
-			}
-		}
-		return result;
-	}
-
-	/**
-	 * @return the direct child Refs (branches) only
-	 * @throws IOException
-	 */
-	public List<Ref> getChildRefs() throws IOException {
-		List<Ref> childRefs = new ArrayList<>();
-		for (IPath myPath : getPathList()) {
-			if (getObject().isPrefixOf(myPath)) {
-				int segmentDiff = myPath.segmentCount()
-						- getObject().segmentCount();
-				if (segmentDiff == 1) {
-					Ref ref = getRepository()
-							.findRef(myPath.toPortableString());
-					childRefs.add(ref);
-				}
-			}
-		}
-		return childRefs;
-	}
-
-	/**
 	 * @return all child Refs reachable from this hierarchy node
 	 * @throws IOException
 	 */
@@ -95,15 +51,4 @@ public class BranchHierarchyNode extends RepositoryTreeNode<IPath> {
 				.filter(ref -> !ref.isSymbolic()).collect(Collectors.toList());
 	}
 
-	private List<IPath> getPathList() throws IOException {
-		List<IPath> result = new ArrayList<>();
-		Map<String, Ref> refsMap = getRepository().getRefDatabase().getRefs(
-				getObject().toPortableString()); // getObject() returns path ending with /
-		for (Map.Entry<String, Ref> entry : refsMap.entrySet()) {
-			if (entry.getValue().isSymbolic())
-				continue;
-			result.add(getObject().append(new Path(entry.getKey())));
-		}
-		return result;
-	}
 }