Skip to content

Commit 8849c31

Browse files
ammachadoclaude
andcommitted
CAMEL-23720: Harden MoreTab invariants and strengthen TUI icon tests
Add a compact constructor to MoreTab that validates the '&' mnemonic marker so a malformed label fails loudly at construction instead of throwing StringIndexOutOfBounds from shortcut() at startup. Replace the last hardcoded More index (selectMoreTab(12)) with a name lookup via moreTabIndex("SQL Query"). Make PRIMARY_TAB_ICONS an unmodifiable List and drop TuiIcons to package-private to match its members. Tests: replace the tautological shortcut test with a literal historical sequence oracle, add shortcut-uniqueness, constructor-validation, moreTabIndex, allTabEntries, and PRIMARY_TAB_ICONS ordering coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 1a3544f commit 8849c31

5 files changed

Lines changed: 103 additions & 13 deletions

File tree

dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TabRegistry.java

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ void initTabs(MonitorContext ctx, DataRefreshService dataService, Runnable reset
135135
resetIntegrationTabState);
136136

137137
sqlTraceTab.setEditSqlAction(sql -> {
138-
selectMoreTab(12); // switch to SQL Query tab
138+
selectMoreTab(moreTabIndex("SQL Query"));
139139
sqlQueryTab.setInputValue("sql", sql);
140140
});
141141

