You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2022/12/07 10:40:05 UTC
[james-project] 08/08: JAMES-3754 LIST STATUS should not read mailboxes information twice
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 f4a708488c9b3c3a3672912ac2d5ce0c24a80a57
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Mon Dec 5 18:44:11 2022 +0700
JAMES-3754 LIST STATUS should not read mailboxes information twice
---
.../org/apache/james/mailbox/MailboxManager.java | 2 ++
.../james/mailbox/model/MailboxMetaData.java | 12 +++++++---
.../james/mailbox/store/StoreMailboxManager.java | 27 ++++++++++++----------
.../apache/james/imap/processor/ListProcessor.java | 23 ++++++++++++++++--
.../james/imap/processor/StatusProcessor.java | 24 +++++++++++--------
.../james/jmap/draft/model/MailboxFactoryTest.java | 9 ++++----
.../webadmin/routes/UserMailboxesRoutesTest.java | 3 ++-
7 files changed, 68 insertions(+), 32 deletions(-)
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 1f1552ca12..8fc1cd8241 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
@@ -148,6 +148,8 @@ public interface MailboxManager extends RequestAware, RightManager, MailboxAnnot
Publisher<MessageManager> getMailboxReactive(MailboxPath mailboxPath, MailboxSession session);
+ MessageManager getMailbox(Mailbox mailbox, MailboxSession session) throws MailboxException;
+
/**
* Creates a new mailbox. Any intermediary mailboxes missing from the
* hierarchy should be created.
diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxMetaData.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxMetaData.java
index dd78368bbc..beb5bba8ba 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxMetaData.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxMetaData.java
@@ -64,6 +64,7 @@ public class MailboxMetaData implements Comparable<MailboxMetaData> {
.<MailboxMetaData, Boolean>comparing(metadata -> metadata.getPath().isInbox()).reversed()
.thenComparing(metadata -> metadata.getPath().getName());
+ private final Mailbox mailbox;
private final MailboxPath path;
private final char delimiter;
private final Children inferiors;
@@ -72,9 +73,10 @@ public class MailboxMetaData implements Comparable<MailboxMetaData> {
private final MailboxACL resolvedAcls;
private final MailboxCounters counters;
- public MailboxMetaData(MailboxPath path, MailboxId mailboxId, char delimiter, Children inferiors, Selectability selectability, MailboxACL resolvedAcls, MailboxCounters counters) {
- this.path = path;
- this.mailboxId = mailboxId;
+ public MailboxMetaData(Mailbox mailbox, char delimiter, Children inferiors, Selectability selectability, MailboxACL resolvedAcls, MailboxCounters counters) {
+ this.mailbox = mailbox;
+ this.mailboxId = mailbox.getMailboxId();
+ this.path = mailbox.generateAssociatedPath();
this.delimiter = delimiter;
this.inferiors = inferiors;
this.selectability = selectability;
@@ -127,6 +129,10 @@ public class MailboxMetaData implements Comparable<MailboxMetaData> {
return mailboxId;
}
+ public Mailbox getMailbox() {
+ return mailbox;
+ }
+
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
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 01eec15d01..9577ac410a 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
@@ -284,22 +284,26 @@ public class StoreMailboxManager implements MailboxManager {
MailboxMapper mapper = mailboxSessionMapperFactory.getMailboxMapper(session);
return mapper.findMailboxByPath(mailboxPath)
- .map(Throwing.<Mailbox, MessageManager>function(mailboxRow -> {
- if (!assertUserHasAccessTo(mailboxRow, session)) {
- LOGGER.info("Mailbox '{}' does not belong to user '{}' but to '{}'", mailboxPath, session.getUser(), mailboxRow.getUser());
- throw new MailboxNotFoundException(mailboxPath);
- }
-
- LOGGER.debug("Loaded mailbox {}", mailboxPath);
-
- return createMessageManager(mailboxRow, session);
- }).sneakyThrow())
+ .map(Throwing.<Mailbox, MessageManager>function(mailboxRow -> getMailbox(mailboxRow, session)).sneakyThrow())
.switchIfEmpty(Mono.fromCallable(() -> {
LOGGER.debug("Mailbox '{}' not found.", mailboxPath);
throw new MailboxNotFoundException(mailboxPath);
}));
}
+ @Override
+ public MessageManager getMailbox(Mailbox mailboxRow, MailboxSession session) throws MailboxException {
+ MailboxPath mailboxPath = mailboxRow.generateAssociatedPath();
+ if (!assertUserHasAccessTo(mailboxRow, session)) {
+ LOGGER.info("Mailbox '{}' does not belong to user '{}' but to '{}'", mailboxPath, session.getUser(), mailboxRow.getUser());
+ throw new MailboxNotFoundException(mailboxPath);
+ }
+
+ LOGGER.debug("Loaded mailbox {}", mailboxPath);
+
+ return createMessageManager(mailboxRow, session);
+ }
+
@Override
public MessageManager getMailbox(MailboxId mailboxId, MailboxSession session) throws MailboxException {
return block(getMailboxReactive(mailboxId, session));
@@ -837,8 +841,7 @@ public class StoreMailboxManager implements MailboxManager {
private MailboxMetaData toMailboxMetadata(MailboxSession session, Map<MailboxPath, Boolean> parentMap, Mailbox mailbox, MailboxCounters counters) throws UnsupportedRightException {
return new MailboxMetaData(
- mailbox.generateAssociatedPath(),
- mailbox.getMailboxId(),
+ mailbox,
getDelimiter(),
computeChildren(parentMap, mailbox),
Selectability.NONE,
diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java
index 8791fbcab8..559529b9ba 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java
@@ -45,9 +45,11 @@ import org.apache.james.imap.api.process.MailboxTyper;
import org.apache.james.imap.main.PathConverter;
import org.apache.james.imap.message.request.ListRequest;
import org.apache.james.imap.message.response.ListResponse;
+import org.apache.james.imap.message.response.MailboxStatusResponse;
import org.apache.james.imap.message.response.MyRightsResponse;
import org.apache.james.mailbox.MailboxManager;
import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
import org.apache.james.mailbox.SubscriptionManager;
import org.apache.james.mailbox.exception.MailboxException;
import org.apache.james.mailbox.model.MailboxACL;
@@ -221,10 +223,18 @@ public class ListProcessor<T extends ListRequest> extends AbstractMailboxProcess
}
})
.doOnNext(metaData -> respondMyRights(request, responder, mailboxSession, metaData))
- .flatMap(metaData -> request.getStatusDataItems().map(statusDataItems -> statusProcessor.sendStatus(metaData.getPath(), statusDataItems, responder, session, mailboxSession)).orElse(Mono.empty()))
+ .flatMap(metaData -> request.getStatusDataItems().map(statusDataItems -> statusProcessor.sendStatus(retrieveMessageManager(metaData, mailboxSession), statusDataItems, responder, session, mailboxSession)).orElse(Mono.empty()))
.then();
}
+ private MessageManager retrieveMessageManager(MailboxMetaData metaData, MailboxSession mailboxSession) {
+ try {
+ return getMailboxManager().getMailbox(metaData.getMailbox(), mailboxSession);
+ } catch (MailboxException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
private Mono<Void> processWithSubscribed(ImapSession session, T request, Responder responder, MailboxSession mailboxSession, boolean isRelative, MailboxQuery mailboxQuery) {
return Mono.zip(getMailboxManager().search(mailboxQuery, Minimal, mailboxSession).collectList()
.map(searchedResultList -> searchedResultList.stream().collect(Collectors.toMap(MailboxMetaData::getPath, Function.identity()))),
@@ -233,10 +243,19 @@ public class ListProcessor<T extends ListRequest> extends AbstractMailboxProcess
.flatMapIterable(list -> list)
.doOnNext(pathAndResponse -> responder.respond(pathAndResponse.getMiddle()))
.doOnNext(pathAndResponse -> pathAndResponse.getRight().ifPresent(mailboxMetaData -> respondMyRights(request, responder, mailboxSession, mailboxMetaData)))
- .flatMap(pathAndResponse -> request.getStatusDataItems().map(statusDataItems -> statusProcessor.sendStatus(pathAndResponse.getLeft(), statusDataItems, responder, session, mailboxSession)).orElse(Mono.empty()))
+ .flatMap(pathAndResponse -> sendStatusWhenSubscribed(session, request, responder, mailboxSession, pathAndResponse))
.then();
}
+ private Mono<MailboxStatusResponse> sendStatusWhenSubscribed(ImapSession session, T request, Responder responder, MailboxSession mailboxSession,
+ Triple<MailboxPath, ListResponse, Optional<MailboxMetaData>> pathAndResponse) {
+ return pathAndResponse.getRight()
+ .map(metaData -> retrieveMessageManager(metaData, mailboxSession))
+ .flatMap(messageManager -> request.getStatusDataItems()
+ .map(statusDataItems -> statusProcessor.sendStatus(messageManager, statusDataItems, responder, session, mailboxSession)))
+ .orElse(Mono.empty());
+ }
+
private List<Triple<MailboxPath, ListResponse, Optional<MailboxMetaData>>> getListResponseForSelectSubscribed(ImapSession session, Map<MailboxPath, MailboxMetaData> searchedResultMap, List<MailboxPath> allSubscribedSearch,
ListRequest listRequest, MailboxSession mailboxSession, boolean relative, MailboxQuery mailboxQuery) {
ImmutableList.Builder<Triple<MailboxPath, ListResponse, Optional<MailboxMetaData>>> responseBuilders = ImmutableList.builder();
diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/StatusProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/StatusProcessor.java
index 541f84f421..1b513bd1a7 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/processor/StatusProcessor.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/StatusProcessor.java
@@ -107,17 +107,21 @@ public class StatusProcessor extends AbstractMailboxProcessor<StatusRequest> imp
.then();
}
- Mono<MailboxStatusResponse> sendStatus(MailboxPath mailboxPath, StatusDataItems statusDataItems, Responder responder, ImapSession session, MailboxSession mailboxSession) {
+ private Mono<MailboxStatusResponse> sendStatus(MailboxPath mailboxPath, StatusDataItems statusDataItems, Responder responder, ImapSession session, MailboxSession mailboxSession) {
return Mono.from(getMailboxManager().getMailboxReactive(mailboxPath, mailboxSession))
- .flatMap(mailbox -> retrieveMetadata(mailbox, statusDataItems, mailboxSession)
- .flatMap(metaData -> computeStatusResponse(mailbox, statusDataItems, metaData, mailboxSession)
- .doOnNext(response -> {
- // Enable CONDSTORE as this is a CONDSTORE enabling command
- if (response.getHighestModSeq() != null) {
- condstoreEnablingCommand(session, responder, metaData, false);
- }
- responder.respond(response);
- })));
+ .flatMap(mailbox -> sendStatus(mailbox, statusDataItems, responder, session, mailboxSession));
+ }
+
+ Mono<MailboxStatusResponse> sendStatus(MessageManager mailbox, StatusDataItems statusDataItems, Responder responder, ImapSession session, MailboxSession mailboxSession) {
+ return retrieveMetadata(mailbox, statusDataItems, mailboxSession)
+ .flatMap(metaData -> computeStatusResponse(mailbox, statusDataItems, metaData, mailboxSession)
+ .doOnNext(response -> {
+ // Enable CONDSTORE as this is a CONDSTORE enabling command
+ if (response.getHighestModSeq() != null) {
+ condstoreEnablingCommand(session, responder, metaData, false);
+ }
+ responder.respond(response);
+ }));
}
private Mono<Void> logInitialRequest(MailboxPath mailboxPath) {
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 41f25eeb47..2d7edee171 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
@@ -38,6 +38,7 @@ 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.model.UidValidity;
import org.apache.james.mailbox.model.search.MailboxQuery;
import org.apache.james.mailbox.quota.QuotaManager;
import org.apache.james.mailbox.quota.QuotaRootResolver;
@@ -179,8 +180,9 @@ public class MailboxFactoryTest {
mailboxManager.createMailbox(mailboxPath, mailboxSession);
+ org.apache.james.mailbox.model.Mailbox mailbox = new org.apache.james.mailbox.model.Mailbox(parentMailboxPath, UidValidity.of(34), parentId);
Optional<MailboxId> id = sut.getParentIdFromMailboxPath(mailboxPath,
- Optional.of(ImmutableMap.of(parentMailboxPath, new MailboxMetaData(parentMailboxPath, parentId, DELIMITER,
+ Optional.of(ImmutableMap.of(parentMailboxPath, new MailboxMetaData(mailbox, DELIMITER,
MailboxMetaData.Children.CHILDREN_ALLOWED_BUT_UNKNOWN, MailboxMetaData.Selectability.NONE, new MailboxACL(),
MailboxCounters.empty(parentId)))),
mailboxSession).block();
@@ -205,16 +207,15 @@ public class MailboxFactoryTest {
@Test
public void buildShouldRelyOnPreloadedMailboxes() throws Exception {
MailboxPath inbox = MailboxPath.inbox(user);
- Optional<MailboxId> inboxId = mailboxManager.createMailbox(inbox, mailboxSession);
Optional<MailboxId> otherId = mailboxManager.createMailbox(inbox.child("child", '.'), mailboxSession);
InMemoryId preLoadedId = InMemoryId.of(45);
+ final org.apache.james.mailbox.model.Mailbox mailbox = new org.apache.james.mailbox.model.Mailbox(inbox, UidValidity.of(34), preLoadedId);
Mailbox retrievedMailbox = sut.builder()
.id(otherId.get())
.session(mailboxSession)
.usingPreloadedMailboxesMetadata(Optional.of(ImmutableMap.of(inbox, new MailboxMetaData(
- inbox,
- preLoadedId,
+ mailbox,
DELIMITER,
MailboxMetaData.Children.NO_INFERIORS,
MailboxMetaData.Selectability.NONE,
diff --git a/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java b/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
index 9b157ad7a8..9a818bc828 100644
--- a/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
+++ b/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
@@ -86,6 +86,7 @@ import org.apache.james.mailbox.model.MailboxMetaData;
import org.apache.james.mailbox.model.MailboxPath;
import org.apache.james.mailbox.model.MessageResult;
import org.apache.james.mailbox.model.ThreadId;
+import org.apache.james.mailbox.model.UidValidity;
import org.apache.james.mailbox.model.UpdatedFlags;
import org.apache.james.mailbox.model.search.MailboxQuery;
import org.apache.james.mailbox.opensearch.IndexAttachments;
@@ -140,7 +141,7 @@ class UserMailboxesRoutesTest {
private static final Logger LOGGER = LoggerFactory.getLogger(UserMailboxesRoutesTest.class);
public static MailboxMetaData testMetadata(MailboxPath path, MailboxId mailboxId, char delimiter) {
- return new MailboxMetaData(path, mailboxId, delimiter, MailboxMetaData.Children.CHILDREN_ALLOWED_BUT_UNKNOWN, MailboxMetaData.Selectability.NONE, new MailboxACL(),
+ return new MailboxMetaData(new Mailbox(path, UidValidity.of(45), mailboxId), delimiter, MailboxMetaData.Children.CHILDREN_ALLOWED_BUT_UNKNOWN, MailboxMetaData.Selectability.NONE, new MailboxACL(),
MailboxCounters.empty(mailboxId));
}
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org