439: Improve command filtering when bridging mails

Reviewed-by: ehelin
openjdk · Aug 31, 2020 · b007595 · b007595 · bridgekeeper · Aug 31, 2020
1 parent e36b503
commit b007595
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 9 deletions.
diff --git a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveMessages.java b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveMessages.java
@@ -18,14 +18,18 @@
 class ArchiveMessages {
     private static final Pattern commentPattern = Pattern.compile("<!--.*?-->",
                                                                   Pattern.DOTALL | Pattern.MULTILINE);
+    private static final Pattern commandPattern = Pattern.compile("^\\s*/([A-Za-z]+).*$", Pattern.MULTILINE);
 
-    private static String filterComments(String body) {
+    private static String filterCommentsAndCommands(String body) {
         var parsedBody = PullRequestBody.parse(body);
         body = parsedBody.bodyText();
 
         var commentMatcher = commentPattern.matcher(body);
         body = commentMatcher.replaceAll("");
 
+        var commandLineMatcher = commandPattern.matcher(body);
+        body = commandLineMatcher.replaceAll("");
+
         body = MarkdownToText.removeFormatting(body);
         return body.strip();
     }
@@ -166,7 +170,7 @@ private static String fetchCommand(PullRequest pr) {
     }
 
     static String composeConversation(PullRequest pr) {
-        var filteredBody = filterComments(pr.body());
+        var filteredBody = filterCommentsAndCommands(pr.body());
         if (filteredBody.isEmpty()) {
             filteredBody = pr.title().strip();
         }
@@ -323,7 +327,7 @@ static String composeIncrementalFooter(PullRequest pr, Repository localRepo, Web
     }
 
     static String composeComment(Comment comment) {
-        return filterComments(comment.body());
+        return filterCommentsAndCommands(comment.body());
     }
 
     static String composeReviewComment(PullRequest pr, ReviewComment reviewComment) {
@@ -342,7 +346,7 @@ static String composeReviewComment(PullRequest pr, ReviewComment reviewComment)
                 body.append("> (failed to retrieve contents of file, check the PR for context)\n");
             }
         }
-        body.append(filterComments(reviewComment.body()));
+        body.append(filterCommentsAndCommands(reviewComment.body()));
         return body.toString();
     }
 
@@ -365,7 +369,7 @@ private static String composeReviewVerdict(Review review, HostUserToUserName hos
 
     static String composeReview(PullRequest pr, Review review, HostUserToUserName hostUserToUserName, HostUserToRole hostUserToRole) {
         if (review.body().isPresent() && !review.body().get().isBlank()) {
-            return filterComments(review.body().get());
+            return filterCommentsAndCommands(review.body().get());
         } else {
             return composeReviewVerdict(review, hostUserToUserName, hostUserToRole);
         }

diff --git a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveWorkItem.java b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveWorkItem.java
@@ -36,7 +36,7 @@
 import java.util.*;
 import java.util.function.*;
 import java.util.logging.Logger;
-import java.util.regex.Pattern;
+import java.util.regex.*;
 import java.util.stream.Collectors;
 
 class ArchiveWorkItem implements WorkItem {
@@ -104,7 +104,7 @@ private Repository materializeArchive(Path scratchPath) {
         }
     }
 
-    private final static Pattern commandPattern = Pattern.compile("^/.*$");
+    private final static Pattern commandPattern = Pattern.compile("^\\s*/([A-Za-z]+).*$");
 
     private boolean ignoreComment(HostUser author, String body) {
         if (pr.repository().forge().currentUser().equals(author)) {
@@ -113,8 +113,11 @@ private boolean ignoreComment(HostUser author, String body) {
         if (bot.ignoredUsers().contains(author.userName())) {
             return true;
         }
-        var commandMatcher = commandPattern.matcher(body);
-        if (commandMatcher.matches()) {
+        // Check if this comment only contains command lines
+        var commandOnly = body.strip().lines()
+                              .map(commandPattern::matcher)
+                              .allMatch(Matcher::matches);
+        if (body.strip().lines().count() > 0 && commandOnly) {
             return true;
         }
         for (var ignoredCommentPattern : bot.ignoredComments()) {

diff --git a/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java b/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java
@@ -1466,7 +1466,10 @@ void filterComments(TestInfo testInfo) throws IOException {
             // Make a bunch of comments
             pr.addComment("Plain comment\n<!-- this is a comment -->");
             pr.addReviewComment(masterHash, editHash, reviewFile.toString(), 2, "Review comment <!-- this is a comment -->\n");
+            pr.addComment("  /cc others");
             pr.addComment("/integrate stuff");
+            pr.addComment("the command is /hello there");
+            pr.addComment("but this will be parsed\n/newline command");
             TestBotRunner.runPeriodicItems(mlBot);
 
             // Run an archive pass
@@ -1483,6 +1486,10 @@ void filterComments(TestInfo testInfo) throws IOException {
             assertTrue(archiveContains(archiveFolder.path(), "Plain comment"));
             assertTrue(archiveContains(archiveFolder.path(), "Review comment"));
             assertFalse(archiveContains(archiveFolder.path(), "/integrate"));
+            assertFalse(archiveContains(archiveFolder.path(), "/cc"));
+            assertTrue(archiveContains(archiveFolder.path(), "/hello there"));
+            assertTrue(archiveContains(archiveFolder.path(), "but this will be parsed"));
+            assertFalse(archiveContains(archiveFolder.path(), "/newline"));
         }
     }