From 4f46470d67f8fbfd5a95dffd4fe4297e46558b0c Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Sat, 8 Aug 2026 17:55:47 -0400 Subject: [PATCH 1/2] fix(metadata): close the DATE-array storage gap left by #275 Two root defects in the JSON array-write storage path, deliberately left unfixed by the #275 batch as out-of-scope scope-creep triggers: - MetaField.setObject(Object,Object) converted via the field's SCALAR getDataType() instead of the array-aware getEffectiveDataType(), so an isArray field's List value was corrupted (comma-joined / bracketed toString()) before setObjectAttribute's own instanceof check rejected it. Broke setBoolean/setInt/setLong/setDouble/setStringArray, and every MetaObjectDeserializer array-read branch that routes through it. - DataConverter had no DATE_ARRAY case (the commented-out `//toDateArray(val)` fragment), so no entry point could store a List on an isArray field.date/field.timestamp field. Fix: getEffectiveDataType() in MetaField.setObject (a strict no-op for every non-array field); a new DataConverter.toDateArray mirroring the sibling toLongArray/toBooleanArray shape, wired into case DATE_ARRAY. BYTE_ARRAY/SHORT_ARRAY stay on the unsupported arm (field.byte/field.short are non-functional stubs). Also closes the deferred findings this unblocked: MetaObjectSerializer's DATE-array element loop now converts via DataConverter.toDate(o) instead of a hard cast; field.timestamp (incl. @localTime) array coverage; a full-Gson-pipeline round trip for the previously-untestable DATE-array read path in MetaObjectDeserializer; null-array-itself pins extended to every touched type; and a stale MetaObjectDeserializer comment describing the DATE-array branch as blocked, now rewritten to match reality. Blast-radius checked: every MetaField.setObject caller either passes a scalar value (no-op under the fix) or already routed around the array bug via setObjectArray/setValue; no caller depended on the corrupting conversion succeeding. Refs #275 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015TqsuDye2SfXGf43vuoD3n --- .../java/com/metaobjects/field/MetaField.java | 6 +- .../object/gson/MetaObjectDeserializer.java | 14 +- .../io/object/gson/MetaObjectSerializer.java | 9 +- .../com/metaobjects/util/DataConverter.java | 29 +- .../gson/GsonArrayWriteRoundTripTest.java | 268 +++++++++++++----- .../metaobjects/util/DataConverterTests.java | 50 +++- .../object/gson/array-primitive-metadata.json | 2 + 7 files changed, 302 insertions(+), 76 deletions(-) diff --git a/server/java/metadata/src/main/java/com/metaobjects/field/MetaField.java b/server/java/metadata/src/main/java/com/metaobjects/field/MetaField.java index 1c6eb1dda..60cf791ea 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/field/MetaField.java +++ b/server/java/metadata/src/main/java/com/metaobjects/field/MetaField.java @@ -1089,7 +1089,11 @@ public void setDate(Object obj, Date value) { } public void setObject(Object obj, Object value) { - setObjectAttribute(obj, DataConverter.toType(getDataType(), value )); + // ADR-0039: use the RESOLVING, array-aware getEffectiveDataType() -- the field's SCALAR + // getDataType() corrupted a List for an isArray field (e.g. comma-joining a STRING array + // into a single string) before setObjectAttribute's own instanceof check rejected it. + // getEffectiveDataType() is a strict no-op for every non-array field (#275 carry-forward). + setObjectAttribute(obj, DataConverter.toType(getEffectiveDataType(), value )); } public void setObjectArray(Object obj, List value) { diff --git a/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectDeserializer.java b/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectDeserializer.java index f97e4b67e..381fdb381 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectDeserializer.java +++ b/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectDeserializer.java @@ -126,14 +126,12 @@ protected void readFieldValue(MetaObject mo, MetaField mf, Object vo, // nothing that parsed before stops parsing). String -> tolerant ISO parse // (TemporalWireFormat). Array: element-wise into a List via // setObjectArray, skipping context.deserialize(el, List.class), which yields a - // type-losing List. setObjectArray only bypasses MetaField's OWN - // DataConverter.toType call; setObjectAttribute still routes through - // AbstractObjectRepresentation.setValue, which unconditionally applies - // DataConverter.toType(effectiveDataType, value) -- and DataConverter's - // DATE_ARRAY case is unimplemented (unsupported()). So today this branch - // throws UnsupportedOperationException for a non-empty date array on the - // default (non-proxy) representation path; it becomes correct once - // DataConverter grows a DATE_ARRAY conversion. + // type-losing List. setObjectArray routes through + // AbstractObjectRepresentation.setValue, which applies + // DataConverter.toType(effectiveDataType, value) -- backed, since the #275 + // carry-forward unit, by DataConverter.toDateArray. This branch genuinely + // round-trips a date array end to end today (see + // GsonArrayWriteRoundTripTest's Step 3b/A6 coverage). if (mf.isArrayType() && el.isJsonArray()) { List dates = new ArrayList<>(); for (JsonElement item : el.getAsJsonArray()) { diff --git a/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectSerializer.java b/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectSerializer.java index 11263b3e0..4758323dd 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectSerializer.java +++ b/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectSerializer.java @@ -8,6 +8,7 @@ import com.metaobjects.loader.MetaDataLoader; import com.metaobjects.object.MetaObject; import com.metaobjects.object.MetaObjectAware; +import com.metaobjects.util.DataConverter; import com.google.gson.*; import static com.metaobjects.io.json.JsonIOUtil.*; @@ -94,7 +95,13 @@ protected void writeField(MetaObject mo, MetaField mf, Object vo, } else { JsonArray arr = new JsonArray(); for (Object o : dates) { - java.util.Date d = (java.util.Date) o; + // C1: route through the shared DataConverter.toDate(Object) rather + // than a hard (Date) cast -- matches the non-array DATE branch below + // (mf.getDate(vo) is itself backed by DataConverter.toDate), and + // accepts the same scalar inputs that converter always has, instead + // of throwing a bare ClassCastException for anything not already a + // java.util.Date. + java.util.Date d = DataConverter.toDate(o); if (d == null) arr.add(JsonNull.INSTANCE); else arr.add(TemporalWireFormat.format(mf, d)); } diff --git a/server/java/metadata/src/main/java/com/metaobjects/util/DataConverter.java b/server/java/metadata/src/main/java/com/metaobjects/util/DataConverter.java index c98c0d6d8..8737bf8f8 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/util/DataConverter.java +++ b/server/java/metadata/src/main/java/com/metaobjects/util/DataConverter.java @@ -47,9 +47,9 @@ public static Object toType( DataTypes dataType, Object val ) { case BYTE_ARRAY://return toByteArray( val ); case SHORT_ARRAY: //return toShortArray( val ); - case DATE_ARRAY: //toDateArray( val ); return unsupported(dataType,val); + case DATE_ARRAY: return toDateArray( val ); case STRING_ARRAY: return toStringArray( val ); case OBJECT_ARRAY: return toObjectArray( val ); @@ -755,6 +755,33 @@ public static List toBooleanArray(Object val) { } } + /** + * Convert value to Date array (List<Date>) + */ + public static List toDateArray(Object val) { + if (val == null) return null; + + if (val instanceof List) { + List list = (List) val; + return list.stream() + .map(DataConverter::toDate) + .collect(java.util.stream.Collectors.toList()); + } else if (val instanceof String) { + String s = (String) val; + if (s.trim().isEmpty()) return new java.util.ArrayList<>(); + + if (s.contains(",")) { + return java.util.Arrays.stream(s.split(",")) + .map(item -> toDate(item.trim())) + .collect(java.util.stream.Collectors.toList()); + } else { + return java.util.Arrays.asList(toDate(s.trim())); + } + } else { + return java.util.Arrays.asList(toDate(val)); + } + } + /** * Convert value to Double array (List<Double>) */ diff --git a/server/java/metadata/src/test/java/com/metaobjects/io/object/gson/GsonArrayWriteRoundTripTest.java b/server/java/metadata/src/test/java/com/metaobjects/io/object/gson/GsonArrayWriteRoundTripTest.java index 99125011e..3462fa56d 100644 --- a/server/java/metadata/src/test/java/com/metaobjects/io/object/gson/GsonArrayWriteRoundTripTest.java +++ b/server/java/metadata/src/test/java/com/metaobjects/io/object/gson/GsonArrayWriteRoundTripTest.java @@ -4,7 +4,6 @@ import com.google.gson.JsonArray; import com.google.gson.JsonElement; import com.google.gson.JsonObject; -import com.metaobjects.InvalidValueException; import com.metaobjects.field.MetaField; import com.metaobjects.loader.MetaDataLoader; import com.metaobjects.object.MetaObject; @@ -29,23 +28,23 @@ * {@code DataConverter.toX(getObjectAttribute(obj))} — for an array-valued field the raw stored * attribute is a {@code List}, and {@code DataConverter.toX} has no {@code List} case for most * primitive types, so it falls through to the bracketed native {@code List.toString()} (or, for - * STRING, a comma-join) — silent round-trip corruption. + * STRING, a comma-join) — silent round-trip corruption. Fixed by the write-side {@code isArrayType()} + * dispatch added in {@code MetaObjectSerializer.writeField}. * - *

