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

149: Improve formatting of bridged emails #236

Closed
wants to merge 3 commits 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
@@ -65,8 +65,11 @@ public boolean concurrentWith(WorkItem other) {
private void postNewMessage(Email email) {
var marker = String.format(bridgedMailMarker,
Base64.getEncoder().encodeToString(email.id().address().getBytes(StandardCharsets.UTF_8)));

var body = marker + "\n" +
"Mailing list message from " + email.author().toString() + "\n\n" +
"*Mailing list message from [" + email.author().fullName().orElse(email.author().localPart()) +
"](mailto:" + email.author().address() + ") on [" + email.sender().localPart() +
"](mailto:" + email.sender().address() + "):*\n\n" +
email.body();
pr.addComment(body);
}
@@ -35,15 +35,15 @@
import java.time.Duration;
import java.util.*;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.*;

class MailingListArchiveReaderBotTests {
private void addReply(Conversation conversation, MailingList mailingList, PullRequest pr) {
var first = conversation.first();

var reply = "Looks good";
var references = first.id().toString();
var email = Email.create(EmailAddress.from("Commenter", "<c@test.test>"), "Re: RFR: " + pr.title(), reply)
var email = Email.create(EmailAddress.from("Commenter", "c@test.test"), "Re: RFR: " + pr.title(), reply)
.recipient(first.author())
.id(EmailAddress.from(UUID.randomUUID() + "@id.id"))
.header("In-Reply-To", first.id().toString())
@@ -115,6 +115,9 @@ void simpleArchive(TestInfo testInfo) throws IOException {
// The bridge should now have processed the reply
var updated = pr.comments();
assertEquals(2, updated.size());
assertTrue(updated.get(1).body().contains("Mailing list message from"));
assertTrue(updated.get(1).body().contains("[Commenter](mailto:c@test.test)"));
assertTrue(updated.get(1).body().contains("[test](mailto:test@" + listAddress.domain() + ")"));
}
}

@@ -197,7 +197,7 @@ void simpleArchive(TestInfo testInfo) throws IOException {
assertEquals("RFR: 1234: This is a pull request", mail.subject());
assertEquals(pr.author().fullName(), mail.author().fullName().orElseThrow());
assertEquals(noreplyAddress(archive), mail.author().address());
assertEquals(from, mail.sender());
assertEquals(listAddress, mail.sender());
assertEquals("val1", mail.headerValue("Extra1"));
assertEquals("val2", mail.headerValue("Extra2"));

@@ -250,7 +250,7 @@ void simpleArchive(TestInfo testInfo) throws IOException {
assertEquals(3, conversations.get(0).allMessages().size());
for (var newMail : conversations.get(0).allMessages()) {
assertEquals(noreplyAddress(archive), newMail.author().address());
assertEquals(from, newMail.sender());
assertEquals(listAddress, newMail.sender());
}
assertTrue(conversations.get(0).allMessages().get(2).body().contains("This is a comment 😄"));
}
@@ -340,7 +340,7 @@ void reviewComment(TestInfo testInfo) throws IOException {
assertEquals(3, conversations.get(0).allMessages().size());
for (var newMail : conversations.get(0).allMessages()) {
assertEquals(noreplyAddress(archive), newMail.author().address());
assertEquals(from, newMail.sender());
assertEquals(listAddress, newMail.sender());
}
}
}
@@ -813,7 +813,7 @@ void incrementalChanges(TestInfo testInfo) throws IOException {
assertEquals(1, conversations.size());
for (var newMail : conversations.get(0).allMessages()) {
assertEquals(noreplyAddress(archive), newMail.author().address());
assertEquals(from, newMail.sender());
assertEquals(listAddress, newMail.sender());
}

// Add a comment
@@ -933,7 +933,7 @@ void rebased(TestInfo testInfo) throws IOException {
assertEquals(1, conversations.size());
for (var newMail : conversations.get(0).allMessages()) {
assertEquals(noreplyAddress(archive), newMail.author().address());
assertEquals(sender, newMail.sender());
assertEquals(listAddress, newMail.sender());
assertFalse(newMail.hasHeader("PR-Head-Hash"));
}
assertEquals("Re: [Rev 01] RFR: This is a pull request", conversations.get(0).allMessages().get(1).subject());
@@ -200,7 +200,7 @@ void testMailingList(TestInfo testInfo) throws IOException {

var conversations = mailmanList.conversations(Duration.ofDays(1));
var email = conversations.get(0).first();
assertEquals(sender, email.sender());
assertEquals(listAddress, email.sender());
assertEquals(sender, email.author());
assertEquals(email.recipients(), List.of(listAddress));
assertTrue(email.subject().contains(": 23456789: More fixes"));
@@ -256,7 +256,7 @@ void testMailingListMultiple(TestInfo testInfo) throws IOException {

var conversations = mailmanList.conversations(Duration.ofDays(1));
var email = conversations.get(0).first();
assertEquals(sender, email.sender());
assertEquals(listAddress, email.sender());
assertEquals(EmailAddress.from("another_author", "another@author.example.com"), email.author());
assertEquals(email.recipients(), List.of(listAddress));
assertTrue(email.subject().contains(": 2 new changesets"));
@@ -306,7 +306,7 @@ void testMailingListSponsored(TestInfo testInfo) throws IOException {

var conversations = mailmanList.conversations(Duration.ofDays(1));
var email = conversations.get(0).first();
assertEquals(sender, email.sender());
assertEquals(listAddress, email.sender());
assertEquals(EmailAddress.from("committer", "committer@test.test"), email.author());
assertEquals(email.recipients(), List.of(listAddress));
assertTrue(email.body().contains("Changeset: " + editHash.abbreviate()));
@@ -357,7 +357,7 @@ void testMailingListMultipleBranches(TestInfo testInfo) throws IOException {

var conversations = mailmanList.conversations(Duration.ofDays(1));
var email = conversations.get(0).first();
assertEquals(sender, email.sender());
assertEquals(listAddress, email.sender());
assertEquals(author, email.author());
assertEquals(email.recipients(), List.of(listAddress));
assertFalse(email.subject().contains("another"));
@@ -379,7 +379,8 @@ void testMailingListMultipleBranches(TestInfo testInfo) throws IOException {
conversations = mailmanList.conversations(Duration.ofDays(1));
conversations.sort(Comparator.comparing(conversation -> conversation.first().subject()));
email = conversations.get(0).first();
assertEquals(email.sender(), sender);
assertEquals(author, email.author());
assertEquals(listAddress, email.sender());
assertEquals(email.recipients(), List.of(listAddress));
assertTrue(email.subject().contains(": another: 456789AB: Yet more fixes"));
assertFalse(email.subject().contains("master"));
@@ -450,7 +451,7 @@ void testMailingListPROnly(TestInfo testInfo) throws IOException {
assertEquals(1, conversations.size());
var first = conversations.get(0).first();
var email = conversations.get(0).replies(first).get(0);
assertEquals(sender, email.sender());
assertEquals(listAddress, email.sender());
assertEquals(author, email.author());
assertEquals(email.recipients(), List.of(listAddress));
assertEquals("Re: [Integrated] RFR: My PR", email.subject());
@@ -537,7 +538,7 @@ void testMailingListPR(TestInfo testInfo) throws IOException {
var pushConversation = conversations.get(1);

var prEmail = prConversation.replies(prConversation.first()).get(0);
assertEquals(sender, prEmail.sender());
assertEquals(listAddress, prEmail.sender());
assertEquals(EmailAddress.from("testauthor", "ta@none.none"), prEmail.author());
assertEquals(prEmail.recipients(), List.of(listAddress));
assertEquals("Re: [Integrated] RFR: My PR", prEmail.subject());
@@ -548,7 +549,7 @@ void testMailingListPR(TestInfo testInfo) throws IOException {
assertFalse(prEmail.body().contains(masterHash.abbreviate()));

var pushEmail = pushConversation.first();
assertEquals(sender, pushEmail.sender());
assertEquals(listAddress, pushEmail.sender());
assertEquals(EmailAddress.from("testauthor", "ta@none.none"), pushEmail.author());
assertEquals(pushEmail.recipients(), List.of(listAddress));
assertTrue(pushEmail.subject().contains("23456789: More fixes"));
@@ -686,7 +687,7 @@ void testMailingListBranch(TestInfo testInfo) throws IOException {

var conversations = mailmanList.conversations(Duration.ofDays(1));
var email = conversations.get(0).first();
assertEquals(sender, email.sender());
assertEquals(listAddress, email.sender());
assertEquals(EmailAddress.from("testauthor", "ta@none.none"), email.author());
assertEquals(email.recipients(), List.of(listAddress));
assertEquals("git: test: created branch newbranch1 based on the branch master containing 2 unique commits", email.subject());
@@ -707,7 +708,7 @@ void testMailingListBranch(TestInfo testInfo) throws IOException {
.filter(c -> !c.equals(conversations.get(0)))
.findFirst().orElseThrow();
email = newConversation.first();
assertEquals(sender, email.sender());
assertEquals(listAddress, email.sender());
assertEquals(sender, email.author());
assertEquals(email.recipients(), List.of(listAddress));
assertEquals("git: test: created branch newbranch2 based on the branch newbranch1 containing 0 unique commits", email.subject());
@@ -42,7 +42,7 @@ public class Mbox {
private final static Pattern fromStringEncodePattern = Pattern.compile("^(>*From )", Pattern.MULTILINE);
private final static Pattern fromStringDecodePattern = Pattern.compile("^>(>*From )", Pattern.MULTILINE);

private static List<Email> splitMbox(String mbox) {
private static List<Email> splitMbox(String mbox, EmailAddress sender) {
// Initial split
var messages = mboxMessagePattern.matcher(mbox).results()
.map(match -> match.group(1))
@@ -57,8 +57,11 @@ private static List<Email> splitMbox(String mbox) {
for (var message : messages) {
messageBuilder.insert(0, message);
try {
var email = Email.parse(messageBuilder.toString());
parsedMails.add(email);
var email = Email.from(Email.parse(messageBuilder.toString()));
if (sender != null) {
email.sender(sender);
}
parsedMails.add(email.build());
messageBuilder.setLength(0);
} catch (RuntimeException ignored) {
}
@@ -79,7 +82,11 @@ private static String decodeFromStrings(String body) {
}

public static List<Conversation> parseMbox(String mbox) {
var emails = splitMbox(mbox);
return parseMbox(mbox, null);
}

public static List<Conversation> parseMbox(String mbox, EmailAddress sender) {
var emails = splitMbox(mbox, sender);
var idToMail = emails.stream().collect(Collectors.toMap(Email::id, Function.identity(), (a, b) -> a));
var idToConversation = idToMail.values().stream()
.filter(email -> !email.hasHeader("In-Reply-To"))
@@ -143,7 +143,7 @@ public List<Conversation> conversations(Duration maxAge) {

if (newContent) {
var concatenatedMbox = String.join("", actualPages);
var mails = Mbox.parseMbox(concatenatedMbox);
var mails = Mbox.parseMbox(concatenatedMbox, listAddress);
var threshold = ZonedDateTime.now().minus(maxAge);
cachedConversations = mails.stream()
.filter(mail -> mail.first().date().isAfter(threshold))
@@ -45,12 +45,16 @@ void simple() throws IOException {
.recipient(EmailAddress.parse(listAddress))
.build();
mailmanList.post(mail);
var expectedMail = Email.from(mail)
.sender(EmailAddress.parse(listAddress))
.build();

testServer.processIncoming();

var conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(1, conversations.size());
var conversation = conversations.get(0);
assertEquals(mail, conversation.first());
assertEquals(expectedMail, conversation.first());
}
}

@@ -67,29 +71,36 @@ void replies() throws IOException {
.build();
mailmanList.post(sentParent);
testServer.processIncoming();
var expectedParent = Email.from(sentParent)
.sender(EmailAddress.parse(listAddress))
.build();

var conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(1, conversations.size());
var conversation = conversations.get(0);
assertEquals(sentParent, conversation.first());
assertEquals(expectedParent, conversation.first());

var replier = EmailAddress.from("Replier", "replier@test.email");
var sentReply = Email.create(replier, "Reply subject", "Reply body")
.recipient(EmailAddress.parse(listAddress))
.header("In-Reply-To", sentParent.id().toString())
.build();
mailmanList.post(sentReply);
var expectedReply = Email.from(sentReply)
.sender(EmailAddress.parse(listAddress))
.build();

testServer.processIncoming();

conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(1, conversations.size());
conversation = conversations.get(0);
assertEquals(sentParent, conversation.first());
assertEquals(expectedParent, conversation.first());

var replies = conversation.replies(conversation.first());
assertEquals(1, replies.size());
var reply = replies.get(0);
assertEquals(sentReply, reply);
assertEquals(expectedReply, reply);
}
}

@@ -107,18 +118,21 @@ void cached() throws IOException {
mailmanList.post(mail);
testServer.processIncoming();

var expectedMail = Email.from(mail)
.sender(EmailAddress.parse(listAddress))
.build();
{
var conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(1, conversations.size());
var conversation = conversations.get(0);
assertEquals(mail, conversation.first());
assertEquals(expectedMail, conversation.first());
assertFalse(testServer.lastResponseCached());
}
{
var conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(1, conversations.size());
var conversation = conversations.get(0);
assertEquals(mail, conversation.first());
assertEquals(expectedMail, conversation.first());
assertTrue(testServer.lastResponseCached());
}
}
@@ -133,22 +147,26 @@ void interval() throws IOException {
var mailmanList = mailmanServer.getList(listAddress);
var sender = EmailAddress.from("Test", "test@test.email");
var mail1 = Email.create(sender, "Subject 1", "Body 1")
.recipient(EmailAddress.parse(listAddress))
.build();
.recipient(EmailAddress.parse(listAddress))
.build();
var mail2 = Email.create(sender, "Subject 2", "Body 2")
.recipient(EmailAddress.parse(listAddress))
.build();
new Thread(() -> {
mailmanList.post(mail1);
mailmanList.post(mail2);
}).start();
var expectedMail = Email.from(mail1)
.sender(EmailAddress.parse(listAddress))
.build();

testServer.processIncoming();
assertThrows(RuntimeException.class, () -> testServer.processIncoming(Duration.ZERO));

var conversations = mailmanList.conversations(Duration.ofDays(1));
assertEquals(1, conversations.size());
var conversation = conversations.get(0);
assertEquals(mail1, conversation.first());
assertEquals(expectedMail, conversation.first());
}
}
}