From 864204f647ae6778976a402449cfec8b40c35ace Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 6 Jul 2017 11:34:45 +0200 Subject: [PATCH] DATACMNS-1102 - Reuse ConversionService in ProjectingMethodInterceptors created by ProxyProjectionFactory. Applied the same to internals of MapDataBinder. --- .../ProjectingMethodInterceptor.java | 29 +++--------- .../projection/ProxyProjectionFactory.java | 5 ++- .../data/web/MapDataBinder.java | 44 +++++++------------ .../ProjectingMethodInterceptorUnitTests.java | 39 +++++++++------- 4 files changed, 50 insertions(+), 67 deletions(-) diff --git a/src/main/java/org/springframework/data/projection/ProjectingMethodInterceptor.java b/src/main/java/org/springframework/data/projection/ProjectingMethodInterceptor.java index f09c347a4..45d03c548 100644 --- a/src/main/java/org/springframework/data/projection/ProjectingMethodInterceptor.java +++ b/src/main/java/org/springframework/data/projection/ProjectingMethodInterceptor.java @@ -15,6 +15,9 @@ */ package org.springframework.data.projection; +import lombok.NonNull; +import lombok.RequiredArgsConstructor; + import java.lang.reflect.Array; import java.util.Arrays; import java.util.Collection; @@ -27,7 +30,6 @@ import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.springframework.core.CollectionFactory; import org.springframework.core.convert.ConversionService; -import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.TypeInformation; import org.springframework.util.Assert; @@ -41,29 +43,12 @@ import org.springframework.util.ObjectUtils; * @author Oliver Gierke * @since 1.10 */ +@RequiredArgsConstructor class ProjectingMethodInterceptor implements MethodInterceptor { - private final ProjectionFactory factory; - private final MethodInterceptor delegate; - private final ConversionService conversionService; - - /** - * Creates a new {@link ProjectingMethodInterceptor} using the given {@link ProjectionFactory} and delegate - * {@link MethodInterceptor}. - * - * @param factory the {@link ProjectionFactory} to use to create projections if types do not match, must not be - * {@literal null}.. - * @param delegate the {@link MethodInterceptor} to trigger to create the source value, must not be {@literal null}.. - */ - public ProjectingMethodInterceptor(ProjectionFactory factory, MethodInterceptor delegate) { - - Assert.notNull(factory, "ProjectionFactory must not be null!"); - Assert.notNull(delegate, "Delegate MethodInterceptor must not be null!"); - - this.factory = factory; - this.delegate = delegate; - this.conversionService = new DefaultConversionService(); - } + private final @NonNull ProjectionFactory factory; + private final @NonNull MethodInterceptor delegate; + private final @NonNull ConversionService conversionService; /* * (non-Javadoc) diff --git a/src/main/java/org/springframework/data/projection/ProxyProjectionFactory.java b/src/main/java/org/springframework/data/projection/ProxyProjectionFactory.java index c2a653c83..117a4dd46 100644 --- a/src/main/java/org/springframework/data/projection/ProxyProjectionFactory.java +++ b/src/main/java/org/springframework/data/projection/ProxyProjectionFactory.java @@ -28,6 +28,8 @@ import org.springframework.aop.framework.Advised; import org.springframework.aop.framework.ProxyFactory; import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.context.ResourceLoaderAware; +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.core.io.ResourceLoader; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -47,6 +49,7 @@ class ProxyProjectionFactory implements ProjectionFactory, ResourceLoaderAware, private static final boolean IS_JAVA_8 = org.springframework.util.ClassUtils.isPresent("java.util.Optional", ProxyProjectionFactory.class.getClassLoader()); + private final ConversionService conversionService = new DefaultConversionService(); private ClassLoader classLoader; /** @@ -152,7 +155,7 @@ class ProxyProjectionFactory implements ProjectionFactory, ResourceLoaderAware, : new PropertyAccessingMethodInterceptor(source); return new ProjectingMethodInterceptor(this, - postProcessAccessorInterceptor(propertyInvocationInterceptor, source, projectionType)); + postProcessAccessorInterceptor(propertyInvocationInterceptor, source, projectionType), conversionService); } /** diff --git a/src/main/java/org/springframework/data/web/MapDataBinder.java b/src/main/java/org/springframework/data/web/MapDataBinder.java index 26c63fe9f..7c0c33099 100644 --- a/src/main/java/org/springframework/data/web/MapDataBinder.java +++ b/src/main/java/org/springframework/data/web/MapDataBinder.java @@ -15,6 +15,9 @@ */ package org.springframework.data.web; +import lombok.NonNull; +import lombok.RequiredArgsConstructor; + import java.beans.PropertyDescriptor; import java.util.HashMap; import java.util.List; @@ -31,7 +34,6 @@ import org.springframework.core.CollectionFactory; 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.mapping.PropertyPath; import org.springframework.data.mapping.PropertyReferenceException; import org.springframework.data.util.TypeInformation; @@ -97,32 +99,15 @@ class MapDataBinder extends WebDataBinder { * @author Oliver Gierke * @since 1.10 */ + @RequiredArgsConstructor private static class MapPropertyAccessor extends AbstractPropertyAccessor { private static final SpelExpressionParser PARSER = new SpelExpressionParser( new SpelParserConfiguration(false, true)); - private final Class type; - private final Map map; - private final ConversionService conversionService; - - /** - * Creates a new {@link MapPropertyAccessor} for the given type, map and {@link ConversionService}. - * - * @param type must not be {@literal null}. - * @param map must not be {@literal null}. - * @param conversionService must not be {@literal null}. - */ - public MapPropertyAccessor(Class type, Map map, ConversionService conversionService) { - - Assert.notNull(type, "Type must not be null!"); - Assert.notNull(map, "Map must not be null!"); - Assert.notNull(conversionService, "ConversionService must not be null!"); - - this.type = type; - this.map = map; - this.conversionService = conversionService; - } + private final @NonNull Class type; + private final @NonNull Map map; + private final @NonNull ConversionService conversionService; /* * (non-Javadoc) @@ -177,7 +162,7 @@ class MapDataBinder extends WebDataBinder { } StandardEvaluationContext context = new StandardEvaluationContext(); - context.addPropertyAccessor(new PropertyTraversingMapAccessor(type, new DefaultConversionService())); + context.addPropertyAccessor(new PropertyTraversingMapAccessor(type, conversionService)); context.setTypeConverter(new StandardTypeConverter(conversionService)); context.setRootObject(map); @@ -191,8 +176,8 @@ class MapDataBinder extends WebDataBinder { if (conversionRequired(value, propertyType.getType())) { - PropertyDescriptor descriptor = BeanUtils - .getPropertyDescriptor(owningType.getType(), leafProperty.getSegment()); + PropertyDescriptor descriptor = BeanUtils.getPropertyDescriptor(owningType.getType(), + leafProperty.getSegment()); MethodParameter methodParameter = new MethodParameter(descriptor.getReadMethod(), -1); TypeDescriptor typeDescriptor = TypeDescriptor.nested(methodParameter, 0); @@ -289,11 +274,12 @@ class MapDataBinder extends WebDataBinder { Class actualPropertyType = path.getType(); - TypeDescriptor valueDescriptor = conversionService.canConvert(String.class, actualPropertyType) ? TypeDescriptor - .valueOf(String.class) : TypeDescriptor.valueOf(HashMap.class); + TypeDescriptor valueDescriptor = conversionService.canConvert(String.class, actualPropertyType) + ? TypeDescriptor.valueOf(String.class) + : TypeDescriptor.valueOf(HashMap.class); - return path.isCollection() ? TypeDescriptor.collection(emptyValue.getClass(), valueDescriptor) : TypeDescriptor - .map(emptyValue.getClass(), TypeDescriptor.valueOf(String.class), valueDescriptor); + return path.isCollection() ? TypeDescriptor.collection(emptyValue.getClass(), valueDescriptor) + : TypeDescriptor.map(emptyValue.getClass(), TypeDescriptor.valueOf(String.class), valueDescriptor); } } diff --git a/src/test/java/org/springframework/data/projection/ProjectingMethodInterceptorUnitTests.java b/src/test/java/org/springframework/data/projection/ProjectingMethodInterceptorUnitTests.java index 3f34aa9cf..df9945523 100644 --- a/src/test/java/org/springframework/data/projection/ProjectingMethodInterceptorUnitTests.java +++ b/src/test/java/org/springframework/data/projection/ProjectingMethodInterceptorUnitTests.java @@ -35,6 +35,8 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.support.DefaultConversionService; /** * Unit tests for {@link ProjectingMethodInterceptor}. @@ -48,6 +50,7 @@ public class ProjectingMethodInterceptorUnitTests { @Mock MethodInterceptor interceptor; @Mock MethodInvocation invocation; @Mock ProjectionFactory factory; + ConversionService conversionService = new DefaultConversionService(); /** * @see DATAREST-221 @@ -55,7 +58,8 @@ public class ProjectingMethodInterceptorUnitTests { @Test public void wrapsDelegateResultInProxyIfTypesDontMatch() throws Throwable { - MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor); + MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor, + conversionService); when(invocation.getMethod()).thenReturn(Helper.class.getMethod("getHelper")); when(interceptor.invoke(invocation)).thenReturn("Foo"); @@ -69,7 +73,7 @@ public class ProjectingMethodInterceptorUnitTests { @Test public void retunsDelegateResultAsIsIfTypesMatch() throws Throwable { - MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(factory, interceptor); + MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(factory, interceptor, conversionService); when(invocation.getMethod()).thenReturn(Helper.class.getMethod("getString")); when(interceptor.invoke(invocation)).thenReturn("Foo"); @@ -83,7 +87,7 @@ public class ProjectingMethodInterceptorUnitTests { @Test public void returnsNullAsIs() throws Throwable { - MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(factory, interceptor); + MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(factory, interceptor, conversionService); when(interceptor.invoke(invocation)).thenReturn(null); @@ -96,7 +100,7 @@ public class ProjectingMethodInterceptorUnitTests { @Test public void considersPrimitivesAsWrappers() throws Throwable { - MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(factory, interceptor); + MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(factory, interceptor, conversionService); when(invocation.getMethod()).thenReturn(Helper.class.getMethod("getPrimitive")); when(interceptor.invoke(invocation)).thenReturn(1L); @@ -112,9 +116,10 @@ public class ProjectingMethodInterceptorUnitTests { @SuppressWarnings("unchecked") public void appliesProjectionToNonEmptySets() throws Throwable { - MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor); - Object result = methodInterceptor.invoke(mockInvocationOf("getHelperCollection", - Collections.singleton(mock(Helper.class)))); + MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor, + conversionService); + Object result = methodInterceptor + .invoke(mockInvocationOf("getHelperCollection", Collections.singleton(mock(Helper.class)))); assertThat(result, is(instanceOf(Set.class))); @@ -130,9 +135,10 @@ public class ProjectingMethodInterceptorUnitTests { @SuppressWarnings("unchecked") public void appliesProjectionToNonEmptyLists() throws Throwable { - MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor); - Object result = methodInterceptor.invoke(mockInvocationOf("getHelperList", - Collections.singletonList(mock(Helper.class)))); + MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor, + conversionService); + Object result = methodInterceptor + .invoke(mockInvocationOf("getHelperList", Collections.singletonList(mock(Helper.class)))); assertThat(result, is(instanceOf(List.class))); @@ -149,7 +155,8 @@ public class ProjectingMethodInterceptorUnitTests { @SuppressWarnings("unchecked") public void allowsMaskingAnArrayIntoACollection() throws Throwable { - MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor); + MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor, + conversionService); Object result = methodInterceptor.invoke(mockInvocationOf("getHelperArray", new Helper[] { mock(Helper.class) })); assertThat(result, is(instanceOf(Collection.class))); @@ -167,10 +174,11 @@ public class ProjectingMethodInterceptorUnitTests { @SuppressWarnings("unchecked") public void appliesProjectionToNonEmptyMap() throws Throwable { - MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor); + MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor, + conversionService); - Object result = methodInterceptor.invoke(mockInvocationOf("getHelperMap", - Collections.singletonMap("foo", mock(Helper.class)))); + Object result = methodInterceptor + .invoke(mockInvocationOf("getHelperMap", Collections.singletonMap("foo", mock(Helper.class)))); assertThat(result, is(instanceOf(Map.class))); @@ -183,7 +191,8 @@ public class ProjectingMethodInterceptorUnitTests { @SuppressWarnings("unchecked") public void returnsSingleElementCollectionForTargetThatReturnsNonCollection() throws Throwable { - MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor); + MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor, + conversionService); Helper reference = mock(Helper.class); Object result = methodInterceptor.invoke(mockInvocationOf("getHelperCollection", reference));