From ff504c21ef3f3f9147da1a0f8d14271058523129 Mon Sep 17 00:00:00 2001 From: Hugo Manrique Date: Sat, 5 Jun 2021 18:31:00 +0200 Subject: [PATCH 01/13] Suggestions provider --- .../proxy/command/SuggestionsProvider.java | 120 ++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java new file mode 100644 index 000000000..7a1ea903b --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java @@ -0,0 +1,120 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package com.velocitypowered.proxy.command; + +import com.google.common.base.Preconditions; +import com.mojang.brigadier.CommandDispatcher; +import com.mojang.brigadier.ParseResults; +import com.mojang.brigadier.StringReader; +import com.mojang.brigadier.context.CommandContextBuilder; +import com.mojang.brigadier.context.StringRange; +import com.mojang.brigadier.suggestion.Suggestions; +import com.mojang.brigadier.tree.CommandNode; +import com.mojang.brigadier.tree.LiteralCommandNode; +import com.mojang.brigadier.tree.RootCommandNode; +import java.util.Collection; +import java.util.concurrent.CompletableFuture; + +/** + * Provides suggestions for a given command input. + * + *

Similar to {@link CommandDispatcher#getCompletionSuggestions(ParseResults)}, except it + * avoids fully parsing the given input and performs exactly one requirement predicate check + * per considered node. + * + * @param the type of the command source + */ +final class SuggestionsProvider { + + private final CommandDispatcher dispatcher; + + SuggestionsProvider(final CommandDispatcher dispatcher) { + this.dispatcher = Preconditions.checkNotNull(dispatcher, "dispatcher"); + } + + /** + * Provides suggestions for the given input and source. + * + * @param input the partial input + * @param source the command source invoking the command + * @return a future that completes with the suggestions + */ + public CompletableFuture provideSuggestions(final String input, final S source) { + final CommandContextBuilder context = new CommandContextBuilder<>( + this.dispatcher, source, this.dispatcher.getRoot(), 0); + return this.provideSuggestions(new StringReader(input), context); + } + + /** + * Provides suggestions for the given input and context. + * + * @param reader the input reader + * @param context an empty context + * @return a future that completes with the suggestions + */ + private CompletableFuture provideSuggestions( + final StringReader reader, final CommandContextBuilder context) { + final StringRange aliasRange = this.consumeAlias(reader); + final String alias = aliasRange.get(reader); // TODO #toLowerCase and test + final LiteralCommandNode literal = + (LiteralCommandNode) context.getRootNode().getChild(alias); + + final boolean hasArguments = reader.canRead(); + if (hasArguments) { + if (literal == null) { + // Input has arguments for non-registered alias + return Suggestions.empty(); + } + context.withNode(literal, aliasRange); + reader.skip(); // separator + // TODO Provide arguments suggestions + } else { + // TODO Provide alias suggestions + } + } + + private StringRange consumeAlias(final StringReader reader) { + final int firstSep = reader.getString().indexOf( + CommandDispatcher.ARGUMENT_SEPARATOR_CHAR, reader.getCursor()); + final StringRange range = StringRange.between( + reader.getCursor(), firstSep == -1 ? reader.getTotalLength() : firstSep); + reader.setCursor(range.getEnd()); + return range; + } + + /** + * Returns alias suggestions for the given input. + * + * @param reader the input reader + * @param contextSoFar an empty context + * @return a future that completes with the suggestions + */ + private CompletableFuture provideAliasSuggestions( + final StringReader reader, final CommandContextBuilder contextSoFar) { + final S source = contextSoFar.getSource(); + final String input = reader.getRead(); + + final Collection> aliases = contextSoFar.getRootNode().getChildren(); + final CompletableFuture[] futures = new CompletableFuture[aliases.size()]; + + } + + /*private RootCommandNode getRoot() { + return this.dispatcher.getRoot(); + }*/ +} From 703b91e0fa9a4b52fcc329d64a94c1707648139b Mon Sep 17 00:00:00 2001 From: Hugo Manrique Date: Sat, 5 Jun 2021 23:19:01 +0200 Subject: [PATCH 02/13] Command implementation refactor --- .../proxy/command/CommandGraphInjector.java | 123 ++++++++ .../command/CommandInvocationFactory.java | 40 --- .../proxy/command/CommandNodeFactory.java | 94 ------ .../proxy/command/SuggestionsProvider.java | 271 ++++++++++++++++-- .../proxy/command/VelocityCommandManager.java | 121 +++++--- .../proxy/command/VelocityCommandMeta.java | 83 +++++- .../proxy/command/VelocityCommands.java | 174 +++++++++++ .../command/VelocityRawCommandInvocation.java | 54 ---- .../VelocitySimpleCommandInvocation.java | 52 ---- .../brigadier/StringArrayArgumentType.java | 46 +++ .../brigadier/VelocityArgumentBuilder.java | 61 ++++ .../VelocityArgumentCommandNode.java | 121 ++++++++ .../AbstractCommandInvocation.java | 30 +- .../invocation/CommandInvocationFactory.java | 78 +++++ .../invocation/RawCommandInvocation.java | 90 ++++++ .../invocation/SimpleCommandInvocation.java | 93 ++++++ .../registrar/AbstractCommandRegistrar.java | 44 +++ .../registrar/BrigadierCommandRegistrar.java | 46 +++ .../command/registrar/CommandRegistrar.java | 33 +++ .../registrar/InvocableCommandRegistrar.java | 105 +++++++ .../registrar/RawCommandRegistrar.java | 24 ++ .../registrar/SimpleCommandRegistrar.java | 24 ++ .../backend/BackendPlaySessionHandler.java | 59 +--- .../proxy/util/BrigadierUtils.java | 169 ----------- .../command/CommandGraphInjectorTests.java | 53 ++++ .../command/SuggestionsProviderTests.java | 5 + .../StringArrayArgumentTypeTests.java | 94 ++++++ .../VelocityArgumentCommandNodeTests.java | 81 ++++++ 28 files changed, 1726 insertions(+), 542 deletions(-) create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/CommandGraphInjector.java delete mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/CommandInvocationFactory.java delete mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/CommandNodeFactory.java create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommands.java delete mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/VelocityRawCommandInvocation.java delete mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/VelocitySimpleCommandInvocation.java create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentType.java create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentBuilder.java create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNode.java rename proxy/src/main/java/com/velocitypowered/proxy/command/{ => invocation}/AbstractCommandInvocation.java (69%) create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/invocation/CommandInvocationFactory.java create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/invocation/RawCommandInvocation.java create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/invocation/SimpleCommandInvocation.java create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/registrar/AbstractCommandRegistrar.java create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/registrar/BrigadierCommandRegistrar.java create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/registrar/CommandRegistrar.java create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/registrar/InvocableCommandRegistrar.java create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/registrar/RawCommandRegistrar.java create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/command/registrar/SimpleCommandRegistrar.java delete mode 100644 proxy/src/main/java/com/velocitypowered/proxy/util/BrigadierUtils.java create mode 100644 proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java create mode 100644 proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java create mode 100644 proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentTypeTests.java create mode 100644 proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNodeTests.java diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/CommandGraphInjector.java b/proxy/src/main/java/com/velocitypowered/proxy/command/CommandGraphInjector.java new file mode 100644 index 000000000..8ea2d5775 --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/CommandGraphInjector.java @@ -0,0 +1,123 @@ +package com.velocitypowered.proxy.command; + +import com.google.common.base.Preconditions; +import com.mojang.brigadier.CommandDispatcher; +import com.mojang.brigadier.StringReader; +import com.mojang.brigadier.builder.ArgumentBuilder; +import com.mojang.brigadier.context.CommandContextBuilder; +import com.mojang.brigadier.context.StringRange; +import com.mojang.brigadier.tree.CommandNode; +import com.mojang.brigadier.tree.LiteralCommandNode; +import com.mojang.brigadier.tree.RootCommandNode; +import com.velocitypowered.proxy.command.brigadier.VelocityArgumentCommandNode; +import java.util.concurrent.locks.Lock; +import org.checkerframework.checker.lock.qual.GuardedBy; +import org.checkerframework.checker.nullness.qual.Nullable; + +/** + * Copies the nodes of a {@link RootCommandNode} to a possibly non-empty + * destination {@link RootCommandNode}, respecting the requirements satisfied + * by a given command source. + * + * @param the type of the source to inject the nodes for + */ +public final class CommandGraphInjector { + + private static final StringRange ALIAS_RANGE = StringRange.at(0); + private static final StringReader ALIAS_READER = new StringReader(""); + + private final @GuardedBy("lock") CommandDispatcher dispatcher; + private final Lock lock; + + CommandGraphInjector(final CommandDispatcher dispatcher, final Lock lock) { + this.dispatcher = Preconditions.checkNotNull(dispatcher, "dispatcher"); + this.lock = Preconditions.checkNotNull(lock, "lock"); + } + + // The term "source" is ambiguous here. We use "origin" when referring to + // the root node we are copying nodes from to the destination node. + + /** + * Adds the node from the root node of this injector to the given root node, + * respecting the requirements satisfied by the given source. + * + *

Prior to adding a literal with the same name as one previously contained + * in the destination node, the old node is removed from the destination node. + * + * @param dest the root node to add the permissible nodes to + * @param source the command source to inject the nodes for + */ + public void inject(final RootCommandNode dest, final S source) { + lock.lock(); + try { + final RootCommandNode origin = this.dispatcher.getRoot(); + final CommandContextBuilder rootContext = + new CommandContextBuilder<>(this.dispatcher, source, origin, 0); + + // Filter alias nodes + for (final CommandNode node : origin.getChildren()) { + if (!node.canUse(source)) { + continue; + } + + final CommandContextBuilder context = rootContext.copy() + .withNode(node, ALIAS_RANGE); + if (!node.canUse(context, ALIAS_READER)) { + continue; + } + + final LiteralCommandNode asLiteral = (LiteralCommandNode) node; + final LiteralCommandNode copy = asLiteral.createBuilder().build(); + final VelocityArgumentCommandNode argsNode = + VelocityCommands.getArgumentsNode(asLiteral); + if (argsNode == null) { + // This literal is associated to a BrigadierCommand, filter normally + this.copyChildren(node, copy, source); + } else { + // Copy all children nodes (arguments node and hints) + for (final CommandNode child : node.getChildren()) { + copy.addChild(child); + } + } + this.addAlias(copy, dest); + } + } finally { + lock.unlock(); + } + } + + private @Nullable CommandNode filterNode(final CommandNode node, final S source) { + // We only check the non-context requirement when filtering alias nodes. + // Otherwise, we would need to manually craft context builder and reader instances, + // which is both incorrect and inefficient. The reason why we can do so for alias + // literals is due to the empty string being a valid and expected input by + // the context-aware requirement (when suggesting the literal name). + if (!node.canUse(source)) { + return null; + } + final ArgumentBuilder builder = node.createBuilder(); + if (node.getRedirect() != null) { + // TODO Document redirects to non-Brigadier commands are not supported + final CommandNode target = this.filterNode(node.getRedirect(), source); + builder.forward(target, builder.getRedirectModifier(), builder.isFork()); + } + final CommandNode result = builder.build(); + this.copyChildren(node, result, source); + return result; + } + + private void copyChildren(final CommandNode parent, final CommandNode dest, + final S source) { + for (final CommandNode child : parent.getChildren()) { + final CommandNode filtered = this.filterNode(child, source); + if (filtered != null) { + dest.addChild(filtered); + } + } + } + + private void addAlias(final LiteralCommandNode node, final RootCommandNode dest) { + dest.removeChildByName(node.getName()); + dest.addChild(node); + } +} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/CommandInvocationFactory.java b/proxy/src/main/java/com/velocitypowered/proxy/command/CommandInvocationFactory.java deleted file mode 100644 index 05a4d3b90..000000000 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/CommandInvocationFactory.java +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright (C) 2018 Velocity Contributors - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - -package com.velocitypowered.proxy.command; - -import com.mojang.brigadier.context.CommandContext; -import com.velocitypowered.api.command.CommandInvocation; -import com.velocitypowered.api.command.CommandSource; - -/** - * Creates command invocation contexts for the given {@link CommandSource} - * and command line arguments. - * - * @param the type of the built invocation - */ -@FunctionalInterface -public interface CommandInvocationFactory> { - - /** - * Returns an invocation context for the given Brigadier context. - * - * @param context the command context - * @return the built invocation context - */ - I create(final CommandContext context); -} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/CommandNodeFactory.java b/proxy/src/main/java/com/velocitypowered/proxy/command/CommandNodeFactory.java deleted file mode 100644 index 5f68066f8..000000000 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/CommandNodeFactory.java +++ /dev/null @@ -1,94 +0,0 @@ -/* - * Copyright (C) 2018 Velocity Contributors - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - -package com.velocitypowered.proxy.command; - -import com.mojang.brigadier.context.CommandContext; -import com.mojang.brigadier.tree.LiteralCommandNode; -import com.velocitypowered.api.command.BrigadierCommand; -import com.velocitypowered.api.command.Command; -import com.velocitypowered.api.command.CommandInvocation; -import com.velocitypowered.api.command.CommandSource; -import com.velocitypowered.api.command.InvocableCommand; -import com.velocitypowered.api.command.RawCommand; -import com.velocitypowered.api.command.SimpleCommand; -import com.velocitypowered.proxy.util.BrigadierUtils; - -@FunctionalInterface -public interface CommandNodeFactory { - - InvocableCommandNodeFactory SIMPLE = - new InvocableCommandNodeFactory() { - @Override - protected SimpleCommand.Invocation createInvocation( - final CommandContext context) { - return VelocitySimpleCommandInvocation.FACTORY.create(context); - } - }; - - InvocableCommandNodeFactory RAW = - new InvocableCommandNodeFactory() { - @Override - protected RawCommand.Invocation createInvocation( - final CommandContext context) { - return VelocityRawCommandInvocation.FACTORY.create(context); - } - }; - - /** - * Returns a Brigadier node for the execution of the given command. - * - * @param alias the command alias - * @param command the command to execute - * @return the command node - */ - LiteralCommandNode create(String alias, T command); - - abstract class InvocableCommandNodeFactory> - implements CommandNodeFactory> { - - @Override - public LiteralCommandNode create( - final String alias, final InvocableCommand command) { - return BrigadierUtils.buildRawArgumentsLiteral(alias, - context -> { - I invocation = createInvocation(context); - if (!command.hasPermission(invocation)) { - return BrigadierCommand.FORWARD; - } - command.execute(invocation); - return 1; - }, - (context, builder) -> { - I invocation = createInvocation(context); - - if (!command.hasPermission(invocation)) { - return builder.buildFuture(); - } - return command.suggestAsync(invocation).thenApply(values -> { - for (String value : values) { - builder.suggest(value); - } - - return builder.build(); - }); - }); - } - - protected abstract I createInvocation(final CommandContext context); - } -} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java index 7a1ea903b..869d7ccb7 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java @@ -21,14 +21,26 @@ import com.google.common.base.Preconditions; import com.mojang.brigadier.CommandDispatcher; import com.mojang.brigadier.ParseResults; import com.mojang.brigadier.StringReader; +import com.mojang.brigadier.context.CommandContext; import com.mojang.brigadier.context.CommandContextBuilder; import com.mojang.brigadier.context.StringRange; +import com.mojang.brigadier.exceptions.CommandSyntaxException; import com.mojang.brigadier.suggestion.Suggestions; +import com.mojang.brigadier.suggestion.SuggestionsBuilder; import com.mojang.brigadier.tree.CommandNode; import com.mojang.brigadier.tree.LiteralCommandNode; -import com.mojang.brigadier.tree.RootCommandNode; +import com.velocitypowered.api.command.Command; +import com.velocitypowered.proxy.command.brigadier.VelocityArgumentCommandNode; +import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Locale; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.locks.Lock; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.checkerframework.checker.lock.qual.GuardedBy; /** * Provides suggestions for a given command input. @@ -41,10 +53,16 @@ import java.util.concurrent.CompletableFuture; */ final class SuggestionsProvider { - private final CommandDispatcher dispatcher; + private static final Logger LOGGER = LogManager.getLogger(SuggestionsProvider.class); - SuggestionsProvider(final CommandDispatcher dispatcher) { + private static final StringRange ALIAS_SUGGESTION_RANGE = StringRange.at(0); + + private final @GuardedBy("lock") CommandDispatcher dispatcher; + private final Lock lock; + + SuggestionsProvider(final CommandDispatcher dispatcher, final Lock lock) { this.dispatcher = Preconditions.checkNotNull(dispatcher, "dispatcher"); + this.lock = Preconditions.checkNotNull(lock, "lock"); } /** @@ -69,22 +87,27 @@ final class SuggestionsProvider { */ private CompletableFuture provideSuggestions( final StringReader reader, final CommandContextBuilder context) { - final StringRange aliasRange = this.consumeAlias(reader); - final String alias = aliasRange.get(reader); // TODO #toLowerCase and test - final LiteralCommandNode literal = - (LiteralCommandNode) context.getRootNode().getChild(alias); + lock.lock(); + try { + final StringRange aliasRange = this.consumeAlias(reader); + final String alias = aliasRange.get(reader).toLowerCase(Locale.ENGLISH); + final LiteralCommandNode literal = + (LiteralCommandNode) context.getRootNode().getChild(alias); - final boolean hasArguments = reader.canRead(); - if (hasArguments) { - if (literal == null) { - // Input has arguments for non-registered alias - return Suggestions.empty(); + final boolean hasArguments = reader.canRead(); + if (hasArguments) { + if (literal == null) { + // Input has arguments for non-registered alias + return Suggestions.empty(); + } + context.withNode(literal, aliasRange); + reader.skip(); // separator + return this.provideArgumentsSuggestions(literal, reader, context); + } else { + return this.provideAliasSuggestions(reader, context); } - context.withNode(literal, aliasRange); - reader.skip(); // separator - // TODO Provide arguments suggestions - } else { - // TODO Provide alias suggestions + } finally { + lock.unlock(); } } @@ -97,6 +120,19 @@ final class SuggestionsProvider { return range; } + /** + * Returns whether a literal node with the given name should be considered for + * suggestions given the specified input. + * + * @param name the literal name + * @param input the partial input + * @return true if the literal should be considered; false otherwise + */ + private static boolean shouldConsider(final String name, final String input) { + // TODO (perf) If we expect input to be lowercase, no need to ignore case + return name.regionMatches(true, 0, input, 0, input.length()); + } + /** * Returns alias suggestions for the given input. * @@ -110,11 +146,206 @@ final class SuggestionsProvider { final String input = reader.getRead(); final Collection> aliases = contextSoFar.getRootNode().getChildren(); + @SuppressWarnings("unchecked") final CompletableFuture[] futures = new CompletableFuture[aliases.size()]; + int i = 0; + for (final CommandNode node : aliases) { + CompletableFuture future = Suggestions.empty(); + final String alias = node.getName(); + if (shouldConsider(alias, input) && node.canUse(source)) { + final CommandContextBuilder context = contextSoFar.copy() + .withNode(node, ALIAS_SUGGESTION_RANGE); + if (node.canUse(context, reader)) { + // LiteralCommandNode#listSuggestions is case insensitive + final SuggestionsBuilder builder = new SuggestionsBuilder(input, 0); + future = builder.suggest(alias).buildFuture(); + } + } + futures[i++] = future; + } + return this.merge(input, futures); } - /*private RootCommandNode getRoot() { - return this.dispatcher.getRoot(); - }*/ + /** + * Merges the suggestions provided by the {@link Command} associated to the given + * alias node and the hints given during registration for the given input. + * + *

The context is not mutated by this method. The reader's cursor may be modified. + * + * @param alias the alias node + * @param reader the input reader + * @param contextSoFar the context, containing {@code alias} + * @return a future that completes with the suggestions + */ + private CompletableFuture provideArgumentsSuggestions( + final LiteralCommandNode alias, final StringReader reader, + final CommandContextBuilder contextSoFar) { + final S source = contextSoFar.getSource(); + final VelocityArgumentCommandNode argsNode = VelocityCommands.getArgumentsNode(alias); + if (argsNode == null) { + // This is a BrigadierCommand, fallback to regular suggestions + reader.setCursor(0); + final ParseResults parse = this.dispatcher.parse(reader, source); + return this.dispatcher.getCompletionSuggestions(parse); + } + + if (!argsNode.canUse(source)) { + return Suggestions.empty(); + } + + final int start = reader.getCursor(); + final CommandContextBuilder context = contextSoFar.copy(); + try { + argsNode.parse(reader, context); // reads remaining input + } catch (final CommandSyntaxException e) { + throw new RuntimeException(e); + } + + if (!argsNode.canUse(context, reader)) { + return Suggestions.empty(); + } + + // Ask the command for suggestions via the arguments node + reader.setCursor(start); + final CompletableFuture cmdSuggestions = + this.getArgumentsNodeSuggestions(argsNode, reader, context); + final boolean hasHints = alias.getChildren().size() > 1; + if (!hasHints) { + return cmdSuggestions; + } + + // Parse the hint nodes to get remaining suggestions + reader.setCursor(start); + final CompletableFuture hintSuggestions = + this.getHintSuggestions(alias, reader, contextSoFar); + + final String fullInput = reader.getString(); + return this.merge(fullInput, cmdSuggestions, hintSuggestions); + } + + /** + * Returns the suggestions provided by the {@link Command} associated to + * the specified arguments node for the given input. + * + *

The reader and context are not mutated by this method. + * + * @param node the arguments node of the command + * @param reader the input reader + * @param context the context, containing an alias node and {@code node} + * @return a future that completes with the suggestions + */ + private CompletableFuture getArgumentsNodeSuggestions( + final VelocityArgumentCommandNode node, final StringReader reader, + final CommandContextBuilder context) { + final int start = reader.getCursor(); + final String fullInput = reader.getString(); + final CommandContext built = context.build(fullInput); + try { + return node.listSuggestions(built, new SuggestionsBuilder(fullInput, start)); + } catch (final Throwable e) { + // Ugly, ugly swallowing of everything Throwable, because plugins are naughty. + LOGGER.error("Arguments node cannot provide suggestions, skipping", e); + return Suggestions.empty(); + } + } + + /** + * Returns the suggestions provided by the matched hint nodes for the given input. + * + *

The reader and context are not mutated by this method. + * + * @param alias the alias node + * @param reader the input reader + * @param context the context, containing {@code alias} + * @return a future that completes with the suggestions + */ + private CompletableFuture getHintSuggestions( + final LiteralCommandNode alias, final StringReader reader, + final CommandContextBuilder context) { + final ParseResults parse = this.parseHints(alias, reader, context); + return this.dispatcher.getCompletionSuggestions(parse); + } + + /** + * Parses the hint nodes under the given node, which is either an alias node of + * a {@link Command} or another hint node. + * + *

The reader and context are not mutated by this method. + * + * @param node the node to parse + * @param originalReader the input reader + * @param contextSoFar the context, containing the alias node of the command + * @return the parse results containing the parsed hint nodes + */ + private ParseResults parseHints(final CommandNode node, final StringReader originalReader, + final CommandContextBuilder contextSoFar) { + // This is a stripped-down version of CommandDispatcher#parseNodes that doesn't + // check the requirements are satisfied and ignores redirects, neither of which + // are used by hint nodes. Parsing errors are ignored. + List> potentials = null; + for (final CommandNode child : node.getRelevantNodes(originalReader)) { + if (VelocityCommands.isArgumentsNode(child)) { + continue; + } + final CommandContextBuilder context = contextSoFar.copy(); + final StringReader reader = new StringReader(originalReader); + try { + // We intentionally don't catch all unchecked exceptions + child.parse(reader, context); + if (reader.canRead() && reader.peek() != CommandDispatcher.ARGUMENT_SEPARATOR_CHAR) { + continue; + } + } catch (final CommandSyntaxException e) { + continue; + } + if (reader.canRead(2)) { // separator + string + reader.skip(); // separator + final ParseResults parse = this.parseHints(child, reader, context); + if (potentials == null) { + potentials = new ArrayList<>(1); + } + potentials.add(parse); + } + } + if (potentials != null) { + if (potentials.size() > 1) { + potentials.sort((a, b) -> { + if (!a.getReader().canRead() && b.getReader().canRead()) { + return -1; + } + if (a.getReader().canRead() && !b.getReader().canRead()) { + return 1; + } + return 0; + }); + } + return potentials.get(0); + } + return new ParseResults<>(contextSoFar, originalReader, Collections.emptyMap()); + } + + /** + * Returns a future that is completed with the result of merging the {@link Suggestions} + * the given futures complete with. The results of the futures that complete exceptionally + * are ignored. + * + * @param fullInput the command input + * @param futures the futures that complete with the suggestions + * @return the future that completes with the merged suggestions + */ + @SafeVarargs + private CompletableFuture merge( + final String fullInput, final CompletableFuture... futures) { + // https://github.com/Mojang/brigadier/pull/81 + return CompletableFuture.allOf(futures).handle((unused, throwable) -> { + final List suggestions = new ArrayList<>(futures.length); + for (final CompletableFuture future : futures) { + if (!future.isCompletedExceptionally()) { + suggestions.add(future.join()); + } + } + return Suggestions.merge(fullInput, suggestions); + }); + } } 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 48295a2bb..0d15f02b6 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java @@ -18,40 +18,61 @@ package com.velocitypowered.proxy.command; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import com.mojang.brigadier.CommandDispatcher; import com.mojang.brigadier.ParseResults; import com.mojang.brigadier.exceptions.CommandSyntaxException; import com.mojang.brigadier.suggestion.Suggestion; -import com.mojang.brigadier.tree.CommandNode; -import com.mojang.brigadier.tree.LiteralCommandNode; +import com.mojang.brigadier.tree.RootCommandNode; import com.velocitypowered.api.command.BrigadierCommand; import com.velocitypowered.api.command.Command; import com.velocitypowered.api.command.CommandManager; import com.velocitypowered.api.command.CommandMeta; import com.velocitypowered.api.command.CommandSource; -import com.velocitypowered.api.command.RawCommand; -import com.velocitypowered.api.command.SimpleCommand; import com.velocitypowered.api.event.command.CommandExecuteEvent; import com.velocitypowered.api.event.command.CommandExecuteEvent.CommandResult; +import com.velocitypowered.proxy.command.registrar.BrigadierCommandRegistrar; +import com.velocitypowered.proxy.command.registrar.CommandRegistrar; +import com.velocitypowered.proxy.command.registrar.RawCommandRegistrar; +import com.velocitypowered.proxy.command.registrar.SimpleCommandRegistrar; import com.velocitypowered.proxy.event.VelocityEventManager; -import com.velocitypowered.proxy.util.BrigadierUtils; -import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; import net.kyori.adventure.identity.Identity; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.format.NamedTextColor; +import org.checkerframework.checker.lock.qual.GuardedBy; public class VelocityCommandManager implements CommandManager { - private final CommandDispatcher dispatcher; - private final VelocityEventManager eventManager; + private final @GuardedBy("lock") CommandDispatcher dispatcher; + private final ReadWriteLock lock; + private final VelocityEventManager eventManager; + private final List> registrars; + private final SuggestionsProvider suggestionsProvider; + private final CommandGraphInjector injector; + + /** + * Constructs a command manager. + * + * @param eventManager the event manager + */ public VelocityCommandManager(final VelocityEventManager eventManager) { - this.eventManager = Preconditions.checkNotNull(eventManager); + this.lock = new ReentrantReadWriteLock(); this.dispatcher = new CommandDispatcher<>(); + this.eventManager = Preconditions.checkNotNull(eventManager); + final RootCommandNode root = this.dispatcher.getRoot(); + this.registrars = ImmutableList.of( + new BrigadierCommandRegistrar(root, this.lock.writeLock()), + new SimpleCommandRegistrar(root, this.lock.writeLock()), + new RawCommandRegistrar(root, this.lock.writeLock())); + this.suggestionsProvider = new SuggestionsProvider<>(this.dispatcher, this.lock.readLock()); + this.injector = new CommandGraphInjector<>(this.dispatcher, this.lock.readLock()); } @Override @@ -77,42 +98,34 @@ public class VelocityCommandManager implements CommandManager { Preconditions.checkNotNull(meta, "meta"); Preconditions.checkNotNull(command, "command"); - Iterator aliasIterator = meta.getAliases().iterator(); - String primaryAlias = aliasIterator.next(); - - LiteralCommandNode node = null; - if (command instanceof BrigadierCommand) { - node = ((BrigadierCommand) command).getNode(); - } else if (command instanceof SimpleCommand) { - node = CommandNodeFactory.SIMPLE.create(primaryAlias, (SimpleCommand) command); - } else if (command instanceof RawCommand) { - node = CommandNodeFactory.RAW.create(primaryAlias, (RawCommand) command); - } else { - throw new IllegalArgumentException("Unknown command implementation for " - + command.getClass().getName()); - } - - if (!(command instanceof BrigadierCommand)) { - for (CommandNode hint : meta.getHints()) { - node.addChild(BrigadierUtils.wrapForHinting(hint, node.getCommand())); + for (final CommandRegistrar registrar : this.registrars) { + if (this.tryRegister(registrar, command, meta)) { + return; } } + throw new IllegalArgumentException( + command + " does not implement a registrable Command subinterface"); + } - dispatcher.getRoot().addChild(node); - while (aliasIterator.hasNext()) { - String currentAlias = aliasIterator.next(); - CommandNode existingNode = dispatcher.getRoot() - .getChild(currentAlias.toLowerCase(Locale.ENGLISH)); - if (existingNode != null) { - dispatcher.getRoot().getChildren().remove(existingNode); - } - dispatcher.getRoot().addChild(BrigadierUtils.buildRedirect(currentAlias, node)); + private boolean tryRegister(final CommandRegistrar registrar, + final Command command, final CommandMeta meta) { + final Class superInterface = registrar.registrableSuperInterface(); + if (!superInterface.isInstance(command)) { + return false; + } + try { + registrar.register(superInterface.cast(command), meta); + return true; + } catch (final IllegalArgumentException ignored) { + return false; } } @Override public void unregister(final String alias) { Preconditions.checkNotNull(alias, "alias"); + // The literals of secondary aliases will preserve the children of + // the removed literal in the graph. dispatcher.getRoot().removeChildByName(alias.toLowerCase(Locale.ENGLISH)); } @@ -134,9 +147,10 @@ public class VelocityCommandManager implements CommandManager { Preconditions.checkNotNull(source, "source"); Preconditions.checkNotNull(cmdLine, "cmdLine"); - ParseResults results = parse(cmdLine, source, true); + final String normalizedInput = VelocityCommands.normalizeInput(cmdLine, true); + final ParseResults parse = this.parse(normalizedInput, source); try { - return dispatcher.execute(results) != BrigadierCommand.FORWARD; + return dispatcher.execute(parse) != BrigadierCommand.FORWARD; } catch (final CommandSyntaxException e) { boolean isSyntaxError = !e.getType().equals( CommandSyntaxException.BUILT_IN_EXCEPTIONS.dispatcherUnknownCommand()); @@ -190,22 +204,32 @@ public class VelocityCommandManager implements CommandManager { Preconditions.checkNotNull(source, "source"); Preconditions.checkNotNull(cmdLine, "cmdLine"); - ParseResults parse = parse(cmdLine, source, false); - return dispatcher.getCompletionSuggestions(parse) - .thenApply(suggestions -> Lists.transform(suggestions.getList(), Suggestion::getText)); + final String normalizedInput = VelocityCommands.normalizeInput(cmdLine, false); + return suggestionsProvider.provideSuggestions(normalizedInput, source) + .thenApply(suggestions -> Lists.transform(suggestions.getList(), Suggestion::getText)); } - private ParseResults parse(final String cmdLine, final CommandSource source, - final boolean trim) { - String normalized = BrigadierUtils.normalizeInput(cmdLine, trim); - return dispatcher.parse(normalized, source); + /** + * Parses the given command input. + * + * @param input the normalized command input, without the leading slash ('/') + * @param source the command source to parse the command for + * @return the parse results + */ + private ParseResults parse(final String input, final CommandSource source) { + lock.readLock().lock(); + try { + return dispatcher.parse(input, source); + } finally { + lock.readLock().unlock(); + } } /** * Returns whether the given alias is registered on this manager. * * @param alias the command alias to check - * @return {@code true} if the alias is registered + * @return true if the alias is registered; false otherwise */ @Override public boolean hasCommand(final String alias) { @@ -214,6 +238,11 @@ public class VelocityCommandManager implements CommandManager { } public CommandDispatcher getDispatcher() { + // TODO Can we remove this? This is only used by tests, and constitutes unsafe publication. return dispatcher; } + + public CommandGraphInjector getInjector() { + return injector; + } } \ No newline at end of file diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandMeta.java b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandMeta.java index c45b51080..b3449e4f3 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandMeta.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandMeta.java @@ -20,15 +20,18 @@ package com.velocitypowered.proxy.command; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.mojang.brigadier.builder.ArgumentBuilder; import com.mojang.brigadier.tree.CommandNode; +import com.velocitypowered.api.command.Command; import com.velocitypowered.api.command.CommandMeta; import com.velocitypowered.api.command.CommandSource; import java.util.Collection; import java.util.List; import java.util.Locale; import java.util.Set; +import java.util.stream.Stream; -final class VelocityCommandMeta implements CommandMeta { +public final class VelocityCommandMeta implements CommandMeta { static final class Builder implements CommandMeta.Builder { @@ -46,9 +49,9 @@ final class VelocityCommandMeta implements CommandMeta { public CommandMeta.Builder aliases(final String... aliases) { Preconditions.checkNotNull(aliases, "aliases"); for (int i = 0, length = aliases.length; i < length; i++) { - final String alias1 = aliases[i]; - Preconditions.checkNotNull(alias1, "alias at index %s", i); - this.aliases.add(alias1.toLowerCase(Locale.ENGLISH)); + final String alias = aliases[i]; + Preconditions.checkNotNull(alias, "alias at index %s", i); + this.aliases.add(alias.toLowerCase(Locale.ENGLISH)); } return this; } @@ -56,16 +59,56 @@ final class VelocityCommandMeta implements CommandMeta { @Override public CommandMeta.Builder hint(final CommandNode node) { Preconditions.checkNotNull(node, "node"); - hints.add(node); + if (node.getCommand() != null) { + throw new IllegalArgumentException("Cannot use executable node for hinting"); + } + if (node.getRedirect() != null) { + throw new IllegalArgumentException("Cannot use a node with a redirect for hinting"); + } + this.hints.add(node); return this; } @Override public CommandMeta build() { - return new VelocityCommandMeta(aliases.build(), hints.build()); + return new VelocityCommandMeta(this.aliases.build(), this.hints.build()); } } + /** + * Creates a node to use for hinting the arguments of a {@link Command}. Hint nodes are + * sent to 1.13+ clients and the proxy uses them for providing suggestions. + * + *

A hint node is used to provide suggestions if and only if the requirements of + * the corresponding {@link CommandNode} are satisfied. The requirement predicate + * of the returned node always returns {@code false}. + * + * @param hint the node containing hinting metadata + * @return the hinting command node + */ + private static CommandNode copyForHinting(final CommandNode hint) { + // We need to perform a deep copy of the hint to prevent the user + // from modifying the nodes and adding a Command or a redirect. + final ArgumentBuilder builder = hint.createBuilder() + // Requirement checking is performed by SuggestionProvider + .requires(source -> false); + for (final CommandNode child : hint.getChildren()) { + builder.then(copyForHinting(child)); + } + return builder.build(); + } + + /** + * Returns a stream of copies of every hint contained in the given metadata object. + * + * @param meta the command metadata + * @return a stream of hinting nodes + */ + // This is a static method because most methods take a CommandMeta. + public static Stream> copyHints(final CommandMeta meta) { + return meta.getHints().stream().map(VelocityCommandMeta::copyForHinting); + } + private final Set aliases; private final List> hints; @@ -77,11 +120,35 @@ final class VelocityCommandMeta implements CommandMeta { @Override public Collection getAliases() { - return aliases; + return this.aliases; } @Override public Collection> getHints() { - return hints; + return this.hints; + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + final VelocityCommandMeta that = (VelocityCommandMeta) o; + + if (!this.aliases.equals(that.aliases)) { + return false; + } + return this.hints.equals(that.hints); + } + + @Override + public int hashCode() { + int result = this.aliases.hashCode(); + result = 31 * result + this.hints.hashCode(); + return result; } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommands.java b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommands.java new file mode 100644 index 000000000..9d903aa40 --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommands.java @@ -0,0 +1,174 @@ +package com.velocitypowered.proxy.command; + +import com.google.common.base.Preconditions; +import com.mojang.brigadier.CommandDispatcher; +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import com.mojang.brigadier.context.CommandContext; +import com.mojang.brigadier.context.CommandContextBuilder; +import com.mojang.brigadier.context.ParsedArgument; +import com.mojang.brigadier.context.ParsedCommandNode; +import com.mojang.brigadier.tree.CommandNode; +import com.mojang.brigadier.tree.LiteralCommandNode; +import com.mojang.brigadier.tree.RootCommandNode; +import com.velocitypowered.api.command.Command; +import com.velocitypowered.api.command.CommandManager; +import com.velocitypowered.api.command.CommandSource; +import com.velocitypowered.api.command.InvocableCommand; +import com.velocitypowered.proxy.command.brigadier.VelocityArgumentCommandNode; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import org.checkerframework.checker.nullness.qual.Nullable; + +/** + * Provides utility methods common to most {@link Command} implementations. + * In particular, {@link InvocableCommand} implementations use the same logic for + * creating and parsing the alias and arguments command nodes, which is contained + * in this class. + */ +public final class VelocityCommands { + + // Normalization + + /** + * Normalizes the given command input. + * + * @param input the raw command input, without the leading slash ('/') + * @param trim whether to remove leading and trailing whitespace from the input + * @return the normalized command input + */ + static String normalizeInput(final String input, final boolean trim) { + final String command = trim ? input.trim() : input; + int firstSep = command.indexOf(CommandDispatcher.ARGUMENT_SEPARATOR_CHAR); + if (firstSep != -1) { + // Aliases are case-insensitive, arguments are not + return command.substring(0, firstSep).toLowerCase(Locale.ENGLISH) + + command.substring(firstSep); + } else { + return command.toLowerCase(Locale.ENGLISH); + } + } + + // Parsing + + /** + * Returns the parsed alias, used to execute the command. + * + * @param nodes the list of parsed nodes, as returned by {@link CommandContext#getNodes()} or + * {@link CommandContextBuilder#getNodes()} + * @return the command alias + */ + public static String readAlias(final List> nodes) { + if (nodes.isEmpty()) { + throw new IllegalArgumentException("Cannot read alias from empty node list"); + } + return nodes.get(0).getNode().getName(); + } + + public static final String ARGS_NODE_NAME = "arguments"; + + /** + * Returns the parsed arguments that come after the command alias, or {@code fallback} if + * no arguments were provided. + * + * @param arguments the map of parsed arguments, as returned by + * {@link CommandContext#getArguments()} or {@link CommandContextBuilder#getArguments()} + * @param type the type class of the arguments + * @param fallback the value to return if no arguments were provided + * @param the type of the arguments + * @return the command arguments + */ + public static V readArguments(final Map> arguments, + final Class type, final V fallback) { + final ParsedArgument argument = arguments.get(ARGS_NODE_NAME); + if (argument == null) { + return fallback; // either no arguments were given or this isn't an InvocableCommand + } + final Object result = argument.getResult(); + try { + return type.cast(result); + } catch (final ClassCastException e) { + throw new IllegalArgumentException("Parsed argument is of type " + result.getClass() + + ", expected " + type, e); + } + } + + // Alias nodes + + /** + * Returns whether a literal node with the given name can be added to + * the {@link RootCommandNode} associated to a {@link CommandManager}. + * + *

This is an internal method and should not be used in user-facing + * methods. Instead, they should lowercase the given aliases themselves. + * + * @param alias the alias to check + * @return true if the alias can be registered; false otherwise + */ + public static boolean isValidAlias(final String alias) { + return alias.equals(alias.toLowerCase(Locale.ENGLISH)); + } + + /** + * Creates a copy of the given literal with the specified name. + * + * @param original the literal node to copy + * @param newName the name of the returned literal node + * @return a copy of the literal with the given name + */ + public static LiteralCommandNode shallowCopy( + final LiteralCommandNode original, final String newName) { + // Brigadier resolves the redirect of a node if further input can be parsed. + // Let be a literal node having a redirect to a literal. Then, + // the context returned by CommandDispatcher#parseNodes when given the input + // string " " does not contain a child context with as its root node. + // Thus, the vanilla client asks the children of for suggestions, instead + // of those of (https://github.com/Mojang/brigadier/issues/46). + // Perform a shallow copy of the literal instead. + Preconditions.checkNotNull(original, "original"); + Preconditions.checkNotNull(newName, "secondaryAlias"); + final LiteralArgumentBuilder builder = LiteralArgumentBuilder + .literal(newName) + .requires(original.getRequirement()) + .requiresWithContext(original.getContextRequirement()) + .forward(original.getRedirect(), original.getRedirectModifier(), original.isFork()) + .executes(original.getCommand()); + for (final CommandNode child : original.getChildren()) { + builder.then(child); + } + return builder.build(); + } + + // Arguments node + + /** + * Returns the arguments node for the command represented by the given alias node, + * if present; otherwise returns {@code null}. + * + * @param alias the alias node + * @param the type of the command source + * @return the arguments node, or null if not present + */ + static @Nullable VelocityArgumentCommandNode getArgumentsNode( + final LiteralCommandNode alias) { + final CommandNode node = alias.getChild(ARGS_NODE_NAME); + if (node instanceof VelocityArgumentCommandNode) { + return (VelocityArgumentCommandNode) node; + } + return null; + } + + /** + * Returns whether the given node is an arguments node. + * + * @param node the node to check + * @return true if the node is an arguments node; false otherwise + */ + public static boolean isArgumentsNode(final CommandNode node) { + return node instanceof VelocityArgumentCommandNode && node.getName().equals(ARGS_NODE_NAME); + } + + private VelocityCommands() { + throw new AssertionError(); + } +} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityRawCommandInvocation.java b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityRawCommandInvocation.java deleted file mode 100644 index 3e41c4436..000000000 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityRawCommandInvocation.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright (C) 2018 Velocity Contributors - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - -package com.velocitypowered.proxy.command; - -import com.google.common.base.Preconditions; -import com.mojang.brigadier.context.CommandContext; -import com.velocitypowered.api.command.CommandSource; -import com.velocitypowered.api.command.RawCommand; -import com.velocitypowered.proxy.util.BrigadierUtils; - -final class VelocityRawCommandInvocation extends AbstractCommandInvocation - implements RawCommand.Invocation { - - static final Factory FACTORY = new Factory(); - - static class Factory implements CommandInvocationFactory { - - @Override - public RawCommand.Invocation create(final CommandContext context) { - return new VelocityRawCommandInvocation( - context.getSource(), - BrigadierUtils.getAlias(context), - BrigadierUtils.getRawArguments(context)); - } - } - - private final String alias; - - private VelocityRawCommandInvocation(final CommandSource source, - final String alias, final String arguments) { - super(source, arguments); - this.alias = Preconditions.checkNotNull(alias); - } - - @Override - public String alias() { - return alias; - } -} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocitySimpleCommandInvocation.java b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocitySimpleCommandInvocation.java deleted file mode 100644 index 74d851c88..000000000 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocitySimpleCommandInvocation.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Copyright (C) 2018 Velocity Contributors - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - -package com.velocitypowered.proxy.command; - -import com.mojang.brigadier.context.CommandContext; -import com.velocitypowered.api.command.CommandSource; -import com.velocitypowered.api.command.SimpleCommand; -import com.velocitypowered.proxy.util.BrigadierUtils; - -final class VelocitySimpleCommandInvocation extends AbstractCommandInvocation - implements SimpleCommand.Invocation { - - static final Factory FACTORY = new Factory(); - - static class Factory implements CommandInvocationFactory { - - @Override - public SimpleCommand.Invocation create(final CommandContext context) { - final String[] arguments = BrigadierUtils.getSplitArguments(context); - final String alias = BrigadierUtils.getAlias(context); - return new VelocitySimpleCommandInvocation(context.getSource(), alias, arguments); - } - } - - private final String alias; - - VelocitySimpleCommandInvocation(final CommandSource source, final String alias, - final String[] arguments) { - super(source, arguments); - this.alias = alias; - } - - @Override - public String alias() { - return this.alias; - } -} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentType.java b/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentType.java new file mode 100644 index 000000000..4b482a909 --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentType.java @@ -0,0 +1,46 @@ +package com.velocitypowered.proxy.command.brigadier; + +import com.google.common.base.Splitter; +import com.mojang.brigadier.CommandDispatcher; +import com.mojang.brigadier.StringReader; +import com.mojang.brigadier.arguments.ArgumentType; +import com.mojang.brigadier.exceptions.CommandSyntaxException; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; + +/** + * An argument type that parses the remaining contents of a {@link StringReader}, + * splitting the input into words and placing the results in a string array. + */ +public final class StringArrayArgumentType implements ArgumentType { + + public static final StringArrayArgumentType INSTANCE = new StringArrayArgumentType(); + public static final String[] EMPTY = new String[0]; + + private static final Splitter WORD_SPLITTER = + Splitter.on(CommandDispatcher.ARGUMENT_SEPARATOR_CHAR); + private static final List EXAMPLES = Arrays.asList("word", "some words"); + + private StringArrayArgumentType() {} + + @Override + public String[] parse(final StringReader reader) throws CommandSyntaxException { + final String text = reader.getRemaining(); + reader.setCursor(reader.getTotalLength()); + if (text.isEmpty()) { + return EMPTY; + } + return WORD_SPLITTER.splitToList(text).toArray(EMPTY); + } + + @Override + public String toString() { + return "stringArray()"; + } + + @Override + public Collection getExamples() { + return EXAMPLES; + } +} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentBuilder.java b/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentBuilder.java new file mode 100644 index 000000000..3f1c1956a --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentBuilder.java @@ -0,0 +1,61 @@ +package com.velocitypowered.proxy.command.brigadier; + +import com.google.common.base.Preconditions; +import com.mojang.brigadier.arguments.ArgumentType; +import com.mojang.brigadier.builder.ArgumentBuilder; +import com.mojang.brigadier.suggestion.SuggestionProvider; +import com.mojang.brigadier.tree.CommandNode; +import org.checkerframework.checker.nullness.qual.Nullable; + +/** + * A builder for creating {@link VelocityArgumentCommandNode}s. + * + * @param the type of the command source + * @param the type of the argument to parse + */ +public final class VelocityArgumentBuilder + extends ArgumentBuilder> { + + public static VelocityArgumentBuilder velocityArgument(final String name, + final ArgumentType type) { + Preconditions.checkNotNull(name, "name"); + Preconditions.checkNotNull(type, "type"); + return new VelocityArgumentBuilder<>(name, type); + } + + private final String name; + private final ArgumentType type; + private SuggestionProvider suggestionsProvider = null; + + public VelocityArgumentBuilder(final String name, final ArgumentType type) { + this.name = name; + this.type = type; + } + + public VelocityArgumentBuilder suggests(final @Nullable SuggestionProvider provider) { + this.suggestionsProvider = provider; + return this; + } + + @Override + public VelocityArgumentBuilder then(final ArgumentBuilder argument) { + throw new UnsupportedOperationException("Cannot add children to a greedy node"); + } + + @Override + public VelocityArgumentBuilder then(final CommandNode argument) { + throw new UnsupportedOperationException("Cannot add children to a greedy node"); + } + + @Override + protected VelocityArgumentBuilder getThis() { + return this; + } + + @Override + public VelocityArgumentCommandNode build() { + return new VelocityArgumentCommandNode<>(this.name, this.type, getCommand(), getRequirement(), + getContextRequirement(), getRedirect(), getRedirectModifier(), isFork(), + this.suggestionsProvider); + } +} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNode.java b/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNode.java new file mode 100644 index 000000000..f8c0299c4 --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNode.java @@ -0,0 +1,121 @@ +package com.velocitypowered.proxy.command.brigadier; + +import com.google.common.base.Preconditions; +import com.mojang.brigadier.Command; +import com.mojang.brigadier.ImmutableStringReader; +import com.mojang.brigadier.RedirectModifier; +import com.mojang.brigadier.StringReader; +import com.mojang.brigadier.arguments.ArgumentType; +import com.mojang.brigadier.arguments.StringArgumentType; +import com.mojang.brigadier.builder.RequiredArgumentBuilder; +import com.mojang.brigadier.context.CommandContext; +import com.mojang.brigadier.context.CommandContextBuilder; +import com.mojang.brigadier.context.ParsedArgument; +import com.mojang.brigadier.exceptions.CommandSyntaxException; +import com.mojang.brigadier.suggestion.SuggestionProvider; +import com.mojang.brigadier.suggestion.Suggestions; +import com.mojang.brigadier.suggestion.SuggestionsBuilder; +import com.mojang.brigadier.tree.ArgumentCommandNode; +import com.mojang.brigadier.tree.CommandNode; +import java.util.Collection; +import java.util.concurrent.CompletableFuture; +import java.util.function.BiPredicate; +import java.util.function.Predicate; + +/** + * An argument node that uses the given (possibly custom) {@link ArgumentType} + * for parsing, while maintaining compatibility with the vanilla client. + * The argument type must be greedy and accept any input. + * + * @param the type of the command source + * @param the type of the argument to parse + */ +public class VelocityArgumentCommandNode extends ArgumentCommandNode { + + private final ArgumentType type; + + public VelocityArgumentCommandNode( + final String name, final ArgumentType type, final Command command, + final Predicate requirement, + final BiPredicate, ImmutableStringReader> contextRequirement, + final CommandNode redirect, final RedirectModifier modifier, final boolean forks, + final SuggestionProvider customSuggestions) { + super(name, StringArgumentType.greedyString(), command, requirement, contextRequirement, + redirect, modifier, forks, customSuggestions); + this.type = Preconditions.checkNotNull(type, "type"); + } + + @Override + public void parse(final StringReader reader, final CommandContextBuilder contextBuilder) + throws CommandSyntaxException { + // Same as super, except we use the rich ArgumentType + final int start = reader.getCursor(); + final T result = this.type.parse(reader); + if (reader.canRead()) { + throw CommandSyntaxException.BUILT_IN_EXCEPTIONS.dispatcherParseException() + .createWithContext(reader, "Expected greedy ArgumentType to parse all input"); + } + + final ParsedArgument parsed = new ParsedArgument<>(start, reader.getCursor(), result); + contextBuilder.withArgument(getName(), parsed); + contextBuilder.withNode(this, parsed.getRange()); + } + + @Override + public CompletableFuture listSuggestions( + final CommandContext context, final SuggestionsBuilder builder) + throws CommandSyntaxException { + if (getCustomSuggestions() == null) { + return Suggestions.empty(); + } + return getCustomSuggestions().getSuggestions(context, builder); + } + + @Override + public RequiredArgumentBuilder createBuilder() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isValidInput(final String input) { + return true; + } + + @Override + public void addChild(final CommandNode node) { + throw new UnsupportedOperationException("Cannot add children to a greedy node"); + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (!(o instanceof VelocityArgumentCommandNode)) { + return false; + } + if (!super.equals(o)) { + return false; + } + + final VelocityArgumentCommandNode that = (VelocityArgumentCommandNode) o; + return this.type.equals(that.type); + } + + @Override + public int hashCode() { + int result = super.hashCode(); + result = 31 * result + this.type.hashCode(); + return result; + } + + @Override + public Collection getExamples() { + return this.type.getExamples(); + } + + @Override + public String toString() { + return ""; + } +} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/AbstractCommandInvocation.java b/proxy/src/main/java/com/velocitypowered/proxy/command/invocation/AbstractCommandInvocation.java similarity index 69% rename from proxy/src/main/java/com/velocitypowered/proxy/command/AbstractCommandInvocation.java rename to proxy/src/main/java/com/velocitypowered/proxy/command/invocation/AbstractCommandInvocation.java index 87bd8e496..43fff5b07 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/AbstractCommandInvocation.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/invocation/AbstractCommandInvocation.java @@ -15,7 +15,7 @@ * along with this program. If not, see . */ -package com.velocitypowered.proxy.command; +package com.velocitypowered.proxy.command.invocation; import com.google.common.base.Preconditions; import com.velocitypowered.api.command.CommandInvocation; @@ -38,11 +38,35 @@ abstract class AbstractCommandInvocation implements CommandInvocation { @Override public CommandSource source() { - return source; + return this.source; } @Override public T arguments() { - return arguments; + return this.arguments; + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + final AbstractCommandInvocation that = (AbstractCommandInvocation) o; + + if (!this.source.equals(that.source)) { + return false; + } + return this.arguments.equals(that.arguments); + } + + @Override + public int hashCode() { + int result = this.source.hashCode(); + result = 31 * result + this.arguments.hashCode(); + return result; } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/invocation/CommandInvocationFactory.java b/proxy/src/main/java/com/velocitypowered/proxy/command/invocation/CommandInvocationFactory.java new file mode 100644 index 000000000..c462a01ce --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/invocation/CommandInvocationFactory.java @@ -0,0 +1,78 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package com.velocitypowered.proxy.command.invocation; + +import com.mojang.brigadier.context.CommandContext; +import com.mojang.brigadier.context.CommandContextBuilder; +import com.mojang.brigadier.context.ParsedArgument; +import com.mojang.brigadier.context.ParsedCommandNode; +import com.velocitypowered.api.command.CommandInvocation; +import com.velocitypowered.api.command.CommandSource; +import java.util.List; +import java.util.Map; + +/** + * Creates command invocation objects from a command context builder or + * a command context. + * + *

Let {@code builder} be a command context builder, and {@code context} + * a context returned by calling {@link CommandContextBuilder#build(String)} on + * {@code builder}. The invocations returned by {@link #create(CommandContext)} + * when given {@code context}, and {@link #create(CommandContextBuilder)} when + * given {@code builder} are equal. + * + * @param the type of the built invocation + */ +public interface CommandInvocationFactory> { + + /** + * Creates an invocation from the given command context. + * + * @param context the command context + * @return the built invocation context + */ + default I create(final CommandContext context) { + return this.create(context.getSource(), context.getNodes(), context.getArguments()); + } + + /** + * Creates an invocation from the given command context builder. + * + * @param context the command context builder + * @return the built invocation context + */ + default I create(final CommandContextBuilder context) { + return this.create(context.getSource(), context.getNodes(), context.getArguments()); + } + + /** + * Creates an invocation from the given parsed nodes and arguments. + * + * @param source the command source + * @param nodes the list of parsed nodes, as returned by {@link CommandContext#getNodes()} and + * {@link CommandContextBuilder#getNodes()} + * @param arguments the list of parsed arguments, as returned by + * {@link CommandContext#getArguments()} and {@link CommandContextBuilder#getArguments()} + * @return the built invocation context + */ + // This provides an abstraction over methods common to CommandContext and CommandContextBuilder. + // Annoyingly, they mostly have the same getters but one is (correctly) not a subclass of + // the other. Subclasses may override the methods above to obtain class-specific data. + I create(final CommandSource source, final List> nodes, + final Map> arguments); +} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/invocation/RawCommandInvocation.java b/proxy/src/main/java/com/velocitypowered/proxy/command/invocation/RawCommandInvocation.java new file mode 100644 index 000000000..2579a2e65 --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/invocation/RawCommandInvocation.java @@ -0,0 +1,90 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package com.velocitypowered.proxy.command.invocation; + +import com.google.common.base.Preconditions; +import com.mojang.brigadier.context.ParsedArgument; +import com.mojang.brigadier.context.ParsedCommandNode; +import com.velocitypowered.api.command.CommandSource; +import com.velocitypowered.api.command.RawCommand; +import com.velocitypowered.proxy.command.VelocityCommands; +import java.util.List; +import java.util.Map; + +public final class RawCommandInvocation extends AbstractCommandInvocation + implements RawCommand.Invocation { + + public static final Factory FACTORY = new Factory(); + + private static class Factory implements CommandInvocationFactory { + + @Override + public RawCommand.Invocation create( + final CommandSource source, final List> nodes, + final Map> arguments) { + final String alias = VelocityCommands.readAlias(nodes); + final String args = VelocityCommands.readArguments(arguments, String.class, ""); + return new RawCommandInvocation(source, alias, args); + } + } + + private final String alias; + + private RawCommandInvocation(final CommandSource source, + final String alias, final String arguments) { + super(source, arguments); + this.alias = Preconditions.checkNotNull(alias, "alias"); + } + + @Override + public String alias() { + return this.alias; + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + if (!super.equals(o)) { + return false; + } + + final RawCommandInvocation that = (RawCommandInvocation) o; + return this.alias.equals(that.alias); + } + + @Override + public int hashCode() { + int result = super.hashCode(); + result = 31 * result + this.alias.hashCode(); + return result; + } + + @Override + public String toString() { + return "RawCommandInvocation{" + + "source='" + this.source() + '\'' + + ", alias='" + this.alias + '\'' + + ", arguments='" + this.arguments() + '\'' + + '}'; + } +} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/invocation/SimpleCommandInvocation.java b/proxy/src/main/java/com/velocitypowered/proxy/command/invocation/SimpleCommandInvocation.java new file mode 100644 index 000000000..a6ec60611 --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/invocation/SimpleCommandInvocation.java @@ -0,0 +1,93 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package com.velocitypowered.proxy.command.invocation; + +import com.google.common.base.Preconditions; +import com.mojang.brigadier.context.ParsedArgument; +import com.mojang.brigadier.context.ParsedCommandNode; +import com.velocitypowered.api.command.CommandSource; +import com.velocitypowered.api.command.SimpleCommand; +import com.velocitypowered.proxy.command.VelocityCommands; +import com.velocitypowered.proxy.command.brigadier.StringArrayArgumentType; +import java.util.Arrays; +import java.util.List; +import java.util.Map; + +public final class SimpleCommandInvocation extends AbstractCommandInvocation + implements SimpleCommand.Invocation { + + public static final Factory FACTORY = new Factory(); + + private static class Factory implements CommandInvocationFactory { + + @Override + public SimpleCommand.Invocation create( + final CommandSource source, final List> nodes, + final Map> arguments) { + final String alias = VelocityCommands.readAlias(nodes); + final String[] args = VelocityCommands.readArguments( + arguments, String[].class, StringArrayArgumentType.EMPTY); + return new SimpleCommandInvocation(source, alias, args); + } + } + + private final String alias; + + SimpleCommandInvocation(final CommandSource source, final String alias, + final String[] arguments) { + super(source, arguments); + this.alias = Preconditions.checkNotNull(alias, "alias"); + } + + @Override + public String alias() { + return this.alias; + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + if (!super.equals(o)) { + return false; + } + + final SimpleCommandInvocation that = (SimpleCommandInvocation) o; + return this.alias.equals(that.alias); + } + + @Override + public int hashCode() { + int result = super.hashCode(); + result = 31 * result + this.alias.hashCode(); + return result; + } + + @Override + public String toString() { + return "SimpleCommandInvocation{" + + "source='" + this.source() + '\'' + + ", alias='" + this.alias + '\'' + + ", arguments='" + Arrays.toString(this.arguments()) + '\'' + + '}'; + } +} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/AbstractCommandRegistrar.java b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/AbstractCommandRegistrar.java new file mode 100644 index 000000000..c4a767be1 --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/AbstractCommandRegistrar.java @@ -0,0 +1,44 @@ +package com.velocitypowered.proxy.command.registrar; + +import com.google.common.base.Preconditions; +import com.mojang.brigadier.tree.LiteralCommandNode; +import com.mojang.brigadier.tree.RootCommandNode; +import com.velocitypowered.api.command.Command; +import com.velocitypowered.api.command.CommandSource; +import com.velocitypowered.proxy.command.VelocityCommands; +import java.util.concurrent.locks.Lock; +import org.checkerframework.checker.lock.qual.GuardedBy; + +/** + * Base class for {@link CommandRegistrar} implementations. + * + * @param the type of the command to register + */ +abstract class AbstractCommandRegistrar implements CommandRegistrar { + + private final @GuardedBy("lock") RootCommandNode root; + private final Lock lock; + + protected AbstractCommandRegistrar(final RootCommandNode root, final Lock lock) { + this.root = Preconditions.checkNotNull(root, "root"); + this.lock = Preconditions.checkNotNull(lock, "lock"); + } + + protected void register(final LiteralCommandNode node) { + lock.lock(); + try { + // Registration overrides previous aliased command + root.removeChildByName(node.getName()); + root.addChild(node); + } finally { + lock.unlock(); + } + } + + protected void register(final LiteralCommandNode node, + final String secondaryAlias) { + final LiteralCommandNode copy = + VelocityCommands.shallowCopy(node, secondaryAlias); + this.register(copy); + } +} 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 new file mode 100644 index 000000000..66ce113a2 --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/BrigadierCommandRegistrar.java @@ -0,0 +1,46 @@ +package com.velocitypowered.proxy.command.registrar; + +import com.mojang.brigadier.tree.LiteralCommandNode; +import com.mojang.brigadier.tree.RootCommandNode; +import com.velocitypowered.api.command.BrigadierCommand; +import com.velocitypowered.api.command.CommandMeta; +import com.velocitypowered.api.command.CommandSource; +import com.velocitypowered.proxy.command.VelocityCommands; +import java.util.concurrent.locks.Lock; + +/** + * Registers {@link BrigadierCommand}s in a root node. + */ +public final class BrigadierCommandRegistrar extends AbstractCommandRegistrar { + + public BrigadierCommandRegistrar(final RootCommandNode root, final Lock lock) { + super(root, lock); + } + + @Override + public void register(final BrigadierCommand command, final CommandMeta meta) { + // 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. + final LiteralCommandNode literal = command.getNode(); + final String primaryAlias = literal.getName(); + if (VelocityCommands.isValidAlias(primaryAlias)) { + // Register directly without copying + this.register(literal); + } + + for (final String alias : meta.getAliases()) { + if (primaryAlias.equals(alias)) { + continue; + } + this.register(literal, alias); + } + + // Brigadier commands don't support hinting, ignore + } + + @Override + public Class registrableSuperInterface() { + return BrigadierCommand.class; + } +} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/CommandRegistrar.java b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/CommandRegistrar.java new file mode 100644 index 000000000..ea9200e0d --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/CommandRegistrar.java @@ -0,0 +1,33 @@ +package com.velocitypowered.proxy.command.registrar; + +import com.mojang.brigadier.tree.LiteralCommandNode; +import com.mojang.brigadier.tree.RootCommandNode; +import com.velocitypowered.api.command.Command; +import com.velocitypowered.api.command.CommandMeta; + +/** + * Creates and registers the {@link LiteralCommandNode} representations of + * a given {@link Command} in a {@link RootCommandNode}. + * + * @param the type of the command to register + */ +public interface CommandRegistrar { + + /** + * Registers the given command. + * + * @param command the command to register + * @param meta the command metadata, including the case-insensitive aliases + * @throws IllegalArgumentException if the given command cannot be registered + */ + void register(final T command, final CommandMeta meta); + + /** + * Returns the superclass or superinterface of all {@link Command} classes + * compatible with this registrar. Note that {@link #register(Command, CommandMeta)} + * may impose additional restrictions on individual {@link Command} instances. + * + * @return the superclass of all the classes compatible with this registrar + */ + Class registrableSuperInterface(); +} 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 new file mode 100644 index 000000000..fd7ba0274 --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/InvocableCommandRegistrar.java @@ -0,0 +1,105 @@ +package com.velocitypowered.proxy.command.registrar; + +import com.google.common.base.Preconditions; +import com.mojang.brigadier.Command; +import com.mojang.brigadier.arguments.ArgumentType; +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import com.mojang.brigadier.context.CommandContextBuilder; +import com.mojang.brigadier.tree.ArgumentCommandNode; +import com.mojang.brigadier.tree.LiteralCommandNode; +import com.mojang.brigadier.tree.RootCommandNode; +import com.velocitypowered.api.command.CommandInvocation; +import com.velocitypowered.api.command.CommandMeta; +import com.velocitypowered.api.command.CommandSource; +import com.velocitypowered.api.command.InvocableCommand; +import com.velocitypowered.proxy.command.VelocityCommandMeta; +import com.velocitypowered.proxy.command.VelocityCommands; +import com.velocitypowered.proxy.command.brigadier.VelocityArgumentBuilder; +import com.velocitypowered.proxy.command.invocation.CommandInvocationFactory; +import java.util.Iterator; +import java.util.concurrent.locks.Lock; +import java.util.function.Predicate; + +/** + * Base class for {@link CommandRegistrar}s capable of registering a subinterface of + * {@link InvocableCommand} in a root node. + */ +abstract class InvocableCommandRegistrar, + I extends CommandInvocation, A> extends AbstractCommandRegistrar { + + private final CommandInvocationFactory invocationFactory; + private final ArgumentType argumentsType; + + protected InvocableCommandRegistrar(final RootCommandNode root, final Lock lock, + final CommandInvocationFactory invocationFactory, + final ArgumentType argumentsType) { + super(root, lock); + this.invocationFactory = Preconditions.checkNotNull(invocationFactory, "invocationFactory"); + this.argumentsType = Preconditions.checkNotNull(argumentsType, "argumentsType"); + } + + @Override + public void register(final T command, final CommandMeta meta) { + final Iterator aliases = meta.getAliases().iterator(); + + final String primaryAlias = aliases.next(); + final LiteralCommandNode literal = + this.createLiteral(command, meta, primaryAlias); + this.register(literal); + + while (aliases.hasNext()) { + final String alias = aliases.next(); + this.register(literal, alias); + } + } + + private LiteralCommandNode createLiteral(final T command, final CommandMeta meta, + final String alias) { + final Predicate> requirement = context -> { + final I invocation = invocationFactory.create(context); + return command.hasPermission(invocation); + }; + final Command callback = context -> { + final I invocation = invocationFactory.create(context); + command.execute(invocation); + return 1; // handled + }; + + final LiteralCommandNode literal = LiteralArgumentBuilder + .literal(alias) + .requiresWithContext((context, reader) -> { + if (reader.canRead()) { + // InvocableCommands do not follow a tree-like permissions checking structure. + // Thus, a CommandSource may be able to execute a command with arguments while + // not being able to execute the argument-less variant. + // Only check for permissions once parsing is complete. + return true; + } + return requirement.test(context); + }) + .executes(callback) + .build(); + + final ArgumentCommandNode arguments = VelocityArgumentBuilder + .velocityArgument(VelocityCommands.ARGS_NODE_NAME, argumentsType) + .requiresWithContext((context, reader) -> requirement.test(context)) + .executes(callback) + .suggests((context, builder) -> { + final I invocation = invocationFactory.create(context); + return command.suggestAsync(invocation).thenApply(suggestions -> { + for (String value : suggestions) { + Preconditions.checkNotNull(value, "suggestion"); + builder.suggest(value); + } + return builder.build(); + }); + }) + .build(); + literal.addChild(arguments); + + // Add hinting nodes + VelocityCommandMeta.copyHints(meta).forEach(literal::addChild); + + return literal; + } +} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/RawCommandRegistrar.java b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/RawCommandRegistrar.java new file mode 100644 index 000000000..25e6acbdd --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/RawCommandRegistrar.java @@ -0,0 +1,24 @@ +package com.velocitypowered.proxy.command.registrar; + +import com.mojang.brigadier.arguments.StringArgumentType; +import com.mojang.brigadier.tree.RootCommandNode; +import com.velocitypowered.api.command.CommandSource; +import com.velocitypowered.api.command.RawCommand; +import com.velocitypowered.proxy.command.invocation.RawCommandInvocation; +import java.util.concurrent.locks.Lock; + +/** + * Registers {@link RawCommand}s in a root node. + */ +public final class RawCommandRegistrar + extends InvocableCommandRegistrar { + + public RawCommandRegistrar(final RootCommandNode root, final Lock lock) { + super(root, lock, RawCommandInvocation.FACTORY, StringArgumentType.greedyString()); + } + + @Override + public Class registrableSuperInterface() { + return RawCommand.class; + } +} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/SimpleCommandRegistrar.java b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/SimpleCommandRegistrar.java new file mode 100644 index 000000000..b0b15857d --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/SimpleCommandRegistrar.java @@ -0,0 +1,24 @@ +package com.velocitypowered.proxy.command.registrar; + +import com.mojang.brigadier.tree.RootCommandNode; +import com.velocitypowered.api.command.CommandSource; +import com.velocitypowered.api.command.SimpleCommand; +import com.velocitypowered.proxy.command.brigadier.StringArrayArgumentType; +import com.velocitypowered.proxy.command.invocation.SimpleCommandInvocation; +import java.util.concurrent.locks.Lock; + +/** + * Registers {@link SimpleCommand}s in a root node. + */ +public final class SimpleCommandRegistrar + extends InvocableCommandRegistrar { + + public SimpleCommandRegistrar(final RootCommandNode root, final Lock lock) { + super(root, lock, SimpleCommandInvocation.FACTORY, StringArrayArgumentType.INSTANCE); + } + + @Override + public Class registrableSuperInterface() { + return SimpleCommand.class; + } +} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java index b3c7133cf..fcb44c7fc 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java @@ -29,6 +29,7 @@ import com.velocitypowered.api.event.connection.PluginMessageEvent; import com.velocitypowered.api.network.ProtocolVersion; import com.velocitypowered.api.proxy.messages.ChannelIdentifier; import com.velocitypowered.proxy.VelocityServer; +import com.velocitypowered.proxy.command.CommandGraphInjector; import com.velocitypowered.proxy.connection.MinecraftConnection; import com.velocitypowered.proxy.connection.MinecraftSessionHandler; import com.velocitypowered.proxy.connection.client.ClientPlaySessionHandler; @@ -201,18 +202,8 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { RootCommandNode rootNode = commands.getRootNode(); if (server.getConfiguration().isAnnounceProxyCommands()) { // Inject commands from the proxy. - RootCommandNode dispatcherRootNode = - (RootCommandNode) - filterNode(server.getCommandManager().getDispatcher().getRoot()); - assert dispatcherRootNode != null : "Filtering root node returned null."; - Collection> proxyNodes = dispatcherRootNode.getChildren(); - for (CommandNode node : proxyNodes) { - CommandNode existingServerChild = rootNode.getChild(node.getName()); - if (existingServerChild != null) { - rootNode.getChildren().remove(existingServerChild); - } - rootNode.addChild(node); - } + final CommandGraphInjector injector = server.getCommandManager().getInjector(); + injector.inject(rootNode, serverConn.getPlayer()); } server.getEventManager().fire( @@ -225,50 +216,6 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { return true; } - /** - * Creates a deep copy of the provided command node, but removes any node that are not accessible - * by the player (respecting the requirement of the node). - * - * @param source source node - * @return filtered node - */ - private CommandNode filterNode(CommandNode source) { - CommandNode dest; - if (source instanceof RootCommandNode) { - dest = new RootCommandNode<>(); - } else { - if (source.getRequirement() != null) { - try { - if (!source.getRequirement().test(serverConn.getPlayer())) { - return null; - } - } catch (Throwable e) { - // swallow everything because plugins - logger.error( - "Requirement test for command node " + source + " encountered an exception", e); - } - } - - ArgumentBuilder destChildBuilder = source.createBuilder(); - destChildBuilder.requires((commandSource) -> true); - if (destChildBuilder.getRedirect() != null) { - destChildBuilder.redirect(filterNode(destChildBuilder.getRedirect())); - } - - dest = destChildBuilder.build(); - } - - for (CommandNode sourceChild : source.getChildren()) { - CommandNode destChild = filterNode(sourceChild); - if (destChild == null) { - continue; - } - dest.addChild(destChild); - } - - return dest; - } - @Override public void handleGeneric(MinecraftPacket packet) { if (packet instanceof PluginMessage) { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/util/BrigadierUtils.java b/proxy/src/main/java/com/velocitypowered/proxy/util/BrigadierUtils.java deleted file mode 100644 index db6a5919d..000000000 --- a/proxy/src/main/java/com/velocitypowered/proxy/util/BrigadierUtils.java +++ /dev/null @@ -1,169 +0,0 @@ -/* - * Copyright (C) 2018 Velocity Contributors - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - -package com.velocitypowered.proxy.util; - -import com.google.common.base.Preconditions; -import com.google.common.base.Splitter; -import com.mojang.brigadier.Command; -import com.mojang.brigadier.arguments.StringArgumentType; -import com.mojang.brigadier.builder.ArgumentBuilder; -import com.mojang.brigadier.builder.LiteralArgumentBuilder; -import com.mojang.brigadier.builder.RequiredArgumentBuilder; -import com.mojang.brigadier.context.CommandContext; -import com.mojang.brigadier.suggestion.SuggestionProvider; -import com.mojang.brigadier.tree.CommandNode; -import com.mojang.brigadier.tree.LiteralCommandNode; -import com.velocitypowered.api.command.CommandSource; -import java.util.Locale; -import org.checkerframework.checker.nullness.qual.Nullable; - -/** - * Provides utilities for working with Brigadier commands. - */ -public final class BrigadierUtils { - - private static final Splitter SPACE_SPLITTER = Splitter.on(' '); - - /** - * Returns a literal node that redirects its execution to - * the given destination node. - * - * @param alias the command alias - * @param destination the destination node - * @return the built node - */ - public static LiteralCommandNode buildRedirect( - final String alias, final LiteralCommandNode destination) { - // Redirects only work for nodes with children, but break the top argument-less command. - // Manually adding the root command after setting the redirect doesn't fix it. - // See https://github.com/Mojang/brigadier/issues/46). Manually clone the node instead. - LiteralArgumentBuilder builder = LiteralArgumentBuilder - .literal(alias.toLowerCase(Locale.ENGLISH)) - .requires(destination.getRequirement()) - .forward( - destination.getRedirect(), destination.getRedirectModifier(), destination.isFork()) - .executes(destination.getCommand()); - for (CommandNode child : destination.getChildren()) { - builder.then(child); - } - return builder.build(); - } - - /** - * Returns a literal node that optionally accepts arguments - * as a raw {@link String}. - * - * @param alias the literal alias - * @param brigadierCommand the command to execute - * @param suggestionProvider the suggestion provider - * @return the built node - */ - public static LiteralCommandNode buildRawArgumentsLiteral( - final String alias, final Command brigadierCommand, - SuggestionProvider suggestionProvider) { - return LiteralArgumentBuilder - .literal(alias.toLowerCase(Locale.ENGLISH)) - .then(RequiredArgumentBuilder - .argument("arguments", StringArgumentType.greedyString()) - .suggests(suggestionProvider) - .executes(brigadierCommand)) - .executes(brigadierCommand) - .build(); - } - - /** - * Returns the used command alias. - * - * @param context the command context - * @return the parsed command alias - */ - public static String getAlias(final CommandContext context) { - return context.getNodes().get(0).getNode().getName(); - } - - /** - * Returns the raw {@link String} arguments of a command execution. - * - * @param context the command context - * @return the parsed arguments - */ - public static String getRawArguments(final CommandContext context) { - String cmdLine = context.getInput(); - int firstSpace = cmdLine.indexOf(' '); - if (firstSpace == -1) { - return ""; - } - return cmdLine.substring(firstSpace + 1); - } - - /** - * Returns the splitted arguments of a command node built with - * {@link #buildRawArgumentsLiteral(String, Command, SuggestionProvider)}. - * - * @param context the command context - * @return the parsed arguments - */ - public static String[] getSplitArguments(final CommandContext context) { - String line = getRawArguments(context); - if (line.isEmpty()) { - return new String[0]; - } - return SPACE_SPLITTER.splitToList(line).toArray(new String[0]); - } - - /** - * Returns the normalized representation of the given command input. - * - * @param cmdLine the command input - * @param trim whether to trim argument-less inputs - * @return the normalized command - */ - public static String normalizeInput(final String cmdLine, final boolean trim) { - // Command aliases are case insensitive, but Brigadier isn't - String command = trim ? cmdLine.trim() : cmdLine; - int firstSpace = command.indexOf(' '); - if (firstSpace != -1) { - return command.substring(0, firstSpace).toLowerCase(Locale.ENGLISH) - + command.substring(firstSpace); - } - return command.toLowerCase(Locale.ENGLISH); - } - - /** - * Prepares the given command node prior for hinting metadata to - * a {@link com.velocitypowered.api.command.Command}. - * - * @param node the command node to be wrapped - * @param command the command to execute - * @return the wrapped command node - */ - public static CommandNode wrapForHinting( - final CommandNode node, final @Nullable Command command) { - Preconditions.checkNotNull(node, "node"); - ArgumentBuilder builder = node.createBuilder(); - builder.executes(command); - for (CommandNode child : node.getChildren()) { - builder.then(wrapForHinting(child, command)); - } - return builder.build(); - } - - private BrigadierUtils() { - throw new AssertionError(); - } -} diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java new file mode 100644 index 000000000..e843cf541 --- /dev/null +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java @@ -0,0 +1,53 @@ +package com.velocitypowered.proxy.command; + +import com.mojang.brigadier.CommandDispatcher; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class CommandGraphInjectorTests { + + private CommandDispatcher dispatcher; + private Lock lock; + private CommandGraphInjector injector; + + @BeforeEach + void setUp() { + this.dispatcher = new CommandDispatcher<>(); + this.lock = new ReentrantLock(); + this.injector = new CommandGraphInjector<>(this.dispatcher, this.lock); + } + + // TODO + + @Test + void testInjectInvocableCommand() { + // Preserves arguments node and hints + } + + @Test + void testFiltersImpermissibleAlias() { + + } + + @Test + void testInjectsBrigadierCommand() { + + } + + @Test + void testFiltersImpermissibleBrigadierCommandChildren() { + + } + + @Test + void testInjectFiltersBrigadierCommandRedirects() { + + } + + @Test + void testInjectOverridesAliasInDestination() { + + } +} diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java new file mode 100644 index 000000000..b3d37c24e --- /dev/null +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java @@ -0,0 +1,5 @@ +package com.velocitypowered.proxy.command; + +public class SuggestionsProviderTests { + // TODO +} diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentTypeTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentTypeTests.java new file mode 100644 index 000000000..64769b1ad --- /dev/null +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentTypeTests.java @@ -0,0 +1,94 @@ +package com.velocitypowered.proxy.command.brigadier; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; + +import com.mojang.brigadier.StringReader; +import com.mojang.brigadier.exceptions.CommandSyntaxException; +import org.junit.jupiter.api.Test; + +public class StringArrayArgumentTypeTests { + + private static final StringArrayArgumentType TYPE = StringArrayArgumentType.INSTANCE; + + @Test + void testEmptyString() throws CommandSyntaxException { + final StringReader reader = new StringReader(""); + assertArrayEquals(new String[0], TYPE.parse(reader)); + } + + @Test + void testParseWord() throws CommandSyntaxException { + final StringReader reader = new StringReader("Hello"); + assertArrayEquals(new String[] { "Hello" }, TYPE.parse(reader)); + assertFalse(reader.canRead()); + } + + @Test + void testParseString() throws CommandSyntaxException { + final StringReader reader = new StringReader("Hello world!"); + assertArrayEquals(new String[] { "Hello", "world!" }, TYPE.parse(reader)); + assertFalse(reader.canRead()); + } + + @Test + void testNoEscaping() throws CommandSyntaxException { + final StringReader reader = new StringReader("\"My house\" is blue"); + assertArrayEquals(new String[] { "\"My", "house\"", "is", "blue" }, TYPE.parse(reader)); + assertFalse(reader.canRead()); + } + + @Test + void testUnbalancedEscapingIsIgnored() throws CommandSyntaxException { + final StringReader reader = new StringReader("This is a \"sentence"); + assertArrayEquals(new String[] { "This", "is", "a", "\"sentence" }, TYPE.parse(reader)); + assertFalse(reader.canRead()); + } + + @Test + void testLeadingWhitespace() throws CommandSyntaxException { + final StringReader reader = new StringReader(" ¡Hola!"); + assertArrayEquals(new String[] { "", "¡Hola!" }, TYPE.parse(reader)); + assertFalse(reader.canRead()); + } + + @Test + void testMultipleLeadingWhitespace() throws CommandSyntaxException { + final StringReader reader = new StringReader(" Anguish Languish"); + assertArrayEquals(new String[] { "", "", "", "Anguish", "Languish" }, TYPE.parse(reader)); + assertFalse(reader.canRead()); + } + + @Test + void testTrailingWhitespace() throws CommandSyntaxException { + final StringReader reader = new StringReader("This is a test. "); + assertArrayEquals(new String[] { "This", "is", "a", "test.", "" }, TYPE.parse(reader)); + assertFalse(reader.canRead()); + } + + @Test + void testMultipleTrailingWhitespace() throws CommandSyntaxException { + final StringReader reader = new StringReader("Lorem ipsum "); + assertArrayEquals(new String[] { "Lorem", "ipsum", "", "" }, TYPE.parse(reader)); + assertFalse(reader.canRead()); + } + + @Test + void testMultipleWhitespaceCharsArePreserved() throws CommandSyntaxException { + final StringReader reader = new StringReader( + " This is a message that shouldn't be normalized "); + assertArrayEquals(new String[] { + "", "This", "", "is", "a", "", "", "message", "", "that", "shouldn't", "", "", "", "be", + "normalized", "", ""}, TYPE.parse(reader)); + assertFalse(reader.canRead()); + } + + @Test + void testRespectsCursor() throws CommandSyntaxException { + final StringReader reader = new StringReader("Hello beautiful world"); + reader.setCursor(6); + + assertArrayEquals(new String[] { "beautiful", "world"}, TYPE.parse(reader)); + assertFalse(reader.canRead()); + } +} diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNodeTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNodeTests.java new file mode 100644 index 000000000..dfbf20374 --- /dev/null +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNodeTests.java @@ -0,0 +1,81 @@ +package com.velocitypowered.proxy.command.brigadier; + +import static com.velocitypowered.proxy.command.brigadier.VelocityArgumentBuilder.velocityArgument; +import static org.junit.jupiter.api.Assertions.*; + +import com.mojang.brigadier.CommandDispatcher; +import com.mojang.brigadier.StringReader; +import com.mojang.brigadier.context.CommandContextBuilder; +import com.mojang.brigadier.context.ParsedArgument; +import com.mojang.brigadier.context.StringRange; +import com.mojang.brigadier.exceptions.CommandSyntaxException; +import com.mojang.brigadier.suggestion.Suggestions; +import com.mojang.brigadier.suggestion.SuggestionsBuilder; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +@SuppressWarnings("unchecked") +public class VelocityArgumentCommandNodeTests { + + private static final StringArrayArgumentType STRING_ARRAY = StringArrayArgumentType.INSTANCE; + + private CommandContextBuilder contextBuilder; + + @BeforeEach + void setUp() { + final CommandDispatcher dispatcher = new CommandDispatcher<>(); + this.contextBuilder = new CommandContextBuilder<>(dispatcher, new Object(), + dispatcher.getRoot(), 0); + } + + @Test + void testParse() throws CommandSyntaxException { + final VelocityArgumentCommandNode node = + velocityArgument("foo", STRING_ARRAY).build(); + final StringReader reader = new StringReader("hello world"); + node.parse(reader, this.contextBuilder); + + final StringRange expectedRange = StringRange.between(0, reader.getTotalLength()); + + assertFalse(reader.canRead()); + + assertFalse(this.contextBuilder.getNodes().isEmpty()); + assertSame(node, this.contextBuilder.getNodes().get(0).getNode()); + assertEquals(expectedRange, this.contextBuilder.getNodes().get(0).getRange()); + + assertTrue(this.contextBuilder.getArguments().containsKey("foo")); + final ParsedArgument parsed = + (ParsedArgument) this.contextBuilder.getArguments().get("foo"); + assertArrayEquals(new String[] { "hello", "world" }, parsed.getResult()); + assertEquals(expectedRange, parsed.getRange()); + } + + @Test + void testDefaultSuggestions() throws CommandSyntaxException { + final VelocityArgumentCommandNode node = + velocityArgument("foo", STRING_ARRAY).build(); + final Suggestions result = node.listSuggestions( + this.contextBuilder.build(""), new SuggestionsBuilder("", 0)).join(); + + assertTrue(result.isEmpty()); + } + + // This only tests delegation to the given SuggestionsProvider; suggestions merging + // and filtering is already tested in Brigadier. + @Test + void testCustomSuggestions() throws CommandSyntaxException { + final VelocityArgumentCommandNode node = + velocityArgument("foo", STRING_ARRAY) + .suggests((context, builder) -> { + builder.suggest("bar"); + builder.suggest("baz"); + return builder.buildFuture(); + }) + .build(); + final Suggestions result = node.listSuggestions( + this.contextBuilder.build(""), new SuggestionsBuilder("", 0)).join(); + + assertEquals("bar", result.getList().get(0).getText()); + assertEquals("baz", result.getList().get(1).getText()); + } +} From ff506dfdf851f4b5c1a3ed0ccf8b5493fa14dc66 Mon Sep 17 00:00:00 2001 From: Hugo Manrique Date: Sat, 5 Jun 2021 23:20:01 +0200 Subject: [PATCH 03/13] Add license headers --- .../proxy/command/CommandGraphInjector.java | 17 +++++++++++++++++ .../proxy/command/VelocityCommands.java | 17 +++++++++++++++++ .../brigadier/StringArrayArgumentType.java | 17 +++++++++++++++++ .../brigadier/VelocityArgumentBuilder.java | 17 +++++++++++++++++ .../brigadier/VelocityArgumentCommandNode.java | 17 +++++++++++++++++ .../registrar/AbstractCommandRegistrar.java | 17 +++++++++++++++++ .../registrar/BrigadierCommandRegistrar.java | 17 +++++++++++++++++ .../command/registrar/CommandRegistrar.java | 17 +++++++++++++++++ .../registrar/InvocableCommandRegistrar.java | 17 +++++++++++++++++ .../command/registrar/RawCommandRegistrar.java | 17 +++++++++++++++++ .../registrar/SimpleCommandRegistrar.java | 17 +++++++++++++++++ .../command/CommandGraphInjectorTests.java | 17 +++++++++++++++++ .../proxy/command/SuggestionsProviderTests.java | 17 +++++++++++++++++ .../brigadier/StringArrayArgumentTypeTests.java | 17 +++++++++++++++++ .../VelocityArgumentCommandNodeTests.java | 17 +++++++++++++++++ 15 files changed, 255 insertions(+) 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 8ea2d5775..7e2e55a9e 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/CommandGraphInjector.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/CommandGraphInjector.java @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command; import com.google.common.base.Preconditions; diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommands.java b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommands.java index 9d903aa40..262812f22 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommands.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommands.java @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command; import com.google.common.base.Preconditions; diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentType.java b/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentType.java index 4b482a909..eefeec46e 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentType.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentType.java @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command.brigadier; import com.google.common.base.Splitter; diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentBuilder.java b/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentBuilder.java index 3f1c1956a..d3f2506de 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentBuilder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentBuilder.java @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command.brigadier; import com.google.common.base.Preconditions; diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNode.java b/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNode.java index f8c0299c4..262621931 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNode.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNode.java @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command.brigadier; import com.google.common.base.Preconditions; diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/AbstractCommandRegistrar.java b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/AbstractCommandRegistrar.java index c4a767be1..c5fa6f46c 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/AbstractCommandRegistrar.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/AbstractCommandRegistrar.java @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command.registrar; import com.google.common.base.Preconditions; 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 66ce113a2..0739f2b76 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 @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command.registrar; import com.mojang.brigadier.tree.LiteralCommandNode; diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/CommandRegistrar.java b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/CommandRegistrar.java index ea9200e0d..b2d9051f9 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/CommandRegistrar.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/CommandRegistrar.java @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command.registrar; import com.mojang.brigadier.tree.LiteralCommandNode; 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 fd7ba0274..423cf3cc3 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 @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command.registrar; import com.google.common.base.Preconditions; diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/RawCommandRegistrar.java b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/RawCommandRegistrar.java index 25e6acbdd..2c1e8f109 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/RawCommandRegistrar.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/RawCommandRegistrar.java @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command.registrar; import com.mojang.brigadier.arguments.StringArgumentType; diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/SimpleCommandRegistrar.java b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/SimpleCommandRegistrar.java index b0b15857d..bfd4ef430 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/SimpleCommandRegistrar.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/registrar/SimpleCommandRegistrar.java @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command.registrar; import com.mojang.brigadier.tree.RootCommandNode; 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 e843cf541..d5c27c5db 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command; import com.mojang.brigadier.CommandDispatcher; diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java index b3d37c24e..bc4b9e10d 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command; public class SuggestionsProviderTests { diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentTypeTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentTypeTests.java index 64769b1ad..d33d4a16b 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentTypeTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentTypeTests.java @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command.brigadier; import static org.junit.jupiter.api.Assertions.assertArrayEquals; diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNodeTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNodeTests.java index dfbf20374..b61d06531 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNodeTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNodeTests.java @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command.brigadier; import static com.velocitypowered.proxy.command.brigadier.VelocityArgumentBuilder.velocityArgument; From d429d8383d4e1456a1172be92920cd3b48b23069 Mon Sep 17 00:00:00 2001 From: Hugo Manrique Date: Mon, 7 Jun 2021 14:13:32 +0200 Subject: [PATCH 04/13] Test execution and injection --- .../proxy/command/CommandGraphInjector.java | 2 + .../proxy/command/VelocityCommandManager.java | 3 +- .../registrar/BrigadierCommandRegistrar.java | 2 +- .../command/registrar/CommandRegistrar.java | 6 +- .../registrar/InvocableCommandRegistrar.java | 2 +- .../proxy/command/BrigadierCommandTests.java | 124 +++++++++++++++ .../command/CommandGraphInjectorTests.java | 144 ++++++++++++++++-- .../proxy/command/CommandTestSuite.java | 33 ++++ ...Tests.java => OldCommandManagerTests.java} | 5 +- .../proxy/command/RawCommandTests.java | 93 +++++++++++ .../proxy/command/SimpleCommandTests.java | 94 ++++++++++++ 11 files changed, 487 insertions(+), 21 deletions(-) create mode 100644 proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java create mode 100644 proxy/src/test/java/com/velocitypowered/proxy/command/CommandTestSuite.java rename proxy/src/test/java/com/velocitypowered/proxy/command/{CommandManagerTests.java => OldCommandManagerTests.java} (99%) create mode 100644 proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java create mode 100644 proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java 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()); + } +} From ea716c15c6300ecda26d90214ed247bb619cb023 Mon Sep 17 00:00:00 2001 From: Hugo Manrique Date: Mon, 7 Jun 2021 14:36:41 +0200 Subject: [PATCH 05/13] Test CommandManager --- .../proxy/command/VelocityCommandManager.java | 7 +- .../command/CommandGraphInjectorTests.java | 4 +- .../proxy/command/CommandManagerTests.java | 179 ++++++++++++++++++ .../proxy/command/CommandTestSuite.java | 2 +- .../proxy/command/OldCommandManagerTests.java | 16 +- 5 files changed, 188 insertions(+), 20 deletions(-) create mode 100644 proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java 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 5d4665a45..6332c51f1 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java @@ -46,6 +46,7 @@ import net.kyori.adventure.identity.Identity; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.format.NamedTextColor; import org.checkerframework.checker.lock.qual.GuardedBy; +import org.jetbrains.annotations.VisibleForTesting; public class VelocityCommandManager implements CommandManager { @@ -238,9 +239,9 @@ public class VelocityCommandManager implements CommandManager { return dispatcher.getRoot().getChild(alias.toLowerCase(Locale.ENGLISH)) != null; } - public CommandDispatcher getDispatcher() { - // TODO Can we remove this? This is only used by tests, and constitutes unsafe publication. - return dispatcher; + @VisibleForTesting // this constitutes unsafe publication + RootCommandNode getRoot() { + return dispatcher.getRoot(); } public CommandGraphInjector getInjector() { 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 b1471009e..d0c3a9414 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java @@ -45,7 +45,7 @@ public class CommandGraphInjectorTests { @BeforeEach void setUp() { - this.manager = new VelocityCommandManager(OldCommandManagerTests.EVENT_MANAGER); + this.manager = CommandManagerTests.newManager(); this.dest = new RootCommandNode<>(); } @@ -56,7 +56,7 @@ public class CommandGraphInjectorTests { manager.getInjector().inject(dest, SOURCE); // Preserves alias and arguments node - final var expected = manager.getDispatcher().getRoot(); + final var expected = manager.getRoot(); assertEquals(expected, dest); } diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java new file mode 100644 index 000000000..0d6570de4 --- /dev/null +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java @@ -0,0 +1,179 @@ +package com.velocitypowered.proxy.command; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import com.velocitypowered.api.command.BrigadierCommand; +import com.velocitypowered.api.command.CommandSource; +import com.velocitypowered.api.command.RawCommand; +import com.velocitypowered.api.command.SimpleCommand; +import com.velocitypowered.proxy.event.MockEventManager; +import com.velocitypowered.proxy.event.VelocityEventManager; +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class CommandManagerTests { + + static final VelocityEventManager EVENT_MANAGER = new MockEventManager(); + + static { + Runtime.getRuntime().addShutdownHook(new Thread(() -> { + try { + EVENT_MANAGER.shutdown(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + })); + } + + static VelocityCommandManager newManager() { + return new VelocityCommandManager(EVENT_MANAGER); + } + + private VelocityCommandManager manager; + + @BeforeEach + void setUp() { + this.manager = newManager(); + } + + // Registration + + @Test + void testRegisterWithMeta() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, DummyCommand.INSTANCE); + + assertTrue(manager.hasCommand("hello")); + } + + @Test + void testRegisterWithMetaContainingMultipleAliases() { + final var meta = manager.metaBuilder("foo") + .aliases("bar") + .aliases("baz", "qux") + .build(); + manager.register(meta, DummyCommand.INSTANCE); + + assertTrue(manager.hasCommand("foo")); + assertTrue(manager.hasCommand("bar")); + assertTrue(manager.hasCommand("baz")); + assertTrue(manager.hasCommand("qux")); + } + + @Test + void testRegisterAliasesAreCaseInsensitive() { + final var meta = manager.metaBuilder("Foo") + .aliases("Bar") + .build(); + manager.register(meta, DummyCommand.INSTANCE); + + assertTrue(manager.hasCommand("foo")); + assertTrue(manager.hasCommand("bar")); + } + + @Test + void testRegisterBrigadierCommand() { + final var node = LiteralArgumentBuilder + .literal("hello") + .build(); + manager.register(new BrigadierCommand(node)); + + assertTrue(manager.hasCommand("hello")); + } + + @Test + void testRegisterOverridesPreviousCommand() { + final var called = new AtomicBoolean(); + + final var oldMeta = manager.metaBuilder("foo").build(); + manager.register(oldMeta, DummyCommand.INSTANCE); // fails on execution + final var newMeta = manager.metaBuilder("foo").build(); + manager.register("foo", (RawCommand) invocation -> called.set(true)); + manager.executeAsync(MockCommandSource.INSTANCE, "foo").join(); + + assertTrue(called.get()); + } + + @Test + void testAddingExecutableHintToMetaThrows() { + final var hintNode = LiteralArgumentBuilder + .literal("hint") + .executes(context -> fail()) + .build(); + + assertThrows(IllegalArgumentException.class, () -> { + manager.metaBuilder("hello").hint(hintNode); + }); + } + + @Test + void testAddingHintWithRedirectToMetaThrows() { + final var targetNode = LiteralArgumentBuilder + .literal("target") + .build(); + final var hintNode = LiteralArgumentBuilder + .literal("origin") + .redirect(targetNode) + .build(); + + assertThrows(IllegalArgumentException.class, () -> { + manager.metaBuilder("hello").hint(hintNode); + }); + } + + // Un-registration + + + @Test + void testUnregisterUnregisteredAliasIsIgnored() { + manager.unregister("hello"); + + assertFalse(manager.hasCommand("hello")); + } + + @Test + void testUnregisterRegisteredAlias() { + manager.register("hello", DummyCommand.INSTANCE); + manager.unregister("hello"); + + assertFalse(manager.hasCommand("hello")); + } + + @Test + void testUnregisterSecondaryAlias() { + final var meta = manager.metaBuilder("foo") + .aliases("bar") + .build(); + manager.register(meta, DummyCommand.INSTANCE); + manager.unregister("bar"); + + assertFalse(manager.hasCommand("bar")); + assertTrue(manager.hasCommand("foo")); + } + + static class DummyCommand implements SimpleCommand { + + static final DummyCommand INSTANCE = new DummyCommand(); + + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + return fail(); + } + + @Override + public boolean hasPermission(final Invocation invocation) { + return fail(); + } + } +} diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandTestSuite.java b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandTestSuite.java index ccbdb798b..ff7748905 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandTestSuite.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandTestSuite.java @@ -15,7 +15,7 @@ abstract class CommandTestSuite { @BeforeEach void setUp() { - this.manager = new VelocityCommandManager(OldCommandManagerTests.EVENT_MANAGER); + this.manager = CommandManagerTests.newManager(); } final void assertHandled(final String input) { diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/OldCommandManagerTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/OldCommandManagerTests.java index 2835c6f7f..76202c833 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/OldCommandManagerTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/OldCommandManagerTests.java @@ -49,27 +49,15 @@ import org.junit.jupiter.api.Test; @Disabled public class OldCommandManagerTests { - public static final VelocityEventManager EVENT_MANAGER = new MockEventManager(); - - static { - Runtime.getRuntime().addShutdownHook(new Thread(() -> { - try { - EVENT_MANAGER.shutdown(); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - })); - } - static VelocityCommandManager createManager() { - return new VelocityCommandManager(EVENT_MANAGER); + return new VelocityCommandManager(CommandManagerTests.EVENT_MANAGER); } @Test void testConstruction() { VelocityCommandManager manager = createManager(); assertFalse(manager.hasCommand("foo")); - assertTrue(manager.getDispatcher().getRoot().getChildren().isEmpty()); + assertTrue(manager.getRoot().getChildren().isEmpty()); assertFalse(manager.executeAsync(MockCommandSource.INSTANCE, "foo").join()); assertFalse(manager.executeImmediatelyAsync(MockCommandSource.INSTANCE, "bar").join()); assertTrue(manager.offerSuggestions(MockCommandSource.INSTANCE, "").join().isEmpty()); From 46b1bee83a5097b3ed95fd620a716e96cf048ceb Mon Sep 17 00:00:00 2001 From: Hugo Manrique Date: Mon, 7 Jun 2021 15:15:48 +0200 Subject: [PATCH 06/13] Test suggestions --- .../proxy/command/BrigadierCommandTests.java | 69 +++++++++- .../proxy/command/CommandManagerTests.java | 4 +- .../proxy/command/RawCommandTests.java | 102 ++++++++++++++ .../proxy/command/SimpleCommandTests.java | 102 ++++++++++++++ .../command/SuggestionsProviderTests.java | 125 +++++++++++++++++- 5 files changed, 395 insertions(+), 7 deletions(-) diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java index 89c263e1f..7b14acd12 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java @@ -2,6 +2,7 @@ package com.velocitypowered.proxy.command; import static com.mojang.brigadier.arguments.IntegerArgumentType.getInteger; import static com.mojang.brigadier.arguments.IntegerArgumentType.integer; +import static com.mojang.brigadier.arguments.StringArgumentType.word; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; @@ -91,8 +92,7 @@ public class BrigadierCommandTests extends CommandTestSuite { assertEquals(2, context.getNodes().size()); callCount.incrementAndGet(); return 1; - }) - ) + })) .build(); manager.register(new BrigadierCommand(node)); @@ -113,12 +113,73 @@ public class BrigadierCommandTests extends CommandTestSuite { .requires(source -> { callCount.incrementAndGet(); return false; - }) - ) + })) .build(); manager.register(new BrigadierCommand(node)); assertHandled("hello world"); assertEquals(1, callCount.get()); } + + // Suggestions + + @Test + void testArgumentSuggestions() { + final var node = LiteralArgumentBuilder + .literal("hello") + .then(RequiredArgumentBuilder + .argument("argument", word()) + .suggests((context, builder) -> builder + .suggest("foo") + .suggest("bar") + .suggest("baz") + .buildFuture())) + .build(); + manager.register(new BrigadierCommand(node)); + + assertSuggestions("hello ", "bar", "baz", "foo"); + assertSuggestions("hello ba", "bar", "baz", "foo"); + assertSuggestions("hello bar", "baz", "foo"); + } + + // The following 2 tests ensure we strictly follow Brigadier's behavior, even + // if it doesn't make much sense. + + @Test + void testSuggestsEvenIfImpermissible() { + final var node = LiteralArgumentBuilder + .literal("parent") + .then(LiteralArgumentBuilder + .literal("child") + .requiresWithContext((context, reader) -> fail())) + .build(); + manager.register(new BrigadierCommand(node)); + + assertSuggestions("parent ", "child"); + assertSuggestions("parent chi", "child"); + } + + @Test + void testDoesNotSuggestIfImpermissibleDuringParse() { + final var callCount = new AtomicInteger(); + + final var node = LiteralArgumentBuilder + .literal("parent") + .then(LiteralArgumentBuilder + .literal("child") + .requiresWithContext((context, reader) -> { + // CommandDispatcher#parseNodes checks whether the child node can be added + // to the context object. CommandDispatcher#getCompletionSuggestions then + // considers a suggestion context with "parent" as the parent, and considers + // the suggestions of relevant children, which includes "child". + assertEquals(2, context.getNodes().size()); + callCount.incrementAndGet(); + return false; + })) + .build(); + manager.register(new BrigadierCommand(node)); + + assertSuggestions("parent child"); + assertEquals(1, callCount.get()); + } } 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 0d6570de4..d6dbbbeb2 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java @@ -157,10 +157,12 @@ public class CommandManagerTests { assertTrue(manager.hasCommand("foo")); } - static class DummyCommand implements SimpleCommand { + static final class DummyCommand implements SimpleCommand { static final DummyCommand INSTANCE = new DummyCommand(); + private DummyCommand() {} + @Override public void execute(final Invocation invocation) { fail(); diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java index 5e37be8b9..ca8f3634b 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java @@ -3,7 +3,10 @@ package com.velocitypowered.proxy.command; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; +import com.google.common.collect.ImmutableList; import com.velocitypowered.api.command.RawCommand; +import java.util.Collections; +import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; @@ -90,4 +93,103 @@ public class RawCommandTests extends CommandTestSuite { assertHandled("color red"); assertEquals(1, callCount.get()); } + + // Suggestions + + @Test + void testSuggestsArgumentsAfterAlias() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + assertEquals("hello", invocation.alias()); + assertEquals("", invocation.arguments()); + return ImmutableList.of("world", "people"); // ensures we don't mutate the user's list + } + }); + + assertSuggestions("hello ", "people", "world"); // in alphabetical order + } + + @Test + void testSuggestsArgumentsAfterPartialArguments() { + final var meta = manager.metaBuilder("numbers").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + assertEquals("12345678", invocation.arguments()); + return Collections.singletonList("9"); + } + }); + + assertSuggestions("numbers 12345678", "9"); + } + + @Test + void testDoesNotSuggestFirstArgumentIfImpermissibleAlias() { + 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("hello", invocation.alias()); + assertEquals("", invocation.arguments()); + callCount.incrementAndGet(); + return false; + } + + @Override + public List suggest(final Invocation invocation) { + return fail(); + } + }); + + assertSuggestions("hello "); + assertEquals(1, callCount.get()); + } + + @Test + void testDoesNotSuggestArgumentsAfterPartialImpermissibleArguments() { + final var callCount = new AtomicInteger(); + + final var meta = manager.metaBuilder("foo").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public boolean hasPermission(final Invocation invocation) { + assertEquals("foo", invocation.alias()); + assertEquals("bar baz ", invocation.arguments()); + callCount.incrementAndGet(); + return false; + } + + @Override + public List suggest(final Invocation invocation) { + return fail(); + } + }); + + assertSuggestions("foo bar baz "); + 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 index f52c8cf3d..3cead1f69 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java @@ -4,7 +4,10 @@ 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.google.common.collect.ImmutableList; import com.velocitypowered.api.command.SimpleCommand; +import java.util.Collections; +import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; @@ -91,4 +94,103 @@ public class SimpleCommandTests extends CommandTestSuite { assertHandled("color red"); assertEquals(1, callCount.get()); } + + // Suggestions + + @Test + void testSuggestsArgumentsAfterAlias() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + assertEquals("hello", invocation.alias()); + assertArrayEquals(new String[0], invocation.arguments()); + return ImmutableList.of("world", "people"); // ensures we don't mutate the user's list + } + }); + + assertSuggestions("hello ", "people", "world"); // in alphabetical order + } + + @Test + void testSuggestsArgumentsAfterPartialArguments() { + final var meta = manager.metaBuilder("numbers").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + assertArrayEquals(new String[] { "12345678" }, invocation.arguments()); + return Collections.singletonList("9"); + } + }); + + assertSuggestions("numbers 12345678", "9"); + } + + @Test + void testDoesNotSuggestFirstArgumentIfImpermissibleAlias() { + 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("hello", invocation.alias()); + assertArrayEquals(new String[0], invocation.arguments()); + callCount.incrementAndGet(); + return false; + } + + @Override + public List suggest(final Invocation invocation) { + return fail(); + } + }); + + assertSuggestions("hello "); + assertEquals(1, callCount.get()); + } + + @Test + void testDoesNotSuggestArgumentsAfterPartialImpermissibleArguments() { + final var callCount = new AtomicInteger(); + + final var meta = manager.metaBuilder("foo").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public boolean hasPermission(final Invocation invocation) { + assertEquals("foo", invocation.alias()); + assertArrayEquals(new String[] { "bar", "baz", "" }, invocation.arguments()); + callCount.incrementAndGet(); + return false; + } + + @Override + public List suggest(final Invocation invocation) { + return fail(); + } + }); + + assertSuggestions("foo bar baz "); + assertEquals(1, callCount.get()); + } } diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java index bc4b9e10d..1cb5d0551 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java @@ -17,6 +17,127 @@ package com.velocitypowered.proxy.command; -public class SuggestionsProviderTests { - // TODO +import static com.mojang.brigadier.arguments.StringArgumentType.word; +import static org.junit.jupiter.api.Assertions.fail; + +import com.google.common.collect.ImmutableList; +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import com.mojang.brigadier.builder.RequiredArgumentBuilder; +import com.velocitypowered.api.command.Command; +import com.velocitypowered.api.command.CommandSource; +import com.velocitypowered.api.command.RawCommand; +import java.util.List; +import org.junit.jupiter.api.Test; + +/** + * Tests {@link Command} implementation-independent suggestion methods of + * {@link SuggestionsProvider}. + */ +public class SuggestionsProviderTests extends CommandTestSuite { + + @Test + void testSuggestsAliasForEmptyInput() { + final var meta = manager.metaBuilder("foo") + .aliases("bar", "baz") + .build(); + manager.register(meta, NoSuggestionsCommand.INSTANCE); + + assertSuggestions("", "bar", "baz", "foo"); + } + + @Test + void testDoesNotSuggestForLeadingWhitespace() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, NoSuggestionsCommand.INSTANCE); + + assertSuggestions(" "); + } + + @Test + void testSuggestsAliasesForPartialAlias() { + final var meta = manager.metaBuilder("foo") + .aliases("bar", "baz") + .build(); + manager.register(meta, NoSuggestionsCommand.INSTANCE); + + assertSuggestions("ba", "bar", "baz"); + assertSuggestions("fo", "foo"); + assertSuggestions("bar"); + } + + @Test + void testSuggestsHintLiteral() { + final var hint = LiteralArgumentBuilder + .literal("hint") + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, NoSuggestionsCommand.INSTANCE); + + assertSuggestions("hello ", "hint"); + assertSuggestions("hello hin", "hint"); + assertSuggestions("hello hint"); + } + + @Test + void testSuggestsHintCustomSuggestions() { + final var hint = RequiredArgumentBuilder + .argument("hint", word()) + .suggests((context, builder) -> builder + .suggest("one") + .suggest("two") + .suggest("three") + .buildFuture()) + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, NoSuggestionsCommand.INSTANCE); + + assertSuggestions("hello ", "one", "three", "two"); + assertSuggestions("hello two", "one", "three"); + } + + @Test + void testSuggestsMergesArgumentsSuggestionsWithHintSuggestions() { + final var hint = LiteralArgumentBuilder + .literal("bar") + .build(); + final var meta = manager.metaBuilder("foo") + .hint(hint) + .build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + return ImmutableList.of("baz", "qux"); + } + }); + + // TODO Fix merging + assertSuggestions("hello ", "bar", "baz", "qux"); + assertSuggestions("hello bar", "baz", "qux"); + } + + static final class NoSuggestionsCommand implements RawCommand { + + static final NoSuggestionsCommand INSTANCE = new NoSuggestionsCommand(); + + private NoSuggestionsCommand() {} + + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + return ImmutableList.of(); + } + } } From a9d40f3ca393371b122b9c89ab807b06e1fdf76d Mon Sep 17 00:00:00 2001 From: Hugo Manrique Date: Wed, 9 Jun 2021 19:15:27 +0200 Subject: [PATCH 07/13] Fix alias typo in suggestion merge test --- .../proxy/command/SuggestionsProviderTests.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java index 1cb5d0551..3c721fc95 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java @@ -119,9 +119,8 @@ public class SuggestionsProviderTests extends CommandTestSuite { } }); - // TODO Fix merging - assertSuggestions("hello ", "bar", "baz", "qux"); - assertSuggestions("hello bar", "baz", "qux"); + assertSuggestions("foo ", "bar", "baz", "qux"); + assertSuggestions("foo bar", "baz", "qux"); } static final class NoSuggestionsCommand implements RawCommand { From bcb68c8d0f29a14f72c24d13417f28c17c9e0828 Mon Sep 17 00:00:00 2001 From: Hugo Manrique Date: Wed, 9 Jun 2021 21:00:17 +0200 Subject: [PATCH 08/13] Test suggestion exception handling --- .../proxy/command/SuggestionsProvider.java | 15 +- .../command/SuggestionsProviderTests.java | 144 ++++++++++++++++++ 2 files changed, 154 insertions(+), 5 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java index 869d7ccb7..259a1ff24 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java @@ -182,6 +182,7 @@ final class SuggestionsProvider { final LiteralCommandNode alias, final StringReader reader, final CommandContextBuilder contextSoFar) { final S source = contextSoFar.getSource(); + final String fullInput = reader.getString(); final VelocityArgumentCommandNode argsNode = VelocityCommands.getArgumentsNode(alias); if (argsNode == null) { // This is a BrigadierCommand, fallback to regular suggestions @@ -212,15 +213,13 @@ final class SuggestionsProvider { this.getArgumentsNodeSuggestions(argsNode, reader, context); final boolean hasHints = alias.getChildren().size() > 1; if (!hasHints) { - return cmdSuggestions; + return this.merge(fullInput, cmdSuggestions); } // Parse the hint nodes to get remaining suggestions reader.setCursor(start); final CompletableFuture hintSuggestions = this.getHintSuggestions(alias, reader, contextSoFar); - - final String fullInput = reader.getString(); return this.merge(fullInput, cmdSuggestions, hintSuggestions); } @@ -245,7 +244,7 @@ final class SuggestionsProvider { return node.listSuggestions(built, new SuggestionsBuilder(fullInput, start)); } catch (final Throwable e) { // Ugly, ugly swallowing of everything Throwable, because plugins are naughty. - LOGGER.error("Arguments node cannot provide suggestions, skipping", e); + LOGGER.error("Arguments node cannot provide suggestions", e); return Suggestions.empty(); } } @@ -264,7 +263,13 @@ final class SuggestionsProvider { final LiteralCommandNode alias, final StringReader reader, final CommandContextBuilder context) { final ParseResults parse = this.parseHints(alias, reader, context); - return this.dispatcher.getCompletionSuggestions(parse); + try { + return this.dispatcher.getCompletionSuggestions(parse); + } catch (final Throwable e) { + // Again, plugins are naughty. + LOGGER.error("Hint node cannot provide suggestions", e); + return Suggestions.empty(); + } } /** diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java index 3c721fc95..66fed8234 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java @@ -23,10 +23,13 @@ import static org.junit.jupiter.api.Assertions.fail; import com.google.common.collect.ImmutableList; import com.mojang.brigadier.builder.LiteralArgumentBuilder; import com.mojang.brigadier.builder.RequiredArgumentBuilder; +import com.spotify.futures.CompletableFutures; import com.velocitypowered.api.command.Command; import com.velocitypowered.api.command.CommandSource; import com.velocitypowered.api.command.RawCommand; +import com.velocitypowered.api.command.SimpleCommand; import java.util.List; +import java.util.concurrent.CompletableFuture; import org.junit.jupiter.api.Test; /** @@ -65,6 +68,51 @@ public class SuggestionsProviderTests extends CommandTestSuite { assertSuggestions("bar"); } + @Test + void testDoesNotSuggestForPartialIncorrectAlias() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, NoSuggestionsCommand.INSTANCE); + + assertSuggestions("yo"); + assertSuggestions("welcome"); + } + + @Test + void testDoesNotSuggestArgumentsForPartialAlias() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + return fail(); + } + }); + + assertSuggestions("hell "); + } + + @Test + void testSuggestsArgumentsIgnoresAliasCase() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + return ImmutableList.of("world"); + } + }); + + assertSuggestions("Hello ", "world"); + } + @Test void testSuggestsHintLiteral() { final var hint = LiteralArgumentBuilder @@ -123,6 +171,102 @@ public class SuggestionsProviderTests extends CommandTestSuite { assertSuggestions("foo bar", "baz", "qux"); } + // Exception handling + + @Test + void testSkipsSuggestionWhenNodeSuggestionProviderFutureCompletesExceptionally() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public CompletableFuture> suggestAsync(final Invocation invocation) { + return CompletableFutures.exceptionallyCompletedFuture(new RuntimeException()); + } + }); + + assertSuggestions("hello "); + } + + @Test + void testSkipsSuggestionWhenNodeSuggestionProviderThrows() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public CompletableFuture> suggestAsync(final Invocation invocation) { + throw new RuntimeException(); + } + }); + + // Also logs an error to the console, but testing this is quite involved + assertSuggestions("hello "); + } + + @Test + void testSkipsSuggestionWhenHintSuggestionProviderFutureCompletesExceptionally() { + final var hint = RequiredArgumentBuilder + .argument("hint", word()) + .suggests((context, builder) -> + CompletableFutures.exceptionallyCompletedFuture(new RuntimeException())) + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, NoSuggestionsCommand.INSTANCE); + + assertSuggestions("hello "); + } + + @Test + void testSkipsSuggestionWhenHintSuggestionProviderThrows() { + final var hint = RequiredArgumentBuilder + .argument("hint", word()) + .suggests((context, builder) -> { + throw new RuntimeException(); + }) + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, NoSuggestionsCommand.INSTANCE); + + // Also logs an error to the console, but testing this is quite involved + assertSuggestions("hello "); + } + + @Test + void testSuggestionMergingIgnoresExceptionallyCompletedSuggestionFutures() { + final var hint = RequiredArgumentBuilder + .argument("hint", word()) + .suggests((context, builder) -> + CompletableFutures.exceptionallyCompletedFuture(new RuntimeException())) + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + return ImmutableList.of("world"); + } + }); + + assertSuggestions("hello ", "world"); + } + static final class NoSuggestionsCommand implements RawCommand { static final NoSuggestionsCommand INSTANCE = new NoSuggestionsCommand(); From ba34e4729b5a0394309f25aba1f7e3e896fc77b2 Mon Sep 17 00:00:00 2001 From: Hugo Manrique Date: Wed, 9 Jun 2021 22:22:15 +0200 Subject: [PATCH 09/13] Add more suggestion tests --- .../proxy/command/SuggestionsProvider.java | 23 ++- .../proxy/command/VelocityCommandManager.java | 14 +- .../proxy/command/BrigadierCommandTests.java | 85 +++++++++++ .../proxy/command/CommandManagerTests.java | 1 - .../proxy/command/RawCommandTests.java | 109 ++++++++++++++ .../proxy/command/SimpleCommandTests.java | 107 ++++++++++++++ .../command/SuggestionsProviderTests.java | 137 ++++++++++-------- 7 files changed, 407 insertions(+), 69 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java index 259a1ff24..2de0503d4 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java @@ -121,16 +121,15 @@ final class SuggestionsProvider { } /** - * Returns whether a literal node with the given name should be considered for + * Returns whether a literal node with the given lowercase name should be considered for * suggestions given the specified input. * - * @param name the literal name + * @param name the lowercase literal name * @param input the partial input * @return true if the literal should be considered; false otherwise */ private static boolean shouldConsider(final String name, final String input) { - // TODO (perf) If we expect input to be lowercase, no need to ignore case - return name.regionMatches(true, 0, input, 0, input.length()); + return name.regionMatches(false, 0, input, 0, input.length()); } /** @@ -143,7 +142,9 @@ final class SuggestionsProvider { private CompletableFuture provideAliasSuggestions( final StringReader reader, final CommandContextBuilder contextSoFar) { final S source = contextSoFar.getSource(); - final String input = reader.getRead(); + // Lowercase the alias here so all comparisons can be case-sensitive (cheaper) + // TODO Is this actually faster? + final String input = reader.getRead().toLowerCase(Locale.ENGLISH); final Collection> aliases = contextSoFar.getRootNode().getChildren(); @SuppressWarnings("unchecked") @@ -188,7 +189,13 @@ final class SuggestionsProvider { // This is a BrigadierCommand, fallback to regular suggestions reader.setCursor(0); final ParseResults parse = this.dispatcher.parse(reader, source); - return this.dispatcher.getCompletionSuggestions(parse); + try { + return this.dispatcher.getCompletionSuggestions(parse); + } catch (final Throwable e) { + // Ugly, ugly swallowing of everything Throwable, because plugins are naughty. + LOGGER.error("Command node cannot provide suggestions for " + fullInput, e); + return Suggestions.empty(); + } } if (!argsNode.canUse(source)) { @@ -243,7 +250,7 @@ final class SuggestionsProvider { try { return node.listSuggestions(built, new SuggestionsBuilder(fullInput, start)); } catch (final Throwable e) { - // Ugly, ugly swallowing of everything Throwable, because plugins are naughty. + // Again, plugins are naughty LOGGER.error("Arguments node cannot provide suggestions", e); return Suggestions.empty(); } @@ -266,7 +273,7 @@ final class SuggestionsProvider { try { return this.dispatcher.getCompletionSuggestions(parse); } catch (final Throwable e) { - // Again, plugins are naughty. + // Yet again, plugins are naughty. LOGGER.error("Hint node cannot provide suggestions", e); return Suggestions.empty(); } 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 6332c51f1..04fe0cda8 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java @@ -25,6 +25,7 @@ import com.mojang.brigadier.ParseResults; import com.mojang.brigadier.exceptions.CommandSyntaxException; import com.mojang.brigadier.suggestion.Suggestion; import com.mojang.brigadier.tree.RootCommandNode; +import com.spotify.futures.CompletableFutures; import com.velocitypowered.api.command.BrigadierCommand; import com.velocitypowered.api.command.Command; import com.velocitypowered.api.command.CommandManager; @@ -150,8 +151,9 @@ public class VelocityCommandManager implements CommandManager { Preconditions.checkNotNull(cmdLine, "cmdLine"); final String normalizedInput = VelocityCommands.normalizeInput(cmdLine, true); - final ParseResults parse = this.parse(normalizedInput, source); try { + // The parse can fail if the requirement predicates throw + final ParseResults parse = this.parse(normalizedInput, source); return dispatcher.execute(parse) != BrigadierCommand.FORWARD; } catch (final CommandSyntaxException e) { boolean isSyntaxError = !e.getType().equals( @@ -207,8 +209,14 @@ public class VelocityCommandManager implements CommandManager { Preconditions.checkNotNull(cmdLine, "cmdLine"); final String normalizedInput = VelocityCommands.normalizeInput(cmdLine, false); - return suggestionsProvider.provideSuggestions(normalizedInput, source) - .thenApply(suggestions -> Lists.transform(suggestions.getList(), Suggestion::getText)); + try { + return suggestionsProvider.provideSuggestions(normalizedInput, source) + .thenApply(suggestions -> Lists.transform(suggestions.getList(), Suggestion::getText)); + } catch (final Throwable e) { + // Again, plugins are naughty + return CompletableFutures.exceptionallyCompletedFuture( + new RuntimeException("Unable to provide suggestions for " + cmdLine + " for " + source, e)); + } } /** diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java index 7b14acd12..3038ce766 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java @@ -4,12 +4,16 @@ import static com.mojang.brigadier.arguments.IntegerArgumentType.getInteger; import static com.mojang.brigadier.arguments.IntegerArgumentType.integer; import static com.mojang.brigadier.arguments.StringArgumentType.word; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; import com.mojang.brigadier.builder.LiteralArgumentBuilder; import com.mojang.brigadier.builder.RequiredArgumentBuilder; +import com.spotify.futures.CompletableFutures; import com.velocitypowered.api.command.BrigadierCommand; import com.velocitypowered.api.command.CommandSource; +import java.util.concurrent.CompletionException; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; @@ -121,6 +125,41 @@ public class BrigadierCommandTests extends CommandTestSuite { assertEquals(1, callCount.get()); } + @Test + void testExecuteAsyncCompletesExceptionallyOnCallbackException() { + final var expected = new RuntimeException(); + final var node = LiteralArgumentBuilder + .literal("hello") + .executes(context -> { + throw expected; + }) + .build(); + manager.register(new BrigadierCommand(node)); + + final Exception wrapper = assertThrows(CompletionException.class, () -> + manager.executeAsync(source, "hello").join()); + + assertSame(expected, wrapper.getCause().getCause()); + } + + @Test + void testExecuteAsyncCompletesExceptionallyOnRequirementException() { + final var expected = new RuntimeException(); + final var node = LiteralArgumentBuilder + .literal("hello") + .requires(source1 -> { + throw expected; + }) + .executes(context -> fail()) // needed for dispatcher to consider the node + .build(); + manager.register(new BrigadierCommand(node)); + + final Exception wrapper = assertThrows(CompletionException.class, () -> + manager.executeAsync(source, "hello").join()); + + assertSame(expected, wrapper.getCause().getCause()); + } + // Suggestions @Test @@ -182,4 +221,50 @@ public class BrigadierCommandTests extends CommandTestSuite { assertSuggestions("parent child"); assertEquals(1, callCount.get()); } + + @Test + void testDoesNotSuggestIfCustomSuggestionProviderFutureCompletesExceptionally() { + final var node = LiteralArgumentBuilder + .literal("parent") + .then(RequiredArgumentBuilder + .argument("child", word()) + .suggests((context, builder) -> + CompletableFutures.exceptionallyCompletedFuture(new RuntimeException()))) + .build(); + manager.register(new BrigadierCommand(node)); + + assertSuggestions("parent "); + } + + @Test + void testDoesNotSuggestIfCustomSuggestionProviderThrows() { + final var node = LiteralArgumentBuilder + .literal("parent") + .then(RequiredArgumentBuilder + .argument("child", word()) + .suggests((context, builder) -> { + throw new RuntimeException(); + })) + .build(); + manager.register(new BrigadierCommand(node)); + + assertSuggestions("parent "); + } + + @Test + void testSuggestCompletesExceptionallyIfRequirementPredicateThrows() { + final var node = LiteralArgumentBuilder + .literal("parent") + .requires(source1 -> { + throw new RuntimeException(); + }) + .then(RequiredArgumentBuilder + .argument("child", word()) + .suggests((context, builder) -> fail())) + .build(); + manager.register(new BrigadierCommand(node)); + + assertThrows(CompletionException.class, () -> + manager.offerSuggestions(source, "parent ").join()); + } } 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 d6dbbbeb2..8bd9236f9 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java @@ -129,7 +129,6 @@ public class CommandManagerTests { // Un-registration - @Test void testUnregisterUnregisteredAliasIsIgnored() { manager.unregister("hello"); diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java index ca8f3634b..3f8ff1bfe 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java @@ -1,12 +1,18 @@ 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.assertThrows; import static org.junit.jupiter.api.Assertions.fail; import com.google.common.collect.ImmutableList; +import com.spotify.futures.CompletableFutures; import com.velocitypowered.api.command.RawCommand; +import com.velocitypowered.api.command.SimpleCommand; import java.util.Collections; import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; @@ -96,6 +102,29 @@ public class RawCommandTests extends CommandTestSuite { // Suggestions + @Test + void testDoesNotSuggestAliasIfImpermissible() { + 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("hello", invocation.alias()); + assertEquals("", invocation.arguments()); + return false; + } + + @Override + public List suggest(final Invocation invocation) { + return fail(); + } + }); + } + @Test void testSuggestsArgumentsAfterAlias() { final var meta = manager.metaBuilder("hello").build(); @@ -116,6 +145,25 @@ public class RawCommandTests extends CommandTestSuite { assertSuggestions("hello ", "people", "world"); // in alphabetical order } + @Test + void testSuggestsArgumentsAfterAliasIgnoresAliasCase() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + assertEquals("hello", invocation.alias()); + return ImmutableList.of("world"); + } + }); + + assertSuggestions("Hello ", "world"); + } + @Test void testSuggestsArgumentsAfterPartialArguments() { final var meta = manager.metaBuilder("numbers").build(); @@ -192,4 +240,65 @@ public class RawCommandTests extends CommandTestSuite { assertSuggestions("foo bar baz "); assertEquals(1, callCount.get()); } + + @Test + void testDoesNotSuggestIfFutureCompletesExceptionally() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public CompletableFuture> suggestAsync(final Invocation invocation) { + return CompletableFutures.exceptionallyCompletedFuture(new RuntimeException()); + } + }); + + assertSuggestions("hello "); + } + + @Test + void testDoesNotSuggestIfSuggestAsyncThrows() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public CompletableFuture> suggestAsync(final Invocation invocation) { + throw new RuntimeException(); + } + }); + + // Also logs an error to the console, but testing this is quite involved + assertSuggestions("hello "); + } + + @Test + void testSuggestCompletesExceptionallyIfHasPermissionThrows() { + 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) { + throw new RuntimeException(); + } + + @Override + public CompletableFuture> suggestAsync(final Invocation invocation) { + return fail(); + } + }); + + assertThrows(CompletionException.class, () -> + manager.offerSuggestions(source, "hello ").join()); + } } diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java index 3cead1f69..e4ccec57d 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java @@ -2,12 +2,16 @@ 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.assertThrows; import static org.junit.jupiter.api.Assertions.fail; import com.google.common.collect.ImmutableList; +import com.spotify.futures.CompletableFutures; import com.velocitypowered.api.command.SimpleCommand; import java.util.Collections; import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; @@ -97,6 +101,29 @@ public class SimpleCommandTests extends CommandTestSuite { // Suggestions + @Test + void testDoesNotSuggestAliasIfImpermissible() { + 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("hello", invocation.alias()); + assertArrayEquals(new String[0], invocation.arguments()); + return false; + } + + @Override + public List suggest(final Invocation invocation) { + return fail(); + } + }); + } + @Test void testSuggestsArgumentsAfterAlias() { final var meta = manager.metaBuilder("hello").build(); @@ -117,6 +144,25 @@ public class SimpleCommandTests extends CommandTestSuite { assertSuggestions("hello ", "people", "world"); // in alphabetical order } + @Test + void testSuggestsArgumentsAfterAliasIgnoresAliasCase() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + assertEquals("hello", invocation.alias()); + return ImmutableList.of("world"); + } + }); + + assertSuggestions("Hello ", "world"); + } + @Test void testSuggestsArgumentsAfterPartialArguments() { final var meta = manager.metaBuilder("numbers").build(); @@ -193,4 +239,65 @@ public class SimpleCommandTests extends CommandTestSuite { assertSuggestions("foo bar baz "); assertEquals(1, callCount.get()); } + + @Test + void testDoesNotSuggestIfFutureCompletesExceptionally() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public CompletableFuture> suggestAsync(final Invocation invocation) { + return CompletableFutures.exceptionallyCompletedFuture(new RuntimeException()); + } + }); + + assertSuggestions("hello "); + } + + @Test + void testDoesNotSuggestIfSuggestAsyncThrows() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public CompletableFuture> suggestAsync(final Invocation invocation) { + throw new RuntimeException(); + } + }); + + // Also logs an error to the console, but testing this is quite involved + assertSuggestions("hello "); + } + + @Test + void testSuggestCompletesExceptionallyIfHasPermissionThrows() { + 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) { + throw new RuntimeException(); + } + + @Override + public CompletableFuture> suggestAsync(final Invocation invocation) { + return fail(); + } + }); + + assertThrows(CompletionException.class, () -> + manager.offerSuggestions(source, "hello ").join()); + } } diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java index 66fed8234..dd61069bc 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java @@ -18,18 +18,22 @@ package com.velocitypowered.proxy.command; import static com.mojang.brigadier.arguments.StringArgumentType.word; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; import com.google.common.collect.ImmutableList; import com.mojang.brigadier.builder.LiteralArgumentBuilder; import com.mojang.brigadier.builder.RequiredArgumentBuilder; import com.spotify.futures.CompletableFutures; +import com.velocitypowered.api.command.BrigadierCommand; import com.velocitypowered.api.command.Command; import com.velocitypowered.api.command.CommandSource; import com.velocitypowered.api.command.RawCommand; import com.velocitypowered.api.command.SimpleCommand; import java.util.List; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import org.junit.jupiter.api.Test; /** @@ -39,13 +43,12 @@ import org.junit.jupiter.api.Test; public class SuggestionsProviderTests extends CommandTestSuite { @Test - void testSuggestsAliasForEmptyInput() { - final var meta = manager.metaBuilder("foo") - .aliases("bar", "baz") - .build(); - manager.register(meta, NoSuggestionsCommand.INSTANCE); + void testSuggestsAliasesForEmptyInput() { + manager.register(manager.metaBuilder("foo").build(), NoSuggestionsCommand.INSTANCE); + manager.register(manager.metaBuilder("bar").build(), NoSuggestionsCommand.INSTANCE); + manager.register(manager.metaBuilder("baz").build(), NoSuggestionsCommand.INSTANCE); - assertSuggestions("", "bar", "baz", "foo"); + assertSuggestions("", "bar", "baz", "foo"); // in alphabetical order } @Test @@ -58,14 +61,20 @@ public class SuggestionsProviderTests extends CommandTestSuite { @Test void testSuggestsAliasesForPartialAlias() { - final var meta = manager.metaBuilder("foo") - .aliases("bar", "baz") - .build(); + manager.register(manager.metaBuilder("hello").build(), NoSuggestionsCommand.INSTANCE); + manager.register(manager.metaBuilder("hey").build(), NoSuggestionsCommand.INSTANCE); + + assertSuggestions("hell", "hello"); + assertSuggestions("He", "hello", "hey"); + } + + @Test + void testDoesNotSuggestForFullAlias() { + final var meta = manager.metaBuilder("hello").build(); manager.register(meta, NoSuggestionsCommand.INSTANCE); - assertSuggestions("ba", "bar", "baz"); - assertSuggestions("fo", "foo"); - assertSuggestions("bar"); + assertSuggestions("hello"); + assertSuggestions("Hello"); } @Test @@ -78,9 +87,9 @@ public class SuggestionsProviderTests extends CommandTestSuite { } @Test - void testDoesNotSuggestArgumentsForPartialAlias() { + void testDoesNotSuggestArgumentsForIncorrectAlias() { final var meta = manager.metaBuilder("hello").build(); - manager.register(meta, new SimpleCommand() { + manager.register(meta, new RawCommand() { @Override public void execute(final Invocation invocation) { fail(); @@ -95,10 +104,26 @@ public class SuggestionsProviderTests extends CommandTestSuite { assertSuggestions("hell "); } + // Secondary aliases + // The following tests check for inconsistencies between the primary alias node and + // a secondary alias literal. + @Test - void testSuggestsArgumentsIgnoresAliasCase() { - final var meta = manager.metaBuilder("hello").build(); - manager.register(meta, new SimpleCommand() { + void testSuggestsAllAliases() { + final var meta = manager.metaBuilder("foo") + .aliases("bar", "baz") + .build(); + manager.register(meta, NoSuggestionsCommand.INSTANCE); + + assertSuggestions("", "bar", "baz", "foo"); + } + + @Test + void testSuggestsArgumentsViaAlias() { + final var meta = manager.metaBuilder("hello") + .aliases("hi") + .build(); + manager.register(meta, new RawCommand() { @Override public void execute(final Invocation invocation) { fail(); @@ -110,9 +135,11 @@ public class SuggestionsProviderTests extends CommandTestSuite { } }); - assertSuggestions("Hello ", "world"); + assertSuggestions("hi ", "world"); } + // Hinting + @Test void testSuggestsHintLiteral() { final var hint = LiteralArgumentBuilder @@ -169,49 +196,26 @@ public class SuggestionsProviderTests extends CommandTestSuite { assertSuggestions("foo ", "bar", "baz", "qux"); assertSuggestions("foo bar", "baz", "qux"); - } - - // Exception handling - - @Test - void testSkipsSuggestionWhenNodeSuggestionProviderFutureCompletesExceptionally() { - final var meta = manager.metaBuilder("hello").build(); - manager.register(meta, new RawCommand() { - @Override - public void execute(final Invocation invocation) { - fail(); - } - - @Override - public CompletableFuture> suggestAsync(final Invocation invocation) { - return CompletableFutures.exceptionallyCompletedFuture(new RuntimeException()); - } - }); - - assertSuggestions("hello "); + assertSuggestions("foo baz", "bar", "qux"); } @Test - void testSkipsSuggestionWhenNodeSuggestionProviderThrows() { - final var meta = manager.metaBuilder("hello").build(); - manager.register(meta, new RawCommand() { - @Override - public void execute(final Invocation invocation) { - fail(); - } + // This doesn't make much sense, but emulates Brigadier behavior + void testSuggestsImpermissibleHint() { + final var hint = LiteralArgumentBuilder + .literal("hint") + .requires(source1 -> false) + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, NoSuggestionsCommand.INSTANCE); - @Override - public CompletableFuture> suggestAsync(final Invocation invocation) { - throw new RuntimeException(); - } - }); - - // Also logs an error to the console, but testing this is quite involved - assertSuggestions("hello "); + assertSuggestions("hello ", "hint"); } @Test - void testSkipsSuggestionWhenHintSuggestionProviderFutureCompletesExceptionally() { + void testDoesNotSuggestHintIfHintSuggestionProviderFutureCompletesExceptionally() { final var hint = RequiredArgumentBuilder .argument("hint", word()) .suggests((context, builder) -> @@ -226,7 +230,7 @@ public class SuggestionsProviderTests extends CommandTestSuite { } @Test - void testSkipsSuggestionWhenHintSuggestionProviderThrows() { + void testDoesNotSuggestHintIfCustomSuggestionProviderThrows() { final var hint = RequiredArgumentBuilder .argument("hint", word()) .suggests((context, builder) -> { @@ -238,10 +242,29 @@ public class SuggestionsProviderTests extends CommandTestSuite { .build(); manager.register(meta, NoSuggestionsCommand.INSTANCE); - // Also logs an error to the console, but testing this is quite involved assertSuggestions("hello "); } + @Test + void testSuggestCompletesExceptionallyIfHintRequirementPredicateThrows() { + final var hint = RequiredArgumentBuilder + .argument("hint", word()) + .requires(source1 -> { + throw new RuntimeException(); + }) + .suggests((context, builder) -> fail()) + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, NoSuggestionsCommand.INSTANCE); + + assertThrows(CompletionException.class, () -> + manager.offerSuggestions(source, "hello ").join()); + } + + /* + @Test void testSuggestionMergingIgnoresExceptionallyCompletedSuggestionFutures() { final var hint = RequiredArgumentBuilder @@ -265,7 +288,7 @@ public class SuggestionsProviderTests extends CommandTestSuite { }); assertSuggestions("hello ", "world"); - } + }*/ static final class NoSuggestionsCommand implements RawCommand { From b18e78eac796a424434dc9b25d491e7bd65facde Mon Sep 17 00:00:00 2001 From: Hugo Manrique Date: Thu, 10 Jun 2021 20:00:24 +0200 Subject: [PATCH 10/13] Add more tests and clean up --- .../proxy/command/SuggestionsProvider.java | 7 + .../proxy/command/BrigadierCommandTests.java | 79 +++ .../command/CommandGraphInjectorTests.java | 24 +- .../proxy/command/CommandManagerTests.java | 60 ++- .../proxy/command/CommandTestSuite.java | 40 +- .../proxy/command/OldCommandManagerTests.java | 495 ------------------ .../proxy/command/RawCommandTests.java | 182 +++++++ .../proxy/command/SimpleCommandTests.java | 182 +++++++ .../command/SuggestionsProviderTests.java | 67 +-- 9 files changed, 541 insertions(+), 595 deletions(-) delete mode 100644 proxy/src/test/java/com/velocitypowered/proxy/command/OldCommandManagerTests.java diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java index 2de0503d4..e6af2d62b 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java @@ -30,6 +30,8 @@ import com.mojang.brigadier.suggestion.SuggestionsBuilder; import com.mojang.brigadier.tree.CommandNode; import com.mojang.brigadier.tree.LiteralCommandNode; import com.velocitypowered.api.command.Command; +import com.velocitypowered.api.command.CommandMeta; +import com.velocitypowered.api.command.CommandSource; import com.velocitypowered.proxy.command.brigadier.VelocityArgumentCommandNode; import java.util.ArrayList; import java.util.Collection; @@ -283,12 +285,17 @@ final class SuggestionsProvider { * Parses the hint nodes under the given node, which is either an alias node of * a {@link Command} or another hint node. * + * The caller must check the requirements + * are satisfied by a given source prior to calling this method. + * *

The reader and context are not mutated by this method. * * @param node the node to parse * @param originalReader the input reader * @param contextSoFar the context, containing the alias node of the command * @return the parse results containing the parsed hint nodes + * @see VelocityCommandMeta#copyHints(CommandMeta) for the conditions under which the returned + * hints can be suggested to a {@link CommandSource}. */ private ParseResults parseHints(final CommandNode node, final StringReader originalReader, final CommandContextBuilder contextSoFar) { diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java index 3038ce766..b1fec4155 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command; import static com.mojang.brigadier.arguments.IntegerArgumentType.getInteger; @@ -41,6 +58,57 @@ public class BrigadierCommandTests extends CommandTestSuite { assertEquals(1, callCount.get()); } + @Test + void testExecuteIgnoresAliasCase() { + final var callCount = new AtomicInteger(); + + final var node = LiteralArgumentBuilder + .literal("hello") + .executes(context -> { + assertEquals("hello", context.getInput()); + callCount.incrementAndGet(); + return 1; + }) + .build(); + manager.register(new BrigadierCommand(node)); + + assertHandled("Hello"); + assertEquals(1, callCount.get()); + } + + @Test + void testExecuteInputIsTrimmed() { + final var callCount = new AtomicInteger(); + + final var node = LiteralArgumentBuilder + .literal("hello") + .executes(context -> { + assertEquals("hello", context.getInput()); + callCount.incrementAndGet(); + return 1; + }) + .build(); + manager.register(new BrigadierCommand(node)); + + assertHandled(" hello"); + assertHandled(" hello"); + assertHandled("hello "); + assertHandled("hello "); + assertEquals(4, callCount.get()); + } + + @Test + void testExecuteAfterUnregisterForwards() { + final var node = LiteralArgumentBuilder + .literal("hello") + .executes(context -> fail()) + .build(); + manager.register(new BrigadierCommand(node)); + manager.unregister("hello"); + + assertForwarded("hello"); + } + @Test void testForwardsAndDoesNotExecuteImpermissibleAlias() { final var callCount = new AtomicInteger(); @@ -162,6 +230,17 @@ public class BrigadierCommandTests extends CommandTestSuite { // Suggestions + @Test + void testDoesNotSuggestAliasAfterUnregister() { + final var node = LiteralArgumentBuilder + .literal("hello") + .build(); + manager.register(new BrigadierCommand(node)); + manager.unregister("hello"); + + assertSuggestions(""); + } + @Test void testArgumentSuggestions() { final var node = LiteralArgumentBuilder 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 d0c3a9414..92a5b8b19 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java @@ -22,7 +22,6 @@ 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; @@ -36,16 +35,13 @@ import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -public class CommandGraphInjectorTests { +public class CommandGraphInjectorTests extends CommandTestSuite { - private static final CommandSource SOURCE = MockCommandSource.INSTANCE; - - private VelocityCommandManager manager; private RootCommandNode dest; @BeforeEach void setUp() { - this.manager = CommandManagerTests.newManager(); + super.setUp(); this.dest = new RootCommandNode<>(); } @@ -53,7 +49,7 @@ public class CommandGraphInjectorTests { void testInjectInvocableCommand() { final var meta = manager.metaBuilder("hello").build(); manager.register(meta, (SimpleCommand) invocation -> fail()); - manager.getInjector().inject(dest, SOURCE); + manager.getInjector().inject(dest, source); // Preserves alias and arguments node final var expected = manager.getRoot(); @@ -73,14 +69,14 @@ public class CommandGraphInjectorTests { @Override public boolean hasPermission(final Invocation invocation) { - assertEquals(SOURCE, invocation.source()); + assertEquals(source, invocation.source()); assertEquals("hello", invocation.alias()); assertArrayEquals(new String[0], invocation.arguments()); callCount.incrementAndGet(); return false; } }); - manager.getInjector().inject(dest, SOURCE); + manager.getInjector().inject(dest, source); assertTrue(dest.getChildren().isEmpty()); assertEquals(1, callCount.get()); @@ -94,7 +90,7 @@ public class CommandGraphInjectorTests { .then(argument("count", integer())) .build(); manager.register(new BrigadierCommand(node)); - manager.getInjector().inject(dest, SOURCE); + manager.getInjector().inject(dest, source); assertEquals(node, dest.getChild("hello")); } @@ -113,7 +109,7 @@ public class CommandGraphInjectorTests { })) .build(); manager.register(new BrigadierCommand(registered)); - manager.getInjector().inject(dest, SOURCE); + manager.getInjector().inject(dest, source); final var expected = LiteralArgumentBuilder .literal("greet") @@ -131,7 +127,7 @@ public class CommandGraphInjectorTests { .build()) .build(); manager.register(new BrigadierCommand(registered)); - manager.getInjector().inject(dest, SOURCE); + manager.getInjector().inject(dest, source); final var expected = LiteralArgumentBuilder .literal("origin") @@ -158,7 +154,7 @@ public class CommandGraphInjectorTests { ) .build(); manager.register(new BrigadierCommand(registered)); - manager.getInjector().inject(dest, SOURCE); + manager.getInjector().inject(dest, source); final var expected = LiteralArgumentBuilder .literal("hello") @@ -181,7 +177,7 @@ public class CommandGraphInjectorTests { .then(literal("baz")) .build(); dest.addChild(original); - manager.getInjector().inject(dest, SOURCE); + manager.getInjector().inject(dest, source); assertEquals(registered, dest.getChild("foo")); } 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 8bd9236f9..c93392379 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -10,37 +27,11 @@ import com.velocitypowered.api.command.BrigadierCommand; import com.velocitypowered.api.command.CommandSource; import com.velocitypowered.api.command.RawCommand; import com.velocitypowered.api.command.SimpleCommand; -import com.velocitypowered.proxy.event.MockEventManager; -import com.velocitypowered.proxy.event.VelocityEventManager; import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -public class CommandManagerTests { - - static final VelocityEventManager EVENT_MANAGER = new MockEventManager(); - - static { - Runtime.getRuntime().addShutdownHook(new Thread(() -> { - try { - EVENT_MANAGER.shutdown(); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - })); - } - - static VelocityCommandManager newManager() { - return new VelocityCommandManager(EVENT_MANAGER); - } - - private VelocityCommandManager manager; - - @BeforeEach - void setUp() { - this.manager = newManager(); - } +public class CommandManagerTests extends CommandTestSuite { // Registration @@ -156,6 +147,21 @@ public class CommandManagerTests { assertTrue(manager.hasCommand("foo")); } + // Execution + + @Test + void testExecuteUnknownAliasIsForwarded() { + assertForwarded(""); + assertForwarded("hello"); + } + + // Suggestions + + @Test + void testEmptyManagerSuggestNoAliases() { + assertSuggestions(""); + } + static final class DummyCommand implements SimpleCommand { static final DummyCommand INSTANCE = new DummyCommand(); diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandTestSuite.java b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandTestSuite.java index ff7748905..62ee06a8c 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandTestSuite.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandTestSuite.java @@ -1,3 +1,20 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -5,17 +22,38 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import com.velocitypowered.api.command.CommandSource; +import com.velocitypowered.proxy.event.MockEventManager; +import com.velocitypowered.proxy.event.VelocityEventManager; import java.util.Arrays; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; abstract class CommandTestSuite { + private static VelocityEventManager eventManager; + protected VelocityCommandManager manager; protected final CommandSource source = MockCommandSource.INSTANCE; + @BeforeAll + static void beforeAll() { + eventManager = new MockEventManager(); + } + + @AfterAll + static void afterAll() { + try { + eventManager.shutdown(); + eventManager = null; + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + @BeforeEach void setUp() { - this.manager = CommandManagerTests.newManager(); + this.manager = new VelocityCommandManager(eventManager); } final void assertHandled(final String input) { diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/OldCommandManagerTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/OldCommandManagerTests.java deleted file mode 100644 index 76202c833..000000000 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/OldCommandManagerTests.java +++ /dev/null @@ -1,495 +0,0 @@ -/* - * Copyright (C) 2018 Velocity Contributors - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - -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.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; -import com.mojang.brigadier.arguments.IntegerArgumentType; -import com.mojang.brigadier.arguments.StringArgumentType; -import com.mojang.brigadier.builder.LiteralArgumentBuilder; -import com.mojang.brigadier.builder.RequiredArgumentBuilder; -import com.mojang.brigadier.tree.ArgumentCommandNode; -import com.mojang.brigadier.tree.CommandNode; -import com.mojang.brigadier.tree.LiteralCommandNode; -import com.velocitypowered.api.command.BrigadierCommand; -import com.velocitypowered.api.command.CommandMeta; -import com.velocitypowered.api.command.CommandSource; -import com.velocitypowered.api.command.RawCommand; -import com.velocitypowered.api.command.SimpleCommand; -import com.velocitypowered.proxy.event.VelocityEventManager; -import com.velocitypowered.proxy.plugin.MockEventManager; -import java.util.List; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; -import org.junit.jupiter.api.Disabled; -import org.junit.jupiter.api.Test; - -@Disabled -public class OldCommandManagerTests { - - static VelocityCommandManager createManager() { - return new VelocityCommandManager(CommandManagerTests.EVENT_MANAGER); - } - - @Test - void testConstruction() { - VelocityCommandManager manager = createManager(); - assertFalse(manager.hasCommand("foo")); - assertTrue(manager.getRoot().getChildren().isEmpty()); - assertFalse(manager.executeAsync(MockCommandSource.INSTANCE, "foo").join()); - assertFalse(manager.executeImmediatelyAsync(MockCommandSource.INSTANCE, "bar").join()); - assertTrue(manager.offerSuggestions(MockCommandSource.INSTANCE, "").join().isEmpty()); - } - - @Test - void testBrigadierRegister() { - VelocityCommandManager manager = createManager(); - LiteralCommandNode node = LiteralArgumentBuilder - .literal("foo") - .build(); - BrigadierCommand command = new BrigadierCommand(node); - manager.register(command); - - assertEquals(node, command.getNode()); - assertTrue(manager.hasCommand("fOo")); - - LiteralCommandNode barNode = LiteralArgumentBuilder - .literal("bar") - .build(); - BrigadierCommand aliasesCommand = new BrigadierCommand(barNode); - CommandMeta meta = manager.metaBuilder(aliasesCommand) - .aliases("baZ") - .build(); - - assertEquals(ImmutableSet.of("bar", "baz"), meta.getAliases()); - assertTrue(meta.getHints().isEmpty()); - manager.register(meta, aliasesCommand); - assertTrue(manager.hasCommand("bAr")); - assertTrue(manager.hasCommand("Baz")); - } - - @Test - void testSimpleRegister() { - VelocityCommandManager manager = createManager(); - SimpleCommand command = new NoopSimpleCommand(); - - manager.register("Foo", command); - assertTrue(manager.hasCommand("foO")); - manager.unregister("fOo"); - assertFalse(manager.hasCommand("foo")); - assertFalse(manager.executeAsync(MockCommandSource.INSTANCE, "foo").join()); - - manager.register("foo", command, "bAr", "BAZ"); - assertTrue(manager.hasCommand("bar")); - assertTrue(manager.hasCommand("bAz")); - } - - @Test - void testRawRegister() { - VelocityCommandManager manager = createManager(); - RawCommand command = new NoopRawCommand(); - - manager.register("foO", command, "BAR"); - assertTrue(manager.hasCommand("fOo")); - assertTrue(manager.hasCommand("bar")); - } - - @Test - void testBrigadierExecute() { - VelocityCommandManager manager = createManager(); - AtomicBoolean executed = new AtomicBoolean(false); - AtomicBoolean checkedRequires = new AtomicBoolean(false); - LiteralCommandNode node = LiteralArgumentBuilder - .literal("buy") - .executes(context -> { - assertEquals(MockCommandSource.INSTANCE, context.getSource()); - assertEquals("buy", context.getInput()); - executed.set(true); - return 1; - }) - .build(); - CommandNode quantityNode = RequiredArgumentBuilder - .argument("quantity", IntegerArgumentType.integer(12, 16)) - .requires(source -> { - assertEquals(MockCommandSource.INSTANCE, source); - checkedRequires.set(true); - return true; - }) - .executes(context -> { - int argument = IntegerArgumentType.getInteger(context, "quantity"); - assertEquals(14, argument); - executed.set(true); - return 1; - }) - .build(); - CommandNode productNode = RequiredArgumentBuilder - .argument("product", StringArgumentType.string()) - .requires(source -> { - checkedRequires.set(true); - return false; - }) - .executes(context -> fail("was executed")) - .build(); - quantityNode.addChild(productNode); - node.addChild(quantityNode); - manager.register(new BrigadierCommand(node)); - - assertTrue(manager.executeAsync(MockCommandSource.INSTANCE, "buy ").join()); - assertTrue(executed.compareAndSet(true, false), "was executed"); - assertTrue(manager.executeImmediatelyAsync(MockCommandSource.INSTANCE, "buy 14").join()); - assertTrue(checkedRequires.compareAndSet(true, false)); - assertTrue(executed.get()); - assertTrue(manager.executeAsync(MockCommandSource.INSTANCE, "buy 9").join(), - "Invalid arg returns false"); - assertTrue(manager.executeImmediatelyAsync(MockCommandSource.INSTANCE, "buy 12 bananas") - .join()); - assertTrue(checkedRequires.get()); - } - - @Test - void testSimpleExecute() { - VelocityCommandManager manager = createManager(); - AtomicBoolean executed = new AtomicBoolean(false); - SimpleCommand command = invocation -> { - assertEquals(MockCommandSource.INSTANCE, invocation.source()); - assertArrayEquals(new String[] {"bar", "254"}, invocation.arguments()); - executed.set(true); - }; - manager.register("foo", command); - - assertTrue(manager.executeAsync(MockCommandSource.INSTANCE, "foo bar 254").join()); - assertTrue(executed.get()); - - SimpleCommand noPermsCommand = new SimpleCommand() { - @Override - public void execute(final Invocation invocation) { - fail("was executed"); - } - - @Override - public boolean hasPermission(final Invocation invocation) { - return false; - } - }; - - manager.register("dangerous", noPermsCommand, "veryDangerous"); - assertFalse(manager.executeAsync(MockCommandSource.INSTANCE, "dangerous").join()); - assertFalse(manager.executeImmediatelyAsync(MockCommandSource.INSTANCE, "verydangerous 123") - .join()); - } - - @Test - void testRawExecute() { - VelocityCommandManager manager = createManager(); - AtomicBoolean executed = new AtomicBoolean(false); - RawCommand command = new RawCommand() { - @Override - public void execute(final Invocation invocation) { - assertEquals(MockCommandSource.INSTANCE, invocation.source()); - assertEquals("lobby 23", invocation.arguments()); - executed.set(true); - } - }; - manager.register("sendMe", command); - - assertTrue(manager.executeImmediatelyAsync(MockCommandSource.INSTANCE, "sendMe lobby 23") - .join()); - assertTrue(executed.compareAndSet(true, false)); - - RawCommand noArgsCommand = new RawCommand() { - @Override - public void execute(final Invocation invocation) { - assertEquals("", invocation.arguments()); - executed.set(true); - } - }; - manager.register("noargs", noArgsCommand); - - assertTrue(manager.executeImmediatelyAsync(MockCommandSource.INSTANCE, "noargs").join()); - assertTrue(executed.get()); - assertTrue(manager.executeImmediatelyAsync(MockCommandSource.INSTANCE, "noargs ").join()); - - RawCommand noPermsCommand = new RawCommand() { - @Override - public void execute(final Invocation invocation) { - fail("was executed"); - } - - @Override - public boolean hasPermission(final Invocation invocation) { - return false; - } - }; - - manager.register("sendThem", noPermsCommand); - assertFalse(manager.executeImmediatelyAsync(MockCommandSource.INSTANCE, "sendThem foo") - .join()); - } - - @Test - void testSuggestions() { - VelocityCommandManager manager = createManager(); - - LiteralCommandNode brigadierNode = LiteralArgumentBuilder - .literal("brigadier") - .build(); - CommandNode nameNode = RequiredArgumentBuilder - .argument("name", StringArgumentType.string()) - .build(); - CommandNode numberNode = RequiredArgumentBuilder - .argument("quantity", IntegerArgumentType.integer()) - .suggests((context, builder) -> builder.suggest(2).suggest(3).buildFuture()) - .build(); - nameNode.addChild(numberNode); - brigadierNode.addChild(nameNode); - manager.register(new BrigadierCommand(brigadierNode)); - - SimpleCommand simpleCommand = new SimpleCommand() { - @Override - public void execute(final Invocation invocation) { - fail(); - } - - @Override - public List suggest(final Invocation invocation) { - switch (invocation.arguments().length) { - case 0: - return ImmutableList.of("foo", "bar"); - case 1: - return ImmutableList.of("123"); - default: - return ImmutableList.of(); - } - } - }; - manager.register("simple", simpleCommand); - - RawCommand rawCommand = new RawCommand() { - @Override - public void execute(final Invocation invocation) { - fail(); - } - - @Override - public List suggest(final Invocation invocation) { - switch (invocation.arguments()) { - case "": - return ImmutableList.of("foo", "baz"); - case "foo ": - return ImmutableList.of("2", "3", "5", "7"); - case "bar ": - return ImmutableList.of("11", "13", "17"); - default: - return ImmutableList.of(); - } - } - }; - manager.register("raw", rawCommand); - - assertEquals( - ImmutableList.of("brigadier", "raw", "simple"), - manager.offerSuggestions(MockCommandSource.INSTANCE, "").join(), - "literals are in alphabetical order"); - assertEquals( - ImmutableList.of("brigadier"), - manager.offerSuggestions(MockCommandSource.INSTANCE, "briga").join()); - assertTrue(manager.offerSuggestions(MockCommandSource.INSTANCE, "brigadier") - .join().isEmpty()); - assertTrue(manager.offerSuggestions(MockCommandSource.INSTANCE, "brigadier ") - .join().isEmpty()); - assertTrue(manager.offerSuggestions(MockCommandSource.INSTANCE, "brigadier foo") - .join().isEmpty()); - assertEquals( - ImmutableList.of("2", "3"), - manager.offerSuggestions(MockCommandSource.INSTANCE, "brigadier foo ").join()); - assertEquals( - ImmutableList.of("bar", "foo"), - manager.offerSuggestions(MockCommandSource.INSTANCE, "simple ").join()); - assertTrue(manager.offerSuggestions(MockCommandSource.INSTANCE, "simple") - .join().isEmpty()); - assertEquals( - ImmutableList.of("123"), - manager.offerSuggestions(MockCommandSource.INSTANCE, "simPle foo").join()); - assertEquals( - ImmutableList.of("baz", "foo"), - manager.offerSuggestions(MockCommandSource.INSTANCE, "raw ").join()); - assertEquals( - ImmutableList.of("2", "3", "5", "7"), - manager.offerSuggestions(MockCommandSource.INSTANCE, "raw foo ").join()); - assertTrue(manager.offerSuggestions(MockCommandSource.INSTANCE, "raw foo") - .join().isEmpty()); - assertEquals( - ImmutableList.of("11", "13", "17"), - manager.offerSuggestions(MockCommandSource.INSTANCE, "rAW bar ").join()); - } - - @Test - void testBrigadierSuggestionPermissions() { - VelocityCommandManager manager = createManager(); - LiteralCommandNode manageNode = LiteralArgumentBuilder - .literal("manage") - .requires(source -> false) - .build(); - CommandNode idNode = RequiredArgumentBuilder - .argument("id", IntegerArgumentType.integer(0)) - .suggests((context, builder) -> fail("called suggestion builder")) - .build(); - manageNode.addChild(idNode); - manager.register(new BrigadierCommand(manageNode)); - - // Brigadier doesn't call the children predicate when requesting suggestions. - // However, it won't query children if the source doesn't pass the parent - // #requires predicate. - assertTrue(manager.offerSuggestions(MockCommandSource.INSTANCE, "manage ") - .join().isEmpty()); - } - - @Test - @Disabled - void testHinting() { - VelocityCommandManager manager = createManager(); - AtomicBoolean executed = new AtomicBoolean(false); - AtomicBoolean calledSuggestionProvider = new AtomicBoolean(false); - AtomicReference expectedArgs = new AtomicReference<>(); - RawCommand command = new RawCommand() { - @Override - public void execute(final Invocation invocation) { - assertEquals(expectedArgs.get(), invocation.arguments()); - executed.set(true); - } - - @Override - public List suggest(final Invocation invocation) { - return ImmutableList.of("raw"); - } - }; - - CommandNode barHint = LiteralArgumentBuilder - .literal("bar") - .executes(context -> fail("hints don't get executed")) - .build(); - ArgumentCommandNode numberArg = RequiredArgumentBuilder - .argument("number", IntegerArgumentType.integer()) - .suggests((context, builder) -> { - calledSuggestionProvider.set(true); - return builder.suggest("456").buildFuture(); - }) - .build(); - barHint.addChild(numberArg); - CommandNode bazHint = LiteralArgumentBuilder - .literal("baz") - .build(); - CommandMeta meta = manager.metaBuilder("foo") - .aliases("foo2") - .hint(barHint) - .hint(bazHint) - .build(); - manager.register(meta, command); - - expectedArgs.set("notBarOrBaz"); - assertTrue(manager.executeAsync(MockCommandSource.INSTANCE, "foo notBarOrBaz").join()); - assertTrue(executed.compareAndSet(true, false)); - expectedArgs.set("anotherArg 123"); - assertTrue(manager.executeAsync(MockCommandSource.INSTANCE, "Foo2 anotherArg 123").join()); - assertTrue(executed.compareAndSet(true, false)); - expectedArgs.set("bar"); - assertTrue(manager.executeAsync(MockCommandSource.INSTANCE, "foo bar").join()); - assertTrue(executed.compareAndSet(true, false)); - expectedArgs.set("bar 123"); - assertTrue(manager.executeAsync(MockCommandSource.INSTANCE, "foo2 bar 123").join()); - assertTrue(executed.compareAndSet(true, false)); - - assertEquals(ImmutableList.of("bar", "baz", "raw"), - manager.offerSuggestions(MockCommandSource.INSTANCE, "foo ").join()); - assertFalse(calledSuggestionProvider.get()); - assertEquals(ImmutableList.of("456"), - manager.offerSuggestions(MockCommandSource.INSTANCE, "foo bar ").join()); - assertTrue(calledSuggestionProvider.compareAndSet(true, false)); - assertEquals(ImmutableList.of(), - manager.offerSuggestions(MockCommandSource.INSTANCE, "foo2 baz ").join()); - } - - @Test - void testSuggestionPermissions() throws ExecutionException, InterruptedException { - VelocityCommandManager manager = createManager(); - RawCommand rawCommand = new RawCommand() { - @Override - public void execute(final Invocation invocation) { - fail("The Command should not be executed while testing suggestions"); - } - - @Override - public boolean hasPermission(Invocation invocation) { - return invocation.arguments().length() > 0; - } - - @Override - public List suggest(final Invocation invocation) { - return ImmutableList.of("suggestion"); - } - }; - - manager.register("foo", rawCommand); - - assertTrue(manager.offerSuggestions(MockCommandSource.INSTANCE, "foo").get().isEmpty()); - assertFalse(manager.offerSuggestions(MockCommandSource.INSTANCE, "foo bar").get().isEmpty()); - - SimpleCommand oldCommand = new SimpleCommand() { - @Override - public void execute(Invocation invocation) { - fail("The Command should not be executed while testing suggestions"); - } - - @Override - public boolean hasPermission(Invocation invocation) { - return invocation.arguments().length > 0; - } - - @Override - public List suggest(Invocation invocation) { - return ImmutableList.of("suggestion"); - } - }; - - manager.register("bar", oldCommand); - - assertTrue(manager.offerSuggestions(MockCommandSource.INSTANCE, "bar").get().isEmpty()); - assertFalse(manager.offerSuggestions(MockCommandSource.INSTANCE, "bar foo").get().isEmpty()); - } - - static class NoopSimpleCommand implements SimpleCommand { - @Override - public void execute(final Invocation invocation) { - - } - } - - static class NoopRawCommand implements RawCommand { - @Override - public void execute(final Invocation invocation) { - - } - } -} \ No newline at end of file diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java index 3f8ff1bfe..2b03ace8d 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java @@ -1,12 +1,33 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command; +import static com.mojang.brigadier.arguments.StringArgumentType.word; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; import com.google.common.collect.ImmutableList; +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import com.mojang.brigadier.builder.RequiredArgumentBuilder; import com.spotify.futures.CompletableFutures; +import com.velocitypowered.api.command.CommandSource; import com.velocitypowered.api.command.RawCommand; import com.velocitypowered.api.command.SimpleCommand; import java.util.Collections; @@ -36,6 +57,47 @@ public class RawCommandTests extends CommandTestSuite { assertEquals(1, callCount.get()); } + @Test + void testExecuteIgnoresAliasCase() { + final var callCount = new AtomicInteger(); + + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, (RawCommand) invocation -> { + assertEquals("hello", invocation.alias()); + callCount.incrementAndGet(); + }); + + assertHandled("Hello"); + assertEquals(1, callCount.get()); + } + + @Test + void testExecuteInputIsTrimmed() { + final var callCount = new AtomicInteger(); + + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, (RawCommand) invocation -> { + assertEquals("hello", invocation.alias()); + assertEquals("", invocation.arguments()); + callCount.incrementAndGet(); + }); + + assertHandled(" hello"); + assertHandled(" hello"); + assertHandled("hello "); + assertHandled("hello "); + assertEquals(4, callCount.get()); + } + + @Test + void testExecuteAfterUnregisterForwards() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, (RawCommand) invocation -> fail()); + manager.unregister("hello"); + + assertForwarded("hello"); + } + @Test void testForwardsAndDoesNotExecuteImpermissibleAlias() { final var callCount = new AtomicInteger(); @@ -125,6 +187,25 @@ public class RawCommandTests extends CommandTestSuite { }); } + @Test + void testDoesNotSuggestAliasAfterUnregister() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + return fail(); + } + }); + manager.unregister("hello"); + + assertSuggestions(""); + } + @Test void testSuggestsArgumentsAfterAlias() { final var meta = manager.metaBuilder("hello").build(); @@ -301,4 +382,105 @@ public class RawCommandTests extends CommandTestSuite { assertThrows(CompletionException.class, () -> manager.offerSuggestions(source, "hello ").join()); } + + // Hinting + + // Even if the following 2 cases look really similar, they test + // different parts of SuggestionsProvider. + @Test + void testDoesNotSuggestHintIfImpermissibleAlias() { + final var hint = LiteralArgumentBuilder + .literal("hint") + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public boolean hasPermission(final Invocation invocation) { + return false; + } + }); + + assertSuggestions("hello "); + } + + @Test + void testDoesNotSuggestHintIfImpermissibleArguments() { + final var hint = LiteralArgumentBuilder + .literal("hint") + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public boolean hasPermission(final Invocation invocation) { + return false; + } + }); + + assertSuggestions("hello hin"); + } + + @Test + void testSuggestsMergesIgnoringHintsWhoseCustomSuggestionProviderFutureCompletesExceptionally() { + final var hint = RequiredArgumentBuilder + .argument("hint", word()) + .suggests((context, builder) -> + CompletableFutures.exceptionallyCompletedFuture(new RuntimeException())) + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + return ImmutableList.of("world"); + } + }); + + assertSuggestions("hello ", "world"); + } + + @Test + void testSuggestsMergesIgnoringHintsWhoseCustomSuggestionProviderThrows() { + final var hint = RequiredArgumentBuilder + .argument("hint", word()) + .suggests((context, builder) -> { + throw new RuntimeException(); + }) + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + return ImmutableList.of("world"); + } + }); + + assertSuggestions("hello ", "world"); + } } diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java index e4ccec57d..644b6a31e 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java @@ -1,12 +1,33 @@ +/* + * Copyright (C) 2018 Velocity Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + package com.velocitypowered.proxy.command; +import static com.mojang.brigadier.arguments.StringArgumentType.word; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; import com.google.common.collect.ImmutableList; +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import com.mojang.brigadier.builder.RequiredArgumentBuilder; import com.spotify.futures.CompletableFutures; +import com.velocitypowered.api.command.CommandSource; import com.velocitypowered.api.command.SimpleCommand; import java.util.Collections; import java.util.List; @@ -35,6 +56,47 @@ public class SimpleCommandTests extends CommandTestSuite { assertEquals(1, callCount.get()); } + @Test + void testExecuteIgnoresAliasCase() { + final var callCount = new AtomicInteger(); + + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, (SimpleCommand) invocation -> { + assertEquals("hello", invocation.alias()); + callCount.incrementAndGet(); + }); + + assertHandled("Hello"); + assertEquals(1, callCount.get()); + } + + @Test + void testExecuteInputIsTrimmed() { + final var callCount = new AtomicInteger(); + + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, (SimpleCommand) invocation -> { + assertEquals("hello", invocation.alias()); + assertArrayEquals(new String[0], invocation.arguments()); + callCount.incrementAndGet(); + }); + + assertHandled(" hello"); + assertHandled(" hello"); + assertHandled("hello "); + assertHandled("hello "); + assertEquals(4, callCount.get()); + } + + @Test + void testExecuteAfterUnregisterForwards() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, (SimpleCommand) invocation -> fail()); + manager.unregister("hello"); + + assertForwarded("hello"); + } + @Test void testForwardsAndDoesNotExecuteImpermissibleAlias() { final var callCount = new AtomicInteger(); @@ -124,6 +186,25 @@ public class SimpleCommandTests extends CommandTestSuite { }); } + @Test + void testDoesNotSuggestAliasAfterUnregister() { + final var meta = manager.metaBuilder("hello").build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + return fail(); + } + }); + manager.unregister("hello"); + + assertSuggestions(""); + } + @Test void testSuggestsArgumentsAfterAlias() { final var meta = manager.metaBuilder("hello").build(); @@ -300,4 +381,105 @@ public class SimpleCommandTests extends CommandTestSuite { assertThrows(CompletionException.class, () -> manager.offerSuggestions(source, "hello ").join()); } + + // Hinting + + // Even if the following 2 cases look really similar, they test + // different parts of SuggestionsProvider. + @Test + void testDoesNotSuggestHintIfImpermissibleAlias() { + final var hint = LiteralArgumentBuilder + .literal("hint") + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public boolean hasPermission(final Invocation invocation) { + return false; + } + }); + + assertSuggestions("hello "); + } + + @Test + void testDoesNotSuggestHintIfImpermissibleArguments() { + final var hint = LiteralArgumentBuilder + .literal("hint") + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public boolean hasPermission(final Invocation invocation) { + return false; + } + }); + + assertSuggestions("hello hin"); + } + + @Test + void testSuggestsMergesIgnoringHintsWhoseCustomSuggestionProviderFutureCompletesExceptionally() { + final var hint = RequiredArgumentBuilder + .argument("hint", word()) + .suggests((context, builder) -> + CompletableFutures.exceptionallyCompletedFuture(new RuntimeException())) + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + return ImmutableList.of("world"); + } + }); + + assertSuggestions("hello ", "world"); + } + + @Test + void testSuggestsMergesIgnoringHintsWhoseCustomSuggestionProviderThrows() { + final var hint = RequiredArgumentBuilder + .argument("hint", word()) + .suggests((context, builder) -> { + throw new RuntimeException(); + }) + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, new SimpleCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public List suggest(final Invocation invocation) { + return ImmutableList.of("world"); + } + }); + + assertSuggestions("hello ", "world"); + } } diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java index dd61069bc..37a582362 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java @@ -18,22 +18,16 @@ package com.velocitypowered.proxy.command; import static com.mojang.brigadier.arguments.StringArgumentType.word; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; import com.google.common.collect.ImmutableList; import com.mojang.brigadier.builder.LiteralArgumentBuilder; import com.mojang.brigadier.builder.RequiredArgumentBuilder; import com.spotify.futures.CompletableFutures; -import com.velocitypowered.api.command.BrigadierCommand; import com.velocitypowered.api.command.Command; import com.velocitypowered.api.command.CommandSource; import com.velocitypowered.api.command.RawCommand; -import com.velocitypowered.api.command.SimpleCommand; import java.util.List; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionException; import org.junit.jupiter.api.Test; /** @@ -196,22 +190,24 @@ public class SuggestionsProviderTests extends CommandTestSuite { assertSuggestions("foo ", "bar", "baz", "qux"); assertSuggestions("foo bar", "baz", "qux"); - assertSuggestions("foo baz", "bar", "qux"); + assertSuggestions("foo baz", "qux"); } + // Hints are suggested iff the source can use the given arguments; see + // VelocityCommandMeta#copyHints. @Test - // This doesn't make much sense, but emulates Brigadier behavior - void testSuggestsImpermissibleHint() { - final var hint = LiteralArgumentBuilder - .literal("hint") - .requires(source1 -> false) + void testSuggestIgnoresHintRequirementPredicateResults() { + final var hint = RequiredArgumentBuilder + .argument("hint", word()) + .requires(source1 -> fail()) + .suggests((context, builder) -> builder.suggest("suggestion").buildFuture()) .build(); final var meta = manager.metaBuilder("hello") .hint(hint) .build(); manager.register(meta, NoSuggestionsCommand.INSTANCE); - assertSuggestions("hello ", "hint"); + assertSuggestions("hello ", "suggestion"); } @Test @@ -245,51 +241,6 @@ public class SuggestionsProviderTests extends CommandTestSuite { assertSuggestions("hello "); } - @Test - void testSuggestCompletesExceptionallyIfHintRequirementPredicateThrows() { - final var hint = RequiredArgumentBuilder - .argument("hint", word()) - .requires(source1 -> { - throw new RuntimeException(); - }) - .suggests((context, builder) -> fail()) - .build(); - final var meta = manager.metaBuilder("hello") - .hint(hint) - .build(); - manager.register(meta, NoSuggestionsCommand.INSTANCE); - - assertThrows(CompletionException.class, () -> - manager.offerSuggestions(source, "hello ").join()); - } - - /* - - @Test - void testSuggestionMergingIgnoresExceptionallyCompletedSuggestionFutures() { - final var hint = RequiredArgumentBuilder - .argument("hint", word()) - .suggests((context, builder) -> - CompletableFutures.exceptionallyCompletedFuture(new RuntimeException())) - .build(); - final var meta = manager.metaBuilder("hello") - .hint(hint) - .build(); - manager.register(meta, new RawCommand() { - @Override - public void execute(final Invocation invocation) { - fail(); - } - - @Override - public List suggest(final Invocation invocation) { - return ImmutableList.of("world"); - } - }); - - assertSuggestions("hello ", "world"); - }*/ - static final class NoSuggestionsCommand implements RawCommand { static final NoSuggestionsCommand INSTANCE = new NoSuggestionsCommand(); From fb7aafe8aee67773958beb0a2834e436a877b3da Mon Sep 17 00:00:00 2001 From: Hugo Manrique Date: Thu, 10 Jun 2021 20:25:52 +0200 Subject: [PATCH 11/13] Fix Brigadier command alias redirects --- .../proxy/command/CommandGraphInjector.java | 8 +++--- .../proxy/command/SuggestionsProvider.java | 2 +- .../proxy/command/VelocityCommandManager.java | 25 +++++++++++++------ .../command/CommandGraphInjectorTests.java | 7 ++++-- 4 files changed, 27 insertions(+), 15 deletions(-) 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 015501a12..0d692891a 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/CommandGraphInjector.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/CommandGraphInjector.java @@ -88,9 +88,7 @@ public final class CommandGraphInjector { final VelocityArgumentCommandNode argsNode = 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 literal is associated to a BrigadierCommand, filter normally. this.copyChildren(node, copy, source); } else { // Copy all children nodes (arguments node and hints) @@ -116,7 +114,9 @@ public final class CommandGraphInjector { } final ArgumentBuilder builder = node.createBuilder(); if (node.getRedirect() != null) { - // TODO Document redirects to non-Brigadier commands are not supported + // Redirects to non-Brigadier commands are not supported. Luckily, + // we don't expose the root node to API users, so they can't access + // nodes associated to other commands. final CommandNode target = this.filterNode(node.getRedirect(), source); builder.forward(target, builder.getRedirectModifier(), builder.isFork()); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java index e6af2d62b..a4faf9fd0 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java @@ -145,7 +145,7 @@ final class SuggestionsProvider { final StringReader reader, final CommandContextBuilder contextSoFar) { final S source = contextSoFar.getSource(); // Lowercase the alias here so all comparisons can be case-sensitive (cheaper) - // TODO Is this actually faster? + // TODO Is this actually faster? It may incur an allocation final String input = reader.getRead().toLowerCase(Locale.ENGLISH); final Collection> aliases = contextSoFar.getRootNode().getChildren(); 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 04fe0cda8..d22115a48 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java @@ -99,29 +99,38 @@ 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. + // TODO Warn if command implements multiple registrable interfaces? for (final CommandRegistrar registrar : this.registrars) { if (this.tryRegister(registrar, command, meta)) { - return; + return; // success } } throw new IllegalArgumentException( command + " does not implement a registrable Command subinterface"); } + /** + * Attempts to register the given command if it implements the + * {@linkplain CommandRegistrar#registrableSuperInterface() registrable superinterface} + * of the given registrar. + * + * @param registrar the registrar to register the command + * @param command the command to register + * @param meta the command metadata + * @param the type of the command + * @return true if the command implements the registrable superinterface of the registrar; + * false otherwise. + * @throws IllegalArgumentException if the registrar cannot register the command + */ private boolean tryRegister(final CommandRegistrar registrar, final Command command, final CommandMeta meta) { final Class superInterface = registrar.registrableSuperInterface(); if (!superInterface.isInstance(command)) { return false; } - try { - registrar.register(meta, superInterface.cast(command)); - return true; - } catch (final IllegalArgumentException ignored) { - return false; - } + registrar.register(meta, superInterface.cast(command)); + return true; } @Override 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 92a5b8b19..6f854aa96 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java @@ -119,11 +119,11 @@ public class CommandGraphInjectorTests extends CommandTestSuite { } @Test - void testBrigadierCommandAliasRedirectsNotAllowed() { + void testInjectPreservesBrigadierCommandAliasRedirect() { final var registered = LiteralArgumentBuilder .literal("origin") .redirect(LiteralArgumentBuilder - .literal("target") + .literal("target") .build()) .build(); manager.register(new BrigadierCommand(registered)); @@ -131,6 +131,9 @@ public class CommandGraphInjectorTests extends CommandTestSuite { final var expected = LiteralArgumentBuilder .literal("origin") + .redirect(LiteralArgumentBuilder + .literal("target") + .build()) .build(); assertEquals(expected, dest.getChild("origin")); } From f6e6f02a84d1c084bdaa384349c9f620b5a80c6b Mon Sep 17 00:00:00 2001 From: Hugo Manrique Date: Thu, 10 Jun 2021 20:35:36 +0200 Subject: [PATCH 12/13] Fix checkstyle violations --- .../proxy/command/SuggestionsProvider.java | 3 --- .../proxy/command/VelocityCommandManager.java | 3 ++- .../command/brigadier/VelocityArgumentBuilder.java | 12 +++++++++++- .../brigadier/VelocityArgumentCommandNode.java | 2 +- .../proxy/command/CommandManagerTests.java | 2 +- .../brigadier/StringArrayArgumentTypeTests.java | 6 +++--- .../brigadier/VelocityArgumentCommandNodeTests.java | 6 +++++- 7 files changed, 23 insertions(+), 11 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java index a4faf9fd0..a2ca7588c 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java @@ -285,9 +285,6 @@ final class SuggestionsProvider { * Parses the hint nodes under the given node, which is either an alias node of * a {@link Command} or another hint node. * - * The caller must check the requirements - * are satisfied by a given source prior to calling this method. - * *

The reader and context are not mutated by this method. * * @param node the node to parse 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 d22115a48..3683c2cf2 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java @@ -220,7 +220,8 @@ public class VelocityCommandManager implements CommandManager { final String normalizedInput = VelocityCommands.normalizeInput(cmdLine, false); try { return suggestionsProvider.provideSuggestions(normalizedInput, source) - .thenApply(suggestions -> Lists.transform(suggestions.getList(), Suggestion::getText)); + .thenApply(suggestions -> + Lists.transform(suggestions.getList(), Suggestion::getText)); } catch (final Throwable e) { // Again, plugins are naughty return CompletableFutures.exceptionallyCompletedFuture( diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentBuilder.java b/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentBuilder.java index d3f2506de..62e8c8b93 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentBuilder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentBuilder.java @@ -33,6 +33,16 @@ import org.checkerframework.checker.nullness.qual.Nullable; public final class VelocityArgumentBuilder extends ArgumentBuilder> { + /** + * Creates a builder for creating {@link VelocityArgumentCommandNode}s with + * the given name and type. + * + * @param name the name of the node + * @param type the type of the argument to parse + * @param the type of the command source + * @param the type of the argument to parse + * @return a builder + */ public static VelocityArgumentBuilder velocityArgument(final String name, final ArgumentType type) { Preconditions.checkNotNull(name, "name"); @@ -44,7 +54,7 @@ public final class VelocityArgumentBuilder private final ArgumentType type; private SuggestionProvider suggestionsProvider = null; - public VelocityArgumentBuilder(final String name, final ArgumentType type) { + private VelocityArgumentBuilder(final String name, final ArgumentType type) { this.name = name; this.type = type; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNode.java b/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNode.java index 262621931..d0bb4f6ec 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNode.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNode.java @@ -51,7 +51,7 @@ public class VelocityArgumentCommandNode extends ArgumentCommandNode type; - public VelocityArgumentCommandNode( + VelocityArgumentCommandNode( final String name, final ArgumentType type, final Command command, final Predicate requirement, final BiPredicate, ImmutableStringReader> contextRequirement, 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 c93392379..e977c12f0 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandManagerTests.java @@ -85,7 +85,7 @@ public class CommandManagerTests extends CommandTestSuite { final var oldMeta = manager.metaBuilder("foo").build(); manager.register(oldMeta, DummyCommand.INSTANCE); // fails on execution final var newMeta = manager.metaBuilder("foo").build(); - manager.register("foo", (RawCommand) invocation -> called.set(true)); + manager.register(newMeta, (RawCommand) invocation -> called.set(true)); manager.executeAsync(MockCommandSource.INSTANCE, "foo").join(); assertTrue(called.get()); diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentTypeTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentTypeTests.java index d33d4a16b..80740ac2a 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentTypeTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/StringArrayArgumentTypeTests.java @@ -93,10 +93,10 @@ public class StringArrayArgumentTypeTests { @Test void testMultipleWhitespaceCharsArePreserved() throws CommandSyntaxException { final StringReader reader = new StringReader( - " This is a message that shouldn't be normalized "); + " This is a message that shouldn't be normalized "); assertArrayEquals(new String[] { - "", "This", "", "is", "a", "", "", "message", "", "that", "shouldn't", "", "", "", "be", - "normalized", "", ""}, TYPE.parse(reader)); + "", "This", "", "is", "a", "", "", "message", "", "that", "shouldn't", "", "", "", "be", + "normalized", "", ""}, TYPE.parse(reader)); assertFalse(reader.canRead()); } diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNodeTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNodeTests.java index b61d06531..1c1d6084b 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNodeTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/brigadier/VelocityArgumentCommandNodeTests.java @@ -18,7 +18,11 @@ package com.velocitypowered.proxy.command.brigadier; import static com.velocitypowered.proxy.command.brigadier.VelocityArgumentBuilder.velocityArgument; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; import com.mojang.brigadier.CommandDispatcher; import com.mojang.brigadier.StringReader; From 61480544f5903143f5db0deba5b65f4ad00f4d16 Mon Sep 17 00:00:00 2001 From: Hugo Manrique Date: Fri, 11 Jun 2021 14:03:38 +0200 Subject: [PATCH 13/13] Apply suggested future-related changes Co-authored-by: A248 --- .../proxy/command/SuggestionsProvider.java | 6 ++- .../proxy/command/VelocityCommandManager.java | 2 +- .../proxy/command/BrigadierCommandTests.java | 4 +- .../command/CommandGraphInjectorTests.java | 45 +++++++++++++++++++ .../proxy/command/RawCommandTests.java | 8 +--- .../proxy/command/SimpleCommandTests.java | 6 +-- .../command/SuggestionsProviderTests.java | 5 +-- 7 files changed, 59 insertions(+), 17 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java index a2ca7588c..24ef2430a 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java @@ -29,6 +29,7 @@ import com.mojang.brigadier.suggestion.Suggestions; import com.mojang.brigadier.suggestion.SuggestionsBuilder; import com.mojang.brigadier.tree.CommandNode; import com.mojang.brigadier.tree.LiteralCommandNode; +import com.spotify.futures.CompletableFutures; import com.velocitypowered.api.command.Command; import com.velocitypowered.api.command.CommandMeta; import com.velocitypowered.api.command.CommandSource; @@ -357,7 +358,10 @@ final class SuggestionsProvider { return CompletableFuture.allOf(futures).handle((unused, throwable) -> { final List suggestions = new ArrayList<>(futures.length); for (final CompletableFuture future : futures) { - if (!future.isCompletedExceptionally()) { + if (future.isCompletedExceptionally()) { + final Throwable exception = CompletableFutures.getException(future); + LOGGER.error("Node cannot provide suggestions", exception); + } else { suggestions.add(future.join()); } } 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 3683c2cf2..b2951590b 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java @@ -224,7 +224,7 @@ public class VelocityCommandManager implements CommandManager { Lists.transform(suggestions.getList(), Suggestion::getText)); } catch (final Throwable e) { // Again, plugins are naughty - return CompletableFutures.exceptionallyCompletedFuture( + return CompletableFuture.failedFuture( new RuntimeException("Unable to provide suggestions for " + cmdLine + " for " + source, e)); } } diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java index b1fec4155..1e6cb706a 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/BrigadierCommandTests.java @@ -27,9 +27,9 @@ import static org.junit.jupiter.api.Assertions.fail; import com.mojang.brigadier.builder.LiteralArgumentBuilder; import com.mojang.brigadier.builder.RequiredArgumentBuilder; -import com.spotify.futures.CompletableFutures; import com.velocitypowered.api.command.BrigadierCommand; import com.velocitypowered.api.command.CommandSource; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; @@ -308,7 +308,7 @@ public class BrigadierCommandTests extends CommandTestSuite { .then(RequiredArgumentBuilder .argument("child", word()) .suggests((context, builder) -> - CompletableFutures.exceptionallyCompletedFuture(new RuntimeException()))) + CompletableFuture.failedFuture(new RuntimeException()))) .build(); manager.register(new BrigadierCommand(node)); 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 6f854aa96..3f86b8c4a 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandGraphInjectorTests.java @@ -30,6 +30,7 @@ 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.RawCommand; import com.velocitypowered.api.command.SimpleCommand; import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.BeforeEach; @@ -82,6 +83,50 @@ public class CommandGraphInjectorTests extends CommandTestSuite { assertEquals(1, callCount.get()); } + @Test + void testInjectsHintsOfInvocableCommand() { + final var hint = LiteralArgumentBuilder + .literal("hint") + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, (SimpleCommand) invocation -> fail()); + manager.getInjector().inject(dest, source); + + // Preserves hint node + final var expected = manager.getRoot(); + assertEquals(expected, dest); + } + + @Test + void testFiltersHintsOfImpermissibleAlias() { + final var callCount = new AtomicInteger(); + + final var hint = LiteralArgumentBuilder + .literal("hint") + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.register(meta, new RawCommand() { + @Override + public void execute(final Invocation invocation) { + fail(); + } + + @Override + public boolean hasPermission(final Invocation invocation) { + callCount.incrementAndGet(); + return false; + } + }); + manager.getInjector().inject(dest, source); + + assertTrue(dest.getChildren().isEmpty()); + assertEquals(1, callCount.get()); // does not call hasPermission for hints + } + @Test void testInjectsBrigadierCommand() { final LiteralCommandNode node = LiteralArgumentBuilder diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java index 2b03ace8d..1c92c7787 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/RawCommandTests.java @@ -18,7 +18,6 @@ package com.velocitypowered.proxy.command; import static com.mojang.brigadier.arguments.StringArgumentType.word; -import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; @@ -26,10 +25,8 @@ import static org.junit.jupiter.api.Assertions.fail; import com.google.common.collect.ImmutableList; import com.mojang.brigadier.builder.LiteralArgumentBuilder; import com.mojang.brigadier.builder.RequiredArgumentBuilder; -import com.spotify.futures.CompletableFutures; import com.velocitypowered.api.command.CommandSource; import com.velocitypowered.api.command.RawCommand; -import com.velocitypowered.api.command.SimpleCommand; import java.util.Collections; import java.util.List; import java.util.concurrent.CompletableFuture; @@ -333,7 +330,7 @@ public class RawCommandTests extends CommandTestSuite { @Override public CompletableFuture> suggestAsync(final Invocation invocation) { - return CompletableFutures.exceptionallyCompletedFuture(new RuntimeException()); + return CompletableFuture.failedFuture(new RuntimeException()); } }); @@ -437,8 +434,7 @@ public class RawCommandTests extends CommandTestSuite { void testSuggestsMergesIgnoringHintsWhoseCustomSuggestionProviderFutureCompletesExceptionally() { final var hint = RequiredArgumentBuilder .argument("hint", word()) - .suggests((context, builder) -> - CompletableFutures.exceptionallyCompletedFuture(new RuntimeException())) + .suggests((context, builder) -> CompletableFuture.failedFuture(new RuntimeException())) .build(); final var meta = manager.metaBuilder("hello") .hint(hint) diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java index 644b6a31e..a6471bf45 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/SimpleCommandTests.java @@ -26,7 +26,6 @@ import static org.junit.jupiter.api.Assertions.fail; import com.google.common.collect.ImmutableList; import com.mojang.brigadier.builder.LiteralArgumentBuilder; import com.mojang.brigadier.builder.RequiredArgumentBuilder; -import com.spotify.futures.CompletableFutures; import com.velocitypowered.api.command.CommandSource; import com.velocitypowered.api.command.SimpleCommand; import java.util.Collections; @@ -332,7 +331,7 @@ public class SimpleCommandTests extends CommandTestSuite { @Override public CompletableFuture> suggestAsync(final Invocation invocation) { - return CompletableFutures.exceptionallyCompletedFuture(new RuntimeException()); + return CompletableFuture.failedFuture(new RuntimeException()); } }); @@ -436,8 +435,7 @@ public class SimpleCommandTests extends CommandTestSuite { void testSuggestsMergesIgnoringHintsWhoseCustomSuggestionProviderFutureCompletesExceptionally() { final var hint = RequiredArgumentBuilder .argument("hint", word()) - .suggests((context, builder) -> - CompletableFutures.exceptionallyCompletedFuture(new RuntimeException())) + .suggests((context, builder) -> CompletableFuture.failedFuture(new RuntimeException())) .build(); final var meta = manager.metaBuilder("hello") .hint(hint) diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java index 37a582362..3a93c2b4c 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java @@ -23,11 +23,11 @@ import static org.junit.jupiter.api.Assertions.fail; import com.google.common.collect.ImmutableList; import com.mojang.brigadier.builder.LiteralArgumentBuilder; import com.mojang.brigadier.builder.RequiredArgumentBuilder; -import com.spotify.futures.CompletableFutures; import com.velocitypowered.api.command.Command; import com.velocitypowered.api.command.CommandSource; import com.velocitypowered.api.command.RawCommand; import java.util.List; +import java.util.concurrent.CompletableFuture; import org.junit.jupiter.api.Test; /** @@ -214,8 +214,7 @@ public class SuggestionsProviderTests extends CommandTestSuite { void testDoesNotSuggestHintIfHintSuggestionProviderFutureCompletesExceptionally() { final var hint = RequiredArgumentBuilder .argument("hint", word()) - .suggests((context, builder) -> - CompletableFutures.exceptionallyCompletedFuture(new RuntimeException())) + .suggests((context, builder) -> CompletableFuture.failedFuture(new RuntimeException())) .build(); final var meta = manager.metaBuilder("hello") .hint(hint)