feature/handle-similar-messages-as-scam #1292

Alathreon · 2025-07-31T21:32:09Z

From in #1283 with those differences:

Whole feature uses its own class
I used a local cache
I didn't use caffeeine
4 new config variables
A few fail safes
Hash is based on SHA

Zabuzard

Good stuff, thanks! I got some remarks regarding readability mostly 👍

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

application/src/main/java/org/togetherjava/tjbot/features/moderation/scam/ScamBlocker.java

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

application/src/main/java/org/togetherjava/tjbot/features/moderation/scam/ScamBlocker.java

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

gradlew.bat

application/src/main/java/org/togetherjava/tjbot/features/moderation/scam/ScamBlocker.java

…ngs and refactored existing code

sonarqubecloud · 2025-08-03T21:55:44Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Zabuzard · 2025-08-05T07:13:21Z

...n/src/main/java/org/togetherjava/tjbot/features/moderation/scam/SimilarMessagesDetector.java

+        if (alreadyFlaggedUsers.contains(userId)) {
+            return true;
+        }
+        if (shouldIgnore(event.getMessage())) {
+            return false;
+        }
+        String hash = addToMessageCache(event).messageHash();
+        if (hasPostedTooManySimilarMessages(userId, hash)) {
+            alreadyFlaggedUsers.add(userId);
+            return true;
+        } else {
+            return false;


i am not sure if that alreadyFlaggedUsers user thing really works.. the bot has a really long uptime usually, sometimes many months.
the way i read the code is that if a user got flagged once, they will then be immune to this check until the bot is restarted again. u could argue that its probably not an issue in practice but it smells a bit. you would need a time based cleanup for alreadyFlaggedUsers as well.

at which point u have like 30 lines of code dealing with what caffeeine gets done in one line, i guess.

private final Cache<String, Instant> userIdToAskedAtCache = Caffeine.newBuilder() .maximumSize(1_000) .expireAfterWrite(Duration.of(10, ChronoUnit.SECONDS)) .build();

the interface is essentially that of a Map, so you have various get and put variants. internally its a LRU map, so it kicks out the entries that werent touched the longest when it reaches the max limit.

how do i make it a set ? if the cache already handles he cleanup, then storing the instant is pointless.
edit: i saw your other comment, i'll see that

caffeine only has a Map. sometimes in the past we also only needed a Set and then mapped it to something. for example to itself, i.e. key == value.

Zabuzard · 2025-08-05T07:16:45Z

application/src/main/java/org/togetherjava/tjbot/features/utils/Hashing.java

+     * @param text the UTF 8 text to hash
+     * @return the computed hash
+     */
+    public static byte[] hashUTF8(String method, String text) {


NIT: should probably be renamed into hashUtf8 for better camelCase readability

Zabuzard · 2025-08-05T07:18:14Z

...n/src/main/java/org/togetherjava/tjbot/features/moderation/scam/SimilarMessagesDetector.java

+    private MessageInfo addToMessageCache(MessageReceivedEvent event) {
+        long userId = event.getAuthor().getIdLong();
+        long channelId = event.getChannel().getIdLong();
+        String messageHash = getHash(event.getMessage());
+        Instant timestamp = event.getMessage().getTimeCreated().toInstant();
+        MessageInfo messageInfo = new MessageInfo(userId, channelId, messageHash, timestamp);
+        messageCache.add(messageInfo);
+        return messageInfo;


id still refactor this and move all the stuff dealing with MessageInfo into the record. so:

record MessageInfo(...) { static MessageInfo fromMessageEvent(MessageReceivedEvent event) {...} }

then this addToMessageCache method becomes obsolete and can be replaced with just messageCache.add(MessageInfo.fromMessageEvent(event)); at the call-site

the hash creation would then also move into the records fromMessageEvent method (or as private static helper in the record)

I don't quite like adding a unnecessary dependencies to a record, and even less logic like how to create a hash or how to create it from an event imo.

mh... but its a factory to create instances of this record. so imo this is exactly the place it should be. what do others here think?

Zabuzard · 2025-08-05T07:20:06Z

...n/src/main/java/org/togetherjava/tjbot/features/moderation/scam/SimilarMessagesDetector.java

+     * @param event the message event
+     * @return true if the user spammed the message in several channels, false otherwise
+     */
+    public boolean doSimilarMessageCheck(MessageReceivedEvent event) {


could u do some reordering so the flow reads top to bottom. this method would then be at the top, as entry-point. and the private methods then below it in the order they are used. the runRoutine can stay at the very bottom, i guess, since its a detached flow.

Zabuzard · 2025-08-05T07:21:56Z

application/config.json.template

-        "suspiciousAttachmentNamePattern": "(image|\\d{1,2})\\.[^.]{0,5}"
+        "suspiciousAttachmentNamePattern": "(image|\\d{1,2})\\.[^.]{0,5}",
+        "maxAllowedSimilarMessages": 2,
+        "similarMessagesWindow": 1,


missing time unit. please add it to the name.

Zabuzard · 2025-08-05T07:22:59Z

application/config.json.template

+        "maxAllowedSimilarMessages": 2,
+        "similarMessagesWindow": 1,
+        "similarMessageLengthIgnore": 10,
+        "similarMessagesWhitelist": []


i dont think we need that, please remove.

the UX for it for maintainers would be really weird, having to bloat the config with 100 character long messages and whatnot. and then it didnt work bc of a smiley or other stuff that ends up slightly different.

All 4? or just the whitelist ?

sorry, the preview didnt turn out the way i wanted. i was referring to the message whitelist.

Zabuzard · 2025-08-05T07:30:03Z

application/src/main/java/org/togetherjava/tjbot/features/moderation/scam/ScamBlocker.java

@@ -141,6 +137,10 @@ public void onMessageReceived(MessageReceivedEvent event) {
            isSafe = false;
        }

+        if (isSafe && similarMessagesDetector.doSimilarMessageCheck(event)) {


doSimilarMessageCheck should be renamed. maybe hasPostedSimilarMessageTooOften(event)

if you dont want it to start with a question-word bc of the side effects, maybe handleHasPostedSimilarMessagesTooOften(event).

but the name should clarify what the boolean it returns means and how it fits into the logic of ScamBlocker

Zabuzard · 2025-08-05T07:31:04Z

...n/src/main/java/org/togetherjava/tjbot/features/moderation/scam/SimilarMessagesDetector.java

+    /**
+     * Has to be called often to clear the cache.
+     */
+    public void runRoutine() {


rename to clearCache or cleanupCache or something.

Zabuzard · 2025-08-05T07:31:34Z

...n/src/main/java/org/togetherjava/tjbot/features/moderation/scam/SimilarMessagesDetector.java

+        return similarMessageCount > scamBlockerConfig.getMaxAllowedSimilarMessages();
+    }
+
+    private boolean isObsolete(MessageInfo messageInfo) {


rename to isOutdated or isExpired

Zabuzard · 2025-08-05T07:33:06Z

...n/src/main/java/org/togetherjava/tjbot/features/moderation/scam/SimilarMessagesDetector.java

+            return true;
+        }
+        return scamBlockerConfig.getSimilarMessagesWhitelist()
+            .contains(message.getContentRaw().toLowerCase());


.toLowerCase needs Locale.US

...n/src/main/java/org/togetherjava/tjbot/features/moderation/scam/SimilarMessagesDetector.java

Alathreon requested a review from a team as a code owner July 31, 2025 21:32

Alathreon linked an issue Jul 31, 2025 that may be closed by this pull request

Auto ban/quarantine based on similar messages #1283

Open

Alathreon force-pushed the feature/handle-similar-messages-as-scam branch from 2842442 to c160a4d Compare July 31, 2025 21:33

SquidXTV added enhancement New feature or request config-changes if your PR contains any changes related to config file labels Aug 1, 2025

Zabuzard requested changes Aug 2, 2025

View reviewed changes

Alathreon added 2 commits August 3, 2025 23:52

[feature/handle-similar-messages-as-scam] Added a method to hash stri…

e800cde

…ngs and refactored existing code

[feature/handle-similar-messages-as-scam]

70375f7

Alathreon force-pushed the feature/handle-similar-messages-as-scam branch from c160a4d to 70375f7 Compare August 3, 2025 21:52

Zabuzard requested changes Aug 5, 2025

View reviewed changes

Uh oh!

feature/handle-similar-messages-as-scam #1292

Are you sure you want to change the base?

feature/handle-similar-messages-as-scam #1292

Uh oh!

Conversation

Alathreon commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Zabuzard left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Aug 3, 2025

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Alathreon Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Alathreon commented Jul 31, 2025 •

edited

Loading

Alathreon Aug 5, 2025 •

edited

Loading