From 7a9b0e04f47a739056ca2ac6d60a2fb76fddb999 Mon Sep 17 00:00:00 2001 From: shayaantx <5449086+shayaantx@users.noreply.github.com> Date: Sat, 17 Jun 2023 14:14:15 -0400 Subject: [PATCH] Fix bug where telegram callback data for add buttons no longer works (#105) Specifically persist callback data in the local database so it can be retrieved by an integer instead --- pom.xml | 4 +- .../java/com/botdarr/clients/ChatClient.java | 1 + .../botdarr/clients/ChatClientBootstrap.java | 1 + .../clients/discord/DiscordChatClient.java | 5 + .../clients/matrix/MatrixChatClient.java | 5 + .../clients/slack/SlackChatClient.java | 5 + .../clients/telegram/TelegramBootstrap.java | 4 +- .../telegram/TelegramCallbackManager.java | 106 ++++++++++++++++++ .../clients/telegram/TelegramChatClient.java | 6 + .../telegram/TelegramResponseBuilder.java | 12 +- .../com/botdarr/scheduling/Scheduler.java | 6 + .../com/botdarr/utilities/DateWrapper.java | 9 ++ .../upgrade/V3__addTelegramCallbackTable.sql | 1 + src/main/resources/version.txt | 2 +- .../com/botdarr/api/ApiRequestsTests.java | 14 +-- .../TelegramCallbackManagerTests.java | 64 +++++++++++ .../com/botdarr/database/MockedDatabase.java | 19 ++++ 17 files changed, 241 insertions(+), 23 deletions(-) create mode 100644 src/main/java/com/botdarr/clients/telegram/TelegramCallbackManager.java create mode 100644 src/main/java/com/botdarr/utilities/DateWrapper.java create mode 100644 src/main/resources/upgrade/V3__addTelegramCallbackTable.sql create mode 100644 src/test/java/com/botdarr/clients/telegram/TelegramCallbackManagerTests.java create mode 100644 src/test/java/com/botdarr/database/MockedDatabase.java diff --git a/pom.xml b/pom.xml index 1c11ddd..1ad4d8d 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ botdarr release botdarr - Discord/Slack/Telegram bot for interfacing with radarr, sonarr + Discord/Slack/Telegram/Matrix bot for interfacing with radarr, sonarr, lidarr 1.8 @@ -91,7 +91,7 @@ com.github.pengrad java-telegram-bot-api - 5.5.0 + 6.6.1 com.j2html diff --git a/src/main/java/com/botdarr/clients/ChatClient.java b/src/main/java/com/botdarr/clients/ChatClient.java index 07d46c8..d48d04b 100644 --- a/src/main/java/com/botdarr/clients/ChatClient.java +++ b/src/main/java/com/botdarr/clients/ChatClient.java @@ -4,4 +4,5 @@ public interface ChatClient { void sendToConfiguredChannels(List chatClientResponses); + void cleanup(); } diff --git a/src/main/java/com/botdarr/clients/ChatClientBootstrap.java b/src/main/java/com/botdarr/clients/ChatClientBootstrap.java index 49a478f..f091475 100644 --- a/src/main/java/com/botdarr/clients/ChatClientBootstrap.java +++ b/src/main/java/com/botdarr/clients/ChatClientBootstrap.java @@ -60,6 +60,7 @@ protected void initScheduling(ChatClient chatC //make sure to always cache before doing any notifications scheduler.initApiCaching(apis); scheduler.initApiNotifications(apis, chatClient, responseBuilder); + scheduler.initCleanup(chatClient); } protected void runAndProcessCommands(String prefix, diff --git a/src/main/java/com/botdarr/clients/discord/DiscordChatClient.java b/src/main/java/com/botdarr/clients/discord/DiscordChatClient.java index bca9b0c..6743988 100644 --- a/src/main/java/com/botdarr/clients/discord/DiscordChatClient.java +++ b/src/main/java/com/botdarr/clients/discord/DiscordChatClient.java @@ -27,6 +27,11 @@ public void sendToConfiguredChannels(List chatClientResponses) }, null); } + @Override + public void cleanup() { + // nothing to cleanup + } + public void sendMessage(DiscordResponse chatClientResponse, String channelName) { sendMessages(channel -> channel.sendMessage(chatClientResponse.getMessage()).queue(), channelName); } diff --git a/src/main/java/com/botdarr/clients/matrix/MatrixChatClient.java b/src/main/java/com/botdarr/clients/matrix/MatrixChatClient.java index 86fc417..d451796 100644 --- a/src/main/java/com/botdarr/clients/matrix/MatrixChatClient.java +++ b/src/main/java/com/botdarr/clients/matrix/MatrixChatClient.java @@ -57,6 +57,11 @@ public void sendToConfiguredChannels(List chatClientResponses) { } } + @Override + public void cleanup() { + // nothing to cleanup + } + public void sendMessage(MatrixResponse response, String roomId) { sendMatrixResponse(response, roomId); } diff --git a/src/main/java/com/botdarr/clients/slack/SlackChatClient.java b/src/main/java/com/botdarr/clients/slack/SlackChatClient.java index 6bf8f63..0cda6f5 100644 --- a/src/main/java/com/botdarr/clients/slack/SlackChatClient.java +++ b/src/main/java/com/botdarr/clients/slack/SlackChatClient.java @@ -152,6 +152,11 @@ public void sendToConfiguredChannels(List chatClientResponses) { sendMessage(chatClientResponses, null); } + @Override + public void cleanup() { + // nothing to cleanup + } + private interface MessageSender { void send(String channel); } diff --git a/src/main/java/com/botdarr/clients/telegram/TelegramBootstrap.java b/src/main/java/com/botdarr/clients/telegram/TelegramBootstrap.java index f761892..c6d6e01 100644 --- a/src/main/java/com/botdarr/clients/telegram/TelegramBootstrap.java +++ b/src/main/java/com/botdarr/clients/telegram/TelegramBootstrap.java @@ -40,8 +40,10 @@ public void init() throws Exception { chat = message.chat(); } else if (update.callbackQuery() != null && !Strings.isEmpty(update.callbackQuery().data())) { // interactive commands with callback data + TelegramCallbackManager telegramCallbackManager = new TelegramCallbackManager(); + String callback = telegramCallbackManager.getCallback(Integer.parseInt(update.callbackQuery().data())); ObjectMapper mapper = new ObjectMapper(); - TelegramCallbackData callbackData = mapper.readValue(update.callbackQuery().data(), TelegramCallbackData.class); + TelegramCallbackData callbackData = mapper.readValue(callback, TelegramCallbackData.class); text = callbackData.getCommand(); chat = update.callbackQuery().message().chat(); } else { diff --git a/src/main/java/com/botdarr/clients/telegram/TelegramCallbackManager.java b/src/main/java/com/botdarr/clients/telegram/TelegramCallbackManager.java new file mode 100644 index 0000000..1f632bd --- /dev/null +++ b/src/main/java/com/botdarr/clients/telegram/TelegramCallbackManager.java @@ -0,0 +1,106 @@ +package com.botdarr.clients.telegram; + +import com.botdarr.database.DatabaseHelper; +import com.botdarr.utilities.DateWrapper; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.util.Strings; + +import java.sql.*; +import java.time.LocalDate; +import java.util.ArrayList; +import java.util.List; + +public class TelegramCallbackManager { + private final DateWrapper dateWrapper; + + public TelegramCallbackManager(DateWrapper dateWrapper) { + this.dateWrapper = dateWrapper; + } + + public TelegramCallbackManager() { + this.dateWrapper = new DateWrapper(); + } + + private void deleteCallback(Connection conn, int id) throws SQLException { + PreparedStatement statement = conn.prepareStatement("delete from telegram_callbacks where id = ?"); + statement.setInt(1, id); + statement.executeUpdate(); + } + + public int saveCallback(String callback) { + String url = databaseHelper.getJdbcUrl(); + try (Connection conn = DriverManager.getConnection(url)) { + PreparedStatement statement = conn.prepareStatement("insert into telegram_callbacks (callback, createdDt) values (?, ?)"); + statement.setString(1, callback); + statement.setDate(2, Date.valueOf(this.dateWrapper.getNow())); + statement.executeUpdate(); + try (ResultSet rs = statement.getGeneratedKeys()) { + if (rs.next()) { + return rs.getInt(1); + } + } + } catch (Exception e) { + LOGGER.error("Error trying to save telegram callback", e); + throw new RuntimeException(e); + } + throw new RuntimeException("Could not save telegram callback"); + } + + public String getCallback(int id) { + String url = databaseHelper.getJdbcUrl(); + try (Connection conn = DriverManager.getConnection(url)) { + PreparedStatement statement = conn.prepareStatement("select callback from telegram_callbacks where id = ?"); + statement.setInt(1, id); + String callback = null; + try (ResultSet rs = statement.executeQuery()) { + if (rs.next()) { + callback = rs.getString("callback"); + } + } + if (!Strings.isEmpty(callback)) { + // delete the callback entry + deleteCallback(conn, id); + return callback; + } + throw new TelegramCallbackMissing(); + } catch (TelegramCallbackMissing e) { + throw e; + } catch (Exception e) { + LOGGER.error("Error trying to get telegram callback", e); + throw new RuntimeException(e); + } + } + + public void deleteOldCallbacks() { + LOGGER.info("Checking for telegram callbacks to delete"); + String url = databaseHelper.getJdbcUrl(); + try (Connection conn = DriverManager.getConnection(url)) { + // if callbacks haven't been used in 10 days, delete them + PreparedStatement statement = conn.prepareStatement("SELECT id FROM telegram_callbacks WHERE createdDt < ?"); + LocalDate tenDaysAgo = this.dateWrapper.getNow().minusDays(10); + statement.setDate(1, Date.valueOf(tenDaysAgo)); + List rowsToDelete = new ArrayList<>(); + try (ResultSet rs = statement.executeQuery()) { + if (rs.next()) { + rowsToDelete.add(rs.getInt("id")); + } + } + LOGGER.info("Found telegram callbacks to delete, count=" + rowsToDelete.size()); + for (int rowIdToDelete : rowsToDelete) { + LOGGER.info("Deleting telegram callback, id=" + rowIdToDelete); + deleteCallback(conn, rowIdToDelete); + } + } catch (Exception e) { + LOGGER.error("Error trying to delete old telegram callback", e); + throw new RuntimeException(e); + } + } + + public static class TelegramCallbackMissing extends RuntimeException { + + } + + private final DatabaseHelper databaseHelper = new DatabaseHelper(); + Logger LOGGER = LogManager.getLogger(); +} diff --git a/src/main/java/com/botdarr/clients/telegram/TelegramChatClient.java b/src/main/java/com/botdarr/clients/telegram/TelegramChatClient.java index a9738f6..f24bdbf 100644 --- a/src/main/java/com/botdarr/clients/telegram/TelegramChatClient.java +++ b/src/main/java/com/botdarr/clients/telegram/TelegramChatClient.java @@ -96,6 +96,12 @@ public void sendToConfiguredChannels(List chatClientResponses) sendMessage(chatClientResponses, null); } + @Override + public void cleanup() { + // remove any 10 day old callbacks + new TelegramCallbackManager().deleteOldCallbacks(); + } + private interface MessageSender { void send(Chat chatChannel); } diff --git a/src/main/java/com/botdarr/clients/telegram/TelegramResponseBuilder.java b/src/main/java/com/botdarr/clients/telegram/TelegramResponseBuilder.java index e11ff5f..4a032ac 100644 --- a/src/main/java/com/botdarr/clients/telegram/TelegramResponseBuilder.java +++ b/src/main/java/com/botdarr/clients/telegram/TelegramResponseBuilder.java @@ -300,7 +300,7 @@ public TelegramResponse build(NewMusicArtistResponse newMusicArtistResponse) { List domContents = new ArrayList<>(); String artistDetail = " (" + lookupArtist.getDisambiguation() + ")"; domContents.add(b(lookupArtist.getArtistName() + (Strings.isEmpty(lookupArtist.getDisambiguation()) ? "" : artistDetail))); - domContents.add(a(lookupArtist.getRemoteImage())); + domContents.add(Strings.isEmpty(lookupArtist.getRemoteImage()) ? code("No image found!") : a(lookupArtist.getRemoteImage())); return getAddResponse(domContents, LidarrCommands.getAddArtistCommandStr(lookupArtist.getArtistName(), lookupArtist.getForeignArtistId())); } @@ -338,11 +338,11 @@ private TelegramResponse getAddResponse(List domContents, String com try { ObjectMapper objectMapper = new ObjectMapper(); String json = objectMapper.writeValueAsString(new TelegramCallbackData(command)); - return new TelegramResponse(domContents, new InlineKeyboardMarkup( - new InlineKeyboardButton[]{ - new InlineKeyboardButton("Add").callbackData(json), - } - )); + TelegramCallbackManager telegramCallbackManager = new TelegramCallbackManager(); + int id = telegramCallbackManager.saveCallback(json); + return new TelegramResponse(domContents, + // callback data can never be larger than 64 bytes + new InlineKeyboardMarkup(new InlineKeyboardButton("Add").callbackData(String.valueOf(id)))); } catch (JsonProcessingException e) { throw new RuntimeException(e); } diff --git a/src/main/java/com/botdarr/scheduling/Scheduler.java b/src/main/java/com/botdarr/scheduling/Scheduler.java index e51e513..7799f39 100644 --- a/src/main/java/com/botdarr/scheduling/Scheduler.java +++ b/src/main/java/com/botdarr/scheduling/Scheduler.java @@ -26,6 +26,11 @@ public static Scheduler getScheduler() { return instance; } + public void initCleanup(ChatClient chatClient) { + if (cleanupFuture == null) { + cleanupFuture = Executors.newScheduledThreadPool(1).scheduleWithFixedDelay(chatClient::cleanup, 0, 5 , TimeUnit.MINUTES); + } + } public void initApiNotifications(List apis, ChatClient chatClient, ChatClientResponseBuilder responseBuilder) { if (notificationFuture == null) { @@ -74,6 +79,7 @@ public void executeCommand(Callable callable) { private ScheduledFuture notificationFuture; private ScheduledFuture cacheFuture; + private ScheduledFuture cleanupFuture; private ExecutorService commandThreadPool; private static volatile Scheduler instance; private static final Logger LOGGER = LogManager.getLogger(); diff --git a/src/main/java/com/botdarr/utilities/DateWrapper.java b/src/main/java/com/botdarr/utilities/DateWrapper.java new file mode 100644 index 0000000..765e4e0 --- /dev/null +++ b/src/main/java/com/botdarr/utilities/DateWrapper.java @@ -0,0 +1,9 @@ +package com.botdarr.utilities; + +import java.time.LocalDate; + +public class DateWrapper { + public LocalDate getNow() { + return LocalDate.now(); + } +} diff --git a/src/main/resources/upgrade/V3__addTelegramCallbackTable.sql b/src/main/resources/upgrade/V3__addTelegramCallbackTable.sql new file mode 100644 index 0000000..5fe7d6b --- /dev/null +++ b/src/main/resources/upgrade/V3__addTelegramCallbackTable.sql @@ -0,0 +1 @@ +create table telegram_callbacks (id integer primary key not null, callback text not null, createdDt date not null); \ No newline at end of file diff --git a/src/main/resources/version.txt b/src/main/resources/version.txt index 3b285e7..7ba0f8a 100644 --- a/src/main/resources/version.txt +++ b/src/main/resources/version.txt @@ -1 +1 @@ -5.6.6 \ No newline at end of file +5.6.7 \ No newline at end of file diff --git a/src/test/java/com/botdarr/api/ApiRequestsTests.java b/src/test/java/com/botdarr/api/ApiRequestsTests.java index 2708f46..71c96d6 100644 --- a/src/test/java/com/botdarr/api/ApiRequestsTests.java +++ b/src/test/java/com/botdarr/api/ApiRequestsTests.java @@ -3,6 +3,7 @@ import com.botdarr.Config; import com.botdarr.database.DatabaseBootstrap; import com.botdarr.database.DatabaseHelper; +import com.botdarr.database.MockedDatabase; import mockit.*; import org.junit.Assert; import org.junit.Before; @@ -243,19 +244,6 @@ public void validate() { private final Throwable expectedThrowable; } - private static class MockedDatabase extends MockUp { - private MockedDatabase(File temporaryDatabase) { - this.temporaryDatabase = temporaryDatabase; - } - - @Mock - public File getDatabaseFile() { - return temporaryDatabase; - } - - private final File temporaryDatabase; - } - @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); } diff --git a/src/test/java/com/botdarr/clients/telegram/TelegramCallbackManagerTests.java b/src/test/java/com/botdarr/clients/telegram/TelegramCallbackManagerTests.java new file mode 100644 index 0000000..cbef79a --- /dev/null +++ b/src/test/java/com/botdarr/clients/telegram/TelegramCallbackManagerTests.java @@ -0,0 +1,64 @@ +package com.botdarr.clients.telegram; + +import com.botdarr.database.DatabaseBootstrap; +import com.botdarr.database.MockedDatabase; +import com.botdarr.utilities.DateWrapper; +import org.junit.*; +import org.junit.rules.TemporaryFolder; + +import java.io.IOException; +import java.time.LocalDate; + +public class TelegramCallbackManagerTests { + @Before + public void beforeTests() throws IOException { + //create temporary database + new MockedDatabase(temporaryFolder.newFile()); + DatabaseBootstrap.init(); + } + + @Test + public void saveCallback_savesAndGetsCallbackDataSuccessfully() { + TelegramCallbackManager telegramCallbackManager = new TelegramCallbackManager(); + String callback = "!movie find new test"; + int result = telegramCallbackManager.saveCallback(callback); + Assert.assertTrue(result > 0); + String actualCallback = telegramCallbackManager.getCallback(result); + Assert.assertEquals(callback, actualCallback); + } + + @Test(expected = TelegramCallbackManager.TelegramCallbackMissing.class) + public void deleteCallbacks_removes10DayOldCallbacks() { + LocalDate fakeNow = LocalDate.now(); + fakeNow = fakeNow.minusDays(11); + + // insert callbacks that are 11 days old + TelegramCallbackManager telegramCallbackManager = new TelegramCallbackManager(new FakeDateWrapper(fakeNow)); + String callback = "!movie find new test"; + int result = telegramCallbackManager.saveCallback(callback); + Assert.assertTrue(result > 0); + + // trigger the callback delete logic + telegramCallbackManager = new TelegramCallbackManager(); + telegramCallbackManager.deleteOldCallbacks(); + + // this should throw an exception now + telegramCallbackManager.getCallback(result); + } + + private static class FakeDateWrapper extends DateWrapper { + private LocalDate now; + + public FakeDateWrapper(LocalDate now) { + this.now = now; + } + + @Override + public LocalDate getNow() { + return this.now; + } + } + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); +} diff --git a/src/test/java/com/botdarr/database/MockedDatabase.java b/src/test/java/com/botdarr/database/MockedDatabase.java new file mode 100644 index 0000000..1ec43be --- /dev/null +++ b/src/test/java/com/botdarr/database/MockedDatabase.java @@ -0,0 +1,19 @@ +package com.botdarr.database; + +import mockit.Mock; +import mockit.MockUp; + +import java.io.File; + +public class MockedDatabase extends MockUp { + public MockedDatabase(File temporaryDatabase) { + this.temporaryDatabase = temporaryDatabase; + } + + @Mock + public File getDatabaseFile() { + return temporaryDatabase; + } + + private final File temporaryDatabase; +} \ No newline at end of file