From 210abad3f4876a694d7e33d86e5a6979ab2754c8 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Tue, 22 Feb 2022 08:56:53 +0100 Subject: [PATCH 1/2] Skipping audit logs against bot message deletions --- .../tjbot/routines/ModAuditLogRoutine.java | 68 +++++++++++-------- 1 file changed, 39 insertions(+), 29 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/routines/ModAuditLogRoutine.java b/application/src/main/java/org/togetherjava/tjbot/routines/ModAuditLogRoutine.java index 2e9cb9f95f..86939ac9e1 100644 --- a/application/src/main/java/org/togetherjava/tjbot/routines/ModAuditLogRoutine.java +++ b/application/src/main/java/org/togetherjava/tjbot/routines/ModAuditLogRoutine.java @@ -27,6 +27,7 @@ import java.time.temporal.TemporalAccessor; import java.util.*; import java.util.List; +import java.util.concurrent.CancellationException; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import java.util.function.BiFunction; @@ -70,33 +71,15 @@ public ModAuditLogRoutine(@NotNull Database database, @NotNull Config config) { this.database = database; } - private static @NotNull RestAction handleAction(@NotNull Action action, + private static @NotNull RestAction handleAction(@NotNull Action action, @NotNull AuditLogEntry entry) { User author = Objects.requireNonNull(entry.getUser()); - return getTargetTagFromEntry(entry).map(targetTag -> createMessage(author, action, - targetTag, entry.getReason(), entry.getTimeCreated())); + return getTargetFromEntryOrNull(entry).map(target -> new AuditLogMessage(author, action, + target, entry.getReason(), entry.getTimeCreated())); } - private static @NotNull MessageEmbed createMessage(@NotNull User author, @NotNull Action action, - @NotNull String targetTag, @Nullable String reason, - @NotNull TemporalAccessor timestamp) { - String description = "%s **%s**.".formatted(action.getVerb(), targetTag); - if (reason != null && !reason.isBlank()) { - description += "\n\nReason: " + reason; - } - return new EmbedBuilder().setAuthor(author.getAsTag(), null, author.getAvatarUrl()) - .setDescription(description) - .setTimestamp(timestamp) - .setColor(AMBIENT_COLOR) - .build(); - } - - private static RestAction getTargetTagFromEntry(@NotNull AuditLogEntry entry) { - // If the target is null, the user got deleted in the meantime - return entry.getJDA() - .retrieveUserById(entry.getTargetIdLong()) - .onErrorMap(error -> null) - .map(target -> target == null ? "(user unknown)" : target.getAsTag()); + private static RestAction getTargetFromEntryOrNull(@NotNull AuditLogEntry entry) { + return entry.getJDA().retrieveUserById(entry.getTargetIdLong()).onErrorMap(error -> null); } private static boolean isSnowflakeAfter(@NotNull ISnowflake snowflake, @@ -178,34 +161,42 @@ private static boolean isSnowflakeAfter(@NotNull ISnowflake snowflake, @NotNull AuditLogEntry entry) { // NOTE Temporary bans are realized as permanent bans with automated unban, // hence we can not differentiate a permanent or a temporary ban here - return Optional.of(handleAction(Action.BAN, entry)); + return Optional.of(handleAction(Action.BAN, entry).map(AuditLogMessage::toEmbed)); } private static @NotNull Optional> handleUnbanEntry( @NotNull AuditLogEntry entry) { - return Optional.of(handleAction(Action.UNBAN, entry)); + return Optional.of(handleAction(Action.UNBAN, entry).map(AuditLogMessage::toEmbed)); } private static @NotNull Optional> handleKickEntry( @NotNull AuditLogEntry entry) { - return Optional.of(handleAction(Action.KICK, entry)); + return Optional.of(handleAction(Action.KICK, entry).map(AuditLogMessage::toEmbed)); } private static @NotNull Optional> handleMuteEntry( @NotNull AuditLogEntry entry) { // NOTE Temporary mutes are realized as permanent mutes with automated unmute, // hence we can not differentiate a permanent or a temporary mute here - return Optional.of(handleAction(Action.MUTE, entry)); + return Optional.of(handleAction(Action.MUTE, entry).map(AuditLogMessage::toEmbed)); } private static @NotNull Optional> handleUnmuteEntry( @NotNull AuditLogEntry entry) { - return Optional.of(handleAction(Action.UNMUTE, entry)); + return Optional.of(handleAction(Action.UNMUTE, entry).map(AuditLogMessage::toEmbed)); } private static @NotNull Optional> handleMessageDeleteEntry( @NotNull AuditLogEntry entry) { - return Optional.of(handleAction(Action.MESSAGE_DELETION, entry)); + return Optional.of(handleAction(Action.MESSAGE_DELETION, entry).map(message -> { + if (message.target() != null && message.target().isBot()) { + // NOTE Unfortunately, JDA does not seem to offer any other way + // to gracefully enter the failed-flow of RestAction, other than throwing. + throw new CancellationException( + "Message deletions against bots should be skipped. Action got cancelled."); + } + return message.toEmbed(); + })); } @Override @@ -359,4 +350,23 @@ String getVerb() { return verb; } } + + private record AuditLogMessage(@NotNull User author, @NotNull Action action, + @Nullable User target, @Nullable String reason, @NotNull TemporalAccessor timestamp) { + @NotNull + MessageEmbed toEmbed() { + String targetTag = target == null ? "(user unknown)" : target.getAsTag(); + String description = "%s **%s**.".formatted(action.getVerb(), targetTag); + + if (reason != null && !reason.isBlank()) { + description += "\n\nReason: " + reason; + } + + return new EmbedBuilder().setAuthor(author.getAsTag(), null, author.getAvatarUrl()) + .setDescription(description) + .setTimestamp(timestamp) + .setColor(AMBIENT_COLOR) + .build(); + } + } } From ef0d97c4d87905b88264700358572527be00c273 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Wed, 23 Feb 2022 11:45:26 +0100 Subject: [PATCH 2/2] Bugfix with broken RestAction pipeline --- .../tjbot/routines/ModAuditLogRoutine.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/routines/ModAuditLogRoutine.java b/application/src/main/java/org/togetherjava/tjbot/routines/ModAuditLogRoutine.java index 86939ac9e1..e3beb603dd 100644 --- a/application/src/main/java/org/togetherjava/tjbot/routines/ModAuditLogRoutine.java +++ b/application/src/main/java/org/togetherjava/tjbot/routines/ModAuditLogRoutine.java @@ -25,9 +25,8 @@ import java.time.*; import java.time.temporal.ChronoUnit; import java.time.temporal.TemporalAccessor; -import java.util.*; import java.util.List; -import java.util.concurrent.CancellationException; +import java.util.*; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import java.util.function.BiFunction; @@ -190,10 +189,8 @@ private static boolean isSnowflakeAfter(@NotNull ISnowflake snowflake, @NotNull AuditLogEntry entry) { return Optional.of(handleAction(Action.MESSAGE_DELETION, entry).map(message -> { if (message.target() != null && message.target().isBot()) { - // NOTE Unfortunately, JDA does not seem to offer any other way - // to gracefully enter the failed-flow of RestAction, other than throwing. - throw new CancellationException( - "Message deletions against bots should be skipped. Action got cancelled."); + // Message deletions against bots should be skipped. Cancel action. + return null; } return message.toEmbed(); })); @@ -265,8 +262,7 @@ private void handleAuditLogs(@NotNull MessageChannel auditLogChannel, .sorted(Comparator.comparing(TimeUtil::getTimeCreated)) .map(entry -> handleAuditLog(auditLogChannel, entry)) .flatMap(Optional::stream) - .reduce((firstMessage, secondMessage) -> firstMessage.flatMap(result -> secondMessage)) - .ifPresent(RestAction::queue); + .forEach(RestAction::queue); database.write(context -> { var entry = context.newRecord(ModAuditLogGuildProcess.MOD_AUDIT_LOG_GUILD_PROCESS); @@ -289,7 +285,13 @@ private Optional> handleAuditLog(@NotNull MessageChannel aud case MESSAGE_DELETE -> handleMessageDeleteEntry(entry); default -> Optional.empty(); }; - return maybeMessage.map(message -> message.flatMap(auditLogChannel::sendMessageEmbeds)); + // It can have 3 states: + // * empty optional - entry is irrelevant and should not be logged + // * has RestAction but that will contain null - entry was relevant at first, but at + // query-time we found out that it is irrelevant + // * has RestAction but will contain a message - entry is relevant, log the message + return maybeMessage + .map(message -> message.flatMap(Objects::nonNull, auditLogChannel::sendMessageEmbeds)); } private @NotNull Optional> handleRoleUpdateEntry(