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 4bfe5b54a7464ae2c649191c1f5dc1aa869c1299 Author: Benoit Tellier <[email protected]> AuthorDate: Mon Apr 6 08:18:11 2020 +0700 [Performance] Avoid reading all mailboxes upon GetMessageList When the user specify a single mailbox inMailboxes filter, we still read all mailboxes, resulting in many unwanted queries --- .../cassandra/mail/CassandraMailboxMapper.java | 10 +++++++ .../james/mailbox/store/StoreMailboxManager.java | 11 +++++-- .../james/mailbox/store/mail/MailboxMapper.java | 15 ++++++++++ .../store/mail/model/MailboxMapperTest.java | 35 ++++++++++++++++++++++ 4 files changed, 69 insertions(+), 2 deletions(-) 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 75b068a..d74ce1c 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 @@ -20,7 +20,9 @@ package org.apache.james.mailbox.cassandra.mail; import java.time.Duration; +import java.util.Collection; import java.util.List; +import java.util.stream.Stream; import javax.inject.Inject; @@ -141,6 +143,14 @@ public class CassandraMailboxMapper implements MailboxMapper { .orElseThrow(() -> new MailboxNotFoundException(id)); } + @Override + public Stream<Mailbox> findMailboxesById(Collection<MailboxId> mailboxIds) { + return Flux.fromIterable(mailboxIds) + .map(CassandraId.class::cast) + .concatMap(this::retrieveMailbox) + .toStream(); + } + private Mono<Mailbox> retrieveMailbox(CassandraId mailboxId) { Mono<MailboxACL> acl = cassandraACLMapper.getACL(mailboxId); Mono<Mailbox> simpleMailbox = mailboxDAO.retrieveMailbox(mailboxId); diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java index c2da6e9..c3d7c02 100644 --- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java +++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java @@ -687,10 +687,10 @@ public class StoreMailboxManager implements MailboxManager { } private Stream<MailboxId> getInMailboxes(ImmutableSet<MailboxId> inMailboxes, MailboxSession session) throws MailboxException { - if (inMailboxes.isEmpty()) { + if (inMailboxes.isEmpty()) { return getAllReadableMailbox(session); } else { - return getAllReadableMailbox(session).filter(inMailboxes::contains); + return filterReadable(inMailboxes, session); } } @@ -700,6 +700,13 @@ public class StoreMailboxManager implements MailboxManager { .map(Mailbox::getMailboxId); } + private Stream<MailboxId> filterReadable(ImmutableSet<MailboxId> inMailboxes, MailboxSession session) throws MailboxException { + return mailboxSessionMapperFactory.getMailboxMapper(session) + .findMailboxesById(inMailboxes) + .filter(Throwing.<Mailbox>predicate(mailbox -> storeRightManager.hasRight(mailbox, Right.Read, session)).sneakyThrow()) + .map(Mailbox::getMailboxId); + } + @Override public Mono<Boolean> mailboxExists(MailboxPath mailboxPath, MailboxSession session) throws MailboxException { MailboxMapper mapper = mailboxSessionMapperFactory.getMailboxMapper(session); diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MailboxMapper.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MailboxMapper.java index f0df8d1..7ace127 100644 --- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MailboxMapper.java +++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MailboxMapper.java @@ -18,7 +18,9 @@ ****************************************************************/ package org.apache.james.mailbox.store.mail; +import java.util.Collection; import java.util.List; +import java.util.stream.Stream; import org.apache.james.core.Username; import org.apache.james.mailbox.acl.ACLDiff; @@ -33,6 +35,8 @@ import org.apache.james.mailbox.model.UidValidity; import org.apache.james.mailbox.model.search.MailboxQuery; import org.apache.james.mailbox.store.transaction.Mapper; +import com.github.fge.lambdas.Throwing; + import reactor.core.publisher.Mono; /** @@ -83,6 +87,17 @@ public interface MailboxMapper extends Mapper { Mailbox findMailboxById(MailboxId mailboxId) throws MailboxException, MailboxNotFoundException; + default Stream<Mailbox> findMailboxesById(Collection<MailboxId> mailboxIds) throws MailboxException { + return mailboxIds.stream() + .flatMap(Throwing.<MailboxId, Stream<Mailbox>>function(id -> { + try { + return Stream.of(findMailboxById(id)); + } catch (MailboxNotFoundException e) { + return Stream.empty(); + } + }).sneakyThrow()); + } + /** * Return a List of {@link Mailbox} for the given userName and matching the right */ diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java index 026e402..3fdf808 100644 --- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java +++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java @@ -25,6 +25,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.util.List; +import java.util.stream.Stream; import org.apache.james.core.Username; import org.apache.james.mailbox.exception.MailboxException; @@ -41,6 +42,8 @@ import org.apache.james.mailbox.store.mail.MailboxMapper; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import com.google.common.collect.ImmutableList; + /** * Generic purpose tests for your implementation MailboxMapper. * @@ -250,6 +253,38 @@ public abstract class MailboxMapperTest { Mailbox actual = mailboxMapper.findMailboxById(benwaInboxMailbox.getMailboxId()); assertThat(actual).isEqualTo(benwaInboxMailbox); } + + @Test + void findMailboxesByIdShouldReturnEmptyWhenNoIdSupplied() throws MailboxException { + createAll(); + + Stream<Mailbox> mailboxes = mailboxMapper.findMailboxesById(ImmutableList.of()); + + assertThat(mailboxes).isEmpty(); + } + + @Test + void findMailboxesByIdShouldReturnMailboxOfSuppliedId() throws MailboxException { + createAll(); + + Stream<Mailbox> mailboxes = mailboxMapper.findMailboxesById(ImmutableList.of( + benwaInboxMailbox.getMailboxId(), + benwaWorkMailbox.getMailboxId())); + + assertThat(mailboxes).containsOnly(benwaWorkMailbox, benwaInboxMailbox); + } + + @Test + void findMailboxesByIdShouldFilterOutNonExistingMailbox() throws MailboxException { + createAll(); + mailboxMapper.delete(benwaWorkMailbox); + + Stream<Mailbox> mailboxes = mailboxMapper.findMailboxesById(ImmutableList.of( + benwaInboxMailbox.getMailboxId(), + benwaWorkMailbox.getMailboxId())); + + assertThat(mailboxes).containsOnly(benwaInboxMailbox); + } @Test void findMailboxByIdShouldFailWhenAbsent() throws MailboxException { --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
