diff --git a/fixtures/template-output-render-conformance/README.md b/fixtures/template-output-render-conformance/README.md index 0afe975a9..e2ef1c8c6 100644 --- a/fixtures/template-output-render-conformance/README.md +++ b/fixtures/template-output-render-conformance/README.md @@ -26,6 +26,10 @@ resolves to `templates/emails/welcome.subject.mustache`, and so on. - `nested/meta.json` — a **no-package** sub-corpus carrying the nested/array email case (`Order` over `Customer` + `Item[]`); see "Nested + array email" below. It shares the same `templates/` dir. +- `xpkg-collision/` — a **multi-package** sub-corpus (`meta.alpha.json` + + `meta.beta.json` + `meta.app.json`) gating **FQN-exact** nested `@objectRef` + resolution across a cross-package short-name collision (ADR-0041); see + "Cross-package short-name collision" below. It also shares `templates/`. - `templates/emails/order.subject.mustache` = `Order for {{customer.name}}` - `templates/emails/order.html.mustache` = `

{{customer.name}}

{{> shared/footer}}` @@ -74,8 +78,10 @@ the XML entity set (`<`→`<`, `>`→`>`, `&`→`&`, `"`→`"`, ` @objectRef, isArray) }`, plus an `email` template `OrderEmail` (`@payloadRef=Order`) over the `order.*` part-refs. It has **no package** on purpose: a bare `@objectRef` resolves identically across ports only when there is -no package (TS resolves an objectRef by short name; the JVM expands a *packaged* -ref to an FQN, so bare == FQN only at the root package). The shared +no package: a bare `@objectRef` matches the value-object's short name in every +port, so this sub-corpus isolates the nested/array/partial shape from any +package-resolution concern. (The *packaged*/fully-qualified `@objectRef` case is +gated separately by `xpkg-collision/` below, per ADR-0041.) The shared `templates/emails/order.*` + `templates/shared/footer` mustaches are reused. Rendered with `{ customer: { name: "Ada" }, items: [ { sku: "A1", qty: 2 }, @@ -95,6 +101,38 @@ section-context drift (a `{{bogus}}` not on the `Item` element type the section pushes) FAILS codegen with `ERR_VAR_NOT_ON_PAYLOAD`, proving the gate walks the nested/section context, not just the root. +## Cross-package short-name collision — `xpkg-collision/`, `DigestDoc` + +A **multi-package** sub-corpus (loaded as three sources — `meta.alpha.json` + +`meta.beta.json` + `meta.app.json`, mirroring +`fixtures/conformance/loader-same-name-distinct-packages`) that gates **FQN-exact** +nested `@objectRef` resolution (ADR-0041): + +- `acme::alpha` declares `object.value Note { alphaText: string }`. +- `acme::beta` declares `object.value Note { betaText: string }` — a **colliding + short name** in a different package. +- `acme::app` declares payload `object.value Digest` with two `field.object` + children referencing the two Notes by **fully-qualified** `@objectRef` + (`acme::alpha::Note`, `acme::beta::Note`), and a `document` `template.output` + `DigestDoc` (`@format=html`, `@textRef="xpkg/digest"`, `@payloadRef="Digest"`). +- `templates/xpkg/digest.mustache` = `Alpha={{fromAlpha.alphaText}} Beta={{fromBeta.betaText}}` + +Each port must resolve a fully-qualified `@objectRef` **exactly** on the +package-qualified name — never a bare-tail fallback that binds whichever `Note` +loads first. A bare-tail resolver collapses BOTH refs to one package's `Note`, so +one of `{{fromAlpha.alphaText}}`/`{{fromBeta.betaText}}` lands on the wrong element +type and the build-time drift gate throws `ERR_VAR_NOT_ON_PAYLOAD`. FQN-exact +resolution binds each ref to its own package, so the clean template passes. + +Rendered with `{ fromAlpha: { alphaText: "AA" }, fromBeta: { betaText: "BB" } }`: + +- `renderDigestDoc(...)` = `"Alpha=AA Beta=BB"` + +(The two colliding VOs share the BARE payload-record type name `Note`; each port's +runner hand-authors — or otherwise reconciles — the payload record for that reason. +The record-name collision is an orthogonal concern; this sub-corpus gates the +`@objectRef` **resolver**, not payload-record naming.) + ## Expected build-time drift FAILURE — `drift/` `drift/meta.json` declares the same `Welcome` VO and a `document` diff --git a/fixtures/template-output-render-conformance/templates/xpkg/digest.mustache b/fixtures/template-output-render-conformance/templates/xpkg/digest.mustache new file mode 100644 index 000000000..52997ca73 --- /dev/null +++ b/fixtures/template-output-render-conformance/templates/xpkg/digest.mustache @@ -0,0 +1 @@ +Alpha={{fromAlpha.alphaText}} Beta={{fromBeta.betaText}} \ No newline at end of file diff --git a/fixtures/template-output-render-conformance/xpkg-collision/meta.alpha.json b/fixtures/template-output-render-conformance/xpkg-collision/meta.alpha.json new file mode 100644 index 000000000..dd60ca834 --- /dev/null +++ b/fixtures/template-output-render-conformance/xpkg-collision/meta.alpha.json @@ -0,0 +1,15 @@ +{ + "metadata.root": { + "package": "acme::alpha", + "children": [ + { + "object.value": { + "name": "Note", + "children": [ + { "field.string": { "name": "alphaText", "@required": true } } + ] + } + } + ] + } +} diff --git a/fixtures/template-output-render-conformance/xpkg-collision/meta.app.json b/fixtures/template-output-render-conformance/xpkg-collision/meta.app.json new file mode 100644 index 000000000..d544ff071 --- /dev/null +++ b/fixtures/template-output-render-conformance/xpkg-collision/meta.app.json @@ -0,0 +1,25 @@ +{ + "metadata.root": { + "package": "acme::app", + "children": [ + { + "object.value": { + "name": "Digest", + "children": [ + { "field.object": { "name": "fromAlpha", "@objectRef": "acme::alpha::Note" } }, + { "field.object": { "name": "fromBeta", "@objectRef": "acme::beta::Note" } } + ] + } + }, + { + "template.output": { + "name": "DigestDoc", + "@kind": "document", + "@payloadRef": "Digest", + "@textRef": "xpkg/digest", + "@format": "html" + } + } + ] + } +} diff --git a/fixtures/template-output-render-conformance/xpkg-collision/meta.beta.json b/fixtures/template-output-render-conformance/xpkg-collision/meta.beta.json new file mode 100644 index 000000000..74ddf76fa --- /dev/null +++ b/fixtures/template-output-render-conformance/xpkg-collision/meta.beta.json @@ -0,0 +1,15 @@ +{ + "metadata.root": { + "package": "acme::beta", + "children": [ + { + "object.value": { + "name": "Note", + "children": [ + { "field.string": { "name": "betaText", "@required": true } } + ] + } + } + ] + } +} diff --git a/server/csharp/MetaObjects.Codegen.Tests/PayloadCodegenTests.cs b/server/csharp/MetaObjects.Codegen.Tests/PayloadCodegenTests.cs index 90bf14d24..675cda8b1 100644 --- a/server/csharp/MetaObjects.Codegen.Tests/PayloadCodegenTests.cs +++ b/server/csharp/MetaObjects.Codegen.Tests/PayloadCodegenTests.cs @@ -87,6 +87,45 @@ public void Fully_qualified_objectRef_strips_to_bare_record_type_and_resolves_ne Assert.DoesNotContain("acme::ai::", src); } + // ADR-0041: cross-package short-name collision. Two object.value `Note`s + // (acme::alpha with alphaText, acme::beta with betaText) and a payload `Digest` + // referencing BOTH by FULLY-QUALIFIED @objectRef. The BuildPayloadFieldTree walk (the + // `dotnet meta verify` field-tree) must bind each ref to its OWN package — not + // bare-tail-collapse both to whichever Note loads first (pre-fix: both got alphaText). + // This is the CLI-verify half of the fix; the render-helper half is gated by + // RenderHelperConformanceTests.Document_DigestDoc_resolves_fqn_nested_objectRef_across_collision. + [Fact] + public void BuildPayloadFieldTree_resolves_fqn_nested_objectRef_across_package_collision() + { + const string alpha = """ + { "metadata.root": { "package": "acme::alpha", "children": [ + { "object.value": { "name": "Note", "children": [ { "field.string": { "name": "alphaText" } } ] } } ] } } + """; + const string beta = """ + { "metadata.root": { "package": "acme::beta", "children": [ + { "object.value": { "name": "Note", "children": [ { "field.string": { "name": "betaText" } } ] } } ] } } + """; + const string app = """ + { "metadata.root": { "package": "acme::app", "children": [ + { "object.value": { "name": "Digest", "children": [ + { "field.object": { "name": "fromAlpha", "@objectRef": "acme::alpha::Note" } }, + { "field.object": { "name": "fromBeta", "@objectRef": "acme::beta::Note" } } ] } } ] } } + """; + var root = new MetaDataLoader().Load([ + new InMemoryStringSource(alpha, id: "a.json"), + new InMemoryStringSource(beta, id: "b.json"), + new InMemoryStringSource(app, id: "c.json"), + ]).Root; + + var tree = PayloadCodegen.BuildPayloadFieldTree(root, "Digest"); + + var fromAlpha = Assert.Single(tree, f => f.Name == "fromAlpha"); + var fromBeta = Assert.Single(tree, f => f.Name == "fromBeta"); + // FQN-exact: each ref binds to its OWN package's Note. + Assert.Equal("alphaText", Assert.Single(fromAlpha.Fields!).Name); + Assert.Equal("betaText", Assert.Single(fromBeta.Fields!).Name); + } + [Fact] public void Emits_render_handle_binding_textRef_and_format() { diff --git a/server/csharp/MetaObjects.Codegen.Tests/RenderHelperConformanceTests.cs b/server/csharp/MetaObjects.Codegen.Tests/RenderHelperConformanceTests.cs index 5fb95d943..7daabefa7 100644 --- a/server/csharp/MetaObjects.Codegen.Tests/RenderHelperConformanceTests.cs +++ b/server/csharp/MetaObjects.Codegen.Tests/RenderHelperConformanceTests.cs @@ -50,6 +50,18 @@ private static MetaRoot LoadFromFile(string metaJsonPath) return r.Root; } + // Multi-file (multi-package) load — one InMemoryStringSource per file, merged into a + // single root (mirrors fixtures/conformance/loader-same-name-distinct-packages). + private static MetaRoot LoadFromFiles(params string[] metaJsonPaths) + { + var sources = metaJsonPaths + .Select((p, i) => (IMetaDataSource)new InMemoryStringSource(File.ReadAllText(p), id: $"corpus{i}.json")) + .ToArray(); + var r = new MetaDataLoader().Load(sources); + Assert.Empty(r.Errors); + return r.Root; + } + private static GenContext Ctx(MetaRoot root) => new() { Entities = root.Objects(), @@ -192,6 +204,53 @@ public void Email_OrderEmail_renders_nested_array_loop_and_partial() Assert.Contains("
Sent by Acme", email.HtmlBody); } + // --------------------------------------------------------------------- + // xpkg-collision/ — cross-package short-name collision (ADR-0041). Two packages + // each declare an object.value `Note` (alpha: alphaText, beta: betaText); the + // payload `Digest` references BOTH by FULLY-QUALIFIED @objectRef. A bare-tail + // resolver binds both refs to whichever Note loads first → one of + // {{fromAlpha.alphaText}}/{{fromBeta.betaText}} lands on the wrong element type and + // the build-time drift gate throws ERR_VAR_NOT_ON_PAYLOAD. FQN-exact resolution binds + // each ref to its own package. The two colliding VOs share the BARE record type name + // `Note`, so (like the TS payloads.ts) the payload record is hand-authored with both + // fields — the record-name collision is an orthogonal concern; this test gates the + // render-helper's FQN-exact @objectRef resolver. + // --------------------------------------------------------------------- + + [Fact] + public void Document_DigestDoc_resolves_fqn_nested_objectRef_across_collision() + { + var corpus = Corpus(); + var dir = Path.Combine(corpus, "xpkg-collision"); + var root = LoadFromFiles( + Path.Combine(dir, "meta.alpha.json"), + Path.Combine(dir, "meta.beta.json"), + Path.Combine(dir, "meta.app.json")); + var templates = Path.Combine(corpus, "templates"); + + // Must NOT throw: the FQN refs resolve to their own package's Note. + var file = Assert.Single(new RenderHelperGenerator(templates).Generate(Ctx(root)), f => f.Path == "DigestDoc.render.cs"); + + var payloadSrc = "namespace Acme.Generated;\n" + + "public sealed record Note { public string? alphaText { get; init; } public string? betaText { get; init; } }\n" + + "public sealed record Digest { public Note fromAlpha { get; init; } public Note fromBeta { get; init; } }\n"; + var asm = CompileToAssembly(file.Content, payloadSrc); + + var noteType = asm.GetType("Acme.Generated.Note")!; + var digestType = asm.GetType("Acme.Generated.Digest")!; + var fromAlpha = Activator.CreateInstance(noteType)!; + noteType.GetProperty("alphaText")!.SetValue(fromAlpha, "AA"); + var fromBeta = Activator.CreateInstance(noteType)!; + noteType.GetProperty("betaText")!.SetValue(fromBeta, "BB"); + var digest = Activator.CreateInstance(digestType)!; + digestType.GetProperty("fromAlpha")!.SetValue(digest, fromAlpha); + digestType.GetProperty("fromBeta")!.SetValue(digest, fromBeta); + + var helper = asm.GetType("Acme.Generated.DigestDocRenderHelper")!; + var outText = (string)helper.GetMethod("Render")!.Invoke(null, [digest, new FilesystemProvider(templates)])!; + Assert.Equal("Alpha=AA Beta=BB", outText); + } + // --------------------------------------------------------------------- // drift/ → the generator THROWS ERR_VAR_NOT_ON_PAYLOAD (fails codegen). // --------------------------------------------------------------------- diff --git a/server/csharp/MetaObjects.Codegen/Generators/RenderHelperGenerator.cs b/server/csharp/MetaObjects.Codegen/Generators/RenderHelperGenerator.cs index 51b3b49f2..50ee9ad17 100644 --- a/server/csharp/MetaObjects.Codegen/Generators/RenderHelperGenerator.cs +++ b/server/csharp/MetaObjects.Codegen/Generators/RenderHelperGenerator.cs @@ -22,10 +22,10 @@ // Reuse, not reimplementation: Renderer.Render (the emitted runtime call), Verify // (the build-time gate), FilesystemProvider (build-time ref resolution), and // EmailDocument (email return type). The payload field tree is walked from the VO -// the same way the other generators walk it — but a nested field.object's -// @objectRef is resolved by BARE short-name (cross-port render-helper consensus: -// TS findObject / Java resolveNestedObjectRef), only recursing into object.value -// (SUBTYPE_VALUE) targets, cycle-guarded. +// the same way the other generators walk it — a nested field.object's @objectRef is +// resolved FQN-exact when fully-qualified (ADR-0041) else by short name (cross-port +// render-helper consensus: TS refMatchesObject / Java+Kotlin resolveNestedObjectRef), +// only recursing into object.value (SUBTYPE_VALUE) targets, cycle-guarded. // // The emitted literal is baked into the RenderRequest.Verify argument // so Renderer's runtime drift check matches the build-time gate that ran here. @@ -231,16 +231,17 @@ private void GateRef(string templateName, string reference, IReadOnlyList DerivePayloadFieldTree( MetaRoot root, MetaData vo, HashSet seen) { - if (vo is null || !seen.Add(vo.Name)) return []; + if (vo is null || !seen.Add(vo.ResolutionKey())) return []; var fields = new List(); foreach (var f in vo.Children().Where(c => c.Type == TYPE_FIELD)) { @@ -263,20 +264,24 @@ private static IReadOnlyList DerivePayloadFieldTree( /// /// Resolve a field.object's @objectRef to its target - /// object.value by BARE short-name — mirroring the TS render-helper's - /// findObject(root, ref) (c.type === OBJECT && c.name === ref) - /// and the Java resolveNestedObjectRef. If the ref carries a package, only - /// the segment after the last :: is compared, against each - /// object.value's own short name. Returns null when no match is found. + /// object.value. ADR-0041: a FULLY-QUALIFIED ref (contains ::) resolves + /// EXACTLY on the package-qualified name (ResolutionKey()/Fqn()) — never + /// a bare-tail fallback that would bind a same-named object.value in the WRONG + /// package on a cross-package short-name collision. A bare ref still matches by short + /// name (first-wins). Mirrors the Java/Kotlin resolveNestedObjectRef + TS + /// refMatchesObject. Returns null when no match is found. /// private static MetaData? ResolveNestedObjectRef(MetaRoot root, string reference) { if (string.IsNullOrEmpty(reference)) return null; + bool fqn = reference.Contains("::"); var refShort = CSharpNaming.StripPkg(reference); // ADR-0039: Children() — resolving root scan (behavior-identical; root has no super). return root.Children().FirstOrDefault(c => c.Type == TYPE_OBJECT && c.SubType == OBJECT_SUBTYPE_VALUE && - CSharpNaming.StripPkg(c.Name) == refShort); + (fqn + ? c.ResolutionKey() == reference || c.Fqn() == reference + : CSharpNaming.StripPkg(c.Name) == refShort)); } /// diff --git a/server/csharp/MetaObjects.Codegen/PayloadCodegen.cs b/server/csharp/MetaObjects.Codegen/PayloadCodegen.cs index a8fc84f60..b8d0f56ec 100644 --- a/server/csharp/MetaObjects.Codegen/PayloadCodegen.cs +++ b/server/csharp/MetaObjects.Codegen/PayloadCodegen.cs @@ -43,8 +43,23 @@ public static class PayloadCodegen private static MetaData? FindObject(MetaData root, string name) => // ADR-0039: Children() — resolving root scan (behavior-identical; root has no super). + // Record emission resolves the payload VO by BARE short name (a C# record identifier + // is always bare, and EmitRecord derives the record name from this same voName). root.Children().FirstOrDefault(c => c.Type == TYPE_OBJECT && c.Name == name); + // ADR-0041: the verify field-tree resolver — a FULLY-QUALIFIED ref (contains ::) resolves + // EXACTLY on the package-qualified name (ResolutionKey()/Fqn()), never a bare-tail fallback + // that would bind a same-named object.value in the WRONG package on a cross-package + // short-name collision. A bare ref matches by short name (first-wins). Kept SEPARATE from + // FindObject so record emission (bare identifiers) is unaffected. Mirrors the render-helper + // ResolveNestedObjectRef + Java/Kotlin + TS refMatchesObject. + private static MetaData? ResolveObjectRef(MetaData root, string reference) + { + bool fqn = reference.Contains("::"); + return root.Children().FirstOrDefault(c => c.Type == TYPE_OBJECT && + (fqn ? c.ResolutionKey() == reference || c.Fqn() == reference : c.Name == reference)); + } + // ADR-0039: resolve array-ness through the super chain (isArray is a native // property, not an attr; the former OwnAttr("isArray") clause was dead code). private static bool IsArrayField(MetaData field) => field.ResolvedIsArray(); @@ -55,9 +70,10 @@ private static (string Type, string? RefVo) FieldType(MetaData owner, MetaData f { // ADR-0039: resolving — @objectRef may be inherited via extends. var refAttr = field.Attr(FIELD_ATTR_OBJECT_REF); - // @objectRef may be authored fully-qualified (acme::sales::Brief) or bare; - // the generated record type + the nested-record lookup key are the BARE - // short name (FindObject matches bare names — every other caller strips too). + // @objectRef may be authored fully-qualified (acme::sales::Brief) or bare; the + // generated record TYPE name is the BARE short name (StripPkg). (The verify + // field-tree path — BuildTree — resolves the FULL ref FQN-exact via ResolveObjectRef + // per ADR-0041; record emission here stays bare: one C# type per short name.) string refName = refAttr is string s ? CSharpNaming.StripPkg(s) : "object"; string? refVo = refAttr is string r ? CSharpNaming.StripPkg(r) : null; return (IsArrayField(field) ? $"IReadOnlyList<{refName}>" : refName, refVo); @@ -155,14 +171,17 @@ public static IReadOnlyList BuildPayloadFieldTree(MetaData root, s private static IReadOnlyList BuildTree(MetaData root, string voName, HashSet visiting) { - var vo = FindObject(root, voName); + // ADR-0041: FQN-exact resolution for the verify field-tree (NOT the bare FindObject + // used by record emission) so a fully-qualified nested @objectRef binds its own package. + var vo = ResolveObjectRef(root, voName); if (vo is null || !visiting.Add(voName)) return []; var fields = new List(); foreach (var f in vo.Children().Where(c => c.Type == TYPE_FIELD)) { // ADR-0039: resolving — @objectRef may be inherited via extends (TS reads f.attr). + // ADR-0041: pass the FULL (possibly FQN) ref — ResolveObjectRef resolves it exactly. if (f.SubType == FIELD_SUBTYPE_OBJECT && f.Attr(FIELD_ATTR_OBJECT_REF) is string refName) - fields.Add(new PayloadField(f.Name, BuildTree(root, CSharpNaming.StripPkg(refName), visiting))); + fields.Add(new PayloadField(f.Name, BuildTree(root, refName, visiting))); else fields.Add(new PayloadField(f.Name)); } diff --git a/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinRenderHelperConformanceTest.kt b/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinRenderHelperConformanceTest.kt index e6f86ee3b..71cca504b 100644 --- a/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinRenderHelperConformanceTest.kt +++ b/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinRenderHelperConformanceTest.kt @@ -201,6 +201,43 @@ class KotlinRenderHelperConformanceTest { } } + // ------------------------------------------------------------------------- + // xpkg-collision/ → FQN-exact nested @objectRef resolution across a cross-package + // short-name collision (ADR-0041). Two packages each declare an object.value `Note` + // (alpha: alphaText, beta: betaText); the payload `Digest` references BOTH by + // FULLY-QUALIFIED @objectRef. A bare-tail resolver binds both refs to whichever Note + // loads first → one of {{fromAlpha.alphaText}}/{{fromBeta.betaText}} lands on the wrong + // element type and the drift gate throws ERR_VAR_NOT_ON_PAYLOAD. Kotlin's render helper + // already resolves FQN-exact, so generation must NOT throw and must emit the helper. + // Loaded as three sources (multi-package), mirroring loader-same-name-distinct-packages. + // ------------------------------------------------------------------------- + @Test fun `xpkg collision resolves FQN nested objectRef`() { + val outDir = Files.createTempDirectory("krhc-xpkg-") + val dir = corpus.resolve("xpkg-collision") + val templates = corpus.resolve("templates") + try { + val loader = com.metaobjects.loader.MetaDataLoader.createManual(false, "rh-conf-xpkg") + loader.init() + loader.load(listOf( + com.metaobjects.loader.InMemoryStringSource(Files.readString(dir.resolve("meta.alpha.json")), "alpha"), + com.metaobjects.loader.InMemoryStringSource(Files.readString(dir.resolve("meta.beta.json")), "beta"), + com.metaobjects.loader.InMemoryStringSource(Files.readString(dir.resolve("meta.app.json")), "app"), + )) + loader.register() + // Must NOT throw: the FQN refs resolve to their own package's Note. + KotlinRenderHelperGenerator().apply { + setArgs(mapOf("outputDir" to outDir.toString(), "templateRoot" to templates.toString())) + }.execute(loader) + val produced = Files.walk(outDir).filter { it.isRegularFile() }.toList() + assertTrue( + produced.any { it.fileName.toString() == "DigestDocRenderHelper.kt" }, + "DigestDocRenderHelper.kt must be generated; files=$produced", + ) + } finally { + outDir.toFile().deleteRecursively() + } + } + // ------------------------------------------------------------------------- // drift/ → the GENERATOR throws GeneratorException (ERR_VAR_NOT_ON_PAYLOAD). // ------------------------------------------------------------------------- diff --git a/server/python/src/metaobjects/codegen/generators/render_helper_generator.py b/server/python/src/metaobjects/codegen/generators/render_helper_generator.py index 1811a7b61..f469c279f 100644 --- a/server/python/src/metaobjects/codegen/generators/render_helper_generator.py +++ b/server/python/src/metaobjects/codegen/generators/render_helper_generator.py @@ -26,10 +26,10 @@ Reuse, not reimplementation: ``render`` (the emitted runtime call), ``verify`` (the build-time gate), ``FilesystemProvider`` (build-time ref resolution), and ``EmailDocument`` (email return type). The payload field tree is walked from the VO -the same way the other generators walk it — but a nested ``field.object``'s -``@objectRef`` is resolved by BARE short-name (cross-port render-helper consensus: -TS ``findObject`` / Java ``resolveNestedObjectRef`` / C# ``ResolveNestedObjectRef``), -only recursing into ``object.value`` targets, cycle-guarded. +the same way the other generators walk it — a nested ``field.object``'s ``@objectRef`` +is resolved FQN-exact when fully-qualified (ADR-0041) else by short name (cross-port +render-helper consensus: TS ``refMatchesObject`` / Java+Kotlin ``resolveNestedObjectRef`` +/ C# ``ResolveNestedObjectRef``), only recursing into ``object.value`` targets, cycle-guarded. Python divergence vs TS / Java / C#: Python's ``RenderRequest`` has NO ``verify`` field — the Python render engine does not run a runtime drift pass — so the emitted @@ -75,19 +75,24 @@ def _py_str(s: str) -> str: # --------------------------------------------------------------------------- -# Payload field-tree walk — nested @objectRef resolved by BARE short-name -# (cross-port render-helper consensus). Object-ref fields recurse into their -# target object.value (SUBTYPE_VALUE only); a `seen` set guards reference cycles. +# Payload field-tree walk — nested @objectRef resolved FQN-exact (ADR-0041). +# Object-ref fields recurse into their target object.value (SUBTYPE_VALUE only); +# a `seen` set guards reference cycles. # --------------------------------------------------------------------------- def _resolve_nested_object_ref(root: MetaData, reference: str) -> MetaObject | None: - """Resolve a ``field.object``'s ``@objectRef`` to its target ``object.value`` by BARE - short-name — mirroring the C# ``ResolveNestedObjectRef`` / TS ``findObject`` / Java - ``resolveNestedObjectRef``. If the ref carries a package, only the segment after the - last ``::`` is compared, against each ``object.value``'s own short name.""" + """Resolve a ``field.object``'s ``@objectRef`` to its target ``object.value``. + + ADR-0041: a FULLY-QUALIFIED ref (contains ``::``) resolves EXACTLY on the + package-qualified name (``resolution_key()``/``fqn()``) — never a bare-tail + fallback that would bind a same-named ``object.value`` in the WRONG package on a + cross-package short-name collision. A bare ref still matches by short name + (first-wins). Mirrors the Java/Kotlin ``resolveNestedObjectRef`` + TS + ``refMatchesObject``.""" if not reference: return None + fqn = PACKAGE_SEP in reference ref_short = reference.rsplit(PACKAGE_SEP, 1)[-1] # ADR-0039 sanctioned own: top-level scan on the loader ROOT (never extended, own == effective) for child in root.own_children(): @@ -95,7 +100,10 @@ def _resolve_nested_object_ref(root: MetaData, reference: str) -> MetaObject | N continue if child.sub_type != OBJECT_SUBTYPE_VALUE: continue - if child.name.rsplit(PACKAGE_SEP, 1)[-1] == ref_short: + if fqn: + if child.resolution_key() == reference or child.fqn() == reference: + return child + elif child.name.rsplit(PACKAGE_SEP, 1)[-1] == ref_short: return child return None @@ -107,9 +115,9 @@ def _derive_payload_field_tree( resolvable ``@objectRef`` to an ``object.value`` becomes a context-pushing node whose children are the target VO's tree (recursed, cycle-guarded). Every other field is a leaf.""" - if vo is None or vo.name in seen: + if vo is None or vo.resolution_key() in seen: return [] - next_seen = seen | {vo.name} + next_seen = seen | {vo.resolution_key()} fields: list[PayloadField] = [] for f in vo.children(): if f.type != TYPE_FIELD or not isinstance(f, MetaField): diff --git a/server/python/tests/codegen/test_render_helper_conformance.py b/server/python/tests/codegen/test_render_helper_conformance.py index a35cca664..0ea6cbd26 100644 --- a/server/python/tests/codegen/test_render_helper_conformance.py +++ b/server/python/tests/codegen/test_render_helper_conformance.py @@ -27,7 +27,7 @@ import pytest import metaobjects.core_types # noqa: F401 — side-effect: registers attr classes -from metaobjects import MetaDataLoader +from metaobjects import InMemoryStringSource, MetaDataLoader from metaobjects.codegen.config import GenConfig from metaobjects.codegen.generator import GenContext from metaobjects.codegen.generators.render_helper_generator import RenderHelperGenerator @@ -45,6 +45,15 @@ def _load_root(meta_json: Path): return res.root +def _load_root_from_files(*meta_jsons: Path): + # Multi-file (multi-package) load — one InMemoryStringSource per file, merged into a + # single root (mirrors fixtures/conformance/loader-same-name-distinct-packages). + sources = [InMemoryStringSource(p.read_text()) for p in meta_jsons] + res = MetaDataLoader().load(sources) + assert res.errors == [], res.errors + return res.root + + def _ctx(root) -> GenContext: return GenContext( entities=[], @@ -156,6 +165,35 @@ def test_email_order_email_renders_nested_array_loop_and_partial(tmp_path) -> No assert "
Sent by Acme" in doc.html_body +# --------------------------------------------------------------------------- +# xpkg-collision/ — cross-package short-name collision (ADR-0041). Two packages +# each declare an object.value `Note` (alpha: alphaText, beta: betaText); the +# payload `Digest` references BOTH by FULLY-QUALIFIED @objectRef. A bare-tail +# resolver binds both refs to whichever Note loads first → one field lands on the +# wrong element type → the drift gate raises. FQN-exact resolution renders both. +# --------------------------------------------------------------------------- + + +def test_document_digest_doc_resolves_fqn_nested_object_ref_across_collision(tmp_path) -> None: + dir_root = CORPUS / "xpkg-collision" + root = _load_root_from_files( + dir_root / "meta.alpha.json", + dir_root / "meta.beta.json", + dir_root / "meta.app.json", + ) + templates = str(CORPUS / "templates") + # Must NOT raise: the FQN refs resolve to their own package's Note. + files = [f for f in RenderHelperGenerator(templates).generate(_ctx(root)) + if f.path == "digest_doc_render_helper.py"] + assert len(files) == 1 + _materialize_and_import(files, tmp_path) + helper = import_module("_rh_conf_pkg.digest_doc_render_helper") + + payload = {"fromAlpha": {"alphaText": "AA"}, "fromBeta": {"betaText": "BB"}} + out = helper.render_digest_doc(payload, FilesystemProvider(templates)) + assert out == "Alpha=AA Beta=BB" + + # --------------------------------------------------------------------------- # drift/ → codegen FAILS (ValueError) with ERR_VAR_NOT_ON_PAYLOAD. # --------------------------------------------------------------------------- diff --git a/server/typescript/packages/cli/src/lib/payload-field-tree.ts b/server/typescript/packages/cli/src/lib/payload-field-tree.ts index 79f3eb76b..babe62f88 100644 --- a/server/typescript/packages/cli/src/lib/payload-field-tree.ts +++ b/server/typescript/packages/cli/src/lib/payload-field-tree.ts @@ -11,12 +11,16 @@ import { TYPE_FIELD, FIELD_SUBTYPE_OBJECT, FIELD_ATTR_OBJECT_REF, + refMatchesObject, } from "@metaobjectsdev/metadata"; import type { PayloadField } from "@metaobjectsdev/render"; function findObject(root: MetaData, name: string): MetaData | undefined { // ADR-0039: effective children — resolve rather than rely on root being unextended. - return root.children().find((c) => c.type === TYPE_OBJECT && c.name === name); + // ADR-0041: FQN-exact — a "::"-qualified @objectRef binds the exact package (never a + // bare-tail fallback), so a same-named object.value in another package can't be wrongly + // bound. The SAME resolver render-helper.ts's findObject uses (the two must not drift). + return root.children().find((c) => c.type === TYPE_OBJECT && refMatchesObject(c, name)); } /** diff --git a/server/typescript/packages/cli/test/unit/payload-field-tree.test.ts b/server/typescript/packages/cli/test/unit/payload-field-tree.test.ts index c398d92d4..ff2cdbd58 100644 --- a/server/typescript/packages/cli/test/unit/payload-field-tree.test.ts +++ b/server/typescript/packages/cli/test/unit/payload-field-tree.test.ts @@ -10,6 +10,14 @@ async function loadRoot(children: unknown[]) { return res.root; } +// Multi-file (multi-package) load — one InMemoryStringSource per doc, merged into a +// single root (mirrors fixtures/conformance/loader-same-name-distinct-packages). +async function loadRootFromDocs(...docs: unknown[]) { + const res = await new MetaDataLoader().load(docs.map((d) => new InMemoryStringSource(JSON.stringify(d)))); + expect(res.errors).toEqual([]); + return res.root; +} + const model = [ { "object.value": { name: "PostBrief", children: [{ "field.string": { name: "title" } }] } }, { @@ -46,4 +54,25 @@ describe("derivePayloadFieldTree — mirrors the payload-codegen VO walk", () => const root = await loadRoot(model); expect(derivePayloadFieldTree(root, "Nope")).toEqual([]); }); + + // ADR-0041: two packages each declare an object.value `Note` (alpha: alphaText, beta: + // betaText), and `Digest` references BOTH by FULLY-QUALIFIED @objectRef. Pre-fix the + // resolver matched the ref by raw bare name, so an FQN ref matched NOTHING and the nested + // subtree was empty; now it binds the exact package. + test("FQN nested @objectRef binds the exact package across a cross-package short-name collision", async () => { + const root = await loadRootFromDocs( + { "metadata.root": { package: "acme::alpha", children: [ + { "object.value": { name: "Note", children: [{ "field.string": { name: "alphaText" } }] } } ] } }, + { "metadata.root": { package: "acme::beta", children: [ + { "object.value": { name: "Note", children: [{ "field.string": { name: "betaText" } }] } } ] } }, + { "metadata.root": { package: "acme::app", children: [ + { "object.value": { name: "Digest", children: [ + { "field.object": { name: "fromAlpha", "@objectRef": "acme::alpha::Note" } }, + { "field.object": { name: "fromBeta", "@objectRef": "acme::beta::Note" } } ] } } ] } }, + ); + expect(derivePayloadFieldTree(root, "Digest")).toEqual([ + { name: "fromAlpha", fields: [{ name: "alphaText" }] }, + { name: "fromBeta", fields: [{ name: "betaText" }] }, + ]); + }); }); diff --git a/server/typescript/packages/codegen-ts/src/generators/template-payload-tree.ts b/server/typescript/packages/codegen-ts/src/generators/template-payload-tree.ts index 7523de057..0224c0db2 100644 --- a/server/typescript/packages/codegen-ts/src/generators/template-payload-tree.ts +++ b/server/typescript/packages/codegen-ts/src/generators/template-payload-tree.ts @@ -21,13 +21,25 @@ import { type MetaObject, type MetaRoot, + TYPE_OBJECT, FIELD_SUBTYPE_OBJECT, FIELD_ATTR_OBJECT_REF, stripPackage, + refMatchesObject, } from "@metaobjectsdev/metadata"; import { isFieldRequired, neutralTypeStr } from "./docs-data-builder.js"; import type { AnnotatePayloadField } from "./template-source-annotate.js"; +// ADR-0041: FQN-exact object resolution — a "::"-qualified @objectRef binds the exact +// package (never a bare-tail fallback), so a same-named object.value in another package +// can't be wrongly bound (which would attach the wrong owner/type/link to a doc node). +// Mirrors render-helper.ts's findObject + the CLI payload-field-tree resolver. +function findObject(root: MetaRoot, ref: string): MetaObject | undefined { + return root.children().find((c) => c.type === TYPE_OBJECT && refMatchesObject(c, ref)) as + | MetaObject + | undefined; +} + /** * Walk the payload VO `voName` into an enriched `AnnotatePayloadField[]`. Each * node carries `owner` (the short name of the VO that DECLARES the field — the @@ -45,7 +57,7 @@ export function buildEnrichedPayloadTree( seen: ReadonlySet = new Set(), ): AnnotatePayloadField[] { if (seen.has(voName)) return []; - const vo: MetaObject | undefined = root.findObject(voName); + const vo: MetaObject | undefined = findObject(root, voName); if (vo === undefined) return []; const owner = stripPackage(vo.name); const nextSeen = new Set(seen).add(voName); @@ -61,8 +73,9 @@ export function buildEnrichedPayloadTree( const ref = f.attr(FIELD_ATTR_OBJECT_REF); if (typeof ref === "string" && ref.length > 0) { // Owner switches to the nested VO for its fields (the recursion below - // re-derives `owner` from the resolved VO's own name). - node.fields = buildEnrichedPayloadTree(root, stripPackage(ref), nextSeen); + // re-derives `owner` from the resolved VO's own name). Pass the FULL ref + // (not stripPackage) so findObject resolves it FQN-exact (ADR-0041). + node.fields = buildEnrichedPayloadTree(root, ref, nextSeen); } } out.push(node); diff --git a/server/typescript/packages/codegen-ts/test/render-helper-conformance.test.ts b/server/typescript/packages/codegen-ts/test/render-helper-conformance.test.ts index c35b410bf..739561b5d 100644 --- a/server/typescript/packages/codegen-ts/test/render-helper-conformance.test.ts +++ b/server/typescript/packages/codegen-ts/test/render-helper-conformance.test.ts @@ -34,6 +34,15 @@ async function loadRootFromFile(metaJsonPath: string) { return res.root; } +// Multi-file (multi-package) load — one InMemoryStringSource per file, merged into +// a single root (mirrors fixtures/conformance/loader-same-name-distinct-packages). +async function loadRootFromFiles(...metaJsonPaths: string[]) { + const sources = metaJsonPaths.map((p) => new InMemoryStringSource(readFileSync(p, "utf-8"))); + const res = await new MetaDataLoader().load(sources); + expect(res.errors).toEqual([]); + return res.root; +} + describe("render-helper conformance — shared cross-port corpus", () => { test("document WelcomePage renders \"Hello Ada\"", async () => { const root = await loadRootFromFile(join(CORPUS, "meta.json")); @@ -97,10 +106,10 @@ describe("render-helper conformance — shared cross-port corpus", () => { // builder + build-time drift gate handle the nested/array shape (clean → no // throw) and that section loops + partials render for email. test("email OrderEmail renders nested customer + array items loop + partial footer", async () => { - // The nested/array VOs live in nested/meta.json (a NO-PACKAGE sub-corpus) so - // a BARE @objectRef resolves identically in both ports' render-helper field-tree - // walks (TS resolves objectRef by short name; the JVM expands a packaged ref to - // an FQN — bare == FQN only when there is no package). Both share templates/. + // The nested/array VOs live in nested/meta.json (a NO-PACKAGE sub-corpus) so a + // BARE @objectRef resolves by short name identically across ports — isolating the + // nested/array/partial shape from package resolution. (The fully-qualified @objectRef + // case is gated separately by xpkg-collision/ below, per ADR-0041.) Both share templates/. const root = await loadRootFromFile(join(CORPUS, "nested", "meta.json")); const provider = new FileSystemProvider(join(CORPUS, "templates")); // The clean nested template must pass the build-time drift gate (no throw). @@ -131,6 +140,43 @@ describe("render-helper conformance — shared cross-port corpus", () => { expect(email.htmlBody).toContain("
Sent by Acme"); }); + // Cross-package short-name collision (ADR-0041): xpkg-collision/ declares an + // object.value `Note` in BOTH acme::alpha (field alphaText) and acme::beta (field + // betaText), and a payload `Digest` whose two field.object children reference them + // by FULLY-QUALIFIED @objectRef (acme::alpha::Note / acme::beta::Note). A bare-tail + // resolver strips both refs to `Note` and binds BOTH to whichever package's Note + // loads first, so exactly one of {{fromAlpha.alphaText}}/{{fromBeta.betaText}} lands + // on the wrong element type and the build-time drift gate throws ERR_VAR_NOT_ON_PAYLOAD. + // FQN-exact resolution binds each ref to its own package, so the clean template passes + // and renders both fields. Render-tier analogue of loader-same-name-distinct-packages. + test("document DigestDoc resolves FQN nested @objectRef across a cross-package short-name collision", async () => { + const dirRoot = join(CORPUS, "xpkg-collision"); + const root = await loadRootFromFiles( + join(dirRoot, "meta.alpha.json"), + join(dirRoot, "meta.beta.json"), + join(dirRoot, "meta.app.json"), + ); + const provider = new FileSystemProvider(join(CORPUS, "templates")); + // Must NOT throw: the FQN refs resolve to their own package's Note. + const src = renderRenderHelper(root, "DigestDoc", provider); + + const dir = mkdtempSync(join(import.meta.dir, "rh-conf-xpkg-")); + TEMP_DIRS.push(dir); + writeFileSync( + join(dir, "payloads.ts"), + "export interface Note { alphaText?: string; betaText?: string; }\n" + + "export interface Digest { fromAlpha: Note; fromBeta: Note; }\n", + ); + writeFileSync(join(dir, "DigestDoc.render.ts"), src); + + const mod = await import(join(dir, "DigestDoc.render.ts")); + const out: string = mod.renderDigestDoc( + { fromAlpha: { alphaText: "AA" }, fromBeta: { betaText: "BB" } }, + provider, + ); + expect(out).toBe("Alpha=AA Beta=BB"); + }); + test("drift case THROWS ERR_VAR_NOT_ON_PAYLOAD (fails codegen)", async () => { const root = await loadRootFromFile(join(CORPUS, "drift", "meta.json")); const provider = new FileSystemProvider(join(CORPUS, "drift", "templates")); diff --git a/server/typescript/packages/codegen-ts/test/template-payload-tree.test.ts b/server/typescript/packages/codegen-ts/test/template-payload-tree.test.ts new file mode 100644 index 000000000..99160a713 --- /dev/null +++ b/server/typescript/packages/codegen-ts/test/template-payload-tree.test.ts @@ -0,0 +1,37 @@ +import { describe, test, expect } from "bun:test"; +import { MetaDataLoader, InMemoryStringSource } from "@metaobjectsdev/metadata"; +import { buildEnrichedPayloadTree } from "../src/generators/template-payload-tree.js"; + +// Multi-file (multi-package) load — one InMemoryStringSource per doc, merged into a +// single root (mirrors fixtures/conformance/loader-same-name-distinct-packages). +async function loadRootFromDocs(...docs: unknown[]) { + const res = await new MetaDataLoader().load(docs.map((d) => new InMemoryStringSource(JSON.stringify(d)))); + expect(res.errors).toEqual([]); + return res.root; +} + +describe("buildEnrichedPayloadTree — docs annotator payload walk", () => { + // ADR-0041: two packages each declare an object.value `Note` (alpha: alphaText, beta: + // betaText), and `Digest` references BOTH by FULLY-QUALIFIED @objectRef. Pre-fix the + // annotator did stripPackage(ref) then a bare root.findObject, binding BOTH refs to + // whichever Note loaded first — so the doc node for fromBeta showed alpha's field. + // FQN-exact resolution binds each ref to its own package. + test("FQN nested @objectRef binds the exact package across a cross-package short-name collision", async () => { + const root = await loadRootFromDocs( + { "metadata.root": { package: "acme::alpha", children: [ + { "object.value": { name: "Note", children: [{ "field.string": { name: "alphaText" } }] } } ] } }, + { "metadata.root": { package: "acme::beta", children: [ + { "object.value": { name: "Note", children: [{ "field.string": { name: "betaText" } }] } } ] } }, + { "metadata.root": { package: "acme::app", children: [ + { "object.value": { name: "Digest", children: [ + { "field.object": { name: "fromAlpha", "@objectRef": "acme::alpha::Note" } }, + { "field.object": { name: "fromBeta", "@objectRef": "acme::beta::Note" } } ] } } ] } }, + ); + const tree = buildEnrichedPayloadTree(root, "Digest"); + const alpha = tree.find((n) => n.name === "fromAlpha"); + const beta = tree.find((n) => n.name === "fromBeta"); + // The inner field name comes from the correctly-bound VO (pre-fix both were alphaText). + expect(alpha?.fields?.map((f) => f.name)).toEqual(["alphaText"]); + expect(beta?.fields?.map((f) => f.name)).toEqual(["betaText"]); + }); +}); diff --git a/spec/decisions/ADR-0041-cross-package-reference-resolution.md b/spec/decisions/ADR-0041-cross-package-reference-resolution.md index 636890e5f..db8079338 100644 --- a/spec/decisions/ADR-0041-cross-package-reference-resolution.md +++ b/spec/decisions/ADR-0041-cross-package-reference-resolution.md @@ -34,6 +34,7 @@ The "referrer's package" is the package of the node carrying the reference. This - **New error code `ERR_AMBIGUOUS_REF`**, registered cross-port (added to each port's error-code set and `fixtures/conformance/ERROR-CODES.json`). - **Resolvers become referrer-package-aware.** Each port routes object-reference resolution through one shared helper (`resolveObjectRef(root, ref, referrerPkg)` in TS) so relationship/reference/origin/extends/template resolution behave identically; the referrer's package is threaded to each call site. Java's bare-tail-before-FQN ordering (`SymbolTable.nameMatches`, `ValidationPhase.nameMatches`) and the Java codegen/runtime helpers that shared it (`SpringM2mSupport.findEntity`, `KotlinRenderHelperGenerator`, and the M:N FK-derivation SSOT `M2MFields` + runtime `M2MResolver`) are corrected to resolve FQNs exactly; `SpringPayloadGenerator`/`KotlinGenUtil` were already FQN-safe and unchanged. The remaining *bare*-collision same-package preference in codegen/runtime is a documented follow-up (#174). +- **Render/verify codegen tier (non-JVM follow-on, 2026-07-06).** The loader-level fix above corrected the JVM render helper (`KotlinRenderHelperGenerator`); the equivalent nested-`@objectRef` field-tree resolvers on the **non-JVM** ports still bare-tail-collapsed a *fully-qualified* nested `@objectRef` — latent until a cross-package short-name collision. They now resolve FQN-exact too: the render-helper generator (C# `RenderHelperGenerator`, Python `render_helper_generator`), the `verify` payload-field-tree (TS `cli` `payload-field-tree`, C# `PayloadCodegen.BuildTree`), and the TS docs-annotator (`codegen-ts` `template-payload-tree`). TS routes the two buggy resolvers through the shared `refMatchesObject` SSOT (its render helper was already FQN-safe); C# keeps a `verify`-only `ResolveObjectRef` *separate* from the bare `FindObject` used by payload-record emission, so generated record identifiers stay bare. Gated by the new `xpkg-collision/` multi-package sub-corpus in `fixtures/template-output-render-conformance/` (Java is already covered by `TemplateVerifyTest`'s own ADR-0041 collision case, so its heavy javac runner is unchanged). Only the FQN-exact half ports here; the *bare*-collision same-package preference remains the #174 follow-up. - **Conformance-gated.** New `fixtures/conformance/` fixtures cover every ref-bearing attribute cross-package (unique-name working cases) plus the collision contract: FQN-exact-across-collision (via `extends`, observable in effective serialization), bare same-package preference, and bare-ambiguous → `ERR_AMBIGUOUS_REF`. All five ports must serialize/err byte-identically. - **Breaking only for models that today rely on a bare reference silently binding to an arbitrarily-chosen same-named entity in another package** — previously undefined behavior, now an explicit `ERR_AMBIGUOUS_REF` that is fixed by qualifying the reference. No change for unique names or explicit FQNs.