JAMES-2557 Deprecate MailAddress::getSender This should rather be handled in MaybeSender
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/5b46ce3c Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/5b46ce3c Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/5b46ce3c Branch: refs/heads/master Commit: 5b46ce3c12235e6c0948779ca66cd3cb552dad99 Parents: 2fe4760 Author: Benoit Tellier <btell...@linagora.com> Authored: Thu Dec 6 11:46:57 2018 +0700 Committer: Benoit Tellier <btell...@linagora.com> Committed: Tue Dec 11 14:00:08 2018 +0700 ---------------------------------------------------------------------- .../java/org/apache/james/core/MailAddress.java | 4 ++ .../java/org/apache/james/core/MaybeSender.java | 23 ++++++++++ .../org/apache/james/core/MaybeSenderTest.java | 48 ++++++++++++++++++++ .../CassandraMailRepositoryMailDAO.java | 7 +-- .../apache/james/queue/jms/JMSMailQueue.java | 4 +- .../james/queue/rabbitmq/MailReferenceDTO.java | 3 +- 6 files changed, 84 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/5b46ce3c/core/src/main/java/org/apache/james/core/MailAddress.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/james/core/MailAddress.java b/core/src/main/java/org/apache/james/core/MailAddress.java index d31a26d..c9ab6e0 100644 --- a/core/src/main/java/org/apache/james/core/MailAddress.java +++ b/core/src/main/java/org/apache/james/core/MailAddress.java @@ -113,6 +113,10 @@ public class MailAddress implements java.io.Serializable { return NULL_SENDER; } + /** + * Prefer using {@link MaybeSender#getMailSender(String)} + */ + @Deprecated public static MailAddress getMailSender(String sender) { if (sender == null || sender.trim().length() <= 0) { return null; http://git-wip-us.apache.org/repos/asf/james-project/blob/5b46ce3c/core/src/main/java/org/apache/james/core/MaybeSender.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/james/core/MaybeSender.java b/core/src/main/java/org/apache/james/core/MaybeSender.java index 6e91fce..cdd4474 100644 --- a/core/src/main/java/org/apache/james/core/MaybeSender.java +++ b/core/src/main/java/org/apache/james/core/MaybeSender.java @@ -24,10 +24,33 @@ import java.util.Objects; import java.util.Optional; import java.util.stream.Stream; +import javax.mail.internet.AddressException; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; public class MaybeSender { + private static final Logger LOGGER = LoggerFactory.getLogger(MaybeSender.class); + + public static MaybeSender getMailSender(String sender) { + if (sender == null || sender.trim().isEmpty()) { + return MaybeSender.nullSender(); + } + if (sender.equals(MailAddress.NULL_SENDER_AS_STRING)) { + return MaybeSender.nullSender(); + } + try { + return MaybeSender.of(new MailAddress(sender)); + } catch (AddressException e) { + // Should never happen as long as the user does not modify the header by himself + LOGGER.warn("Unable to parse the sender address {}, so we fallback to a null sender", sender, e); + return MaybeSender.nullSender(); + } + } + public static MaybeSender nullSender() { return new MaybeSender(Optional.empty()); } http://git-wip-us.apache.org/repos/asf/james-project/blob/5b46ce3c/core/src/test/java/org/apache/james/core/MaybeSenderTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/james/core/MaybeSenderTest.java b/core/src/test/java/org/apache/james/core/MaybeSenderTest.java index 627aacd..6423835 100644 --- a/core/src/test/java/org/apache/james/core/MaybeSenderTest.java +++ b/core/src/test/java/org/apache/james/core/MaybeSenderTest.java @@ -32,6 +32,7 @@ import org.junit.jupiter.api.Test; import nl.jqno.equalsverifier.EqualsVerifier; class MaybeSenderTest { + private static final String GOOD_ADDRESS = "server-dev@james.apache.org"; private static final String MAIL_ADDRESS_STRING = "a...@domain.tld"; private MailAddress mailAddress; @@ -155,4 +156,51 @@ class MaybeSenderTest { .isEqualTo("default"); } + @Test + void getMailSenderShouldReturnNullSenderWhenNullSender() { + assertThat(MaybeSender.getMailSender(MailAddress.NULL_SENDER_AS_STRING)) + .isEqualTo(MaybeSender.nullSender()); + } + + @Test + void getMailSenderShouldReturnParsedAddressWhenNotNullAddress() throws Exception { + assertThat(MaybeSender.getMailSender(GOOD_ADDRESS)) + .isEqualTo(MaybeSender.of(new MailAddress(GOOD_ADDRESS))); + } + + @Test + void getMailSenderShouldReturnNullSenderWhenNull() { + assertThat(MaybeSender.getMailSender(null)) + .isEqualTo(MaybeSender.nullSender()); + } + + @Test + void getMailSenderShouldReturnNullSenderWhenEmptyString() { + assertThat(MaybeSender.getMailSender("")) + .isEqualTo(MaybeSender.nullSender()); + } + + @Test + void getMailSenderShouldReturnNullSenderWhenOnlySpaces() { + assertThat(MaybeSender.getMailSender(" ")) + .isEqualTo(MaybeSender.nullSender()); + } + + @Test + void getMailSenderShouldReturnNullSenderWhenBadValue() { + assertThat(MaybeSender.getMailSender("this@is@a@bad@address")) + .isEqualTo(MaybeSender.nullSender()); + } + + @Test + void equalsShouldReturnFalseWhenOnlyFirstMemberIsANullSender() { + assertThat(MaybeSender.getMailSender(GOOD_ADDRESS)) + .isNotEqualTo(MaybeSender.nullSender()); + } + + @Test + void equalsShouldReturnFalseWhenOnlySecondMemberIsANullSender() { + assertThat(MaybeSender.nullSender()) + .isNotEqualTo(MaybeSender.getMailSender(GOOD_ADDRESS)); + } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/james-project/blob/5b46ce3c/server/mailrepository/mailrepository-cassandra/src/main/java/org/apache/james/mailrepository/cassandra/CassandraMailRepositoryMailDAO.java ---------------------------------------------------------------------- diff --git a/server/mailrepository/mailrepository-cassandra/src/main/java/org/apache/james/mailrepository/cassandra/CassandraMailRepositoryMailDAO.java b/server/mailrepository/mailrepository-cassandra/src/main/java/org/apache/james/mailrepository/cassandra/CassandraMailRepositoryMailDAO.java index 5645a59..51bbe30 100644 --- a/server/mailrepository/mailrepository-cassandra/src/main/java/org/apache/james/mailrepository/cassandra/CassandraMailRepositoryMailDAO.java +++ b/server/mailrepository/mailrepository-cassandra/src/main/java/org/apache/james/mailrepository/cassandra/CassandraMailRepositoryMailDAO.java @@ -68,6 +68,7 @@ import org.apache.james.backends.cassandra.init.CassandraTypesProvider; import org.apache.james.backends.cassandra.utils.CassandraAsyncExecutor; import org.apache.james.blob.api.BlobId; import org.apache.james.core.MailAddress; +import org.apache.james.core.MaybeSender; import org.apache.james.mailrepository.api.MailKey; import org.apache.james.mailrepository.api.MailRepositoryUrl; import org.apache.james.server.core.MailImpl; @@ -171,9 +172,9 @@ public class CassandraMailRepositoryMailDAO { } private MailDTO toMail(Row row) { - MailAddress sender = Optional.ofNullable(row.getString(SENDER)) - .map(MailAddress::getMailSender) - .orElse(null); + MaybeSender sender = Optional.ofNullable(row.getString(SENDER)) + .map(MaybeSender::getMailSender) + .orElse(MaybeSender.nullSender()); List<MailAddress> recipients = row.getList(RECIPIENTS, String.class) .stream() .map(Throwing.function(MailAddress::new)) http://git-wip-us.apache.org/repos/asf/james-project/blob/5b46ce3c/server/queue/queue-jms/src/main/java/org/apache/james/queue/jms/JMSMailQueue.java ---------------------------------------------------------------------- diff --git a/server/queue/queue-jms/src/main/java/org/apache/james/queue/jms/JMSMailQueue.java b/server/queue/queue-jms/src/main/java/org/apache/james/queue/jms/JMSMailQueue.java index 0684d45..07b3128 100644 --- a/server/queue/queue-jms/src/main/java/org/apache/james/queue/jms/JMSMailQueue.java +++ b/server/queue/queue-jms/src/main/java/org/apache/james/queue/jms/JMSMailQueue.java @@ -51,6 +51,7 @@ import javax.mail.internet.MimeMessage; import org.apache.commons.collections.iterators.EnumerationIterator; import org.apache.james.core.MailAddress; +import org.apache.james.core.MaybeSender; import org.apache.james.lifecycle.api.Disposable; import org.apache.james.metrics.api.Gauge; import org.apache.james.metrics.api.GaugeRegistry; @@ -419,7 +420,8 @@ public class JMSMailQueue implements ManageableMailQueue, JMSSupport, MailPriori splitter.split(attributeNames) .forEach(name -> setMailAttribute(message, mail, name)); - mail.setSender(MailAddress.getMailSender(message.getStringProperty(JAMES_MAIL_SENDER))); + MaybeSender.getMailSender(message.getStringProperty(JAMES_MAIL_SENDER)) + .asOptional().ifPresent(mail::setSender); mail.setState(message.getStringProperty(JAMES_MAIL_STATE)); } http://git-wip-us.apache.org/repos/asf/james-project/blob/5b46ce3c/server/queue/queue-rabbitmq/src/main/java/org/apache/james/queue/rabbitmq/MailReferenceDTO.java ---------------------------------------------------------------------- diff --git a/server/queue/queue-rabbitmq/src/main/java/org/apache/james/queue/rabbitmq/MailReferenceDTO.java b/server/queue/queue-rabbitmq/src/main/java/org/apache/james/queue/rabbitmq/MailReferenceDTO.java index 306e7f9..80cadcb 100644 --- a/server/queue/queue-rabbitmq/src/main/java/org/apache/james/queue/rabbitmq/MailReferenceDTO.java +++ b/server/queue/queue-rabbitmq/src/main/java/org/apache/james/queue/rabbitmq/MailReferenceDTO.java @@ -34,6 +34,7 @@ import javax.mail.internet.MimeMessage; import org.apache.commons.lang3.tuple.Pair; import org.apache.james.blob.mail.MimeMessagePartsId; import org.apache.james.core.MailAddress; +import org.apache.james.core.MaybeSender; import org.apache.james.server.core.MailImpl; import org.apache.mailet.Attribute; import org.apache.mailet.AttributeName; @@ -190,7 +191,7 @@ class MailReferenceDTO { MailImpl toMailWithMimeMessage(MimeMessage mimeMessage) throws MessagingException { MailImpl mail = new MailImpl(name, - sender.map(MailAddress::getMailSender).orElse(null), + sender.map(MaybeSender::getMailSender).orElse(MaybeSender.nullSender()).asOptional().orElse(null), recipients.stream() .map(Throwing.<String, MailAddress>function(MailAddress::new).sneakyThrow()) .collect(Guavate.toImmutableList()), --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org