JAMES-1676 setMessages/update: moved json parsing introducing validator and validation results for UpdateMessagePatch and move parsing and validation logic out of SetMessageMethod SetMessagesMethod: extracted updates processing to a class (SetMessagesUpdateProcessor)
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/35b62b23 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/35b62b23 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/35b62b23 Branch: refs/heads/master Commit: 35b62b23a7ee21dc74161760a59bfbdce8593fca Parents: 41aebb7 Author: Fabien Vignon <[email protected]> Authored: Fri Feb 5 15:31:28 2016 +0100 Committer: Fabien Vignon <[email protected]> Committed: Thu Feb 11 12:37:54 2016 +0100 ---------------------------------------------------------------------- .../org/apache/james/jmap/MethodsModule.java | 2 + .../jmap/methods/SetMessagesMethodTest.java | 37 ++++- .../james/jmap/methods/SetMessagesMethod.java | 124 +------------- .../methods/SetMessagesUpdateProcessor.java | 163 +++++++++++++++++++ .../methods/UpdateMessagePatchConverter.java | 58 +++++++ .../methods/UpdateMessagePatchValidator.java | 78 +++++++++ .../james/jmap/methods/ValidationResult.java | 69 ++++++++ .../apache/james/jmap/methods/Validator.java | 27 +++ .../james/jmap/model/SetMessagesRequest.java | 22 ++- .../james/jmap/model/SetMessagesResponse.java | 19 +++ .../james/jmap/model/UpdateMessagePatch.java | 51 ++++-- .../UpdateMessagePatchValidatorTest.java | 76 +++++++++ .../jmap/model/SetMessagesResponseTest.java | 95 +++++++++++ .../jmap/model/UpdateMessagePatchTest.java | 83 ++++++++++ 14 files changed, 762 insertions(+), 142 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/35b62b23/server/container/cassandra-guice/src/main/java/org/apache/james/jmap/MethodsModule.java ---------------------------------------------------------------------- diff --git a/server/container/cassandra-guice/src/main/java/org/apache/james/jmap/MethodsModule.java b/server/container/cassandra-guice/src/main/java/org/apache/james/jmap/MethodsModule.java index 71e2dcd..27faf5a 100644 --- a/server/container/cassandra-guice/src/main/java/org/apache/james/jmap/MethodsModule.java +++ b/server/container/cassandra-guice/src/main/java/org/apache/james/jmap/MethodsModule.java @@ -29,6 +29,7 @@ import org.apache.james.jmap.methods.JmapResponseWriter; import org.apache.james.jmap.methods.JmapResponseWriterImpl; import org.apache.james.jmap.methods.Method; import org.apache.james.jmap.methods.SetMessagesMethod; +import org.apache.james.jmap.methods.SetMessagesUpdateProcessor; import org.apache.james.mailbox.cassandra.CassandraId; import com.google.inject.AbstractModule; @@ -52,6 +53,7 @@ public class MethodsModule extends AbstractModule { methods.addBinding().to(new TypeLiteral<GetMessageListMethod<CassandraId>>(){}); methods.addBinding().to(new TypeLiteral<GetMessagesMethod<CassandraId>>(){}); methods.addBinding().to(new TypeLiteral<SetMessagesMethod<CassandraId>>(){}); + bind(SetMessagesUpdateProcessor.class).to(new TypeLiteral<SetMessagesUpdateProcessor<CassandraId>>(){}); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/35b62b23/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 c584f4c..97be3d4 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 @@ -31,6 +31,7 @@ import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.isEmptyOrNullString; import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.startsWith; import static org.hamcrest.collection.IsMapWithSize.aMapWithSize; import static org.hamcrest.collection.IsMapWithSize.anEmptyMap; @@ -46,6 +47,7 @@ import org.apache.james.mailbox.elasticsearch.EmbeddedElasticSearch; import org.apache.james.mailbox.exception.MailboxException; import org.apache.james.mailbox.model.MailboxConstants; import org.apache.james.mailbox.model.MailboxPath; +import org.junit.Ignore; import com.google.common.base.Charsets; import com.jayway.restassured.RestAssured; @@ -54,7 +56,6 @@ import com.jayway.restassured.http.ContentType; import com.jayway.restassured.specification.ResponseSpecification; import org.hamcrest.Matchers; import org.junit.Before; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.RuleChain; @@ -512,7 +513,39 @@ public abstract class SetMessagesMethodTest { .body("[0][1].notUpdated", hasKey(messageId)) .body("[0][1].notUpdated[\""+messageId+"\"].type", equalTo("invalidProperties")) .body("[0][1].notUpdated[\""+messageId+"\"].properties[0]", equalTo("isUnread")) - .body("[0][1].notUpdated[\""+messageId+"\"].description", startsWith("Can not construct instance of java.lang.Boolean from String value '123': only \"true\" or \"false\" recognized")) + .body("[0][1].notUpdated[\""+messageId+"\"].description", startsWith("isUnread: Can not construct instance of java.lang.Boolean from String value '123': only \"true\" or \"false\" recognized\n" + + " at [Source: {\"isUnread\":\"123\"}; line: 1, column: 2] (through reference chain: org.apache.james.jmap.model.Builder[\"isUnread\"])")) + .body("[0][1].updated", hasSize(0)); + } + + @Test + @Ignore("Jackson json deserializer stops after first error found") + public void setMessagesShouldRejectUpdateWhenPropertiesHaveWrongTypes() throws MailboxException { + jmapServer.serverProbe().createMailbox(MailboxConstants.USER_NAMESPACE, username, "mailbox"); + jmapServer.serverProbe().appendMessage(username, new MailboxPath(MailboxConstants.USER_NAMESPACE, username, "mailbox"), + new ByteArrayInputStream("Subject: test\r\n\r\ntestmail".getBytes()), new Date(), false, new Flags()); + + embeddedElasticSearch.awaitForElasticSearch(); + + String messageId = username + "|mailbox|1"; + + given() + .accept(ContentType.JSON) + .contentType(ContentType.JSON) + .header("Authorization", accessToken.serialize()) + .body(String.format("[[\"setMessages\", {\"update\": {\"%s\" : { \"isUnread\" : \"123\", \"isFlagged\" : 456 } } }, \"#0\"]]", messageId)) + .when() + .post("/jmap") + .then() + .log().ifValidationFails() + .statusCode(200) + .body("[0][0]", equalTo("messagesSet")) + .body("[0][1].notUpdated", hasKey(messageId)) + .body("[0][1].notUpdated[\""+messageId+"\"].type", equalTo("invalidProperties")) + .body("[0][1].notUpdated[\""+messageId+"\"].properties", hasSize(2)) + .body("[0][1].notUpdated[\""+messageId+"\"].properties[0]", equalTo("isUnread")) + .body("[0][1].notUpdated[\""+messageId+"\"].properties[1]", equalTo("isFlagged")) + // .body("[0][1].notUpdated[\""+messageId+"\"].description", startsWith("Can not construct instance of java.lang.Boolean from String value '123': only \"true\" or \"false\" recognized")) .body("[0][1].updated", hasSize(0)); } http://git-wip-us.apache.org/repos/asf/james-project/blob/35b62b23/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesMethod.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesMethod.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesMethod.java index d93b65a..eeb7a65 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesMethod.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesMethod.java @@ -19,32 +19,20 @@ package org.apache.james.jmap.methods; -import java.io.IOException; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.NoSuchElementException; +import java.util.*; import java.util.function.Consumer; import java.util.stream.Stream; - import javax.inject.Inject; -import javax.mail.Flags; import org.apache.james.jmap.exceptions.MessageNotFoundException; -import org.apache.james.jmap.json.ObjectMapperFactory; import org.apache.james.jmap.model.ClientId; import org.apache.james.jmap.model.MessageId; -import org.apache.james.jmap.model.MessageProperties; -import org.apache.james.jmap.model.MessageProperties.MessageProperty; 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.UpdateMessagePatch; import org.apache.james.mailbox.MailboxSession; -import org.apache.james.mailbox.MessageManager; import org.apache.james.mailbox.exception.MailboxException; import org.apache.james.mailbox.model.MessageRange; -import org.apache.james.mailbox.store.FlagsUpdateCalculator; import org.apache.james.mailbox.store.MailboxSessionMapperFactory; import org.apache.james.mailbox.store.mail.MailboxMapperFactory; import org.apache.james.mailbox.store.mail.MessageMapper; @@ -52,19 +40,11 @@ import org.apache.james.mailbox.store.mail.MessageMapper.FetchType; import org.apache.james.mailbox.store.mail.model.Mailbox; import org.apache.james.mailbox.store.mail.model.MailboxId; import org.apache.james.mailbox.store.mail.model.MailboxMessage; -import org.apache.james.util.streams.Collectors; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import com.fasterxml.jackson.databind.JsonMappingException; -import com.fasterxml.jackson.databind.JsonMappingException.Reference; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class SetMessagesMethod<Id extends MailboxId> implements Method { @@ -75,13 +55,14 @@ public class SetMessagesMethod<Id extends MailboxId> implements Method { private final MailboxMapperFactory<Id> mailboxMapperFactory; private final MailboxSessionMapperFactory<Id> mailboxSessionMapperFactory; - private final ObjectMapper jsonParser; + private final SetMessagesUpdateProcessor<Id> messageUpdater; @Inject - @VisibleForTesting SetMessagesMethod(MailboxMapperFactory<Id> mailboxMapperFactory, MailboxSessionMapperFactory<Id> mailboxSessionMapperFactory, ObjectMapperFactory objectMapperFactory) { + @VisibleForTesting SetMessagesMethod(MailboxMapperFactory<Id> mailboxMapperFactory, + MailboxSessionMapperFactory<Id> mailboxSessionMapperFactory, SetMessagesUpdateProcessor<Id> messageUpdater) { this.mailboxMapperFactory = mailboxMapperFactory; this.mailboxSessionMapperFactory = mailboxSessionMapperFactory; - jsonParser = objectMapperFactory.forParsing(); + this.messageUpdater = messageUpdater; } @Override @@ -114,99 +95,10 @@ public class SetMessagesMethod<Id extends MailboxId> implements Method { private SetMessagesResponse setMessagesResponse(SetMessagesRequest request, MailboxSession mailboxSession) throws MailboxException { SetMessagesResponse.Builder responseBuilder = SetMessagesResponse.builder(); processDestroy(request.getDestroy(), mailboxSession, responseBuilder); - processUpdates(request.getUpdate(), mailboxSession, responseBuilder); + messageUpdater.processUpdates(request, mailboxSession).mergeInto(responseBuilder); return responseBuilder.build(); } - private void processUpdates(Map<MessageId, ObjectNode> mapOfMessagePatchesById, MailboxSession mailboxSession, - SetMessagesResponse.Builder responseBuilder) { - mapOfMessagePatchesById.entrySet().stream() - .forEach(kv -> parseAndUpdate(mailboxSession, responseBuilder, kv.getKey(), kv.getValue())); - } - - private void parseAndUpdate(MailboxSession mailboxSession, SetMessagesResponse.Builder responseBuilder, MessageId messageId, ObjectNode json) { - try { - UpdateMessagePatch updateMessagePatch = jsonParser.readValue(json.toString(), UpdateMessagePatch.class); - update(messageId, updateMessagePatch, mailboxSession, responseBuilder); - } catch (JsonMappingException e) { - handleInvalidField(messageId, responseBuilder, e); - } catch (IOException e) { - handleInvalidRequest(messageId, responseBuilder, e); - } - } - - private void update(MessageId messageId, UpdateMessagePatch updateMessagePatch, MailboxSession mailboxSession, SetMessagesResponse.Builder builder){ - try { - MessageMapper<Id> messageMapper = mailboxSessionMapperFactory.createMessageMapper(mailboxSession); - Mailbox<Id> mailbox = mailboxMapperFactory.getMailboxMapper(mailboxSession).findMailboxByPath(messageId.getMailboxPath(mailboxSession)); - Iterator<MailboxMessage<Id>> mailboxMessage = messageMapper.findInMailbox(mailbox, MessageRange.one(messageId.getUid()), FetchType.Metadata, LIMIT_BY_ONE); - MailboxMessage<Id> messageWithUpdatedFlags = applyMessagePatch(messageId, mailboxMessage.next(), updateMessagePatch, builder); - savePatchedMessage(mailbox, messageId, messageWithUpdatedFlags, messageMapper); - } catch(NoSuchElementException e) { - addMessageIdNotFoundToResponse(messageId, builder); - } catch(MailboxException e) { - handleMessageUpdateException(messageId, builder, e); - } - } - - private void handleInvalidField(MessageId messageId, SetMessagesResponse.Builder builder, JsonMappingException e) { - LOGGER.error("Invalid field in update request", e); - builder.notUpdated(ImmutableMap.of(messageId, SetError.builder() - .type("invalidProperties") - .description(e.getMessage()) - .properties(fromJsonReferences(e.getPath())) - .build())); - } - - private ImmutableSet<MessageProperty> fromJsonReferences(List<Reference> references) { - return references.stream() - .map(Reference::getFieldName) - .map(MessageProperty::valueOf) - .collect(Collectors.toImmutableSet()); - } - - private void handleInvalidRequest(MessageId messageId, SetMessagesResponse.Builder builder, IOException e) { - LOGGER.error("Invalid update request", e); - builder.notUpdated(ImmutableMap.of(messageId, SetError.builder() - .type("invalidProperties") - .description(e.getMessage()) - .build())); - } - - private void handleMessageUpdateException(MessageId messageId, SetMessagesResponse.Builder builder, MailboxException e) { - LOGGER.error("An error occurred when updating a message", e); - builder.notUpdated(ImmutableMap.of(messageId, SetError.builder() - .type("anErrorOccurred") - .description("An error occurred when updating a message") - .build())); - } - - private boolean savePatchedMessage(Mailbox<Id> mailbox, MessageId messageId, - MailboxMessage<Id> message, - MessageMapper<Id> messageMapper) throws MailboxException { - return messageMapper.updateFlags(mailbox, new FlagsUpdateCalculator(message.createFlags(), - MessageManager.FlagsUpdateMode.REPLACE), - MessageRange.one(messageId.getUid())) - .hasNext() - ; - } - - private void addMessageIdNotFoundToResponse(MessageId messageId, SetMessagesResponse.Builder builder) { - builder.notUpdated(ImmutableMap.of( messageId, - SetError.builder() - .type("notFound") - .properties(ImmutableSet.of(MessageProperties.MessageProperty.id)) - .description("message not found") - .build())); - } - - private MailboxMessage<Id> applyMessagePatch(MessageId messageId, MailboxMessage<Id> message, UpdateMessagePatch updatePatch, SetMessagesResponse.Builder builder) { - Flags newStateFlags = updatePatch.applyToState(message.isSeen(), message.isAnswered(), message.isFlagged()); - message.setFlags(newStateFlags); - builder.updated(ImmutableList.of(messageId)); - return message; - } - private void processDestroy(List<MessageId> messageIds, MailboxSession mailboxSession, SetMessagesResponse.Builder responseBuilder) throws MailboxException { MessageMapper<Id> messageMapper = mailboxSessionMapperFactory.createMessageMapper(mailboxSession); Consumer<? super MessageId> delete = delete(messageMapper, mailboxSession, responseBuilder); http://git-wip-us.apache.org/repos/asf/james-project/blob/35b62b23/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessor.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessor.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessor.java new file mode 100644 index 0000000..bec653e --- /dev/null +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMessagesUpdateProcessor.java @@ -0,0 +1,163 @@ +/**************************************************************** + * 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.methods; + +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.Set; +import java.util.stream.Collectors; +import javax.inject.Inject; +import javax.mail.Flags; + +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.UpdateMessagePatch; +import org.apache.james.mailbox.MailboxSession; +import org.apache.james.mailbox.MessageManager; +import org.apache.james.mailbox.exception.MailboxException; +import org.apache.james.mailbox.model.MessageRange; +import org.apache.james.mailbox.store.FlagsUpdateCalculator; +import org.apache.james.mailbox.store.MailboxSessionMapperFactory; +import org.apache.james.mailbox.store.mail.MailboxMapperFactory; +import org.apache.james.mailbox.store.mail.MessageMapper; +import org.apache.james.mailbox.store.mail.model.Mailbox; +import org.apache.james.mailbox.store.mail.model.MailboxId; +import org.apache.james.mailbox.store.mail.model.MailboxMessage; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class SetMessagesUpdateProcessor<Id extends MailboxId> { + + private static final int LIMIT_BY_ONE = 1; + private static final Logger LOGGER = LoggerFactory.getLogger(SetMessagesUpdateProcessor.class); + + private final UpdateMessagePatchConverter updatePatchConverter; + private final MailboxMapperFactory<Id> mailboxMapperFactory; + private final MailboxSessionMapperFactory<Id> mailboxSessionMapperFactory; + + @Inject + @VisibleForTesting SetMessagesUpdateProcessor( + UpdateMessagePatchConverter updatePatchConverter, + MailboxMapperFactory<Id> mailboxMapperFactory, + MailboxSessionMapperFactory<Id> mailboxSessionMapperFactory) { + this.updatePatchConverter = updatePatchConverter; + this.mailboxMapperFactory = mailboxMapperFactory; + this.mailboxSessionMapperFactory = mailboxSessionMapperFactory; + } + + public SetMessagesResponse processUpdates(SetMessagesRequest request, MailboxSession mailboxSession) { + SetMessagesResponse.Builder responseBuilder = SetMessagesResponse.builder(); + + List<Map.Entry<MessageId, UpdateMessagePatch>> updatePatchesById = request.buildUpdatePatchs(updatePatchConverter) + .entrySet().stream() + .collect(Collectors.toList()); + updatePatchesById.stream() + .filter(mwp -> ! mwp.getValue().isValid()) + .forEach(mwp -> handleInvalidRequest(responseBuilder, mwp.getKey(), mwp.getValue().getValidationErrors())); + updatePatchesById.stream() + .filter(mwp -> mwp.getValue().isValid()) + .forEach(e -> update(e.getKey(), e.getValue(), mailboxSession, responseBuilder)); + return responseBuilder.build(); + } + + private void update(MessageId messageId, UpdateMessagePatch updateMessagePatch, MailboxSession mailboxSession, + SetMessagesResponse.Builder builder) { + try { + MessageMapper<Id> messageMapper = mailboxSessionMapperFactory.createMessageMapper(mailboxSession); + Mailbox<Id> mailbox = mailboxMapperFactory.getMailboxMapper(mailboxSession) + .findMailboxByPath(messageId.getMailboxPath(mailboxSession)); + Iterator<MailboxMessage<Id>> mailboxMessage = messageMapper.findInMailbox( + mailbox, MessageRange.one(messageId.getUid()), MessageMapper.FetchType.Metadata, LIMIT_BY_ONE); + MailboxMessage<Id> messageWithUpdatedFlags = applyMessagePatch(messageId, mailboxMessage.next(), updateMessagePatch, builder); + savePatchedMessage(mailbox, messageId, messageWithUpdatedFlags, messageMapper); + } catch (NoSuchElementException e) { + addMessageIdNotFoundToResponse(messageId, builder); + } catch (MailboxException e) { + handleMessageUpdateException(messageId, builder, e); + } + } + + private boolean savePatchedMessage(Mailbox<Id> mailbox, MessageId messageId, + MailboxMessage<Id> message, + MessageMapper<Id> messageMapper) throws MailboxException { + return messageMapper.updateFlags(mailbox, new FlagsUpdateCalculator(message.createFlags(), + MessageManager.FlagsUpdateMode.REPLACE), + MessageRange.one(messageId.getUid())) + .hasNext() + ; + } + + private void addMessageIdNotFoundToResponse(MessageId messageId, SetMessagesResponse.Builder builder) { + builder.notUpdated(ImmutableMap.of(messageId, + SetError.builder() + .type("notFound") + .properties(ImmutableSet.of(MessageProperties.MessageProperty.id)) + .description("message not found") + .build())); + } + + private MailboxMessage<Id> applyMessagePatch(MessageId messageId, MailboxMessage<Id> message, + UpdateMessagePatch updatePatch, SetMessagesResponse.Builder builder) { + Flags newStateFlags = updatePatch.applyToState(message.isSeen(), message.isAnswered(), message.isFlagged()); + message.setFlags(newStateFlags); + builder.updated(ImmutableList.of(messageId)); + return message; + } + + private void handleMessageUpdateException(MessageId messageId, + SetMessagesResponse.Builder builder, + MailboxException e) { + LOGGER.error("An error occurred when updating a message", e); + builder.notUpdated(ImmutableMap.of(messageId, SetError.builder() + .type("anErrorOccurred") + .description("An error occurred when updating a message") + .build())); + } + + private void handleInvalidRequest(SetMessagesResponse.Builder responseBuilder, MessageId messageId, + List<ValidationResult> validationErrors) { + LOGGER.error("Invalid update request for message #", messageId.toString()); + + String formattedValidationErrorMessage = validationErrors.stream() + .map(err -> err.getProperty() + ": " + err.getErrorMessage()) + .collect(Collectors.joining(", ")); + + Set<MessageProperties.MessageProperty> properties = validationErrors.stream() + .flatMap(err -> MessageProperties.MessageProperty.find(err.getProperty())) + .collect(Collectors.toSet()); + + responseBuilder.notUpdated(ImmutableMap.of(messageId, SetError.builder() + .type("invalidProperties") + .properties(properties) + .description(formattedValidationErrorMessage) + .build())); + + } +} http://git-wip-us.apache.org/repos/asf/james-project/blob/35b62b23/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/UpdateMessagePatchConverter.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/UpdateMessagePatchConverter.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/UpdateMessagePatchConverter.java new file mode 100644 index 0000000..4e4e523 --- /dev/null +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/UpdateMessagePatchConverter.java @@ -0,0 +1,58 @@ +/**************************************************************** + * 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.methods; + +import java.io.IOException; +import javax.inject.Inject; + +import org.apache.james.jmap.model.UpdateMessagePatch; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Throwables; + +public class UpdateMessagePatchConverter { + + private final ObjectMapper jsonParser; + private final UpdateMessagePatchValidator validator; + + @Inject + @VisibleForTesting + UpdateMessagePatchConverter(ObjectMapper jsonParser, UpdateMessagePatchValidator validator) { + this.jsonParser = jsonParser; + this.validator = validator; + } + + public UpdateMessagePatch fromJsonNode(ObjectNode updatePatchNode) { + if (updatePatchNode == null || updatePatchNode.isNull() || updatePatchNode.isMissingNode()) { + throw new IllegalArgumentException("updatePatchNode"); + } + if (! validator.isValid(updatePatchNode)) { + return UpdateMessagePatch.builder() + .validationResult(validator.validate(updatePatchNode)) + .build(); + } + try { + return jsonParser.readerFor(UpdateMessagePatch.class).<UpdateMessagePatch>readValue(updatePatchNode); + } catch (IOException e) { + throw Throwables.propagate(e); + } + } +} http://git-wip-us.apache.org/repos/asf/james-project/blob/35b62b23/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/UpdateMessagePatchValidator.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/UpdateMessagePatchValidator.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/UpdateMessagePatchValidator.java new file mode 100644 index 0000000..e170d66 --- /dev/null +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/UpdateMessagePatchValidator.java @@ -0,0 +1,78 @@ +/**************************************************************** + * 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.methods; + +import java.io.IOException; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import javax.inject.Inject; + +import org.apache.james.jmap.model.MessageProperties; +import org.apache.james.jmap.model.UpdateMessagePatch; +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableSet; + +public class UpdateMessagePatchValidator implements Validator<ObjectNode> { + + private final ObjectMapper parser; + + @Inject + @VisibleForTesting UpdateMessagePatchValidator(ObjectMapper parser) { + this.parser = parser; + } + + @Override + public boolean isValid(ObjectNode patch) { + return validate(patch).isEmpty(); + } + + @Override + public Set<ValidationResult> validate(ObjectNode json) { + ImmutableSet<ValidationResult> compilation = ImmutableSet.of(); + try { + parser.readValue(json.toString(), UpdateMessagePatch.class); + } catch (JsonMappingException e) { + compilation = ImmutableSet.of(ValidationResult.builder() + .property(firstFieldFrom(e.getPath()).orElse(ValidationResult.UNDEFINED_PROPERTY)) + .message(e.getMessage()) + .build()); + } catch (IOException e) { + compilation = ImmutableSet.of(ValidationResult.builder() + .message(e.getMessage()) + .build()); + } + return compilation; + } + + private Optional<String> firstFieldFrom(List<JsonMappingException.Reference> references) { + if(references == null) { + throw new IllegalArgumentException("references"); + } + return references.stream() + .map(JsonMappingException.Reference::getFieldName) + .map(MessageProperties.MessageProperty::valueOf) + .findFirst() + .map(MessageProperties.MessageProperty::asFieldName); + } +} http://git-wip-us.apache.org/repos/asf/james-project/blob/35b62b23/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 new file mode 100644 index 0000000..b48c88f --- /dev/null +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/ValidationResult.java @@ -0,0 +1,69 @@ +/**************************************************************** + * 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.methods; + +import com.google.common.annotations.VisibleForTesting; + +public class ValidationResult { + + public static final String UNDEFINED_PROPERTY = "__UNDEFINED__"; + + public static Builder builder() { + return new Builder(); + } + + public static class Builder { + private String property; + private String errorMessage; + + public Builder property(String property) { + this.property = property; + return this; + } + + public Builder message(String message) { + this.errorMessage = message; + return this; + } + + public ValidationResult build() { + return new ValidationResult(property, errorMessage); + } + + } + + private final String property; + private final String errorMessage; + + @VisibleForTesting + ValidationResult(String property, String errorMessage) { + this.property = property; + this.errorMessage = errorMessage; + } + + public String getProperty() { + return property; + } + + public String getErrorMessage() { + return errorMessage; + } + +} http://git-wip-us.apache.org/repos/asf/james-project/blob/35b62b23/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/Validator.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/Validator.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/Validator.java new file mode 100644 index 0000000..6e2f20a --- /dev/null +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/Validator.java @@ -0,0 +1,27 @@ +/**************************************************************** + * 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.methods; + +import java.util.Set; + +public interface Validator<T> { + boolean isValid(T item); + Set<ValidationResult> validate(T item); +} http://git-wip-us.apache.org/repos/asf/james-project/blob/35b62b23/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/SetMessagesRequest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/SetMessagesRequest.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/SetMessagesRequest.java index b1f3a60..f53c918 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/SetMessagesRequest.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/SetMessagesRequest.java @@ -22,9 +22,10 @@ package org.apache.james.jmap.model; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.function.Function; -import org.apache.commons.lang.NotImplementedException; import org.apache.james.jmap.methods.JmapRequest; +import org.apache.james.jmap.methods.UpdateMessagePatchConverter; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder; @@ -32,6 +33,8 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; +import org.apache.commons.lang.NotImplementedException; @JsonDeserialize(builder = SetMessagesRequest.Builder.class) public class SetMessagesRequest implements JmapRequest { @@ -46,12 +49,13 @@ public class SetMessagesRequest implements JmapRequest { private String accountId; private String ifInState; private ImmutableList.Builder<Message> create; - private ImmutableMap.Builder<MessageId, ObjectNode> update; + private ImmutableMap.Builder<MessageId, Function<UpdateMessagePatchConverter, UpdateMessagePatch>> updatesProvider; + private ImmutableList.Builder<MessageId> destroy; private Builder() { create = ImmutableList.builder(); - update = ImmutableMap.builder(); + updatesProvider = ImmutableMap.builder(); destroy = ImmutableList.builder(); } @@ -77,7 +81,7 @@ public class SetMessagesRequest implements JmapRequest { } public Builder update(Map<MessageId, ObjectNode> updates) { - this.update.putAll(updates); + this.updatesProvider.putAll(Maps.transformValues(updates, json -> converter -> converter.fromJsonNode(json))); return this; } @@ -87,17 +91,17 @@ public class SetMessagesRequest implements JmapRequest { } public SetMessagesRequest build() { - return new SetMessagesRequest(Optional.ofNullable(accountId), Optional.ofNullable(ifInState), create.build(), update.build(), destroy.build()); + return new SetMessagesRequest(Optional.ofNullable(accountId), Optional.ofNullable(ifInState), create.build(), updatesProvider.build(), destroy.build()); } } private final Optional<String> accountId; private final Optional<String> ifInState; private final List<Message> create; - private final Map<MessageId, ObjectNode> update; + private final Map<MessageId, Function<UpdateMessagePatchConverter, UpdateMessagePatch>> update; private final List<MessageId> destroy; - @VisibleForTesting SetMessagesRequest(Optional<String> accountId, Optional<String> ifInState, List<Message> create, Map<MessageId, ObjectNode> update, List<MessageId> destroy) { + @VisibleForTesting SetMessagesRequest(Optional<String> accountId, Optional<String> ifInState, List<Message> create, Map<MessageId, Function<UpdateMessagePatchConverter, UpdateMessagePatch>> update, List<MessageId> destroy) { this.accountId = accountId; this.ifInState = ifInState; this.create = create; @@ -117,8 +121,8 @@ public class SetMessagesRequest implements JmapRequest { return create; } - public Map<MessageId, ObjectNode> getUpdate() { - return update; + public Map<MessageId, UpdateMessagePatch> buildUpdatePatchs(UpdateMessagePatchConverter converter) { + return Maps.transformValues(update, func -> func.apply(converter)); } public List<MessageId> getDestroy() { http://git-wip-us.apache.org/repos/asf/james-project/blob/35b62b23/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 67bdd46..304f8c3 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 @@ -21,6 +21,7 @@ package org.apache.james.jmap.model; import java.util.List; import java.util.Map; +import com.google.common.base.Strings; import org.apache.commons.lang.NotImplementedException; import org.apache.james.jmap.methods.Method; @@ -185,4 +186,22 @@ public class SetMessagesResponse implements Method.Response { public Map<MessageId, SetError> getNotDestroyed() { return notDestroyed; } + + public void mergeInto(SetMessagesResponse.Builder responseBuilder) { + responseBuilder.created(getCreated()); + responseBuilder.updated(getUpdated()); + responseBuilder.destroyed(getDestroyed()); + responseBuilder.notCreated(getNotCreated()); + responseBuilder.notUpdated(getNotUpdated()); + responseBuilder.notDestroyed(getNotDestroyed()); + if(! Strings.isNullOrEmpty(getAccountId())) { + responseBuilder.accountId(getAccountId()); + } + if(! Strings.isNullOrEmpty(getOldState())) { + responseBuilder.accountId(getOldState()); + } + if(! Strings.isNullOrEmpty(getNewState())) { + responseBuilder.accountId(getAccountId()); + } + } } http://git-wip-us.apache.org/repos/asf/james-project/blob/35b62b23/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/UpdateMessagePatch.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/UpdateMessagePatch.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/UpdateMessagePatch.java index 3751368..bcbbf9e 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/UpdateMessagePatch.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/UpdateMessagePatch.java @@ -21,10 +21,13 @@ package org.apache.james.jmap.model; import java.util.List; import java.util.Optional; +import java.util.Set; import javax.mail.Flags; +import com.google.common.collect.ImmutableSet; import org.apache.commons.lang.NotImplementedException; +import org.apache.james.jmap.methods.ValidationResult; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder; import com.google.common.annotations.VisibleForTesting; @@ -43,6 +46,7 @@ public class UpdateMessagePatch { private Optional<Boolean> isFlagged = Optional.empty(); private Optional<Boolean> isUnread = Optional.empty(); private Optional<Boolean> isAnswered = Optional.empty(); + private Set<ValidationResult> validationResult = ImmutableSet.of(); public Builder mailboxIds(Optional<List<String>> mailboxIds) { if (mailboxIds.isPresent()) { @@ -51,24 +55,28 @@ public class UpdateMessagePatch { return this; } - public Builder isFlagged(Optional<Boolean> isFlagged) { - this.isFlagged = isFlagged; + public Builder isFlagged(Boolean isFlagged) { + this.isFlagged = Optional.of(isFlagged); return this; } - public Builder isUnread(Optional<Boolean> isUnread) { - this.isUnread = isUnread; + public Builder isUnread(Boolean isUnread) { + this.isUnread = Optional.of(isUnread); return this; } - public Builder isAnswered(Optional<Boolean> isAnswered) { - this.isAnswered = isAnswered; + public Builder isAnswered(Boolean isAnswered) { + this.isAnswered = Optional.of(isAnswered); return this; } - public UpdateMessagePatch build() { + public Builder validationResult(Set<ValidationResult> validationResult) { + this.validationResult = ImmutableSet.copyOf(validationResult); + return this; + } - return new UpdateMessagePatch(mailboxIds.build(), isUnread, isFlagged, isAnswered); + public UpdateMessagePatch build() { + return new UpdateMessagePatch(mailboxIds.build(), isUnread, isFlagged, isAnswered, ImmutableList.copyOf(validationResult)); } } @@ -77,16 +85,20 @@ public class UpdateMessagePatch { private final Optional<Boolean> isFlagged; private final Optional<Boolean> isAnswered; + private final ImmutableList<ValidationResult> validationErrors; + @VisibleForTesting UpdateMessagePatch(List<String> mailboxIds, Optional<Boolean> isUnread, Optional<Boolean> isFlagged, - Optional<Boolean> isAnswered) { + Optional<Boolean> isAnswered, + ImmutableList<ValidationResult> validationResults) { this.mailboxIds = mailboxIds; this.isUnread = isUnread; this.isFlagged = isFlagged; this.isAnswered = isAnswered; + this.validationErrors = validationResults; } public List<String> getMailboxIds() { @@ -105,21 +117,30 @@ public class UpdateMessagePatch { return isAnswered; } + public ImmutableList<ValidationResult> getValidationErrors() { + return validationErrors; + } + public boolean isValid() { - return true; // to be implemented when UpdateMessagePatch would allow any message property to be set + return getValidationErrors().isEmpty(); } public Flags applyToState(boolean isSeen, boolean isAnswered, boolean isFlagged) { Flags newStateFlags = new Flags(); - if (!isSeen && isUnread().isPresent() && !isUnread().get()) { - newStateFlags.add(Flags.Flag.SEEN); + + boolean shouldMessageBeFlagged = isFlagged().isPresent() && isFlagged().get() || (!isFlagged().isPresent() && isFlagged); + if (shouldMessageBeFlagged) { + newStateFlags.add(Flags.Flag.FLAGGED); } - if (!isAnswered && isAnswered().isPresent() && isAnswered().get()) { + boolean shouldMessageBeMarkAnswered = isAnswered().isPresent() && isAnswered().get() || (!isAnswered().isPresent() && isAnswered); + if (shouldMessageBeMarkAnswered) { newStateFlags.add(Flags.Flag.ANSWERED); } - if (!isFlagged && isFlagged().isPresent() && isFlagged().get()) { - newStateFlags.add(Flags.Flag.FLAGGED); + boolean shouldMessageBeMarkSeen = isUnread().isPresent() && !isUnread().get() || (!isUnread().isPresent() && isSeen); + if (shouldMessageBeMarkSeen) { + newStateFlags.add(Flags.Flag.SEEN); } return newStateFlags; } + } http://git-wip-us.apache.org/repos/asf/james-project/blob/35b62b23/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/UpdateMessagePatchValidatorTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/UpdateMessagePatchValidatorTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/UpdateMessagePatchValidatorTest.java new file mode 100644 index 0000000..bbeb955 --- /dev/null +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/UpdateMessagePatchValidatorTest.java @@ -0,0 +1,76 @@ +/**************************************************************** + * 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.methods; + +import static org.assertj.core.api.Assertions.assertThat;; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.util.Set; + +import org.apache.james.jmap.model.UpdateMessagePatch; +import org.junit.Test; +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; + +public class UpdateMessagePatchValidatorTest { + + @Test + public void validateShouldReturnPropertyNameWhenPropertyHasInvalidType() throws IOException { + ObjectMapper mapper = new ObjectMapper(); + + String jsonContent = "{ \"isUnread\" : \"123\" }"; + ObjectNode rootNode = mapper.readValue(jsonContent, ObjectNode.class); + + UpdateMessagePatchValidator sut = new UpdateMessagePatchValidator(mapper); + Set<ValidationResult> result = sut.validate(rootNode); + assertThat(result).extracting(ValidationResult::getProperty).contains("isUnread"); + } + + @Test + public void isValidShouldReturnTrueWhenPatchWellFormatted() throws IOException { + ObjectMapper mapper = new ObjectMapper(); + + String jsonContent = "{ \"isUnread\" : \"true\" }"; + ObjectNode rootNode = mapper.readValue(jsonContent, ObjectNode.class); + + UpdateMessagePatchValidator sut = new UpdateMessagePatchValidator(mapper); + assertThat(sut.isValid(rootNode)).isTrue(); + } + + @Test + public void validateShouldReturnANonEmptyResultWhenParsingThrows() throws IOException { + // Given + ObjectNode emptyRootNode = new ObjectMapper().createObjectNode(); + ObjectMapper mapper = mock(ObjectMapper.class); + when(mapper.readValue(anyString(), eq(UpdateMessagePatch.class))) + .thenThrow(JsonMappingException.class); + UpdateMessagePatchValidator sut = new UpdateMessagePatchValidator(mapper); + // When + Set<ValidationResult> result = sut.validate(emptyRootNode); + // Then + assertThat(result).isNotEmpty(); + assertThat(result).extracting(ValidationResult::getProperty).contains(ValidationResult.UNDEFINED_PROPERTY); + } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/james-project/blob/35b62b23/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 23743f3..2c02bac 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,13 @@ 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 java.util.stream.Stream; +import com.google.common.collect.ImmutableSet; import org.apache.commons.lang.NotImplementedException; +import org.junit.Ignore; import org.junit.Test; import com.google.common.collect.ImmutableList; @@ -83,4 +88,94 @@ public class SetMessagesResponseTest { assertThat(setMessagesResponse).isEqualToComparingFieldByField(expected); } + + @Test + public void mergeIntoShouldCopyItemsWhenBuilderIsEmpty() { + // Given + SetMessagesResponse.Builder emptyBuilder = SetMessagesResponse.builder(); + SetMessagesResponse testee = SetMessagesResponse.builder() + .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())) + .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(); + + // When + testee.mergeInto(emptyBuilder); + // Then + assertThat(emptyBuilder.build()).isEqualToComparingFieldByField(testee); + } + + private ImmutableList<Message> buildMessage(String messageId) { + return ImmutableList.of(Message.builder() + .id(MessageId.of(messageId)) + .blobId("blobId") + .threadId("threadId") + .mailboxIds(ImmutableList.of()) + .headers(ImmutableMap.of()) + .subject("subject") + .size(0) + .date(ZonedDateTime.now()) + .preview("preview") + .build()); + } + + @Test + public void mergeIntoShouldMergeUpdatedLists() { + // Given + MessageId buildersUpdatedMessageId = MessageId.of("user|inbox|1"); + SetMessagesResponse.Builder nonEmptyBuilder = SetMessagesResponse.builder() + .updated(ImmutableList.of(buildersUpdatedMessageId)); + MessageId updatedMessageId = MessageId.of("user|inbox|2"); + SetMessagesResponse testee = SetMessagesResponse.builder() + .updated(ImmutableList.of(updatedMessageId)) + .build(); + // When + testee.mergeInto(nonEmptyBuilder); + SetMessagesResponse mergedResponse = nonEmptyBuilder.build(); + + // Then + assertThat(mergedResponse.getUpdated()).containsExactly(buildersUpdatedMessageId, updatedMessageId); + } + + @Test + public void mergeIntoShouldMergeCreatedLists() { + // Given + String buildersCreatedMessageId = "user|inbox|1"; + SetMessagesResponse.Builder nonEmptyBuilder = SetMessagesResponse.builder() + .created(buildMessage(buildersCreatedMessageId)); + String createdMessageId = "user|inbox|2"; + SetMessagesResponse testee = SetMessagesResponse.builder() + .created(buildMessage(createdMessageId)) + .build(); + // When + testee.mergeInto(nonEmptyBuilder); + SetMessagesResponse mergedResponse = nonEmptyBuilder.build(); + + // Then + List<String> createdMessageIds = mergedResponse.getCreated().stream() + .map(m -> m.getId().serialize()) + .collect(Collectors.toList()); + assertThat(createdMessageIds).containsExactly(buildersCreatedMessageId, createdMessageId); + } + + @Test + public void mergeIntoShouldMergeDestroyedLists() { + // Given + MessageId buildersDestroyedMessageId = MessageId.of("user|inbox|1"); + SetMessagesResponse.Builder nonEmptyBuilder = SetMessagesResponse.builder() + .destroyed(ImmutableList.of(buildersDestroyedMessageId)); + MessageId destroyedMessageId = MessageId.of("user|inbox|2"); + SetMessagesResponse testee = SetMessagesResponse.builder() + .destroyed(ImmutableList.of(destroyedMessageId)) + .build(); + // When + testee.mergeInto(nonEmptyBuilder); + SetMessagesResponse mergedResponse = nonEmptyBuilder.build(); + + // Then + assertThat(mergedResponse.getDestroyed()).containsExactly(buildersDestroyedMessageId, destroyedMessageId); + } } http://git-wip-us.apache.org/repos/asf/james-project/blob/35b62b23/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/UpdateMessagePatchTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/UpdateMessagePatchTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/UpdateMessagePatchTest.java new file mode 100644 index 0000000..877fa87 --- /dev/null +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/UpdateMessagePatchTest.java @@ -0,0 +1,83 @@ +/**************************************************************** + * 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.assertj.core.api.Assertions.assertThat; + +import java.util.Arrays; +import java.util.List; +import javax.mail.Flags; + +import org.junit.Test; + +public class UpdateMessagePatchTest { + + @Test + public void UnsetUpdatePatchShouldBeValid() { + UpdateMessagePatch emptyPatch = UpdateMessagePatch.builder().build(); + assertThat(emptyPatch.isValid()).isTrue(); + assertThat(emptyPatch.isUnread()).isEmpty(); + assertThat(emptyPatch.isFlagged()).isEmpty(); + assertThat(emptyPatch.isAnswered()).isEmpty(); + } + + @Test + public void builderShouldSetSeenFlagWhenBuiltWithIsUnreadFalse() { + UpdateMessagePatch testee = UpdateMessagePatch.builder().isUnread(false).build(); + assertThat(testee.isUnread()).isPresent(); + assertThat(testee.isUnread().get()).isFalse(); + assertThat(testee.isFlagged()).isEmpty(); // belts and braces ! + } + + @Test + public void applyStateShouldSetFlaggedOnlyWhenUnsetPatchAppliedToFlaggedState() { + UpdateMessagePatch testee = UpdateMessagePatch.builder().build(); + boolean isFlaggedSet = true; + List<Flags.Flag> updatedFlags = Arrays.asList(testee.applyToState(false, false, isFlaggedSet).getSystemFlags()); + assertThat(updatedFlags).containsExactly(Flags.Flag.FLAGGED); + } + + + @Test + public void applyStateShouldReturnUnreadFlagWhenUnreadSetOnSeenMessage() { + UpdateMessagePatch testee = UpdateMessagePatch.builder().isUnread(true).build(); + boolean isSeen = true; + List<Flags.Flag> updatedFlags = Arrays.asList(testee.applyToState(isSeen, false, false).getSystemFlags()); + assertThat(updatedFlags).doesNotContain(Flags.Flag.SEEN); + } + + @Test + public void applyStateShouldReturnFlaggedWhenEmptyPatchOnFlaggedMessage() { + UpdateMessagePatch testee = UpdateMessagePatch.builder().build(); + boolean isFlagged = true; + List<Flags.Flag> updatedFlags = Arrays.asList(testee.applyToState(false, false, isFlagged).getSystemFlags()); + assertThat(updatedFlags).containsExactly(Flags.Flag.FLAGGED); + } + + @Test + public void applyStateShouldReturnSeenWhenPatchSetsSeenOnSeenMessage() { + UpdateMessagePatch testee = UpdateMessagePatch.builder().isUnread(false).build(); + boolean isSeen = true; + List<Flags.Flag> updatedFlags = Arrays.asList(testee.applyToState(isSeen, false, false).getSystemFlags()); + assertThat(updatedFlags).containsExactly(Flags.Flag.SEEN); + } + +} \ No newline at end of file --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
