Skip to content

Commit 6a58732

Browse files
dmealingclaude
andcommitted
fix(#213): exclude derived fields from the entity write table + write codecs
#213 hole 1 (the silent-corruption half): an FR-024 §7 entity read-view's derived (origin-bearing) field leaked onto the entity's WRITE table. `buildExpectedSchema` built a column for every field with no origin check, so a join-derived passthrough became a spurious column on the write table — and collided with a hand-written `SELECT o.*, extra` view's alias (PG duplicate-output-column). The same unfiltered `fields()` iteration also declared the column in the generated Drizzle table def and accepted it in the Insert/Update Zod schemas. A derived field is read-only and lives only on the read (view) side (ADR-0028), so it must be excluded from every write-side artifact, in lockstep, or generated runtime addresses a column migrate no longer creates. - metadata: new `MetaField.isDerived()` (own origin child — origin never inherits, ADR-0029), the shared SSOT for "this field is derived". - migrate-ts `expected-schema.ts`: skip derived fields in the write-table column loop AND the TPH subtype fold. - codegen-ts `drizzle-schema.ts`: skip derived fields in the table column loop + the TPH fold. - codegen-ts `zod-validators.ts`: derived ⇒ read-only, excluded from the Insert / Update / preserving schemas (same treatment as @readonly, all four field loops). This is #213's schema/write half (TS-owned, ADR-0015). The READ half — emitting the replica view (hole 2, `isWriteThrough` → buildProjectionViews) and routing reads to it + the entity's read TYPE — follows; the cross-port codegen exclusions (other ports' ORM/DTO generators) are a separate coordinated issue. Verified: metadata 2179 pass (incl. isDerived), migrate-ts 341 (new write-table exclusion test), codegen-ts 959 (new Drizzle + Zod exclusion test); typecheck exit 0. No existing golden changed (no fixture had a write-through entity with derived fields). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGQ7oSuNcjhsMHWwZzhBwr
1 parent 14d93ad commit 6a58732

7 files changed

Lines changed: 204 additions & 6 deletions

File tree

server/typescript/packages/codegen-ts/src/templates/drizzle-schema.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@ export function renderDrizzleSchema(obj: MetaObject, ctx: RenderContext): Code {
6666
// Collect CHECK constraints for enum columns; emitted as table-level check() callbacks.
6767
const checkConstraints: Array<{ name: string; expr: string }> = [];
6868
for (const child of obj.fields()) {
69+
// #213 — a derived (origin-bearing) field is read-only, materialized on the
70+
// read (view) side, NOT a column on the entity's write table (FR-024 §7).
71+
// Emitting it here would declare a Drizzle column for a table column migrate
72+
// no longer creates.
73+
if (child.isDerived()) continue;
6974
const isPk = pkFieldNames.has(child.name);
7075
const isUnique = uniqueFieldNames.has(child.name) && !isPk;
7176
const fkInfo = fkMap.get(child.name);
@@ -90,6 +95,8 @@ export function renderDrizzleSchema(obj: MetaObject, ctx: RenderContext): Code {
9095
// stamp onto other-subtype inserts), regardless of the field's @required.
9196
// Subtype entities emit no table of their own (the value-object path).
9297
for (const child of collectTphSubtypeFields(obj, ctx.loadedRoot)) {
98+
// #213 — a TPH subtype's derived field is read-only too; never a table column.
99+
if (child.isDerived()) continue;
93100
const spec = mapColumnType(child, ctx.dialect, ctx.columnNamingStrategy, ctx.timestampMode);
94101
const fieldDocs = renderDocsFor(child);
95102
const columnLine = renderColumn(

server/typescript/packages/codegen-ts/src/templates/zod-validators.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,9 @@ export function renderInsertSchemaOnly(obj: MetaObject, ctx?: RenderContext): Co
175175
// FR-013: @readOnly fields are populated by DB / replication / external
176176
// owner; the application has no path to write them. Exclude from the
177177
// create-shape schema entirely.
178-
if (child.attr(FIELD_ATTR_READ_ONLY) === true) continue;
178+
// #213 — a derived (origin-bearing) field is read-only (FR-024 §7 / ADR-0028):
179+
// excluded from the Insert/Update/preserving schemas, exactly like @readOnly.
180+
if (child.attr(FIELD_ATTR_READ_ONLY) === true || child.isDerived()) continue;
179181

180182
// FR-017 Tier 1: TPH subtype pins its discriminator field to z.literal(...).
181183
if (tphPin !== undefined && child.name === tphPin.fieldName) {
@@ -239,7 +241,9 @@ export function insertSchemaFields(obj: MetaObject): SchemaFieldShape[] {
239241
const out: SchemaFieldShape[] = [];
240242
for (const child of obj.fields()) {
241243
if (autoGenPkFields.has(child.name)) continue;
242-
if (child.attr(FIELD_ATTR_READ_ONLY) === true) continue;
244+
// #213 — a derived (origin-bearing) field is read-only (FR-024 §7 / ADR-0028):
245+
// excluded from the Insert/Update/preserving schemas, exactly like @readOnly.
246+
if (child.attr(FIELD_ATTR_READ_ONLY) === true || child.isDerived()) continue;
243247
if (tphPin !== undefined && child.name === tphPin.fieldName) {
244248
out.push({ name: child.name, optional: false, pinnedLiteral: tphPin.value });
245249
continue;
@@ -269,7 +273,9 @@ export function updateSchemaFields(obj: MetaObject): SchemaFieldShape[] {
269273
const out: SchemaFieldShape[] = [];
270274
for (const child of obj.fields()) {
271275
if (autoGenPkFields.has(child.name)) continue;
272-
if (child.attr(FIELD_ATTR_READ_ONLY) === true) continue;
276+
// #213 — a derived (origin-bearing) field is read-only (FR-024 §7 / ADR-0028):
277+
// excluded from the Insert/Update/preserving schemas, exactly like @readOnly.
278+
if (child.attr(FIELD_ATTR_READ_ONLY) === true || child.isDerived()) continue;
273279
// TPH subtype discriminator: omitted from the update schema entirely.
274280
if (tphPin !== undefined && child.name === tphPin.fieldName) continue;
275281
const autoSet = child.attr(FIELD_ATTR_AUTO_SET);
@@ -306,7 +312,9 @@ export function renderZodValidators(obj: MetaObject, ctx?: RenderContext): Code
306312
// The DB / trigger / replication owns the write path; the app must not
307313
// pass these values in POST/PATCH bodies (routesFile enforces the same
308314
// contract at the boundary with a 400 response).
309-
if (child.attr(FIELD_ATTR_READ_ONLY) === true) continue;
315+
// #213 — a derived (origin-bearing) field is read-only (FR-024 §7 / ADR-0028):
316+
// excluded from the Insert/Update/preserving schemas, exactly like @readOnly.
317+
if (child.attr(FIELD_ATTR_READ_ONLY) === true || child.isDerived()) continue;
310318

311319
// FR-017 Tier 1: TPH subtype pins its discriminator field to z.literal(...).
312320
// The discriminator is implicit on subtype rows (controlled by URL / insert
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// #213 — the write-side codegen must exclude a derived (origin-bearing) field, in
2+
// lockstep with migrate-ts dropping it from the write-table DDL: otherwise the
3+
// generated Drizzle table declares a column, and the Insert/Update Zod schemas
4+
// accept a field, that no longer exists on the physical write table.
5+
6+
import { describe, test, expect } from "bun:test";
7+
import { MetaDataLoader, InMemoryStringSource } from "@metaobjectsdev/metadata";
8+
import { renderDrizzleSchema } from "../../src/templates/drizzle-schema.js";
9+
import { renderZodValidators } from "../../src/templates/zod-validators.js";
10+
import { makeRenderContext } from "../../src/render-context.js";
11+
import { buildPkMap } from "../../src/pk-resolver.js";
12+
import { buildRelationMap } from "../../src/relation-resolver.js";
13+
14+
async function loadWriteThroughOrder() {
15+
const json = JSON.stringify({
16+
"metadata.root": {
17+
package: "acme",
18+
children: [
19+
{ "object.entity": { name: "Customer", children: [
20+
{ "source.rdb": { "@table": "customers" } },
21+
{ "field.long": { name: "id" } },
22+
{ "field.string": { name: "name" } },
23+
{ "identity.primary": { name: "pk", "@fields": "id" } },
24+
] } },
25+
{ "object.entity": { name: "Order", children: [
26+
{ "source.rdb": { "@role": "primary", "@table": "orders" } },
27+
{ "source.rdb": { "@role": "replica", "@kind": "view", "@table": "v_order" } },
28+
{ "field.long": { name: "id" } },
29+
{ "field.long": { name: "customerId", "@required": true } },
30+
{ "field.string": { name: "customerName", children: [
31+
{ "origin.passthrough": { "@from": "Customer.name", "@via": "Order.customer" } } ] } },
32+
{ "relationship.association": { name: "customer", "@objectRef": "Customer", "@cardinality": "one" } },
33+
{ "identity.primary": { name: "pk", "@fields": "id" } },
34+
{ "identity.reference": { name: "ref_customer", "@fields": "customerId", "@references": "Customer" } },
35+
] } },
36+
],
37+
},
38+
});
39+
const { root, errors } = await new MetaDataLoader().load([new InMemoryStringSource(json)]);
40+
expect(errors).toEqual([]);
41+
const order = root.objects().find((o) => o.name === "Order")!;
42+
const ctx = makeRenderContext({
43+
dialect: "postgres", loadedRoot: root, outDir: "/x", dbImport: "~/db",
44+
pkMap: buildPkMap(root), relationMap: buildRelationMap(root),
45+
});
46+
return { order, ctx };
47+
}
48+
49+
describe("#213 — write-side codegen excludes derived fields", () => {
50+
test("Drizzle table def omits the derived customerName column", async () => {
51+
const { order, ctx } = await loadWriteThroughOrder();
52+
const out = renderDrizzleSchema(order, ctx).toString();
53+
expect(out).toContain("customerId"); // stored FK stays
54+
expect(out).not.toContain("customerName"); // derived join column excluded
55+
});
56+
57+
test("Insert + Update Zod schemas omit the derived customerName field", async () => {
58+
const { order, ctx } = await loadWriteThroughOrder();
59+
const out = renderZodValidators(order, ctx).toString();
60+
expect(out).toContain("OrderInsertSchema");
61+
expect(out).toContain("customerId");
62+
expect(out).not.toContain("customerName");
63+
});
64+
});

server/typescript/packages/metadata/src/core/field/meta-field.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
DATA_TYPE_OBJECT,
1717
} from "../../data-type.js";
1818
import { convertToDataType } from "../../data-converter.js";
19-
import { TYPE_VALIDATOR, TYPE_VIEW, SUBTYPE_BASE } from "../../shared/base-types.js";
19+
import { TYPE_VALIDATOR, TYPE_VIEW, TYPE_ORIGIN, SUBTYPE_BASE } from "../../shared/base-types.js";
2020
import {
2121
FIELD_SUBTYPE_STRING,
2222
FIELD_SUBTYPE_INT,
@@ -196,6 +196,18 @@ export class MetaField extends MetaData implements DataTypeAware {
196196
);
197197
}
198198

199+
/**
200+
* True when this field carries an `origin.*` child (passthrough / aggregate /
201+
* computed / first): its value is DERIVED from other data, not stored in the
202+
* object's own writable table. A derived field is read-only wherever it lives
203+
* (ADR-0028) — excluded from the write-table DDL, the ORM table def, and the
204+
* Insert/Update codecs; it exists only on the read (view) side. `origin.*` never
205+
* inherits (ADR-0029), so the check is own-only (category 4). */
206+
isDerived(): boolean {
207+
// ADR-0039 own (category 4): origin.* never inherits (ADR-0029) — own by policy.
208+
return this.ownChildren().some((c) => c.type === TYPE_ORIGIN);
209+
}
210+
199211
/** The typed supertype field if `extends:` resolved, else undefined. */
200212
resolveSuper(): MetaField | undefined {
201213
return this.superData as MetaField | undefined;
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// #213 — MetaField.isDerived(): true when a field carries an origin.* child (its
2+
// value is derived, not stored in the object's own writable table). Drives the
3+
// write-side exclusions (migrate table DDL, ORM table def, Insert/Update codecs).
4+
5+
import { describe, test, expect } from "bun:test";
6+
import { MetaDataLoader, InMemoryStringSource } from "@metaobjectsdev/metadata";
7+
8+
async function loadOrder() {
9+
const json = JSON.stringify({
10+
"metadata.root": {
11+
package: "acme",
12+
children: [
13+
{ "object.entity": { name: "Customer", children: [
14+
{ "source.rdb": { "@table": "customers" } },
15+
{ "field.long": { name: "id" } },
16+
{ "field.string": { name: "name" } },
17+
{ "identity.primary": { name: "pk", "@fields": "id" } },
18+
] } },
19+
{ "object.entity": { name: "Order", children: [
20+
{ "source.rdb": { "@role": "primary", "@table": "orders" } },
21+
{ "source.rdb": { "@role": "replica", "@kind": "view", "@table": "v_order" } },
22+
{ "field.long": { name: "id" } },
23+
{ "field.long": { name: "customerId", "@required": true } },
24+
// Derived field — a joined passthrough (has an origin child).
25+
{ "field.string": { name: "customerName", children: [
26+
{ "origin.passthrough": { "@from": "Customer.name", "@via": "Order.customer" } } ] } },
27+
{ "relationship.association": { name: "customer", "@objectRef": "Customer", "@cardinality": "one" } },
28+
{ "identity.primary": { name: "pk", "@fields": "id" } },
29+
{ "identity.reference": { name: "ref_customer", "@fields": "customerId", "@references": "Customer" } },
30+
] } },
31+
],
32+
},
33+
});
34+
const { root, errors } = await new MetaDataLoader().load([new InMemoryStringSource(json)]);
35+
expect(errors).toEqual([]);
36+
return root.objects().find((o) => o.name === "Order")!;
37+
}
38+
39+
describe("MetaField.isDerived", () => {
40+
test("true for an origin-bearing field, false for stored fields", async () => {
41+
const order = await loadOrder();
42+
expect(order.findField("customerName")!.isDerived()).toBe(true);
43+
expect(order.findField("id")!.isDerived()).toBe(false);
44+
expect(order.findField("customerId")!.isDerived()).toBe(false);
45+
});
46+
});

server/typescript/packages/migrate-ts/src/expected-schema.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { ColumnNamingStrategy, MetaData, MetaObject, MetaRoot, MetaValidator } from "@metaobjectsdev/metadata";
1+
import type { ColumnNamingStrategy, MetaData, MetaField, MetaObject, MetaRoot, MetaValidator } from "@metaobjectsdev/metadata";
22
import {
33
VALIDATOR_SUBTYPE_NUMERIC, VALIDATOR_SUBTYPE_LENGTH, VALIDATOR_SUBTYPE_REGEX,
44
VALIDATOR_SUBTYPE_COMPARISON, VALIDATOR_SUBTYPE_REQUIRED_WHEN,
@@ -375,6 +375,12 @@ function buildTable(
375375

376376
const columns: ColumnDescriptor[] = [];
377377
for (const field of entity.fields()) {
378+
// #213 — a derived (origin-bearing) field is read-only and lives only on the
379+
// read (view) side (FR-024 §7 / ADR-0028); it is NOT a physical column on the
380+
// entity's write table. Excluding it here stops the leak that put a joined
381+
// passthrough onto the write table (and collided with a hand-written
382+
// `SELECT o.*, extra` view's alias).
383+
if (field.isDerived()) continue;
378384
const isPk = pkJsNames.includes(field.name);
379385
if (
380386
field.subType === FIELD_SUBTYPE_OBJECT &&
@@ -401,6 +407,8 @@ function buildTable(
401407
// fields (inherited base fields are already emitted on the base table).
402408
for (const field of sub.ownChildren()) {
403409
if (field.type !== TYPE_FIELD) continue;
410+
// #213 — a TPH subtype's derived field is read-only too; never a column.
411+
if ((field as MetaField).isDerived()) continue;
404412
const col = buildColumn(field, false, undefined, strategy);
405413
if (existing.has(col.name)) continue;
406414
col.nullable = true; // subtype-only columns are always nullable in TPH
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// #213 — FR-024 §7 entity read-view: a derived (origin-bearing) field must NOT
2+
// become a column on the entity's WRITE table. Before the fix, buildExpectedSchema
3+
// built a column for every field, so a joined-passthrough field leaked onto the
4+
// write table (and collided with a hand-written `SELECT o.*, extra` view's alias).
5+
6+
import { describe, test, expect } from "bun:test";
7+
import { MetaDataLoader, InMemoryStringSource } from "@metaobjectsdev/metadata";
8+
import { buildExpectedSchema } from "../src/expected-schema.js";
9+
10+
async function loadWriteThroughOrder() {
11+
const json = JSON.stringify({
12+
"metadata.root": {
13+
package: "acme",
14+
children: [
15+
{ "object.entity": { name: "Customer", children: [
16+
{ "source.rdb": { "@table": "customers" } },
17+
{ "field.long": { name: "id" } },
18+
{ "field.string": { name: "name" } },
19+
{ "identity.primary": { name: "pk", "@fields": "id" } },
20+
] } },
21+
// Entity read-view (FR-024 §7): writable table + non-primary read view +
22+
// a derived join column.
23+
{ "object.entity": { name: "Order", children: [
24+
{ "source.rdb": { "@role": "primary", "@table": "orders" } },
25+
{ "source.rdb": { "@role": "replica", "@kind": "view", "@table": "v_order_with_customer" } },
26+
{ "field.long": { name: "id" } },
27+
{ "field.long": { name: "customerId", "@required": true } },
28+
{ "field.string": { name: "customerName", children: [
29+
{ "origin.passthrough": { "@from": "Customer.name", "@via": "Order.customer" } } ] } },
30+
{ "relationship.association": { name: "customer", "@objectRef": "Customer", "@cardinality": "one" } },
31+
{ "identity.primary": { name: "pk", "@fields": "id" } },
32+
{ "identity.reference": { name: "ref_customer", "@fields": "customerId", "@references": "Customer" } },
33+
] } },
34+
],
35+
},
36+
});
37+
const { root, errors } = await new MetaDataLoader().load([new InMemoryStringSource(json)]);
38+
expect(errors).toEqual([]);
39+
return root;
40+
}
41+
42+
describe("#213 — derived fields excluded from the write table DDL", () => {
43+
test("the orders table carries id + customer_id but NOT the derived customer_name", async () => {
44+
const snap = buildExpectedSchema(await loadWriteThroughOrder(), { dialect: "postgres" });
45+
const orders = snap.tables.find((t) => t.name === "orders")!;
46+
expect(orders).toBeDefined();
47+
const cols = orders.columns.map((c) => c.name);
48+
expect(cols).toContain("id");
49+
expect(cols).toContain("customer_id");
50+
// The derived join column must NOT be a physical column on the write table.
51+
expect(cols).not.toContain("customer_name");
52+
});
53+
});

0 commit comments

Comments
 (0)