JAMES-2557 Improve RRT processor code quality - Avoid passing some parameters - Remove uneeded exceptions - Avoid knowledge duplication with Stream partitioning
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/4d2f085e Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/4d2f085e Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/4d2f085e Branch: refs/heads/master Commit: 4d2f085e4c8579d08c1b7dd3fecd6a67cc4647f4 Parents: d29eec7 Author: Benoit Tellier <[email protected]> Authored: Fri Oct 26 10:54:33 2018 +0700 Committer: Benoit Tellier <[email protected]> Committed: Tue Oct 30 09:39:30 2018 +0700 ---------------------------------------------------------------------- .../mailets/RecipientRewriteTableProcessor.java | 42 ++++++++------------ .../RecipientRewriteTableProcessorTest.java | 18 ++++----- 2 files changed, 26 insertions(+), 34 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/4d2f085e/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessor.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessor.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessor.java index a986733..0cb116d 100644 --- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessor.java +++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessor.java @@ -19,12 +19,14 @@ package org.apache.james.transport.mailets; +import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.function.Function; import java.util.function.Supplier; +import java.util.stream.Collectors; import javax.mail.MessagingException; -import javax.mail.internet.MimeMessage; import org.apache.james.core.Domain; import org.apache.james.core.MailAddress; @@ -44,7 +46,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.github.fge.lambdas.Throwing; -import com.github.steveash.guavate.Guavate; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -163,52 +164,43 @@ public class RecipientRewriteTableProcessor { Mappings mappings = virtualTableStore.getMappings(recipient.getLocalPart(), recipient.getDomain()); if (mappings != null && !mappings.isEmpty()) { - List<MailAddress> newMailAddresses = handleMappings(mappings, mail, recipient, mail.getMessage()); + List<MailAddress> newMailAddresses = handleMappings(mappings, mail, recipient); return RrtExecutionResult.success(newMailAddresses); } return RrtExecutionResult.success(recipient); - } catch (ErrorMappingException | RecipientRewriteTableException | MessagingException e) { + } catch (ErrorMappingException | RecipientRewriteTableException e) { LOGGER.warn("Could not rewrite recipient {}", recipient, e); return RrtExecutionResult.error(recipient); } } @VisibleForTesting - List<MailAddress> handleMappings(Mappings mappings, Mail mail, MailAddress recipient, MimeMessage message) throws MessagingException { - ImmutableList<MailAddress> mailAddresses = mappings.asStream() + List<MailAddress> handleMappings(Mappings mappings, Mail mail, MailAddress recipient) { + boolean isLocal = true; + Map<Boolean, List<MailAddress>> mailAddressSplit = mappings.asStream() .map(mapping -> mapping.appendDomainIfNone(defaultDomainSupplier)) .map(Mapping::asMailAddress) .flatMap(OptionalUtils::toStream) - .collect(Guavate.toImmutableList()); + .collect(Collectors.partitioningBy(mailAddress -> mailetContext.isLocalServer(mailAddress.getDomain()))); - forwardToRemoteAddress(mail, recipient, message, mailAddresses); + forwardToRemoteAddress(mail, recipient, mailAddressSplit.get(!isLocal)); - return getLocalAddresses(mailAddresses); + return mailAddressSplit.get(isLocal); } - private ImmutableList<MailAddress> getLocalAddresses(ImmutableList<MailAddress> mailAddresses) { - return mailAddresses.stream() - .filter(mailAddress -> mailetContext.isLocalServer(mailAddress.getDomain())) - .collect(Guavate.toImmutableList()); - } - - private void forwardToRemoteAddress(Mail mail, MailAddress recipient, MimeMessage message, ImmutableList<MailAddress> mailAddresses) { - ImmutableList<MailAddress> remoteAddresses = mailAddresses.stream() - .filter(mailAddress -> !mailetContext.isLocalServer(mailAddress.getDomain())) - .collect(Guavate.toImmutableList()); - - if (!remoteAddresses.isEmpty()) { + private void forwardToRemoteAddress(Mail mail, MailAddress recipient, Collection<MailAddress> remoteRecipients) { + if (!remoteRecipients.isEmpty()) { try { mailetContext.sendMail( MailImpl.builder() .name(mail.getName()) .sender(mail.getMaybeSender()) - .recipients(remoteAddresses) - .mimeMessage(message) + .recipients(ImmutableList.copyOf(remoteRecipients)) + .mimeMessage(mail.getMessage()) .build()); - LOGGER.info("Mail for {} forwarded to {}", recipient, remoteAddresses); + LOGGER.info("Mail for {} forwarded to {}", recipient, remoteRecipients); } catch (MessagingException ex) { - LOGGER.warn("Error forwarding mail to {}", remoteAddresses); + LOGGER.warn("Error forwarding mail to {}", remoteRecipients); } } } http://git-wip-us.apache.org/repos/asf/james-project/blob/4d2f085e/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessorTest.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessorTest.java b/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessorTest.java index 06bbad7..789cee9 100644 --- a/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessorTest.java +++ b/server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/RecipientRewriteTableProcessorTest.java @@ -86,7 +86,7 @@ public class RecipientRewriteTableProcessorTest { .add(MailAddressFixture.OTHER_AT_JAMES.toString()) .build(); - processor.handleMappings(mappings, FakeMail.builder().sender(MailAddressFixture.ANY_AT_JAMES).build(), MailAddressFixture.OTHER_AT_JAMES, message); + processor.handleMappings(mappings, FakeMail.builder().sender(MailAddressFixture.ANY_AT_JAMES).build(), MailAddressFixture.OTHER_AT_JAMES); } @Test @@ -97,7 +97,7 @@ public class RecipientRewriteTableProcessorTest { .add(MailAddressFixture.OTHER_AT_JAMES.toString()) .build(); - processor.handleMappings(mappings, mail, MailAddressFixture.OTHER_AT_JAMES, message); + processor.handleMappings(mappings, mail, MailAddressFixture.OTHER_AT_JAMES); } @Test @@ -109,7 +109,7 @@ public class RecipientRewriteTableProcessorTest { .add(MailAddressFixture.OTHER_AT_JAMES.toString()) .build(); - Collection<MailAddress> result = processor.handleMappings(mappings, mail, MailAddressFixture.OTHER_AT_JAMES, message); + Collection<MailAddress> result = processor.handleMappings(mappings, mail, MailAddressFixture.OTHER_AT_JAMES); assertThat(result).containsOnly(nonDomainWithDefaultLocal); } @@ -125,7 +125,7 @@ public class RecipientRewriteTableProcessorTest { .add(MailAddressFixture.OTHER_AT_JAMES.toString()) .build(); - Collection<MailAddress> result = processor.handleMappings(mappings, mail, MailAddressFixture.OTHER_AT_JAMES, message); + Collection<MailAddress> result = processor.handleMappings(mappings, mail, MailAddressFixture.OTHER_AT_JAMES); assertThat(result).containsOnly(MailAddressFixture.ANY_AT_LOCAL); } @@ -140,7 +140,7 @@ public class RecipientRewriteTableProcessorTest { .add(MailAddressFixture.OTHER_AT_JAMES.toString()) .build(); - Collection<MailAddress> result = processor.handleMappings(mappings, mail, MailAddressFixture.OTHER_AT_JAMES, message); + Collection<MailAddress> result = processor.handleMappings(mappings, mail, MailAddressFixture.OTHER_AT_JAMES); assertThat(result).containsOnly(nonDomainWithDefaultLocal); } @@ -156,7 +156,7 @@ public class RecipientRewriteTableProcessorTest { .add(MailAddressFixture.OTHER_AT_JAMES.toString()) .build(); - processor.handleMappings(mappings, mail, MailAddressFixture.OTHER_AT_JAMES, message); + processor.handleMappings(mappings, mail, MailAddressFixture.OTHER_AT_JAMES); FakeMailContext.SentMail expected = FakeMailContext.sentMailBuilder() .sender(MailAddressFixture.ANY_AT_JAMES) @@ -177,7 +177,7 @@ public class RecipientRewriteTableProcessorTest { .add(INVALID_MAIL_ADDRESS) .build(); - processor.handleMappings(mappings, mail, MailAddressFixture.OTHER_AT_JAMES, message); + processor.handleMappings(mappings, mail, MailAddressFixture.OTHER_AT_JAMES); assertThat(mailetContext.getSentMails()).isEmpty(); } @@ -192,7 +192,7 @@ public class RecipientRewriteTableProcessorTest { .add(MailAddressFixture.ANY_AT_LOCAL.toString()) .build(); - Collection<MailAddress> result = processor.handleMappings(mappings, mail, MailAddressFixture.OTHER_AT_JAMES, message); + Collection<MailAddress> result = processor.handleMappings(mappings, mail, MailAddressFixture.OTHER_AT_JAMES); assertThat(result).containsOnly(nonDomainWithDefaultLocal, MailAddressFixture.ANY_AT_LOCAL); } @@ -206,7 +206,7 @@ public class RecipientRewriteTableProcessorTest { .add(MailAddressFixture.OTHER_AT_JAMES.toString()) .build(); - Collection<MailAddress> result = processor.handleMappings(mappings, mail, MailAddressFixture.OTHER_AT_JAMES, message); + Collection<MailAddress> result = processor.handleMappings(mappings, mail, MailAddressFixture.OTHER_AT_JAMES); FakeMailContext.SentMail expected = FakeMailContext.sentMailBuilder() .sender(MailAddressFixture.ANY_AT_JAMES) --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
