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.

Sign up for GitHub

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

Jump to bottom

HttpSecurity - replace apply with with #455

Merged
merged 2 commits into from
Dec 5, 2023
Merged

HttpSecurity - replace apply with with #455

merged 2 commits into from
Dec 5, 2023

Conversation

nmck257
Copy link
Collaborator

@nmck257 nmck257 commented Dec 5, 2023

Fixes #449

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

  • Added another SourceSet for Spring Security 6.2
  • The overloaded constructor options on ConvertToSecurityDsl are starting to feel a little clunky, but I think that with the addition of this recipe we've pretty much covered the full scope of the Lambda DSL migration, so I don't anticipate we'd need to extend this visitor much further
  • (not directly related to the PR) I wonder if we could create some generic / fluent API for slicing and dicing a method invocation chain... I feel like I've seen that problem solved several times in individual recipes/visitors, and it tends to be non-trivial, but with the right abstraction, it might be reusable?...

Anyone you would like to review specifically?

Others who've been in this file a bunch: @shanman190 @BoykoAlex

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

@timtebeek timtebeek requested a review from shanman190 December 5, 2023 19:39
shanman190
shanman190 approved these changes Dec 5, 2023
View reviewed changes
Copy link
Contributor

@shanman190 shanman190 left a comment

Choose a reason for hiding this comment

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

LGTM!

@shanman190 shanman190 merged commit 3e778c9 into main Dec 5, 2023
1 check passed
@shanman190 shanman190 deleted the feature/449-apply-with branch December 5, 2023 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shanman190 shanman190 shanman190 approved these changes

Assignees

@nmck257 nmck257

Labels
boot-3.2 recipe Recipe requested spring-security
Projects
OpenRewrite
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Spring Security 6.2 - HttpSecurity apply to with
3 participants
@nmck257 @shanman190 @timtebeek