Allow deletion of branch hierarchy nodes

Up to now the user could not delete the branch hierarchy nodes. For
gerrit users this is a pain when trying to mass delete gerrit branches
with hierarchical layout being active.

The change expands the menu visibility, property testing and the
deletion command to explicitly handle the hierarchy branch nodes. A new
method has been introduced to get all refs of the hierarchy nodes
recursively. The deletion command then handles this list of refs,
instead of only taking one ref per selected node.

Overlapping selections (of branch nodes and their parent branch
hierarchy nodes) are handled correctly, because of the Set
implementation in the delete command.

Bug: 444869
Change-Id: I956954d07fcc44ca4a51ec3f681cf4f8ef97f86b
Signed-off-by: Michael Keppler <michael.keppler@gmx.de>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

5 files changed, 110 insertions, 36 deletions

diff --git a/org.eclipse.egit.ui/plugin.xml b/org.eclipse.egit.ui/plugin.xml
index da37e720be..dd0345182d 100644
--- a/org.eclipse.egit.ui/plugin.xml
+++ b/org.eclipse.egit.ui/plugin.xml
@@ -1487,6 +1487,9 @@
                            <instanceof
                                  value="org.eclipse.egit.ui.internal.repository.tree.RefNode">
                            </instanceof>
+                           <instanceof
+                                 value="org.eclipse.egit.ui.internal.repository.tree.BranchHierarchyNode">
+                           </instanceof>
                            <test
                                  property="GitRepository.isLocalBranch">
                            </test>
@@ -2394,6 +2397,9 @@
                            <instanceof
                                  value="org.eclipse.egit.ui.internal.repository.tree.RefNode">
                            </instanceof>
