From 76f97da4120f8453c049828e29a92e89a2ac21f5 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 19 Sep 2022 14:22:08 +0100 Subject: [PATCH] Polishing validation code See gh-445 --- .../AnnotatedControllerConfigurer.java | 33 ++--- .../support/DataFetcherHandlerMethod.java | 9 +- .../support/HandlerMethodInputValidator.java | 98 --------------- .../HandlerMethodValidationHelper.java | 113 ++++++++++++++++++ ...> HandlerMethodValidationHelperTests.java} | 51 ++++---- 5 files changed, 160 insertions(+), 144 deletions(-) delete mode 100644 spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodInputValidator.java create mode 100644 spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodValidationHelper.java rename spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/{HandlerMethodInputValidatorTests.java => HandlerMethodValidationHelperTests.java} (76%) 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 2ae093f1..a11e3768 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 @@ -30,8 +30,6 @@ import java.util.concurrent.Executor; import java.util.function.Consumer; import java.util.stream.Collectors; -import javax.validation.Validator; - import graphql.schema.DataFetcher; import graphql.schema.DataFetchingEnvironment; import graphql.schema.FieldCoordinates; @@ -127,7 +125,7 @@ public class AnnotatedControllerConfigurer private HandlerMethodArgumentResolverComposite argumentResolvers; @Nullable - private HandlerMethodInputValidator validator; + private HandlerMethodValidationHelper validationHelper; @Nullable private Consumer dataBinderInitializer; @@ -175,7 +173,8 @@ public class AnnotatedControllerConfigurer this.argumentResolvers = initArgumentResolvers(); if (beanValidationPresent) { - this.validator = HandlerMethodInputValidatorFactory.create(obtainApplicationContext()); + this.validationHelper = + HandlerMethodValidationHelper.createIfValidatorAvailable(obtainApplicationContext()); } } @@ -229,7 +228,7 @@ public class AnnotatedControllerConfigurer findHandlerMethods().forEach((info) -> { DataFetcher dataFetcher; if (!info.isBatchMapping()) { - dataFetcher = new SchemaMappingDataFetcher(info, this.argumentResolvers, this.validator, this.executor); + dataFetcher = new SchemaMappingDataFetcher(info, this.argumentResolvers, this.validationHelper, this.executor); } else { String dataLoaderKey = registerBatchLoader(info); @@ -477,21 +476,21 @@ public class AnnotatedControllerConfigurer private final HandlerMethodArgumentResolverComposite argumentResolvers; @Nullable - private final HandlerMethodInputValidator validator; + private final HandlerMethodValidationHelper validatorHelper; @Nullable private final Executor executor; private final boolean subscription; - public SchemaMappingDataFetcher( + SchemaMappingDataFetcher( MappingInfo info, HandlerMethodArgumentResolverComposite resolvers, - @Nullable HandlerMethodInputValidator validator, + @Nullable HandlerMethodValidationHelper validatorHelper, @Nullable Executor executor) { this.info = info; this.argumentResolvers = resolvers; - this.validator = validator; + this.validatorHelper = validatorHelper; this.executor = executor; this.subscription = this.info.getCoordinates().getTypeName().equalsIgnoreCase("Subscription"); } @@ -509,7 +508,7 @@ public class AnnotatedControllerConfigurer public Object get(DataFetchingEnvironment environment) throws Exception { DataFetcherHandlerMethod handlerMethod = new DataFetcherHandlerMethod( - getHandlerMethod(), this.argumentResolvers, this.validator, this.executor, this.subscription); + getHandlerMethod(), this.argumentResolvers, this.validatorHelper, this.executor, this.subscription); return handlerMethod.invoke(environment); } @@ -520,7 +519,7 @@ public class AnnotatedControllerConfigurer private final String dataLoaderKey; - public BatchMappingDataFetcher(String dataLoaderKey) { + BatchMappingDataFetcher(String dataLoaderKey) { this.dataLoaderKey = dataLoaderKey; } @@ -534,16 +533,4 @@ public class AnnotatedControllerConfigurer } } - /** - * Look for a Validator bean in the context and configure validation support - */ - static class HandlerMethodInputValidatorFactory { - - @Nullable - static HandlerMethodInputValidator create(ApplicationContext context) { - Validator validator = context.getBeanProvider(Validator.class).getIfAvailable(); - return validator != null ? new HandlerMethodInputValidator(validator) : null; - } - } - } 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 fe21a7df..8843751a 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 @@ -50,7 +50,7 @@ public class DataFetcherHandlerMethod extends InvocableHandlerMethodSupport { private final HandlerMethodArgumentResolverComposite resolvers; @Nullable - private final HandlerMethodInputValidator validator; + private final HandlerMethodValidationHelper validator; private final ParameterNameDiscoverer parameterNameDiscoverer = new DefaultParameterNameDiscoverer(); @@ -65,7 +65,7 @@ public class DataFetcherHandlerMethod extends InvocableHandlerMethodSupport { * @param subscription whether the field being fetched is of subscription type */ public DataFetcherHandlerMethod(HandlerMethod handlerMethod, - HandlerMethodArgumentResolverComposite resolvers, @Nullable HandlerMethodInputValidator validator, + HandlerMethodArgumentResolverComposite resolvers, @Nullable HandlerMethodValidationHelper validator, @Nullable Executor executor, boolean subscription) { super(handlerMethod, executor); @@ -85,12 +85,15 @@ public class DataFetcherHandlerMethod extends InvocableHandlerMethodSupport { /** * Return the configured input validator. + * @deprecated as of 1.1 without a replacement */ + @Deprecated @Nullable - public HandlerMethodInputValidator getValidator() { + public HandlerMethodValidationHelper getValidator() { return this.validator; } + /** * Invoke the method after resolving its argument values in the context of * the given {@link DataFetchingEnvironment}. diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodInputValidator.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodInputValidator.java deleted file mode 100644 index d775ca56..00000000 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodInputValidator.java +++ /dev/null @@ -1,98 +0,0 @@ -/* - * 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.Set; - -import javax.validation.ConstraintViolation; -import javax.validation.ConstraintViolationException; -import javax.validation.Validation; -import javax.validation.Validator; - -import org.springframework.core.annotation.AnnotationUtils; -import org.springframework.graphql.data.method.HandlerMethod; -import org.springframework.graphql.data.method.HandlerMethodArgumentResolver; -import org.springframework.util.Assert; -import org.springframework.validation.annotation.Validated; -import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean; -import org.springframework.validation.beanvalidation.SpringValidatorAdapter; - -/** - * Strategy for validating a {@link HandlerMethod} input before invocation, based on JSR-303. - * This is called after all {@link HandlerMethodArgumentResolver} have been involved. - * - * @author Brian Clozel - */ -class HandlerMethodInputValidator { - - private final Validator validator; - - /** - * Create the input validator backed by a JSR-303 Validator instance. - */ - public HandlerMethodInputValidator(Validator validator) { - Assert.notNull(validator, "validator should not be null"); - if (validator instanceof LocalValidatorFactoryBean) { - this.validator = ((LocalValidatorFactoryBean) validator).getValidator(); - } - else if (validator instanceof SpringValidatorAdapter) { - this.validator = validator.unwrap(Validator.class); - } - else { - this.validator = validator; - } - } - - /** - * Create the input validator backed by a default - * {@link Validation#buildDefaultValidatorFactory() factory instance}. - */ - public HandlerMethodInputValidator() { - this(Validation.buildDefaultValidatorFactory().getValidator()); - } - - /** - * Validate the {@link HandlerMethod} input before invocation, throwing - * an {@link ConstraintViolationException} if validation fails. - * - * @param handlerMethod the handler method for the current request - * @param arguments the resolved arguments for the method invocation - */ - public void validate(HandlerMethod handlerMethod, Object[] arguments) { - Class[] validationGroups = determineValidationGroups(handlerMethod); - Set> result = this.validator.forExecutables() - .validateParameters(handlerMethod.getBean(), handlerMethod.getMethod(), arguments, validationGroups); - if (!result.isEmpty()) { - throw new ConstraintViolationException(result); - } - } - - /** - * Determine the validation groups to validate against for the given handler method. - *

Default are the validation groups as specified in the {@link Validated} annotation - * on the containing target class of the method. - * @param method the current HandlerMethod - * @return the applicable validation groups as a Class array - */ - private Class[] determineValidationGroups(HandlerMethod method) { - Validated validatedAnn = AnnotationUtils.findAnnotation(method.getMethod(), Validated.class); - if (validatedAnn == null) { - validatedAnn = AnnotationUtils.findAnnotation(method.getBeanType(), Validated.class); - } - return (validatedAnn != null ? validatedAnn.value() : new Class[0]); - } -} 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 new file mode 100644 index 00000000..3de659bd --- /dev/null +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodValidationHelper.java @@ -0,0 +1,113 @@ +/* + * Copyright 2020-2022 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.annotation.Annotation; +import java.util.Set; + +import javax.validation.ConstraintViolation; +import javax.validation.ConstraintViolationException; +import javax.validation.Validator; + +import org.springframework.context.ApplicationContext; +import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.graphql.data.method.HandlerMethod; +import org.springframework.lang.Nullable; +import org.springframework.util.Assert; +import org.springframework.validation.annotation.Validated; +import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean; +import org.springframework.validation.beanvalidation.SpringValidatorAdapter; + +/** + * Helper class to apply standard bean validation to a {@link HandlerMethod}. + * + * @author Brian Clozel + * @author Rossen Stoyanchev + * @since 1.0 + */ +final class HandlerMethodValidationHelper { + + private final Validator validator; + + + /** + * Constructor with the {@link Validator} instance to use. + */ + public HandlerMethodValidationHelper(Validator validator) { + Assert.notNull(validator, "validator should not be null"); + if (validator instanceof LocalValidatorFactoryBean) { + this.validator = ((LocalValidatorFactoryBean) validator).getValidator(); + } + else if (validator instanceof SpringValidatorAdapter) { + this.validator = validator.unwrap(Validator.class); + } + else { + this.validator = validator; + } + } + + + /** + * Validate the input values to a the {@link HandlerMethod} and throw a + * {@link ConstraintViolationException} in case of violations. + * @param handlerMethod the handler method to validate + * @param arguments the input argument values + */ + public void validate(HandlerMethod handlerMethod, Object[] arguments) { + Set> result = + this.validator.forExecutables().validateParameters( + handlerMethod.getBean(), handlerMethod.getMethod(), arguments, + determineValidationGroups(handlerMethod)); + if (!result.isEmpty()) { + throw new ConstraintViolationException(result); + } + } + + /** + * Determine the validation groups to apply to a handler method, specified + * through the {@link Validated} annotation on the method or on the class. + * @param method the method to check + * @return the applicable validation groups as a Class array + */ + private Class[] determineValidationGroups(HandlerMethod method) { + Validated annotation = findAnnotation(method, Validated.class); + return (annotation != null ? annotation.value() : new Class[0]); + } + + @Nullable + private static A findAnnotation(HandlerMethod method, Class annotationType) { + A annotation = AnnotationUtils.findAnnotation(method.getMethod(), annotationType); + if (annotation == null) { + annotation = AnnotationUtils.findAnnotation(method.getBeanType(), annotationType); + } + return annotation; + } + + + /** + * Factory method for {@link HandlerMethodValidationHelper} if a + * {@link Validator} can be found. + * @param context the context to look up a {@code Validator} bean from + * @return the helper instance, or {@code null + */ + @Nullable + public static HandlerMethodValidationHelper createIfValidatorAvailable(ApplicationContext context) { + Validator validator = context.getBeanProvider(Validator.class).getIfAvailable(); + return (validator != null ? new HandlerMethodValidationHelper(validator) : null); + } + +} diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodInputValidatorTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodValidationHelperTests.java similarity index 76% rename from spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodInputValidatorTests.java rename to spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodValidationHelperTests.java index b7c6955e..81e50c12 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodInputValidatorTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/HandlerMethodValidationHelperTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2021 the original author or authors. + * Copyright 2020-2022 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. @@ -23,6 +23,7 @@ import java.util.Arrays; import javax.validation.ConstraintViolation; import javax.validation.ConstraintViolationException; +import javax.validation.Validation; import javax.validation.constraints.Max; import javax.validation.constraints.NotNull; @@ -39,27 +40,29 @@ import static org.assertj.core.api.Assertions.assertThatNoException; import static org.assertj.core.api.Assertions.assertThatThrownBy; /** - * Tests for {@link HandlerMethodInputValidator} + * Unit tests for {@link HandlerMethodValidationHelper}. * @author Brian Clozel */ -class HandlerMethodInputValidatorTests { +class HandlerMethodValidationHelperTests { + + private final HandlerMethodValidationHelper validator = + new HandlerMethodValidationHelper(Validation.buildDefaultValidatorFactory().getValidator()); - private final HandlerMethodInputValidator validator = new HandlerMethodInputValidator(); @Test void shouldFailWithNullValidator() { - assertThatThrownBy(() -> new HandlerMethodInputValidator(null)).isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> new HandlerMethodValidationHelper(null)).isInstanceOf(IllegalArgumentException.class); } @Test - void shouldIgnoreMethodsWithoutAnnotations() throws Exception { - HandlerMethod method = findHandlerMethod(MyValidBean.class, "notValidatedMethod"); + void shouldIgnoreMethodsWithoutAnnotations() { + HandlerMethod method = findHandlerMethod(MyBean.class, "notValidatedMethod"); assertThatNoException().isThrownBy(() -> validator.validate(method, new Object[] {"test", 12})); } @Test - void shouldRaiseValidationErrorForAnnotatedParams() throws Exception { - HandlerMethod method = findHandlerMethod(MyValidBean.class, "myValidMethod"); + void shouldRaiseValidationErrorForAnnotatedParams() { + HandlerMethod method = findHandlerMethod(MyBean.class, "myValidMethod"); assertViolations(() -> validator.validate(method, new Object[] {null, 2})) .anyMatch(violation -> violation.getPropertyPath().toString().equals("myValidMethod.arg0")); assertViolations(() -> validator.validate(method, new Object[] {"test", 12})) @@ -67,11 +70,12 @@ class HandlerMethodInputValidatorTests { } @Test - void shouldRaiseValidationErrorForAnnotatedParamsWithGroups() throws Exception { - HandlerMethod myValidMethodWithGroup = findHandlerMethod(MyValidBeanWithGroup.class, "myValidMethodWithGroup"); + void shouldRaiseValidationErrorForAnnotatedParamsWithGroups() { + HandlerMethod myValidMethodWithGroup = findHandlerMethod(MyValidationGroupsBean.class, "myValidMethodWithGroup"); assertViolations(() -> validator.validate(myValidMethodWithGroup, new Object[] {null})) .anyMatch(violation -> violation.getPropertyPath().toString().equals("myValidMethodWithGroup.arg0")); - HandlerMethod myValidMethodWithGroupOnType = findHandlerMethod(MyValidBeanWithGroup.class, "myValidMethodWithGroupOnType"); + + HandlerMethod myValidMethodWithGroupOnType = findHandlerMethod(MyValidationGroupsBean.class, "myValidMethodWithGroupOnType"); assertViolations(() -> validator.validate(myValidMethodWithGroupOnType, new Object[] {null})) .anyMatch(violation -> violation.getPropertyPath().toString().equals("myValidMethodWithGroupOnType.arg0")); } @@ -93,7 +97,9 @@ class HandlerMethodInputValidatorTests { .asInstanceOf(InstanceOfAssertFactories.iterable(ConstraintViolation.class)); } - public static class MyValidBean { + + @SuppressWarnings("unused") + private static class MyBean { public String notValidatedMethod(String arg0, int arg1) { return ""; @@ -105,27 +111,32 @@ class HandlerMethodInputValidatorTests { } - public interface FirstGroup { + + interface FirstGroup { } - public interface SecondGroup { + interface SecondGroup { } + @Validated(FirstGroup.class) @Retention(RetentionPolicy.RUNTIME) - public @interface GroupOnParam { + @interface MethodLevelGroup { } + @Validated(SecondGroup.class) @Retention(RetentionPolicy.RUNTIME) - public @interface GroupOnType { + @interface TypeLevelGroup { } - @GroupOnType - public static class MyValidBeanWithGroup { - @GroupOnParam + @TypeLevelGroup + @SuppressWarnings("unused") + private static class MyValidationGroupsBean { + + @MethodLevelGroup public Object myValidMethodWithGroup(@NotNull(groups = {FirstGroup.class}) String arg0) { return null; }