Skip to content

Commit f44255b

Browse files
odrotbohmodrotbohm
committed
DATACMNS-1102 - Reuse ConversionService in ProjectingMethodInterceptors created by ProxyProjectionFactory.
Applied the same to internals of MapDataBinder.
1 parent 3dbbb15 commit f44255b

File tree

6 files changed

+43
-60
lines changed

6 files changed

+43
-60
lines changed

‎src/main/java/org/springframework/data/projection/ProjectingMethodInterceptor.java

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
*/
1616
package org.springframework.data.projection;
1717

18+
import lombok.NonNull;
19+
import lombok.RequiredArgsConstructor;
20+
1821
import java.lang.reflect.Array;
1922
import java.util.Arrays;
2023
import java.util.Collection;
@@ -27,7 +30,6 @@
2730
import org.aopalliance.intercept.MethodInvocation;
2831
import org.springframework.core.CollectionFactory;
2932
import org.springframework.core.convert.ConversionService;
30-
import org.springframework.core.convert.support.DefaultConversionService;
3133
import org.springframework.data.util.ClassTypeInformation;
3234
import org.springframework.data.util.TypeInformation;
3335
import org.springframework.util.Assert;
@@ -41,29 +43,12 @@
4143
* @author Oliver Gierke
4244
* @since 1.10
4345
*/
46+
@RequiredArgsConstructor
4447
class ProjectingMethodInterceptor implements MethodInterceptor {
4548

46-
private final ProjectionFactory factory;
47-
private final MethodInterceptor delegate;
48-
private final ConversionService conversionService;
49-
50-
/**
51-
* Creates a new {@link ProjectingMethodInterceptor} using the given {@link ProjectionFactory} and delegate
52-
* {@link MethodInterceptor}.
53-
*
54-
* @param factory the {@link ProjectionFactory} to use to create projections if types do not match, must not be
55-
* {@literal null}..
56-
* @param delegate the {@link MethodInterceptor} to trigger to create the source value, must not be {@literal null}..
57-
*/
58-
public ProjectingMethodInterceptor(ProjectionFactory factory, MethodInterceptor delegate) {
59-
60-
Assert.notNull(factory, "ProjectionFactory must not be null!");
61-
Assert.notNull(delegate, "Delegate MethodInterceptor must not be null!");
62-
63-
this.factory = factory;
64-
this.delegate = delegate;
65-
this.conversionService = new DefaultConversionService();
66-
}
49+
private final @NonNull ProjectionFactory factory;
50+
private final @NonNull MethodInterceptor delegate;
51+
private final @NonNull ConversionService conversionService;
6752

6853
/*
6954
* (non-Javadoc)

‎src/main/java/org/springframework/data/projection/ProxyProjectionFactory.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import org.springframework.aop.framework.ProxyFactory;
2929
import org.springframework.beans.factory.BeanClassLoaderAware;
3030
import org.springframework.context.ResourceLoaderAware;
31+
import org.springframework.core.convert.ConversionService;
32+
import org.springframework.core.convert.support.DefaultConversionService;
3133
import org.springframework.core.io.ResourceLoader;
3234
import org.springframework.util.Assert;
3335
import org.springframework.util.ClassUtils;
@@ -49,6 +51,7 @@ class ProxyProjectionFactory implements ProjectionFactory, ResourceLoaderAware,
4951
ProxyProjectionFactory.class.getClassLoader());
5052

5153
private final List<MethodInterceptorFactory> factories;
54+
private final ConversionService conversionService;
5255
private ClassLoader classLoader;
5356

5457
/**
@@ -59,6 +62,8 @@ protected ProxyProjectionFactory() {
5962
this.factories = new ArrayList<>();
6063
this.factories.add(MapAccessingMethodInterceptorFactory.INSTANCE);
6164
this.factories.add(PropertyAccessingMethodInvokerFactory.INSTANCE);
65+
66+
this.conversionService = new DefaultConversionService();
6267
}
6368

6469
/**
@@ -176,7 +181,7 @@ private MethodInterceptor getMethodInterceptor(Object source, Class<?> projectio
176181
.createMethodInterceptor(source, projectionType);
177182

178183
return new ProjectingMethodInterceptor(this,
179-
postProcessAccessorInterceptor(propertyInvocationInterceptor, source, projectionType));
184+
postProcessAccessorInterceptor(propertyInvocationInterceptor, source, projectionType), conversionService);
180185
}
181186

182187
/**

‎src/main/java/org/springframework/data/web/MapDataBinder.java

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
*/
1616
package org.springframework.data.web;
1717

18+
import lombok.NonNull;
19+
import lombok.RequiredArgsConstructor;
20+
1821
import java.beans.PropertyDescriptor;
1922
import java.util.HashMap;
2023
import java.util.List;
@@ -31,7 +34,6 @@
3134
import org.springframework.core.MethodParameter;
3235
import org.springframework.core.convert.ConversionService;
3336
import org.springframework.core.convert.TypeDescriptor;
34-
import org.springframework.core.convert.support.DefaultConversionService;
3537
import org.springframework.data.mapping.PropertyPath;
3638
import org.springframework.data.mapping.PropertyReferenceException;
3739
import org.springframework.data.util.TypeInformation;
@@ -97,32 +99,15 @@ protected ConfigurablePropertyAccessor getPropertyAccessor() {
9799
* @author Oliver Gierke
98100
* @since 1.10
99101
*/
102+
@RequiredArgsConstructor
100103
private static class MapPropertyAccessor extends AbstractPropertyAccessor {
101104

102105
private static final SpelExpressionParser PARSER = new SpelExpressionParser(
103106
new SpelParserConfiguration(false, true));
104107

105-
private final Class<?> type;
106-
private final Map<String, Object> map;
107-
private final ConversionService conversionService;
108-
109-
/**
110-
* Creates a new {@link MapPropertyAccessor} for the given type, map and {@link ConversionService}.
111-
*
112-
* @param type must not be {@literal null}.
113-
* @param map must not be {@literal null}.
114-
* @param conversionService must not be {@literal null}.
115-
*/
116-
public MapPropertyAccessor(Class<?> type, Map<String, Object> map, ConversionService conversionService) {
117-
118-
Assert.notNull(type, "Type must not be null!");
119-
Assert.notNull(map, "Map must not be null!");
120-
Assert.notNull(conversionService, "ConversionService must not be null!");
121-
122-
this.type = type;
123-
this.map = map;
124-
this.conversionService = conversionService;
125-
}
108+
private final @NonNull Class<?> type;
109+
private final @NonNull Map<String, Object> map;
110+
private final @NonNull ConversionService conversionService;
126111

127112
/*
128113
* (non-Javadoc)
@@ -177,7 +162,7 @@ public void setPropertyValue(String propertyName, Object value) throws BeansExce
177162
}
178163

179164
StandardEvaluationContext context = new StandardEvaluationContext();
180-
context.addPropertyAccessor(new PropertyTraversingMapAccessor(type, new DefaultConversionService()));
165+
context.addPropertyAccessor(new PropertyTraversingMapAccessor(type, conversionService));
181166
context.setTypeConverter(new StandardTypeConverter(conversionService));
182167
context.setRootObject(map);
183168

@@ -290,7 +275,8 @@ private TypeDescriptor getDescriptor(PropertyPath path, Object emptyValue) {
290275
Class<?> actualPropertyType = path.getType();
291276

292277
TypeDescriptor valueDescriptor = conversionService.canConvert(String.class, actualPropertyType)
293-
? TypeDescriptor.valueOf(String.class) : TypeDescriptor.valueOf(HashMap.class);
278+
? TypeDescriptor.valueOf(String.class)
279+
: TypeDescriptor.valueOf(HashMap.class);
294280

295281
return path.isCollection() ? TypeDescriptor.collection(emptyValue.getClass(), valueDescriptor)
296282
: TypeDescriptor.map(emptyValue.getClass(), TypeDescriptor.valueOf(String.class), valueDescriptor);

‎src/test/java/org/springframework/data/projection/ProjectingMethodInterceptorUnitTests.java

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.springframework.data.projection;
1717

1818
import static org.assertj.core.api.Assertions.*;
19+
import static org.mockito.ArgumentMatchers.*;
1920
import static org.mockito.Mockito.*;
2021

2122
import java.util.Collection;
@@ -30,6 +31,8 @@
3031
import org.junit.runner.RunWith;
3132
import org.mockito.Mock;
3233
import org.mockito.junit.MockitoJUnitRunner;
34+
import org.springframework.core.convert.ConversionService;
35+
import org.springframework.core.convert.support.DefaultConversionService;
3336

3437
/**
3538
* Unit tests for {@link ProjectingMethodInterceptor}.
@@ -43,11 +46,13 @@ public class ProjectingMethodInterceptorUnitTests {
4346
@Mock MethodInterceptor interceptor;
4447
@Mock MethodInvocation invocation;
4548
@Mock ProjectionFactory factory;
49+
ConversionService conversionService = new DefaultConversionService();
4650

4751
@Test // DATAREST-221
4852
public void wrapsDelegateResultInProxyIfTypesDontMatch() throws Throwable {
4953

50-
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor);
54+
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor,
55+
conversionService);
5156

5257
when(invocation.getMethod()).thenReturn(Helper.class.getMethod("getHelper"));
5358
when(interceptor.invoke(invocation)).thenReturn("Foo");
@@ -58,7 +63,7 @@ public void wrapsDelegateResultInProxyIfTypesDontMatch() throws Throwable {
5863
@Test // DATAREST-221
5964
public void retunsDelegateResultAsIsIfTypesMatch() throws Throwable {
6065

61-
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(factory, interceptor);
66+
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(factory, interceptor, conversionService);
6267

6368
when(invocation.getMethod()).thenReturn(Helper.class.getMethod("getString"));
6469
when(interceptor.invoke(invocation)).thenReturn("Foo");
@@ -69,7 +74,7 @@ public void retunsDelegateResultAsIsIfTypesMatch() throws Throwable {
6974
@Test // DATAREST-221
7075
public void returnsNullAsIs() throws Throwable {
7176

72-
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(factory, interceptor);
77+
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(factory, interceptor, conversionService);
7378

7479
when(interceptor.invoke(invocation)).thenReturn(null);
7580

@@ -79,20 +84,21 @@ public void returnsNullAsIs() throws Throwable {
7984
@Test // DATAREST-221
8085
public void considersPrimitivesAsWrappers() throws Throwable {
8186

82-
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(factory, interceptor);
87+
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(factory, interceptor, conversionService);
8388

8489
when(invocation.getMethod()).thenReturn(Helper.class.getMethod("getPrimitive"));
8590
when(interceptor.invoke(invocation)).thenReturn(1L);
8691

8792
assertThat(methodInterceptor.invoke(invocation)).isEqualTo(1L);
88-
verify(factory, times(0)).createProjection((Class<?>) anyObject(), anyObject());
93+
verify(factory, times(0)).createProjection((Class<?>) any(), any());
8994
}
9095

9196
@Test // DATAREST-394, DATAREST-408
9297
@SuppressWarnings("unchecked")
9398
public void appliesProjectionToNonEmptySets() throws Throwable {
9499

95-
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor);
100+
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor,
101+
conversionService);
96102
Object result = methodInterceptor
97103
.invoke(mockInvocationOf("getHelperCollection", Collections.singleton(mock(Helper.class))));
98104

@@ -107,7 +113,8 @@ public void appliesProjectionToNonEmptySets() throws Throwable {
107113
@SuppressWarnings("unchecked")
108114
public void appliesProjectionToNonEmptyLists() throws Throwable {
109115

110-
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor);
116+
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor,
117+
conversionService);
111118
Object result = methodInterceptor
112119
.invoke(mockInvocationOf("getHelperList", Collections.singletonList(mock(Helper.class))));
113120

@@ -123,7 +130,8 @@ public void appliesProjectionToNonEmptyLists() throws Throwable {
123130
@SuppressWarnings("unchecked")
124131
public void allowsMaskingAnArrayIntoACollection() throws Throwable {
125132

126-
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor);
133+
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor,
134+
conversionService);
127135
Object result = methodInterceptor.invoke(mockInvocationOf("getHelperArray", new Helper[] { mock(Helper.class) }));
128136

129137
assertThat(result).isInstanceOf(Collection.class);
@@ -138,7 +146,8 @@ public void allowsMaskingAnArrayIntoACollection() throws Throwable {
138146
@SuppressWarnings("unchecked")
139147
public void appliesProjectionToNonEmptyMap() throws Throwable {
140148

141-
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor);
149+
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor,
150+
conversionService);
142151

143152
Object result = methodInterceptor
144153
.invoke(mockInvocationOf("getHelperMap", Collections.singletonMap("foo", mock(Helper.class))));
@@ -154,7 +163,8 @@ public void appliesProjectionToNonEmptyMap() throws Throwable {
154163
@Test
155164
public void returnsSingleElementCollectionForTargetThatReturnsNonCollection() throws Throwable {
156165

157-
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor);
166+
MethodInterceptor methodInterceptor = new ProjectingMethodInterceptor(new ProxyProjectionFactory(), interceptor,
167+
conversionService);
158168

159169
Helper reference = mock(Helper.class);
160170
Object result = methodInterceptor.invoke(mockInvocationOf("getHelperCollection", reference));

‎src/test/java/org/springframework/data/projection/ProxyProjectionFactoryUnitTests.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ public void createsProjectingProxy() {
7777
}
7878

7979
@Test // DATAREST-221, DATACMNS-630
80-
@SuppressWarnings("rawtypes")
8180
public void proxyExposesTargetClassAware() {
8281

8382
CustomerExcerpt proxy = factory.createProjection(CustomerExcerpt.class);

‎src/test/java/org/springframework/data/web/MapDataBinderUnitTests.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ public void honorsFormattingAnnotationOnAccessor() {
5656
}
5757

5858
@Test // DATACMNS-630
59-
@SuppressWarnings("rawtypes")
6059
public void bindsNestedCollectionElement() {
6160

6261
MutablePropertyValues values = new MutablePropertyValues();
@@ -71,7 +70,6 @@ public void bindsNestedCollectionElement() {
7170
}
7271

7372
@Test // DATACMNS-630
74-
@SuppressWarnings("rawtypes")
7573
public void bindsNestedPrimitive() {
7674

7775
MutablePropertyValues values = new MutablePropertyValues();

0 commit comments

Comments
 (0)