Reuse custom converters from AggregateReferenceConverters.

Let the AggregateReference converters now receive also custom converters as delegates.
Remove the ArrayToObjectConverter which just uses the first element of the array from the DefaultConversion service.

This does NOT solve the underlying problem, of having two DefaultConversionServices at work.
One is in the store conversion, one constructed in the AbstractRelationalConverter.
Although it looks like they get merged, the former contains converters, using that ConversionService as a delegate.

Attempts were made to move AggregateReferenceConverters out of the store converters and just register them directly,
but then no custom read/write targets are detected, leading to failed conversions.
The same problem prevents a registration in CustomConversions as a Function<ConversionService, GenericConverter> or similar, which would allow late registration.
The problem here is that custom read/write targets require the function to get evaluated, before custom read/write targets can be determined.

Closes #1750
Original pull request: #1785

Author: schauder

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcCustomConversions.java

@@ -20,7 +20,7 @@
 import java.util.Collections;
 import java.util.List;
 
-import org.springframework.core.convert.ConversionService;
+import org.springframework.core.convert.converter.Converter;
 import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair;
 import org.springframework.core.convert.support.DefaultConversionService;
 import org.springframework.data.convert.CustomConversions;
@@ -52,9 +52,6 @@ public class JdbcCustomConversions extends CustomConversions {
 
 	}
 
-	private static final StoreConversions STORE_CONVERSIONS = StoreConversions.of(JdbcSimpleTypes.HOLDER,
-			STORE_CONVERTERS);
-
 	/**
 	 * Creates an empty {@link JdbcCustomConversions} object.
 	 */
