From ed297c08fe947944ee1d22dd1dc8d28d476f9774 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 10 Mar 2020 16:08:04 +0100 Subject: [PATCH] #289 - Polishing. Refactored DefaultDatabaseClientUnitTests in order to make the relevant differences in setup easier to spot. Formatting and nullability annotations. Original pull request: #307. --- .../data/r2dbc/core/StatementMapper.java | 11 +++---- .../data/r2dbc/query/Criteria.java | 1 + .../data/r2dbc/query/QueryMapper.java | 2 +- .../data/r2dbc/query/CriteriaUnitTests.java | 28 ++++++++++++++++- .../r2dbc/query/QueryMapperUnitTests.java | 30 ++++++++++++------- 5 files changed, 54 insertions(+), 18 deletions(-) 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 a9c29f65..3af2c6a2 100644 --- a/src/main/java/org/springframework/data/r2dbc/core/StatementMapper.java +++ b/src/main/java/org/springframework/data/r2dbc/core/StatementMapper.java @@ -219,8 +219,11 @@ public interface StatementMapper { * @since 1.1 */ public static SelectSpec create(SqlIdentifier table) { - return new SelectSpec(Table.create(table), Collections.emptyList(), Collections.emptyList(), Criteria.empty(), - Sort.unsorted(), -1, -1); + + List projectedFields = Collections.emptyList(); + List selectList = Collections.emptyList(); + return new SelectSpec(Table.create(table), projectedFields, selectList, Criteria.empty(), Sort.unsorted(), -1, + -1); } public SelectSpec doWithTable(BiFunction function) { @@ -367,7 +370,6 @@ public interface StatementMapper { return Collections.unmodifiableList(selectList); } - @Nullable public Criteria getCriteria() { return this.criteria; } @@ -460,8 +462,7 @@ public interface StatementMapper { class UpdateSpec { private final SqlIdentifier table; - @Nullable - private final Update update; + @Nullable private final Update update; private final Criteria 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 4d2d9ee1..c86e0487 100644 --- a/src/main/java/org/springframework/data/r2dbc/query/Criteria.java +++ b/src/main/java/org/springframework/data/r2dbc/query/Criteria.java @@ -287,6 +287,7 @@ public class Criteria { } for (Criteria criteria : group) { + if (!criteria.isEmpty()) { return false; } 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 52547560..4b265619 100644 --- a/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java +++ b/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java @@ -444,7 +444,7 @@ public class QueryMapper { return entity == null ? new Field(key) : new MetadataBackedField(key, entity, mappingContext); } - Class getTypeHint(Object mappedValue, Class propertyType, SettableValue settableValue) { + Class getTypeHint(@Nullable Object mappedValue, Class propertyType, SettableValue settableValue) { if (mappedValue == null || propertyType.equals(Object.class)) { return settableValue.getType(); 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 16b88fb4..b2395bb0 100644 --- a/src/test/java/org/springframework/data/r2dbc/query/CriteriaUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/query/CriteriaUnitTests.java @@ -20,8 +20,8 @@ import static org.springframework.data.r2dbc.query.Criteria.*; import java.util.Arrays; +import org.assertj.core.api.SoftAssertions; import org.junit.Test; - import org.springframework.data.r2dbc.query.Criteria.*; import org.springframework.data.relational.core.sql.SqlIdentifier; @@ -53,6 +53,29 @@ public class CriteriaUnitTests { assertThat(criteria).isSameAs(nested); } + @Test // gh-289 + public void isEmpty() { + + SoftAssertions.assertSoftly(softly -> { + + Criteria empty = empty(); + Criteria notEmpty = where("foo").is("bar"); + + assertThat(empty.isEmpty()).isTrue(); + assertThat(notEmpty.isEmpty()).isFalse(); + + assertThat(Criteria.from(notEmpty).isEmpty()).isFalse(); + assertThat(Criteria.from(notEmpty, notEmpty).isEmpty()).isFalse(); + + assertThat(Criteria.from(empty).isEmpty()).isTrue(); + assertThat(Criteria.from(empty, empty).isEmpty()).isTrue(); + + assertThat(Criteria.from(empty, notEmpty).isEmpty()).isFalse(); + assertThat(Criteria.from(notEmpty, empty).isEmpty()).isFalse(); + + }); + } + @Test // gh-64 public void andChainedCriteria() { @@ -83,6 +106,7 @@ public class CriteriaUnitTests { criteria = criteria.getPrevious(); + assertThat(criteria).isNotNull(); assertThat(criteria.getColumn()).isEqualTo(SqlIdentifier.unquoted("foo")); assertThat(criteria.getComparator()).isEqualTo(Comparator.EQ); assertThat(criteria.getValue()).isEqualTo("bar"); @@ -98,6 +122,7 @@ public class CriteriaUnitTests { criteria = criteria.getPrevious(); + assertThat(criteria).isNotNull(); assertThat(criteria.getPrevious()).isNull(); assertThat(criteria.getValue()).isEqualTo("bar"); } @@ -114,6 +139,7 @@ public class CriteriaUnitTests { criteria = criteria.getPrevious(); + assertThat(criteria).isNotNull(); assertThat(criteria.getColumn()).isEqualTo(SqlIdentifier.unquoted("foo")); assertThat(criteria.getComparator()).isEqualTo(Comparator.EQ); assertThat(criteria.getValue()).isEqualTo("bar"); 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 d732a1ee..9fccf254 100644 --- a/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java @@ -22,7 +22,6 @@ import static org.springframework.data.domain.Sort.Order.*; import java.util.Collections; import org.junit.Test; - import org.springframework.data.domain.Sort; import org.springframework.data.r2dbc.convert.MappingR2dbcConverter; import org.springframework.data.r2dbc.convert.R2dbcConverter; @@ -43,7 +42,9 @@ import org.springframework.data.relational.core.sql.Table; */ public class QueryMapperUnitTests { - R2dbcConverter converter = new MappingR2dbcConverter(new R2dbcMappingContext()); + R2dbcMappingContext context = new R2dbcMappingContext(); + R2dbcConverter converter = new MappingR2dbcConverter(context); + QueryMapper mapper = new QueryMapper(PostgresDialect.INSTANCE, converter); BindTarget bindTarget = mock(BindTarget.class); @@ -90,8 +91,13 @@ public class QueryMapperUnitTests { 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))); + 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(); @@ -104,8 +110,10 @@ public class QueryMapperUnitTests { @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)); + Criteria criteria = Criteria.from(Criteria.where("name").is("Foo")) // + .and(Criteria.where("name").is("Bar") // + .or("age").lessThan(49) // + ); assertThat(criteria.isEmpty()).isFalse(); @@ -149,7 +157,7 @@ public class QueryMapperUnitTests { Table table = Table.create("my_table").as("my_aliased_table"); Expression mappedObject = mapper.getMappedObject(table.column("alternative").as("my_aliased_col"), - converter.getMappingContext().getRequiredPersistentEntity(Person.class)); + context.getRequiredPersistentEntity(Person.class)); assertThat(mappedObject).hasToString("my_aliased_table.another_name AS my_aliased_col"); } @@ -160,7 +168,7 @@ public class QueryMapperUnitTests { Table table = Table.create("my_table").as("my_aliased_table"); Expression mappedObject = mapper.getMappedObject(Functions.count(table.column("alternative")), - converter.getMappingContext().getRequiredPersistentEntity(Person.class)); + context.getRequiredPersistentEntity(Person.class)); assertThat(mappedObject).hasToString("COUNT(my_aliased_table.another_name)"); } @@ -171,7 +179,7 @@ public class QueryMapperUnitTests { Table table = Table.create("my_table").as("my_aliased_table"); Expression mappedObject = mapper.getMappedObject(table.column("unknown").as("my_aliased_col"), - converter.getMappingContext().getRequiredPersistentEntity(Person.class)); + context.getRequiredPersistentEntity(Person.class)); assertThat(mappedObject).hasToString("my_aliased_table.unknown AS my_aliased_col"); } @@ -352,7 +360,7 @@ public class QueryMapperUnitTests { Sort sort = Sort.by(desc("alternative")); - Sort mapped = mapper.getMappedObject(sort, converter.getMappingContext().getRequiredPersistentEntity(Person.class)); + Sort mapped = mapper.getMappedObject(sort, context.getRequiredPersistentEntity(Person.class)); assertThat(mapped.getOrderFor("another_name")).isEqualTo(desc("another_name")); assertThat(mapped.getOrderFor("alternative")).isNull(); @@ -363,7 +371,7 @@ public class QueryMapperUnitTests { BindMarkersFactory markers = BindMarkersFactory.indexed("$", 1); return mapper.getMappedObject(markers.create(), criteria, Table.create("person"), - converter.getMappingContext().getRequiredPersistentEntity(Person.class)); + context.getRequiredPersistentEntity(Person.class)); } static class Person {