Repository navigation
CAMEL-25378: Add camel-test-citrus component for Citrus message validation - #27792
Merged
christophd merged 17 commits intoOct 16, 2026
Merged
Conversation
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
github-actions
Bot
added
components
core
catalog
docs
dsl
tooling
tooling-maven
core-build-and-dependencies
labels
gnodet-bot
reviewed
Oct 13, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Testing auth - ignore
gnodet-bot
reviewed
Oct 13, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Test with one inline comment
gnodet-bot
reviewed
Oct 13, 2026
gnodet-bot
reviewed
Oct 13, 2026
gnodet-bot
reviewed
Oct 13, 2026
gnodet-bot
suggested changes
Oct 13, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
This PR introduces a well-structured camel-test-citrus component with a clean fluent API and good test coverage. A few issues to address:
operationparameter not validated — invalid URIs silently produce non-functional endpointsresolveValidationSpecreturningnullcauses silent pass-through — no validation, no warningexpectedMessageCount()doesn't reset counters — stalereceivedCountcauses false assertion failures on reuse@UriPathonoperationmissingenums— valid values not enforced at the metadata level- Shared mutable state fields not
volatile— cross-thread visibility issues in test scenarios CitrusContextcreated indoStart()but never closed — resource leak when the Camel context stops
This review was generated by an AI agent, Hermès on behalf of @gnodet.
Contributor
✅ Generated files are up to dateAn earlier CI run reported uncommitted generated changes; the latest run no longer does. |
gnodet-bot
reviewed
Oct 13, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Following up on my earlier review — 0 of 6 issues have been addressed. The new commit (6ba771c) is a pure DSL regeneration and does not touch any of the reported files.
Findings outstanding:
⚠️ operationnot validated — invalid values silently produce non-functional endpoints (CitrusComponent.java)⚠️ resolveValidationSpecreturningnullcauses silent pass-through — no validation, no warning (CitrusProducer.java)⚠️ expectedMessageCount()does not reset counters — stalereceivedCountcauses false assertion failures on reuse (CitrusEndpoint.java)- 💡
@UriPathonoperationmissingenums— valid values not enforced at metadata level (CitrusEndpoint.java) ⚠️ Mutable state fieldsallMessagesSpec,allMessagesResponseSpecnotvolatile— cross-thread visibility issues (CitrusEndpoint.java)⚠️ CitrusContextcreated indoStart()but never closed — resource leak on Camel context stop (CitrusProducer.java)
This review was generated by an AI agent, Hermès on behalf of @gnodet.
gnodet-bot
reviewed
Oct 13, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Following up on my earlier review — 0 of 6 issues have been addressed. The new commit (6ba771c) is a pure DSL regeneration and does not touch any of the reported files.
Findings outstanding:
⚠️ operationnot validated — invalid values silently produce non-functional endpoints (CitrusComponent.java)⚠️ resolveValidationSpecreturningnullcauses silent pass-through — no validation, no warning (CitrusProducer.java)⚠️ expectedMessageCount()does not reset counters — stalereceivedCountcauses false assertion failures on reuse (CitrusEndpoint.java)- 💡
@UriPathonoperationmissingenums— valid values not enforced at metadata level (CitrusEndpoint.java) ⚠️ Mutable state fieldsallMessagesSpec,allMessagesResponseSpecnotvolatile— cross-thread visibility issues (CitrusEndpoint.java)⚠️ CitrusContextcreated indoStart()but never closed — resource leak on Camel context stop (CitrusProducer.java)
This review was generated by an AI agent, Hermès on behalf of @gnodet.
gnodet-bot
reviewed
Oct 13, 2026
gnodet-bot
reviewed
Oct 13, 2026
gnodet-bot
reviewed
Oct 13, 2026
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 592 of 706 tested, 0 compile-only — current: 70 all testedMaveniverse Scalpel detected 592 affected modules (current approach: 70). Skip-tests mode would test 592 modules (12 direct + 37 downstream), skip tests for 0 (generated code, meta-modules)
|
| Module | Duration | Status |
|---|---|---|
| Camel :: Component DSL | 50.7s | SUCCESS |
| Camel :: Main | 33.2s | SUCCESS |
| Camel :: Catalog :: Camel Catalog | 22.5s | SUCCESS |
| Camel :: Test :: Citrus | 19.1s | SUCCESS |
| Camel :: Docs | 14.8s | SUCCESS |
| Camel :: Maven Plugins :: Camel Maven Package | 12.9s | SUCCESS |
| Camel :: Kamelet Main | 11.7s | SUCCESS |
| Camel :: Util | 9.4s | SUCCESS |
| Camel :: All Components Sync point | 5.0s | SUCCESS |
| Camel :: Parent | 1.3s | SUCCESS |
| Camel :: Test :: Parent | 0.8s | SUCCESS |
| Camel :: BOM | 0.2s | SUCCESS |
| Camel :: Endpoint DSL | n/a |
Top 20 slowest modules:
Camel :: Component DSL(50.7s)Camel :: Main(33.2s)Camel :: Catalog :: Camel Catalog(22.5s)Camel :: Test :: Citrus(19.1s)Camel :: Docs(14.8s)Camel :: Maven Plugins :: Camel Maven Package(12.9s)Camel :: Kamelet Main(11.7s)Camel :: Util(9.4s)Camel :: All Components Sync point(5.0s)Camel :: Parent(1.3s)Camel :: Test :: Parent(0.8s)Camel :: BOM(0.2s)
davsclaus
reviewed
Oct 14, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Thanks @christophd. It is good to see Citrus validation inside Camel route tests.
What I checked
- CI is green (build on JDK 17 and 25, license check, dependency review), and the generated files are up to date.
- Build integration is complete: the module is in the
camel-testparent pom, thecamel-testlist inMojoHelper,parent/pom.xml,camel-bom,camel-allcomponents, the catalog (components.properties,docs.properties,citrus.json),nav.adocand the docs symlinks. TheArtifactUtilsand kamelet-main entries are generated. citrus-version(5.0.2) is already managed inparent/pom.xmland used bycamel-jbang-plugin-test. The PR adds no new version property.- I read the source, the docs page, all 33 tests and the open gnodet-bot threads. I checked the Citrus 5.0.2 API in the local Maven repo where noted below.
On the gnodet-bot findings
I agree with all six. The most important ones are the silent pass when no spec is configured and the CitrusContext that is never closed. On the volatile point, the LinkedHashMap spec maps are written by the test thread and read by the routing thread too, so a ConcurrentHashMap matters more than the field modifiers. enums would also help on messageType. I do not repeat these below.
Must fix
firstVersionis wrong. 4.23.0 is tagged andcamel-4.23.xis branched, so this ships in 4.24.0. ChangefirstVersionin the pom and in@UriEndpoint, and:since:in the docs page to 4.24, then regenerate.
Design questions (these matter most to me, as the API is hard to change after release)
- A validation failure is thrown into the route. This means the route's own error handling runs on a test assertion.
onExceptionor a dead letter channel can swallow the failure, and with redelivery the producer runs again. Each retry moves the exchange index and the received count, somessage(1)is applied to a retry of the first exchange.MockEndpointrecords failures and reports them inassertIsSatisfied(), and route behaviour does not change. ForvalidateI would record failures and not throw by default, perhaps with a fail-fast option. Forreceive, throwing makes sense, because it simulates a service that rejects a bad request. Either way, please document it and add a test with redelivery oronException. - The endpoint is not part of the existing camel-test helpers.
MockEndpoint.assertIsSatisfied(context)andMockEndpoint.resetMocks(context)only seeMockEndpointinstances. So a test that uses the context-wide assert skips the Citrus endpoints and passes. Did you consider extendingMockEndpoint? That gives the context-wide assert and reset, and all the existingexpected*methods, with Citrus validation added on top. The other option is a staticassertIsSatisfied(CamelContext)for Citrus endpoints. assertIsSatisfied()passes whenexpectedMessageCount()was never called, even if specs are configured and no exchange arrived. This is a different case from gnodet-bot's null-spec finding. I suggest defaulting the expected count to the number of indexed specs, or failing when specs exist and nothing arrived.CitrusContextlifecycle. Each producer creates its ownCitrusContext, which does the Citrus registry lookups each time. The component could own one context (created indoStart, closed indoStop) and accept an existing one from the registry. With an existing context, a test that runs under the Citrus JUnit extension can share its context and variables. This also fixes the leak gnodet-bot reported.- The received message is built from the raw body and a plain copy of the headers. An
InputStreamorStreamCachebody is read by Citrus'DefaultTypeConverterand not reset afterwards. So the next step in the route may see an empty body, although the docs sayvalidatedoes not modify the exchange. POJO orMapbodies also go through Citrus' converter, not Camel's. Camel headers are case-insensitive, but the copy into aLinkedHashMaploses that. Citrus 5.0.2 hasHeaderValidationContext.Builder#ignoreCase(true)for this. Details are in the inline comments. - Only exchange properties become Citrus variables. Camel 4 exchange variables (
setVariable) seem a closer match for Citrus test variables. Also,@variable()@extraction is lost after each exchange, because theTestContextis created per exchange. Within one exchange it can only feed thereceiveresponse.
Docs
- The docs page has no Maven dependency snippet. This matters more than usual here, because the artifactId (
camel-test-citrus) differs from the scheme and the dependency should use test scope. - The "Path validation" section describes automatic classification of
$and/expressions, but this component does not do that. The real API,allMessages().validate(new DefaultValidations().jsonPath()...), is not shown anywhere on the page. The JSON and XML tests have good examples to use instead. - The docs say a missing spec fails with an error. That is only true when some indexed spec exists. See gnodet-bot's null-spec finding.
Tests and conventions
- New components use
camel-test-junit6. Every component pom added in the last four months does. The explicitjunit-jupiterdependency is redundant, and theassertj-coreversion is already managed inparent/pom.xml. - There is a fully qualified class name in
CitrusValidateJsonTest(inline comment). - The PR description lists tests that do not exist: XML namespace-aware validation, plain text ignore expressions and YAML ignore expressions. There are 33 tests, not 34. The positional
@ignore(n)@plain text section in the docs has no test. There are no tests for theassertIsSatisfied()failure paths (count mismatch, timeout, collected failures). Please add the missing tests or correct the description. - Minor:
toUpperCase()withoutLocale.ROOTinresolveMessageType.
Not blocking, for later
The expectations can only be set through the Java fluent API, so YAML and XML routes and the Camel CLI (camel test) cannot use them. The JIRA says Java DSL comes first, which is fine. A follow-up that loads expected templates from a resource, for example citrus:validate?expected=classpath:order.json, would make this usable outside Java tests.
I am leaving a comment rather than a change request, since gnodet-bot's change request is still open. Item 1 is a real must-fix. Items 2 to 4 are what I would like to settle before merge.
Checked against the project conventions in CLAUDE.md. This is a rules-and-conventions review and does not replace CodeRabbit, Sourcery or SonarCloud.
Claude Code on behalf of @davsclaus.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
gnodet-bot
reviewed
Oct 14, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Re-review after latest commits — 4 issues found, of which the previous concerns have been addressed.
The new commits brought good fixes (validated operation guard, expectedMessageCount counter management, @UriPath enum annotation, volatile fields, CitrusContext lifecycle). What remains are four new or remaining concerns with the updated code.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
gnodet-bot
reviewed
Oct 15, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Re-review after latest commits — all prior findings have been addressed ✅
Summary of addressed issues:
namefield restored inCitrusEndpointwith propergetName()/setName()overrides that keep the local field in sync with the parentMockEndpointfield- Catalog JSON corrected:
nameparameter now showsrequired: false(matching@UriPathwithout@Metadata(required = true)) - Empty section comments (
--- Validation spec ---,--- Response spec ---) removed - Error handling documented in
citrus-component.adocwith a dedicated "Error handling and redelivery" section CitrusReceiveErrorHandlingTestadded with two tests covering the error swallowing vs propagation semanticswriteBackExtractedVariablescontainsKeyguard removed;CitrusVariableTestextended with 3 tests (propagation, extraction, extraction-when-property-exists)
No new issues found in the latest commits. The implementation is clean — good job addressing all review feedback.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
Contributor
Author
|
@davsclaus ready to merge when you say "go" 😄 |
davsclaus
requested changes
Oct 15, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Thanks @christophd for working through all of this. I compared 9b9a117..0929ff0 (1 new commit) and went through my earlier points:
namepath option: fixed. The field is back with its own@UriPathdescription, andgetName()/setName()are overridden.citrus.json, the catalog copy,CitrusEndpointUriFactoryand the endpoint DSL all sayrequired: falsewith the Citrus description now.- Implied minimum count: fixed and tested.
shouldFailWhenImpliedMinimumCountNotMetcovers it, and the docs explain it. - Variable tests: added.
CitrusVariableTestcovers a variable set before the endpoint, a value extracted with@variable()@after it, and an extracted value that overwrites a variable while the property keeps its value. onExceptiontest: added.CitrusReceiveErrorHandlingTestpins thatvalidateclears the exception andreceivekeeps it, and the new "Error handling and redelivery" section explains it.- Minor: the empty section comments are gone, the double registration in
receive()has a comment, and the PR description is up to date.
New finding, from the write-back change: every exchange property now becomes an exchange variable (inline). propagateExchangeVariables copies all exchange properties into the TestContext, and writeBackExtractedVariables now writes every context variable back with exchange.setVariable(...), so after citrus:validate a property foo also exists as a variable foo (and so does any global Citrus variable). That goes against "validate validates exchanges without modifying them" in the docs and the operation option. Writing back only the keys that are new or changed since the propagation keeps shouldExtractVariableEvenWhenPropertyExists passing. Please add a test that an unrelated property does not show up as a variable.
Non-blocking:
- The new error-handling section says redelivery fires for
receive, but not that each retry counts as a new exchange in the mock counter, soreceive(2)applies to the retry of the first exchange. One sentence would cover it. - The thread on the operation list (line 43) is still open: the failure behaviour of
validateis not stated up front. The new section covers it further down, so this is a nit.
CI is green on 0929ff0. I am keeping the change request only for the property-to-variable side effect, and will approve once that is fixed and tested.
Claude Code on behalf of @davsclaus.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
gnodet-bot
reviewed
Oct 15, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Following up on my earlier review — all outstanding issues are resolved.
✅ Fixed in 0929ff0d:
- Dead
namefield —getName()/setName()override now correctly delegate to the CitrusEndpoint field. - Lambda NPE risk —
createTestContext()guards against a nullcitrusContextwith an explicitIllegalStateException.
✅ Fixed in c09d53df (this commit):
writeBackExtractedVariablesproperty leakage — the snapshot-and-diff approach is correct. Taking aHashMapsnapshot oftestContext.getVariables()afterpropagateExchangeVariables()and then writing back only new or changed entries cleanly prevents exchange properties from leaking into exchange variables while preserving all Citrus extraction semantics. All three code paths are covered by existing tests: the newshouldNotWriteBackPropagatedPropertiesAsVariablestest covers the fix directly,shouldExtractVariableFromBodyvalidates new extractions still work, andshouldExtractVariableEvenWhenPropertyExistscovers the changed-value write-back path.
Re: double registration in WhenSpec.receive() / ThenSpec.respond() — the previous concern was a design misread. The initial registration in receive(int) is intentional and necessary: it ensures receive() alone (without then().respond()) still performs validation. ThenSpec.respond() replaces it with a processor that adds a response. The comment in the code documents this correctly.
LGTM.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
gnodet-bot
reviewed
Oct 15, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Following up on my earlier review — 4 of 4 issues have been addressed. The refactoring to extend MockEndpoint is well-executed. Three new concerns emerge from the current architecture, one of which is a correctness bug in performAssertions().
1. performAssertions() adds failures in receive mode — breaks assertIsSatisfied() for handled errors
In performAssertions(), the exception is added to getFailures() unconditionally for both validate and receive modes. In receive mode, the intent is that Camel's onException handles the error — the exception correctly stays on the exchange. But because it also lands in the failures list, any subsequent call to citrus.assertIsSatisfied() will throw AssertionError even when the route's error handler fully managed the failure.
The test receiveOperationTriggersOnException() masks this: it never calls citrus.assertIsSatisfied() after the handled failure. If a user calls MockEndpoint.assertIsSatisfied(context) (the documented idiom for multi-endpoint test teardown), all CitrusEndpoint instances are checked — including ones in receive mode where an error was already handled. The test coverage gap lets the bug go undetected.
Fix: only add to getFailures() in validate mode; let receive-mode errors propagate without polluting the failure list.
2. Spec is mutable after processor registration (documentation gap, async risk)
validate(), validateAll(), and when().receive() create and register the processor immediately, then return the mutable MessageValidationSpec for fluent .body()/.header() chaining. The processor holds a direct reference to the spec. MessageValidationSpec fields (body, bodyBuilder, headers) have no synchronization.
With a DirectComponent consumer this is safe because spec setup happens-before template.send(). With an async consumer (SEDA, timer, or any multi-threaded route), an exchange can arrive and the processor can read spec fields mid-setup. This is at minimum a documentation gap — users of async routes will hit intermittent test failures with no clear cause.
3. Shared nextSpecIndex between validate() and when().receive() — collision risk when mixing explicit and auto-indexed calls
Both validate() and WhenSpec.receive() call nextSpecIndex.incrementAndGet(). Calling citrus.validate(2) explicitly then citrus.validate() auto-increments to 1 — the counter doesn't track which explicit indices are already occupied. Mixing validate(N) and validate() auto-indexed calls can silently overwrite registrations. The documentation doesn't warn about this constraint.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
davsclaus
approved these changes
Oct 15, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Thanks @christophd. I re-read c09d53d:
- The property-to-variable side effect is fixed.
process()now copies the context variables right after the propagation, andwriteBackExtractedVariableswrites back only the keys that are new or changed. So an exchange property (or a global Citrus variable) no longer turns into an exchange variable, while an extracted value that differs from an existing property is still written, asshouldExtractVariableEvenWhenPropertyExistschecks. shouldNotWriteBackPropagatedPropertiesAsVariablescovers it.
Optional, for here or a follow-up: the operation list near the top of the docs could say that a validate failure only shows up through assertIsSatisfied() (the open thread), and the error-handling section could mention that each redelivery counts as a new exchange in the mock counter, so receive(2) applies to the retry of the first exchange.
CI is still running on c09d53d.
Thanks for working through all the rounds on this one.
Claude Code on behalf of @davsclaus.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
gnodet-bot
reviewed
Oct 15, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Following up on my earlier review — all 3 open issues have been addressed.
The new commits (13 commits since the previous review) bring a clean architectural refactoring: CitrusEndpoint now extends MockEndpoint, CitrusProducer became CitrusValidationProcessor (a Processor), and CitrusComponent extends MockComponent. This correctly leverages MockEndpoint's processor dispatch, locking, exchange recording, and assertion infrastructure.
All concerns verified as non-issues:
namefield shadowing:CitrusEndpoint.setName()correctly callssuper.setName(), andMockEndpoint.getName()uses the getter — no desync.performAssertions()flow:super.performAssertions()callstryProcessing()which catches processor exceptions and sets them on the exchange;CitrusEndpoint.performAssertions()then correctly handles them by mode (validatecollects + clears,receiveleaves on exchange foronException).WhenSpec.receive(index)double registration:MockEndpoint.whenExchangeReceived()usesprocessors.put()which replaces — the validation-only processor is atomically overwritten by the validation+response processor whenthen().respond()is called. No race.CitrusContextthread safety:MockEndpoint.onExchange()runs underReentrantLock— only one exchange is processed at a time per endpoint.createTestContext()is never called concurrently.writeBackExtractedVariablesfilter: FreshCitrusContext.createTestContext()starts with onlyGlobalVariables(empty by default) — no pre-populated Citrus internals to filter. Thecitrus./CITRUS_filter is a correct defensive safeguard.
Maven dependency section, docs error-handling section, and firstVersion are all correct.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
davsclaus
requested changes
Oct 15, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Claude Code on behalf of davsclaus
Thanks @christophd. I compared c09d53d..a3b3b02 (1 new commit) and the code changes look good:
- Docs, failure behaviour up front: fixed. The operation list now says where
validateandreceivefailures show up. I resolved my thread on it. - Redelivery note: added. The NOTE explains that a retry counts as a new exchange in the mock counter.
receiveno longer records failures. This matches whatMockEndpointdoes when awhenExchangeReceivedprocessor throws (tryProcessingsets the exception on the exchange and adds nothing to the failures), andreceiveOperationTriggersOnExceptionnow pins it. The changes also answer the three open @gnodet-bot threads (thereceivefailure recording, the async setup note and the index-mixing Javadoc).
CI fails, and the failure is caused by this PR (must fix). Both build (17, false) and build (25, false) fail in mvn -Pregen:
Non-resolvable parent POM for org.apache.camel:camel-test-citrus:4.23.0-SNAPSHOT:
Could not find artifact org.apache.camel:camel-test-parent:pom:4.23.0-SNAPSHOT
main moved to 4.24.0-SNAPSHOT today (#27901), and CI builds the PR merged with main. The new module's pom still says 4.23.0-SNAPSHOT (inline suggestion). Please rebase on main and bump the parent version. Merging as it is would break the main build.
The catalog copy of the docs page is stale (must fix). catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/citrus-component.adoc does not have the three doc changes from a3b3b02 (the two operation-list sentences and the two NOTEs). After the rebase, a build of the module and the catalog regenerates it. CI's uncommitted-changes check fails without it, and the regen step does not get that far today.
Non-blocking: one more sentence for the redelivery NOTE (inline). Now that receive failures are not recorded, a failed first attempt is not reported anywhere if the retry reaches an index with no spec.
I am requesting changes only for the version bump and the catalog copy. Both are mechanical, and I will approve again once CI is green.
Checked against the project conventions in CLAUDE.md. This is a rules-and-conventions review and does not replace CodeRabbit, Sourcery or SonarCloud.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
…ation Introduces a new camel-test-citrus component that integrates with the Citrus integration test framework for message validation in Camel route tests. Validate operation: - Template-based message validation using Citrus validation modules (JSON, XML, YAML, plain text, binary, Hamcrest) - Fluent API: allMessages() for uniform validation, message(index) for per-exchange sequential validation with automatic fallback - Support for Citrus ignore expressions, validation matchers, and test variable resolution via exchange properties - Path validation (JsonPath/XPath) through Citrus validation contexts - Auto-detection of message type from body content Receive operation (request/response simulation): - Gherkin-style fluent API: when().message(n).receive() for expected request validation, then().message(n).respond() for simulated response definition - Response bodies support Citrus test variables and functions - Per-exchange and allMessages() fallback for both request validation and response specs - send() alias for respond() - Response-only mode (no request validation) Message payload builders: - body(MessagePayloadBuilder) overload on both validation and response specs, accepting Citrus payload builders (GroovyScriptPayloadBuilder, marshalling builders, custom implementations) and lambdas Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Validate operation errors: record failures instead of throwing, reported via assertIsSatisfied() (MockEndpoint pattern) - Receive operation: still throws on validation failure - Add operation validation in CitrusComponent (reject invalid ops) - Add volatile to shared mutable fields for thread safety - Reset counters in expectedMessageCount() - Auto-derive expected count from indexed specs - Add CitrusContext lifecycle management (doStop) - Propagate exchange variables (both directions) between Camel/Citrus - Use Locale.ROOT for case conversions - Switch test dependency to camel-test-junit6 - Update firstVersion to 4.24.0 - Add operation enum constraint - Improve documentation: Maven dependency snippet, path validation API usage, missing spec error behavior, exchange variables - Regenerate catalog and endpoint DSL artifacts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…idation Leverage whenExchangeReceived() and whenAnyExchangeReceived() from MockEndpoint to dispatch Citrus validation and response logic via CitrusValidationProcessor. This replaces the previous custom spec resolution and indexing approach. Benefits: - Eliminates custom spec resolution, indexing and state tracking - Reuses MockEndpoint's built-in processor dispatch (indexed and default) for both validation and response handling - Simplifies the Gherkin-style API with a fluent when().receive().body().then().respond().body() chain - Failure reporting uses performAssertions() to capture processor exceptions from the exchange and add them to the failures list; for validate mode the exception is cleared so it does not propagate through the route Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…e build Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Handle StreamCache body in buildReceivedMessage to prevent stream drain on pass-through validate operation - Filter Citrus-internal variables (citrus.*, CITRUS_*) in writeBackExtractedVariables to avoid polluting exchange variables - Fix documentation to use 1-based indexing (matching MockEndpoint) - Rewrite receive operation docs to match actual chained API Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Camel uses CaseInsensitiveMap for exchange headers, so Citrus header validation should ignore case to match Camel's behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix WhenMessageValidationSpec.validate() to return the correct covariant type so .validate(...).then().respond() chains compile - Fix docs: unmatched exchanges pass through silently, not as failures Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Make nextSpecIndex field final with set(0) in reset() instead of replacing the AtomicInteger reference - Allow Citrus-extracted variables to overwrite existing exchange variables so @variable()@ extractions are not silently dropped Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Override @UriPath name with Citrus-specific description to prevent inheriting MockEndpoint's metadata - Inject Supplier<TestContext> into CitrusValidationProcessor so specs configured before doStart() (e.g. isUseAdviceWith tests) work correctly - Indexed specs imply a minimum expected message count when the test has not set one explicitly Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove shadowing `name` field from CitrusEndpoint — MockEndpoint already declares @UriPath name, and the inherited getName()/setName() handle the runtime value. The local field was never written to. - Extract createTestContext() method with null guard to prevent NPE if the supplier is invoked after doStop() nulls citrusContext. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ng tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lidation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
christophd
force-pushed
the
feature/CAMEL-25378-test-citrus
branch
from
October 15, 2026 17:57
a3b3b02 to
ac01829
Compare
gnodet-bot
reviewed
Oct 15, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Following up on my earlier review — all prior findings are resolved.
✅ Issues from my 08:55 review (addressed before this commit):
- Dead
namefield —getName()/setName()overrides now correctly delegate to bothCitrusEndpoint's own field andsuper.setName(), keeping both fields in sync. The catalogrequired: falseonnameis correct. - Lambda NPE after
doStop()—createTestContext()now capturesCitrusContext ctx = citrusContextas a local before using it, throwsIllegalStateException(not NPE) when the endpoint is stopped. Race-safe. - Double registration in
receive(index).then().respond()— acknowledged by design (first registration ensuresreceive()without.respond()still validates; the overwrite is intentional and documented in the comment). The wasted allocation is minor and acceptable.
✅ davsclaus's "must fix" issues (addressed in this commit ac018291):
- Parent POM version — bumped from
4.23.0-SNAPSHOT→4.24.0-SNAPSHOTincamel-test-citrus/pom.xmland all generated artifacts (citrus.json,component.properties). CImvn -Pregenfailure resolved. - Stale catalog copy —
catalog/camel-catalog/src/generated/resources/.../citrus-component.adocnow matches the source doc with the two operation-list sentences and both NOTEs. Uncommitted-changes CI check will pass. - Redelivery NOTE extended — the additional sentence ("If no spec exists for the retry, it passes without validation and the earlier failure is not recorded, so set
expectedMessageCount(...)to catch the extra exchange") is present in both the source and catalog copies.
CI is still running on this commit. No new issues introduced.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
davsclaus
approved these changes
Oct 16, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Claude Code on behalf of davsclaus
Thanks @christophd. I compared a3b3b02..ac01829 (rebased on main, plus one new commit). The parent and BOM now use 4.24.0-SNAPSHOT, the catalog copy of the docs page matches the source page again, and the redelivery NOTE has the extra sentence. CI is green on ac01829, including the regen check and the doc validation. The remaining @gnodet-bot threads are addressed by the earlier commits, so all my points are settled.
This review does not replace specialized AI review tools or static analysis such as SonarCloud.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a new
camel-test-citruscomponent that integrates the Citrus integration test framework for message validation in Camel route tests.Architecture
CitrusEndpointextendsMockEndpoint, inheriting all standard mock features:assertIsSatisfied()/resetMocks()/expectedMessageCount()work out of the boxMockEndpoint.assertIsSatisfied(context)picks up Citrus endpoints transparentlywhenExchangeReceived()/whenAnyExchangeReceived()Validation is performed by
CitrusValidationProcessor(aProcessor, not aProducer), which:TestContextlazily viaSupplier<TestContext>forisUseAdviceWith()compatibilityCaseInsensitiveMapFeatures
validateoperation — pass-through validation (body, headers, JsonPath, XPath); exceptions captured, not thrownreceiveoperation — request/response simulation with Gherkin-style API (when().receive().body().then().respond().body()); exceptions propagate toonExceptionvalidate(index)for sequential validation with auto-incrementing indexvalidateAll()/receiveAll()as default for unmatched exchangesexpectedMinimumMessageCount@ignore@,@greaterThan()@,@variable()@, etc.@variable('name')@writes back to exchange variablesTest coverage
CitrusValidateJsonTest— JSON body/header validation, ignore expressions, validation matchersCitrusValidateXmlTest— XML body validation with namespace handlingCitrusSequentialValidationTest— per-exchange specs, fallback, implied minimum count failureCitrusReceiveResponseTest— request/response simulation with Gherkin APICitrusReceiveErrorHandlingTest—validateclears exception (noonException),receivekeeps it (onExceptionfires)CitrusVariableTest— exchange variable propagation, extraction via@variable()@, overwrite semanticsCitrusValidateStreamCacheTest,CitrusValidatePlainTextTest,CitrusValidateYamlTestReview feedback addressed
namefield restored with@UriPathand own description;getName()/setName()overrides keep it in sync withMockEndpointcreateTestContext()supplier to prevent NPE afterdoStop()firstVersion = "4.24.0"with all generated artifacts matchingHeaderValidationContext.Builder().ignoreCase(true)validate()returns covariantWhenMessageValidationSpecfor method chainingnextSpecIndexisfinal AtomicIntegerwith null guard inreset()citrus./CITRUS_prefixesperformAssertions()captures processor exceptions into failures listapplyImpliedMinimumCount(index)with test and documentationreceive(int)documented with code commentStill open (minor, can be follow-up)
String—byte[]may be safer for binary/gzip message typesCitrusContextownership (component-level vs endpoint-level) — tracked as follow-up🤖 Generated with Claude Code