From 0c575378f3173536f53fe58320c6bf9efe989437 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beatrice=20Dellac=C3=A0?= Date: Fri, 5 Sep 2025 12:49:37 +0200 Subject: [PATCH] fix concurrency and ack --- .../wtf/beatrice/hidekobot/HidekoBot.java | 3 + .../hidekobot/commands/base/CoinFlip.java | 58 +++++--- .../hidekobot/commands/base/Trivia.java | 137 ++++++++++-------- .../commands/base/UrbanDictionary.java | 5 +- .../hidekobot/services/CommandService.java | 25 +++- src/main/resources/application.properties | 2 +- 6 files changed, 146 insertions(+), 84 deletions(-) diff --git a/src/main/java/wtf/beatrice/hidekobot/HidekoBot.java b/src/main/java/wtf/beatrice/hidekobot/HidekoBot.java index 6293900..f0eab9b 100644 --- a/src/main/java/wtf/beatrice/hidekobot/HidekoBot.java +++ b/src/main/java/wtf/beatrice/hidekobot/HidekoBot.java @@ -8,6 +8,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.boot.system.ApplicationHome; import org.springframework.context.ConfigurableApplicationContext; import wtf.beatrice.hidekobot.commands.completer.ProfileImageCommandCompleter; import wtf.beatrice.hidekobot.commands.message.HelloCommand; @@ -68,6 +69,8 @@ public class HidekoBot return; } + ApplicationHome home = new ApplicationHome(HidekoBot.class); + System.setProperty("APP_HOME", home.getDir().getAbsolutePath()); ConfigurableApplicationContext context = SpringApplication.run(HidekoBot.class, args); CommandService commandService = context.getBean(CommandService.class); diff --git a/src/main/java/wtf/beatrice/hidekobot/commands/base/CoinFlip.java b/src/main/java/wtf/beatrice/hidekobot/commands/base/CoinFlip.java index 945c9c1..5715179 100644 --- a/src/main/java/wtf/beatrice/hidekobot/commands/base/CoinFlip.java +++ b/src/main/java/wtf/beatrice/hidekobot/commands/base/CoinFlip.java @@ -9,6 +9,7 @@ import net.dv8tion.jda.api.interactions.components.buttons.Button; import wtf.beatrice.hidekobot.Cache; import wtf.beatrice.hidekobot.util.RandomUtil; +import java.util.ArrayList; import java.util.List; public class CoinFlip @@ -43,27 +44,46 @@ public class CoinFlip public static void buttonReFlip(ButtonInteractionEvent event) { - // check if the user interacting is the same one who ran the command - if (!(Cache.getServices().databaseService().isUserTrackedFor(event.getUser().getId(), event.getMessageId()))) - { - event.reply("❌ You did not run this command!").setEphemeral(true).queue(); - return; - } + // Ack ASAP to avoid 3s timeout + event.deferEdit().queue(hook -> { + // Permission check **after** ack + if (!Cache.getServices().databaseService().isUserTrackedFor(event.getUser().getId(), event.getMessageId())) + { + hook.sendMessage("❌ You did not run this command!").setEphemeral(true).queue(); + return; + } - // set old message's button as disabled - List actionRows = event.getMessage().getActionRows(); - actionRows.set(0, actionRows.get(0).asDisabled()); - event.editComponents(actionRows).queue(); + // Disable all components on the original message + List oldRows = event.getMessage().getActionRows(); + List disabledRows = new ArrayList<>(oldRows.size()); + for (ActionRow row : oldRows) + { + disabledRows.add(row.asDisabled()); + } + hook.editOriginalComponents(disabledRows).queue(); - // perform coin flip - event.getHook().sendMessage(genRandom()) - .addActionRow(getReflipButton()) - .queue((message) -> - { - // set the command as expiring and restrict it to the user who ran it - trackAndRestrict(message, event.getUser()); - }, (error) -> { - }); + // Send a follow-up with a fresh button + hook.sendMessage(genRandom()) + .addActionRow(getReflipButton()) + .queue(msg -> trackAndRestrict(msg, event.getUser()), err -> { + }); + }, failure -> { + // Rare: if we couldn't ack, try best-effort fallbacks + try + { + List oldRows = event.getMessage().getActionRows(); + List disabledRows = new ArrayList<>(oldRows.size()); + for (ActionRow row : oldRows) disabledRows.add(row.asDisabled()); + event.getMessage().editMessageComponents(disabledRows).queue(); + } catch (Exception ignored) + { + } + + event.getChannel().sendMessage(genRandom()) + .addActionRow(getReflipButton()) + .queue(msg -> trackAndRestrict(msg, event.getUser()), err -> { + }); + }); } public static void trackAndRestrict(Message replyMessage, User user) diff --git a/src/main/java/wtf/beatrice/hidekobot/commands/base/Trivia.java b/src/main/java/wtf/beatrice/hidekobot/commands/base/Trivia.java index bcc7857..59f2681 100644 --- a/src/main/java/wtf/beatrice/hidekobot/commands/base/Trivia.java +++ b/src/main/java/wtf/beatrice/hidekobot/commands/base/Trivia.java @@ -26,9 +26,10 @@ import java.io.InputStreamReader; import java.net.URL; import java.net.URLConnection; import java.util.ArrayList; -import java.util.HashMap; +import java.util.Collections; import java.util.LinkedList; import java.util.List; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -44,13 +45,13 @@ public class Trivia private static final String TRIVIA_API_LINK = "https://opentdb.com/api.php?amount=10&type=multiple&category="; private static final String TRIVIA_API_CATEGORIES_LINK = "https://opentdb.com/api_category.php"; - public static List channelsRunningTrivia = new ArrayList<>(); + public static List channelsRunningTrivia = Collections.synchronizedList(new ArrayList<>()); // first string is the channelId, the list contain all users who responded there - public static HashMap> channelAndWhoResponded = new HashMap<>(); + public static ConcurrentHashMap> channelAndWhoResponded = new ConcurrentHashMap<>(); // first string is the channelId, the list contain all score records for that channel - public static HashMap> channelAndScores = new HashMap<>(); + public static ConcurrentHashMap> channelAndScores = new ConcurrentHashMap<>(); public static String getTriviaLink(int categoryId) { @@ -180,53 +181,63 @@ public class Trivia public static void handleAnswer(ButtonInteractionEvent event, AnswerType answerType) { - User user = event.getUser(); - String channelId = event.getChannel().getId(); + // Ack immediately with an ephemeral deferral to avoid 3s timeout + event.deferReply(true).queue(hook -> { + User user = event.getUser(); + String channelId = event.getChannel().getId(); - if (trackResponse(user, event.getChannel())) - { - LinkedList scores = channelAndScores.get(channelId); - if (scores == null) scores = new LinkedList<>(); - TriviaScore currentUserScore = null; - for (TriviaScore score : scores) + if (trackResponse(user, event.getChannel())) { - if (score.getUser().equals(user)) + LinkedList scores = channelAndScores.get(channelId); + if (scores == null) scores = new LinkedList<>(); + + TriviaScore currentUserScore = null; + for (TriviaScore score : scores) { - currentUserScore = score; - scores.remove(score); - break; + if (score.getUser().equals(user)) + { + currentUserScore = score; + scores.remove(score); + break; + } } - } - if (currentUserScore == null) - { - currentUserScore = new TriviaScore(user); - } + if (currentUserScore == null) + { + currentUserScore = new TriviaScore(user); + } - if (answerType.equals(AnswerType.CORRECT)) - { - - event.reply(user.getAsMention() + " got it right! \uD83E\uDD73 (**+3**)").queue(); - currentUserScore.changeScore(3); + if (answerType.equals(AnswerType.CORRECT)) + { + // Public message in channel + event.getChannel().sendMessage(user.getAsMention() + " got it right! \uD83E\uDD73 (**+3**)").queue(); + currentUserScore.changeScore(3); + } else + { + event.getChannel().sendMessage("❌ " + user.getAsMention() + ", that's not the right answer! (**-1**)").queue(); + currentUserScore.changeScore(-1); + } + scores.add(currentUserScore); + channelAndScores.put(channelId, scores); } else { - event.reply("❌ " + user.getAsMention() + ", that's not the right answer! (**-1**)").queue(); - currentUserScore.changeScore(-1); + // Show the warning **in the original ephemeral message**, then delete it after 5s. + hook.editOriginal("☹️ " + user.getAsMention() + ", you can't answer twice!").queue(v -> + hook.deleteOriginal().queueAfter(3, TimeUnit.SECONDS, null, __ -> { + }) + ); + return; // don't run the generic cleanup below; we want the message visible for ~5s } - scores.add(currentUserScore); - channelAndScores.put(channelId, scores); - } else - { - event.reply("☹️ " + user.getAsMention() + ", you can't answer twice!") - .queue(interaction -> - Cache.getTaskScheduler().schedule(() -> - interaction.deleteOriginal().queue(), 3, TimeUnit.SECONDS)); - } + // Clean up the ephemeral deferral (no visible ephemeral message left) for the normal path + hook.deleteOriginal().queue(null, __ -> { + }); + }, __ -> { + }); } - private static boolean trackResponse(User user, MessageChannel channel) + private static synchronized boolean trackResponse(User user, MessageChannel channel) { String userId = user.getId(); String channelId = channel.getId(); @@ -251,24 +262,32 @@ public class Trivia public static void handleMenuSelection(StringSelectInteractionEvent event) { - // check if the user interacting is the same one who ran the command - if (!(Cache.getServices().databaseService().isUserTrackedFor(event.getUser().getId(), event.getMessageId()))) - { - event.reply("❌ You did not run this command!").setEphemeral(true).queue(); - return; - } + // Ack immediately (ephemeral) so we can safely do DB/work + event.deferReply(true).queue(hook -> { + // check if the user interacting is the same one who ran the command + if (!(Cache.getServices().databaseService().isUserTrackedFor(event.getUser().getId(), event.getMessageId()))) + { + hook.sendMessage("❌ You did not run this command!").setEphemeral(true).queue(); + return; + } - // todo: we shouldn't use this method, since it messes with the database... look at coin reflip - Cache.getServices().commandService().disableExpired(event.getMessageId()); + // Disable buttons on the original message via service (uses separate REST calls) + Cache.getServices().commandService().disableExpired(event.getMessageId()); - SelectOption pickedOption = event.getInteraction().getSelectedOptions().get(0); - String categoryName = pickedOption.getLabel(); - String categoryIdString = pickedOption.getValue(); - Integer categoryId = Integer.parseInt(categoryIdString); + SelectOption pickedOption = event.getInteraction().getSelectedOptions().get(0); + String categoryName = pickedOption.getLabel(); + String categoryIdString = pickedOption.getValue(); + Integer categoryId = Integer.parseInt(categoryIdString); - TriviaCategory category = new TriviaCategory(categoryName, categoryId); + TriviaCategory category = new TriviaCategory(categoryName, categoryId); - startTrivia(event, category); + startTrivia(event, category); + + // remove the ephemeral deferral to keep things clean + hook.deleteOriginal().queue(null, __ -> { + }); + }, __ -> { + }); } public static void startTrivia(StringSelectInteractionEvent event, TriviaCategory category) @@ -279,19 +298,17 @@ public class Trivia if (Trivia.channelsRunningTrivia.contains(channel.getId())) { - // todo nicer looking - // todo: also what if the bot stops (database...?) - // todo: also what if the message is already deleted - Message err = event.reply("Trivia is already running here!").complete().retrieveOriginal().complete(); - Cache.getTaskScheduler().schedule(() -> err.delete().queue(), 10, TimeUnit.SECONDS); + // Already running: inform ephemerally via hook (the interaction was deferred in the caller) + event.getHook().sendMessage(Trivia.getTriviaAlreadyRunningError()) + .setEphemeral(true) + .queue(msg -> Cache.getTaskScheduler().schedule(() -> msg.delete().queue(), 10, TimeUnit.SECONDS)); return; } else { - // todo nicer looking - event.reply("Starting new Trivia session!").queue(); + // Public info that a new session is starting + channel.sendMessage("Starting new Trivia session!").queue(); } - TriviaTask triviaTask = new TriviaTask(author, channel, category, Cache.getServices().databaseService(), Cache.getServices().commandService()); ScheduledFuture future = diff --git a/src/main/java/wtf/beatrice/hidekobot/commands/base/UrbanDictionary.java b/src/main/java/wtf/beatrice/hidekobot/commands/base/UrbanDictionary.java index 887f236..35470e2 100644 --- a/src/main/java/wtf/beatrice/hidekobot/commands/base/UrbanDictionary.java +++ b/src/main/java/wtf/beatrice/hidekobot/commands/base/UrbanDictionary.java @@ -119,6 +119,7 @@ public class UrbanDictionary public static void changePage(ButtonInteractionEvent event, ChangeType changeType) { + event.deferEdit().queue(); String messageId = event.getMessageId(); DatabaseService database = Cache.getServices().databaseService(); @@ -180,7 +181,9 @@ public class UrbanDictionary ActionRow currentRow = ActionRow.of(components); // update the message - event.editComponents(currentRow).setEmbeds(updatedEmbed).queue(); + event.getHook().editOriginalEmbeds(updatedEmbed) + .setComponents(currentRow) + .queue(); database.setUrbanPage(messageId, page); database.resetExpiryTimestamp(messageId); } diff --git a/src/main/java/wtf/beatrice/hidekobot/services/CommandService.java b/src/main/java/wtf/beatrice/hidekobot/services/CommandService.java index d0f8735..e0cc310 100644 --- a/src/main/java/wtf/beatrice/hidekobot/services/CommandService.java +++ b/src/main/java/wtf/beatrice/hidekobot/services/CommandService.java @@ -51,9 +51,28 @@ public class CommandService return; } - // delete the message - event.getInteraction().getMessage().delete().queue(); - // no need to manually untrack it from database, it will be purged on the next planned check. + // Acknowledge immediately so the interaction token stays valid + event.deferEdit().queue(hook -> { + // Try deleting via the interaction webhook (works for original interaction responses) + hook.deleteOriginal().queue( + success -> { /* optional: databaseService.untrackExpiredMessage(event.getMessageId()); */ }, + failure -> { + // Fallback to channel delete (works even if webhook token expired) + event.getChannel().deleteMessageById(event.getMessageId()).queue( + null, + __ -> { /* ignore if already deleted */ } + ); + } + ); + }, + failure -> { + // If we failed to acknowledge (interaction already expired), try channel delete anyway + event.getChannel().deleteMessageById(event.getMessageId()).queue( + null, + __ -> { /* ignore if already deleted */ } + ); + } + ); } diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 7b93f20..0f81dec 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -1,4 +1,4 @@ -spring.datasource.url=jdbc:sqlite:/absolute/path/to/db.sqlite +spring.datasource.url=jdbc:sqlite:${APP_HOME}/db.sqlite spring.datasource.driver-class-name=org.sqlite.JDBC # let Hibernate create/update tables for you during the migration spring.jpa.hibernate.ddl-auto=update