It would be useful to have ability to use @DefaultValue annotation with records and Lombok's auto-generated constructors. I've made small code changes in spring-boot, which provide such an ability.
I've created a sample project which shows desired usage, please see source code and test in this public repo: https://github.com/An1s9n/spring-boot-sample.
In case of any questions, feel free to contact me, preferred (and fastest as well) way is Telegram: https://t.me/An1s9n.
Comment From: pivotal-cla
@An1s9n Please sign the Contributor License Agreement!
Click here to manually synchronize the status of this Pull Request.
See the FAQ for frequently asked questions.
Comment From: pivotal-cla
@An1s9n Thank you for signing the Contributor License Agreement!
Comment From: An1s9n
Hi! How often do you organise team meetings? :)
Comment From: bclozel
@An1s9n we've just discussed this in a team meeting.
While we think it could be an interesting addition for Lombok users, we don't think that adding field+method targets for this annotation is worth it, since it will let most Java users in a position where they can use it in an invalid arrangement.
The record component target is valuable for all Spring Boot users and we'd like to implement this. Would you like to amend your PR with the following?
- only target the record component variant
- add unit tests for this case. Note that since Spring Boot 2.x is built with Java 8, we can't have records in our test source code, so we're executing conditional tests for Java 16+ and writing source classes compiled on the fly (you can see that arrangement in action in 681df90).
Since this requires more work here and this won't address the Lombok use case, feel free to let us know if you don't feel like tackling this issue.
Thanks!
Comment From: An1s9n
@bclozel, hi! Yes, I'd like to amend my PR as you described, because I want to contribute to Spring Boot hah :)) I'll try to do it asap.
Comment From: An1s9n
@bclozel, I've done what you've asked me for :) Let me know if something else is required - I'll fix it
Comment From: snicoll
I am not sure that the annotation processor would handle such use of @DefaultValue out-of-the-box. We should give that a try and create a separate issue if needed.
Comment From: An1s9n
@snicoll, you mean meta-data generation, am I right?
Comment From: An1s9n
@snicoll, luckily it works just perfect without any additional changes! :) I've added a unit test for such a case.
Comment From: wilkinsona
This is targeted at 2.7.x where we compile using Java 8 but ElementType.RECORD_COMPONENT isn't available in Java 8. We need to decide if we're happy to wait till 3.0 to ship this enhancement, or if we want to build and ship a multi-release jar for users on Java 17. https://github.com/spring-projects/spring-boot/issues/25090 is related to the latter option.
Comment From: philwebb
I think we should move this to 3.0 given the amount of work it will take to get our build to produce a multi-release jar.
Comment From: wilkinsona
Thanks very much, @An1s9n.
Comment From: wilkinsona
Since merging these changes, we've learned that they weren't actually necessary. The existing PARAMETER target on @DefaultValue means that @DefaultValue is already propagated from the record component to its corresponding constructor parameters. It's picked up from there by both the binder and the configuration metadata annotation processor. The change made in this issue just means that @DefaultValue will be propagated to the record component itself and available from there using reflection. That isn't necessary for our purposes but it probably doesn't do any harm. I'll flag this for discussion at a forthcoming team meeting so that we can decide if we want to keep the RECORD_COMPONENT target or revert it.
Comment From: wilkinsona
https://github.com/spring-projects/spring-boot/issues/30460 has updated the docs to describe our record support with @DefaultValue and backport some tests to verify that it works as the docs describe.