diff --git a/.changeset/28697-refused-upgrade-keeps-installed.md b/.changeset/28697-refused-upgrade-keeps-installed.md new file mode 100644 index 00000000000..3ba6452cc42 --- /dev/null +++ b/.changeset/28697-refused-upgrade-keeps-installed.md @@ -0,0 +1,15 @@ +--- +"@objectstack/objectql": patch +--- + +Clause-②: yes (widening) — the published `SchemaRegistry` gains one public method, `restoringOnRefusal` +Metadata-protocol: none + +fix(objectql): a refused upgrade leaves the installed version serving + +When `ObjectQL.registerApp` re-registers a package that is already installed and the registry refuses the new version, the installed version keeps serving, whole, whatever point of the registration the refusal came from. + +- The package record, every object, app and item, picklist options and extensions, namespaces, permission-set names and default record pages read as they did before the refused upgrade. +- Another package's extension of the installed version's objects and a tenant's overlay of them are untouched and still served. The restore never goes through uninstall. +- What is refused, and every refusal's code, status and words, is unchanged. A clean upgrade and a refused first install behave as before. +- `SchemaRegistry.restoringOnRefusal(register)` runs one synchronous registration and, when it throws, puts back exactly the registry entries it wrote, then throws the same error. The engine runs an upgrade's registration through it. diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 1ce606e2bd5..581f6ea63b2 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -9329,8 +9329,19 @@ export class ObjectQL implements IObjectQLEngine { // record, object, app or item behind; asked here, before anything is // written. See {@link refusedRegistrationWithdrawable}. const withdrawable = refusedRegistrationWithdrawable(this._registry, id, manifest, this.residualOwnerSources); + // [#28697] A re-register of an installed id is an upgrade: refused + // part-way, the registry puts back exactly the entries it wrote, so the + // installed version keeps serving whole, with other packages' + // extensions of its objects and tenant overlays as they were. Never + // through uninstall, which refuses an extended object and drops a + // tenant's overlay. See {@link SchemaRegistry.restoringOnRefusal}. + const upgrade = Boolean(id) && this._registry.getPackage(id) !== undefined; try { - this.registerPackageBody(manifest, id, namespace, version, source, scope); + if (upgrade) { + this._registry.restoringOnRefusal(() => this.registerPackageBody(manifest, id, namespace, version, source, scope)); + } else { + this.registerPackageBody(manifest, id, namespace, version, source, scope); + } } catch (err) { if (withdrawable) this.withdrawRefusedRegistration(id, manifest, err); throw err; diff --git a/packages/objectql/src/package-refused-registration-withdrawn.test.ts b/packages/objectql/src/package-refused-registration-withdrawn.test.ts index 14b142b876a..e32178dbdcd 100644 --- a/packages/objectql/src/package-refused-registration-withdrawn.test.ts +++ b/packages/objectql/src/package-refused-registration-withdrawn.test.ts @@ -23,7 +23,9 @@ * - CONTROLS: a clean body registers everything as before; a body that took * over a tenant's object is not withdrawn (the withdrawal would drop the * tenant's layer); nor is an installed id refused part-way by the registry - * (the withdrawal would take the installed version with it). + * (the withdrawal would take the installed version with it), which keeps + * serving its installed version instead + * (`package-refused-upgrade-keeps-installed.test.ts`). */ import { describe, it, expect } from 'vitest'; @@ -225,22 +227,22 @@ describe('[#28697] controls', () => { expect(engine.registry.getPackage(PKG), 'not withdrawn').toBeDefined(); }); - it('an installed id refused part-way by the registry is not withdrawn', () => { + it('an installed id refused part-way by the registry is not withdrawn: the installed version keeps serving', () => { const engine = new ObjectQL(); engine.registerApp({ id: HOST, name: HOST, objects: [object('probe_held')] } as any); engine.registerApp({ id: PKG, name: PKG, version: '1.0.0', objects: [object('probe_a'), object('probe_old')], apps: [app('probe_app')] } as any); + const before = trace(engine); const err = refusal(() => engine.registerApp(body({ version: '2.0.0', objects: [object('probe_a', { label: 'v2 label' }), object('probe_held')], }) as any)); expect(err).toMatchObject({ code: 'OBJECT_OWNERSHIP_CONFLICT', status: 422 }); - const t = trace(engine); - // Not withdrawn: the withdrawal would take v1 with it. - expect(t.pkg?.manifest.version).toBe('2.0.0'); - expect((engine.registry.getObject('probe_a') as any)?.label).toBe('v2 label'); - expect(t.objects).toEqual(['probe_a', 'probe_held', 'probe_old']); - expect(t.apps).toEqual(['probe_app']); - // What `registerApp` writes only once the body registered whole is not written. - expect({ nav: t.nav, kinds: t.kinds, datasources: t.datasources }).toEqual({ nav: [], kinds: [], datasources: [] }); + // Not withdrawn (the withdrawal would take v1 with it), and nothing of + // v2 stays: the registry puts back what the refused upgrade wrote. + expect(trace(engine)).toEqual(before); + expect(engine.registry.getPackage(PKG)?.manifest.version).toBe('1.0.0'); + expect((engine.registry.getObject('probe_a') as any)?.label).toBe('probe_a'); + expect(trace(engine).objects).toEqual(['probe_a', 'probe_held', 'probe_old']); + expect(engine.registry.getObjectOwner('probe_held')?.packageId).toBe(HOST); }); }); diff --git a/packages/objectql/src/package-refused-upgrade-keeps-installed.test.ts b/packages/objectql/src/package-refused-upgrade-keeps-installed.test.ts new file mode 100644 index 00000000000..1352600024e --- /dev/null +++ b/packages/objectql/src/package-refused-upgrade-keeps-installed.test.ts @@ -0,0 +1,273 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#28697] A refused upgrade leaves the installed version serving, whole. + * + * ## What this file pins + * + * An installed `1.0.0` of a package (a v1-only object, a shared object, an + * object a tenant has overlaid, an object another package extends, an app, a + * page, a default record page, a permission set, a picklist, a datasource) is + * re-registered as a `2.0.0` that the + * registry refuses part-way, once for each refusal the registry raises after + * the new package record is written: + * + * - the refusal keeps its code and status; + * - the package record, every object (as served and as layered), every item, + * picklist options, namespaces, security-catalog names, default record page + * holders and datasource definitions read exactly as before; + * - the other package's extension and the tenant's overlay still serve. + * + * CONTROLS: a clean `2.0.0` upgrades as before (and keeps the extension and + * the overlay); a refusal the package door raises before its first write leaves + * v1 as before; the other package's own entries are not touched by a refused + * upgrade of this one. + */ + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; + +const PKG = 'com.probe.upgrade'; +const HOST = 'com.probe.host'; +const EXT = 'com.probe.extender'; +const NS = 'probeup'; + +const field = (label: string) => ({ type: 'text', label }); +const object = (name: string, extra: Record = {}) => ({ + name, + label: name, + fields: { title: field('Title') }, + ...extra, +}); +const app = (name: string, objectName: string) => ({ + name, + label: name, + navigation: [{ id: `nav_${objectName}`, type: 'object', label: objectName, objectName }], +}); + +const V1 = () => ({ + id: PKG, + name: PKG, + namespace: NS, + version: '1.0.0', + objects: [ + object('probeup_a'), + object('probeup_shared', { label: 'v1 shared' }), + object('probeup_old', { fields: { title: field('Title'), tier: { type: 'select', label: 'Tier', picklist: 'probeup_tier' } } }), + ], + apps: [app('probeup_app', 'probeup_a')], + pages: [{ name: 'probeup_v1_page', type: 'app' }, { name: 'probeup_v1_record', type: 'record', object: 'probeup_a', isDefault: true }], + permissions: [{ name: 'probeup_v1_ps', objects: {} }], + picklists: [{ name: 'probeup_tier', label: 'Tier', options: [{ label: 'Gold', value: 'gold' }] }], + datasources: [{ name: 'probeup_ds_v1', driver: 'memory' }], +}); + +/** A `2.0.0` that rewrites, adds and drops, with `fault` appended where the registry refuses it. */ +const V2 = (fault: Record = {}) => { + const { objects: faultObjects = [], ...rest } = fault as { objects?: unknown[] }; + return { + id: PKG, + name: PKG, + namespace: NS, + version: '2.0.0', + objects: [ + object('probeup_a', { label: 'v2 a' }), + object('probeup_shared', { label: 'v2 shared' }), + object('probeup_new'), + ...faultObjects, + ], + apps: [app('probeup_app_v2', 'probeup_new')], + pages: [{ name: 'probeup_v2_page', type: 'app' }, { name: 'probeup_v2_record', type: 'record', object: 'probeup_new', isDefault: true }], + permissions: [{ name: 'probeup_v2_ps', objects: {} }], + picklists: [{ name: 'probeup_tier', label: 'Tier', options: [{ label: 'Gold', value: 'gold' }, { label: 'Platinum', value: 'platinum' }] }], + picklistExtensions: [{ extend: 'probeup_industry', options: [{ label: 'Retail', value: 'retail' }] }], + datasources: [{ name: 'probeup_ds_v2', driver: 'memory' }], + ...rest, + }; +}; + +const INDUSTRY = { name: 'probeup_industry', label: 'Industry', options: [{ label: 'Finance', value: 'finance' }] }; +const RECORD_AMOUNT = { + title: field('Title'), + currency_code: { type: 'text', label: 'Currency', valueDomain: 'iso_4217_currency' }, + amount: { type: 'currency', label: 'Amount', currencyConfig: { currencyMode: 'record', currencyField: 'currency_code' } }, +}; + +/** An engine holding HOST, PKG@1.0.0, EXT's extension of a v1 object and a tenant overlay of another. */ +function installed(): ObjectQL { + const engine = new ObjectQL(); + engine.registerApp({ + id: HOST, + name: HOST, + objects: [object('probeup_held', { label: 'Host held' })], + picklists: [INDUSTRY], + picklistExtensions: [{ extend: 'probeup_tier', options: [{ label: 'Silver', value: 'silver' }] }], + } as any); + engine.registerApp(V1() as any); + engine.registerApp({ + id: EXT, + name: EXT, + objectExtensions: [{ extend: 'probeup_old', fields: { ext_note: field('Extender note') } }], + } as any); + engine.registry.registerObject( + { ...object('probeup_a', { label: 'Tenant label' }), _provenance: 'org' } as any, + 'sys_metadata', + undefined, + 'overlay', + ); + return engine; +} + +/** Everything the engine serves, read through its public verbs. */ +function served(engine: ObjectQL) { + const registry = engine.registry; + const objects = registry.getAllObjects().map((o: any) => o.name as string).filter((n) => n.startsWith('probeup_')).sort(); + return { + packages: Object.fromEntries([PKG, HOST, EXT].map((id) => [id, registry.getPackage(id)])), + objects: Object.fromEntries(objects.map((name) => [name, { + served: registry.getObject(name), + layers: registry.getObjectContributors(name).map((c) => ({ packageId: c.packageId, ownership: c.ownership, priority: c.priority, definition: c.definition })), + }])), + items: Object.fromEntries(registry.getRegisteredTypes().filter((t) => t !== 'object').sort().map((type) => [type, registry.listItems(type)])), + picklists: Object.fromEntries(['probeup_tier', 'probeup_industry', 'probeup_list'].map((p) => [p, registry.resolvePicklistOptions(p)])), + namespaces: Object.fromEntries([NS, 'probeupinner'].map((ns) => [ns, registry.getNamespaceOwners(ns)])), + catalog: (['permission', 'position', 'capability'] as const).map((type) => registry.securityCatalogNamesHeldBy(PKG, type as any)), + defaultPages: ['probeup_a', 'probeup_new', 'probeup_held'].map((o) => registry.defaultRecordPageHoldersOf(o)), + datasources: engine.listDatasourceDefs().map((d) => d.name).filter((n) => n.startsWith('probeup_')).sort(), + }; +} + +function refusal(fn: () => void): any { + try { + fn(); + } catch (e) { + return e; + } + throw new Error('expected the upgrade to be refused'); +} + +// ── refusals the registry raises after the 2.0.0 record is written ───────── + +const PART_WAY: Array<{ label: string; fault: Record; code: string; status: number }> = [ + { + label: 'an object field with no `type`', + fault: { objects: [{ name: 'probeup_bad', fields: { t: { label: 'No type' } } }] }, + code: 'INVALID_METADATA', status: 422, + }, + { + label: 'an object declaring its own `queue` field', + fault: { objects: [object('probeup_queued', { fields: { title: field('Title'), queue: field('Queue') } })] }, + code: 'INVALID_METADATA', status: 422, + }, + { + label: 'an object action its schema refuses', + fault: { objects: [object('probeup_acted', { actions: [{ name: 'probeup_go', type: 'nonsense' }] })] }, + code: 'INVALID_METADATA', status: 422, + }, + { + label: 'a roll-up of a record-currency amount in an object registered just before it', + fault: { + objects: [ + object('probeup_line', { fields: RECORD_AMOUNT }), + object('probeup_rollup', { fields: { title: field('Title'), total: { type: 'summary', label: 'Total', summaryOperations: { object: 'probeup_line', field: 'amount', function: 'sum' } } } }), + ], + }, + code: 'INVALID_METADATA', status: 422, + }, + { + label: 'an object another package owns', + fault: { objects: [object('probeup_held')] }, + code: 'OBJECT_OWNERSHIP_CONFLICT', status: 422, + }, + { + label: 'an extension field with no `type`', + fault: { objectExtensions: [{ extend: 'probeup_held', fields: { t: { label: 'No type' } } }] }, + code: 'INVALID_METADATA', status: 422, + }, + { + label: 'a standalone action its schema refuses', + fault: { actions: [{ name: 'probeup_loose', type: 'nonsense' }] }, + code: 'INVALID_METADATA', status: 422, + }, + { + label: 'a picklist repeating a value another package\'s extension already adds to it', + fault: { picklists: [{ name: 'probeup_tier', label: 'Tier', options: [{ label: 'Gold', value: 'gold' }, { label: 'Silver', value: 'silver' }] }] }, + code: 'INVALID_METADATA', status: 422, + }, + { + label: 'a picklist extension repeating a value another package\'s list carries', + fault: { picklistExtensions: [{ extend: 'probeup_industry', options: [{ label: 'Fintech', value: 'finance' }] }] }, + code: 'INVALID_METADATA', status: 422, + }, + { + // After the body registered whole (its picklist and its extension of + // the host's list included), and after a nested object whose + // record-currency amount makes the registry resolve every object. + label: 'the same from a nested plugin, after the body registered', + fault: { + plugins: [{ + name: 'probeup_inner', + namespace: 'probeupinner', + objects: [object('probeup_inner_line', { fields: RECORD_AMOUNT })], + picklistExtensions: [{ extend: 'probeup_industry', options: [{ label: 'Fintech', value: 'finance' }] }], + }], + }, + code: 'INVALID_METADATA', status: 422, + }, +]; + +describe('[#28697] an upgrade the registry refuses part-way leaves the installed version serving', () => { + for (const c of PART_WAY) { + it(c.label, () => { + const engine = installed(); + const before = served(engine); + const err = refusal(() => engine.registerApp(V2(c.fault) as any)); + expect(err).toMatchObject({ code: c.code, status: c.status }); + expect(served(engine)).toEqual(before); + expect(engine.registry.getPackage(PKG)?.manifest.version).toBe('1.0.0'); + expect((engine.registry.getObject('probeup_shared') as any)?.label).toBe('v1 shared'); + expect((engine.registry.getObject('probeup_a') as any)?.label).toBe('Tenant label'); + expect(Object.keys((engine.registry.getObject('probeup_old') as any)?.fields ?? {})).toContain('ext_note'); + expect(engine.registry.getObject('probeup_new')).toBeUndefined(); + expect(engine.registry.resolvePicklistOptions('probeup_industry')?.map((o) => o.value)).toEqual(['finance']); + expect(((engine.registry.getObject('probeup_old') as any)?.fields?.tier?.options ?? []).map((o: any) => o.value)).toEqual(['gold', 'silver']); + }); + } +}); + +// ── controls ─────────────────────────────────────────────────────────────── + +describe('[#28697] refused-upgrade controls', () => { + it('a clean 2.0.0 upgrades as before, keeping the extension and the overlay', () => { + const engine = installed(); + engine.registerApp(V2() as any); + const registry = engine.registry; + expect(registry.getPackage(PKG)?.manifest.version).toBe('2.0.0'); + expect((registry.getObject('probeup_shared') as any)?.label).toBe('v2 shared'); + expect(registry.getObject('probeup_new')).toBeDefined(); + expect((registry.getObject('probeup_a') as any)?.label).toBe('Tenant label'); + expect(Object.keys((registry.getObject('probeup_old') as any)?.fields ?? {})).toContain('ext_note'); + expect(registry.listItems('app').map((a: any) => a.name)).toContain('probeup_app_v2'); + expect(engine.listDatasourceDefs().map((d) => d.name)).toContain('probeup_ds_v2'); + expect(registry.resolvePicklistOptions('probeup_industry')?.map((o) => o.value)).toEqual(['finance', 'retail']); + expect(((registry.getObject('probeup_old') as any)?.fields?.tier?.options ?? []).map((o: any) => o.value)).toEqual(['gold', 'platinum', 'silver']); + expect(registry.securityCatalogNamesHeldBy(PKG, 'permission' as any)).toEqual(['probeup_v1_ps', 'probeup_v2_ps']); + expect(registry.defaultRecordPageHoldersOf('probeup_new').map((h: any) => h.page)).toEqual(['probeup_v2_record']); + }); + + it('a refusal the package door raises before its first write leaves v1 as before', () => { + const engine = installed(); + const before = served(engine); + const err = refusal(() => engine.registerApp({ ...V2(), pages: [{ name: 'probeup_misplaced', type: 'app', isDefault: true }] } as any)); + expect(err).toMatchObject({ status: 422 }); + expect(served(engine)).toEqual(before); + }); + + it('a refused upgrade does not move the other package\'s entries', () => { + const engine = installed(); + const hostBefore = { pkg: engine.registry.getPackage(HOST), held: engine.registry.getObjectContributors('probeup_held'), industry: engine.registry.resolvePicklistOptions('probeup_industry') }; + refusal(() => engine.registerApp(V2({ objects: [object('probeup_held')] }) as any)); + expect({ pkg: engine.registry.getPackage(HOST), held: engine.registry.getObjectContributors('probeup_held'), industry: engine.registry.resolvePicklistOptions('probeup_industry') }).toEqual(hostBefore); + expect(engine.registry.getObjectOwner('probeup_held')?.packageId).toBe(HOST); + }); +}); diff --git a/packages/objectql/src/registry-module-mock.ts b/packages/objectql/src/registry-module-mock.ts index 970d66da527..88d4a40e309 100644 --- a/packages/objectql/src/registry-module-mock.ts +++ b/packages/objectql/src/registry-module-mock.ts @@ -183,6 +183,10 @@ export function createRegistryModuleMock( // `TypeError` at every `registerApp`. getPackage: vi.fn(() => undefined), getObjectContributors: vi.fn(() => []), + // [#28697] …and an upgrade (a `getPackage` hit) runs its body through this + // member, so a refused upgrade is put back. The double runs the body as + // given: it records nothing, so it has nothing to put back. + restoringOnRefusal: vi.fn((register: () => unknown) => register()), reset: vi.fn(), metadata: { get: vi.fn(() => new Map()) }, ...instanceOverrides, diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index 117441bad1f..783a7483467 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -3141,6 +3141,34 @@ function withoutField(schema: ServiceObject, name: string): ServiceObject { return { ...schema, fields: rest } as ServiceObject; } +/** + * [#28697] The undo record of one registration run under + * {@link SchemaRegistry.restoringOnRefusal}: for each registry entry the run + * writes, the way to put back what the entry held just before the run's FIRST + * write to it. An entry the run never writes is never recorded, so a restore + * puts back exactly the entries the run touched and nothing else — another + * package's entries and a tenant's layers are left as they are. + */ +class RegistrationRestorePoint { + private readonly touched = new Set(); + private readonly undo: Array<() => void> = []; + + /** + * Record `key` before its first write in this run. `capture` reads the + * entry as it is now and returns how to put it back. + */ + note(key: string, capture: () => () => void): void { + if (this.touched.has(key)) return; + this.touched.add(key); + this.undo.push(capture()); + } + + /** Put every recorded entry back, the last one written first. */ + restore(): void { + for (let i = this.undo.length - 1; i >= 0; i--) this.undo[i](); + } +} + export class SchemaRegistry { // ========================================== // Logging control @@ -3423,6 +3451,170 @@ export class SchemaRegistry { this.initialDisabledPackageIds = new Set(ids); } + // ========================================== + // Restore on refusal + // ========================================== + + /** The open {@link RegistrationRestorePoint}, while {@link restoringOnRefusal} runs. */ + private restorePoint: RegistrationRestorePoint | undefined; + + /** + * [#28697] Run `register` — one registration made of several registry + * writes, such as an upgrade of an installed package — and, when it throws, + * put back exactly the entries it wrote, then throw the same error. + * + * Every refusal a verb below raises keeps its code and words; this decides + * only what is left afterwards, wherever in the run the refusal came from: + * the entries as they were before the run. + * + * What is recorded, each entry before the run's first write to it: an + * object's contributor list (an `extend` or overlay layer of another package + * or a tenant sits in it and is put back unchanged), a namespace's owners, + * an item slot (the package record included), this package's picklist + * extension of one list, and the security catalog and default record page + * claims the package door records. Nothing the run does not write is + * recorded or put back. After a restore every merged object is + * re-resolved, since a fold computed during the run may have read an entry + * the restore put back. + * + * ⛔ Not a way back from an operator's verb: {@link uninstallPackage}, + * {@link unregisterObject} and the overlay removals are never journaled. + * Synchronous by contract: `register` must not return a promise, so no + * other caller's write can land between the run's writes and its restore. + * A nested call joins the run that is already open. + */ + restoringOnRefusal(register: () => T): T { + if (this.restorePoint) return register(); + const point = new RegistrationRestorePoint(); + this.restorePoint = point; + try { + const result = register(); + if (result instanceof Promise) { + throw new TypeError('[Registry] restoringOnRefusal takes a synchronous registration; it was handed one that returned a promise.'); + } + return result; + } catch (err) { + point.restore(); + this.invalidateAll(); + throw err; + } finally { + this.restorePoint = undefined; + } + } + + /** Record an object's contributor list (and each contributor's mutable slots) before its first write. */ + private noteObjectEntry(fqn: string): void { + this.restorePoint?.note(`object\u0000${fqn}`, () => { + const list = this.objectContributors.get(fqn); + if (!list) return () => this.closeObjectEntry(fqn); + // The late-install branch re-classifies a sitting contributor in place, + // so its ownership and priority are kept beside the reference. + const saved = list.map((c) => ({ c, ownership: c.ownership, priority: c.priority })); + return () => { + for (const s of saved) { + s.c.ownership = s.ownership; + s.c.priority = s.priority; + } + list.splice(0, list.length, ...saved.map((s) => s.c)); + this.mergedObjectCache.delete(fqn); + }; + }); + } + + /** Record a namespace's owner set before its first write. */ + private noteNamespace(namespace: string): void { + this.restorePoint?.note(`namespace\u0000${namespace}`, () => { + const owners = this.namespaceRegistry.get(namespace); + const saved = owners ? new Set(owners) : undefined; + return () => { + if (saved) this.namespaceRegistry.set(namespace, saved); + else this.namespaceRegistry.delete(namespace); + }; + }); + } + + /** Record whether an item type had a store at all, before the run can create one. */ + private noteItemType(type: string): void { + this.restorePoint?.note(`type\u0000${type}`, () => { + const hadType = this.metadata.has(type); + return () => { + if (!hadType && this.metadata.get(type)?.size === 0) this.metadata.delete(type); + }; + }); + } + + /** Record one item slot before its first write. */ + private noteItem(type: string, key: string): void { + this.restorePoint?.note(`item\u0000${type}\u0000${key}`, () => { + const collection = this.metadata.get(type); + const had = collection?.has(key) ?? false; + const value = collection?.get(key); + return () => { + const now = this.metadata.get(type); + if (had) now?.set(key, value); + else now?.delete(key); + }; + }); + } + + /** Record this package's extension of one picklist before its first write. */ + private notePicklistExtension(target: string, packageKey: string): void { + this.restorePoint?.note(`picklistExtension\u0000${target}\u0000${packageKey}`, () => { + const byPackage = this.picklistExtensionContributions.get(target); + const saved = byPackage?.get(packageKey); + return () => { + let now = this.picklistExtensionContributions.get(target); + if (saved) { + if (!now) this.picklistExtensionContributions.set(target, now = new Map()); + now.set(packageKey, saved); + } else if (now) { + now.delete(packageKey); + if (now.size === 0) this.picklistExtensionContributions.delete(target); + } + }; + }); + } + + /** Record who holds one security catalog name before its first write. */ + private noteSecurityCatalogClaim(type: SecurityCatalogType, name: string): void { + this.restorePoint?.note(`catalogClaim\u0000${type}\u0000${name}`, () => { + const saved = this.securityCatalogClaims.get(type)?.get(name); + return () => { + let byName = this.securityCatalogClaims.get(type); + if (saved !== undefined) { + if (!byName) this.securityCatalogClaims.set(type, byName = new Map()); + byName.set(name, saved); + } else { + byName?.delete(name); + } + }; + }); + } + + /** + * Record the default record page claims before `packageId`'s first rewrite + * of them ({@link recordDefaultRecordPageClaims} drops all of the package's + * claims, then adds what the manifest declares). Put back per object, and + * only for the objects `packageId` held a claim on before or after, each in + * its holders' original order. + */ + private noteDefaultRecordPageClaims(packageId: string): void { + this.restorePoint?.note(`defaultPageClaims\u0000${packageId}`, () => { + const saved = new Map>(); + for (const [object, byPackage] of this.defaultRecordPageClaims) saved.set(object, new Map(byPackage)); + return () => { + const objects = new Set(); + for (const [object, byPackage] of saved) if (byPackage.has(packageId)) objects.add(object); + for (const [object, byPackage] of this.defaultRecordPageClaims) if (byPackage.has(packageId)) objects.add(object); + for (const object of objects) { + const before = saved.get(object); + if (before) this.defaultRecordPageClaims.set(object, new Map(before)); + else this.defaultRecordPageClaims.delete(object); + } + }; + }); + } + // ========================================== // Namespace Management // ========================================== @@ -3433,6 +3625,7 @@ export class SchemaRegistry { */ registerNamespace(namespace: string, packageId: string): void { if (!namespace) return; + this.noteNamespace(namespace); let owners = this.namespaceRegistry.get(namespace); if (!owners) { @@ -3449,6 +3642,7 @@ export class SchemaRegistry { unregisterNamespace(namespace: string, packageId: string): void { const owners = this.namespaceRegistry.get(namespace); if (owners) { + this.noteNamespace(namespace); owners.delete(packageId); if (owners.size === 0) { this.namespaceRegistry.delete(namespace); @@ -3623,6 +3817,7 @@ export class SchemaRegistry { * {@link defaultRecordPageClaims} for why a re-install replaces). */ private recordDefaultRecordPageClaims(packageId: string, manifest: ObjectStackManifest): void { + this.noteDefaultRecordPageClaims(packageId); this.forgetDefaultRecordPageClaims(packageId); for (const { object, page } of declaredDefaultRecordPages(manifest)) { let byPackage = this.defaultRecordPageClaims.get(object); @@ -3752,6 +3947,7 @@ export class SchemaRegistry { byName = new Map(); this.securityCatalogClaims.set(type, byName); } + this.noteSecurityCatalogClaim(type, name); byName.set(name, packageId); } } @@ -4101,6 +4297,11 @@ export class SchemaRegistry { // record — skip rather than registering `undefined` as an owner (which // would also mean widening `registerNamespace`'s own required-string // contract, out of scope here). + // [#28697] Recorded before the first write below (the namespace claim), + // so a refusal raised after it — the ownership conflict — puts this + // object's layers back exactly ({@link restoringOnRefusal}). + this.noteObjectEntry(fqn); + if (namespace && packageId) { this.registerNamespace(namespace, packageId); } @@ -6122,6 +6323,7 @@ export class SchemaRegistry { * Universal Register Method for non-object metadata. */ registerItem(type: string, item: T, keyField: keyof T = 'name' as keyof T, packageId?: string) { + this.noteItemType(type); if (!this.metadata.has(type)) { this.metadata.set(type, new Map()); } @@ -6400,6 +6602,7 @@ export class SchemaRegistry { } } + this.noteItem(type, storageKey); collection.set(storageKey, item); if (type === 'picklist') this.invalidateAll(); // [#24400] A bound action is attached to its target on read @@ -7165,6 +7368,7 @@ export class SchemaRegistry { this.registerNamespace(manifest.namespace, manifest.id); } + this.noteItemType('package'); if (!this.metadata.has('package')) { this.metadata.set('package', new Map()); } @@ -7174,6 +7378,7 @@ export class SchemaRegistry { // a normal path, not an error — keep it at debug (#3420). this.debug(`[Registry] Overwriting package: ${manifest.id}`); } + this.noteItem('package', manifest.id); collection.set(manifest.id, pkg); // The names this package now holds, recorded whether or not an item store // entry will carry them (see `securityCatalogClaims`). @@ -7699,6 +7904,7 @@ export class SchemaRegistry { let byPackage = this.picklistExtensionContributions.get(target); if (!byPackage) this.picklistExtensionContributions.set(target, byPackage = new Map()); + this.notePicklistExtension(target, packageKey); byPackage.set(packageKey, { packageId, options: merged }); this.invalidateAll(); this.log(`[Registry] Registered picklist extension: ${target} (+${extension.options?.length ?? 0}) from ${packageId}`); diff --git a/packages/qa/dogfood/test/install-local-refused-package-leaves-nothing.dogfood.test.ts b/packages/qa/dogfood/test/install-local-refused-package-leaves-nothing.dogfood.test.ts index fa0447e9e9e..bc4f5c774b7 100644 --- a/packages/qa/dogfood/test/install-local-refused-package-leaves-nothing.dogfood.test.ts +++ b/packages/qa/dogfood/test/install-local-refused-package-leaves-nothing.dogfood.test.ts @@ -15,7 +15,12 @@ // still reads; // 3. a refused upgrade of an installed package (v2's connector wears the // showcase's id) leaves v1 serving: its object, its app and the package at -// version 1.0.0 read, and v2's new object does not. +// version 1.0.0 read, and v2's new object does not; +// 4. an upgrade the registry refuses part-way (v2's third object is one the +// showcase owns) is refused 422 `PLUGIN_REGISTER_FAILED`, and v1 keeps +// serving whole: the package at 1.0.0, its object with v1's label and the +// field another installed package extends it with, its app; v2's new +// object does not read. // // Boots a fixture stack of its own (the install-local plugin mounted), so it // stays out of `SHARED_SHOWCASE`. @@ -37,6 +42,8 @@ const HOST_OBJECT = 'showcase_product'; const FRESH_ID = 'com.dogfood.refusedfresh'; const MIDWAY_ID = 'com.dogfood.refusedmidway'; const UPGRADE_ID = 'com.dogfood.refusedupgrade'; +const MIDUP_ID = 'com.dogfood.refusedmidup'; +const MIDUP_EXTENDER_ID = 'com.dogfood.refusedmidupext'; const object = (name: string) => ({ name, label: name, fields: { title: { type: 'text', label: 'Title' } } }); const app = (name: string, objectName: string) => ({ @@ -79,6 +86,18 @@ const UPGRADE_V2 = install(UPGRADE_ID, 'refusedupgrade', '2.0.0', { connectors: [stampedConnector('refusedupgrade_conn')], }); +const MIDUP_V1 = install(MIDUP_ID, 'refusedmidup', '1.0.0', { + objects: [{ ...object('refusedmidup_thing'), label: 'Midup v1' }], + apps: [app('refusedmidup_app', 'refusedmidup_thing')], +}); +const MIDUP_EXTENDER = install(MIDUP_EXTENDER_ID, 'refusedmidupext', '1.0.0', { + objectExtensions: [{ extend: 'refusedmidup_thing', fields: { ext_note: { type: 'text', label: 'Extender note' } } }], +}); +const MIDUP_V2 = install(MIDUP_ID, 'refusedmidup', '2.0.0', { + objects: [{ ...object('refusedmidup_thing'), label: 'Midup v2' }, object('refusedmidup_new'), object(HOST_OBJECT)], + apps: [app('refusedmidup_app', 'refusedmidup_thing')], +}); + type Answer = { status: number; body: any }; async function json(res: Response): Promise { @@ -120,6 +139,16 @@ describe('dogfood: a package the install refuses part-way leaves nothing of itse answers.upgradeObject = await get('/meta/object/refusedupgrade_thing'); answers.upgradeApp = await get('/meta/app/refusedupgrade_app'); answers.upgradeNew = await get('/meta/object/refusedupgrade_new'); + + // 4 + answers.midupV1 = await post(MIDUP_V1); + answers.midupExtender = await post(MIDUP_EXTENDER); + answers.midupV2 = await post(MIDUP_V2); + answers.midupPackage = await get(`/packages/${MIDUP_ID}`); + answers.midupObject = await get('/meta/object/refusedmidup_thing'); + answers.midupApp = await get('/meta/app/refusedmidup_app'); + answers.midupNew = await get('/meta/object/refusedmidup_new'); + answers.midupHostObject = await get(`/meta/object/${HOST_OBJECT}`); }, 300_000); afterAll(async () => { @@ -161,4 +190,21 @@ describe('dogfood: a package the install refuses part-way leaves nothing of itse expect(answers.upgradeApp.status).toBe(200); expect(answers.upgradeNew.status, shown(answers.upgradeNew)).toBe(404); }); + + it('4: an upgrade the registry refuses part-way leaves the installed version serving whole', () => { + expect(answers.midupV1.status, shown(answers.midupV1)).toBe(200); + expect(answers.midupExtender.status, shown(answers.midupExtender)).toBe(200); + expect(answers.midupV2.status, shown(answers.midupV2)).toBe(422); + expect(answers.midupV2.body?.error?.code).toBe('PLUGIN_REGISTER_FAILED'); + expect(answers.midupV2.body?.error?.message).toContain(`already owned by package "${HOST_PACKAGE_ID}"`); + expect(answers.midupPackage.status, shown(answers.midupPackage)).toBe(200); + expect(JSON.stringify(answers.midupPackage.body)).toContain('"version":"1.0.0"'); + expect(JSON.stringify(answers.midupPackage.body)).not.toContain('"version":"2.0.0"'); + expect(answers.midupObject.status, shown(answers.midupObject)).toBe(200); + expect(answers.midupObject.body?.item?.label).toBe('Midup v1'); + expect(Object.keys(answers.midupObject.body?.item?.fields ?? {})).toContain('ext_note'); + expect(answers.midupApp.status).toBe(200); + expect(answers.midupNew.status, shown(answers.midupNew)).toBe(404); + expect(answers.midupHostObject.status).toBe(200); + }); });