gnodet-bot commented on code in PR #27960:
URL: https://github.com/apache/camel/pull/27960#discussion_r4297693069
##########
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:
⚠️ **Stale advisory text:** Since CAMEL-25663, `loadMockData` handles
`getResponses().get("200") == null` gracefully by returning HTTP 204 with an
empty body (see `DefaultRestOpenapiProcessorStrategy`, lines 336–387). The
message `"fails with an error, as the operation has no '200' response"` no
longer reflects runtime behavior. Update to something like:
```suggestion
withoutFile.put(method, "answers HTTP 204 with an empty
body (no '200' response defined)");
```
##########
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:
🔍 **Nit:** `firstExample()` picks the first non-null example across all
media types without content-type matching, while `loadMockData` does content
negotiation at runtime. The description `"answers the example of its '200'
response with HTTP 200"` conflates the two. Consider `"answers an example of
its '200' response with HTTP 200 (content type matched by the request)"`.
##########
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:
💡 **Naming:** `filePath` for parametric routes holds the OpenAPI template
(`camel-mock/pets/{petId}.json`), not the ready-to-use path. The test
explicitly calls `.replace("{petId}", "42")` on it before writing the file.
Consider renaming to `filePathTemplate`, or at least add a Javadoc comment:
`/** OpenAPI path template — callers must substitute path parameters (e.g.
{petId}) before using this as a filesystem path. */`
--
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]
