Skip to content

Commit 4dfb662

Browse files
bclozelbclozel
committed
Refactor input argument binding
Prior to this commit, `@Argument`-annotated controller method parameters were bound from the environment by serializing the environment argument map to JSON and deserialized back to the target parameter type. This allowed to cover a large spectrum of cases but showed some limitations: performance could be improved and custom scalars could be overridden in the process. This commit removes the (de)serialization process and instead: * runs through the environment argument map to collect all properties in a `MutablePropertyValues` map * instantiate the target type if necessary * binds properties to the target type using a `DataBinder` With this change, scalar types should not be erased and arguments now support a wider range of Collection types. Closes gh-122
1 parent 762f59b commit 4dfb662

File tree

3 files changed

+259
-159
lines changed

3 files changed

+259
-159
lines changed

‎spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedDataFetcherConfigurer.java

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,8 @@ public void setApplicationContext(ApplicationContext applicationContext) {
159159
@Override
160160
public void afterPropertiesSet() {
161161
this.argumentResolvers = new HandlerMethodArgumentResolverComposite();
162-
this.argumentResolvers.addResolver(initInputArgumentMethodArgumentResolver());
163162
this.argumentResolvers.addResolver(new ArgumentMapMethodArgumentResolver());
163+
this.argumentResolvers.addResolver(new ArgumentMethodArgumentResolver());
164164
this.argumentResolvers.addResolver(new DataFetchingEnvironmentMethodArgumentResolver());
165165
this.argumentResolvers.addResolver(new DataLoaderMethodArgumentResolver());
166166

@@ -172,22 +172,6 @@ public void afterPropertiesSet() {
172172
this.argumentResolvers.addResolver(new SourceMethodArgumentResolver());
173173
}
174174

175-
private ArgumentMethodArgumentResolver initInputArgumentMethodArgumentResolver() {
176-
ArgumentMethodArgumentResolver argumentResolver;
177-
if (this.jsonMessageConverter != null) {
178-
argumentResolver = new ArgumentMethodArgumentResolver(this.jsonMessageConverter);
179-
}
180-
else if (this.jsonEncoder != null && this.jsonDecoder != null) {
181-
argumentResolver = new ArgumentMethodArgumentResolver(this.jsonDecoder, this.jsonEncoder);
182-
}
183-
else {
184-
throw new IllegalArgumentException(
185-
"Neither HttpMessageConverter nor Encoder/Decoder for JSON provided");
186-
}
187-
return argumentResolver;
188-
}
189-
190-
191175
@Override
192176
public void configure(RuntimeWiring.Builder builder) {
193177
Assert.notNull(this.applicationContext, "ApplicationContext is required");

‎spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolver.java

Lines changed: 74 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -15,73 +15,45 @@
1515
*/
1616
package org.springframework.graphql.data.method.annotation.support;
1717

18-
import java.io.ByteArrayInputStream;
19-
import java.io.ByteArrayOutputStream;
20-
import java.io.IOException;
21-
import java.io.InputStream;
22-
import java.io.OutputStream;
23-
import java.util.Collections;
24-
import java.util.List;
18+
import java.lang.reflect.Constructor;
19+
import java.util.Collection;
20+
import java.util.Iterator;
21+
import java.util.Map;
2522
import java.util.Optional;
23+
import java.util.Stack;
2624

2725
import graphql.schema.DataFetchingEnvironment;
2826

27+
import org.springframework.beans.BeanUtils;
28+
import org.springframework.beans.MutablePropertyValues;
29+
import org.springframework.core.CollectionFactory;
2930
import org.springframework.core.MethodParameter;
30-
import org.springframework.core.ResolvableType;
31-
import org.springframework.core.codec.Decoder;
32-
import org.springframework.core.codec.Encoder;
33-
import org.springframework.core.io.buffer.DataBuffer;
34-
import org.springframework.core.io.buffer.DefaultDataBufferFactory;
31+
import org.springframework.core.convert.TypeDescriptor;
3532
import org.springframework.graphql.data.method.HandlerMethodArgumentResolver;
3633
import org.springframework.graphql.data.method.annotation.Argument;
3734
import org.springframework.graphql.data.method.annotation.ValueConstants;
38-
import org.springframework.http.HttpHeaders;
39-
import org.springframework.http.HttpInputMessage;
40-
import org.springframework.http.HttpOutputMessage;
41-
import org.springframework.http.MediaType;
42-
import org.springframework.http.converter.GenericHttpMessageConverter;
43-
import org.springframework.lang.Nullable;
4435
import org.springframework.util.Assert;
45-
import org.springframework.util.MimeTypeUtils;
4636
import org.springframework.util.StringUtils;
37+
import org.springframework.validation.DataBinder;
4738

4839
/**
4940
* Resolver for {@link Argument @Argument} annotated method parameters, obtained
5041
* via {@link DataFetchingEnvironment#getArgument(String)} and converted to the
5142
* declared type of the method parameter.
5243
*
5344
* @author Rossen Stoyanchev
45+
* @author Brian Clozel
5446
* @since 1.0.0
5547
*/
5648
public class ArgumentMethodArgumentResolver implements HandlerMethodArgumentResolver {
5749

58-
private final ArgumentConverter argumentConverter;
59-
60-
/**
61-
* Constructor with an
62-
* {@link org.springframework.http.converter.HttpMessageConverter} to convert
63-
* Map-based input arguments to higher level Objects.
64-
*/
65-
public ArgumentMethodArgumentResolver(GenericHttpMessageConverter<Object> converter) {
66-
this.argumentConverter = new MessageConverterArgumentConverter(converter);
67-
}
68-
69-
/**
70-
* Variant of
71-
* {@link #ArgumentMethodArgumentResolver(GenericHttpMessageConverter)}
72-
* to use an {@link Encoder} and {@link Decoder} to convert input arguments.
73-
*/
74-
public ArgumentMethodArgumentResolver(Decoder<Object> decoder, Encoder<Object> encoder) {
75-
this.argumentConverter = new CodecArgumentConverter(decoder, encoder);
76-
}
77-
78-
7950
@Override
8051
public boolean supportsParameter(MethodParameter parameter) {
8152
return parameter.getParameterAnnotation(Argument.class) != null;
8253
}
8354

8455
@Override
56+
@SuppressWarnings("unchecked")
8557
public Object resolveArgument(MethodParameter parameter, DataFetchingEnvironment environment) throws Exception {
8658
Argument annotation = parameter.getParameterAnnotation(Argument.class);
8759
Assert.notNull(annotation, "No @Argument annotation");
@@ -99,136 +71,96 @@ public Object resolveArgument(MethodParameter parameter, DataFetchingEnvironment
9971
environment.getArgument(name) :
10072
environment.getArgumentOrDefault(name, annotation.defaultValue()));
10173

102-
Class<?> parameterType = parameter.getParameterType();
74+
TypeDescriptor parameterType = new TypeDescriptor(parameter);
10375

10476
if (rawValue == null) {
10577
if (annotation.required()) {
10678
throw new MissingArgumentException(name, parameter);
10779
}
108-
if (parameterType.equals(Optional.class)) {
80+
if (parameterType.getType().equals(Optional.class)) {
10981
return Optional.empty();
11082
}
11183
return null;
11284
}
11385

114-
if (parameterType.isAssignableFrom(rawValue.getClass())) {
115-
return returnValue(rawValue, parameterType);
86+
if (CollectionFactory.isApproximableCollectionType(rawValue.getClass())) {
87+
Assert.isAssignable(Collection.class, parameterType.getType(),
88+
"Argument '" + name + "' is a Collection while the @Argument method parameter is " + parameterType.getType());
89+
Collection<Object> rawCollection = (Collection<Object>) rawValue;
90+
Collection<Object> values = CollectionFactory.createApproximateCollection(rawValue, rawCollection.size());
91+
Class<?> elementType = parameterType.getElementTypeDescriptor().getType();
92+
rawCollection.forEach(item -> values.add(convert(item, elementType)));
93+
return values;
11694
}
11795

118-
if (rawValue instanceof List) {
119-
Assert.isAssignable(List.class, parameterType,
120-
"Argument '" + name + "' is a List while the @Argument method parameter is " + parameterType);
121-
List<?> valueList = (List<?>) rawValue;
122-
Class<?> elementType = parameter.nestedIfOptional().getNestedParameterType();
123-
if (valueList.isEmpty() || elementType.isAssignableFrom(valueList.get(0).getClass())) {
124-
return returnValue(rawValue, parameterType);
125-
}
126-
}
127-
128-
Object decodedValue = this.argumentConverter.convert(rawValue, parameter);
129-
Assert.notNull(decodedValue, "Argument '" + name + "' with raw value '" + rawValue + "'was decoded to null");
130-
return returnValue(decodedValue, parameterType);
96+
MethodParameter nestedParameter = parameter.nestedIfOptional();
97+
Object value = convert(rawValue, nestedParameter.getNestedParameterType());
98+
return returnValue(value, parameterType.getType());
13199
}
132100

133101
private Object returnValue(Object value, Class<?> parameterType) {
134102
return (parameterType.equals(Optional.class) ? Optional.of(value) : value);
135103
}
136104

137-
138-
/**
139-
* Contract to abstract use of an HttpMessageConverter vs Encoder/Decoder.
140-
*/
141-
private interface ArgumentConverter {
142-
143-
@Nullable
144-
Object convert(Object rawValue, MethodParameter targetParameter) throws Exception;
145-
146-
}
147-
148-
149-
/**
150-
* HttpMessageConverter based implementation of ArgumentConverter.
151-
*/
152-
private static class MessageConverterArgumentConverter implements ArgumentConverter {
153-
154-
private final GenericHttpMessageConverter<Object> converter;
155-
156-
public MessageConverterArgumentConverter(GenericHttpMessageConverter<Object> converter) {
157-
this.converter = converter;
105+
@SuppressWarnings("unchecked")
106+
private Object convert(Object rawValue, Class<?> targetType) {
107+
Object target;
108+
if (rawValue instanceof Map) {
109+
Constructor<?> ctor = BeanUtils.getResolvableConstructor(targetType);
110+
target = BeanUtils.instantiateClass(ctor);
111+
DataBinder dataBinder = new DataBinder(target);
112+
Assert.isTrue(ctor.getParameterCount() == 0,
113+
() -> "Argument of type [" + targetType.getName() +
114+
"] cannot be instantiated because of missing default constructor.");
115+
MutablePropertyValues mpvs = extractPropertyValues((Map) rawValue);
116+
dataBinder.bind(mpvs);
158117
}
159-
160-
@Override
161-
public Object convert(Object rawValue, MethodParameter targetParameter) throws IOException {
162-
HttpOutputMessageAdapter outMessage = new HttpOutputMessageAdapter();
163-
this.converter.write(rawValue, MediaType.APPLICATION_JSON, outMessage);
164-
165-
HttpInputMessageAdapter inMessage = new HttpInputMessageAdapter(outMessage);
166-
return this.converter.read(targetParameter.getGenericParameterType(), rawValue.getClass(), inMessage);
118+
else if (targetType.isAssignableFrom(rawValue.getClass())) {
119+
return returnValue(rawValue, targetType);
167120
}
168-
}
169-
170-
171-
/**
172-
* Encoder/Decoder based implementation of ArgumentConverter.
173-
*/
174-
private static class CodecArgumentConverter implements ArgumentConverter {
175-
176-
private final Decoder<Object> decoder;
177-
178-
private final Encoder<Object> encoder;
179-
180-
public CodecArgumentConverter(Decoder<Object> decoder, Encoder<Object> encoder) {
181-
Assert.notNull(decoder, "Decoder is required");
182-
Assert.notNull(encoder, "Encoder is required");
183-
this.decoder = decoder;
184-
this.encoder = encoder;
185-
}
186-
187-
@Override
188-
public Object convert(Object rawValue, MethodParameter targetParameter) {
189-
DataBuffer dataBuffer = this.encoder.encodeValue(
190-
rawValue, DefaultDataBufferFactory.sharedInstance, ResolvableType.forInstance(rawValue),
191-
MimeTypeUtils.APPLICATION_JSON, Collections.emptyMap());
192-
193-
return this.decoder.decode(
194-
dataBuffer, ResolvableType.forMethodParameter(targetParameter.nestedIfOptional()),
195-
MimeTypeUtils.APPLICATION_JSON, Collections.emptyMap());
121+
else {
122+
DataBinder converter = new DataBinder(null);
123+
target = converter.convertIfNecessary(rawValue, targetType);
124+
Assert.isTrue(target != null,
125+
() -> "Value of type [" + rawValue.getClass() + "] cannot be converted to argument of type [" +
126+
targetType.getName() + "].");
196127
}
128+
return target;
197129
}
198130

199-
200-
private static class HttpInputMessageAdapter extends ByteArrayInputStream implements HttpInputMessage {
201-
202-
HttpInputMessageAdapter(HttpOutputMessageAdapter messageAdapter) {
203-
super(messageAdapter.toByteArray());
204-
}
205-
206-
@Override
207-
public InputStream getBody() {
208-
return this;
209-
}
210-
211-
@Override
212-
public HttpHeaders getHeaders() {
213-
return HttpHeaders.EMPTY;
214-
}
215-
131+
private MutablePropertyValues extractPropertyValues(Map<String, Object> arguments) {
132+
MutablePropertyValues mpvs = new MutablePropertyValues();
133+
Stack<String> path = new Stack<>();
134+
visitArgumentMap(arguments, mpvs, path);
135+
return mpvs;
216136
}
217137

218-
private static class HttpOutputMessageAdapter extends ByteArrayOutputStream implements HttpOutputMessage {
219-
220-
private static final HttpHeaders noOpHeaders = new HttpHeaders();
221-
222-
@Override
223-
public OutputStream getBody() {
224-
return this;
138+
@SuppressWarnings("unchecked")
139+
private void visitArgumentMap(Map<String, Object> arguments, MutablePropertyValues mpvs, Stack<String> path) {
140+
for (String key : arguments.keySet()) {
141+
path.push(key);
142+
Object value = arguments.get(key);
143+
if (value instanceof Map) {
144+
visitArgumentMap((Map<String, Object>) value, mpvs, path);
145+
}
146+
else {
147+
String propertyName = pathToPropertyName(path);
148+
mpvs.add(propertyName, value);
149+
}
150+
path.pop();
225151
}
152+
}
226153

227-
@Override
228-
public HttpHeaders getHeaders() {
229-
return noOpHeaders;
154+
private String pathToPropertyName(Stack<String> path) {
155+
StringBuilder sb = new StringBuilder();
156+
Iterator<String> it = path.iterator();
157+
while (it.hasNext()) {
158+
sb.append(it.next());
159+
if (it.hasNext()) {
160+
sb.append(".");
161+
}
230162
}
231-
163+
return sb.toString();
232164
}
233165

234166
}

0 commit comments

Comments
 (0)