spring-data-rest
spring-data-rest copied to clipboard
Feature regression : Spring Data REST controller must not use @RequestMapping on class level as this would cause double registration with Spring MVC
On latest version @RequestMapping isn't more allowed at class level due to this change =>
https://github.com/spring-projects/spring-data-rest/commit/46dc6e03fcd13cadfafeab22ba813c12a8dc9688
@RequestMapping is now only allowed on method level but that isn't an valid option for method on an abstract controller.
@RepositoryRestController
@RequestMapping("/api/widgets")
public class WidgetController extends BaseControler {
// ...
}
public class BaseControler {
@GetMapping("/get")
public ResponseEntity<String> get() {
return ResponseEntity.ok().body(....);
}
}
Proposition of fix =>
Add a field "path/value" on the annotation @RepositoryRestController to be able to set a root path at class level.
@RepositoryRestController("/api/widgets")
@RequiredArgsConstructor
public class WidgetController extends BaseControler {
// ...
}
public class BaseControler {
@GetMapping("/get")
public ResponseEntity<String> get() {
return ResponseEntity.ok().body(....);
}
}
Stackoverflow discussion => https://stackoverflow.com/questions/69825704/spring-data-rest-controller-must-not-use-requestmapping-on-class-level-as-this
I was hit also by this change and moving request mapping into class level does behave differently than mapping on class level. There is perhaps involved some miss-configuration (or miss-use) of paths. My change on simple (showcase for this problematic update) project, which caused uri routing for my case to broke: https://github.com/luvarqpp/poc-springRequestMapping/commit/5606c6e0faf1c19be59f567c85fa2d1f72d51c7d
This issue is also blocking us from upgrading to 2.5.6 or later. We do use abstract controllers extensively and we know of no other work-around. Any chance we could get some movement on the associated pull request?
We unfortunately cannot move back to the original arrangement as those declarations, in combination with how Spring MVC handles controllers in the 5.x generation, is effectively the reason for a CVE.
I'll review what it'd mean to support type level mappings on our very own @RepositoryRestController
but the workaround for the 2.5.x branch is to move to path definitions on the individual method level. Reuse of common prefixes can be achieved through defining constants for the paths in use.
We unfortunately cannot move back to the original arrangement as those declarations, in combination with how Spring MVC handles controllers in the 5.x generation, is effectively the reason for a CVE.
I'll review what it'd mean to support type level mappings on our very own
@RepositoryRestController
but the workaround for the 2.5.x branch is to move to path definitions on the individual method level. Reuse of common prefixes can be achieved through defining constants for the paths in use.
We're absolutely fine with not going back to the old solution, but we'd like to avoid having to just duplicating constant values from our abstract controller classes onto a load of methods (also increasing the risk of future additions ignoring or not adhering to the current base paths). That really feels like moving backwards.
We'd thus also gladly move to a new approach according to MR #2088. If we can help getting this forward in any way let us know.
The fix in PR #2088 would work for us as we are running into this issue as well, and we have a tone of Abstract Controllers with a lot of methods. Duplicating all of the @RequestMappings and overriding all of the methods would be hundreds of developer hours to complete, and this seems like a simple solution.
Should fixed from Spring data version 4.0.0-M5
I see, the solution is actually quite nice going forward @odrotbohm 👍
The only issue we have is that now we are stuck on Spring-Boot 2.5.5 - unless we:
- migrate all regular controllers with class-level
@RequestMapping
to use static base paths on all controller methods instead - refactor all our abstract controllers (with their implementations) to some sort of template pattern, so the abstract controllers don't define the mappings anymore, but just provide the common logic in some protected methods and maybe some constants for the common paths to be used in concrete child classes - and then duplicate all the actual controller methods & annotations on all those concrete child classes
- this is just so we can upgrade to latest 2.7 branch - which is a pre-condition (or at least highly recommended) for the upgrade to 3.0
- upgrade to 3.0 will in turn offer the solution we need to avoid migrating all our abstract controllers (from #2157), and allow us to migrate everything back 🤔
Which means we're kind of deadlocked or at least running in circles 😁 Don't get me wrong - this can be dealt with, and I understand why it was necessary. It just still leaves me a bit clueless about how to proceed 😕
Would it maybe be possible to backport that change to the 2.7 branch?
It turns out this was already released with 3.7.2 and I just forgot to close the ticket, i.e. this should already be available in the most recent Spring Boot 2.7.