Skip to content

Use of @Value in compact constructor of a record should not register method injection#31433

Closed
martin-lukas wants to merge 1 commit intospring-projects:mainfrom
martin-lukas:fix-log-when-value-on-record-property
Closed

Use of @Value in compact constructor of a record should not register method injection#31433
martin-lukas wants to merge 1 commit intospring-projects:mainfrom
martin-lukas:fix-log-when-value-on-record-property

Conversation

@martin-lukas
Copy link
Contributor

@martin-lukas martin-lukas commented Oct 15, 2023

When I try to use @Value annotation a record property, I get a warning in the logs that autowiring should only be used on methods with parameters. What that seems to imply is that attaching @Value on the property is actually attaching it on the generated method property(), which has no parameters.

Example:

@Component
public record MyRecord(@Value("${myProp}") String prop) {}

Output:

02:21:15.043 [main] INFO  o.s.b.f.a.AutowiredAnnotationBeanPostProcessor - Autowired annotation should only be used on methods with parameters: public java.lang.String com.example.MyRecord.prop()

My fix is to just check first, if the bean is a record or not.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 15, 2023
@snicoll
Copy link
Member

snicoll commented Oct 16, 2023

Thanks for the PR but are you sure that the code you've shared above currently triggers the warning? Does it work at all?

It fails for me as follows:

org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'myComponent': Injection of autowired dependencies failed; nested exception is java.lang.IllegalAccessException: Can not set final java.lang.String field com.example.demo.MyComponent.prop to java.lang.String
	at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor.postProcessProperties(AutowiredAnnotationBeanPostProcessor.java:405) ~[spring-beans-5.3.29.jar:5.3.29]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.populateBean(AbstractAutowireCapableBeanFactory.java:1431) ~[spring-beans-5.3.29.jar:5.3.29]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:619) ~[spring-beans-5.3.29.jar:5.3.29]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:542) ~[spring-beans-5.3.29.jar:5.3.29]
	at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:335) ~[spring-beans-5.3.29.jar:5.3.29]
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:234) ~[spring-beans-5.3.29.jar:5.3.29]
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:333) ~[spring-beans-5.3.29.jar:5.3.29]
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:208) ~[spring-beans-5.3.29.jar:5.3.29]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:955) ~[spring-beans-5.3.29.jar:5.3.29]
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:921) ~[spring-context-5.3.29.jar:5.3.29]
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:583) ~[spring-context-5.3.29.jar:5.3.29]
	at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.refresh(ServletWebServerApplicationContext.java:147) ~[spring-boot-2.7.14.jar:2.7.14]
	at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:731) ~[spring-boot-2.7.14.jar:2.7.14]
	at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:408) ~[spring-boot-2.7.14.jar:2.7.14]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:307) ~[spring-boot-2.7.14.jar:2.7.14]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1303) ~[spring-boot-2.7.14.jar:2.7.14]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1292) ~[spring-boot-2.7.14.jar:2.7.14]
	at com.example.demo.DemoApplication.main(DemoApplication.java:12) ~[classes/:na]
Caused by: java.lang.IllegalAccessException: Can not set final java.lang.String field com.example.demo.MyComponent.prop to java.lang.String
	at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.throwFinalFieldIllegalAccessException(UnsafeFieldAccessorImpl.java:76) ~[na:na]
	at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.throwFinalFieldIllegalAccessException(UnsafeFieldAccessorImpl.java:80) ~[na:na]
	at java.base/jdk.internal.reflect.UnsafeQualifiedObjectFieldAccessorImpl.set(UnsafeQualifiedObjectFieldAccessorImpl.java:79) ~[na:na]
	at java.base/java.lang.reflect.Field.set(Field.java:799) ~[na:na]
	at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor$AutowiredFieldElement.inject(AutowiredAnnotationBeanPostProcessor.java:646) ~[spring-beans-5.3.29.jar:5.3.29]
	at org.springframework.beans.factory.annotation.InjectionMetadata.inject(InjectionMetadata.java:119) ~[spring-beans-5.3.29.jar:5.3.29]
	at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor.postProcessProperties(AutowiredAnnotationBeanPostProcessor.java:399) ~[spring-beans-5.3.29.jar:5.3.29]
	... 17 common frames omitted

Turning a record into a bean is a bit odd. In any case, please clarify your assumptions as we can't really process this until we understand it.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Oct 16, 2023
@quaff
Copy link
Contributor

quaff commented Oct 16, 2023

@snicoll I confirm that log is triggered.

 o.s.b.f.a.AutowiredAnnotationBeanPostProcessor INFO Autowired annotation should only be used on methods with parameters: public java.lang.String io.example.showcase.MyRecord.prop()

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Oct 16, 2023
@snicoll
Copy link
Member

snicoll commented Oct 16, 2023

Thanks. "logging a warning" broke my brain, I was looking for a warning, not an info message. The bits on testing and use of record remains though.

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Oct 16, 2023
@martin-lukas
Copy link
Contributor Author

martin-lukas commented Oct 16, 2023 •
edited
Loading

@snicoll Sorry for the misunderstanding, it is an INFO log, but communicating a sort of warning, didn't know how to call it.

It works well for me with the code I provided (+ basic Spring Boot project, latest, JDK 21 or 17).

As for record being odd when used as a bean. I was just experimenting with records + Spring. Using records as a bean seems mostly equivalent to e.g.:

@Component
public class MyClass {
  private String prop;
  public MyClass(@Value("${myProp}") String prop) {
    this.prop = prop;
  }
  // getter
}

But you're kinda right. Now that I think about the use cases, it might not be that needed. If I have some service that needs a property value, and I use a record for it, I'd get an unwanted property accessor automatically, which is not great. I was thinking more like loading properties into a "configuration" carrying bean, but that's also possible with e.g.:

@ConfigurationProperties(prefix = "config")
public record Config(String first, String second) {}

which works well.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Oct 16, 2023
@snicoll snicoll added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Oct 23, 2023
@snicoll snicoll self-assigned this Oct 23, 2023
@snicoll snicoll added this to the 6.0.14 milestone Oct 23, 2023
@snicoll
Copy link
Member

snicoll commented Oct 23, 2023

I was thinking more like loading properties into a "configuration" carrying bean,

Yeah, sorry. You're totally right. The @Value with a record makes sense indeed.

@snicoll snicoll changed the title Don't log warning when @Value is on record property Use of @Value in compact constructor of a record should not register method injection Oct 23, 2023
snicoll pushed a commit that referenced this pull request Oct 23, 2023
@snicoll
Ignore @value on record property
70cb96c 
See gh-31433
snicoll added a commit that referenced this pull request Oct 23, 2023
@snicoll
Polish "Ignore @value on record property"
f3dce4b 
See gh-31433
@snicoll snicoll closed this in bb446a3 Oct 23, 2023
@snicoll
Copy link
Member

snicoll commented Oct 23, 2023

@martin-lukas thanks very much for making your first contribution to Spring Framework.

@jhoeller jhoeller added the in: core Issues in core modules (aop, beans, core, context, expression) label Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@snicoll snicoll

Labels

in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug

Projects

None yet

Milestone

6.0.14

Development

Successfully merging this pull request may close these issues.

5 participants

@martin-lukas @snicoll @quaff @jhoeller @spring-projects-issues