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 <btell...@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("u...@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("u...@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("u...@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-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org