Using Spring Boot 2.2.4.RELEASE.
This is the same problem as in #13155, the difference is that a Flyway bean is being provided using a @Configuration class.
@Configuration
public class FlywayConfiguration {
@Bean
public Flyway myFlyway(DataSource dataSource) {
return new Flyway(...);
}
// This is needed so we can use FlywayMigrationStrategy
@Bean
public FlywayMigrationInitializer myFlywayInitializer(Flyway myFlyway,
ObjectProvider<FlywayMigrationStrategy> migrationStrategy) {
return new FlywayMigrationInitializer(myFlyway, migrationStrategy.getIfAvailable());
}
}
I checked the code and the problem is that in FlywayAutoConfiguration the FlywayMigrationInitializer*DependsOnPostProcessor classes are only imported if Flyway is not defined:
@Configuration(proxyBeanMethods = false)
@ConditionalOnMissingBean(Flyway.class)
@EnableConfigurationProperties({ DataSourceProperties.class, FlywayProperties.class })
@Import({ FlywayMigrationInitializerEntityManagerFactoryDependsOnPostProcessor.class,
FlywayMigrationInitializerJdbcOperationsDependsOnPostProcessor.class,
FlywayMigrationInitializerNamedParameterJdbcOperationsDependsOnPostProcessor.class })
public static class FlywayConfiguration {
As a workaround we can define our own DependsOnPostProcessor class and import it on the @Configuration class.
static class FlywayMigrationInitializerJdbcOperationsDependsOnPostProcessor
extends JdbcOperationsDependsOnPostProcessor {
FlywayMigrationInitializerJdbcOperationsDependsOnPostProcessor() {
super(FlywayMigrationInitializer.class);
}
}
However, this is not ideal because those things are supposed to work out of the box.
The desired behavior is that we would be able to declare our own Flyway bean and everything would work the same way as using the auto-config declared bean.
The solution, in my opinion, is to declare flywayInitializer bean outside FlywayConfiguration class and import FlywayMigrationInitializer*DependsOnPostProcessor in FlywayAutoConfiguration class.
In the attached demo application, uncomment the commented out code in FlywayConfiguration to start the application without error.
Comment From: wilkinsona
Thanks for the analysis.
It's not clear why you've defined both a Flyway bean and a FlywayMigrationInitializer. I believe you could combine things and avoid the problem that you're seeing:
@Bean
public Flyway myFlyway(DataSource dataSource, ObjectProvider<FlywayMigrationStrategy> migrationStrategy) {
Flyway customFlyway = new Flyway();
migrationStrategy.getIfAvailable(() -> (flyway) -> flyway.migrate()).migrate(customFlyway);
return customFlyway;
}
However, this is not ideal because those things are supposed to work out of the box.
The current intent of the auto-configuration is that it will back off if you provide your own Flyway bean. Backing off includes not instructing Flyway to perform its migration as it's expected that this will be done as part of the bean being defined. This is why the FlywayMigrationInitializer and the dependencies upon it are defined in FlywayConfiguration so that they back of in the presence of a Flyway bean.
Comment From: jordanms
Thanks for the reply, @wilkinsona.
It's not clear why you've defined both a Flyway bean and a FlywayMigrationInitializer
As I said, I define it because I want to use FlywayMigrationStrategy. In my current project, we use it for testing so we can clean the db before running tests (don't judge, it's not my call not to use in-memory db for testing :) ).
@Bean
public FlywayMigrationStrategy cleanMigrationStrategy() {
return flyway -> {
flyway.clean();
flyway.migrate();
};
}
I understand the current intent, but for me, they are two different concerns. One is to configure Flyway and the other is to run the migration. Your proposed solution is mixing those two concerns as both are done in the same method -- and a bean declaration is not supposed to do anything else. The other point is that it is basically duplicating code from FlywayMigrationInitializer.
I do think that separating the concerns is the best approach as it is the cleaner solution and FlywayMigrationStrategy does it very well. I just think that they should be separated in the configuration as well.
Comment From: jordanms
Putting it in another way, it should be possible to configure flyway just like that:
@Configuration
public class FlywayConfiguration {
@Bean
public Flyway myFlyway(DataSource dataSource) {
return new Flyway();
}
}
This is currently not possible. The minimum you have to do is:
@Configuration
public class FlywayConfiguration {
@Bean(initMethod = "migrate")
public Flyway myFlyway(DataSource dataSource) {
return new Flyway();
}
}
Comment From: wilkinsona
This is really a question of how and when the auto-configuration should back off. I am not sure that I agree that it should be possible to configure Flyway with just a Flyway @Bean and rely upon the auto-configuration to trigger the migration. That would inconvenience users who want to take complete control over how and when the migration is performed as they'd have to exclude FlywayAutoConfiguration explicitly rather than relying on it backing off.
Comment From: jordanms
That would inconvenience users who want to take complete control over how and when the migration is performed as they'd have to exclude FlywayAutoConfiguration explicitly rather than relying on it backing off
We all agree that any setup we choose will be inconvenient for some use cases. Therefore, we have to choose the one that is convenient for the majority of the use cases. Spring boot definitely does that with auto-configuration the way it is, using application.propperties to configure Flyway. Then we have the less common use cases (the ones we are discussing here): 1. Manually configure Flyway. 2. Manually trigger the migration. 3. Manually configure Flyway and trigger the migration.
Solution with the current implementation:
1.
@Bean(initMethod = "migrate")
public Flyway myFlyway(DataSource dataSource) {
return new Flyway();
}
The problem with that solution is that while your intent is to manually configure Flyway, you are forced to trigger the migration too. You also lose the flywayMigrationStrategy feature (unless you configure it manually too). Basically, you are forced into use case 3.
- See here
@Bean
public FlywayMigrationStrategy flywayMigrationStrategy() {
return flyway -> {
// do nothing
};
}
Exactly what you expect.
@Bean
public Flyway myFlyway(DataSource dataSource) {
return new Flyway();
}
// More code somewhere else to do what's needed.
While the developers get the behaviour they want, the mechanism for disabling auto migration is hidden in the auto-configuration logic. If the developer is not aware of the details, one might spend a considerable amount of time to figure out why the FlywayMigrationStrategy that was defined doesn't work.
Solution with the proposed implementation:
1.
@Bean
public Flyway myFlyway(DataSource dataSource) {
return new Flyway();
}
Exactly what you expect.
2.
@Bean
public FlywayMigrationStrategy flywayMigrationStrategy() {
return flyway -> {
// do nothing
};
}
Exactly what you expect.
@Bean
public Flyway myFlyway(DataSource dataSource) {
return new Flyway();
}
@Bean
public FlywayMigrationStrategy flywayMigrationStrategy() {
return flyway -> {
// do nothing
};
}
// More code somewhere else to do what's needed.
Few lines more of code, but both concerns are clearly overridden (no hidden magic), which reduces the chances of a mistake from the developer, makes the code cleaner and easier to maintain.
Last but not least, among the three use cases we discussed here, the third is clearly the least frequent.
Comment From: wilkinsona
We've discussed this one today and concluded that things should be left as they are. FlywayMigrationStrategy is intended for customization of the migration of the auto-configured Flyway bean. If you are defining your own Flyway bean that is a signal that you wish to take control of how the Flyway bean is defined and migrated. If the behaviour was changed as you have proposed, we may break existing applications that are relying on the definition of a Flyway bean allowing them to take this control.