Skip to content

Associate value with isTrue/isFalse criteria operators #1188

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Associate value with isTrue/isFalse criteria operators #1188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8866f2f
Associate value with isTrue/isFalse criteria operators.
mp911de Feb 28, 2022
79d9ab0
Polishing.
mp911de Feb 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions ...g-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/QueryMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
58 changes: 48 additions & 10 deletions ...g-data-r2dbc/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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() {
Expand Down Expand Up @@ -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");
}
Expand All @@ -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)");
}
Expand All @@ -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");
}
Expand Down Expand Up @@ -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();
Expand All @@ -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"));
}

Expand All @@ -427,19 +440,44 @@ void mapQueryForEnumArrayShouldMapToStringList() {
assertThat(bindings.getCondition()).hasToString("person.enum_value IN (?[$1], ?[$2])");
}

@Test // gh-733
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These test are in the R2DBC part, but the changes are for JDBC.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the changes are in Relational but we don't have a query mapper for Spring Data Relational and we never ensured in R2DBC how isTrue is bound in R2DBC. The JDBC counterpart should be in #908.

The changes in the Criteria API are now covered with additional assertions and the JDBC change went into effect with #908.

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 {

String name;
@Column("another_name") String alternative;
MyEnum enumValue;

boolean state;
}

enum MyEnum {
Expand Down
4 changes: 2 additions & 2 deletions ...ata-relational/src/main/java/org/springframework/data/relational/core/query/Criteria.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
48 changes: 25 additions & 23 deletions ...ional/src/test/java/org/springframework/data/relational/core/query/CriteriaUnitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
Expand All @@ -57,7 +57,7 @@ public void fromCriteriaOptimized() {
}

@Test // DATAJDBC-513
public void isEmpty() {
void isEmpty() {

assertSoftly(softly -> {

Expand All @@ -79,7 +79,7 @@ public void isEmpty() {
}

@Test // DATAJDBC-513
public void andChainedCriteria() {
void andChainedCriteria() {

Criteria criteria = where("foo").is("bar").and("baz").isNotNull();

Expand All @@ -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;
Expand All @@ -118,7 +118,7 @@ public void andGroupedCriteria() {
}

@Test // DATAJDBC-513
public void orChainedCriteria() {
void orChainedCriteria() {

Criteria criteria = where("foo").is("bar").or("baz").isNotNull();

Expand All @@ -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"));

Expand All @@ -151,7 +151,7 @@ public void orGroupedCriteria() {
}

@Test // DATAJDBC-513
public void shouldBuildEqualsCriteria() {
void shouldBuildEqualsCriteria() {

Criteria criteria = where("foo").is("bar");

Expand All @@ -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"));
Expand All @@ -171,7 +171,7 @@ public void shouldBuildEqualsIgnoreCaseCriteria() {
}

@Test // DATAJDBC-513
public void shouldBuildNotEqualsCriteria() {
void shouldBuildNotEqualsCriteria() {

Criteria criteria = where("foo").not("bar");

Expand All @@ -181,7 +181,7 @@ public void shouldBuildNotEqualsCriteria() {
}

@Test // DATAJDBC-513
public void shouldBuildInCriteria() {
void shouldBuildInCriteria() {

Criteria criteria = where("foo").in("bar", "baz");

Expand All @@ -192,7 +192,7 @@ public void shouldBuildInCriteria() {
}

@Test // DATAJDBC-513
public void shouldBuildNotInCriteria() {
void shouldBuildNotInCriteria() {

Criteria criteria = where("foo").notIn("bar", "baz");

Expand All @@ -202,7 +202,7 @@ public void shouldBuildNotInCriteria() {
}

@Test // DATAJDBC-513
public void shouldBuildGtCriteria() {
void shouldBuildGtCriteria() {

Criteria criteria = where("foo").greaterThan(1);

Expand All @@ -212,7 +212,7 @@ public void shouldBuildGtCriteria() {
}

@Test // DATAJDBC-513
public void shouldBuildGteCriteria() {
void shouldBuildGteCriteria() {

Criteria criteria = where("foo").greaterThanOrEquals(1);

Expand All @@ -222,7 +222,7 @@ public void shouldBuildGteCriteria() {
}

@Test // DATAJDBC-513
public void shouldBuildLtCriteria() {
void shouldBuildLtCriteria() {

Criteria criteria = where("foo").lessThan(1);

Expand All @@ -232,7 +232,7 @@ public void shouldBuildLtCriteria() {
}

@Test // DATAJDBC-513
public void shouldBuildLteCriteria() {
void shouldBuildLteCriteria() {

Criteria criteria = where("foo").lessThanOrEquals(1);

Expand All @@ -242,7 +242,7 @@ public void shouldBuildLteCriteria() {
}

@Test // DATAJDBC-513
public void shouldBuildLikeCriteria() {
void shouldBuildLikeCriteria() {

Criteria criteria = where("foo").like("hello%");

Expand All @@ -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"));
Expand All @@ -261,7 +261,7 @@ public void shouldBuildNotLikeCriteria() {
}

@Test // DATAJDBC-513
public void shouldBuildIsNullCriteria() {
void shouldBuildIsNullCriteria() {

Criteria criteria = where("foo").isNull();

Expand All @@ -270,7 +270,7 @@ public void shouldBuildIsNullCriteria() {
}

@Test // DATAJDBC-513
public void shouldBuildIsNotNullCriteria() {
void shouldBuildIsNotNullCriteria() {

Criteria criteria = where("foo").isNotNull();

Expand All @@ -279,20 +279,22 @@ public void shouldBuildIsNotNullCriteria() {
}

@Test // DATAJDBC-513
public void shouldBuildIsTrueCriteria() {
void shouldBuildIsTrueCriteria() {

Criteria criteria = where("foo").isTrue();

assertThat(criteria.getColumn()).isEqualTo(SqlIdentifier.unquoted("foo"));
assertThat(criteria.getComparator()).isEqualTo(CriteriaDefinition.Comparator.IS_TRUE);
assertThat(criteria.getValue()).isEqualTo(true);
}

@Test // DATAJDBC-513
public void shouldBuildIsFalseCriteria() {
void shouldBuildIsFalseCriteria() {

Criteria criteria = where("foo").isFalse();

assertThat(criteria.getColumn()).isEqualTo(SqlIdentifier.unquoted("foo"));
assertThat(criteria.getComparator()).isEqualTo(CriteriaDefinition.Comparator.IS_FALSE);
assertThat(criteria.getValue()).isEqualTo(false);
}
}