gnodet-bot commented on code in PR #29397: URL: https://github.com/apache/camel/pull/29397#discussion_r4504007522
########## dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/ChaosSuggestList.java: ########## @@ -0,0 +1,171 @@ +/* + * 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.tui; + +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.List; + +import dev.tamboui.layout.Constraint; +import dev.tamboui.layout.Layout; +import dev.tamboui.layout.Rect; +import dev.tamboui.style.Overflow; +import dev.tamboui.style.Style; +import dev.tamboui.terminal.Frame; +import dev.tamboui.text.Line; +import dev.tamboui.text.Span; +import dev.tamboui.text.Text; +import dev.tamboui.tui.event.KeyEvent; +import dev.tamboui.widgets.Clear; +import dev.tamboui.widgets.block.Block; +import dev.tamboui.widgets.block.BorderType; +import dev.tamboui.widgets.block.Borders; +import dev.tamboui.widgets.paragraph.Paragraph; +import org.apache.camel.util.json.JsonObject; + +/** + * What to try ({@code w} on the Chaos screen, CAMEL-26727): the rules the integration suggests per route from what its + * routes do, the most worth trying first, each with why. Enter types the selected rule in, to change or add. + */ +final class ChaosSuggestList { + + /** What a key did to the list. */ + enum Outcome { + NONE, + CLOSED, + PICKED + } + + /** A suggestion, as the chaos action of the integration gives it. */ + record Item(String route, String kind, String title, String rule, String why, boolean tried) { + } + + private final String name; + private final List<Item> items = new ArrayList<>(); + private int selected; + + ChaosSuggestList(String name, JsonObject reply) { + this.name = name; + if (reply != null && reply.getCollection("suggestions") != null) { + for (Object o : reply.getCollection("suggestions")) { + if (o instanceof JsonObject jo && jo.getString("rule") != null) { + items.add(new Item( + jo.getString("route"), jo.getString("kind"), jo.getString("title"), + jo.getString("rule"), jo.getString("why"), jo.getBooleanOrDefault("tried", false))); + } + } + } + } + + List<Item> items() { + return items; + } + + /** The selected suggestion, or null when there are none. */ + Item selected() { + return items.isEmpty() ? null : items.get(selected); + } + + Outcome handleKey(KeyEvent ke) { + if (ke.isCancel()) { + return Outcome.CLOSED; + } + if (ke.isUp() && selected > 0) { + selected--; + } else if (ke.isDown() && selected < items.size() - 1) { + selected++; + } else if (ke.isConfirm() && !items.isEmpty()) { + return Outcome.PICKED; + } + return Outcome.NONE; + } + + void render(Frame frame, Rect area) { + int width = Math.min(area.width() - 4, 150); + int height = Math.min(area.height() - 2, 30); + Rect popup = DialogHelper.centered(area, width, height); + frame.renderWidget(Clear.INSTANCE, popup); + Block outer = Block.builder().borderType(BorderType.ROUNDED).borders(Borders.ALL).borderStyle(Theme.warning()) + .title(" What to try in " + name + " ").build(); + frame.renderWidget(outer, popup); + Rect inner = outer.inner(popup); + if (items.isEmpty()) { + frame.renderWidget(Paragraph.builder().overflow(Overflow.WRAP_WORD).text(Text.from(List.of(Line.empty(), + Line.from(Span.styled(" Nothing to suggest: the routes call nothing remote, and have no input from" + + " outside, dead letter channel or hand-off to another route", + Theme.muted()))))) + .build(), + inner); + return; + } + List<Rect> cols = Layout.horizontal() + .constraints(Constraint.length(Math.min(56, inner.width() / 2)), Constraint.fill()) + .split(inner); + renderList(frame, cols.get(0)); + renderDetail(frame, cols.get(1)); + } + + private void renderList(Frame frame, Rect area) { + List<Line> lines = new ArrayList<>(); + lines.add(Line.from(Span.styled(" The most worth trying first", Style.EMPTY.bold()))); + int selectedLine = 0; + for (int i = 0; i < items.size(); i++) { + Item item = items.get(i); + boolean sel = i == selected; + if (sel) { + selectedLine = lines.size(); + } + Style style = sel ? Theme.selectionBg().bold() : Style.EMPTY; + List<Span> spans = new ArrayList<>(); + spans.add(Span.styled((sel ? " > " : " ") + item.title(), style)); + spans.add(Span.styled(" " + item.route(), Theme.muted())); + if (item.tried()) { + spans.add(Span.styled(" tried", Theme.success())); + } Review Comment: ⚠️ **Dead code — `item.tried()` is always `false`:** Since `ChaosManager.suggest()` never sets the `"tried"` key in the JSON, `getBooleanOrDefault("tried", false)` (line 68) always returns `false`. This `if (item.tried())` block and the "tried" `Span` are never rendered. ########## components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosManager.java: ########## @@ -397,6 +397,33 @@ public boolean hasRule(String id) { } } + /** + * What to try (CAMEL-26727): rules suggested per route from what the routes do, the most worth trying first, each + * with the known rule it comes from and why, such as an HTTP call without a circuit breaker, a dead letter channel, + * or a hand-off to another route over seda. + * + * @return {@code suggestions}: a list of {@code route}, {@code kind}, {@code recipe}, {@code title}, {@code rule} + * and {@code why} + */ + public Map<String, Object> suggest() { + JsonArray list = new JsonArray(); + for (ChaosAdvisor.Suggestion s : ChaosAdvisor.suggest(camelContext)) { + JsonObject jo = new JsonObject(); + jo.put("route", s.route()); + jo.put("kind", s.kind().key); + if (s.recipe() != null) { + jo.put("recipe", s.recipe()); + } + jo.put("title", s.title()); + jo.put("rule", s.rule()); + jo.put("why", s.why()); Review Comment: ⚠️ **`"tried"` key is never set here:** This method populates each suggestion JSON object with `route`, `kind`, `recipe`, `title`, `rule`, and `why` — but never `"tried"`. `ChaosSuggestList` reads `jo.getBooleanOrDefault("tried", false)`, which will always be `false`. The badge and detail panel in `ChaosSuggestList` that display "tried" status are dead code. If `tried` is intended for a future feature (tracking which rules have been exercised), consider either omitting the field from `ChaosSuggestList` for now, or documenting where it will be set. ########## components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosAdvisor.java: ########## @@ -0,0 +1,240 @@ +/* + * 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.component.chaos; + +import java.util.ArrayList; +import java.util.Comparator; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.apache.camel.CamelContext; +import org.apache.camel.Endpoint; +import org.apache.camel.EndpointAware; +import org.apache.camel.Processor; +import org.apache.camel.Route; +import org.apache.camel.model.CircuitBreakerDefinition; +import org.apache.camel.model.ProcessorDefinitionHelper; +import org.apache.camel.model.RouteDefinition; +import org.apache.camel.model.errorhandler.DeadLetterChannelDefinition; +import org.apache.camel.support.ScheduledPollConsumer; + +/** + * What to try (CAMEL-26727): chaos rules suggested per route from what the route does, ranked, each a known rule of the + * chaos model ({@link ChaosModel#recipeList()}) with its target filled in and a line that says why. The facts come from + * the running routes: their sends, their circuit breakers, their dead letter channel, their input, and the hand-offs + * between them. + */ +final class ChaosAdvisor { + + /** The most suggestions in all, so a big integration does not flood a tool. */ + static final int MAX = 40; + + // the inputs that can deliver a message again (at least once), so a call out may be made twice + private static final Set<String> AT_LEAST_ONCE = Set.of( + "kafka", "jms", "activemq", "activemq6", "amqp", "sjms", "sjms2", "aws2-sqs", "google-pubsub", + "spring-rabbitmq", "rabbitmq", "nats", "paho", "paho-mqtt5", "azure-servicebus", "ibm-mq"); + // a hand-off to another route of the same application, which does not wait for it + private static final Set<String> HAND_OFF = Set.of("seda", "vm", "disruptor", "disruptor-vm"); + + /** The kinds of faults, which the report also uses for what has been tried. */ + enum Kind { + ERRORS("errors"), + SLOW("slow"), + LOST("lost"), + CORRUPT("corrupt"), + DUPLICATE("duplicate"), + CONSUMER("consumer"), + LIFECYCLE("lifecycle"), + DEAD_LETTER("dead letter channel"); + + final String key; + + Kind(String key) { + this.key = key; + } + } + + /** + * A rule to try. + * + * @param route the route it is about + * @param kind the kind of fault + * @param recipe the id of the known rule it comes from (null for one of its own) + * @param title the known rule in plain words, such as "Returns an error (HTTP status)" + * @param rule the rule, ready to add + * @param why why it is worth trying for this route + * @param rank higher is tried first + */ + record Suggestion(String route, Kind kind, String recipe, String title, String rule, String why, int rank) { + } + + private ChaosAdvisor() { + } + + /** The rules to try for the routes of the context, the most worth trying first, at most {@link #MAX}. */ + static List<Suggestion> suggest(CamelContext camelContext) { + Map<String, ChaosModel.Recipe> recipes = new LinkedHashMap<>(); + for (ChaosModel.Recipe recipe : ChaosModel.recipeList()) { + recipes.put(recipe.id(), recipe); + } + List<Route> routes = camelContext.getRoutes(); + // the routes that consume an endpoint, for the hand-offs + Map<String, String> consumers = new LinkedHashMap<>(); + for (Route route : routes) { + if (route.getEndpoint() != null) { + consumers.put(uri(route.getEndpoint()), route.getRouteId()); + } + } + List<Suggestion> answer = new ArrayList<>(); + for (Route route : routes) { + try { + suggest(route, recipes, consumers, answer); + } catch (RuntimeException e) { + // the route changed while it was read (such as a reload): nothing to say for it now + } Review Comment: ⚠️ **Silent exception swallowing:** This catch is intentionally broad to handle `ConcurrentModificationException` during route reloads, but it also silently swallows any genuine bug — `NullPointerException`, `ClassCastException`, `IllegalStateException` — with no log output. Consider adding at least a `LOG.debug()` so unexpected failures are surfaced: ```suggestion } catch (RuntimeException e) { // the route changed while it was read (such as a reload): nothing to say for it now LOG.debug("Skipping route suggestion due to exception", e); } ``` (This would require adding a `private static final Logger LOG = LoggerFactory.getLogger(ChaosAdvisor.class);` field.) ########## dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/ChaosSuggestList.java: ########## @@ -0,0 +1,171 @@ +/* + * 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.tui; + +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.List; + +import dev.tamboui.layout.Constraint; +import dev.tamboui.layout.Layout; +import dev.tamboui.layout.Rect; +import dev.tamboui.style.Overflow; +import dev.tamboui.style.Style; +import dev.tamboui.terminal.Frame; +import dev.tamboui.text.Line; +import dev.tamboui.text.Span; +import dev.tamboui.text.Text; +import dev.tamboui.tui.event.KeyEvent; +import dev.tamboui.widgets.Clear; +import dev.tamboui.widgets.block.Block; +import dev.tamboui.widgets.block.BorderType; +import dev.tamboui.widgets.block.Borders; +import dev.tamboui.widgets.paragraph.Paragraph; +import org.apache.camel.util.json.JsonObject; + +/** + * What to try ({@code w} on the Chaos screen, CAMEL-26727): the rules the integration suggests per route from what its + * routes do, the most worth trying first, each with why. Enter types the selected rule in, to change or add. + */ +final class ChaosSuggestList { + + /** What a key did to the list. */ + enum Outcome { + NONE, + CLOSED, + PICKED + } + + /** A suggestion, as the chaos action of the integration gives it. */ + record Item(String route, String kind, String title, String rule, String why, boolean tried) { + } + + private final String name; + private final List<Item> items = new ArrayList<>(); + private int selected; + + ChaosSuggestList(String name, JsonObject reply) { + this.name = name; + if (reply != null && reply.getCollection("suggestions") != null) { + for (Object o : reply.getCollection("suggestions")) { + if (o instanceof JsonObject jo && jo.getString("rule") != null) { + items.add(new Item( + jo.getString("route"), jo.getString("kind"), jo.getString("title"), + jo.getString("rule"), jo.getString("why"), jo.getBooleanOrDefault("tried", false))); + } + } + } + } + + List<Item> items() { + return items; + } + + /** The selected suggestion, or null when there are none. */ + Item selected() { + return items.isEmpty() ? null : items.get(selected); + } + + Outcome handleKey(KeyEvent ke) { + if (ke.isCancel()) { + return Outcome.CLOSED; + } + if (ke.isUp() && selected > 0) { + selected--; + } else if (ke.isDown() && selected < items.size() - 1) { + selected++; + } else if (ke.isConfirm() && !items.isEmpty()) { + return Outcome.PICKED; + } + return Outcome.NONE; + } + + void render(Frame frame, Rect area) { + int width = Math.min(area.width() - 4, 150); + int height = Math.min(area.height() - 2, 30); + Rect popup = DialogHelper.centered(area, width, height); + frame.renderWidget(Clear.INSTANCE, popup); + Block outer = Block.builder().borderType(BorderType.ROUNDED).borders(Borders.ALL).borderStyle(Theme.warning()) + .title(" What to try in " + name + " ").build(); + frame.renderWidget(outer, popup); + Rect inner = outer.inner(popup); + if (items.isEmpty()) { + frame.renderWidget(Paragraph.builder().overflow(Overflow.WRAP_WORD).text(Text.from(List.of(Line.empty(), + Line.from(Span.styled(" Nothing to suggest: the routes call nothing remote, and have no input from" + + " outside, dead letter channel or hand-off to another route", + Theme.muted()))))) + .build(), + inner); + return; + } + List<Rect> cols = Layout.horizontal() + .constraints(Constraint.length(Math.min(56, inner.width() / 2)), Constraint.fill()) + .split(inner); + renderList(frame, cols.get(0)); + renderDetail(frame, cols.get(1)); + } + + private void renderList(Frame frame, Rect area) { + List<Line> lines = new ArrayList<>(); + lines.add(Line.from(Span.styled(" The most worth trying first", Style.EMPTY.bold()))); + int selectedLine = 0; + for (int i = 0; i < items.size(); i++) { + Item item = items.get(i); + boolean sel = i == selected; + if (sel) { + selectedLine = lines.size(); + } + Style style = sel ? Theme.selectionBg().bold() : Style.EMPTY; + List<Span> spans = new ArrayList<>(); + spans.add(Span.styled((sel ? " > " : " ") + item.title(), style)); + spans.add(Span.styled(" " + item.route(), Theme.muted())); + if (item.tried()) { + spans.add(Span.styled(" tried", Theme.success())); + } + lines.add(Line.from(spans)); + } + // keep the selected one in sight + int scroll = Math.max(0, selectedLine - (area.height() - 2)); + frame.renderWidget(Paragraph.builder().text(Text.from(lines)).scroll(scroll) + .block(Block.builder().borders(EnumSet.of(Borders.RIGHT)).borderStyle(Theme.border()).build()).build(), area); + } + + private void renderDetail(Frame frame, Rect area) { + Item item = selected(); + List<Line> lines = new ArrayList<>(); + lines.add(Line.from(Span.styled(" " + item.title(), Style.EMPTY.bold()), + Span.styled(" route " + item.route() + " · " + item.kind(), Theme.muted()))); + lines.add(Line.empty()); + lines.add(Line.from(Span.raw(" " + item.why()))); + lines.add(Line.empty()); + lines.add(Line.from(Span.styled(" " + item.rule(), Theme.warning()))); + if (item.tried()) { + lines.add(Line.empty()); + lines.add(Line.from(Span.styled(" A rule of this kind was tried in route " + item.route() + " already", + Theme.success()))); + } Review Comment: ⚠️ **Dead code — same `tried` issue as `renderList`:** `item.tried()` is always `false` (see the companion comment on line 136). This detail panel text about "A rule of this kind was tried" is never displayed. -- 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]
