JAMES-1935: Optimize SetMessagesCreationProcessor. Do not search all mailboxes when look up mailbox by role
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/d12050e3 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/d12050e3 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/d12050e3 Branch: refs/heads/master Commit: d12050e30dcc475fa4a27341d7df203b4e9ae68c Parents: a4d8c8e Author: Quynh Nguyen <[email protected]> Authored: Fri Feb 10 14:29:52 2017 +0700 Committer: Antoine Duprat <[email protected]> Committed: Fri Feb 10 16:43:14 2017 +0100 ---------------------------------------------------------------------- .../apache/james/jmap/model/mailbox/Role.java | 33 ++++++-- .../jmap/utils/SystemMailboxesProviderImpl.java | 21 ++++- .../jmap/send/PostDequeueDecoratorTest.java | 8 +- .../utils/SystemMailboxesProviderImplTest.java | 87 ++------------------ 4 files changed, 55 insertions(+), 94 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/d12050e3/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/mailbox/Role.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/mailbox/Role.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/mailbox/Role.java index a29c168..7bc6bf1 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/mailbox/Role.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/mailbox/Role.java @@ -24,6 +24,8 @@ import java.util.Optional; import java.util.function.Function; import java.util.stream.Collectors; +import org.apache.james.jmap.DefaultMailboxes; + import com.fasterxml.jackson.annotation.JsonValue; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; @@ -34,12 +36,12 @@ public class Role { public static final String USER_DEFINED_ROLE_PREFIX = "x-"; - public static final Role INBOX = new Role("inbox"); + public static final Role INBOX = new Role("inbox", DefaultMailboxes.INBOX); + public static final Role DRAFTS = new Role("drafts", DefaultMailboxes.DRAFTS); + public static final Role OUTBOX = new Role("outbox", DefaultMailboxes.OUTBOX); + public static final Role SENT = new Role("sent", DefaultMailboxes.SENT); + public static final Role TRASH = new Role("trash", DefaultMailboxes.TRASH); public static final Role ARCHIVE = new Role("archive"); - public static final Role DRAFTS = new Role("drafts"); - public static final Role OUTBOX = new Role("outbox"); - public static final Role SENT = new Role("sent"); - public static final Role TRASH = new Role("trash"); public static final Role SPAM = new Role("spam"); public static final Role TEMPLATES = new Role("templates"); @@ -49,9 +51,16 @@ public class Role { .collect(Collectors.toMap((Role x) -> x.name.toLowerCase(Locale.ENGLISH), Function.identity())); private final String name; + private final String defaultMailbox; + + @VisibleForTesting Role(String name, String defaultMailbox) { + this.name = name; + this.defaultMailbox = defaultMailbox; + } @VisibleForTesting Role(String name) { this.name = name; + this.defaultMailbox = null; } public static Optional<Role> from(String name) { @@ -79,22 +88,30 @@ public class Role { return name; } + public String getDefaultMailbox() { + return defaultMailbox; + } + @Override public int hashCode() { - return Objects.hashCode(name); + return Objects.hashCode(name, defaultMailbox); } @Override public boolean equals(Object object) { if (object instanceof Role) { Role that = (Role) object; - return Objects.equal(this.name, that.name); + return Objects.equal(this.name, that.name) + && Objects.equal(this.defaultMailbox, that.defaultMailbox); } return false; } @Override public String toString() { - return MoreObjects.toStringHelper(this).add("name", name).toString(); + return MoreObjects.toStringHelper(this) + .add("name", name) + .add("defaultMailbox", defaultMailbox) + .toString(); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/d12050e3/server/protocols/jmap/src/main/java/org/apache/james/jmap/utils/SystemMailboxesProviderImpl.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/utils/SystemMailboxesProviderImpl.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/utils/SystemMailboxesProviderImpl.java index 181fafd..3a804ba 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/utils/SystemMailboxesProviderImpl.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/utils/SystemMailboxesProviderImpl.java @@ -28,6 +28,8 @@ import org.apache.james.mailbox.MailboxManager; import org.apache.james.mailbox.MailboxSession; import org.apache.james.mailbox.MessageManager; import org.apache.james.mailbox.exception.MailboxException; +import org.apache.james.mailbox.exception.MailboxNotFoundException; +import org.apache.james.mailbox.model.MailboxConstants; import org.apache.james.mailbox.model.MailboxMetaData; import org.apache.james.mailbox.model.MailboxPath; import org.apache.james.mailbox.model.MailboxQuery; @@ -48,13 +50,26 @@ public class SystemMailboxesProviderImpl implements SystemMailboxesProvider { private boolean hasRole(Role aRole, MailboxPath mailBoxPath) { return Role.from(mailBoxPath.getName()) - .map(aRole::equals) - .orElse(false); + .map(aRole::equals) + .orElse(false); } public Stream<MessageManager> listMailboxes(Role aRole, MailboxSession session) throws MailboxException { + MailboxPath mailboxPath = new MailboxPath(MailboxConstants.USER_NAMESPACE, session.getUser().getUserName(), aRole.getDefaultMailbox()); + try { + return Stream.of(mailboxManager.getMailbox(mailboxPath, session)); + } catch (MailboxNotFoundException e) { + return searchMessageManagerByMailboxRole(aRole, session); + } + } + + private Stream<MessageManager> searchMessageManagerByMailboxRole(Role aRole, MailboxSession session) throws MailboxException { ThrowingFunction<MailboxPath, MessageManager> loadMailbox = path -> mailboxManager.getMailbox(path, session); - return mailboxManager.search(MailboxQuery.builder(session).privateUserMailboxes().build(), session) + MailboxQuery mailboxQuery = MailboxQuery.builder(session) + .privateUserMailboxes() + .expression(aRole.getDefaultMailbox()) + .build(); + return mailboxManager.search(mailboxQuery, session) .stream() .map(MailboxMetaData::getPath) .filter(path -> hasRole(aRole, path)) http://git-wip-us.apache.org/repos/asf/james-project/blob/d12050e3/server/protocols/jmap/src/test/java/org/apache/james/jmap/send/PostDequeueDecoratorTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/send/PostDequeueDecoratorTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/send/PostDequeueDecoratorTest.java index 3c6fd2e..32c75d8 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/send/PostDequeueDecoratorTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/send/PostDequeueDecoratorTest.java @@ -18,17 +18,17 @@ ****************************************************************/ package org.apache.james.jmap.send; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.assertj.core.api.Assertions.assertThat; - import java.io.ByteArrayInputStream; import java.util.Date; import javax.mail.Flags; +import org.apache.james.jmap.DefaultMailboxes; import org.apache.james.jmap.exceptions.MailboxRoleNotFoundException; import org.apache.james.jmap.send.exception.MailShouldBeInOutboxException; import org.apache.james.jmap.utils.SystemMailboxesProviderImpl; @@ -55,8 +55,8 @@ import org.slf4j.LoggerFactory; public class PostDequeueDecoratorTest { private static final Logger LOGGER = LoggerFactory.getLogger(PostDequeueDecoratorTest.class); - private static final String OUTBOX = "OUTBOX"; - private static final String SENT = "SENT"; + private static final String OUTBOX = DefaultMailboxes.OUTBOX; + private static final String SENT = DefaultMailboxes.SENT; private static final String USERNAME = "[email protected]"; private static final MessageUid UID = MessageUid.of(1); private static final MailboxPath OUTBOX_MAILBOX_PATH = new MailboxPath(MailboxConstants.USER_NAMESPACE, USERNAME, OUTBOX); http://git-wip-us.apache.org/repos/asf/james-project/blob/d12050e3/server/protocols/jmap/src/test/java/org/apache/james/jmap/utils/SystemMailboxesProviderImplTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/utils/SystemMailboxesProviderImplTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/utils/SystemMailboxesProviderImplTest.java index 59633e8..7ea5d9d 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/utils/SystemMailboxesProviderImplTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/utils/SystemMailboxesProviderImplTest.java @@ -25,20 +25,16 @@ import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import java.util.stream.Stream; - -import org.apache.james.jmap.exceptions.MailboxRoleNotFoundException; import org.apache.james.jmap.model.mailbox.Role; import org.apache.james.mailbox.MailboxManager; import org.apache.james.mailbox.MailboxSession; import org.apache.james.mailbox.MessageManager; -import org.apache.james.mailbox.exception.MailboxException; +import org.apache.james.mailbox.exception.MailboxNotFoundException; import org.apache.james.mailbox.manager.MailboxManagerFixture; import org.apache.james.mailbox.mock.MockMailboxSession; 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.MailboxQuery; import org.apache.james.mailbox.model.TestId; import org.apache.james.mailbox.store.SimpleMailboxMetaData; import org.junit.Before; @@ -46,8 +42,6 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import com.google.common.collect.ImmutableList; - public class SystemMailboxesProviderImplTest { private static final MailboxPath INBOX = MailboxManagerFixture.MAILBOX_PATH1; @@ -79,83 +73,18 @@ public class SystemMailboxesProviderImplTest { } @Test - public void findMailboxesShouldReturnEmptyWhenEmptySearchResult() throws Exception { - when(mailboxManager.search(any(MailboxQuery.class), eq(mailboxSession))).thenReturn(ImmutableList.of()); + public void getMailboxByRoleShouldReturnEmptyWhenNoMailbox() throws Exception { + when(mailboxManager.getMailbox(eq(MailboxManagerFixture.MAILBOX_PATH1), eq(mailboxSession))).thenThrow(MailboxNotFoundException.class); assertThat(systemMailboxProvider.listMailboxes(Role.INBOX, mailboxSession)).isEmpty(); } @Test - public void findMailboxesShouldFilterTheMailboxByItsRole() throws Exception { - when(mailboxManager.search(any(MailboxQuery.class), eq(mailboxSession))).thenReturn(ImmutableList.of(inboxMetadata, outboxMetadata)); - when(mailboxManager.getMailbox(eq(INBOX), eq(mailboxSession))).thenReturn(inboxMessageManager); - - Stream<MessageManager> result = systemMailboxProvider.listMailboxes(Role.INBOX, mailboxSession); - - assertThat(result).hasSize(1).containsOnly(inboxMessageManager); - } - - @Test - @SuppressWarnings("unchecked") - public void findMailboxesShouldThrowWhenMailboxManagerHasErrorWhenSearching() throws Exception { - expectedException.expect(MailboxException.class); - - when(mailboxManager.search(any(MailboxQuery.class), eq(mailboxSession))).thenThrow(MailboxException.class); + public void getMailboxByRoleShouldReturnMailboxByRole() throws Exception { + when(mailboxManager.getMailbox(eq(MailboxManagerFixture.MAILBOX_PATH1), eq(mailboxSession))).thenReturn(inboxMessageManager); - systemMailboxProvider.listMailboxes(Role.INBOX, mailboxSession); + assertThat(systemMailboxProvider.listMailboxes(Role.INBOX, mailboxSession)) + .hasSize(1) + .containsOnly(inboxMessageManager); } - - @Test - @SuppressWarnings("unchecked") - public void findMailboxesShouldBeEmptyWhenMailboxManagerCanNotGetMailbox() throws Exception { - expectedException.expect(MailboxException.class); - - when(mailboxManager.search(any(MailboxQuery.class), eq(mailboxSession))).thenReturn(ImmutableList.of(inboxMetadata, outboxMetadata)); - when(mailboxManager.getMailbox(eq(INBOX), eq(mailboxSession))).thenThrow(MailboxException.class); - - assertThat(systemMailboxProvider.listMailboxes(Role.INBOX, mailboxSession)).isEmpty(); - } - - @Test - @SuppressWarnings("unchecked") - public void findMailboxesShouldReturnWhenMailboxManagerCanNotGetMailboxOfNonFilterMailbox() throws Exception { - when(mailboxManager.search(any(MailboxQuery.class), eq(mailboxSession))).thenReturn(ImmutableList.of(inboxMetadata, outboxMetadata)); - - when(mailboxManager.getMailbox(eq(INBOX), eq(mailboxSession))).thenReturn(inboxMessageManager); - when(mailboxManager.getMailbox(eq(OUTBOX), eq(mailboxSession))).thenThrow(MailboxException.class); - - Stream<MessageManager> result = systemMailboxProvider.listMailboxes(Role.INBOX, mailboxSession); - - assertThat(result).hasSize(1).containsOnly(inboxMessageManager); - - } - - @Test - public void findMailboxShouldThrowWhenEmptySearchResult() throws Exception { - expectedException.expect(MailboxRoleNotFoundException.class); - - when(mailboxManager.search(any(MailboxQuery.class), eq(mailboxSession))).thenReturn(ImmutableList.of()); - - systemMailboxProvider.findMailbox(Role.INBOX, mailboxSession); - } - - @Test - public void findMailboxShouldThrowWhenCanNotFindAny() throws Exception { - expectedException.expect(MailboxRoleNotFoundException.class); - - when(mailboxManager.search(any(MailboxQuery.class), eq(mailboxSession))).thenReturn(ImmutableList.of(outboxMetadata)); - - systemMailboxProvider.findMailbox(Role.INBOX, mailboxSession); - } - - @Test - public void findMailboxShouldReturnMailboxByRole() throws Exception { - when(mailboxManager.search(any(MailboxQuery.class), eq(mailboxSession))).thenReturn(ImmutableList.of(inboxMetadata, outboxMetadata)); - when(mailboxManager.getMailbox(eq(INBOX), eq(mailboxSession))).thenReturn(inboxMessageManager); - - MessageManager result = systemMailboxProvider.findMailbox(Role.INBOX, mailboxSession); - - assertThat(result).isEqualTo(inboxMessageManager); - } - } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
