Skip to content

Add Kotlin support to PreFilter and PostFilter annotations #15095

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

Merged
merged 1 commit into from
May 31, 2024
Merged

Add Kotlin support to PreFilter and PostFilter annotations #15095

merged 1 commit into from
May 31, 2024

Conversation

call-me-baki
Copy link
Contributor

@call-me-baki call-me-baki commented May 18, 2024

Closes the following issue:

Summary
Adds support for Kotlin in DefaultMethodSecurityExpressionHandler.

It additionally updates the documentation (both javadoc and asciidoc) to reflect the current implementation details and usage.

Implementation details
Attempts to mutate the state of either the Collection or the Map:

  • if an UnsupportedOperationException is not thrown, returns the instance with its state mutated.
  • if an UnsupportedOperationException is thrown, returns a new instance with the filtered state.

Tests
The original Java test DefaultMethodSecurityExpressionHandlerTests has been left unchanged and acts as a regression test.

A new test DefaultMethodSecurityExpressionHandlerKotlinTests has been added to assert over the correct behavior when using Kotlin instead of Java.

Copy link
Contributor

@jzheaux jzheaux left a comment

Choose a reason for hiding this comment

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

Thanks, @call-me-baki! I've left some feedback inline.

@call-me-baki
Copy link
Contributor Author

Thank you @jzheaux for the feedback! 🙏🏼

I've addressed the requested changes, would you please have a look again?

Regarding adding myself to JavaDoc's authors, I'm completely fine if we revert that commit should this change not warrant it.

@jzheaux jzheaux self-assigned this May 21, 2024
@jzheaux jzheaux added in: core An issue in spring-security-core type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels May 21, 2024
@jzheaux jzheaux added this to the 6.4.0-M1 milestone May 21, 2024
@jzheaux
Copy link
Contributor

jzheaux commented May 23, 2024

Thanks for the updates, @call-me-baki! In preparation for merging, will you please do the following:

  1. Run ./gradlew format && ./gradlew :spring-security-core:check and make any changes the output requires
  2. Squash your commits into one
  3. Ensure your commit message is brief and also contains Closes gh-15093, for example:
Add Kotlin support for Pre-PostFilter Annotations

Closes gh-15093

@call-me-baki call-me-baki changed the title #15093 Add Kotlin support in DefaultMethodSecurityExpressionHandler Add Kotlin support to PreFilter and PostFilter annotations May 23, 2024
@call-me-baki
Copy link
Contributor Author

call-me-baki commented May 23, 2024

Hey @jzheaux! Thanks for your support and guidance.

I ran ./gradlew format && ./gradlew :spring-security-core:check, there wasn't anything actionable.
The commits have been squashed and I hope that the commit message is fitting.

Looking forward to contributing to Spring Security in the future!

@jzheaux jzheaux merged commit 63f4816 into spring-projects:main May 31, 2024
4 checks passed
@jzheaux
Copy link
Contributor

jzheaux commented May 31, 2024

Thanks again, @call-me-baki! This is now merged into main and will go out in the next milestone release. I also added a slight polish in aa9bf83 to clarify variable names and align logging levels.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core An issue in spring-security-core type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants