Ensure resolved bean instance is used for validation

Closes gh-624
spring-projects · Mar 6, 2023 · 581b110 · 581b110
1 parent 3b100c6
commit 581b110
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 34 deletions.
diff --git a/...springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java b/...springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java
@@ -27,6 +27,7 @@
 import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.concurrent.Executor;
+import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
@@ -492,7 +493,7 @@ static class SchemaMappingDataFetcher implements DataFetcher<Object> {
 		private final HandlerMethodArgumentResolverComposite argumentResolvers;
 
 		@Nullable
-		private final Consumer<Object[]> methodValidationHelper;
+		private final BiConsumer<Object, Object[]> methodValidationHelper;
 
 		@Nullable
 		private final Executor executor;

diff --git a/.../org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethod.java b/.../org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethod.java
@@ -17,7 +17,7 @@
 
 import java.util.Arrays;
 import java.util.concurrent.Executor;
-import java.util.function.Consumer;
+import java.util.function.BiConsumer;
 
 import graphql.schema.DataFetchingEnvironment;
 import org.reactivestreams.Publisher;
@@ -50,7 +50,7 @@ public class DataFetcherHandlerMethod extends InvocableHandlerMethodSupport {
 
 	private final HandlerMethodArgumentResolverComposite resolvers;
 
-	private final Consumer<Object[]> validationHelper;
+	private final BiConsumer<Object, Object[]> validationHelper;
 
 	private final ParameterNameDiscoverer parameterNameDiscoverer = new DefaultParameterNameDiscoverer();
 
@@ -66,12 +66,13 @@ public class DataFetcherHandlerMethod extends InvocableHandlerMethodSupport {
 	 */
 	public DataFetcherHandlerMethod(
 			HandlerMethod handlerMethod, HandlerMethodArgumentResolverComposite resolvers,
-			@Nullable Consumer<Object[]> validationHelper, @Nullable Executor executor, boolean subscription) {
+			@Nullable BiConsumer<Object, Object[]> validationHelper, @Nullable Executor executor,
+			boolean subscription) {
 
 		super(handlerMethod, executor);
 		Assert.isTrue(!resolvers.getResolvers().isEmpty(), "No argument resolvers");
 		this.resolvers = resolvers;
-		this.validationHelper = (validationHelper != null ? validationHelper : args -> {});
+		this.validationHelper = (validationHelper != null ? validationHelper : (controller, args) -> {});
 		this.subscription = subscription;
 	}
 
@@ -179,7 +180,7 @@ private Object[] getMethodArgumentValues(
 
 	@Nullable
 	private Object validateAndInvoke(Object[] args, DataFetchingEnvironment environment) {
-		this.validationHelper.accept(args);
+		this.validationHelper.accept(getBean(), args);
 		return doInvoke(environment.getGraphQlContext(), args);
 	}
 

diff --git a/...ain/java/org/springframework/graphql/data/method/annotation/support/ValidationHelper.java b/...ain/java/org/springframework/graphql/data/method/annotation/support/ValidationHelper.java
@@ -16,8 +16,9 @@
 package org.springframework.graphql.data.method.annotation.support;
 
 import java.lang.annotation.Annotation;
+import java.lang.reflect.Method;
 import java.util.Set;
-import java.util.function.Consumer;
+import java.util.function.BiConsumer;
 
 import jakarta.validation.Constraint;
 import jakarta.validation.ConstraintViolation;
@@ -61,7 +62,7 @@ private ValidationHelper(Validator validator) {
 	 * {@link Validated}, {@link Valid}, or {@link Constraint} annotations.
 	 */
 	@Nullable
-	public Consumer<Object[]> getValidationHelperFor(HandlerMethod handlerMethod) {
+	public BiConsumer<Object, Object[]> getValidationHelperFor(HandlerMethod handlerMethod) {
 
 		boolean requiresMethodValidation = false;
 		Class<?>[] methodValidationGroups = null;
@@ -75,7 +76,7 @@ else if (findAnnotation(handlerMethod, Valid.class) != null) {
 			requiresMethodValidation = true;
 		}
 
-		Consumer<Object[]> parameterValidator = null;
+		BiConsumer<Object, Object[]> parameterValidator = null;
 		MethodParameter[] parameters = handlerMethod.getMethodParameters();
 
 		for (int i = 0; i < parameters.length; i++) {
@@ -94,7 +95,7 @@ else if (annot.annotationType().equals(Validated.class)) {
 			}
 		}
 
-		Consumer<Object[]> result = (requiresMethodValidation ?
+		BiConsumer<Object, Object[]> result = (requiresMethodValidation ?
 				new HandlerMethodValidator(handlerMethod, methodValidationGroups) : null);
 
 		if (parameterValidator != null) {
@@ -141,24 +142,24 @@ public static ValidationHelper create(Validator validator) {
 	/**
 	 * Callback to apply validation to the invocation of a {@link HandlerMethod}.
 	 */
-	private class HandlerMethodValidator implements Consumer<Object[]> {
+	private class HandlerMethodValidator implements BiConsumer<Object, Object[]> {
 
-		private final HandlerMethod handlerMethod;
+		private final Method method;
 
 		private final Class<?>[] validationGroups;
 
 		HandlerMethodValidator(HandlerMethod handlerMethod, @Nullable Class<?>[] validationGroups) {
 			Assert.notNull(handlerMethod, "HandlerMethod is required");
-			this.handlerMethod = handlerMethod;
+			this.method = handlerMethod.getMethod();
 			this.validationGroups = (validationGroups != null ? validationGroups : new Class<?>[] {});
 		}
 
 		@Override
-		public void accept(Object[] arguments) {
+		public void accept(Object controller, Object[] arguments) {
 
 			Set<ConstraintViolation<Object>> violations =
-					ValidationHelper.this.validator.forExecutables().validateParameters(
-							this.handlerMethod.getBean(), this.handlerMethod.getMethod(), arguments, this.validationGroups);
+					ValidationHelper.this.validator.forExecutables()
+							.validateParameters(controller, this.method, arguments, this.validationGroups);
 
 			if (!violations.isEmpty()) {
 				throw new ConstraintViolationException(violations);
@@ -172,7 +173,7 @@ public void accept(Object[] arguments) {
 	 * because it's annotated with Spring's {@code @Validated} rather than with
 	 * {@code @Valid}.
 	 */
-	private class MethodParameterValidator implements Consumer<Object[]> {
+	private class MethodParameterValidator implements BiConsumer<Object, Object[]> {
 
 		private final int index;
 
@@ -184,7 +185,7 @@ private class MethodParameterValidator implements Consumer<Object[]> {
 		}
 
 		@Override
-		public void accept(Object[] arguments) {
+		public void accept(Object controller, Object[] arguments) {
 
 			Set<ConstraintViolation<Object>> violations =
 					ValidationHelper.this.validator.validate(arguments[this.index], this.validationGroups);

diff --git a/...ava/org/springframework/graphql/data/method/annotation/support/ValidationHelperTests.java b/...ava/org/springframework/graphql/data/method/annotation/support/ValidationHelperTests.java
@@ -20,7 +20,7 @@
 import java.lang.annotation.RetentionPolicy;
 import java.lang.reflect.Method;
 import java.util.Arrays;
-import java.util.function.Consumer;
+import java.util.function.BiConsumer;
 
 import jakarta.validation.ConstraintViolation;
 import jakarta.validation.ConstraintViolationException;
@@ -54,45 +54,49 @@ void shouldFailWithNullValidator() {
 
 	@Test
 	void shouldIgnoreMethodsWithoutAnnotations() {
-		Consumer<Object[]> validator = createValidator(MyBean.class, "notValidatedMethod");
+		BiConsumer<Object, Object[]> validator = createValidator(MyBean.class, "notValidatedMethod");
 		assertThat(validator).isNull();
 	}
 
 	@Test
 	void shouldRaiseValidationErrorForAnnotatedParams() {
-		Consumer<Object[]> validator1 = createValidator(MyBean.class, "myValidMethod");
-		assertViolation(() -> validator1.accept(new Object[] {null, 2}), "myValidMethod.arg0");
-		assertViolation(() -> validator1.accept(new Object[] {"test", 12}), "myValidMethod.arg1");
+		MyBean bean = new MyBean();
 
-		Consumer<Object[]> validator2 = createValidator(MyBean.class, "myValidatedParameterMethod");
-		assertViolation(() -> validator2.accept(new Object[] {new ConstrainedInput(100)}), "integerValue");
+		BiConsumer<Object, Object[]> validator1 = createValidator(MyBean.class, "myValidMethod");
+		assertViolation(() -> validator1.accept(bean, new Object[] {null, 2}), "myValidMethod.arg0");
+		assertViolation(() -> validator1.accept(bean, new Object[] {"test", 12}), "myValidMethod.arg1");
+
+		BiConsumer<Object, Object[]> validator2 = createValidator(MyBean.class, "myValidatedParameterMethod");
+		assertViolation(() -> validator2.accept(bean, new Object[] {new ConstrainedInput(100)}), "integerValue");
 	}
 
 	@Test
 	void shouldRaiseValidationErrorForAnnotatedParamsWithGroups() {
-		Consumer<Object[]> validator1 = createValidator(MyValidationGroupsBean.class, "myValidMethodWithGroup");
-		assertViolation(() -> validator1.accept(new Object[] {null}), "myValidMethodWithGroup.arg0");
+		MyValidationGroupsBean bean = new MyValidationGroupsBean();
+
+		BiConsumer<Object, Object[]> validator1 = createValidator(MyValidationGroupsBean.class, "myValidMethodWithGroup");
+		assertViolation(() -> validator1.accept(bean, new Object[] {null}), "myValidMethodWithGroup.arg0");
 
-		Consumer<Object[]> validator2 = createValidator(MyValidationGroupsBean.class, "myValidMethodWithGroupOnType");
-		assertViolation(() -> validator2.accept(new Object[] {null}), "myValidMethodWithGroupOnType.arg0");
+		BiConsumer<Object, Object[]> validator2 = createValidator(MyValidationGroupsBean.class, "myValidMethodWithGroupOnType");
+		assertViolation(() -> validator2.accept(bean, new Object[] {null}), "myValidMethodWithGroupOnType.arg0");
 	}
 
 	@Test
 	void shouldRecognizeMethodsThatRequireValidation() {
-		Consumer<Object[]> validator1 = createValidator(RequiresValidationBean.class, "processConstrainedValue");
+		BiConsumer<Object, Object[]> validator1 = createValidator(RequiresValidationBean.class, "processConstrainedValue");
 		assertThat(validator1).isNotNull();
 
-		Consumer<Object[]> validator2 = createValidator(RequiresValidationBean.class, "processValidInput");
+		BiConsumer<Object, Object[]> validator2 = createValidator(RequiresValidationBean.class, "processValidInput");
 		assertThat(validator2).isNotNull();
 
-		Consumer<Object[]> validator3 = createValidator(RequiresValidationBean.class, "processValidatedInput");
+		BiConsumer<Object, Object[]> validator3 = createValidator(RequiresValidationBean.class, "processValidatedInput");
 		assertThat(validator3).isNotNull();
 
-		Consumer<Object[]> validator4 = createValidator(RequiresValidationBean.class, "processValue");
+		BiConsumer<Object, Object[]> validator4 = createValidator(RequiresValidationBean.class, "processValue");
 		assertThat(validator4).isNull();
 	}
 
-	private Consumer<Object[]> createValidator(Class<?> handlerType, String methodName) {
+	private BiConsumer<Object, Object[]> createValidator(Class<?> handlerType, String methodName) {
 		return ValidationHelper.create(Validation.buildDefaultValidatorFactory().getValidator())
 				.getValidationHelperFor(findHandlerMethod(handlerType, methodName));
 	}