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 rc...@apache.org on 2020/08/21 02:26:51 UTC
[james-project] 03/14: JAMES-3359 Mailbox/set update name support
This is an automated email from the ASF dual-hosted git repository.
rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 0b2d84224c31fb9ebc57eb7bec7193764a17b6d1
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Mon Aug 17 12:38:14 2020 +0700
JAMES-3359 Mailbox/set update name support
This only comprises basic support, error handling and advanced features needs to be implemented.
TODO update using creation ids
TODO invalid path handling (unknown property, invalid name)
TODO mailbox already exists, too long name
TODO disable destroy / rename of sustem mailbox
TODO test that renames keeps subscriptions
TODO renaming delegated mailboxes is not allowed
---
.../contract/MailboxSetMethodContract.scala | 90 ++++++++++++++++++++
.../org/apache/james/jmap/json/Serializer.scala | 2 +-
.../org/apache/james/jmap/mail/MailboxSet.scala | 31 ++++++-
.../james/jmap/method/MailboxSetMethod.scala | 95 ++++++++++++++++++++--
4 files changed, 208 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 f8d8a96..b1853b9 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
@@ -1993,4 +1993,94 @@ trait MailboxSetMethodContract {
| ]
|}""".stripMargin)
}
+
+ @Test
+ def updateShouldRenameMailboxes(server: GuiceJamesServer): Unit = {
+ val mailboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB, "mailbox1"))
+
+ val request =
+ s"""
+ |{
+ | "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ],
+ | "methodCalls": [
+ | ["Mailbox/set",
+ | {
+ | "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "update": {
+ | "${mailboxId.serialize()}" : {
+ | "/name": "newName"
+ | }
+ | }
+ | },
+ | "c2"],
+ | ["Mailbox/get",
+ | {
+ | "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "ids": ["${mailboxId.serialize()}"]
+ | },
+ | "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",
+ | "updated": {
+ | "1": {}
+ | }
+ | }, "c2"],
+ | ["Mailbox/get", {
+ | "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "state": "000001",
+ | "list": [{
+ | "id": "1",
+ | "name": "newName",
+ | "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)
+ }
+
+ // TODO update using creation ids
+ // TODO invalid path handling (unknown property, invalid name)
+ // TODO mailbox already exists, too long name
+ // TODO disable destroy / rename of sustem mailbox
+ // TODO test that renames keeps subscriptions
+ // TODO renaming delegated mailboxes is not allowed
}
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 131bb41..b655527 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
@@ -276,7 +276,7 @@ class Serializer @Inject() (mailboxIdFactory: MailboxId.Factory) {
private implicit val mailboxSetCreationResponseWrites: Writes[MailboxCreationResponse] = Json.writes[MailboxCreationResponse]
- private implicit val mailboxSetUpdateResponseWrites: Writes[MailboxUpdateResponse] = Json.writes[MailboxUpdateResponse]
+ private implicit val mailboxSetUpdateResponseWrites: Writes[MailboxUpdateResponse] = Json.valueWrites[MailboxUpdateResponse]
private implicit val propertiesWrites: Writes[Properties] = Json.writes[Properties]
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 f50e5ff..51821dd 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
@@ -29,7 +29,7 @@ import org.apache.james.jmap.model.AccountId
import org.apache.james.jmap.model.State.State
import org.apache.james.mailbox.Role
import org.apache.james.mailbox.model.MailboxId
-import play.api.libs.json.JsObject
+import play.api.libs.json.{JsObject, JsString, JsValue}
case class MailboxSetRequest(accountId: AccountId,
ifInState: Option[State],
@@ -55,7 +55,14 @@ case class MailboxCreationRequest(name: MailboxName,
isSubscribed: Option[IsSubscribed],
rights: Option[Rights])
-case class MailboxPatchObject(value: Map[String, JsObject])
+case class MailboxPatchObject(value: Map[String, JsValue]) {
+ def updates: Iterable[Either[PatchUpdateValidationException, Update]] = value.map({
+ case (property, newValue) => property match {
+ case "/name" => NameUpdate.parse(newValue)
+ case property => Left(UnsupportedPropertyUpdated(property))
+ }
+ })
+}
case class MailboxSetResponse(accountId: AccountId,
oldState: Option[State],
@@ -98,4 +105,22 @@ case class MailboxCreationResponse(id: MailboxId,
isSubscribed: IsSubscribed
)
-case class MailboxUpdateResponse(value: JsObject)
\ No newline at end of file
+object MailboxSetResponse {
+ def empty: MailboxUpdateResponse = MailboxUpdateResponse(JsObject(Map[String, JsValue]()))
+}
+
+case class MailboxUpdateResponse(value: JsObject)
+
+object NameUpdate {
+ def parse(newValue: JsValue): Either[PatchUpdateValidationException, Update] = newValue match {
+ case JsString(newName) => scala.Right(NameUpdate(newName))
+ case _ => Left(InvalidUpdate("name", "Expectint a JSON string as an argument"))
+ }
+}
+
+sealed trait Update
+case class NameUpdate(newName: String) extends Update
+
+class PatchUpdateValidationException() extends IllegalArgumentException
+case class UnsupportedPropertyUpdated(property: String) extends PatchUpdateValidationException
+case class InvalidUpdate(property: String, cause: String) extends PatchUpdateValidationException
\ No newline at end of file
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 6ac4d5a..81fc67d 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
@@ -23,20 +23,23 @@ import eu.timepit.refined.auto._
import javax.inject.Inject
import org.apache.james.jmap.json.Serializer
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.mail.{IsSubscribed, MailboxCreationRequest, MailboxCreationResponse, MailboxPatchObject, MailboxRights, MailboxSetError, MailboxSetRequest, MailboxSetResponse, MailboxUpdateResponse, NameUpdate, PatchUpdateValidationException, 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.{ClientId, Id, Invocation, ServerId, State}
import org.apache.james.jmap.routes.ProcessingContext
+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}
-import org.apache.james.mailbox.{MailboxManager, MailboxSession, SubscriptionManager}
+import org.apache.james.mailbox.{MailboxManager, MailboxSession, MessageManager, SubscriptionManager}
import org.apache.james.metrics.api.MetricFactory
import org.reactivestreams.Publisher
import play.api.libs.json._
import reactor.core.scala.publisher.{SFlux, SMono}
import reactor.core.scheduler.Schedulers
+import scala.jdk.CollectionConverters._
+
case class MailboxHasMailException(mailboxId: MailboxId) extends Exception
case class MailboxHasChildException(mailboxId: MailboxId) extends Exception
case class MailboxCreationParseException(mailboxSetError: MailboxSetError) extends Exception
@@ -111,6 +114,25 @@ case class DeletionResults(results: Seq[DeletionResult]) {
.toMap
}
+sealed trait UpdateResult
+case class UpdateSuccess(mailboxId: MailboxId) extends UpdateResult
+case class UpdateFailure(mailboxId: MailboxId, exception: Throwable) extends UpdateResult {
+ def asMailboxSetError: MailboxSetError = exception match {
+ case _ => MailboxSetError.serverFail(Some(SetErrorDescription(exception.getMessage)), None)
+ }
+}
+case class UpdateResults(results: Seq[UpdateResult]) {
+ def updated: Map[MailboxId, MailboxUpdateResponse] =
+ results.flatMap(result => result match {
+ case success: UpdateSuccess => Some((success.mailboxId, MailboxSetResponse.empty))
+ case _ => None
+ }).toMap
+ def notUpdated: Map[MailboxId, MailboxSetError] = results.flatMap(result => result match {
+ case failure: UpdateFailure => Some(failure.mailboxId, failure.asMailboxSetError)
+ case _ => None
+ }).toMap
+}
+
class MailboxSetMethod @Inject()(serializer: Serializer,
mailboxManager: MailboxManager,
subscriptionManager: SubscriptionManager,
@@ -125,10 +147,70 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
for {
creationResults <- createMailboxes(mailboxSession, mailboxSetRequest, processingContext)
deletionResults <- deleteMailboxes(mailboxSession, mailboxSetRequest, processingContext)
- } yield createResponse(invocation, mailboxSetRequest, creationResults, deletionResults)
+ updateResults <- updateMailboxes(mailboxSession, mailboxSetRequest, processingContext)
+ } yield createResponse(invocation, mailboxSetRequest, creationResults, deletionResults, updateResults)
}))
}
+ private def updateMailboxes(mailboxSession: MailboxSession,
+ mailboxSetRequest: MailboxSetRequest,
+ processingContext: ProcessingContext): SMono[UpdateResults] = {
+ SFlux.fromIterable(mailboxSetRequest.update.getOrElse(Seq()))
+ .flatMap({
+ case (mailboxId: MailboxId, patch: MailboxPatchObject) => updateMailbox(mailboxSession, mailboxId, patch)
+ .onErrorResume(e => SMono.just(UpdateFailure(mailboxId, e)))
+ })
+ .collectSeq()
+ .map(UpdateResults)
+ }
+
+ private def updateMailbox(mailboxSession: MailboxSession,
+ maiboxId: MailboxId,
+ patch: MailboxPatchObject): SMono[UpdateResult] = {
+ val maybeParseException: Option[PatchUpdateValidationException] = patch.updates
+ .flatMap(x => x match {
+ case Left(e) => Some(e)
+ case _ => None
+ }).headOption
+
+ val maybeNameUpdate: Option[NameUpdate] = patch.updates
+ .flatMap(x => x match {
+ case Right(NameUpdate(newName)) => Some(NameUpdate(newName))
+ case _ => None
+ }).headOption
+
+ def renameMailbox: SMono[UpdateResult] = updateMailboxPath(maiboxId, maybeNameUpdate, mailboxSession)
+
+ maybeParseException.map(e => SMono.just[UpdateResult](UpdateFailure(maiboxId, e)))
+ .getOrElse(renameMailbox)
+ }
+
+ private def updateMailboxPath(maiboxId: MailboxId, maybeNameUpdate: Option[NameUpdate], mailboxSession: MailboxSession): SMono[UpdateResult] = {
+ maybeNameUpdate.map(nameUpdate => {
+ SMono.fromCallable(() => {
+ val mailbox = mailboxManager.getMailbox(maiboxId, mailboxSession)
+ mailboxManager.renameMailbox(maiboxId,
+ computeMailboxPath(mailbox, nameUpdate, mailboxSession),
+ RenameOption.RENAME_SUBSCRIPTIONS,
+ mailboxSession)
+ }).`then`(SMono.just[UpdateResult](UpdateSuccess(maiboxId)))
+ .subscribeOn(Schedulers.elastic())
+ })
+ // No updated properties passed. Noop.
+ .getOrElse(SMono.just[UpdateResult](UpdateSuccess(maiboxId)))
+ }
+
+ private def computeMailboxPath(mailbox: MessageManager, nameUpdate: NameUpdate, mailboxSession: MailboxSession): MailboxPath = {
+ val originalPath: MailboxPath = mailbox.getMailboxPath
+ val maybeParentPath: Option[MailboxPath] = originalPath.getHierarchyLevels(mailboxSession.getPathDelimiter)
+ .asScala
+ .reverse
+ .drop(1)
+ .headOption
+ maybeParentPath.map(_.child(nameUpdate.newName, mailboxSession.getPathDelimiter))
+ .getOrElse(MailboxPath.forUser(mailboxSession.getUser, nameUpdate.newName))
+ }
+
private def deleteMailboxes(mailboxSession: MailboxSession, mailboxSetRequest: MailboxSetRequest, processingContext: ProcessingContext): SMono[DeletionResults] = {
SFlux.fromIterable(mailboxSetRequest.destroy.getOrElse(Seq()))
.flatMap(id => delete(mailboxSession, processingContext, id)
@@ -257,7 +339,8 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
private def createResponse(invocation: Invocation,
mailboxSetRequest: MailboxSetRequest,
creationResults: CreationResults,
- deletionResults: DeletionResults): Invocation = {
+ deletionResults: DeletionResults,
+ updateResults: UpdateResults): Invocation = {
val response = MailboxSetResponse(
mailboxSetRequest.accountId,
oldState = None,
@@ -265,8 +348,8 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
destroyed = Some(deletionResults.destroyed).filter(_.nonEmpty),
created = Some(creationResults.retrieveCreated).filter(_.nonEmpty),
notCreated = Some(creationResults.retrieveErrors).filter(_.nonEmpty),
- updated = None,
- notUpdated = None,
+ updated = Some(updateResults.updated).filter(_.nonEmpty),
+ notUpdated = Some(updateResults.notUpdated).filter(_.nonEmpty),
notDestroyed = Some(deletionResults.retrieveErrors).filter(_.nonEmpty))
Invocation(methodName,
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org