Conditionally create transactionmanager in SimpleBatchConfiguration [BATCH-2766]

[spring-projects-issues]

David J. M. Karlsen opened BATCH-2766 and commented

I think 

SimpleBatchConfiguration.transactionManager() should be conditional on missing bean - since most of the time an application will have a tx manager defined elsewhere.

 

***************************

APPLICATION FAILED TO START

***************************

 

Description:

 

The bean 'transactionManager', defined in com.edb.fs.tac.jfr.srv.dao.infra.DataSourceConfig, could not be registered. A bean with that name has already been defined in class path resource [org/springframework/batch/core/configuration/annotation/SimpleBatchConfiguration.class] and overriding is disabled.

 

Action:

 

Consider renaming one of the beans or enabling overriding by setting spring.main.allow-bean-definition-overriding=true

 

---

Affects: 4.0.1

[spring-projects-issues]

Michael Minella commented

SimpleBatchConfiguration is not autoconfiguration.  Autoconfiguration depends on Spring Boot functionality which Spring Batch does not.  If you have a TransactionManager defined in your context, you either need to re-name it or not use @EnableBatchProcessing (or, as the error states, consider re-activating bean definition overriding which was turned off in recent Boot versions).

[spring-projects-issues]

David J. M. Karlsen commented

It is not autoconfiguration - but I still think it is room for improvement here...

It is very unnatural to have a datasource - but not an according transactionManager along with it. The default name for it is also transactionManager - as you want one - and one only - to coordinate the transactions.

It would make a lot more sense for this constructor to take a transactionmanager as well:

public void setDataSource(DataSource dataSource) {
 if(this.dataSource == null) {
 this.dataSource = dataSource;
 }

 if(this.transactionManager == null) {
 this.transactionManager = new DataSourceTransactionManager(this.dataSource);
 }
}

 

or for it to use an overridable getTransactionManager() so that it could be provided that way.

Fromt the docs: https://github.com/spring-projects/spring-batch/blob/master/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/annotation/EnableBatchProcessing.java

"
|The user should to provide a {@link DataSource} as a bean in the context, or else implement {@link BatchConfigurer} in the configuration class itself, e.g.|

"

Also https://docs.spring.io/spring-boot/docs/current/reference/htmlsingle/#howto-batch-applications suggests this - the current implementation comes in the way of that in a clean way.

[spring-projects-issues]

David J. M. Karlsen commented

Michael Minella WDYT - could you re-consider this?