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

UseNewRequestMatchers should also replace with 5.7 on classpath #398

Merged
merged 6 commits into from
Jul 28, 2023

Conversation

timtebeek
Copy link
Contributor

What's changed?

Adopt JavaTemplate to replace methods.

What's your motivation?

Replacements failed as we looked for a replacement method in the target type, but only 5.7 classes were available, leading to a mismatch and thus no replacement. Fixes #350

Anything in particular you'd like reviewers to focus on?

Called out below.

@timtebeek timtebeek self-assigned this Jul 27, 2023
@timtebeek timtebeek added bug Something isn't working boot-3.0 spring-security labels Jul 27, 2023
@timtebeek
Copy link
Contributor Author

Verified locally with ./gradlew pTML and mod run --skipBuild --recipeGAVs=org.openrewrite.recipe:rewrite-spring:5.1.0-SNAPSHOT --recipeName=org.openrewrite.java.spring.boot3.UpgradeSpringBoot_3_0 --verbose on a sample project this now replaces the methods and everything compiles after.

Comment on lines -97 to -99
return methods.stream()
.filter(m -> m.getName().equals("requestMatchers"))
.filter(m -> m.getParameterTypes().equals(parameterTypes))
Copy link
Contributor Author

@timtebeek timtebeek Jul 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lookup failed, as at recipe runtime only the 5.7 class is available, meaning no replacement method was found. The recipes tests hid that fact, by using classpathResources of 5.8+.

@timtebeek timtebeek requested a review from sambsnyd July 27, 2023 17:22
Copy link
Member

@sambsnyd sambsnyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking a look at this

@timtebeek timtebeek requested a review from joanvr July 28, 2023 07:32
@timtebeek timtebeek merged commit 20e4f75 into main Jul 28, 2023
1 check passed
@timtebeek timtebeek deleted the tim/UseNewRequestMatchers_through_JavaTemplate branch July 28, 2023 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
boot-3.0 bug Something isn't working spring-security
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Spring Security 5.7.8 -> 6.0 method name renaming
4 participants