Skip to content

Commit d231d2d

Browse files
web-padawanclaude
web-padawan
and
claude
authored
feat: add Mode.ROUTER guard on Breadcrumbs child management methods (#9485)
## Description Fixes #9482 Depends on #9483 Task 6 of the Breadcrumbs SDD - based on tasks reorder in vaadin/web-components#11912. Added overrides to disallow manual children modifications when using `Mode.ROUTER`. ## Type of change - Feature --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 691149c commit d231d2d

2 files changed

Lines changed: 201 additions & 3 deletions

File tree

‎vaadin-breadcrumbs-flow-parent/vaadin-breadcrumbs-flow/src/main/java/com/vaadin/flow/component/breadcrumbs/Breadcrumbs.java‎

Lines changed: 125 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,12 @@
1616
package com.vaadin.flow.component.breadcrumbs;
1717

1818
import java.io.Serializable;
19+
import java.util.Collection;
20+
import java.util.List;
1921
import java.util.Objects;
2022

23+
import org.jspecify.annotations.Nullable;
24+
2125
import com.fasterxml.jackson.annotation.JsonInclude;
2226
import com.vaadin.experimental.FeatureFlags;
2327
import com.vaadin.flow.component.AttachEvent;
@@ -31,7 +35,10 @@
3135
import com.vaadin.flow.component.dependency.JsModule;
3236
import com.vaadin.flow.component.dependency.NpmPackage;
3337
import com.vaadin.flow.component.shared.HasThemeVariant;
38+
import com.vaadin.flow.function.SerializableFunction;
3439
import com.vaadin.flow.internal.JacksonUtils;
40+
import com.vaadin.flow.internal.nodefeature.SignalBindingFeature;
41+
import com.vaadin.flow.signals.Signal;
3542

3643
/**
3744
* Breadcrumbs is a component for displaying a navigation trail that shows the
@@ -66,6 +73,8 @@ public enum Mode {
6673

6774
private Mode mode;
6875

76+
private boolean internalChildUpdate;
77+
6978
private BreadcrumbsI18n i18n;
7079

7180
/**
@@ -106,15 +115,130 @@ public Mode getMode() {
106115
* @param newMode
107116
* the mode that determines how the trail is populated, not
108117
* {@code null}
118+
* @throws IllegalStateException
119+
* if a children binding set via {@code bindChildren} is active;
120+
* such a binding takes over the trail and cannot be handed back
121+
* to component-controlled population
109122
*/
110123
public void setMode(Mode newMode) {
111124
if (newMode == mode) {
112125
return;
113126
}
127+
if (hasChildrenBinding()) {
128+
throw new IllegalStateException(
129+
"Cannot change the mode while a children binding is active.");
130+
}
114131
this.mode = newMode;
115132
// Listener register/unregister and the initial router rebuild are
116133
// deferred to a later task; for now just clear the existing children.
117-
removeAll();
134+
updateChildrenInternal(List.of());
135+
}
136+
137+
@Override
138+
public void add(BreadcrumbsItem... components) {
139+
checkManualMutationAllowed();
140+
HasComponentsOfType.super.add(components);
141+
}
142+
143+
@Override
144+
public void add(Collection<BreadcrumbsItem> components) {
145+
checkManualMutationAllowed();
146+
HasComponentsOfType.super.add(components);
147+
}
148+
149+
@Override
150+
public void remove(BreadcrumbsItem... components) {
151+
checkManualMutationAllowed();
152+
HasComponentsOfType.super.remove(components);
153+
}
154+
155+
@Override
156+
public void remove(Collection<BreadcrumbsItem> components) {
157+
checkManualMutationAllowed();
158+
HasComponentsOfType.super.remove(components);
159+
}
160+
161+
@Override
162+
public void removeAll() {
163+
checkManualMutationAllowed();
164+
HasComponentsOfType.super.removeAll();
165+
}
166+
167+
@Override
168+
public void addComponentAsFirst(BreadcrumbsItem component) {
169+
checkManualMutationAllowed();
170+
HasComponentsOfType.super.addComponentAsFirst(component);
171+
}
172+
173+
@Override
174+
public void addComponentAtIndex(int index, BreadcrumbsItem component) {
175+
checkManualMutationAllowed();
176+
HasComponentsOfType.super.addComponentAtIndex(index, component);
177+
}
178+
179+
@Override
180+
public void replace(BreadcrumbsItem oldComponent,
181+
BreadcrumbsItem newComponent) {
182+
checkManualMutationAllowed();
183+
HasComponentsOfType.super.replace(oldComponent, newComponent);
184+
}
185+
186+
@Override
187+
public <V extends @Nullable Object, S extends Signal<V>> void bindChildren(
188+
Signal<List<S>> list,
189+
SerializableFunction<S, BreadcrumbsItem> childFactory) {
190+
checkManualMutationAllowed();
191+
HasComponentsOfType.super.bindChildren(list, childFactory);
192+
}
193+
194+
/**
195+
* Throws if manual child mutation is not allowed in the current mode.
196+
* <p>
197+
* Mutating children is rejected while in {@link Mode#ROUTER}, unless the
198+
* internal {@link #internalChildUpdate} flag is set (i.e. the change
199+
* originates from an internal child update such as a mode switch or a
200+
* router-driven rebuild via {@link #updateChildrenInternal(List)}).
201+
*/
202+
private void checkManualMutationAllowed() {
203+
if (mode == Mode.ROUTER && !internalChildUpdate) {
204+
throw new IllegalStateException(
205+
"Cannot modify breadcrumb items manually in Mode.ROUTER. "
206+
+ "Switch to Mode.MANUAL to manage items directly.");
207+
}
208+
}
209+
210+
/**
211+
* Replaces the current children with the given trail, bypassing the
212+
* {@link Mode#ROUTER} mutation guard for the duration of the update. Used
213+
* both for the router-driven rebuild and for clearing children on a mode
214+
* switch.
215+
*
216+
* @param trail
217+
* the breadcrumb items to set as the new children
218+
*/
219+
void updateChildrenInternal(List<BreadcrumbsItem> trail) {
220+
internalChildUpdate = true;
221+
try {
222+
removeAll();
223+
add(trail.toArray(BreadcrumbsItem[]::new));
224+
} finally {
225+
internalChildUpdate = false;
226+
}
227+
}
228+
229+
/**
230+
* Checks whether a children binding (set via {@code bindChildren}) is
231+
* currently active on this component's element. Mirrors the internal check
232+
* Flow core performs, reading the binding state from the element node.
233+
*
234+
* @return {@code true} if a children binding is active
235+
*/
236+
private boolean hasChildrenBinding() {
237+
return getElement().getNode()
238+
.getFeatureIfInitialized(SignalBindingFeature.class)
239+
.map(feature -> feature
240+
.hasBinding(SignalBindingFeature.CHILDREN))
241+
.orElse(false);
118242
}
119243

120244
@Override

‎vaadin-breadcrumbs-flow-parent/vaadin-breadcrumbs-flow/src/test/java/com/vaadin/flow/component/breadcrumbs/tests/BreadcrumbsModeTest.java‎

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,18 @@
1515
*/
1616
package com.vaadin.flow.component.breadcrumbs.tests;
1717

18+
import java.util.List;
19+
1820
import org.junit.jupiter.api.Assertions;
1921
import org.junit.jupiter.api.Test;
2022

2123
import com.vaadin.flow.component.breadcrumbs.Breadcrumbs;
2224
import com.vaadin.flow.component.breadcrumbs.Breadcrumbs.Mode;
2325
import com.vaadin.flow.component.breadcrumbs.BreadcrumbsItem;
26+
import com.vaadin.flow.signals.local.ValueSignal;
27+
import com.vaadin.tests.AbstractSignalsTest;
2428

25-
class BreadcrumbsModeTest {
29+
class BreadcrumbsModeTest extends AbstractSignalsTest {
2630

2731
@Test
2832
void defaultConstructor_modeIsRouter() {
@@ -36,12 +40,68 @@ void modeConstructor_modeIsSet() {
3640
}
3741

3842
@Test
39-
void manualModeWithChildren_setModeRouter_clearsChildren() {
43+
void manualMode_mutatingMethods_doNotThrow() {
44+
var breadcrumbs = new Breadcrumbs(Mode.MANUAL);
45+
var home = new BreadcrumbsItem("Home");
46+
var page = new BreadcrumbsItem("Page");
47+
48+
Assertions.assertDoesNotThrow(() -> breadcrumbs.add(home));
49+
Assertions.assertDoesNotThrow(() -> breadcrumbs.remove(home));
50+
Assertions.assertDoesNotThrow(breadcrumbs::removeAll);
51+
Assertions.assertDoesNotThrow(() -> breadcrumbs.add(home));
52+
Assertions.assertDoesNotThrow(() -> breadcrumbs.replace(home, page));
53+
Assertions.assertDoesNotThrow(
54+
() -> breadcrumbs.addComponentAsFirst(home));
55+
Assertions.assertDoesNotThrow(
56+
() -> breadcrumbs.addComponentAtIndex(0, page));
57+
}
58+
59+
@Test
60+
void routerMode_mutatingMethods_throw() {
61+
var breadcrumbs = new Breadcrumbs(Mode.ROUTER);
62+
var home = new BreadcrumbsItem("Home");
63+
var page = new BreadcrumbsItem("Page");
64+
65+
Assertions.assertThrows(IllegalStateException.class,
66+
() -> breadcrumbs.add(home));
67+
Assertions.assertThrows(IllegalStateException.class,
68+
() -> breadcrumbs.remove(home));
69+
Assertions.assertThrows(IllegalStateException.class,
70+
breadcrumbs::removeAll);
71+
Assertions.assertThrows(IllegalStateException.class,
72+
() -> breadcrumbs.replace(home, page));
73+
Assertions.assertThrows(IllegalStateException.class,
74+
() -> breadcrumbs.addComponentAsFirst(home));
75+
Assertions.assertThrows(IllegalStateException.class,
76+
() -> breadcrumbs.addComponentAtIndex(0, page));
77+
Assertions.assertThrows(IllegalStateException.class,
78+
() -> breadcrumbs.bindChildren(null, null));
79+
}
80+
81+
@Test
82+
void getChildren_worksInBothModes() {
83+
var manual = new Breadcrumbs(Mode.MANUAL);
84+
manual.add(new BreadcrumbsItem("Home"));
85+
Assertions.assertEquals(1, manual.getChildren().count());
86+
87+
var router = new Breadcrumbs(Mode.ROUTER);
88+
Assertions.assertEquals(0, router.getChildren().count());
89+
}
90+
91+
@Test
92+
void manualModeWithChildren_setModeRouter_clearsChildrenAndReappliesGuard() {
4093
var breadcrumbs = new Breadcrumbs(Mode.MANUAL);
4194
breadcrumbs.add(new BreadcrumbsItem("Home"),
4295
new BreadcrumbsItem("Page"));
96+
97+
// MANUAL -> ROUTER clears manually-added children without throwing,
98+
// exercising the updateChildrenInternal(List.of()) bypass.
4399
breadcrumbs.setMode(Mode.ROUTER);
44100
Assertions.assertEquals(0, breadcrumbs.getChildren().count());
101+
102+
// The bypass flag is reset afterwards, so a guarded add throws again.
103+
Assertions.assertThrows(IllegalStateException.class,
104+
() -> breadcrumbs.add(new BreadcrumbsItem("Other")));
45105
}
46106

47107
@Test
@@ -54,4 +114,18 @@ void setModeSameValue_isNoOp_childrenUnchanged() {
54114
Assertions.assertEquals(item,
55115
breadcrumbs.getChildren().findFirst().orElse(null));
56116
}
117+
118+
@Test
119+
void setMode_withActiveChildrenBinding_throws() {
120+
var breadcrumbs = new Breadcrumbs(Mode.MANUAL);
121+
var itemSignal = new ValueSignal<>("Home");
122+
var listSignal = new ValueSignal<>(List.of(itemSignal));
123+
breadcrumbs.bindChildren(listSignal,
124+
signal -> new BreadcrumbsItem(signal.peek()));
125+
126+
// A binding controls the children and cannot be removed; switching
127+
// modes would clear them, so setMode must fail.
128+
Assertions.assertThrows(IllegalStateException.class,
129+
() -> breadcrumbs.setMode(Mode.ROUTER));
130+
}
57131
}

0 commit comments

Comments
 (0)