JAMES-1874 Upon mailbox creation avoid retrieving mailbox by path Retrieving mailbox by path in Cassandra relies on an Index
Also subscripbe only if creation succeeded Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/f2c58211 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/f2c58211 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/f2c58211 Branch: refs/heads/master Commit: f2c58211c9f554b6ec97ba8a0a045d7b1e31829c Parents: 45dfd90 Author: Benoit Tellier <btell...@linagora.com> Authored: Mon Feb 13 09:54:57 2017 +0700 Committer: Benoit Tellier <btell...@linagora.com> Committed: Mon Feb 20 16:06:24 2017 +0700 ---------------------------------------------------------------------- .../org/apache/james/mailbox/MailboxManager.java | 4 ++-- .../apache/james/util/OptionalConverterTest.java | 1 + .../james/jmap/methods/GetMailboxesMethod.java | 11 ++++++----- .../methods/SetMailboxesCreationProcessor.java | 7 ++++--- .../apache/james/jmap/model/MailboxFactory.java | 14 -------------- .../james/jmap/model/MailboxFactoryTest.java | 18 ------------------ 6 files changed, 13 insertions(+), 42 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/f2c58211/mailbox/api/src/main/java/org/apache/james/mailbox/MailboxManager.java ---------------------------------------------------------------------- diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/MailboxManager.java b/mailbox/api/src/main/java/org/apache/james/mailbox/MailboxManager.java index 3fcd03e..8e48df4 100644 --- a/mailbox/api/src/main/java/org/apache/james/mailbox/MailboxManager.java +++ b/mailbox/api/src/main/java/org/apache/james/mailbox/MailboxManager.java @@ -154,8 +154,8 @@ public interface MailboxManager extends RequestAware, MailboxListenerSupport { * the context for this call, not null * @throws MailboxException * when creation fails - * @return Empty optional when the name is empty. If mailbox is created, the id of the mailboxPath specified as - * parameter is returned (and not potential mailboxIds of parent mailboxes created in the process will be omitted) + * @return Empty optional when the mailbox name is empty. If mailbox is created, the id of the mailboxPath specified as + * parameter is returned (and potential mailboxIds of parent mailboxes created in the process will be omitted) */ Optional<MailboxId> createMailbox(MailboxPath mailboxPath, MailboxSession mailboxSession) throws MailboxException; http://git-wip-us.apache.org/repos/asf/james-project/blob/f2c58211/server/container/util-java8/src/test/java/org/apache/james/util/OptionalConverterTest.java ---------------------------------------------------------------------- diff --git a/server/container/util-java8/src/test/java/org/apache/james/util/OptionalConverterTest.java b/server/container/util-java8/src/test/java/org/apache/james/util/OptionalConverterTest.java index baa3aa0..5356f63 100644 --- a/server/container/util-java8/src/test/java/org/apache/james/util/OptionalConverterTest.java +++ b/server/container/util-java8/src/test/java/org/apache/james/util/OptionalConverterTest.java @@ -21,6 +21,7 @@ package org.apache.james.util; import static org.assertj.core.api.Assertions.assertThat; import java.util.Optional; +import java.util.stream.Collectors; import org.junit.Rule; import org.junit.Test; http://git-wip-us.apache.org/repos/asf/james-project/blob/f2c58211/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 be59d6e..58954f3 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 @@ -39,6 +39,7 @@ import org.apache.james.mailbox.exception.MailboxException; import org.apache.james.mailbox.model.MailboxId; import org.apache.james.mailbox.model.MailboxMetaData; import org.apache.james.mailbox.model.MailboxQuery; +import org.apache.james.util.OptionalConverter; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; @@ -91,8 +92,6 @@ public class GetMailboxesMethod implements Method { try { Optional<ImmutableList<MailboxId>> mailboxIds = mailboxesRequest.getIds(); retrieveMailboxes(mailboxIds, mailboxSession) - .filter(Optional::isPresent) - .map(Optional::get) .sorted(Comparator.comparing(Mailbox::getSortOrder)) .forEach(builder::add); return builder.build(); @@ -101,11 +100,12 @@ public class GetMailboxesMethod implements Method { } } - private Stream<Optional<Mailbox>> retrieveMailboxes(Optional<ImmutableList<MailboxId>> mailboxIds, MailboxSession mailboxSession) throws MailboxException { + private Stream<Mailbox> retrieveMailboxes(Optional<ImmutableList<MailboxId>> mailboxIds, MailboxSession mailboxSession) throws MailboxException { if (mailboxIds.isPresent()) { return mailboxIds.get() .stream() - .map(mailboxId -> mailboxFactory.fromMailboxId(mailboxId, mailboxSession)); + .map(mailboxId -> mailboxFactory.fromMailboxId(mailboxId, mailboxSession)) + .flatMap(OptionalConverter::toStream); } else { List<MailboxMetaData> userMailboxes = mailboxManager.search( MailboxQuery.builder(mailboxSession).privateUserMailboxes().build(), @@ -113,7 +113,8 @@ public class GetMailboxesMethod implements Method { return userMailboxes .stream() .map(MailboxMetaData::getId) - .map(mailboxId -> mailboxFactory.fromMailboxId(mailboxId, userMailboxes, mailboxSession)); + .map(mailboxId -> mailboxFactory.fromMailboxId(mailboxId, userMailboxes, mailboxSession)) + .flatMap(OptionalConverter::toStream); } } } http://git-wip-us.apache.org/repos/asf/james-project/blob/f2c58211/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesCreationProcessor.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesCreationProcessor.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesCreationProcessor.java index a4e0a28..40221e9 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesCreationProcessor.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/SetMailboxesCreationProcessor.java @@ -46,6 +46,7 @@ import org.apache.james.mailbox.exception.TooLongMailboxNameException; import org.apache.james.mailbox.model.MailboxId; import org.apache.james.mailbox.model.MailboxId.Factory; import org.apache.james.mailbox.model.MailboxPath; +import org.apache.james.util.OptionalConverter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -104,10 +105,10 @@ public class SetMailboxesCreationProcessor implements SetMailboxesProcessor { try { ensureValidMailboxName(mailboxRequest, mailboxSession); MailboxPath mailboxPath = getMailboxPath(mailboxRequest, creationIdsToCreatedMailboxId, mailboxSession); - mailboxManager.createMailbox(mailboxPath, mailboxSession); - subscriptionManager.subscribe(mailboxSession, mailboxPath.getName()); - Optional<Mailbox> mailbox = mailboxFactory.fromMailboxPath(mailboxPath, mailboxSession); + Optional<MailboxId> mailboxId = OptionalConverter.fromGuava(mailboxManager.createMailbox(mailboxPath, mailboxSession)); + Optional<Mailbox> mailbox = mailboxId.flatMap(id -> mailboxFactory.fromMailboxId(id, mailboxSession)); if (mailbox.isPresent()) { + subscriptionManager.subscribe(mailboxSession, mailboxPath.getName()); builder.created(mailboxCreationId, mailbox.get()); creationIdsToCreatedMailboxId.put(mailboxCreationId, mailbox.get().getId()); } else { http://git-wip-us.apache.org/repos/asf/james-project/blob/f2c58211/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 cb0c10b..b8a42c2 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 @@ -34,31 +34,17 @@ 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.slf4j.Logger; -import org.slf4j.LoggerFactory; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Splitter; public class MailboxFactory { - private static final Logger LOGGER = LoggerFactory.getLogger(MailboxFactory.class); - private final MailboxManager mailboxManager; @Inject public MailboxFactory(MailboxManager mailboxManager) { this.mailboxManager = mailboxManager; } - - public Optional<Mailbox> fromMailboxPath(MailboxPath mailboxPath, MailboxSession mailboxSession) { - try { - MessageManager mailbox = mailboxManager.getMailbox(mailboxPath, mailboxSession); - return fromMessageManager(mailbox, Optional.empty(), mailboxSession); - } catch (MailboxException e) { - LOGGER.warn("Cannot find mailbox for: " + mailboxPath.getName(), e); - return Optional.empty(); - } - } public Optional<Mailbox> fromMailboxId(MailboxId mailboxId, MailboxSession mailboxSession) { try { http://git-wip-us.apache.org/repos/asf/james-project/blob/f2c58211/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 7be9c73..3e83baf 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 @@ -75,24 +75,6 @@ public class MailboxFactoryTest { } @Test - public void mailboxFromMailboxPathShouldReturnNotEmptyWhenMailboxExists() throws Exception { - MailboxPath mailboxPath = new MailboxPath("#private", user, "mailbox"); - mailboxManager.createMailbox(mailboxPath, mailboxSession); - - Optional<Mailbox> optionalMailbox = sut.fromMailboxPath(mailboxPath, mailboxSession); - assertThat(optionalMailbox).isPresent(); - } - - @Test - public void mailboxFromMailboxPathShouldReturnEmptyWhenMailboxDoesntExist() throws Exception { - MailboxPath mailboxPath = new MailboxPath("#private", user, "mailbox"); - - Optional<Mailbox> optionalMailbox = sut.fromMailboxPath(mailboxPath, mailboxSession); - assertThat(optionalMailbox).isEmpty(); - } - - - @Test public void getNameShouldReturnMailboxNameWhenRootMailbox() throws Exception { String expected = "mailbox"; MailboxPath mailboxPath = new MailboxPath("#private", user, expected); --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
