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 rc...@apache.org on 2020/02/18 02:48:11 UTC

[james-project] branch master updated (96205cb -> cf844df)

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 96205cb  JAMES-3049 Distributed admin procedures: Event Bus
     new fa99c39  JAMES-3066 Add method in CanSendFrom to list all the  valid from addresses of a user
     new c845dfe  JAMES-3066 Add allowed From headers list route in WebAdmin
     new bab4cf5  JAMES-3066 Strong type CanSendFromImpl.allValidFromAddressesForUser
     new e0d28d9  JAMES-3050 Distributed admin procedures: ElasticSearch indexing
     new 05c0fab  JAMES-3056 renaming mailboxes doesn't need ACL to be involved
     new 40b671e  JAMES-3057 Rename some variables in StoreMailboxManager
     new d9d098d  JAMES-3057 Add a create default method in MailboxMapper
     new cf844df  JAMES-3057 Separate the rename and create mailbox logic in CassandraMailboxMapper

The 8 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:
 .../main/java/org/apache/james/core/Username.java  |   4 +
 .../cassandra/mail/CassandraMailboxMapper.java     |  40 +++++--
 .../cassandra/mail/CassandraMailboxMapperTest.java | 132 +++++++--------------
 .../james/mailbox/store/StoreMailboxManager.java   |  26 ++--
 .../james/mailbox/store/mail/MailboxMapper.java    |  13 +-
 .../store/mail/model/MailboxMapperTest.java        |  37 +++++-
 .../java/org/apache/james/rrt/api/CanSendFrom.java |   8 ++
 .../org/apache/james/rrt/lib/MappingSource.java    |   4 +
 .../apache/james/rrt/lib/CanSendFromContract.java  |  72 +++++++++--
 .../org/apache/james/rrt/lib/CanSendFromImpl.java  |  44 +++++++
 .../apache/james/webadmin/routes/UserRoutes.java   |  62 +++++++++-
 .../apache/james/webadmin/service/UserService.java |  13 ++
 .../james/webadmin/routes/UserRoutesTest.java      |  96 ++++++++++++++-
 .../server/manage-guice-distributed-james.md       |  55 +++++++++
 src/site/markdown/server/manage-webadmin.md        |  19 +++
 15 files changed, 500 insertions(+), 125 deletions(-)


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


[james-project] 02/08: JAMES-3066 Add allowed From headers list route in WebAdmin

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 c845dfe8bd2f7e1447386f4f36a8412b2d484a2f
Author: Gautier DI FOLCO <gd...@linagora.com>
AuthorDate: Thu Feb 13 12:57:46 2020 +0100

    JAMES-3066 Add allowed From headers list route in WebAdmin
---
 .../apache/james/webadmin/routes/UserRoutes.java   | 62 +++++++++++++-
 .../apache/james/webadmin/service/UserService.java | 13 +++
 .../james/webadmin/routes/UserRoutesTest.java      | 96 +++++++++++++++++++++-
 src/site/markdown/server/manage-webadmin.md        | 19 +++++
 4 files changed, 186 insertions(+), 4 deletions(-)

diff --git a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java
index 6d3b641..d0b78ee 100644
--- a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java
+++ b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/UserRoutes.java
@@ -22,6 +22,8 @@ package org.apache.james.webadmin.routes;
 import static org.apache.james.webadmin.Constants.SEPARATOR;
 import static spark.Spark.halt;
 
+import java.util.List;
+
 import javax.inject.Inject;
 import javax.ws.rs.DELETE;
 import javax.ws.rs.GET;
@@ -29,7 +31,11 @@ import javax.ws.rs.PUT;
 import javax.ws.rs.Path;
 import javax.ws.rs.Produces;
 
+import org.apache.james.core.MailAddress;
 import org.apache.james.core.Username;
+import org.apache.james.rrt.api.CanSendFrom;
+import org.apache.james.rrt.api.RecipientRewriteTable;
+import org.apache.james.rrt.api.RecipientRewriteTableException;
 import org.apache.james.user.api.InvalidUsernameException;
 import org.apache.james.user.api.UsersRepositoryException;
 import org.apache.james.webadmin.Routes;
@@ -46,6 +52,8 @@ import org.eclipse.jetty.http.HttpStatus;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.github.steveash.guavate.Guavate;
+
 import io.swagger.annotations.Api;
 import io.swagger.annotations.ApiImplicitParam;
 import io.swagger.annotations.ApiImplicitParams;
