CAMEL-25138: Add Java and XML declarations in camel-semantic - #27082
luigidemasi merged 11 commits into
Conversation
Register fluent Java and native XML declarations in the shared semantic question registry before routes initialize. Preserve validation, source ownership, reload behavior, and the existing YAML and provider contracts. Preserve declarations in Java/YAML model exports, generate XML schemas and catalog metadata, and document both declaration forms. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Luigi De Masi <ldemasi@redhat.com>
Order semantic declarations before routes in the generated schemas and XML writer, with schema validation coverage for routes and camel documents. Use the supplied source consistently for question ownership, document the reserved model: prefix, and remove redundant XML declaration assignments. Document Java builder ordering and the runtime route dump limitation: direct model exports preserve declarations, but runtime dumps omit them. Explain the handwritten semantic serializers in the generator templates. XML resources are parsed during preparse so shared questions exist before route initialization. The loader's pending-cache invalidation is retained because a failed batch prevents earlier builders from clearing their input; retrying must read corrected resources. Validation: 928 focused tests passed, 2 skipped; full repository clean install with tests skipped; generated XML IO and Spring schema validation; formatter validation and import-order checks. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Luigi De Masi <ldemasi@redhat.com>
|
🌟 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
components-ai
labels
gnodet-bot
left a comment
gnodet-bot
left a comment
There was a problem hiding this comment.
Well-structured feature PR with comprehensive test coverage (382-line dedicated test class, 11 parameterized invalid-input scenarios, Java+XML+YAML roundtrip, batch-failure recovery, reload/rename/delete lifecycle). A few observations for consideration:
This review was generated by an AI agent, Hermès on behalf of @gnodet.
apupier
left a comment
apupier
left a comment
There was a problem hiding this comment.
[ERROR] Failures:
[ERROR] org.apache.camel.catalog.DocExamplesXmlSchemaTest.everyXmlExampleOfTheDocumentationValidates
[ERROR] Run 1: DocExamplesXmlSchemaTest.everyXmlExampleOfTheDocumentationValidates:145 Documentation XML examples that do not validate:
semantic-language.adoc:144 example 1: cvc-elt.1.a: Cannot find the declaration of element 'camel'. ==> expected: <true> but was: <false>
[ERROR] Run 2: DocExamplesXmlSchemaTest.everyXmlExampleOfTheDocumentationValidates:145 Documentation XML examples that do not validate:
semantic-language.adoc:144 example 1: cvc-elt.1.a: Cannot find the declaration of element 'camel'. ==> expected: <true> but was: <false>
[ERROR] Run 3: DocExamplesXmlSchemaTest.everyXmlExampleOfTheDocumentationValidates:145 Documentation XML examples that do not validate:
semantic-language.adoc:144 example 1: cvc-elt.1.a: Cannot find the declaration of element 'camel'. ==> expected: <true> but was: <false>
Use the XML IO namespace for the native XML declaration example so the catalog documentation schema check validates it, and regenerate its mirror. Identify threshold and uncertainty in non-numeric validation errors while preserving the question context and original cause. Cover both fields in the existing invalid-reload tests, including preservation of the previous definitions and recovery after corrected input. Clarify why duplicate discovery of the stateless default configurer is harmless. Validation: 109 semantic tests and 1082 catalog tests passed, including the documentation schema checks. Full 695-module clean install passed with tests skipped. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Luigi De Masi <ldemasi@redhat.com>
|
@apupier Fixed the namespace in b6a0e7e: the XML IO example now declares http://camel.apache.org/schema/xml-io, and the catalog documentation mirror has been regenerated. All 1,082 catalog tests pass locally, including all four DocExamplesXmlSchemaTest checks; the full 695-module clean install also passes with tests skipped. This addresses your requested change. Codex on behalf of luigidemasi. |
gnodet-bot
left a comment
gnodet-bot
left a comment
There was a problem hiding this comment.
Re-review of b6a0e7e — all previous findings addressed.
apupier's CHANGES_REQUESTED (DocExamplesXmlSchemaTest failure): Addressed — the XML documentation example namespace was corrected from camel-spring to camel-xml-io. The <camel> root element is declared in the xml-io schema, so the catalog XSD validation test should now pass.
gnodet-bot threshold parsing (DefaultSemanticDefinitionConfigurer.java): Addressed — parseDouble(String, String) helper added exactly as suggested, wrapping NumberFormatException into a clean IllegalArgumentException with field name and value. Two new parameterized test cases cover threshold="abc" and uncertainty="abc".
gnodet-bot TOCTOU on configurer discovery (SemanticDefinition.java): Addressed — clarifying comment explains that concurrent discovery creates equivalent stateless configurer instances, while the semantic module synchronizes access to shared question state.
No new issues in the follow-up commit.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 10 of 697 tested, 2 compile-only — current: 10 all testedMaveniverse Scalpel detected 10 affected modules (current approach: 10). Skip-tests mode would test 10 modules (5 direct + 0 downstream), skip tests for 2 (generated code, meta-modules) Modules Scalpel would test (10)
Modules with tests skipped (2)
All tested modules (37 modules, 5m 35s total)Total reactor time: 5m 35s
Top 20 slowest modules:
|
|
slightly different test error: |
Read the generated schema's target namespace in the declaration-order test. A clean test build produces the Spring namespace, while an incremental build after packaging can retain the XML IO namespace. The hard-coded Spring namespace therefore failed in CI's Java 17 test phase. Accept only the two expected namespaces and keep schema validation for semantic declarations before routes under both routes and camel roots. Validation: reproduced the two CI failures on Java 17 before the fix. The XML IO suite passes on Java 17 with clean and packaged schemas and on Java 25 (425 tests per run, including two existing skips). The full 695-module clean install passes with tests skipped. All 11 CI-selected modules also pass on Java 17: 2928 tests, no failures or errors, and two existing skips. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Luigi De Masi <ldemasi@redhat.com>
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Thanks @luigidemasi, bringing Java and XML to parity with the YAML semantic block is nicely done, and the generated artifacts (xml-io parser/writer, XSDs, model JSON, catalog mirror) are all committed. Some findings:
- JAXB XML DSL drops
<semantic>:JaxbXmlRoutesBuilderLoader(camel-xml-jaxb-dsl, not touched by this PR) only copiesroutes.getRoutes(). Now that the model and schema accept<routes><semantic>, JAXB parses it intoRoutesDefinition.semantic, but it is silently ignored and the routes then fail with "Unknown semantic question". Either addgetRouteCollection().setSemantic(routes.getSemantic())there, or document that only xml-io supports it. - Catalog metadata for the numeric options (inline).
- Stale preparse cache in the xml-io loader (inline).
- Java export crashes on placeholders (inline).
- Design question: this puts an AI-specific concept into the core model (
RoutesDefinition/BeansDefinition) andRouteBuilder.semanticQuestions(). There is precedent (tokenizer()), so I'm not against it, but two things would make it sit better in core:SemanticDefinitiondoes a FactoryFinder/context-plugin lookup in a staticconfigure(), and I'd rather keep model classes data-only and move that into a helper; andSemanticDefinitionConfigureris an SPI placed inmodel.apprather than anspipackage. A@since 4.23on the newRouteBuildermethod would also be good. - Minor: the MCP
TransformToolskeeps declarations for XML→YAML but drops them for YAML→XML and Java→YAML/XML. - Nit: in
RoutesDefinitionthe newsemanticgetter/setter sit between the fields and the constructor; please move them next to the other accessors.
Also, @apupier's CHANGES_REQUESTED (the DocExamplesXmlSchemaTest namespace failure) looks addressed by b6a0e7e, so a re-review from him would unblock that.
Claude Code on behalf of davsclaus
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
…port Keep semantic declarations when loading JAXB routes and converting YAML or Java routes through MCP. Export registered question policies and prepare Java expression models without starting routes or invoking providers. Refresh XML preparse caches when resource content changes after a failed batch, while preserving unchanged preparse and deferred bean ownership. Add regression coverage for foreign-loader and earlier-builder failures. Preserve numeric placeholders in Java exports and resolve them before runtime validation. Add numeric catalog types and defaults, move optional component discovery into a helper, relocate the configurer SPI, and keep RoutesDefinition accessors together. Validation: 2978 tests across the affected Java 17 module suites, with no failures or errors and two existing skips. Full repository clean install with tests skipped passes on Java 25. Regenerated artifacts are included. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Luigi De Masi <ldemasi@redhat.com>
|
@davsclaus, the remaining points from your review are fixed in 02f4ad4b4f46:
Generated by Codex via /oss-address-review on behalf of luigidemasi. |
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Thanks @luigidemasi, all points from my previous review are addressed, with good regression coverage (JAXB retention, content-aware preparse cache incl. <camel> roots and deferred beans, placeholder-preserving Java export, MCP round-trips). The model.spi move and SemanticDefinitionHelper make the core side sit much better.
A few optional follow-ups (inline), none blocking.
@apupier your CHANGES_REQUESTED (schema failures) appears addressed by 987488d and CI is green; could you re-review?
Claude Code on behalf of davsclaus
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
|
||
| RoutesDefinition rd = new RoutesDefinition(); | ||
| rd.setRoutes(routeDefs); | ||
| rd.setSemantic(DefaultSemanticDefinitionConfigurer.getDefinition(ctx)); |
There was a problem hiding this comment.
Nit: this ties the MCP module to the camel-semantic implementation class through a public static helper. Adding an export method to org.apache.camel.model.spi.SemanticDefinitionConfigurer (looked up as a context plugin like SemanticDefinitionHelper does) would keep MCP on the SPI only. Also note the conversion writes resolved values and explicit defaults, and a placeholder with no default would probably fail to resolve in this throwaway context.
There was a problem hiding this comment.
Fixed in 0bb166a78dfe. Export now goes through SemanticDefinitionConfigurer, with context-plugin lookup and lazy discovery in SemanticDefinitionHelper; this also covers YAML-only registrations. MCP no longer references the implementation class.
The placeholder observation is correct: context-based conversion needs a resolved numeric value. The documentation now states that requirement, and tests cover both defaulted placeholders and missing required properties. Conversion errors now include the underlying cause so YAML reports the missing key instead of only a preparse failure.
Generated by Codex via /oss-address-review on behalf of luigidemasi.
There was a problem hiding this comment.
The architecture changed in 4e03971ee3ec to keep declarations entirely inside camel-semantic; the core SPI and model exporter are removed. MCP now checks the public SemanticQuestions registry and rejects conversions containing declarations, explaining that declarations must be kept separately. This avoids silently dropping definitions without adding a semantic SPI to core. Numeric placeholder errors remain covered.
Generated by Codex via /oss-address-review on behalf of luigidemasi.
|
|
||
| // Java expression clauses are normally materialized when processors are created. | ||
| routeDefs.forEach(route -> ProcessorDefinitionHelper.filterTypeInOutputs(route.getOutputs(), ExpressionNode.class) | ||
| .forEach(ExpressionNode::preCreateProcessor)); |
There was a problem hiding this comment.
This pre-creates expression clauses for every converted Java route, not only semantic ones. Looks like a general fix for Java→YAML/XML. Could you add a small test with a non-semantic expression clause (e.g. .filter().simple(...)) so it's covered?
There was a problem hiding this comment.
Added in 0bb166a78dfe. The parameterized regression converts a non-semantic .filter().simple(...) with a nested .setBody().simple(...) to both YAML and XML, reloads each output, and verifies that both Simple expressions are preserved.
Generated by Codex via /oss-address-review on behalf of luigidemasi.
Export registered questions through the model configurer SPI, including lazy discovery for YAML-only declarations, and omit default boolean decision policies from converted output. Cover Java filter and nested Simple expression conversion to YAML/XML, default-policy round trips, and numeric placeholders. Include the underlying error when conversion fails and document temporary-context property resolution. Validation: 549 tests passed on Java 17 across core-model, semantic and MCP. Full repository clean install with tests skipped passed on Java 21 across all 695 modules. Regenerated catalog documentation is included. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Luigi De Masi <ldemasi@redhat.com>
Use a fluent helper in ordinary Java RouteBuilder classes and an optional semantic.xml loader for standalone declarations or declarations alongside routes. Reuse existing builder lifecycle and XML parsing hooks, removing the semantic model, SPI, writers and schema changes from core. Preserve atomic validation and source ownership during reload. Reject lossy generic conversions and document the declaration/export boundary. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Luigi De Masi <ldemasi@redhat.com>
luigidemasi
changed the title
gnodet-bot
left a comment
gnodet-bot
left a comment
There was a problem hiding this comment.
Re-review of 4e03971 — the architectural pivot to keep declarations entirely in camel-semantic is clean.
Previous findings status:
-
apupier's CHANGES_REQUESTED (DocExamplesXmlSchemaTest namespace failure): Addressed since b6a0e7e. The semantic-language documentation examples are now correctly skipped in
DocExamplesXmlSchemaTestsince they use a component-owned loader outside the core schemas. -
apupier's follow-up (ModelWriterTest.semanticDeclarationsBeforeRoutesConformToTheSchema failure): Addressed — the test no longer exists because 4e03971 removes the semantic model from
camel-core-modelentirely. No semantic references remain inModelWriterTest. -
davsclaus review (7 findings): All addressed. The major design concern (#5 — AI concept in core model) is fully resolved by moving declarations, the configurer SPI, and the XML loader into
camel-semantic. MCPTransformToolsnow rejects inputs with declarations instead of silently losing them. -
gnodet-bot findings (threshold parsing, TOCTOU, cache clearing): All previously addressed; the refactored code in
SemanticQuestionBuilder.parseDouble()retains the field-specific diagnostics.
New code review:
-
SemanticXmlRoutesBuilderLoader: XXE protection is complete (disallow-doctype-decl,FEATURE_SECURE_PROCESSING, emptyACCESS_EXTERNAL_*on bothDocumentBuilderFactoryandTransformerFactory). Test confirms DTD rejection. Namespace validation covers empty, semantic, xml-io, and spring namespaces. Error paths are clean — failed parsing does not replace previous definitions. -
SemanticQuestionsBuilder/DeclarationsLifecycle: TheWeakHashMap-backedregisteredset is accessed only undersynchronized(questions)(sameSemanticQuestionsmonitor in bothregister()andafterConfigure()). Thread-safe. -
SemanticQuestions.isEmpty(): Not synchronized, but only called fromTransformTools.requireSeparateDeclarations()on a throwaway per-call context with no concurrent access. Fine. -
Test coverage: 488-line
SemanticDeclarationDslTestcovers Java/XML/namespace variants, reload/rename/delete lifecycle, failed batch recovery, cross-resource ordering, placeholder resolution, and 13 parameterized invalid-input scenarios. Solid.
No new issues.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
Automatically install XML declaration support through the existing context service plugin. Detect declarations or their namespace, delegate ordinary XML to the standard loader, and retain application loader precedence. Preserve resource ownership and manage loader startup, shutdown and restart. Use ordinary XML filenames throughout the documentation and cover both layouts, Camel Main, custom registries, namespace handling and reloads. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Luigi De Masi <ldemasi@redhat.com>
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Moving this out of core is a good change, thanks. I re-reviewed the new design:
- Routes in
*.semantic.xmllose their source location and get wrong line numbers.SemanticXmlRoutesBuilderLoader.parse()removes the<semantic>element from the DOM, re-serialises the document and parses it withnew ModelParser(new StringReader(xml), namespace). With the Reader constructor the resource is null, so no location is set, and line numbers point into the re-serialised text, shifted by the removed block. This breaks the debugger, the dev console route source andXmlPullParserLocationExceptionmessages whensourceLocationEnabledis on. Suggestion: parse the originalResourcewith aModelParsersubclass whoseignoreUnexpectedElementskipssemanticunderroutes. That also removes the DOM → Transformer round trip. DefaultRoutesLoadertakes everything after the first dot as the extension, somy.tickets.semantic.xmlfinds no loader. Please document that the base name must not contain dots (or handle it).camel-jbang-mcpgets a new compile dependency oncamel-semanticonly forSemanticQuestions.isEmpty()inrequireSeparateDeclarations. Is the clearer error worth the dependency?
Claude Code on behalf of davsclaus
gnodet-bot
left a comment
gnodet-bot
left a comment
There was a problem hiding this comment.
Re-review of 438d870 — automatic XML discovery is a clean follow-up.
davsclaus's 3 findings status:
-
Source location/line numbers (Transformer re-serialization in
SemanticXmlRoutesBuilderLoader.parse()): Still present when routes sit alongside<semantic>blocks —new ModelParser(new StringReader(xml.toString()), namespace)loses the resource URI. However, the new auto-discovery wrapper (SemanticXmlLoader) routes ordinary XML files without declarations through the standard loader, preserving full location tracking for the common case. Only files with explicit<semantic>blocks lose source location — a narrowed scope compared to the previous*.semantic.xml-only design. -
Dot-in-filename (
my.tickets.semantic.xmlfails withDefaultRoutesLoader): Effectively addressed — users now use ordinary*.xmlfiles. The*.semantic.xmlextension remains as a registered fallback via@RoutesLoader("semantic.xml")but is no longer the documented path. -
MCP dependency on camel-semantic: Still present —
camel-jbang-mcpdepends oncamel-semanticforSemanticQuestions.isEmpty()inrequireSeparateDeclarations(). This is a conscious design choice: MCP needs to detect and reject lossy conversions with declarations.
New code review:
-
SemanticXmlLoader: Clean wrapper pattern.hasDeclarations()uses StAX for lightweight scanning with proper XXE protection (SUPPORT_DTD=false,IS_SUPPORTING_EXTERNAL_ENTITIES=false). Stream is in try-with-resources, reader is closed in finally.CachedResourcesnapshot shares bytes within a single call without persistent caching. -
isSupportedExtension(): Correctly yields to application-registered XML loaders by checking the registry. Thethisidentity check avoids self-matching. -
delegate(): Synchronized lazy init resolves the real XML loader viaBootstrapFactoryFinderto avoid circular lookup throughRoutesLoader. Delegate lifecycle is managed indoStop(). -
SemanticReloadPlugin.installXmlLoader(): Defensive installation — skips whenModelParserclass is absent, when the loader is already registered, or when another XML-capable loader exists. TheonContextInitializinglifecycle callback handles applications that replace the registry after the eager build phase. -
afterConfigureinterceptor on delegated XML builders: Correctly removes declarations from the semantic registry when a file that previously had declarations is reloaded without them. Keyed byresource.getLocation()(original, not snapshot). -
Test coverage:
SemanticXmlAutoDiscoveryTest(181 lines) covers Camel Main integration, delegate beans/route-configuration/lifecycle, custom loader precedence (both naming conventions), missing XML support graceful fallback, application registry contexts, and context restart/reinstall. ExistingSemanticDeclarationDslTestupdated to use ordinary.xmlfilenames with asemantic-extensionvariant preserving backward-compat coverage.
No new issues.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
Parse routes from their original resource bytes while skipping the validated semantic declaration subtree. Accept dotted XML filenames through the component loader and retain custom loader precedence. Discover the optional semantic registry at runtime before generic MCP exports, preserving rejection of lossy conversions without a production dependency on camel-semantic. Cover original locations and diagnostics, dotted filenames, and empty registries; document the XML filename support. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Luigi De Masi <ldemasi@redhat.com>
|
Addressed your review in
Validation: 567 Java 17 module tests, 11 checks without the semantic dependency, and the full 695-module build with tests skipped passed. Generated by Codex via /oss-address-review on behalf of luigidemasi. |
gnodet-bot
left a comment
gnodet-bot
left a comment
There was a problem hiding this comment.
Re-review of dc1a33c — all three davsclaus findings addressed.
davsclaus finding #1 (Source location/line numbers): Addressed — the Transformer re-serialization is completely removed. SemanticModelParser extends ModelParser, sets this.resource = resource in the constructor (preserving the resource URI), and overrides handleUnexpectedElement to skip <semantic> blocks at depth 2 via parser.skipSubTree(). Routes are parsed from the original bytes with correct line numbers. declarationsPreserveOriginalRouteSourceLocations verifies route line 8, input line 9, filter line 10, log line 12 across four filename variants including dotted names. nestedSemanticElementIsRejectedWithOriginalSourceLocation confirms that a misplaced <semantic/> inside a filter produces a XmlPullParserLocationException with the correct resource name and line number.
davsclaus finding #2 (Dot-in-filename): Addressed — isSupportedExtension now uses extension.endsWith(".xml") instead of "camel.xml".equals(extension). Application loaders for specific extensions retain precedence via the registry check. applicationLoaderForDottedExtensionTakesPrecedence verifies a custom "tickets.xml" loader is resolved ahead of the wrapper. Parameterized tests cover my.tickets.xml and my.tickets.semantic.xml.
davsclaus finding #3 (MCP compile dependency): Addressed — camel-semantic moved from compile to test scope in camel-jbang-mcp. The requireSeparateDeclarations guard now uses context.getClassResolver().resolveClass(...) + reflection to discover the optional registry at runtime. When the class is absent, the guard returns cleanly. emptySemanticRegistryDoesNotPreventConversion covers the empty-registry path.
No new issues.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
|
Follow-up: my previous review was written against 4e03971 and crossed with 438d870 / dc1a33c, which already address all three points. Thanks:
Remaining small points on the new design:
Claude Code on behalf of davsclaus |
Limit declaration detection to top-level elements, skip route subtrees and stop at the closing routes tag while allowing declarations after routes. Reuse lazily discovered application loaders and refresh discovery at startup, reload and registry replacement instead of scanning on each check. Remove the migration-guide note for the new component and retain its component documentation. Cover discovery reuse, refresh and late XML declarations with regression tests. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Luigi De Masi <ldemasi@redhat.com>
|
Addressed the three remaining points in your follow-up in
Validation: 611 Java 17 tests passed, followed by the full 695-module Generated by Codex via /oss-address-review on behalf of luigidemasi. |
gnodet-bot
left a comment
gnodet-bot
left a comment
There was a problem hiding this comment.
Re-review of a52e1b0 — all 3 davsclaus follow-up points addressed.
davsclaus finding #1 (upgrade guide): Addressed — the camel-semantic - automatic XML route loading section is removed from camel-4x-upgrade-guide-4_23.adoc. Feature is new in 4.23, no migration needed.
davsclaus finding #2 (StAX probe scans full document): Addressed — hasDeclarations() now skips non-declaration subtrees via skipSubtree() and returns false at the closing </routes> tag (--depth == 0). Only direct children of <routes> are inspected by name; nested route content is consumed structurally without checking element names or namespaces. nestedRoutesDoNotHideFollowingDeclarations test confirms that <semantic/> inside CDATA is not falsely detected, while a real <semantic> block after routes is still found.
davsclaus finding #3 (per-call registry lookup in isSupportedExtension): Addressed — otherLoaders() caches discovered loaders keyed by registry identity, reused across repeated isSupportedExtension calls. Cache is invalidated on context initialization (onContextInitializing), route reload (onReload), and stop (doStop) via resetLoaderDiscovery(). SemanticReloadPlugin.xmlLoader field narrowed to SemanticXmlLoader for the resetLoaderDiscovery() call. loaderDiscoveryIsReusedAndRefreshedOnReload test asserts exactly 1 registry lookup across 5 extension checks, refresh to 2 after custom loader registration, and back to 3 after removal.
No new issues in the follow-up commit.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
The service restart changes on main track services in a ConcurrentHashMap. The downloader test proxy returned null for hashCode, so all nine properties function tests failed during service registration before their assertions. Give the recording proxy identity-based hashCode and equals implementations and a useful toString without changing the downloader assertions. Failed run: https://github.com/apache/camel/actions/runs/36707716997 Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Luigi De Masi <ldemasi@redhat.com>
gnodet-bot
left a comment
gnodet-bot
left a comment
There was a problem hiding this comment.
Re-review of f57b10f — test proxy identity fix.
The recording DependencyDownloader proxy lacked hashCode/equals/toString, which broke when CAMEL-25070 service-restart changes on main use the proxy as a map key. The fix is textbook-correct: identity-based hashCode via System.identityHashCode, reference equality for equals, and a descriptive toString. Test-only, no production code touched.
All previous findings (davsclaus: upgrade guide, StAX probe scope, loader discovery caching; gnodet-bot: threshold parsing, TOCTOU, XXE protection) remain addressed from prior commits.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
4 participants
Adds named semantic question declarations through extensions supplied by
camel-semantic, with no changes to the Camel core model, SPI, standard XML loaders, model writers or core XSDs.Java uses a static fluent helper inside an ordinary
RouteBuilder:Import
org.apache.camel.semantic.SemanticQuestionsBuilder.semanticQuestions. Multiple questions form one group through.end().question(name);.register()validates the complete group before replacing its source's definitions.XML support is discovered automatically when
camel-semanticand the XML DSL are present. Ordinary*.xmlfiles (including the*.camel.xmlalias) can contain either a standalone<semantic>document or<routes><semantic>...</semantic><route>...</route></routes>. The component recognizes declaration blocks or the semantic namespace. A declaration block can use its own semantic namespace inside standard Camel routes. No application loader registration is required. Dotted filenames such asmy.tickets.xmlandmy.tickets.semantic.xmlare supported. Route parsing uses the original resource bytes and preserves source locations and line numbers, including parser diagnostics.The component wrapper delegates XML without declarations to the standard XML loader, retaining its beans and route configuration support. Custom application loaders registered before route resource discovery retain precedence. The wrapper lazily discovers other loaders once per registry and refreshes that snapshot at startup and reload, avoiding a registry scan for every extension check. The wrapper preserves resource ownership for reload and deletion tracking, manages its delegate lifecycle, and handles context restarts and application registries. Detection shares a resource snapshot only for the current call; it adds no persistent preparse cache. The probe skips non-declaration subtrees and stops at the end of the routes root, while still detecting declarations placed after routes.
Java, XML and existing YAML declarations share the context-wide registry, adapter contract, defaults and
ref:/refs:evaluation. Coverage includes mixed batches, state selection, policies, duplicate and malformed declarations, numeric placeholders, cross-resource loading, replacement/removal, and deleted or renamed resources. Loading declarations performs no inference. The Java/XML dependencies are optional; existing language and YAML use does not require the new XML extension.Declarations remain outside the core route model. Generic route dumps omit them, so applications must keep and load declarations separately. MCP conversion rejects Java/YAML inputs containing declarations instead of silently losing them. Its guard discovers the optional semantic registry at runtime;
camel-semanticis a test dependency only, so ordinary conversions do not require the component. A failure to inspect an available registry propagates as a conversion failure. Extended XML also needs to be separated before generic conversion. The XML extension does not validate against the standard core XSDs; its syntax is validated by its loader. Automatic discovery applies to Camel route resources, not Spring XML application-context parsing or direct JAXB unmarshalling. Component documentation uses ordinary XML filenames throughout. There is no migration-guide entry because camel-semantic is new in 4.23.The failed CI run also exposed a pre-existing downloader test double defect when combined with the service-restart changes on
main(CAMEL-25070, #27080). The recording proxy now implements identity-basedhashCodeandequals, plustoString, so service registration can use it as a map key. All existing downloader assertions are unchanged; this follow-up changes test code only.Validation:
clean install -DskipTests: all 695 modules passed.core/verified.https://issues.apache.org/jira/browse/CAMEL-25138
Generated by Codex via /oss-address-review and /oss-fix-ci-errors on behalf of luigidemasi.