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/16 06:20:54 UTC

[james-project] branch master updated (61ce101 -> 78c4b2a)

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

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


    from 61ce101  JAMES-3261 Guice memory docker should reuse guice memory ZIP app
     new 9befe34  JAMES-3558 JMAP Email/changes: When created + updated return both
     new 78c4b2a  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] 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 master
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit 78c4b2a96d16546838be4277bab6aeeb12f2e047
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


[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 master
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit 9befe3464c61a6ae1a7419c028b671a3d486bb7b
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