From cac1a113f47a8a77e07059ad284d4cd96e02e396 Mon Sep 17 00:00:00 2001 From: Ryland Degnan Date: Wed, 21 Nov 2018 13:24:49 -0500 Subject: [PATCH 1/6] #22 - Remove restriction on CollectionLike types. EntityRowMapper now passes-thru values for Collection-like types such as array. Arrays are supported by Postgres. Original pull request: #22. --- .../data/r2dbc/function/convert/EntityRowMapper.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/main/java/org/springframework/data/r2dbc/function/convert/EntityRowMapper.java b/src/main/java/org/springframework/data/r2dbc/function/convert/EntityRowMapper.java index c1c67b26..33cffd8f 100644 --- a/src/main/java/org/springframework/data/r2dbc/function/convert/EntityRowMapper.java +++ b/src/main/java/org/springframework/data/r2dbc/function/convert/EntityRowMapper.java @@ -66,10 +66,7 @@ public T apply(Row row, RowMetadata metadata) { continue; } - if (property.isCollectionLike()) { - throw new UnsupportedOperationException(); - } else if (property.isMap()) { - + if (property.isMap()) { throw new UnsupportedOperationException(); } else { propertyAccessor.setProperty(property, readFrom(row, property, "")); From aa5d752500075aaa72670c83913e8076ec819ed5 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 3 Dec 2018 17:30:48 +0100 Subject: [PATCH 2/6] #22 - Polishing. Add author tag. Add unit test for EntityRowMapper. --- .../function/convert/EntityRowMapper.java | 30 ++++--- .../convert/EntityRowMapperUnitTests.java | 79 +++++++++++++++++++ 2 files changed, 96 insertions(+), 13 deletions(-) create mode 100644 src/test/java/org/springframework/data/r2dbc/function/convert/EntityRowMapperUnitTests.java diff --git a/src/main/java/org/springframework/data/r2dbc/function/convert/EntityRowMapper.java b/src/main/java/org/springframework/data/r2dbc/function/convert/EntityRowMapper.java index 33cffd8f..2f9e89c3 100644 --- a/src/main/java/org/springframework/data/r2dbc/function/convert/EntityRowMapper.java +++ b/src/main/java/org/springframework/data/r2dbc/function/convert/EntityRowMapper.java @@ -23,7 +23,6 @@ import java.sql.ResultSet; import java.util.function.BiFunction; -import org.springframework.core.convert.ConversionService; import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PersistentPropertyAccessor; @@ -39,7 +38,7 @@ * Maps a {@link io.r2dbc.spi.Row} to an entity of type {@code T}, including entities referenced. * * @author Mark Paluch - * @since 1.0 + * @author Ryland Degnan */ public class EntityRowMapper implements BiFunction { @@ -52,13 +51,17 @@ public EntityRowMapper(RelationalPersistentEntity entity, RelationalConverter this.converter = converter; } + /* + * (non-Javadoc) + * @see java.util.function.BiFunction#apply(java.lang.Object, java.lang.Object) + */ @Override public T apply(Row row, RowMetadata metadata) { T result = createInstance(row, "", entity); - ConvertingPropertyAccessor propertyAccessor = new ConvertingPropertyAccessor(entity.getPropertyAccessor(result), - converter.getConversionService()); + ConvertingPropertyAccessor propertyAccessor = new ConvertingPropertyAccessor<>( + entity.getPropertyAccessor(result), converter.getConversionService()); for (RelationalPersistentProperty property : entity) { @@ -93,7 +96,7 @@ private Object readFrom(Row row, RelationalPersistentProperty property, String p return readEntityFrom(row, property); } - return row.get(prefix + property.getColumnName()); + return converter.readValue(row.get(prefix + property.getColumnName()), property.getTypeInformation()); } catch (Exception o_O) { throw new MappingException(String.format("Could not read property %s from result set!", property), o_O); @@ -104,7 +107,6 @@ private S readEntityFrom(Row row, PersistentProperty property) { String prefix = property.getName() + "_"; - @SuppressWarnings("unchecked") RelationalPersistentEntity entity = (RelationalPersistentEntity) converter.getMappingContext() .getRequiredPersistentEntity(property.getActualType()); @@ -114,8 +116,8 @@ private S readEntityFrom(Row row, PersistentProperty property) { S instance = createInstance(row, prefix, entity); - PersistentPropertyAccessor accessor = entity.getPropertyAccessor(instance); - ConvertingPropertyAccessor propertyAccessor = new ConvertingPropertyAccessor(accessor, + PersistentPropertyAccessor accessor = entity.getPropertyAccessor(instance); + ConvertingPropertyAccessor propertyAccessor = new ConvertingPropertyAccessor<>(accessor, converter.getConversionService()); for (RelationalPersistentProperty p : entity) { @@ -129,8 +131,7 @@ private S readEntityFrom(Row row, PersistentProperty property) { private S createInstance(Row row, String prefix, RelationalPersistentEntity entity) { - RowParameterValueProvider rowParameterValueProvider = new RowParameterValueProvider(row, entity, - converter.getConversionService(), prefix); + RowParameterValueProvider rowParameterValueProvider = new RowParameterValueProvider(row, entity, converter, prefix); return converter.createInstance(entity, rowParameterValueProvider::getParameterValue); } @@ -140,7 +141,7 @@ private static class RowParameterValueProvider implements ParameterValueProvider private final @NonNull Row resultSet; private final @NonNull RelationalPersistentEntity entity; - private final @NonNull ConversionService conversionService; + private final @NonNull RelationalConverter converter; private final @NonNull String prefix; /* @@ -151,10 +152,13 @@ private static class RowParameterValueProvider implements ParameterValueProvider @Nullable public T getParameterValue(Parameter parameter) { - String column = prefix + entity.getRequiredPersistentProperty(parameter.getName()).getColumnName(); + RelationalPersistentProperty property = entity.getRequiredPersistentProperty(parameter.getName()); + String column = prefix + property.getColumnName(); try { - return conversionService.convert(resultSet.get(column), parameter.getType().getType()); + + Object value = converter.readValue(resultSet.get(column), property.getTypeInformation()); + return converter.getConversionService().convert(value, parameter.getType().getType()); } catch (Exception o_O) { throw new MappingException(String.format("Couldn't read column %s from Row.", column), o_O); } diff --git a/src/test/java/org/springframework/data/r2dbc/function/convert/EntityRowMapperUnitTests.java b/src/test/java/org/springframework/data/r2dbc/function/convert/EntityRowMapperUnitTests.java new file mode 100644 index 00000000..a8ba3da3 --- /dev/null +++ b/src/test/java/org/springframework/data/r2dbc/function/convert/EntityRowMapperUnitTests.java @@ -0,0 +1,79 @@ +package org.springframework.data.r2dbc.function.convert; + +import static org.assertj.core.api.Assertions.*; +import static org.mockito.Mockito.*; + +import io.r2dbc.spi.Row; +import io.r2dbc.spi.RowMetadata; +import lombok.RequiredArgsConstructor; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.data.r2dbc.dialect.PostgresDialect; +import org.springframework.data.r2dbc.function.DefaultReactiveDataAccessStrategy; +import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; + +/** + * Unit tests for {@link EntityRowMapper}. + * + * @author Mark Paluch + */ +@RunWith(MockitoJUnitRunner.class) +public class EntityRowMapperUnitTests { + + DefaultReactiveDataAccessStrategy strategy = new DefaultReactiveDataAccessStrategy(PostgresDialect.INSTANCE); + + Row rowMock = mock(Row.class); + RowMetadata metadata = mock(RowMetadata.class); + + @Test // gh-22 + public void shouldMapSimpleEntity() { + + EntityRowMapper mapper = getRowMapper(SimpleEntity.class); + when(rowMock.get("id")).thenReturn("foo"); + + SimpleEntity result = mapper.apply(rowMock, metadata); + assertThat(result.id).isEqualTo("foo"); + } + + @Test // gh-22 + public void shouldMapSimpleEntityWithConstructorCreation() { + + EntityRowMapper mapper = getRowMapper(SimpleEntityConstructorCreation.class); + when(rowMock.get("id")).thenReturn("foo"); + + SimpleEntityConstructorCreation result = mapper.apply(rowMock, metadata); + assertThat(result.id).isEqualTo("foo"); + } + + @Test // gh-22 + public void shouldApplyConversionWithConstructorCreation() { + + EntityRowMapper mapper = getRowMapper(ConversionWithConstructorCreation.class); + when(rowMock.get("id")).thenReturn((byte) 0x24); + + ConversionWithConstructorCreation result = mapper.apply(rowMock, metadata); + assertThat(result.id).isEqualTo(36L); + } + + private EntityRowMapper getRowMapper(Class type) { + RelationalPersistentEntity entity = (RelationalPersistentEntity) strategy.getMappingContext() + .getRequiredPersistentEntity(type); + return new EntityRowMapper<>(entity, strategy.getRelationalConverter()); + } + + static class SimpleEntity { + String id; + } + + @RequiredArgsConstructor + static class SimpleEntityConstructorCreation { + final String id; + } + + @RequiredArgsConstructor + static class ConversionWithConstructorCreation { + final long id; + } +} From ccadfad3ef45c9b7946480b992b9c155fd835307 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 3 Dec 2018 17:39:07 +0100 Subject: [PATCH 3/6] #30 - Add custom conversion support. We now support custom conversions via R2dbcCustomConversions. Custom conversions introduces simple types that depend on the used dialect. Custom conversions and simple types are held in RelationalConverter and MappingContext. Simple types and conversions are used by DatabaseClient and repository support to properly apply registered converters and support native types such as array-columns. Related tickets: #22, #26. --- .../config/AbstractR2dbcConfiguration.java | 43 +++++++-- .../data/r2dbc/dialect/Dialect.java | 37 ++++++++ .../data/r2dbc/dialect/PostgresDialect.java | 31 ++++++ .../data/r2dbc/dialect/SqlServerDialect.java | 17 ++++ .../DefaultReactiveDataAccessStrategy.java | 95 ++++++++++++++++++- .../function/ReactiveDataAccessStrategy.java | 9 ++ .../convert/MappingR2dbcConverter.java | 27 ------ .../convert/R2dbcCustomConversions.java | 26 +++++ .../config/R2dbcRepositoriesRegistrar.java | 1 - .../support/SimpleR2dbcRepository.java | 2 +- .../dialect/PostgresDialectUnitTests.java | 21 ++++ .../dialect/SqlServerDialectUnitTests.java | 11 +++ ...ltReactiveDataAccessStrategyUnitTests.java | 37 ++++++++ .../convert/EntityRowMapperUnitTests.java | 17 ++++ 14 files changed, 334 insertions(+), 40 deletions(-) create mode 100644 src/main/java/org/springframework/data/r2dbc/function/convert/R2dbcCustomConversions.java diff --git a/src/main/java/org/springframework/data/r2dbc/config/AbstractR2dbcConfiguration.java b/src/main/java/org/springframework/data/r2dbc/config/AbstractR2dbcConfiguration.java index e6b11e88..4ebfcb5a 100644 --- a/src/main/java/org/springframework/data/r2dbc/config/AbstractR2dbcConfiguration.java +++ b/src/main/java/org/springframework/data/r2dbc/config/AbstractR2dbcConfiguration.java @@ -17,15 +17,20 @@ import io.r2dbc.spi.ConnectionFactory; +import java.util.Collections; import java.util.Optional; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.convert.converter.Converter; +import org.springframework.data.convert.CustomConversions; +import org.springframework.data.convert.CustomConversions.StoreConversions; import org.springframework.data.r2dbc.dialect.Database; import org.springframework.data.r2dbc.dialect.Dialect; import org.springframework.data.r2dbc.function.DatabaseClient; import org.springframework.data.r2dbc.function.DefaultReactiveDataAccessStrategy; import org.springframework.data.r2dbc.function.ReactiveDataAccessStrategy; +import org.springframework.data.r2dbc.function.convert.R2dbcCustomConversions; import org.springframework.data.r2dbc.support.R2dbcExceptionTranslator; import org.springframework.data.r2dbc.support.SqlErrorCodeR2dbcExceptionTranslator; import org.springframework.data.relational.core.conversion.BasicRelationalConverter; @@ -95,15 +100,21 @@ public DatabaseClient databaseClient(ReactiveDataAccessStrategy dataAccessStrate * Register a {@link RelationalMappingContext} and apply an optional {@link NamingStrategy}. * * @param namingStrategy optional {@link NamingStrategy}. Use {@link NamingStrategy#INSTANCE} as fallback. + * @param r2dbcCustomConversions customized R2DBC conversions. * @return must not be {@literal null}. * @throws IllegalArgumentException if any of the required args is {@literal null}. */ @Bean - public RelationalMappingContext r2dbcMappingContext(Optional namingStrategy) { + public RelationalMappingContext r2dbcMappingContext(Optional namingStrategy, + R2dbcCustomConversions r2dbcCustomConversions) { Assert.notNull(namingStrategy, "NamingStrategy must not be null!"); - return new RelationalMappingContext(namingStrategy.orElse(NamingStrategy.INSTANCE)); + RelationalMappingContext relationalMappingContext = new RelationalMappingContext( + namingStrategy.orElse(NamingStrategy.INSTANCE)); + relationalMappingContext.setSimpleTypeHolder(r2dbcCustomConversions.getSimpleTypeHolder()); + + return relationalMappingContext; } /** @@ -111,17 +122,37 @@ public RelationalMappingContext r2dbcMappingContext(Optional nam * RelationalMappingContext}. * * @param mappingContext the configured {@link RelationalMappingContext}. + * @param r2dbcCustomConversions customized R2DBC conversions. * @return must not be {@literal null}. - * @see #r2dbcMappingContext(Optional) + * @see #r2dbcMappingContext(Optional, R2dbcCustomConversions) * @see #getDialect(ConnectionFactory) * @throws IllegalArgumentException if any of the {@literal mappingContext} is {@literal null}. */ @Bean - public ReactiveDataAccessStrategy reactiveDataAccessStrategy(RelationalMappingContext mappingContext) { + public ReactiveDataAccessStrategy reactiveDataAccessStrategy(RelationalMappingContext mappingContext, + R2dbcCustomConversions r2dbcCustomConversions) { Assert.notNull(mappingContext, "MappingContext must not be null!"); - return new DefaultReactiveDataAccessStrategy(getDialect(connectionFactory()), - new BasicRelationalConverter(mappingContext)); + + BasicRelationalConverter converter = new BasicRelationalConverter(mappingContext, r2dbcCustomConversions); + + return new DefaultReactiveDataAccessStrategy(getDialect(connectionFactory()), converter); + } + + /** + * Register custom {@link Converter}s in a {@link CustomConversions} object if required. These + * {@link CustomConversions} will be registered with the {@link BasicRelationalConverter} and + * {@link #r2dbcMappingContext(Optional, R2dbcCustomConversions)}. Returns an empty {@link R2dbcCustomConversions} + * instance by default. + * + * @return must not be {@literal null}. + */ + @Bean + public R2dbcCustomConversions r2dbcCustomConversions() { + + Dialect dialect = getDialect(connectionFactory()); + StoreConversions storeConversions = StoreConversions.of(dialect.getSimpleTypeHolder()); + return new R2dbcCustomConversions(storeConversions, Collections.emptyList()); } /** diff --git a/src/main/java/org/springframework/data/r2dbc/dialect/Dialect.java b/src/main/java/org/springframework/data/r2dbc/dialect/Dialect.java index 3371d623..411f3839 100644 --- a/src/main/java/org/springframework/data/r2dbc/dialect/Dialect.java +++ b/src/main/java/org/springframework/data/r2dbc/dialect/Dialect.java @@ -1,5 +1,11 @@ package org.springframework.data.r2dbc.dialect; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; + +import org.springframework.data.mapping.model.SimpleTypeHolder; + /** * Represents a dialect that is implemented by a particular database. * @@ -26,10 +32,41 @@ public interface Dialect { @Deprecated String generatedKeysClause(); + /** + * Return a collection of types that are natively supported by this database/driver. Defaults to + * {@link Collections#emptySet()}. + * + * @return a collection of types that are natively supported by this database/driver. Defaults to + * {@link Collections#emptySet()}. + */ + default Collection> getSimpleTypes() { + return Collections.emptySet(); + } + + /** + * Return the {@link SimpleTypeHolder} for this dialect. + * + * @return the {@link SimpleTypeHolder} for this dialect. + * @see #getSimpleTypes() + */ + default SimpleTypeHolder getSimpleTypeHolder() { + return new SimpleTypeHolder(new HashSet<>(getSimpleTypes()), true); + } + /** * Return the {@link LimitClause} used by this dialect. * * @return the {@link LimitClause} used by this dialect. */ LimitClause limit(); + + /** + * Returns {@literal true} whether this dialect supports array-typed column. Collection-typed columns can map their + * content to native array types. + * + * @return {@literal true} whether this dialect supports array-typed columns. + */ + default boolean supportsArrayColumns() { + return false; + } } diff --git a/src/main/java/org/springframework/data/r2dbc/dialect/PostgresDialect.java b/src/main/java/org/springframework/data/r2dbc/dialect/PostgresDialect.java index fd5da050..8d0c0b3b 100644 --- a/src/main/java/org/springframework/data/r2dbc/dialect/PostgresDialect.java +++ b/src/main/java/org/springframework/data/r2dbc/dialect/PostgresDialect.java @@ -1,5 +1,15 @@ package org.springframework.data.r2dbc.dialect; +import java.net.InetAddress; +import java.net.URI; +import java.net.URL; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.UUID; + /** * An SQL dialect for Postgres. * @@ -7,6 +17,9 @@ */ public class PostgresDialect implements Dialect { + private static final Set> SIMPLE_TYPES = new HashSet<>( + Arrays.asList(List.class, Collection.class, String[].class, UUID.class, URL.class, URI.class, InetAddress.class)); + /** * Singleton instance. */ @@ -62,6 +75,15 @@ public String generatedKeysClause() { return "RETURNING *"; } + /* + * (non-Javadoc) + * @see org.springframework.data.r2dbc.dialect.Dialect#getSimpleTypesKeys() + */ + @Override + public Collection> getSimpleTypes() { + return SIMPLE_TYPES; + } + /* * (non-Javadoc) * @see org.springframework.data.r2dbc.dialect.Dialect#limit() @@ -70,4 +92,13 @@ public String generatedKeysClause() { public LimitClause limit() { return LIMIT_CLAUSE; } + + /* + * (non-Javadoc) + * @see org.springframework.data.r2dbc.dialect.Dialect#supportsArrayColumns() + */ + @Override + public boolean supportsArrayColumns() { + return true; + } } diff --git a/src/main/java/org/springframework/data/r2dbc/dialect/SqlServerDialect.java b/src/main/java/org/springframework/data/r2dbc/dialect/SqlServerDialect.java index ccbc993a..bf7199e0 100644 --- a/src/main/java/org/springframework/data/r2dbc/dialect/SqlServerDialect.java +++ b/src/main/java/org/springframework/data/r2dbc/dialect/SqlServerDialect.java @@ -1,5 +1,11 @@ package org.springframework.data.r2dbc.dialect; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; +import java.util.UUID; + /** * An SQL dialect for Microsoft SQL Server. * @@ -7,6 +13,8 @@ */ public class SqlServerDialect implements Dialect { + private static final Set> SIMPLE_TYPES = new HashSet<>(Collections.singletonList(UUID.class)); + /** * Singleton instance. */ @@ -63,6 +71,15 @@ public String generatedKeysClause() { return "select SCOPE_IDENTITY() AS GENERATED_KEYS"; } + /* + * (non-Javadoc) + * @see org.springframework.data.r2dbc.dialect.Dialect#getSimpleTypesKeys() + */ + @Override + public Collection> getSimpleTypes() { + return SIMPLE_TYPES; + } + /* * (non-Javadoc) * @see org.springframework.data.r2dbc.dialect.Dialect#limit() diff --git a/src/main/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategy.java b/src/main/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategy.java index dcf77534..5d865e77 100644 --- a/src/main/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategy.java @@ -19,6 +19,7 @@ import io.r2dbc.spi.RowMetadata; import io.r2dbc.spi.Statement; +import java.lang.reflect.Array; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -29,22 +30,28 @@ import java.util.function.BiFunction; import java.util.function.Function; +import org.springframework.dao.InvalidDataAccessApiUsageException; +import org.springframework.dao.InvalidDataAccessResourceUsageException; +import org.springframework.data.convert.CustomConversions.StoreConversions; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Order; import org.springframework.data.mapping.PersistentPropertyAccessor; +import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.r2dbc.dialect.BindMarker; import org.springframework.data.r2dbc.dialect.BindMarkers; import org.springframework.data.r2dbc.dialect.Dialect; import org.springframework.data.r2dbc.dialect.LimitClause; import org.springframework.data.r2dbc.dialect.LimitClause.Position; import org.springframework.data.r2dbc.function.convert.EntityRowMapper; +import org.springframework.data.r2dbc.function.convert.R2dbcCustomConversions; import org.springframework.data.r2dbc.function.convert.SettableValue; import org.springframework.data.relational.core.conversion.BasicRelationalConverter; import org.springframework.data.relational.core.conversion.RelationalConverter; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -57,8 +64,9 @@ */ public class DefaultReactiveDataAccessStrategy implements ReactiveDataAccessStrategy { - private final RelationalConverter relationalConverter; private final Dialect dialect; + private final RelationalConverter relationalConverter; + private final MappingContext, ? extends RelationalPersistentProperty> mappingContext; /** * Creates a new {@link DefaultReactiveDataAccessStrategy} given {@link Dialect}. @@ -66,7 +74,28 @@ public class DefaultReactiveDataAccessStrategy implements ReactiveDataAccessStra * @param dialect the {@link Dialect} to use. */ public DefaultReactiveDataAccessStrategy(Dialect dialect) { - this(dialect, new BasicRelationalConverter(new RelationalMappingContext())); + this(dialect, createConverter(dialect)); + } + + private static BasicRelationalConverter createConverter(Dialect dialect) { + + Assert.notNull(dialect, "Dialect must not be null"); + + R2dbcCustomConversions customConversions = new R2dbcCustomConversions( + StoreConversions.of(dialect.getSimpleTypeHolder()), Collections.emptyList()); + + RelationalMappingContext context = new RelationalMappingContext(); + context.setSimpleTypeHolder(customConversions.getSimpleTypeHolder()); + + return new BasicRelationalConverter(context, customConversions); + } + + public RelationalConverter getRelationalConverter() { + return relationalConverter; + } + + public MappingContext, ? extends RelationalPersistentProperty> getMappingContext() { + return mappingContext; } /** @@ -75,12 +104,15 @@ public DefaultReactiveDataAccessStrategy(Dialect dialect) { * @param dialect the {@link Dialect} to use. * @param converter must not be {@literal null}. */ + @SuppressWarnings("unchecked") public DefaultReactiveDataAccessStrategy(Dialect dialect, RelationalConverter converter) { Assert.notNull(dialect, "Dialect must not be null"); Assert.notNull(converter, "RelationalConverter must not be null"); this.relationalConverter = converter; + this.mappingContext = (MappingContext, ? extends RelationalPersistentProperty>) relationalConverter + .getMappingContext(); this.dialect = dialect; } @@ -121,7 +153,7 @@ public List getValuesToInsert(Object object) { for (RelationalPersistentProperty property : entity) { - Object value = propertyAccessor.getProperty(property); + Object value = getWriteValue(propertyAccessor, property); if (value == null) { continue; @@ -133,6 +165,31 @@ public List getValuesToInsert(Object object) { return values; } + /* + * (non-Javadoc) + * @see org.springframework.data.r2dbc.function.ReactiveDataAccessStrategy#getColumnsToUpdate(java.lang.Object) + */ + public Map getColumnsToUpdate(Object object) { + + Assert.notNull(object, "Entity object must not be null!"); + + Class userClass = ClassUtils.getUserClass(object); + RelationalPersistentEntity entity = getRequiredPersistentEntity(userClass); + + Map update = new LinkedHashMap<>(); + + PersistentPropertyAccessor propertyAccessor = entity.getPropertyAccessor(object); + + for (RelationalPersistentProperty property : entity) { + + Object writeValue = getWriteValue(propertyAccessor, property); + + update.put(property.getColumnName(), new SettableValue(property.getColumnName(), writeValue, property.getType())); + } + + return update; + } + /* * (non-Javadoc) * @see org.springframework.data.r2dbc.function.ReactiveDataAccessStrategy#getMappedSort(java.lang.Class, org.springframework.data.domain.Sort) @@ -181,12 +238,40 @@ public String getTableName(Class type) { } private RelationalPersistentEntity getRequiredPersistentEntity(Class typeToRead) { - return relationalConverter.getMappingContext().getRequiredPersistentEntity(typeToRead); + return mappingContext.getRequiredPersistentEntity(typeToRead); } @Nullable private RelationalPersistentEntity getPersistentEntity(Class typeToRead) { - return relationalConverter.getMappingContext().getPersistentEntity(typeToRead); + return mappingContext.getPersistentEntity(typeToRead); + } + + private Object getWriteValue(PersistentPropertyAccessor propertyAccessor, RelationalPersistentProperty property) { + + TypeInformation type = property.getTypeInformation(); + Object value = relationalConverter.writeValue(propertyAccessor.getProperty(property), type); + + if (type.isCollectionLike()) { + + RelationalPersistentEntity nestedEntity = mappingContext + .getPersistentEntity(type.getRequiredActualType().getType()); + + if (nestedEntity != null) { + throw new InvalidDataAccessApiUsageException("Nested entities are not supported"); + } + + if (!dialect.supportsArrayColumns()) { + throw new InvalidDataAccessResourceUsageException( + "Dialect " + dialect.getClass().getName() + " does not support array columns"); + } + + if (!property.isArray()) { + Object zeroLengthArray = Array.newInstance(property.getActualType(), 0); + return relationalConverter.getConversionService().convert(value, zeroLengthArray.getClass()); + } + } + + return value; } /* diff --git a/src/main/java/org/springframework/data/r2dbc/function/ReactiveDataAccessStrategy.java b/src/main/java/org/springframework/data/r2dbc/function/ReactiveDataAccessStrategy.java index f3c8a9f2..8b038e98 100644 --- a/src/main/java/org/springframework/data/r2dbc/function/ReactiveDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/r2dbc/function/ReactiveDataAccessStrategy.java @@ -20,6 +20,7 @@ import io.r2dbc.spi.Statement; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.function.BiFunction; @@ -49,6 +50,14 @@ public interface ReactiveDataAccessStrategy { */ List getValuesToInsert(Object object); + /** + * Returns a {@link Map} that maps column names to a {@link SettableValue} value. + * + * @param object must not be {@literal null}. + * @return + */ + Map getColumnsToUpdate(Object object); + /** * Map the {@link Sort} object to apply field name mapping using {@link Class the type to read}. * diff --git a/src/main/java/org/springframework/data/r2dbc/function/convert/MappingR2dbcConverter.java b/src/main/java/org/springframework/data/r2dbc/function/convert/MappingR2dbcConverter.java index 6b291e86..54b26ec1 100644 --- a/src/main/java/org/springframework/data/r2dbc/function/convert/MappingR2dbcConverter.java +++ b/src/main/java/org/springframework/data/r2dbc/function/convert/MappingR2dbcConverter.java @@ -21,7 +21,6 @@ import java.util.LinkedHashMap; import java.util.Map; -import java.util.Optional; import java.util.function.BiFunction; import org.springframework.core.convert.ConversionService; @@ -65,32 +64,6 @@ public MappingR2dbcConverter(RelationalConverter converter) { this.relationalConverter = converter; } - /** - * Returns a {@link Map} that maps column names to an {@link Optional} value. Used {@link Optional#empty()} if the - * underlying property is {@literal null}. - * - * @param object must not be {@literal null}. - * @return - */ - public Map getColumnsToUpdate(Object object) { - - Assert.notNull(object, "Entity object must not be null!"); - - Class userClass = ClassUtils.getUserClass(object); - RelationalPersistentEntity entity = getMappingContext().getRequiredPersistentEntity(userClass); - - Map update = new LinkedHashMap<>(); - - PersistentPropertyAccessor propertyAccessor = entity.getPropertyAccessor(object); - - for (RelationalPersistentProperty property : entity) { - update.put(property.getColumnName(), - new SettableValue(property.getColumnName(), propertyAccessor.getProperty(property), property.getType())); - } - - return update; - } - /** * Returns a {@link java.util.function.Function} that populates the id property of the {@code object} from a * {@link Row}. diff --git a/src/main/java/org/springframework/data/r2dbc/function/convert/R2dbcCustomConversions.java b/src/main/java/org/springframework/data/r2dbc/function/convert/R2dbcCustomConversions.java new file mode 100644 index 00000000..8c3a692e --- /dev/null +++ b/src/main/java/org/springframework/data/r2dbc/function/convert/R2dbcCustomConversions.java @@ -0,0 +1,26 @@ +package org.springframework.data.r2dbc.function.convert; + +import java.util.Collection; + +import org.springframework.data.convert.CustomConversions; + +/** + * Value object to capture custom conversion. {@link R2dbcCustomConversions} also act as factory for + * {@link org.springframework.data.mapping.model.SimpleTypeHolder} + * + * @author Mark Paluch + * @see CustomConversions + * @see org.springframework.data.mapping.model.SimpleTypeHolder + */ +public class R2dbcCustomConversions extends CustomConversions { + + /** + * Creates a new {@link CustomConversions} instance registering the given converters. + * + * @param storeConversions must not be {@literal null}. + * @param converters must not be {@literal null}. + */ + public R2dbcCustomConversions(StoreConversions storeConversions, Collection converters) { + super(storeConversions, converters); + } +} diff --git a/src/main/java/org/springframework/data/r2dbc/repository/config/R2dbcRepositoriesRegistrar.java b/src/main/java/org/springframework/data/r2dbc/repository/config/R2dbcRepositoriesRegistrar.java index 5279e09f..91949b31 100644 --- a/src/main/java/org/springframework/data/r2dbc/repository/config/R2dbcRepositoriesRegistrar.java +++ b/src/main/java/org/springframework/data/r2dbc/repository/config/R2dbcRepositoriesRegistrar.java @@ -24,7 +24,6 @@ * R2DBC-specific {@link org.springframework.context.annotation.ImportBeanDefinitionRegistrar}. * * @author Mark Paluch - * @since 2.0 */ class R2dbcRepositoriesRegistrar extends RepositoryBeanDefinitionRegistrarSupport { diff --git a/src/main/java/org/springframework/data/r2dbc/repository/support/SimpleR2dbcRepository.java b/src/main/java/org/springframework/data/r2dbc/repository/support/SimpleR2dbcRepository.java index 23830347..ce2e4a55 100644 --- a/src/main/java/org/springframework/data/r2dbc/repository/support/SimpleR2dbcRepository.java +++ b/src/main/java/org/springframework/data/r2dbc/repository/support/SimpleR2dbcRepository.java @@ -71,7 +71,7 @@ public Mono save(S objectToSave) { } Object id = entity.getRequiredId(objectToSave); - Map columns = converter.getColumnsToUpdate(objectToSave); + Map columns = accessStrategy.getColumnsToUpdate(objectToSave); columns.remove(getIdColumnName()); // do not update the Id column. String idColumnName = getIdColumnName(); BindIdOperation update = accessStrategy.updateById(entity.getTableName(), columns.keySet(), idColumnName); diff --git a/src/test/java/org/springframework/data/r2dbc/dialect/PostgresDialectUnitTests.java b/src/test/java/org/springframework/data/r2dbc/dialect/PostgresDialectUnitTests.java index 3b0168d5..29f5d2ca 100644 --- a/src/test/java/org/springframework/data/r2dbc/dialect/PostgresDialectUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/dialect/PostgresDialectUnitTests.java @@ -2,7 +2,11 @@ import static org.assertj.core.api.Assertions.*; +import java.util.Collection; +import java.util.List; + import org.junit.Test; +import org.springframework.data.mapping.model.SimpleTypeHolder; /** * Unit tests for {@link PostgresDialect}. @@ -22,4 +26,21 @@ public void shouldUsePostgresPlaceholders() { assertThat(first.getPlaceholder()).isEqualTo("$1"); assertThat(second.getPlaceholder()).isEqualTo("$2"); } + + @Test // gh-30 + public void shouldConsiderCollectionTypesAsSimple() { + + SimpleTypeHolder holder = PostgresDialect.INSTANCE.getSimpleTypeHolder(); + + assertThat(holder.isSimpleType(List.class)).isTrue(); + assertThat(holder.isSimpleType(Collection.class)).isTrue(); + } + + @Test // gh-30 + public void shouldConsiderStringArrayTypeAsSimple() { + + SimpleTypeHolder holder = PostgresDialect.INSTANCE.getSimpleTypeHolder(); + + assertThat(holder.isSimpleType(String[].class)).isTrue(); + } } diff --git a/src/test/java/org/springframework/data/r2dbc/dialect/SqlServerDialectUnitTests.java b/src/test/java/org/springframework/data/r2dbc/dialect/SqlServerDialectUnitTests.java index 0e848015..1e96e389 100644 --- a/src/test/java/org/springframework/data/r2dbc/dialect/SqlServerDialectUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/dialect/SqlServerDialectUnitTests.java @@ -2,7 +2,10 @@ import static org.assertj.core.api.Assertions.*; +import java.util.UUID; + import org.junit.Test; +import org.springframework.data.mapping.model.SimpleTypeHolder; /** * Unit tests for {@link SqlServerDialect}. @@ -22,4 +25,12 @@ public void shouldUseNamedPlaceholders() { assertThat(first.getPlaceholder()).isEqualTo("@P0"); assertThat(second.getPlaceholder()).isEqualTo("@P1_foobar"); } + + @Test // gh-30 + public void shouldConsiderUuidAsSimple() { + + SimpleTypeHolder holder = SqlServerDialect.INSTANCE.getSimpleTypeHolder(); + + assertThat(holder.isSimpleType(UUID.class)).isTrue(); + } } diff --git a/src/test/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategyUnitTests.java b/src/test/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategyUnitTests.java index df279946..a8e7a5d3 100644 --- a/src/test/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategyUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategyUnitTests.java @@ -8,9 +8,12 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.List; +import java.util.Map; import org.junit.Test; import org.springframework.data.r2dbc.dialect.PostgresDialect; +import org.springframework.data.r2dbc.function.convert.SettableValue; /** * Unit tests for {@link DefaultReactiveDataAccessStrategy}. @@ -101,4 +104,38 @@ public void shouldRenderDeleteByIdInQuery() { operation.bindId(statement, "bar"); assertThat(operation.toQuery()).isEqualTo("DELETE FROM table WHERE id IN ($1, $2)"); } + + @Test // gh-22 + public void shouldUpdateArray() { + + Map columnsToUpdate = strategy + .getColumnsToUpdate(new WithCollectionTypes(new String[] { "one", "two" }, null)); + + Object stringArray = columnsToUpdate.get("string_array").getValue(); + assertThat(stringArray).isInstanceOf(String[].class); + assertThat((String[]) stringArray).hasSize(2).contains("one", "two"); + } + + @Test // gh-22 + public void shouldConvertListToArray() { + + Map columnsToUpdate = strategy + .getColumnsToUpdate(new WithCollectionTypes(null, Arrays.asList("one", "two"))); + + Object stringArray = columnsToUpdate.get("string_collection").getValue(); + assertThat(stringArray).isInstanceOf(String[].class); + assertThat((String[]) stringArray).hasSize(2).contains("one", "two"); + } + + static class WithCollectionTypes { + + String[] stringArray; + + List stringCollection; + + WithCollectionTypes(String[] stringArray, List stringCollection) { + this.stringArray = stringArray; + this.stringCollection = stringCollection; + } + } } diff --git a/src/test/java/org/springframework/data/r2dbc/function/convert/EntityRowMapperUnitTests.java b/src/test/java/org/springframework/data/r2dbc/function/convert/EntityRowMapperUnitTests.java index a8ba3da3..b17c080d 100644 --- a/src/test/java/org/springframework/data/r2dbc/function/convert/EntityRowMapperUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/function/convert/EntityRowMapperUnitTests.java @@ -7,6 +7,8 @@ import io.r2dbc.spi.RowMetadata; import lombok.RequiredArgsConstructor; +import java.util.List; + import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.junit.MockitoJUnitRunner; @@ -57,6 +59,17 @@ public void shouldApplyConversionWithConstructorCreation() { assertThat(result.id).isEqualTo(36L); } + @Test // gh-30 + public void shouldConvertArrayToCollection() { + + EntityRowMapper mapper = getRowMapper(EntityWithCollection.class); + when(rowMock.get("ids")).thenReturn((new String[] { "foo", "bar" })); + + EntityWithCollection result = mapper.apply(rowMock, metadata); + assertThat(result.ids).contains("foo", "bar"); + } + + @SuppressWarnings("unchecked") private EntityRowMapper getRowMapper(Class type) { RelationalPersistentEntity entity = (RelationalPersistentEntity) strategy.getMappingContext() .getRequiredPersistentEntity(type); @@ -76,4 +89,8 @@ static class SimpleEntityConstructorCreation { static class ConversionWithConstructorCreation { final long id; } + + static class EntityWithCollection { + List ids; + } } From 384f3b466f8cf82e7d12ca6c3a097dd1fbaf9a80 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 3 Dec 2018 17:42:24 +0100 Subject: [PATCH 4/6] #30 - Prepare issue branch. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 9ff59187..5e5eb075 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ org.springframework.data spring-data-r2dbc - 1.0.0.BUILD-SNAPSHOT + 1.0.0.gh-30-SNAPSHOT Spring Data R2DBC Spring Data module for R2DBC. From 299c0c5428d9a4f5f699734dc05d1f6063640546 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 4 Dec 2018 14:25:03 +0100 Subject: [PATCH 5/6] #30 - Polishing. Minor formatting. Add suggestions. Update src/test/java/org/springframework/data/r2dbc/dialect/PostgresDialectUnitTests.java Co-Authored-By: mp911de --- .../config/AbstractR2dbcConfiguration.java | 2 +- .../DefaultReactiveDataAccessStrategy.java | 2 ++ .../dialect/PostgresDialectUnitTests.java | 16 ++++++++++ ...ltReactiveDataAccessStrategyUnitTests.java | 3 ++ .../convert/EntityRowMapperUnitTests.java | 32 +++++++++++++++++++ 5 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/springframework/data/r2dbc/config/AbstractR2dbcConfiguration.java b/src/main/java/org/springframework/data/r2dbc/config/AbstractR2dbcConfiguration.java index 4ebfcb5a..57c1e3c9 100644 --- a/src/main/java/org/springframework/data/r2dbc/config/AbstractR2dbcConfiguration.java +++ b/src/main/java/org/springframework/data/r2dbc/config/AbstractR2dbcConfiguration.java @@ -118,7 +118,7 @@ public RelationalMappingContext r2dbcMappingContext(Optional nam } /** - * Creates a {@link ReactiveDataAccessStrategy} using the configured {@link #r2dbcMappingContext(Optional) + * Creates a {@link ReactiveDataAccessStrategy} using the configured {@link #r2dbcMappingContext(Optional, R2dbcCustomConversions)} * RelationalMappingContext}. * * @param mappingContext the configured {@link RelationalMappingContext}. diff --git a/src/main/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategy.java b/src/main/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategy.java index 5d865e77..abe26fc3 100644 --- a/src/main/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategy.java @@ -261,11 +261,13 @@ private Object getWriteValue(PersistentPropertyAccessor propertyAccessor, Relati } if (!dialect.supportsArrayColumns()) { + throw new InvalidDataAccessResourceUsageException( "Dialect " + dialect.getClass().getName() + " does not support array columns"); } if (!property.isArray()) { + Object zeroLengthArray = Array.newInstance(property.getActualType(), 0); return relationalConverter.getConversionService().convert(value, zeroLengthArray.getClass()); } diff --git a/src/test/java/org/springframework/data/r2dbc/dialect/PostgresDialectUnitTests.java b/src/test/java/org/springframework/data/r2dbc/dialect/PostgresDialectUnitTests.java index 29f5d2ca..7df2e22f 100644 --- a/src/test/java/org/springframework/data/r2dbc/dialect/PostgresDialectUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/dialect/PostgresDialectUnitTests.java @@ -42,5 +42,21 @@ public void shouldConsiderStringArrayTypeAsSimple() { SimpleTypeHolder holder = PostgresDialect.INSTANCE.getSimpleTypeHolder(); assertThat(holder.isSimpleType(String[].class)).isTrue(); + + @Test // gh-30 + public void shouldConsiderIntArrayTypeAsSimple() { + + SimpleTypeHolder holder = PostgresDialect.INSTANCE.getSimpleTypeHolder(); + + assertThat(holder.isSimpleType(int[].class)).isTrue(); + } + + @Test // gh-30 + public void shouldConsiderIntegerArrayTypeAsSimple() { + + SimpleTypeHolder holder = PostgresDialect.INSTANCE.getSimpleTypeHolder(); + + assertThat(holder.isSimpleType(Integer[].class)).isTrue(); + } } } diff --git a/src/test/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategyUnitTests.java b/src/test/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategyUnitTests.java index a8e7a5d3..120d6d4e 100644 --- a/src/test/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategyUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategyUnitTests.java @@ -112,6 +112,7 @@ public void shouldUpdateArray() { .getColumnsToUpdate(new WithCollectionTypes(new String[] { "one", "two" }, null)); Object stringArray = columnsToUpdate.get("string_array").getValue(); + assertThat(stringArray).isInstanceOf(String[].class); assertThat((String[]) stringArray).hasSize(2).contains("one", "two"); } @@ -123,6 +124,7 @@ public void shouldConvertListToArray() { .getColumnsToUpdate(new WithCollectionTypes(null, Arrays.asList("one", "two"))); Object stringArray = columnsToUpdate.get("string_collection").getValue(); + assertThat(stringArray).isInstanceOf(String[].class); assertThat((String[]) stringArray).hasSize(2).contains("one", "two"); } @@ -134,6 +136,7 @@ static class WithCollectionTypes { List stringCollection; WithCollectionTypes(String[] stringArray, List stringCollection) { + this.stringArray = stringArray; this.stringCollection = stringCollection; } diff --git a/src/test/java/org/springframework/data/r2dbc/function/convert/EntityRowMapperUnitTests.java b/src/test/java/org/springframework/data/r2dbc/function/convert/EntityRowMapperUnitTests.java index b17c080d..d7023ace 100644 --- a/src/test/java/org/springframework/data/r2dbc/function/convert/EntityRowMapperUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/function/convert/EntityRowMapperUnitTests.java @@ -69,6 +69,35 @@ public void shouldConvertArrayToCollection() { assertThat(result.ids).contains("foo", "bar"); } + @Test // gh-30 + public void shouldConvertArrayToSet() { + + EntityRowMapper mapper = getRowMapper(EntityWithCollection.class); + when(rowMock.get("integerSet")).thenReturn((new int[] { 3, 14 })); + + EntityWithCollection result = mapper.apply(rowMock, metadata); + assertThat(result.integerSet).contains(3, 14); + } + + @Test // gh-30 + public void shouldConvertArrayMembers() { + + EntityRowMapper mapper = getRowMapper(EntityWithCollection.class); + when(rowMock.get("primitiveIntegers")).thenReturn((new long[] { 3L, 14L })); + + EntityWithCollection result = mapper.apply(rowMock, metadata); + assertThat(result.primitiveIntegers).contains(3, 14); + } + + @Test // gh-30 + public void shouldConvertArrayToBoxedArray() { + + EntityRowMapper mapper = getRowMapper(EntityWithCollection.class); + when(rowMock.get("boxedIntegers")).thenReturn((new int[] { 3, 11 })); + + EntityWithCollection result = mapper.apply(rowMock, metadata); + assertThat(result.boxedIntegers).contains(3, 11); + } @SuppressWarnings("unchecked") private EntityRowMapper getRowMapper(Class type) { RelationalPersistentEntity entity = (RelationalPersistentEntity) strategy.getMappingContext() @@ -92,5 +121,8 @@ static class ConversionWithConstructorCreation { static class EntityWithCollection { List ids; + Set integerSet; + Integer[] boxedIntegers; + int[] primitiveIntegers; } } From b5c90d17a192b44681eb9be26c841c0533578743 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 5 Dec 2018 10:07:25 +0100 Subject: [PATCH 6/6] #30 - Address review feedback. Introduce ArrayColumns type to encapsulate Dialect-specific array support. Apply array conversion for properties that do not match the native array type. Add integration tests for Postgres array columns. --- .../data/r2dbc/dialect/ArrayColumns.java | 53 +++++++ .../data/r2dbc/dialect/Dialect.java | 10 +- .../data/r2dbc/dialect/PostgresDialect.java | 48 +++++- .../DefaultReactiveDataAccessStrategy.java | 24 ++- .../function/convert/EntityRowMapper.java | 7 +- .../dialect/PostgresDialectUnitTests.java | 49 ++++-- .../dialect/SqlServerDialectUnitTests.java | 9 ++ .../function/PostgresIntegrationTests.java | 147 ++++++++++++++++++ .../convert/EntityRowMapperUnitTests.java | 9 +- 9 files changed, 316 insertions(+), 40 deletions(-) create mode 100644 src/main/java/org/springframework/data/r2dbc/dialect/ArrayColumns.java create mode 100644 src/test/java/org/springframework/data/r2dbc/function/PostgresIntegrationTests.java diff --git a/src/main/java/org/springframework/data/r2dbc/dialect/ArrayColumns.java b/src/main/java/org/springframework/data/r2dbc/dialect/ArrayColumns.java new file mode 100644 index 00000000..5259af22 --- /dev/null +++ b/src/main/java/org/springframework/data/r2dbc/dialect/ArrayColumns.java @@ -0,0 +1,53 @@ +package org.springframework.data.r2dbc.dialect; + +/** + * Interface declaring methods that express how a dialect supports array-typed columns. + * + * @author Mark Paluch + */ +public interface ArrayColumns { + + /** + * Returns {@literal true} if the dialect supports array-typed columns. + * + * @return {@literal true} if the dialect supports array-typed columns. + */ + boolean isSupported(); + + /** + * Translate the {@link Class user type} of an array into the dialect-specific type. This method considers only the + * component type. + * + * @param userType component type of the array. + * @return the dialect-supported array type. + * @throws UnsupportedOperationException if array typed columns are not supported. + * @throws IllegalArgumentException if the {@code userType} is not a supported array type. + */ + Class getArrayType(Class userType); + + /** + * Default {@link ArrayColumns} implementation for dialects that do not support array-typed columns. + */ + enum Unsupported implements ArrayColumns { + + INSTANCE; + + /* + * (non-Javadoc) + * @see org.springframework.data.r2dbc.dialect.ArrayColumns#isSupported() + */ + @Override + public boolean isSupported() { + return false; + } + + /* + * (non-Javadoc) + * @see org.springframework.data.r2dbc.dialect.ArrayColumns#getArrayType(java.lang.Class) + */ + @Override + public Class getArrayType(Class userType) { + throw new UnsupportedOperationException("Array types not supported"); + } + } +} diff --git a/src/main/java/org/springframework/data/r2dbc/dialect/Dialect.java b/src/main/java/org/springframework/data/r2dbc/dialect/Dialect.java index 411f3839..e909f722 100644 --- a/src/main/java/org/springframework/data/r2dbc/dialect/Dialect.java +++ b/src/main/java/org/springframework/data/r2dbc/dialect/Dialect.java @@ -5,6 +5,7 @@ import java.util.HashSet; import org.springframework.data.mapping.model.SimpleTypeHolder; +import org.springframework.data.r2dbc.dialect.ArrayColumns.Unsupported; /** * Represents a dialect that is implemented by a particular database. @@ -61,12 +62,11 @@ default SimpleTypeHolder getSimpleTypeHolder() { LimitClause limit(); /** - * Returns {@literal true} whether this dialect supports array-typed column. Collection-typed columns can map their - * content to native array types. + * Returns the array support object that describes how array-typed columns are supported by this dialect. * - * @return {@literal true} whether this dialect supports array-typed columns. + * @return the array support object that describes how array-typed columns are supported by this dialect. */ - default boolean supportsArrayColumns() { - return false; + default ArrayColumns getArraySupport() { + return Unsupported.INSTANCE; } } diff --git a/src/main/java/org/springframework/data/r2dbc/dialect/PostgresDialect.java b/src/main/java/org/springframework/data/r2dbc/dialect/PostgresDialect.java index 8d0c0b3b..d0b1bf0b 100644 --- a/src/main/java/org/springframework/data/r2dbc/dialect/PostgresDialect.java +++ b/src/main/java/org/springframework/data/r2dbc/dialect/PostgresDialect.java @@ -1,15 +1,20 @@ package org.springframework.data.r2dbc.dialect; +import lombok.RequiredArgsConstructor; + import java.net.InetAddress; import java.net.URI; import java.net.URL; import java.util.Arrays; import java.util.Collection; import java.util.HashSet; -import java.util.List; import java.util.Set; import java.util.UUID; +import org.springframework.data.mapping.model.SimpleTypeHolder; +import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; + /** * An SQL dialect for Postgres. * @@ -18,7 +23,7 @@ public class PostgresDialect implements Dialect { private static final Set> SIMPLE_TYPES = new HashSet<>( - Arrays.asList(List.class, Collection.class, String[].class, UUID.class, URL.class, URI.class, InetAddress.class)); + Arrays.asList(UUID.class, URL.class, URI.class, InetAddress.class)); /** * Singleton instance. @@ -57,6 +62,8 @@ public Position getClausePosition() { } }; + private final PostgresArrayColumns ARRAY_COLUMNS = new PostgresArrayColumns(getSimpleTypeHolder()); + /* * (non-Javadoc) * @see org.springframework.data.r2dbc.dialect.Dialect#getBindMarkersFactory() @@ -95,10 +102,41 @@ public LimitClause limit() { /* * (non-Javadoc) - * @see org.springframework.data.r2dbc.dialect.Dialect#supportsArrayColumns() + * @see org.springframework.data.r2dbc.dialect.Dialect#getArraySupport() */ @Override - public boolean supportsArrayColumns() { - return true; + public ArrayColumns getArraySupport() { + return ARRAY_COLUMNS; + } + + @RequiredArgsConstructor + static class PostgresArrayColumns implements ArrayColumns { + + private final SimpleTypeHolder simpleTypes; + + /* + * (non-Javadoc) + * @see org.springframework.data.r2dbc.dialect.ArrayColumns#isSupported() + */ + @Override + public boolean isSupported() { + return true; + } + + /* + * (non-Javadoc) + * @see org.springframework.data.r2dbc.dialect.ArrayColumns#getArrayType(java.lang.Class) + */ + @Override + public Class getArrayType(Class userType) { + + Assert.notNull(userType, "Array component type must not be null"); + + if (!simpleTypes.isSimpleType(userType)) { + throw new IllegalArgumentException("Unsupported array type: " + ClassUtils.getQualifiedName(userType)); + } + + return ClassUtils.resolvePrimitiveIfNecessary(userType); + } } } diff --git a/src/main/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategy.java b/src/main/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategy.java index abe26fc3..537dcdd0 100644 --- a/src/main/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/r2dbc/function/DefaultReactiveDataAccessStrategy.java @@ -38,6 +38,7 @@ import org.springframework.data.domain.Sort.Order; import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.context.MappingContext; +import org.springframework.data.r2dbc.dialect.ArrayColumns; import org.springframework.data.r2dbc.dialect.BindMarker; import org.springframework.data.r2dbc.dialect.BindMarkers; import org.springframework.data.r2dbc.dialect.Dialect; @@ -249,7 +250,7 @@ private RelationalPersistentEntity getPersistentEntity(Class typeToRead) { private Object getWriteValue(PersistentPropertyAccessor propertyAccessor, RelationalPersistentProperty property) { TypeInformation type = property.getTypeInformation(); - Object value = relationalConverter.writeValue(propertyAccessor.getProperty(property), type); + Object value = propertyAccessor.getProperty(property); if (type.isCollectionLike()) { @@ -260,17 +261,28 @@ private Object getWriteValue(PersistentPropertyAccessor propertyAccessor, Relati throw new InvalidDataAccessApiUsageException("Nested entities are not supported"); } - if (!dialect.supportsArrayColumns()) { + ArrayColumns arrayColumns = dialect.getArraySupport(); + + if (!arrayColumns.isSupported()) { throw new InvalidDataAccessResourceUsageException( "Dialect " + dialect.getClass().getName() + " does not support array columns"); } - if (!property.isArray()) { + return getArrayValue(arrayColumns, property, value); + } - Object zeroLengthArray = Array.newInstance(property.getActualType(), 0); - return relationalConverter.getConversionService().convert(value, zeroLengthArray.getClass()); - } + return value; + } + + private Object getArrayValue(ArrayColumns arrayColumns, RelationalPersistentProperty property, Object value) { + + Class targetType = arrayColumns.getArrayType(property.getActualType()); + + if (!property.isArray() || !property.getActualType().equals(targetType)) { + + Object zeroLengthArray = Array.newInstance(targetType, 0); + return relationalConverter.getConversionService().convert(value, zeroLengthArray.getClass()); } return value; diff --git a/src/main/java/org/springframework/data/r2dbc/function/convert/EntityRowMapper.java b/src/main/java/org/springframework/data/r2dbc/function/convert/EntityRowMapper.java index 2f9e89c3..515718cc 100644 --- a/src/main/java/org/springframework/data/r2dbc/function/convert/EntityRowMapper.java +++ b/src/main/java/org/springframework/data/r2dbc/function/convert/EntityRowMapper.java @@ -96,7 +96,8 @@ private Object readFrom(Row row, RelationalPersistentProperty property, String p return readEntityFrom(row, property); } - return converter.readValue(row.get(prefix + property.getColumnName()), property.getTypeInformation()); + Object value = row.get(prefix + property.getColumnName()); + return converter.readValue(value, property.getTypeInformation()); } catch (Exception o_O) { throw new MappingException(String.format("Could not read property %s from result set!", property), o_O); @@ -156,9 +157,7 @@ public T getParameterValue(Parameter parame String column = prefix + property.getColumnName(); try { - - Object value = converter.readValue(resultSet.get(column), property.getTypeInformation()); - return converter.getConversionService().convert(value, parameter.getType().getType()); + return converter.getConversionService().convert(resultSet.get(column), parameter.getType().getType()); } catch (Exception o_O) { throw new MappingException(String.format("Couldn't read column %s from Row.", column), o_O); } diff --git a/src/test/java/org/springframework/data/r2dbc/dialect/PostgresDialectUnitTests.java b/src/test/java/org/springframework/data/r2dbc/dialect/PostgresDialectUnitTests.java index 7df2e22f..e1fe8607 100644 --- a/src/test/java/org/springframework/data/r2dbc/dialect/PostgresDialectUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/dialect/PostgresDialectUnitTests.java @@ -1,8 +1,8 @@ package org.springframework.data.r2dbc.dialect; import static org.assertj.core.api.Assertions.*; +import static org.assertj.core.api.SoftAssertions.*; -import java.util.Collection; import java.util.List; import org.junit.Test; @@ -28,35 +28,50 @@ public void shouldUsePostgresPlaceholders() { } @Test // gh-30 - public void shouldConsiderCollectionTypesAsSimple() { + public void shouldConsiderSimpleTypes() { SimpleTypeHolder holder = PostgresDialect.INSTANCE.getSimpleTypeHolder(); - assertThat(holder.isSimpleType(List.class)).isTrue(); - assertThat(holder.isSimpleType(Collection.class)).isTrue(); + assertSoftly(it -> { + it.assertThat(holder.isSimpleType(String.class)).isTrue(); + it.assertThat(holder.isSimpleType(int.class)).isTrue(); + it.assertThat(holder.isSimpleType(Integer.class)).isTrue(); + }); } @Test // gh-30 - public void shouldConsiderStringArrayTypeAsSimple() { + public void shouldSupportArrays() { - SimpleTypeHolder holder = PostgresDialect.INSTANCE.getSimpleTypeHolder(); + ArrayColumns arrayColumns = PostgresDialect.INSTANCE.getArraySupport(); + + assertThat(arrayColumns.isSupported()).isTrue(); + } + + @Test // gh-30 + public void shouldUseBoxedArrayTypesForPrimitiveTypes() { - assertThat(holder.isSimpleType(String[].class)).isTrue(); + ArrayColumns arrayColumns = PostgresDialect.INSTANCE.getArraySupport(); - @Test // gh-30 - public void shouldConsiderIntArrayTypeAsSimple() { + assertSoftly(it -> { + it.assertThat(arrayColumns.getArrayType(int.class)).isEqualTo(Integer.class); + it.assertThat(arrayColumns.getArrayType(double.class)).isEqualTo(Double.class); + it.assertThat(arrayColumns.getArrayType(String.class)).isEqualTo(String.class); + }); + } - SimpleTypeHolder holder = PostgresDialect.INSTANCE.getSimpleTypeHolder(); + @Test // gh-30 + public void shouldRejectNonSimpleArrayTypes() { - assertThat(holder.isSimpleType(int[].class)).isTrue(); - } + ArrayColumns arrayColumns = PostgresDialect.INSTANCE.getArraySupport(); - @Test // gh-30 - public void shouldConsiderIntegerArrayTypeAsSimple() { + assertThatThrownBy(() -> arrayColumns.getArrayType(getClass())).isInstanceOf(IllegalArgumentException.class); + } + + @Test // gh-30 + public void shouldRejectNestedCollections() { - SimpleTypeHolder holder = PostgresDialect.INSTANCE.getSimpleTypeHolder(); + ArrayColumns arrayColumns = PostgresDialect.INSTANCE.getArraySupport(); - assertThat(holder.isSimpleType(Integer[].class)).isTrue(); - } + assertThatThrownBy(() -> arrayColumns.getArrayType(List.class)).isInstanceOf(IllegalArgumentException.class); } } diff --git a/src/test/java/org/springframework/data/r2dbc/dialect/SqlServerDialectUnitTests.java b/src/test/java/org/springframework/data/r2dbc/dialect/SqlServerDialectUnitTests.java index 1e96e389..cb39be65 100644 --- a/src/test/java/org/springframework/data/r2dbc/dialect/SqlServerDialectUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/dialect/SqlServerDialectUnitTests.java @@ -33,4 +33,13 @@ public void shouldConsiderUuidAsSimple() { assertThat(holder.isSimpleType(UUID.class)).isTrue(); } + + @Test // gh-30 + public void shouldNotSupportArrays() { + + ArrayColumns arrayColumns = SqlServerDialect.INSTANCE.getArraySupport(); + + assertThat(arrayColumns.isSupported()).isFalse(); + assertThatThrownBy(() -> arrayColumns.getArrayType(String.class)).isInstanceOf(UnsupportedOperationException.class); + } } diff --git a/src/test/java/org/springframework/data/r2dbc/function/PostgresIntegrationTests.java b/src/test/java/org/springframework/data/r2dbc/function/PostgresIntegrationTests.java new file mode 100644 index 00000000..8321eddb --- /dev/null +++ b/src/test/java/org/springframework/data/r2dbc/function/PostgresIntegrationTests.java @@ -0,0 +1,147 @@ +/* + * Copyright 2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.r2dbc.function; + +import static org.assertj.core.api.Assertions.*; + +import io.r2dbc.spi.ConnectionFactory; +import lombok.AllArgsConstructor; +import reactor.test.StepVerifier; + +import java.util.Arrays; +import java.util.List; +import java.util.function.Consumer; + +import javax.sql.DataSource; + +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Ignore; +import org.junit.Test; +import org.springframework.data.r2dbc.testing.ExternalDatabase; +import org.springframework.data.r2dbc.testing.PostgresTestSupport; +import org.springframework.data.r2dbc.testing.R2dbcIntegrationTestSupport; +import org.springframework.data.relational.core.mapping.Table; +import org.springframework.jdbc.core.JdbcTemplate; + +/** + * Integration tests for PostgreSQL-specific features such as array support. + * + * @author Mark Paluch + */ +public class PostgresIntegrationTests extends R2dbcIntegrationTestSupport { + + @ClassRule public static final ExternalDatabase database = PostgresTestSupport.database(); + + DataSource dataSource = PostgresTestSupport.createDataSource(database); + ConnectionFactory connectionFactory = PostgresTestSupport.createConnectionFactory(database); + JdbcTemplate template = createJdbcTemplate(dataSource); + DatabaseClient client = DatabaseClient.create(connectionFactory); + + @Before + public void before() { + + template.execute("DROP TABLE IF EXISTS with_arrays"); + template.execute("CREATE TABLE with_arrays (" // + + "boxed_array INT[]," // + + "primitive_array INT[]," // + + "multidimensional_array INT[]," // + + "collection_array INT[][])"); + } + + @Test // gh-30 + @Ignore("https://github.com/r2dbc/r2dbc-postgresql/issues/40, r2dbc-postgresql returns Object[] instead of Integer[]") + public void shouldReadAndWritePrimitiveSingleDimensionArrays() { + + EntityWithArrays withArrays = new EntityWithArrays(null, new int[] { 1, 2, 3 }, null, null); + + insert(withArrays); + selectAndAssert(actual -> { + assertThat(actual.primitiveArray).containsExactly(1, 2, 3); + }); + } + + @Test // gh-30 + public void shouldReadAndWriteBoxedSingleDimensionArrays() { + + EntityWithArrays withArrays = new EntityWithArrays(new Integer[] { 1, 2, 3 }, null, null, null); + + insert(withArrays); + + selectAndAssert(actual -> { + + assertThat(actual.boxedArray).containsExactly(1, 2, 3); + + }); + } + + @Test // gh-30 + public void shouldReadAndWriteConvertedDimensionArrays() { + + EntityWithArrays withArrays = new EntityWithArrays(null, null, null, Arrays.asList(5, 6, 7)); + + insert(withArrays); + + selectAndAssert(actual -> { + assertThat(actual.collectionArray).containsExactly(5, 6, 7); + }); + } + + @Test // gh-30 + @Ignore("https://github.com/r2dbc/r2dbc-postgresql/issues/42, Multi-dimensional arrays not supported yet") + public void shouldReadAndWriteMultiDimensionArrays() { + + EntityWithArrays withArrays = new EntityWithArrays(null, null, new int[][] { { 1, 2, 3 }, { 4, 5 } }, null); + + insert(withArrays); + + selectAndAssert(actual -> { + + assertThat(actual.multidimensionalArray).hasSize(2); + assertThat(actual.multidimensionalArray[0]).containsExactly(1, 2, 3); + assertThat(actual.multidimensionalArray[1]).containsExactly(4, 5, 6); + }); + } + + private void insert(EntityWithArrays object) { + + client.insert() // + .into(EntityWithArrays.class) // + .using(object) // + .then() // + .as(StepVerifier::create) // + .verifyComplete(); + } + + private void selectAndAssert(Consumer assertion) { + + client.select() // + .from(EntityWithArrays.class).fetch() // + .first() // + .as(StepVerifier::create) // + .consumeNextWith(assertion).verifyComplete(); + } + + @Table("with_arrays") + @AllArgsConstructor + static class EntityWithArrays { + + Integer[] boxedArray; + int[] primitiveArray; + int[][] multidimensionalArray; + List collectionArray; + } +} diff --git a/src/test/java/org/springframework/data/r2dbc/function/convert/EntityRowMapperUnitTests.java b/src/test/java/org/springframework/data/r2dbc/function/convert/EntityRowMapperUnitTests.java index d7023ace..91dceff2 100644 --- a/src/test/java/org/springframework/data/r2dbc/function/convert/EntityRowMapperUnitTests.java +++ b/src/test/java/org/springframework/data/r2dbc/function/convert/EntityRowMapperUnitTests.java @@ -8,6 +8,7 @@ import lombok.RequiredArgsConstructor; import java.util.List; +import java.util.Set; import org.junit.Test; import org.junit.runner.RunWith; @@ -20,6 +21,7 @@ * Unit tests for {@link EntityRowMapper}. * * @author Mark Paluch + * @author Jens Schauder */ @RunWith(MockitoJUnitRunner.class) public class EntityRowMapperUnitTests { @@ -73,7 +75,7 @@ public void shouldConvertArrayToCollection() { public void shouldConvertArrayToSet() { EntityRowMapper mapper = getRowMapper(EntityWithCollection.class); - when(rowMock.get("integerSet")).thenReturn((new int[] { 3, 14 })); + when(rowMock.get("integer_set")).thenReturn((new int[] { 3, 14 })); EntityWithCollection result = mapper.apply(rowMock, metadata); assertThat(result.integerSet).contains(3, 14); @@ -83,7 +85,7 @@ public void shouldConvertArrayToSet() { public void shouldConvertArrayMembers() { EntityRowMapper mapper = getRowMapper(EntityWithCollection.class); - when(rowMock.get("primitiveIntegers")).thenReturn((new long[] { 3L, 14L })); + when(rowMock.get("primitive_integers")).thenReturn((new Long[] { 3L, 14L })); EntityWithCollection result = mapper.apply(rowMock, metadata); assertThat(result.primitiveIntegers).contains(3, 14); @@ -93,11 +95,12 @@ public void shouldConvertArrayMembers() { public void shouldConvertArrayToBoxedArray() { EntityRowMapper mapper = getRowMapper(EntityWithCollection.class); - when(rowMock.get("boxedIntegers")).thenReturn((new int[] { 3, 11 })); + when(rowMock.get("boxed_integers")).thenReturn((new int[] { 3, 11 })); EntityWithCollection result = mapper.apply(rowMock, metadata); assertThat(result.boxedIntegers).contains(3, 11); } + @SuppressWarnings("unchecked") private EntityRowMapper getRowMapper(Class type) { RelationalPersistentEntity entity = (RelationalPersistentEntity) strategy.getMappingContext()