diff --git a/api/src/main/java/com/velocitypowered/api/command/CommandManager.java b/api/src/main/java/com/velocitypowered/api/command/CommandManager.java index 928d55d6f..021b6ca1e 100644 --- a/api/src/main/java/com/velocitypowered/api/command/CommandManager.java +++ b/api/src/main/java/com/velocitypowered/api/command/CommandManager.java @@ -75,6 +75,13 @@ public interface CommandManager { */ void unregister(String alias); + /** + * Unregisters the specified command from the manager, if registered. + * + * @param meta the command to unregister + */ + void unregister(CommandMeta meta); + /** * Retrieves the {@link CommandMeta} from the specified command alias, if registered. * @param alias the command alias to lookup diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java index 7f9280c8e..2feb88dce 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java @@ -156,11 +156,23 @@ public class VelocityCommandManager implements CommandManager { // The literals of secondary aliases will preserve the children of // the removed literal in the graph. dispatcher.getRoot().removeChildByName(alias.toLowerCase(Locale.ENGLISH)); + commandMetas.remove(alias); + } finally { + lock.writeLock().unlock(); + } + } - CommandMeta meta = commandMetas.get(alias); - if (meta != null) { - for (String metaAlias : meta.getAliases()) { - commandMetas.remove(metaAlias, meta); + @Override + public void unregister(CommandMeta meta) { + Preconditions.checkNotNull(meta, "meta"); + lock.writeLock().lock(); + try { + // The literals of secondary aliases will preserve the children of + // the removed literal in the graph. + for (String alias : meta.getAliases()) { + final String lowercased = alias.toLowerCase(Locale.ENGLISH); + if (commandMetas.remove(lowercased, meta)) { + dispatcher.getRoot().removeChildByName(lowercased); } } } finally { diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java index bc5c826b4..37cccc4f6 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java @@ -19,6 +19,7 @@ package com.velocitypowered.proxy.command; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -44,6 +45,7 @@ public class CommandManagerTests extends CommandTestSuite { assertTrue(manager.hasCommand("hello")); assertRegisteredAliases("hello"); + assertEquals(meta, manager.getCommandMeta("hello")); } @Test @@ -59,6 +61,10 @@ public class CommandManagerTests extends CommandTestSuite { assertTrue(manager.hasCommand("baz")); assertTrue(manager.hasCommand("qux")); assertRegisteredAliases("foo", "bar", "baz", "qux"); + assertEquals(meta, manager.getCommandMeta("foo")); + assertEquals(meta, manager.getCommandMeta("bar")); + assertEquals(meta, manager.getCommandMeta("baz")); + assertEquals(meta, manager.getCommandMeta("qux")); } @Test @@ -90,8 +96,11 @@ public class CommandManagerTests extends CommandTestSuite { final var oldMeta = manager.metaBuilder("foo").build(); manager.register(oldMeta, DummyCommand.INSTANCE); // fails on execution + assertEquals(oldMeta, manager.getCommandMeta("foo")); + final var newMeta = manager.metaBuilder("foo").build(); manager.register(newMeta, (RawCommand) invocation -> called.set(true)); + assertEquals(newMeta, manager.getCommandMeta("foo")); manager.executeAsync(MockCommandSource.INSTANCE, "foo").join(); assertTrue(called.get()); @@ -153,9 +162,39 @@ public class CommandManagerTests extends CommandTestSuite { assertFalse(manager.hasCommand("bar")); assertTrue(manager.hasCommand("foo")); + assertEquals(meta, manager.getCommandMeta("foo")); assertRegisteredAliases("foo"); } + @Test + void testUnregisterAllAliases() { + final var meta = manager.metaBuilder("foo") + .aliases("bar") + .build(); + manager.register(meta, DummyCommand.INSTANCE); + manager.unregister(meta); + + assertFalse(manager.hasCommand("bar")); + assertFalse(manager.hasCommand("foo")); + } + + @Test + void testUnregisterAliasOverlap() { + final var meta1 = manager.metaBuilder("foo") + .aliases("bar") + .build(); + manager.register(meta1, DummyCommand.INSTANCE); + final var meta2 = manager.metaBuilder("bar") + .build(); + manager.register(meta2, DummyCommand.INSTANCE); + assertEquals(meta1, manager.getCommandMeta("foo")); + assertEquals(meta2, manager.getCommandMeta("bar")); + + manager.unregister(meta1); + assertNull(manager.getCommandMeta("foo")); + assertEquals(meta2, manager.getCommandMeta("bar")); + } + // Execution @Test