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 2023/04/06 02:17:25 UTC

[james-project] branch master updated (7b064d702c -> 7c63388313)

This is an automated email from the ASF dual-hosted git repository.

rcordier pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git


    from 7b064d702c [FIX] Ranged search in CassandraFirstUnseenDAO was including too much results
     new 26239da838 RightManager should support reactive api
     new 1d7dfd874e IMAP - Make reactive SetACLProcessor
     new 7c63388313 IMAP - Make reactive DeleteACLProcessor

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../org/apache/james/mailbox/RightManager.java     |   6 +
 .../james/mailbox/store/StoreMailboxManager.java   |  15 +++
 .../james/mailbox/store/StoreRightManager.java     |  75 ++++++-----
 .../james/mailbox/store/StoreRightManagerTest.java |   4 +-
 .../james/imap/processor/DeleteACLProcessor.java   | 124 ++++++++---------
 .../james/imap/processor/SetACLProcessor.java      | 150 +++++++++++----------
 .../imap/processor/DeleteACLProcessorTest.java     |  39 +++---
 .../james/imap/processor/SetACLProcessorTest.java  |  42 +++---
 8 files changed, 243 insertions(+), 212 deletions(-)


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


[james-project] 01/03: RightManager should support reactive api

Posted by rc...@apache.org.
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 26239da838ca51d766b4416840afdee65fef5dd5
Author: Tung Van TRAN <vt...@linagora.com>
AuthorDate: Wed Mar 29 16:44:45 2023 +0700

    RightManager should support reactive api
---
 .../org/apache/james/mailbox/RightManager.java     |  6 ++
 .../james/mailbox/store/StoreMailboxManager.java   | 15 +++++
 .../james/mailbox/store/StoreRightManager.java     | 75 +++++++++++++---------
 .../james/mailbox/store/StoreRightManagerTest.java |  4 +-
 4 files changed, 66 insertions(+), 34 deletions(-)

diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/RightManager.java b/mailbox/api/src/main/java/org/apache/james/mailbox/RightManager.java
index 2eb2a2721d..01fa333a44 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/RightManager.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/RightManager.java
@@ -45,6 +45,8 @@ public interface RightManager {
      */
     boolean hasRight(MailboxPath mailboxPath, Right right, MailboxSession session) throws MailboxException;
 
+    Publisher<Boolean> hasRightReactive(MailboxPath mailboxPath, Right right, MailboxSession session);
+
     boolean hasRight(Mailbox mailbox, Right right, MailboxSession session) throws MailboxException;
 
     /**
@@ -104,6 +106,8 @@ public interface RightManager {
      */
     Rfc4314Rights myRights(MailboxPath mailboxPath, MailboxSession session) throws MailboxException;
 
+    Publisher<Rfc4314Rights> myRightsReactive(MailboxPath mailboxPath, MailboxSession session);
+
     /**
      * Returns the rights applicable to the user who has sent the current
      * request on the mailbox designated by this mailboxPath.
@@ -140,6 +144,8 @@ public interface RightManager {
      */
     void applyRightsCommand(MailboxPath mailboxPath, MailboxACL.ACLCommand mailboxACLCommand, MailboxSession session) throws MailboxException;
 
+    Publisher<Void> applyRightsCommandReactive(MailboxPath mailboxPath, MailboxACL.ACLCommand mailboxACLCommand, MailboxSession session);
+
     /**
      * Update the Mailbox ACL of the designated mailbox. We can either ADD REPLACE or REMOVE entries.
      *
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 1a5304e0e1..788c53b47e 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
@@ -937,6 +937,11 @@ public class StoreMailboxManager implements MailboxManager {
         return storeRightManager.hasRight(mailboxPath, right, session);
     }
 
+    @Override
+    public Publisher<Boolean> hasRightReactive(MailboxPath mailboxPath, Right right, MailboxSession session) {
+        return storeRightManager.hasRightReactive(mailboxPath, right, session);
+    }
+
     @Override
     public boolean hasRight(Mailbox mailbox, Right right, MailboxSession session) throws MailboxException {
         return storeRightManager.hasRight(mailbox, right, session);
@@ -952,6 +957,11 @@ public class StoreMailboxManager implements MailboxManager {
         return storeRightManager.myRights(mailboxPath, session);
     }
 
+    @Override
+    public Publisher<Rfc4314Rights> myRightsReactive(MailboxPath mailboxPath, MailboxSession session) {
+        return storeRightManager.myRightsReactive(mailboxPath, session);
+    }
+
     @Override
     public Mono<Rfc4314Rights> myRights(MailboxId mailboxId, MailboxSession session) {
         return storeRightManager.myRights(mailboxId, session);
@@ -987,6 +997,11 @@ public class StoreMailboxManager implements MailboxManager {
         storeRightManager.applyRightsCommand(mailboxPath, mailboxACLCommand, session);
     }
 
+    @Override
+    public Publisher<Void> applyRightsCommandReactive(MailboxPath mailboxPath, MailboxACL.ACLCommand mailboxACLCommand, MailboxSession session) {
+        return storeRightManager.applyRightsCommandReactive(mailboxPath, mailboxACLCommand, session);
+    }
+
     @Override
     public void applyRightsCommand(MailboxId mailboxId, MailboxACL.ACLCommand mailboxACLCommand, MailboxSession session) throws MailboxException {
         storeRightManager.applyRightsCommand(mailboxId, mailboxACLCommand, session);
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreRightManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreRightManager.java
index d8034cba36..5c4b20b07f 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreRightManager.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreRightManager.java
@@ -34,6 +34,7 @@ import org.apache.james.core.Username;
 import org.apache.james.events.EventBus;
 import org.apache.james.mailbox.MailboxSession;
 import org.apache.james.mailbox.RightManager;
+import org.apache.james.mailbox.acl.ACLDiff;
 import org.apache.james.mailbox.acl.MailboxACLResolver;
 import org.apache.james.mailbox.events.MailboxIdRegistrationKey;
 import org.apache.james.mailbox.exception.DifferentDomainException;
@@ -78,6 +79,13 @@ public class StoreRightManager implements RightManager {
         return myRights(mailboxPath, session).contains(right);
     }
 
+    @Override
+    public Mono<Boolean> hasRightReactive(MailboxPath mailboxPath, Right right, MailboxSession session) {
+        return myRightsReactive(mailboxPath, session)
+            .filter(rights -> rights.contains(right))
+            .hasElement();
+    }
+
     @Override
     public boolean hasRight(MailboxId mailboxId, Right right, MailboxSession session) throws MailboxException {
         return block(Mono.from(myRights(mailboxId, session))).contains(right);
@@ -90,17 +98,20 @@ public class StoreRightManager implements RightManager {
 
     @Override
     public Rfc4314Rights myRights(MailboxPath mailboxPath, MailboxSession session) throws MailboxException {
-        if (mailboxPath.belongsTo(session)) {
-            if (mailboxSessionMapperFactory.getMailboxMapper(session).pathExists(mailboxPath).block()) {
-                return MailboxACL.FULL_RIGHTS;
-            } else {
-                throw new MailboxNotFoundException(mailboxPath);
-            }
-        }
-        MailboxMapper mapper = mailboxSessionMapperFactory.getMailboxMapper(session);
-        Mailbox mailbox = blockOptional(mapper.findMailboxByPath(mailboxPath))
-            .orElseThrow(() -> new MailboxNotFoundException(mailboxPath));
-        return myRights(mailbox, session);
+        return MailboxReactorUtils.block(myRightsReactive(mailboxPath, session));
+    }
+
+    @Override
+    public Mono<Rfc4314Rights> myRightsReactive(MailboxPath mailboxPath, MailboxSession session) {
+        return mailboxSessionMapperFactory.getMailboxMapper(session)
+            .findMailboxByPath(mailboxPath)
+            .map(mailbox -> {
+                if (mailboxPath.belongsTo(session)) {
+                    return MailboxACL.FULL_RIGHTS;
+                } else {
+                    return myRights(mailbox, session);
+                }
+            }).switchIfEmpty(Mono.error(new MailboxNotFoundException(mailboxPath)));
     }
 
     @Override
@@ -158,21 +169,27 @@ public class StoreRightManager implements RightManager {
 
     @Override
     public void applyRightsCommand(MailboxPath mailboxPath, ACLCommand mailboxACLCommand, MailboxSession session) throws MailboxException {
-        assertSharesBelongsToUserDomain(mailboxPath.getUser(), mailboxACLCommand);
-        MailboxMapper mapper = mailboxSessionMapperFactory.getMailboxMapper(session);
-        block(mapper.findMailboxByPath(mailboxPath)
-            .flatMap(Throwing.<Mailbox, Mono<Void>>function(mailbox -> {
-                assertHaveAccessTo(mailbox, session);
+        MailboxReactorUtils.block(applyRightsCommandReactive(mailboxPath, mailboxACLCommand, session));
+    }
 
-                return mapper.updateACL(mailbox, mailboxACLCommand)
-                    .flatMap(aclDiff -> eventBus.dispatch(EventFactory.aclUpdated()
-                            .randomEventId()
-                            .mailboxSession(session)
-                            .mailbox(mailbox)
-                            .aclDiff(aclDiff)
-                            .build(),
-                        new MailboxIdRegistrationKey(mailbox.getMailboxId())));
-            }).sneakyThrow()));
+    @Override
+    public Mono<Void> applyRightsCommandReactive(MailboxPath mailboxPath, ACLCommand mailboxACLCommand, MailboxSession session) {
+        return Mono.just(mailboxSessionMapperFactory.getMailboxMapper(session))
+            .doOnNext(Throwing.consumer(mapper -> assertSharesBelongsToUserDomain(mailboxPath.getUser(), mailboxACLCommand)))
+            .flatMap(mapper -> mapper.findMailboxByPath(mailboxPath)
+                .doOnNext(Throwing.consumer(mailbox -> assertHaveAccessTo(mailbox, session)))
+                .flatMap(mailbox -> mapper.updateACL(mailbox, mailboxACLCommand)
+                    .flatMap(aclDiff -> dispatchACLUpdateEvent(session, mailbox, aclDiff))));
+    }
+
+    private Mono<Void> dispatchACLUpdateEvent(MailboxSession session, Mailbox mailbox, ACLDiff aclDiff) {
+        return eventBus.dispatch(EventFactory.aclUpdated()
+                .randomEventId()
+                .mailboxSession(session)
+                .mailbox(mailbox)
+                .aclDiff(aclDiff)
+                .build(),
+            new MailboxIdRegistrationKey(mailbox.getMailboxId()));
     }
 
     @Override
@@ -275,13 +292,7 @@ public class StoreRightManager implements RightManager {
 
     private Mono<Void> setRights(MailboxACL mailboxACL, MailboxMapper mapper, Mailbox mailbox, MailboxSession session) {
         return mapper.setACL(mailbox, mailboxACL)
-            .flatMap(aclDiff -> eventBus.dispatch(EventFactory.aclUpdated()
-                    .randomEventId()
-                    .mailboxSession(session)
-                    .mailbox(mailbox)
-                    .aclDiff(aclDiff)
-                    .build(),
-                new MailboxIdRegistrationKey(mailbox.getMailboxId())));
+            .flatMap(aclDiff -> dispatchACLUpdateEvent(session, mailbox, aclDiff));
     }
 
     /**
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreRightManagerTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreRightManagerTest.java
index 0dad3c0519..f9360efe97 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreRightManagerTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreRightManagerTest.java
@@ -82,8 +82,8 @@ class StoreRightManagerTest {
     @Test
     void hasRightShouldThrowMailboxNotFoundExceptionWhenMailboxDoesNotExist() {
         MailboxPath mailboxPath = MailboxPath.forUser(MailboxFixture.ALICE, "unexisting mailbox");
-        when(mockedMailboxMapper.pathExists(mailboxPath))
-            .thenReturn(Mono.just(false));
+        when(mockedMailboxMapper.findMailboxByPath(mailboxPath))
+            .thenReturn(Mono.empty());
 
         assertThatThrownBy(() ->
             storeRightManager.hasRight(mailboxPath, Right.Read, aliceSession))


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


[james-project] 02/03: IMAP - Make reactive SetACLProcessor

Posted by rc...@apache.org.
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 1d7dfd874ed925350378b2520e2d361ee6a3bb16
Author: Tung Van TRAN <vt...@linagora.com>
AuthorDate: Wed Mar 29 16:45:34 2023 +0700

    IMAP - Make reactive SetACLProcessor
---
 .../james/imap/processor/SetACLProcessor.java      | 150 +++++++++++----------
 .../james/imap/processor/SetACLProcessorTest.java  |  42 +++---
 2 files changed, 100 insertions(+), 92 deletions(-)

diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/SetACLProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/SetACLProcessor.java
index 269963abc0..cda9a652f7 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/processor/SetACLProcessor.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/SetACLProcessor.java
@@ -19,10 +19,14 @@
 
 package org.apache.james.imap.processor;
 
+import static org.apache.james.util.ReactorUtils.logOnError;
+
 import java.util.List;
 
 import javax.inject.Inject;
 
+import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.tuple.Pair;
 import org.apache.james.imap.api.ImapConstants;
 import org.apache.james.imap.api.display.HumanReadableText;
 import org.apache.james.imap.api.message.Capability;
@@ -41,12 +45,15 @@ import org.apache.james.mailbox.model.MailboxACL.EntryKey;
 import org.apache.james.mailbox.model.MailboxACL.Rfc4314Rights;
 import org.apache.james.mailbox.model.MailboxPath;
 import org.apache.james.metrics.api.MetricFactory;
+import org.apache.james.util.FunctionalUtils;
 import org.apache.james.util.MDCBuilder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.collect.ImmutableList;
 
+import reactor.core.publisher.Mono;
+
 /**
  * SETACL Processor.
  */
@@ -62,19 +69,39 @@ public class SetACLProcessor extends AbstractMailboxProcessor<SetACLRequest> imp
     }
 
     @Override
