CAMEL-24117: camel-rest-openapi - Create per-endpoint strategy instance#24783
Conversation
…ce to fix shared state cross-talk 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.
LGTM — correct fix for shared strategy cross-talk.
Verification:
DefaultRestOpenapiProcessorStrategyhas per-consumer mutable state:urislist (line 77, accumulates HTTP endpoint registrations),missingOperation(line 414), andmockIncludePattern(line 424). Sharing one instance across multiplerest().openApi()consumers causes last-writer-wins on configuration and URI mixing.- The fix correctly distinguishes user-supplied (
restOpenapiProcessorStrategy != null— field access, not getter) from auto-created strategies. User-supplied strategies are shared as intended; default strategies are created per-endpoint. - Removing the auto-creation from
doInit()is safe — the field stays null unless explicitly set by the user, and each endpoint gets its own fresh instance increateEndpoint(). - Compatible with #24782 (lifecycle fix): the component's
doStop()handles null viaServiceHelper.stopService(), and per-endpoint strategies are stopped by each endpoint's owndoStop().
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: 18 tested, 29 compile-only — current: 18 all testedMaveniverse Scalpel detected 47 affected modules (current approach: 18).
|
oscerd
left a comment
There was a problem hiding this comment.
The fix is correct — DefaultRestOpenapiProcessorStrategy holds per-consumer mutable state (uris, missingOperation, producerCache), and on main a single auto-created instance was shared across every endpoint; since ServiceHelper.initService is idempotent, a second consumer's re-init was skipped and uris accumulated (last-writer-wins cross-talk). Creating a fresh instance per endpoint (sharing only an explicitly user-supplied one) is the right fix, and each per-endpoint strategy is init/started/stopped by its owning processor.
One thing before merge: the multi-consumer cross-talk this fixes ships with no regression test — two rest().openApi() consumers asserting their URIs/config don't bleed into each other is exactly the scenario that silently regressed, and it's straightforward to unit-test. Also note the per-endpoint strategies only actually get stopped once #24782 lands (this PR adds no stop path on its own), so the two are best merged together.
Reviewed with Claude Code on behalf of Andrea Cosentino. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
Summary
DefaultRestOpenapiProcessorStrategyinstance causing cross-talk between multiplerest().openApi()consumersmissingOperation/mockIncludePatternoverwrite, silent fail-fast loss, andurislist mixing across consumersRoot cause
RestOpenApiComponent.doInit()created ONEDefaultRestOpenapiProcessorStrategyandcreateEndpoint()shared it across all endpoints. Each consumer'safterPropertiesConfigured()then calledsetMissingOperation()/setMockIncludePattern()on the shared object — last writer wins. Combined withServiceHelper.initServiceidempotency, the second consumer's nullmissingOperationwas never resolved, causingvalidateOpenApi()to silently skip all fail/ignore/mock branches.Changes
RestOpenApiComponent.createEndpoint(): Create a freshDefaultRestOpenapiProcessorStrategyper endpoint when no user-supplied custom strategy exists. Share only an explicitly user-supplied strategy.RestOpenApiComponent.doInit(): Remove auto-creation of shared default strategy (no longer needed).Claude Code on behalf of davsclaus
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com