Test-setup note: {@code MetaField.setObject(Object,Object)} (the method backing - * {@code setBoolean}/{@code setInt}/{@code setLong}/{@code setDouble}/{@code setStringArray}) is - * itself broken for an {@code isArray} field — it converts via {@code DataConverter.toType( - * getDataType(), value)}, the field's SCALAR (not effective/array) type, so a {@code List} is - * corrupted before {@code setObjectAttribute}'s own instanceof check rejects it. This is a - * separate, pre-existing, out-of-scope defect (see the "discovered but out of scope" tests below) - * — NOT fixed here. It means the standard {@code mf.setX(vo, list)} entry points cannot be used to - * build array-valued fixtures. Instead, array fields here are populated via {@link ValueObject}'s - * own {@code Map} interface ({@code vo.put(name, list)}), which routes through - * {@code DataObjectBase._setObjectAttribute} — a DIFFERENT storage path that correctly converts - * via the field's EFFECTIVE (array) data type — for every type this task's write-side fix covers - * EXCEPT {@code field.date}/{@code field.timestamp}, whose {@code DATE_ARRAY} conversion is wholly - * unimplemented in {@code DataConverter} (see the DATE tests, which instead call {@code - * MetaObjectSerializer.writeField} directly against a plain {@code Map}-backed value object, - * sidestepping the storage layer entirely to isolate the WRITE-side logic under test). + *

