Skip to content

Commit

Permalink
JAMES-1756 Improve handling of error message when moving a message
Browse files Browse the repository at this point in the history
  • Loading branch information
@Laura-Royet
Laura-Royet committed Jun 28, 2016
1 parent 6468afc commit caf66ab
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 20 deletions.
72 changes: 59 additions & 13 deletions ...common/src/test/java/org/apache/james/jmap/methods/integration/SetMessagesMethodTest.java
View file
Expand Up @@ -36,6 +36,7 @@
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;
Expand All @@ -49,6 +50,7 @@
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;
Expand All @@ -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;

Expand Down Expand Up @@ -351,7 +354,7 @@ private ResponseSpecification getSetMessagesUpdateOKResponseAssertions(String me
.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();
}

Expand Down Expand Up @@ -504,10 +507,10 @@ public void setMessagesShouldRejectUpdateWhenPropertyHasWrongType() throws Mailb
.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));
}
Expand All @@ -532,11 +535,11 @@ public void setMessagesShouldRejectUpdateWhenPropertiesHaveWrongTypes() throws M
.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));
}

Expand Down Expand Up @@ -605,9 +608,9 @@ public void setMessageShouldReturnNotFoundWhenUpdateUnknownMessage() {
.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));
}

Expand Down Expand Up @@ -1317,4 +1320,47 @@ private boolean isHtmlMessageReceived(AccessToken recipientToken) {
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));
}
}
5 changes: 3 additions & 2 deletions ...otocols/jmap/src/main/java/org/apache/james/jmap/methods/UpdateMessagePatchValidator.java
View file
Expand Up @@ -25,6 +25,7 @@
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;
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion server/protocols/jmap/src/main/java/org/apache/james/jmap/model/UpdateMessagePatch.java
View file
Expand Up @@ -50,7 +50,7 @@ public static class Builder {

public Builder mailboxIds(Optional<List<String>> mailboxIds) {
if (mailboxIds.isPresent()) {
throw new NotImplementedException();
throw new NotImplementedException("moving a message is not supported");
}
return this;
}
Expand Down
29 changes: 25 additions & 4 deletions ...ols/jmap/src/test/java/org/apache/james/jmap/methods/UpdateMessagePatchValidatorTest.java
View file
Expand Up @@ -28,23 +28,34 @@
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;
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");
}
Expand All @@ -53,25 +64,35 @@ public void validateShouldReturnPropertyNameWhenPropertyHasInvalidType() throws
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);
}

}

0 comments on commit caf66ab

Please sign in to comment.