Skip to content

Mark GrantedAuthority#getAuthority as @Nullable#18014

Merged
jzheaux merged 2 commits intospring-projects:mainfrom
therepanic:gh-17999
Oct 20, 2025
Merged

Mark GrantedAuthority#getAuthority as @Nullable#18014
jzheaux merged 2 commits intospring-projects:mainfrom
therepanic:gh-17999

Conversation

@therepanic
Copy link
Copy Markdown
Contributor

@therepanic therepanic commented Oct 6, 2025

Since the GrantedAuthority#getAuthority contract itself implies that it can return null, we should mark it as @Nullable.

Closes: gh-17999

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 6, 2025
@therepanic
Copy link
Copy Markdown
Contributor Author

therepanic commented Oct 6, 2025 •
edited
Loading

Currently, the build crashes in the docs module, and it's not entirely clear why. Can you explain please why the build crashes in the following class test?

ObtainingMoreAuthorizationTests. profileWhenMissingAuthorityConfigurationThenRedirectsToAuthorizationServer()
ObtainingMoreAuthorizationTests. profileWhenMissingAuthorityConfigurationThenRedirectsToAuthorizationServer()

Note: Now it is fixed

@therepanic therepanic force-pushed the gh-17999 branch 3 times, most recently from 54f53fc to e16ab55 Compare October 11, 2025 14:00
ronodhirSoumik
ronodhirSoumik reviewed Oct 11, 2025
View reviewed changes
core/src/main/java/org/springframework/security/core/GrantedAuthority.java
* precision).
*/
String getAuthority();
@Nullable String getAuthority();
Copy link
Copy Markdown
Contributor

@ronodhirSoumik ronodhirSoumik Oct 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Question :: though this implementation supports null case, but it is supposed to be avoided (as per the documentation -> returning null should be avoided unless actually required. ) So dont we losing the feature/control with the overall changes?
@therepanic @rwinch

Copy link
Copy Markdown
Contributor

@jzheaux jzheaux Oct 17, 2025

Choose a reason for hiding this comment

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

@ronodhirSoumik, good question. The documentation also says that null "should be returned" in cases where it cannot be expressed with sufficient precision. In other words, returning null is recommended when needed.

jzheaux
jzheaux requested changes Oct 17, 2025
View reviewed changes
Copy link
Copy Markdown
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, @therepanic! I've left some feedback inline.

@therepanic
Copy link
Copy Markdown
Contributor Author

therepanic commented Oct 18, 2025

Thanks for the review, @jzheaux.

I agree with everything. It was a bad idea to mark authority as @Nullable when there was a simpler way to do it. I implemented this change as a separate commit to make it easier to read the changes.

I don't think the build crashes because of the changes in the PR.

@therepanic @jzheaux
Mark GrantedAuthority#getAuthority as @Nullable
05830d5 
Closes: spring-projectsgh-17999

Signed-off-by: Andrey Litvitski <andrey1010102008@gmail.com>
@jzheaux jzheaux self-assigned this Oct 20, 2025
@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 Oct 20, 2025
@jzheaux jzheaux added this to the 7.0.0 milestone Oct 20, 2025
@jzheaux jzheaux merged commit 0a2f55d into spring-projects:main Oct 20, 2025
6 checks passed
@jzheaux
Copy link
Copy Markdown
Contributor

jzheaux commented Oct 20, 2025

Thanks, @therepanic! This is now merged into main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jzheaux jzheaux jzheaux approved these changes

+1 more reviewer

@ronodhirSoumik ronodhirSoumik ronodhirSoumik left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@jzheaux jzheaux

Labels

in: core An issue in spring-security-core type: enhancement A general enhancement

Projects

None yet

Milestone

7.0.0

Development

Successfully merging this pull request may close these issues.

GrantedAuthority.getAuthority() should be marked @Nullable

4 participants

@therepanic @jzheaux @ronodhirSoumik @spring-projects-issues