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 2020/08/19 07:38:29 UTC

[james-project] branch master updated (678397f -> 167c6d5)

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

btellier pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git.


    from 678397f  Merge pull request #201 from dleangen/docs/concepts/storage
     new a0534da  JAMES-3355 Mailbox/set destroy implementation
     new 8b360fa  JAMES-3356 Allow the use of MailboxCreationId as arguments of destroy
     new 6e15127  JAMES-3356 Refactoring: unify error handling for mailbox creation parsing/handling
     new 5aaedd1  JAMES-3356 Allow the use of Mailbox CreationId for parentId
     new 2a78c15  JAMES-3356 Refactor: simplify createResponse
     new 6e8161f  JAMES-3356 Allow the use of creationIds in Mailbox/Get
     new 8852e1d  JAMES-3356 Fix cosmetic issues in MailboxSetMethodContract
     new 2afc8e9  JAMES-3357 Mailbox/set create should handle isSubscribed
     new 4d0a14f  JAMES-3357 Mailbox/set destroy should handle isSubscribed
     new 8bd2880  JAMES-3357 MailboxSetMethodContract: add missing spaces
     new 167c6d5  JAMES-3357 MailboxSetMethodContract: respect indent conventions

The 11 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:
 .../contract/MailboxGetMethodContract.scala        |   82 ++
 .../contract/MailboxSetMethodContract.scala        | 1128 +++++++++++++++++++-
 .../org/apache/james/jmap/json/Serializer.scala    |   15 +-
 .../org/apache/james/jmap/mail/MailboxGet.scala    |    6 +-
 .../org/apache/james/jmap/mail/MailboxSet.scala    |   22 +-
 .../apache/james/jmap/method/CoreEchoMethod.scala  |    3 +-
 .../james/jmap/method/MailboxGetMethod.scala       |   19 +-
 .../james/jmap/method/MailboxSetMethod.scala       |  238 +++--
 .../org/apache/james/jmap/method/Method.scala      |    3 +-
 .../scala/org/apache/james/jmap/model/Id.scala     |   11 +-
 .../org/apache/james/jmap/model/Invocation.scala   |    1 -
 .../apache/james/jmap/model/RequestObject.scala    |   11 +-
 .../apache/james/jmap/routes/JMAPApiRoutes.scala   |   10 +-
 .../james/jmap/routes/ProcessingContext.scala      |   52 +
 .../jmap/json/MailboxGetSerializationTest.scala    |   17 +-
 .../james/jmap/method/CoreEchoMethodTest.scala     |    5 +-
 .../james/jmap/routes/JMAPApiRoutesTest.scala      |    2 +-
 17 files changed, 1508 insertions(+), 117 deletions(-)
 create mode 100644 server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/ProcessingContext.scala


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


[james-project] 04/11: JAMES-3356 Allow the use of Mailbox CreationId for parentId

Posted by bt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 5aaedd129fd3c358701cd4d494b3f910ace033ad
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Fri Aug 14 10:51:13 2020 +0700

    JAMES-3356 Allow the use of Mailbox CreationId for parentId
---
 .../contract/MailboxSetMethodContract.scala        | 120 ++++++++++++++++++++-
 .../org/apache/james/jmap/mail/MailboxSet.scala    |   2 +-
 .../james/jmap/method/MailboxSetMethod.scala       |  35 +++---
 3 files changed, 139 insertions(+), 18 deletions(-)

diff --git a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
index becc612..28bd5fd 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
+++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
@@ -219,7 +219,7 @@ trait MailboxSetMethodContract {
          |      "notCreated": {
          |        "C42": {
          |          "type": "invalidArguments",
-         |          "description": "'/parentId' property in mailbox object is not valid"
+         |          "description": "'/parentId' property in mailbox object is not valid: Predicate isEmpty() did not fail."
          |        }
          |      }
          |    },
@@ -1195,6 +1195,124 @@ trait MailboxSetMethodContract {
   }
 
   @Test
+  def createParentIdShouldAcceptCreationIdsWithinTheSameRequest(server: GuiceJamesServer): Unit = {
+    val request =
+      s"""
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |       ["Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "create": {
+        |                    "C42": {
+        |                      "name": "parent"
+        |                    }
+        |                }
+        |           },
+        |    "c1"],
+        |       ["Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "create": {
+        |                    "C43": {
+        |                      "name": "child",
+        |                      "parentId": "#C42"
+        |                    }
+        |                }
+        |           },
+        |    "c2"]
+        |   ]
+        |}
+        |""".stripMargin
+
+    val response = `given`
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .when
+      .post
+      .`then`
+      .log().ifValidationFails()
+      .statusCode(SC_OK)
+      .contentType(JSON)
+      .extract
+      .body
+      .asString
+
+    val parentId: String = server.getProbe(classOf[MailboxProbeImpl])
+      .getMailboxId("#private", BOB.asString(), "parent")
+      .serialize()
+    val childId: String = server.getProbe(classOf[MailboxProbeImpl])
+      .getMailboxId("#private", BOB.asString(), "parent.child")
+      .serialize()
+
+    assertThatJson(response).isEqualTo(
+      s"""{
+         |    "sessionState": "75128aab4b1b",
+         |    "methodResponses": [
+         |        [
+         |            "Mailbox/set",
+         |            {
+         |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |                "newState": "000001",
+         |                "created": {
+         |                    "C42": {
+         |                        "id": "$parentId",
+         |                        "totalEmails": 0,
+         |                        "unreadEmails": 0,
+         |                        "totalThreads": 0,
+         |                        "unreadThreads": 0,
+         |                        "myRights": {
+         |                            "mayReadItems": true,
+         |                            "mayAddItems": true,
+         |                            "mayRemoveItems": true,
+         |                            "maySetSeen": true,
+         |                            "maySetKeywords": true,
+         |                            "mayCreateChild": true,
+         |                            "mayRename": true,
+         |                            "mayDelete": true,
+         |                            "maySubmit": true
+         |                        },
+         |                        "isSubscribed": true
+         |                    }
+         |                }
+         |            },
+         |            "c1"
+         |        ],
+         |        [
+         |            "Mailbox/set",
+         |            {
+         |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |                "newState": "000001",
+         |                "created": {
+         |                    "C43": {
+         |                        "id": "$childId",
+         |                        "totalEmails": 0,
+         |                        "unreadEmails": 0,
+         |                        "totalThreads": 0,
+         |                        "unreadThreads": 0,
+         |                        "myRights": {
+         |                            "mayReadItems": true,
+         |                            "mayAddItems": true,
+         |                            "mayRemoveItems": true,
+         |                            "maySetSeen": true,
+         |                            "maySetKeywords": true,
+         |                            "mayCreateChild": true,
+         |                            "mayRename": true,
+         |                            "mayDelete": true,
+         |                            "maySubmit": true
+         |                        },
+         |                        "isSubscribed": true
+         |                    }
+         |                }
+         |            },
+         |            "c2"
+         |        ]
+         |    ]
+         |}""".stripMargin)
+  }
+
+  @Test
   def creationIdReferencesShouldFailWhenWrongOrder(server: GuiceJamesServer): Unit = {
     val request =
       s"""
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
index 2b2fc13..fdf2d18 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
@@ -43,7 +43,7 @@ object MailboxSetRequest {
 }
 
 case class RemoveEmailsOnDestroy(value: Boolean) extends AnyVal
-case class MailboxCreationRequest(name: MailboxName, parentId: Option[MailboxId])
+case class MailboxCreationRequest(name: MailboxName, parentId: Option[UnparsedMailboxId])
 
 case class MailboxPatchObject(value: Map[String, JsObject])
 
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
index 94b64b2..61f8452 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
@@ -114,19 +114,6 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
         }))
   }
 
-  def parseCreate(jsObject: JsObject): Either[MailboxCreationParseException, MailboxCreationRequest] =
-    Json.fromJson(jsObject)(serializer.mailboxCreationRequest) match {
-      case JsSuccess(creationRequest, _) => Right(creationRequest)
-      case JsError(errors) => Left(MailboxCreationParseException(mailboxSetError(errors)))
-    }
-
-  private def mailboxSetError(errors: collection.Seq[(JsPath, collection.Seq[JsonValidationError])]): MailboxSetError =
-    errors.head match {
-      case (path, Seq()) => MailboxSetError.invalidArgument(Some(SetErrorDescription(s"'$path' property in mailbox object is not valid")), None)
-      case (path, Seq(JsonValidationError(Seq("error.path.missing")))) => MailboxSetError("invalidArguments", Some(SetErrorDescription(s"Missing '$path' property in mailbox object")), None)
-      case (path, _) => MailboxSetError.invalidArgument(Some(SetErrorDescription(s"Unknown error on property '$path'")), None)
-    }
-
   private def deleteMailboxes(mailboxSession: MailboxSession, mailboxSetRequest: MailboxSetRequest, processingContext: ProcessingContext): SMono[DeletionResults] = {
     SFlux.fromIterable(mailboxSetRequest.destroy.getOrElse(Seq()))
       .flatMap(id => delete(mailboxSession, processingContext, id)
@@ -176,11 +163,25 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
                             jsObject: JsObject,
                             processingContext: ProcessingContext): CreationResult = {
     parseCreate(jsObject)
-        .flatMap(mailboxCreationRequest => resolvePath(mailboxSession, mailboxCreationRequest))
+      .flatMap(mailboxCreationRequest => resolvePath(mailboxSession, mailboxCreationRequest, processingContext))
       .map(path => createMailbox(mailboxSession, mailboxCreationId, processingContext, path))
       .fold(e => CreationFailure(mailboxCreationId, e), r => r)
   }
 
+  private def parseCreate(jsObject: JsObject): Either[MailboxCreationParseException, MailboxCreationRequest] =
+    Json.fromJson(jsObject)(serializer.mailboxCreationRequest) match {
+      case JsSuccess(creationRequest, _) => Right(creationRequest)
+      case JsError(errors) => Left(MailboxCreationParseException(mailboxSetError(errors)))
+    }
+
+  private def mailboxSetError(errors: collection.Seq[(JsPath, collection.Seq[JsonValidationError])]): MailboxSetError =
+    errors.head match {
+      case (path, Seq()) => MailboxSetError.invalidArgument(Some(SetErrorDescription(s"'$path' property in mailbox object is not valid")), None)
+      case (path, Seq(JsonValidationError(Seq("error.path.missing")))) => MailboxSetError("invalidArguments", Some(SetErrorDescription(s"Missing '$path' property in mailbox object")), None)
+      case (path, Seq(JsonValidationError(Seq(message)))) => MailboxSetError("invalidArguments", Some(SetErrorDescription(s"'$path' property in mailbox object is not valid: $message")), None)
+      case (path, _) => MailboxSetError.invalidArgument(Some(SetErrorDescription(s"Unknown error on property '$path'")), None)
+    }
+
   private def createMailbox(mailboxSession: MailboxSession,
                             mailboxCreationId: MailboxCreationId,
                             processingContext: ProcessingContext,
@@ -207,9 +208,11 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
   }
 
   private def resolvePath(mailboxSession: MailboxSession,
-                          mailboxCreationRequest: MailboxCreationRequest): Either[Exception, MailboxPath] = {
+                          mailboxCreationRequest: MailboxCreationRequest,
+                          processingContext: ProcessingContext): Either[Exception, MailboxPath] = {
     mailboxCreationRequest.parentId
-      .map(parentId => for {
+      .map(maybeParentId => for {
+        parentId <- processingContext.resolveMailboxId(maybeParentId, mailboxIdFactory)
         parentPath <- retrievePath(parentId, mailboxSession)
       } yield {
         parentPath.child(mailboxCreationRequest.name, mailboxSession.getPathDelimiter)


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


[james-project] 05/11: JAMES-3356 Refactor: simplify createResponse

Posted by bt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 2a78c157f6a1c634e46eeef8ecfe1845753ae973
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Fri Aug 14 11:07:40 2020 +0700

    JAMES-3356 Refactor: simplify createResponse
---
 .../james/jmap/method/MailboxSetMethod.scala       | 50 ++++++++++++----------
 1 file changed, 27 insertions(+), 23 deletions(-)

diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
index 61f8452..24b7556 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
@@ -56,12 +56,26 @@ case class CreationFailure(mailboxCreationId: MailboxCreationId, exception: Exce
   }
 }
 case class CreationResults(created: Seq[CreationResult]) {
-  def retrieveCreated: Map[MailboxCreationId, MailboxId] = created
+  def retrieveCreated: Map[MailboxCreationId, MailboxCreationResponse] = created
     .flatMap(result => result match {
       case success: CreationSuccess => Some(success.mailboxCreationId, success.mailboxId)
       case _ => None
     })
     .toMap
+    .map(creation => (creation._1, toCreationResponse(creation._2)))
+
+  private def toCreationResponse(mailboxId: MailboxId): MailboxCreationResponse = MailboxCreationResponse(
+    id = mailboxId,
+    role = None,
+    totalEmails = TotalEmails(0L),
+    unreadEmails = UnreadEmails(0L),
+    totalThreads = TotalThreads(0L),
+    unreadThreads = UnreadThreads(0L),
+    myRights = MailboxRights.FULL,
+    rights = None,
+    namespace = None,
+    quotas = None,
+    isSubscribed = IsSubscribed(true))
 
   def retrieveErrors: Map[MailboxCreationId, MailboxSetError] = created
     .flatMap(result => result match {
@@ -83,11 +97,11 @@ case class DeletionFailure(mailboxId: UnparsedMailboxId, exception: Throwable) e
   }
 }
 case class DeletionResults(results: Seq[DeletionResult]) {
-  def destroyed: Seq[DeletionSuccess] =
+  def destroyed: Seq[MailboxId] =
     results.flatMap(result => result match {
       case success: DeletionSuccess => Some(success)
       case _ => None
-    })
+    }).map(_.mailboxId)
 
   def retrieveErrors: Map[UnparsedMailboxId, MailboxSetError] =
     results.flatMap(result => result match {
@@ -226,34 +240,24 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
       case e: Exception => Left(e)
     }
 
-  private def createResponse(invocation: Invocation, mailboxSetRequest: MailboxSetRequest,
+  private def createResponse(invocation: Invocation,
+                             mailboxSetRequest: MailboxSetRequest,
                              creationResults: CreationResults,
                              deletionResults: DeletionResults): Invocation = {
-    val created: Map[MailboxCreationId, MailboxId] = creationResults.retrieveCreated
-
-    Invocation(methodName, Arguments(serializer.serialize(MailboxSetResponse(
+    val response = MailboxSetResponse(
       mailboxSetRequest.accountId,
       oldState = None,
       newState = State.INSTANCE,
-      destroyed = Some(deletionResults.destroyed.map(_.mailboxId)).filter(_.nonEmpty),
-      created = Some(created.map(creation => (creation._1, MailboxCreationResponse(
-        id = creation._2,
-        role = None,
-        totalEmails = TotalEmails(0L),
-        unreadEmails = UnreadEmails(0L),
-        totalThreads = TotalThreads(0L),
-        unreadThreads = UnreadThreads(0L),
-        myRights = MailboxRights.FULL,
-        rights = None,
-        namespace = None,
-        quotas = None,
-        isSubscribed = IsSubscribed(true)
-      )))).filter(_.nonEmpty),
+      destroyed = Some(deletionResults.destroyed).filter(_.nonEmpty),
+      created = Some(creationResults.retrieveCreated).filter(_.nonEmpty),
       notCreated = Some(creationResults.retrieveErrors).filter(_.nonEmpty),
       updated = None,
       notUpdated = None,
-      notDestroyed = Some(deletionResults.retrieveErrors).filter(_.nonEmpty)
-    )).as[JsObject]), invocation.methodCallId)
+      notDestroyed = Some(deletionResults.retrieveErrors).filter(_.nonEmpty))
+    
+    Invocation(methodName,
+      Arguments(serializer.serialize(response).as[JsObject]),
+      invocation.methodCallId)
   }
 
   private def asMailboxSetRequest(arguments: Arguments): SMono[MailboxSetRequest] = {


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


[james-project] 09/11: JAMES-3357 Mailbox/set destroy should handle isSubscribed

Posted by bt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 4d0a14f9540b70f57160628b282c0634674ae1b4
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Fri Aug 14 11:59:19 2020 +0700

    JAMES-3357 Mailbox/set destroy should handle isSubscribed
---
 .../contract/MailboxSetMethodContract.scala        | 45 ++++++++++++++++++++++
 .../james/jmap/method/MailboxSetMethod.scala       |  7 ++--
 2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
index 073b482..bf6c8aa 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
+++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
@@ -495,6 +495,51 @@ trait MailboxSetMethodContract {
   }
 
   @Test
+  def destroyShouldUnsubscribeMailboxes(server: GuiceJamesServer): Unit = {
+    val request=
+      """
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |       ["Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "create": {
+        |                    "C42": {
+        |                      "name": "myMailbox"
+        |                    }
+        |                }
+        |           },
+        |    "c1"],
+        |       [
+        |           "Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "destroy": ["#C42"]
+        |           },
+        |    "c2"]
+        |   ]
+        |}
+        |""".stripMargin
+
+      `given`
+        .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+        .body(request)
+      .when
+        .post
+      .`then`
+        .log().ifValidationFails()
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+    Assertions.assertThat(server.getProbe(classOf[MailboxProbeImpl])
+      .listSubscriptions(BOB.asString())).doesNotContain("myMailbox")
+  }
+
+  @Test
   def mailboxSetShouldReturnCreatedWhenOnlyName(server: GuiceJamesServer): Unit = {
     val request =
       """
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
index 9aa20b4..3c7470c 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
@@ -29,7 +29,7 @@ import org.apache.james.jmap.model.Invocation.{Arguments, MethodName}
 import org.apache.james.jmap.model.{ClientId, Id, Invocation, ServerId, State}
 import org.apache.james.jmap.routes.ProcessingContext
 import org.apache.james.mailbox.exception.{InsufficientRightsException, MailboxExistsException, MailboxNameException, MailboxNotFoundException}
