Skip to content

Commit 3bff085

Browse files
Artur-Artur-
authored
feat: Replace array-based type encoding with @v object format (#22396)
Replaces the array-based type encoding system [typeId, ...data] with - Nodes: {"@v": "node", "id": nodeId} → {"@v-node": nodeId} - Return channels: {"@v": "return", "nodeId": x, "channelId": y} → {"@v-return": [x, y]} - Removed legacy NODE_TYPE, ARRAY_TYPE, RETURN_CHANNEL_TYPE constants Reject objects with unknown @V- prefixed properties to maintain forward compatibility. This ensures that if new types are added in the future, older clients will fail cleanly rather than misinterpreting the data.
1 parent 5fa9e1a commit 3bff085

File tree

9 files changed

+285
-146
lines changed

9 files changed

+285
-146
lines changed

‎CLAUDE.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ mvn checkstyle:check
8787

8888
- Use triple quotes (`"""`) for multi-line string blocks in Java text blocks
8989
- **When tests fail, code doesn't compile, or similar issues occur: Always analyze why first, do not start rewriting code**
90+
- **When writing code, names and comments should describe how the code works and why, not what has changed from previous versions. Commit messages capture change information, not the code itself.**
91+
- **Always create proper tests for what should work first. If the tests expose problems in the implementation, fix the implementation after the tests have been created.**
9092

9193
### Frontend Development
9294

‎flow-client/src/main/java/com/vaadin/client/flow/TreeChangeProcessor.java‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
*/
1616
package com.vaadin.client.flow;
1717

18+
import com.google.gwt.core.client.GWT;
19+
1820
import com.vaadin.client.WidgetUtil;
1921
import com.vaadin.client.flow.collection.JsArray;
2022
import com.vaadin.client.flow.collection.JsCollections;
@@ -217,7 +219,7 @@ private static void processSpliceChange(JsonObject change, StateNode node) {
217219
JsonArray addJson = change
218220
.getArray(JsonConstants.CHANGE_SPLICE_ADD);
219221

220-
JsArray<Object> add = ClientJsonCodec.jsonArrayAsJsArray(addJson);
222+
JsArray<Object> add = jsonArrayAsJsArray(addJson);
221223

222224
list.splice(index, remove, add);
223225
} else if (change.hasKey(JsonConstants.CHANGE_SPLICE_ADD_NODES)) {
@@ -248,4 +250,29 @@ private static void processClearChange(JsonObject change, StateNode node) {
248250
NodeList list = node.getList(nsId);
249251
list.clear();
250252
}
253+
254+
/**
255+
* Converts a JSON array to a JS array. This is a no-op in compiled
256+
* JavaScript, but needs special handling for tests running in the JVM.
257+
*
258+
* @param jsonArray
259+
* the JSON array to convert
260+
* @return the converted JS array
261+
*/
262+
private static JsArray<Object> jsonArrayAsJsArray(JsonArray jsonArray) {
263+
JsArray<Object> jsArray;
264+
if (GWT.isScript()) {
265+
jsArray = WidgetUtil.crazyJsCast(jsonArray);
266+
} else {
267+
jsArray = JsCollections.array();
268+
for (int i = 0; i < jsonArray.length(); i++) {
269+
// Decode the value to unwrap JsonString to primitive string
270+
Object decoded = ClientJsonCodec
271+
.decodeWithoutTypeInfo(jsonArray.get(i));
272+
jsArray.push(decoded);
273+
}
274+
}
275+
return jsArray;
276+
}
277+
251278
}

‎flow-client/src/main/java/com/vaadin/client/flow/util/ClientJsonCodec.java‎

Lines changed: 82 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import elemental.dom.Node;
2828
import elemental.json.Json;
2929
import elemental.json.JsonArray;
30+
import elemental.json.JsonObject;
3031
import elemental.json.JsonType;
3132
import elemental.json.JsonValue;
3233

@@ -59,21 +60,21 @@ private ClientJsonCodec() {
5960
* @return the decoded state node if any
6061
*/
6162
public static StateNode decodeStateNode(StateTree tree, JsonValue json) {
62-
if (json.getType() == JsonType.ARRAY) {
63-
JsonArray array = (JsonArray) json;
64-
int typeId = (int) array.getNumber(0);
65-
switch (typeId) {
66-
case JsonCodec.NODE_TYPE: {
67-
int nodeId = (int) array.getNumber(1);
63+
if (json.getType() == JsonType.OBJECT) {
64+
// Check for @v-node format
65+
JsonValue nodeIdValue = ((JsonObject) json).get("@v-node");
66+
if (nodeIdValue != null) {
67+
if (nodeIdValue.getType() != JsonType.NUMBER) {
68+
throw new IllegalArgumentException(
69+
"@v-node value must be a number, got "
70+
+ nodeIdValue.getType() + " in "
71+
+ json.toJson());
72+
}
73+
int nodeId = (int) nodeIdValue.asNumber();
6874
return tree.getNode(nodeId);
6975
}
70-
case JsonCodec.ARRAY_TYPE:
71-
case JsonCodec.RETURN_CHANNEL_TYPE:
72-
return null;
73-
default:
74-
throw new IllegalArgumentException(
75-
"Unsupported complex type in " + array.toJson());
76-
}
76+
77+
return null;
7778
} else {
7879
return null;
7980
}
@@ -90,25 +91,54 @@ public static StateNode decodeStateNode(StateTree tree, JsonValue json) {
9091
* @return the decoded value
9192
*/
9293
public static Object decodeWithTypeInfo(StateTree tree, JsonValue json) {
93-
if (json.getType() == JsonType.ARRAY) {
94-
JsonArray array = (JsonArray) json;
95-
int typeId = (int) array.getNumber(0);
96-
switch (typeId) {
97-
case JsonCodec.NODE_TYPE: {
98-
int nodeId = (int) array.getNumber(1);
94+
if (json.getType() == JsonType.OBJECT) {
95+
JsonObject jsonObject = (JsonObject) json;
96+
// Check for @v-node format
97+
JsonValue nodeIdValue = jsonObject.get("@v-node");
98+
if (nodeIdValue != null) {
99+
if (nodeIdValue.getType() != JsonType.NUMBER) {
100+
throw new IllegalArgumentException(
101+
"@v-node value must be a number, got "
102+
+ nodeIdValue.getType() + " in "
103+
+ json.toJson());
104+
}
105+
int nodeId = (int) nodeIdValue.asNumber();
99106
Node domNode = tree.getNode(nodeId).getDomNode();
100107
return domNode;
101108
}
102-
case JsonCodec.ARRAY_TYPE:
103-
return jsonArrayAsJsArray(array.getArray(1));
104-
case JsonCodec.RETURN_CHANNEL_TYPE:
105-
return createReturnChannelCallback((int) array.getNumber(1),
106-
(int) array.getNumber(2),
109+
110+
// Check for @v-return format
111+
JsonValue returnArray = jsonObject.get("@v-return");
112+
if (returnArray != null) {
113+
if (returnArray.getType() != JsonType.ARRAY) {
114+
throw new IllegalArgumentException(
115+
"@v-return value must be an array, got "
116+
+ returnArray.getType() + " in "
117+
+ json.toJson());
118+
}
119+
JsonArray array = (JsonArray) returnArray;
120+
if (array.length() < 2) {
121+
throw new IllegalArgumentException(
122+
"@v-return array must have at least 2 elements, got "
123+
+ array.length() + " in " + json.toJson());
124+
}
125+
int returnNodeId = (int) array.getNumber(0);
126+
int channelId = (int) array.getNumber(1);
127+
return createReturnChannelCallback(returnNodeId, channelId,
107128
tree.getRegistry().getServerConnector());
108-
default:
109-
throw new IllegalArgumentException(
110-
"Unsupported complex type in " + array.toJson());
111129
}
130+
131+
// Check for unknown @v- types
132+
for (String key : jsonObject.keys()) {
133+
if (key.startsWith("@v-")) {
134+
throw new IllegalArgumentException("Unsupported @v type '"
135+
+ key + "' in " + json.toJson());
136+
}
137+
}
138+
139+
return decodeObjectWithTypeInfo(tree, jsonObject);
140+
} else if (json.getType() == JsonType.ARRAY) {
141+
return decodeArrayWithTypeInfo(tree, (JsonArray) json);
112142
} else {
113143
return decodeWithoutTypeInfo(json);
114144
}
@@ -186,22 +216,32 @@ public static JsonValue encodeWithoutTypeInfo(Object value) {
186216
}
187217

188218
/**
189-
* Converts a JSON array to a JS array. This is a no-op in compiled
190-
* JavaScript, but needs special handling for tests running in the JVM.
191-
*
192-
* @param jsonArray
193-
* the JSON array to convert
194-
* @return the converted JS array
219+
* Recursively decodes a JSON object, processing any nested @v references.
220+
* Returns a native JS object that can store decoded values including DOM
221+
* elements.
195222
*/
196-
public static JsArray<Object> jsonArrayAsJsArray(JsonArray jsonArray) {
197-
JsArray<Object> jsArray;
198-
if (GWT.isScript()) {
199-
jsArray = WidgetUtil.crazyJsCast(jsonArray);
200-
} else {
201-
jsArray = JsCollections.array();
202-
for (int i = 0; i < jsonArray.length(); i++) {
203-
jsArray.push(decodeWithoutTypeInfo(jsonArray.get(i)));
204-
}
223+
private static JsonObject decodeObjectWithTypeInfo(StateTree tree,
224+
JsonObject jsonObject) {
225+
226+
for (String key : jsonObject.keys()) {
227+
JsonValue orignalValue = jsonObject.get(key);
228+
Object decoded = decodeWithTypeInfo(tree, orignalValue);
229+
JsonValue newValue = WidgetUtil.crazyJsoCast(decoded);
230+
jsonObject.put(key, newValue);
231+
}
232+
return jsonObject;
233+
}
234+
235+
/**
236+
* Recursively decodes a JSON array, processing any nested @v references.
237+
*/
238+
private static JsArray<Object> decodeArrayWithTypeInfo(StateTree tree,
239+
JsonArray jsonArray) {
240+
JsArray<Object> jsArray = JsCollections.array();
241+
for (int i = 0; i < jsonArray.length(); i++) {
242+
JsonValue originalValue = jsonArray.get(i);
243+
Object decoded = decodeWithTypeInfo(tree, originalValue);
244+
jsArray.push(decoded);
205245
}
206246
return jsArray;
207247
}

‎flow-client/src/test-gwt/java/com/vaadin/client/GwtExecuteJavaScriptElementUtilsTest.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -302,10 +302,12 @@ public void sendReturnChannelMessage(
302302
}
303303
});
304304

305-
JsonArray serializedChannel = Json.createArray();
306-
serializedChannel.set(0, JsonCodec.RETURN_CHANNEL_TYPE);
307-
serializedChannel.set(1, expectedNodeId);
308-
serializedChannel.set(2, expectedChannelId);
305+
// Create @v-return format
306+
elemental.json.JsonObject serializedChannel = Json.createObject();
307+
JsonArray channelArray = Json.createArray();
308+
channelArray.set(0, expectedNodeId);
309+
channelArray.set(1, expectedChannelId);
310+
serializedChannel.put("@v-return", channelArray);
309311

310312
JsonArray invocation = Json.createArray();
311313
// Assign channel as $0

‎flow-client/src/test-gwt/java/com/vaadin/client/flow/util/GwtClientJsonCodecTest.java‎

Lines changed: 113 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,16 @@
3030
import elemental.json.JsonValue;
3131

3232
public class GwtClientJsonCodecTest extends ClientEngineTestBase {
33+
34+
/**
35+
* Helper method to get property from a decoded object. The decoded object
36+
* is a native JS object, so we need to use native access.
37+
*/
38+
private static native Object getObjectProperty(Object obj, String key)
39+
/*-{
40+
return obj[key];
41+
}-*/;
42+
3343
@Test
3444
public void decodeWithoutTypeInfo() {
3545
decodePrimitiveValues(ClientJsonCodec::decodeWithoutTypeInfo);
@@ -89,12 +99,9 @@ public void decodeWithTypeInfo_element() {
8999
JsElement element = createTestElement();
90100
node.setDomNode(element);
91101

92-
// Create array format [NODE_TYPE, nodeId]
93-
JsonArray json = Json.createArray();
94-
json.set(0, 0); // NODE_TYPE = 0
95-
json.set(1, node.getId());
96-
97-
Object decoded = ClientJsonCodec.decodeWithTypeInfo(tree, json);
102+
// Parse @v-node format from JSON string
103+
Object decoded = ClientJsonCodec.decodeWithTypeInfo(tree,
104+
Json.parse("{\"@v-node\": 42}"));
98105

99106
assertSame(element, decoded);
100107
}
@@ -156,8 +163,107 @@ private static void encodePrimitiveValues(
156163

157164
private static void assertJsonEquals(JsonValue expected, JsonValue actual) {
158165
assertEquals(
159-
actual.toJson() + " does not equal " + expected.toJson(),
166+
"JSON values do not match",
160167
expected.toJson(), actual.toJson());
161168
}
162169

170+
@Test
171+
public void decodeWithTypeInfo_unknownVType_throwsException() {
172+
// Test that unknown @v- prefixed types are rejected for forward
173+
// compatibility
174+
elemental.json.JsonObject unknownType = Json.createObject();
175+
unknownType.put("@v-unknown", "someValue");
176+
177+
try {
178+
ClientJsonCodec.decodeWithTypeInfo(null, unknownType);
179+
fail(
180+
"Expected IllegalArgumentException for unknown @v- type");
181+
} catch (IllegalArgumentException e) {
182+
assertTrue(
183+
"Exception message should mention the unknown key",
184+
e.getMessage().contains("@v-unknown"));
185+
}
186+
}
187+
188+
@Test
189+
public void decodeStateNode_unknownVType_throwsException() {
190+
// Test that unknown @v- prefixed types are rejected in decodeStateNode
191+
// too
192+
elemental.json.JsonObject unknownType = Json.createObject();
193+
unknownType.put("@v-future", 42);
194+
195+
try {
196+
ClientJsonCodec.decodeStateNode(null, unknownType);
197+
fail(
198+
"Expected IllegalArgumentException for unknown @v- type");
199+
} catch (IllegalArgumentException e) {
200+
assertTrue(
201+
"Exception message should mention the unknown key",
202+
e.getMessage().contains("@v-future"));
203+
}
204+
}
205+
206+
@Test
207+
public void decodeWithTypeInfo_deeplyNestedRecursiveDecoding() {
208+
StateTree tree = new StateTree(null);
209+
StateNode node = new StateNode(400, tree);
210+
tree.registerNode(node);
211+
212+
JsElement element = createTestElement();
213+
node.setDomNode(element);
214+
215+
// Parse deeply nested structure from JSON string: object -> array -> object ->
216+
// array -> @v-node
217+
String json = "" //
218+
+ "{" //
219+
+ " \"level\": 1," //
220+
+ " \"nested\": [" //
221+
+ " \"top\"," //
222+
+ " {" //
223+
+ " \"level\": 2," //
224+
+ " \"data\": [" //
225+
+ " {" //
226+
+ " \"level\": 3," //
227+
+ " \"content\": [" //
228+
+ " \"deep\"," //
229+
+ " {\"@v-node\": 400}" //
230+
+ " ]" //
231+
+ " }," //
232+
+ " true" //
233+
+ " ]" //
234+
+ " }" //
235+
+ " ]" //
236+
+ "}";
237+
JsonValue jsonValue = Json.parse(json);
238+
239+
Object decoded = ClientJsonCodec.decodeWithTypeInfo(tree, jsonValue);
240+
241+
// Should return a native JS object with recursively decoded nested
242+
// structures
243+
assertEquals(Double.valueOf(1.0),
244+
getObjectProperty(decoded, "level"));
245+
246+
JsArray<?> nestedArray = (JsArray<?>) getObjectProperty(decoded,
247+
"nested");
248+
assertEquals("top", nestedArray.get(0));
249+
250+
Object midResult = nestedArray.get(1);
251+
assertEquals(Double.valueOf(2.0),
252+
getObjectProperty(midResult, "level"));
253+
254+
JsArray<?> dataArray = (JsArray<?>) getObjectProperty(midResult,
255+
"data");
256+
assertEquals(Boolean.TRUE, dataArray.get(1));
257+
258+
Object deepResult = dataArray.get(0);
259+
assertEquals(3.0,
260+
getObjectProperty(deepResult, "level"));
261+
262+
JsArray<?> contentArray = (JsArray<?>) getObjectProperty(deepResult,
263+
"content");
264+
assertEquals("deep", contentArray.get(0));
265+
assertSame("Deeply nested element should be decoded to DOM node",
266+
element, contentArray.get(1));
267+
}
268+
163269
}

0 commit comments

Comments
 (0)