CAMEL-24189: camel-smb - Fix SmbStreamDownloadIT reading stream after close#24818
Conversation
… close Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
left a comment
There was a problem hiding this comment.
Clean, well-targeted fix. The root cause analysis is spot-on.
Analysis
The issue is a lifecycle timing problem: GenericFileOnCompletion.onComplete() fires after the route completes (via processStrategyCommit() → processStrategy.commit()), which for stream-downloaded files closes the underlying SMB file handle. By the time the test calls mock.getExchanges().get(0).getIn().getBody(InputStream.class), the stream is already closed — IOHelper.loadText() reads zero bytes, producing an empty string.
The fix correctly moves stream consumption into a .process() step within the route, where the stream is guaranteed to be open. Stashing the content in a header for post-route verification is a well-established pattern in Camel tests.
Verified
- ✅
GenericFileOnCompletion.processStrategyCommit()callsprocessStrategy.commit(operations, endpoint, exchange, file)at line 137 — confirmed this closes the file resources - ✅
assertInstanceOfcalls insideprocess()properly verify body types (SmbFile wrapping InputStream) - ✅ No
Thread.sleep()introduced - ✅ Imports are clean — removed unused
Assertions/assertNotNull, added usedassertEquals/assertInstanceOf - ✅ This likely resolves the
camel-smbintegration test failures observed in CI runs for other recent PRs
Checklist
| Check | Status |
|---|---|
| Tests | ✅ This IS the test fix |
| Thread.sleep() | ✅ None |
| Commit convention | ✅ CAMEL-24189: format |
| Public API | N/A (test-only) |
| Documentation | N/A |
Reviewed with Claude Code on behalf of gnodet. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 1 tested, 0 compile-only — current: 9 all testedMaveniverse Scalpel detected 1 affected modules (current approach: 9). Modules only in current approach (8)
Skip-tests mode would test 1 modules (1 direct + 0 downstream), skip tests for 0 (generated code, meta-modules) Modules Scalpel would test (1)
All tested modules (9 modules)
|
Claude Code on behalf of davsclaus
Summary
The
SmbStreamDownloadIT.testStreamDownloadtest was reading theInputStreambody aftermock.assertIsSatisfied(), but by that pointGenericFileOnCompletionhas already firedprocessStrategy.commit()which callsreleaseRetrievedFileResources()— closing theSmbFileClosingInputStreamand its underlying SMB file handle. Reading a closed stream returns empty content.Fix: move the stream reading and assertions into a
process()step inside the route where the stream is still open. The content is stashed in a header and verified afterassertIsSatisfied().Test plan
SmbStreamDownloadITpasses (was failing consistently withexpected: <World\n> but was: <>)🤖 Generated with Claude Code