From b03097ad47d44b6162b62241a4437bd8cdb112a8 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Fri, 24 Sep 2021 13:47:56 +0200 Subject: [PATCH] Remove attributes from Argument annotation Prior to this commit, the `@Argument` annotation would support two attributes: `required` and `defaultValue`, specifying whether arguments should be considered as required and default values to use if they're not present. This can be problematic as this can already be specified at the schema level. To avoid duplication and invalid setups betweens schema and Controllers, this commit removes those attributes in favor of the schema definition. Closes gh-150 --- .../src/main/resources/application.properties | 2 -- .../src/docs/asciidoc/index.adoc | 7 ++--- .../data/method/annotation/Argument.java | 25 ++++------------ .../ArgumentMethodArgumentResolver.java | 8 +---- .../ArgumentMethodArgumentResolverTests.java | 30 ------------------- 5 files changed, 9 insertions(+), 63 deletions(-) diff --git a/samples/webmvc-http/src/main/resources/application.properties b/samples/webmvc-http/src/main/resources/application.properties index d31504ed..80387b27 100644 --- a/samples/webmvc-http/src/main/resources/application.properties +++ b/samples/webmvc-http/src/main/resources/application.properties @@ -1,5 +1,3 @@ management.endpoints.web.exposure.include=health,metrics,info spring.graphql.schema.printer.enabled=true - -spring.graph \ No newline at end of file diff --git a/spring-graphql-docs/src/docs/asciidoc/index.adoc b/spring-graphql-docs/src/docs/asciidoc/index.adoc index e59374dd..602c3e48 100644 --- a/spring-graphql-docs/src/docs/asciidoc/index.adoc +++ b/spring-graphql-docs/src/docs/asciidoc/index.adoc @@ -577,8 +577,7 @@ values. The arguments are available as simple scalar values such as String, or a of values for more complex input, or a `List` of values. Use `@Argument` to access an argument for the field that maps to the handler method. You -can declare such a method parameter to be of any type. If necessary, Spring GraphQL -converts the value by serializing it to JSON first and then to the target type. +can declare such a method parameter to be of any type. [source,java,indent=0,subs="verbatim,quotes"] ---- @@ -601,8 +600,8 @@ You can explicitly specify the argument name, for example `@Argument("bookInput" it not specified, it defaults to the method parameter name, but this requires the `-parameters` compiler flag with Java 8+ or debugging information from the compiler. -By default, an `@Argument` is required, but you can make it optional by setting the -`required` flag to false or by declaring the argument with `java.util.Optional`. +The "required" character of an `@Argument` or its default value is controlled at the schema +level. You can use `@Argument` on a `Map` argument, to obtain all argument values. The name attribute on `@Argument` must not be set. diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/Argument.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/Argument.java index 4e781f9c..5690d67a 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/Argument.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/Argument.java @@ -21,17 +21,20 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; - import org.springframework.core.annotation.AliasFor; /** * Annotation to bind a method parameter to a GraphQL input * {@link graphql.schema.DataFetchingEnvironment#getArgument(String) argument}. * - *

If the method parameter is {@link java.util.Map Map<String, Object>} or + *

If the method parameter is {@link java.util.Map Map<String, Object>} * and a parameter name is not specified, then the map parameter is populated * via {@link graphql.schema.DataFetchingEnvironment#getArguments()}. * + *

This annotation does not specify whether the input argument is required + * and if it should use a default value: this should be done at the schema + * in order to be enforced by the GraphQL engine itself. + * * @author Rossen Stoyanchev * @since 1.0.0 */ @@ -52,22 +55,4 @@ public @interface Argument { @AliasFor("value") String name() default ""; - /** - * Whether the input argument is required. - *

Defaults to {@code true}, leading to an exception being thrown - * if the argument is missing. Switch this to {@code false} if you prefer - * a {@code null} value when the parameter is not present. - *

Alternatively, provide a {@link #defaultValue}, which implicitly - * sets this flag to {@code false}. - */ - boolean required() default true; - - /** - * The default value to use as a fallback when an input argument is - * not present or has an empty value. - *

Supplying a default value implicitly sets {@link #required} to - * {@code false}. - */ - String defaultValue() default ValueConstants.DEFAULT_NONE; - } 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 7bc5f647..617e19b2 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 @@ -64,16 +64,10 @@ public class ArgumentMethodArgumentResolver implements HandlerMethodArgumentReso } } - Object rawValue = (ValueConstants.DEFAULT_NONE.equals(annotation.defaultValue()) ? - environment.getArgument(name) : - environment.getArgumentOrDefault(name, annotation.defaultValue())); - + Object rawValue = environment.getArgument(name); TypeDescriptor parameterType = new TypeDescriptor(parameter); if (rawValue == null) { - if (annotation.required()) { - throw new MissingArgumentException(name, parameter); - } return returnValue(rawValue, parameterType.getType()); } 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 d369feb5..ee327e26 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 @@ -86,26 +86,6 @@ class ArgumentMethodArgumentResolverTests { .hasFieldOrPropertyWithValue("authorId", 42L); } - @Test - void shouldResolveDefaultValue() throws Exception { - Method findWithDefault = ClassUtils.getMethod(BookController.class, "findWithDefault", Long.class); - String payload = "{\"name\": \"test\" }"; - DataFetchingEnvironment environment = initEnvironment(payload); - MethodParameter methodParameter = getMethodParameter(findWithDefault, 0); - Object result = resolver.resolveArgument(methodParameter, environment); - assertThat(result).isNotNull().isInstanceOf(Long.class).isEqualTo(42L); - } - - @Test - void shouldNotFailIfArgumentNotRequired() throws Exception { - Method findByKeywords = ClassUtils.getMethod(BookController.class, "findByKeywords", List.class); - String payload = "{ }"; - DataFetchingEnvironment environment = initEnvironment(payload); - MethodParameter methodParameter = getMethodParameter(findByKeywords, 0); - Object result = resolver.resolveArgument(methodParameter, environment); - assertThat(result).isNull(); - } - @Test void shouldResolveListOfJavaBeansArgument() throws Exception { Method addBooks = ClassUtils.getMethod(BookController.class, "addBooks", List.class); @@ -142,16 +122,6 @@ class ArgumentMethodArgumentResolverTests { return null; } - @QueryMapping - public Book findWithDefault(@Argument(defaultValue = "42") Long id) { - return null; - } - - @QueryMapping - public Book findByKeywords(@Argument(required = false) List keywords) { - return null; - } - @MutationMapping public Book addBook(@Argument BookInput bookInput) { return null;