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

MigrateHandlerInterceptor recipe migrates too much #622

Conversation

jevanlingen
Copy link
Contributor

@jevanlingen jevanlingen commented Nov 8, 2024

What's changed?

Improve/Bugfix the MigrateHandlerInterceptorrecipe to only migrate classes that directly extends the HandlerInterceptorAdapter.

What's your motivation?

Migration is targeted broadly, so it can migrate super calls when it is not intended to. This can lead to bugs, see

for example.

Any additional context

The migration does now convert super.preHandle(..) to HandlerInterceptor.super.preHandle(..). According to mr unconditional, you should migrate to return true. See this SO post for more information.

Also this blog seems to suggest that this is the right move (as in, when you move from SB 2 to 3, you likely have a return true kind of implementation).

There are two other methods avaliable in the HandlerInterceptorAdapter/HandlerInterceptor classes: postHandle and afterCompletion. Both of them have empty default implementation, So maybe the recipe should be doing something else:

  • preHandle: super.preHandle(..) -> return true
  • postHandle: super.postHandle(..) -> <remove super call>
  • afterCompletion: super.afterCompletion(..) -> <remove super call>

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

@jevanlingen jevanlingen requested a review from timtebeek November 8, 2024 10:42
@jevanlingen jevanlingen self-assigned this Nov 8, 2024
@jevanlingen jevanlingen added bug Something isn't working recipe Recipe requested labels Nov 8, 2024
@jevanlingen jevanlingen changed the title Checkout 620 custom handlerinterceptor with spring boot 3 migration does not compile MigrateHandlerInterceptor recipe migrates too much Nov 8, 2024
@timtebeek timtebeek self-requested a review November 12, 2024 20:27
@timtebeek timtebeek merged commit 2cf117d into main Nov 12, 2024
2 checks passed
@timtebeek timtebeek deleted the checkout-620-custom-handlerinterceptor-with-spring-boot-3-migration-does-not-compile branch November 12, 2024 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working recipe Recipe requested
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Custom HandlerInterceptor with Spring Boot 3 migration does not compile
2 participants