Skip to content
Permalink
Browse files
843: Mails are not forwarded to a closed PR any more
Reviewed-by: ehelin
  • Loading branch information
rwestberg committed Apr 14, 2021
1 parent 0a79bfc commit f8abe35329c90b07c90348c5999c9c3bd6595f44
Showing with 485 additions and 344 deletions.
  1. +3 −3 bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveReaderWorkItem.java
  2. +9 −6 bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveWorkItem.java
  3. +6 −8 bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/MailingListArchiveReaderBot.java
  4. +14 −14 bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotFactory.java
  5. +14 −14 bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListArchiveReaderBotTests.java
  6. +8 −8 bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java
  7. +10 −10 bots/notify/src/main/java/org/openjdk/skara/bots/notify/mailinglist/MailingListNotifier.java
  8. +6 −6 bots/notify/src/main/java/org/openjdk/skara/bots/notify/mailinglist/MailingListNotifierBuilder.java
  9. +1 −1 bots/notify/src/main/java/org/openjdk/skara/bots/notify/mailinglist/MailingListNotifierFactory.java
  10. +32 −32 bots/notify/src/test/java/org/openjdk/skara/bots/notify/mailinglist/MailingListNotifierTests.java
  11. +27 −21 mailinglist/src/main/java/org/openjdk/skara/mailinglist/Conversation.java
  12. +2 −5 mailinglist/src/main/java/org/openjdk/skara/mailinglist/{MailingList.java → MailingListReader.java}
  13. +4 −1 mailinglist/src/main/java/org/openjdk/skara/mailinglist/MailingListServer.java
  14. +0 −1 mailinglist/src/main/java/org/openjdk/skara/mailinglist/MailingListServerFactory.java
  15. +3 −8 mailinglist/src/main/java/org/openjdk/skara/mailinglist/Mbox.java
  16. +41 −35 ...t/src/main/java/org/openjdk/skara/mailinglist/mailman/{MailmanList.java → MailmanListReader.java}
  17. +11 −6 mailinglist/src/main/java/org/openjdk/skara/mailinglist/mailman/MailmanServer.java
  18. +0 −100 mailinglist/src/main/java/org/openjdk/skara/mailinglist/mboxfile/MboxFileList.java
  19. +66 −0 mailinglist/src/main/java/org/openjdk/skara/mailinglist/mboxfile/MboxFileListReader.java
  20. +51 −6 mailinglist/src/main/java/org/openjdk/skara/mailinglist/mboxfile/MboxFileListServer.java
  21. +11 −12 mailinglist/src/test/java/org/openjdk/skara/mailinglist/MailmanTests.java
  22. +165 −46 mailinglist/src/test/java/org/openjdk/skara/mailinglist/MboxTests.java
  23. +1 −1 test/src/main/java/org/openjdk/skara/test/TestMailmanServer.java
@@ -23,17 +23,17 @@
package org.openjdk.skara.bots.mlbridge;

import org.openjdk.skara.bot.WorkItem;
import org.openjdk.skara.mailinglist.MailingList;
import org.openjdk.skara.mailinglist.MailingListReader;

import java.nio.file.Path;
import java.time.Duration;
import java.util.*;

