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 f6670e96f631ef64a526aacf2cf72ab351d8f01c
Author: Benoit Tellier <btell...@linagora.com>
AuthorDate: Thu Nov 7 14:46:45 2019 +0700
JAMES-2632 Optimize get all mailboxes
---
.../jmap/draft/methods/GetMailboxesMethod.java | 5 +-
.../james/jmap/draft/model/MailboxFactory.java | 76 ++++++++++++++++++----
.../james/jmap/draft/model/MailboxFactoryTest.java | 58 ++++++++++++++++-
3 files changed, 122 insertions(+), 17 deletions(-)
diff --git
a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetMailboxesMethod.java
b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetMailboxesMethod.java
index b53565f..64623d1 100644
---
a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetMailboxesMethod.java
+++
b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetMailboxesMethod.java
@@ -151,9 +151,8 @@ public class GetMailboxesMethod implements Method {
return userMailboxes
.stream()
- .map(MailboxMetaData::getId)
- .map(mailboxId -> mailboxFactory.builder()
- .id(mailboxId)
+ .map(mailboxMetaData -> mailboxFactory.builder()
+ .mailboxMetadata(mailboxMetaData)
.session(mailboxSession)
.usingPreloadedMailboxesMetadata(Optional.of(userMailboxes))
.quotaLoader(quotaLoader)
diff --git
a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/MailboxFactory.java
b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/MailboxFactory.java
index a8f097d..796cf59 100644
---
a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/MailboxFactory.java
+++
b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/MailboxFactory.java
@@ -37,16 +37,22 @@ import org.apache.james.mailbox.MessageManager;
import org.apache.james.mailbox.Role;
import org.apache.james.mailbox.exception.MailboxException;
import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxACL;
import org.apache.james.mailbox.model.MailboxCounters;
import org.apache.james.mailbox.model.MailboxId;
import org.apache.james.mailbox.model.MailboxMetaData;
import org.apache.james.mailbox.model.MailboxPath;
import org.apache.james.mailbox.quota.QuotaManager;
import org.apache.james.mailbox.quota.QuotaRootResolver;
+import org.apache.james.util.OptionalUtils;
+import com.github.fge.lambdas.Throwing;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
+import com.google.common.primitives.Booleans;
+
+import reactor.core.publisher.Mono;
public class MailboxFactory {
private final MailboxManager mailboxManager;
@@ -57,7 +63,8 @@ public class MailboxFactory {
private final MailboxFactory mailboxFactory;
private QuotaLoader quotaLoader;
private MailboxSession session;
- private MailboxId id;
+ private Optional<MailboxId> id = Optional.empty();
+ private Optional<MailboxMetaData> mailboxMetaData = Optional.empty();
private Optional<List<MailboxMetaData>> userMailboxesMetadata =
Optional.empty();
private MailboxBuilder(MailboxFactory mailboxFactory, QuotaLoader
quotaLoader) {
@@ -66,7 +73,12 @@ public class MailboxFactory {
}
public MailboxBuilder id(MailboxId id) {
- this.id = id;
+ this.id = Optional.of(id);
+ return this;
+ }
+
+ public MailboxBuilder mailboxMetadata(MailboxMetaData mailboxMetaData)
{
+ this.mailboxMetaData = Optional.of(mailboxMetaData);
return this;
}
@@ -86,18 +98,55 @@ public class MailboxFactory {
}
public Optional<Mailbox> build() {
- Preconditions.checkNotNull(id);
Preconditions.checkNotNull(session);
try {
- MessageManager mailbox =
mailboxFactory.mailboxManager.getMailbox(id, session);
- return Optional.of(mailboxFactory.fromMessageManager(mailbox,
userMailboxesMetadata, quotaLoader, session));
+ MailboxId mailboxId = computeMailboxId();
+ Mono<MessageManager> mailbox = mailbox(mailboxId).cache();
+
+ MailboxACL mailboxACL =
mailboxMetaData.map(MailboxMetaData::getResolvedAcls)
+ .orElseGet(Throwing.supplier(() ->
retrieveCachedMailbox(mailboxId,
mailbox).getResolvedAcl(session)).sneakyThrow());
+
+ MailboxPath mailboxPath =
mailboxMetaData.map(MailboxMetaData::getPath)
+ .orElseGet(Throwing.supplier(() ->
retrieveCachedMailbox(mailboxId, mailbox).getMailboxPath()).sneakyThrow());
+
+ MailboxCounters mailboxCounters =
mailboxMetaData.map(MailboxMetaData::getCounters)
+ .orElseGet(Throwing.supplier(() ->
retrieveCachedMailbox(mailboxId,
mailbox).getMailboxCounters(session)).sneakyThrow());
+
+ return Optional.of(mailboxFactory.from(
+ mailboxId,
+ mailboxPath,
+ mailboxCounters,
+ mailboxACL,
+ userMailboxesMetadata,
+ quotaLoader,
+ session));
} catch (MailboxNotFoundException e) {
return Optional.empty();
} catch (MailboxException e) {
throw new RuntimeException(e);
}
}
+
+ private MailboxId computeMailboxId() {
+ int idCount = Booleans.countTrue(id.isPresent(),
mailboxMetaData.isPresent());
+ Preconditions.checkState(idCount == 1, "You need exectly one 'id'
'mailboxMetaData'");
+ return OptionalUtils.or(
+ id,
+ mailboxMetaData.map(MailboxMetaData::getId))
+ .get();
+ }
+
+ private Mono<MessageManager> mailbox(MailboxId mailboxId) {
+ return Mono.fromCallable(() ->
mailboxFactory.mailboxManager.getMailbox(mailboxId, session));
+ }
+
+ private MessageManager retrieveCachedMailbox(MailboxId mailboxId,
Mono<MessageManager> mailbox) throws MailboxNotFoundException {
+ return mailbox
+ .onErrorResume(MailboxNotFoundException.class, any ->
Mono.empty())
+ .blockOptional()
+ .orElseThrow(() -> new MailboxNotFoundException(mailboxId));
+ }
}
@Inject
@@ -112,23 +161,24 @@ public class MailboxFactory {
return new MailboxBuilder(this, defaultQuotaLoader);
}
- private Mailbox fromMessageManager(MessageManager messageManager,
- Optional<List<MailboxMetaData>>
userMailboxesMetadata,
- QuotaLoader quotaLoader,
- MailboxSession mailboxSession) throws
MailboxException {
- MailboxPath mailboxPath = messageManager.getMailboxPath();
+ private Mailbox from(MailboxId mailboxId,
+ MailboxPath mailboxPath,
+ MailboxCounters mailboxCounters,
+ MailboxACL resolvedAcl,
+ Optional<List<MailboxMetaData>> userMailboxesMetadata,
+ QuotaLoader quotaLoader,
+ MailboxSession mailboxSession) throws
MailboxException {
boolean isOwner = mailboxPath.belongsTo(mailboxSession);
Optional<Role> role = Role.from(mailboxPath.getName());
- MailboxCounters mailboxCounters =
messageManager.getMailboxCounters(mailboxSession);
- Rights rights =
Rights.fromACL(messageManager.getResolvedAcl(mailboxSession))
+ Rights rights = Rights.fromACL(resolvedAcl)
.removeEntriesFor(mailboxPath.getUser());
Username username = mailboxSession.getUser();
Quotas quotas = quotaLoader.getQuotas(mailboxPath);
return Mailbox.builder()
- .id(messageManager.getId())
+ .id(mailboxId)
.name(getName(mailboxPath, mailboxSession))
.parentId(getParentIdFromMailboxPath(mailboxPath,
userMailboxesMetadata, mailboxSession).orElse(null))
.role(role)
diff --git
a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/model/MailboxFactoryTest.java
b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/model/MailboxFactoryTest.java
index a082e7e..f9c437a 100644
---
a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/model/MailboxFactoryTest.java
+++
b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/model/MailboxFactoryTest.java
@@ -18,13 +18,17 @@
****************************************************************/
package org.apache.james.jmap.draft.model;
+import static
org.apache.james.mailbox.manager.ManagerTestProvisionner.OTHER_USER;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.mock;
import java.util.Optional;
import org.apache.james.core.Username;
import org.apache.james.jmap.draft.model.mailbox.Mailbox;
import org.apache.james.jmap.draft.model.mailbox.MailboxNamespace;
+import org.apache.james.jmap.draft.model.mailbox.Rights;
import org.apache.james.mailbox.MailboxSession;
import org.apache.james.mailbox.inmemory.InMemoryId;
import org.apache.james.mailbox.inmemory.manager.InMemoryIntegrationResources;
@@ -34,6 +38,7 @@ import org.apache.james.mailbox.model.MailboxConstants;
import org.apache.james.mailbox.model.MailboxId;
import org.apache.james.mailbox.model.MailboxMetaData;
import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.search.MailboxQuery;
import org.apache.james.mailbox.quota.QuotaManager;
import org.apache.james.mailbox.quota.QuotaRootResolver;
import org.apache.james.mailbox.store.StoreMailboxManager;
@@ -43,6 +48,7 @@ import org.junit.Rule;
import org.junit.Test;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
public class MailboxFactoryTest {
public static final char DELIMITER = '.';
@@ -64,7 +70,7 @@ public class MailboxFactoryTest {
QuotaManager quotaManager =
mailboxManager.getQuotaComponents().getQuotaManager();
user = ManagerTestProvisionner.USER;
- otherUser = ManagerTestProvisionner.OTHER_USER;
+ otherUser = OTHER_USER;
mailboxSession = mailboxManager.login(user,
ManagerTestProvisionner.USER_PASS);
otherMailboxSession = mailboxManager.login(otherUser,
ManagerTestProvisionner.OTHER_USER_PASS);
sut = new MailboxFactory(mailboxManager, quotaManager,
quotaRootResolver);
@@ -306,4 +312,54 @@ public class MailboxFactoryTest {
softly.assertThat(retrievedMailbox.isMayRemoveItems()).isTrue();
softly.assertThat(retrievedMailbox.isMayRename()).isFalse();
}
+
+ @Test
+ public void mailboxFromMetaDataShouldReturnPresentStoredValue() throws
Exception {
+ String name = "myBox";
+ MailboxPath mailboxPath = MailboxPath.forUser(user, name);
+ mailboxManager.createMailbox(mailboxPath, mailboxSession);
+ mailboxManager.setRights(mailboxPath,
MailboxACL.EMPTY.apply(MailboxACL.command()
+ .forUser(OTHER_USER)
+ .rights(MailboxACL.Right.Lookup, MailboxACL.Right.Read)
+ .asAddition()),
+ mailboxSession);
+ MailboxMetaData metaData =
mailboxManager.search(MailboxQuery.privateMailboxesBuilder(mailboxSession).build(),
mailboxSession)
+ .stream()
+ .filter(metadata -> metadata.getPath().equals(mailboxPath))
+ .findFirst()
+ .get();
+
+ Optional<Mailbox> mailbox = sut.builder()
+ .mailboxMetadata(metaData)
+ .session(mailboxSession)
+ .build();
+
+ softly.assertThat(mailbox).isPresent();
+
softly.assertThat(mailbox).map(Mailbox::getId).contains(metaData.getId());
+ softly.assertThat(mailbox).map(Mailbox::getName).contains(name);
+
softly.assertThat(mailbox).map(Mailbox::getTotalMessages).contains(Number.ZERO);
+
softly.assertThat(mailbox).map(Mailbox::getUnreadMessages).contains(Number.ZERO);
+ softly.assertThat(mailbox).map(Mailbox::getSharedWith).contains(new
Rights(ImmutableMap.of(
+ OTHER_USER, ImmutableList.of(Rights.Right.Lookup,
Rights.Right.Read))));
+ }
+
+ @Test
+ public void buildShouldThrowWhenBothMetadataAndId() {
+ assertThatThrownBy(() ->
+ sut.builder()
+ .session(mailboxSession)
+ .id(mock(MailboxId.class))
+ .mailboxMetadata(mock(MailboxMetaData.class))
+ .build())
+ .isInstanceOf(IllegalStateException.class);
+ }
+
+ @Test
+ public void buildShouldThrowWhenNoId() {
+ assertThatThrownBy(() ->
+ sut.builder()
+ .session(mailboxSession)
+ .build())
+ .isInstanceOf(IllegalStateException.class);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
