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 <btell...@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-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org