You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2020/04/10 06:41:43 UTC
[james-project] 01/02: [Performance] Avoid reading all mailboxes
upon GetMessageList
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 <bt...@linagora.com>
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: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org