You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by rc...@apache.org on 2023/06/28 09:44:09 UTC

[james-project] 01/02: JAMES-3918 Force deletion of user mailboxes (#1608)

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

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

commit 6ada84438d79c7266f98eaaeb12e4622e5bf93c5
Author: Benoit TELLIER <bt...@linagora.com>
AuthorDate: Tue Jun 27 07:08:52 2023 +0200

    JAMES-3918 Force deletion of user mailboxes (#1608)
---
 .../docs/modules/ROOT/pages/operate/webadmin.adoc  |  9 +++--
 .../james/webadmin/routes/UserMailboxesRoutes.java | 23 +++++++----
 .../webadmin/service/UserMailboxesService.java     | 37 ++++++++++--------
 .../webadmin/routes/UserMailboxesRoutesTest.java   | 44 ++++++++++++++++++++--
 src/site/markdown/server/manage-webadmin.md        | 11 +++---
 5 files changed, 87 insertions(+), 37 deletions(-)

diff --git a/server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc b/server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc
index eaebfa653d..e2b60a493f 100644
--- a/server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc
+++ b/server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc
@@ -1221,7 +1221,7 @@ Response codes:
 
 * 204: The mailbox now exists on the server
 * 400: Invalid mailbox name
-* 404: The user name does not exist
+* 404: The user name does not exist. Note that this check can be bypassed by specifying the `force` query parameter.
 
 To create nested mailboxes, for instance a work mailbox inside the INBOX
 mailbox, people should use the . separator. The sample query is:
@@ -1243,7 +1243,7 @@ Response codes:
 
 * 204: The mailbox now does not exist on the server
 * 400: Invalid mailbox name
-* 404: The user name does not exist
+* 404: The user name does not exist. Note that this check can be bypassed by specifying the `force` query parameter.
 
 === Testing existence of a mailbox
 
@@ -1277,7 +1277,8 @@ Resource name `usernameToBeUsed` should be an existing user
 Response codes:
 
 * 200: The mailboxes list was successfully retrieved
-* 404: The user name does not exist
+* 404: The user name does not exist, the mailbox does not exist. Note that this check can be bypassed by specifying the `force` query parameter.
+
 
 === Deleting user mailboxes
 
@@ -1290,7 +1291,7 @@ Resource name `usernameToBeUsed` should be an existing user
 Response codes:
 
 * 204: The user do not have mailboxes anymore
-* 404: The user name does not exist
+* 404: The user name does not exist. Note that this check can be bypassed by specifying the `force` query parameter.
 
 === Exporting user mailboxes
 
diff --git a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java
index 365c8e80d0..30c813fe1a 100644
--- a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java
+++ b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/routes/UserMailboxesRoutes.java
@@ -73,6 +73,13 @@ public class UserMailboxesRoutes implements Routes {
         return Username.of(request.params(USER_NAME));
     }
 
+    private static UserMailboxesService.Options getOptions(Request request) {
+        if (request.queryParams().contains("force")) {
+            return UserMailboxesService.Options.Force;
+        }
+        return UserMailboxesService.Options.Check;
+    }
+
     public static final String MAILBOX_NAME = ":mailboxName";
     public static final String MAILBOXES = "mailboxes";
     private static final String USER_NAME = ":userName";
@@ -134,7 +141,7 @@ public class UserMailboxesRoutes implements Routes {
         service.get(USER_MAILBOXES_BASE, (request, response) -> {
             response.status(HttpStatus.OK_200);
             try {
-                return userMailboxesService.listMailboxes(getUsernameParam(request));
+                return userMailboxesService.listMailboxes(getUsernameParam(request), getOptions(request));
             } catch (IllegalStateException e) {
                 LOGGER.info("Invalid get on user mailboxes", e);
                 throw ErrorResponder.builder()
@@ -157,7 +164,7 @@ public class UserMailboxesRoutes implements Routes {
     public void defineDeleteUserMailbox() {
         service.delete(SPECIFIC_MAILBOX, (request, response) -> {
             try {
-                userMailboxesService.deleteMailbox(getUsernameParam(request), new MailboxName(request.params(MAILBOX_NAME)));
+                userMailboxesService.deleteMailbox(getUsernameParam(request), new MailboxName(request.params(MAILBOX_NAME)), getOptions(request));
                 return Responses.returnNoContent(response);
             } catch (IllegalStateException e) {
                 LOGGER.info("Invalid delete on user mailbox", e);
@@ -190,7 +197,7 @@ public class UserMailboxesRoutes implements Routes {
     public void defineDeleteUserMailboxes() {
         service.delete(USER_MAILBOXES_BASE, (request, response) -> {
             try {
-                userMailboxesService.deleteMailboxes(getUsernameParam(request));
+                userMailboxesService.deleteMailboxes(getUsernameParam(request), getOptions(request));
                 return Responses.returnNoContent(response);
             } catch (IllegalStateException e) {
                 LOGGER.info("Invalid delete on user mailboxes", e);
@@ -207,7 +214,7 @@ public class UserMailboxesRoutes implements Routes {
     public void defineMailboxExists() {
         service.get(SPECIFIC_MAILBOX, (request, response) -> {
             try {
-                if (userMailboxesService.testMailboxExists(getUsernameParam(request), new MailboxName(request.params(MAILBOX_NAME)))) {
+                if (userMailboxesService.testMailboxExists(getUsernameParam(request), new MailboxName(request.params(MAILBOX_NAME)), getOptions(request))) {
                     return Responses.returnNoContent(response);
                 } else {
                     throw ErrorResponder.builder()
@@ -239,7 +246,7 @@ public class UserMailboxesRoutes implements Routes {
     public void defineCreateUserMailbox() {
         service.put(SPECIFIC_MAILBOX, (request, response) -> {
             try {
-                userMailboxesService.createMailbox(getUsernameParam(request), new MailboxName(request.params(MAILBOX_NAME)));
+                userMailboxesService.createMailbox(getUsernameParam(request), new MailboxName(request.params(MAILBOX_NAME)), getOptions(request));
                 return Responses.returnNoContent(response);
             } catch (IllegalStateException e) {
                 LOGGER.info("Invalid put on user mailbox", e);
@@ -264,7 +271,7 @@ public class UserMailboxesRoutes implements Routes {
     public void messageCount() {
         service.get(MESSAGE_COUNT_PATH, (request, response) -> {
             try {
-                return userMailboxesService.messageCount(getUsernameParam(request), new MailboxName(request.params(MAILBOX_NAME)));
+                return userMailboxesService.messageCount(getUsernameParam(request), new MailboxName(request.params(MAILBOX_NAME)), getOptions(request));
             } catch (IllegalStateException | MailboxNotFoundException e) {
                 LOGGER.info("Invalid get on user mailbox", e);
                 throw ErrorResponder.builder()
@@ -288,7 +295,7 @@ public class UserMailboxesRoutes implements Routes {
     public void unseenMessageCount() {
         service.get(UNSEEN_MESSAGE_COUNT_PATH, (request, response) -> {
             try {
-                return userMailboxesService.unseenMessageCount(getUsernameParam(request), new MailboxName(request.params(MAILBOX_NAME)));
+                return userMailboxesService.unseenMessageCount(getUsernameParam(request), new MailboxName(request.params(MAILBOX_NAME)), getOptions(request));
             } catch (IllegalStateException | MailboxNotFoundException e) {
                 LOGGER.info("Invalid get on user mailbox", e);
                 throw ErrorResponder.builder()
@@ -313,7 +320,7 @@ public class UserMailboxesRoutes implements Routes {
         Username username = getUsernameParam(request);
         MailboxName mailboxName = new MailboxName(request.params(MAILBOX_NAME));
         try {
-            userMailboxesService.usernamePreconditions(username);
+            userMailboxesService.usernamePreconditions(username, getOptions(request));
             userMailboxesService.mailboxExistPreconditions(username, mailboxName);
         } catch (IllegalStateException e) {
             LOGGER.info("Invalid put on user mailbox", e);
diff --git a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java
index cff29bc088..39785ae34a 100644
--- a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java
+++ b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java
@@ -57,6 +57,11 @@ import reactor.core.publisher.Mono;
 import reactor.core.scheduler.Schedulers;
 
 public class UserMailboxesService {
+    public enum Options {
+        Force,
+        Check
+    }
+
     private static final Logger LOGGER = LoggerFactory.getLogger(UserMailboxesService.class);
 
     private final MailboxManager mailboxManager;
@@ -68,8 +73,8 @@ public class UserMailboxesService {
         this.usersRepository = usersRepository;
     }
 
-    public void createMailbox(Username username, MailboxName mailboxName) throws MailboxException, UsersRepositoryException {
-        usernamePreconditions(username);
+    public void createMailbox(Username username, MailboxName mailboxName, Options options) throws MailboxException, UsersRepositoryException {
+        usernamePreconditions(username, options);
         MailboxSession mailboxSession = mailboxManager.createSystemSession(username);
         try {
             MailboxPath mailboxPath = MailboxPath.forUser(username, mailboxName.asString())
@@ -80,24 +85,24 @@ public class UserMailboxesService {
         }
     }
 
-    public void deleteMailboxes(Username username) throws UsersRepositoryException {
-        usernamePreconditions(username);
+    public void deleteMailboxes(Username username, Options options) throws UsersRepositoryException {
+        usernamePreconditions(username, options);
         MailboxSession mailboxSession = mailboxManager.createSystemSession(username);
         listUserMailboxes(mailboxSession)
             .map(MailboxMetaData::getPath)
             .forEach(Throwing.consumer(mailboxPath -> deleteMailbox(mailboxSession, mailboxPath)));
     }
 
-    public List<MailboxResponse> listMailboxes(Username username) throws UsersRepositoryException {
-        usernamePreconditions(username);
+    public List<MailboxResponse> listMailboxes(Username username, Options options) throws UsersRepositoryException {
+        usernamePreconditions(username, options);
         MailboxSession mailboxSession = mailboxManager.createSystemSession(username);
         return listUserMailboxes(mailboxSession)
             .map(mailboxMetaData -> new MailboxResponse(mailboxMetaData.getPath().getName(), mailboxMetaData.getId()))
             .collect(ImmutableList.toImmutableList());
     }
 
-    public boolean testMailboxExists(Username username, MailboxName mailboxName) throws MailboxException, UsersRepositoryException {
-        usernamePreconditions(username);
+    public boolean testMailboxExists(Username username, MailboxName mailboxName, Options options) throws MailboxException, UsersRepositoryException {
+        usernamePreconditions(username, options);
         MailboxSession mailboxSession = mailboxManager.createSystemSession(username);
         MailboxPath mailboxPath = MailboxPath.forUser(username, mailboxName.asString())
             .assertAcceptable(mailboxSession.getPathDelimiter());
@@ -136,8 +141,8 @@ public class UserMailboxesService {
             .subscribeOn(Schedulers.elastic());
     }
 
-    public void deleteMailbox(Username username, MailboxName mailboxName) throws MailboxException, UsersRepositoryException, MailboxHaveChildrenException {
-        usernamePreconditions(username);
+    public void deleteMailbox(Username username, MailboxName mailboxName, Options options) throws MailboxException, UsersRepositoryException, MailboxHaveChildrenException {
+        usernamePreconditions(username, options);
         MailboxSession mailboxSession = mailboxManager.createSystemSession(username);
         MailboxPath mailboxPath = MailboxPath.forUser(username, mailboxName.asString())
             .assertAcceptable(mailboxSession.getPathDelimiter());
@@ -145,14 +150,14 @@ public class UserMailboxesService {
             .forEach(Throwing.consumer(path -> deleteMailbox(mailboxSession, path)));
     }
 
-    public long messageCount(Username username, MailboxName mailboxName) throws UsersRepositoryException, MailboxException {
-        usernamePreconditions(username);
+    public long messageCount(Username username, MailboxName mailboxName, Options options) throws UsersRepositoryException, MailboxException {
+        usernamePreconditions(username, options);
         MailboxSession mailboxSession = mailboxManager.createSystemSession(username);
         return mailboxManager.getMailbox(MailboxPath.forUser(username, mailboxName.asString()), mailboxSession).getMessageCount(mailboxSession);
     }
 
-    public long unseenMessageCount(Username username, MailboxName mailboxName) throws UsersRepositoryException, MailboxException {
-        usernamePreconditions(username);
+    public long unseenMessageCount(Username username, MailboxName mailboxName, Options options) throws UsersRepositoryException, MailboxException {
+        usernamePreconditions(username, options);
         MailboxSession mailboxSession = mailboxManager.createSystemSession(username);
         return mailboxManager.getMailbox(MailboxPath.forUser(username, mailboxName.asString()), mailboxSession)
             .getMailboxCounters(mailboxSession)
@@ -173,8 +178,8 @@ public class UserMailboxesService {
         }
     }
 
-    public void usernamePreconditions(Username username) throws UsersRepositoryException {
-        Preconditions.checkState(usersRepository.contains(username), "User does not exist");
+    public void usernamePreconditions(Username username, Options options) throws UsersRepositoryException {
+        Preconditions.checkState(options == Options.Force || usersRepository.contains(username), "User does not exist");
     }
 
     public void mailboxExistPreconditions(Username username, MailboxName mailboxName) throws MailboxException {
diff --git a/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java b/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
index 861cb7bd54..93c301d4bb 100644
--- a/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
+++ b/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
@@ -267,7 +267,7 @@ class UserMailboxesRoutesTest {
         }
 
         @Test
-        void putShouldThrowWhenMailboxNameWithDots() throws Exception {
+        void putShouldThrowWhenMailboxNameWithDots() {
             Map<String, Object> errors = when()
                 .put(MAILBOX_NAME_WITH_DOTS)
             .then()
@@ -349,7 +349,7 @@ class UserMailboxesRoutesTest {
             when(usersRepository.contains(USERNAME)).thenReturn(false);
 
             Map<String, Object> errors = when()
-                .put(MAILBOX_NAME)
+                .delete(MAILBOX_NAME)
             .then()
                 .statusCode(NOT_FOUND_404)
                 .contentType(JSON)
@@ -361,12 +361,36 @@ class UserMailboxesRoutesTest {
             assertThat(errors)
                 .containsEntry("statusCode", NOT_FOUND_404)
                 .containsEntry("type", ERROR_TYPE_NOTFOUND)
-                .containsEntry("message", "Invalid get on user mailboxes")
+                .containsEntry("message", "Invalid delete on user mailboxes")
                 .containsEntry("details", "User does not exist");
         }
 
         @Test
-        void getShouldReturnUserErrorWithInvalidWildcardMailboxName() throws Exception {
+        void putShouldReturn204WhenForceNonExistingUser() throws Exception {
+            when(usersRepository.contains(USERNAME)).thenReturn(false);
+
+            given()
+                .queryParam("force")
+            .when()
+                .put(MAILBOX_NAME)
+            .then()
+                .statusCode(NO_CONTENT_204);
+        }
+
+        @Test
+        void deleteShouldReturn204WhenForceNonExistingUser() throws Exception {
+            when(usersRepository.contains(USERNAME)).thenReturn(false);
+
+            given()
+                .queryParam("force")
+            .when()
+                .delete(MAILBOX_NAME)
+            .then()
+                .statusCode(NO_CONTENT_204);
+        }
+
+        @Test
+        void getShouldReturnUserErrorWithInvalidWildcardMailboxName() {
             Map<String, Object> errors = when()
                 .get(MAILBOX_NAME + "*")
             .then()
@@ -602,6 +626,18 @@ class UserMailboxesRoutesTest {
                 .containsEntry("message", "Invalid delete on user mailboxes");
         }
 
+        @Test
+        void deleteMailboxesShouldReturn204UserErrorWithNonExistingUser() throws Exception {
+            when(usersRepository.contains(USERNAME)).thenReturn(false);
+
+            given()
+                .queryParam("force")
+            .when()
+                .delete()
+            .then()
+                .statusCode(NO_CONTENT_204);
+        }
+
         @Test
         void getMailboxesShouldReturnEmptyListByDefault() {
             List<Object> list =
diff --git a/src/site/markdown/server/manage-webadmin.md b/src/site/markdown/server/manage-webadmin.md
index c7b2496d09..ca024dfc1c 100644
--- a/src/site/markdown/server/manage-webadmin.md
+++ b/src/site/markdown/server/manage-webadmin.md
@@ -995,7 +995,8 @@ by an admin to ensure Cassandra message consistency.
  - [Recomputing User JMAP fast message view projection](#Recomputing_User_JMAP_fast_message_view_projection)
  - [Counting emails](#Counting_emails)
  - [Counting unseen emails](#Couting_unseen_emails)
- - [Clearing mailbox content][#Clearing_mailbox_content]   
+ - [Clearing mailbox content][#Clearing_mailbox_content]
+
 ### Creating a mailbox
 
 ```
@@ -1009,7 +1010,7 @@ Response codes:
 
  - 204: The mailbox now exists on the server
  - 400: Invalid mailbox name
- - 404: The user name does not exist
+ - 404: The user name does not exist. Note that this check can be bypassed by specifying the `force` query parameter.
 
  To create nested mailboxes, for instance a work mailbox inside the INBOX mailbox, people should use the . separator. The sample query is:
 
@@ -1030,7 +1031,7 @@ Response codes:
 
  - 204: The mailbox now does not exist on the server
  - 400: Invalid mailbox name
- - 404: The user name does not exist
+ - 404: The user name does not exist. Note that this check can be bypassed by specifying the `force` query parameter.
 
 ### Testing existence of a mailbox
 
@@ -1064,7 +1065,7 @@ Resource name `usernameToBeUsed` should be an existing user
 Response codes:
 
  - 200: The mailboxes list was successfully retrieved
- - 404: The user name does not exist
+ - 404: The user name does not exist. Note that this check can be bypassed by specifying the `force` query parameter.
 
 ### Deleting user mailboxes
 
@@ -1077,7 +1078,7 @@ Resource name `usernameToBeUsed` should be an existing user
 Response codes:
 
  - 204: The user do not have mailboxes anymore
- - 404: The user name does not exist
+ - 404: The user name does not exist. Note that this check can be bypassed by specifying the `force` query parameter.
 
 ### Exporting user mailboxes
 


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