Skip to content

Commit d215568

Browse files
dmealingclaude
andcommitted
fix(codegen-csharp): the EF Core target emits real foreign keys, carrying @onDelete (#294)
DbContextGenerator emitted no 1:N relationship configuration at all. A generated entity carries a bare scalar FK property and — since ADR-0038 replaced reverse navigation with explicit FK finders — no navigation on either side, so EF Core built nothing: GetForeignKeys() returned an empty collection and every DeleteBehavior was EF's convention, on a port whose database has carried the declared ON DELETE correctly all along (the DDL is TS-owned, ADR-0015). Every enforced identity.reference now emits HasOne/WithMany/HasForeignKey with the action INLINE on the establishing call. That placement is the point: EF reconciles TPH relationships after OnModelCreating returns and can replace the FK metadata object, so a post-hoc GetForeignKeys().Single(...) assignment is silently discarded — an adopter measured 134 of 135 FKs taking the mutation, the exception being a TPH base+subtype dual-declared FK. The precedence is a tier-for-tier port of migrate-ts's referential-actions.ts (the cross-port SSOT), including the @through exclusion, the ambiguity guard, and the inferred-set-null-on-NOT-NULL satisfiability guard. Decided here: a TPH base and subtype declaring the same FK configure it once on the base; an M:N junction's sides ride on the UsingEntity call that creates them; a resolved set-null over a @required FK warns rather than failing EF model validation. @onUpdate has no EF representation and stays a DDL-level fact. The gate builds the real EF model from generated code and reads DeleteBehavior off the finalized IModel — confirmed to fail without the fix with an empty FK collection. A source-string assertion could not have caught this: it passes equally for the mutation that does not stick. relationships.md had documented this exact emission for releases with nothing comparing it to reality. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DhpswkF1NvwxhFWMmdAT15
1 parent 5e0b13f commit d215568

8 files changed

Lines changed: 870 additions & 10 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,52 @@ same blindness that let the original defect ship. `migrate-ts`'s `loadFixture` n
9595
loader errors instead of discarding them — a checked-in fixture had been declaring the
9696
illegal form and driving those suites green. All five ports green.
9797

98+
### Fixed — the C# EF Core target had no foreign keys at all, so `@onDelete` was inert ([#294](https://github.com/metaobjectsdev/metaobjects/issues/294))
99+
100+
**The issue reports that `DbContextGenerator` ignores `@onDelete`. It is worse than that:
101+
the generator emitted no 1:N relationship configuration whatsoever.** A generated entity
102+
carries a bare scalar FK property (`public long ProgramId`) and — since ADR-0038 replaced
103+
reverse navigation with explicit FK finders — no navigation property on either side. EF
104+
Core builds a relationship from a navigation or an explicit `HasOne`; with neither, it
105+
built **nothing**. `HasOne` appeared exactly once in the whole generated `AppDbContext`,
106+
inside the M:N `UsingEntity` call. So `modelBuilder.Entity<Week>().Metadata.GetForeignKeys()`
107+
returned an **empty collection**, and there was no foreign key for a delete behaviour to
108+
attach to — on a port where the database, whose DDL the TypeScript engine owns (ADR-0015),
109+
has carried `ON DELETE CASCADE` correctly all along.
110+
111+
**Now every enforced `identity.reference` emits an explicit relationship with the action
112+
INLINE on the establishing call**, which is what the issue asks for and the reason is
113+
specific: EF reconciles TPH relationships *after* `OnModelCreating` returns and may replace
114+
the FK metadata object, so a later `GetForeignKeys().Single(...).DeleteBehavior = ...` is
115+
silently discarded. The adopter measured exactly that — 134 of 135 FKs took the mutation;
116+
the one that did not was a TPH base+subtype dual-declared FK, reading back as EF's
117+
convention default with nothing thrown. Configuring the relationship as it is *established*
118+
is durable by construction.
119+
120+
The precedence is a **port of `migrate-ts`'s `referential-actions.ts`**, the cross-port SSOT,
121+
tier for tier: reference-level `@onDelete` → a correlated sibling relationship → the
122+
parent-side reverse relationship, package-aware (ADR-0042), with `@through` excluded, the
123+
more-than-one-reference-to-the-same-target ambiguity guard, and the inferred-`set-null`-on-
124+
NOT-NULL satisfiability guard. Three behaviours are decided rather than inherited: a TPH
125+
base and subtype declaring the **same** FK configure it **once**, on the base that owns the
126+
shared column (a duplicate is the ambiguity the adopter was working around); an M:N
127+
junction's FK sides ride on the `UsingEntity` call that creates them, so the per-reference
128+
pass skips junction entities rather than configuring one FK twice; and a resolved
129+
`set-null` over a `@required` FK **warns and emits no clause**, because EF fails MODEL
130+
VALIDATION there and would take down the entire `DbContext` rather than one relationship.
131+
`@onUpdate` is deliberately not surfaced — `DeleteBehavior` covers deletes only, so it
132+
remains a DDL-level fact.
133+
134+
**The durable lesson is the familiar one: the thing that would have caught this did not
135+
exist.** [`docs/features/relationships.md`](docs/features/relationships.md) has shown the
136+
exact `HasOne<Author>().WithMany().HasForeignKey(...).OnDelete(DeleteBehavior.Cascade)`
137+
snippet as the C# port's output for releases, and nothing ever compared it to what the
138+
generator emits. A string assertion over generated source would not have closed it either —
139+
it passes just as happily for the post-hoc mutation that does not stick. The new gate builds
140+
the **real** EF model from the generated code and reads `DeleteBehavior` back off the
141+
finalized `IModel` (model-only, no container), and it was confirmed to fail without the fix
142+
with `Collection: []` — the empty FK set above.
143+
98144
### Fixed — `meta upgrade` rewrote nothing on a YAML estate, then called it clean ([#339](https://github.com/metaobjectsdev/metaobjects/issues/339))
99145

100146
`meta upgrade` is the fixer for everything `0.24.0` retired. On a **YAML** estate it

‎docs/features/relationships.md‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -238,13 +238,20 @@ KEY ... ON DELETE CASCADE` in the Postgres DDL.
238238

239239
```csharp
240240
// generated/AppDbContext.cs (excerpt)
241-
modelBuilder.Entity<Post>()
242-
.HasOne<Author>()
243-
.WithMany()
244-
.HasForeignKey(p => p.AuthorId)
245-
.OnDelete(DeleteBehavior.Cascade);
241+
modelBuilder.Entity<Post>().HasOne<Author>().WithMany()
242+
.HasForeignKey(nameof(Post.AuthorId)).OnDelete(DeleteBehavior.Cascade);
246243
```
247244

245+
The action rides on the call that establishes the foreign key, never a later
246+
`GetForeignKeys(...)` mutation: EF Core reconciles TPH relationships *after*
247+
`OnModelCreating` returns and can replace the foreign-key metadata object, which
248+
silently discards a post-hoc assignment ([#294](https://github.com/metaobjectsdev/metaobjects/issues/294)).
249+
`WithMany()` is inverse-less because the port emits no reverse collection
250+
navigations at all — reverse traversal is the explicit FK finders of ADR-0038.
251+
252+
`@onUpdate` has no EF Core representation (`DeleteBehavior` covers deletes only),
253+
so it stays a DDL-level fact emitted by the TypeScript-owned migration engine.
254+
248255
```sql
249256
-- emitted by `meta migrate`
250257
ALTER TABLE "posts"
Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
// #294 — the C# EF Core target emits explicit 1:N relationship configuration carrying
2+
// the ADR-0047 referential action.
3+
//
4+
// Before this, DbContextGenerator emitted NO relationship configuration for a plain
5+
// foreign key at all: the generated entity carries a bare scalar FK property and no
6+
// navigation, so EF Core built no relationship, GetForeignKeys() was empty, and every
7+
// DeleteBehavior fell back to EF's convention regardless of what the metadata — or the
8+
// database — said. @onDelete was inert on this port.
9+
//
10+
// These tests pin the RESOLUTION (the ADR-0047 precedence, ported from the migrate-ts
11+
// SSOT) and the EMISSION SHAPE. That the emitted model is durable through EF's own
12+
// finalization — the TPH failure the issue reports — is pinned separately by
13+
// Issue294EfModelDeleteBehaviorTests, which builds a real EF model and reads the
14+
// behavior back.
15+
16+
using MetaObjects.Codegen;
17+
using MetaObjects.Codegen.Generators;
18+
using MetaObjects.Loader;
19+
using MetaObjects.Meta;
20+
using Xunit;
21+
22+
namespace MetaObjects.Codegen.Tests;
23+
24+
public class Issue294ReferentialActionTests
25+
{
26+
// Program (parent, declares the to-many composition) / Week (owns the FK).
27+
// The reference carries whatever @onDelete the test injects; the parent-side
28+
// relationship is the documented authoring shape and drives tier-3 correlation.
29+
// Tokens are substituted before parsing, so they never reach the JSON reader.
30+
// (String tokens rather than interpolation: the model is dense in `}}`, which an
31+
// interpolated raw string would read as an interpolation hole.)
32+
private const string TwoEntityTemplate = """
33+
{ "metadata.root": { "package": "acme", "children": [
34+
{ "object.entity": { "name": "Program", "children": [
35+
{ "source.rdb": { "@table": "programs" } },
36+
{ "field.long": { "name": "id" } },
37+
{ "identity.primary": { "@fields": "id" } },
38+
{ "relationship.composition": { "name": "weeks", "@cardinality": "many", "@objectRef": "Week" } }
39+
]}},
40+
{ "object.entity": { "name": "Week", "children": [
41+
{ "source.rdb": { "@table": "weeks" } },
42+
{ "field.long": { "name": "id" } },
43+
{ "field.long": { "name": "programId"/*FK_ATTRS*/ } },
44+
{ "identity.primary": { "@fields": "id" } }/*WEEK_REL*/,
45+
{ "identity.reference": { "name": "refProgram", "@fields": "programId",
46+
"@references": "Program"/*REF_ATTRS*/ } }
47+
]}}
48+
]}}
49+
""";
50+
51+
private static string TwoEntityModel(
52+
string referenceAttrs = "", string weekRelationship = "", string programIdAttrs = "") =>
53+
TwoEntityTemplate
54+
.Replace("/*REF_ATTRS*/", referenceAttrs)
55+
.Replace("/*WEEK_REL*/", weekRelationship)
56+
.Replace("/*FK_ATTRS*/", programIdAttrs);
57+
58+
private static (string Source, List<string> Warnings) Generate(string model)
59+
{
60+
var result = new MetaDataLoader().Load([new InMemoryStringSource(model, id: "issue-294.json")]);
61+
Assert.Empty(result.Errors);
62+
63+
var warnings = new List<string>();
64+
var ctx = new GenContext
65+
{
66+
Entities = result.Root.Objects(),
67+
Root = result.Root,
68+
Config = new GenConfig { OutDir = "/unused", Namespace = "Acme.Generated" },
69+
Warn = warnings.Add,
70+
};
71+
var file = Assert.Single(new DbContextGenerator().Generate(ctx));
72+
return (file.Content, warnings);
73+
}
74+
75+
private static string WeekFkLine(string source) =>
76+
Assert.Single(source.Split('\n'), l => l.Contains("HasOne<Program>()")).Trim();
77+
78+
[Fact]
79+
public void A_plain_reference_now_configures_a_real_foreign_key()
80+
{
81+
var (source, _) = Generate(TwoEntityModel());
82+
// The FK must be established by the config call itself — never a later
83+
// GetForeignKeys() mutation, which TPH finalization can silently discard (#294).
84+
Assert.Equal(
85+
"modelBuilder.Entity<Week>().HasOne<Program>().WithMany()"
86+
+ ".HasForeignKey(nameof(Week.ProgramId)).OnDelete(DeleteBehavior.Cascade);",
87+
WeekFkLine(source));
88+
Assert.DoesNotContain("GetForeignKeys", source);
89+
}
90+
91+
[Fact]
92+
public void Tier1_an_action_on_the_reference_itself_wins()
93+
{
94+
var (source, _) = Generate(TwoEntityModel(referenceAttrs: """, "@onDelete": "restrict" """));
95+
Assert.Contains(".OnDelete(DeleteBehavior.Restrict)", WeekFkLine(source));
96+
}
97+
98+
[Fact]
99+
public void Tier2_a_sibling_relationship_beats_the_parent_side_one()
100+
{
101+
// Week's own association (restrict) must win over Program's composition (cascade).
102+
var (source, _) = Generate(TwoEntityModel(weekRelationship: """
103+
,{ "relationship.association": { "name": "program", "@cardinality": "one", "@objectRef": "Program" } }
104+
"""));
105+
Assert.Contains(".OnDelete(DeleteBehavior.Restrict)", WeekFkLine(source));
106+
}
107+
108+
[Fact]
109+
public void Tier3_the_parent_side_composition_supplies_its_subtype_default()
110+
{
111+
// No action anywhere: the only source is Program's to-many composition, whose
112+
// subtype default is cascade. This is the shape the authoring docs teach.
113+
var (source, _) = Generate(TwoEntityModel());
114+
Assert.Contains(".OnDelete(DeleteBehavior.Cascade)", WeekFkLine(source));
115+
}
116+
117+
[Fact]
118+
public void No_action_emits_no_clause_because_it_is_the_database_default()
119+
{
120+
var (source, _) = Generate(TwoEntityModel(referenceAttrs: """, "@onDelete": "no-action" """));
121+
var line = WeekFkLine(source);
122+
Assert.Contains("HasForeignKey(nameof(Week.ProgramId));", line);
123+
Assert.DoesNotContain(".OnDelete(", line);
124+
}
125+
126+
[Fact]
127+
public void Set_null_over_a_required_fk_warns_instead_of_breaking_model_validation()
128+
{
129+
// EF fails MODEL VALIDATION for SetNull over a non-nullable FK, which would take
130+
// down the entire DbContext rather than one relationship. Warn and omit.
131+
var (source, warnings) = Generate(TwoEntityModel(
132+
referenceAttrs: """, "@onDelete": "set-null" """,
133+
programIdAttrs: ""","@required": true"""));
134+
135+
Assert.DoesNotContain(".OnDelete(", WeekFkLine(source));
136+
Assert.Contains(warnings, w => w.Contains("SET NULL") && w.Contains("programId"));
137+
}
138+
139+
[Fact]
140+
public void A_logical_only_reference_configures_nothing()
141+
{
142+
// ADR-0046 — @enforce:false is a navigation-only reference with no FK at all.
143+
var (source, _) = Generate(TwoEntityModel(referenceAttrs: """, "@enforce": false """));
144+
Assert.DoesNotContain("HasOne<Program>()", source);
145+
}
146+
147+
// A TPH base and one concrete subtype declaring the SAME foreign key — the exact
148+
// shape #294 reports. The physical column is one column on the shared table, so it
149+
// must be configured exactly ONCE; a duplicate configuration is what made the
150+
// adopter's post-hoc mutation unreliable.
151+
private const string TphDualDeclaration = """
152+
{ "metadata.root": { "package": "acme", "children": [
153+
{ "object.entity": { "name": "User", "children": [
154+
{ "source.rdb": { "@table": "users" } },
155+
{ "field.long": { "name": "id" } },
156+
{ "identity.primary": { "@fields": "id" } }
157+
]}},
158+
{ "object.entity": { "name": "Item", "@discriminator": "kind", "children": [
159+
{ "source.rdb": { "@table": "items" } },
160+
{ "field.long": { "name": "id" } },
161+
{ "field.enum": { "name": "kind", "@values": ["Alpha", "Beta"] } },
162+
{ "field.long": { "name": "senderId" } },
163+
{ "identity.primary": { "@fields": "id" } },
164+
{ "identity.reference": { "name": "refSender", "@fields": "senderId",
165+
"@references": "User", "@onDelete": "cascade" } }
166+
]}},
167+
{ "object.entity": { "name": "AlphaItem", "extends": "Item", "@discriminatorValue": "Alpha",
168+
"children": [
169+
{ "identity.reference": { "name": "refSenderAlpha", "@fields": "senderId",
170+
"@references": "User", "@onDelete": "cascade" } }
171+
]}},
172+
{ "object.entity": { "name": "BetaItem", "extends": "Item", "@discriminatorValue": "Beta",
173+
"children": [
174+
{ "field.string": { "name": "note", "@maxLength": 40 } }
175+
]}}
176+
]}}
177+
""";
178+
179+
[Fact]
180+
public void A_tph_base_and_subtype_declaring_the_same_fk_configure_it_once()
181+
{
182+
var (source, _) = Generate(TphDualDeclaration);
183+
var fkLines = source.Split('\n').Where(l => l.Contains("HasOne<User>()")).ToList();
184+
185+
var line = Assert.Single(fkLines);
186+
// Configured on the BASE, which owns the shared table's column.
187+
Assert.Contains("modelBuilder.Entity<Item>()", line);
188+
Assert.Contains(".OnDelete(DeleteBehavior.Cascade)", line);
189+
}
190+
191+
[Fact]
192+
public void A_model_with_no_enforced_reference_is_unchanged()
193+
{
194+
// Guards the byte-identical promise for models this feature does not touch.
195+
var (source, _) = Generate("""
196+
{ "metadata.root": { "package": "acme", "children": [
197+
{ "object.entity": { "name": "Solo", "children": [
198+
{ "source.rdb": { "@table": "solos" } },
199+
{ "field.long": { "name": "id" } },
200+
{ "identity.primary": { "@fields": "id" } }
201+
]}}
202+
]}}
203+
""");
204+
Assert.DoesNotContain("HasOne<", source);
205+
Assert.DoesNotContain("OnDelete(", source);
206+
}
207+
}

0 commit comments

Comments
 (0)