From 882ad0eb265fdfce1be3f60f2d66ca92ec3efbb4 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Thu, 13 Apr 2023 18:37:44 +0100 Subject: [PATCH] Allow fallback on field access for argument binding Closes gh-599 --- .../src/docs/asciidoc/index.adoc | 4 ++ .../graphql/data/GraphQlArgumentBinder.java | 42 ++++++++++++++----- .../AnnotatedControllerConfigurer.java | 18 +++++++- .../data/GraphQlArgumentBinderTests.java | 36 ++++++++++++++-- 4 files changed, 86 insertions(+), 14 deletions(-) diff --git a/spring-graphql-docs/src/docs/asciidoc/index.adoc b/spring-graphql-docs/src/docs/asciidoc/index.adoc index 5f60a121..b3e9348f 100644 --- a/spring-graphql-docs/src/docs/asciidoc/index.adoc +++ b/spring-graphql-docs/src/docs/asciidoc/index.adoc @@ -1514,6 +1514,10 @@ accordingly. For example: } ---- +TIP: If the target object doesn't have setters, and you can't change that, you can use a +property on `AnnotatedControllerConfigurer` to allow falling back on binding via direct +field access. + By default, if the method parameter name is available (requires the `-parameters` compiler flag with Java 8+ or debugging info from the compiler), it is used to look up the argument. If needed, you can customize the name through the annotation, e.g. `@Argument("bookInput")`. diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/GraphQlArgumentBinder.java b/spring-graphql/src/main/java/org/springframework/graphql/data/GraphQlArgumentBinder.java index 8688283e..4bf0d545 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/GraphQlArgumentBinder.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/GraphQlArgumentBinder.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2022 the original author or authors. + * Copyright 2020-2023 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. @@ -17,6 +17,7 @@ package org.springframework.graphql.data; import java.lang.reflect.Constructor; +import java.lang.reflect.Field; import java.util.Collection; import java.util.Collections; import java.util.Map; @@ -39,8 +40,10 @@ import org.springframework.core.MethodParameter; import org.springframework.core.ResolvableType; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.TypeDescriptor; +import org.springframework.data.util.DirectFieldAccessFallbackBeanWrapper; import org.springframework.lang.Nullable; import org.springframework.util.ClassUtils; +import org.springframework.util.ReflectionUtils; import org.springframework.validation.AbstractBindingResult; import org.springframework.validation.BindException; import org.springframework.validation.DataBinder; @@ -78,22 +81,34 @@ public class GraphQlArgumentBinder { @Nullable private final SimpleTypeConverter typeConverter; + private final boolean fallBackOnDirectFieldAccess; + public GraphQlArgumentBinder() { this(null); } public GraphQlArgumentBinder(@Nullable ConversionService conversionService) { - if (conversionService != null) { - this.typeConverter = new SimpleTypeConverter(); - this.typeConverter.setConversionService(conversionService); - } - else { - // Not thread-safe when using PropertyEditors - this.typeConverter = null; - } + this(conversionService, false); } + public GraphQlArgumentBinder(@Nullable ConversionService conversionService, boolean fallBackOnDirectFieldAccess) { + this.typeConverter = initTypeConverter(conversionService); + this.fallBackOnDirectFieldAccess = fallBackOnDirectFieldAccess; + } + + @Nullable + private static SimpleTypeConverter initTypeConverter(@Nullable ConversionService conversionService) { + if (conversionService == null) { + // Not thread-safe when using PropertyEditors + return null; + } + SimpleTypeConverter typeConverter = new SimpleTypeConverter(); + typeConverter.setConversionService(conversionService); + return typeConverter; + } + + /** * Add a {@link DataBinder} consumer that initializes the binder instance @@ -297,11 +312,18 @@ public class GraphQlArgumentBinder { ArgumentsBindingResult bindingResult) { Object target = BeanUtils.instantiateClass(constructor); - BeanWrapper beanWrapper = PropertyAccessorFactory.forBeanPropertyAccess(target); + BeanWrapper beanWrapper = (this.fallBackOnDirectFieldAccess ? + new DirectFieldAccessFallbackBeanWrapper(target) : PropertyAccessorFactory.forBeanPropertyAccess(target)); for (Map.Entry entry : rawMap.entrySet()) { String key = entry.getKey(); TypeDescriptor typeDescriptor = beanWrapper.getPropertyTypeDescriptor(key); + if (typeDescriptor == null && this.fallBackOnDirectFieldAccess) { + Field field = ReflectionUtils.findField(beanWrapper.getWrappedClass(), key); + if (field != null) { + typeDescriptor = new TypeDescriptor(field); + } + } if (typeDescriptor == null) { // Ignore unknown property continue; 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 1f94d16c..8949e3d7 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 @@ -139,6 +139,8 @@ public class AnnotatedControllerConfigurer implements ApplicationContextAware, I private final FormattingConversionService conversionService = new DefaultFormattingConversionService(); + private boolean fallBackOnDirectFieldAccess; + private final List customArgumentResolvers = new ArrayList<>(8); @Nullable @@ -167,6 +169,17 @@ public class AnnotatedControllerConfigurer implements ApplicationContextAware, I registrar.registerFormatters(this.conversionService); } + /** + * Whether binding GraphQL arguments onto + * {@link org.springframework.graphql.data.method.annotation.Argument @Argument} + * should falls back to direct field access in case the target object does + * not use accessor methods. + * @since 1.2 + */ + public void setFallBackOnDirectFieldAccess(boolean fallBackOnDirectFieldAccess) { + this.fallBackOnDirectFieldAccess = fallBackOnDirectFieldAccess; + } + /** * Add a {@link HandlerMethodArgumentResolver} for custom controller method * arguments. Such custom resolvers are ordered after built-in resolvers @@ -257,7 +270,10 @@ public class AnnotatedControllerConfigurer implements ApplicationContextAware, I // Must be ahead of ArgumentMethodArgumentResolver resolvers.addResolver(new ProjectedPayloadMethodArgumentResolver(obtainApplicationContext())); } - GraphQlArgumentBinder argumentBinder = new GraphQlArgumentBinder(this.conversionService); + + GraphQlArgumentBinder argumentBinder = + new GraphQlArgumentBinder(this.conversionService, this.fallBackOnDirectFieldAccess); + resolvers.addResolver(new ArgumentMethodArgumentResolver(argumentBinder)); resolvers.addResolver(new ArgumentsMethodArgumentResolver(argumentBinder)); resolvers.addResolver(new ContextValueMethodArgumentResolver()); diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/GraphQlArgumentBinderTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/GraphQlArgumentBinderTests.java index c8f2ff2d..74bb3841 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/GraphQlArgumentBinderTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/GraphQlArgumentBinderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2022 the original author or authors. + * Copyright 2020-2023 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. @@ -17,6 +17,7 @@ package org.springframework.graphql.data; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -118,6 +119,19 @@ class GraphQlArgumentBinderTests { assertThat(((ItemListHolder) result).getItems()).hasSize(0); } + @Test // gh-599 + void dataBindingWithDirectFieldAccess() throws Exception { + + Object result = bind( + new GraphQlArgumentBinder(new DefaultFormattingConversionService(), true /* fallBackOnFieldAccess */), + "{\"items\":[{\"name\":\"first\"},{\"name\":\"second\"}]}", + ResolvableType.forClass(DirectFieldAccessItemListHolder.class)); + + assertThat(result).isNotNull().isInstanceOf(DirectFieldAccessItemListHolder.class); + DirectFieldAccessItemListHolder holder = (DirectFieldAccessItemListHolder) result; + assertThat(holder.items).hasSize(2).extracting("name").containsExactly("first", "second"); + } + @Test // gh-349 void dataBindingToBeanWithEnumGenericType() throws Exception { @@ -389,14 +403,19 @@ class GraphQlArgumentBinderTests { assertThat(input.enums()).hasSize(2).containsExactly(FancyEnum.ONE, FancyEnum.TWO); } - @SuppressWarnings("unchecked") @Nullable private Object bind(String json, ResolvableType targetType) throws Exception { + return bind(this.binder, json, targetType); + } + + @SuppressWarnings("unchecked") + @Nullable + private Object bind(GraphQlArgumentBinder binder, String json, ResolvableType targetType) throws Exception { DataFetchingEnvironment environment = DataFetchingEnvironmentImpl.newDataFetchingEnvironment() .arguments(this.mapper.readValue("{\"key\":" + json + "}", Map.class)) .build(); - return this.binder.bind(environment, "key", targetType); + return binder.bind(environment, "key", targetType); } @@ -607,6 +626,17 @@ class GraphQlArgumentBinderTests { } + static class DirectFieldAccessItemListHolder { + + private List items = new ArrayList<>(); + + public DirectFieldAccessItemListHolder addItem(Item item) { + this.items.add(item); + return this; + } + } + + @SuppressWarnings("unused") static class Item {