Repository navigation
CAMEL-25068: camel-core - Route templates: the caller's bean wins over a template bean of the same name (item 1) - #27466
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
🧪 CI tested the following changed modules:
🔁 6 tests passed only after a retry on JDK 25 (6 retried attempts) Recovered flaky tests on JDK 25 (6)
🔬 Scalpel shadow comparison — Scalpel: 558 of 699 tested, 27 compile-only — current: 559 all testedMaveniverse Scalpel detected 558 affected modules (current approach: 559). Skip-tests mode would test 558 modules (4 direct + 555 downstream), skip tests for 27 (generated code, meta-modules) Modules only in current approach (1)
Modules Scalpel would test (558)
Modules with tests skipped (27)
Build reactor — dependencies compiled but only changed modules were tested (4 modules, 24.9s total)Total reactor time: 24.9s
Top 20 slowest modules:
|
davsclaus
left a comment
There was a problem hiding this comment.
Thanks, the caller's bean now wins, and Kamelets are not affected since addRouteFromKamelet creates a new context with parameters only. The route template and Kamelet tests pass locally.
A note, non-blocking: when the same RouteTemplateContext is reused for several addRouteFromTemplate calls, the template beans bound for the first route are now seen as caller beans, so later routes share that instance instead of each getting a new one. A sentence in the docs or upgrade guide would help.
Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying. It does not replace specialized review tools or static analysis.
…eral routes shares the template beans of the first route Review of apache#27466: the template beans bound for the first route are kept in the RouteTemplateContext and are then seen as beans of the caller, so the later routes share them. Say so in route-template.adoc and the 4.23 upgrade guide. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks @davsclaus. Added the note in 74e855c: About the red
So nothing changes here for CI. The camel-core expected files (or the dump order) need a fix on Claude Code on behalf of allthingssecurity |
…r a template bean of the same name (item 1) A bean bound by TemplatedRouteBuilder.bean or in a templatedRoute was replaced by a templateBean with the same name when both had the same type, and lost every lookup by the type of the template bean, while a lookup by name found it. addTemplateBeans now skips a template bean whose name the caller has bound, so the caller's bean is used for every lookup, as with a bean bound in the configurer and as a parameter overrides the default value of the template. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eral routes shares the template beans of the first route Review of apache#27466: the template beans bound for the first route are kept in the RouteTemplateContext and are then seen as beans of the caller, so the later routes share them. Say so in route-template.adoc and the 4.23 upgrade guide. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
74e855c to
e052bf1
Compare
oscerd
left a comment
There was a problem hiding this comment.
Verified. addTemplateBeans now snapshots the caller-bound bean names from the LocalBeanRegistry (local.keys()) up front — at that point only the caller's beans are in the local repo, since doAddRouteFromTemplate binds the caller's beans before it calls addTemplateBeans — and skips any template bean whose name is already there, so the template bean is never created and every lookup (by name or by type) resolves to the caller's bean. That's option (a), and it collapses the four rows of the main-branch matrix into a single predictable "caller wins", consistent with how the configurer and template parameters already override the template. The Collections.emptySet() fallback for a non-LocalBeanRegistry repo preserves the old behaviour where the snapshot isn't available.
RouteTemplateCallerBeanTest covers lookup-by-type and lookup-by-name for each binding path, and the upgrade guide + route-template.adoc document the resolved precedence. CI is green.
Approving.
This review was generated with AI assistance and reviewed/issued by the human operator. Claude Code on behalf of oscerd
Description
CAMEL-25068 item 1
Follow-up from the deep review of CAMEL-25048. When the caller binds a bean with the same name as a
templateBeanof the template, which of the two beans the route gets depended on how the caller bound it and on how the route looks it up. Template:On
main:greetingwithprocess("greeting")(lookup byProcessor)to("bean:{{greeting}}")(lookup by name)TemplatedRouteBuilder...bean("greeting", mine)TemplatedRouteBuilder...bean("greeting", Processor.class, mine)TemplatedRouteDefinition.bean("greeting", mine)(templatedRoutein Java, XML, YAML)TemplatedRouteBuilder...configure(rtc -> rtc.bind("greeting", Processor.class, mine))The caller's beans are bound in the route template context before
DefaultModel.doAddRouteFromTemplate, which then binds the template beans into the same local registry (addTemplateBeans). The registry keeps one entry per name and type, so a template bean with the same type replaces the caller's bean, and one with another type is added next to it; a lookup by type then finds the template bean, and a lookup by name finds whichever entry was bound first. A configurer runs later, when the route is created, so its bean replaces the template bean of the same type.This change implements option (a) from the JIRA analysis: the caller's bean wins.
addTemplateBeansskips a template bean whose name the caller has already bound, so the template bean is not created and every lookup finds the caller's bean. This is consistent with:TemplatedRouteBuilderruns after the one of the template, so it can override it);Kamelets are not affected:
addRouteFromKameletcreates a new route template context with parameters only, so no template bean is skipped there (camel-kamelet suite below).Other options, for the review:
FailedToCreateRouteFromTemplateExceptionon a clash. Strict, and it breaks callers that override on purpose.Behaviour change: the upgrade guide (4.23, "Route templates") gets one more item, and
route-template.adoc(section "Binding Beans to Route Templates") a sentence. The javadoc of the threeTemplatedRouteBuilder.beanmethods now says that the bean takes precedence over a template bean with the same name.Not changed:
rtc.bind("greeting", mine), keyed by the class ofmine) after the template bean is bound still loses a lookup by the template bean's type, as onmain. Making that path agree too would needDefaultRouteTemplateContext.bindto replace every entry of the name, which changes the registry semantics; I left it out. Happy to add it if you prefer.TemplatedRouteBuilder(orRouteTemplateContext) used to add several routes: the template beans bound for the first route are in its local registry when the next route is added, so they are now kept for the next route instead of bound again. Camel itself always uses a new context per route (TemplatedRouteBuilder.builder,addRouteFromTemplatedRoute, the parameter map variants ofaddRouteFromTemplate, Kamelets). Bothroute-template.adocand the upgrade guide now say so, and that a newRouteTemplateContextper route gives new template beans for each route (review follow-up).The precedence rule was checked with a small Lean 4 model of
SimpleRegistry.bind/SupplierRegistry.lookupByNameAndTypeand the three binding steps (caller, template, configurer). It reproduces the four rows above, proves that with the change every lookup of a name the caller bound (by any type) returns the caller's bean, and that the change binds exactly whatmainbinds when no template bean has a name the caller bound. It also shows the configurer-without-type case above.Tests:
RouteTemplateCallerBeanTest(camel-core), one test per binding path:builderBeanTakesPrecedence:TemplatedRouteBuilder.beanwithout and with a type; also checks that the template bean with the same name is not created, and that the other template bean (suffix) is still used;templatedRouteBeanTakesPrecedence:TemplatedRouteDefinition.beanviaaddRouteFromTemplatedRoute;configurerBeanTakesPrecedence:configure(rtc -> rtc.bind(...)).The first two fail on
main(two runs each):expected: "caller!" but was: "template!". The configurer test passes onmain: it pins the path the change aligns with.With the change:
FileConsumerIdempotentKeyNameAndSizeTestfailed once and passed on the rerun), and so do the suites of the core modules it is built with (9454 tests in total);Target
mainbranch)Tracking
Apache Camel coding standards and style
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.(I built and tested the core modules and the modules listed above, including the formatter and import-sort plugins. No generated files change. I did not run the full root build.)
AI-assisted contributions
Co-authored-bytrailers) and the PR description identifies the AI tool used.This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a
Co-Authored-Bytrailer.Claude Code on behalf of allthingssecurity
🤖 Generated with Claude Code