public class ArchiveReaderWorkItem implements WorkItem {
private final MailingListArchiveReaderBot bot;
private final MailingList list;
private final MailingListReader list;

ArchiveReaderWorkItem(MailingListArchiveReaderBot bot, MailingList list) {
ArchiveReaderWorkItem(MailingListArchiveReaderBot bot, MailingListReader list) {
this.bot = bot;
this.list = list;
}
@@ -171,7 +171,7 @@ private void updateWebrevComment(List<Comment> comments, int index, List<WebrevD
}
}

private List<Email> parseArchive(MailingList archive) {
private List<Email> parseArchive(MailingListReader archive) {
var conversations = archive.conversations(Duration.ofDays(365));

if (conversations.size() == 0) {
@@ -256,7 +256,7 @@ public Collection<WorkItem> run(Path scratchPath) {
var archiveRepo = materializeArchive(path);
var mboxBasePath = path.resolve(bot.codeRepo().name());
var mbox = MailingListServerFactory.createMboxFileServer(mboxBasePath);
var reviewArchiveList = mbox.getList(pr.id());
var reviewArchiveList = mbox.getListReader(pr.id());
var sentMails = parseArchive(reviewArchiveList);
var labels = new HashSet<>(pr.labelNames());

@@ -334,8 +334,6 @@ public Collection<WorkItem> run(Path scratchPath) {

var webrevPath = scratchPath.resolve("mlbridge-webrevs");
var listServer = MailingListServerFactory.createMailmanServer(bot.listArchive(), bot.smtpServer(), bot.sendInterval());
var list = listServer.getList(recipients.get(0).toString());

var archiver = new ReviewArchive(pr, bot.emailAddress());

// Regular comments
@@ -382,7 +380,12 @@ public Collection<WorkItem> run(Path scratchPath) {
}

// Push all new mails to the archive repository
newMails.forEach(reviewArchiveList::post);
for (var newMail : newMails) {
var forArchiving = Email.from(newMail)
.recipient(EmailAddress.from(pr.id() + "@mbox"))
.build();
mbox.post(forArchiving);
}
pushMbox(archiveRepo, "Adding comments for PR " + bot.codeRepo().name() + "/" + pr.id());

// Finally post all new mails to the actual list
@@ -396,7 +399,7 @@ public Collection<WorkItem> run(Path scratchPath) {
.headers(bot.headers())
.recipients(recipients)
.build();
list.post(filteredEmail);
listServer.post(filteredEmail);
}
} catch (IOException e) {
throw new UncheckedIOException(e);
@@ -36,7 +36,7 @@

public class MailingListArchiveReaderBot implements Bot {
private final EmailAddress archivePoster;
private final Set<MailingList> lists;
private final MailingListReader list;
private final Set<HostedRepository> repositories;
private final Map<EmailAddress, String> parsedConversations = new HashMap<>();
private final Map<EmailAddress, PullRequest> resolvedPullRequests = new HashMap<>();
@@ -45,9 +45,9 @@ public class MailingListArchiveReaderBot implements Bot {
private final Pattern pullRequestLinkPattern = Pattern.compile("^(?:PR: |Pull request:\\R)(.*?)$", Pattern.MULTILINE);
private final Logger log = Logger.getLogger("org.openjdk.skara.bots.mlbridge");

MailingListArchiveReaderBot(EmailAddress archivePoster, Set<MailingList> lists, Set<HostedRepository> repositories) {
MailingListArchiveReaderBot(EmailAddress archivePoster, MailingListReader list, Set<HostedRepository> repositories) {
this.archivePoster = archivePoster;
this.lists = lists;
this.list = list;
this.repositories = repositories;
}

@@ -131,11 +131,9 @@ synchronized void inspect(Conversation conversation) {

@Override
public List<WorkItem> getPeriodicItems() {
var readerItems = lists.stream()
.map(list -> new ArchiveReaderWorkItem(this, list))
.collect(Collectors.toList());

var ret = new ArrayList<WorkItem>(readerItems);
var readerItems = new ArchiveReaderWorkItem(this, list);
var ret = new ArrayList<WorkItem>();
ret.add(readerItems);

// Check if there are any potential new comments to post
var item = commentQueue.poll();
@@ -88,7 +88,6 @@ public List<Bot> create(BotConfiguration configuration) {
var archiveRef = configuration.repositoryRef(specific.get("archive").asString());
var issueTracker = URIBuilder.base(specific.get("issues").asString()).build();

var listNamesForReading = new HashSet<EmailAddress>();
var allRepositories = new HashSet<HostedRepository>();

var readyLabels = specific.get("ready").get("labels").stream()
@@ -99,6 +98,7 @@ public List<Bot> create(BotConfiguration configuration) {
.collect(Collectors.toMap(obj -> obj.get("user").asString(),
obj -> Pattern.compile(obj.get("pattern").asString())));
var cooldown = specific.contains("cooldown") ? Duration.parse(specific.get("cooldown").asString()) : Duration.ofMinutes(1);
var mailmanServer = MailingListServerFactory.createMailmanServer(listArchive, listSmtp, Duration.ZERO);

for (var repoConfig : specific.get("repositories").asArray()) {
var repo = repoConfig.get("repository").asString();
@@ -109,7 +109,20 @@ public List<Bot> create(BotConfiguration configuration) {
repoConfig.get("headers").fields().stream()
.collect(Collectors.toMap(JSONObject.Field::name, field -> field.value().asString())) :
Map.of();

var lists = parseLists(repoConfig.get("lists"));
if (!repoConfig.contains("bidirectional") || repoConfig.get("bidirectional").asBoolean()) {
var listNamesForReading = new HashSet<EmailAddress>();
for (var list : lists) {
listNamesForReading.add(list.list());
}
var listsForReading = listNamesForReading.stream()
.map(EmailAddress::localPart)
.collect(Collectors.toList());
var bot = new MailingListArchiveReaderBot(from, mailmanServer.getListReader(listsForReading.toArray(new String[0])), allRepositories);
ret.add(bot);
}

var folder = repoConfig.contains("folder") ? repoConfig.get("folder").asString() : configuration.repositoryName(repo);

var webrevGenerateHTML = true;
@@ -156,22 +169,9 @@ public List<Bot> create(BotConfiguration configuration) {
}
ret.add(botBuilder.build());

if (!repoConfig.contains("bidirectional") || repoConfig.get("bidirectional").asBoolean()) {
for (var list : lists) {
listNamesForReading.add(list.list());
}
}
allRepositories.add(configuration.repository(repo));
}

var mailmanServer = MailingListServerFactory.createMailmanServer(listArchive, listSmtp, Duration.ZERO);
var listsForReading = listNamesForReading.stream()
.map(name -> mailmanServer.getList(name.toString()))
.collect(Collectors.toSet());

var bot = new MailingListArchiveReaderBot(from, listsForReading, allRepositories);
ret.add(bot);

return ret;
}
}
@@ -38,7 +38,7 @@
import static org.junit.jupiter.api.Assertions.*;

class MailingListArchiveReaderBotTests {
private void addReply(Conversation conversation, EmailAddress recipient, MailingList mailingList, PullRequest pr, String reply) {
private void addReply(Conversation conversation, EmailAddress recipient, MailingListServer mailingListServer, PullRequest pr, String reply) {
var first = conversation.first();
var references = first.id().toString();
var email = Email.create(EmailAddress.from("Commenter", "c@test.test"), "Re: RFR: " + pr.title(), reply)
@@ -47,11 +47,11 @@ private void addReply(Conversation conversation, EmailAddress recipient, Mailing
.header("In-Reply-To", first.id().toString())
.header("References", references)
.build();
mailingList.post(email);
mailingListServer.post(email);
}

private void addReply(Conversation conversation, EmailAddress recipient, MailingList mailingList, PullRequest pr) {
addReply(conversation, recipient, mailingList, pr, "Looks good");
private void addReply(Conversation conversation, EmailAddress recipient, MailingListServer mailingListServer, PullRequest pr) {
addReply(conversation, recipient, mailingListServer, pr, "Looks good");
}

@Test
@@ -86,8 +86,8 @@ void simpleArchive(TestInfo testInfo) throws IOException {
// The mailing list as well
var mailmanServer = MailingListServerFactory.createMailmanServer(listServer.getArchive(), listServer.getSMTP(),
Duration.ZERO);
var mailmanList = mailmanServer.getList(listAddress.address());
var readerBot = new MailingListArchiveReaderBot(from, Set.of(mailmanList), Set.of(archive));
var mailmanList = mailmanServer.getListReader(listAddress.address());
var readerBot = new MailingListArchiveReaderBot(from, mailmanList, Set.of(archive));

// Populate the projects repository
var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType());
@@ -113,7 +113,7 @@ void simpleArchive(TestInfo testInfo) throws IOException {
// Post a reply directly to the list
var conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(1, conversations.size());
addReply(conversations.get(0), listAddress, mailmanList, pr);
addReply(conversations.get(0), listAddress, mailmanServer, pr);
listServer.processIncoming();

// Another archive reader pass - has to be done twice
@@ -162,8 +162,8 @@ void rememberBridged(TestInfo testInfo) throws IOException {
// The mailing list as well
var mailmanServer = MailingListServerFactory.createMailmanServer(listServer.getArchive(), listServer.getSMTP(),
Duration.ZERO);
var mailmanList = mailmanServer.getList(listAddress.address());
var readerBot = new MailingListArchiveReaderBot(from, Set.of(mailmanList), Set.of(archive));
var mailmanList = mailmanServer.getListReader(listAddress.address());
var readerBot = new MailingListArchiveReaderBot(from, mailmanList, Set.of(archive));

// Populate the projects repository
var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType());
@@ -185,7 +185,7 @@ void rememberBridged(TestInfo testInfo) throws IOException {
// Post a reply directly to the list
var conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(1, conversations.size());
addReply(conversations.get(0), listAddress, mailmanList, pr);
addReply(conversations.get(0), listAddress, mailmanServer, pr);
listServer.processIncoming();

// Another archive reader pass - has to be done twice
@@ -196,7 +196,7 @@ void rememberBridged(TestInfo testInfo) throws IOException {
var updated = pr.comments();
assertEquals(2, updated.size());

var newReaderBot = new MailingListArchiveReaderBot(from, Set.of(mailmanList), Set.of(archive));
var newReaderBot = new MailingListArchiveReaderBot(from, mailmanList, Set.of(archive));
TestBotRunner.runPeriodicItems(newReaderBot);
TestBotRunner.runPeriodicItems(newReaderBot);

@@ -238,8 +238,8 @@ void largeEmail(TestInfo testInfo) throws IOException {
// The mailing list as well
var mailmanServer = MailingListServerFactory.createMailmanServer(listServer.getArchive(), listServer.getSMTP(),
Duration.ZERO);
var mailmanList = mailmanServer.getList(listAddress.address());
var readerBot = new MailingListArchiveReaderBot(from, Set.of(mailmanList), Set.of(archive));
var mailmanList = mailmanServer.getListReader(listAddress.address());
var readerBot = new MailingListArchiveReaderBot(from, mailmanList, Set.of(archive));

// Populate the projects repository
var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType());
@@ -267,7 +267,7 @@ void largeEmail(TestInfo testInfo) throws IOException {
assertEquals(1, conversations.size());

var replyBody = "This line is about 30 bytes long\n".repeat(1000 * 10);
addReply(conversations.get(0), listAddress, mailmanList, pr, replyBody);
addReply(conversations.get(0), listAddress, mailmanServer, pr, replyBody);
listServer.processIncoming();

// Another archive reader pass - has to be done twice
@@ -214,7 +214,7 @@ void simpleArchive(TestInfo testInfo) throws IOException {
// The mailing list as well
listServer.processIncoming();
var mailmanServer = MailingListServerFactory.createMailmanServer(listServer.getArchive(), listServer.getSMTP(), Duration.ZERO);
var mailmanList = mailmanServer.getList(listAddress.address());
var mailmanList = mailmanServer.getListReader(listAddress.address());
var conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(1, conversations.size());
var mail = conversations.get(0).first();
@@ -796,7 +796,7 @@ void reviewComment(TestInfo testInfo) throws IOException {

// The mailing list as well
var mailmanServer = MailingListServerFactory.createMailmanServer(listServer.getArchive(), listServer.getSMTP(), Duration.ZERO);
var mailmanList = mailmanServer.getList(listAddress.address());
var mailmanList = mailmanServer.getListReader(listAddress.address());
var conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(1, conversations.size());
var mail = conversations.get(0).first();
@@ -884,7 +884,7 @@ void combineComments(TestInfo testInfo) throws IOException {

// As well as the mailing list
var mailmanServer = MailingListServerFactory.createMailmanServer(listServer.getArchive(), listServer.getSMTP(), Duration.ZERO);
var mailmanList = mailmanServer.getList(listAddress.address());
var mailmanList = mailmanServer.getListReader(listAddress.address());
var conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(1, conversations.size());
var mail = conversations.get(0).first();
@@ -1001,7 +1001,7 @@ void commentThreading(TestInfo testInfo) throws IOException {

// Check the mailing list
var mailmanServer = MailingListServerFactory.createMailmanServer(listServer.getArchive(), listServer.getSMTP(), Duration.ZERO);
var mailmanList = mailmanServer.getList(listAddress.address());
var mailmanList = mailmanServer.getListReader(listAddress.address());
var conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(1, conversations.size());
var mail = conversations.get(0).first();
@@ -1607,7 +1607,7 @@ void incrementalChanges(TestInfo testInfo) throws IOException {

// Check that sender address is set properly
var mailmanServer = MailingListServerFactory.createMailmanServer(listServer.getArchive(), listServer.getSMTP(), Duration.ZERO);
var mailmanList = mailmanServer.getList(listAddress.address());
var mailmanList = mailmanServer.getListReader(listAddress.address());
var conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(1, conversations.size());
for (var newMail : conversations.get(0).allMessages()) {
@@ -1734,7 +1734,7 @@ void rebased(TestInfo testInfo) throws IOException {

// Check that sender address is set properly
var mailmanServer = MailingListServerFactory.createMailmanServer(listServer.getArchive(), listServer.getSMTP(), Duration.ZERO);
var mailmanList = mailmanServer.getList(listAddress.address());
var mailmanList = mailmanServer.getListReader(listAddress.address());
var conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(1, conversations.size());
for (var newMail : conversations.get(0).allMessages()) {
@@ -2899,7 +2899,7 @@ void multipleRecipients(TestInfo testInfo) throws IOException {

// The mail should have been sent to list1
var mailmanServer = MailingListServerFactory.createMailmanServer(listServer.getArchive(), listServer.getSMTP(), Duration.ZERO);
var mailmanList = mailmanServer.getList(listAddress1.address());
var mailmanList = mailmanServer.getListReader(listAddress1.address());
var conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(1, conversations.size());
var mail = conversations.get(0).first();
@@ -3031,7 +3031,7 @@ void jsonArchive(TestInfo testInfo) throws IOException {
// The mailing list as well
listServer.processIncoming();
var mailmanServer = MailingListServerFactory.createMailmanServer(listServer.getArchive(), listServer.getSMTP(), Duration.ZERO);
var mailmanList = mailmanServer.getList(listAddress.address());
var mailmanList = mailmanServer.getListReader(listAddress.address());
var conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(1, conversations.size());
var mail = conversations.get(0).first();

1 comment on commit f8abe35

@openjdk-notifier
Copy link

@openjdk-notifier openjdk-notifier bot commented on f8abe35 Apr 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.