From fc4e20f2efcecf41e6fc6359c1c01bfb25876641 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Thu, 4 Jul 2024 16:09:50 +0200 Subject: [PATCH 1/5] 1828-aggregate-ref-with-convertable - Prepare branch --- pom.xml | 2 +- spring-data-jdbc-distribution/pom.xml | 2 +- spring-data-jdbc/pom.xml | 4 ++-- spring-data-r2dbc/pom.xml | 4 ++-- spring-data-relational/pom.xml | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pom.xml b/pom.xml index eaee823fa0..25f119ff69 100644 --- a/pom.xml +++ b/pom.xml @@ -7,7 +7,7 @@ org.springframework.data spring-data-relational-parent - 4.0.0-SNAPSHOT + 4.0.0-1828-aggregate-ref-with-convertable-SNAPSHOT pom Spring Data Relational Parent diff --git a/spring-data-jdbc-distribution/pom.xml b/spring-data-jdbc-distribution/pom.xml index b3c39e64c3..b6d1148cbe 100644 --- a/spring-data-jdbc-distribution/pom.xml +++ b/spring-data-jdbc-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 4.0.0-SNAPSHOT + 4.0.0-1828-aggregate-ref-with-convertable-SNAPSHOT ../pom.xml diff --git a/spring-data-jdbc/pom.xml b/spring-data-jdbc/pom.xml index e61fd64020..0eecd3490a 100644 --- a/spring-data-jdbc/pom.xml +++ b/spring-data-jdbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-jdbc - 4.0.0-SNAPSHOT + 4.0.0-1828-aggregate-ref-with-convertable-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 4.0.0-SNAPSHOT + 4.0.0-1828-aggregate-ref-with-convertable-SNAPSHOT diff --git a/spring-data-r2dbc/pom.xml b/spring-data-r2dbc/pom.xml index 3ee76fd3c1..4bf3670065 100644 --- a/spring-data-r2dbc/pom.xml +++ b/spring-data-r2dbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-r2dbc - 4.0.0-SNAPSHOT + 4.0.0-1828-aggregate-ref-with-convertable-SNAPSHOT Spring Data R2DBC Spring Data module for R2DBC @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 4.0.0-SNAPSHOT + 4.0.0-1828-aggregate-ref-with-convertable-SNAPSHOT diff --git a/spring-data-relational/pom.xml b/spring-data-relational/pom.xml index 8fd6d7a6f0..8928842b1b 100644 --- a/spring-data-relational/pom.xml +++ b/spring-data-relational/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-relational - 4.0.0-SNAPSHOT + 4.0.0-1828-aggregate-ref-with-convertable-SNAPSHOT Spring Data Relational Spring Data Relational support @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 4.0.0-SNAPSHOT + 4.0.0-1828-aggregate-ref-with-convertable-SNAPSHOT From fad6568bf5c76ca1f228ba9d050e94fc99711036 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Thu, 4 Jul 2024 16:43:19 +0200 Subject: [PATCH 2/5] Support AggregateReferences with custom id type. Restructured reading conversion process into: - converting technology base types (JDBC Arrays). - standard and custom conversions. - module specific conversions (AggregateReference). Closes #1828 Original pull request #2062 --- .../core/convert/MappingJdbcConverter.java | 53 +++--- ...ConverterAggregateReferenceUnitTests.java} | 6 +- .../MappingJdbcConverterUnitTests.java | 160 +++++------------- ...oryCrossAggregateHsqlIntegrationTests.java | 64 ++++++- ...rossAggregateHsqlIntegrationTests-hsql.sql | 14 +- .../core/ReactiveDataAccessStrategyTests.java | 9 +- .../MappingRelationalConverter.java | 113 ++++++++----- 7 files changed, 231 insertions(+), 188 deletions(-) rename spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/{BasicRelationalConverterAggregateReferenceUnitTests.java => MappingJdbcConverterAggregateReferenceUnitTests.java} (93%) 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 2c3feffdb6..64447713dc 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 @@ -27,9 +27,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.context.ApplicationContextAware; -import org.springframework.core.convert.ConverterNotFoundException; import org.springframework.core.convert.converter.Converter; -import org.springframework.core.convert.converter.ConverterRegistry; import org.springframework.data.convert.CustomConversions; import org.springframework.data.jdbc.core.mapping.AggregateReference; import org.springframework.data.jdbc.core.mapping.JdbcValue; @@ -91,8 +89,6 @@ public MappingJdbcConverter(RelationalMappingContext context, RelationResolver r this.typeFactory = JdbcTypeFactory.unsupported(); this.relationResolver = relationResolver; - - registerAggregateReferenceConverters(); } /** @@ -112,14 +108,6 @@ public MappingJdbcConverter(RelationalMappingContext context, RelationResolver r this.typeFactory = typeFactory; this.relationResolver = relationResolver; - - registerAggregateReferenceConverters(); - } - - private void registerAggregateReferenceConverters() { - - ConverterRegistry registry = (ConverterRegistry) getConversionService(); - AggregateReferenceConverters.getConvertersToRegister(getConversionService()).forEach(registry::addConverter); } @Nullable @@ -185,33 +173,48 @@ private Class doGetColumnType(RelationalPersistentProperty property) { } @Override - @Nullable - public Object readValue(@Nullable Object value, TypeInformation type) { - - if (value == null) { - return value; - } + protected Object readTechnologyType(Object value) { if (value instanceof Array array) { try { - return super.readValue(array.getArray(), type); - } catch (SQLException | ConverterNotFoundException e) { + return array.getArray(); + } catch (SQLException e) { LOG.info("Failed to extract a value of type %s from an Array; Attempting to use standard conversions", e); + } } - return super.readValue(value, type); + return value; } @Override + protected TypeInformation determineModuleReadTarget(TypeInformation ultimateTargetType) { + + if (AggregateReference.class.isAssignableFrom(ultimateTargetType.getType())) { + // the id type of a AggregateReference + return ultimateTargetType.getTypeArguments().get(1); + } + return ultimateTargetType; + } + + @Override + protected Object readModuleType(Object value, TypeInformation targetType) { + + if (AggregateReference.class.isAssignableFrom(targetType.getType())) { + return AggregateReference.to(value); + } + return value; + } + @Nullable - public Object writeValue(@Nullable Object value, TypeInformation type) { + @Override + protected Object getPotentiallyConvertedSimpleWrite(Object value, TypeInformation type) { - if (value == null) { - return null; + if (value instanceof AggregateReference aggregateReference) { + return writeValue(aggregateReference.getId(), type); } - return super.writeValue(value, type); + return super.getPotentiallyConvertedSimpleWrite(value, type); } private boolean canWriteAsJdbcValue(@Nullable Object value) { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/BasicRelationalConverterAggregateReferenceUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverterAggregateReferenceUnitTests.java similarity index 93% rename from spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/BasicRelationalConverterAggregateReferenceUnitTests.java rename to spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverterAggregateReferenceUnitTests.java index e2b25f5087..616710f2a8 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/BasicRelationalConverterAggregateReferenceUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverterAggregateReferenceUnitTests.java @@ -32,7 +32,7 @@ * * @author Jens Schauder */ -public class BasicRelationalConverterAggregateReferenceUnitTests { +class MappingJdbcConverterAggregateReferenceUnitTests { JdbcMappingContext context = new JdbcMappingContext(); JdbcConverter converter = new MappingJdbcConverter(context, mock(RelationResolver.class)); @@ -40,7 +40,7 @@ public class BasicRelationalConverterAggregateReferenceUnitTests { RelationalPersistentEntity entity = context.getRequiredPersistentEntity(DummyEntity.class); @Test // DATAJDBC-221 - public void convertsToAggregateReference() { + void convertsToAggregateReference() { final RelationalPersistentProperty property = entity.getRequiredPersistentProperty("reference"); @@ -51,7 +51,7 @@ public void convertsToAggregateReference() { } @Test // DATAJDBC-221 - public void convertsFromAggregateReference() { + void convertsFromAggregateReference() { final RelationalPersistentProperty property = entity.getRequiredPersistentProperty("reference"); diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverterUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverterUnitTests.java index 9859151633..129530e50d 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverterUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverterUnitTests.java @@ -40,6 +40,7 @@ import org.junit.jupiter.api.Test; import org.springframework.core.convert.converter.Converter; import org.springframework.data.annotation.Id; +import org.springframework.data.convert.WritingConverter; import org.springframework.data.jdbc.core.mapping.AggregateReference; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.jdbc.core.mapping.JdbcValue; @@ -60,8 +61,7 @@ class MappingJdbcConverterUnitTests { private static final UUID UUID = java.util.UUID.fromString("87a48aa8-a071-705e-54a9-e52fe3a012f1"); private static final byte[] BYTES_REPRESENTING_UUID = { -121, -92, -118, -88, -96, 113, 112, 94, 84, -87, -27, 47, - -29, - -96, 18, -15 }; + -29, -96, 18, -15 }; private JdbcMappingContext context = new JdbcMappingContext(); private StubbedJdbcTypeFactory typeFactory = new StubbedJdbcTypeFactory(); @@ -70,7 +70,7 @@ class MappingJdbcConverterUnitTests { (identifier, path) -> { throw new UnsupportedOperationException(); }, // - new JdbcCustomConversions(), // + new JdbcCustomConversions(List.of(CustomIdToLong.INSTANCE)), // typeFactory // ); @@ -91,6 +91,7 @@ void testTargetTypesForPropertyType() { checkTargetType(softly, entity, "date", Date.class); checkTargetType(softly, entity, "timestamp", Timestamp.class); checkTargetType(softly, entity, "uuid", UUID.class); + checkTargetType(softly, entity, "reference", Long.class); softly.assertAll(); } @@ -216,117 +217,26 @@ private void checkTargetType(SoftAssertions softly, RelationalPersistentEntity reference; - private final UUID uuid; - private final AggregateReference uuidRef; - private final Optional optionalUuid; - - // DATAJDBC-259 - private final List listOfString; - private final String[] arrayOfString; - private final List listOfEntity; - private final OtherEntity[] arrayOfEntity; - - private DummyEntity(Long id, SomeEnum someEnum, LocalDateTime localDateTime, LocalDate localDate, - LocalTime localTime, ZonedDateTime zonedDateTime, OffsetDateTime offsetDateTime, Instant instant, Date date, - Timestamp timestamp, AggregateReference reference, UUID uuid, - AggregateReference uuidRef, Optional optionalUUID, List listOfString, String[] arrayOfString, - List listOfEntity, OtherEntity[] arrayOfEntity) { - this.id = id; - this.someEnum = someEnum; - this.localDateTime = localDateTime; - this.localDate = localDate; - this.localTime = localTime; - this.zonedDateTime = zonedDateTime; - this.offsetDateTime = offsetDateTime; - this.instant = instant; - this.date = date; - this.timestamp = timestamp; - this.reference = reference; - this.uuid = uuid; - this.uuidRef = uuidRef; - this.optionalUuid = optionalUUID; - this.listOfString = listOfString; - this.arrayOfString = arrayOfString; - this.listOfEntity = listOfEntity; - this.arrayOfEntity = arrayOfEntity; - } - - public Long getId() { - return this.id; - } - - public SomeEnum getSomeEnum() { - return this.someEnum; - } - - public LocalDateTime getLocalDateTime() { - return this.localDateTime; - } - - public LocalDate getLocalDate() { - return this.localDate; - } - - public LocalTime getLocalTime() { - return this.localTime; - } - - public ZonedDateTime getZonedDateTime() { - return this.zonedDateTime; - } - - public OffsetDateTime getOffsetDateTime() { - return this.offsetDateTime; - } - - public Instant getInstant() { - return this.instant; - } - - public Date getDate() { - return this.date; - } - - public Timestamp getTimestamp() { - return this.timestamp; - } - - public AggregateReference getReference() { - return this.reference; - } - - public UUID getUuid() { - return this.uuid; - } - - public List getListOfString() { - return this.listOfString; - } - - public String[] getArrayOfString() { - return this.arrayOfString; - } - - public List getListOfEntity() { - return this.listOfEntity; - } - - public OtherEntity[] getArrayOfEntity() { - return this.arrayOfEntity; - } + private record DummyEntity( // + @Id Long id, // + SomeEnum someEnum, // + LocalDateTime localDateTime, // + LocalDate localDate, // + LocalTime localTime, // + ZonedDateTime zonedDateTime, // + OffsetDateTime offsetDateTime, // + Instant instant, // + Date date, // + Timestamp timestamp, // + AggregateReference reference, // + UUID uuid, // + AggregateReference uuidRef, // + Optional optionalUuid, // + List listOfString, // + String[] arrayOfString, // + List listOfEntity, // + OtherEntity[] arrayOfEntity // + ) { } @SuppressWarnings("unused") @@ -337,6 +247,18 @@ private enum SomeEnum { @SuppressWarnings("unused") private static class OtherEntity {} + private static class EnumIdEntity { + @Id SomeEnum id; + } + + private static class CustomIdEntity { + @Id CustomId id; + } + + private record CustomId(Long id) { + + } + private static class StubbedJdbcTypeFactory implements JdbcTypeFactory { Object[] arraySource; @@ -366,4 +288,14 @@ public UUID convert(byte[] source) { return new UUID(high, low); } } + + @WritingConverter + enum CustomIdToLong implements Converter { + INSTANCE; + + @Override + public Long convert(CustomId source) { + return source.id; + } + } } 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 1c52709796..e975fa9465 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 @@ -15,15 +15,22 @@ */ 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; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.FilterType; import org.springframework.context.annotation.Import; +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.convert.JdbcCustomConversions; import org.springframework.data.jdbc.core.mapping.AggregateReference; import org.springframework.data.jdbc.repository.config.EnableJdbcRepositories; import org.springframework.data.jdbc.testing.DatabaseType; @@ -52,13 +59,19 @@ public class JdbcRepositoryCrossAggregateHsqlIntegrationTests { @Configuration @Import(TestConfiguration.class) @EnableJdbcRepositories(considerNestedRepositories = true, - includeFilters = @ComponentScan.Filter(value = Ones.class, type = FilterType.ASSIGNABLE_TYPE)) + includeFilters = @ComponentScan.Filter(value = { Ones.class, ReferencingAggregateRepository.class }, + type = FilterType.ASSIGNABLE_TYPE)) static class Config { + @Bean + JdbcCustomConversions jdbcCustomConversions() { + return new JdbcCustomConversions(asList(AggregateIdToLong.INSTANCE, LongToAggregateId.INSTANCE)); + } } @Autowired NamedParameterJdbcTemplate template; @Autowired Ones ones; + @Autowired ReferencingAggregateRepository referencingAggregates; @Autowired RelationalMappingContext context; @SuppressWarnings("ConstantConditions") @@ -95,6 +108,19 @@ public void savesAndUpdate() { ).isEqualTo(1); } + @Test // GH-1828 + @Disabled + public void savesAndReadWithConvertableId() { + + AggregateReference idReference = AggregateReference + .to(new AggregateId(TWO_ID)); + ReferencingAggregate reference = referencingAggregates + .save(new ReferencingAggregate(null, "Reference", idReference)); + + ReferencingAggregate reloaded = referencingAggregates.findById(reference.id).get(); + assertThat(reloaded.id()).isEqualTo(idReference); + } + interface Ones extends CrudRepository {} static class AggregateOne { @@ -109,4 +135,40 @@ 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 + enum AggregateIdToLong implements Converter { + INSTANCE; + + @Override + public Long convert(AggregateId source) { + return source.value; + } + } + + @ReadingConverter + enum LongToAggregateId implements Converter { + INSTANCE; + + @Override + public AggregateId convert(Long source) { + return new AggregateId(source); + } + } } diff --git a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryCrossAggregateHsqlIntegrationTests-hsql.sql b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryCrossAggregateHsqlIntegrationTests-hsql.sql index f03df7b7ea..74944aeca6 100644 --- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryCrossAggregateHsqlIntegrationTests-hsql.sql +++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.repository/JdbcRepositoryCrossAggregateHsqlIntegrationTests-hsql.sql @@ -1 +1,13 @@ -CREATE TABLE aggregate_one ( id BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, NAME VARCHAR(100), two INTEGER); +CREATE TABLE aggregate_one +( + id BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, + NAME VARCHAR(100), + two INTEGER +); + +CREATE TABLE REFERENCING_AGGREGATE +( + ID BIGINT GENERATED BY DEFAULT AS IDENTITY ( START WITH 1 ) PRIMARY KEY, + NAME VARCHAR(100), + REF INTEGER +); 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 4389bb91c4..19edbae919 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 @@ -21,8 +21,8 @@ import java.util.Arrays; import java.util.UUID; +import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.Test; - import org.springframework.core.convert.converter.Converter; import org.springframework.data.convert.ReadingConverter; import org.springframework.data.convert.WritingConverter; @@ -50,8 +50,11 @@ public void shouldConvertParameter() { UUID value = UUID.randomUUID(); - assertThat(strategy.getBindValue(Parameter.from(value))).isEqualTo(Parameter.from(value.toString())); - assertThat(strategy.getBindValue(Parameter.from(Condition.New))).isEqualTo(Parameter.from("New")); + SoftAssertions.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")); + }); } @Test // gh-305 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 82e7e133c3..51c36303fc 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 @@ -568,7 +568,9 @@ private void readProperties(ConversionContext context, RelationalPersistentEntit continue; } - accessor.setProperty(property, valueProviderToUse.getPropertyValue(property)); + Object propertyValue = valueProviderToUse.getPropertyValue(property); + propertyValue = readValue(propertyValue, property.getTypeInformation()); + accessor.setProperty(property, propertyValue); } } @@ -619,34 +621,65 @@ private boolean shouldReadEmbeddable(ConversionContext context, RelationalPersis return false; } + /** + * Read and convert a single value that is comming from a database to the {@literal targetType} expected by the domain + * model. + * + * @param value a value as it is returned by the driver accessing the persistence store. May be {@code null}. + * @param targetType {@link TypeInformation} into which the value is to be converted. Must not be {@code null}. + * @return + */ @Override @Nullable - public Object readValue(@Nullable Object value, TypeInformation type) { + public Object readValue(@Nullable Object value, TypeInformation targetType) { if (null == value) { return null; } - return getPotentiallyConvertedSimpleRead(value, type); + TypeInformation originalTargetType = targetType; + value = readTechnologyType(value); + targetType = determineModuleReadTarget(targetType); + + return readModuleType(getPotentiallyConvertedSimpleRead(value, targetType), originalTargetType); } /** - * Checks whether we have a custom conversion registered for the given value into an arbitrary simple JDBC type. - * Returns the converted value if so. If not, we perform special enum handling or simply return the value as is. - * - * @param value to be converted. Must not be {@code null}. - * @return the converted value if a conversion applies or the original value. Might return {@code null}. + * Convert a read value using module dependent special conversions. Spring Data JDBC for example uses this to + * implement the conversion of AggregateReferences. There is no guarantee that the value is converted to the exact + * required TypeInformation, nor that it is converted at all. + * + * @param value the value read from the database. Must not be {@literal null}. + * @param targetType the type to which the value should get converted if possible. Must not be {@literal null}. + * @return a potentially converted value. */ - @Nullable - private Object getPotentiallyConvertedSimpleWrite(Object value) { - - Optional> customTarget = getConversions().getCustomWriteTarget(value.getClass()); + protected Object readModuleType(Object value, TypeInformation targetType) { + return value; + } - if (customTarget.isPresent()) { - return getConversionService().convert(value, customTarget.get()); - } + /** + * Read technology specific values into objects that then can be fed in the normal conversion process for reading. An + * example are the conversion of JDBCs {@literal Array} type to normal java arrays. + * + * @param value a value read from the database + * @return a preprocessed value suitable for technology-agnostic further processing. + */ + protected Object readTechnologyType(Object value) { + return value; + } - return Enum.class.isAssignableFrom(value.getClass()) ? ((Enum) value).name() : value; + /** + * When type is a type that has special support, this returns the type a value should be converted to before the + * conversion to the special type happens. For example if type is AggregateReference this method returns the second + * parameter type of AggregateReference, in order to allow conversions to handle that type. + * + * @param ultimateTargetType ultimate target type to be returned by the conversion process. Must not be + * {@literal null}. + * @return a type that can be converted to the ultimate target type by module specific handling. Must not be + * {@literal null}. + */ + protected TypeInformation determineModuleReadTarget(TypeInformation ultimateTargetType) { + return ultimateTargetType; } /** @@ -696,33 +729,28 @@ public Object writeValue(@Nullable Object value, TypeInformation type) { return null; } - if (getConversions().isSimpleType(value.getClass())) { + // custom conversion + Optional> customWriteTarget = determinCustomWriteTarget(value, type); - Optional> customWriteTarget = getConversions().hasCustomWriteTarget(value.getClass(), type.getType()) - ? getConversions().getCustomWriteTarget(value.getClass(), type.getType()) - : getConversions().getCustomWriteTarget(type.getType()); - - if (customWriteTarget.isPresent()) { - return getConversionService().convert(value, customWriteTarget.get()); - } + if (customWriteTarget.isPresent()) { + return getConversionService().convert(value, customWriteTarget.get()); + } - if (TypeInformation.OBJECT != type) { + return getPotentiallyConvertedSimpleWrite(value, type); + } - if (type.getType().isAssignableFrom(value.getClass())) { + private Optional> determinCustomWriteTarget(Object value, TypeInformation type) { - if (value.getClass().isEnum()) { - return getPotentiallyConvertedSimpleWrite(value); - } + return getConversions().getCustomWriteTarget(value.getClass(), type.getType()) + .or(() -> getConversions().getCustomWriteTarget(type.getType())) + .or(() -> getConversions().getCustomWriteTarget(value.getClass())); + } - return value; - } else { - if (getConversionService().canConvert(value.getClass(), type.getType())) { - value = getConversionService().convert(value, type.getType()); - } - } - } + @Nullable + protected Object getPotentiallyConvertedSimpleWrite(Object value, TypeInformation type) { - return getPotentiallyConvertedSimpleWrite(value); + if (value instanceof Enum enumValue) { + return enumValue.name(); } if (value.getClass().isArray()) { @@ -744,9 +772,10 @@ public Object writeValue(@Nullable Object value, TypeInformation type) { } } - return - - getConversionService().convert(value, type.getType()); + if (getConversionService().canConvert(value.getClass(), type.getType())) { + return getConversionService().convert(value, type.getType()); + } + return value; } private Object writeArray(Object value, TypeInformation type) { @@ -1187,7 +1216,9 @@ public Object getValue(AggregatePath path) { return null; } - return context.convert(value, path.getRequiredLeafProperty().getTypeInformation()); + // TODO: converting here seems wrong, since we have the ConvertingParameterValueProvider + // return context.convert(value, path.getRequiredLeafProperty().getTypeInformation()); + return value; } @Override From 49f26e015ba0c97a2df4d43fcc759a99f8d8f611 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 27 May 2025 10:12:16 +0200 Subject: [PATCH 3/5] Process review comments. - Wrapped SqlException and rethrow it. - remove AggregateReferenceConverters completely. It was no longer used. Closes #1828 Original pull request #2062 --- .../convert/AggregateReferenceConverters.java | 136 ------------------ .../core/convert/MappingJdbcConverter.java | 14 +- ...AggregateReferenceConvertersUnitTests.java | 99 ------------- 3 files changed, 12 insertions(+), 237 deletions(-) delete mode 100644 spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/AggregateReferenceConverters.java delete mode 100644 spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/AggregateReferenceConvertersUnitTests.java diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/AggregateReferenceConverters.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/AggregateReferenceConverters.java deleted file mode 100644 index b3d0a2ce3c..0000000000 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/AggregateReferenceConverters.java +++ /dev/null @@ -1,136 +0,0 @@ -/* - * Copyright 2021-2025 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 - * - * https://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.jdbc.core.convert; - -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.Set; - -import org.springframework.core.ResolvableType; -import org.springframework.core.convert.ConversionService; -import org.springframework.core.convert.TypeDescriptor; -import org.springframework.core.convert.converter.GenericConverter; -import org.springframework.data.convert.ReadingConverter; -import org.springframework.data.convert.WritingConverter; -import org.springframework.data.jdbc.core.mapping.AggregateReference; -import org.springframework.lang.Nullable; - -/** - * Converters for aggregate references. They need a {@link ConversionService} in order to delegate the conversion of the - * content of the {@link AggregateReference}. - * - * @author Jens Schauder - * @author Mark Paluch - * @since 2.3 - */ -class AggregateReferenceConverters { - - /** - * Returns the converters to be registered. - * - * @return a collection of converters. Guaranteed to be not {@literal null}. - */ - public static Collection getConvertersToRegister(ConversionService conversionService) { - - return Arrays.asList(new AggregateReferenceToSimpleTypeConverter(conversionService), - new SimpleTypeToAggregateReferenceConverter(conversionService)); - } - - /** - * Converts from an AggregateReference to its id, leaving the conversion of the id to the ultimate target type to the - * delegate {@link ConversionService}. - */ - @WritingConverter - private static class AggregateReferenceToSimpleTypeConverter implements GenericConverter { - - private static final Set CONVERTIBLE_TYPES = Collections - .singleton(new ConvertiblePair(AggregateReference.class, Object.class)); - - private final ConversionService delegate; - - AggregateReferenceToSimpleTypeConverter(ConversionService delegate) { - this.delegate = delegate; - } - - @Override - public Set getConvertibleTypes() { - return CONVERTIBLE_TYPES; - } - - @Override - public Object convert(@Nullable Object source, TypeDescriptor sourceDescriptor, TypeDescriptor targetDescriptor) { - - if (source == null) { - return null; - } - - // if the target type is an AggregateReference we are going to assume it is of the correct type, - // because it was already converted. - Class objectType = targetDescriptor.getObjectType(); - if (objectType.isAssignableFrom(AggregateReference.class)) { - return source; - } - - Object id = ((AggregateReference) source).getId(); - - if (id == null) { - throw new IllegalStateException( - String.format("Aggregate references id must not be null when converting to %s from %s to %s", source, - sourceDescriptor, targetDescriptor)); - } - - return delegate.convert(id, TypeDescriptor.valueOf(id.getClass()), targetDescriptor); - } - } - - /** - * Convert any simple type to an {@link AggregateReference}. If the {@literal targetDescriptor} contains information - * about the generic type id will properly get converted to the desired type by the delegate - * {@link ConversionService}. - */ - @ReadingConverter - private static class SimpleTypeToAggregateReferenceConverter implements GenericConverter { - - private static final Set CONVERTIBLE_TYPES = Collections - .singleton(new ConvertiblePair(Object.class, AggregateReference.class)); - - private final ConversionService delegate; - - SimpleTypeToAggregateReferenceConverter(ConversionService delegate) { - this.delegate = delegate; - } - - @Override - public Set getConvertibleTypes() { - return CONVERTIBLE_TYPES; - } - - @Override - public Object convert(@Nullable Object source, TypeDescriptor sourceDescriptor, TypeDescriptor targetDescriptor) { - - if (source == null) { - return null; - } - - ResolvableType componentType = targetDescriptor.getResolvableType().getGenerics()[1]; - TypeDescriptor targetType = TypeDescriptor.valueOf(componentType.resolve()); - Object convertedId = delegate.convert(source, TypeDescriptor.valueOf(source.getClass()), targetType); - - return AggregateReference.to(convertedId); - } - } -} 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 64447713dc..bffe0ddf33 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 @@ -27,7 +27,11 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.context.ApplicationContextAware; +import org.springframework.core.convert.ConversionException; +import org.springframework.core.convert.ConversionFailedException; import org.springframework.core.convert.converter.Converter; +import org.springframework.dao.DataAccessException; +import org.springframework.dao.NonTransientDataAccessException; import org.springframework.data.convert.CustomConversions; import org.springframework.data.jdbc.core.mapping.AggregateReference; import org.springframework.data.jdbc.core.mapping.JdbcValue; @@ -35,6 +39,7 @@ import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mapping.model.SimpleTypeHolder; import org.springframework.data.mapping.model.ValueExpressionEvaluator; +import org.springframework.data.relational.core.conversion.DbActionExecutionException; import org.springframework.data.relational.core.conversion.MappingRelationalConverter; import org.springframework.data.relational.core.conversion.ObjectPath; import org.springframework.data.relational.core.conversion.RelationalConverter; @@ -179,8 +184,7 @@ protected Object readTechnologyType(Object value) { try { return array.getArray(); } catch (SQLException e) { - LOG.info("Failed to extract a value of type %s from an Array; Attempting to use standard conversions", e); - + throw new FailedToAccessJdbcArrayException(e); } } @@ -301,6 +305,12 @@ protected RelationalPropertyValueProvider newValueProvider(RowDocumentAccessor d return super.newValueProvider(documentAccessor, evaluator, context); } + private static class FailedToAccessJdbcArrayException extends NonTransientDataAccessException { + public FailedToAccessJdbcArrayException(SQLException e) { + super("Failed to read array", e); + } + } + /** * {@link RelationalPropertyValueProvider} using a resolving context to lookup relations. This is highly * context-sensitive. Note that the identifier is held here because of a chicken and egg problem, while diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/AggregateReferenceConvertersUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/AggregateReferenceConvertersUnitTests.java deleted file mode 100644 index eab84c6a76..0000000000 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/AggregateReferenceConvertersUnitTests.java +++ /dev/null @@ -1,99 +0,0 @@ -/* - * Copyright 2021-2025 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 - * - * https://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.jdbc.core.convert; - -import static org.assertj.core.api.Assertions.*; - -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -import org.springframework.core.ResolvableType; -import org.springframework.core.convert.TypeDescriptor; -import org.springframework.core.convert.support.ConfigurableConversionService; -import org.springframework.core.convert.support.DefaultConversionService; -import org.springframework.data.jdbc.core.mapping.AggregateReference; - -/** - * Tests for converters from an to {@link org.springframework.data.jdbc.core.mapping.AggregateReference}. - * - * @author Jens Schauder - * @author Mark Paluch - */ -class AggregateReferenceConvertersUnitTests { - - ConfigurableConversionService conversionService; - - @BeforeEach - void setUp() { - conversionService = new DefaultConversionService(); - AggregateReferenceConverters.getConvertersToRegister(DefaultConversionService.getSharedInstance()) - .forEach(it -> conversionService.addConverter(it)); - } - - @Test // GH-992 - void convertsFromSimpleValue() { - - ResolvableType aggregateReferenceWithIdTypeInteger = ResolvableType.forClassWithGenerics(AggregateReference.class, - String.class, Integer.class); - Object converted = conversionService.convert(23, TypeDescriptor.forObject(23), - new TypeDescriptor(aggregateReferenceWithIdTypeInteger, null, null)); - - assertThat(converted).isEqualTo(AggregateReference.to(23)); - } - - @Test // GH-992 - void convertsFromSimpleValueThatNeedsSeparateConversion() { - - ResolvableType aggregateReferenceWithIdTypeInteger = ResolvableType.forClassWithGenerics(AggregateReference.class, - String.class, Long.class); - Object converted = conversionService.convert(23, TypeDescriptor.forObject(23), - new TypeDescriptor(aggregateReferenceWithIdTypeInteger, null, null)); - - assertThat(converted).isEqualTo(AggregateReference.to(23L)); - } - - @Test // GH-992 - void convertsFromSimpleValueWithMissingTypeInformation() { - - Object converted = conversionService.convert(23, TypeDescriptor.forObject(23), - TypeDescriptor.valueOf(AggregateReference.class)); - - assertThat(converted).isEqualTo(AggregateReference.to(23)); - } - - @Test // GH-992 - void convertsToSimpleValue() { - - AggregateReference source = AggregateReference.to(23); - - Object converted = conversionService.convert(source, TypeDescriptor.forObject(source), - TypeDescriptor.valueOf(Integer.class)); - - assertThat(converted).isEqualTo(23); - } - - @Test // GH-992 - void convertsToSimpleValueThatNeedsSeparateConversion() { - - AggregateReference source = AggregateReference.to(23); - - Object converted = conversionService.convert(source, TypeDescriptor.forObject(source), - TypeDescriptor.valueOf(Long.class)); - - assertThat(converted).isEqualTo(23L); - } - -} From a891485e87703b692f024e068573e4ee73a3da9d Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 27 May 2025 14:28:21 +0200 Subject: [PATCH 4/5] Simplified structure of MappingRelationalConverter.readValue(). See #1828 Original pull request #2062 --- .../core/convert/MappingJdbcConverter.java | 44 +++++++++++++---- .../MappingRelationalConverter.java | 48 ++----------------- 2 files changed, 38 insertions(+), 54 deletions(-) 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 bffe0ddf33..069df8e697 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 @@ -27,10 +27,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.context.ApplicationContextAware; -import org.springframework.core.convert.ConversionException; -import org.springframework.core.convert.ConversionFailedException; import org.springframework.core.convert.converter.Converter; -import org.springframework.dao.DataAccessException; import org.springframework.dao.NonTransientDataAccessException; import org.springframework.data.convert.CustomConversions; import org.springframework.data.jdbc.core.mapping.AggregateReference; @@ -39,7 +36,6 @@ import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mapping.model.SimpleTypeHolder; import org.springframework.data.mapping.model.ValueExpressionEvaluator; -import org.springframework.data.relational.core.conversion.DbActionExecutionException; import org.springframework.data.relational.core.conversion.MappingRelationalConverter; import org.springframework.data.relational.core.conversion.ObjectPath; import org.springframework.data.relational.core.conversion.RelationalConverter; @@ -177,8 +173,33 @@ private Class doGetColumnType(RelationalPersistentProperty property) { return componentColumnType; } + /** + * Read and convert a single value that is coming from a database to the {@literal targetType} expected by the domain + * model. + * + * @param value a value as it is returned by the driver accessing the persistence store. May be {@code null}. + * @param targetType {@link TypeInformation} into which the value is to be converted. Must not be {@code null}. + * @return + */ @Override - protected Object readTechnologyType(Object value) { + @Nullable + public Object readValue(@Nullable Object value, TypeInformation targetType) { + + if (null == value) { + return null; + } + + TypeInformation originalTargetType = targetType; + value = readJdbcArray(value); + targetType = determineNestedTargetType(targetType); + + return readToAggregateReference(getPotentiallyConvertedSimpleRead(value, targetType), originalTargetType); + } + + /** + * Unwrap a Jdbc array, if such a value is provided + */ + private Object readJdbcArray(Object value) { if (value instanceof Array array) { try { @@ -191,8 +212,11 @@ protected Object readTechnologyType(Object value) { return value; } - @Override - protected TypeInformation determineModuleReadTarget(TypeInformation ultimateTargetType) { + /** + * 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 @@ -201,8 +225,10 @@ protected TypeInformation determineModuleReadTarget(TypeInformation ultima return ultimateTargetType; } - @Override - protected Object readModuleType(Object value, TypeInformation targetType) { + /** + * Convert value to an {@link AggregateReference} if that is specified by the parameter targetType. + */ + private Object readToAggregateReference(Object value, TypeInformation targetType) { if (AggregateReference.class.isAssignableFrom(targetType.getType())) { return AggregateReference.to(value); 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 51c36303fc..ebc3326d85 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 @@ -622,9 +622,9 @@ private boolean shouldReadEmbeddable(ConversionContext context, RelationalPersis } /** - * Read and convert a single value that is comming from a database to the {@literal targetType} expected by the domain + * Read and convert a single value that is coming from a database to the {@literal targetType} expected by the domain * model. - * + * * @param value a value as it is returned by the driver accessing the persistence store. May be {@code null}. * @param targetType {@link TypeInformation} into which the value is to be converted. Must not be {@code null}. * @return @@ -637,49 +637,7 @@ public Object readValue(@Nullable Object value, TypeInformation targetType) { return null; } - TypeInformation originalTargetType = targetType; - value = readTechnologyType(value); - targetType = determineModuleReadTarget(targetType); - - return readModuleType(getPotentiallyConvertedSimpleRead(value, targetType), originalTargetType); - } - - /** - * Convert a read value using module dependent special conversions. Spring Data JDBC for example uses this to - * implement the conversion of AggregateReferences. There is no guarantee that the value is converted to the exact - * required TypeInformation, nor that it is converted at all. - * - * @param value the value read from the database. Must not be {@literal null}. - * @param targetType the type to which the value should get converted if possible. Must not be {@literal null}. - * @return a potentially converted value. - */ - protected Object readModuleType(Object value, TypeInformation targetType) { - return value; - } - - /** - * Read technology specific values into objects that then can be fed in the normal conversion process for reading. An - * example are the conversion of JDBCs {@literal Array} type to normal java arrays. - * - * @param value a value read from the database - * @return a preprocessed value suitable for technology-agnostic further processing. - */ - protected Object readTechnologyType(Object value) { - return value; - } - - /** - * When type is a type that has special support, this returns the type a value should be converted to before the - * conversion to the special type happens. For example if type is AggregateReference this method returns the second - * parameter type of AggregateReference, in order to allow conversions to handle that type. - * - * @param ultimateTargetType ultimate target type to be returned by the conversion process. Must not be - * {@literal null}. - * @return a type that can be converted to the ultimate target type by module specific handling. Must not be - * {@literal null}. - */ - protected TypeInformation determineModuleReadTarget(TypeInformation ultimateTargetType) { - return ultimateTargetType; + return getPotentiallyConvertedSimpleRead(value, targetType); } /** From ef1a0cbcab548dc9d57957763f8839fc4203161a Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Thu, 3 Jul 2025 15:48:30 +0200 Subject: [PATCH 5/5] Fixed writing AggregateRef with custom write target. The TODO mentioned in review was already resolved. Added the proper conversion step during writing. Fixed the wrong assumption in the test, that the database will return a Long. Original pull request #2062 See #1828 --- .../core/convert/MappingJdbcConverter.java | 35 ++++++++++++------- ...oryCrossAggregateHsqlIntegrationTests.java | 11 +++--- .../MappingRelationalConverter.java | 13 +------ 3 files changed, 28 insertions(+), 31 deletions(-) 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 069df8e697..79f2cf5191 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 @@ -193,7 +193,7 @@ public Object readValue(@Nullable Object value, TypeInformation targetType) { value = readJdbcArray(value); targetType = determineNestedTargetType(targetType); - return readToAggregateReference(getPotentiallyConvertedSimpleRead(value, targetType), originalTargetType); + return possiblyReadToAggregateReference(getPotentiallyConvertedSimpleRead(value, targetType), originalTargetType); } /** @@ -228,7 +228,7 @@ private TypeInformation determineNestedTargetType(TypeInformation ultimate /** * Convert value to an {@link AggregateReference} if that is specified by the parameter targetType. */ - private Object readToAggregateReference(Object value, TypeInformation targetType) { + private Object possiblyReadToAggregateReference(Object value, TypeInformation targetType) { if (AggregateReference.class.isAssignableFrom(targetType.getType())) { return AggregateReference.to(value); @@ -277,28 +277,37 @@ private boolean canWriteAsJdbcValue(@Nullable Object value) { 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); + } + Object convertedValue = writeValue(value, targetType); if (convertedValue instanceof JdbcValue result) { return result; } - if (convertedValue == null || !convertedValue.getClass().isArray()) { - return JdbcValue.of(convertedValue, sqlType); + if (convertedValue == null) { + return JdbcValue.of(null, sqlType); } - Class componentType = convertedValue.getClass().getComponentType(); - if (componentType != byte.class && componentType != Byte.class) { + if (convertedValue.getClass().isArray()) {// array conversion + Class componentType = convertedValue.getClass().getComponentType(); + if (componentType != byte.class && componentType != Byte.class) { - Object[] objectArray = requireObjectArray(convertedValue); - return JdbcValue.of(typeFactory.createArray(objectArray), JDBCType.ARRAY); - } + Object[] objectArray = requireObjectArray(convertedValue); + return JdbcValue.of(typeFactory.createArray(objectArray), JDBCType.ARRAY); + } - if (componentType == Byte.class) { - convertedValue = ArrayUtils.toPrimitive((Byte[]) convertedValue); - } + if (componentType == Byte.class) { + convertedValue = ArrayUtils.toPrimitive((Byte[]) convertedValue); + } - return JdbcValue.of(convertedValue, JDBCType.BINARY); + return JdbcValue.of(convertedValue, JDBCType.BINARY); + } + + return JdbcValue.of(convertedValue, sqlType); } @SuppressWarnings("unchecked") 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 e975fa9465..384245c357 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 @@ -65,7 +65,7 @@ static class Config { @Bean JdbcCustomConversions jdbcCustomConversions() { - return new JdbcCustomConversions(asList(AggregateIdToLong.INSTANCE, LongToAggregateId.INSTANCE)); + return new JdbcCustomConversions(asList(AggregateIdToLong.INSTANCE, NumberToAggregateId.INSTANCE)); } } @@ -109,7 +109,6 @@ public void savesAndUpdate() { } @Test // GH-1828 - @Disabled public void savesAndReadWithConvertableId() { AggregateReference idReference = AggregateReference @@ -118,7 +117,7 @@ public void savesAndReadWithConvertableId() { .save(new ReferencingAggregate(null, "Reference", idReference)); ReferencingAggregate reloaded = referencingAggregates.findById(reference.id).get(); - assertThat(reloaded.id()).isEqualTo(idReference); + assertThat(reloaded.ref()).isEqualTo(idReference); } interface Ones extends CrudRepository {} @@ -163,12 +162,12 @@ public Long convert(AggregateId source) { } @ReadingConverter - enum LongToAggregateId implements Converter { + enum NumberToAggregateId implements Converter { INSTANCE; @Override - public AggregateId convert(Long source) { - return new AggregateId(source); + public AggregateId convert(Number source) { + return new AggregateId(source.longValue()); } } } 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 ebc3326d85..71947ae800 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,16 +44,7 @@ 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.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.mapping.model.*; import org.springframework.data.projection.EntityProjection; import org.springframework.data.projection.EntityProjectionIntrospector; import org.springframework.data.projection.EntityProjectionIntrospector.ProjectionPredicate; @@ -1174,8 +1165,6 @@ public Object getValue(AggregatePath path) { return null; } - // TODO: converting here seems wrong, since we have the ConvertingParameterValueProvider - // return context.convert(value, path.getRequiredLeafProperty().getTypeInformation()); return value; }