From 1773f6ed333434e4a454522ef331221f08c48433 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 19 Sep 2022 15:33:25 +0100 Subject: [PATCH] Apply validation only when annotations detected Closes gh-445 --- .../support/DataFetcherHandlerMethod.java | 2 +- .../HandlerMethodValidationHelper.java | 33 +++++++++ .../HandlerMethodValidationHelperTests.java | 67 +++++++++++++++++++ 3 files changed, 101 insertions(+), 1 deletion(-) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethod.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethod.java index 8843751a..e33d9635 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethod.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethod.java @@ -71,7 +71,7 @@ public class DataFetcherHandlerMethod extends InvocableHandlerMethodSupport { super(handlerMethod, executor); Assert.isTrue(!resolvers.getResolvers().isEmpty(), "No argument resolvers"); this.resolvers = resolvers; - this.validator = validator; + this.validator = (validator != null && validator.requiresValidation(handlerMethod) ? validator : null); this.subscription = subscription; } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodValidationHelper.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodValidationHelper.java index 3de659bd..3d7493cc 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodValidationHelper.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodValidationHelper.java @@ -19,12 +19,17 @@ package org.springframework.graphql.data.method.annotation.support; import java.lang.annotation.Annotation; import java.util.Set; +import javax.validation.Constraint; import javax.validation.ConstraintViolation; import javax.validation.ConstraintViolationException; +import javax.validation.Valid; import javax.validation.Validator; +import javax.validation.metadata.BeanDescriptor; import org.springframework.context.ApplicationContext; +import org.springframework.core.MethodParameter; import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.core.annotation.MergedAnnotations; import org.springframework.graphql.data.method.HandlerMethod; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -97,6 +102,34 @@ final class HandlerMethodValidationHelper { return annotation; } + /** + * Whether the given method requires standard bean validation. This is the + * case if the method or one of its parameters are annotated with + * {@link Valid} or {@link Validated}, or if any method parameter is declared + * with a {@link Constraint constraint}, or the method parameter type is + * itself {@link BeanDescriptor#isBeanConstrained() constrained}. + * @param method the handler method to check + * @return {@code true} if validation is required, {@code false} otherwise + */ + public boolean requiresValidation(HandlerMethod method) { + if (findAnnotation(method, Validated.class) != null || findAnnotation(method, Valid.class) != null) { + return true; + } + for (MethodParameter parameter : method.getMethodParameters()) { + for (Annotation annotation : parameter.getParameterAnnotations()) { + MergedAnnotations merged = MergedAnnotations.from(annotation); + if (merged.isPresent(Valid.class) || merged.isPresent(Constraint.class) || merged.isPresent(Validated.class)) { + return true; + } + } + Class paramType = parameter.nestedIfOptional().getNestedParameterType(); + if (this.validator.getConstraintsForClass(paramType).isBeanConstrained()) { + return true; + } + } + return false; + } + /** * Factory method for {@link HandlerMethodValidationHelper} if a diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodValidationHelperTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodValidationHelperTests.java index 81e50c12..629a4780 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodValidationHelperTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodValidationHelperTests.java @@ -20,9 +20,11 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.reflect.Method; import java.util.Arrays; +import java.util.Optional; import javax.validation.ConstraintViolation; import javax.validation.ConstraintViolationException; +import javax.validation.Valid; import javax.validation.Validation; import javax.validation.constraints.Max; import javax.validation.constraints.NotNull; @@ -36,6 +38,7 @@ import org.springframework.beans.BeanUtils; import org.springframework.graphql.data.method.HandlerMethod; import org.springframework.validation.annotation.Validated; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatNoException; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -80,6 +83,27 @@ class HandlerMethodValidationHelperTests { .anyMatch(violation -> violation.getPropertyPath().toString().equals("myValidMethodWithGroupOnType.arg0")); } + @Test + void shouldRecognizeMethodsThatRequireValidation() { + HandlerMethod method = findHandlerMethod(RequiresValidationBean.class, "processConstrainedValue"); + assertThat(validator.requiresValidation(method)).isTrue(); + + method = findHandlerMethod(RequiresValidationBean.class, "processValidInput"); + assertThat(validator.requiresValidation(method)).isTrue(); + + method = findHandlerMethod(RequiresValidationBean.class, "processValidatedInput"); + assertThat(validator.requiresValidation(method)).isTrue(); + + method = findHandlerMethod(RequiresValidationBean.class, "processInputWithConstrainedValue"); + assertThat(validator.requiresValidation(method)).isTrue(); + + method = findHandlerMethod(RequiresValidationBean.class, "processOptionalInputWithConstrainedValue"); + assertThat(validator.requiresValidation(method)).isTrue(); + + method = findHandlerMethod(RequiresValidationBean.class, "processValue"); + assertThat(validator.requiresValidation(method)).isFalse(); + } + private HandlerMethod findHandlerMethod(Class handlerType, String methodName) { Object handler = BeanUtils.instantiateClass(handlerType); @@ -147,4 +171,47 @@ class HandlerMethodValidationHelperTests { } + + @SuppressWarnings("unused") + private static class RequiresValidationBean { + + public void processConstrainedValue(@Max(99) int i) { + } + + public void processValidInput(@Valid MyInput input) { + } + + public void processValidatedInput(@Validated MyInput input) { + } + + public void processInputWithConstrainedValue(MyConstrainedInput input) { + } + + public void processOptionalInputWithConstrainedValue(Optional input) { + } + + public void processValue(int i) { + } + + } + + + private static class MyInput { + } + + private static class MyConstrainedInput { + + @Max(99) + private int i; + + public int getI() { + return this.i; + } + + public void setI(int i) { + this.i = i; + } + + } + } \ No newline at end of file