You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2018/05/08 02:17:22 UTC

[3/7] james-project git commit: JAMES-2390 Accessible messages should only make one ACL call per mailbox

JAMES-2390 Accessible messages should only make one ACL call per mailbox

Upon checking whether a set of messages is accessible, the containing mailbox rights were checks on a per-mailbox base. This is sub-optimal as some messages might be in the same mailbox, whose
rights will be needlessly checked several times.

This change inserts smoothly into the codebase, the tools for checking rights once per mailbox is already implemented. Just not used in that case.


Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/9180da41
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/9180da41
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/9180da41

Branch: refs/heads/master
Commit: 9180da413956e9af659f4c0c00446405e93ba211
Parents: ce9321a
Author: benwa <bt...@linagora.com>
Authored: Sun May 6 09:16:19 2018 +0700
Committer: benwa <bt...@linagora.com>
Committed: Tue May 8 09:15:36 2018 +0700

----------------------------------------------------------------------
 .../mailbox/store/StoreMessageIdManager.java    | 23 ++++++++++++++------
 1 file changed, 16 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/james-project/blob/9180da41/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageIdManager.java
----------------------------------------------------------------------
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageIdManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageIdManager.java
index d65e6c6..bcda820 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageIdManager.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageIdManager.java
@@ -131,9 +131,12 @@ public class StoreMessageIdManager implements MessageIdManager {
     public Set<MessageId> accessibleMessages(Collection<MessageId> messageIds, MailboxSession mailboxSession) throws MailboxException {
         MessageIdMapper messageIdMapper = mailboxSessionMapperFactory.getMessageIdMapper(mailboxSession);
         List<MailboxMessage> messageList = messageIdMapper.find(messageIds, MessageMapper.FetchType.Metadata);
+
+        ImmutableSet<MailboxId> allowedMailboxIds = getAllowedMailboxIds(mailboxSession, messageList, Right.Read);
+
         return messageList.stream()
-            .filter(hasRightsOn(mailboxSession, Right.Read))
-            .map(message -> message.getComposedMessageIdWithMetaData().getComposedMessageId().getMessageId())
+            .filter(message -> allowedMailboxIds.contains(message.getMailboxId()))
+            .map(MailboxMessage::getMessageId)
             .collect(Guavate.toImmutableSet());
     }
 
@@ -142,11 +145,7 @@ public class StoreMessageIdManager implements MessageIdManager {
         MessageIdMapper messageIdMapper = mailboxSessionMapperFactory.getMessageIdMapper(mailboxSession);
         List<MailboxMessage> messageList = messageIdMapper.find(messageIds, MessageMapper.FetchType.Full);
 
-        ImmutableSet<MailboxId> allowedMailboxIds = messageList.stream()
-            .map(MailboxMessage::getMailboxId)
-            .distinct()
-            .filter(hasRightsOnMailbox(mailboxSession, Right.Read))
-            .collect(Guavate.toImmutableSet());
+        ImmutableSet<MailboxId> allowedMailboxIds = getAllowedMailboxIds(mailboxSession, messageList, Right.Read);
 
         return messageList.stream()
             .filter(inMailboxes(allowedMailboxIds))
@@ -154,6 +153,14 @@ public class StoreMessageIdManager implements MessageIdManager {
             .collect(Guavate.toImmutableList());
     }
 
+    private ImmutableSet<MailboxId> getAllowedMailboxIds(MailboxSession mailboxSession, List<MailboxMessage> messageList, Right... rights) {
+        return messageList.stream()
+                .map(MailboxMessage::getMailboxId)
+                .distinct()
+                .filter(hasRightsOnMailbox(mailboxSession, rights))
+                .collect(Guavate.toImmutableSet());
+    }
+
     @Override
     public void delete(MessageId messageId, List<MailboxId> mailboxIds, MailboxSession mailboxSession) throws MailboxException {
         MessageIdMapper messageIdMapper = mailboxSessionMapperFactory.getMessageIdMapper(mailboxSession);
@@ -212,6 +219,7 @@ public class StoreMessageIdManager implements MessageIdManager {
 
     private List<MailboxMessage> findRelatedMailboxMessages(MessageId messageId, MailboxSession mailboxSession) throws MailboxException {
         MessageIdMapper messageIdMapper = mailboxSessionMapperFactory.getMessageIdMapper(mailboxSession);
+
         return messageIdMapper.find(ImmutableList.of(messageId), MessageMapper.FetchType.Full)
             .stream()
             .filter(hasRightsOn(mailboxSession, Right.Read))
@@ -338,6 +346,7 @@ public class StoreMessageIdManager implements MessageIdManager {
         return message -> hasRightsOnMailbox(session, rights).test(message.getMailboxId());
     }
 
+
     private Predicate<MailboxId> hasRightsOnMailbox(MailboxSession session, Right... rights) {
         return Throwing.predicate((MailboxId mailboxId) -> mailboxManager.myRights(mailboxId, session).contains(rights))
             .fallbackTo(any -> false);


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org