You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2016/06/23 09:32:23 UTC
[4/5] james-project git commit: JAMES-1759 Deleting a mailbox should
also delete its children
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 <bt...@linagora.com>
Authored: Thu Jun 23 09:27:48 2016 +0700
Committer: Benoit Tellier <bt...@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