You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@directory.apache.org by bd...@apache.org on 2022/09/15 23:20:37 UTC
[directory-scimple] branch rm-dangling-filter created (now 125dbb6c)
This is an automated email from the ASF dual-hosted git repository.
bdemers pushed a change to branch rm-dangling-filter
in repository https://gitbox.apache.org/repos/asf/directory-scimple.git
at 125dbb6c Removed dangling logical filter methods
This branch includes the following new commits:
new 125dbb6c Removed dangling logical filter methods
The 1 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.
[directory-scimple] 01/01: Removed dangling logical filter methods
Posted by bd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
bdemers pushed a commit to branch rm-dangling-filter
in repository https://gitbox.apache.org/repos/asf/directory-scimple.git
commit 125dbb6ccfe8e8db276d438f3140583501e382a0
Author: Brian Demers <bd...@apache.org>
AuthorDate: Sat Sep 10 00:07:57 2022 -0400
Removed dangling logical filter methods
IMHO, These methods were esthetically pleasing, but they required complex logic, and make it easy for the user to do something incorrect.
Deleting them keeps things simple
---
.../spec/filter/ComplexLogicalFilterBuilder.java | 67 +++++++++---------
.../directory/scim/spec/filter/FilterBuilder.java | 4 --
.../spec/filter/SimpleLogicalFilterBuilder.java | 73 +------------------
.../scim/spec/filter/FilterBuilderPresentTest.java | 6 +-
.../scim/spec/filter/FilterBuilderTest.java | 82 +++++++++++++---------
5 files changed, 82 insertions(+), 150 deletions(-)
diff --git a/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/ComplexLogicalFilterBuilder.java b/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/ComplexLogicalFilterBuilder.java
index 7cae6afc..b2118088 100644
--- a/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/ComplexLogicalFilterBuilder.java
+++ b/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/ComplexLogicalFilterBuilder.java
@@ -4,57 +4,56 @@ abstract class ComplexLogicalFilterBuilder extends SimpleLogicalFilterBuilder {
@Override
public FilterBuilder or(FilterExpression fe1) {
- if (filterExpression instanceof AttributeComparisonExpression) {
- LogicalExpression logicalExpression = new LogicalExpression();
- logicalExpression.setLeft(groupIfNeeded(filterExpression));
- logicalExpression.setRight(groupIfNeeded(fe1));
- logicalExpression.setOperator(LogicalOperator.OR);
- filterExpression = logicalExpression;
- return this;
+ if (filterExpression == null) {
+ throw new IllegalStateException("Cannot call or(Filter), call or(Filter, Filter) instead.");
}
-
LogicalExpression logicalExpression = new LogicalExpression();
- logicalExpression.setLeft(fe1);
+ logicalExpression.setLeft(groupIfNeeded(filterExpression));
+ logicalExpression.setRight(groupIfNeeded(fe1));
logicalExpression.setOperator(LogicalOperator.OR);
-
- return handleLogicalExpression(logicalExpression, LogicalOperator.OR);
+ filterExpression = logicalExpression;
+ return this;
}
@Override
public FilterBuilder or(FilterExpression fe1, FilterExpression fe2) {
- LogicalExpression logicalExpression = new LogicalExpression();
- logicalExpression.setLeft(groupIfNeeded(fe1));
- logicalExpression.setRight(groupIfNeeded(fe2));
- logicalExpression.setOperator(LogicalOperator.OR);
-
- return handleLogicalExpression(logicalExpression, LogicalOperator.OR);
+ if (filterExpression == null) {
+ LogicalExpression logicalExpression = new LogicalExpression();
+ logicalExpression.setLeft(groupIfNeeded(fe1));
+ logicalExpression.setRight(groupIfNeeded(fe2));
+ logicalExpression.setOperator(LogicalOperator.OR);
+ filterExpression = logicalExpression;
+ } else {
+ this.or(fe1).or(fe2);
+ }
+ return this;
}
@Override
public FilterBuilder and(FilterExpression fe1, FilterExpression fe2) {
- LogicalExpression logicalExpression = new LogicalExpression();
- logicalExpression.setLeft(groupIfNeeded(fe1));
- logicalExpression.setRight(groupIfNeeded(fe2));
- logicalExpression.setOperator(LogicalOperator.AND);
-
- return handleLogicalExpression(logicalExpression, LogicalOperator.AND);
+ if (filterExpression == null) {
+ LogicalExpression logicalExpression = new LogicalExpression();
+ logicalExpression.setLeft(groupIfNeeded(fe1));
+ logicalExpression.setRight(groupIfNeeded(fe2));
+ logicalExpression.setOperator(LogicalOperator.AND);
+ filterExpression = logicalExpression;
+ } else {
+ this.and(fe1).and(fe2);
+ }
+ return this;
}
@Override
public FilterBuilder and(FilterExpression fe1) {
- if (filterExpression instanceof AttributeComparisonExpression) {
- LogicalExpression logicalExpression = new LogicalExpression();
- logicalExpression.setLeft(filterExpression);
- logicalExpression.setRight(groupIfNeeded(fe1));
- logicalExpression.setOperator(LogicalOperator.AND);
- filterExpression = logicalExpression;
- return this;
+ if (filterExpression == null) {
+ throw new IllegalStateException("Cannot call and(Filter), call and(Filter, Filter) instead.");
}
-
+
LogicalExpression logicalExpression = new LogicalExpression();
- logicalExpression.setLeft(fe1);
+ logicalExpression.setLeft(filterExpression);
+ logicalExpression.setRight(groupIfNeeded(fe1));
logicalExpression.setOperator(LogicalOperator.AND);
-
- return handleLogicalExpression(logicalExpression, LogicalOperator.AND);
+ filterExpression = logicalExpression;
+ return this;
}
}
diff --git a/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/FilterBuilder.java b/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/FilterBuilder.java
index 3c569cc0..aa2fb3fd 100644
--- a/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/FilterBuilder.java
+++ b/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/FilterBuilder.java
@@ -7,8 +7,6 @@ import java.util.function.UnaryOperator;
public interface FilterBuilder {
- FilterBuilder and();
-
FilterBuilder and(FilterExpression fe1);
default FilterBuilder and(UnaryOperator<FilterBuilder> filter) {
@@ -29,8 +27,6 @@ public interface FilterBuilder {
return and(left.apply(FilterBuilder.create()).build(), right.apply(FilterBuilder.create()).build());
}
- FilterBuilder or();
-
FilterBuilder or(FilterExpression fe1);
default FilterBuilder or(Filter filter) {
diff --git a/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/SimpleLogicalFilterBuilder.java b/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/SimpleLogicalFilterBuilder.java
index 8f871363..95c5bd59 100644
--- a/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/SimpleLogicalFilterBuilder.java
+++ b/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/SimpleLogicalFilterBuilder.java
@@ -7,83 +7,12 @@ abstract class SimpleLogicalFilterBuilder implements FilterBuilder {
protected FilterExpression filterExpression;
- @Override
- public FilterBuilder and() {
- if (filterExpression == null) {
- throw new IllegalStateException();
- }
-
- LogicalExpression logicalExpression = new LogicalExpression();
- logicalExpression.setLeft(groupIfNeeded(filterExpression));
- logicalExpression.setOperator(LogicalOperator.AND);
- filterExpression = logicalExpression;
-
- return this;
- }
-
- @Override
- public FilterBuilder or() {
- if (filterExpression == null) {
- throw new IllegalStateException();
- }
-
- LogicalExpression logicalExpression = new LogicalExpression();
- logicalExpression.setLeft(groupIfNeeded(filterExpression));
- logicalExpression.setOperator(LogicalOperator.OR);
- filterExpression = logicalExpression;
-
- return this;
- }
-
static FilterExpression groupIfNeeded(FilterExpression filterExpression) {
return (filterExpression instanceof LogicalExpression)
? new GroupExpression(false, filterExpression)
: filterExpression;
}
- protected FilterBuilder handleLogicalExpression(LogicalExpression expression, LogicalOperator operator) {
- log.info("In handleLogicalExpression");
- if (filterExpression == null) {
- filterExpression = expression;
- } else if (filterExpression instanceof AttributeComparisonExpression) {
- log.info("Adding a logical expression as the new root");
-
- log.info("Setting as left: " + filterExpression.toFilter());
- expression.setLeft(filterExpression);
- log.info("Setting as right: " + expression.toFilter());
-
- filterExpression = expression;
- } else if (filterExpression instanceof LogicalExpression) {
- log.info("filter exression is a logical expression");
- LogicalExpression le = (LogicalExpression) filterExpression;
-
- if (le.getLeft() == null) {
- log.info("Setting left to: " + expression.toFilter());
- le.setLeft(expression);
- } else if (le.getRight() == null) {
- log.info("Setting right to: " + expression.toFilter());
- le.setRight(expression);
- } else {
- log.info("The current base is complete, raising up one level");
- LogicalExpression newRoot = new LogicalExpression();
- log.info("Setting left to: " + expression);
- newRoot.setLeft(expression);
- filterExpression = newRoot;
- }
- } else if (filterExpression instanceof GroupExpression) {
- log.info("Found group expression");
- LogicalExpression newRoot = new LogicalExpression();
- newRoot.setLeft(filterExpression);
- newRoot.setRight(expression);
- newRoot.setOperator(operator);
- filterExpression = newRoot;
- }
-
- log.info("New filter expression: " + filterExpression.toFilter());
-
- return this;
- }
-
protected void handleComparisonExpression(FilterExpression expression) {
if (expression == null) {
@@ -94,7 +23,7 @@ abstract class SimpleLogicalFilterBuilder implements FilterBuilder {
filterExpression = expression;
} else {
if (!(filterExpression instanceof LogicalExpression)) {
- throw new IllegalStateException();
+ throw new IllegalStateException("Invalid filter state");
}
LogicalExpression le = (LogicalExpression) filterExpression;
diff --git a/scim-spec/scim-spec-schema/src/test/java/org/apache/directory/scim/spec/filter/FilterBuilderPresentTest.java b/scim-spec/scim-spec-schema/src/test/java/org/apache/directory/scim/spec/filter/FilterBuilderPresentTest.java
index f9cb4978..0b6e5c9a 100644
--- a/scim-spec/scim-spec-schema/src/test/java/org/apache/directory/scim/spec/filter/FilterBuilderPresentTest.java
+++ b/scim-spec/scim-spec-schema/src/test/java/org/apache/directory/scim/spec/filter/FilterBuilderPresentTest.java
@@ -20,9 +20,6 @@
package org.apache.directory.scim.spec.filter;
import lombok.extern.slf4j.Slf4j;
-import org.apache.directory.scim.spec.filter.Filter;
-import org.apache.directory.scim.spec.filter.FilterBuilder;
-import org.apache.directory.scim.spec.filter.FilterParseException;
import org.junit.jupiter.api.Test;
import static org.assertj.core.api.Assertions.assertThat;
@@ -41,8 +38,7 @@ public class FilterBuilderPresentTest {
public void testStartsWith() throws FilterParseException {
Filter filter = FilterBuilder.create()
.present("address.streetAddress")
- .and()
- .equalTo("address.region", "CA")
+ .and(f -> f.equalTo("address.region", "CA"))
.build();
Filter expected = new Filter("address.streetAddress PR AND address.region EQ \"CA\"");
assertThat(filter).isEqualTo(expected);
diff --git a/scim-spec/scim-spec-schema/src/test/java/org/apache/directory/scim/spec/filter/FilterBuilderTest.java b/scim-spec/scim-spec-schema/src/test/java/org/apache/directory/scim/spec/filter/FilterBuilderTest.java
index 641fad00..e044cdad 100644
--- a/scim-spec/scim-spec-schema/src/test/java/org/apache/directory/scim/spec/filter/FilterBuilderTest.java
+++ b/scim-spec/scim-spec-schema/src/test/java/org/apache/directory/scim/spec/filter/FilterBuilderTest.java
@@ -20,9 +20,6 @@
package org.apache.directory.scim.spec.filter;
import lombok.extern.slf4j.Slf4j;
-import org.apache.directory.scim.spec.filter.Filter;
-import org.apache.directory.scim.spec.filter.FilterBuilder;
-import org.apache.directory.scim.spec.filter.FilterParseException;
import org.junit.jupiter.api.Test;
import java.io.UnsupportedEncodingException;
@@ -37,8 +34,7 @@ public class FilterBuilderTest {
public void testSimpleAnd() throws FilterParseException {
Filter filter = FilterBuilder.create()
.equalTo("name.givenName", "Bilbo")
- .and()
- .equalTo("name.familyName", "Baggins")
+ .and(r -> r.equalTo("name.familyName", "Baggins"))
.build();
assertThat(filter).isEqualTo(new Filter("name.givenName EQ \"Bilbo\" AND name.familyName EQ \"Baggins\""));
}
@@ -47,8 +43,8 @@ public class FilterBuilderTest {
public void testSimpleOr() throws FilterParseException {
Filter filter = FilterBuilder.create()
.equalTo("name.givenName", "Bilbo")
- .or()
- .equalTo("name.familyName", "Baggins").build();
+ .or(r -> r.equalTo("name.familyName", "Baggins"))
+ .build();
assertThat(filter).isEqualTo(new Filter("name.givenName EQ \"Bilbo\" OR name.familyName EQ \"Baggins\""));
}
@@ -58,14 +54,13 @@ public class FilterBuilderTest {
Filter filter = FilterBuilder.create()
.equalTo("name.givenName", "Bilbo")
- .or()
- .equalTo("name.givenName", "Frodo")
- .and()
- .equalTo("name.familyName", "Baggins").build();
+ .or(f -> f.equalTo("name.givenName", "Frodo"))
+ .and(f -> f.equalTo("name.familyName", "Baggins"))
+ .build();
Filter expected = new Filter("(name.givenName EQ \"Bilbo\" OR name.givenName EQ \"Frodo\") AND name.familyName EQ \"Baggins\"");
- assertThat(filter).isEqualTo(expected);
+ assertThat(filter.getFilter()).isEqualTo(expected.getFilter());
}
@Test
@@ -75,8 +70,7 @@ public class FilterBuilderTest {
.equalTo("name.givenName", "Bilbo")
.and(FilterBuilder.create()
.equalTo("name.givenName", "Frodo")
- .and()
- .equalTo("name.familyName", "Baggins")
+ .and(f -> f.equalTo("name.familyName", "Baggins"))
.build())
.build();
@@ -91,8 +85,7 @@ public class FilterBuilderTest {
.equalTo("name.givenName", "Bilbo")
.or(FilterBuilder.create()
.equalTo("name.givenName", "Frodo")
- .and()
- .equalTo("name.familyName", "Baggins")
+ .and(f -> f.equalTo("name.familyName", "Baggins"))
.build())
.build();
@@ -106,36 +99,30 @@ public class FilterBuilderTest {
FilterBuilder b1 = FilterBuilder.create()
.equalTo("name.givenName", "Bilbo")
- .or()
- .equalTo("name.givenName", "Frodo")
- .and()
- .equalTo("name.familyName", "Baggins");
+ .or(f -> f.equalTo("name.givenName", "Frodo"))
+ .and(f -> f.equalTo("name.familyName", "Baggins"));
FilterBuilder b2 = FilterBuilder.create().equalTo("address.streetAddress", "Underhill")
- .or()
- .equalTo("address.streetAddress", "Overhill")
- .and()
- .equalTo("address.postalCode", "16803");
+ .or(f -> f.equalTo("address.streetAddress", "Overhill"))
+ .and(f -> f.equalTo("address.postalCode", "16803"));
Filter filter = FilterBuilder.create().and(b1.build(), b2.build()).build();
Filter expected = new Filter("((name.givenName EQ \"Bilbo\" OR name.givenName EQ \"Frodo\") AND name.familyName EQ \"Baggins\") AND ((address.streetAddress EQ \"Underhill\" OR address.streetAddress EQ \"Overhill\") AND address.postalCode EQ \"16803\")");
- assertThat(filter).isEqualTo(expected);
+ assertThat(filter.getFilter()).isEqualTo(expected.getFilter());
}
@Test
public void testNot() throws FilterParseException {
FilterBuilder b1 = FilterBuilder.create()
.equalTo("name.givenName", "Bilbo")
- .or()
- .equalTo("name.givenName", "Frodo")
- .and()
- .equalTo("name.familyName", "Baggins");
+ .or(f -> f.equalTo("name.givenName", "Frodo"))
+ .and(f -> f.equalTo("name.familyName", "Baggins"));
Filter filter = FilterBuilder.create().not(b1.build()).build();
Filter expected = new Filter("NOT((name.givenName EQ \"Bilbo\" OR name.givenName EQ \"Frodo\") AND name.familyName EQ \"Baggins\")");
- assertThat(filter).isEqualTo(expected);
+ assertThat(filter.getFilter()).isEqualTo(expected.getFilter());
}
@Test
@@ -157,10 +144,8 @@ public class FilterBuilderTest {
FilterBuilder b1 = FilterBuilder.create()
.equalTo("name.givenName", "Bilbo")
- .or()
- .equalTo("name.givenName", "Frodo")
- .and()
- .equalTo("name.familyName", "Baggins");
+ .or(f -> f.equalTo("name.givenName", "Frodo"))
+ .and(f -> f.equalTo("name.familyName", "Baggins"));
FilterBuilder b2 = FilterBuilder.create().attributeHas("address", b1.build());
@@ -176,11 +161,38 @@ public class FilterBuilderTest {
@Test
public void testAttributeContainsDeeplyEmbedded() throws FilterParseException {
- FilterBuilder b1 = FilterBuilder.create().equalTo("name.givenName", "Bilbo").or().equalTo("name.givenName", "Frodo").and().equalTo("name.familyName", "Baggins");
+ FilterBuilder b1 = FilterBuilder.create()
+ .equalTo("name.givenName", "Bilbo")
+ .or(f -> f.equalTo("name.givenName", "Frodo"))
+ .and(f -> f.equalTo("name.familyName", "Baggins"));
FilterBuilder b2 = FilterBuilder.create().attributeHas("address", b1.build());
FilterBuilder b3 = FilterBuilder.create().equalTo("name.giveName", "Gandalf").and(b2.build());
FilterBuilder b4 = FilterBuilder.create().attributeHas("address", b3.build());
assertThrows(FilterParseException.class, () -> new Filter( b4.toString()));
}
+
+ @Test
+ public void complexOrPresent() throws FilterParseException {
+ Filter filter = FilterBuilder.create().or(l -> l.present("name.givenName"), r -> r.present("name.familyName")).build();
+ assertThat(filter).isEqualTo(new Filter("name.givenName PR OR name.familyName PR"));
+ }
+
+ @Test
+ public void complexPresentThenOr() throws FilterParseException {
+ Filter filter = FilterBuilder.create().present("name.givenName").or(f -> f.present("name.familyName")).build();
+ assertThat(filter).isEqualTo(new Filter("name.givenName PR OR name.familyName PR"));
+ }
+
+ @Test
+ public void expressionThenTwoAnds() {
+ Filter filter = FilterBuilder.create().present("name.givenName").and(l -> l.present("name.familyName"), r -> r.present("name.middleName")).build();
+ assertThat(filter.getFilter()).isEqualTo("(name.givenName PR AND name.familyName PR) AND name.middleName PR");
+ }
+
+ @Test
+ public void expressionThenTwoOrs() {
+ Filter filter = FilterBuilder.create().present("name.givenName").or(l -> l.present("name.familyName"), r -> r.present("name.middleName")).build();
+ assertThat(filter.getFilter()).isEqualTo("(name.givenName PR OR name.familyName PR) OR name.middleName PR");
+ }
}