Skip to content

Commit

Permalink
Fix bug where telegram callback data for add buttons no longer works (#…
Browse files Browse the repository at this point in the history
…105)

Specifically persist callback data in the local database so it can be retrieved by an integer instead
  • Loading branch information
shayaantx authored Jun 17, 2023
1 parent 44c2774 commit 7a9b0e0
Show file tree
Hide file tree
Showing 17 changed files with 241 additions and 23 deletions.
4 changes: 2 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<artifactId>botdarr</artifactId>
<version>release</version>
<name>botdarr</name>
<description>Discord/Slack/Telegram bot for interfacing with radarr, sonarr</description>
<description>Discord/Slack/Telegram/Matrix bot for interfacing with radarr, sonarr, lidarr</description>

<properties>
<java.version>1.8</java.version>
Expand Down Expand Up @@ -91,7 +91,7 @@
<dependency>
<groupId>com.github.pengrad</groupId>
<artifactId>java-telegram-bot-api</artifactId>
<version>5.5.0</version>
<version>6.6.1</version>
</dependency>
<dependency>
<groupId>com.j2html</groupId>
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/botdarr/clients/ChatClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@

public interface ChatClient<T extends ChatClientResponse> {
void sendToConfiguredChannels(List<T> chatClientResponses);
void cleanup();
}
1 change: 1 addition & 0 deletions src/main/java/com/botdarr/clients/ChatClientBootstrap.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ protected <T extends ChatClientResponse> void initScheduling(ChatClient<T> chatC
//make sure to always cache before doing any notifications
scheduler.initApiCaching(apis);
scheduler.initApiNotifications(apis, chatClient, responseBuilder);
scheduler.initCleanup(chatClient);
}

protected <T extends ChatClientResponse> void runAndProcessCommands(String prefix,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ public void sendToConfiguredChannels(List<DiscordResponse> chatClientResponses)
}, null);
}

@Override
public void cleanup() {
// nothing to cleanup
}

public void sendMessage(DiscordResponse chatClientResponse, String channelName) {
sendMessages(channel -> channel.sendMessage(chatClientResponse.getMessage()).queue(), channelName);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ public void sendToConfiguredChannels(List<MatrixResponse> chatClientResponses) {
}
}

@Override
public void cleanup() {
// nothing to cleanup
}

public void sendMessage(MatrixResponse response, String roomId) {
sendMatrixResponse(response, roomId);
}
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/com/botdarr/clients/slack/SlackChatClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ public void sendToConfiguredChannels(List<SlackResponse> chatClientResponses) {
sendMessage(chatClientResponses, null);
}

@Override
public void cleanup() {
// nothing to cleanup
}

private interface MessageSender {
void send(String channel);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Integer> 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();
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ public void sendToConfiguredChannels(List<TelegramResponse> chatClientResponses)
sendMessage(chatClientResponses, null);
}

@Override
public void cleanup() {
// remove any 10 day old callbacks
new TelegramCallbackManager().deleteOldCallbacks();
}

private interface MessageSender {
void send(Chat chatChannel);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ public TelegramResponse build(NewMusicArtistResponse newMusicArtistResponse) {
List<DomContent> 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()));
}

Expand Down Expand Up @@ -338,11 +338,11 @@ private TelegramResponse getAddResponse(List<DomContent> 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);
}
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/com/botdarr/scheduling/Scheduler.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ public static Scheduler getScheduler() {
return instance;
}

public <T extends ChatClientResponse> void initCleanup(ChatClient<T> chatClient) {
if (cleanupFuture == null) {
cleanupFuture = Executors.newScheduledThreadPool(1).scheduleWithFixedDelay(chatClient::cleanup, 0, 5 , TimeUnit.MINUTES);
}
}

public <T extends ChatClientResponse> void initApiNotifications(List<Api> apis, ChatClient<T> chatClient, ChatClientResponseBuilder<T> responseBuilder) {
if (notificationFuture == null) {
Expand Down Expand Up @@ -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();
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/com/botdarr/utilities/DateWrapper.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.botdarr.utilities;

import java.time.LocalDate;

public class DateWrapper {
public LocalDate getNow() {
return LocalDate.now();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
create table telegram_callbacks (id integer primary key not null, callback text not null, createdDt date not null);
2 changes: 1 addition & 1 deletion src/main/resources/version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
5.6.6
5.6.7
14 changes: 1 addition & 13 deletions src/test/java/com/botdarr/api/ApiRequestsTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -243,19 +244,6 @@ public void validate() {
private final Throwable expectedThrowable;
}

private static class MockedDatabase extends MockUp<DatabaseHelper> {
private MockedDatabase(File temporaryDatabase) {
this.temporaryDatabase = temporaryDatabase;
}

@Mock
public File getDatabaseFile() {
return temporaryDatabase;
}

private final File temporaryDatabase;
}

@Rule
public TemporaryFolder temporaryFolder = new TemporaryFolder();
}
Original file line number Diff line number Diff line change
@@ -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();
}
19 changes: 19 additions & 0 deletions src/test/java/com/botdarr/database/MockedDatabase.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.botdarr.database;

import mockit.Mock;
import mockit.MockUp;

import java.io.File;

public class MockedDatabase extends MockUp<DatabaseHelper> {
public MockedDatabase(File temporaryDatabase) {
this.temporaryDatabase = temporaryDatabase;
}

@Mock
public File getDatabaseFile() {
return temporaryDatabase;
}

private final File temporaryDatabase;
}

0 comments on commit 7a9b0e0

Please sign in to comment.