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 bt...@apache.org on 2019/11/28 02:12:39 UTC

[james-project] 22/23: JAMES-2988 Restrict MessageView to the minimal one in GetResponse

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

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

commit b9af21bced5e8f1478a0e2bd4186dc48e320b7bf
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Tue Nov 26 16:54:55 2019 +0700

    JAMES-2988 Restrict MessageView to the minimal one in GetResponse
---
 .../jmap/draft/methods/GetMessagesMethod.java      |  19 +--
 .../message/view/MessageHeaderViewFactory.java     |   4 +-
 .../message/view/MessageMetadataViewFactory.java   |   5 +-
 ...iewFactory.java => MetaMessageViewFactory.java} |  47 ++++----
 .../jmap/draft/methods/GetMessagesMethodTest.java  | 129 +++++++++++++++++++--
 5 files changed, 160 insertions(+), 44 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 966ec40..01c2c25 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
@@ -33,8 +33,9 @@ import org.apache.james.jmap.draft.model.GetMessagesResponse;
 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.MessageFullViewFactory;
+import org.apache.james.jmap.draft.model.message.view.MessageView;
+import org.apache.james.jmap.draft.model.message.view.MessageViewFactory;
+import org.apache.james.jmap.draft.model.message.view.MetaMessageViewFactory;
 import org.apache.james.mailbox.MailboxSession;
 import org.apache.james.mailbox.MessageIdManager;
 import org.apache.james.mailbox.exception.MailboxException;
@@ -59,16 +60,16 @@ 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 final MessageFullViewFactory messageFullViewFactory;
+    private final MetaMessageViewFactory messageViewFactory;
     private final MessageIdManager messageIdManager;
     private final MetricFactory metricFactory;
 
     @Inject
     @VisibleForTesting GetMessagesMethod(
-            MessageFullViewFactory messageFullViewFactory,
+            MetaMessageViewFactory messageViewFactory,
             MessageIdManager messageIdManager,
             MetricFactory metricFactory) {
-        this.messageFullViewFactory = messageFullViewFactory;
+        this.messageViewFactory = messageViewFactory;
         this.messageIdManager = messageIdManager;
         this.metricFactory = metricFactory;
     }
