spring-attic / spring-native

Spring Native is now superseded by Spring Boot 3 official native support
https://docs.spring.io/spring-boot/docs/current/reference/html/native-image.html
Apache License 2.0
2.74k stars 355 forks source link

afterPropertiesSet of InitializingBean is not called as expected #1676

Closed xuanyue202 closed 2 years ago

xuanyue202 commented 2 years ago

Following #1455, I able to make an application with com.alibaba.druid compiled. However, on startup I got the following error:

java.sql.SQLException: url not set
        at com.alibaba.druid.pool.DruidDataSource.resolveDriver(DruidDataSource.java:1194) ~[druid-1.2.11.jar!/:1.2.11]
        at com.alibaba.druid.pool.DruidDataSource.init(DruidDataSource.java:865) ~[druid-1.2.11.jar!/:1.2.11]
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:na]

Then I looked into the code, it should load the property from application.yml via DruidDataSourceWrapper.basicProperties but actually did not happen. I read from the issue list that the ConfigurationProperties annotation has been supported. Noted that DataSourceProperties implements InitializingBean interface, but the afterPropertiesSet method has never been called.

@ConfigurationProperties(prefix = "spring.datasource")
public class DataSourceProperties implements BeanClassLoaderAware, **InitializingBean** {
...

I created a demo project at https://github.com/xuanyue202/aot-druid-demo

java -jar build/libs/demo-0.0.1-SNAPSHOT.jar run successfully while java -DspringAot=true -jar build/libs/demo-0.0.1-SNAPSHOT.jar throws the error above.

snicoll commented 2 years ago

Thanks for the report. This is an ordering problem. Without AOT afterPropertiesSet is called, and then the init method of the parent is invoked. With AOT, it is the reverse order.

This clearly is a bug but DataSourceWrapper could be rearchitectured to rely on the init method rather than introducing another init method. It should rather override init and perform that code prior to calling super.init().

xuanyue202 commented 2 years ago

yes, this is what exactly I did to work around the problem. but i think fixing the bug might help to make spring-native more robust to third party libraries

snicoll commented 2 years ago

I did to work around the problem.

Just a small nit, I don't see it as a workaround. While I consider the difference in ordering between AOT and non-AOT to be a bug, you shouldn't be relying on the order of execution if you need your customizations to apply before the init method is invoked. What you've changed is more idiomatic and would also work if the class was initialized manually.

but i think fixing the bug might help to make spring-native more robust to third party libraries

I agree. Spring Framework 6 is not using the same mechanism and is registering the init method on the bean definition directly, which is more idiomatic and consistent with the non-AOT use case. As Spring Native is in minimal maintenance mode, I am going to close this issue in favor of what we've done in Spring Boot 3. Please give it a try if you get a chance. Thanks!

xuanyue202 commented 2 years ago

@snicoll can you explain a bit more on "Spring Native is minimal maintenance mode". Because we are planning to migrate our springboot services to graalvm, what is your suggestion given that Spring Native is minimal maintenance mode?

bclozel commented 2 years ago

@xuanyue202 we're focusing on Spring Framework 6 and Spring Boot 3, as announced here.