Spring/spring-graphql
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Polish "Support contructor binding for input arguments"

Browse Source 
Prior to this commit, only default constructors were supported for
instantiating input argument types. This commit adds a new
`GraphQlInstantiator` class that manages the instantiation and binding
of data fetching environment arguments. Default constructors and
primary constructors are now handled.

Also, List-like properties were not bound from the data fetching
environment arguments to the `MutablePropertyValues` used for binding.
This commit ensures that List elements are bound to the property values
with array-like property paths.

Fixes gh-139
Fixes gh-141
This commit is contained in:
Brian Clozel
2021-09-21 09:45:45 +02:00
parent 2f5aa588ef
commit 02b16bdc65
7 changed files with 333 additions and 126 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
build.gradle
View File

@@ -1,5 +1,6 @@
plugins {
id 'io.spring.dependency-management' version '1.0.11.RELEASE'
id 'org.jetbrains.kotlin.jvm' version '1.5.31' apply false
}
ext {
@@ -35,6 +36,24 @@ configure(moduleProjects) {
targetCompatibility = JavaVersion.VERSION_1_8
}
pluginManager.withPlugin("kotlin") {
compileKotlin {
kotlinOptions {
jvmTarget = "1.8"
languageVersion = "1.3"
apiVersion = "1.3"
freeCompilerArgs = ["-Xjsr305=strict", "-Xsuppress-version-warnings", "-Xopt-in=kotlin.RequiresOptIn"]
allWarningsAsErrors = true
}
}
compileTestKotlin {
kotlinOptions {
jvmTarget = "1.8"
freeCompilerArgs = ["-Xjsr305=strict"]
}
}
}
dependencyManagement {
imports {
mavenBom "com.fasterxml.jackson:jackson-bom:2.12.5"
@@ -42,6 +61,7 @@ configure(moduleProjects) {
mavenBom "org.springframework:spring-framework-bom:5.3.9"
mavenBom "org.springframework.data:spring-data-bom:2021.0.4"
mavenBom "org.springframework.security:spring-security-bom:5.5.2"
mavenBom "org.jetbrains.kotlin:kotlin-bom:1.5.31"
mavenBom "org.jetbrains.kotlinx:kotlinx-coroutines-bom:1.5.2"
mavenBom "org.junit:junit-bom:5.7.2"
}

19
spring-graphql/build.gradle
View File

@@ -1,8 +1,7 @@
plugins {
id 'org.jetbrains.kotlin.jvm' version '1.5.31'
}
description = "GraphQL Support for Spring Applications"
apply plugin: "kotlin"
dependencies {
api 'com.graphql-java:graphql-java'
api 'io.projectreactor:reactor-core'
@@ -21,6 +20,7 @@ dependencies {
compileOnly 'com.google.code.findbugs:jsr305'
compileOnly 'org.jetbrains.kotlinx:kotlinx-coroutines-core'
compileOnly 'org.jetbrains.kotlin:kotlin-stdlib'
testImplementation 'org.junit.jupiter:junit-jupiter'
testImplementation 'org.assertj:assertj-core'
@@ -46,16 +46,3 @@ test {
events "passed", "skipped", "failed"
}
}
repositories {
mavenCentral()
}
compileKotlin {
kotlinOptions {
jvmTarget = "1.8"
}
}
compileTestKotlin {
kotlinOptions {
jvmTarget = "1.8"
}
}

81
spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolver.java
View File

@@ -15,18 +15,12 @@
*/
package org.springframework.graphql.data.method.annotation.support;
import java.lang.reflect.Constructor;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Stack;
import graphql.schema.DataFetchingEnvironment;
import org.springframework.beans.BeanUtils;
import org.springframework.beans.MutablePropertyValues;
import org.springframework.core.CollectionFactory;
import org.springframework.core.MethodParameter;
import org.springframework.core.convert.TypeDescriptor;
@@ -48,6 +42,8 @@ import org.springframework.validation.DataBinder;
*/
public class ArgumentMethodArgumentResolver implements HandlerMethodArgumentResolver {
private final GraphQlArgumentInstantiator instantiator = new GraphQlArgumentInstantiator();
@Override
public boolean supportsParameter(MethodParameter parameter) {
return parameter.getParameterAnnotation(Argument.class) != null;
@@ -78,10 +74,7 @@ public class ArgumentMethodArgumentResolver implements HandlerMethodArgumentReso
if (annotation.required()) {
throw new MissingArgumentException(name, parameter);
}
if (parameterType.getType().equals(Optional.class)) {
return Optional.empty();
}
return null;
returnValue(rawValue, parameterType.getType());
}
if (CollectionFactory.isApproximableCollectionType(rawValue.getClass())) {
@@ -100,40 +93,17 @@ public class ArgumentMethodArgumentResolver implements HandlerMethodArgumentReso
}
private Object returnValue(Object value, Class<?> parameterType) {
return (parameterType.equals(Optional.class) ? Optional.of(value) : value);
if (parameterType.equals(Optional.class)) {
return Optional.ofNullable(value);
}
return value;
}
@SuppressWarnings("unchecked")
private Object convert(Object rawValue, Class<?> targetType) {
Object target;
if (rawValue instanceof Map) {
Constructor<?> ctor = BeanUtils.getResolvableConstructor(targetType);
MutablePropertyValues propertyValues = extractPropertyValues((Map) rawValue);
if (ctor.getParameterCount() == 0) {
target = BeanUtils.instantiateClass(ctor);
DataBinder dataBinder = new DataBinder(target);
dataBinder.bind(propertyValues);
} else {
// Data class constructor
DataBinder binder = new DataBinder(null);
String[] paramNames = BeanUtils.getParameterNames(ctor);
Class<?>[] paramTypes = ctor.getParameterTypes();
Object[] args = new Object[paramTypes.length];
for (int i = 0; i < paramNames.length; i++) {
String paramName = paramNames[i];
Object value = propertyValues.get(paramName);
value = (value instanceof List ? ((List<?>) value).toArray() : value);
MethodParameter methodParam = new MethodParameter(ctor, i);
if (value == null && methodParam.isOptional()) {
args[i] = (methodParam.getParameterType() == Optional.class ? Optional.empty() : null);
}
else {
args[i] = binder.convertIfNecessary(value, paramTypes[i], methodParam);
}
}
target = BeanUtils.instantiateClass(ctor, args);
}
target = this.instantiator.instantiate(targetType, (Map<String, Object>) rawValue);
}
else if (targetType.isAssignableFrom(rawValue.getClass())) {
return returnValue(rawValue, targetType);
@@ -148,39 +118,4 @@ public class ArgumentMethodArgumentResolver implements HandlerMethodArgumentReso
return target;
}
private MutablePropertyValues extractPropertyValues(Map<String, Object> arguments) {
MutablePropertyValues mpvs = new MutablePropertyValues();
Stack<String> path = new Stack<>();
visitArgumentMap(arguments, mpvs, path);
return mpvs;
}
@SuppressWarnings("unchecked")
private void visitArgumentMap(Map<String, Object> arguments, MutablePropertyValues mpvs, Stack<String> path) {
for (String key : arguments.keySet()) {
path.push(key);
Object value = arguments.get(key);
if (value instanceof Map) {
visitArgumentMap((Map<String, Object>) value, mpvs, path);
}
else {
String propertyName = pathToPropertyName(path);
mpvs.add(propertyName, value);
}
path.pop();
}
}
private String pathToPropertyName(Stack<String> path) {
StringBuilder sb = new StringBuilder();
Iterator<String> it = path.iterator();
while (it.hasNext()) {
sb.append(it.next());
if (it.hasNext()) {
sb.append(".");
}
}
return sb.toString();
}
}

131
spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiator.java Normal file
View File

@@ -0,0 +1,131 @@
/*
* Copyright 2020-2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.graphql.data.method.annotation.support;
import java.lang.reflect.Constructor;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Stack;
import org.springframework.beans.BeanUtils;
import org.springframework.beans.MutablePropertyValues;
import org.springframework.core.MethodParameter;
import org.springframework.validation.DataBinder;
/**
* Instantiate a target type and bind data from
* {@link graphql.schema.DataFetchingEnvironment} arguments.
*
* @author Brian Clozel
*/
class GraphQlArgumentInstantiator {
/**
* Instantiate the given target type and bind data from
* {@link graphql.schema.DataFetchingEnvironment} arguments.
* <p>This is considering the default constructor or a primary constructor
* if available.
*
* @param targetType the type of the argument to instantiate
* @param arguments the data fetching environment arguments
* @param <T> the type of the input argument
* @return the instantiated and populated input argument.
* @throws IllegalStateException if there is no suitable constructor.
*/
@SuppressWarnings("unchecked")
public <T> T instantiate(Class<T> targetType, Map<String, Object> arguments) {
Object target;
Constructor<?> ctor = BeanUtils.getResolvableConstructor(targetType);
MutablePropertyValues propertyValues = extractPropertyValues(arguments);
if (ctor.getParameterCount() == 0) {
target = BeanUtils.instantiateClass(ctor);
DataBinder dataBinder = new DataBinder(target);
dataBinder.bind(propertyValues);
}
else {
// Data class constructor
DataBinder binder = new DataBinder(null);
String[] paramNames = BeanUtils.getParameterNames(ctor);
Class<?>[] paramTypes = ctor.getParameterTypes();
Object[] args = new Object[paramTypes.length];
for (int i = 0; i < paramNames.length; i++) {
String paramName = paramNames[i];
Object value = propertyValues.get(paramName);
value = (value instanceof List ? ((List<?>) value).toArray() : value);
MethodParameter methodParam = new MethodParameter(ctor, i);
if (value == null && methodParam.isOptional()) {
args[i] = (methodParam.getParameterType() == Optional.class ? Optional.empty() : null);
}
else {
args[i] = binder.convertIfNecessary(value, paramTypes[i], methodParam);
}
}
target = BeanUtils.instantiateClass(ctor, args);
}
return (T) target;
}
/**
* Perform a Depth First Search in the given JSON map to collect attribute values
* as {@link MutablePropertyValues} using the full property path as key.
*/
private MutablePropertyValues extractPropertyValues(Map<String, Object> arguments) {
MutablePropertyValues mpvs = new MutablePropertyValues();
Stack<String> path = new Stack<>();
visitArgumentMap(arguments, mpvs, path);
return mpvs;
}
@SuppressWarnings("unchecked")
private void visitArgumentMap(Map<String, Object> arguments, MutablePropertyValues mpvs, Stack<String> path) {
for (String key : arguments.keySet()) {
Object value = arguments.get(key);
if (value instanceof List) {
List<Object> items = (List<Object>) value;
Map<String, Object> subValues = new HashMap<>(items.size());
for (int i = 0; i < items.size(); i++) {
subValues.put(key + "[" + i + "]", items.get(i));
}
visitArgumentMap(subValues, mpvs, path);
}
else if (value instanceof Map) {
path.push(key);
path.push(".");
visitArgumentMap((Map<String, Object>) value, mpvs, path);
path.pop();
path.pop();
}
else {
path.push(key);
String propertyName = pathToPropertyName(path);
mpvs.add(propertyName, value);
path.pop();
}
}
}
private String pathToPropertyName(Stack<String> path) {
StringBuilder sb = new StringBuilder();
for (String s : path) {
sb.append(s);
}
return sb.toString();
}
}

42
spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolverTests.java
View File

@@ -87,15 +87,13 @@ class ArgumentMethodArgumentResolverTests {
}
@Test
void shouldResolveKotlinBeanArgument() throws Exception {
Method addBook = ClassUtils.getMethod(BookController.class, "ktAddBook", KotlinBookInput.class);
String payload = "{\"bookInput\": { \"name\": \"test name\", \"authorId\": 42} }";
void shouldResolveDefaultValue() throws Exception {
Method findWithDefault = ClassUtils.getMethod(BookController.class, "findWithDefault", Long.class);
String payload = "{\"name\": \"test\" }";
DataFetchingEnvironment environment = initEnvironment(payload);
MethodParameter methodParameter = getMethodParameter(addBook, 0);
MethodParameter methodParameter = getMethodParameter(findWithDefault, 0);
Object result = resolver.resolveArgument(methodParameter, environment);
assertThat(result).isNotNull().isInstanceOf(KotlinBookInput.class);
assertThat((KotlinBookInput) result).hasFieldOrPropertyWithValue("name", "test name")
.hasFieldOrPropertyWithValue("authorId", 42L);
assertThat(result).isNotNull().isInstanceOf(Long.class).isEqualTo(42L);
}
@Test
@@ -110,20 +108,6 @@ class ArgumentMethodArgumentResolverTests {
.extracting("name").containsExactly("first", "second");
}
@Test
void shouldResolveNestedJavaBeanArgument() throws Exception {
Method addBookWithNestedAuthor = ClassUtils.getMethod(BookController.class, "addBookWithNestedAuthor", Book.class);
String payload = "{\"book\": { \"name\": \"test name\", \"author\": { \"firstName\": \"Jane\", \"lastName\": \"Spring\"} } }";
DataFetchingEnvironment environment = initEnvironment(payload);
MethodParameter methodParameter = getMethodParameter(addBookWithNestedAuthor, 0);
Object result = resolver.resolveArgument(methodParameter, environment);
assertThat(result).isNotNull().isInstanceOf(Book.class);
assertThat((Book) result).hasFieldOrPropertyWithValue("name", "test name");
assertThat(((Book) result).getAuthor()).isNotNull()
.hasFieldOrPropertyWithValue("firstName", "Jane")
.hasFieldOrPropertyWithValue("lastName", "Spring");
}
private MethodParameter getMethodParameter(Method method, int index) {
MethodParameter methodParameter = new MethodParameter(method, index);
methodParameter.initParameterNameDiscovery(new DefaultParameterNameDiscoverer());
@@ -136,12 +120,6 @@ class ArgumentMethodArgumentResolverTests {
return DataFetchingEnvironmentImpl.newDataFetchingEnvironment().arguments(arguments).build();
}
// BeanWrapper
// ModelAttributeProcessor for constructor based instantiation
// look at DGS PR for this issue
@Controller
static class BookController {
@@ -154,13 +132,13 @@ class ArgumentMethodArgumentResolverTests {
return null;
}
@MutationMapping
public Book addBook(@Argument BookInput bookInput) {
@QueryMapping
public Book findWithDefault(@Argument(defaultValue = "42") Long id) {
return null;
}
@MutationMapping
public Book ktAddBook(@Argument KotlinBookInput bookInput) {
public Book addBook(@Argument BookInput bookInput) {
return null;
}
@@ -169,10 +147,6 @@ class ArgumentMethodArgumentResolverTests {
return null;
}
@MutationMapping
public Book addBookWithNestedAuthor(@Argument Book book) {
return null;
}
}
static class BookInput {

163
spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiatorTests.java Normal file
View File

@@ -0,0 +1,163 @@
/*
* Copyright 2020-2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.graphql.data.method.annotation.support;
import java.util.List;
import java.util.Map;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import graphql.schema.DataFetchingEnvironment;
import graphql.schema.DataFetchingEnvironmentImpl;
import org.junit.jupiter.api.Test;
import org.springframework.graphql.Book;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
/**
* Tests for {@link GraphQlArgumentInstantiator}
*
* @author Brian Clozel
*/
class GraphQlArgumentInstantiatorTests {
private ObjectMapper mapper = new ObjectMapper();
private GraphQlArgumentInstantiator instantiator = new GraphQlArgumentInstantiator();
@Test
void shouldInstantiateDefaultConstructor() throws Exception {
String payload = "{\"simpleBean\": { \"name\": \"test\"} }";
DataFetchingEnvironment environment = initEnvironment(payload);
SimpleBean result = instantiator.instantiate(SimpleBean.class, environment.getArgument("simpleBean"));
assertThat(result).isNotNull().isInstanceOf(SimpleBean.class);
assertThat(result).hasFieldOrPropertyWithValue("name", "test");
}
@Test
void shouldInstantiatePrimaryConstructor() throws Exception {
String payload = "{\"constructorBean\": { \"name\": \"test\"} }";
DataFetchingEnvironment environment = initEnvironment(payload);
ContructorBean result = instantiator.instantiate(ContructorBean.class, environment.getArgument("constructorBean"));
assertThat(result).isNotNull().isInstanceOf(ContructorBean.class);
assertThat(result).hasFieldOrPropertyWithValue("name", "test");
}
@Test
void shouldFailIfNoPrimaryConstructor() throws Exception {
String payload = "{\"noPrimary\": { \"name\": \"test\"} }";
DataFetchingEnvironment environment = initEnvironment(payload);
assertThatThrownBy(() -> instantiator.instantiate(NoPrimaryConstructor.class, environment.getArgument("noPrimary")))
.isInstanceOf(IllegalStateException.class).hasMessageContaining("No primary or single public constructor found");
}
@Test
void shouldInstantiateNestedBean() throws Exception {
String payload = "{\"book\": { \"name\": \"test name\", \"author\": { \"firstName\": \"Jane\", \"lastName\": \"Spring\"} } }";
DataFetchingEnvironment environment = initEnvironment(payload);
Book result = instantiator.instantiate(Book.class, environment.getArgument("book"));
assertThat(result).isNotNull().isInstanceOf(Book.class);
assertThat(result).hasFieldOrPropertyWithValue("name", "test name");
assertThat(result.getAuthor()).isNotNull()
.hasFieldOrPropertyWithValue("firstName", "Jane")
.hasFieldOrPropertyWithValue("lastName", "Spring");
}
@Test
void shouldInstantiateNestedBeanLists() throws Exception {
String payload = "{\"nestedList\": { \"items\": [ {\"name\": \"first\"}, {\"name\": \"second\"}] } }";
DataFetchingEnvironment environment = initEnvironment(payload);
NestedList result = instantiator.instantiate(NestedList.class, environment.getArgument("nestedList"));
assertThat(result).isNotNull().isInstanceOf(NestedList.class);
assertThat(result.getItems()).hasSize(2).extracting("name").containsExactly("first", "second");
}
private DataFetchingEnvironment initEnvironment(String jsonPayload) throws JsonProcessingException {
Map<String, Object> arguments = this.mapper.readValue(jsonPayload, new TypeReference<Map<String, Object>>() {
});
return DataFetchingEnvironmentImpl.newDataFetchingEnvironment().arguments(arguments).build();
}
static class SimpleBean {
String name;
public String getName() {
return this.name;
}
public void setName(String name) {
this.name = name;
}
}
static class ContructorBean {
final String name;
public ContructorBean(String name) {
this.name = name;
}
public String getName() {
return this.name;
}
}
static class NoPrimaryConstructor {
NoPrimaryConstructor(String name) {
}
NoPrimaryConstructor(String name, Long id) {
}
}
static class NestedList {
List<Item> items;
public List<Item> getItems() {
return this.items;
}
public void setItems(List<Item> items) {
this.items = items;
}
}
static class Item {
String name;
public String getName() {
return this.name;
}
public void setName(String name) {
this.name = name;
}
}
}

3
spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/KotlinBookInput.kt
View File

@@ -1,3 +0,0 @@
package org.springframework.graphql.data.method.annotation.support;
data class KotlinBookInput(val name: String, val authorId: Long)