Skip to content

Commit 6cba7ca

Browse files
schauderschauder
committed
Fix problems with reading converters.
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
1 parent d56a99a commit 6cba7ca

File tree

3 files changed

+103
-15
lines changed

3 files changed

+103
-15
lines changed

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

+26-8
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import java.util.Collections;
2121
import java.util.List;
2222

23-
import org.springframework.core.convert.ConversionService;
23+
import org.springframework.core.convert.converter.Converter;
2424
import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair;
2525
import org.springframework.core.convert.support.DefaultConversionService;
2626
import org.springframework.data.convert.CustomConversions;
@@ -44,10 +44,6 @@ public class JdbcCustomConversions extends CustomConversions {
4444
static {
4545

4646
List<Object> converters = new ArrayList<>(Jsr310TimestampBasedConverters.getConvertersToRegister());
47-
48-
converters
49-
.addAll(AggregateReferenceConverters.getConvertersToRegister(DefaultConversionService.getSharedInstance()));
50-
5147
STORE_CONVERTERS = Collections.unmodifiableCollection(converters);
5248

5349
}
@@ -70,11 +66,33 @@ public JdbcCustomConversions() {
7066
*/
7167
public JdbcCustomConversions(List<?> converters) {
7268

73-
super(new ConverterConfiguration( //
74-
STORE_CONVERSIONS, //
69+
super(constructConverterConfiguration(converters));
70+
}
71+
72+
private static ConverterConfiguration constructConverterConfiguration(List<?> converters) {
73+
74+
StoreConversions storeConversions = storeConversions(converters);
75+
76+
return new ConverterConfiguration( //
77+
storeConversions, //
7578
converters, //
7679
JdbcCustomConversions::excludeConversionsBetweenDateAndJsr310Types //
77-
));
80+
);
81+
}
82+
83+
private static StoreConversions storeConversions(List<?> userConverters) {
84+
85+
List<Object> converters = new ArrayList<>(Jsr310TimestampBasedConverters.getConvertersToRegister());
86+
87+
DefaultConversionService defaultConversionService = new DefaultConversionService();
88+
for (Object userConverter : userConverters) {
89+
if (userConverter instanceof Converter<?, ?> converter)
90+
defaultConversionService.addConverter(converter);
91+
}
92+
93+
converters.addAll(AggregateReferenceConverters.getConvertersToRegister(defaultConversionService));
94+
95+
return StoreConversions.of(JdbcSimpleTypes.HOLDER, Collections.unmodifiableCollection(converters));
7896
}
7997

8098
/**

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

+66-5
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.assertj.core.api.SoftAssertions.*;
2020
import static org.mockito.Mockito.*;
2121

22+
import java.nio.ByteBuffer;
2223
import java.sql.Array;
2324
import java.sql.Timestamp;
2425
import java.time.Instant;
@@ -28,13 +29,16 @@
2829
import java.time.OffsetDateTime;
2930
import java.time.ZoneOffset;
3031
import java.time.ZonedDateTime;
32+
import java.util.Collections;
3133
import java.util.Date;
3234
import java.util.List;
3335
import java.util.Map;
36+
import java.util.Optional;
3437
import java.util.UUID;
3538

3639
import org.assertj.core.api.SoftAssertions;
3740
import org.junit.jupiter.api.Test;
41+
import org.springframework.core.convert.converter.Converter;
3842
import org.springframework.data.annotation.Id;
3943
import org.springframework.data.jdbc.core.mapping.AggregateReference;
4044
import org.springframework.data.jdbc.core.mapping.JdbcMappingContext;
@@ -50,9 +54,14 @@
5054
* Unit tests for {@link MappingJdbcConverter}.
5155
*
5256
* @author Mark Paluch
57+
* @author Jens Schauder
5358
*/
5459
public class MappingJdbcConverterUnitTests {
5560

61+
public static final UUID UUID = java.util.UUID.fromString("87a48aa8-a071-705e-54a9-e52fe3a012f1");
62+
public static final byte[] BYTES_REPRESENTING_UUID = { -121, -92, -118, -88, -96, 113, 112, 94, 84, -87, -27, 47, -29,
63+
-96, 18, -15 };
64+
5665
JdbcMappingContext context = new JdbcMappingContext();
5766
StubbedJdbcTypeFactory typeFactory = new StubbedJdbcTypeFactory();
5867
MappingJdbcConverter converter = new MappingJdbcConverter( //
@@ -61,7 +70,7 @@ public class MappingJdbcConverterUnitTests {
6170
throw new UnsupportedOperationException();
6271
}, //
6372
new JdbcCustomConversions(), //
64-
typeFactory //
73+
typeFactory //
6574
);
6675

6776
@Test // DATAJDBC-104, DATAJDBC-1384
@@ -152,6 +161,39 @@ void accessesCorrectValuesForOneToOneRelationshipWithIdenticallyNamedIdPropertie
152161
assertThat(result).isEqualTo(new WithOneToOne("one", new Referenced(23L)));
153162
}
154163

164+
@Test // GH-1750
165+
void readByteArrayToNestedUuidWithCustomConverter() {
166+
167+
JdbcMappingContext context = new JdbcMappingContext();
168+
StubbedJdbcTypeFactory typeFactory = new StubbedJdbcTypeFactory();
169+
Converter<byte[], UUID> customConverter = new ByteArrayToUuid();
170+
MappingJdbcConverter converter = new MappingJdbcConverter( //
171+
context, //
172+
(identifier, path) -> {
173+
throw new UnsupportedOperationException();
174+
}, //
175+
new JdbcCustomConversions(Collections.singletonList(customConverter)), //
176+
typeFactory //
177+
);
178+
179+
SoftAssertions.assertSoftly(softly -> {
180+
checkReadConversion(softly, converter, "uuidRef", AggregateReference.to(UUID));
181+
checkReadConversion(softly, converter, "uuid", UUID);
182+
checkReadConversion(softly, converter, "optionalUuid", Optional.of(UUID));
183+
});
184+
185+
}
186+
187+
private static void checkReadConversion(SoftAssertions softly, MappingJdbcConverter converter, String propertyName,
188+
Object expected) {
189+
190+
RelationalPersistentProperty property = converter.getMappingContext().getRequiredPersistentEntity(DummyEntity.class)
191+
.getRequiredPersistentProperty(propertyName);
192+
Object value = converter.readValue(BYTES_REPRESENTING_UUID, property.getTypeInformation() //
193+
);
194+
195+
softly.assertThat(value).isEqualTo(expected);
196+
}
155197

156198
private void checkConversionToTimestampAndBack(SoftAssertions softly, RelationalPersistentEntity<?> persistentEntity,
157199
String propertyName, Object value) {
@@ -187,6 +229,8 @@ private static class DummyEntity {
187229
private final Timestamp timestamp;
188230
private final AggregateReference<DummyEntity, Long> reference;
189231
private final UUID uuid;
232+
private final AggregateReference<ReferencedByUuid, UUID> uuidRef;
233+
private final Optional<UUID> optionalUuid;
190234

191235
// DATAJDBC-259
192236
private final List<String> listOfString;
@@ -195,9 +239,10 @@ private static class DummyEntity {
195239
private final OtherEntity[] arrayOfEntity;
196240

197241
private DummyEntity(Long id, SomeEnum someEnum, LocalDateTime localDateTime, LocalDate localDate,
198-
LocalTime localTime, ZonedDateTime zonedDateTime, OffsetDateTime offsetDateTime, Instant instant, Date date,
199-
Timestamp timestamp, AggregateReference<DummyEntity, Long> reference, UUID uuid, List<String> listOfString,
200-
String[] arrayOfString, List<OtherEntity> listOfEntity, OtherEntity[] arrayOfEntity) {
242+
LocalTime localTime, ZonedDateTime zonedDateTime, OffsetDateTime offsetDateTime, Instant instant, Date date,
243+
Timestamp timestamp, AggregateReference<DummyEntity, Long> reference, UUID uuid,
244+
AggregateReference<ReferencedByUuid, UUID> uuidRef, Optional<java.util.UUID> optionalUUID, List<String> listOfString, String[] arrayOfString,
245+
List<OtherEntity> listOfEntity, OtherEntity[] arrayOfEntity) {
201246
this.id = id;
202247
this.someEnum = someEnum;
203248
this.localDateTime = localDateTime;
@@ -210,6 +255,8 @@ private DummyEntity(Long id, SomeEnum someEnum, LocalDateTime localDateTime, Loc
210255
this.timestamp = timestamp;
211256
this.reference = reference;
212257
this.uuid = uuid;
258+
this.uuidRef = uuidRef;
259+
this.optionalUuid = optionalUUID;
213260
this.listOfString = listOfString;
214261
this.arrayOfString = arrayOfString;
215262
this.listOfEntity = listOfEntity;
@@ -299,9 +346,23 @@ public Array createArray(Object[] value) {
299346
}
300347
}
301348

302-
record WithOneToOne(@Id String id,@MappedCollection(idColumn = "renamed") Referenced referenced){}
349+
record WithOneToOne(@Id String id, @MappedCollection(idColumn = "renamed") Referenced referenced) {
350+
}
303351

304352
record Referenced(@Id Long id) {
305353
}
306354

355+
record ReferencedByUuid(@Id UUID id) {
356+
}
357+
358+
class ByteArrayToUuid implements Converter<byte[], UUID> {
359+
@Override
360+
public UUID convert(byte[] source) {
361+
362+
ByteBuffer byteBuffer = ByteBuffer.wrap(source);
363+
long high = byteBuffer.getLong();
364+
long low = byteBuffer.getLong();
365+
return new UUID(high, low);
366+
}
367+
}
307368
}

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

+11-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import java.util.Collections;
1919

20+
import org.jetbrains.annotations.NotNull;
2021
import org.springframework.core.convert.ConversionService;
2122
import org.springframework.core.convert.support.ConfigurableConversionService;
2223
import org.springframework.core.convert.support.DefaultConversionService;
@@ -47,7 +48,7 @@ public abstract class AbstractRelationalConverter implements RelationalConverter
4748
* @param context must not be {@literal null}.
4849
*/
4950
public AbstractRelationalConverter(RelationalMappingContext context) {
50-
this(context, new CustomConversions(StoreConversions.NONE, Collections.emptyList()), new DefaultConversionService(),
51+
this(context, new CustomConversions(StoreConversions.NONE, Collections.emptyList()), createBaseConversionService(),
5152
new EntityInstantiators());
5253
}
5354

@@ -58,7 +59,7 @@ public AbstractRelationalConverter(RelationalMappingContext context) {
5859
* @param conversions must not be {@literal null}.
5960
*/
6061
public AbstractRelationalConverter(RelationalMappingContext context, CustomConversions conversions) {
61-
this(context, conversions, new DefaultConversionService(), new EntityInstantiators());
62+
this(context, conversions, createBaseConversionService(), new EntityInstantiators());
6263
}
6364

6465
private AbstractRelationalConverter(RelationalMappingContext context, CustomConversions conversions,
@@ -75,6 +76,14 @@ private AbstractRelationalConverter(RelationalMappingContext context, CustomConv
7576
conversions.registerConvertersIn(this.conversionService);
7677
}
7778

79+
@NotNull
80+
private static DefaultConversionService createBaseConversionService() {
81+
82+
DefaultConversionService conversionService = new DefaultConversionService();
83+
conversionService.removeConvertible(Object[].class, Object.class);
84+
return conversionService;
85+
}
86+
7887
@Override
7988
public ConversionService getConversionService() {
8089
return conversionService;

0 commit comments

Comments
 (0)