JAMES-2268 Functionnal style and tests for PostmasterAlias PostmasterAlias was altering directly mail recipients and thus triggered an error while modifying Immutable collection
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/dfd3f420 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/dfd3f420 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/dfd3f420 Branch: refs/heads/master Commit: dfd3f420bfe4cb6ad38150034380221d793aa6a8 Parents: 02f9631 Author: benwa <btell...@linagora.com> Authored: Thu Dec 21 14:00:29 2017 +0700 Committer: benwa <btell...@linagora.com> Committed: Mon Dec 25 11:25:18 2017 +0700 ---------------------------------------------------------------------- .../transport/mailets/PostmasterAlias.java | 48 ++++--- .../transport/mailets/PostmasterAliasTest.java | 125 +++++++++++++++++++ 2 files changed, 147 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/dfd3f420/mailet/standard/src/main/java/org/apache/james/transport/mailets/PostmasterAlias.java ---------------------------------------------------------------------- diff --git a/mailet/standard/src/main/java/org/apache/james/transport/mailets/PostmasterAlias.java b/mailet/standard/src/main/java/org/apache/james/transport/mailets/PostmasterAlias.java index 04c8c72..3366c86 100644 --- a/mailet/standard/src/main/java/org/apache/james/transport/mailets/PostmasterAlias.java +++ b/mailet/standard/src/main/java/org/apache/james/transport/mailets/PostmasterAlias.java @@ -17,26 +17,24 @@ * under the License. * ****************************************************************/ - - package org.apache.james.transport.mailets; import java.util.Collection; -import java.util.Vector; +import java.util.stream.Stream; import javax.mail.MessagingException; -import org.apache.mailet.Mail; import org.apache.james.core.MailAddress; -import org.apache.mailet.MailetContext; +import org.apache.mailet.Mail; import org.apache.mailet.base.GenericMailet; +import com.github.steveash.guavate.Guavate; + /** * Rewrites recipient addresses to make sure email for the postmaster is * always handled. This mailet is silently inserted at the top of the root * spool processor. All recipients mapped to postmaster@<servernames> are * changed to the postmaster account as specified in the server conf. - * */ public class PostmasterAlias extends GenericMailet { @@ -49,30 +47,28 @@ public class PostmasterAlias extends GenericMailet { * @throws MessagingException if an error is encountered while modifying the message */ public void service(Mail mail) throws MessagingException { - Collection<MailAddress> recipients = mail.getRecipients(); - Collection<MailAddress> recipientsToRemove = null; - MailetContext mailetContext = getMailetContext(); - boolean postmasterAddressed = false; + Collection<MailAddress> postmasterAliases = mail.getRecipients() + .stream() + .filter(this::isPostmasterAlias) + .collect(Guavate.toImmutableList()); - for (MailAddress addr : recipients) { - if (addr.getLocalPart().equalsIgnoreCase("postmaster") && - mailetContext.isLocalServer(addr.getDomain()) && !mailetContext.isLocalEmail(addr)) { - //Should remove this address... we want to replace it with - // the server's postmaster address - if (recipientsToRemove == null) { - recipientsToRemove = new Vector<>(); - } - recipientsToRemove.add(addr); - //Flag this as having found the postmaster - postmasterAddressed = true; - } - } - if (postmasterAddressed) { - recipients.removeAll(recipientsToRemove); - recipients.add(getMailetContext().getPostmaster()); + if (!postmasterAliases.isEmpty()) { + mail.setRecipients( + Stream.concat( + mail.getRecipients() + .stream() + .filter(address -> !postmasterAliases.contains(address)), + Stream.of(getMailetContext().getPostmaster())) + .collect(Guavate.toImmutableSet())); } } + private boolean isPostmasterAlias(MailAddress addr) { + return addr.getLocalPart().equalsIgnoreCase("postmaster") + && getMailetContext().isLocalServer(addr.getDomain()) + && !getMailetContext().isLocalEmail(addr); + } + /** * Return a string describing this mailet. * http://git-wip-us.apache.org/repos/asf/james-project/blob/dfd3f420/mailet/standard/src/test/java/org/apache/james/transport/mailets/PostmasterAliasTest.java ---------------------------------------------------------------------- diff --git a/mailet/standard/src/test/java/org/apache/james/transport/mailets/PostmasterAliasTest.java b/mailet/standard/src/test/java/org/apache/james/transport/mailets/PostmasterAliasTest.java new file mode 100644 index 0000000..f485158 --- /dev/null +++ b/mailet/standard/src/test/java/org/apache/james/transport/mailets/PostmasterAliasTest.java @@ -0,0 +1,125 @@ +/**************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one * + * or more contributor license agreements. See the NOTICE file * + * distributed with this work for additional information * + * regarding copyright ownership. The ASF licenses this file * + * to you under the Apache License, Version 2.0 (the * + * "License"); you may not use this file except in compliance * + * with the License. You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, * + * software distributed under the License is distributed on an * + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * + * KIND, either express or implied. See the License for the * + * specific language governing permissions and limitations * + * under the License. * + ****************************************************************/ + +package org.apache.james.transport.mailets; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.apache.james.core.MailAddress; +import org.apache.mailet.Mail; +import org.apache.mailet.base.MailAddressFixture; +import org.apache.mailet.base.test.FakeMail; +import org.apache.mailet.base.test.FakeMailContext; +import org.apache.mailet.base.test.FakeMailetConfig; +import org.junit.Before; +import org.junit.Test; + +public class PostmasterAliasTest { + private PostmasterAlias testee; + private MailAddress postmaster; + private MailAddress postmasterAlias; + + @Before + public void setUp() throws Exception { + postmaster = new MailAddress("admin@localhost"); + postmasterAlias = new MailAddress("postmaster@localhost"); + testee = new PostmasterAlias(); + testee.init(FakeMailetConfig.builder() + .mailetContext(FakeMailContext.builder() + .postmaster(postmaster)) + .build()); + } + + @Test + public void serviceShouldAcceptMailsWithNoRecipients() throws Exception { + Mail mail = FakeMail.builder().build(); + + testee.service(mail); + + assertThat(mail.getRecipients()).isEmpty(); + } + + @Test + public void serviceShouldNotAlterMailsForPostmaster() throws Exception { + Mail mail = FakeMail.builder() + .recipient(postmaster) + .build(); + + testee.service(mail); + + assertThat(mail.getRecipients()).containsOnly(postmaster); + } + + @Test + public void serviceShouldNotAlterMailForOtherUsers() throws Exception { + Mail mail = FakeMail.builder() + .recipient(MailAddressFixture.ANY_AT_JAMES) + .build(); + + testee.service(mail); + + assertThat(mail.getRecipients()).containsOnly(MailAddressFixture.ANY_AT_JAMES); + } + + @Test + public void serviceShouldNotAlterPostmasterAliasWhenForOtherDomains() throws Exception { + MailAddress otherDomainPostmaster = new MailAddress("postmaster@otherDomain"); + Mail mail = FakeMail.builder() + .recipient(otherDomainPostmaster) + .build(); + + testee.service(mail); + + assertThat(mail.getRecipients()).containsOnly(otherDomainPostmaster); + } + + @Test + public void serviceShouldRewritePostmasterAlias() throws Exception { + Mail mail = FakeMail.builder() + .recipient(postmasterAlias) + .build(); + + testee.service(mail); + + assertThat(mail.getRecipients()).containsOnly(postmaster); + } + + @Test + public void serviceShouldNotAlterOtherRecipientsWhenRewritingPostmaster() throws Exception { + Mail mail = FakeMail.builder() + .recipients(postmasterAlias, MailAddressFixture.ANY_AT_JAMES) + .build(); + + testee.service(mail); + + assertThat(mail.getRecipients()).containsOnly(postmaster, MailAddressFixture.ANY_AT_JAMES); + } + + @Test + public void serviceShouldNotDuplicatePostmaster() throws Exception { + Mail mail = FakeMail.builder() + .recipients(postmasterAlias, postmaster) + .build(); + + testee.service(mail); + + assertThat(mail.getRecipients()).containsOnly(postmaster); + } + +} \ No newline at end of file --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
