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 08e3d1dec0f1ae74b3621d3aea9b174db1661596 Author: Benoit Tellier <[email protected]> AuthorDate: Tue Oct 6 11:28:34 2020 +0700 JAMES-3404 Implicitly handle client id resolution with JSON substitution This empowers implicit handling, making it easier to implement client Id resolution --- .../contract/MailboxSetMethodContract.scala | 10 ++--- .../apache/james/jmap/json/MailboxSerializer.scala | 3 +- .../james/jmap/json/VacationSerializer.scala | 14 ++++--- .../org/apache/james/jmap/mail/MailboxGet.scala | 28 ++++++++++++- .../org/apache/james/jmap/mail/MailboxSet.scala | 30 +++++--------- .../apache/james/jmap/mail/VacationResponse.scala | 1 + .../james/jmap/method/MailboxGetMethod.scala | 15 ++++--- .../james/jmap/method/MailboxSetMethod.scala | 46 ++++++++++++---------- .../jmap/method/VacationResponseGetMethod.scala | 13 +++--- .../james/jmap/routes/ProcessingContext.scala | 45 +++++++++------------ .../jmap/json/MailboxGetSerializationTest.scala | 18 +-------- .../VacationResponseGetSerializationTest.scala | 3 -- 12 files changed, 109 insertions(+), 117 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 413aadb..c81aef5 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 @@ -2807,7 +2807,7 @@ trait MailboxSetMethodContract { | "notDestroyed": { | "#C42": { | "type": "invalidArguments", - | "description": "#C42 is not a mailboxId: ClientId(#C42) was not used in previously defined creationIds" + | "description": "#C42 is not a mailboxId: #C42 was not used in previously defined creationIds" | } | } | }, "c2"], @@ -2881,7 +2881,7 @@ trait MailboxSetMethodContract { | "notDestroyed": { | "#C42": { | "type": "invalidArguments", - | "description": "#C42 is not a mailboxId: ClientId(#C42) was not used in previously defined creationIds" + | "description": "#C42 is not a mailboxId: #C42 was not used in previously defined creationIds" | } | } | }, "c2"] @@ -2919,7 +2919,7 @@ trait MailboxSetMethodContract { .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." + val message = "# is not a mailboxId: # was not used in previously defined creationIds" assertThatJson(response).isEqualTo( s"""{ | "sessionState": "75128aab4b1b", @@ -6728,7 +6728,7 @@ trait MailboxSetMethodContract { | "notUpdated": { | "${mailboxId.serialize}": { | "type": "invalidArguments", - | "description": "ClientId(#C42) was not used in previously defined creationIds", + | "description": "#C42 was not used in previously defined creationIds", | "properties": ["parentId"] | } | } @@ -7357,7 +7357,7 @@ trait MailboxSetMethodContract { | "notUpdated": { | "#invalid": { | "type": "invalidArguments", - | "description": "ClientId(#invalid) was not used in previously defined creationIds" + | "description": "#invalid was not used in previously defined creationIds" | } | } | }, diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/MailboxSerializer.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/MailboxSerializer.scala index d8e4467..edb6d11 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/MailboxSerializer.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/MailboxSerializer.scala @@ -23,7 +23,8 @@ import eu.timepit.refined._ import eu.timepit.refined.collection.NonEmpty import javax.inject.Inject import org.apache.james.core.{Domain, Username} -import org.apache.james.jmap.mail.MailboxSetRequest.{MailboxCreationId, UnparsedMailboxId, UnparsedMailboxIdConstraint} +import org.apache.james.jmap.mail.MailboxGet.{UnparsedMailboxId, UnparsedMailboxIdConstraint} +import org.apache.james.jmap.mail.MailboxSetRequest.MailboxCreationId import org.apache.james.jmap.mail.{DelegatedNamespace, Ids, IsSubscribed, Mailbox, MailboxCreationRequest, MailboxCreationResponse, MailboxGetRequest, MailboxGetResponse, MailboxNamespace, MailboxPatchObject, MailboxRights, MailboxSetRequest, MailboxSetResponse, MailboxUpdateResponse, MayAddItems, MayCreateChild, MayDelete, MayReadItems, MayRemoveItems, MayRename, MaySetKeywords, MaySetSeen, MaySubmit, NotFound, PersonalNamespace, Quota, QuotaId, QuotaRoot, Quotas, RemoveEmailsOnDestroy, [...] import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier import org.apache.james.jmap.model._ diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/VacationSerializer.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/VacationSerializer.scala index 70f61b6..38c830c 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/VacationSerializer.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/VacationSerializer.scala @@ -21,9 +21,8 @@ package org.apache.james.jmap.json import java.time.format.DateTimeFormatter -import org.apache.james.jmap.mail.MailboxSetRequest.UnparsedMailboxId import org.apache.james.jmap.mail.VacationResponse.{UnparsedVacationResponseId, VACATION_RESPONSE_ID} -import org.apache.james.jmap.mail.{FromDate, HtmlBody, IsEnabled, NotFound, Subject, TextBody, ToDate, VacationResponse, VacationResponseGetRequest, VacationResponseGetResponse, VacationResponseId, VacationResponseIds, VacationResponseNotFound} +import org.apache.james.jmap.mail.{FromDate, HtmlBody, IsEnabled, Subject, TextBody, ToDate, VacationResponse, VacationResponseGetRequest, VacationResponseGetResponse, VacationResponseId, VacationResponseIds, VacationResponseNotFound} import org.apache.james.jmap.model._ import org.apache.james.jmap.vacation.{VacationResponsePatchObject, VacationResponseSetError, VacationResponseSetRequest, VacationResponseSetResponse, VacationResponseUpdateResponse} import play.api.libs.json._ @@ -38,9 +37,6 @@ object VacationSerializer { } private implicit val vacationResponseSetRequestReads: Reads[VacationResponseSetRequest] = Json.reads[VacationResponseSetRequest] - private implicit def notFoundWrites(implicit mailboxIdWrites: Writes[UnparsedMailboxId]): Writes[NotFound] = - notFound => JsArray(notFound.value.toList.map(mailboxIdWrites.writes)) - private implicit val vacationResponseSetUpdateResponseWrites: Writes[VacationResponseUpdateResponse] = Json.valueWrites[VacationResponseUpdateResponse] private implicit val vacationResponseSetErrorWrites: Writes[VacationResponseSetError] = Json.writes[VacationResponseSetError] @@ -51,6 +47,11 @@ object VacationSerializer { utcDate => JsString(utcDate.asUTC.format(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ssX"))) private implicit val vacationResponseIdWrites: Writes[VacationResponseId] = _ => JsString(VACATION_RESPONSE_ID.value) + private implicit val vacationResponseIdReads: Reads[VacationResponseId] = { + case JsString("singleton") => JsSuccess(VacationResponseId()) + case JsString(_) => JsError("Only singleton is supported as a VacationResponseId") + case _ => JsError("Expecting JsString(singleton) to represent a VacationResponseId") + } private implicit val isEnabledWrites: Writes[IsEnabled] = Json.valueWrites[IsEnabled] private implicit val fromDateWrites: Writes[FromDate] = Json.valueWrites[FromDate] private implicit val toDateWrites: Writes[ToDate] = Json.valueWrites[ToDate] @@ -61,7 +62,8 @@ object VacationSerializer { implicit def vacationResponseWrites(properties: Properties): Writes[VacationResponse] = Json.writes[VacationResponse] .transform(properties.filter(_)) - private implicit val vacationResponseIdReads: Reads[VacationResponseIds] = Json.valueReads[VacationResponseIds] + private implicit val vacationResponseIdsReads: Reads[VacationResponseIds] = Json.valueReads[VacationResponseIds] + private implicit val vacationResponseGetRequest: Reads[VacationResponseGetRequest] = Json.reads[VacationResponseGetRequest] private implicit def vacationResponseNotFoundWrites(implicit idWrites: Writes[UnparsedVacationResponseId]): Writes[VacationResponseNotFound] = 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 b230a77..bd7b8ba 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 @@ -19,10 +19,36 @@ package org.apache.james.jmap.mail -import org.apache.james.jmap.mail.MailboxSetRequest.UnparsedMailboxId +import eu.timepit.refined +import eu.timepit.refined.api.Refined +import eu.timepit.refined.collection.NonEmpty +import org.apache.james.jmap.mail.MailboxGet.UnparsedMailboxId import org.apache.james.jmap.method.WithAccountId import org.apache.james.jmap.model.State.State import org.apache.james.jmap.model.{AccountId, Properties} +import org.apache.james.mailbox.model.MailboxId + +import scala.util.{Failure, Try} + +object MailboxGet { + type UnparsedMailboxIdConstraint = 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 + } + + def parse(mailboxIdFactory: MailboxId.Factory)(unparsed: UnparsedMailboxId): Try[MailboxId] = + parseString(mailboxIdFactory)(unparsed.value) + + def parseString(mailboxIdFactory: MailboxId.Factory)(unparsed: String): Try[MailboxId] = + unparsed match { + case a if a.startsWith("#") => + Failure(new IllegalArgumentException(s"$unparsed was not used in previously defined creationIds")) + case _ => Try(mailboxIdFactory.fromString(unparsed)) + } +} case class Ids(value: List[UnparsedMailboxId]) 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 cbd034e..20801bd 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,7 +19,6 @@ 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 @@ -27,15 +26,15 @@ import eu.timepit.refined.refineV import eu.timepit.refined.types.string.NonEmptyString import org.apache.james.core.Username import org.apache.james.jmap.json.MailboxSerializer +import org.apache.james.jmap.mail.MailboxGet.UnparsedMailboxId 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, UnparsedMailboxIdConstraint} +import org.apache.james.jmap.mail.MailboxSetRequest.MailboxCreationId import org.apache.james.jmap.method.{MailboxCreationParseException, WithAccountId} import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier import org.apache.james.jmap.model.SetError.{SetErrorDescription, SetErrorType} import org.apache.james.jmap.model.State.State import org.apache.james.jmap.model.{AccountId, CapabilityIdentifier, Properties, SetError} -import org.apache.james.jmap.routes.ProcessingContext import org.apache.james.mailbox.model.{MailboxId, MailboxACL => JavaMailboxACL} import org.apache.james.mailbox.{MailboxSession, Role} import play.api.libs.json.{JsBoolean, JsError, JsNull, JsObject, JsString, JsSuccess, JsValue} @@ -48,14 +47,7 @@ case class MailboxSetRequest(accountId: AccountId, onDestroyRemoveEmails: Option[RemoveEmailsOnDestroy]) extends WithAccountId object MailboxSetRequest { - type UnparsedMailboxIdConstraint = NonEmpty type MailboxCreationId = 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 @@ -114,12 +106,11 @@ object MailboxPatchObject { } case class MailboxPatchObject(value: Map[String, JsValue]) { - def validate(processingContext: ProcessingContext, - mailboxIdFactory: MailboxId.Factory, + def validate(mailboxIdFactory: MailboxId.Factory, serializer: MailboxSerializer, capabilities: Set[CapabilityIdentifier], mailboxSession: MailboxSession): Either[PatchUpdateValidationException, ValidatedMailboxPatchObject] = { - val asUpdatedIterable = updates(serializer, capabilities, processingContext, mailboxIdFactory, mailboxSession) + val asUpdatedIterable = updates(serializer, capabilities, mailboxIdFactory, mailboxSession) val maybeParseException: Option[PatchUpdateValidationException] = asUpdatedIterable .flatMap(x => x match { @@ -174,12 +165,11 @@ case class MailboxPatchObject(value: Map[String, JsValue]) { def updates(serializer: MailboxSerializer, capabilities: Set[CapabilityIdentifier], - processingContext: ProcessingContext, mailboxIdFactory: MailboxId.Factory, mailboxSession: MailboxSession): Iterable[Either[PatchUpdateValidationException, Update]] = value.map({ case (property, newValue) => property match { case "name" => NameUpdate.parse(newValue, mailboxSession) - case "parentId" => ParentIdUpdate.parse(newValue, processingContext, mailboxIdFactory) + case "parentId" => ParentIdUpdate.parse(newValue, mailboxIdFactory) case "sharedWith" => SharedWithResetUpdate.parse(serializer, capabilities)(newValue) case "role" => Left(ServerSetPropertyException(MailboxPatchObject.roleProperty)) case "sortOrder" => Left(ServerSetPropertyException(MailboxPatchObject.sortOrderProperty)) @@ -331,15 +321,13 @@ object SharedWithPartialUpdate { } object ParentIdUpdate { - def parse(newValue: JsValue, processingContext: ProcessingContext, mailboxIdFactory: MailboxId.Factory): + def parse(newValue: JsValue, mailboxIdFactory: MailboxId.Factory): Either[PatchUpdateValidationException, Update] = (newValue match { case JsString(id) => - for { - unparsedMailboxId <- refineV[UnparsedMailboxIdConstraint](id) - mailboxId <- processingContext.resolveMailboxId(unparsedMailboxId, mailboxIdFactory).left.map(_.getMessage) - } yield changeParentTo(mailboxId) - + MailboxGet.parseString(mailboxIdFactory)(id) + .fold(e => Left(e.getMessage), + mailboxId => scala.Right(changeParentTo(mailboxId))) case JsNull => scala.Right(removeParent()) case _ => Left("Expecting a JSON string or null as an argument") diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/VacationResponse.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/VacationResponse.scala index db02ec3..d25c603 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/VacationResponse.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/VacationResponse.scala @@ -38,6 +38,7 @@ case class HtmlBody(value: String) object VacationResponse { val VACATION_RESPONSE_ID: Id = "singleton" + val UNPARSED_SINGLETON: UnparsedVacationResponseId = "singleton" type UnparsedVacationResponseId = String Refined NonEmpty 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 15c01c9..154da66 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 @@ -23,14 +23,13 @@ import eu.timepit.refined.auto._ import javax.inject.Inject import org.apache.james.jmap.http.SessionSupplier import org.apache.james.jmap.json.{MailboxSerializer, ResponseSerializer} -import org.apache.james.jmap.mail.MailboxSetRequest.UnparsedMailboxId +import org.apache.james.jmap.mail.MailboxGet.UnparsedMailboxId import org.apache.james.jmap.mail._ import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier import org.apache.james.jmap.model.DefaultCapabilities.{CORE_CAPABILITY, MAIL_CAPABILITY} import org.apache.james.jmap.model.Invocation.{Arguments, MethodName} import org.apache.james.jmap.model.State.INSTANCE import org.apache.james.jmap.model.{AccountId, Capabilities, CapabilityIdentifier, ErrorCode, Invocation, MailboxFactory, Properties, Subscriptions} -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 @@ -42,6 +41,7 @@ import reactor.core.scala.publisher.{SFlux, SMono} import reactor.core.scheduler.Schedulers import scala.jdk.CollectionConverters._ +import scala.util.Try class MailboxGetMethod @Inject() (serializer: MailboxSerializer, mailboxManager: MailboxManager, @@ -59,7 +59,7 @@ class MailboxGetMethod @Inject() (serializer: MailboxSerializer, def empty(): MailboxGetResults = MailboxGetResults(Set.empty, NotFound(Set.empty)) def found(mailbox: Mailbox): MailboxGetResults = MailboxGetResults(Set(mailbox), NotFound(Set.empty)) def notFound(mailboxId: UnparsedMailboxId): MailboxGetResults = MailboxGetResults(Set.empty, NotFound(Set(mailboxId))) - def notFound(mailboxId: MailboxId): MailboxGetResults = MailboxGetResults(Set.empty, NotFound(Set(MailboxSetRequest.asUnparsed(mailboxId)))) + def notFound(mailboxId: MailboxId): MailboxGetResults = MailboxGetResults(Set.empty, NotFound(Set(MailboxGet.asUnparsed(mailboxId)))) } case class MailboxGetResults(mailboxes: Set[Mailbox], notFound: NotFound) { @@ -75,7 +75,7 @@ class MailboxGetMethod @Inject() (serializer: MailboxSerializer, override def doProcess(capabilities: Set[CapabilityIdentifier], invocation: InvocationWithContext, mailboxSession: MailboxSession, request: MailboxGetRequest): SMono[InvocationWithContext] = { val requestedProperties: Properties = request.properties.getOrElse(Mailbox.allProperties) (requestedProperties -- Mailbox.allProperties match { - case invalidProperties if invalidProperties.isEmpty() => getMailboxes(capabilities, request, invocation.processingContext, mailboxSession) + case invalidProperties if invalidProperties.isEmpty() => getMailboxes(capabilities, request, mailboxSession) .reduce(MailboxGetResults.empty(), MailboxGetResults.merge) .map(mailboxes => mailboxes.asResponse(request.accountId)) .map(mailboxGetResponse => Invocation( @@ -101,16 +101,15 @@ class MailboxGetMethod @Inject() (serializer: MailboxSerializer, 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 => processingContext.resolveMailboxId(id, mailboxIdFactory) - .fold(e => SMono.just(MailboxGetResults.notFound(id)), - mailboxId => getMailboxResultById(capabilities, mailboxId, mailboxSession))) + .flatMap(id => Try(mailboxIdFactory.fromString(id.value)) + .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/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 5c9a50a..6a40581 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,8 +23,9 @@ import eu.timepit.refined.auto._ import javax.inject.Inject import org.apache.james.jmap.http.SessionSupplier import org.apache.james.jmap.json.{MailboxSerializer, ResponseSerializer} -import org.apache.james.jmap.mail.MailboxSetRequest.{MailboxCreationId, UnparsedMailboxId} -import org.apache.james.jmap.mail.{InvalidPatchException, InvalidPropertyException, InvalidUpdateException, IsSubscribed, MailboxCreationRequest, MailboxCreationResponse, MailboxPatchObject, MailboxRights, MailboxSetError, MailboxSetRequest, MailboxSetResponse, MailboxUpdateResponse, NameUpdate, ParentIdUpdate, RemoveEmailsOnDestroy, ServerSetPropertyException, SortOrder, TotalEmails, TotalThreads, UnreadEmails, UnreadThreads, UnsupportedPropertyUpdatedException, ValidatedMailboxPatchObject} +import org.apache.james.jmap.mail.MailboxGet.UnparsedMailboxId +import org.apache.james.jmap.mail.MailboxSetRequest.MailboxCreationId +import org.apache.james.jmap.mail.{InvalidPatchException, InvalidPropertyException, InvalidUpdateException, IsSubscribed, MailboxCreationRequest, MailboxCreationResponse, MailboxGet, MailboxPatchObject, MailboxRights, MailboxSetError, MailboxSetRequest, MailboxSetResponse, MailboxUpdateResponse, NameUpdate, ParentIdUpdate, RemoveEmailsOnDestroy, ServerSetPropertyException, SortOrder, TotalEmails, TotalThreads, UnreadEmails, UnreadThreads, UnsupportedPropertyUpdatedException, ValidatedMai [...] import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier import org.apache.james.jmap.model.DefaultCapabilities.{CORE_CAPABILITY, MAIL_CAPABILITY} import org.apache.james.jmap.model.Invocation.{Arguments, MethodName} @@ -42,6 +43,7 @@ import reactor.core.scala.publisher.{SFlux, SMono} import reactor.core.scheduler.Schedulers import scala.jdk.CollectionConverters._ +import scala.util.Try case class MailboxHasMailException(mailboxId: MailboxId) extends Exception case class SystemMailboxChangeException(mailboxId: MailboxId) extends Exception @@ -155,7 +157,7 @@ class MailboxSetMethod @Inject()(serializer: MailboxSerializer, override def doProcess(capabilities: Set[CapabilityIdentifier], invocation: InvocationWithContext, mailboxSession: MailboxSession, request: MailboxSetRequest): SMono[InvocationWithContext] = for { creationResultsWithUpdatedProcessingContext <- createMailboxes(mailboxSession, request, invocation.processingContext) - deletionResults <- deleteMailboxes(mailboxSession, request, invocation.processingContext) + deletionResults <- deleteMailboxes(mailboxSession, request) updateResults <- updateMailboxes(mailboxSession, request, invocation.processingContext, capabilities) } yield InvocationWithContext(createResponse(capabilities, invocation.invocation, request, creationResultsWithUpdatedProcessingContext._1, deletionResults, updateResults), creationResultsWithUpdatedProcessingContext._2) @@ -168,9 +170,10 @@ class MailboxSetMethod @Inject()(serializer: MailboxSerializer, SFlux.fromIterable(mailboxSetRequest.update.getOrElse(Seq())) .flatMap({ case (unparsedMailboxId: UnparsedMailboxId, patch: MailboxPatchObject) => - processingContext.resolveMailboxId(unparsedMailboxId, mailboxIdFactory).fold( + MailboxGet.parse(mailboxIdFactory)(unparsedMailboxId) + .fold( e => SMono.just(UpdateFailure(unparsedMailboxId, e, None)), - mailboxId => updateMailbox(mailboxSession, processingContext, mailboxId, unparsedMailboxId, patch, capabilities)) + mailboxId => updateMailbox(mailboxSession, mailboxId, unparsedMailboxId, patch, capabilities)) .onErrorResume(e => SMono.just(UpdateFailure(unparsedMailboxId, e, None))) }) .collectSeq() @@ -178,12 +181,11 @@ class MailboxSetMethod @Inject()(serializer: MailboxSerializer, } private def updateMailbox(mailboxSession: MailboxSession, - processingContext: ProcessingContext, mailboxId: MailboxId, unparsedMailboxId: UnparsedMailboxId, patch: MailboxPatchObject, capabilities: Set[CapabilityIdentifier]): SMono[UpdateResult] = { - patch.validate(processingContext, mailboxIdFactory, serializer, capabilities, mailboxSession) + patch.validate(mailboxIdFactory, serializer, capabilities, mailboxSession) .fold(e => SMono.raiseError(e), validatedPatch => updateMailboxRights(mailboxId, validatedPatch, mailboxSession) .`then`(updateSubscription(mailboxId, validatedPatch, mailboxSession)) @@ -299,24 +301,24 @@ class MailboxSetMethod @Inject()(serializer: MailboxSerializer, } - private def deleteMailboxes(mailboxSession: MailboxSession, mailboxSetRequest: MailboxSetRequest, processingContext: ProcessingContext): SMono[DeletionResults] = { + private def deleteMailboxes(mailboxSession: MailboxSession, mailboxSetRequest: MailboxSetRequest): SMono[DeletionResults] = { SFlux.fromIterable(mailboxSetRequest.destroy.getOrElse(Seq())) - .flatMap(id => delete(mailboxSession, processingContext, id, mailboxSetRequest.onDestroyRemoveEmails.getOrElse(RemoveEmailsOnDestroy(false))) + .flatMap(id => delete(mailboxSession, id, mailboxSetRequest.onDestroyRemoveEmails.getOrElse(RemoveEmailsOnDestroy(false))) .onErrorRecover(e => DeletionFailure(id, e))) .collectSeq() .map(DeletionResults) } - private def delete(mailboxSession: MailboxSession, processingContext: ProcessingContext, id: UnparsedMailboxId, onDestroy: RemoveEmailsOnDestroy): SMono[DeletionResult] = { - processingContext.resolveMailboxId(id, mailboxIdFactory) match { - case Right(mailboxId) => SMono.fromCallable(() => delete(mailboxSession, mailboxId, onDestroy)) - .subscribeOn(Schedulers.elastic()) - .`then`(SMono.just[DeletionResult](DeletionSuccess(mailboxId))) - case Left(e) => SMono.raiseError(e) - } + private def delete(mailboxSession: MailboxSession, id: UnparsedMailboxId, onDestroy: RemoveEmailsOnDestroy): SMono[DeletionResult] = { + MailboxGet.parse(mailboxIdFactory)(id) + .fold(e => SMono.raiseError(e), + id => SMono.fromCallable(() => doDelete(mailboxSession, id, onDestroy)) + .subscribeOn(Schedulers.elastic()) + .`then`(SMono.just[DeletionResult](DeletionSuccess(id)))) + } - private def delete(mailboxSession: MailboxSession, id: MailboxId, onDestroy: RemoveEmailsOnDestroy): Unit = { + private def doDelete(mailboxSession: MailboxSession, id: MailboxId, onDestroy: RemoveEmailsOnDestroy): Unit = { val mailbox = mailboxManager.getMailbox(id, mailboxSession) if (isASystemMailbox(mailbox)) { @@ -363,7 +365,7 @@ class MailboxSetMethod @Inject()(serializer: MailboxSerializer, jsObject: JsObject, processingContext: ProcessingContext): (CreationResult, ProcessingContext) = { parseCreate(jsObject) - .flatMap(mailboxCreationRequest => resolvePath(mailboxSession, mailboxCreationRequest, processingContext) + .flatMap(mailboxCreationRequest => resolvePath(mailboxSession, mailboxCreationRequest) .flatMap(path => createMailbox(mailboxSession = mailboxSession, path = path, mailboxCreationRequest = mailboxCreationRequest))) @@ -441,14 +443,16 @@ class MailboxSetMethod @Inject()(serializer: MailboxSerializer, } private def resolvePath(mailboxSession: MailboxSession, - mailboxCreationRequest: MailboxCreationRequest, - processingContext: ProcessingContext): Either[Exception, MailboxPath] = { + mailboxCreationRequest: MailboxCreationRequest): Either[Exception, MailboxPath] = { if (mailboxCreationRequest.name.value.contains(mailboxSession.getPathDelimiter)) { return Left(new MailboxNameException(s"The mailbox '${mailboxCreationRequest.name.value}' contains an illegal character: '${mailboxSession.getPathDelimiter}'")) } mailboxCreationRequest.parentId .map(maybeParentId => for { - parentId <- processingContext.resolveMailboxId(maybeParentId, mailboxIdFactory) + parentId <- Try(mailboxIdFactory.fromString(maybeParentId.value)) + .toEither + .left + .map(e => new IllegalArgumentException(e.getMessage, e)) parentPath <- retrievePath(parentId, mailboxSession) } yield { parentPath.child(mailboxCreationRequest.name, mailboxSession.getPathDelimiter) diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/VacationResponseGetMethod.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/VacationResponseGetMethod.scala index 2978afd..ebbc939 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/VacationResponseGetMethod.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/VacationResponseGetMethod.scala @@ -24,14 +24,13 @@ import javax.inject.Inject import org.apache.james.jmap.api.vacation.{VacationRepository, AccountId => JavaAccountId} import org.apache.james.jmap.http.SessionSupplier import org.apache.james.jmap.json.{ResponseSerializer, VacationSerializer} -import org.apache.james.jmap.mail.VacationResponse.UnparsedVacationResponseId +import org.apache.james.jmap.mail.VacationResponse.{UNPARSED_SINGLETON, UnparsedVacationResponseId} import org.apache.james.jmap.mail.{VacationResponse, VacationResponseGetRequest, VacationResponseGetResponse, VacationResponseNotFound} import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier import org.apache.james.jmap.model.DefaultCapabilities.{CORE_CAPABILITY, MAIL_CAPABILITY, VACATION_RESPONSE_CAPABILITY} import org.apache.james.jmap.model.Invocation.{Arguments, MethodCallId, MethodName} import org.apache.james.jmap.model.State.INSTANCE import org.apache.james.jmap.model.{AccountId, Capabilities, ErrorCode, Invocation, MissingCapabilityException, Properties} -import org.apache.james.jmap.routes.ProcessingContext import org.apache.james.mailbox.MailboxSession import org.apache.james.metrics.api.MetricFactory import play.api.libs.json.{JsError, JsObject, JsSuccess} @@ -67,7 +66,7 @@ class VacationResponseGetMethod @Inject()(vacationRepository: VacationRepository { val requestedProperties: Properties = request.properties.getOrElse(VacationResponse.allProperties) (requestedProperties -- VacationResponse.allProperties match { - case invalidProperties if invalidProperties.isEmpty() => getVacationResponse(request, invocation.processingContext, mailboxSession) + case invalidProperties if invalidProperties.isEmpty() => getVacationResponse(request, mailboxSession) .reduce(VacationResponseGetResult.empty, VacationResponseGetResult.merge) .map(vacationResult => vacationResult.asResponse(request.accountId)) .map(vacationResponseGetResponse => Invocation( @@ -97,16 +96,16 @@ class VacationResponseGetMethod @Inject()(vacationRepository: VacationRepository } private def getVacationResponse(vacationResponseGetRequest: VacationResponseGetRequest, - processingContext: ProcessingContext, mailboxSession: MailboxSession): SFlux[VacationResponseGetResult] = vacationResponseGetRequest.ids match { case None => getVacationSingleton(mailboxSession) .map(VacationResponseGetResult.found) .flux() case Some(ids) => SFlux.fromIterable(ids.value) - .flatMap(id => processingContext.resolveVacationResponseId(id) - .fold(_ => SMono.just(VacationResponseGetResult.notFound(id)), - _ => getVacationSingleton(mailboxSession).map(VacationResponseGetResult.found))) + .flatMap(id => id match { + case UNPARSED_SINGLETON => getVacationSingleton(mailboxSession).map(VacationResponseGetResult.found) + case _ => SMono.just(VacationResponseGetResult.notFound(id)) + }) } private def getVacationSingleton(mailboxSession: MailboxSession): SMono[VacationResponse] = { 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 index 8506f2f..a814df4 100644 --- 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 @@ -23,13 +23,9 @@ import eu.timepit.refined.numeric.NonNegative import eu.timepit.refined.refineV import eu.timepit.refined.types.numeric.NonNegInt import org.apache.james.jmap.json.BackReferenceDeserializer -import org.apache.james.jmap.mail.MailboxSetRequest.UnparsedMailboxId -import org.apache.james.jmap.mail.VacationResponse.{UnparsedVacationResponseId, VACATION_RESPONSE_ID} -import org.apache.james.jmap.model.Id.Id import org.apache.james.jmap.model.Invocation.{Arguments, MethodCallId, MethodName} import org.apache.james.jmap.model.{ClientId, Id, Invocation, ServerId} -import org.apache.james.mailbox.model.MailboxId -import play.api.libs.json.{JsArray, JsError, JsObject, JsResult, JsSuccess, JsValue, Reads} +import play.api.libs.json.{JsArray, JsError, JsObject, JsResult, JsString, JsSuccess, JsValue, Reads} import scala.util.Try @@ -115,7 +111,7 @@ case class JsonPath(parts: List[JsonPathPart]) { } } - private def readWildcard(jsValue: JsValue) = jsValue match { + private def readWildcard(jsValue: JsValue): JsResult[JsValue] = jsValue match { case JsArray(arrayItems) => val evaluationResults: List[JsResult[JsValue]] = arrayItems.toList.map(evaluate) @@ -146,8 +142,7 @@ case class InvalidResultReferenceException(message: String) extends IllegalArgum case class ProcessingContext(private val creationIds: Map[ClientId, ServerId], private val invocations: Map[MethodCallId, Invocation]) { - def recordCreatedId(clientId: ClientId, serverId: ServerId): ProcessingContext = ProcessingContext(creationIds + (clientId -> serverId), invocations) - private def retrieveServerId(clientId: ClientId): Option[ServerId] = creationIds.get(clientId) + def recordCreatedId(clientId: ClientId, serverId: ServerId): ProcessingContext = ProcessingContext(creationIds + (clientId -> serverId), invocations) def recordInvocation(invocation: Invocation): ProcessingContext = ProcessingContext(creationIds, invocations + (invocation.methodCallId -> invocation)) @@ -163,6 +158,9 @@ case class ProcessingContext(private val creationIds: Map[ClientId, ServerId], p private def backReferenceResolver(): Reads[JsValue] = { case JsArray(value) => resolveBackReferences(value) case JsObject(underlying) => resolveBackReference(underlying) + case JsString(value) if value.startsWith("#") => resolveCreationId(value) + .fold(_ => JsSuccess(JsString(value)), + serverId => JsSuccess(JsString(serverId.value.value))) case others: JsValue => JsSuccess(others) } @@ -174,7 +172,7 @@ case class ProcessingContext(private val creationIds: Map[ClientId, ServerId], p } private def resolveBackReference(underlying: collection.Map[String, JsValue]): JsResult[JsObject] = { - val resolutions = underlying.map(resolveBackReference) + val resolutions = underlying.map(resolveBackReference(_)) val firstError = resolutions.flatMap({ case Left(jsError) => Some(jsError) @@ -196,7 +194,12 @@ case class ProcessingContext(private val creationIds: Map[ClientId, ServerId], p BackReferenceDeserializer.deserializeBackReference(entry._2) match { case JsSuccess(backReference, _) => resolveBackReference(newEntry, backReference) // If the JSON object is not a back-reference continue parsing (it could be a creationId) - case JsError(_) => propagateBackReferenceResolution(entry) + case JsError(_) => + backReferenceResolver().reads(entry._2) + .fold(e => Left(JsError(e)), + json => resolveCreationId(entry._1) + .fold(_ => Right((entry._1, json)), + serverId => Right((serverId.value.value, json)))) } } else { propagateBackReferenceResolution(entry) @@ -225,17 +228,9 @@ case class ProcessingContext(private val creationIds: Map[ClientId, ServerId], p .map(backReference.resolve) .getOrElse(JsError("Back reference could not be resolved")) - 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 resolveCreationId(creationId: String): Either[IllegalArgumentException, ServerId] = + Id.validate(creationId) + .flatMap(id => resolveServerId(ClientId(id))) private def resolveServerId(id: ClientId): Either[IllegalArgumentException, ServerId] = id.retrieveOriginalClientId @@ -244,10 +239,6 @@ case class ProcessingContext(private val creationIds: Map[ClientId, ServerId], p .getOrElse(Left[IllegalArgumentException, ServerId](new IllegalArgumentException(s"$id was not used in previously defined creationIds"))))) .getOrElse(Right(ServerId(id.value))) - def resolveVacationResponseId(unparsedVacationId: UnparsedVacationResponseId): Either[IllegalArgumentException, Id] = - if (unparsedVacationId.equals(VACATION_RESPONSE_ID)) { - Right(VACATION_RESPONSE_ID) - } else { - Left(new IllegalArgumentException(s"$unparsedVacationId is not a valid VacationResponse ID")) - } + private def retrieveServerId(clientId: ClientId): Option[ServerId] = creationIds.get(clientId) + } 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 de24595..c2841fc 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,7 +24,7 @@ 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.MailboxGet.UnparsedMailboxId import org.apache.james.jmap.mail._ import org.apache.james.jmap.model.{AccountId, Properties} import org.apache.james.mailbox.model.{MailboxId, TestId} @@ -47,22 +47,6 @@ object MailboxGetSerializationTest { class MailboxGetSerializationTest extends AnyWordSpec with Matchers { "Deserialize MailboxGetRequest" should { - "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) should equal(JsSuccess(expectedRequestObject)) - } - "succeed when properties are missing" in { val expectedRequestObject = MailboxGetRequest( accountId = ACCOUNT_ID, diff --git a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/json/VacationResponseGetSerializationTest.scala b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/json/VacationResponseGetSerializationTest.scala index e3028aa..35a0e4e 100644 --- a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/json/VacationResponseGetSerializationTest.scala +++ b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/json/VacationResponseGetSerializationTest.scala @@ -27,14 +27,11 @@ import org.apache.james.jmap.json.VacationResponseSerializationTest.VACATION_RES import org.apache.james.jmap.mail.VacationResponse.UnparsedVacationResponseId import org.apache.james.jmap.mail.{VacationResponse, VacationResponseGetRequest, VacationResponseGetResponse, VacationResponseIds, VacationResponseNotFound} import org.apache.james.jmap.model.{AccountId, Properties} -import org.apache.james.mailbox.model.{MailboxId, TestId} import org.scalatest.matchers.should.Matchers import org.scalatest.wordspec.AnyWordSpec import play.api.libs.json.{JsSuccess, Json} object VacationResponseGetSerializationTest { - private val FACTORY: MailboxId.Factory = new TestId.Factory - private val ACCOUNT_ID: AccountId = AccountId(id) private val SINGLETON_ID: UnparsedVacationResponseId = "singleton" --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
