JAMES-1877 Tests MX iteration behavior upon deliver
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/c06d312a Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/c06d312a Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/c06d312a Branch: refs/heads/master Commit: c06d312a4facc24e281934706f7439ae20ef29aa Parents: 97e23ae Author: Benoit Tellier <[email protected]> Authored: Wed Dec 7 11:43:35 2016 +0700 Committer: Benoit Tellier <[email protected]> Committed: Tue Jan 10 18:14:27 2017 +0700 ---------------------------------------------------------------------- .../mailets/remoteDelivery/MailDelivrer.java | 91 +++------ .../remoteDelivery/MailDelivrerToHost.java | 74 ++++--- .../remoteDelivery/MailDelivrerTest.java | 201 +++++++++++++++++-- 3 files changed, 245 insertions(+), 121 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/c06d312a/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 f42a0fc..1036734 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 @@ -37,6 +37,8 @@ import org.apache.mailet.MailAddress; import org.slf4j.Logger; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import com.google.common.collect.Iterables; public class MailDelivrer { @@ -62,15 +64,13 @@ public class MailDelivrer { } /** - * We can assume that the recipients of this message are all going to the - * same mail server. We will now rely on the DNS server to do DNS MX record - * lookup and try to deliver to the multiple mail servers. If it fails, it - * should throw an exception. + * We can assume that the recipients of this message are all going to the same mail server. We will now rely on the + * DNS server to do DNS MX record lookup and try to deliver to the multiple mail servers. If it fails, it should + * throw an exception. * * @param mail org.apache.james.core.MailImpl * @param session javax.mail.Session - * @return boolean Whether the delivery was successful and the message can - * be deleted + * @return boolean Whether the delivery was successful and the message can be deleted */ public ExecutionResult deliver(Mail mail) { try { @@ -78,25 +78,13 @@ public class MailDelivrer { } catch (SendFailedException sfe) { return handleSenderFailedException(mail, sfe); } catch (MessagingException ex) { - // We should do a better job checking this... if the failure is a general - // connect exception, this is less descriptive than more specific SMTP command - // failure... have to lookup and see what are the various Exception possibilities - - // Unable to deliver message after numerous tries... fail accordingly - // We check whether this is a 5xx error message, which indicates a permanent failure (like account doesn't exist // or mailbox is full or domain is setup wrong). We fail permanently if this was a 5xx error - - boolean isPermanent = '5' == ex.getMessage().charAt(0); - ExecutionResult executionResult = ExecutionResult.onFailure(isPermanent, ex); - logger.debug(messageComposer.composeFailLogMessage(mail, executionResult)); - return executionResult; + boolean isPermanent = new EnhancedMessagingException(ex).isServerError(); + return logAndReturn(mail, ExecutionResult.onFailure(isPermanent, ex)); } catch (Exception ex) { - logger.error("Generic exception = permanent failure: " + ex.getMessage(), ex); - // Generic exception = permanent failure - ExecutionResult executionResult = ExecutionResult.permanentFailure(ex); - logger.debug(messageComposer.composeFailLogMessage(mail, executionResult)); - return executionResult; + logger.error("Generic exception = permanent failure: {}", ex.getMessage(), ex); + return logAndReturn(mail, ExecutionResult.permanentFailure(ex)); } } @@ -107,7 +95,7 @@ public class MailDelivrer { return ExecutionResult.permanentFailure(new Exception("No recipients specified for " + mail.getName() + " sent by " + mail.getSender())); } if (configuration.isDebug()) { - logger.debug("Attempting to deliver " + mail.getName()); + logger.debug("Attempting to deliver {}", mail.getName()); } String host = retrieveTargetHostname(mail); @@ -126,7 +114,8 @@ public class MailDelivrer { } private String retrieveTargetHostname(Mail mail) { - MailAddress rcpt = mail.getRecipients().iterator().next(); + Preconditions.checkArgument(!mail.getRecipients().isEmpty(), "Mail should have recipients to attempt delivery"); + MailAddress rcpt = Iterables.getFirst(mail.getRecipients(), null); return rcpt.getDomain(); } @@ -136,9 +125,7 @@ public class MailDelivrer { while (targetServers.hasNext()) { try { - if (mailDelivrerToHost.tryDeliveryToHost(mail, addr, targetServers.next())) { - return ExecutionResult.success(); - } + return mailDelivrerToHost.tryDeliveryToHost(mail, addr, targetServers.next()); } catch (SendFailedException sfe) { lastError = handleSendFailExceptionOnMxIteration(mail, sfe); } catch (MessagingException me) { @@ -163,21 +150,16 @@ public class MailDelivrer { } private MessagingException handleMessagingException(Mail mail, MessagingException me) throws MessagingException { - logger.debug("Exception delivering message (" + mail.getName() + ") - " + me.getMessage()); + logger.debug("Exception delivering message ({}) - {}", mail.getName(), me.getMessage()); if ((me.getNextException() != null) && (me.getNextException() instanceof IOException)) { - // This is more than likely a temporary failure - // If it's an IO exception with no nested exception, it's probably // some socket or weird I/O related problem. return me; } else { - // This was not a connection or I/O error particular to one - // SMTP server of an MX set. Instead, it is almost certainly - // a protocol level error. In this case we assume that this - // is an error we'd encounter with any of the SMTP servers - // associated with this MX record, and we pass the exception - // to the code in the outer block that determines its - // severity. + // This was not a connection or I/O error particular to one SMTP server of an MX set. Instead, it is almost + // certainly a protocol level error. In this case we assume that this is an error we'd encounter with any of + // the SMTP servers associated with this MX record, and we pass the exception to the code in the outer block + // that determines its severity. throw me; } } @@ -189,12 +171,12 @@ public class MailDelivrer { List<MailAddress> invalidAddresses = SFEHelper.getAddressesAsMailAddress(sfe.getInvalidAddresses(), logger); List<MailAddress> validUnsentAddresses = SFEHelper.getAddressesAsMailAddress(sfe.getValidUnsentAddresses(), logger); if (configuration.isDebug()) { - logger.debug("Mail " + mail.getName() + " has initially recipients: " + mail.getRecipients()); + logger.debug("Mail {} has initially recipients: {}", mail.getName(), mail.getRecipients()); if (!invalidAddresses.isEmpty()) { - logger.debug("Invalid recipients: " + invalidAddresses); + logger.debug("Invalid recipients: {}", invalidAddresses); } if (!validUnsentAddresses.isEmpty()) { - logger.debug("Unsent recipients: " + validUnsentAddresses); + logger.debug("Unsent recipients: {}", validUnsentAddresses); } } if (!validUnsentAddresses.isEmpty()) { @@ -234,14 +216,10 @@ public class MailDelivrer { if (sfe.getValidSentAddresses() != null) { Address[] validSent = sfe.getValidSentAddresses(); if (validSent.length > 0) { - logger.debug( "Mail (" + mail.getName() + ") sent successfully for " + Arrays.asList(validSent)); + logger.debug( "Mail ({}) sent successfully for {}", mail.getName(), Arrays.asList(validSent)); } } - /* - * SMTPSendFailedException introduced in JavaMail 1.3.2, and - * provides detailed protocol reply code for the operation - */ EnhancedMessagingException enhancedMessagingException = new EnhancedMessagingException(sfe); if (enhancedMessagingException.isServerError()) { throw sfe; @@ -249,7 +227,7 @@ public class MailDelivrer { if (sfe.getValidUnsentAddresses() != null && sfe.getValidUnsentAddresses().length > 0) { if (configuration.isDebug()) - logger.debug("Send failed, " + sfe.getValidUnsentAddresses().length + " valid addresses remain, continuing with any other servers"); + logger.debug("Send failed, {} valid addresses remain, continuing with any other servers", sfe.getValidUnsentAddresses().length); return sfe; } else { // There are no valid addresses left to send, so rethrow @@ -258,10 +236,9 @@ public class MailDelivrer { } private ExecutionResult handleNoTargetServer(Mail mail, String host) { - logger.info("No mail server found for: " + host); + logger.info("No mail server found for: {}", host); MessagingException messagingException = new MessagingException("There are no DNS entries for the hostname " + host + ". I cannot determine where to send this message."); int retry = DeliveryRetriesHelper.retrieveRetries(mail); - System.out.println("retyry " + retry); if (retry >= configuration.getDnsProblemRetry()) { return logAndReturn(mail, ExecutionResult.permanentFailure(messagingException)); } else { @@ -273,13 +250,10 @@ public class MailDelivrer { if (configuration.isDebug()) { EnhancedMessagingException enhancedMessagingException = new EnhancedMessagingException(sfe); if (enhancedMessagingException.hasReturnCode()) { - logger.debug("SMTP SEND FAILED:"); - logger.debug(sfe.toString()); - logger.debug(" Command: " + enhancedMessagingException.computeCommand()); - logger.debug(" RetCode: " + enhancedMessagingException.getReturnCode()); - logger.debug(" Response: " + sfe.getMessage()); + logger.debug("SMTP SEND FAILED: Command [{}] RetCode: [{}] Response[{}]", enhancedMessagingException.computeCommand(), + enhancedMessagingException.getReturnCode(), sfe.getMessage()); } else { - logger.debug("Send failed: " + sfe.toString()); + logger.debug("Send failed: {}", sfe.toString()); } logLevels(sfe); } @@ -291,12 +265,9 @@ public class MailDelivrer { me = (MessagingException) ne; EnhancedMessagingException enhancedMessagingException = new EnhancedMessagingException(me); if (me.getClass().getName().endsWith(".SMTPAddressFailedException") || me.getClass().getName().endsWith(".SMTPAddressSucceededException")) { - logger.debug("ADDRESS " + enhancedMessagingException.computeAction() + ":"); - logger.debug(me.toString()); - logger.debug(" Address: " + enhancedMessagingException.computeAddress()); - logger.debug(" Command: " + enhancedMessagingException.computeCommand()); - logger.debug(" RetCode: " + enhancedMessagingException.getReturnCode()); - logger.debug(" Response: " + me.getMessage()); + logger.debug("ADDRESS :[{}] Address:[{}] Command : [{}] RetCode[{}] Response [{}]", + enhancedMessagingException.computeAction(), me.toString(), enhancedMessagingException.computeAddress(), + enhancedMessagingException.computeCommand(), enhancedMessagingException.getReturnCode()); } } } http://git-wip-us.apache.org/repos/asf/james-project/blob/c06d312a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/MailDelivrerToHost.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/MailDelivrerToHost.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/MailDelivrerToHost.java index a9f5758..a6ffbad 100644 --- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/MailDelivrerToHost.java +++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/MailDelivrerToHost.java @@ -50,24 +50,16 @@ public class MailDelivrerToHost { this.logger = logger; } - public boolean tryDeliveryToHost(Mail mail, InternetAddress[] addr, HostAddress outgoingMailServer) throws MessagingException { - Properties props = session.getProperties(); - if (mail.getSender() == null) { - props.put("mail.smtp.from", "<>"); - } else { - String sender = mail.getSender().toString(); - props.put("mail.smtp.from", sender); - } - logger.debug("Attempting delivery of " + mail.getName() + " to host " + outgoingMailServer.getHostName() - + " at " + outgoingMailServer.getHost() + " from " + props.get("mail.smtp.from")); + public ExecutionResult tryDeliveryToHost(Mail mail, InternetAddress[] addr, HostAddress outgoingMailServer) throws MessagingException { + Properties props = getPropertiesForMail(mail); + logger.debug("Attempting delivery of {} to host {} at {} from {}", + mail.getName(), outgoingMailServer.getHostName(), outgoingMailServer.getHost(), props.get("mail.smtp.from")); // Many of these properties are only in later JavaMail versions // "mail.smtp.ehlo" //default true // "mail.smtp.auth" //default false - // "mail.smtp.dsn.ret" //default to nothing... appended as - // RET= after MAIL FROM line. - // "mail.smtp.dsn.notify" //default to nothing...appended as - // NOTIFY= after RCPT TO line. + // "mail.smtp.dsn.ret" //default to nothing... appended as RET= after MAIL FROM line. + // "mail.smtp.dsn.notify" //default to nothing... appended as NOTIFY= after RCPT TO line. SMTPTransport transport = null; try { @@ -75,12 +67,23 @@ public class MailDelivrerToHost { transport.setLocalHost( props.getProperty("mail.smtp.localhost", configuration.getHeloNameProvider().getHeloName()) ); connect(outgoingMailServer, transport); transport.sendMessage(adaptToTransport(mail.getMessage(), transport), addr); - logger.debug("Mail (" + mail.getName() + ") sent successfully to " + outgoingMailServer.getHostName() + - " at " + outgoingMailServer.getHost() + " from " + props.get("mail.smtp.from") + " for " + mail.getRecipients()); - return true; + logger.debug("Mail ({}) sent successfully to {} at {} from {} for {}", mail.getName(), outgoingMailServer.getHostName(), + outgoingMailServer.getHost(), props.get("mail.smtp.from"), mail.getRecipients()); } finally { closeTransport(mail, outgoingMailServer, transport); } + return ExecutionResult.success(); + } + + private Properties getPropertiesForMail(Mail mail) { + Properties props = session.getProperties(); + if (mail.getSender() == null) { + props.put("mail.smtp.from", "<>"); + } else { + String sender = mail.getSender().toString(); + props.put("mail.smtp.from", sender); + } + return props; } private void connect(HostAddress outgoingMailServer, SMTPTransport transport) throws MessagingException { @@ -92,27 +95,7 @@ public class MailDelivrerToHost { } private MimeMessage adaptToTransport(MimeMessage message, SMTPTransport transport) throws MessagingException { - // if the transport is a SMTPTransport (from sun) some - // performance enhancement can be done. - if (transport.getClass().getName().endsWith(".SMTPTransport")) { - // if the message is alredy 8bit or binary and the server doesn't support the 8bit extension it has - // to be converted to 7bit. Javamail api doesn't perform - // that conversion, but it is required to be a rfc-compliant smtp server. - - // Temporarily disabled. See JAMES-638 - if (!transport.supportsExtension(BIT_MIME_8)) { - try { - converter7Bit.convertTo7Bit(message); - } catch (IOException e) { - // An error has occured during the 7bit conversion. - // The error is logged and the message is sent anyway. - - logger.error("Error during the conversion to 7 bit.", e); - } - } - } else { - // If the transport is not the one developed by Sun we are not sure of how it - // handles the 8 bit mime stuff, so I convert the message to 7bit. + if (shouldAdapt(transport)) { try { converter7Bit.convertTo7Bit(message); } catch (IOException e) { @@ -122,6 +105,16 @@ public class MailDelivrerToHost { return message; } + private boolean shouldAdapt(SMTPTransport transport) { + // If the transport is a SMTPTransport (from sun) some performance enhancement can be done. + // If the transport is not the one developed by Sun we are not sure of how it handles the 8 bit mime stuff, so I + // convert the message to 7bit. + return !transport.getClass().getName().endsWith(".SMTPTransport") + || !transport.supportsExtension(BIT_MIME_8); + // if the message is already 8bit or binary and the server doesn't support the 8bit extension it has to be converted + // to 7bit. Javamail api doesn't perform that conversion, but it is required to be a rfc-compliant smtp server. + } + private void closeTransport(Mail mail, HostAddress outgoingMailServer, SMTPTransport transport) { if (transport != null) { try { @@ -131,8 +124,9 @@ public class MailDelivrerToHost { // of the mail transaction (MAIL, RCPT, DATA). transport.close(); } catch (MessagingException e) { - logger.error("Warning: could not close the SMTP transport after sending mail (" + mail.getName() + ") to " + outgoingMailServer.getHostName() + " at " + outgoingMailServer.getHost() + " for " + mail.getRecipients() + "; probably the server has already closed the " - + "connection. Message is considered to be delivered. Exception: " + e.getMessage()); + logger.error("Warning: could not close the SMTP transport after sending mail ({}) to {} at {} for {}; " + + "probably the server has already closed the connection. Message is considered to be delivered. Exception: {}", + mail.getName(), outgoingMailServer.getHostName(), outgoingMailServer.getHost(), mail.getRecipients(), e.getMessage()); } transport = null; } http://git-wip-us.apache.org/repos/asf/james-project/blob/c06d312a/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 3f7b726..b77fcb4 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 @@ -20,12 +20,18 @@ package org.apache.james.transport.mailets.remoteDelivery; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import java.io.IOException; + import javax.mail.Address; +import javax.mail.MessagingException; import javax.mail.SendFailedException; import javax.mail.internet.InternetAddress; @@ -47,16 +53,32 @@ import com.sun.mail.smtp.SMTPSenderFailedException; public class MailDelivrerTest { private static final Logger LOGGER = LoggerFactory.getLogger(MailDelivrerTest.class); + public static final String MX1_HOSTNAME = "mx1." + MailAddressFixture.JAMES2_APACHE_ORG; + public static final String MX2_HOSTNAME = "mx2." + MailAddressFixture.JAMES2_APACHE_ORG; + public static final String SMTP_URI2 = "protocol://userid:password@host:119/file1"; + public static final String SMTP_URI1 = "protocol://userid:password@host:119/file2"; + @SuppressWarnings("deprecation") + public static final HostAddress HOST_ADDRESS_1 = new HostAddress(MX1_HOSTNAME, SMTP_URI1); + @SuppressWarnings("deprecation") + public static final HostAddress HOST_ADDRESS_2 = new HostAddress(MX2_HOSTNAME, SMTP_URI2); private MailDelivrer testee; private Bouncer bouncer; private DnsHelper dnsHelper; + private MailDelivrerToHost mailDelivrerToHost; @Before public void setUp() { bouncer = mock(Bouncer.class); dnsHelper = mock(DnsHelper.class); - testee = new MailDelivrer(mock(RemoteDeliveryConfiguration.class), mock(MailDelivrerToHost.class), dnsHelper, bouncer, LOGGER); + mailDelivrerToHost = mock(MailDelivrerToHost.class); + RemoteDeliveryConfiguration configuration = new RemoteDeliveryConfiguration(FakeMailetConfig.builder() + .setProperty(RemoteDeliveryConfiguration.DELIVERY_THREADS, "1") + .setProperty(RemoteDeliveryConfiguration.MAX_DNS_PROBLEM_RETRIES, "3") + .setProperty(RemoteDeliveryConfiguration.DEBUG, "true") + .build(), + mock(DomainList.class)); + testee = new MailDelivrer(configuration, mailDelivrerToHost, dnsHelper, bouncer, LOGGER); } @Test @@ -224,11 +246,6 @@ public class MailDelivrerTest { @Test public void deliverShouldReturnTemporaryErrorWhenFirstDNSProblem() throws Exception { Mail mail = FakeMail.builder().recipients(MailAddressFixture.ANY_AT_JAMES, MailAddressFixture.OTHER_AT_JAMES).build(); - FakeMailetConfig mailetConfig = FakeMailetConfig.builder() - .setProperty(RemoteDeliveryConfiguration.DELIVERY_THREADS, "1") - .setProperty(RemoteDeliveryConfiguration.MAX_DNS_PROBLEM_RETRIES, "3") - .build(); - testee = new MailDelivrer(new RemoteDeliveryConfiguration(mailetConfig, mock(DomainList.class)), mock(MailDelivrerToHost.class), dnsHelper, bouncer, LOGGER); UnmodifiableIterator<HostAddress> empty = ImmutableList.<HostAddress>of().iterator(); when(dnsHelper.retrieveHostAddressIterator(MailAddressFixture.JAMES_APACHE_ORG)).thenReturn(empty); @@ -242,12 +259,7 @@ public class MailDelivrerTest { @Test public void deliverShouldReturnTemporaryErrorWhenToleratedDNSProblem() throws Exception { Mail mail = FakeMail.builder().recipients(MailAddressFixture.ANY_AT_JAMES, MailAddressFixture.OTHER_AT_JAMES).build(); - FakeMailetConfig mailetConfig = FakeMailetConfig.builder() - .setProperty(RemoteDeliveryConfiguration.DELIVERY_THREADS, "1") - .setProperty(RemoteDeliveryConfiguration.MAX_DNS_PROBLEM_RETRIES, "3") - .build(); DeliveryRetriesHelper.incrementRetries(mail); - testee = new MailDelivrer(new RemoteDeliveryConfiguration(mailetConfig, mock(DomainList.class)), mock(MailDelivrerToHost.class), dnsHelper, bouncer, LOGGER); UnmodifiableIterator<HostAddress> empty = ImmutableList.<HostAddress>of().iterator(); when(dnsHelper.retrieveHostAddressIterator(MailAddressFixture.JAMES_APACHE_ORG)).thenReturn(empty); @@ -261,14 +273,9 @@ public class MailDelivrerTest { @Test public void deliverShouldReturnPermanentErrorWhenLimitDNSProblemReached() throws Exception { Mail mail = FakeMail.builder().recipients(MailAddressFixture.ANY_AT_JAMES, MailAddressFixture.OTHER_AT_JAMES).build(); - FakeMailetConfig mailetConfig = FakeMailetConfig.builder() - .setProperty(RemoteDeliveryConfiguration.DELIVERY_THREADS, "1") - .setProperty(RemoteDeliveryConfiguration.MAX_DNS_PROBLEM_RETRIES, "3") - .build(); DeliveryRetriesHelper.incrementRetries(mail); DeliveryRetriesHelper.incrementRetries(mail); DeliveryRetriesHelper.incrementRetries(mail); - testee = new MailDelivrer(new RemoteDeliveryConfiguration(mailetConfig, mock(DomainList.class)), mock(MailDelivrerToHost.class), dnsHelper, bouncer, LOGGER); UnmodifiableIterator<HostAddress> empty = ImmutableList.<HostAddress>of().iterator(); when(dnsHelper.retrieveHostAddressIterator(MailAddressFixture.JAMES_APACHE_ORG)).thenReturn(empty); @@ -282,15 +289,11 @@ public class MailDelivrerTest { @Test public void deliverShouldReturnPermanentErrorWhenLimitDNSProblemExceeded() throws Exception { Mail mail = FakeMail.builder().recipients(MailAddressFixture.ANY_AT_JAMES, MailAddressFixture.OTHER_AT_JAMES).build(); - FakeMailetConfig mailetConfig = FakeMailetConfig.builder() - .setProperty(RemoteDeliveryConfiguration.DELIVERY_THREADS, "1") - .setProperty(RemoteDeliveryConfiguration.MAX_DNS_PROBLEM_RETRIES, "3") - .build(); + DeliveryRetriesHelper.incrementRetries(mail); DeliveryRetriesHelper.incrementRetries(mail); DeliveryRetriesHelper.incrementRetries(mail); DeliveryRetriesHelper.incrementRetries(mail); - testee = new MailDelivrer(new RemoteDeliveryConfiguration(mailetConfig, mock(DomainList.class)), mock(MailDelivrerToHost.class), dnsHelper, bouncer, LOGGER); UnmodifiableIterator<HostAddress> empty = ImmutableList.<HostAddress>of().iterator(); when(dnsHelper.retrieveHostAddressIterator(MailAddressFixture.JAMES_APACHE_ORG)).thenReturn(empty); @@ -300,4 +303,160 @@ public class MailDelivrerTest { assertThat(executionResult.getExecutionState()).isEqualTo(ExecutionResult.ExecutionState.PERMANENT_FAILURE); } + @SuppressWarnings("deprecation") + @Test + public void deliverShouldWork() throws Exception { + Mail mail = FakeMail.builder().recipients(MailAddressFixture.ANY_AT_JAMES, MailAddressFixture.OTHER_AT_JAMES).build(); + + UnmodifiableIterator<HostAddress> dnsEntries = ImmutableList.of( + HOST_ADDRESS_1, + HOST_ADDRESS_2).iterator(); + when(dnsHelper.retrieveHostAddressIterator(MailAddressFixture.JAMES_APACHE_ORG)).thenReturn(dnsEntries); + when(mailDelivrerToHost.tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), any(HostAddress.class))) + .thenReturn(ExecutionResult.success()); + ExecutionResult executionResult = testee.deliver(mail); + + verify(mailDelivrerToHost, times(1)).tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), any(HostAddress.class)); + assertThat(executionResult.getExecutionState()).isEqualTo(ExecutionResult.ExecutionState.SUCCESS); + } + + @SuppressWarnings("deprecation") + @Test + public void deliverShouldAbortWhenServerError() throws Exception { + Mail mail = FakeMail.builder().recipients(MailAddressFixture.ANY_AT_JAMES, MailAddressFixture.OTHER_AT_JAMES).build(); + + UnmodifiableIterator<HostAddress> dnsEntries = ImmutableList.of( + HOST_ADDRESS_1, + HOST_ADDRESS_2).iterator(); + when(dnsHelper.retrieveHostAddressIterator(MailAddressFixture.JAMES_APACHE_ORG)).thenReturn(dnsEntries); + when(mailDelivrerToHost.tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), any(HostAddress.class))) + .thenThrow(new MessagingException("500 : Horrible way to manage Server Return code")); + ExecutionResult executionResult = testee.deliver(mail); + + verify(mailDelivrerToHost, times(1)).tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), any(HostAddress.class)); + assertThat(executionResult.getExecutionState()).isEqualTo(ExecutionResult.ExecutionState.PERMANENT_FAILURE); + } + + @SuppressWarnings("deprecation") + @Test + public void deliverShouldAbortWithTemporaryWhenMessagingExceptionCauseUnknown() throws Exception { + Mail mail = FakeMail.builder().recipients(MailAddressFixture.ANY_AT_JAMES, MailAddressFixture.OTHER_AT_JAMES).build(); + + UnmodifiableIterator<HostAddress> dnsEntries = ImmutableList.of( + HOST_ADDRESS_1, + HOST_ADDRESS_2).iterator(); + when(dnsHelper.retrieveHostAddressIterator(MailAddressFixture.JAMES_APACHE_ORG)).thenReturn(dnsEntries); + when(mailDelivrerToHost.tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), any(HostAddress.class))) + .thenThrow(new MessagingException("400 : Horrible way to manage Server Return code")); + ExecutionResult executionResult = testee.deliver(mail); + + verify(mailDelivrerToHost, times(1)).tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), any(HostAddress.class)); + assertThat(executionResult.getExecutionState()).isEqualTo(ExecutionResult.ExecutionState.TEMPORARY_FAILURE); + } + + @SuppressWarnings("deprecation") + @Test + public void deliverShouldTryTwiceOnIOException() throws Exception { + Mail mail = FakeMail.builder().recipients(MailAddressFixture.ANY_AT_JAMES, MailAddressFixture.OTHER_AT_JAMES).build(); + + UnmodifiableIterator<HostAddress> dnsEntries = ImmutableList.of( + HOST_ADDRESS_1, + HOST_ADDRESS_2).iterator(); + when(dnsHelper.retrieveHostAddressIterator(MailAddressFixture.JAMES_APACHE_ORG)).thenReturn(dnsEntries); + when(mailDelivrerToHost.tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), eq(HOST_ADDRESS_1))) + .thenThrow(new MessagingException("400 : Horrible way to manage Server Return code", new IOException())); + when(mailDelivrerToHost.tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), eq(HOST_ADDRESS_2))) + .thenReturn(ExecutionResult.success()); + ExecutionResult executionResult = testee.deliver(mail); + + verify(mailDelivrerToHost, times(2)).tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), any(HostAddress.class)); + assertThat(executionResult.getExecutionState()).isEqualTo(ExecutionResult.ExecutionState.SUCCESS); + } + + @SuppressWarnings("deprecation") + @Test + public void deliverShouldAbortWhenServerErrorSFE() throws Exception { + Mail mail = FakeMail.builder().recipients(MailAddressFixture.ANY_AT_JAMES, MailAddressFixture.OTHER_AT_JAMES).build(); + + UnmodifiableIterator<HostAddress> dnsEntries = ImmutableList.of( + HOST_ADDRESS_1, + HOST_ADDRESS_2).iterator(); + when(dnsHelper.retrieveHostAddressIterator(MailAddressFixture.JAMES_APACHE_ORG)).thenReturn(dnsEntries); + when(mailDelivrerToHost.tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), any(HostAddress.class))) + .thenThrow(new SMTPSenderFailedException(new InternetAddress(MailAddressFixture.ANY_AT_JAMES.toString()), "command", 505, "Big failure")); + ExecutionResult executionResult = testee.deliver(mail); + + verify(mailDelivrerToHost, times(1)).tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), any(HostAddress.class)); + assertThat(executionResult.getExecutionState()).isEqualTo(ExecutionResult.ExecutionState.PERMANENT_FAILURE); + } + + @SuppressWarnings("deprecation") + @Test + public void deliverShouldAttemptDeliveryOnlyOnceIfNoMoreValidUnsent() throws Exception { + Mail mail = FakeMail.builder().recipients(MailAddressFixture.ANY_AT_JAMES, MailAddressFixture.OTHER_AT_JAMES).build(); + + UnmodifiableIterator<HostAddress> dnsEntries = ImmutableList.of( + HOST_ADDRESS_1, + HOST_ADDRESS_2).iterator(); + when(dnsHelper.retrieveHostAddressIterator(MailAddressFixture.JAMES_APACHE_ORG)).thenReturn(dnsEntries); + when(mailDelivrerToHost.tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), any(HostAddress.class))) + .thenThrow(new SendFailedException()); + ExecutionResult executionResult = testee.deliver(mail); + + verify(mailDelivrerToHost, times(1)).tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), any(HostAddress.class)); + assertThat(executionResult.getExecutionState()).isEqualTo(ExecutionResult.ExecutionState.TEMPORARY_FAILURE); + } + + @SuppressWarnings("deprecation") + @Test + public void deliverShouldAttemptDeliveryOnBothMXIfStillRecipients() 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 = {}; + SendFailedException sfe = new SendFailedException("Message", + new Exception(), + validSent, + validUnsent, + invalid); + + UnmodifiableIterator<HostAddress> dnsEntries = ImmutableList.of( + HOST_ADDRESS_1, + HOST_ADDRESS_2).iterator(); + when(dnsHelper.retrieveHostAddressIterator(MailAddressFixture.JAMES_APACHE_ORG)).thenReturn(dnsEntries); + when(mailDelivrerToHost.tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), any(HostAddress.class))) + .thenThrow(sfe); + ExecutionResult executionResult = testee.deliver(mail); + + verify(mailDelivrerToHost, times(2)).tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), any(HostAddress.class)); + assertThat(executionResult.getExecutionState()).isEqualTo(ExecutionResult.ExecutionState.TEMPORARY_FAILURE); + } + + @SuppressWarnings("deprecation") + @Test + public void deliverShouldWorkIfOnlyMX2Valid() 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 = {}; + SendFailedException sfe = new SendFailedException("Message", + new Exception(), + validSent, + validUnsent, + invalid); + + UnmodifiableIterator<HostAddress> dnsEntries = ImmutableList.of( + HOST_ADDRESS_1, + HOST_ADDRESS_2).iterator(); + when(dnsHelper.retrieveHostAddressIterator(MailAddressFixture.JAMES_APACHE_ORG)).thenReturn(dnsEntries); + when(mailDelivrerToHost.tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), eq(HOST_ADDRESS_1))) + .thenThrow(sfe); + when(mailDelivrerToHost.tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), eq(HOST_ADDRESS_2))) + .thenReturn(ExecutionResult.success()); + ExecutionResult executionResult = testee.deliver(mail); + + verify(mailDelivrerToHost, times(2)).tryDeliveryToHost(any(Mail.class), any(InternetAddress[].class), any(HostAddress.class)); + assertThat(executionResult.getExecutionState()).isEqualTo(ExecutionResult.ExecutionState.SUCCESS); + } + } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
