From 05e96557bd5cb01c149fdb714db80a6be102350d Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 28 Feb 2022 15:17:44 +0100 Subject: [PATCH] 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. Orginal pull request #1188 --- .../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 b758c4746..97626cb5f 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 @@ class QueryMapper { 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 760a62bd9..8b2f4fcb7 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.junit.jupiter.api.Test; 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 @@ import org.springframework.r2dbc.core.binding.BindTarget; */ 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 @@ class QueryMapperUnitTests { 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 @@ class QueryMapperUnitTests { 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 @@ class QueryMapperUnitTests { 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 @@ class QueryMapperUnitTests { 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 @@ class QueryMapperUnitTests { 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 @@ class QueryMapperUnitTests { 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 @@ class QueryMapperUnitTests { 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 105e79015..34281a30a 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 class Criteria implements CriteriaDefinition { @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 6529ad761..02439f851 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 class CriteriaUnitTests { 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 class CriteriaUnitTests { assertThat(criteria.getColumn()).isEqualTo(SqlIdentifier.unquoted("foo")); assertThat(criteria.getComparator()).isEqualTo(CriteriaDefinition.Comparator.IS_FALSE); + assertThat(criteria.getValue()).isEqualTo(false); } }