Repository navigation
CAMEL-25950: camel-sjms - SjmsMessage keeps the attachments when the exchange is copied - #28644
Conversation
…exchange is copied SjmsMessage.copyFrom overrides MessageSupport.copyFrom without calling it, and copies only the body and the headers. Since CAMEL-21755 the attachments of a message are its ATTACHMENTS trait (before, they were exchange properties that every exchange copy kept), so a copy of an exchange received from sjms lost them: Multicast, Recipient List, Split, Wire Tap and the mock endpoint all work on copies. camel-jms had the same gap in JmsMessage, fixed by CAMEL-23193, removed by CAMEL-24365 and fixed again by CAMEL-25779. SjmsMessage.copyFrom now copies the ATTACHMENTS trait into a new map, as MessageSupport and JmsMessage do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Same as JmsMessage.copyFrom and MessageSupport.copyFromWithNewBody: the ATTACHMENTS trait is copied into a new map, newInstance() returns an SjmsMessage so the multicast copy goes through it, and sjms2 gets the fix too. Test style matches the module. Will approve once CI is green; the branch is ~330 commits behind main so a fresh merge run matters.
Suggestion: MessageSupport.copyFrom also copies the DATA_AWARE trait, SjmsMessage/JmsMessage do not (pre-existing, possible follow-up). Nit: unchecked cast without @SuppressWarnings.
Reviewed with Claude Code on behalf of @davsclaus. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
|
🧪 CI tested the following changed modules:
🔁 2 tests passed only after a retry on JDK 21 (2 retried attempts) Recovered flaky tests on JDK 21 (2)
🔬 Scalpel shadow comparison — Scalpel: 10 of 709 tested, 27 compile-only — current: 10 all testedMaveniverse Scalpel detected 10 affected modules (current approach: 10). Skip-tests mode would test 10 modules (1 direct + 9 downstream), skip tests for 27 (generated code, meta-modules) Modules Scalpel would test (10)
Modules with tests skipped (27)
All tested modules (37 modules, 6m 8s total)Total reactor time: 6m 8s
Top 20 slowest modules:
|
…ttachments The branch was about 330 commits behind main; merged (not rebased) so the review history stays. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ed warning of the attachments cast Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@davsclaus thanks for the review. Changes on top of the branch:
Open follow-up: Claude Code on behalf of allthingssecurity |
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
The changes since my review address my points; CI is green. Thanks.
Reviewed with Claude Code on behalf of @davsclaus. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
Description
CAMEL-25950
SjmsMessage.copyFromoverridesMessageSupport.copyFromwithout calling it. It copies the body and the headers, but not theMessageTrait.ATTACHMENTSpayload, where the attachments of a message are kept (camel-attachmentsAttachmentMessagestores them there).So when an exchange received from an sjms endpoint is copied, the copy has no attachments. Multicast, Recipient List, Split and Wire Tap all work on copies, and so does a mock endpoint. A route like
from("sjms:queue:in")that adds attachments and then multicasts or splits sends the copies on without them, and nothing is logged.Until CAMEL-21755 (4.10.1) the attachments were exchange properties, and every exchange copy kept them. CAMEL-21755 moved them into a message trait, which only
MessageSupport.copyFromWithNewBodycopies. camel-jms had the same gap inJmsMessage.copyFrom: CAMEL-23193 fixed it, CAMEL-24365 removed that copy, and CAMEL-25779 (#28083) restored it.SjmsMessagenever got the copy.Changes:
SjmsMessage.copyFromcopies theATTACHMENTStrait into a new map, the same way asMessageSupportandJmsMessage(CAMEL-25779: camel-jms: JmsMessage loses attachments when exchange is copied #28083).SjmsMessage, so it is fixed as well.SjmsMessageis not affected:DefaultMessage.reset()already removes the trait.port/*labels): camel-4.23.x and 4.22.x have the sameSjmsMessageas main. camel-4.18.x and 4.14.x differ in other parts of the class, but theircopyFromdoes not copy the attachments either. I read the code on those branches but did not run the tests there.Not changed:
MailMessage.copyFrom(camel-mail) does not copy the trait either whenmapMailMessage=false. In that mode the consumer does not extract attachments, so only attachments added by the route are affected. I can file a follow-up JIRA if wanted.Tests:
SjmsMessageCopyAttachmentsTest(JUnit assertions, as in the module). It uses the embedded in-VM Artemis broker of the other sjms tests (JmsTestSupport).multicastCopyKeepsTheAttachmentsOfAnSjmsMessage: a route from sjms adds an attachment and multicasts to a mock. Result:the copy made by the multicast must keep the attachments ==> expected: not <null>.copyKeepsTheAttachments(SjmsMessage.copy()):copy() must keep the attachments ==> expected: not <null>.copyOfAMessageWithoutAttachmentsHasNoneis a control that passes on main and with the fix.-Dasm-version=9.11because asm 9.11.1 was not in my local repository.Target
mainbranch)Tracking
Apache Camel coding standards and style
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.(I built camel-sjms and ran its tests, including the formatter and import-sort plugins. No generated file changes. I did not run the full root build.)
AI-assisted contributions
Co-authored-bytrailers) and the PR description identifies the AI tool used.This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a
Co-Authored-Bytrailer.Claude Code on behalf of allthingssecurity
🤖 Generated with Claude Code