gnodet-bot commented on code in PR #27595:
URL: https://github.com/apache/camel/pull/27595#discussion_r4225650163
##########
components/camel-rest-postman/src/main/java/org/apache/camel/component/rest/postman/RestPostmanConfiguration.java:
##########
@@ -62,6 +62,15 @@ public class RestPostmanConfiguration implements Cloneable {
+ " resolved. When false the placeholder is left
as-is.")
private boolean failOnUnresolvedVariable;
+ @UriParam(label = "common,security")
+ @Metadata(description = "Whether a {{variable}} placeholder that neither
the collection nor the variables option"
+ + " defines is resolved from Camel properties,
which by default also cover JVM system"
+ + " properties and OS environment variables. When
not set, this is done for a collection"
+ + " read from the classpath or the file system,
and not for one fetched from the Postman"
+ + " cloud, over HTTP or through any other resource
scheme, because whoever edits or serves"
+ + " such a collection could otherwise copy those
values into an outgoing request.")
+ private Boolean resolveVariablesFromProperties;
Review Comment:
⚠️ **Catalog/API mismatch:** This field is `Boolean` (nullable) with no
initializer, so its unset value is `null`. Yet the generated catalog JSON
declares `"defaultValue": false` for this property. The actual behavior when
`null` is auto-detect via `isLocalSource()` — effectively `true` for
classpath/file sources. The catalog description even contradicts its own
`defaultValue` by saying *"When not set, this is done for a collection read
from the classpath or the file system"*.
Consider adding `@Metadata(defaultValue = "")` or similar to suppress the
spurious `false` in the catalog, or at least align the description so it
doesn't conflict.
##########
components/camel-rest-postman/src/main/java/org/apache/camel/component/rest/postman/collection/PostmanCollectionLoader.java:
##########
@@ -73,6 +73,21 @@ public static boolean isCloudSource(String source, String
sourceType) {
return UID_PATTERN.matcher(source).matches();
}
+ /**
+ * Whether the source is read from the application's own classpath or file
system: a {@code classpath:} or
+ * {@code file:} URI, or a name without a scheme, which is resolved from
the classpath. A cloud source, an
+ * {@code http:} or {@code https:} URI and any other scheme are not local.
+ *
+ * @param source the collection source
+ * @param sourceType {@code auto}, {@code resource} or {@code cloud}
+ */
+ public static boolean isLocalSource(String source, String sourceType) {
+ if (isCloudSource(source, sourceType)) {
+ return false;
+ }
+ return source.startsWith("classpath:") || source.startsWith("file:")
|| source.indexOf(':') < 0;
Review Comment:
💡 **Edge case:** `source.indexOf(':') < 0` correctly identifies scheme-less
classpath resource names, but would misclassify a Windows absolute path like
`C:\collection.json` (where `indexOf(':') == 1`) as non-local, disabling
property resolution for it. Low severity since Camel typically uses `file:`
URIs on all platforms, but worth a comment clarifying the intent.
--
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]
