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/24 02:22:34 UTC

[james-project] 05/13: JAMES-3357 Mailbox/set creation response needs to display extension properties

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 ecb6553156f93b71108cb604d371b78438f79340
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Wed Aug 19 18:06:15 2020 +0700

    JAMES-3357 Mailbox/set creation response needs to display extension properties
    
    Note that rights are explicitly specified by the client, or empty, thus never needs to be returned.
    
    Roles also always are empty as the user cannot create system mailboxes, that are automatically provisionned.
---
 .../contract/MailboxSetMethodContract.scala        | 75 ++++++++++++++++++++++
 .../org/apache/james/jmap/json/Serializer.scala    | 22 +++++--
 .../org/apache/james/jmap/mail/MailboxSet.scala    | 29 +++++++--
 .../james/jmap/method/MailboxSetMethod.scala       | 53 +++++++--------
 4 files changed, 141 insertions(+), 38 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 3bbc06d..b1d70b0 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
@@ -910,6 +910,81 @@ trait MailboxSetMethodContract {
   }
 
   @Test
+  def mailboxCreationShouldReturnQuotaWhenUsingQuotaExtension(server: GuiceJamesServer): Unit = {
+    val request =
+      """
+        |{
+        |   "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail", "urn:apache:james:params:jmap:mail:quota" ],
+        |   "methodCalls": [
+        |       [
+        |           "Mailbox/set",
+        |           {
+        |                "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+        |                "create": {
+        |                    "C42": {
+        |                      "name": "myMailbox"
+        |                    }
+        |                }
+        |           },
+        |    "c1"
+        |       ]
+        |   ]
+        |}
+        |""".stripMargin
+
+    val response: String =
+      `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",
+         |          "isSubscribed":true,
+         |          "myRights":{"mayAddItems":true,"mayCreateChild":true,"mayDelete":true,"mayReadItems":true,"mayRemoveItems":true,"mayRename":true,"maySetKeywords":true,"maySetSeen":true,"maySubmit":true},
+         |          "totalEmails":0,
+         |          "totalThreads":0,
+         |          "unreadEmails":0,
+         |          "unreadThreads":0,
+         |          "quotas": {
+         |            "#private&bob@domain.tld": {
+         |              "Storage": {
+         |                "used": 0
+         |              },
+         |              "Message": {
+         |                "used": 0
+         |              }
+         |            }
+         |          }
+         |        }
+         |      }
+         |    },
+         |    "c1"]]
+         |}""".stripMargin)
+  }
+
+  @Test
   def mailboxSetShouldCreateMailboxWhenNameAndParentId(server: GuiceJamesServer): Unit = {
     val mailboxId: MailboxId  = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB, "parentMailbox"))
     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 2140562..2f8a2a8 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
@@ -232,6 +232,10 @@ class Serializer @Inject() (mailboxIdFactory: MailboxId.Factory) {
   implicit def mailboxWrites(properties: Set[String]): Writes[Mailbox] = Json.writes[Mailbox]
     .transform((o: JsObject) => JsObject(o.fields.filter(entry => properties.contains(entry._1))))
 
+  implicit def mailboxCreationResponseWrites(properties: Set[String]): Writes[MailboxCreationResponse] =
+    Json.writes[MailboxCreationResponse]
+      .transform((o: JsObject) => JsObject(o.fields.filter(entry => properties.contains(entry._1))))
+
   private implicit val idsRead: Reads[Ids] = Json.valueReads[Ids]
   private implicit val propertiesRead: Reads[Properties] = Json.valueReads[Properties]
   private implicit val mailboxGetRequest: Reads[MailboxGetRequest] = Json.reads[MailboxGetRequest]
@@ -286,10 +290,7 @@ class Serializer @Inject() (mailboxIdFactory: MailboxId.Factory) {
 
   private implicit def mailboxGetResponseWrites(implicit mailboxWrites: Writes[Mailbox]): Writes[MailboxGetResponse] = Json.writes[MailboxGetResponse]
 
-  private implicit val mailboxSetResponseWrites: Writes[MailboxSetResponse] = Json.writes[MailboxSetResponse]
-
-
-  private implicit val mailboxSetCreationResponseWrites: Writes[MailboxCreationResponse] = Json.writes[MailboxCreationResponse]
+  private implicit def mailboxSetResponseWrites(implicit mailboxCreationResponseWrites: Writes[MailboxCreationResponse]): Writes[MailboxSetResponse] = Json.writes[MailboxSetResponse]
 
   private implicit val mailboxSetUpdateResponseWrites: Writes[MailboxUpdateResponse] = Json.valueWrites[MailboxUpdateResponse]
 
@@ -321,7 +322,7 @@ class Serializer @Inject() (mailboxIdFactory: MailboxId.Factory) {
       })
     }
 
-  private implicit def mailboxMapCreationResponseWrites: Writes[Map[MailboxCreationId, MailboxCreationResponse]] =
+  private implicit def mailboxMapCreationResponseWrites(implicit mailboxSetCreationResponseWrites: Writes[MailboxCreationResponse]): Writes[Map[MailboxCreationId, MailboxCreationResponse]] =
     (m: Map[MailboxCreationId, MailboxCreationResponse]) => {
       m.foldLeft(JsObject.empty)((jsObject, kv) => {
         val (mailboxCreationId: MailboxCreationId, mailboxCreationResponse: MailboxCreationResponse) = kv
@@ -357,6 +358,10 @@ class Serializer @Inject() (mailboxIdFactory: MailboxId.Factory) {
     mailboxWrites(Mailbox.propertiesFiltered(properties, capabilities))
   }
 
+  private def mailboxCreationResponseWritesWithFilteredProperties(capabilities: Set[CapabilityIdentifier]): Writes[MailboxCreationResponse] = {
+    mailboxCreationResponseWrites(MailboxCreationResponse.propertiesFiltered(capabilities))
+  }
+
   private implicit def jsErrorWrites: Writes[JsError] = Json.writes[JsError]
 
   private implicit val problemDetailsWrites: Writes[ProblemDetails] = Json.writes[ProblemDetails]
@@ -376,7 +381,12 @@ class Serializer @Inject() (mailboxIdFactory: MailboxId.Factory) {
   def serialize(mailboxGetResponse: MailboxGetResponse, properties: Option[Properties], capabilities: Set[CapabilityIdentifier]): JsValue =
     serialize(mailboxGetResponse)(mailboxWritesWithFilteredProperties(properties, capabilities))
 
-  def serialize(mailboxSetResponse: MailboxSetResponse): JsValue = Json.toJson(mailboxSetResponse)(mailboxSetResponseWrites)
+  def serialize(mailboxSetResponse: MailboxSetResponse)
+               (implicit mailboxCreationResponseWrites: Writes[MailboxCreationResponse]): JsValue =
+    Json.toJson(mailboxSetResponse)(mailboxSetResponseWrites(mailboxCreationResponseWrites))
+
+  def serialize(mailboxSetResponse: MailboxSetResponse, capabilities: Set[CapabilityIdentifier]): JsValue =
+    serialize(mailboxSetResponse)(mailboxCreationResponseWritesWithFilteredProperties(capabilities))
 
   def serialize(errors: JsError): JsValue = Json.toJson(errors)
 
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 739fa5c..4ae37ae 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
@@ -30,8 +30,9 @@ import org.apache.james.jmap.json.Serializer
 import org.apache.james.jmap.mail.MailboxName.MailboxName
 import org.apache.james.jmap.mail.MailboxPatchObject.MailboxPatchObjectKey
 import org.apache.james.jmap.mail.MailboxSetRequest.{MailboxCreationId, UnparsedMailboxId}
-import org.apache.james.jmap.model.AccountId
+import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier
 import org.apache.james.jmap.model.State.State
+import org.apache.james.jmap.model.{AccountId, CapabilityIdentifier}
 import org.apache.james.mailbox.Role
 import org.apache.james.mailbox.model.MailboxId
 import play.api.libs.json._
@@ -109,19 +110,33 @@ object MailboxSetError {
 
 case class MailboxSetError(`type`: SetErrorType, description: Option[SetErrorDescription], properties: Option[Properties])
 
+
+object MailboxCreationResponse {
+  def allProperties: Set[String] = Set("id", "role", "totalEmails", "unreadEmails",
+    "totalThreads", "unreadThreads", "myRights", "isSubscribed", "quotas")
+
+  def propertiesFiltered(allowedCapabilities : Set[CapabilityIdentifier]) : Set[String] = {
+    val propertiesForCapabilities: Map[CapabilityIdentifier, Set[String]] = Map(
+      CapabilityIdentifier.JAMES_QUOTA -> Set("quotas"))
+
+    val propertiesToHide = propertiesForCapabilities.filterNot(entry => allowedCapabilities.contains(entry._1))
+      .flatMap(_._2)
+      .toSet
+
+    allProperties -- propertiesToHide
+  }
+}
+
 case class MailboxCreationResponse(id: MailboxId,
-                                   role: Option[Role],//TODO see if we need to return this, if a role is set by the server during creation
+                                   role: Option[Role],
                                    sortOrder: SortOrder,
                                    totalEmails: TotalEmails,
                                    unreadEmails: UnreadEmails,
                                    totalThreads: TotalThreads,
                                    unreadThreads: UnreadThreads,
                                    myRights: MailboxRights,
-                                   rights: Option[Rights],//TODO display only if RightsExtension and if some rights are set by the server during creation
-                                   namespace: Option[MailboxNamespace], //TODO display only if RightsExtension
-                                   quotas: Option[Quotas],//TODO display only if QuotasExtension
-                                   isSubscribed: IsSubscribed
-                                  )
+                                   quotas: Option[Quotas],
+                                   isSubscribed: IsSubscribed)
 
 object MailboxSetResponse {
   def empty: MailboxUpdateResponse = MailboxUpdateResponse(JsObject(Map[String, JsValue]()))
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 b8c465a..fd76d3d 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
@@ -28,6 +28,7 @@ import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier
 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.jmap.utils.quotas.QuotaLoaderWithPreloadedDefaultFactory
 import org.apache.james.mailbox.MailboxManager.RenameOption
 import org.apache.james.mailbox.exception.{InsufficientRightsException, MailboxExistsException, MailboxNameException, MailboxNotFoundException}
 import org.apache.james.mailbox.model.{FetchGroup, MailboxId, MailboxPath, MessageRange}
@@ -48,7 +49,7 @@ case class MailboxCreationParseException(mailboxSetError: MailboxSetError) exten
 sealed trait CreationResult {
   def mailboxCreationId: MailboxCreationId
 }
-case class CreationSuccess(mailboxCreationId: MailboxCreationId, mailboxId: MailboxId) extends CreationResult
+case class CreationSuccess(mailboxCreationId: MailboxCreationId, mailboxCreationResponse: MailboxCreationResponse) 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"))))
@@ -62,25 +63,11 @@ case class CreationFailure(mailboxCreationId: MailboxCreationId, exception: Exce
 case class CreationResults(created: Seq[CreationResult]) {
   def retrieveCreated: Map[MailboxCreationId, MailboxCreationResponse] = created
     .flatMap(result => result match {
-      case success: CreationSuccess => Some(success.mailboxCreationId, success.mailboxId)
+      case success: CreationSuccess => Some(success.mailboxCreationId, success.mailboxCreationResponse)
       case _ => None
     })
     .toMap
-    .map(creation => (creation._1, toCreationResponse(creation._2)))
-
-  private def toCreationResponse(mailboxId: MailboxId): MailboxCreationResponse = MailboxCreationResponse(
-    id = mailboxId,
-    role = None,
-    sortOrder = SortOrder.defaultSortOrder,
-    totalEmails = TotalEmails(0L),
-    unreadEmails = UnreadEmails(0L),
-    totalThreads = TotalThreads(0L),
-    unreadThreads = UnreadThreads(0L),
-    myRights = MailboxRights.FULL,
-    rights = None,
-    namespace = None,
-    quotas = None,
-    isSubscribed = IsSubscribed(true))
+    .map(creation => (creation._1, creation._2))
 
   def retrieveErrors: Map[MailboxCreationId, MailboxSetError] = created
     .flatMap(result => result match {
@@ -148,6 +135,7 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
                                  mailboxManager: MailboxManager,
                                  subscriptionManager: SubscriptionManager,
                                  mailboxIdFactory: MailboxId.Factory,
+                                 quotaFactory : QuotaLoaderWithPreloadedDefaultFactory,
                                  metricFactory: MetricFactory) extends Method {
   override val methodName: MethodName = MethodName("Mailbox/set")
 
@@ -159,7 +147,7 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
             creationResults <- createMailboxes(mailboxSession, mailboxSetRequest, processingContext)
             deletionResults <- deleteMailboxes(mailboxSession, mailboxSetRequest, processingContext)
             updateResults <- updateMailboxes(mailboxSession, mailboxSetRequest, processingContext)
-          } yield createResponse(invocation, mailboxSetRequest, creationResults, deletionResults, updateResults)
+          } yield createResponse(capabilities, invocation, mailboxSetRequest, creationResults, deletionResults, updateResults)
         }))
   }
 
@@ -316,9 +304,9 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
           path = path,
           mailboxCreationRequest = mailboxCreationRequest)))
       .fold(e => CreationFailure(mailboxCreationId, e),
-        mailboxId => {
-          recordCreationIdInProcessingContext(mailboxCreationId, processingContext, mailboxId)
-          CreationSuccess(mailboxCreationId, mailboxId)
+        creationResponse => {
+          recordCreationIdInProcessingContext(mailboxCreationId, processingContext, creationResponse.id)
+          CreationSuccess(mailboxCreationId, creationResponse)
         })
   }
 
@@ -338,7 +326,7 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
 
   private def createMailbox(mailboxSession: MailboxSession,
                             path: MailboxPath,
-                            mailboxCreationRequest: MailboxCreationRequest): Either[Exception, MailboxId] = {
+                            mailboxCreationRequest: MailboxCreationRequest): Either[Exception, MailboxCreationResponse] = {
     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()
@@ -350,7 +338,21 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
       mailboxCreationRequest.rights
           .foreach(rights => mailboxManager.setRights(mailboxId, rights.toMailboxAcl.asJava, mailboxSession))
 
-      Right(mailboxId)
+      val quotas = quotaFactory.loadFor(mailboxSession)
+        .flatMap(quotaLoader => quotaLoader.getQuotas(path))
+        .block()
+
+      Right(MailboxCreationResponse(
+        id = mailboxId,
+        sortOrder = SortOrder.defaultSortOrder,
+        role = None,
+        totalEmails = TotalEmails(0L),
+        unreadEmails = UnreadEmails(0L),
+        totalThreads = TotalThreads(0L),
+        unreadThreads = UnreadThreads(0L),
+        myRights = MailboxRights.FULL,
+        quotas = Some(quotas),
+        isSubscribed = IsSubscribed(true)))
     } catch {
       case error: Exception => Left(error)
     }
@@ -386,7 +388,8 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
       case e: Exception => Left(e)
     }
 
-  private def createResponse(invocation: Invocation,
+  private def createResponse(capabilities: Set[CapabilityIdentifier],
+                             invocation: Invocation,
                              mailboxSetRequest: MailboxSetRequest,
                              creationResults: CreationResults,
                              deletionResults: DeletionResults,
@@ -403,7 +406,7 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
       notDestroyed = Some(deletionResults.retrieveErrors).filter(_.nonEmpty))
     
     Invocation(methodName,
-      Arguments(serializer.serialize(response).as[JsObject]),
+      Arguments(serializer.serialize(response, capabilities).as[JsObject]),
       invocation.methodCallId)
   }
 


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