gnodet-bot commented on code in PR #26574:
URL: https://github.com/apache/camel/pull/26574#discussion_r4046132238
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/BeanRefChecks.java:
##########
@@ -337,8 +342,51 @@ static int indentOf(String line) {
}
/**
- * A class named with its package that is neither next to the route nor on
the CLI classpath: the wrong package
- * (org.apache.camel.support.StringAggregationStrategy) or a missing
dependency. Null when the class is fine.
+ * The classes camel run resolves to a Maven dependency and downloads on
demand (camel-kamelet-main's
+ * camel-main-known-dependencies.properties,
camel-component-known-dependencies.properties and the generated
+ * camel-thirdparty-known-dependencies.properties of CAMEL-24809), so a
#class:org.postgresql.ds.PGSimpleDataSource
+ * bean is fine without a dependency declared even though the class is not
on the CLI classpath. Matched the way the
+ * runtime matches: the class name, then each enclosing package.
+ */
+ private static volatile Map<String, String> knownDependencies;
Review Comment:
⚠️ **Volatile lazy init — still missing the intent comment**
This is a lock-free "benign data race" idiom: two threads can both read
`null`, both build a `HashMap` independently, and the one that writes last wins
— both maps contain the same entries so the result is correct. Without a
comment, the next reader will either wonder why there is no `synchronized`, or
will "helpfully" add one.
Suggested addition:
```suggestion
// Lazy-initialised via a benign data race: two threads may both build
the map if they race
// on null; both produce identical maps so the last writer wins without
correctness impact.
private static volatile Map<String, String> knownDependencies;
```
##########
dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/SourceValidatorBeanRefsTest.java:
##########
@@ -350,6 +350,44 @@ void aClassInTheWrongPackageIsReported(@TempDir Path dir)
throws IOException {
.contains("did you mean
org.apache.camel.processor.aggregate.StringAggregationStrategy?");
}
+ @Test
+ void aClassCamelRunDownloadsIsNotReportedAsMissing(@TempDir Path dir)
throws IOException {
+ // the Postgres datasource and the Artemis connection factory are not
on the CLI classpath, but camel run
+ // resolves them to their Maven dependency
(camel-main-known-dependencies.properties) and downloads it, so a
+ // bean of that type runs; the validator must not contradict the
runtime
+ List<String> msgs = SourceValidator.validate("r.camel.yaml", """
+ - beans:
+ - name: postgresDS
+ type: "#class:org.postgresql.ds.PGSimpleDataSource"
+ properties:
+ url: "jdbc:postgresql://localhost:5432/postgres"
+ - name: artemisCF
+ type:
"#class:org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory"
+ - route:
+ from:
+ uri: "timer:tick?period=1000"
+ steps:
+ - to:
+ uri: "sql:select 1?dataSource=#postgresDS"
+ """, CATALOG, null, dir);
+ assertThat(msgs).isEmpty();
+
assertThat(BeanRefChecks.knownDependency("org.postgresql.ds.PGSimpleDataSource"))
+ .startsWith("org.postgresql:postgresql");
+
assertThat(BeanRefChecks.knownDependency("com.example.NoSuchThing")).isNull();
Review Comment:
💡 **Missing fallback path: properties files absent from classpath**
The first test (`aClassCamelRunDownloadsIsNotReportedAsMissing`) passes
because `camel-main-known-dependencies.properties` is on the test classpath. If
`knownDependency()` silently gets no resources (empty map), the same YAML would
now produce a false-positive error — the very regression this PR fixes. There
is no test that covers that path.
The `catch (Exception e)` block swallows all I/O errors by design, so a
missing properties file is indistinguishable from an empty one. A test using
`BeanRefChecks.knownDependency("org.postgresql.ds.PGSimpleDataSource")`
directly would surface a broken classpath setup during the build rather than
silently regressing in production.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
