diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d09f79179..7c6afb0c2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,29 @@ 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. + 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`, + `common-attr-per-type-after`, `common-attr-per-type-before` 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..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/) | 9 (5 error-shape + 4 compose-load) | ✓ | ✓ | — (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 be13b96183..d2e1383c56 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. All three are machine-enforced — you will hit an error, not a review comment. ### 1. Own vs projected attributes (ADR-0050) @@ -80,6 +80,28 @@ 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`, whether the second provider registers the type or `extend`s it; +- 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 +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 +521,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)`, 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 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..25ae559688 100644 --- a/fixtures/conformance/ERROR-CODES.json +++ b/fixtures/conformance/ERROR-CODES.json @@ -27,7 +27,8 @@ "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).", "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..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 @@ -11,7 +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_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). | ## Corpus shape @@ -112,6 +113,73 @@ 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`. | +| `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). 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 `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"]`, `["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) + +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": [ // 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; `common-attr-owners.json` proves distinct common attr +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 - `ERR_REGISTRY_SEALED` is a **five-port** invariant here. It was previously exercised 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..285e042179 --- /dev/null +++ b/fixtures/provider-composition-conformance/attr-ownership/common-attr-owners.json @@ -0,0 +1,25 @@ +{ + "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", + "owner": "common-owner" + }, + { + "attr": "selfCommonAttr", + "owner": "common-self" + } + ] +} 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-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/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/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/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/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..8debb3c513 100644 --- a/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs +++ b/server/csharp/MetaObjects.Conformance.Tests/ProviderCompositionConformanceTests.cs @@ -116,6 +116,97 @@ 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."), + })); + } + } +} + +/// 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; 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 bool _selfClaim; + + public CommonAttrTestProvider(string id, string[] dependencies, string attr, bool selfClaim = false) + { + Id = id; + Dependencies = dependencies; + _attr = attr; + _selfClaim = selfClaim; + } + + public string Id { get; } + public IReadOnlyList Dependencies { get; } + + public void RegisterTypes(TypeRegistry registry) + { + RegisterCommon(registry); + if (_selfClaim) + { + 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 { private static readonly IReadOnlyDictionary Providers = @@ -131,6 +222,19 @@ 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")), + ["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", 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. @@ -285,4 +389,53 @@ 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 CommonOwnerExpectation(string Attr, string Owner); + + private sealed record OwnershipManifest( + string[] Providers, OwnerExpectation[]? ExpectOwners, CommonOwnerExpectation[]? ExpectCommonOwners); + + 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()); + 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/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..accf05604f 100644 --- a/server/csharp/MetaObjects/Registry.cs +++ b/server/csharp/MetaObjects/Registry.cs @@ -250,6 +250,70 @@ 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; 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. + /// + 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) + { + 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 +361,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 @@ -359,18 +430,50 @@ 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, 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. /// 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) + { + if (claimant is null) break; + string owner = CommonAttrOrigin(attr.Name); + 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) { if (attr.ValueType == SUBTYPE_BASE) { @@ -398,13 +501,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 } _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(); @@ -482,11 +592,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/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 2f930b8ff3..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 @@ -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 @@ -108,6 +111,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,16 +376,21 @@ 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. + 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()); @@ -506,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()); } @@ -545,6 +560,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); @@ -554,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(), @@ -821,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. 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}, @@ -839,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; } @@ -846,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. @@ -893,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. * @@ -1142,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. */ @@ -1154,13 +1202,175 @@ 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) { + 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; + 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 — 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; + 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); + } + 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); + } + } + + /** + * 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} * 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; @@ -1174,39 +1384,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); } /** @@ -1222,10 +1438,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/RegistryManifest.java b/server/java/metadata/src/main/java/com/metaobjects/registry/RegistryManifest.java index 9c97e70d21..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 @@ -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/registry/TypeDefinitionBuilder.java b/server/java/metadata/src/main/java/com/metaobjects/registry/TypeDefinitionBuilder.java index 37c0e714f9..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,8 +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); - if (existing != null && !existing.equals(requirement)) { + // 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) + && namedAttr + && registry != null + && !registry.describeSlotHolder(type, subType, requirement.getName()).isEmpty(); + 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# @@ -452,7 +473,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( @@ -495,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/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 f5d2619491..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 @@ -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; @@ -47,12 +48,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 +111,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 +186,77 @@ 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 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); + 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); if (p != null) return p; @@ -181,6 +265,19 @@ 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; + 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; + 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"); @@ -275,6 +372,71 @@ 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); + + 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(), + registry.getAttrProvider( + expected.get("type").getAsString(), + 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())); + }); + } + } + // ------------------------------------------------------------------ // #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..8823137851 --- /dev/null +++ b/server/java/metadata/src/test/java/com/metaobjects/registry/AttrOwnershipTest.java @@ -0,0 +1,171 @@ +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 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( + 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/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/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..f0ccee113e 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 @@ -146,8 +149,60 @@ 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; 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. + """ + 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] + 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 @@ -218,11 +273,37 @@ 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, 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 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 @@ -239,8 +320,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.""" @@ -317,11 +405,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..e9530d38b1 100644 --- a/server/python/tests/conformance/test_provider_composition_conformance.py +++ b/server/python/tests/conformance/test_provider_composition_conformance.py @@ -119,8 +119,77 @@ 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 + + +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; `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, self_claim: bool = False) -> None: + super().__init__(provider_id, dependencies=deps) + self._attr_name = attr_name + self._self_claim = self_claim + + def register_types(self, registry: TypeRegistry) -> None: # noqa: D401 + 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")]) + + 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", 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") + ), + "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 +294,29 @@ 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.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/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..028526c966 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,93 @@ 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; 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. + */ + 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) ?? []) { + 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 +295,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 { @@ -239,12 +347,39 @@ 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, 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: 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 && 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) { throw new Error( @@ -267,12 +402,18 @@ 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); } } + /** 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 +447,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 +476,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..a8ba5a43c9 100644 --- a/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts +++ b/server/typescript/packages/metadata/test/provider-composition-conformance.test.ts @@ -17,9 +17,9 @@ 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 } 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,114 @@ 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 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.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, + "shadow-base-late": shadowBaseLateProvider, + "shadow-sub-distinct": shadowSubDistinctProvider, + "shadow-self": shadowSelfProvider, "duplicate-x": duplicateXProvider, "duplicate-x-clone": duplicateXCloneProvider, "depends-on-missing": dependsOnMissingProvider, @@ -176,6 +283,9 @@ 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 }[]; + expectCommonOwners?: { attr: string; owner: string }[]; } /** Pull the stable ERR_ code off a caught error, if it carries one. */ @@ -280,3 +390,31 @@ 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) + (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/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..7df201d68d --- /dev/null +++ b/spec/decisions/ADR-0062-provider-attribute-ownership.md @@ -0,0 +1,97 @@ +# 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, 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 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 + 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 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 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 + (`[]` 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 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`, `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 + +- **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