diff --git a/CHANGELOG.md b/CHANGELOG.md index a2158b708..913d1257e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,92 @@ here.** ### Fixed +- **An M:N relationship inherited through `extends` derived its junction FK columns + against the wrong entity — and in codegen the failure was silent.** The derivation + classified the self-join, and matched the junction's source-side + `identity.reference`, against the entity the CALLER was iterating. Every caller walks + a resolving relationship accessor, so for a relationship declared on a base and + reached through a subclass that is the INHERITING entity, not the one that declared + it. Two failures followed: an inherited self-join compared `@objectRef` (the base) + against the child, read as hetero, looked for a junction reference to the child and + found none; and an inherited hetero whose junction references the base found nothing + either. TypeScript codegen and the docs-site link graph catch the resulting error and + return `null`, so the navigation was **dropped from the generated output with no + error at all**; C# codegen did the same; the TypeScript, Java, Kotlin and Python + runtime and codegen paths let it escape, so the traversal or the generation run + failed outright. + + The declaring entity now comes from the relationship's own parent — the same shape as + [#368](https://github.com/metaobjectsdev/metaobjects/issues/368)'s loader fix — in all + four derivations (TypeScript, Java, C#, Python; Kotlin calls the Java helper). The + entity being navigated from is kept alongside it rather than discarded: under + inheritance both are legitimate names for the relationship's subject, because a + junction FK usually references the concrete child while `@objectRef` on a hoisted + self-join names the base. The authoring contract this establishes — **the junction FK + may reference either the declaring base or the concrete child, and only those two** — + is now written down in + [`docs/features/relationships.md`](docs/features/relationships.md). + + C# additionally fixes `M2MNavigation.IsSelfJoin`, which had the same confusion one + layer up. It never ran on an inherited self-join before (the derivation threw first), + and `DbContextGenerator` uses it to decide whether to emit EF `UsingEntity` wiring — + so fixing only the derivation would have turned a silent drop into silently wrong EF + configuration. + + **Not a pure widening.** One shape that derived before now refuses: a base declaring + `@objectRef: ` + `@through` with neither `@symmetric` nor `@sourceRefField`, + reached through a subclass, used to be misread as hetero and returned an arbitrary FK + direction; it is now correctly recognised as an ambiguous self-join and refused. That + model was already broken — deriving the same relationship from the base itself threw — + so codegen emitted for the child and dropped it for the base. The refusal is the + correct behaviour, but on Java, Kotlin and Python, whose callers do not catch, it + moves from "generates wrongly" to "the generation run fails", and the fix is to add + `@symmetric` or `@sourceRefField`. + + Three narrower resolution changes come with moving the junction matches onto identity, + all three matching what the Java port already did. A junction `@references` (or an + `@objectRef`) that **is** package-qualified must now resolve **exactly**: a + partially-qualified or stale package no longer falls back to matching the bare tail, so + a reference that used to bind by luck now matches neither the subject **nor** the + target — both sides are affected, not just the subject side. A **bare** reference whose + short name exists in more than one package resolves first-declared-wins, which can pick + the wrong-package entity — the pre-existing + [#174](https://github.com/metaobjectsdev/metaobjects/issues/174) behaviour, now reached + by M:N derivation as well. And on **Python only**, a junction whose `@references` use + the dotted `Entity.field` form now resolves: that port compared the whole attr value, so + `Team.id` never matched the entity `Team` and an M:N through such a junction failed + derivation outright. Both junction matches now take the entity head through the same + canonical parse the loader uses, so those models derive where they previously raised. + + **Cross-port divergence goes DOWN, not up.** Both junction matches — "does this + reference name the relationship's subject?" and "does this one name the target?" — now + resolve the name to an ENTITY and compare identity in all four derivations, which is + what the Java port already did on both sides. Matching only one side would be worse + than matching neither: the two searches are independent and nothing excludes the + source-side reference from the target search, so a cross-package M:N could bind the + same junction column as BOTH sides and emit `(srcFk, srcFk)` silently. C#'s + `M2MNavigation` descriptor resolves its target the same way for the same reason — its + `IsSelfJoin` feeds the EF `UsingEntity` wiring, and a descriptor that disagreed with + the derivation would mis-map the relationship. TypeScript, C# and Python had been + comparing package-stripped short names, so a genuine cross-package hetero M:N onto a + target whose short name matched the subject's (`a::NodeBase` relating to `b::NodeBase`) + was misread as a self-join on those three — a regression the two-name subject + introduced, caught in review and fixed rather than documented. + + What changed, exactly: both junction matches in the four derivations, the C# navigation + builder's own target/junction resolution (so the descriptor cannot disagree with the + derivation feeding it), and Python's reference head-parse, which now delegates to the + loader's canonical helper instead of keeping a third copy. `@through` resolution and + every comparison outside M:N derivation are untouched, and this is **not** a general + [ADR-0041](spec/decisions/ADR-0041-cross-package-reference-resolution.md) sweep — the + resolver added here is deliberately narrow and is not the port's general reference + resolver. + + No vocabulary change: `metamodelVersion` stays `1.0` and the registry manifest is + untouched. + +||||||| 7dafb055e + - **Both shipped libraries failed `meta verify`'s requirement gate**, in metadata an adopter cannot fix: every L4 in `ai` claimed FIELDS (`ERR_REQUIREMENT_L4_NOT_OBJECT`), and both libraries wrote their concerns as SIBLINGS of the L2 segment their own comments diff --git a/docs/features/relationships.md b/docs/features/relationships.md index b4ee52e8a..9caf2a569 100644 --- a/docs/features/relationships.md +++ b/docs/features/relationships.md @@ -102,6 +102,9 @@ metadata: | `@cardinality` | `relationship.composition` | `one` / `many` | Multiplicity on the target side | | `@fields` | `identity.reference` | One field name or array | The FK column(s) on this entity | | `@references` | `identity.reference` | Entity name | The target entity (PK on the other side) | +| `@through` | `relationship.*` | Junction entity name | Makes the relationship M:N. With `@cardinality: many`, names the junction entity whose two `identity.reference` children the FK columns are DERIVED from — the relationship never restates them. | +| `@sourceRefField` | `relationship.*` | FK field name | On an M:N, names the source-side FK field on the junction (a DIRECTED self-join). On a `@cardinality: one` relationship, picks which of several `identity.reference` nodes onto the same target it navigates (see below). Mutually exclusive with `@symmetric`. | +| `@symmetric` | `relationship.*` | `true` | Marks an UNDIRECTED M:N self-join (union-on-read). Valid only when `@objectRef` is the relationship's own subject. Mutually exclusive with `@sourceRefField`. | | `@onDelete` | `relationship.*` and `identity.reference` | `cascade` / `set-null` / `restrict` / `no-action` | RDB referential action. Default derives from the relationship subtype: composition -> `cascade`, aggregation -> `set-null`, association -> `restrict`. | | `@onUpdate` | `relationship.*` and `identity.reference` | same as `@onDelete` (default `cascade` when a relationship correlates) | RDB referential action | @@ -240,6 +243,79 @@ See [ADR-0029](../../spec/decisions/ADR-0029-entity-child-extends-and-via-infere Amendment 1 for the full ladder specification, including why suffix-stripping applies to candidates only. +## Inheriting an M:N relationship through `extends` + +An M:N relationship declared on an abstract base is visible on every entity that +`extends` it — relationship accessors are RESOLVING, so `Post` sees the `tags` +relationship its `PostBase` declared. The junction's two `identity.reference` +children are what give the FK direction, and under inheritance there are two +defensible entities for the source-side reference to name: + +- the **declaring base** (`PostBase`) — the entity the relationship is written on, and + what `@objectRef` names for a self-join hoisted onto a base; or +- the **concrete child** (`Post`) — usually what the FK actually references, because + an abstract base has no table for a foreign key to point at. + +**Both are accepted, and only those two.** The FK derivation treats the declaring +entity and the entity you are navigating from as the relationship's *subject*: the +source-side junction reference may name either, and `@objectRef` naming either makes +the relationship a self-join. Nothing else counts — in particular an entity lying +strictly *between* the declaring base and the navigating entity in a deeper hierarchy +is **not** accepted, and a junction reference naming one fails derivation with +`ERR_INVALID_RELATIONSHIP`. + +```yaml +# PostBase (abstract) declares the M:N; Post extends it. The junction may reference +# EITHER PostBase or Post — both derive postId/tagId for Post.tags. +- object.entity: + name: PostBase + isAbstract: true + children: + - relationship.association: + name: tags + objectRef: Tag + cardinality: many + through: PostTag +- object.entity: + name: Post + extends: PostBase +- object.entity: + name: PostTag + children: + - identity.reference: + name: fkPost + fields: postId + references: Post # or PostBase — either resolves + - identity.reference: + name: fkTag + fields: tagId + references: Tag +``` + +The same rule governs an inherited **self-join**: a base declaring +`@objectRef: ` with `@symmetric` or `@sourceRefField` derives the same two FK +sides whichever subclass you reach it through. The derivation's answer never depends +on which entity's effective view got there first — that independence is the point, and +it is what +[#368](https://github.com/metaobjectsdev/metaobjects/issues/368)'s loader fix +established for validation and this rule extends to FK derivation. + +**Cross-package targets are safe.** All five ports resolve `@objectRef` and each junction +`identity.reference` to an ENTITY and compare identity — on **both** sides of the +derivation, the source-side match and the target-side match alike. So a genuine +cross-package hetero M:N whose target's short name happens to match the source's +(`a::Account` relating to `b::Account`, or `a::NodeBase` to `b::NodeBase`) binds each +junction reference to its own entity instead of matching one of them twice. A +package-qualified name resolves exactly +([ADR-0041](../../spec/decisions/ADR-0041-cross-package-reference-resolution.md)); a bare +name matches a short name, where a collision across packages is the deferred follow-up +[#174](https://github.com/metaobjectsdev/metaobjects/issues/174), the same as everywhere +else a bare reference is resolved. Two consequences worth knowing when authoring: a +junction `@references` that *is* package-qualified must resolve **exactly** — a +partially-qualified or stale package no longer falls back to a bare-tail match — and a +**bare** `@references` whose short name exists in more than one package binds the first +declared, which is #174 and not specific to M:N. + ## What each port generates ### TypeScript @@ -372,6 +448,17 @@ Cross-port runner coverage: TS / Java / Kotlin / C# / Python all execute these via their respective conformance runners. See [`docs/CONFORMANCE.md`](../CONFORMANCE.md) for the per-port pass/skip ledger. +**Not fixture-gated, and why.** The M:N junction-FK DERIVATION — including the +inherited-relationship rule above — cannot be expressed in `fixtures/conformance/`: +that corpus is a load→canonical-serialize round-trip, and the serializer preserves the +declared `@objectRef` / `@through` / `@sourceRefField` strings without ever surfacing +which junction column the derivation picked. Two models that derive differently +serialize identically. It is gated instead by per-port unit tests over the shared +derivation helper (`relationship-m2m.test.ts`, `M2MSlimVocabularyTest.java`, +`M2MInheritedDeclaringEntityTests.cs`, `test_derive_m2m_declaring_entity.py`, +`KotlinM2mCodegenTest.kt`), which is the same call the M:N FQN-collision cases already +made. + ## See also - [entities.md](entities.md) — host node `object.entity` diff --git a/server/csharp/MetaObjects.Codegen.Tests/M2MInheritedDeclaringEntityTests.cs b/server/csharp/MetaObjects.Codegen.Tests/M2MInheritedDeclaringEntityTests.cs new file mode 100644 index 000000000..5fe2981f9 --- /dev/null +++ b/server/csharp/MetaObjects.Codegen.Tests/M2MInheritedDeclaringEntityTests.cs @@ -0,0 +1,338 @@ +// Follow-up to #368 — M:N derivation used the VISITING entity, not the entity that +// DECLARES the relationship. +// +// M2MNavigationBuilder.For walks the RESOLVING Relationships(), so a M:N declared on +// an abstract base is reached again through every entity that extends it, with the +// INHERITING entity as `source`. M2MDerivation then compared @objectRef against that +// entity: an inherited self-join read as hetero, looked for a junction reference to +// the child, found none and threw — and M2MNavigationBuilder.Build CATCHES +// M2MDerivationException and returns null, so the navigation vanished from the +// generated entity, DbContext and routes with no error at all. +// +// Both authoring shapes must work: the junction FK may reference the declaring BASE, +// or (more usually) the CONCRETE child, since the abstract base has no table. The +// counter-case test below guards the second. +// +// The child is declared BEFORE the base so the walk reaches it first — the order +// shape the #368 loader regressions established. (DbContextGenerator additionally +// sorts entities by name, and "Node" < "NodeBase" ordinally, so that path sees the +// child first too.) + +using MetaObjects.Codegen.Generators; +using MetaObjects.Core.Relationship; +using MetaObjects.Loader; +using MetaObjects.Meta; +using Xunit; + +namespace MetaObjects.Codegen.Tests; + +public class M2MInheritedDeclaringEntityTests +{ + // NodeBase declares a @symmetric self-join onto ITSELF; the junction references + // the BASE. Node extends it and is declared first. + private const string SelfJoinModel = """ + { "metadata.root": { "package": "acme", "children": [ + { "object.entity": { "name": "Node", "extends": "NodeBase", "children": [ + { "source.rdb": { "@table": "nodes" } }, + { "field.long": { "name": "id" } }, + { "identity.primary": { "@fields": "id" } } + ]}}, + { "object.entity": { "name": "NodeBase", "@isAbstract": true, "children": [ + { "relationship.association": { "name": "peers", "@cardinality": "many", "@objectRef": "NodeBase", "@through": "NodeLink", "@symmetric": true } } + ]}}, + { "object.entity": { "name": "NodeLink", "children": [ + { "source.rdb": { "@table": "node_links" } }, + { "field.long": { "name": "aId" } }, + { "field.long": { "name": "bId" } }, + { "identity.primary": { "@fields": ["aId", "bId"] } }, + { "identity.reference": { "name": "fkA", "@fields": "aId", "@references": "NodeBase" } }, + { "identity.reference": { "name": "fkB", "@fields": "bId", "@references": "NodeBase" } } + ]}} + ]}} + """; + + // ArticleBase declares a HETERO M:N; the junction references the BASE. + private const string HeteroBaseRefModel = """ + { "metadata.root": { "package": "acme", "children": [ + { "object.entity": { "name": "Article", "extends": "ArticleBase", "children": [ + { "source.rdb": { "@table": "articles" } }, + { "field.long": { "name": "id" } }, + { "identity.primary": { "@fields": "id" } } + ]}}, + { "object.entity": { "name": "ArticleBase", "@isAbstract": true, "children": [ + { "relationship.association": { "name": "tags", "@cardinality": "many", "@objectRef": "Tag", "@through": "ArticleTag" } } + ]}}, + { "object.entity": { "name": "Tag", "children": [ + { "source.rdb": { "@table": "tags" } }, + { "field.long": { "name": "id" } }, + { "identity.primary": { "@fields": "id" } } + ]}}, + { "object.entity": { "name": "ArticleTag", "children": [ + { "source.rdb": { "@table": "article_tags" } }, + { "field.long": { "name": "articleId" } }, + { "field.long": { "name": "tagId" } }, + { "identity.primary": { "@fields": ["articleId", "tagId"] } }, + { "identity.reference": { "name": "fkArticle", "@fields": "articleId", "@references": "ArticleBase" } }, + { "identity.reference": { "name": "fkTag", "@fields": "tagId", "@references": "Tag" } } + ]}} + ]}} + """; + + // The other legitimate shape: the junction references the CONCRETE child. + private const string HeteroConcreteRefModel = """ + { "metadata.root": { "package": "acme", "children": [ + { "object.entity": { "name": "Post", "extends": "PostBase", "children": [ + { "source.rdb": { "@table": "posts" } }, + { "field.long": { "name": "id" } }, + { "identity.primary": { "@fields": "id" } } + ]}}, + { "object.entity": { "name": "PostBase", "@isAbstract": true, "children": [ + { "relationship.association": { "name": "tags", "@cardinality": "many", "@objectRef": "Tag", "@through": "PostTag" } } + ]}}, + { "object.entity": { "name": "Tag", "children": [ + { "source.rdb": { "@table": "tags" } }, + { "field.long": { "name": "id" } }, + { "identity.primary": { "@fields": "id" } } + ]}}, + { "object.entity": { "name": "PostTag", "children": [ + { "source.rdb": { "@table": "post_tags" } }, + { "field.long": { "name": "postId" } }, + { "field.long": { "name": "tagId" } }, + { "identity.primary": { "@fields": ["postId", "tagId"] } }, + { "identity.reference": { "name": "fkPost", "@fields": "postId", "@references": "Post" } }, + { "identity.reference": { "name": "fkTag", "@fields": "tagId", "@references": "Tag" } } + ]}} + ]}} + """; + + private static MetaRoot Load(string model) + { + var r = new MetaDataLoader().Load([new InMemoryStringSource(model, id: "inherited-m2m.json")]); + Assert.Empty(r.Errors); + return r.Root; + } + + private static MetaRelationship Rel(MetaObject entity, string name) => + entity.Relationships().Single(r => r.Name == name); + + [Fact] + public void Derive_inherited_self_join_uses_the_declaring_entity() + { + var root = Load(SelfJoinModel); + // Pin the premise: the child is walked before the base it inherits from. + Assert.Equal(["Node", "NodeBase", "NodeLink"], root.Objects().Select(o => o.Name).ToArray()); + + var node = root.FindObject("Node")!; + var rel = Rel(node, "peers"); + Assert.Empty(node.OwnRelationships()); // genuinely inherited + Assert.Equal("NodeBase", (rel.Parent as MetaObject)!.Name); + + var fields = M2MDerivation.DeriveM2MFields(rel, node, root); + Assert.Equal("aId", fields.SourceField); + Assert.Equal("bId", fields.TargetField); + // The declaring entity itself must agree — same node, same answer. + Assert.Equal(fields, M2MDerivation.DeriveM2MFields(rel, root.FindObject("NodeBase")!, root)); + } + + [Fact] + public void Derive_inherited_hetero_matches_the_bases_junction_reference() + { + var root = Load(HeteroBaseRefModel); + var article = root.FindObject("Article")!; + var fields = M2MDerivation.DeriveM2MFields(Rel(article, "tags"), article, root); + Assert.Equal("articleId", fields.SourceField); + Assert.Equal("tagId", fields.TargetField); + } + + [Fact] + public void Derive_inherited_hetero_matches_a_concrete_junction_reference() + { + // Counter-case: accepting ONLY the declaring entity would break this shape, + // which is the common one (the abstract base has no table). + var root = Load(HeteroConcreteRefModel); + var post = root.FindObject("Post")!; + var rel = Rel(post, "tags"); + Assert.Equal("PostBase", (rel.Parent as MetaObject)!.Name); + var fields = M2MDerivation.DeriveM2MFields(rel, post, root); + Assert.Equal("postId", fields.SourceField); + Assert.Equal("tagId", fields.TargetField); + } + + // REGRESSION — a cross-package hetero M:N must not be read as a self-join just + // because the target's SHORT name matches one of the subject's. + // + // a::NodeBase declares a genuine cross-package hetero M:N onto b::NodeBase, and + // a::Node extends a::NodeBase. Deriving from a::Node the subject is + // {a::NodeBase, a::Node}; under the old StripPackage compare "b::NodeBase" + // stripped to "NodeBase", landed in the subject set, and the relationship refused + // to derive as an ambiguous self-join. It had derived correctly before the subject + // set grew to two names, so that was a regression, not a pre-existing gap. + // + // Fixed by comparing RESOLVED OBJECT IDENTITY, which is what the Java port has + // always done — this is the C# half of the pair with + // M2MSlimVocabularyTest.deriveCrossPackageHeteroBindsCorrectPackage, and it + // REDUCES the cross-port divergence rather than pinning it. + // + // The junction's SOURCE reference names the DECLARING BASE ("a::NodeBase") — the + // shape docs/features/relationships.md blesses, and the one whose short name + // collides with the target's. That matters: making isSelfJoin identity-based while + // the hetero TARGET search was still a bare compare let that search re-match this + // very reference (nothing excludes sourceRef from it, unlike the directed self-join + // branch) and return (srcId, srcId) silently. Both searches are identity-based now. + private const string XpkgAModel = """ + { "metadata.root": { "package": "a", "children": [ + { "object.entity": { "name": "Node", "extends": "a::NodeBase", "children": [ + { "field.long": { "name": "id" } }, + { "identity.primary": { "@fields": "id" } } + ]}}, + { "object.entity": { "name": "NodeBase", "@isAbstract": true, "children": [ + { "relationship.association": { "name": "links", "@cardinality": "many", "@objectRef": "b::NodeBase", "@through": "L" } } + ]}}, + { "object.entity": { "name": "L", "children": [ + { "field.long": { "name": "srcId" } }, + { "field.long": { "name": "dstId" } }, + { "identity.primary": { "@fields": ["srcId", "dstId"] } }, + { "identity.reference": { "name": "s", "@fields": "srcId", "@references": "a::NodeBase" } }, + { "identity.reference": { "name": "d", "@fields": "dstId", "@references": "b::NodeBase" } } + ]}} + ]}} + """; + + private const string XpkgBModel = """ + { "metadata.root": { "package": "b", "children": [ + { "object.entity": { "name": "NodeBase", "children": [ + { "field.long": { "name": "id" } }, + { "identity.primary": { "@fields": "id" } } + ]}} + ]}} + """; + + // Java's deriveCrossPackageHeteroBindsCorrectPackage model, ported. No inheritance + // is needed to reach the defect: `a::Account` relates to `b::Account` through a + // junction holding one reference to each. The two junction searches are + // INDEPENDENT and nothing excludes the source ref from the target search, so a + // bare-name target match found `ownerRef` a second time and returned + // (ownerId, ownerId) — silently. Pre-branch this threw loudly. + private const string XpkgHeteroA = """ + { "metadata.root": { "package": "a", "children": [ + { "object.entity": { "name": "Account", "children": [ + { "field.long": { "name": "id" } }, + { "relationship.association": { "name": "partners", "@cardinality": "many", "@objectRef": "b::Account", "@through": "AccountLink" } }, + { "identity.primary": { "@fields": "id" } } + ]}}, + { "object.entity": { "name": "AccountLink", "children": [ + { "field.long": { "name": "ownerId" } }, + { "field.long": { "name": "partnerId" } }, + { "identity.primary": { "@fields": ["ownerId", "partnerId"] } }, + { "identity.reference": { "name": "ownerRef", "@fields": "ownerId", "@references": "a::Account" } }, + { "identity.reference": { "name": "partnerRef", "@fields": "partnerId", "@references": "b::Account" } } + ]}} + ]}} + """; + + private const string XpkgHeteroB = """ + { "metadata.root": { "package": "b", "children": [ + { "object.entity": { "name": "Account", "children": [ + { "field.long": { "name": "id" } }, + { "identity.primary": { "@fields": "id" } } + ]}} + ]}} + """; + + [Fact] + public void Cross_package_hetero_matches_each_junction_reference_to_its_own_entity() + { + var r = new MetaDataLoader().Load([ + new InMemoryStringSource(XpkgHeteroA, id: "a.json"), + new InMemoryStringSource(XpkgHeteroB, id: "b.json"), + ]); + Assert.Empty(r.Errors); + var account = r.Root.Objects().First(o => o.ResolutionKey() == "a::Account"); + var fields = M2MDerivation.DeriveM2MFields(Rel(account, "partners"), account, r.Root); + // Was (ownerId, ownerId): the target search re-matched the SOURCE reference. + Assert.Equal("ownerId", fields.SourceField); + Assert.Equal("partnerId", fields.TargetField); + } + + [Fact] + public void Navigation_builder_agrees_with_the_derivation_on_a_cross_package_hetero() + { + // Asserted through M2MNavigationBuilder.For — the path DbContextGenerator, + // EntityGenerator, RoutesGenerator and CSharpApiModelBuilder actually take, and + // the one nothing covered. IsSelfJoin must agree with the derivation: a + // cross-package M:N whose target merely shares a short name with the source is + // HETERO, and calling it a self-join would skip its EF UsingEntity wiring and + // mark the navigation [NotMapped]. + var r = new MetaDataLoader().Load([ + new InMemoryStringSource(XpkgHeteroA, id: "a.json"), + new InMemoryStringSource(XpkgHeteroB, id: "b.json"), + ]); + Assert.Empty(r.Errors); + var account = r.Root.Objects().First(o => o.ResolutionKey() == "a::Account"); + + var nav = Assert.Single(M2MNavigationBuilder.For(account, r.Root)); + Assert.Equal("partners", nav.Name); + Assert.Equal("ownerId", nav.SourceField); + Assert.Equal("partnerId", nav.TargetField); + // The target is the OTHER package's Account, not this one. + Assert.Equal("b::Account", nav.Target.ResolutionKey()); + Assert.False(nav.IsSelfJoin); + } + + [Fact] + public void Cross_package_target_sharing_a_subject_short_name_is_not_a_self_join() + { + var r = new MetaDataLoader().Load([ + new InMemoryStringSource(XpkgAModel, id: "a.json"), + new InMemoryStringSource(XpkgBModel, id: "b.json"), + ]); + // The model itself is perfectly legal — the loader raises nothing. + Assert.Empty(r.Errors); + var node = r.Root.Objects().First(o => o.Name == "Node"); + var rel = Rel(node, "links"); + + // Identity resolution binds "b::NodeBase" to the b-package entity, which is + // neither subject — so this stays hetero and derives, exactly as Java does. + var fields = M2MDerivation.DeriveM2MFields(rel, node, r.Root); + Assert.Equal("srcId", fields.SourceField); + Assert.Equal("dstId", fields.TargetField); + } + + [Fact] + public void Navigation_builder_emits_the_inherited_self_join_and_flags_it() + { + var root = Load(SelfJoinModel); + var node = root.FindObject("Node")!; + + // Was: Build caught M2MDerivationException and returned null -> empty list. + var nav = Assert.Single(M2MNavigationBuilder.For(node, root)); + Assert.Equal("peers", nav.Name); + Assert.Equal("NodeLink", nav.Junction.Name); + Assert.Equal("aId", nav.SourceField); + Assert.Equal("bId", nav.TargetField); + Assert.True(nav.Symmetric); + + // IsSelfJoin must compare against the DECLARING entity too: Source is the + // inheriting "Node" while Target is "NodeBase". This is exactly the filter + // DbContextGenerator applies — a false here would emit EF UsingEntity wiring + // for a self-join, turning the old silent drop into silently wrong output. + Assert.Equal("Node", nav.Source.Name); + Assert.Equal("NodeBase", nav.Target.Name); + Assert.Equal("NodeBase", nav.DeclaringEntity.Name); + Assert.True(nav.IsSelfJoin); + Assert.DoesNotContain(M2MNavigationBuilder.For(node, root), n => !n.IsSelfJoin); + } + + [Fact] + public void Navigation_builder_emits_the_inherited_hetero_navigation() + { + var root = Load(HeteroBaseRefModel); + var article = root.FindObject("Article")!; + var nav = Assert.Single(M2MNavigationBuilder.For(article, root)); + Assert.Equal("tags", nav.Name); + Assert.Equal("Tag", nav.Target.Name); + Assert.Equal("articleId", nav.SourceField); + Assert.Equal("tagId", nav.TargetField); + Assert.False(nav.IsSelfJoin); + } +} diff --git a/server/csharp/MetaObjects.Codegen/Generators/M2MNavigation.cs b/server/csharp/MetaObjects.Codegen/Generators/M2MNavigation.cs index fee80330e..ed873807c 100644 --- a/server/csharp/MetaObjects.Codegen/Generators/M2MNavigation.cs +++ b/server/csharp/MetaObjects.Codegen/Generators/M2MNavigation.cs @@ -38,9 +38,34 @@ public sealed record M2MNavigation( /// The navigation member name (the relationship name, e.g. "tags"). public string Name => Relationship.Name; - /// True when the target entity is the source entity (self-join). - public bool IsSelfJoin => ReferenceEquals(Source, Target) || - string.Equals(Source.Name, Target.Name, StringComparison.Ordinal); + /// + /// The entity that DECLARES the relationship. Under extends that is not + /// : the builder walks the RESOLVING Relationships(), so + /// is whichever entity INHERITED the relationship, while + /// Relationship.Parent is the one that declared it. + /// + public MetaObject DeclaringEntity => Relationship.Parent as MetaObject ?? Source; + + /// + /// True when the target entity is the relationship's subject (self-join) — matched + /// against BOTH the navigating and the + /// , the same pair M2MDerivation accepts. + /// + /// Compared by OBJECT IDENTITY, never by short name — the same predicate + /// M2MDerivation uses, so the descriptor cannot disagree with the FK + /// derivation that produced it. A short-name compare said "self-join" for a + /// cross-package M:N whose target merely shares a short name with the subject, + /// while the derivation correctly called it hetero. + /// + /// Load-bearing: DbContextGenerator excludes self-joins from the EF + /// UsingEntity wiring and EntityGenerator marks their navigation + /// [NotMapped] (it is route-traversed). Before the derivation fix an + /// inherited self-join threw and the whole navigation was dropped, so this never + /// ran on one; getting it wrong now turns that silent drop into silently WRONG + /// EF configuration. + /// + public bool IsSelfJoin => + ReferenceEquals(Source, Target) || ReferenceEquals(DeclaringEntity, Target); } /// @@ -51,10 +76,13 @@ public sealed record M2MNavigation( public static class M2MNavigationBuilder { /// - /// All M:N navigations declared on (own relationships - /// with @cardinality: "many" + @through). Returns an empty list for - /// an entity with no M:N relationships. A relationship whose junction FK columns - /// cannot be derived is skipped (the loader validation surfaces the error). + /// All M:N navigations VISIBLE on — every relationship in + /// its EFFECTIVE view (Relationships() is resolving: own + inherited via + /// extends) carrying @cardinality: "many" + @through. NOT own-only: + /// believing otherwise is what produced the declaring-vs-visiting bug this class's + /// now guards. Returns an empty list for an + /// entity with no M:N relationships. A relationship whose junction FK columns cannot be + /// derived is skipped (the loader validation surfaces the error). /// public static IReadOnlyList For(MetaObject entity, MetaRoot root) { @@ -70,8 +98,16 @@ public static IReadOnlyList For(MetaObject entity, MetaRoot root) private static M2MNavigation? Build(MetaObject source, MetaRelationship rel, MetaRoot root) { if (rel.ObjectRef is not { } targetRef || rel.Through is not { } throughRef) return null; - var target = root.FindObject(CSharpNaming.StripPkg(targetRef)); - var junction = root.FindObject(CSharpNaming.StripPkg(throughRef)); + // Resolve by the SAME rule the derivation uses (FQN-exact when qualified, bare + // short name otherwise), or IsSelfJoin below can disagree with the FK derivation: + // StripPkg + FindObject binds "b::Account" to a same-short-named `a::Account`, + // which then compares identity-equal to the source and reports a cross-package + // hetero M:N as a self-join. Falls back to the previous resolution when the + // qualified name does not resolve exactly, so nothing that bound before stops. + var target = M2MDerivation.ResolveEntity(root, targetRef) + ?? root.FindObject(CSharpNaming.StripPkg(targetRef)); + var junction = M2MDerivation.ResolveEntity(root, throughRef) + ?? root.FindObject(CSharpNaming.StripPkg(throughRef)); if (target is null || junction is null) return null; M2MFields fields; diff --git a/server/csharp/MetaObjects/Core/Relationship/M2MFields.cs b/server/csharp/MetaObjects/Core/Relationship/M2MFields.cs index 00e92baaa..789a207f4 100644 --- a/server/csharp/MetaObjects/Core/Relationship/M2MFields.cs +++ b/server/csharp/MetaObjects/Core/Relationship/M2MFields.cs @@ -19,6 +19,26 @@ // two references are taken in declaration order (sourceField = first, // targetField = second). Resolution unions both at read time. // Ambiguous (source == target, neither @sourceRefField nor @symmetric) -> throw. +// +// "source" above means the relationship's SUBJECT, and under `extends` there are two +// legitimate names for it. Every caller walks the RESOLVING Relationships() +// (M2MNavigationBuilder for codegen, M2MResolver at run time) and passes the entity it +// is ITERATING, which for an inherited relationship is not the one that declared it. +// So the DECLARING entity is resolved here from rel.Parent (same shape as the #368 +// loader fix, ValidationPasses' `declaringEntity = rel.Parent ?? obj`), and the passed +// `source` is kept alongside it rather than discarded: a junction FK usually references +// the CONCRETE entity, because the abstract base has no table, while @objectRef on an +// inherited self-join names the base. Both are accepted, for the self-join +// classification and the hetero reference match alike. Not covered: a junction +// reference naming an entity strictly BETWEEN the base and the navigating entity in a +// deeper hierarchy. +// +// The subject comparison is made on RESOLVED OBJECT IDENTITY, not on stripped short +// names, matching the Java reference (M2MFields.java). Bare-name equality cannot tell +// `a::NodeBase` from `b::NodeBase`, so once the subject set held two names a genuine +// CROSS-PACKAGE hetero M:N whose target shares a short name with the subject read as a +// self-join and refused to derive. Scoped deliberately to this one predicate — every +// other name comparison in this file is untouched. using MetaObjects.Meta; @@ -58,62 +78,138 @@ private static string StripPackage(string name) return idx < 0 ? name : name[(idx + PACKAGE_SEPARATOR.Length)..]; } + /// + /// , exposed so a codegen DESCRIPTOR can resolve an entity + /// reference by the SAME rule this derivation uses. + /// + /// + /// M2MNavigation.IsSelfJoin compares the descriptor's target against + /// its source by identity; while M2MNavigationBuilder.Build resolved that + /// target by a package-stripped name and the derivation resolved it exactly, the two + /// disagreed — b::Account bound to a same-short-named a::Account, which + /// then compared identity-equal to the source and reported a cross-package hetero M:N + /// as a self-join, and the EF wiring follows the descriptor. Build now calls this. + /// So this is additive to the DERIVATION — no derivation behaviour changed — but it + /// deliberately DID change resolution at that one call site, which is the point. + /// NOT the port's general reference resolver. It is deliberately narrow (see + /// ) and has no referrer-package awareness: a bare name is + /// resolved against every root object, not against the referrer's package first. + /// NamingRefs.ResolveObjectRef is the package-aware resolver the loader uses; + /// prefer it anywhere that is not matching a junction reference to an entity, and see + /// issue #174 for the bare-collision case both leave open. + /// + public static MetaObject? ResolveEntity(MetaRoot root, string? name) => FindEntity(root, name); + + /// + /// The root entity a reference name denotes, or null. Mirrors the Java + /// reference's M2MFields.findObject exactly: a FULLY-QUALIFIED name (one + /// containing ::) resolves EXACTLY on the object's package-folded key, never + /// a bare-tail fallback; a bare name matches a short name, first match wins (the + /// bare-collision case is the deferred follow-up Java records as issue #174). + /// + /// This exists so the junction matches can be made on object IDENTITY the way Java's + /// already are. A bare-name compare cannot tell a::NodeBase from + /// b::NodeBase, which made a genuine cross-package hetero M:N read as a + /// self-join the moment the subject set held two names — and, once the subject side + /// alone was fixed, made the target search re-match the source-side reference. + /// + private static MetaObject? FindEntity(MetaRoot root, string? name) + { + if (string.IsNullOrEmpty(name)) return null; + var objects = root.Objects(); + if (name.Contains(PACKAGE_SEPARATOR, StringComparison.Ordinal)) + return objects.FirstOrDefault(o => string.Equals(o.ResolutionKey(), name, StringComparison.Ordinal)); + var bare = StripPackage(name); + return objects.FirstOrDefault(o => string.Equals(o.Name, bare, StringComparison.Ordinal)); + } + /// /// Derive the source/target junction FK fields for a M:N relationship. /// /// the M:N relationship (carries @objectRef + @through + optional @sourceRefField / @symmetric). - /// the entity declaring . + /// the entity the caller is navigating from. Accepted alongside + /// rel.Parent as a name for the relationship's subject, and used as the + /// declaring entity when has no parent. /// the loaded model root (to find the junction entity). /// /// when the junction is missing/malformed or the self-join is ambiguous. /// public static M2MFields DeriveM2MFields(MetaRelationship rel, MetaObject source, MetaRoot root) { + // The entity that DECLARES `rel` — see the header note. Parent is the owning + // entity for both an own declaration and an inherited one (an unmodified + // inherited child is the SAME node object; an override is a different node + // whose parent is the overriding entity, also correct). + var declaring = rel.Parent as MetaObject ?? source; + // The relationship's subject: either name is valid (see the header note). + var subjectNames = declaring.Name == source.Name + ? new[] { declaring.Name } + : new[] { declaring.Name, source.Name }; + var subjectLabel = string.Join(" or ", subjectNames.Select(n => $"\"{n}\"")); + // Compared by resolved object IDENTITY, matching the Java reference. A + // StripPackage compare cannot distinguish `a::NodeBase` from `b::NodeBase`, so + // with two names in the set a genuine cross-package hetero M:N read as a + // self-join and refused to derive. + bool IsSubject(MetaObject? entity) => + entity is not null && (ReferenceEquals(entity, declaring) || ReferenceEquals(entity, source)); + bool IsSubjectName(string? name) => name is not null && subjectNames.Contains(StripPackage(name)); + string? throughName = rel.Through; if (throughName is null) { throw new M2MDerivationException( - $"relationship \"{source.Name}.{rel.Name}\" is missing @through (required for M:N derivation)"); + $"relationship \"{declaring.Name}.{rel.Name}\" is missing @through (required for M:N derivation)"); } var junction = root.FindObject(throughName); if (junction is null) { throw new M2MDerivationException( - $"relationship \"{source.Name}.{rel.Name}\" @through \"{throughName}\" does not resolve to an entity"); + $"relationship \"{declaring.Name}.{rel.Name}\" @through \"{throughName}\" does not resolve to an entity"); } string? targetName = rel.ObjectRef; if (targetName is null) { throw new M2MDerivationException( - $"relationship \"{source.Name}.{rel.Name}\" is missing @objectRef (the M:N target)"); + $"relationship \"{declaring.Name}.{rel.Name}\" is missing @objectRef (the M:N target)"); } var refs = junction.ReferenceIdentities(); if (refs.Count != 2) { throw new M2MDerivationException( - $"junction \"{throughName}\" for relationship \"{source.Name}.{rel.Name}\" must declare exactly two " + + $"junction \"{throughName}\" for relationship \"{declaring.Name}.{rel.Name}\" must declare exactly two " + $"identity.reference children (found {refs.Count})"); } - bool isSelfJoin = StripPackage(targetName) == source.Name; + // Defensive bare fallback when @objectRef does not resolve — loader validation + // normally guarantees it does. Same carve-out the Java reference makes. + var targetEntityNode = FindEntity(root, targetName); + bool isSelfJoin = targetEntityNode is not null + ? IsSubject(targetEntityNode) + : IsSubjectName(targetName); if (!isSelfJoin) { - // Hetero: match each reference by the entity it resolves to. - var sourceRef = refs.FirstOrDefault( - r => r.TargetEntity is not null && StripPackage(r.TargetEntity) == source.Name); - var targetRef = refs.FirstOrDefault( - r => r.TargetEntity is not null && StripPackage(r.TargetEntity) == StripPackage(targetName)); + // Hetero: match each reference by the ENTITY OBJECT it resolves to. + var sourceRef = refs.FirstOrDefault(r => IsSubject(FindEntity(root, r.TargetEntity))); + // Identity here too. The two searches are INDEPENDENT — nothing excludes + // sourceRef from this one, unlike the directed self-join branch below — so a + // bare compare could match the SOURCE-side reference again whenever the + // target's short name equals the source's, and silently return (srcFk, srcFk). + // Java matches identity on both sides (findRefToSubject + findRefToObject). + var targetRef = targetEntityNode is not null + ? refs.FirstOrDefault(r => ReferenceEquals(FindEntity(root, r.TargetEntity), targetEntityNode)) + : refs.FirstOrDefault( + r => r.TargetEntity is not null && StripPackage(r.TargetEntity) == StripPackage(targetName)); var sourceField = sourceRef is not null ? RefFkField(sourceRef) : null; var targetField = targetRef is not null ? RefFkField(targetRef) : null; if (sourceField is null || targetField is null) { throw new M2MDerivationException( - $"junction \"{throughName}\" for relationship \"{source.Name}.{rel.Name}\" must declare one " + - $"identity.reference to \"{source.Name}\" and one to \"{StripPackage(targetName)}\""); + $"junction \"{throughName}\" for relationship \"{declaring.Name}.{rel.Name}\" must declare one " + + $"identity.reference to {subjectLabel} and one to \"{StripPackage(targetName)}\""); } return new M2MFields(sourceField, targetField); } @@ -127,7 +223,7 @@ public static M2MFields DeriveM2MFields(MetaRelationship rel, MetaObject source, if (a is null || b is null) { throw new M2MDerivationException( - $"symmetric junction \"{throughName}\" for \"{source.Name}.{rel.Name}\" has a reference with no @fields"); + $"symmetric junction \"{throughName}\" for \"{declaring.Name}.{rel.Name}\" has a reference with no @fields"); } return new M2MFields(a, b); } @@ -136,7 +232,7 @@ public static M2MFields DeriveM2MFields(MetaRelationship rel, MetaObject source, if (sourceRefField is null) { throw new M2MDerivationException( - $"self-join relationship \"{source.Name}.{rel.Name}\" through \"{throughName}\" is ambiguous: " + + $"self-join relationship \"{declaring.Name}.{rel.Name}\" through \"{throughName}\" is ambiguous: " + "set @sourceRefField (directed) or @symmetric (undirected)"); } @@ -145,7 +241,7 @@ public static M2MFields DeriveM2MFields(MetaRelationship rel, MetaObject source, if (directedSourceRef is null) { throw new M2MDerivationException( - $"@sourceRefField \"{sourceRefField}\" on \"{source.Name}.{rel.Name}\" does not match any " + + $"@sourceRefField \"{sourceRefField}\" on \"{declaring.Name}.{rel.Name}\" does not match any " + $"identity.reference FK field on junction \"{throughName}\""); } var directedTargetRef = refs.FirstOrDefault(r => !ReferenceEquals(r, directedSourceRef)); @@ -153,7 +249,7 @@ public static M2MFields DeriveM2MFields(MetaRelationship rel, MetaObject source, if (directedTargetField is null) { throw new M2MDerivationException( - $"junction \"{throughName}\" for \"{source.Name}.{rel.Name}\" has no distinct target-side reference"); + $"junction \"{throughName}\" for \"{declaring.Name}.{rel.Name}\" has no distinct target-side reference"); } return new M2MFields(sourceRefField, directedTargetField); } diff --git a/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinM2mCodegenTest.kt b/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinM2mCodegenTest.kt index 9487bdab7..6607cc6ce 100644 --- a/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinM2mCodegenTest.kt +++ b/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinM2mCodegenTest.kt @@ -1,8 +1,10 @@ package com.metaobjects.generator.kotlin +import com.metaobjects.`object`.MetaObject import com.metaobjects.metadata.ktx.loadString import java.nio.file.Files import kotlin.test.Test +import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertTrue @@ -174,4 +176,57 @@ class KotlinM2mCodegenTest { assertFalse("/{id}/" in src && "Query(id)" in src, "junction controller must not emit an M:N traversal endpoint; saw:\n$src") } + + // --- Inherited M:N — the declaring-entity follow-up to #368 --------------------- + // + // codegen-kotlin reaches the SAME cross-port SSOT (com.metaobjects.relationship + // .M2MFields.derive) as codegen-spring and omdb, so it carried the same defect: + // KotlinM2mSupport.resolve walks the RESOLVING `entity.relationships` and passed + // the entity it was iterating, so a M:N declared on an abstract base and reached + // through a concrete child was derived against the CHILD. An inherited self-join + // then read as hetero and threw — and resolve() does NOT catch, so the whole + // Kotlin generation run failed rather than silently dropping the navigation. + // + // This test pins that codegen-kotlin genuinely reaches the fixed helper; the + // derivation logic itself is gated by the Java M2MSlimVocabularyTest. The child + // is declared BEFORE the base, the #368 order convention. + private val inheritedSelfJoinFixture = """{ + "metadata.root": { "package": "acme::graph", "children": [ + { "object.entity": { "name": "Node", "extends": "NodeBase", "children": [ + { "source.rdb": { "@table": "nodes" } }, + { "field.long": { "name": "id" } }, + { "field.string": { "name": "label", "@required": true, "@maxLength": 80 } }, + { "identity.primary": { "@fields": "id", "@generation": "increment" } } + ] } }, + { "object.entity": { "name": "NodeBase", "@isAbstract": true, "children": [ + { "relationship.association": { "name": "peers", "@cardinality": "many", + "@objectRef": "NodeBase", "@through": "NodeLink", "@symmetric": true } } + ] } }, + { "object.entity": { "name": "NodeLink", "children": [ + { "source.rdb": { "@table": "node_links" } }, + { "field.long": { "name": "aId", "@required": true } }, + { "field.long": { "name": "bId", "@required": true } }, + { "identity.primary": { "@fields": ["aId", "bId"] } }, + { "identity.reference": { "name": "fkA", "@fields": "aId", "@references": "NodeBase" } }, + { "identity.reference": { "name": "fkB", "@fields": "bId", "@references": "NodeBase" } } + ] } } + ] } + }""".trimIndent() + + @Test fun inheritedSelfJoinResolvesThroughTheSharedDerivation() { + val loader = loadString("km2m-inherited", inheritedSelfJoinFixture) + val node = loader.root.getChildren(MetaObject::class.java, false) + .first { it.name == "acme::graph::Node" } + // Pin the premise: the child is reached before the base it inherits from. + assertEquals( + listOf("acme::graph::Node", "acme::graph::NodeBase", "acme::graph::NodeLink"), + loader.root.getChildren(MetaObject::class.java, false).map { it.name }, + ) + val navs = KotlinM2mSupport.resolve(node, loader) + assertEquals(1, navs.size, "expected the inherited peers navigation; saw $navs") + assertEquals("peers", navs[0].relationName) + assertEquals("aId", navs[0].sourceField) + assertEquals("bId", navs[0].targetField) + assertTrue(navs[0].symmetric) + } } diff --git a/server/java/metadata/src/main/java/com/metaobjects/relationship/M2MFields.java b/server/java/metadata/src/main/java/com/metaobjects/relationship/M2MFields.java index 16ef5aee0..0ec2a7d7f 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/relationship/M2MFields.java +++ b/server/java/metadata/src/main/java/com/metaobjects/relationship/M2MFields.java @@ -46,6 +46,20 @@ * * Ambiguous (source == target, neither {@code @sourceRefField} nor {@code @symmetric}) → throw. * + *

