From 016aab33f2a4e8e2c93fec47f151b52e014f4f5b Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 3 May 2021 15:53:15 +0200 Subject: [PATCH] Correctly translate absent convertible enum values to String[] using Postgres. We now correctly obtain the target type for a persistent property that should be converted into an array value prior to constructing Parameter. Previously, we fell back to the actual type without considering potential converters which left collection-like enum properties without a value with the enum array type that was potentially not supported by the database driver. Closes #593 --- .../DefaultReactiveDataAccessStrategy.java | 6 +- ...stgresReactiveDataAccessStrategyTests.java | 78 ++++++++++++++++--- .../r2dbc/query/QueryMapperUnitTests.java | 15 ++++ 3 files changed, 86 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/springframework/data/r2dbc/core/DefaultReactiveDataAccessStrategy.java b/src/main/java/org/springframework/data/r2dbc/core/DefaultReactiveDataAccessStrategy.java index 70fa1b604..3401640ea 100644 --- a/src/main/java/org/springframework/data/r2dbc/core/DefaultReactiveDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/r2dbc/core/DefaultReactiveDataAccessStrategy.java @@ -247,14 +247,16 @@ public class DefaultReactiveDataAccessStrategy implements ReactiveDataAccessStra Class actualType = null; if (value.getValue() instanceof Collection) { actualType = CollectionUtils.findCommonElementType((Collection) value.getValue()); - } else if (value.getClass().isArray()) { - actualType = value.getClass().getComponentType(); + } else if (!value.isEmpty() && value.getValue().getClass().isArray()) { + actualType = value.getValue().getClass().getComponentType(); } if (actualType == null) { actualType = property.getActualType(); } + actualType = converter.getTargetType(actualType); + if (value.isEmpty()) { Class targetType = arrayColumns.getArrayType(actualType); diff --git a/src/test/java/org/springframework/data/r2dbc/core/PostgresReactiveDataAccessStrategyTests.java b/src/test/java/org/springframework/data/r2dbc/core/PostgresReactiveDataAccessStrategyTests.java index af3998b22..238a818ef 100644 --- a/src/test/java/org/springframework/data/r2dbc/core/PostgresReactiveDataAccessStrategyTests.java +++ b/src/test/java/org/springframework/data/r2dbc/core/PostgresReactiveDataAccessStrategyTests.java @@ -29,6 +29,7 @@ import org.junit.jupiter.api.Test; import org.springframework.core.convert.converter.Converter; import org.springframework.data.convert.WritingConverter; +import org.springframework.data.r2dbc.convert.EnumWriteSupport; import org.springframework.data.r2dbc.dialect.PostgresDialect; import org.springframework.data.r2dbc.mapping.OutboundRow; import org.springframework.data.relational.core.sql.SqlIdentifier; @@ -126,38 +127,90 @@ public class PostgresReactiveDataAccessStrategyTests extends ReactiveDataAccessS assertThat(value.getType()).isEqualTo(String.class); } - @Test // gh-252 - void shouldConvertSetOfEnumToString() { + @Test // gh-252, gh-593 + void shouldConvertCollectionOfEnumToString() { - DefaultReactiveDataAccessStrategy strategy = new DefaultReactiveDataAccessStrategy(PostgresDialect.INSTANCE, - Collections.singletonList(MyObjectsToStringConverter.INSTANCE)); + DefaultReactiveDataAccessStrategy strategy = new DefaultReactiveDataAccessStrategy(PostgresDialect.INSTANCE); WithEnumCollections withEnums = new WithEnumCollections(); withEnums.enumSet = EnumSet.of(MyEnum.ONE, MyEnum.TWO); + withEnums.enumList = Arrays.asList(MyEnum.ONE, MyEnum.TWO); + withEnums.enumArray = new MyEnum[] { MyEnum.ONE, MyEnum.TWO }; OutboundRow outboundRow = strategy.getOutboundRow(withEnums); assertThat(outboundRow).containsKey(SqlIdentifier.unquoted("enum_set")); + assertThat(outboundRow.get(SqlIdentifier.unquoted("enum_set")).getValue()).isEqualTo(new String[] { "ONE", "TWO" }); + + assertThat(outboundRow).containsKey(SqlIdentifier.unquoted("enum_array")); + assertThat(outboundRow.get(SqlIdentifier.unquoted("enum_array")).getValue()) + .isEqualTo(new String[] { "ONE", "TWO" }); + + assertThat(outboundRow).containsKey(SqlIdentifier.unquoted("enum_list")); + assertThat(outboundRow.get(SqlIdentifier.unquoted("enum_list")).getValue()) + .isEqualTo(new String[] { "ONE", "TWO" }); + } + + @Test // gh-593 + void shouldCorrectlyWriteConvertedEnumNullValues() { + + DefaultReactiveDataAccessStrategy strategy = new DefaultReactiveDataAccessStrategy(PostgresDialect.INSTANCE); + + WithEnumCollections withEnums = new WithEnumCollections(); + + OutboundRow outboundRow = strategy.getOutboundRow(withEnums); + + assertThat(outboundRow).containsKey(SqlIdentifier.unquoted("enum_set")); + assertThat(outboundRow.get(SqlIdentifier.unquoted("enum_set")).getType()).isEqualTo(String[].class); + + assertThat(outboundRow).containsKey(SqlIdentifier.unquoted("enum_array")); + assertThat(outboundRow.get(SqlIdentifier.unquoted("enum_array")).getType()).isEqualTo(String[].class); - Parameter value = outboundRow.get(SqlIdentifier.unquoted("enum_set")); - assertThat(value.getValue()).isEqualTo(new String[] { "ONE", "TWO" }); + assertThat(outboundRow).containsKey(SqlIdentifier.unquoted("enum_list")); + assertThat(outboundRow.get(SqlIdentifier.unquoted("enum_list")).getType()).isEqualTo(String[].class); } - @Test // gh-252 - void shouldConvertArrayOfEnumToString() { + @Test // gh-593 + void shouldConvertCollectionOfEnumNatively() { DefaultReactiveDataAccessStrategy strategy = new DefaultReactiveDataAccessStrategy(PostgresDialect.INSTANCE, - Collections.singletonList(MyObjectsToStringConverter.INSTANCE)); + Collections.singletonList(new MyEnumSupport())); WithEnumCollections withEnums = new WithEnumCollections(); + withEnums.enumSet = EnumSet.of(MyEnum.ONE, MyEnum.TWO); + withEnums.enumList = Arrays.asList(MyEnum.ONE, MyEnum.TWO); withEnums.enumArray = new MyEnum[] { MyEnum.ONE, MyEnum.TWO }; OutboundRow outboundRow = strategy.getOutboundRow(withEnums); + assertThat(outboundRow).containsKey(SqlIdentifier.unquoted("enum_set")); + assertThat(outboundRow.get(SqlIdentifier.unquoted("enum_set")).getValue()).isInstanceOf(MyEnum[].class); + + assertThat(outboundRow).containsKey(SqlIdentifier.unquoted("enum_array")); + assertThat(outboundRow.get(SqlIdentifier.unquoted("enum_array")).getValue()).isInstanceOf(MyEnum[].class); + + assertThat(outboundRow).containsKey(SqlIdentifier.unquoted("enum_list")); + assertThat(outboundRow.get(SqlIdentifier.unquoted("enum_list")).getValue()).isInstanceOf(MyEnum[].class); + } + + @Test // gh-593 + void shouldCorrectlyWriteNativeEnumNullValues() { + + DefaultReactiveDataAccessStrategy strategy = new DefaultReactiveDataAccessStrategy(PostgresDialect.INSTANCE, + Collections.singletonList(new MyEnumSupport())); + + WithEnumCollections withEnums = new WithEnumCollections(); + + OutboundRow outboundRow = strategy.getOutboundRow(withEnums); + + assertThat(outboundRow).containsKey(SqlIdentifier.unquoted("enum_set")); + assertThat(outboundRow.get(SqlIdentifier.unquoted("enum_set")).getType()).isEqualTo(MyEnum[].class); + assertThat(outboundRow).containsKey(SqlIdentifier.unquoted("enum_array")); + assertThat(outboundRow.get(SqlIdentifier.unquoted("enum_array")).getType()).isEqualTo(MyEnum[].class); - Parameter value = outboundRow.get(SqlIdentifier.unquoted("enum_array")); - assertThat(value.getValue()).isEqualTo(new String[] { "ONE", "TWO" }); + assertThat(outboundRow).containsKey(SqlIdentifier.unquoted("enum_list")); + assertThat(outboundRow.get(SqlIdentifier.unquoted("enum_list")).getType()).isEqualTo(MyEnum[].class); } @RequiredArgsConstructor @@ -182,6 +235,7 @@ public class PostgresReactiveDataAccessStrategyTests extends ReactiveDataAccessS MyEnum[] enumArray; Set enumSet; + List enumList; } static class WithConversion { @@ -216,4 +270,6 @@ public class PostgresReactiveDataAccessStrategyTests extends ReactiveDataAccessS return myObjects.toString(); } } + + private static class MyEnumSupport extends EnumWriteSupport {} } 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 2c5592b56..dca6f465f 100644 --- a/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java @@ -417,6 +417,16 @@ class QueryMapperUnitTests { assertThat(bindings.getCondition()).hasToString("person.alternative_name = ?[$1]"); } + @Test // gh-593 + void mapQueryForEnumArrayShouldMapToStringList() { + + Criteria criteria = Criteria.where("enumValue").in(MyEnum.ONE, MyEnum.TWO); + + BoundCondition bindings = map(criteria); + + assertThat(bindings.getCondition()).hasToString("person.enum_value IN (?[$1], ?[$2])"); + } + private BoundCondition map(Criteria criteria) { BindMarkersFactory markers = BindMarkersFactory.indexed("$", 1); @@ -429,5 +439,10 @@ class QueryMapperUnitTests { String name; @Column("another_name") String alternative; + MyEnum enumValue; + } + + enum MyEnum { + ONE, TWO, } }