diff --git a/pom.xml b/pom.xml index ebd3103251..1458884755 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,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-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 7460931dab..8585e99fc5 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,8 @@ 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.dao.NonTransientDataAccessException; import org.springframework.data.convert.CustomConversions; import org.springframework.data.jdbc.core.mapping.AggregateReference; import org.springframework.data.jdbc.core.mapping.JdbcValue; @@ -80,7 +79,7 @@ public class MappingJdbcConverter extends MappingRelationalConverter implements * {@link #MappingJdbcConverter(RelationalMappingContext, RelationResolver, CustomConversions, JdbcTypeFactory)} * (MappingContext, RelationResolver, JdbcTypeFactory)} to convert arrays and large objects into JDBC-specific types. * - * @param context must not be {@literal null}. + * @param context must not be {@literal null}. * @param relationResolver used to fetch additional relations from the database. Must not be {@literal null}. */ public MappingJdbcConverter(RelationalMappingContext context, RelationResolver relationResolver) { @@ -91,19 +90,17 @@ public MappingJdbcConverter(RelationalMappingContext context, RelationResolver r this.typeFactory = JdbcTypeFactory.unsupported(); this.relationResolver = relationResolver; - - registerAggregateReferenceConverters(); } /** * Creates a new {@link MappingJdbcConverter} given {@link MappingContext}. * - * @param context must not be {@literal null}. + * @param context must not be {@literal null}. * @param relationResolver used to fetch additional relations from the database. Must not be {@literal null}. - * @param typeFactory must not be {@literal null} + * @param typeFactory must not be {@literal null} */ public MappingJdbcConverter(RelationalMappingContext context, RelationResolver relationResolver, - CustomConversions conversions, JdbcTypeFactory typeFactory) { + CustomConversions conversions, JdbcTypeFactory typeFactory) { super(context, conversions); @@ -112,14 +109,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 @@ -184,34 +173,78 @@ 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 @Nullable - public Object readValue(@Nullable Object value, TypeInformation type) { + public Object readValue(@Nullable Object value, TypeInformation targetType) { - if (value == null) { - return value; + 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 { - return super.readValue(array.getArray(), type); - } catch (SQLException | ConverterNotFoundException e) { - LOG.info("Failed to extract a value of type %s from an Array; Attempting to use standard conversions", e); + return array.getArray(); + } catch (SQLException e) { + throw new FailedToAccessJdbcArrayException(e); } } - return super.readValue(value, type); + 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 readToAggregateReference(Object value, TypeInformation targetType) { + + if (AggregateReference.class.isAssignableFrom(targetType.getType())) { + return AggregateReference.to(value); + } + return value; } - @Override @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) { @@ -285,7 +318,7 @@ public R readAndResolve(TypeInformation type, RowDocument source, Identif @Override protected RelationalPropertyValueProvider newValueProvider(RowDocumentAccessor documentAccessor, - ValueExpressionEvaluator evaluator, ConversionContext context) { + ValueExpressionEvaluator evaluator, ConversionContext context) { if (context instanceof ResolvingConversionContext rcc) { @@ -298,6 +331,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 @@ -314,7 +353,7 @@ class ResolvingRelationalPropertyValueProvider implements RelationalPropertyValu private final Identifier identifier; private ResolvingRelationalPropertyValueProvider(AggregatePathValueProvider delegate, RowDocumentAccessor accessor, - ResolvingConversionContext context, Identifier identifier) { + ResolvingConversionContext context, Identifier identifier) { AggregatePath path = context.aggregatePath(); @@ -323,7 +362,7 @@ private ResolvingRelationalPropertyValueProvider(AggregatePathValueProvider dele this.context = context; this.identifier = path.isEntity() ? potentiallyAppendIdentifier(identifier, path.getRequiredLeafEntity(), - property -> delegate.getValue(path.append(property))) + property -> delegate.getValue(path.append(property))) : identifier; } @@ -331,7 +370,7 @@ private ResolvingRelationalPropertyValueProvider(AggregatePathValueProvider dele * Conditionally append the identifier if the entity has an identifier property. */ static Identifier potentiallyAppendIdentifier(Identifier base, RelationalPersistentEntity entity, - Function getter) { + Function getter) { if (entity.hasIdProperty()) { @@ -460,7 +499,7 @@ public RelationalPropertyValueProvider withContext(ConversionContext context) { return context == this.context ? this : new ResolvingRelationalPropertyValueProvider(delegate.withContext(context), accessor, - (ResolvingConversionContext) context, identifier); + (ResolvingConversionContext) context, identifier); } } @@ -472,7 +511,7 @@ public RelationalPropertyValueProvider withContext(ConversionContext context) { * @param identifier */ private record ResolvingConversionContext(ConversionContext delegate, AggregatePath aggregatePath, - Identifier identifier) implements ConversionContext { + Identifier identifier) implements ConversionContext { @Override public S convert(Object source, TypeInformation typeHint) { 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); - } - -} 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 b623b6ef94..dfecac2fb5 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/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-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/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 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 395c64e677..4e8fb44224 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 @@ -559,7 +559,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); } } @@ -606,34 +608,23 @@ private boolean shouldReadEmbeddable(ConversionContext context, RelationalPersis return false; } - @Override - @Nullable - public Object readValue(@Nullable Object value, TypeInformation type) { - - if (null == value) { - return null; - } - - return getPotentiallyConvertedSimpleRead(value, type); - } - /** - * 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. + * Read and convert a single value that is coming from a database to the {@literal targetType} expected by the domain + * model. * - * @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}. + * @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 - private Object getPotentiallyConvertedSimpleWrite(Object value) { - - Optional> customTarget = getConversions().getCustomWriteTarget(value.getClass()); + public Object readValue(@Nullable Object value, TypeInformation targetType) { - if (customTarget.isPresent()) { - return getConversionService().convert(value, customTarget.get()); + if (null == value) { + return null; } - return Enum.class.isAssignableFrom(value.getClass()) ? ((Enum) value).name() : value; + return getPotentiallyConvertedSimpleRead(value, targetType); } /** @@ -683,33 +674,28 @@ public Object writeValue(@Nullable Object value, TypeInformation type) { return null; } - if (getConversions().isSimpleType(value.getClass())) { - - Optional> customWriteTarget = getConversions().hasCustomWriteTarget(value.getClass(), type.getType()) - ? getConversions().getCustomWriteTarget(value.getClass(), type.getType()) - : getConversions().getCustomWriteTarget(type.getType()); + // custom conversion + Optional> customWriteTarget = determinCustomWriteTarget(value, type); - 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()) { @@ -731,9 +717,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) { @@ -1174,7 +1161,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