JAMES-2041 Skip faulty messages in GetMessages JMAP method and log error
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/9705fbf7 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/9705fbf7 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/9705fbf7 Branch: refs/heads/master Commit: 9705fbf715bf3e463a27d3089fce5f044bcbe42a Parents: 58f626f Author: benwa <[email protected]> Authored: Fri Jun 2 09:45:59 2017 +0700 Committer: benwa <[email protected]> Committed: Fri Jun 2 19:01:43 2017 +0700 ---------------------------------------------------------------------- .../james/jmap/methods/GetMessagesMethod.java | 47 +++++++++++++++----- .../jmap/methods/GetMessagesMethodTest.java | 42 ++++++++++++++++- 2 files changed, 76 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/9705fbf7/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMessagesMethod.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMessagesMethod.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMessagesMethod.java index 37a0b97..acaf9cc 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMessagesMethod.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMessagesMethod.java @@ -20,7 +20,9 @@ package org.apache.james.jmap.methods; import java.util.Collection; +import java.util.List; import java.util.Optional; +import java.util.function.Function; import java.util.stream.Stream; import javax.inject.Inject; @@ -30,6 +32,7 @@ import org.apache.james.jmap.json.FieldNamePropertyFilter; import org.apache.james.jmap.model.ClientId; import org.apache.james.jmap.model.GetMessagesRequest; import org.apache.james.jmap.model.GetMessagesResponse; +import org.apache.james.jmap.model.Message; import org.apache.james.jmap.model.MessageFactory; import org.apache.james.jmap.model.MessageFactory.MetaDataWithContent; import org.apache.james.jmap.model.MessageProperties; @@ -38,14 +41,15 @@ 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.metrics.api.TimeMetric; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.fasterxml.jackson.databind.ser.PropertyFilter; import com.fasterxml.jackson.databind.ser.impl.SimpleFilterProvider; -import com.github.fge.lambdas.Throwing; -import com.github.fge.lambdas.functions.ThrowingFunction; import com.github.steveash.guavate.Guavate; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; @@ -55,6 +59,7 @@ import com.google.common.collect.ImmutableSet; public class GetMessagesMethod implements Method { public static final String HEADERS_FILTER = "headersFilter"; + 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 MessageFactory messageFactory; @@ -126,8 +131,8 @@ public class GetMessagesMethod implements Method { .values() .stream() .filter(collection -> !collection.isEmpty()) - .map(Throwing.function(toMetaDataWithContent()).sneakyThrow()) - .map(Throwing.function(messageFactory::fromMetaDataWithContent).sneakyThrow()) + .flatMap(toMetaDataWithContent()) + .flatMap(toMessage()) .collect(Guavate.toImmutableList())) .expectedMessageIds(getMessagesRequest.getIds()) .build(); @@ -136,16 +141,34 @@ public class GetMessagesMethod implements Method { } } - private ThrowingFunction<Collection<MessageResult>, MetaDataWithContent> toMetaDataWithContent() { + private Function<MetaDataWithContent, Stream<Message>> toMessage() { + return metaDataWithContent -> { + try { + return Stream.of(messageFactory.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() { return messageResults -> { MessageResult firstMessageResult = messageResults.iterator().next(); - return MetaDataWithContent.builderFromMessageResult(firstMessageResult) - .messageId(firstMessageResult.getMessageId()) - .mailboxIds(messageResults.stream() - .map(MessageResult::getMailboxId) - .distinct() - .collect(Guavate.toImmutableList())) - .build(); + List<MailboxId> mailboxIds = messageResults.stream() + .map(MessageResult::getMailboxId) + .distinct() + .collect(Guavate.toImmutableList()); + try { + return Stream.of( + MetaDataWithContent.builderFromMessageResult(firstMessageResult) + .messageId(firstMessageResult.getMessageId()) + .mailboxIds(mailboxIds) + .build()); + } catch (Exception e) { + LOGGER.error("Can not convert MessageResults to MetaData with content for messageId " + firstMessageResult.getMessageId() + " in " + mailboxIds, e); + return Stream.of(); + } }; } http://git-wip-us.apache.org/repos/asf/james-project/blob/9705fbf7/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 7a0ab78..c8de63a 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 @@ -20,7 +20,9 @@ package org.apache.james.jmap.methods; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.io.ByteArrayInputStream; import java.util.Date; @@ -32,6 +34,8 @@ import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; +import javax.mail.Flags; + import org.apache.commons.lang.NotImplementedException; import org.apache.james.jmap.model.ClientId; import org.apache.james.jmap.model.GetMessagesRequest; @@ -65,6 +69,7 @@ import org.junit.Test; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ser.impl.SimpleBeanPropertyFilter; import com.fasterxml.jackson.databind.ser.impl.SimpleFilterProvider; +import com.github.steveash.guavate.Guavate; import com.google.common.base.Charsets; import com.google.common.collect.ImmutableList; import com.google.common.collect.Sets; @@ -72,6 +77,10 @@ import com.jayway.jsonpath.JsonPath; public class GetMessagesMethodTest { + public static final Flags FLAGS = null; + public static final boolean NOT_RECENT = false; + private MessageIdManager messageIdManager; + private static class User implements org.apache.james.mailbox.MailboxSession.User { final String username; final String password; @@ -128,7 +137,8 @@ public class GetMessagesMethodTest { customMailboxPath = new MailboxPath(inboxPath, "custom"); mailboxManager.createMailbox(inboxPath, session); mailboxManager.createMailbox(customMailboxPath, session); - testee = new GetMessagesMethod(messageFactory, inMemoryIntegrationResources.createMessageIdManager(mailboxManager), new DefaultMetricFactory()); + messageIdManager = inMemoryIntegrationResources.createMessageIdManager(mailboxManager); + testee = new GetMessagesMethod(messageFactory, messageIdManager, new DefaultMetricFactory()); } @Test @@ -466,4 +476,34 @@ public class GetMessagesMethodTest { assertThat(getMessagesResponse.list()).hasSize(1); assertThat(getMessagesResponse.list().get(0).getMailboxIds()).containsOnly(customMailboxId, message1.getMailboxId()); } + + @Test + public void processShouldNotFailOnSingleMessageFailure() throws Exception { + MessageFactory messageFactory = mock(MessageFactory.class); + testee = new GetMessagesMethod(messageFactory, messageIdManager, new DefaultMetricFactory()); + MessageManager inbox = mailboxManager.getMailbox(inboxPath, session); + Date now = new Date(); + ByteArrayInputStream message1Content = new ByteArrayInputStream(("From: [email protected]\r\n" + + "header1: Header1Content\r\n" + + "HEADer2: Header2Content\r\n" + + "Subject: message 1 subject\r\n\r\nmy message").getBytes(Charsets.UTF_8)); + ComposedMessageId message1 = inbox.appendMessage(message1Content, now, session, NOT_RECENT, FLAGS); + ComposedMessageId message2 = inbox.appendMessage(message1Content, now, session, NOT_RECENT, FLAGS); + when(messageFactory.fromMetaDataWithContent(any())) + .thenReturn(mock(Message.class)) + .thenThrow(new RuntimeException()); + + GetMessagesRequest request = GetMessagesRequest.builder() + .ids(ImmutableList.of(message1.getMessageId(), message2.getMessageId())) + .properties(ImmutableList.of("mailboxIds")) + .build(); + + List<JmapResponse> responses = testee.process(request, clientId, session).collect(Guavate.toImmutableList()); + + assertThat(responses).hasSize(1); + Method.Response response = responses.get(0).getResponse(); + assertThat(response).isInstanceOf(GetMessagesResponse.class); + GetMessagesResponse getMessagesResponse = (GetMessagesResponse) response; + assertThat(getMessagesResponse.list()).hasSize(1); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
