Mark `GrantedAuthority#getAuthority` as `@Nullable` by therepanic · Pull Request #18014

Mark `GrantedAuthority#getAuthority` as `@Nullable` by therepanic · Pull Request #18014 · spring-projects/spring-security

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

Closes: gh-17999

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

		* precision).
		*/
		String getAuthority();
		@Nullable String getAuthority();

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

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.

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.

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.