Skip to content

Only the contents of collections gets converted. #1356

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 3 commits into from
Closed

Only the contents of collections gets converted. #1356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ec556bd
prepare branch for 1343-converter-for-iterable
schauder Oct 11, 2022
d0a08f6
Polishing.
schauder Oct 11, 2022
365586c
Only the contents of collections gets converted.
schauder Oct 11, 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
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>3.1.0-SNAPSHOT</version>
<version>3.1.0-1343-converter-for-iterable-SNAPSHOT</version>
<packaging>pom</packaging>

<name>Spring Data Relational Parent</name>
Expand Down
2 changes: 1 addition & 1 deletion spring-data-jdbc-distribution/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>3.1.0-SNAPSHOT</version>
<version>3.1.0-1343-converter-for-iterable-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
4 changes: 2 additions & 2 deletions spring-data-jdbc/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<modelVersion>4.0.0</modelVersion>

<artifactId>spring-data-jdbc</artifactId>
<version>3.1.0-SNAPSHOT</version>
<version>3.1.0-1343-converter-for-iterable-SNAPSHOT</version>

<name>Spring Data JDBC</name>
<description>Spring Data module for JDBC repositories.</description>
Expand All @@ -15,7 +15,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>3.1.0-SNAPSHOT</version>
<version>3.1.0-1343-converter-for-iterable-SNAPSHOT</version>
</parent>

<properties>
Expand Down
3 changes: 2 additions & 1 deletion ...bc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.lang.reflect.Constructor;
import java.sql.SQLType;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;

