Skip to content

Commit 3113fad

Browse files
Artur-Artur-
authored
chore!: Use Jackson for all JSON deserialization in codecs (#22393)
Simplify JacksonCodec and JsonCodec by using Jackson for all type deserialization instead of custom primitive type handling. This provides consistent behavior and better error messages but removes lenient type conversions (e.g., boolean to integer).
1 parent cdd49b3 commit 3113fad

File tree

8 files changed

+79
-121
lines changed

8 files changed

+79
-121
lines changed

‎flow-polymer-template/src/test/java/com/vaadin/flow/server/communication/rpc/PublishedServerEventHandlerRpcHandlerTest.java‎ renamed to ‎flow-polymer-template/src/test/java/com/vaadin/flow/server/communication/rpc/PolymerPublishedServerEventHandlerRpcHandlerTest.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
import com.vaadin.flow.shared.JsonConstants;
3838

3939
@NotThreadSafe
40-
public class PublishedServerEventHandlerRpcHandlerTest {
40+
public class PolymerPublishedServerEventHandlerRpcHandlerTest {
4141

4242
private VaadinService service;
4343

@@ -452,21 +452,21 @@ public void methodWithVarArg_arrayIsCorrectlyHandled() {
452452
}
453453

454454
@Test
455-
public void nullValueAreAcceptedForPrimitive() {
455+
public void nullValueShouldFailForPrimitive() {
456456
ArrayNode array = JacksonUtils.createArrayNode();
457457
array.add(JacksonUtils.nullNode());
458458
MethodWithParameters component = new MethodWithParameters();
459459
component.intArg = -1;
460460
component.booleanArg = true;
461-
PublishedServerEventHandlerRpcHandler.invokeMethod(component,
462-
component.getClass(), "intMethod", array, -1);
463-
464-
Assert.assertEquals(0, component.intArg);
465461

466-
PublishedServerEventHandlerRpcHandler.invokeMethod(component,
467-
component.getClass(), "booleanMethod", array, -1);
462+
// Passing null to a primitive parameter should throw an exception
463+
Assert.assertThrows(IllegalArgumentException.class, () -> {
464+
PublishedServerEventHandlerRpcHandler.invokeMethod(component,
465+
component.getClass(), "intMethod", array, -1);
466+
});
468467

469-
Assert.assertFalse(component.booleanArg);
468+
// Verify the field was not modified
469+
Assert.assertEquals(-1, component.intArg);
470470
}
471471

472472
@Test(expected = IllegalStateException.class)

‎flow-server/src/main/java/com/vaadin/flow/internal/JacksonCodec.java‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -254,20 +254,11 @@ public static <T> T decodeAs(JsonNode json, Class<T> type) {
254254
if (json.getNodeType() == JsonNodeType.NULL && !type.isPrimitive()) {
255255
return null;
256256
}
257-
Class<?> convertedType = ReflectTools.convertPrimitiveType(type);
258-
if (type == String.class) {
259-
return type.cast(json.asText(""));
260-
} else if (convertedType == Boolean.class) {
261-
return (T) convertedType
262-
.cast(Boolean.valueOf(json.asBoolean(false)));
263-
} else if (convertedType == Double.class) {
264-
return (T) convertedType.cast(Double.valueOf(json.asDouble(0.0)));
265-
} else if (convertedType == Integer.class) {
266-
return (T) convertedType.cast(Integer.valueOf(json.asInt(0)));
267-
} else if (JsonNode.class.isAssignableFrom(type)) {
257+
if (JsonNode.class.isAssignableFrom(type)) {
268258
return type.cast(json);
269259
} else {
270-
// Try to deserialize as a bean using Jackson
260+
// Use Jackson for all deserialization - it handles primitives,
261+
// strings, and complex objects uniformly
271262
try {
272263
return JacksonUtils.getMapper().treeToValue(json, type);
273264
} catch (Exception e) {

‎flow-server/src/main/java/com/vaadin/flow/internal/JsonCodec.java‎

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -245,21 +245,11 @@ public static <T> T decodeAs(JsonValue json, Class<T> type) {
245245
if (json.getType() == JsonType.NULL && !type.isPrimitive()) {
246246
return null;
247247
}
248-
Class<?> convertedType = ReflectTools.convertPrimitiveType(type);
249-
if (type == String.class) {
250-
return type.cast(json.asString());
251-
} else if (convertedType == Boolean.class) {
252-
return (T) convertedType.cast(Boolean.valueOf(json.asBoolean()));
253-
} else if (convertedType == Double.class) {
254-
return (T) convertedType.cast(Double.valueOf(json.asNumber()));
255-
} else if (convertedType == Integer.class) {
256-
return (T) convertedType
257-
.cast(Integer.valueOf((int) json.asNumber()));
258-
} else if (JsonValue.class.isAssignableFrom(type)) {
248+
if (JsonValue.class.isAssignableFrom(type)) {
259249
return type.cast(json);
260250
} else {
261-
// Try to deserialize as a bean using Jackson via JsonValue
262-
// conversion
251+
// Use Jackson for all deserialization via JsonValue conversion -
252+
// it handles primitives, strings, and complex objects uniformly
263253
try {
264254
// Convert JsonValue to JsonNode for Jackson deserialization
265255
JsonNode jsonNode = JacksonUtils.getMapper()

‎flow-server/src/test/java/com/vaadin/flow/component/ComponentEventBusTest.java‎

Lines changed: 41 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -173,20 +173,22 @@ public void serverNoDataEvent_fire_noListeners() {
173173
@Test
174174
public void mappedDomEvent_fire_noListeners() {
175175
TestComponent c = new TestComponent();
176-
fireDomEvent(c, "dom-event", JacksonUtils.createObjectNode());
176+
fireDomEvent(c, "dom-event", createMinimalEventData());
177177
}
178178

179179
@Test
180-
public void mappedDomEvent_fire_missingData() {
180+
public void mappedDomEvent_fire_missingData_shouldFail() {
181181
TestComponent c = new TestComponent();
182182
EventTracker<MappedToDomEvent> eventListener = new EventTracker<>();
183183
c.addListener(MappedToDomEvent.class, eventListener);
184-
fireDomEvent(c, "dom-event", createData("event.someData", 2));
185-
eventListener.assertEventCalled(c, true);
186-
Assert.assertEquals(2, eventListener.getEvent().getSomeData());
187-
Assert.assertNull(eventListener.getEvent().getMoreData());
188-
Assert.assertFalse(eventListener.getEvent().getPrimitiveBoolean());
189-
Assert.assertNull(eventListener.getEvent().getObjectBoolean());
184+
185+
// Missing primitive boolean data should cause event creation to fail
186+
Assert.assertThrows(IllegalArgumentException.class, () -> {
187+
fireDomEvent(c, "dom-event", createData("event.someData", 2));
188+
});
189+
190+
// Event should not have been called due to the failure
191+
eventListener.assertEventNotCalled();
190192
}
191193

192194
@Test
@@ -391,6 +393,23 @@ private JsonNode createData(String key, Object value, String key2,
391393
return data;
392394
}
393395

396+
private JsonNode createCompleteEventData(int someData, String moreData) {
397+
ObjectNode data = JacksonUtils.createObjectNode();
398+
data.set("event.someData",
399+
JacksonCodec.encodeWithoutTypeInfo(someData));
400+
data.set("event.moreData",
401+
JacksonCodec.encodeWithoutTypeInfo(moreData));
402+
data.set("event.primitiveBoolean",
403+
JacksonCodec.encodeWithoutTypeInfo(false));
404+
data.set("event.objectBoolean",
405+
JacksonCodec.encodeWithoutTypeInfo(null));
406+
return data;
407+
}
408+
409+
private JsonNode createMinimalEventData() {
410+
return createCompleteEventData(0, "");
411+
}
412+
394413
@Test
395414
public void domEvent_removeListener() {
396415
TestComponent component = new TestComponent();
@@ -399,8 +418,7 @@ public void domEvent_removeListener() {
399418
eventTracker);
400419
remover.remove();
401420

402-
JsonNode eventData = createData("event.someData", 42, "event.moreData",
403-
1);
421+
JsonNode eventData = createCompleteEventData(42, "1");
404422
fireDomEvent(component, "dom-event", eventData);
405423

406424
eventTracker.assertEventNotCalled();
@@ -417,14 +435,15 @@ public void domEvent_fireClientEvent() {
417435
EventTracker<MappedToDomEvent> eventTracker = new EventTracker<>();
418436
component.addListener(MappedToDomEvent.class, eventTracker);
419437

420-
JsonNode eventData = createData("event.someData", 42, "event.moreData",
421-
1);
438+
JsonNode eventData = createCompleteEventData(42, "1");
422439
fireDomEvent(component, "dom-event", eventData);
423440

424441
eventTracker.assertEventCalled(component, true);
425442
MappedToDomEvent event = eventTracker.getEvent();
426443
Assert.assertEquals(42, event.getSomeData());
427444
Assert.assertEquals("1", event.getMoreData());
445+
Assert.assertFalse(event.getPrimitiveBoolean());
446+
Assert.assertNull(event.getObjectBoolean());
428447
}
429448

430449
@Test
@@ -433,15 +452,15 @@ public void domEvent_fireServerEvent() {
433452
EventTracker<MappedToDomEvent> eventTracker = new EventTracker<>();
434453
component.addListener(MappedToDomEvent.class, eventTracker);
435454

436-
ObjectNode eventData = JacksonUtils.createObjectNode();
437-
eventData.put("event.someData", 42);
438-
eventData.put("event.moreData", 1);
455+
JsonNode eventData = createCompleteEventData(42, "1");
439456
fireDomEvent(component, "dom-event", eventData);
440457

441458
eventTracker.assertEventCalled(component, true);
442459
MappedToDomEvent event = eventTracker.getEvent();
443460
Assert.assertEquals(42, event.getSomeData());
444461
Assert.assertEquals("1", event.getMoreData());
462+
Assert.assertFalse(event.getPrimitiveBoolean());
463+
Assert.assertNull(event.getObjectBoolean());
445464
}
446465

447466
@Test
@@ -503,7 +522,7 @@ public void domEvent_addSameListenerTwice() {
503522
Assert.assertEquals(2, component.getEventBus().componentEventData
504523
.get(MappedToDomEvent.class).size());
505524

506-
fireDomEvent(component, "dom-event", JacksonUtils.createObjectNode());
525+
fireDomEvent(component, "dom-event", createMinimalEventData());
507526
Assert.assertEquals(2, calls);
508527

509528
reg1.remove();
@@ -512,15 +531,15 @@ public void domEvent_addSameListenerTwice() {
512531
Assert.assertEquals(1, component.getEventBus().componentEventData
513532
.get(MappedToDomEvent.class).size());
514533

515-
fireDomEvent(component, "dom-event", JacksonUtils.createObjectNode());
534+
fireDomEvent(component, "dom-event", createMinimalEventData());
516535

517536
Assert.assertEquals(3, calls);
518537

519538
reg2.remove();
520539
Assert.assertEquals(0,
521540
component.getEventBus().componentEventData.size());
522541

523-
fireDomEvent(component, "dom-event", JacksonUtils.createObjectNode());
542+
fireDomEvent(component, "dom-event", createMinimalEventData());
524543
Assert.assertEquals(3, calls);
525544
}
526545

@@ -536,8 +555,7 @@ public void multipleEventsForSameDomEvent_removeListener() {
536555
.addListener(MappedToDomNoDataEvent.class, eventTracker2);
537556
remover.remove();
538557

539-
JsonNode eventData = createData("event.someData", 42, "event.moreData",
540-
1);
558+
JsonNode eventData = createCompleteEventData(42, "1");
541559
fireDomEvent(component, "dom-event", eventData);
542560

543561
eventTracker.assertEventNotCalled();
@@ -556,8 +574,7 @@ public void multipleEventsForSameDomEvent_fireEvent() {
556574
component.addListener(MappedToDomEvent.class, eventTracker);
557575
component.addListener(MappedToDomNoDataEvent.class, eventTracker2);
558576

559-
JsonNode eventData = createData("event.someData", 42, "event.moreData",
560-
19);
577+
JsonNode eventData = createCompleteEventData(42, "19");
561578
fireDomEvent(component, "dom-event", eventData);
562579

563580
eventTracker.assertEventCalled(component, true);
@@ -577,8 +594,7 @@ public void multipleListenersForSameEvent_fireEvent() {
577594
component.addListener(MappedToDomEvent.class, eventTracker);
578595
component.addListener(MappedToDomEvent.class, eventTracker2);
579596

580-
JsonNode eventData = createData("event.someData", 42, "event.moreData",
581-
19);
597+
JsonNode eventData = createCompleteEventData(42, "19");
582598
fireDomEvent(component, "dom-event", eventData);
583599

584600
eventTracker.assertEventCalled(component, true);
@@ -603,8 +619,7 @@ public void multipleListenersForSameEvent_removeListener() {
603619
eventTracker2);
604620
remover.remove();
605621

606-
JsonNode eventData = createData("event.someData", 42, "event.moreData",
607-
19);
622+
JsonNode eventData = createCompleteEventData(42, "19");
608623
fireDomEvent(component, "dom-event", eventData);
609624

610625
eventTracker.assertEventNotCalled();

‎flow-server/src/test/java/com/vaadin/flow/internal/JacksonCodecTest.java‎

Lines changed: 11 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -152,36 +152,32 @@ private static void assertJsonEquals(JsonNode expected, JsonNode actual) {
152152
public void decodeAs_booleanJson() {
153153
JsonNode json = objectMapper.valueToTree(true);
154154
Assert.assertTrue(JacksonCodec.decodeAs(json, Boolean.class));
155-
Assert.assertEquals("true", JacksonCodec.decodeAs(json, String.class));
156-
Assert.assertEquals(Integer.valueOf(0),
157-
JacksonCodec.decodeAs(json, Integer.class));
158-
Assert.assertEquals(Double.valueOf(0.0),
159-
JacksonCodec.decodeAs(json, Double.class));
160155
Assert.assertEquals(json, JacksonCodec.decodeAs(json, JsonNode.class));
161156
}
162157

163158
@Test
164159
public void decodeAs_stringJson() {
165160
JsonNode json = objectMapper.valueToTree("Test123 String\n !%");
166-
Assert.assertFalse(JacksonCodec.decodeAs(json, Boolean.class));
167161
Assert.assertEquals("Test123 String\n !%",
168162
JacksonCodec.decodeAs(json, String.class));
169-
Assert.assertEquals(Integer.valueOf(0),
170-
JacksonCodec.decodeAs(json, Integer.class));
171-
Assert.assertFalse(JacksonCodec.decodeAs(json, Double.class).isNaN());
172163
Assert.assertEquals(json, JacksonCodec.decodeAs(json, JsonNode.class));
173164
}
174165

175166
@Test
176167
public void decodeAs_numberJson() {
177-
JsonNode json = objectMapper.valueToTree(15.7);
178-
Assert.assertFalse(JacksonCodec.decodeAs(json, Boolean.class));
179-
Assert.assertEquals("15.7", JacksonCodec.decodeAs(json, String.class));
168+
// Test integer
169+
JsonNode intJson = objectMapper.valueToTree(15);
180170
Assert.assertEquals(Integer.valueOf(15),
181-
JacksonCodec.decodeAs(json, Integer.class));
171+
JacksonCodec.decodeAs(intJson, Integer.class));
172+
Assert.assertEquals(Double.valueOf(15.0),
173+
JacksonCodec.decodeAs(intJson, Double.class));
174+
175+
// Test double
176+
JsonNode doubleJson = objectMapper.valueToTree(15.7);
182177
Assert.assertEquals(Double.valueOf(15.7),
183-
JacksonCodec.decodeAs(json, Double.class));
184-
Assert.assertEquals(json, JacksonCodec.decodeAs(json, JsonNode.class));
178+
JacksonCodec.decodeAs(doubleJson, Double.class));
179+
Assert.assertEquals(doubleJson,
180+
JacksonCodec.decodeAs(doubleJson, JsonNode.class));
185181
}
186182

187183
@Test
@@ -198,34 +194,7 @@ public void decodeAs_nullJson() {
198194
public void decodeAs_jsonValue() {
199195
ObjectNode json = objectMapper.createObjectNode();
200196
json.put("foo", "bar");
201-
Assert.assertEquals("", JacksonCodec.decodeAs(json, String.class));
202197
Assert.assertEquals(json, JacksonCodec.decodeAs(json, JsonNode.class));
203-
// boolean
204-
Assert.assertFalse(JacksonCodec.decodeAs(json, Boolean.class));
205-
Assert.assertNull(
206-
JacksonCodec.decodeAs(objectMapper.nullNode(), Boolean.class));
207-
Assert.assertFalse(JacksonCodec.decodeAs(json, boolean.class));
208-
Assert.assertFalse(
209-
JacksonCodec.decodeAs(objectMapper.nullNode(), boolean.class));
210-
// integer
211-
Assert.assertEquals(Integer.valueOf(0),
212-
JacksonCodec.decodeAs(json, Integer.class));
213-
Assert.assertNull(
214-
JacksonCodec.decodeAs(objectMapper.nullNode(), Integer.class));
215-
Assert.assertEquals(Integer.valueOf(0),
216-
JacksonCodec.decodeAs(json, int.class));
217-
Assert.assertEquals(Integer.valueOf(0),
218-
JacksonCodec.decodeAs(objectMapper.nullNode(), int.class));
219-
// double
220-
Assert.assertNull(
221-
JacksonCodec.decodeAs(objectMapper.nullNode(), Double.class));
222-
Assert.assertEquals(Double.valueOf(0.0),
223-
JacksonCodec.decodeAs(json, Double.class));
224-
Assert.assertEquals(Double.valueOf(0.0),
225-
JacksonCodec.decodeAs(json, double.class));
226-
Assert.assertEquals(0.0d,
227-
JacksonCodec.decodeAs(objectMapper.nullNode(), double.class),
228-
0.0001d);
229198
}
230199

231200
@Test(expected = ClassCastException.class)

‎flow-server/src/test/java/com/vaadin/flow/server/communication/rpc/PublishedServerEventHandlerRpcHandlerTest.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -495,21 +495,21 @@ public void methodWithVarArg_arrayIsCorrectlyHandled() {
495495
}
496496

497497
@Test
498-
public void nullValueAreAcceptedForPrimitive() {
498+
public void nullValueShouldFailForPrimitive() {
499499
ArrayNode array = JacksonUtils.createArrayNode();
500500
array.add(JacksonUtils.nullNode());
501501
MethodWithParameters component = new MethodWithParameters();
502502
component.intArg = -1;
503503
component.booleanArg = true;
504-
PublishedServerEventHandlerRpcHandler.invokeMethod(component,
505-
component.getClass(), "intMethod", array, -1);
506-
507-
Assert.assertEquals(0, component.intArg);
508504

509-
PublishedServerEventHandlerRpcHandler.invokeMethod(component,
510-
component.getClass(), "booleanMethod", array, -1);
505+
// Passing null to a primitive parameter should throw an exception
506+
Assert.assertThrows(IllegalArgumentException.class, () -> {
507+
PublishedServerEventHandlerRpcHandler.invokeMethod(component,
508+
component.getClass(), "intMethod", array, -1);
509+
});
511510

512-
Assert.assertFalse(component.booleanArg);
511+
// Verify the field was not modified
512+
Assert.assertEquals(-1, component.intArg);
513513
}
514514

515515
@Test(expected = IllegalStateException.class)

‎flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/JavaScriptReturnValueView.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ public JavaScriptReturnValueView() {
5757
valueSelect.addOption("String", "'foo'");
5858
valueSelect.addOption("Number", "42");
5959
valueSelect.addOption("null", "null");
60-
valueSelect.addOption("Error", "new Error('message')", "error-value");
6160

6261
// Promise semantics to use
6362
NativeRadioButtonGroup<String> resolveRejectSelect = new NativeRadioButtonGroup<>(

0 commit comments

Comments
 (0)