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.

Sign up for GitHub

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

Jump to bottom

Use tag annotation if present when sending notifications #243

Closed
wants to merge 4 commits into from
Closed

Use tag annotation if present when sending notifications #243

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
649928d
Send notifications for non-OpenJDK-compliant tags as well, use tag an…
Nov 7, 2019
1edc0fd
Fix failing test
Nov 7, 2019
fe09eed
Adjust email formatting
Nov 8, 2019
e02eb57
Adjust after review
Nov 8, 2019
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
45 changes: 30 additions & 15 deletions bots/notify/src/main/java/org/openjdk/skara/bots/notify/JNotifyBot.java
View file
Open in desktop
Expand Up @@ -35,7 +35,7 @@
import java.util.*;
import java.util.logging.Logger;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.*;

class JNotifyBot implements Bot, WorkItem {
private final Logger log = Logger.getLogger("org.openjdk.skara.bots");;
Expand Down Expand Up @@ -166,33 +166,48 @@ private void handleTags(Repository localRepo, UpdateHistory history) throws IOEx
.map(Optional::get)
.collect(Collectors.toSet());
var newJdkTags = newTags.stream()
.map(OpenJDKTag::create)
.filter(Optional::isPresent)
.map(Optional::get)
.sorted(Comparator.comparingInt(OpenJDKTag::buildNum))
.collect(Collectors.toList());
.map(OpenJDKTag::create)
.filter(Optional::isPresent)
.map(Optional::get)
.sorted(Comparator.comparingInt(OpenJDKTag::buildNum))
.map(OpenJDKTag::tag);
var newNonJdkTags = newTags.stream()
.filter(tag -> OpenJDKTag.create(tag).isEmpty());

for (var tag : newJdkTags) {
var sortedNewTags = Stream.concat(newJdkTags, newNonJdkTags).collect(Collectors.toList());
for (var tag : sortedNewTags) {
// Update the history first - if there is a problem here we don't want to send out multiple updates
history.addTags(List.of(tag.tag()));
history.addTags(List.of(tag));

var commits = new ArrayList<Commit>();
var previous = existingPrevious(tag, allJdkTags);
if (previous.isEmpty()) {
var commit = localRepo.lookup(tag.tag());

// Try to determine which commits are new since the last build
var openjdkTag = OpenJDKTag.create(tag);
if (openjdkTag.isPresent()) {
var previous = existingPrevious(openjdkTag.get(), allJdkTags);
if (previous.isPresent()) {
commits.addAll(localRepo.commits(previous.get().tag() + ".." + tag).asList());
}
}

// If none are found, just include the commit that was tagged
if (commits.isEmpty()) {
var commit = localRepo.lookup(tag);
if (commit.isEmpty()) {
throw new RuntimeException("Failed to lookup tag '" + tag.toString() + "'");
} else {
commits.add(commit.get());
log.warning("No previous tag found for '" + tag.tag() + "'");
}
} else {
commits.addAll(localRepo.commits(previous.get().tag() + ".." + tag.tag()).asList());
}

Collections.reverse(commits);
var annotation = localRepo.annotate(tag);
for (var updater : updaters) {
updater.handleTagCommits(repository, commits, tag);
if (annotation.isPresent()) {
updater.handleAnnotatedTagCommits(repository, commits, tag, annotation.get());
} else {
updater.handleTagCommits(repository, commits, tag);
}
}
}
}
Expand Down
13 changes: 11 additions & 2 deletions bots/notify/src/main/java/org/openjdk/skara/bots/notify/JsonUpdater.java
View file
Open in desktop
Expand Up @@ -86,8 +86,12 @@ public void handleCommits(HostedRepository repository, List<Commit> commits, Bra
}

@Override
public void handleTagCommits(HostedRepository repository, List<Commit> commits, OpenJDKTag tag) {
var build = String.format("b%02d", tag.buildNum());
public void handleTagCommits(HostedRepository repository, List<Commit> commits, Tag tag) {
var openjdkTag = OpenJDKTag.create(tag);
if (openjdkTag.isEmpty()) {
return;
}
var build = String.format("b%02d", openjdkTag.get().buildNum());
try (var writer = new JsonUpdateWriter(path, repository.name())) {
var issues = new ArrayList<Issue>();
for (var commit : commits) {
Expand All @@ -99,6 +103,11 @@ public void handleTagCommits(HostedRepository repository, List<Commit> commits,
}
}

@Override
public void handleAnnotatedTagCommits(HostedRepository repository, List<Commit> commits, Tag tag, Tag.Annotated annotation) {
handleTagCommits(repository, commits, tag);
}

@Override
public void handleNewBranch(HostedRepository repository, List<Commit> commits, Branch parent, Branch branch) {

Expand Down
88 changes: 64 additions & 24 deletions bots/notify/src/main/java/org/openjdk/skara/bots/notify/MailingListUpdater.java
View file
Open in desktop
Expand Up @@ -32,9 +32,9 @@
import java.time.Duration;
import java.time.format.DateTimeFormatter;
import java.util.*;
import java.util.logging.Logger;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.logging.Logger;

public class MailingListUpdater implements UpdateConsumer {
private final MailingList list;
Expand Down Expand Up @@ -101,9 +101,24 @@ private String commitToText(HostedRepository repository, Commit commit) {
return writer.toString();
}

private EmailAddress commitsToAuthor(List<Commit> commits) {
var commit = commits.get(commits.size() - 1);
var commitAddress = EmailAddress.from(commit.committer().name(), commit.committer().email());
private String tagAnnotationToText(HostedRepository repository, Tag.Annotated annotation) {
var writer = new StringWriter();
var printer = new PrintWriter(writer);

printer.println("Changeset: " + annotation.target().abbreviate());
printer.println("Author: " + annotation.author().name() + " <" + annotation.author().email() + ">");
printer.println("Date: " + annotation.date().format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss +0000")));
printer.println("URL: " + repository.webUrl(annotation.target()));
printer.println();
printer.print(String.join("\n", annotation.message()));

return writer.toString();
}

private EmailAddress filteredAuthor(EmailAddress commitAddress) {
if (author != null) {
return author;
}
var allowedAuthorMatcher = allowedAuthorDomains.matcher(commitAddress.domain());
if (!allowedAuthorMatcher.matches()) {
return sender;
Expand All @@ -112,6 +127,14 @@ private EmailAddress commitsToAuthor(List<Commit> commits) {
}
}

private EmailAddress commitToAuthor(Commit commit) {
return filteredAuthor(EmailAddress.from(commit.committer().name(), commit.committer().email()));
}

private EmailAddress annotationToAuthor(Tag.Annotated annotation) {
return filteredAuthor(EmailAddress.from(annotation.author().name(), annotation.author().email()));
}

private String commitsToSubject(HostedRepository repository, List<Commit> commits, Branch branch) {
var subject = new StringBuilder();
subject.append(repository.repositoryType().shortName());
Expand All @@ -131,12 +154,12 @@ private String commitsToSubject(HostedRepository repository, List<Commit> commit
return subject.toString();
}

private String tagToSubject(HostedRepository repository, Hash hash, OpenJDKTag tag) {
private String tagToSubject(HostedRepository repository, Hash hash, Tag tag) {
return repository.repositoryType().shortName() +
": " +
repository.name() +
": Added tag " +
tag.tag() +
tag +
" for changeset " +
hash.abbreviate();
}
Expand Down Expand Up @@ -170,11 +193,11 @@ private List<Commit> filterAndSendPrCommits(HostedRepository repository, List<Co
continue;
}
var rfr = rfrCandidates.get(0);
var finalAuthor = author != null ? author : commitsToAuthor(commits);

var body = commitToText(repository, commit);
var email = Email.reply(rfr, "Re: [Integrated] " + rfr.subject(), body)
.sender(sender)
.author(finalAuthor)
.author(commitToAuthor(commit))
.recipient(recipient)
.headers(headers)
.build();
Expand All @@ -197,10 +220,11 @@ private void sendCombinedCommits(HostedRepository repository, List<Commit> commi
}

var subject = commitsToSubject(repository, commits, branch);
var finalAuthor = author != null ? author : commitsToAuthor(commits);
var lastCommit = commits.get(commits.size() - 1);
var commitAddress = filteredAuthor(EmailAddress.from(lastCommit.committer().name(), lastCommit.committer().email()));
var email = Email.create(subject, writer.toString())
.sender(sender)
.author(finalAuthor)
.author(commitAddress)
.recipient(recipient)
.headers(headers)
.build();
Expand All @@ -224,34 +248,49 @@ public void handleCommits(HostedRepository repository, List<Commit> commits, Bra
}

@Override
public void handleTagCommits(HostedRepository repository, List<Commit> commits, OpenJDKTag tag) {
public void handleAnnotatedTagCommits(HostedRepository repository, List<Commit> commits, Tag tag, Tag.Annotated annotation) {
if (mode == Mode.PR_ONLY) {
return;
}
var writer = new StringWriter();
var printer = new PrintWriter(writer);

printer.println("The following commits are included in " + tag.tag());
printer.println("========================================================");
for (var commit : commits) {
printer.print(commit.hash().abbreviate());
if (commit.message().size() > 0) {
printer.print(": " + commit.message().get(0));
if (annotation != null) {
printer.println(tagAnnotationToText(repository, annotation));
}

var openjdkTag = OpenJDKTag.create(tag);
if (openjdkTag.isPresent()) {
printer.println("The following commits are included in " + tag);
printer.println("========================================================");
for (var commit : commits) {
printer.print(commit.hash().abbreviate());
if (commit.message().size() > 0) {
printer.print(": " + commit.message().get(0));
}
printer.println();
}
printer.println();
}

var tagCommit = commits.get(commits.size() - 1);
var subject = tagToSubject(repository, tagCommit.hash(), tag);
var finalAuthor = author != null ? author : commitsToAuthor(commits);
var email = Email.create(subject, writer.toString())
.sender(sender)
.author(finalAuthor)
.recipient(recipient)
.headers(headers)
.build();
.headers(headers);

list.post(email);
if (annotation != null) {
email.author(annotationToAuthor(annotation));
} else {
email.author(commitToAuthor(tagCommit));
}

list.post(email.build());
}

@Override
public void handleTagCommits(HostedRepository repository, List<Commit> commits, Tag tag) {
handleAnnotatedTagCommits(repository, commits, tag, null);
}

private String newBranchSubject(HostedRepository repository, List<Commit> commits, Branch parent, Branch branch) {
Expand Down Expand Up @@ -293,7 +332,8 @@ public void handleNewBranch(HostedRepository repository, List<Commit> commits, B
}

var subject = newBranchSubject(repository, commits, parent, branch);
var finalAuthor = author != null ? author : commits.size() > 0 ? commitsToAuthor(commits) : sender;
var finalAuthor = commits.size() > 0 ? commitToAuthor(commits.get(commits.size() - 1)) : sender;

var email = Email.create(subject, writer.toString())
.sender(sender)
.author(finalAuthor)
Expand Down
4 changes: 2 additions & 2 deletions bots/notify/src/main/java/org/openjdk/skara/bots/notify/UpdateConsumer.java
View file
Open in desktop
Expand Up @@ -24,12 +24,12 @@

import org.openjdk.skara.forge.HostedRepository;
import org.openjdk.skara.vcs.*;
import org.openjdk.skara.vcs.openjdk.OpenJDKTag;

import java.util.List;

public interface UpdateConsumer {
void handleCommits(HostedRepository repository, List<Commit> commits, Branch branch);
void handleTagCommits(HostedRepository repository, List<Commit> commits, OpenJDKTag tag);
void handleTagCommits(HostedRepository repository, List<Commit> commits, Tag tag);
void handleAnnotatedTagCommits(HostedRepository repository, List<Commit> commits, Tag tag, Tag.Annotated annotation);
void handleNewBranch(HostedRepository repository, List<Commit> commits, Branch parent, Branch branch);
}
17 changes: 11 additions & 6 deletions bots/notify/src/test/java/org/openjdk/skara/bots/notify/UpdaterTests.java
View file
Open in desktop
Expand Up @@ -566,7 +566,7 @@ void testMailinglistTag(TestInfo testInfo) throws IOException {
var localRepo = CheckableRepository.init(localRepoFolder, repo.repositoryType());
credentials.commitLock(localRepo);
var masterHash = localRepo.resolve("master").orElseThrow();
localRepo.tag(masterHash, "jdk-12+1", "Added tag 1", "Duke", "duke@openjdk.java.net");
localRepo.tag(masterHash, "jdk-12+1", "Added tag 1", "Duke Tagger", "tagger@openjdk.java.net");
localRepo.pushAll(repo.url());

var listAddress = EmailAddress.parse(listServer.createList("test"));
Expand All @@ -591,23 +591,24 @@ void testMailinglistTag(TestInfo testInfo) throws IOException {

var editHash = CheckableRepository.appendAndCommit(localRepo, "Another line", "23456789: More fixes");
localRepo.fetch(repo.url(), "history:history");
localRepo.tag(editHash, "jdk-12+2", "Added tag 2", "Duke", "duke@openjdk.java.net");
localRepo.tag(editHash, "jdk-12+2", "Added tag 2", "Duke Tagger", "tagger@openjdk.java.net");
CheckableRepository.appendAndCommit(localRepo, "Another line 1", "34567890: Even more fixes");
CheckableRepository.appendAndCommit(localRepo, "Another line 2", "45678901: Yet even more fixes");
var editHash2 = CheckableRepository.appendAndCommit(localRepo, "Another line 3", "56789012: Still even more fixes");
localRepo.tag(editHash2, "jdk-12+4", "Added tag 3", "Duke", "duke@openjdk.java.net");
localRepo.tag(editHash2, "jdk-12+4", "Added tag 3", "Duke Tagger", "tagger@openjdk.java.net");
CheckableRepository.appendAndCommit(localRepo, "Another line 4", "67890123: Brand new fixes");
var editHash3 = CheckableRepository.appendAndCommit(localRepo, "Another line 5", "78901234: More brand new fixes");
localRepo.tag(editHash3, "jdk-13+0", "Added tag 4", "Duke", "duke@openjdk.java.net");
localRepo.tag(editHash3, "jdk-13+0", "Added tag 4", "Duke Tagger", "tagger@openjdk.java.net");
localRepo.pushAll(repo.url());

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

var conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(3, conversations.size());
assertEquals(4, conversations.size());

for (var conversation : conversations) {
var email = conversation.first();
Expand All @@ -618,27 +619,31 @@ void testMailinglistTag(TestInfo testInfo) throws IOException {
assertFalse(email.body().contains("56789012: Still even more fixes"));
assertFalse(email.body().contains("67890123: Brand new fixes"));
assertFalse(email.body().contains("78901234: More brand new fixes"));
assertEquals(EmailAddress.from("Duke Tagger", "tagger@openjdk.java.net"), email.author());
} else if (email.subject().equals("git: test: Added tag jdk-12+4 for changeset " + editHash2.abbreviate())) {
assertFalse(email.body().contains("23456789: More fixes"));
assertTrue(email.body().contains("34567890: Even more fixes"));
assertTrue(email.body().contains("45678901: Yet even more fixes"));
assertTrue(email.body().contains("56789012: Still even more fixes"));
assertFalse(email.body().contains("67890123: Brand new fixes"));
assertFalse(email.body().contains("78901234: More brand new fixes"));
assertEquals(EmailAddress.from("Duke Tagger", "tagger@openjdk.java.net"), email.author());
} else if (email.subject().equals("git: test: Added tag jdk-13+0 for changeset " + editHash3.abbreviate())) {
assertFalse(email.body().contains("23456789: More fixes"));
assertFalse(email.body().contains("34567890: Even more fixes"));
assertFalse(email.body().contains("45678901: Yet even more fixes"));
assertFalse(email.body().contains("56789012: Still even more fixes"));
assertFalse(email.body().contains("67890123: Brand new fixes"));
assertTrue(email.body().contains("78901234: More brand new fixes"));
} else if (email.subject().equals("git: test: 4 new changesets")) {
assertEquals(EmailAddress.from("Duke Tagger", "tagger@openjdk.java.net"), email.author());
} else if (email.subject().equals("git: test: 6 new changesets")) {
assertTrue(email.body().contains("23456789: More fixes"));
assertTrue(email.body().contains("34567890: Even more fixes"));
assertTrue(email.body().contains("45678901: Yet even more fixes"));
assertTrue(email.body().contains("56789012: Still even more fixes"));
assertTrue(email.body().contains("67890123: Brand new fixes"));
assertTrue(email.body().contains("78901234: More brand new fixes"));
assertEquals(EmailAddress.from("testauthor", "ta@none.none"), email.author());
} else {
fail("Mismatched subject: " + email.subject());
}
Expand Down