Skip to content

Support @Value for record injection #28774

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

Closed

Conversation

wine-area
Copy link
Contributor

@wine-area wine-area commented Jul 8, 2022

Add @Value support for record injection by disabling populateBean() for records, since records are immutable.

Addresses #28770

@wine-area wine-area changed the title finish record value inject add support for record value inject Jul 8, 2022
@wine-area wine-area changed the title add support for record value inject add support for record value injection Jul 8, 2022
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 8, 2022
@rstoyanchev rstoyanchev linked an issue Feb 9, 2023 that may be closed by this pull request
@rstoyanchev rstoyanchev added the in: core Issues in core modules (aop, beans, core, context, expression) label Feb 9, 2023
@rstoyanchev rstoyanchev changed the title add support for record value injection Support @Value for record injection Feb 9, 2023
@rstoyanchev
Copy link
Contributor

This aims to fix #28770.

@sbrannen sbrannen added the type: enhancement A general enhancement label Feb 28, 2023
@sbrannen sbrannen self-assigned this Feb 28, 2023
@sbrannen sbrannen removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 28, 2023
@sbrannen sbrannen added this to the 6.0.6 milestone Feb 28, 2023
@@ -1352,6 +1352,10 @@ protected BeanWrapper autowireConstructor(
*/
@SuppressWarnings("deprecation") // for postProcessPropertyValues
protected void populateBean(String beanName, RootBeanDefinition mbd, @Nullable BeanWrapper bw) {
// record has not set methods
if (mbd.getBeanClass().isRecord()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is insufficient, since the bean definition may not have a "bean class".

In any case, I will address this after merging this PR.

So there is no need to modify this PR.

@@ -506,4 +516,8 @@ public TestBean testBean() throws IOException {
}
}


@Component
static record RecordBean(@Value("recordBeanName") String name) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To access the recordBeanName system property, this would need to be changed to @Value("${recordBeanName}") or @Value("#{systemProperties[recordBeanName]}").

I will address this after merging this PR, so there is no need to modify this PR.

void testValueInjectionWithRecord() {
System.setProperty("recordBeanName", "recordBean");
GenericApplicationContext context = new AnnotationConfigApplicationContext(RecordBean.class);
context.refresh();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: a context cannot be refreshed multiple times.

This results in:

java.lang.IllegalStateException: GenericApplicationContext does not support multiple refresh attempts: just call 'refresh' once

I will address this after merging the PR.

@sbrannen sbrannen closed this in 7c9fc57 Feb 28, 2023
sbrannen added a commit that referenced this pull request Feb 28, 2023
@sbrannen
Copy link
Member

This has been merged into main in 7c9fc57 and revised in 00c2c1d.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support @Value on records
4 participants