From 2f0ee15051e00e403e9b79b0ceaf6a607a54eb2b Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Thu, 22 Oct 2020 00:13:20 -0400 Subject: [PATCH] Remove deprecated Velocity 1.0.0 Command API. --- .../velocitypowered/api/command/Command.java | 60 ------------ .../api/command/CommandManager.java | 17 +--- .../api/command/RawCommand.java | 75 -------------- .../proxy/command/CommandNodeFactory.java | 27 ------ .../proxy/command/VelocityCommandManager.java | 34 +------ .../proxy/command/CommandManagerTests.java | 97 ++++--------------- 6 files changed, 25 insertions(+), 285 deletions(-) diff --git a/api/src/main/java/com/velocitypowered/api/command/Command.java b/api/src/main/java/com/velocitypowered/api/command/Command.java index ea095327d..684589578 100644 --- a/api/src/main/java/com/velocitypowered/api/command/Command.java +++ b/api/src/main/java/com/velocitypowered/api/command/Command.java @@ -32,66 +32,6 @@ import org.checkerframework.checker.nullness.qual.NonNull; * to Velocity. * * - * - *

For this reason, the legacy {@code execute}, {@code suggest} and - * {@code hasPermission} methods are deprecated and will be removed - * in Velocity 2.0.0. We suggest implementing one of the more specific - * subinterfaces instead. - * The legacy methods are executed by a {@link CommandManager} if and only if - * the given command directly implements this interface. */ public interface Command { - - /** - * Executes the command for the specified source. - * - * @param source the source to execute the command for - * @param args the arguments for the command - * @deprecated see {@link Command} - */ - @Deprecated - default void execute(final CommandSource source, final String @NonNull [] args) { - throw new UnsupportedOperationException(); - } - - /** - * Provides tab complete suggestions for the specified source. - * - * @param source the source to execute the command for - * @param currentArgs the partial arguments for the command - * @return the tab complete suggestions - * @deprecated see {@link Command} - */ - @Deprecated - default List suggest(final CommandSource source, final String @NonNull [] currentArgs) { - return ImmutableList.of(); - } - - /** - * Provides tab complete suggestions for the specified source. - * - * @param source the source to execute the command for - * @param currentArgs the partial arguments for the command - * @return the tab complete suggestions - * @deprecated see {@link Command} - */ - @Deprecated - default CompletableFuture> suggestAsync(final CommandSource source, - String @NonNull [] currentArgs) { - return CompletableFuture.completedFuture(suggest(source, currentArgs)); - } - - /** - * Tests to check if the source has permission to perform the command with - * the provided arguments. - * - * @param source the source to execute the command for - * @param args the arguments for the command - * @return {@code true} if the source has permission - * @deprecated see {@link Command} - */ - @Deprecated - default boolean hasPermission(final CommandSource source, final String @NonNull [] args) { - return true; - } } 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 f53a44360..968c7a259 100644 --- a/api/src/main/java/com/velocitypowered/api/command/CommandManager.java +++ b/api/src/main/java/com/velocitypowered/api/command/CommandManager.java @@ -33,19 +33,6 @@ public interface CommandManager { */ CommandMeta.Builder metaBuilder(BrigadierCommand command); - /** - * Registers the specified command with the specified aliases. - * - * @param command the command to register - * @param aliases the command aliases - * - * @throws IllegalArgumentException if one of the given aliases is already registered - * @deprecated This method requires at least one alias, but this is only enforced at runtime. - * Prefer {@link #register(String, Command, String...)} - */ - @Deprecated - void register(Command command, String... aliases); - /** * Registers the specified command with the specified aliases. * @@ -54,7 +41,9 @@ public interface CommandManager { * @param otherAliases additional aliases * @throws IllegalArgumentException if one of the given aliases is already registered */ - void register(String alias, Command command, String... otherAliases); + default void register(String alias, Command command, String... otherAliases) { + register(metaBuilder(alias).aliases(otherAliases).build(), command); + } /** * Registers the specified Brigadier command. diff --git a/api/src/main/java/com/velocitypowered/api/command/RawCommand.java b/api/src/main/java/com/velocitypowered/api/command/RawCommand.java index 9ff291268..9cd78d220 100644 --- a/api/src/main/java/com/velocitypowered/api/command/RawCommand.java +++ b/api/src/main/java/com/velocitypowered/api/command/RawCommand.java @@ -7,10 +7,6 @@ package com.velocitypowered.api.command; -import java.util.List; -import java.util.concurrent.CompletableFuture; -import org.checkerframework.checker.nullness.qual.NonNull; - /** * A specialized sub-interface of {@code Command} which indicates the proxy should pass * the command and its arguments directly without further processing. @@ -18,77 +14,6 @@ import org.checkerframework.checker.nullness.qual.NonNull; */ public interface RawCommand extends InvocableCommand { - /** - * Executes the command for the specified source. - * - * @param source the source to execute the command for - * @param cmdLine the arguments for the command - * @deprecated see {@link Command} - */ - @Deprecated - default void execute(final CommandSource source, final String cmdLine) { - throw new UnsupportedOperationException(); - } - - @Deprecated - @Override - default void execute(final CommandSource source, final String @NonNull [] args) { - execute(source, String.join(" ", args)); - } - - @Override - default void execute(Invocation invocation) { - // Guarantees ABI compatibility - } - - /** - * Provides tab complete suggestions for the specified source. - * - * @param source the source to execute the command for - * @param currentArgs the partial arguments for the command - * @return the tab complete suggestions - * @deprecated see {@link Command} - */ - @Deprecated - default CompletableFuture> suggest(final CommandSource source, - final String currentArgs) { - // This method even has an inconsistent return type - throw new UnsupportedOperationException(); - } - - @Deprecated - @Override - default List suggest(final CommandSource source, final String @NonNull [] currentArgs) { - return suggestAsync(source, currentArgs).join(); - } - - @Deprecated - @Override - default CompletableFuture> suggestAsync(final CommandSource source, - final String @NonNull [] currentArgs) { - return suggest(source, String.join(" ", currentArgs)); - } - - /** - * Tests to check if the source has permission to perform the command with - * the provided arguments. - * - * @param source the source to execute the command for - * @param cmdLine the arguments for the command - * @return {@code true} if the source has permission - * @deprecated see {@link Command} - */ - @Deprecated - default boolean hasPermission(final CommandSource source, final String cmdLine) { - throw new UnsupportedOperationException(); - } - - @Deprecated - @Override - default boolean hasPermission(final CommandSource source, final String @NonNull [] args) { - return hasPermission(source, String.join(" ", args)); - } - /** * Contains the invocation data for a raw command. */ diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/CommandNodeFactory.java b/proxy/src/main/java/com/velocitypowered/proxy/command/CommandNodeFactory.java index 8146ff9df..5f68066f8 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/CommandNodeFactory.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/CommandNodeFactory.java @@ -49,33 +49,6 @@ public interface CommandNodeFactory { } }; - CommandNodeFactory FALLBACK = (alias, command) -> - BrigadierUtils.buildRawArgumentsLiteral(alias, - context -> { - CommandSource source = context.getSource(); - String[] args = BrigadierUtils.getSplitArguments(context); - - if (!command.hasPermission(source, args)) { - return BrigadierCommand.FORWARD; - } - command.execute(source, args); - return 1; - }, - (context, builder) -> { - String[] args = BrigadierUtils.getSplitArguments(context); - if (!command.hasPermission(context.getSource(), args)) { - return builder.buildFuture(); - } - - return command.suggestAsync(context.getSource(), args).thenApply(values -> { - for (String value : values) { - builder.suggest(value); - } - - return builder.build(); - }); - }); - /** * Returns a Brigadier node for the execution of the given command. * 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 349bf2ed2..a7bf32596 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java @@ -36,14 +36,12 @@ import com.velocitypowered.api.event.command.CommandExecuteEvent; import com.velocitypowered.api.event.command.CommandExecuteEvent.CommandResult; import com.velocitypowered.proxy.plugin.VelocityEventManager; import com.velocitypowered.proxy.util.BrigadierUtils; -import java.util.Arrays; import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.concurrent.CompletableFuture; import net.kyori.adventure.identity.Identity; import net.kyori.adventure.text.Component; -import net.kyori.adventure.text.TextComponent; import net.kyori.adventure.text.format.NamedTextColor; public class VelocityCommandManager implements CommandManager { @@ -68,20 +66,6 @@ public class VelocityCommandManager implements CommandManager { return new VelocityCommandMeta.Builder(command.getNode().getName()); } - @Override - public void register(final Command command, final String... aliases) { - Preconditions.checkArgument(aliases.length > 0, "no aliases provided"); - register(aliases[0], command, Arrays.copyOfRange(aliases, 1, aliases.length)); - } - - @Override - public void register(final String alias, final Command command, final String... otherAliases) { - Preconditions.checkNotNull(alias, "alias"); - Preconditions.checkNotNull(command, "command"); - Preconditions.checkNotNull(otherAliases, "otherAliases"); - register(metaBuilder(alias).aliases(otherAliases).build(), command); - } - @Override public void register(final BrigadierCommand command) { Preconditions.checkNotNull(command, "command"); @@ -102,20 +86,10 @@ public class VelocityCommandManager implements CommandManager { } else if (command instanceof SimpleCommand) { node = CommandNodeFactory.SIMPLE.create(primaryAlias, (SimpleCommand) command); } else if (command instanceof RawCommand) { - // This ugly hack will be removed in Velocity 2.0. Most if not all plugins - // have side-effect free #suggest methods. We rely on the newer RawCommand - // throwing UOE. - RawCommand asRaw = (RawCommand) command; - try { - asRaw.suggest(null, new String[0]); - } catch (final UnsupportedOperationException e) { - node = CommandNodeFactory.RAW.create(primaryAlias, asRaw); - } catch (final Exception ignored) { - // The implementation probably relies on a non-null source - } - } - if (node == null) { - node = CommandNodeFactory.FALLBACK.create(primaryAlias, command); + node = CommandNodeFactory.RAW.create(primaryAlias, (RawCommand) command); + } else { + throw new IllegalArgumentException("Unknown command implementation for " + + command.getClass().getName()); } if (!(command instanceof BrigadierCommand)) { 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 cdbee1759..d653ea858 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java @@ -125,20 +125,21 @@ public class CommandManagerTests { VelocityCommandManager manager = createManager(); RawCommand command = new NoopRawCommand(); - assertThrows(IllegalArgumentException.class, () -> manager.register(command), - "no aliases throws"); - manager.register(command, "foO", "BAR"); + manager.register("foO", command, "BAR"); assertTrue(manager.hasCommand("fOo")); assertTrue(manager.hasCommand("bar")); } @Test - void testDeprecatedRegister() { + void testAlreadyRegisteredThrows() { VelocityCommandManager manager = createManager(); - Command command = new NoopDeprecatedCommand(); - - manager.register("foo", command); - assertTrue(manager.hasCommand("foO")); + manager.register("BAR", new NoopSimpleCommand()); + assertThrows(IllegalArgumentException.class, () -> { + CommandMeta meta = manager.metaBuilder("baz") + .aliases("BAr") + .build(); + manager.register(meta, new NoopRawCommand()); + }); } @Test @@ -269,35 +270,6 @@ public class CommandManagerTests { assertFalse(manager.executeImmediately(MockCommandSource.INSTANCE, "sendThem foo")); } - @Test - void testDeprecatedExecute() { - VelocityCommandManager manager = createManager(); - AtomicBoolean executed = new AtomicBoolean(false); - Command command = new Command() { - @Override - public void execute(final CommandSource source, final String @NonNull [] args) { - assertEquals(MockCommandSource.INSTANCE, source); - assertArrayEquals(new String[] { "boo", "123" }, args); - executed.set(true); - } - }; - manager.register("foo", command); - - assertTrue(manager.execute(MockCommandSource.INSTANCE, "foo boo 123")); - assertTrue(executed.get()); - - Command noPermsCommand = new Command() { - @Override - public boolean hasPermission(final CommandSource source, final String @NonNull [] args) { - return false; - } - }; - - manager.register("oof", noPermsCommand, "veryOof"); - assertFalse(manager.execute(MockCommandSource.INSTANCE, "veryOOF")); - assertFalse(manager.executeImmediately(MockCommandSource.INSTANCE, "ooF boo 54321")); - } - @Test void testSuggestions() { VelocityCommandManager manager = createManager(); @@ -358,24 +330,8 @@ public class CommandManagerTests { }; manager.register("raw", rawCommand); - Command deprecatedCommand = new Command() { - @Override - public List suggest( - final CommandSource source, final String @NonNull [] currentArgs) { - switch (currentArgs.length) { - case 0: - return ImmutableList.of("boo", "scary"); - case 1: - return ImmutableList.of("123", "456"); - default: - return ImmutableList.of(); - } - } - }; - manager.register("deprecated", deprecatedCommand); - assertEquals( - ImmutableList.of("brigadier", "deprecated", "raw", "simple"), + ImmutableList.of("brigadier", "raw", "simple"), manager.offerSuggestions(MockCommandSource.INSTANCE, "").join(), "literals are in alphabetical order"); assertEquals( @@ -409,16 +365,6 @@ public class CommandManagerTests { assertEquals( ImmutableList.of("11", "13", "17"), manager.offerSuggestions(MockCommandSource.INSTANCE, "rAW bar ").join()); - assertEquals( - ImmutableList.of("boo", "scary"), - manager.offerSuggestions(MockCommandSource.INSTANCE, "deprecated ").join()); - assertTrue(manager.offerSuggestions(MockCommandSource.INSTANCE, "deprecated") - .join().isEmpty()); - assertEquals( - ImmutableList.of("123", "456"), - manager.offerSuggestions(MockCommandSource.INSTANCE, "deprEcated foo").join()); - assertTrue(manager.offerSuggestions(MockCommandSource.INSTANCE, "deprecated foo 789 ") - .join().isEmpty()); } @Test @@ -528,29 +474,29 @@ public class CommandManagerTests { } }; - manager.register(rawCommand, "foo"); + manager.register("foo", rawCommand); assertTrue(manager.offerSuggestions(MockCommandSource.INSTANCE, "foo").get().isEmpty()); assertFalse(manager.offerSuggestions(MockCommandSource.INSTANCE, "foo bar").get().isEmpty()); - Command oldCommand = new Command() { + Command oldCommand = new SimpleCommand() { @Override - public void execute(CommandSource source, String @NonNull [] args) { + public void execute(Invocation invocation) { fail("The Command should not be executed while testing suggestions"); } @Override - public boolean hasPermission(CommandSource source, String @NonNull [] args) { - return args.length > 0; + public List suggest(Invocation invocation) { + return ImmutableList.of("suggestion"); } @Override - public List suggest(CommandSource source, String @NonNull [] currentArgs) { - return ImmutableList.of("suggestion"); + public boolean hasPermission(Invocation invocation) { + return invocation.arguments().length > 0; } }; - manager.register(oldCommand, "bar"); + manager.register("bar", oldCommand); assertTrue(manager.offerSuggestions(MockCommandSource.INSTANCE, "bar").get().isEmpty()); assertFalse(manager.offerSuggestions(MockCommandSource.INSTANCE, "bar foo").get().isEmpty()); @@ -569,11 +515,4 @@ public class CommandManagerTests { } } - - static class NoopDeprecatedCommand implements Command { - @Override - public void execute(final CommandSource source, final String @NonNull [] args) { - - } - } }