+                           <instanceof
+                                 value="org.eclipse.egit.ui.internal.repository.tree.BranchHierarchyNode">
+                           </instanceof>
                            <test
                                  property="GitRepository.isLocalBranch">
                            </test>
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 d5b56a12fb..ce4516f598 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
@@ -63,7 +63,7 @@ public class BranchHierarchyNode extends RepositoryTreeNode<IPath> {
 	}
 
 	/**
-	 * @return the child Refs (branches)
+	 * @return the direct child Refs (branches) only
 	 * @throws IOException
 	 */
 	public List<Ref> getChildRefs() throws IOException {
@@ -81,6 +81,21 @@ public class BranchHierarchyNode extends RepositoryTreeNode<IPath> {
 		return childRefs;
 	}
 
+	/**
+	 * @return all child Refs reachable from this hierarchy node
+	 * @throws IOException
+	 */
+	public List<Ref> getChildRefsRecursive() throws IOException {
+		List<Ref> childRefs = new ArrayList<>();
+		for (IPath myPath : getPathList()) {
+			if (getObject().isPrefixOf(myPath)) {
+				Ref ref = getRepository().exactRef(myPath.toPortableString());
+				childRefs.add(ref);
+			}
+		}
+		return childRefs;
+	}
+
 	private List<IPath> getPathList() throws IOException {
 		List<IPath> result = new ArrayList<>();
 		Map<String, Ref> refsMap = getRepository().getRefDatabase().getRefs(
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/RepositoriesViewPropertyTester.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/RepositoriesViewPropertyTester.java
index 6930020b10..c173518fec 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/RepositoriesViewPropertyTester.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/RepositoriesViewPropertyTester.java
@@ -66,28 +66,22 @@ public class RepositoriesViewPropertyTester extends PropertyTester {
 		}
 
 		if (property.equals("isRefCheckedOut")) { //$NON-NLS-1$
-			if (!(node.getObject() instanceof Ref))
-				return false;
-			Ref ref = (Ref) node.getObject();
-			try {
-				if (ref.getName().startsWith(Constants.R_REFS)) {
-					return ref.getName().equals(repository.getFullBranch());
-				} else if (ref.getName().equals(Constants.HEAD)) {
-					return true;
-				} else {
-					String leafname = ref.getLeaf().getName();
-					if (leafname.startsWith(Constants.R_REFS)
-							&& leafname.equals(repository.getFullBranch())) {
-						return true;
-					} else {
-						ObjectId objectId = ref.getLeaf().getObjectId();
-						return objectId != null && objectId
-								.equals(repository.resolve(Constants.HEAD));
+			if (node instanceof BranchHierarchyNode) {
+				try {
+					for (Ref ref : ((BranchHierarchyNode) node)
+							.getChildRefsRecursive()) {
+						if (isRefCheckedOut(repository, ref)) {
+							return true;
+						}
 					}
+				} catch (IOException e) {
+					return false;
 				}
-			} catch (IOException e) {
-				return false;
 			}
+			if (!(node.getObject() instanceof Ref))
+				return false;
+			Ref ref = (Ref) node.getObject();
+			return isRefCheckedOut(repository, ref);
 		}
 		if (property.equals("isLocalBranch")) { //$NON-NLS-1$
 			if (!(node.getObject() instanceof Ref))
@@ -152,6 +146,28 @@ public class RepositoriesViewPropertyTester extends PropertyTester {
 		return false;
 	}
 
+	private boolean isRefCheckedOut(Repository repository, Ref ref) {
+		try {
+			if (ref.getName().startsWith(Constants.R_REFS)) {
+				return ref.getName().equals(repository.getFullBranch());
+			} else if (ref.getName().equals(Constants.HEAD)) {
+				return true;
+			} else {
+				String leafname = ref.getLeaf().getName();
+				if (leafname.startsWith(Constants.R_REFS)
+						&& leafname.equals(repository.getFullBranch())) {
+					return true;
+				} else {
+					ObjectId objectId = ref.getLeaf().getObjectId();
+					return objectId != null && objectId
+							.equals(repository.resolve(Constants.HEAD));
+				}
+			}
+		} catch (IOException e) {
+			return false;
+		}
+	}
+
 	private boolean containsHead(Repository repository) {
 		try {
 			return repository.resolve(Constants.HEAD) != null;
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/RepositoryTreeNode.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/RepositoryTreeNode.java
index b54f230ca7..5f592f1e00 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/RepositoryTreeNode.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/RepositoryTreeNode.java
@@ -116,6 +116,10 @@ public abstract class RepositoryTreeNode<T> extends PlatformObject implements Co
 	 * <td>{@link String}</td>
 	 * </tr>
 	 * <tr>
+	 * <td>{@link RepositoryTreeNodeType#BRANCHHIERARCHY}</td>
+	 * <td>{@link IPath}</td>
+	 * </tr>
+	 * <tr>
 	 * <td>{@link RepositoryTreeNodeType#LOCAL}</td>
 	 * <td>{@link String}</td>
 	 * </tr>
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/command/DeleteBranchCommand.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/command/DeleteBranchCommand.java
index d8d18d00de..eea0518ad4 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/command/DeleteBranchCommand.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/command/DeleteBranchCommand.java
@@ -10,9 +10,13 @@
  *******************************************************************************/
 package org.eclipse.egit.ui.internal.repository.tree.command;
 
+import java.io.IOException;
 import java.lang.reflect.InvocationTargetException;
 import java.util.ArrayList;
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.eclipse.core.commands.ExecutionEvent;
@@ -25,24 +29,28 @@ import org.eclipse.egit.core.op.DeleteBranchOperation;
 import org.eclipse.egit.ui.Activator;
 import org.eclipse.egit.ui.internal.UIText;
 import org.eclipse.egit.ui.internal.dialogs.UnmergedBranchDialog;
+import org.eclipse.egit.ui.internal.repository.tree.BranchHierarchyNode;
 import org.eclipse.egit.ui.internal.repository.tree.RefNode;
+import org.eclipse.egit.ui.internal.repository.tree.RepositoryTreeNode;
 import org.eclipse.jface.dialogs.MessageDialog;
 import org.eclipse.jface.dialogs.ProgressMonitorDialog;
 import org.eclipse.jface.operation.IRunnableWithProgress;
 import org.eclipse.jface.window.Window;
 import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
 import org.eclipse.swt.widgets.Shell;
 
 /**
  * Deletes a branch.
  */
 public class DeleteBranchCommand extends
-		RepositoriesViewCommandHandler<RefNode> {
+		RepositoriesViewCommandHandler<RepositoryTreeNode> {
 
 	@Override
 	public Object execute(final ExecutionEvent event) throws ExecutionException {
-		final List<RefNode> nodes = getSelectedNodes(event);
-		final AtomicReference<List<RefNode>> unmergedNodesRef = new AtomicReference<>();
+		final List<RepositoryTreeNode> nodes = getSelectedNodes(event);
+		final Map<Ref, Repository> refs = getRefsToDelete(nodes);
+		final AtomicReference<Map<Ref, Repository>> unmergedNodesRef = new AtomicReference<>();
 		final Shell shell = getShell(event);
 
 		try {
@@ -52,7 +60,8 @@ public class DeleteBranchCommand extends
 						public void run(IProgressMonitor monitor)
 								throws InvocationTargetException,
 								InterruptedException {
-							List<RefNode> unmergedNodes = deleteBranches(nodes, false, monitor);
+							Map<Ref, Repository> unmergedNodes = deleteBranches(
+									refs, false, monitor);
 							unmergedNodesRef.set(unmergedNodes);
 						}
 					});
@@ -66,7 +75,7 @@ public class DeleteBranchCommand extends
 		if (unmergedNodesRef.get().isEmpty())
 			return null;
 		MessageDialog messageDialog = new UnmergedBranchDialog<>(
-					shell, unmergedNodesRef.get());
+					shell, new ArrayList<>(unmergedNodesRef.get().keySet()));
 		if (messageDialog.open() != Window.OK)
 			return null;
 		try {
@@ -90,19 +99,43 @@ public class DeleteBranchCommand extends
 		return null;
 	}
 
-	private List<RefNode> deleteBranches(final List<RefNode> nodes,
+	private Map<Ref, Repository> getRefsToDelete(
+			List<RepositoryTreeNode> nodes) {
+		LinkedHashMap<Ref, Repository> refs = new LinkedHashMap<>();
+		for (RepositoryTreeNode node : nodes) {
+			if (node instanceof BranchHierarchyNode) {
+				try {
+					for (Ref ref : ((BranchHierarchyNode) node)
+							.getChildRefsRecursive()) {
+						refs.put(ref, node.getRepository());
+					}
+				} catch (IOException e) {
+					// ignore
+				}
+			} else if (node instanceof RefNode) {
+				refs.put((Ref) node.getObject(), node.getRepository());
+			}
+		}
+		return refs;
+	}
+
+	private Map<Ref, Repository> deleteBranches(final Map<Ref, Repository> refs,
 			final boolean forceDeletionOfUnmergedBranches,
 			IProgressMonitor progressMonitor) throws InvocationTargetException {
-		final List<RefNode> unmergedNodes = new ArrayList<>();
+		final Map<Ref, Repository> unmergedNodes = new LinkedHashMap<>();
 		try {
 			ResourcesPlugin.getWorkspace().run(new IWorkspaceRunnable() {
 
 				@Override
 				public void run(IProgressMonitor monitor) throws CoreException {
-					monitor.beginTask(UIText.DeleteBranchCommand_DeletingBranchesProgress, nodes.size());
-					for (RefNode refNode : nodes) {
-						int result = deleteBranch(refNode, refNode
-								.getObject(), forceDeletionOfUnmergedBranches);
+					monitor.beginTask(
+							UIText.DeleteBranchCommand_DeletingBranchesProgress,
+							refs.size());
+					for (Entry<Ref, Repository> entry : refs.entrySet()) {
+						Repository repository = entry.getValue();
+						Ref ref = entry.getKey();
+						int result = deleteBranch(repository, ref,
+								forceDeletionOfUnmergedBranches);
 						if (result == DeleteBranchOperation.REJECTED_CURRENT) {
 							throw new CoreException(
 									Activator
@@ -110,7 +143,7 @@ public class DeleteBranchCommand extends
 											UIText.DeleteBranchCommand_CannotDeleteCheckedOutBranch,
 											null));
 						} else if (result == DeleteBranchOperation.REJECTED_UNMERGED) {
-							unmergedNodes.add(refNode);
+							unmergedNodes.put(ref, repository);
 						} else
 							monitor.worked(1);
 					}
@@ -125,10 +158,10 @@ public class DeleteBranchCommand extends
 		return unmergedNodes;
 	}
 
-	private int deleteBranch(final RefNode node, final Ref ref, boolean force)
-			throws CoreException {
-		DeleteBranchOperation dbop = new DeleteBranchOperation(node
-				.getRepository(), ref, force);
+	private int deleteBranch(final Repository repo, final Ref ref,
+			boolean force) throws CoreException {
+		DeleteBranchOperation dbop = new DeleteBranchOperation(repo, ref,
+				force);
 		dbop.execute(null);
 		return dbop.getStatus();
 	}