JAMES-1692 send messages requests validation is deferred ... to method handler processing setMessages response property notCreated is a map of creationId (not MessageId)
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/078ba1a6 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/078ba1a6 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/078ba1a6 Branch: refs/heads/master Commit: 078ba1a6caae80a425778b3eb305f24f0df7568d Parents: 8983dbb Author: Fabien Vignon <[email protected]> Authored: Wed Feb 24 17:43:24 2016 +0100 Committer: Raphael Ouazana <[email protected]> Committed: Tue Mar 1 15:46:13 2016 +0100 ---------------------------------------------------------------------- .../jmap/methods/SetMessagesMethodTest.java | 162 +++++++++++++- .../jmap/methods/MIMEMessageConverter.java | 27 ++- .../methods/SetMessagesCreationProcessor.java | 48 ++++- .../james/jmap/methods/ValidationResult.java | 24 +++ .../james/jmap/model/CreationMessage.java | 213 ++++++++++++++++--- .../james/jmap/model/SetMessagesResponse.java | 24 +-- .../jmap/methods/MIMEMessageConverterTest.java | 10 +- .../SetMessagesCreationProcessorTest.java | 6 +- .../james/jmap/model/CreationMessageTest.java | 87 ++++++++ .../jmap/model/SetMessagesResponseTest.java | 6 +- 10 files changed, 538 insertions(+), 69 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/078ba1a6/server/protocols/jmap-integration-testing/src/test/java/org/apache/james/jmap/methods/SetMessagesMethodTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap-integration-testing/src/test/java/org/apache/james/jmap/methods/SetMessagesMethodTest.java b/server/protocols/jmap-integration-testing/src/test/java/org/apache/james/jmap/methods/SetMessagesMethodTest.java index 3acaf8c..70dcd8f 100644 --- a/server/protocols/jmap-integration-testing/src/test/java/org/apache/james/jmap/methods/SetMessagesMethodTest.java +++ b/server/protocols/jmap-integration-testing/src/test/java/org/apache/james/jmap/methods/SetMessagesMethodTest.java @@ -25,6 +25,7 @@ import static com.jayway.restassured.config.EncoderConfig.encoderConfig; import static com.jayway.restassured.config.RestAssuredConfig.newConfig; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasKey; @@ -68,10 +69,10 @@ public abstract class SetMessagesMethodTest { private static final String NAME = "[0][0]"; private static final String ARGUMENTS = "[0][1]"; - private TemporaryFolder temporaryFolder = new TemporaryFolder(); - private EmbeddedElasticSearch embeddedElasticSearch = new EmbeddedElasticSearch(); - private EmbeddedCassandra cassandra = EmbeddedCassandra.createStartServer(); - private JmapServer jmapServer = jmapServer(temporaryFolder, embeddedElasticSearch, cassandra); + private final TemporaryFolder temporaryFolder = new TemporaryFolder(); + private final EmbeddedElasticSearch embeddedElasticSearch = new EmbeddedElasticSearch(); + private final EmbeddedCassandra cassandra = EmbeddedCassandra.createStartServer(); + private final JmapServer jmapServer = jmapServer(temporaryFolder, embeddedElasticSearch, cassandra); private String outboxId; protected abstract JmapServer jmapServer(TemporaryFolder temporaryFolder, EmbeddedElasticSearch embeddedElasticSearch, EmbeddedCassandra cassandra); @@ -743,4 +744,157 @@ public abstract class SetMessagesMethodTest { .body(ARGUMENTS + ".list[0].subject", equalTo(messageSubject)) .log().ifValidationFails(); } + + @Test + public void setMessagesShouldRejectWhenSendingMessageHasNoValidAddress() { + String messageCreationId = "user|inbox|1"; + String requestBody = "[" + + " [" + + " \"setMessages\","+ + " {" + + " \"create\": { \"" + messageCreationId + "\" : {" + + " \"from\": { \"name\": \"MAILER-DEAMON\", \"email\": \"[email protected]\"}," + + " \"to\": [{ \"name\": \"BOB\", \"email\": \"[email protected]@example.com\"}]," + + " \"cc\": [{ \"name\": \"ALICE\"}]," + + " \"subject\": \"Thank you for joining example.com!\"," + + " \"textBody\": \"Hello someone, and thank you for joining example.com!\"," + + " \"mailboxIds\": [\"" + outboxId + "\"]" + + " }}" + + " }," + + " \"#0\"" + + " ]" + + "]"; + + given() + .accept(ContentType.JSON) + .contentType(ContentType.JSON) + .header("Authorization", accessToken.serialize()) + .body(requestBody) + .when() + .post("/jmap") + .then() + .log().ifValidationFails() + .statusCode(200) + .body(NAME, equalTo("messagesSet")) + + .body(ARGUMENTS + ".notCreated", hasKey(messageCreationId)) + .body(ARGUMENTS + ".notCreated[\""+messageCreationId+"\"].type", equalTo("invalidProperties")) + .body(ARGUMENTS + ".notCreated[\""+messageCreationId+"\"].description", endsWith("no recipient address set")) + .body(ARGUMENTS + ".created", aMapWithSize(0)); + } + + @Test + public void setMessagesShouldRejectWhenSendingMessageHasMissingFrom() { + String messageCreationId = "user|inbox|1"; + String requestBody = "[" + + " [" + + " \"setMessages\","+ + " {" + + " \"create\": { \"" + messageCreationId + "\" : {" + + " \"to\": [{ \"name\": \"BOB\", \"email\": \"[email protected]\"}]," + + " \"cc\": [{ \"name\": \"ALICE\"}]," + + " \"subject\": \"Thank you for joining example.com!\"," + + " \"textBody\": \"Hello someone, and thank you for joining example.com!\"," + + " \"mailboxIds\": [\"" + outboxId + "\"]" + + " }}" + + " }," + + " \"#0\"" + + " ]" + + "]"; + + given() + .accept(ContentType.JSON) + .contentType(ContentType.JSON) + .header("Authorization", accessToken.serialize()) + .body(requestBody) + .when() + .post("/jmap") + .then() + .log().ifValidationFails() + .statusCode(200) + .body(NAME, equalTo("messagesSet")) + .body(ARGUMENTS + ".notCreated", hasKey(messageCreationId)) + .body(ARGUMENTS + ".notCreated[\""+messageCreationId+"\"].type", equalTo("invalidProperties")) + .body(ARGUMENTS + ".notCreated[\""+messageCreationId+"\"].description", endsWith("'from' address is mandatory")) + .body(ARGUMENTS + ".notCreated[\""+messageCreationId+"\"].properties", hasSize(1)) + .body(ARGUMENTS + ".notCreated[\""+messageCreationId+"\"].properties", contains("from")) + .body(ARGUMENTS + ".created", aMapWithSize(0)); + } + + @Test + public void setMessagesShouldSucceedWhenSendingMessageWithOnlyFromAddress() { + String messageCreationId = "user|inbox|1"; + String fromAddress = "[email protected]"; + String requestBody = "[" + + " [" + + " \"setMessages\","+ + " {" + + " \"create\": { \"" + messageCreationId + "\" : {" + + " \"from\": { \"email\": \"" + fromAddress + "\"}," + + " \"to\": [{ \"name\": \"BOB\", \"email\": \"[email protected]\"}]," + + " \"cc\": [{ \"name\": \"ALICE\"}]," + + " \"subject\": \"Thank you for joining example.com!\"," + + " \"textBody\": \"Hello someone, and thank you for joining example.com!\"," + + " \"mailboxIds\": [\"" + outboxId + "\"]" + + " }}" + + " }," + + " \"#0\"" + + " ]" + + "]"; + + given() + .accept(ContentType.JSON) + .contentType(ContentType.JSON) + .header("Authorization", accessToken.serialize()) + .body(requestBody) + .when() + .post("/jmap") + .then() + .log().ifValidationFails() + .statusCode(200) + .body(NAME, equalTo("messagesSet")) + .body(ARGUMENTS + ".created", aMapWithSize(1)) + .body(ARGUMENTS + ".created", hasKey(messageCreationId)) + .body(ARGUMENTS + ".created[\""+messageCreationId+"\"].headers.from", equalTo(fromAddress)) + .body(ARGUMENTS + ".created[\""+messageCreationId+"\"].from.name", equalTo(fromAddress)) + .body(ARGUMENTS + ".created[\""+messageCreationId+"\"].from.email", equalTo(fromAddress)); + } + + @Test + public void setMessagesShouldRejectWhenSendingMessageHasMissingSubject() { + String messageCreationId = "user|inbox|1"; + String requestBody = "[" + + " [" + + " \"setMessages\","+ + " {" + + " \"create\": { \"" + messageCreationId + "\" : {" + + " \"from\": { \"name\": \"MAILER-DEAMON\", \"email\": \"[email protected]\"}," + + " \"to\": [{ \"name\": \"BOB\", \"email\": \"[email protected]\"}]," + + " \"cc\": [{ \"name\": \"ALICE\"}]," + + " \"textBody\": \"Hello someone, and thank you for joining example.com!\"," + + " \"mailboxIds\": [\"" + outboxId + "\"]" + + " }}" + + " }," + + " \"#0\"" + + " ]" + + "]"; + + given() + .accept(ContentType.JSON) + .contentType(ContentType.JSON) + .header("Authorization", accessToken.serialize()) + .body(requestBody) + .when() + .post("/jmap") + .then() + .log().ifValidationFails() + .statusCode(200) + .body(NAME, equalTo("messagesSet")) + .body(ARGUMENTS + ".notCreated", hasKey(messageCreationId)) + .body(ARGUMENTS + ".notCreated[\""+messageCreationId+"\"].type", equalTo("invalidProperties")) + .body(ARGUMENTS + ".notCreated[\""+messageCreationId+"\"].properties", hasSize(1)) + .body(ARGUMENTS + ".notCreated[\""+messageCreationId+"\"].properties", contains("subject")) + .body(ARGUMENTS + ".notCreated[\""+messageCreationId+"\"].description", endsWith("'subject' is missing")) + .body(ARGUMENTS + ".created", aMapWithSize(0)); + } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/james-project/blob/078ba1a6/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MIMEMessageConverter.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MIMEMessageConverter.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MIMEMessageConverter.java index 05d0229..783ced0 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MIMEMessageConverter.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MIMEMessageConverter.java @@ -19,15 +19,17 @@ package org.apache.james.jmap.methods; +import static org.apache.james.jmap.model.CreationMessage.DraftEmailer; + import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.Date; +import java.util.Optional; import java.util.TimeZone; import java.util.function.Consumer; import java.util.stream.Collectors; import org.apache.james.jmap.model.CreationMessage; -import org.apache.james.jmap.model.Emailer; import org.apache.james.mime4j.Charsets; import org.apache.james.mime4j.codec.DecodeMonitor; import org.apache.james.mime4j.dom.FieldParser; @@ -88,27 +90,27 @@ public class MIMEMessageConverter { Header messageHeaders = new HeaderImpl(); // add From: and Sender: headers - newMessage.getFrom().map(this::convertEmailToMimeHeader) - .map(Fields::from) - .ifPresent(f -> messageHeaders.addField(f)); - newMessage.getFrom().map(this::convertEmailToMimeHeader) - .map(Fields::sender) - .ifPresent(f -> messageHeaders.addField(f)); + Optional<Mailbox> fromAddress = newMessage.getFrom().filter(DraftEmailer::hasValidEmail).map(this::convertEmailToMimeHeader); + fromAddress.map(Fields::from).ifPresent(messageHeaders::addField); + fromAddress.map(Fields::sender).ifPresent(messageHeaders::addField); // add Reply-To: messageHeaders.addField(Fields.replyTo(newMessage.getReplyTo().stream() - .map(rt -> convertEmailToMimeHeader(rt)) + .map(this::convertEmailToMimeHeader) .collect(Collectors.toList()))); // add To: headers messageHeaders.addField(Fields.to(newMessage.getTo().stream() + .filter(DraftEmailer::hasValidEmail) .map(this::convertEmailToMimeHeader) .collect(Collectors.toList()))); // add Cc: headers messageHeaders.addField(Fields.cc(newMessage.getCc().stream() + .filter(DraftEmailer::hasValidEmail) .map(this::convertEmailToMimeHeader) .collect(Collectors.toList()))); // add Bcc: headers messageHeaders.addField(Fields.bcc(newMessage.getBcc().stream() + .filter(DraftEmailer::hasValidEmail) .map(this::convertEmailToMimeHeader) .collect(Collectors.toList()))); // add Subject: header @@ -143,8 +145,11 @@ public class MIMEMessageConverter { } } - private Mailbox convertEmailToMimeHeader(Emailer address) { - String[] splitAddress = address.getEmail().split("@", 2); - return new Mailbox(address.getName(), null, splitAddress[0], splitAddress[1]); + private Mailbox convertEmailToMimeHeader(DraftEmailer address) { + if (!address.hasValidEmail()) { + throw new IllegalArgumentException("address"); + } + CreationMessage.EmailUserAndDomain emailUserAndDomain = address.getEmailUserAndDomain(); + return new Mailbox(address.getName().orElse(null), null, emailUserAndDomain.getUser().orElse(null), emailUserAndDomain.getDomain().orElse(null)); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/078ba1a6/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 3cb11db..1177e54 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 @@ -19,9 +19,17 @@ package org.apache.james.jmap.methods; +import static org.apache.james.jmap.model.MessageProperties.MessageProperty; + +import java.util.AbstractMap; import java.util.Date; +import java.util.List; +import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.function.Function; +import java.util.function.Predicate; +import java.util.stream.Collectors; import javax.inject.Inject; import javax.mail.Flags; @@ -32,6 +40,8 @@ import org.apache.james.jmap.exceptions.MailboxRoleNotFoundException; import org.apache.james.jmap.model.CreationMessage; import org.apache.james.jmap.model.Message; import org.apache.james.jmap.model.MessageId; +import org.apache.james.jmap.model.MessageProperties; +import org.apache.james.jmap.model.SetError; import org.apache.james.jmap.model.SetMessagesRequest; import org.apache.james.jmap.model.SetMessagesResponse; import org.apache.james.jmap.model.mailbox.Role; @@ -54,6 +64,7 @@ import org.slf4j.LoggerFactory; import com.github.fge.lambdas.functions.ThrowingFunction; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Splitter; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableMap; @@ -70,7 +81,8 @@ public class SetMessagesCreationProcessor<Id extends MailboxId> implements SetMe @VisibleForTesting SetMessagesCreationProcessor(MailboxMapperFactory<Id> mailboxMapperFactory, MailboxManager mailboxManager, - MailboxSessionMapperFactory<Id> mailboxSessionMapperFactory, MIMEMessageConverter mimeMessageConverter) { + MailboxSessionMapperFactory<Id> mailboxSessionMapperFactory, + MIMEMessageConverter mimeMessageConverter) { this.mailboxMapperFactory = mailboxMapperFactory; this.mailboxManager = mailboxManager; this.mailboxSessionMapperFactory = mailboxSessionMapperFactory; @@ -86,11 +98,43 @@ public class SetMessagesCreationProcessor<Id extends MailboxId> implements SetMe LOGGER.error("Unable to find a mailbox with role 'outbox'!"); throw Throwables.propagate(e); } + + // handle errors + Predicate<CreationMessage> validMessagesTester = CreationMessage::isValid; + Predicate<CreationMessage> invalidMessagesTester = validMessagesTester.negate(); + SetMessagesResponse.Builder responseBuilder = SetMessagesResponse.builder() + .notCreated(handleCreationErrors(invalidMessagesTester, request)); + return request.getCreate().entrySet().stream() + .filter(e -> validMessagesTester.test(e.getValue())) .map(e -> new MessageWithId.CreationMessageEntry(e.getKey(), e.getValue())) .map(nuMsg -> createMessageInOutbox(nuMsg, mailboxSession, outbox, buildMessageIdFunc(mailboxSession, outbox))) .map(msg -> SetMessagesResponse.builder().created(ImmutableMap.of(msg.getCreationId(), msg.getMessage())).build()) - .reduce(SetMessagesResponse.builder(), SetMessagesResponse.Builder::accumulator, SetMessagesResponse.Builder::combiner) + .reduce(responseBuilder, SetMessagesResponse.Builder::accumulator, SetMessagesResponse.Builder::combiner) + .build(); + } + + private Map<String, SetError> handleCreationErrors(Predicate<CreationMessage> invalidMessagesTester, + SetMessagesRequest request) { + return request.getCreate().entrySet().stream() + .filter(e -> invalidMessagesTester.test(e.getValue())) + .map(e -> new AbstractMap.SimpleEntry<>(e.getKey(), buildSetErrorFromValidationResult(e.getValue().validate()))) + .collect(Collectors.toMap(AbstractMap.SimpleEntry::getKey, AbstractMap.SimpleEntry::getValue)); + } + + private SetError buildSetErrorFromValidationResult(List<ValidationResult> validationErrors) { + String formattedValidationErrorMessage = validationErrors.stream() + .map(err -> err.getProperty() + ": " + err.getErrorMessage()) + .collect(Collectors.joining("\\n")); + Splitter propertiesSplitter = Splitter.on(',').trimResults().omitEmptyStrings(); + Set<MessageProperties.MessageProperty> properties = validationErrors.stream() + .flatMap(err -> propertiesSplitter.splitToList(err.getProperty()).stream()) + .flatMap(MessageProperty::find) + .collect(Collectors.toSet()); + return SetError.builder() + .type("invalidProperties") + .properties(properties) + .description(formattedValidationErrorMessage) .build(); } http://git-wip-us.apache.org/repos/asf/james-project/blob/078ba1a6/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/ValidationResult.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/ValidationResult.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/ValidationResult.java index b48c88f..9540ce6 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/ValidationResult.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/ValidationResult.java @@ -19,6 +19,8 @@ package org.apache.james.jmap.methods; +import java.util.Objects; + import com.google.common.annotations.VisibleForTesting; public class ValidationResult { @@ -66,4 +68,26 @@ public class ValidationResult { return errorMessage; } + @Override + public boolean equals(Object o) { + if (o instanceof ValidationResult) { + ValidationResult otherEMailer = (ValidationResult) o; + return Objects.equals(property, otherEMailer.property) + && Objects.equals(errorMessage, otherEMailer.errorMessage); + } + return false; + } + + @Override + public int hashCode() { + return Objects.hash(property, errorMessage); + } + + @Override + public String toString() { + return com.google.common.base.Objects.toStringHelper(getClass()) + .add("property", property) + .add("errorMessage", errorMessage) + .toString(); + } } http://git-wip-us.apache.org/repos/asf/james-project/blob/078ba1a6/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/CreationMessage.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/CreationMessage.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/CreationMessage.java index 697c9c7..4fac7e8 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/CreationMessage.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/CreationMessage.java @@ -19,15 +19,20 @@ package org.apache.james.jmap.model; +import static org.apache.james.jmap.model.MessageProperties.MessageProperty; + import java.time.ZonedDateTime; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; +import java.util.stream.Collectors; + +import javax.mail.internet.AddressException; +import javax.mail.internet.InternetAddress; -import org.apache.james.jmap.methods.GetMessagesMethod; -import org.apache.james.jmap.methods.JmapResponseWriterImpl; +import org.apache.james.jmap.methods.ValidationResult; -import com.fasterxml.jackson.annotation.JsonFilter; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder; import com.google.common.annotations.VisibleForTesting; @@ -39,6 +44,10 @@ import com.google.common.collect.ImmutableMap; @JsonDeserialize(builder = CreationMessage.Builder.class) public class CreationMessage { + private static final String RECIPIENT_PROPERTY_NAMES = ImmutableList.of(MessageProperty.to, MessageProperty.cc, MessageProperty.bcc).stream() + .map(MessageProperty::asFieldName) + .collect(Collectors.joining(", ")); + public static Builder builder() { return new Builder(); } @@ -52,11 +61,11 @@ public class CreationMessage { private boolean isAnswered; private boolean isDraft; private final ImmutableMap.Builder<String, String> headers; - private Emailer from; - private final ImmutableList.Builder<Emailer> to; - private final ImmutableList.Builder<Emailer> cc; - private final ImmutableList.Builder<Emailer> bcc; - private final ImmutableList.Builder<Emailer> replyTo; + private Optional<DraftEmailer> from = Optional.empty(); + private final ImmutableList.Builder<DraftEmailer> to; + private final ImmutableList.Builder<DraftEmailer> cc; + private final ImmutableList.Builder<DraftEmailer> bcc; + private final ImmutableList.Builder<DraftEmailer> replyTo; private String subject; private ZonedDateTime date; private String textBody; @@ -109,27 +118,27 @@ public class CreationMessage { return this; } - public Builder from(Emailer from) { - this.from = from; + public Builder from(DraftEmailer from) { + this.from = Optional.ofNullable(from); return this; } - public Builder to(List<Emailer> to) { + public Builder to(List<DraftEmailer> to) { this.to.addAll(to); return this; } - public Builder cc(List<Emailer> cc) { + public Builder cc(List<DraftEmailer> cc) { this.cc.addAll(cc); return this; } - public Builder bcc(List<Emailer> bcc) { + public Builder bcc(List<DraftEmailer> bcc) { this.bcc.addAll(bcc); return this; } - public Builder replyTo(List<Emailer> replyTo) { + public Builder replyTo(List<DraftEmailer> replyTo) { this.replyTo.addAll(replyTo); return this; } @@ -173,7 +182,6 @@ public class CreationMessage { public CreationMessage build() { Preconditions.checkState(mailboxIds != null, "'mailboxIds' is mandatory"); Preconditions.checkState(headers != null, "'headers' is mandatory"); - Preconditions.checkState(!Strings.isNullOrEmpty(subject), "'subject' is mandatory"); ImmutableList<Attachment> attachments = this.attachments.build(); ImmutableMap<String, SubMessage> attachedMessages = this.attachedMessages.build(); Preconditions.checkState(areAttachedMessagesKeysInAttachments(attachments, attachedMessages), "'attachedMessages' keys must be in 'attachments'"); @@ -182,7 +190,7 @@ public class CreationMessage { date = ZonedDateTime.now(); } - return new CreationMessage(mailboxIds, Optional.ofNullable(inReplyToMessageId), isUnread, isFlagged, isAnswered, isDraft, headers.build(), Optional.ofNullable(from), + return new CreationMessage(mailboxIds, Optional.ofNullable(inReplyToMessageId), isUnread, isFlagged, isAnswered, isDraft, headers.build(), from, to.build(), cc.build(), bcc.build(), replyTo.build(), subject, date, Optional.ofNullable(textBody), Optional.ofNullable(htmlBody), attachments, attachedMessages); } } @@ -194,11 +202,11 @@ public class CreationMessage { private final boolean isAnswered; private final boolean isDraft; private final ImmutableMap<String, String> headers; - private final Optional<Emailer> from; - private final ImmutableList<Emailer> to; - private final ImmutableList<Emailer> cc; - private final ImmutableList<Emailer> bcc; - private final ImmutableList<Emailer> replyTo; + private final Optional<DraftEmailer> from; + private final ImmutableList<DraftEmailer> to; + private final ImmutableList<DraftEmailer> cc; + private final ImmutableList<DraftEmailer> bcc; + private final ImmutableList<DraftEmailer> replyTo; private final String subject; private final ZonedDateTime date; private final Optional<String> textBody; @@ -207,8 +215,8 @@ public class CreationMessage { private final ImmutableMap<String, SubMessage> attachedMessages; @VisibleForTesting - CreationMessage(ImmutableList<String> mailboxIds, Optional<String> inReplyToMessageId, boolean isUnread, boolean isFlagged, boolean isAnswered, boolean isDraft, ImmutableMap<String, String> headers, Optional<Emailer> from, - ImmutableList<Emailer> to, ImmutableList<Emailer> cc, ImmutableList<Emailer> bcc, ImmutableList<Emailer> replyTo, String subject, ZonedDateTime date, Optional<String> textBody, Optional<String> htmlBody, ImmutableList<Attachment> attachments, + CreationMessage(ImmutableList<String> mailboxIds, Optional<String> inReplyToMessageId, boolean isUnread, boolean isFlagged, boolean isAnswered, boolean isDraft, ImmutableMap<String, String> headers, Optional<DraftEmailer> from, + ImmutableList<DraftEmailer> to, ImmutableList<DraftEmailer> cc, ImmutableList<DraftEmailer> bcc, ImmutableList<DraftEmailer> replyTo, String subject, ZonedDateTime date, Optional<String> textBody, Optional<String> htmlBody, ImmutableList<Attachment> attachments, ImmutableMap<String, SubMessage> attachedMessages) { this.mailboxIds = mailboxIds; this.inReplyToMessageId = inReplyToMessageId; @@ -258,23 +266,23 @@ public class CreationMessage { return headers; } - public Optional<Emailer> getFrom() { + public Optional<DraftEmailer> getFrom() { return from; } - public ImmutableList<Emailer> getTo() { + public ImmutableList<DraftEmailer> getTo() { return to; } - public ImmutableList<Emailer> getCc() { + public ImmutableList<DraftEmailer> getCc() { return cc; } - public ImmutableList<Emailer> getBcc() { + public ImmutableList<DraftEmailer> getBcc() { return bcc; } - public ImmutableList<Emailer> getReplyTo() { + public ImmutableList<DraftEmailer> getReplyTo() { return replyTo; } @@ -302,4 +310,153 @@ public class CreationMessage { return attachedMessages; } + public boolean isValid() { + return validate().isEmpty(); + } + + public List<ValidationResult> validate() { + ImmutableList.Builder<ValidationResult> errors = ImmutableList.builder(); + assertValidFromProvided(errors); + assertAtLeastOneValidRecipient(errors); + assertSubjectProvided(errors); + return errors.build(); + } + + private void assertSubjectProvided(ImmutableList.Builder<ValidationResult> errors) { + if (Strings.isNullOrEmpty(subject)) { + errors.add(ValidationResult.builder().message("'subject' is missing").property(MessageProperty.subject.asFieldName()).build()); + } + } + + private void assertAtLeastOneValidRecipient(ImmutableList.Builder<ValidationResult> errors) { + ImmutableList<DraftEmailer> recipients = ImmutableList.<DraftEmailer>builder().addAll(to).addAll(cc).addAll(bcc).build(); + boolean hasAtLeastOneAddressToSendTo = recipients.stream().anyMatch(DraftEmailer::hasValidEmail); + boolean recipientsHaveValidAddresses = recipients.stream().allMatch(e1 -> e1.getEmail() != null); + if (!(recipientsHaveValidAddresses && hasAtLeastOneAddressToSendTo)) { + errors.add(ValidationResult.builder().message("no recipient address set").property(RECIPIENT_PROPERTY_NAMES).build()); + } + } + + private void assertValidFromProvided(ImmutableList.Builder<ValidationResult> errors) { + ValidationResult invalidPropertyFrom = ValidationResult.builder() + .property(MessageProperty.from.asFieldName()) + .message("'from' address is mandatory") + .build(); + if (!from.isPresent()) { + errors.add(invalidPropertyFrom); + } + from.filter(f -> !f.hasValidEmail()).ifPresent(f -> errors.add(invalidPropertyFrom)); + } + + @JsonDeserialize(builder = DraftEmailer.Builder.class) + public static class DraftEmailer { + + public static Builder builder() { + return new Builder(); + } + + @JsonPOJOBuilder(withPrefix = "") + public static class Builder { + private Optional<String> name = Optional.empty(); + private Optional<String> email = Optional.empty(); + + public Builder name(String name) { + this.name = Optional.ofNullable(name); + return this; + } + + public Builder email(String email) { + this.email = Optional.ofNullable(email); + return this; + } + + public DraftEmailer build() { + return new DraftEmailer(name, email); + } + } + + private final Optional<String> name; + private final Optional<String> email; + private final EmailValidator emailValidator; + + @VisibleForTesting + DraftEmailer(Optional<String> name, Optional<String> email) { + this.name = name; + this.email = email; + this.emailValidator = new EmailValidator(); + } + + public Optional<String> getName() { + return name; + } + + public Optional<String> getEmail() { + return email; + } + + public boolean hasValidEmail() { + return getEmail().isPresent() && emailValidator.isValid(getEmail().get()); + } + + public EmailUserAndDomain getEmailUserAndDomain() { + String[] splitAddress = email.get().split("@", 2); + return new EmailUserAndDomain(Optional.ofNullable(splitAddress[0]), Optional.ofNullable(splitAddress[1])); + } + + @Override + public boolean equals(Object o) { + if (o instanceof DraftEmailer) { + DraftEmailer otherEMailer = (DraftEmailer) o; + return Objects.equals(name, otherEMailer.name) + && Objects.equals(email.orElse("<unset>"), otherEMailer.email.orElse("<unset>") ); + } + return false; + } + + @Override + public int hashCode() { + return Objects.hash(name, email); + } + + @Override + public String toString() { + return com.google.common.base.Objects.toStringHelper(this) + .add("name", name) + .add("email", email.orElse("<unset>")) + .toString(); + } + } + + public static class EmailUserAndDomain { + private final Optional<String> user; + private final Optional<String> domain; + + public EmailUserAndDomain(Optional<String> user, Optional<String> domain) { + this.user = user; + this.domain = domain; + } + + public Optional<String> getUser() { + return user; + } + + public Optional<String> getDomain() { + return domain; + } + } + + public static class EmailValidator { + + public boolean isValid(String email) { + boolean result = true; + try { + InternetAddress emailAddress = new InternetAddress(email); + // verrrry permissive validator ! + emailAddress.validate(); + } catch (AddressException ex) { + result = false; + } + return result; + } + } } http://git-wip-us.apache.org/repos/asf/james-project/blob/078ba1a6/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/SetMessagesResponse.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/SetMessagesResponse.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/SetMessagesResponse.java index c096c8a..004b11b 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/SetMessagesResponse.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/SetMessagesResponse.java @@ -20,17 +20,15 @@ package org.apache.james.jmap.model; import java.util.List; import java.util.Map; -import java.util.function.BiFunction; -import com.google.common.base.Strings; import org.apache.commons.lang.NotImplementedException; -import org.apache.james.jmap.methods.MessageWithId; import org.apache.james.jmap.methods.Method; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder; import com.fasterxml.jackson.databind.annotation.JsonSerialize; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -55,12 +53,12 @@ public class SetMessagesResponse implements Method.Response { private String accountId; private String oldState; private String newState; - private ImmutableMap.Builder<String, Message> created; - private ImmutableList.Builder<MessageId> updated; - private ImmutableList.Builder<MessageId> destroyed; - private ImmutableMap.Builder<MessageId, SetError> notCreated; - private ImmutableMap.Builder<MessageId, SetError> notUpdated; - private ImmutableMap.Builder<MessageId, SetError> notDestroyed; + private final ImmutableMap.Builder<String, Message> created; + private final ImmutableList.Builder<MessageId> updated; + private final ImmutableList.Builder<MessageId> destroyed; + private final ImmutableMap.Builder<String, SetError> notCreated; + private final ImmutableMap.Builder<MessageId, SetError> notUpdated; + private final ImmutableMap.Builder<MessageId, SetError> notDestroyed; private Builder() { created = ImmutableMap.builder(); @@ -103,7 +101,7 @@ public class SetMessagesResponse implements Method.Response { return this; } - public Builder notCreated(Map<MessageId, SetError> notCreated) { + public Builder notCreated(Map<String, SetError> notCreated) { this.notCreated.putAll(notCreated); return this; } @@ -135,12 +133,12 @@ public class SetMessagesResponse implements Method.Response { private final Map<String, Message> created; private final List<MessageId> updated; private final List<MessageId> destroyed; - private final Map<MessageId, SetError> notCreated; + private final Map<String, SetError> notCreated; private final Map<MessageId, SetError> notUpdated; private final Map<MessageId, SetError> notDestroyed; @VisibleForTesting SetMessagesResponse(String accountId, String oldState, String newState, Map<String, Message> created, List<MessageId> updated, List<MessageId> destroyed, - Map<MessageId, SetError> notCreated, Map<MessageId, SetError> notUpdated, Map<MessageId, SetError> notDestroyed) { + Map<String, SetError> notCreated, Map<MessageId, SetError> notUpdated, Map<MessageId, SetError> notDestroyed) { this.accountId = accountId; this.oldState = oldState; this.newState = newState; @@ -183,7 +181,7 @@ public class SetMessagesResponse implements Method.Response { } @JsonSerialize - public Map<MessageId, SetError> getNotCreated() { + public Map<String, SetError> getNotCreated() { return notCreated; } http://git-wip-us.apache.org/repos/asf/james-project/blob/078ba1a6/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/MIMEMessageConverterTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/MIMEMessageConverterTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/MIMEMessageConverterTest.java index af7d17a..0224f49 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/MIMEMessageConverterTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/MIMEMessageConverterTest.java @@ -19,6 +19,7 @@ package org.apache.james.jmap.methods; +import static org.apache.james.jmap.model.CreationMessage.DraftEmailer; import static org.assertj.core.api.Assertions.assertThat; import java.sql.Date; @@ -27,7 +28,6 @@ import java.time.ZoneId; import java.time.ZonedDateTime; import org.apache.james.jmap.model.CreationMessage; -import org.apache.james.jmap.model.Emailer; import org.apache.james.mime4j.dom.Message; import org.apache.james.mime4j.dom.address.Mailbox; import org.apache.james.mime4j.stream.Field; @@ -43,8 +43,9 @@ public class MIMEMessageConverterTest { String matchingMessageId = "unique-message-id"; CreationMessage messageHavingInReplyTo = CreationMessage.builder() + .from(DraftEmailer.builder().name("sender").build()) .inReplyToMessageId(matchingMessageId) - .mailboxIds(ImmutableList.of("Hey!")) + .mailboxIds(ImmutableList.of("dead-beef-1337")) .subject("subject") .build(); @@ -73,7 +74,7 @@ public class MIMEMessageConverterTest { CreationMessage testMessage = CreationMessage.builder() .mailboxIds(ImmutableList.of("deadbeef-dead-beef-dead-beef")) .subject("subject") - .from(Emailer.builder().email(joesEmail).name("joe").build()) + .from(DraftEmailer.builder().email(joesEmail).name("joe").build()) .build(); // When @@ -81,7 +82,7 @@ public class MIMEMessageConverterTest { "user|mailbox|1", testMessage)); // Then - assertThat(result.getFrom()).extracting(Mailbox::getAddress).allMatch(f -> f.toString().equals(joesEmail)); + assertThat(result.getFrom()).extracting(Mailbox::getAddress).allMatch(f -> f.equals(joesEmail)); assertThat(result.getSender().getAddress()).isEqualTo(joesEmail); } @@ -96,6 +97,7 @@ public class MIMEMessageConverterTest { CreationMessage testMessage = CreationMessage.builder() .mailboxIds(ImmutableList.of("dead-bada55")) .subject("subject") + .from(DraftEmailer.builder().name("sender").build()) .date(messageDate) .build(); http://git-wip-us.apache.org/repos/asf/james-project/blob/078ba1a6/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesCreationProcessorTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesCreationProcessorTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesCreationProcessorTest.java index 06dc7c4..c5c8db6 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesCreationProcessorTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/SetMessagesCreationProcessorTest.java @@ -19,6 +19,7 @@ package org.apache.james.jmap.methods; +import static org.apache.james.jmap.model.CreationMessage.DraftEmailer; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; @@ -32,7 +33,6 @@ import java.util.function.Function; import org.apache.james.jmap.exceptions.MailboxRoleNotFoundException; import org.apache.james.jmap.model.CreationMessage; -import org.apache.james.jmap.model.Emailer; import org.apache.james.jmap.model.Message; import org.apache.james.jmap.model.MessageId; import org.apache.james.jmap.model.SetMessagesRequest; @@ -165,8 +165,8 @@ public class SetMessagesCreationProcessorTest { private SetMessagesRequest buildFakeCreationRequest() { return SetMessagesRequest.builder() .create(ImmutableMap.of("anything-really", CreationMessage.builder() - .from(Emailer.builder().name("alice").email("[email protected]").build()) - .to(ImmutableList.of(Emailer.builder().name("bob").email("[email protected]").build())) + .from(DraftEmailer.builder().name("alice").email("[email protected]").build()) + .to(ImmutableList.of(DraftEmailer.builder().name("bob").email("[email protected]").build())) .subject("Hey! ") .mailboxIds(ImmutableList.of("mailboxId")) .build() http://git-wip-us.apache.org/repos/asf/james-project/blob/078ba1a6/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/CreationMessageTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/CreationMessageTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/CreationMessageTest.java new file mode 100644 index 0000000..48fca1c --- /dev/null +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/CreationMessageTest.java @@ -0,0 +1,87 @@ +/**************************************************************** + * 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 static org.apache.james.jmap.model.CreationMessage.DraftEmailer; +import static org.apache.james.jmap.model.MessageProperties.MessageProperty; +import static org.assertj.core.api.Assertions.assertThat; + +import org.apache.james.jmap.methods.ValidationResult; +import org.junit.Before; +import org.junit.Test; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; + +public class CreationMessageTest { + + private CreationMessage.Builder testedBuilder; + + @Before + public void setUp() { + testedBuilder = CreationMessage.builder() + .mailboxIds(ImmutableList.of("ba9-0f-dead-beef")) + .headers(ImmutableMap.of()) + .subject("anything"); + } + + @Test + public void validateShouldReturnErrorWhenFromIsMissing() { + CreationMessage sut = testedBuilder.build(); + + assertThat(sut.validate()).contains(ValidationResult.builder() + .message("'from' address is mandatory") + .property(MessageProperty.from.asFieldName()) + .build() + ); + } + + @Test + public void validateShouldReturnErrorWhenFromIsInvalid() { + CreationMessage sut = testedBuilder.from(DraftEmailer.builder().name("bob").email("[email protected]@example.com").build()).build(); + + assertThat(sut.validate()).contains(ValidationResult.builder() + .message("'from' address is mandatory") + .property(MessageProperty.from.asFieldName()) + .build() + ); + } + + @Test + public void validateShouldReturnErrorWhenNoRecipientSet () { + CreationMessage sut = testedBuilder + .from(DraftEmailer.builder().name("bob").email("[email protected]").build()).build(); + + assertThat(sut.validate()).extracting(ValidationResult::getErrorMessage).contains("no recipient address set"); + } + + @Test + public void validateShouldReturnErrorWhenNoValidRecipientSet () { + CreationMessage sut = testedBuilder + .from(DraftEmailer.builder().name("bob").email("[email protected]").build()) + .to(ImmutableList.of(DraftEmailer.builder().name("riri").email("[email protected]@example.com").build())) + .cc(ImmutableList.of(DraftEmailer.builder().name("fifi").email("[email protected]@example.com").build())) + .bcc(ImmutableList.of(DraftEmailer.builder().name("loulou").email("[email protected]@example.com").build())) + .build(); + + assertThat(sut.validate()).extracting(ValidationResult::getErrorMessage).contains("no recipient address set"); + } + +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/james-project/blob/078ba1a6/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/SetMessagesResponseTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/SetMessagesResponseTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/SetMessagesResponseTest.java index 9877651..373f1b3 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/SetMessagesResponseTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/SetMessagesResponseTest.java @@ -23,8 +23,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.time.ZonedDateTime; -import java.util.List; -import java.util.stream.Collectors; import org.apache.commons.lang.NotImplementedException; import org.junit.Test; @@ -69,7 +67,7 @@ public class SetMessagesResponseTest { .build()); ImmutableList<MessageId> updated = ImmutableList.of(MessageId.of("user|updated|1")); ImmutableList<MessageId> destroyed = ImmutableList.of(MessageId.of("user|destroyed|1")); - ImmutableMap<MessageId, SetError> notCreated = ImmutableMap.of(MessageId.of("user|created|2"), SetError.builder().type("created").build()); + ImmutableMap<String, SetError> notCreated = ImmutableMap.of("dead-beef-defec8", SetError.builder().type("created").build()); ImmutableMap<MessageId, SetError> notUpdated = ImmutableMap.of(MessageId.of("user|update|2"), SetError.builder().type("updated").build()); ImmutableMap<MessageId, SetError> notDestroyed = ImmutableMap.of(MessageId.of("user|destroyed|3"), SetError.builder().type("destroyed").build()); SetMessagesResponse expected = new SetMessagesResponse(null, null, null, created, updated, destroyed, notCreated, notUpdated, notDestroyed); @@ -94,7 +92,7 @@ public class SetMessagesResponseTest { .created(buildMessage("user|inbox|1")) .updated(ImmutableList.of(MessageId.of("user|inbox|2"))) .destroyed(ImmutableList.of(MessageId.of("user|inbox|3"))) - .notCreated(ImmutableMap.of( MessageId.of("user|inbox|4"), SetError.builder().type("type").build())) + .notCreated(ImmutableMap.of( "dead-beef-defec8", SetError.builder().type("type").build())) .notUpdated(ImmutableMap.of(MessageId.of("user|inbox|5"), SetError.builder().type("type").build())) .notDestroyed(ImmutableMap.of(MessageId.of("user|inbox|6"), SetError.builder().type("type").build())) .build(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
