gnodet-bot commented on code in PR #27960:
URL: https://github.com/apache/camel/pull/27960#discussion_r4296847489
##########
dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/OpenApiTools.java:
##########
@@ -422,99 +415,43 @@ private Map.Entry<String, ApiResponse>
findFirstSuccessResponse(ApiResponses res
return null;
}
- private void checkSecurityScheme(
- String name, SecurityScheme scheme,
- List<DiagnosticMessage> warnings, List<DiagnosticMessage> info) {
-
+ private void checkSecurityScheme(String name, SecurityScheme scheme,
List<DiagnosticMessage> warnings) {
if (scheme.getType() == null) {
return;
}
-
- switch (scheme.getType()) {
- case APIKEY:
- if (scheme.getIn() == SecurityScheme.In.QUERY) {
- info.add(new DiagnosticMessage(
- "SECURITY_APIKEY_QUERY",
- "Security scheme '" + name + "' (apiKey in query)
is supported by Camel.",
- null));
- } else {
- warnings.add(new DiagnosticMessage(
- "SECURITY_APIKEY_NOT_QUERY",
- "Security scheme '" + name + "' (apiKey in " +
scheme.getIn()
- + ") is defined but
not enforced by Camel's REST OpenAPI integration. "
- + "You must handle
authentication in your route logic.",
- null));
- }
- break;
- case HTTP:
- warnings.add(new DiagnosticMessage(
- "SECURITY_HTTP",
- "Security scheme '" + name + "' (HTTP " +
scheme.getScheme()
- + ") is defined but not enforced by
Camel's REST OpenAPI integration. "
- + "You must handle authentication in
your route logic.",
- null));
- break;
- case OAUTH2:
- warnings.add(new DiagnosticMessage(
- "SECURITY_OAUTH2",
- "Security scheme '" + name
- + "' (OAuth2) is defined but not
enforced by Camel's REST OpenAPI integration. "
- + "You must handle authentication
in your route logic.",
- null));
- break;
- case OPENIDCONNECT:
- warnings.add(new DiagnosticMessage(
- "SECURITY_OPENIDCONNECT",
- "Security scheme '" + name
- + "' (OpenID Connect) is
defined but not enforced by Camel's REST OpenAPI integration. "
- + "You must handle
authentication in your route logic.",
- null));
- break;
- case MUTUALTLS:
- warnings.add(new DiagnosticMessage(
- "SECURITY_MUTUALTLS",
- "Security scheme '" + name
- + "' (Mutual TLS) is defined but
not enforced by Camel's REST OpenAPI integration. "
- + "You must handle
authentication in your route logic.",
- null));
- break;
- default:
- break;
- }
+ String kind = switch (scheme.getType()) {
+ case APIKEY -> "apiKey in " + scheme.getIn();
+ case HTTP -> "HTTP " + scheme.getScheme();
Review Comment:
⚠️ **Potential NPE on malformed specs**
`scheme.getIn()` (APIKEY) and `scheme.getScheme()` (HTTP) are not
null-checked before string concatenation. On a malformed or incomplete OpenAPI
spec, these can return `null`, producing misleading diagnostics like `"apiKey
in null"` or `"HTTP null"`. Consider guarding with a fallback:
```suggestion
case APIKEY -> "apiKey in " +
Objects.requireNonNullElse(scheme.getIn(), "<unset>");
case HTTP -> "HTTP " +
Objects.requireNonNullElse(scheme.getScheme(), "<unset>");
```
##########
dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/OpenApiTools.java:
##########
@@ -407,6 +306,100 @@ private List<OperationStub> collectOperationStubs(OpenAPI
openAPI) {
return stubs;
}
+ /**
+ * One mock file per path of the spec. For an operation without a route,
camel-rest-openapi serves the first
+ * resource of mockIncludePattern (default classpath:camel-mock/**) whose
location without extension ends with the
+ * request path, whatever the method: a path parameter is matched by its
value, so {@code /pets/{petId}} takes one
+ * file per value, such as {@code camel-mock/pets/1.json}.
+ */
+ private List<MockFileInfo> collectMockFiles(OpenAPI openAPI) {
+ List<MockFileInfo> files = new ArrayList<>();
+ if (openAPI.getPaths() == null) {
+ return files;
+ }
+ for (Map.Entry<String, PathItem> pathEntry :
openAPI.getPaths().entrySet()) {
+ String path = pathEntry.getKey();
+ Map<PathItem.HttpMethod, Operation> operations =
pathEntry.getValue().readOperationsMap();
+ if (operations == null || operations.isEmpty()) {
+ continue;
+ }
+
+ List<String> operationIds = new ArrayList<>();
+ Map<String, String> withoutFile = new LinkedHashMap<>();
+ String responseType = null;
+ String example = null;
+ for (Map.Entry<PathItem.HttpMethod, Operation> opEntry :
operations.entrySet()) {
+ String method = opEntry.getKey().name();
+ Operation op = opEntry.getValue();
+ operationIds.add(operationId(path, opEntry.getKey(), op));
+
+ if (op.getResponses() != null) {
+ Map.Entry<String, ApiResponse> success =
findFirstSuccessResponse(op.getResponses());
+ if (responseType == null && success != null &&
success.getValue().getContent() != null
+ && !success.getValue().getContent().isEmpty()) {
+ responseType =
success.getValue().getContent().keySet().iterator().next();
+ }
+ }
+ if (BODY_VERBS.contains(method)) {
+ withoutFile.put(method, "echoes the request body with HTTP
200");
+ continue;
+ }
+ ApiResponse ok = op.getResponses() != null ?
op.getResponses().get("200") : null;
+ String opExample = firstExample(ok);
+ if (opExample != null) {
+ withoutFile.put(method, "answers the example of its '200'
response with HTTP 200");
+ if (example == null) {
+ example = opExample;
+ }
+ } else if (ok != null || op.getResponses() == null) {
+ withoutFile.put(method, "answers HTTP 204 with an empty
body");
+ } else {
+ // camel-rest-openapi reads the '200' response of the
operation without checking it exists
+ withoutFile.put(method, "fails with an error, as the
operation has no '200' response: add the file");
+ }
+ }
+
+ String ext = responseType != null && responseType.contains("xml")
? ".xml" : ".json";
+ StringBuilder note = new StringBuilder("Served with HTTP 200 for
any method of ").append(path)
+ .append(" without a route.");
+ if (path.contains("{")) {
+ String sample = path.replaceAll("\\{[^}]+}", "1");
+ note.append(" Camel matches the request path, so replace each
{parameter} with a value:"
+ + " one file per value (camel-mock")
+ .append(sample).append(ext).append(" answers
").append(sample).append(").");
+ }
+ note.append(" Without the file, ");
+ withoutFile.forEach((method, answer) ->
note.append(method).append(' ').append(answer).append("; "));
+ note.setLength(note.length() - 2);
+ note.append('.');
+
+ files.add(new MockFileInfo("camel-mock" + path + ext, path,
operationIds, example, note.toString()));
Review Comment:
🐛 **Root path `/` produces a hidden file**
When an OpenAPI spec includes a path `"/"`, this expression produces
`"camel-mock" + "/" + ".json"` = `"camel-mock/.json"`, which is a hidden file
on Unix (the filename starts with `.` after the directory separator). Most file
managers, editors, and `ls` without `-a` will silently ignore it. Consider
stripping the leading `/` and replacing empty strings with a placeholder:
```suggestion
files.add(new MockFileInfo("camel-mock/" + (path.equals("/") ?
"root" : path.substring(1).replace('/', '-')) + ext, path, operationIds,
example, note.toString()));
```
--
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]
