Replace deprecated #check calls with #authorize #16965
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
| @@ -67,20 +67,33 @@ public ObservationAuthorizationManager(ObservationRegistry registry, Authorizati | |||
| @Deprecated | |||
| @Override | |||
| public AuthorizationDecision check(Supplier<Authentication> authentication, T object) { | |||
There was a problem hiding this comment.
I think it's better to leave the check method as is. There's no point in calling the authorize method from a deprecated method.
There was a problem hiding this comment.
I think if we are moving away from check method it's reasonable to stop calling user-defined check implementations in our code, instead it's good to indicate to the user that they should fully migrate to using authorize method if they have mix of both, the same approach is used in AuthorizationManagers#anyOf for example.
There was a problem hiding this comment.
@jzheaux what do you think?
There was a problem hiding this comment.
Good points from both of you.
Given that this is work that can be done in advance of the next major, I think that it is worthwhile. In this way, the only thing that remains to be done when the time comes is for check to be removed.
|
Hey @evgeniycheban , It would be nice if the |
Yeah, good catch, I must have missed that one. |
jzheaux
added
in: core
| @@ -73,7 +73,7 @@ void setup() { | |||
| @Test | |||
| void verifyWhenDefaultsThenObserves() { | |||
| given(this.handler.supportsContext(any())).willReturn(true); | |||
| given(this.authorizationManager.check(any(), any())).willReturn(this.grant); | |||
There was a problem hiding this comment.
Let's leave unit tests as-is so that we know we are still backward compatible.
There was a problem hiding this comment.
I'll change it to use willCallRealMethod instead.
| @@ -67,20 +67,33 @@ public ObservationAuthorizationManager(ObservationRegistry registry, Authorizati | |||
| @Deprecated | |||
| @Override | |||
| public AuthorizationDecision check(Supplier<Authentication> authentication, T object) { | |||
There was a problem hiding this comment.
Good points from both of you.
Given that this is work that can be done in advance of the next major, I think that it is worthwhile. In this way, the only thing that remains to be done when the time comes is for check to be removed.
Closes spring-projectsgh-16936 Signed-off-by: Evgeniy Cheban <[email protected]>
|
Hi, @jzheaux I've updated the PR. |
|
It would be nice to make the same improvement for |
Closes gh-16936