Skip to content

Commit bb4cac2

Browse files
dmealingclaude
dmealing
and
claude
committed
fix(codegen-kotlin): #368 — port the referential-action correlation fix
KotlinExposedTableGenerator still had the defect migrate-ts/referential-actions.ts and the C# ReferentialActions port were fixed for, so this branch left the ports DISAGREEING where they had previously been consistently wrong. Tier 2 matched a sibling relationship on the TARGET ALONE (`firstOrNull { resolveObjectByShortOrFqn(...) === target }`), so with Match.homeTeamRef (@onDelete: restrict) and Match.awayTeamRef (@onDelete: cascade) both FKs were emitted with the FIRST relationship's action. Tier 3 took `firstOrNull` on the reverse relationship where TS/C# now fail closed. The port is a direct inversion, not a parallel rule: Kotlin is on the JVM and RelationshipReferences.resolveRelationshipReference is already on the classpath, so tier 2 now asks whether the ladder applied to `rel` resolves back to `ref` ITSELF, and tier 3 returns `singleOrNull()` over the reverse candidates. KotlinExposedTableTwoRefsTest covers both tiers and was verified to FAIL on the pre-fix generator (both tests red, with the wrong ReferenceOption in the emitted MatchTable). Full codegen-kotlin suite: 377 tests, 0 failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lfd8nat1WcSpXffd8YetiC
1 parent 423959e commit bb4cac2

2 files changed

Lines changed: 166 additions & 8 deletions

File tree

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

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import com.metaobjects.loader.MetaDataLoader
1616
import com.metaobjects.`object`.MetaObject
1717
import com.metaobjects.relationship.CompositionRelationship
1818
import com.metaobjects.relationship.MetaRelationship
19+
import com.metaobjects.relationship.RelationshipReferences
1920
import com.metaobjects.source.MetaSource
2021
import com.metaobjects.source.RdbSource
2122
import com.squareup.kotlinpoet.ClassName
@@ -1576,9 +1577,13 @@ open class KotlinExposedTableGenerator : MultiFileDirectGeneratorBase<MetaObject
15761577
// parent-side authoring shape) — a relationship contributes its explicit
15771578
// action, else its subtype default (composition→cascade,
15781579
// aggregation→set-null, association→restrict; onUpdate default cascade).
1579-
// Guards: @through (M:N) relationships never correlate with a direct FK,
1580-
// and the reverse relationship contributes nothing when this entity holds
1581-
// more than one enforced reference to the same target.
1580+
// Guards: @through (M:N) relationships never correlate with a direct FK;
1581+
// the child-side tier correlates by INVERTING the #368 relationship->
1582+
// reference ladder (not by target alone, which let a second FK inherit
1583+
// the first relationship's actions); and the reverse relationship
1584+
// contributes nothing when this entity holds more than one enforced
1585+
// reference to the same target, or when the target declares more than
1586+
// one relationship back at this entity.
15821587
val (resolvedOnDelete, resolvedOnUpdate) =
15831588
resolveDecorationActions(loader, entity, child, target)
15841589
val refSuffix = referentialActionSuffix(resolvedOnDelete, resolvedOnUpdate)
@@ -1609,9 +1614,24 @@ open class KotlinExposedTableGenerator : MultiFileDirectGeneratorBase<MetaObject
16091614

