gnodet-bot commented on code in PR #26800: URL: https://github.com/apache/camel/pull/26800#discussion_r4084426505
########## dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/BodyTypeFlow.java: ########## @@ -0,0 +1,448 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.dsl.yaml.validator; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import com.fasterxml.jackson.databind.JsonNode; +import com.networknt.schema.Error; +import com.networknt.schema.path.NodePath; + +/** + * Where the body comes from, across the routes of a file. + * <p/> + * A step that reads the message body - a jsonpath, jq or xpath expression - fails at runtime when there is no body. A + * route reached with {@code direct:} has the body of its caller, so the question is not answered inside one route: the + * routes of the file form a graph through their {@code direct:} and {@code seda:} endpoints, the way + * {@code DefaultRouteTopologyDumper} builds it from the route definitions at runtime, and the answer follows the edges + * (CAMEL-24844). + * <p/> + * This first pass reports one thing, and only when it is certain: a route that reads the body although nothing in it, + * or in any route that calls it, ever sets one. It does not claim to know the type - a POST carries a body that no + * route sets - it reports that the file itself never produces one. + */ +final class BodyTypeFlow { + + /** The expressions that read the message body and fail when there is none. */ + private static final Set<String> READS_THE_BODY = Set.of("jsonpath", "jq", "xpath", "xquery", "xtokenize"); + + /** The steps that work on the body itself, and have nothing to work on when there is none. */ + private static final Set<String> STEPS_THAT_NEED_THE_BODY = Set.of("unmarshal", "marshal", "convertBodyTo"); + + /** Steps that put something in the body, whatever it is. */ + private static final Set<String> SETS_THE_BODY = Set.of("setBody", "transform", "unmarshal", "marshal", + "convertBodyTo", "convertVariableTo", "poll", "pollEnrich", "enrich", "process", "bean", "to", "toD", + "recipientList", "serviceCall", "claimCheck", "aggregate", "split", "loadBalance", "removeBody"); + + /** The REST verbs that carry no body, so a route they send to starts with none. */ + private static final Set<String> VERBS_WITHOUT_BODY = Set.of("get", "delete", "head"); + + /** The REST verbs that carry one. */ + private static final Set<String> VERBS_WITH_BODY = Set.of("post", "put", "patch"); Review Comment: ⚠️ **Unused constant** — `VERBS_WITH_BODY` is never referenced in any method. The logic that distinguishes verbs-with-body (POST/PUT/PATCH, which carry a body so no certainty claim is possible) is implemented by the `VERBS_WITHOUT_BODY` allowlist — everything not in that set is treated as body-bearing. `VERBS_WITH_BODY` is redundant and dead. ```suggestion ``` ########## dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/RouteGraph.java: ########## @@ -0,0 +1,154 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.dsl.yaml.validator; + +import java.util.ArrayList; +import java.util.List; +import java.util.Set; + +import com.fasterxml.jackson.databind.JsonNode; + +/** + * The routes of a file and the endpoints between them: what the route topology is at runtime, read from the source. + * <p/> + * {@code DefaultRouteTopologyDumper} builds the same graph from the route definitions of a running context, by indexing + * each route's input and matching the outputs against it. Here the routes are still text, so an endpoint has to be + * recognised in both the forms the YAML DSL allows (CAMEL-24844). + */ +final class RouteGraph { Review Comment: ⚠️ **Dead class** — `RouteGraph` is never referenced from any other class in this PR. Its public static methods (`routes()`, `endpointOf()`, `normalize()`, `scheme()`, `sendsTo()`) are all duplicated verbatim as private statics inside `BodyTypeFlow`. Either: - delete `RouteGraph` and keep `BodyTypeFlow`'s private copies, or - make `BodyTypeFlow` delegate to `RouteGraph` (replace the private copies with calls to the shared utility) As-is this class will silently drift from `BodyTypeFlow` whenever either is updated. ########## dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java: ########## @@ -130,6 +130,14 @@ public List<Error> validate(File file) throws Exception { } public List<Error> validate(String content) throws Exception { + return validate(content, java.util.Set.of()); + } + + /** + * @param bodylessEndpoints endpoints the caller knows deliver no body, such as the {@code direct:} endpoint of a + * GET operation of an OpenAPI specification the file binds to (CAMEL-24844) + */ + public List<Error> validate(String content, java.util.Set<String> bodylessEndpoints) throws Exception { Review Comment: 💡 Same: `java.util.Set<String>` → `Set<String>` (already imported). ```suggestion public List<Error> validate(String content, Set<String> bodylessEndpoints) throws Exception { ``` ########## dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/BodyTypeFlow.java: ########## @@ -0,0 +1,448 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.dsl.yaml.validator; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import com.fasterxml.jackson.databind.JsonNode; +import com.networknt.schema.Error; +import com.networknt.schema.path.NodePath; + +/** + * Where the body comes from, across the routes of a file. + * <p/> + * A step that reads the message body - a jsonpath, jq or xpath expression - fails at runtime when there is no body. A + * route reached with {@code direct:} has the body of its caller, so the question is not answered inside one route: the + * routes of the file form a graph through their {@code direct:} and {@code seda:} endpoints, the way + * {@code DefaultRouteTopologyDumper} builds it from the route definitions at runtime, and the answer follows the edges + * (CAMEL-24844). + * <p/> + * This first pass reports one thing, and only when it is certain: a route that reads the body although nothing in it, + * or in any route that calls it, ever sets one. It does not claim to know the type - a POST carries a body that no + * route sets - it reports that the file itself never produces one. + */ +final class BodyTypeFlow { + + /** The expressions that read the message body and fail when there is none. */ + private static final Set<String> READS_THE_BODY = Set.of("jsonpath", "jq", "xpath", "xquery", "xtokenize"); + + /** The steps that work on the body itself, and have nothing to work on when there is none. */ + private static final Set<String> STEPS_THAT_NEED_THE_BODY = Set.of("unmarshal", "marshal", "convertBodyTo"); + + /** Steps that put something in the body, whatever it is. */ + private static final Set<String> SETS_THE_BODY = Set.of("setBody", "transform", "unmarshal", "marshal", + "convertBodyTo", "convertVariableTo", "poll", "pollEnrich", "enrich", "process", "bean", "to", "toD", + "recipientList", "serviceCall", "claimCheck", "aggregate", "split", "loadBalance", "removeBody"); + + /** The REST verbs that carry no body, so a route they send to starts with none. */ + private static final Set<String> VERBS_WITHOUT_BODY = Set.of("get", "delete", "head"); + + /** The REST verbs that carry one. */ + private static final Set<String> VERBS_WITH_BODY = Set.of("post", "put", "patch"); + + /** The consumers that produce no body of their own, so the message reaching the route has none. */ + private static final Set<String> NO_BODY_CONSUMER = Set.of("timer", "quartz", "scheduler", "cron"); + + /** The consumers that hand a route a body of their own. */ + private static final Set<String> BODY_FROM_OUTSIDE = Set.of("file", "ftp", "ftps", "sftp", "smb", "kafka", "jms", + "activemq", "amqp", "sqs", "sns", "aws2-sqs", "aws2-s3", "mail", "imap", "pop3", "stream", "netty", + "mllp", "micrometer", "paho", "mqtt", "rabbitmq", "pulsar", "nats", "azure-servicebus", "google-pubsub"); + + private BodyTypeFlow() { + } + + /** One route of the file: where it starts, what it does, and where it sends. */ + private record Route(String id, String fromUri, JsonNode steps, JsonNode node) { + } + + static void check(JsonNode target, NodePath path, List<Error> errors) { + check(target, path, errors, Set.of()); + } + + /** + * @param known endpoints the caller knows deliver no body, such as {@code direct:getStock} for the GET operation of Review Comment: 💡 Javadoc nit: `@param known endpoints the caller knows…` reads oddly — `endpoints` should be preceded by `the`. ```suggestion * @param known the endpoints the caller knows deliver no body, such as {@code direct:getStock} for the GET operation of ``` ########## dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/BodyTypeFlow.java: ########## @@ -0,0 +1,448 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.dsl.yaml.validator; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import com.fasterxml.jackson.databind.JsonNode; +import com.networknt.schema.Error; +import com.networknt.schema.path.NodePath; + +/** + * Where the body comes from, across the routes of a file. + * <p/> + * A step that reads the message body - a jsonpath, jq or xpath expression - fails at runtime when there is no body. A + * route reached with {@code direct:} has the body of its caller, so the question is not answered inside one route: the + * routes of the file form a graph through their {@code direct:} and {@code seda:} endpoints, the way + * {@code DefaultRouteTopologyDumper} builds it from the route definitions at runtime, and the answer follows the edges + * (CAMEL-24844). + * <p/> + * This first pass reports one thing, and only when it is certain: a route that reads the body although nothing in it, + * or in any route that calls it, ever sets one. It does not claim to know the type - a POST carries a body that no + * route sets - it reports that the file itself never produces one. + */ +final class BodyTypeFlow { + + /** The expressions that read the message body and fail when there is none. */ + private static final Set<String> READS_THE_BODY = Set.of("jsonpath", "jq", "xpath", "xquery", "xtokenize"); + + /** The steps that work on the body itself, and have nothing to work on when there is none. */ + private static final Set<String> STEPS_THAT_NEED_THE_BODY = Set.of("unmarshal", "marshal", "convertBodyTo"); + + /** Steps that put something in the body, whatever it is. */ + private static final Set<String> SETS_THE_BODY = Set.of("setBody", "transform", "unmarshal", "marshal", + "convertBodyTo", "convertVariableTo", "poll", "pollEnrich", "enrich", "process", "bean", "to", "toD", + "recipientList", "serviceCall", "claimCheck", "aggregate", "split", "loadBalance", "removeBody"); + + /** The REST verbs that carry no body, so a route they send to starts with none. */ + private static final Set<String> VERBS_WITHOUT_BODY = Set.of("get", "delete", "head"); + + /** The REST verbs that carry one. */ + private static final Set<String> VERBS_WITH_BODY = Set.of("post", "put", "patch"); + + /** The consumers that produce no body of their own, so the message reaching the route has none. */ + private static final Set<String> NO_BODY_CONSUMER = Set.of("timer", "quartz", "scheduler", "cron"); + + /** The consumers that hand a route a body of their own. */ + private static final Set<String> BODY_FROM_OUTSIDE = Set.of("file", "ftp", "ftps", "sftp", "smb", "kafka", "jms", + "activemq", "amqp", "sqs", "sns", "aws2-sqs", "aws2-s3", "mail", "imap", "pop3", "stream", "netty", + "mllp", "micrometer", "paho", "mqtt", "rabbitmq", "pulsar", "nats", "azure-servicebus", "google-pubsub"); Review Comment: ⚠️ **Unused constant** — `BODY_FROM_OUTSIDE` is never referenced in any method. The handling for file/ftp/kafka/etc. consumers is already correct via the fallthrough in `certainlyWithoutABody`: a route whose scheme is not in `NO_BODY_CONSUMER` and is not `direct`/`seda`/`direct-vm` returns `false` (unknown consumer, assumed to bring a body). The constant adds no behaviour and will mislead readers into thinking these consumers are explicitly tracked. ```suggestion ``` ########## dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java: ########## @@ -130,6 +130,14 @@ public List<Error> validate(File file) throws Exception { } public List<Error> validate(String content) throws Exception { + return validate(content, java.util.Set.of()); Review Comment: 💡 `java.util.Set` is already imported in this file — use the simple name. ```suggestion return validate(content, Set.of()); ``` -- 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]
