963: Skara mlbridge bot sometimes resends old messages

Reviewed-by: ehelin
openjdk · Apr 14, 2021 · e3b75ff · e3b75ff · openjdk-notifier · Apr 14, 2021
1 parent f8abe35
commit e3b75ff
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 17 deletions.
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
@@ -31,7 +31,8 @@
 import org.openjdk.skara.vcs.*;
 
 import java.io.*;
-import java.nio.file.Path;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.*;
 import java.time.*;
 import java.util.*;
 import java.util.function.*;
@@ -171,19 +172,6 @@ private void updateWebrevComment(List<Comment> comments, int index, List<WebrevD
         }
     }
 
-    private List<Email> parseArchive(MailingListReader archive) {
-        var conversations = archive.conversations(Duration.ofDays(365));
-
-        if (conversations.size() == 0) {
-            return new ArrayList<>();
-        } else if (conversations.size() == 1) {
-            var conversation = conversations.get(0);
-            return conversation.allMessages();
-        } else {
-            throw new RuntimeException("Something is wrong with the mbox");
-        }
-    }
-
     private EmailAddress getAuthorAddress(CensusInstance censusInstance, HostUser originalAuthor) {
         if (bot.ignoredUsers().contains(originalAuthor.username())) {
             return bot.emailAddress();
@@ -255,9 +243,14 @@ public Collection<WorkItem> run(Path scratchPath) {
         var path = scratchPath.resolve("mlbridge");
         var archiveRepo = materializeArchive(path);
         var mboxBasePath = path.resolve(bot.codeRepo().name());
-        var mbox = MailingListServerFactory.createMboxFileServer(mboxBasePath);
-        var reviewArchiveList = mbox.getListReader(pr.id());
-        var sentMails = parseArchive(reviewArchiveList);
+
+        var sentMails = new ArrayList<Email>();
+        try {
+            var archiveContents = Files.readString(mboxBasePath.resolve(pr.id() + ".mbox"), StandardCharsets.UTF_8);
+            sentMails.addAll(Mbox.splitMbox(archiveContents, bot.emailAddress()));
+        } catch (IOException ignored) {
+        }
+
         var labels = new HashSet<>(pr.labelNames());
 
         // First determine if this PR should be inspected further or not
@@ -380,6 +373,7 @@ public Collection<WorkItem> run(Path scratchPath) {
             }
 
             // Push all new mails to the archive repository
+            var mbox = MailingListServerFactory.createMboxFileServer(mboxBasePath);
             for (var newMail : newMails) {
                 var forArchiving = Email.from(newMail)
                                         .recipient(EmailAddress.from(pr.id() + "@mbox"))

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
@@ -3348,6 +3348,12 @@ void commentWithQuoteFromBridged(TestInfo testInfo) throws IOException {
             pr.addComment(">First comm\n\nreply to first");
             TestBotRunner.runPeriodicItems(mlBot);
             listServer.processIncoming();
+            listServer.processIncoming();
+
+            // Ensure that the PR is considered again - no duplicates should be sent
+            pr.addLabel("ping");
+            TestBotRunner.runPeriodicItems(mlBot);
+            assertThrows(RuntimeException.class, () -> listServer.processIncoming(Duration.ofMillis(1)));
 
             // The first comment should be replied to once, and the original post once
             Repository.materialize(archiveFolder.path(), archive.url(), "master");