diff --git a/.changeset/7650-metadata-item-reference-canonicalisation.md b/.changeset/7650-metadata-item-reference-canonicalisation.md new file mode 100644 index 0000000000..c56c6aef45 --- /dev/null +++ b/.changeset/7650-metadata-item-reference-canonicalisation.md @@ -0,0 +1,30 @@ +--- +'@object-ui/app-shell': patch +--- + +Canonicalize `reference` / `reference_to` on the BY-NAME object-schema serve path +too (objectui#7650). + +`MetadataProvider` has two paths that hand an object schema to a reader, and only +one of them normalized. `ensureType('object')` — the LIST path — has run +`normalizeSchemaReferenceKeys` over every fetched item since objectui#2407 / PR +#2587. `getItem('object', name)` — the BY-NAME path, behind the published +`useMetadataItem` hook — ran `extractItem`, which unwraps the `{ item }` envelope +and normalizes nothing. + +So which spelling a reader saw depended on **cache order** rather than on the +document: warm (the list had already been fetched) it got the def with both keys +stamped, cold it got whichever single key the producer had stored. A consumer that +reads one key rendered a raw id in the cold case and the relation in the warm one — +the objectui#2407 bug, still reachable through the one door left open. + +This matters because the serve path never parses. objectui#7650 measured that +`ObjectStackAdapter.getObjectSchema` applies exactly two mutations and runs no +`ObjectSchema.parse` at all, so `FieldSchema` strictness gates the WRITE door only: +a document stored before a key was tightened is served back verbatim, forever, and +a host with its own `getObjectSchema` is served straight through. + +`getItem` now applies the same idempotent, in-place stamp for `type === 'object'`, +so a def the list pass already normalized is untouched, and no other metadata type +is affected. Nothing is dropped or overwritten: a spelling the producer set keeps +its own value. diff --git a/packages/app-shell/src/providers/MetadataProvider.tsx b/packages/app-shell/src/providers/MetadataProvider.tsx index 500f0996e9..d9a75c607c 100644 --- a/packages/app-shell/src/providers/MetadataProvider.tsx +++ b/packages/app-shell/src/providers/MetadataProvider.tsx @@ -786,6 +786,23 @@ export function MetadataProvider({ children, adapter, ttlMs = DEFAULT_TTL_MS }: const promise = fetchItem .then((res: unknown) => { const item = extractItem(res); + // objectui#7650 — the BY-NAME serve path needs the same + // canonicalization the LIST path applies in `ensureType` above. + // + // `extractItem` only unwraps the `{ item }` envelope; it normalizes + // nothing. So an object def fetched by name reached readers carrying + // whichever single spelling its producer stored, while the very same + // def arriving through `ensureType` carried both. Which one a reader + // got depended on cache order, not on the document — a cold + // `useMetadataItem('object', name)` took this path, a warm one hit + // the list-populated `byName` entry above and saw the stamped def. + // + // This is a serve path, not an ingestion path in miniature: the + // published `useMetadataItem` hook is exported from + // `@object-ui/app-shell`, so the raw def reaches out-of-repo + // consumers too. Idempotent and in place, so a def the list pass + // already stamped is untouched. + if (type === 'object' && item) normalizeSchemaReferenceKeys(item); if (item) entry.byName.set(name, item); debug(`fetched item type=${type} name=${name} in ${Date.now() - started}ms`); pending!.delete(name); diff --git a/packages/app-shell/src/providers/__tests__/MetadataProvider.itemReferenceKeys-7650.test.tsx b/packages/app-shell/src/providers/__tests__/MetadataProvider.itemReferenceKeys-7650.test.tsx new file mode 100644 index 0000000000..9c7deddf1c --- /dev/null +++ b/packages/app-shell/src/providers/__tests__/MetadataProvider.itemReferenceKeys-7650.test.tsx @@ -0,0 +1,182 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#7650 — the BY-NAME object-schema serve path canonicalizes too. + * + * ## What was broken + * + * `MetadataProvider` has two paths that hand an object schema to a reader, and + * only one of them normalized: + * + * - `ensureType('object')` — the LIST path — ran `normalizeSchemaReferenceKeys` + * over every fetched item (objectui#2407 / PR #2587). + * - `getItem('object', name)` — the BY-NAME path, behind the PUBLISHED + * `useMetadataItem` hook — ran `extractItem`, which only unwraps the + * `{ item }` envelope and normalizes nothing. + * + * So which spelling a reader saw depended on cache order rather than on the + * document: warm (list already fetched) it got the stamped def, cold it got + * whatever single key the producer stored. objectui#7650 measured that the + * serve path never parses, so `FieldSchema` strictness is no evidence a legacy + * spelling cannot reach a consumer — a document stored before the key was + * tightened is served verbatim, forever. + * + * ## What these pins hold + * + * The assertions read the value a CONSUMER gets back from `getItem`, never the + * provider's internals — a re-plumbing that still served an unstamped def by + * some other route would satisfy an internals assertion and violate the card. + * + * The negative pins are the load-bearing half: a normalizer that fired for + * every metadata type, or that overwrote a key the producer had already set, + * would pass every positive assertion here. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { render, waitFor } from '@testing-library/react'; +import { MetadataProvider, useMetadata } from '../MetadataProvider'; + +type Ctx = ReturnType; + +/** + * Serves ONE named document per type from `docs`, and records every by-name + * fetch so the cache-order pin can prove which path answered. + */ +function makeAdapter(docs: Record>, itemCalls: string[]) { + return { + clearCache: vi.fn(), + getClient: () => ({ + meta: { + // The list path serves nothing: every test here starts from a COLD + // by-name cache, which is exactly the state the bug needed. + getItems: (type: string) => Promise.resolve({ type, items: [] }), + getItem: (type: string, name: string) => { + itemCalls.push(`${type}/${name}`); + const doc = docs[`${type}/${name}`]; + // Fresh clone per call — the provider normalizes IN PLACE, and a + // shared fixture object would let one test's stamp leak into the next + // assertion and turn a red into a green. + return Promise.resolve({ item: doc ? structuredClone(doc) : null }); + }, + }, + }), + } as unknown as Parameters[0]['adapter']; +} + +/** Mounts the provider and hands the live context back to the test body. */ +async function withProvider( + docs: Record>, + itemCalls: string[] = [], +): Promise { + let ctx: Ctx | null = null; + function Probe() { + ctx = useMetadata(); + return null; + } + render( + + + , + ); + await waitFor(() => expect(ctx).not.toBeNull()); + return ctx as unknown as Ctx; +} + +describe('MetadataProvider.getItem canonicalizes object schemas (objectui#7650)', () => { + it('stamps `reference_to` on a by-name object def that spells only `reference`', async () => { + const ctx = await withProvider({ + 'object/account': { + name: 'account', + fields: { owner: { type: 'lookup', reference: 'user' } }, + }, + }); + + const item = await ctx.getItem('object', 'account'); + + expect(item.fields.owner.reference).toBe('user'); + expect(item.fields.owner.reference_to).toBe('user'); + }); + + it('stamps `reference` on a by-name object def that spells only the legacy `reference_to`', async () => { + const ctx = await withProvider({ + 'object/contact': { + name: 'contact', + fields: { account_id: { type: 'lookup', reference_to: 'account' } }, + }, + }); + + const item = await ctx.getItem('object', 'contact'); + + expect(item.fields.account_id.reference).toBe('account'); + expect(item.fields.account_id.reference_to).toBe('account'); + }); + + it('normalizes the ARRAY field-container shape the metadata API also serves', async () => { + const ctx = await withProvider({ + 'object/lead': { + name: 'lead', + fields: [{ name: 'owner', type: 'lookup', reference: 'user' }], + }, + }); + + const item = await ctx.getItem('object', 'lead'); + + expect(item.fields[0].reference_to).toBe('user'); + }); + + it('NEGATIVE — leaves a non-`object` metadata type untouched', async () => { + const ctx = await withProvider({ + // A `view` document that happens to carry a `fields` map. Nothing but the + // `object` type is a field-def carrier, and normalizing one would be this + // provider inventing a convention the contract does not declare. + 'view/account_list': { + name: 'account_list', + fields: { owner: { type: 'lookup', reference: 'user' } }, + }, + }); + + const item = await ctx.getItem('view', 'account_list'); + + expect(item.fields.owner.reference).toBe('user'); + expect(item.fields.owner.reference_to).toBeUndefined(); + }); + + it('NEGATIVE — never overwrites a spelling the producer already set', async () => { + const ctx = await withProvider({ + 'object/opportunity': { + name: 'opportunity', + fields: { + // Deliberately inconsistent: if the stamp overwrote rather than + // filled, one of these two values would change. + owner: { type: 'lookup', reference: 'user', reference_to: 'legacy_user' }, + }, + }, + }); + + const item = await ctx.getItem('object', 'opportunity'); + + expect(item.fields.owner.reference).toBe('user'); + expect(item.fields.owner.reference_to).toBe('legacy_user'); + }); + + it('serves the SAME canonical shape on a repeat read, without a second fetch', async () => { + const itemCalls: string[] = []; + const ctx = await withProvider( + { + 'object/account': { + name: 'account', + fields: { owner: { type: 'lookup', reference: 'user' } }, + }, + }, + itemCalls, + ); + + await ctx.getItem('object', 'account'); + const second = await ctx.getItem('object', 'account'); + + // The by-name cache answered the second read (so the normalization must + // have stuck to the cached object, not to a throwaway copy). + expect(itemCalls.filter((c) => c === 'object/account')).toHaveLength(1); + expect(second.fields.owner.reference_to).toBe('user'); + }); +}); diff --git a/packages/core/src/utils/__tests__/reference-keys.legacyWarning-6837.test.ts b/packages/core/src/utils/__tests__/reference-keys.legacyWarning-6837.test.ts index a3f23b257d..3bba179598 100644 --- a/packages/core/src/utils/__tests__/reference-keys.legacyWarning-6837.test.ts +++ b/packages/core/src/utils/__tests__/reference-keys.legacyWarning-6837.test.ts @@ -65,8 +65,9 @@ * ## ⛔ WHAT THIS WARNING DOES NOT COVER * * It fires only where `normalizeSchemaReferenceKeys` runs, which in production - * is exactly the two ingestion choke points — both of which also STAMP the def, - * so it fires where nothing is broken. A hand-written schema served through any + * is exactly the three ingestion choke points (objectui#7650 added the third, + * `MetadataProvider.getItem`) — all of which also STAMP the def, so it fires + * where nothing is broken. A hand-written schema served through any * other `DataSource` reaches a reader raw and warns nothing. The reader-side * diagnostic that would cover that is still open on objectui#6837. */ diff --git a/packages/core/src/utils/reference-keys.ts b/packages/core/src/utils/reference-keys.ts index 9303d05b19..e4a95dc046 100644 --- a/packages/core/src/utils/reference-keys.ts +++ b/packages/core/src/utils/reference-keys.ts @@ -50,14 +50,23 @@ * ## ⛔ WHAT THIS WARNING DOES NOT COVER — state it, do not overclaim it * * This warning fires ONLY where this file runs, and this file runs at exactly - * two production call sites, both of them ingestion choke points: + * three production call sites, all of them ingestion choke points: * - * packages/app-shell/src/providers/MetadataProvider.tsx (metadata type `object`) + * packages/app-shell/src/providers/MetadataProvider.tsx (`ensureType`, metadata type `object`) + * packages/app-shell/src/providers/MetadataProvider.tsx (`getItem`, metadata type `object`) * packages/data-objectstack/src/index.ts (ObjectStackAdapter.getObjectSchema) * - * Both of those STAMP the def, so a def that triggers this warning is also a - * def that still resolves. ⇒ The warning fires precisely where nothing is - * broken. + * All three STAMP the def, so a def that triggers this warning is also a def + * that still resolves. ⇒ The warning fires precisely where nothing is broken. + * + * ⚠️ The third of those is new in objectui#7650 and the count above used to + * read TWO. The old count was true about where this file RAN and false about + * the serve surface it was cited for: `MetadataProvider` normalized on its + * LIST path only, so an object def fetched BY NAME — the published + * `useMetadataItem` hook, and any cold-cache read behind it — was served with + * whichever single spelling the producer stored. ⛔ Do not re-derive this list + * by grepping for the call: derive it from the serve paths that hand an object + * schema to a reader, and check each one calls in. * * ⚠️ The BREAK surface is the complement of that: `getObjectSchema` is a * required member of the published `DataSource` interface and the readers call