diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java index dac7216e0feb..0196a47b7495 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java @@ -191,21 +191,20 @@ private RequestMappingInfo createRequestMappingInfo(AnnotatedElement element) { RequestCondition customCondition = (element instanceof Class clazz ? getCustomTypeCondition(clazz) : getCustomMethodCondition((Method) element)); - MergedAnnotations mergedAnnotations = MergedAnnotations.from(element, SearchStrategy.TYPE_HIERARCHY, - RepeatableContainers.none()); + List descriptors = getAnnotationDescriptors(element); - List> requestMappings = getAnnotationDescriptors( - mergedAnnotations, RequestMapping.class); + List requestMappings = descriptors.stream() + .filter(desc -> desc.annotation instanceof RequestMapping).toList(); if (!requestMappings.isEmpty()) { if (requestMappings.size() > 1 && logger.isWarnEnabled()) { logger.warn("Multiple @RequestMapping annotations found on %s, but only the first will be used: %s" .formatted(element, requestMappings)); } - requestMappingInfo = createRequestMappingInfo(requestMappings.get(0).annotation, customCondition); + requestMappingInfo = createRequestMappingInfo((RequestMapping) requestMappings.get(0).annotation, customCondition); } - List> httpExchanges = getAnnotationDescriptors( - mergedAnnotations, HttpExchange.class); + List httpExchanges = descriptors.stream() + .filter(desc -> desc.annotation instanceof HttpExchange).toList(); if (!httpExchanges.isEmpty()) { Assert.state(requestMappingInfo == null, () -> "%s is annotated with @RequestMapping and @HttpExchange annotations, but only one is allowed: %s" @@ -213,7 +212,7 @@ private RequestMappingInfo createRequestMappingInfo(AnnotatedElement element) { Assert.state(httpExchanges.size() == 1, () -> "Multiple @HttpExchange annotations found on %s, but only one is allowed: %s" .formatted(element, httpExchanges)); - requestMappingInfo = createRequestMappingInfo(httpExchanges.get(0).annotation, customCondition); + requestMappingInfo = createRequestMappingInfo((HttpExchange) httpExchanges.get(0).annotation, customCondition); } return requestMappingInfo; @@ -438,29 +437,29 @@ private String resolveCorsAnnotationValue(String value) { } } - private static List> getAnnotationDescriptors( - MergedAnnotations mergedAnnotations, Class annotationType) { - - return mergedAnnotations.stream(annotationType) + private static List getAnnotationDescriptors(AnnotatedElement element) { + return MergedAnnotations.from(element, SearchStrategy.TYPE_HIERARCHY, RepeatableContainers.none()) + .stream() + .filter(MergedAnnotationPredicates.typeIn(RequestMapping.class, HttpExchange.class)) .filter(MergedAnnotationPredicates.firstRunOf(MergedAnnotation::getAggregateIndex)) .map(AnnotationDescriptor::new) .distinct() .toList(); } - private static class AnnotationDescriptor { + private static class AnnotationDescriptor { - private final A annotation; + private final Annotation annotation; private final MergedAnnotation root; - AnnotationDescriptor(MergedAnnotation mergedAnnotation) { + AnnotationDescriptor(MergedAnnotation mergedAnnotation) { this.annotation = mergedAnnotation.synthesize(); this.root = mergedAnnotation.getRoot(); } @Override public boolean equals(Object obj) { - return (obj instanceof AnnotationDescriptor that && this.annotation.equals(that.annotation)); + return (obj instanceof AnnotationDescriptor that && this.annotation.equals(that.annotation)); } @Override diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java index 4c91e819842b..4662a529265b 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java @@ -222,6 +222,36 @@ void httpExchangeWithMixedAnnotationsAtMethodLevel() throws NoSuchMethodExceptio ); } + @Test // gh-32065 + void httpExchangeAnnotationsOverriddenAtClassLevel() throws NoSuchMethodException { + this.handlerMapping.afterPropertiesSet(); + + Class controllerClass = ClassLevelOverriddenHttpExchangeAnnotationsController.class; + Method method = controllerClass.getDeclaredMethod("post"); + + RequestMappingInfo info = this.handlerMapping.getMappingForMethod(method, controllerClass); + + assertThat(info).isNotNull(); + assertThat(info.getPatternsCondition()).isNotNull(); + assertThat(info.getPatternsCondition().getPatterns()).extracting(PathPattern::getPatternString) + .containsOnly("/controller/postExchange"); + } + + @Test // gh-32065 + void httpExchangeAnnotationsOverriddenAtMethodLevel() throws NoSuchMethodException { + this.handlerMapping.afterPropertiesSet(); + + Class controllerClass = MethodLevelOverriddenHttpExchangeAnnotationsController.class; + Method method = controllerClass.getDeclaredMethod("post"); + + RequestMappingInfo info = this.handlerMapping.getMappingForMethod(method, controllerClass); + + assertThat(info).isNotNull(); + assertThat(info.getPatternsCondition()).isNotNull(); + assertThat(info.getPatternsCondition().getPatterns()).extracting(PathPattern::getPatternString) + .containsOnly("/controller/postMapping"); + } + @SuppressWarnings("DataFlowIssue") @Test void httpExchangeWithDefaultValues() throws NoSuchMethodException { @@ -417,6 +447,33 @@ static class MixedMethodLevelAnnotationsController { void post() {} } + @HttpExchange("/service") + interface Service { + + @PostExchange("/postExchange") + void post(); + + } + + + @Controller + @RequestMapping("/controller") + static class ClassLevelOverriddenHttpExchangeAnnotationsController implements Service { + + @Override + public void post() {} + } + + + @Controller + @RequestMapping("/controller") + static class MethodLevelOverriddenHttpExchangeAnnotationsController implements Service { + + @PostMapping("/postMapping") + @Override + public void post() {} + } + @HttpExchange @Target(ElementType.TYPE) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java index d3340bc78c9f..02bfe40ee0b1 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java @@ -351,21 +351,20 @@ private RequestMappingInfo createRequestMappingInfo(AnnotatedElement element) { RequestCondition customCondition = (element instanceof Class clazz ? getCustomTypeCondition(clazz) : getCustomMethodCondition((Method) element)); - MergedAnnotations mergedAnnotations = MergedAnnotations.from(element, SearchStrategy.TYPE_HIERARCHY, - RepeatableContainers.none()); + List descriptors = getAnnotationDescriptors(element); - List> requestMappings = getAnnotationDescriptors( - mergedAnnotations, RequestMapping.class); + List requestMappings = descriptors.stream() + .filter(desc -> desc.annotation instanceof RequestMapping).toList(); if (!requestMappings.isEmpty()) { if (requestMappings.size() > 1 && logger.isWarnEnabled()) { logger.warn("Multiple @RequestMapping annotations found on %s, but only the first will be used: %s" .formatted(element, requestMappings)); } - requestMappingInfo = createRequestMappingInfo(requestMappings.get(0).annotation, customCondition); + requestMappingInfo = createRequestMappingInfo((RequestMapping) requestMappings.get(0).annotation, customCondition); } - List> httpExchanges = getAnnotationDescriptors( - mergedAnnotations, HttpExchange.class); + List httpExchanges = descriptors.stream() + .filter(desc -> desc.annotation instanceof HttpExchange).toList(); if (!httpExchanges.isEmpty()) { Assert.state(requestMappingInfo == null, () -> "%s is annotated with @RequestMapping and @HttpExchange annotations, but only one is allowed: %s" @@ -373,7 +372,7 @@ private RequestMappingInfo createRequestMappingInfo(AnnotatedElement element) { Assert.state(httpExchanges.size() == 1, () -> "Multiple @HttpExchange annotations found on %s, but only one is allowed: %s" .formatted(element, httpExchanges)); - requestMappingInfo = createRequestMappingInfo(httpExchanges.get(0).annotation, customCondition); + requestMappingInfo = createRequestMappingInfo((HttpExchange) httpExchanges.get(0).annotation, customCondition); } return requestMappingInfo; @@ -617,29 +616,29 @@ private String resolveCorsAnnotationValue(String value) { } } - private static List> getAnnotationDescriptors( - MergedAnnotations mergedAnnotations, Class annotationType) { - - return mergedAnnotations.stream(annotationType) + private static List getAnnotationDescriptors(AnnotatedElement element) { + return MergedAnnotations.from(element, SearchStrategy.TYPE_HIERARCHY, RepeatableContainers.none()) + .stream() + .filter(MergedAnnotationPredicates.typeIn(RequestMapping.class, HttpExchange.class)) .filter(MergedAnnotationPredicates.firstRunOf(MergedAnnotation::getAggregateIndex)) .map(AnnotationDescriptor::new) .distinct() .toList(); } - private static class AnnotationDescriptor { + private static class AnnotationDescriptor { - private final A annotation; + private final Annotation annotation; private final MergedAnnotation root; - AnnotationDescriptor(MergedAnnotation mergedAnnotation) { + AnnotationDescriptor(MergedAnnotation mergedAnnotation) { this.annotation = mergedAnnotation.synthesize(); this.root = mergedAnnotation.getRoot(); } @Override public boolean equals(Object obj) { - return (obj instanceof AnnotationDescriptor that && this.annotation.equals(that.annotation)); + return (obj instanceof AnnotationDescriptor that && this.annotation.equals(that.annotation)); } @Override diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java index a0c6f7c1e6c6..b52fc844f462 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java @@ -344,6 +344,48 @@ void httpExchangeWithMixedAnnotationsAtMethodLevel() throws NoSuchMethodExceptio ); } + @Test // gh-32065 + void httpExchangeAnnotationsOverriddenAtClassLevel() throws NoSuchMethodException { + RequestMappingHandlerMapping mapping = createMapping(); + + Class controllerClass = ClassLevelOverriddenHttpExchangeAnnotationsController.class; + Method method = controllerClass.getDeclaredMethod("post"); + + RequestMappingInfo info = mapping.getMappingForMethod(method, controllerClass); + + assertThat(info).isNotNull(); + assertThat(info.getActivePatternsCondition()).isNotNull(); + + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/service/postExchange"); + initRequestPath(mapping, request); + assertThat(info.getActivePatternsCondition().getMatchingCondition(request)).isNull(); + + request = new MockHttpServletRequest("POST", "/controller/postExchange"); + initRequestPath(mapping, request); + assertThat(info.getActivePatternsCondition().getMatchingCondition(request)).isNotNull(); + } + + @Test // gh-32065 + void httpExchangeAnnotationsOverriddenAtMethodLevel() throws NoSuchMethodException { + RequestMappingHandlerMapping mapping = createMapping(); + + Class controllerClass = MethodLevelOverriddenHttpExchangeAnnotationsController.class; + Method method = controllerClass.getDeclaredMethod("post"); + + RequestMappingInfo info = mapping.getMappingForMethod(method, controllerClass); + + assertThat(info).isNotNull(); + assertThat(info.getActivePatternsCondition()).isNotNull(); + + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/service/postExchange"); + initRequestPath(mapping, request); + assertThat(info.getActivePatternsCondition().getMatchingCondition(request)).isNull(); + + request = new MockHttpServletRequest("POST", "/controller/postMapping"); + initRequestPath(mapping, request); + assertThat(info.getActivePatternsCondition().getMatchingCondition(request)).isNotNull(); + } + @SuppressWarnings("DataFlowIssue") @Test void httpExchangeWithDefaultValues() throws NoSuchMethodException { @@ -542,6 +584,34 @@ void post() {} } + @HttpExchange("/service") + interface Service { + + @PostExchange("/postExchange") + void post(); + + } + + + @Controller + @RequestMapping("/controller") + static class ClassLevelOverriddenHttpExchangeAnnotationsController implements Service { + + @Override + public void post() {} + } + + + @Controller + @RequestMapping("/controller") + static class MethodLevelOverriddenHttpExchangeAnnotationsController implements Service { + + @PostMapping("/postMapping") + @Override + public void post() {} + } + + @HttpExchange @Target(ElementType.TYPE) @Retention(RetentionPolicy.RUNTIME)