JAMES-1877 Bounce on invalid addresses before discarding them
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/96465710 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/96465710 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/96465710 Branch: refs/heads/master Commit: 9646571062ae917dd678c4c15c90012a648c2ab8 Parents: 1c6d1d0 Author: Benoit Tellier <[email protected]> Authored: Wed Dec 7 10:13:38 2016 +0700 Committer: Benoit Tellier <[email protected]> Committed: Tue Jan 10 18:14:17 2017 +0700 ---------------------------------------------------------------------- .../org/apache/mailet/base/test/FakeMail.java | 18 +++++++--- .../james/transport/mailets/RemoteDelivery.java | 38 ++++++++++---------- .../remoteDelivery/DeliveryRunnable.java | 6 ++-- .../mailets/remoteDelivery/MailDelivrer.java | 8 ++++- .../remoteDelivery/MailDelivrerTest.java | 24 ++++++++++++- .../remoteDelivery/RemoteDeliveryTest.java | 5 ++- 6 files changed, 67 insertions(+), 32 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/96465710/mailet/base/src/test/java/org/apache/mailet/base/test/FakeMail.java ---------------------------------------------------------------------- diff --git a/mailet/base/src/test/java/org/apache/mailet/base/test/FakeMail.java b/mailet/base/src/test/java/org/apache/mailet/base/test/FakeMail.java index 661882a..59a3b89 100644 --- a/mailet/base/src/test/java/org/apache/mailet/base/test/FakeMail.java +++ b/mailet/base/src/test/java/org/apache/mailet/base/test/FakeMail.java @@ -58,6 +58,19 @@ public class FakeMail implements Mail { new ByteArrayInputStream(text.getBytes(javaEncodingCharset)))) .build(); } + + public static FakeMail fromMail(Mail mail) throws MessagingException { + return new FakeMail(mail.getMessage(), + Lists.newArrayList(mail.getRecipients()), + mail.getName(), + mail.getSender(), + mail.getState(), + mail.getErrorMessage(), + mail.getLastUpdated(), + attributes(mail), + mail.getMessageSize(), + mail.getRemoteAddr()); + } public static Builder builder() { return new Builder(); @@ -212,11 +225,6 @@ public class FakeMail implements Mail { this.remoteAddr = remoteAddr; } - public FakeMail(Mail mail) throws MessagingException { - this(mail.getMessage(), Lists.newArrayList(mail.getRecipients()), mail.getName(), mail.getSender(), mail.getState(), mail.getErrorMessage(), - mail.getLastUpdated(), attributes(mail), mail.getMessageSize(), mail.getRemoteAddr()); - } - @Override public String getName() { return name; http://git-wip-us.apache.org/repos/asf/james-project/blob/96465710/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RemoteDelivery.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RemoteDelivery.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RemoteDelivery.java index 8a40298..3a1f17d 100644 --- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RemoteDelivery.java +++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RemoteDelivery.java @@ -23,7 +23,8 @@ import java.net.UnknownHostException; import java.util.Collection; import java.util.Locale; import java.util.Map; -import java.util.Vector; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import javax.inject.Inject; import javax.mail.MessagingException; @@ -36,6 +37,7 @@ import org.apache.james.queue.api.MailPrioritySupport; import org.apache.james.queue.api.MailQueue; import org.apache.james.queue.api.MailQueue.MailQueueException; import org.apache.james.queue.api.MailQueueFactory; +import org.apache.james.transport.mailets.remoteDelivery.Bouncer; import org.apache.james.transport.mailets.remoteDelivery.DeliveryRunnable; import org.apache.james.transport.mailets.remoteDelivery.RemoteDeliveryConfiguration; import org.apache.james.transport.mailets.remoteDelivery.RemoteDeliverySocketFactory; @@ -119,34 +121,37 @@ import com.google.common.collect.HashMultimap; */ public class RemoteDelivery extends GenericMailet { + public enum THREAD_STATE { + START_THREADS, + DO_NOT_START_THREADS + } + private static final String OUTGOING_MAILS = "outgoingMails"; - private static final boolean DEFAULT_START_THREADS = true; public static final String NAME_JUNCTION = "-to-"; private final DNSService dnsServer; private final DomainList domainList; private final MailQueueFactory queueFactory; private final Metric outgoingMailsMetric; - private final Collection<Thread> workersThreads; private final VolatileIsDestroyed volatileIsDestroyed; - private final boolean startThreads; + private final THREAD_STATE startThreads; private MailQueue queue; private Logger logger; private RemoteDeliveryConfiguration configuration; + private ExecutorService executor; @Inject public RemoteDelivery(DNSService dnsServer, DomainList domainList, MailQueueFactory queueFactory, MetricFactory metricFactory) { - this(dnsServer, domainList, queueFactory, metricFactory, DEFAULT_START_THREADS); + this(dnsServer, domainList, queueFactory, metricFactory, THREAD_STATE.START_THREADS); } - public RemoteDelivery(DNSService dnsServer, DomainList domainList, MailQueueFactory queueFactory, MetricFactory metricFactory, boolean startThreads) { + public RemoteDelivery(DNSService dnsServer, DomainList domainList, MailQueueFactory queueFactory, MetricFactory metricFactory, THREAD_STATE startThreads) { this.dnsServer = dnsServer; this.domainList = domainList; this.queueFactory = queueFactory; this.outgoingMailsMetric = metricFactory.generate(OUTGOING_MAILS); this.volatileIsDestroyed = new VolatileIsDestroyed(); - this.workersThreads = new Vector<Thread>(); this.startThreads = startThreads; } @@ -160,25 +165,23 @@ public class RemoteDelivery extends GenericMailet { } catch (UnknownHostException e) { log("Invalid bind setting (" + configuration.getBindAddress() + "): " + e.toString()); } - if (startThreads) { + if (startThreads == THREAD_STATE.START_THREADS) { initDeliveryThreads(); } } private void initDeliveryThreads() { + executor = Executors.newFixedThreadPool(configuration.getWorkersThreadCount()); for (int a = 0; a < configuration.getWorkersThreadCount(); a++) { - String threadName = "Remote delivery thread (" + a + ")"; - Thread t = new Thread( + executor.execute( new DeliveryRunnable(queue, configuration, dnsServer, outgoingMailsMetric, logger, getMailetContext(), - volatileIsDestroyed), - threadName); - t.start(); - workersThreads.add(t); + new Bouncer(configuration, getMailetContext(), logger), + volatileIsDestroyed)); } } @@ -255,12 +258,9 @@ public class RemoteDelivery extends GenericMailet { */ @Override public synchronized void destroy() { - if (startThreads) { + if (startThreads == THREAD_STATE.START_THREADS) { volatileIsDestroyed.markAsDestroyed(); - // Wake up all threads from waiting for an accept - for (Thread t : workersThreads) { - t.interrupt(); - } + executor.shutdown(); notifyAll(); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/96465710/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/DeliveryRunnable.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/DeliveryRunnable.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/DeliveryRunnable.java index bc01245..7af269b 100644 --- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/DeliveryRunnable.java +++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/DeliveryRunnable.java @@ -54,9 +54,9 @@ public class DeliveryRunnable implements Runnable { private final Supplier<Date> dateSupplier; public DeliveryRunnable(MailQueue queue, RemoteDeliveryConfiguration configuration, DNSService dnsServer, Metric outgoingMailsMetric, - Logger logger, MailetContext mailetContext, VolatileIsDestroyed volatileIsDestroyed) { - this(queue, configuration, outgoingMailsMetric, logger, new Bouncer(configuration, mailetContext, logger), - new MailDelivrer(configuration, new MailDelivrerToHost(configuration, mailetContext, logger), dnsServer, logger), + Logger logger, MailetContext mailetContext, Bouncer bouncer, VolatileIsDestroyed volatileIsDestroyed) { + this(queue, configuration, outgoingMailsMetric, logger, bouncer, + new MailDelivrer(configuration, new MailDelivrerToHost(configuration, mailetContext, logger), dnsServer, bouncer, logger), volatileIsDestroyed, CURRENT_DATE_SUPPLIER); } http://git-wip-us.apache.org/repos/asf/james-project/blob/96465710/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/MailDelivrer.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/MailDelivrer.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/MailDelivrer.java index c89a2a0..bb03b62 100644 --- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/MailDelivrer.java +++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/MailDelivrer.java @@ -45,11 +45,13 @@ public class MailDelivrer { private final MailDelivrerToHost mailDelivrerToHost; private final DnsHelper dnsHelper; private final MessageComposer messageComposer; + private final Bouncer bouncer; private final Logger logger; - public MailDelivrer(RemoteDeliveryConfiguration configuration, MailDelivrerToHost mailDelivrerToHost, DNSService dnsServer, Logger logger) { + public MailDelivrer(RemoteDeliveryConfiguration configuration, MailDelivrerToHost mailDelivrerToHost, DNSService dnsServer, Bouncer bouncer, Logger logger) { this.configuration = configuration; this.mailDelivrerToHost = mailDelivrerToHost; + this.bouncer = bouncer; this.dnsHelper = new DnsHelper(dnsServer, configuration, logger); this.messageComposer = new MessageComposer(configuration); this.logger = logger; @@ -191,6 +193,10 @@ public class MailDelivrer { } } if (!validUnsentAddresses.isEmpty()) { + if (!invalidAddresses.isEmpty()) { + mail.setRecipients(invalidAddresses); + bouncer.bounce(mail, sfe); + } mail.setRecipients(validUnsentAddresses); if (enhancedMessagingException.hasReturnCode()) { boolean isPermanent = enhancedMessagingException.isServerError(); http://git-wip-us.apache.org/repos/asf/james-project/blob/96465710/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/MailDelivrerTest.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/MailDelivrerTest.java b/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/MailDelivrerTest.java index 9c5fe69..c96e9a1 100644 --- a/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/MailDelivrerTest.java +++ b/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/MailDelivrerTest.java @@ -21,6 +21,8 @@ package org.apache.james.transport.mailets.remoteDelivery; import static org.mockito.Mockito.mock; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import javax.mail.Address; import javax.mail.SendFailedException; @@ -42,10 +44,12 @@ public class MailDelivrerTest { private static final Logger LOGGER = LoggerFactory.getLogger(MailDelivrerTest.class); private MailDelivrer testee; + private Bouncer bouncer; @Before public void setUp() { - testee = new MailDelivrer(mock(RemoteDeliveryConfiguration.class), mock(MailDelivrerToHost.class), mock(DNSService.class), LOGGER); + bouncer = mock(Bouncer.class); + testee = new MailDelivrer(mock(RemoteDeliveryConfiguration.class), mock(MailDelivrerToHost.class), mock(DNSService.class), bouncer, LOGGER); } @Test @@ -180,4 +184,22 @@ public class MailDelivrerTest { assertThat(mail.getRecipients()).containsOnly(MailAddressFixture.OTHER_AT_JAMES); } + + @Test + public void handleSenderFailedExceptionShouldBounceInvalidAddressesOnBothInvalidAndValidUnsent() throws Exception { + Mail mail = FakeMail.builder().recipients(MailAddressFixture.ANY_AT_JAMES, MailAddressFixture.OTHER_AT_JAMES).build(); + + Address[] validSent = {}; + Address[] validUnsent = {new InternetAddress(MailAddressFixture.OTHER_AT_JAMES.asString())}; + Address[] invalid = {new InternetAddress(MailAddressFixture.ANY_AT_JAMES.asString())}; + SendFailedException sfe = new SendFailedException("Message", + new Exception(), + validSent, + validUnsent, + invalid); + testee.handleSenderFailedException(mail, sfe); + + verify(bouncer).bounce(mail, sfe); + verifyNoMoreInteractions(bouncer); + } } http://git-wip-us.apache.org/repos/asf/james-project/blob/96465710/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/RemoteDeliveryTest.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/RemoteDeliveryTest.java b/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/RemoteDeliveryTest.java index 2012a2e..6e944e3 100644 --- a/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/RemoteDeliveryTest.java +++ b/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/RemoteDeliveryTest.java @@ -66,7 +66,7 @@ public class RemoteDeliveryTest { @Override public void enQueue(Mail mail) throws MailQueueException { try { - enqueuedMail.add(new FakeMail(mail)); + enqueuedMail.add(FakeMail.fromMail(mail)); } catch (MessagingException e) { throw Throwables.propagate(e); } @@ -82,7 +82,6 @@ public class RemoteDeliveryTest { } } - public static final boolean DONT_START_THREADS = false; private RemoteDelivery remoteDelivery; private FakeMailQueue mailQueue; @@ -91,7 +90,7 @@ public class RemoteDeliveryTest { MailQueueFactory queueFactory = mock(MailQueueFactory.class); mailQueue = new FakeMailQueue(); when(queueFactory.getQueue(RemoteDeliveryConfiguration.OUTGOING)).thenReturn(mailQueue); - remoteDelivery = new RemoteDelivery(mock(DNSService.class), mock(DomainList.class), queueFactory, mock(MetricFactory.class), DONT_START_THREADS); + remoteDelivery = new RemoteDelivery(mock(DNSService.class), mock(DomainList.class), queueFactory, mock(MetricFactory.class), RemoteDelivery.THREAD_STATE.DO_NOT_START_THREADS); } @Test --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
