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

280: Notifier should support repo/branch prefixes #442

Closed
wants to merge 1 commit into from
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
@@ -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();