Browse Source

Port changes to composite id handling from R2DBC to JDBC QueryMapper.

Also Criteria.where(...).notIn() with an empty collection should render to 1 = 1 (DATAJDBC-604).

See: #2225
pull/1213/merge
Christoph Strobl 1 month ago
parent
commit
ca3689412c
No known key found for this signature in database
GPG Key ID: E6054036D0C37A4B
  1. 106
      spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/QueryMapper.java
  2. 4
      spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlParametersFactory.java
  3. 44
      spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/QueryMapperUnitTests.java
  4. 2
      spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java
  5. 2
      spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java

106
spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/QueryMapper.java

@ -55,6 +55,7 @@ import org.springframework.util.ClassUtils; @@ -55,6 +55,7 @@ import org.springframework.util.ClassUtils;
* @author Jens Schauder
* @author Yan Qiang
* @author Mikhail Fedorov
* @author Christoph Strobl
* @since 3.0
*/
public class QueryMapper {
@ -277,9 +278,50 @@ public class QueryMapper { @@ -277,9 +278,50 @@ public class QueryMapper {
Assert.notNull(criteriaColumn, "Column must not be null");
Field propertyField = createPropertyField(entity, criteriaColumn, this.mappingContext);
Comparator comparator = criteria.getComparator();
Object value = criteria.getValue();
// Single embedded entity
if (propertyField.isEmbedded()) {
// IN/NOT_IN with collection of composite/embedded values: expand to (… AND …) OR (…)
if ((Comparator.IN.equals(comparator) || Comparator.NOT_IN.equals(comparator))
&& value instanceof Collection<?> collection) {
Condition condition = null;
for (Object o : collection) {
CriteriaWrapper cw = new CriteriaWrapper(criteria) {
@Override
public @Nullable Comparator getComparator() {
return Comparator.IN.equals(comparator) ? Comparator.EQ : Comparator.NEQ;
}
@Nullable
@Override
public Object getValue() {
return o;
}
@Override
public @Nullable SqlIdentifier getColumn() {
return criteriaColumn;
}
};
Condition c = mapCondition(cw, parameterSource, table, entity);
condition = condition == null ? c : condition.or(c);
}
if (condition == null) {
return Comparator.IN.equals(comparator) ? Conditions.unrestricted().not() : Conditions.unrestricted();
}
return condition;
}
PersistentPropertyPath<RelationalPersistentProperty> path = ((MetadataBackedField) propertyField).getPath();
Assert.state(path != null, "Path must not be null");
@ -291,7 +333,6 @@ public class QueryMapper { @@ -291,7 +333,6 @@ public class QueryMapper {
Column column = table.column(propertyField.getMappedColumnName());
Object mappedValue;
SQLType sqlType;
Comparator comparator = criteria.getComparator();
Assert.notNull(comparator, "Comparator must not be null");
@ -680,6 +721,69 @@ public class QueryMapper { @@ -680,6 +721,69 @@ public class QueryMapper {
return uniqueName;
}
abstract static class CriteriaWrapper implements CriteriaDefinition {
private final CriteriaDefinition delegate;
public CriteriaWrapper(CriteriaDefinition delegate) {
this.delegate = delegate;
}
@Nullable
@Override
public Comparator getComparator() {
return delegate.getComparator();
}
@Override
public boolean isIgnoreCase() {
return delegate.isIgnoreCase();
}
@Override
public boolean isGroup() {
return false;
}
@Override
public List<CriteriaDefinition> getGroup() {
return List.of();
}
@Nullable
@Override
public SqlIdentifier getColumn() {
return null;
}
@Nullable
@Override
public Object getValue() {
return null;
}
@Nullable
@Override
public CriteriaDefinition getPrevious() {
return null;
}
@Override
public boolean hasPrevious() {
return false;
}
@Override
public boolean isEmpty() {
return false;
}
@Override
public Combinator getCombinator() {
throw new UnsupportedOperationException("No combinator for AbstractCriteria");
}
}
/**
* Value object to represent a field and its meta-information.
*/

4
spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlParametersFactory.java

@ -154,7 +154,7 @@ public class SqlParametersFactory { @@ -154,7 +154,7 @@ public class SqlParametersFactory {
ParameterSourceHolder holder = new ParameterSourceHolder();
BiFunction<Object, AggregatePath, Object> valueExtractor = getIdMapper(complexId);
List<@Nullable Object> parameterValues = new ArrayList<>(ids instanceof Collection<?> c ? c.size() : 16);
List<Object> parameterValues = new ArrayList<>(ids instanceof Collection<?> c ? c.size() : 16);
if (complexId == null || columns.size() == 1) {
@ -164,7 +164,7 @@ public class SqlParametersFactory { @@ -164,7 +164,7 @@ public class SqlParametersFactory {
} else {
for (Object id : ids) {
List<@Nullable Object> tuple = new ArrayList<>(columns.size());
List<Object> tuple = new ArrayList<>(columns.size());
appendIdentifier(holder, columns, id, valueExtractor, tuple);
parameterValues.add(tuple.toArray(new Object[0]));
}

44
spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/QueryMapperUnitTests.java

@ -29,6 +29,7 @@ import org.junit.jupiter.api.Test; @@ -29,6 +29,7 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.springframework.core.convert.converter.Converter;
import org.springframework.data.annotation.Id;
import org.springframework.data.domain.Sort;
import org.springframework.data.jdbc.core.mapping.JdbcMappingContext;
import org.springframework.data.relational.core.mapping.Column;
@ -47,6 +48,7 @@ import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; @@ -47,6 +48,7 @@ import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
* @author Mark Paluch
* @author Jens Schauder
* @author Mikhail Fedorov
* @author Christoph Strobl
*/
public class QueryMapperUnitTests {
@ -322,6 +324,36 @@ public class QueryMapperUnitTests { @@ -322,6 +324,36 @@ public class QueryMapperUnitTests {
assertThat(condition).hasToString("person.\"NAME\" NOT IN (?[:name], ?[:name1], ?[:name2])");
}
@Test // GH-2208
void shouldMapInComposite() {
Criteria criteria = Criteria.where("id").in(new CompositeId(1, "a"));
assertThat(map(criteria, WithCompositeId.class))
.hasToString("(withcompositeid.\"TENANT\" = ?[:tenant] AND withcompositeid.\"NAME\" = ?[:name])");
criteria = Criteria.where("id").in(new CompositeId(1, "a"), new CompositeId(2, "b"));
assertThat(map(criteria, WithCompositeId.class)).hasToString(
"(withcompositeid.\"TENANT\" = ?[:tenant2] AND withcompositeid.\"NAME\" = ?[:name3]) OR (withcompositeid.\"TENANT\" = ?[:tenant4] AND withcompositeid.\"NAME\" = ?[:name5])");
criteria = Criteria.where("id").in();
assertThat(map(criteria, WithCompositeId.class)).hasToString("1 = 0");
}
@Test // GH-2208
void shouldMapNotInComposite() {
Criteria criteria = Criteria.where("id").notIn(new CompositeId(1, "a"));
assertThat(map(criteria, WithCompositeId.class))
.hasToString("(withcompositeid.\"TENANT\" != ?[:tenant] AND withcompositeid.\"NAME\" != ?[:name])");
criteria = Criteria.where("id").notIn(new CompositeId(1, "a"), new CompositeId(2, "b"));
assertThat(map(criteria, WithCompositeId.class)).hasToString(
"(withcompositeid.\"TENANT\" != ?[:tenant2] AND withcompositeid.\"NAME\" != ?[:name3]) OR (withcompositeid.\"TENANT\" != ?[:tenant4] AND withcompositeid.\"NAME\" != ?[:name5])");
criteria = Criteria.where("id").notIn();
assertThat(map(criteria, WithCompositeId.class)).hasToString("1 = 1");
}
@Test
void shouldMapIsNotInWithCollectionToStringConverter() {
@ -464,12 +496,24 @@ public class QueryMapperUnitTests { @@ -464,12 +496,24 @@ public class QueryMapperUnitTests {
context.getRequiredPersistentEntity(Person.class));
}
private Condition map(Criteria criteria, Class<?> entityType) {
return mapper.getMappedObject(parameterSource, criteria, Table.create("withcompositeid"),
context.getRequiredPersistentEntity(entityType));
}
static class Person {
String name;
@Column("another_name") String alternative;
}
record CompositeId(int tenant, String name) {
}
record WithCompositeId(@Id CompositeId id) {
}
enum CollectionToStringConverter implements Converter<Collection<?>, String> {
INSTANCE;

2
spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java

@ -396,7 +396,7 @@ public class QueryMapper { @@ -396,7 +396,7 @@ public class QueryMapper {
}
if (condition == null) {
return Conditions.unrestricted().not();
return Comparator.IN.equals(comparator) ? Conditions.unrestricted().not() : Conditions.unrestricted();
}
return condition;

2
spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java

@ -395,7 +395,7 @@ class QueryMapperUnitTests { @@ -395,7 +395,7 @@ class QueryMapperUnitTests {
"(withcompositeid.tenant != ?[$1] AND withcompositeid.name != ?[$2]) OR (withcompositeid.tenant != ?[$3] AND withcompositeid.name != ?[$4])");
criteria = Criteria.where("id").notIn();
assertThat(map(criteria, WithCompositeId.class).getCondition()).hasToString("1 = 0");
assertThat(map(criteria, WithCompositeId.class).getCondition()).hasToString("1 = 1");
}
@Test

Loading…
Cancel
Save