You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2021/04/01 04:57:41 UTC
[GitHub] [james-project] chibenwa commented on a change in pull request #346: JAMES-3529 Validate version when set Filter
chibenwa commented on a change in pull request #346:
URL: https://github.com/apache/james-project/pull/346#discussion_r605374428
##########
File path: server/data/data-jmap/src/test/java/org/apache/james/jmap/api/filtering/FilteringManagementContract.java
##########
@@ -76,8 +76,8 @@ default void listingRulesShouldReturnDefinedRules(EventStore eventStore) {
default void listingRulesShouldReturnLastDefinedRules(EventStore eventStore) {
FilteringManagement testee = instantiateFilteringManagement(eventStore);
- Mono.from(testee.defineRulesForUser(USERNAME, RULE_1, RULE_2)).block();
- Mono.from(testee.defineRulesForUser(USERNAME, RULE_2, RULE_1)).block();
+ Mono.from(testee.defineRulesForUser(USERNAME, Optional.empty(), RULE_1, RULE_2)).block();
+ Mono.from(testee.defineRulesForUser(USERNAME, Optional.of(new Version(0)), RULE_2, RULE_1)).block();
Review comment:
Optional.empty() here too
##########
File path: server/data/data-jmap/src/test/java/org/apache/james/jmap/api/filtering/FilteringManagementContract.java
##########
@@ -122,8 +122,8 @@ default void definingRulesShouldKeepOrdering(EventStore eventStore) {
default void definingEmptyRuleListShouldRemoveExistingRules(EventStore eventStore) {
FilteringManagement testee = instantiateFilteringManagement(eventStore);
- Mono.from(testee.defineRulesForUser(USERNAME, RULE_3, RULE_2, RULE_1)).block();
- Mono.from(testee.clearRulesForUser(USERNAME)).block();
+ Mono.from(testee.defineRulesForUser(USERNAME, Optional.empty(), RULE_3, RULE_2, RULE_1)).block();
+ Mono.from(testee.clearRulesForUser(USERNAME, Optional.of(new Version(0)))).block();
Review comment:
Optional.empty() here too?
##########
File path: server/data/data-jmap/src/test/java/org/apache/james/jmap/api/filtering/FilteringManagementContract.java
##########
@@ -133,11 +133,37 @@ default void definingEmptyRuleListShouldRemoveExistingRules(EventStore eventStor
default void allFieldsAndComparatorShouldWellBeStored(EventStore eventStore) {
FilteringManagement testee = instantiateFilteringManagement(eventStore);
- Mono.from(testee.defineRulesForUser(USERNAME, RULE_FROM, RULE_RECIPIENT, RULE_SUBJECT, RULE_TO, RULE_1)).block();
-
+ Mono.from(testee.defineRulesForUser(USERNAME, Optional.empty(), RULE_FROM, RULE_RECIPIENT, RULE_SUBJECT, RULE_TO, RULE_1)).block();
assertThat(Mono.from(testee.listRulesForUser(USERNAME)).block())
.isEqualTo(new Rules(ImmutableList.of(RULE_FROM, RULE_RECIPIENT, RULE_SUBJECT, RULE_TO, RULE_1), new Version(0)));
}
+ @Test
+ default void setRulesWithEmptyVersionShouldSucceed(EventStore eventStore) {
+ FilteringManagement testee = instantiateFilteringManagement(eventStore);
+
+ assertThat(Mono.from(testee.defineRulesForUser(USERNAME, Optional.empty(), RULE_3, RULE_2, RULE_1)).block())
+ .isEqualTo(new Version(0));
+ }
+
+ @Test
+ default void modifyExistingRulesWithWrongCurrentVersionShouldFail(EventStore eventStore) {
+ FilteringManagement testee = instantiateFilteringManagement(eventStore);
+
+ Mono.from(testee.defineRulesForUser(USERNAME, Optional.empty(), RULE_3, RULE_2, RULE_1)).block();
+
+ assertThatThrownBy(() -> Mono.from(testee.defineRulesForUser(USERNAME, Optional.of(new Version(1)), RULE_2, RULE_1)).block())
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ default void modifyExistingRulesWithRightVersionShouldSucceed(EventStore eventStore) {
+ FilteringManagement testee = instantiateFilteringManagement(eventStore);
+
+ Mono.from(testee.defineRulesForUser(USERNAME, Optional.empty(), RULE_3, RULE_2, RULE_1)).block();
+
+ assertThat(Mono.from(testee.defineRulesForUser(USERNAME, Optional.of(new Version(0)), RULE_3, RULE_2)).block())
+ .isEqualTo(new Version(1));
+ }
Review comment:
I see a lot of missing test before I can give my trust to such a piece of code...
- Given say a rules with version=1 Am I able to update it specifying ifInState=1 ?
- Can I use ifInState=INITIAL when no state is defined ?
- Does using ifInState=INITIAL fails when a state is defined?
- Is idInState=1 is well rejected when no rules are persisted yet?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org