-import org.apache.james.mailbox.model.{FetchGroup, Mailbox, MailboxId, MailboxPath, MessageRange}
+import org.apache.james.mailbox.model.{FetchGroup, MailboxId, MailboxPath, MessageRange}
 import org.apache.james.mailbox.{MailboxManager, MailboxSession, SubscriptionManager}
 import org.apache.james.metrics.api.MetricFactory
 import org.reactivestreams.Publisher
@@ -146,7 +146,7 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
     }
   }
 
-  private def delete(mailboxSession: MailboxSession, id: MailboxId): Mailbox = {
+  private def delete(mailboxSession: MailboxSession, id: MailboxId): Unit = {
     val mailbox = mailboxManager.getMailbox(id, mailboxSession)
     if (mailbox.getMessages(MessageRange.all(), FetchGroup.MINIMAL, mailboxSession).hasNext) {
       throw MailboxHasMailException(id)
@@ -154,7 +154,8 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
     if (mailboxManager.hasChildren(mailbox.getMailboxPath, mailboxSession)) {
       throw MailboxHasChildException(id)
     }
-    mailboxManager.deleteMailbox(id, mailboxSession)
+    val deletedMailbox = mailboxManager.deleteMailbox(id, mailboxSession)
+    subscriptionManager.unsubscribe(mailboxSession, deletedMailbox.getName)
   }
 
   private def createMailboxes(mailboxSession: MailboxSession,


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


[james-project] 02/11: JAMES-3356 Allow the use of MailboxCreationId as arguments of destroy

Posted by bt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 8b360fa6a6b63c245327c45ce52f9dd426f135d8
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Thu Aug 13 17:47:02 2020 +0700

    JAMES-3356 Allow the use of MailboxCreationId as arguments of destroy
    
    JMAP mailbox ids prefixed by # references previously created records within the same request. The server needs
    to resolve that mailboxId, and replace the given creationId with the mailboxId it did allocate.
    
    In order to do so, we need to record as part of the processing the ids allocated to the creationId.
---
 .../contract/MailboxSetMethodContract.scala        | 229 ++++++++++++++++++++-
 .../org/apache/james/jmap/json/Serializer.scala    |   7 +
 .../apache/james/jmap/method/CoreEchoMethod.scala  |   3 +-
 .../james/jmap/method/MailboxGetMethod.scala       |   3 +-
 .../james/jmap/method/MailboxSetMethod.scala       |  91 +++++---
 .../org/apache/james/jmap/method/Method.scala      |   3 +-
 .../scala/org/apache/james/jmap/model/Id.scala     |  11 +-
 .../apache/james/jmap/model/RequestObject.scala    |  11 +-
 .../apache/james/jmap/routes/JMAPApiRoutes.scala   |   9 +-
 .../james/jmap/routes/ProcessingContext.scala      |  52 +++++
 .../james/jmap/method/CoreEchoMethodTest.scala     |   5 +-
 .../james/jmap/routes/JMAPApiRoutesTest.scala      |   2 +-
 12 files changed, 387 insertions(+), 39 deletions(-)

diff --git a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
index 8a67f7a..becc612 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
+++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
@@ -38,6 +38,7 @@ import org.apache.james.mime4j.dom.Message
 import org.apache.james.modules.{ACLProbeImpl, MailboxProbeImpl}
 import org.apache.james.utils.DataProbeImpl
 import org.assertj.core.api.Assertions
+import org.hamcrest.Matchers.{equalTo, hasSize}
 import org.junit.jupiter.api.{BeforeEach, Disabled, Test}
 
 trait MailboxSetMethodContract {
@@ -1132,6 +1133,7 @@ trait MailboxSetMethodContract {
         .body
         .asString
 
+    val message: String = "invalid is not a mailboxId: For input string: \\\"invalid\\\""
     assertThatJson(response).isEqualTo(
       s"""{
          |  "sessionState": "75128aab4b1b",
@@ -1143,11 +1145,236 @@ trait MailboxSetMethodContract {
          |      "notDestroyed": {
          |        "invalid": {
          |          "type": "invalidArguments",
-         |          "description": "invalid is not a mailboxId"
+         |          "description": "$message"
          |        }
          |      }
          |    },
          |    "c1"]]
          |}""".stripMargin)
   }
+
+  @Test
+  def deleteShouldAcceptCreationIdsWithinTheSameRequest(): Unit = {
+    val request =
+      s"""
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |       ["Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "create": {
+        |                    "C42": {
+        |                      "name": "myMailbox"
+        |                    }
+        |                }
+        |           },
+        |    "c1"],
+        |       ["Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "destroy": ["#C42"]
+        |           },
+        |    "c2"]
+        |   ]
+        |}
+        |""".stripMargin
+
+     `given`
+        .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+        .body(request)
+      .when
+        .post
+      .`then`
+        .log().ifValidationFails()
+        .statusCode(SC_OK)
+        .contentType(JSON)
+       // We need to limit ourself to simple body assertions in order not to infer id allocation
+       .body("methodResponses[0][1].created.C42.totalThreads", equalTo(0))
+       .body("methodResponses[1][1].destroyed", hasSize(1))
+  }
+
+  @Test
+  def creationIdReferencesShouldFailWhenWrongOrder(server: GuiceJamesServer): Unit = {
+    val request =
+      s"""
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |      ["Mailbox/set",
+        |          {
+        |               "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |               "destroy": ["#C42"]
+        |          },
+        |   "c2"],
+        |       ["Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "create": {
+        |                    "C42": {
+        |                      "name": "myMailbox"
+        |                    }
+        |                }
+        |           },
+        |    "c1"]
+        |   ]
+        |}
+        |""".stripMargin
+
+    val response = `given`
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+    .when
+      .post
+    .`then`
+      .log().ifValidationFails()
+      .statusCode(SC_OK)
+      .contentType(JSON)
+      .extract
+      .body
+      .asString
+
+    val mailboxId: String = server.getProbe(classOf[MailboxProbeImpl])
+      .getMailboxId("#private", BOB.asString(), "myMailbox")
+      .serialize()
+
+    assertThatJson(response).isEqualTo(
+      s"""{
+         |	"sessionState": "75128aab4b1b",
+         |	"methodResponses": [
+         |		["Mailbox/set", {
+         |			"accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |			"newState": "000001",
+         |			"notDestroyed": {
+         |				"#C42": {
+         |					"type": "invalidArguments",
+         |					"description": "#C42 is not a mailboxId: ClientId(#C42) was not used in previously defined creationIds"
+         |				}
+         |			}
+         |		}, "c2"],
+         |		["Mailbox/set", {
+         |			"accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |			"newState": "000001",
+         |			"created": {
+         |				"C42": {
+         |					"id": "$mailboxId",
+         |					"totalEmails": 0,
+         |					"unreadEmails": 0,
+         |					"totalThreads": 0,
+         |					"unreadThreads": 0,
+         |					"myRights": {
+         |						"mayReadItems": true,
+         |						"mayAddItems": true,
+         |						"mayRemoveItems": true,
+         |						"maySetSeen": true,
+         |						"maySetKeywords": true,
+         |						"mayCreateChild": true,
+         |						"mayRename": true,
+         |						"mayDelete": true,
+         |						"maySubmit": true
+         |					},
+         |					"isSubscribed": true
+         |				}
+         |			}
+         |		}, "c1"]
+         |	]
+         |}""".stripMargin)
+  }
+
+  @Test
+  def creationIdReferencesShouldFailWhenNone(server: GuiceJamesServer): Unit = {
+    val request =
+      s"""
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |      ["Mailbox/set",
+        |          {
+        |               "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |               "destroy": ["#C42"]
+        |          },
+        |   "c2"]
+        |   ]
+        |}
+        |""".stripMargin
+
+    val response = `given`
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+    .when
+      .post
+    .`then`
+      .log().ifValidationFails()
+      .statusCode(SC_OK)
+      .contentType(JSON)
+      .extract
+      .body
+      .asString
+
+    assertThatJson(response).isEqualTo(
+      s"""{
+         |	"sessionState": "75128aab4b1b",
+         |	"methodResponses": [
+         |		["Mailbox/set", {
+         |			"accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |			"newState": "000001",
+         |			"notDestroyed": {
+         |				"#C42": {
+         |					"type": "invalidArguments",
+         |					"description": "#C42 is not a mailboxId: ClientId(#C42) was not used in previously defined creationIds"
+         |				}
+         |			}
+         |		}, "c2"]
+         |	]
+         |}""".stripMargin)
+  }
+
+  @Test
+  def emptyCreationIdReferencesShouldFail(): Unit = {
+    val request =
+      s"""
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |      ["Mailbox/set",
+        |          {
+        |               "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |               "destroy": ["#"]
+        |          },
+        |   "c2"]
+        |   ]
+        |}
+        |""".stripMargin
+
+    val response = `given`
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+    .when
+      .post
+    .`then`
+      .log().ifValidationFails()
+      .statusCode(SC_OK)
+      .contentType(JSON)
+      .extract
+      .body
+      .asString
+
+    val message = "# is not a mailboxId: Left predicate of ((!(0 < 1) && !(0 > 255)) && \\\"\\\".matches(\\\"^[#a-zA-Z0-9-_]*$\\\")) failed: Predicate taking size() = 0 failed: Left predicate of (!(0 < 1) && !(0 > 255)) failed: Predicate (0 < 1) did not fail."
+    assertThatJson(response).isEqualTo(
+      s"""{
+         |	"sessionState": "75128aab4b1b",
+         |	"methodResponses": [
+         |		["Mailbox/set", {
+         |			"accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |			"newState": "000001",
+         |			"notDestroyed": {
+         |				"#": {
+         |					"type": "invalidArguments",
+         |					"description": "$message"
+         |				}
+         |			}
+         |		}, "c2"]
+         |	]
+         |}""".stripMargin)
+  }
 }
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/Serializer.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/Serializer.scala
index f15f1ce..5ba43fb 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/Serializer.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/Serializer.scala
@@ -284,6 +284,13 @@ class Serializer @Inject() (mailboxIdFactory: MailboxId.Factory) {
         jsObject.+(mailboxId.serialize(), mailboxSetErrorWrites.writes(mailboxSetError))
       })
     }
+  private implicit def mailboxMapSetErrorWritesByClientId: Writes[Map[ClientId, MailboxSetError]] =
+    (m: Map[ClientId, MailboxSetError]) => {
+      m.foldLeft(JsObject.empty)((jsObject, kv) => {
+        val (clientId: ClientId, mailboxSetError: MailboxSetError) = kv
+        jsObject.+(clientId.value, mailboxSetErrorWrites.writes(mailboxSetError))
+      })
+    }
 
   private implicit def mailboxMapCreationResponseWrites: Writes[Map[MailboxCreationId, MailboxCreationResponse]] =
     (m: Map[MailboxCreationId, MailboxCreationResponse]) => {
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/CoreEchoMethod.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/CoreEchoMethod.scala
index d1dd286..9e9c432 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/CoreEchoMethod.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/CoreEchoMethod.scala
@@ -23,6 +23,7 @@ import eu.timepit.refined.auto._
 import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier
 import org.apache.james.jmap.model.Invocation
 import org.apache.james.jmap.model.Invocation.MethodName
+import org.apache.james.jmap.routes.ProcessingContext
 import org.apache.james.mailbox.MailboxSession
 import org.reactivestreams.Publisher
 import reactor.core.scala.publisher.SMono
@@ -30,5 +31,5 @@ import reactor.core.scala.publisher.SMono
 class CoreEchoMethod extends Method {
   override val methodName = MethodName("Core/echo")
 
-  override def process(capabilities: Set[CapabilityIdentifier], invocation: Invocation, mailboxSession: MailboxSession): Publisher[Invocation] = SMono.just(invocation)
+  override def process(capabilities: Set[CapabilityIdentifier], invocation: Invocation, mailboxSession: MailboxSession, processingContext: ProcessingContext): Publisher[Invocation] = SMono.just(invocation)
 }
\ No newline at end of file
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxGetMethod.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxGetMethod.scala
index ed582f4..7e66b5e 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxGetMethod.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxGetMethod.scala
@@ -27,6 +27,7 @@ import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier
 import org.apache.james.jmap.model.Invocation.{Arguments, MethodName}
 import org.apache.james.jmap.model.State.INSTANCE
 import org.apache.james.jmap.model.{CapabilityIdentifier, ErrorCode, Invocation, MailboxFactory}
+import org.apache.james.jmap.routes.ProcessingContext
 import org.apache.james.jmap.utils.quotas.{QuotaLoader, QuotaLoaderWithPreloadedDefaultFactory}
 import org.apache.james.mailbox.exception.MailboxNotFoundException
 import org.apache.james.mailbox.model.search.MailboxQuery
@@ -54,7 +55,7 @@ class MailboxGetMethod @Inject() (serializer: Serializer,
     def merge(other: MailboxGetResults): MailboxGetResults = MailboxGetResults(this.mailboxes ++ other.mailboxes, this.notFound.merge(other.notFound))
   }
 
-  override def process(capabilities: Set[CapabilityIdentifier], invocation: Invocation, mailboxSession: MailboxSession): Publisher[Invocation] = {
+  override def process(capabilities: Set[CapabilityIdentifier], invocation: Invocation, mailboxSession: MailboxSession, processingContext: ProcessingContext): Publisher[Invocation] = {
     metricFactory.decoratePublisherWithTimerMetricLogP99(JMAP_RFC8621_PREFIX + methodName.value,
       asMailboxGetRequest(invocation.arguments)
         .flatMap(mailboxGetRequest => {
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
index 44a7a76..ae3bec0 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
@@ -26,7 +26,8 @@ import org.apache.james.jmap.mail.MailboxSetRequest.{MailboxCreationId, Unparsed
 import org.apache.james.jmap.mail.{IsSubscribed, MailboxCreationRequest, MailboxCreationResponse, MailboxRights, MailboxSetError, MailboxSetRequest, MailboxSetResponse, Properties, SetErrorDescription, TotalEmails, TotalThreads, UnreadEmails, UnreadThreads}
 import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier
 import org.apache.james.jmap.model.Invocation.{Arguments, MethodName}
-import org.apache.james.jmap.model.{Invocation, State}
+import org.apache.james.jmap.model.{ClientId, Id, Invocation, ServerId, State}
+import org.apache.james.jmap.routes.ProcessingContext
 import org.apache.james.mailbox.exception.{InsufficientRightsException, MailboxExistsException, MailboxNameException, MailboxNotFoundException}
 import org.apache.james.mailbox.model.{FetchGroup, Mailbox, MailboxId, MailboxPath, MessageRange}
 import org.apache.james.mailbox.{MailboxManager, MailboxSession}
@@ -77,7 +78,7 @@ case class DeletionFailure(mailboxId: UnparsedMailboxId, exception: Throwable) e
     case e: MailboxNotFoundException => MailboxSetError.notFound(Some(SetErrorDescription(e.getMessage)))
     case e: MailboxHasMailException => MailboxSetError.mailboxHasEmail(Some(SetErrorDescription(s"${e.mailboxId.serialize} is not empty")))
     case e: MailboxHasChildException => MailboxSetError.mailboxHasChild(Some(SetErrorDescription(s"${e.mailboxId.serialize} has child mailboxes")))
-    case e: IllegalArgumentException => MailboxSetError.invalidArgument(Some(SetErrorDescription(s"${mailboxId} is not a mailboxId")), None)
+    case e: IllegalArgumentException => MailboxSetError.invalidArgument(Some(SetErrorDescription(s"${mailboxId} is not a mailboxId: ${e.getMessage}")), None)
     case _ => MailboxSetError.serverFail(Some(SetErrorDescription(exception.getMessage)), None)
   }
 }
@@ -102,15 +103,14 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
                                  metricFactory: MetricFactory) extends Method {
   override val methodName: MethodName = MethodName("Mailbox/set")
 
-
-  override def process(capabilities: Set[CapabilityIdentifier], invocation: Invocation, mailboxSession: MailboxSession): Publisher[Invocation] = {
+  override def process(capabilities: Set[CapabilityIdentifier], invocation: Invocation, mailboxSession: MailboxSession, processingContext: ProcessingContext): Publisher[Invocation] = {
     metricFactory.decoratePublisherWithTimerMetricLogP99(JMAP_RFC8621_PREFIX + methodName.value,
       asMailboxSetRequest(invocation.arguments)
         .flatMap(mailboxSetRequest => {
           val (unparsableCreateRequests, createRequests) = parseCreateRequests(mailboxSetRequest)
           for {
-            creationResults <- createMailboxes(mailboxSession, createRequests)
-            deletionResults <- deleteMailboxes(mailboxSession, mailboxSetRequest.destroy.getOrElse(Seq()))
+            creationResults <- createMailboxes(mailboxSession, createRequests, processingContext)
+            deletionResults <- deleteMailboxes(mailboxSession, mailboxSetRequest.destroy.getOrElse(Seq()), processingContext)
           } yield createResponse(invocation, mailboxSetRequest, unparsableCreateRequests, creationResults, deletionResults)
         }))
   }
@@ -136,18 +136,23 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
       case (path, _) => MailboxSetError.invalidArgument(Some(SetErrorDescription(s"Unknown error on property '$path'")), None)
     }
 
-  private def deleteMailboxes(mailboxSession: MailboxSession, deleteRequests: immutable.Iterable[UnparsedMailboxId]): SMono[DeletionResults] = {
+  private def deleteMailboxes(mailboxSession: MailboxSession, deleteRequests: immutable.Iterable[UnparsedMailboxId], processingContext: ProcessingContext): SMono[DeletionResults] = {
     SFlux.fromIterable(deleteRequests)
-      .flatMap(id => SMono.just(id)
-        .map(id => mailboxIdFactory.fromString(id))
-        .flatMap(mailboxId => SMono.fromCallable(() => delete(mailboxSession, mailboxId))
-          .subscribeOn(Schedulers.elastic())
-          .`then`(SMono.just[DeletionResult](DeletionSuccess(mailboxId))))
+      .flatMap(id => delete(mailboxSession, processingContext, id)
         .onErrorRecover(e => DeletionFailure(id, e)))
       .collectSeq()
       .map(DeletionResults)
   }
 
+  private def delete(mailboxSession: MailboxSession, processingContext: ProcessingContext, id: UnparsedMailboxId): SMono[DeletionResult] = {
+    processingContext.resolveMailboxId(id, mailboxIdFactory) match {
+      case Right(mailboxId) => SMono.fromCallable(() => delete(mailboxSession, mailboxId))
+        .subscribeOn(Schedulers.elastic())
+        .`then`(SMono.just[DeletionResult](DeletionSuccess(mailboxId)))
+      case Left(e) => SMono.raiseError(e)
+    }
+  }
+
   private def delete(mailboxSession: MailboxSession, id: MailboxId): Mailbox = {
     val mailbox = mailboxManager.getMailbox(id, mailboxSession)
     if (mailbox.getMessages(MessageRange.all(), FetchGroup.MINIMAL, mailboxSession).hasNext) {
@@ -159,34 +164,70 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
     mailboxManager.deleteMailbox(id, mailboxSession)
   }
 
-  private def createMailboxes(mailboxSession: MailboxSession, createRequests: immutable.Iterable[(MailboxCreationId, MailboxCreationRequest)]): SMono[CreationResults] = {
+  private def createMailboxes(mailboxSession: MailboxSession,
+                              createRequests: immutable.Iterable[(MailboxCreationId, MailboxCreationRequest)],
+                              processingContext: ProcessingContext): SMono[CreationResults] = {
     SFlux.fromIterable(createRequests).flatMap {
-      case (mailboxCreationId: MailboxCreationId, mailboxCreationRequest: MailboxCreationRequest) => {
+      case (mailboxCreationId: MailboxCreationId, mailboxCreationRequest: MailboxCreationRequest) =>
         SMono.fromCallable(() => {
-          createMailbox(mailboxSession, mailboxCreationId, mailboxCreationRequest)
+          createMailbox(mailboxSession, mailboxCreationId, mailboxCreationRequest, processingContext)
         }).subscribeOn(Schedulers.elastic())
-      }
     }
       .collectSeq()
       .map(CreationResults)
   }
 
-  private def createMailbox(mailboxSession: MailboxSession, mailboxCreationId: MailboxCreationId, mailboxCreationRequest: MailboxCreationRequest): CreationResult = {
+  private def createMailbox(mailboxSession: MailboxSession,
+                            mailboxCreationId: MailboxCreationId,
+                            mailboxCreationRequest: MailboxCreationRequest,
+                            processingContext: ProcessingContext): CreationResult = {
+    resolvePath(mailboxSession, mailboxCreationRequest)
+      .map(path => createMailbox(mailboxSession, mailboxCreationId, processingContext, path))
+      .fold(e => CreationFailure(mailboxCreationId, e), r => r)
+  }
+
+  private def createMailbox(mailboxSession: MailboxSession,
+                            mailboxCreationId: MailboxCreationId,
+                            processingContext: ProcessingContext,
+                            path: MailboxPath): CreationResult = {
     try {
-      val path: MailboxPath = if (mailboxCreationRequest.parentId.isEmpty) {
-        MailboxPath.forUser(mailboxSession.getUser, mailboxCreationRequest.name)
-      } else {
-        val parentId: MailboxId = mailboxCreationRequest.parentId.get
-        val parentPath: MailboxPath = mailboxManager.getMailbox(parentId, mailboxSession).getMailboxPath
-        parentPath.child(mailboxCreationRequest.name, mailboxSession.getPathDelimiter)
-      }
       //can safely do a get as the Optional is empty only if the mailbox name is empty which is forbidden by the type constraint on MailboxName
-      CreationSuccess(mailboxCreationId, mailboxManager.createMailbox(path, mailboxSession).get())
+      val mailboxId = mailboxManager.createMailbox(path, mailboxSession).get()
+      recordCreationIdInProcessingContext(mailboxCreationId, processingContext, mailboxId)
+      CreationSuccess(mailboxCreationId, mailboxId)
     } catch {
       case error: Exception => CreationFailure(mailboxCreationId, error)
     }
   }
 
+  private def recordCreationIdInProcessingContext(mailboxCreationId: MailboxCreationId,
+                                                  processingContext: ProcessingContext,
+                                                  mailboxId: MailboxId): Unit = {
+    for {
+      creationId <- Id.validate(mailboxCreationId)
+      serverAssignedId <- Id.validate(mailboxId.serialize())
+    } yield {
+      processingContext.recordCreatedId(ClientId(creationId), ServerId(serverAssignedId))
+    }
+  }
+
+  private def resolvePath(mailboxSession: MailboxSession,
+                          mailboxCreationRequest: MailboxCreationRequest): Either[Exception, MailboxPath] = {
+    mailboxCreationRequest.parentId
+      .map(parentId => for {
+        parentPath <- retrievePath(parentId, mailboxSession)
+      } yield {
+        parentPath.child(mailboxCreationRequest.name, mailboxSession.getPathDelimiter)
+      })
+      .getOrElse(Right(MailboxPath.forUser(mailboxSession.getUser, mailboxCreationRequest.name)))
+  }
+
+  private def retrievePath(mailboxId: MailboxId, mailboxSession: MailboxSession): Either[Exception, MailboxPath] = try {
+      Right(mailboxManager.getMailbox(mailboxId, mailboxSession).getMailboxPath)
+    } catch {
+      case e: Exception => Left(e)
+    }
+
   private def createResponse(invocation: Invocation, mailboxSetRequest: MailboxSetRequest,
                              unparsableCreateRequests: immutable.Iterable[(MailboxCreationId, MailboxSetError)],
                              creationResults: CreationResults, deletionResults: DeletionResults): Invocation = {
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/Method.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/Method.scala
index 078929e..937112b 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/Method.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/Method.scala
@@ -22,6 +22,7 @@ package org.apache.james.jmap.method
 import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier
 import org.apache.james.jmap.model.Invocation
 import org.apache.james.jmap.model.Invocation.MethodName
+import org.apache.james.jmap.routes.ProcessingContext
 import org.apache.james.mailbox.MailboxSession
 import org.reactivestreams.Publisher
 
@@ -30,6 +31,6 @@ trait Method {
 
   val methodName: MethodName
 
-  def process(capabilities: Set[CapabilityIdentifier], invocation: Invocation, mailboxSession: MailboxSession): Publisher[Invocation]
+  def process(capabilities: Set[CapabilityIdentifier], invocation: Invocation, mailboxSession: MailboxSession, processingContext: ProcessingContext): Publisher[Invocation]
 }
 
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/Id.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/Id.scala
index f1178ae..f532fd8 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/Id.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/Id.scala
@@ -19,6 +19,7 @@
 
 package org.apache.james.jmap.model
 
+import eu.timepit.refined
 import eu.timepit.refined.api.Refined
 import eu.timepit.refined.boolean.And
 import eu.timepit.refined.collection.Size
@@ -26,7 +27,13 @@ import eu.timepit.refined.numeric.Interval
 import eu.timepit.refined.string.MatchesRegex
 
 object Id {
-  type Id = String Refined And[
+  type IdConstraint = And[
     Size[Interval.Closed[1, 255]],
-    MatchesRegex["^[a-zA-Z0-9-_]*$"]]
+    MatchesRegex["^[#a-zA-Z0-9-_]*$"]]
+  type Id = String Refined IdConstraint
+
+  def validate(string: String): Either[IllegalArgumentException, Id] =
+    refined.refineV[IdConstraint](string)
+      .left
+      .map(new IllegalArgumentException(_))
 }
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/RequestObject.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/RequestObject.scala
index 12e2223..f356663 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/RequestObject.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/RequestObject.scala
@@ -22,7 +22,16 @@ package org.apache.james.jmap.model
 import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier
 import org.apache.james.jmap.model.Id.Id
 
-final case class ClientId(value: Id)
+final case class ClientId(value: Id) {
+  def referencesPreviousCreationId: Boolean = value.value.startsWith("#")
+  def retrieveOriginalClientId: Option[Either[IllegalArgumentException, ClientId]] =
+    if (referencesPreviousCreationId) {
+      Some(Id.validate(value.value.substring(1, value.value.length))
+        .map(ClientId))
+    } else {
+      None
+    }
+}
 
 final case class ServerId(value: Id)
 
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/JMAPApiRoutes.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/JMAPApiRoutes.scala
index cfdbc2d..a3dc83f 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/JMAPApiRoutes.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/JMAPApiRoutes.scala
@@ -112,6 +112,7 @@ class JMAPApiRoutes (val authenticator: Authenticator,
   private def process(requestObject: RequestObject,
                       httpServerResponse: HttpServerResponse,
                       mailboxSession: MailboxSession): SMono[Void] = {
+    val processingContext: ProcessingContext = new ProcessingContext
     val unsupportedCapabilities = requestObject.using.toSet -- DefaultCapabilities.SUPPORTED.ids
 
     if (unsupportedCapabilities.nonEmpty) {
@@ -119,8 +120,8 @@ class JMAPApiRoutes (val authenticator: Authenticator,
     } else {
       requestObject
         .methodCalls
-        .map(invocation => this.processMethodWithMatchName(requestObject.using.toSet, invocation, mailboxSession))
-        .foldLeft(SFlux.empty[Invocation]) { (flux: SFlux[Invocation], mono: SMono[Invocation]) => flux.mergeWith(mono) }
+        .map(invocation => this.processMethodWithMatchName(requestObject.using.toSet, invocation, mailboxSession, processingContext))
+        .foldLeft(SFlux.empty[Invocation]) { (flux: SFlux[Invocation], mono: SMono[Invocation]) => flux.concatWith(mono) }
         .collectSeq()
         .flatMap((invocations: Seq[Invocation]) =>
           SMono.fromPublisher(httpServerResponse.status(OK)
@@ -134,9 +135,9 @@ class JMAPApiRoutes (val authenticator: Authenticator,
     }
   }
 
-  private def processMethodWithMatchName(capabilities: Set[CapabilityIdentifier], invocation: Invocation, mailboxSession: MailboxSession): SMono[Invocation] =
+  private def processMethodWithMatchName(capabilities: Set[CapabilityIdentifier], invocation: Invocation, mailboxSession: MailboxSession, processingContext: ProcessingContext): SMono[Invocation] =
     SMono.justOrEmpty(methodsByName.get(invocation.methodName))
-      .flatMap(method => SMono.fromPublisher(method.process(capabilities, invocation, mailboxSession)))
+      .flatMap(method => SMono.fromPublisher(method.process(capabilities, invocation, mailboxSession, processingContext)))
       .onErrorResume(throwable => SMono.just(Invocation.error(ErrorCode.ServerFail, throwable.getMessage, invocation.methodCallId)))
       .switchIfEmpty(SMono.just(Invocation.error(ErrorCode.UnknownMethod, invocation.methodCallId)))
 
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/ProcessingContext.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/ProcessingContext.scala
new file mode 100644
index 0000000..e2e0e9a
--- /dev/null
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/ProcessingContext.scala
@@ -0,0 +1,52 @@
+/****************************************************************
+ * 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.jmap.routes
+
+import org.apache.james.jmap.mail.MailboxSetRequest.UnparsedMailboxId
+import org.apache.james.jmap.model.{ClientId, Id, ServerId}
+import org.apache.james.mailbox.model.MailboxId
+
+import scala.collection.mutable
+
+class ProcessingContext {
+ private val creationIds: mutable.Map[ClientId, ServerId] = mutable.Map()
+
+ def recordCreatedId(clientId: ClientId, serverId: ServerId): Unit = creationIds.put(clientId, serverId)
+ private def retrieveServerId(clientId: ClientId): Option[ServerId] = creationIds.get(clientId)
+
+ def resolveMailboxId(unparsedMailboxId: UnparsedMailboxId, mailboxIdFactory: MailboxId.Factory): Either[IllegalArgumentException, MailboxId] =
+  Id.validate(unparsedMailboxId.value)
+      .flatMap(id => resolveServerId(ClientId(id)))
+      .flatMap(serverId => parseMailboxId(mailboxIdFactory, serverId))
+
+ private def parseMailboxId(mailboxIdFactory: MailboxId.Factory, serverId: ServerId) =
+  try {
+   Right(mailboxIdFactory.fromString(serverId.value.value))
+  } catch {
+   case e: IllegalArgumentException => Left(e)
+  }
+
+ private def resolveServerId(id: ClientId): Either[IllegalArgumentException, ServerId] =
+  id.retrieveOriginalClientId
+    .map(maybePreviousClientId => maybePreviousClientId.flatMap(previousClientId => retrieveServerId(previousClientId)
+      .map(serverId => Right(serverId))
+      .getOrElse(Left[IllegalArgumentException, ServerId](new IllegalArgumentException(s"$id was not used in previously defined creationIds")))))
+    .getOrElse(Right(ServerId(id.value)))
+}
diff --git a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/method/CoreEchoMethodTest.scala b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/method/CoreEchoMethodTest.scala
index 9415c16..1f7d859 100644
--- a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/method/CoreEchoMethodTest.scala
+++ b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/method/CoreEchoMethodTest.scala
@@ -21,6 +21,7 @@ package org.apache.james.jmap.method
 import org.apache.james.jmap.json.Fixture.{invocation1, invocation2}
 import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier
 import org.apache.james.jmap.model.{CapabilityIdentifier, Invocation}
+import org.apache.james.jmap.routes.ProcessingContext
 import org.apache.james.mailbox.MailboxSession
 import org.mockito.Mockito.mock
 import org.scalatest.matchers.should.Matchers
@@ -36,14 +37,14 @@ class CoreEchoMethodTest extends AnyWordSpec with Matchers {
     "Process" should {
       "success and return the same with parameters as the invocation request" in {
         val expectedResponse: Invocation = invocation1
-        val dataResponse = SMono.fromPublisher(echoMethod.process(capabilities, invocation1, mockedSession)).block()
+        val dataResponse = SMono.fromPublisher(echoMethod.process(capabilities, invocation1, mockedSession, new ProcessingContext)).block()
 
         dataResponse shouldBe expectedResponse
       }
 
       "success and not return anything else different than the original invocation" in {
         val wrongExpected: Invocation = invocation2
-        val dataResponse = SMono.fromPublisher(echoMethod.process(capabilities, invocation1, mockedSession)).block()
+        val dataResponse = SMono.fromPublisher(echoMethod.process(capabilities, invocation1, mockedSession, new ProcessingContext)).block()
         
         dataResponse should not be(wrongExpected)
       }
diff --git a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/routes/JMAPApiRoutesTest.scala b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/routes/JMAPApiRoutesTest.scala
index 8c2f9b8..28a1640 100644
--- a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/routes/JMAPApiRoutesTest.scala
+++ b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/routes/JMAPApiRoutesTest.scala
@@ -439,7 +439,7 @@ class JMAPApiRoutesTest extends AnyFlatSpec with BeforeAndAfter with Matchers {
     doThrow(new RuntimeException("Unexpected Exception occur, the others method may proceed normally"))
       .doCallRealMethod()
       .when(mockCoreEchoMethod)
-      .process(any[Set[CapabilityIdentifier]], any(), any())
+      .process(any[Set[CapabilityIdentifier]], any(), any(), any())
 
     when(mockCoreEchoMethod.methodName).thenReturn(MethodName("Core/echo"))
 


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


[james-project] 10/11: JAMES-3357 MailboxSetMethodContract: add missing spaces

Posted by bt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 8bd2880e1d105d28dea94dd9f894657402a78f2a
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Mon Aug 17 16:30:53 2020 +0700

    JAMES-3357 MailboxSetMethodContract: add missing spaces
---
 .../james/jmap/rfc8621/contract/MailboxSetMethodContract.scala      | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
index bf6c8aa..eae547b 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
+++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
@@ -310,7 +310,7 @@ trait MailboxSetMethodContract {
 
   @Test
   def mailboxSetShouldNotSubscribeMailboxWhenRequired(server: GuiceJamesServer): Unit = {
-    val request=
+    val request =
       """
         |{
         |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
@@ -351,7 +351,7 @@ trait MailboxSetMethodContract {
 
   @Test
   def mailboxSetShouldSubscribeMailboxByDefault(server: GuiceJamesServer): Unit = {
-    val request=
+    val request =
       """
         |{
         |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
@@ -496,7 +496,7 @@ trait MailboxSetMethodContract {
 
   @Test
   def destroyShouldUnsubscribeMailboxes(server: GuiceJamesServer): Unit = {
-    val request=
+    val request =
       """
         |{
         |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],


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


[james-project] 07/11: JAMES-3356 Fix cosmetic issues in MailboxSetMethodContract

Posted by bt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 8852e1d0e7805e76fde80be3cfa68304ac50eb17
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Wed Aug 19 09:20:32 2020 +0700

    JAMES-3356 Fix cosmetic issues in MailboxSetMethodContract
---
 .../contract/MailboxSetMethodContract.scala        | 28 +++++++++++-----------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
index 21e234e..ac8b4a6 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
+++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
@@ -59,7 +59,7 @@ trait MailboxSetMethodContract {
 
   @Test
   def mailboxSetShouldReturnNotCreatedWhenNameIsMissing(): Unit = {
-    val request=
+    val request =
       """
         |{
         |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
@@ -115,7 +115,7 @@ trait MailboxSetMethodContract {
   @Test
   @Disabled("should we support that? Anyway seems hard with Play-JSON")
   def mailboxSetShouldReturnNotCreatedWhenUnknownParameter(): Unit = {
-    val request=
+    val request =
       """
         |{
         |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
@@ -172,7 +172,7 @@ trait MailboxSetMethodContract {
 
   @Test
   def mailboxSetShouldReturnNotCreatedWhenBadParameter(): Unit = {
-    val request=
+    val request =
       """
         |{
         |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
@@ -229,7 +229,7 @@ trait MailboxSetMethodContract {
 
   @Test
   def mailboxSetShouldCreateMailboxWhenOnlyName(server: GuiceJamesServer): Unit = {
-    val request=
+    val request =
       """
         |{
         |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
@@ -269,7 +269,7 @@ trait MailboxSetMethodContract {
 
   @Test
   def mailboxGetShouldAllowTheUseOfCreationIds(server: GuiceJamesServer): Unit = {
-    val request=
+    val request =
       """
         |{
         |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
@@ -374,7 +374,7 @@ trait MailboxSetMethodContract {
 
   @Test
   def mailboxSetShouldReturnCreatedWhenOnlyName(server: GuiceJamesServer): Unit = {
-    val request=
+    val request =
       """
         |{
         |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
@@ -439,7 +439,7 @@ trait MailboxSetMethodContract {
 
   @Test
   def mailboxSetShouldReturnCreatedAndNotCreatedWhenOneWithOnlyNameAndOneWithoutName(server: GuiceJamesServer): Unit = {
-    val request=
+    val request =
       """
         |{
         |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
@@ -513,7 +513,7 @@ trait MailboxSetMethodContract {
   @Test
   def mailboxSetShouldCreateMailboxWhenNameAndParentId(server: GuiceJamesServer): Unit = {
     val mailboxId: MailboxId  = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB, "parentMailbox"))
-    val request=
+    val request =
       s"""
         |{
         |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
@@ -552,7 +552,7 @@ trait MailboxSetMethodContract {
   @Test
   def mailboxSetShouldNotCreateMailboxWhenParentIdNotFound(): Unit = {
     val mailboxId: MailboxId  = randomMailboxId
-    val request=
+    val request =
       s"""
         |{
         |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
@@ -610,7 +610,7 @@ trait MailboxSetMethodContract {
   @Test
   def mailboxSetShouldNotCreateMailboxWhenNameExists(server: GuiceJamesServer): Unit = {
     server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB, "mailbox"))
-    val request=
+    val request =
       s"""
         |{
         |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
@@ -666,7 +666,7 @@ trait MailboxSetMethodContract {
 
   @Test
   def mailboxSetShouldNotCreateMailboxWhenNameTooLong(): Unit = {
-    val request=
+    val request =
       s"""
         |{
         |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
@@ -1294,9 +1294,9 @@ trait MailboxSetMethodContract {
         .log().ifValidationFails()
         .statusCode(SC_OK)
         .contentType(JSON)
-       // We need to limit ourself to simple body assertions in order not to infer id allocation
-       .body("methodResponses[0][1].created.C42.totalThreads", equalTo(0))
-       .body("methodResponses[1][1].destroyed", hasSize(1))
+         // We need to limit ourself to simple body assertions in order not to infer id allocation
+         .body("methodResponses[0][1].created.C42.totalThreads", equalTo(0))
+         .body("methodResponses[1][1].destroyed", hasSize(1))
   }
 
   @Test


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


[james-project] 01/11: JAMES-3355 Mailbox/set destroy implementation

Posted by bt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit a0534dacbf5250687986963ebeb291f58ecfdb6a
Author: Rémi Kowalski <rk...@linagora.com>
AuthorDate: Wed Jul 29 17:03:49 2020 +0200

    JAMES-3355 Mailbox/set destroy implementation
---
 .../contract/MailboxSetMethodContract.scala        | 485 ++++++++++++++++++++-
 .../org/apache/james/jmap/mail/MailboxSet.scala    |  13 +-
 .../james/jmap/method/MailboxSetMethod.scala       |  72 ++-
 .../org/apache/james/jmap/model/Invocation.scala   |   1 -
 .../apache/james/jmap/routes/JMAPApiRoutes.scala   |   1 -
 5 files changed, 555 insertions(+), 17 deletions(-)

diff --git a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
index d248288..8a67f7a 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
+++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
@@ -19,16 +19,22 @@
 
 package org.apache.james.jmap.rfc8621.contract
 
+import java.nio.charset.StandardCharsets
+
 import io.netty.handler.codec.http.HttpHeaderNames.ACCEPT
 import io.restassured.RestAssured._
 import io.restassured.http.ContentType.JSON
 import net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson
+import net.javacrumbs.jsonunit.core.Option
+import net.javacrumbs.jsonunit.core.internal.Options
 import org.apache.http.HttpStatus.SC_OK
 import org.apache.james.GuiceJamesServer
 import org.apache.james.jmap.http.UserCredential
-import org.apache.james.jmap.rfc8621.contract.Fixture._
+import org.apache.james.jmap.rfc8621.contract.Fixture.{ACCEPT_RFC8621_VERSION_HEADER, ANDRE, BOB, BOB_PASSWORD, DOMAIN, authScheme, baseRequestSpecBuilder}
+import org.apache.james.mailbox.MessageManager.AppendCommand
 import org.apache.james.mailbox.model.MailboxACL.Right
 import org.apache.james.mailbox.model.{MailboxACL, MailboxId, MailboxPath}
+import org.apache.james.mime4j.dom.Message
 import org.apache.james.modules.{ACLProbeImpl, MailboxProbeImpl}
 import org.apache.james.utils.DataProbeImpl
 import org.assertj.core.api.Assertions
@@ -438,7 +444,7 @@ trait MailboxSetMethodContract {
   }
 
   @Test
-  def mailboxSetShouldNotCreateMailboxWhenParentIdNotFound(server: GuiceJamesServer): Unit = {
+  def mailboxSetShouldNotCreateMailboxWhenParentIdNotFound(): Unit = {
     val mailboxId: MailboxId  = randomMailboxId
     val request=
       s"""
@@ -669,4 +675,479 @@ trait MailboxSetMethodContract {
          |    "c1"]]
          |}""".stripMargin)
   }
+
+  @Test
+  def deleteShouldSucceedWhenMailboxExists(server: GuiceJamesServer): Unit = {
+    val mailboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB, "mailbox"))
+
+    val request =
+      s"""
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |       [
+        |           "Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "destroy": ["${mailboxId.serialize}"]
+        |           },
+        |    "c1"
+        |       ]
+        |   ]
+        |}
+        |""".stripMargin
+
+     val response = `given`
+        .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+        .body(request)
+      .when
+        .post
+      .`then`
+        .log().ifValidationFails()
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+    assertThatJson(response).isEqualTo(
+      s"""{
+         |  "sessionState": "75128aab4b1b",
+         |  "methodResponses": [[
+         |    "Mailbox/set",
+         |    {
+         |      "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |      "newState": "000001",
+         |      "destroyed": ["${mailboxId.serialize}"]
+         |    },
+         |    "c1"]]
+         |}""".stripMargin)
+  }
+
+  @Test
+  def deleteShouldRemoveExistingMailbox(server: GuiceJamesServer): Unit = {
+    val mailboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB, "mailbox"))
+
+    val request =
+      s"""
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |       [
+        |           "Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "destroy": ["${mailboxId.serialize}"]
+        |           },
+        |    "c1"
+        |       ]
+        |   ]
+        |}
+        |""".stripMargin
+
+     `given`
+        .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+        .body(request)
+      .when
+        .post
+      .`then`
+        .log().ifValidationFails()
+        .statusCode(SC_OK)
+        .contentType(JSON)
+
+    val response = `given`
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(s"""{
+               |  "using": [
+               |    "urn:ietf:params:jmap:core",
+               |    "urn:ietf:params:jmap:mail",
+               |    "urn:apache:james:params:jmap:mail:quota"],
+               |  "methodCalls": [[
+               |      "Mailbox/get",
+               |      {
+               |        "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+               |        "ids": ["${mailboxId.serialize()}"]
+               |      },
+               |      "c1"]]
+               |}""".stripMargin)
+    .when
+      .post
+    .`then`()
+      .extract
+      .body
+      .asString
+
+    assertThatJson(response).isEqualTo(
+      s"""{
+         |    "sessionState": "75128aab4b1b",
+         |    "methodResponses": [
+         |        [
+         |            "Mailbox/get",
+         |            {
+         |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |                "state": "000001",
+         |                "list": [
+         |
+         |                ],
+         |                "notFound": [
+         |                    "${mailboxId.serialize()}"
+         |                ]
+         |            },
+         |            "c1"
+         |        ]
+         |    ]
+         |}""".stripMargin)
+  }
+
+  @Test
+  def deleteShouldRemoveExistingMailboxes(server: GuiceJamesServer): Unit = {
+    val mailboxId1: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB, "mailbox1"))
+    val mailboxId2: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB, "mailbox2"))
+
+    val request =
+      s"""
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |       [
+        |           "Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "destroy": ["${mailboxId1.serialize}", "${mailboxId2.serialize}"]
+        |           },
+        |    "c1"
+        |       ]
+        |   ]
+        |}
+        |""".stripMargin
+
+     `given`
+        .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+        .body(request)
+      .when
+        .post
+      .`then`
+        .log().ifValidationFails()
+        .statusCode(SC_OK)
+        .contentType(JSON)
+
+    val response = `given`
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(s"""{
+               |  "using": [
+               |    "urn:ietf:params:jmap:core",
+               |    "urn:ietf:params:jmap:mail",
+               |    "urn:apache:james:params:jmap:mail:quota"],
+               |  "methodCalls": [[
+               |      "Mailbox/get",
+               |      {
+               |        "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+               |        "ids": ["${mailboxId1.serialize()}", "${mailboxId2.serialize()}"]
+               |      },
+               |      "c1"]]
+               |}""".stripMargin)
+    .when
+      .post
+    .`then`()
+      .extract
+      .body
+      .asString
+
+    assertThatJson(response)
+      .withOptions(new Options(Option.IGNORING_ARRAY_ORDER))
+      .isEqualTo(
+      s"""{
+         |    "sessionState": "75128aab4b1b",
+         |    "methodResponses": [
+         |        [
+         |            "Mailbox/get",
+         |            {
+         |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |                "state": "000001",
+         |                "list": [
+         |
+         |                ],
+         |                "notFound": [
+         |                    "${mailboxId1.serialize()}", "${mailboxId2.serialize()}"
+         |                ]
+         |            },
+         |            "c1"
+         |        ]
+         |    ]
+         |}""".stripMargin)
+  }
+
+  @Test
+  def deleteShouldFailWhenMailboxDoesNotExist(): Unit = {
+    val mailboxId = randomMailboxId
+    val request =
+      s"""
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |       [
+        |           "Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "destroy": ["${mailboxId.serialize()}"]
+        |           },
+        |    "c1"
+        |       ]
+        |   ]
+        |}
+        |""".stripMargin
+
+     val response = `given`
+        .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+        .body(request)
+      .when
+        .post
+      .`then`
+        .log().ifValidationFails()
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+    assertThatJson(response).isEqualTo(
+      s"""{
+         |  "sessionState": "75128aab4b1b",
+         |  "methodResponses": [[
+         |    "Mailbox/set",
+         |    {
+         |      "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |      "newState": "000001",
+         |      "notDestroyed": {
+         |        "${mailboxId.serialize()}": {
+         |          "type": "notFound",
+         |          "description": "${mailboxId.serialize()} can not be found"
+         |        }
+         |      }
+         |    },
+         |    "c1"]]
+         |}""".stripMargin)
+  }
+
+  @Test
+  def deleteShouldFailWhenMailboxIsNotEmpty(server: GuiceJamesServer): Unit = {
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+
+    val mailboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB, "mailbox1"))
+    server.getProbe(classOf[MailboxProbeImpl])
+      .appendMessage(BOB.asString, MailboxPath.forUser(BOB, "mailbox1"), AppendCommand.from(message))
+
+    val request =
+      s"""
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |       [
+        |           "Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "destroy": ["${mailboxId.serialize()}"]
+        |           },
+        |    "c1"
+        |       ]
+        |   ]
+        |}
+        |""".stripMargin
+
+     val response = `given`
+        .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+        .body(request)
+      .when
+        .post
+      .`then`
+        .log().ifValidationFails()
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+    assertThatJson(response).isEqualTo(
+      s"""{
+         |  "sessionState": "75128aab4b1b",
+         |  "methodResponses": [[
+         |    "Mailbox/set",
+         |    {
+         |      "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |      "newState": "000001",
+         |      "notDestroyed": {
+         |        "${mailboxId.serialize()}": {
+         |          "type": "mailboxHasEmail",
+         |          "description": "${mailboxId.serialize()} is not empty"
+         |        }
+         |      }
+         |    },
+         |    "c1"]]
+         |}""".stripMargin)
+  }
+
+  @Test
+  def deleteShouldFailWhenMailboxHasChild(server: GuiceJamesServer): Unit = {
+    val mailboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB, "mailbox1"))
+    server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB, "mailbox1.mailbox2"))
+
+    val request =
+      s"""
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |       [
+        |           "Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "destroy": ["${mailboxId.serialize()}"]
+        |           },
+        |    "c1"
+        |       ]
+        |   ]
+        |}
+        |""".stripMargin
+
+     val response = `given`
+        .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+        .body(request)
+      .when
+        .post
+      .`then`
+        .log().ifValidationFails()
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+    assertThatJson(response).isEqualTo(
+      s"""{
+         |  "sessionState": "75128aab4b1b",
+         |  "methodResponses": [[
+         |    "Mailbox/set",
+         |    {
+         |      "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |      "newState": "000001",
+         |      "notDestroyed": {
+         |        "${mailboxId.serialize()}": {
+         |          "type": "mailboxHasChild",
+         |          "description": "${mailboxId.serialize()} has child mailboxes"
+         |        }
+         |      }
+         |    },
+         |    "c1"]]
+         |}""".stripMargin)
+  }
+
+  @Test
+  def deleteShouldFailWhenNotEnoughRights(server: GuiceJamesServer): Unit = {
+    val path = MailboxPath.forUser(ANDRE, "mailbox")
+    val mailboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(path)
+    server.getProbe(classOf[ACLProbeImpl])
+      .replaceRights(path, BOB.asString, new MailboxACL.Rfc4314Rights(Right.Lookup, Right.Read, Right.CreateMailbox))
+
+    val request =
+      s"""
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |       [
+        |           "Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "destroy": ["${mailboxId.serialize()}"]
+        |           },
+        |    "c1"
+        |       ]
+        |   ]
+        |}
+        |""".stripMargin
+
+     val response = `given`
+        .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+        .body(request)
+      .when
+        .post
+      .`then`
+        .log().ifValidationFails()
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+    assertThatJson(response).isEqualTo(
+      s"""{
+         |  "sessionState": "75128aab4b1b",
+         |  "methodResponses": [[
+         |    "Mailbox/set",
+         |    {
+         |      "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |      "newState": "000001",
+         |      "notDestroyed": {
+         |        "${mailboxId.serialize()}": {
+         |          "type": "notFound",
+         |          "description": "#private:andre@domain.tld:mailbox"
+         |        }
+         |      }
+         |    },
+         |    "c1"]]
+         |}""".stripMargin)
+  }
+
+  @Test
+  def deleteShouldHandleInvalidMailboxId(): Unit = {
+    val request =
+      s"""
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |       [
+        |           "Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "destroy": ["invalid"]
+        |           },
+        |    "c1"
+        |       ]
+        |   ]
+        |}
+        |""".stripMargin
+
+     val response = `given`
+        .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+        .body(request)
+      .when
+        .post
+      .`then`
+        .log().ifValidationFails()
+        .statusCode(SC_OK)
+        .contentType(JSON)
+        .extract
+        .body
+        .asString
+
+    assertThatJson(response).isEqualTo(
+      s"""{
+         |  "sessionState": "75128aab4b1b",
+         |  "methodResponses": [[
+         |    "Mailbox/set",
+         |    {
+         |      "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |      "newState": "000001",
+         |      "notDestroyed": {
+         |        "invalid": {
+         |          "type": "invalidArguments",
+         |          "description": "invalid is not a mailboxId"
+         |        }
+         |      }
+         |    },
+         |    "c1"]]
+         |}""".stripMargin)
+  }
 }
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
index a55a8a3..2b2fc13 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
@@ -23,7 +23,7 @@ import eu.timepit.refined.api.Refined
 import eu.timepit.refined.auto._
 import eu.timepit.refined.collection.NonEmpty
 import org.apache.james.jmap.mail.MailboxName.MailboxName
-import org.apache.james.jmap.mail.MailboxSetRequest.MailboxCreationId
+import org.apache.james.jmap.mail.MailboxSetRequest.{MailboxCreationId, UnparsedMailboxId}
 import org.apache.james.jmap.model.AccountId
 import org.apache.james.jmap.model.State.State
 import org.apache.james.mailbox.Role
@@ -34,11 +34,12 @@ case class MailboxSetRequest(accountId: AccountId,
                              ifInState: Option[State],
                              create: Option[Map[MailboxCreationId, JsObject]],
                              update: Option[Map[MailboxId, MailboxPatchObject]],
-                             destroy: Option[Seq[MailboxId]],
+                             destroy: Option[Seq[UnparsedMailboxId]],
                              onDestroyRemoveEmails: Option[RemoveEmailsOnDestroy])
 
 object MailboxSetRequest {
   type MailboxCreationId = String Refined NonEmpty
+  type UnparsedMailboxId = String Refined NonEmpty
 }
 
 case class RemoveEmailsOnDestroy(value: Boolean) extends AnyVal
@@ -54,15 +55,21 @@ case class MailboxSetResponse(accountId: AccountId,
                               destroyed: Option[Seq[MailboxId]],
                               notCreated: Option[Map[MailboxCreationId, MailboxSetError]],
                               notUpdated: Option[Map[MailboxId, MailboxSetError]],
-                              notDestroyed: Option[Map[MailboxId, MailboxSetError]])
+                              notDestroyed: Option[Map[UnparsedMailboxId, MailboxSetError]])
 
 object MailboxSetError {
   val invalidArgumentValue: SetErrorType = "invalidArguments"
   val serverFailValue: SetErrorType = "serverFail"
+  val notFoundValue: SetErrorType = "notFound"
+  val mailboxHasEmailValue: SetErrorType = "mailboxHasEmail"
+  val mailboxHasChildValue: SetErrorType = "mailboxHasChild"
   val forbiddenValue: SetErrorType = "forbidden"
 
   def invalidArgument(description: Option[SetErrorDescription], properties: Option[Properties]) = MailboxSetError(invalidArgumentValue, description, properties)
   def serverFail(description: Option[SetErrorDescription], properties: Option[Properties]) = MailboxSetError(serverFailValue, description, properties)
+  def notFound(description: Option[SetErrorDescription]) = MailboxSetError(notFoundValue, description, None)
+  def mailboxHasEmail(description: Option[SetErrorDescription]) = MailboxSetError(mailboxHasEmailValue, description, None)
+  def mailboxHasChild(description: Option[SetErrorDescription]) = MailboxSetError(mailboxHasChildValue, description, None)
   def forbidden(description: Option[SetErrorDescription], properties: Option[Properties]) = MailboxSetError(forbiddenValue, description, properties)
 }
 
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
index c2fcd51..44a7a76 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
@@ -22,13 +22,13 @@ package org.apache.james.jmap.method
 import eu.timepit.refined.auto._
 import javax.inject.Inject
 import org.apache.james.jmap.json.Serializer
-import org.apache.james.jmap.mail.MailboxSetRequest.MailboxCreationId
+import org.apache.james.jmap.mail.MailboxSetRequest.{MailboxCreationId, UnparsedMailboxId}
 import org.apache.james.jmap.mail.{IsSubscribed, MailboxCreationRequest, MailboxCreationResponse, MailboxRights, MailboxSetError, MailboxSetRequest, MailboxSetResponse, Properties, SetErrorDescription, TotalEmails, TotalThreads, UnreadEmails, UnreadThreads}
 import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier
 import org.apache.james.jmap.model.Invocation.{Arguments, MethodName}
 import org.apache.james.jmap.model.{Invocation, State}
 import org.apache.james.mailbox.exception.{InsufficientRightsException, MailboxExistsException, MailboxNameException, MailboxNotFoundException}
-import org.apache.james.mailbox.model.{MailboxId, MailboxPath}
+import org.apache.james.mailbox.model.{FetchGroup, Mailbox, MailboxId, MailboxPath, MessageRange}
 import org.apache.james.mailbox.{MailboxManager, MailboxSession}
 import org.apache.james.metrics.api.MetricFactory
 import org.reactivestreams.Publisher
@@ -38,12 +38,13 @@ import reactor.core.scheduler.Schedulers
 
 import scala.collection.immutable
 
+case class MailboxHasMailException(mailboxId: MailboxId) extends Exception
+case class MailboxHasChildException(mailboxId: MailboxId) extends Exception
+
 sealed trait CreationResult {
   def mailboxCreationId: MailboxCreationId
 }
-
 case class CreationSuccess(mailboxCreationId: MailboxCreationId, mailboxId: MailboxId) extends CreationResult
-
 case class CreationFailure(mailboxCreationId: MailboxCreationId, exception: Exception) extends CreationResult {
   def asMailboxSetError: MailboxSetError = exception match {
     case e: MailboxNotFoundException => MailboxSetError.invalidArgument(Some(SetErrorDescription(e.getMessage)), Some(Properties(List("parentId"))))
@@ -53,7 +54,6 @@ case class CreationFailure(mailboxCreationId: MailboxCreationId, exception: Exce
     case _ => MailboxSetError.serverFail(Some(SetErrorDescription(exception.getMessage)), None)
   }
 }
-
 case class CreationResults(created: Seq[CreationResult]) {
   def retrieveCreated: Map[MailboxCreationId, MailboxId] = created
     .flatMap(result => result match {
@@ -70,8 +70,35 @@ case class CreationResults(created: Seq[CreationResult]) {
     .toMap
 }
 
+sealed trait DeletionResult
+case class DeletionSuccess(mailboxId: MailboxId) extends DeletionResult
+case class DeletionFailure(mailboxId: UnparsedMailboxId, exception: Throwable) extends DeletionResult {
+  def asMailboxSetError: MailboxSetError = exception match {
+    case e: MailboxNotFoundException => MailboxSetError.notFound(Some(SetErrorDescription(e.getMessage)))
+    case e: MailboxHasMailException => MailboxSetError.mailboxHasEmail(Some(SetErrorDescription(s"${e.mailboxId.serialize} is not empty")))
+    case e: MailboxHasChildException => MailboxSetError.mailboxHasChild(Some(SetErrorDescription(s"${e.mailboxId.serialize} has child mailboxes")))
+    case e: IllegalArgumentException => MailboxSetError.invalidArgument(Some(SetErrorDescription(s"${mailboxId} is not a mailboxId")), None)
+    case _ => MailboxSetError.serverFail(Some(SetErrorDescription(exception.getMessage)), None)
+  }
+}
+case class DeletionResults(results: Seq[DeletionResult]) {
+  def destroyed: Seq[DeletionSuccess] =
+    results.flatMap(result => result match {
+      case success: DeletionSuccess => Some(success)
+      case _ => None
+    })
+
+  def retrieveErrors: Map[UnparsedMailboxId, MailboxSetError] =
+    results.flatMap(result => result match {
+      case failure: DeletionFailure => Some(failure.mailboxId, failure.asMailboxSetError)
+      case _ => None
+    })
+    .toMap
+}
+
 class MailboxSetMethod @Inject()(serializer: Serializer,
                                  mailboxManager: MailboxManager,
+                                 mailboxIdFactory: MailboxId.Factory,
                                  metricFactory: MetricFactory) extends Method {
   override val methodName: MethodName = MethodName("Mailbox/set")
 
@@ -83,7 +110,8 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
           val (unparsableCreateRequests, createRequests) = parseCreateRequests(mailboxSetRequest)
           for {
             creationResults <- createMailboxes(mailboxSession, createRequests)
-          } yield createResponse(invocation, mailboxSetRequest, unparsableCreateRequests, creationResults)
+            deletionResults <- deleteMailboxes(mailboxSession, mailboxSetRequest.destroy.getOrElse(Seq()))
+          } yield createResponse(invocation, mailboxSetRequest, unparsableCreateRequests, creationResults, deletionResults)
         }))
   }
 
@@ -108,6 +136,29 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
       case (path, _) => MailboxSetError.invalidArgument(Some(SetErrorDescription(s"Unknown error on property '$path'")), None)
     }
 
+  private def deleteMailboxes(mailboxSession: MailboxSession, deleteRequests: immutable.Iterable[UnparsedMailboxId]): SMono[DeletionResults] = {
+    SFlux.fromIterable(deleteRequests)
+      .flatMap(id => SMono.just(id)
+        .map(id => mailboxIdFactory.fromString(id))
+        .flatMap(mailboxId => SMono.fromCallable(() => delete(mailboxSession, mailboxId))
+          .subscribeOn(Schedulers.elastic())
+          .`then`(SMono.just[DeletionResult](DeletionSuccess(mailboxId))))
+        .onErrorRecover(e => DeletionFailure(id, e)))
+      .collectSeq()
+      .map(DeletionResults)
+  }
+
+  private def delete(mailboxSession: MailboxSession, id: MailboxId): Mailbox = {
+    val mailbox = mailboxManager.getMailbox(id, mailboxSession)
+    if (mailbox.getMessages(MessageRange.all(), FetchGroup.MINIMAL, mailboxSession).hasNext) {
+      throw MailboxHasMailException(id)
+    }
+    if (mailboxManager.hasChildren(mailbox.getMailboxPath, mailboxSession)) {
+      throw MailboxHasChildException(id)
+    }
+    mailboxManager.deleteMailbox(id, mailboxSession)
+  }
+
   private def createMailboxes(mailboxSession: MailboxSession, createRequests: immutable.Iterable[(MailboxCreationId, MailboxCreationRequest)]): SMono[CreationResults] = {
     SFlux.fromIterable(createRequests).flatMap {
       case (mailboxCreationId: MailboxCreationId, mailboxCreationRequest: MailboxCreationRequest) => {
@@ -136,13 +187,16 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
     }
   }
 
-  private def createResponse(invocation: Invocation, mailboxSetRequest: MailboxSetRequest, unparsableCreateRequests: immutable.Iterable[(MailboxCreationId, MailboxSetError)], creationResults: CreationResults): Invocation = {
+  private def createResponse(invocation: Invocation, mailboxSetRequest: MailboxSetRequest,
+                             unparsableCreateRequests: immutable.Iterable[(MailboxCreationId, MailboxSetError)],
+                             creationResults: CreationResults, deletionResults: DeletionResults): Invocation = {
     val created: Map[MailboxCreationId, MailboxId] = creationResults.retrieveCreated
 
     Invocation(methodName, Arguments(serializer.serialize(MailboxSetResponse(
       mailboxSetRequest.accountId,
       oldState = None,
       newState = State.INSTANCE,
+      destroyed = Some(deletionResults.destroyed.map(_.mailboxId)).filter(_.nonEmpty),
       created = Some(created.map(creation => (creation._1, MailboxCreationResponse(
         id = creation._2,
         role = None,
@@ -155,13 +209,11 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
         namespace = None,
         quotas = None,
         isSubscribed = IsSubscribed(true)
-
       )))).filter(_.nonEmpty),
       notCreated = Some(unparsableCreateRequests.toMap ++ creationResults.retrieveErrors).filter(_.nonEmpty),
       updated = None,
       notUpdated = None,
-      destroyed = None,
-      notDestroyed = None
+      notDestroyed = Some(deletionResults.retrieveErrors).filter(_.nonEmpty)
     )).as[JsObject]), invocation.methodCallId)
   }
 
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/Invocation.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/Invocation.scala
index 1072310..fd75472 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/Invocation.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/Invocation.scala
@@ -34,7 +34,6 @@ object Invocation {
   case class Arguments(value: JsObject) extends AnyVal
   case class MethodCallId(value: NonEmptyString)
 
-
   def error(errorCode: ErrorCode, description: String, methodCallId: MethodCallId): Invocation =
     Invocation(MethodName("error"),
       Arguments(JsObject(Seq("type" -> JsString(errorCode.code), "description" -> JsString(description)))),
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/JMAPApiRoutes.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/JMAPApiRoutes.scala
index 972bccf..cfdbc2d 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/JMAPApiRoutes.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/JMAPApiRoutes.scala
@@ -24,7 +24,6 @@ import java.util.stream
 import java.util.stream.Stream
 
 import com.fasterxml.jackson.core.JsonParseException
-import eu.timepit.refined.auto._
 import io.netty.handler.codec.http.HttpHeaderNames.CONTENT_TYPE
 import io.netty.handler.codec.http.HttpMethod
 import io.netty.handler.codec.http.HttpResponseStatus.OK


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


[james-project] 11/11: JAMES-3357 MailboxSetMethodContract: respect indent conventions

Posted by bt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 167c6d5a2f82f5abd487aa923ec9f5bbc0fb1c4a
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Mon Aug 17 16:33:02 2020 +0700

    JAMES-3357 MailboxSetMethodContract: respect indent conventions
---
 .../rfc8621/contract/MailboxSetMethodContract.scala  | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
index eae547b..1e7dec4 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
+++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
@@ -141,9 +141,9 @@ trait MailboxSetMethodContract {
       `given`
         .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
         .body(request)
-        .when
+      .when
         .post
-        .`then`
+      .`then`
         .log().ifValidationFails()
         .statusCode(SC_OK)
         .contentType(JSON)
@@ -294,9 +294,9 @@ trait MailboxSetMethodContract {
     `given`
       .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
       .body(request)
-      .when
+    .when
       .post
-      .`then`
+    .`then`
       .log().ifValidationFails()
       .statusCode(SC_OK)
       .contentType(JSON)
@@ -335,9 +335,9 @@ trait MailboxSetMethodContract {
     `given`
       .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
       .body(request)
-      .when
+    .when
       .post
-      .`then`
+    .`then`
       .log().ifValidationFails()
       .statusCode(SC_OK)
       .contentType(JSON)
@@ -375,9 +375,9 @@ trait MailboxSetMethodContract {
     `given`
       .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
       .body(request)
-      .when
+    .when
       .post
-      .`then`
+    .`then`
       .log().ifValidationFails()
       .statusCode(SC_OK)
       .contentType(JSON)
@@ -1501,9 +1501,9 @@ trait MailboxSetMethodContract {
     val response = `given`
       .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
       .body(request)
-      .when
+    .when
       .post
-      .`then`
+    .`then`
       .log().ifValidationFails()
       .statusCode(SC_OK)
       .contentType(JSON)


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


[james-project] 08/11: JAMES-3357 Mailbox/set create should handle isSubscribed

Posted by bt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 2afc8e9f20e80ad77bbe60072f156203dd0953da
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Fri Aug 14 11:53:22 2020 +0700

    JAMES-3357 Mailbox/set create should handle isSubscribed
---
 .../contract/MailboxSetMethodContract.scala        | 124 ++++++++++++++++++++-
 .../org/apache/james/jmap/json/Serializer.scala    |   4 +-
 .../org/apache/james/jmap/mail/MailboxSet.scala    |   2 +-
 .../james/jmap/method/MailboxSetMethod.scala       |  28 +++--
 4 files changed, 143 insertions(+), 15 deletions(-)

diff --git a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
index ac8b4a6..073b482 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
+++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
@@ -268,6 +268,128 @@ trait MailboxSetMethodContract {
   }
 
   @Test
+  def mailboxSetShouldSubscribeMailboxWhenRequired(server: GuiceJamesServer): Unit = {
+    val request =
+      """
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |       [
+        |           "Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "create": {
+        |                    "C42": {
+        |                      "name": "myMailbox",
+        |                      "isSubscribed": true
+        |                    }
+        |                }
+        |           },
+        |    "c1"
+        |       ]
+        |   ]
+        |}
+        |""".stripMargin
+
+    `given`
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .when
+      .post
+      .`then`
+      .log().ifValidationFails()
+      .statusCode(SC_OK)
+      .contentType(JSON)
+      .extract
+      .body
+      .asString
+
+    Assertions.assertThat(server.getProbe(classOf[MailboxProbeImpl])
+      .listSubscriptions(BOB.asString())).contains("myMailbox")
+  }
+
+  @Test
+  def mailboxSetShouldNotSubscribeMailboxWhenRequired(server: GuiceJamesServer): Unit = {
+    val request=
+      """
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |       [
+        |           "Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "create": {
+        |                    "C42": {
+        |                      "name": "myMailbox",
+        |                      "isSubscribed": false
+        |                    }
+        |                }
+        |           },
+        |    "c1"
+        |       ]
+        |   ]
+        |}
+        |""".stripMargin
+
+    `given`
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .when
+      .post
+      .`then`
+      .log().ifValidationFails()
+      .statusCode(SC_OK)
+      .contentType(JSON)
+      .extract
+      .body
+      .asString
+
+    Assertions.assertThat(server.getProbe(classOf[MailboxProbeImpl])
+      .listSubscriptions(BOB.asString())).doesNotContain("myMailbox")
+  }
+
+  @Test
+  def mailboxSetShouldSubscribeMailboxByDefault(server: GuiceJamesServer): Unit = {
+    val request=
+      """
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |       [
+        |           "Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "create": {
+        |                    "C42": {
+        |                      "name": "myMailbox"
+        |                    }
+        |                }
+        |           },
+        |    "c1"
+        |       ]
+        |   ]
+        |}
+        |""".stripMargin
+
+    `given`
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+      .when
+      .post
+      .`then`
+      .log().ifValidationFails()
+      .statusCode(SC_OK)
+      .contentType(JSON)
+      .extract
+      .body
+      .asString
+
+    Assertions.assertThat(server.getProbe(classOf[MailboxProbeImpl])
+      .listSubscriptions(BOB.asString())).contains("myMailbox")
+  }
+
+  @Test
   def mailboxGetShouldAllowTheUseOfCreationIds(server: GuiceJamesServer): Unit = {
     val request =
       """
@@ -364,7 +486,7 @@ trait MailboxSetMethodContract {
          |					"mayDelete": true,
          |					"maySubmit": true
          |				},
-         |				"isSubscribed": false
+         |				"isSubscribed": true
          |			}],
          |      "notFound":[]
          |		}, "c2"]
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/Serializer.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/Serializer.scala
index e635254..ad8a19b 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/Serializer.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/Serializer.scala
@@ -25,7 +25,6 @@ import java.net.URL
 import eu.timepit.refined._
 import eu.timepit.refined.auto._
 import javax.inject.Inject
-
 import org.apache.james.core.{Domain, Username}
 import org.apache.james.jmap.mail.MailboxSetRequest.{MailboxCreationId, UnparsedMailboxId}
 import org.apache.james.jmap.mail.{DelegatedNamespace, Ids, IsSubscribed, Mailbox, MailboxCreationRequest, MailboxCreationResponse, MailboxGetRequest, MailboxGetResponse, MailboxNamespace, MailboxPatchObject, MailboxRights, MailboxSetError, MailboxSetRequest, MailboxSetResponse, MailboxUpdateResponse, MayAddItems, MayCreateChild, MayDelete, MayReadItems, MayRemoveItems, MayRename, MaySetKeywords, MaySetSeen, MaySubmit, NotFound, PersonalNamespace, Properties, Quota, QuotaId, QuotaRoot, Q [...]
@@ -35,7 +34,6 @@ import org.apache.james.jmap.model.Invocation.{Arguments, MethodCallId, MethodNa
 import org.apache.james.jmap.model.{Account, Invocation, Session, _}
 import org.apache.james.mailbox.Role
 import org.apache.james.mailbox.model.{MailboxACL, MailboxId}
-
 import play.api.libs.functional.syntax._
 import play.api.libs.json._
 
@@ -159,7 +157,7 @@ class Serializer @Inject() (mailboxIdFactory: MailboxId.Factory) {
   private implicit val unreadEmailsWrites: Writes[UnreadEmails] = Json.valueWrites[UnreadEmails]
   private implicit val totalThreadsWrites: Writes[TotalThreads] = Json.valueWrites[TotalThreads]
   private implicit val unreadThreadsWrites: Writes[UnreadThreads] = Json.valueWrites[UnreadThreads]
-  private implicit val isSubscribedWrites: Writes[IsSubscribed] = Json.valueWrites[IsSubscribed]
+  private implicit val isSubscribedWrites: Format[IsSubscribed] = Json.valueFormat[IsSubscribed]
 
   private implicit val mayReadItemsWrites: Writes[MayReadItems] = Json.valueWrites[MayReadItems]
   private implicit val mayAddItemsWrites: Writes[MayAddItems] = Json.valueWrites[MayAddItems]
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
index 794a6a6..2d2c0b7 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
@@ -50,7 +50,7 @@ object MailboxSetRequest {
 }
 
 case class RemoveEmailsOnDestroy(value: Boolean) extends AnyVal
-case class MailboxCreationRequest(name: MailboxName, parentId: Option[UnparsedMailboxId])
+case class MailboxCreationRequest(name: MailboxName, parentId: Option[UnparsedMailboxId], isSubscribed: Option[IsSubscribed])
 
 case class MailboxPatchObject(value: Map[String, JsObject])
 
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
index 24b7556..9aa20b4 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
@@ -30,7 +30,7 @@ import org.apache.james.jmap.model.{ClientId, Id, Invocation, ServerId, State}
 import org.apache.james.jmap.routes.ProcessingContext
 import org.apache.james.mailbox.exception.{InsufficientRightsException, MailboxExistsException, MailboxNameException, MailboxNotFoundException}
 import org.apache.james.mailbox.model.{FetchGroup, Mailbox, MailboxId, MailboxPath, MessageRange}
-import org.apache.james.mailbox.{MailboxManager, MailboxSession}
+import org.apache.james.mailbox.{MailboxManager, MailboxSession, SubscriptionManager}
 import org.apache.james.metrics.api.MetricFactory
 import org.reactivestreams.Publisher
 import play.api.libs.json._
@@ -113,6 +113,7 @@ case class DeletionResults(results: Seq[DeletionResult]) {
 
 class MailboxSetMethod @Inject()(serializer: Serializer,
                                  mailboxManager: MailboxManager,
+                                 subscriptionManager: SubscriptionManager,
                                  mailboxIdFactory: MailboxId.Factory,
                                  metricFactory: MetricFactory) extends Method {
   override val methodName: MethodName = MethodName("Mailbox/set")
@@ -177,9 +178,15 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
                             jsObject: JsObject,
                             processingContext: ProcessingContext): CreationResult = {
     parseCreate(jsObject)
-      .flatMap(mailboxCreationRequest => resolvePath(mailboxSession, mailboxCreationRequest, processingContext))
-      .map(path => createMailbox(mailboxSession, mailboxCreationId, processingContext, path))
-      .fold(e => CreationFailure(mailboxCreationId, e), r => r)
+      .flatMap(mailboxCreationRequest => resolvePath(mailboxSession, mailboxCreationRequest, processingContext)
+        .flatMap(path => createMailbox(mailboxSession = mailboxSession,
+          path = path,
+          isSubscribed = mailboxCreationRequest.isSubscribed.getOrElse(IsSubscribed(true)))))
+      .fold(e => CreationFailure(mailboxCreationId, e),
+        mailboxId => {
+          recordCreationIdInProcessingContext(mailboxCreationId, processingContext, mailboxId)
+          CreationSuccess(mailboxCreationId, mailboxId)
+        })
   }
 
   private def parseCreate(jsObject: JsObject): Either[MailboxCreationParseException, MailboxCreationRequest] =
@@ -197,16 +204,17 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
     }
 
   private def createMailbox(mailboxSession: MailboxSession,
-                            mailboxCreationId: MailboxCreationId,
-                            processingContext: ProcessingContext,
-                            path: MailboxPath): CreationResult = {
+                            path: MailboxPath,
+                            isSubscribed: IsSubscribed): Either[Exception, MailboxId] = {
     try {
       //can safely do a get as the Optional is empty only if the mailbox name is empty which is forbidden by the type constraint on MailboxName
       val mailboxId = mailboxManager.createMailbox(path, mailboxSession).get()
-      recordCreationIdInProcessingContext(mailboxCreationId, processingContext, mailboxId)
-      CreationSuccess(mailboxCreationId, mailboxId)
+      if (isSubscribed.value) {
+        subscriptionManager.subscribe(mailboxSession, path.getName)
+      }
+      Right(mailboxId)
     } catch {
-      case error: Exception => CreationFailure(mailboxCreationId, error)
+      case error: Exception => Left(error)
     }
   }
 


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


[james-project] 06/11: JAMES-3356 Allow the use of creationIds in Mailbox/Get

Posted by bt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 6e8161ff71943d486f3717c2547ee4deb53d90fe
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Fri Aug 14 14:06:05 2020 +0700

    JAMES-3356 Allow the use of creationIds in Mailbox/Get
---
 .../contract/MailboxGetMethodContract.scala        |  82 ++++++++++++++++
 .../contract/MailboxSetMethodContract.scala        | 105 +++++++++++++++++++++
 .../org/apache/james/jmap/json/Serializer.scala    |   4 +-
 .../org/apache/james/jmap/mail/MailboxGet.scala    |   6 +-
 .../org/apache/james/jmap/mail/MailboxSet.scala    |   9 +-
 .../james/jmap/method/MailboxGetMethod.scala       |  18 +++-
 .../jmap/json/MailboxGetSerializationTest.scala    |  17 +++-
 7 files changed, 226 insertions(+), 15 deletions(-)

diff --git a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxGetMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxGetMethodContract.scala
index b2c2d31..7776b26 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxGetMethodContract.scala
+++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxGetMethodContract.scala
@@ -560,6 +560,88 @@ trait MailboxGetMethodContract {
   }
 
   @Test
+  def getMailboxesShouldReturnNotFoundWhenInvalid(): Unit = {
+    val response: String = `given`
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(s"""{
+               |  "using": [
+               |    "urn:ietf:params:jmap:core",
+               |    "urn:ietf:params:jmap:mail"],
+               |  "methodCalls": [[
+               |    "Mailbox/get",
+               |    {
+               |      "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+               |      "properties": ["id", "name", "rights"],
+               |      "ids": ["invalid"]
+               |    },
+               |    "c1"]]
+               |}""".stripMargin)
+    .when
+      .post
+    .`then`
+      .statusCode(SC_OK)
+      .contentType(JSON)
+      .extract
+      .body
+      .asString
+
+    assertThatJson(response).isEqualTo(
+      s"""{
+         |  "sessionState": "75128aab4b1b",
+         |  "methodResponses": [[
+         |    "Mailbox/get",
+         |    {
+         |      "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |      "state": "000001",
+         |      "list": [],
+         |      "notFound": ["invalid"]
+         |    },
+         |    "c1"]]
+         |}""".stripMargin)
+  }
+
+  @Test
+  def getMailboxesShouldReturnNotFoundWhenUnknownClientId(): Unit = {
+    val response: String = `given`
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(s"""{
+               |  "using": [
+               |    "urn:ietf:params:jmap:core",
+               |    "urn:ietf:params:jmap:mail"],
+               |  "methodCalls": [[
+               |    "Mailbox/get",
+               |    {
+               |      "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+               |      "properties": ["id", "name", "rights"],
+               |      "ids": ["#C42"]
+               |    },
+               |    "c1"]]
+               |}""".stripMargin)
+    .when
+      .post
+    .`then`
+      .statusCode(SC_OK)
+      .contentType(JSON)
+      .extract
+      .body
+      .asString
+
+    assertThatJson(response).isEqualTo(
+      s"""{
+         |  "sessionState": "75128aab4b1b",
+         |  "methodResponses": [[
+         |    "Mailbox/get",
+         |    {
+         |      "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |      "state": "000001",
+         |      "list": [],
+         |      "notFound": ["#C42"]
+         |    },
+         |    "c1"]]
+         |}""".stripMargin)
+  }
+
+  @Test
   def getMailboxesShouldReturnInvalidArgumentsErrorWhenInvalidProperty(server: GuiceJamesServer): Unit = {
     val mailboxId: String = server.getProbe(classOf[MailboxProbeImpl])
       .createMailbox(MailboxPath.forUser(BOB, "custom"))
diff --git a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
index 28bd5fd..21e234e 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
+++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
@@ -268,6 +268,111 @@ trait MailboxSetMethodContract {
   }
 
   @Test
+  def mailboxGetShouldAllowTheUseOfCreationIds(server: GuiceJamesServer): Unit = {
+    val request=
+      """
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+        |   "methodCalls": [
+        |       [
+        |           "Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "create": {
+        |                    "C42": {
+        |                      "name": "myMailbox"
+        |                    }
+        |                }
+        |           },
+        |    "c1"
+        |       ],
+        |       ["Mailbox/get",
+        |         {
+        |           "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |           "ids": ["#C42"]
+        |          },
+        |       "c2"]
+        |   ]
+        |}
+        |""".stripMargin
+
+    val response = `given`
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(request)
+    .when
+      .post
+    .`then`
+      .log().ifValidationFails()
+      .statusCode(SC_OK)
+      .contentType(JSON)
+      .extract
+      .body
+      .asString
+
+    val mailboxId: String = server.getProbe(classOf[MailboxProbeImpl])
+      .getMailboxId("#private", BOB.asString(), "myMailbox")
+      .serialize()
+
+    assertThatJson(response).isEqualTo(
+      s"""{
+         |	"sessionState": "75128aab4b1b",
+         |	"methodResponses": [
+         |		["Mailbox/set", {
+         |			"accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |			"newState": "000001",
+         |			"created": {
+         |				"C42": {
+         |					"id": "$mailboxId",
+         |					"totalEmails": 0,
+         |					"unreadEmails": 0,
+         |					"totalThreads": 0,
+         |					"unreadThreads": 0,
+         |					"myRights": {
+         |						"mayReadItems": true,
+         |						"mayAddItems": true,
+         |						"mayRemoveItems": true,
+         |						"maySetSeen": true,
+         |						"maySetKeywords": true,
+         |						"mayCreateChild": true,
+         |						"mayRename": true,
+         |						"mayDelete": true,
+         |						"maySubmit": true
+         |					},
+         |					"isSubscribed": true
+         |				}
+         |			}
+         |		}, "c1"],
+         |		["Mailbox/get", {
+         |			"accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |			"state": "000001",
+         |			"list": [{
+         |				"id": "$mailboxId",
+         |				"name": "myMailbox",
+         |				"sortOrder": 1000,
+         |				"totalEmails": 0,
+         |				"unreadEmails": 0,
+         |				"totalThreads": 0,
+         |				"unreadThreads": 0,
+         |				"myRights": {
+         |					"mayReadItems": true,
+         |					"mayAddItems": true,
+         |					"mayRemoveItems": true,
+         |					"maySetSeen": true,
+         |					"maySetKeywords": true,
+         |					"mayCreateChild": true,
+         |					"mayRename": true,
+         |					"mayDelete": true,
+         |					"maySubmit": true
+         |				},
+         |				"isSubscribed": false
+         |			}],
+         |      "notFound":[]
+         |		}, "c2"]
+         |	]
+         |}""".stripMargin)
+  }
+
+  @Test
   def mailboxSetShouldReturnCreatedWhenOnlyName(server: GuiceJamesServer): Unit = {
     val request=
       """
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/Serializer.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/Serializer.scala
index 5ba43fb..e635254 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/Serializer.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/Serializer.scala
@@ -27,7 +27,7 @@ import eu.timepit.refined.auto._
 import javax.inject.Inject
 
 import org.apache.james.core.{Domain, Username}
-import org.apache.james.jmap.mail.MailboxSetRequest.MailboxCreationId
+import org.apache.james.jmap.mail.MailboxSetRequest.{MailboxCreationId, UnparsedMailboxId}
 import org.apache.james.jmap.mail.{DelegatedNamespace, Ids, IsSubscribed, Mailbox, MailboxCreationRequest, MailboxCreationResponse, MailboxGetRequest, MailboxGetResponse, MailboxNamespace, MailboxPatchObject, MailboxRights, MailboxSetError, MailboxSetRequest, MailboxSetResponse, MailboxUpdateResponse, MayAddItems, MayCreateChild, MayDelete, MayReadItems, MayRemoveItems, MayRename, MaySetKeywords, MaySetSeen, MaySubmit, NotFound, PersonalNamespace, Properties, Quota, QuotaId, QuotaRoot, Q [...]
 import org.apache.james.jmap.model
 import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier
@@ -252,7 +252,7 @@ class Serializer @Inject() (mailboxIdFactory: MailboxId.Factory) {
 
   private implicit val mailboxSetRequestReads: Reads[MailboxSetRequest] = Json.reads[MailboxSetRequest]
 
-  private implicit def notFoundWrites(implicit mailboxIdWrites: Writes[MailboxId]): Writes[NotFound] =
+  private implicit def notFoundWrites(implicit mailboxIdWrites: Writes[UnparsedMailboxId]): Writes[NotFound] =
     notFound => JsArray(notFound.value.toList.map(mailboxIdWrites.writes))
 
   private implicit def mailboxGetResponseWrites(implicit mailboxWrites: Writes[Mailbox]): Writes[MailboxGetResponse] = Json.writes[MailboxGetResponse]
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxGet.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxGet.scala
index 613d418..1169652 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxGet.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxGet.scala
@@ -20,11 +20,11 @@
 package org.apache.james.jmap.mail
 
 import eu.timepit.refined.types.string.NonEmptyString
+import org.apache.james.jmap.mail.MailboxSetRequest.UnparsedMailboxId
 import org.apache.james.jmap.model.AccountId
 import org.apache.james.jmap.model.State.State
-import org.apache.james.mailbox.model.MailboxId
 
-case class Ids(value: List[MailboxId])
+case class Ids(value: List[UnparsedMailboxId])
 
 case class Properties(value: List[NonEmptyString]) {
   def asSetOfString: Set[String] = value.map(_.toString()).toSet
@@ -35,7 +35,7 @@ case class MailboxGetRequest(accountId: AccountId,
                              ids: Option[Ids],
                              properties: Option[Properties])
 
-case class NotFound(value: Set[MailboxId]) {
+case class NotFound(value: Set[UnparsedMailboxId]) {
   def merge(other: NotFound): NotFound = NotFound(this.value ++ other.value)
 }
 
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
index fdf2d18..794a6a6 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
@@ -19,6 +19,7 @@
 
 package org.apache.james.jmap.mail
 
+import eu.timepit.refined
 import eu.timepit.refined.api.Refined
 import eu.timepit.refined.auto._
 import eu.timepit.refined.collection.NonEmpty
@@ -38,8 +39,14 @@ case class MailboxSetRequest(accountId: AccountId,
                              onDestroyRemoveEmails: Option[RemoveEmailsOnDestroy])
 
 object MailboxSetRequest {
+  type UnparsedMailboxIdConstraint = NonEmpty
   type MailboxCreationId = String Refined NonEmpty
-  type UnparsedMailboxId = String Refined NonEmpty
+  type UnparsedMailboxId = String Refined UnparsedMailboxIdConstraint
+
+  def asUnparsed(mailboxId: MailboxId): UnparsedMailboxId = refined.refineV[UnparsedMailboxIdConstraint](mailboxId.serialize()) match {
+    case Left(e) => throw new IllegalArgumentException(e)
+    case scala.Right(value) => value
+  }
 }
 
 case class RemoveEmailsOnDestroy(value: Boolean) extends AnyVal
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxGetMethod.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxGetMethod.scala
index 7e66b5e..992cb74 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxGetMethod.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxGetMethod.scala
@@ -22,6 +22,7 @@ package org.apache.james.jmap.method
 import eu.timepit.refined.auto._
 import javax.inject.Inject
 import org.apache.james.jmap.json.Serializer
+import org.apache.james.jmap.mail.MailboxSetRequest.UnparsedMailboxId
 import org.apache.james.jmap.mail._
 import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier
 import org.apache.james.jmap.model.Invocation.{Arguments, MethodName}
@@ -42,20 +43,25 @@ import reactor.core.scheduler.Schedulers
 class MailboxGetMethod @Inject() (serializer: Serializer,
                                   mailboxManager: MailboxManager,
                                   quotaFactory : QuotaLoaderWithPreloadedDefaultFactory,
+                                  mailboxIdFactory: MailboxId.Factory,
                                   mailboxFactory: MailboxFactory,
                                   metricFactory: MetricFactory) extends Method {
   override val methodName: MethodName = MethodName("Mailbox/get")
 
   object MailboxGetResults {
     def found(mailbox: Mailbox): MailboxGetResults = MailboxGetResults(Set(mailbox), NotFound(Set.empty))
-    def notFound(mailboxId: MailboxId): MailboxGetResults = MailboxGetResults(Set.empty, NotFound(Set(mailboxId)))
+    def notFound(mailboxId: UnparsedMailboxId): MailboxGetResults = MailboxGetResults(Set.empty, NotFound(Set(mailboxId)))
+    def notFound(mailboxId: MailboxId): MailboxGetResults = MailboxGetResults(Set.empty, NotFound(Set(MailboxSetRequest.asUnparsed(mailboxId))))
   }
 
   case class MailboxGetResults(mailboxes: Set[Mailbox], notFound: NotFound) {
     def merge(other: MailboxGetResults): MailboxGetResults = MailboxGetResults(this.mailboxes ++ other.mailboxes, this.notFound.merge(other.notFound))
   }
 
-  override def process(capabilities: Set[CapabilityIdentifier], invocation: Invocation, mailboxSession: MailboxSession, processingContext: ProcessingContext): Publisher[Invocation] = {
+  override def process(capabilities: Set[CapabilityIdentifier],
+                       invocation: Invocation,
+                       mailboxSession: MailboxSession,
+                       processingContext: ProcessingContext): Publisher[Invocation] = {
     metricFactory.decoratePublisherWithTimerMetricLogP99(JMAP_RFC8621_PREFIX + methodName.value,
       asMailboxGetRequest(invocation.arguments)
         .flatMap(mailboxGetRequest => {
@@ -64,7 +70,7 @@ class MailboxGetMethod @Inject() (serializer: Serializer,
               SMono.just(Invocation.error(errorCode = ErrorCode.InvalidArguments,
                 description = s"The following properties [${properties.asSetOfString.diff(Mailbox.allProperties).mkString(", ")}] do not exist.",
                 methodCallId = invocation.methodCallId))
-            case _ => getMailboxes(capabilities, mailboxGetRequest, mailboxSession)
+            case _ => getMailboxes(capabilities, mailboxGetRequest, processingContext, mailboxSession)
               .reduce(MailboxGetResults(Set.empty, NotFound(Set.empty)), (result1: MailboxGetResults, result2: MailboxGetResults) => result1.merge(result2))
               .map(mailboxes => MailboxGetResponse(
                 accountId = mailboxGetRequest.accountId,
@@ -89,12 +95,16 @@ class MailboxGetMethod @Inject() (serializer: Serializer,
 
   private def getMailboxes(capabilities: Set[CapabilityIdentifier],
                            mailboxGetRequest: MailboxGetRequest,
+                           processingContext: ProcessingContext,
                            mailboxSession: MailboxSession): SFlux[MailboxGetResults] =
+
     mailboxGetRequest.ids match {
       case None => getAllMailboxes(capabilities, mailboxSession)
         .map(MailboxGetResults.found)
       case Some(ids) => SFlux.fromIterable(ids.value)
-        .flatMap(id => getMailboxResultById(capabilities, id, mailboxSession))
+          .flatMap(id => processingContext.resolveMailboxId(id, mailboxIdFactory)
+            .fold(e => SMono.just(MailboxGetResults.notFound(id)),
+              mailboxId => getMailboxResultById(capabilities, mailboxId, mailboxSession)))
     }
 
   private def getMailboxResultById(capabilities: Set[CapabilityIdentifier],
diff --git a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/json/MailboxGetSerializationTest.scala b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/json/MailboxGetSerializationTest.scala
index aa22699..3fed3f5 100644
--- a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/json/MailboxGetSerializationTest.scala
+++ b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/json/MailboxGetSerializationTest.scala
@@ -24,12 +24,13 @@ import net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson
 import org.apache.james.jmap.json.Fixture._
 import org.apache.james.jmap.json.MailboxGetSerializationTest._
 import org.apache.james.jmap.json.MailboxSerializationTest.MAILBOX
+import org.apache.james.jmap.mail.MailboxSetRequest.UnparsedMailboxId
 import org.apache.james.jmap.mail._
 import org.apache.james.jmap.model.AccountId
 import org.apache.james.mailbox.model.{MailboxId, TestId}
 import org.scalatest.matchers.should.Matchers
 import org.scalatest.wordspec.AnyWordSpec
-import play.api.libs.json.{JsError, JsSuccess, Json}
+import play.api.libs.json.{JsSuccess, Json}
 
 object MailboxGetSerializationTest {
   private val FACTORY: MailboxId.Factory = new TestId.Factory
@@ -38,22 +39,28 @@ object MailboxGetSerializationTest {
 
   private val ACCOUNT_ID: AccountId = AccountId(id)
 
-  private val MAILBOX_ID_1: MailboxId = FACTORY.fromString("1")
-  private val MAILBOX_ID_2: MailboxId = FACTORY.fromString("2")
+  private val MAILBOX_ID_1: UnparsedMailboxId = "1"
+  private val MAILBOX_ID_2: UnparsedMailboxId = "2"
 
   private val PROPERTIES: Properties = Properties(List("name", "role"))
 }
 
 class MailboxGetSerializationTest extends AnyWordSpec with Matchers {
   "Deserialize MailboxGetRequest" should {
-    "fail when MailboxId.Factory can't deserialize MailboxId" in {
+    "succeed on invalid mailboxId" in {
+      // as they are unparsed
+      val expectedRequestObject = MailboxGetRequest(
+        accountId = ACCOUNT_ID,
+        ids = Some(Ids(List("ab#?"))),
+        properties = None)
+
       SERIALIZER.deserializeMailboxGetRequest(
         """
           |{
           |  "accountId": "aHR0cHM6Ly93d3cuYmFzZTY0ZW5jb2RlLm9yZy8",
           |  "ids": ["ab#?"]
           |}
-          |""".stripMargin) shouldBe a [JsError]
+          |""".stripMargin) should equal(JsSuccess(expectedRequestObject))
     }
 
     "succeed when properties are missing" in {


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


[james-project] 03/11: JAMES-3356 Refactoring: unify error handling for mailbox creation parsing/handling

Posted by bt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 6e15127facd944a90d318909d60707293dac79b9
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Fri Aug 14 10:34:45 2020 +0700

    JAMES-3356 Refactoring: unify error handling for mailbox creation parsing/handling
    
    This makes it easier to follow the error management logic as it all goes though one place.
    
    Methods arguments within MailboxSetMethod are furthermore simplified.
---
 .../james/jmap/method/MailboxSetMethod.scala       | 55 ++++++++++------------
 1 file changed, 25 insertions(+), 30 deletions(-)

diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
index ae3bec0..94b64b2 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
@@ -37,10 +37,9 @@ import play.api.libs.json._
 import reactor.core.scala.publisher.{SFlux, SMono}
 import reactor.core.scheduler.Schedulers
 
-import scala.collection.immutable
-
 case class MailboxHasMailException(mailboxId: MailboxId) extends Exception
 case class MailboxHasChildException(mailboxId: MailboxId) extends Exception
+case class MailboxCreationParseException(mailboxSetError: MailboxSetError) extends Exception
 
 sealed trait CreationResult {
   def mailboxCreationId: MailboxCreationId
@@ -51,6 +50,7 @@ case class CreationFailure(mailboxCreationId: MailboxCreationId, exception: Exce
     case e: MailboxNotFoundException => MailboxSetError.invalidArgument(Some(SetErrorDescription(e.getMessage)), Some(Properties(List("parentId"))))
     case e: MailboxExistsException => MailboxSetError.invalidArgument(Some(SetErrorDescription(e.getMessage)), Some(Properties(List("name"))))
     case e: MailboxNameException => MailboxSetError.invalidArgument(Some(SetErrorDescription(e.getMessage)), Some(Properties(List("name"))))
+    case e: MailboxCreationParseException => e.mailboxSetError
     case _: InsufficientRightsException => MailboxSetError.forbidden(Some(SetErrorDescription("Insufficient rights")), Some(Properties(List("parentId"))))
     case _ => MailboxSetError.serverFail(Some(SetErrorDescription(exception.getMessage)), None)
   }
@@ -107,27 +107,18 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
     metricFactory.decoratePublisherWithTimerMetricLogP99(JMAP_RFC8621_PREFIX + methodName.value,
       asMailboxSetRequest(invocation.arguments)
         .flatMap(mailboxSetRequest => {
-          val (unparsableCreateRequests, createRequests) = parseCreateRequests(mailboxSetRequest)
           for {
-            creationResults <- createMailboxes(mailboxSession, createRequests, processingContext)
-            deletionResults <- deleteMailboxes(mailboxSession, mailboxSetRequest.destroy.getOrElse(Seq()), processingContext)
-          } yield createResponse(invocation, mailboxSetRequest, unparsableCreateRequests, creationResults, deletionResults)
+            creationResults <- createMailboxes(mailboxSession, mailboxSetRequest, processingContext)
+            deletionResults <- deleteMailboxes(mailboxSession, mailboxSetRequest, processingContext)
+          } yield createResponse(invocation, mailboxSetRequest, creationResults, deletionResults)
         }))
   }
 
-  private def parseCreateRequests(mailboxSetRequest: MailboxSetRequest): (immutable.Iterable[(MailboxCreationId, MailboxSetError)], immutable.Iterable[(MailboxCreationId, MailboxCreationRequest)]) = {
-    mailboxSetRequest.create
-      .getOrElse(Map.empty)
-      .view
-      .mapValues(value => Json.fromJson(value)(serializer.mailboxCreationRequest))
-      .toMap
-      .partitionMap { case (creationId, creationRequestParseResult) =>
-        creationRequestParseResult match {
-          case JsSuccess(creationRequest, _) => Right((creationId, creationRequest))
-          case JsError(errors) => Left(creationId, mailboxSetError(errors))
-        }
-      }
-  }
+  def parseCreate(jsObject: JsObject): Either[MailboxCreationParseException, MailboxCreationRequest] =
+    Json.fromJson(jsObject)(serializer.mailboxCreationRequest) match {
+      case JsSuccess(creationRequest, _) => Right(creationRequest)
+      case JsError(errors) => Left(MailboxCreationParseException(mailboxSetError(errors)))
+    }
 
   private def mailboxSetError(errors: collection.Seq[(JsPath, collection.Seq[JsonValidationError])]): MailboxSetError =
     errors.head match {
@@ -136,8 +127,8 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
       case (path, _) => MailboxSetError.invalidArgument(Some(SetErrorDescription(s"Unknown error on property '$path'")), None)
     }
 
-  private def deleteMailboxes(mailboxSession: MailboxSession, deleteRequests: immutable.Iterable[UnparsedMailboxId], processingContext: ProcessingContext): SMono[DeletionResults] = {
-    SFlux.fromIterable(deleteRequests)
+  private def deleteMailboxes(mailboxSession: MailboxSession, mailboxSetRequest: MailboxSetRequest, processingContext: ProcessingContext): SMono[DeletionResults] = {
+    SFlux.fromIterable(mailboxSetRequest.destroy.getOrElse(Seq()))
       .flatMap(id => delete(mailboxSession, processingContext, id)
         .onErrorRecover(e => DeletionFailure(id, e)))
       .collectSeq()
@@ -165,12 +156,15 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
   }
 
   private def createMailboxes(mailboxSession: MailboxSession,
-                              createRequests: immutable.Iterable[(MailboxCreationId, MailboxCreationRequest)],
+                              mailboxSetRequest: MailboxSetRequest,
                               processingContext: ProcessingContext): SMono[CreationResults] = {
-    SFlux.fromIterable(createRequests).flatMap {
-      case (mailboxCreationId: MailboxCreationId, mailboxCreationRequest: MailboxCreationRequest) =>
+    SFlux.fromIterable(mailboxSetRequest.create
+      .getOrElse(Map.empty)
+      .view)
+      .flatMap {
+      case (mailboxCreationId: MailboxCreationId, jsObject: JsObject) =>
         SMono.fromCallable(() => {
-          createMailbox(mailboxSession, mailboxCreationId, mailboxCreationRequest, processingContext)
+          createMailbox(mailboxSession, mailboxCreationId, jsObject, processingContext)
         }).subscribeOn(Schedulers.elastic())
     }
       .collectSeq()
@@ -179,9 +173,10 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
 
   private def createMailbox(mailboxSession: MailboxSession,
                             mailboxCreationId: MailboxCreationId,
-                            mailboxCreationRequest: MailboxCreationRequest,
+                            jsObject: JsObject,
                             processingContext: ProcessingContext): CreationResult = {
-    resolvePath(mailboxSession, mailboxCreationRequest)
+    parseCreate(jsObject)
+        .flatMap(mailboxCreationRequest => resolvePath(mailboxSession, mailboxCreationRequest))
       .map(path => createMailbox(mailboxSession, mailboxCreationId, processingContext, path))
       .fold(e => CreationFailure(mailboxCreationId, e), r => r)
   }
@@ -229,8 +224,8 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
     }
 
   private def createResponse(invocation: Invocation, mailboxSetRequest: MailboxSetRequest,
-                             unparsableCreateRequests: immutable.Iterable[(MailboxCreationId, MailboxSetError)],
-                             creationResults: CreationResults, deletionResults: DeletionResults): Invocation = {
+                             creationResults: CreationResults,
+                             deletionResults: DeletionResults): Invocation = {
     val created: Map[MailboxCreationId, MailboxId] = creationResults.retrieveCreated
 
     Invocation(methodName, Arguments(serializer.serialize(MailboxSetResponse(
@@ -251,7 +246,7 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
         quotas = None,
         isSubscribed = IsSubscribed(true)
       )))).filter(_.nonEmpty),
-      notCreated = Some(unparsableCreateRequests.toMap ++ creationResults.retrieveErrors).filter(_.nonEmpty),
+      notCreated = Some(creationResults.retrieveErrors).filter(_.nonEmpty),
       updated = None,
       notUpdated = None,
       notDestroyed = Some(deletionResults.retrieveErrors).filter(_.nonEmpty)


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