Skip to content

Refine SimpleGrantedAuthority#17859

Closed
quaff wants to merge 1 commit intospring-projects:mainfrom
quaff:patch-12
Closed

Refine SimpleGrantedAuthority#17859
quaff wants to merge 1 commit intospring-projects:mainfrom
quaff:patch-12

Conversation

@quaff
Copy link
Copy Markdown
Contributor

@quaff quaff commented Sep 9, 2025 •
edited
Loading

  1. Deprecate field role in favor of authority since role is specific authority.
  2. Add Javadoc to state that role should be prefixed.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 9, 2025
@rwinch
Copy link
Copy Markdown
Member

rwinch commented Sep 9, 2025

Can you elaborate on why you want to make this change? Given that role is internal, I don't see a lot of value making a breaking change.

@rwinch rwinch added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 9, 2025
@rwinch rwinch self-assigned this Sep 9, 2025
@rwinch rwinch added in: core An issue in spring-security-core type: breaks-passivity A change that breaks passivity with the previous release labels Sep 9, 2025
@quaff
Copy link
Copy Markdown
Contributor Author

quaff commented Sep 10, 2025

Can you elaborate on why you want to make this change? Given that role is internal, I don't see a lot of value making a breaking change.

I'm confused by the name while reading the source, I force pushed a commit, it's not a breaking change now.
I'm OK to revert the code changes, but I think it's valuable to improve Javadoc to elaborate why the name must be role not authority, and if the authority is a role it should be prefixed.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 10, 2025
@quaff
Refine SimpleGrantedAuthority
1c5bf07 
1. Deprecate field `role` in favor of `authority` since role is specific authority.
2. Add Javadoc to state that role should be prefixed.

Signed-off-by: Yanming Zhou <zhouyanming@gmail.com>
@rwinch
Copy link
Copy Markdown
Member

rwinch commented Oct 16, 2025

We can change the constructor argument name & add documentation, but the private member variable name is not public and I'd prefer to keep that the same to avoid breaking our users unnecessarily.

I'm closing this PR, but feel free to create a new one as outlined above.

@rwinch rwinch closed this Oct 16, 2025
@rwinch rwinch added status: invalid An issue that we don't feel is valid and removed status: feedback-provided Feedback has been provided labels Oct 16, 2025
@quaff quaff mentioned this pull request Oct 17, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@rwinch rwinch

Labels

in: core An issue in spring-security-core status: invalid An issue that we don't feel is valid type: breaks-passivity A change that breaks passivity with the previous release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@quaff @rwinch @spring-projects-issues