diff --git a/.changeset/adr-0048-package-scoped-overlays.md b/.changeset/adr-0048-package-scoped-overlays.md new file mode 100644 index 0000000000..d6262d718f --- /dev/null +++ b/.changeset/adr-0048-package-scoped-overlays.md @@ -0,0 +1,38 @@ +--- +"@objectstack/objectql": minor +"@objectstack/metadata-core": minor +--- + +feat(metadata): package-scoped customization overlays (ADR-0048 #1824) + +A `sys_metadata` customization overlay is now keyed by `(type, name, +organization_id, package_id)`, so two installed packages shipping an item of the +same `type`/`name` can each carry their **own** overlay. Previously the overlay +uniqueness key was `(type, name, organization_id)` — physically one row per +name — so customizing one package's item shadowed both, and a package-scoped +read fell back to whichever row existed. + +- **Index**: `idx_sys_metadata_overlay_active` / `…_draft` now include + `package_id`. The runtime migration (`ensureOverlayIndex`) uses + `COALESCE(package_id, '')` so package-less (global) overlays stay unique among + themselves (a plain unique index treats NULLs as distinct). DROP-then-CREATE, + idempotent; existing rows migrate safely (the old key already guaranteed one + row per `(type, name, org)`). +- **Write**: `SysMetadataRepository.whereFor`/`put`/`get` scope the upsert to the + requested package, so a save bound to package B no longer finds and overwrites + package A's same-name overlay. A package-less save (`packageId` null) targets + the global row. +- **Read**: `getMetaItem` / `getMetaItemLayered` overlay lookups already prefer + the package-scoped row; the fallback now resolves only the **global** + (`package_id IS NULL`) overlay, never a *different* package's row. Package-less + readers are unchanged (match-any, back-compat). + +Verified live against a real collision (two packages each shipping +`page/showcase_task_workbench`): two overlay rows coexist, and `?package=` single +reads + the `?layers=true` Studio editor view each return that package's own +overlay; the unique index migrated in place. + +Known follow-up: the *unscoped list* (`GET /meta/:type` with no `?package=`) +still dedupes by bare name, so when two packages both carry an overlay on the +same name the list collapses them — the per-package single-item and editor paths +are unaffected. Tracked for the list-dedup-by-name work. diff --git a/packages/metadata-core/src/objects/sys-metadata.object.ts b/packages/metadata-core/src/objects/sys-metadata.object.ts index 984c58efd1..f24e88cb60 100644 --- a/packages/metadata-core/src/objects/sys-metadata.object.ts +++ b/packages/metadata-core/src/objects/sys-metadata.object.ts @@ -199,15 +199,20 @@ export const SysMetadataObject = ObjectSchema.create({ }, indexes: [ - // ADR-0005 (revised 2026-05): overlay uniqueness is scoped by - // (type, name, organization_id), restricted to active rows so resets - // / archived versions don't collide. environment_id is deprecated and - // not part of the discriminator. The runtime layer (protocol.ts - // ensureOverlayIndex) issues a DROP-then-CREATE migration to - // replace any pre-existing legacy composite index in-place. + // ADR-0005 (revised 2026-05) + ADR-0048: overlay uniqueness is scoped by + // (type, name, organization_id, package_id), restricted to active rows so + // resets / archived versions don't collide. `package_id` is part of the + // discriminator so two installed packages shipping the same `type`/`name` + // each get their OWN customization row (a package-less / global overlay + // uses NULL). environment_id is deprecated and not part of the + // discriminator. The runtime layer (protocol.ts ensureOverlayIndex) issues + // a DROP-then-CREATE migration that uses `COALESCE(package_id,'')` so the + // package-less rows stay unique among themselves (SQLite treats NULLs as + // distinct in a plain unique index); this declaration is the fallback shape + // for drivers without the runtime migration. { name: 'idx_sys_metadata_overlay_active', - fields: ['type', 'name', 'organization_id'], + fields: ['type', 'name', 'organization_id', 'package_id'], unique: true, partial: "state = 'active'", }, diff --git a/packages/objectql/src/protocol-meta.test.ts b/packages/objectql/src/protocol-meta.test.ts index ff3c0e72c8..3129818c29 100644 --- a/packages/objectql/src/protocol-meta.test.ts +++ b/packages/objectql/src/protocol-meta.test.ts @@ -60,7 +60,9 @@ describe('ObjectStackProtocolImplementation - Metadata Persistence', () => { organizationId: 'org_alpha', }); expect(mockEngine.findOne).toHaveBeenCalledWith('sys_metadata', { - where: { type: 'app', name: 'test_app', organization_id: 'org_alpha', state: 'active' }, + // ADR-0048 — a package-less save scopes the upsert lookup to the + // GLOBAL row (package_id IS NULL), not any package's row. + where: { type: 'app', name: 'test_app', organization_id: 'org_alpha', state: 'active', package_id: null }, }); expect(mockEngine.insert).toHaveBeenCalledWith('sys_metadata', expect.objectContaining({ organization_id: 'org_alpha', @@ -241,7 +243,8 @@ describe('ObjectStackProtocolImplementation - Metadata Persistence', () => { await protocol.saveMetaItem({ type: 'app', name: 'test_app', item: sampleApp }); expect(mockEngine.findOne).toHaveBeenCalledWith('sys_metadata', { - where: { type: 'app', name: 'test_app', organization_id: null, state: 'active' } + // ADR-0048 — package-less save scopes the lookup to the global row. + where: { type: 'app', name: 'test_app', organization_id: null, state: 'active', package_id: null } }); expect(mockEngine.insert).toHaveBeenCalledWith('sys_metadata', expect.objectContaining({ name: 'test_app', @@ -252,6 +255,25 @@ describe('ObjectStackProtocolImplementation - Metadata Persistence', () => { }), expect.anything()); }); + it('scopes the upsert lookup to the requested package (ADR-0048 #1824)', async () => { + // A save bound to package B must look up B's own row, not match (and + // overwrite) package A's same-name overlay — that is what lets two + // installed packages keep independent customizations. + mockEngine.findOne.mockResolvedValue(null); + + await protocol.saveMetaItem({ + type: 'app', name: 'test_app', item: sampleApp, + organizationId: 'org_alpha', packageId: 'com.acme.beta', + }); + + expect(mockEngine.findOne).toHaveBeenCalledWith('sys_metadata', { + where: { type: 'app', name: 'test_app', organization_id: 'org_alpha', state: 'active', package_id: 'com.acme.beta' }, + }); + expect(mockEngine.insert).toHaveBeenCalledWith('sys_metadata', expect.objectContaining({ + package_id: 'com.acme.beta', + }), expect.anything()); + }); + it('should update an existing record in the database and increment version', async () => { const existingRecord = { id: 'existing-uuid', version: 2 }; mockEngine.findOne.mockResolvedValue(existingRecord); diff --git a/packages/objectql/src/protocol.ts b/packages/objectql/src/protocol.ts index 98a32926e0..36a0589979 100644 --- a/packages/objectql/src/protocol.ts +++ b/packages/objectql/src/protocol.ts @@ -750,20 +750,25 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { throw new Error('driver has neither raw nor execute'); } }; - // ADR-0005 (revised 2026-05): per-env DBs replace the old + // ADR-0005 (revised 2026-05) + ADR-0048: per-env DBs replace the old // "per-project" isolation, so `environment_id` is no longer a // discriminator. Overlay uniqueness is `(type, name, - // organization_id)` filtered to active rows. Drop the legacy - // composite index first so the new partial UNIQUE can claim - // the same name — DROP INDEX IF EXISTS is idempotent. + // organization_id, COALESCE(package_id,''))` filtered to active + // rows — `package_id` is in the key so two installed packages + // shipping the same name each get their own overlay, while + // `COALESCE(...,'')` keeps the package-less (global) rows unique + // among themselves (a plain unique index would treat NULLs as + // distinct and allow duplicate globals). Drop the legacy composite + // index first so the new partial UNIQUE can claim the same name — + // DROP INDEX IF EXISTS is idempotent. try { await exec("DROP INDEX IF EXISTS idx_sys_metadata_overlay_active"); } catch { /* best-effort */ } const partialSql = "CREATE UNIQUE INDEX IF NOT EXISTS idx_sys_metadata_overlay_active " + - "ON sys_metadata (type, name, organization_id) " + + "ON sys_metadata (type, name, organization_id, COALESCE(package_id, '')) " + "WHERE state = 'active'"; const fallbackSql = "CREATE INDEX IF NOT EXISTS idx_sys_metadata_overlay_active " + - "ON sys_metadata (type, name, organization_id)"; + "ON sys_metadata (type, name, organization_id, package_id)"; try { await exec(partialSql); } catch (err: any) { @@ -779,12 +784,14 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { } // Mirror the same partial-UNIQUE for draft rows so a second // simultaneous draft cannot be inserted for the same - // (type,name,org). The unique-active index above already + // (type,name,org,package). The unique-active index above already // guards published rows; the two never collide because the - // `state` predicate disambiguates them. + // `state` predicate disambiguates them. DROP first so an existing + // legacy 3-column draft index is replaced in-place (ADR-0048). + try { await exec("DROP INDEX IF EXISTS idx_sys_metadata_overlay_draft"); } catch { /* best-effort */ } const draftPartialSql = "CREATE UNIQUE INDEX IF NOT EXISTS idx_sys_metadata_overlay_draft " + - "ON sys_metadata (type, name, organization_id) " + + "ON sys_metadata (type, name, organization_id, COALESCE(package_id, '')) " + "WHERE state = 'draft'"; try { await exec(draftPartialSql); @@ -794,7 +801,7 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { try { await exec( "CREATE INDEX IF NOT EXISTS idx_sys_metadata_overlay_draft " + - "ON sys_metadata (type, name, organization_id)", + "ON sys_metadata (type, name, organization_id, package_id)", ); } catch { // ignore — best effort @@ -1428,6 +1435,11 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { where: { ...base, package_id: request.packageId }, }); if (scoped) return scoped; + // ADR-0048 — global (package-less) draft only, never + // another package's draft. + return await this.engine.findOne('sys_metadata', { + where: { ...base, package_id: null }, + }); } return await this.engine.findOne('sys_metadata', { where: base }); }; @@ -1476,7 +1488,15 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { where: { ...base, package_id: request.packageId }, }); if (scoped) return scoped; + // ADR-0048 — no package-owned overlay; fall back to the + // GLOBAL (package-less) overlay only. Must NOT match a + // different package's row, or a collision would serve + // package B's customization for a package A read. + return await this.engine.findOne('sys_metadata', { + where: { ...base, package_id: null }, + }); } + // No package context (legacy/runtime reader) — match any. return await this.engine.findOne('sys_metadata', { where: base }); }; const rec = await lookup(request.type); @@ -1748,6 +1768,11 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { where: { ...base, package_id: request.packageId }, }); if (scoped) return scoped; + // ADR-0048 — fall back to the GLOBAL (package-less) + // overlay only, never another package's row. + return await this.engine.findOne('sys_metadata', { + where: { ...base, package_id: null }, + }); } return await this.engine.findOne('sys_metadata', { where: base }); }; @@ -3722,8 +3747,13 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { // Parent is scoped to the lifecycle we're about to write: // a draft's parent is the current draft hash (or null // for the first draft); a publish's parent is the - // current published hash. - const current = await repo.get(ref, { state: mode === 'draft' ? 'draft' : 'active' }); + // current published hash. ADR-0048 — scope to the same + // package the upsert targets so a collision's other-package + // row is never read as this item's parent. + const current = await repo.get(ref, { + state: mode === 'draft' ? 'draft' : 'active', + packageId: request.packageId ?? null, + }); parentVersion = current?.hash ?? null; } try { diff --git a/packages/objectql/src/sys-metadata-repository.ts b/packages/objectql/src/sys-metadata-repository.ts index cd3ab6d6dc..5980002177 100644 --- a/packages/objectql/src/sys-metadata-repository.ts +++ b/packages/objectql/src/sys-metadata-repository.ts @@ -250,11 +250,17 @@ export class SysMetadataRepository implements MetadataRepository { * live published row (`'active'`). Pass `'draft'` to read the pending * unpublished revision (if any). */ - async get(ref: MetaRef, opts?: { state?: OverlayState }): Promise { + async get( + ref: MetaRef, + opts?: { state?: OverlayState; packageId?: string | null }, + ): Promise { this.assertOpen(); const state = opts?.state ?? 'active'; + // ADR-0048 — when a package scope is supplied, resolve the row owned by + // that package (used by saveMetaItem to read the correct parent-version + // lineage before an upsert). Omitted → legacy "any package" match. const row = await this.engine.findOne('sys_metadata', { - where: this.whereFor(ref, state), + where: this.whereFor(ref, state, opts && 'packageId' in opts ? (opts.packageId ?? null) : undefined), }); if (!row) return null; return this.rowToItem(ref, row); @@ -314,8 +320,11 @@ export class SysMetadataRepository implements MetadataRepository { // Run all reads + writes inside one transaction so the optimistic // lock, the parent-row mutation, and the history append are atomic. const result = await this.withTxn(async (ctx) => { + // ADR-0048 — scope the existing-row lookup to the requested package so a + // save for package B does not find (and overwrite) package A's same-name + // overlay. A package-less save (packageId null) targets the global row. const existing = await this.engine.findOne('sys_metadata', { - where: this.whereFor(ref, state), + where: this.whereFor(ref, state, opts.packageId ?? null), context: ctx, }); const existingHash: string | null = existing?.checksum ?? null; @@ -903,13 +912,22 @@ export class SysMetadataRepository implements MetadataRepository { private whereFor( ref: Pick, state: OverlayState = 'active', + packageId?: string | null, ): Record { - return { + const where: Record = { type: ref.type, name: ref.name, organization_id: this.organizationId, state, }; + // ADR-0048 — when the caller scopes by package, the overlay row is keyed by + // `(org, type, name, package_id)` so two installed packages shipping the + // same name each get their OWN customization row (a package-less / global + // overlay uses `package_id IS NULL`). When `packageId` is omitted (legacy + // callers — delete/promote/restore), the package dimension is left out so + // the query keeps its historical "match any package" behaviour. + if (packageId !== undefined) where.package_id = packageId; // string → eq; null → IS NULL + return where; } private fullRef(ref: Pick): MetaRef {