JAMES-2214 Make calls to MailFactory less ambiguous by only requiring an Envelope and not a full JMAP Message
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/a04c071c Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/a04c071c Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/a04c071c Branch: refs/heads/master Commit: a04c071c42926fb917f7df127d8dde2b69252e5e Parents: 2b22852 Author: benwa <[email protected]> Authored: Mon Nov 13 16:48:48 2017 +0700 Committer: benwa <[email protected]> Committed: Wed Nov 15 18:05:45 2017 +0700 ---------------------------------------------------------------------- .../james/jmap/methods/MessageSender.java | 28 ++--- .../methods/SetMessagesCreationProcessor.java | 4 +- .../org/apache/james/jmap/model/Envelope.java | 116 +++++++++++++++++++ .../org/apache/james/jmap/send/MailFactory.java | 46 ++------ .../apache/james/jmap/send/MailFactoryTest.java | 7 +- 5 files changed, 142 insertions(+), 59 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/a04c071c/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MessageSender.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MessageSender.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MessageSender.java index 6a1a089..8d86355 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MessageSender.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MessageSender.java @@ -24,7 +24,7 @@ import java.io.IOException; import javax.inject.Inject; import javax.mail.MessagingException; -import org.apache.james.jmap.model.Message; +import org.apache.james.jmap.model.Envelope; import org.apache.james.jmap.model.MessageFactory; import org.apache.james.jmap.send.MailFactory; import org.apache.james.jmap.send.MailMetadata; @@ -44,39 +44,31 @@ public class MessageSender { this.mailFactory = mailFactory; } - public void sendMessage(Message jmapMessage, - MessageFactory.MetaDataWithContent message, + public void sendMessage(MessageFactory.MetaDataWithContent message, + Envelope envelope, MailboxSession session) throws MailboxException, MessagingException { - assertUserIsInSenders(jmapMessage, session); - Mail mail = buildMessage(message, jmapMessage); + assertUserIsInSenders(envelope, session); + Mail mail = buildMessage(message, envelope); try { - MailMetadata metadata = new MailMetadata(jmapMessage.getId(), session.getUser().getUserName()); + MailMetadata metadata = new MailMetadata(message.getMessageId(), session.getUser().getUserName()); mailSpool.send(mail, metadata); } finally { LifecycleUtil.dispose(mail); } } - private Mail buildMessage(MessageFactory.MetaDataWithContent message, Message jmapMessage) throws MessagingException { + private Mail buildMessage(MessageFactory.MetaDataWithContent message, Envelope envelope) throws MessagingException { try { - return mailFactory.build(message, jmapMessage); + return mailFactory.build(message, envelope); } catch (IOException e) { throw new MessagingException("error building message to send", e); } } - private void assertUserIsInSenders(Message message, MailboxSession session) throws MailboxSendingNotAllowedException { + private void assertUserIsInSenders(Envelope envelope, MailboxSession session) throws MailboxSendingNotAllowedException { String allowedSender = session.getUser().getUserName(); - if (!isAllowedFromAddress(message, allowedSender)) { + if (!session.getUser().isSameUser(envelope.getFrom().asString())) { throw new MailboxSendingNotAllowedException(allowedSender); } } - - private boolean isAllowedFromAddress(Message message, String allowedFromMailAddress) { - return message.getFrom() - .map(draftEmailer -> draftEmailer.getEmail() - .map(allowedFromMailAddress::equals) - .orElse(false)) - .orElse(false); - } } http://git-wip-us.apache.org/repos/asf/james-project/blob/a04c071c/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesCreationProcessor.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesCreationProcessor.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesCreationProcessor.java index 19dc981..4bb43dc 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesCreationProcessor.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesCreationProcessor.java @@ -36,6 +36,7 @@ import org.apache.james.jmap.exceptions.MailboxNotOwnedException; import org.apache.james.jmap.methods.ValueWithId.CreationMessageEntry; import org.apache.james.jmap.methods.ValueWithId.MessageWithId; import org.apache.james.jmap.model.CreationMessage; +import org.apache.james.jmap.model.Envelope; import org.apache.james.jmap.model.Keyword; import org.apache.james.jmap.model.Message; import org.apache.james.jmap.model.MessageFactory; @@ -235,7 +236,8 @@ public class SetMessagesCreationProcessor implements SetMessagesProcessor { MessageManager outbox = getMailboxWithRole(session, Role.OUTBOX).orElseThrow(() -> new MailboxNotFoundException(Role.OUTBOX.serialize())); MetaDataWithContent newMessage = messageAppender.appendMessageInMailbox(entry, outbox, session); Message jmapMessage = messageFactory.fromMetaDataWithContent(newMessage); - messageSender.sendMessage(jmapMessage, newMessage, session); + Envelope envelope = Envelope.fromMessage(jmapMessage); + messageSender.sendMessage(newMessage, envelope, session); return new ValueWithId.MessageWithId(entry.getCreationId(), jmapMessage); } http://git-wip-us.apache.org/repos/asf/james-project/blob/a04c071c/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/Envelope.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/Envelope.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/Envelope.java new file mode 100644 index 0000000..e823aca --- /dev/null +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/Envelope.java @@ -0,0 +1,116 @@ +/**************************************************************** + * 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.jmap.model; + +import java.util.List; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; + +import javax.mail.internet.AddressException; + +import org.apache.james.core.MailAddress; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.base.Preconditions; +import com.google.common.base.Throwables; + +public class Envelope { + private static final Logger LOGGER = LoggerFactory.getLogger(Envelope.class); + + public static Envelope fromMessage(Message jmapMessage) { + MailAddress sender = jmapMessage.getFrom() + .map(Envelope::emailerToMailAddress) + .orElseThrow(() -> new RuntimeException("Sender is mandatory")); + Set<MailAddress> to = emailersToMailAddressSet(jmapMessage.getTo()); + Set<MailAddress> cc = emailersToMailAddressSet(jmapMessage.getCc()); + Set<MailAddress> bcc = emailersToMailAddressSet(jmapMessage.getBcc()); + + return new Envelope(sender, to, cc, bcc); + } + + private static Set<MailAddress> emailersToMailAddressSet(List<Emailer> emailers) { + return emailers.stream() + .map(Envelope::emailerToMailAddress) + .collect(Collectors.toSet()); + } + + private static MailAddress emailerToMailAddress(Emailer emailer) { + Preconditions.checkArgument(emailer.getEmail().isPresent(), "eMailer mail address should be present when sending a mail using JMAP"); + try { + return new MailAddress(emailer.getEmail().get()); + } catch (AddressException e) { + LOGGER.error("Invalid mail address", emailer.getEmail()); + throw Throwables.propagate(e); + } + } + + private final MailAddress from; + private final Set<MailAddress> to; + private final Set<MailAddress> cc; + private final Set<MailAddress> bcc; + + private Envelope(MailAddress from, Set<MailAddress> to, Set<MailAddress> cc, Set<MailAddress> bcc) { + Preconditions.checkNotNull(from); + Preconditions.checkNotNull(to); + Preconditions.checkNotNull(cc); + Preconditions.checkNotNull(bcc); + + this.from = from; + this.to = to; + this.cc = cc; + this.bcc = bcc; + } + + public MailAddress getFrom() { + return from; + } + + public Set<MailAddress> getTo() { + return to; + } + + public Set<MailAddress> getCc() { + return cc; + } + + public Set<MailAddress> getBcc() { + return bcc; + } + + @Override + public final boolean equals(Object o) { + if (o instanceof Envelope) { + Envelope envelope = (Envelope) o; + + return Objects.equals(this.from, envelope.from) + && Objects.equals(this.to, envelope.to) + && Objects.equals(this.cc, envelope.cc) + && Objects.equals(this.bcc, envelope.bcc); + } + return false; + } + + @Override + public final int hashCode() { + return Objects.hash(from, to, cc, bcc); + } +} http://git-wip-us.apache.org/repos/asf/james-project/blob/a04c071c/server/protocols/jmap/src/main/java/org/apache/james/jmap/send/MailFactory.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/send/MailFactory.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/send/MailFactory.java index 4fa46c5..7db6a9b 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/send/MailFactory.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/send/MailFactory.java @@ -20,60 +20,30 @@ package org.apache.james.jmap.send; import java.io.IOException; -import java.util.List; -import java.util.Set; -import java.util.stream.Collectors; import javax.mail.MessagingException; -import javax.mail.internet.AddressException; -import org.apache.james.server.core.MailImpl; -import org.apache.james.jmap.model.Emailer; -import org.apache.james.jmap.model.Message; +import org.apache.james.core.MailAddress; +import org.apache.james.jmap.model.Envelope; import org.apache.james.jmap.model.MessageFactory.MetaDataWithContent; +import org.apache.james.server.core.MailImpl; import org.apache.mailet.Mail; -import org.apache.james.core.MailAddress; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Preconditions; -import com.google.common.base.Throwables; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; public class MailFactory { - private static final Logger LOGGER = LoggerFactory.getLogger(MailFactory.class); @VisibleForTesting MailFactory() { } - public Mail build(MetaDataWithContent message, Message jmapMessage) throws MessagingException, IOException { - MailAddress sender = jmapMessage.getFrom() - .map(this::emailerToMailAddress) - .orElseThrow(() -> new RuntimeException("Sender is mandatory")); - Set<MailAddress> to = emailersToMailAddressSet(jmapMessage.getTo()); - Set<MailAddress> cc = emailersToMailAddressSet(jmapMessage.getCc()); - Set<MailAddress> bcc = emailersToMailAddressSet(jmapMessage.getBcc()); + public Mail build(MetaDataWithContent message, Envelope envelope) throws MessagingException, IOException { ImmutableSet<MailAddress> recipients = Sets.union( - Sets.union(to, cc), - bcc).immutableCopy(); - return new MailImpl(jmapMessage.getId().serialize(), sender, recipients, message.getContent()); + Sets.union(envelope.getTo(), envelope.getCc()), + envelope.getBcc()).immutableCopy(); + return new MailImpl(message.getMessageId().serialize(), + envelope.getFrom(), recipients, message.getContent()); } - private MailAddress emailerToMailAddress(Emailer emailer) { - Preconditions.checkArgument(emailer.getEmail().isPresent(), "eMailer mail address should be present when sending a mail using JMAP"); - try { - return new MailAddress(emailer.getEmail().get()); - } catch (AddressException e) { - LOGGER.error("Invalid mail address", emailer.getEmail()); - throw Throwables.propagate(e); - } - } - - private Set<MailAddress> emailersToMailAddressSet(List<Emailer> emailers) { - return emailers.stream() - .map(this::emailerToMailAddress) - .collect(Collectors.toSet()); - } } http://git-wip-us.apache.org/repos/asf/james-project/blob/a04c071c/server/protocols/jmap/src/test/java/org/apache/james/jmap/send/MailFactoryTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/send/MailFactoryTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/send/MailFactoryTest.java index 501e9c1..7dfffc1 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/send/MailFactoryTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/send/MailFactoryTest.java @@ -29,6 +29,7 @@ import java.util.Collection; import javax.mail.util.SharedByteArrayInputStream; import org.apache.james.core.MailAddress; +import org.apache.james.jmap.model.Envelope; import org.apache.james.jmap.model.Keyword; import org.apache.james.jmap.model.Keywords; import org.apache.james.jmap.model.Message; @@ -57,6 +58,7 @@ public class MailFactoryTest { private MailFactory testee; private MetaDataWithContent message; private Message jmapMessage; + private Envelope envelope; @Before public void init() throws MailboxException { @@ -88,11 +90,12 @@ public class MailFactoryTest { when(blobManager.toBlobId(any(MessageId.class))).thenReturn(BlobId.fromString("fake")); MessageFactory messageFactory = new MessageFactory(blobManager, messagePreview, messageContentExtractor, htmlTextExtractor); jmapMessage = messageFactory.fromMetaDataWithContent(message); + envelope = Envelope.fromMessage(jmapMessage); } @Test(expected=NullPointerException.class) public void buildMailShouldThrowWhenNullMailboxMessage() throws Exception { - testee.build(null, jmapMessage); + testee.build(null, envelope); } @Test(expected=NullPointerException.class) @@ -110,7 +113,7 @@ public class MailFactoryTest { new MailAddress("[email protected]"), new MailAddress("[email protected]")); - Mail actual = testee.build(message, jmapMessage); + Mail actual = testee.build(message, envelope); assertThat(actual.getName()).isEqualTo(expectedName); assertThat(actual.getSender()).isEqualTo(expectedSender); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
