Test execution and injection

proxy/src/main/java/com/velocitypowered/proxy/command/CommandGraphInjector.java

@@ -89,6 +89,8 @@ public final class CommandGraphInjector<S> {
                 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)

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;

proxy/src/main/java/com/velocitypowered/proxy/command/registrar/BrigadierCommandRegistrar.java

@@ -35,7 +35,7 @@ public final class BrigadierCommandRegistrar extends AbstractCommandRegistrar<Br
   }
 
   @Override
-  public void register(final BrigadierCommand command, final CommandMeta meta) {
+  public void register(final CommandMeta meta, final BrigadierCommand command) {
     // The literal name might not match any aliases on the given meta.
     // Register it (if valid), since it's probably what the user expects.
     // If invalid, the metadata contains the same alias, but in lowercase.

proxy/src/main/java/com/velocitypowered/proxy/command/registrar/CommandRegistrar.java

@@ -33,15 +33,15 @@ public interface CommandRegistrar<T extends Command> {
   /**
    * 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

proxy/src/main/java/com/velocitypowered/proxy/command/registrar/InvocableCommandRegistrar.java

@@ -56,7 +56,7 @@ abstract class InvocableCommandRegistrar<T extends InvocableCommand<I>,
   }
 
   @Override
-  public void register(final T command, final CommandMeta meta) {
+  public void register(final CommandMeta meta, final T command) {
     final Iterator<String> aliases = meta.getAliases().iterator();
 
     final String primaryAlias = aliases.next();

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
+            .<CommandSource>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
+            .<CommandSource>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
+            .<CommandSource>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
+            .<CommandSource>literal("buy")
+            .executes(context -> fail())
+            .then(RequiredArgumentBuilder
+                    .<CommandSource, Integer>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
+            .<CommandSource>literal("hello")
+            .executes(context -> fail())
+            .then(LiteralArgumentBuilder
+                    .<CommandSource>literal("world")
+                    .executes(context -> fail())
+                    .requires(source -> {
+                      callCount.incrementAndGet();
+                      return false;
+                    })
+            )
+            .build();
+    manager.register(new BrigadierCommand(node));
+
+    assertHandled("hello world");
+    assertEquals(1, callCount.get());
+  }
+}

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 static com.mojang.brigadier.arguments.IntegerArgumentType.integer;
-import java.util.concurrent.locks.Lock;
+import static com.mojang.brigadier.builder.LiteralArgumentBuilder.literal;
-import java.util.concurrent.locks.ReentrantLock;
+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<Object> dispatcher;
+  private static final CommandSource SOURCE = MockCommandSource.INSTANCE;
-  private Lock lock;
+
-  private CommandGraphInjector<Object> injector;
+  private VelocityCommandManager manager;
+  private RootCommandNode<CommandSource> dest;
 
   @BeforeEach
   void setUp() {
-    this.dispatcher = new CommandDispatcher<>();
+    this.manager = new VelocityCommandManager(OldCommandManagerTests.EVENT_MANAGER);
-    this.lock = new ReentrantLock();
+    this.dest = new RootCommandNode<>();
-    this.injector = new CommandGraphInjector<>(this.dispatcher, this.lock);
   }
 
-  // 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<CommandSource> node = LiteralArgumentBuilder
+            .<CommandSource>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
+            .<CommandSource>literal("greet")
+            .then(LiteralArgumentBuilder
+                    .<CommandSource>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
+            .<CommandSource>literal("origin")
+            .redirect(LiteralArgumentBuilder
+                    .<CommandSource>literal("target")
+                    .build())
+            .build();
+    manager.register(new BrigadierCommand(registered));
+    manager.getInjector().inject(dest, SOURCE);
 
+    final var expected = LiteralArgumentBuilder
+            .<CommandSource>literal("origin")
+            .build();
+    assertEquals(expected, dest.getChild("origin"));
+  }
+
+  @Test
+  void testFiltersImpermissibleBrigadierCommandRedirects() {
+    final var callCount = new AtomicInteger();
+
+    final var registered = LiteralArgumentBuilder
+            .<CommandSource>literal("hello")
+            .then(LiteralArgumentBuilder
+                .<CommandSource>literal("origin")
+                .redirect(LiteralArgumentBuilder
+                    .<CommandSource>literal("target")
+                    .requires(source -> {
+                      callCount.incrementAndGet();
+                      return false;
+                    })
+                    .build()
+                )
+            )
+            .build();
+    manager.register(new BrigadierCommand(registered));
+    manager.getInjector().inject(dest, SOURCE);
+
+    final var expected = LiteralArgumentBuilder
+            .<CommandSource>literal("hello")
+            .then(literal("origin"))
+            .build();
+    assertEquals(expected, dest.getChild("hello"));
+    assertEquals(1, callCount.get());
   }
 
   @Test
   void testInjectOverridesAliasInDestination() {
+    final var registered = LiteralArgumentBuilder
+            .<CommandSource>literal("foo")
+            .then(literal("bar"))
+            .build();
+    manager.register(new BrigadierCommand(registered));
 
+    final var original = LiteralArgumentBuilder
+            .<CommandSource>literal("foo")
+            .then(literal("baz"))
+            .build();
+    dest.addChild(original);
+    manager.getInjector().inject(dest, SOURCE);
+
+    assertEquals(registered, dest.getChild("foo"));
   }
 }

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);
+  }
+}

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(() -> {

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());
+  }
+}

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());
+  }
+}