"source" above means the relationship's SUBJECT, and under {@code extends} there are + * two legitimate names for it. Every caller walks the RESOLVING {@code getRelationships()} + * (SpringM2mSupport and KotlinM2mSupport for codegen, omdb's M2MResolver at run time) and + * passes the entity it is ITERATING, which for an inherited relationship is not the one + * that declared it. So the DECLARING entity is resolved here from {@code rel.getParent()} + * (same shape as the #368 loader fix in {@code ValidationPhase}), and the passed + * {@code source} is kept alongside it rather than discarded: a junction FK usually + * references the CONCRETE entity, because the abstract base has no table, while + * {@code @objectRef} on an inherited self-join names the base. Both are accepted, for the + * self-join classification and the hetero reference match alike. {@code source} is also the + * fallback when {@code rel} has no entity parent, which keeps the signature unchanged. Not + * covered: a junction reference naming an entity strictly BETWEEN the base and the + * navigating entity in a deeper hierarchy.

+ * *

This carries the same semantics as the loader-phase M:N validation * ({@code ValidationPhase.validateRelationshipsM2M}); the validation pass guarantees a * well-formed junction (exactly two references, matching {@code @sourceRefField}) before @@ -83,30 +97,39 @@ public M2MDerivationException(String message) { * * @param rel the M:N relationship (carries {@code @objectRef} + {@code @through} * + optional {@code @sourceRefField} / {@code @symmetric}) - * @param source the entity declaring {@code rel} + * @param source the entity the caller is navigating from. Accepted alongside + * {@code rel.getParent()} as a name for the relationship's subject, + * and used as the declaring entity when {@code rel} has no parent * @param root the loaded model root (to find the junction entity) * @return the derived source/target junction FK fields * @throws M2MDerivationException when the junction is missing/malformed or the * self-join is ambiguous. */ public static M2MFields derive(MetaRelationship rel, MetaObject source, MetaRoot root) { + // The entity that DECLARES `rel` — see the class note. getParent() is the + // owning entity for both an own declaration and an inherited one (an + // unmodified inherited child is the SAME node object; an override is a + // different node whose parent is the overriding entity, also correct). + MetaData relParent = rel.getParent(); + MetaObject declaring = (relParent instanceof MetaObject) ? (MetaObject) relParent : source; + String throughName = rel.getThrough(); if (throughName == null || throughName.isEmpty()) { throw new M2MDerivationException( - "relationship \"" + source.getShortName() + "." + rel.getShortName() + "relationship \"" + declaring.getShortName() + "." + rel.getShortName() + "\" is missing @through (required for M:N derivation)"); } MetaObject junction = findObject(root, throughName); if (junction == null) { throw new M2MDerivationException( - "relationship \"" + source.getShortName() + "." + rel.getShortName() + "relationship \"" + declaring.getShortName() + "." + rel.getShortName() + "\" @through \"" + throughName + "\" does not resolve to an entity"); } String targetName = rel.getObjectRef(); if (targetName == null || targetName.isEmpty()) { throw new M2MDerivationException( - "relationship \"" + source.getShortName() + "." + rel.getShortName() + "relationship \"" + declaring.getShortName() + "." + rel.getShortName() + "\" is missing @objectRef (the M:N target)"); } MetaObject target = findObject(root, targetName); @@ -114,34 +137,49 @@ public static M2MFields derive(MetaRelationship rel, MetaObject source, MetaRoot List refs = referenceIdentities(junction); if (refs.size() != 2) { throw new M2MDerivationException( - "junction \"" + throughName + "\" for relationship \"" + source.getShortName() + "junction \"" + throughName + "\" for relationship \"" + declaring.getShortName() + "." + rel.getShortName() + "\" must declare exactly two" + " identity.reference children (found " + refs.size() + ")"); } - // ADR-0041: classify the self-join by RESOLVED object identity (FQN-exact), - // never a stripped bare tail. A cross-package hetero M:N whose target shares - // a bare name with the source (or an unrelated entity) must NOT be mis-read - // as a self-join, and an FQN @objectRef must bind the correct package. Falls - // back to a bare-name compare only when @objectRef is unresolvable (defensive - // — loader validation normally guarantees resolution before derive runs). + // The relationship's SUBJECT — the entity the M:N hangs off. Under `extends` + // there are two legitimate names for it and BOTH occur in real models: the + // DECLARING entity (what @objectRef names for a self-join declared on an + // abstract base, and what a junction reference names when the FK points at + // the base type), and the NAVIGATING entity (`source`, the concrete entity + // the caller is iterating — usually what a junction FK references, because + // that is the entity with the physical table). Accepting either is what + // makes derivation independent of which entity's effective view reached the + // relationship. Not covered: a junction reference naming an entity strictly + // BETWEEN the base and the navigating entity in a deeper hierarchy. + // + // ADR-0041: classify by RESOLVED object identity (FQN-exact), never a + // stripped bare tail. A cross-package hetero M:N whose target shares a bare + // name with the subject must NOT be mis-read as a self-join, and an FQN + // @objectRef must bind the correct package. Falls back to a bare-name + // compare only when @objectRef is unresolvable (defensive — loader + // validation normally guarantees resolution before derive runs). boolean isSelfJoin = (target != null) - ? target.getName().equals(source.getName()) - : stripPackage(targetName).equals(source.getShortName()); + ? isSubject(target, declaring, source) + : (stripPackage(targetName).equals(declaring.getShortName()) + || stripPackage(targetName).equals(source.getShortName())); if (!isSelfJoin) { // Hetero: match each reference by the ENTITY OBJECT its @references // resolves to (FQN-exact), so a same-bare-name cross-package reference // binds the correct package rather than the first bare-tail match. - MetaIdentity sourceRef = findRefToObject(root, refs, source); + MetaIdentity sourceRef = findRefToSubject(root, refs, declaring, source); MetaIdentity targetRef = findRefToObject(root, refs, target); String sourceField = sourceRef != null ? refFkField(sourceRef) : null; String targetField = targetRef != null ? refFkField(targetRef) : null; if (sourceField == null || targetField == null) { + String subjectLabel = declaring.getName().equals(source.getName()) + ? "\"" + declaring.getShortName() + "\"" + : "\"" + declaring.getShortName() + "\" or \"" + source.getShortName() + "\""; throw new M2MDerivationException( - "junction \"" + throughName + "\" for relationship \"" + source.getShortName() - + "." + rel.getShortName() + "\" must declare one identity.reference to \"" - + source.getShortName() + "\" and one to \"" + stripPackage(targetName) + "\""); + "junction \"" + throughName + "\" for relationship \"" + declaring.getShortName() + + "." + rel.getShortName() + "\" must declare one identity.reference to " + + subjectLabel + " and one to \"" + stripPackage(targetName) + "\""); } return new M2MFields(sourceField, targetField); } @@ -153,7 +191,7 @@ public static M2MFields derive(MetaRelationship rel, MetaObject source, MetaRoot String b = refFkField(refs.get(1)); if (a == null || b == null) { throw new M2MDerivationException( - "symmetric junction \"" + throughName + "\" for \"" + source.getShortName() + "symmetric junction \"" + throughName + "\" for \"" + declaring.getShortName() + "." + rel.getShortName() + "\" has a reference with no @fields"); } return new M2MFields(a, b); @@ -162,7 +200,7 @@ public static M2MFields derive(MetaRelationship rel, MetaObject source, MetaRoot String sourceRefField = rel.getSourceRefField(); if (sourceRefField == null || sourceRefField.isEmpty()) { throw new M2MDerivationException( - "self-join relationship \"" + source.getShortName() + "." + rel.getShortName() + "self-join relationship \"" + declaring.getShortName() + "." + rel.getShortName() + "\" through \"" + throughName + "\" is ambiguous: set @sourceRefField" + " (directed) or @symmetric (undirected)"); } @@ -177,7 +215,7 @@ public static M2MFields derive(MetaRelationship rel, MetaObject source, MetaRoot } if (sourceRef == null) { throw new M2MDerivationException( - "@sourceRefField \"" + sourceRefField + "\" on \"" + source.getShortName() + "." + "@sourceRefField \"" + sourceRefField + "\" on \"" + declaring.getShortName() + "." + rel.getShortName() + "\" does not match any identity.reference FK field on" + " junction \"" + throughName + "\""); } @@ -190,7 +228,7 @@ public static M2MFields derive(MetaRelationship rel, MetaObject source, MetaRoot } if (targetField == null) { throw new M2MDerivationException( - "junction \"" + throughName + "\" for \"" + source.getShortName() + "." + "junction \"" + throughName + "\" for \"" + declaring.getShortName() + "." + rel.getShortName() + "\" has no distinct target-side reference"); } return new M2MFields(sourceRefField, targetField); @@ -249,6 +287,26 @@ private static MetaObject refTargetObject(MetaRoot root, MetaIdentity ref) { * bare tail — two junction references to same-bare-name entities in different * packages must be distinguished by their full package-qualified name. */ + /** + * True when {@code candidate} is one of the relationship's SUBJECT entities — + * the declaring entity or the entity the caller is navigating from. Compared by + * package-qualified name (ADR-0041 identity), never a bare tail. + */ + private static boolean isSubject(MetaObject candidate, MetaObject declaring, MetaObject source) { + if (candidate == null) return false; + return candidate.getName().equals(declaring.getName()) + || candidate.getName().equals(source.getName()); + } + + /** The junction reference resolving to either subject entity, or {@code null}. */ + private static MetaIdentity findRefToSubject(MetaRoot root, List refs, + MetaObject declaring, MetaObject source) { + for (MetaIdentity ref : refs) { + if (isSubject(refTargetObject(root, ref), declaring, source)) return ref; + } + return null; + } + private static MetaIdentity findRefToObject(MetaRoot root, List refs, MetaObject entity) { if (entity == null) return null; for (MetaIdentity ref : refs) { diff --git a/server/java/metadata/src/test/java/com/metaobjects/relationship/M2MSlimVocabularyTest.java b/server/java/metadata/src/test/java/com/metaobjects/relationship/M2MSlimVocabularyTest.java index ff42f134b..229f9f6a1 100644 --- a/server/java/metadata/src/test/java/com/metaobjects/relationship/M2MSlimVocabularyTest.java +++ b/server/java/metadata/src/test/java/com/metaobjects/relationship/M2MSlimVocabularyTest.java @@ -403,6 +403,15 @@ private static MetaRelationship relOf(MetaObject obj, String relName) { + " { \"identity.reference\": { \"name\": \"partnerRef\", \"@fields\": \"partnerId\", \"@references\": \"xpkg::partner::Account\" } } ] } }" + "] } }"; + // CROSS-PORT NOTE: this port's identity-based subject comparison is the reference the + // other three were brought into line with. TS, C# and Python used to compare stripped + // short names, so the shape below — a cross-package hetero target whose bare name + // collides with the subject's — misread as an ambiguous self-join there once the + // subject set held two names (declaring + navigating entity). All four now resolve + // the name to an ENTITY and compare identity, and each of the three carries the + // matching regression test (relationship-m2m.test.ts, + // M2MInheritedDeclaringEntityTests.cs, test_derive_m2m_declaring_entity.py). Keep + // this test and those four in step. @Test public void deriveCrossPackageHeteroBindsCorrectPackage() { // ADR-0041: same-bare-name entities/junctions in different packages. Under the @@ -465,4 +474,133 @@ public void deriveCrossPackageJunctionCollisionBindsCorrectPackage() { assertEquals("postId", f.getSourceField()); assertEquals("tagId", f.getTargetField()); } + + // --- 6. Declaring entity vs. visiting entity (the #368 follow-up) ------------------ + // + // Same confusion as the #368 loader passes, one layer down: derive() classified the + // self-join, and matched the hetero junction reference, against the `source` entity + // its CALLER passed. Every caller walks the RESOLVING getRelationships() + // (SpringM2mSupport, KotlinM2mSupport, omdb's M2MResolver) and passes the entity it + // is iterating — so for a relationship inherited via `extends` that is the + // INHERITING entity, not the one that declared it. + // + // Unlike the loader passes there is no once-per-node `checked` set here, so the + // defect is NOT gated on visit order — it is wrong for every inheriting entity in + // any order. The fixtures still declare the child BEFORE the base and pin the root + // visit order, matching the #368 convention. + + /** Node extends NodeBase, which declares a @symmetric self-join onto ITSELF. Node + * is declared FIRST so the root's child order is child-before-base. */ + private static final String INHERITED_SELF_JOIN = + "{ \"metadata.root\": { \"package\": \"acme\", \"children\": [" + + " { \"object.entity\": { \"name\": \"Node\", \"extends\": \"NodeBase\", \"children\": [" + + " { \"field.long\": { \"name\": \"id\" } }," + + " { \"identity.primary\": { \"@fields\": \"id\" } } ] } }," + + " { \"object.entity\": { \"name\": \"NodeBase\", \"@isAbstract\": true, \"children\": [" + + " { \"relationship.association\": { \"name\": \"peers\", \"@cardinality\": \"many\"," + + " \"@objectRef\": \"NodeBase\", \"@through\": \"NodeLink\", \"@symmetric\": true } } ] } }," + + " { \"object.entity\": { \"name\": \"NodeLink\", \"children\": [" + + " { \"field.long\": { \"name\": \"id\" } }," + + " { \"field.long\": { \"name\": \"aId\" } }," + + " { \"field.long\": { \"name\": \"bId\" } }," + + " { \"identity.primary\": { \"@fields\": \"id\" } }," + + " { \"identity.reference\": { \"name\": \"aRef\", \"@fields\": \"aId\", \"@references\": \"NodeBase\" } }," + + " { \"identity.reference\": { \"name\": \"bRef\", \"@fields\": \"bId\", \"@references\": \"NodeBase\" } } ] } }" + + "] } }"; + + /** Article extends ArticleBase, which declares a HETERO M:N whose junction references + * the BASE — so a match against the visiting child finds nothing. */ + private static final String INHERITED_HETERO = + "{ \"metadata.root\": { \"package\": \"acme\", \"children\": [" + + " { \"object.entity\": { \"name\": \"Article\", \"extends\": \"ArticleBase\", \"children\": [" + + " { \"field.long\": { \"name\": \"id\" } }," + + " { \"identity.primary\": { \"@fields\": \"id\" } } ] } }," + + " { \"object.entity\": { \"name\": \"ArticleBase\", \"@isAbstract\": true, \"children\": [" + + " { \"relationship.association\": { \"name\": \"tags\", \"@cardinality\": \"many\"," + + " \"@objectRef\": \"Tag\", \"@through\": \"ArticleTag\" } } ] } }," + + " { \"object.entity\": { \"name\": \"Tag\", \"children\": [" + + " { \"field.long\": { \"name\": \"id\" } }," + + " { \"identity.primary\": { \"@fields\": \"id\" } } ] } }," + + " { \"object.entity\": { \"name\": \"ArticleTag\", \"children\": [" + + " { \"field.long\": { \"name\": \"id\" } }," + + " { \"field.long\": { \"name\": \"articleId\" } }," + + " { \"field.long\": { \"name\": \"tagId\" } }," + + " { \"identity.primary\": { \"@fields\": \"id\" } }," + + " { \"identity.reference\": { \"name\": \"aRef\", \"@fields\": \"articleId\", \"@references\": \"ArticleBase\" } }," + + " { \"identity.reference\": { \"name\": \"tRef\", \"@fields\": \"tagId\", \"@references\": \"Tag\" } } ] } }" + + "] } }"; + + /** Root-level object order, so a test's child-before-base premise fails loudly if + * the loader ever stops preserving declaration order. */ + private static List objectOrder(MetaDataLoader loader) { + List out = new java.util.ArrayList<>(); + for (MetaObject mo : loader.getRoot().getChildren(MetaObject.class, false)) out.add(mo.getName()); + return out; + } + + /** The OTHER legitimate shape, and the common one: the base is abstract (no table), + * so the junction FK references the CONCRETE child. Both names of the subject must + * be accepted, or fixing the base-referencing shape breaks this one. */ + private static final String INHERITED_HETERO_CONCRETE_REF = + "{ \"metadata.root\": { \"package\": \"acme\", \"children\": [" + + " { \"object.entity\": { \"name\": \"Post\", \"extends\": \"PostBase\", \"children\": [" + + " { \"field.long\": { \"name\": \"id\" } }," + + " { \"identity.primary\": { \"@fields\": \"id\" } } ] } }," + + " { \"object.entity\": { \"name\": \"PostBase\", \"@isAbstract\": true, \"children\": [" + + " { \"relationship.association\": { \"name\": \"tags\", \"@cardinality\": \"many\"," + + " \"@objectRef\": \"Tag\", \"@through\": \"PostTag\" } } ] } }," + + " { \"object.entity\": { \"name\": \"Tag\", \"children\": [" + + " { \"field.long\": { \"name\": \"id\" } }," + + " { \"identity.primary\": { \"@fields\": \"id\" } } ] } }," + + " { \"object.entity\": { \"name\": \"PostTag\", \"children\": [" + + " { \"field.long\": { \"name\": \"id\" } }," + + " { \"field.long\": { \"name\": \"postId\" } }," + + " { \"field.long\": { \"name\": \"tagId\" } }," + + " { \"identity.primary\": { \"@fields\": \"id\" } }," + + " { \"identity.reference\": { \"name\": \"pRef\", \"@fields\": \"postId\", \"@references\": \"Post\" } }," + + " { \"identity.reference\": { \"name\": \"tRef\", \"@fields\": \"tagId\", \"@references\": \"Tag\" } } ] } }" + + "] } }"; + + @Test + public void deriveInheritedHeteroWithConcreteJunctionReference() { + MetaDataLoader loader = loadThrough(INHERITED_HETERO_CONCRETE_REF, "inherited-hetero-concrete.json"); + MetaObject post = objExact(loader, "acme::Post"); + MetaRelationship rel = relOf(post, "tags"); + assertEquals("acme::PostBase", ((MetaObject) rel.getParent()).getName()); + M2MFields f = M2MFields.derive(rel, post, loader.getRoot()); + assertEquals("postId", f.getSourceField()); + assertEquals("tagId", f.getTargetField()); + } + + @Test + public void deriveInheritedSelfJoinUsesDeclaringEntity() { + MetaDataLoader loader = loadThrough(INHERITED_SELF_JOIN, "inherited-self-join.json"); + assertEquals(List.of("acme::Node", "acme::NodeBase", "acme::NodeLink"), objectOrder(loader)); + MetaObject node = objExact(loader, "acme::Node"); + MetaRelationship rel = relOf(node, "peers"); + // The relationship is INHERITED: it is not one of Node's own children. + assertTrue(node.getRelationships(false).isEmpty()); + assertEquals("acme::NodeBase", ((MetaObject) rel.getParent()).getName()); + // Pre-fix: @objectRef "NodeBase" vs the visiting "acme::Node" => not a self-join + // => hetero branch => no junction reference to Node => M2MDerivationException. + M2MFields f = M2MFields.derive(rel, node, loader.getRoot()); + assertEquals("aId", f.getSourceField()); + assertEquals("bId", f.getTargetField()); + // The declaring entity itself must agree — same node, same answer. + MetaObject base = objExact(loader, "acme::NodeBase"); + M2MFields viaBase = M2MFields.derive(rel, base, loader.getRoot()); + assertEquals(f.getSourceField(), viaBase.getSourceField()); + assertEquals(f.getTargetField(), viaBase.getTargetField()); + } + + @Test + public void deriveInheritedHeteroMatchesDeclaringEntityReference() { + MetaDataLoader loader = loadThrough(INHERITED_HETERO, "inherited-hetero.json"); + assertEquals(List.of("acme::Article", "acme::ArticleBase", "acme::Tag", "acme::ArticleTag"), + objectOrder(loader)); + MetaObject article = objExact(loader, "acme::Article"); + M2MFields f = M2MFields.derive(relOf(article, "tags"), article, loader.getRoot()); + assertEquals("articleId", f.getSourceField()); + assertEquals("tagId", f.getTargetField()); + } } diff --git a/server/python/src/metaobjects/meta/core/relationship/derive_m2m_fields.py b/server/python/src/metaobjects/meta/core/relationship/derive_m2m_fields.py index abc6dc062..58c1f55cb 100644 --- a/server/python/src/metaobjects/meta/core/relationship/derive_m2m_fields.py +++ b/server/python/src/metaobjects/meta/core/relationship/derive_m2m_fields.py @@ -16,13 +16,35 @@ two references are taken in declaration order (source_field = first, target_field = second). Resolution unions both at read time. Ambiguous (source == target, neither @sourceRefField nor @symmetric) → raise. + +"source" above means the relationship's SUBJECT, and under ``extends`` there are two +legitimate names for it. Every caller walks a RESOLVING accessor — +``resolve_n2m_descriptor`` over ``children()``, ``m2m_codegen.m2m_relationships`` over +``entity.children()`` — and passes the entity it is ITERATING, which for an inherited +relationship is not the one that declared it. So the DECLARING entity is resolved here +from ``rel.parent`` (same shape as the #368 loader fix, ``validation_passes``' +``declaring_entity = rel.parent if rel.parent is not None else obj``), and the passed +``source`` is kept alongside it rather than discarded: a junction FK usually references +the CONCRETE entity, because the abstract base has no table, while ``@objectRef`` on an +inherited self-join names the base. Both are accepted, for the self-join classification +and the hetero reference match alike. ``source`` is also the fallback when ``rel`` has no +object parent, which keeps the signature unchanged. Not covered: a junction reference +naming an entity strictly BETWEEN the base and the navigating entity in a deeper +hierarchy. + +The subject comparison is made on RESOLVED OBJECT IDENTITY, not on stripped short +names, matching the Java reference (``M2MFields.java``). Bare-name equality cannot +tell ``a::NodeBase`` from ``b::NodeBase``, so once the subject set held two names a +genuine CROSS-PACKAGE hetero M:N whose target shares a short name with the subject +read as a self-join and refused to derive. Scoped deliberately to this one predicate +— every other name comparison in this module is untouched. """ from __future__ import annotations from dataclasses import dataclass from ...meta_data import MetaData -from ....shared.base_types import TYPE_IDENTITY +from ....shared.base_types import TYPE_IDENTITY, TYPE_OBJECT from ....shared.separators import PACKAGE_SEP from ..identity.identity_constants import ( IDENTITY_ATTR_FIELDS, @@ -89,11 +111,72 @@ def _ref_target_entity(ref: MetaData) -> str | None: ``relationship_references.reference_target_entity`` (the #368 fix's canonical head-parse) and then strips any package prefix to keep this function's bare-name contract. + + Note this function is now reached only on the DEFENSIVE fallback path — when + ``@objectRef`` does not resolve to an entity at all, which loader validation + normally prevents. Both junction matches otherwise run through + :func:`_ref_target_qualified`, which keeps the package and compares by + resolved entity identity. """ target = reference_target_entity(ref) return _strip_package(target) if target else None +def _ref_target_qualified(ref: MetaData) -> str | None: + """The target-entity half of a reference's ``@references``, PACKAGE INTACT. + + Distinct from :func:`_ref_target_entity`, which strips the package for the + legacy bare-name compare. Identity resolution needs the qualified form to tell + ``a::NodeBase`` from ``b::NodeBase``, and needs the dotted ``Entity.field`` + form reduced to its entity head — which is exactly what the canonical + :func:`~metaobjects.meta.core.relationship.relationship_references.reference_target_entity` + already computes, so this delegates rather than keeping a third copy of the + parse. That helper searches the dot only AFTER the last ``::``, so a package + segment can never be mistaken for the field separator; a local ``find(".")`` + would have assumed packages never contain a dot instead of declining the + assumption. + + Delegating also finishes the seam with the branch that makes + :func:`_ref_target_entity` split: one parse, one behaviour, both sides of the + same ``if``. + """ + return reference_target_entity(ref) + + +def _root_objects(node: MetaData) -> list[MetaData]: + """Every top-level object of the tree *node* belongs to, in declaration order. + + Walks up to the root rather than taking the caller's ``object_index``: that + index is keyed by BARE name, so two same-short-name entities in different + packages collapse to one entry and FQN-exact resolution is impossible from it. + """ + root = node + while root.parent is not None: + root = root.parent + return [c for c in root.children() if c.type == TYPE_OBJECT] + + +def _find_entity(objects: list[MetaData], name: str | None) -> MetaData | None: + """The root entity a reference name denotes, or ``None``. + + Mirrors the Java reference's ``M2MFields.findObject`` exactly: a FULLY-QUALIFIED + name (one containing ``::``) resolves EXACTLY on the object's package-folded key, + never a bare-tail fallback; a bare name matches a short name, first match wins + (the bare-collision case is the deferred follow-up Java records as issue #174). + + This exists so the SUBJECT comparison can be made on object IDENTITY the way + Java's already is. A bare-name compare cannot tell ``a::NodeBase`` from + ``b::NodeBase``, which made a genuine cross-package hetero M:N read as a + self-join the moment the subject set held two names. + """ + if not name: + return None + if PACKAGE_SEP in name: + return next((o for o in objects if o.resolution_key() == name), None) + bare = _strip_package(name) + return next((o for o in objects if o.name == bare), None) + + def derive_m2m_fields( rel: MetaRelationship, source: MetaData, @@ -103,55 +186,123 @@ def derive_m2m_fields( *object_index* is a bare-name → object map of the loaded model's top-level objects (the Python loader's resolution surface; mirrors the TS - ``root.findObject``). Raises :class:`M2MDerivationError` when the junction is - missing/malformed or the self-join is ambiguous. + ``root.findObject``). *source* is the entity the caller is navigating from; + it is accepted alongside ``rel.parent`` as a name for the relationship's + subject, and used as the declaring entity when *rel* has no object parent — + see the module docstring. Raises + :class:`M2MDerivationError` when the junction is missing/malformed or the + self-join is ambiguous. """ + # The entity that DECLARES ``rel`` — see the module docstring. ``parent`` is + # the owning entity for both an own declaration and an inherited one (an + # unmodified inherited child is the SAME node object; an override is a + # different node whose parent is the overriding entity, also correct). + rel_parent = rel.parent + declaring = ( + rel_parent + if rel_parent is not None and rel_parent.type == TYPE_OBJECT + else source + ) + through_name = rel.through() if through_name is None: raise M2MDerivationError( - f'relationship "{source.name}.{rel.name}" is missing @through ' + f'relationship "{declaring.name}.{rel.name}" is missing @through ' f"(required for M:N derivation)" ) junction = object_index.get(_strip_package(through_name)) if junction is None: raise M2MDerivationError( - f'relationship "{source.name}.{rel.name}" @through "{through_name}" ' + f'relationship "{declaring.name}.{rel.name}" @through "{through_name}" ' f"does not resolve to an entity" ) target_name = rel.object_ref() if target_name is None: raise M2MDerivationError( - f'relationship "{source.name}.{rel.name}" is missing @objectRef ' + f'relationship "{declaring.name}.{rel.name}" is missing @objectRef ' f"(the M:N target)" ) refs = _reference_children(junction) if len(refs) != 2: raise M2MDerivationError( - f'junction "{through_name}" for relationship "{source.name}.{rel.name}" ' + f'junction "{through_name}" for relationship "{declaring.name}.{rel.name}" ' f"must declare exactly two identity.reference children " f"(found {len(refs)})" ) - is_self_join = _strip_package(target_name) == source.name + # The relationship's SUBJECT — the entity the M:N hangs off. Under ``extends`` + # there are two legitimate names for it and BOTH occur in real models: the + # DECLARING entity (what @objectRef names for a self-join declared on an + # abstract base, and what a junction reference names when the FK points at the + # base type), and the NAVIGATING entity (*source*, the concrete entity the + # caller is iterating — usually what a junction FK references, because that is + # the entity with the physical table). Accepting either is what makes the + # derivation independent of which entity's effective view reached the + # relationship. Not covered: a junction reference naming an entity strictly + # BETWEEN the base and the navigating entity in a deeper hierarchy. + subject_names = [declaring.name] + if source.name != declaring.name: + subject_names.append(source.name) + subject_label = " or ".join(f'"{n}"' for n in subject_names) + + # Compared by resolved object IDENTITY, matching the Java reference. A + # package-stripped compare cannot distinguish ``a::NodeBase`` from + # ``b::NodeBase``, so with two names in the set a genuine cross-package hetero + # M:N read as a self-join and refused to derive. + root_objects = _root_objects(declaring) + + def _is_subject(entity: MetaData | None) -> bool: + return entity is not None and (entity is declaring or entity is source) + + # Defensive bare fallback when @objectRef does not resolve — loader validation + # normally guarantees it does. Same carve-out the Java reference makes. + target_entity_node = _find_entity(root_objects, target_name) + is_self_join = ( + _is_subject(target_entity_node) + if target_entity_node is not None + else _strip_package(target_name) in subject_names + ) if not is_self_join: - # Hetero: match each reference by the entity it resolves to. + # Hetero: match each reference by the ENTITY OBJECT it resolves to. source_ref = next( - (r for r in refs if _ref_target_entity(r) == source.name), None - ) - target_ref = next( - (r for r in refs if _ref_target_entity(r) == _strip_package(target_name)), + ( + r + for r in refs + if _is_subject(_find_entity(root_objects, _ref_target_qualified(r))) + ), None, ) + # Identity here too. The two searches are INDEPENDENT — nothing excludes + # source_ref from this one, unlike the directed self-join branch below — so a + # bare compare could match the SOURCE-side reference again whenever the + # target's short name equals the source's, and silently return (src_fk, src_fk). + # Java matches identity on both sides (findRefToSubject + findRefToObject). + target_ref = ( + next( + ( + r + for r in refs + if _find_entity(root_objects, _ref_target_qualified(r)) + is target_entity_node + ), + None, + ) + if target_entity_node is not None + else next( + (r for r in refs if _ref_target_entity(r) == _strip_package(target_name)), + None, + ) + ) source_field = _ref_fk_field(source_ref) if source_ref is not None else None target_field = _ref_fk_field(target_ref) if target_ref is not None else None if source_field is None or target_field is None: raise M2MDerivationError( f'junction "{through_name}" for relationship ' - f'"{source.name}.{rel.name}" must declare one identity.reference ' - f'to "{source.name}" and one to "{_strip_package(target_name)}"' + f'"{declaring.name}.{rel.name}" must declare one identity.reference ' + f'to {subject_label} and one to "{_strip_package(target_name)}"' ) return M2MFields(source_field=source_field, target_field=target_field) @@ -163,14 +314,14 @@ def derive_m2m_fields( if a is None or b is None: raise M2MDerivationError( f'symmetric junction "{through_name}" for ' - f'"{source.name}.{rel.name}" has a reference with no @fields' + f'"{declaring.name}.{rel.name}" has a reference with no @fields' ) return M2MFields(source_field=a, target_field=b) source_ref_field = rel.source_ref_field() if source_ref_field is None: raise M2MDerivationError( - f'self-join relationship "{source.name}.{rel.name}" through ' + f'self-join relationship "{declaring.name}.{rel.name}" through ' f'"{through_name}" is ambiguous: set @sourceRefField (directed) or ' f"@symmetric (undirected)" ) @@ -182,7 +333,7 @@ def derive_m2m_fields( ) if source_ref is None: raise M2MDerivationError( - f'@sourceRefField "{source_ref_field}" on "{source.name}.{rel.name}" ' + f'@sourceRefField "{source_ref_field}" on "{declaring.name}.{rel.name}" ' f"does not match any identity.reference FK field on junction " f'"{through_name}"' ) @@ -190,7 +341,7 @@ def derive_m2m_fields( target_field = _ref_fk_field(target_ref) if target_ref is not None else None if target_field is None: raise M2MDerivationError( - f'junction "{through_name}" for "{source.name}.{rel.name}" has no ' + f'junction "{through_name}" for "{declaring.name}.{rel.name}" has no ' f"distinct target-side reference" ) return M2MFields(source_field=source_ref_field, target_field=target_field) diff --git a/server/python/tests/unit/test_derive_m2m_declaring_entity.py b/server/python/tests/unit/test_derive_m2m_declaring_entity.py new file mode 100644 index 000000000..e7e63e43d --- /dev/null +++ b/server/python/tests/unit/test_derive_m2m_declaring_entity.py @@ -0,0 +1,358 @@ +"""Follow-up to #368 — M:N derivation used the VISITING entity, not the DECLARING one. + +``derive_m2m_fields`` classified the self-join, and matched the hetero junction +reference, against the ``source`` entity its CALLER passed. Every caller walks a +RESOLVING accessor — ``resolve_n2m_descriptor`` iterates ``source_entity.children()``, +``m2m_codegen.m2m_relationships`` iterates ``entity.children()`` — and passes the +entity it is iterating, so for a relationship inherited via ``extends`` that is the +INHERITING entity. An inherited self-join then read as hetero and raised; an +inherited hetero whose junction references the BASE raised too. + +The fix accepts BOTH names of the relationship's subject — the declaring entity +(``rel.parent``) and the navigating entity — so the pre-existing shape (junction FK +references the CONCRETE child, pinned by ``test_n2m_resolver_inherited.py``) keeps +working. ``test_inherited_hetero_concrete_junction_reference`` below is the +counter-case that guards that. + +Unlike the #368 loader passes there is no once-per-node ``checked`` set here, so the +defect is not gated on visit order — it is wrong for every inheriting entity in any +order. The fixtures still declare the child BEFORE the base and pin the root object +order, matching the #368 convention. +""" +from __future__ import annotations + +import json + +from metaobjects import InMemoryStringSource, MetaDataLoader, load_string +from metaobjects.meta.core.object.meta_object import MetaObject +from metaobjects.meta.core.relationship.derive_m2m_fields import derive_m2m_fields +from metaobjects.meta.core.relationship.meta_relationship import MetaRelationship +from metaobjects.runtime.n2m_resolver import resolve_n2m_descriptor + + +def _entity(name: str, children: list[dict], **extra: object) -> dict: + return {"object.entity": {"name": name, "children": children, **extra}} + + +def _pk(field: str = "id") -> dict: + return {"identity.primary": {"name": field, "@fields": field}} + + +def _ref(name: str, fk: str, target: str) -> dict: + return {"identity.reference": {"name": name, "@fields": fk, "@references": target}} + + +# Node extends NodeBase, which declares a @symmetric self-join onto ITSELF; the +# junction references the BASE. Node is declared FIRST (child before base). +SELF_JOIN = { + "metadata.root": { + "package": "acme", + "children": [ + _entity("Node", [{"field.long": {"name": "id"}}, _pk()], extends="NodeBase"), + _entity( + "NodeBase", + [ + { + "relationship.association": { + "name": "peers", + "@cardinality": "many", + "@objectRef": "NodeBase", + "@through": "NodeLink", + "@symmetric": True, + } + } + ], + **{"@isAbstract": True}, + ), + _entity( + "NodeLink", + [ + {"field.long": {"name": "id"}}, + {"field.long": {"name": "aId"}}, + {"field.long": {"name": "bId"}}, + _pk(), + _ref("aRef", "aId", "NodeBase"), + _ref("bRef", "bId", "NodeBase"), + ], + ), + ], + } +} + +# Article extends ArticleBase; the junction references the BASE. +HETERO_BASE_REF = { + "metadata.root": { + "package": "acme", + "children": [ + _entity("Article", [{"field.long": {"name": "id"}}, _pk()], extends="ArticleBase"), + _entity( + "ArticleBase", + [ + { + "relationship.association": { + "name": "tags", + "@cardinality": "many", + "@objectRef": "Tag", + "@through": "ArticleTag", + } + } + ], + **{"@isAbstract": True}, + ), + _entity("Tag", [{"field.long": {"name": "id"}}, _pk()]), + _entity( + "ArticleTag", + [ + {"field.long": {"name": "id"}}, + {"field.long": {"name": "articleId"}}, + {"field.long": {"name": "tagId"}}, + _pk(), + _ref("aRef", "articleId", "ArticleBase"), + _ref("tRef", "tagId", "Tag"), + ], + ), + ], + } +} + +# The OTHER legitimate shape: the junction references the CONCRETE child. +HETERO_CONCRETE_REF = { + "metadata.root": { + "package": "acme", + "children": [ + _entity("Post", [{"field.long": {"name": "id"}}, _pk()], extends="PostBase"), + _entity( + "PostBase", + [ + { + "relationship.association": { + "name": "tags", + "@cardinality": "many", + "@objectRef": "Tag", + "@through": "PostTag", + } + } + ], + **{"@isAbstract": True}, + ), + _entity("Tag", [{"field.long": {"name": "id"}}, _pk()]), + _entity( + "PostTag", + [ + {"field.long": {"name": "id"}}, + {"field.long": {"name": "postId"}}, + {"field.long": {"name": "tagId"}}, + _pk(), + _ref("pRef", "postId", "Post"), + _ref("tRef", "tagId", "Tag"), + ], + ), + ], + } +} + + +def _index(meta: dict) -> dict[str, MetaObject]: + root = load_string(json.dumps(meta)).root + return {c.name: c for c in root.children() if isinstance(c, MetaObject)} + + +def _rel(entity: MetaObject, name: str) -> MetaRelationship: + for c in entity.children(): + if isinstance(c, MetaRelationship) and c.name == name: + return c + raise AssertionError(f"no relationship {name} on {entity.name}") + + +def test_inherited_self_join_derives_both_fk_sides() -> None: + index = _index(SELF_JOIN) + # Pin the premise: the child is reached before the base it inherits from. + assert list(index) == ["Node", "NodeBase", "NodeLink"] + node = index["Node"] + rel = _rel(node, "peers") + # Genuinely inherited — not one of Node's own children. + assert "peers" not in {c.name for c in node.own_children()} + assert rel.parent is not None and rel.parent.name == "NodeBase" + + fields = derive_m2m_fields(rel, node, index) + assert fields.source_field == "aId" + assert fields.target_field == "bId" + # The declaring entity itself must agree — same node, same answer. + assert derive_m2m_fields(rel, index["NodeBase"], index) == fields + + +def test_inherited_hetero_matches_the_declaring_bases_reference() -> None: + index = _index(HETERO_BASE_REF) + assert list(index) == ["Article", "ArticleBase", "Tag", "ArticleTag"] + fields = derive_m2m_fields(_rel(index["Article"], "tags"), index["Article"], index) + assert fields.source_field == "articleId" + assert fields.target_field == "tagId" + + +def test_inherited_hetero_concrete_junction_reference() -> None: + """Counter-case: accepting ONLY the declaring entity would break this shape.""" + index = _index(HETERO_CONCRETE_REF) + rel = _rel(index["Post"], "tags") + assert rel.parent is not None and rel.parent.name == "PostBase" + fields = derive_m2m_fields(rel, index["Post"], index) + assert fields.source_field == "postId" + assert fields.target_field == "tagId" + + +def test_runtime_resolver_traverses_an_inherited_self_join() -> None: + """The runtime path re-raises the derivation error, so an inherited self-join + was untraversable at run time, not merely dropped from generated code.""" + index = _index(SELF_JOIN) + desc = resolve_n2m_descriptor(index["Node"], "peers", index) + assert desc is not None + assert desc.source_entity_name == "Node" + assert desc.source_field == "aId" + assert desc.target_field == "bId" + assert desc.symmetric is True + +# The Python codegen path (``m2m_codegen.resolve_m2m_descriptors``) calls the same +# SSOT and so is fixed transitively; it is not exercised here because it additionally +# requires a physical ``source.rdb`` on the junction and target, which an abstract +# declaring base does not have — a separate concern from the derivation. + + +# REGRESSION — a cross-package hetero M:N must not be read as a self-join just because +# the target's SHORT name matches one of the subject's. +# +# ``a::NodeBase`` declares a genuine cross-package hetero M:N onto ``b::NodeBase``, and +# ``a::Node`` extends ``a::NodeBase``. Deriving from ``a::Node`` the subject is +# {a::NodeBase, a::Node}; under the old package-stripped compare "b::NodeBase" stripped +# to "NodeBase", landed in the subject set, and the relationship refused to derive as an +# ambiguous self-join. It had derived correctly before the subject set grew to two names, +# so that was a regression, not a pre-existing gap. +# +# Fixed by comparing RESOLVED OBJECT IDENTITY, which is what the Java port has always +# done — this is the Python half of the pair with +# M2MSlimVocabularyTest.deriveCrossPackageHeteroBindsCorrectPackage, and it REDUCES the +# cross-port divergence rather than pinning it. +# +# The junction's SOURCE reference names the DECLARING BASE ("a::NodeBase") — the shape +# docs/features/relationships.md blesses, and the one whose short name collides with the +# target's. That matters: making is_self_join identity-based while the hetero TARGET +# search was still a bare compare let that search re-match this very reference (nothing +# excludes source_ref from it, unlike the directed self-join branch) and return +# (srcId, srcId) silently. Both searches are identity-based now, as in Java. +XPKG_A = { + "metadata.root": { + "package": "a", + "children": [ + _entity("Node", [{"field.long": {"name": "id"}}, _pk()], extends="a::NodeBase"), + _entity( + "NodeBase", + [ + { + "relationship.association": { + "name": "links", + "@cardinality": "many", + "@objectRef": "b::NodeBase", + "@through": "L", + } + } + ], + **{"@isAbstract": True}, + ), + _entity( + "L", + [ + {"field.long": {"name": "srcId"}}, + {"field.long": {"name": "dstId"}}, + {"identity.primary": {"name": "id", "@fields": ["srcId", "dstId"]}}, + _ref("s", "srcId", "a::NodeBase"), + _ref("d", "dstId", "b::NodeBase"), + ], + ), + ], + } +} + +XPKG_B = { + "metadata.root": { + "package": "b", + "children": [_entity("NodeBase", [{"field.long": {"name": "id"}}, _pk()])], + } +} + + +def test_cross_package_target_sharing_a_subject_short_name_is_not_a_self_join() -> None: + """Identity resolution keeps this hetero, exactly as Java does.""" + result = MetaDataLoader().load([ + InMemoryStringSource(json.dumps(XPKG_A), id="a.json"), + InMemoryStringSource(json.dumps(XPKG_B), id="b.json"), + ]) + # The model itself is perfectly legal — the loader raises nothing. + assert result.errors == [] + root = result.root + objects = [c for c in root.children() if isinstance(c, MetaObject)] + # Pin the premise: the child is reached before the base it inherits from. + assert [o.name for o in objects] == ["Node", "NodeBase", "L", "NodeBase"] + index = {o.name: o for o in objects} + node = objects[0] + rel = _rel(node, "links") + fields = derive_m2m_fields(rel, node, index) + assert fields.source_field == "srcId" + assert fields.target_field == "dstId" + + +# Java's deriveCrossPackageHeteroBindsCorrectPackage model, ported. No inheritance is +# needed to reach the same defect: ``a::Account`` relates to ``b::Account`` through a +# junction holding one reference to each. The two junction searches are INDEPENDENT, so +# a bare-name target match found ``ownerRef`` a second time and returned +# (ownerId, ownerId). Measured before the fix; pre-branch it threw loudly instead. +XPKG_HETERO_A = { + "metadata.root": { + "package": "a", + "children": [ + _entity( + "Account", + [ + {"field.long": {"name": "id"}}, + { + "relationship.association": { + "name": "partners", + "@cardinality": "many", + "@objectRef": "b::Account", + "@through": "AccountLink", + } + }, + _pk(), + ], + ), + _entity( + "AccountLink", + [ + {"field.long": {"name": "ownerId"}}, + {"field.long": {"name": "partnerId"}}, + {"identity.primary": {"name": "id", "@fields": ["ownerId", "partnerId"]}}, + _ref("ownerRef", "ownerId", "a::Account"), + _ref("partnerRef", "partnerId", "b::Account"), + ], + ), + ], + } +} + +XPKG_HETERO_B = { + "metadata.root": { + "package": "b", + "children": [_entity("Account", [{"field.long": {"name": "id"}}, _pk()])], + } +} + + +def test_cross_package_hetero_matches_each_reference_to_its_own_entity() -> None: + result = MetaDataLoader().load([ + InMemoryStringSource(json.dumps(XPKG_HETERO_A), id="a.json"), + InMemoryStringSource(json.dumps(XPKG_HETERO_B), id="b.json"), + ]) + assert result.errors == [] + objects = [c for c in result.root.children() if isinstance(c, MetaObject)] + account = next(o for o in objects if o.resolution_key() == "a::Account") + index = {o.name: o for o in objects} + fields = derive_m2m_fields(_rel(account, "partners"), account, index) + assert fields.source_field == "ownerId" + assert fields.target_field == "partnerId" # was "ownerId" diff --git a/server/typescript/packages/codegen-ts/test/relation-resolver-inherited-m2m.test.ts b/server/typescript/packages/codegen-ts/test/relation-resolver-inherited-m2m.test.ts new file mode 100644 index 000000000..573beb448 --- /dev/null +++ b/server/typescript/packages/codegen-ts/test/relation-resolver-inherited-m2m.test.ts @@ -0,0 +1,108 @@ +// Follow-up to #368 — M:N derivation used the VISITING entity, not the entity +// that DECLARES the relationship. +// +// buildRelationMap walks `root.objects()` and, for each, the RESOLVING +// `obj.relationships()` — so a M:N relationship declared on an abstract base +// is reached again through every entity that extends it, with `obj` being the +// INHERITING entity. buildM2mEntry passed that entity to deriveM2MFields as +// the source, so: +// * an inherited SELF-JOIN compared @objectRef (the base) against the child, +// concluded "hetero", looked for a junction reference to the child, found +// none and threw; +// * an inherited HETERO relationship looked for a junction reference to the +// child when the junction references the base, and threw too. +// buildM2mEntry catches and returns null, so the navigation was silently +// dropped from the generated relations() block — no error, no output. +// +// The child is declared BEFORE the base in both fixtures, so the walk reaches +// it first (the order shape the #368 loader regressions established). + +import { describe, expect, test } from "bun:test"; +import { MetaDataLoader, InMemoryStringSource, type MetaRoot } from "@metaobjectsdev/metadata"; +import { buildRelationMap } from "../src/relation-resolver.js"; + +const SELF_JOIN = { + "metadata.root": { + package: "repro", + children: [ + { "object.entity": { name: "Node", extends: "NodeBase", children: [ + { "field.int": { name: "id" } }, + { "identity.primary": { name: "id", "@fields": ["id"], "@generation": "increment" } }, + ] } }, + { "object.entity": { name: "NodeBase", "@isAbstract": true, children: [ + { "relationship.association": { name: "peers", "@cardinality": "many", "@objectRef": "NodeBase", + "@through": "NodeLink", "@symmetric": true } }, + ] } }, + { "object.entity": { name: "NodeLink", children: [ + { "field.int": { name: "id" } }, + { "field.int": { name: "aId" } }, + { "field.int": { name: "bId" } }, + { "identity.primary": { name: "id", "@fields": ["id"], "@generation": "increment" } }, + { "identity.reference": { name: "a", "@fields": ["aId"], "@references": "NodeBase" } }, + { "identity.reference": { name: "b", "@fields": ["bId"], "@references": "NodeBase" } }, + ] } }, + ], + }, +}; + +const HETERO = { + "metadata.root": { + package: "repro", + children: [ + { "object.entity": { name: "Article", extends: "ArticleBase", children: [ + { "field.int": { name: "id" } }, + { "identity.primary": { name: "id", "@fields": ["id"], "@generation": "increment" } }, + ] } }, + { "object.entity": { name: "ArticleBase", "@isAbstract": true, children: [ + { "relationship.association": { name: "tags", "@cardinality": "many", "@objectRef": "Tag", + "@through": "ArticleTag" } }, + ] } }, + { "object.entity": { name: "Tag", children: [ + { "field.int": { name: "id" } }, + { "identity.primary": { name: "id", "@fields": ["id"], "@generation": "increment" } }, + ] } }, + { "object.entity": { name: "ArticleTag", children: [ + { "field.int": { name: "id" } }, + { "field.int": { name: "articleId" } }, + { "field.int": { name: "tagId" } }, + { "identity.primary": { name: "id", "@fields": ["id"], "@generation": "increment" } }, + { "identity.reference": { name: "articleRef", "@fields": ["articleId"], "@references": "ArticleBase" } }, + { "identity.reference": { name: "tagRef", "@fields": ["tagId"], "@references": "Tag" } }, + ] } }, + ], + }, +}; + +async function load(model: unknown): Promise { + const res = await new MetaDataLoader().load([new InMemoryStringSource(JSON.stringify(model))]); + expect(res.errors).toEqual([]); + return res.root; +} + +describe("buildRelationMap — M:N inherited via extends", () => { + test("inherited self-join emits a navigation on the child (was: silently dropped)", async () => { + const root = await load(SELF_JOIN); + // Pin the premise: the child is walked before the base it inherits from. + expect(root.objects().map((o) => o.name)).toEqual(["Node", "NodeBase", "NodeLink"]); + + const entries = buildRelationMap(root).get("Node") ?? []; + const peers = entries.find((e) => e.name === "peers"); + expect(peers).toBeDefined(); + expect(peers!.cardinality).toBe("many"); + expect(peers!.junctionEntity).toBe("NodeLink"); + expect(peers!.sourceJoinField).toBe("aId"); + expect(peers!.targetJoinField).toBe("bId"); + expect(peers!.symmetric).toBe(true); + }); + + test("inherited hetero M:N emits a navigation on the child (was: silently dropped)", async () => { + const root = await load(HETERO); + const entries = buildRelationMap(root).get("Article") ?? []; + const tags = entries.find((e) => e.name === "tags"); + expect(tags).toBeDefined(); + expect(tags!.targetEntity).toBe("Tag"); + expect(tags!.junctionEntity).toBe("ArticleTag"); + expect(tags!.sourceJoinField).toBe("articleId"); + expect(tags!.targetJoinField).toBe("tagId"); + }); +}); diff --git a/server/typescript/packages/metadata/src/core/relationship/derive-m2m-fields.ts b/server/typescript/packages/metadata/src/core/relationship/derive-m2m-fields.ts index 695f8433f..c99a0de97 100644 --- a/server/typescript/packages/metadata/src/core/relationship/derive-m2m-fields.ts +++ b/server/typescript/packages/metadata/src/core/relationship/derive-m2m-fields.ts @@ -17,12 +17,35 @@ // two references are taken in declaration order (sourceField = first, // targetField = second). Resolution unions both at read time. // Ambiguous (source == target, neither @sourceRefField nor @symmetric) → throw. +// +// "source" above means the relationship's SUBJECT, and under `extends` there are two +// legitimate names for it. Every caller walks the RESOLVING `obj.relationships()` +// (codegen-ts's relation-resolver, runtime-ts's n2m-resolver, docs-site's link-graph) +// and passes the entity it is ITERATING, which for an inherited relationship is not +// the one that declared it. So the DECLARING entity is resolved here from `rel.parent` +// (same shape as the #368 loader fix, validation-passes' `declaringEntity = rel.parent +// ?? obj`), and the passed `source` is kept alongside it rather than discarded: a +// junction FK usually references the CONCRETE entity, because the abstract base has no +// table, while @objectRef on an inherited self-join names the base. Both are accepted, +// for the self-join classification and the hetero reference match alike. `source` is +// also the fallback when `rel` has no entity parent, which keeps the exported signature +// unchanged. Not covered: a junction reference naming an entity strictly BETWEEN the +// base and the navigating entity in a deeper hierarchy. +// +// The subject comparison is made on RESOLVED OBJECT IDENTITY, not on stripped +// short names, matching the Java reference (M2MFields.java). Bare-name equality +// cannot tell `a::NodeBase` from `b::NodeBase`, so once the subject set held two +// names a genuine CROSS-PACKAGE hetero M:N whose target shares a short name with +// the subject read as a self-join and refused to derive. Scoped deliberately to +// this one predicate — every other name comparison in this file is untouched. import type { MetaObject } from "../object/meta-object.js"; import type { MetaRoot } from "../../shared/meta-root.js"; import type { MetaRelationship } from "./meta-relationship.js"; import type { MetaReferenceIdentity } from "../identity/meta-identity.js"; import { stripPackage } from "../../naming.js"; +import { TYPE_OBJECT } from "../../shared/base-types.js"; +import { PACKAGE_SEPARATOR } from "../../shared/structural.js"; /** Thrown when a M:N relationship's junction FK fields cannot be derived. */ export class M2MDerivationError extends Error { @@ -45,12 +68,36 @@ function refFkField(ref: MetaReferenceIdentity): string | undefined { return ref.fields[0]; } +/** + * The root entity a reference name denotes, or undefined. Mirrors the Java + * reference's `M2MFields.findObject` exactly: a FULLY-QUALIFIED name (one + * containing "::") resolves EXACTLY on the object's package-folded key, never a + * bare-tail fallback; a bare name matches a short name, first match wins (the + * bare-collision case is the deferred follow-up Java records as issue #174). + * + * This exists so the SUBJECT comparison below can be made on object IDENTITY the + * way Java's already is. A bare-name compare cannot tell `a::NodeBase` from + * `b::NodeBase`, which made a genuine cross-package hetero M:N read as a + * self-join the moment the subject set held two names. + */ +function findEntity(root: MetaRoot, name: string | undefined): MetaObject | undefined { + if (name === undefined || name === "") return undefined; + const objects = root.objects(); + if (name.includes(PACKAGE_SEPARATOR)) { + return objects.find((o) => o.resolutionKey() === name); + } + const bare = stripPackage(name); + return objects.find((o) => o.name === bare); +} + /** * Derive the source/target junction FK fields for a M:N relationship. * * @param rel the M:N relationship (carries @objectRef + @through + optional * @sourceRefField / @symmetric) - * @param source the entity declaring `rel` + * @param source the entity the caller is navigating from. Accepted alongside + * `rel.parent` as a name for the relationship's subject, and used + * as the declaring entity when `rel` has no entity parent. * @param root the loaded model root (to find the junction entity) * @throws M2MDerivationError when the junction is missing/malformed or the * self-join is ambiguous. @@ -60,10 +107,21 @@ export function deriveM2MFields( source: MetaObject, root: MetaRoot, ): M2MFields { + // The entity that DECLARES `rel` — see the header note. `rel.parent` is the + // owning entity for both an own declaration and an inherited one (an unmodified + // inherited child is the SAME node object, reused in place by + // MetaData._effectiveChildren; an override is a genuinely different node whose + // parent is the overriding entity, which is also correct). + const relParent = rel.parent; + const declaringEntity: MetaObject = + relParent !== undefined && relParent.type === TYPE_OBJECT + ? (relParent as MetaObject) + : source; + const throughName = rel.through; if (throughName === undefined) { throw new M2MDerivationError( - `relationship "${source.name}.${rel.name}" is missing @through (required for M:N derivation)`, + `relationship "${declaringEntity.name}.${rel.name}" is missing @through (required for M:N derivation)`, ); } // @through may be package-qualified (FQN); findObject is keyed by bare name, @@ -76,37 +134,79 @@ export function deriveM2MFields( : undefined); if (junction === undefined) { throw new M2MDerivationError( - `relationship "${source.name}.${rel.name}" @through "${throughName}" does not resolve to an entity`, + `relationship "${declaringEntity.name}.${rel.name}" @through "${throughName}" does not resolve to an entity`, ); } const targetName = rel.objectRef; if (targetName === undefined) { throw new M2MDerivationError( - `relationship "${source.name}.${rel.name}" is missing @objectRef (the M:N target)`, + `relationship "${declaringEntity.name}.${rel.name}" is missing @objectRef (the M:N target)`, ); } const refs = junction.referenceIdentities(); if (refs.length !== 2) { throw new M2MDerivationError( - `junction "${throughName}" for relationship "${source.name}.${rel.name}" must declare exactly two ` + + `junction "${throughName}" for relationship "${declaringEntity.name}.${rel.name}" must declare exactly two ` + `identity.reference children (found ${refs.length})`, ); } - const isSelfJoin = stripPackage(targetName) === source.name; + // The relationship's SUBJECT — the entity the M:N hangs off. Under `extends` + // there are two legitimate names for it and BOTH occur in real models: + // * the DECLARING entity (rel.parent) — what @objectRef names for a self-join + // declared on an abstract base, and what a junction reference names when the + // author points the FK at the base type; + // * the NAVIGATING entity (`source`) — the concrete entity the caller is + // iterating, which is what a junction FK usually references, because that is + // the entity with the physical table. + // Accepting either is what makes the derivation independent of which entity's + // effective view reached the relationship. (Not covered: a junction reference + // naming an entity strictly BETWEEN the declaring base and the navigating + // entity in a deeper hierarchy — no model does that, and widening to the whole + // super chain would make the "must declare one identity.reference to ..." error + // unfalsifiable.) + const subjectNames = declaringEntity.name === source.name + ? [declaringEntity.name] + : [declaringEntity.name, source.name]; + const subjectLabel = subjectNames.map((n) => `"${n}"`).join(" or "); + // Compared by resolved object IDENTITY, matching the Java reference. A + // stripPackage() compare cannot distinguish `a::NodeBase` from `b::NodeBase`, + // so with two names in the set a genuine cross-package hetero M:N read as a + // self-join and refused to derive. + const isSubject = (entity: MetaObject | undefined): boolean => + entity !== undefined && (entity === declaringEntity || entity === source); + const isSubjectName = (name: string | undefined): boolean => + name !== undefined && subjectNames.includes(stripPackage(name)); + + // Defensive bare fallback when @objectRef does not resolve — loader validation + // normally guarantees it does. Same carve-out the Java reference makes. + const targetEntityNode = findEntity(root, targetName); + const isSelfJoin = targetEntityNode !== undefined + ? isSubject(targetEntityNode) + : isSubjectName(targetName); if (!isSelfJoin) { - // Hetero: match each reference by the entity it resolves to. - const sourceRef = refs.find((r) => r.targetEntity !== undefined && stripPackage(r.targetEntity) === source.name); - const targetRef = refs.find((r) => r.targetEntity !== undefined && stripPackage(r.targetEntity) === stripPackage(targetName)); + // Hetero: match each reference by the ENTITY OBJECT it resolves to. + const sourceRef = refs.find((r) => isSubject(findEntity(root, r.targetEntity))); + // Identity here too. The two searches are INDEPENDENT — nothing excludes + // sourceRef from this one, unlike the directed self-join branch below — so a + // bare compare could match the SOURCE-side reference again whenever the + // target's short name equals the source's, and silently return (srcFk, srcFk). + // Structurally unreachable while isSelfJoin was also bare (a colliding short + // name forced the self-join branch); making only isSelfJoin identity-based + // broke that invariant. Java matches identity on both sides (findRefToSubject + // + findRefToObject) and never had the hole. + const targetRef = targetEntityNode !== undefined + ? refs.find((r) => findEntity(root, r.targetEntity) === targetEntityNode) + : refs.find((r) => r.targetEntity !== undefined && stripPackage(r.targetEntity) === stripPackage(targetName)); const sourceField = sourceRef ? refFkField(sourceRef) : undefined; const targetField = targetRef ? refFkField(targetRef) : undefined; if (sourceField === undefined || targetField === undefined) { throw new M2MDerivationError( - `junction "${throughName}" for relationship "${source.name}.${rel.name}" must declare one ` + - `identity.reference to "${source.name}" and one to "${stripPackage(targetName)}"`, + `junction "${throughName}" for relationship "${declaringEntity.name}.${rel.name}" must declare one ` + + `identity.reference to ${subjectLabel} and one to "${stripPackage(targetName)}"`, ); } return { sourceField, targetField }; @@ -119,7 +219,7 @@ export function deriveM2MFields( const b = refFkField(refs[1]!); if (a === undefined || b === undefined) { throw new M2MDerivationError( - `symmetric junction "${throughName}" for "${source.name}.${rel.name}" has a reference with no @fields`, + `symmetric junction "${throughName}" for "${declaringEntity.name}.${rel.name}" has a reference with no @fields`, ); } return { sourceField: a, targetField: b }; @@ -128,7 +228,7 @@ export function deriveM2MFields( const sourceRefField = rel.sourceRefField; if (sourceRefField === undefined) { throw new M2MDerivationError( - `self-join relationship "${source.name}.${rel.name}" through "${throughName}" is ambiguous: ` + + `self-join relationship "${declaringEntity.name}.${rel.name}" through "${throughName}" is ambiguous: ` + `set @sourceRefField (directed) or @symmetric (undirected)`, ); } @@ -137,7 +237,7 @@ export function deriveM2MFields( const sourceRef = refs.find((r) => refFkField(r) === sourceRefField); if (sourceRef === undefined) { throw new M2MDerivationError( - `@sourceRefField "${sourceRefField}" on "${source.name}.${rel.name}" does not match any ` + + `@sourceRefField "${sourceRefField}" on "${declaringEntity.name}.${rel.name}" does not match any ` + `identity.reference FK field on junction "${throughName}"`, ); } @@ -145,7 +245,7 @@ export function deriveM2MFields( const targetField = targetRef ? refFkField(targetRef) : undefined; if (targetField === undefined) { throw new M2MDerivationError( - `junction "${throughName}" for "${source.name}.${rel.name}" has no distinct target-side reference`, + `junction "${throughName}" for "${declaringEntity.name}.${rel.name}" has no distinct target-side reference`, ); } return { sourceField: sourceRefField, targetField }; diff --git a/server/typescript/packages/metadata/test/relationship-m2m.test.ts b/server/typescript/packages/metadata/test/relationship-m2m.test.ts index 6d0a45765..de719e017 100644 --- a/server/typescript/packages/metadata/test/relationship-m2m.test.ts +++ b/server/typescript/packages/metadata/test/relationship-m2m.test.ts @@ -695,3 +695,292 @@ describe("FR-017 #368 order-dependence regressions (declaring entity vs. visitin expect(codesOf(errors)).not.toContain("ERR_INVALID_RELATIONSHIP"); }); }); + +// --------------------------------------------------------------------------- +// deriveM2MFields — declaring entity vs. visiting entity (the #368 follow-up). +// +// Same confusion as the two regressions above, one layer down: the derivation +// classified the self-join, and matched the hetero junction reference, against +// the `source` entity its CALLER passed. Every caller walks the RESOLVING +// `obj.relationships()` (codegen-ts's relation-resolver, runtime-ts's +// n2m-resolver, docs-site's link-graph, and their Java/C#/Python twins) and +// passes the entity it is iterating — so for a relationship inherited via +// `extends` that is the INHERITING entity, not the one that declared it. +// +// Unlike the loader-pass regressions above, this defect is NOT gated on visit +// order: there is no once-per-node `checked` set here, so the derivation is +// simply wrong for every inheriting entity, whichever order they are reached +// in. The fixtures still declare the child BEFORE the base and pin the visit +// order, matching the #368 convention and covering the order-sensitive shape +// for free. +// --------------------------------------------------------------------------- + +describe("FR-017 deriveM2MFields uses the DECLARING entity, not the visiting one", () => { + function objectVisitOrder(root: { children(): readonly { type: string; resolutionKey(): string }[] }): string[] { + return root.children().filter((c) => c.type === TYPE_OBJECT).map((o) => o.resolutionKey()); + } + + test("inherited symmetric self-join derives both FK sides (was: read as hetero, threw)", async () => { + // NodeBase declares a @symmetric self-join onto itself; Node extends it and + // is declared FIRST. Reached through Node's effective view, the derivation + // used to compare @objectRef "NodeBase" against the VISITING "Node", + // conclude "not a self-join", take the hetero branch, look for a junction + // reference to "Node" (there is none — both point at NodeBase) and throw + // M2MDerivationError. Codegen swallows that throw, so the navigation just + // vanished from the generated output. + const { root, errors } = await loadDoc({ "metadata.root": { package: "acme", children: [ + { "object.entity": { name: "Node", "extends": "NodeBase", children: [ + { "field.long": { name: "id" } }, + { "identity.primary": { "name": "id", "@fields": "id" } } ] } }, + { "object.entity": { name: "NodeBase", "@isAbstract": true, children: [ + { "relationship.association": { name: "peers", "@cardinality": "many", "@objectRef": "NodeBase", + "@through": "NodeLink", "@symmetric": true } } ] } }, + { "object.entity": { name: "NodeLink", children: [ + { "field.long": { name: "id" } }, + { "field.long": { name: "aId" } }, + { "field.long": { name: "bId" } }, + { "identity.primary": { "name": "id", "@fields": "id" } }, + { "identity.reference": { name: "a", "@fields": ["aId"], "@references": "NodeBase" } }, + { "identity.reference": { name: "b", "@fields": ["bId"], "@references": "NodeBase" } } ] } }, + ] } }); + expect(errors).toHaveLength(0); + expect(objectVisitOrder(root)).toEqual(["acme::Node", "acme::NodeBase", "acme::NodeLink"]); + + const node = findObj(root, "Node"); + // RESOLVING accessor — this is exactly what every caller walks, and it is + // what surfaces the inherited relationship on the child. + const rel = node.relationships().find((r) => r.name === "peers") as MetaRelationship; + expect(node.ownRelationships()).toHaveLength(0); + expect(rel.parent?.name).toBe("NodeBase"); + + const derived = deriveM2MFields(rel, node, root); + expect(derived.sourceField).toBe("aId"); + expect(derived.targetField).toBe("bId"); + // And the declaring entity itself must still agree — same node, same answer. + expect(deriveM2MFields(rel, findObj(root, "NodeBase"), root)).toEqual(derived); + }); + + test("inherited directed self-join honours @sourceRefField through the child's view", async () => { + const { root, errors } = await loadDoc({ "metadata.root": { package: "acme", children: [ + { "object.entity": { name: "Person", "extends": "PartyBase", children: [ + { "field.long": { name: "id" } }, + { "identity.primary": { "name": "id", "@fields": "id" } } ] } }, + { "object.entity": { name: "PartyBase", "@isAbstract": true, children: [ + { "relationship.association": { name: "follows", "@cardinality": "many", "@objectRef": "PartyBase", + "@through": "Follow", "@sourceRefField": "followerId" } } ] } }, + { "object.entity": { name: "Follow", children: [ + { "field.long": { name: "id" } }, + { "field.long": { name: "followerId" } }, + { "field.long": { name: "followeeId" } }, + { "identity.primary": { "name": "id", "@fields": "id" } }, + { "identity.reference": { name: "followerRef", "@fields": ["followerId"], "@references": "PartyBase" } }, + { "identity.reference": { name: "followeeRef", "@fields": ["followeeId"], "@references": "PartyBase" } } ] } }, + ] } }); + expect(errors).toHaveLength(0); + expect(objectVisitOrder(root)).toEqual(["acme::Person", "acme::PartyBase", "acme::Follow"]); + + const person = findObj(root, "Person"); + const rel = person.relationships().find((r) => r.name === "follows") as MetaRelationship; + const derived = deriveM2MFields(rel, person, root); + expect(derived.sourceField).toBe("followerId"); + expect(derived.targetField).toBe("followeeId"); + }); + + test("inherited HETERO M:N matches the junction reference to the declaring base", async () => { + // The mirror image: the junction references the DECLARING base (ArticleBase), + // so a hetero match against the visiting child (Article) found nothing and + // threw the "must declare one identity.reference to ..." error. + const { root, errors } = await loadDoc({ "metadata.root": { package: "acme", children: [ + { "object.entity": { name: "Article", "extends": "ArticleBase", children: [ + { "field.long": { name: "id" } }, + { "identity.primary": { "name": "id", "@fields": "id" } } ] } }, + { "object.entity": { name: "ArticleBase", "@isAbstract": true, children: [ + { "relationship.association": { name: "tags", "@cardinality": "many", "@objectRef": "Tag", + "@through": "ArticleTag" } } ] } }, + { "object.entity": { name: "Tag", children: [ + { "field.long": { name: "id" } }, + { "identity.primary": { "name": "id", "@fields": "id" } } ] } }, + { "object.entity": { name: "ArticleTag", children: [ + { "field.long": { name: "id" } }, + { "field.long": { name: "articleId" } }, + { "field.long": { name: "tagId" } }, + { "identity.primary": { "name": "id", "@fields": "id" } }, + { "identity.reference": { name: "articleRef", "@fields": ["articleId"], "@references": "ArticleBase" } }, + { "identity.reference": { name: "tagRef", "@fields": ["tagId"], "@references": "Tag" } } ] } }, + ] } }); + expect(errors).toHaveLength(0); + expect(objectVisitOrder(root)).toEqual(["acme::Article", "acme::ArticleBase", "acme::Tag", "acme::ArticleTag"]); + + const article = findObj(root, "Article"); + const rel = article.relationships().find((r) => r.name === "tags") as MetaRelationship; + const derived = deriveM2MFields(rel, article, root); + expect(derived.sourceField).toBe("articleId"); + expect(derived.targetField).toBe("tagId"); + }); + + test("inherited HETERO whose junction references the CONCRETE child still derives", async () => { + // The other legitimate authoring shape, and the common one: the base is + // abstract (no table), so the junction FK references the CONCRETE entity. + // Both names — the declaring base and the navigating child — must be + // accepted as the relationship's subject, or fixing the base-referencing + // shape would break this one. Pinned by + // python/tests/unit/test_n2m_resolver_inherited.py, which is authored this way. + const { root, errors } = await loadDoc({ "metadata.root": { package: "acme", children: [ + { "object.entity": { name: "Post", "extends": "PostBase", children: [ + { "field.long": { name: "id" } }, + { "identity.primary": { "name": "id", "@fields": "id" } } ] } }, + { "object.entity": { name: "PostBase", "@isAbstract": true, children: [ + { "relationship.association": { name: "tags", "@cardinality": "many", "@objectRef": "Tag", + "@through": "PostTag" } } ] } }, + { "object.entity": { name: "Tag", children: [ + { "field.long": { name: "id" } }, + { "identity.primary": { "name": "id", "@fields": "id" } } ] } }, + { "object.entity": { name: "PostTag", children: [ + { "field.long": { name: "id" } }, + { "field.long": { name: "postId" } }, + { "field.long": { name: "tagId" } }, + { "identity.primary": { "name": "id", "@fields": "id" } }, + { "identity.reference": { name: "postRef", "@fields": ["postId"], "@references": "Post" } }, + { "identity.reference": { name: "tagRef", "@fields": ["tagId"], "@references": "Tag" } } ] } }, + ] } }); + expect(errors).toHaveLength(0); + const post = findObj(root, "Post"); + const rel = post.relationships().find((r) => r.name === "tags") as MetaRelationship; + expect(rel.parent?.name).toBe("PostBase"); + const derived = deriveM2MFields(rel, post, root); + expect(derived.sourceField).toBe("postId"); + expect(derived.targetField).toBe("tagId"); + }); + + // REGRESSION — a cross-package hetero M:N must not be read as a self-join just + // because the target's SHORT name matches one of the subject's. + // + // `a::NodeBase` declares a genuine cross-package hetero M:N onto `b::NodeBase`, + // and `a::Node extends a::NodeBase`. Deriving from `a::Node` the subject is + // {a::NodeBase, a::Node}; under the old stripPackage() compare "b::NodeBase" + // stripped to "NodeBase", landed in the subject set, and the relationship + // refused to derive as an ambiguous self-join. It had derived correctly before + // the subject set grew to two names, so that was a regression, not a + // pre-existing gap. + // + // Fixed by comparing RESOLVED OBJECT IDENTITY, which is what the Java port has + // always done — this is the TS half of the pair with + // M2MSlimVocabularyTest.deriveCrossPackageHeteroBindsCorrectPackage, and it + // REDUCES the cross-port divergence rather than pinning it. + // + // The junction's SOURCE reference names the DECLARING BASE ("a::NodeBase") — the + // shape docs/features/relationships.md blesses, and the one whose short name + // collides with the target's. That matters: making isSelfJoin identity-based + // while the hetero TARGET search was still a bare compare let that search + // re-match this very reference (nothing excludes sourceRef from it, unlike the + // directed self-join branch) and return (srcId, srcId) silently. Both searches + // are identity-based now, as in Java. + test("a cross-package hetero target sharing a subject short name is NOT a self-join", async () => { + const aDoc = { "metadata.root": { package: "a", children: [ + { "object.entity": { name: "Node", "extends": "a::NodeBase", children: [ + { "field.long": { name: "id" } }, + { "identity.primary": { "name": "id", "@fields": "id" } } ] } }, + { "object.entity": { name: "NodeBase", "@isAbstract": true, children: [ + { "relationship.association": { name: "links", "@cardinality": "many", + "@objectRef": "b::NodeBase", "@through": "L" } } ] } }, + { "object.entity": { name: "L", children: [ + { "field.long": { name: "srcId" } }, + { "field.long": { name: "dstId" } }, + { "identity.primary": { "name": "id", "@fields": ["srcId", "dstId"] } }, + { "identity.reference": { name: "s", "@fields": ["srcId"], "@references": "a::NodeBase" } }, + { "identity.reference": { name: "d", "@fields": ["dstId"], "@references": "b::NodeBase" } } ] } }, + ] } }; + const bDoc = { "metadata.root": { package: "b", children: [ + { "object.entity": { name: "NodeBase", children: [ + { "field.long": { name: "id" } }, + { "identity.primary": { "name": "id", "@fields": "id" } } ] } }, + ] } }; + const { root, errors } = await new MetaDataLoader().load([ + new InMemoryStringSource(JSON.stringify(aDoc), { id: "a.json" }), + new InMemoryStringSource(JSON.stringify(bDoc), { id: "b.json" }), + ]); + // The model itself is perfectly legal — the loader raises nothing. + expect(errors).toHaveLength(0); + expect(objectVisitOrder(root)).toEqual(["a::Node", "a::NodeBase", "a::L", "b::NodeBase"]); + + const node = findObj(root, "Node"); + const rel = node.relationships().find((r) => r.name === "links") as MetaRelationship; + // Identity resolution binds "b::NodeBase" to the b-package entity, which is + // neither subject — so this stays hetero and derives, exactly as Java does. + const derived = deriveM2MFields(rel, node, root); + expect(derived.sourceField).toBe("srcId"); + expect(derived.targetField).toBe("dstId"); + }); + + // Java's deriveCrossPackageHeteroBindsCorrectPackage model, ported. No inheritance + // is needed to reach the same defect: `a::Account` relates to `b::Account` through a + // junction holding one reference to each. The two junction searches are INDEPENDENT, + // so a bare-name target match found `ownerRef` a second time and returned + // (ownerId, ownerId). Measured before the fix; pre-branch it threw loudly instead. + test("cross-package hetero matches each junction reference to its OWN entity", async () => { + const aDoc = { "metadata.root": { package: "a", children: [ + { "object.entity": { name: "Account", children: [ + { "field.long": { name: "id" } }, + { "relationship.association": { name: "partners", "@cardinality": "many", + "@objectRef": "b::Account", "@through": "AccountLink" } }, + { "identity.primary": { "name": "id", "@fields": "id" } } ] } }, + { "object.entity": { name: "AccountLink", children: [ + { "field.long": { name: "ownerId" } }, + { "field.long": { name: "partnerId" } }, + { "identity.primary": { "name": "id", "@fields": ["ownerId", "partnerId"] } }, + { "identity.reference": { name: "ownerRef", "@fields": ["ownerId"], "@references": "a::Account" } }, + { "identity.reference": { name: "partnerRef", "@fields": ["partnerId"], "@references": "b::Account" } } ] } }, + ] } }; + const bDoc = { "metadata.root": { package: "b", children: [ + { "object.entity": { name: "Account", children: [ + { "field.long": { name: "id" } }, + { "identity.primary": { "name": "id", "@fields": "id" } } ] } }, + ] } }; + const { root, errors } = await new MetaDataLoader().load([ + new InMemoryStringSource(JSON.stringify(aDoc), { id: "a.json" }), + new InMemoryStringSource(JSON.stringify(bDoc), { id: "b.json" }), + ]); + expect(errors).toHaveLength(0); + const account = root.objects().find((o) => o.resolutionKey() === "a::Account")!; + const rel = account.relationships().find((r) => r.name === "partners") as MetaRelationship; + const derived = deriveM2MFields(rel, account, root); + expect(derived.sourceField).toBe("ownerId"); + expect(derived.targetField).toBe("partnerId"); // was "ownerId" + }); + + test("an OWN relationship still derives against its own entity (no regression)", async () => { + // The override case: Sub re-declares `peers` itself, so rel.parent IS Sub and + // the self-join must be classified against Sub, not the base it shadows. + const { root, errors } = await loadDoc({ "metadata.root": { package: "acme", children: [ + { "object.entity": { name: "Sub", "extends": "Base", children: [ + { "field.long": { name: "id" } }, + { "relationship.association": { name: "peers", "@cardinality": "many", "@objectRef": "Sub", + "@through": "SubLink", "@symmetric": true } }, + { "identity.primary": { "name": "id", "@fields": "id" } } ] } }, + { "object.entity": { name: "Base", "@isAbstract": true, children: [ + { "relationship.association": { name: "peers", "@cardinality": "many", "@objectRef": "Base", + "@through": "BaseLink", "@symmetric": true } } ] } }, + { "object.entity": { name: "SubLink", children: [ + { "field.long": { name: "id" } }, + { "field.long": { name: "leftId" } }, + { "field.long": { name: "rightId" } }, + { "identity.primary": { "name": "id", "@fields": "id" } }, + { "identity.reference": { name: "l", "@fields": ["leftId"], "@references": "Sub" } }, + { "identity.reference": { name: "r", "@fields": ["rightId"], "@references": "Sub" } } ] } }, + { "object.entity": { name: "BaseLink", children: [ + { "field.long": { name: "id" } }, + { "field.long": { name: "aId" } }, + { "field.long": { name: "bId" } }, + { "identity.primary": { "name": "id", "@fields": "id" } }, + { "identity.reference": { name: "a", "@fields": ["aId"], "@references": "Base" } }, + { "identity.reference": { name: "b", "@fields": ["bId"], "@references": "Base" } } ] } }, + ] } }); + expect(errors).toHaveLength(0); + const sub = findObj(root, "Sub"); + const rel = sub.relationships().find((r) => r.name === "peers") as MetaRelationship; + expect(rel.parent?.name).toBe("Sub"); + const derived = deriveM2MFields(rel, sub, root); + expect(derived.sourceField).toBe("leftId"); + expect(derived.targetField).toBe("rightId"); + }); +}); diff --git a/server/typescript/packages/runtime-ts/test/n2m-resolver.test.ts b/server/typescript/packages/runtime-ts/test/n2m-resolver.test.ts index 056c6d834..a55c4da16 100644 --- a/server/typescript/packages/runtime-ts/test/n2m-resolver.test.ts +++ b/server/typescript/packages/runtime-ts/test/n2m-resolver.test.ts @@ -243,3 +243,79 @@ describe("symmetric self-join — User.friends via Friendship (union on read)", expect((targetSpec!.where as { values: unknown[] }).values).toEqual([5]); }); }); + +// --------------------------------------------------------------------------- +// Inherited M:N — the runtime resolver reaches a relationship declared on a +// base entity (sourceEntity.children() is RESOLVING) and used to hand the +// derivation the VISITING entity. For a self-join declared on the base that +// read as hetero and threw; the runtime surfaced it as a MetadataError, so an +// inherited self-join was untraversable at run time, not just at codegen time. +// +// The child is declared BEFORE the base so the model's visit order is +// child-first — the shape the #368 loader regressions established. +// --------------------------------------------------------------------------- + +const INHERITED_SELF_JOIN = { + "metadata.root": { + package: "demo", + children: [ + { "object.entity": { name: "Node", extends: "NodeBase", children: [ + { "field.long": { name: "id" } }, + primary("id"), + ] } }, + { "object.entity": { name: "NodeBase", "@isAbstract": true, children: [ + relMany("peers", "NodeBase", "NodeLink", { "@symmetric": true }), + ] } }, + entity("NodeLink", ["aId", "bId"], [ + primary("aId", "bId"), + reference("a", "aId", "NodeBase"), + reference("b", "bId", "NodeBase"), + ]), + ], + }, +}; + +const INHERITED_HETERO = { + "metadata.root": { + package: "demo", + children: [ + { "object.entity": { name: "Article", extends: "ArticleBase", children: [ + { "field.long": { name: "id" } }, + primary("id"), + ] } }, + { "object.entity": { name: "ArticleBase", "@isAbstract": true, children: [ + relMany("tags", "Tag", "ArticleTag"), + ] } }, + entity("Tag", ["id", "name"], [primary("id")]), + entity("ArticleTag", ["articleId", "tagId"], [ + primary("articleId", "tagId"), + reference("articleRef", "articleId", "ArticleBase"), + reference("tagRef", "tagId", "Tag"), + ]), + ], + }, +}; + +describe("inherited M:N — derivation follows the DECLARING entity", () => { + test("self-join inherited via extends resolves on the child (was: MetadataError)", async () => { + const root = await load(INHERITED_SELF_JOIN); + // Pin the premise: Node is reached before NodeBase. + expect(root.ownChildren().filter((c) => c.type === "object").map((c) => c.name)) + .toEqual(["Node", "NodeBase", "NodeLink"]); + const node = root.ownChildByName("Node")!; + const desc = resolveN2mDescriptor(node, "peers", root)!; + expect(desc.sourceJoinField).toBe("aId"); + expect(desc.targetJoinField).toBe("bId"); + expect(desc.symmetric).toBe(true); + // The descriptor still names the entity actually being queried. + expect(desc.sourceEntityName).toBe("Node"); + }); + + test("hetero inherited via extends matches the base's junction reference", async () => { + const root = await load(INHERITED_HETERO); + const article = root.ownChildByName("Article")!; + const desc = resolveN2mDescriptor(article, "tags", root)!; + expect(desc.sourceJoinField).toBe("articleId"); + expect(desc.targetJoinField).toBe("tagId"); + }); +});