16101615
// (2) child-side correlation, then (3) parent-side reverse correlation.
16111616
// ADR-0039: relationships are inheritable — RESOLVE via getRelationships(true).
1617+
//
1618+
// #368: [entity] may declare MORE THAN ONE identity.reference onto this same
1619+
// target (Match.homeTeamRef / awayTeamRef both -> Team). Matching `rel` on the
1620+
// target ALONE cannot say which of those references `rel` supplies actions FOR,
1621+
// so every FK past the first silently inherited the FIRST relationship's
1622+
// @onDelete / @onUpdate — homeTeamRef's `restrict` landing on awayTeamRef's
1623+
// `cascade` FK, in a table Exposed compiles and the database accepts. Resolve it
1624+
// with the INVERSE of the relationship->reference ladder: `rel` belongs to `ref`
1625+
// iff the ladder, applied to `rel`, resolves back to `ref` ITSELF — not merely
1626+
// "to some reference on this target". `rel` and `ref` are both declared on (or
1627+
// inherited into) `entity`, the exact shape the ladder is built for, so this is
1628+
// a direct inversion rather than a second parallel rule. Mirrors the TS migrate
1629+
// engine's resolveReferentialActions and the C# ReferentialActions port.
16121630
val childSide = entity.relationships.firstOrNull { rel ->
16131631
rel.through == null && rel.objectRef != null &&
1614-
KotlinGenUtil.resolveObjectByShortOrFqn(loader, rel.objectRef) === target
1632+
KotlinGenUtil.resolveObjectByShortOrFqn(loader, rel.objectRef) === target &&
1633+
RelationshipReferences.resolveRelationshipReference(
1634+
entity, rel.shortName, rel.objectRef, rel.sourceRefField) === ref
16151635
}
16161636
var rel = childSide
16171637
// When the tier-3 satisfiability guard fires, the reverse relationship's
@@ -1651,10 +1671,11 @@ open class KotlinExposedTableGenerator : MultiFileDirectGeneratorBase<MetaObject
16511671
/**
16521672
* ADR-0047 tier-3 correlation — the relationship declared on the TARGET (parent)
16531673
* entity pointing back at the FK-owning entity. Fails closed (null) when the
1654-
* relationship is M:N (`@through`), or when [entity] does not hold exactly one
1674+
* relationship is M:N (`@through`), when [entity] does not hold exactly one
16551675
* enforced reference to [target] that is [ref] itself (the reverse relationship
16561676
* cannot say which FK carries the ownership edge; a soft `@enforce: false` ref
1657-
* never correlates). Mirrors the TS migrate engine's findReverseRelationship.
1677+
* never correlates), or when [target] declares more than one relationship back at
1678+
* [entity]. Mirrors the TS migrate engine's findReverseRelationship.
16581679
*/
16591680
private fun findReverseRelationship(
16601681
loader: MetaDataLoader,
@@ -1670,10 +1691,18 @@ open class KotlinExposedTableGenerator : MultiFileDirectGeneratorBase<MetaObject
16701691
KotlinGenUtil.resolveObjectByShortOrFqn(loader, r.targetEntity) === target
16711692
}
16721693
if (enforcedRefsToTarget.singleOrNull() !== ref) return null
1673-
return target.relationships.firstOrNull { rel ->
1694+
// #368: fail closed on ambiguity rather than taking the first match — when more
1695+
// than one non-@through relationship on [target] resolves back to [entity] (e.g.
1696+
// a "posts" composition and a separate "latestPost" association both pointing at
1697+
// Post), none of them is preferred. This is the tier-2 ambiguity's mirror image
1698+
// (multiple RELATIONSHIPS rather than multiple REFERENCES) and the ladder cannot
1699+
// resolve it: the ladder picks among references declared on the SAME object as
1700+
// the relationship, whereas here the candidates live on [target] while [ref]
1701+
// lives on [entity]. Mirrors the TS findReverseRelationship.
1702+
return target.relationships.filter { rel ->
16741703
rel.through == null && rel.objectRef != null &&
16751704
KotlinGenUtil.resolveObjectByShortOrFqn(loader, rel.objectRef) === entity
1676-
}
1705+
}.singleOrNull()
16771706
}
16781707

