Apply suggested future-related changes

Co-authored-by: A248 <theanandbeh@gmail.com>

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<S> {
     return CompletableFuture.allOf(futures).handle((unused, throwable) -> {
       final List<Suggestions> suggestions = new ArrayList<>(futures.length);
       for (final CompletableFuture<Suggestions> 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());
         }
       }

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

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
                     .<CommandSource, String>argument("child", word())
                     .suggests((context, builder) ->
-                          CompletableFutures.exceptionallyCompletedFuture(new RuntimeException())))
+                        CompletableFuture.failedFuture(new RuntimeException())))
             .build();
     manager.register(new BrigadierCommand(node));
 

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
+            .<CommandSource>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
+            .<CommandSource>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<CommandSource> node = LiteralArgumentBuilder

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<List<String>> 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
             .<CommandSource, String>argument("hint", word())
-            .suggests((context, builder) ->
+            .suggests((context, builder) -> CompletableFuture.failedFuture(new RuntimeException()))
-                    CompletableFutures.exceptionallyCompletedFuture(new RuntimeException()))
             .build();
     final var meta = manager.metaBuilder("hello")
             .hint(hint)

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<List<String>> 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
             .<CommandSource, String>argument("hint", word())
-            .suggests((context, builder) ->
+            .suggests((context, builder) -> CompletableFuture.failedFuture(new RuntimeException()))
-                    CompletableFutures.exceptionallyCompletedFuture(new RuntimeException()))
             .build();
     final var meta = manager.metaBuilder("hello")
             .hint(hint)

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
             .<CommandSource, String>argument("hint", word())
-            .suggests((context, builder) ->
+            .suggests((context, builder) -> CompletableFuture.failedFuture(new RuntimeException()))
-                CompletableFutures.exceptionallyCompletedFuture(new RuntimeException()))
             .build();
     final var meta = manager.metaBuilder("hello")
             .hint(hint)