-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Enable AOT repository generation by default #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
Conversation
…AOT is enabled or not Signed-off-by: Hyunsang Han <gustkd3@gmail.com>
mp911de
left a comment
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.
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.
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
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.
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 <gustkd3@gmail.com>
Signed-off-by: Hyunsang Han <gustkd3@gmail.com>
Signed-off-by: Hyunsang Han <gustkd3@gmail.com>
|
Thank you for your contribution. That's merged and polished now. |
Resolves: #3899
Currently, to leverage AOT optimized repositories on the JVM with Spring Data JPA, users need to explicitly set both
spring.aot.enabled=trueandspring.aot.repositories.enabled=true.This change updates the behavior so that if Spring Framework's AOT processing is active (e.g.,
spring.aot.enabled=trueis 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.