You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by ro...@apache.org on 2017/01/10 14:18:48 UTC
[33/50] [abbrv] james-project git commit: JAMES-1877 Refactor
decision making upon SFE exception
JAMES-1877 Refactor decision making upon SFE exception
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/1c6d1d06
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/1c6d1d06
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/1c6d1d06
Branch: refs/heads/master
Commit: 1c6d1d06c60c911a59f43df50e233e1319796587
Parents: 6a2fe8a
Author: Benoit Tellier <bt...@linagora.com>
Authored: Wed Dec 7 09:16:44 2016 +0700
Committer: Benoit Tellier <bt...@linagora.com>
Committed: Tue Jan 10 15:12:52 2017 +0700
----------------------------------------------------------------------
.../mailets/remoteDelivery/ExecutionResult.java | 16 ++
.../mailets/remoteDelivery/MailDelivrer.java | 125 ++++---------
.../mailets/remoteDelivery/SFEHelper.java | 66 +++++++
.../remoteDelivery/MailDelivrerTest.java | 183 +++++++++++++++++++
4 files changed, 297 insertions(+), 93 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/james-project/blob/1c6d1d06/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/ExecutionResult.java
----------------------------------------------------------------------
diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/ExecutionResult.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/ExecutionResult.java
index f984fd1..b408159 100644
--- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/ExecutionResult.java
+++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/ExecutionResult.java
@@ -19,6 +19,7 @@
package org.apache.james.transport.mailets.remoteDelivery;
+import com.google.common.base.Objects;
import com.google.common.base.Optional;
public class ExecutionResult {
@@ -72,5 +73,20 @@ public class ExecutionResult {
public boolean isPermanent() {
return executionState == ExecutionState.PERMANENT_FAILURE;
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (o instanceof ExecutionResult) {
+ ExecutionResult that = (ExecutionResult) o;
+ return Objects.equal(this.executionState, that.executionState)
+ && Objects.equal(this.exception, that.exception);
+ }
+ return false;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(executionState, exception);
+ }
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/1c6d1d06/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 df77eb8..c89a2a0 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
@@ -20,17 +20,15 @@
package org.apache.james.transport.mailets.remoteDelivery;
import java.io.IOException;
-import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collection;
import java.util.Iterator;
+import java.util.List;
import javax.mail.Address;
import javax.mail.MessagingException;
import javax.mail.SendFailedException;
import javax.mail.internet.InternetAddress;
import javax.mail.internet.MimeMessage;
-import javax.mail.internet.ParseException;
import org.apache.james.dnsservice.api.DNSService;
import org.apache.james.dnsservice.api.TemporaryResolutionException;
@@ -39,7 +37,7 @@ import org.apache.mailet.Mail;
import org.apache.mailet.MailAddress;
import org.slf4j.Logger;
-import com.google.common.base.Optional;
+import com.google.common.annotations.VisibleForTesting;
public class MailDelivrer {
@@ -177,105 +175,46 @@ public class MailDelivrer {
}
}
- private ExecutionResult handleSenderFailedException(Mail mail, SendFailedException sfe) {
+ @VisibleForTesting
+ ExecutionResult handleSenderFailedException(Mail mail, SendFailedException sfe) {
logSendFailedException(sfe);
-
- // Copy the recipients as direct modification may not be possible
- Collection<MailAddress> recipients = new ArrayList<MailAddress>(mail.getRecipients());
-
- ExecutionResult deleteMessage = ExecutionResult.temporaryFailure();
EnhancedMessagingException enhancedMessagingException = new EnhancedMessagingException(sfe);
-
- /*
- * If you send a message that has multiple invalid addresses, you'll
- * get a top-level SendFailedException that that has the valid,
- * valid-unsent, and invalid address lists, with all of the server
- * response messages will be contained within the nested exceptions.
- * [Note: the content of the nested exceptions is implementation
- * dependent.]
- *
- * sfe.getInvalidAddresses() should be considered permanent.
- * sfe.getValidUnsentAddresses() should be considered temporary.
- *
- * JavaMail v1.3 properly populates those collections based upon the
- * 4xx and 5xx response codes to RCPT TO. Some servers, such as
- * Yahoo! don't respond to the RCPT TO, and provide a 5xx reply
- * after DATA. In that case, we will pick up the failure from
- * SMTPSendFailedException.
- */
-
- /*
- * SMTPSendFailedException introduced in JavaMail 1.3.2, and
- * provides detailed protocol reply code for the operation
- */
- if (enhancedMessagingException.hasReturnCode() || enhancedMessagingException.hasNestedReturnCode()) {
- if (enhancedMessagingException.isServerError()) {
- deleteMessage = ExecutionResult.permanentFailure(sfe);
- } else {
- deleteMessage = ExecutionResult.temporaryFailure(sfe);
+ 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());
+ if (!invalidAddresses.isEmpty()) {
+ logger.debug("Invalid recipients: " + invalidAddresses);
+ }
+ if (!validUnsentAddresses.isEmpty()) {
+ logger.debug("Unsent recipients: " + validUnsentAddresses);
}
}
-
- // log the original set of intended recipients
- if (configuration.isDebug())
- logger.debug("Recipients: " + recipients);
-
- if (sfe.getInvalidAddresses() != null) {
- Address[] address = sfe.getInvalidAddresses();
- if (address.length > 0) {
- recipients.clear();
- for (Address addres : address) {
- try {
- recipients.add(new MailAddress(addres.toString()));
- } catch (ParseException pe) {
- // this should never happen ... we should have
- // caught malformed addresses long before we
- // got to this code.
- logger.debug("Can't parse invalid address: " + pe.getMessage());
- }
- }
- // Set the recipients for the mail
- mail.setRecipients(recipients);
-
- if (configuration.isDebug())
- logger.debug("Invalid recipients: " + recipients);
- deleteMessage = ExecutionResult.permanentFailure(sfe);
- logger.debug(messageComposer.composeFailLogMessage(mail, deleteMessage));
+ if (!validUnsentAddresses.isEmpty()) {
+ mail.setRecipients(validUnsentAddresses);
+ if (enhancedMessagingException.hasReturnCode()) {
+ boolean isPermanent = enhancedMessagingException.isServerError();
+ return logAndReturn(mail, ExecutionResult.onFailure(isPermanent, sfe));
+ } else {
+ return logAndReturn(mail, ExecutionResult.temporaryFailure(sfe));
}
}
+ if (!invalidAddresses.isEmpty()) {
+ mail.setRecipients(invalidAddresses);
+ return logAndReturn(mail, ExecutionResult.permanentFailure(sfe));
+ }
- if (sfe.getValidUnsentAddresses() != null) {
- Address[] address = sfe.getValidUnsentAddresses();
- if (address.length > 0) {
- recipients.clear();
- for (Address addres : address) {
- try {
- recipients.add(new MailAddress(addres.toString()));
- } catch (ParseException pe) {
- // this should never happen ... we should have
- // caught malformed addresses long before we
- // got to this code.
- logger.debug("Can't parse unsent address: " + pe.getMessage());
- }
- }
- // Set the recipients for the mail
- mail.setRecipients(recipients);
- if (configuration.isDebug())
- logger.debug("Unsent recipients: " + recipients);
-
- if (enhancedMessagingException.hasReturnCode()) {
- boolean isPermanent = enhancedMessagingException.isServerError();
- deleteMessage = ExecutionResult.onFailure(isPermanent, sfe);
- logger.debug(messageComposer.composeFailLogMessage(mail, deleteMessage));
- } else {
- deleteMessage = ExecutionResult.temporaryFailure(sfe);
- logger.debug(messageComposer.composeFailLogMessage(mail, deleteMessage));
- }
+ if (enhancedMessagingException.hasReturnCode() || enhancedMessagingException.hasNestedReturnCode()) {
+ if (enhancedMessagingException.isServerError()) {
+ return ExecutionResult.permanentFailure(sfe);
}
}
+ return ExecutionResult.temporaryFailure(sfe);
+ }
-
- return deleteMessage;
+ private ExecutionResult logAndReturn(Mail mail, ExecutionResult executionResult) {
+ logger.debug(messageComposer.composeFailLogMessage(mail, executionResult));
+ return executionResult;
}
private MessagingException handleSendFailException(Mail mail, SendFailedException sfe) throws SendFailedException {
http://git-wip-us.apache.org/repos/asf/james-project/blob/1c6d1d06/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/SFEHelper.java
----------------------------------------------------------------------
diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/SFEHelper.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/SFEHelper.java
new file mode 100644
index 0000000..a9650d3
--- /dev/null
+++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/SFEHelper.java
@@ -0,0 +1,66 @@
+/****************************************************************
+ * 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.remoteDelivery;
+
+import java.util.Arrays;
+import java.util.List;
+
+import javax.mail.Address;
+import javax.mail.internet.AddressException;
+
+import org.apache.mailet.MailAddress;
+import org.slf4j.Logger;
+
+import com.google.common.base.Function;
+import com.google.common.base.Optional;
+import com.google.common.base.Predicate;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableList;
+
+public class SFEHelper {
+
+ public static List<MailAddress> getAddressesAsMailAddress(Address[] addresses, final Logger logger) {
+ if (addresses == null) {
+ return ImmutableList.of();
+ }
+ return FluentIterable.from(Arrays.asList(addresses)).transform(new Function<Address, Optional<MailAddress>>() {
+ @Override
+ public Optional<MailAddress> apply(Address input) {
+ try {
+ return Optional.of(new MailAddress(input.toString()));
+ } catch (AddressException e) {
+ logger.debug("Can't parse unsent address: " + e.getMessage());
+ return Optional.absent();
+ }
+ }
+ }).filter(new Predicate<Optional<MailAddress>>() {
+ @Override
+ public boolean apply(Optional<MailAddress> input) {
+ return input.isPresent();
+ }
+ }).transform(new Function<Optional<MailAddress>, MailAddress>() {
+ @Override
+ public MailAddress apply(Optional<MailAddress> input) {
+ return input.get();
+ }
+ }).toList();
+ }
+
+}
http://git-wip-us.apache.org/repos/asf/james-project/blob/1c6d1d06/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
new file mode 100644
index 0000000..9c5fe69
--- /dev/null
+++ b/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remoteDelivery/MailDelivrerTest.java
@@ -0,0 +1,183 @@
+/****************************************************************
+ * 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.remoteDelivery;
+
+import static org.mockito.Mockito.mock;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import javax.mail.Address;
+import javax.mail.SendFailedException;
+import javax.mail.internet.InternetAddress;
+
+import org.apache.james.dnsservice.api.DNSService;
+import org.apache.mailet.Mail;
+import org.apache.mailet.base.MailAddressFixture;
+import org.apache.mailet.base.test.FakeMail;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.sun.mail.smtp.SMTPSenderFailedException;
+
+public class MailDelivrerTest {
+ private static final Logger LOGGER = LoggerFactory.getLogger(MailDelivrerTest.class);
+
+ private MailDelivrer testee;
+
+ @Before
+ public void setUp() {
+ testee = new MailDelivrer(mock(RemoteDeliveryConfiguration.class), mock(MailDelivrerToHost.class), mock(DNSService.class), LOGGER);
+ }
+
+ @Test
+ public void handleSenderFailedExceptionShouldReturnTemporaryFailureByDefault() throws Exception {
+ Mail mail = FakeMail.builder().recipients(MailAddressFixture.ANY_AT_JAMES, MailAddressFixture.OTHER_AT_JAMES).build();
+
+ SendFailedException sfe = new SendFailedException();
+ ExecutionResult executionResult = testee.handleSenderFailedException(mail, sfe);
+
+ assertThat(executionResult).isEqualTo(ExecutionResult.temporaryFailure(sfe));
+ }
+
+ @Test
+ public void handleSenderFailedExceptionShouldReturnTemporaryFailureWhenNotServerException() throws Exception {
+ Mail mail = FakeMail.builder().recipients(MailAddressFixture.ANY_AT_JAMES, MailAddressFixture.OTHER_AT_JAMES).build();
+
+ SendFailedException sfe = new SMTPSenderFailedException(new InternetAddress(MailAddressFixture.OTHER_AT_JAMES.asString()), "Comand", 400, "An temporary error");
+ ExecutionResult executionResult = testee.handleSenderFailedException(mail, sfe);
+
+ assertThat(executionResult).isEqualTo(ExecutionResult.temporaryFailure(sfe));
+ }
+
+ @Ignore("Return code is always ignored")
+ @Test
+ public void handleSenderFailedExceptionShouldReturnPermanentFailureWhenServerException() throws Exception {
+ Mail mail = FakeMail.builder().recipients(MailAddressFixture.ANY_AT_JAMES, MailAddressFixture.OTHER_AT_JAMES).build();
+
+ SendFailedException sfe = new SMTPSenderFailedException(new InternetAddress(MailAddressFixture.OTHER_AT_JAMES.asString()), "Comand", 505, "An temporary error");
+ ExecutionResult executionResult = testee.handleSenderFailedException(mail, sfe);
+
+ assertThat(executionResult).isEqualTo(ExecutionResult.permanentFailure(sfe));
+ }
+
+ @Test
+ public void handleSenderFailedExceptionShouldReturnPermanentFailureWhenInvalidAndNotValidUnsent() throws Exception {
+ Mail mail = FakeMail.builder().recipients(MailAddressFixture.ANY_AT_JAMES, MailAddressFixture.OTHER_AT_JAMES).build();
+
+ Address[] validSent = {};
+ Address[] validUnsent = {};
+ Address[] invalid = {new InternetAddress(MailAddressFixture.ANY_AT_JAMES.asString())};
+ SendFailedException sfe = new SendFailedException("Message",
+ new Exception(),
+ validSent,
+ validUnsent,
+ invalid);
+ ExecutionResult executionResult = testee.handleSenderFailedException(mail, sfe);
+
+ assertThat(executionResult).isEqualTo(ExecutionResult.permanentFailure(sfe));
+ }
+
+ @Test
+ public void handleSenderFailedExceptionShouldReturnTemporaryFailureWhenValidUnsent() 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);
+ ExecutionResult executionResult = testee.handleSenderFailedException(mail, sfe);
+
+ assertThat(executionResult).isEqualTo(ExecutionResult.temporaryFailure(sfe));
+ }
+
+ @Test
+ public void handleSenderFailedExceptionShouldReturnTemporaryFailureWhenInvalidAndValidUnsent() 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);
+ ExecutionResult executionResult = testee.handleSenderFailedException(mail, sfe);
+
+ assertThat(executionResult).isEqualTo(ExecutionResult.temporaryFailure(sfe));
+ }
+
+ @Test
+ public void handleSenderFailedExceptionShouldSetRecipientToInvalidWhenOnlyInvalid() throws Exception {
+ Mail mail = FakeMail.builder().recipients(MailAddressFixture.ANY_AT_JAMES, MailAddressFixture.OTHER_AT_JAMES).build();
+
+ Address[] validSent = {};
+ Address[] validUnsent = {};
+ Address[] invalid = {new InternetAddress(MailAddressFixture.ANY_AT_JAMES.asString())};
+ SendFailedException sfe = new SendFailedException("Message",
+ new Exception(),
+ validSent,
+ validUnsent,
+ invalid);
+ testee.handleSenderFailedException(mail, sfe);
+
+ assertThat(mail.getRecipients()).containsOnly(MailAddressFixture.ANY_AT_JAMES);
+ }
+
+ @Test
+ public void handleSenderFailedExceptionShouldSetRecipientToValidUnsentWhenOnlyValidUnsent() 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);
+ testee.handleSenderFailedException(mail, sfe);
+
+ assertThat(mail.getRecipients()).containsOnly(MailAddressFixture.OTHER_AT_JAMES);
+ }
+
+ @Test
+ public void handleSenderFailedExceptionShouldSetRecipientToValidUnsentWhenValidUnsentAndInvalid() 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);
+
+ assertThat(mail.getRecipients()).containsOnly(MailAddressFixture.OTHER_AT_JAMES);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org