From 8a2ce30e700ff40e88695678512704e4b36e51c9 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 14 Nov 2016 13:52:02 +0100 Subject: [PATCH] DATACMNS-836 - Encapsulate reactive type conversion in ReactiveWrapperConverters. Reactive type conversion now happens fully inside of ReactiveWrapperConverters and does not require an external ConversionService. This change allows changes to reactive type conversion without changing the API since all details are encapsulated by ReactiveWrapperConverters. --- .../support/QueryExecutionResultHandler.java | 6 +- .../ReactiveRepositoryInformation.java | 7 +- .../support/RepositoryFactorySupport.java | 15 +-- .../util/ReactiveWrapperConverters.java | 105 ++++++++++-------- ...eactiveRepositoryInformationUnitTests.java | 8 -- ...pperRepositoryFactorySupportUnitTests.java | 6 - 6 files changed, 73 insertions(+), 74 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/core/support/QueryExecutionResultHandler.java b/src/main/java/org/springframework/data/repository/core/support/QueryExecutionResultHandler.java index a7dd4ae82..f9013b440 100644 --- a/src/main/java/org/springframework/data/repository/core/support/QueryExecutionResultHandler.java +++ b/src/main/java/org/springframework/data/repository/core/support/QueryExecutionResultHandler.java @@ -44,7 +44,6 @@ class QueryExecutionResultHandler { GenericConversionService conversionService = new DefaultConversionService(); QueryExecutionConverters.registerConvertersIn(conversionService); - ReactiveWrapperConverters.registerConvertersIn(conversionService); this.conversionService = conversionService; } @@ -87,6 +86,11 @@ class QueryExecutionResultHandler { } if (result != null) { + + if (ReactiveWrapperConverters.supports(expectedReturnType)) { + return ReactiveWrapperConverters.toWrapper(result, expectedReturnType); + } + return conversionService.canConvert(result.getClass(), expectedReturnType) ? conversionService.convert(result, expectedReturnType) : result; } diff --git a/src/main/java/org/springframework/data/repository/core/support/ReactiveRepositoryInformation.java b/src/main/java/org/springframework/data/repository/core/support/ReactiveRepositoryInformation.java index 5fc749187..b53b10b98 100644 --- a/src/main/java/org/springframework/data/repository/core/support/ReactiveRepositoryInformation.java +++ b/src/main/java/org/springframework/data/repository/core/support/ReactiveRepositoryInformation.java @@ -27,11 +27,11 @@ import java.util.function.BiPredicate; import org.springframework.core.MethodParameter; import org.springframework.core.convert.ConversionService; -import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.util.QueryExecutionConverters; import org.springframework.data.repository.util.ReactiveWrapperConverters; +import org.springframework.data.repository.util.ReactiveWrappers; import org.springframework.util.Assert; /** @@ -189,8 +189,6 @@ public class ReactiveRepositoryInformation extends DefaultRepositoryInformation @RequiredArgsConstructor(staticName = "of") static class WrapperConversionMatch implements BiPredicate, Integer> { - private static final ConversionService CONVERSION_SERVICE = ReactiveWrapperConverters - .registerConvertersIn(new DefaultConversionService()); private final Class[] declaredParameterTypes; /* @@ -208,7 +206,8 @@ public class ReactiveRepositoryInformation extends DefaultRepositoryInformation return false; } - return CONVERSION_SERVICE.canConvert(declaredParameterTypes[index], candidateParameterType); + return ReactiveWrappers.isAvailable() + && ReactiveWrapperConverters.canConvert(declaredParameterTypes[index], candidateParameterType); } } diff --git a/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java b/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java index e7f65eae8..3a7a8c993 100644 --- a/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java +++ b/src/main/java/org/springframework/data/repository/core/support/RepositoryFactorySupport.java @@ -35,9 +35,7 @@ import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.core.GenericTypeResolver; import org.springframework.core.MethodParameter; -import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.TypeDescriptor; -import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.repository.Repository; @@ -53,6 +51,7 @@ import org.springframework.data.repository.query.QueryMethod; import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.data.repository.util.ClassUtils; import org.springframework.data.repository.util.ReactiveWrapperConverters; +import org.springframework.data.repository.util.ReactiveWrappers; import org.springframework.data.util.ReflectionUtils; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; @@ -583,9 +582,6 @@ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware, */ public class ConvertingImplementationMethodExecutionInterceptor extends ImplementationMethodExecutionInterceptor { - private final ConversionService conversionService = ReactiveWrapperConverters - .registerConvertersIn(new DefaultConversionService()); - /** * @param repositoryInformation * @param customImplementation @@ -624,13 +620,12 @@ public abstract class RepositoryFactorySupport implements BeanClassLoaderAware, continue; } - if (parameterTypes[i].isAssignableFrom(parameters[i].getClass()) - || !conversionService.canConvert(parameters[i].getClass(), parameterTypes[i])) { - - result[i] = parameters[i]; + if (!parameterTypes[i].isAssignableFrom(parameters[i].getClass()) && ReactiveWrappers.isAvailable() + && ReactiveWrapperConverters.canConvert(parameters[i].getClass(), parameterTypes[i])) { + result[i] = ReactiveWrapperConverters.toWrapper(parameters[i], parameterTypes[i]); } else { - result[i] = conversionService.convert(parameters[i], parameterTypes[i]); + result[i] = parameters[i]; } } diff --git a/src/main/java/org/springframework/data/repository/util/ReactiveWrapperConverters.java b/src/main/java/org/springframework/data/repository/util/ReactiveWrapperConverters.java index af9fe156c..532211dca 100644 --- a/src/main/java/org/springframework/data/repository/util/ReactiveWrapperConverters.java +++ b/src/main/java/org/springframework/data/repository/util/ReactiveWrapperConverters.java @@ -90,7 +90,7 @@ public class ReactiveWrapperConverters { * * @param conversionService must not be {@literal null}. */ - public static ConversionService registerConvertersIn(ConfigurableConversionService conversionService) { + private static ConversionService registerConvertersIn(ConfigurableConversionService conversionService) { Assert.notNull(conversionService, "ConversionService must not be null!"); @@ -171,23 +171,23 @@ public class ReactiveWrapperConverters { } /** - * Casts or converts the given wrapper type into a different wrapper type. + * Casts or adopts the given wrapper type to a target wrapper type. * - * @param stream the stream, must not be {@literal null}. - * @param expectedWrapperType must not be {@literal null}. + * @param reactiveObject the stream, must not be {@literal null}. + * @param targetWrapperType must not be {@literal null}. * @return */ @SuppressWarnings("unchecked") - public static T toWrapper(Object stream, Class expectedWrapperType) { + public static T toWrapper(Object reactiveObject, Class targetWrapperType) { - Assert.notNull(stream, "Stream must not be null!"); - Assert.notNull(expectedWrapperType, "Converter must not be null!"); + Assert.notNull(reactiveObject, "Reactive source object must not be null!"); + Assert.notNull(targetWrapperType, "Reactive target type must not be null!"); - if (expectedWrapperType.isAssignableFrom(stream.getClass())) { - return (T) stream; + if (targetWrapperType.isAssignableFrom(reactiveObject.getClass())) { + return (T) reactiveObject; } - return GENERIC_CONVERSION_SERVICE.convert(stream, expectedWrapperType); + return GENERIC_CONVERSION_SERVICE.convert(reactiveObject, targetWrapperType); } /** @@ -210,6 +210,21 @@ public class ReactiveWrapperConverters { .orElseThrow(() -> new IllegalStateException(String.format("Cannot apply converter to %s", reactiveObject))); } + /** + * Return {@literal true} if objects of {@code sourceType} can be converted to the {@code targetType}. + * + * @param sourceType must not be {@literal null}. + * @param targetType must not be {@literal null}. + * @return {@literal true} if a conversion can be performed. + */ + public static boolean canConvert(Class sourceType, Class targetType) { + + Assert.notNull(sourceType, "Source type must not be null!"); + Assert.notNull(targetType, "Target type must not be null!"); + + return GENERIC_CONVERSION_SERVICE.canConvert(sourceType, targetType); + } + // ------------------------------------------------------------------------- // Wrapper descriptors // ------------------------------------------------------------------------- @@ -417,7 +432,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum PublisherToFluxConverter implements Converter, Flux> { + private enum PublisherToFluxConverter implements Converter, Flux> { INSTANCE; @@ -433,7 +448,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum PublisherToMonoConverter implements Converter, Mono> { + private enum PublisherToMonoConverter implements Converter, Mono> { INSTANCE; @@ -453,7 +468,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum PublisherToRxJava1SingleConverter implements Converter, Single> { + private enum PublisherToRxJava1SingleConverter implements Converter, Single> { INSTANCE; @@ -469,7 +484,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum PublisherToRxJava1CompletableConverter implements Converter, Completable> { + private enum PublisherToRxJava1CompletableConverter implements Converter, Completable> { INSTANCE; @@ -485,7 +500,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum PublisherToRxJava1ObservableConverter implements Converter, Observable> { + private enum PublisherToRxJava1ObservableConverter implements Converter, Observable> { INSTANCE; @@ -501,7 +516,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava1SingleToPublisherConverter implements Converter, Publisher> { + private enum RxJava1SingleToPublisherConverter implements Converter, Publisher> { INSTANCE; @@ -517,7 +532,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava1SingleToMonoConverter implements Converter, Mono> { + private enum RxJava1SingleToMonoConverter implements Converter, Mono> { INSTANCE; @@ -533,7 +548,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava1SingleToFluxConverter implements Converter, Flux> { + private enum RxJava1SingleToFluxConverter implements Converter, Flux> { INSTANCE; @@ -549,7 +564,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava1CompletableToPublisherConverter implements Converter> { + private enum RxJava1CompletableToPublisherConverter implements Converter> { INSTANCE; @@ -565,7 +580,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava1CompletableToMonoConverter implements Converter> { + private enum RxJava1CompletableToMonoConverter implements Converter> { INSTANCE; @@ -581,7 +596,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava1ObservableToPublisherConverter implements Converter, Publisher> { + private enum RxJava1ObservableToPublisherConverter implements Converter, Publisher> { INSTANCE; @@ -597,7 +612,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava1ObservableToMonoConverter implements Converter, Mono> { + private enum RxJava1ObservableToMonoConverter implements Converter, Mono> { INSTANCE; @@ -613,7 +628,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava1ObservableToFluxConverter implements Converter, Flux> { + private enum RxJava1ObservableToFluxConverter implements Converter, Flux> { INSTANCE; @@ -629,7 +644,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava1ObservableToSingleConverter implements Converter, Single> { + private enum RxJava1ObservableToSingleConverter implements Converter, Single> { INSTANCE; @@ -645,7 +660,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava1SingleToObservableConverter implements Converter, Observable> { + private enum RxJava1SingleToObservableConverter implements Converter, Observable> { INSTANCE; @@ -665,7 +680,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum PublisherToRxJava2SingleConverter implements Converter, io.reactivex.Single> { + private enum PublisherToRxJava2SingleConverter implements Converter, io.reactivex.Single> { INSTANCE; @@ -682,7 +697,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum PublisherToRxJava2CompletableConverter implements Converter, io.reactivex.Completable> { + private enum PublisherToRxJava2CompletableConverter implements Converter, io.reactivex.Completable> { INSTANCE; @@ -699,7 +714,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum PublisherToRxJava2ObservableConverter implements Converter, io.reactivex.Observable> { + private enum PublisherToRxJava2ObservableConverter implements Converter, io.reactivex.Observable> { INSTANCE; @@ -716,7 +731,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava2SingleToPublisherConverter implements Converter, Publisher> { + private enum RxJava2SingleToPublisherConverter implements Converter, Publisher> { INSTANCE; @@ -732,7 +747,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava2SingleToMonoConverter implements Converter, Mono> { + private enum RxJava2SingleToMonoConverter implements Converter, Mono> { INSTANCE; @@ -748,7 +763,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava2SingleToFluxConverter implements Converter, Flux> { + private enum RxJava2SingleToFluxConverter implements Converter, Flux> { INSTANCE; @@ -764,7 +779,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava2CompletableToPublisherConverter implements Converter> { + private enum RxJava2CompletableToPublisherConverter implements Converter> { INSTANCE; @@ -780,7 +795,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava2CompletableToMonoConverter implements Converter> { + private enum RxJava2CompletableToMonoConverter implements Converter> { INSTANCE; @@ -796,7 +811,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava2ObservableToPublisherConverter implements Converter, Publisher> { + private enum RxJava2ObservableToPublisherConverter implements Converter, Publisher> { INSTANCE; @@ -812,7 +827,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava2ObservableToMonoConverter implements Converter, Mono> { + private enum RxJava2ObservableToMonoConverter implements Converter, Mono> { INSTANCE; @@ -828,7 +843,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava2ObservableToFluxConverter implements Converter, Flux> { + private enum RxJava2ObservableToFluxConverter implements Converter, Flux> { INSTANCE; @@ -844,7 +859,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum PublisherToRxJava2FlowableConverter implements Converter, io.reactivex.Flowable> { + private enum PublisherToRxJava2FlowableConverter implements Converter, io.reactivex.Flowable> { INSTANCE; @@ -860,7 +875,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava2FlowableToPublisherConverter implements Converter, Publisher> { + private enum RxJava2FlowableToPublisherConverter implements Converter, Publisher> { INSTANCE; @@ -876,7 +891,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum PublisherToRxJava2MaybeConverter implements Converter, io.reactivex.Maybe> { + private enum PublisherToRxJava2MaybeConverter implements Converter, io.reactivex.Maybe> { INSTANCE; @@ -892,7 +907,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava2MaybeToPublisherConverter implements Converter, Publisher> { + private enum RxJava2MaybeToPublisherConverter implements Converter, Publisher> { INSTANCE; @@ -908,7 +923,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava2MaybeToMonoConverter implements Converter, Mono> { + private enum RxJava2MaybeToMonoConverter implements Converter, Mono> { INSTANCE; @@ -924,7 +939,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava2MaybeToFluxConverter implements Converter, Flux> { + private enum RxJava2MaybeToFluxConverter implements Converter, Flux> { INSTANCE; @@ -940,7 +955,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava2ObservableToSingleConverter + private enum RxJava2ObservableToSingleConverter implements Converter, io.reactivex.Single> { INSTANCE; @@ -957,7 +972,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava2ObservableToMaybeConverter + private enum RxJava2ObservableToMaybeConverter implements Converter, io.reactivex.Maybe> { INSTANCE; @@ -974,7 +989,7 @@ public class ReactiveWrapperConverters { * @author Mark Paluch * @author 2.0 */ - public enum RxJava2SingleToObservableConverter + private enum RxJava2SingleToObservableConverter implements Converter, io.reactivex.Observable> { INSTANCE; diff --git a/src/test/java/org/springframework/data/repository/core/support/ReactiveRepositoryInformationUnitTests.java b/src/test/java/org/springframework/data/repository/core/support/ReactiveRepositoryInformationUnitTests.java index 9e24f8b60..1483d36dc 100644 --- a/src/test/java/org/springframework/data/repository/core/support/ReactiveRepositoryInformationUnitTests.java +++ b/src/test/java/org/springframework/data/repository/core/support/ReactiveRepositoryInformationUnitTests.java @@ -27,12 +27,10 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.runners.MockitoJUnitRunner; import org.reactivestreams.Publisher; -import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.reactive.ReactiveCrudRepository; import org.springframework.data.repository.reactive.ReactiveSortingRepository; import org.springframework.data.repository.reactive.RxJava1CrudRepository; -import org.springframework.data.repository.util.ReactiveWrapperConverters; /** * Unit tests for {@link ReactiveRepositoryInformation}. @@ -66,9 +64,6 @@ public class ReactiveRepositoryInformationUnitTests { @Test public void discoversMethodWithConvertibleArguments() throws Exception { - DefaultConversionService conversionService = new DefaultConversionService(); - ReactiveWrapperConverters.registerConvertersIn(conversionService); - Method method = RxJava1InterfaceWithGenerics.class.getMethod("save", Observable.class); RepositoryMetadata metadata = new DefaultRepositoryMetadata(RxJava1InterfaceWithGenerics.class); DefaultRepositoryInformation information = new ReactiveRepositoryInformation(metadata, REPOSITORY, null); @@ -117,9 +112,6 @@ public class ReactiveRepositoryInformationUnitTests { @Test public void discoversMethodExactObjectArguments() throws Exception { - DefaultConversionService conversionService = new DefaultConversionService(); - ReactiveWrapperConverters.registerConvertersIn(conversionService); - Method method = ReactiveJavaInterfaceWithGenerics.class.getMethod("save", Object.class); RepositoryMetadata metadata = new DefaultRepositoryMetadata(ReactiveJavaInterfaceWithGenerics.class); DefaultRepositoryInformation information = new ReactiveRepositoryInformation(metadata, REPOSITORY, null); diff --git a/src/test/java/org/springframework/data/repository/core/support/ReactiveWrapperRepositoryFactorySupportUnitTests.java b/src/test/java/org/springframework/data/repository/core/support/ReactiveWrapperRepositoryFactorySupportUnitTests.java index 2eb06863f..2fc05c7ad 100644 --- a/src/test/java/org/springframework/data/repository/core/support/ReactiveWrapperRepositoryFactorySupportUnitTests.java +++ b/src/test/java/org/springframework/data/repository/core/support/ReactiveWrapperRepositoryFactorySupportUnitTests.java @@ -30,10 +30,8 @@ import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; -import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.data.repository.Repository; import org.springframework.data.repository.reactive.ReactiveSortingRepository; -import org.springframework.data.repository.util.ReactiveWrapperConverters; /** * Unit tests for {@link RepositoryFactorySupport} using reactive wrapper types. @@ -52,10 +50,6 @@ public class ReactiveWrapperRepositoryFactorySupportUnitTests { @Before public void setUp() { - - DefaultConversionService defaultConversionService = new DefaultConversionService(); - ReactiveWrapperConverters.registerConvertersIn(defaultConversionService); - factory = new DummyRepositoryFactory(backingRepo); }