diff --git a/.changeset/meta-list-i18n-bundle-key.md b/.changeset/meta-list-i18n-bundle-key.md new file mode 100644 index 0000000000..9a67bc8f9d --- /dev/null +++ b/.changeset/meta-list-i18n-bundle-key.md @@ -0,0 +1,53 @@ +--- +"@objectstack/metadata-protocol": patch +"@objectstack/metadata-core": patch +"@objectstack/objectql": patch +--- + +fix(metadata-protocol): keep every i18n bundle member through the `/meta` list merge (#7774) + +#7730 taught the `SchemaRegistry` that an `email_template`'s identity is +`(name, locale)`, so `listItems('email_template')` returns every member of a +declared i18n bundle. `GET /meta/` then merges that listing with two +higher layers, and both merges keyed by `(package, name)` with no +discriminator — so the bundle survived registration only to collapse one layer +later, and the list served a single locale. + +**Both merges now key on the pair.** `metaItemKey` takes an optional third +component and `mergePackageAwareOverlay` buckets per slot rather than per name; +both derive the value from the shared discriminator table, and both are +byte-identical for a type that declares no discriminator — which is every type +except `email_template` today. + +- **The MetadataService merge** is the path the issue named: with a `metadata` + service installed and answering non-empty for the type, the second member's + `Map.set` overwrote the first. +- **The `sys_metadata` overlay merge** was predicted to need no change, on the + ground that overlay rows are unique on `type+name+organization_id+package_id` + and carry no locale column. That is true of the rows and beside the point: + the base of that merge is the registry's bundle, so bucketing by bare name + dropped a locale as soon as a single overlay row existed for the type — and + the row that survived was the overlay body, whichever member it customizes. + An overlay (or a draft preview) now lands on its own locale member and the + rest of the bundle is served untouched. Across the env-wide and org tiers, + rows that customize different members are likewise two slots instead of one; + org-over-env precedence is unchanged within a member. + +**The discriminator table moved to `@objectstack/metadata-core`.** +`ITEM_KEY_DISCRIMINATORS` was declared in `@objectstack/objectql`'s +`registry.ts`, and `@objectstack/objectql` depends on +`@objectstack/metadata-protocol`, so the protocol package could not import it +without closing a dependency cycle. metadata-core is the package both already +depend on and depends on neither — the same criterion that sank the engine +write-verb dispatch predicates (#5619) and the audit-field governance table +(#4513) there. **No public surface changes:** `registry.ts` re-exports +`ITEM_KEY_DISCRIMINATORS` under its original name from its original module, so +every existing import keeps working; `@objectstack/metadata-core` gains it plus +`readDiscriminatorValue` / `itemDiscriminator` as additive exports. The +registry's storage-key *format* (`name@` composite keys and their +parser) deliberately did not move — it encodes the registry's own Map keys, +which no other package reads. + +For an app this is Studio's metadata list and `GET /meta/email_template` +showing both the en-US and the zh-CN copy of a template instead of whichever +one the merge happened to keep. diff --git a/packages/metadata-core/src/index.ts b/packages/metadata-core/src/index.ts index 863984b96f..2163601e84 100644 --- a/packages/metadata-core/src/index.ts +++ b/packages/metadata-core/src/index.ts @@ -53,3 +53,13 @@ export * from './injected-system-columns.js'; // outlet, or the mask is decoration") is only true if they all run the same // projection rather than a copy each. export * from './object-schema-fls.js'; + +// [#7730 / #7774] The i18n-bundle DISCRIMINATOR table — which metadata types +// are identified by `(name, )` rather than by `name` alone — sunk here +// by the same criterion as the governance table above. `@objectstack/objectql` +// (the SchemaRegistry, #7730) and `@objectstack/metadata-protocol` (the +// unscoped `/meta` list merge, #7774) both key metadata by name, objectql +// depends on metadata-protocol, and a bundle that survives one layer's key but +// not the other's is still collapsed. `objectql` re-exports +// `ITEM_KEY_DISCRIMINATORS` from `registry.ts`, so its surface is unchanged. +export * from './item-key-discriminators.js'; diff --git a/packages/metadata-core/src/item-key-discriminators.ts b/packages/metadata-core/src/item-key-discriminators.ts new file mode 100644 index 0000000000..ede23d754e --- /dev/null +++ b/packages/metadata-core/src/item-key-discriminators.ts @@ -0,0 +1,92 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#7730 / #7774] i18n bundles — metadata types whose IDENTITY is a pair. + * + * Most metadata types are identified by `name` alone. `email_template` + * declares otherwise: `EmailTemplateDefinitionSchema` states that "multiple + * rows with the same `name` but different `locale` form an i18n bundle; the + * service picks the best match for the recipient's locale, falling back to + * `en-US`" (`packages/spec/src/system/email-template.zod.ts`), and its header + * says a template "is resolved by `(name, locale)`". + * + * Every layer that keys metadata by `name` therefore has to agree on ONE + * answer to "what else is part of this type's identity?", or a bundle survives + * one layer and collapses at the next: + * + * - `@objectstack/objectql`'s `SchemaRegistry` keys its item collections + * (`registerItem` / `getItem` / `listItems`) — fixed by #7730; + * - `@objectstack/metadata-protocol`'s unscoped `/meta/` list merge + * keys the sys_metadata overlay and MetadataService merges (`metaItemKey`, + * `mergePackageAwareOverlay`) — fixed by #7774, which is why the table + * lives HERE rather than in the registry that first needed it. + * + * `@objectstack/objectql` depends on `@objectstack/metadata-protocol`, so the + * protocol package cannot import the table from the registry — the reverse + * import would close a cycle turbo rejects. This package is the one both sides + * already depend on and it depends on neither, the same criterion that sank + * the engine write-verb dispatch predicates (#5619), the audit-field + * governance table (#4513) and the injected-system-column definitions (#6562) + * here. `objectql` re-exports `ITEM_KEY_DISCRIMINATORS` from `registry.ts`, so + * its public surface is unchanged. + * + * What deliberately did NOT move: the registry's storage-key FORMAT + * (`BUNDLE_KEY_SEPARATOR`, `withDiscriminator`, `bundleBaseKey`, + * `collectBundle`). Those encode a discriminator into the registry's own + * `:` Map keys and parse it back out again; the protocol + * layer builds its own NUL-separated merge key and never parses a registry + * key, so it needs the IDENTITY question answered — the table and + * {@link itemDiscriminator} — not the encoding. Moving the encoding too would + * have published a registry-internal key format as a cross-package contract. + */ + +/** + * Metadata types whose identity is `(name, )`. + * + * The discriminator is declared PER TYPE rather than duck-typed off a `locale` + * property, because the key computation is generic to every registered + * metadata type: reading whatever `item.locale` happened to be set would + * silently re-key any other type that grows a locale-ish field, which is a much + * larger contract change than the one this table makes. `email_template` is the + * only type whose schema declares a top-level `locale` that is part of its + * identity. + * + * `canonical` is the bundle member a bare-name read resolves to, and the value + * a member that declares no discriminator is keyed as. It mirrors the schema's + * own `locale` default and `sendTemplate`'s documented fallback; + * `registry-i18n-bundle-key.test.ts` (objectql) pins the two together so this + * copy cannot drift from the spec. + */ +export const ITEM_KEY_DISCRIMINATORS: Readonly> = { + email_template: { field: 'locale', canonical: 'en-US' }, +}; + +/** + * The discriminator value an item declares, trimmed; `''` when it declares + * none. `content[field]` is consulted too, because a stored metadata body may + * carry the definition nested under `content`. + */ +export function readDiscriminatorValue(item: unknown, field: string): string { + const holder = item as Record | null | undefined; + const raw = holder?.[field] ?? holder?.content?.[field]; + return typeof raw === 'string' ? raw.trim() : ''; +} + +/** + * The canonical-normalized discriminator of `item` under `type`, or + * `undefined` when `type` declares none. + * + * `undefined` is the load-bearing return: every caller appends this to a key + * ONLY when it is defined, so an undiscriminated type's key stays + * byte-identical to what it was before this table existed. A discriminated + * item that declares no value is keyed as the `canonical` member — the same + * row a bare-name read resolves to — so the bundle-blind and bundle-aware + * answers agree for a single-member "bundle". + * + * @param type Singular metadata type name (`'email_template'`, not the plural). + */ +export function itemDiscriminator(type: string, item: unknown): string | undefined { + const disc = ITEM_KEY_DISCRIMINATORS[type]; + if (!disc) return undefined; + return readDiscriminatorValue(item, disc.field) || disc.canonical; +} diff --git a/packages/metadata-protocol/src/protocol.i18n-bundle-list-merge.test.ts b/packages/metadata-protocol/src/protocol.i18n-bundle-list-merge.test.ts new file mode 100644 index 0000000000..62614bca8f --- /dev/null +++ b/packages/metadata-protocol/src/protocol.i18n-bundle-list-merge.test.ts @@ -0,0 +1,420 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#7774] `GET /meta/` keeps every member of an i18n bundle. + * + * --------------------------------------------------------------------------- + * The gap this file pins + * --------------------------------------------------------------------------- + * `EmailTemplateDefinitionSchema` declares that "multiple rows with the same + * `name` but different `locale` form an i18n bundle" and that a template "is + * resolved by `(name, locale)`". #7730 taught the `SchemaRegistry` that key, so + * `listItems('email_template')` returns EVERY member. `getMetaItems` then + * merges that listing with two higher layers, and both merges keyed by + * `(package, name)` with no discriminator: + * + * • the MetadataService merge — `metaItemKey` in the `itemMap` loops. The + * second member's `Map.set` overwrote the first, so the list served one + * locale. This is the path the card names. + * • the `sys_metadata` overlay merge — `mergePackageAwareOverlay`, which + * bucketed by bare `name` and emitted one row per `(bucket, package)`. + * The card predicted this half needed NO change, on the (correct) ground + * that overlay ROWS are unique on `type+name+organization_id+package_id` + * and carry no locale column. The rows were never the problem: the BASE + * items are, and they are the registry's bundle. One unrelated overlay row + * for the type was enough to drop a locale — and the row that survived was + * the overlay body, whichever member it actually customizes. + * + * --------------------------------------------------------------------------- + * Why there was no coverage before + * --------------------------------------------------------------------------- + * Both merge blocks are conditional. The MetadataService block runs only when + * a `metadata` service is installed AND answers non-empty for the type; the + * overlay block runs only when `sys_metadata` yields at least one active row. + * A harness that omits either passes against the bug, which is why every case + * below installs the precondition it needs and the first `describe` asserts + * the precondition itself. + * + * --------------------------------------------------------------------------- + * Reverse verification, direction predicted BEFORE running + * --------------------------------------------------------------------------- + * Restoring `metaItemKey` to its two-component `origin/main` body (dropping + * the discriminator argument) must turn the bundle cases red naming the + * collapse — one row where two were served — and must leave the + * byte-identical-key guards GREEN, because those assert the behaviour of + * UNDISCRIMINATED types, which the discriminator never touches. A guard that + * goes red under the revert would mean this change altered a key it promised + * not to. Measured in the PR body. + */ +import { describe, expect, it } from 'vitest'; +// [#7774] The identity table's home is `@objectstack/metadata-core`, not the +// `SchemaRegistry` that first needed it: `@objectstack/objectql` depends on +// THIS package, so importing the registry's copy would close a cycle. +import { ITEM_KEY_DISCRIMINATORS } from '@objectstack/metadata-core'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +const PKG = 'com.acme.crm'; + +/** A spec-shaped email template body; `locale` is supplied per case. */ +function tpl(name: string, locale: string | undefined, extra: Record = {}) { + return { + name, + label: `Label ${locale ?? '(default)'}`, + subject: `Subject ${locale ?? '(default)'}`, + bodyHtml: `

${locale ?? '(default)'}

`, + ...(locale === undefined ? {} : { locale }), + ...extra, + }; +} + +interface Row { + id: string; + type: string; + name: string; + organization_id: string | null; + package_id: string | null; + state: string; + metadata: string; +} + +function row( + partial: Omit, 'metadata'> & { name: string; type: string; metadata: unknown }, +): Row { + return { + id: `row_${partial.type}_${partial.name}_${partial.organization_id ?? 'env'}`, + organization_id: null, + package_id: null, + state: 'active', + ...partial, + metadata: JSON.stringify(partial.metadata), + }; +} + +/** + * The registry stub answers `listItems` with the bundle exactly as the real + * `SchemaRegistry` does since #7730 — every member, each tagged with its + * owning `_packageId`. It deliberately omits `getArtifactItem`, so + * `lookupArtifactItem` takes its documented partial-mock fallback. + */ +function makeEngine(opts: { items?: unknown[]; rows?: Row[] } = {}) { + const registered: Array<{ type: string; name: string }> = []; + return { + registered, + engine: { + registry: { + listItems: (type: string, packageId?: string) => { + const all = (opts.items ?? []) as any[]; + const forType = all.filter((i) => i.__type === type); + return (packageId ? forType.filter((i) => i._packageId === packageId) : forType) + // Strip the harness-only tag so the merge sees a real body. + .map(({ __type, ...rest }) => rest); + }, + isPackageDisabled: () => false, + registerItem: (type: string, item: any) => { registered.push({ type, name: item?.name }); }, + getItem: () => undefined, + applyNavContributions: (app: unknown) => app, + }, + async find(table: string, q: { where: Record }) { + if (table !== 'sys_metadata') return []; + return (opts.rows ?? []).filter((r) => { + for (const [k, v] of Object.entries(q.where)) { + if (v === undefined) continue; + if ((r as any)[k] !== v) return false; + } + return true; + }); + }, + async findOne() { return null; }, + } as any, + }; +} + +/** A `metadata` service that answers `list(type)` from a fixed table. */ +function servicesWithMetadata(byType: Record) { + return () => new Map([ + ['metadata', { list: async (type: string) => byType[type] ?? [] }], + ]); +} + +/** Every `(name, locale)` pair the list served, sorted for stable compare. */ +function pairs(items: any[]): string[] { + return items.map((i) => `${i.name}@${i.locale ?? '(none)'}`).sort(); +} + +describe('[#7774] the unscoped /meta list keeps every i18n bundle member', () => { + describe('the preconditions this defect hides behind', () => { + it('only `email_template` declares a discriminator today', () => { + // A guard on the blast radius, mirroring objectql's own pin: every + // other type's merge key is byte-identical to `origin/main`'s + // precisely because it is absent from this table. + expect(Object.keys(ITEM_KEY_DISCRIMINATORS)).toEqual(['email_template']); + }); + + it('serves the bundle when NEITHER merge block runs — the case a naive test writes', () => { + // No `metadata` service and no sys_metadata row: both merges are + // skipped and the registry listing passes straight through. This + // case passes on `origin/main` too, and that is the point — it is + // the shape of harness that made the defect invisible. + const { engine } = makeEngine({ + items: [ + { __type: 'email_template', ...tpl('auth.welcome', 'en-US'), _packageId: PKG }, + { __type: 'email_template', ...tpl('auth.welcome', 'zh-CN'), _packageId: PKG }, + ], + }); + const protocol = new ObjectStackProtocolImplementation(engine); + return protocol.getMetaItems({ type: 'email_template' }).then((res) => { + expect(pairs(res.items as any[])).toEqual(['auth.welcome@en-US', 'auth.welcome@zh-CN']); + }); + }); + }); + + describe('the MetadataService merge (the path the card names)', () => { + it('keeps both locales when a metadata service answers non-empty', async () => { + const { engine } = makeEngine({ + items: [ + { __type: 'email_template', ...tpl('auth.welcome', 'en-US'), _packageId: PKG }, + { __type: 'email_template', ...tpl('auth.welcome', 'zh-CN'), _packageId: PKG }, + ], + }); + const protocol = new ObjectStackProtocolImplementation( + engine, + // Non-empty for the type — without this the whole block is + // skipped and the bug cannot reproduce. + servicesWithMetadata({ email_template: [{ ...tpl('billing.invoice', 'en-US'), _packageId: PKG }] }), + ); + + const res = await protocol.getMetaItems({ type: 'email_template' }); + expect(pairs(res.items as any[])).toEqual([ + 'auth.welcome@en-US', + 'auth.welcome@zh-CN', + 'billing.invoice@en-US', + ]); + }); + + it('lets the service contribute a locale the registry does not ship', async () => { + // The baseline direction of the same key: a runtime-registered + // member of an existing bundle is an ADDITION, not a duplicate. + const { engine } = makeEngine({ + items: [{ __type: 'email_template', ...tpl('auth.welcome', 'en-US'), _packageId: PKG }], + }); + const protocol = new ObjectStackProtocolImplementation( + engine, + servicesWithMetadata({ + email_template: [ + { ...tpl('auth.welcome', 'en-US'), _packageId: PKG, _fromService: true }, + { ...tpl('auth.welcome', 'ja-JP'), _packageId: PKG, _fromService: true }, + ], + }), + ); + + const res = await protocol.getMetaItems({ type: 'email_template' }); + expect(pairs(res.items as any[])).toEqual(['auth.welcome@en-US', 'auth.welcome@ja-JP']); + // The registry's own en-US still wins its slot — the service is a + // baseline under the registry, never over it. + const enUs = (res.items as any[]).find((i) => i.locale === 'en-US'); + expect(enUs._fromService).toBeUndefined(); + }); + }); + + describe('the sys_metadata overlay merge (the premise the card said needed no change)', () => { + it('keeps every registry member when ONE overlay row exists for the type', async () => { + // The overlay customizes zh-CN. en-US is not customized and must + // still be served — bucketed by bare name it was not. + const { engine } = makeEngine({ + items: [ + { __type: 'email_template', ...tpl('auth.welcome', 'en-US'), _packageId: PKG }, + { __type: 'email_template', ...tpl('auth.welcome', 'zh-CN'), _packageId: PKG }, + ], + rows: [row({ + type: 'email_template', + name: 'auth.welcome', + package_id: PKG, + metadata: tpl('auth.welcome', 'zh-CN', { subject: 'CUSTOMIZED zh-CN' }), + })], + }); + const protocol = new ObjectStackProtocolImplementation(engine); + + const res = await protocol.getMetaItems({ type: 'email_template' }); + expect(pairs(res.items as any[])).toEqual(['auth.welcome@en-US', 'auth.welcome@zh-CN']); + const byLocale = Object.fromEntries((res.items as any[]).map((i) => [i.locale, i])); + // …and the overlay landed on ITS OWN member, not on the bundle. + expect(byLocale['zh-CN'].subject).toBe('CUSTOMIZED zh-CN'); + expect(byLocale['en-US'].subject).toBe('Subject en-US'); + }); + + it('does not let an overlay of one locale displace a sibling from another package', async () => { + const { engine } = makeEngine({ + items: [ + { __type: 'email_template', ...tpl('auth.welcome', 'en-US'), _packageId: PKG }, + { __type: 'email_template', ...tpl('auth.welcome', 'zh-CN'), _packageId: 'com.acme.hr' }, + ], + rows: [row({ + type: 'email_template', + name: 'auth.welcome', + package_id: PKG, + metadata: tpl('auth.welcome', 'en-US', { subject: 'CUSTOMIZED en-US' }), + })], + }); + const protocol = new ObjectStackProtocolImplementation(engine); + + const res = await protocol.getMetaItems({ type: 'email_template' }); + expect((res.items as any[]).length).toBe(2); + const byPkg = Object.fromEntries((res.items as any[]).map((i) => [i._packageId, i])); + expect(byPkg[PKG].subject).toBe('CUSTOMIZED en-US'); + expect(byPkg['com.acme.hr'].locale).toBe('zh-CN'); + }); + + it('keys an env-wide row and an org row of DIFFERENT locales as different slots', async () => { + // The store's unique index is `(type, name, organization_id, + // package_id)`, so an org cannot hold two rows differing only by + // body locale — but the env-wide tier and the org tier can, and + // keying them together made the org's row displace the env-wide + // one. Precedence within a member is unchanged; see the next case. + const { engine } = makeEngine({ + items: [ + { __type: 'email_template', ...tpl('auth.welcome', 'en-US'), _packageId: PKG }, + { __type: 'email_template', ...tpl('auth.welcome', 'zh-CN'), _packageId: PKG }, + ], + rows: [ + row({ + type: 'email_template', name: 'auth.welcome', package_id: PKG, + organization_id: null, + metadata: tpl('auth.welcome', 'en-US', { subject: 'ENV en-US' }), + }), + row({ + type: 'email_template', name: 'auth.welcome', package_id: PKG, + organization_id: 'org_1', + metadata: tpl('auth.welcome', 'zh-CN', { subject: 'ORG zh-CN' }), + }), + ], + }); + const protocol = new ObjectStackProtocolImplementation(engine); + + const res = await protocol.getMetaItems({ type: 'email_template', organizationId: 'org_1' }); + const byLocale = Object.fromEntries((res.items as any[]).map((i) => [i.locale, i])); + expect(Object.keys(byLocale).sort()).toEqual(['en-US', 'zh-CN']); + expect(byLocale['en-US'].subject).toBe('ENV en-US'); + expect(byLocale['zh-CN'].subject).toBe('ORG zh-CN'); + }); + + it('still lets an org row override the env-wide row of the SAME locale', async () => { + // ADR-0005 org-over-env precedence, unchanged where it was ever + // meaningful: two rows of one member still resolve to one row. + const { engine } = makeEngine({ + items: [{ __type: 'email_template', ...tpl('auth.welcome', 'en-US'), _packageId: PKG }], + rows: [ + row({ + type: 'email_template', name: 'auth.welcome', package_id: PKG, + organization_id: null, + metadata: tpl('auth.welcome', 'en-US', { subject: 'ENV en-US' }), + }), + row({ + type: 'email_template', name: 'auth.welcome', package_id: PKG, + organization_id: 'org_1', + metadata: tpl('auth.welcome', 'en-US', { subject: 'ORG en-US' }), + }), + ], + }); + const protocol = new ObjectStackProtocolImplementation(engine); + + const res = await protocol.getMetaItems({ type: 'email_template', organizationId: 'org_1' }); + expect((res.items as any[]).length).toBe(1); + expect((res.items as any[])[0].subject).toBe('ORG en-US'); + }); + + it('treats a member that declares no locale as the canonical member', async () => { + // `locale` has a schema default of `en-US`; an author who omits it + // is authoring the canonical member, and an en-US overlay must + // land on it rather than beside it. + const { engine } = makeEngine({ + items: [{ __type: 'email_template', ...tpl('auth.welcome', undefined), _packageId: PKG }], + rows: [row({ + type: 'email_template', name: 'auth.welcome', package_id: PKG, + metadata: tpl('auth.welcome', 'en-US', { subject: 'CUSTOMIZED en-US' }), + })], + }); + const protocol = new ObjectStackProtocolImplementation(engine); + + const res = await protocol.getMetaItems({ type: 'email_template' }); + expect((res.items as any[]).length).toBe(1); + expect((res.items as any[])[0].subject).toBe('CUSTOMIZED en-US'); + }); + }); + + describe('the draft-preview merge', () => { + it('previews a draft of one locale without dropping its siblings', async () => { + const { engine } = makeEngine({ + items: [ + { __type: 'email_template', ...tpl('auth.welcome', 'en-US'), _packageId: PKG }, + { __type: 'email_template', ...tpl('auth.welcome', 'zh-CN'), _packageId: PKG }, + ], + rows: [row({ + type: 'email_template', name: 'auth.welcome', package_id: PKG, state: 'draft', + metadata: tpl('auth.welcome', 'zh-CN', { subject: 'DRAFT zh-CN' }), + })], + }); + const protocol = new ObjectStackProtocolImplementation(engine); + + const res = await protocol.getMetaItems({ type: 'email_template', previewDrafts: true }); + const byLocale = Object.fromEntries((res.items as any[]).map((i) => [i.locale, i])); + expect(Object.keys(byLocale).sort()).toEqual(['en-US', 'zh-CN']); + expect(byLocale['zh-CN'].subject).toBe('DRAFT zh-CN'); + expect(byLocale['zh-CN']._draft).toBe(true); + expect(byLocale['en-US']._draft).toBeUndefined(); + }); + }); + + describe('undiscriminated types keep a byte-identical key', () => { + it('ADR-0048: two packages shipping `page/home` stay two rows through the service merge', async () => { + const { engine } = makeEngine({ + items: [ + { __type: 'page', name: 'home', _packageId: PKG }, + { __type: 'page', name: 'home', _packageId: 'com.acme.hr' }, + ], + }); + const protocol = new ObjectStackProtocolImplementation( + engine, + servicesWithMetadata({ page: [{ name: 'about', _packageId: PKG }] }), + ); + + const res = await protocol.getMetaItems({ type: 'page' }); + expect((res.items as any[]).map((i) => `${i._packageId}/${i.name}`).sort()) + .toEqual([`${PKG}/about`, `${PKG}/home`, 'com.acme.hr/home']); + }); + + it('ADR-0048: an overlay row still collapses onto its own package slot', async () => { + const { engine } = makeEngine({ + items: [ + { __type: 'page', name: 'home', _packageId: PKG, title: 'code' }, + { __type: 'page', name: 'home', _packageId: 'com.acme.hr', title: 'code' }, + ], + rows: [row({ type: 'page', name: 'home', package_id: PKG, metadata: { name: 'home', title: 'overlay' } })], + }); + const protocol = new ObjectStackProtocolImplementation(engine); + + const res = await protocol.getMetaItems({ type: 'page' }); + expect((res.items as any[]).length).toBe(2); + const byPkg = Object.fromEntries((res.items as any[]).map((i) => [i._packageId, i.title])); + expect(byPkg).toEqual({ [PKG]: 'overlay', 'com.acme.hr': 'code' }); + }); + + it('a same-name page in one package is still ONE row, not two', async () => { + // The complement of the bundle cases: without a discriminator two + // same-name rows of one package are a genuine collision and must + // still resolve to one row. A slot key that leaked into every type + // would break exactly this. + const { engine } = makeEngine({ + items: [ + { __type: 'page', name: 'home', _packageId: PKG, title: 'first' }, + { __type: 'page', name: 'home', _packageId: PKG, title: 'second' }, + ], + rows: [row({ type: 'page', name: 'home', package_id: PKG, metadata: { name: 'home', title: 'overlay' } })], + }); + const protocol = new ObjectStackProtocolImplementation(engine); + + const res = await protocol.getMetaItems({ type: 'page' }); + expect((res.items as any[]).length).toBe(1); + expect((res.items as any[])[0].title).toBe('overlay'); + }); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index c84c059af2..77d9f58ccc 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -28,6 +28,11 @@ import { // definition table — see {@link governServedItem} / {@link stripServedSystemColumns}. applyInjectedSystemColumns, stripInjectedSystemColumns, + // [#7774] The i18n-bundle identity table (#7730) and its reader. A type + // listed here is identified by `(name, )`, so every merge in the + // unscoped `/meta` list keys on the pair — see {@link metaItemKey}. + ITEM_KEY_DISCRIMINATORS, + itemDiscriminator, type MetadataItem, } from '@objectstack/metadata-core'; // [#5532] One vocabulary of "which driver read errors are benign", shared with @@ -984,9 +989,52 @@ function mergeArtifactProtection(item: unknown, artifactItem: unknown): unknown * `${packageId}:${name}` keys. Any list-merge that deduplicates by bare `name` * collapses the two packages' rows into one (last-write-wins), which is the * bug this key closes. A `NUL` separator keeps names containing `:` unambiguous. + * + * [#7774] `discriminator` is a third component, and it is OPTIONAL on purpose: + * omitting it produces the exact two-component string this function has always + * produced, so every type identified by `(package, name)` keeps a + * byte-identical key and this change's blast radius is provable rather than + * argued. It is supplied only for a type listed in `ITEM_KEY_DISCRIMINATORS` + * (`email_template` today), whose identity the spec declares as + * `(name, locale)` — an i18n bundle. Without it the second locale's + * `Map.set` overwrote the first and `GET /meta/email_template` served one + * member of a bundle it had every member of. + */ +function metaItemKey( + packageId: string | null | undefined, + name: unknown, + discriminator?: string, +): string { + const base = `${packageId ?? ''}\u0000${String(name)}`; + return discriminator === undefined ? base : `${base}\u0000${discriminator}`; +} + +/** + * [#7774] The bundle discriminator a STORED `sys_metadata` row declares, read + * off the row's serialized body. + * + * The row's own COLUMNS cannot answer this: `sys_metadata`'s overlay + * uniqueness is `(type, name, organization_id, package_id)` — declared on + * `idx_sys_metadata_overlay_active` in `sys-metadata.object.ts` — and the + * table has no locale column at all. An `email_template` overlay's locale + * lives inside the `metadata` JSON payload, which is where its identity is. + * So a row-level merge keying on columns alone cannot tell two members of one + * bundle apart, and the body has to be consulted. + * + * The parse is skipped entirely for an undiscriminated type (all but one + * today): `undefined` comes back before any JSON work, so this costs nothing + * on the paths it does not serve. A body that fails to parse falls back to the + * canonical member instead of throwing — the caller a few lines on parses it + * again for real, and that failure is the one that should surface. */ -function metaItemKey(packageId: string | null | undefined, name: unknown): string { - return `${packageId ?? ''}\u0000${String(name)}`; +function storedRowDiscriminator(type: string, record: unknown): string | undefined { + if (!ITEM_KEY_DISCRIMINATORS[type]) return undefined; + const raw = (record as { metadata?: unknown } | null | undefined)?.metadata; + let body: unknown = raw; + if (typeof raw === 'string') { + try { body = JSON.parse(raw); } catch { body = undefined; } + } + return itemDiscriminator(type, body); } /** @@ -1013,25 +1061,47 @@ function metaItemKey(packageId: string | null | undefined, name: unknown): strin * `transform(data, prev)` runs on each `records` body before it enters the * merge (view-identity healing, draft tagging); `prev` is the base row it * shadows at the same slot (or any same-name base row), else undefined. + * + * [#7774] The bucket is per SLOT, not per name, and for a type in + * `ITEM_KEY_DISCRIMINATORS` the slot is `(name, discriminator)`. This is the + * half of the collapse that #7774's card predicted would need no change, and + * it needed one: the card reasoned about the OVERLAY rows (correctly — they + * are unique on `type+name+organization_id+package_id` and carry no locale + * column), but the rows are only the higher layer. `baseItems` is the + * SchemaRegistry's listing, and since #7730 that listing carries every member + * of a bundle. Bucketed by bare name, two members of one bundle landed in one + * bucket and the loop below emitted exactly one row per `(bucket, package)` — + * so a single unrelated overlay row for the type was enough to drop a locale, + * and the surviving row was the overlay body regardless of which member it + * actually customizes. Slot-keyed, the overlay lands on its OWN member and the + * others are served untouched. An undiscriminated type's slot is its name, so + * its buckets are unchanged. + * + * @param type Canonical (singular) metadata type of every row being merged. */ function mergePackageAwareOverlay( + type: string, baseItems: unknown[], records: Array<{ data: unknown; packageId: string | undefined }>, transform?: (data: any, prev: any) => any, ): unknown[] { - // Per-name, layer-ordered contributions; `pkg: undefined` = package-less. + // Per-SLOT, layer-ordered contributions; `pkg: undefined` = package-less. const buckets = new Map>(); - const order: string[] = []; // first-seen name order → stable output - const push = (name: string, pkg: string | undefined, item: any) => { - let list = buckets.get(name); - if (!list) { buckets.set(name, (list = [])); order.push(name); } + const order: string[] = []; // first-seen slot order → stable output + const slotOf = (item: unknown, name: unknown): string => { + const disc = itemDiscriminator(type, item); + return disc === undefined ? String(name) : `${String(name)}\u0000${disc}`; + }; + const push = (slot: string, pkg: string | undefined, item: any) => { + let list = buckets.get(slot); + if (!list) { buckets.set(slot, (list = [])); order.push(slot); } list.push({ pkg, item }); }; for (const raw of baseItems) { const item = raw as any; if (item && typeof item === 'object' && 'name' in item) { - push(item.name, (item._packageId ?? undefined) as string | undefined, item); + push(slotOf(item, item.name), (item._packageId ?? undefined) as string | undefined, item); } } for (const { data, packageId } of records) { @@ -1039,19 +1109,20 @@ function mergePackageAwareOverlay( if (!(body && typeof body === 'object' && 'name' in body)) continue; // The base row this record shadows at its own slot (for view-identity // healing): a same-package row, else a package-less one, else any - // same-name row it stands in for. - const list = buckets.get(body.name); + // same-slot row it stands in for. + const slot = slotOf(body, body.name); + const list = buckets.get(slot); const prev = list ? (list.find((c) => c.pkg === packageId)?.item ?? list.find((c) => c.pkg === undefined)?.item ?? list[0]?.item) : undefined; - push(body.name, packageId, transform ? transform(body, prev) : body); + push(slot, packageId, transform ? transform(body, prev) : body); } const out: unknown[] = []; - for (const name of order) { - const list = buckets.get(name)!; + for (const slot of order) { + const list = buckets.get(slot)!; const reals = Array.from(new Set(list.filter((c) => c.pkg !== undefined).map((c) => c.pkg))); if (reals.length === 0) { out.push(list[list.length - 1].item); // latest package-less row wins @@ -4203,9 +4274,23 @@ export class ObjectStackProtocolImplementation implements // ADR-0048 (#1828) — key by (package, name), not bare name, so a // package A row and a package B row of the same name do not // collapse; org-over-env precedence still holds within each slot. + // + // [#7774] …and for a bundled type the slot is `(package, name, + // locale)`. Within ONE org this changes nothing — the store's own + // unique index is `(type, name, organization_id, package_id)`, so + // an org cannot hold two rows that differ only by body locale. + // Across the two tiers it can: an env-wide row and this org's row + // may customize DIFFERENT members of one bundle, and keying them + // together made the org's zh-CN row silently displace the + // env-wide en-US one. Precedence is unchanged where it was ever + // meaningful — an org row still overrides the env-wide row of the + // same member — and an undiscriminated type keeps a + // byte-identical key. const mergedMap = new Map(); - for (const r of envWideRecords) mergedMap.set(metaItemKey(r.package_id, r.name), r); - for (const r of orgRecords) mergedMap.set(metaItemKey(r.package_id, r.name), r); + const rowKey = (r: any): string => + metaItemKey(r.package_id, r.name, storedRowDiscriminator(request.type, r)); + for (const r of envWideRecords) mergedMap.set(rowKey(r), r); + for (const r of orgRecords) mergedMap.set(rowKey(r), r); const records = Array.from(mergedMap.values()); if (records && records.length > 0) { const isView = (PLURAL_TO_SINGULAR[request.type] ?? request.type) === 'view'; @@ -4239,8 +4324,11 @@ export class ObjectStackProtocolImplementation implements // not collapsed to one. #2555 — heal identity-less view overlays // from the entry they shadow (a raw-config row would otherwise // drop viewKind/object and vanish the view from switcher/list - // consumers); the overlay's own fields still win. - items = mergePackageAwareOverlay(items, overlays, (data, prev) => { + // consumers); the overlay's own fields still win. [#7774] The + // merge slot carries the bundle discriminator, so an + // `email_template` overlay lands on its own locale member + // instead of flattening every member of the bundle onto it. + items = mergePackageAwareOverlay(request.type, items, overlays, (data, prev) => { if (isView && data && typeof data === 'object') { const patch = viewIdentityPatch(data as Record, prev); if (patch) Object.assign(data as Record, patch); @@ -4325,7 +4413,10 @@ export class ObjectStackProtocolImplementation implements } return { data, packageId: recPkg }; }); - items = mergePackageAwareOverlay(items, drafts, (data) => { + // [#7774] Same bundle slot as the active merge above — a + // draft of one locale must preview over that locale, not + // over the whole bundle. + items = mergePackageAwareOverlay(request.type, items, drafts, (data) => { if (data && typeof data === 'object') (data as any)._draft = true; return data; }); @@ -4356,11 +4447,27 @@ export class ObjectStackProtocolImplementation implements // Merge, avoiding duplicates. ADR-0048 (#1828) — key by // (package, name), not bare name, so a runtime item from one // package does not collapse a same-name item from another. + // + // [#7774] …and by `(package, name, locale)` for a type + // whose identity the spec declares as a pair. This loop is + // where the i18n bundle actually died: `items` already + // held both members (the registry keeps them since #7730), + // the second `set` overwrote the first, and + // `GET /meta/email_template` served one locale. It only + // ever ran with a `metadata` service installed AND + // answering non-empty for the type — which is why the + // regression was invisible to every suite that omits one. const itemMap = new Map(); + const entryKey = (entry: any): string => + metaItemKey( + entry._packageId ?? undefined, + entry.name, + itemDiscriminator(request.type, entry), + ); for (const item of items) { const entry = item as any; if (entry && typeof entry === 'object' && 'name' in entry) { - itemMap.set(metaItemKey(entry._packageId ?? undefined, entry.name), entry); + itemMap.set(entryKey(entry), entry); } } for (const item of runtimeItems) { @@ -4374,7 +4481,7 @@ export class ObjectStackProtocolImplementation implements // view overlays disappear from list endpoints on // refresh (detail endpoint kept showing the // overlay because it uses a different code path). - const key = metaItemKey(entry._packageId ?? undefined, entry.name); + const key = entryKey(entry); if (!itemMap.has(key)) { itemMap.set(key, entry); } diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index 4cfe84e254..b2fb0cce3c 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -10,6 +10,12 @@ import { TENANT_SCOPE_FIELD_DEF, OWNER_FIELD_DEF, OWNING_BUSINESS_UNIT_FIELD_DEF, + // [#7774] The i18n-bundle identity table and its value reader, sunk into + // metadata-core so this registry and the `/meta` list merge in + // `@objectstack/metadata-protocol` answer "what is this type's identity?" + // from one place. Re-exported below under its original name. + ITEM_KEY_DISCRIMINATORS, + readDiscriminatorValue as discriminatorValue, } from '@objectstack/metadata-core'; import { SystemFieldName } from '@objectstack/spec/system'; import { resolveTenancyPosture, resolveSearchPinyinEnabled } from '@objectstack/types'; @@ -901,34 +907,26 @@ function isCodeArtifactBody(item: unknown): boolean { * * `registerItem` keys every item by `name` (composite `:` when * a package ships it). For most types that IS the identity. `email_template` - * declares otherwise: `EmailTemplateDefinitionSchema` states that "multiple - * rows with the same `name` but different `locale` form an i18n bundle; the - * service picks the best match for the recipient's locale, falling back to - * `en-US`" (`packages/spec/src/system/email-template.zod.ts`), and its header - * says a template "is resolved by `(name, locale)`". A name-only key cannot - * hold that: the second locale collided with the first and overwrote it - * through the `[Registry] Overwriting …` path, so a stack authoring en-US and - * zh-CN copies materialized ONE row into `sys_email_template` — declared, not - * enforced. + * declares otherwise — see the table's own TSDoc for the schema clause it + * enforces and for why the discriminator is declared per type. * - * The discriminator is declared PER TYPE rather than duck-typed off a `locale` - * property, because the key computation is generic to every registered - * metadata type: reading whatever `item.locale` happened to be set would - * silently re-key any other type that grows a locale-ish field, which is a much - * larger contract change than the one this table makes. `email_template` is the - * only type on `main` whose schema declares a top-level `locale` that is part - * of its identity (`grep ' locale:' packages/spec/src/**\/*.zod.ts` — the - * other hits are SCIM users, execution context, discovery and translation - * payloads, none of which is a registered metadata type). + * [#7774] The table itself now lives in `@objectstack/metadata-core` and is + * re-exported here unchanged. It moved because this registry is not the only + * layer that keys metadata by name: `@objectstack/metadata-protocol`'s + * unscoped `/meta/` list merge keys by `(package, name)` too, and a + * bundle that survives registration only to collapse in that merge is still + * collapsed. objectql depends on metadata-protocol, so the protocol package + * cannot import from here; metadata-core is the package both already depend + * on. The re-export is deliberate and load-bearing — + * `registry-i18n-bundle-key.test.ts` imports `ITEM_KEY_DISCRIMINATORS` from + * this module by name. * - * `canonical` is the bundle member a bare-name read resolves to. It mirrors the - * schema's own `locale` default and `sendTemplate`'s documented fallback, and - * `registry-i18n-bundle-key.test.ts` pins the two together so this copy cannot - * drift from the spec. + * The storage-key FORMAT below ({@link BUNDLE_KEY_SEPARATOR}, + * {@link withDiscriminator}, {@link bundleBaseKey}, {@link collectBundle}) + * stayed here on purpose: it encodes a discriminator into THIS registry's + * `:` Map keys, which no other package reads or writes. */ -export const ITEM_KEY_DISCRIMINATORS: Readonly> = { - email_template: { field: 'locale', canonical: 'en-US' }, -}; +export { ITEM_KEY_DISCRIMINATORS }; /** * Separator between an item's name and its bundle discriminator inside a @@ -942,13 +940,6 @@ export const ITEM_KEY_DISCRIMINATORS: Readonly | null | undefined; - const raw = holder?.[field] ?? holder?.content?.[field]; - return typeof raw === 'string' ? raw.trim() : ''; -} - /** `pkg:auth.welcome` + `zh-CN` → `pkg:auth.welcome@zh-CN`. */ function withDiscriminator(baseKey: string, value: string): string { return `${baseKey}${BUNDLE_KEY_SEPARATOR}${value}`;