gnodet-bot commented on code in PR #28586:
URL: https://github.com/apache/camel/pull/28586#discussion_r4397482892
##########
components/camel-diagram/src/main/java/org/apache/camel/diagram/RouteDiagramLayoutEngine.java:
##########
@@ -564,9 +620,82 @@ public LayoutRoute layoutRoute(RouteInfo route, int
startY) {
lr.maxX = extent.maxX + overhang;
lr.maxY = Math.max(lr.maxY, extent.maxY);
+ if (problems) {
+ layoutProblems(lr);
+ }
return lr;
}
+ private void layoutProblems(LayoutRoute lr) {
+ // in the order of the steps in the route (the order of the source)
+ List<LayoutNode> withProblems = lr.nodes.stream()
+ .filter(ln -> !ln.tableRow && ln.treeNode != null &&
!ln.treeNode.info.problems.isEmpty())
+ .toList();
+ if (withProblems.isEmpty()) {
+ return;
+ }
+ int lineHeight = fontMetrics.getHeight();
+ lr.problemList = lr.nodes.stream().anyMatch(ln ->
isBranchingEip(ln.type));
+ if (!lr.problemList) {
+ // a column of callouts beside the route, each at the height of
its step, below the one above it
+ int x = lr.maxX + CALLOUT_GAP;
+ int width = calloutWidth(nodeWidth);
+ int bottom = Integer.MIN_VALUE;
+ // without branches the order of the steps is from the top down
+ for (LayoutNode ln : withProblems) {
+ Callout c = new Callout();
+ c.node = ln;
+ c.lines = problemLines(ln.treeNode.info.problems, width - 2 *
CALLOUT_PAD);
+ c.x = x;
+ c.width = width;
+ c.height = c.lines.size() * lineHeight + 2 * CALLOUT_PAD;
+ c.y = Math.max(ln.y + ln.height / 2 - c.height / 2, bottom +
CALLOUT_PAD);
+ bottom = c.y + c.height;
+ lr.callouts.add(c);
+ lr.maxY = Math.max(lr.maxY, bottom);
+ }
+ lr.maxX = x + width;
+ } else {
+ // a numbered list below the steps, as wide as the route
+ int width = Math.max(lr.maxX - PADDING, nodeWidth * 2);
+ int y = lr.maxY + V_GAP / 2;
+ int number = 0;
+ for (LayoutNode ln : withProblems) {
+ Callout c = new Callout();
+ c.node = ln;
+ c.number = ++number;
+ c.lines = problemLines(ln.treeNode.info.problems, width -
PROBLEM_NUMBER_WIDTH);
+ c.x = PADDING;
+ c.y = y;
+ c.width = width;
+ c.height = c.lines.size() * lineHeight;
+ y += c.height + CALLOUT_PAD;
+ lr.callouts.add(c);
+ }
+ lr.maxY = y;
+ lr.maxX = Math.max(lr.maxX, PADDING + width);
+ }
+ }
+
+ /**
+ * The problems wrapped to the width, each from a line of its own, at most
six lines: a problem that does not fit
+ * has the rest of its text on its last line, which the renderer cuts with
an ellipsis, and the problems after it
+ * are left out, which an ellipsis says.
+ */
+ List<String> problemLines(List<String> problems, int width) {
+ List<String> lines = new ArrayList<>();
+ for (String problem : problems) {
+ int room = MAX_CALLOUT_LINES - lines.size();
+ if (room == 0) {
+ int last = lines.size() - 1;
+ lines.set(last, lines.get(last) + " …");
Review Comment:
💬 **Note (contract):** When `room == 0`, the ellipsis is appended to the
last line of the *previous* problem's wrapped text. If that problem happened to
fill `wrapLabel` exactly to `width` characters, this line now exceeds the
limit. No functional bug — `fit()` in the renderer truncates it visually — but
the contract implicitly relies on the renderer always calling `fit()`. A brief
Javadoc note on `problemLines` documenting this behaviour would help future
maintainers.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/action/RouteDiagramProblems.java:
##########
@@ -0,0 +1,153 @@
+/*
+ * 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.action;
+
+import java.io.IOException;
+import java.nio.charset.CharacterCodingException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Locale;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.camel.catalog.CamelCatalog;
+import org.apache.camel.catalog.DefaultCamelCatalog;
+import org.apache.camel.diagram.RouteDiagramLayoutEngine.NodeInfo;
+import org.apache.camel.diagram.RouteDiagramLayoutEngine.RouteInfo;
+import org.apache.camel.dsl.jbang.core.commands.ai.RouteAssist;
+import org.apache.camel.dsl.jbang.core.commands.ai.SourceValidator;
+
+/**
+ * The problems of the source files of a route diagram, on the steps they are
of: the checks of {@code camel validate}
+ * for a YAML route, and for Java and XML the checks that read the source
without compiling it ({@link RouteAssist}), as
+ * the route diagram reads Java routes without compiling them.
+ */
+final class RouteDiagramProblems {
+
+ /** A problem of a line of the source; line 0 for a problem of the file. */
+ record Problem(int line, String text) {
+ }
+
+ private static final Pattern MESSAGE_LINE = Pattern.compile("^Line (\\d+):
(.*)$", Pattern.DOTALL);
+ // the step and route a message names first, which the diagram shows
already: "setBody: route orders: "
+ private static final Pattern STEP_PREFIX =
Pattern.compile("^[A-Za-z][\\w-]*: route [^:\\s]+: ");
+
+ private RouteDiagramProblems() {
+ }
+
+ /**
+ * Puts the problems of the source files on the steps of their routes
({@link NodeInfo#problems}): a problem on the
+ * step whose line is the last one before it (the line of the option or
expression that is wrong is inside its
+ * step), a problem of the file on the first step: also that the file
cannot be read to check it.
+ */
+ static void attach(List<RouteInfo> routes, List<String> files) {
+ CamelCatalog catalog = null;
+ for (String file : files) {
+ Path path = Path.of(file);
+ if (!Files.isRegularFile(path)) {
+ continue;
+ }
+ String name = path.getFileName().toString();
+ List<NodeInfo> nodes = routes.stream()
+ .filter(r -> sourceOf(r, name))
+ .flatMap(r -> r.nodes.stream())
+ .toList();
+ if (nodes.isEmpty()) {
+ continue;
+ }
+ String content;
+ try {
+ content = Files.readString(path);
+ } catch (IOException e) {
+ // the diagram shows the routes of the file, so it says why
their problems are not there
+ add(nodes.get(0), "the file cannot be read to check it: "
+ + (e instanceof CharacterCodingException ?
"it is not UTF-8 text" : e.getMessage()));
+ continue;
+ }
+ if (catalog == null) {
+ catalog = new DefaultCamelCatalog();
+ }
+ for (Problem p : problems(name, content, catalog,
path.toAbsolutePath().getParent())) {
+ add(stepAt(nodes, p.line()), p.text());
+ }
+ }
+ }
+
+ private static void add(NodeInfo node, String problem) {
+ List<String> list = new ArrayList<>(node.problems);
+ list.add(problem);
+ node.problems = List.copyOf(list);
+ }
+
+ /** The problems of a source file. */
+ static List<Problem> problems(String name, String content, CamelCatalog
catalog, Path directory) {
+ String lower = name.toLowerCase(Locale.ROOT);
+ List<Problem> answer = new ArrayList<>();
+ if (lower.endsWith(".yaml") || lower.endsWith(".yml")) {
+ for (String msg : SourceValidator.validate(name, content, catalog,
null, directory)) {
+ answer.add(problem(msg));
+ }
+ } else if (RouteAssist.supports(name, content)) {
+ for (RouteAssist.Diagnostic d : RouteAssist.diagnostics(name,
content, catalog, directory)) {
Review Comment:
⚠️ **Exception safety:** `SourceValidator.validate` and
`RouteAssist.diagnostics` can throw unexpected `RuntimeException`s (e.g. a
malformed YAML file hitting a parser bug). Neither call is wrapped in a
try-catch here, so the exception would propagate up through `problems()` →
`attach()` and crash the entire diagram command. The `IOException` path in
`attach()` already converts I/O errors into a user-visible problem on the first
step — a `catch (Exception e)` around the `for (Problem p : problems(...))`
call site would give the same graceful treatment to validator crashes.
--
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]
