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:56 UTC

[james-project] branch master updated (cfc6b0d -> 1060cb0)

This is an automated email from the ASF dual-hosted git repository.

rcordier pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git.


    from cfc6b0d  JAMES-2891 redirect to JMAP session resource from /.well-known/jmap
     new f5967df  JAMES-3412 Email/set update keywords partial update
     new 4306950  JAMES-3412 Email/set update keywords partial update: validation
     new 1060cb0  [REFACTORING] Solve intelliJ warnings in CassandraMessageDAO

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../cassandra/mail/CassandraMessageDAO.java        |  34 +--
 .../cassandra/CassandraMailboxManagerTest.java     |   1 -
 .../cassandra/mail/CassandraMessageDAOTest.java    |   1 -
 .../rfc8621/contract/EmailSetMethodContract.scala  | 283 ++++++++++++++++++++-
 .../james/jmap/json/EmailSetSerializer.scala       |  52 +++-
 .../org/apache/james/jmap/mail/EmailSet.scala      |  67 ++---
 .../apache/james/jmap/method/EmailSetMethod.scala  |  23 +-
 .../org/apache/james/jmap/model/Keywords.scala     |   4 +
 8 files changed, 391 insertions(+), 74 deletions(-)


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[james-project] 03/03: [REFACTORING] Solve intelliJ warnings in CassandraMessageDAO

Posted by rc...@apache.org.
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 1060cb0f12769961295e4d765e2fe5052e35a7e4
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Tue Oct 20 10:21:29 2020 +0700

    [REFACTORING] Solve intelliJ warnings in CassandraMessageDAO
---
 .../cassandra/mail/CassandraMessageDAO.java        | 34 +++++-----------------
 .../cassandra/CassandraMailboxManagerTest.java     |  1 -
 .../cassandra/mail/CassandraMessageDAOTest.java    |  1 -
 3 files changed, 7 insertions(+), 29 deletions(-)

diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAO.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAO.java
index 7e7f06b..a321551 100644
--- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAO.java
+++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAO.java
@@ -48,7 +48,6 @@ import javax.mail.util.SharedByteArrayInputStream;
 
 import org.apache.commons.io.IOUtils;
 import org.apache.james.backends.cassandra.init.CassandraTypesProvider;
-import org.apache.james.backends.cassandra.init.configuration.CassandraConfiguration;
 import org.apache.james.backends.cassandra.init.configuration.CassandraConsistenciesConfiguration;
 import org.apache.james.backends.cassandra.utils.CassandraAsyncExecutor;
 import org.apache.james.blob.api.BlobId;
@@ -76,7 +75,6 @@ import com.datastax.driver.core.Session;
 import com.datastax.driver.core.UDTValue;
 import com.datastax.driver.core.querybuilder.QueryBuilder;
 import com.github.steveash.guavate.Guavate;
-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.common.primitives.Bytes;
 
