Removes final modifier from ProblemDetailsExceptionHandler implementations to allow proxying these beans. See #34426 for the issue.
Comment From: philwebb
I wonder if we should add some tests just to stop final sneaking back in the future?
Comment From: vy
@philwebb, sounds like a good idea.
I first thought of adding a @SpringBootTest similar to the one shared in #34426. That requires the spring-boot-starter-aop module, though the classes to be tested are located in spring-boot-autoconfigure. Hence, I settled on ArchUnit tests in s-b-autoconfigure module. Note that while both test classes look identical, they are not due to the scanned packages and the visibility of ProblemDetailsExceptionHandler interfaces.
Comment From: philwebb
Thanks @vy! Rather than a new dependency we might change this to use standard reflection when we merge it.
Comment From: vy
@philwebb, ArchUnit was already used in TaskConfigurationAvoidanceTests, hence was my preference for it. But I can replace it with reflection, if that is what you want.
If you prefer reflection, please also let me know if you want to target ProblemDetailsExceptionHandler interfaces or ResponseEntityExceptionHandler subclasses, since the latter will require some stunt to pull.
Comment From: philwebb
Ahh, I missed that we're already using it. No worries then. We'll take a judgement call when we merge it but don't worry about updating it.
Comment From: wilkinsona
Thanks very much for the PR, @vy. I decided to test the changes by verifying that the exception handlers could be proxied. If you're interested, please see https://github.com/spring-projects/spring-boot/commit/1eb5bbe3ea638e292463a5eca2c3adfc38c4450d for details.
Comment From: vy
@wilkinsona, thanks so much for the prompt processing of the ticket and keeping me in the loop. :bow: I didn't know about AopUtils.isCglibProxy, which makes your tests precisely demonstrative of the bug reported in the first place. :100:
Comment From: sbrannen
@wilkinsona, at a glance it appears that those two assertThat(AopUtils.isCglibProxy(context.getBean(ProblemDetailsExceptionHandler.class))); assertions are missing .isTrue().
Comment From: wilkinsona
Good catch, Sam. Thank you. Corrected in b91f814e427ccd5b99f0f7d1fafaae77ad1fb318.