You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by rc...@apache.org on 2020/10/21 02:16:57 UTC

[james-project] 01/03: JAMES-3412 Email/set update keywords partial update

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 <vd...@linagora.com>
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: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org