Croway commented on code in PR #27960:
URL: https://github.com/apache/camel/pull/27960#discussion_r4300241210
##########
dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/OpenApiTools.java:
##########
@@ -407,6 +307,107 @@ 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");
+ }
Review Comment:
Done in b66295875a5d with your text: the two branches are one `else`, and
the PR description no longer lists CAMEL-25663 as out of scope. Checked
`loadMockData` on the rebased branch: a missing `200` response gives a null
content, and the answer is HTTP 204 with an empty body. Test
`scaffoldNamesThePathParameterMockFileWithASampleValue` covers a GET with only
a `201` response.
_Claude Code on behalf of Croway_
##########
dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/OpenApiTools.java:
##########
@@ -407,6 +307,107 @@ 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('.');
+
+ // the component matches a resource whose location without
extension ends with the request path without its
+ // leading slash; for the root path that is the empty string, so
any mock file matches
+ String fileName = "/".equals(path) ? "camel-mock/index" + ext :
"camel-mock" + path + ext;
Review Comment:
Done in b66295875a5d: `filePath` carries the sample value the note already
used, `camel-mock/pets/1.json`, and `requestPath` keeps the template; the note
says the file name carries the value the request uses, one file per value, and
the `MockFileInfo` javadoc says the same. The MCP page now reads "a path
parameter carries a sample value, such as `camel-mock/pets/1.json` for
`/pets/{petId}`, which answers `/pets/1`". `OpenApiScaffoldRestOpenApiTest`
writes the file at `filePath()` as given, without the `replace`, and requests
`/v1/pets/1`.
_Claude Code on behalf of Croway_
##########
dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/OpenApiTools.java:
##########
@@ -407,6 +307,107 @@ 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");
Review Comment:
Done in b66295875a5d, in the per-method note and in the mode explanation:
the example of the `200` response is answered when the request's Content-Type
or Accept header names a media type of that response (Accept `*/*` takes
application/json, then application/xml), otherwise HTTP 204 with an empty body,
a request without an Accept header included. The MCP page has the same sentence.
_Claude Code on behalf of Croway_
##########
dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/OpenApiTools.java:
##########
@@ -407,6 +307,107 @@ 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");
Review Comment:
Done in b66295875a5d: one `else` branch, "answers HTTP 204 with an empty
body", with a comment naming CAMEL-25663.
_Claude Code on behalf of Croway_
##########
dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/OpenApiTools.java:
##########
@@ -407,6 +307,107 @@ 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.");
Review Comment:
Done in b66295875a5d (same change as the thread above).
_Claude Code on behalf of Croway_
##########
dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/OpenApiTools.java:
##########
@@ -543,22 +488,16 @@ public record ScaffoldResult(
int operationCount,
String specFilename,
String missingOperation,
- String apiTitle) {
+ String apiTitle,
+ String modeExplanation,
+ List<MockFileInfo> mockFiles) {
}
public record MockFileInfo(
String filePath,
Review Comment:
Done in b66295875a5d, the other way round: `filePath` now carries the sample
value (`camel-mock/pets/1.json`), so it is the file to write as given, and
`requestPath` keeps the template; the `MockFileInfo` javadoc documents both,
and the test no longer substitutes the parameter.
_Claude Code on behalf of Croway_
##########
dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/OpenApiTools.java:
##########
@@ -407,6 +307,107 @@ 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");
Review Comment:
Done in b66295875a5d: the note and the mode explanation say which media type
is taken (the request's Content-Type or Accept header, with the `*/*` fallback
order) and that any other request gets HTTP 204.
_Claude Code on behalf of Croway_
--
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]
