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 2023/04/21 04:19:39 UTC

[james-project] 02/06: JAMES-3777 Incremental change POJO for JMAP filtering: handle updates

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 cebc98c0afbbb1d67931a85abd4af52ecccf915b
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Tue Apr 18 18:33:28 2023 +0700

    JAMES-3777 Incremental change POJO for JMAP filtering: handle updates
    
    TODO:
     - Plug it in the event source system (aggregate, cassadra
     event serialization)
     - Conditionally disable incremental change via ENV and
     explain that it should be disable during rolling updates
     of the upgrade.
     - Rebase the subscriber JAMES-3777: subscriber first might
     need to load the aggregate again?
---
 .../api/filtering/impl/IncrementalRuleChange.java  | 26 ++++++++----
 .../james/jmap/api/filtering/RuleFixture.java      |  7 +++-
 .../filtering/impl/IncrementalRuleChangeTest.java  | 47 +++++++++++++---------
 3 files changed, 53 insertions(+), 27 deletions(-)

diff --git a/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/filtering/impl/IncrementalRuleChange.java b/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/filtering/impl/IncrementalRuleChange.java
index 792b0fb8b9..9207663b35 100644
--- a/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/filtering/impl/IncrementalRuleChange.java
+++ b/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/filtering/impl/IncrementalRuleChange.java
@@ -22,8 +22,6 @@ package org.apache.james.jmap.api.filtering.impl;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.Set;
-import java.util.stream.Collectors;
 
 import org.apache.james.eventsourcing.AggregateId;
 import org.apache.james.eventsourcing.Event;
@@ -34,7 +32,6 @@ import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.ImmutableSortedSet;
 import com.google.common.collect.Sets;
 
 public class IncrementalRuleChange implements Event {
@@ -54,6 +51,7 @@ public class IncrementalRuleChange implements Event {
         List<Rule.Id> commonElements = ImmutableList.copyOf(Sets.intersection(idsBefore, idsAfter).immutableCopy());
 
         ImmutableList<Rule.Id> idsAfterList = idsAfter.asList();
+        ImmutableList.Builder<Rule> updatedRules = ImmutableList.builder();
         int prependedItems = 0;
         int postPendedItems = 0;
         boolean inPrepended = true;
@@ -98,8 +96,9 @@ public class IncrementalRuleChange implements Event {
                     return Optional.empty();
                 }
                 if (!beforeIndexed.get(id).equals(afterIndexed.get(id))) {
-                    // Rule content changed
-                    return Optional.empty();
+                    updatedRules.add(afterIndexed.get(id));
+                    position++;
+                    continue;
                 }
                 // All fine
                 position++;
@@ -122,7 +121,7 @@ public class IncrementalRuleChange implements Event {
             .reverse();
 
         return Optional.of(new IncrementalRuleChange(aggregateId, eventId,
-            preprended, postPended, deleted));
+            preprended, postPended, deleted, updatedRules.build()));
     }
 
     private final FilteringAggregateId aggregateId;
