From 43f4a542f9bb644d61ae0b33a9445218ce482bdb Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 16 Jul 2025 14:45:35 +0200 Subject: [PATCH] Polishing. Use Standard Exception handling for JDBC SQLException. Introduce easier and consistent mechanism to construct JdbcCustomConversions. Add tests and fix mocking setup. Fix typos. See #1828 Original pull request #2062 --- .../core/convert/JdbcCustomConversions.java | 168 ++++++++++++++++-- .../core/convert/MappingJdbcConverter.java | 95 +++++----- .../jdbc/core/mapping/AggregateReference.java | 9 + .../config/AbstractJdbcConfiguration.java | 12 +- .../query/StringBasedJdbcQuery.java | 3 +- ...cConverterAggregateReferenceUnitTests.java | 99 ++++++++++- ...oryCrossAggregateHsqlIntegrationTests.java | 11 +- .../JdbcRepositoryIntegrationTests.java | 2 +- .../query/StringBasedJdbcQueryUnitTests.java | 2 +- .../core/ReactiveDataAccessStrategyTests.java | 17 +- .../MappingRelationalConverter.java | 22 ++- 11 files changed, 352 insertions(+), 88 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcCustomConversions.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcCustomConversions.java index 81befa9ab..8eaa6f2b4 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcCustomConversions.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcCustomConversions.java @@ -15,13 +15,23 @@ */ package org.springframework.data.jdbc.core.convert; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.function.Consumer; +import org.springframework.core.convert.converter.Converter; +import org.springframework.core.convert.converter.ConverterFactory; +import org.springframework.core.convert.converter.GenericConverter; import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair; +import org.springframework.data.convert.ConverterBuilder; import org.springframework.data.convert.CustomConversions; import org.springframework.data.jdbc.core.mapping.JdbcSimpleTypes; +import org.springframework.data.relational.core.dialect.Dialect; +import org.springframework.lang.Contract; +import org.springframework.util.Assert; /** * Value object to capture custom conversion. {@link JdbcCustomConversions} also act as factory for @@ -50,10 +60,10 @@ public class JdbcCustomConversions extends CustomConversions { * Create a new {@link JdbcCustomConversions} instance registering the given converters and the default store * converters. * - * @param converters must not be {@literal null}. + * @param userConverters must not be {@literal null}. */ - public JdbcCustomConversions(List converters) { - super(constructConverterConfiguration(converters)); + public JdbcCustomConversions(List userConverters) { + this(StoreConversions.of(JdbcSimpleTypes.HOLDER, STORE_CONVERTERS), userConverters); } /** @@ -63,12 +73,7 @@ public class JdbcCustomConversions extends CustomConversions { * @since 2.3 */ public JdbcCustomConversions(StoreConversions storeConversions, List userConverters) { - - super(new ConverterConfiguration( // - storeConversions, // - userConverters, // - JdbcCustomConversions::excludeConversionsBetweenDateAndJsr310Types // - )); + super(JdbcConverterConfigurer.from(storeConversions).registerConverters(userConverters).createConfiguration()); } /** @@ -82,15 +87,40 @@ public class JdbcCustomConversions extends CustomConversions { super(converterConfiguration); } - private static ConverterConfiguration constructConverterConfiguration(List converters) { + /** + * Create a new {@link JdbcCustomConversions} from the given {@link Dialect} and {@code converters}. + * + * @param dialect must not be {@literal null}. + * @param converters must not be {@literal null}. + * @return a new {@link JdbcCustomConversions} instance configured from the given dialect and configured converters. + * @since 4.0 + */ + public static JdbcCustomConversions of(Dialect dialect, Collection converters) { + + Assert.notNull(dialect, "Dialect must not be null"); + Assert.notNull(converters, "Converters must not be null"); - return new ConverterConfiguration( // - StoreConversions.of(JdbcSimpleTypes.HOLDER, STORE_CONVERTERS), // - converters, // - JdbcCustomConversions::excludeConversionsBetweenDateAndJsr310Types // - ); + return create(dialect, configurer -> configurer.registerConverters(converters)); } + /** + * Create a new {@link JdbcCustomConversions} instance using the given {@link Dialect} and + * {@link JdbcConverterConfigurer} callback to configure converters. + * + * @param dialect the {@link Dialect} to use, must not be {@literal null}. + * @param configurer the configurer callback to configure converters, must not be {@literal null}. + * @return a new {@link JdbcCustomConversions} instance configured from the given dialect and configured converters. + */ + public static JdbcCustomConversions create(Dialect dialect, Consumer configurer) { + + Assert.notNull(dialect, "Dialect must not be null"); + Assert.notNull(configurer, "JdbcConverterConfigurer Consumer must not be null"); + + JdbcConverterConfigurer converterConfigurer = JdbcConverterConfigurer.from(dialect); + configurer.accept(converterConfigurer); + + return new JdbcCustomConversions(converterConfigurer.createConfiguration()); + } /** * Obtain a read only copy of default store converters. @@ -118,4 +148,112 @@ public class JdbcCustomConversions extends CustomConversions { private static boolean excludeConversionsBetweenDateAndJsr310Types(ConvertiblePair cp) { return !isDateTimeApiConversion(cp); } + + /** + * {@link JdbcConverterConfigurer} encapsulates creation of + * {@link org.springframework.data.convert.CustomConversions.ConverterConfiguration} with JDBC specifics. + * + * @author Mark Paluch + * @since 4.0 + */ + public static class JdbcConverterConfigurer { + + private final StoreConversions storeConversions; + private final List customConverters = new ArrayList<>(); + + private JdbcConverterConfigurer(StoreConversions storeConversions) { + this.storeConversions = storeConversions; + } + + /** + * Create a {@link JdbcConverterConfigurer} using the provided {@code dialect} and our own codecs for JSR-310 types. + * + * @param dialect must not be {@literal null}. + * @return + */ + static JdbcConverterConfigurer from(Dialect dialect) { + + List converters = new ArrayList<>(); + converters.addAll(dialect.getConverters()); + converters.addAll(JdbcCustomConversions.storeConverters()); + + StoreConversions storeConversions = StoreConversions.of(JdbcSimpleTypes.HOLDER, converters); + + return new JdbcConverterConfigurer(storeConversions); + } + + /** + * Create a {@link JdbcConverterConfigurer} using the provided {@code storeConversions}. + * + * @param storeConversions must not be {@literal null}. + * @return + */ + static JdbcConverterConfigurer from(StoreConversions storeConversions) { + return new JdbcConverterConfigurer(storeConversions); + } + + /** + * Add a custom {@link Converter} implementation. + * + * @param converter must not be {@literal null}. + * @return this. + */ + @Contract("_ -> this") + public JdbcConverterConfigurer registerConverter(Converter converter) { + + Assert.notNull(converter, "Converter must not be null"); + customConverters.add(converter); + return this; + } + + /** + * Add {@link Converter converters}, {@link ConverterFactory factories}, {@link ConverterBuilder.ConverterAware + * converter-aware objects}, and {@link GenericConverter generic converters}. + * + * @param converters must not be {@literal null} nor contain {@literal null} values. + * @return this. + */ + @Contract("_ -> this") + public JdbcConverterConfigurer registerConverters(Object... converters) { + return registerConverters(Arrays.asList(converters)); + } + + /** + * Add {@link Converter converters}, {@link ConverterFactory factories}, {@link ConverterBuilder.ConverterAware + * converter-aware objects}, and {@link GenericConverter generic converters}. + * + * @param converters must not be {@literal null} nor contain {@literal null} values. + * @return this. + */ + @Contract("_ -> this") + public JdbcConverterConfigurer registerConverters(Collection converters) { + + Assert.notNull(converters, "Converters must not be null"); + Assert.noNullElements(converters, "Converters must not be null nor contain null values"); + + customConverters.addAll(converters); + return this; + } + + /** + * Add a custom {@link ConverterFactory} implementation. + * + * @param converterFactory must not be {@literal null}. + * @return this. + */ + @Contract("_ -> this") + public JdbcConverterConfigurer registerConverterFactory(ConverterFactory converterFactory) { + + Assert.notNull(converterFactory, "ConverterFactory must not be null"); + customConverters.add(converterFactory); + return this; + } + + ConverterConfiguration createConfiguration() { + return new ConverterConfiguration(storeConversions, this.customConverters, + JdbcCustomConversions::excludeConversionsBetweenDateAndJsr310Types); + } + + } + } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverter.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverter.java index 79f2cf519..87ef5bb03 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverter.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverter.java @@ -20,15 +20,14 @@ import java.sql.JDBCType; import java.sql.SQLException; import java.sql.SQLType; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.function.Function; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.springframework.context.ApplicationContextAware; import org.springframework.core.convert.converter.Converter; -import org.springframework.dao.NonTransientDataAccessException; +import org.springframework.dao.DataAccessException; import org.springframework.data.convert.CustomConversions; import org.springframework.data.jdbc.core.mapping.AggregateReference; import org.springframework.data.jdbc.core.mapping.JdbcValue; @@ -46,6 +45,10 @@ import org.springframework.data.relational.core.mapping.RelationalPersistentEnti import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; import org.springframework.data.relational.domain.RowDocument; import org.springframework.data.util.TypeInformation; +import org.springframework.jdbc.UncategorizedSQLException; +import org.springframework.jdbc.support.SQLErrorCodeSQLExceptionTranslator; +import org.springframework.jdbc.support.SQLExceptionSubclassTranslator; +import org.springframework.jdbc.support.SQLExceptionTranslator; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -67,9 +70,9 @@ import org.springframework.util.Assert; */ public class MappingJdbcConverter extends MappingRelationalConverter implements JdbcConverter, ApplicationContextAware { - private static final Log LOG = LogFactory.getLog(MappingJdbcConverter.class); private static final Converter, Map> ITERABLE_OF_ENTRY_TO_MAP_CONVERTER = new IterableOfEntryToMapConverter(); + private SQLExceptionTranslator exceptionTranslator = new SQLExceptionSubclassTranslator(); private final JdbcTypeFactory typeFactory; private final RelationResolver relationResolver; @@ -111,6 +114,15 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements this.relationResolver = relationResolver; } + /** + * Set the exception translator for this instance. Defaults to a {@link SQLErrorCodeSQLExceptionTranslator}. + * + * @see SQLExceptionSubclassTranslator + */ + public void setExceptionTranslator(SQLExceptionTranslator exceptionTranslator) { + this.exceptionTranslator = exceptionTranslator; + } + @Nullable private Class getEntityColumnType(TypeInformation type) { @@ -189,59 +201,44 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements return null; } - TypeInformation originalTargetType = targetType; - value = readJdbcArray(value); - targetType = determineNestedTargetType(targetType); + value = potentiallyUnwrapArray(value); - return possiblyReadToAggregateReference(getPotentiallyConvertedSimpleRead(value, targetType), originalTargetType); + if (AggregateReference.class.isAssignableFrom(targetType.getType())) { + + List> types = targetType.getTypeArguments(); + TypeInformation idType = types.get(1); + if (value instanceof AggregateReference ref) { + return AggregateReference.to(readValue(ref.getId(), idType)); + } else { + return AggregateReference.to(readValue(value, idType)); + } + } + + return getPotentiallyConvertedSimpleRead(value, targetType); } /** * Unwrap a Jdbc array, if such a value is provided */ - private Object readJdbcArray(Object value) { + private Object potentiallyUnwrapArray(Object value) { if (value instanceof Array array) { try { return array.getArray(); } catch (SQLException e) { - throw new FailedToAccessJdbcArrayException(e); + throw translateException("Array.getArray()", null, e); } } return value; } - /** - * Determine the id type of an {@link AggregateReference} that the rest of the conversion infrastructure needs to use - * as a conversion target. - */ - private TypeInformation determineNestedTargetType(TypeInformation ultimateTargetType) { - - if (AggregateReference.class.isAssignableFrom(ultimateTargetType.getType())) { - // the id type of a AggregateReference - return ultimateTargetType.getTypeArguments().get(1); - } - return ultimateTargetType; - } - - /** - * Convert value to an {@link AggregateReference} if that is specified by the parameter targetType. - */ - private Object possiblyReadToAggregateReference(Object value, TypeInformation targetType) { - - if (AggregateReference.class.isAssignableFrom(targetType.getType())) { - return AggregateReference.to(value); - } - return value; - } - @Nullable @Override protected Object getPotentiallyConvertedSimpleWrite(Object value, TypeInformation type) { - if (value instanceof AggregateReference aggregateReference) { - return writeValue(aggregateReference.getId(), type); + if (value instanceof AggregateReference ref) { + return writeValue(ref.getId(), type); } return super.getPotentiallyConvertedSimpleWrite(value, type); @@ -277,11 +274,11 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements public JdbcValue writeJdbcValue(@Nullable Object value, TypeInformation columnType, SQLType sqlType) { TypeInformation targetType = canWriteAsJdbcValue(value) ? TypeInformation.of(JdbcValue.class) : columnType; - - if (value instanceof AggregateReference aggregateReference) { - return writeJdbcValue(aggregateReference.getId(), columnType, sqlType); + + if (value instanceof AggregateReference ref) { + return writeJdbcValue(ref.getId(), columnType, sqlType); } - + Object convertedValue = writeValue(value, targetType); if (convertedValue instanceof JdbcValue result) { @@ -306,7 +303,7 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements return JdbcValue.of(convertedValue, JDBCType.BINARY); } - + return JdbcValue.of(convertedValue, sqlType); } @@ -340,10 +337,18 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements return super.newValueProvider(documentAccessor, evaluator, context); } - private static class FailedToAccessJdbcArrayException extends NonTransientDataAccessException { - public FailedToAccessJdbcArrayException(SQLException e) { - super("Failed to read array", e); - } + /** + * Translate the given {@link SQLException} into a generic {@link DataAccessException}. + * + * @param task readable text describing the task being attempted + * @param sql the SQL query or update that caused the problem (can be {@code null}) + * @param ex the offending {@code SQLException} + * @return a DataAccessException wrapping the {@code SQLException} (never {@code null}) + */ + private DataAccessException translateException(String task, @org.jspecify.annotations.Nullable String sql, + SQLException ex) { + DataAccessException dae = exceptionTranslator.translate(task, sql, ex); + return (dae != null ? dae : new UncategorizedSQLException(task, sql, ex)); } /** diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/AggregateReference.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/AggregateReference.java index a75054588..2ff0dfb30 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/AggregateReference.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/AggregateReference.java @@ -16,6 +16,7 @@ package org.springframework.data.jdbc.core.mapping; import java.util.Objects; +import java.util.function.Function; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -31,6 +32,14 @@ import org.springframework.util.Assert; */ public interface AggregateReference { + /** + * Creates an {@link AggregateReference} that refers to the target aggregate root with the given id. + * + * @param id aggregate identifier. Can be a simple value or an composite id (complex object). + * @return + * @param target aggregate type. + * @param target aggregate identifier type. + */ static AggregateReference to(ID id) { return new IdOnlyAggregateReference<>(id); } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/AbstractJdbcConfiguration.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/AbstractJdbcConfiguration.java index e482daf73..17824fe6f 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/AbstractJdbcConfiguration.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/AbstractJdbcConfiguration.java @@ -49,7 +49,9 @@ import org.springframework.data.relational.core.mapping.DefaultNamingStrategy; import org.springframework.data.relational.core.mapping.NamingStrategy; import org.springframework.data.relational.core.mapping.Table; import org.springframework.data.util.TypeScanner; +import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; +import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; import org.springframework.util.StringUtils; /** @@ -155,7 +157,15 @@ public class AbstractJdbcConfiguration implements ApplicationContextAware { : JdbcArrayColumns.DefaultSupport.INSTANCE; DefaultJdbcTypeFactory jdbcTypeFactory = new DefaultJdbcTypeFactory(operations.getJdbcOperations(), arrayColumns); - return new MappingJdbcConverter(mappingContext, relationResolver, conversions, jdbcTypeFactory); + MappingJdbcConverter mappingJdbcConverter = new MappingJdbcConverter(mappingContext, relationResolver, conversions, + jdbcTypeFactory); + + if (operations instanceof NamedParameterJdbcTemplate namedParameterJdbcTemplate + && namedParameterJdbcTemplate.getJdbcOperations() instanceof JdbcTemplate jdbcTemplate) { + mappingJdbcConverter.setExceptionTranslator(jdbcTemplate.getExceptionTranslator()); + } + + return mappingJdbcConverter; } /** diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java index c6808b093..814daac7d 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java @@ -242,7 +242,8 @@ public class StringBasedJdbcQuery extends AbstractJdbcQuery { JdbcParameters.JdbcParameter parameter = getQueryMethod().getParameters() .getParameter(bindableParameter.getIndex()); - JdbcValue jdbcValue = writeValue(value, parameter.getTypeInformation(), parameter); + JdbcValue jdbcValue = writeValue(value, parameter.getTypeInformation(), + parameter); SQLType jdbcType = jdbcValue.getJdbcType(); if (jdbcType == null) { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverterAggregateReferenceUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverterAggregateReferenceUnitTests.java index 616710f2a..7f8f6d4b3 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverterAggregateReferenceUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverterAggregateReferenceUnitTests.java @@ -18,24 +18,41 @@ package org.springframework.data.jdbc.core.convert; import static org.assertj.core.api.Assertions.*; import static org.mockito.Mockito.*; -import org.assertj.core.api.Assertions; +import java.util.List; + import org.junit.jupiter.api.Test; + +import org.springframework.core.ResolvableType; +import org.springframework.core.convert.converter.Converter; import org.springframework.data.annotation.Id; +import org.springframework.data.convert.ReadingConverter; +import org.springframework.data.convert.WritingConverter; +import org.springframework.data.jdbc.core.dialect.JdbcPostgresDialect; import org.springframework.data.jdbc.core.mapping.AggregateReference; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.repository.CrudRepository; import org.springframework.data.util.TypeInformation; /** * Unit tests for the handling of {@link AggregateReference}s in the {@link MappingJdbcConverter}. * * @author Jens Schauder + * @author Mark Paluch */ class MappingJdbcConverterAggregateReferenceUnitTests { + JdbcCustomConversions conversions = JdbcCustomConversions.of(JdbcPostgresDialect.INSTANCE, + List.of(AggregateIdToLong.INSTANCE, NumberToAggregateId.INSTANCE)); + JdbcMappingContext context = new JdbcMappingContext(); - JdbcConverter converter = new MappingJdbcConverter(context, mock(RelationResolver.class)); + JdbcConverter converter = new MappingJdbcConverter(context, mock(RelationResolver.class), conversions, + JdbcTypeFactory.unsupported()); + + MappingJdbcConverterAggregateReferenceUnitTests() { + context.setSimpleTypeHolder(conversions.getSimpleTypeHolder()); + } RelationalPersistentEntity entity = context.getRequiredPersistentEntity(DummyEntity.class); @@ -46,20 +63,41 @@ class MappingJdbcConverterAggregateReferenceUnitTests { Object readValue = converter.readValue(23, property.getTypeInformation()); - Assertions.assertThat(readValue).isInstanceOf(AggregateReference.class); + assertThat(readValue).isInstanceOf(AggregateReference.class); assertThat(((AggregateReference) readValue).getId()).isEqualTo(23L); } @Test // DATAJDBC-221 void convertsFromAggregateReference() { - final RelationalPersistentProperty property = entity.getRequiredPersistentProperty("reference"); + RelationalPersistentProperty property = entity.getRequiredPersistentProperty("reference"); AggregateReference reference = AggregateReference.to(23); Object writeValue = converter.writeValue(reference, TypeInformation.of(converter.getColumnType(property))); - Assertions.assertThat(writeValue).isEqualTo(23L); + assertThat(writeValue).isEqualTo(23L); + } + + @Test // GH-1828 + void considersReadConverter() { + + TypeInformation targetType = TypeInformation + .of(ResolvableType.forClassWithGenerics(AggregateReference.class, Object.class, AggregateId.class)); + + AggregateReference reference = AggregateReference.to(23); + AggregateReference converted = (AggregateReference) converter.readValue(reference, targetType); + + assertThat(converted.getId()).isEqualTo(new AggregateId(23L)); + } + + @Test // GH-1828 + void considersWriteConverter() { + + AggregateReference reference = AggregateReference.to(new AggregateId(23L)); + Object converted = converter.writeValue(reference, TypeInformation.of(Long.class)); + + assertThat(converted).isEqualTo(23L); } private static class DummyEntity { @@ -67,4 +105,55 @@ class MappingJdbcConverterAggregateReferenceUnitTests { @Id Long simple; AggregateReference reference; } + + static class AggregateOne { + + @Id Long id; + String name; + AggregateReference two; + } + + static class AggregateTwo { + + @Id Long id; + String name; + } + + interface ReferencingAggregateRepository extends CrudRepository { + + } + + record AggregateWithConvertableId(@Id AggregateId id, String name) { + + } + + record AggregateId(Long value) { + + } + + record ReferencingAggregate(@Id Long id, String name, + AggregateReference ref) { + } + + @WritingConverter + private enum AggregateIdToLong implements Converter { + + INSTANCE; + + @Override + public Long convert(AggregateId source) { + return source.value; + } + } + + @ReadingConverter + private enum NumberToAggregateId implements Converter { + + INSTANCE; + + @Override + public AggregateId convert(Number source) { + return new AggregateId(source.longValue()); + } + } } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryCrossAggregateHsqlIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryCrossAggregateHsqlIntegrationTests.java index 384245c35..5580c2571 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryCrossAggregateHsqlIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryCrossAggregateHsqlIntegrationTests.java @@ -18,8 +18,8 @@ package org.springframework.data.jdbc.repository; import static java.util.Arrays.*; import static org.assertj.core.api.Assertions.*; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.ComponentScan; @@ -47,9 +47,7 @@ import org.springframework.test.jdbc.JdbcTestUtils; * * @author Jens Schauder * @author Salim Achouche - * @author Salim Achouche */ - @IntegrationTest @EnabledOnDatabase(DatabaseType.HSQL) public class JdbcRepositoryCrossAggregateHsqlIntegrationTests { @@ -152,7 +150,8 @@ public class JdbcRepositoryCrossAggregateHsqlIntegrationTests { } @WritingConverter - enum AggregateIdToLong implements Converter { + private enum AggregateIdToLong implements Converter { + INSTANCE; @Override @@ -162,7 +161,8 @@ public class JdbcRepositoryCrossAggregateHsqlIntegrationTests { } @ReadingConverter - enum NumberToAggregateId implements Converter { + private enum NumberToAggregateId implements Converter { + INSTANCE; @Override @@ -170,4 +170,5 @@ public class JdbcRepositoryCrossAggregateHsqlIntegrationTests { return new AggregateId(source.longValue()); } } + } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java index fe2df745e..52200a02f 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/JdbcRepositoryIntegrationTests.java @@ -352,7 +352,7 @@ public class JdbcRepositoryIntegrationTests { @ParameterizedTest @NullSource @EnumSource(value = EnumClass.class) // GH-2007 - void shouldSaveWithCustomSpellExpressions(EnumClass value) { + void shouldSaveWithCustomSpelExpressions(EnumClass value) { expressionSqlTypePropagationRepository.saveWithSpel(new ExpressionSqlTypePropagation(1L, value)); diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java index 5dd1e8b10..dba34b2b9 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java @@ -100,7 +100,7 @@ class StringBasedJdbcQueryUnitTests { this.defaultRowMapper = mock(RowMapper.class); this.operations = mock(NamedParameterJdbcOperations.class); - this.context = mock(RelationalMappingContext.class, RETURNS_DEEP_STUBS); + this.context = new RelationalMappingContext(); this.converter = new MappingJdbcConverter(context, mock(RelationResolver.class)); this.delegate = ValueExpressionDelegate.create(); } diff --git a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/core/ReactiveDataAccessStrategyTests.java b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/core/ReactiveDataAccessStrategyTests.java index 19edbae91..4d341da05 100644 --- a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/core/ReactiveDataAccessStrategyTests.java +++ b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/core/ReactiveDataAccessStrategyTests.java @@ -15,6 +15,7 @@ */ package org.springframework.data.r2dbc.core; +import static org.assertj.core.api.SoftAssertions.*; import static org.mockito.Mockito.*; import static org.springframework.data.r2dbc.testing.Assertions.*; @@ -38,19 +39,19 @@ import org.springframework.r2dbc.core.binding.BindTarget; * * @author Mark Paluch */ -public class ReactiveDataAccessStrategyTests { +class ReactiveDataAccessStrategyTests { - BindTarget bindTarget = mock(BindTarget.class); + private BindTarget bindTarget = mock(BindTarget.class); - ReactiveDataAccessStrategy strategy = new DefaultReactiveDataAccessStrategy(MySqlDialect.INSTANCE, + private ReactiveDataAccessStrategy strategy = new DefaultReactiveDataAccessStrategy(MySqlDialect.INSTANCE, Arrays.asList(UuidToStringConverter.INSTANCE, StringToUuidConverter.INSTANCE)); @Test // gh-305 - public void shouldConvertParameter() { + void shouldConvertParameter() { UUID value = UUID.randomUUID(); - SoftAssertions.assertSoftly(softly -> { + assertSoftly(softly -> { softly.assertThat(strategy.getBindValue(Parameter.from(value))).isEqualTo(Parameter.from(value.toString())); softly.assertThat(strategy.getBindValue(Parameter.from(Condition.New))).isEqualTo(Parameter.from("New")); @@ -58,14 +59,14 @@ public class ReactiveDataAccessStrategyTests { } @Test // gh-305 - public void shouldConvertEmptyParameter() { + void shouldConvertEmptyParameter() { assertThat(strategy.getBindValue(Parameter.empty(UUID.class))).isEqualTo(Parameter.empty(String.class)); assertThat(strategy.getBindValue(Parameter.empty(Condition.class))).isEqualTo(Parameter.empty(String.class)); } @Test // gh-305 - public void shouldConvertCriteria() { + void shouldConvertCriteria() { UUID value = UUID.randomUUID(); @@ -80,7 +81,7 @@ public class ReactiveDataAccessStrategyTests { } @Test // gh-305 - public void shouldConvertAssignment() { + void shouldConvertAssignment() { UUID value = UUID.randomUUID(); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MappingRelationalConverter.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MappingRelationalConverter.java index 71947ae80..7c30e4de3 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MappingRelationalConverter.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MappingRelationalConverter.java @@ -44,7 +44,16 @@ import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.PersistentPropertyPathAccessor; import org.springframework.data.mapping.context.MappingContext; -import org.springframework.data.mapping.model.*; +import org.springframework.data.mapping.model.CachingValueExpressionEvaluatorFactory; +import org.springframework.data.mapping.model.ConvertingPropertyAccessor; +import org.springframework.data.mapping.model.EntityInstantiator; +import org.springframework.data.mapping.model.ParameterValueProvider; +import org.springframework.data.mapping.model.PersistentEntityParameterValueProvider; +import org.springframework.data.mapping.model.PropertyValueProvider; +import org.springframework.data.mapping.model.SimpleTypeHolder; +import org.springframework.data.mapping.model.SpELContext; +import org.springframework.data.mapping.model.ValueExpressionEvaluator; +import org.springframework.data.mapping.model.ValueExpressionParameterValueProvider; import org.springframework.data.projection.EntityProjection; import org.springframework.data.projection.EntityProjectionIntrospector; import org.springframework.data.projection.EntityProjectionIntrospector.ProjectionPredicate; @@ -679,7 +688,7 @@ public class MappingRelationalConverter extends AbstractRelationalConverter } // custom conversion - Optional> customWriteTarget = determinCustomWriteTarget(value, type); + Optional> customWriteTarget = determineCustomWriteTarget(value, type); if (customWriteTarget.isPresent()) { return getConversionService().convert(value, customWriteTarget.get()); @@ -688,7 +697,7 @@ public class MappingRelationalConverter extends AbstractRelationalConverter return getPotentiallyConvertedSimpleWrite(value, type); } - private Optional> determinCustomWriteTarget(Object value, TypeInformation type) { + private Optional> determineCustomWriteTarget(Object value, TypeInformation type) { return getConversions().getCustomWriteTarget(value.getClass(), type.getType()) .or(() -> getConversions().getCustomWriteTarget(type.getType())) @@ -721,10 +730,11 @@ public class MappingRelationalConverter extends AbstractRelationalConverter } } - if (getConversionService().canConvert(value.getClass(), type.getType())) { - return getConversionService().convert(value, type.getType()); + if (type.getType().isInstance(value) || !getConversionService().canConvert(value.getClass(), type.getType())) { + return value; } - return value; + + return getConversionService().convert(value, type.getType()); } private Object writeArray(Object value, TypeInformation type) {