From 86367b74cba655fd5acfa43c3c5872d6041ce689 Mon Sep 17 00:00:00 2001 From: Tijs Date: Fri, 30 Sep 2022 15:49:09 +0200 Subject: [PATCH 1/2] Bugfix In BotCore: Removed requireSlashCommand, and added requireUserInteractor. This works based on generics, very useful. Also added UserInteractorPrefix#getPrefixedName(String) --- .../tjbot/commands/UserInteractorPrefix.java | 10 ++++ .../tjbot/commands/system/BotCore.java | 57 +++++++++++++------ 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/UserInteractorPrefix.java b/application/src/main/java/org/togetherjava/tjbot/commands/UserInteractorPrefix.java index e4537853e7..5af1a4b527 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/UserInteractorPrefix.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/UserInteractorPrefix.java @@ -36,6 +36,16 @@ public String getPrefix() { return prefix; } + /** + * Returns the name, attached with the prefix in front of it. + * + * @param name the name + * @return the name, with the prefix in front of it. + */ + public String getPrefixedName(String name) { + return prefix + name; + } + /** * The class type that should receive the prefix * diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/system/BotCore.java b/application/src/main/java/org/togetherjava/tjbot/commands/system/BotCore.java index 8c0d5cb0b4..92f0ce0f76 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/system/BotCore.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/system/BotCore.java @@ -2,13 +2,16 @@ import net.dv8tion.jda.api.JDA; import net.dv8tion.jda.api.entities.Channel; +import net.dv8tion.jda.api.events.interaction.command.MessageContextInteractionEvent; import net.dv8tion.jda.api.events.interaction.command.SlashCommandInteractionEvent; +import net.dv8tion.jda.api.events.interaction.command.UserContextInteractionEvent; import net.dv8tion.jda.api.events.interaction.component.ButtonInteractionEvent; import net.dv8tion.jda.api.events.interaction.component.SelectMenuInteractionEvent; import net.dv8tion.jda.api.events.message.MessageReceivedEvent; import net.dv8tion.jda.api.events.message.MessageUpdateEvent; import net.dv8tion.jda.api.hooks.ListenerAdapter; import net.dv8tion.jda.api.interactions.components.ComponentInteraction; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Unmodifiable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,6 +34,8 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import static org.togetherjava.tjbot.commands.UserInteractorPrefix.*; + /** * The bot core is the core of command handling in this application. *