@@ -130,13 +129,15 @@ public class IncrementalRuleChange implements Event {
     private final ImmutableList<Rule> rulesPrepended;
     private final ImmutableList<Rule> rulesPostpended;
     private final ImmutableSet<Rule.Id> rulesDeleted;
+    private final ImmutableList<Rule> rulesUpdated;
 
-    public IncrementalRuleChange(FilteringAggregateId aggregateId, EventId eventId, ImmutableList<Rule> rulesPrepended, ImmutableList<Rule> rulesPostpended, ImmutableSet<Rule.Id> rulesDeleted) {
+    public IncrementalRuleChange(FilteringAggregateId aggregateId, EventId eventId, ImmutableList<Rule> rulesPrepended, ImmutableList<Rule> rulesPostpended, ImmutableSet<Rule.Id> rulesDeleted, ImmutableList<Rule> rulesUpdated) {
         this.aggregateId = aggregateId;
         this.eventId = eventId;
         this.rulesPrepended = rulesPrepended;
         this.rulesPostpended = rulesPostpended;
         this.rulesDeleted = rulesDeleted;
+        this.rulesUpdated = rulesUpdated;
     }
 
     @Override
@@ -162,10 +163,17 @@ public class IncrementalRuleChange implements Event {
     }
 
     public ImmutableList<Rule> apply(ImmutableList<Rule> rules) {
+        ImmutableMap<Rule.Id, Rule> indexedUpdates = rulesUpdated.stream()
+            .collect(ImmutableMap.toImmutableMap(
+                Rule::getId,
+                rule -> rule));
+
         return ImmutableList.<Rule>builder()
             .addAll(rulesPrepended)
             .addAll(rules.stream()
                 .filter(rule -> !rulesDeleted.contains(rule.getId()))
+                .map(rule -> Optional.ofNullable(indexedUpdates.get(rule.getId()))
+                    .orElse(rule))
                 .collect(ImmutableList.toImmutableList()))
             .addAll(rulesPostpended)
             .build();
@@ -184,12 +192,13 @@ public class IncrementalRuleChange implements Event {
             Objects.equals(eventId, that.eventId) &&
             Objects.equals(rulesDeleted, that.rulesDeleted) &&
             Objects.equals(rulesPrepended, that.rulesPrepended) &&
+            Objects.equals(rulesUpdated, that.rulesUpdated) &&
             Objects.equals(rulesPostpended, that.rulesPostpended);
     }
 
     @Override
     public final int hashCode() {
-        return Objects.hash(aggregateId, eventId, rulesPrepended, rulesPostpended, rulesDeleted);
+        return Objects.hash(aggregateId, eventId, rulesPrepended, rulesPostpended, rulesDeleted, rulesDeleted);
     }
 
     @Override
@@ -200,6 +209,7 @@ public class IncrementalRuleChange implements Event {
             .add("rulesDeleted", rulesDeleted)
             .add("rulesPrepended", rulesPrepended)
             .add("rulesPostpended", rulesPostpended)
+            .add("rulesUpdated", rulesUpdated)
             .toString();
     }
 }
diff --git a/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/filtering/RuleFixture.java b/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/filtering/RuleFixture.java
index 5166a2cefb..d39bb25d2c 100644
--- a/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/filtering/RuleFixture.java
+++ b/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/filtering/RuleFixture.java
@@ -25,7 +25,12 @@ public interface RuleFixture {
     Rule.Action ACTION = Rule.Action.of(Rule.Action.AppendInMailboxes.withMailboxIds("id-01"));
     Rule.Builder RULE_BUILDER = Rule.builder().name(NAME).condition(CONDITION).action(ACTION);
     Rule RULE_1 = RULE_BUILDER.id(Rule.Id.of("1")).build();
-    Rule RULE_1_MODIFIED = RULE_BUILDER.id(Rule.Id.of("1")).name("newname").build();
+    Rule RULE_1_MODIFIED = Rule.builder()
+        .condition(CONDITION)
+        .action(ACTION)
+        .id(Rule.Id.of("1"))
+        .name("newname")
+        .build();
     Rule RULE_2 = RULE_BUILDER.id(Rule.Id.of("2")).build();
     Rule RULE_3 = RULE_BUILDER.id(Rule.Id.of("3")).build();
 
diff --git a/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/filtering/impl/IncrementalRuleChangeTest.java b/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/filtering/impl/IncrementalRuleChangeTest.java
index 6fa4c3fd09..fd91990aa4 100644
--- a/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/filtering/impl/IncrementalRuleChangeTest.java
+++ b/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/filtering/impl/IncrementalRuleChangeTest.java
@@ -43,7 +43,7 @@ class IncrementalRuleChangeTest {
             ImmutableList.of(RULE_1),
             ImmutableList.of()))
             .contains(new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(), ImmutableList.of(),
-                ImmutableSet.of(RULE_1.getId())));
+                ImmutableSet.of(RULE_1.getId()), ImmutableList.of()));
     }
 
     @Test
@@ -52,7 +52,7 @@ class IncrementalRuleChangeTest {
             ImmutableList.of(RULE_1, RULE_2),
             ImmutableList.of(RULE_2)))
             .contains(new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(), ImmutableList.of(),
-                ImmutableSet.of(RULE_1.getId())));
+                ImmutableSet.of(RULE_1.getId()), ImmutableList.of()));
     }
 
     @Test
@@ -61,7 +61,7 @@ class IncrementalRuleChangeTest {
             ImmutableList.of(RULE_1, RULE_2, RULE_3),
             ImmutableList.of(RULE_1, RULE_3)))
             .contains(new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(), ImmutableList.of(),
-                ImmutableSet.of(RULE_2.getId())));
+                ImmutableSet.of(RULE_2.getId()), ImmutableList.of()));
     }
 
     @Test
