diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/CommandGraphInjector.java b/proxy/src/main/java/com/velocitypowered/proxy/command/CommandGraphInjector.java index 7e2e55a9e..015501a12 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/CommandGraphInjector.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/CommandGraphInjector.java @@ -89,6 +89,8 @@ public final class CommandGraphInjector { VelocityCommands.getArgumentsNode(asLiteral); if (argsNode == null) { // This literal is associated to a BrigadierCommand, filter normally + // TODO Document alias redirects are not supported, see + // CommandGraphInjectorTests#testBrigadierCommandAliasRedirectsNotAllowed. this.copyChildren(node, copy, source); } else { // Copy all children nodes (arguments node and hints) 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 0d15f02b6..5d4665a45 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java @@ -97,6 +97,7 @@ public class VelocityCommandManager implements CommandManager { public void register(final CommandMeta meta, final Command command) { Preconditions.checkNotNull(meta, "meta"); Preconditions.checkNotNull(command, "command"); + // TODO This is quite ugly; find registrar and then attempt registering. for (final CommandRegistrar registrar : this.registrars) { if (this.tryRegister(registrar, command, meta)) { @@ -114,7 +115,7 @@ public class VelocityCommandManager implements CommandManager { return false; } try { - registrar.register(superInterface.cast(command), meta); + registrar.register(meta, superInterface.cast(command)); return true; } catch (final IllegalArgumentException ignored) { return false; diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/BrigadierCommandRegistrar.java b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/BrigadierCommandRegistrar.java index 0739f2b76..432065f3f 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/BrigadierCommandRegistrar.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/BrigadierCommandRegistrar.java @@ -35,7 +35,7 @@ public final class BrigadierCommandRegistrar extends AbstractCommandRegistrar
{ /** * Registers the given command. * - * @param command the command to register * @param meta the command metadata, including the case-insensitive aliases + * @param command the command to register * @throws IllegalArgumentException if the given command cannot be registered */ - void register(final T command, final CommandMeta meta); + void register(final CommandMeta meta, final T command); /** * Returns the superclass or superinterface of all {@link Command} classes - * compatible with this registrar. Note that {@link #register(Command, CommandMeta)} + * compatible with this registrar. Note that {@link #register(CommandMeta, Command)} * may impose additional restrictions on individual {@link Command} instances. * * @return the superclass of all the classes compatible with this registrar diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/InvocableCommandRegistrar.java b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/InvocableCommandRegistrar.java index 423cf3cc3..e6ff7129c 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/InvocableCommandRegistrar.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/InvocableCommandRegistrar.java @@ -56,7 +56,7 @@ abstract class InvocableCommandRegistrar, } @Override - public void register(final T command, final CommandMeta meta) { + public void register(final CommandMeta meta, final T command) { final Iterator aliases = meta.getAliases().iterator(); final String primaryAlias = aliases.next(); diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java new file mode 100644 index 000000000..89c263e1f --- /dev/null +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java @@ -0,0 +1,124 @@ +package com.velocitypowered.proxy.command; + +import static com.mojang.brigadier.arguments.IntegerArgumentType.getInteger; +import static com.mojang.brigadier.arguments.IntegerArgumentType.integer; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; + +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import com.mojang.brigadier.builder.RequiredArgumentBuilder; +import com.velocitypowered.api.command.BrigadierCommand; +import com.velocitypowered.api.command.CommandSource; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; + +public class BrigadierCommandTests extends CommandTestSuite { + + // Execution + + @Test + void testExecutesAlias() { + final var callCount = new AtomicInteger(); + + final var node = LiteralArgumentBuilder + .literal("hello") + .executes(context -> { + assertEquals(source, context.getSource()); + assertEquals("hello", context.getInput()); + assertEquals(1, context.getNodes().size()); + callCount.incrementAndGet(); + return 1; + }) + .build(); + manager.register(new BrigadierCommand(node)); + + assertHandled("hello"); + assertEquals(1, callCount.get()); + } + + @Test + void testForwardsAndDoesNotExecuteImpermissibleAlias() { + final var callCount = new AtomicInteger(); + + final var node = LiteralArgumentBuilder + .literal("hello") + .executes(context -> fail()) + .requires(actualSource -> { + assertEquals(source, actualSource); + callCount.incrementAndGet(); + return false; + }) + .build(); + manager.register(new BrigadierCommand(node)); + + assertForwarded("hello"); + assertEquals(1, callCount.get()); + } + + @Test + void testForwardsAndDoesNotExecuteContextImpermissibleAlias() { + final var callCount = new AtomicInteger(); + + final var node = LiteralArgumentBuilder + .literal("hello") + .executes(context -> fail()) + .requiresWithContext((context, reader) -> { + assertEquals(source, context.getSource()); + assertEquals("hello", reader.getRead()); + assertEquals(1, context.getNodes().size()); + callCount.incrementAndGet(); + return false; + }) + .build(); + manager.register(new BrigadierCommand(node)); + + assertForwarded("hello"); + assertEquals(1, callCount.get()); + } + + @Test + void testExecutesNonAliasLevelNode() { + final var callCount = new AtomicInteger(); + + final var node = LiteralArgumentBuilder + .literal("buy") + .executes(context -> fail()) + .then(RequiredArgumentBuilder + .argument("quantity", integer()) + .executes(context -> { + assertEquals("buy 12", context.getInput()); + assertEquals(12, getInteger(context, "quantity")); + assertEquals(2, context.getNodes().size()); + callCount.incrementAndGet(); + return 1; + }) + ) + .build(); + manager.register(new BrigadierCommand(node)); + + assertHandled("buy 12"); + assertEquals(1, callCount.get()); + } + + @Test + void testHandlesAndDoesNotExecuteWithImpermissibleNonAliasLevelNode() { + final var callCount = new AtomicInteger(); + + final var node = LiteralArgumentBuilder + .literal("hello") + .executes(context -> fail()) + .then(LiteralArgumentBuilder + .literal("world") + .executes(context -> fail()) + .requires(source -> { + callCount.incrementAndGet(); + return false; + }) + ) + .build(); + manager.register(new BrigadierCommand(node)); + + assertHandled("hello world"); + assertEquals(1, callCount.get()); + } +} diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java index d5c27c5db..b1471009e 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java @@ -17,54 +17,172 @@ package com.velocitypowered.proxy.command; -import com.mojang.brigadier.CommandDispatcher; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReentrantLock; +import static com.mojang.brigadier.arguments.IntegerArgumentType.integer; +import static com.mojang.brigadier.builder.LiteralArgumentBuilder.literal; +import static com.mojang.brigadier.builder.RequiredArgumentBuilder.argument; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import com.mojang.brigadier.tree.LiteralCommandNode; +import com.mojang.brigadier.tree.RootCommandNode; +import com.velocitypowered.api.command.BrigadierCommand; +import com.velocitypowered.api.command.CommandSource; +import com.velocitypowered.api.command.SimpleCommand; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; public class CommandGraphInjectorTests { - private CommandDispatcher dispatcher; - private Lock lock; - private CommandGraphInjector injector; + private static final CommandSource SOURCE = MockCommandSource.INSTANCE; + + private VelocityCommandManager manager; + private RootCommandNode dest; @BeforeEach void setUp() { - this.dispatcher = new CommandDispatcher<>(); - this.lock = new ReentrantLock(); - this.injector = new CommandGraphInjector<>(this.dispatcher, this.lock); + this.manager = new VelocityCommandManager(OldCommandManagerTests.EVENT_MANAGER); + this.dest = new RootCommandNode<>(); } - // TODO - @Test void testInjectInvocableCommand() { - // Preserves arguments node and hints + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, (SimpleCommand) invocation -> fail()); + manager.getInjector().inject(dest, SOURCE); + + // Preserves alias and arguments node + final var expected = manager.getDispatcher().getRoot(); + assertEquals(expected, dest); } @Test void testFiltersImpermissibleAlias() { + final var callCount = new AtomicInteger(); + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public boolean hasPermission(final Invocation invocation) { + assertEquals(SOURCE, invocation.source()); + assertEquals("hello", invocation.alias()); + assertArrayEquals(new String[0], invocation.arguments()); + callCount.incrementAndGet(); + return false; + } + }); + manager.getInjector().inject(dest, SOURCE); + + assertTrue(dest.getChildren().isEmpty()); + assertEquals(1, callCount.get()); } @Test void testInjectsBrigadierCommand() { + final LiteralCommandNode node = LiteralArgumentBuilder + .literal("hello") + .then(literal("world")) + .then(argument("count", integer())) + .build(); + manager.register(new BrigadierCommand(node)); + manager.getInjector().inject(dest, SOURCE); + assertEquals(node, dest.getChild("hello")); } @Test void testFiltersImpermissibleBrigadierCommandChildren() { + final var callCount = new AtomicInteger(); + final var registered = LiteralArgumentBuilder + .literal("greet") + .then(LiteralArgumentBuilder + .literal("somebody") + .requires(source -> { + callCount.incrementAndGet(); + return false; + })) + .build(); + manager.register(new BrigadierCommand(registered)); + manager.getInjector().inject(dest, SOURCE); + + final var expected = LiteralArgumentBuilder + .literal("greet") + .build(); + assertEquals(expected, dest.getChild("greet")); + assertEquals(1, callCount.get()); } @Test - void testInjectFiltersBrigadierCommandRedirects() { + void testBrigadierCommandAliasRedirectsNotAllowed() { + final var registered = LiteralArgumentBuilder + .literal("origin") + .redirect(LiteralArgumentBuilder + .literal("target") + .build()) + .build(); + manager.register(new BrigadierCommand(registered)); + manager.getInjector().inject(dest, SOURCE); + final var expected = LiteralArgumentBuilder + .literal("origin") + .build(); + assertEquals(expected, dest.getChild("origin")); + } + + @Test + void testFiltersImpermissibleBrigadierCommandRedirects() { + final var callCount = new AtomicInteger(); + + final var registered = LiteralArgumentBuilder + .literal("hello") + .then(LiteralArgumentBuilder + .literal("origin") + .redirect(LiteralArgumentBuilder + .literal("target") + .requires(source -> { + callCount.incrementAndGet(); + return false; + }) + .build() + ) + ) + .build(); + manager.register(new BrigadierCommand(registered)); + manager.getInjector().inject(dest, SOURCE); + + final var expected = LiteralArgumentBuilder + .literal("hello") + .then(literal("origin")) + .build(); + assertEquals(expected, dest.getChild("hello")); + assertEquals(1, callCount.get()); } @Test void testInjectOverridesAliasInDestination() { + final var registered = LiteralArgumentBuilder + .literal("foo") + .then(literal("bar")) + .build(); + manager.register(new BrigadierCommand(registered)); + final var original = LiteralArgumentBuilder + .literal("foo") + .then(literal("baz")) + .build(); + dest.addChild(original); + manager.getInjector().inject(dest, SOURCE); + + assertEquals(registered, dest.getChild("foo")); } } diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandTestSuite.java b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandTestSuite.java new file mode 100644 index 000000000..ccbdb798b --- /dev/null +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandTestSuite.java @@ -0,0 +1,33 @@ +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.assertTrue; + +import com.velocitypowered.api.command.CommandSource; +import java.util.Arrays; +import org.junit.jupiter.api.BeforeEach; + +abstract class CommandTestSuite { + + protected VelocityCommandManager manager; + protected final CommandSource source = MockCommandSource.INSTANCE; + + @BeforeEach + void setUp() { + this.manager = new VelocityCommandManager(OldCommandManagerTests.EVENT_MANAGER); + } + + final void assertHandled(final String input) { + assertTrue(manager.executeAsync(source, input).join()); + } + + final void assertForwarded(final String input) { + assertFalse(manager.executeAsync(source, input).join()); + } + + final void assertSuggestions(final String input, final String... expectedSuggestions) { + final var actual = manager.offerSuggestions(source, input).join(); + assertEquals(Arrays.asList(expectedSuggestions), actual); + } +} diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/OldCommandManagerTests.java similarity index 99% rename from proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java rename to proxy/src/test/java/com/velocitypowered/proxy/command/OldCommandManagerTests.java index 467882469..2835c6f7f 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/OldCommandManagerTests.java @@ -46,9 +46,10 @@ import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; -public class CommandManagerTests { +@Disabled +public class OldCommandManagerTests { - private static final VelocityEventManager EVENT_MANAGER = new MockEventManager(); + public static final VelocityEventManager EVENT_MANAGER = new MockEventManager(); static { Runtime.getRuntime().addShutdownHook(new Thread(() -> { diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java new file mode 100644 index 000000000..5e37be8b9 --- /dev/null +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java @@ -0,0 +1,93 @@ +package com.velocitypowered.proxy.command; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; + +import com.velocitypowered.api.command.RawCommand; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; + +public class RawCommandTests extends CommandTestSuite { + + // Execution + + @Test + void testExecuteAlias() { + final var callCount = new AtomicInteger(); + + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, (RawCommand) invocation -> { + assertEquals(source, invocation.source()); + assertEquals("hello", invocation.alias()); + assertEquals("", invocation.arguments()); + callCount.incrementAndGet(); + }); + + assertHandled("hello"); + assertEquals(1, callCount.get()); + } + + @Test + void testForwardsAndDoesNotExecuteImpermissibleAlias() { + final var callCount = new AtomicInteger(); + + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public boolean hasPermission(final Invocation invocation) { + assertEquals(source, invocation.source()); + assertEquals("hello", invocation.alias()); + assertEquals("", invocation.arguments()); + callCount.incrementAndGet(); + return false; + } + }); + + assertForwarded("hello"); + assertEquals(1, callCount.get()); + } + + @Test + void testExecutesWithArguments() { + final var callCount = new AtomicInteger(); + + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, (RawCommand) invocation -> { + assertEquals("hello", invocation.alias()); + assertEquals("dear world", invocation.arguments()); + callCount.incrementAndGet(); + }); + + assertHandled("hello dear world"); + assertEquals(1, callCount.get()); + } + + @Test + void testHandlesAndDoesNotExecuteWithImpermissibleArgs() { + final var callCount = new AtomicInteger(); + + final var meta = manager.metaBuilder("color").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public boolean hasPermission(final Invocation invocation) { + assertEquals("color", invocation.alias()); + assertEquals("red", invocation.arguments()); + callCount.incrementAndGet(); + return false; + } + }); + + assertHandled("color red"); + assertEquals(1, callCount.get()); + } +} diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java new file mode 100644 index 000000000..f52c8cf3d --- /dev/null +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java @@ -0,0 +1,94 @@ +package com.velocitypowered.proxy.command; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; + +import com.velocitypowered.api.command.SimpleCommand; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; + +public class SimpleCommandTests extends CommandTestSuite { + + // Execution + + @Test + void testExecutesAlias() { + final var callCount = new AtomicInteger(); + + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, (SimpleCommand) invocation -> { + assertEquals(source, invocation.source()); + assertEquals("hello", invocation.alias()); + assertArrayEquals(new String[0], invocation.arguments()); + callCount.incrementAndGet(); + }); + + assertHandled("hello"); + assertEquals(1, callCount.get()); + } + + @Test + void testForwardsAndDoesNotExecuteImpermissibleAlias() { + final var callCount = new AtomicInteger(); + + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public boolean hasPermission(final Invocation invocation) { + assertEquals(source, invocation.source()); + assertEquals("hello", invocation.alias()); + assertArrayEquals(new String[0], invocation.arguments()); + callCount.incrementAndGet(); + return false; + } + }); + + assertForwarded("hello"); + assertEquals(1, callCount.get()); + } + + @Test + void testExecutesWithArguments() { + final var callCount = new AtomicInteger(); + + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, (SimpleCommand) invocation -> { + assertEquals("hello", invocation.alias()); + assertArrayEquals(new String[] { "dear", "world" }, invocation.arguments()); + callCount.incrementAndGet(); + }); + + assertHandled("hello dear world"); + assertEquals(1, callCount.get()); + } + + @Test + void testHandlesAndDoesNotExecuteWithImpermissibleArgs() { + final var callCount = new AtomicInteger(); + + final var meta = manager.metaBuilder("color").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public boolean hasPermission(final Invocation invocation) { + assertEquals("color", invocation.alias()); + assertArrayEquals(new String[] { "red" }, invocation.arguments()); + callCount.incrementAndGet(); + return false; + } + }); + + assertHandled("color red"); + assertEquals(1, callCount.get()); + } +}