From aea4f4e95c76942cb1fbe97f63938be50d90a02c Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 9 Dec 2019 10:44:42 +0100 Subject: [PATCH] #252 - Fix EnumSet conversion. We now convert properly collections of enum values for read and write. --- .../r2dbc/convert/MappingR2dbcConverter.java | 203 ++++++++++++++++-- .../DefaultReactiveDataAccessStrategy.java | 13 +- .../convert/EntityRowMapperUnitTests.java | 27 +++ ...stgresReactiveDataAccessStrategyTests.java | 46 ++++ 4 files changed, 276 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java b/src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java index 18857b424..eae9519ab 100644 --- a/src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java +++ b/src/main/java/org/springframework/data/r2dbc/convert/MappingR2dbcConverter.java @@ -19,13 +19,16 @@ import io.r2dbc.spi.ColumnMetadata; import io.r2dbc.spi.Row; import io.r2dbc.spi.RowMetadata; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.function.BiFunction; +import org.springframework.core.CollectionFactory; import org.springframework.core.convert.ConversionService; import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.convert.CustomConversions; @@ -49,6 +52,7 @@ import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import org.springframework.util.CollectionUtils; /** * Converter for R2DBC. @@ -164,13 +168,76 @@ public class MappingR2dbcConverter extends BasicRelationalConverter implements R } Object value = row.get(identifier); - return getPotentiallyConvertedSimpleRead(value, property.getTypeInformation().getType()); + return readValue(value, property.getTypeInformation()); } catch (Exception o_O) { throw new MappingException(String.format("Could not read property %s from result set!", property), o_O); } } + public Object readValue(@Nullable Object value, TypeInformation type) { + + if (null == value) { + return null; + } + + if (getConversions().hasCustomReadTarget(value.getClass(), type.getType())) { + return getConversionService().convert(value, type.getType()); + } else if (value instanceof Collection || value.getClass().isArray()) { + return readCollectionOrArray(asCollection(value), type); + } else { + return getPotentiallyConvertedSimpleRead(value, type.getType()); + } + } + + /** + * Reads the given value into a collection of the given {@link TypeInformation}. + * + * @param source must not be {@literal null}. + * @param targetType must not be {@literal null}. + * @return the converted {@link Collection} or array, will never be {@literal null}. + */ + @SuppressWarnings("unchecked") + private Object readCollectionOrArray(Collection source, TypeInformation targetType) { + + Assert.notNull(targetType, "Target type must not be null!"); + + Class collectionType = targetType.isSubTypeOf(Collection.class) // + ? targetType.getType() // + : List.class; + + TypeInformation componentType = targetType.getComponentType() != null // + ? targetType.getComponentType() // + : ClassTypeInformation.OBJECT; + Class rawComponentType = componentType.getType(); + + Collection items = targetType.getType().isArray() // + ? new ArrayList<>(source.size()) // + : CollectionFactory.createCollection(collectionType, rawComponentType, source.size()); + + if (source.isEmpty()) { + return getPotentiallyConvertedSimpleRead(items, targetType.getType()); + } + + for (Object element : source) { + + if (!Object.class.equals(rawComponentType) && element instanceof Collection) { + if (!rawComponentType.isArray() && !ClassUtils.isAssignable(Iterable.class, rawComponentType)) { + throw new MappingException(String.format( + "Cannot convert %1$s of type %2$s into an instance of %3$s! Implement a custom Converter<%2$s, %3$s> and register it with the CustomConversions", + element, element.getClass(), rawComponentType)); + } + } + if (element instanceof List) { + items.add(readCollectionOrArray((Collection) element, componentType)); + } else { + items.add(getPotentiallyConvertedSimpleRead(element, rawComponentType)); + } + } + + return getPotentiallyConvertedSimpleRead(items, targetType.getType()); + } + /** * Checks whether we have a custom conversion for the given simple object. Converts the given value if so, applies * {@link Enum} handling or returns the value as is. @@ -283,15 +350,11 @@ public class MappingR2dbcConverter extends BasicRelationalConverter implements R continue; } - if (!getConversions().isSimpleType(value.getClass())) { - - RelationalPersistentEntity nestedEntity = getMappingContext().getPersistentEntity(property.getActualType()); - if (nestedEntity != null) { - throw new InvalidDataAccessApiUsageException("Nested entities are not supported"); - } + if (getConversions().isSimpleType(value.getClass())) { + writeSimpleInternal(sink, value, property); + } else { + writePropertyInternal(sink, value, property); } - - writeSimpleInternal(sink, value, property); } } @@ -299,6 +362,75 @@ public class MappingR2dbcConverter extends BasicRelationalConverter implements R sink.put(property.getColumnName(), SettableValue.from(getPotentiallyConvertedSimpleWrite(value))); } + private void writePropertyInternal(OutboundRow sink, Object value, RelationalPersistentProperty property) { + + TypeInformation valueType = ClassTypeInformation.from(value.getClass()); + + if (valueType.isCollectionLike()) { + + if (valueType.getActualType() != null && valueType.getRequiredActualType().isCollectionLike()) { + + // pass-thru nested collections + writeSimpleInternal(sink, value, property); + return; + } + + List collectionInternal = createCollection(asCollection(value), property); + sink.put(property.getColumnName(), SettableValue.from(collectionInternal)); + return; + } + + throw new InvalidDataAccessApiUsageException("Nested entities are not supported"); + } + + /** + * Writes the given {@link Collection} using the given {@link RelationalPersistentProperty} information. + * + * @param collection must not be {@literal null}. + * @param property must not be {@literal null}. + * @return + */ + protected List createCollection(Collection collection, RelationalPersistentProperty property) { + return writeCollectionInternal(collection, property.getTypeInformation(), new ArrayList<>()); + } + + /** + * Populates the given {@link Collection sink} with converted values from the given {@link Collection source}. + * + * @param source the collection to create a {@link Collection} for, must not be {@literal null}. + * @param type the {@link TypeInformation} to consider or {@literal null} if unknown. + * @param sink the {@link Collection} to write to. + * @return + */ + @SuppressWarnings("unchecked") + private List writeCollectionInternal(Collection source, @Nullable TypeInformation type, + Collection sink) { + + TypeInformation componentType = null; + + List collection = sink instanceof List ? (List) sink : new ArrayList<>(sink); + + if (type != null) { + componentType = type.getComponentType(); + } + + for (Object element : source) { + + Class elementType = element == null ? null : element.getClass(); + + if (elementType == null || getConversions().isSimpleType(elementType)) { + collection.add(getPotentiallyConvertedSimpleWrite(element, + componentType != null ? componentType.getType() : Object.class)); + } else if (element instanceof Collection || elementType.isArray()) { + collection.add(writeCollectionInternal(asCollection(element), componentType, new ArrayList<>())); + } else { + throw new InvalidDataAccessApiUsageException("Nested entities are not supported"); + } + } + + return collection; + } + private void writeNullInternal(OutboundRow sink, RelationalPersistentProperty property) { sink.put(property.getColumnName(), SettableValue.empty(getPotentiallyConvertedSimpleNullType(property.getType()))); @@ -321,19 +453,38 @@ public class MappingR2dbcConverter extends BasicRelationalConverter implements R } /** - * Checks whether we have a custom conversion registered for the given value into an arbitrary simple Mongo type. - * Returns the converted value if so. If not, we perform special enum handling or simply return the value as is. + * Checks whether we have a custom conversion registered for the given value into an arbitrary simple type. Returns + * the converted value if so. If not, we perform special enum handling or simply return the value as is. * * @param value * @return */ @Nullable private Object getPotentiallyConvertedSimpleWrite(@Nullable Object value) { + return getPotentiallyConvertedSimpleWrite(value, Object.class); + } + + /** + * Checks whether we have a custom conversion registered for the given value into an arbitrary simple type. Returns + * the converted value if so. If not, we perform special enum handling or simply return the value as is. + * + * @param value + * @return + */ + @Nullable + private Object getPotentiallyConvertedSimpleWrite(@Nullable Object value, Class typeHint) { if (value == null) { return null; } + if (Object.class != typeHint) { + + if (getConversionService().canConvert(value.getClass(), typeHint)) { + value = getConversionService().convert(value, typeHint); + } + } + Optional> customTarget = getConversions().getCustomWriteTarget(value.getClass()); if (customTarget.isPresent()) { @@ -350,7 +501,18 @@ public class MappingR2dbcConverter extends BasicRelationalConverter implements R @Override public Object getArrayValue(ArrayColumns arrayColumns, RelationalPersistentProperty property, Object value) { - Class targetType = arrayColumns.getArrayType(property.getActualType()); + Class actualType = null; + if (value instanceof Collection) { + actualType = CollectionUtils.findCommonElementType((Collection) value); + } else if (value.getClass().isArray()) { + actualType = value.getClass().getComponentType(); + } + + if (actualType == null) { + actualType = property.getActualType(); + } + + Class targetType = arrayColumns.getArrayType(actualType); if (!property.isArray() || !targetType.isAssignableFrom(value.getClass())) { @@ -427,6 +589,23 @@ public class MappingR2dbcConverter extends BasicRelationalConverter implements R return (RelationalPersistentEntity) getMappingContext().getRequiredPersistentEntity(type); } + /** + * Returns given object as {@link Collection}. Will return the {@link Collection} as is if the source is a + * {@link Collection} already, will convert an array into a {@link Collection} or simply create a single element + * collection for everything else. + * + * @param source + * @return + */ + private static Collection asCollection(Object source) { + + if (source instanceof Collection) { + return (Collection) source; + } + + return source.getClass().isArray() ? CollectionUtils.arrayToList(source) : Collections.singleton(source); + } + private static Map createMetadataMap(RowMetadata metadata) { Map columns = new LinkedHashMap<>(); 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 f62d93b94..9e36de377 100644 --- a/src/main/java/org/springframework/data/r2dbc/core/DefaultReactiveDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/r2dbc/core/DefaultReactiveDataAccessStrategy.java @@ -47,6 +47,7 @@ import org.springframework.data.relational.core.mapping.RelationalPersistentProp import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import org.springframework.util.CollectionUtils; /** * Default {@link ReactiveDataAccessStrategy} implementation. @@ -244,7 +245,17 @@ public class DefaultReactiveDataAccessStrategy implements ReactiveDataAccessStra throw new InvalidDataAccessResourceUsageException( "Dialect " + this.dialect.getClass().getName() + " does not support array columns"); } - Class actualType = property.getActualType(); + + Class actualType = null; + if (value instanceof Collection) { + actualType = CollectionUtils.findCommonElementType((Collection) value); + } else if (value.getClass().isArray()) { + actualType = value.getClass().getComponentType(); + } + + if (actualType == null) { + actualType = property.getActualType(); + } if (value.isEmpty()) { diff --git a/src/test/java/org/springframework/data/r2dbc/convert/EntityRowMapperUnitTests.java b/src/test/java/org/springframework/data/r2dbc/convert/EntityRowMapperUnitTests.java index 360b528d4..1bd63b66d 100644 --- a/src/test/java/org/springframework/data/r2dbc/convert/EntityRowMapperUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/convert/EntityRowMapperUnitTests.java @@ -8,6 +8,7 @@ import io.r2dbc.spi.RowMetadata; import lombok.RequiredArgsConstructor; import java.util.Collection; +import java.util.EnumSet; import java.util.List; import java.util.Set; @@ -111,6 +112,20 @@ public class EntityRowMapperUnitTests { assertThat(result.boxedIntegers).contains(3, 11); } + @Test // gh-252 + public void shouldReadEnums() { + + EntityRowMapper mapper = getRowMapper(WithEnumCollections.class); + when(rowMock.get("enum_array")).thenReturn((new String[] { "ONE", "TWO" })); + when(rowMock.get("set_of_enum")).thenReturn((new String[] { "ONE", "THREE" })); + when(rowMock.get("enum_set")).thenReturn((new String[] { "ONE", "TWO" })); + + WithEnumCollections result = mapper.apply(rowMock, metadata); + assertThat(result.enumArray).contains(MyEnum.ONE, MyEnum.TWO); + assertThat(result.setOfEnum).contains(MyEnum.ONE, MyEnum.THREE); + assertThat(result.enumSet).contains(MyEnum.ONE, MyEnum.TWO); + } + private EntityRowMapper getRowMapper(Class type) { return new EntityRowMapper<>(type, strategy.getConverter()); } @@ -135,4 +150,16 @@ public class EntityRowMapperUnitTests { Integer[] boxedIntegers; int[] primitiveIntegers; } + + static class WithEnumCollections { + + MyEnum[] enumArray; + Set setOfEnum; + EnumSet enumSet; + } + + enum MyEnum { + ONE, TWO, THREE; + } + } 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 b0303fd55..333d82502 100644 --- a/src/test/java/org/springframework/data/r2dbc/core/PostgresReactiveDataAccessStrategyTests.java +++ b/src/test/java/org/springframework/data/r2dbc/core/PostgresReactiveDataAccessStrategyTests.java @@ -21,7 +21,9 @@ import lombok.RequiredArgsConstructor; import java.util.Arrays; import java.util.Collections; +import java.util.EnumSet; import java.util.List; +import java.util.Set; import org.junit.Test; @@ -123,6 +125,40 @@ public class PostgresReactiveDataAccessStrategyTests extends ReactiveDataAccessS assertThat(value.getType()).isEqualTo(String.class); } + @Test // gh-252 + public void shouldConvertSetOfEnumToString() { + + DefaultReactiveDataAccessStrategy strategy = new DefaultReactiveDataAccessStrategy(PostgresDialect.INSTANCE, + Collections.singletonList(MyObjectsToStringConverter.INSTANCE)); + + WithEnumCollections withEnums = new WithEnumCollections(); + withEnums.enumSet = EnumSet.of(MyEnum.ONE, MyEnum.TWO); + + OutboundRow outboundRow = strategy.getOutboundRow(withEnums); + + assertThat(outboundRow).containsKey("enum_set"); + + SettableValue value = outboundRow.get("enum_set"); + assertThat(value.getValue()).isEqualTo(new String[] { "ONE", "TWO" }); + } + + @Test // gh-252 + public void shouldConvertArrayOfEnumToString() { + + DefaultReactiveDataAccessStrategy strategy = new DefaultReactiveDataAccessStrategy(PostgresDialect.INSTANCE, + Collections.singletonList(MyObjectsToStringConverter.INSTANCE)); + + WithEnumCollections withEnums = new WithEnumCollections(); + withEnums.enumArray = new MyEnum[] { MyEnum.ONE, MyEnum.TWO }; + + OutboundRow outboundRow = strategy.getOutboundRow(withEnums); + + assertThat(outboundRow).containsKey("enum_array"); + + SettableValue value = outboundRow.get("enum_array"); + assertThat(value.getValue()).isEqualTo(new String[] { "ONE", "TWO" }); + } + @RequiredArgsConstructor static class WithMultidimensionalArray { @@ -141,6 +177,12 @@ public class PostgresReactiveDataAccessStrategyTests extends ReactiveDataAccessS List stringList; } + static class WithEnumCollections { + + MyEnum[] enumArray; + Set enumSet; + } + static class WithConversion { List myObjects; @@ -159,6 +201,10 @@ public class PostgresReactiveDataAccessStrategyTests extends ReactiveDataAccessS } } + enum MyEnum { + ONE, TWO, THREE; + } + @WritingConverter enum MyObjectsToStringConverter implements Converter, String> {