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

[JENKINS-32301] Support more than one Reply-To Address #26

Merged
merged 2 commits into from Mar 28, 2016
Merged

[JENKINS-32301] Support more than one Reply-To Address #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b619a50
[JENKINS-32301] Support more than one Reply-To Address
Mar 3, 2016
5aebf60
[JENKINS-32301] Document that MimeMessageBuilder is not thread-safe.
Mar 14, 2016
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
68 changes: 57 additions & 11 deletions src/main/java/jenkins/plugins/mailer/tasks/MimeMessageBuilder.java
View file
Open in desktop
Expand Up @@ -24,6 +24,7 @@
*/
package jenkins.plugins.mailer.tasks;

import com.google.common.collect.Lists;
import hudson.model.TaskListener;
import hudson.remoting.Base64;
import hudson.tasks.Mailer;
Expand All @@ -49,14 +50,17 @@

import java.io.UnsupportedEncodingException;
import java.security.interfaces.RSAPublicKey;
import java.util.Collection;
import java.util.Date;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.StringTokenizer;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
* Builder for {@link MimeMessage}. This class is NOT thread-safe.
* @author <a href="mailto:tom.fennelly@gmail.com">tom.fennelly@gmail.com</a>
*/
public class MimeMessageBuilder {
Expand All @@ -68,7 +72,7 @@ public class MimeMessageBuilder {
private TaskListener listener;
private String defaultSuffix;
private String from;
private String replyTo;
private Set<InternetAddress> replyTo = new LinkedHashSet<InternetAddress>();
private String subject;
private String body;
private AddressFilter recipientFilter;
Expand All @@ -81,8 +85,13 @@ public MimeMessageBuilder() {
JenkinsLocationConfiguration jlc = JenkinsLocationConfiguration.get();
if (jlc != null) {
defaultSuffix = Mailer.descriptor().getDefaultSuffix();
from = jlc.getAdminAddress();
replyTo = Mailer.descriptor().getReplyToAddress();
from = JenkinsLocationConfiguration.get().getAdminAddress();
final String rto = Mailer.descriptor().getReplyToAddress();
try {
replyTo.addAll(toNormalizedAddresses(rto));
} catch(UnsupportedEncodingException e) {
logError("Unable to parse Reply-To Addresses " + rto, e);
}
}
}
}
Expand Down Expand Up @@ -113,10 +122,27 @@ public MimeMessageBuilder setFrom(@Nonnull String from) {
}

public MimeMessageBuilder setReplyTo(@Nonnull String replyTo) {
this.replyTo = replyTo;
try {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MimeMessageBuilder is single-thread, right? If no, the new code introduces concurrency issues

final List<InternetAddress> addresses = toNormalizedAddresses(replyTo);
// Done after to leave the current value untouched if there is a parsing error.
this.replyTo.clear();
this.replyTo.addAll(addresses);
} catch(UnsupportedEncodingException e) {
logError("Unable to parse Reply-To Addresses " + replyTo, e);
}
return this;
}

public MimeMessageBuilder addReplyTo(@Nonnull String replyTo) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used only for testing, right?

try {
this.replyTo.addAll(toNormalizedAddresses(replyTo));
} catch(UnsupportedEncodingException e) {
logError("Unable to parse Reply-To Addresses " + replyTo, e);
}
return this;
}


public MimeMessageBuilder setSubject(@Nonnull String subject) {
this.subject = subject;
return this;
Expand Down Expand Up @@ -178,8 +204,8 @@ public MimeMessage buildMimeMessage() throws MessagingException, UnsupportedEnco
addBody(msg);
addRecipients(msg);

if (StringUtils.isNotBlank(replyTo)) {
msg.setReplyTo(new Address[]{toNormalizedAddress(replyTo)});
if (!replyTo.isEmpty()) {
msg.setReplyTo(toAddressArray(replyTo));
}
return msg;
}
Expand All @@ -199,6 +225,15 @@ private void setJenkinsInstanceIdent(MimeMessage msg) throws MessagingException
}
}

private static Address[] toAddressArray(Collection<InternetAddress> c) {
if (c == null || c.isEmpty()) {
return new Address[0];
}
final Address[] addresses = new Address[c.size()];
c.toArray(addresses);
return addresses;
}

public static void setInReplyTo(@Nonnull MimeMessage msg, @Nonnull String inReplyTo) throws MessagingException {
msg.setHeader("In-Reply-To", inReplyTo);
msg.setHeader("References", inReplyTo);
Expand Down Expand Up @@ -234,17 +269,28 @@ private void addRecipients(MimeMessage msg) throws UnsupportedEncodingException,
addRecipients(msg, cc, Message.RecipientType.CC);
addRecipients(msg, bcc, Message.RecipientType.BCC);
}

private void addRecipients(MimeMessage msg, Set<InternetAddress> recipientList, Message.RecipientType recipientType) throws UnsupportedEncodingException, MessagingException {
if (recipientList.isEmpty()) {
return;
}
final Collection<InternetAddress> recipients = recipientFilter != null ? recipientFilter.apply(recipientList) : recipientList;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this code related to the PR title? I doubt. Maybe a standalone bugfix

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a small refactor to share the code needed for the PR with the existing one.

msg.setRecipients(recipientType, toAddressArray(recipients));
}

