davsclaus commented on code in PR #26920: URL: https://github.com/apache/camel/pull/26920#discussion_r4114649160
########## dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/EndpointConsumerChecks.java: ########## @@ -0,0 +1,128 @@ +/* + * 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.jbang.core.commands.ai; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.apache.camel.dsl.yaml.validator.EndpointConsumers; + +/** + * A {@code direct:} or {@code seda:} endpoint a YAML route sends to, and no route of the application consumes + * (CAMEL-24955). The routes of an application are spread over the files of its directory, so the endpoints the other + * route files consume - YAML, Java and XML - are read from them and handed to the check. + */ +public final class EndpointConsumerChecks { + + /** A Java DSL route input: from( not called on something else, so not Instant.from( or List.from(. */ + private static final Pattern JAVA_FROM = Pattern.compile("(?<![.\\w])from\\s*\\(\\s*(\"([^\"]*)\")?"); + private static final Pattern XML_FROM = Pattern.compile("<from\\s[^>]*?\\buri\\s*=\\s*[\"']([^\"']*)[\"']"); + + private EndpointConsumerChecks() { + } + + /** + * @param content the YAML route file + * @param directory the directory of the application's route files; null says nothing + * @param excludeFile the file being validated, whose routes come from the content + * @return the messages, one per endpoint no route consumes + */ + public static List<String> validateYamlConsumers(String content, Path directory, String excludeFile) { + return EndpointConsumers.check(content, consumed(directory, excludeFile)); + } + + /** + * The {@code direct:} and {@code seda:} endpoints the route files of the directory consume, leaving out the file + * being validated; null when they cannot be known: no directory, or a route input the scan cannot read (a Java + * {@code from(} with no literal, a placeholder), which could be any endpoint. + */ + static Set<String> consumed(Path directory, String excludeFile) { + if (directory == null || !Files.isDirectory(directory)) { + return null; + } + Set<String> answer = new HashSet<>(); + try (var stream = Files.list(directory)) { Review Comment: This lists only the top level of `directory`. `camel_write_file` passes the project directory, and `file` may be `routes/a.camel.yaml` (subdirectories are allowed), so consumers in `routes/` are not scanned, and `excludeFile` (the relative path) never matches a file name here. The Maven layout (`src/main/resources/camel/*.yaml` with RouteBuilders under `src/main/java`) has the same gap. Could it use the validated file's own parent, or stay quiet when the layout is not a flat directory? ########## dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/EndpointConsumers.java: ########## @@ -0,0 +1,150 @@ +/* + * 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.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; + +import static org.apache.camel.dsl.yaml.validator.RouteGraph.Route; +import static org.apache.camel.dsl.yaml.validator.RouteGraph.normalize; +import static org.apache.camel.dsl.yaml.validator.RouteGraph.routes; +import static org.apache.camel.dsl.yaml.validator.RouteGraph.scheme; +import static org.apache.camel.dsl.yaml.validator.RouteGraph.sendsTo; + +/** + * A {@code direct:} or {@code seda:} endpoint a route sends to, and no route of the application consumes (CAMEL-24955). + * With {@code direct:} the route fails to start - <i>No consumers available on endpoint</i>; with {@code seda:} nothing + * fails, and the message is queued and never read. + * <p/> + * The routes of an application are spread over files, so a file on its own cannot answer: the caller scans the other + * route files of the directory and passes what they consume. Without them the check says nothing. + */ +public final class EndpointConsumers { + + /** The components whose consumer is a route of the same application. */ + private static final Set<String> CHECKED = Set.of("direct", "seda"); + + private static final ObjectMapper MAPPER = new ObjectMapper(new YAMLFactory()); + + private EndpointConsumers() { + } + + /** + * The {@code direct:} and {@code seda:} endpoints the routes of a YAML file consume, without their options. Empty + * when the file is not YAML the routes can be read from; null when a route consumes an endpoint only known at + * runtime ({@code from: direct:{{name}}}), which could be any of them. + */ + public static Set<String> consumed(String yaml) { + JsonNode target = read(yaml); + return target != null ? consumed(routes(target)) : Set.of(); + } + + /** + * @param yaml the YAML DSL source + * @param consumedElsewhere the endpoints the other route files of the application consume, as returned by + * {@link #consumed(String)}; null when they are not known, which keeps the check quiet + * @return a message for each endpoint a route sends to and no route consumes + */ + public static List<String> check(String yaml, Set<String> consumedElsewhere) { + if (consumedElsewhere == null) { + return List.of(); + } + JsonNode target = read(yaml); + if (target == null) { + return List.of(); + } + List<Route> routes = routes(target); + Set<String> own = consumed(routes); + if (own == null) { + return List.of(); + } + Set<String> consumed = new HashSet<>(consumedElsewhere); + consumed.addAll(own); + Set<String> messages = new LinkedHashSet<>(); + for (Route r : routes) { + for (String uri : sendsTo(r.steps())) { + String endpoint = endpoint(uri); + if (endpoint == null || consumed.contains(endpoint)) { + continue; + } + messages.add((r.id() != null ? "route " + r.id() + ": " : "") + + "sends to " + endpoint + ", and no route consumes it - not in this file, nor in the" + + " other route files of the directory: " + + ("direct".equals(scheme(endpoint)) + ? "the route fails to start with No consumers available on endpoint" + : "nothing fails, the messages are queued and never read") + + "; add a route with from: " + endpoint + ", or correct the name"); + } + } + return new ArrayList<>(messages); + } + + private static Set<String> consumed(List<Route> routes) { Review Comment: `RouteGraph.routes()` only returns `route` and top-level `from` entries, so `routeTemplate` / `templatedRoute` (and kamelets) are invisible here. A file with a `routeTemplate` whose `from` is `direct:{{name}}`, a `templatedRoute` with `name: lookup`, and a route doing `to: direct:lookup` is reported as "fails to start", and the placeholder safeguard never triggers because the template's `from` is never read. Suggest returning null (staying quiet) when the file has a `routeTemplate`/`templatedRoute`, or reading the template's `from` as a consumer, plus a test. ########## dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/EndpointConsumerChecks.java: ########## @@ -0,0 +1,128 @@ +/* + * 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.jbang.core.commands.ai; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.apache.camel.dsl.yaml.validator.EndpointConsumers; + +/** + * A {@code direct:} or {@code seda:} endpoint a YAML route sends to, and no route of the application consumes + * (CAMEL-24955). The routes of an application are spread over the files of its directory, so the endpoints the other + * route files consume - YAML, Java and XML - are read from them and handed to the check. + */ +public final class EndpointConsumerChecks { + + /** A Java DSL route input: from( not called on something else, so not Instant.from( or List.from(. */ + private static final Pattern JAVA_FROM = Pattern.compile("(?<![.\\w])from\\s*\\(\\s*(\"([^\"]*)\")?"); Review Comment: Some Java consumers slip through as "known" instead of "unknown", which gives false positives: - `routeTemplate("t").templateParameter("n").from("direct:{{n}}")`: the `(?<![.\\w])` lookbehind skips `.from(` - `fromF("direct:%s", name)` does not match at all - `from("direct:" + NAME)` captures `"direct:"`, `endpoint()` returns null, and `add()` returns true, so the file counts as consuming nothing Maybe return null when a `routeTemplate(` / `fromF(` is present, or when a `direct:`/`seda:` literal gives no endpoint. ########## dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/SourceValidator.java: ########## @@ -114,6 +114,8 @@ public static List<String> validate( msgs.addAll(validateYamlBeanRefs(content, declarations, catalog)); msgs.addAll(validateResourceRefs(content, directory)); msgs.addAll(GroovyImportChecks.validateYamlGroovyImports(content, null, declarations.javaClasses())); + // a direct: or seda: endpoint no route of the application consumes (CAMEL-24955) + msgs.addAll(EndpointConsumerChecks.validateYamlConsumers(content, directory, fileName)); Review Comment: `SourceValidator.validate` is also the gate of `camel_write_file` / `camel_edit_file` (`AuthoringTools.writeFile`, always `validate=true`) and of the TUI `McpFacade.writeFile`: any message means "The file was not written". With this check an agent cannot write `main.camel.yaml` (with `to: direct:lookup`) before `lookup.camel.yaml`, and two files that call each other's `direct:` endpoints can never be written through the tools. A missing consumer is often "not written yet" rather than invalid content. Could this be a non-blocking warning, or be left out of the write 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]
