From 4f4014dc8e01da9d3b76374a0a32f7f8e62e2cfb Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 15 Jul 2021 11:47:04 +0200 Subject: [PATCH] Polishing. Hide actual converters by making these private classes, expose factory method to construct converters. Avoid stream API usage on converter code paths. Remove unused imports. Clean up tests. Original pull request: #993. See #992 --- .../convert/AggregateReferenceConverters.java | 30 ++++++++--- .../jdbc/core/convert/BasicJdbcConverter.java | 14 ++++-- .../core/convert/JdbcCustomConversions.java | 7 +-- ...AggregateReferenceConvertersUnitTests.java | 50 ++++++++++++------- .../conversion/BasicRelationalConverter.java | 2 - 5 files changed, 67 insertions(+), 36 deletions(-) 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 index f8d7ea0b5..f3d917292 100644 --- 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 @@ -15,6 +15,8 @@ */ package org.springframework.data.jdbc.core.convert; +import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.Set; @@ -32,20 +34,31 @@ import org.springframework.lang.Nullable; * content of the {@link AggregateReference}. * * @author Jens Schauder - * @since 2.6 + * @author Mark Paluch + * @since 2.3 */ class AggregateReferenceConverters { + /** - * Prevent instantiation. + * Returns the converters to be registered. + * + * @return a collection of converters. Guaranteed to be not {@literal null}. */ - private AggregateReferenceConverters() {} + 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 - static class AggregateReferenceToSimpleTypeConverter implements GenericConverter { + private static class AggregateReferenceToSimpleTypeConverter implements GenericConverter { + + private static final Set CONVERTIBLE_TYPES = Collections + .singleton(new ConvertiblePair(AggregateReference.class, Object.class)); private final ConversionService delegate; @@ -55,7 +68,7 @@ class AggregateReferenceConverters { @Override public Set getConvertibleTypes() { - return Collections.singleton(new ConvertiblePair(AggregateReference.class, Object.class)); + return CONVERTIBLE_TYPES; } @Override @@ -90,7 +103,10 @@ class AggregateReferenceConverters { * {@link ConversionService}. */ @ReadingConverter - static class SimpleTypeToAggregateReferenceConverter implements GenericConverter { + private static class SimpleTypeToAggregateReferenceConverter implements GenericConverter { + + private static final Set CONVERTIBLE_TYPES = Collections + .singleton(new ConvertiblePair(Object.class, AggregateReference.class)); private final ConversionService delegate; @@ -100,7 +116,7 @@ class AggregateReferenceConverters { @Override public Set getConvertibleTypes() { - return Collections.singleton(new ConvertiblePair(Object.class, AggregateReference.class)); + return CONVERTIBLE_TYPES; } @Override diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java index 4a2a13945..2ac78871a 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverter.java @@ -19,11 +19,13 @@ import java.sql.Array; import java.sql.JDBCType; import java.sql.ResultSet; import java.sql.SQLException; +import java.util.List; import java.util.Map; import java.util.Optional; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.core.ResolvableType; @@ -224,7 +226,7 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc if (getConversions().hasCustomReadTarget(value.getClass(), type.getType())) { TypeDescriptor sourceDescriptor = TypeDescriptor.valueOf(value.getClass()); - TypeDescriptor targetDescriptor = typeInformationToTypeDescriptor(type); + TypeDescriptor targetDescriptor = createTypeDescriptor(type); return getConversionService().convert(value, sourceDescriptor, targetDescriptor); @@ -241,11 +243,15 @@ public class BasicJdbcConverter extends BasicRelationalConverter implements Jdbc return super.readValue(value, type); } - private static TypeDescriptor typeInformationToTypeDescriptor(TypeInformation type) { + private static TypeDescriptor createTypeDescriptor(TypeInformation type) { - Class[] generics = type.getTypeArguments().stream().map(TypeInformation::getType).toArray(Class[]::new); + List> typeArguments = type.getTypeArguments(); + Class[] generics = new Class[typeArguments.size()]; + for (int i = 0; i < typeArguments.size(); i++) { + generics[i] = typeArguments.get(i).getType(); + } - return new TypeDescriptor(ResolvableType.forClassWithGenerics(type.getType(), generics), null, null); + return new TypeDescriptor(ResolvableType.forClassWithGenerics(type.getType(), generics), type.getType(), null); } /* diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcCustomConversions.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcCustomConversions.java index a0a6809c1..9107db8c5 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcCustomConversions.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcCustomConversions.java @@ -20,8 +20,6 @@ import java.util.Collection; 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; @@ -46,9 +44,8 @@ public class JdbcCustomConversions extends CustomConversions { List converters = new ArrayList<>(Jsr310TimestampBasedConverters.getConvertersToRegister()); - ConversionService conversionService = DefaultConversionService.getSharedInstance(); - converters.add(new AggregateReferenceConverters.AggregateReferenceToSimpleTypeConverter(conversionService)); - converters.add(new AggregateReferenceConverters.SimpleTypeToAggregateReferenceConverter(conversionService)); + converters + .addAll(AggregateReferenceConverters.getConvertersToRegister(DefaultConversionService.getSharedInstance())); STORE_CONVERTERS = Collections.unmodifiableCollection(converters); 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 index a585f2eef..9bfd9a757 100644 --- 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 @@ -15,69 +15,83 @@ */ 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; -import static org.assertj.core.api.Assertions.*; - /** * Tests for converters from an to {@link org.springframework.data.jdbc.core.mapping.AggregateReference}. * * @author Jens Schauder + * @author Mark Paluch */ class AggregateReferenceConvertersUnitTests { - AggregateReferenceConverters.SimpleTypeToAggregateReferenceConverter simpleToAggregate = new AggregateReferenceConverters.SimpleTypeToAggregateReferenceConverter(DefaultConversionService.getSharedInstance()); - AggregateReferenceConverters.AggregateReferenceToSimpleTypeConverter aggregateToSimple = new AggregateReferenceConverters.AggregateReferenceToSimpleTypeConverter(DefaultConversionService.getSharedInstance()); + ConfigurableConversionService conversionService; + + @BeforeEach + void setUp() { + conversionService = new DefaultConversionService(); + AggregateReferenceConverters.getConvertersToRegister(DefaultConversionService.getSharedInstance()) + .forEach(it -> conversionService.addConverter(it)); + } - @Test // #992 + @Test // GH-992 void convertsFromSimpleValue() { ResolvableType aggregateReferenceWithIdTypeInteger = ResolvableType.forClassWithGenerics(AggregateReference.class, String.class, Integer.class); - final Object converted = simpleToAggregate.convert(23, TypeDescriptor.forObject(23), new TypeDescriptor(aggregateReferenceWithIdTypeInteger, null, null)); + Object converted = conversionService.convert(23, TypeDescriptor.forObject(23), + new TypeDescriptor(aggregateReferenceWithIdTypeInteger, null, null)); assertThat(converted).isEqualTo(AggregateReference.to(23)); } - @Test // #992 + @Test // GH-992 void convertsFromSimpleValueThatNeedsSeparateConversion() { ResolvableType aggregateReferenceWithIdTypeInteger = ResolvableType.forClassWithGenerics(AggregateReference.class, String.class, Long.class); - final Object converted = simpleToAggregate.convert(23, TypeDescriptor.forObject(23), new TypeDescriptor(aggregateReferenceWithIdTypeInteger, null, null)); + Object converted = conversionService.convert(23, TypeDescriptor.forObject(23), + new TypeDescriptor(aggregateReferenceWithIdTypeInteger, null, null)); assertThat(converted).isEqualTo(AggregateReference.to(23L)); } - @Test // #992 + @Test // GH-992 void convertsFromSimpleValueWithMissingTypeInformation() { - final Object converted = simpleToAggregate.convert(23, TypeDescriptor.forObject(23), TypeDescriptor.valueOf(AggregateReference.class)); + Object converted = conversionService.convert(23, TypeDescriptor.forObject(23), + TypeDescriptor.valueOf(AggregateReference.class)); assertThat(converted).isEqualTo(AggregateReference.to(23)); } - @Test // #992 + @Test // GH-992 void convertsToSimpleValue() { - final AggregateReference source = AggregateReference.to(23); + AggregateReference source = AggregateReference.to(23); - final Object converted = aggregateToSimple.convert(source, TypeDescriptor.forObject(source), TypeDescriptor.valueOf(Integer.class)); + Object converted = conversionService.convert(source, TypeDescriptor.forObject(source), + TypeDescriptor.valueOf(Integer.class)); assertThat(converted).isEqualTo(23); } - @Test // #992 + @Test // GH-992 void convertsToSimpleValueThatNeedsSeparateConversion() { - final AggregateReference source = AggregateReference.to(23); + AggregateReference source = AggregateReference.to(23); - final Object converted = aggregateToSimple.convert(source, TypeDescriptor.forObject(source), TypeDescriptor.valueOf(Long.class)); + Object converted = conversionService.convert(source, TypeDescriptor.forObject(source), + TypeDescriptor.valueOf(Long.class)); assertThat(converted).isEqualTo(23L); } - -} \ No newline at end of file +} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/BasicRelationalConverter.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/BasicRelationalConverter.java index d83bad48d..fad388e71 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/BasicRelationalConverter.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/BasicRelationalConverter.java @@ -19,9 +19,7 @@ import java.util.Collections; import java.util.Optional; import java.util.function.Function; -import org.springframework.core.ResolvableType; import org.springframework.core.convert.ConversionService; -import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.support.ConfigurableConversionService; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.data.convert.CustomConversions;