JAMES-2242 Make JamesMailetContext match its documentation - sendMail(mail) intend to have the same behaviour as incoming SMTP - It was not the case, mail was set on top of the mail state, Hence the mail was directly processed by THAT processor, and not executed by ROOT processor - When correcting that issue, we then miss a way to send a mail to a specific processor. We need to add a method for that.
Hence, I added sendMail(mail, state) This solves the `Boucing issues` encountered in RemoteDelivery Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/9c1a7cdb Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/9c1a7cdb Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/9c1a7cdb Branch: refs/heads/master Commit: 9c1a7cdb57f3a39d392d51c512fd036b2475358c Parents: 8e60652 Author: benwa <btell...@linagora.com> Authored: Wed Nov 29 16:49:49 2017 +0700 Committer: Antoine Duprat <adup...@linagora.com> Committed: Mon Dec 4 14:42:20 2017 +0100 ---------------------------------------------------------------------- .../java/org/apache/mailet/MailetContext.java | 17 +++ .../mailet/base/test/FakeMailContext.java | 9 +- .../GatewayRemoteDeliveryIntegrationTest.java | 8 -- .../impl/JamesMailetContext.java | 9 +- .../impl/JamesMailetContextTest.java | 113 ++++++++++++++++++- .../mailets/remoteDelivery/Bouncer.java | 3 +- 6 files changed, 144 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/9c1a7cdb/mailet/api/src/main/java/org/apache/mailet/MailetContext.java ---------------------------------------------------------------------- diff --git a/mailet/api/src/main/java/org/apache/mailet/MailetContext.java b/mailet/api/src/main/java/org/apache/mailet/MailetContext.java index 202caf7..e43b201 100644 --- a/mailet/api/src/main/java/org/apache/mailet/MailetContext.java +++ b/mailet/api/src/main/java/org/apache/mailet/MailetContext.java @@ -338,6 +338,23 @@ public interface MailetContext { void sendMail(Mail mail) throws MessagingException; + + + /** + * Sends an outgoing message to the top of this mailet container's root queue, + * targeting a specific processing state. + * + * This functionally allows mail treatment done out of the MailetProcessor to be sent + * to a specific processor inside the MailetContainer. This is for instance useful for bouncing mail + * being remote delivered (asynchronously to original mail treatment) + * + * @param message The message to send + * @param state The state of the message, indicating the name of the processor for + * which the message will be queued + * @throws MessagingException if an error occurs accessing or sending the message + */ + void sendMail(Mail mail, String state) throws MessagingException; + /** * Bounces the message using a standard format with the given message. * <p/> http://git-wip-us.apache.org/repos/asf/james-project/blob/9c1a7cdb/mailet/base/src/test/java/org/apache/mailet/base/test/FakeMailContext.java ---------------------------------------------------------------------- diff --git a/mailet/base/src/test/java/org/apache/mailet/base/test/FakeMailContext.java b/mailet/base/src/test/java/org/apache/mailet/base/test/FakeMailContext.java index 9c54dc0..62d2c9d 100644 --- a/mailet/base/src/test/java/org/apache/mailet/base/test/FakeMailContext.java +++ b/mailet/base/src/test/java/org/apache/mailet/base/test/FakeMailContext.java @@ -27,13 +27,14 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Optional; + import javax.mail.MessagingException; import javax.mail.internet.MimeMessage; +import org.apache.james.core.MailAddress; import org.apache.mailet.HostAddress; import org.apache.mailet.LookupException; import org.apache.mailet.Mail; -import org.apache.james.core.MailAddress; import org.apache.mailet.MailetContext; import org.slf4j.Logger; @@ -404,6 +405,12 @@ public class FakeMailContext implements MailetContext { } public void sendMail(Mail mail) throws MessagingException { + sendMail(mail, Mail.DEFAULT); + } + + @Override + public void sendMail(Mail mail, String state) throws MessagingException { + mail.setState(state); sentMails.add(fromMail(mail)); } http://git-wip-us.apache.org/repos/asf/james-project/blob/9c1a7cdb/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/GatewayRemoteDeliveryIntegrationTest.java ---------------------------------------------------------------------- diff --git a/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/GatewayRemoteDeliveryIntegrationTest.java b/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/GatewayRemoteDeliveryIntegrationTest.java index 2e01488..e325ef2 100644 --- a/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/GatewayRemoteDeliveryIntegrationTest.java +++ b/server/mailet/integration-testing/src/test/java/org/apache/james/mailets/GatewayRemoteDeliveryIntegrationTest.java @@ -52,7 +52,6 @@ import org.apache.james.utils.IMAPMessageReader; import org.apache.james.utils.SMTPMessageSender; import org.junit.After; import org.junit.Before; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.RuleChain; @@ -220,13 +219,6 @@ public class GatewayRemoteDeliveryIntegrationTest { .body("", hasSize(0)); } } - - @Ignore("JAMES-2242" + - "https://issues.apache.org/jira/browse/JAMES-2242?focusedCommentId=16270289&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16270289"; + - "Bouncing is failing due to several issues:" + - " - A loop in Bounce processor, keeps posting in bounce processor (DSNBounce processor don't rest mail state)" + - " - Memory mail queue don't preserve state field like other Mail Queue implementations" + - " - Memory DNS returns RuntimeExceptions upon absent address, not DNS failures") @Test public void remoteDeliveryShouldBounceUponFailure() throws Exception { String gatewayProperty = "invalid.domain"; http://git-wip-us.apache.org/repos/asf/james-project/blob/9c1a7cdb/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/JamesMailetContext.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/JamesMailetContext.java b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/JamesMailetContext.java index f460d55..51d01f9 100644 --- a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/JamesMailetContext.java +++ b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/JamesMailetContext.java @@ -406,15 +406,20 @@ public class JamesMailetContext implements MailetContext, Configurable { @Override public void sendMail(Mail mail) throws MessagingException { + sendMail(mail, Mail.DEFAULT); + } + + @Override + public void sendMail(Mail mail, String state) throws MessagingException { mail.setAttribute(Mail.SENT_BY_MAILET, "true"); + mail.setState(state); rootMailQueue.enQueue(mail); } public void sendMail(MailAddress sender, Collection<MailAddress> recipients, MimeMessage message, String state) throws MessagingException { MailImpl mail = new MailImpl(MailImpl.getId(), sender, recipients, message); try { - mail.setState(state); - sendMail(mail); + sendMail(mail, state); } finally { LifecycleUtil.dispose(mail); } http://git-wip-us.apache.org/repos/asf/james-project/blob/9c1a7cdb/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/impl/JamesMailetContextTest.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/impl/JamesMailetContextTest.java b/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/impl/JamesMailetContextTest.java index ccb8abd..61abeeb 100644 --- a/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/impl/JamesMailetContextTest.java +++ b/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/impl/JamesMailetContextTest.java @@ -21,11 +21,14 @@ package org.apache.james.mailetcontainer.impl; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import javax.mail.internet.MimeMessage; + import org.apache.commons.configuration.HierarchicalConfiguration; import org.apache.james.core.MailAddress; import org.apache.james.dnsservice.api.DNSService; @@ -35,9 +38,11 @@ import org.apache.james.queue.api.MailQueue; import org.apache.james.queue.api.MailQueueFactory; import org.apache.james.server.core.MailImpl; import org.apache.james.user.memory.MemoryUsersRepository; +import org.apache.mailet.Mail; import org.apache.mailet.base.test.MimeMessageBuilder; import org.junit.Before; import org.junit.Test; +import org.mockito.ArgumentCaptor; import com.google.common.collect.ImmutableList; @@ -52,6 +57,7 @@ public class JamesMailetContextTest { private MemoryUsersRepository usersRepository; private JamesMailetContext testee; private MailAddress mailAddress; + private MailQueue spoolMailQueue; @Before public void setUp() throws Exception { @@ -65,7 +71,8 @@ public class JamesMailetContextTest { usersRepository.setDomainList(domainList); testee = new JamesMailetContext(); MailQueueFactory mailQueueFactory = mock(MailQueueFactory.class); - when(mailQueueFactory.getQueue(anyString())).thenReturn(mock(MailQueue.class)); + spoolMailQueue = mock(MailQueue.class); + when(mailQueueFactory.getQueue(MailQueueFactory.SPOOL)).thenReturn(spoolMailQueue); testee.retrieveRootMailQueue(mailQueueFactory); testee.setDomainList(domainList); testee.setUsersRepository(usersRepository); @@ -160,4 +167,106 @@ public class JamesMailetContextTest { mail.setMessage(MimeMessageBuilder.defaultMimeMessage()); testee.bounce(mail, "message"); } + + @Test + public void bounceShouldEnqueueEmailWithRootState() throws Exception { + MailImpl mail = new MailImpl(); + mail.setSender(mailAddress); + mail.setRecipients(ImmutableList.of(mailAddress)); + mail.setMessage(MimeMessageBuilder.defaultMimeMessage()); + testee.bounce(mail, "message"); + + ArgumentCaptor<Mail> mailArgumentCaptor = ArgumentCaptor.forClass(Mail.class); + verify(spoolMailQueue).enQueue(mailArgumentCaptor.capture()); + verifyNoMoreInteractions(spoolMailQueue); + + assertThat(mailArgumentCaptor.getValue().getState()).isEqualTo(Mail.DEFAULT); + } + + @Test + public void sendMailShouldEnqueueEmailWithRootState() throws Exception { + MailImpl mail = new MailImpl(); + mail.setSender(mailAddress); + mail.setRecipients(ImmutableList.of(mailAddress)); + mail.setMessage(MimeMessageBuilder.defaultMimeMessage()); + testee.sendMail(mail); + + ArgumentCaptor<Mail> mailArgumentCaptor = ArgumentCaptor.forClass(Mail.class); + verify(spoolMailQueue).enQueue(mailArgumentCaptor.capture()); + verifyNoMoreInteractions(spoolMailQueue); + + assertThat(mailArgumentCaptor.getValue().getState()).isEqualTo(Mail.DEFAULT); + } + + @Test + public void sendMailShouldEnqueueEmailWithOtherStateWhenSpecified() throws Exception { + MailImpl mail = new MailImpl(); + mail.setSender(mailAddress); + mail.setRecipients(ImmutableList.of(mailAddress)); + mail.setMessage(MimeMessageBuilder.defaultMimeMessage()); + String other = "other"; + testee.sendMail(mail, other); + + ArgumentCaptor<Mail> mailArgumentCaptor = ArgumentCaptor.forClass(Mail.class); + verify(spoolMailQueue).enQueue(mailArgumentCaptor.capture()); + verifyNoMoreInteractions(spoolMailQueue); + + assertThat(mailArgumentCaptor.getValue().getState()).isEqualTo(other); + } + + @Test + public void sendMailForMessageShouldEnqueueEmailWithRootState() throws Exception { + MimeMessage message = MimeMessageBuilder.mimeMessageBuilder() + .addFrom(mailAddress.asString()) + .addToRecipient(mailAddress.asString()) + .setText("Simple text") + .build(); + + testee.sendMail(message); + + ArgumentCaptor<Mail> mailArgumentCaptor = ArgumentCaptor.forClass(Mail.class); + verify(spoolMailQueue).enQueue(mailArgumentCaptor.capture()); + verifyNoMoreInteractions(spoolMailQueue); + + assertThat(mailArgumentCaptor.getValue().getState()).isEqualTo(Mail.DEFAULT); + } + + @Test + public void sendMailForMessageAndEnvelopeShouldEnqueueEmailWithRootState() throws Exception { + MimeMessage message = MimeMessageBuilder.mimeMessageBuilder() + .addFrom(mailAddress.asString()) + .addToRecipient(mailAddress.asString()) + .setText("Simple text") + .build(); + + MailAddress sender = mailAddress; + ImmutableList<MailAddress> recipients = ImmutableList.of(mailAddress); + testee.sendMail(sender, recipients, message); + + ArgumentCaptor<Mail> mailArgumentCaptor = ArgumentCaptor.forClass(Mail.class); + verify(spoolMailQueue).enQueue(mailArgumentCaptor.capture()); + verifyNoMoreInteractions(spoolMailQueue); + + assertThat(mailArgumentCaptor.getValue().getState()).isEqualTo(Mail.DEFAULT); + } + + @Test + public void sendMailForMessageAndEnvelopeShouldEnqueueEmailWithOtherStateWhenSpecified() throws Exception { + MimeMessage message = MimeMessageBuilder.mimeMessageBuilder() + .addFrom(mailAddress.asString()) + .addToRecipient(mailAddress.asString()) + .setText("Simple text") + .build(); + + MailAddress sender = mailAddress; + ImmutableList<MailAddress> recipients = ImmutableList.of(mailAddress); + String otherState = "other"; + testee.sendMail(sender, recipients, message, otherState); + + ArgumentCaptor<Mail> mailArgumentCaptor = ArgumentCaptor.forClass(Mail.class); + verify(spoolMailQueue).enQueue(mailArgumentCaptor.capture()); + verifyNoMoreInteractions(spoolMailQueue); + + assertThat(mailArgumentCaptor.getValue().getState()).isEqualTo(otherState); + } } http://git-wip-us.apache.org/repos/asf/james-project/blob/9c1a7cdb/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/Bouncer.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/Bouncer.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/Bouncer.java index 05a302b..ce2539c 100644 --- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/Bouncer.java +++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/Bouncer.java @@ -52,9 +52,8 @@ public class Bouncer { } else { if (configuration.getBounceProcessor() != null) { mail.setAttribute(DELIVERY_ERROR, getErrorMsg(ex)); - mail.setState(configuration.getBounceProcessor()); try { - mailetContext.sendMail(mail); + mailetContext.sendMail(mail, configuration.getBounceProcessor()); } catch (MessagingException e) { LOGGER.warn("Exception re-inserting failed mail: ", e); } --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
