From 6af485bf6b92b898b936f042609ecbefed21be08 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Thu, 21 Nov 2019 07:45:09 +0100 Subject: [PATCH 1/3] DATACMNS-1615 - Prepare issue branch. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 6a81501ac0..cfd2284848 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 2.3.0.BUILD-SNAPSHOT + 2.3.0.DATACMNS-1615-SNAPSHOT Spring Data Core From 580e55a1b66fed55d43bb4d35c2321bf3cc16b25 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 22 Nov 2019 09:25:43 +0100 Subject: [PATCH 2/3] DATACMNS-1615 - Allow fine grained store specific converter registration. CustomConversions now registers all given user defined converters and selects only those converters from the StoreConversions that convert to or from a store supported simple type. By doing so we make sure to only register supported converters which removes warning messages from the log and reduces the number of converters within the ConversionService. --- .../data/convert/CustomConversions.java | 190 ++++++++++++++++-- .../convert/CustomConversionsUnitTests.java | 28 ++- 2 files changed, 196 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/springframework/data/convert/CustomConversions.java b/src/main/java/org/springframework/data/convert/CustomConversions.java index a52e0a249a..92d2d6b879 100644 --- a/src/main/java/org/springframework/data/convert/CustomConversions.java +++ b/src/main/java/org/springframework/data/convert/CustomConversions.java @@ -25,6 +25,7 @@ import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; +import java.util.stream.Collectors; import org.springframework.core.GenericTypeResolver; import org.springframework.core.convert.converter.Converter; @@ -57,6 +58,10 @@ public class CustomConversions { private static final String READ_CONVERTER_NOT_SIMPLE = "Registering converter from %s to %s as reading converter although it doesn't convert from a store-supported type! You might wanna check you annotation setup at the converter implementation."; private static final String WRITE_CONVERTER_NOT_SIMPLE = "Registering converter from %s to %s as writing converter although it doesn't convert to a store-supported type! You might wanna check you annotation setup at the converter implementation."; private static final String NOT_A_CONVERTER = "Converter %s is neither a Spring Converter, GenericConverter or ConverterFactory!"; + private static final String CONVERTER_FILTER = "converter from %s to %s as %s converter."; + private static final String ADD_CONVERTER = "Adding %s" + CONVERTER_FILTER; + private static final String SKIP_CONVERTER = "Skipping " + CONVERTER_FILTER + + "%s is not a store supported simple type!"; private static final List DEFAULT_CONVERTERS; static { @@ -89,7 +94,10 @@ public class CustomConversions { convertiblePair.getSourceType(), null, writingPairs); /** - * Creates a new {@link CustomConversions} instance registering the given converters. + * Creates a new {@link CustomConversions} instance registering all given user defined converters and selecting + * {@link Converter converters} from {@link StoreConversions} depending on + * {@link StoreConversions#getSimpleTypeHolder() store simple types} only considering those that either convert + * to/from a store supported type. * * @param storeConversions must not be {@literal null}. * @param converters must not be {@literal null}. @@ -99,21 +107,47 @@ public CustomConversions(StoreConversions storeConversions, Collection conver Assert.notNull(storeConversions, "StoreConversions must not be null!"); Assert.notNull(converters, "List of converters must not be null!"); - List toRegister = new ArrayList<>(); + List registeredConverters = collectPotentialConverterRegistrations(storeConversions, converters).stream() // + .filter(this::isSupportedConverter) // + .map(ConverterRegistrationIntent::getConverterRegistration) // + .map(this::register) // + .distinct() // + .collect(Collectors.toList()); - // Add user provided converters to make sure they can override the defaults - toRegister.addAll(converters); - toRegister.addAll(storeConversions.getStoreConverters()); - toRegister.addAll(DEFAULT_CONVERTERS); + Collections.reverse(registeredConverters); - toRegister.stream()// - .flatMap(it -> storeConversions.getRegistrationsFor(it).stream())// - .forEach(this::register); + this.converters = Collections.unmodifiableList(registeredConverters); + this.simpleTypeHolder = new SimpleTypeHolder(customSimpleTypes, storeConversions.getStoreTypeHolder()); + } - Collections.reverse(toRegister); + /** + * Validate a given {@link ConverterRegistration} in a specific setup.
+ * Non {@link ReadingConverter reading} and user defined {@link Converter converters} are only considered supported if + * the {@link ConverterRegistrationIntent#isSimpleTargetType() target type} is considered to be a store simple type. + * + * @param registration + * @return + * @since 2.3 + */ + protected boolean isSupportedConverter(ConverterRegistrationIntent registration) { - this.converters = Collections.unmodifiableList(toRegister); - this.simpleTypeHolder = new SimpleTypeHolder(customSimpleTypes, storeConversions.getStoreTypeHolder()); + boolean register = registration.isUserConverter() || (registration.isReading() && registration.isSimpleSourceType()) + || (registration.isWriting() && registration.isSimpleTargetType()); + + if (LOG.isDebugEnabled()) { + + if (register) { + LOG.debug(String.format(ADD_CONVERTER, registration.isUserConverter() ? "user defined " : "", + registration.getSourceType(), registration.getTargetType(), + registration.isReading() ? "reading" : "writing")); + } else { + LOG.debug(String.format(SKIP_CONVERTER, registration.getSourceType(), registration.getTargetType(), + registration.isReading() ? "reading" : "writing", + registration.isReading() ? registration.getSourceType() : registration.getTargetType())); + } + } + + return register; } /** @@ -152,6 +186,36 @@ public void registerConvertersIn(ConverterRegistry conversionService) { converters.forEach(it -> registerConverterIn(it, conversionService)); } + /** + * Get all converters and add some origin information + * + * @param storeConversions + * @param converters + * @return + */ + private List collectPotentialConverterRegistrations(StoreConversions storeConversions, + Collection converters) { + + List converterRegistrations = new ArrayList<>(); + + converterRegistrations.addAll(converters.stream() // + .flatMap(it -> storeConversions.getRegistrationsFor(it).stream()) // + .map(ConverterRegistrationIntent::userConverters) // + .collect(Collectors.toList())); + + converterRegistrations.addAll(storeConversions.getStoreConverters().stream() // + .flatMap(it -> storeConversions.getRegistrationsFor(it).stream()) // + .map(ConverterRegistrationIntent::storeConverters) // + .collect(Collectors.toList())); + + converterRegistrations.addAll(DEFAULT_CONVERTERS.stream() // + .flatMap(it -> storeConversions.getRegistrationsFor(it).stream()) // + .map(ConverterRegistrationIntent::defaultConverters) // + .collect(Collectors.toList())); + + return converterRegistrations; + } + /** * Registers the given converter in the given {@link GenericConversionService}. * @@ -193,7 +257,7 @@ private void registerConverterIn(Object candidate, ConverterRegistry conversionS * * @param converterRegistration */ - private void register(ConverterRegistration converterRegistration) { + private Object register(ConverterRegistration converterRegistration) { Assert.notNull(converterRegistration, "Converter registration must not be null!"); @@ -217,6 +281,8 @@ private void register(ConverterRegistration converterRegistration) { LOG.warn(String.format(WRITE_CONVERTER_NOT_SIMPLE, pair.getSourceType(), pair.getTargetType())); } } + + return converterRegistration.getConverter(); } /** @@ -442,6 +508,80 @@ public Class computeIfAbsent(Class targetType, Function getSourceType() { + return delegate.getConvertiblePair().getSourceType(); + } + + Class getTargetType() { + return delegate.getConvertiblePair().getTargetType(); + } + + public boolean isWriting() { + return delegate.isWriting(); + } + + public boolean isReading() { + return delegate.isReading(); + } + + public boolean isSimpleSourceType() { + return delegate.isSimpleSourceType(); + } + + public boolean isSimpleTargetType() { + return delegate.isSimpleTargetType(); + } + + public boolean isUserConverter() { + return isConverterOfSource(ConverterOrigin.USER_DEFINED); + } + + public boolean isDefaultConveter() { + return isConverterOfSource(ConverterOrigin.DEFAULT); + } + + public ConverterRegistration getConverterRegistration() { + return delegate; + } + + private boolean isConverterOfSource(ConverterOrigin source) { + return origin.equals(source); + } + + protected enum ConverterOrigin { + DEFAULT, USER_DEFINED, STORE + } + } + /** * Conversion registration information. * @@ -449,8 +589,9 @@ public Class computeIfAbsent(Class targetType, Function getRegistrationsFor(Object converter) { return convertibleTypes == null // ? Streamable.empty() // - : Streamable.of(convertibleTypes).map(it -> register(it, isReading, isWriting)); + : Streamable.of(convertibleTypes).map(it -> register(converter, it, isReading, isWriting)); } else if (converter instanceof ConverterFactory) { @@ -600,15 +748,17 @@ private Streamable getRegistrationFor(Object converter, C throw new IllegalStateException(String.format("Couldn't resolve type arguments for %s!", converterType)); } - return Streamable.of(register(arguments[0], arguments[1], isReading, isWriting)); + return Streamable.of(register(converter, arguments[0], arguments[1], isReading, isWriting)); } - private ConverterRegistration register(Class source, Class target, boolean isReading, boolean isWriting) { - return register(new ConvertiblePair(source, target), isReading, isWriting); + private ConverterRegistration register(Object converter, Class source, Class target, boolean isReading, + boolean isWriting) { + return register(converter, new ConvertiblePair(source, target), isReading, isWriting); } - private ConverterRegistration register(ConvertiblePair pair, boolean isReading, boolean isWriting) { - return new ConverterRegistration(pair, this, isReading, isWriting); + private ConverterRegistration register(Object converter, ConvertiblePair pair, boolean isReading, + boolean isWriting) { + return new ConverterRegistration(converter, pair, this, isReading, isWriting); } private boolean isStoreSimpleType(Class type) { diff --git a/src/test/java/org/springframework/data/convert/CustomConversionsUnitTests.java b/src/test/java/org/springframework/data/convert/CustomConversionsUnitTests.java index 75e99b8b85..aafb802140 100644 --- a/src/test/java/org/springframework/data/convert/CustomConversionsUnitTests.java +++ b/src/test/java/org/springframework/data/convert/CustomConversionsUnitTests.java @@ -16,6 +16,7 @@ package org.springframework.data.convert; import static org.assertj.core.api.Assertions.*; +import static org.mockito.Mockito.*; import java.text.DateFormat; import java.text.Format; @@ -27,17 +28,17 @@ import org.joda.time.DateTime; import org.junit.Test; - import org.springframework.aop.framework.ProxyFactory; import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.converter.ConverterFactory; +import org.springframework.core.convert.converter.ConverterRegistry; import org.springframework.core.convert.support.ConfigurableConversionService; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.core.convert.support.GenericConversionService; import org.springframework.data.convert.ConverterBuilder.ConverterAware; import org.springframework.data.convert.CustomConversions.StoreConversions; +import org.springframework.data.convert.ThreeTenBackPortConverters.LocalDateTimeToJavaTimeInstantConverter; import org.springframework.data.mapping.model.SimpleTypeHolder; - import org.threeten.bp.LocalDateTime; /** @@ -194,6 +195,29 @@ public void registersConverterFromConverterAware() { assertThat(conversionService.canConvert(Locale.class, CustomType.class)).isTrue(); } + @Test // DATACMNS-1615 + public void skipsUnsupportedDefaultWritingConverter() { + + ConverterRegistry registry = mock(ConverterRegistry.class); + StoreConversions storeConversions = StoreConversions + .of(new SimpleTypeHolder(Collections.singleton(Date.class), true) { + + @Override + public boolean isSimpleType(Class type) { + + if (type.getName().startsWith("java.time")) { + return false; + } + + return super.isSimpleType(type); + } + }); + + new CustomConversions(storeConversions, Collections.emptyList()).registerConvertersIn(registry); + + verify(registry, never()).addConverter(any(LocalDateTimeToJavaTimeInstantConverter.class)); + } + private static Class createProxyTypeFor(Class type) { ProxyFactory factory = new ProxyFactory(); From 0640d0575ab1d89911734fc615dfc76769992ad5 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 25 Nov 2019 10:07:23 +0100 Subject: [PATCH 3/3] DATACMNS-1615 - Allow deferred converter configuration. Add a Supplier variant for constructing new CustomConversions. This allows store implementations to hook into the actual configuration based on user code. Required for messing with store specific simple type registration based on user decisions. --- .../data/convert/CustomConversions.java | 214 +++++++++++++----- .../convert/CustomConversionsUnitTests.java | 62 +++-- 2 files changed, 209 insertions(+), 67 deletions(-) diff --git a/src/main/java/org/springframework/data/convert/CustomConversions.java b/src/main/java/org/springframework/data/convert/CustomConversions.java index 92d2d6b879..457862ead3 100644 --- a/src/main/java/org/springframework/data/convert/CustomConversions.java +++ b/src/main/java/org/springframework/data/convert/CustomConversions.java @@ -22,9 +22,19 @@ import lombok.Value; import lombok.extern.slf4j.Slf4j; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; +import java.util.function.Supplier; import java.util.stream.Collectors; import org.springframework.core.GenericTypeResolver; @@ -61,7 +71,7 @@ public class CustomConversions { private static final String CONVERTER_FILTER = "converter from %s to %s as %s converter."; private static final String ADD_CONVERTER = "Adding %s" + CONVERTER_FILTER; private static final String SKIP_CONVERTER = "Skipping " + CONVERTER_FILTER - + "%s is not a store supported simple type!"; + + " %s is not a store supported simple type!"; private static final List DEFAULT_CONVERTERS; static { @@ -84,6 +94,8 @@ public class CustomConversions { private final ConversionTargetsCache customReadTargetTypes = new ConversionTargetsCache(); private final ConversionTargetsCache customWriteTargetTypes = new ConversionTargetsCache(); + private final ConverterConfiguration converterConfiguration; + private final Function> getReadTarget = convertiblePair -> getCustomTarget( convertiblePair.getSourceType(), convertiblePair.getTargetType(), readingPairs); @@ -94,60 +106,51 @@ public class CustomConversions { convertiblePair.getSourceType(), null, writingPairs); /** - * Creates a new {@link CustomConversions} instance registering all given user defined converters and selecting - * {@link Converter converters} from {@link StoreConversions} depending on - * {@link StoreConversions#getSimpleTypeHolder() store simple types} only considering those that either convert - * to/from a store supported type. + * Deferred initialization allowing store implementations use a callback/consumer driven configuration of the actual + * {@link CustomConversions}. * - * @param storeConversions must not be {@literal null}. - * @param converters must not be {@literal null}. + * @param converterConfiguration the {@link Supplier} providing a {@link ConverterConfiguration}. + * @since 2.3 */ - public CustomConversions(StoreConversions storeConversions, Collection converters) { + protected CustomConversions(Supplier converterConfiguration) { + this(converterConfiguration.get()); + } - Assert.notNull(storeConversions, "StoreConversions must not be null!"); - Assert.notNull(converters, "List of converters must not be null!"); + /** + * @param converterConfiguration the {@link ConverterConfiguration} to apply. + * @since 2.3 + */ + protected CustomConversions(ConverterConfiguration converterConfiguration) { - List registeredConverters = collectPotentialConverterRegistrations(storeConversions, converters).stream() // - .filter(this::isSupportedConverter) // - .map(ConverterRegistrationIntent::getConverterRegistration) // - .map(this::register) // - .distinct() // - .collect(Collectors.toList()); + this.converterConfiguration = converterConfiguration; + + List registeredConverters = collectPotentialConverterRegistrations( + converterConfiguration.getStoreConversions(), converterConfiguration.getUserConverters()).stream() // + .filter(this::isSupportedConverter) // + .filter(it -> !skip(it)) // + .map(ConverterRegistrationIntent::getConverterRegistration) // + .map(this::register) // + .distinct() // + .collect(Collectors.toList()); Collections.reverse(registeredConverters); this.converters = Collections.unmodifiableList(registeredConverters); - this.simpleTypeHolder = new SimpleTypeHolder(customSimpleTypes, storeConversions.getStoreTypeHolder()); + this.simpleTypeHolder = new SimpleTypeHolder(customSimpleTypes, + converterConfiguration.getStoreConversions().getStoreTypeHolder()); } /** - * Validate a given {@link ConverterRegistration} in a specific setup.
- * Non {@link ReadingConverter reading} and user defined {@link Converter converters} are only considered supported if - * the {@link ConverterRegistrationIntent#isSimpleTargetType() target type} is considered to be a store simple type. - * - * @param registration - * @return - * @since 2.3 + * Creates a new {@link CustomConversions} instance registering all given user defined converters and selecting + * {@link Converter converters} from {@link StoreConversions} depending on + * {@link StoreConversions#getSimpleTypeHolder() store simple types} only considering those that either convert + * to/from a store supported type. + * + * @param storeConversions must not be {@literal null}. + * @param converters must not be {@literal null}. */ - protected boolean isSupportedConverter(ConverterRegistrationIntent registration) { - - boolean register = registration.isUserConverter() || (registration.isReading() && registration.isSimpleSourceType()) - || (registration.isWriting() && registration.isSimpleTargetType()); - - if (LOG.isDebugEnabled()) { - - if (register) { - LOG.debug(String.format(ADD_CONVERTER, registration.isUserConverter() ? "user defined " : "", - registration.getSourceType(), registration.getTargetType(), - registration.isReading() ? "reading" : "writing")); - } else { - LOG.debug(String.format(SKIP_CONVERTER, registration.getSourceType(), registration.getTargetType(), - registration.isReading() ? "reading" : "writing", - registration.isReading() ? registration.getSourceType() : registration.getTargetType())); - } - } - - return register; + public CustomConversions(StoreConversions storeConversions, Collection converters) { + this(new ConverterConfiguration(storeConversions, new ArrayList<>(converters))); } /** @@ -187,11 +190,12 @@ public void registerConvertersIn(ConverterRegistry conversionService) { } /** - * Get all converters and add some origin information + * Get all converters and add origin information * * @param storeConversions * @param converters * @return + * @since 2.3 */ private List collectPotentialConverterRegistrations(StoreConversions storeConversions, Collection converters) { @@ -224,31 +228,27 @@ private List collectPotentialConverterRegistrations */ private void registerConverterIn(Object candidate, ConverterRegistry conversionService) { - boolean added = false; - if (candidate instanceof Converter) { conversionService.addConverter(Converter.class.cast(candidate)); - added = true; + return; } if (candidate instanceof ConverterFactory) { conversionService.addConverterFactory(ConverterFactory.class.cast(candidate)); - added = true; + return; } if (candidate instanceof GenericConverter) { conversionService.addConverter(GenericConverter.class.cast(candidate)); - added = true; + return; } if (candidate instanceof ConverterAware) { ConverterAware.class.cast(candidate).getConverters().forEach(it -> registerConverterIn(it, conversionService)); - added = true; + return; } - if (!added) { - throw new IllegalArgumentException(String.format(NOT_A_CONVERTER, candidate)); - } + throw new IllegalArgumentException(String.format(NOT_A_CONVERTER, candidate)); } /** @@ -285,6 +285,48 @@ private Object register(ConverterRegistration converterRegistration) { return converterRegistration.getConverter(); } + /** + * Validate a given {@link ConverterRegistration} in a specific setup.
+ * Non {@link ReadingConverter reading} and user defined {@link Converter converters} are only considered supported if + * the {@link ConverterRegistrationIntent#isSimpleTargetType() target type} is considered to be a store simple type. + * + * @param registrationIntent + * @return {@literal true} if supported. + * @since 2.3 + */ + private boolean isSupportedConverter(ConverterRegistrationIntent registrationIntent) { + + boolean register = registrationIntent.isUserConverter() + || (registrationIntent.isReading() && registrationIntent.isSimpleSourceType()) + || (registrationIntent.isWriting() && registrationIntent.isSimpleTargetType()); + + if (LOG.isDebugEnabled()) { + + if (register) { + LOG.debug(String.format(ADD_CONVERTER, registrationIntent.isUserConverter() ? "user defined " : "", + registrationIntent.getSourceType(), registrationIntent.getTargetType(), + registrationIntent.isReading() ? "reading" : "writing")); + } else { + LOG.debug(String.format(SKIP_CONVERTER, registrationIntent.getSourceType(), registrationIntent.getTargetType(), + registrationIntent.isReading() ? "reading" : "writing", + registrationIntent.isReading() ? registrationIntent.getSourceType() : registrationIntent.getTargetType())); + } + } + + return register; + } + + /** + * @param intent must not be {@literal null}. + * @return {@literal true} if the given {@link ConverterRegistration} shall be skipped. + * @since 2.3 + */ + private boolean skip(ConverterRegistrationIntent intent) { + + return intent.isDefaultConveter() + && converterConfiguration.getSkipFor().contains(intent.getConverterRegistration().getConvertiblePair()); + } + /** * Returns the target type to convert to in case we have a custom conversion registered to convert the given source * type into a Mongo native one. @@ -765,4 +807,68 @@ private boolean isStoreSimpleType(Class type) { return storeTypeHolder.isSimpleType(type); } } + + /** + * Value object holding the actual {@link StoreConversions} and custom {@link Converter converters} configured for + * registration. + * + * @author Christoph Strobl + * @since 2.3 + */ + protected static class ConverterConfiguration { + + private final StoreConversions storeConversions; + private final List userConverters; + private final Set skipFor; + + /** + * Create a new ConverterConfiguration holding the given {@link StoreConversions} and user defined converters. + * + * @param storeConversions must not be {@literal null}. + * @param userConverters must not be {@literal null} use {@link Collections#emptyList()} instead. + */ + public ConverterConfiguration(StoreConversions storeConversions, List userConverters) { + this(storeConversions, userConverters, Collections.emptySet()); + } + + /** + * Create a new ConverterConfiguration holding the given {@link StoreConversions} and user defined converters as + * well as a {@link Collection} of {@link ConvertiblePair} for which to skip the registration of default converters. + *
+ * This allows store implementations to modify default converter registration based on specific needs and + * configurations. User defined converters will are never subject of filtering. + * + * @param storeConversions must not be {@literal null}. + * @param userConverters must not be {@literal null} use {@link Collections#emptyList()} instead. + * @param skipFor must not be {@literal null} use {@link Collections#emptyList()} instead. + */ + public ConverterConfiguration(StoreConversions storeConversions, List userConverters, + Collection skipFor) { + + this.storeConversions = storeConversions; + this.userConverters = new ArrayList<>(userConverters); + this.skipFor = new HashSet<>(skipFor); + } + + /** + * @return never {@literal null} + */ + StoreConversions getStoreConversions() { + return storeConversions; + } + + /** + * @return never {@literal null}. + */ + List getUserConverters() { + return userConverters; + } + + /** + * @return never {@literal null}. + */ + Set getSkipFor() { + return skipFor; + } + } } diff --git a/src/test/java/org/springframework/data/convert/CustomConversionsUnitTests.java b/src/test/java/org/springframework/data/convert/CustomConversionsUnitTests.java index aafb802140..3164d457ce 100644 --- a/src/test/java/org/springframework/data/convert/CustomConversionsUnitTests.java +++ b/src/test/java/org/springframework/data/convert/CustomConversionsUnitTests.java @@ -27,16 +27,19 @@ import java.util.Locale; import org.joda.time.DateTime; -import org.junit.Test; +import org.junit.jupiter.api.Test; import org.springframework.aop.framework.ProxyFactory; import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.converter.ConverterFactory; import org.springframework.core.convert.converter.ConverterRegistry; +import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair; import org.springframework.core.convert.support.ConfigurableConversionService; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.core.convert.support.GenericConversionService; import org.springframework.data.convert.ConverterBuilder.ConverterAware; +import org.springframework.data.convert.CustomConversions.ConverterConfiguration; import org.springframework.data.convert.CustomConversions.StoreConversions; +import org.springframework.data.convert.Jsr310Converters.LocalDateTimeToDateConverter; import org.springframework.data.convert.ThreeTenBackPortConverters.LocalDateTimeToJavaTimeInstantConverter; import org.springframework.data.mapping.model.SimpleTypeHolder; import org.threeten.bp.LocalDateTime; @@ -51,6 +54,15 @@ */ public class CustomConversionsUnitTests { + static final SimpleTypeHolder DATE_EXCLUDING_SIMPLE_TYPE_HOLDER = new SimpleTypeHolder( + Collections.singleton(Date.class), true) { + + @Override + public boolean isSimpleType(Class type) { + return type.getName().startsWith("java.time") ? false : super.isSimpleType(type); + } + }; + @Test // DATACMNS-1035 public void findsBasicReadAndWriteConversions() { @@ -199,23 +211,47 @@ public void registersConverterFromConverterAware() { public void skipsUnsupportedDefaultWritingConverter() { ConverterRegistry registry = mock(ConverterRegistry.class); - StoreConversions storeConversions = StoreConversions - .of(new SimpleTypeHolder(Collections.singleton(Date.class), true) { - @Override - public boolean isSimpleType(Class type) { + new CustomConversions(StoreConversions.of(DATE_EXCLUDING_SIMPLE_TYPE_HOLDER), Collections.emptyList()) + .registerConvertersIn(registry); + + verify(registry, never()).addConverter(any(LocalDateTimeToJavaTimeInstantConverter.class)); + } + + @Test // DATACMNS-1615 + public void doesNotSkipUnsupportedUserConverter() { - if (type.getName().startsWith("java.time")) { - return false; - } + ConverterRegistry registry = mock(ConverterRegistry.class); - return super.isSimpleType(type); - } - }); + new CustomConversions(StoreConversions.of(DATE_EXCLUDING_SIMPLE_TYPE_HOLDER), + Collections.singletonList(LocalDateTimeToJavaTimeInstantConverter.INSTANCE)).registerConvertersIn(registry); - new CustomConversions(storeConversions, Collections.emptyList()).registerConvertersIn(registry); + verify(registry).addConverter(any(LocalDateTimeToJavaTimeInstantConverter.class)); + } - verify(registry, never()).addConverter(any(LocalDateTimeToJavaTimeInstantConverter.class)); + @Test // DATACMNS-1615 + public void skipsConverterBasedOnConfiguration() { + + ConverterRegistry registry = mock(ConverterRegistry.class); + + ConverterConfiguration config = new ConverterConfiguration(StoreConversions.NONE, Collections.emptyList(), + Collections.singleton(new ConvertiblePair(java.time.LocalDateTime.class, Date.class))); + new CustomConversions(config).registerConvertersIn(registry); + + verify(registry, never()).addConverter(any(LocalDateTimeToDateConverter.class)); + } + + @Test // DATACMNS-1615 + public void doesNotSkipUserConverterConverterEvenWhenConfigurationWouldNotAllowIt() { + + ConverterRegistry registry = mock(ConverterRegistry.class); + + ConverterConfiguration config = new ConverterConfiguration(StoreConversions.NONE, + Collections.singletonList(LocalDateTimeToDateConverter.INSTANCE), + Collections.singleton(new ConvertiblePair(java.time.LocalDateTime.class, Date.class))); + new CustomConversions(config).registerConvertersIn(registry); + + verify(registry).addConverter(any(LocalDateTimeToDateConverter.class)); } private static Class createProxyTypeFor(Class type) {