From 16b3b5abcb667574ff5f993a7258b33720934936 Mon Sep 17 00:00:00 2001 From: Matt Date: Sat, 2 Nov 2024 00:50:34 -0400 Subject: [PATCH] Fix tree node removal while filter is active deleting the entire tree --- .../ui/control/tree/FilterableTreeItem.java | 15 ++++++++---- .../ui/control/tree/WorkspaceTreeNode.java | 23 +++++++++++++++++- .../control/tree/WorkspaceTreeNodeTest.java | 24 +++++++++++++++++++ 3 files changed, 57 insertions(+), 5 deletions(-) diff --git a/recaf-ui/src/main/java/software/coley/recaf/ui/control/tree/FilterableTreeItem.java b/recaf-ui/src/main/java/software/coley/recaf/ui/control/tree/FilterableTreeItem.java index 44ee076d9..d96fc54ce 100644 --- a/recaf-ui/src/main/java/software/coley/recaf/ui/control/tree/FilterableTreeItem.java +++ b/recaf-ui/src/main/java/software/coley/recaf/ui/control/tree/FilterableTreeItem.java @@ -59,6 +59,14 @@ protected FilterableTreeItem() { setUnderlyingChildren(filteredChildren); } + /** + * @return Predicate property. + */ + @Nonnull + public ObjectProperty>> predicateProperty() { + return predicate; + } + @Override @Deprecated(since = "Use FilterableTreeItem dedicated methods for interacting with children") public ObservableList> getChildren() { @@ -210,11 +218,10 @@ public > void sortChildren(@Nonnull Comparator comparat } /** - * @return Predicate property. + * @return {@code true} when this tree item is a true leaf based on the unfiltered children. */ - @Nonnull - public ObjectProperty>> predicateProperty() { - return predicate; + protected boolean isUnfilteredLeaf() { + return sourceChildren.isEmpty(); } static { diff --git a/recaf-ui/src/main/java/software/coley/recaf/ui/control/tree/WorkspaceTreeNode.java b/recaf-ui/src/main/java/software/coley/recaf/ui/control/tree/WorkspaceTreeNode.java index 8102f7d35..e09c3f5ea 100644 --- a/recaf-ui/src/main/java/software/coley/recaf/ui/control/tree/WorkspaceTreeNode.java +++ b/recaf-ui/src/main/java/software/coley/recaf/ui/control/tree/WorkspaceTreeNode.java @@ -8,6 +8,7 @@ import software.coley.recaf.path.BundlePathNode; import software.coley.recaf.path.DirectoryPathNode; import software.coley.recaf.path.PathNode; +import software.coley.recaf.util.StringUtil; /** * Tree item subtype for more convenience tree building operations. @@ -43,7 +44,7 @@ public synchronized boolean removeNodeByPath(@Nonnull PathNode path) { WorkspaceTreeNode parentNode = nodeByPath.getParentNode(); if (parentNode != null) { boolean removed = parentNode.removeSourceChild(nodeByPath); - while (parentNode.isLeaf() && parentNode.getParentNode() != null) { + while (parentNode.isUnfilteredLeaf() && parentNode.getParentNode() != null) { WorkspaceTreeNode parentOfParent = parentNode.getParentNode(); parentOfParent.removeSourceChild(parentNode); parentNode = parentOfParent; @@ -151,6 +152,26 @@ public String toString() { return getClass().getSimpleName() + "[" + getValue().toString() + "]"; } + /** + * A debugging util to inspect the state of the tree without having to dig through + * the actual references in the debugger. + * + * @return String representation of this tree and all of its descendants. + */ + @Nonnull + public String printTree() { + StringBuilder sb = new StringBuilder(toString()); + for (TreeItem> child : getSourceChildren()) { + if (child instanceof WorkspaceTreeNode childNode) { + String childTree = childNode.printTree(); + for (String childTreeEntry : StringUtil.fastSplit(childTree, false, '\n')) { + sb.append("\n ").append(childTreeEntry); + } + } + } + return sb.toString(); + } + /** * Get/insert a {@link WorkspaceTreeNode} holding the given {@link PathNode} from/to the tree model. * diff --git a/recaf-ui/src/test/java/software/coley/recaf/ui/control/tree/WorkspaceTreeNodeTest.java b/recaf-ui/src/test/java/software/coley/recaf/ui/control/tree/WorkspaceTreeNodeTest.java index 6a63e4d72..3483d412a 100644 --- a/recaf-ui/src/test/java/software/coley/recaf/ui/control/tree/WorkspaceTreeNodeTest.java +++ b/recaf-ui/src/test/java/software/coley/recaf/ui/control/tree/WorkspaceTreeNodeTest.java @@ -403,6 +403,30 @@ void removeWhileFilteredStillUpdatesChildren() { assertTrue(workspaceNode.removeNodeByPath(p1a), "Class could not be removed"); assertNull(workspaceNode.getNodeByPath(p1a), "Class accessible after removed"); } + + @Test + void removeWhileFilteredDoesNotEliminateOtherClasses() { + // Create workspace root and insert the class, which should generate all paths between the class and the workspace node. + WorkspaceTreeNode workspaceNode = new WorkspaceTreeNode(p5); + WorkspaceTreeNode classNodeA = getOrInsertIntoTree(workspaceNode, p1a); + WorkspaceTreeNode classNodeB = getOrInsertIntoTree(workspaceNode, p1b); + WorkspaceTreeNode classNodeC = getOrInsertIntoTree(workspaceNode, p1c); + assertNotNull(classNodeA, "Class not yielded by original assert"); + assertNotNull(classNodeB, "Class not yielded by original assert"); + assertNotNull(classNodeC, "Class not yielded by original assert"); + assertNotNull(workspaceNode.getNodeByPath(p1a), "Could not get class after setup"); + assertNotNull(workspaceNode.getNodeByPath(p1b), "Could not get class after setup"); + assertNotNull(workspaceNode.getNodeByPath(p1c), "Could not get class after setup"); + + // Apply a filter so that nothing is shown. + workspaceNode.predicateProperty().set(p -> false); + + // Remove 'Class A' and verify 'Class B' and 'Class C' are still accessible + assertTrue(workspaceNode.removeNodeByPath(p1a), "Could not remove class after filtering"); + assertNull(workspaceNode.getNodeByPath(p1a), "Class A accessible after removed"); + assertNotNull(workspaceNode.getNodeByPath(p1b), "Class B not accessible after adjacent leaf removed"); + assertNotNull(workspaceNode.getNodeByPath(p1c), "Class C not accessible after adjacent leaf removed"); + } } @Nested