Consistently expose parameter annotations from base classes as well

Closes gh-25788
spring-projects · Jan 8, 2024 · bb1cdb6 · bb1cdb6
1 parent 37fa82c
commit bb1cdb6
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 13 deletions.
diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedMethod.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedMethod.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 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.
@@ -28,7 +28,6 @@
 import org.springframework.lang.NonNull;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
-import org.springframework.util.ClassUtils;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.ReflectionUtils;
 import org.springframework.util.StringUtils;
@@ -54,7 +53,7 @@ public class AnnotatedMethod {
 	private final MethodParameter[] parameters;
 
 	@Nullable
-	private volatile List<Annotation[][]> interfaceParameterAnnotations;
+	private volatile List<Annotation[][]> inheritedParameterAnnotations;
 
 
 	/**
@@ -77,7 +76,7 @@ protected AnnotatedMethod(AnnotatedMethod annotatedMethod) {
 		this.method = annotatedMethod.method;
 		this.bridgedMethod = annotatedMethod.bridgedMethod;
 		this.parameters = annotatedMethod.parameters;
-		this.interfaceParameterAnnotations = annotatedMethod.interfaceParameterAnnotations;
+		this.inheritedParameterAnnotations = annotatedMethod.inheritedParameterAnnotations;
 	}
 
 
@@ -164,18 +163,32 @@ public <A extends Annotation> boolean hasMethodAnnotation(Class<A> annotationTyp
 		return AnnotatedElementUtils.hasAnnotation(this.method, annotationType);
 	}
 
-	private List<Annotation[][]> getInterfaceParameterAnnotations() {
-		List<Annotation[][]> parameterAnnotations = this.interfaceParameterAnnotations;
+	private List<Annotation[][]> getInheritedParameterAnnotations() {
+		List<Annotation[][]> parameterAnnotations = this.inheritedParameterAnnotations;
 		if (parameterAnnotations == null) {
 			parameterAnnotations = new ArrayList<>();
-			for (Class<?> ifc : ClassUtils.getAllInterfacesForClassAsSet(this.method.getDeclaringClass())) {
-				for (Method candidate : ifc.getMethods()) {
-					if (isOverrideFor(candidate)) {
-						parameterAnnotations.add(candidate.getParameterAnnotations());
+			Class<?> clazz = this.method.getDeclaringClass();
+			while (clazz != null) {
+				for (Class<?> ifc : clazz.getInterfaces()) {
+					for (Method candidate : ifc.getMethods()) {
+						if (isOverrideFor(candidate)) {
+							parameterAnnotations.add(candidate.getParameterAnnotations());
+						}
+					}
+				}
+				clazz = clazz.getSuperclass();
+				if (clazz == Object.class) {
+					clazz = null;
+				}
+				if (clazz != null) {
+					for (Method candidate : clazz.getMethods()) {
+						if (isOverrideFor(candidate)) {
+							parameterAnnotations.add(candidate.getParameterAnnotations());
+						}
 					}
 				}
 			}
-			this.interfaceParameterAnnotations = parameterAnnotations;
+			this.inheritedParameterAnnotations = parameterAnnotations;
 		}
 		return parameterAnnotations;
 	}
@@ -281,7 +294,7 @@ public Annotation[] getParameterAnnotations() {
 				anns = super.getParameterAnnotations();
 				int index = getParameterIndex();
 				if (index >= 0) {
-					for (Annotation[][] ifcAnns : getInterfaceParameterAnnotations()) {
+					for (Annotation[][] ifcAnns : getInheritedParameterAnnotations()) {
 						if (index < ifcAnns.length) {
 							Annotation[] paramAnns = ifcAnns[index];
 							if (paramAnns.length > 0) {

diff --git a/...gframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java b/...gframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 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.
@@ -765,6 +765,24 @@ void resolveArgumentTypeVariableWithGenericInterfaceAndSubclass() throws Excepti
 		assertThat(value).isEqualTo("foo");
 	}
 
+	@Test  // gh-25788
+	void resolveArgumentTypeVariableWithAbstractMethod() throws Exception {
+		this.servletRequest.setContent("\"foo\"".getBytes(StandardCharsets.UTF_8));
+		this.servletRequest.setContentType(MediaType.APPLICATION_JSON_VALUE);
+
+		Method method = SubControllerImplementingAbstractMethod.class.getMethod("handle", Object.class);
+		HandlerMethod handlerMethod = new HandlerMethod(new SubControllerImplementingAbstractMethod(), method);
+		MethodParameter methodParameter = handlerMethod.getMethodParameters()[0];
+
+		List<HttpMessageConverter<?>> converters = List.of(new MappingJackson2HttpMessageConverter());
+		RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor(converters);
+
+		assertThat(processor.supportsParameter(methodParameter)).isTrue();
+		String value = (String) processor.readWithMessageConverters(
+				this.request, methodParameter, methodParameter.getGenericParameterType());
+		assertThat(value).isEqualTo("foo");
+	}
+
 	private void assertContentDisposition(RequestResponseBodyMethodProcessor processor,
 			boolean expectContentDisposition, String requestURI, String comment) throws Exception {
 
@@ -1149,4 +1167,19 @@ public String handle(String arg) {
 		}
 	}
 
+
+	abstract static class MyControllerWithAbstractMethod<A> {
+
+		public abstract A handle(@RequestBody A arg);
+	}
+
+
+	static class SubControllerImplementingAbstractMethod extends MyControllerWithAbstractMethod<String> {
+
+		@Override
+		public String handle(String arg) {
+			return arg;
+		}
+	}
+
 }