@@ -123,6 +124,8 @@ public class GetMessagesMethod implements Method {
 
         try {
             MessageProperties.ReadProfile readProfile = getMessagesRequest.getProperties().computeReadLevel();
+            MessageViewFactory factory = messageViewFactory.getFactory(readProfile);
+
             return GetMessagesResponse.builder()
                 .messages(
                     messageIdManager.getMessages(getMessagesRequest.getIds(), FetchGroup.FULL_CONTENT, mailboxSession)
@@ -132,7 +135,7 @@ public class GetMessagesMethod implements Method {
                         .values()
                         .stream()
                         .filter(collection -> !collection.isEmpty())
-                        .flatMap(toMessageViews())
+                        .flatMap(toMessageViews(factory))
                         .collect(Guavate.toImmutableList()))
                 .expectedMessageIds(getMessagesRequest.getIds())
                 .build();
@@ -141,10 +144,10 @@ public class GetMessagesMethod implements Method {
         }
     }
 
-    private Function<Collection<MessageResult>, Stream<MessageFullView>> toMessageViews() {
+    private Function<Collection<MessageResult>, Stream<MessageView>> toMessageViews(MessageViewFactory factory) {
         return messageResults -> {
             try {
-                return Stream.of(messageFullViewFactory.fromMessageResults(messageResults));
+                return Stream.of(factory.fromMessageResults(messageResults));
             } catch (Exception e) {
                 LOGGER.error("Can not convert MessageResults 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/MessageHeaderViewFactory.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/message/view/MessageHeaderViewFactory.java
index 6995b44..27392f7 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/message/view/MessageHeaderViewFactory.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/message/view/MessageHeaderViewFactory.java
@@ -36,13 +36,15 @@ import org.apache.james.mailbox.model.MessageResult;
 import org.apache.james.mime4j.dom.Message;
 import org.apache.james.mime4j.stream.MimeConfig;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 
 public class MessageHeaderViewFactory implements MessageViewFactory<MessageHeaderView> {
     private final BlobManager blobManager;
 
     @Inject
-    MessageHeaderViewFactory(BlobManager blobManager) {
+    @VisibleForTesting
+    public MessageHeaderViewFactory(BlobManager blobManager) {
         this.blobManager = blobManager;
     }
 
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/message/view/MessageMetadataViewFactory.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/message/view/MessageMetadataViewFactory.java
index 95a382f..e98f609 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/message/view/MessageMetadataViewFactory.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/message/view/MessageMetadataViewFactory.java
@@ -30,11 +30,14 @@ import org.apache.james.mailbox.exception.MailboxException;
 import org.apache.james.mailbox.model.MailboxId;
 import org.apache.james.mailbox.model.MessageResult;
 
+import com.google.common.annotations.VisibleForTesting;
+
 public class MessageMetadataViewFactory implements MessageViewFactory<MessageMetadataView> {
     private final BlobManager blobManager;
 
     @Inject
-    MessageMetadataViewFactory(BlobManager blobManager) {
+    @VisibleForTesting
+    public MessageMetadataViewFactory(BlobManager blobManager) {
         this.blobManager = blobManager;
     }
 
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/message/view/MessageMetadataViewFactory.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/message/view/MetaMessageViewFactory.java
similarity index 50%
copy from server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/message/view/MessageMetadataViewFactory.java
copy to server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/message/view/MetaMessageViewFactory.java
index 95a382f..f513613 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/message/view/MessageMetadataViewFactory.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/message/view/MetaMessageViewFactory.java
@@ -19,39 +19,32 @@
 
 package org.apache.james.jmap.draft.model.message.view;
 
-import java.util.Collection;
-import java.util.List;
-
 import javax.inject.Inject;
 
-import org.apache.james.jmap.draft.model.BlobId;
-import org.apache.james.mailbox.BlobManager;
-import org.apache.james.mailbox.exception.MailboxException;
-import org.apache.james.mailbox.model.MailboxId;
-import org.apache.james.mailbox.model.MessageResult;
+import org.apache.james.jmap.draft.model.MessageProperties;
 
-public class MessageMetadataViewFactory implements MessageViewFactory<MessageMetadataView> {
-    private final BlobManager blobManager;
+public class MetaMessageViewFactory {
+    private final MessageFullViewFactory messageFullViewFactory;
+    private final MessageHeaderViewFactory messageHeaderViewFactory;
+    private final MessageMetadataViewFactory messageMetadataViewFactory;
 
     @Inject
-    MessageMetadataViewFactory(BlobManager blobManager) {
-        this.blobManager = blobManager;
+    public MetaMessageViewFactory(MessageFullViewFactory messageFullViewFactory, MessageHeaderViewFactory messageHeaderViewFactory, MessageMetadataViewFactory messageMetadataViewFactory) {
+        this.messageFullViewFactory = messageFullViewFactory;
+        this.messageHeaderViewFactory = messageHeaderViewFactory;
+        this.messageMetadataViewFactory = messageMetadataViewFactory;
     }
 
-    @Override
-    public MessageMetadataView fromMessageResults(Collection<MessageResult> messageResults) throws MailboxException {
-        assertOneMessageId(messageResults);
-
-        MessageResult firstMessageResult = messageResults.iterator().next();
-        List<MailboxId> mailboxIds = getMailboxIds(messageResults);
-
-        return MessageMetadataView.messageMetadataBuilder()
-            .id(firstMessageResult.getMessageId())
-            .mailboxIds(mailboxIds)
-            .blobId(BlobId.of(blobManager.toBlobId(firstMessageResult.getMessageId())))
-            .threadId(firstMessageResult.getMessageId().serialize())
-            .keywords(getKeywords(messageResults))
-            .size(firstMessageResult.getSize())
-            .build();
+    public MessageViewFactory getFactory(MessageProperties.ReadProfile readProfile) {
+        switch (readProfile) {
+            case Full:
+                return messageFullViewFactory;
+            case Header:
+                return messageHeaderViewFactory;
+            case Metadata:
+                return messageMetadataViewFactory;
+            default:
+                throw new IllegalArgumentException(readProfile + " is not a James JMAP draft supported view");
+        }
     }
 }
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 6fa5adb..420761e 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
@@ -45,6 +45,11 @@ import org.apache.james.jmap.draft.model.MessageProperties.MessageProperty;
 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.MessageFullViewFactory;
+import org.apache.james.jmap.draft.model.message.view.MessageHeaderView;
+import org.apache.james.jmap.draft.model.message.view.MessageHeaderViewFactory;
+import org.apache.james.jmap.draft.model.message.view.MessageMetadataView;
+import org.apache.james.jmap.draft.model.message.view.MessageMetadataViewFactory;
+import org.apache.james.jmap.draft.model.message.view.MetaMessageViewFactory;
 import org.apache.james.jmap.draft.utils.HtmlTextExtractor;
 import org.apache.james.jmap.draft.utils.JsoupHtmlTextExtractor;
 import org.apache.james.mailbox.BlobManager;
@@ -96,7 +101,7 @@ public class GetMessagesMethodTest {
     private MailboxPath inboxPath;
     private MailboxPath customMailboxPath;
     private MethodCallId methodCallId;
-    private MessageFullViewFactory messageFullViewFactory;
+    private MessageMetadataViewFactory messageMetadataViewFactory;
 
     @Before
     public void setup() throws Exception {
@@ -106,7 +111,12 @@ public class GetMessagesMethodTest {
         MessageContentExtractor messageContentExtractor = new MessageContentExtractor();
         BlobManager blobManager = mock(BlobManager.class);
         when(blobManager.toBlobId(any(MessageId.class))).thenReturn(BlobId.fromString("fake"));
-        messageFullViewFactory = spy(new MessageFullViewFactory(blobManager, messagePreview, messageContentExtractor, htmlTextExtractor));
+        messageMetadataViewFactory = spy(new MessageMetadataViewFactory(blobManager));
+        MetaMessageViewFactory metaMessageViewFactory = new MetaMessageViewFactory(
+            new MessageFullViewFactory(blobManager, messagePreview, messageContentExtractor, htmlTextExtractor),
+            new MessageHeaderViewFactory(blobManager),
+            messageMetadataViewFactory);
+
         InMemoryIntegrationResources resources = InMemoryIntegrationResources.defaultResources();
         mailboxManager = resources.getMailboxManager();
 
@@ -116,7 +126,7 @@ public class GetMessagesMethodTest {
         mailboxManager.createMailbox(inboxPath, session);
         mailboxManager.createMailbox(customMailboxPath, session);
         messageIdManager = resources.getMessageIdManager();
-        testee = new GetMessagesMethod(messageFullViewFactory, messageIdManager, new DefaultMetricFactory());
+        testee = new GetMessagesMethod(metaMessageViewFactory, messageIdManager, new DefaultMetricFactory());
 
         messageContent1 = org.apache.james.mime4j.dom.Message.Builder.of()
             .setSubject("message 1 subject")
@@ -470,13 +480,118 @@ public class GetMessagesMethodTest {
         assertThat(response).isInstanceOf(GetMessagesResponse.class);
         GetMessagesResponse getMessagesResponse = (GetMessagesResponse) response;
         assertThat(getMessagesResponse.list()).hasSize(1)
-            .hasOnlyElementsOfType(MessageFullView.class)
-            .extracting(MessageFullView.class::cast)
-            .flatExtracting(MessageFullView::getMailboxIds)
+            .hasOnlyElementsOfType(MessageMetadataView.class)
+            .extracting(MessageMetadataView.class::cast)
+            .flatExtracting(MessageMetadataView::getMailboxIds)
             .containsOnly(customMailboxId, message1.getMailboxId());
     }
 
     @Test
+    public void processShouldReturnMetadataWhenOnlyMailboxIds() throws Exception {
+        MessageManager inbox = mailboxManager.getMailbox(inboxPath, session);
+
+        ComposedMessageId message1 = inbox.appendMessage(
+            AppendCommand.from(
+                org.apache.james.mime4j.dom.Message.Builder.of()
+                    .setFrom("user@domain.tld")
+                    .setField(new RawField("header1", "Header1Content"))
+                    .setField(new RawField("HEADer2", "Header2Content"))
+                    .setSubject("message 1 subject")
+                    .setBody("my message", StandardCharsets.UTF_8)),
+            session);
+
+        MailboxId customMailboxId = mailboxManager.getMailbox(customMailboxPath, session).getId();
+        messageIdManager.setInMailboxes(message1.getMessageId(),
+            ImmutableList.of(message1.getMailboxId(), customMailboxId),
+            session);
+
+        GetMessagesRequest request = GetMessagesRequest.builder()
+            .ids(ImmutableList.of(message1.getMessageId()))
+            .properties(ImmutableList.of("mailboxIds"))
+            .build();
+
+        List<JmapResponse> result = testee.process(request, methodCallId, session).collect(Collectors.toList());
+
+        assertThat(result).hasSize(1);
+        Method.Response response = result.get(0).getResponse();
+        assertThat(response).isInstanceOf(GetMessagesResponse.class);
+        GetMessagesResponse getMessagesResponse = (GetMessagesResponse) response;
+        assertThat(getMessagesResponse.list())
+            .hasSize(1)
+            .hasOnlyElementsOfType(MessageMetadataView.class);
+    }
+
+    @Test
+    public void processShouldReturnFullViewWhenRequestedTextBody() throws Exception {
+        MessageManager inbox = mailboxManager.getMailbox(inboxPath, session);
+
+        ComposedMessageId message1 = inbox.appendMessage(
+            AppendCommand.from(
+                org.apache.james.mime4j.dom.Message.Builder.of()
+                    .setFrom("user@domain.tld")
+                    .setField(new RawField("header1", "Header1Content"))
+                    .setField(new RawField("HEADer2", "Header2Content"))
+                    .setSubject("message 1 subject")
+                    .setBody("my message", StandardCharsets.UTF_8)),
+            session);
+
+        MailboxId customMailboxId = mailboxManager.getMailbox(customMailboxPath, session).getId();
+        messageIdManager.setInMailboxes(message1.getMessageId(),
+            ImmutableList.of(message1.getMailboxId(), customMailboxId),
+            session);
+
+        GetMessagesRequest request = GetMessagesRequest.builder()
+            .ids(ImmutableList.of(message1.getMessageId()))
+            .properties(ImmutableList.of("mailboxIds", "textBody"))
+            .build();
+
+        List<JmapResponse> result = testee.process(request, methodCallId, session).collect(Collectors.toList());
+
+        assertThat(result).hasSize(1);
+        Method.Response response = result.get(0).getResponse();
+        assertThat(response).isInstanceOf(GetMessagesResponse.class);
+        GetMessagesResponse getMessagesResponse = (GetMessagesResponse) response;
+        assertThat(getMessagesResponse.list())
+            .hasSize(1)
+            .hasOnlyElementsOfType(MessageFullView.class);
+    }
+
+    @Test
+    public void processShouldReturnHeaderViewWhenRequestedTo() throws Exception {
+        MessageManager inbox = mailboxManager.getMailbox(inboxPath, session);
+
+        ComposedMessageId message1 = inbox.appendMessage(
+            AppendCommand.from(
+                org.apache.james.mime4j.dom.Message.Builder.of()
+                    .setFrom("user@domain.tld")
+                    .setField(new RawField("header1", "Header1Content"))
+                    .setField(new RawField("HEADer2", "Header2Content"))
+                    .setSubject("message 1 subject")
+                    .setBody("my message", StandardCharsets.UTF_8)),
+            session);
+
+        MailboxId customMailboxId = mailboxManager.getMailbox(customMailboxPath, session).getId();
+        messageIdManager.setInMailboxes(message1.getMessageId(),
+            ImmutableList.of(message1.getMailboxId(), customMailboxId),
+            session);
+
+        GetMessagesRequest request = GetMessagesRequest.builder()
+            .ids(ImmutableList.of(message1.getMessageId()))
+            .properties(ImmutableList.of("mailboxIds", "to"))
+            .build();
+
+        List<JmapResponse> result = testee.process(request, methodCallId, session).collect(Collectors.toList());
+
+        assertThat(result).hasSize(1);
+        Method.Response response = result.get(0).getResponse();
+        assertThat(response).isInstanceOf(GetMessagesResponse.class);
+        GetMessagesResponse getMessagesResponse = (GetMessagesResponse) response;
+        assertThat(getMessagesResponse.list())
+            .hasSize(1)
+            .hasOnlyElementsOfType(MessageHeaderView.class);
+    }
+
+    @Test
     public void processShouldNotFailOnSingleMessageFailure() throws Exception {
         MessageManager inbox = mailboxManager.getMailbox(inboxPath, session);
 
@@ -493,7 +608,7 @@ public class GetMessagesMethodTest {
 
         doCallRealMethod()
             .doThrow(new RuntimeException())
-            .when(messageFullViewFactory)
+            .when(messageMetadataViewFactory)
             .fromMessageResults(any());
 
         GetMessagesRequest request = GetMessagesRequest.builder()


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