gnodet-bot commented on code in PR #27206:
URL: https://github.com/apache/camel/pull/27206#discussion_r4155835882
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/catalog/KameletCatalogHelper.java:
##########
@@ -38,6 +38,23 @@ public static List<String> findKameletNames(String version,
String repositories)
return new ArrayList<>(kamelets.keySet());
}
+ /**
+ * Returns a map of kamelet name → spec.dependencies for all official
kamelets in the catalog. Loaded in a single
+ * pass to avoid repeated catalog downloads.
+ */
+ public static Map<String, List<String>> findKameletDependencies(String
version, String repositories)
+ throws Exception {
+ Map<String, Object> kamelets = loadKamelets(version, repositories);
+ Map<String, List<String>> result = new LinkedHashMap<>();
+ for (Map.Entry<String, Object> entry : kamelets.entrySet()) {
+ List<String> deps = getDependencies(entry.getValue());
+ if (deps != null && !deps.isEmpty()) {
+ result.put(entry.getKey(), deps);
+ }
+ }
Review Comment:
⚠️ **Subtle behavior change:** `findKameletDependencies()` only includes
kamelets that have non-empty `spec.dependencies` (line 51: `if (deps != null &&
!deps.isEmpty())`). When `ExportBaseCommand` derives `officialKamelets` from
`officialKameletDeps.keySet()`, any official kamelet with zero declared
dependencies would be excluded from the list.
Currently all official kamelets declare at least `camel:kamelet`, so this
isn't a production bug *today*. But the contract has silently changed —
`findKameletNames()` returned **all** catalog entries, while
`findKameletDependencies().keySet()` returns only a subset. If a kamelet
without dependencies is ever added to the catalog, it won't be recognized as
"official" and the `asfKamelets` flag won't be set, breaking the kamelets BOM
inclusion.
Two options:
**Option A** — Include all kamelets in the map (with `List.of()` for those
without deps):
```suggestion
public static Map<String, List<String>> findKameletDependencies(String
version, String repositories)
throws Exception {
Map<String, Object> kamelets = loadKamelets(version, repositories);
Map<String, List<String>> result = new LinkedHashMap<>();
for (Map.Entry<String, Object> entry : kamelets.entrySet()) {
List<String> deps = getDependencies(entry.getValue());
result.put(entry.getKey(), deps != null ? deps : List.of());
}
return result;
}
```
**Option B** — Keep calling `findKameletNames()` for the official list and
use `findKameletDependencies()` only for the fallback lookup.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ExportBaseCommand.java:
##########
@@ -659,8 +659,12 @@ protected Set<String> resolveDependencies(Path settings,
Path profile) throws Ex
List<String> lines = RuntimeUtil.loadPropertiesLines(settings);
- // check if we use custom and/or official ASF kamelets
- List<String> officialKamelets =
KameletCatalogHelper.findKameletNames(kameletsVersion, mavenResolver.repos());
+ // check if we use custom and/or official ASF kamelets; also pre-load
their spec.dependencies
+ // so they can be added as a fallback when the runSilently mechanism
misses them (e.g. when
+ // Java route compilation fails before the kamelet is loaded and its
deps are recorded)
+ Map<String, List<String>> officialKameletDeps
+ =
KameletCatalogHelper.findKameletDependencies(kameletsVersion,
mavenResolver.repos());
+ List<String> officialKamelets = new
ArrayList<>(officialKameletDeps.keySet());
Review Comment:
💡 This derives the official kamelet list from
`officialKameletDeps.keySet()`, which — per the comment above on
`findKameletDependencies` — may miss kamelets with empty `spec.dependencies`.
See suggestion on `KameletCatalogHelper.java`.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ExportBaseCommand.java:
##########
@@ -781,6 +785,25 @@ protected Set<String> resolveDependencies(Path settings,
Path profile) throws Ex
}
}
+ // Defensive fallback: add spec.dependencies from official kamelets
used in this integration.
+ // The primary mechanism (runSilently download listener) only captures
kamelet-declared
+ // dependencies when the kamelet is loaded during the silent run. If a
Java route builder
+ // fails to compile (e.g., missing jackson-databind) before the
kamelet is reached, its
+ // spec.dependencies are never written to settings, causing them to be
absent in the
+ // exported pom.xml. This fallback guarantees they are always included.
+ for (String line : lines) {
+ if (line.startsWith("kamelet=")) {
+ String kameletName = StringHelper.after(line, "kamelet=");
+ List<String> kameletDeps =
officialKameletDeps.getOrDefault(kameletName, List.of());
+ for (String dep : kameletDeps) {
+ // skip the kamelets BOM itself — it is already handled
above
+ if
(!dep.contains("org.apache.camel.kamelets:camel-kamelets")) {
+ answer.add(dep);
+ }
+ }
+ }
+ }
+
Review Comment:
💡 The fallback logic is correct: iterating `kamelet=` entries, looking up
their deps in the pre-loaded map, and filtering out the kamelets BOM. Since
`answer` is a `TreeSet`, duplicate additions (e.g. `camel:kamelet` already
present from the primary mechanism) are harmless.
One minor observation: this second loop over `lines` could be folded into
the existing loop above (lines 671–783) where `kamelet=` entries are already
being scanned, avoiding the duplicate iteration. Not a correctness issue, but a
readability improvement.
--
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]
