-
Notifications
You must be signed in to change notification settings - Fork 1.3k
AOT contributions will be registered for JbcOAuth2AuthorizationService subclasses #1778
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @billkoch. Please see review comments.
Also, can you please rebase off the 1.2.x
branch and I'll take care of forward porting it to main
.
@@ -43,8 +43,10 @@ | |||
import org.springframework.security.oauth2.core.oidc.user.OidcUserAuthority; | |||
import org.springframework.security.oauth2.core.user.DefaultOAuth2User; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update copyright year
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦♂️ Fixed
@@ -0,0 +1,108 @@ | |||
/* | |||
* Copyright 2020-2021 the original author or authors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update copyright year
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
class OAuth2AuthorizationServerBeanRegistrationAotProcessorTests { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add javadoc similar to other tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
|
||
class OAuth2AuthorizationServerBeanRegistrationAotProcessorTests { | ||
|
||
OAuth2AuthorizationServerBeanRegistrationAotProcessor processor; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make the members private
@ParameterizedTest | ||
@ValueSource(classes = { JdbcOAuth2AuthorizationService.class, CustomJdbcOAuth2AuthorizationService.class, | ||
JdbcRegisteredClientRepository.class, CustomJdbcRegisteredClientRepository.class }) | ||
void whenABeanRegistrationAotContributionShouldBeReturned(Class<?> beanClass) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to processAheadOfTimeWhenBeanTypeJdbcBasedImplThenReturnContribution
@ParameterizedTest | ||
@ValueSource(classes = { InMemoryOAuth2AuthorizationService.class, InMemoryRegisteredClientRepository.class, | ||
Object.class }) | ||
void whenABeanRegistrationAotContributionShouldNotBeReturned(Class<?> beanClass) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to processAheadOfTimeWhenBeanTypeNotJdbcBasedImplThenDoesNotReturnContribution
} | ||
|
||
@Test | ||
void aotContributionOnlyNeedsSpecifiedOnce() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to processAheadOfTimeWhenMultipleBeanTypeJdbcBasedImplThenReturnContributionOnce
…e subclasses Prior to this commit, String-based class name comparisons were used for determining if a bean was of type JdbcOAuth2AuthorizationService or JdbcRegisteredClientRepository. Now JdbcOAuth2AuthorizationService.class.isAssignableFrom(...) and JdbcRegisteredClientRepository.class.isAssignableFrom(...) is used so that any subclasses are detected and the necessary AOT hints are contributed. closes spring-projectsgh-1737
@jgrandja Thanks for the review! I believe I've addressed all the comments. Please let me know if there's anything else you'd like me to change. |
Thanks for the updates @billkoch. This is now merged! |
AOT contributions will be registered for JbcOAuth2AuthorizationService subclasses
Prior to this commit, String-based class name comparisons were used for determining if a bean was of type JdbcOAuth2AuthorizationService or JdbcRegisteredClientRepository.
Now JdbcOAuth2AuthorizationService.class.isAssignableFrom(...) and JdbcRegisteredClientRepository.class.isAssignableFrom(...) is used so that any subclasses are detected and the necessary AOT hints are contributed.
closes gh-1737