From 8866f2f8f8723bac14b343179f07def7ba3f78a6 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 28 Feb 2022 15:17:44 +0100 Subject: [PATCH 1/2] Associate value with isTrue/isFalse criteria operators. We now associate a boolean value with both operators as those operators are rendered using equals comparison in the actual SQL text. --- .../jdbc/repository/query/QueryMapper.java | 6 +- .../r2dbc/query/QueryMapperUnitTests.java | 58 +++++++++++++++---- .../data/relational/core/query/Criteria.java | 4 +- .../core/query/CriteriaUnitTests.java | 2 + 4 files changed, 56 insertions(+), 14 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/QueryMapper.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/QueryMapper.java index b758c4746a..97626cb5f8 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/QueryMapper.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/QueryMapper.java @@ -494,13 +494,15 @@ private Condition createCondition(Column column, @Nullable Object mappedValue, S if (comparator == Comparator.IS_TRUE) { - Expression bind = bindBoolean(column, parameterSource, true); + Expression bind = bindBoolean(column, parameterSource, + mappedValue instanceof Boolean ? (Boolean) mappedValue : true); return column.isEqualTo(bind); } if (comparator == Comparator.IS_FALSE) { - Expression bind = bindBoolean(column, parameterSource, false); + Expression bind = bindBoolean(column, parameterSource, + mappedValue instanceof Boolean ? (Boolean) mappedValue : false); return column.isEqualTo(bind); } diff --git a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java index 760a62bd9d..8b2f4fcb7b 100644 --- a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java +++ b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java @@ -26,7 +26,10 @@ import org.springframework.data.domain.Sort; import org.springframework.data.r2dbc.convert.MappingR2dbcConverter; import org.springframework.data.r2dbc.convert.R2dbcConverter; +import org.springframework.data.r2dbc.convert.R2dbcCustomConversions; +import org.springframework.data.r2dbc.dialect.MySqlDialect; import org.springframework.data.r2dbc.dialect.PostgresDialect; +import org.springframework.data.r2dbc.dialect.R2dbcDialect; import org.springframework.data.r2dbc.mapping.R2dbcMappingContext; import org.springframework.data.relational.core.mapping.Column; import org.springframework.data.relational.core.query.Criteria; @@ -45,11 +48,21 @@ */ class QueryMapperUnitTests { - private R2dbcMappingContext context = new R2dbcMappingContext(); - private R2dbcConverter converter = new MappingR2dbcConverter(context); - - private QueryMapper mapper = new QueryMapper(PostgresDialect.INSTANCE, converter); private BindTarget bindTarget = mock(BindTarget.class); + private QueryMapper mapper = createMapper(PostgresDialect.INSTANCE); + + QueryMapper createMapper(R2dbcDialect dialect) { + + R2dbcCustomConversions conversions = R2dbcCustomConversions.of(dialect); + + R2dbcMappingContext context = new R2dbcMappingContext(); + context.setSimpleTypeHolder(conversions.getSimpleTypeHolder()); + context.afterPropertiesSet(); + + R2dbcConverter converter = new MappingR2dbcConverter(context, conversions); + + return new QueryMapper(dialect, converter); + } @Test // gh-289 void shouldNotMapEmptyCriteria() { @@ -189,7 +202,7 @@ void shouldMapExpression() { Table table = Table.create("my_table").as("my_aliased_table"); Expression mappedObject = mapper.getMappedObject(table.column("alternative").as("my_aliased_col"), - context.getRequiredPersistentEntity(Person.class)); + mapper.getMappingContext().getRequiredPersistentEntity(Person.class)); assertThat(mappedObject).hasToString("my_aliased_table.another_name AS my_aliased_col"); } @@ -200,7 +213,7 @@ void shouldMapCountFunction() { Table table = Table.create("my_table").as("my_aliased_table"); Expression mappedObject = mapper.getMappedObject(Functions.count(table.column("alternative")), - context.getRequiredPersistentEntity(Person.class)); + mapper.getMappingContext().getRequiredPersistentEntity(Person.class)); assertThat(mappedObject).hasToString("COUNT(my_aliased_table.another_name)"); } @@ -211,7 +224,7 @@ void shouldMapExpressionToUnknownColumn() { Table table = Table.create("my_table").as("my_aliased_table"); Expression mappedObject = mapper.getMappedObject(table.column("unknown").as("my_aliased_col"), - context.getRequiredPersistentEntity(Person.class)); + mapper.getMappingContext().getRequiredPersistentEntity(Person.class)); assertThat(mappedObject).hasToString("my_aliased_table.unknown AS my_aliased_col"); } @@ -392,7 +405,7 @@ void shouldMapSort() { Sort sort = Sort.by(desc("alternative")); - Sort mapped = mapper.getMappedObject(sort, context.getRequiredPersistentEntity(Person.class)); + Sort mapped = mapper.getMappedObject(sort, mapper.getMappingContext().getRequiredPersistentEntity(Person.class)); assertThat(mapped.getOrderFor("another_name")).isEqualTo(desc("another_name")); assertThat(mapped.getOrderFor("alternative")).isNull(); @@ -403,7 +416,7 @@ void mapSortForPropertyPathInPrimitiveShouldFallBackToColumnName() { Sort sort = Sort.by(desc("alternative_name")); - Sort mapped = mapper.getMappedObject(sort, context.getRequiredPersistentEntity(Person.class)); + Sort mapped = mapper.getMappedObject(sort, mapper.getMappingContext().getRequiredPersistentEntity(Person.class)); assertThat(mapped.getOrderFor("alternative_name")).isEqualTo(desc("alternative_name")); } @@ -427,12 +440,35 @@ void mapQueryForEnumArrayShouldMapToStringList() { assertThat(bindings.getCondition()).hasToString("person.enum_value IN (?[$1], ?[$2])"); } + @Test // gh-733 + void shouldMapBooleanConditionProperly() { + + Criteria criteria = Criteria.where("state").isFalse(); + + BoundCondition bindings = map(criteria); + + assertThat(bindings.getCondition()).hasToString("person.state = ?[$1]"); + assertThat(bindings.getBindings().iterator().next().getValue()).isEqualTo(false); + } + + @Test // gh-733 + void shouldMapAndConvertBooleanConditionProperly() { + + mapper = createMapper(MySqlDialect.INSTANCE); + Criteria criteria = Criteria.where("state").isTrue(); + + BoundCondition bindings = map(criteria); + + assertThat(bindings.getCondition()).hasToString("person.state = ?[$1]"); + assertThat(bindings.getBindings().iterator().next().getValue()).isEqualTo((byte) 1); + } + private BoundCondition map(Criteria criteria) { BindMarkersFactory markers = BindMarkersFactory.indexed("$", 1); return mapper.getMappedObject(markers.create(), criteria, Table.create("person"), - context.getRequiredPersistentEntity(Person.class)); + mapper.getMappingContext().getRequiredPersistentEntity(Person.class)); } static class Person { @@ -440,6 +476,8 @@ static class Person { String name; @Column("another_name") String alternative; MyEnum enumValue; + + boolean state; } enum MyEnum { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/query/Criteria.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/query/Criteria.java index 105e790154..34281a30ab 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/query/Criteria.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/query/Criteria.java @@ -781,12 +781,12 @@ public Criteria isNotNull() { @Override public Criteria isTrue() { - return createCriteria(Comparator.IS_TRUE, null); + return createCriteria(Comparator.IS_TRUE, true); } @Override public Criteria isFalse() { - return createCriteria(Comparator.IS_FALSE, null); + return createCriteria(Comparator.IS_FALSE, false); } protected Criteria createCriteria(Comparator comparator, @Nullable Object value) { diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/query/CriteriaUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/query/CriteriaUnitTests.java index 6529ad7610..02439f851d 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/query/CriteriaUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/query/CriteriaUnitTests.java @@ -285,6 +285,7 @@ public void shouldBuildIsTrueCriteria() { assertThat(criteria.getColumn()).isEqualTo(SqlIdentifier.unquoted("foo")); assertThat(criteria.getComparator()).isEqualTo(CriteriaDefinition.Comparator.IS_TRUE); + assertThat(criteria.getValue()).isEqualTo(true); } @Test // DATAJDBC-513 @@ -294,5 +295,6 @@ public void shouldBuildIsFalseCriteria() { assertThat(criteria.getColumn()).isEqualTo(SqlIdentifier.unquoted("foo")); assertThat(criteria.getComparator()).isEqualTo(CriteriaDefinition.Comparator.IS_FALSE); + assertThat(criteria.getValue()).isEqualTo(false); } } From 79d9ab066dc21b884c049a10e480bda5ce0938fc Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 28 Feb 2022 15:18:01 +0100 Subject: [PATCH 2/2] Polishing. Reduce test method visibility. --- .../core/query/CriteriaUnitTests.java | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/query/CriteriaUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/query/CriteriaUnitTests.java index 02439f851d..fa05131edc 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/query/CriteriaUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/query/CriteriaUnitTests.java @@ -32,10 +32,10 @@ * @author Jens Schauder * @author Roman Chigvintsev */ -public class CriteriaUnitTests { +class CriteriaUnitTests { @Test // DATAJDBC-513 - public void fromCriteria() { + void fromCriteria() { Criteria nested1 = where("foo").isNotNull(); Criteria nested2 = where("foo").isNull(); @@ -48,7 +48,7 @@ public void fromCriteria() { } @Test // DATAJDBC-513 - public void fromCriteriaOptimized() { + void fromCriteriaOptimized() { Criteria nested = where("foo").is("bar").and("baz").isNotNull(); CriteriaDefinition criteria = Criteria.from(nested); @@ -57,7 +57,7 @@ public void fromCriteriaOptimized() { } @Test // DATAJDBC-513 - public void isEmpty() { + void isEmpty() { assertSoftly(softly -> { @@ -79,7 +79,7 @@ public void isEmpty() { } @Test // DATAJDBC-513 - public void andChainedCriteria() { + void andChainedCriteria() { Criteria criteria = where("foo").is("bar").and("baz").isNotNull(); @@ -97,7 +97,7 @@ public void andChainedCriteria() { } @Test // DATAJDBC-513 - public void andGroupedCriteria() { + void andGroupedCriteria() { Criteria grouped = where("foo").is("bar").and(where("foo").is("baz").or("bar").isNotNull()); Criteria criteria = grouped; @@ -118,7 +118,7 @@ public void andGroupedCriteria() { } @Test // DATAJDBC-513 - public void orChainedCriteria() { + void orChainedCriteria() { Criteria criteria = where("foo").is("bar").or("baz").isNotNull(); @@ -133,7 +133,7 @@ public void orChainedCriteria() { } @Test // DATAJDBC-513 - public void orGroupedCriteria() { + void orGroupedCriteria() { Criteria criteria = where("foo").is("bar").or(where("foo").is("baz")); @@ -151,7 +151,7 @@ public void orGroupedCriteria() { } @Test // DATAJDBC-513 - public void shouldBuildEqualsCriteria() { + void shouldBuildEqualsCriteria() { Criteria criteria = where("foo").is("bar"); @@ -161,7 +161,7 @@ public void shouldBuildEqualsCriteria() { } @Test - public void shouldBuildEqualsIgnoreCaseCriteria() { + void shouldBuildEqualsIgnoreCaseCriteria() { Criteria criteria = where("foo").is("bar").ignoreCase(true); assertThat(criteria.getColumn()).isEqualTo(SqlIdentifier.unquoted("foo")); @@ -171,7 +171,7 @@ public void shouldBuildEqualsIgnoreCaseCriteria() { } @Test // DATAJDBC-513 - public void shouldBuildNotEqualsCriteria() { + void shouldBuildNotEqualsCriteria() { Criteria criteria = where("foo").not("bar"); @@ -181,7 +181,7 @@ public void shouldBuildNotEqualsCriteria() { } @Test // DATAJDBC-513 - public void shouldBuildInCriteria() { + void shouldBuildInCriteria() { Criteria criteria = where("foo").in("bar", "baz"); @@ -192,7 +192,7 @@ public void shouldBuildInCriteria() { } @Test // DATAJDBC-513 - public void shouldBuildNotInCriteria() { + void shouldBuildNotInCriteria() { Criteria criteria = where("foo").notIn("bar", "baz"); @@ -202,7 +202,7 @@ public void shouldBuildNotInCriteria() { } @Test // DATAJDBC-513 - public void shouldBuildGtCriteria() { + void shouldBuildGtCriteria() { Criteria criteria = where("foo").greaterThan(1); @@ -212,7 +212,7 @@ public void shouldBuildGtCriteria() { } @Test // DATAJDBC-513 - public void shouldBuildGteCriteria() { + void shouldBuildGteCriteria() { Criteria criteria = where("foo").greaterThanOrEquals(1); @@ -222,7 +222,7 @@ public void shouldBuildGteCriteria() { } @Test // DATAJDBC-513 - public void shouldBuildLtCriteria() { + void shouldBuildLtCriteria() { Criteria criteria = where("foo").lessThan(1); @@ -232,7 +232,7 @@ public void shouldBuildLtCriteria() { } @Test // DATAJDBC-513 - public void shouldBuildLteCriteria() { + void shouldBuildLteCriteria() { Criteria criteria = where("foo").lessThanOrEquals(1); @@ -242,7 +242,7 @@ public void shouldBuildLteCriteria() { } @Test // DATAJDBC-513 - public void shouldBuildLikeCriteria() { + void shouldBuildLikeCriteria() { Criteria criteria = where("foo").like("hello%"); @@ -252,7 +252,7 @@ public void shouldBuildLikeCriteria() { } @Test - public void shouldBuildNotLikeCriteria() { + void shouldBuildNotLikeCriteria() { Criteria criteria = where("foo").notLike("hello%"); assertThat(criteria.getColumn()).isEqualTo(SqlIdentifier.unquoted("foo")); @@ -261,7 +261,7 @@ public void shouldBuildNotLikeCriteria() { } @Test // DATAJDBC-513 - public void shouldBuildIsNullCriteria() { + void shouldBuildIsNullCriteria() { Criteria criteria = where("foo").isNull(); @@ -270,7 +270,7 @@ public void shouldBuildIsNullCriteria() { } @Test // DATAJDBC-513 - public void shouldBuildIsNotNullCriteria() { + void shouldBuildIsNotNullCriteria() { Criteria criteria = where("foo").isNotNull(); @@ -279,7 +279,7 @@ public void shouldBuildIsNotNullCriteria() { } @Test // DATAJDBC-513 - public void shouldBuildIsTrueCriteria() { + void shouldBuildIsTrueCriteria() { Criteria criteria = where("foo").isTrue(); @@ -289,7 +289,7 @@ public void shouldBuildIsTrueCriteria() { } @Test // DATAJDBC-513 - public void shouldBuildIsFalseCriteria() { + void shouldBuildIsFalseCriteria() { Criteria criteria = where("foo").isFalse();