Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

427: Allow bridging PR updates to multiple recipients #667

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -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