import org.springframework.beans.BeanUtils;
Expand Down Expand Up @@ -189,7 +190,7 @@ private void convertAndAddParameter(MapSqlParameterSource parameters, Parameter
Assert.notNull(type, "@Query parameter type could not be resolved");

JdbcValue jdbcValue;
if (value instanceof Iterable) {
if (value instanceof Collection && resolvableType.hasGenerics()) {
Copy link
Member

Choose a reason for hiding this comment

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

It would make sense to follow our general conventions to include Iterable and Streamable types but exclude custom domain types that implement Iterable.

See https://github.com/spring-projects/spring-data-commons/blob/main/src/main/java/org/springframework/data/util/TypeDiscoverer.java#L118-L126 for further reference. Also, we should consider CustomCollections to support usage of e.g. vavr collection types as follow up.


List<Object> mapped = new ArrayList<>();
SQLType jdbcType = null;
Expand Down
138 changes: 104 additions & 34 deletions ...st/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@
import java.sql.JDBCType;
import java.sql.ResultSet;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Properties;
import java.util.Set;
import java.util.stream.Stream;

import org.assertj.core.api.Assertions;
import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
Expand Down Expand Up @@ -194,57 +196,96 @@ void pageQueryNotSupported() {
@Test // GH-1212
void convertsEnumCollectionParameterIntoStringCollectionParameter() {

JdbcQueryMethod queryMethod = createMethod("findByEnumTypeIn", Set.class);
BasicJdbcConverter converter = new BasicJdbcConverter(mock(RelationalMappingContext.class),
mock(RelationResolver.class));
StringBasedJdbcQuery query = new StringBasedJdbcQuery(queryMethod, operations, result -> mock(RowMapper.class),
converter, evaluationContextProvider);
SqlParameterSource sqlParameterSource = forMethod("findByEnumTypeIn", Set.class)
.withArguments(Set.of(Direction.LEFT, Direction.RIGHT)).extractParameterSource();

query.execute(new Object[] { Set.of(Direction.LEFT, Direction.RIGHT) });

ArgumentCaptor<SqlParameterSource> captor = ArgumentCaptor.forClass(SqlParameterSource.class);
verify(operations).query(anyString(), captor.capture(), any(ResultSetExtractor.class));

SqlParameterSource sqlParameterSource = captor.getValue();
assertThat(sqlParameterSource.getValue("directions")).asList().containsExactlyInAnyOrder("LEFT", "RIGHT");
}

@Test // GH-1212
void convertsEnumCollectionParameterUsingCustomConverterWhenRegisteredForType() {

JdbcQueryMethod queryMethod = createMethod("findByEnumTypeIn", Set.class);
BasicJdbcConverter converter = new BasicJdbcConverter(mock(RelationalMappingContext.class),
mock(RelationResolver.class),
new JdbcCustomConversions(List.of(DirectionToIntegerConverter.INSTANCE, IntegerToDirectionConverter.INSTANCE)),
JdbcTypeFactory.unsupported(), IdentifierProcessing.ANSI);
StringBasedJdbcQuery query = new StringBasedJdbcQuery(queryMethod, operations, result -> mock(RowMapper.class),
converter, evaluationContextProvider);

query.execute(new Object[] { Set.of(Direction.LEFT, Direction.RIGHT) });

ArgumentCaptor<SqlParameterSource> captor = ArgumentCaptor.forClass(SqlParameterSource.class);
verify(operations).query(anyString(), captor.capture(), any(ResultSetExtractor.class));
SqlParameterSource sqlParameterSource = forMethod("findByEnumTypeIn", Set.class) //
.withCustomConverters(DirectionToIntegerConverter.INSTANCE, IntegerToDirectionConverter.INSTANCE)
.withArguments(Set.of(Direction.LEFT, Direction.RIGHT)) //
.extractParameterSource();

SqlParameterSource sqlParameterSource = captor.getValue();
assertThat(sqlParameterSource.getValue("directions")).asList().containsExactlyInAnyOrder(-1, 1);
}

@Test // GH-1212
void doesNotConvertNonCollectionParameter() {

JdbcQueryMethod queryMethod = createMethod("findBySimpleValue", Integer.class);
BasicJdbcConverter converter = new BasicJdbcConverter(mock(RelationalMappingContext.class),
mock(RelationResolver.class));
StringBasedJdbcQuery query = new StringBasedJdbcQuery(queryMethod, operations, result -> mock(RowMapper.class),
converter, evaluationContextProvider);
SqlParameterSource sqlParameterSource = forMethod("findBySimpleValue", Integer.class) //
.withArguments(1) //
.extractParameterSource();

query.execute(new Object[] { 1 });
assertThat(sqlParameterSource.getValue("value")).isEqualTo(1);
}

ArgumentCaptor<SqlParameterSource> captor = ArgumentCaptor.forClass(SqlParameterSource.class);
verify(operations).query(anyString(), captor.capture(), any(ResultSetExtractor.class));
@Test // GH-1343
void appliesConverterToIterable() {

SqlParameterSource sqlParameterSource = captor.getValue();
assertThat(sqlParameterSource.getValue("value")).isEqualTo(1);
SqlParameterSource sqlParameterSource = forMethod("findByListContainer", ListContainer.class) //
.withCustomConverters(ListContainerToStringConverter.INSTANCE)
.withArguments(new ListContainer("one", "two", "three")) //
.extractParameterSource();

assertThat(sqlParameterSource.getValue("value")).isEqualTo("one");

}

QueryFixture forMethod(String name, Class... paramTypes) {
return new QueryFixture(createMethod(name, paramTypes));
}

private class QueryFixture {

private final JdbcQueryMethod method;
private Object[] arguments;
private BasicJdbcConverter converter;

public QueryFixture(JdbcQueryMethod method) {
this.method = method;
}

public QueryFixture withArguments(Object... arguments) {

this.arguments = arguments;

return this;
}

public SqlParameterSource extractParameterSource() {

BasicJdbcConverter converter = this.converter == null //
? new BasicJdbcConverter(mock(RelationalMappingContext.class), //
mock(RelationResolver.class))
: this.converter;

StringBasedJdbcQuery query = new StringBasedJdbcQuery(method, operations, result -> mock(RowMapper.class),
converter, evaluationContextProvider);

query.execute(arguments);

ArgumentCaptor<SqlParameterSource> captor = ArgumentCaptor.forClass(SqlParameterSource.class);
verify(operations).query(anyString(), captor.capture(), any(ResultSetExtractor.class));

return captor.getValue();
}

public QueryFixture withConverter(BasicJdbcConverter converter) {

this.converter = converter;

return this;
}

public QueryFixture withCustomConverters(Object... converters) {

return withConverter(new BasicJdbcConverter(mock(RelationalMappingContext.class), mock(RelationResolver.class),
new JdbcCustomConversions(List.of(converters)), JdbcTypeFactory.unsupported(), IdentifierProcessing.ANSI));
}
}

private JdbcQueryMethod createMethod(String methodName, Class<?>... paramTypes) {
Expand Down Expand Up @@ -293,6 +334,9 @@ interface MyRepository extends Repository<Object, Long> {
@Query(value = "some sql statement")
List<Object> findBySimpleValue(Integer value);

@Query(value = "some sql statement")
List<Object> findByListContainer(ListContainer value);

@Query("SELECT * FROM table WHERE c = :#{myext.testValue} AND c2 = :#{myext.doSomething()}")
Object findBySpelExpression(Object object);
}
Expand Down Expand Up @@ -389,6 +433,32 @@ public Direction convert(Integer source) {
}
}

static class ListContainer implements Iterable<String> {

private final List<String> values;

ListContainer(String... values) {
this.values = List.of(values);
}

@NotNull
@Override
public Iterator<String> iterator() {
return values.iterator();
}
}

@WritingConverter
enum ListContainerToStringConverter implements Converter<ListContainer, String> {

INSTANCE;

@Override
public String convert(ListContainer source) {
return source.values.get(0);
}
}

private static class DummyEntity {
private Long id;

Expand Down
4 changes: 2 additions & 2 deletions spring-data-r2dbc/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<modelVersion>4.0.0</modelVersion>

<artifactId>spring-data-r2dbc</artifactId>
<version>3.1.0-SNAPSHOT</version>
<version>3.1.0-1343-converter-for-iterable-SNAPSHOT</version>

<name>Spring Data R2DBC</name>
<description>Spring Data module for R2DBC</description>
Expand All @@ -15,7 +15,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>3.1.0-SNAPSHOT</version>
<version>3.1.0-1343-converter-for-iterable-SNAPSHOT</version>
</parent>

<properties>
Expand Down
4 changes: 2 additions & 2 deletions spring-data-relational/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@
<modelVersion>4.0.0</modelVersion>

<artifactId>spring-data-relational</artifactId>
<version>3.1.0-SNAPSHOT</version>
<version>3.1.0-1343-converter-for-iterable-SNAPSHOT</version>

<name>Spring Data Relational</name>
<description>Spring Data Relational support</description>

<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>3.1.0-SNAPSHOT</version>
<version>3.1.0-1343-converter-for-iterable-SNAPSHOT</version>
</parent>

<properties>
Expand Down