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 <btell...@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-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org