Skip to content
Permalink
Browse files
280: Notifier should support repo/branch prefixes
Reviewed-by: ehelin
  • Loading branch information
rwestberg committed Feb 17, 2020
1 parent 38bc15e commit e4b8ae3d065d8a967f56515d290597c82b37d492
Showing 4 changed files with 142 additions and 7 deletions.
@@ -24,11 +24,12 @@

import org.openjdk.skara.email.*;
import org.openjdk.skara.forge.HostedRepository;
import org.openjdk.skara.mailinglist.*;
import org.openjdk.skara.mailinglist.MailingList;
import org.openjdk.skara.vcs.*;
import org.openjdk.skara.vcs.openjdk.OpenJDKTag;

import java.io.*;
import java.nio.file.Path;
import java.time.Duration;
import java.time.format.DateTimeFormatter;
import java.util.*;
@@ -48,6 +49,8 @@ public class MailingListUpdater implements RepositoryUpdateConsumer {
private final Mode mode;
private final Map<String, String> headers;
private final Pattern allowedAuthorDomains;
private final boolean repoInSubject;
private final Pattern branchInSubject;
private final Logger log = Logger.getLogger("org.openjdk.skara.bots.notify");

enum Mode {
@@ -58,7 +61,8 @@ enum Mode {

MailingListUpdater(MailingList list, EmailAddress recipient, EmailAddress sender, EmailAddress author,
boolean includeBranch, boolean reportNewTags, boolean reportNewBranches, boolean reportNewBuilds,
Mode mode, Map<String, String> headers, Pattern allowedAuthorDomains) {
Mode mode, Map<String, String> headers, Pattern allowedAuthorDomains, boolean repoInSubject,
Pattern branchInSubject) {
this.list = list;
this.recipient = recipient;
this.sender = sender;
@@ -70,6 +74,8 @@ enum Mode {
this.mode = mode;
this.headers = headers;
this.allowedAuthorDomains = allowedAuthorDomains;
this.repoInSubject = repoInSubject;
this.branchInSubject = branchInSubject;
}

static MailingListUpdaterBuilder newBuilder() {
@@ -137,7 +143,29 @@ private String tagToSubject(HostedRepository repository, Hash hash, Tag tag) {
hash.abbreviate();
}

private List<Commit> filterAndSendPrCommits(HostedRepository repository, List<Commit> commits) {
private String subjectPrefix(HostedRepository repository, Branch branch) {
var ret = new StringBuilder();
var branchName = branch.name();
var repoName = Path.of(repository.name()).getFileName().toString();
var useBranchInSubject = branchInSubject.matcher(branchName).matches();

if (useBranchInSubject || repoInSubject) {
ret.append("[");
if (repoInSubject) {
ret.append(repoName);
if (useBranchInSubject) {
ret.append(":");
}
}
if (useBranchInSubject) {
ret.append(branchName);
}
ret.append("] ");
}
return ret.toString();
}

private List<Commit> filterAndSendPrCommits(HostedRepository repository, List<Commit> commits, Branch branch) {
var ret = new ArrayList<Commit>();

var rfrsConvos = list.conversations(Duration.ofDays(365)).stream()
@@ -175,7 +203,8 @@ private List<Commit> filterAndSendPrCommits(HostedRepository repository, List<Co
}

var body = CommitFormatters.toText(repository, commit);
var email = Email.reply(rfrConv.first(), "Re: [Integrated] " + rfrConv.first().subject(), body)
var email = Email.reply(rfrConv.first(), "Re: " + subjectPrefix(repository, branch) +
"[Integrated] " + rfrConv.first().subject(), body)
.sender(sender)
.author(commitToAuthor(commit))
.recipient(recipient)
@@ -216,10 +245,10 @@ private void sendCombinedCommits(HostedRepository repository, List<Commit> commi
public void handleCommits(HostedRepository repository, Repository localRepository, List<Commit> commits, Branch branch) {
switch (mode) {
case PR_ONLY:
filterAndSendPrCommits(repository, commits);
filterAndSendPrCommits(repository, commits, branch);
break;
case PR:
commits = filterAndSendPrCommits(repository, commits);
commits = filterAndSendPrCommits(repository, commits, branch);
// fall-through
case ALL:
sendCombinedCommits(repository, commits, branch);
@@ -40,6 +40,8 @@ public class MailingListUpdaterBuilder {
private MailingListUpdater.Mode mode = MailingListUpdater.Mode.ALL;
private Map<String, String> headers = Map.of();
private Pattern allowedAuthorDomains = Pattern.compile(".*");
private boolean repoInSubject = false;
private Pattern branchInSubject = Pattern.compile("a^"); // Does not match anything

public MailingListUpdaterBuilder list(MailingList list) {
this.list = list;
@@ -96,8 +98,18 @@ public MailingListUpdaterBuilder allowedAuthorDomains(Pattern allowedAuthorDomai
return this;
}

public MailingListUpdaterBuilder repoInSubject(boolean repoInSubject) {
this.repoInSubject = repoInSubject;
return this;
}

public MailingListUpdaterBuilder branchInSubject(Pattern branchInSubject) {
this.branchInSubject = branchInSubject;
return this;
}

public MailingListUpdater build() {
return new MailingListUpdater(list, recipient, sender, author, includeBranch, reportNewTags, reportNewBranches,
reportNewBuilds, mode, headers, allowedAuthorDomains);
reportNewBuilds, mode, headers, allowedAuthorDomains, repoInSubject, branchInSubject);
}
}
@@ -145,6 +145,13 @@ public List<Bot> create(BotConfiguration configuration) {
if (mailinglist.contains("builds")) {
mailingListUpdaterBuilder.reportNewBuilds(mailinglist.get("builds").asBoolean());
}
if (mailinglist.contains("reponame")) {
mailingListUpdaterBuilder.repoInSubject(mailinglist.get("reponame").asBoolean());
}
if (mailinglist.contains("branchname")) {
mailingListUpdaterBuilder.branchInSubject(Pattern.compile(mailinglist.get("branchname").asString()));
}

updaters.add(mailingListUpdaterBuilder.build());
}
}
@@ -1085,6 +1085,93 @@ void testMailingListBranch(TestInfo testInfo) throws IOException {
}
}

@Test
void testMailingListBranchPrefix(TestInfo testInfo) throws IOException {
try (var listServer = new TestMailmanServer();
var credentials = new HostCredentials(testInfo);
var tempFolder = new TemporaryDirectory()) {
var repo = credentials.getHostedRepository();
var repoFolder = tempFolder.path().resolve("repo");
var localRepo = CheckableRepository.init(repoFolder, repo.repositoryType());
var masterHash = localRepo.resolve("master").orElseThrow();
credentials.commitLock(localRepo);
localRepo.pushAll(repo.url());

var listAddress = EmailAddress.parse(listServer.createList("test"));
var mailmanServer = MailingListServerFactory.createMailmanServer(listServer.getArchive(), listServer.getSMTP(), Duration.ZERO);
var mailmanList = mailmanServer.getList(listAddress.address());
var tagStorage = createTagStorage(repo);
var branchStorage = createBranchStorage(repo);
var prIssuesStorage = createPullRequestIssuesStorage(repo);
var storageFolder = tempFolder.path().resolve("storage");

var sender = EmailAddress.from("duke", "duke@duke.duke");
var updater = MailingListUpdater.newBuilder()
.list(mailmanList)
.recipient(listAddress)
.sender(sender)
.reportNewTags(false)
.reportNewBranches(false)
.reportNewBuilds(false)
.mode(MailingListUpdater.Mode.PR)
.repoInSubject(true)
.branchInSubject(Pattern.compile(".*"))
.build();
var notifyBot = NotifyBot.newBuilder()
.repository(repo)
.storagePath(storageFolder)
.branches(Pattern.compile("master"))
.tagStorageBuilder(tagStorage)
.branchStorageBuilder(branchStorage)
.prIssuesStorageBuilder(prIssuesStorage)
.updaters(List.of(updater))
.build();

// No mail should be sent on the first run as there is no history
TestBotRunner.runPeriodicItems(notifyBot);
assertThrows(RuntimeException.class, () -> listServer.processIncoming(Duration.ofMillis(1)));

var editHash = CheckableRepository.appendAndCommit(localRepo, "Another line", "23456789: More fixes");
localRepo.push(editHash, repo.url(), "edit");
var pr = credentials.createPullRequest(repo, "master", "edit", "RFR: My PR");

// PR hasn't been integrated yet, so there should be no mail
TestBotRunner.runPeriodicItems(notifyBot);
assertThrows(RuntimeException.class, () -> listServer.processIncoming(Duration.ofMillis(1)));

// Simulate an RFR email
var rfr = Email.create("RFR: My PR", "PR:\n" + pr.webUrl().toString())
.author(EmailAddress.from("duke", "duke@duke.duke"))
.recipient(listAddress)
.build();
mailmanList.post(rfr);
listServer.processIncoming();

// And an integration
pr.addComment("Pushed as commit " + editHash.hex() + ".");
localRepo.push(editHash, repo.url(), "master");

TestBotRunner.runPeriodicItems(notifyBot);
listServer.processIncoming();

var conversations = mailmanList.conversations(Duration.ofDays(1));
conversations.sort(Comparator.comparing(conversation -> conversation.first().subject()));
assertEquals(1, conversations.size());

var prConversation = conversations.get(0);

var prEmail = prConversation.replies(prConversation.first()).get(0);
assertEquals(listAddress, prEmail.sender());
assertEquals(EmailAddress.from("testauthor", "ta@none.none"), prEmail.author());
assertEquals(prEmail.recipients(), List.of(listAddress));
assertEquals("[" + repo.name() + ":master] [Integrated] RFR: My PR", prEmail.subject());
assertTrue(prEmail.body().contains("Changeset: " + editHash.abbreviate()));
assertTrue(prEmail.body().contains("23456789: More fixes"));
assertFalse(prEmail.body().contains("Committer"));
assertFalse(prEmail.body().contains(masterHash.abbreviate()));
}
}

@Test
void testMailingListNoIdempotence(TestInfo testInfo) throws IOException {
try (var listServer = new TestMailmanServer();

0 comments on commit e4b8ae3

Please sign in to comment.