Fix tree node removal not working while a filter is applied

recaf-ui/src/main/java/software/coley/recaf/ui/control/tree/FilterableTreeItem.java

@@ -1,6 +1,7 @@
 package software.coley.recaf.ui.control.tree;
 
 import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
 import javafx.beans.binding.Bindings;
 import javafx.beans.property.ObjectProperty;
 import javafx.beans.property.SimpleObjectProperty;
@@ -73,13 +74,21 @@ public ObservableList<TreeItem<T>> getSourceChildren() {
 	}
 
 	/**
-	 * @return Source parent, ignoring filtering.
+	 * @return Source parent property, ignoring filtering.
 	 */
 	@Nonnull
 	public ObjectProperty<TreeItem<T>> sourceParentProperty() {
 		return sourceParent;
 	}
 
+	/**
+	 * @return Source parent, ignoring filtering.
+	 */
+	@Nullable
+	public TreeItem<T> getSourceParent() {
+		return sourceParent.get();
+	}
+
 	/**
 	 * @return {@code true} when the item MUST be shown.
 	 */

recaf-ui/src/main/java/software/coley/recaf/ui/control/tree/WorkspaceTreeNode.java

@@ -35,23 +35,21 @@ public WorkspaceTreeNode(PathNode<?> path) {
 	 * {@code false} if nothing was removed.
 	 */
 	public synchronized boolean removeNodeByPath(@Nonnull PathNode<?> path) {
-		// Call from root node only.
-		WorkspaceTreeNode root = this;
-		while (root.getParent() instanceof WorkspaceTreeNode parentNode)
-			root = parentNode;
-
-		// Get node by path.
-		WorkspaceTreeNode nodeByPath = root.getNodeByPath(path);
+		// Get node by path from the root.
+		WorkspaceTreeNode nodeByPath = getRoot().getNodeByPath(path);
 
 		// Get that node's parent, remove the child.
-		if (nodeByPath != null && nodeByPath.getParent() instanceof WorkspaceTreeNode parentNode) {
-			boolean removed = parentNode.removeSourceChild(nodeByPath);
-			while (parentNode.isLeaf() && parentNode.getParentNode() != null) {
-				WorkspaceTreeNode parentOfParent = parentNode.getParentNode();
-				parentOfParent.removeSourceChild(parentNode);
-				parentNode = parentOfParent;
+		if (nodeByPath != null) {
+			WorkspaceTreeNode parentNode = nodeByPath.getParentNode();
+			if (parentNode != null) {
+				boolean removed = parentNode.removeSourceChild(nodeByPath);
+				while (parentNode.isLeaf() && parentNode.getParentNode() != null) {
+					WorkspaceTreeNode parentOfParent = parentNode.getParentNode();
+					parentOfParent.removeSourceChild(parentNode);
+					parentNode = parentOfParent;
+				}
+				return removed;
 			}
-			return removed;
 		}
 
 		// No known node by path.
@@ -69,9 +67,7 @@ public synchronized boolean removeNodeByPath(@Nonnull PathNode<?> path) {
 	@Nonnull
 	public synchronized WorkspaceTreeNode getOrCreateNodeByPath(@Nonnull PathNode<?> path) {
 		// Call from root node only.
-		WorkspaceTreeNode root = this;
-		while (root.getParent() instanceof WorkspaceTreeNode parentNode)
-			root = parentNode;
+		WorkspaceTreeNode root = getRoot();
 
 		// Lookup and/or create nodes for path.
 		return getOrInsertIntoTree(root, path);
@@ -86,13 +82,14 @@ public synchronized WorkspaceTreeNode getOrCreateNodeByPath(@Nonnull PathNode<?>
 	 * @return Node containing the path in the tree.
 	 */
 	@Nullable
-	@SuppressWarnings("deprecation")
 	public synchronized WorkspaceTreeNode getNodeByPath(@Nonnull PathNode<?> path) {
+		// Base case, we are that path.
 		PathNode<?> value = getValue();
 		if (path.equals(value))
 			return this;
 
-		for (TreeItem<PathNode<?>> child : getChildren())
+		// Check all children for a match, regardless of the current filter.
+		for (TreeItem<PathNode<?>> child : getSourceChildren())
 			if (path.isDescendantOf(child.getValue()) && child instanceof WorkspaceTreeNode childNode)
 				return childNode.getNodeByPath(path);
 
@@ -103,13 +100,29 @@ public synchronized WorkspaceTreeNode getNodeByPath(@Nonnull PathNode<?> path) {
 	 * @return First child tree node. {@code null} if no child is found.
 	 */
 	@Nullable
-	@SuppressWarnings("deprecation")
 	public synchronized WorkspaceTreeNode getFirstChild() {
-		return getChildren().isEmpty()
-				? null : getChildren().getFirst() instanceof WorkspaceTreeNode node
+		// Get first child, regardless of the current filter.
+		var children = getSourceChildren();
+		return children.isEmpty()
+				? null : children.getFirst() instanceof WorkspaceTreeNode node
 				? node : null;
 	}
 
+	/**
+	 * @return The root of this tree node's parent hierarchy.
+	 */
+	@Nonnull
+	public WorkspaceTreeNode getRoot() {
+		WorkspaceTreeNode root = this;
+		while (true) {
+			WorkspaceTreeNode parentNode = root.getParentNode();
+			if (parentNode == null)
+				break;
+			root = parentNode;
+		}
+		return root;
+	}
+
 	/**
 	 * @param path
 	 * 		Path to check against.
@@ -121,11 +134,11 @@ public boolean matches(@Nonnull PathNode<?> path) {
 	}
 
 	/**
-	 * @return {@link #getParent()} but cast to {@link WorkspaceTreeNode}.
+	 * @return {@link #getSourceParent()} but cast to {@link WorkspaceTreeNode}.
 	 */
 	@Nullable
 	public WorkspaceTreeNode getParentNode() {
-		return (WorkspaceTreeNode) getParent();
+		return (WorkspaceTreeNode) getSourceParent();
 	}
 
 	@Override

recaf-ui/src/test/java/software/coley/recaf/ui/control/tree/WorkspaceTreeNodeTest.java

@@ -3,12 +3,19 @@
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
+import software.coley.collections.Unchecked;
 import software.coley.recaf.info.BasicFileInfo;
 import software.coley.recaf.info.FileInfo;
 import software.coley.recaf.info.JvmClassInfo;
 import software.coley.recaf.info.StubFileInfo;
 import software.coley.recaf.info.properties.BasicPropertyContainer;
-import software.coley.recaf.path.*;
+import software.coley.recaf.path.BundlePathNode;
+import software.coley.recaf.path.ClassPathNode;
+import software.coley.recaf.path.DirectoryPathNode;
+import software.coley.recaf.path.FilePathNode;
+import software.coley.recaf.path.PathNodes;
+import software.coley.recaf.path.ResourcePathNode;
+import software.coley.recaf.path.WorkspacePathNode;
 import software.coley.recaf.test.dummy.AccessibleFields;
 import software.coley.recaf.test.dummy.HelloWorld;
 import software.coley.recaf.test.dummy.StringConsumer;
@@ -358,6 +365,45 @@ void matches() {
 		assertTrue(child.matches(p1a));
 	}
 
+	@Nested
+	class Filtered {
+		@Test
+		void insertWhileFilteredStillUpdatesChildren() {
+			// Create workspace root, but apply a filter so that nothing is shown
+			WorkspaceTreeNode workspaceNode = new WorkspaceTreeNode(p5);
+			workspaceNode.predicateProperty().set(p -> false);
+
+			// Insert the class, which should generate all paths between the class and the workspace node.
+			WorkspaceTreeNode classNode = getOrInsertIntoTree(workspaceNode, p1a);
+			assertNotNull(classNode, "Class not yielded by original assert");
+
+			// Validate the filtered view is still empty, but the unfiltered model has children
+			WorkspaceTreeNode node = workspaceNode;
+			for (int d = 0; d < 5; d++) {
+				assertTrue(node.getChildren().isEmpty(), "Filtered children list is not empty: depth=" + d);
+				assertFalse(node.getSourceChildren().isEmpty(), "Unfiltered children list was empty: depth=" + d);
+				node = Unchecked.cast(workspaceNode.getSourceChildren().getFirst());
+			}
+		}
+
+		@Test
+		void removeWhileFilteredStillUpdatesChildren() {
+			// 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 classNode = getOrInsertIntoTree(workspaceNode, p1a);
+			assertNotNull(classNode, "Class not yielded by original assert");
+			assertNotNull(workspaceNode.getNodeByPath(p1a), "Could not get class after setup");
+
+			// Apply a filter so that nothing is shown.
+			// We should still be able to access items via path-lookup though.
+			workspaceNode.predicateProperty().set(p -> false);
+			assertNotNull(workspaceNode.getNodeByPath(p1a), "Could not get class after filtering");
+
+			// Validate the filtered view is still empty, but the unfiltered model has children
+			assertTrue(workspaceNode.removeNodeByPath(p1a), "Class could not be removed");
+			assertNull(workspaceNode.getNodeByPath(p1a), "Class accessible after removed");
+		}
+	}
 
 	@Nested
 	class Insertion {