@@ -69,7 +69,7 @@ class IncrementalRuleChangeTest {
         assertThat(IncrementalRuleChange.ofDiff(AGGREGATE_ID, EVENT_ID,
             ImmutableList.of(RULE_1, RULE_2, RULE_3),
             ImmutableList.of(RULE_1, RULE_2, RULE_3)))
-            .contains(new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(), ImmutableList.of(), ImmutableSet.of()));
+            .contains(new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(), ImmutableList.of(), ImmutableSet.of(), ImmutableList.of()));
     }
 
     @Test
@@ -93,7 +93,7 @@ class IncrementalRuleChangeTest {
         assertThat(IncrementalRuleChange.ofDiff(AGGREGATE_ID, EVENT_ID,
             ImmutableList.of(RULE_1, RULE_2),
             ImmutableList.of(RULE_1, RULE_2, RULE_3)))
-            .contains(new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(), ImmutableList.of(RULE_3), ImmutableSet.of()));
+            .contains(new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(), ImmutableList.of(RULE_3), ImmutableSet.of(), ImmutableList.of()));
     }
 
     @Test
@@ -101,7 +101,7 @@ class IncrementalRuleChangeTest {
         assertThat(IncrementalRuleChange.ofDiff(AGGREGATE_ID, EVENT_ID,
             ImmutableList.of(RULE_1, RULE_2),
             ImmutableList.of(RULE_3, RULE_1, RULE_2)))
-            .contains(new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(RULE_3), ImmutableList.of(), ImmutableSet.of()));
+            .contains(new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(RULE_3), ImmutableList.of(), ImmutableSet.of(), ImmutableList.of()));
     }
 
     @Test
@@ -109,7 +109,7 @@ class IncrementalRuleChangeTest {
         assertThat(IncrementalRuleChange.ofDiff(AGGREGATE_ID, EVENT_ID,
             ImmutableList.of(RULE_1),
             ImmutableList.of(RULE_3, RULE_1, RULE_2)))
-            .contains(new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(RULE_3), ImmutableList.of(RULE_2), ImmutableSet.of()));
+            .contains(new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(RULE_3), ImmutableList.of(RULE_2), ImmutableSet.of(), ImmutableList.of()));
     }
 
     @Test
@@ -117,22 +117,23 @@ class IncrementalRuleChangeTest {
         assertThat(IncrementalRuleChange.ofDiff(AGGREGATE_ID, EVENT_ID,
             ImmutableList.of(RULE_1),
             ImmutableList.of(RULE_3, RULE_2)))
-            .contains(new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(RULE_3, RULE_2), ImmutableList.of(), ImmutableSet.of(RULE_1.getId())));
+            .contains(new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(RULE_3, RULE_2), ImmutableList.of(), ImmutableSet.of(RULE_1.getId()), ImmutableList.of()));
     }
 
     @Test
-    void ruleModificationIsNotManagedByIncrement() {
+    void ruleModification() {
         assertThat(IncrementalRuleChange.ofDiff(AGGREGATE_ID, EVENT_ID,
             ImmutableList.of(RULE_1),
             ImmutableList.of(RULE_1_MODIFIED)))
-            .isEmpty();
+            .contains(new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(), ImmutableList.of(), ImmutableSet.of(),
+                ImmutableList.of(RULE_1_MODIFIED)));
     }
 
     @Test
     void removingOneRuleShouldBeWellApplied() {
         ImmutableList<Rule> origin = ImmutableList.of(RULE_1);
         IncrementalRuleChange incrementalChange = new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(), ImmutableList.of(),
-            ImmutableSet.of(RULE_1.getId()));
+            ImmutableSet.of(RULE_1.getId()), ImmutableList.of());
         assertThat(incrementalChange.apply(origin))
             .isEqualTo(ImmutableList.of());
     }
@@ -141,7 +142,7 @@ class IncrementalRuleChangeTest {
     void removingOneRuleOutOfTwoShouldBeWellApplied() {
         ImmutableList<Rule> origin = ImmutableList.of(RULE_1, RULE_2);
         IncrementalRuleChange incrementalChange = new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(), ImmutableList.of(),
-            ImmutableSet.of(RULE_1.getId()));
+            ImmutableSet.of(RULE_1.getId()), ImmutableList.of());
         assertThat(incrementalChange.apply(origin))
             .isEqualTo(ImmutableList.of(RULE_2));
     }
