JAMES-1874 Avoid reading parent mailboxId when we already have it in memory
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/bdc76d13 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/bdc76d13 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/bdc76d13 Branch: refs/heads/master Commit: bdc76d13e2d6f4a02d8c27b6c2e738fbc25740d3 Parents: 052a344 Author: Benoit Tellier <btell...@linagora.com> Authored: Mon Feb 13 09:09:35 2017 +0700 Committer: Benoit Tellier <btell...@linagora.com> Committed: Mon Feb 20 16:05:39 2017 +0700 ---------------------------------------------------------------------- .../james/jmap/methods/GetMailboxesMethod.java | 11 +++--- .../apache/james/jmap/model/MailboxFactory.java | 38 +++++++++++++++----- .../james/jmap/model/MailboxFactoryTest.java | 27 ++++++++++++-- 3 files changed, 59 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/bdc76d13/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMailboxesMethod.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMailboxesMethod.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMailboxesMethod.java index d926d21..be59d6e 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMailboxesMethod.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMailboxesMethod.java @@ -90,8 +90,7 @@ public class GetMailboxesMethod implements Method { GetMailboxesResponse.Builder builder = GetMailboxesResponse.builder(); try { Optional<ImmutableList<MailboxId>> mailboxIds = mailboxesRequest.getIds(); - retrieveMailboxIds(mailboxIds, mailboxSession) - .map(mailboxId -> mailboxFactory.fromMailboxId(mailboxId, mailboxSession)) + retrieveMailboxes(mailboxIds, mailboxSession) .filter(Optional::isPresent) .map(Optional::get) .sorted(Comparator.comparing(Mailbox::getSortOrder)) @@ -102,17 +101,19 @@ public class GetMailboxesMethod implements Method { } } - private Stream<MailboxId> retrieveMailboxIds(Optional<ImmutableList<MailboxId>> mailboxIds, MailboxSession mailboxSession) throws MailboxException { + private Stream<Optional<Mailbox>> retrieveMailboxes(Optional<ImmutableList<MailboxId>> mailboxIds, MailboxSession mailboxSession) throws MailboxException { if (mailboxIds.isPresent()) { return mailboxIds.get() - .stream(); + .stream() + .map(mailboxId -> mailboxFactory.fromMailboxId(mailboxId, mailboxSession)); } else { List<MailboxMetaData> userMailboxes = mailboxManager.search( MailboxQuery.builder(mailboxSession).privateUserMailboxes().build(), mailboxSession); return userMailboxes .stream() - .map(MailboxMetaData::getId); + .map(MailboxMetaData::getId) + .map(mailboxId -> mailboxFactory.fromMailboxId(mailboxId, userMailboxes, mailboxSession)); } } } http://git-wip-us.apache.org/repos/asf/james-project/blob/bdc76d13/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MailboxFactory.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MailboxFactory.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MailboxFactory.java index 701d2fc..cb0c10b 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MailboxFactory.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/MailboxFactory.java @@ -53,7 +53,7 @@ public class MailboxFactory { public Optional<Mailbox> fromMailboxPath(MailboxPath mailboxPath, MailboxSession mailboxSession) { try { MessageManager mailbox = mailboxManager.getMailbox(mailboxPath, mailboxSession); - return fromMessageManager(mailbox, mailboxSession); + return fromMessageManager(mailbox, Optional.empty(), mailboxSession); } catch (MailboxException e) { LOGGER.warn("Cannot find mailbox for: " + mailboxPath.getName(), e); return Optional.empty(); @@ -63,20 +63,30 @@ public class MailboxFactory { public Optional<Mailbox> fromMailboxId(MailboxId mailboxId, MailboxSession mailboxSession) { try { MessageManager mailbox = mailboxManager.getMailbox(mailboxId, mailboxSession); - return fromMessageManager(mailbox, mailboxSession); + return fromMessageManager(mailbox, Optional.empty(), mailboxSession); } catch (MailboxException e) { return Optional.empty(); } } - private Optional<Mailbox> fromMessageManager(MessageManager messageManager, MailboxSession mailboxSession) throws MailboxException { + public Optional<Mailbox> fromMailboxId(MailboxId mailboxId, List<MailboxMetaData> userMailboxesMetadata, MailboxSession mailboxSession) { + try { + MessageManager mailbox = mailboxManager.getMailbox(mailboxId, mailboxSession); + return fromMessageManager(mailbox, Optional.of(userMailboxesMetadata), mailboxSession); + } catch (MailboxException e) { + return Optional.empty(); + } + } + + private Optional<Mailbox> fromMessageManager(MessageManager messageManager, Optional<List<MailboxMetaData>> userMailboxesMetadata, + MailboxSession mailboxSession) throws MailboxException { MailboxPath mailboxPath = messageManager.getMailboxPath(); Optional<Role> role = Role.from(mailboxPath.getName()); MailboxCounters mailboxCounters = messageManager.getMailboxCounters(mailboxSession); return Optional.ofNullable(Mailbox.builder() .id(messageManager.getId()) .name(getName(mailboxPath, mailboxSession)) - .parentId(getParentIdFromMailboxPath(mailboxPath, mailboxSession).orElse(null)) + .parentId(getParentIdFromMailboxPath(mailboxPath, userMailboxesMetadata, mailboxSession).orElse(null)) .role(role) .unreadMessages(mailboxCounters.getUnseen()) .totalMessages(mailboxCounters.getCount()) @@ -93,17 +103,27 @@ public class MailboxFactory { return name; } - @VisibleForTesting Optional<MailboxId> getParentIdFromMailboxPath(MailboxPath mailboxPath, MailboxSession mailboxSession) throws MailboxException { + @VisibleForTesting Optional<MailboxId> getParentIdFromMailboxPath(MailboxPath mailboxPath, Optional<List<MailboxMetaData>> userMailboxesMetadata, + MailboxSession mailboxSession) throws MailboxException { List<MailboxPath> levels = mailboxPath.getHierarchyLevels(mailboxSession.getPathDelimiter()); if (levels.size() <= 1) { return Optional.empty(); } MailboxPath parent = levels.get(levels.size() - 2); - return Optional.of(getMailboxId(parent, mailboxSession)); + return userMailboxesMetadata.map(list -> retrieveParentFromMetadata(parent, list)) + .orElse(retrieveParentFromBackend(mailboxSession, parent)); + } + + private Optional<MailboxId> retrieveParentFromBackend(MailboxSession mailboxSession, MailboxPath parent) throws MailboxException { + return Optional.of( + mailboxManager.getMailbox(parent, mailboxSession) + .getId()); } - private MailboxId getMailboxId(MailboxPath mailboxPath, MailboxSession mailboxSession) throws MailboxException { - return mailboxManager.getMailbox(mailboxPath, mailboxSession) - .getId(); + private Optional<MailboxId> retrieveParentFromMetadata(MailboxPath parent, List<MailboxMetaData> list) { + return list.stream() + .filter(metadata -> metadata.getPath().equals(parent)) + .map(MailboxMetaData::getId) + .findAny(); } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/james-project/blob/bdc76d13/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MailboxFactoryTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MailboxFactoryTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MailboxFactoryTest.java index 9964921..7be9c73 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MailboxFactoryTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/MailboxFactoryTest.java @@ -30,13 +30,17 @@ import org.apache.james.mailbox.inmemory.InMemoryId; import org.apache.james.mailbox.inmemory.manager.InMemoryIntegrationResources; import org.apache.james.mailbox.model.MailboxId; import org.apache.james.mailbox.model.MailboxPath; +import org.apache.james.mailbox.store.SimpleMailboxMetaData; import org.junit.Before; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.collect.ImmutableList; + public class MailboxFactoryTest { private static final Logger LOGGER = LoggerFactory.getLogger(MailboxUtilsTest.class); + public static final char DELIMITER = '.'; private MailboxManager mailboxManager; private MailboxSession mailboxSession; @@ -120,7 +124,7 @@ public class MailboxFactoryTest { MailboxPath mailboxPath = new MailboxPath("#private", user, "mailbox"); mailboxManager.createMailbox(mailboxPath, mailboxSession); - Optional<MailboxId> id = sut.getParentIdFromMailboxPath(mailboxPath, mailboxSession); + Optional<MailboxId> id = sut.getParentIdFromMailboxPath(mailboxPath, Optional.empty(), mailboxSession); assertThat(id).isEmpty(); } @@ -133,7 +137,7 @@ public class MailboxFactoryTest { MailboxPath mailboxPath = new MailboxPath("#private", user, "inbox.mailbox"); mailboxManager.createMailbox(mailboxPath, mailboxSession); - Optional<MailboxId> id = sut.getParentIdFromMailboxPath(mailboxPath, mailboxSession); + Optional<MailboxId> id = sut.getParentIdFromMailboxPath(mailboxPath, Optional.empty(), mailboxSession); assertThat(id).contains(parentId); } @@ -148,7 +152,24 @@ public class MailboxFactoryTest { mailboxManager.createMailbox(mailboxPath, mailboxSession); - Optional<MailboxId> id = sut.getParentIdFromMailboxPath(mailboxPath, mailboxSession); + Optional<MailboxId> id = sut.getParentIdFromMailboxPath(mailboxPath, Optional.empty(), mailboxSession); + assertThat(id).contains(parentId); + } + + @Test + public void getParentIdFromMailboxPathShouldWorkWhenUserMailboxesProvided() throws Exception { + MailboxPath mailboxPath = new MailboxPath("#private", user, "inbox.children.mailbox"); + mailboxManager.createMailbox(new MailboxPath("#private", user, "inbox"), mailboxSession); + + MailboxPath parentMailboxPath = new MailboxPath("#private", user, "inbox.children"); + mailboxManager.createMailbox(parentMailboxPath, mailboxSession); + MailboxId parentId = mailboxManager.getMailbox(parentMailboxPath, mailboxSession).getId(); + + mailboxManager.createMailbox(mailboxPath, mailboxSession); + + Optional<MailboxId> id = sut.getParentIdFromMailboxPath(mailboxPath, + Optional.of(ImmutableList.of(new SimpleMailboxMetaData(parentMailboxPath, parentId, DELIMITER))), + mailboxSession); assertThat(id).contains(parentId); } --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org