james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From matth...@apache.org
Subject [04/15] james-project git commit: JAMES-1676 setMessages: handle parsing error for each update request
Date Mon, 15 Feb 2016 12:54:54 GMT
JAMES-1676 setMessages: handle parsing error for each update request


Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/41aebb7b
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/41aebb7b
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/41aebb7b

Branch: refs/heads/master
Commit: 41aebb7b01d8feee85e148e90792b966518a4ebe
Parents: 958a96e
Author: Raphael Ouazana <raphael.ouazana@linagora.com>
Authored: Fri Feb 5 10:40:23 2016 +0100
Committer: Fabien Vignon <fvignon@linagora.com>
Committed: Wed Feb 10 17:47:25 2016 +0100

----------------------------------------------------------------------
 .../jmap/methods/SetMessagesMethodTest.java     |  6 +--
 .../james/jmap/methods/SetMessagesMethod.java   | 54 ++++++++++++++++++--
 .../james/jmap/model/SetMessagesRequest.java    | 11 ++--
 3 files changed, 58 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/james-project/blob/41aebb7b/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 04921b3..c584f4c 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
@@ -489,7 +489,6 @@ public abstract class SetMessagesMethodTest {
     }
 
     @Test
-    @Ignore("Unable to deal with invalid types from SetMessages requests handler")
     public void setMessagesShouldRejectUpdateWhenPropertyHasWrongType() throws MailboxException
{
         jmapServer.serverProbe().createMailbox(MailboxConstants.USER_NAMESPACE, username,
"mailbox");
         jmapServer.serverProbe().appendMessage(username, new MailboxPath(MailboxConstants.USER_NAMESPACE,
username, "mailbox"),
@@ -504,7 +503,6 @@ public abstract class SetMessagesMethodTest {
                 .contentType(ContentType.JSON)
                 .header("Authorization", accessToken.serialize())
                 .body(String.format("[[\"setMessages\", {\"update\": {\"%s\" : { \"isUnread\"
: \"123\" } } }, \"#0\"]]", messageId))
-                // Does not work, jackson throws InvalidFormatException way before SetMessageMethod
can deal with !
         .when()
                 .post("/jmap")
         .then()
@@ -513,8 +511,8 @@ public abstract class SetMessagesMethodTest {
                 .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", equalTo("isUnread"))
-                .body("[0][1].notUpdated[\""+messageId+"\"].description", equalTo("invalid
properties"))
+                .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].updated", hasSize(0));
     }
 

http://git-wip-us.apache.org/repos/asf/james-project/blob/41aebb7b/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 c849718..d93b65a 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,19 +19,23 @@
 
 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.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;
@@ -48,8 +52,14 @@ 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;
@@ -65,11 +75,13 @@ public class SetMessagesMethod<Id extends MailboxId> implements
Method {
 
     private final MailboxMapperFactory<Id> mailboxMapperFactory;
     private final MailboxSessionMapperFactory<Id> mailboxSessionMapperFactory;
+    private final ObjectMapper jsonParser;
 
     @Inject
-    @VisibleForTesting SetMessagesMethod(MailboxMapperFactory<Id> mailboxMapperFactory,
MailboxSessionMapperFactory<Id> mailboxSessionMapperFactory) {
+    @VisibleForTesting SetMessagesMethod(MailboxMapperFactory<Id> mailboxMapperFactory,
MailboxSessionMapperFactory<Id> mailboxSessionMapperFactory, ObjectMapperFactory objectMapperFactory)
{
         this.mailboxMapperFactory = mailboxMapperFactory;
         this.mailboxSessionMapperFactory = mailboxSessionMapperFactory;
+        jsonParser = objectMapperFactory.forParsing();
     }
 
     @Override
@@ -106,11 +118,21 @@ public class SetMessagesMethod<Id extends MailboxId> implements
Method {
         return responseBuilder.build();
     }
 
-    private void processUpdates(Map<MessageId, UpdateMessagePatch> mapOfMessagePatchesById,
MailboxSession mailboxSession,
+    private void processUpdates(Map<MessageId, ObjectNode> mapOfMessagePatchesById,
MailboxSession mailboxSession,
                                 SetMessagesResponse.Builder responseBuilder) {
         mapOfMessagePatchesById.entrySet().stream()
-                .filter(kv -> kv.getValue().isValid())
-                .forEach(kv -> update(kv.getKey(), kv.getValue(), mailboxSession, responseBuilder));
+                .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){
@@ -127,6 +149,30 @@ public class SetMessagesMethod<Id extends MailboxId> implements
Method {
         }
     }
 
+    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()

http://git-wip-us.apache.org/repos/asf/james-project/blob/41aebb7b/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 1986df0..b1f3a60 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
@@ -28,6 +28,7 @@ import org.apache.james.jmap.methods.JmapRequest;
 
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder;
+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;
@@ -45,7 +46,7 @@ public class SetMessagesRequest implements JmapRequest {
         private String accountId;
         private String ifInState;
         private ImmutableList.Builder<Message> create;
-        private ImmutableMap.Builder<MessageId, UpdateMessagePatch> update;
+        private ImmutableMap.Builder<MessageId, ObjectNode> update;
         private ImmutableList.Builder<MessageId> destroy;
 
         private Builder() {
@@ -75,7 +76,7 @@ public class SetMessagesRequest implements JmapRequest {
             return this;
         }
 
-        public Builder update(Map<MessageId, UpdateMessagePatch> updates) {
+        public Builder update(Map<MessageId, ObjectNode> updates) {
             this.update.putAll(updates);
             return this;
         }
@@ -93,10 +94,10 @@ public class SetMessagesRequest implements JmapRequest {
     private final Optional<String> accountId;
     private final Optional<String> ifInState;
     private final List<Message> create;
-    private final Map<MessageId, UpdateMessagePatch> update;
+    private final Map<MessageId, ObjectNode> update;
     private final List<MessageId> destroy;
 
-    @VisibleForTesting SetMessagesRequest(Optional<String> accountId, Optional<String>
ifInState, List<Message> create, Map<MessageId, UpdateMessagePatch> update, List<MessageId>
destroy) {
+    @VisibleForTesting SetMessagesRequest(Optional<String> accountId, Optional<String>
ifInState, List<Message> create, Map<MessageId, ObjectNode> update, List<MessageId>
destroy) {
         this.accountId = accountId;
         this.ifInState = ifInState;
         this.create = create;
@@ -116,7 +117,7 @@ public class SetMessagesRequest implements JmapRequest {
         return create;
     }
 
-    public Map<MessageId, UpdateMessagePatch> getUpdate() {
+    public Map<MessageId, ObjectNode> getUpdate() {
         return update;
     }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Mime
View raw message