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

JobRepositoryTestUtils should work against the JobRepository interface #4070

Closed
aritzbastida opened this issue Feb 21, 2022 · 9 comments
Closed

Comments

@aritzbastida
Copy link

aritzbastida commented Feb 21, 2022

The utility class JobRepositoryTestUtils (injected via @SpringBatchTest annotation), uses an incorrect datasource when there is more than one available in the application context, and the one defined as @Primary is not referring to the Spring Batch schema.

Sure, this is already documented in the Javadoc for @SpringBatchTest annotation, but it happens to be a limitation in the infrastructure, and also may lead unexpected behavior:

  • The inner JdbcTemplate instance used for removing job executions may refer to a different datasource as the one for JobRepository.
  • It does not take into account Spring Boot's @BatchDataSource qualifier, or any qualifier, for that matter.

In my scenario, the job is launched fine but then cannot be removed, because the DELETE statements are executed against another database schema.

Ideally, JdbcTemplate should not be needed, and should infer the DataSource from JobRepository, extending it with removal operations. If that's not possible, at least it should be possible to configure the datasource dependency, so that it is used the one annotated with @BatchDataSource.

@aritzbastida aritzbastida added status: waiting-for-triage Issues that we did not analyse yet type: bug labels Feb 21, 2022
@ConnerBabbOCTanner
Copy link

ConnerBabbOCTanner commented Mar 10, 2022

Extending DefaultBatchConfigurer with your configuration and overriding createJobRepository() should allow you to set a datasource explicitly when there are two datasources and even while using SpringBatchTest and SpringBootTest in combination with eachother, but perhaps there is a way to do this with the application.yml or application.properties file also.

@aritzbastida
Copy link
Author

aritzbastida commented Mar 11, 2022

Not exactly. The problem is not with JobRepository, which can be configured as you said; but with JdbcTemplate, which is injected with the "primary" datasource, not necessarily the same one used by JobRepository. The instance of JdbcTemplate is used for explicit removal of job executions (DELETE statements), since JobRepository interface offers no methods to do that.

public JobRepositoryTestUtils(JobRepository jobRepository, DataSource dataSource) {
    super();
    this.jobRepository = jobRepository;
    setDataSource(dataSource);
}

JobRepositoryTestUtils cannot be configured to use a secondary datasource for JdbcTemplate, even if annotated with @BatchDataSource.

@ConnerBabbOCTanner
Copy link

ConnerBabbOCTanner commented Mar 11, 2022

In order to DELETE job executions try something like this:

@Autowire
@Qualifier("batchDataSource")
Datasource batchDataSource

@BeforeEach
public void cleanUp() throws SQLException {
   batchDataSource.getConnection().prepareCall(" ***SQL HERE*** ").execute();
}

There should be a way to set the datasource with the application.yml or application.properties file. Adding the datasource to the application config would make JobRepository more intuitive, annotating with @BatchDataSource should do the same and JobRepository should have context to be able to get the datasource to make statements.

@aritzbastida
Copy link
Author

IMHO, executing my own SQL statements defeats the whole purpose of using JobRepositoryTestUtils altogether.

All I'm saying is that JobRepositoryTestUtils.removeJobExecutions() always uses the primary datasource. Moreover, the remaining methods (such as createJobExecutions()) don't necessarily access the same datasource, depending on how JobRepository is configured. This could lead to unintended results.

Anyway, I applied a simple workaround in an auto-configuration class to fix it: jobRepositoryTestUtils.setDataSource(ds);

But I thought that I should report it, and contribute to Spring Batch in some way. If you ask me, I would move removeJobExecutions() as a new method in JobRepository interface, encapsulating all repository logic behind it. This would guarantee that the same datasource is used every time. That method could be used, not only by JobRepositoryTestUtils, but also Spring Cloud Data Flow (which also has similar DELETE statements).

@fmbenhassine fmbenhassine added in: test and removed status: waiting-for-triage Issues that we did not analyse yet labels May 5, 2022
@fmbenhassine fmbenhassine added this to the 5.0.0 milestone May 5, 2022
@fmbenhassine fmbenhassine modified the milestones: 5.0.0, 5.0.0-M5 Jun 8, 2022
@fmbenhassine
Copy link
Contributor

@aritzbastida Thank you for reporting this issue! I agree with you. The datasource is an implementation detail of the JDBC-based JobRepository. I believe it should not have been configurable in the JobRepositoryTestUtils in the first place, because it is not at the same level of abstraction as the JobRepository.

JobRepositoryTestUtils should be configurable with a JobRepository and that is it, no matter what implementation is used behind the scene. The problem will be more visible when a NoSQL Job repository implementation will be introduced in the project. For instance, with a MongoDB JobRepository, there is no need for a datasource bean at all, and we should still be able to use the JobRepositoryTestUtils to populate/cleanup the job repository as needed in tests.

So yes, there should be methods in JobRepository that allow JobRepositoryTestUtils to implement its utilities without knowing about the implementation details of the JobRepository itself.

This enhancement will supersede #4178. I will try to include it in the upcoming 5.0.0-M5 release.

@aritzbastida
Copy link
Author

Thank you for considering to implement this issue. :)

@fmbenhassine fmbenhassine changed the title JobRepositoryTestUtils (from @SpringBatchTest) using incorrect datasource JobRepositoryTestUtils should work against the JobRepository interface Aug 23, 2022
@suhana9010
Copy link

@fmbenhassine @aritzbastida what if we use mongodb? We don't have datasource here

@fmbenhassine
Copy link
Contributor

@suhana9010

what if we use mongodb? We don't have datasource here

As mentioned in my previous comment, we should be able to use the utility against any JobRepository (and I gave the mongo db sample where no datasource is involved). This was fixed in c425137. If you define a JobRepository in the test context, the utility will use it (no matter if there is a datasource bean or not).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants