You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2021/04/17 00:57:58 UTC

[james-project] branch 3.6.x updated (b02976b -> dc43544)

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

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


    from b02976b  JAMES-3537 (Email/set create should allow to attach mails) to 3.6.x branch
     new 6b4c985  JAMES-3558 JMAP Email/changes: When created + updated return both
     new dc43544  JAMES-3558 JMAP Email/changes: moves should be considered as updates

The 2 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:
 .../apache/james/jmap/api/change/EmailChange.java  | 46 +++++++----
 .../apache/james/jmap/api/change/EmailChanges.java |  3 +-
 .../api/change/EmailChangeRepositoryContract.java  |  4 +-
 .../contract/EmailChangesMethodContract.scala      | 94 +++++++++++++++++++++-
 .../james/jmap/change/MailboxChangeListener.scala  |  6 +-
 .../jmap/change/MailboxChangeListenerTest.scala    |  2 +-
 6 files changed, 128 insertions(+), 27 deletions(-)

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


[james-project] 01/02: JAMES-3558 JMAP Email/changes: When created + updated return both

Posted by bt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

btellier pushed a commit to branch 3.6.x
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit 6b4c9854f954d4e5be34c6c2071c5a3b0a49b526
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Fri Apr 9 13:16:20 2021 +0700

    JAMES-3558 JMAP Email/changes: When created + updated return both
    
    As we are not able to distinguish mailboxIds updates from creates, we
    shall return them both.
---
 .../apache/james/jmap/api/change/EmailChange.java  | 46 ++++++++++++++--------
 .../apache/james/jmap/api/change/EmailChanges.java |  3 +-
 .../api/change/EmailChangeRepositoryContract.java  |  4 +-
 .../contract/EmailChangesMethodContract.scala      |  2 +-
 4 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/change/EmailChange.java b/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/change/EmailChange.java
index 375899d..6210da1 100644
--- a/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/change/EmailChange.java
+++ b/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/change/EmailChange.java
@@ -23,20 +23,28 @@ import java.time.ZonedDateTime;
 import java.util.Collection;
 import java.util.List;
 import java.util.Objects;
+import java.util.Set;
 import java.util.stream.Stream;
 
 import javax.inject.Inject;
 
+import org.apache.james.core.Username;
 import org.apache.james.jmap.api.model.AccountId;
+import org.apache.james.mailbox.MessageIdManager;
+import org.apache.james.mailbox.SessionProvider;
 import org.apache.james.mailbox.events.MailboxEvents.Added;
 import org.apache.james.mailbox.events.MailboxEvents.Expunged;
 import org.apache.james.mailbox.events.MailboxEvents.FlagsUpdated;
+import org.apache.james.mailbox.exception.MailboxException;
 import org.apache.james.mailbox.model.MessageId;
 
