Document that properties migrator does not consider properties added late to the environment

[surfsoft]

Spring version: 2.0.2.RELEASE
Java version: 1.8.0_171

I've a Spring Boot application I've been upgrading from 1.5.6 to 2.0.2. I've added spring boot properties migrator as a runtime dependency to highlight deprecated properties in the property files that I need to change as part of the migration, and the migrator does indeed run at start-up.

The PropertiesMigrationListener class provided as part of the property migrator creates a report of deprecated properties when the ApplicationPreparedEvent event is handled and issues that report when it receives either the ApplicationReadyEvent or ApplicationFailedEvent:

@Override
public void onApplicationEvent(SpringApplicationEvent event) {
    if (event instanceof ApplicationPreparedEvent) {
        onApplicationPreparedEvent((ApplicationPreparedEvent) event);
    }
    if (event instanceof ApplicationReadyEvent
        || event instanceof ApplicationFailedEvent) {
        logLegacyPropertiesReport();
    }
}

I've a default application properties file default.properties. It ships as part of the deployed jar and is specified as a property source against the Application class:

@Configuration
@EnableAutoConfiguration
@ComponentScan(basePackageClasses = Application.class)
@PropertySource(value = "classpath:/default.properties")
@EnableAspectJAutoProxy
@EnableCaching
public class Application {
    public static void main(String[] args) {
    ...
    }
}

What I've discovered is that when the ApplicationPreparedEvent is handled, the default.properties file hasn't been consumed. By the time that the ApplicationReadyEvent event is handled however it has been consumed. I've ascertained this by examining the contents of event.getApplicationContext().getEnvironment() whilst debugging the application start-up. Between the two events the list of property sources returned has grown by four elements, including the default.properties file.

The overall effect is that spring-boot-properties-migrator doesn't report on deprecated properties present in my default.properties file, or any other property source added between the ApplicationPreparedEvent and the ApplicationReadyEvent.

[wilkinsona]

Thanks for the detailed analysis. We do not recommend using @PropertySource for exactly this sort of reason. Customisation of the environment should be done with an EnvironmentPostProcessor instead.

I’ll leave this open to see if we can change the migrator to cope with your scenario but I’d recommend moving to an EnvironmentPostProcessor regardless of the migrator’s behaviour.

[snicoll]

There are two roles in the migration process: one is to report deprecation and errors but another one is to migrate properties automatically if we can. And this must happen early so that auto-configuration can use it properly. ApplicationReadyEvent is most certainly too late for that.

Given that we do not recommend using @PropertySource and the tool is targeted at making sure auto-configuration works as expected, my vote is to document this limitation.

[surfsoft]

I'm OK with you documenting the limitation.

…

On Mon, Jul 9, 2018 at 8:51 AM Stéphane Nicoll ***@***.***> wrote: There are two roles in the migration process: one is to report deprecation and errors but another one is to migrate properties automatically if we can. And this must happen early so that auto-configuration can use it properly. ApplicationReadyEvent is most certainly too late for that. Given that we do not recommend using @propertysource and the tool is targeted at making sure auto-configuration works as expected, my vote is to document this limitation. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#13600 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA2hmTn3cLdx_sppxTNX-IgdyWKdvsfiks5uEwuDgaJpZM4U5iSY> .