You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by rc...@apache.org on 2019/11/25 09:06:49 UTC

[james-project] 06/22: JAMES-2987 MessageViewFactory::fromMessageResults

This is an automated email from the ASF dual-hosted git repository.

rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit 95631b9571a1fd6fb09e7dd9c7c1fbbf275e807b
Author: Tran Tien Duc <dt...@linagora.com>
AuthorDate: Thu Nov 21 11:04:14 2019 +0700

    JAMES-2987 MessageViewFactory::fromMessageResults
---
 .../jmap/draft/methods/GetMessagesMethod.java      | 43 ++-------------------
 .../model/message/view/MessageViewFactory.java     | 44 +++++++++++++++++++++-
 .../jmap/draft/methods/GetMessagesMethodTest.java  | 17 +++++----
 3 files changed, 56 insertions(+), 48 deletions(-)

diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetMessagesMethod.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetMessagesMethod.java
index cc0508a..6ac3f13 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetMessagesMethod.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetMessagesMethod.java
@@ -20,7 +20,6 @@
 package org.apache.james.jmap.draft.methods;
 
 import java.util.Collection;
-import java.util.List;
 import java.util.Optional;
 import java.util.function.Function;
 import java.util.stream.Stream;
@@ -31,19 +30,15 @@ import org.apache.james.jmap.draft.JmapFieldNotSupportedException;
 import org.apache.james.jmap.draft.json.FieldNamePropertyFilter;
 import org.apache.james.jmap.draft.model.GetMessagesRequest;
 import org.apache.james.jmap.draft.model.GetMessagesResponse;
-import org.apache.james.jmap.draft.model.Keywords;
 import org.apache.james.jmap.draft.model.MessageProperties;
 import org.apache.james.jmap.draft.model.MessageProperties.HeaderProperty;
 import org.apache.james.jmap.draft.model.MethodCallId;
 import org.apache.james.jmap.draft.model.message.view.MessageFullView;
 import org.apache.james.jmap.draft.model.message.view.MessageViewFactory;
-import org.apache.james.jmap.draft.model.message.view.MessageViewFactory.MetaDataWithContent;
-import org.apache.james.jmap.draft.utils.KeywordsCombiner;
 import org.apache.james.mailbox.MailboxSession;
 import org.apache.james.mailbox.MessageIdManager;
 import org.apache.james.mailbox.exception.MailboxException;
 import org.apache.james.mailbox.model.FetchGroupImpl;
-import org.apache.james.mailbox.model.MailboxId;
 import org.apache.james.mailbox.model.MessageResult;
 import org.apache.james.metrics.api.MetricFactory;
 import org.apache.james.util.MDCBuilder;
