JAMES-1965 Make htmlBody and textBody should be Optional
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/030ed62a Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/030ed62a Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/030ed62a Branch: refs/heads/master Commit: 030ed62af30ee659cdcd37d5690441f836f81cd1 Parents: 4b70df9 Author: Quynh Nguyen <qngu...@linagora.com> Authored: Fri Mar 17 10:55:53 2017 +0700 Committer: Quynh Nguyen <qngu...@linagora.com> Committed: Thu Mar 23 16:03:17 2017 +0700 ---------------------------------------------------------------------- .../org/apache/james/jmap/model/Message.java | 10 ++++---- .../jmap/model/MessageContentExtractor.java | 20 +++++++++------- .../apache/james/jmap/model/MessageFactory.java | 25 ++++++++++---------- .../org/apache/james/jmap/model/SubMessage.java | 10 ++++---- .../james/jmap/json/ParsingWritingObjects.java | 5 ++-- .../jmap/methods/GetMessagesMethodTest.java | 4 ++-- .../jmap/model/MessageContentExtractorTest.java | 8 +++---- .../james/jmap/model/MessageFactoryTest.java | 8 +++---- .../apache/james/jmap/model/MessageTest.java | 4 ++-- .../james/jmap/model/SubMailboxMessageTest.java | 12 ++++++---- 10 files changed, 58 insertions(+), 48 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/030ed62a/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 4799b3f..9351460 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 @@ -71,8 +71,8 @@ public class Message { private ZonedDateTime date; private Long size; private String preview; - private String textBody; - private String htmlBody; + private Optional<String> textBody = Optional.empty(); + private Optional<String> htmlBody = Optional.empty(); private final ImmutableList.Builder<Attachment> attachments; private final ImmutableMap.Builder<BlobId, SubMessage> attachedMessages; @@ -190,12 +190,12 @@ public class Message { return this; } - public Builder textBody(String textBody) { + public Builder textBody(Optional<String> textBody) { this.textBody = textBody; return this; } - public Builder htmlBody(String htmlBody) { + public Builder htmlBody(Optional<String> htmlBody) { this.htmlBody = htmlBody; return this; } @@ -225,7 +225,7 @@ public class Message { boolean hasAttachment = hasAttachment(attachments); return new Message(id, blobId, threadId, mailboxIds, Optional.ofNullable(inReplyToMessageId), isUnread, isFlagged, isAnswered, isDraft, hasAttachment, headers, Optional.ofNullable(from), - to.build(), cc.build(), bcc.build(), replyTo.build(), subject, date, size, preview, Optional.ofNullable(textBody), Optional.ofNullable(htmlBody), attachments, attachedMessages); + to.build(), cc.build(), bcc.build(), replyTo.build(), subject, date, size, preview, textBody, htmlBody, attachments, attachedMessages); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/030ed62a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MessageContentExtractor.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MessageContentExtractor.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MessageContentExtractor.java index c27bc3e..d5b096d 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MessageContentExtractor.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MessageContentExtractor.java @@ -22,6 +22,7 @@ package org.apache.james.jmap.model; import java.io.IOException; import java.util.Objects; import java.util.Optional; +import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Stream; @@ -55,7 +56,7 @@ public class MessageContentExtractor { } private MessageContent parseTextBody(Entity entity, TextBody textBody) throws IOException { - String bodyContent = asString(textBody); + Optional<String> bodyContent = asString(textBody); if (TEXT_HTML.equals(entity.getMimeType())) { return MessageContent.ofHtmlOnly(bodyContent); } @@ -89,8 +90,8 @@ public class MessageContentExtractor { .orElse(MessageContent.empty()); } - private String asString(TextBody textBody) throws IOException { - return IOUtils.toString(textBody.getInputStream(), textBody.getMimeCharset()); + private Optional<String> asString(TextBody textBody) throws IOException { + return Optional.ofNullable(IOUtils.toString(textBody.getInputStream(), textBody.getMimeCharset())); } private MessageContent retrieveHtmlAndPlainTextContent(Multipart multipart) throws IOException { @@ -141,6 +142,9 @@ public class MessageContentExtractor { } private Optional<String> getFirstMatchingTextBody(Multipart multipart, String mimeType, Predicate<Entity> condition) { + Function<TextBody, Optional<String>> textBodyOptionalFunction = Throwing + .<TextBody, Optional<String>>function(textBody -> asString(textBody)).sneakyThrow(); + return multipart.getBodyParts() .stream() .filter(part -> mimeType.equals(part.getMimeType())) @@ -149,7 +153,7 @@ public class MessageContentExtractor { .filter(TextBody.class::isInstance) .map(TextBody.class::cast) .findFirst() - .map(Throwing.function(this::asString).sneakyThrow()); + .flatMap(textBodyOptionalFunction); } private boolean isNotAttachment(Entity part) { @@ -169,12 +173,12 @@ public class MessageContentExtractor { this.htmlBody = htmlBody; } - public static MessageContent ofTextOnly(String textBody) { - return new MessageContent(Optional.of(textBody), Optional.empty()); + public static MessageContent ofTextOnly(Optional<String> textBody) { + return new MessageContent(textBody, Optional.empty()); } - public static MessageContent ofHtmlOnly(String htmlBody) { - return new MessageContent(Optional.empty(), Optional.of(htmlBody)); + public static MessageContent ofHtmlOnly(Optional<String> htmlBody) { + return new MessageContent(Optional.empty(), htmlBody); } public static MessageContent empty() { http://git-wip-us.apache.org/repos/asf/james-project/blob/030ed62a/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 5d7b2fa..b49e3dc 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 @@ -72,8 +72,6 @@ public class MessageFactory { private static final String EMPTY_FILE_NAME = ""; - private static final String NO_BODY = ""; - private final MessagePreviewGenerator messagePreview; private final MessageContentExtractor messageContentExtractor; private final TextExtractor textExtractor; @@ -88,8 +86,8 @@ public class MessageFactory { public Message fromMetaDataWithContent(MetaDataWithContent message) throws MailboxException { org.apache.james.mime4j.dom.Message mimeMessage = parse(message); MessageContent messageContent = extractContent(mimeMessage); - String htmlBody = messageContent.getHtmlBody().orElse(NO_BODY); - String textBody = messageContent.getTextBody().orElseGet(() -> textBodyFromHtmlBody(htmlBody).orElse(NO_BODY)); + Optional<String> htmlBody = messageContent.getHtmlBody(); + Optional<String> textBody = textBodyAndComputeFromHtmlBodyIfNeeded(htmlBody, messageContent.getTextBody()); return Message.builder() .id(message.getMessageId()) .blobId(BlobId.of(String.valueOf(message.getUid().asLong()))) @@ -116,9 +114,15 @@ public class MessageFactory { .build(); } - private Optional<String> textBodyFromHtmlBody(String htmlBody) { + private Optional<String> textBodyAndComputeFromHtmlBodyIfNeeded(Optional<String> htmlBody, Optional<String> textBody) { + if (textBody.isPresent()) { + return textBody; + } + if (!htmlBody.isPresent()) { + return Optional.empty(); + } try { - return Optional.of(textExtractor.extractContent(new ByteArrayInputStream(htmlBody.getBytes()), HTML_CONTENT, EMPTY_FILE_NAME).getTextualContent()); + return Optional.of(textExtractor.extractContent(new ByteArrayInputStream(htmlBody.get().getBytes()), HTML_CONTENT, EMPTY_FILE_NAME).getTextualContent()); } catch (Exception e) { return Optional.empty(); } @@ -145,14 +149,11 @@ public class MessageFactory { } } - private String getPreview(MessageContent messageContent, String computedTextBody) { - if (messageContent.getHtmlBody().isPresent()) { - if (!messageContent.getTextBody().isPresent()) { - return messagePreview.forTextBody(Optional.of(computedTextBody)); - } + private String getPreview(MessageContent messageContent, Optional<String> computedTextBody) { + if (messageContent.getHtmlBody().isPresent() && messageContent.getTextBody().isPresent()) { return messagePreview.forHTMLBody(messageContent.getHtmlBody()); } - return messagePreview.forTextBody(messageContent.getTextBody()); + return messagePreview.forTextBody(computedTextBody); } private Emailer firstFromMailboxList(MailboxList list) { http://git-wip-us.apache.org/repos/asf/james-project/blob/030ed62a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/SubMessage.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/SubMessage.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/SubMessage.java index cbaf1cd..95be80a 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/SubMessage.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/SubMessage.java @@ -49,8 +49,8 @@ public class SubMessage { private final ImmutableList.Builder<Emailer> replyTo; private String subject; private ZonedDateTime date; - private String textBody; - private String htmlBody; + private Optional<String> textBody = Optional.empty(); + private Optional<String> htmlBody = Optional.empty(); private final ImmutableList.Builder<Attachment> attachments; private final ImmutableMap.Builder<BlobId, SubMessage> attachedMessages; @@ -103,12 +103,12 @@ public class SubMessage { return this; } - public Builder textBody(String textBody) { + public Builder textBody(Optional<String> textBody) { this.textBody = textBody; return this; } - public Builder htmlBody(String htmlBody) { + public Builder htmlBody(Optional<String> htmlBody) { this.htmlBody = htmlBody; return this; } @@ -131,7 +131,7 @@ public class SubMessage { ImmutableMap<BlobId, SubMessage> attachedMessages = this.attachedMessages.build(); Preconditions.checkState(Message.areAttachedMessagesKeysInAttachments(attachments, attachedMessages), "'attachedMessages' keys must be in 'attachements'"); return new SubMessage(headers, Optional.ofNullable(from), to.build(), cc.build(), bcc.build(), - replyTo.build(), subject, date, Optional.ofNullable(textBody), Optional.ofNullable(htmlBody), + replyTo.build(), subject, date, textBody, htmlBody, attachments, attachedMessages ); } http://git-wip-us.apache.org/repos/asf/james-project/blob/030ed62a/server/protocols/jmap/src/test/java/org/apache/james/jmap/json/ParsingWritingObjects.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/json/ParsingWritingObjects.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/json/ParsingWritingObjects.java index 62a9de4..3814b86 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/json/ParsingWritingObjects.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/json/ParsingWritingObjects.java @@ -21,6 +21,7 @@ package org.apache.james.jmap.json; import java.time.ZoneId; import java.time.ZonedDateTime; +import java.util.Optional; import org.apache.james.jmap.model.BlobId; import org.apache.james.jmap.model.Emailer; @@ -61,8 +62,8 @@ public interface ParsingWritingObjects { ZonedDateTime DATE = ZonedDateTime.parse("2014-10-30T14:12:00Z").withZoneSameLocal(ZoneId.of("GMT")); int SIZE = 1024; String PREVIEW = "myPreview"; - String TEXT_BODY = "myTextBody"; - String HTML_BODY = "<h1>myHtmlBody</h1>"; + Optional<String> TEXT_BODY = Optional.of("myTextBody"); + Optional<String> HTML_BODY = Optional.of("<h1>myHtmlBody</h1>"); } Message MESSAGE = Message.builder() http://git-wip-us.apache.org/repos/asf/james-project/blob/030ed62a/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 0c231ba..4686c3a 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 @@ -328,7 +328,7 @@ public class GetMessagesMethodTest { } @Test - public void processShouldEmptyTextBodyAndHtmlBodyWhenThoseAreEmpty() throws MailboxException { + public void processShouldReturnEmptyTextBodyAndHtmlBodyWhenThoseAreEmpty() throws MailboxException { MessageManager inbox = mailboxManager.getMailbox(inboxPath, session); Date now = new Date(); ByteArrayInputStream messageContent = new ByteArrayInputStream(("Content-Type: text/html\r\n" @@ -348,7 +348,7 @@ public class GetMessagesMethodTest { .extracting(GetMessagesResponse.class::cast) .flatExtracting(GetMessagesResponse::list) .extracting(Message::getId, Message::getTextBody, Message::getHtmlBody) - .containsOnly(Tuple.tuple(message.getMessageId(), Optional.empty(), Optional.empty())); + .containsOnly(Tuple.tuple(message.getMessageId(), Optional.empty(), Optional.of(""))); } @Test http://git-wip-us.apache.org/repos/asf/james-project/blob/030ed62a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MessageContentExtractorTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MessageContentExtractorTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MessageContentExtractorTest.java index 84f78c4..0182e63 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MessageContentExtractorTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MessageContentExtractorTest.java @@ -435,8 +435,8 @@ public class MessageContentExtractorTest { @Test public void mergeMessageContentShouldReturnMixWhenFirstTextOnlyAndSecondHtmlOnly() { - MessageContent messageContent1 = MessageContent.ofTextOnly(TEXT_CONTENT); - MessageContent messageContent2 = MessageContent.ofHtmlOnly(HTML_CONTENT); + MessageContent messageContent1 = MessageContent.ofTextOnly(Optional.of(TEXT_CONTENT)); + MessageContent messageContent2 = MessageContent.ofHtmlOnly(Optional.of(HTML_CONTENT)); MessageContent expected = new MessageContent(Optional.of(TEXT_CONTENT), Optional.of(HTML_CONTENT)); MessageContent actual = messageContent1.merge(messageContent2); @@ -446,8 +446,8 @@ public class MessageContentExtractorTest { @Test public void mergeMessageContentShouldReturnMixWhenFirstHtmlOnlyAndSecondTextOnly() { - MessageContent messageContent1 = MessageContent.ofHtmlOnly(HTML_CONTENT); - MessageContent messageContent2 = MessageContent.ofTextOnly(TEXT_CONTENT); + MessageContent messageContent1 = MessageContent.ofHtmlOnly(Optional.of(HTML_CONTENT)); + MessageContent messageContent2 = MessageContent.ofTextOnly(Optional.of(TEXT_CONTENT)); MessageContent expected = new MessageContent(Optional.of(TEXT_CONTENT), Optional.of(HTML_CONTENT)); MessageContent actual = messageContent1.merge(messageContent2); http://git-wip-us.apache.org/repos/asf/james-project/blob/030ed62a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MessageFactoryTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MessageFactoryTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MessageFactoryTest.java index 026d62b..74dad6f 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MessageFactoryTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MessageFactoryTest.java @@ -167,8 +167,8 @@ public class MessageFactoryTest { .date(ZONED_DATE) .size(headers.length()) .preview("(Empty)") - .textBody("") - .htmlBody("") + .textBody(Optional.of("")) + .htmlBody(Optional.empty()) .build(); assertThat(testee).isEqualToComparingFieldByField(expected); } @@ -362,7 +362,7 @@ public class MessageFactoryTest { Message testee = messageFactory.fromMetaDataWithContent(testMail); assertThat(testee.getPreview()).isEqualTo(MessagePreviewGenerator.NO_BODY); - assertThat(testee.getHtmlBody()).hasValue(""); - assertThat(testee.getTextBody()).hasValue(""); + assertThat(testee.getHtmlBody()).contains(""); + assertThat(testee.getTextBody()).isEmpty(); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/030ed62a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MessageTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MessageTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MessageTest.java index 3ecac4a..844396d 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MessageTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MessageTest.java @@ -203,8 +203,8 @@ public class MessageTest { .date(currentDate) .size(123) .preview("preview") - .textBody("textBody") - .htmlBody("htmlBody") + .textBody(Optional.of("textBody")) + .htmlBody(Optional.of("htmlBody")) .attachments(attachments) .attachedMessages(attachedMessages) .build(); http://git-wip-us.apache.org/repos/asf/james-project/blob/030ed62a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/SubMailboxMessageTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/SubMailboxMessageTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/SubMailboxMessageTest.java index daa2a8a..7047638 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/SubMailboxMessageTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/SubMailboxMessageTest.java @@ -29,6 +29,10 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; public class SubMailboxMessageTest { + + private static final Optional<String> DEFAULT_TEXT_BODY = Optional.of("textBody"); + private static final Optional<String> DEFAULT_HTML_BODY = Optional.of("htmlBody"); + @Test(expected=IllegalStateException.class) public void buildShouldThrowWhenHeadersIsNull() { SubMessage.builder().build(); @@ -106,8 +110,8 @@ public class SubMailboxMessageTest { replyTo, "subject", currentDate, - Optional.of("textBody"), - Optional.of("htmlBody"), + DEFAULT_TEXT_BODY, + DEFAULT_HTML_BODY, attachments, attachedMessages); SubMessage tested = SubMessage.builder() @@ -119,8 +123,8 @@ public class SubMailboxMessageTest { .replyTo(replyTo) .subject("subject") .date(currentDate) - .textBody("textBody") - .htmlBody("htmlBody") + .textBody(DEFAULT_TEXT_BODY) + .htmlBody(DEFAULT_HTML_BODY) .attachments(attachments) .attachedMessages(attachedMessages) .build(); --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
