james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From btell...@apache.org
Subject [4/5] james-project git commit: JAMES-1759 Deleting a mailbox should also delete its children
Date Thu, 23 Jun 2016 09:32:23 GMT
JAMES-1759 Deleting a mailbox should also delete its children


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

Branch: refs/heads/master
Commit: ff3412fbe0f28560ae78407a5339bfa0fe2643bf
Parents: a1b8aea
Author: Benoit Tellier <btellier@linagora.com>
Authored: Thu Jun 23 09:27:48 2016 +0700
Committer: Benoit Tellier <btellier@linagora.com>
Committed: Thu Jun 23 16:31:27 2016 +0700

----------------------------------------------------------------------
 server/protocols/webadmin/README.adoc           |  19 +-
 .../webadmin/routes/UserMailboxesRoutes.java    |  22 ++-
 .../webadmin/service/UserMailboxesService.java  |  39 ++--
 .../james/webadmin/validation/MailboxName.java  |  40 +++++
 .../routes/UserMailboxesRoutesTest.java         | 179 +++++++++++++++++--
 5 files changed, 247 insertions(+), 52 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/james-project/blob/ff3412fb/server/protocols/webadmin/README.adoc
----------------------------------------------------------------------
diff --git a/server/protocols/webadmin/README.adoc b/server/protocols/webadmin/README.adoc
index adc3aa5..40942fd 100644
--- a/server/protocols/webadmin/README.adoc
+++ b/server/protocols/webadmin/README.adoc
@@ -132,11 +132,12 @@ curl -XPUT http://ip:port/users/usernameToBeUsed/mailboxes/mailboxNameToBeCreate
 ====
 
 Resource name usernameToBeUsed should be an existing user
-Resource name mailboxNameToBeCreated should not be empty
+Resource name mailboxNameToBeCreated should not be empty, nor contain # & % * characters.
 
 Response codes :
  - 204 : The mailbox now exists on the server
- - 400 : The user name does not exist
+ - 400 : Invalid mailbox name
+ - 404 : The user name does not exist
  - 500 : Internal error
 
  To create nested mailboxes, for instance a work mailbox inside the INBOX mailbox, people
should use the . separator. The sample query is :
@@ -146,7 +147,7 @@ Response codes :
  curl -XDELETE http://ip:port/users/usernameToBeUsed/mailboxes/INBOX.work
  ====
 
-=== Deleting a mailbox
+=== Deleting a mailbox and its children
 
 .bash
 ====
@@ -158,8 +159,8 @@ Resource name mailboxNameToBeCreated should not be empty
 
 Response codes :
  - 204 : The mailbox now does not exist on the server
- - 400 : The user name does not exist
- - 409 : The mailbox can not be deleted due to its children mailboxes. Delete children mailboxes
first.
+ - 400 : Invalid mailbox name
+ - 404 : The user name does not exist
  - 500 : Internal error
 
 === Testing existence of a mailbox
@@ -174,8 +175,8 @@ Resource name mailboxNameToBeCreated should not be empty
 
 Response codes :
  - 204 : The mailbox exists
- - 400 : The user name does not exist
- - 404 : The mailbox does not exist
+ - 400 : Invalid mailbox name
+ - 404 : The user name does not exist, the mailbox does not exist
  - 500 : Internal error
 
 === Listing user mailboxes
@@ -196,7 +197,7 @@ Resource name usernameToBeUsed should be an existing user
 
 Response codes :
  - 200 : The mailboxes list was successfully retrieved
- - 400 : The user name does not exist
+ - 404 : The user name does not exist
  - 500 : Internal error
 
 === Deleting user mailboxes
@@ -210,5 +211,5 @@ Resource name usernameToBeUsed should be an existing user
 
 Response codes :
  - 204 : The user do not have mailboxes anymore
- - 400 : The user name does not exist
+ - 404 : The user name does not exist
  - 500 : Internal error
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/james-project/blob/ff3412fb/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java
----------------------------------------------------------------------
diff --git a/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java
b/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java
index 127fc1b..717ef2b 100644
--- a/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java
+++ b/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java
@@ -26,6 +26,7 @@ import org.apache.james.webadmin.Routes;
 import org.apache.james.webadmin.service.UserMailboxesService;
 import org.apache.james.webadmin.utils.JsonTransformer;
 import org.apache.james.webadmin.utils.MailboxHaveChildrenException;
+import org.apache.james.webadmin.validation.MailboxName;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -55,10 +56,13 @@ public class UserMailboxesRoutes implements Routes {
 
         service.put(SPECIFIC_MAILBOX, (request, response) -> {
             try {
-                userMailboxesService.createMailbox(request.params(USER_NAME), request.params(MAILBOX_NAME));
+                userMailboxesService.createMailbox(request.params(USER_NAME), new MailboxName(request.params(MAILBOX_NAME)));
                 response.status(204);
             } catch (IllegalStateException e) {
                 LOGGER.info("Invalid put on user mailbox", e);
+                response.status(404);
+            } catch (IllegalArgumentException e) {
+                LOGGER.info("Attempt to create an invalid mailbox");
                 response.status(400);
             }
             return Constants.EMPTY_BODY;
@@ -66,14 +70,17 @@ public class UserMailboxesRoutes implements Routes {
 
         service.delete(SPECIFIC_MAILBOX, (request, response) -> {
             try {
-                userMailboxesService.deleteMailbox(request.params(USER_NAME), request.params(MAILBOX_NAME));
+                userMailboxesService.deleteMailbox(request.params(USER_NAME), new MailboxName(request.params(MAILBOX_NAME)));
                 response.status(204);
             } catch (IllegalStateException e) {
                 LOGGER.info("Invalid delete on user mailbox", e);
-                response.status(400);
+                response.status(404);
             } catch (MailboxHaveChildrenException e) {
                 LOGGER.info("Attempt to delete a mailbox with children");
                 response.status(409);
+            } catch (IllegalArgumentException e) {
+                LOGGER.info("Attempt to create an invalid mailbox");
+                response.status(400);
             }
             return Constants.EMPTY_BODY;
         });
@@ -84,20 +91,23 @@ public class UserMailboxesRoutes implements Routes {
                 response.status(204);
             } catch (IllegalStateException e) {
                 LOGGER.info("Invalid delete on user mailboxes", e);
-                response.status(400);
+                response.status(404);
             }
             return Constants.EMPTY_BODY;
         });
 
         service.get(SPECIFIC_MAILBOX, (request, response) -> {
             try {
-                if (userMailboxesService.testMailboxExists(request.params(USER_NAME), request.params(MAILBOX_NAME)))
{
+                if (userMailboxesService.testMailboxExists(request.params(USER_NAME), new
MailboxName(request.params(MAILBOX_NAME)))) {
                     response.status(204);
                 } else {
                     response.status(404);
                 }
             } catch (IllegalStateException e) {
                 LOGGER.info("Invalid get on user mailbox", e);
+                response.status(404);
+            } catch (IllegalArgumentException e) {
+                LOGGER.info("Attempt to create an invalid mailbox");
                 response.status(400);
             }
             return Constants.EMPTY_BODY;
@@ -109,7 +119,7 @@ public class UserMailboxesRoutes implements Routes {
                 return userMailboxesService.listMailboxes(request.params(USER_NAME));
             } catch (IllegalStateException e) {
                 LOGGER.info("Invalid get on user mailboxes", e);
-                response.status(400);
+                response.status(404);
                 return Constants.EMPTY_BODY;
             }
         }, jsonTransformer);

http://git-wip-us.apache.org/repos/asf/james-project/blob/ff3412fb/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java
----------------------------------------------------------------------
diff --git a/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java
b/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java
index 7043d25..ef3945e 100644
--- a/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java
+++ b/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java
@@ -37,6 +37,7 @@ import org.apache.james.user.api.UsersRepositoryException;
 import org.apache.james.util.streams.ImmutableCollectors;
 import org.apache.james.webadmin.model.MailboxResponse;
 import org.apache.james.webadmin.utils.MailboxHaveChildrenException;
+import org.apache.james.webadmin.validation.MailboxName;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -59,13 +60,12 @@ public class UserMailboxesService {
         this.usersRepository = usersRepository;
     }
 
-    public void createMailbox(String username, String mailboxName) throws MailboxException,
UsersRepositoryException {
+    public void createMailbox(String username, MailboxName mailboxName) throws MailboxException,
UsersRepositoryException {
         usernamePreconditions(username);
-        Preconditions.checkArgument(!Strings.isNullOrEmpty(mailboxName));
         MailboxSession mailboxSession = mailboxManager.createSystemSession(USER_NAME, LOGGER);
         try {
             mailboxManager.createMailbox(
-                convertToMailboxPath(username, mailboxName, mailboxSession),
+                convertToMailboxPath(username, mailboxName.asString(), mailboxSession),
                 mailboxSession);
         } catch (MailboxExistsException e) {
             LOGGER.info("Attempt to create mailbox {} for user {} that already exists", mailboxName,
username);
@@ -88,40 +88,27 @@ public class UserMailboxesService {
             .collect(ImmutableCollectors.toImmutableList());
     }
 
-    public boolean testMailboxExists(String username, String mailboxName) throws MailboxException,
UsersRepositoryException {
+    public boolean testMailboxExists(String username, MailboxName mailboxName) throws MailboxException,
UsersRepositoryException {
         usernamePreconditions(username);
-        Preconditions.checkArgument(!Strings.isNullOrEmpty(mailboxName));
         MailboxSession mailboxSession = mailboxManager.createSystemSession(USER_NAME, LOGGER);
         return mailboxManager.mailboxExists(
-            convertToMailboxPath(username, mailboxName, mailboxSession),
+            convertToMailboxPath(username, mailboxName.asString(), mailboxSession),
             mailboxSession);
     }
 
-    public void deleteMailbox(String username, String mailboxName) throws MailboxException,
UsersRepositoryException, MailboxHaveChildrenException {
+    public void deleteMailbox(String username, MailboxName mailboxName) throws MailboxException,
UsersRepositoryException, MailboxHaveChildrenException {
         usernamePreconditions(username);
-        Preconditions.checkArgument(!Strings.isNullOrEmpty(mailboxName));
         MailboxSession mailboxSession = mailboxManager.createSystemSession(USER_NAME, LOGGER);
-        MailboxPath mailboxPath = convertToMailboxPath(username, mailboxName, mailboxSession);
-        try {
-            if (!haveChildren(mailboxPath, mailboxSession)) {
-                deleteMailbox(mailboxSession, mailboxPath);
-            } else {
-                throw new MailboxHaveChildrenException(mailboxName);
-            }
-        } catch (MailboxNotFoundException e) {
-            LOGGER.info("Attempt to delete mailbox {} for user {} that does not exists",
mailboxPath.getName(), mailboxPath.getUser());
-        }
+        MailboxPath mailboxPath = convertToMailboxPath(username, mailboxName.asString(),
mailboxSession);
+        listChildren(mailboxPath, mailboxSession)
+            .forEach(Throwing.consumer(path -> deleteMailbox(mailboxSession, path)));
     }
 
-    private boolean haveChildren(MailboxPath mailboxPath, MailboxSession mailboxSession)
throws MailboxException {
-        return mailboxManager.search(
-            MailboxQuery.builder()
-                .base(mailboxPath)
-                .build(), mailboxSession)
+    private Stream<MailboxPath> listChildren(MailboxPath mailboxPath, MailboxSession
mailboxSession) throws MailboxException {
+        return mailboxManager.search(createUserMailboxesQuery(mailboxPath.getUser()), mailboxSession)
             .stream()
-            .findAny()
-            .map(mailboxMetaData -> mailboxMetaData.inferiors() == MailboxMetaData.Children.HAS_CHILDREN)
-            .orElseThrow(() -> new MailboxNotFoundException(mailboxPath));
+            .map(MailboxMetaData::getPath)
+            .filter(path -> path.getHierarchyLevels(mailboxSession.getPathDelimiter()).contains(mailboxPath));
     }
 
     private void deleteMailbox(MailboxSession mailboxSession, MailboxPath mailboxPath) throws
MailboxException {

http://git-wip-us.apache.org/repos/asf/james-project/blob/ff3412fb/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/validation/MailboxName.java
----------------------------------------------------------------------
diff --git a/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/validation/MailboxName.java
b/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/validation/MailboxName.java
new file mode 100644
index 0000000..fdd079d
--- /dev/null
+++ b/server/protocols/webadmin/src/main/java/org/apache/james/webadmin/validation/MailboxName.java
@@ -0,0 +1,40 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.webadmin.validation;
+
+import com.google.common.base.CharMatcher;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
+
+public class MailboxName {
+
+    public static final CharMatcher INVALID_CHARS_MATCHER = CharMatcher.anyOf("%*&#");
+    private final String mailboxName;
+
+    public MailboxName(String mailboxName) {
+        Preconditions.checkArgument(!Strings.isNullOrEmpty(mailboxName));
+        Preconditions.checkArgument(INVALID_CHARS_MATCHER.matchesNoneOf(mailboxName));
+        this.mailboxName = mailboxName;
+    }
+
+    public String asString() {
+        return mailboxName;
+    }
+}

http://git-wip-us.apache.org/repos/asf/james-project/blob/ff3412fb/server/protocols/webadmin/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
----------------------------------------------------------------------
diff --git a/server/protocols/webadmin/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
b/server/protocols/webadmin/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
index a1782ab..d2ebfb5 100644
--- a/server/protocols/webadmin/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
+++ b/server/protocols/webadmin/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
@@ -109,36 +109,156 @@ public class UserMailboxesRoutesTest {
             when()
                 .get()
             .then()
-                .statusCode(400);
+                .statusCode(404);
         }
 
         @Test
-        public void getShouldReturnUserErrorWithNonExistingUser() throws Exception {
+        public void getShouldReturnNotFoundWithNonExistingUser() throws Exception {
             when(usersRepository.contains(USERNAME)).thenReturn(false);
 
             when()
                 .get(MAILBOX_NAME)
             .then()
-                .statusCode(400);
+                .statusCode(404);
         }
 
         @Test
-        public void putShouldReturnUserErrorWithNonExistingUser() throws Exception {
+        public void putShouldReturnNotFoundWithNonExistingUser() throws Exception {
             when(usersRepository.contains(USERNAME)).thenReturn(false);
 
             when()
                 .put(MAILBOX_NAME)
             .then()
-                .statusCode(400);
+                .statusCode(404);
         }
 
         @Test
-        public void deleteShouldReturnUserErrorWithNonExistingUser() throws Exception {
+        public void deleteShouldReturnNotFoundWithNonExistingUser() throws Exception {
             when(usersRepository.contains(USERNAME)).thenReturn(false);
 
             when()
                 .put(MAILBOX_NAME)
             .then()
+                .statusCode(404);
+        }
+
+        @Test
+        public void getShouldReturnUserErrorWithInvalidWildcardMailboxName() throws Exception
{
+            when(usersRepository.contains(USERNAME)).thenReturn(false);
+
+            when()
+                .get(MAILBOX_NAME + "*")
+            .then()
+                .statusCode(400);
+        }
+
+        @Test
+        public void putShouldReturnUserErrorWithInvalidWildcardMailboxName() throws Exception
{
+            when(usersRepository.contains(USERNAME)).thenReturn(false);
+
+            when()
+                .put(MAILBOX_NAME+ "*")
+            .then()
+                .statusCode(400);
+        }
+
+        @Test
+        public void deleteShouldReturnUserErrorWithInvalidWildcardMailboxName() throws Exception
{
+            when(usersRepository.contains(USERNAME)).thenReturn(false);
+
+            when()
+                .put(MAILBOX_NAME + "*")
+            .then()
+                .statusCode(400);
+        }
+
+        @Test
+        public void getShouldReturnUserErrorWithInvalidPercentMailboxName() throws Exception
{
+            when(usersRepository.contains(USERNAME)).thenReturn(false);
+
+            when()
+                .get(MAILBOX_NAME + "%")
+            .then()
+                .statusCode(400);
+        }
+
+        @Test
+        public void putShouldReturnUserErrorWithInvalidPercentMailboxName() throws Exception
{
+            when(usersRepository.contains(USERNAME)).thenReturn(false);
+
+            when()
+                .put(MAILBOX_NAME+ "%")
+            .then()
+                .statusCode(400);
+        }
+
+        @Test
+        public void deleteShouldReturnUserErrorWithInvalidPercentMailboxName() throws Exception
{
+            when(usersRepository.contains(USERNAME)).thenReturn(false);
+
+            when()
+                .put(MAILBOX_NAME + "%")
+            .then()
+                .statusCode(400);
+        }
+
+        @Test
+        public void getShouldReturnUserErrorWithInvalidSharpMailboxName() throws Exception
{
+            when(usersRepository.contains(USERNAME)).thenReturn(false);
+
+            when()
+                .get(MAILBOX_NAME + "#")
+            .then()
+                .statusCode(400);
+        }
+
+        @Test
+        public void putShouldReturnUserErrorWithInvalidSharpMailboxName() throws Exception
{
+            when(usersRepository.contains(USERNAME)).thenReturn(false);
+
+            when()
+                .put(MAILBOX_NAME+ "#")
+            .then()
+                .statusCode(400);
+        }
+
+        @Test
+        public void deleteShouldReturnUserErrorWithInvalidSharpMailboxName() throws Exception
{
+            when(usersRepository.contains(USERNAME)).thenReturn(false);
+
+            when()
+                .put(MAILBOX_NAME + "#")
+            .then()
+                .statusCode(400);
+        }
+
+        @Test
+        public void getShouldReturnUserErrorWithInvalidAndMailboxName() throws Exception
{
+            when(usersRepository.contains(USERNAME)).thenReturn(false);
+
+            when()
+                .get(MAILBOX_NAME + "&")
+            .then()
+                .statusCode(400);
+        }
+
+        @Test
+        public void putShouldReturnUserErrorWithInvalidAndMailboxName() throws Exception
{
+            when(usersRepository.contains(USERNAME)).thenReturn(false);
+
+            when()
+                .put(MAILBOX_NAME+ "&")
+            .then()
+                .statusCode(400);
+        }
+
+        @Test
+        public void deleteShouldReturnUserErrorWithInvalidAndMailboxName() throws Exception
{
+            when(usersRepository.contains(USERNAME)).thenReturn(false);
+
+            when()
+                .put(MAILBOX_NAME + "&")
+            .then()
                 .statusCode(400);
         }
 
@@ -149,7 +269,7 @@ public class UserMailboxesRoutesTest {
             when()
                 .delete()
             .then()
-                .statusCode(400);
+                .statusCode(404);
         }
 
         @Test
@@ -298,7 +418,7 @@ public class UserMailboxesRoutesTest {
         }
 
         @Test
-        public void deleteShouldReturnAConflictWhenMailboxHasChildren() {
+        public void deleteShouldReturnOkWhenMailboxHasChildren() {
             with()
                 .put(MAILBOX_NAME);
 
@@ -308,7 +428,44 @@ public class UserMailboxesRoutesTest {
             when()
                 .delete(MAILBOX_NAME)
             .then()
-                .statusCode(409);
+                .statusCode(204);
+        }
+
+        @Test
+        public void deleteShouldDeleteAMailboxAndItsChildren() {
+            with()
+                .put(MAILBOX_NAME);
+
+            with()
+                .put(MAILBOX_NAME + ".child");
+
+            with()
+                .delete(MAILBOX_NAME);
+
+            when()
+                .get()
+            .then()
+                .statusCode(200)
+                .body(is("[]"));
+        }
+
+        @Test
+        public void deleteShouldNotDeleteUnrelatedMailbox() {
+            String mailboxName = MAILBOX_NAME + "!child";
+            with()
+                .put(MAILBOX_NAME);
+
+            with()
+                .put(mailboxName);
+
+            with()
+                .delete(MAILBOX_NAME);
+
+            when()
+                .get()
+            .then()
+                .statusCode(200)
+                .body(is("[{\"mailboxName\":\"" + mailboxName + "\"}]"));
         }
 
         @Test
@@ -388,7 +545,7 @@ public class UserMailboxesRoutesTest {
 
         @Test
         public void deleteShouldGenerateInternalErrorOnUnknownExceptionOnDelete() throws
Exception {
-            when(mailboxManager.search(any(), any())).thenReturn(ImmutableList.of(new SimpleMailboxMetaData(new
MailboxPath("#private", USERNAME, "any"), '.')));
+            when(mailboxManager.search(any(), any())).thenReturn(ImmutableList.of(new SimpleMailboxMetaData(new
MailboxPath("#private", USERNAME, MAILBOX_NAME), '.')));
             doThrow(new RuntimeException()).when(mailboxManager).deleteMailbox(any(), any());
 
             when()
@@ -409,7 +566,7 @@ public class UserMailboxesRoutesTest {
 
         @Test
         public void deleteShouldGenerateInternalErrorOnUnknownMailboxExceptionOnDelete()
throws Exception {
-            when(mailboxManager.search(any(), any())).thenReturn(ImmutableList.of(new SimpleMailboxMetaData(new
MailboxPath("#private", USERNAME, "any"), '.')));
+            when(mailboxManager.search(any(), any())).thenReturn(ImmutableList.of(new SimpleMailboxMetaData(new
MailboxPath("#private", USERNAME, MAILBOX_NAME), '.')));
             doThrow(new MailboxException()).when(mailboxManager).deleteMailbox(any(), any());
 
             when()


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


Mime
View raw message