From a1d3f7deda5d92a1be9eca7ee0552a9d47bcbf10 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Wed, 4 Nov 2026 01:08:59 -0500 Subject: [PATCH 1/6] feat(metadata): refuse two providers claiming one attribute name on a node type (#527) The registry now records which provider registered each attribute and refuses a second provider's claim at composition, in TypeScript, Python, C#, Java and Kotlin. - Same (type, subType): ERR_PROVIDER_ATTR_CONFLICT, message now names the owner. - Shared .base root or a descendant subtype: new ERR_PROVIDER_ATTR_SHADOW, checked in both directions so registration order does not matter. - The same provider repeating its own name, sibling subtypes, and registration outside a provider's turn stay legal. A refusal leaves nothing in the registry. - The rule is scoped to one node type so the JVM registry's metadata.base chain does not make Java stricter than the other ports; the shipped core has no collision. - meta types --detail prints the registering provider; JSON carries `provider`. - ADR-0062, extending-with-providers docs, authoring skill, CHANGELOG, and shared provider-composition fixtures (attr-shadow-*, attr-ownership/). No wire-name attribute is added. Fixes #527 --- CHANGELOG.md | 16 ++ .../references/choosing-a-shape.md | 8 + docs/CONFORMANCE.md | 2 +- docs/features/extending-with-providers.md | 26 ++- .../references/choosing-a-shape.md | 8 + .../references/choosing-a-shape.md | 8 + .../references/choosing-a-shape.md | 8 + .../references/choosing-a-shape.md | 8 + .../references/choosing-a-shape.md | 8 + fixtures/conformance/ERROR-CODES.json | 1 + .../README.md | 42 +++++ .../attr-ownership/distinct-names-load.json | 21 +++ .../same-provider-redeclare.json | 20 +++ .../attr-shadow-descendant.json | 8 + .../attr-shadow-inherited.json | 8 + scripts/check-doc-examples.ts | 1 + .../AttrOwnershipTests.cs | 150 +++++++++++++++++ .../ProviderCompositionConformanceTests.cs | 84 +++++++++ server/csharp/MetaObjects/Errors.cs | 3 + server/csharp/MetaObjects/Registry.cs | 64 +++++++ .../main/java/com/metaobjects/ErrorCode.java | 6 + .../registry/MetaDataRegistry.java | 159 ++++++++++++++++++ .../registry/TypeDefinitionBuilder.java | 11 +- .../ProviderCompositionConformanceTest.java | 120 +++++++++++++ .../registry/AttrOwnershipTest.java | 144 ++++++++++++++++ server/python/src/metaobjects/errors.py | 3 + server/python/src/metaobjects/registry.py | 46 +++++ .../test_provider_composition_conformance.py | 60 +++++++ .../unit/test_registry_attr_ownership.py | 117 +++++++++++++ .../packages/cli/src/commands/types.ts | 28 ++- .../packages/cli/test/types-command.test.ts | 23 +++ .../packages/metadata/src/errors.ts | 3 + .../packages/metadata/src/provider.ts | 4 +- .../packages/metadata/src/registry.ts | 112 +++++++++++- .../metadata/src/vocabulary-catalog.ts | 27 ++- .../provider-composition-conformance.test.ts | 82 ++++++++- .../test/registry-attr-ownership.test.ts | 125 ++++++++++++++ .../ADR-0062-provider-attribute-ownership.md | 82 +++++++++ spec/decisions/README.md | 1 + 39 files changed, 1626 insertions(+), 21 deletions(-) create mode 100644 fixtures/provider-composition-conformance/attr-ownership/distinct-names-load.json create mode 100644 fixtures/provider-composition-conformance/attr-ownership/same-provider-redeclare.json create mode 100644 fixtures/provider-composition-conformance/attr-shadow-descendant.json create mode 100644 fixtures/provider-composition-conformance/attr-shadow-inherited.json create mode 100644 server/csharp/MetaObjects.Conformance.Tests/AttrOwnershipTests.cs create mode 100644 server/java/metadata/src/test/java/com/metaobjects/registry/AttrOwnershipTest.java create mode 100644 server/python/tests/unit/test_registry_attr_ownership.py create mode 100644 server/typescript/packages/metadata/test/registry-attr-ownership.test.ts create mode 100644 spec/decisions/ADR-0062-provider-attribute-ownership.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d09f79179..398f233329 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,22 @@ here.** declared rule is now rejected at the model where it used to reach the database. Gated by `fixtures/validation-conformance/cross-field-cases.json` in all five ports. + +- **Two providers can no longer claim one attribute name on a node type.** The registry now + records which provider registered each attribute and refuses a second provider's claim at + composition, in all five ports (TypeScript, Python, C#, Java, Kotlin). A repeat on the same + `(type, subType)` keeps `ERR_PROVIDER_ATTR_CONFLICT` (its message now names the owner); a name + another provider already holds on the shared root or a descendant subtype is the new + `ERR_PROVIDER_ATTR_SHADOW`. The refusal names both providers and the attribute, whichever + registers first, and leaves nothing in the registry. The same provider repeating its own name, + sibling subtypes sharing a name, and registration outside a provider all stay legal. The shipped + core providers have no such collision, so existing projects see no change. `meta types --detail` + prints the registering provider beside each attribute, and its JSON carries a `provider` field. + This is how per-paradigm physical-name words (`@column`, `@field`) stay safe as providers + multiply; no wire-name attribute is added. See ADR-0062 and + `docs/features/extending-with-providers.md`. New conformance fixtures: `attr-shadow-inherited`, + `attr-shadow-descendant` and the `attr-ownership/` sub-corpus. Fixes #527. + ### Fixed - **TypeScript: every `meta` command now loads the providers in `metaobjects.config.ts`.** diff --git a/agent-context/skills/metaobjects-authoring/references/choosing-a-shape.md b/agent-context/skills/metaobjects-authoring/references/choosing-a-shape.md index ee3b6858a5..bb4979af93 100644 --- a/agent-context/skills/metaobjects-authoring/references/choosing-a-shape.md +++ b/agent-context/skills/metaobjects-authoring/references/choosing-a-shape.md @@ -186,6 +186,14 @@ and address-free — the subtype names the *concept*; transport/protocol is a cl Declare config as the *names* of required keys (values stay in env/config) and generate a fail-closed check. Reference typed payloads instead of inlining shapes. +**Name your attributes so they cannot collide.** Two providers may not claim one attribute +name on a node type (ADR-0062): the registry refuses the second at composition with +`ERR_PROVIDER_ATTR_CONFLICT` (same `(type, subType)`) or `ERR_PROVIDER_ATTR_SHADOW` (the shared +`.base` root, or a subtype of it), naming both providers. A physical address takes your +store's own idiomatic word (`@column` relational, `@field` document), unique on that node type; +`meta types --detail` shows which provider holds each name. A store with no provider of its own +uses the registered `attr.properties` bag rather than an invented attribute. + **Lifecycle.** Register with explicit provider wiring — never loosen `strict` to free-ride ad-hoc attrs. When core later ships the concept your provider shrinks from `register` to `extend` (the `template.toolcall` precedent). A second independent diff --git a/docs/CONFORMANCE.md b/docs/CONFORMANCE.md index 0addaf83d3..27cee9166f 100644 --- a/docs/CONFORMANCE.md +++ b/docs/CONFORMANCE.md @@ -45,7 +45,7 @@ regenerate with `ls -d fixtures//*/ | wc -l` for directory-shaped corpor | [`fixtures/template-codegen-conformance/`](../fixtures/template-codegen-conformance/) | 3 | ✓ | ✓ | ✓ | ✓ | ✓ | | [`fixtures/template-output-render-conformance/`](../fixtures/template-output-render-conformance/) | 5 | ✓ | ✓ | ✓ | ✓ | ✓ | | [`fixtures/generator-registry-conformance/`](../fixtures/generator-registry-conformance/) | 1 canonical manifest | ✓ | ✓ | ✓ | ✓ | ✓ | -| [`fixtures/provider-composition-conformance/`](../fixtures/provider-composition-conformance/) | 9 (5 error-shape + 4 compose-load) | ✓ | ✓ | — (JVM registry via Java) | ✓ | ✓ | +| [`fixtures/provider-composition-conformance/`](../fixtures/provider-composition-conformance/) | 13 (7 error-shape + 4 compose-load + 2 attr-ownership) | ✓ | ✓ | — (JVM registry via Java) | ✓ | ✓ | | [`fixtures/source-resolution-conformance/`](../fixtures/source-resolution-conformance/) | 25 cases | ✓ (reference implementation) | ✓ | inherits via Java | ✓ | ✓ | | [`fixtures/scope-conformance/`](../fixtures/scope-conformance/) | 10 cases | ✓ (reference implementation) | ✓ | inherits via Java | ✓ | ✓ | | [`fixtures/dependency-conformance/`](../fixtures/dependency-conformance/) | 37 cases (26 resolution + 11 check) + 14 transport cases | ✓ (reference implementation; all three arms) | ✓ (resolution arm, all 26; no check, so the 11 check cases are not run) | inherits via Java | ✓ (resolution arm, all 26; no check, so the 11 check cases are not run) | ✓ (resolution arm, all 26 identically; no check yet, so the 11 check cases are not run; the transport arm is Node-only) | diff --git a/docs/features/extending-with-providers.md b/docs/features/extending-with-providers.md index be13b96183..108f6c695d 100644 --- a/docs/features/extending-with-providers.md +++ b/docs/features/extending-with-providers.md @@ -39,9 +39,9 @@ unregistered id throws `ERR_PROVIDER_MISSING_DEPENDENCY`; a cycle throws `ERR_PROVIDER_DUPLICATE_ID`. All three codes are stable across ports — see [`../../fixtures/conformance/ERROR-CODES.json`](../../fixtures/conformance/ERROR-CODES.json). -## The two rules that govern every provider +## The three rules that govern every provider -Before the mechanics, the two rules a provider must obey. Both are machine-enforced — +Before the mechanics, the three rules a provider must obey. Both are machine-enforced — you will hit an error, not a review comment. ### 1. Own vs projected attributes (ADR-0050) @@ -80,6 +80,25 @@ So vendor or domain vocabulary enters by **registration**: "Declared" includes "declared by your provider" — that is what keeps strict provenance meaningful while still letting you extend the metamodel. +### 3. One attribute name, one provider, per node type (ADR-0062) + +**Two providers may not claim the same attribute name on a node type.** The registry records +which provider registered each attribute, and refuses a second provider's claim at composition: + +- on the same `(type, subType)` — `ERR_PROVIDER_ATTR_CONFLICT`; +- on the shared root `(type, base)` that the subtype inherits from, or on a subtype of it — + `ERR_PROVIDER_ATTR_SHADOW`. + +The message names both providers and the attribute. The check runs both ways, so it does not +matter which provider registers first. The same provider repeating its own name, sibling +subtypes using one name, and registration outside a provider's turn are all legal. + +This is what keeps per-paradigm physical names safe (ADR-0007, ADR-0018). Name your store's +address in your store's own word (`@column` for a relational field, `@field` for a document +field) and pick a word no other provider on that node type already holds. A store you have no +provider for goes in the `attr.properties` bag, not an invented attribute. `meta types +--detail` prints the registering provider next to each attribute, so you can see what is taken. + ## What modularity does and does not mean > **MetaObjects modularity is additive and behavioural, never subtractive: every port @@ -499,7 +518,8 @@ Composition surfaces these codes consistently across all ports (see | `ERR_PROVIDER_DUPLICATE_ID` | Two provider objects report the same `id` | `provider-extension-duplicate-id` | | `ERR_PROVIDER_MISSING_DEPENDENCY` | Provider declares dep on an unregistered id | `provider-extension-missing-dependency` | | `ERR_PROVIDER_DEPENDENCY_CYCLE` | Provider dependencies form a cycle | `provider-extension-dependency-cycle` | -| `ERR_PROVIDER_ATTR_CONFLICT` | Two providers declare the same attr on the same `(type, subType)` | (covered by negative tests in core) | +| `ERR_PROVIDER_ATTR_CONFLICT` | Two providers declare the same attr on the same `(type, subType)` | `attr-conflict` (provider-composition corpus) | +| `ERR_PROVIDER_ATTR_SHADOW` | A provider declares an attr another provider already holds on the shared root or a descendant subtype of the same type | `attr-shadow-inherited`, `attr-shadow-descendant` (provider-composition corpus) | Each conformance fixture's `providers.json: string[]` lists the provider ids to compose for that test — the same wire format consumers use at runtime. Each diff --git a/fixtures/agent-context-conformance/java-kotlin-react-tanstack/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md b/fixtures/agent-context-conformance/java-kotlin-react-tanstack/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md index ee3b6858a5..bb4979af93 100644 --- a/fixtures/agent-context-conformance/java-kotlin-react-tanstack/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md +++ b/fixtures/agent-context-conformance/java-kotlin-react-tanstack/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md @@ -186,6 +186,14 @@ and address-free — the subtype names the *concept*; transport/protocol is a cl Declare config as the *names* of required keys (values stay in env/config) and generate a fail-closed check. Reference typed payloads instead of inlining shapes. +**Name your attributes so they cannot collide.** Two providers may not claim one attribute +name on a node type (ADR-0062): the registry refuses the second at composition with +`ERR_PROVIDER_ATTR_CONFLICT` (same `(type, subType)`) or `ERR_PROVIDER_ATTR_SHADOW` (the shared +`.base` root, or a subtype of it), naming both providers. A physical address takes your +store's own idiomatic word (`@column` relational, `@field` document), unique on that node type; +`meta types --detail` shows which provider holds each name. A store with no provider of its own +uses the registered `attr.properties` bag rather than an invented attribute. + **Lifecycle.** Register with explicit provider wiring — never loosen `strict` to free-ride ad-hoc attrs. When core later ships the concept your provider shrinks from `register` to `extend` (the `template.toolcall` precedent). A second independent diff --git a/fixtures/agent-context-conformance/java-react/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md b/fixtures/agent-context-conformance/java-react/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md index ee3b6858a5..bb4979af93 100644 --- a/fixtures/agent-context-conformance/java-react/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md +++ b/fixtures/agent-context-conformance/java-react/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md @@ -186,6 +186,14 @@ and address-free — the subtype names the *concept*; transport/protocol is a cl Declare config as the *names* of required keys (values stay in env/config) and generate a fail-closed check. Reference typed payloads instead of inlining shapes. +**Name your attributes so they cannot collide.** Two providers may not claim one attribute +name on a node type (ADR-0062): the registry refuses the second at composition with +`ERR_PROVIDER_ATTR_CONFLICT` (same `(type, subType)`) or `ERR_PROVIDER_ATTR_SHADOW` (the shared +`.base` root, or a subtype of it), naming both providers. A physical address takes your +store's own idiomatic word (`@column` relational, `@field` document), unique on that node type; +`meta types --detail` shows which provider holds each name. A store with no provider of its own +uses the registered `attr.properties` bag rather than an invented attribute. + **Lifecycle.** Register with explicit provider wiring — never loosen `strict` to free-ride ad-hoc attrs. When core later ships the concept your provider shrinks from `register` to `extend` (the `template.toolcall` precedent). A second independent diff --git a/fixtures/agent-context-conformance/python/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md b/fixtures/agent-context-conformance/python/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md index ee3b6858a5..bb4979af93 100644 --- a/fixtures/agent-context-conformance/python/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md +++ b/fixtures/agent-context-conformance/python/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md @@ -186,6 +186,14 @@ and address-free — the subtype names the *concept*; transport/protocol is a cl Declare config as the *names* of required keys (values stay in env/config) and generate a fail-closed check. Reference typed payloads instead of inlining shapes. +**Name your attributes so they cannot collide.** Two providers may not claim one attribute +name on a node type (ADR-0062): the registry refuses the second at composition with +`ERR_PROVIDER_ATTR_CONFLICT` (same `(type, subType)`) or `ERR_PROVIDER_ATTR_SHADOW` (the shared +`.base` root, or a subtype of it), naming both providers. A physical address takes your +store's own idiomatic word (`@column` relational, `@field` document), unique on that node type; +`meta types --detail` shows which provider holds each name. A store with no provider of its own +uses the registered `attr.properties` bag rather than an invented attribute. + **Lifecycle.** Register with explicit provider wiring — never loosen `strict` to free-ride ad-hoc attrs. When core later ships the concept your provider shrinks from `register` to `extend` (the `template.toolcall` precedent). A second independent diff --git a/fixtures/agent-context-conformance/ts-react-tanstack/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md b/fixtures/agent-context-conformance/ts-react-tanstack/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md index ee3b6858a5..bb4979af93 100644 --- a/fixtures/agent-context-conformance/ts-react-tanstack/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md +++ b/fixtures/agent-context-conformance/ts-react-tanstack/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md @@ -186,6 +186,14 @@ and address-free — the subtype names the *concept*; transport/protocol is a cl Declare config as the *names* of required keys (values stay in env/config) and generate a fail-closed check. Reference typed payloads instead of inlining shapes. +**Name your attributes so they cannot collide.** Two providers may not claim one attribute +name on a node type (ADR-0062): the registry refuses the second at composition with +`ERR_PROVIDER_ATTR_CONFLICT` (same `(type, subType)`) or `ERR_PROVIDER_ATTR_SHADOW` (the shared +`.base` root, or a subtype of it), naming both providers. A physical address takes your +store's own idiomatic word (`@column` relational, `@field` document), unique on that node type; +`meta types --detail` shows which provider holds each name. A store with no provider of its own +uses the registered `attr.properties` bag rather than an invented attribute. + **Lifecycle.** Register with explicit provider wiring — never loosen `strict` to free-ride ad-hoc attrs. When core later ships the concept your provider shrinks from `register` to `extend` (the `template.toolcall` precedent). A second independent diff --git a/fixtures/agent-context-conformance/ts-requirements/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md b/fixtures/agent-context-conformance/ts-requirements/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md index ee3b6858a5..bb4979af93 100644 --- a/fixtures/agent-context-conformance/ts-requirements/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md +++ b/fixtures/agent-context-conformance/ts-requirements/expected/.claude/skills/metaobjects-authoring/references/choosing-a-shape.md @@ -186,6 +186,14 @@ and address-free — the subtype names the *concept*; transport/protocol is a cl Declare config as the *names* of required keys (values stay in env/config) and generate a fail-closed check. Reference typed payloads instead of inlining shapes. +**Name your attributes so they cannot collide.** Two providers may not claim one attribute +name on a node type (ADR-0062): the registry refuses the second at composition with +`ERR_PROVIDER_ATTR_CONFLICT` (same `(type, subType)`) or `ERR_PROVIDER_ATTR_SHADOW` (the shared +`.base` root, or a subtype of it), naming both providers. A physical address takes your +store's own idiomatic word (`@column` relational, `@field` document), unique on that node type; +`meta types --detail` shows which provider holds each name. A store with no provider of its own +uses the registered `attr.properties` bag rather than an invented attribute. + **Lifecycle.** Register with explicit provider wiring — never loosen `strict` to free-ride ad-hoc attrs. When core later ships the concept your provider shrinks from `register` to `extend` (the `template.toolcall` precedent). A second independent diff --git a/fixtures/conformance/ERROR-CODES.json b/fixtures/conformance/ERROR-CODES.json index 278b1b50f0..914a67d4cf 100644 --- a/fixtures/conformance/ERROR-CODES.json +++ b/fixtures/conformance/ERROR-CODES.json @@ -28,6 +28,7 @@ "ERR_PROVIDER_DUPLICATE_ID": "Two providers in a composition share the same id.", "ERR_PROVIDER_MISSING_DEPENDENCY": "A provider declares a dependency id with no matching provider.", "ERR_PROVIDER_ATTR_CONFLICT": "A provider extend redefines an attr another provider already declared.", + "ERR_PROVIDER_ATTR_SHADOW": "A provider declares an attribute name on a type that another provider already declares on a related slot: the type it inherits from (.base), or a subtype that inherits from it (ADR-0062). Two providers may not claim one attribute name on a node type; the second needs its own idiomatic name. The same provider re-declaring its own attribute stays legal.", "ERR_EXTEND_REQUIRED_ATTR": "A provider tried to project a REQUIRED attribute onto a type it does not own (via extends). Projection is optional-only: a required attr registered this way disappears silently whenever that provider is composed out, taking its validation rule with it. Declare it with the type instead (ADR-0050).", "ERR_SUBTYPE_RULE_VIOLATION": "A node violates a subtype composition rule (e.g. a value object with an identity or a source — FR-024 value purity; a projection extending a non-projection — FR-024/ADR-0028).", "ERR_OVERLAY_NO_TARGET": "An overlay node has no existing target to merge into.", diff --git a/fixtures/provider-composition-conformance/README.md b/fixtures/provider-composition-conformance/README.md index 8ba95d0a90..e03d09181b 100644 --- a/fixtures/provider-composition-conformance/README.md +++ b/fixtures/provider-composition-conformance/README.md @@ -12,6 +12,7 @@ cross-port. | `ERR_PROVIDER_MISSING_DEPENDENCY` | A provider declares a dependency id absent from the set. | | `ERR_PROVIDER_DEPENDENCY_CYCLE` | Providers form a dependency cycle (topo-sort cannot proceed). | | `ERR_PROVIDER_ATTR_CONFLICT` | A provider `extend`s a type, redefining an attr another provider already declared. | +| `ERR_PROVIDER_ATTR_SHADOW` | A provider declares an attr name that another provider already declares on the type it inherits from (`.base`), or on a subtype that inherits from it (ADR-0062). | | `ERR_REGISTRY_SEALED` | A registration is attempted against a registry sealed after bootstrap (ADR-0023). | ## Corpus shape @@ -112,6 +113,47 @@ Composing `["attr-conflict-base", "attr-conflict-clash"]` → `ERR_PROVIDER_ATTR sealed registry this must throw `ERR_REGISTRY_SEALED`. This provider is **only** used via a manifest's `sealThenRegister` — never composed directly. +### Attribute-ownership providers (ADR-0062) + +All six register on the `template` type's shared root (`template.base`) and one fresh +subtype `template.shadowprobe`; each attr is a single optional string. They compose on +their own (no `composeWithCore`), so each registers the root it needs. + +| id | dependencies | registers | +|----|--------------|-----------| +| `shadow-base` | none | `template.base` with attr `shadowAttr`. | +| `shadow-sub-clash` | `shadow-base` | `template.shadowprobe` with attr `shadowAttr` (the name `shadow-base` owns on the root). | +| `shadow-sub-owner` | none | `template.shadowprobe` with attr `reverseAttr`. | +| `shadow-base-late` | `shadow-sub-owner` | `template.base` with attr `reverseAttr` (the name `shadow-sub-owner` owns on the subtype). | +| `shadow-sub-distinct` | `shadow-base` | `template.shadowprobe` with attr `distinctAttr`. | +| `shadow-self` | none | `template.base` with attr `selfAttr`, then `template.shadowprobe` with the SAME attr `selfAttr`. | + +Composing `["shadow-base", "shadow-sub-clash"]` and `["shadow-sub-owner", "shadow-base-late"]` +→ `ERR_PROVIDER_ATTR_SHADOW` (the two directions: the verdict must not depend on which +provider registered first). Two providers claiming one name on the SAME type is the +existing `attr-conflict` scenario (`ERR_PROVIDER_ATTR_CONFLICT`). + +## The `attr-ownership/` subdir (ADR-0062) + +Positive scenarios: composition succeeds and the registry reports which provider owns each +attribute. Own subdirectory for the same backward-compat reason as `compose-load/` (the flat +runners list the parent non-recursively and require `expectedError`). Shape: + +```jsonc +{ + "description": "...", + "providers": ["shadow-base", "shadow-sub-distinct"], // composed in listed order; must succeed + "expectOwners": [ // the port's per-slot attr-owner lookup + { "type": "template", "subType": "base", "attr": "shadowAttr", "owner": "shadow-base" } + ] +} +``` + +`expectOwners` asks for the provider that registered `attr` on exactly `(type, subType)` — an +inherited attr is asked of the slot that declares it. `distinct-names-load.json` proves distinct +names load; `same-provider-redeclare.json` proves one provider re-declaring its own name on a type +and on the type it inherits from stays legal. + ## Notes - `ERR_REGISTRY_SEALED` is a **five-port** invariant here. It was previously exercised diff --git a/fixtures/provider-composition-conformance/attr-ownership/distinct-names-load.json b/fixtures/provider-composition-conformance/attr-ownership/distinct-names-load.json new file mode 100644 index 0000000000..0fa2c8271d --- /dev/null +++ b/fixtures/provider-composition-conformance/attr-ownership/distinct-names-load.json @@ -0,0 +1,21 @@ +{ + "description": "Two providers declaring DISTINCT attr names on a type and the type it inherits from compose cleanly, and the registry records which provider owns each attribute.", + "providers": [ + "shadow-base", + "shadow-sub-distinct" + ], + "expectOwners": [ + { + "type": "template", + "subType": "base", + "attr": "shadowAttr", + "owner": "shadow-base" + }, + { + "type": "template", + "subType": "shadowprobe", + "attr": "distinctAttr", + "owner": "shadow-sub-distinct" + } + ] +} diff --git a/fixtures/provider-composition-conformance/attr-ownership/same-provider-redeclare.json b/fixtures/provider-composition-conformance/attr-ownership/same-provider-redeclare.json new file mode 100644 index 0000000000..a8b5d9fc07 --- /dev/null +++ b/fixtures/provider-composition-conformance/attr-ownership/same-provider-redeclare.json @@ -0,0 +1,20 @@ +{ + "description": "One provider declaring the same attr name on a type and on the type it inherits from stays legal; the registry records that provider as owner of both.", + "providers": [ + "shadow-self" + ], + "expectOwners": [ + { + "type": "template", + "subType": "base", + "attr": "selfAttr", + "owner": "shadow-self" + }, + { + "type": "template", + "subType": "shadowprobe", + "attr": "selfAttr", + "owner": "shadow-self" + } + ] +} diff --git a/fixtures/provider-composition-conformance/attr-shadow-descendant.json b/fixtures/provider-composition-conformance/attr-shadow-descendant.json new file mode 100644 index 0000000000..8e932880c4 --- /dev/null +++ b/fixtures/provider-composition-conformance/attr-shadow-descendant.json @@ -0,0 +1,8 @@ +{ + "description": "A provider declares an attr on the shared root (.base) after another provider declared that name on a subtype that inherits from it (the verdict must not depend on registration order).", + "providers": [ + "shadow-sub-owner", + "shadow-base-late" + ], + "expectedError": "ERR_PROVIDER_ATTR_SHADOW" +} diff --git a/fixtures/provider-composition-conformance/attr-shadow-inherited.json b/fixtures/provider-composition-conformance/attr-shadow-inherited.json new file mode 100644 index 0000000000..1d959ba527 --- /dev/null +++ b/fixtures/provider-composition-conformance/attr-shadow-inherited.json @@ -0,0 +1,8 @@ +{ + "description": "A provider declares an attr on a subtype whose shared root (.base) another provider already declares with that name.", + "providers": [ + "shadow-base", + "shadow-sub-clash" + ], + "expectedError": "ERR_PROVIDER_ATTR_SHADOW" +} diff --git a/scripts/check-doc-examples.ts b/scripts/check-doc-examples.ts index 37ad016a7d..97ea0ee1a6 100644 --- a/scripts/check-doc-examples.ts +++ b/scripts/check-doc-examples.ts @@ -229,6 +229,7 @@ const FRAGMENT_CODES = new Set([ // Environment-shaped: about the registry/provider wiring, never about the example. "ERR_REGISTRY_SEALED", "ERR_PROVIDER_ATTR_CONFLICT", + "ERR_PROVIDER_ATTR_SHADOW", "ERR_PROVIDER_DEPENDENCY_CYCLE", "ERR_PROVIDER_DUPLICATE_ID", "ERR_PROVIDER_MISSING_DEPENDENCY", diff --git a/server/csharp/MetaObjects.Conformance.Tests/AttrOwnershipTests.cs b/server/csharp/MetaObjects.Conformance.Tests/AttrOwnershipTests.cs new file mode 100644 index 0000000000..84f8cf9a78 --- /dev/null +++ b/server/csharp/MetaObjects.Conformance.Tests/AttrOwnershipTests.cs @@ -0,0 +1,150 @@ +// ADR-0062 — the registry records which provider registered each attribute and refuses a +// second provider's claim on the same name. The cross-port scenarios live in +// fixtures/provider-composition-conformance/; this file pins what the shared corpus cannot +// (message text, no-trace-on-refusal, unowned registrations, the shipped core). + +using MetaObjects; +using MetaObjects.Meta; +using Xunit; + +namespace MetaObjects.Conformance.Tests; + +public sealed class AttrOwnershipTests +{ + private sealed class Probe : IMetaDataTypeProvider + { + private readonly Action _register; + + public Probe(string id, Action register, params string[] dependencies) + { + Id = id; + Dependencies = dependencies; + _register = register; + } + + public string Id { get; } + public IReadOnlyList Dependencies { get; } + public void RegisterTypes(TypeRegistry registry) => _register(registry); + } + + private static TypeDefinition Def(string subType, params string[] names) => + new( + typeId: new TypeId("template", subType), + description: "test", + childRules: new List(), + factory: (typeId, name) => new MetaTemplate(typeId, name), + attributes: names.Select(n => new AttrSchema(n, "string", Required: false, Description: n)).ToList()); + + [Fact] + public void ShadowingRegistrationNamesBothProvidersAndTheAttribute() + { + var ex = Assert.Throws(() => Provider.ComposeRegistry(new IMetaDataTypeProvider[] + { + new Probe("owner-a", r => r.Register(Def("base", "physicalName"))), + new Probe("owner-b", r => r.Register(Def("probe", "physicalName")), "owner-a"), + })); + Assert.Equal(ErrorCode.ERR_PROVIDER_ATTR_SHADOW, ex.Code); + Assert.Contains("owner-a", ex.Message); + Assert.Contains("owner-b", ex.Message); + Assert.Contains("physicalName", ex.Message); + } + + [Fact] + public void ExtendOntoTheSharedRootCollidesWithASubtypeName() + { + var ex = Assert.Throws(() => Provider.ComposeRegistry(new IMetaDataTypeProvider[] + { + new Probe("owner-a", r => + { + r.Register(Def("base", "other")); + r.Register(Def("probe", "physicalName")); + }), + new Probe("owner-b", r => r.Extend("template", "base", + attributes: new List { new("physicalName", "string", Required: false, Description: "x") }), "owner-a"), + })); + Assert.Equal(ErrorCode.ERR_PROVIDER_ATTR_SHADOW, ex.Code); + } + + [Fact] + public void SameSlotExtendByAnotherProviderKeepsAttrConflictAndNamesTheOwner() + { + var ex = Assert.Throws(() => Provider.ComposeRegistry(new IMetaDataTypeProvider[] + { + new Probe("owner-a", r => r.Register(Def("probe", "physicalName"))), + new Probe("owner-b", r => r.Extend("template", "probe", + attributes: new List { new("physicalName", "string", Required: false, Description: "x") }), "owner-a"), + })); + Assert.Equal(ErrorCode.ERR_PROVIDER_ATTR_CONFLICT, ex.Code); + Assert.Contains("owner-a", ex.Message); + Assert.Contains("owner-b", ex.Message); + } + + [Fact] + public void SiblingSubtypesMayUseOneName() + { + var registry = Provider.ComposeRegistry(new IMetaDataTypeProvider[] + { + new Probe("owner-a", r => r.Register(Def("probe-a", "physicalName"))), + new Probe("owner-b", r => r.Register(Def("probe-b", "physicalName")), "owner-a"), + }); + Assert.Equal("owner-a", registry.AttrOrigin("template", "probe-a", "physicalName")); + Assert.Equal("owner-b", registry.AttrOrigin("template", "probe-b", "physicalName")); + } + + [Fact] + public void RegistrationOutsideAProviderTurnClaimsNothingAndCollidesWithNothing() + { + var registry = Provider.ComposeRegistry(new IMetaDataTypeProvider[] + { + new Probe("owner-a", r => r.Register(Def("base", "physicalName"))), + }); + + // After compose no provider is running: this registration is unowned, so it is not + // refused even though owner-a holds the same name on the type's shared root. + registry.Register(Def("probe", "physicalName")); + + Assert.Equal("owner-a", registry.AttrOrigin("template", "base", "physicalName")); + Assert.Equal(TypeRegistry.LibrarySentinel, registry.AttrOrigin("template", "probe", "physicalName")); + } + + [Fact] + public void RefusedRegistrationLeavesNoTrace() + { + bool refused = false; + var registry = Provider.ComposeRegistry(new IMetaDataTypeProvider[] + { + new Probe("owner-a", r => r.Register(Def("base", "physicalName"))), + new Probe("owner-b", r => + { + try + { + r.Register(Def("probe", "physicalName")); + } + catch (MetaModelException) + { + refused = true; + } + }, "owner-a"), + }); + + Assert.True(refused); + Assert.Null(registry.Find("template", "probe")); + Assert.Equal(TypeRegistry.LibrarySentinel, registry.AttrOrigin("template", "probe", "physicalName")); + } + + [Fact] + public void ShippedCoreProvidersComposeWithNoCollisionAndEveryAttrHasAnOwner() + { + var registry = Provider.ComposeRegistry(CoreTypes.LibraryProviders); + var unowned = new List(); + foreach (var id in registry.AllTypes()) + { + foreach (var attr in registry.AttrsOf(id.Type, id.SubType)) + { + if (registry.AttrOrigin(id.Type, id.SubType, attr.Name) == TypeRegistry.LibrarySentinel) + unowned.Add($"{id}.{attr.Name}"); + } + } + Assert.Empty(unowned); + } +} diff --git a/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs b/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs index ccd7acb7db..811043973b 100644 --- a/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs +++ b/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs @@ -116,6 +116,44 @@ public void RegisterTypes(TypeRegistry registry) } } +/// +/// ADR-0062 attribute-ownership providers (see the corpus README). Each registers one string +/// attr per listed (subtype, attr) pair on the template type: base is the shared +/// root, shadowprobe a fresh subtype. +/// +internal sealed class ShadowTestProvider : IMetaDataTypeProvider +{ + public const string ShadowSubType = "shadowprobe"; + + private readonly (string SubType, string Attr)[] _types; + + public ShadowTestProvider(string id, string[] dependencies, params (string SubType, string Attr)[] types) + { + Id = id; + Dependencies = dependencies; + _types = types; + } + + public string Id { get; } + public IReadOnlyList Dependencies { get; } + + public void RegisterTypes(TypeRegistry registry) + { + foreach (var (subType, attr) in _types) + { + registry.Register(new TypeDefinition( + typeId: new TypeId("template", subType), + description: "Test-only — attribute-ownership probe.", + childRules: new List(), + factory: (typeId, name) => new MetaTemplate(typeId, name), + attributes: new List + { + new AttrSchema(attr, "string", Required: false, Description: "Ownership probe attr."), + })); + } + } +} + public sealed class ProviderCompositionConformanceTests { private static readonly IReadOnlyDictionary Providers = @@ -131,6 +169,12 @@ public sealed class ProviderCompositionConformanceTests ["attr-conflict-clash"] = new AttrConflictClashProvider(), ["seal-probe"] = new SealProbeProvider(), ["extend-spec-subtype"] = new ExtendSpecSubtypeProvider(), + ["shadow-base"] = new ShadowTestProvider("shadow-base", System.Array.Empty(), ("base", "shadowAttr")), + ["shadow-sub-clash"] = new ShadowTestProvider("shadow-sub-clash", new[] { "shadow-base" }, (ShadowTestProvider.ShadowSubType, "shadowAttr")), + ["shadow-sub-owner"] = new ShadowTestProvider("shadow-sub-owner", System.Array.Empty(), (ShadowTestProvider.ShadowSubType, "reverseAttr")), + ["shadow-base-late"] = new ShadowTestProvider("shadow-base-late", new[] { "shadow-sub-owner" }, ("base", "reverseAttr")), + ["shadow-sub-distinct"] = new ShadowTestProvider("shadow-sub-distinct", new[] { "shadow-base" }, (ShadowTestProvider.ShadowSubType, "distinctAttr")), + ["shadow-self"] = new ShadowTestProvider("shadow-self", System.Array.Empty(), ("base", "selfAttr"), (ShadowTestProvider.ShadowSubType, "selfAttr")), }; // Flat-corpus (error-code) manifest shape — unchanged. @@ -285,4 +329,44 @@ public void ProviderCompositionComposeLoad(string fileName) Assert.Equal(expectedCodes, actualCodes); } } + + // ----------------------------------------------------------------------- + // ADR-0062 `attr-ownership/` corpus — composition succeeds and the registry names + // the provider that owns each attribute. + // ----------------------------------------------------------------------- + + private sealed record OwnerExpectation(string Type, string SubType, string Attr, string Owner); + + private sealed record OwnershipManifest(string[] Providers, OwnerExpectation[] ExpectOwners); + + private static string OwnershipCorpusRootPath() => + Path.Combine(CorpusRootPath(), "attr-ownership"); + + public static IEnumerable OwnershipManifestFiles() + { + foreach (var file in Directory.GetFiles(OwnershipCorpusRootPath(), "*.json").OrderBy(f => f, System.StringComparer.Ordinal)) + yield return new object[] { Path.GetFileName(file) }; + } + + [Fact] + public void OwnershipCorpusIsNonEmpty() + { + Assert.NotEmpty(OwnershipManifestFiles()); + } + + [Theory] + [MemberData(nameof(OwnershipManifestFiles))] + public void ProviderCompositionAttrOwnership(string fileName) + { + var opts = new JsonSerializerOptions { PropertyNameCaseInsensitive = true }; + var manifest = JsonSerializer.Deserialize( + File.ReadAllText(Path.Combine(OwnershipCorpusRootPath(), fileName)), opts)!; + + var registry = Provider.ComposeRegistry(manifest.Providers.Select(Resolve).ToList()); + Assert.NotEmpty(manifest.ExpectOwners); + foreach (var expected in manifest.ExpectOwners) + { + Assert.Equal(expected.Owner, registry.AttrOrigin(expected.Type, expected.SubType, expected.Attr)); + } + } } diff --git a/server/csharp/MetaObjects/Errors.cs b/server/csharp/MetaObjects/Errors.cs index da46527c5a..b98e164351 100644 --- a/server/csharp/MetaObjects/Errors.cs +++ b/server/csharp/MetaObjects/Errors.cs @@ -57,6 +57,9 @@ public enum ErrorCode ERR_PROVIDER_DUPLICATE_ID, ERR_PROVIDER_MISSING_DEPENDENCY, ERR_PROVIDER_ATTR_CONFLICT, + // ADR-0062 — a provider declared an attribute name another provider already declares + // on the type it inherits from (or on a subtype that inherits from it). + ERR_PROVIDER_ATTR_SHADOW, // ADR-0050 — a provider tried to project a REQUIRED attribute onto a type it // does not own (via TypeRegistry.Extend). Projection is optional-only: a // required attr registered this way disappears silently whenever that diff --git a/server/csharp/MetaObjects/Registry.cs b/server/csharp/MetaObjects/Registry.cs index 64eee45660..8b8a031a2c 100644 --- a/server/csharp/MetaObjects/Registry.cs +++ b/server/csharp/MetaObjects/Registry.cs @@ -250,6 +250,59 @@ private void StampAttrProvenance(string type, string subType, string attrName) public string AttrOrigin(string type, string subType, string attrName) => _attrProvenance.TryGetValue((type, subType, attrName), out string? origin) ? origin : LibrarySentinel; + /// + /// ADR-0062 — the provider-collision rule. Provider P may not put attribute + /// on (type, subType) when a DIFFERENT provider Q already put + /// the same name on a related slot: the slot itself, the type it inherits from + /// (<type>.base) or, when this slot is the shared root, any subtype that + /// inherits from it. Checked in both directions so the verdict does not depend on which + /// provider registered first. The same provider declaring a name on several related + /// slots stays legal, and an attr registered outside any provider's turn (no current + /// provider, or the library sentinel) claims nothing. + /// + private void CheckAttrOwnership(string type, string subType, string name) + { + string? claimant = CurrentProviderId; + if (claimant is null) return; + + var related = new List { subType }; + if (subType == SUBTYPE_BASE) + { + if (_subTypes.TryGetValue(type, out List? subs)) + { + related.AddRange(subs.Where(sub => sub != SUBTYPE_BASE)); + } + } + else if (_defs.ContainsKey($"{type}.{SUBTYPE_BASE}")) + { + related.Add(SUBTYPE_BASE); + } + + foreach (string sub in related) + { + if (!_attrProvenance.TryGetValue((type, sub, name), out string? owner) + || owner == LibrarySentinel || owner == claimant) + { + continue; + } + + if (sub == subType) + { + throw new MetaModelException( + $"TypeRegistry.Extend: attribute \"{name}\" is already declared on \"{type}.{subType}\" " + + $"by provider \"{owner}\"; provider \"{claimant}\" cannot declare it again (ADR-0062)", + ErrorCode.ERR_PROVIDER_ATTR_CONFLICT); + } + + throw new MetaModelException( + $"Provider \"{claimant}\" declares attribute \"{name}\" on \"{type}.{subType}\", but provider " + + $"\"{owner}\" already declares it on the related slot \"{type}.{sub}\". Two providers may not " + + "claim one attribute name on a type and the type it inherits from; give the second provider " + + "its own idiomatic name (ADR-0062).", + ErrorCode.ERR_PROVIDER_ATTR_SHADOW); + } + } + private void CheckNotSealed(string operation) { if (_sealed) @@ -297,6 +350,13 @@ public void Register(TypeDefinition def) } } + // ADR-0062: refuse a name another provider already put on a related slot, BEFORE + // the definition becomes visible, so a refused registration leaves no trace. + foreach (AttrSchema attr in def.Attributes) + { + CheckAttrOwnership(def.TypeId.Type, def.TypeId.SubType, attr.Name); + } + _defs[key] = def; // #265 — stamp provenance for every attr present at registration time (also @@ -482,11 +542,15 @@ public void Extend(string type, string subType, IReadOnlyList? attri if (def.Attributes.Any(a => a.Name == attr.Name)) { + // ADR-0062: name the provider that holds the slot when it is a different one. + CheckAttrOwnership(type, subType, attr.Name); throw new MetaModelException( $"TypeRegistry.Extend: attribute \"{attr.Name}\" is already declared on \"{type}.{subType}\"", ErrorCode.ERR_PROVIDER_ATTR_CONFLICT); } + CheckAttrOwnership(type, subType, attr.Name); + // ADR-0050: a REQUIRED attr may never be PROJECTED onto someone else's type. // Extend is how a concern provider decorates a type another provider owns, and // a concern can be composed out. A required attr arriving this way makes the diff --git a/server/java/metadata/src/main/java/com/metaobjects/ErrorCode.java b/server/java/metadata/src/main/java/com/metaobjects/ErrorCode.java index 01355a0b6b..3f2433e6c5 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/ErrorCode.java +++ b/server/java/metadata/src/main/java/com/metaobjects/ErrorCode.java @@ -159,6 +159,12 @@ public enum ErrorCode { /** A provider extend redefines an attr another provider already declared. */ ERR_PROVIDER_ATTR_CONFLICT, + /** + * ADR-0062: a provider declares an attribute name another provider already declares on + * the type it inherits from (or on a subtype that inherits from it). + */ + ERR_PROVIDER_ATTR_SHADOW, + /** * ADR-0050: a provider tried to project a REQUIRED attribute onto a type it does * not own (via extends). Projection is optional-only — a required attr registered diff --git a/server/java/metadata/src/main/java/com/metaobjects/registry/MetaDataRegistry.java b/server/java/metadata/src/main/java/com/metaobjects/registry/MetaDataRegistry.java index 2f930b8ff3..381e459168 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/registry/MetaDataRegistry.java +++ b/server/java/metadata/src/main/java/com/metaobjects/registry/MetaDataRegistry.java @@ -108,6 +108,9 @@ public class MetaDataRegistry { */ static final String LIBRARY_PROVENANCE = "_LIBRARY"; + /** The shared-root subtype every type's concrete subtypes inherit from. */ + private static final String SUBTYPE_BASE = "base"; + /** * #265 — the provider id currently executing its {@code registerTypes} turn * inside {@link #registerProviders}, or {@code null} outside that scope. Set @@ -370,6 +373,22 @@ public MetaDataRegistry extendType(Class metaDataClass, Cons // ADR-0050: fails closed — checked BEFORE the commit below, so a violation // never reaches typeDefinitions (see checkNoProjectedRequiredAttr). checkNoProjectedRequiredAttr(typeIdToExtend, existing, extendedDefinition); + // ADR-0062: same fail-closed position — a name another provider already holds on a + // related slot never reaches typeDefinitions. + Set existingNames = new HashSet<>(); + for (ChildRequirement req : existing.getChildRequirements()) { + if (isNamedAttrRequirement(req)) { + existingNames.add(req.getName()); + } + } + List claimed = new ArrayList<>(); + for (ChildRequirement req : extendedDefinition.getDirectChildRequirements()) { + if (isNamedAttrRequirement(req) && !existingNames.contains(req.getName())) { + claimed.add(req.getName()); + } + } + checkAttrOwnership(typeIdToExtend, extendedDefinition.getParentType(), + extendedDefinition.getParentSubType(), claimed); typeDefinitions.put(typeIdToExtend, extendedDefinition); // #265 — extendType() rebuilds from `existing` and writes straight into // typeDefinitions WITHOUT going through register(), so it must stamp @@ -545,6 +564,26 @@ public void register(TypeDefinition definition) { ); } + // ADR-0062: refuse a name another provider already put on a related slot, BEFORE + // the definition becomes visible, so a refused registration leaves no trace. + // Only names NEW to this slot are checked: re-registering a slot a provider + // already owns replaces its complete definition and is not a new claim. + Set alreadyDeclared = new HashSet<>(); + if (existing != null) { + for (ChildRequirement req : existing.getChildRequirements()) { + if (isNamedAttrRequirement(req)) { + alreadyDeclared.add(req.getName()); + } + } + } + List claimed = new ArrayList<>(); + for (ChildRequirement req : definition.getDirectChildRequirements()) { + if (isNamedAttrRequirement(req) && !alreadyDeclared.contains(req.getName())) { + claimed.add(req.getName()); + } + } + checkAttrOwnership(typeId, definition.getParentType(), definition.getParentSubType(), claimed); + // Resolve inheritance if this type has a parent resolveInheritance(definition); @@ -1154,6 +1193,126 @@ private static boolean isNamedAttrRequirement(ChildRequirement req) { return name != null && !"*".equals(name); // false for the any-attr wildcard } + /** + * ADR-0062 — the provider that registered {@code attrName} on exactly + * {@code (type, subType)}, or {@code null} when it is not registered there or was + * registered outside any provider's turn. An inherited attr reports its declaring slot, + * so ask {@code .base} for those. + */ + public String getAttrProvider(String type, String subType, String attrName) { + Map byName = attrProvenance.get(new MetaDataTypeId(type, subType)); + String providerId = byName != null ? byName.get(attrName) : null; + return LIBRARY_PROVENANCE.equals(providerId) ? null : providerId; + } + + /** + * ADR-0062 — the slots related to {@code typeId} for the provider-collision rule: the + * same-node-type slots it inherits from (its explicit parent chain, plus the + * {@code .base} shared root by convention) and, when {@code typeId} is a shared + * root or a parent, every registered subtype of that type that inherits from it. + * {@code parentType}/{@code parentSubType} are the definition's own parent, passed in + * because a definition being registered is not in {@link #typeDefinitions} yet. + * + *

The rule is scoped to ONE node type. The JVM registry additionally chains every + * {@code .base} to {@code metadata.base} (the attrs every node shares, owned by + * the core-types provider); no other port has that edge, and several providers re-declare + * {@code @isAbstract}/{@code @description} on their own root with the same meaning. A + * cross-type walk would make Java refuse what the other four ports accept, so the walk + * stops at the node-type boundary.

+ */ + private Set relatedSlots(MetaDataTypeId typeId, String parentType, String parentSubType) { + Set related = new LinkedHashSet<>(); + String nextType = parentType; + String nextSubType = parentSubType; + while (nextType != null && nextSubType != null) { + MetaDataTypeId parent = new MetaDataTypeId(nextType, nextSubType); + if (!parent.type().equals(typeId.type()) || parent.equals(typeId) || !related.add(parent)) { + break; + } + TypeDefinition parentDef = typeDefinitions.get(parent); + nextType = parentDef != null && parentDef.hasParent() ? parentDef.getParentType() : null; + nextSubType = parentDef != null && parentDef.hasParent() ? parentDef.getParentSubType() : null; + } + boolean isSharedRoot = SUBTYPE_BASE.equals(typeId.subType()); + if (!isSharedRoot) { + MetaDataTypeId root = new MetaDataTypeId(typeId.type(), SUBTYPE_BASE); + if (typeDefinitions.containsKey(root)) { + related.add(root); + } + } + for (Map.Entry entry : typeDefinitions.entrySet()) { + MetaDataTypeId other = entry.getKey(); + if (other.equals(typeId)) { + continue; + } + if (isSharedRoot && other.type().equals(typeId.type())) { + related.add(other); + continue; + } + TypeDefinition def = entry.getValue(); + while (def != null && def.hasParent()) { + MetaDataTypeId parent = new MetaDataTypeId(def.getParentType(), def.getParentSubType()); + if (!parent.type().equals(typeId.type())) { + break; + } + if (parent.equals(typeId)) { + related.add(other); + break; + } + def = typeDefinitions.get(parent); + } + } + return related; + } + + /** + * ADR-0062 — the provider-collision rule. Provider P may not put attribute {@code name} + * on {@code typeId} when a DIFFERENT provider Q already put the same name on a related + * slot (see {@link #relatedSlots}). Checked in both directions so the verdict does not + * depend on which provider registered first. The same provider declaring a name on + * several related slots stays legal, and an attr registered outside any provider's turn + * claims nothing. + */ + private void checkAttrOwnership(MetaDataTypeId typeId, String parentType, String parentSubType, + Collection names) { + String claimant = currentProviderId; + if (claimant == null || names.isEmpty()) { + return; + } + Set related = relatedSlots(typeId, parentType, parentSubType); + for (String name : names) { + for (MetaDataTypeId slot : related) { + Map byName = attrProvenance.get(slot); + String owner = byName != null ? byName.get(name) : null; + if (owner == null || LIBRARY_PROVENANCE.equals(owner) || owner.equals(claimant)) { + continue; + } + throw new MetaDataException( + "Provider \"" + claimant + "\" declares attribute \"" + name + "\" on \"" + + typeId.toQualifiedName() + "\", but provider \"" + owner + + "\" already declares it on the related slot \"" + slot.toQualifiedName() + + "\". Two providers may not claim one attribute name on a type and the type it " + + "inherits from; give the second provider its own idiomatic name (ADR-0062).", + com.metaobjects.ErrorCode.ERR_PROVIDER_ATTR_SHADOW); + } + } + } + + /** + * ADR-0062 — text appended to a same-slot {@code ERR_PROVIDER_ATTR_CONFLICT} so it names + * the provider that holds the slot. Empty when the holder is unknown or is the provider + * running now (a plain duplicate declaration, not a cross-provider claim). + */ + String describeSlotHolder(String type, String subType, String attrName) { + String owner = getAttrProvider(type, subType, attrName); + String claimant = currentProviderId; + if (owner == null || claimant == null || owner.equals(claimant)) { + return ""; + } + return " Declared by provider \"" + owner + "\"; provider \"" + claimant + + "\" cannot declare it again (ADR-0062)."; + } + /** * #265 — record provenance for every named attr requirement in {@code directRequirements} * onto {@code typeId}, stamping {@link #currentProviderId} (or {@link #LIBRARY_PROVENANCE} diff --git a/server/java/metadata/src/main/java/com/metaobjects/registry/TypeDefinitionBuilder.java b/server/java/metadata/src/main/java/com/metaobjects/registry/TypeDefinitionBuilder.java index 37c0e714f9..61c0bab6a2 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/registry/TypeDefinitionBuilder.java +++ b/server/java/metadata/src/main/java/com/metaobjects/registry/TypeDefinitionBuilder.java @@ -441,7 +441,13 @@ private void addChildRequirement(ChildRequirement requirement) { } ChildRequirement existing = childRequirements.get(key); - if (existing != null && !existing.equals(requirement)) { + // ADR-0062: an IDENTICAL re-declaration is idempotent for the provider that owns the + // slot's attr, but another provider repeating it is still a second claim on the name. + boolean foreignRepeat = existing != null && existing.equals(requirement) + && "attr".equals(requirement.getExpectedType()) + && registry != null + && !registry.describeSlotHolder(type, subType, requirement.getName()).isEmpty(); + if (existing != null && (foreignRepeat || !existing.equals(requirement))) { // A colliding ATTR child requirement is a provider redefining an attr // another provider already declared on this type — the cross-port // ERR_PROVIDER_ATTR_CONFLICT invariant (mirrors the TS/Python/C# @@ -452,7 +458,8 @@ private void addChildRequirement(ChildRequirement requirement) { throw new com.metaobjects.MetaDataException( "Attribute '" + requirement.getName() + "' is already declared on this type " + "(a provider redefined an attr another provider already declared). " - + "Existing: " + existing + ", New: " + requirement, + + "Existing: " + existing + ", New: " + requirement + + (registry != null ? registry.describeSlotHolder(type, subType, requirement.getName()) : ""), com.metaobjects.ErrorCode.ERR_PROVIDER_ATTR_CONFLICT); } throw new IllegalArgumentException( diff --git a/server/java/metadata/src/test/java/com/metaobjects/conformance/ProviderCompositionConformanceTest.java b/server/java/metadata/src/test/java/com/metaobjects/conformance/ProviderCompositionConformanceTest.java index f5d2619491..b63658e850 100644 --- a/server/java/metadata/src/test/java/com/metaobjects/conformance/ProviderCompositionConformanceTest.java +++ b/server/java/metadata/src/test/java/com/metaobjects/conformance/ProviderCompositionConformanceTest.java @@ -47,12 +47,14 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.function.Consumer; import java.util.stream.Collectors; import java.util.stream.Stream; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; /** @@ -108,6 +110,16 @@ static final class SealProbeTemplate extends MetaTemplate { SealProbeTemplate(String name) { super("sealprobe", name); } } + // ADR-0062 attribute-ownership probes: the shared root template.base and the fresh + // subtype template.shadowprobe (see the corpus README). + static final class ShadowRootTemplate extends MetaTemplate { + ShadowRootTemplate(String name) { super(TemplateConstants.SUBTYPE_BASE, name); } + } + + static final class ShadowProbeTemplate extends MetaTemplate { + ShadowProbeTemplate(String name) { super("shadowprobe", name); } + } + // ------------------------------------------------------------------ // Canonical named-provider set (test-only; identical id/deps/behavior // cross-port). The no-op cycle/duplicate/missing providers are reused from @@ -173,6 +185,52 @@ static final class SealProbeTemplate extends MetaTemplate { @Override public String getDescription() { return "Test-only — #265 compose-load probe: extends view.currency with @decimals."; } }; + /** Registers the shared root {@code template.base} with one optional string attr. */ + private static void registerShadowRoot(MetaDataRegistry registry, String attr) { + registry.registerType(ShadowRootTemplate.class, def -> { + def.type(TemplateConstants.TYPE_TEMPLATE).subType(TemplateConstants.SUBTYPE_BASE) + .description("Test-only — attribute-ownership probe root."); + def.optionalAttributeWithConstraints(attr) + .ofType(StringAttribute.SUBTYPE_STRING).asSingle(); + }); + } + + /** Registers {@code template.shadowprobe} (inheriting the root) with one optional string attr. */ + private static void registerShadowProbe(MetaDataRegistry registry, String attr) { + registry.registerType(ShadowProbeTemplate.class, def -> { + def.type(TemplateConstants.TYPE_TEMPLATE).subType("shadowprobe") + .description("Test-only — attribute-ownership probe subtype.") + .inheritsFrom(TemplateConstants.TYPE_TEMPLATE, TemplateConstants.SUBTYPE_BASE); + def.optionalAttributeWithConstraints(attr) + .ofType(StringAttribute.SUBTYPE_STRING).asSingle(); + }); + } + + private static MetaDataTypeProvider shadowProvider(String id, String[] deps, Consumer body) { + return new MetaDataTypeProvider() { + @Override public String getProviderId() { return id; } + @Override public String[] getDependencies() { return deps; } + @Override public void registerTypes(MetaDataRegistry registry) { body.accept(registry); } + @Override public String getDescription() { return "Test-only ADR-0062 attribute-ownership provider: " + id; } + }; + } + + private static final MetaDataTypeProvider SHADOW_BASE = + shadowProvider("shadow-base", new String[0], r -> registerShadowRoot(r, "shadowAttr")); + private static final MetaDataTypeProvider SHADOW_SUB_CLASH = + shadowProvider("shadow-sub-clash", new String[]{"shadow-base"}, r -> registerShadowProbe(r, "shadowAttr")); + private static final MetaDataTypeProvider SHADOW_SUB_OWNER = + shadowProvider("shadow-sub-owner", new String[0], r -> registerShadowProbe(r, "reverseAttr")); + private static final MetaDataTypeProvider SHADOW_BASE_LATE = + shadowProvider("shadow-base-late", new String[]{"shadow-sub-owner"}, r -> registerShadowRoot(r, "reverseAttr")); + private static final MetaDataTypeProvider SHADOW_SUB_DISTINCT = + shadowProvider("shadow-sub-distinct", new String[]{"shadow-base"}, r -> registerShadowProbe(r, "distinctAttr")); + private static final MetaDataTypeProvider SHADOW_SELF = + shadowProvider("shadow-self", new String[0], r -> { + registerShadowRoot(r, "selfAttr"); + registerShadowProbe(r, "selfAttr"); + }); + private static MetaDataTypeProvider resolve(String id) { MetaDataTypeProvider p = ConformanceTestProviders.TEST_PROVIDERS.get(id); if (p != null) return p; @@ -181,6 +239,12 @@ private static MetaDataTypeProvider resolve(String id) { case "attr-conflict-clash": return ATTR_CONFLICT_CLASH; case "seal-probe": return SEAL_PROBE; case "extend-spec-subtype": return EXTEND_SPEC_SUBTYPE; + case "shadow-base": return SHADOW_BASE; + case "shadow-sub-clash": return SHADOW_SUB_CLASH; + case "shadow-sub-owner": return SHADOW_SUB_OWNER; + case "shadow-base-late": return SHADOW_BASE_LATE; + case "shadow-sub-distinct": return SHADOW_SUB_DISTINCT; + case "shadow-self": return SHADOW_SELF; default: throw new IllegalArgumentException( "Unknown named provider \"" + id + "\" in provider-composition corpus"); @@ -275,6 +339,62 @@ public void providerComposition() throws IOException { } } + // ------------------------------------------------------------------ + // ADR-0062 attr-ownership corpus — composition succeeds and the registry + // names the provider that owns each attribute. + // ------------------------------------------------------------------ + + @RunWith(Parameterized.class) + public static class AttrOwnership { + + private static Path ownershipCorpusRoot() { + return corpusRoot().resolve("attr-ownership"); + } + + @Parameters(name = "{0}") + public static Collection manifests() { + try (Stream files = Files.list(ownershipCorpusRoot())) { + List names = files + .filter(p -> p.getFileName().toString().endsWith(".json")) + .map(p -> p.getFileName().toString()) + .sorted() + .collect(Collectors.toList()); + if (names.isEmpty()) { + throw new AssertionError("provider-composition attr-ownership corpus is empty (mis-pathed root?)"); + } + List rows = new ArrayList<>(names.size()); + for (String n : names) rows.add(new Object[]{n}); + return rows; + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + @Parameter(0) + public String fileName; + + @Test + public void providerCompositionAttrOwnership() throws IOException { + JsonObject manifest = JsonParser.parseString( + Files.readString(ownershipCorpusRoot().resolve(fileName))).getAsJsonObject(); + List resolved = new ArrayList<>(); + manifest.getAsJsonArray("providers").forEach(e -> resolved.add(resolve(e.getAsString()))); + + MetaDataRegistry registry = MetaDataRegistry.compose(resolved); + + assertTrue(manifest.getAsJsonArray("expectOwners").size() > 0); + manifest.getAsJsonArray("expectOwners").forEach(el -> { + JsonObject expected = el.getAsJsonObject(); + assertEquals( + expected.get("owner").getAsString(), + registry.getAttrProvider( + expected.get("type").getAsString(), + expected.get("subType").getAsString(), + expected.get("attr").getAsString())); + }); + } + } + // ------------------------------------------------------------------ // #265 compose-load corpus — see the corpus README "The `compose-load/` // subdir". Own directory, own nested runner: a manifest here never carries diff --git a/server/java/metadata/src/test/java/com/metaobjects/registry/AttrOwnershipTest.java b/server/java/metadata/src/test/java/com/metaobjects/registry/AttrOwnershipTest.java new file mode 100644 index 0000000000..16a8e9a33d --- /dev/null +++ b/server/java/metadata/src/test/java/com/metaobjects/registry/AttrOwnershipTest.java @@ -0,0 +1,144 @@ +package com.metaobjects.registry; + +import com.metaobjects.ErrorCode; +import com.metaobjects.MetaDataException; +import com.metaobjects.attr.StringAttribute; +import com.metaobjects.template.MetaTemplate; +import com.metaobjects.template.TemplateConstants; +import org.junit.Test; + +import java.util.List; +import java.util.function.Consumer; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * ADR-0062 — the registry records which provider registered each attribute and refuses a + * second provider's claim on the same name. The cross-port scenarios live in + * {@code fixtures/provider-composition-conformance/}; this class pins what the shared corpus + * cannot (message text, same-slot extend, unowned registrations, the shipped core). + */ +public class AttrOwnershipTest { + + static final class RootTemplate extends MetaTemplate { + RootTemplate(String name) { super(TemplateConstants.SUBTYPE_BASE, name); } + } + + static final class ProbeTemplate extends MetaTemplate { + ProbeTemplate(String name) { super("probe", name); } + } + + static final class SiblingTemplate extends MetaTemplate { + SiblingTemplate(String name) { super("sibling", name); } + } + + private static void root(MetaDataRegistry r, String attr) { + r.registerType(RootTemplate.class, def -> { + def.type(TemplateConstants.TYPE_TEMPLATE).subType(TemplateConstants.SUBTYPE_BASE).description("root"); + def.optionalAttributeWithConstraints(attr).ofType(StringAttribute.SUBTYPE_STRING).asSingle(); + }); + } + + private static void probe(MetaDataRegistry r, String attr) { + r.registerType(ProbeTemplate.class, def -> { + def.type(TemplateConstants.TYPE_TEMPLATE).subType("probe").description("probe") + .inheritsFrom(TemplateConstants.TYPE_TEMPLATE, TemplateConstants.SUBTYPE_BASE); + def.optionalAttributeWithConstraints(attr).ofType(StringAttribute.SUBTYPE_STRING).asSingle(); + }); + } + + private static void sibling(MetaDataRegistry r, String attr) { + r.registerType(SiblingTemplate.class, def -> { + def.type(TemplateConstants.TYPE_TEMPLATE).subType("sibling").description("sibling"); + def.optionalAttributeWithConstraints(attr).ofType(StringAttribute.SUBTYPE_STRING).asSingle(); + }); + } + + private static MetaDataTypeProvider provider(String id, String[] deps, Consumer body) { + return new MetaDataTypeProvider() { + @Override public String getProviderId() { return id; } + @Override public String[] getDependencies() { return deps; } + @Override public void registerTypes(MetaDataRegistry registry) { body.accept(registry); } + @Override public String getDescription() { return "test " + id; } + }; + } + + private static MetaDataException refusal(MetaDataTypeProvider... providers) { + try { + MetaDataRegistry.compose(List.of(providers)); + } catch (MetaDataException e) { + return e; + } + fail("expected the composition to be refused"); + return null; + } + + @Test + public void shadowingRegistrationNamesBothProvidersAndTheAttribute() { + MetaDataException e = refusal( + provider("owner-a", new String[0], r -> root(r, "physicalName")), + provider("owner-b", new String[]{"owner-a"}, r -> probe(r, "physicalName"))); + assertEquals(ErrorCode.ERR_PROVIDER_ATTR_SHADOW, e.getCode().orElseThrow()); + assertTrue(e.getMessage().contains("owner-a")); + assertTrue(e.getMessage().contains("owner-b")); + assertTrue(e.getMessage().contains("physicalName")); + } + + @Test + public void extendOntoTheSharedRootCollidesWithASubtypeName() { + MetaDataException e = refusal( + provider("owner-a", new String[0], r -> { + root(r, "other"); + probe(r, "physicalName"); + }), + provider("owner-b", new String[]{"owner-a"}, r -> + r.extendType(RootTemplate.class, def -> + def.optionalAttributeWithConstraints("physicalName") + .ofType(StringAttribute.SUBTYPE_STRING).asSingle()))); + assertEquals(ErrorCode.ERR_PROVIDER_ATTR_SHADOW, e.getCode().orElseThrow()); + } + + @Test + public void sameSlotExtendByAnotherProviderKeepsAttrConflictAndNamesTheOwner() { + MetaDataException e = refusal( + provider("owner-a", new String[0], r -> probe(r, "physicalName")), + provider("owner-b", new String[]{"owner-a"}, r -> + r.extendType(ProbeTemplate.class, def -> + def.optionalAttributeWithConstraints("physicalName") + .ofType(StringAttribute.SUBTYPE_STRING).asSingle()))); + assertEquals(ErrorCode.ERR_PROVIDER_ATTR_CONFLICT, e.getCode().orElseThrow()); + assertTrue(e.getMessage().contains("owner-a")); + assertTrue(e.getMessage().contains("owner-b")); + } + + @Test + public void siblingSubtypesMayUseOneName() { + MetaDataRegistry registry = MetaDataRegistry.compose(List.of( + provider("owner-a", new String[0], r -> probe(r, "physicalName")), + provider("owner-b", new String[]{"owner-a"}, r -> sibling(r, "physicalName")))); + assertEquals("owner-a", registry.getAttrProvider("template", "probe", "physicalName")); + assertEquals("owner-b", registry.getAttrProvider("template", "sibling", "physicalName")); + } + + @Test + public void registrationOutsideAProviderTurnClaimsNothingAndCollidesWithNothing() { + MetaDataRegistry registry = MetaDataRegistry.compose(List.of( + provider("owner-a", new String[0], r -> root(r, "physicalName")))); + // After compose no provider is running: unowned, so not refused. + probe(registry, "physicalName"); + assertEquals("owner-a", registry.getAttrProvider("template", "base", "physicalName")); + assertNull(registry.getAttrProvider("template", "probe", "physicalName")); + } + + @Test + public void shippedCoreProvidersComposeWithNoCollisionAndNameTheirOwner() { + // Composing the shipped core is itself the audit: any cross-provider claim on a + // node type would throw ERR_PROVIDER_ATTR_SHADOW here. + MetaDataRegistry registry = RegistryManifest.composeMetamodelRegistry(List.of()); + assertNotNull(registry.getAttrProvider("metadata", "base", "isAbstract")); + } +} diff --git a/server/python/src/metaobjects/errors.py b/server/python/src/metaobjects/errors.py index fc8a3b55e5..eac4041d32 100644 --- a/server/python/src/metaobjects/errors.py +++ b/server/python/src/metaobjects/errors.py @@ -53,6 +53,9 @@ class ErrorCode(str, Enum): ERR_PROVIDER_DUPLICATE_ID = "ERR_PROVIDER_DUPLICATE_ID" ERR_PROVIDER_MISSING_DEPENDENCY = "ERR_PROVIDER_MISSING_DEPENDENCY" ERR_PROVIDER_ATTR_CONFLICT = "ERR_PROVIDER_ATTR_CONFLICT" + # ADR-0062: a provider declared an attribute name another provider already declares + # on the type it inherits from (or on a subtype that inherits from it). + ERR_PROVIDER_ATTR_SHADOW = "ERR_PROVIDER_ATTR_SHADOW" # ADR-0050: a provider tried to PROJECT a REQUIRED attribute onto a type it does # not own (via TypeRegistry.extend). Projection is optional-only — a required # attr registered this way disappears silently whenever that provider is diff --git a/server/python/src/metaobjects/registry.py b/server/python/src/metaobjects/registry.py index 5113ca3ead..b3da77247d 100644 --- a/server/python/src/metaobjects/registry.py +++ b/server/python/src/metaobjects/registry.py @@ -146,8 +146,51 @@ def _check_not_sealed(self, operation: str) -> None: ErrorCode.ERR_REGISTRY_SEALED, ) + def _check_attr_ownership(self, type_: str, sub_type: str, name: str) -> None: + """ADR-0062 -- the provider-collision rule. + + Provider P may not put attribute ``name`` on ``(type_, sub_type)`` when a DIFFERENT + provider Q already put the same name on a related slot: the slot itself, the type + it inherits from (``.base``) or, when this slot is the shared root, any + subtype that inherits from it. Checked in both directions so the verdict does not + depend on which provider registered first. The same provider declaring a name on + several related slots stays legal, and an attr registered outside any provider's + turn (no current provider, or the library sentinel) claims nothing. + """ + claimant = self._current_provider_id + if claimant is None: + return + related = [sub_type] + if sub_type == SUBTYPE_BASE: + related += [s for (t, s) in self._defs if t == type_ and s != SUBTYPE_BASE] + elif (type_, SUBTYPE_BASE) in self._defs: + related.append(SUBTYPE_BASE) + for sub in related: + owner = self._attr_provenance.get((type_, sub, name)) + if owner is None or owner == LIBRARY_ATTR_ORIGIN or owner == claimant: + continue + if sub == sub_type: + raise ParseError( + f'TypeRegistry.extend: attribute "{name}" is already declared on ' + f'"{type_}.{sub_type}" by provider "{owner}"; provider "{claimant}" ' + "cannot declare it again (ADR-0062)", + ErrorCode.ERR_PROVIDER_ATTR_CONFLICT, + ) + raise ParseError( + f'Provider "{claimant}" declares attribute "{name}" on "{type_}.{sub_type}", ' + f'but provider "{owner}" already declares it on the related slot ' + f'"{type_}.{sub}". Two providers may not claim one attribute name on a type ' + "and the type it inherits from; give the second provider its own idiomatic " + "name (ADR-0062).", + ErrorCode.ERR_PROVIDER_ATTR_SHADOW, + ) + def register(self, definition: TypeDefinition) -> None: self._check_not_sealed(f'register("{definition.key}")') + # ADR-0062: refuse a name another provider already put on a related slot, BEFORE + # the definition becomes visible, so a refused registration leaves no trace. + for attr in definition.attrs: + self._check_attr_ownership(definition.type, definition.sub_type, attr.name) # Store a per-registry COPY of the definition's mutable lists. Providers # hold their TypeDefinition objects as long-lived singletons (re-used across # every compose_registry call); a later provider's extend() does @@ -317,11 +360,14 @@ def extend( f"which is not valid for attrs. Use None for a polymorphic/untyped attr." ) if any(existing.name == attr.name for existing in definition.attrs): + # ADR-0062: name the provider that holds the slot when it is a different one. + self._check_attr_ownership(type_, sub_type, attr.name) raise ParseError( f'TypeRegistry.extend: attribute "{attr.name}" is already declared ' f'on "{type_}.{sub_type}"', ErrorCode.ERR_PROVIDER_ATTR_CONFLICT, ) + self._check_attr_ownership(type_, sub_type, attr.name) # ADR-0050: a REQUIRED attr may never be PROJECTED onto someone else's # type. A provider may only project OPTIONAL attributes onto a type it # does not own — a required attr registered this way disappears, diff --git a/server/python/tests/conformance/test_provider_composition_conformance.py b/server/python/tests/conformance/test_provider_composition_conformance.py index 71a3d6e78b..67d8e938ab 100644 --- a/server/python/tests/conformance/test_provider_composition_conformance.py +++ b/server/python/tests/conformance/test_provider_composition_conformance.py @@ -119,8 +119,44 @@ def register_types(self, registry: TypeRegistry) -> None: # noqa: D401 ) +# ADR-0062 -- attribute-ownership providers (see the corpus README). All register on +# `template.base` and the fresh subtype `template.shadowprobe`. +_SHADOW_SUBTYPE = "shadowprobe" + + +def _template_type(sub_type: str, attr_name: str) -> TypeDefinition: + return TypeDefinition( + type="template", + sub_type=sub_type, + factory=lambda t, s, n: MetaTemplate(t, s, n), + attrs=[AttrSchema(attr_name, "string")], + child_rules=[], + ) + + +def _shadow_provider(provider_id: str, deps: tuple[str, ...], *types: tuple[str, str]) -> Provider: + p = Provider(provider_id, dependencies=deps) + for sub_type, attr_name in types: + p.add(_template_type(sub_type, attr_name)) + return p + + def _providers() -> dict[str, Provider]: return { + "shadow-base": _shadow_provider("shadow-base", (), ("base", "shadowAttr")), + "shadow-sub-clash": _shadow_provider( + "shadow-sub-clash", ("shadow-base",), (_SHADOW_SUBTYPE, "shadowAttr") + ), + "shadow-sub-owner": _shadow_provider("shadow-sub-owner", (), (_SHADOW_SUBTYPE, "reverseAttr")), + "shadow-base-late": _shadow_provider( + "shadow-base-late", ("shadow-sub-owner",), ("base", "reverseAttr") + ), + "shadow-sub-distinct": _shadow_provider( + "shadow-sub-distinct", ("shadow-base",), (_SHADOW_SUBTYPE, "distinctAttr") + ), + "shadow-self": _shadow_provider( + "shadow-self", (), ("base", "selfAttr"), (_SHADOW_SUBTYPE, "selfAttr") + ), "duplicate-x": _noop_provider("duplicate-x"), # Same `.id` as duplicate-x — surfaces ERR_PROVIDER_DUPLICATE_ID at compose time. "duplicate-x-clone": _noop_provider("duplicate-x"), @@ -225,3 +261,27 @@ def test_provider_composition_compose_load(manifest_path: Path) -> None: actual_codes = sorted(_error_code(e) for e in result.errors) expected_codes = sorted(manifest.get("expectErrors", [])) assert actual_codes == expected_codes + + +# --------------------------------------------------------------------------- +# ADR-0062 `attr-ownership/` corpus -- composition succeeds and the registry +# names the provider that owns each attribute. +# --------------------------------------------------------------------------- + + +def _ownership_manifest_files() -> list[Path]: + return sorted((_corpus_root() / "attr-ownership").glob("*.json")) + + +def test_attr_ownership_corpus_non_empty() -> None: + assert len(_ownership_manifest_files()) > 0 + + +@pytest.mark.parametrize("manifest_path", _ownership_manifest_files(), ids=lambda p: p.name) +def test_provider_composition_attr_ownership(manifest_path: Path) -> None: + manifest = json.loads(manifest_path.read_text(encoding="utf-8")) + providers = _providers() + registry = compose_registry([_resolve(providers, pid) for pid in manifest["providers"]]) + assert manifest["expectOwners"] + for owner in manifest["expectOwners"]: + assert registry.attr_origin(owner["type"], owner["subType"], owner["attr"]) == owner["owner"] diff --git a/server/python/tests/unit/test_registry_attr_ownership.py b/server/python/tests/unit/test_registry_attr_ownership.py new file mode 100644 index 0000000000..7aa418c580 --- /dev/null +++ b/server/python/tests/unit/test_registry_attr_ownership.py @@ -0,0 +1,117 @@ +"""ADR-0062 -- the registry records which provider registered each attribute and refuses a +second provider's claim on the same name. The cross-port scenarios live in +fixtures/provider-composition-conformance/; this file pins what the shared corpus cannot +(message text, no-trace-on-refusal, unowned registrations, the shipped core).""" +from __future__ import annotations + +import pytest + +from metaobjects.core_types import core_providers +from metaobjects.errors import ErrorCode, ParseError +from metaobjects.meta.template.meta_template import MetaTemplate +from metaobjects.provider import Provider, compose_registry +from metaobjects.registry import LIBRARY_ATTR_ORIGIN, AttrSchema, TypeDefinition, TypeRegistry + + +def _type(sub_type: str, *names: str) -> TypeDefinition: + return TypeDefinition( + type="template", + sub_type=sub_type, + factory=lambda t, s, n: MetaTemplate(t, s, n), + attrs=[AttrSchema(n, "string") for n in names], + child_rules=[], + ) + + +def _provider(provider_id: str, *types: TypeDefinition, deps: tuple[str, ...] = ()) -> Provider: + p = Provider(provider_id, dependencies=deps) + for t in types: + p.add(t) + return p + + +def test_shadowing_registration_names_both_providers_and_the_attribute() -> None: + with pytest.raises(ParseError) as exc: + compose_registry( + [ + _provider("owner-a", _type("base", "physicalName")), + _provider("owner-b", _type("probe", "physicalName"), deps=("owner-a",)), + ] + ) + assert exc.value.code == ErrorCode.ERR_PROVIDER_ATTR_SHADOW + for needle in ("owner-a", "owner-b", "physicalName"): + assert needle in str(exc.value) + + +def test_extend_onto_the_shared_root_collides_with_a_subtype_name() -> None: + class _Extender(Provider): + def register_types(self, registry: TypeRegistry) -> None: + registry.extend("template", "base", attributes=[AttrSchema("physicalName", "string")]) + + extender = _Extender("owner-b", dependencies=("owner-a",)) + with pytest.raises(ParseError) as exc: + compose_registry( + [ + _provider("owner-a", _type("base", "other"), _type("probe", "physicalName")), + extender, + ] + ) + assert exc.value.code == ErrorCode.ERR_PROVIDER_ATTR_SHADOW + + +def test_same_slot_extend_by_another_provider_keeps_attr_conflict_and_names_the_owner() -> None: + class _Extender(Provider): + def register_types(self, registry: TypeRegistry) -> None: + registry.extend("template", "probe", attributes=[AttrSchema("physicalName", "string")]) + + with pytest.raises(ParseError) as exc: + compose_registry( + [ + _provider("owner-a", _type("probe", "physicalName")), + _Extender("owner-b", dependencies=("owner-a",)), + ] + ) + assert exc.value.code == ErrorCode.ERR_PROVIDER_ATTR_CONFLICT + assert "owner-a" in str(exc.value) and "owner-b" in str(exc.value) + + +def test_sibling_subtypes_may_use_one_name() -> None: + registry = compose_registry( + [ + _provider("owner-a", _type("probe-a", "physicalName")), + _provider("owner-b", _type("probe-b", "physicalName"), deps=("owner-a",)), + ] + ) + assert registry.attr_origin("template", "probe-a", "physicalName") == "owner-a" + assert registry.attr_origin("template", "probe-b", "physicalName") == "owner-b" + + +def test_refused_registration_leaves_no_trace() -> None: + registry = TypeRegistry() + registry._current_provider_id = "owner-a" # noqa: SLF001 + registry.register(_type("base", "physicalName")) + registry._current_provider_id = "owner-b" # noqa: SLF001 + with pytest.raises(ParseError): + registry.register(_type("probe", "physicalName")) + assert registry.find("template", "probe") is None + assert registry.attr_origin("template", "probe", "physicalName") == LIBRARY_ATTR_ORIGIN + + +def test_registration_outside_a_provider_turn_claims_nothing() -> None: + registry = TypeRegistry() + registry.register(_type("base", "physicalName")) + registry._current_provider_id = "owner-b" # noqa: SLF001 + registry.register(_type("probe", "physicalName")) + assert registry.attr_origin("template", "base", "physicalName") == LIBRARY_ATTR_ORIGIN + assert registry.attr_origin("template", "probe", "physicalName") == "owner-b" + + +def test_shipped_core_providers_compose_with_no_collision_and_every_attr_has_an_owner() -> None: + registry = compose_registry(core_providers) + unowned = [ + f"{t}.{s}.{a.name}" + for (t, s) in registry._defs # noqa: SLF001 + for a in registry.attrs_of(t, s) + if registry.attr_origin(t, s, a.name) == LIBRARY_ATTR_ORIGIN + ] + assert unowned == [] diff --git a/server/typescript/packages/cli/src/commands/types.ts b/server/typescript/packages/cli/src/commands/types.ts index 8378583e3b..8e47d88192 100644 --- a/server/typescript/packages/cli/src/commands/types.ts +++ b/server/typescript/packages/cli/src/commands/types.ts @@ -134,6 +134,8 @@ interface Entry { tsOnly: boolean; /** A type's shared root (`.base`): its attrs apply to every subtype. */ sharedRoot: boolean; + /** An attr entry: the id of the provider that registered it (ADR-0062). */ + provider: string | undefined; raw: unknown; // the catalog node } @@ -159,9 +161,10 @@ interface RawAttr { whenToUse?: string; } -function attrRecord(at: RawAttr): Record { +function attrRecord(at: RawAttr, provider?: string): Record { return { name: at.name, + ...(provider !== undefined ? { provider } : {}), valueType: at.valueType, isArray: at.isArray, required: at.required, @@ -187,12 +190,14 @@ function payloadMatch(e: Entry): Record { ...(e.whenToUse !== undefined ? { whenToUse: e.whenToUse } : {}), }; if (e.kind === "subtype") { - out.attrs = (e.raw as { attrs: RawAttr[] }).attrs.map(attrRecord); + const mt = e.raw as { attrs: RawAttr[]; attrProviders: Record }; + out.attrs = mt.attrs.map((at) => attrRecord(at, mt.attrProviders[at.name])); } else { const at = e.raw as RawAttr; out.valueType = at.valueType; out.isArray = at.isArray; out.required = at.required; + if (e.provider !== undefined) out.provider = e.provider; if (at.allowedValues !== undefined) out.allowedValues = at.allowedValues; } return out; @@ -280,13 +285,13 @@ export async function typesCommand(args: string[], fmt: OutputFormat = "text"): kind: "subtype", name: tsName, owner: tsName, type: mt.type, description: mt.description, whenToUse: mt.whenToUse, attrNames: mt.attrs.map((a) => `@${a.name}`), - common: false, tsOnly: !mt.crossPort, sharedRoot: mt.sharedRoot, raw: mt, + common: false, tsOnly: !mt.crossPort, sharedRoot: mt.sharedRoot, provider: undefined, raw: mt, }); for (const at of mt.attrs) { entries.push({ kind: "attr", name: `${tsName} @${at.name}`, owner: tsName, type: mt.type, description: at.description, whenToUse: at.whenToUse, attrNames: [], - common: false, tsOnly: !mt.crossPort, sharedRoot: false, raw: at, + common: false, tsOnly: !mt.crossPort, sharedRoot: false, provider: mt.attrProviders[at.name], raw: at, }); } } @@ -299,7 +304,7 @@ export async function typesCommand(args: string[], fmt: OutputFormat = "text"): entries.push({ kind: "attr", name: `@${at.name}`, owner: "(any node)", type: "", description: at.description, whenToUse: at.whenToUse, attrNames: [], - common: true, tsOnly: false, sharedRoot: false, raw: at, + common: true, tsOnly: false, sharedRoot: false, provider: catalog.commonAttrProviders[at.name], raw: at, }); } @@ -359,12 +364,19 @@ export async function typesCommand(args: string[], fmt: OutputFormat = "text"): log.info(" TypeScript-only: registered here, but not part of the cross-port metamodel contract."); if (e.whenToUse) log.info(` → reach for it when: ${e.whenToUse}`); if (e.kind === "subtype") { - const mt = e.raw as { attrs: { name: string; valueType: string | null; required: boolean; description: string }[] }; + const mt = e.raw as { + attrs: { name: string; valueType: string | null; required: boolean; description: string }[]; + attrProviders: Record; + }; if (mt.attrs.length) { log.info(" @attrs:"); - for (const at of mt.attrs) - log.info(` @${at.name} ${at.valueType ?? "any"}${at.required ? " REQUIRED" : ""} ${at.description}`); + for (const at of mt.attrs) { + const by = mt.attrProviders[at.name]; + log.info(` @${at.name} ${at.valueType ?? "any"}${at.required ? " REQUIRED" : ""}${by !== undefined ? ` [${by}]` : ""} ${at.description}`); + } } + } else if (e.provider !== undefined) { + log.info(` registered by provider: ${e.provider}`); } } } else { diff --git a/server/typescript/packages/cli/test/types-command.test.ts b/server/typescript/packages/cli/test/types-command.test.ts index 91acd508e5..47d7b153ad 100644 --- a/server/typescript/packages/cli/test/types-command.test.ts +++ b/server/typescript/packages/cli/test/types-command.test.ts @@ -318,3 +318,26 @@ describe("--detail prints each attr once", () => { expect(out).toContain("field.string @maxLength (attr)"); }); }); + +describe("ADR-0062 every attribute names the provider that registered it", () => { + test("--detail lists the owning provider beside each @attr", async () => { + const out = await run(["field.string", "--detail"]); + // @column is registered by the db provider onto a type the core provider owns. + expect(out).toMatch(/@column .*\[metaobjects-db\]/); + }); + + test("the JSON answer carries `provider` on each attr", async () => { + const calls: string[] = []; + const spy = spyOn(console, "log").mockImplementation((...a: unknown[]) => { + calls.push(a.map(String).join(" ")); + }); + try { + expect(await typesCommand(["field.string", "--limit", "0"], "json")).toBe(0); + } finally { + spy.mockRestore(); + } + const doc = JSON.parse(calls[0] as string) as { matches: { name: string; attrs?: { name: string; provider?: string }[] }[] }; + const subtype = doc.matches.find((m) => m.name === "field.string"); + expect(subtype?.attrs?.find((a) => a.name === "column")?.provider).toBe("metaobjects-db"); + }); +}); diff --git a/server/typescript/packages/metadata/src/errors.ts b/server/typescript/packages/metadata/src/errors.ts index 504ca1ec0d..e9616a15a8 100644 --- a/server/typescript/packages/metadata/src/errors.ts +++ b/server/typescript/packages/metadata/src/errors.ts @@ -76,6 +76,9 @@ export const ERROR_CODES = [ "ERR_PROVIDER_DUPLICATE_ID", "ERR_PROVIDER_MISSING_DEPENDENCY", "ERR_PROVIDER_ATTR_CONFLICT", + // ADR-0062 — a provider declared an attribute name another provider already declares on + // the type it inherits from (or on a subtype that inherits from it). + "ERR_PROVIDER_ATTR_SHADOW", "ERR_EXTEND_REQUIRED_ATTR", "ERR_MALFORMED_JSON", "ERR_MISSING_REQUIRED_ATTR", diff --git a/server/typescript/packages/metadata/src/provider.ts b/server/typescript/packages/metadata/src/provider.ts index 1aabdc5ee1..88f74fed53 100644 --- a/server/typescript/packages/metadata/src/provider.ts +++ b/server/typescript/packages/metadata/src/provider.ts @@ -35,7 +35,9 @@ export function composeRegistry( const ordered = topoSortProviders(providers); const registry = new TypeRegistry(); for (const provider of ordered) { - provider.registerTypes(registry); + // ADR-0062: the registry attributes every attribute registered or extended during + // this turn to the provider, and refuses another provider's claim on the same name. + registry.runAsProvider(provider.id, () => provider.registerTypes(registry)); } // FR-033 — opt-in metamodel contradiction gate. The library bootstrap (the // loader's default registry) sets validate:true so a contradictory provider set diff --git a/server/typescript/packages/metadata/src/registry.ts b/server/typescript/packages/metadata/src/registry.ts index edb12b2014..426da223fd 100644 --- a/server/typescript/packages/metadata/src/registry.ts +++ b/server/typescript/packages/metadata/src/registry.ts @@ -129,6 +129,20 @@ export class TypeRegistry { /** Keyed by `"type.subType"`. */ private readonly _defs = new Map(); + /** + * ADR-0062 — the provider whose `registerTypes` turn is running, set by + * `runAsProvider` (which `composeRegistry` wraps around every provider). `undefined` + * outside a compose loop (a hand-built registry in a unit test), where nothing is + * attributed and nothing can collide. + */ + private _currentProviderId: string | undefined; + + /** ADR-0062 — `"type.subType"` -> (attr name -> id of the provider that registered it). */ + private readonly _attrOwners = new Map>(); + + /** ADR-0062 — common attr name -> id of the provider that registered it. */ + private readonly _commonAttrOwners = new Map(); + /** Per-type insertion-ordered subtype lists. */ private readonly _subTypes = new Map(); @@ -158,6 +172,83 @@ export class TypeRegistry { return this._sealed; } + /** + * Run `fn` with `providerId` recorded as the owner of every attribute registered or + * extended during the call (ADR-0062). `composeRegistry` wraps each provider's + * `registerTypes` in this; a hand-rolled composition may do the same. + */ + runAsProvider(providerId: string, fn: () => T): T { + const previous = this._currentProviderId; + this._currentProviderId = providerId; + try { + return fn(); + } finally { + this._currentProviderId = previous; + } + } + + /** + * The id of the provider that registered attribute `name` on exactly + * `(type, subType)`, or `undefined` when it is not registered there or was registered + * outside any provider's turn. Inherited attributes report the id of their declaring + * slot, so ask `.base` for those. Used by the collision rule and by `meta types`. + */ + attrOwner(type: string, subType: string, name: string): string | undefined { + return this._attrOwners.get(`${type}.${subType}`)?.get(name); + } + + private stampAttrOwner(type: string, subType: string, name: string): void { + if (this._currentProviderId === undefined) return; + const key = `${type}.${subType}`; + let owners = this._attrOwners.get(key); + if (owners === undefined) { + owners = new Map(); + this._attrOwners.set(key, owners); + } + owners.set(name, this._currentProviderId); + } + + /** + * ADR-0062 — the provider-collision rule. Provider P may not put attribute `name` on + * `(type, subType)` when a DIFFERENT provider Q already put the same name on a related + * slot: the slot itself, a type it inherits from (`.base`), or — when this slot + * is the shared root — any subtype that inherits from it. Checked in both directions + * so the verdict does not depend on which provider happened to register first. The + * same provider declaring a name on several related slots stays legal, and an + * attribute registered outside any provider's turn claims nothing. + */ + private checkAttrOwnership(type: string, subType: string, name: string): void { + const claimant = this._currentProviderId; + if (claimant === undefined) return; + const related: string[] = []; + if (subType === SUBTYPE_BASE) { + for (const sub of this._subTypes.get(type) ?? []) { + if (sub !== SUBTYPE_BASE) related.push(sub); + } + } else if (this._defs.has(`${type}.${SUBTYPE_BASE}`)) { + related.push(SUBTYPE_BASE); + } + related.unshift(subType); + for (const sub of related) { + const owner = this._attrOwners.get(`${type}.${sub}`)?.get(name); + if (owner === undefined || owner === claimant) continue; + if (sub === subType) { + throw new MetaModelError( + `TypeRegistry.extend: attribute "${name}" is already declared on "${type}.${subType}" ` + + `by provider "${owner}"; provider "${claimant}" cannot declare it again (ADR-0062)`, + { code: "ERR_PROVIDER_ATTR_CONFLICT" }, + ); + } + throw new MetaModelError( + `Provider "${claimant}" declares attribute "${name}" on "${type}.${subType}", but provider ` + + `"${owner}" already declares it on the related slot "${type}.${sub}". Two providers ` + + `may not claim one attribute name on a type and the type it inherits from; give the ` + + `second provider its own idiomatic name (ADR-0062).`, + { code: "ERR_PROVIDER_ATTR_SHADOW" }, + ); + } + } + /** Guard at the top of every mutating registration method. */ private checkNotSealed(operation: string): void { if (this._sealed) { @@ -194,14 +285,21 @@ export class TypeRegistry { } } - this._defs.set(key, def); - + // ADR-0062: refuse a name another provider already put on a related slot, BEFORE + // the definition becomes visible, so a refused registration leaves no trace. const list = this._subTypes.get(def.typeId.type); + for (const attr of def.attributes) { + this.checkAttrOwnership(def.typeId.type, def.typeId.subType, attr.name); + } + this._defs.set(key, def); if (list !== undefined) { list.push(def.typeId.subType); } else { this._subTypes.set(def.typeId.type, [def.typeId.subType]); } + for (const attr of def.attributes) { + this.stampAttrOwner(def.typeId.type, def.typeId.subType, attr.name); + } } find(type: string, subType: string): TypeDefinition | undefined { @@ -270,9 +368,15 @@ export class TypeRegistry { continue; // dedupe same-name re-registration; conflict-with-per-type-attr is checked at validation time } this._commonAttrs.push(attr); + if (this._currentProviderId !== undefined) this._commonAttrOwners.set(attr.name, this._currentProviderId); } } + /** The id of the provider that registered common attr `name`, if one was composed in. */ + commonAttrOwner(name: string): string | undefined { + return this._commonAttrOwners.get(name); + } + /** Returns the registered common attrs (attrs accepted on every metatype). * Returns a defensive copy so callers cannot mutate internal state, matching * the spread-copy pattern used by `attrsOf` and `allSubTypesOf`. */ @@ -306,11 +410,14 @@ export class TypeRegistry { ); } if (def.attributes.some((a) => a.name === attr.name)) { + // ADR-0062: name the provider that holds the slot when it is a different one. + this.checkAttrOwnership(type, subType, attr.name); throw new MetaModelError( `TypeRegistry.extend: attribute "${attr.name}" is already declared on "${type}.${subType}"`, { code: "ERR_PROVIDER_ATTR_CONFLICT" }, ); } + this.checkAttrOwnership(type, subType, attr.name); // ADR-0050: a REQUIRED attr may never be PROJECTED onto someone else's type. // `extend` is how a concern provider decorates a type another provider owns, and // a concern can be composed out. A required attr arriving this way makes the @@ -332,6 +439,7 @@ export class TypeRegistry { ); } def.attributes.push(attr); + this.stampAttrOwner(type, subType, attr.name); } for (const rule of ext.childRules ?? []) { def.childRules.push(rule); diff --git a/server/typescript/packages/metadata/src/vocabulary-catalog.ts b/server/typescript/packages/metadata/src/vocabulary-catalog.ts index 953b168265..0eaa1481e9 100644 --- a/server/typescript/packages/metadata/src/vocabulary-catalog.ts +++ b/server/typescript/packages/metadata/src/vocabulary-catalog.ts @@ -42,6 +42,12 @@ export interface VocabularyType { example?: string; whenToUse?: string; attrs: ManifestAttr[]; + /** + * `attr name -> id of the provider that registered it on exactly this (type, subType)` + * (ADR-0062). Only attrs a composed provider registered appear; an attr this slot + * inherits from `.base` is listed on the base row. + */ + attrProviders: Record; children: ManifestChild[]; parents?: string[]; /** @@ -68,10 +74,12 @@ export interface VocabularyCatalog { types: VocabularyType[]; /** Attrs accepted on EVERY node, whatever its type (`@title`, `@description`, …). */ commonAttrs: ManifestAttr[]; + /** `common attr name -> id of the provider that registered it` (ADR-0062). */ + commonAttrProviders: Record; defaultSubTypes: Record; } -function toVocabularyType(def: TypeDefinition): VocabularyType { +function toVocabularyType(def: TypeDefinition, registry: TypeRegistry): VocabularyType { const { type, subType } = def.typeId; const reason = classifyTypeSubType(type, subType); const out: VocabularyType = { @@ -89,11 +97,16 @@ function toVocabularyType(def: TypeDefinition): VocabularyType { // CROSS-PORT value during a reference-first rollout, and this answer is about what // THIS registry requires of an author right now. attrs: sortedAttrs(def.attributes.filter((a) => classifyPerTypeAttr(a.name) === INCLUDED)), + attrProviders: {}, children: sortedChildren(def.childRules), crossPort: reason === undefined, ...(reason !== undefined ? { portPrivateReason: reason } : {}), sharedRoot: subType === SUBTYPE_BASE, }; + for (const attr of out.attrs) { + const owner = registry.attrOwner(type, subType, attr.name); + if (owner !== undefined) out.attrProviders[attr.name] = owner; + } if (def.parents !== undefined && def.parents.length > 0) { out.parents = [...def.parents].sort(compareStrings); } @@ -112,7 +125,7 @@ function toVocabularyType(def: TypeDefinition): VocabularyType { export function buildVocabularyCatalog(registry: TypeRegistry): VocabularyCatalog { const types = registry .allTypes() - .map((typeId) => toVocabularyType(registry.find(typeId.type, typeId.subType) as TypeDefinition)) + .map((typeId) => toVocabularyType(registry.find(typeId.type, typeId.subType) as TypeDefinition, registry)) .sort((a, b) => compareStrings(`${a.type}.${a.subType}`, `${b.type}.${b.subType}`), ); @@ -124,10 +137,18 @@ export function buildVocabularyCatalog(registry: TypeRegistry): VocabularyCatalo if (defaultSub !== undefined) defaultSubTypes[typeName] = defaultSub; } + const commonAttrs = sortedAttrs(registry.getCommonAttrs()); + const commonAttrProviders: Record = {}; + for (const attr of commonAttrs) { + const owner = registry.commonAttrOwner(attr.name); + if (owner !== undefined) commonAttrProviders[attr.name] = owner; + } + return { metamodelVersion: METAMODEL_VERSION, types, - commonAttrs: sortedAttrs(registry.getCommonAttrs()), + commonAttrs, + commonAttrProviders, defaultSubTypes, }; } diff --git a/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts b/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts index 9ed1034141..d94065564d 100644 --- a/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts +++ b/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts @@ -19,7 +19,7 @@ import type { MetaDataTypeProvider } from "../src/provider.js"; import { composeRegistry } from "../src/provider.js"; import { TypeRegistry, TypeId } from "../src/registry.js"; import { MetaTemplate } from "../src/template/meta-template.js"; -import { TYPE_TEMPLATE, TYPE_ATTR, TYPE_VIEW } from "../src/shared/base-types.js"; +import { TYPE_TEMPLATE, TYPE_ATTR, TYPE_VIEW, SUBTYPE_BASE } from "../src/shared/base-types.js"; import { CHILD_RULE_WILDCARD } from "../src/shared/structural.js"; import { ATTR_SUBTYPE_STRING, ATTR_SUBTYPE_INT } from "../src/core/attr/attr-constants.js"; import { VIEW_SUBTYPE_CURRENCY } from "../src/presentation/view/view-constants.js"; @@ -147,7 +147,60 @@ const extendSpecSubtypeProvider: MetaDataTypeProvider = { }, }; +// ADR-0062 — attribute-ownership providers (see the corpus README). All register on +// `template.base` and the fresh subtype `template.shadowprobe`. +const SHADOW_SUBTYPE = "shadowprobe"; + +function templateType(subType: string, attrName: string): Parameters[0] { + return { + typeId: new TypeId(TYPE_TEMPLATE, subType), + description: "Test-only — attribute-ownership probe.", + factory: (typeId, name) => new MetaTemplate(typeId, name), + childRules: [], + attributes: [ + { name: attrName, valueType: ATTR_SUBTYPE_STRING, required: false, description: "Ownership probe attr." }, + ], + }; +} + +const shadowBaseProvider: MetaDataTypeProvider = { + id: "shadow-base", + registerTypes: (registry) => registry.register(templateType(SUBTYPE_BASE, "shadowAttr")), +}; +const shadowSubClashProvider: MetaDataTypeProvider = { + id: "shadow-sub-clash", + dependencies: ["shadow-base"], + registerTypes: (registry) => registry.register(templateType(SHADOW_SUBTYPE, "shadowAttr")), +}; +const shadowSubOwnerProvider: MetaDataTypeProvider = { + id: "shadow-sub-owner", + registerTypes: (registry) => registry.register(templateType(SHADOW_SUBTYPE, "reverseAttr")), +}; +const shadowBaseLateProvider: MetaDataTypeProvider = { + id: "shadow-base-late", + dependencies: ["shadow-sub-owner"], + registerTypes: (registry) => registry.register(templateType(SUBTYPE_BASE, "reverseAttr")), +}; +const shadowSubDistinctProvider: MetaDataTypeProvider = { + id: "shadow-sub-distinct", + dependencies: ["shadow-base"], + registerTypes: (registry) => registry.register(templateType(SHADOW_SUBTYPE, "distinctAttr")), +}; +const shadowSelfProvider: MetaDataTypeProvider = { + id: "shadow-self", + registerTypes(registry) { + registry.register(templateType(SUBTYPE_BASE, "selfAttr")); + registry.register(templateType(SHADOW_SUBTYPE, "selfAttr")); + }, +}; + const PROVIDERS: Readonly> = { + "shadow-base": shadowBaseProvider, + "shadow-sub-clash": shadowSubClashProvider, + "shadow-sub-owner": shadowSubOwnerProvider, + "shadow-base-late": shadowBaseLateProvider, + "shadow-sub-distinct": shadowSubDistinctProvider, + "shadow-self": shadowSelfProvider, "duplicate-x": duplicateXProvider, "duplicate-x-clone": duplicateXCloneProvider, "depends-on-missing": dependsOnMissingProvider, @@ -176,6 +229,8 @@ interface Manifest { expectAttrs?: ExpectAttrs; metadata?: unknown; expectErrors?: string[]; + // ADR-0062 attr-ownership shape — see README.md "The `attr-ownership/` subdir". + expectOwners?: { type: string; subType: string; attr: string; owner: string }[]; } /** Pull the stable ERR_ code off a caught error, if it carries one. */ @@ -280,3 +335,28 @@ for (const file of composeLoadManifestFiles) { } }); } + +// --------------------------------------------------------------------------- +// ADR-0062 attr-ownership corpus — composition succeeds and the registry names +// the provider that owns each attribute. +// --------------------------------------------------------------------------- + +const OWNERSHIP_CORPUS = join(CORPUS, "attr-ownership"); +const ownershipFiles = readdirSync(OWNERSHIP_CORPUS) + .filter((f) => f.endsWith(".json")) + .sort(); + +test("provider-composition attr-ownership corpus is non-empty (guards against a mis-pathed OWNERSHIP_CORPUS)", () => { + expect(ownershipFiles.length).toBeGreaterThan(0); +}); + +for (const file of ownershipFiles) { + test(`provider-composition (attr-ownership): ${file}`, async () => { + const manifest: Manifest = await Bun.file(join(OWNERSHIP_CORPUS, file)).json(); + const registry = composeRegistry(manifest.providers.map(resolve)); + expect(manifest.expectOwners?.length ?? 0).toBeGreaterThan(0); + for (const { type, subType, attr, owner } of manifest.expectOwners ?? []) { + expect(registry.attrOwner(type, subType, attr)).toBe(owner); + } + }); +} diff --git a/server/typescript/packages/metadata/test/registry-attr-ownership.test.ts b/server/typescript/packages/metadata/test/registry-attr-ownership.test.ts new file mode 100644 index 0000000000..80af17ebf3 --- /dev/null +++ b/server/typescript/packages/metadata/test/registry-attr-ownership.test.ts @@ -0,0 +1,125 @@ +// ADR-0062 — the registry records which provider registered each attribute and refuses a +// second provider's claim on the same name. The cross-port scenarios live in +// fixtures/provider-composition-conformance/; this file pins the TS-specific details the +// shared corpus cannot (error text, no-trace-on-refusal, unowned registrations). + +import { describe, expect, test } from "bun:test"; +import { composeRegistry, type MetaDataTypeProvider } from "../src/provider.js"; +import { TypeId, TypeRegistry, type TypeDefinition } from "../src/registry.js"; +import { MetaTemplate } from "../src/template/meta-template.js"; +import { TYPE_TEMPLATE, SUBTYPE_BASE } from "../src/shared/base-types.js"; +import { ATTR_SUBTYPE_STRING } from "../src/core/attr/attr-constants.js"; +import { coreProviders } from "../src/core-types.js"; +import { MetaModelError } from "../src/errors.js"; + +function def(subType: string, ...attrNames: string[]): TypeDefinition { + return { + typeId: new TypeId(TYPE_TEMPLATE, subType), + description: "test", + factory: (typeId, name) => new MetaTemplate(typeId, name), + childRules: [], + attributes: attrNames.map((name) => ({ + name, + valueType: ATTR_SUBTYPE_STRING, + required: false, + description: name, + })), + }; +} + +function provider( + id: string, + registerTypes: MetaDataTypeProvider["registerTypes"], + dependencies: string[] = [], +): MetaDataTypeProvider { + return { id, dependencies, registerTypes }; +} + +function codeOf(fn: () => unknown): { code: string; message: string } { + try { + fn(); + } catch (err) { + const e = err as MetaModelError; + return { code: e.code ?? "", message: e.message }; + } + throw new Error("expected the call to throw"); +} + +describe("attribute ownership (ADR-0062)", () => { + test("a shadowing registration names both providers and the attribute", () => { + const { code, message } = codeOf(() => + composeRegistry([ + provider("owner-a", (r) => r.register(def(SUBTYPE_BASE, "physicalName"))), + provider("owner-b", (r) => r.register(def("probe", "physicalName")), ["owner-a"]), + ]), + ); + expect(code).toBe("ERR_PROVIDER_ATTR_SHADOW"); + expect(message).toContain("owner-a"); + expect(message).toContain("owner-b"); + expect(message).toContain("physicalName"); + }); + + test("a same-slot extend by another provider keeps ERR_PROVIDER_ATTR_CONFLICT and names the owner", () => { + const { code, message } = codeOf(() => + composeRegistry([ + provider("owner-a", (r) => r.register(def("probe", "physicalName"))), + provider("owner-b", (r) => r.extend(TYPE_TEMPLATE, "probe", { attributes: def("probe", "physicalName").attributes }), ["owner-a"]), + ]), + ); + expect(code).toBe("ERR_PROVIDER_ATTR_CONFLICT"); + expect(message).toContain("owner-a"); + expect(message).toContain("owner-b"); + }); + + test("an extend onto the shared root collides with a subtype's name", () => { + const { code } = codeOf(() => + composeRegistry([ + provider("owner-a", (r) => { + r.register(def(SUBTYPE_BASE, "other")); + r.register(def("probe", "physicalName")); + }), + provider("owner-b", (r) => r.extend(TYPE_TEMPLATE, SUBTYPE_BASE, { attributes: def("x", "physicalName").attributes }), ["owner-a"]), + ]), + ); + expect(code).toBe("ERR_PROVIDER_ATTR_SHADOW"); + }); + + test("sibling subtypes may use one name: they are different slots", () => { + const registry = composeRegistry([ + provider("owner-a", (r) => r.register(def("probe-a", "physicalName"))), + provider("owner-b", (r) => r.register(def("probe-b", "physicalName")), ["owner-a"]), + ]); + expect(registry.attrOwner(TYPE_TEMPLATE, "probe-a", "physicalName")).toBe("owner-a"); + expect(registry.attrOwner(TYPE_TEMPLATE, "probe-b", "physicalName")).toBe("owner-b"); + }); + + test("a refused registration leaves no trace in the registry", () => { + const registry = new TypeRegistry(); + registry.runAsProvider("owner-a", () => registry.register(def(SUBTYPE_BASE, "physicalName"))); + expect(() => registry.runAsProvider("owner-b", () => registry.register(def("probe", "physicalName")))).toThrow(); + expect(registry.has(TYPE_TEMPLATE, "probe")).toBe(false); + expect(registry.allSubTypesOf(TYPE_TEMPLATE)).toEqual([SUBTYPE_BASE]); + expect(registry.attrOwner(TYPE_TEMPLATE, "probe", "physicalName")).toBeUndefined(); + }); + + test("a registration outside any provider's turn claims nothing and collides with nothing", () => { + const registry = new TypeRegistry(); + registry.register(def(SUBTYPE_BASE, "physicalName")); + registry.runAsProvider("owner-b", () => registry.register(def("probe", "physicalName"))); + expect(registry.attrOwner(TYPE_TEMPLATE, SUBTYPE_BASE, "physicalName")).toBeUndefined(); + expect(registry.attrOwner(TYPE_TEMPLATE, "probe", "physicalName")).toBe("owner-b"); + }); + + test("the shipped core providers compose with no collision and name an owner for every attribute", () => { + const registry = composeRegistry([...coreProviders], { validate: true }); + const unowned: string[] = []; + for (const id of registry.allTypes()) { + for (const attr of registry.attrsOf(id.type, id.subType)) { + if (registry.attrOwner(id.type, id.subType, attr.name) === undefined) { + unowned.push(`${id.toString()}.${attr.name}`); + } + } + } + expect(unowned).toEqual([]); + }); +}); diff --git a/spec/decisions/ADR-0062-provider-attribute-ownership.md b/spec/decisions/ADR-0062-provider-attribute-ownership.md new file mode 100644 index 0000000000..4eb646e631 --- /dev/null +++ b/spec/decisions/ADR-0062-provider-attribute-ownership.md @@ -0,0 +1,82 @@ +# ADR-0062: Two providers may not claim one attribute name on a node type + +## Status + +**Accepted** (2026-11-04). Registers no vocabulary: `metamodelVersion` is unchanged. Adds one +error code, `ERR_PROVIDER_ATTR_SHADOW`, and records which provider registered each attribute. +Amends nothing; it states how ADR-0007 and ADR-0018 are meant to scale. Closes #527. + +## Context + +ADR-0007 gave each storage paradigm its own physical-name word on the node it describes +(`@column` for a relational field, `@field` for a document field) and rejected a shared +`@locator`-style attribute, because one word cannot hold a multi-sourced field's address in two +stores at once. ADR-0018 did the same per kind (`@table`, `@view`). That scheme works only while +no two providers choose the same word. A provider is free to register or extend a type, and +the registry used to stop only the narrowest case: a second `extend` of the same +`(type, subType)` redeclaring an attribute (`ERR_PROVIDER_ATTR_CONFLICT`). It did not stop a provider +that put `@column` on `field.base` while another already had `@column` on `field.string`, or +the reverse. Both registered, the loader saw one name with two meanings, and which won depended +on registration order. Issue #527 asked where a per-provider physical name goes and what stops +a collision. + +## Decision + +1. **A per-paradigm idiomatic word, unique across providers on a node type.** Each provider + names its physical address in the word its own store uses. Two providers do not share a word + on a node type. The registry enforces it at registration, not in review. + +2. **The rule.** A provider that registers or extends `(T, S)` with attribute `A` is refused when + a different provider already registered `A` on a *related slot* of the same node type `T`: + - the same slot `(T, S)` — `ERR_PROVIDER_ATTR_CONFLICT` (unchanged code; its message now also + names the owning provider); + - an inherited slot, the shared root `(T, base)` — `ERR_PROVIDER_ATTR_SHADOW`; + - when `S` is the shared root, any registered subtype `(T, S')` — `ERR_PROVIDER_ATTR_SHADOW`. + + The check runs in both directions, so the verdict does not depend on which provider registers + first. A refusal names both providers and the attribute, and leaves nothing behind in the + registry. + +3. **What stays legal.** The same provider declaring a name on several related slots (its own + `@x` on the root and on a subtype) and re-registering its own slot. Sibling subtypes using + one name, because they are different slots and never co-resolve. Registration outside any + provider's turn (a direct `registry.register` call in a test or script) claims nothing. + +4. **Scope is one node type.** The JVM registry also chains every `.base` to + `metadata.base`, which the core-types provider owns. No other port has that edge, and seven + shipped providers re-declare `@isAbstract` and/or `@description` on their own root with the + same meaning. The rule stops at the node-type boundary so all five ports reach the same + verdict, and no shipped provider has to change. + +5. **Owner tracking.** The registry records the registering provider per `(type, subType, + attribute)`, and per common attribute. The error uses it, and `meta types` shows it + (`[]` beside each attribute with `--detail`, and a `provider` field in JSON). The + owner is not written into the expected-registry manifest, which is byte-matched across ports. + +6. **A store with no provider uses a property bag.** Vocabulary an adopter has no provider for + goes in the registered `attr.properties` bag (ADR-0023), not in an invented attribute. + +7. **No wire-name attribute is added here.** If one is added later, it follows this rule with + its own idiomatic word, and does not take a name another provider already holds. + +## Consequences + +- The shipped core providers compose with no collision in any port (audited before the check was + turned on), so existing adopters see no change. +- A third-party provider that reuses a core or sibling provider's attribute name now fails at + composition with a message that names both providers. The fix is a different idiomatic word. +- The two codes split by where the clash is, not by severity: `ERR_PROVIDER_ATTR_CONFLICT` keeps + its established same-slot meaning and fixture, and `ERR_PROVIDER_ATTR_SHADOW` is the new + inherited/descendant case. +- Conformance: `fixtures/provider-composition-conformance/` carries the refusals + (`attr-shadow-inherited`, `attr-shadow-descendant`) and an `attr-ownership/` sub-corpus where + composition succeeds and each port reports the same owner per attribute. + +## Alternatives considered + +- **A generic physical-address attribute** — rejected in ADR-0007 for the same reasons. +- **Prefix every provider attribute with the provider id** — rejected in ADR-0018: it makes the + authoring surface ugly to fix a problem a registry check solves. +- **Last registration wins** — what happened before; the result depends on provider order. +- **One new code for all three cases** — would have changed the established + `ERR_PROVIDER_ATTR_CONFLICT` fixture and message for existing consumers. diff --git a/spec/decisions/README.md b/spec/decisions/README.md index 02ad241caf..6e0de22349 100644 --- a/spec/decisions/README.md +++ b/spec/decisions/README.md @@ -58,6 +58,7 @@ Write one when a decision is **cross-cutting** (affects more than one language p - [ADR-0059 — References resolve once, after the tree](ADR-0059-references-resolve-once-after-the-tree.md) — *Accepted* (sources keep references as written; one resolver per port binds them before freeze and the serializer writes them canonical; supersedes ADR-0032 §3–§4, amends ADR-0042) - [ADR-0060 — Java and Kotlin codegen decide overwrites from the committed hash manifest](ADR-0060-jvm-codegen-hash-manifest.md) — *Accepted* (the JVM plugin records each file it writes in `.metaobjects/.gen-state/.hashes.json`, refuses an edited one, and deletes an untouched orphan, owned per generator and output directory) - [ADR-0061 — A template body may use a per-port engine, chosen by its file](ADR-0061-per-port-template-engines.md) — *Accepted* (Python and TypeScript render and check Jinja bodies — `.jinja` / `.j2` / `.jinja2` — against the same payload check; the metadata never names an engine; Java, Kotlin and C# refuse Jinja with `ERR_TEMPLATE_ENGINE_UNSUPPORTED`) +- [ADR-0062 — Two providers may not claim one attribute name on a node type](ADR-0062-provider-attribute-ownership.md) — *Accepted* (the registry records which provider registered each attribute and refuses a second provider's claim on the same name on the same slot, the shared root or a descendant subtype: `ERR_PROVIDER_ATTR_CONFLICT` for the same slot, new `ERR_PROVIDER_ATTR_SHADOW` for the related slots; each provider keeps its own idiomatic physical-name word, a store without a provider uses `attr.properties`, no wire-name attribute is added) > **Index gap — ADR-0031 through ADR-0051 are on disk but not listed above.** The index stopped > being maintained after ADR-0030; the files are authoritative, this list is not. Read From d93a7859247e562416a782cbfaaef582de935214 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Wed, 4 Nov 2026 03:48:21 -0500 Subject: [PATCH 2/6] no-mistakes(review): apply reviewer inline fixes --- docs/features/extending-with-providers.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/features/extending-with-providers.md b/docs/features/extending-with-providers.md index 108f6c695d..99e6fd75d2 100644 --- a/docs/features/extending-with-providers.md +++ b/docs/features/extending-with-providers.md @@ -41,7 +41,7 @@ unregistered id throws `ERR_PROVIDER_MISSING_DEPENDENCY`; a cycle throws ## The three rules that govern every provider -Before the mechanics, the three rules a provider must obey. Both are machine-enforced — +Before the mechanics, the three rules a provider must obey. All three are machine-enforced — you will hit an error, not a review comment. ### 1. Own vs projected attributes (ADR-0050) From 805e3e3c84eb2aa54262be8d9d0540a74959c2e1 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Wed, 4 Nov 2026 04:08:48 -0500 Subject: [PATCH 3/6] no-mistakes(review): Close Java extend shadow gap; refuse duplicate common attrs --- CHANGELOG.md | 11 +- docs/CONFORMANCE.md | 2 +- docs/features/extending-with-providers.md | 8 +- .../README.md | 32 +++- .../attr-ownership/common-attr-owners.json | 17 ++ .../attr-shadow-extend.json | 9 ++ .../common-attr-conflict.json | 8 + .../ProviderCompositionConformanceTests.cs | 66 +++++++- server/csharp/MetaObjects/Registry.cs | 38 ++++- .../registry/MetaDataRegistry.java | 148 +++++++++++------- .../registry/TypeDefinitionBuilder.java | 27 +++- .../ProviderCompositionConformanceTest.java | 35 ++++- .../registry/AttrOwnershipTest.java | 27 ++++ server/python/src/metaobjects/registry.py | 30 +++- .../test_provider_composition_conformance.py | 33 +++- .../packages/metadata/src/registry.ts | 23 ++- .../provider-composition-conformance.test.ts | 44 +++++- .../ADR-0062-provider-attribute-ownership.md | 21 ++- 18 files changed, 482 insertions(+), 97 deletions(-) create mode 100644 fixtures/provider-composition-conformance/attr-ownership/common-attr-owners.json create mode 100644 fixtures/provider-composition-conformance/attr-shadow-extend.json create mode 100644 fixtures/provider-composition-conformance/common-attr-conflict.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 398f233329..f3b3e17a44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,10 +37,15 @@ here.** sibling subtypes sharing a name, and registration outside a provider all stay legal. The shipped core providers have no such collision, so existing projects see no change. `meta types --detail` prints the registering provider beside each attribute, and its JSON carries a `provider` field. - This is how per-paradigm physical-name words (`@column`, `@field`) stay safe as providers - multiply; no wire-name attribute is added. See ADR-0062 and + Two providers registering one common attribute name is also `ERR_PROVIDER_ATTR_CONFLICT` (it + used to be first-wins), and the registry records each common attribute's provider. A common + attribute and a per-type attribute of the same name from two providers are not refused yet: + Java's core providers declare a per-type `@description` beside the documentation provider's + common one. This is how per-paradigm physical-name words (`@column`, `@field`) stay safe as + providers multiply; no wire-name attribute is added. See ADR-0062 and `docs/features/extending-with-providers.md`. New conformance fixtures: `attr-shadow-inherited`, - `attr-shadow-descendant` and the `attr-ownership/` sub-corpus. Fixes #527. + `attr-shadow-descendant`, `attr-shadow-extend`, `common-attr-conflict` and the + `attr-ownership/` sub-corpus. Fixes #527. ### Fixed diff --git a/docs/CONFORMANCE.md b/docs/CONFORMANCE.md index 27cee9166f..f2c2a53fa3 100644 --- a/docs/CONFORMANCE.md +++ b/docs/CONFORMANCE.md @@ -45,7 +45,7 @@ regenerate with `ls -d fixtures//*/ | wc -l` for directory-shaped corpor | [`fixtures/template-codegen-conformance/`](../fixtures/template-codegen-conformance/) | 3 | ✓ | ✓ | ✓ | ✓ | ✓ | | [`fixtures/template-output-render-conformance/`](../fixtures/template-output-render-conformance/) | 5 | ✓ | ✓ | ✓ | ✓ | ✓ | | [`fixtures/generator-registry-conformance/`](../fixtures/generator-registry-conformance/) | 1 canonical manifest | ✓ | ✓ | ✓ | ✓ | ✓ | -| [`fixtures/provider-composition-conformance/`](../fixtures/provider-composition-conformance/) | 13 (7 error-shape + 4 compose-load + 2 attr-ownership) | ✓ | ✓ | — (JVM registry via Java) | ✓ | ✓ | +| [`fixtures/provider-composition-conformance/`](../fixtures/provider-composition-conformance/) | 16 (9 error-shape + 4 compose-load + 3 attr-ownership) | ✓ | ✓ | — (JVM registry via Java) | ✓ | ✓ | | [`fixtures/source-resolution-conformance/`](../fixtures/source-resolution-conformance/) | 25 cases | ✓ (reference implementation) | ✓ | inherits via Java | ✓ | ✓ | | [`fixtures/scope-conformance/`](../fixtures/scope-conformance/) | 10 cases | ✓ (reference implementation) | ✓ | inherits via Java | ✓ | ✓ | | [`fixtures/dependency-conformance/`](../fixtures/dependency-conformance/) | 37 cases (26 resolution + 11 check) + 14 transport cases | ✓ (reference implementation; all three arms) | ✓ (resolution arm, all 26; no check, so the 11 check cases are not run) | inherits via Java | ✓ (resolution arm, all 26; no check, so the 11 check cases are not run) | ✓ (resolution arm, all 26 identically; no check yet, so the 11 check cases are not run; the transport arm is Node-only) | diff --git a/docs/features/extending-with-providers.md b/docs/features/extending-with-providers.md index 99e6fd75d2..29fdaf05f0 100644 --- a/docs/features/extending-with-providers.md +++ b/docs/features/extending-with-providers.md @@ -87,7 +87,9 @@ which provider registered each attribute, and refuses a second provider's claim - on the same `(type, subType)` — `ERR_PROVIDER_ATTR_CONFLICT`; - on the shared root `(type, base)` that the subtype inherits from, or on a subtype of it — - `ERR_PROVIDER_ATTR_SHADOW`. + `ERR_PROVIDER_ATTR_SHADOW`, whether the second provider registers the type or `extend`s it; +- as a common attribute (accepted on every node type) another provider already registered — + `ERR_PROVIDER_ATTR_CONFLICT`. The message names both providers and the attribute. The check runs both ways, so it does not matter which provider registers first. The same provider repeating its own name, sibling @@ -518,8 +520,8 @@ Composition surfaces these codes consistently across all ports (see | `ERR_PROVIDER_DUPLICATE_ID` | Two provider objects report the same `id` | `provider-extension-duplicate-id` | | `ERR_PROVIDER_MISSING_DEPENDENCY` | Provider declares dep on an unregistered id | `provider-extension-missing-dependency` | | `ERR_PROVIDER_DEPENDENCY_CYCLE` | Provider dependencies form a cycle | `provider-extension-dependency-cycle` | -| `ERR_PROVIDER_ATTR_CONFLICT` | Two providers declare the same attr on the same `(type, subType)` | `attr-conflict` (provider-composition corpus) | -| `ERR_PROVIDER_ATTR_SHADOW` | A provider declares an attr another provider already holds on the shared root or a descendant subtype of the same type | `attr-shadow-inherited`, `attr-shadow-descendant` (provider-composition corpus) | +| `ERR_PROVIDER_ATTR_CONFLICT` | Two providers declare the same attr on the same `(type, subType)`, or register the same common attr | `attr-conflict`, `common-attr-conflict` (provider-composition corpus) | +| `ERR_PROVIDER_ATTR_SHADOW` | A provider declares an attr another provider already holds on the shared root or a descendant subtype of the same type | `attr-shadow-inherited`, `attr-shadow-descendant`, `attr-shadow-extend` (provider-composition corpus) | Each conformance fixture's `providers.json: string[]` lists the provider ids to compose for that test — the same wire format consumers use at runtime. Each diff --git a/fixtures/provider-composition-conformance/README.md b/fixtures/provider-composition-conformance/README.md index e03d09181b..65896eb341 100644 --- a/fixtures/provider-composition-conformance/README.md +++ b/fixtures/provider-composition-conformance/README.md @@ -11,8 +11,8 @@ cross-port. | `ERR_PROVIDER_DUPLICATE_ID` | Two providers in a composition report the same `id`. | | `ERR_PROVIDER_MISSING_DEPENDENCY` | A provider declares a dependency id absent from the set. | | `ERR_PROVIDER_DEPENDENCY_CYCLE` | Providers form a dependency cycle (topo-sort cannot proceed). | -| `ERR_PROVIDER_ATTR_CONFLICT` | A provider `extend`s a type, redefining an attr another provider already declared. | -| `ERR_PROVIDER_ATTR_SHADOW` | A provider declares an attr name that another provider already declares on the type it inherits from (`.base`), or on a subtype that inherits from it (ADR-0062). | +| `ERR_PROVIDER_ATTR_CONFLICT` | A provider `extend`s a type, redefining an attr another provider already declared; or registers a common attr another provider already registered (ADR-0062). | +| `ERR_PROVIDER_ATTR_SHADOW` | A provider declares an attr name that another provider already declares on the type it inherits from (`.base`), or on a subtype that inherits from it, whether it registers or `extend`s the type (ADR-0062). | | `ERR_REGISTRY_SEALED` | A registration is attempted against a registry sealed after bootstrap (ADR-0023). | ## Corpus shape @@ -127,11 +127,24 @@ their own (no `composeWithCore`), so each registers the root it needs. | `shadow-base-late` | `shadow-sub-owner` | `template.base` with attr `reverseAttr` (the name `shadow-sub-owner` owns on the subtype). | | `shadow-sub-distinct` | `shadow-base` | `template.shadowprobe` with attr `distinctAttr`. | | `shadow-self` | none | `template.base` with attr `selfAttr`, then `template.shadowprobe` with the SAME attr `selfAttr`. | +| `shadow-sub-extend-clash` | `shadow-sub-distinct` | `extend`s `template.shadowprobe` with attr `shadowAttr` (the name `shadow-base` owns on the root, which the subtype only inherits). | Composing `["shadow-base", "shadow-sub-clash"]` and `["shadow-sub-owner", "shadow-base-late"]` → `ERR_PROVIDER_ATTR_SHADOW` (the two directions: the verdict must not depend on which -provider registered first). Two providers claiming one name on the SAME type is the -existing `attr-conflict` scenario (`ERR_PROVIDER_ATTR_CONFLICT`). +provider registered first). Composing `["shadow-base", "shadow-sub-distinct", "shadow-sub-extend-clash"]` +→ `ERR_PROVIDER_ATTR_SHADOW` too: `extend` is held to the same rule as `register`. Two providers +claiming one name on the SAME type is the existing `attr-conflict` scenario +(`ERR_PROVIDER_ATTR_CONFLICT`). + +The common-attr providers each register one optional string common attr: + +| id | dependencies | registers | +|----|--------------|-----------| +| `common-owner` | none | common attr `commonAttr`. | +| `common-clash` | `common-owner` | common attr `commonAttr` (the name `common-owner` owns). | +| `common-self` | none | common attr `selfCommonAttr`, then the SAME common attr again. | + +Composing `["common-owner", "common-clash"]` → `ERR_PROVIDER_ATTR_CONFLICT`. ## The `attr-ownership/` subdir (ADR-0062) @@ -143,16 +156,23 @@ runners list the parent non-recursively and require `expectedError`). Shape: { "description": "...", "providers": ["shadow-base", "shadow-sub-distinct"], // composed in listed order; must succeed - "expectOwners": [ // the port's per-slot attr-owner lookup + "expectOwners": [ // OPTIONAL — the port's per-slot attr-owner lookup { "type": "template", "subType": "base", "attr": "shadowAttr", "owner": "shadow-base" } + ], + "expectCommonOwners": [ // OPTIONAL — the port's common-attr owner lookup + { "attr": "commonAttr", "owner": "common-owner" } ] } ``` +A manifest carries at least one of `expectOwners` / `expectCommonOwners`. + `expectOwners` asks for the provider that registered `attr` on exactly `(type, subType)` — an inherited attr is asked of the slot that declares it. `distinct-names-load.json` proves distinct names load; `same-provider-redeclare.json` proves one provider re-declaring its own name on a type -and on the type it inherits from stays legal. +and on the type it inherits from stays legal; `common-attr-owners.json` proves distinct common attr +names from two providers load, one provider re-registering its own common attr stays legal, and each +port reports the same common-attr owner. ## Notes diff --git a/fixtures/provider-composition-conformance/attr-ownership/common-attr-owners.json b/fixtures/provider-composition-conformance/attr-ownership/common-attr-owners.json new file mode 100644 index 0000000000..c6b5aad2cd --- /dev/null +++ b/fixtures/provider-composition-conformance/attr-ownership/common-attr-owners.json @@ -0,0 +1,17 @@ +{ + "description": "Two providers registering DISTINCT common attr names compose cleanly, one provider re-registering its own common attr stays legal, and the registry records which provider owns each common attr.", + "providers": [ + "common-owner", + "common-self" + ], + "expectCommonOwners": [ + { + "attr": "commonAttr", + "owner": "common-owner" + }, + { + "attr": "selfCommonAttr", + "owner": "common-self" + } + ] +} diff --git a/fixtures/provider-composition-conformance/attr-shadow-extend.json b/fixtures/provider-composition-conformance/attr-shadow-extend.json new file mode 100644 index 0000000000..4db73a32ee --- /dev/null +++ b/fixtures/provider-composition-conformance/attr-shadow-extend.json @@ -0,0 +1,9 @@ +{ + "description": "A provider extends a subtype with an attr name that another provider already declares on the subtype's shared root (.base). The subtype only inherits the name, so the extend is a shadow, not a same-slot redefinition.", + "providers": [ + "shadow-base", + "shadow-sub-distinct", + "shadow-sub-extend-clash" + ], + "expectedError": "ERR_PROVIDER_ATTR_SHADOW" +} diff --git a/fixtures/provider-composition-conformance/common-attr-conflict.json b/fixtures/provider-composition-conformance/common-attr-conflict.json new file mode 100644 index 0000000000..40d0af620a --- /dev/null +++ b/fixtures/provider-composition-conformance/common-attr-conflict.json @@ -0,0 +1,8 @@ +{ + "description": "A provider registers a common attr whose name another provider already registers as a common attr.", + "providers": [ + "common-owner", + "common-clash" + ], + "expectedError": "ERR_PROVIDER_ATTR_CONFLICT" +} diff --git a/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs b/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs index 811043973b..da8642ceb7 100644 --- a/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs +++ b/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs @@ -154,6 +154,53 @@ public void RegisterTypes(TypeRegistry registry) } } +/// ADR-0062 — extends template.shadowprobe with the name shadow-base owns on the root. +internal sealed class ShadowSubExtendClashProvider : IMetaDataTypeProvider +{ + public string Id => "shadow-sub-extend-clash"; + public IReadOnlyList Dependencies => new[] { "shadow-sub-distinct" }; + + public void RegisterTypes(TypeRegistry registry) + { + registry.Extend( + "template", + ShadowTestProvider.ShadowSubType, + attributes: new List + { + new AttrSchema("shadowAttr", "string", Required: false, Description: "Ownership probe attr."), + }); + } +} + +/// ADR-0062 — registers one optional string common attr, times times. +internal sealed class CommonAttrTestProvider : IMetaDataTypeProvider +{ + private readonly string _attr; + private readonly int _times; + + public CommonAttrTestProvider(string id, string[] dependencies, string attr, int times = 1) + { + Id = id; + Dependencies = dependencies; + _attr = attr; + _times = times; + } + + public string Id { get; } + public IReadOnlyList Dependencies { get; } + + public void RegisterTypes(TypeRegistry registry) + { + for (int i = 0; i < _times; i++) + { + registry.RegisterCommonAttrs(new[] + { + new AttrSchema(_attr, "string", Required: false, Description: "Ownership probe common attr."), + }); + } + } +} + public sealed class ProviderCompositionConformanceTests { private static readonly IReadOnlyDictionary Providers = @@ -175,6 +222,10 @@ public sealed class ProviderCompositionConformanceTests ["shadow-base-late"] = new ShadowTestProvider("shadow-base-late", new[] { "shadow-sub-owner" }, ("base", "reverseAttr")), ["shadow-sub-distinct"] = new ShadowTestProvider("shadow-sub-distinct", new[] { "shadow-base" }, (ShadowTestProvider.ShadowSubType, "distinctAttr")), ["shadow-self"] = new ShadowTestProvider("shadow-self", System.Array.Empty(), ("base", "selfAttr"), (ShadowTestProvider.ShadowSubType, "selfAttr")), + ["shadow-sub-extend-clash"] = new ShadowSubExtendClashProvider(), + ["common-owner"] = new CommonAttrTestProvider("common-owner", System.Array.Empty(), "commonAttr"), + ["common-clash"] = new CommonAttrTestProvider("common-clash", new[] { "common-owner" }, "commonAttr"), + ["common-self"] = new CommonAttrTestProvider("common-self", System.Array.Empty(), "selfCommonAttr", times: 2), }; // Flat-corpus (error-code) manifest shape — unchanged. @@ -337,7 +388,10 @@ public void ProviderCompositionComposeLoad(string fileName) private sealed record OwnerExpectation(string Type, string SubType, string Attr, string Owner); - private sealed record OwnershipManifest(string[] Providers, OwnerExpectation[] ExpectOwners); + private sealed record CommonOwnerExpectation(string Attr, string Owner); + + private sealed record OwnershipManifest( + string[] Providers, OwnerExpectation[]? ExpectOwners, CommonOwnerExpectation[]? ExpectCommonOwners); private static string OwnershipCorpusRootPath() => Path.Combine(CorpusRootPath(), "attr-ownership"); @@ -363,10 +417,16 @@ public void ProviderCompositionAttrOwnership(string fileName) File.ReadAllText(Path.Combine(OwnershipCorpusRootPath(), fileName)), opts)!; var registry = Provider.ComposeRegistry(manifest.Providers.Select(Resolve).ToList()); - Assert.NotEmpty(manifest.ExpectOwners); - foreach (var expected in manifest.ExpectOwners) + OwnerExpectation[] owners = manifest.ExpectOwners ?? []; + CommonOwnerExpectation[] commonOwners = manifest.ExpectCommonOwners ?? []; + Assert.True(owners.Length + commonOwners.Length > 0); + foreach (var expected in owners) { Assert.Equal(expected.Owner, registry.AttrOrigin(expected.Type, expected.SubType, expected.Attr)); } + foreach (var expected in commonOwners) + { + Assert.Equal(expected.Owner, registry.CommonAttrOrigin(expected.Attr)); + } } } diff --git a/server/csharp/MetaObjects/Registry.cs b/server/csharp/MetaObjects/Registry.cs index 8b8a031a2c..66efa3a0e7 100644 --- a/server/csharp/MetaObjects/Registry.cs +++ b/server/csharp/MetaObjects/Registry.cs @@ -419,18 +419,39 @@ public void SetDefaultSubType(string type, string subType) private readonly List _commonAttrs = new(); + /// ADR-0062 — common attr name -> the provider id that registered it + /// ( when registered outside any provider's turn). + private readonly Dictionary _commonAttrProvenance = new(); + /// /// Register attrs that are accepted on every metatype. Used by providers to - /// declare cross-cutting attrs (e.g. documentation attrs). Repeated - /// registration of the same name is silently deduped (first registration - /// wins); conflicts with per-type attrs are detected at validation time. + /// declare cross-cutting attrs (e.g. documentation attrs). The same provider (or + /// a registration outside any provider's turn) re-registering a name is deduped; a + /// different provider registering a name another provider holds throws + /// ERR_PROVIDER_ATTR_CONFLICT (ADR-0062). Conflicts with per-type attrs are + /// detected at validation time. /// Throws if any attr has /// ValueType == SUBTYPE_BASE. /// public void RegisterCommonAttrs(IEnumerable attrs) { CheckNotSealed("RegisterCommonAttrs"); - foreach (AttrSchema attr in attrs) + List batch = attrs.ToList(); + // ADR-0062: checked for the whole batch first, so a refusal leaves no trace. + string? claimant = CurrentProviderId; + foreach (AttrSchema attr in batch) + { + string owner = CommonAttrOrigin(attr.Name); + if (claimant is not null && owner != LibrarySentinel && owner != claimant) + { + throw new MetaModelException( + $"Provider \"{claimant}\" registers common attribute \"{attr.Name}\", but provider \"{owner}\" " + + $"already registers it as a common attribute; provider \"{claimant}\" cannot declare it again (ADR-0062)", + ErrorCode.ERR_PROVIDER_ATTR_CONFLICT); + } + } + + foreach (AttrSchema attr in batch) { if (attr.ValueType == SUBTYPE_BASE) { @@ -458,13 +479,20 @@ public void RegisterCommonAttrs(IEnumerable attrs) if (_commonAttrs.Any(existing => existing.Name == attr.Name)) { - continue; // first-wins dedupe; per-type conflicts handled at validation time + continue; // same-name dedupe; per-type conflicts handled at validation time } _commonAttrs.Add(attr); + _commonAttrProvenance[attr.Name] = claimant ?? LibrarySentinel; } } + /// ADR-0062 — the provider id that registered common attr + /// , or if unregistered or + /// registered outside any provider's turn. + public string CommonAttrOrigin(string attrName) => + _commonAttrProvenance.TryGetValue(attrName, out string? origin) ? origin : LibrarySentinel; + /// Returns the registered common attrs (defensive copy). public IReadOnlyList GetCommonAttrs() => _commonAttrs.ToList(); diff --git a/server/java/metadata/src/main/java/com/metaobjects/registry/MetaDataRegistry.java b/server/java/metadata/src/main/java/com/metaobjects/registry/MetaDataRegistry.java index 381e459168..5a6f824ad3 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/registry/MetaDataRegistry.java +++ b/server/java/metadata/src/main/java/com/metaobjects/registry/MetaDataRegistry.java @@ -85,6 +85,9 @@ public class MetaDataRegistry { */ private final Map commonAttributes = new ConcurrentHashMap<>(); + /** ADR-0062 — common attr name -> the provider that registered it (or {@link #LIBRARY_PROVENANCE}). */ + private final Map commonAttrProvenance = new ConcurrentHashMap<>(); + /** * #265 — attr-name provenance stamped during provider composition: * {@code (type, subType) -> (attrName -> providerId)}. Consulted by @@ -375,30 +378,19 @@ public MetaDataRegistry extendType(Class metaDataClass, Cons checkNoProjectedRequiredAttr(typeIdToExtend, existing, extendedDefinition); // ADR-0062: same fail-closed position — a name another provider already holds on a // related slot never reaches typeDefinitions. - Set existingNames = new HashSet<>(); - for (ChildRequirement req : existing.getChildRequirements()) { - if (isNamedAttrRequirement(req)) { - existingNames.add(req.getName()); - } - } - List claimed = new ArrayList<>(); - for (ChildRequirement req : extendedDefinition.getDirectChildRequirements()) { - if (isNamedAttrRequirement(req) && !existingNames.contains(req.getName())) { - claimed.add(req.getName()); - } - } + List claimed = newAttrClaims(typeIdToExtend, builder); checkAttrOwnership(typeIdToExtend, extendedDefinition.getParentType(), extendedDefinition.getParentSubType(), claimed); typeDefinitions.put(typeIdToExtend, extendedDefinition); // #265 — extendType() rebuilds from `existing` and writes straight into // typeDefinitions WITHOUT going through register(), so it must stamp - // provenance itself. Unlike register()'s blanket stamp, this DIFFS: only - // attr names newly present vs. `existing` get stamped with the current - // provider. Blanket-stamping here would mis-attribute every attr `existing` - // already carried (e.g. re-registering core's own attrs as this provider's) - // — and a same-name redefinition (the attr-conflict probe) must keep its - // PRIOR origin, not be silently reassigned to whoever redefined it. - stampNewAttrProvenance(typeIdToExtend, existing, extendedDefinition); + // provenance itself. Unlike register()'s blanket stamp, only the names this + // extension claims get stamped with the current provider. Blanket-stamping here + // would mis-attribute every attr `existing` already carried (e.g. re-registering + // core's own attrs as this provider's) — and a same-name redefinition (the + // attr-conflict probe) must keep its PRIOR origin, not be silently reassigned to + // whoever redefined it. + stampAttrProvenance(typeIdToExtend, claimed); log.debug("Extended type: {} with additional attributes/children", typeIdToExtend.toQualifiedName()); @@ -525,6 +517,10 @@ void applyExtendsAttrs(String type, String subType, atb.asSingle(); } } + // ADR-0062: register() only checks names new to the slot's full requirement set, so + // a name this slot inherits and the directive re-declares is checked here. + checkAttrOwnership(typeId, existing.getParentType(), existing.getParentSubType(), + newAttrClaims(typeId, builder)); register(builder.build()); } @@ -593,7 +589,7 @@ public void register(TypeDefinition definition) { // provider's turn). Blanket, not diffed: register() always REPLACES the // (type, subType)'s complete definition (unlike extendType(), which // rebuilds an already-registered type from `existing` — see - // stampNewAttrProvenance for why that path diffs instead). + // stampAttrProvenance for why that path stamps only claimed names). recordAttrProvenance(typeId, definition.getDirectChildRequirements()); log.debug("Registered type: {} -> {} (parent: {})", typeId.toQualifiedName(), definition.getImplementationClass().getSimpleName(), @@ -860,7 +856,9 @@ public void addGlobalChildRequirement(String parentType, String parentSubType, C * {@code CanonicalJsonParser.processAttributes} fires for any parent. * * - *

Idempotent: re-registering the same attr name is a no-op.

+ *

Idempotent: re-registering the same attr name is a no-op for the provider that + * registered it; another provider registering the same common attr name is refused + * (ADR-0062, {@code ERR_PROVIDER_ATTR_CONFLICT}).

* * @param name bare attribute name (no {@code @} prefix; e.g. {@code "description"}) * @param valueType attribute value subtype (e.g. {@link StringAttribute#SUBTYPE_STRING}, @@ -878,6 +876,7 @@ public void registerCommonAttribute(String name, String valueType, boolean isArr // Idempotent: skip re-registration of the same attr name (allows tests that // re-initialise the registry to not blow up on duplicate constraints). This // precedes the seal guard so a no-op re-registration never trips the seal. + checkCommonAttrOwnership(name); if (commonAttributes.containsKey(name)) { return; } @@ -885,6 +884,7 @@ public void registerCommonAttribute(String name, String valueType, boolean isArr CommonAttributeDef def = new CommonAttributeDef(name, valueType, isArray); commonAttributes.put(name, def); + commonAttrProvenance.put(name, currentProviderId != null ? currentProviderId : LIBRARY_PROVENANCE); // 1. Global ChildRequirement under the UNIVERSAL_PARENT_KEY tier — makes // acceptsChild() return true for this attr on any (parentType, parentSubType) pair. @@ -932,6 +932,15 @@ public CommonAttributeDef getCommonAttribute(String name) { return commonAttributes.get(name); } + /** + * ADR-0062 — the provider that registered common attribute {@code name}, or + * {@code null} when it is not registered or was registered outside any provider's turn. + */ + public String getCommonAttrProvider(String name) { + String providerId = commonAttrProvenance.get(name); + return LIBRARY_PROVENANCE.equals(providerId) ? null : providerId; + } + /** * Whether any common attribute is registered. * @@ -1181,7 +1190,7 @@ private static boolean isPrunableAttr(ChildRequirement req) { /** * #265 — true for a NAMED {@code attr}-typed requirement (concrete, non-wildcard - * name): the shape {@link #recordAttrProvenance}/{@link #stampNewAttrProvenance} + * name): the shape {@link #recordAttrProvenance}/{@link #stampAttrProvenance} * track provenance for, and the shape {@link #isPrunableAttr} narrows further by * spec-inclusion. False for structural placement rules and the any-attr wildcard. */ @@ -1298,6 +1307,26 @@ private void checkAttrOwnership(MetaDataTypeId typeId, String parentType, String } } + /** + * ADR-0062 — provider P may not register common attr {@code name} when a DIFFERENT + * provider already registered it as a common attr. Re-registration by the same + * provider, and registration outside any provider's turn, stay legal. + */ + private void checkCommonAttrOwnership(String name) { + String claimant = currentProviderId; + if (claimant == null) { + return; + } + String commonOwner = getCommonAttrProvider(name); + if (commonOwner != null && !commonOwner.equals(claimant)) { + throw new MetaDataException( + "Provider \"" + claimant + "\" registers common attribute \"" + name + "\", but provider \"" + + commonOwner + "\" already registers it as a common attribute; provider \"" + claimant + + "\" cannot declare it again (ADR-0062).", + com.metaobjects.ErrorCode.ERR_PROVIDER_ATTR_CONFLICT); + } + } + /** * ADR-0062 — text appended to a same-slot {@code ERR_PROVIDER_ATTR_CONFLICT} so it names * the provider that holds the slot. Empty when the holder is unknown or is the provider @@ -1318,8 +1347,8 @@ String describeSlotHolder(String type, String subType, String attrName) { * onto {@code typeId}, stamping {@link #currentProviderId} (or {@link #LIBRARY_PROVENANCE} * outside a provider's turn). Called from {@link #register}, which always replaces the * (type, subType)'s COMPLETE definition — so a blanket stamp is correct here (contrast - * {@link #stampNewAttrProvenance}, which diffs because {@link #extendType} rebuilds from - * an already-registered definition). + * {@link #stampAttrProvenance}, which stamps only claimed names because + * {@link #extendType} rebuilds from an already-registered definition). */ private void recordAttrProvenance(MetaDataTypeId typeId, Collection directRequirements) { String providerId = currentProviderId != null ? currentProviderId : LIBRARY_PROVENANCE; @@ -1333,39 +1362,45 @@ private void recordAttrProvenance(MetaDataTypeId typeId, Collection existingNames = new HashSet<>(); - for (ChildRequirement req : existing.getChildRequirements()) { - if (isNamedAttrRequirement(req)) { - existingNames.add(req.getName()); - } - } + private void stampAttrProvenance(MetaDataTypeId typeId, Collection names) { String providerId = currentProviderId != null ? currentProviderId : LIBRARY_PROVENANCE; - for (ChildRequirement req : extended.getDirectChildRequirements()) { - if (!isNamedAttrRequirement(req) || existingNames.contains(req.getName())) { - continue; + for (String name : names) { + attrProvenance.computeIfAbsent(typeId, k -> new ConcurrentHashMap<>()).put(name, providerId); + } + } + + /** + * ADR-0062 — the attr names a builder seeded from {@code typeId}'s definition declares + * as a new claim on that slot: every declared name the slot does not already declare + * itself. That includes a name the slot only inherits, which is a claim on a related + * slot's name. A name the slot already declares is the builder's same-slot check. + */ + private List newAttrClaims(MetaDataTypeId typeId, TypeDefinitionBuilder builder) { + List claimed = new ArrayList<>(); + for (String name : builder.getDeclaredAttrNames()) { + if (!declaresAttrOn(typeId.type(), typeId.subType(), name)) { + claimed.add(name); } - attrProvenance.computeIfAbsent(typeId, k -> new ConcurrentHashMap<>()) - .put(req.getName(), providerId); } + return claimed; + } + + /** + * ADR-0062 — whether {@code (type, subType)} itself declares {@code attrName} (it was + * registered or extended onto exactly that slot), as opposed to inheriting it. + */ + boolean declaresAttrOn(String type, String subType, String attrName) { + Map byName = attrProvenance.get(new MetaDataTypeId(type, subType)); + return byName != null && byName.containsKey(attrName); } /** @@ -1381,10 +1416,9 @@ private void stampNewAttrProvenance(MetaDataTypeId typeId, TypeDefinition existi *

If the attr is genuinely required, it is OWN, not projected: declare it with the * type, in the type's own provider.

* - *

Diffs {@code extended}'s named attr requirements against {@code existing}'s (the - * same base set {@link #stampNewAttrProvenance} diffs against, for the same reason — - * {@code existing}'s FULL direct+inherited requirements, since {@code extended}'s - * direct-only set would otherwise see every previously-inherited attr as "new") and + *

Diffs {@code extended}'s named attr requirements against {@code existing}'s + * FULL direct+inherited requirements, since {@code extended}'s + * direct-only set would otherwise see every previously-inherited attr as "new", and * throws on the first newly-added attr that is required. Callers MUST invoke this * BEFORE committing {@code extended} into {@code typeDefinitions} so a violation * fails closed — the projected attr is never applied. diff --git a/server/java/metadata/src/main/java/com/metaobjects/registry/TypeDefinitionBuilder.java b/server/java/metadata/src/main/java/com/metaobjects/registry/TypeDefinitionBuilder.java index 61c0bab6a2..818b478ffd 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/registry/TypeDefinitionBuilder.java +++ b/server/java/metadata/src/main/java/com/metaobjects/registry/TypeDefinitionBuilder.java @@ -56,6 +56,10 @@ public class TypeDefinitionBuilder { private String parentType; private String parentSubType; private final Map childRequirements = new HashMap<>(); + // ADR-0062: attr names copied in by from() (the slot's own AND inherited names), and + // the attr names this builder's caller declared afterwards. + private final java.util.Set seededAttrNames = new java.util.HashSet<>(); + private final java.util.Set declaredAttrNames = new java.util.LinkedHashSet<>(); // Auto-constraint generation support private MetaDataRegistry registry; @@ -124,6 +128,8 @@ public static TypeDefinitionBuilder from(TypeDefinition existing) { if ("*".equals(keyToUse)) { // For wildcard requirements, create unique keys to avoid overwrites keyToUse = "*:" + req.getExpectedType() + ":" + req.getExpectedSubType(); + } else if ("attr".equals(req.getExpectedType())) { + builder.seededAttrNames.add(keyToUse); } builder.childRequirements.put(keyToUse, req); } @@ -440,14 +446,23 @@ private void addChildRequirement(ChildRequirement requirement) { key = "*:" + requirement.getExpectedType() + ":" + requirement.getExpectedSubType(); } + boolean namedAttr = "attr".equals(requirement.getExpectedType()) && !"*".equals(requirement.getName()); + if (namedAttr) { + declaredAttrNames.add(key); + } ChildRequirement existing = childRequirements.get(key); + // ADR-0062: a name this slot only INHERITS is not a same-slot redefinition. The + // registry's ownership check (extendType / applyExtendsAttrs) decides whether + // declaring it here is legal, so the declaration replaces the inherited copy. + boolean inheritedOrigin = existing != null && namedAttr && registry != null + && seededAttrNames.contains(key) && !registry.declaresAttrOn(type, subType, key); // ADR-0062: an IDENTICAL re-declaration is idempotent for the provider that owns the // slot's attr, but another provider repeating it is still a second claim on the name. boolean foreignRepeat = existing != null && existing.equals(requirement) - && "attr".equals(requirement.getExpectedType()) + && namedAttr && registry != null && !registry.describeSlotHolder(type, subType, requirement.getName()).isEmpty(); - if (existing != null && (foreignRepeat || !existing.equals(requirement))) { + if (existing != null && !inheritedOrigin && (foreignRepeat || !existing.equals(requirement))) { // A colliding ATTR child requirement is a provider redefining an attr // another provider already declared on this type — the cross-port // ERR_PROVIDER_ATTR_CONFLICT invariant (mirrors the TS/Python/C# @@ -502,6 +517,14 @@ public TypeDefinition build() { return td; } + /** + * ADR-0062 — the named attr names declared on this builder after it was created, in + * declaration order (excludes the requirements {@link #from} copied in). + */ + java.util.Set getDeclaredAttrNames() { + return java.util.Collections.unmodifiableSet(declaredAttrNames); + } + /** * Get the current type being built (for debugging) * diff --git a/server/java/metadata/src/test/java/com/metaobjects/conformance/ProviderCompositionConformanceTest.java b/server/java/metadata/src/test/java/com/metaobjects/conformance/ProviderCompositionConformanceTest.java index b63658e850..9a2e74cb71 100644 --- a/server/java/metadata/src/test/java/com/metaobjects/conformance/ProviderCompositionConformanceTest.java +++ b/server/java/metadata/src/test/java/com/metaobjects/conformance/ProviderCompositionConformanceTest.java @@ -15,6 +15,7 @@ */ package com.metaobjects.conformance; +import com.google.gson.JsonArray; import com.google.gson.JsonObject; import com.google.gson.JsonParser; import com.metaobjects.ErrorCode; @@ -231,6 +232,23 @@ private static MetaDataTypeProvider shadowProvider(String id, String[] deps, Con registerShadowProbe(r, "selfAttr"); }); + private static final MetaDataTypeProvider SHADOW_SUB_EXTEND_CLASH = + shadowProvider("shadow-sub-extend-clash", new String[]{"shadow-sub-distinct"}, + r -> r.extendType(ShadowProbeTemplate.class, def -> def + .optionalAttributeWithConstraints("shadowAttr") + .ofType(StringAttribute.SUBTYPE_STRING).asSingle())); + private static final MetaDataTypeProvider COMMON_OWNER = + shadowProvider("common-owner", new String[0], + r -> r.registerCommonAttribute("commonAttr", StringAttribute.SUBTYPE_STRING, false)); + private static final MetaDataTypeProvider COMMON_CLASH = + shadowProvider("common-clash", new String[]{"common-owner"}, + r -> r.registerCommonAttribute("commonAttr", StringAttribute.SUBTYPE_STRING, false)); + private static final MetaDataTypeProvider COMMON_SELF = + shadowProvider("common-self", new String[0], r -> { + r.registerCommonAttribute("selfCommonAttr", StringAttribute.SUBTYPE_STRING, false); + r.registerCommonAttribute("selfCommonAttr", StringAttribute.SUBTYPE_STRING, false); + }); + private static MetaDataTypeProvider resolve(String id) { MetaDataTypeProvider p = ConformanceTestProviders.TEST_PROVIDERS.get(id); if (p != null) return p; @@ -245,6 +263,10 @@ private static MetaDataTypeProvider resolve(String id) { case "shadow-base-late": return SHADOW_BASE_LATE; case "shadow-sub-distinct": return SHADOW_SUB_DISTINCT; case "shadow-self": return SHADOW_SELF; + case "shadow-sub-extend-clash": return SHADOW_SUB_EXTEND_CLASH; + case "common-owner": return COMMON_OWNER; + case "common-clash": return COMMON_CLASH; + case "common-self": return COMMON_SELF; default: throw new IllegalArgumentException( "Unknown named provider \"" + id + "\" in provider-composition corpus"); @@ -382,8 +404,11 @@ public void providerCompositionAttrOwnership() throws IOException { MetaDataRegistry registry = MetaDataRegistry.compose(resolved); - assertTrue(manifest.getAsJsonArray("expectOwners").size() > 0); - manifest.getAsJsonArray("expectOwners").forEach(el -> { + JsonArray owners = manifest.has("expectOwners") ? manifest.getAsJsonArray("expectOwners") : new JsonArray(); + JsonArray commonOwners = manifest.has("expectCommonOwners") + ? manifest.getAsJsonArray("expectCommonOwners") : new JsonArray(); + assertTrue(owners.size() + commonOwners.size() > 0); + owners.forEach(el -> { JsonObject expected = el.getAsJsonObject(); assertEquals( expected.get("owner").getAsString(), @@ -392,6 +417,12 @@ public void providerCompositionAttrOwnership() throws IOException { expected.get("subType").getAsString(), expected.get("attr").getAsString())); }); + commonOwners.forEach(el -> { + JsonObject expected = el.getAsJsonObject(); + assertEquals( + expected.get("owner").getAsString(), + registry.getCommonAttrProvider(expected.get("attr").getAsString())); + }); } } diff --git a/server/java/metadata/src/test/java/com/metaobjects/registry/AttrOwnershipTest.java b/server/java/metadata/src/test/java/com/metaobjects/registry/AttrOwnershipTest.java index 16a8e9a33d..8823137851 100644 --- a/server/java/metadata/src/test/java/com/metaobjects/registry/AttrOwnershipTest.java +++ b/server/java/metadata/src/test/java/com/metaobjects/registry/AttrOwnershipTest.java @@ -102,6 +102,33 @@ public void extendOntoTheSharedRootCollidesWithASubtypeName() { assertEquals(ErrorCode.ERR_PROVIDER_ATTR_SHADOW, e.getCode().orElseThrow()); } + @Test + public void extendingASubtypeWithANameItOnlyInheritsIsAShadowEvenWhenTheSpecDiffers() { + MetaDataException e = refusal( + provider("owner-a", new String[0], r -> { + root(r, "physicalName"); + probe(r, "other"); + }), + provider("owner-b", new String[]{"owner-a"}, r -> + r.extendType(ProbeTemplate.class, def -> + def.optionalAttributeWithConstraints("physicalName") + .ofType(StringAttribute.SUBTYPE_STRING).asArray()))); + assertEquals(ErrorCode.ERR_PROVIDER_ATTR_SHADOW, e.getCode().orElseThrow()); + } + + @Test + public void ownerExtendingItsSubtypeWithAnInheritedNameIsLegalAndRecorded() { + MetaDataRegistry registry = MetaDataRegistry.compose(List.of( + provider("owner-a", new String[0], r -> { + root(r, "physicalName"); + probe(r, "other"); + r.extendType(ProbeTemplate.class, def -> + def.optionalAttributeWithConstraints("physicalName") + .ofType(StringAttribute.SUBTYPE_STRING).asSingle()); + }))); + assertEquals("owner-a", registry.getAttrProvider(TemplateConstants.TYPE_TEMPLATE, "probe", "physicalName")); + } + @Test public void sameSlotExtendByAnotherProviderKeepsAttrConflictAndNamesTheOwner() { MetaDataException e = refusal( diff --git a/server/python/src/metaobjects/registry.py b/server/python/src/metaobjects/registry.py index b3da77247d..8003159556 100644 --- a/server/python/src/metaobjects/registry.py +++ b/server/python/src/metaobjects/registry.py @@ -125,6 +125,9 @@ def __init__(self) -> None: # on a spec-declared subtype is never pruned by the library's strict # per-subtype allow-list — only LIBRARY-origin attrs are. self._attr_provenance: dict[tuple[str, str, str], str] = {} + # ADR-0062 -- common attr name -> the provider id that registered it + # (LIBRARY_ATTR_ORIGIN when registered outside any provider's turn). + self._common_attr_provenance: dict[str, str] = {} def seal(self) -> None: """Seal the registry: every subsequent mutating registration raises @@ -261,11 +264,25 @@ def default_sub_type_of(self, type_: str) -> str | None: return self._default_sub_types.get(type_) def register_common_attrs(self, attrs: list[AttrSchema]) -> None: - """Register attrs accepted on every metatype. First-wins dedupe by name. + """Register attrs accepted on every metatype. - Conflict with per-type attrs is detected at validation time, not here. + The same provider (or a registration outside any provider's turn) re-registering a + name is deduped; a different provider registering a name another provider holds + raises ``ERR_PROVIDER_ATTR_CONFLICT`` (ADR-0062). Conflict with per-type attrs is + detected at validation time, not here. """ self._check_not_sealed("register_common_attrs") + # ADR-0062: checked for the whole batch first, so a refusal leaves no trace. + claimant = self._current_provider_id + for attr in attrs: + owner = self._common_attr_provenance.get(attr.name, LIBRARY_ATTR_ORIGIN) + if claimant is not None and owner not in (LIBRARY_ATTR_ORIGIN, claimant): + raise ParseError( + f'Provider "{claimant}" registers common attribute "{attr.name}", but ' + f'provider "{owner}" already registers it as a common attribute; provider ' + f'"{claimant}" cannot declare it again (ADR-0062)', + ErrorCode.ERR_PROVIDER_ATTR_CONFLICT, + ) for attr in attrs: # ADR-0050, the second door. A common attr is projected onto EVERY registered # type, so a required one is the same defect as a required extend() -- with the @@ -282,8 +299,15 @@ def register_common_attrs(self, attrs: list[AttrSchema]) -> None: ErrorCode.ERR_EXTEND_REQUIRED_ATTR, ) if any(existing.name == attr.name for existing in self._common_attrs): - continue # first registration wins + continue # dedupe same-name re-registration self._common_attrs.append(attr) + self._common_attr_provenance[attr.name] = claimant or LIBRARY_ATTR_ORIGIN + + def common_attr_origin(self, attr_name: str) -> str: + """The provider id that registered common attr ``attr_name``, or + :data:`LIBRARY_ATTR_ORIGIN` if unregistered or registered outside any provider's + turn (ADR-0062).""" + return self._common_attr_provenance.get(attr_name, LIBRARY_ATTR_ORIGIN) def get_common_attrs(self) -> list[AttrSchema]: """Return a defensive copy of the registered common attrs.""" diff --git a/server/python/tests/conformance/test_provider_composition_conformance.py b/server/python/tests/conformance/test_provider_composition_conformance.py index 67d8e938ab..c95bcc2296 100644 --- a/server/python/tests/conformance/test_provider_composition_conformance.py +++ b/server/python/tests/conformance/test_provider_composition_conformance.py @@ -141,8 +141,35 @@ def _shadow_provider(provider_id: str, deps: tuple[str, ...], *types: tuple[str, return p +class _ShadowSubExtendClashProvider(Provider): + """Extends `template.shadowprobe` with the name `shadow-base` owns on the root.""" + + def __init__(self) -> None: + super().__init__("shadow-sub-extend-clash", dependencies=("shadow-sub-distinct",)) + + def register_types(self, registry: TypeRegistry) -> None: # noqa: D401 + registry.extend("template", _SHADOW_SUBTYPE, attributes=[AttrSchema("shadowAttr", "string")]) + + +class _CommonAttrProvider(Provider): + """Registers one optional string common attr, `times` times.""" + + def __init__(self, provider_id: str, deps: tuple[str, ...], attr_name: str, times: int = 1) -> None: + super().__init__(provider_id, dependencies=deps) + self._attr_name = attr_name + self._times = times + + def register_types(self, registry: TypeRegistry) -> None: # noqa: D401 + for _ in range(self._times): + registry.register_common_attrs([AttrSchema(self._attr_name, "string")]) + + def _providers() -> dict[str, Provider]: return { + "shadow-sub-extend-clash": _ShadowSubExtendClashProvider(), + "common-owner": _CommonAttrProvider("common-owner", (), "commonAttr"), + "common-clash": _CommonAttrProvider("common-clash", ("common-owner",), "commonAttr"), + "common-self": _CommonAttrProvider("common-self", (), "selfCommonAttr", times=2), "shadow-base": _shadow_provider("shadow-base", (), ("base", "shadowAttr")), "shadow-sub-clash": _shadow_provider( "shadow-sub-clash", ("shadow-base",), (_SHADOW_SUBTYPE, "shadowAttr") @@ -282,6 +309,8 @@ def test_provider_composition_attr_ownership(manifest_path: Path) -> None: manifest = json.loads(manifest_path.read_text(encoding="utf-8")) providers = _providers() registry = compose_registry([_resolve(providers, pid) for pid in manifest["providers"]]) - assert manifest["expectOwners"] - for owner in manifest["expectOwners"]: + assert manifest.get("expectOwners") or manifest.get("expectCommonOwners") + for owner in manifest.get("expectOwners", []): assert registry.attr_origin(owner["type"], owner["subType"], owner["attr"]) == owner["owner"] + for owner in manifest.get("expectCommonOwners", []): + assert registry.common_attr_origin(owner["attr"]) == owner["owner"] diff --git a/server/typescript/packages/metadata/src/registry.ts b/server/typescript/packages/metadata/src/registry.ts index 426da223fd..d44a784a27 100644 --- a/server/typescript/packages/metadata/src/registry.ts +++ b/server/typescript/packages/metadata/src/registry.ts @@ -337,12 +337,27 @@ export class TypeRegistry { /** * Register attrs that are accepted on every metatype. Callers (providers) use - * this to declare cross-cutting attrs (e.g. doc attrs). Repeated registration - * of the same name is silently deduped (first registration wins); conflicts with - * per-type attrs are resolved at validation time (Task 1.3). + * this to declare cross-cutting attrs (e.g. doc attrs). The same provider (or a + * registration outside any provider's turn) re-registering a name is deduped; a + * different provider registering a name another provider holds is + * `ERR_PROVIDER_ATTR_CONFLICT` (ADR-0062). Conflicts with per-type attrs are + * resolved at validation time (Task 1.3). */ registerCommonAttrs(attrs: AttrSchema[]): void { this.checkNotSealed("registerCommonAttrs"); + // ADR-0062: another provider may not register a common attr name a provider already + // holds. Checked for the whole batch first, so a refusal leaves no trace. + const claimant = this._currentProviderId; + for (const attr of attrs) { + const owner = this._commonAttrOwners.get(attr.name); + if (owner !== undefined && claimant !== undefined && owner !== claimant) { + throw new MetaModelError( + `Provider "${claimant}" registers common attribute "${attr.name}", but provider "${owner}" ` + + `already registers it as a common attribute; provider "${claimant}" cannot declare it again (ADR-0062)`, + { code: "ERR_PROVIDER_ATTR_CONFLICT" }, + ); + } + } for (const attr of attrs) { if (attr.valueType === SUBTYPE_BASE) { throw new Error( @@ -368,7 +383,7 @@ export class TypeRegistry { continue; // dedupe same-name re-registration; conflict-with-per-type-attr is checked at validation time } this._commonAttrs.push(attr); - if (this._currentProviderId !== undefined) this._commonAttrOwners.set(attr.name, this._currentProviderId); + if (claimant !== undefined) this._commonAttrOwners.set(attr.name, claimant); } } diff --git a/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts b/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts index d94065564d..b2c8835cc9 100644 --- a/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts +++ b/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts @@ -17,7 +17,7 @@ import { join } from "node:path"; import { readdirSync } from "node:fs"; import type { MetaDataTypeProvider } from "../src/provider.js"; import { composeRegistry } from "../src/provider.js"; -import { TypeRegistry, TypeId } from "../src/registry.js"; +import { TypeRegistry, TypeId, type AttrSchema } from "../src/registry.js"; import { MetaTemplate } from "../src/template/meta-template.js"; import { TYPE_TEMPLATE, TYPE_ATTR, TYPE_VIEW, SUBTYPE_BASE } from "../src/shared/base-types.js"; import { CHILD_RULE_WILDCARD } from "../src/shared/structural.js"; @@ -194,7 +194,43 @@ const shadowSelfProvider: MetaDataTypeProvider = { }, }; +const shadowSubExtendClashProvider: MetaDataTypeProvider = { + id: "shadow-sub-extend-clash", + dependencies: ["shadow-sub-distinct"], + registerTypes: (registry) => + registry.extend(TYPE_TEMPLATE, SHADOW_SUBTYPE, { + attributes: [ + { name: "shadowAttr", valueType: ATTR_SUBTYPE_STRING, required: false, description: "Ownership probe attr." }, + ], + }), +}; + +function commonAttr(name: string): AttrSchema { + return { name, valueType: ATTR_SUBTYPE_STRING, required: false, description: "Ownership probe common attr." }; +} + +const commonOwnerProvider: MetaDataTypeProvider = { + id: "common-owner", + registerTypes: (registry) => registry.registerCommonAttrs([commonAttr("commonAttr")]), +}; +const commonClashProvider: MetaDataTypeProvider = { + id: "common-clash", + dependencies: ["common-owner"], + registerTypes: (registry) => registry.registerCommonAttrs([commonAttr("commonAttr")]), +}; +const commonSelfProvider: MetaDataTypeProvider = { + id: "common-self", + registerTypes(registry) { + registry.registerCommonAttrs([commonAttr("selfCommonAttr")]); + registry.registerCommonAttrs([commonAttr("selfCommonAttr")]); + }, +}; + const PROVIDERS: Readonly> = { + "shadow-sub-extend-clash": shadowSubExtendClashProvider, + "common-owner": commonOwnerProvider, + "common-clash": commonClashProvider, + "common-self": commonSelfProvider, "shadow-base": shadowBaseProvider, "shadow-sub-clash": shadowSubClashProvider, "shadow-sub-owner": shadowSubOwnerProvider, @@ -231,6 +267,7 @@ interface Manifest { expectErrors?: string[]; // ADR-0062 attr-ownership shape — see README.md "The `attr-ownership/` subdir". expectOwners?: { type: string; subType: string; attr: string; owner: string }[]; + expectCommonOwners?: { attr: string; owner: string }[]; } /** Pull the stable ERR_ code off a caught error, if it carries one. */ @@ -354,9 +391,12 @@ for (const file of ownershipFiles) { test(`provider-composition (attr-ownership): ${file}`, async () => { const manifest: Manifest = await Bun.file(join(OWNERSHIP_CORPUS, file)).json(); const registry = composeRegistry(manifest.providers.map(resolve)); - expect(manifest.expectOwners?.length ?? 0).toBeGreaterThan(0); + expect((manifest.expectOwners?.length ?? 0) + (manifest.expectCommonOwners?.length ?? 0)).toBeGreaterThan(0); for (const { type, subType, attr, owner } of manifest.expectOwners ?? []) { expect(registry.attrOwner(type, subType, attr)).toBe(owner); } + for (const { attr, owner } of manifest.expectCommonOwners ?? []) { + expect(registry.commonAttrOwner(attr)).toBe(owner); + } }); } diff --git a/spec/decisions/ADR-0062-provider-attribute-ownership.md b/spec/decisions/ADR-0062-provider-attribute-ownership.md index 4eb646e631..f662ef9c89 100644 --- a/spec/decisions/ADR-0062-provider-attribute-ownership.md +++ b/spec/decisions/ADR-0062-provider-attribute-ownership.md @@ -34,8 +34,14 @@ a collision. - when `S` is the shared root, any registered subtype `(T, S')` — `ERR_PROVIDER_ATTR_SHADOW`. The check runs in both directions, so the verdict does not depend on which provider registers - first. A refusal names both providers and the attribute, and leaves nothing behind in the - registry. + first, and whether the second provider registers the slot or `extend`s it. A name a subtype + only inherits from the shared root is the root's, so declaring it on the subtype is a shadow, + not a same-slot repeat. A refusal names both providers and the attribute, and leaves nothing + behind in the registry. + + A common attribute (accepted on every node type) follows the same rule against other common + attributes: a provider that registers common attribute `A` when a different provider already + registered common attribute `A` is refused with `ERR_PROVIDER_ATTR_CONFLICT`. 3. **What stays legal.** The same provider declaring a name on several related slots (its own `@x` on the root and on a subtype) and re-registering its own slot. Sibling subtypes using @@ -65,12 +71,19 @@ a collision. turned on), so existing adopters see no change. - A third-party provider that reuses a core or sibling provider's attribute name now fails at composition with a message that names both providers. The fix is a different idiomatic word. +- A common attribute and a per-type attribute of the same name, from two providers, are not + refused yet. The Java port's core providers (`core-types`, `object-types`, `identity-types`, + `relationship-types` and `index-types`) declare a per-type `@description` beside the + documentation provider's common `@description`. Giving that name one owner removes the attribute + from those providers' own registrations, which is a change to their public attribute contract, + so it is left for a separate decision. Until then, that pair loads as it did before this ADR. - The two codes split by where the clash is, not by severity: `ERR_PROVIDER_ATTR_CONFLICT` keeps its established same-slot meaning and fixture, and `ERR_PROVIDER_ATTR_SHADOW` is the new inherited/descendant case. - Conformance: `fixtures/provider-composition-conformance/` carries the refusals - (`attr-shadow-inherited`, `attr-shadow-descendant`) and an `attr-ownership/` sub-corpus where - composition succeeds and each port reports the same owner per attribute. + (`attr-shadow-inherited`, `attr-shadow-descendant`, `attr-shadow-extend`, + `common-attr-conflict`) and an `attr-ownership/` sub-corpus where composition succeeds and each + port reports the same owner per attribute and per common attribute. ## Alternatives considered From fca4d7fa63f3a60e6cc7af9edde4272637f5a85d Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Wed, 4 Nov 2026 04:24:02 -0500 Subject: [PATCH 4/6] no-mistakes(review): Refuse common-vs-per-type attr collisions; Java @description single owner --- CHANGELOG.md | 17 +++++---- docs/CONFORMANCE.md | 2 +- docs/features/extending-with-providers.md | 7 ++-- fixtures/conformance/ERROR-CODES.json | 2 +- .../README.md | 16 +++++--- .../attr-ownership/common-attr-owners.json | 10 ++++- .../common-attr-per-type-after.json | 8 ++++ .../common-attr-per-type-before.json | 8 ++++ .../ProviderCompositionConformanceTests.cs | 29 +++++++++----- server/csharp/MetaObjects/Registry.cs | 34 ++++++++++++++--- .../main/java/com/metaobjects/MetaData.java | 4 +- .../metaobjects/identity/PrimaryIdentity.java | 2 - .../identity/ReferenceIdentity.java | 3 -- .../identity/SecondaryIdentity.java | 2 - .../com/metaobjects/index/LookupIndex.java | 5 --- .../com/metaobjects/object/MetaObject.java | 1 - .../registry/MetaDataRegistry.java | 32 +++++++++++++--- .../registry/RegistryManifest.java | 8 +--- .../relationship/MetaRelationship.java | 6 --- .../ProviderCompositionConformanceTest.java | 11 ++++++ .../RegistryManifestConformanceTest.java | 2 - server/python/src/metaobjects/registry.py | 31 ++++++++++++--- .../test_provider_composition_conformance.py | 16 +++++--- .../packages/metadata/src/registry.ts | 38 +++++++++++++++---- .../provider-composition-conformance.test.ts | 18 +++++++++ .../ADR-0062-provider-attribute-ownership.md | 28 +++++++------- 26 files changed, 240 insertions(+), 100 deletions(-) create mode 100644 fixtures/provider-composition-conformance/common-attr-per-type-after.json create mode 100644 fixtures/provider-composition-conformance/common-attr-per-type-before.json diff --git a/CHANGELOG.md b/CHANGELOG.md index f3b3e17a44..099a8d9db2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,15 +37,18 @@ here.** sibling subtypes sharing a name, and registration outside a provider all stay legal. The shipped core providers have no such collision, so existing projects see no change. `meta types --detail` prints the registering provider beside each attribute, and its JSON carries a `provider` field. - Two providers registering one common attribute name is also `ERR_PROVIDER_ATTR_CONFLICT` (it - used to be first-wins), and the registry records each common attribute's provider. A common - attribute and a per-type attribute of the same name from two providers are not refused yet: - Java's core providers declare a per-type `@description` beside the documentation provider's - common one. This is how per-paradigm physical-name words (`@column`, `@field`) stay safe as + A common attribute is accepted on every node type, so it counts as a claim on every slot: a + second provider registering a common attribute name another provider holds, as a common + attribute or on any type, and a provider declaring a per-type attribute whose name another + provider registered as common, are both `ERR_PROVIDER_ATTR_CONFLICT`, in either order (two + common registrations used to be first-wins). The registry records each common attribute's + provider. Java's core providers no longer declare a per-type `@description` beside the + documentation provider's common one (same type, same meaning); `@description` is still accepted + on every node. This is how per-paradigm physical-name words (`@column`, `@field`) stay safe as providers multiply; no wire-name attribute is added. See ADR-0062 and `docs/features/extending-with-providers.md`. New conformance fixtures: `attr-shadow-inherited`, - `attr-shadow-descendant`, `attr-shadow-extend`, `common-attr-conflict` and the - `attr-ownership/` sub-corpus. Fixes #527. + `attr-shadow-descendant`, `attr-shadow-extend`, `common-attr-conflict`, + `common-attr-per-type-after`, `common-attr-per-type-before` and the `attr-ownership/` sub-corpus. Fixes #527. ### Fixed diff --git a/docs/CONFORMANCE.md b/docs/CONFORMANCE.md index f2c2a53fa3..a4d34495c3 100644 --- a/docs/CONFORMANCE.md +++ b/docs/CONFORMANCE.md @@ -45,7 +45,7 @@ regenerate with `ls -d fixtures//*/ | wc -l` for directory-shaped corpor | [`fixtures/template-codegen-conformance/`](../fixtures/template-codegen-conformance/) | 3 | ✓ | ✓ | ✓ | ✓ | ✓ | | [`fixtures/template-output-render-conformance/`](../fixtures/template-output-render-conformance/) | 5 | ✓ | ✓ | ✓ | ✓ | ✓ | | [`fixtures/generator-registry-conformance/`](../fixtures/generator-registry-conformance/) | 1 canonical manifest | ✓ | ✓ | ✓ | ✓ | ✓ | -| [`fixtures/provider-composition-conformance/`](../fixtures/provider-composition-conformance/) | 16 (9 error-shape + 4 compose-load + 3 attr-ownership) | ✓ | ✓ | — (JVM registry via Java) | ✓ | ✓ | +| [`fixtures/provider-composition-conformance/`](../fixtures/provider-composition-conformance/) | 18 (11 error-shape + 4 compose-load + 3 attr-ownership) | ✓ | ✓ | — (JVM registry via Java) | ✓ | ✓ | | [`fixtures/source-resolution-conformance/`](../fixtures/source-resolution-conformance/) | 25 cases | ✓ (reference implementation) | ✓ | inherits via Java | ✓ | ✓ | | [`fixtures/scope-conformance/`](../fixtures/scope-conformance/) | 10 cases | ✓ (reference implementation) | ✓ | inherits via Java | ✓ | ✓ | | [`fixtures/dependency-conformance/`](../fixtures/dependency-conformance/) | 37 cases (26 resolution + 11 check) + 14 transport cases | ✓ (reference implementation; all three arms) | ✓ (resolution arm, all 26; no check, so the 11 check cases are not run) | inherits via Java | ✓ (resolution arm, all 26; no check, so the 11 check cases are not run) | ✓ (resolution arm, all 26 identically; no check yet, so the 11 check cases are not run; the transport arm is Node-only) | diff --git a/docs/features/extending-with-providers.md b/docs/features/extending-with-providers.md index 29fdaf05f0..d2e1383c56 100644 --- a/docs/features/extending-with-providers.md +++ b/docs/features/extending-with-providers.md @@ -88,8 +88,9 @@ which provider registered each attribute, and refuses a second provider's claim - on the same `(type, subType)` — `ERR_PROVIDER_ATTR_CONFLICT`; - on the shared root `(type, base)` that the subtype inherits from, or on a subtype of it — `ERR_PROVIDER_ATTR_SHADOW`, whether the second provider registers the type or `extend`s it; -- as a common attribute (accepted on every node type) another provider already registered — - `ERR_PROVIDER_ATTR_CONFLICT`. +- as a common attribute (accepted on every node type), or on any type when another provider + already registered it as a common attribute — `ERR_PROVIDER_ATTR_CONFLICT`. A common attribute + is on every slot, so it collides with the same name on any type, in either order. The message names both providers and the attribute. The check runs both ways, so it does not matter which provider registers first. The same provider repeating its own name, sibling @@ -520,7 +521,7 @@ Composition surfaces these codes consistently across all ports (see | `ERR_PROVIDER_DUPLICATE_ID` | Two provider objects report the same `id` | `provider-extension-duplicate-id` | | `ERR_PROVIDER_MISSING_DEPENDENCY` | Provider declares dep on an unregistered id | `provider-extension-missing-dependency` | | `ERR_PROVIDER_DEPENDENCY_CYCLE` | Provider dependencies form a cycle | `provider-extension-dependency-cycle` | -| `ERR_PROVIDER_ATTR_CONFLICT` | Two providers declare the same attr on the same `(type, subType)`, or register the same common attr | `attr-conflict`, `common-attr-conflict` (provider-composition corpus) | +| `ERR_PROVIDER_ATTR_CONFLICT` | Two providers declare the same attr on the same `(type, subType)`, or one registers a common attr whose name the other holds as a common attr or on any type | `attr-conflict`, `common-attr-conflict`, `common-attr-per-type-after`, `common-attr-per-type-before` (provider-composition corpus) | | `ERR_PROVIDER_ATTR_SHADOW` | A provider declares an attr another provider already holds on the shared root or a descendant subtype of the same type | `attr-shadow-inherited`, `attr-shadow-descendant`, `attr-shadow-extend` (provider-composition corpus) | Each conformance fixture's `providers.json: string[]` lists the provider ids to diff --git a/fixtures/conformance/ERROR-CODES.json b/fixtures/conformance/ERROR-CODES.json index 914a67d4cf..25ae559688 100644 --- a/fixtures/conformance/ERROR-CODES.json +++ b/fixtures/conformance/ERROR-CODES.json @@ -27,7 +27,7 @@ "ERR_PROVIDER_DEPENDENCY_CYCLE": "Provider composition has a dependency cycle.", "ERR_PROVIDER_DUPLICATE_ID": "Two providers in a composition share the same id.", "ERR_PROVIDER_MISSING_DEPENDENCY": "A provider declares a dependency id with no matching provider.", - "ERR_PROVIDER_ATTR_CONFLICT": "A provider extend redefines an attr another provider already declared.", + "ERR_PROVIDER_ATTR_CONFLICT": "A provider extend redefines an attr another provider already declared, or a provider registers a common attr whose name another provider holds as a common attr or on any type, or the reverse (ADR-0062).", "ERR_PROVIDER_ATTR_SHADOW": "A provider declares an attribute name on a type that another provider already declares on a related slot: the type it inherits from (.base), or a subtype that inherits from it (ADR-0062). Two providers may not claim one attribute name on a node type; the second needs its own idiomatic name. The same provider re-declaring its own attribute stays legal.", "ERR_EXTEND_REQUIRED_ATTR": "A provider tried to project a REQUIRED attribute onto a type it does not own (via extends). Projection is optional-only: a required attr registered this way disappears silently whenever that provider is composed out, taking its validation rule with it. Declare it with the type instead (ADR-0050).", "ERR_SUBTYPE_RULE_VIOLATION": "A node violates a subtype composition rule (e.g. a value object with an identity or a source — FR-024 value purity; a projection extending a non-projection — FR-024/ADR-0028).", diff --git a/fixtures/provider-composition-conformance/README.md b/fixtures/provider-composition-conformance/README.md index 65896eb341..3e408215e8 100644 --- a/fixtures/provider-composition-conformance/README.md +++ b/fixtures/provider-composition-conformance/README.md @@ -11,7 +11,7 @@ cross-port. | `ERR_PROVIDER_DUPLICATE_ID` | Two providers in a composition report the same `id`. | | `ERR_PROVIDER_MISSING_DEPENDENCY` | A provider declares a dependency id absent from the set. | | `ERR_PROVIDER_DEPENDENCY_CYCLE` | Providers form a dependency cycle (topo-sort cannot proceed). | -| `ERR_PROVIDER_ATTR_CONFLICT` | A provider `extend`s a type, redefining an attr another provider already declared; or registers a common attr another provider already registered (ADR-0062). | +| `ERR_PROVIDER_ATTR_CONFLICT` | A provider `extend`s a type, redefining an attr another provider already declared; or a common attr and another provider's common or per-type attr share a name, in either order (ADR-0062). | | `ERR_PROVIDER_ATTR_SHADOW` | A provider declares an attr name that another provider already declares on the type it inherits from (`.base`), or on a subtype that inherits from it, whether it registers or `extend`s the type (ADR-0062). | | `ERR_REGISTRY_SEALED` | A registration is attempted against a registry sealed after bootstrap (ADR-0023). | @@ -142,9 +142,15 @@ The common-attr providers each register one optional string common attr: |----|--------------|-----------| | `common-owner` | none | common attr `commonAttr`. | | `common-clash` | `common-owner` | common attr `commonAttr` (the name `common-owner` owns). | -| `common-self` | none | common attr `selfCommonAttr`, then the SAME common attr again. | +| `common-self` | none | common attr `selfCommonAttr`, then `template.shadowprobe` with attr `selfCommonAttr`, then the SAME common attr again. | +| `common-sub-clash` | `common-owner` | `template.shadowprobe` with attr `commonAttr` (the name `common-owner` owns as common). | +| `common-sub-owner` | none | `template.shadowprobe` with attr `lateCommonAttr`. | +| `common-late` | `common-sub-owner` | common attr `lateCommonAttr` (the name `common-sub-owner` owns per-type). | -Composing `["common-owner", "common-clash"]` → `ERR_PROVIDER_ATTR_CONFLICT`. +Composing `["common-owner", "common-clash"]`, `["common-owner", "common-sub-clash"]` and +`["common-sub-owner", "common-late"]` → `ERR_PROVIDER_ATTR_CONFLICT`. A common attr is on every +node type, so it collides with another provider's common attr and, in both orders, with another +provider's per-type attr of the same name. ## The `attr-ownership/` subdir (ADR-0062) @@ -171,8 +177,8 @@ A manifest carries at least one of `expectOwners` / `expectCommonOwners`. inherited attr is asked of the slot that declares it. `distinct-names-load.json` proves distinct names load; `same-provider-redeclare.json` proves one provider re-declaring its own name on a type and on the type it inherits from stays legal; `common-attr-owners.json` proves distinct common attr -names from two providers load, one provider re-registering its own common attr stays legal, and each -port reports the same common-attr owner. +names from two providers load, one provider declaring its own common attr name per-type and +re-registering the common attr stays legal, and each port reports the same owners. ## Notes diff --git a/fixtures/provider-composition-conformance/attr-ownership/common-attr-owners.json b/fixtures/provider-composition-conformance/attr-ownership/common-attr-owners.json index c6b5aad2cd..285e042179 100644 --- a/fixtures/provider-composition-conformance/attr-ownership/common-attr-owners.json +++ b/fixtures/provider-composition-conformance/attr-ownership/common-attr-owners.json @@ -1,9 +1,17 @@ { - "description": "Two providers registering DISTINCT common attr names compose cleanly, one provider re-registering its own common attr stays legal, and the registry records which provider owns each common attr.", + "description": "Two providers registering DISTINCT common attr names compose cleanly, one provider declaring its own common attr name per-type and re-registering it as a common attr stays legal, and the registry records which provider owns each common attr and each per-type attr.", "providers": [ "common-owner", "common-self" ], + "expectOwners": [ + { + "type": "template", + "subType": "shadowprobe", + "attr": "selfCommonAttr", + "owner": "common-self" + } + ], "expectCommonOwners": [ { "attr": "commonAttr", diff --git a/fixtures/provider-composition-conformance/common-attr-per-type-after.json b/fixtures/provider-composition-conformance/common-attr-per-type-after.json new file mode 100644 index 0000000000..3473184de8 --- /dev/null +++ b/fixtures/provider-composition-conformance/common-attr-per-type-after.json @@ -0,0 +1,8 @@ +{ + "description": "A provider declares a per-type attr whose name another provider already registers as a common attr (accepted on every node type).", + "providers": [ + "common-owner", + "common-sub-clash" + ], + "expectedError": "ERR_PROVIDER_ATTR_CONFLICT" +} diff --git a/fixtures/provider-composition-conformance/common-attr-per-type-before.json b/fixtures/provider-composition-conformance/common-attr-per-type-before.json new file mode 100644 index 0000000000..d7971f5395 --- /dev/null +++ b/fixtures/provider-composition-conformance/common-attr-per-type-before.json @@ -0,0 +1,8 @@ +{ + "description": "A provider registers a common attr (accepted on every node type) whose name another provider already declares per-type. The reverse order of common-attr-per-type-after; the verdict must not depend on which provider registers first.", + "providers": [ + "common-sub-owner", + "common-late" + ], + "expectedError": "ERR_PROVIDER_ATTR_CONFLICT" +} diff --git a/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs b/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs index da8642ceb7..8debb3c513 100644 --- a/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs +++ b/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs @@ -172,18 +172,19 @@ public void RegisterTypes(TypeRegistry registry) } } -///

ADR-0062 — registers one optional string common attr, times times. +/// ADR-0062 — registers one optional string common attr; selfClaim also declares +/// it on template.shadowprobe and then registers the common attr again. internal sealed class CommonAttrTestProvider : IMetaDataTypeProvider { private readonly string _attr; - private readonly int _times; + private readonly bool _selfClaim; - public CommonAttrTestProvider(string id, string[] dependencies, string attr, int times = 1) + public CommonAttrTestProvider(string id, string[] dependencies, string attr, bool selfClaim = false) { Id = id; Dependencies = dependencies; _attr = attr; - _times = times; + _selfClaim = selfClaim; } public string Id { get; } @@ -191,14 +192,19 @@ public CommonAttrTestProvider(string id, string[] dependencies, string attr, int public void RegisterTypes(TypeRegistry registry) { - for (int i = 0; i < _times; i++) + RegisterCommon(registry); + if (_selfClaim) { - registry.RegisterCommonAttrs(new[] - { - new AttrSchema(_attr, "string", Required: false, Description: "Ownership probe common attr."), - }); + new ShadowTestProvider(Id, System.Array.Empty(), (ShadowTestProvider.ShadowSubType, _attr)).RegisterTypes(registry); + RegisterCommon(registry); } } + + private void RegisterCommon(TypeRegistry registry) => + registry.RegisterCommonAttrs(new[] + { + new AttrSchema(_attr, "string", Required: false, Description: "Ownership probe common attr."), + }); } public sealed class ProviderCompositionConformanceTests @@ -225,7 +231,10 @@ public sealed class ProviderCompositionConformanceTests ["shadow-sub-extend-clash"] = new ShadowSubExtendClashProvider(), ["common-owner"] = new CommonAttrTestProvider("common-owner", System.Array.Empty(), "commonAttr"), ["common-clash"] = new CommonAttrTestProvider("common-clash", new[] { "common-owner" }, "commonAttr"), - ["common-self"] = new CommonAttrTestProvider("common-self", System.Array.Empty(), "selfCommonAttr", times: 2), + ["common-self"] = new CommonAttrTestProvider("common-self", System.Array.Empty(), "selfCommonAttr", selfClaim: true), + ["common-sub-clash"] = new ShadowTestProvider("common-sub-clash", new[] { "common-owner" }, (ShadowTestProvider.ShadowSubType, "commonAttr")), + ["common-sub-owner"] = new ShadowTestProvider("common-sub-owner", System.Array.Empty(), (ShadowTestProvider.ShadowSubType, "lateCommonAttr")), + ["common-late"] = new CommonAttrTestProvider("common-late", new[] { "common-sub-owner" }, "lateCommonAttr"), }; // Flat-corpus (error-code) manifest shape — unchanged. diff --git a/server/csharp/MetaObjects/Registry.cs b/server/csharp/MetaObjects/Registry.cs index 66efa3a0e7..accf05604f 100644 --- a/server/csharp/MetaObjects/Registry.cs +++ b/server/csharp/MetaObjects/Registry.cs @@ -255,7 +255,8 @@ public string AttrOrigin(string type, string subType, string attrName) => /// on (type, subType) when a DIFFERENT provider Q already put /// the same name on a related slot: the slot itself, the type it inherits from /// (<type>.base) or, when this slot is the shared root, any subtype that - /// inherits from it. Checked in both directions so the verdict does not depend on which + /// inherits from it; or registered it as a common attr, which every node type accepts. + /// Checked in both directions so the verdict does not depend on which /// provider registered first. The same provider declaring a name on several related /// slots stays legal, and an attr registered outside any provider's turn (no current /// provider, or the library sentinel) claims nothing. @@ -265,6 +266,16 @@ private void CheckAttrOwnership(string type, string subType, string name) string? claimant = CurrentProviderId; if (claimant is null) return; + string commonOwner = CommonAttrOrigin(name); + if (commonOwner != LibrarySentinel && commonOwner != claimant) + { + throw new MetaModelException( + $"Provider \"{claimant}\" declares attribute \"{name}\" on \"{type}.{subType}\", but provider " + + $"\"{commonOwner}\" already registers it as a common attribute, which every node type accepts; " + + $"provider \"{claimant}\" cannot declare it again (ADR-0062)", + ErrorCode.ERR_PROVIDER_ATTR_CONFLICT); + } + var related = new List { subType }; if (subType == SUBTYPE_BASE) { @@ -427,9 +438,9 @@ public void SetDefaultSubType(string type, string subType) /// Register attrs that are accepted on every metatype. Used by providers to /// declare cross-cutting attrs (e.g. documentation attrs). The same provider (or /// a registration outside any provider's turn) re-registering a name is deduped; a - /// different provider registering a name another provider holds throws - /// ERR_PROVIDER_ATTR_CONFLICT (ADR-0062). Conflicts with per-type attrs are - /// detected at validation time. + /// different provider registering a name another provider holds, as a common attr or on + /// any type, throws ERR_PROVIDER_ATTR_CONFLICT (ADR-0062). A name one provider + /// holds both ways is resolved at validation time. /// Throws if any attr has /// ValueType == SUBTYPE_BASE. /// @@ -441,14 +452,25 @@ public void RegisterCommonAttrs(IEnumerable attrs) string? claimant = CurrentProviderId; foreach (AttrSchema attr in batch) { + if (claimant is null) break; string owner = CommonAttrOrigin(attr.Name); - if (claimant is not null && owner != LibrarySentinel && owner != claimant) + if (owner != LibrarySentinel && owner != claimant) { throw new MetaModelException( $"Provider \"{claimant}\" registers common attribute \"{attr.Name}\", but provider \"{owner}\" " + $"already registers it as a common attribute; provider \"{claimant}\" cannot declare it again (ADR-0062)", ErrorCode.ERR_PROVIDER_ATTR_CONFLICT); } + + foreach (var ((slotType, slotSubType, slotAttr), slotOwner) in _attrProvenance) + { + if (slotAttr != attr.Name || slotOwner == LibrarySentinel || slotOwner == claimant) continue; + throw new MetaModelException( + $"Provider \"{claimant}\" registers common attribute \"{attr.Name}\", which every node type accepts, " + + $"but provider \"{slotOwner}\" already declares it on \"{slotType}.{slotSubType}\"; provider " + + $"\"{claimant}\" cannot declare it again (ADR-0062)", + ErrorCode.ERR_PROVIDER_ATTR_CONFLICT); + } } foreach (AttrSchema attr in batch) @@ -479,7 +501,7 @@ public void RegisterCommonAttrs(IEnumerable attrs) if (_commonAttrs.Any(existing => existing.Name == attr.Name)) { - continue; // same-name dedupe; per-type conflicts handled at validation time + continue; // same-name dedupe } _commonAttrs.Add(attr); diff --git a/server/java/metadata/src/main/java/com/metaobjects/MetaData.java b/server/java/metadata/src/main/java/com/metaobjects/MetaData.java index 91866188c2..adb4ac9809 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/MetaData.java +++ b/server/java/metadata/src/main/java/com/metaobjects/MetaData.java @@ -17,7 +17,6 @@ import com.metaobjects.attr.BooleanAttribute; import com.metaobjects.attr.MetaAttribute; -import com.metaobjects.attr.StringAttribute; import com.metaobjects.field.MetaField; // ✅ MIGRATED: MetaKey import removed - using MetaRelationship instead import com.metaobjects.object.MetaObject; @@ -148,7 +147,7 @@ public class MetaData implements Cloneable, Serializable { public static void registerTypes(MetaDataRegistry registry) { try { // Check if MetaData base type is already registered to prevent duplicates - if (registry.hasConstraint("metadata.base.description.placement")) { + if (registry.hasConstraint("metadata.base.isAbstract.placement")) { log.debug("MetaData base type already registered, skipping"); return; } @@ -158,7 +157,6 @@ public static void registerTypes(MetaDataRegistry registry) { .description("Base metadata type for inheritance hierarchy - enforces abstract requirements") // COMMON ATTRIBUTES available to all metadata types - .optionalAttribute(ATTR_DESCRIPTION, StringAttribute.SUBTYPE_STRING) // Description attribute for all metadata .optionalAttribute(ATTR_IS_ABSTRACT, BooleanAttribute.SUBTYPE_BOOLEAN) // Abstract flag for all metadata // ROOT LEVEL can contain top-level metadata types diff --git a/server/java/metadata/src/main/java/com/metaobjects/identity/PrimaryIdentity.java b/server/java/metadata/src/main/java/com/metaobjects/identity/PrimaryIdentity.java index 7707736146..9ed59d7cb4 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/identity/PrimaryIdentity.java +++ b/server/java/metadata/src/main/java/com/metaobjects/identity/PrimaryIdentity.java @@ -7,7 +7,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static com.metaobjects.object.MetaObject.ATTR_DESCRIPTION; import static com.metaobjects.attr.StringAttribute.SUBTYPE_STRING; /** @@ -49,7 +48,6 @@ public static void registerTypes(MetaDataRegistry registry) { // the dedicated validateIdentityFieldsAndGeneration pass already enforces it). def.requiredAttributeWithConstraints(ATTR_FIELDS).ofType(StringAttribute.SUBTYPE_STRING).asArray(); def.optionalAttributeWithConstraints(ATTR_GENERATION).ofType(StringAttribute.SUBTYPE_STRING).withEnum(GENERATION_INCREMENT, GENERATION_UUID, GENERATION_ASSIGNED); - def.optionalAttributeWithConstraints(ATTR_DESCRIPTION).ofType(StringAttribute.SUBTYPE_STRING).asSingle(); // ACCEPTS ANY ATTRIBUTES (for extensibility from service providers) // ADR-0051: NO any-attr wildcard. An undeclared attribute is ERR_UNKNOWN_ATTR in diff --git a/server/java/metadata/src/main/java/com/metaobjects/identity/ReferenceIdentity.java b/server/java/metadata/src/main/java/com/metaobjects/identity/ReferenceIdentity.java index b803e61750..1e8df0b0a5 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/identity/ReferenceIdentity.java +++ b/server/java/metadata/src/main/java/com/metaobjects/identity/ReferenceIdentity.java @@ -12,8 +12,6 @@ import java.util.Collections; import java.util.List; -import static com.metaobjects.object.MetaObject.ATTR_DESCRIPTION; - /** * Reference identity — declares that this entity has fields whose value(s) * identify an instance of another entity. @@ -65,7 +63,6 @@ public static void registerTypes(MetaDataRegistry registry) { def.reference(new com.metaobjects.validation.ReferenceDescriptor( ATTR_REFERENCES, com.metaobjects.object.MetaObject.TYPE_OBJECT, null, true, "ERR_INVALID_REFERENCE")); def.optionalAttributeWithConstraints(ATTR_ENFORCE).ofType(BooleanAttribute.SUBTYPE_BOOLEAN).asSingle(); - def.optionalAttributeWithConstraints(ATTR_DESCRIPTION).ofType(StringAttribute.SUBTYPE_STRING).asSingle(); // RDB-physical FK constraint-name override contributed by the db provider // (db.json extends identity.reference). Description sourced from the embedded // spec/metamodel/db.json by applySpecDescriptions. diff --git a/server/java/metadata/src/main/java/com/metaobjects/identity/SecondaryIdentity.java b/server/java/metadata/src/main/java/com/metaobjects/identity/SecondaryIdentity.java index 4ead072b01..2874ff7045 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/identity/SecondaryIdentity.java +++ b/server/java/metadata/src/main/java/com/metaobjects/identity/SecondaryIdentity.java @@ -7,7 +7,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static com.metaobjects.object.MetaObject.ATTR_DESCRIPTION; import static com.metaobjects.attr.StringAttribute.SUBTYPE_STRING; /** @@ -61,7 +60,6 @@ public static void registerTypes(MetaDataRegistry registry) { // @fields XOR @expr, enforced by ValidationPhase (ERR_INVALID_INDEX), // because an exclusive-or is not expressible as a per-attr required flag. def.optionalAttributeWithConstraints(ATTR_FIELDS).ofType(StringAttribute.SUBTYPE_STRING).asArray(); - def.optionalAttributeWithConstraints(ATTR_DESCRIPTION).ofType(StringAttribute.SUBTYPE_STRING).asSingle(); // RDB-physical index attrs (@orders / @where / @expr / @using) are // contributed by CoreDBMetaDataProvider via registry.extendType, mirroring // the TS db provider's "extends" blocks in spec/metamodel/db.json. diff --git a/server/java/metadata/src/main/java/com/metaobjects/index/LookupIndex.java b/server/java/metadata/src/main/java/com/metaobjects/index/LookupIndex.java index 24dcfe8ec1..b1b20b5156 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/index/LookupIndex.java +++ b/server/java/metadata/src/main/java/com/metaobjects/index/LookupIndex.java @@ -20,8 +20,6 @@ import com.metaobjects.attr.StringAttribute; import com.metaobjects.registry.MetaDataRegistry; -import static com.metaobjects.object.MetaObject.ATTR_DESCRIPTION; - /** * A non-unique lookup index on one or more fields ({@code index.lookup}). * @@ -63,9 +61,6 @@ public static void registerTypes(MetaDataRegistry registry) { def.optionalAttributeWithConstraints(ATTR_FIELDS) .ofType(StringAttribute.SUBTYPE_STRING).asArray(); - def.optionalAttributeWithConstraints(ATTR_DESCRIPTION) - .ofType(StringAttribute.SUBTYPE_STRING).asSingle(); - // ACCEPTS ANY CHILD ATTRIBUTES (for extensibility from service providers) // ADR-0051: NO any-attr wildcard. An undeclared attribute is ERR_UNKNOWN_ATTR in // every port; extension is REGISTRATION -- a downstream provider declares its diff --git a/server/java/metadata/src/main/java/com/metaobjects/object/MetaObject.java b/server/java/metadata/src/main/java/com/metaobjects/object/MetaObject.java index d744a023ba..ac83f3d6eb 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/object/MetaObject.java +++ b/server/java/metadata/src/main/java/com/metaobjects/object/MetaObject.java @@ -135,7 +135,6 @@ public static void registerTypes(MetaDataRegistry registry) { def.optionalAttributeWithConstraints(ATTR_IS_INTERFACE).ofType(BooleanAttribute.SUBTYPE_BOOLEAN).asSingle(); // OBJECT-SPECIFIC ATTRIBUTES - def.optionalAttributeWithConstraints(ATTR_DESCRIPTION).ofType(StringAttribute.SUBTYPE_STRING).asSingle(); def.optionalAttributeWithConstraints(ATTR_OBJECT).ofType(StringAttribute.SUBTYPE_STRING).asSingle(); // NOTE: @objectRef is NOT registered on object.base. It is a FIELD-level // attr (registered on field.base — see MetaField) consumed by field.object diff --git a/server/java/metadata/src/main/java/com/metaobjects/registry/MetaDataRegistry.java b/server/java/metadata/src/main/java/com/metaobjects/registry/MetaDataRegistry.java index 5a6f824ad3..5c70193dc1 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/registry/MetaDataRegistry.java +++ b/server/java/metadata/src/main/java/com/metaobjects/registry/MetaDataRegistry.java @@ -857,8 +857,8 @@ public void addGlobalChildRequirement(String parentType, String parentSubType, C * * *

Idempotent: re-registering the same attr name is a no-op for the provider that - * registered it; another provider registering the same common attr name is refused - * (ADR-0062, {@code ERR_PROVIDER_ATTR_CONFLICT}).

+ * registered it. A common attr is on every node type, so another provider may not claim + * its name, as a common attr or on any type (ADR-0062, {@code ERR_PROVIDER_ATTR_CONFLICT}).

* * @param name bare attribute name (no {@code @} prefix; e.g. {@code "description"}) * @param valueType attribute value subtype (e.g. {@link StringAttribute#SUBTYPE_STRING}, @@ -1290,6 +1290,15 @@ private void checkAttrOwnership(MetaDataTypeId typeId, String parentType, String } Set related = relatedSlots(typeId, parentType, parentSubType); for (String name : names) { + String commonOwner = getCommonAttrProvider(name); + if (commonOwner != null && !commonOwner.equals(claimant)) { + throw new MetaDataException( + "Provider \"" + claimant + "\" declares attribute \"" + name + "\" on \"" + + typeId.toQualifiedName() + "\", but provider \"" + commonOwner + + "\" already registers it as a common attribute, which every node type accepts; provider \"" + + claimant + "\" cannot declare it again (ADR-0062).", + com.metaobjects.ErrorCode.ERR_PROVIDER_ATTR_CONFLICT); + } for (MetaDataTypeId slot : related) { Map byName = attrProvenance.get(slot); String owner = byName != null ? byName.get(name) : null; @@ -1308,9 +1317,10 @@ private void checkAttrOwnership(MetaDataTypeId typeId, String parentType, String } /** - * ADR-0062 — provider P may not register common attr {@code name} when a DIFFERENT - * provider already registered it as a common attr. Re-registration by the same - * provider, and registration outside any provider's turn, stay legal. + * ADR-0062 — a common attr is on every node type, so provider P may not register common + * attr {@code name} when a DIFFERENT provider already registered that name as a common + * attr or on any type. Re-registration by the same provider, and registration outside + * any provider's turn, stay legal. */ private void checkCommonAttrOwnership(String name) { String claimant = currentProviderId; @@ -1325,6 +1335,18 @@ private void checkCommonAttrOwnership(String name) { + "\" cannot declare it again (ADR-0062).", com.metaobjects.ErrorCode.ERR_PROVIDER_ATTR_CONFLICT); } + for (Map.Entry> entry : attrProvenance.entrySet()) { + String owner = entry.getValue().get(name); + if (owner == null || LIBRARY_PROVENANCE.equals(owner) || owner.equals(claimant)) { + continue; + } + throw new MetaDataException( + "Provider \"" + claimant + "\" registers common attribute \"" + name + "\", which every node " + + "type accepts, but provider \"" + owner + "\" already declares it on \"" + + entry.getKey().toQualifiedName() + "\"; provider \"" + claimant + + "\" cannot declare it again (ADR-0062).", + com.metaobjects.ErrorCode.ERR_PROVIDER_ATTR_CONFLICT); + } } /** diff --git a/server/java/metadata/src/main/java/com/metaobjects/registry/RegistryManifest.java b/server/java/metadata/src/main/java/com/metaobjects/registry/RegistryManifest.java index 9c97e70d21..1b1a7d0252 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/registry/RegistryManifest.java +++ b/server/java/metadata/src/main/java/com/metaobjects/registry/RegistryManifest.java @@ -334,8 +334,6 @@ public enum ExclusionReason { NATIVE_BINDING, /** Bare structural / OO-shape keyword (isArray/isAbstract/extends/implements/isInterface). */ STRUCTURAL_KEYWORD, - /** A commonAttr ({@code description}) re-registered per-type — belongs in commonAttrs. */ - COMMON_ATTR_DUP, /** The {@code metadata.base} per-port inheritance anchor (deferred inheritsFrom facet). */ INHERITANCE_ANCHOR, /** TS-web-presentation-only facet (the generic {@code view.*} controls). */ @@ -362,8 +360,7 @@ public enum ExclusionReason { * NOT per-type attributes — Java registers them on {@code object.base} * (inherited by entity/value), so the classification is what drops them from * the emitter (no-op for TS/C#/Python). They remain REGISTERED (the loader - * must accept an authored {@code extends:}). {@code description} is carved out - * ONLY per-type — it remains in the {@code commonAttrs} block. {@code object} + * must accept an authored {@code extends:}). {@code object} * (ADR-0001 class-FQN type binding) and {@code objectAdapter} (ADR-0005 * value-access seam) are per-port binding mechanisms (same category as native * type bindings); they stay REGISTERED in Java (read by the value-access @@ -376,8 +373,7 @@ public enum ExclusionReason { com.metaobjects.object.MetaObject.ATTR_IMPLEMENTS, ExclusionReason.STRUCTURAL_KEYWORD, com.metaobjects.object.MetaObject.ATTR_IS_INTERFACE, ExclusionReason.STRUCTURAL_KEYWORD, com.metaobjects.object.MetaObject.ATTR_OBJECT, ExclusionReason.NATIVE_BINDING, - com.metaobjects.object.AbstractObjectRepresentation.ATTR_OBJECT_ADAPTER, ExclusionReason.NATIVE_BINDING, - MetaData.ATTR_DESCRIPTION, ExclusionReason.COMMON_ATTR_DUP); + com.metaobjects.object.AbstractObjectRepresentation.ATTR_OBJECT_ADAPTER, ExclusionReason.NATIVE_BINDING); /** * Classify a per-type attr: an {@link ExclusionReason} (carved out) or diff --git a/server/java/metadata/src/main/java/com/metaobjects/relationship/MetaRelationship.java b/server/java/metadata/src/main/java/com/metaobjects/relationship/MetaRelationship.java index 8f8363173b..d17082f134 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/relationship/MetaRelationship.java +++ b/server/java/metadata/src/main/java/com/metaobjects/relationship/MetaRelationship.java @@ -8,8 +8,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static com.metaobjects.object.MetaObject.ATTR_DESCRIPTION; - /** * Abstract base relationship metadata for expressing object relationships. * Provides simplified model-driven relationship patterns optimized for AI generation. @@ -144,10 +142,6 @@ public static void registerTypes(MetaDataRegistry registry) { .ofType(BooleanAttribute.SUBTYPE_BOOLEAN) .asSingle(); - def.optionalAttributeWithConstraints(ATTR_DESCRIPTION) - .ofType(StringAttribute.SUBTYPE_STRING) - .asSingle(); - // @onDelete / @onUpdate — enum-constrained; withEnum also marks as single def.optionalAttributeWithConstraints(ATTR_ON_DELETE) .ofType(StringAttribute.SUBTYPE_STRING) diff --git a/server/java/metadata/src/test/java/com/metaobjects/conformance/ProviderCompositionConformanceTest.java b/server/java/metadata/src/test/java/com/metaobjects/conformance/ProviderCompositionConformanceTest.java index 9a2e74cb71..e2278dd23a 100644 --- a/server/java/metadata/src/test/java/com/metaobjects/conformance/ProviderCompositionConformanceTest.java +++ b/server/java/metadata/src/test/java/com/metaobjects/conformance/ProviderCompositionConformanceTest.java @@ -246,8 +246,16 @@ private static MetaDataTypeProvider shadowProvider(String id, String[] deps, Con private static final MetaDataTypeProvider COMMON_SELF = shadowProvider("common-self", new String[0], r -> { r.registerCommonAttribute("selfCommonAttr", StringAttribute.SUBTYPE_STRING, false); + registerShadowProbe(r, "selfCommonAttr"); r.registerCommonAttribute("selfCommonAttr", StringAttribute.SUBTYPE_STRING, false); }); + private static final MetaDataTypeProvider COMMON_SUB_CLASH = + shadowProvider("common-sub-clash", new String[]{"common-owner"}, r -> registerShadowProbe(r, "commonAttr")); + private static final MetaDataTypeProvider COMMON_SUB_OWNER = + shadowProvider("common-sub-owner", new String[0], r -> registerShadowProbe(r, "lateCommonAttr")); + private static final MetaDataTypeProvider COMMON_LATE = + shadowProvider("common-late", new String[]{"common-sub-owner"}, + r -> r.registerCommonAttribute("lateCommonAttr", StringAttribute.SUBTYPE_STRING, false)); private static MetaDataTypeProvider resolve(String id) { MetaDataTypeProvider p = ConformanceTestProviders.TEST_PROVIDERS.get(id); @@ -267,6 +275,9 @@ private static MetaDataTypeProvider resolve(String id) { case "common-owner": return COMMON_OWNER; case "common-clash": return COMMON_CLASH; case "common-self": return COMMON_SELF; + case "common-sub-clash": return COMMON_SUB_CLASH; + case "common-sub-owner": return COMMON_SUB_OWNER; + case "common-late": return COMMON_LATE; default: throw new IllegalArgumentException( "Unknown named provider \"" + id + "\" in provider-composition corpus"); diff --git a/server/java/metadata/src/test/java/com/metaobjects/registry/RegistryManifestConformanceTest.java b/server/java/metadata/src/test/java/com/metaobjects/registry/RegistryManifestConformanceTest.java index c395cec386..f4622e7897 100644 --- a/server/java/metadata/src/test/java/com/metaobjects/registry/RegistryManifestConformanceTest.java +++ b/server/java/metadata/src/test/java/com/metaobjects/registry/RegistryManifestConformanceTest.java @@ -216,8 +216,6 @@ public void carveOutsCarryTheirDeclaredReasonCategory() { RegistryManifest.classifyPerTypeAttr("object")); assertEquals(RegistryManifest.ExclusionReason.NATIVE_BINDING, RegistryManifest.classifyPerTypeAttr("objectAdapter")); - assertEquals(RegistryManifest.ExclusionReason.COMMON_ATTR_DUP, - RegistryManifest.classifyPerTypeAttr("description")); // Genuinely logical attrs are INCLUDED. assertEquals(RegistryManifest.ExclusionReason.INCLUDED, RegistryManifest.classifyPerTypeAttr("column")); diff --git a/server/python/src/metaobjects/registry.py b/server/python/src/metaobjects/registry.py index 8003159556..f0ccee113e 100644 --- a/server/python/src/metaobjects/registry.py +++ b/server/python/src/metaobjects/registry.py @@ -155,7 +155,8 @@ def _check_attr_ownership(self, type_: str, sub_type: str, name: str) -> None: Provider P may not put attribute ``name`` on ``(type_, sub_type)`` when a DIFFERENT provider Q already put the same name on a related slot: the slot itself, the type it inherits from (``.base``) or, when this slot is the shared root, any - subtype that inherits from it. Checked in both directions so the verdict does not + subtype that inherits from it; or registered it as a common attr, which every node + type accepts. Checked in both directions so the verdict does not depend on which provider registered first. The same provider declaring a name on several related slots stays legal, and an attr registered outside any provider's turn (no current provider, or the library sentinel) claims nothing. @@ -163,6 +164,14 @@ def _check_attr_ownership(self, type_: str, sub_type: str, name: str) -> None: claimant = self._current_provider_id if claimant is None: return + common_owner = self._common_attr_provenance.get(name, LIBRARY_ATTR_ORIGIN) + if common_owner not in (LIBRARY_ATTR_ORIGIN, claimant): + raise ParseError( + f'Provider "{claimant}" declares attribute "{name}" on "{type_}.{sub_type}", but ' + f'provider "{common_owner}" already registers it as a common attribute, which every ' + f'node type accepts; provider "{claimant}" cannot declare it again (ADR-0062)', + ErrorCode.ERR_PROVIDER_ATTR_CONFLICT, + ) related = [sub_type] if sub_type == SUBTYPE_BASE: related += [s for (t, s) in self._defs if t == type_ and s != SUBTYPE_BASE] @@ -267,22 +276,34 @@ def register_common_attrs(self, attrs: list[AttrSchema]) -> None: """Register attrs accepted on every metatype. The same provider (or a registration outside any provider's turn) re-registering a - name is deduped; a different provider registering a name another provider holds - raises ``ERR_PROVIDER_ATTR_CONFLICT`` (ADR-0062). Conflict with per-type attrs is - detected at validation time, not here. + name is deduped; a different provider registering a name another provider holds, as + a common attr or on any type, raises ``ERR_PROVIDER_ATTR_CONFLICT`` (ADR-0062). A + name one provider holds both ways is resolved at validation time. """ self._check_not_sealed("register_common_attrs") # ADR-0062: checked for the whole batch first, so a refusal leaves no trace. claimant = self._current_provider_id for attr in attrs: + if claimant is None: + break owner = self._common_attr_provenance.get(attr.name, LIBRARY_ATTR_ORIGIN) - if claimant is not None and owner not in (LIBRARY_ATTR_ORIGIN, claimant): + if owner not in (LIBRARY_ATTR_ORIGIN, claimant): raise ParseError( f'Provider "{claimant}" registers common attribute "{attr.name}", but ' f'provider "{owner}" already registers it as a common attribute; provider ' f'"{claimant}" cannot declare it again (ADR-0062)', ErrorCode.ERR_PROVIDER_ATTR_CONFLICT, ) + for (slot_type, slot_sub, slot_attr), slot_owner in self._attr_provenance.items(): + if slot_attr != attr.name or slot_owner in (LIBRARY_ATTR_ORIGIN, claimant): + continue + raise ParseError( + f'Provider "{claimant}" registers common attribute "{attr.name}", which every ' + f'node type accepts, but provider "{slot_owner}" already declares it on ' + f'"{slot_type}.{slot_sub}"; provider "{claimant}" cannot declare it again ' + "(ADR-0062)", + ErrorCode.ERR_PROVIDER_ATTR_CONFLICT, + ) for attr in attrs: # ADR-0050, the second door. A common attr is projected onto EVERY registered # type, so a required one is the same defect as a required extend() -- with the diff --git a/server/python/tests/conformance/test_provider_composition_conformance.py b/server/python/tests/conformance/test_provider_composition_conformance.py index c95bcc2296..e9530d38b1 100644 --- a/server/python/tests/conformance/test_provider_composition_conformance.py +++ b/server/python/tests/conformance/test_provider_composition_conformance.py @@ -152,15 +152,18 @@ def register_types(self, registry: TypeRegistry) -> None: # noqa: D401 class _CommonAttrProvider(Provider): - """Registers one optional string common attr, `times` times.""" + """Registers one optional string common attr; `self_claim` also declares it on + `template.shadowprobe` and then registers the common attr again.""" - def __init__(self, provider_id: str, deps: tuple[str, ...], attr_name: str, times: int = 1) -> None: + def __init__(self, provider_id: str, deps: tuple[str, ...], attr_name: str, self_claim: bool = False) -> None: super().__init__(provider_id, dependencies=deps) self._attr_name = attr_name - self._times = times + self._self_claim = self_claim def register_types(self, registry: TypeRegistry) -> None: # noqa: D401 - for _ in range(self._times): + registry.register_common_attrs([AttrSchema(self._attr_name, "string")]) + if self._self_claim: + registry.register(_template_type(_SHADOW_SUBTYPE, self._attr_name)) registry.register_common_attrs([AttrSchema(self._attr_name, "string")]) @@ -169,7 +172,10 @@ def _providers() -> dict[str, Provider]: "shadow-sub-extend-clash": _ShadowSubExtendClashProvider(), "common-owner": _CommonAttrProvider("common-owner", (), "commonAttr"), "common-clash": _CommonAttrProvider("common-clash", ("common-owner",), "commonAttr"), - "common-self": _CommonAttrProvider("common-self", (), "selfCommonAttr", times=2), + "common-self": _CommonAttrProvider("common-self", (), "selfCommonAttr", self_claim=True), + "common-sub-clash": _shadow_provider("common-sub-clash", ("common-owner",), (_SHADOW_SUBTYPE, "commonAttr")), + "common-sub-owner": _shadow_provider("common-sub-owner", (), (_SHADOW_SUBTYPE, "lateCommonAttr")), + "common-late": _CommonAttrProvider("common-late", ("common-sub-owner",), "lateCommonAttr"), "shadow-base": _shadow_provider("shadow-base", (), ("base", "shadowAttr")), "shadow-sub-clash": _shadow_provider( "shadow-sub-clash", ("shadow-base",), (_SHADOW_SUBTYPE, "shadowAttr") diff --git a/server/typescript/packages/metadata/src/registry.ts b/server/typescript/packages/metadata/src/registry.ts index d44a784a27..028526c966 100644 --- a/server/typescript/packages/metadata/src/registry.ts +++ b/server/typescript/packages/metadata/src/registry.ts @@ -212,7 +212,8 @@ export class TypeRegistry { * ADR-0062 — the provider-collision rule. Provider P may not put attribute `name` on * `(type, subType)` when a DIFFERENT provider Q already put the same name on a related * slot: the slot itself, a type it inherits from (`.base`), or — when this slot - * is the shared root — any subtype that inherits from it. Checked in both directions + * is the shared root — any subtype that inherits from it; or registered it as a common + * attribute, which every node type accepts. Checked in both directions * so the verdict does not depend on which provider happened to register first. The * same provider declaring a name on several related slots stays legal, and an * attribute registered outside any provider's turn claims nothing. @@ -220,6 +221,15 @@ export class TypeRegistry { private checkAttrOwnership(type: string, subType: string, name: string): void { const claimant = this._currentProviderId; if (claimant === undefined) return; + const commonOwner = this._commonAttrOwners.get(name); + if (commonOwner !== undefined && commonOwner !== claimant) { + throw new MetaModelError( + `Provider "${claimant}" declares attribute "${name}" on "${type}.${subType}", but provider ` + + `"${commonOwner}" already registers it as a common attribute, which every node type accepts; ` + + `provider "${claimant}" cannot declare it again (ADR-0062)`, + { code: "ERR_PROVIDER_ATTR_CONFLICT" }, + ); + } const related: string[] = []; if (subType === SUBTYPE_BASE) { for (const sub of this._subTypes.get(type) ?? []) { @@ -339,24 +349,36 @@ export class TypeRegistry { * Register attrs that are accepted on every metatype. Callers (providers) use * this to declare cross-cutting attrs (e.g. doc attrs). The same provider (or a * registration outside any provider's turn) re-registering a name is deduped; a - * different provider registering a name another provider holds is - * `ERR_PROVIDER_ATTR_CONFLICT` (ADR-0062). Conflicts with per-type attrs are - * resolved at validation time (Task 1.3). + * different provider registering a name another provider holds, as a common attr or + * on any type, is `ERR_PROVIDER_ATTR_CONFLICT` (ADR-0062). A name one provider holds + * both ways is resolved at validation time (Task 1.3). */ registerCommonAttrs(attrs: AttrSchema[]): void { this.checkNotSealed("registerCommonAttrs"); - // ADR-0062: another provider may not register a common attr name a provider already - // holds. Checked for the whole batch first, so a refusal leaves no trace. + // ADR-0062: a common attr is on every node type, so another provider may not register + // a common attr name a provider already holds, as a common attr or on any type. + // Checked for the whole batch first, so a refusal leaves no trace. const claimant = this._currentProviderId; for (const attr of attrs) { + if (claimant === undefined) break; const owner = this._commonAttrOwners.get(attr.name); - if (owner !== undefined && claimant !== undefined && owner !== claimant) { + if (owner !== undefined && owner !== claimant) { throw new MetaModelError( `Provider "${claimant}" registers common attribute "${attr.name}", but provider "${owner}" ` + `already registers it as a common attribute; provider "${claimant}" cannot declare it again (ADR-0062)`, { code: "ERR_PROVIDER_ATTR_CONFLICT" }, ); } + for (const [slot, owners] of this._attrOwners) { + const slotOwner = owners.get(attr.name); + if (slotOwner === undefined || slotOwner === claimant) continue; + throw new MetaModelError( + `Provider "${claimant}" registers common attribute "${attr.name}", which every node type accepts, ` + + `but provider "${slotOwner}" already declares it on "${slot}"; provider "${claimant}" cannot ` + + `declare it again (ADR-0062)`, + { code: "ERR_PROVIDER_ATTR_CONFLICT" }, + ); + } } for (const attr of attrs) { if (attr.valueType === SUBTYPE_BASE) { @@ -380,7 +402,7 @@ export class TypeRegistry { ); } if (this._commonAttrs.some((existing) => existing.name === attr.name)) { - continue; // dedupe same-name re-registration; conflict-with-per-type-attr is checked at validation time + continue; // dedupe same-name re-registration } this._commonAttrs.push(attr); if (claimant !== undefined) this._commonAttrOwners.set(attr.name, claimant); diff --git a/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts b/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts index b2c8835cc9..a8ba5a43c9 100644 --- a/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts +++ b/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts @@ -222,15 +222,33 @@ const commonSelfProvider: MetaDataTypeProvider = { id: "common-self", registerTypes(registry) { registry.registerCommonAttrs([commonAttr("selfCommonAttr")]); + registry.register(templateType(SHADOW_SUBTYPE, "selfCommonAttr")); registry.registerCommonAttrs([commonAttr("selfCommonAttr")]); }, }; +const commonSubClashProvider: MetaDataTypeProvider = { + id: "common-sub-clash", + dependencies: ["common-owner"], + registerTypes: (registry) => registry.register(templateType(SHADOW_SUBTYPE, "commonAttr")), +}; +const commonSubOwnerProvider: MetaDataTypeProvider = { + id: "common-sub-owner", + registerTypes: (registry) => registry.register(templateType(SHADOW_SUBTYPE, "lateCommonAttr")), +}; +const commonLateProvider: MetaDataTypeProvider = { + id: "common-late", + dependencies: ["common-sub-owner"], + registerTypes: (registry) => registry.registerCommonAttrs([commonAttr("lateCommonAttr")]), +}; const PROVIDERS: Readonly> = { "shadow-sub-extend-clash": shadowSubExtendClashProvider, "common-owner": commonOwnerProvider, "common-clash": commonClashProvider, "common-self": commonSelfProvider, + "common-sub-clash": commonSubClashProvider, + "common-sub-owner": commonSubOwnerProvider, + "common-late": commonLateProvider, "shadow-base": shadowBaseProvider, "shadow-sub-clash": shadowSubClashProvider, "shadow-sub-owner": shadowSubOwnerProvider, diff --git a/spec/decisions/ADR-0062-provider-attribute-ownership.md b/spec/decisions/ADR-0062-provider-attribute-ownership.md index f662ef9c89..2ba2cc8a63 100644 --- a/spec/decisions/ADR-0062-provider-attribute-ownership.md +++ b/spec/decisions/ADR-0062-provider-attribute-ownership.md @@ -39,9 +39,11 @@ a collision. not a same-slot repeat. A refusal names both providers and the attribute, and leaves nothing behind in the registry. - A common attribute (accepted on every node type) follows the same rule against other common - attributes: a provider that registers common attribute `A` when a different provider already - registered common attribute `A` is refused with `ERR_PROVIDER_ATTR_CONFLICT`. + A common attribute is accepted on every node type, so it is on every slot at once. A provider + that registers common attribute `A` is refused with `ERR_PROVIDER_ATTR_CONFLICT` when a + different provider already registered `A` as a common attribute or on any `(T, S)`; and a + provider that registers or extends any `(T, S)` with `A` is refused with the same code when a + different provider already registered common attribute `A`. Both orders reach the same verdict. 3. **What stays legal.** The same provider declaring a name on several related slots (its own `@x` on the root and on a subtype) and re-registering its own slot. Sibling subtypes using @@ -49,9 +51,8 @@ a collision. provider's turn (a direct `registry.register` call in a test or script) claims nothing. 4. **Scope is one node type.** The JVM registry also chains every `.base` to - `metadata.base`, which the core-types provider owns. No other port has that edge, and seven - shipped providers re-declare `@isAbstract` and/or `@description` on their own root with the - same meaning. The rule stops at the node-type boundary so all five ports reach the same + `metadata.base`, which the core-types provider owns. No other port has that edge, and several + shipped providers re-declare `@isAbstract` on their own root with the same meaning. The rule stops at the node-type boundary so all five ports reach the same verdict, and no shipped provider has to change. 5. **Owner tracking.** The registry records the registering provider per `(type, subType, @@ -71,18 +72,19 @@ a collision. turned on), so existing adopters see no change. - A third-party provider that reuses a core or sibling provider's attribute name now fails at composition with a message that names both providers. The fix is a different idiomatic word. -- A common attribute and a per-type attribute of the same name, from two providers, are not - refused yet. The Java port's core providers (`core-types`, `object-types`, `identity-types`, - `relationship-types` and `index-types`) declare a per-type `@description` beside the - documentation provider's common `@description`. Giving that name one owner removes the attribute - from those providers' own registrations, which is a change to their public attribute contract, - so it is left for a separate decision. Until then, that pair loads as it did before this ADR. +- The Java port's core providers (`core-types`, `object-types`, `identity-types`, + `relationship-types` and `index-types`) used to declare a per-type `@description` beside the + documentation provider's common `@description`, each an optional single string with the same + meaning. They no longer do: the documentation provider is that name's one owner, as it already + is in the other ports, and `@description` is still accepted on every node through the common + attribute. The registry manifest's `description` carve-out for the per-type copies is gone with + them. - The two codes split by where the clash is, not by severity: `ERR_PROVIDER_ATTR_CONFLICT` keeps its established same-slot meaning and fixture, and `ERR_PROVIDER_ATTR_SHADOW` is the new inherited/descendant case. - Conformance: `fixtures/provider-composition-conformance/` carries the refusals (`attr-shadow-inherited`, `attr-shadow-descendant`, `attr-shadow-extend`, - `common-attr-conflict`) and an `attr-ownership/` sub-corpus where composition succeeds and each + `common-attr-conflict`, `common-attr-per-type-after`, `common-attr-per-type-before`) and an `attr-ownership/` sub-corpus where composition succeeds and each port reports the same owner per attribute and per common attribute. ## Alternatives considered From 87ddc914f5dead37e445432ce11eefcdc58a9551 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Wed, 4 Nov 2026 04:26:23 -0500 Subject: [PATCH 5/6] no-mistakes(review): apply reviewer inline fixes --- spec/decisions/ADR-0062-provider-attribute-ownership.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/decisions/ADR-0062-provider-attribute-ownership.md b/spec/decisions/ADR-0062-provider-attribute-ownership.md index 2ba2cc8a63..7df201d68d 100644 --- a/spec/decisions/ADR-0062-provider-attribute-ownership.md +++ b/spec/decisions/ADR-0062-provider-attribute-ownership.md @@ -53,7 +53,7 @@ a collision. 4. **Scope is one node type.** The JVM registry also chains every `.base` to `metadata.base`, which the core-types provider owns. No other port has that edge, and several shipped providers re-declare `@isAbstract` on their own root with the same meaning. The rule stops at the node-type boundary so all five ports reach the same - verdict, and no shipped provider has to change. + verdict, and the only shipped-provider change is the Java `@description` copy removed under Consequences. 5. **Owner tracking.** The registry records the registering provider per `(type, subType, attribute)`, and per common attribute. The error uses it, and `meta types` shows it From df951d217ab0f6b475ebc462564f0ee7219b0a91 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Wed, 4 Nov 2026 06:09:26 -0500 Subject: [PATCH 6/6] no-mistakes(document): Fixed three stale doc spots: Java manifest comment, provider-composition README code count, stray CHANGELOG blank line. Docs state the full ADR-0062 rule. No unresolved findings. --- CHANGELOG.md | 1 - fixtures/provider-composition-conformance/README.md | 2 +- .../com/metaobjects/registry/RegistryManifest.java | 10 +++++----- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 099a8d9db2..7c6afb0c2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,6 @@ here.** declared rule is now rejected at the model where it used to reach the database. Gated by `fixtures/validation-conformance/cross-field-cases.json` in all five ports. - - **Two providers can no longer claim one attribute name on a node type.** The registry now records which provider registered each attribute and refuses a second provider's claim at composition, in all five ports (TypeScript, Python, C#, Java, Kotlin). A repeat on the same diff --git a/fixtures/provider-composition-conformance/README.md b/fixtures/provider-composition-conformance/README.md index 3e408215e8..8d44e8a12b 100644 --- a/fixtures/provider-composition-conformance/README.md +++ b/fixtures/provider-composition-conformance/README.md @@ -1,6 +1,6 @@ # provider-composition conformance -Five registry/provider error codes are Tier-1 cross-port invariants that the +Six registry/provider error codes are Tier-1 cross-port invariants that the existing `fixtures/conformance/error-*` corpus **cannot** reach: that corpus is metadata-input → error, but these codes are triggered by *how providers are composed and sealed*, not by any metadata document. This small corpus gates them diff --git a/server/java/metadata/src/main/java/com/metaobjects/registry/RegistryManifest.java b/server/java/metadata/src/main/java/com/metaobjects/registry/RegistryManifest.java index 1b1a7d0252..89c236a088 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/registry/RegistryManifest.java +++ b/server/java/metadata/src/main/java/com/metaobjects/registry/RegistryManifest.java @@ -303,11 +303,11 @@ public static List metamodelProviders() { // SP-G Phase1 Units2-3 — manifest emitter exclusions (documented, uniform // across all four ports; see fixtures/registry-conformance/README.md // "EXCLUDED" list + the SP-G divergence analysis buckets C-2/C-3/C-5/B-2): - // - structural keywords (isArray/isAbstract) + the `description` commonAttr - // are NOT per-type attrs. Java DOES register all three as ordinary - // per-type attrs (on metadata.base / field.base, inherited everywhere) — - // so this filter is what drops them from Java's emitter output. They are - // no-ops for TS/C#/Python. `description` stays in the commonAttrs block. + // - structural keywords (isArray/isAbstract) are NOT per-type attrs. Java + // DOES register them as ordinary per-type attrs (on metadata.base / + // field.base, inherited everywhere) — so this filter is what drops them + // from Java's emitter output. They are no-ops for TS/C#/Python. + // `description` lives only in the commonAttrs block (ADR-0062). // - metadata.base is Java's internal inheritance anchor — other ports // register only metadata.root (the deferred inheritsFrom anchor). // - the generic view.* controls are a TS-web-presentation facet. Java