fix root command unregister (#389)

Incendo · Aug 26, 2022 · f1da897 · f1da897
1 parent 9a298d0
commit f1da897
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 10 deletions.
diff --git a/cloud-core/src/main/java/cloud/commandframework/CommandManager.java b/cloud-core/src/main/java/cloud/commandframework/CommandManager.java
@@ -545,7 +545,7 @@ public void deleteRootCommand(final @NonNull String rootCommand) throws CloudCap
         this.commandRegistrationHandler.unregisterRootCommand((StaticArgument<?>) node.getValue());
 
         // We then delete it from the tree.
-        this.commandTree.deleteRecursively(node);
+        this.commandTree.deleteRecursively(node, true);
 
         // And lastly we re-build the entire tree.
         this.commandTree.verifyAndRegister();

diff --git a/cloud-core/src/main/java/cloud/commandframework/CommandTree.java b/cloud-core/src/main/java/cloud/commandframework/CommandTree.java
@@ -952,23 +952,22 @@ private void checkAmbiguity(final @NonNull Node<@Nullable CommandArgument<C, ?>>
         return null;
     }
 
-    void deleteRecursively(final @NonNull Node<@Nullable CommandArgument<C, ?>> node) {
+    void deleteRecursively(final @NonNull Node<@Nullable CommandArgument<C, ?>> node, final boolean root) {
         for (final Node<@Nullable CommandArgument<C, ?>> child : new ArrayList<>(node.children)) {
-            this.deleteRecursively(child);
+            this.deleteRecursively(child, false);
         }
 
-        // We need to remove it from the tree.
-        this.removeNode(node);
+        this.removeNode(node, root);
     }
 
-    private boolean removeNode(final @NonNull Node<@Nullable CommandArgument<C, ?>> node) {
-        if (this.getRootNodes().contains(node)) {
-            this.internalTree.removeChild(node);
+    private boolean removeNode(final @NonNull Node<@Nullable CommandArgument<C, ?>> node, final boolean root) {
+        if (root) {
+            // root command node - remove it from the root tree
+            return this.internalTree.removeChild(node);
         } else {
+            // child node - remove it from the parent node
             return node.getParent().removeChild(node);
         }
-
-        return false;
     }
 
     /**

diff --git a/cloud-core/src/test/java/cloud/commandframework/CommandDeletionTest.java b/cloud-core/src/test/java/cloud/commandframework/CommandDeletionTest.java
@@ -146,4 +146,29 @@ void deleteIntermediateCommand() {
 
         assertThat(this.commandManager.commandTree().getRootNodes()).isEmpty();
     }
+
+    @Test
+    void deleteCommandWithSameArgumentNameAsRootCommand() {
+        // Arrange
+        this.commandManager.command(this.commandManager.commandBuilder("test").build());
+        this.commandManager.command(this.commandManager.commandBuilder("hello").literal("test").build());
+
+        // Pre-assert.
+        this.commandManager.executeCommand(new TestCommandSender(), "test").join();
+        this.commandManager.executeCommand(new TestCommandSender(), "hello test").join();
+
+        // Act
+        this.commandManager.deleteRootCommand("hello");
+
+        // Assert
+        this.commandManager.executeCommand(new TestCommandSender(), "test").join();
+        final CompletionException completionException = assertThrows(
+                CompletionException.class,
+                () -> this.commandManager.executeCommand(new TestCommandSender(), "hello").join()
+        );
+        assertThat(completionException).hasCauseThat().isInstanceOf(NoSuchCommandException.class);
+
+        assertThat(this.commandManager.suggest(new TestCommandSender(), "")).contains("test");
+        assertThat(this.commandManager.commandTree().getRootNodes()).hasSize(1);
+    }
 }