Skip to content

Commit

Permalink
JAMES-2041 Skip faulty messages in GetMessages JMAP method and log error
Browse files Browse the repository at this point in the history
  • Loading branch information
@chibenwa
chibenwa committed Jun 2, 2017
1 parent 58f626f commit 9705fbf
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 13 deletions.
47 changes: 35 additions & 12 deletions server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMessagesMethod.java
View file
Expand Up @@ -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;
Expand All @@ -30,6 +32,7 @@
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;
Expand All @@ -38,14 +41,15 @@
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;
Expand All @@ -55,6 +59,7 @@
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;
Expand Down Expand Up @@ -126,8 +131,8 @@ private GetMessagesResponse getMessagesResponse(MailboxSession mailboxSession, G
.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();
Expand All @@ -136,16 +141,34 @@ private GetMessagesResponse getMessagesResponse(MailboxSession mailboxSession, G
}
}

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();
}
};
}

Expand Down
42 changes: 41 additions & 1 deletion server/protocols/jmap/src/test/java/org/apache/james/jmap/methods/GetMessagesMethodTest.java
View file
Expand Up @@ -20,7 +20,9 @@

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;
Expand All @@ -32,6 +34,8 @@
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;
Expand Down Expand Up @@ -65,13 +69,18 @@
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;
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;
Expand Down Expand Up @@ -128,7 +137,8 @@ public void setup() throws Exception {
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
Expand Down Expand Up @@ -466,4 +476,34 @@ public void processShouldReturnOneMessageWhenMessageInSeveralMailboxes() throws
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: user@domain.tld\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);
}
}

0 comments on commit 9705fbf

Please sign in to comment.