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

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

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