From 6975e6c37ba099f6ad2f9050344f97391e840658 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Thu, 30 Sep 2021 15:26:22 +0200 Subject: [PATCH] Configure ConversionService for argument conversion This commit configures a `FormattingConversionService` into the argument resolution infrastructure, using the expected conversion service for both MVC and WebFlux. Closes gh-148 --- .../boot/GraphQlAutoConfiguration.java | 6 ---- .../boot/GraphQlWebFluxAutoConfiguration.java | 10 +++++++ .../boot/GraphQlWebMvcAutoConfiguration.java | 9 ++++++ .../AnnotatedControllerConfigurer.java | 10 ++++++- .../ArgumentMethodArgumentResolver.java | 12 +++++++- .../support/GraphQlArgumentInstantiator.java | 9 +++++- .../ArgumentMethodArgumentResolverTests.java | 28 ++++++++++++++++++- .../GraphQlArgumentInstantiatorTests.java | 2 +- 8 files changed, 75 insertions(+), 11 deletions(-) diff --git a/graphql-spring-boot-starter/src/main/java/org/springframework/graphql/boot/GraphQlAutoConfiguration.java b/graphql-spring-boot-starter/src/main/java/org/springframework/graphql/boot/GraphQlAutoConfiguration.java index c06b04d7..ac030a7c 100644 --- a/graphql-spring-boot-starter/src/main/java/org/springframework/graphql/boot/GraphQlAutoConfiguration.java +++ b/graphql-spring-boot-starter/src/main/java/org/springframework/graphql/boot/GraphQlAutoConfiguration.java @@ -36,7 +36,6 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.io.Resource; import org.springframework.core.io.support.ResourcePatternResolver; -import org.springframework.graphql.data.method.annotation.support.AnnotatedControllerConfigurer; import org.springframework.graphql.execution.DataFetcherExceptionResolver; import org.springframework.graphql.execution.GraphQlSource; import org.springframework.graphql.execution.MissingSchemaException; @@ -57,11 +56,6 @@ public class GraphQlAutoConfiguration { private static final Log logger = LogFactory.getLog(GraphQlAutoConfiguration.class); - @Bean - public AnnotatedControllerConfigurer annotatedControllerConfigurer() { - return new AnnotatedControllerConfigurer(); - } - @Bean public GraphQlSource graphQlSource(ResourcePatternResolver resourcePatternResolver, GraphQlProperties properties, ObjectProvider exceptionResolversProvider, diff --git a/graphql-spring-boot-starter/src/main/java/org/springframework/graphql/boot/GraphQlWebFluxAutoConfiguration.java b/graphql-spring-boot-starter/src/main/java/org/springframework/graphql/boot/GraphQlWebFluxAutoConfiguration.java index 7abd59c1..2600133c 100644 --- a/graphql-spring-boot-starter/src/main/java/org/springframework/graphql/boot/GraphQlWebFluxAutoConfiguration.java +++ b/graphql-spring-boot-starter/src/main/java/org/springframework/graphql/boot/GraphQlWebFluxAutoConfiguration.java @@ -24,6 +24,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.ObjectProvider; +import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; @@ -36,7 +37,9 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.io.Resource; import org.springframework.core.io.ResourceLoader; +import org.springframework.format.support.FormattingConversionService; import org.springframework.graphql.GraphQlService; +import org.springframework.graphql.data.method.annotation.support.AnnotatedControllerConfigurer; import org.springframework.graphql.execution.GraphQlSource; import org.springframework.graphql.web.WebGraphQlHandler; import org.springframework.graphql.web.WebInterceptor; @@ -78,6 +81,13 @@ public class GraphQlWebFluxAutoConfiguration { private static final Log logger = LogFactory.getLog(GraphQlWebFluxAutoConfiguration.class); + @Bean + public AnnotatedControllerConfigurer annotatedControllerConfigurer(@Qualifier("webFluxConversionService") FormattingConversionService conversionService) { + AnnotatedControllerConfigurer annotatedControllerConfigurer = new AnnotatedControllerConfigurer(); + annotatedControllerConfigurer.setConversionService(conversionService); + return annotatedControllerConfigurer; + } + @Bean @ConditionalOnBean(GraphQlService.class) @ConditionalOnMissingBean diff --git a/graphql-spring-boot-starter/src/main/java/org/springframework/graphql/boot/GraphQlWebMvcAutoConfiguration.java b/graphql-spring-boot-starter/src/main/java/org/springframework/graphql/boot/GraphQlWebMvcAutoConfiguration.java index 11f07a31..91d8675f 100644 --- a/graphql-spring-boot-starter/src/main/java/org/springframework/graphql/boot/GraphQlWebMvcAutoConfiguration.java +++ b/graphql-spring-boot-starter/src/main/java/org/springframework/graphql/boot/GraphQlWebMvcAutoConfiguration.java @@ -27,6 +27,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.ObjectProvider; +import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; @@ -40,7 +41,9 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.io.Resource; import org.springframework.core.io.ResourceLoader; +import org.springframework.format.support.FormattingConversionService; import org.springframework.graphql.GraphQlService; +import org.springframework.graphql.data.method.annotation.support.AnnotatedControllerConfigurer; import org.springframework.graphql.execution.GraphQlSource; import org.springframework.graphql.execution.ThreadLocalAccessor; import org.springframework.graphql.web.WebGraphQlHandler; @@ -85,6 +88,12 @@ public class GraphQlWebMvcAutoConfiguration { private static final Log logger = LogFactory.getLog(GraphQlWebMvcAutoConfiguration.class); + @Bean + public AnnotatedControllerConfigurer annotatedControllerConfigurer(@Qualifier("mvcConversionService") FormattingConversionService conversionService) { + AnnotatedControllerConfigurer annotatedControllerConfigurer = new AnnotatedControllerConfigurer(); + annotatedControllerConfigurer.setConversionService(conversionService); + return annotatedControllerConfigurer; + } @Bean @ConditionalOnBean(GraphQlService.class) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java index 38603e3b..82a6d5c7 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java @@ -45,6 +45,7 @@ import org.springframework.core.KotlinDetector; import org.springframework.core.MethodIntrospector; import org.springframework.core.MethodParameter; import org.springframework.core.annotation.AnnotatedElementUtils; +import org.springframework.core.convert.ConversionService; import org.springframework.graphql.data.method.HandlerMethod; import org.springframework.graphql.data.method.HandlerMethodArgumentResolver; import org.springframework.graphql.data.method.HandlerMethodArgumentResolverComposite; @@ -90,12 +91,19 @@ public class AnnotatedControllerConfigurer @Nullable private HandlerMethodArgumentResolverComposite argumentResolvers; + @Nullable + private ConversionService conversionService; + @Override public void setApplicationContext(ApplicationContext applicationContext) { this.applicationContext = applicationContext; } + public void setConversionService(ConversionService conversionService) { + this.conversionService = conversionService; + } + protected final ApplicationContext obtainApplicationContext() { Assert.state(this.applicationContext != null, "No ApplicationContext"); return this.applicationContext; @@ -106,7 +114,7 @@ public class AnnotatedControllerConfigurer public void afterPropertiesSet() { this.argumentResolvers = new HandlerMethodArgumentResolverComposite(); this.argumentResolvers.addResolver(new ArgumentMapMethodArgumentResolver()); - this.argumentResolvers.addResolver(new ArgumentMethodArgumentResolver()); + this.argumentResolvers.addResolver(new ArgumentMethodArgumentResolver(this.conversionService)); this.argumentResolvers.addResolver(new DataFetchingEnvironmentMethodArgumentResolver()); this.argumentResolvers.addResolver(new DataLoaderMethodArgumentResolver()); diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolver.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolver.java index 82b0c31c..8bab887c 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolver.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolver.java @@ -23,9 +23,11 @@ import graphql.schema.DataFetchingEnvironment; 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.graphql.data.method.HandlerMethodArgumentResolver; import org.springframework.graphql.data.method.annotation.Argument; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.validation.DataBinder; @@ -41,7 +43,14 @@ import org.springframework.validation.DataBinder; */ public class ArgumentMethodArgumentResolver implements HandlerMethodArgumentResolver { - private final GraphQlArgumentInstantiator instantiator = new GraphQlArgumentInstantiator(); + private final GraphQlArgumentInstantiator instantiator; + + private final ConversionService conversionService; + + public ArgumentMethodArgumentResolver(@Nullable ConversionService conversionService) { + this.conversionService = conversionService; + this.instantiator = new GraphQlArgumentInstantiator(conversionService); + } @Override public boolean supportsParameter(MethodParameter parameter) { @@ -100,6 +109,7 @@ public class ArgumentMethodArgumentResolver implements HandlerMethodArgumentReso } else { DataBinder converter = new DataBinder(null); + converter.setConversionService(this.conversionService); target = converter.convertIfNecessary(rawValue, targetType); Assert.isTrue(target != null, () -> "Value of type [" + rawValue.getClass() + "] cannot be converted to argument of type [" + diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiator.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiator.java index 4ff76915..75fef488 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiator.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiator.java @@ -28,7 +28,9 @@ 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.ConversionService; import org.springframework.core.convert.TypeDescriptor; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.validation.DataBinder; @@ -40,7 +42,12 @@ import org.springframework.validation.DataBinder; */ class GraphQlArgumentInstantiator { - private final DataBinder converter = new DataBinder(null); + private final DataBinder converter; + + public GraphQlArgumentInstantiator(@Nullable ConversionService conversionService) { + this.converter = new DataBinder(null); + this.converter.setConversionService(conversionService); + } /** * Instantiate the given target type and bind data from diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolverTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolverTests.java index ee327e26..bda1202b 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolverTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/ArgumentMethodArgumentResolverTests.java @@ -30,6 +30,8 @@ import org.junit.jupiter.api.Test; import org.springframework.core.DefaultParameterNameDiscoverer; import org.springframework.core.MethodParameter; +import org.springframework.format.support.DefaultFormattingConversionService; +import org.springframework.format.support.FormattingConversionService; import org.springframework.graphql.Book; import org.springframework.graphql.data.method.annotation.Argument; import org.springframework.graphql.data.method.annotation.MutationMapping; @@ -48,7 +50,7 @@ class ArgumentMethodArgumentResolverTests { private final ObjectMapper mapper = new ObjectMapper(); - ArgumentMethodArgumentResolver resolver = new ArgumentMethodArgumentResolver(); + ArgumentMethodArgumentResolver resolver = new ArgumentMethodArgumentResolver(new DefaultFormattingConversionService()); @Test void shouldSupportAnnotatedParameters() { @@ -98,6 +100,17 @@ class ArgumentMethodArgumentResolverTests { .extracting("name").containsExactly("first", "second"); } + @Test + void shouldResolveArgumentWithConversionService() throws Exception { + Method bookByKeyword = ClassUtils.getMethod(BookController.class, "bookByKeyword", Keyword.class); + String payload = "{\"keyword\": \"test\" }"; + DataFetchingEnvironment environment = initEnvironment(payload); + MethodParameter methodParameter = getMethodParameter(bookByKeyword, 0); + Object result = resolver.resolveArgument(methodParameter, environment); + assertThat(result).isNotNull().isInstanceOf(Keyword.class); + assertThat((Keyword) result).hasFieldOrPropertyWithValue("term", "test"); + } + private MethodParameter getMethodParameter(Method method, int index) { MethodParameter methodParameter = new MethodParameter(method, index); methodParameter.initParameterNameDiscovery(new DefaultParameterNameDiscoverer()); @@ -132,6 +145,11 @@ class ArgumentMethodArgumentResolverTests { return null; } + @QueryMapping + public List bookByKeyword(@Argument Keyword keyword) { + return null; + } + } static class BookInput { @@ -161,6 +179,14 @@ class ArgumentMethodArgumentResolverTests { String term; + private Keyword(String term) { + this.term = term; + } + + public static Keyword of(String term) { + return new Keyword(term); + } + public String getTerm() { return this.term; } diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiatorTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiatorTests.java index ee5a9092..3f01cd15 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiatorTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/GraphQlArgumentInstantiatorTests.java @@ -40,7 +40,7 @@ class GraphQlArgumentInstantiatorTests { private ObjectMapper mapper = new ObjectMapper(); - private GraphQlArgumentInstantiator instantiator = new GraphQlArgumentInstantiator(); + private GraphQlArgumentInstantiator instantiator = new GraphQlArgumentInstantiator(null); @Test void shouldInstantiateDefaultConstructor() throws Exception {