Skip to content

Commit c2bc4e2

Browse files
dmealingclaude
andcommitted
fix(migrate-ts,codegen-kotlin): review fixes for the ADR-0047 correlation
Four review findings on the previous commit, all fixed: 1. HIGH - tier 2 was an exact-string match, so a child-side relationship authored with an FQN @objectref missed tier 2 and the new parent-side tier OVERRODE the child's declaration (association restrict silently became composition cascade). Tier 2 now matches package-aware against the resolved @references target (refMatchesObject / ADR-0042), with a legacy exact-string fallback only when the target does not resolve. 2. MEDIUM - tier 2 had no @through guard: an M:N relationship wrongly correlated with a sibling DIRECT FK to the same target, arming cascade on it. The M:N exclusion now applies at both relationship tiers (matching the Kotlin mirror, which already excluded it). 3. LOW - the tier-3 satisfiability guard discarded the whole reverse relationship including an AUTHORED @onUpdate. It now drops only the inferred contributions; an explicit @onUpdate survives. Mirrored in Kotlin. 4. LOW - Kotlin findReverseRelationship omitted TS's "the single enforced reference must be the one being resolved" clause; aligned (a soft @Enforce:false reference never correlates). ADR-0047 amended: tier-2 matching + guard wording, and the blanket "existing models are byte-identical" claim replaced with the precise output-compatibility contour (the FQN child-side shape now correlates - intent previously silently dropped). CHANGELOG gains a prominent adopter-visible callout: the first migrate after upgrade adds ON DELETE actions to live FKs whose parent-side relationship was previously ignored - production delete semantics change deliberately. Gates re-run green: migrate-ts 426 (4 new pins, one per finding), view-lifecycle sqlite + real-Postgres byte-compat, TS conformance 534, migrate-ts typecheck, codegen-kotlin mvn. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KTGT5ksntpcJDZVJ5VyXHS
1 parent 123c691 commit c2bc4e2

6 files changed

Lines changed: 238 additions & 81 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,18 @@ this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
99

1010
### Fixed — parent-side `relationship.composition` reaches the child's FK (migrate-ts; ADR-0047)
1111

