Repository: james-project Updated Branches: refs/heads/master 6468afca0 -> caf66abd0
JAMES-1756 Improve handling of error message when moving a message Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/caf66abd Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/caf66abd Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/caf66abd Branch: refs/heads/master Commit: caf66abd0e398fe2f7addbfc21547849a195f94a Parents: 6468afc Author: Laura Royet <lro...@linagora.com> Authored: Thu Jun 23 11:40:31 2016 +0200 Committer: Laura Royet <lro...@linagora.com> Committed: Tue Jun 28 16:49:44 2016 +0200 ---------------------------------------------------------------------- .../integration/SetMessagesMethodTest.java | 72 ++++++++++++++++---- .../methods/UpdateMessagePatchValidator.java | 5 +- .../james/jmap/model/UpdateMessagePatch.java | 2 +- .../UpdateMessagePatchValidatorTest.java | 29 ++++++-- 4 files changed, 88 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/caf66abd/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SetMessagesMethodTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SetMessagesMethodTest.java b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SetMessagesMethodTest.java index e93ba7a..f10c73d 100644 --- a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SetMessagesMethodTest.java +++ b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SetMessagesMethodTest.java @@ -36,6 +36,7 @@ import static org.hamcrest.collection.IsMapWithSize.aMapWithSize; import static org.hamcrest.collection.IsMapWithSize.anEmptyMap; import java.io.ByteArrayInputStream; +import java.time.ZonedDateTime; import java.util.Date; import java.util.List; import java.util.Map; @@ -49,6 +50,7 @@ import org.apache.james.jmap.api.access.AccessToken; import org.apache.james.mailbox.exception.MailboxException; import org.apache.james.mailbox.model.MailboxConstants; import org.apache.james.mailbox.model.MailboxPath; +import org.apache.james.mailbox.store.mail.model.Mailbox; import org.hamcrest.Matchers; import org.junit.After; import org.junit.Before; @@ -72,6 +74,7 @@ public abstract class SetMessagesMethodTest { private static final String SECOND_NAME = "[1][0]"; private static final String SECOND_ARGUMENTS = "[1][1]"; private static final String USERS_DOMAIN = "domain.tld"; + private static final String NOT_UPDATED = ARGUMENTS + ".notUpdated"; private ConditionFactory calmlyAwait; @@ -351,7 +354,7 @@ public abstract class SetMessagesMethodTest { .expectBody(ARGUMENTS + ".updated", hasSize(1)) .expectBody(ARGUMENTS + ".updated", contains(messageId)) .expectBody(ARGUMENTS + ".error", isEmptyOrNullString()) - .expectBody(ARGUMENTS + ".notUpdated", not(hasKey(messageId))); + .expectBody(NOT_UPDATED, not(hasKey(messageId))); return builder.build(); } @@ -504,10 +507,10 @@ public abstract class SetMessagesMethodTest { .log().ifValidationFails() .statusCode(200) .body(NAME, equalTo("messagesSet")) - .body(ARGUMENTS + ".notUpdated", hasKey(messageId)) - .body(ARGUMENTS + ".notUpdated[\""+messageId+"\"].type", equalTo("invalidProperties")) - .body(ARGUMENTS + ".notUpdated[\""+messageId+"\"].properties[0]", equalTo("isUnread")) - .body(ARGUMENTS + ".notUpdated[\""+messageId+"\"].description", equalTo("isUnread: Can not construct instance of java.lang.Boolean from String value '123': only \"true\" or \"false\" recognized\n" + + .body(NOT_UPDATED, hasKey(messageId)) + .body(NOT_UPDATED + "[\""+messageId+"\"].type", equalTo("invalidProperties")) + .body(NOT_UPDATED + "[\""+messageId+"\"].properties[0]", equalTo("isUnread")) + .body(NOT_UPDATED + "[\""+messageId+"\"].description", equalTo("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(ARGUMENTS + ".updated", hasSize(0)); } @@ -532,11 +535,11 @@ public abstract class SetMessagesMethodTest { .log().ifValidationFails() .statusCode(200) .body(NAME, equalTo("messagesSet")) - .body(ARGUMENTS + ".notUpdated", hasKey(messageId)) - .body(ARGUMENTS + ".notUpdated[\""+messageId+"\"].type", equalTo("invalidProperties")) - .body(ARGUMENTS + ".notUpdated[\""+messageId+"\"].properties", hasSize(2)) - .body(ARGUMENTS + ".notUpdated[\""+messageId+"\"].properties[0]", equalTo("isUnread")) - .body(ARGUMENTS + ".notUpdated[\""+messageId+"\"].properties[1]", equalTo("isFlagged")) + .body(NOT_UPDATED, hasKey(messageId)) + .body(NOT_UPDATED + "[\""+messageId+"\"].type", equalTo("invalidProperties")) + .body(NOT_UPDATED + "[\""+messageId+"\"].properties", hasSize(2)) + .body(NOT_UPDATED + "[\""+messageId+"\"].properties[0]", equalTo("isUnread")) + .body(NOT_UPDATED + "[\""+messageId+"\"].properties[1]", equalTo("isFlagged")) .body(ARGUMENTS + ".updated", hasSize(0)); } @@ -605,9 +608,9 @@ public abstract class SetMessagesMethodTest { .log().ifValidationFails() .statusCode(200) .body(NAME, equalTo("messagesSet")) - .body(ARGUMENTS + ".notUpdated", hasKey(nonExistingMessageId)) - .body(ARGUMENTS + ".notUpdated[\""+nonExistingMessageId+"\"].type", equalTo("notFound")) - .body(ARGUMENTS + ".notUpdated[\""+nonExistingMessageId+"\"].description", equalTo("message not found")) + .body(NOT_UPDATED, hasKey(nonExistingMessageId)) + .body(NOT_UPDATED + "[\""+nonExistingMessageId+"\"].type", equalTo("notFound")) + .body(NOT_UPDATED + "[\""+nonExistingMessageId+"\"].description", equalTo("message not found")) .body(ARGUMENTS + ".updated", hasSize(0)); } @@ -1317,4 +1320,47 @@ public abstract class SetMessagesMethodTest { return false; } } + + @Test + public void movingAMessageIsNotSupported() throws Exception { + String newMailboxName = "heartFolder"; + jmapServer.serverProbe().createMailbox("#private", username, newMailboxName); + Mailbox heartFolder = jmapServer.serverProbe().getMailbox("#private", username, newMailboxName); + String heartFolderId = heartFolder.getMailboxId().serialize(); + + ZonedDateTime dateTime = ZonedDateTime.parse("2014-10-30T14:12:00Z"); + jmapServer.serverProbe().appendMessage(username, new MailboxPath("#private", username, "inbox"), + new ByteArrayInputStream("Subject: my test subject\r\n\r\ntestmail".getBytes()), Date.from(dateTime.toInstant()), false, new Flags()); + + String messageToMoveId = "user|inbox|1"; + + String requestBody = "[" + + " [" + + " \"setMessages\","+ + " {" + + " \"update\": { \"" + messageToMoveId + "\" : {" + + " \"mailboxIds\": [\"" + heartFolderId + "\"]" + + " }}" + + " }," + + " \"#0\"" + + " ]" + + "]"; + + given() + .accept(ContentType.JSON) + .contentType(ContentType.JSON) + .header("Authorization", this.accessToken.serialize()) + .body(requestBody) + .when() + .post("/jmap") + .then() + .statusCode(200) + .body(NAME, equalTo("messagesSet")) + .body(NOT_UPDATED, hasKey(messageToMoveId)) + .body(NOT_UPDATED + "[\""+messageToMoveId+"\"].type", equalTo("invalidProperties")) + .body(NOT_UPDATED + "[\""+messageToMoveId+"\"].properties[0]", equalTo("mailboxIds")) + .body(NOT_UPDATED + "[\""+messageToMoveId+"\"].description", equalTo("mailboxIds: moving a message is not supported " + + "(through reference chain: org.apache.james.jmap.model.Builder[\"mailboxIds\"])")) + .body(ARGUMENTS + ".updated", hasSize(0)); + } } http://git-wip-us.apache.org/repos/asf/james-project/blob/caf66abd/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 index e170d66..963c074 100644 --- 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 @@ -25,6 +25,7 @@ import java.util.Optional; import java.util.Set; import javax.inject.Inject; +import org.apache.james.jmap.json.ObjectMapperFactory; import org.apache.james.jmap.model.MessageProperties; import org.apache.james.jmap.model.UpdateMessagePatch; import com.fasterxml.jackson.databind.JsonMappingException; @@ -38,8 +39,8 @@ public class UpdateMessagePatchValidator implements Validator<ObjectNode> { private final ObjectMapper parser; @Inject - @VisibleForTesting UpdateMessagePatchValidator(ObjectMapper parser) { - this.parser = parser; + @VisibleForTesting UpdateMessagePatchValidator(ObjectMapperFactory parserFactory) { + this.parser = parserFactory.forParsing(); } @Override http://git-wip-us.apache.org/repos/asf/james-project/blob/caf66abd/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 bcbbf9e..05eded8 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 @@ -50,7 +50,7 @@ public class UpdateMessagePatch { public Builder mailboxIds(Optional<List<String>> mailboxIds) { if (mailboxIds.isPresent()) { - throw new NotImplementedException(); + throw new NotImplementedException("moving a message is not supported"); } return this; } http://git-wip-us.apache.org/repos/asf/james-project/blob/caf66abd/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 index 10aa5fd..db8d8d0 100644 --- 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 @@ -28,7 +28,9 @@ import static org.mockito.Mockito.when; import java.io.IOException; import java.util.Set; +import org.apache.james.jmap.json.ObjectMapperFactory; import org.apache.james.jmap.model.UpdateMessagePatch; +import org.junit.Before; import org.junit.Test; import com.fasterxml.jackson.databind.JsonMappingException; @@ -36,15 +38,24 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; public class UpdateMessagePatchValidatorTest { + private ObjectMapperFactory objectMapperFactory; + + @Before + public void setUp() { + objectMapperFactory = mock(ObjectMapperFactory.class); + } @Test public void validateShouldReturnPropertyNameWhenPropertyHasInvalidType() throws IOException { ObjectMapper mapper = new ObjectMapper(); + when(objectMapperFactory.forParsing()) + .thenReturn(mapper); + String jsonContent = "{ \"isUnread\" : \"123\" }"; ObjectNode rootNode = mapper.readValue(jsonContent, ObjectNode.class); - UpdateMessagePatchValidator sut = new UpdateMessagePatchValidator(mapper); + UpdateMessagePatchValidator sut = new UpdateMessagePatchValidator(objectMapperFactory); Set<ValidationResult> result = sut.validate(rootNode); assertThat(result).extracting(ValidationResult::getProperty).contains("isUnread"); } @@ -53,25 +64,35 @@ public class UpdateMessagePatchValidatorTest { public void isValidShouldReturnTrueWhenPatchWellFormatted() throws IOException { ObjectMapper mapper = new ObjectMapper(); + when(objectMapperFactory.forParsing()) + .thenReturn(mapper); + String jsonContent = "{ \"isUnread\" : \"true\" }"; ObjectNode rootNode = mapper.readValue(jsonContent, ObjectNode.class); - UpdateMessagePatchValidator sut = new UpdateMessagePatchValidator(mapper); + UpdateMessagePatchValidator sut = new UpdateMessagePatchValidator(objectMapperFactory); assertThat(sut.isValid(rootNode)).isTrue(); } @Test public void validateShouldReturnANonEmptyResultWhenParsingThrows() throws IOException { - // Given + //Given ObjectNode emptyRootNode = new ObjectMapper().createObjectNode(); + ObjectMapper mapper = mock(ObjectMapper.class); when(mapper.readValue(anyString(), eq(UpdateMessagePatch.class))) .thenThrow(new JsonMappingException("Exception when parsing")); - UpdateMessagePatchValidator sut = new UpdateMessagePatchValidator(mapper); + + when(objectMapperFactory.forParsing()) + .thenReturn(mapper); + + UpdateMessagePatchValidator sut = new UpdateMessagePatchValidator(objectMapperFactory); + // 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 --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org