Skip to content

Commit 41039de

Browse files
dmealingclaude
andcommitted
fix(codegen): a field.map's value objects were written unvalidated — Java TPH, and every C# path (#362)
`field.map @objectRef` reaches the generated write surface, and its VALUES are value objects carrying their own constraints. Those constraints were not enforced. The failure mode is the bad one: a POST or PATCH carrying `{"sites": {"hq": {…missing @required street…}}}` returned 201/200 and the row was written. No error, nothing in a log, and nothing an adopter could find by reading their own source — the defect lives in generated code. **Java — TPH only.** Both per-subtype write paths validate field-by-field with `validator.validateValue`, which applies a property's own constraints and stops; it does not cascade `@Valid` into a nested bean. The vanilla handler has an explicit cascade for exactly this reason, and it is not invoked on the TPH path. Create now runs a null-guarded loop over the map's values (the body binds the union `<Base>Dto`, whose accessors are plain, so a null check is the whole guard); PATCH reuses the shared presence-guarded loop, since `<Sub>Patch` carries the `hasX()`/`x()` tristate. **C# — all three: vanilla create, vanilla PATCH, TPH.** Two independent causes, and fixing either alone would have left it broken: 1. `RoutesGenerator.ValueObjectFields` admits `field.object` only, so a map never reached the validating arms at all. A sibling `MapValueObjectFields` now feeds them — deliberately NOT folded into `VoField`, which drives owned-nav machinery (EF `OwnsMany`/`.ToJson`, array null-clears) that a plain jsonb-converted property must not be dragged through. PATCH gets a typed arm ahead of the generic property path, so a present key is validated before assignment rather than after persistence. 2. `ValueObjectValidator` treated a `Dictionary` as a plain `IEnumerable`. Enumerating one yields `KeyValuePair` structs, whose assembly is CoreLib, so the graph reported "no value objects here" and validated **vacuously**. An `IDictionary` arm now unwraps to `.Values`, placed before the `IEnumerable` arm because every dictionary satisfies both. This also fixes a map nested INSIDE a value object, which the property walk reaches by the same route. TypeScript and Python were never affected; Kotlin writes no map column. Scalar-valued maps (`@valueType`) carry no bean and are untouched — asserted as the control in both new tests, so an implementation that validated *every* map would fail rather than pass. **Proven, not assumed.** The new Java test was run against the un-fixed generator and fails there (`expected the TPH create to iterate the map's VALUES`). On the C# side the previous session had written a characterization test that PINNED the behaviour, asserting 201 on an invalid nested value; it is INVERTED rather than deleted — silent acceptance is invisible by nature, so the case that proved it quiet is the one that must now prove it loud, live against a real generated server on Testcontainers Postgres. Docs follow the code: `docs/features/field-types.md` and the audit skill's `field.map` rung stop describing an open hole, and both now tell a reader on 1.0.0 that it IS open for them — the version is the thing that decides, and an adopter who added their own boundary check needs to know it became redundant rather than wrong. Agent-context corpus regenerated in the same commit (five stacks), and the `@Valid` grounding exemption added earlier today is removed again: the prose that named it is gone, so the exemption would have been a comment describing a rung that no longer says that. Closes #362. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTcEKXTQMYt84fAjuw5A2M
1 parent e98d79a commit 41039de

14 files changed

Lines changed: 269 additions & 45 deletions

File tree

‎agent-context/skills/metaobjects-audit/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ Per finding: `file:line` → what → generated-equivalent exists? → recommend
366366

367367
**Do NOT flag:** a bag whose readers genuinely pass it through (logged, echoed, stored, forwarded to a third party) without reading a key; a third-party or LLM raw response whose shape is not the adopter's to declare; an **array of scalars**, which is `isArray: true` on the base subtype and never an object; a bag the metadata's own comment justifies as open with a reason.
368368

