validate step listeners #3708

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

Jump to bottom

Open

hosuaby wants to merge 1 commit into spring-projects:main

from hosuaby:feature/checkListeners

Open

validate step listeners #3708

hosuaby wants to merge 1 commit into spring-projects:main from hosuaby:feature/checkListeners

+223 −153

Conversation

hosuaby

Copy link

@hosuaby hosuaby commented May 8, 2020

The problem: StepBuilderFactory is very permissive concerning listeners.

Method AbstractTaskletStepBuilder#listener(Object listener) accepts an Object as parameter, and this can lead to wrong step configurations as following:

@Configuration
public class BadJobConfig {
 @Autowired private JobBuilderFactory jobs;
 @Autowired private StepBuilderFactory steps;
 @Bean
 public Job someJob(Step someStep) {
 return jobs
 .get("some-job")
 .start(someStep)
 .build();
 }
 @Bean
 public Step someStep() {
 return steps
 .get("some-step")
 .tasklet((contribution, chunkContext) -> null)
 .listener(new DummyJobExecutionListener()) // mistake! It's job listener, not step listener
 .build();
 }
 static class DummyJobExecutionListener implements JobExecutionListener {
 @Override
 public void beforeJob(JobExecution jobExecution) {
 }
 @Override
 public void afterJob(JobExecution jobExecution) {
 }
 }
}

As you see DummyJobExecutionListener is a job listener! Not step listener. Code compiles and executes without a single warning. This mis-configuration can lead to long painful hours of debugging, before user will notice that he passed wrong listener.

Solution: add runtime assert that will check that Object listener is a valid step execution listener. We can simply use method StepListenerFactoryBean#isListener.

With this feature, the precedent code will throw an exception during application start-up:

Caused by: java.lang.IllegalArgumentException: Object of type [com.adelean.a2.batch.admin.config.BadJobConfig$DummyJobExecutionListener] is not a valid step listener. It must ether implement StepListener interface or have methods annotated with any of:
	- @BeforeStep
	- @AfterStep
	- @BeforeChunk
	- @AfterChunk
	- @AfterChunkError
	- @BeforeRead
	- @AfterRead
	- @OnReadError
	- @BeforeProcess
	- @AfterProcess
	- @OnProcessError
	- @BeforeWrite
	- @AfterWrite
	- @OnWriteError
	- @OnSkipInRead
	- @OnSkipInProcess
	- @OnSkipInWrite

@hosuaby hosuaby force-pushed the feature/checkListeners branch from d7e5bf7 to cb71ecf Compare

May 10, 2020 18:58

@hosuaby

Copy link

Author

hosuaby commented May 10, 2020

Works also with XML config:

<?xml version="1.0" encoding="UTF-8"?>
<beans:beans
 xmlns="http://www.springframework.org/schema/batch"
 xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
 xmlns:beans="http://www.springframework.org/schema/beans"
 xsi:schemaLocation="http://www.springframework.org/schema/batch
 http://www.springframework.org/schema/batch/spring-batch.xsd
 http://www.springframework.org/schema/beans
 http://www.springframework.org/schema/beans/spring-beans.xsd">
 <job id="someJob">
 <step id="someStep">
 <tasklet ref="someTasklet"/>
 <listeners>
 <listener ref="dummyJobExecutionListener" />
 </listeners>
 </step>
 </job>
</beans:beans>