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 <[email protected]> Authored: Thu Jun 23 09:27:48 2016 +0700 Committer: Benoit Tellier <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
