Support @ClientRegistrationId at Class Level #17838
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
|
Sorry for the confusion! I accidentally referenced the wrong issue above. This PR is not related to that issue. |
There was a problem hiding this comment.
Thanks for the pull request @bbudano! I've provided feedback inline.
In addition, please update the documentation at docs/modules/ROOT/pages/whats-new.adoc and docs/modules/ROOT/pages/features/integrations/rest/http-interface.adoc to account for the new feature.
| @@ -37,17 +38,20 @@ public final class ClientRegistrationIdProcessor implements HttpRequestValues.Pr | |||
|
|
|||
| public static ClientRegistrationIdProcessor DEFAULT_INSTANCE = new ClientRegistrationIdProcessor(); | |||
|
|
|||
| private ClientRegistrationIdProcessor() { | |||
There was a problem hiding this comment.
Please don't make changes that are unrelated to the PR
There was a problem hiding this comment.
This change occurred while formatting and wasn't intentional , I will revert it.
| } | ||
|
|
||
| @Test | ||
| void processWhenNoClientRegistrationIdPresentThenNull() { | ||
| HttpRequestValues.Builder builder = HttpRequestValues.builder(); | ||
| Method hasClientRegistrationId = ReflectionUtils.findMethod(RestService.class, "noClientRegistrationId"); |
There was a problem hiding this comment.
While these changes are valuable, please stick to the changes related to this ticket. This helps us to track issues and ensure that, when applicable (e.g. a bug fix that is applicable to older supported branches), we can easily selectively apply changes to other branches.
There was a problem hiding this comment.
I will revert these changes.
| @interface MetaClientRegistrationId { | ||
|
|
||
| } | ||
|
|
||
| @ClientRegistrationId(REGISTRATION_ID) | ||
| interface AnnotatedRestService { |
There was a problem hiding this comment.
| interface AnnotatedRestService { | |
| interface TypeAnnotatedRestService { |
This is more clear since all of the interfaces are annotated.
| @@ -37,17 +38,20 @@ public final class ClientRegistrationIdProcessor implements HttpRequestValues.Pr | |||
|
|
|||
| public static ClientRegistrationIdProcessor DEFAULT_INSTANCE = new ClientRegistrationIdProcessor(); | |||
|
|
|||
There was a problem hiding this comment.
| private SecurityAnnotationScanner<ClientRegistrationId> scanner = SecurityAnnotationScanners.requireUnique(ClientRegistrationId.class); |
Use a SecurityAnnotationScanner<ClientRegistrationId> instead. This will automatically follow the same logic as we do for method security. It ensures that we do not get duplicate annotations which might provide conflicting information and thus use the wrong registration id.
| ClientRegistrationId registeredId = Optional | ||
| .ofNullable(AnnotationUtils.findAnnotation(method, ClientRegistrationId.class)) | ||
| .orElseGet(() -> AnnotationUtils.findAnnotation(method.getDeclaringClass(), ClientRegistrationId.class)); | ||
|
|
There was a problem hiding this comment.
| ClientRegistrationId registeredId = Optional | |
| .ofNullable(AnnotationUtils.findAnnotation(method, ClientRegistrationId.class)) | |
| .orElseGet(() -> AnnotationUtils.findAnnotation(method.getDeclaringClass(), ClientRegistrationId.class)); | |
| ClientRegistrationId registeredId = this.scanner.scan(method, method.getDeclaringClass()); |
| @@ -39,6 +39,8 @@ | |||
| */ | |||
| class ClientRegistrationIdProcessorTests { | |||
There was a problem hiding this comment.
Add tests that verify failure in the event that there are duplicate annotations. This should be handled by the SecurityAnnotationScanners.requireUnique. An example would be AggregateService.toTest() should fail due to duplicate annotations at the same level.
@ClientRegistrationId("a")
interface AService {}
@ClientRegistrationId("b")
interface BService {}
interface AggregateService extends AService, BService {
void toTest();
}
rwinch
added
in: oauth2
|
Thanks for the PR! I pushed an example of using |
|
@rwinch Cool, thanks for the opportunity! |
spring-projects-issues
added
status: feedback-provided
Closes spring-projectsgh-17806 Signed-off-by: Bernard Budano <[email protected]>
Closes spring-projectsgh-17806 Signed-off-by: Bernard Budano <[email protected]>
If
@ClientRegistrationIdis not found on a method, the declaring class will be checked instead.Closes gh-17806