Context
While looking into the codebase, I noticed a slight inconsistency in JdbcAggregateTemplate.streamAll(Class<T>). It seems this method is currently only used in test code (AbstractJdbcAggregateTemplateIntegrationTests, MyBatisDataAccessStrategyUnitTests), so this might not be a critical issue. However, I wanted to share this finding as its behavior differs from the other streamAll overloads.
Description
JdbcAggregateTemplate.streamAll(Class<T> domainType) currently triggers AfterConvertEvent and AfterConvertCallback twice per entity instead of once.
Current Implementation
The implementation calls triggerAfterConvert twice on the same entities:
public <T> Stream<T> streamAll(Class<T> domainType) {
Iterable<T> items = triggerAfterConvert(accessStrategy.findAll(domainType)); // 1st trigger (eager)
return StreamSupport.stream(items.spliterator(), false).map(this::triggerAfterConvert); // 2nd trigger
}
triggerAfterConvert(Iterable<T>) eagerly iterates every element and calls triggerAfterConvert(T) on each (firing AfterConvertEvent + AfterConvertCallback). Then .map(this::triggerAfterConvert) fires them again when the stream is consumed.
Eager vs Lazy Evaluation
Additionally, the method uses accessStrategy.findAll() (eager — loads all results into List<T>) instead of accessStrategy.streamAll() (lazy cursor-backed stream via queryForStream).
All three sibling overloads added in the same commit use the lazy pattern:
// streamAll(Class, Sort) — consistent
return accessStrategy.streamAll(domainType, sort).map(this::triggerAfterConvert);
// streamAll(Query, Class) — consistent
return accessStrategy.streamAll(query, domainType).map(this::triggerAfterConvert);
// streamAllByIds(...) — consistent
return accessStrategy.streamAllByIds(ids, domainType).map(this::triggerAfterConvert);
Expected vs Current Behavior
- Expected:
AfterConvertEvent and AfterConvertCallback fire exactly once per entity.
- Current: Both fire twice per entity.
Verified with a counter listener: 3 entities → 6 events (expected 3).
streamAll(Class) AfterConvertEvent count: 6 ← current
streamAll(Class, Sort) AfterConvertEvent count: 3 ← expected
Reproduction
AtomicInteger count = new AtomicInteger();
// Register AfterConvertEvent listener for Department
context.addApplicationListener((AfterConvertEvent<?> e) -> {
if (e.getEntity() instanceof Department) count.incrementAndGet();
});
// Save 3 entities, then:
template.streamAll(Department.class).collect(Collectors.toList());
assertThat(count.get()).isEqualTo(3); // FAILS — actual: 6
Suggested Change
public <T> Stream<T> streamAll(Class<T> domainType) {
return accessStrategy.streamAll(domainType).map(this::triggerAfterConvert);
}
This minor refactoring would:
- Remove the double trigger (events fire once per entity).
- Make the stream truly lazy (cursor-backed via
queryForStream), aligning it with all sibling overloads.
Context
While looking into the codebase, I noticed a slight inconsistency in
JdbcAggregateTemplate.streamAll(Class<T>). It seems this method is currently only used in test code (AbstractJdbcAggregateTemplateIntegrationTests,MyBatisDataAccessStrategyUnitTests), so this might not be a critical issue. However, I wanted to share this finding as its behavior differs from the otherstreamAlloverloads.Description
JdbcAggregateTemplate.streamAll(Class<T> domainType)currently triggersAfterConvertEventandAfterConvertCallbacktwice per entity instead of once.Current Implementation
The implementation calls
triggerAfterConverttwice on the same entities:triggerAfterConvert(Iterable<T>)eagerly iterates every element and callstriggerAfterConvert(T)on each (firingAfterConvertEvent+AfterConvertCallback). Then.map(this::triggerAfterConvert)fires them again when the stream is consumed.Eager vs Lazy Evaluation
Additionally, the method uses
accessStrategy.findAll()(eager — loads all results intoList<T>) instead ofaccessStrategy.streamAll()(lazy cursor-backed stream viaqueryForStream).All three sibling overloads added in the same commit use the lazy pattern:
Expected vs Current Behavior
AfterConvertEventandAfterConvertCallbackfire exactly once per entity.Verified with a counter listener: 3 entities → 6 events (expected 3).
Reproduction
Suggested Change
This minor refactoring would:
queryForStream), aligning it with all sibling overloads.