@@ -69,14 +77,16 @@ public class UserRoutes implements Routes {
 
     private final UserService userService;
     private final JsonTransformer jsonTransformer;
+    private final CanSendFrom canSendFrom;
     private final JsonExtractor<AddUserRequest> jsonExtractor;
 
     private Service service;
 
     @Inject
-    public UserRoutes(UserService userService, JsonTransformer jsonTransformer) {
+    public UserRoutes(UserService userService, CanSendFrom canSendFrom, JsonTransformer jsonTransformer) {
         this.userService = userService;
         this.jsonTransformer = jsonTransformer;
+        this.canSendFrom = canSendFrom;
         this.jsonExtractor = new JsonExtractor<>(AddUserRequest.class);
     }
 
@@ -94,6 +104,8 @@ public class UserRoutes implements Routes {
         defineCreateUser();
 
         defineDeleteUser();
+
+        defineAllowedFromHeaders();
     }
 
     @DELETE
@@ -142,6 +154,25 @@ public class UserRoutes implements Routes {
             jsonTransformer);
     }
 
+    @GET
+    @Path("/{username}/allowedFromHeaders")
+    @ApiOperation(value = "List all possible From header value for an existing user")
+    @ApiImplicitParams({
+        @ApiImplicitParam(required = true, dataType = "string", name = "username", paramType = "path")
+    })
+    @ApiResponses(value = {
+        @ApiResponse(code = HttpStatus.NO_CONTENT_204, message = "OK.", response = List.class),
+        @ApiResponse(code = HttpStatus.BAD_REQUEST_400, message = "user is not valid."),
+        @ApiResponse(code = HttpStatus.NOT_FOUND_404, message = "user does not exist."),
+        @ApiResponse(code = HttpStatus.INTERNAL_SERVER_ERROR_500,
+            message = "Internal server error - Something went bad on the server side.")
+    })
+    public void defineAllowedFromHeaders() {
+        service.get(USERS + SEPARATOR + USER_NAME + SEPARATOR + "allowedFromHeaders",
+            this::allowedFromHeaders,
+            jsonTransformer);
+    }
+
     private String removeUser(Request request, Response response) {
         Username username = extractUsername(request);
         try {
@@ -184,6 +215,35 @@ public class UserRoutes implements Routes {
         }
     }
 
+    private List<String> allowedFromHeaders(Request request, Response response) {
+        Username username = extractUsername(request);
+
+        try {
+            if (!userService.existUser(username)) {
+                LOGGER.info("allowed From headers on an unknown user: '{}", username.asString());
+                throw ErrorResponder.builder()
+                    .statusCode(HttpStatus.NOT_FOUND_404)
+                    .type(ErrorType.INVALID_ARGUMENT)
+                    .message("user '" + username.asString() + "' does not exist")
+                    .haltError();
+            }
+
+            return canSendFrom
+                .allValidFromAddressesForUser(username)
+                .map(MailAddress::asString)
+                .collect(Guavate.toImmutableList());
+        } catch (RecipientRewriteTable.ErrorMappingException | RecipientRewriteTableException | UsersRepositoryException e) {
+            String errorMessage = String.format("Error while listing allowed From headers for user '%s'", username);
+            LOGGER.info(errorMessage, e);
+            throw ErrorResponder.builder()
+                .statusCode(HttpStatus.INTERNAL_SERVER_ERROR_500)
+                .type(ErrorType.SERVER_ERROR)
+                .message(errorMessage)
+                .cause(e)
+                .haltError();
+        }
+    }
+
     private Username extractUsername(Request request) {
         return Username.of(request.params(USER_NAME));
     }
diff --git a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/UserService.java b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/UserService.java
index 78501da..cd36afb 100644
--- a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/UserService.java
+++ b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/service/UserService.java
@@ -24,6 +24,7 @@ import java.util.Optional;
 import java.util.stream.Stream;
 
 import javax.inject.Inject;
+import javax.mail.internet.AddressException;
 
 import org.apache.james.core.Username;
 import org.apache.james.user.api.UsersRepository;
@@ -31,11 +32,15 @@ import org.apache.james.user.api.UsersRepositoryException;
 import org.apache.james.user.api.model.User;
 import org.apache.james.util.streams.Iterators;
 import org.apache.james.webadmin.dto.UserResponse;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.github.steveash.guavate.Guavate;
 
 public class UserService {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger(UserService.class);
+
     private final UsersRepository usersRepository;
 
     @Inject
@@ -61,6 +66,14 @@ public class UserService {
         upsert(user, username, password);
     }
 
+    public boolean existUser(Username username) throws UsersRepositoryException {
+        try {
+            return usersRepository.contains(usersRepository.getUser(username.asMailAddress()));
+        } catch (AddressException e) {
+            LOGGER.info("Unable to parse address '%s'", username.asString(), e);
+            return false;
+        }
+    }
 
     private void upsert(User user, Username username, char[] password) throws UsersRepositoryException {
         if (user == null) {
diff --git a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UserRoutesTest.java b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UserRoutesTest.java
index 89112c7..db1dba9 100644
--- a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UserRoutesTest.java
+++ b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/UserRoutesTest.java
@@ -22,6 +22,7 @@ package org.apache.james.webadmin.routes;
 import static io.restassured.RestAssured.given;
 import static io.restassured.RestAssured.when;
 import static io.restassured.RestAssured.with;
+import static org.apache.james.webadmin.Constants.SEPARATOR;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.hamcrest.Matchers.is;
 import static org.mockito.ArgumentMatchers.any;
@@ -39,6 +40,12 @@ import org.apache.james.core.Domain;
 import org.apache.james.core.Username;
 import org.apache.james.domainlist.api.DomainListException;
 import org.apache.james.domainlist.api.mock.SimpleDomainList;
+import org.apache.james.rrt.api.CanSendFrom;
+import org.apache.james.rrt.api.RecipientRewriteTable;
+import org.apache.james.rrt.api.RecipientRewriteTableException;
+import org.apache.james.rrt.lib.CanSendFromImpl;
+import org.apache.james.rrt.lib.MappingSource;
+import org.apache.james.rrt.memory.MemoryRecipientRewriteTable;
 import org.apache.james.user.api.UsersRepository;
 import org.apache.james.user.api.UsersRepositoryException;
 import org.apache.james.user.api.model.User;
@@ -90,12 +97,17 @@ class UserRoutesTest {
 
         final MemoryUsersRepository usersRepository;
         final SimpleDomainList domainList;
+        final MemoryRecipientRewriteTable recipientRewriteTable;
+        final CanSendFrom canSendFrom;
 
         WebAdminServer webAdminServer;
 
         UserRoutesExtension(MemoryUsersRepository usersRepository, SimpleDomainList domainList) {
             this.usersRepository = spy(usersRepository);
             this.domainList = domainList;
+            this.recipientRewriteTable = new MemoryRecipientRewriteTable();
+            this.recipientRewriteTable.setDomainList(domainList);
+            this.canSendFrom = new CanSendFromImpl(recipientRewriteTable);
         }
 
         @Override
@@ -112,16 +124,25 @@ class UserRoutesTest {
         public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) throws ParameterResolutionException {
             return parameterContext.getParameter()
                 .getType()
-                .isAssignableFrom(UsersRepository.class);
+                .isAssignableFrom(UsersRepository.class)
+                || parameterContext.getParameter()
+                .getType()
+                .isAssignableFrom(RecipientRewriteTable.class);
         }
 
         @Override
         public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) throws ParameterResolutionException {
-            return usersRepository;
+            if (parameterContext.getParameter().getType().isAssignableFrom(UsersRepository.class)) {
+                return usersRepository;
+            }
+            if (parameterContext.getParameter().getType().isAssignableFrom(RecipientRewriteTable.class)) {
+                return recipientRewriteTable;
+            }
+            throw new RuntimeException("Unknown parameter type: " + parameterContext.getParameter().getType());
         }
 
         private WebAdminServer startServer(UsersRepository usersRepository) {
-            WebAdminServer server = WebAdminUtils.createWebAdminServer(new UserRoutes(new UserService(usersRepository), new JsonTransformer()))
+            WebAdminServer server = WebAdminUtils.createWebAdminServer(new UserRoutes(new UserService(usersRepository), canSendFrom, new JsonTransformer()))
                 .start();
 
             RestAssured.requestSpecification = WebAdminUtils.buildRequestSpecification(server)
@@ -573,6 +594,75 @@ class UserRoutesTest {
                 .statusCode(HttpStatus.NO_CONTENT_204);
         }
 
+        @Test
+        void allowedFromHeadersShouldHaveUsersMailAddress() {
+            // Given
+            with()
+                .body("{\"password\":\"password\"}")
+                .put(USERNAME_WITH_DOMAIN.asString());
+
+            // Then
+            List<String> allowedFroms =
+                when()
+                    .get(USERNAME_WITH_DOMAIN.asString() + SEPARATOR + "allowedFromHeaders")
+                .then()
+                    .statusCode(HttpStatus.OK_200)
+                    .contentType(ContentType.JSON)
+                    .extract()
+                    .body()
+                    .jsonPath()
+                    .getList(".");
+
+            assertThat(allowedFroms).containsExactly(USERNAME_WITH_DOMAIN.asString());
+        }
+
+        @Test
+        void allowedFromHeadersShouldHaveAllMailAddressesWhenAliasAdded(RecipientRewriteTable recipientRewriteTable) throws RecipientRewriteTableException {
+            // Given
+            with()
+                .body("{\"password\":\"password\"}")
+                .put(USERNAME_WITH_DOMAIN.asString());
+
+            String aliasAddress = "alias@" + DOMAIN.asString();
+            recipientRewriteTable.addAliasMapping(MappingSource.fromUser(Username.of(aliasAddress)), USERNAME_WITH_DOMAIN.asString());
+
+            // Then
+            List<String> allowedFroms =
+                when()
+                    .get(USERNAME_WITH_DOMAIN.asString() + SEPARATOR + "allowedFromHeaders")
+                .then()
+                    .statusCode(HttpStatus.OK_200)
+                    .contentType(ContentType.JSON)
+                    .extract()
+                    .body()
+                    .jsonPath()
+                    .getList(".");
+
+            assertThat(allowedFroms).containsExactly(USERNAME_WITH_DOMAIN.asString(), aliasAddress);
+        }
+
+        @Test
+        void allowedFromHeadersShouldReturn404WhenUserDoesNotExist() {
+            when()
+                .get(USERNAME_WITH_DOMAIN.asString() + SEPARATOR + "allowedFromHeaders")
+            .then()
+                .statusCode(HttpStatus.NOT_FOUND_404)
+                .body("statusCode", is(HttpStatus.NOT_FOUND_404))
+                .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+                .body("message", is("user 'username@domain' does not exist"));
+        }
+
+        @Test
+        void allowedFromHeadersShouldReturn404WhenUserIsInvalid() {
+            when()
+                .get("@@" + SEPARATOR + "allowedFromHeaders")
+            .then()
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .body("statusCode", is(HttpStatus.BAD_REQUEST_400))
+                .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+                .body("message", is("Invalid arguments supplied in the user request"));
+        }
+
         @Nested
         class IllegalCharacterErrorHandlingTest implements UserRoutesContract.IllegalCharactersErrorHandlingContract {
 
diff --git a/src/site/markdown/server/manage-webadmin.md b/src/site/markdown/server/manage-webadmin.md
index b2e0142..76dccc6 100644
--- a/src/site/markdown/server/manage-webadmin.md
+++ b/src/site/markdown/server/manage-webadmin.md
@@ -292,6 +292,7 @@ Response codes:
    - [Updating a user password](#Updating_a_user_password)
    - [Deleting a domain](#Deleting_a_user)
    - [Retrieving the user list](#Retrieving_the_user_list)
+   - [Retrieving the list of allowed `From` headers for a given user](Retrieving_the_list_of_allowed_From_headers_for_a_given_user)
 
 ### Create a user
 
@@ -343,6 +344,24 @@ Response codes:
 
  - 200: The user name list was successfully retrieved
 
+### Retrieving the list of allowed `From` headers for a given user
+
+```
+curl -XGET http://ip:port/users/givenUser/allowedFromHeaders
+```
+
+The answer looks like:
+
+```
+["user@domain.tld","alias@domain.tld"]
+```
+
+Response codes:
+
+ - 200: The list was successfully retrieved
+ - 400: The user is invalid
+ - 404: The user is unknown
+
 ## Administrating mailboxes
 
 ### All mailboxes


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


[james-project] 03/08: JAMES-3066 Strong type CanSendFromImpl.allValidFromAddressesForUser

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 bab4cf5744cc74af78fe416a89159d45ff9a91e3
Author: Gautier DI FOLCO <gd...@linagora.com>
AuthorDate: Mon Feb 17 09:57:59 2020 +0100

    JAMES-3066 Strong type CanSendFromImpl.allValidFromAddressesForUser
---
 .../main/java/org/apache/james/core/Username.java  |  4 +++
 .../org/apache/james/rrt/lib/MappingSource.java    |  4 +++
 .../org/apache/james/rrt/lib/CanSendFromImpl.java  | 31 +++++++++++++---------
 3 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/core/src/main/java/org/apache/james/core/Username.java b/core/src/main/java/org/apache/james/core/Username.java
index 63cd8a7..c27d980 100644
--- a/core/src/main/java/org/apache/james/core/Username.java
+++ b/core/src/main/java/org/apache/james/core/Username.java
@@ -97,6 +97,10 @@ public class Username {
         return domainPart;
     }
 
+    public Username withOtherDomain(Optional<Domain> domain) {
+        return new Username(localPart, domain);
+    }
+
     public boolean hasDomainPart() {
         return domainPart.isPresent();
     }
diff --git a/server/data/data-api/src/main/java/org/apache/james/rrt/lib/MappingSource.java b/server/data/data-api/src/main/java/org/apache/james/rrt/lib/MappingSource.java
index 178823b..8bf6cd4 100644
--- a/server/data/data-api/src/main/java/org/apache/james/rrt/lib/MappingSource.java
+++ b/server/data/data-api/src/main/java/org/apache/james/rrt/lib/MappingSource.java
@@ -94,6 +94,10 @@ public class MappingSource implements Serializable {
         this.wildcard = wildcard;
     }
 
+    public Optional<Username> asUsername() {
+        return user;
+    }
+
     public Optional<Domain> asDomain() {
         return domain;
     }
diff --git a/server/data/data-library/src/main/java/org/apache/james/rrt/lib/CanSendFromImpl.java b/server/data/data-library/src/main/java/org/apache/james/rrt/lib/CanSendFromImpl.java
index 97130c6..930cc58 100644
--- a/server/data/data-library/src/main/java/org/apache/james/rrt/lib/CanSendFromImpl.java
+++ b/server/data/data-library/src/main/java/org/apache/james/rrt/lib/CanSendFromImpl.java
@@ -23,6 +23,7 @@ import static org.apache.james.rrt.lib.Mapping.Type.Domain;
 
 import java.util.EnumSet;
 import java.util.List;
+import java.util.Optional;
 import java.util.stream.Stream;
 
 import javax.inject.Inject;
@@ -59,12 +60,13 @@ public class CanSendFromImpl implements CanSendFrom {
 
     @Override
     public Stream<MailAddress> allValidFromAddressesForUser(Username user) throws RecipientRewriteTable.ErrorMappingException, RecipientRewriteTableException {
-        List<String> domains = relatedDomains(user).collect(Guavate.toImmutableList());
+        List<Domain> domains = relatedDomains(user).collect(Guavate.toImmutableList());
 
         return relatedAliases(user)
-            .flatMap(userPart -> domains.stream()
-                .map(domainPart -> userPart + "@" + domainPart)
-            .map(Throwing.<String, MailAddress>function(MailAddress::new).sneakyThrow()));
+            .flatMap(allowedUser -> domains.stream()
+                .map(Optional::of)
+                .map(allowedUser::withOtherDomain)
+                .map(Throwing.function(Username::asMailAddress).sneakyThrow()));
     }
 
     private boolean emailIsAnAliasOfTheConnectedUser(Username connectedUser, Username fromUser) throws RecipientRewriteTable.ErrorMappingException, RecipientRewriteTableException {
@@ -77,24 +79,29 @@ public class CanSendFromImpl implements CanSendFrom {
             .anyMatch(alias -> alias.equals(connectedUser));
     }
 
-    private Stream<String> relatedAliases(Username user) throws RecipientRewriteTableException {
+    private Stream<Username> relatedAliases(Username user) throws RecipientRewriteTableException {
         return Stream.concat(
-            Stream.of(user.getLocalPart()),
-            recipientRewriteTable.listSources(Mapping.alias(user.asString()))
-                .map(MappingSource::getFixedUser)
+            Stream.of(user),
+            recipientRewriteTable
+                .listSources(Mapping.alias(user.asString()))
+                .map(MappingSource::asUsername)
+                .flatMap(OptionalUtils::toStream)
         );
     }
 
-    private Stream<String> relatedDomains(Username user) {
+    private Stream<Domain> relatedDomains(Username user) {
         return user.getDomainPart()
             .map(Throwing.function(this::fetchDomains).sneakyThrow())
             .orElseGet(Stream::empty);
     }
 
-    private Stream<String> fetchDomains(Domain domain) throws RecipientRewriteTableException {
+    private Stream<Domain> fetchDomains(Domain domain) throws RecipientRewriteTableException {
         return Stream.concat(
-            Stream.of(domain.asString()),
-            recipientRewriteTable.listSources(Mapping.domain(domain)).map(MappingSource::getFixedDomain)
+          Stream.of(domain),
+          recipientRewriteTable
+              .listSources(Mapping.domain(domain))
+              .map(MappingSource::asDomain)
+              .flatMap(OptionalUtils::toStream)
         );
     }
 }


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


[james-project] 07/08: JAMES-3057 Add a create default method in MailboxMapper

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 d9d098d9a1b326a0266f836e6e96e8051a76384e
Author: Rene Cordier <rc...@linagora.com>
AuthorDate: Tue Feb 11 11:49:32 2020 +0700

    JAMES-3057 Add a create default method in MailboxMapper
---
 .../james/mailbox/store/StoreMailboxManager.java   |  2 +-
 .../james/mailbox/store/mail/MailboxMapper.java    | 13 +++++++-
 .../store/mail/model/MailboxMapperTest.java        | 37 +++++++++++++++++++++-
 3 files changed, 49 insertions(+), 3 deletions(-)

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 bcad33d..f023f5f 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
@@ -385,7 +385,7 @@ public class StoreMailboxManager implements MailboxManager {
                 Mailbox mailbox = doCreateMailbox(mailboxPath);
                 MailboxMapper mapper = mailboxSessionMapperFactory.getMailboxMapper(mailboxSession);
                 try {
-                    mapper.execute(Mapper.toTransaction(() -> mailboxIds.add(mapper.save(mailbox))));
+                    mapper.execute(Mapper.toTransaction(() -> mailboxIds.add(mapper.create(mailbox))));
                     // notify listeners
                     eventBus.dispatch(EventFactory.mailboxAdded()
                         .randomEventId()
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 3ca9c5f..65a7489 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
@@ -32,13 +32,24 @@ import org.apache.james.mailbox.model.MailboxPath;
 import org.apache.james.mailbox.model.search.MailboxQuery;
 import org.apache.james.mailbox.store.transaction.Mapper;
 
+import com.google.common.base.Preconditions;
+
 /**
  * Mapper for {@link Mailbox} actions. A {@link MailboxMapper} has a lifecycle from the start of a request 
  * to the end of the request.
  *
  */
 public interface MailboxMapper extends Mapper {
-    
+
+    /**
+     * Create the given {@link Mailbox} to the underlying storage
+     */
+    default MailboxId create(Mailbox mailbox) throws MailboxException {
+        Preconditions.checkArgument(mailbox.getMailboxId() == null, "A mailbox we want to create should not have a mailboxId set already");
+
+        return save(mailbox);
+    }
+
     /**
      * Save the give {@link Mailbox} to the underlying storage
      */
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java
index e90e01c..125292b 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java
@@ -82,12 +82,47 @@ public abstract class MailboxMapperTest {
     }
 
     @Test
-    void findMailboxByPathWhenAbsentShouldFail() throws MailboxException {
+    void findMailboxByPathWhenAbsentShouldFail() {
         assertThatThrownBy(() -> mailboxMapper.findMailboxByPath(MailboxPath.forUser(BENWA, "INBOX")))
             .isInstanceOf(MailboxNotFoundException.class);
     }
 
     @Test
+    void createShouldPersistTheMailbox() throws MailboxException {
+        benwaInboxMailbox.setMailboxId(null);
+        mailboxMapper.create(benwaInboxMailbox);
+
+        MailboxAssert.assertThat(mailboxMapper.findMailboxByPath(benwaInboxPath)).isEqualTo(benwaInboxMailbox);
+        MailboxAssert.assertThat(mailboxMapper.findMailboxById(benwaInboxMailbox.getMailboxId())).isEqualTo(benwaInboxMailbox);
+    }
+
+    @Test
+    void createShouldThrowWhenMailboxAlreadyExists() throws MailboxException {
+        benwaInboxMailbox.setMailboxId(null);
+        mailboxMapper.create(benwaInboxMailbox);
+
+        Mailbox mailbox = new Mailbox(benwaInboxMailbox);
+        mailbox.setMailboxId(null);
+
+        assertThatThrownBy(() -> mailboxMapper.create(mailbox))
+            .isInstanceOf(MailboxExistsException.class);
+    }
+
+    @Test
+    void createShouldSetAMailboxIdForMailbox() throws MailboxException {
+        benwaInboxMailbox.setMailboxId(null);
+        MailboxId mailboxId = mailboxMapper.create(benwaInboxMailbox);
+
+        assertThat(mailboxId).isNotNull();
+    }
+
+    @Test
+    void createShouldThrowWhenMailboxIdNotNull() {
+        assertThatThrownBy(() -> mailboxMapper.create(benwaInboxMailbox))
+            .isInstanceOf(IllegalArgumentException.class);
+    }
+
+    @Test
     void saveShouldPersistTheMailbox() throws MailboxException {
         mailboxMapper.save(benwaInboxMailbox);
         assertThat(mailboxMapper.findMailboxByPath(benwaInboxPath)).isEqualTo(benwaInboxMailbox);


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


[james-project] 04/08: JAMES-3050 Distributed admin procedures: ElasticSearch indexing

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 e0d28d9eaa4d806dea62716e109d155d4142c3dd
Author: Rene Cordier <rc...@linagora.com>
AuthorDate: Fri Feb 14 15:37:59 2020 +0700

    JAMES-3050 Distributed admin procedures: ElasticSearch indexing
---
 .../server/manage-guice-distributed-james.md       | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/src/site/markdown/server/manage-guice-distributed-james.md b/src/site/markdown/server/manage-guice-distributed-james.md
index 8996af9..6be9cb3 100644
--- a/src/site/markdown/server/manage-guice-distributed-james.md
+++ b/src/site/markdown/server/manage-guice-distributed-james.md
@@ -19,6 +19,7 @@ advanced users.
  - [Basic Monitoring](#basic-monitoring)
  - [Mailbox Event Bus](#mailbox-event-bus)
  - [Mail Processing](#mail-processing)
+ - [ElasticSearch Indexing](#elasticsearch-indexing)
 
 ## Overall architecture
 
@@ -205,3 +206,57 @@ reprocessing all the failed events registered in event dead letters.
 If for some other reason you don't need to redeliver all events, you have more fine-grained operations allowing you to
 [redeliver group events](manage-webadmin.html#Redeliver_group_events) or even just
 [redeliver a single event](manage-webadmin.html#Redeliver_a_single_event).
+
+## ElasticSearch Indexing
+
+A projection of messages is maintained in ElasticSearch via a listener plugged into the mailbox event bus in order to enable search features.
+
+You can find more information about ElasticSearch configuration [here](config-elasticsearch.html).
+
+### Usual troubleshooting procedures
+
+As explained in the [Mailbox Event Bus](#mailbox-event-bus) section, processing those events can fail sometimes.
+
+Currently, an administrator can monitor indexation failures through `ERROR` log review. You can as well
+[list failed events](manage-webadmin.html#Listing_failed_events) by looking with the group called 
+`org.apache.james.mailbox.elasticsearch.events.ElasticSearchListeningMessageSearchIndex$ElasticSearchListeningMessageSearchIndexGroup`.
+A first on-the-fly solution could be to just 
+[redeliver those group events with event dead letter](#mailbox-event-bus).
+
+If the event storage in dead-letters fails (for instance in the face of Cassandra storage exceptions), 
+then you might need to use our WebAdmin reIndexing tasks.
+
+From there, you have multiple choices. You can
+[reIndex all mails](manage-webadmin.html#ReIndexing_all_mails),
+[reIndex mails from a mailbox](manage-webadmin.html#ReIndexing_a_mailbox_mails)
+or even just [reIndex a single mail](manage-webadmin.html#ReIndexing_a_single_mail).
+
+When checking the result of a reIndexing task, you might have failed reprocessed mails. You can still use the task ID to
+[reprocess previously failed reIndexing mails](manage-webadmin.html#Fixing_previously_failed_ReIndexing).
+
+### On the fly ElasticSearch Index setting update
+
+Sometimes you might need to update index settings. Cases when an administrator might want to update index settings include:
+
+ - Scaling out: increasing the shard count might be needed.
+ - Changing string analysers, for instance to target another language
+ - etc.
+
+In order to achieve such a procedure, you need to:
+
+ - [Create the new index](https://www.elastic.co/guide/en/elasticsearch/reference/6.3/indices-create-index.html) with the right
+settings and mapping
+ - James uses two aliases on the mailbox index: one for reading (`mailboxReadAlias`) and one for writing (`mailboxWriteAlias`).
+First [add an alias](https://www.elastic.co/guide/en/elasticsearch/reference/6.3/indices-aliases.html) `mailboxWriteAlias` to that new index,
+so that now James writes on the old and new indexes, while only keeping reading on the first one
+ - Now trigger a [reindex](https://www.elastic.co/guide/en/elasticsearch/reference/6.3/docs-reindex.html)
+from the old index to the new one (this actively relies on `_source` field being present)
+ - When this is done, add the `mailboxReadAlias` alias to the new index
+ - Now that the migration to the new index is done, you can 
+[drop the old index](https://www.elastic.co/guide/en/elasticsearch/reference/6.3/indices-delete-index.html)
+ - You might want as well modify the James configuration file 
+[elasticsearch.properties](https://github.com/apache/james-project/blob/master/dockerfiles/run/guice/cassandra-rabbitmq/destination/conf/elasticsearch.properties)
+by setting the parameter `elasticsearch.index.mailbox.name` to the name of your new index. This is to avoid that James 
+re-creates index upon restart
+
+_Note_: keep in mind that reindexing can be a very long operation depending on the volume of mails you have stored.


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


[james-project] 01/08: JAMES-3066 Add method in CanSendFrom to list all the valid from addresses of a user

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 fa99c394508161794b14b24df7ad4cbf74d41342
Author: RĂ©mi KOWALSKI <rk...@linagora.com>
AuthorDate: Tue Feb 11 11:57:17 2020 +0100

    JAMES-3066 Add method in CanSendFrom to list all the  valid from addresses of a user
---
 .../java/org/apache/james/rrt/api/CanSendFrom.java |  8 +++
 .../apache/james/rrt/lib/CanSendFromContract.java  | 72 +++++++++++++++++++---
 .../org/apache/james/rrt/lib/CanSendFromImpl.java  | 37 +++++++++++
 3 files changed, 109 insertions(+), 8 deletions(-)

diff --git a/server/data/data-api/src/main/java/org/apache/james/rrt/api/CanSendFrom.java b/server/data/data-api/src/main/java/org/apache/james/rrt/api/CanSendFrom.java
index 41e495c..794f5c4 100644
--- a/server/data/data-api/src/main/java/org/apache/james/rrt/api/CanSendFrom.java
+++ b/server/data/data-api/src/main/java/org/apache/james/rrt/api/CanSendFrom.java
@@ -18,6 +18,9 @@
  ****************************************************************/
 package org.apache.james.rrt.api;
 
+import java.util.stream.Stream;
+
+import org.apache.james.core.MailAddress;
 import org.apache.james.core.Username;
 
 public interface CanSendFrom {
@@ -27,4 +30,9 @@ public interface CanSendFrom {
      */
     boolean userCanSendFrom(Username connectedUser, Username fromUser);
 
+    /**
+     * For a given user, return all the addresses he can use in the from clause of an email.
+     */
+    Stream<MailAddress> allValidFromAddressesForUser(Username user) throws RecipientRewriteTable.ErrorMappingException, RecipientRewriteTableException;
+
 }
diff --git a/server/data/data-api/src/test/java/org/apache/james/rrt/lib/CanSendFromContract.java b/server/data/data-api/src/test/java/org/apache/james/rrt/lib/CanSendFromContract.java
index b806193..13df95e 100644
--- a/server/data/data-api/src/test/java/org/apache/james/rrt/lib/CanSendFromContract.java
+++ b/server/data/data-api/src/test/java/org/apache/james/rrt/lib/CanSendFromContract.java
@@ -23,6 +23,8 @@ import static org.assertj.core.api.Assertions.assertThat;
 import org.apache.james.core.Domain;
 import org.apache.james.core.Username;
 import org.apache.james.rrt.api.CanSendFrom;
+
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 public interface CanSendFromContract {
@@ -41,33 +43,32 @@ public interface CanSendFromContract {
 
     void addGroupMapping(String group, Username user) throws Exception;
 
-
     @Test
-    default void assertUserCanSendFromShouldBeFalseWhenSenderIsNotTheUser() {
+    default void userCanSendFromShouldBeFalseWhenSenderIsNotTheUser() {
         assertThat(canSendFrom().userCanSendFrom(USER, OTHER_USER)).isFalse();
     }
 
     @Test
-    default void assertUserCanSendFromShouldBeTrueWhenSenderIsTheUser() {
+    default void userCanSendFromShouldBeTrueWhenSenderIsTheUser() {
         assertThat(canSendFrom().userCanSendFrom(USER, USER)).isTrue();
     }
 
     @Test
-    default void assertUserCanSendFromShouldBeFalseWhenSenderIsAnAliasOfAnotherUser() throws Exception {
+    default void userCanSendFromShouldBeFalseWhenSenderIsAnAliasOfAnotherUser() throws Exception {
         addAliasMapping(USER_ALIAS, OTHER_USER);
 
         assertThat(canSendFrom().userCanSendFrom(USER, USER_ALIAS)).isFalse();
     }
 
     @Test
-    default void assertUserCanSendFromShouldBeTrueWhenSenderIsAnAliasOfTheUser() throws Exception {
+    default void userCanSendFromShouldBeTrueWhenSenderIsAnAliasOfTheUser() throws Exception {
         addAliasMapping(USER_ALIAS, USER);
 
         assertThat(canSendFrom().userCanSendFrom(USER, USER_ALIAS)).isTrue();
     }
 
     @Test
-    default void assertUserCanSendFromShouldBeTrueWhenSenderIsAnAliasOfAnAliasOfTheUser() throws Exception {
+    default void userCanSendFromShouldBeTrueWhenSenderIsAnAliasOfAnAliasOfTheUser() throws Exception {
         Username userAliasBis = Username.of("aliasbis@" + DOMAIN.asString());
         addAliasMapping(userAliasBis, USER_ALIAS);
         addAliasMapping(USER_ALIAS, USER);
@@ -76,7 +77,7 @@ public interface CanSendFromContract {
     }
 
     @Test
-    default void assertUserCanSendFromShouldBeTrueWhenSenderIsAnAliasOfTheDomainUser() throws Exception {
+    default void userCanSendFromShouldBeTrueWhenSenderIsAnAliasOfTheDomainUser() throws Exception {
         Username fromUser = Username.of(USER.getLocalPart() + "@" + OTHER_DOMAIN.asString());
 
         addDomainMapping(OTHER_DOMAIN, DOMAIN);
@@ -85,7 +86,7 @@ public interface CanSendFromContract {
     }
 
     @Test
-    default void assertUserCanSendFromShouldBeFalseWhenWhenSenderIsAnAliasOfTheUserFromAGroupAlias() throws Exception {
+    default void userCanSendFromShouldBeFalseWhenWhenSenderIsAnAliasOfTheUserFromAGroupAlias() throws Exception {
         Username fromGroup = Username.of("group@example.com");
 
         addGroupMapping("group@example.com", USER);
@@ -93,4 +94,59 @@ public interface CanSendFromContract {
         assertThat(canSendFrom().userCanSendFrom(USER, fromGroup)).isFalse();
 
     }
+
+    @Test
+    default void allValidFromAddressesShouldContainOnlyUserAddressWhenUserHasNoAlias() throws Exception {
+        assertThat(canSendFrom().allValidFromAddressesForUser(USER))
+            .containsExactly(USER.asMailAddress());
+    }
+
+    @Test
+    default void allValidFromAddressesShouldContainOnlyUserAddressWhenUserHasNoAliasAndAnotherUserHasOne() throws Exception {
+        addAliasMapping(USER_ALIAS, OTHER_USER);
+        assertThat(canSendFrom().allValidFromAddressesForUser(USER))
+            .containsExactly(USER.asMailAddress());
+    }
+
+    @Test
+    default void allValidFromAddressesShouldContainUserAddressAndAnAliasOfTheUser() throws Exception {
+        addAliasMapping(USER_ALIAS, USER);
+
+        assertThat(canSendFrom().allValidFromAddressesForUser(USER))
+            .containsExactlyInAnyOrder(USER.asMailAddress(), USER_ALIAS.asMailAddress());
+    }
+
+    @Test
+    @Disabled("Recursive aliases are not supported yet")
+    default void allValidFromAddressesFromShouldBeTrueWhenSenderIsAnAliasOfAnAliasOfTheUser() throws Exception {
+        Username userAliasBis = Username.of("aliasbis@" + DOMAIN.asString());
+        addAliasMapping(userAliasBis, USER_ALIAS);
+        addAliasMapping(USER_ALIAS, USER);
+
+        assertThat(canSendFrom().allValidFromAddressesForUser(USER))
+            .containsExactlyInAnyOrder(USER.asMailAddress(), USER_ALIAS.asMailAddress(), userAliasBis.asMailAddress());
+    }
+
+    @Test
+    default void allValidFromAddressesShouldContainUserAddressAndAnAliasOfTheDomainUser() throws Exception {
+        Username fromUser = Username.of(USER.getLocalPart() + "@" + OTHER_DOMAIN.asString());
+
+        addDomainMapping(OTHER_DOMAIN, DOMAIN);
+
+        assertThat(canSendFrom().allValidFromAddressesForUser(USER))
+            .containsExactlyInAnyOrder(USER.asMailAddress(), fromUser.asMailAddress());
+    }
+
+    @Test
+    default void allValidFromAddressesShouldContainUserAddressAndAnAliasOfTheDomainUserFromAnotherDomain() throws Exception {
+        Username userAliasOtherDomain = Username.of(USER_ALIAS.getLocalPart() + "@" + OTHER_DOMAIN.asString());
+
+        addDomainMapping(OTHER_DOMAIN, DOMAIN);
+        addAliasMapping(userAliasOtherDomain, USER);
+
+        Username userAliasMainDomain = Username.of(USER_ALIAS.getLocalPart() + "@" + DOMAIN.asString());
+        Username userOtherDomain = Username.of(USER.getLocalPart() + "@" + OTHER_DOMAIN.asString());
+        assertThat(canSendFrom().allValidFromAddressesForUser(USER))
+            .containsExactlyInAnyOrder(USER.asMailAddress(), userAliasOtherDomain.asMailAddress(), userAliasMainDomain.asMailAddress(), userOtherDomain.asMailAddress());
+    }
 }
diff --git a/server/data/data-library/src/main/java/org/apache/james/rrt/lib/CanSendFromImpl.java b/server/data/data-library/src/main/java/org/apache/james/rrt/lib/CanSendFromImpl.java
index e82cbe1..97130c6 100644
--- a/server/data/data-library/src/main/java/org/apache/james/rrt/lib/CanSendFromImpl.java
+++ b/server/data/data-library/src/main/java/org/apache/james/rrt/lib/CanSendFromImpl.java
@@ -22,15 +22,22 @@ import static org.apache.james.rrt.lib.Mapping.Type.Alias;
 import static org.apache.james.rrt.lib.Mapping.Type.Domain;
 
 import java.util.EnumSet;
+import java.util.List;
+import java.util.stream.Stream;
 
 import javax.inject.Inject;
 
+import org.apache.james.core.Domain;
+import org.apache.james.core.MailAddress;
 import org.apache.james.core.Username;
 import org.apache.james.rrt.api.CanSendFrom;
 import org.apache.james.rrt.api.RecipientRewriteTable;
 import org.apache.james.rrt.api.RecipientRewriteTableException;
 import org.apache.james.util.OptionalUtils;
 
+import com.github.fge.lambdas.Throwing;
+import com.github.steveash.guavate.Guavate;
+
 public class CanSendFromImpl implements CanSendFrom {
 
     public static final EnumSet<Mapping.Type> ALIAS_TYPES_ACCEPTED_IN_FROM = EnumSet.of(Alias, Domain);
@@ -50,6 +57,16 @@ public class CanSendFromImpl implements CanSendFrom {
         }
     }
 
+    @Override
+    public Stream<MailAddress> allValidFromAddressesForUser(Username user) throws RecipientRewriteTable.ErrorMappingException, RecipientRewriteTableException {
+        List<String> domains = relatedDomains(user).collect(Guavate.toImmutableList());
+
+        return relatedAliases(user)
+            .flatMap(userPart -> domains.stream()
+                .map(domainPart -> userPart + "@" + domainPart)
+            .map(Throwing.<String, MailAddress>function(MailAddress::new).sneakyThrow()));
+    }
+
     private boolean emailIsAnAliasOfTheConnectedUser(Username connectedUser, Username fromUser) throws RecipientRewriteTable.ErrorMappingException, RecipientRewriteTableException {
         return fromUser.getDomainPart().isPresent()
             && recipientRewriteTable.getResolvedMappings(fromUser.getLocalPart(), fromUser.getDomainPart().get(), ALIAS_TYPES_ACCEPTED_IN_FROM)
@@ -60,4 +77,24 @@ public class CanSendFromImpl implements CanSendFrom {
             .anyMatch(alias -> alias.equals(connectedUser));
     }
 
+    private Stream<String> relatedAliases(Username user) throws RecipientRewriteTableException {
+        return Stream.concat(
+            Stream.of(user.getLocalPart()),
+            recipientRewriteTable.listSources(Mapping.alias(user.asString()))
+                .map(MappingSource::getFixedUser)
+        );
+    }
+
+    private Stream<String> relatedDomains(Username user) {
+        return user.getDomainPart()
+            .map(Throwing.function(this::fetchDomains).sneakyThrow())
+            .orElseGet(Stream::empty);
+    }
+
+    private Stream<String> fetchDomains(Domain domain) throws RecipientRewriteTableException {
+        return Stream.concat(
+            Stream.of(domain.asString()),
+            recipientRewriteTable.listSources(Mapping.domain(domain)).map(MappingSource::getFixedDomain)
+        );
+    }
 }


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


[james-project] 05/08: JAMES-3056 renaming mailboxes doesn't need ACL to be involved

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 05c0fab6e0334f71cf378d126f4597aea4e20d22
Author: Tran Tien Duc <dt...@linagora.com>
AuthorDate: Wed Feb 12 16:51:11 2020 +0700

    JAMES-3056 renaming mailboxes doesn't need ACL to be involved
---
 .../cassandra/mail/CassandraMailboxMapper.java     | 16 ++--
 .../cassandra/mail/CassandraMailboxMapperTest.java | 94 ----------------------
 2 files changed, 8 insertions(+), 102 deletions(-)

diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java
index 09ed684..5b01741 100644
--- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java
+++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java
@@ -21,7 +21,6 @@ package org.apache.james.mailbox.cassandra.mail;
 
 import java.util.Collections;
 import java.util.List;
-import java.util.Optional;
 import java.util.StringTokenizer;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
@@ -177,13 +176,14 @@ public class CassandraMailboxMapper implements MailboxMapper {
     }
 
     private boolean trySave(Mailbox cassandraMailbox, CassandraId cassandraId) {
-        boolean isCreated = mailboxPathV2DAO.save(cassandraMailbox.generateAssociatedPath(), cassandraId).block();
-        if (isCreated) {
-            Optional<Mailbox> simpleMailbox = retrieveMailbox(cassandraId).blockOptional();
-            simpleMailbox.ifPresent(mbx -> mailboxPathV2DAO.delete(mbx.generateAssociatedPath()).block());
-            mailboxDAO.save(cassandraMailbox).block();
-        }
-        return isCreated;
+        return mailboxPathV2DAO.save(cassandraMailbox.generateAssociatedPath(), cassandraId)
+            .filter(isCreated -> isCreated)
+            .flatMap(mailboxHasCreated -> mailboxDAO.retrieveMailbox(cassandraId)
+                .flatMap(mailbox -> mailboxPathV2DAO.delete(mailbox.generateAssociatedPath()))
+                .then(mailboxDAO.save(cassandraMailbox))
+                .thenReturn(true))
+            .switchIfEmpty(Mono.just(false))
+            .block();
     }
 
     private CassandraId retrieveId(Mailbox cassandraMailbox) {
diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java
index b2854eb..e62e158 100644
--- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java
+++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java
@@ -162,68 +162,6 @@ class CassandraMailboxMapperTest {
         }
 
         @Test
-        void saveOnRenameThenFailToGetACLShouldBeConsistentWhenFindByInbox() throws Exception {
-            testee.save(inbox);
-
-            when(aclMapper.getACL(inboxId))
-                .thenReturn(Mono.error(new RuntimeException("mock exception")))
-                .thenCallRealMethod();
-
-            doQuietly(() -> testee.save(inboxRenamed));
-
-            SoftAssertions.assertSoftly(Throwing.consumer(softly -> {
-                softly(softly)
-                    .assertThat(testee.findMailboxById(inboxId))
-                    .isEqualTo(inbox);
-                softly(softly)
-                    .assertThat(testee.findMailboxByPath(inboxPath))
-                    .isEqualTo(inbox);
-                softly.assertThat(testee.findMailboxWithPathLike(inboxSearchQuery))
-                    .hasOnlyOneElementSatisfying(searchMailbox -> softly(softly)
-                        .assertThat(searchMailbox)
-                        .isEqualTo(inbox));
-            }));
-        }
-
-        @Disabled("JAMES-3056 returning two mailboxes with same name and id")
-        @Test
-        void saveOnRenameThenFailToGetACLShouldBeConsistentWhenFindAll() throws Exception {
-            testee.save(inbox);
-
-            when(aclMapper.getACL(inboxId))
-                .thenReturn(Mono.error(new RuntimeException("mock exception")))
-                .thenCallRealMethod();
-
-            doQuietly(() -> testee.save(inboxRenamed));
-
-            SoftAssertions.assertSoftly(Throwing.consumer(softly -> {
-                softly.assertThat(testee.findMailboxWithPathLike(allMailboxesSearchQuery))
-                    .hasOnlyOneElementSatisfying(searchMailbox -> softly(softly)
-                        .assertThat(searchMailbox)
-                        .isEqualTo(inbox));
-            }));
-        }
-
-        @Disabled("JAMES-3056 find by renamed name returns unexpected results")
-        @Test
-        void saveOnRenameThenFailToGetACLShouldBeConsistentWhenFindByRenamedInbox() throws Exception {
-            testee.save(inbox);
-
-            when(aclMapper.getACL(inboxId))
-                .thenReturn(Mono.error(new RuntimeException("mock exception")))
-                .thenCallRealMethod();
-
-            doQuietly(() -> testee.save(inboxRenamed));
-
-            SoftAssertions.assertSoftly(Throwing.consumer(softly -> {
-                softly.assertThatThrownBy(() -> testee.findMailboxByPath(inboxPathRenamed))
-                    .isInstanceOf(MailboxNotFoundException.class);
-                softly.assertThat(testee.findMailboxWithPathLike(inboxRenamedSearchQuery))
-                    .isEmpty();
-            }));
-        }
-
-        @Test
         void saveOnRenameThenFailToRetrieveMailboxShouldBeConsistentWhenFindByInbox() throws Exception {
             testee.save(inbox);
 
@@ -506,38 +444,6 @@ class CassandraMailboxMapperTest {
 
         @Disabled("JAMES-3056 mailbox name is not updated to INBOX_RENAMED")
         @Test
-        void renameAfterRenameFailOnGetACLShouldRenameTheMailbox() throws Exception {
-            testee.save(inbox);
-
-            when(aclMapper.getACL(inboxId))
-                .thenReturn(Mono.error(new RuntimeException("mock exception")))
-                .thenCallRealMethod();
-
-            doQuietly(() -> testee.save(inboxRenamed));
-            doQuietly(() -> testee.save(inboxRenamed));
-
-            SoftAssertions.assertSoftly(Throwing.consumer(softly -> {
-                softly(softly)
-                    .assertThat(testee.findMailboxById(inboxId))
-                    .isEqualTo(inboxRenamed);
-                softly(softly)
-                    .assertThat(testee.findMailboxByPath(inboxPathRenamed))
-                    .isEqualTo(inboxRenamed);
-                softly.assertThat(testee.findMailboxWithPathLike(inboxSearchQuery))
-                    .isEmpty();
-                softly.assertThat(testee.findMailboxWithPathLike(inboxRenamedSearchQuery))
-                    .hasOnlyOneElementSatisfying(searchMailbox -> softly(softly)
-                        .assertThat(searchMailbox)
-                        .isEqualTo(inboxRenamed));
-                softly.assertThat(testee.findMailboxWithPathLike(allMailboxesSearchQuery))
-                    .hasOnlyOneElementSatisfying(searchMailbox -> softly(softly)
-                        .assertThat(searchMailbox)
-                        .isEqualTo(inboxRenamed));
-            }));
-        }
-
-        @Disabled("JAMES-3056 mailbox name is not updated to INBOX_RENAMED")
-        @Test
         void renameAfterRenameFailOnDeletePathShouldRenameTheMailbox() throws Exception {
             testee.save(inbox);
 


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


[james-project] 06/08: JAMES-3057 Rename some variables in StoreMailboxManager

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 40b671e4fc21c2cb7c848833917179360f9b5589
Author: Rene Cordier <rc...@linagora.com>
AuthorDate: Tue Feb 11 11:27:40 2020 +0700

    JAMES-3057 Rename some variables in StoreMailboxManager
---
 .../james/mailbox/store/StoreMailboxManager.java   | 26 +++++++++++-----------
 1 file changed, 13 insertions(+), 13 deletions(-)

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 940c88d..bcad33d 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
@@ -353,20 +353,20 @@ public class StoreMailboxManager implements MailboxManager {
 
         return intermediatePaths
             .stream()
-            .flatMap(Throwing.<MailboxPath, Stream<MailboxId>>function(mailbox -> manageMailboxCreation(mailboxSession, isRootPath, mailbox)).sneakyThrow())
+            .flatMap(Throwing.<MailboxPath, Stream<MailboxId>>function(mailboxPath -> manageMailboxCreation(mailboxSession, isRootPath, mailboxPath)).sneakyThrow())
             .collect(Guavate.toImmutableList());
     }
 
-    private Stream<MailboxId> manageMailboxCreation(MailboxSession mailboxSession, boolean isRootPath, MailboxPath mailbox) throws MailboxException {
-        if (mailbox.isInbox()) {
+    private Stream<MailboxId> manageMailboxCreation(MailboxSession mailboxSession, boolean isRootPath, MailboxPath mailboxPath) throws MailboxException {
+        if (mailboxPath.isInbox()) {
             if (hasInbox(mailboxSession)) {
-                return duplicatedINBOXCreation(isRootPath, mailbox);
+                return duplicatedINBOXCreation(isRootPath, mailboxPath);
             }
 
             return performConcurrentMailboxCreation(mailboxSession, MailboxPath.inbox(mailboxSession)).stream();
         }
 
-        return performConcurrentMailboxCreation(mailboxSession, mailbox).stream();
+        return performConcurrentMailboxCreation(mailboxSession, mailboxPath).stream();
     }
 
 
@@ -378,24 +378,24 @@ public class StoreMailboxManager implements MailboxManager {
         return Stream.empty();
     }
 
-    private List<MailboxId> performConcurrentMailboxCreation(MailboxSession mailboxSession, MailboxPath mailbox) throws MailboxException {
+    private List<MailboxId> performConcurrentMailboxCreation(MailboxSession mailboxSession, MailboxPath mailboxPath) throws MailboxException {
         List<MailboxId> mailboxIds = new ArrayList<>();
-        locker.executeWithLock(mailbox, (LockAwareExecution<Void>) () -> {
-            if (!mailboxExists(mailbox, mailboxSession)) {
-                Mailbox m = doCreateMailbox(mailbox);
+        locker.executeWithLock(mailboxPath, (LockAwareExecution<Void>) () -> {
+            if (!mailboxExists(mailboxPath, mailboxSession)) {
+                Mailbox mailbox = doCreateMailbox(mailboxPath);
                 MailboxMapper mapper = mailboxSessionMapperFactory.getMailboxMapper(mailboxSession);
                 try {
-                    mapper.execute(Mapper.toTransaction(() -> mailboxIds.add(mapper.save(m))));
+                    mapper.execute(Mapper.toTransaction(() -> mailboxIds.add(mapper.save(mailbox))));
                     // notify listeners
                     eventBus.dispatch(EventFactory.mailboxAdded()
                         .randomEventId()
                         .mailboxSession(mailboxSession)
-                        .mailbox(m)
+                        .mailbox(mailbox)
                         .build(),
-                        new MailboxIdRegistrationKey(m.getMailboxId()))
+                        new MailboxIdRegistrationKey(mailbox.getMailboxId()))
                         .block();
                 } catch (MailboxExistsException e) {
-                    LOGGER.info("{} mailbox was created concurrently", m.generateAssociatedPath());
+                    LOGGER.info("{} mailbox was created concurrently", mailbox.generateAssociatedPath());
                 }
             }
             return null;


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


[james-project] 08/08: JAMES-3057 Separate the rename and create mailbox logic in CassandraMailboxMapper

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 cf844dff46c8c6f9ef99198233676250eeb51af9
Author: Rene Cordier <rc...@linagora.com>
AuthorDate: Tue Feb 11 11:49:59 2020 +0700

    JAMES-3057 Separate the rename and create mailbox logic in CassandraMailboxMapper
---
 .../cassandra/mail/CassandraMailboxMapper.java     | 24 +++++++++++
 .../cassandra/mail/CassandraMailboxMapperTest.java | 46 ++++++++++++++++++++++
 .../store/mail/model/MailboxMapperTest.java        |  4 +-
 3 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java
index 5b01741..5aebd17 100644
--- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java
+++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java
@@ -46,6 +46,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.github.steveash.guavate.Guavate;
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 
 import reactor.core.publisher.Flux;
@@ -166,6 +167,29 @@ public class CassandraMailboxMapper implements MailboxMapper {
     }
 
     @Override
+    public MailboxId create(Mailbox mailbox) throws MailboxException {
+        Preconditions.checkArgument(mailbox.getMailboxId() == null, "A mailbox we want to create should not have a mailboxId set already");
+
+        CassandraId cassandraId = CassandraId.timeBased();
+        mailbox.setMailboxId(cassandraId);
+        if (!tryCreate(mailbox, cassandraId).block()) {
+            throw new MailboxExistsException(mailbox.generateAssociatedPath().asString());
+        }
+        return cassandraId;
+    }
+
+    private Mono<Boolean> tryCreate(Mailbox cassandraMailbox, CassandraId cassandraId) {
+        return mailboxPathV2DAO.save(cassandraMailbox.generateAssociatedPath(), cassandraId)
+            .flatMap(isCreated -> {
+                if (isCreated) {
+                    return mailboxDAO.save(cassandraMailbox)
+                        .thenReturn(isCreated);
+                }
+                return Mono.just(isCreated);
+            });
+    }
+
+    @Override
     public MailboxId save(Mailbox mailbox) throws MailboxException {
         CassandraId cassandraId = retrieveId(mailbox);
         mailbox.setMailboxId(cassandraId);
diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java
index e62e158..d5fbf3e 100644
--- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java
+++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java
@@ -142,6 +142,25 @@ class CassandraMailboxMapperTest {
         }
 
         @Test
+        void createShouldBeConsistentWhenFailToPersistMailbox() {
+            doReturn(Mono.error(new RuntimeException("mock exception")))
+                .when(mailboxDAO)
+                .save(inbox);
+
+            inbox.setMailboxId(null);
+            doQuietly(() -> testee.create(inbox));
+
+            SoftAssertions.assertSoftly(softly -> {
+                softly.assertThatThrownBy(() -> testee.findMailboxByPath(inboxPath))
+                    .isInstanceOf(MailboxNotFoundException.class);
+                softly.assertThat(testee.findMailboxWithPathLike(inboxSearchQuery))
+                    .isEmpty();
+                softly.assertThat(testee.findMailboxWithPathLike(allMailboxesSearchQuery))
+                    .isEmpty();
+            });
+        }
+
+        @Test
         void saveOnCreateShouldBeConsistentWhenFailToPersistMailbox() {
             doReturn(Mono.error(new RuntimeException("mock exception")))
                 .when(mailboxDAO)
@@ -330,6 +349,33 @@ class CassandraMailboxMapperTest {
                 .isNotEqualTo(testee.findMailboxById(inboxId).getName());
         }
 
+        @Disabled("JAMES-3057 org.apache.james.mailbox.exception.MailboxNotFoundException: INBOX can not be found")
+        @Test
+        void createAfterPreviousFailedCreateShouldCreateAMailbox() {
+            when(mailboxDAO.save(inbox))
+                .thenReturn(Mono.error(new RuntimeException("mock exception")))
+                .thenCallRealMethod();
+
+            inbox.setMailboxId(null);
+            doQuietly(() -> testee.create(inbox));
+            inbox.setMailboxId(null);
+            doQuietly(() -> testee.create(inbox));
+
+            SoftAssertions.assertSoftly(Throwing.consumer(softly -> {
+                softly(softly)
+                    .assertThat(testee.findMailboxByPath(inboxPath))
+                    .isEqualTo(inbox);
+                softly.assertThat(testee.findMailboxWithPathLike(inboxSearchQuery))
+                    .hasOnlyOneElementSatisfying(searchMailbox -> softly(softly)
+                        .assertThat(searchMailbox)
+                        .isEqualTo(inbox));
+                softly.assertThat(testee.findMailboxWithPathLike(allMailboxesSearchQuery))
+                    .hasOnlyOneElementSatisfying(searchMailbox -> softly(softly)
+                        .assertThat(searchMailbox)
+                        .isEqualTo(inbox));
+            }));
+        }
+
         @Disabled("JAMES-3056 org.apache.james.mailbox.exception.MailboxNotFoundException: 'mailboxId' can not be found")
         @Test
         void saveAfterPreviousFailedSaveShouldCreateAMailbox() {
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java
index 125292b..9963ba7 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java
@@ -92,8 +92,8 @@ public abstract class MailboxMapperTest {
         benwaInboxMailbox.setMailboxId(null);
         mailboxMapper.create(benwaInboxMailbox);
 
-        MailboxAssert.assertThat(mailboxMapper.findMailboxByPath(benwaInboxPath)).isEqualTo(benwaInboxMailbox);
-        MailboxAssert.assertThat(mailboxMapper.findMailboxById(benwaInboxMailbox.getMailboxId())).isEqualTo(benwaInboxMailbox);
+        assertThat(mailboxMapper.findMailboxByPath(benwaInboxPath)).isEqualTo(benwaInboxMailbox);
+        assertThat(mailboxMapper.findMailboxById(benwaInboxMailbox.getMailboxId())).isEqualTo(benwaInboxMailbox);
     }
 
     @Test


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