369-
**Remedy — a ladder, chosen by the WRITER's declared type, and every rung gives the consumer a TYPED HANDLE rather than a string to get and set.** `list[X]` ⇒ the base subtype + `isArray: true`. `dict[str, X]` with a known `X` ⇒ **`field.map` + `@objectRef`** (a value object) or `@valueType` (a scalar) — registered vocabulary that emits `jsonb("col").$type<Record<string, X>>()` and `z.record(z.string(), X)`. **Check the CONSUMER before recommending this rung: `field.map` codegen now completes on all five ports** (Java types it `java.util.Map<String, V>`; C# emits the `Dictionary<string, V>` property and its EF jsonb storage mapping), **but no persistence- or api-contract-conformance fixture exercises `field.map` on ANY port, and the runtime persistence tier is uneven** — only Python's `ObjectManager` encodes a map, while `runtime-ts`, Java's OMDB and the Kotlin Exposed lane carry none. So where GENERATED CODE is the consumer, check the port before recommending this rung — a `field.map @objectRef` writes its nested value-object values **unvalidated** on Java TPH (discriminator-rooted) write paths (per-field `validateValue` does not cascade `@Valid`) and on EVERY C# write path — vanilla create, vanilla PATCH, and TPH alike (the map never reaches the recursively-validating value-object arms; the generic arms check the dictionary property itself, never its values). TypeScript (`z.record` over the VO's insert schema) and Python (`dict[str, VO]` Pydantic) validate map values, Kotlin writes no map column, and scalar-valued maps (`@valueType`) are unaffected everywhere. Recommend the rung for a Java TPH entity or for C# only with that said and the adopter validating map values at their own boundary before write; [issue #362](https://github.com/metaobjectsdev/metaobjects/issues/362) tracks the gap. Prefer a value object where a PORT RUNTIME must read the column back; a genuinely dynamic key set stays a bag. A serialized DTO / dataclass / `@Serializable` class ⇒ declare it as an `object.value` and point the field at it with `field.object` + `@objectRef` + `@storage: jsonb`. Only `dict[str, Any]` whose readers pin no key stays an open bag. **What the adopter GETS, per port** — this is the point of the conversion and worth stating in the finding: TypeScript emits the Drizzle column as `.$type<VO>()` plus the VO's own Zod schema and inferred type; Python annotates the field with the referenced VO's Pydantic model (and its `<Ref>Create` on the wire, so a nested violation still validates); Kotlin emits typed Exposed jsonb codecs backed by a shared Jackson `MetaJsonbMapper`. In none of them does the consumer touch a JSON string — that is what the open bag costs them today, since `unknown` forces every reader to cast. The column stays jsonb, so this is a codegen/contract change and not necessarily a migration; what changes is that `unknown` becomes a generated type, the casts delete, and the keys become metadata-owned — which also promotes their literals into drift signature 11's scope, where the `->>'key'` sites were previously exempt because the metadata owned nothing. **Verify the column type is genuinely unchanged before promising no migration**, and parity-gate each converted read against the raw-SQL one before deleting it. Convert one column at a time: the shape is a judgment per bag, and a wrong guess declares a contract the writers do not honor.
369+
**Remedy — a ladder, chosen by the WRITER's declared type, and every rung gives the consumer a TYPED HANDLE rather than a string to get and set.** `list[X]` ⇒ the base subtype + `isArray: true`. `dict[str, X]` with a known `X` ⇒ **`field.map` + `@objectRef`** (a value object) or `@valueType` (a scalar) — registered vocabulary that emits `jsonb("col").$type<Record<string, X>>()` and `z.record(z.string(), X)`. **Check the CONSUMER before recommending this rung: `field.map` codegen now completes on all five ports** (Java types it `java.util.Map<String, V>`; C# emits the `Dictionary<string, V>` property and its EF jsonb storage mapping), **but no persistence- or api-contract-conformance fixture exercises `field.map` on ANY port, and the runtime persistence tier is uneven** — only Python's `ObjectManager` encodes a map, while `runtime-ts`, Java's OMDB and the Kotlin Exposed lane carry none. So where GENERATED CODE is the consumer, check the port before recommending this rung — nested map values are validated on every write path on every port as of 1.0.1 ([#362](https://github.com/metaobjectsdev/metaobjects/issues/362) — before that, Java skipped the cascade on TPH write paths and C# skipped it everywhere, silently accepting invalid values rather than erroring). **Check the adopter's pinned version before recommending this rung to a Java-TPH or C# consumer: on 1.0.0 the hole is open and they need their own boundary check.** Prefer a value object where a PORT RUNTIME must read the column back; a genuinely dynamic key set stays a bag. A serialized DTO / dataclass / `@Serializable` class ⇒ declare it as an `object.value` and point the field at it with `field.object` + `@objectRef` + `@storage: jsonb`. Only `dict[str, Any]` whose readers pin no key stays an open bag. **What the adopter GETS, per port** — this is the point of the conversion and worth stating in the finding: TypeScript emits the Drizzle column as `.$type<VO>()` plus the VO's own Zod schema and inferred type; Python annotates the field with the referenced VO's Pydantic model (and its `<Ref>Create` on the wire, so a nested violation still validates); Kotlin emits typed Exposed jsonb codecs backed by a shared Jackson `MetaJsonbMapper`. In none of them does the consumer touch a JSON string — that is what the open bag costs them today, since `unknown` forces every reader to cast. The column stays jsonb, so this is a codegen/contract change and not necessarily a migration; what changes is that `unknown` becomes a generated type, the casts delete, and the keys become metadata-owned — which also promotes their literals into drift signature 11's scope, where the `->>'key'` sites were previously exempt because the metadata owned nothing. **Verify the column type is genuinely unchanged before promising no migration**, and parity-gate each converted read against the raw-SQL one before deleting it. Convert one column at a time: the shape is a judgment per bag, and a wrong guess declares a contract the writers do not honor.
370370

