JAMES-2122 Adding a log on swallowed mailboxes
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/9507ba73 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/9507ba73 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/9507ba73 Branch: refs/heads/master Commit: 9507ba73787c4092c579e803d8bb609c6e9ead31 Parents: a27e73a Author: benwa <btell...@linagora.com> Authored: Thu Aug 17 10:01:25 2017 +0700 Committer: Matthieu Baechler <matth...@apache.org> Committed: Thu Aug 17 13:13:33 2017 +0200 ---------------------------------------------------------------------- .../cassandra/mail/CassandraMailboxMapper.java | 13 ++++++-- .../cassandra/mail/CassandraMessageMapper.java | 8 ++--- .../apache/james/util/OptionalConverter.java | 12 +++++++ .../james/util/OptionalConverterTest.java | 34 ++++++++++++++++++++ 4 files changed, 60 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/9507ba73/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java ---------------------------------------------------------------------- diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java index 229ce05..13feba8 100644 --- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java @@ -46,6 +46,9 @@ import org.apache.james.mailbox.store.mail.model.Mailbox; import org.apache.james.mailbox.store.mail.model.impl.SimpleMailbox; import org.apache.james.util.CompletableFutureUtil; import org.apache.james.util.FluentFutureStream; +import org.apache.james.util.OptionalConverter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.datastax.driver.core.Session; import com.datastax.driver.core.exceptions.InvalidQueryException; @@ -57,6 +60,7 @@ public class CassandraMailboxMapper implements MailboxMapper { public static final String WILDCARD = "%"; public static final String VALUES_MAY_NOT_BE_LARGER_THAN_64_K = "Index expression values may not be larger than 64K"; public static final String CLUSTERING_COLUMNS_IS_TOO_LONG = "The sum of all clustering columns is too long"; + public static final Logger LOGGER = LoggerFactory.getLogger(CassandraMailboxMapper.class); private final CassandraAsyncExecutor cassandraAsyncExecutor; private final CassandraMailboxPathDAO mailboxPathDAO; @@ -117,12 +121,17 @@ public class CassandraMailboxMapper implements MailboxMapper { return FluentFutureStream.of(mailboxPathDAO.listUserMailboxes(path.getNamespace(), path.getUser())) .filter(idAndPath -> regex.matcher(idAndPath.getMailboxPath().getName()).matches()) - .map(CassandraMailboxPathDAO.CassandraIdAndPath::getCassandraId) - .thenFlatComposeOnOptional(mailboxDAO::retrieveMailbox) + .thenFlatComposeOnOptional(this::retrieveMailbox) .join() .collect(Guavate.toImmutableList()); } + private CompletableFuture<Optional<SimpleMailbox>> retrieveMailbox(CassandraMailboxPathDAO.CassandraIdAndPath idAndPath) { + return mailboxDAO.retrieveMailbox(idAndPath.getCassandraId()) + .thenApply(optional -> OptionalConverter.ifEmpty(optional, + () -> LOGGER.warn("Could not retrieve mailbox {} with path {} in mailbox table.", idAndPath.getCassandraId(), idAndPath.getMailboxPath()))); + } + @Override public MailboxId save(Mailbox mailbox) throws MailboxException { Preconditions.checkArgument(mailbox instanceof SimpleMailbox); http://git-wip-us.apache.org/repos/asf/james-project/blob/9507ba73/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageMapper.java ---------------------------------------------------------------------- diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageMapper.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageMapper.java index eda305b..7236567 100644 --- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageMapper.java +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageMapper.java @@ -56,6 +56,7 @@ import org.apache.james.mailbox.store.mail.model.Mailbox; import org.apache.james.mailbox.store.mail.model.MailboxMessage; import org.apache.james.mailbox.store.mail.model.impl.SimpleMailboxMessage; import org.apache.james.util.FluentFutureStream; +import org.apache.james.util.OptionalConverter; import org.apache.james.util.streams.JamesCollectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -235,11 +236,8 @@ public class CassandraMessageMapper implements MessageMapper { private CompletableFuture<Optional<ComposedMessageIdWithMetaData>> retrieveComposedId(CassandraId mailboxId, MessageUid uid) { return messageIdDAO.retrieve(mailboxId, uid) - .thenApply(value -> Optional.of(value) - .orElseGet(() -> { - LOGGER.warn("Could not retrieve message {} {}", mailboxId, uid); - return Optional.empty(); - })); + .thenApply(optional -> OptionalConverter.ifEmpty(optional, + () -> LOGGER.warn("Could not retrieve message {} {}", mailboxId, uid))); } private CompletableFuture<Stream<Pair<MessageWithoutAttachment, Stream<MessageAttachmentRepresentation>>>> retrieveMessagesAndDoMigrationIfNeeded( http://git-wip-us.apache.org/repos/asf/james-project/blob/9507ba73/server/container/util-java8/src/main/java/org/apache/james/util/OptionalConverter.java ---------------------------------------------------------------------- diff --git a/server/container/util-java8/src/main/java/org/apache/james/util/OptionalConverter.java b/server/container/util-java8/src/main/java/org/apache/james/util/OptionalConverter.java index fafec7a..5150be1 100644 --- a/server/container/util-java8/src/main/java/org/apache/james/util/OptionalConverter.java +++ b/server/container/util-java8/src/main/java/org/apache/james/util/OptionalConverter.java @@ -23,6 +23,18 @@ import java.util.stream.Stream; public class OptionalConverter { + @FunctionalInterface + public interface Operation { + void perform(); + } + + public static <T> Optional<T> ifEmpty(Optional<T> optional, Operation operation) { + if (!optional.isPresent()) { + operation.perform(); + } + return optional; + } + public static <T> Optional<T> fromGuava(com.google.common.base.Optional<T> guava) { return Optional.ofNullable(guava.orNull()); } http://git-wip-us.apache.org/repos/asf/james-project/blob/9507ba73/server/container/util-java8/src/test/java/org/apache/james/util/OptionalConverterTest.java ---------------------------------------------------------------------- diff --git a/server/container/util-java8/src/test/java/org/apache/james/util/OptionalConverterTest.java b/server/container/util-java8/src/test/java/org/apache/james/util/OptionalConverterTest.java index baa3aa0..000ea2c 100644 --- a/server/container/util-java8/src/test/java/org/apache/james/util/OptionalConverterTest.java +++ b/server/container/util-java8/src/test/java/org/apache/james/util/OptionalConverterTest.java @@ -21,6 +21,7 @@ package org.apache.james.util; import static org.assertj.core.api.Assertions.assertThat; import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.Rule; import org.junit.Test; @@ -34,6 +35,39 @@ public class OptionalConverterTest { public ExpectedException expectedException = ExpectedException.none(); @Test + public void ifEmptyShouldPreserveValueOfEmptyOptionals() { + Optional<Object> expected = OptionalConverter.ifEmpty(Optional.empty(), () -> { }); + + assertThat(expected).isEmpty(); + } + + @Test + public void ifEmptyShouldPreserveValueOfPresentOptionals() { + String value = "value"; + Optional<String> expected = OptionalConverter.ifEmpty(Optional.of(value), () -> { }); + + assertThat(expected).contains(value); + } + + @Test + public void ifEmptyShouldPerformOperationIfEmpty() { + AtomicInteger operationCounter = new AtomicInteger(0); + + OptionalConverter.ifEmpty(Optional.empty(), operationCounter::incrementAndGet); + + assertThat(operationCounter.get()).isEqualTo(1); + } + + @Test + public void ifEmptyShouldNotPerformOperationIfPresent() { + AtomicInteger operationCounter = new AtomicInteger(0); + + OptionalConverter.ifEmpty(Optional.of("value"), operationCounter::incrementAndGet); + + assertThat(operationCounter.get()).isEqualTo(0); + } + + @Test public void toStreamShouldConvertEmptyOptionalToEmptyStream() { assertThat( OptionalConverter.toStream(Optional.empty()) --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
