Skip to content

Commit

Permalink
JAMES-1692 send messages requests validation is deferred
Browse files Browse the repository at this point in the history 
... to method handler processing
setMessages response property notCreated is a map of creationId (not MessageId)
  • Loading branch information
@fvn-linagora
fvn-linagora authored and Raphael Ouazana committed Mar 1, 2016
1 parent 8983dbb commit 078ba1a
Show file tree
Hide file tree
Showing 10 changed files with 538 additions and 69 deletions.
162 changes: 158 additions & 4 deletions ...ntegration-testing/src/test/java/org/apache/james/jmap/methods/SetMessagesMethodTest.java
View file
Expand Up @@ -25,6 +25,7 @@
import static com.jayway.restassured.config.RestAssuredConfig.newConfig; import static com.jayway.restassured.config.RestAssuredConfig.newConfig;
import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.hasKey;
Expand Down Expand Up @@ -68,10 +69,10 @@ public abstract class SetMessagesMethodTest {
private static final String NAME = "[0][0]"; private static final String NAME = "[0][0]";
private static final String ARGUMENTS = "[0][1]"; private static final String ARGUMENTS = "[0][1]";


private TemporaryFolder temporaryFolder = new TemporaryFolder(); private final TemporaryFolder temporaryFolder = new TemporaryFolder();
private EmbeddedElasticSearch embeddedElasticSearch = new EmbeddedElasticSearch(); private final EmbeddedElasticSearch embeddedElasticSearch = new EmbeddedElasticSearch();
private EmbeddedCassandra cassandra = EmbeddedCassandra.createStartServer(); private final EmbeddedCassandra cassandra = EmbeddedCassandra.createStartServer();
private JmapServer jmapServer = jmapServer(temporaryFolder, embeddedElasticSearch, cassandra); private final JmapServer jmapServer = jmapServer(temporaryFolder, embeddedElasticSearch, cassandra);
private String outboxId; private String outboxId;


protected abstract JmapServer jmapServer(TemporaryFolder temporaryFolder, EmbeddedElasticSearch embeddedElasticSearch, EmbeddedCassandra cassandra); protected abstract JmapServer jmapServer(TemporaryFolder temporaryFolder, EmbeddedElasticSearch embeddedElasticSearch, EmbeddedCassandra cassandra);
Expand Down Expand Up @@ -743,4 +744,157 @@ public void setMessagesShouldCreateMessageInOutboxWhenSendingMessage() throws Ma
.body(ARGUMENTS + ".list[0].subject", equalTo(messageSubject)) .body(ARGUMENTS + ".list[0].subject", equalTo(messageSubject))
.log().ifValidationFails(); .log().ifValidationFails();
} }

@Test
public void setMessagesShouldRejectWhenSendingMessageHasNoValidAddress() {
String messageCreationId = "user|inbox|1";
String requestBody = "[" +
" [" +
" \"setMessages\","+
" {" +
" \"create\": { \"" + messageCreationId + "\" : {" +
" \"from\": { \"name\": \"MAILER-DEAMON\", \"email\": \"postmaster@example.com\"}," +
" \"to\": [{ \"name\": \"BOB\", \"email\": \"someone@example.com@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\": \"someone@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("'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 = "postmaster@example.com";
String requestBody = "[" +
" [" +
" \"setMessages\","+
" {" +
" \"create\": { \"" + messageCreationId + "\" : {" +
" \"from\": { \"email\": \"" + fromAddress + "\"}," +
" \"to\": [{ \"name\": \"BOB\", \"email\": \"someone@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 + ".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\": \"postmaster@example.com\"}," +
" \"to\": [{ \"name\": \"BOB\", \"email\": \"someone@example.com\"}]," +
" \"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));
}
} }
27 changes: 16 additions & 11 deletions server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/MIMEMessageConverter.java
View file
Expand Up @@ -19,15 +19,17 @@


package org.apache.james.jmap.methods; package org.apache.james.jmap.methods;


