From 143b497090d353e341c9f7016dc9cba47e5d4557 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 5 Jul 2017 09:29:04 +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 | 7 +++- .../data/web/MapDataBinder.java | 31 ++++----------- .../ProjectingMethodInterceptorUnitTests.java | 39 ++++++++++++------- .../ProxyProjectionFactoryUnitTests.java | 1 - .../data/web/MapDataBinderUnitTests.java | 2 - 6 files changed, 45 insertions(+), 64 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 94f07e004..40bb0b01b 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; @@ -48,6 +50,7 @@ class ProxyProjectionFactory implements ProjectionFactory, ResourceLoaderAware, ProxyProjectionFactory.class.getClassLoader()); private final List factories; + private final ConversionService conversionService; private ClassLoader classLoader; /** @@ -58,6 +61,8 @@ class ProxyProjectionFactory implements ProjectionFactory, ResourceLoaderAware, this.factories = new ArrayList(); this.factories.add(MapAccessingMethodInterceptorFactory.INSTANCE); this.factories.add(PropertyAccessingMethodInvokerFactory.INSTANCE); + + this.conversionService = new DefaultConversionService(); } /** @@ -175,7 +180,7 @@ class ProxyProjectionFactory implements ProjectionFactory, ResourceLoaderAware, .createMethodInterceptor(source, projectionType); 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..8c6b973da 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); diff --git a/src/test/java/org/springframework/data/projection/ProjectingMethodInterceptorUnitTests.java b/src/test/java/org/springframework/data/projection/ProjectingMethodInterceptorUnitTests.java index 5f869124b..67fe6f33f 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,11 +50,13 @@ public class ProjectingMethodInterceptorUnitTests { @Mock MethodInterceptor interceptor; @Mock MethodInvocation invocation; @Mock ProjectionFactory factory; + ConversionService conversionService = new DefaultConversionService(); @Test // DATAREST-221 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"); @@ -63,7 +67,7 @@ public class ProjectingMethodInterceptorUnitTests { @Test // DATAREST-221 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"); @@ -74,7 +78,7 @@ public class ProjectingMethodInterceptorUnitTests { @Test // DATAREST-221 public void returnsNullAsIs() throws Throwable { - MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(factory, interceptor); + MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(factory, interceptor, conversionService); when(interceptor.invoke(invocation)).thenReturn(null); @@ -84,7 +88,7 @@ public class ProjectingMethodInterceptorUnitTests { @Test // DATAREST-221 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); @@ -97,9 +101,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))); @@ -112,9 +117,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))); @@ -128,7 +134,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))); @@ -143,10 +150,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))); @@ -159,7 +167,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)); diff --git a/src/test/java/org/springframework/data/projection/ProxyProjectionFactoryUnitTests.java b/src/test/java/org/springframework/data/projection/ProxyProjectionFactoryUnitTests.java index 1670bce81..f7f840d12 100644 --- a/src/test/java/org/springframework/data/projection/ProxyProjectionFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/projection/ProxyProjectionFactoryUnitTests.java @@ -78,7 +78,6 @@ public class ProxyProjectionFactoryUnitTests { } @Test // DATAREST-221, DATACMNS-630 - @SuppressWarnings("rawtypes") public void proxyExposesTargetClassAware() { CustomerExcerpt proxy = factory.createProjection(CustomerExcerpt.class); diff --git a/src/test/java/org/springframework/data/web/MapDataBinderUnitTests.java b/src/test/java/org/springframework/data/web/MapDataBinderUnitTests.java index 9c965b04a..1f775ce26 100644 --- a/src/test/java/org/springframework/data/web/MapDataBinderUnitTests.java +++ b/src/test/java/org/springframework/data/web/MapDataBinderUnitTests.java @@ -57,7 +57,6 @@ public class MapDataBinderUnitTests { } @Test // DATACMNS-630 - @SuppressWarnings("rawtypes") public void bindsNestedCollectionElement() { MutablePropertyValues values = new MutablePropertyValues(); @@ -72,7 +71,6 @@ public class MapDataBinderUnitTests { } @Test // DATACMNS-630 - @SuppressWarnings("rawtypes") public void bindsNestedPrimitive() { MutablePropertyValues values = new MutablePropertyValues();