Skip to content

Commit

Permalink
Log warning if multiple @⁠RequestMapping annotations are declared
Browse files Browse the repository at this point in the history
If multiple request mapping annotations are discovered, Spring MVC and
Spring WebFlux now log a warning similar to the following (without
newlines).

Multiple @⁠RequestMapping annotations found on
void org.example.MyController.put(), but only the first will be used:
[
@⁠org.springframework.web.bind.annotation.PutMapping(consumes={}, headers={}, name="", params={}, path={"/put"}, produces={}, value={"/put"}),
@⁠org.springframework.web.bind.annotation.PostMapping(consumes={}, headers={}, name="", params={}, path={"/put"}, produces={}, value={"/put"})
]

Closes gh-31962
  • Loading branch information
sbrannen committed Jan 17, 2024
1 parent 5bf74ca commit 699da7c
Show file tree
Hide file tree
Showing 12 changed files with 191 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ because, arguably, most controller methods should be mapped to a specific HTTP m
using `@RequestMapping`, which, by default, matches to all HTTP methods. At the same time, a
`@RequestMapping` is still needed at the class level to express shared mappings.

NOTE: `@RequestMapping` cannot be used in conjunction with other `@RequestMapping`
annotations that are declared on the same element (class, interface, or method). If
multiple `@RequestMapping` annotations are detected on the same element, a warning will
be logged, and only the first mapping will be used. This also applies to composed
`@RequestMapping` annotations such as `@GetMapping`, `@PostMapping`, etc.

The following example uses type and method level mappings:

[tabs]
Expand Down Expand Up @@ -439,6 +445,12 @@ controller methods should be mapped to a specific HTTP method versus using `@Req
which, by default, matches to all HTTP methods. If you need an example of how to implement
a composed annotation, look at how those are declared.

NOTE: `@RequestMapping` cannot be used in conjunction with other `@RequestMapping`
annotations that are declared on the same element (class, interface, or method). If
multiple `@RequestMapping` annotations are detected on the same element, a warning will
be logged, and only the first mapping will be used. This also applies to composed
`@RequestMapping` annotations such as `@GetMapping`, `@PostMapping`, etc.

Spring WebFlux also supports custom request mapping attributes with custom request matching
logic. This is a more advanced option that requires sub-classing
`RequestMappingHandlerMapping` and overriding the `getCustomMethodCondition` method, where
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ arguably, most controller methods should be mapped to a specific HTTP method ver
using `@RequestMapping`, which, by default, matches to all HTTP methods.
A `@RequestMapping` is still needed at the class level to express shared mappings.

NOTE: `@RequestMapping` cannot be used in conjunction with other `@RequestMapping`
annotations that are declared on the same element (class, interface, or method). If
multiple `@RequestMapping` annotations are detected on the same element, a warning will
be logged, and only the first mapping will be used. This also applies to composed
`@RequestMapping` annotations such as `@GetMapping`, `@PostMapping`, etc.

The following example has type and method level mappings:

[tabs]
Expand Down Expand Up @@ -489,6 +495,12 @@ controller methods should be mapped to a specific HTTP method versus using `@Req
which, by default, matches to all HTTP methods. If you need an example of how to implement
a composed annotation, look at how those are declared.

NOTE: `@RequestMapping` cannot be used in conjunction with other `@RequestMapping`
annotations that are declared on the same element (class, interface, or method). If
multiple `@RequestMapping` annotations are detected on the same element, a warning will
be logged, and only the first mapping will be used. This also applies to composed
`@RequestMapping` annotations such as `@GetMapping`, `@PostMapping`, etc.