@@ -64,11 +59,9 @@ public class GetMessagesMethod implements Method {
     private static final Logger LOGGER = LoggerFactory.getLogger(GetMessagesMethod.class);
     private static final Method.Request.Name METHOD_NAME = Method.Request.name("getMessages");
     private static final Method.Response.Name RESPONSE_NAME = Method.Response.name("messages");
-    private static final KeywordsCombiner ACCUMULATOR = new KeywordsCombiner();
     private final MessageViewFactory messageViewFactory;
     private final MessageIdManager messageIdManager;
     private final MetricFactory metricFactory;
-    private final Keywords.KeywordsFactory keywordsFactory;
 
     @Inject
     @VisibleForTesting GetMessagesMethod(
@@ -78,7 +71,6 @@ public class GetMessagesMethod implements Method {
         this.messageViewFactory = messageViewFactory;
         this.messageIdManager = messageIdManager;
         this.metricFactory = metricFactory;
-        this.keywordsFactory = Keywords.lenientFactory();
     }
     
     @Override
@@ -140,8 +132,7 @@ public class GetMessagesMethod implements Method {
                         .values()
                         .stream()
                         .filter(collection -> !collection.isEmpty())
-                        .flatMap(toMetaDataWithContent())
-                        .flatMap(toMessage())
+                        .flatMap(toMessageViews())
                         .collect(Guavate.toImmutableList()))
                 .expectedMessageIds(getMessagesRequest.getIds())
                 .build();
@@ -150,38 +141,12 @@ public class GetMessagesMethod implements Method {
         }
     }
 
-    private Function<MetaDataWithContent, Stream<MessageFullView>> toMessage() {
-        return metaDataWithContent -> {
-            try {
-                return Stream.of(messageViewFactory.fromMetaDataWithContent(metaDataWithContent));
-            } catch (Exception e) {
-                LOGGER.error("Can not convert metaData with content to Message for {}", metaDataWithContent.getMessageId(), e);
-                return Stream.of();
-            }
-        };
-    }
-
-    private Function<Collection<MessageResult>, Stream<MetaDataWithContent>> toMetaDataWithContent() {
+    private Function<Collection<MessageResult>, Stream<MessageFullView>> toMessageViews() {
         return messageResults -> {
-            MessageResult firstMessageResult = messageResults.iterator().next();
-            List<MailboxId> mailboxIds = messageResults.stream()
-                .map(MessageResult::getMailboxId)
-                .distinct()
-                .collect(Guavate.toImmutableList());
             try {
-                Keywords keywords = messageResults.stream()
-                    .map(MessageResult::getFlags)
-                    .map(keywordsFactory::fromFlags)
-                    .reduce(ACCUMULATOR)
-                    .get();
-                return Stream.of(
-                    MetaDataWithContent.builderFromMessageResult(firstMessageResult)
-                        .messageId(firstMessageResult.getMessageId())
-                        .mailboxIds(mailboxIds)
-                        .keywords(keywords)
-                        .build());
+                return Stream.of(messageViewFactory.fromMessageResults(messageResults));
             } catch (Exception e) {
-                LOGGER.error("Can not convert MessageResults to MetaData with content for messageId {} in {}", firstMessageResult.getMessageId(), mailboxIds, e);
+                LOGGER.error("Can not convert metaData with content to Message for {}", messageResults.iterator().next().getMessageId().serialize(), e);
                 return Stream.of();
             }
         };
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/message/view/MessageViewFactory.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/message/view/MessageViewFactory.java
index e250298..90ca232 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/message/view/MessageViewFactory.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/message/view/MessageViewFactory.java
@@ -40,6 +40,7 @@ import org.apache.james.jmap.draft.model.Emailer;
 import org.apache.james.jmap.draft.model.Keywords;
 import org.apache.james.jmap.draft.model.MessagePreviewGenerator;
 import org.apache.james.jmap.draft.utils.HtmlTextExtractor;
+import org.apache.james.jmap.draft.utils.KeywordsCombiner;
 import org.apache.james.mailbox.BlobManager;
 import org.apache.james.mailbox.MessageUid;
 import org.apache.james.mailbox.exception.MailboxException;
@@ -66,20 +67,28 @@ import com.google.common.collect.Multimaps;
 import com.google.common.collect.Sets;
 
 public class MessageViewFactory {
-
     public static final String JMAP_MULTIVALUED_FIELD_DELIMITER = "\n";
 
+    private static final KeywordsCombiner ACCUMULATOR = new KeywordsCombiner();
+
     private final BlobManager blobManager;
     private final MessagePreviewGenerator messagePreview;
     private final MessageContentExtractor messageContentExtractor;
     private final HtmlTextExtractor htmlTextExtractor;
+    private final Keywords.KeywordsFactory keywordsFactory;
 
     @Inject
-    public MessageViewFactory(BlobManager blobManager, MessagePreviewGenerator messagePreview, MessageContentExtractor messageContentExtractor, HtmlTextExtractor htmlTextExtractor) {
+    public MessageViewFactory(BlobManager blobManager, MessagePreviewGenerator messagePreview, MessageContentExtractor messageContentExtractor,
+                              HtmlTextExtractor htmlTextExtractor) {
         this.blobManager = blobManager;
         this.messagePreview = messagePreview;
         this.messageContentExtractor = messageContentExtractor;
         this.htmlTextExtractor = htmlTextExtractor;
+        this.keywordsFactory = Keywords.lenientFactory();
+    }
+
+    public MessageFullView fromMessageResults(Collection<MessageResult> messageResults) throws MailboxException {
+        return fromMetaDataWithContent(toMetaDataWithContent(messageResults));
     }
 
     public MessageFullView fromMetaDataWithContent(MetaDataWithContent message) throws MailboxException {
@@ -112,6 +121,37 @@ public class MessageViewFactory {
                 .build();
     }
 
+    private MetaDataWithContent toMetaDataWithContent(Collection<MessageResult> messageResults) throws MailboxException {
+        Preconditions.checkArgument(!messageResults.isEmpty(), "MessageResults cannot be empty");
+        Preconditions.checkArgument(hasOnlyOneMessageId(messageResults), "MessageResults need to share the same messageId");
+
+        MessageResult firstMessageResult = messageResults.iterator().next();
+        List<MailboxId> mailboxIds = messageResults.stream()
+            .map(MessageResult::getMailboxId)
+            .distinct()
+            .collect(Guavate.toImmutableList());
+
+        Keywords keywords = messageResults.stream()
+            .map(MessageResult::getFlags)
+            .map(keywordsFactory::fromFlags)
+            .reduce(ACCUMULATOR)
+            .get();
+
+        return MetaDataWithContent.builderFromMessageResult(firstMessageResult)
+            .messageId(firstMessageResult.getMessageId())
+            .mailboxIds(mailboxIds)
+            .keywords(keywords)
+            .build();
+    }
+
+    private boolean hasOnlyOneMessageId(Collection<MessageResult> messageResults) {
+        return messageResults
+            .stream()
+            .map(MessageResult::getMessageId)
+            .distinct()
+            .count() == 1;
+    }
+
     private Instant getDateFromHeaderOrInternalDateOtherwise(org.apache.james.mime4j.dom.Message mimeMessage, MetaDataWithContent message) {
         return Optional.ofNullable(mimeMessage.getDate())
             .map(Date::toInstant)
diff --git a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/methods/GetMessagesMethodTest.java b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/methods/GetMessagesMethodTest.java
index 1f9f8db..3cb8ec3 100644
--- a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/methods/GetMessagesMethodTest.java
+++ b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/methods/GetMessagesMethodTest.java
@@ -21,7 +21,9 @@ package org.apache.james.jmap.draft.methods;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doCallRealMethod;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 
 import java.nio.charset.StandardCharsets;
@@ -94,7 +96,8 @@ public class GetMessagesMethodTest {
     private MailboxPath inboxPath;
     private MailboxPath customMailboxPath;
     private MethodCallId methodCallId;
-    
+    private MessageViewFactory messageViewFactory;
+
     @Before
     public void setup() throws Exception {
         methodCallId = MethodCallId.of("#0");
@@ -103,7 +106,7 @@ public class GetMessagesMethodTest {
         MessageContentExtractor messageContentExtractor = new MessageContentExtractor();
         BlobManager blobManager = mock(BlobManager.class);
         when(blobManager.toBlobId(any(MessageId.class))).thenReturn(BlobId.fromString("fake"));
-        MessageViewFactory messageViewFactory = new MessageViewFactory(blobManager, messagePreview, messageContentExtractor, htmlTextExtractor);
+        messageViewFactory = spy(new MessageViewFactory(blobManager, messagePreview, messageContentExtractor, htmlTextExtractor));
         InMemoryIntegrationResources resources = InMemoryIntegrationResources.defaultResources();
         mailboxManager = resources.getMailboxManager();
 
@@ -475,8 +478,6 @@ public class GetMessagesMethodTest {
 
     @Test
     public void processShouldNotFailOnSingleMessageFailure() throws Exception {
-        MessageViewFactory messageViewFactory = mock(MessageViewFactory.class);
-        testee = new GetMessagesMethod(messageViewFactory, messageIdManager, new DefaultMetricFactory());
         MessageManager inbox = mailboxManager.getMailbox(inboxPath, session);
 
         org.apache.james.mime4j.dom.Message messageContent = org.apache.james.mime4j.dom.Message.Builder.of()
@@ -489,9 +490,11 @@ public class GetMessagesMethodTest {
 
         ComposedMessageId message1 = inbox.appendMessage(AppendCommand.from(messageContent), session);
         ComposedMessageId message2 = inbox.appendMessage(AppendCommand.from(messageContent), session);
-        when(messageViewFactory.fromMetaDataWithContent(any()))
-            .thenReturn(mock(MessageFullView.class))
-            .thenThrow(new RuntimeException());
+
+        doCallRealMethod()
+            .doThrow(new RuntimeException())
+            .when(messageViewFactory)
+            .fromMessageResults(any());
 
         GetMessagesRequest request = GetMessagesRequest.builder()
             .ids(ImmutableList.of(message1.getMessageId(), message2.getMessageId()))


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org