From 0e5a584277b90b5126be4fd7dba725337c79a38f Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 18 Feb 2020 09:33:31 +0100 Subject: [PATCH] #289 - Add support for Criteria composition. We now support composition of Criteria objects to create a Criteria from one or more top-level criteria and to compose nested AND/OR Criteria objects: Criteria.where("name").is("Foo")).and(Criteria.where("name").is("Bar").or("age") .lessThan(49).or(Criteria.where("name").not("Bar").and("age").greaterThan(49)) Original pull request: #307. --- .../r2dbc/core/DefaultStatementMapper.java | 6 +- .../data/r2dbc/core/StatementMapper.java | 18 +- .../data/r2dbc/query/Criteria.java | 213 ++++++++++++++++-- .../data/r2dbc/query/QueryMapper.java | 89 +++++++- .../data/r2dbc/query/CriteriaUnitTests.java | 55 +++++ .../r2dbc/query/QueryMapperUnitTests.java | 70 ++++++ 6 files changed, 416 insertions(+), 35 deletions(-) diff --git a/src/main/java/org/springframework/data/r2dbc/core/DefaultStatementMapper.java b/src/main/java/org/springframework/data/r2dbc/core/DefaultStatementMapper.java index 2ccc8fc7..52a29e05 100644 --- a/src/main/java/org/springframework/data/r2dbc/core/DefaultStatementMapper.java +++ b/src/main/java/org/springframework/data/r2dbc/core/DefaultStatementMapper.java @@ -88,7 +88,7 @@ class DefaultStatementMapper implements StatementMapper { BindMarkers bindMarkers = this.dialect.getBindMarkersFactory().create(); Bindings bindings = Bindings.empty(); - if (selectSpec.getCriteria() != null) { + if (!selectSpec.getCriteria().isEmpty()) { BoundCondition mappedObject = this.updateMapper.getMappedObject(bindMarkers, selectSpec.getCriteria(), table, entity); @@ -203,7 +203,7 @@ class DefaultStatementMapper implements StatementMapper { Update update; - if (updateSpec.getCriteria() != null) { + if (!updateSpec.getCriteria().isEmpty()) { BoundCondition boundCondition = this.updateMapper.getMappedObject(bindMarkers, updateSpec.getCriteria(), table, entity); @@ -237,7 +237,7 @@ class DefaultStatementMapper implements StatementMapper { Bindings bindings = Bindings.empty(); Delete delete; - if (deleteSpec.getCriteria() != null) { + if (!deleteSpec.getCriteria().isEmpty()) { BoundCondition boundCondition = this.updateMapper.getMappedObject(bindMarkers, deleteSpec.getCriteria(), table, entity); diff --git a/src/main/java/org/springframework/data/r2dbc/core/StatementMapper.java b/src/main/java/org/springframework/data/r2dbc/core/StatementMapper.java index 88495514..a9c29f65 100644 --- a/src/main/java/org/springframework/data/r2dbc/core/StatementMapper.java +++ b/src/main/java/org/springframework/data/r2dbc/core/StatementMapper.java @@ -185,7 +185,7 @@ public interface StatementMapper { private final Table table; private final List projectedFields; private final List selectList; - private final @Nullable Criteria criteria; + private final Criteria criteria; private final Sort sort; private final long offset; private final int limit; @@ -219,7 +219,7 @@ public interface StatementMapper { * @since 1.1 */ public static SelectSpec create(SqlIdentifier table) { - return new SelectSpec(Table.create(table), Collections.emptyList(), Collections.emptyList(), null, + return new SelectSpec(Table.create(table), Collections.emptyList(), Collections.emptyList(), Criteria.empty(), Sort.unsorted(), -1, -1); } @@ -463,9 +463,9 @@ public interface StatementMapper { @Nullable private final Update update; - private final @Nullable Criteria criteria; + private final Criteria criteria; - protected UpdateSpec(SqlIdentifier table, @Nullable Update update, @Nullable Criteria criteria) { + protected UpdateSpec(SqlIdentifier table, @Nullable Update update, Criteria criteria) { this.table = table; this.update = update; @@ -490,7 +490,7 @@ public interface StatementMapper { * @since 1.1 */ public static UpdateSpec create(SqlIdentifier table, Update update) { - return new UpdateSpec(table, update, null); + return new UpdateSpec(table, update, Criteria.empty()); } /** @@ -512,7 +512,6 @@ public interface StatementMapper { return this.update; } - @Nullable public Criteria getCriteria() { return this.criteria; } @@ -525,9 +524,9 @@ public interface StatementMapper { private final SqlIdentifier table; - private final @Nullable Criteria criteria; + private final Criteria criteria; - protected DeleteSpec(SqlIdentifier table, @Nullable Criteria criteria) { + protected DeleteSpec(SqlIdentifier table, Criteria criteria) { this.table = table; this.criteria = criteria; } @@ -550,7 +549,7 @@ public interface StatementMapper { * @since 1.1 */ public static DeleteSpec create(SqlIdentifier table) { - return new DeleteSpec(table, null); + return new DeleteSpec(table, Criteria.empty()); } /** @@ -567,7 +566,6 @@ public interface StatementMapper { return this.table; } - @Nullable public Criteria getCriteria() { return this.criteria; } diff --git a/src/main/java/org/springframework/data/r2dbc/query/Criteria.java b/src/main/java/org/springframework/data/r2dbc/query/Criteria.java index 858ad83a..4d2d9ee1 100644 --- a/src/main/java/org/springframework/data/r2dbc/query/Criteria.java +++ b/src/main/java/org/springframework/data/r2dbc/query/Criteria.java @@ -17,6 +17,8 @@ package org.springframework.data.r2dbc.query; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; +import java.util.List; import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.relational.core.sql.SqlIdentifier; @@ -27,33 +29,106 @@ import org.springframework.util.Assert; * Central class for creating queries. It follows a fluent API style so that you can easily chain together multiple * criteria. Static import of the {@code Criteria.property(…)} method will improve readability as in * {@code where(property(…).is(…)}. + *

