feature/handle-similar-messages-as-scam #1292
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| package org.togetherjava.tjbot.features.moderation.scam; | ||
|
|
||
|
|
||
| import java.time.Instant; | ||
| import java.util.Objects; | ||
|
|
||
| /** | ||
| * Information about a message, used to detect spam of the same message by the same user in | ||
| * different channels. | ||
| * | ||
| * @param userId the id of the user | ||
| * @param channelId the channel where the message was posted | ||
| * @param messageHash the hash of the message | ||
| * @param timestamp when the message was posted | ||
| */ | ||
| public record MessageInfo(long userId, long channelId, String messageHash, Instant timestamp) { | ||
|
|
||
| @Override | ||
| public boolean equals(Object other) { | ||
| return other instanceof MessageInfo message && this.userId == message.userId | ||
| && this.channelId == message.channelId; | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(userId, channelId); | ||
| } | ||
|
Comment on lines
+18
to
+27
There was a problem hiding this comment. i find that quite confusing, not what someone using this record would expect. if you need this to determine duplicates in ur There was a problem hiding this comment. So I tried and the problem is that by using a map it just makes the code less readable. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| package org.togetherjava.tjbot.features.moderation.scam; | ||
|
|
||
| import net.dv8tion.jda.api.entities.Message; | ||
| import net.dv8tion.jda.api.events.message.MessageReceivedEvent; | ||
|
|
||
| import org.togetherjava.tjbot.config.ScamBlockerConfig; | ||
| import org.togetherjava.tjbot.features.utils.Hashing; | ||
|
|
||
| import java.time.Instant; | ||
| import java.time.temporal.ChronoUnit; | ||
| import java.util.HashSet; | ||
| import java.util.Set; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| /** | ||
| * Class which tries to detect scams by monitoring similar messages. | ||
| */ | ||
| public class SimilarMessagesDetector { | ||
| private static final String HASH_METHOD = "SHA"; | ||
|
|
||
| private final ScamBlockerConfig scamBlockerConfig; | ||
| private final Set<MessageInfo> messageCache; | ||
| private final Set<Long> alreadyFlaggedUsers; | ||
|
|
||
| /** | ||
| * Creates an instance of this class by using the given config. | ||
| * | ||
| * @param scamBlockerConfig the scam config | ||
| */ | ||
| public SimilarMessagesDetector(ScamBlockerConfig scamBlockerConfig) { | ||
| this.scamBlockerConfig = scamBlockerConfig; | ||
| this.messageCache = new HashSet<>(); | ||
| this.alreadyFlaggedUsers = new HashSet<>(); | ||
| } | ||
|
|
||
| private boolean shouldIgnore(Message message) { | ||
| if (!message.getAttachments().isEmpty()) { | ||
| return false; | ||
| } | ||
| if (message.getContentRaw().length() <= scamBlockerConfig.getSimilarMessageLengthIgnore()) { | ||
| return true; | ||
| } | ||
| return scamBlockerConfig.getSimilarMessagesWhitelist() | ||
| .contains(message.getContentRaw().toLowerCase()); | ||
|
|
||
| } | ||
|
|
||
| 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; | ||
|
Comment on lines
+47
to
+54
There was a problem hiding this comment. id still refactor this and move all the stuff dealing with record MessageInfo(...) {
static MessageInfo fromMessageEvent(MessageReceivedEvent event) {...}
}then this the hash creation would then also move into the records There was a problem hiding this comment. 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. There was a problem hiding this comment. 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? |
||
| } | ||
|
|
||
| private String getHash(Message message) { | ||
| String wholeText = message.getContentRaw() + message.getAttachments() | ||
| .stream() | ||
| .map(Message.Attachment::getFileName) | ||
| .collect(Collectors.joining()); | ||
| return Hashing.bytesToHex(Hashing.hashUTF8(HASH_METHOD, wholeText)); | ||
| } | ||
|
|
||
| private boolean hasPostedTooManySimilarMessages(long userId, String messageHash) { | ||
| long similarMessageCount = messageCache.stream() | ||
| .filter(m -> m.userId() == userId && m.messageHash().equals(messageHash) | ||
| && !isObsolete(m)) | ||
| .count(); | ||
| return similarMessageCount > scamBlockerConfig.getMaxAllowedSimilarMessages(); | ||
Zabuzard marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| private boolean isObsolete(MessageInfo messageInfo) { | ||
|
There was a problem hiding this comment. rename to |
||
| return messageInfo.timestamp() | ||
| .plus(scamBlockerConfig.getSimilarMessagesWindow(), ChronoUnit.MINUTES) | ||
| .isBefore(Instant.now()); | ||
| } | ||
|
|
||
| /** | ||
| * Stores message data and if many messages of same author, different channel and same content | ||
| * is posted several times, returns true. | ||
| * | ||
| * @param event the message event | ||
| * @return true if the user spammed the message in several channels, false otherwise | ||
| */ | ||
| public boolean doSimilarMessageCheck(MessageReceivedEvent event) { | ||
|
There was a problem hiding this comment. 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 |
||
| long userId = event.getAuthor().getIdLong(); | ||
| 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; | ||
|
Comment on lines
+88
to
+99
There was a problem hiding this comment. i am not sure if that 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 There was a problem hiding this comment. how do i make it a set ? if the cache already handles he cleanup, then storing the instant is pointless. There was a problem hiding this comment. caffeine only has a |
||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Has to be called often to clear the cache. | ||
| */ | ||
| public void runRoutine() { | ||
|
There was a problem hiding this comment. rename to |
||
| messageCache.removeIf(this::isObsolete); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.