16791708
/**

‎server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableTwoRefsTest.kt‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
package com.metaobjects.generator.kotlin
2+
3+
import com.metaobjects.metadata.ktx.loadString
4+
import java.nio.file.Files
5+
import kotlin.test.Test
6+
import kotlin.test.assertTrue
7+
8+
/**
9+
* #368 — an entity may legally declare more than one `identity.reference` onto the
10+
* SAME target (`Match.homeTeamRef` and `Match.awayTeamRef`, both -> `Team`).
11+
*
12+
* `KotlinExposedTableGenerator`'s ADR-0047 referential-action correlation had the
13+
* defect the TS `migrate-ts` engine and the C# `ReferentialActions` port were fixed
14+
* for: tier 2 matched a sibling relationship on the TARGET ALONE, so every FK past
15+
* the first silently inherited the FIRST relationship's `@onDelete` / `@onUpdate`,
16+
* and tier 3 took the first reverse relationship instead of failing closed. The
17+
* emitted Exposed table compiles and the database accepts the DDL — the only symptom
18+
* is the wrong referential action on the wrong FK.
19+
*/
20+
class KotlinExposedTableTwoRefsTest {
21+
22+
/**
23+
* Tier 2 (child-side correlation). `homeTeam` declares `@onDelete: restrict`,
24+
* `awayTeam` declares `@onDelete: cascade`; each must land on ITS OWN FK column.
25+
* Before the fix both columns carried RESTRICT — the first relationship's action.
26+
*
27+
* The relationships resolve by ladder step 3 (name pairing: `homeTeam` pairs with
28+
* `homeTeamRef` / `homeTeamId`), so the model loads clean with no `@sourceRefField`.
29+
*/
30+
@Test fun `each FK carries its own relationship's referential actions`() {
31+
val model = """{
32+
"metadata.root": { "package": "x", "children": [
33+
{ "object.entity": { "name": "Team", "children": [
34+
{ "field.long": { "name": "id" } },
35+
{ "source.rdb": { "@table": "teams" } },
36+
{ "identity.primary": { "@fields": "id" } }
37+
] } },
38+
{ "object.entity": { "name": "Match", "children": [
39+
{ "field.long": { "name": "id" } },
40+
{ "field.long": { "name": "homeTeamId" } },
41+
{ "field.long": { "name": "awayTeamId" } },
42+
{ "source.rdb": { "@table": "matches" } },
43+
{ "identity.primary": { "@fields": "id" } },
44+
{ "identity.reference": { "name": "homeTeamRef",
45+
"@fields": "homeTeamId", "@references": "Team" } },
46+
{ "identity.reference": { "name": "awayTeamRef",
47+
"@fields": "awayTeamId", "@references": "Team" } },
48+
{ "relationship.association": { "name": "homeTeam", "@objectRef": "Team",
49+
"@cardinality": "one", "@onDelete": "restrict" } },
50+
{ "relationship.association": { "name": "awayTeam", "@objectRef": "Team",
51+
"@cardinality": "one", "@onDelete": "cascade" } }
52+
] } }
53+
] }
54+
}""".trimIndent()
55+
val outDir = Files.createTempDirectory("ktbl-368-tier2-")
56+
try {
57+
val gen = KotlinExposedTableGenerator()
58+
gen.setArgs(mapOf("outputDir" to outDir.toString()))
59+
gen.execute(loadString("issue368-tier2", model))
60+
61+
val src = Files.readString(outDir.resolve("x/MatchTable.kt"))
62+
assertTrue(
63+
("val homeTeamId = long(\"home_team_id\").references(TeamTable.id, " +
64+
"onDelete = ReferenceOption.RESTRICT, onUpdate = ReferenceOption.CASCADE)") in src,
65+
"expected homeTeamId to carry homeTeam's @onDelete: restrict; saw:\n$src",
66+
)
67+
assertTrue(
68+
("val awayTeamId = long(\"away_team_id\").references(TeamTable.id, " +
69+
"onDelete = ReferenceOption.CASCADE, onUpdate = ReferenceOption.CASCADE)") in src,
70+
"expected awayTeamId to carry awayTeam's OWN @onDelete: cascade, not " +
71+
"homeTeam's restrict (the #368 defect); saw:\n$src",
72+
)
73+
} finally {
74+
outDir.toFile().deleteRecursively()
75+
}
76+
}
77+
78+
/**
79+
* Tier 3 (parent-side reverse correlation) fails closed. `Team` declares TWO
80+
* non-`@through` relationships back at `Match` — a `cascade` composition and a
81+
* `restrict` association — and `Match` declares no relationship of its own, so
82+
* tier 2 contributes nothing and tier 3 must choose. It cannot: neither candidate
83+
* is preferred, so the FK is emitted BARE rather than arbitrarily armed with the
84+
* first one's CASCADE.
85+
*/
86+
@Test fun `an ambiguous reverse relationship contributes no referential action`() {
87+
val model = """{
88+
"metadata.root": { "package": "x", "children": [
89+
{ "object.entity": { "name": "Team", "children": [
90+
{ "field.long": { "name": "id" } },
91+
{ "source.rdb": { "@table": "teams" } },
92+
{ "identity.primary": { "@fields": "id" } },
93+
{ "relationship.composition": { "name": "matches", "@objectRef": "Match",
94+
"@cardinality": "many", "@onDelete": "cascade" } },
95+
{ "relationship.association": { "name": "playedMatches", "@objectRef": "Match",
96+
"@cardinality": "many", "@onDelete": "restrict" } }
97+
] } },
98+
{ "object.entity": { "name": "Match", "children": [
99+
{ "field.long": { "name": "id" } },
100+
{ "field.long": { "name": "teamId" } },
101+
{ "source.rdb": { "@table": "matches" } },
102+
{ "identity.primary": { "@fields": "id" } },
103+
{ "identity.reference": { "name": "teamRef",
104+
"@fields": "teamId", "@references": "Team" } }
105+
] } }
106+
] }
107+
}""".trimIndent()
108+
val outDir = Files.createTempDirectory("ktbl-368-tier3-")
109+
try {
110+
val gen = KotlinExposedTableGenerator()
111+
gen.setArgs(mapOf("outputDir" to outDir.toString()))
112+
gen.execute(loadString("issue368-tier3", model))
113+
114+
val src = Files.readString(outDir.resolve("x/MatchTable.kt"))
115+
assertTrue(
116+
"val teamId = long(\"team_id\").references(TeamTable.id).nullable()" in src ||
117+
"val teamId = long(\"team_id\").references(TeamTable.id)" in src,
118+
"expected a bare .references(TeamTable.id) FK; saw:\n$src",
119+
)
120+
assertTrue(
121+
"ReferenceOption" !in src,
122+
"an ambiguous reverse relationship must contribute NO referential action " +
123+
"rather than the first candidate's; saw:\n$src",
124+
)
125+
} finally {
126+
outDir.toFile().deleteRecursively()
127+
}
128+
}
129+
}

0 commit comments

Comments
 (0)