if (recipientFilter != null) {
Set<InternetAddress> filteredList = recipientFilter.apply(recipientList);
msg.setRecipients(recipientType, filteredList.toArray(new InternetAddress[filteredList.size()]));
} else {
msg.setRecipients(recipientType, recipientList.toArray(new InternetAddress[recipientList.size()]));
private List<InternetAddress> toNormalizedAddresses(String addresses) throws UnsupportedEncodingException {
final List<InternetAddress> list = Lists.newLinkedList();
if (StringUtils.isNotBlank(addresses)) {
StringTokenizer tokens = new StringTokenizer(addresses, " \t\n\r\f,");
while (tokens.hasMoreTokens()) {
String addressToken = tokens.nextToken();
InternetAddress internetAddress = toNormalizedAddress(addressToken);
if (internetAddress != null) {
list.add(internetAddress);
}
}
}
return list;
}

private InternetAddress toNormalizedAddress(String address) throws UnsupportedEncodingException {
Expand Down
44 changes: 38 additions & 6 deletions src/test/java/jenkins/plugins/mailer/tasks/MimeMessageBuilderTest.java
View file
Open in desktop
Expand Up @@ -51,14 +51,22 @@
* @author <a href="mailto:tom.fennelly@gmail.com">tom.fennelly@gmail.com</a>
*/
public class MimeMessageBuilderTest {
/** Address constant A. */
private static final String A = "tom.aaaa@gmail.com";
/** Address constant X. */
private static final String X = "tom.xxxx@gmail.com";
/** Address constant Y. */
private static final String Y = "tom.yyyy@gmail.com";
/** Address constant Z. */
private static final String Z = "tom.zzzz@gmail.com";

@Rule
public JenkinsRule jenkinsRule = new JenkinsRule();

@Before
public void setup() {
JenkinsLocationConfiguration.get().setAdminAddress("tom.aaaa@gmail.com");
Mailer.descriptor().setReplyToAddress("tom.aaaa@gmail.com");
JenkinsLocationConfiguration.get().setAdminAddress(A);
Mailer.descriptor().setReplyToAddress(A);
}

@Test
Expand All @@ -72,18 +80,18 @@ public void test_construction() throws Exception {
Address[] from = mimeMessage.getFrom();
Assert.assertNotNull(from);
Assert.assertEquals(1, from.length);
Assert.assertEquals("tom.aaaa@gmail.com", from[0].toString());
Assert.assertEquals(A, from[0].toString());
Address[] replyTo = mimeMessage.getReplyTo();
Assert.assertNotNull(from);
Assert.assertEquals(1, replyTo.length);
Assert.assertEquals("tom.aaaa@gmail.com", replyTo[0].toString());
Assert.assertEquals(A, replyTo[0].toString());

// check the recipient list...
Address[] allRecipients = mimeMessage.getAllRecipients();
Assert.assertNotNull(allRecipients);
Assert.assertEquals(2, allRecipients.length);
Assert.assertEquals("tom.xxxx@gmail.com", allRecipients[0].toString());
Assert.assertEquals("tom.yyyy@gmail.com", allRecipients[1].toString());
Assert.assertEquals(X, allRecipients[0].toString());
Assert.assertEquals(Y, allRecipients[1].toString());

// Make sure we can regen the instance identifier public key
String encodedIdent = mimeMessage.getHeader("X-Instance-Identity")[0];
Expand Down Expand Up @@ -164,4 +172,28 @@ public void test_addRecipients_tokenizer() throws Exception {
Assert.assertEquals("tom.yyyy@gmail.com", recipients[1].toString());
Assert.assertEquals("tom.zzzz@gmail.com", recipients[2].toString());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use constants everywhere since you are doing it in other places, just to be consistent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to avoid "unrelated changes", will do a further clean up in another PR.

}

@Test
@Issue("JENKINS-32301")
public void testMultipleReplyToAddress() throws Exception {
checkMultipleReplyToAddress("%s %s %s");
checkMultipleReplyToAddress("%s , %s %s");
checkMultipleReplyToAddress("%s %s, %s");
checkMultipleReplyToAddress("%s,%s,%s");
checkMultipleReplyToAddress(new MimeMessageBuilder().setReplyTo(X).addReplyTo(Y).addReplyTo(Z));
}

private void checkMultipleReplyToAddress(String replyTo) throws Exception {
checkMultipleReplyToAddress(new MimeMessageBuilder().setReplyTo(String.format(replyTo, X, Y, Z)));
}

private void checkMultipleReplyToAddress(MimeMessageBuilder messageBuilder) throws Exception {
MimeMessage mimeMessage = messageBuilder.buildMimeMessage();
Address[] recipients = mimeMessage.getReplyTo();
Assert.assertEquals(3, recipients.length);
Assert.assertEquals(X, recipients[0].toString());
Assert.assertEquals(Y, recipients[1].toString());
Assert.assertEquals(Z, recipients[2].toString());
}

}