Spring MVC also supports custom request-mapping attributes with custom request-matching
logic. This is a more advanced option that requires subclassing
`RequestMappingHandlerMapping` and overriding the `getCustomMethodCondition` method, where
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 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.
Expand Down Expand Up @@ -31,6 +31,13 @@
* <p>Specifically, {@code @DeleteMapping} is a <em>composed annotation</em> that
* acts as a shortcut for {@code @RequestMapping(method = RequestMethod.DELETE)}.
*
* <p><strong>NOTE:</strong> This annotation cannot be used in conjunction with
* other {@code @RequestMapping} annotations that are declared on the same method.
* If multiple {@code @RequestMapping} annotations are detected on the same method,
* a warning will be logged, and only the first mapping will be used. This applies
* to {@code @RequestMapping} as well as composed {@code @RequestMapping} annotations
* such as {@code @GetMapping}, {@code @PostMapping}, etc.
*
* @author Sam Brannen
* @since 4.3
* @see GetMapping
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 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.
Expand Down Expand Up @@ -31,6 +31,13 @@
* <p>Specifically, {@code @GetMapping} is a <em>composed annotation</em> that
* acts as a shortcut for {@code @RequestMapping(method = RequestMethod.GET)}.
*
* <p><strong>NOTE:</strong> This annotation cannot be used in conjunction with
* other {@code @RequestMapping} annotations that are declared on the same method.
* If multiple {@code @RequestMapping} annotations are detected on the same method,
* a warning will be logged, and only the first mapping will be used. This applies
* to {@code @RequestMapping} as well as composed {@code @RequestMapping} annotations
* such as {@code @PutMapping}, {@code @PostMapping}, etc.
*
* @author Sam Brannen
* @since 4.3
* @see PostMapping
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 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.
Expand Down Expand Up @@ -31,6 +31,13 @@
* <p>Specifically, {@code @PatchMapping} is a <em>composed annotation</em> that
* acts as a shortcut for {@code @RequestMapping(method = RequestMethod.PATCH)}.
*
* <p><strong>NOTE:</strong> This annotation cannot be used in conjunction with
* other {@code @RequestMapping} annotations that are declared on the same method.
* If multiple {@code @RequestMapping} annotations are detected on the same method,
* a warning will be logged, and only the first mapping will be used. This applies
* to {@code @RequestMapping} as well as composed {@code @RequestMapping} annotations
* such as {@code @GetMapping}, {@code @PostMapping}, etc.
*
* @author Sam Brannen
* @since 4.3
* @see GetMapping
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 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.
Expand Down Expand Up @@ -31,6 +31,13 @@
* <p>Specifically, {@code @PostMapping} is a <em>composed annotation</em> that
* acts as a shortcut for {@code @RequestMapping(method = RequestMethod.POST)}.
*
* <p><strong>NOTE:</strong> This annotation cannot be used in conjunction with
* other {@code @RequestMapping} annotations that are declared on the same method.
* If multiple {@code @RequestMapping} annotations are detected on the same method,
* a warning will be logged, and only the first mapping will be used. This applies
* to {@code @RequestMapping} as well as composed {@code @RequestMapping} annotations
* such as {@code @GetMapping}, {@code @PutMapping}, etc.
*
* @author Sam Brannen
* @since 4.3
* @see GetMapping
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 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.
Expand Down Expand Up @@ -31,6 +31,13 @@
* <p>Specifically, {@code @PutMapping} is a <em>composed annotation</em> that
* acts as a shortcut for {@code @RequestMapping(method = RequestMethod.PUT)}.
*
* <p><strong>NOTE:</strong> This annotation cannot be used in conjunction with
* other {@code @RequestMapping} annotations that are declared on the same method.
* If multiple {@code @RequestMapping} annotations are detected on the same method,
* a warning will be logged, and only the first mapping will be used. This applies
* to {@code @RequestMapping} as well as composed {@code @RequestMapping} annotations
* such as {@code @GetMapping}, {@code @PostMapping}, etc.
*
* @author Sam Brannen
* @since 4.3
* @see GetMapping
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@
* {@link PutMapping @PutMapping}, {@link DeleteMapping @DeleteMapping}, or
* {@link PatchMapping @PatchMapping}.
*
* <p><strong>NOTE:</strong> This annotation cannot be used in conjunction with
* other {@code @RequestMapping} annotations that are declared on the same element
* (class, interface, or method). If multiple {@code @RequestMapping} annotations
* are detected on the same element, a warning will be logged, and only the first
* mapping will be used. This also applies to composed {@code @RequestMapping}
* annotations such as {@code @GetMapping}, {@code @PostMapping}, etc.
*
* <p><b>NOTE:</b> When using controller interfaces (e.g. for AOP proxying),
* make sure to consistently put <i>all</i> your mapping annotations &mdash; such
* as {@code @RequestMapping} and {@code @SessionAttributes} &mdash; on
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -16,18 +16,23 @@

package org.springframework.web.reactive.result.method.annotation;

import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Predicate;

import org.springframework.context.EmbeddedValueResolverAware;
import org.springframework.core.annotation.AnnotatedElementUtils;
import org.springframework.core.annotation.MergedAnnotation;
import org.springframework.core.annotation.MergedAnnotationPredicates;
import org.springframework.core.annotation.MergedAnnotations;
import org.springframework.core.annotation.MergedAnnotations.SearchStrategy;
import org.springframework.core.annotation.RepeatableContainers;
import org.springframework.lang.Nullable;
import org.springframework.stereotype.Controller;
import org.springframework.util.Assert;
Expand Down Expand Up @@ -182,9 +187,20 @@ private RequestMappingInfo createRequestMappingInfo(AnnotatedElement element) {
RequestCondition<?> customCondition = (element instanceof Class<?> clazz ?
getCustomTypeCondition(clazz) : getCustomMethodCondition((Method) element));

RequestMapping requestMapping = AnnotatedElementUtils.findMergedAnnotation(element, RequestMapping.class);
if (requestMapping != null) {
return createRequestMappingInfo(requestMapping, customCondition);
MergedAnnotations mergedAnnotations = MergedAnnotations.from(element, SearchStrategy.TYPE_HIERARCHY,
RepeatableContainers.none());
List<AnnotationDescriptor<RequestMapping>> requestMappings = mergedAnnotations.stream(RequestMapping.class)
.filter(MergedAnnotationPredicates.firstRunOf(MergedAnnotation::getAggregateIndex))
.map(AnnotationDescriptor::new)
.distinct()
.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));
}
return createRequestMappingInfo(requestMappings.get(0).annotation, customCondition);
}

