From 61480544f5903143f5db0deba5b65f4ad00f4d16 Mon Sep 17 00:00:00 2001 From: Hugo Manrique Date: Fri, 11 Jun 2021 14:03:38 +0200 Subject: [PATCH] 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)