@@ -150,7 +151,7 @@ class IncrementalRuleChangeTest {
     void removingMiddleRuleShouldBeWellApplied() {
         ImmutableList<Rule> origin = ImmutableList.of(RULE_1, RULE_2, RULE_3);
         IncrementalRuleChange incrementalChange = new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(), ImmutableList.of(),
-            ImmutableSet.of(RULE_2.getId()));
+            ImmutableSet.of(RULE_2.getId()), ImmutableList.of());
         assertThat(incrementalChange.apply(origin))
             .isEqualTo(ImmutableList.of(RULE_1, RULE_3));
     }
@@ -159,7 +160,7 @@ class IncrementalRuleChangeTest {
     void noopShouldBeWellApplied() {
         ImmutableList<Rule> origin = ImmutableList.of(RULE_1, RULE_2, RULE_3);
 
-        IncrementalRuleChange incrementalRuleChange = new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(), ImmutableList.of(), ImmutableSet.of());
+        IncrementalRuleChange incrementalRuleChange = new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(), ImmutableList.of(), ImmutableSet.of(), ImmutableList.of());
 
         assertThat(incrementalRuleChange.apply(origin))
             .isEqualTo(origin);
@@ -169,7 +170,7 @@ class IncrementalRuleChangeTest {
     void postPendingOneRuleShouldBeWellApplied() {
         ImmutableList<Rule> origin = ImmutableList.of(RULE_1, RULE_2);
 
-        IncrementalRuleChange incrementalRuleChange = new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(), ImmutableList.of(RULE_3), ImmutableSet.of());
+        IncrementalRuleChange incrementalRuleChange = new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(), ImmutableList.of(RULE_3), ImmutableSet.of(), ImmutableList.of());
 
         assertThat(incrementalRuleChange.apply(origin))
             .isEqualTo(ImmutableList.of(RULE_1, RULE_2, RULE_3));
@@ -179,7 +180,7 @@ class IncrementalRuleChangeTest {
     void prependingOneRuleShouldBeWellApplied() {
         ImmutableList<Rule> origin = ImmutableList.of(RULE_1, RULE_2);
 
-        IncrementalRuleChange incrementalRuleChange = new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(RULE_3), ImmutableList.of(), ImmutableSet.of());
+        IncrementalRuleChange incrementalRuleChange = new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(RULE_3), ImmutableList.of(), ImmutableSet.of(), ImmutableList.of());
 
         assertThat(incrementalRuleChange.apply(origin))
             .isEqualTo(ImmutableList.of(RULE_3, RULE_1, RULE_2));
@@ -189,7 +190,7 @@ class IncrementalRuleChangeTest {
     void prependingAndPostpendingShouldBeWellApplied() {
         ImmutableList<Rule> origin = ImmutableList.of(RULE_1);
 
-        IncrementalRuleChange incrementalRuleChange = new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(RULE_3), ImmutableList.of(RULE_2), ImmutableSet.of());
+        IncrementalRuleChange incrementalRuleChange = new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(RULE_3), ImmutableList.of(RULE_2), ImmutableSet.of(), ImmutableList.of());
 
         assertThat(incrementalRuleChange.apply(origin))
             .isEqualTo(ImmutableList.of(RULE_3, RULE_1, RULE_2));
@@ -199,9 +200,19 @@ class IncrementalRuleChangeTest {
     void prependingAndPostpendingAndRemovalShouldBeWellApplied() {
         ImmutableList<Rule> origin = ImmutableList.of(RULE_1);
 
-        IncrementalRuleChange incrementalRuleChange = new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(RULE_3), ImmutableList.of(RULE_2), ImmutableSet.of(RULE_1.getId()));
+        IncrementalRuleChange incrementalRuleChange = new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(RULE_3), ImmutableList.of(RULE_2), ImmutableSet.of(RULE_1.getId()), ImmutableList.of());
 
         assertThat(incrementalRuleChange.apply(origin))
             .isEqualTo(ImmutableList.of(RULE_3, RULE_2));
     }
+
+    @Test
+    void ruleModificationShouldBeWellApplied() {
+        ImmutableList<Rule> origin = ImmutableList.of(RULE_1);
+        IncrementalRuleChange incrementalRuleChange = new IncrementalRuleChange(AGGREGATE_ID, EVENT_ID, ImmutableList.of(),
+            ImmutableList.of(), ImmutableSet.of(), ImmutableList.of(RULE_1_MODIFIED));
+
+        assertThat(incrementalRuleChange.apply(origin))
+            .isEqualTo(ImmutableList.of(RULE_1_MODIFIED));
+    }
 }
\ No newline at end of file


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