-
Notifications
You must be signed in to change notification settings - Fork 6.2k
Improve @AuthenticationPrincipal meta-annotations #15344
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
Conversation
jzheaux
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for contributing this, @kse-music! I've left some initial feedback inline.
.../org/springframework/security/messaging/context/AuthenticationPrincipalArgumentResolver.java
Outdated
Show resolved
Hide resolved
.../org/springframework/security/messaging/context/AuthenticationPrincipalArgumentResolver.java
Outdated
Show resolved
Hide resolved
.../org/springframework/security/messaging/context/AuthenticationPrincipalArgumentResolver.java
Outdated
Show resolved
Hide resolved
0d0f8ec to
7bb8a85
Compare
|
Thanks,@jzheaux ,all feedback is done |
7def1ce to
6f8cf72
Compare
|
Thanks, @kse-music! Given that this needs to be placed in the other |
747299c to
e5f2230
Compare
All non-deprecated instances of AuthenticationPrincipalArgumentResolver in the project has done. |
c627031 to
25edd68
Compare
|
Thanks, @kse-music, this is taking shape very nicely. I believe our last step is to make this simple to configure by picking up the I've added documentation so you can see the expectation that someone should be able to do: @Bean
public AnnotationTemplateExpressionDefaults templateExpressionDefaults() {
return new AnnotationTemplateExpressionDefaults();
}And now they will be able to use placeholders in any Are you able to add this to the following |
|
@jzheaux ,I tried to add it |
69aced4 to
7cf93db
Compare
|
@jzheaux I have added the code, please help to review it.Thank you |
…mplateExpressionDefaults bean is Present
Since there is nothing specific to configuring pre/post annotations, there is no need for the extra class. If a need like this does arise in the future, either AnnotationTemplateExpressionDefaults can be sub- classed, or it can have introduced a Map field holding custom properties. Issue spring-projectsgh-15286
|
Thanks for all your help with this PR, @kse-music! I added a couple of commits to add more testing and also to deprecate |
|
If you are interested, it would be nice to do the same with |
Closes gh-15286