Skip to content

Commit

Permalink
JAMES-1965 Make htmlBody and textBody should be Optional
Browse files Browse the repository at this point in the history
  • Loading branch information
Quynh Nguyen committed Mar 23, 2017
1 parent 4b70df9 commit 030ed62
Show file tree
Hide file tree
Showing 10 changed files with 58 additions and 48 deletions.
Expand Up @@ -71,8 +71,8 @@ public static class Builder {
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;

Expand Down Expand Up @@ -190,12 +190,12 @@ public Builder preview(String preview) {
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;
}
Expand Down Expand Up @@ -225,7 +225,7 @@ public Message build() {
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);
}
}

Expand Down
Expand Up @@ -22,6 +22,7 @@
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;

Expand Down Expand Up @@ -55,7 +56,7 @@ public MessageContent extract(org.apache.james.mime4j.dom.Message message) throw
}

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);
}
Expand Down Expand Up @@ -89,8 +90,8 @@ private MessageContent parseFirstFoundMultipart(Multipart multipart) throws IOEx
.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 {
Expand Down Expand Up @@ -141,6 +142,9 @@ private Optional<String> getFirstMatchingTextBody(Multipart multipart, String mi
}

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()))
Expand All @@ -149,7 +153,7 @@ private Optional<String> getFirstMatchingTextBody(Multipart multipart, String mi
.filter(TextBody.class::isInstance)
.map(TextBody.class::cast)
.findFirst()
.map(Throwing.function(this::asString).sneakyThrow());
.flatMap(textBodyOptionalFunction);
}

private boolean isNotAttachment(Entity part) {
Expand All @@ -169,12 +173,12 @@ public MessageContent(Optional<String> textBody, Optional<String> htmlBody) {
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() {
Expand Down
Expand Up @@ -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;
Expand All @@ -88,8 +86,8 @@ public MessageFactory(MessagePreviewGenerator messagePreview, MessageContentExtr
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())))
Expand All @@ -116,9 +114,15 @@ public Message fromMetaDataWithContent(MetaDataWithContent message) throws Mailb
.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();
}
Expand All @@ -145,14 +149,11 @@ private MessageContent extractContent(org.apache.james.mime4j.dom.Message mimeMe
}
}

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) {
Expand Down
Expand Up @@ -49,8 +49,8 @@ public static class Builder {
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;

Expand Down Expand Up @@ -103,12 +103,12 @@ public Builder date(ZonedDateTime date) {
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;
}
Expand All @@ -131,7 +131,7 @@ public SubMessage build() {
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
);
}
Expand Down
Expand Up @@ -21,6 +21,7 @@

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;
Expand Down Expand Up @@ -61,8 +62,8 @@ public interface Common {
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()
Expand Down
Expand Up @@ -328,7 +328,7 @@ public void processShouldReturnTextBodyWhenEmptyTextBodyAndNotEmptyHtmlBody() th
}

@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"
Expand All @@ -348,7 +348,7 @@ public void processShouldEmptyTextBodyAndHtmlBodyWhenThoseAreEmpty() throws Mail
.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
Expand Down
Expand Up @@ -435,8 +435,8 @@ public void mergeMessageContentShouldReturnSecondWhenFirstEmpty() {

@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);
Expand All @@ -446,8 +446,8 @@ public void mergeMessageContentShouldReturnMixWhenFirstTextOnlyAndSecondHtmlOnly

@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);
Expand Down
Expand Up @@ -167,8 +167,8 @@ public void headersShouldBeSetIntoMessage() throws Exception {
.date(ZONED_DATE)
.size(headers.length())
.preview("(Empty)")
.textBody("")
.htmlBody("")
.textBody(Optional.of(""))
.htmlBody(Optional.empty())
.build();
assertThat(testee).isEqualToComparingFieldByField(expected);
}
Expand Down Expand Up @@ -362,7 +362,7 @@ public void textBodyShouldBeEmptyInCaseOfEmptyHtmlBodyAndEmptyTextBody() throws
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();
}
}
Expand Up @@ -203,8 +203,8 @@ public void buildShouldWorkWhenAllFieldsArePresent() {
.date(currentDate)
.size(123)
.preview("preview")
.textBody("textBody")
.htmlBody("htmlBody")
.textBody(Optional.of("textBody"))
.htmlBody(Optional.of("htmlBody"))
.attachments(attachments)
.attachedMessages(attachedMessages)
.build();
Expand Down
Expand Up @@ -29,6 +29,10 @@
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();
Expand Down Expand Up @@ -106,8 +110,8 @@ public void buildShouldWorkWhenAllFieldsArePresent() {
replyTo,
"subject",
currentDate,
Optional.of("textBody"),
Optional.of("htmlBody"),
DEFAULT_TEXT_BODY,
DEFAULT_HTML_BODY,
attachments,
attachedMessages);
SubMessage tested = SubMessage.builder()
Expand All @@ -119,8 +123,8 @@ public void buildShouldWorkWhenAllFieldsArePresent() {
.replyTo(replyTo)
.subject("subject")
.date(currentDate)
.textBody("textBody")
.htmlBody("htmlBody")
.textBody(DEFAULT_TEXT_BODY)
.htmlBody(DEFAULT_HTML_BODY)
.attachments(attachments)
.attachedMessages(attachedMessages)
.build();
Expand Down

0 comments on commit 030ed62

Please sign in to comment.