You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by rc...@apache.org on 2021/01/05 04:55:53 UTC
[james-project] 14/24: JAMES-3485 Searching all mailboxes reads too
much ACLs
This is an automated email from the ASF dual-hosted git repository.
rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 0ad37ca0a5cb230f7fed2c8d76d0c49cfd5f2547
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Fri Dec 25 19:51:23 2020 +0700
JAMES-3485 Searching all mailboxes reads too much ACLs
Reference updater is called after sending a message. In order
to retrieve the message being replied/ forwarded and update its flags, a full mailbox search is being performed.
This unrestricted search was leading to unecessary ACL reads (1 ACL reads per mailbox accessible by the owner)
Upon Message search, we can skip safely
---
.../mailbox/model/MultimailboxesSearchQuery.java | 12 ++++++++
.../james/mailbox/store/StoreMailboxManager.java | 33 ++++++++++++++++------
.../james/mailbox/store/mail/MailboxMapper.java | 12 ++++++++
3 files changed, 49 insertions(+), 8 deletions(-)
diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/MultimailboxesSearchQuery.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/MultimailboxesSearchQuery.java
index 540f05f..4785111 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/MultimailboxesSearchQuery.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/MultimailboxesSearchQuery.java
@@ -40,6 +40,8 @@ public class MultimailboxesSearchQuery {
public interface Namespace {
boolean keepAccessible(Mailbox mailbox);
+ boolean accessDelegatedMailboxes();
+
MailboxQuery associatedMailboxSearchQuery();
}
@@ -61,6 +63,11 @@ public class MultimailboxesSearchQuery {
.matchesAllMailboxNames()
.build();
}
+
+ @Override
+ public boolean accessDelegatedMailboxes() {
+ return false;
+ }
}
public static class AccessibleNamespace implements Namespace {
@@ -83,6 +90,11 @@ public class MultimailboxesSearchQuery {
public boolean equals(Object obj) {
return obj instanceof AccessibleNamespace;
}
+
+ @Override
+ public boolean accessDelegatedMailboxes() {
+ return true;
+ }
}
public static class Builder {
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 23b3e25..3ec0836 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
@@ -688,6 +688,15 @@ public class StoreMailboxManager implements MailboxManager {
.filter(Throwing.predicate(mailbox -> storeRightManager.hasRight(mailbox, right, session)));
}
+ private Flux<MailboxId> accessibleMailboxIds(MultimailboxesSearchQuery.Namespace namespace, Right right, MailboxSession session) {
+ MailboxMapper mailboxMapper = mailboxSessionMapperFactory.getMailboxMapper(session);
+ Flux<MailboxId> baseMailboxes = mailboxMapper
+ .userMailboxes(session.getUser());
+ Flux<MailboxId> delegatedMailboxes = getDelegatedMailboxes(mailboxMapper, namespace, right, session);
+ return Flux.concat(baseMailboxes, delegatedMailboxes)
+ .distinct();
+ }
+
static MailboxQuery.UserBound toSingleUserQuery(MailboxQuery mailboxQuery, MailboxSession mailboxSession) {
return MailboxQuery.builder()
.namespace(mailboxQuery.getNamespace().orElse(MailboxConstants.USER_NAMESPACE))
@@ -706,6 +715,15 @@ public class StoreMailboxManager implements MailboxManager {
return mailboxMapper.findNonPersonalMailboxes(session.getUser(), right);
}
+ private Flux<MailboxId> getDelegatedMailboxes(MailboxMapper mailboxMapper, MultimailboxesSearchQuery.Namespace namespace,
+ Right right, MailboxSession session) {
+ if (!namespace.accessDelegatedMailboxes()) {
+ return Flux.empty();
+ }
+ return mailboxMapper.findNonPersonalMailboxes(session.getUser(), right)
+ .map(Mailbox::getMailboxId);
+ }
+
private MailboxMetaData toMailboxMetadata(MailboxSession session, List<Mailbox> mailboxes, Mailbox mailbox, MailboxCounters counters) throws UnsupportedRightException {
return new MailboxMetaData(
mailbox.generateAssociatedPath(),
@@ -732,20 +750,19 @@ public class StoreMailboxManager implements MailboxManager {
@Override
public Flux<MessageId> search(MultimailboxesSearchQuery expression, MailboxSession session, long limit) throws MailboxException {
- return getInMailboxes(expression, session)
- .filter(id -> !expression.getNotInMailboxes().contains(id.getMailboxId()))
- .filter(mailbox -> expression.getNamespace().keepAccessible(mailbox))
- .map(Mailbox::getMailboxId)
+ return getInMailboxIds(expression, session)
+ .filter(id -> !expression.getNotInMailboxes().contains(id))
.collect(Guavate.toImmutableSet())
.flatMapMany(Throwing.function(ids -> index.search(session, ids, expression.getSearchQuery(), limit)));
}
-
- private Flux<Mailbox> getInMailboxes(MultimailboxesSearchQuery expression, MailboxSession session) throws MailboxException {
+ private Flux<MailboxId> getInMailboxIds(MultimailboxesSearchQuery expression, MailboxSession session) throws MailboxException {
if (expression.getInMailboxes().isEmpty()) {
- return searchMailboxes(expression.getNamespace().associatedMailboxSearchQuery(), session, Right.Read);
+ return accessibleMailboxIds(expression.getNamespace(), Right.Read, session);
} else {
- return filterReadable(expression.getInMailboxes(), session);
+ return filterReadable(expression.getInMailboxes(), session)
+ .filter(mailbox -> expression.getNamespace().keepAccessible(mailbox))
+ .map(Mailbox::getMailboxId);
}
}
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MailboxMapper.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MailboxMapper.java
index 8743be6..a88d6a1 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MailboxMapper.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MailboxMapper.java
@@ -29,6 +29,7 @@ import org.apache.james.mailbox.model.MailboxId;
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.model.search.Wildcard;
import org.apache.james.mailbox.store.transaction.Mapper;
import reactor.core.publisher.Flux;
@@ -81,6 +82,17 @@ public interface MailboxMapper extends Mapper {
*/
Flux<Mailbox> findMailboxWithPathLike(MailboxQuery.UserBound query);
+ default Flux<MailboxId> userMailboxes(Username username) {
+ return findMailboxWithPathLike(
+ MailboxQuery.builder()
+ .privateNamespace()
+ .username(username)
+ .expression(Wildcard.INSTANCE)
+ .build()
+ .asUserBound())
+ .map(Mailbox::getMailboxId);
+ }
+
/**
* Return if the given {@link Mailbox} has children
*
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org