@@ -245,9 +250,13 @@ private Stream getMessageReceiversSubscribedTo(Channel channel) @Override public void onSlashCommandInteraction(SlashCommandInteractionEvent event) { - logger.debug("Received slash command '{}' (#{}) on guild '{}'", event.getName(), - event.getId(), event.getGuild()); - COMMAND_SERVICE.execute(() -> requireSlashCommand(event.getName()).onSlashCommand(event)); + String name = event.getName(); + + logger.debug("Received slash command '{}' (#{}) on guild '{}'", name, event.getId(), + event.getGuild()); + COMMAND_SERVICE.execute( + () -> this.requireUserInteractor(SLASH_COMMAND.getPrefixedName(name)) + .onSlashCommand(event)); } @Override @@ -266,6 +275,31 @@ public void onSelectMenuInteraction(SelectMenuInteractionEvent event) { .execute(() -> forwardComponentCommand(event, UserInteractor::onSelectionMenu)); } + @Override + public void onMessageContextInteraction(@NotNull final MessageContextInteractionEvent event) { + String name = event.getName(); + + logger.debug("Received message context command '{}' (#{}) on guild '{}'", name, + event.getId(), event.getGuild()); + COMMAND_SERVICE.execute( + () -> this + .requireUserInteractor( + MESSAGE_CONTEXT_COMMAND.getPrefixedName(name)) + .onMessageContext(event)); + } + + @Override + public void onUserContextInteraction(@NotNull final UserContextInteractionEvent event) { + String name = event.getName(); + + logger.debug("Received user context command '{}' (#{}) on guild '{}'", name, event.getId(), + event.getGuild()); + COMMAND_SERVICE.execute(() -> this + .requireUserInteractor(USER_CONTEXT_COMMAND.getPrefixedName(name)) + .onUserContext(event)); + } + + /** * Forwards the given component event to the associated user interactor. *

@@ -319,20 +353,9 @@ private void forwardComponentCommand(T event, * @return the command with the given name * @throws NullPointerException if the command with the given name was not registered */ - private SlashCommand requireSlashCommand(String name) { - return getInteractor(name, SlashCommand.class).orElseThrow( - () -> new NullPointerException("There is no slash command with name " + name)); - } - - /** - * Gets the given user interactor by its name and requires that it exists. - * - * @param name the name of the user interactor to get - * @return the user interactor with the given name - * @throws NoSuchElementException if the user interactor with the given name was not registered - */ - private UserInteractor requireUserInteractor(final String name) { - return getInteractor(name).orElseThrow(); + private T requireUserInteractor(String name) { + return (T) getInteractor(name).orElseThrow( + () -> new NullPointerException("There is no interactor with name " + name)); } From 124e229244c077560e6503ad483bb5f39cfa3d4c Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Fri, 30 Sep 2022 17:05:48 +0200 Subject: [PATCH 2/2] improvements - fixed issues with requireUserInteractor and javadoc - removed obsolete prefix check that was done twice - fixed "escape of this during construction" - fail-fast for null check --- .../org/togetherjava/tjbot/Application.java | 4 ++ .../tjbot/commands/UserInteractorPrefix.java | 16 +++-- .../tjbot/commands/system/BotCore.java | 70 ++++++++----------- 3 files changed, 46 insertions(+), 44 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/Application.java b/application/src/main/java/org/togetherjava/tjbot/Application.java index 97d038c5c8..5927afe8af 100644 --- a/application/src/main/java/org/togetherjava/tjbot/Application.java +++ b/application/src/main/java/org/togetherjava/tjbot/Application.java @@ -83,7 +83,11 @@ public static void runBot(Config config) { .build(); jda.awaitReady(); + BotCore core = new BotCore(jda, database, config); + CommandReloading.reloadCommands(jda, core); + core.scheduleRoutines(jda); + jda.addEventListener(core); logger.info("Bot is ready"); diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/UserInteractorPrefix.java b/application/src/main/java/org/togetherjava/tjbot/commands/UserInteractorPrefix.java index 5af1a4b527..53c76a3b5c 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/UserInteractorPrefix.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/UserInteractorPrefix.java @@ -7,18 +7,24 @@ *

* This is used for separate interactors with the same name, by command type (and possibly more in * the future). Our system doesn't allow multiple interactors with the same name, while having a - * slash-command and a message-context-command with the same name can be really useful. + * slash-command, and a message-context-command with the same name can be useful. */ public enum UserInteractorPrefix { - /* - * Implementations that are none of the following have no dedicated prefix - */ + // Implementations that are none of the following have no dedicated prefix. + /** + * Prefix for slash commands. + */ SLASH_COMMAND(SlashCommand.class, "s-"), + /** + * Prefix for message context commands. + */ MESSAGE_CONTEXT_COMMAND(MessageContextCommand.class, "mc-"), + /** + * Prefix for user context commands. + */ USER_CONTEXT_COMMAND(UserContextCommand.class, "uc-"); - private final Class classType; private final String prefix; diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/system/BotCore.java b/application/src/main/java/org/togetherjava/tjbot/commands/system/BotCore.java index 92f0ce0f76..f4b1d6d1ba 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/system/BotCore.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/system/BotCore.java @@ -15,7 +15,6 @@ import org.jetbrains.annotations.Unmodifiable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.togetherjava.tjbot.CommandReloading; import org.togetherjava.tjbot.commands.*; import org.togetherjava.tjbot.commands.componentids.ComponentId; import org.togetherjava.tjbot.commands.componentids.ComponentIdParser; @@ -34,8 +33,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import static org.togetherjava.tjbot.commands.UserInteractorPrefix.*; - /** * The bot core is the core of command handling in this application. *

@@ -43,7 +40,6 @@ * events. It forwards events to their corresponding commands and does the heavy lifting on all sort * of event parsing. *

- *

* Commands are made available via {@link Features}, then the system has to be added to JDA as an * event listener, using {@link net.dv8tion.jda.api.JDA#addEventListener(Object...)}. Afterwards, * the system is ready and will correctly forward events to all commands. @@ -117,25 +113,16 @@ public BotCore(JDA jda, Database database, Config config) { if (logger.isInfoEnabled()) { logger.info("Available user interactors: {}", interactors); } - - - CommandReloading.reloadCommands(jda, this); - scheduleRoutines(jda); } /** - * Returns a predicate which validates the given interactor + * Returns a predicate, which validates the given interactor * - * @return A predicate which validates the given interactor + * @return A predicate, which validates the given interactor */ private static Predicate validateInteractorPredicate() { return interactor -> { - String name = interactor.getName(); - - if (name == null) { - logger.error("An interactor's name shouldn't be null! {}", interactor); - return false; - } + String name = Objects.requireNonNull(interactor.getName()); for (UserInteractorPrefix value : UserInteractorPrefix.values()) { String prefix = value.getPrefix(); @@ -147,11 +134,6 @@ private static Predicate validateInteractorPredicate() { } } - if (name.startsWith("s-") || name.startsWith("mc-") || name.startsWith("uc-")) { - throw new IllegalArgumentException( - "The interactor's name cannot start with any of the prefixes."); - } - return true; }; } @@ -254,8 +236,8 @@ public void onSlashCommandInteraction(SlashCommandInteractionEvent event) { logger.debug("Received slash command '{}' (#{}) on guild '{}'", name, event.getId(), event.getGuild()); - COMMAND_SERVICE.execute( - () -> this.requireUserInteractor(SLASH_COMMAND.getPrefixedName(name)) + COMMAND_SERVICE.execute(() -> requireUserInteractor( + UserInteractorPrefix.SLASH_COMMAND.getPrefixedName(name), SlashCommand.class) .onSlashCommand(event)); } @@ -281,11 +263,9 @@ public void onMessageContextInteraction(@NotNull final MessageContextInteraction logger.debug("Received message context command '{}' (#{}) on guild '{}'", name, event.getId(), event.getGuild()); - COMMAND_SERVICE.execute( - () -> this - .requireUserInteractor( - MESSAGE_CONTEXT_COMMAND.getPrefixedName(name)) - .onMessageContext(event)); + COMMAND_SERVICE.execute(() -> requireUserInteractor( + UserInteractorPrefix.MESSAGE_CONTEXT_COMMAND.getPrefixedName(name), + MessageContextCommand.class).onMessageContext(event)); } @Override @@ -294,9 +274,9 @@ public void onUserContextInteraction(@NotNull final UserContextInteractionEvent logger.debug("Received user context command '{}' (#{}) on guild '{}'", name, event.getId(), event.getGuild()); - COMMAND_SERVICE.execute(() -> this - .requireUserInteractor(USER_CONTEXT_COMMAND.getPrefixedName(name)) - .onUserContext(event)); + COMMAND_SERVICE.execute(() -> requireUserInteractor( + UserInteractorPrefix.USER_CONTEXT_COMMAND.getPrefixedName(name), + UserContextCommand.class).onUserContext(event)); } @@ -340,22 +320,34 @@ private void forwardComponentCommand(T event, } ComponentId componentId = componentIdOpt.orElseThrow(); - UserInteractor interactor = requireUserInteractor(componentId.userInteractorName()); + UserInteractor interactor = + requireUserInteractor(componentId.userInteractorName(), UserInteractor.class); logger.trace("Routing a component event with id '{}' back to user interactor '{}'", event.getComponentId(), interactor.getName()); interactorArgumentConsumer.accept(interactor, event, componentId.elements()); } /** - * Gets the given slash command by its name and requires that it exists. + * Gets the given user interactor by its full name, requires it exists and is of the given type. * - * @param name the name of the command to get - * @return the command with the given name - * @throws NullPointerException if the command with the given name was not registered + * @param fullName the full name of the interactor, including the prefix + * @param typeToken a token of the type to expect + * @return the user interactor with the given name + * @param the type to expect the user interactor to be of */ - private T requireUserInteractor(String name) { - return (T) getInteractor(name).orElseThrow( - () -> new NullPointerException("There is no interactor with name " + name)); + private T requireUserInteractor(String fullName, + Class typeToken) { + UserInteractor userInteractor = getInteractor(fullName).orElseThrow( + () -> new IllegalArgumentException("There is no interactor with name " + fullName)); + + if (!typeToken.isInstance(userInteractor)) { + throw new IllegalArgumentException( + "The interactor %s is not of the expected type %s, but instead %s".formatted( + fullName, typeToken.getSimpleName(), + fullName.getClass().getSimpleName())); + } + + return typeToken.cast(userInteractor); }