12+
> **ADOPTER-VISIBLE MIGRATION — production delete semantics change.** After
13+
> upgrading, the first `meta migrate` against a live database emits a one-time
14+
> migration **adding `ON DELETE CASCADE` (or the relationship's declared/default
15+
> action) to FKs whose parent-side relationship was previously silently
16+
> ignored** — deleting a parent row then deletes its children instead of
17+
> failing. This is the documented semantic the metadata always declared, but it
18+
> was never enforced, so review that migration deliberately; pin
19+
> `@onDelete: "no-action"` on any relationship whose DB behavior you want kept.
20+
> The same applies to a child-side relationship authored with an FQN
21+
> `@objectRef`, which the old exact-string correlation silently missed.
22+
> Give this entry top billing at cut time.
23+
1224
The authoring the docs and the `metaobjects-authoring` skill teach — declaring the
1325
relationship on the PARENT (`relationship.composition { @objectRef: "Post",
1426
@cardinality: "many" }` on `Author`, with the subtype implying the default action) —
@@ -18,14 +30,21 @@ relationships declared on the FK-owning child, so the documented "composition
1830
FK violation at runtime. The resolver now correlates the **reverse relationship on the
1931
target entity** as a third precedence tier (reference-level attr → child-side
2032
relationship → parent-side relationship; each relationship contributing its explicit
21-
action, else its subtype default). Guards, each failing closed: an M:N (`@through`)
22-
relationship never correlates with a direct FK; a reverse relationship contributes
23-
nothing when the child holds more than one enforced reference to the same target
24-
(it cannot say which FK carries the ownership edge); and an INFERRED set-null default
25-
(parent-side aggregation, no explicit `@onDelete`) on a NOT NULL FK contributes
26-
nothing rather than turning a previously-valid model into a hard
27-
`SetNullNotNullableError` (an explicit `set-null` still errors loudly). Models with no
28-
parent-side relationship are byte-identical. The Kotlin Exposed table generator's
33+
action, else its subtype default). Tier-2 correlation is now **package-aware**
34+
(`refMatchesObject` / ADR-0042) and excludes M:N `@through` relationships — an FQN
35+
child-side `@objectRef` previously missed the exact-string match and contributed
36+
nothing (post-fix it would otherwise have been OVERRIDDEN by the parent side), and a
37+
`@through` relationship wrongly armed cascade on a sibling direct FK to the same
38+
target. Guards on the new reverse tier, each failing closed: the M:N exclusion; a
39+
reverse relationship contributes nothing unless the child holds exactly one enforced
40+
reference to the target (it cannot say which FK carries the ownership edge); and an
41+
INFERRED set-null default (parent-side aggregation, no explicit `@onDelete`) on a NOT
42+
NULL FK drops its inferred contributions rather than turning a previously-valid model
43+
into a hard `SetNullNotNullableError` — while an explicitly authored `set-null` still
44+
errors loudly and an explicitly authored `@onUpdate` on that same relationship is
45+
still honored. Models whose FKs already resolved an action are byte-identical; the
46+
two intentional output changes both restore declared intent that was silently
47+
dropped. The Kotlin Exposed table generator's
2948
decorated `identity.reference` columns now resolve actions with the same precedence
3049
(they previously emitted raw relationship attrs only, and the canonical
3150
parent-relationship + child-reference shape lost the action entirely through the

‎docs/features/relationships.md‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -117,14 +117,19 @@ action, highest first:
117117
relationship node) or on an **M:N junction's** FK sides (no relationship
118118
ever correlates with a junction FK).
119119
2. A relationship declared on the FK-owning (child) entity targeting the
120-
referenced entity: its explicit action, else its subtype default.
120+
referenced entity (matched package-aware, so bare and FQN spellings pair
121+
correctly; an M:N `@through` relationship never correlates with a direct
122+
FK): its explicit action, else its subtype default.
121123
3. A relationship declared on the **referenced (parent) entity** pointing back
122124
at the FK-owning entity — the canonical parent-side authoring above
123125
(`Author` declares `posts`): its explicit action, else its subtype default.
124-
Guards: an M:N (`@through`) relationship never correlates with a direct FK,
125-
and when the child holds more than one FK to the same parent the parent-side
126-
relationship contributes to none of them (it cannot say which FK carries the
127-
ownership edge).
126+
Guards: the M:N exclusion again; when the child holds more than one FK to
127+
the same parent the parent-side relationship contributes to none of them
128+
(it cannot say which FK carries the ownership edge); and an inferred
129+
set-null default (aggregation, no explicit `@onDelete`) on a NOT NULL FK
130+
contributes nothing — SET NULL cannot fire there (an explicit `set-null`
131+
instead fails migration generation loudly, telling you to make the FK
132+
nullable).
128133
4. None of the above → the FK is emitted with no `ON DELETE` / `ON UPDATE`
129134
clause (SQL `NO ACTION`).
130135

