Skip to content

Commit a6efb2c

Browse files
committed
applies FirasRG first review
Comments: 1. improved settings 2. using meaningful names 3. validations 4. some debug logs
1 parent 0946b54 commit a6efb2c

File tree

4 files changed

+71
-37
lines changed

4 files changed

+71
-37
lines changed

application/config.json.template

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,9 @@
194194
"videoLinkPattern": "http(s)?://www\\.youtube.com.*",
195195
"pollIntervalInMinutes": 10
196196
},
197-
"quoteMessagesConfig": {
198-
"minimumReactions": 2,
199-
"boardChannelPattern": "quotes",
197+
"quoteBoardConfig": {
198+
"minimumReactionsToTrigger": 5,
199+
"channel": "quotes",
200200
"reactionEmoji": "⭐"
201201
},
202202
"memberCountCategoryPattern": "Info",

application/src/main/java/org/togetherjava/tjbot/config/Config.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public final class Config {
4848
private final RSSFeedsConfig rssFeedsConfig;
4949
private final String selectRolesChannelPattern;
5050
private final String memberCountCategoryPattern;
51-
private final QuoteBoardConfig quoteMessagesConfig;
51+
private final QuoteBoardConfig quoteBoardConfig;
5252
private final TopHelpersConfig topHelpers;
5353

5454
@SuppressWarnings("ConstructorWithTooManyParameters")
@@ -103,8 +103,8 @@ private Config(@JsonProperty(value = "token", required = true) String token,
103103
@JsonProperty(value = "rssConfig", required = true) RSSFeedsConfig rssFeedsConfig,
104104
@JsonProperty(value = "selectRolesChannelPattern",
105105
required = true) String selectRolesChannelPattern,
106-
@JsonProperty(value = "quoteMessagesConfig",
107-
required = true) QuoteBoardConfig quoteMessagesConfig,
106+
@JsonProperty(value = "quoteBoardConfig",
107+
required = true) QuoteBoardConfig quoteBoardConfig,
108108
@JsonProperty(value = "topHelpers", required = true) TopHelpersConfig topHelpers) {
109109
this.token = Objects.requireNonNull(token);
110110
this.githubApiKey = Objects.requireNonNull(githubApiKey);
@@ -140,7 +140,7 @@ private Config(@JsonProperty(value = "token", required = true) String token,
140140
this.featureBlacklistConfig = Objects.requireNonNull(featureBlacklistConfig);
141141
this.rssFeedsConfig = Objects.requireNonNull(rssFeedsConfig);
142142
this.selectRolesChannelPattern = Objects.requireNonNull(selectRolesChannelPattern);
143-
this.quoteMessagesConfig = Objects.requireNonNull(quoteMessagesConfig);
143+
this.quoteBoardConfig = Objects.requireNonNull(quoteBoardConfig);
144144
this.topHelpers = Objects.requireNonNull(topHelpers);
145145
}
146146

@@ -443,8 +443,8 @@ public String getSelectRolesChannelPattern() {
443443
*
444444
* @return configuration of quote messages config
445445
*/
446-
public QuoteBoardConfig getQuoteMessagesConfig() {
447-
return quoteMessagesConfig;
446+
public QuoteBoardConfig getQuoteBoardConfig() {
447+
return quoteBoardConfig;
448448
}
449449

450450
/**

application/src/main/java/org/togetherjava/tjbot/config/QuoteBoardConfig.java

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.fasterxml.jackson.annotation.JsonProperty;
44
import com.fasterxml.jackson.annotation.JsonRootName;
5+
import org.apache.logging.log4j.LogManager;
56

67
import org.togetherjava.tjbot.features.basic.QuoteBoardForwarder;
78

@@ -10,21 +11,33 @@
1011
/**
1112
* Configuration for the quote board feature, see {@link QuoteBoardForwarder}.
1213
*/
13-
@JsonRootName("coolMessagesConfig")
14+
@JsonRootName("quoteBoardConfig")
1415
public record QuoteBoardConfig(
15-
@JsonProperty(value = "minimumReactions", required = true) int minimumReactions,
16-
@JsonProperty(value = "boardChannelPattern", required = true) String boardChannelPattern,
16+
@JsonProperty(value = "minimumReactionsToTrigger", required = true) int minimumReactions,
17+
@JsonProperty(required = true) String channel,
1718
@JsonProperty(value = "reactionEmoji", required = true) String reactionEmoji) {
1819

1920
/**
2021
* Creates a QuoteBoardConfig.
2122
*
2223
* @param minimumReactions the minimum amount of reactions
23-
* @param boardChannelPattern the pattern for the board channel
24+
* @param channel the pattern for the board channel
2425
* @param reactionEmoji the emoji with which users should react to
2526
*/
2627
public QuoteBoardConfig {
27-
Objects.requireNonNull(boardChannelPattern);
28+
if (minimumReactions <= 0) {
29+
throw new IllegalArgumentException("minimumReactions must be greater than zero");
30+
}
31+
Objects.requireNonNull(channel);
32+
if (channel.isBlank()) {
33+
throw new IllegalArgumentException("channel must not be empty or blank");
34+
}
2835
Objects.requireNonNull(reactionEmoji);
36+
if (reactionEmoji.isBlank()) {
37+
throw new IllegalArgumentException("reactionEmoji must not be empty or blank");
38+
}
39+
LogManager.getLogger(QuoteBoardConfig.class)
40+
.debug("Quote-Board configs loaded: minimumReactions={}, channel='{}', reactionEmoji='{}'",
41+
minimumReactions, channel, reactionEmoji);
2942
}
3043
}

application/src/main/java/org/togetherjava/tjbot/features/basic/QuoteBoardForwarder.java

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.togetherjava.tjbot.config.QuoteBoardConfig;
1515
import org.togetherjava.tjbot.features.MessageReceiverAdapter;
1616

17+
import java.util.List;
1718
import java.util.Optional;
1819
import java.util.function.Predicate;
1920
import java.util.regex.Pattern;
@@ -45,47 +46,59 @@ public final class QuoteBoardForwarder extends MessageReceiverAdapter {
4546
* including the reaction emoji and the pattern to match board channel names
4647
*/
4748
public QuoteBoardForwarder(Config config) {
48-
this.config = config.getQuoteMessagesConfig();
49+
this.config = config.getQuoteBoardConfig();
4950
this.triggerReaction = Emoji.fromUnicode(this.config.reactionEmoji());
5051

51-
isQuoteBoardChannelName =
52-
Pattern.compile(this.config.boardChannelPattern()).asMatchPredicate();
52+
this.isQuoteBoardChannelName = Pattern.compile(this.config.channel()).asMatchPredicate();
5353
}
5454

5555
@Override
5656
public void onMessageReactionAdd(MessageReactionAddEvent event) {
57+
logger.debug("Received MessageReactionAddEvent: messageId={}, channelId={}, userId={}",
58+
event.getMessageId(), event.getChannel().getId(), event.getUserId());
59+
5760
final MessageReaction messageReaction = event.getReaction();
58-
boolean isTriggerEmoji = messageReaction.getEmoji().equals(triggerReaction);
59-
long guildId = event.getGuild().getIdLong();
6061

61-
if (!isTriggerEmoji) {
62+
if (!messageReaction.getEmoji().equals(triggerReaction)) {
63+
logger.debug("Reaction emoji '{}' does not match trigger emoji '{}'. Ignoring.",
64+
messageReaction.getEmoji(), triggerReaction);
6265
return;
6366
}
6467

6568
if (hasAlreadyForwardedMessage(event.getJDA(), messageReaction)) {
69+
logger.debug("Message has already been forwarded by the bot. Skipping.");
6670
return;
6771
}
6872

69-
final int reactionsCount = (int) messageReaction.retrieveUsers().stream().count();
70-
71-
if (reactionsCount < config.minimumReactions()) {
73+
long reactionCount = messageReaction.retrieveUsers().stream().count();
74+
if (reactionCount < config.minimumReactions()) {
75+
logger.debug("Reaction count {} is less than minimum required {}. Skipping.",
76+
reactionCount, config.minimumReactions());
7277
return;
7378
}
7479

80+
final long guildId = event.getGuild().getIdLong();
81+
7582
Optional<TextChannel> boardChannel = findQuoteBoardChannel(event.getJDA(), guildId);
7683

7784
if (boardChannel.isEmpty()) {
7885
logger.warn(
7986
"Could not find board channel with pattern '{}' in server with ID '{}'. Skipping reaction handling...",
80-
this.config.boardChannelPattern(), guildId);
87+
this.config.channel(), guildId);
8188
return;
8289
}
8390

91+
logger.debug("Forwarding message to quote board channel: {}", boardChannel.get().getName());
92+
8493
event.retrieveMessage()
85-
.queue(message -> markAsProcessed(message).flatMap(v -> message
86-
.forwardTo(boardChannel.orElseThrow())).queue(), e -> logger.warn(
87-
"Unknown error while attempting to retrieve and forward message for quote-board, message is ignored.",
88-
e));
94+
.queue(message -> markAsProcessed(message)
95+
.flatMap(v -> message.forwardTo(boardChannel.orElseThrow()))
96+
.queue(_ -> logger.debug("Message forwarded to quote board channel: {}",
97+
boardChannel.get().getName())),
98+
99+
e -> logger.warn(
100+
"Unknown error while attempting to retrieve and forward message for quote-board, message is ignored.",
101+
e));
89102

90103
}
91104

@@ -101,18 +114,26 @@ private RestAction<Void> markAsProcessed(Message message) {
101114
* @return the board text channel
102115
*/
103116
private Optional<TextChannel> findQuoteBoardChannel(JDA jda, long guildId) {
104-
return jda.getGuildById(guildId)
105-
.getTextChannelCache()
117+
var guild = jda.getGuildById(guildId);
118+
119+
if (guild == null) {
120+
throw new IllegalStateException(
121+
String.format("Guild with ID '%d' not found.", guildId));
122+
}
123+
124+
List<TextChannel> matchingChannels = guild.getTextChannelCache()
106125
.stream()
107126
.filter(channel -> isQuoteBoardChannelName.test(channel.getName()))
108-
.findAny();
109-
}
127+
.toList();
110128

111-
/**
112-
* Inserts a message to the specified text channel
113-
*
114-
* @return a {@link MessageCreateAction} of the call to make
115-
*/
129+
if (matchingChannels.size() > 1) {
130+
logger.warn(
131+
"Multiple quote board channels found matching pattern '{}' in guild with ID '{}'. Selecting the first one anyway.",
132+
this.config.channel(), guildId);
133+
}
134+
135+
return matchingChannels.stream().findFirst();
136+
}
116137

117138
/**
118139
* Checks a {@link MessageReaction} to see if the bot has reacted to it.

0 commit comments

Comments
 (0)