import static org.apache.james.jmap.model.CreationMessage.DraftEmailer;

import java.io.ByteArrayOutputStream; import java.io.ByteArrayOutputStream;
import java.io.IOException; import java.io.IOException;
import java.util.Date; import java.util.Date;
import java.util.Optional;
import java.util.TimeZone; import java.util.TimeZone;
import java.util.function.Consumer; import java.util.function.Consumer;
import java.util.stream.Collectors; import java.util.stream.Collectors;


import org.apache.james.jmap.model.CreationMessage; 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.Charsets;
import org.apache.james.mime4j.codec.DecodeMonitor; import org.apache.james.mime4j.codec.DecodeMonitor;
import org.apache.james.mime4j.dom.FieldParser; import org.apache.james.mime4j.dom.FieldParser;
Expand Down Expand Up @@ -88,27 +90,27 @@ private Header buildMimeHeaders(String creationId, CreationMessage newMessage) {
Header messageHeaders = new HeaderImpl(); Header messageHeaders = new HeaderImpl();


// add From: and Sender: headers // add From: and Sender: headers
newMessage.getFrom().map(this::convertEmailToMimeHeader) Optional<Mailbox> fromAddress = newMessage.getFrom().filter(DraftEmailer::hasValidEmail).map(this::convertEmailToMimeHeader);
.map(Fields::from) fromAddress.map(Fields::from).ifPresent(messageHeaders::addField);
.ifPresent(f -> messageHeaders.addField(f)); fromAddress.map(Fields::sender).ifPresent(messageHeaders::addField);
newMessage.getFrom().map(this::convertEmailToMimeHeader)
.map(Fields::sender)
.ifPresent(f -> messageHeaders.addField(f));


// add Reply-To: // add Reply-To:
messageHeaders.addField(Fields.replyTo(newMessage.getReplyTo().stream() messageHeaders.addField(Fields.replyTo(newMessage.getReplyTo().stream()
.map(rt -> convertEmailToMimeHeader(rt)) .map(this::convertEmailToMimeHeader)
.collect(Collectors.toList()))); .collect(Collectors.toList())));
// add To: headers // add To: headers
messageHeaders.addField(Fields.to(newMessage.getTo().stream() messageHeaders.addField(Fields.to(newMessage.getTo().stream()
.filter(DraftEmailer::hasValidEmail)
.map(this::convertEmailToMimeHeader) .map(this::convertEmailToMimeHeader)
.collect(Collectors.toList()))); .collect(Collectors.toList())));
// add Cc: headers // add Cc: headers
messageHeaders.addField(Fields.cc(newMessage.getCc().stream() messageHeaders.addField(Fields.cc(newMessage.getCc().stream()
.filter(DraftEmailer::hasValidEmail)
.map(this::convertEmailToMimeHeader) .map(this::convertEmailToMimeHeader)
.collect(Collectors.toList()))); .collect(Collectors.toList())));
// add Bcc: headers // add Bcc: headers
messageHeaders.addField(Fields.bcc(newMessage.getBcc().stream() messageHeaders.addField(Fields.bcc(newMessage.getBcc().stream()
.filter(DraftEmailer::hasValidEmail)
.map(this::convertEmailToMimeHeader) .map(this::convertEmailToMimeHeader)
.collect(Collectors.toList()))); .collect(Collectors.toList())));
// add Subject: header // add Subject: header
Expand Down Expand Up @@ -143,8 +145,11 @@ private TextBody createTextBody(CreationMessage newMessage) {
} }
} }


private Mailbox convertEmailToMimeHeader(Emailer address) { private Mailbox convertEmailToMimeHeader(DraftEmailer address) {
String[] splitAddress = address.getEmail().split("@", 2); if (!address.hasValidEmail()) {
return new Mailbox(address.getName(), null, splitAddress[0], splitAddress[1]); 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));
} }
} }
48 changes: 46 additions & 2 deletions ...tocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesCreationProcessor.java
View file
Expand Up @@ -19,9 +19,17 @@