‎server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableGenerator.kt‎

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1382,39 +1382,53 @@ open class KotlinExposedTableGenerator : MultiFileDirectGeneratorBase<MetaObject
13821382
rel.through == null && rel.objectRef != null &&
13831383
KotlinGenUtil.resolveObjectByShortOrFqn(loader, rel.objectRef) === target
13841384
}
1385-
var rel = childSide ?: findReverseRelationship(loader, entity, target)
1386-
1387-
// Tier-3 satisfiability guard (mirrors TS resolveReferentialActions): an
1388-
// INFERRED set-null default (parent-side aggregation, no explicit @onDelete)
1389-
// is unsatisfiable on a NOT NULL FK column — the correlation contributes
1390-
// nothing rather than emitting a ReferenceOption the database would reject.
1391-
// An EXPLICIT set-null still flows through (the author asked for it).
1392-
if (childSide == null && rel != null && rel.onDeleteRaw == null &&
1393-
MetaRelationship.ON_DELETE_DEFAULT_BY_SUBTYPE[rel.subType] == MetaRelationship.ACTION_SET_NULL
1394-
) {
1395-
val fkField = ref.fields.singleOrNull()?.let { fn ->
1396-
entity.metaFields.firstOrNull { it.name == fn }
1385+
var rel = childSide
1386+
// When the tier-3 satisfiability guard fires, the reverse relationship's
1387+
// AUTHORED @onUpdate still applies (only the inferred contributions drop).
1388+
var suppressedReverseOnUpdate: String? = null
1389+
if (rel == null) {
1390+
val reverse = findReverseRelationship(loader, entity, ref, target)
1391+
if (reverse != null) {
1392+
// Tier-3 satisfiability guard (mirrors TS resolveReferentialActions):
1393+
// an INFERRED set-null default (parent-side aggregation, no explicit
1394+
// @onDelete) is unsatisfiable on a NOT NULL FK column — the INFERRED
1395+
// contributions drop rather than emitting a ReferenceOption the
1396+
// database would reject, while anything explicitly authored survives
1397+
// (an explicit set-null flows through; an explicit @onUpdate is
1398+
// honored rather than silently discarded with the guard).
1399+
val fkField = ref.fields.singleOrNull()?.let { fn ->
1400+
entity.metaFields.firstOrNull { it.name == fn }
1401+
}
1402+
val unsatisfiableInferredSetNull = reverse.onDeleteRaw == null &&
1403+
MetaRelationship.ON_DELETE_DEFAULT_BY_SUBTYPE[reverse.subType] == MetaRelationship.ACTION_SET_NULL &&
1404+
fkField != null && KotlinGenUtil.isRequiredField(fkField)
1405+
if (unsatisfiableInferredSetNull) {
1406+
suppressedReverseOnUpdate = reverse.onUpdateRaw
1407+
} else {
1408+
rel = reverse
1409+
}
13971410
}
1398-
if (fkField != null && KotlinGenUtil.isRequiredField(fkField)) rel = null
13991411
}
14001412

14011413
val onDelete = refOnDelete
14021414
?: rel?.let { it.onDeleteRaw ?: MetaRelationship.ON_DELETE_DEFAULT_BY_SUBTYPE[it.subType] }
14031415
val onUpdate = refOnUpdate
1404-
?: rel?.let { it.onUpdateRaw ?: MetaRelationship.ON_UPDATE_DEFAULT }
1416+
?: (rel?.let { it.onUpdateRaw ?: MetaRelationship.ON_UPDATE_DEFAULT } ?: suppressedReverseOnUpdate)
14051417
return onDelete to onUpdate
14061418
}
14071419

14081420
/**
14091421
* ADR-0047 tier-3 correlation — the relationship declared on the TARGET (parent)
14101422
* entity pointing back at the FK-owning entity. Fails closed (null) when the
1411-
* relationship is M:N (`@through`), or when [entity] holds more than one enforced
1412-
* reference to [target] (the reverse relationship cannot say which FK carries the
1413-
* ownership edge). Mirrors the TS migrate engine's findReverseRelationship.
1423+
* relationship is M:N (`@through`), or when [entity] does not hold exactly one
1424+
* enforced reference to [target] that is [ref] itself (the reverse relationship
1425+
* cannot say which FK carries the ownership edge; a soft `@enforce: false` ref
1426+
* never correlates). Mirrors the TS migrate engine's findReverseRelationship.
14141427
*/
14151428
private fun findReverseRelationship(
14161429
loader: MetaDataLoader,
14171430
entity: MetaObject,
1431+
ref: ReferenceIdentity,
14181432
target: MetaObject,
14191433
): MetaRelationship? {
14201434
// ADR-0039: identity.reference children are inheritable — RESOLVE via getIdentities(true).
@@ -1424,7 +1438,7 @@ open class KotlinExposedTableGenerator : MultiFileDirectGeneratorBase<MetaObject
14241438
r.isEnforced && r.targetEntity != null &&
14251439
KotlinGenUtil.resolveObjectByShortOrFqn(loader, r.targetEntity) === target
14261440
}
1427-
if (enforcedRefsToTarget.size != 1) return null
1441+
if (enforcedRefsToTarget.singleOrNull() !== ref) return null
14281442
return target.relationships.firstOrNull { rel ->
14291443
rel.through == null && rel.objectRef != null &&
14301444
KotlinGenUtil.resolveObjectByShortOrFqn(loader, rel.objectRef) === entity

‎server/typescript/packages/migrate-ts/src/referential-actions.ts‎

Lines changed: 57 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,12 @@ export function isRequired(field: MetaData): boolean {
5151
* Precedence (highest first):
5252
* 1. @onDelete / @onUpdate declared DIRECTLY on the identity.reference — the
5353
* reference IS the FK, so the action may be declared right where the FK is.
54-
* 2. A correlated sibling relationship on the same entity (matched on
55-
* target-entity name): its explicit @onDelete, else its subtype default
56-
* (composition→cascade, aggregation→set-null, association→restrict);
57-
* onUpdate defaults to "cascade".
54+
* 2. A correlated sibling relationship on the same entity — matched
55+
* package-aware against the resolved @references target (refMatchesObject
56+
* / ADR-0042, so bare and FQN forms pair correctly); an M:N relationship
57+
* (@through) never correlates with a direct FK. Its explicit @onDelete,
58+
* else its subtype default (composition→cascade, aggregation→set-null,
59+
* association→restrict); onUpdate defaults to "cascade".
5860
* 3. A correlated REVERSE relationship on the TARGET entity — the documented
5961
* parent-side authoring shape ("Program owns weeks": composition declared
6062
* on the parent with @objectRef back at this FK-owning entity). Same
@@ -65,8 +67,9 @@ export function isRequired(field: MetaData): boolean {
6567
* relationship contributes nothing (it cannot say which FK carries the
6668
* ownership edge — arming all of them could cascade through an edge the
6769
* author never designated; fail closed); and an INFERRED set-null default
68-
* (parent-side aggregation, no explicit @onDelete) on a NOT NULL FK also
69-
* contributes nothing (unsatisfiable — see the in-body guard comment).
70+
* (parent-side aggregation, no explicit @onDelete) on a NOT NULL FK drops
71+
* the INFERRED contributions only — an authored @onUpdate on that same
72+
* relationship still applies (see the in-body guard comment).
7073
* 4. None → undefined (no ON DELETE / ON UPDATE clause).
7174
*
7275
* - Resolved "no-action" → undefined: introspection in introspect/{postgres,sqlite}.ts
@@ -89,47 +92,69 @@ export function resolveReferentialActions(
8992
const target = ref.targetEntity;
9093
if (target === undefined) return { onDelete: undefined, onUpdate: undefined };
9194

92-
// Correlation is by exact-string match. Every fixture in the corpus uses
93-
// bare entity names for @objectRef and @references (no `::`-FQN form), so
94-
// bare-vs-bare matching is sufficient today. If a future author writes an
95-
// FQN value on either side, this find returns undefined and both actions
96-
// resolve to undefined (no clause emitted) — surfacing the mismatch as a
97-
// silent loss of intent rather than a wrong action. Cross-language ports
98-
// should match the same correlation rule.
9995
// (1) Actions declared directly on the FK-defining reference win.
10096
const refOnDelete = ref.onDelete;
10197
const refOnUpdate = ref.onUpdate;
10298

99+
// Resolve the reference's target ONCE, package-aware (ADR-0042: a bare
100+
// @references resolves in the DECLARING owner's package). Both relationship
101+
// tiers then correlate against the resolved node with refMatchesObject, so a
102+
// bare @references pairs correctly with an FQN @objectRef (and vice versa) —
103+
// an exact-string tier 2 used to miss the FQN form, letting the parent-side
104+
// tier override a child-side declaration.
105+
const root = entity.parent;
106+
const refOwner = ref.parent ?? entity;
107+
const refOwnerPkg = refOwner.package ?? refOwner.fileDefaultPackage ?? "";
108+
const targetObj =
109+
root !== undefined ? (resolveObjectRef(root, target, refOwnerPkg).node as MetaObject | undefined) : undefined;
110+
103111
// (2) Otherwise correlate with a sibling relationship and use its action /
104112
// subtype default. onUpdate's "cascade" default only applies when a
105113
// relationship is present, so a reference-only FK with no explicit
106-
// @onUpdate emits no ON UPDATE clause.
114+
// @onUpdate emits no ON UPDATE clause. An M:N relationship (@through)
115+
// never correlates — it describes the junction path, not this direct FK.
116+
// When the target does not resolve (dangling @references — normally a
117+
// load error), fall back to the legacy exact-string match so behavior on
118+
// partially-valid trees is unchanged.
107119
// (3) Failing that, correlate the REVERSE relationship declared on the
108120
// TARGET entity (the documented parent-side authoring shape).
109-
let rel = entity.relationships().find((r) => r.objectRef === target);
110-
if (rel === undefined) {
111-
const reverse = findReverseRelationship(entity, ref, target);
121+
let rel = entity.relationships().find((r) => {
122+
if (r.through !== undefined) return false;
123+
const objectRef = r.objectRef;
124+
if (objectRef === undefined) return false;
125+
if (targetObj === undefined) return objectRef === target;
126+
const relOwner = r.parent ?? entity;
127+
const relOwnerPkg = relOwner.package ?? relOwner.fileDefaultPackage ?? "";
128+
return refMatchesObject(targetObj, objectRef, relOwnerPkg);
129+
});
130+
// When the tier-3 satisfiability guard fires, the reverse relationship's
131+
// AUTHORED @onUpdate still applies (only the inferred contributions drop).
132+
let suppressedReverseOnUpdate: string | undefined;
133+
if (rel === undefined && targetObj !== undefined) {
134+
const reverse = findReverseRelationship(entity, ref, targetObj);
112135
// Tier-3 satisfiability guard: an INFERRED set-null default (a parent-side
113136
// aggregation with no explicit @onDelete) is unsatisfiable when any FK
114137
// column is NOT NULL — SET NULL cannot fire there, and letting it through
115138
// would turn a previously-valid model into a hard SetNullNotNullableError
116139
// purely because the correlation got smarter. An inferred default never
117-
// breaks a model: the correlation contributes nothing (today's bare FK).
118-
// An EXPLICIT @onDelete: "set-null" on the reverse relationship still flows
119-
// through and hits the loud validateSetNullNullability error — the author
120-
// asked for it, so silently dropping it would be the original bug again.
121-
if (
122-
reverse !== undefined &&
123-
!(
140+
// breaks a model: the INFERRED contributions drop (today's bare FK), while
141+
// anything the author explicitly wrote survives — an EXPLICIT @onDelete:
142+
// "set-null" flows through and hits the loud validateSetNullNullability
143+
// error (the author asked for it), and an EXPLICIT @onUpdate is honored
144+
// (silently dropping it would be the original bug again).
145+
if (reverse !== undefined) {
146+
const unsatisfiableInferredSetNull =
124147
reverse.onDelete === undefined &&
125148
ON_DELETE_DEFAULT_BY_SUBTYPE[reverse.subType] === "set-null" &&
126149
readIdentityFields(ref).some((jsName) => {
127150
const field = findField(entity, jsName);
128151
return field !== undefined && isRequired(field);
129-
})
130-
)
131-
) {
132-
rel = reverse;
152+
});
153+
if (unsatisfiableInferredSetNull) {
154+
suppressedReverseOnUpdate = reverse.onUpdate;
155+
} else {
156+
rel = reverse;
157+
}
133158
}
134159
}
135160

@@ -138,7 +163,7 @@ export function resolveReferentialActions(
138163
(rel ? (rel.onDelete ?? ON_DELETE_DEFAULT_BY_SUBTYPE[rel.subType]) : undefined);
139164
const onUpdateRaw =
140165
refOnUpdate ??
141-
(rel ? (rel.onUpdate ?? ON_UPDATE_DEFAULT) : undefined);
166+
(rel ? (rel.onUpdate ?? ON_UPDATE_DEFAULT) : suppressedReverseOnUpdate);
142167

143168
return {
144169
onDelete: normalize(onDeleteRaw),
@@ -153,12 +178,9 @@ export function resolveReferentialActions(
153178
* "Post", @cardinality: "many" }` on Author, while Post owns the FK).
154179
*
155180
* Guards (each fails closed to "no contribution"):
156-
* - The target entity must resolve from the root (package-aware via
157-
* resolveObjectRef; the reference's declaring owner supplies the package
158-
* context, mirroring the loader's ADR-0042 contract).
159181
* - An M:N relationship (`@through`) never correlates — it describes the
160182
* junction path, not this direct FK (the junction's own FKs correlate via
161-
* its own identity.reference children).
183+
* its own identity.reference children). The same guard applies at tier 2.
162184
* - When the FK-owning entity holds more than one enforced reference to the
163185
* same target, the reverse relationship cannot say WHICH FK carries the
164186
* ownership edge, so it contributes to none of them (arming every FK could
@@ -170,18 +192,11 @@ export function resolveReferentialActions(
170192
function findReverseRelationship(
171193
entity: MetaObject,
172194
ref: MetaReferenceIdentity,
173-
target: string,
195+
targetObj: MetaObject,
174196
): MetaRelationship | undefined {
175197
const root = entity.parent;
176198
if (root === undefined) return undefined;
177199

178-
// ADR-0042: a bare @references resolves in the DECLARING owner's package (an
179-
// inherited reference resolves in the package that declared it).
180-
const refOwner = ref.parent ?? entity;
181-
const refOwnerPkg = refOwner.package ?? refOwner.fileDefaultPackage ?? "";
182-
const targetObj = resolveObjectRef(root, target, refOwnerPkg).node;
183-
if (targetObj === undefined) return undefined;
184-
185200
// Ambiguity guard: exactly one enforced reference from `entity` to this
186201
// target, and it must be `ref` itself.
187202
const refsToTarget = entity.referenceIdentities().filter((r) => {
@@ -196,7 +211,7 @@ function findReverseRelationship(
196211

197212
// The reverse relationship's bare @objectRef resolves in ITS declaring
198213
// owner's package (normally the target entity's own package).
199-
return (targetObj as MetaObject).relationships().find((r) => {
214+
return targetObj.relationships().find((r) => {
200215
if (r.through !== undefined) return false; // M:N — junction path, not this FK
201216
const objectRef = r.objectRef;
202217
if (objectRef === undefined) return false;

0 commit comments

Comments
 (0)