HttpExchange httpExchange = AnnotatedElementUtils.findMergedAnnotation(element, HttpExchange.class);
Expand Down Expand Up @@ -414,4 +430,32 @@ private String resolveCorsAnnotationValue(String value) {
}
}

private static class AnnotationDescriptor<A extends Annotation> {

private final A annotation;
private final Annotation source;

AnnotationDescriptor(MergedAnnotation<A> mergedAnnotation) {
this.annotation = mergedAnnotation.synthesize();
this.source = (mergedAnnotation.getDistance() > 0 ?
mergedAnnotation.getRoot().synthesize() : this.annotation);
}

@Override
public boolean equals(Object obj) {
return (obj instanceof AnnotationDescriptor<?> that && this.annotation.equals(that.annotation));
}

@Override
public int hashCode() {
return this.annotation.hashCode();
}

@Override
public String toString() {
return this.source.toString();
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PatchMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
Expand All @@ -60,6 +59,7 @@
*
* @author Rossen Stoyanchev
* @author Olga Maciaszek-Sharma
* @author Sam Brannen
*/
class RequestMappingHandlerMappingTests {

Expand Down Expand Up @@ -231,7 +231,9 @@ private RequestMappingInfo assertComposedAnnotationMapping(


@Controller @SuppressWarnings("unused")
// gh-31962: The presence of multiple @RequestMappings is intentional.
@RequestMapping(consumes = MediaType.APPLICATION_JSON_VALUE)
@ExtraRequestMapping
static class ComposedAnnotationController {

@RequestMapping
Expand All @@ -250,7 +252,10 @@ public void get() {
public void post(@RequestBody(required = false) Foo foo) {
}

@PutMapping("/put")
// gh-31962: The presence of multiple @RequestMappings is intentional.
@PatchMapping("/put")
@RequestMapping(path = "/put", method = RequestMethod.PUT) // local @RequestMapping overrides meta-annotations
@PostMapping("/put")
public void put() {
}

Expand All @@ -267,6 +272,13 @@ private static class Foo {
}


@RequestMapping
@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
@interface ExtraRequestMapping {
}


@RequestMapping(method = RequestMethod.POST,
produces = MediaType.APPLICATION_JSON_VALUE,
consumes = MediaType.APPLICATION_JSON_VALUE)
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -16,6 +16,7 @@

package org.springframework.web.servlet.mvc.method.annotation;

import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
Expand All @@ -30,7 +31,10 @@
import org.springframework.context.EmbeddedValueResolverAware;
import org.springframework.core.annotation.AnnotatedElementUtils;
import org.springframework.core.annotation.MergedAnnotation;
import org.springframework.core.annotation.MergedAnnotationPredicates;
import org.springframework.core.annotation.MergedAnnotations;
import org.springframework.core.annotation.MergedAnnotations.SearchStrategy;
import org.springframework.core.annotation.RepeatableContainers;
import org.springframework.lang.Nullable;
import org.springframework.stereotype.Controller;
import org.springframework.util.Assert;
Expand Down Expand Up @@ -343,9 +347,20 @@ private RequestMappingInfo createRequestMappingInfo(AnnotatedElement element) {
RequestCondition<?> customCondition = (element instanceof Class<?> clazz ?
getCustomTypeCondition(clazz) : getCustomMethodCondition((Method) element));

RequestMapping requestMapping = AnnotatedElementUtils.findMergedAnnotation(element, RequestMapping.class);
if (requestMapping != null) {
return createRequestMappingInfo(requestMapping, customCondition);
MergedAnnotations mergedAnnotations = MergedAnnotations.from(element, SearchStrategy.TYPE_HIERARCHY,
RepeatableContainers.none());
List<AnnotationDescriptor<RequestMapping>> requestMappings = mergedAnnotations.stream(RequestMapping.class)
.filter(MergedAnnotationPredicates.firstRunOf(MergedAnnotation::getAggregateIndex))
.map(AnnotationDescriptor::new)
.distinct()
.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));
}
return createRequestMappingInfo(requestMappings.get(0).annotation, customCondition);
}

HttpExchange httpExchange = AnnotatedElementUtils.findMergedAnnotation(element, HttpExchange.class);
Expand Down Expand Up @@ -594,4 +609,32 @@ private String resolveCorsAnnotationValue(String value) {
}
}

private static class AnnotationDescriptor<A extends Annotation> {

private final A annotation;
private final Annotation source;

AnnotationDescriptor(MergedAnnotation<A> mergedAnnotation) {
this.annotation = mergedAnnotation.synthesize();
this.source = (mergedAnnotation.getDistance() > 0 ?
mergedAnnotation.getRoot().synthesize() : this.annotation);
}

@Override
public boolean equals(Object obj) {
return (obj instanceof AnnotationDescriptor<?> that && this.annotation.equals(that.annotation));
}

@Override
public int hashCode() {
return this.annotation.hashCode();
}

@Override
public String toString() {
return this.source.toString();
}

}

}
Loading

0 comments on commit 699da7c

Please sign in to comment.