Skip to content
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

java.time.Duration properties can't be injected through @Value #13237

Closed
detouched opened this issue May 23, 2018 · 6 comments
Closed

java.time.Duration properties can't be injected through @Value #13237

detouched opened this issue May 23, 2018 · 6 comments
Labels
status: duplicate A duplicate of another issue

Comments

@detouched
Copy link

Introduced in #11078 injection of java.time.Duration properties is incomplete. It works for properties injected into a structured object through @ConfigurationProperties, but it does not work for properties injected through @Value.

E.g. something like this:

    @Bean
    public String demoBean(@Value("property") Duration property) {
        return "I was given a property through @Value: " + property;
    }

fails to start up with an error:

org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'demoBean' defined in com.github.detouched.DemoValueApp: Unsatisfied dependency expressed through method 'demoBean' parameter 0; nested exception is org.springframework.beans.ConversionNotSupportedException: Failed to convert value of type 'java.lang.String' to required type 'java.time.Duration'; nested exception is java.lang.IllegalStateException: Cannot convert value of type 'java.lang.String' to required type 'java.time.Duration': no matching editors or conversion strategy found
	at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:729) ~[spring-beans-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.beans.factory.support.ConstructorResolver.instantiateUsingFactoryMethod(ConstructorResolver.java:470) ~[spring-beans-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.instantiateUsingFactoryMethod(AbstractAutowireCapableBeanFactory.java:1254) ~[spring-beans-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1103) ~[spring-beans-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:541) ~[spring-beans-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:501) ~[spring-beans-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:317) ~[spring-beans-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:228) ~[spring-beans-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:315) ~[spring-beans-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:199) ~[spring-beans-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:760) ~[spring-beans-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:869) ~[spring-context-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:550) ~[spring-context-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.refresh(ServletWebServerApplicationContext.java:140) ~[spring-boot-2.0.2.RELEASE.jar:2.0.2.RELEASE]
	at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:759) ~[spring-boot-2.0.2.RELEASE.jar:2.0.2.RELEASE]
	at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:395) ~[spring-boot-2.0.2.RELEASE.jar:2.0.2.RELEASE]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:327) ~[spring-boot-2.0.2.RELEASE.jar:2.0.2.RELEASE]
	at org.springframework.boot.builder.SpringApplicationBuilder.run(SpringApplicationBuilder.java:137) [spring-boot-2.0.2.RELEASE.jar:2.0.2.RELEASE]
	at com.github.detouched.DemoValueApp.main(DemoValueApp.java:23) [classes/:na]
Caused by: org.springframework.beans.ConversionNotSupportedException: Failed to convert value of type 'java.lang.String' to required type 'java.time.Duration'; nested exception is java.lang.IllegalStateException: Cannot convert value of type 'java.lang.String' to required type 'java.time.Duration': no matching editors or conversion strategy found
	at org.springframework.beans.TypeConverterSupport.doConvert(TypeConverterSupport.java:77) ~[spring-beans-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.beans.TypeConverterSupport.convertIfNecessary(TypeConverterSupport.java:52) ~[spring-beans-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.doResolveDependency(DefaultListableBeanFactory.java:1093) ~[spring-beans-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.resolveDependency(DefaultListableBeanFactory.java:1065) ~[spring-beans-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.beans.factory.support.ConstructorResolver.resolveAutowiredArgument(ConstructorResolver.java:815) ~[spring-beans-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:721) ~[spring-beans-5.0.6.RELEASE.jar:5.0.6.RELEASE]
	... 18 common frames omitted
Caused by: java.lang.IllegalStateException: Cannot convert value of type 'java.lang.String' to required type 'java.time.Duration': no matching editors or conversion strategy found

Meanwhile this works just fine:

    @Bean
    public String demoBean(Props props) {
        return "I was given a property through @ConfigurationProperties: " + props.getProperty();
    }

    @ConfigurationProperties
    public static class Props {
        private Duration property;

        public Duration getProperty() {
            return property;
        }

        public void setProperty(Duration property) {
            this.property = property;
        }
    }

This looks at least inconsistent and doesn't allow the short syntax.

You can find the full code for both use cases in this repository.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 23, 2018
@snicoll
Copy link
Member

snicoll commented May 23, 2018

Introduced in #11078 injection of java.time.Duration properties is incomplete.

I can understand why you feel that way but it isn't. The purpose of that issue was to upgrade Spring Boot's binding and @Value is a framework feature. It would be nice if our converters were also registered in the context though.

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label May 23, 2018
@philwebb
Copy link
Member

philwebb commented May 23, 2018

We considered that but the ApplicationConversionService was added quite late so we didn't want to risk registering it by default. I think this is probably a duplicate of #12148 which we might want to consider again for 2.1.

@detouched
Copy link
Author

Cool, thanks for the explanation. I'll watch #12148 then, I'm happy to close this one if it doesn't make sense on its own.

Is there a reason why Spring Boot has its own conversion mechanism and converters? Properties resolution seems something fundamental to me.

@philwebb
Copy link
Member

@detouched The Duration conversion was added primarily for application.properties support and initially was part of the Binder. Fairly late in the day we realized a general application conversion service would be a good idea. The ConversionService in Spring Boot builds on the one in Spring Framework. It's quite possible that in the future some of these converters might get moved up.

@detouched
Copy link
Author

Ok, I see the picture now. Well, looking forward to the next releases then :)

@snicoll
Copy link
Member

snicoll commented May 30, 2018

Thanks @detouched, we've discussed this today and #12148 is our proposal to support this.

@snicoll snicoll closed this as completed May 30, 2018
@snicoll snicoll added status: duplicate A duplicate of another issue and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels May 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

4 participants