Skip to content

Commit 312f1bd

Browse files
dmealingclaude
andcommitted
fix(metadata): DATE-array null-element read + pin scalar-on-array-field widening (#275)
Review fix round 1 on the #275 DATE-array storage gap fix. Important #1: MetaObjectDeserializer's readDateElement fell through to el.getAsString() for a JsonNull element (JsonNull is not a JsonPrimitive), which throws UnsupportedOperationException naming no field -- the same bare-throw shape C1 eliminated on the write side. Reachable by design: MetaObjectSerializer deliberately emits JsonNull.INSTANCE at a null element position (pinned by an existing test), so the serializer's own output could not be read back by its sibling deserializer. Fixed with an isJsonNull() guard as the first line of readDateElement, matching what write already emits; added a full-pipeline null-element round-trip test; corrected the readFieldValue DATE-case comment that (falsely) claimed the branch already round-tripped end to end. Important #2: the report's blast-radius argument conflated "no-op for a non-array field" with "no-op for every setObject call site" -- getEffectiveDataType() == getDataType() is a property of the field, not the value. An array-typed field receiving a scalar JSON value (live in MetaObjectDeserializer's own else-arms) genuinely changed from a loud InvalidValueException to a silent single-element wrap (or, for STRING, a comma-split) -- verified both directions by temporarily reverting the E2 fix and confirming the old exception. Not a new rule: it converges with DataObjectBase._setObjectAttribute's pre-existing effective-type conversion, which is what produced this task's own E1 RED evidence in the first place. Added six pinning tests and corrected the report's claim in place. Refs #275 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015TqsuDye2SfXGf43vuoD3n
1 parent 4f46470 commit 312f1bd

2 files changed

Lines changed: 106 additions & 3 deletions

File tree

server/java/metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectDeserializer.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,10 @@ protected void readFieldValue(MetaObject mo, MetaField mf, Object vo,
130130
// AbstractObjectRepresentation.setValue, which applies
131131
// DataConverter.toType(effectiveDataType, value) -- backed, since the #275
132132
// carry-forward unit, by DataConverter.toDateArray. This branch genuinely
133-
// round-trips a date array end to end today (see
134-
// GsonArrayWriteRoundTripTest's Step 3b/A6 coverage).
133+
// round-trips a date array end to end today, INCLUDING a null element (see
134+
// readDateElement's isJsonNull() guard -- required because the write side,
135+
// MetaObjectSerializer, deliberately emits JsonNull.INSTANCE at a null element
136+
// position; see GsonArrayWriteRoundTripTest's Step 3b/A6 coverage).
135137
if (mf.isArrayType() && el.isJsonArray()) {
136138
List<Date> dates = new ArrayList<>();
137139
for (JsonElement item : el.getAsJsonArray()) {
@@ -201,8 +203,15 @@ protected void readFieldValue(MetaObject mo, MetaField mf, Object vo,
201203
}
202204
}
203205

204-
/** Single JSON array element of a DATE-array field: number -> epoch millis, string -> tolerant ISO parse. */
206+
/** Single JSON array element of a DATE-array field: null -> null (JsonNull.getAsString()
207+
* throws UnsupportedOperationException, so this must be checked before isJsonPrimitive --
208+
* JsonNull is not a JsonPrimitive), number -> epoch millis, string -> tolerant ISO parse.
209+
* Mirrors what MetaObjectSerializer.writeField's DATE-array branch emits at a null element
210+
* position (JsonNull.INSTANCE), so a null element round-trips instead of throwing. */
205211
private Date readDateElement(MetaField mf, JsonElement el) {
212+
if (el.isJsonNull()) {
213+
return null;
214+
}
206215
if (el.isJsonPrimitive() && el.getAsJsonPrimitive().isNumber()) {
207216
return new Date(el.getAsLong());
208217
}

server/java/metadata/src/test/java/com/metaobjects/io/object/gson/GsonArrayWriteRoundTripTest.java

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,28 @@ public void dateArray_roundTripsThroughFullGsonPipeline() {
346346
Assert.assertEquals(dates, result.get("dates"));
347347
}
348348

349+
@Test
350+
public void dateArray_withNullElement_roundTripsThroughFullGsonPipeline() {
351+
// Review fix round 1, Important #1: the write side (MetaObjectSerializer, pinned by
352+
// dateArray_withNullElement_writesJsonNullAtThatPosition above) deliberately emits
353+
// JsonNull.INSTANCE at a null element position. The read side must accept what the write
354+
// side produces -- readDateElement checks isJsonNull() first (JsonNull is not a
355+
// JsonPrimitive, so el.getAsString() would otherwise throw
356+
// UnsupportedOperationException, naming no field).
357+
Gson gson = MetaObjectGsonInitializer.getBuilderWithAdapters(arrayLoader).create();
358+
359+
ValueObject vo = newArrayThing();
360+
List<Date> dates = Arrays.asList(utc(2026, 6, 3, 0, 0, 0, 0), null);
361+
vo.put("dates", dates);
362+
363+
String json = gson.toJson(vo);
364+
Assert.assertTrue("expected a null element in the wire array, was: " + json,
365+
json.contains("\"2026-06-03\"") && json.contains("null"));
366+
367+
ValueObject result = (ValueObject) gson.fromJson(json, ValueObject.class);
368+
Assert.assertEquals(dates, result.get("dates"));
369+
}
370+
349371
@Test
350372
public void timestampArray_roundTripsThroughFullGsonPipeline() {
351373
Gson gson = MetaObjectGsonInitializer.getBuilderWithAdapters(arrayLoader).create();
@@ -454,4 +476,76 @@ public void scalarFields_serializeUnaffectedByArraySupport() {
454476
Assert.assertEquals("2026-06-03", obj.get("day").getAsString());
455477
Assert.assertEquals(new java.math.BigDecimal("19.99"), obj.get("price").getAsBigDecimal());
456478
}
479+
480+
// -----------------------------------------------------------------------
481+
// Step 7 — review fix round 1, Important #2: a SCALAR JSON value (not wrapped in an array)
482+
// read onto an array-typed field. Before E2, MetaField.setObject converted via the field's
483+
// scalar getDataType() -- e.g. toType(STRING, "a,b") -> "a,b" identity -- and
484+
// setObjectAttribute's effective-class check (List required for an isArray field) rejected
485+
// it with InvalidValueException: a loud failure. After E2, setObject converts via
486+
// getEffectiveDataType() (the *_ARRAY variant), so the value is coerced into a single- (or,
487+
// for STRING specifically, comma-split multi-) element List and stored WITHOUT error. This is
488+
// not a new behavior invented by this task: DataObjectBase._setObjectAttribute (the
489+
// vo.put()/ValueObject.Map path exercised throughout this file) already converted against the
490+
// effective type before this task, and is exactly what made the RED evidence in this task's
491+
// report (`vo.put("dates", list)` throwing before DataConverter.toDateArray existed) possible
492+
// in the first place. E2 converges MetaObjectDeserializer's setObject-based read path with
493+
// that pre-existing behavior rather than introducing a new rule. Pinned here per line, not
494+
// asserted in prose only, since a live JSON read path (a scalar value where an array is
495+
// expected -- e.g. a legacy or hand-written payload) can reach every one of these.
496+
// -----------------------------------------------------------------------
497+
498+
private ValueObject deserializeArrayThing(String json) {
499+
Gson gson = MetaObjectGsonInitializer.getBuilderWithAdapters(arrayLoader).create();
500+
return (ValueObject) gson.fromJson(json, ValueObject.class);
501+
}
502+
503+
@Test
504+
public void stringArray_scalarJsonValue_commaSplitsIntoMultiElementList() {
505+
// The headline case: a bare JSON string containing a comma is not merely wrapped, it is
506+
// SPLIT -- DataConverter.toStringArray's String branch treats a comma-containing string
507+
// as delimited. Silent, and worth a name of its own.
508+
ValueObject result = deserializeArrayThing(
509+
"{\"@type\":\"test::arrays::ArrayThing\",\"tags\":\"a,b\"}");
510+
Assert.assertEquals(Arrays.asList("a", "b"), result.get("tags"));
511+
}
512+
513+
@Test
514+
public void booleanArray_scalarJsonValue_wrapsAsSingleElementList() {
515+
ValueObject result = deserializeArrayThing(
516+
"{\"@type\":\"test::arrays::ArrayThing\",\"flags\":true}");
517+
Assert.assertEquals(Arrays.asList(true), result.get("flags"));
518+
}
519+
520+
@Test
521+
public void intArray_scalarJsonValue_wrapsAsSingleElementList() {
522+
ValueObject result = deserializeArrayThing(
523+
"{\"@type\":\"test::arrays::ArrayThing\",\"counts\":5}");
524+
Assert.assertEquals(Arrays.asList(5), result.get("counts"));
525+
}
526+
527+
@Test
528+
public void longArray_scalarJsonValue_wrapsAsSingleElementList() {
529+
ValueObject result = deserializeArrayThing(
530+
"{\"@type\":\"test::arrays::ArrayThing\",\"bigCounts\":10}");
531+
Assert.assertEquals(Arrays.asList(10L), result.get("bigCounts"));
532+
}
533+
534+
@Test
535+
public void doubleArray_scalarJsonValue_wrapsAsSingleElementList() {
536+
ValueObject result = deserializeArrayThing(
537+
"{\"@type\":\"test::arrays::ArrayThing\",\"amounts\":3.5}");
538+
Assert.assertEquals(Arrays.asList(3.5), result.get("amounts"));
539+
}
540+
541+
@Test
542+
public void dateArray_scalarJsonNumber_wrapsAsSingleElementList() {
543+
// DATE's "scalar value on an array field" branch is reached independently of the
544+
// isArrayType() check that guards BOOLEAN/INT/LONG/DOUBLE/STRING above -- the DATE case's
545+
// number/string arms run whenever the element isn't a JSON array, array-typed field or
546+
// not (MetaObjectDeserializer.java's DATE case, the `else if isNumber` arm).
547+
ValueObject result = deserializeArrayThing(
548+
"{\"@type\":\"test::arrays::ArrayThing\",\"dates\":1749000000000}");
549+
Assert.assertEquals(Arrays.asList(new Date(1749000000000L)), result.get("dates"));
550+
}
457551
}

0 commit comments

Comments
 (0)