gnodet-bot commented on code in PR #26635:
URL: https://github.com/apache/camel/pull/26635#discussion_r4057504160
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ExportBaseCommand.java:
##########
@@ -881,6 +881,43 @@ private static void addRuntimeSpecificDependencies(String
deps, Set<String> answ
}
}
+ /** resource:classpath:x or resource:file:x in a route file: the file x is
referenced by name. */
+ private static final Pattern RESOURCE_REF_PATTERN =
Pattern.compile("resource:(?:classpath|file):([^\"'\\s?&,]+)");
+
+ /**
+ * The names of the files the route files
(camel.main.routesIncludePattern) reference as resource:classpath: or
+ * resource:file:, so the export can keep them where the reference
resolves.
+ */
+ Set<String> resourceReferencedFiles(String routeFiles) {
+ Set<String> names = new HashSet<>();
+ if (routeFiles == null || routeFiles.isBlank()) {
+ return names;
+ }
+ for (String f : routeFiles.split(",")) {
+ f = f.trim();
+ String scheme = getScheme(f);
+ if (scheme != null) {
+ if (!"file".equals(scheme)) {
+ continue;
+ }
+ f = f.substring(scheme.length() + 1);
+ }
+ Path path = Paths.get(f);
+ if (!Files.isRegularFile(path)) {
+ continue;
+ }
+ try {
+ Matcher m =
RESOURCE_REF_PATTERN.matcher(Files.readString(path));
+ while (m.find()) {
+ names.add(FileUtil.stripPath(m.group(1)));
Review Comment:
🔍 **Basename-only matching:** `FileUtil.stripPath(m.group(1))` reduces
`resource:classpath:scripts/mapping.groovy` to just `mapping.groovy`. If two
Groovy files in different subdirectories share a basename, both will be routed
to the resources root even if only one is referenced. This is consistent with
how `f` is stripped on the other side (line 989), so the matching is symmetric
— but the imprecision is worth documenting. Consider retaining the relative
path in the set and matching with `f.endsWith(name)` to narrow the collision
window.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ExportBaseCommand.java:
##########
@@ -881,6 +881,43 @@ private static void addRuntimeSpecificDependencies(String
deps, Set<String> answ
}
}
+ /** resource:classpath:x or resource:file:x in a route file: the file x is
referenced by name. */
+ private static final Pattern RESOURCE_REF_PATTERN =
Pattern.compile("resource:(?:classpath|file):([^\"'\\s?&,]+)");
+
+ /**
+ * The names of the files the route files
(camel.main.routesIncludePattern) reference as resource:classpath: or
+ * resource:file:, so the export can keep them where the reference
resolves.
+ */
+ Set<String> resourceReferencedFiles(String routeFiles) {
+ Set<String> names = new HashSet<>();
+ if (routeFiles == null || routeFiles.isBlank()) {
+ return names;
+ }
+ for (String f : routeFiles.split(",")) {
+ f = f.trim();
+ String scheme = getScheme(f);
+ if (scheme != null) {
+ if (!"file".equals(scheme)) {
+ continue;
Review Comment:
⚠️ **Silent gap — `classpath:` route files are not scanned:** When
`camel.main.routesIncludePattern` contains `classpath:route.yaml` entries
(written by `Run` for classpath-loaded routes), this branch silently skips
them. Any Groovy resource referenced inside such a route will not be detected
and will therefore be placed in `camel-groovy` instead of the resources root —
which is the exact bug this PR is fixing.
The Javadoc describes the method as scanning "the route files
(`camel.main.routesIncludePattern`)" without qualifying that `classpath:`
entries are excluded. At a minimum, update the Javadoc to document the
limitation. Ideally, resolve classpath entries against the classpath or the
settings-adjacent resource directories.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ExportBaseCommand.java:
##########
@@ -881,6 +881,43 @@ private static void addRuntimeSpecificDependencies(String
deps, Set<String> answ
}
}
+ /** resource:classpath:x or resource:file:x in a route file: the file x is
referenced by name. */
+ private static final Pattern RESOURCE_REF_PATTERN =
Pattern.compile("resource:(?:classpath|file):([^\"'\\s?&,]+)");
+
+ /**
+ * The names of the files the route files
(camel.main.routesIncludePattern) reference as resource:classpath: or
+ * resource:file:, so the export can keep them where the reference
resolves.
+ */
+ Set<String> resourceReferencedFiles(String routeFiles) {
Review Comment:
💡 **Method visibility / `static`:** `resourceReferencedFiles` accesses no
instance state — it reads only its argument and the static
`RESOURCE_REF_PATTERN`. It should be `private static`. As written
(package-private instance method) it unnecessarily widens the implicit API
surface of `ExportBaseCommand`.
```suggestion
private static Set<String> resourceReferencedFiles(String routeFiles) {
```
--
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]
