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

Author: schauder

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")

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<AggregateWithConvertableId, AggregateId> 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<AggregateOne, Long> {}
@@ -163,12 +162,12 @@ public Long convert(AggregateId source) {
 	}
 
 	@ReadingConverter
-	enum LongToAggregateId implements Converter<Long, AggregateId> {
+	enum NumberToAggregateId implements Converter<Number, AggregateId> {
 		INSTANCE;
 
 		@Override
-		public AggregateId convert(Long source) {
-			return new AggregateId(source);
+		public AggregateId convert(Number source) {
+			return new AggregateId(source.longValue());
 		}
 	}
 }

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;
 		}