Repository: james-project Updated Branches: refs/heads/master f88895834 -> 1616042be
JAMES-1805 Allow empty subject when sending mail Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/a897f587 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/a897f587 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/a897f587 Branch: refs/heads/master Commit: a897f587f539eb02f3e4925e220d7a5db21c4e09 Parents: 0ea95c3 Author: Laura Royet <lro...@linagora.com> Authored: Fri Jul 22 12:11:04 2016 +0200 Committer: Laura Royet <lro...@linagora.com> Committed: Mon Jul 25 10:08:21 2016 +0200 ---------------------------------------------------------------------- .../integration/SetMessagesMethodTest.java | 82 +++++++++++++++++--- .../james/jmap/model/CreationMessage.java | 11 +-- .../org/apache/james/jmap/model/Message.java | 1 - .../apache/james/jmap/model/MessageFactory.java | 13 ++-- .../jmap/methods/GetMessagesMethodTest.java | 2 +- .../james/jmap/model/CreationMessageTest.java | 50 ++++++++++-- .../james/jmap/model/MailboxMessageTest.java | 2 +- 7 files changed, 126 insertions(+), 35 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/a897f587/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 6ea553f..af0dc11 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 @@ -686,6 +686,74 @@ public abstract class SetMessagesMethodTest { } @Test + public void setMessageShouldReturnCreatedMessageWithEmptySubjectWhenSubjectIsNull() { + String messageCreationId = "user|inbox|1"; + String fromAddress = username; + String requestBody = "[" + + " [" + + " \"setMessages\","+ + " {" + + " \"create\": { \"" + messageCreationId + "\" : {" + + " \"from\": { \"name\": \"Me\", \"email\": \"" + fromAddress + "\"}," + + " \"to\": [{ \"name\": \"BOB\", \"email\": \"some...@example.com\"}]," + + " \"subject\": null," + + " \"mailboxIds\": [\"" + getOutboxId(accessToken) + "\"]" + + " }}" + + " }," + + " \"#0\"" + + " ]" + + "]"; + + given() + .header("Authorization", accessToken.serialize()) + .body(requestBody) + .when() + .post("/jmap") + .then() + .log().ifValidationFails() + .statusCode(200) + .body(NAME, equalTo("messagesSet")) + .body(ARGUMENTS + ".notCreated", aMapWithSize(0)) + .body(ARGUMENTS + ".created", aMapWithSize(1)) + .body(ARGUMENTS + ".created", hasKey(messageCreationId)) + .body(ARGUMENTS + ".created[\""+messageCreationId+"\"].subject", equalTo("")); + } + + @Test + public void setMessageShouldReturnCreatedMessageWithEmptySubjectWhenSubjectIsEmpty() { + String messageCreationId = "user|inbox|1"; + String fromAddress = username; + String requestBody = "[" + + " [" + + " \"setMessages\","+ + " {" + + " \"create\": { \"" + messageCreationId + "\" : {" + + " \"from\": { \"name\": \"Me\", \"email\": \"" + fromAddress + "\"}," + + " \"to\": [{ \"name\": \"BOB\", \"email\": \"some...@example.com\"}]," + + " \"subject\": \"\"," + + " \"mailboxIds\": [\"" + getOutboxId(accessToken) + "\"]" + + " }}" + + " }," + + " \"#0\"" + + " ]" + + "]"; + + given() + .header("Authorization", accessToken.serialize()) + .body(requestBody) + .when() + .post("/jmap") + .then() + .log().ifValidationFails() + .statusCode(200) + .body(NAME, equalTo("messagesSet")) + .body(ARGUMENTS + ".notCreated", aMapWithSize(0)) + .body(ARGUMENTS + ".created", aMapWithSize(1)) + .body(ARGUMENTS + ".created", hasKey(messageCreationId)) + .body(ARGUMENTS + ".created[\""+messageCreationId+"\"].subject", equalTo("")); + } + + @Test public void setMessageShouldSupportArbitraryMessageId() { String messageCreationId = "1717fcd1-603e-44a5-b2a6-1234dbcd5723"; String fromAddress = username; @@ -998,9 +1066,8 @@ public abstract class SetMessagesMethodTest { calmlyAwait.atMost(30, TimeUnit.SECONDS).until( () -> messageHasBeenMovedToSentBox(sentMailboxId)); } - @Test - public void setMessagesShouldRejectWhenSendingMessageHasMissingSubject() { + public void setMessagesShouldNotRejectWhenSendingMessageHasMissingSubject() { String messageCreationId = "user|inbox|1"; String fromAddress = username; String requestBody = "[" + @@ -1028,14 +1095,9 @@ public abstract class SetMessagesMethodTest { .log().ifValidationFails() .statusCode(200) .body(NAME, equalTo("messagesSet")) - .body(ARGUMENTS + ".notCreated", hasKey(messageCreationId)) - .body(ARGUMENTS + ".notCreated[\""+messageCreationId+"\"].type", equalTo("invalidProperties")) - .body(ARGUMENTS + ".notCreated[\""+messageCreationId+"\"].properties", hasSize(1)) - .body(ARGUMENTS + ".notCreated[\""+messageCreationId+"\"].properties", contains("subject")) - .body(ARGUMENTS + ".notCreated[\""+messageCreationId+"\"].description", endsWith("'subject' is missing")) - .body(ARGUMENTS + ".created", aMapWithSize(0)); - } - + .body(ARGUMENTS + ".notCreated", aMapWithSize(0)) + .body(ARGUMENTS + ".created", aMapWithSize(1)); +} @Test public void setMessagesShouldRejectWhenSendingMessageUseSomeoneElseFromAddress() { http://git-wip-us.apache.org/repos/asf/james-project/blob/a897f587/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/CreationMessage.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/CreationMessage.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/CreationMessage.java index 5202a15..b925cc5 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/CreationMessage.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/CreationMessage.java @@ -151,8 +151,8 @@ public class CreationMessage { return this; } - public Builder subject(String subject) { - this.subject = subject; + public Builder subject (String subject) { + this.subject = Strings.nullToEmpty(subject); return this; } @@ -338,16 +338,9 @@ public class CreationMessage { ImmutableList.Builder<ValidationResult> errors = ImmutableList.builder(); assertValidFromProvided(errors); assertAtLeastOneValidRecipient(errors); - assertSubjectProvided(errors); return errors.build(); } - private void assertSubjectProvided(ImmutableList.Builder<ValidationResult> errors) { - if (Strings.isNullOrEmpty(subject)) { - errors.add(ValidationResult.builder().message("'subject' is missing").property(MessageProperty.subject.asFieldName()).build()); - } - } - private void assertAtLeastOneValidRecipient(ImmutableList.Builder<ValidationResult> errors) { ImmutableList<DraftEmailer> recipients = ImmutableList.<DraftEmailer>builder().addAll(to).addAll(cc).addAll(bcc).build(); boolean hasAtLeastOneAddressToSendTo = recipients.stream().anyMatch(DraftEmailer::hasValidEmail); http://git-wip-us.apache.org/repos/asf/james-project/blob/a897f587/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/Message.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/Message.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/Message.java index 2121843..1e334aa 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/Message.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/Message.java @@ -205,7 +205,6 @@ public class Message { Preconditions.checkState(!Strings.isNullOrEmpty(threadId), "'threadId' is mandatory"); Preconditions.checkState(mailboxIds != null, "'mailboxIds' is mandatory"); Preconditions.checkState(headers != null, "'headers' is mandatory"); - Preconditions.checkState(!Strings.isNullOrEmpty(subject), "'subject' is mandatory"); Preconditions.checkState(size != null, "'size' is mandatory"); Preconditions.checkState(date != null, "'date' is mandatory"); Preconditions.checkState(!Strings.isNullOrEmpty(preview), "'preview' is mandatory"); http://git-wip-us.apache.org/repos/asf/james-project/blob/a897f587/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MessageFactory.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MessageFactory.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MessageFactory.java index fb3571d..6eb9bda 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MessageFactory.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MessageFactory.java @@ -44,7 +44,6 @@ import com.google.common.collect.Multimap; public class MessageFactory { - public static final String NO_SUBJECT = "(No subject)"; public static final String MULTIVALUED_HEADERS_SEPARATOR = ", "; public static final ZoneId UTC_ZONE_ID = ZoneId.of("Z"); @@ -96,14 +95,12 @@ public class MessageFactory { } private String getSubject(IndexableMessage im) { - return Optional.ofNullable( - Strings.emptyToNull( - im.getSubjects() - .stream() - .collect(Collectors.joining(MULTIVALUED_HEADERS_SEPARATOR)))) - .orElse(NO_SUBJECT); + return im.getSubjects() + .stream() + .map(String::trim) + .collect(Collectors.joining(MULTIVALUED_HEADERS_SEPARATOR)); } - + private Emailer firstElasticSearchEmailers(Set<EMailer> emailers) { return emailers.stream() .findFirst() http://git-wip-us.apache.org/repos/asf/james-project/blob/a897f587/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/GetMessagesMethodTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/GetMessagesMethodTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/GetMessagesMethodTest.java index 6bca35f..54463ea 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/GetMessagesMethodTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/GetMessagesMethodTest.java @@ -188,7 +188,7 @@ public class GetMessagesMethodTest { .containsOnly( Tuple.tuple(message1Uid, "message 1 subject", Optional.of("my message")), Tuple.tuple(message2Uid, "message 2 subject", Optional.of("my message")), - Tuple.tuple(message3Uid, "(No subject)", Optional.of("my message"))); + Tuple.tuple(message3Uid, "", Optional.of("my message"))); } @Test http://git-wip-us.apache.org/repos/asf/james-project/blob/a897f587/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/CreationMessageTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/CreationMessageTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/CreationMessageTest.java index 4b23626..a72647f 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/CreationMessageTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/CreationMessageTest.java @@ -38,12 +38,14 @@ public class CreationMessageTest { public void setUp() { testedBuilder = CreationMessage.builder() .mailboxIds(ImmutableList.of("ba9-0f-dead-beef")) - .headers(ImmutableMap.of()) - .subject("anything"); + .headers(ImmutableMap.of()); } @Test public void validateShouldReturnErrorWhenFromIsMissing() { + testedBuilder = testedBuilder + .subject("anything"); + CreationMessage sut = testedBuilder.build(); assertThat(sut.validate()).contains(ValidationResult.builder() @@ -55,6 +57,9 @@ public class CreationMessageTest { @Test public void validateShouldReturnErrorWhenFromIsInvalid() { + testedBuilder = testedBuilder + .subject("anything"); + CreationMessage sut = testedBuilder.from(DraftEmailer.builder().name("bob").email("b...@domain.com@example.com").build()).build(); assertThat(sut.validate()).contains(ValidationResult.builder() @@ -66,6 +71,9 @@ public class CreationMessageTest { @Test public void validateShouldReturnErrorWhenNoRecipientSet () { + testedBuilder = testedBuilder + .subject("anything"); + CreationMessage sut = testedBuilder .from(DraftEmailer.builder().name("bob").email("b...@example.com").build()).build(); @@ -74,6 +82,9 @@ public class CreationMessageTest { @Test public void validateShouldReturnErrorWhenNoValidRecipientSet () { + testedBuilder = testedBuilder + .subject("anything"); + CreationMessage sut = testedBuilder .from(DraftEmailer.builder().name("bob").email("b...@example.com").build()) .to(ImmutableList.of(DraftEmailer.builder().name("riri").email("r...@acme.com@example.com").build())) @@ -86,10 +97,39 @@ public class CreationMessageTest { @Test public void validateShouldReturnEmptyListWhenNoErrors () { + testedBuilder = testedBuilder + .subject("anything"); + + CreationMessage sut = testedBuilder + .from(DraftEmailer.builder().name("bob").email("b...@example.com").build()) + .to(ImmutableList.of(DraftEmailer.builder().name("riri").email("r...@example.com").build())) + .build(); + + assertThat(sut.validate()).isEmpty(); + } + + @Test + public void validateShouldReturnEmptyListWhenSubjectIsNull () { + testedBuilder = testedBuilder + .subject(null); + CreationMessage sut = testedBuilder - .from(DraftEmailer.builder().name("bob").email("b...@example.com").build()) - .to(ImmutableList.of(DraftEmailer.builder().name("riri").email("r...@example.com").build())) - .build(); + .from(DraftEmailer.builder().name("bob").email("b...@example.com").build()) + .to(ImmutableList.of(DraftEmailer.builder().name("riri").email("r...@example.com").build())) + .build(); + + assertThat(sut.validate()).isEmpty(); + } + + @Test + public void validateShouldReturnEmptyListWhenSubjectIsEmpty () { + testedBuilder = testedBuilder + .subject(""); + + CreationMessage sut = testedBuilder + .from(DraftEmailer.builder().name("bob").email("b...@example.com").build()) + .to(ImmutableList.of(DraftEmailer.builder().name("riri").email("r...@example.com").build())) + .build(); assertThat(sut.validate()).isEmpty(); } http://git-wip-us.apache.org/repos/asf/james-project/blob/a897f587/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MailboxMessageTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MailboxMessageTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MailboxMessageTest.java index ac8e736..a9280e0 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MailboxMessageTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MailboxMessageTest.java @@ -255,7 +255,7 @@ public class MailboxMessageTest { Message testee = messageFactory.fromMailboxMessage(testMail, ImmutableList.of(), x -> MessageId.of("user|box|" + x)); assertThat(testee) .extracting(Message::getPreview, Message::getSize, Message::getSubject, Message::getHeaders, Message::getDate) - .containsExactly("(Empty)", 0L, "(No subject)", ImmutableMap.of(), ZONED_DATE); + .containsExactly("(Empty)", 0L, "", ImmutableMap.of(), ZONED_DATE); } @Test --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org