@@ -333,6 +333,14 @@ record TabEntry(String icon, String name, String description, String shortcut, i
333333
*/
334334
record MoreTab(String icon, String name, String label, MonitorTab tab) {
335335

336+
MoreTab {
337+
int i = TuiIcons.mnemonicIndex(label);
338+
if (i < 0 || i >= TuiIcons.stripMnemonic(label).length()) {
339+
throw new IllegalArgumentException(
340+
"label must contain a '" + TuiIcons.MNEMONIC_MARKER + "' marker before a letter: " + label);
341+
}
342+
}
343+
336344
String displayName() {
337345
return TuiIcons.stripMnemonic(label);
338346
}
@@ -350,6 +358,16 @@ List<MoreTab> moreTabs() {
350358
return moreTabs;
351359
}
352360

361+
/** Position of the More tab with the given programmatic {@link MoreTab#name() name}, or -1 when absent. */
362+
int moreTabIndex(String name) {
363+
for (int i = 0; i < moreTabs.size(); i++) {
364+
if (moreTabs.get(i).name().equals(name)) {
365+
return i;
366+
}
367+
}
368+
return -1;
369+
}
370+
353371
List<TabEntry> allTabEntries() {
354372
List<TabEntry> entries = new ArrayList<>();
355373
entries.add(new TabEntry(icon(TAB_OVERVIEW), "Overview", overviewTab.description(), "1", TAB_OVERVIEW, -1));
@@ -371,6 +389,6 @@ List<TabEntry> allTabEntries() {
371389
}
372390

373391
private static String icon(int primaryTabIndex) {
374-
return TuiIcons.PRIMARY_TAB_ICONS[primaryTabIndex];
392+
return TuiIcons.PRIMARY_TAB_ICONS.get(primaryTabIndex);
375393
}
376394
}

dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiIcons.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@
1616
*/
1717
package org.apache.camel.dsl.jbang.core.commands.tui;
1818

19+
import java.util.List;
20+
1921
/**
2022
* Single source of truth for emoji and symbolic icons used across the Camel TUI.
2123
* <p/>
2224
* Tab/menu icons use plain 2-column-wide emoji without VS16 variation selectors (see CAMEL-23818). Doctor and legacy
2325
* status glyphs may still use mixed-width symbols until migrated.
2426
*/
25-
public final class TuiIcons {
27+
final class TuiIcons {
2628

2729
// ---- Brand & runtime ----
2830
static final String CAMEL = "🐪";
@@ -139,10 +141,9 @@ public final class TuiIcons {
139141
static final String TAB_THREADS = "🧵";
140142

141143
/** Icons for {@link TabRegistry#TAB_OVERVIEW}..{@link TabRegistry#TAB_MORE} (in order). */
142-
static final String[] PRIMARY_TAB_ICONS = {
144+
static final List<String> PRIMARY_TAB_ICONS = List.of(
143145
TAB_OVERVIEW, TAB_LOG, TAB_DIAGRAM, TAB_ROUTES, TAB_ENDPOINTS,
144-
TAB_HTTP, TAB_HEALTH, TAB_INSPECT, TAB_ERRORS, TAB_MORE
145-
};
146+
TAB_HTTP, TAB_HEALTH, TAB_INSPECT, TAB_ERRORS, TAB_MORE);
146147

147148
/** Marker placed immediately before a label's keyboard-shortcut letter (Windows-style mnemonic). */
148149
static final char MNEMONIC_MARKER = '&';

dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/TabBarRenderTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ class TabBarRenderTest {
3535
void primaryTabHeadersStartWithIconAndTwoSpaces() {
3636
String[] names = { "Overview", "Log", "Diagram", "Route", "Endpoint", "HTTP", "Health", "Inspect", "Errors" };
3737
for (int i = 0; i < names.length; i++) {
38-
String header = TuiIcons.primaryTabHeader(TuiIcons.PRIMARY_TAB_ICONS[i], String.valueOf(i + 1), names[i]);
39-
assertTrue(header.startsWith(TuiIcons.PRIMARY_TAB_ICONS[i] + " " + (i + 1)),
38+
String header = TuiIcons.primaryTabHeader(TuiIcons.PRIMARY_TAB_ICONS.get(i), String.valueOf(i + 1), names[i]);
39+
assertTrue(header.startsWith(TuiIcons.PRIMARY_TAB_ICONS.get(i) + " " + (i + 1)),
4040
"Tab header should be '<icon> <key> <name>': " + header);
4141
}
4242
}

dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/TabRegistryTest.java

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import java.nio.file.Path;
2020
import java.util.List;
21+
import java.util.Set;
2122
import java.util.concurrent.atomic.AtomicReference;
2223

2324
import dev.tamboui.text.CharWidth;
@@ -27,6 +28,7 @@
2728

2829
import static org.junit.jupiter.api.Assertions.assertEquals;
2930
import static org.junit.jupiter.api.Assertions.assertFalse;
31+
import static org.junit.jupiter.api.Assertions.assertThrows;
3032
import static org.junit.jupiter.api.Assertions.assertTrue;
3133

3234
/**
@@ -102,10 +104,63 @@ void moreTabIconsAreTwoColumnsWideWithoutVariationSelector() {
102104
}
103105

104106
@Test
105-
void shortcutIsDerivedFromMnemonicLabelForEveryTab() {
106-
for (TabRegistry.MoreTab mt : registry.moreTabs()) {
107-
char expected = Character.toUpperCase(mt.displayName().charAt(mt.mnemonicIndex()));
108-
assertEquals(expected, mt.shortcut(), "Shortcut for '" + mt.name() + "' must be its highlighted letter");
107+
void moreTabShortcutsMatchHistoricalSequence() {
108+
// Independent oracle (not a re-derivation of shortcut()): these are the exact letters the hand-maintained
109+
// MORE_SHORTCUTS array carried before the MoreTab refactor. A label edit that repoints a key must fail here.
110+
List<Character> shortcuts = registry.moreTabs().stream().map(TabRegistry.MoreTab::shortcut).toList();
111+
assertEquals(
112+
List.of('B', 'W', 'C', 'A', 'G', 'N', 'D', 'H', 'I', 'M', 'K', 'E', 'Q', 'R', 'O', 'P', 'S', 'T'),
113+
shortcuts, "More tab shortcut letters must match the historical sequence");
114+
}
115+
116+
@Test
117+
void moreTabShortcutsAreUnique() {
118+
// morePopupShortcut() returns the first matching tab, so a duplicated letter would silently shadow a later tab.
119+
List<Character> shortcuts = registry.moreTabs().stream().map(TabRegistry.MoreTab::shortcut).toList();
120+
assertEquals(shortcuts.size(), Set.copyOf(shortcuts).size(),
121+
"More tab shortcut letters must be unique: " + shortcuts);
122+
}
123+
124+
@Test
125+
void moreTabRejectsLabelWithoutUsableMnemonicMarker() {
126+
// The record's compact constructor enforces the '&'-before-a-letter invariant that shortcut() relies on,
127+
// so a mistyped literal fails loudly at construction instead of throwing StringIndexOutOfBounds later.
128+
assertThrows(IllegalArgumentException.class,
129+
() -> new TabRegistry.MoreTab(TuiIcons.TAB_BEANS, "Beans", "Beans", null));
130+
assertThrows(IllegalArgumentException.class,
131+
() -> new TabRegistry.MoreTab(TuiIcons.TAB_BEANS, "Beans", "Beans&", null));
132+
}
133+
134+
@Test
135+
void moreTabIndexResolvesSqlQueryByNameForEditSqlJump() {
136+
// Guards the selectMoreTab(moreTabIndex("SQL Query")) wiring that replaced a hardcoded index.
137+
int idx = registry.moreTabIndex("SQL Query");
138+
assertEquals("SQL Query", registry.moreTabs().get(idx).name());
139+
assertEquals(-1, registry.moreTabIndex("Nonexistent"), "Unknown name resolves to -1");
140+
}
141+
142+
@Test
143+
void allTabEntriesExposeDigitsIconsAndMoreShortcuts() {
144+
List<TabRegistry.TabEntry> entries = registry.allTabEntries();
145+
assertEquals(9 + registry.moreTabs().size(), entries.size());
146+
147+
// Primary tabs: digit shortcuts 1-9, moreIndex -1, icon indexed by tabIndex.
148+
for (int i = 0; i < 9; i++) {
149+
TabRegistry.TabEntry e = entries.get(i);
150+
assertEquals(String.valueOf(i + 1), e.shortcut(), "Primary tab " + e.name() + " digit shortcut");
151+
assertEquals(-1, e.moreIndex(), "Primary tab " + e.name() + " has no More index");
152+
assertEquals(TuiIcons.PRIMARY_TAB_ICONS.get(e.tabIndex()), e.icon());
153+
}
154+
155+
// More tabs: tabIndex TAB_MORE, ascending moreIndex, shortcut/name/icon carried from the owning MoreTab.
156+
for (int i = 0; i < registry.moreTabs().size(); i++) {
157+
TabRegistry.TabEntry e = entries.get(9 + i);
158+
TabRegistry.MoreTab mt = registry.moreTabs().get(i);
159+
assertEquals(TabRegistry.TAB_MORE, e.tabIndex());
160+
assertEquals(i, e.moreIndex());
161+
assertEquals(String.valueOf(mt.shortcut()), e.shortcut());
162+
assertEquals(mt.name(), e.name());
163+
assertEquals(mt.icon(), e.icon());
109164
}
110165
}
111166

dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiIconsTest.java

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,23 @@ class TuiIconsTest {
3232

3333
@Test
3434
void primaryTabIconCountMatchesTabRegistry() {
35-
assertEquals(TabRegistry.NUM_TABS, TuiIcons.PRIMARY_TAB_ICONS.length);
35+
assertEquals(TabRegistry.NUM_TABS, TuiIcons.PRIMARY_TAB_ICONS.size());
36+
}
37+
38+
@Test
39+
void primaryTabIconsAreOrderedByTabIndex() {
40+
// Guards that PRIMARY_TAB_ICONS is indexed by the TAB_* constants: reordering the list must break this,
41+
// otherwise TabRegistry.icon(index) would attach the wrong emoji to Go-to entries.
42+
assertEquals(TuiIcons.TAB_OVERVIEW, TuiIcons.PRIMARY_TAB_ICONS.get(TabRegistry.TAB_OVERVIEW));
43+
assertEquals(TuiIcons.TAB_LOG, TuiIcons.PRIMARY_TAB_ICONS.get(TabRegistry.TAB_LOG));
44+
assertEquals(TuiIcons.TAB_DIAGRAM, TuiIcons.PRIMARY_TAB_ICONS.get(TabRegistry.TAB_DIAGRAM));
45+
assertEquals(TuiIcons.TAB_ROUTES, TuiIcons.PRIMARY_TAB_ICONS.get(TabRegistry.TAB_ROUTES));
46+
assertEquals(TuiIcons.TAB_ENDPOINTS, TuiIcons.PRIMARY_TAB_ICONS.get(TabRegistry.TAB_ENDPOINTS));
47+
assertEquals(TuiIcons.TAB_HTTP, TuiIcons.PRIMARY_TAB_ICONS.get(TabRegistry.TAB_HTTP));
48+
assertEquals(TuiIcons.TAB_HEALTH, TuiIcons.PRIMARY_TAB_ICONS.get(TabRegistry.TAB_HEALTH));
49+
assertEquals(TuiIcons.TAB_INSPECT, TuiIcons.PRIMARY_TAB_ICONS.get(TabRegistry.TAB_HISTORY));
50+
assertEquals(TuiIcons.TAB_ERRORS, TuiIcons.PRIMARY_TAB_ICONS.get(TabRegistry.TAB_ERRORS));
51+
assertEquals(TuiIcons.TAB_MORE, TuiIcons.PRIMARY_TAB_ICONS.get(TabRegistry.TAB_MORE));
3652
}
3753

3854
@Test

0 commit comments

Comments
 (0)