-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Default spring.aot.repositories.enabled to true when AOT is active #3904
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
base: main
Are you sure you want to change the base?
Conversation
…AOT is enabled or not Signed-off-by: Hyunsang Han <[email protected]>
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 submitting a pull request. I left a few comments so you can improve your test code.
|
||
GenericApplicationContext context = new GenericApplicationContext(); | ||
|
||
System.setProperty(AotDetector.AOT_ENABLED, "true"); |
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.
Setting system properties can be achieved via @SetSystemProperty(…)
to avoid leaking system properties state if the test fails.
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.
@mp911de
Thank you for your feedback! I completely understand your point and I agree with you.
To use @SetSystemProperty(…)
and @ClearSystemProperty
, we need to add the junit-pioneer dependency.
Do you think it would be a good idea to include junit-pioneer in this project?
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.
We have it already in Spring Data MonoDB, feel free to include it in JPA as well.
} | ||
|
||
@Test // GH-3899 | ||
void repositoryProcessorShouldNotEnableAotRepositoriesByDefaultWhenAotIsDisabled() { |
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.
You could use @ClearSystemProperty
here
System.setProperty(AotDetector.AOT_ENABLED, "true"); | ||
|
||
try { | ||
JpaRepositoryContributor contributor = new JpaRepositoryConfigExtension.JpaRepositoryRegistrationAotProcessor() |
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.
This snippet could be extracted into a utility method within the test class to not repeat ourselves.
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.
Thank you. I created a private method and refactored these.
System.setProperty(AotDetector.AOT_ENABLED, "true"); | ||
|
||
try { | ||
MockPropertySource propertySource = new MockPropertySource() |
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.
This could be handled by system properties as well as GenericApplicationContext
creates StandardEnvironment
that utilizes system properties and system environment for property sources. That would simplify the test arrangement.
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.
Good point. I changed to use SetSystemProperty
for this!
Signed-off-by: Hyunsang Han <[email protected]>
Signed-off-by: Hyunsang Han <[email protected]>
Signed-off-by: Hyunsang Han <[email protected]>
Resolves: #3899
Currently, to leverage AOT optimized repositories on the JVM with Spring Data JPA, users need to explicitly set both
spring.aot.enabled=true
andspring.aot.repositories.enabled=true
.This change updates the behavior so that if Spring Framework's AOT processing is active (e.g.,
spring.aot.enabled=true
is set, or the application is being prepared for a native image), Spring Data JPA's AOT repository generation (spring.aot.repositories.enabled
) will also be enabled by default.