+ * The Criteria API supports composition with a {@link #empty() NULL object} and a {@link #from(List) static factory + * method}. Example usage: + * + * 

+ * Criteria.from(Criteria.where("name").is("Foo"), Criteria.from(Criteria.where("age").greaterThan(42)));
+ *
+ * + * rendering: + * + * 
+ * WHERE name = 'Foo' AND age > 42
+ *
* * @author Mark Paluch * @author Oliver Drotbohm */ public class Criteria { + private static final Criteria EMPTY = new Criteria(SqlIdentifier.EMPTY, Comparator.INITIAL, null); + private final @Nullable Criteria previous; private final Combinator combinator; + private final List group; - private final SqlIdentifier column; - private final Comparator comparator; + private final @Nullable SqlIdentifier column; + private final @Nullable Comparator comparator; private final @Nullable Object value; private Criteria(SqlIdentifier column, Comparator comparator, @Nullable Object value) { - this(null, Combinator.INITIAL, column, comparator, value); + this(null, Combinator.INITIAL, Collections.emptyList(), column, comparator, value); } - private Criteria(@Nullable Criteria previous, Combinator combinator, SqlIdentifier column, Comparator comparator, - @Nullable Object value) { + private Criteria(@Nullable Criteria previous, Combinator combinator, List group, + @Nullable SqlIdentifier column, @Nullable Comparator comparator, @Nullable Object value) { this.previous = previous; - this.combinator = combinator; + this.combinator = previous != null && previous.isEmpty() ? Combinator.INITIAL : combinator; + this.group = group; this.column = column; this.comparator = comparator; this.value = value; } + private Criteria(@Nullable Criteria previous, Combinator combinator, List group) { + + this.previous = previous; + this.combinator = previous != null && previous.isEmpty() ? Combinator.INITIAL : combinator; + this.group = group; + this.column = null; + this.comparator = null; + this.value = null; + } + + /** + * Static factory method to create an empty Criteria. + * + * @return an empty {@link Criteria}. + * @since 1.1 + */ + public static Criteria empty() { + return EMPTY; + } + + /** + * Create a new {@link Criteria} and combine it as group with {@code AND} using the provided {@link List Criterias}. + * + * @return new {@link Criteria}. + * @since 1.1 + */ + public static Criteria from(Criteria... criteria) { + + Assert.notNull(criteria, "Criteria must not be null"); + Assert.noNullElements(criteria, "Criteria must not contain null elements"); + + return from(Arrays.asList(criteria)); + } + + /** + * Create a new {@link Criteria} and combine it as group with {@code AND} using the provided {@link List Criterias}. + * + * @return new {@link Criteria}. + * @since 1.1 + */ + public static Criteria from(List criteria) { + + Assert.notNull(criteria, "Criteria must not be null"); + Assert.noNullElements(criteria, "Criteria must not contain null elements"); + + if (criteria.isEmpty()) { + return EMPTY; + } + + if (criteria.size() == 1) { + return criteria.get(0); + } + + return EMPTY.and(criteria); + } + /** * Static factory method to create a Criteria using the provided {@code column} name. * @@ -77,14 +152,43 @@ public class Criteria { Assert.hasText(column, "Column name must not be null or empty!"); - return new DefaultCriteriaStep(SqlIdentifier.unquoted(column)) { + SqlIdentifier identifier = SqlIdentifier.unquoted(column); + return new DefaultCriteriaStep(identifier) { @Override protected Criteria createCriteria(Comparator comparator, Object value) { - return new Criteria(Criteria.this, Combinator.AND, SqlIdentifier.unquoted(column), comparator, value); + return new Criteria(Criteria.this, Combinator.AND, Collections.emptyList(), identifier, comparator, value); } }; } + /** + * Create a new {@link Criteria} and combine it as group with {@code AND} using the provided {@link Criteria} group. + * + * @param criteria criteria object. + * @return a new {@link Criteria} object. + * @since 1.1 + */ + public Criteria and(Criteria criteria) { + + Assert.notNull(criteria, "Criteria must not be null!"); + + return and(Collections.singletonList(criteria)); + } + + /** + * Create a new {@link Criteria} and combine it as group with {@code AND} using the provided {@link Criteria} group. + * + * @param criteria criteria objects. + * @return a new {@link Criteria} object. + * @since 1.1 + */ + public Criteria and(List criteria) { + + Assert.notNull(criteria, "Criteria must not be null!"); + + return new Criteria(Criteria.this, Combinator.AND, criteria); + } + /** * Create a new {@link Criteria} and combine it with {@code OR} using the provided {@code column} name. * @@ -95,14 +199,43 @@ public class Criteria { Assert.hasText(column, "Column name must not be null or empty!"); - return new DefaultCriteriaStep(SqlIdentifier.unquoted(column)) { + SqlIdentifier identifier = SqlIdentifier.unquoted(column); + return new DefaultCriteriaStep(identifier) { @Override protected Criteria createCriteria(Comparator comparator, Object value) { - return new Criteria(Criteria.this, Combinator.OR, SqlIdentifier.unquoted(column), comparator, value); + return new Criteria(Criteria.this, Combinator.OR, Collections.emptyList(), identifier, comparator, value); } }; } + /** + * Create a new {@link Criteria} and combine it as group with {@code OR} using the provided {@link Criteria} group. + * + * @param criteria criteria object. + * @return a new {@link Criteria} object. + * @since 1.1 + */ + public Criteria or(Criteria criteria) { + + Assert.notNull(criteria, "Criteria must not be null!"); + + return or(Collections.singletonList(criteria)); + } + + /** + * Create a new {@link Criteria} and combine it as group with {@code OR} using the provided {@link Criteria} group. + * + * @param criteria criteria object. + * @return a new {@link Criteria} object. + * @since 1.1 + */ + public Criteria or(List criteria) { + + Assert.notNull(criteria, "Criteria must not be null!"); + + return new Criteria(Criteria.this, Combinator.OR, criteria); + } + /** * @return the previous {@link Criteria} object. Can be {@literal null} if there is no previous {@link Criteria}. * @see #hasPrevious() @@ -119,6 +252,56 @@ public class Criteria { return previous != null; } + /** + * @return {@literal true} if this {@link Criteria} is empty. + * @since 1.1 + */ + public boolean isEmpty() { + + if (!doIsEmpty()) { + return false; + } + + Criteria parent = this.previous; + + while (parent != null) { + + if (!parent.doIsEmpty()) { + return false; + } + + parent = parent.previous; + } + + return true; + } + + private boolean doIsEmpty() { + + if (this.comparator == Comparator.INITIAL) { + return true; + } + + if (this.column != null) { + return false; + } + + for (Criteria criteria : group) { + if (!criteria.isEmpty()) { + return false; + } + } + + return true; + } + + /** + * @return {@literal true} if this {@link Criteria} is empty. + */ + boolean isGroup() { + return !this.group.isEmpty(); + } + /** * @return {@link Combinator} to combine this criteria with a previous one. */ @@ -126,9 +309,14 @@ public class Criteria { return combinator; } + List getGroup() { + return group; + } + /** * @return the column/property name. */ + @Nullable SqlIdentifier getColumn() { return column; } @@ -136,6 +324,7 @@ public class Criteria { /** * @return {@link Comparator}. */ + @Nullable Comparator getComparator() { return comparator; } @@ -149,7 +338,7 @@ public class Criteria { } enum Comparator { - EQ, NEQ, LT, LTE, GT, GTE, IS_NULL, IS_NOT_NULL, LIKE, NOT_IN, IN, + INITIAL, EQ, NEQ, LT, LTE, GT, GTE, IS_NULL, IS_NOT_NULL, LIKE, NOT_IN, IN, } enum Combinator { @@ -240,13 +429,11 @@ public class Criteria { /** * Creates a {@link Criteria} using {@code IS NULL}. - * */ Criteria isNull(); /** * Creates a {@link Criteria} using {@code IS NOT NULL}. - * */ Criteria isNotNull(); } diff --git a/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java b/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java index 8170646f..52547560 100644 --- a/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java +++ b/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java @@ -195,9 +195,22 @@ public class QueryMapper { Assert.notNull(criteria, "Criteria must not be null!"); Assert.notNull(table, "Table must not be null!"); - Criteria current = criteria; MutableBindings bindings = new MutableBindings(markers); + if (criteria.isEmpty()) { + throw new IllegalArgumentException("Cannot map empty Criteria"); + } + + Condition mapped = unroll(criteria, table, entity, bindings); + + return new BoundCondition(bindings, mapped); + } + + private Condition unroll(Criteria criteria, Table table, @Nullable RelationalPersistentEntity entity, + MutableBindings bindings) { + + Criteria current = criteria; + // reverse unroll criteria chain Map forwardChain = new HashMap<>(); @@ -210,25 +223,83 @@ public class QueryMapper { Condition mapped = getCondition(current, bindings, table, entity); while (forwardChain.containsKey(current)) { - Criteria nextCriteria = forwardChain.get(current); + Criteria criterion = forwardChain.get(current); + Condition result = null; - if (nextCriteria.getCombinator() == Combinator.AND) { - mapped = mapped.and(getCondition(nextCriteria, bindings, table, entity)); + Condition condition = getCondition(criterion, bindings, table, entity); + if (condition != null) { + result = combine(criterion, mapped, criterion.getCombinator(), condition); } - if (nextCriteria.getCombinator() == Combinator.OR) { - mapped = mapped.or(getCondition(nextCriteria, bindings, table, entity)); + if (result != null) { + mapped = result; } - - current = nextCriteria; + current = criterion; } - return new BoundCondition(bindings, mapped); + if (mapped == null) { + throw new IllegalStateException("Cannot map empty Criteria"); + } + + return mapped; } + @Nullable + private Condition unrollGroup(List criteria, Table table, Combinator combinator, + @Nullable RelationalPersistentEntity entity, MutableBindings bindings) { + + Condition mapped = null; + for (Criteria criterion : criteria) { + + if (criterion.isEmpty()) { + continue; + } + + Condition condition = unroll(criterion, table, entity, bindings); + + mapped = combine(criterion, mapped, combinator, condition); + } + + return mapped; + } + + @Nullable private Condition getCondition(Criteria criteria, MutableBindings bindings, Table table, @Nullable RelationalPersistentEntity entity) { + if (criteria.isEmpty()) { + return null; + } + + if (criteria.isGroup()) { + + Condition condition = unrollGroup(criteria.getGroup(), table, criteria.getCombinator(), entity, bindings); + + return condition == null ? null : Conditions.nest(condition); + } + + return mapCondition(criteria, bindings, table, entity); + } + + private Condition combine(Criteria criteria, @Nullable Condition currentCondition, Combinator combinator, + Condition nextCondition) { + + if (currentCondition == null) { + currentCondition = nextCondition; + } else if (combinator == Combinator.AND) { + currentCondition = currentCondition.and(nextCondition); + } else if (combinator == Combinator.OR) { + currentCondition = currentCondition.or(nextCondition); + } else { + throw new IllegalStateException("Combinator " + criteria.getCombinator() + " not supported"); + } + + return currentCondition; + } + + private Condition mapCondition(Criteria criteria, MutableBindings bindings, Table table, + @Nullable RelationalPersistentEntity entity) { + Field propertyField = createPropertyField(entity, criteria.getColumn(), this.mappingContext); Column column = table.column(propertyField.getMappedColumnName()); TypeInformation actualType = propertyField.getTypeHint().getRequiredActualType(); diff --git a/src/test/java/org/springframework/data/r2dbc/query/CriteriaUnitTests.java b/src/test/java/org/springframework/data/r2dbc/query/CriteriaUnitTests.java index 887585ba..16b88fb4 100644 --- a/src/test/java/org/springframework/data/r2dbc/query/CriteriaUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/query/CriteriaUnitTests.java @@ -32,6 +32,27 @@ import org.springframework.data.relational.core.sql.SqlIdentifier; */ public class CriteriaUnitTests { + @Test // gh-289 + public void fromCriteria() { + + Criteria nested1 = where("foo").isNotNull(); + Criteria nested2 = where("foo").isNull(); + Criteria criteria = Criteria.from(nested1, nested2); + + assertThat(criteria.isGroup()).isTrue(); + assertThat(criteria.getGroup()).containsExactly(nested1, nested2); + assertThat(criteria.getPrevious()).isEqualTo(Criteria.empty()); + } + + @Test // gh-289 + public void fromCriteriaOptimized() { + + Criteria nested = where("foo").is("bar").and("baz").isNotNull(); + Criteria criteria = Criteria.from(nested); + + assertThat(criteria).isSameAs(nested); + } + @Test // gh-64 public void andChainedCriteria() { @@ -50,6 +71,23 @@ public class CriteriaUnitTests { assertThat(criteria.getValue()).isEqualTo("bar"); } + @Test // gh-289 + public void andGroupedCriteria() { + + Criteria criteria = where("foo").is("bar").and(where("foo").is("baz")); + + assertThat(criteria.isGroup()).isTrue(); + assertThat(criteria.getGroup()).hasSize(1); + assertThat(criteria.getGroup().get(0).getColumn()).isEqualTo(SqlIdentifier.unquoted("foo")); + assertThat(criteria.getCombinator()).isEqualTo(Combinator.AND); + + criteria = criteria.getPrevious(); + + assertThat(criteria.getColumn()).isEqualTo(SqlIdentifier.unquoted("foo")); + assertThat(criteria.getComparator()).isEqualTo(Comparator.EQ); + assertThat(criteria.getValue()).isEqualTo("bar"); + } + @Test // gh-64 public void orChainedCriteria() { @@ -64,6 +102,23 @@ public class CriteriaUnitTests { assertThat(criteria.getValue()).isEqualTo("bar"); } + @Test // gh-289 + public void orGroupedCriteria() { + + Criteria criteria = where("foo").is("bar").or(where("foo").is("baz")); + + assertThat(criteria.isGroup()).isTrue(); + assertThat(criteria.getGroup()).hasSize(1); + assertThat(criteria.getGroup().get(0).getColumn()).isEqualTo(SqlIdentifier.unquoted("foo")); + assertThat(criteria.getCombinator()).isEqualTo(Combinator.OR); + + criteria = criteria.getPrevious(); + + assertThat(criteria.getColumn()).isEqualTo(SqlIdentifier.unquoted("foo")); + assertThat(criteria.getComparator()).isEqualTo(Comparator.EQ); + assertThat(criteria.getValue()).isEqualTo("bar"); + } + @Test // gh-64 public void shouldBuildEqualsCriteria() { diff --git a/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java b/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java index d3ea3580..d732a1ee 100644 --- a/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java @@ -19,6 +19,8 @@ import static org.assertj.core.api.Assertions.*; import static org.mockito.Mockito.*; import static org.springframework.data.domain.Sort.Order.*; +import java.util.Collections; + import org.junit.Test; import org.springframework.data.domain.Sort; @@ -45,6 +47,74 @@ public class QueryMapperUnitTests { QueryMapper mapper = new QueryMapper(PostgresDialect.INSTANCE, converter); BindTarget bindTarget = mock(BindTarget.class); + @Test // gh-289 + public void shouldNotMapEmptyCriteria() { + + Criteria criteria = Criteria.empty(); + + assertThatIllegalArgumentException().isThrownBy(() -> map(criteria)); + } + + @Test // gh-289 + public void shouldNotMapEmptyAndCriteria() { + + Criteria criteria = Criteria.empty().and(Collections.emptyList()); + + assertThatIllegalArgumentException().isThrownBy(() -> map(criteria)); + } + + @Test // gh-289 + public void shouldNotMapEmptyNestedCriteria() { + + Criteria criteria = Criteria.empty().and(Collections.emptyList()).and(Criteria.empty().and(Criteria.empty())); + + assertThat(criteria.isEmpty()).isTrue(); + assertThatIllegalArgumentException().isThrownBy(() -> map(criteria)); + } + + @Test // gh-289 + public void shouldMapSomeNestedCriteria() { + + Criteria criteria = Criteria.empty().and(Collections.emptyList()) + .and(Criteria.empty().and(Criteria.where("name").is("Hank"))); + + assertThat(criteria.isEmpty()).isFalse(); + + BoundCondition bindings = map(criteria); + + assertThat(bindings.getCondition().toString()).isEqualTo("((person.name = ?[$1]))"); + } + + @Test // gh-289 + public void shouldMapNestedGroup() { + + Criteria initial = Criteria.empty(); + + Criteria criteria = initial.and(Criteria.where("name").is("Foo")).and(Criteria.where("name").is("Bar").or("age") + .lessThan(49).or(Criteria.where("name").not("Bar").and("age").greaterThan(49))); + + assertThat(criteria.isEmpty()).isFalse(); + + BoundCondition bindings = map(criteria); + + assertThat(bindings.getCondition().toString()).isEqualTo( + "(person.name = ?[$1]) AND (person.name = ?[$2] OR person.age < ?[$3] OR (person.name != ?[$4] AND person.age > ?[$5]))"); + } + + @Test // gh-289 + public void shouldMapFrom() { + + Criteria criteria = Criteria.from(Criteria.where("name").is("Foo")) + .and(Criteria.where("name").is("Bar").or("age").lessThan(49)); + + assertThat(criteria.isEmpty()).isFalse(); + + BoundCondition bindings = map(criteria); + + assertThat(bindings.getCondition().toString()) + .isEqualTo("person.name = ?[$1] AND (person.name = ?[$2] OR person.age < ?[$3])"); + } + @Test // gh-64 public void shouldMapSimpleCriteria() {