gnodet-bot commented on code in PR #26604:
URL: https://github.com/apache/camel/pull/26604#discussion_r4050388495
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/ExampleHelper.java:
##########
@@ -135,15 +152,155 @@ private static boolean matches(JsonObject entry, String
filter) {
return false;
}
+ /**
+ * The groups of the example ladder in reading order: level, title, and
the one-line introduction the README of
+ * camel-jbang-examples uses. Quick start comes first, showcase last; a
level not listed here sorts after them.
+ */
+ private static final String[][] GROUPS = {
+ {
+ "quick-start", "Quick start",
+ "The first ten minutes: generic examples with no story and
no service, each running in seconds." },
+ { "run", "Run", "Running Camel: timers and cron schedules, a bean
in a route, properties and profiles." },
+ { "transform", "Transform and map", "JSON, XML and CSV in and out,
field-by-field mapping, Groovy and XSLT." },
+ {
+ "route", "Route",
+ "The routing patterns: content-based router, splitter,
aggregator, filter and multicast." },
+ {
+ "fail-well", "Fail well",
+ "Retries, a dead letter channel, and a circuit breaker in
front of a flaky service." },
+ {
+ "connect", "Connect without a service",
+ "Files, an HTTP client and a REST server; everything runs
inside the example." },
+ {
+ "connect-service", "Connect to one service",
+ "SQL, JMS, MQTT, Kafka and FTP against a service the Camel
CLI starts with camel infra run." },
+ {
+ "contracts", "Contracts and security",
+ "An OpenAPI contract served and called, and an API
protected by Keycloak." },
+ { "ai", "AI", "A local model writing text, routes exposed as MCP
tools, RAG over documents, PII redaction." },
+ {
+ "cloud", "Cloud",
+ "A cloud service, run locally through LocalStack and
switched to the real thing by properties." },
+ {
+ "showcase", "Showcase",
+ "Tooling demos outside the ladder: the TUI, a memory leak,
message sizes, log analysis." },
+ };
+
+ /**
+ * The levels of the ladder in reading order.
+ */
+ public static List<String> getGroupOrder() {
+ List<String> order = new ArrayList<>();
+ for (String[] g : GROUPS) {
+ order.add(g[0]);
+ }
+ return order;
+ }
+
+ /**
+ * The title of a group (level), for example "Quick start" for
quick-start; an unknown level is capitalized.
+ */
+ public static String getGroupTitle(String level) {
+ for (String[] g : GROUPS) {
+ if (g[0].equals(level)) {
+ return g[1];
+ }
+ }
+ return formatCategory(level);
+ }
+
+ /**
+ * The one-line introduction of a group (level), or an empty string for an
unknown level.
+ */
+ public static String getGroupIntro(String level) {
+ for (String[] g : GROUPS) {
+ if (g[0].equals(level)) {
+ return g[2];
+ }
+ }
+ return "";
+ }
+
+ /**
+ * Groups the examples by level in ladder order, each group sorted by
name; empty groups are left out and levels not
+ * on the ladder come last in the order they appear.
+ */
+ public static Map<String, List<JsonObject>> groupByLevel(List<JsonObject>
catalog) {
+ Map<String, List<JsonObject>> groups = new LinkedHashMap<>();
+ for (String level : getGroupOrder()) {
+ groups.put(level, new ArrayList<>());
+ }
+ for (JsonObject entry : catalog) {
+ String level = entry.getStringOrDefault("level", "other");
+ groups.computeIfAbsent(level, k -> new ArrayList<>()).add(entry);
+ }
+ groups.values().removeIf(List::isEmpty);
+ for (List<JsonObject> entries : groups.values()) {
+ entries.sort(Comparator.comparingInt(ExampleHelper::getOrder)
+ .thenComparing(e -> e.getStringOrDefault("name", "")));
+ }
+ return groups;
+ }
+
+ /**
+ * The reading order of the example within its group from the metadata, or
a large number when it has none, so
+ * examples with an order come first and the rest sort by name.
+ */
+ public static int getOrder(JsonObject entry) {
+ Object order = entry.get("order");
+ if (order instanceof Number n) {
+ return n.intValue();
+ }
+ return Integer.MAX_VALUE;
+ }
+
+ /**
+ * What the example teaches, as one line: the components and the EIPs from
its metadata, or an empty string.
+ */
+ @SuppressWarnings("unchecked")
+ public static String getTeachesSummary(JsonObject entry) {
+ JsonObject teaches = entry.getMap("teaches");
+ if (teaches == null || teaches.isEmpty()) {
+ return "";
+ }
+ StringBuilder sb = new StringBuilder();
+ for (String key : new String[] { "components", "eips", "languages",
"dataformats" }) {
+ Collection<String> values = (Collection<String>) teaches.get(key);
Review Comment:
💡 **Unchecked cast without safety note.** `teaches.get(key)` returns
`Object` from `JsonArray` (which extends `ArrayList<Object>`). Casting to
`Collection<String>` is an erasure cast — the JVM only sees `Collection` at
runtime. `String.join(CharSequence, Iterable<? extends CharSequence>)` will
throw `ClassCastException` at runtime if any element is not a `CharSequence`.
This is the same pattern used elsewhere in the file for `tags`,
`infraServices`, and `files` (all guarded by `@SuppressWarnings("unchecked")`),
and in practice the catalog JSON always has string arrays here. But there is no
compile-time protection — a malformed catalog entry (e.g. `"components": [1,
2]`) would blow up on the `--example` list command with an unhelpful stack
trace.
Consider a brief comment explaining why the cast is safe, or add a defensive
check:
```java
Object raw = teaches.get(key);
if (!(raw instanceof Collection<?> col)) continue;
for (Object item : col) {
if (item instanceof String s) {
if (sb.length() > 0) sb.append(", ");
sb.append(s);
}
}
```
##########
dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/common/ExampleHelperTest.java:
##########
@@ -125,31 +125,67 @@ void shouldGetFiles() {
@Test
void shouldExtractBundledExample() throws Exception {
List<JsonObject> catalog = ExampleHelper.loadCatalog();
- JsonObject entry = ExampleHelper.findExample(catalog,
"eip/circuit-breaker");
+ JsonObject entry = ExampleHelper.findExample(catalog,
"fail-well/circuit-breaker");
Path tempDir = ExampleHelper.extractBundledExample(entry);
- assertTrue(Files.exists(tempDir.resolve("route.camel.yaml")));
- String content = Files.readString(tempDir.resolve("route.camel.yaml"));
+
assertTrue(Files.exists(tempDir.resolve("circuit-breaker.camel.yaml")));
+ String content =
Files.readString(tempDir.resolve("circuit-breaker.camel.yaml"));
assertFalse(content.isEmpty());
}
@Test
- void shouldExtractBundledExampleWithSubdirectory() throws Exception {
+ void shouldExtractBundledExampleWithJavaAndBeans() throws Exception {
List<JsonObject> catalog = ExampleHelper.loadCatalog();
- JsonObject entry = ExampleHelper.findExample(catalog,
"transformation/xslt");
+ JsonObject entry = ExampleHelper.findExample(catalog,
"quick-start/routes");
Path tempDir = ExampleHelper.extractBundledExample(entry);
- assertTrue(Files.exists(tempDir.resolve("consumer.camel.yaml")));
- assertTrue(Files.exists(tempDir.resolve("stylesheet.xsl")));
- assertTrue(Files.exists(tempDir.resolve("input/account.xml")));
+ assertTrue(Files.exists(tempDir.resolve("routes.camel.yaml")));
+ assertTrue(Files.exists(tempDir.resolve("Greeter.java")));
+ assertTrue(Files.exists(tempDir.resolve("beans.yaml")));
+ }
+
+ @Test
+ void shouldGroupByLevelInLadderOrder() {
+ List<JsonObject> catalog = ExampleHelper.loadCatalog();
+ java.util.Map<String, List<JsonObject>> groups =
ExampleHelper.groupByLevel(catalog);
+ List<String> levels = new java.util.ArrayList<>(groups.keySet());
+ assertEquals("quick-start", levels.get(0));
+ assertTrue(levels.indexOf("route") < levels.indexOf("fail-well"));
+ assertTrue(levels.indexOf("connect") <
levels.indexOf("connect-service"));
+ assertEquals("showcase", levels.get(levels.size() - 1));
+ assertEquals("Quick start",
ExampleHelper.getGroupTitle("quick-start"));
+ assertEquals("Connect to one service",
ExampleHelper.getGroupTitle("connect-service"));
+ assertFalse(ExampleHelper.getGroupIntro("fail-well").isEmpty());
+ for (List<JsonObject> entries : groups.values()) {
+ assertFalse(entries.isEmpty());
+ }
+ }
+
+ @Test
+ void shouldSummarizeTeachesAndCiSkip() {
+ List<JsonObject> catalog = ExampleHelper.loadCatalog();
+ JsonObject aggregator = ExampleHelper.findExample(catalog,
"route/aggregator");
+ String teaches = ExampleHelper.getTeachesSummary(aggregator);
+ assertTrue(teaches.contains("eips: "), teaches);
+ assertTrue(teaches.contains("aggregate"), teaches);
+ assertFalse(ExampleHelper.isCiSkip(aggregator));
+ JsonObject chat = ExampleHelper.findExample(catalog,
"ai/langchain4j-chat");
+ assertTrue(ExampleHelper.isCiSkip(chat));
+ }
+
+ @Test
+ void shouldFindAmbiguousShortNames() {
+ List<JsonObject> catalog = ExampleHelper.loadCatalog();
+ assertEquals(1, ExampleHelper.findExamplesByShortName(catalog,
"aggregator").size());
+ assertTrue(ExampleHelper.findExamplesByShortName(catalog,
"no-such-example").isEmpty());
Review Comment:
💡 **Test covers 1-match and 0-match, but not the 2-match case.**
`findExamplesByShortName` is only called in production to detect ambiguity
(size > 1) and display the error in `runExample()`. The test only asserts `size
== 1` (for `"aggregator"`) and `size == 0` (for unknown name). Neither path
exercises the error message that fires when two examples share a short name.
Since the current catalog has no duplicate short names this is not a defect
today, but the test gives false confidence. A minimal additional assertion:
```java
// synthetic ambiguity — the 2-match case feeds the error path in Run.java
JsonObject a = new JsonObject(); a.put("name", "group-a/foo");
JsonObject b = new JsonObject(); b.put("name", "group-b/foo");
assertEquals(2, ExampleHelper.findExamplesByShortName(List.of(a, b),
"foo").size());
```
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/ExampleBrowserPopup.java:
##########
@@ -139,9 +138,22 @@ boolean handleKeyEvent(KeyEvent ke) {
return true;
}
if (ke.isChar('d')) {
- loadDocFromExample();
+ loadDocForSelected();
return true;
}
+ if (currentFolder == null) {
+ // 1 to 9 open the first nine groups, 0 the tenth
+ for (char c = '0'; c <= '9'; c++) {
+ if (ke.isChar(c)) {
+ int n = c == '0' ? 10 : c - '0';
+ List<String> levels = new
ArrayList<>(ExampleHelper.groupByLevel(catalog).keySet());
Review Comment:
💡 **`groupByLevel(catalog)` called up to 4× per render frame.**
`handleKeyEvent` (here), `buildTopLevelItems`, `buildFolderItems`, and
`folderExampleCount` each call `ExampleHelper.groupByLevel(catalog)`, which
builds a fresh `LinkedHashMap` by iterating and sorting the full catalog.
The catalog is ~40 entries today so the wall-clock cost is negligible, but
the pattern is wrong: the grouped result is a pure function of `catalog`, which
only changes on `reload()`. Cache it:
```java
private Map<String, List<JsonObject>> groupedCatalog;
void reload() {
this.catalog = loadAndSortExamples();
this.groupedCatalog = ExampleHelper.groupByLevel(catalog);
}
```
Then replace all four `ExampleHelper.groupByLevel(catalog)` call-sites with
`groupedCatalog`.
--
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]
