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 f5967df4345ec0d575036dc29afba94a29719d9f Author: duc91 <[email protected]> AuthorDate: Tue Oct 20 11:21:25 2020 +0700 JAMES-3412 Email/set update keywords partial update --- .../rfc8621/contract/EmailSetMethodContract.scala | 69 ++++++++++++++++++++++ .../james/jmap/json/EmailSetSerializer.scala | 47 +++++++++++++-- .../org/apache/james/jmap/mail/EmailSet.scala | 67 ++++++++++++--------- .../apache/james/jmap/method/EmailSetMethod.scala | 23 +++++--- .../org/apache/james/jmap/model/Keywords.scala | 4 ++ 5 files changed, 169 insertions(+), 41 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/EmailSetMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailSetMethodContract.scala index 3267358..26316a2 100644 --- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailSetMethodContract.scala +++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailSetMethodContract.scala @@ -601,6 +601,75 @@ trait EmailSetMethodContract { } @Test + def emailSetShouldPartiallyUpdateKeywords(server: GuiceJamesServer): Unit = { + val message: Message = Fixture.createTestMessage + + val flags: Flags = FlagsBuilder.builder() + .add(Flags.Flag.ANSWERED) + .add(Flags.Flag.SEEN) + .build() + + val bobPath = MailboxPath.inbox(BOB) + server.getProbe(classOf[MailboxProbeImpl]).createMailbox(bobPath) + val messageId: MessageId = server.getProbe(classOf[MailboxProbeImpl]).appendMessage(BOB.asString(), bobPath, AppendCommand.builder() + .withFlags(flags) + .build(message)) + .getMessageId + + val request = String.format( + s"""{ + | "using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail"], + | "methodCalls": [ + | ["Email/set", { + | "accountId": "$ACCOUNT_ID", + | "update": { + | "${messageId.serialize}":{ + | "keywords/music": true, + | "keywords/%s": null + | } + | } + | }, "c1"], + | ["Email/get", + | { + | "accountId": "$ACCOUNT_ID", + | "ids": ["${messageId.serialize}"], + | "properties": ["keywords"] + | }, + | "c2"]] + |}""".stripMargin, "$Seen") + + val response = `given` + .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER) + .body(request) + .when + .post + .`then` + .statusCode(SC_OK) + .contentType(JSON) + .extract + .body + .asString + + assertThatJson(response) + .inPath("methodResponses[0][1].updated") + .isEqualTo(s"""{ + | "${messageId.serialize}": null + |} + """.stripMargin) + assertThatJson(response) + .inPath("methodResponses[1][1].list[0]") + .isEqualTo(String.format( + """{ + | "id":"%s", + | "keywords": { + | "$Answered": true, + | "music": true + | } + |} + """.stripMargin, messageId.serialize)) + } + + @Test def emailSetShouldDestroyEmail(server: GuiceJamesServer): Unit = { val mailboxProbe = server.getProbe(classOf[MailboxProbeImpl]) mailboxProbe.createMailbox(MailboxPath.inbox(BOB)) diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/EmailSetSerializer.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/EmailSetSerializer.scala index 28e79c2..4a7f14d 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/EmailSetSerializer.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/EmailSetSerializer.scala @@ -19,10 +19,12 @@ package org.apache.james.jmap.json +import cats.implicits._ import eu.timepit.refined.refineV import javax.inject.Inject import org.apache.james.jmap.mail.EmailSet.{UnparsedMessageId, UnparsedMessageIdConstraint} import org.apache.james.jmap.mail.{DestroyIds, EmailSetRequest, EmailSetResponse, EmailSetUpdate, MailboxIds} +import org.apache.james.jmap.model.KeywordsFactory.STRICT_KEYWORDS_FACTORY import org.apache.james.jmap.model.{Keyword, Keywords, SetError} import org.apache.james.mailbox.model.{MailboxId, MessageId} import play.api.libs.json.{JsBoolean, JsError, JsNull, JsObject, JsResult, JsString, JsSuccess, JsValue, Json, OWrites, Reads, Writes} @@ -68,14 +70,40 @@ class EmailSetSerializer @Inject()(messageIdFactory: MessageId.Factory, mailboxI .filter(_.nonEmpty) .map(MailboxIds) - JsSuccess(EmailSetUpdate(keywords = keywordsReset, - mailboxIds = mailboxReset, - mailboxIdsToAdd = mailboxesToAdd, - mailboxIdsToRemove = mailboxesToRemove)) + val keywordsToAdd: Try[Option[Keywords]] = Some(entries + .flatMap { + case update: KeywordAddition => Some(update) + case _ => None + }.map(_.keyword).toSet) + .filter(_.nonEmpty) + .map(STRICT_KEYWORDS_FACTORY.fromSet) + .sequence + + val keywordsToRemove: Try[Option[Keywords]] = Some(entries + .flatMap { + case update: KeywordRemoval => Some(update) + case _ => None + }.map(_.keyword).toSet) + .filter(_.nonEmpty) + .map(STRICT_KEYWORDS_FACTORY.fromSet) + .sequence + + keywordsToAdd.flatMap(maybeKeywordsToAdd => keywordsToRemove + .map(maybeKeywordsToRemove => (maybeKeywordsToAdd, maybeKeywordsToRemove))) + .fold(e => JsError(e.getMessage), + { + case (maybeKeywordsToAdd, maybeKeywordsToRemove) => JsSuccess(EmailSetUpdate(keywords = keywordsReset, + keywordsToAdd = maybeKeywordsToAdd, + keywordsToRemove = maybeKeywordsToRemove, + mailboxIds = mailboxReset, + mailboxIdsToAdd = mailboxesToAdd, + mailboxIdsToRemove = mailboxesToRemove)) + }) }) object EntryValidation { private val mailboxIdPrefix: String = "mailboxIds/" + private val keywordsPrefix: String = "keywords/" def from(property: String, value: JsValue): EntryValidation = property match { case "mailboxIds" => mailboxIdsReads.reads(value) @@ -93,6 +121,13 @@ class EmailSetSerializer @Inject()(messageIdFactory: MessageId.Factory, mailboxI case JsNull => MailboxRemoval(id) case _ => InvalidPatchEntryValue(property, "MailboxId partial updates requires a JsBoolean(true) (set) or a JsNull (unset)") }) + case name if name.startsWith(keywordsPrefix) => Keyword.parse(name.substring(keywordsPrefix.length)) + .fold(e => InvalidPatchEntryNameWithDetails(property, e), + keyword => value match { + case JsBoolean(true) => KeywordAddition(keyword) + case JsNull => KeywordRemoval(keyword) + case _ => InvalidPatchEntryValue(property, "Keywords partial updates requires a JsBoolean(true) (set) or a JsNull (unset)") + }) case _ => InvalidPatchEntryName(property) } } @@ -121,6 +156,10 @@ class EmailSetSerializer @Inject()(messageIdFactory: MessageId.Factory, mailboxI private case class KeywordsReset(keywords: Keywords) extends EntryValidation + private case class KeywordAddition(keyword: Keyword) extends EntryValidation + + private case class KeywordRemoval(keyword: Keyword) extends EntryValidation + } private implicit val messageIdWrites: Writes[MessageId] = messageId => JsString(messageId.serialize) diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailSet.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailSet.scala index c2ea230..8e36b33 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailSet.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/EmailSet.scala @@ -58,42 +58,51 @@ case class EmailSetResponse(accountId: AccountId, notDestroyed: Option[Map[UnparsedMessageId, SetError]]) case class EmailSetUpdate(keywords: Option[Keywords], + keywordsToAdd: Option[Keywords], + keywordsToRemove: Option[Keywords], mailboxIds: Option[MailboxIds], mailboxIdsToAdd: Option[MailboxIds], mailboxIdsToRemove: Option[MailboxIds]) { - def validate: Either[IllegalArgumentException, ValidatedEmailSetUpdate] = if (mailboxIds.isDefined && (mailboxIdsToAdd.isDefined || mailboxIdsToRemove.isDefined)) { - Left(new IllegalArgumentException("Partial update and reset specified")) - } else { - val identity: Function[MailboxIds, MailboxIds] = ids => ids - val mailboxIdsAddition: Function[MailboxIds, MailboxIds] = mailboxIdsToAdd - .map(toBeAdded => (ids: MailboxIds) => ids ++ toBeAdded) - .getOrElse(identity) - val mailboxIdsRemoval: Function[MailboxIds, MailboxIds] = mailboxIdsToRemove - .map(toBeRemoved => (ids: MailboxIds) => ids -- toBeRemoved) - .getOrElse(identity) - val mailboxIdsReset: Function[MailboxIds, MailboxIds] = mailboxIds - .map(toReset => (_: MailboxIds) => toReset) - .getOrElse(identity) - val mailboxIdsTransformation: Function[MailboxIds, MailboxIds] = mailboxIdsAddition - .compose(mailboxIdsRemoval) - .compose(mailboxIdsReset) - Right(mailboxIdsTransformation) - .flatMap(mailboxIdsTransformation => validateKeywords - .map(validatedKeywords => ValidatedEmailSetUpdate(validatedKeywords, mailboxIdsTransformation))) - } + def validate: Either[IllegalArgumentException, ValidatedEmailSetUpdate] = { + if (mailboxIds.isDefined && (mailboxIdsToAdd.isDefined || mailboxIdsToRemove.isDefined)) { + Left(new IllegalArgumentException("Partial update and reset specified for mailboxIds")) + } else if (keywords.isDefined && (keywordsToAdd.isDefined || keywordsToRemove.isDefined)) { + Left(new IllegalArgumentException("Partial update and reset specified for keywords")) + } else { + val mailboxIdsIdentity: Function[MailboxIds, MailboxIds] = ids => ids + val mailboxIdsAddition: Function[MailboxIds, MailboxIds] = mailboxIdsToAdd + .map(toBeAdded => (ids: MailboxIds) => ids ++ toBeAdded) + .getOrElse(mailboxIdsIdentity) + val mailboxIdsRemoval: Function[MailboxIds, MailboxIds] = mailboxIdsToRemove + .map(toBeRemoved => (ids: MailboxIds) => ids -- toBeRemoved) + .getOrElse(mailboxIdsIdentity) + val mailboxIdsReset: Function[MailboxIds, MailboxIds] = mailboxIds + .map(toReset => (_: MailboxIds) => toReset) + .getOrElse(mailboxIdsIdentity) + val mailboxIdsTransformation: Function[MailboxIds, MailboxIds] = mailboxIdsAddition + .compose(mailboxIdsRemoval) + .compose(mailboxIdsReset) + + val keywordsIdentity: Function[Keywords, Keywords] = keywords => keywords + val keywordsAddition: Function[Keywords, Keywords] = keywordsToAdd + .map(toBeAdded => (keywords: Keywords) => keywords ++ toBeAdded) + .getOrElse(keywordsIdentity) + val keywordsRemoval: Function[Keywords, Keywords] = keywordsToRemove + .map(toBeRemoved => (keywords: Keywords) => keywords -- toBeRemoved) + .getOrElse(keywordsIdentity) + val keywordsReset: Function[Keywords, Keywords] = keywords + .map(toReset => (_: Keywords) => toReset) + .getOrElse(keywordsIdentity) + val keywordsTransformation: Function[Keywords, Keywords] = keywordsAddition + .compose(keywordsRemoval) + .compose(keywordsReset) - private def validateKeywords: Either[IllegalArgumentException, Option[Keywords]] = { - keywords.map(_.getKeywords) - .map(STRICT_KEYWORDS_FACTORY.fromSet) - .map { - case Success(validatedKeywords: Keywords) => Right(Some(validatedKeywords)) - case Failure(throwable: IllegalArgumentException) => Left(throwable) - } - .getOrElse(Right(None)) + Right(ValidatedEmailSetUpdate(keywordsTransformation, mailboxIdsTransformation)) + } } } -case class ValidatedEmailSetUpdate private (keywords: Option[Keywords], +case class ValidatedEmailSetUpdate private (keywords: Function[Keywords, Keywords], mailboxIdsTransformation: Function[MailboxIds, MailboxIds]) class EmailUpdateValidationException() extends IllegalArgumentException diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSetMethod.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSetMethod.scala index 74d72b3..8455658 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSetMethod.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/EmailSetMethod.scala @@ -29,6 +29,7 @@ import org.apache.james.jmap.mail.{DestroyIds, EmailSet, EmailSetRequest, EmailS 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.KeywordsFactory.LENIENT_KEYWORDS_FACTORY import org.apache.james.jmap.model.SetError.SetErrorDescription import org.apache.james.jmap.model.{Capabilities, Invocation, SetError, State} import org.apache.james.mailbox.MessageManager.FlagsUpdateMode @@ -224,7 +225,7 @@ class EmailSetMethod @Inject()(serializer: EmailSetSerializer, .fold( e => SMono.just(UpdateFailure(EmailSet.asUnparsed(messageId), e)), validatedUpdate => - resetFlags(messageId, validatedUpdate, mailboxIds, originFlags, session) + updateFlags(messageId, validatedUpdate, mailboxIds, originFlags, session) .flatMap { case failure: UpdateFailure => SMono.just[UpdateResult](failure) case _: UpdateSuccess => updateMailboxIds(messageId, validatedUpdate, mailboxIds, session) @@ -247,14 +248,20 @@ class EmailSetMethod @Inject()(serializer: EmailSetSerializer, } } - private def resetFlags(messageId: MessageId, update: ValidatedEmailSetUpdate, mailboxIds: MailboxIds, originalFlags: Flags, session: MailboxSession): SMono[UpdateResult] = - update.keywords - .map(keywords => keywords.asFlagsWithRecentAndDeletedFrom(originalFlags)) - .map(flags => SMono.fromCallable(() => - messageIdManager.setFlags(flags, FlagsUpdateMode.REPLACE, messageId, ImmutableList.copyOf(mailboxIds.value.asJavaCollection), session)) + private def updateFlags(messageId: MessageId, update: ValidatedEmailSetUpdate, mailboxIds: MailboxIds, originalFlags: Flags, session: MailboxSession): SMono[UpdateResult] = { + val newFlags = update.keywords + .apply(LENIENT_KEYWORDS_FACTORY.fromFlags(originalFlags).get) + .asFlagsWithRecentAndDeletedFrom(originalFlags) + + if (newFlags.equals(originalFlags)) { + SMono.just[UpdateResult](UpdateSuccess(messageId)) + } else { + SMono.fromCallable(() => + messageIdManager.setFlags(newFlags, FlagsUpdateMode.REPLACE, messageId, ImmutableList.copyOf(mailboxIds.value.asJavaCollection), session)) .subscribeOn(Schedulers.elastic()) - .`then`(SMono.just[UpdateResult](UpdateSuccess(messageId)))) - .getOrElse(SMono.just[UpdateResult](UpdateSuccess(messageId))) + .`then`(SMono.just[UpdateResult](UpdateSuccess(messageId))) + } + } private def deleteMessage(destroyId: UnparsedMessageId, mailboxSession: MailboxSession): SMono[DestroyResult] = EmailSet.parse(messageIdFactory)(destroyId) diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/Keywords.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/Keywords.scala index 468a899..813938b 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/Keywords.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/model/Keywords.scala @@ -93,6 +93,10 @@ final case class Keywords(keywords: Set[Keyword]) { def getKeywords: Set[Keyword] = keywords + def ++(other: Keywords): Keywords = Keywords(keywords ++ other.keywords) + + def --(other: Keywords): Keywords = Keywords(keywords -- other.keywords) + def contains(keyword: Keyword): Boolean = keywords.contains(keyword) } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
