Repository navigation
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
✅ Generated files are up to dateAn earlier CI run reported uncommitted generated changes; the latest run no longer does. |
gnodet-bot
left a comment
There was a problem hiding this comment.
Well-designed feature — the route checksum + revision concept is clean, the SPI is well-placed, and the test coverage is solid.
One minor grammar nit in the description strings.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
… tell which routes changed between two runs or after a reload Each route has a checksum of the route as written. It is a short hash of the route's steps, options and expressions, with property placeholders as written. It leaves out the ids Camel generates, the source locations, and the parts that route configurations add to every route. The same route gives the same checksum in every run. A new SPI, ModelChecksumGenerator, computes it when the route is prepared, before the route configurations are merged in. It is implemented in camel-xml-io with the generated ModelWriter, so one implementation gives every application the same checksum. Without camel-xml-io the checksum is null. The revision counts, by route id, the times a route was added again with another checksum in this run. Where both show: - Route, NamedRoute and the managed route (Checksum, Revision); - the route console; the route-structure and route-dump consoles have the checksum; - camel get route and camel top route, in their JSON and with --checksum; - the reload summary, which says how many routes changed and marks each one (changed) or (new); - the descriptions of the MCP tools, which say how to use the checksum. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
…um of the routes of a source file Routes read from a source file without running them have the checksum they have when they run. A tool can compare the file with a running integration (camel get route --checksum). - The JSON route dump has the checksum of each route. - The routes of a Java source read by the Java DSL parser get it when the parser read the whole source. - camel cmd route-structure shows it under the source of each route. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
…descriptions (address review) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
c76501b to
f4147ca
Compare
|
🧪 CI tested the following changed modules:
🔁 7 tests passed only after a retry on JDK 25 (7 retried attempts) Recovered flaky tests on JDK 25 (7)
🔬 Scalpel shadow comparison — Scalpel: 577 of 707 tested, 23 compile-only — current: 578 all testedMaveniverse Scalpel detected 577 affected modules (current approach: 578). Skip-tests mode would test 577 modules (16 direct + 565 downstream), skip tests for 23 (generated code, meta-modules) Modules only in current approach (1)
Modules Scalpel would test (577)
Modules with tests skipped (23)
💡 Manual integration tests recommended:
Build reactor — dependencies compiled but only changed modules were tested (16 modules, 2m 53s total)Total reactor time: 2m 53s
Top 20 slowest modules:
|
oscerd
left a comment
There was a problem hiding this comment.
TL;DR
The design is good: one generator in camel-xml-io, placeholders hashed as written, computed once when the route is prepared, and the revision kept under routesLock. But a route created from a route template or a Kamelet includes a node prefix in its checksum that Camel generates from a JVM-wide counter, so its checksum changes every time the route is created again. Could we fix that before merge?
Blockers
RouteDefinitionHelper.java:427/LwModelChecksumGenerator.java:54: the generatednodePrefixIdof templated and Kamelet routes is part of the checksum (see 1).
Other findings: 6 (2 non-blocking, 4 suggestions)
1. Templated and Kamelet routes (blocking)
RouteTemplateDefinition.asRouteDefinition() sets nodePrefixId to <prefix>-<COUNTER.incrementAndGet()> (RouteTemplateDefinition.java:446, with a static COUNTER at :53). Kamelet.templateToRoute sets it to a SimpleUuidGenerator counter (Kamelet.java:463, :72). DefaultModel.java:651 builds that copy, :713 prepares it, and ModelWriter.java:1643 writes nodePrefixId. A reload re-creates templated routes (DefaultRoutesLoader.java:325 → RouteBuilder.java:847), so a templatedRoute with its own route id, saved again unchanged, gets a new checksum: reviseRoute increments its revision and the summary marks it (changed). The same happens to a Kamelet route with its own id when its parent route reloads. Across two runs the checksums only match if the templates are created in the same order. This prefix is an id Camel generates, which the PR intends to leave out. One option is a checksum flag on LwModelToXMLDumper.write that reuses the existing restorers in the extractor:
if (checksum && Boolean.TRUE.equals(route.isTemplate())) {
String prefix = route.getNodePrefixId(); // from a counter (template, Kamelet), not as written
route.setNodePrefixId(null);
restorers.add(() -> route.setNodePrefixId(prefix));
}It would also help to add a RouteChecksumTest case that creates the same templated route twice (stop and remove in between) and expects the same checksum and revision 1.
2. Endpoint DSL options that are objects (non-blocking)
resolveEndpointDslUris writes builder.getRawUri() (LwModelToXMLDumper.java:346/353/361). With no context, any option that is not a String, Number, Boolean or Enum (a DataSource, SSLContextParameters, an SDK client) goes into remaining (AbstractEndpointBuilder.java:125), and the URI gets hash= + remaining.hashCode() (:78). That is an identity hash, new in every run and for every new instance when a Java route reloads. With the same checksum flag we could drop the hash parameter; the docs already say beans are outside the checksum.
3. Literal secrets go into the digest (hardening, non-blocking)
Placeholders and vault references are hashed as written, and nothing loads a class or reaches the network, also from camel cmd route-structure. That part is good. A password written literally in a URI is hashed unmasked, though. route-dump and JMX dumpRouteAsXml already show it, but camel.main.dumpRoutes=json masks to URIs (SendDefinition.java:117) and now prints the checksum beside them. That gives anyone reading the logs a 48-bit offline check for a weak literal password. Running uri values through URISupport.sanitizeUri in the checksum input would close it, and it fits "a changed property value is configuration".
4. Smaller points
routeRevisions(AbstractCamelContext.java:256) is never pruned. An app that adds and removes routes with unique ids keeps growing it for the life of the JVM (Kamelet routes get<template>-<counter>ids by default). Could we add a cap, or a note in the Javadoc?RouteAddedEventandonRoutesAddfire (RouteService.java:228/233) beforeaddRoute(:237) sets the revision. Listeners see revision 0 next to a checksum, but the Javadoc says 0 means the checksum is not known.- CI tested only the 14 changed modules (dependents are over the threshold). Every route preparation now runs the XML writer, so a
test-dependentsrun, or at least camel-endpointdsl, camel-kamelet and camel-yaml-dsl, would add confidence. - The new test classes and methods are
public; package-private is preferred for new tests.
Reviewed with Claude Code on behalf of oscerd. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
… endpoint DSL make from counters, masks URI secrets, and the revision is set before the route is announced (address review) - a route from a route template or a Kamelet: the node prefix, the id of a Kamelet route without one of its own, and the routeId of its kamelet:source and kamelet:sink, all made from a counter, are no part of the checksum - the hash an endpoint DSL builder adds for its options that are objects (an identity hash) is no part of it - the secrets in endpoint URIs are masked before they are hashed - the revision is set before RouteAddedEvent and onRoutesAdd, and the revisions of the 10000 route ids used last are kept - the new test classes are package-private Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
|
Thanks @oscerd, all points are addressed in 1102549. 1. Templated and Kamelet routes (blocker): fixed.
2. Endpoint DSL options that are objects: fixed. In checksum mode the 3. Literal secrets: fixed. In checksum mode each 4. Smaller points
Claude Code on behalf of davsclaus |
gnodet-bot
left a comment
There was a problem hiding this comment.
All existing review comments appear to have been addressed. One minor observation on the new code:
This review was generated by an AI agent, Hermès on behalf of @gnodet.
The routeId pattern captures the value, which is compared to the id, and the & after a parameter is no part of the match, so a routeId of another value does not hide the one after it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
gnodet-bot
left a comment
There was a problem hiding this comment.
Previous findings are all addressed.
- ✅ Grammar nit: "added again changed" → "added again with another checksum" fixed in all 5 locations (RouteDevConsole, ManagedRouteMBean, route.json schemas).
- ✅ Pattern caching:
HASH_PARAMETERandROUTE_ID_PARAMETERare now static compiled constants.
No new issues in the latest commits. The leaveOutCounters() logic correctly handles Kamelet routes and non-Kamelet template routes, reviseRoute() is idempotent, the LRU routeRevisions map is safely locked, all new test classes are package-private, and sanitizeUri() covers URI secret masking.
LGTM.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
A tool that watches a running Camel app (the Camel CLI, the MCP tools, an IDE, an AI agent) knows the routes by id and file. After a restart or a reload it cannot tell which routes changed: every route of a file comes back under the same file name, and a generated id such as
route3may now belong to another route. This gives each route two values, shown the same way everywhere (CAMEL-25771).Checksum: a short hex digest (12 characters of a SHA-256) of the route as written:
routeIdof itskamelet:sourceandkamelet:sink. A templated route created again, such as by a reload, keeps its checksum;onException,interceptandonCompletionthat route configurations add to every route, so one edit to a shared configuration does not mark every route changed. TheonExceptionof the route itself is part of it.Revision: a counter per route id in the running app.
RouteAddedEventandonRoutesAdd, so listeners and JMX see it. Camel keeps the revisions of the 10000 route ids used last.How
ModelChecksumGenerator(camel-api,@since 4.24), computes the checksum when a route is prepared, before the route configurations are merged in (RouteDefinitionHelper.prepareRoute).ModelWriterwrites every option of the model straight into the digest. That is a single implementation on purpose:LwModelToXMLDumpernow shares its writing with the generator: a refactor with no change to the dumps (the camel-xml-io tests and the route dump tests pass).Where it shows
Route.getChecksum()/getRevision()andNamedRoute.getChecksum(), as default methods,@since 4.24.JMX: the
ChecksumandRevisionattributes of a managed route.Developer consoles:
checksumandrevisionper route in therouteconsole. The CLI status file and the MCPcamel_runtime_routestool read that console as is, so they have them with no extra code. Theroute-structureandroute-dumpconsoles have the checksum of each route.Camel CLI:
camel get routeandcamel top routehave both in their JSON, and as columns with--checksum.Camel CLI:
camel cmd route-structure <file>gives the checksum of each route of a source file without running it, and it is the checksum the route has when it runs:The JSON route dump (
camel.main.dumpRoutes=json) has the checksum too.The reload summary:
Routes reloaded summary (total:3 started:3 changed:2), with(changed)or(new)on each route the reload changed.MCP: the descriptions of
camel_runtime_routesandcamel_runtime_route_structuresay how to use the checksum.Cost: measured on 1000 routes with a choice, a split and a filter each: about 12 µs a route after warm-up, about 12 ms in all, so it is computed always.
Limits, documented in the routes page:
.process(...)dumps as a bareprocess, and a change inside a bean is not seen.Tests
RouteChecksumTest(camel-core):onException, placeholder values and debugging (with source locations) do not change it, while the route's ownonExceptiondoes;RouteAddedEventsee it;KameletRouteChecksumTest(camel-kamelet): Kamelet routes with and without an id of their own keep their checksums in a second run.EndpointDslRouteChecksumTest(camel-endpointdsl): a newExchangeFormatterinstance keeps the checksum.LwModelToXMLDumperRawUriTest(camel-xml-io) covers removing thehashparameter, and therouteIdof the route but not one of another value. Both patterns are compiled once.RouteWatcherReloadChangedRoutesTest: the reload summary, its changed and new markers, andchanged:0when a file is saved again as it was.RouteActionJavaSourceTest: a Java route read bycamel cmd route-structurewithout compiling it has the checksum of the same route compiled and added to a CamelContext.ManagedRouteChecksumTest,RouteChecksumDevConsoleTest, and a new case inCamelRouteStatusTest.test-dependentslabel.JsonDataFormatReifierTestfor Gson and Jsonb, fail on main too, and CAMEL-24426: camel-core - JsonDataFormatReifierTest expects allowUnmarshallType for Gson and Jsonb #28208 fixes them.orders.camel.yaml:camel cmd route-structureand running (camel get route --checksum), it gives the same checksum;Next, in follow-ups:
camel validatecan use a running integration's body types only for the routes whose checksum did not change.Claude Code on behalf of davsclaus
🤖 Generated with Claude Code