@@ -91,54 +89,36 @@ public class CassandraMessageDAO {
     private final CassandraTypesProvider typesProvider;
     private final BlobStore blobStore;
     private final BlobId.Factory blobIdFactory;
-    private final CassandraConfiguration configuration;
-    private final CassandraMessageId.Factory messageIdFactory;
     private final PreparedStatement insert;
     private final PreparedStatement delete;
     private final PreparedStatement select;
-    private final PreparedStatement selectAllMessagesWithAttachment;
     private final Cid.CidParser cidParser;
     private final ConsistencyLevel consistencyLevel;
 
     @Inject
-    public CassandraMessageDAO(Session session, CassandraTypesProvider typesProvider, BlobStore blobStore,
-                               BlobId.Factory blobIdFactory, CassandraConfiguration cassandraConfiguration,
-                               CassandraConsistenciesConfiguration consistenciesConfiguration,
-                               CassandraMessageId.Factory messageIdFactory) {
+    public CassandraMessageDAO(Session session,
+                               CassandraTypesProvider typesProvider,
+                               BlobStore blobStore,
+                               BlobId.Factory blobIdFactory,
+                               CassandraConsistenciesConfiguration consistenciesConfiguration) {
         this.cassandraAsyncExecutor = new CassandraAsyncExecutor(session);
         this.consistencyLevel = consistenciesConfiguration.getRegular();
         this.typesProvider = typesProvider;
         this.blobStore = blobStore;
         this.blobIdFactory = blobIdFactory;
-        this.configuration = cassandraConfiguration;
-        this.messageIdFactory = messageIdFactory;
 
         this.insert = prepareInsert(session);
         this.delete = prepareDelete(session);
         this.select = prepareSelect(session);
-        this.selectAllMessagesWithAttachment = prepareSelectAllMessagesWithAttachment(session);
         this.cidParser = Cid.parser().relaxed();
     }
 
-    @VisibleForTesting
-    public CassandraMessageDAO(Session session, CassandraTypesProvider typesProvider, BlobStore blobStore,
-                               BlobId.Factory blobIdFactory, CassandraMessageId.Factory messageIdFactory,
-                               CassandraConsistenciesConfiguration consistenciesConfiguration) {
-        this(session, typesProvider, blobStore,  blobIdFactory, CassandraConfiguration.DEFAULT_CONFIGURATION,
-            consistenciesConfiguration, messageIdFactory);
-    }
-
     private PreparedStatement prepareSelect(Session session) {
         return session.prepare(select()
             .from(TABLE_NAME)
             .where(eq(MESSAGE_ID, bindMarker(MESSAGE_ID))));
     }
 
-    private PreparedStatement prepareSelectAllMessagesWithAttachment(Session session) {
-        return session.prepare(select(MESSAGE_ID, ATTACHMENTS)
-            .from(TABLE_NAME));
-    }
-
     private PreparedStatement prepareInsert(Session session) {
         return session.prepare(insertInto(TABLE_NAME)
             .value(MESSAGE_ID, bindMarker(MESSAGE_ID))
@@ -227,11 +207,11 @@ public class CassandraMessageDAO {
     }
 
     public Mono<MessageRepresentation> retrieveMessage(CassandraMessageId cassandraMessageId, FetchType fetchType) {
-        return retrieveRow(cassandraMessageId, fetchType)
+        return retrieveRow(cassandraMessageId)
                 .flatMap(resultSet -> message(resultSet, cassandraMessageId, fetchType));
     }
 
-    private Mono<ResultSet> retrieveRow(CassandraMessageId messageId, FetchType fetchType) {
+    private Mono<ResultSet> retrieveRow(CassandraMessageId messageId) {
         return cassandraAsyncExecutor.execute(select
             .bind()
             .setUUID(MESSAGE_ID, messageId.get())
diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraMailboxManagerTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraMailboxManagerTest.java
index f2a98f2..83e024e 100644
--- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraMailboxManagerTest.java
+++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraMailboxManagerTest.java
@@ -826,7 +826,6 @@ public class CassandraMailboxManagerTest extends MailboxManagerTest<CassandraMai
                 cassandraCluster.getTypesProvider(),
                 mock(BlobStore.class),
                 new HashBlobId.Factory(),
-                new CassandraMessageId.Factory(),
                 cassandra.getCassandraConsistenciesConfiguration());
         }
     }
diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAOTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAOTest.java
index 7fa6f49..b255ac7 100644
--- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAOTest.java
+++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAOTest.java
@@ -94,7 +94,6 @@ class CassandraMessageDAOTest {
             cassandra.getTypesProvider(),
             blobStore,
             blobIdFactory,
-            new CassandraMessageId.Factory(),
             cassandraCluster.getCassandraConsistenciesConfiguration());
 
         messageIdWithMetadata = ComposedMessageIdWithMetaData.builder()


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


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

Posted by rc...@apache.org.
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


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

Posted by rc...@apache.org.
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 4306950317d246aa8799a9d9ebe2659667ef6920
Author: duc91 <vd...@linagora.com>
AuthorDate: Tue Oct 20 11:59:54 2020 +0700

    JAMES-3412 Email/set update keywords partial update: validation
    
    Delegated mailboxes testing, as well as not overwriting stored
    Recent and Deleted flags is already covered as part of reset work.
---
 .../rfc8621/contract/EmailSetMethodContract.scala  | 214 ++++++++++++++++++++-
 .../james/jmap/json/EmailSetSerializer.scala       |   5 +-
 2 files changed, 215 insertions(+), 4 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 26316a2..8a92b53 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
@@ -276,7 +276,7 @@ trait EmailSetMethodContract {
         s"""{
            |  "${messageId.serialize}":{
            |      "type":"invalidPatch",
-           |      "description":"Message 1 update is invalid: Does not allow to update 'Deleted' or 'Recent' flag"}
+           |      "description":"Message 1 update is invalid: List((,List(JsonValidationError(List(Value associated with keywords is invalid: List((,List(JsonValidationError(List(Does not allow to update 'Deleted' or 'Recent' flag),ArraySeq()))))),ArraySeq()))))"}
            |  }
            |}"""
           .stripMargin)
@@ -670,6 +670,216 @@ trait EmailSetMethodContract {
   }
 
   @Test
+  def emailSetShouldRejectPartiallyUpdateAndResetKeywordsAtTheSameTime(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,
+         |          "keywords": {
+         |             "movie": true
+         |          }
+         |        }
+         |      }
+         |    }, "c1"]]
+         |}""".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].notUpdated")
+      .isEqualTo(s"""{
+          |  "${messageId.serialize}": {
+          |     "type": "invalidPatch",
+          |     "description": "Message 1 update is invalid: Partial update and reset specified for keywords"
+          |   }
+          |}
+      """.stripMargin)
+  }
+
+  @Test
+  def emailSetShouldRejectPartiallyUpdateWhenInvalidKeyword(server: GuiceJamesServer): Unit = {
+    val message: Message = Fixture.createTestMessage
+
+    val flags: Flags = new Flags(Flags.Flag.ANSWERED)
+
+    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 =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [
+         |    ["Email/set", {
+         |      "accountId": "$ACCOUNT_ID",
+         |      "update": {
+         |        "${messageId.serialize}":{
+         |          "keywords/mus*c": true
+         |        }
+         |      }
+         |    }, "c1"]]
+         |}""".stripMargin
+
+    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(s"methodResponses[0][1].notUpdated.${messageId.serialize}")
+      .isEqualTo(
+        """|{
+           |   "type":"invalidPatch",
+           |   "description": "Message 1 update is invalid: List((,List(JsonValidationError(List(keywords/mus*c is an invalid entry in an Email/set update patch: FlagName must not be null or empty, must have length form 1-255,must not contain characters with hex from '\\u0000' to '\\u00019' or {'(' ')' '{' ']' '%' '*' '\"' '\\'} ),ArraySeq()))))"}"
+           |}""".stripMargin)
+  }
+
+  @Test
+  def emailSetShouldRejectPartiallyUpdateWhenFalseValue(server: GuiceJamesServer): Unit = {
+    val message: Message = Fixture.createTestMessage
+
+    val flags: Flags = new Flags(Flags.Flag.ANSWERED)
+
+    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 =
+      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/movie": false
+         |        }
+         |      }
+         |    }, "c1"]]
+         |}""".stripMargin
+
+    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(s"methodResponses[0][1].notUpdated.${messageId.serialize}")
+      .isEqualTo(
+        """|{
+          |   "type":"invalidPatch",
+          |   "description": "Message 1 update is invalid: List((,List(JsonValidationError(List(Value associated with keywords/movie is invalid: Keywords partial updates requires a JsBoolean(true) (set) or a JsNull (unset)),ArraySeq()))))"
+          |}""".stripMargin)
+  }
+
+  @ParameterizedTest
+  @ValueSource(strings = Array(
+    "$Recent",
+    "$Deleted"
+  ))
+  def partialUpdateShouldRejectNonExposedKeyword(unexposedKeyword: String, server: GuiceJamesServer): Unit = {
+    val message: Message = Fixture.createTestMessage
+
+    val flags: Flags = new Flags(Flags.Flag.ANSWERED)
+
+    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/$unexposedKeyword": true
+         |        }
+         |      }
+         |    }, "c1"]]
+         |}""".stripMargin)
+
+    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].notUpdated")
+      .isEqualTo(
+        s"""{
+           |  "${messageId.serialize}":{
+           |      "type":"invalidPatch",
+           |      "description":"Message 1 update is invalid: List((,List(JsonValidationError(List(Does not allow to update 'Deleted' or 'Recent' flag),ArraySeq()))))"}
+           |  }
+           |}"""
+          .stripMargin)
+  }
+
+  @Test
   def emailSetShouldDestroyEmail(server: GuiceJamesServer): Unit = {
     val mailboxProbe = server.getProbe(classOf[MailboxProbeImpl])
     mailboxProbe.createMailbox(MailboxPath.inbox(BOB))
@@ -1322,7 +1532,7 @@ trait EmailSetMethodContract {
       s"""{
          |  "1": {
          |    "type": "invalidPatch",
-         |    "description": "Message 1 update is invalid: Partial update and reset specified"
+         |    "description": "Message 1 update is invalid: Partial update and reset specified for mailboxIds"
          |  }
          |}""".stripMargin)
   }
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 4a7f14d..2928c03 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
@@ -206,8 +206,9 @@ class EmailSetSerializer @Inject()(messageIdFactory: MessageId.Factory, mailboxI
         case JsBoolean(false) => JsError("keyword value can only be true")
         case _ => JsError("Expecting keyword value to be a boolean")
       })
-  private implicit val keywordsReads: Reads[Keywords] = jsValue => keywordsMapReads.reads(jsValue).map(
-    keywordsMap => Keywords(keywordsMap.keys.toSet))
+  private implicit val keywordsReads: Reads[Keywords] = jsValue => keywordsMapReads.reads(jsValue).flatMap(
+    keywordsMap => STRICT_KEYWORDS_FACTORY.fromSet(keywordsMap.keys.toSet)
+      .fold(e => JsError(e.getMessage), keywords => JsSuccess(keywords)))
 
   private implicit val unitWrites: Writes[Unit] = _ => JsNull
   private implicit val updatedWrites: Writes[Map[MessageId, Unit]] = mapWrites[MessageId, Unit](_.serialize, unitWrites)


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org