From 40ce505f1f7410ba0f40c516ce4352bae69487d0 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Fri, 7 Oct 2022 12:30:12 +0200 Subject: [PATCH 1/3] Moved abbreviate into dedicated class, with unit test --- .../tjbot/commands/utils/MessageUtils.java | 25 +++++++++++- .../logging/discord/DiscordLogForwarder.java | 15 +------ .../commands/utils/MessageUtilsTest.java | 40 ++++++++++++++----- 3 files changed, 57 insertions(+), 23 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/utils/MessageUtils.java b/application/src/main/java/org/togetherjava/tjbot/commands/utils/MessageUtils.java index 978d8843c4..4768006d75 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/utils/MessageUtils.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/utils/MessageUtils.java @@ -17,6 +17,8 @@ * other commands to avoid similar methods appearing everywhere. */ public class MessageUtils { + private static final String ABBREVIATION = "..."; + private MessageUtils() { throw new UnsupportedOperationException("Utility class, construction not supported"); } @@ -43,7 +45,7 @@ public static void disableButtons(Message message) { /** * Escapes every markdown content in the given string. - * + *

* If the escaped message is sent to Discord, it will display the original message. * * @param text the text to escape @@ -86,4 +88,25 @@ public static RestAction mentionSlashCommand(Guild guild, String command }); } + /** + * Abbreviates the given text if it is too long. + *

+ * Abbreviation is done by adding {@value ABBREVIATION}. + * + * @param text the text to abbreviate + * @param maxLength the maximal length of the abbreviated text + * @return the abbreviated text, guaranteed to be smaller than the given length + */ + public static String abbreviate(String text, int maxLength) { + if (text.length() <= maxLength) { + return text; + } + + if (maxLength < ABBREVIATION.length()) { + return text.substring(0, maxLength); + } + + return text.substring(0, maxLength - ABBREVIATION.length()) + ABBREVIATION; + } + } diff --git a/application/src/main/java/org/togetherjava/tjbot/logging/discord/DiscordLogForwarder.java b/application/src/main/java/org/togetherjava/tjbot/logging/discord/DiscordLogForwarder.java index f3cac0731a..0339ea84b7 100644 --- a/application/src/main/java/org/togetherjava/tjbot/logging/discord/DiscordLogForwarder.java +++ b/application/src/main/java/org/togetherjava/tjbot/logging/discord/DiscordLogForwarder.java @@ -10,6 +10,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.togetherjava.tjbot.commands.utils.MessageUtils; import org.togetherjava.tjbot.logging.LogMarkers; import java.io.PrintWriter; @@ -122,7 +123,7 @@ private static LogMessage ofEvent(LogEvent event) { String authorName = event.getLoggerName(); String title = event.getLevel().name(); int colorDecimal = Objects.requireNonNull(LEVEL_TO_AMBIENT_COLOR.get(event.getLevel())); - String description = abbreviate(describeLogEvent(event), MAX_EMBED_DESCRIPTION); + String description = MessageUtils.abbreviate(describeLogEvent(event), MAX_EMBED_DESCRIPTION); Instant timestamp = Instant.ofEpochMilli(event.getInstant().getEpochMillisecond()); WebhookEmbed embed = new WebhookEmbedBuilder() @@ -150,18 +151,6 @@ private static String describeLogEvent(LogEvent event) { return logMessage + "\n" + exceptionWriter.toString().replace("\t", "> "); } - private static String abbreviate(String text, int maxLength) { - if (text.length() < maxLength) { - return text; - } - - if (maxLength < 3) { - return text.substring(0, maxLength); - } - - return text.substring(0, maxLength - 3) + "..."; - } - @Override public int compareTo(@NotNull LogMessage o) { return timestamp.compareTo(o.timestamp); diff --git a/application/src/test/java/org/togetherjava/tjbot/commands/utils/MessageUtilsTest.java b/application/src/test/java/org/togetherjava/tjbot/commands/utils/MessageUtilsTest.java index 79327494e4..c97a948387 100644 --- a/application/src/test/java/org/togetherjava/tjbot/commands/utils/MessageUtilsTest.java +++ b/application/src/test/java/org/togetherjava/tjbot/commands/utils/MessageUtilsTest.java @@ -10,11 +10,11 @@ final class MessageUtilsTest { @Test void escapeMarkdown() { - List tests = List.of(new TestCase("empty", "", ""), - new TestCase("no markdown", "hello world", "hello world"), - new TestCase( + List tests = List.of(new TestCaseEscape("empty", "", ""), + new TestCaseEscape("no markdown", "hello world", "hello world"), + new TestCaseEscape( "basic markdown", "\\*\\*hello\\*\\* \\_world\\_", "**hello** _world_"), - new TestCase("code block", """ + new TestCaseEscape("code block", """ \\`\\`\\`java int x = 5; \\`\\`\\` @@ -23,9 +23,9 @@ void escapeMarkdown() { int x = 5; ``` """), - new TestCase("escape simple", "hello\\\\\\\\world\\\\\\\\test", + new TestCaseEscape("escape simple", "hello\\\\\\\\world\\\\\\\\test", "hello\\\\world\\\\test"), - new TestCase("escape complex", """ + new TestCaseEscape("escape complex", """ Hello\\\\\\\\world \\`\\`\\`java Hello\\\\\\\\ @@ -46,7 +46,7 @@ void escapeMarkdown() { "Hello \\" World\\\\\\"" haha ``` """), - new TestCase("escape real example", + new TestCaseEscape("escape real example", """ Graph traversal can be accomplished easily using \\*\\*BFS\\*\\* or \\*\\*DFS\\*\\*. The algorithms only differ in the order in which nodes are visited: https://i.imgur.com/n9WrkQG.png @@ -158,12 +158,34 @@ void escapeMarkdown() { ``` """)); - for (TestCase test : tests) { + for (TestCaseEscape test : tests) { assertEquals(test.escapedMessage(), MessageUtils.escapeMarkdown(test.originalMessage()), "Test failed: " + test.testName()); } } - private record TestCase(String testName, String escapedMessage, String originalMessage) { + private record TestCaseEscape(String testName, String escapedMessage, String originalMessage) { + } + + @Test + void abbreviate() { + List tests = + List.of(new TestCaseAbbreviate("base case", "hello...", "hello world", 8), + new TestCaseAbbreviate("long enough", "hello world", "hello world", 15), + new TestCaseAbbreviate("exact size", "hello world", "hello world", 11), + new TestCaseAbbreviate("very small limit", "he", "hello world", 2), + new TestCaseAbbreviate("empty input", "", "", 0), + new TestCaseAbbreviate("zero limit", "", "hello world", 0), + new TestCaseAbbreviate("small limit", "h...", "hello world", 4)); + + for (TestCaseAbbreviate test : tests) { + assertEquals(test.abbreviatedMessage(), + MessageUtils.abbreviate(test.originalMessage(), test.limit()), + "Test failed: " + test.testName()); + } + } + + private record TestCaseAbbreviate(String testName, String abbreviatedMessage, + String originalMessage, int limit) { } } From ab7fb56d7fe06aa65001f51ba8e51e23e2603c65 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Fri, 7 Oct 2022 12:31:02 +0200 Subject: [PATCH 2/3] reduced size of top helper result * limit long user names * move description before table * reduce amount of helpers listed --- .../commands/tophelper/TopHelpersCommand.java | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/tophelper/TopHelpersCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/tophelper/TopHelpersCommand.java index c6e7c57cc6..27dc57ee6d 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/tophelper/TopHelpersCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/tophelper/TopHelpersCommand.java @@ -17,6 +17,7 @@ import org.togetherjava.tjbot.commands.CommandVisibility; import org.togetherjava.tjbot.commands.SlashCommandAdapter; +import org.togetherjava.tjbot.commands.utils.MessageUtils; import org.togetherjava.tjbot.db.Database; import javax.annotation.Nullable; @@ -42,7 +43,9 @@ public final class TopHelpersCommand extends SlashCommandAdapter { private static final Logger logger = LoggerFactory.getLogger(TopHelpersCommand.class); private static final String COMMAND_NAME = "top-helpers"; private static final String MONTH_OPTION = "at-month"; - private static final int TOP_HELPER_LIMIT = 20; + private static final int TOP_HELPER_LIMIT = 18; + + private static final int MAX_USER_NAME_LIMIT = 15; private final Database database; @@ -143,8 +146,11 @@ private static void handleTopHelpers(Collection topHelpers, userIdToMember.get(topHelper.authorId()))) .toList(); - String message = - "```java%n%s%n```".formatted(dataTableToString(topHelpersDataTable, timeRange)); + String message = """ + ```java + // for %s + %s + ```""".formatted(timeRange.description(), dataTableToString(topHelpersDataTable)); event.getHook().editOriginal(message).queue(); } @@ -152,20 +158,18 @@ private static void handleTopHelpers(Collection topHelpers, private static List topHelperToDataRow(TopHelperResult topHelper, @Nullable Member member) { String id = Long.toString(topHelper.authorId()); - String name = member == null ? "UNKNOWN_USER" : member.getEffectiveName(); + String name = MessageUtils.abbreviate( + member == null ? "UNKNOWN_USER" : member.getEffectiveName(), MAX_USER_NAME_LIMIT); String messageLengths = Long.toString(topHelper.messageLengths().longValue()); return List.of(id, name, messageLengths); } - private static String dataTableToString(Collection> dataTable, - TimeRange timeRange) { + private static String dataTableToString(Collection> dataTable) { return dataTableToAsciiTable(dataTable, List.of(new ColumnSetting("Id", HorizontalAlign.RIGHT), new ColumnSetting("Name", HorizontalAlign.RIGHT), - new ColumnSetting( - "Message lengths (for %s)".formatted(timeRange.description()), - HorizontalAlign.RIGHT))); + new ColumnSetting("Message lengths", HorizontalAlign.RIGHT))); } private static String dataTableToAsciiTable(Collection> dataTable, From 4944510687a83cde06c7d175291bfe1251f8ef8a Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Mon, 10 Oct 2022 09:19:14 +0200 Subject: [PATCH 3/3] spotless --- .../tjbot/logging/discord/DiscordLogForwarder.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/logging/discord/DiscordLogForwarder.java b/application/src/main/java/org/togetherjava/tjbot/logging/discord/DiscordLogForwarder.java index 0339ea84b7..14f3bd2f23 100644 --- a/application/src/main/java/org/togetherjava/tjbot/logging/discord/DiscordLogForwarder.java +++ b/application/src/main/java/org/togetherjava/tjbot/logging/discord/DiscordLogForwarder.java @@ -123,7 +123,8 @@ private static LogMessage ofEvent(LogEvent event) { String authorName = event.getLoggerName(); String title = event.getLevel().name(); int colorDecimal = Objects.requireNonNull(LEVEL_TO_AMBIENT_COLOR.get(event.getLevel())); - String description = MessageUtils.abbreviate(describeLogEvent(event), MAX_EMBED_DESCRIPTION); + String description = + MessageUtils.abbreviate(describeLogEvent(event), MAX_EMBED_DESCRIPTION); Instant timestamp = Instant.ofEpochMilli(event.getInstant().getEpochMillisecond()); WebhookEmbed embed = new WebhookEmbedBuilder()