371371
---
372372

‎docs/features/field-types.md‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -47,22 +47,23 @@ Three rows need a footnote:
4747
explicit converter/comparer pair, so the property lands on the `jsonb` column the TS-owned
4848
migration creates instead of on whatever an unmapped dictionary would resolve to.
4949

50-
> ⚠️ **A `field.map @objectRef` can write its nested value-object values UNVALIDATED — and
51-
> the scope is per port.** TypeScript (`z.record` over the VO's insert schema) and Python
52-
> (`dict[str, VO]` Pydantic) validate map values, and Kotlin writes no map column at all. The
53-
> hole is in the other two: **Java** validates nested map values on its vanilla
54-
> create/PATCH handlers but NOT on TPH (discriminator-rooted) write paths, which validate
55-
> field-by-field with `validateValue` — that does not cascade `@Valid` into a nested bean,
56-
> and the explicit cascade the vanilla handler runs is not invoked there. **C# validates
57-
> them on NO write path — not TPH, not vanilla create, not vanilla PATCH**: the map property
58-
> never reaches the recursively-validating value-object arms (they admit `field.object` only),
59-
> and the generic arms check the dictionary property itself, never its values. A posted map
60-
> value that violates the referenced `object.value`'s own constraints is accepted and
61-
> written. Scalar-valued maps (`@valueType`) are unaffected: there is no nested bean to
62-
> validate. This is generated code, so **reading your own source will not reveal it** — the
63-
> failure is silent acceptance, not an error. Validate map values at your own boundary
64-
> before write. [Issue #362](https://github.com/metaobjectsdev/metaobjects/issues/362) tracks
65-
> the gap.
50+
**Nested map values are validated on every write path, on every port.** A posted map value
51+
that violates the referenced `object.value`'s own constraints is rejected with the cross-port
52+
`{"error":"validation"}` envelope before the row is written. Scalar-valued maps
53+
(`@valueType`) carry no nested bean, so nothing extra runs for them.
54+
55+
> This was a real hole until 1.0.1, and it is worth knowing what it was, because the failure
56+
> mode was **silent acceptance rather than an error** — reading your own source would never
57+
> have revealed it. **Java** validated nested map values on its vanilla create/PATCH handlers
58+
> but not on TPH (discriminator-rooted) write paths, which validate field-by-field with
59+
> `validateValue` — that does not cascade `@Valid` into a nested bean. **C#** validated them
60+
> on no write path at all: the map never reached the recursively-validating value-object arms
61+
> (they admit `field.object` only), and `ValueObjectValidator` treated a `Dictionary` as a
62+
> plain `IEnumerable`, so it validated `KeyValuePair` structs instead of the values.
63+
> TypeScript and Python were never affected, and Kotlin writes no map column.
64+
> ([#362](https://github.com/metaobjectsdev/metaobjects/issues/362); fixed in 1.0.1.) If you
65+
> added a boundary check of your own while this was open, it is now redundant rather than
66+
> load-bearing.
6667
6768
**The RUNTIME tier is not there yet, and no conformance corpus covers it.** No
6869
persistence- or api-contract-conformance fixture exercises `field.map` on any port; it is

‎fixtures/agent-context-conformance/java-kotlin-react-tanstack/expected/.claude/skills/metaobjects-audit/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ Per finding: `file:line` → what → generated-equivalent exists? → recommend
366366

367367
**Do NOT flag:** a bag whose readers genuinely pass it through (logged, echoed, stored, forwarded to a third party) without reading a key; a third-party or LLM raw response whose shape is not the adopter's to declare; an **array of scalars**, which is `isArray: true` on the base subtype and never an object; a bag the metadata's own comment justifies as open with a reason.
368368

369-
**Remedy — a ladder, chosen by the WRITER's declared type, and every rung gives the consumer a TYPED HANDLE rather than a string to get and set.** `list[X]` ⇒ the base subtype + `isArray: true`. `dict[str, X]` with a known `X` ⇒ **`field.map` + `@objectRef`** (a value object) or `@valueType` (a scalar) — registered vocabulary that emits `jsonb("col").$type<Record<string, X>>()` and `z.record(z.string(), X)`. **Check the CONSUMER before recommending this rung: `field.map` codegen now completes on all five ports** (Java types it `java.util.Map<String, V>`; C# emits the `Dictionary<string, V>` property and its EF jsonb storage mapping), **but no persistence- or api-contract-conformance fixture exercises `field.map` on ANY port, and the runtime persistence tier is uneven** — only Python's `ObjectManager` encodes a map, while `runtime-ts`, Java's OMDB and the Kotlin Exposed lane carry none. So where GENERATED CODE is the consumer, check the port before recommending this rung — a `field.map @objectRef` writes its nested value-object values **unvalidated** on Java TPH (discriminator-rooted) write paths (per-field `validateValue` does not cascade `@Valid`) and on EVERY C# write path — vanilla create, vanilla PATCH, and TPH alike (the map never reaches the recursively-validating value-object arms; the generic arms check the dictionary property itself, never its values). TypeScript (`z.record` over the VO's insert schema) and Python (`dict[str, VO]` Pydantic) validate map values, Kotlin writes no map column, and scalar-valued maps (`@valueType`) are unaffected everywhere. Recommend the rung for a Java TPH entity or for C# only with that said and the adopter validating map values at their own boundary before write; [issue #362](https://github.com/metaobjectsdev/metaobjects/issues/362) tracks the gap. Prefer a value object where a PORT RUNTIME must read the column back; a genuinely dynamic key set stays a bag. A serialized DTO / dataclass / `@Serializable` class ⇒ declare it as an `object.value` and point the field at it with `field.object` + `@objectRef` + `@storage: jsonb`. Only `dict[str, Any]` whose readers pin no key stays an open bag. **What the adopter GETS, per port** — this is the point of the conversion and worth stating in the finding: TypeScript emits the Drizzle column as `.$type<VO>()` plus the VO's own Zod schema and inferred type; Python annotates the field with the referenced VO's Pydantic model (and its `<Ref>Create` on the wire, so a nested violation still validates); Kotlin emits typed Exposed jsonb codecs backed by a shared Jackson `MetaJsonbMapper`. In none of them does the consumer touch a JSON string — that is what the open bag costs them today, since `unknown` forces every reader to cast. The column stays jsonb, so this is a codegen/contract change and not necessarily a migration; what changes is that `unknown` becomes a generated type, the casts delete, and the keys become metadata-owned — which also promotes their literals into drift signature 11's scope, where the `->>'key'` sites were previously exempt because the metadata owned nothing. **Verify the column type is genuinely unchanged before promising no migration**, and parity-gate each converted read against the raw-SQL one before deleting it. Convert one column at a time: the shape is a judgment per bag, and a wrong guess declares a contract the writers do not honor.
369+
**Remedy — a ladder, chosen by the WRITER's declared type, and every rung gives the consumer a TYPED HANDLE rather than a string to get and set.** `list[X]` ⇒ the base subtype + `isArray: true`. `dict[str, X]` with a known `X` ⇒ **`field.map` + `@objectRef`** (a value object) or `@valueType` (a scalar) — registered vocabulary that emits `jsonb("col").$type<Record<string, X>>()` and `z.record(z.string(), X)`. **Check the CONSUMER before recommending this rung: `field.map` codegen now completes on all five ports** (Java types it `java.util.Map<String, V>`; C# emits the `Dictionary<string, V>` property and its EF jsonb storage mapping), **but no persistence- or api-contract-conformance fixture exercises `field.map` on ANY port, and the runtime persistence tier is uneven** — only Python's `ObjectManager` encodes a map, while `runtime-ts`, Java's OMDB and the Kotlin Exposed lane carry none. So where GENERATED CODE is the consumer, check the port before recommending this rung — nested map values are validated on every write path on every port as of 1.0.1 ([#362](https://github.com/metaobjectsdev/metaobjects/issues/362) — before that, Java skipped the cascade on TPH write paths and C# skipped it everywhere, silently accepting invalid values rather than erroring). **Check the adopter's pinned version before recommending this rung to a Java-TPH or C# consumer: on 1.0.0 the hole is open and they need their own boundary check.** Prefer a value object where a PORT RUNTIME must read the column back; a genuinely dynamic key set stays a bag. A serialized DTO / dataclass / `@Serializable` class ⇒ declare it as an `object.value` and point the field at it with `field.object` + `@objectRef` + `@storage: jsonb`. Only `dict[str, Any]` whose readers pin no key stays an open bag. **What the adopter GETS, per port** — this is the point of the conversion and worth stating in the finding: TypeScript emits the Drizzle column as `.$type<VO>()` plus the VO's own Zod schema and inferred type; Python annotates the field with the referenced VO's Pydantic model (and its `<Ref>Create` on the wire, so a nested violation still validates); Kotlin emits typed Exposed jsonb codecs backed by a shared Jackson `MetaJsonbMapper`. In none of them does the consumer touch a JSON string — that is what the open bag costs them today, since `unknown` forces every reader to cast. The column stays jsonb, so this is a codegen/contract change and not necessarily a migration; what changes is that `unknown` becomes a generated type, the casts delete, and the keys become metadata-owned — which also promotes their literals into drift signature 11's scope, where the `->>'key'` sites were previously exempt because the metadata owned nothing. **Verify the column type is genuinely unchanged before promising no migration**, and parity-gate each converted read against the raw-SQL one before deleting it. Convert one column at a time: the shape is a judgment per bag, and a wrong guess declares a contract the writers do not honor.
370370

371371
---
372372

0 commit comments

Comments
 (0)