+import com.github.fge.lambdas.Throwing;
 import com.github.steveash.guavate.Guavate;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
 
 public class EmailChange implements JmapChange {
     public static class Builder {
@@ -123,10 +131,14 @@ public class EmailChange implements JmapChange {
 
     public static class Factory {
         private final State.Factory stateFactory;
+        private final MessageIdManager messageIdManager;
+        private final SessionProvider sessionProvider;
 
         @Inject
-        public Factory(State.Factory stateFactory) {
+        public Factory(State.Factory stateFactory, MessageIdManager messageIdManager, SessionProvider sessionProvider) {
             this.stateFactory = stateFactory;
+            this.messageIdManager = messageIdManager;
+            this.sessionProvider = sessionProvider;
         }
 
         public List<JmapChange> fromAdded(Added messageAdded, ZonedDateTime now, List<AccountId> sharees) {
@@ -173,27 +185,29 @@ public class EmailChange implements JmapChange {
                 .collect(Guavate.toImmutableList());
         }
 
-        public List<JmapChange> fromExpunged(Expunged expunged, ZonedDateTime now, List<AccountId> sharees) {
-            EmailChange ownerChange = EmailChange.builder()
-                .accountId(AccountId.fromUsername(expunged.getUsername()))
-                .state(stateFactory.generate())
-                .date(now)
-                .isDelegated(false)
-                .destroyed(expunged.getMessageIds())
-                .build();
+        public List<JmapChange> fromExpunged(Expunged expunged, ZonedDateTime now, List<Username> sharees) throws MailboxException {
+
+            EmailChange ownerChange = fromExpunged(expunged, now, expunged.getUsername());
 
             Stream<EmailChange> shareeChanges = sharees.stream()
-                .map(shareeId -> EmailChange.builder()
-                    .accountId(shareeId)
-                    .state(stateFactory.generate())
-                    .date(now)
-                    .isDelegated(true)
-                    .destroyed(expunged.getMessageIds())
-                    .build());
+                .map(Throwing.<Username, EmailChange>function(shareeId -> fromExpunged(expunged, now, shareeId)).sneakyThrow());
 
             return Stream.concat(Stream.of(ownerChange), shareeChanges)
                 .collect(Guavate.toImmutableList());
         }
+
+        private EmailChange fromExpunged(Expunged expunged, ZonedDateTime now, Username username) throws MailboxException {
+            Set<MessageId> accessibleMessageIds = messageIdManager.accessibleMessages(expunged.getMessageIds(), sessionProvider.createSystemSession(username));
+
+            return EmailChange.builder()
+                .accountId(AccountId.fromUsername(username))
+                .state(stateFactory.generate())
+                .date(now)
+                .isDelegated(false)
+                .updated(Sets.intersection(ImmutableSet.copyOf(expunged.getMessageIds()), accessibleMessageIds))
+                .destroyed(Sets.difference(ImmutableSet.copyOf(expunged.getMessageIds()), accessibleMessageIds))
+                .build();
+        }
     }
 
     private final AccountId accountId;
diff --git a/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/change/EmailChanges.java b/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/change/EmailChanges.java
index c037f76..1376506 100644
--- a/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/change/EmailChanges.java
+++ b/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/change/EmailChanges.java
@@ -107,8 +107,7 @@ public class EmailChanges {
             Set<MessageId> updatedTemp = Sets.difference(
                 ImmutableSet.<MessageId>builder()
                     .addAll(updated)
-                    .addAll(Sets.difference(ImmutableSet.copyOf(change.getUpdated()),
-                        createdTemp))
+                    .addAll(ImmutableSet.copyOf(change.getUpdated()))
                     .build(),
                 ImmutableSet.copyOf(change.getDestroyed()));
             destroyedTemp.addAll(Sets.difference(
diff --git a/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/change/EmailChangeRepositoryContract.java b/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/change/EmailChangeRepositoryContract.java
index bf0ca99..d72b4a4 100644
--- a/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/change/EmailChangeRepositoryContract.java
+++ b/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/change/EmailChangeRepositoryContract.java
@@ -659,7 +659,7 @@ public interface EmailChangeRepositoryContract {
 
         SoftAssertions.assertSoftly(softly -> {
             softly.assertThat(emailChanges.getCreated()).containsExactlyInAnyOrder(messageId2, messageId3, messageId6, messageId7, messageId8);
-            softly.assertThat(emailChanges.getUpdated()).containsExactlyInAnyOrder(messageId1);
+            softly.assertThat(emailChanges.getUpdated()).containsExactlyInAnyOrder(messageId2, messageId3, messageId6, messageId7, messageId1);
             softly.assertThat(emailChanges.getDestroyed()).containsExactlyInAnyOrder(messageId9, messageId10);
         });
     }
@@ -1279,7 +1279,7 @@ public interface EmailChangeRepositoryContract {
 
         SoftAssertions.assertSoftly(softly -> {
             softly.assertThat(emailChanges.getCreated()).containsExactlyInAnyOrder(messageId2, messageId3, messageId6, messageId7, messageId8);
-            softly.assertThat(emailChanges.getUpdated()).containsExactlyInAnyOrder(messageId1);
+            softly.assertThat(emailChanges.getUpdated()).containsExactlyInAnyOrder(messageId2, messageId3, messageId6, messageId7, messageId1);
             softly.assertThat(emailChanges.getDestroyed()).containsExactlyInAnyOrder(messageId9, messageId10);
         });
     }
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/EmailChangesMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailChangesMethodContract.scala
index 7a2936c..404019e 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailChangesMethodContract.scala
+++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailChangesMethodContract.scala
@@ -455,7 +455,7 @@ trait EmailChangesMethodContract {
              |        "oldState": "${State.INITIAL.getValue}",
              |        "hasMoreChanges": false,
              |        "created": ["${messageId1.serialize}", "${messageId2.serialize}"],
-             |        "updated": [],
+             |        "updated": ["${messageId2.serialize}"],
              |        "destroyed": []
              |      }, "c1"]
              |    ]

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


[james-project] 02/02: JAMES-3558 JMAP Email/changes: moves should be considered as updates

Posted by bt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

btellier pushed a commit to branch 3.6.x
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit dc43544821c59bd194566d792cc48fc3f8ecf827
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Fri Apr 9 13:18:07 2021 +0700

    JAMES-3558 JMAP Email/changes: moves should be considered as updates
---
 .../contract/EmailChangesMethodContract.scala      | 92 +++++++++++++++++++++-
 .../james/jmap/change/MailboxChangeListener.scala  |  6 +-
 .../jmap/change/MailboxChangeListenerTest.scala    |  2 +-
 3 files changed, 94 insertions(+), 6 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/EmailChangesMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailChangesMethodContract.scala
index 404019e..80f7306 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailChangesMethodContract.scala
+++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailChangesMethodContract.scala
@@ -24,7 +24,7 @@ import java.time.Duration
 import java.util.concurrent.TimeUnit
 
 import io.netty.handler.codec.http.HttpHeaderNames.ACCEPT
-import io.restassured.RestAssured.{`given`, requestSpecification}
+import io.restassured.RestAssured.{`given`, `with`, requestSpecification}
 import io.restassured.builder.ResponseSpecBuilder
 import io.restassured.http.ContentType.JSON
 import javax.mail.Flags
@@ -38,7 +38,7 @@ import org.apache.james.jmap.api.model.AccountId
 import org.apache.james.jmap.core.ResponseObject.SESSION_STATE
 import org.apache.james.jmap.draft.JmapGuiceProbe
 import org.apache.james.jmap.http.UserCredential
-import org.apache.james.jmap.rfc8621.contract.Fixture.{ACCEPT_RFC8621_VERSION_HEADER, ANDRE, ANDRE_ACCOUNT_ID, ANDRE_PASSWORD, BOB, BOB_PASSWORD, DOMAIN, authScheme, baseRequestSpecBuilder}
+import org.apache.james.jmap.rfc8621.contract.Fixture.{ACCEPT_RFC8621_VERSION_HEADER, ACCOUNT_ID, ANDRE, ANDRE_ACCOUNT_ID, ANDRE_PASSWORD, BOB, BOB_PASSWORD, DOMAIN, authScheme, baseRequestSpecBuilder}
 import org.apache.james.mailbox.MessageManager.AppendCommand
 import org.apache.james.mailbox.model.MailboxACL.Right
 import org.apache.james.mailbox.model.{MailboxACL, MailboxPath, MessageId}
@@ -198,6 +198,94 @@ trait EmailChangesMethodContract {
   }
 
   @Test
+  def shouldReturnUpdatedWhenMessageMove(server: GuiceJamesServer): Unit = {
+    val mailboxProbe: MailboxProbeImpl = server.getProbe(classOf[MailboxProbeImpl])
+    val path: MailboxPath = MailboxPath.forUser(BOB, "mailbox1")
+
+    val mailboxId1 = mailboxProbe.createMailbox(path)
+    val mailboxId2 = mailboxProbe.createMailbox(MailboxPath.forUser(BOB, "mailbox2"))
+
+    val message: Message = Message.Builder
+      .of
+      .setSubject("test")
+      .setBody("testmail", StandardCharsets.UTF_8)
+      .build
+    val messageId: MessageId = mailboxProbe.appendMessage(BOB.asString(), path, AppendCommand.from(message)).getMessageId
+
+    val oldState: State = waitForNextState(server, AccountId.fromUsername(BOB), State.INITIAL)
+
+    val updateEmail =
+      s"""{
+         |  "using": [
+         |    "urn:ietf:params:jmap:core",
+         |    "urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [
+         |  ["Email/set",
+         |    {
+         |      "accountId": "$ACCOUNT_ID",
+         |      "update": {
+         |        "${messageId.serialize}":{
+         |          "mailboxIds": {
+         |            "${mailboxId2.serialize()}": true
+         |          }
+         |        }
+         |      }
+         |    },
+         |    "c1"]]
+         |}""".stripMargin
+
+    `with`()
+      .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+      .body(updateEmail)
+      .post
+    .`then`
+      .statusCode(SC_OK)
+      .contentType(JSON)
+      .extract
+      .body
+      .asString
+
+    val request =
+      s"""{
+         |  "using": ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail"],
+         |  "methodCalls": [[
+         |    "Email/changes",
+         |    {
+         |      "accountId": "$ACCOUNT_ID",
+         |      "sinceState": "${oldState.getValue}"
+         |    },
+         |    "c1"]]
+         |}""".stripMargin
+
+    awaitAtMostTenSeconds.untilAsserted { () =>
+      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)
+        .whenIgnoringPaths("methodResponses[0][1].newState")
+        .inPath("methodResponses[0][1]")
+        .isEqualTo(
+          s"""{
+             |  "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+             |  "oldState": "${oldState.getValue}",
+             |  "hasMoreChanges": false,
+             |  "created": ["${messageId.serialize()}"],
+             |  "updated": ["${messageId.serialize()}"],
+             |  "destroyed": []
+             |}""".stripMargin)
+    }
+  }
+
+  @Test
   def emailChangesShouldReturnUpdatedChangesWhenRemoveFlags(server: GuiceJamesServer): Unit = {
     val mailboxProbe: MailboxProbeImpl = server.getProbe(classOf[MailboxProbeImpl])
     val path: MailboxPath = MailboxPath.forUser(BOB, "mailbox1")
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/MailboxChangeListener.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/MailboxChangeListener.scala
index 8a5f7e2..3fc846a 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/MailboxChangeListener.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/change/MailboxChangeListener.scala
@@ -89,9 +89,9 @@ case class MailboxChangeListener @Inject() (@Named(InjectionKeys.JMAP) eventBus:
           mailboxChangeFactory.fromFlagsUpdated(flagsUpdated, now, sharees).asScala
             .concat(emailChangeFactory.fromFlagsUpdated(flagsUpdated, now, sharees).asScala)
         case expunged: Expunged =>
-          val sharees = getSharees(mailboxId, username).asJava
-          mailboxChangeFactory.fromExpunged(expunged, now, sharees).asScala
-            .concat(emailChangeFactory.fromExpunged(expunged, now, sharees).asScala)
+          val sharees = getSharees(mailboxId, username)
+          mailboxChangeFactory.fromExpunged(expunged, now, sharees.asJava).asScala
+            .concat(emailChangeFactory.fromExpunged(expunged, now, sharees.map(_.getIdentifier).map(Username.of).asJava).asScala)
       })
       .flatMap(saveChangeEvent, DEFAULT_CONCURRENCY)
       .`then`()
diff --git a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/change/MailboxChangeListenerTest.scala b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/change/MailboxChangeListenerTest.scala
index 17a91a5..23e5a2e 100644
--- a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/change/MailboxChangeListenerTest.scala
+++ b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/change/MailboxChangeListenerTest.scala
@@ -72,7 +72,7 @@ class MailboxChangeListenerTest {
     stateFactory = new State.DefaultFactory
     mailboxChangeFactory = new MailboxChange.Factory(stateFactory)
     mailboxChangeRepository = new MemoryMailboxChangeRepository()
-    emailChangeFactory = new EmailChange.Factory(stateFactory)
+    emailChangeFactory = new EmailChange.Factory(stateFactory, resources.getMessageIdManager, resources.getMailboxManager)
     emailChangeRepository = new MemoryEmailChangeRepository()
     val eventBus = new EventBus {
       override def register(listener: EventListener.ReactiveEventListener, key: RegistrationKey): Publisher[Registration] = Mono.empty()

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