-    protected void processRequest(SetACLRequest request, ImapSession session, Responder responder) {
-
+    protected Mono<Void> processRequestReactive(SetACLRequest request, ImapSession session, Responder responder) {
         final MailboxManager mailboxManager = getMailboxManager();
         final MailboxSession mailboxSession = session.getMailboxSession();
         final String mailboxName = request.getMailboxName();
         final String identifier = request.getIdentifier();
-        try {
-            
-            /* parsing the rights is the the cheapest thing to begin with */
-            EditMode editMode = EditMode.REPLACE;
-            String rights = request.getRights();
-            if (rights != null && rights.length() > 0) {
-                switch (rights.charAt(0)) {
+        MailboxPath mailboxPath = PathConverter.forSession(session).buildFullPath(mailboxName);
+
+        return checkLookupRight(request, responder, mailboxManager, mailboxSession, mailboxPath)
+            .filter(FunctionalUtils.identityPredicate())
+            .flatMap(hasLookupRight -> checkAdminRight(request, responder, mailboxManager, mailboxSession, mailboxName, mailboxPath))
+            .filter(FunctionalUtils.identityPredicate())
+            .flatMap(hasAdminRight -> applyRight(mailboxManager, mailboxSession, identifier, request, mailboxPath)
+                .then(Mono.fromRunnable(() -> okComplete(request, responder)))
+                .then())
+            .onErrorResume(UnsupportedRightException.class,
+                error -> Mono.fromRunnable(() -> taggedBad(request, responder,
+                    new HumanReadableText(
+                        HumanReadableText.UNSUPPORTED_RIGHT_KEY,
+                        HumanReadableText.UNSUPPORTED_RIGHT_DEFAULT_VALUE,
+                        error.getUnsupportedRight()))))
+            .onErrorResume(MailboxNotFoundException.class,
+                error -> Mono.fromRunnable(() -> no(request, responder, HumanReadableText.MAILBOX_NOT_FOUND)))
+            .doOnEach(logOnError(MailboxException.class, e -> LOGGER.error("{} failed for mailbox {}", request.getCommand().getName(), mailboxName, e)))
+            .onErrorResume(MailboxException.class,
+                error -> Mono.fromRunnable(() -> no(request, responder, HumanReadableText.GENERIC_FAILURE_DURING_PROCESSING)));
+    }
+
+    /* parsing the rights is the cheapest thing to begin with */
+    private Pair<Rfc4314Rights, EditMode> parsingRightAndEditMode(SetACLRequest request) throws UnsupportedRightException {
+        EditMode editMode = EditMode.REPLACE;
+        String rights = request.getRights();
+        if (StringUtils.isNotEmpty(rights)) {
+            switch (rights.charAt(0)) {
                 case MailboxACL.ADD_RIGHTS_MARKER:
                     editMode = EditMode.ADD;
                     rights = rights.substring(1);
@@ -83,74 +110,49 @@ public class SetACLProcessor extends AbstractMailboxProcessor<SetACLRequest> imp
                     editMode = EditMode.REMOVE;
                     rights = rights.substring(1);
                     break;
-                }
-            }
-            Rfc4314Rights mailboxAclRights = Rfc4314Rights.fromSerializedRfc4314Rights(rights);
-
-            MailboxPath mailboxPath = PathConverter.forSession(session).buildFullPath(mailboxName);
-            // Check that mailbox exists
-            mailboxManager.getMailbox(mailboxPath, mailboxSession);
-
-            /*
-             * RFC 4314 section 6.
-             * An implementation MUST make sure the ACL commands themselves do
-             * not give information about mailboxes with appropriately
-             * restricted ACLs. For example, when a user agent executes a GETACL
-             * command on a mailbox that the user has no permission to LIST, the
-             * server would respond to that request with the same error that
-             * would be used if the mailbox did not exist, thus revealing no
-             * existence information, much less the mailbox’s ACL.
-             */
-            if (!mailboxManager.hasRight(mailboxPath, MailboxACL.Right.Lookup, mailboxSession)) {
-                no(request, responder, HumanReadableText.MAILBOX_NOT_FOUND);
-            } else if (!mailboxManager.hasRight(mailboxPath, MailboxACL.Right.Administer, mailboxSession)) {
-                /* RFC 4314 section 4. */
-                Object[] params = new Object[] {
-                        MailboxACL.Right.Administer.toString(),
-                        request.getCommand().getName(),
-                        mailboxName
-                };
-                HumanReadableText text = new HumanReadableText(HumanReadableText.UNSUFFICIENT_RIGHTS_KEY, HumanReadableText.UNSUFFICIENT_RIGHTS_DEFAULT_VALUE, params);
-                no(request, responder, text);
-            } else {
-                
-                EntryKey key = EntryKey.deserialize(identifier);
-                
-                // FIXME check if identifier is a valid user or group
-                // FIXME Servers, when processing a command that has an identifier as a
-                // parameter (i.e., any of SETACL, DELETEACL, and LISTRIGHTS commands),
-                // SHOULD first prepare the received identifier using "SASLprep" profile
-                // [SASLprep] of the "stringprep" algorithm [Stringprep].  If the
-                // preparation of the identifier fails or results in an empty string,
-                // the server MUST refuse to perform the command with a BAD response.
-                // Note that Section 6 recommends additional identifier’s verification
-                // steps.
-
-                mailboxManager.applyRightsCommand(mailboxPath,
-                    MailboxACL.command().key(key).mode(editMode).rights(mailboxAclRights).build(),
-                    mailboxSession);
-
-                okComplete(request, responder);
-                // FIXME should we send unsolicited responses here?
-                // unsolicitedResponses(session, responder, false);
             }
-        } catch (UnsupportedRightException e) {
-            /*
-             * RFc 4314, section 3.1
-             * Note that an unrecognized right MUST cause the command to return the
-             * BAD response.  In particular, the server MUST NOT silently ignore
-             * unrecognized rights.
-             * */
-            Object[] params = new Object[] {e.getUnsupportedRight()};
-            HumanReadableText text = new HumanReadableText(HumanReadableText.UNSUPPORTED_RIGHT_KEY, HumanReadableText.UNSUPPORTED_RIGHT_DEFAULT_VALUE, params);
-            taggedBad(request, responder, text);
-        } catch (MailboxNotFoundException e) {
-            no(request, responder, HumanReadableText.MAILBOX_NOT_FOUND);
-        } catch (MailboxException e) {
-            LOGGER.error("{} failed for mailbox {}", request.getCommand().getName(), mailboxName, e);
-            no(request, responder, HumanReadableText.GENERIC_FAILURE_DURING_PROCESSING);
         }
+        return Pair.of(Rfc4314Rights.fromSerializedRfc4314Rights(rights), editMode);
+    }
+
+    private Mono<Void> applyRight(MailboxManager mailboxManager,
+                                  MailboxSession mailboxSession,
+                                  String identifier,
+                                  SetACLRequest request,
+                                  MailboxPath mailboxPath) {
+        return Mono.fromCallable(() -> parsingRightAndEditMode(request))
+            .flatMap(rightAndEditMode -> Mono.from(mailboxManager.applyRightsCommandReactive(
+                mailboxPath,
+                MailboxACL.command()
+                    .key(EntryKey.deserialize(identifier))
+                    .mode(rightAndEditMode.getRight())
+                    .rights(rightAndEditMode.getLeft())
+                    .build(),
+                mailboxSession)));
+    }
 
+    private Mono<Boolean> checkAdminRight(SetACLRequest request, Responder responder, MailboxManager mailboxManager, MailboxSession mailboxSession, String mailboxName, MailboxPath mailboxPath) {
+        return Mono.from(mailboxManager.hasRightReactive(mailboxPath, MailboxACL.Right.Administer, mailboxSession))
+            .doOnNext(hasRight -> {
+                if (!hasRight) {
+                    no(request, responder,
+                        new HumanReadableText(
+                            HumanReadableText.UNSUFFICIENT_RIGHTS_KEY,
+                            HumanReadableText.UNSUFFICIENT_RIGHTS_DEFAULT_VALUE,
+                            MailboxACL.Right.Administer.toString(),
+                            request.getCommand().getName(),
+                            mailboxName));
+                }
+            });
+    }
+
+    private Mono<Boolean> checkLookupRight(SetACLRequest request, Responder responder, MailboxManager mailboxManager, MailboxSession mailboxSession, MailboxPath mailboxPath) {
+        return Mono.from(mailboxManager.hasRightReactive(mailboxPath, MailboxACL.Right.Lookup, mailboxSession))
+            .doOnNext(hasRight -> {
+                if (!hasRight) {
+                    no(request, responder, HumanReadableText.MAILBOX_NOT_FOUND);
+                }
+            });
     }
 
     @Override
diff --git a/protocols/imap/src/test/java/org/apache/james/imap/processor/SetACLProcessorTest.java b/protocols/imap/src/test/java/org/apache/james/imap/processor/SetACLProcessorTest.java
index 791fb347d0..847d3fe223 100644
--- a/protocols/imap/src/test/java/org/apache/james/imap/processor/SetACLProcessorTest.java
+++ b/protocols/imap/src/test/java/org/apache/james/imap/processor/SetACLProcessorTest.java
@@ -41,7 +41,6 @@ import org.apache.james.mailbox.MailboxSessionUtil;
 import org.apache.james.mailbox.MessageManager;
 import org.apache.james.mailbox.MessageManager.MailboxMetaData;
 import org.apache.james.mailbox.MessageManager.MailboxMetaData.FetchGroup;
-import org.apache.james.mailbox.exception.MailboxException;
 import org.apache.james.mailbox.exception.MailboxNotFoundException;
 import org.apache.james.mailbox.model.MailboxACL;
 import org.apache.james.mailbox.model.MailboxACL.EditMode;
@@ -109,11 +108,13 @@ class SetACLProcessorTest {
     }
     
     @Test
-    void testUnsupportedRight() throws Exception {
+    void testUnsupportedRight() {
         SetACLRequest setACLRequest = new SetACLRequest(TAG, MAILBOX_NAME, USER_1.asString(), UNSUPPORTED_RIGHT);
 
-        when(mailboxManager.hasRight(path, MailboxACL.Right.Lookup, mailboxSession))
-            .thenReturn(false);
+        when(mailboxManager.hasRightReactive(path, MailboxACL.Right.Lookup, mailboxSession))
+            .thenReturn(Mono.just(true));
+        when(mailboxManager.hasRightReactive(path, MailboxACL.Right.Administer, mailboxSession))
+            .thenReturn(Mono.just(true));
 
         subject.doProcess(setACLRequest, responder, imapSession).block();
 
@@ -127,11 +128,11 @@ class SetACLProcessorTest {
     }
     
     @Test
-    void testNoAdminRight() throws Exception {
-        when(mailboxManager.hasRight(path, MailboxACL.Right.Lookup, mailboxSession))
-            .thenReturn(true);
-        when(mailboxManager.hasRight(path, MailboxACL.Right.Administer, mailboxSession))
-            .thenReturn(false);
+    void testNoAdminRight() {
+        when(mailboxManager.hasRightReactive(path, MailboxACL.Right.Lookup, mailboxSession))
+            .thenReturn(Mono.just(true));
+        when(mailboxManager.hasRightReactive(path, MailboxACL.Right.Administer, mailboxSession))
+            .thenReturn(Mono.just(false));
 
         subject.doProcess(replaceAclRequest, responder, imapSession).block();
 
@@ -145,9 +146,10 @@ class SetACLProcessorTest {
     }
     
     @Test
-    void testInexistentMailboxName() throws Exception {
-        when(mailboxManager.getMailbox(any(MailboxPath.class), any(MailboxSession.class)))
-            .thenThrow(new MailboxNotFoundException(""));
+    void testInexistentMailboxName() {
+        when(mailboxManager.hasRightReactive(any(MailboxPath.class),
+            any(),any(MailboxSession.class)))
+            .thenReturn(Mono.error(new MailboxNotFoundException("")));
 
         subject.doProcess(replaceAclRequest, responder, imapSession).block();
 
@@ -175,16 +177,20 @@ class SetACLProcessorTest {
         testOp("", EditMode.REPLACE);
     }
     
-    private void testOp(String prefix, EditMode editMode) throws MailboxException {
-        when(mailboxManager.hasRight(path, MailboxACL.Right.Lookup, mailboxSession))
-            .thenReturn(true);
-        when(mailboxManager.hasRight(path, MailboxACL.Right.Administer, mailboxSession))
-            .thenReturn(true);
+    private void testOp(String prefix, EditMode editMode) {
+        when(mailboxManager.hasRightReactive(path, MailboxACL.Right.Lookup, mailboxSession))
+            .thenReturn(Mono.just(true));
+        when(mailboxManager.hasRightReactive(path, MailboxACL.Right.Administer, mailboxSession))
+            .thenReturn(Mono.just(true));
+        when(mailboxManager.applyRightsCommandReactive(path,
+            MailboxACL.command().key(user1Key).rights(setRights).mode(editMode).build(),
+            mailboxSession))
+            .thenReturn(Mono.empty());
 
         SetACLRequest r = new SetACLRequest(TAG, MAILBOX_NAME, USER_1.asString(), prefix + SET_RIGHTS);
         subject.doProcess(r, responder, imapSession).block();
 
-        verify(mailboxManager).applyRightsCommand(path,
+        verify(mailboxManager).applyRightsCommandReactive(path,
             MailboxACL.command().key(user1Key).rights(setRights).mode(editMode).build(),
             mailboxSession);
 


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


[james-project] 03/03: IMAP - Make reactive DeleteACLProcessor

Posted by rc...@apache.org.
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 7c63388313d6c407cd039cc33a907745a1a8483a
Author: Tung Van TRAN <vt...@linagora.com>
AuthorDate: Wed Mar 29 16:45:43 2023 +0700

    IMAP - Make reactive DeleteACLProcessor
---
 .../james/imap/processor/DeleteACLProcessor.java   | 124 ++++++++++-----------
 .../imap/processor/DeleteACLProcessorTest.java     |  39 ++++---
 2 files changed, 77 insertions(+), 86 deletions(-)

diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/DeleteACLProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/DeleteACLProcessor.java
index 20d7f1b1e7..4edaf17ab5 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/processor/DeleteACLProcessor.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/DeleteACLProcessor.java
@@ -19,6 +19,8 @@
 
 package org.apache.james.imap.processor;
 
+import static org.apache.james.util.ReactorUtils.logOnError;
+
 import java.util.List;
 
 import javax.inject.Inject;
@@ -39,12 +41,15 @@ import org.apache.james.mailbox.model.MailboxACL;
 import org.apache.james.mailbox.model.MailboxACL.EntryKey;
 import org.apache.james.mailbox.model.MailboxPath;
 import org.apache.james.metrics.api.MetricFactory;
+import org.apache.james.util.FunctionalUtils;
 import org.apache.james.util.MDCBuilder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.collect.ImmutableList;
 
+import reactor.core.publisher.Mono;
+
 /**
  * DELETEACL Processor.
  */
@@ -59,79 +64,62 @@ public class DeleteACLProcessor extends AbstractMailboxProcessor<DeleteACLReques
     }
 
     @Override
-    protected void processRequest(DeleteACLRequest request, ImapSession session, Responder responder) {
-
+    protected Mono<Void> processRequestReactive(DeleteACLRequest request, ImapSession session, Responder responder) {
         final MailboxManager mailboxManager = getMailboxManager();
         final MailboxSession mailboxSession = session.getMailboxSession();
         final String mailboxName = request.getMailboxName();
         final String identifier = request.getIdentifier();
-        try {
-
-            MailboxPath mailboxPath = PathConverter.forSession(session).buildFullPath(mailboxName);
-            // Check that mailbox exists
-            mailboxManager.getMailbox(mailboxPath, mailboxSession);
-            /*
-             * RFC 4314 section 6.
-             * An implementation MUST make sure the ACL commands themselves do
-             * not give information about mailboxes with appropriately
-             * restricted ACLs. For example, when a user agent executes a GETACL
-             * command on a mailbox that the user has no permission to LIST, the
-             * server would respond to that request with the same error that
-             * would be used if the mailbox did not exist, thus revealing no
-             * existence information, much less the mailbox’s ACL.
-             */
-            if (!mailboxManager.hasRight(mailboxPath, MailboxACL.Right.Lookup, mailboxSession)) {
-                no(request, responder, HumanReadableText.MAILBOX_NOT_FOUND);
-            } else if (!mailboxManager.hasRight(mailboxPath, MailboxACL.Right.Administer, mailboxSession)) {
-                /* RFC 4314 section 4. */
-                Object[] params = new Object[] {
-                        MailboxACL.Right.Administer.toString(),
-                        request.getCommand().getName(),
-                        mailboxName
-                };
-                HumanReadableText text = new HumanReadableText(HumanReadableText.UNSUFFICIENT_RIGHTS_KEY, HumanReadableText.UNSUFFICIENT_RIGHTS_DEFAULT_VALUE, params);
-                no(request, responder, text);
-            } else {
-                
-                EntryKey key = EntryKey.deserialize(identifier);
-                
-                // FIXME check if identifier is a valid user or group
-                // FIXME Servers, when processing a command that has an identifier as a
-                // parameter (i.e., any of SETACL, DELETEACL, and LISTRIGHTS commands),
-                // SHOULD first prepare the received identifier using "SASLprep" profile
-                // [SASLprep] of the "stringprep" algorithm [Stringprep].  If the
-                // preparation of the identifier fails or results in an empty string,
-                // the server MUST refuse to perform the command with a BAD response.
-                // Note that Section 6 recommends additional identifier’s verification
-                // steps.
-
-                mailboxManager.applyRightsCommand(
-                    mailboxPath,
-                    MailboxACL.command().key(key).noRights().asReplacement(),
-                    mailboxSession
-                );
-
-                okComplete(request, responder);
-                // FIXME should we send unsolicited responses here?
-                // unsolicitedResponses(session, responder, false);
-            }
-        } catch (UnsupportedRightException e) {
-            /*
-             * RFc 4314, section 3.1
-             * Note that an unrecognized right MUST cause the command to return the
-             * BAD response.  In particular, the server MUST NOT silently ignore
-             * unrecognized rights.
-             * */
-            Object[] params = new Object[] {e.getUnsupportedRight()};
-            HumanReadableText text = new HumanReadableText(HumanReadableText.UNSUPPORTED_RIGHT_KEY, HumanReadableText.UNSUPPORTED_RIGHT_DEFAULT_VALUE, params);
-            taggedBad(request, responder, text);
-        } catch (MailboxNotFoundException e) {
-            no(request, responder, HumanReadableText.MAILBOX_NOT_FOUND);
-        } catch (MailboxException e) {
-            LOGGER.error("{} failed for mailbox {}", request.getCommand().getName(), mailboxName, e);
-            no(request, responder, HumanReadableText.GENERIC_FAILURE_DURING_PROCESSING);
-        }
+        MailboxPath mailboxPath = PathConverter.forSession(session).buildFullPath(mailboxName);
+
+        return checkLookupRight(request, responder, mailboxManager, mailboxSession, mailboxPath)
+            .filter(FunctionalUtils.identityPredicate())
+            .flatMap(hasLookupRight -> checkAdminRight(request, responder, mailboxManager, mailboxSession, mailboxName, mailboxPath))
+            .filter(FunctionalUtils.identityPredicate())
+            .flatMap(hasAdminRight -> applyRight(mailboxManager, mailboxSession, identifier, mailboxPath)
+                .then(Mono.fromRunnable(() -> okComplete(request, responder)))
+                .then())
+            .onErrorResume(UnsupportedRightException.class,
+                error -> Mono.fromRunnable(() -> taggedBad(request, responder,
+                    new HumanReadableText(
+                        HumanReadableText.UNSUPPORTED_RIGHT_KEY,
+                        HumanReadableText.UNSUPPORTED_RIGHT_DEFAULT_VALUE,
+                        error.getUnsupportedRight()))))
+            .onErrorResume(MailboxNotFoundException.class,
+                error -> Mono.fromRunnable(() -> no(request, responder, HumanReadableText.MAILBOX_NOT_FOUND)))
+            .doOnEach(logOnError(MailboxException.class, e -> LOGGER.error("{} failed for mailbox {}", request.getCommand().getName(), mailboxName, e)))
+            .onErrorResume(MailboxException.class,
+                error -> Mono.fromRunnable(() -> no(request, responder, HumanReadableText.GENERIC_FAILURE_DURING_PROCESSING)));
+    }
+
+    private static Mono<Void> applyRight(MailboxManager mailboxManager, MailboxSession mailboxSession, String identifier, MailboxPath mailboxPath) {
+        return Mono.from(mailboxManager.applyRightsCommandReactive(
+            mailboxPath,
+            MailboxACL.command().key(EntryKey.deserialize(identifier)).noRights().asReplacement(),
+            mailboxSession));
+    }
+
+    private Mono<Boolean> checkAdminRight(DeleteACLRequest request, Responder responder, MailboxManager mailboxManager, MailboxSession mailboxSession, String mailboxName, MailboxPath mailboxPath) {
+        return Mono.from(mailboxManager.hasRightReactive(mailboxPath, MailboxACL.Right.Administer, mailboxSession))
+            .doOnNext(hasRight -> {
+                if (!hasRight) {
+                    no(request, responder,
+                        new HumanReadableText(
+                            HumanReadableText.UNSUFFICIENT_RIGHTS_KEY,
+                            HumanReadableText.UNSUFFICIENT_RIGHTS_DEFAULT_VALUE,
+                            MailboxACL.Right.Administer.toString(),
+                            request.getCommand().getName(),
+                            mailboxName));
+                }
+            });
+    }
 
+    private Mono<Boolean> checkLookupRight(DeleteACLRequest request, Responder responder, MailboxManager mailboxManager, MailboxSession mailboxSession, MailboxPath mailboxPath) {
+        return Mono.from(mailboxManager.hasRightReactive(mailboxPath, MailboxACL.Right.Lookup, mailboxSession))
+            .doOnNext(hasRight -> {
+                if (!hasRight) {
+                    no(request, responder, HumanReadableText.MAILBOX_NOT_FOUND);
+                }
+            });
     }
 
     @Override
diff --git a/protocols/imap/src/test/java/org/apache/james/imap/processor/DeleteACLProcessorTest.java b/protocols/imap/src/test/java/org/apache/james/imap/processor/DeleteACLProcessorTest.java
index f709722568..8ee9813ba7 100644
--- a/protocols/imap/src/test/java/org/apache/james/imap/processor/DeleteACLProcessorTest.java
+++ b/protocols/imap/src/test/java/org/apache/james/imap/processor/DeleteACLProcessorTest.java
@@ -41,7 +41,6 @@ import org.apache.james.mailbox.MailboxSessionUtil;
 import org.apache.james.mailbox.MessageManager;
 import org.apache.james.mailbox.MessageManager.MailboxMetaData;
 import org.apache.james.mailbox.MessageManager.MailboxMetaData.FetchGroup;
-import org.apache.james.mailbox.exception.MailboxException;
 import org.apache.james.mailbox.exception.MailboxNotFoundException;
 import org.apache.james.mailbox.model.MailboxACL;
 import org.apache.james.mailbox.model.MailboxACL.EntryKey;
@@ -106,9 +105,9 @@ class DeleteACLProcessorTest {
     }
     
     @Test
-    void testNoListRight() throws Exception {
-        when(mailboxManager.hasRight(path, MailboxACL.Right.Lookup, mailboxSession))
-            .thenReturn(false);
+    void testNoListRight() {
+        when(mailboxManager.hasRightReactive(path, MailboxACL.Right.Lookup, mailboxSession))
+            .thenReturn(Mono.just(false));
 
         subject.doProcess(deleteACLRequest, responder, imapSession).block();
 
@@ -122,11 +121,11 @@ class DeleteACLProcessorTest {
     }
 
     @Test
-    void testNoAdminRight() throws Exception {
-        when(mailboxManager.hasRight(path, MailboxACL.Right.Lookup, mailboxSession))
-            .thenReturn(true);
-        when(mailboxManager.hasRight(path, MailboxACL.Right.Administer, mailboxSession))
-            .thenReturn(false);
+    void testNoAdminRight() {
+        when(mailboxManager.hasRightReactive(path, MailboxACL.Right.Lookup, mailboxSession))
+            .thenReturn(Mono.just(true));
+        when(mailboxManager.hasRightReactive(path, MailboxACL.Right.Administer, mailboxSession))
+            .thenReturn(Mono.just(false));
 
         subject.doProcess(deleteACLRequest, responder, imapSession).block();
 
@@ -140,9 +139,10 @@ class DeleteACLProcessorTest {
     }
 
     @Test
-    void testNonExistentMailboxName() throws Exception {
-        when(mailboxManager.getMailbox(any(MailboxPath.class), any(MailboxSession.class)))
-            .thenThrow(new MailboxNotFoundException(""));
+    void testNonExistentMailboxName() {
+        when(mailboxManager.hasRightReactive(any(MailboxPath.class),
+            any(),any(MailboxSession.class)))
+            .thenReturn(Mono.error(new MailboxNotFoundException("")));
 
         subject.doProcess(deleteACLRequest, responder, imapSession).block();
 
@@ -157,17 +157,20 @@ class DeleteACLProcessorTest {
 
     
     @Test
-    void testDelete() throws MailboxException {
+    void testDelete() {
         MailboxACL acl = MailboxACL.OWNER_FULL_ACL;
-        when(mailboxManager.hasRight(path, MailboxACL.Right.Lookup, mailboxSession))
-            .thenReturn(true);
-        when(mailboxManager.hasRight(path, MailboxACL.Right.Administer, mailboxSession))
-            .thenReturn(true);
+        when(mailboxManager.hasRightReactive(path, MailboxACL.Right.Lookup, mailboxSession))
+            .thenReturn(Mono.just(true));
+        when(mailboxManager.hasRightReactive(path, MailboxACL.Right.Administer, mailboxSession))
+            .thenReturn(Mono.just(true));
         when(metaData.getACL()).thenReturn(acl);
+        when(mailboxManager.applyRightsCommandReactive(path, MailboxACL.command().key(user1Key).noRights().asReplacement(),
+            mailboxSession))
+            .thenReturn(Mono.empty());
 
         subject.doProcess(deleteACLRequest, responder, imapSession).block();
 
-        verify(mailboxManager).applyRightsCommand(path,
+        verify(mailboxManager).applyRightsCommandReactive(path,
             MailboxACL.command().key(user1Key).noRights().asReplacement(),
             mailboxSession);
 


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