@@ -70,11 +67,7 @@ public JdbcCustomConversions() {
 	 */
 	public JdbcCustomConversions(List<?> converters) {
 
-		super(new ConverterConfiguration( //
-				STORE_CONVERSIONS, //
-				converters, //
-				JdbcCustomConversions::excludeConversionsBetweenDateAndJsr310Types //
-		));
+		super(constructConverterConfiguration(converters));
 	}
 
 	/**
@@ -103,6 +96,32 @@ public JdbcCustomConversions(ConverterConfiguration converterConfiguration) {
 		super(converterConfiguration);
 	}
 
+	private static ConverterConfiguration constructConverterConfiguration(List<?> converters) {
+
+		StoreConversions storeConversions = storeConversions(converters);
+
+		return new ConverterConfiguration( //
+				storeConversions, //
+				converters, //
+				JdbcCustomConversions::excludeConversionsBetweenDateAndJsr310Types //
+		);
+	}
+
+	private static StoreConversions storeConversions(List<?> userConverters) {
+
+		List<Object> converters = new ArrayList<>(Jsr310TimestampBasedConverters.getConvertersToRegister());
+
+		DefaultConversionService defaultConversionService = new DefaultConversionService();
+		for (Object userConverter : userConverters) {
+			if (userConverter instanceof Converter<?, ?> converter)
+				defaultConversionService.addConverter(converter);
+		}
+
+		converters.addAll(AggregateReferenceConverters.getConvertersToRegister(defaultConversionService));
+
+		return StoreConversions.of(JdbcSimpleTypes.HOLDER, Collections.unmodifiableCollection(converters));
+	}
+
 	/**
 	 * Obtain a read only copy of default store converters.
 	 *

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/MappingJdbcConverterUnitTests.java

@@ -19,6 +19,7 @@
 import static org.assertj.core.api.SoftAssertions.*;
 import static org.mockito.Mockito.*;
 
+import java.nio.ByteBuffer;
 import java.sql.Array;
 import java.sql.Timestamp;
 import java.time.Instant;
@@ -28,13 +29,16 @@
 import java.time.OffsetDateTime;
 import java.time.ZoneOffset;
 import java.time.ZonedDateTime;
+import java.util.Collections;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 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.annotation.Id;
 import org.springframework.data.jdbc.core.mapping.AggregateReference;
 import org.springframework.data.jdbc.core.mapping.JdbcMappingContext;
@@ -50,9 +54,14 @@
  * Unit tests for {@link MappingJdbcConverter}.
  *
  * @author Mark Paluch
+ * @author Jens Schauder
  */
 public class MappingJdbcConverterUnitTests {
 
+	public static final UUID UUID = java.util.UUID.fromString("87a48aa8-a071-705e-54a9-e52fe3a012f1");
+	public static final byte[] BYTES_REPRESENTING_UUID = { -121, -92, -118, -88, -96, 113, 112, 94, 84, -87, -27, 47, -29,
+			-96, 18, -15 };
+
 	JdbcMappingContext context = new JdbcMappingContext();
 	StubbedJdbcTypeFactory typeFactory = new StubbedJdbcTypeFactory();
 	MappingJdbcConverter converter = new MappingJdbcConverter( //
@@ -61,7 +70,7 @@ public class MappingJdbcConverterUnitTests {
 				throw new UnsupportedOperationException();
 			}, //
 			new JdbcCustomConversions(), //
-			typeFactory  //
+			typeFactory //
 	);
 
 	@Test // DATAJDBC-104, DATAJDBC-1384
@@ -152,6 +161,39 @@ void accessesCorrectValuesForOneToOneRelationshipWithIdenticallyNamedIdPropertie
 		assertThat(result).isEqualTo(new WithOneToOne("one", new Referenced(23L)));
 	}
 
+	@Test // GH-1750
+	void readByteArrayToNestedUuidWithCustomConverter() {
+
+		JdbcMappingContext context = new JdbcMappingContext();
+		StubbedJdbcTypeFactory typeFactory = new StubbedJdbcTypeFactory();
+		Converter<byte[], UUID> customConverter = new ByteArrayToUuid();
+		MappingJdbcConverter converter = new MappingJdbcConverter( //
+				context, //
+				(identifier, path) -> {
+					throw new UnsupportedOperationException();
+				}, //
+				new JdbcCustomConversions(Collections.singletonList(customConverter)), //
+				typeFactory //
+		);
+
+		SoftAssertions.assertSoftly(softly -> {
+			checkReadConversion(softly, converter, "uuidRef", AggregateReference.to(UUID));
+			checkReadConversion(softly, converter, "uuid", UUID);
+			checkReadConversion(softly, converter, "optionalUuid", Optional.of(UUID));
+		});
+
+	}
+
+	private static void checkReadConversion(SoftAssertions softly, MappingJdbcConverter converter, String propertyName,
+			Object expected) {
+
+		RelationalPersistentProperty property = converter.getMappingContext().getRequiredPersistentEntity(DummyEntity.class)
+				.getRequiredPersistentProperty(propertyName);
+		Object value = converter.readValue(BYTES_REPRESENTING_UUID, property.getTypeInformation() //
+		);
+
+		softly.assertThat(value).isEqualTo(expected);
+	}
 
 	private void checkConversionToTimestampAndBack(SoftAssertions softly, RelationalPersistentEntity<?> persistentEntity,
 			String propertyName, Object value) {
@@ -187,6 +229,8 @@ private static class DummyEntity {
 		private final Timestamp timestamp;
 		private final AggregateReference<DummyEntity, Long> reference;
 		private final UUID uuid;
+		private final AggregateReference<ReferencedByUuid, UUID> uuidRef;
+		private final Optional<UUID> optionalUuid;
 
 		// DATAJDBC-259
 		private final List<String> listOfString;
@@ -195,9 +239,10 @@ private static class DummyEntity {
 		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<DummyEntity, Long> reference, UUID uuid, List<String> listOfString,
-				String[] arrayOfString, List<OtherEntity> listOfEntity, OtherEntity[] arrayOfEntity) {
+							LocalTime localTime, ZonedDateTime zonedDateTime, OffsetDateTime offsetDateTime, Instant instant, Date date,
+							Timestamp timestamp, AggregateReference<DummyEntity, Long> reference, UUID uuid,
+							AggregateReference<ReferencedByUuid, UUID> uuidRef, Optional<java.util.UUID> optionalUUID, List<String> listOfString, String[] arrayOfString,
+							List<OtherEntity> listOfEntity, OtherEntity[] arrayOfEntity) {
 			this.id = id;
 			this.someEnum = someEnum;
 			this.localDateTime = localDateTime;
@@ -210,6 +255,8 @@ private DummyEntity(Long id, SomeEnum someEnum, LocalDateTime localDateTime, Loc
 			this.timestamp = timestamp;
 			this.reference = reference;
 			this.uuid = uuid;
+			this.uuidRef = uuidRef;
+			this.optionalUuid = optionalUUID;
 			this.listOfString = listOfString;
 			this.arrayOfString = arrayOfString;
 			this.listOfEntity = listOfEntity;
@@ -299,9 +346,23 @@ public Array createArray(Object[] value) {
 		}
 	}
 
-	record WithOneToOne(@Id String id,@MappedCollection(idColumn = "renamed") Referenced referenced){}
+	record WithOneToOne(@Id String id, @MappedCollection(idColumn = "renamed") Referenced referenced) {
+	}
 
 	record Referenced(@Id Long id) {
 	}
 
+	record ReferencedByUuid(@Id UUID id) {
+	}
+
+	class ByteArrayToUuid implements Converter<byte[], UUID> {
+		@Override
+		public UUID convert(byte[] source) {
+
+			ByteBuffer byteBuffer = ByteBuffer.wrap(source);
+			long high = byteBuffer.getLong();
+			long low = byteBuffer.getLong();
+			return new UUID(high, low);
+		}
+	}
 }

spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/AbstractRelationalConverter.java

@@ -17,6 +17,7 @@
 
 import java.util.Collections;
 
+import org.jetbrains.annotations.NotNull;
 import org.springframework.core.convert.ConversionService;
 import org.springframework.core.convert.support.ConfigurableConversionService;
 import org.springframework.core.convert.support.DefaultConversionService;
@@ -47,7 +48,7 @@ public abstract class AbstractRelationalConverter implements RelationalConverter
 	 * @param context must not be {@literal null}.
 	 */
 	public AbstractRelationalConverter(RelationalMappingContext context) {
-		this(context, new CustomConversions(StoreConversions.NONE, Collections.emptyList()), new DefaultConversionService(),
+		this(context, new CustomConversions(StoreConversions.NONE, Collections.emptyList()), createBaseConversionService(),
 				new EntityInstantiators());
 	}
 
@@ -58,7 +59,7 @@ public AbstractRelationalConverter(RelationalMappingContext context) {
 	 * @param conversions must not be {@literal null}.
 	 */
 	public AbstractRelationalConverter(RelationalMappingContext context, CustomConversions conversions) {
-		this(context, conversions, new DefaultConversionService(), new EntityInstantiators());
+		this(context, conversions, createBaseConversionService(), new EntityInstantiators());
 	}
 
 	private AbstractRelationalConverter(RelationalMappingContext context, CustomConversions conversions,
@@ -75,6 +76,14 @@ private AbstractRelationalConverter(RelationalMappingContext context, CustomConv
 		conversions.registerConvertersIn(this.conversionService);
 	}
 
+	@NotNull
+	private static DefaultConversionService createBaseConversionService() {
+
+		DefaultConversionService conversionService = new DefaultConversionService();
+		conversionService.removeConvertible(Object[].class, Object.class);
+		return conversionService;
+	}
+
 	@Override
 	public ConversionService getConversionService() {
 		return conversionService;

spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/MappingRelationalConverter.java

@@ -624,14 +624,6 @@ public Object readValue(@Nullable Object value, TypeInformation<?> type) {
 			return null;
 		}
 
-		if (getConversions().hasCustomReadTarget(value.getClass(), type.getType())) {
-
-			TypeDescriptor sourceDescriptor = TypeDescriptor.valueOf(value.getClass());
-			TypeDescriptor targetDescriptor = createTypeDescriptor(type);
-
-			return getConversionService().convert(value, sourceDescriptor, targetDescriptor);
-		}
-
 		return getPotentiallyConvertedSimpleRead(value, type);
 	}