package org.apache.james.jmap.methods; 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.Date;
import java.util.List;
import java.util.Map;
import java.util.Optional; import java.util.Optional;
import java.util.Set;
import java.util.function.Function; import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.Collectors;


import javax.inject.Inject; import javax.inject.Inject;
import javax.mail.Flags; import javax.mail.Flags;
Expand All @@ -32,6 +40,8 @@
import org.apache.james.jmap.model.CreationMessage; import org.apache.james.jmap.model.CreationMessage;
import org.apache.james.jmap.model.Message; import org.apache.james.jmap.model.Message;
import org.apache.james.jmap.model.MessageId; 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.SetMessagesRequest;
import org.apache.james.jmap.model.SetMessagesResponse; import org.apache.james.jmap.model.SetMessagesResponse;
import org.apache.james.jmap.model.mailbox.Role; import org.apache.james.jmap.model.mailbox.Role;
Expand All @@ -54,6 +64,7 @@


import com.github.fge.lambdas.functions.ThrowingFunction; import com.github.fge.lambdas.functions.ThrowingFunction;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Splitter;
import com.google.common.base.Throwables; import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap;


Expand All @@ -70,7 +81,8 @@ public class SetMessagesCreationProcessor<Id extends MailboxId> implements SetMe
@VisibleForTesting @VisibleForTesting
SetMessagesCreationProcessor(MailboxMapperFactory<Id> mailboxMapperFactory, SetMessagesCreationProcessor(MailboxMapperFactory<Id> mailboxMapperFactory,
MailboxManager mailboxManager, MailboxManager mailboxManager,
MailboxSessionMapperFactory<Id> mailboxSessionMapperFactory, MIMEMessageConverter mimeMessageConverter) { MailboxSessionMapperFactory<Id> mailboxSessionMapperFactory,
MIMEMessageConverter mimeMessageConverter) {
this.mailboxMapperFactory = mailboxMapperFactory; this.mailboxMapperFactory = mailboxMapperFactory;
this.mailboxManager = mailboxManager; this.mailboxManager = mailboxManager;
this.mailboxSessionMapperFactory = mailboxSessionMapperFactory; this.mailboxSessionMapperFactory = mailboxSessionMapperFactory;
Expand All @@ -86,11 +98,43 @@ public SetMessagesResponse process(SetMessagesRequest request, MailboxSession ma
LOGGER.error("Unable to find a mailbox with role 'outbox'!"); LOGGER.error("Unable to find a mailbox with role 'outbox'!");
throw Throwables.propagate(e); 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() return request.getCreate().entrySet().stream()
.filter(e -> validMessagesTester.test(e.getValue()))
.map(e -> new MessageWithId.CreationMessageEntry(e.getKey(), e.getValue())) .map(e -> new MessageWithId.CreationMessageEntry(e.getKey(), e.getValue()))
.map(nuMsg -> createMessageInOutbox(nuMsg, mailboxSession, outbox, buildMessageIdFunc(mailboxSession, outbox))) .map(nuMsg -> createMessageInOutbox(nuMsg, mailboxSession, outbox, buildMessageIdFunc(mailboxSession, outbox)))
.map(msg -> SetMessagesResponse.builder().created(ImmutableMap.of(msg.getCreationId(), msg.getMessage())).build()) .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(); .build();
} }


Expand Down
24 changes: 24 additions & 0 deletions server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/ValidationResult.java
View file
Expand Up @@ -19,6 +19,8 @@


package org.apache.james.jmap.methods; package org.apache.james.jmap.methods;


import java.util.Objects;

import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;


public class ValidationResult { public class ValidationResult {
Expand Down Expand Up @@ -66,4 +68,26 @@ public String getErrorMessage() {
return errorMessage; 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();
}
} }

0 comments on commit 078ba1a

Please sign in to comment.