The sibling STORAGE-layer defect this class also exercises (the #275 carry-forward unit): + * {@code MetaField.setObject(Object,Object)} — the method backing {@code setBoolean}/{@code + * setInt}/{@code setLong}/{@code setDouble}/{@code setStringArray}, and called directly by {@code + * MetaObjectDeserializer}'s array-read branches — used to convert via {@code + * DataConverter.toType(getDataType(), value)}, the field's SCALAR (not effective/array) type, so a + * {@code List} was corrupted before {@code setObjectAttribute}'s own instanceof check rejected it. + * Now fixed: {@code setObject} converts via {@code getEffectiveDataType()}, which is the + * array-equivalent type for an {@code isArray} field and a strict no-op for every scalar field. + * {@code DataConverter} also gained a {@code DATE_ARRAY} case ({@code toDateArray}), so {@code + * field.date}/{@code field.timestamp} array fields now have a working storage path end to end. + * Array fields here are populated via {@link ValueObject}'s {@code Map} interface + * ({@code vo.put(name, list)}, routing through {@code DataObjectBase._setObjectAttribute}) — + * that path already converted via the field's EFFECTIVE (array) data type even before this fix, so + * it exercises the SAME storage layer {@code MetaField.setObject} now also correctly reaches. */ public class GsonArrayWriteRoundTripTest { @@ -173,16 +172,94 @@ public void doubleArray_writesProperJsonArray() { } // ----------------------------------------------------------------------- - // Step 3 — DATE array. field.date/field.timestamp isArray has NO working storage path - // ANYWHERE in this codebase today (DataConverter.toType has no DATE_ARRAY case, hit - // unconditionally by AbstractObjectRepresentation.setValue regardless of entry point -- - // see the class Javadoc). So these tests call MetaObjectSerializer.writeField DIRECTLY - // (same package; protected access) against a plain Map-backed "value object", which is - // read via AbstractObjectRepresentation.getValue's Map branch (a raw, unconverted get) -- - // isolating the WRITE-side logic under test from that separate, out-of-scope defect. + // Step 2b — null-array-itself pin (C3), extended from DATE (Step 3 below already covered + // it) to every other touched type: the whole field is JSON null, not an empty/absent array. + // + // .serializeNulls() is required here: these branches write via context.serialize(...), and + // Gson's default (serializeNulls==false) DROPS a JsonObject member whose value is + // JsonNull.INSTANCE when the tree is re-written by the outer toJsonTree() pass -- unrelated + // to this task's storage fix, just Gson's ordinary null-suppression default. The DATE branch + // below doesn't need this because its null-array-itself pin calls writeField() directly + // against a raw JsonObject, bypassing that outer re-write pass entirely. + // ----------------------------------------------------------------------- + + @Test + public void stringArray_nullArrayItself_writesJsonNullForWholeField() { + Gson gson = MetaObjectGsonInitializer.getBuilderWithAdapters(arrayLoader).serializeNulls().create(); + + ValueObject vo = newArrayThing(); + vo.put("tags", null); + + JsonObject obj = gson.toJsonTree(vo).getAsJsonObject(); + JsonElement el = obj.get("tags"); + + Assert.assertTrue("expected JSON null for the whole field, was: " + el, el.isJsonNull()); + } + + @Test + public void intArray_nullArrayItself_writesJsonNullForWholeField() { + Gson gson = MetaObjectGsonInitializer.getBuilderWithAdapters(arrayLoader).serializeNulls().create(); + + ValueObject vo = newArrayThing(); + vo.put("counts", null); + + JsonObject obj = gson.toJsonTree(vo).getAsJsonObject(); + JsonElement el = obj.get("counts"); + + Assert.assertTrue("expected JSON null for the whole field, was: " + el, el.isJsonNull()); + } + + @Test + public void longArray_nullArrayItself_writesJsonNullForWholeField() { + Gson gson = MetaObjectGsonInitializer.getBuilderWithAdapters(arrayLoader).serializeNulls().create(); + + ValueObject vo = newArrayThing(); + vo.put("bigCounts", null); + + JsonObject obj = gson.toJsonTree(vo).getAsJsonObject(); + JsonElement el = obj.get("bigCounts"); + + Assert.assertTrue("expected JSON null for the whole field, was: " + el, el.isJsonNull()); + } + + @Test + public void booleanArray_nullArrayItself_writesJsonNullForWholeField() { + Gson gson = MetaObjectGsonInitializer.getBuilderWithAdapters(arrayLoader).serializeNulls().create(); + + ValueObject vo = newArrayThing(); + vo.put("flags", null); + + JsonObject obj = gson.toJsonTree(vo).getAsJsonObject(); + JsonElement el = obj.get("flags"); + + Assert.assertTrue("expected JSON null for the whole field, was: " + el, el.isJsonNull()); + } + + @Test + public void doubleArray_nullArrayItself_writesJsonNullForWholeField() { + Gson gson = MetaObjectGsonInitializer.getBuilderWithAdapters(arrayLoader).serializeNulls().create(); + + ValueObject vo = newArrayThing(); + vo.put("amounts", null); + + JsonObject obj = gson.toJsonTree(vo).getAsJsonObject(); + JsonElement el = obj.get("amounts"); + + Assert.assertTrue("expected JSON null for the whole field, was: " + el, el.isJsonNull()); + } + + // ----------------------------------------------------------------------- + // Step 3 — DATE array, write side in isolation. field.date/field.timestamp isArray DOES + // have a working storage path now (DataConverter.toDateArray backs DATE_ARRAY; see the + // full-pipeline round trips in Step 3b below). These tests still call + // MetaObjectSerializer.writeField DIRECTLY (same package; protected access) against a plain + // Map-backed "value object", read via AbstractObjectRepresentation.getValue's Map branch (a + // raw, unconverted get) -- isolating the WRITE-side formatting logic from storage entirely, + // the same isolation technique used before the storage fix, kept because it targets a + // narrower unit than the full round trip. // ----------------------------------------------------------------------- - private JsonObject writeDatesField(List dates) { + private JsonObject writeDatesField(List dates) { MetaObject mo = arrayMetaObject(); MetaField mf = arrayField("dates"); Map vo = new HashMap<>(); @@ -207,6 +284,22 @@ public void dateArray_writesJsonArrayOfWireFormStrings() { Assert.assertEquals("2026-07-04", arr.get(1).getAsString()); } + @Test + public void dateArray_withNonDateConvertibleElement_convertsViaDataConverter() { + // C1: MetaObjectSerializer's DATE-array element loop routes each element through + // DataConverter.toDate(o) rather than a hard (Date) cast -- so a non-Date element that + // toDate CAN convert (e.g. a Long epoch-millis value, same as the scalar DATE branch + // would accept) converts instead of throwing a bare ClassCastException. + JsonObject obj = writeDatesField(Arrays.asList( + utc(2026, 6, 3, 0, 0, 0, 0), + utc(2026, 7, 4, 0, 0, 0, 0).getTime())); + JsonArray arr = obj.get("dates").getAsJsonArray(); + + Assert.assertEquals(2, arr.size()); + Assert.assertEquals("2026-06-03", arr.get(0).getAsString()); + Assert.assertEquals("2026-07-04", arr.get(1).getAsString()); + } + @Test public void dateArray_withNullElement_writesJsonNullAtThatPosition() { JsonObject obj = writeDatesField(Arrays.asList(utc(2026, 6, 3, 0, 0, 0, 0), null)); @@ -225,64 +318,111 @@ public void dateArray_nullArrayItself_writesJsonNullForWholeField() { Assert.assertTrue("expected JSON null for the whole field, was: " + el, el.isJsonNull()); } + // ----------------------------------------------------------------------- + // Step 3b — DATE array (field.date) and TIMESTAMP array (field.timestamp, plain and + // @localTime) through the REAL Gson pipeline end to end: vo.put populates via the storage + // layer this task fixes (DataConverter.toDateArray backing DATE_ARRAY), gson.toJson writes, + // gson.fromJson reads back through MetaObjectDeserializer's own array-read branch. Before + // this fix vo.put("dates", list) itself threw UnsupportedOperationException (DATE_ARRAY was + // unimplemented), so this path -- including the deserializer's DATE-array READ branch -- was + // untestable dead code (C4/C5/A6). + // ----------------------------------------------------------------------- + + @Test + public void dateArray_roundTripsThroughFullGsonPipeline() { + Gson gson = MetaObjectGsonInitializer.getBuilderWithAdapters(arrayLoader).create(); + + ValueObject vo = newArrayThing(); + List dates = Arrays.asList( + utc(2026, 6, 3, 0, 0, 0, 0), + utc(2026, 7, 4, 0, 0, 0, 0)); + vo.put("dates", dates); + + String json = gson.toJson(vo); + Assert.assertTrue("expected wire-form date strings, was: " + json, + json.contains("\"2026-06-03\"") && json.contains("\"2026-07-04\"")); + + ValueObject result = (ValueObject) gson.fromJson(json, ValueObject.class); + Assert.assertEquals(dates, result.get("dates")); + } + + @Test + public void timestampArray_roundTripsThroughFullGsonPipeline() { + Gson gson = MetaObjectGsonInitializer.getBuilderWithAdapters(arrayLoader).create(); + + ValueObject vo = newArrayThing(); + List timestamps = Arrays.asList( + utc(2026, 6, 3, 14, 30, 0, 123), + utc(2026, 7, 4, 0, 0, 0, 0)); + vo.put("timestamps", timestamps); + + String json = gson.toJson(vo); + Assert.assertTrue("expected tz-aware wire-form timestamp strings, was: " + json, + json.contains("\"2026-06-03T14:30:00.123Z\"") && json.contains("\"2026-07-04T00:00:00Z\"")); + + ValueObject result = (ValueObject) gson.fromJson(json, ValueObject.class); + Assert.assertEquals(timestamps, result.get("timestamps")); + } + + @Test + public void localTimestampArray_roundTripsThroughFullGsonPipeline() { + Gson gson = MetaObjectGsonInitializer.getBuilderWithAdapters(arrayLoader).create(); + + ValueObject vo = newArrayThing(); + List timestamps = Arrays.asList( + utc(2026, 6, 3, 14, 30, 0, 123), + utc(2026, 7, 4, 0, 0, 0, 0)); + vo.put("localTimestamps", timestamps); + + String json = gson.toJson(vo); + Assert.assertTrue("expected naive wall-clock wire-form strings (no trailing Z), was: " + json, + json.contains("\"2026-06-03T14:30:00.123\"") && json.contains("\"2026-07-04T00:00:00\"")); + + ValueObject result = (ValueObject) gson.fromJson(json, ValueObject.class); + Assert.assertEquals(timestamps, result.get("localTimestamps")); + } + // ----------------------------------------------------------------------- // Step 5 — round trip through the existing (unmodified) MetaObjectDeserializer. // - // BLOCKED by a separate, pre-existing, out-of-scope defect: MetaObjectDeserializer's own - // array-read branches populate the field via MetaField.setStringArray/setObject/setObjectArray - // -- and MetaField.setObject(Object,Object) converts via DataConverter.toType(getDataType(), - // value), the field's SCALAR type, not its EFFECTIVE (array) type -- so a List is corrupted - // (STRING: comma-joined to "a,b"; numeric types: bracketed toString() fed to a parser) BEFORE - // setObjectAttribute's own instanceof-against-List check rejects it. This affects every - // BOOLEAN/BYTE/SHORT/INT/LONG/FLOAT/DOUBLE/STRING isArray field via MetaField.setObject, and - // separately affects DATE via DataConverter's wholly-unimplemented DATE_ARRAY case. It is - // NOT specific to Gson or to this task's write-side fix -- it would break ANY caller trying to - // populate an isArray primitive field through MetaField's typed setters, deserializer included. - // Out of scope per the brief's Stop-and-escalate clause (fixing it requires touching MetaField - // and/or DataConverter, not MetaObjectSerializer.writeField). Pinned here, not fixed, as - // evidence + a regression guard for whenever that separate defect is addressed. + // Previously BLOCKED by a separate, pre-existing defect, now fixed: MetaObjectDeserializer's + // own array-read branches populate the field via MetaField.setStringArray/setObject/ + // setObjectArray -- and MetaField.setObject(Object,Object) used to convert via + // DataConverter.toType(getDataType(), value), the field's SCALAR type, not its EFFECTIVE + // (array) type, corrupting a List before setObjectAttribute's own instanceof-against-List + // check rejected it. setObject now converts via getEffectiveDataType(), so these round-trip. // ----------------------------------------------------------------------- @Test - public void stringArray_roundTripThroughDeserializer_blockedByPreexistingSetterBug() { + public void stringArray_roundTripsThroughDeserializer() { Gson gson = MetaObjectGsonInitializer.getBuilderWithAdapters(arrayLoader).create(); ValueObject vo = newArrayThing(); - vo.put("tags", Arrays.asList("a", "b")); + List tags = Arrays.asList("a", "b"); + vo.put("tags", tags); String json = gson.toJson(vo); - try { - gson.fromJson(json, ValueObject.class); - Assert.fail("Expected the pre-existing MetaField.setObject scalar-dataType bug to " - + "throw; if this now succeeds, that separate defect has been fixed and this " - + "test should be replaced with a real round-trip assertion."); - } catch (InvalidValueException e) { - Assert.assertTrue(e.getMessage(), e.getMessage().contains("expected class")); - } + ValueObject result = (ValueObject) gson.fromJson(json, ValueObject.class); + Assert.assertEquals(tags, result.get("tags")); } @Test - public void longArray_roundTripThroughDeserializer_blockedByPreexistingSetterBug() { + public void longArray_roundTripsThroughDeserializer() { Gson gson = MetaObjectGsonInitializer.getBuilderWithAdapters(arrayLoader).create(); ValueObject vo = newArrayThing(); - vo.put("bigCounts", Arrays.asList(10L, 20L)); + List bigCounts = Arrays.asList(10L, 20L); + vo.put("bigCounts", bigCounts); String json = gson.toJson(vo); - try { - gson.fromJson(json, ValueObject.class); - Assert.fail("Expected the pre-existing MetaField.setObject scalar-dataType bug to " - + "throw; if this now succeeds, that separate defect has been fixed and this " - + "test should be replaced with a real round-trip assertion."); - } catch (NumberFormatException e) { - // DataConverter.toLong(list.toString()) -- see class Javadoc. The elements read - // ".0"-suffixed (Gson's context.deserialize(el, List.class) widens JSON numbers to - // Double by default, absent generic type info -- the pre-existing, separately - // out-of-scope numeric-widening wart the brief itself calls out) rather than the - // "[10, 20]" the ORIGINAL List above would stringify to; this asserts what the - // round trip actually produces, not what the write-side input looked like. - Assert.assertTrue(e.getMessage(), e.getMessage().contains("[10.0, 20.0]")); - } + // Gson's context.deserialize(el, List.class) widens JSON numbers to Double by default + // (absent generic type info -- the pre-existing, separately out-of-scope numeric-array + // read widening the brief itself calls out and this task does not redesign). setObject + // now routes that List through DataConverter.toLongArray, which maps each + // element back through DataConverter.toLong (10.0 -> 10L), so the widening is invisible + // here: the round trip still lands on List, not List. + ValueObject result = (ValueObject) gson.fromJson(json, ValueObject.class); + Assert.assertEquals(bigCounts, result.get("bigCounts")); } // ----------------------------------------------------------------------- diff --git a/server/java/metadata/src/test/java/com/metaobjects/util/DataConverterTests.java b/server/java/metadata/src/test/java/com/metaobjects/util/DataConverterTests.java index 9a1b1cc42..953c3bffe 100644 --- a/server/java/metadata/src/test/java/com/metaobjects/util/DataConverterTests.java +++ b/server/java/metadata/src/test/java/com/metaobjects/util/DataConverterTests.java @@ -330,7 +330,55 @@ public void testToStringArray() { assertEquals(1, numberResult.size()); assertEquals("42", numberResult.get(0)); } - + + /** + * #275 carry-forward: DATE_ARRAY had no DataConverter implementation at all (the + * unsupported() arm). Mirrors testToStringArray's shape, against toDate's epoch-millis + * String contract (not ISO -- TemporalWireFormat owns ISO parsing, out of scope here). + */ + @Test + public void testToDateArray() { + // Test null input + assertNull(DataConverter.toDateArray(null)); + + // Test existing List + ArrayList existingList = new ArrayList<>(); + existingList.add(new Date(5)); + existingList.add(new Date(10)); + List result = DataConverter.toDateArray(existingList); + assertEquals(existingList, result); + + // Test comma-separated epoch-millis string + List csvResult = DataConverter.toDateArray("5,10,15"); + assertEquals(3, csvResult.size()); + assertEquals(new Date(5), csvResult.get(0)); + assertEquals(new Date(10), csvResult.get(1)); + assertEquals(new Date(15), csvResult.get(2)); + + // Test single epoch-millis string + List singleResult = DataConverter.toDateArray("23412341234"); + assertEquals(1, singleResult.size()); + assertEquals(new Date(23412341234L), singleResult.get(0)); + + // Test empty string + List emptyResult = DataConverter.toDateArray(""); + assertTrue(emptyResult.isEmpty()); + + // Test non-List, non-String value (single element via toDate) + List longResult = DataConverter.toDateArray(5L); + assertEquals(1, longResult.size()); + assertEquals(new Date(5), longResult.get(0)); + + // Test a List containing a null element (element-wise toDate(null) -> null, not dropped) + List withNull = new ArrayList<>(); + withNull.add(new Date(5)); + withNull.add(null); + List nullElementResult = DataConverter.toDateArray(withNull); + assertEquals(2, nullElementResult.size()); + assertEquals(new Date(5), nullElementResult.get(0)); + assertNull(nullElementResult.get(1)); + } + /** * CUSTOM type is opaque to the generic converter; values must pass through unchanged. * This allows per-type codecs (e.g. TimeCodec for TimeField) to handle their own diff --git a/server/java/metadata/src/test/resources/com/metaobjects/io/object/gson/array-primitive-metadata.json b/server/java/metadata/src/test/resources/com/metaobjects/io/object/gson/array-primitive-metadata.json index 174105f02..fbed83803 100644 --- a/server/java/metadata/src/test/resources/com/metaobjects/io/object/gson/array-primitive-metadata.json +++ b/server/java/metadata/src/test/resources/com/metaobjects/io/object/gson/array-primitive-metadata.json @@ -14,6 +14,8 @@ { "field.double": { "name": "amount" } }, { "field.date": { "name": "dates", "isArray": true } }, { "field.date": { "name": "day" } }, + { "field.timestamp": { "name": "timestamps", "isArray": true } }, + { "field.timestamp": { "name": "localTimestamps", "isArray": true, "@localTime": true } }, { "field.decimal": { "name": "price" } } ]}} ] From 312f1bda586edc26d78bdae9d6326fd119c79ebf Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Sat, 8 Aug 2026 18:11:48 -0400 Subject: [PATCH 2/2] 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) Claude-Session: https://claude.ai/code/session_015TqsuDye2SfXGf43vuoD3n --- .../object/gson/MetaObjectDeserializer.java | 15 ++- .../gson/GsonArrayWriteRoundTripTest.java | 94 +++++++++++++++++++ 2 files changed, 106 insertions(+), 3 deletions(-) diff --git a/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectDeserializer.java b/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectDeserializer.java index 381fdb381..3574b45dd 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectDeserializer.java +++ b/server/java/metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectDeserializer.java @@ -130,8 +130,10 @@ protected void readFieldValue(MetaObject mo, MetaField mf, Object vo, // AbstractObjectRepresentation.setValue, which applies // DataConverter.toType(effectiveDataType, value) -- backed, since the #275 // carry-forward unit, by DataConverter.toDateArray. This branch genuinely - // round-trips a date array end to end today (see - // GsonArrayWriteRoundTripTest's Step 3b/A6 coverage). + // round-trips a date array end to end today, INCLUDING a null element (see + // readDateElement's isJsonNull() guard -- required because the write side, + // MetaObjectSerializer, deliberately emits JsonNull.INSTANCE at a null element + // position; see GsonArrayWriteRoundTripTest's Step 3b/A6 coverage). if (mf.isArrayType() && el.isJsonArray()) { List dates = new ArrayList<>(); for (JsonElement item : el.getAsJsonArray()) { @@ -201,8 +203,15 @@ protected void readFieldValue(MetaObject mo, MetaField mf, Object vo, } } - /** Single JSON array element of a DATE-array field: number -> epoch millis, string -> tolerant ISO parse. */ + /** Single JSON array element of a DATE-array field: null -> null (JsonNull.getAsString() + * throws UnsupportedOperationException, so this must be checked before isJsonPrimitive -- + * JsonNull is not a JsonPrimitive), number -> epoch millis, string -> tolerant ISO parse. + * Mirrors what MetaObjectSerializer.writeField's DATE-array branch emits at a null element + * position (JsonNull.INSTANCE), so a null element round-trips instead of throwing. */ private Date readDateElement(MetaField mf, JsonElement el) { + if (el.isJsonNull()) { + return null; + } if (el.isJsonPrimitive() && el.getAsJsonPrimitive().isNumber()) { return new Date(el.getAsLong()); } diff --git a/server/java/metadata/src/test/java/com/metaobjects/io/object/gson/GsonArrayWriteRoundTripTest.java b/server/java/metadata/src/test/java/com/metaobjects/io/object/gson/GsonArrayWriteRoundTripTest.java index 3462fa56d..093a03811 100644 --- a/server/java/metadata/src/test/java/com/metaobjects/io/object/gson/GsonArrayWriteRoundTripTest.java +++ b/server/java/metadata/src/test/java/com/metaobjects/io/object/gson/GsonArrayWriteRoundTripTest.java @@ -346,6 +346,28 @@ public void dateArray_roundTripsThroughFullGsonPipeline() { Assert.assertEquals(dates, result.get("dates")); } + @Test + public void dateArray_withNullElement_roundTripsThroughFullGsonPipeline() { + // Review fix round 1, Important #1: the write side (MetaObjectSerializer, pinned by + // dateArray_withNullElement_writesJsonNullAtThatPosition above) deliberately emits + // JsonNull.INSTANCE at a null element position. The read side must accept what the write + // side produces -- readDateElement checks isJsonNull() first (JsonNull is not a + // JsonPrimitive, so el.getAsString() would otherwise throw + // UnsupportedOperationException, naming no field). + Gson gson = MetaObjectGsonInitializer.getBuilderWithAdapters(arrayLoader).create(); + + ValueObject vo = newArrayThing(); + List dates = Arrays.asList(utc(2026, 6, 3, 0, 0, 0, 0), null); + vo.put("dates", dates); + + String json = gson.toJson(vo); + Assert.assertTrue("expected a null element in the wire array, was: " + json, + json.contains("\"2026-06-03\"") && json.contains("null")); + + ValueObject result = (ValueObject) gson.fromJson(json, ValueObject.class); + Assert.assertEquals(dates, result.get("dates")); + } + @Test public void timestampArray_roundTripsThroughFullGsonPipeline() { Gson gson = MetaObjectGsonInitializer.getBuilderWithAdapters(arrayLoader).create(); @@ -454,4 +476,76 @@ public void scalarFields_serializeUnaffectedByArraySupport() { Assert.assertEquals("2026-06-03", obj.get("day").getAsString()); Assert.assertEquals(new java.math.BigDecimal("19.99"), obj.get("price").getAsBigDecimal()); } + + // ----------------------------------------------------------------------- + // Step 7 — review fix round 1, Important #2: a SCALAR JSON value (not wrapped in an array) + // read onto an array-typed field. Before E2, MetaField.setObject converted via the field's + // scalar getDataType() -- e.g. toType(STRING, "a,b") -> "a,b" identity -- and + // setObjectAttribute's effective-class check (List required for an isArray field) rejected + // it with InvalidValueException: a loud failure. After E2, setObject converts via + // getEffectiveDataType() (the *_ARRAY variant), so the value is coerced into a single- (or, + // for STRING specifically, comma-split multi-) element List and stored WITHOUT error. This is + // not a new behavior invented by this task: DataObjectBase._setObjectAttribute (the + // vo.put()/ValueObject.Map path exercised throughout this file) already converted against the + // effective type before this task, and is exactly what made the RED evidence in this task's + // report (`vo.put("dates", list)` throwing before DataConverter.toDateArray existed) possible + // in the first place. E2 converges MetaObjectDeserializer's setObject-based read path with + // that pre-existing behavior rather than introducing a new rule. Pinned here per line, not + // asserted in prose only, since a live JSON read path (a scalar value where an array is + // expected -- e.g. a legacy or hand-written payload) can reach every one of these. + // ----------------------------------------------------------------------- + + private ValueObject deserializeArrayThing(String json) { + Gson gson = MetaObjectGsonInitializer.getBuilderWithAdapters(arrayLoader).create(); + return (ValueObject) gson.fromJson(json, ValueObject.class); + } + + @Test + public void stringArray_scalarJsonValue_commaSplitsIntoMultiElementList() { + // The headline case: a bare JSON string containing a comma is not merely wrapped, it is + // SPLIT -- DataConverter.toStringArray's String branch treats a comma-containing string + // as delimited. Silent, and worth a name of its own. + ValueObject result = deserializeArrayThing( + "{\"@type\":\"test::arrays::ArrayThing\",\"tags\":\"a,b\"}"); + Assert.assertEquals(Arrays.asList("a", "b"), result.get("tags")); + } + + @Test + public void booleanArray_scalarJsonValue_wrapsAsSingleElementList() { + ValueObject result = deserializeArrayThing( + "{\"@type\":\"test::arrays::ArrayThing\",\"flags\":true}"); + Assert.assertEquals(Arrays.asList(true), result.get("flags")); + } + + @Test + public void intArray_scalarJsonValue_wrapsAsSingleElementList() { + ValueObject result = deserializeArrayThing( + "{\"@type\":\"test::arrays::ArrayThing\",\"counts\":5}"); + Assert.assertEquals(Arrays.asList(5), result.get("counts")); + } + + @Test + public void longArray_scalarJsonValue_wrapsAsSingleElementList() { + ValueObject result = deserializeArrayThing( + "{\"@type\":\"test::arrays::ArrayThing\",\"bigCounts\":10}"); + Assert.assertEquals(Arrays.asList(10L), result.get("bigCounts")); + } + + @Test + public void doubleArray_scalarJsonValue_wrapsAsSingleElementList() { + ValueObject result = deserializeArrayThing( + "{\"@type\":\"test::arrays::ArrayThing\",\"amounts\":3.5}"); + Assert.assertEquals(Arrays.asList(3.5), result.get("amounts")); + } + + @Test + public void dateArray_scalarJsonNumber_wrapsAsSingleElementList() { + // DATE's "scalar value on an array field" branch is reached independently of the + // isArrayType() check that guards BOOLEAN/INT/LONG/DOUBLE/STRING above -- the DATE case's + // number/string arms run whenever the element isn't a JSON array, array-typed field or + // not (MetaObjectDeserializer.java's DATE case, the `else if isNumber` arm). + ValueObject result = deserializeArrayThing( + "{\"@type\":\"test::arrays::ArrayThing\",\"dates\":1749000000000}"); + Assert.assertEquals(Arrays.asList(new Date(1749000000000L)), result.get("dates")); + } }