Revert use of leafBean in MethodValidationAdapter

The goal for #31530 was to support bean validation on Set and other
method parameters that are containers of value(s) for which there is
a registered Jakarta Validation ValueExtractor.

Unfortunately, bean validation does not expose the unwrapped value
for a Path.Node, which we need for a method parameter in order to
create a BindingResult for the specific bean within the container,
and the leafBean that we tried to use is really the node at the
very bottom of the property path (i.e. not what we need).

This change removes the use of beanLeaf, restores the logic as it
was before, adds support for arrays, and a new test class for
scenarios with cascaded violations.

See gh-31746

Author: rstoyanchev

spring-context/src/main/java/org/springframework/validation/beanvalidation/MethodValidationAdapter.java

@@ -49,6 +49,7 @@
 import org.springframework.core.ParameterNameDiscoverer;
 import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.lang.Nullable;
+import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.function.SingletonSupplier;
 import org.springframework.validation.BeanPropertyBindingResult;
@@ -302,7 +303,7 @@ private MethodValidationResult adaptViolations(
 			Function<Integer, Object> argumentFunction) {
 
 		Map<MethodParameter, ParamResultBuilder> paramViolations = new LinkedHashMap<>();
-		Map<BeanResultKey, BeanResultBuilder> beanViolations = new LinkedHashMap<>();
+		Map<Path.Node, BeanResultBuilder> beanViolations = new LinkedHashMap<>();
 
 		for (ConstraintViolation<Object> violation : violations) {
 			Iterator<Path.Node> itr = violation.getPropertyPath().iterator();
@@ -328,10 +329,37 @@ else if (node.getKind().equals(ElementKind.RETURN_VALUE)) {
 							.addViolation(violation);
 				}
 				else {
-					Object leafBean = violation.getLeafBean();
-					BeanResultKey key = new BeanResultKey(node, leafBean);
+					// If the argument is a container of elements, we need the specific element,
+					// but the only option is to check for a parent container index/key in the
+					// next part of the property path.
+					Path.Node paramNode = node;
+					node = itr.next();
+
+					Object bean;
+					Object container;
+					Integer containerIndex = node.getIndex();
+					Object containerKey = node.getKey();
+					if (containerIndex != null && arg instanceof List<?> list) {
+						bean = list.get(containerIndex);
+						container = list;
+					}
+					else if (containerIndex != null && arg instanceof Object[] array) {
+						bean = array[containerIndex];
+						container = array;
+					}
+					else if (containerKey != null && arg instanceof Map<?, ?> map) {
+						bean = map.get(containerKey);
+						container = map;
+					}
+					else {
+						Assert.state(!node.isInIterable(), "No way to unwrap Iterable without index");
+						bean = arg;
+						container = null;
+					}
+
 					beanViolations
-							.computeIfAbsent(key, k -> new BeanResultBuilder(parameter, arg, itr.next(), leafBean))
+							.computeIfAbsent(paramNode, k ->
+									new BeanResultBuilder(parameter, bean, container, containerIndex, containerKey))
 							.addViolation(violation);
 				}
 				break;
@@ -448,13 +476,16 @@ private final class BeanResultBuilder {
 
 		private final Set<ConstraintViolation<Object>> violations = new LinkedHashSet<>();
 
-		public BeanResultBuilder(MethodParameter param, @Nullable Object arg, Path.Node node, @Nullable Object leafBean) {
+		public BeanResultBuilder(
+				MethodParameter param, @Nullable Object bean, @Nullable Object container,
+				@Nullable Integer containerIndex, @Nullable Object containerKey) {
+
 			this.parameter = param;
-			this.bean = leafBean;
-			this.container = (arg != null && !arg.equals(leafBean) ? arg : null);
-			this.containerIndex = node.getIndex();
-			this.containerKey = node.getKey();
-			this.errors = createBindingResult(param, leafBean);
+			this.bean = bean;
+			this.container = container;
+			this.containerIndex = containerIndex;
+			this.containerKey = containerKey;
+			this.errors = createBindingResult(param, this.bean);
 		}
 
 		public void addViolation(ConstraintViolation<Object> violation) {
@@ -470,22 +501,6 @@ public ParameterErrors build() {
 	}
 
 
-	/**
-	 * Unique key for cascaded violations associated with a bean.
-	 * <p>The bean may be an element within a container such as a List, Set, array,
-	 * Map, Optional, and others. In that case the {@link Path.Node} represents
-	 * the container element with its index or key, if applicable, while the
-	 * {@link ConstraintViolation#getLeafBean() leafBean} is the actual
-	 * element instance. The pair should be unique. For example in a Set, the
-	 * node is the same but element instances are unique. In a List or Map the
-	 * node is further qualified by an index or key while element instances
-	 * may be the same.
-	 * @param node the path to the bean associated with the violation
-	 * @param leafBean the bean instance
-	 */
-	record BeanResultKey(Path.Node node, Object leafBean) { }
-
-
 	/**
 	 * Default algorithm to select an object name, as described in
 	 * {@link #setObjectNameResolver(ObjectNameResolver)}.

spring-context/src/main/java/org/springframework/validation/method/ParameterErrors.java

@@ -32,12 +32,11 @@
  * {@link Errors#getAllErrors()}, but this subclass provides access to the same
  * as {@link FieldError}s.
  *
- * <p>When the method parameter is a container with multiple elements such as a
- * {@link List}, {@link java.util.Set}, array, {@link java.util.Map}, or others,
- * then a separate {@link ParameterErrors} is created for each element that has
- * errors. In that case, the {@link #getContainer() container},
- * {@link #getContainerIndex() containerIndex}, and {@link #getContainerKey() containerKey}
- * provide additional context.
+ * <p>When the method parameter is a container such as a {@link List}, array,
+ * or {@link java.util.Map}, then a separate {@link ParameterErrors} is created
+ * for each element that has errors. In that case, the properties
+ * {@link #getContainer() container}, {@link #getContainerIndex() containerIndex},
+ * and {@link #getContainerKey() containerKey} provide additional context.
  *
  * @author Rossen Stoyanchev
  * @since 6.1

spring-context/src/test/java/org/springframework/validation/beanvalidation/MethodValidationAdapterPropertyPathTests.java

@@ -0,0 +1,243 @@
+/*
+ * Copyright 2002-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.
+ * 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.validation.beanvalidation;
+
+import java.lang.reflect.Method;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+import jakarta.validation.Valid;
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.Size;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+
+import org.springframework.lang.Nullable;
+import org.springframework.util.ClassUtils;
+import org.springframework.validation.FieldError;
+import org.springframework.validation.method.MethodValidationResult;
+import org.springframework.validation.method.ParameterErrors;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Test method validation scenarios with cascaded violations on different types
+ * of method parameters and return values.
+ *
+ * @author Rossen Stoyanchev
+ */
+public class MethodValidationAdapterPropertyPathTests {
+
+	private static final Person validPerson = new Person("John");
+
+	private static final Person invalidPerson = new Person("Long John Silver");
+
+	private static final Class<?>[] HINTS = new Class<?>[0];
+
+
+	private final MethodValidationAdapter validationAdapter = new MethodValidationAdapter();
+
+
+	@Nested
+	class ArgumentTests {
+
+		@Test
+		void fieldOfObjectPropertyOfBean() {
+			Method method = getMethod("addCourse");
+			Object[] args = {new Course("CS 101", invalidPerson, Collections.emptyList())};
+
+			MethodValidationResult result =
+					validationAdapter.validateArguments(new MyService(), method, null, args, HINTS);
+
+			assertThat(result.getAllErrors()).hasSize(1);
+			ParameterErrors errors = result.getBeanResults().get(0);
+			assertSingleFieldError(errors, 1, null, null, null, "professor.name", invalidPerson.name());
+		}
+
+		@Test
+		void fieldOfObjectPropertyOfListElement() {
+			Method method = getMethod("addCourseList");
+			List<Course> courses = List.of(new Course("CS 101", invalidPerson, Collections.emptyList()));
+
+			MethodValidationResult result = validationAdapter.validateArguments(
+					new MyService(), method, null, new Object[] {courses}, HINTS);
+
+			assertThat(result.getAllErrors()).hasSize(1);
+			ParameterErrors errors = result.getBeanResults().get(0);
+			assertSingleFieldError(errors, 1, courses, 0, null, "professor.name", invalidPerson.name());
+		}
+
+		@Test
+		void fieldOfObjectPropertyOfListElements() {
+			Method method = getMethod("addCourseList");
+			List<Course> courses = List.of(
+					new Course("CS 101", invalidPerson, Collections.emptyList()),
+					new Course("CS 102", invalidPerson, Collections.emptyList()));
+
+			MethodValidationResult result = validationAdapter.validateArguments(
+					new MyService(), method, null, new Object[] {courses}, HINTS);
+
+			assertThat(result.getAllErrors()).hasSize(2);
+			for (int i = 0; i < 2; i++) {
+				ParameterErrors errors = result.getBeanResults().get(i);
+				assertThat(errors.getContainerIndex()).isEqualTo(i);
+				assertThat(errors.getFieldError().getField()).isEqualTo("professor.name");
+			}
+
+		}
+
+		@Test
+		void fieldOfObjectPropertyUnderListPropertyOfListElement() {
+			Method method = getMethod("addCourseList");
+			Course cs101 = new Course("CS 101", invalidPerson, Collections.emptyList());
+			Course cs201 = new Course("CS 201", validPerson, List.of(cs101));
+			Course cs301 = new Course("CS 301", validPerson, List.of(cs201));
+			List<Course> courses = List.of(cs301);
+			Object[] args = {courses};
+
+			MethodValidationResult result =
+					validationAdapter.validateArguments(new MyService(), method, null, args, HINTS);
+
+			assertThat(result.getAllErrors()).hasSize(1);
+			ParameterErrors errors = result.getBeanResults().get(0);
+
+			assertSingleFieldError(errors, 1, courses, 0, null,
+					"requiredCourses[0].requiredCourses[0].professor.name", invalidPerson.name());
+		}
+
+		@Test
+		void fieldOfObjectPropertyOfArrayElement() {
+			Method method = getMethod("addCourseArray");
+			Course[] courses = new Course[] {new Course("CS 101", invalidPerson, Collections.emptyList())};
+
+			MethodValidationResult result = validationAdapter.validateArguments(
+					new MyService(), method, null, new Object[] {courses}, HINTS);
+
+			assertThat(result.getAllErrors()).hasSize(1);
+			ParameterErrors errors = result.getBeanResults().get(0);
+			assertSingleFieldError(errors, 1, courses, 0, null, "professor.name", invalidPerson.name());
+		}
+
+		@Test
+		void fieldOfObjectPropertyOfMapValue() {
+			Method method = getMethod("addCourseMap");
+			Map<String, Course> courses = Map.of("CS 101", new Course("CS 101", invalidPerson, Collections.emptyList()));
+
+			MethodValidationResult result = validationAdapter.validateArguments(
+					new MyService(), method, null, new Object[] {courses}, HINTS);
+
+			assertThat(result.getAllErrors()).hasSize(1);
+			ParameterErrors errors = result.getBeanResults().get(0);
+			assertSingleFieldError(errors, 1, courses, null, "CS 101", "professor.name", invalidPerson.name());
+		}
+
+	}
+
+
+	@Nested
+	class ReturnValueTests {
+
+		@Test
+		void fieldOfObjectPropertyOfBean() {
+			Method method = getMethod("getCourse");
+			Course course = new Course("CS 101", invalidPerson, Collections.emptyList());
+
+			MethodValidationResult result =
+					validationAdapter.validateReturnValue(new MyService(), method, null, course, HINTS);
+
+			assertThat(result.getAllErrors()).hasSize(1);
+			ParameterErrors errors = result.getBeanResults().get(0);
+			assertSingleFieldError(errors, 1, null, null, null, "professor.name", invalidPerson.name());
+		}
+
+		@Test
+		void fieldOfObjectPropertyOfListElement() {
+			Method method = getMethod("addCourseList");
+			List<Course> courses = List.of(new Course("CS 101", invalidPerson, Collections.emptyList()));
+
+			MethodValidationResult result = validationAdapter.validateArguments(
+					new MyService(), method, null, new Object[] {courses}, HINTS);
+
+			assertThat(result.getAllErrors()).hasSize(1);
+			ParameterErrors errors = result.getBeanResults().get(0);
+			assertSingleFieldError(errors, 1, courses, 0, null, "professor.name", invalidPerson.name());
+		}
+
+	}
+
+
+	private void assertSingleFieldError(
+			ParameterErrors errors, int errorCount,
+			@Nullable Object container, @Nullable Integer index, @Nullable Object key,
+			String field, Object rejectedValue) {
+
+		assertThat(errors.getErrorCount()).isEqualTo(errorCount);
+		assertThat(errors.getErrorCount()).isEqualTo(1);
+		assertThat(errors.getContainer()).isEqualTo(container);
+		assertThat(errors.getContainerIndex()).isEqualTo(index);
+		assertThat(errors.getContainerKey()).isEqualTo(key);
+
+		FieldError fieldError = errors.getFieldError();
+		assertThat(fieldError).isNotNull();
+		assertThat(fieldError.getField()).isEqualTo(field);
+		assertThat(fieldError.getRejectedValue()).isEqualTo(rejectedValue);
+	}
+
+
+	private static Method getMethod(String methodName) {
+		return ClassUtils.getMethod(MyService.class, methodName, (Class<?>[]) null);
+	}
+
+
+	@SuppressWarnings("unused")
+	private static class MyService {
+
+		public void addCourse(@Valid Course course) {
+		}
+
+		public void addCourseList(@Valid List<Course> courses) {
+		}
+
+		public void addCourseArray(@Valid Course[] courses) {
+		}
+
+		public void addCourseMap(@Valid Map<String, Course> courses) {
+		}
+
+		@Valid
+		public Course getCourse(Course course) {
+			throw new UnsupportedOperationException();
+		}
+
+		@Valid
+		public List<Course> getCourseList() {
+			throw new UnsupportedOperationException();
+		}
+
+	}
+
+
+	private record Course(@NotBlank String title, @Valid Person professor, @Valid List<Course> requiredCourses) {
+	}
+
+
+	@SuppressWarnings("unused")
+	private record Person(@Size(min = 1, max = 5) String name) {
+	}
+
+}