Skip to content

Conversation

@therepanic
Copy link
Contributor

@therepanic therepanic commented Dec 9, 2025

Added BeanNameAware to event item readers to use the bean name when no explicit name is provided. Removed name assertions from the corresponding builders.

I've added a BeanNameAware implementation wherever the builders have a name. I believe this is what was meant in #5149. In any case, we can look into it.

Closes: #5149

…assertions

Added BeanNameAware to event item readers to use the bean name when no
explicit  name is provided. Removed name assertions from the
corresponding builders.

Closes: spring-projectsgh-5149

Signed-off-by: Andrey Litvitski <[email protected]>
@therepanic
Copy link
Contributor Author

Can you tell me roughly why this problem occurs?

Error: Failures:
Error: VirtualThreadsSupportFunctionalTests.testParallelStepsWithVirtualThreads:128 expected: <exitCode=COMPLETED;exitDescription=> but was: <exitCode=FAILED;exitDescription=>

@quaff
Copy link
Contributor

quaff commented Dec 10, 2025

Can you tell me roughly why this problem occurs?

Error: Failures:
Error: VirtualThreadsSupportFunctionalTests.testParallelStepsWithVirtualThreads:128 expected: <exitCode=COMPLETED;exitDescription=> but was: <exitCode=FAILED;exitDescription=>

It's a flaky test.

@banseok1216
Copy link

I don’t think need to add BeanNameAware explicitly to the event-based item readers.

They already get BeanNameAware via the common hierarchy:

Object
└─ ItemStreamSupport implements ItemStream, BeanNameAware
└─ AbstractItemStreamItemReader
└─ AbstractItemCountingItemStreamItemReader
└─ MongoCursorItemReader / other readers...

ItemStreamSupport already sets a default name based on the short class name and only overrides it with the bean name when no explicit name has been set. So removing the strict name assertions in the builders makes sense, but adding BeanNameAware again on the concrete readers looks redundant unless there is a reader that does not extend ItemStreamSupport.

On a related note, I’m a bit unsure about removing the old assertion in the builders:

if (this.saveState) {
    Assert.hasText(this.name, "A name is required when saveState is set to true");
}

When saveState is true, the reader is effectively promising to participate in restart by storing its position in the ExecutionContext under a name-based key. With the assertion gone, misconfiguration around names won’t fail fast anymore, but may instead surface later as subtle restart issues. It might be worth adding a test around this to ensure the implicit naming behaviour is still safe for restarts.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

could we source and default the name from the bean name by having the various types implement BeanNameAware?

3 participants