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

Chunk oriented step builders must validate that an ItemWriter is provided [BATCH-2624] #979

Closed
spring-issuemaster opened this issue Jun 22, 2017 · 3 comments

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Jun 22, 2017

Marten Deinum opened BATCH-2624 and commented

Using Java based configuration in a project we accidentally created a chunk based step without a writer.

stepBuilder.chunk(50).reader(someReader).processor(someProcessor).build();

The SimpleChunkProcessor expects both the ItemProcessor and ItemWriter to be set (according to the validation done in the afterPropertiesSet method.

@Override
protected Tasklet createTasklet() {
     Assert.state(reader != null, "ItemReader must be provided");
     Assert.state(processor != null || writer != null, "ItemWriter or ItemProcessor must be provided");
     RepeatOperations repeatOperations = createChunkOperations();
     SimpleChunkProvider<I> chunkProvider = new SimpleChunkProvider<I>(reader, repeatOperations);
     SimpleChunkProcessor<I, O> chunkProcessor = new SimpleChunkProcessor<I, O>(processor, writer);
     chunkProvider.setListeners(new ArrayList<StepListener>(itemListeners));
     chunkProcessor.setListeners(new ArrayList<StepListener>(itemListeners));
     ChunkOrientedTasklet<I> tasklet = new ChunkOrientedTasklet<I>(chunkProvider, chunkProcessor);
     tasklet.setBuffering(!readerTransactionalQueue);
     return tasklet;
}

The createTasklet method in both the SimpleStepBuilder as well as the FaultTolerantStepBuilder accept a null ItemWriter as long as there is a processor. I would expect a ItemWriter next to an ItemReader to be mandatory (which is also what is expressed in the Spring Batch Documentation) but the java config isn't enforcing this, leading to unexpected behavior.

In the XML configuration an ItemReader and ItemWriter are required where an ItemProcessor is optional. (See the ChunkElementParser for that).

The change that lead to this was introduced in BATCH-1520 by @david_syer (so maybe there is a valid reason for this, but still it is weird that XML and Java config lead to different results).


Affects: 3.0.7

Referenced from: pull request #491

Backported to: 4.1.0.M1, 4.0.2, 3.0.10

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jun 22, 2017

Michael Minella commented

The reason those methods accept null is due to the late binding aspects to how it works. I disagree SimpleStepBuilder should trigger #afterPropertiesSet(). That's really the responsibility of the container. That being said, we should be able to do better validation in the #build() method to prevent this from happening.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jun 22, 2017

Marten Deinum commented

I created a pull request for this.

#491

It aligns the XML and Java validations. Nonetheless there must have been a reason for BATCH-1520 that it was implemented like this.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 23, 2018

Mahmoud Ben Hassine commented

I agree with Michael, calling #afterPropertiesSet() is the responsibility of the container.
The important part of this issue is the ability to end up with a chunk oriented step without the mandatory item writer, so I updated the title accordingly.

Marten Deinum Thank you for your PR, it looks good to me. I added a couple of notes on github. If you can manage to address them, your PR will be ready to merge. Thank you upfront.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.