Fix registration of SecurityExceptionHandling controller advice #802
Conversation
When test classes live in the same package, where auto-configuration classes under test also live, these tests are not pure anymore because they are under the same component scan, as main classes. This component scan could create beans and other entities that are not supposed to be created and will not be created in a real application that will use problem-spring-web-autoconfigure module. This commit moves all test classes into a sibling package, so component scan will not find any components from the main code, and container will contain only beans created by auto-configurations.
When bean-factory method returns `AdviceTrait` interface that is not directly annotated with `@ControllerAdvice`, Spring doesn't register this bean as controller advice, so all `@ExceptionHander`s it provides are also not registered. This happens probably because of ordering between beans instantiation and registration of controller advices in Spring MVC. This commit changes the return type of bean-factory methods to a more concrete type, that is directly annotated with `@ControllerAdvice` annotation. This makes Spring always find the `@ControllerAdvice` annotation and properly register these controller advices and any exception handlers they contain.
Previously the test only checked that ExceptionHandling bean is present in the context. But the presense of the bean doesn't mean the controller advice and associated exception handlers are registered in Spring MVC. Now the test will also check if HTTP request is really handled by one of problem-spring-web exception handlers.
xak2000
requested review from
whiskeysierra,
lukasniemeier-zalando and
AlexanderYastrebov
as code owners
|
Recently I want to use this lib found the same bug. The reason is as you say, when spring find annotation on bean which hasn't been loaded, it uses the type of the bean declare. Change the return type is just the best solution as you mention. So good But why the pull request haven't been accepted yet? A little question, I think |
|
I have a more question. I found that Are they duplicate? or I misunderstanding? |
|
I don't know. Without this fix autoconfiguration just doesn't work in any version after 0.25.2. The workaround it simple, though: create your own @ControllerAdvice
final class SecurityExceptionHandling implements ProblemHandling, SecurityAdviceTrait {
} |
|
Any update? |
|
@MALPI You seem to be active on the code-base. This issue is a bit iffy - could a fix be merged onto main and a do a release? It's not so good to have to create a hack config class in own package: @ControllerAdvice
final class Hack implements ProblemHandling, SecurityAdviceTrait {}in all apps just to work around it. |
Description
Fix bug introduced in PR #413. Default
SecurityExceptionHandlingcontroller advice is not registered in an application.See #707 (comment) for the detailed explanation.
Motivation and Context
Fixes #707, #541.
Types of changes
Checklist: