Conversation
…ap keyed by name
Task 1 of CAMEL-24704: the lenient camelYamlDsl.json schema now accepts
beans: written as a map from the bean name to the rest of the bean
definition (beans: {myBean: {type: ...}}), as an alternative to the
canonical list form (- name: myBean / type: ...). This applies to the
top-level beans, and to beans inside routeTemplate and templatedRoute.
The canonical schema and the runtime writer are unaffected: the list
stays the only canonical form.
- Add YamlProperty.mapKey(): when set on an array: property, the mojo
emits a oneOf accepting either the array or a map keyed by that
property, referencing a generated "<Type>By<MapKey>" definition (the
item's own definition without the key property and without it in
required).
- Mark the three beans YamlProperty declarations (BeansDeserializer,
RouteTemplateDefinitionDeserializer, TemplatedRouteDefinitionDeserializer)
with mapKey = "name".
- Regenerate camelYamlDsl.json; camelYamlDsl-canonical.json and
camelYamlDsl-model.json are unchanged, confirming the canonical form
is untouched.
- Add validator tests: beans as a map passes the classic (lenient)
validator but not the canonical one, and a name: inside a map-form
bean is rejected (the key is the name). Extend the existing map/list
hint test with a canonical-mode assertion and a lenient scalar
assertion.
_Claude Code on behalf of Adriano Machado (@ammachado)_
_This was generated by an AI agent and may contain inaccuracies.
Please verify before relying on it._
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…e loaded BeansDeserializer.asBeanDefinitions(Node) now accepts the beans: node as either the canonical list (each item carrying its own name:) or a map keyed by the bean name (CAMEL-24704). For the map form, each entry is rewritten internally as a "- name: <key>" list item followed by its properties before delegating to the existing BeanFactoryDefinition deserialization, so resource assignment, notNull/script checks, the #class: prefix and the pre-parse/parse dedup cache are unchanged for both forms. A bean entry under name: with its own name: property, a non-map value (missing properties indented under the name), or a name declared twice in the map now fail with a message that names the offending bean and says what to write. RouteTemplateDefinitionDeserializer and TemplatedRouteDefinitionDeserializer now delegate to the same helper for their beans: property so routeTemplate and templatedRoute accept the map form too. _Claude Code on behalf of Adriano Machado (@ammachado)_ _This was generated by an AI agent and may contain inaccuracies. Please verify before relying on it._ Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
… a map BeanRefChecks.declaredBeans scanned only the canonical `- name: x` list form under `beans:`, so a bean written as a map keyed by its name (supported by the runtime and schema since earlier CAMEL-24704 commits) was reported as an undeclared reference. The line scanner now also recognizes the first indent level under a beans: block as a map key (the bean name) when the line is not a list item, ignoring comment lines so they neither set the child indent nor are read as names. The canonical list form is unaffected. _Claude Code on behalf of Adriano Machado (@ammachado)_ _This was generated by an AI agent and may contain inaccuracies. Please verify before relying on it._ Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
declaredBeanTypes bound each type: line to the last `- name: x` it saw, with no awareness of a beans: block's boundaries or of the map-keyed form added earlier in CAMEL-24704: a map-form bean's type was silently attributed to an earlier, unrelated list-form bean name (or the last name from a previous beans: block), so the interface check for options like aggregationStrategy/idempotentRepository could miss a real mismatch or misreport it against the wrong bean. Both declaredBeans and declaredBeanTypes now share a single scanBeansBlocks walk of each beans: block (list or map form, comments ignored), which also resets "the last bean seen" whenever a block closes, so a type: line outside any bean, or in a later block, is never attributed to a stale name. _Claude Code on behalf of Adriano Machado (@ammachado)_ _This was generated by an AI agent and may contain inaccuracies. Please verify before relying on it._ Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
yaml-dsl.adoc gains a NOTE next to the first beans example showing the map form (beans keyed by name, without name:), and stating that the list is the canonical form written by camel validate yaml --canonical, the canonical schema and the YAML Camel writes. The 4.23 upgrade guide documents the schema change for tools that read camelYamlDsl.json (beans is now oneOf a list or a map keyed by name, with the new BeanFactoryDefinitionByName definition); the canonical schema is unchanged. The catalog's mirrored copy of yaml-dsl.adoc is regenerated to match (catalog/camel-catalog -Dquickly). docs/components/modules/others/pages/yaml-dsl.adoc is a symlink to the source doc, so it needs no separate change. _Claude Code on behalf of Adriano Machado (@ammachado)_ _This was generated by an AI agent and may contain inaccuracies. Please verify before relying on it._ Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…t, not a sample GenerateDocSamplesMojo treats every [source,yaml] block of yaml-dsl.adoc whose first line starts with "- " as a route example: it is validated and, for yaml-dsl.adoc, sampled as a "beans" entry served to AI models by camel_catalog_sample. The map-form example added for CAMEL-24704 started with "- beans:", so it became the 2nd beans sample and displaced the canonical "declare a bean and call it from a route" sample that CatalogSamplesTest.beansIsATopLevelEntryWithASample expects at index 1. Drop the leading "- " so the example is a fragment (the value of `beans:`, not a full route): the mojo then neither validates nor samples it, and models keep being taught the canonical list form. Also renamed the example bean to myMapBean so it no longer reuses beanFromMap from the list example right above it, and added a short phrase to the NOTE clarifying the fragment is the value of `beans:`. Regenerated the catalog's mirrored copy of yaml-dsl.adoc (catalog/camel-catalog -Dquickly). Rebuilt camel-jbang-core (-Dquickly) and confirmed eip-samples.json is unchanged from HEAD. _Claude Code on behalf of Adriano Machado (@ammachado)_ _This was generated by an AI agent and may contain inaccuracies. Please verify before relying on it._ Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…map form
F1: the missing-dash list-form mistake (- forgotten before name: myBean)
was read as beans: {name: myBean, type: ...} and lost its hint, both in
the lenient validator ("string found, object expected" with no hint) and
at runtime (the misleading "a bean written as a map is name: ..." message).
Add a validator hint for .*/beans/name "object expected" errors, and have
BeansDeserializer.asBeanDefinitions recognize a "name" key with a
non-mapping value as this same mistake.
F2: GenerateYamlSchemaMojo.generateKeyedDefinitions and allowMapForm used
withObject(), which silently creates an empty node for a renamed or
missing item definition, or lets a second mapKey overwrite the first for
the same item type. Fail the build loudly in both cases instead.
F3: the id/ref/class unknownProperty hint only matched the list form's
numeric index (/\d+/beans/\d+); extend it to also match a bean written as
a map keyed by its name (/\d+/beans/[^/]+), while keeping the "a bean item
is written as - name: ..." hint restricted to the list form, where an
unknown key does mean the item is keyed by name.
F4: clarify YamlProperty.mapKey()'s Javadoc: only the YAML schema
generator reads it, so the deserializer of the property that carries it
must accept the map form itself.
_Claude Code on behalf of Adriano Machado (@ammachado)_
_This was generated by an AI agent and may contain inaccuracies.
Please verify before relying on it._
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:
|
gnodet-bot
left a comment
There was a problem hiding this comment.
Solid feature PR — well-structured code, comprehensive error handling, and thorough test coverage across all three beans: entry points. The map-to-list conversion in asBeanDefinitions() is clean, the schema generation correctly produces oneOf with a keyed definition, and the scanBeansBlocks() refactoring properly isolates blocks to prevent cross-block name/type leakage. One doc nit below.
Milestone: this targets main and the feature is @since 4.23 — should carry milestone 4.23.0.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 646 of 694 tested, 22 compile-only — current: 80 all testedMaveniverse Scalpel detected 646 affected modules (current approach: 80). Skip-tests mode would test 646 modules (9 direct + 643 downstream), skip tests for 22 (generated code, meta-modules)
|
| Module | Duration | Status |
|---|---|---|
| Camel :: Catalog :: Camel Catalog | 27.7s | SUCCESS |
| Camel :: Docs | 20.0s | SUCCESS |
| Camel :: YAML DSL | 13.1s | SUCCESS |
| Camel :: API | 8.1s | SUCCESS |
| Camel :: YAML DSL :: Validator | 7.2s | SUCCESS |
| Camel :: YAML DSL :: Deserializers | 5.8s | SUCCESS |
| Camel :: SPI Annotations | 2.4s | SUCCESS |
| Camel :: YAML DSL :: Maven Plugins | 2.2s | SUCCESS |
| Camel :: JBang :: Core | n/a |
Top 20 slowest modules:
Camel :: Catalog :: Camel Catalog(27.7s)Camel :: Docs(20.0s)Camel :: YAML DSL(13.1s)Camel :: API(8.1s)Camel :: YAML DSL :: Validator(7.2s)Camel :: YAML DSL :: Deserializers(5.8s)Camel :: SPI Annotations(2.4s)Camel :: YAML DSL :: Maven Plugins(2.2s)
davsclaus
left a comment
There was a problem hiding this comment.
Thanks @ammachado, this is a careful implementation of the JIRA proposal. The list stays canonical: the canonical schema, the writer and --canonical still use it, and the map form is only in the lenient schema. The map-to-list conversion reuses every existing check and the pre-parse dedup. The "forgotten - " case gets its own error, so the two forms don't blur, and the upgrade-guide note for schema consumers is appreciated.
One small consistency point, inline: the new hint suggests the flow form myBean: {type: ...}, but the jbang bean-reference scanner only recognizes the block form. A model that follows the hint then gets "bean not declared", and camel_write_file refuses the file. Either write the hint in block form, or teach MAP_KEY_PATTERN the flow form.
This review was generated by an AI agent on behalf of davsclaus and may contain inaccuracies. Please verify all suggestions before applying.
| append("type", ".*/beans/name", m -> m.message().contains("object expected"), | ||
| m -> "beans is a list: each bean starts with \"- \": - name: myBean followed by type:" | ||
| + " \"#class:com.example.MyBean\" (indented under the -); or a map keyed by the bean name:" | ||
| + " myBean: {type: ...}"), |
There was a problem hiding this comment.
This suggests the flow form myBean: {type: ...}, which the deserializer accepts, but BeanRefChecks.MAP_KEY_PATTERN only matches a key line ending in :. So declaredBeans does not see the bean, a ref: myBean / ${bean:myBean} is reported as undeclared, and camel_write_file refuses the file. Maybe write the hint in block form (myBean: then type: ... indented under it), which matches the docs example?
|
|
||
| /** The beans declared under {@code beans:} with a {@code #class:} type, name to fully qualified class name. */ | ||
| /** A bean written as a map: the name alone as the key, its properties indented below (CAMEL-24704). */ | ||
| static final Pattern MAP_KEY_PATTERN = Pattern.compile("^(\"[^\"]+\"|'[^']+'|[^\\s:#\"'][^:#]*?):\\s*$"); |
There was a problem hiding this comment.
Alternatively (or as well), this could recognize a flow-mapping bean at the child indent (myBean: {type: "#class:..."}) so the reference and type checks match what the deserializer accepts. A test with the flow form would cover it.
davsclaus
left a comment
There was a problem hiding this comment.
Follow-up on the hints (this replaces my earlier "write the hint in block form" suggestion; my approval of the change itself stands).
The hints are what steer AI models writing Camel YAML, so they should only teach the canonical form. The list stays canonical in this PR (canonical schema, writer, docs), so a hint should not advertise the map form as an alternative, in flow style or block style. Two spots, inline:
- The missing-
-hint offersmyBean: {type: ...}as an alternative. I'd drop that tail so it only shows the list form. This also avoids theBeanRefChecksblind spot for the flow form, where a model that follows the hint has its file refused as "bean not declared". - The widened id/ref/class hint reads oddly for a map-form bean: for
myBean: {class: ...}it says "(name instead of class, type instead of class)", but the name is already the key. The new test only checks the "type instead of class" part, so it doesn't catch this.
Separately, MAP_KEY_PATTERN still doesn't see a flow-style map bean (myBean: {type: "#class:..."}). The runtime accepts it and a model may write it unprompted, so a small fix plus a test would be good to have.
This review was generated by an AI agent on behalf of davsclaus and may contain inaccuracies. Please verify all suggestions before applying.
| m -> "beans is a list: each bean starts with \"- \": - name: myBean followed by type:" | ||
| + " \"#class:com.example.MyBean\" (indented under the -); or a map keyed by the bean name:" | ||
| + " myBean: {type: ...}"), |
There was a problem hiding this comment.
Hints should steer towards the canonical list form only, so I'd not advertise the map form (especially the flow style, which BeanRefChecks does not recognise):
| m -> "beans is a list: each bean starts with \"- \": - name: myBean followed by type:" | |
| + " \"#class:com.example.MyBean\" (indented under the -); or a map keyed by the bean name:" | |
| + " myBean: {type: ...}"), | |
| m -> "beans is a list: each bean starts with \"- \": - name: myBean followed by type:" | |
| + " \"#class:com.example.MyBean\" (indented under the -)"), |
| unknownProperty("/\\d+/beans/\\d+", m -> Set.of("id", "ref", "class").contains(m.unknown()), | ||
| // - id: myBean / class: ... , or myBean: {class: ...} in the map form: the bean properties are name | ||
| // and type (CAMEL-24704 F3: the map form's item is keyed by name, not by an index) | ||
| unknownProperty("/\\d+/beans/[^/]+", m -> Set.of("id", "ref", "class").contains(m.unknown()), |
There was a problem hiding this comment.
Now that this also matches a map-form bean (/beans/myBean), the message "(name instead of class, type instead of class)" is misleading there, because the name is already the key. Could the "name instead of ..." part be emitted only when the last path segment is a numeric index? A test asserting the map-form message does not say "name instead of" would pin it.
davsclaus
left a comment
There was a problem hiding this comment.
Stepping back from the hint details: I think the PR does exactly what CAMEL-24704 proposed, but on reflection that proposal (accept the map form, keep the list canonical) leaves us with two styles for the same thing. All docs, examples, the YAML writer, the canonical schema and Kaoto use the list, while the runtime and the lenient schema quietly accept a map. Users and AI models will copy whichever they see first, and every tool that reads the schema has to handle both. I don't want us in that half-and-half state.
So I think we should pick one:
- Switch fully to the name-keyed map. Change all docs and examples, the YAML writer, the canonical schema, the hints and Kaoto to the map form, and deprecate the list form (still accepted, but no longer shown anywhere). If we go this way it should be a deliberate decision, discussed on the dev list since it affects Kaoto and other schema consumers. It should cover beans first, with the same plan for the other name-keyed lists in the JIRA (parameters, headers, variables, param).
- Stay with the list only. Don't accept the map form, and fix the "AI writes a map" problem through the hints alone: a clear "beans is a list: - name: myBean ..." message on the map shape, which the validator already largely gives.
We need to think more about this before going further, so I'm changing my review to request changes to hold the merge until it's decided. Sorry for the back-and-forth, @ammachado. The implementation itself is careful, and if we choose option 1 most of it (deserializer, schema oneOf, the mapKey annotation) is the foundation we'd need anyway.
This review was generated by an AI agent on behalf of davsclaus and may contain inaccuracies. Please verify all suggestions before applying.
Description
CAMEL-24704: the YAML DSL now accepts
beanswritten as a map keyed by the bean name, next to the canonical list:This is the shape people and models write first; it was rejected with "object found, array expected". The map form is accepted at the top level, in
routeTemplateand intemplatedRoute. The list stays the canonical form:camelYamlDsl-canonical.json(andcamelYamlDsl-model.jsonderived from it) is unchanged, and the YAML Camel writes still uses the list.Schema
@YamlProperty.mapKey()(tooling/spi-annotations, copied intocamel-api,@since 4.23): marks a list whose items may also be written as a map keyed by that property. Only the schema generator reads it; the deserializer of the property accepts the map form itself.GenerateYamlSchemaMojo, non-canonical only: such a property becomesoneOf [array, object], with a synthesizedBeanFactoryDefinitionByName(the bean definition withoutname) for the map values. The mojo fails the build if the item definition is missing or two properties give the same item type different keys.beans(top level,routeTemplate,templatedRoute) is marked withmapKey = "name".Runtime
BeansDeserializer.asBeanDefinitions(Node)reads either form and is used by the three deserializers (including the pre-parse). A map entry is read as the list item- name: <key>followed by its properties, so every existing check, the#class:prefix and the pre-parse dedup cache behave the same.name:inside a map-form bean, a name declared twice, and an entry without properties under it. A list-form bean written without its-keeps a "beans is a list" message instead of being read as a bean calledname.Validator and camel-jbang
SchemaHints: the missing--bean keeps its "beans is a list" hint under the newoneOf, and theid:/class:hint also covers map-form beans.BeanRefChecks:declaredBeansanddeclaredBeanTypesrecognize map-form names through one sharedbeans:block scanner.declaredBeanTypesused to bind everytype:in the file to the lastname:seen, which could attribute a bean's type to another bean; it now stays insidebeans:blocks.Docs
yaml-dsl.adoc: a NOTE with a fragment example of the map form. It is written as a fragment on purpose, so the doc sample generator does not offer it as abeanssample incamel_catalog_sample; the samples keep teaching the list.camelYamlDsl.json(editors such as Kaoto) and expectbeansto betype: arrayneed to handle theoneOf.Out of scope, as the JIRA suggests doing them one at a time: the other name-keyed lists (
setHeaders,setVariables, templateparameters, restparam, ...). WithmapKeyin place, each is an annotation plus its deserializer branch.Tests: map form at the top level, in
routeTemplateand intemplatedRoute; empty map; quoted and dotted names; no double registration through the pre-parse; the error cases above; lenient vs canonical validation; the validator hints; the jbang bean reference and bean type checks. Results on the final head: camel-yaml-dsl 419/419, camel-yaml-dsl-validator 155/155, camel-yaml-dsl-maven-plugin 2/2, camel-jbang-coreai.*268 run, 0 failures, 1 skipped (ExpressionEvaluatorLanguageTest, an existing skip).Target
mainbranch)Tracking
Apache Camel coding standards and style
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.Not from the root folder: I built the touched modules (
tooling/spi-annotations,core/camel-api, the fourdsl/camel-yaml-dslmodules,catalog/camel-catalog,dsl/camel-jbang/camel-jbang-core,dsl/camel-jbang/camel-jbang-plugin-tui), committed the regeneratedcamelYamlDsl.json,YamlPropertycopy and catalogyaml-dsl.adoc, andgit statuswas clean afterwards.AI-assisted contributions
Co-authored-bytrailers) and the PR description identifies the AI tool used.🤖 Generated with Claude Code on behalf of Adriano Machado (@ammachado)
This was generated by an AI agent and may contain inaccuracies. Please verify before relying on it.