Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log warning if multiple @PostMapping, @GetMapping, etc. annotations are declared #31962

Closed
calliduslynx opened this issue Jan 6, 2024 · 3 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: documentation A documentation task type: enhancement A general enhancement
Milestone

Comments

@calliduslynx
Copy link

calliduslynx commented Jan 6, 2024

If more than one of the @(Method)Mapping annotations are used, only one seems to be recognized.

Reproduction:

Put @GetMapping and @PostMapping together on a method of a controller:

@RestController
class AnyController {
  @GetMapping("x")
  @PostMapping("x")
  fun x() = println("x")
}

If you now test the endpoint you see that GET /x is mapped, but POST /x is not (405 - Method Not Allowed).

Expectation:

The expectation is that both annotations are processed and both endpoints would be available.

At least there should be a warning that one is ignored.

Tested with:

Spring Boot 3.0.13 on Java 17.0.7

Workaround:

Use @RequestMapping(method = { ... }).

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 6, 2024
@bclozel bclozel transferred this issue from spring-projects/spring-boot Jan 6, 2024
@calliduslynx
Copy link
Author

@cdmatta Of course you're right from technical point of view. And the way how these things are "implemented" as a composed annotation is extremely elegant.

But from a Spring Boot (novice) users view I see these annotations in hundreds of tutorials and articles and if I use them combined they don't cause any problems (compile error, start error, log). One of them just don't work. And even for me (who uses Spring Boot every day for years) it was not clear.

And the comment you marked is not very obvious. I read it and did not know that @RequestMapping is not repeatable. And even with this knowledge you have to think twice to realize that two @(Method)Mapping annotations will not work.

@sdeleuze sdeleuze changed the title @PostMethod, @GetMethod, etc not recognized if more than one @PostMapping, @GetMapping, etc not recognized if more than one Jan 7, 2024
@sdeleuze sdeleuze added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jan 7, 2024
@sbrannen sbrannen changed the title @PostMapping, @GetMapping, etc not recognized if more than one Only first @PostMapping, @GetMapping, etc is honored Jan 8, 2024
@sbrannen
Copy link
Member

sbrannen commented Jan 8, 2024

If more than one of the @(Method)Mapping annotations are used only one seems to be recognized.

That's correct. Spring looks for the first @RequestMapping annotation present and uses that one. If multiple @RequestMapping annotations are meta-present (i.e., used as meta-annotations like on @GetMapping, etc.), the first one is used, and the rest are ignored.

In addition, please note that a locally declared @RequestMapping annotation will always override/hide @GetMapping, @PostMapping, etc.

The expectation is that both annotations are processed and both endpoints would be available.

I don't think that would be a good idea, since each annotation could provide conflicting values for other annotation attributes in @RequestMapping.

The only way to map multiple explicit methods is via @RequestMapping -- for example, @RequestMapping(method = {RequestMethod.GET, RequestMethod.POST}).

At least there should be a warning that one is ignored.

We can definitely mention in the documentation that only a single @RequestMapping, @GetMapping, etc. is supported.

As for logging a warning, I'm not sure if we want to go that far; however, we'll discuss it within the team.

@sbrannen sbrannen self-assigned this Jan 16, 2024
@sbrannen sbrannen added type: documentation A documentation task and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 16, 2024
@sbrannen sbrannen changed the title Only first @PostMapping, @GetMapping, etc is honored Warn if multiple @PostMapping, @GetMapping, etc. annotations are declared Jan 16, 2024
@sbrannen
Copy link
Member

sbrannen commented Jan 16, 2024

Team Decision:

For Spring Framework 6.1.x, we have decided to detect multiple @RequestMapping declarations on a handler method and log a warning stating that only the first such annotation will be used. We may also update the documentation for the affected annotations to point out the current limitation.

For Spring Framework 6.2, we would like to investigate the feasibility of supporting multiple @RequestMapping annotations on a single controller method. See #32043.

@sbrannen sbrannen added the type: enhancement A general enhancement label Jan 16, 2024
@sbrannen sbrannen changed the title Warn if multiple @PostMapping, @GetMapping, etc. annotations are declared Log warning if multiple @PostMapping, @GetMapping, etc. annotations are declared Jan 16, 2024
@sbrannen sbrannen added this to the 6.1.4 milestone Jan 16, 2024
sbrannen added a commit that referenced this issue Jan 18, 2024
sbrannen added a commit that referenced this issue Jan 19, 2024
…& WebFlux

This commit updates the RequestMappingHandlerMapping implementations in
Spring MVC and Spring WebFlux so that mixed @⁠RequestMapping and
@⁠HttpExchange declarations on the same element are rejected.

Note, however, that a @⁠Controller class which implements an interface
annotated with @⁠HttpExchange annotations can still inherit the
@⁠HttpExchange declarations from the interface or optionally override
them locally with @⁠HttpExchange or @⁠RequestMapping annotations.

See gh-31962
See gh-32049
Closes gh-32065
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: documentation A documentation task type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants