Fix duplicate listener registration in ChunkOrientedStepBuilder

[n-dlms]

Summary

Fixes #5466

When an ItemReader, ItemProcessor, or ItemWriter implements a StepListener sub-interface (e.g. ItemProcessListener) and also has a method annotated with a listener annotation (e.g. @BeforeStep), the object is registered as a listener twice, so every interface callback fires twice per item. The annotated method itself fires once; only the interface callbacks are doubled.

Root cause

ChunkOrientedStepBuilder#addAsStreamAndListener has two non-exclusive branches:

if (itemHandler instanceof StepListener listener) {
    this.stepListeners.add(listener);       // raw object
}
if (StepListenerFactoryBean.isListener(itemHandler)) {
    StepListener listener = StepListenerFactoryBean.getListener(itemHandler);
    this.stepListeners.add(listener);       // raw object OR proxy
}

When the object only implements the interface, getListener returns the delegate itself, so the LinkedHashSet deduplicates and callbacks fire once. But when an annotation is present, getListener returns a proxy (a different instance), so both the raw object and the proxy are registered and interface callbacks fire twice.

This is a regression from 5.2.x where SimpleStepBuilder#registerAsStreamsAndListeners used a single StepListenerFactoryBean.isListener(...) branch.

Fix

Make the two branches mutually exclusive, preferring the factory branch (it already covers both interface implementations and annotation-based listeners):

if (StepListenerFactoryBean.isListener(itemHandler)) {
    this.stepListeners.add(StepListenerFactoryBean.getListener(itemHandler));
}
else if (itemHandler instanceof StepListener listener) {
    this.stepListeners.add(listener);
}

Tests

Added two regression tests to ChunkOrientedStepTests:

• testListenerCallbacksFireOnceWhenProcessorImplementsListenerOnly — verifies interface callbacks fire exactly once per item (no annotations)
• testListenerCallbacksFireOnceWhenProcessorAlsoHasAnnotatedMethod — verifies interface callbacks fire exactly once per item even when a @BeforeStep annotated method is present (fails before the fix, passes after)

Verification

• New tests: pass
• Full spring-batch-core unit suite: 806 tests run, 0 failures

[benelog]

Hi @n-dlms, thanks for looking into this.

Heads-up that this duplicates #5467, which I opened on July 22 (I reported #5466). The production change is functionally the same in both PRs, and the same as the "Suggested fix" in the issue description; only the comment wording and an inlined local differ. Yours is the better comment, since it says why the branches must be mutually exclusive.

The real difference is the tests:

• Fix duplicate listener registration in ChunkOrientedStepBuilder #5467: one test in ChunkOrientedStepIntegrationTests, starting a real job through JobOperator with a JDBC job repository.
• Fix duplicate listener registration in ChunkOrientedStepBuilder #5478: two tests in ChunkOrientedStepTests, calling ChunkOrientedStep#execute directly, with a control case (interface only) alongside the failing case.

Both cover the regression, so whichever one the maintainers prefer is fine by me: I can close #5467, or fold your control case into it.

Two nits here, if this one moves forward: import ...infrastructure.item.ItemProcessor; is redundant next to the existing import ...infrastructure.item.*;, and new java.util.ArrayList<>() is fully qualified inline.

[benelog]

Follow-up: I've added the control case to #5467 as well (testItemHandlerImplementingListenerInterfaceIsRegisteredOnce), so both PRs now cover the same two cases. It's the integration-test equivalent of callbacksFireOnceWhenOnlyTheInterfaceIsImplemented from the reproducer linked in #5466, but your PR was a good reminder that it belongs upstream too, not just in the reproducer.

With that, the only remaining difference is the level the tests run at: #5467 starts a real job through JobOperator with a JDBC job repository, whereas this one calls ChunkOrientedStep#execute directly. Either works, so whichever one the maintainers prefer is fine by me.

[n-dlms]

@benelog Thanks for the follow-up, Sanghyuk. I appreciate you adding the control case to #5467 and for the clarification on the test-level difference. Glad the PR was useful in surfacing that case for the upstream tests. Either approach makes sense to me, so I'm happy to leave the final choice to the maintainers.