Skip to content
Permalink
Browse files
427: Allow bridging PR updates to multiple recipients
Reviewed-by: ehelin
  • Loading branch information
rwestberg committed Jun 17, 2020
1 parent 9af4807 commit 3431a9e75f1eb4a70461ceb25d84f768d2cb6320
@@ -239,11 +239,10 @@ public Collection<WorkItem> run(Path scratchPath) {
var mbox = MailingListServerFactory.createMboxFileServer(mboxBasePath);
var reviewArchiveList = mbox.getList(pr.id());
var sentMails = parseArchive(reviewArchiveList);
var labels = new HashSet<>(pr.labels());

// First determine if this PR should be inspected further or not
if (sentMails.isEmpty()) {
var labels = new HashSet<>(pr.labels());

if (pr.state() == Issue.State.OPEN) {
for (var readyLabel : bot.readyLabels()) {
if (!labels.contains(readyLabel)) {
@@ -281,6 +280,25 @@ public Collection<WorkItem> run(Path scratchPath) {
}
}

// Determine recipient list(s)
var recipients = new ArrayList<EmailAddress>();
for (var candidateList : bot.lists()) {
if (candidateList.labels().isEmpty()) {
recipients.add(candidateList.list());
continue;
}
for (var label : labels) {
if (candidateList.labels().contains(label)) {
recipients.add(candidateList.list());
break;
}
}
}
if (recipients.isEmpty()) {
log.severe("PR does not match any recipient list: " + pr.repository().name() + "#" + pr.id());
return List.of();
}

var census = CensusInstance.create(bot.censusRepo(), bot.censusRef(), scratchPath.resolve("census"), pr);
var jbs = census.configuration().general().jbs();
if (jbs == null) {
@@ -297,7 +315,7 @@ 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(bot.listAddress().address());
var list = listServer.getList(recipients.get(0).toString());

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

@@ -354,6 +372,7 @@ public Collection<WorkItem> run(Path scratchPath) {
var filteredEmail = Email.from(newMail)
.replaceHeaders(filteredHeaders)
.headers(bot.headers())
.recipients(recipients)
.build();
list.post(filteredEmail);
}
@@ -40,7 +40,7 @@ public class MailingListBridgeBot implements Bot {
private final String archiveRef;
private final HostedRepository censusRepo;
private final String censusRef;
private final EmailAddress listAddress;
private final List<MailingListConfiguration> lists;
private final Set<String> ignoredUsers;
private final Set<Pattern> ignoredComments;
private final URI listArchive;
@@ -64,7 +64,7 @@ public class MailingListBridgeBot implements Bot {
private ZonedDateTime lastFullUpdate;

MailingListBridgeBot(EmailAddress from, HostedRepository repo, HostedRepository archive, String archiveRef,
HostedRepository censusRepo, String censusRef, EmailAddress list,
HostedRepository censusRepo, String censusRef, List<MailingListConfiguration> lists,
Set<String> ignoredUsers, Set<Pattern> ignoredComments, URI listArchive, String smtpServer,
HostedRepository webrevStorageRepository, String webrevStorageRef,
Path webrevStorageBase, URI webrevStorageBaseUri, Set<String> readyLabels,
@@ -77,7 +77,7 @@ public class MailingListBridgeBot implements Bot {
this.archiveRef = archiveRef;
this.censusRepo = censusRepo;
this.censusRef = censusRef;
listAddress = list;
this.lists = lists;
this.ignoredUsers = ignoredUsers;
this.ignoredComments = ignoredComments;
this.listArchive = listArchive;
@@ -126,8 +126,8 @@ EmailAddress emailAddress() {
return emailAddress;
}

EmailAddress listAddress() {
return listAddress;
List<MailingListConfiguration> lists() {
return lists;
}

Duration sendInterval() {
@@ -38,7 +38,7 @@ public class MailingListBridgeBotBuilder {
private String archiveRef = "master";
private HostedRepository censusRepo;
private String censusRef = "master";
private EmailAddress list;
private List<MailingListConfiguration> lists;
private Set<String> ignoredUsers = Set.of();
private Set<Pattern> ignoredComments = Set.of();
private URI listArchive;
@@ -90,8 +90,8 @@ public MailingListBridgeBotBuilder censusRef(String censusRef) {
return this;
}

public MailingListBridgeBotBuilder list(EmailAddress list) {
this.list = list;
public MailingListBridgeBotBuilder lists(List<MailingListConfiguration> lists) {
this.lists = lists;
return this;
}

@@ -181,7 +181,7 @@ public MailingListBridgeBotBuilder seedStorage(Path seedStorage) {
}

public MailingListBridgeBot build() {
return new MailingListBridgeBot(from, repo, archive, archiveRef, censusRepo, censusRef, list,
return new MailingListBridgeBot(from, repo, archive, archiveRef, censusRepo, censusRef, lists,
ignoredUsers, ignoredComments, listArchive, smtpServer,
webrevStorageRepository, webrevStorageRef, webrevStorageBase, webrevStorageBaseUri,
readyLabels, readyComments, issueTracker, headers, sendInterval, cooldown,
@@ -41,6 +41,27 @@ public String name() {
return "mlbridge";
}

private MailingListConfiguration parseList(JSONObject configuration) {
var listAddress = EmailAddress.parse(configuration.get("email").asString());
Set<String> labels = configuration.contains("labels") ?
configuration.get("labels").stream()
.map(JSONValue::asString)
.collect(Collectors.toSet()) :
Set.of();
return new MailingListConfiguration(listAddress, labels);
}

private List<MailingListConfiguration> parseLists(JSONValue configuration) {
if (configuration.isArray()) {
return configuration.stream()
.map(JSONValue::asObject)
.map(this::parseList)
.collect(Collectors.toList());
} else {
return List.of(parseList(configuration.asObject()));
}
}

@Override
public List<Bot> create(BotConfiguration configuration) {
var ret = new ArrayList<Bot>();
@@ -87,16 +108,15 @@ public List<Bot> create(BotConfiguration configuration) {
repoConfig.get("headers").fields().stream()
.collect(Collectors.toMap(JSONObject.Field::name, field -> field.value().asString())) :
Map.of();

var list = EmailAddress.parse(repoConfig.get("list").asString());
var lists = parseLists(repoConfig.get("lists"));
var folder = repoConfig.contains("folder") ? repoConfig.get("folder").asString() : configuration.repositoryName(repo);
var botBuilder = MailingListBridgeBot.newBuilder().from(from)
.repo(configuration.repository(repo))
.archive(archiveRepo)
.archiveRef(archiveRef)
.censusRepo(censusRepo)
.censusRef(censusRef)
.list(list)
.lists(lists)
.ignoredUsers(ignoredUsers)
.ignoredComments(ignoredComments)
.listArchive(listArchive)
@@ -122,7 +142,9 @@ public List<Bot> create(BotConfiguration configuration) {
ret.add(botBuilder.build());

if (!repoConfig.contains("bidirectional") || repoConfig.get("bidirectional").asBoolean()) {
listNamesForReading.add(list);
for (var list : lists) {
listNamesForReading.add(list.list());
}
}
allRepositories.add(configuration.repository(repo));
}
@@ -0,0 +1,45 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package org.openjdk.skara.bots.mlbridge;

import org.openjdk.skara.email.EmailAddress;

import java.util.*;

class MailingListConfiguration {
private EmailAddress list;
private Set<String> labels;

MailingListConfiguration(EmailAddress list, Set<String> labels) {
this.list = list;
this.labels = labels;
}

EmailAddress list() {
return list;
}

Set<String> labels() {
return labels;
}
}
@@ -38,20 +38,20 @@
import static org.junit.jupiter.api.Assertions.*;

class MailingListArchiveReaderBotTests {
private void addReply(Conversation conversation, MailingList mailingList, PullRequest pr, String reply) {
private void addReply(Conversation conversation, EmailAddress recipient, MailingList mailingList, 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)
.recipient(first.author())
.recipient(recipient)
.id(EmailAddress.from(UUID.randomUUID() + "@id.id"))
.header("In-Reply-To", first.id().toString())
.header("References", references)
.build();
mailingList.post(email);
}

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

@Test
@@ -72,7 +72,7 @@ void simpleArchive(TestInfo testInfo) throws IOException {
.repo(author)
.archive(archive)
.censusRepo(censusBuilder.build())
.list(listAddress)
.lists(List.of(new MailingListConfiguration(listAddress, Set.of())))
.ignoredUsers(Set.of(ignored.forge().currentUser().userName()))
.listArchive(listServer.getArchive())
.smtpServer(listServer.getSMTP())
@@ -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), mailmanList, pr);
addReply(conversations.get(0), listAddress, mailmanList, pr);
listServer.processIncoming();

// Another archive reader pass - has to be done twice
@@ -147,7 +147,7 @@ void rememberBridged(TestInfo testInfo) throws IOException {
.repo(author)
.archive(archive)
.censusRepo(censusBuilder.build())
.list(listAddress)
.lists(List.of(new MailingListConfiguration(listAddress, Set.of())))
.ignoredUsers(Set.of(ignored.forge().currentUser().userName()))
.listArchive(listServer.getArchive())
.smtpServer(listServer.getSMTP())
@@ -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), mailmanList, pr);
addReply(conversations.get(0), listAddress, mailmanList, pr);
listServer.processIncoming();

// Another archive reader pass - has to be done twice
@@ -224,7 +224,7 @@ void largeEmail(TestInfo testInfo) throws IOException {
.repo(author)
.archive(archive)
.censusRepo(censusBuilder.build())
.list(listAddress)
.lists(List.of(new MailingListConfiguration(listAddress, Set.of())))
.ignoredUsers(Set.of(ignored.forge().currentUser().userName()))
.listArchive(listServer.getArchive())
.smtpServer(listServer.getSMTP())
@@ -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), mailmanList, pr, replyBody);
addReply(conversations.get(0), listAddress, mailmanList, pr, replyBody);
listServer.processIncoming();

// Another archive reader pass - has to be done twice

1 comment on commit 3431a9e

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented on 3431a9e Jun 17, 2020

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.