diff --git a/.changeset/23415-drop-storage-other-rows.md b/.changeset/23415-drop-storage-other-rows.md index 7d459e30fa..dc7e8a2cae 100644 --- a/.changeset/23415-drop-storage-other-rows.md +++ b/.changeset/23415-drop-storage-other-rows.md @@ -14,7 +14,7 @@ A `sys_metadata` row is keyed by its package (ADR-0048), so one object can hold **FROM.** `DELETE /api/v1/meta/object/my_object?package=com.acme.a&dropStorage=true` (`deleteMetaItem({ type: 'object', name, packageId, dropStorage: true })` in process), of an object a second package `com.acme.b` also holds, removed `com.acme.a`'s row, answered `200` with a receipt saying `com.acme.b`'s row remains, and dropped the object's table. The object stayed defined and served by `com.acme.b`'s row while its records were gone: its next data read answered `500`. The same held for a package-less row left beside the deleted package's row, and for an uninstall (`deletePackage`, which deletes each of the package's rows with `dropStorage` unless `keepData` is set). -**TO.** Such a delete is refused `409 RESOURCE_CONFLICT` before anything is written: both rows, the table and its records stay. The message names the rows that remain and the way out. An uninstall reports the refusal for that object in its `failed[]` list, as it reports any per-item refusal, and leaves the object's row and table in place. +**TO.** Such a delete is refused `409 RESOURCE_CONFLICT` before anything is written: both rows, the table and its records stay. The message names the rows that remain and the way out. An uninstall of a package holding such an object, without `keepData`, is refused whole, `409 RESOURCE_CONFLICT`, before anything is removed; its message names the object, the rows that keep it, and the ways out. **Unchanged.** The delete of an object's last stored row with `dropStorage` still drops its table. A delete without `dropStorage` still removes the named package's row and keeps the table, and its receipt names the rows that remain. Only `active` rows count: a draft is never materialised, so a remaining draft does not keep the table. diff --git a/.changeset/23473-uninstall-shared-object-refusal.md b/.changeset/23473-uninstall-shared-object-refusal.md new file mode 100644 index 0000000000..5720ab57e1 --- /dev/null +++ b/.changeset/23473-uninstall-shared-object-refusal.md @@ -0,0 +1,21 @@ +--- +'@objectstack/metadata-protocol': minor +--- + +fix(metadata-protocol)!: an uninstall without `keepData` of a package holding an object another stored row also holds is refused `409 RESOURCE_CONFLICT` before anything is removed, instead of dropping the object's table while the other row still serves it (ADR-0048) + +Clause-②: no (narrowing) + + + +**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes. + +A `sys_metadata` row is keyed by its package (ADR-0048), so one object can hold one row per package, and a package-less row beside them, all served over the one table its name maps to. An uninstall drops each of its objects' tables unless `keepData` is set, and a table is dropped only with the object's last stored row. + +**FROM.** `DELETE /api/v1/packages/com.acme.a` (`deletePackage({ packageId: 'com.acme.a' })` in process), for a package whose object a second package `com.acme.b` (or the package-less row) also holds, removed the package and its rows and dropped the shared object's table along with the package's row of it. The object stayed defined and served by `com.acme.b`'s row while its records were gone: its next data read answered `500`. + +**TO.** Such an uninstall is refused `409 RESOURCE_CONFLICT` before its first durable step: the `sys_packages` row, the registry entry, every row, the tables and their records all stay. The message names each shared object and the rows that keep it, and the ways out. + +**Unchanged.** An uninstall with `keepData` drops no table, so a shared object does not refuse it. An uninstall whose objects no other row holds still drops their tables. + +**Remedy.** Uninstall with `?keepData=true` (`keepData: true` in process) to keep every table of the package; or first delete the package's own row of each shared object without `dropStorage` (`DELETE /api/v1/meta/object/NAME?package=com.acme.a`), which keeps that table for the other rows, then uninstall again to drop the tables the package alone held. diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 087fab444f..493aeec043 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -25889,6 +25889,9 @@ export class ObjectStackProtocolImplementation implements * 3. the registry's uninstall refusal (another package extends an object * this one owns, ADR-0029), asked through * `SchemaRegistry.assertPackageUninstallable` — pure; thrown as is; + * then [#23473] the shared-object refusal (another stored row holds an + * object whose table this uninstall would drop) — reads only, the + * per-item delete's own address reads; `RESOURCE_CONFLICT` / 409; * 4. the `sys_packages` delete through the `package` service — the FIRST * durable step; a refusal, returned or thrown, is thrown as this verb's * failure (see {@link packagePersistFailureError}); @@ -25978,6 +25981,73 @@ export class ObjectStackProtocolImplementation implements packageRegistry.assertPackageUninstallable(request.packageId); } + // [#23473, ADR-0048] THE SHARED-OBJECT REFUSAL, ALSO ASKED BEFORE THE + // STORE DELETE. A `sys_metadata` row is keyed by its package, so an + // object this package holds can also be held by another package's row, + // or by the package-less row, all served over the ONE table its name + // maps to. The per-item delete below drops an object's table only with + // its last stored row and otherwise refuses `409` + // ({@link dropStorageRemainingRowsRefusal}). Met there, that refusal + // came after the stored package row was gone, so it left the package + // half-uninstalled: no `sys_packages` row, and its row of the shared + // object still stored and served. + // + // So the same question is asked here, of the same rows, before anything + // durable: for each active object row whose table this uninstall would + // drop ({@link shouldDropStorage}), the per-item delete's own address + // read ({@link packagelessAddress}, the type folded as that delete + // folds it). An object this package holds there beside any other row + // refuses the whole uninstall, with nothing removed, as step 3 does + // for an extended object. ⛔ Not "uninstall and keep the shared table": + // dropping the tables is this verb's declared default, and an + // uninstall that quietly drops fewer than it says is an opt-in the + // runtime does not honour. + // + // With `keepData: true` no table is dropped, so sharing decides + // nothing and nothing is read. `RESOURCE_CONFLICT` / 409, no code + // minted: the per-item refusal's code for the same rows (environment + // state, not the request: the same uninstall is accepted once the other + // rows, or this package's row, are gone). A failed read is this verb's + // step-2 read failure ({@link metadataReadFailureError}), never "not + // shared". The prescription first: a 4xx message can be truncated. + if (request.keepData !== true) { + const shared: string[] = []; + const asked = new Set(); + for (const row of rows) { + if (row?.state === 'draft' || typeof row?.name !== 'string') continue; + const canonical = canonicalMetaType(String(row.type)); + const type = PLURAL_TO_SINGULAR[canonical] ?? canonical; + if (!this.shouldDropStorage(type, row.name, true, 'active') || asked.has(row.name)) continue; + asked.add(row.name); + let at: { candidates: readonly string[]; unbound: boolean }; + try { + at = await this.packagelessAddress(type, row.name, undefined, 'active'); + } catch (e) { + throw metadataReadFailureError(e); + } + if (!at.candidates.includes(request.packageId)) continue; + const others = at.candidates.filter((id) => id !== request.packageId); + if (others.length === 0 && !at.unbound) continue; + shared.push(`'${row.name}' (${remainingRowsPhrase(others, at.unbound, 'row')})`); + } + if (shared.length > 0) { + const more = shared.length > 5 ? `; and ${shared.length - 5} more` : ''; + const err = new Error( + `Uninstall package '${request.packageId}' with keepData (?keepData=true; keepData: true in ` + + 'process), which keeps all its object tables, or first delete, without dropStorage, its row ' + + 'or the other rows of each object below (DELETE /api/v1/meta/object/NAME?package=ID). ' + + "Nothing was removed: an uninstall drops an object's table only with the object's last row, and " + + (shared.length === 1 + ? 'an object of this package is' + : `${shared.length} objects of this package are`) + + ` also held by other rows: ${shared.slice(0, 5).join('; ')}${more}.`, + ) as Error & { code: 'RESOURCE_CONFLICT'; status: 409 }; + err.code = 'RESOURCE_CONFLICT'; + err.status = 409; + throw err; + } + } + // [#21276] THE STORE DELETE COMES FIRST, and its refusal is this verb's // refusal. Triage's ruling: refuse before withdrawing, not undo. Every // step above this one only reads; every step below it — the per-item diff --git a/packages/runtime/src/package-uninstall-shared-object.integration.test.ts b/packages/runtime/src/package-uninstall-shared-object.integration.test.ts new file mode 100644 index 0000000000..63c04f84e2 --- /dev/null +++ b/packages/runtime/src/package-uninstall-shared-object.integration.test.ts @@ -0,0 +1,310 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #23473 — `DELETE /api/v1/packages/:id` of a package that shares an object + * with another stored row. + * + * ## The defect, as it stood once the metadata delete door refused the drop + * + * A `sys_metadata` row is keyed by its package, so one object can hold one row + * per package (and a package-less row), all served over the ONE table its name + * maps to. An uninstall drops each of its objects' tables by default, through + * the per-item metadata delete with `dropStorage`, and that delete drops a + * table only with the object's last stored row: while another row stays, it + * is refused `409 RESOURCE_CONFLICT`. `deletePackage` met that refusal after + * its first durable step, the `sys_packages` delete, and collected it in + * `failed[]`: the door answered `400 PACKAGE_DELETE_PARTIAL` over a package + * that was no longer installed while its row of the shared object stayed + * stored and served — a half-uninstalled package. + * + * ## The contract pinned here (lane decision: refuse the whole uninstall) + * + * 1. Without `keepData`, an uninstall of a package holding an object another + * package's row also holds is refused `409 RESOURCE_CONFLICT` before + * anything durable: the `sys_packages` row, the registry entry, both rows, + * the table and its records all stay. The refusal names the object and the + * row that keeps it, and prescribes `keepData`. + * 2. The same with the package-less row as the other holder. + * 3. The ways out the refusal prescribes complete, with nothing in `failed[]`: + * `?keepData=true` removes the package and keeps the table; deleting the + * package's own row of the shared object first (no `dropStorage`) lets the + * uninstall complete, keep the shared table and still drop the tables the + * package alone held. Either way the running registry then serves the + * other package's definition of the object. + * 4. CONTROL: an object one package holds — the uninstall drops its table. + * + * ## The composition — the shipped pieces + * + * The sibling `package-uninstall-store-refusal.integration.test.ts` harness: a + * REAL `ObjectQL` over a REAL `SqlDriver` (better-sqlite3, on disk, so the + * object tables are real); the REAL `PackageServicePlugin`, which owns + * `sys_packages`; the REAL `ObjectStackProtocolImplementation`; and the REAL + * `HttpDispatcher`, whose `handlePackages` is the uninstall door. The objects + * are published through the protocol's own save (`saveMetaItem`, which + * creates the table), as the metadata door's `PUT ?package=` does. + * + * ⚠️ `@objectstack/metadata-protocol` resolves through `exports` to its + * **`dist/`** (`check-test-source-alias.mjs`, `KNOWN_UNALIASED_TEST_IMPORTS`): + * rebuild it before reading a result here after touching `protocol.ts`. + */ + +import { describe, it, expect, beforeAll, afterAll, afterEach, beforeEach, vi } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { + SysMetadataObject, + SysMetadataHistoryObject, + SysMetadataAuditObject, + hashSpec, +} from '@objectstack/metadata-core'; +import { PackageServicePlugin } from '@objectstack/service-package'; +import { HttpDispatcher } from './http-dispatcher.js'; + +const P = 'com.probe.p'; +const Q = 'com.probe.q'; +const PLATFORM_PKG = 'com.objectstack.platform'; +const ENV = 'platform'; +const SHARED = 'probe_ticket'; +const PRIVATE = 'probe_note'; + +/** A signed-in admin — the caller of the sibling harness. */ +const CALLER: any = { + request: {}, + environmentId: ENV, + executionContext: { + userId: 'u_admin', + isSystem: false, + systemPermissions: ['manage_metadata', 'studio.access'], + tenantId: 'org_acme', + }, +}; + +const quiet = { debug() {}, info() {}, warn() {}, error() {} }; + +let cleanup: Array<() => void | Promise> = []; +afterEach(async () => { + for (const c of cleanup.reverse()) await c(); + cleanup = []; +}); + +const envSnapshot = { OS_HOME: process.env.OS_HOME }; +let home: string; +beforeAll(() => { + home = mkdtempSync(join(tmpdir(), 'os-23473-home-')); + process.env.OS_HOME = home; +}); +afterAll(() => { + if (envSnapshot.OS_HOME === undefined) delete process.env.OS_HOME; + else process.env.OS_HOME = envSnapshot.OS_HOME; + rmSync(home, { recursive: true, force: true }); +}); + +let warnSpy: ReturnType; +beforeEach(() => { warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); }); +afterEach(() => { warnSpy.mockRestore(); }); + +const objectBody = (name: string, label: string) => ({ + name, + label, + // [#8310] The runtime object door requires an authored OWD. + sharingModel: 'private', + fields: { title: { name: 'title', type: 'text' as const, label: 'Title' } }, +}); + +async function boot() { + const dir = mkdtempSync(join(tmpdir(), 'os-23473-db-')); + cleanup.push(() => rmSync(dir, { recursive: true, force: true })); + const driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: join(dir, 'data.sqlite') }, + useNullAsDefault: true, + }); + const objects = [SysMetadataObject, SysMetadataHistoryObject, SysMetadataAuditObject] as any[]; + await driver.initObjects(objects); + + const engine = new ObjectQL(); + engine.registerDriver(driver as any, true); + await engine.init(); + for (const o of objects) engine.registry.registerObject(o, PLATFORM_PKG); + cleanup.push(() => engine.destroy()); + + const services = new Map([['objectql', engine]]); + const ctx: any = { + logger: quiet, + getService: (name: string) => services.get(name), + registerService: (name: string, service: unknown) => { services.set(name, service); }, + }; + const plugin = new PackageServicePlugin(); + await plugin.init(ctx); + await plugin.start(ctx); + + const protocol = new ObjectStackProtocolImplementation(engine as any, () => services as any, undefined, 'package-author'); + services.set('protocol', protocol); + const get = (name: string) => services.get(name) ?? null; + const dispatcher = new HttpDispatcher({ context: { getService: get }, getService: get, getServiceAsync: async (n: string) => get(n) } as any); + const knex = (driver as any).knex; + + return { + engine, + protocol, + /** `DELETE /api/v1/packages/:id[?keepData=true]` and the other package verbs, at the real door. */ + async call(method: string, path: string, query: Record = {}) { + const res = await dispatcher.handlePackages(path, method, undefined, query, CALLER); + const body = JSON.parse(JSON.stringify(res.response?.body ?? null)); + return { status: res.response?.status ?? 0, code: body?.error?.code as string | undefined, body }; + }, + async install(id: string) { + await protocol.installPackage({ manifest: { id, name: id, version: '1.0.0', type: 'app' } } as any); + }, + /** A publish of `name` bound to `packageId` — the save creates the table. */ + async publish(packageId: string, name: string) { + await (protocol as any).saveMetaItem({ type: 'object', name, item: objectBody(name, `${name} ${packageId}`), packageId }); + }, + /** + * A package-less row of the object as it exists at rest — the save door + * resolves a package-less save of an item a package holds to that + * package's row, so such a row is only ever a stored one. + */ + async seedUnboundRow(name: string) { + const body = objectBody(name, `${name} unbound`); + const now = new Date().toISOString(); + await engine.insert('sys_metadata', { + id: `seed_active_unbound_${name}`, type: 'object', name, organization_id: null, + package_id: null, state: 'active', metadata: JSON.stringify(body), checksum: hashSpec(body, 'object'), + version: 1, created_at: now, updated_at: now, + }, { context: { isSystem: true } } as any); + }, + /** The object's stored active rows, by binding — the store, not the door's word for it. */ + async storedBindings(name: string) { + const rows = (await knex('sys_metadata').where({ type: 'object', name, state: 'active' }).select('package_id')) as any[]; + return rows.map((r) => (r.package_id ?? null) as string | null).sort((a, b) => String(a).localeCompare(String(b))); + }, + /** Whether `sys_packages` still holds the package — the table, not the registry. */ + async storedPackage(id: string) { + return ((await knex('sys_packages').where({ id }).select('id')) as any[]).length > 0; + }, + hasTable: (name: string) => knex.schema.hasTable(name) as Promise, + async records(name: string) { + return ((await knex(name).select('title')) as Array<{ title: string }>).map((r) => r.title); + }, + insertRecord: (name: string, title: string) => engine.insert(name, { title }, { context: { isSystem: true } } as any), + }; +} + +type Process = Awaited>; + +/** P and Q both hold `probe_ticket`; its table holds one record. */ +async function twoPackageObject(): Promise { + const h = await boot(); + await h.install(P); + await h.install(Q); + await h.publish(P, SHARED); + // The registry holds one owner per object name, so Q's write-through is + // refused (and logged) at save time; Q's row is stored. + await h.publish(Q, SHARED); + await h.insertRecord(SHARED, 'kept'); + expect(await h.storedBindings(SHARED), 'precondition: both packages hold the object').toEqual([P, Q]); + expect(await h.hasTable(SHARED)).toBe(true); + expect(await h.records(SHARED)).toEqual(['kept']); + return h; +} + +/** Everything a refused uninstall must leave as it was. */ +async function expectUntouched(h: Process, others: Array) { + expect(await h.storedPackage(P), 'the sys_packages row of P').toBe(true); + expect((await h.call('GET', `/${P}`)).status, 'P is still installed in this process').toBe(200); + expect(await h.storedBindings(SHARED)).toEqual([P, ...others].sort((a, b) => String(a).localeCompare(String(b)))); + expect(await h.hasTable(SHARED)).toBe(true); + expect(await h.records(SHARED)).toEqual(['kept']); + // The running registry still serves P's definition of the object. + expect(h.engine.registry.getObject(SHARED)).toMatchObject({ label: `${SHARED} ${P}` }); +} + +/** After P's row of the shared object is gone, the running registry serves Q's. */ +const expectServedByQ = (h: Process) => + expect(h.engine.registry.getObject(SHARED)).toMatchObject({ label: `${SHARED} ${Q}` }); + +describe('[#23473] DELETE /packages/:id of a package whose object another stored row also holds', () => { + it('without keepData: refused 409 RESOURCE_CONFLICT before anything durable — the package, both rows, the table and its record stay', async () => { + const h = await twoPackageObject(); + + const answer = await h.call('DELETE', `/${P}`); + + expect({ status: answer.status, code: answer.code }, JSON.stringify(answer.body)).toEqual({ status: 409, code: 'RESOURCE_CONFLICT' }); + const message = String(answer.body?.error?.message); + // Located: the shared object and the row that keeps it; the way out. + expect(message).toContain(`'${SHARED}'`); + expect(message).toContain(Q); + expect(message).toContain('keepData'); + await expectUntouched(h, [Q]); + }, 60_000); + + it('a package-less row of the object counts as another holder', async () => { + const h = await boot(); + await h.install(P); + await h.publish(P, SHARED); + await h.seedUnboundRow(SHARED); + await h.insertRecord(SHARED, 'kept'); + expect(await h.storedBindings(SHARED)).toEqual([P, null]); + + const answer = await h.call('DELETE', `/${P}`); + + expect({ status: answer.status, code: answer.code }, JSON.stringify(answer.body)).toEqual({ status: 409, code: 'RESOURCE_CONFLICT' }); + await expectUntouched(h, [null]); + }, 60_000); + + it('the way out, keepData: the uninstall completes with nothing failed, and the shared table, Q\'s row and the record stay', async () => { + const h = await twoPackageObject(); + + const answer = await h.call('DELETE', `/${P}`, { keepData: 'true' }); + + expect(answer.status, JSON.stringify(answer.body)).toBe(200); + expect(answer.body?.data?.persisted).toMatchObject({ success: true, failedCount: 0, failed: [] }); + expect(await h.storedPackage(P)).toBe(false); + expect((await h.call('GET', `/${P}`)).status).toBe(404); + expect(await h.storedBindings(SHARED)).toEqual([Q]); + expect(await h.hasTable(SHARED)).toBe(true); + expect(await h.records(SHARED)).toEqual(['kept']); + expectServedByQ(h); + }, 60_000); + + it('the way out, P\'s own row first: the uninstall completes, keeps the shared table and still drops the table P alone held', async () => { + const h = await twoPackageObject(); + await h.publish(P, PRIVATE); + expect(await h.hasTable(PRIVATE)).toBe(true); + // The metadata door's delete of P's row, without dropStorage: the table + // stays for Q's row. + await (h.protocol as any).deleteMetaItem({ type: 'object', name: SHARED, packageId: P }); + expect(await h.storedBindings(SHARED)).toEqual([Q]); + + const answer = await h.call('DELETE', `/${P}`); + + expect(answer.status, JSON.stringify(answer.body)).toBe(200); + expect(answer.body?.data?.persisted).toMatchObject({ success: true, failedCount: 0, failed: [] }); + expect(await h.storedPackage(P)).toBe(false); + expect(await h.storedBindings(SHARED)).toEqual([Q]); + expect(await h.hasTable(SHARED)).toBe(true); + expect(await h.records(SHARED)).toEqual(['kept']); + expectServedByQ(h); + expect(await h.hasTable(PRIVATE)).toBe(false); + }, 60_000); + + it('CONTROL: an object one package holds — the uninstall removes its row and drops its table', async () => { + const h = await boot(); + await h.install(P); + await h.publish(P, SHARED); + await h.insertRecord(SHARED, 'gone'); + + const answer = await h.call('DELETE', `/${P}`); + + expect(answer.status, JSON.stringify(answer.body)).toBe(200); + expect(answer.body?.data?.persisted).toMatchObject({ success: true, failedCount: 0 }); + expect(await h.storedPackage(P)).toBe(false); + expect(await h.storedBindings(SHARED)).toEqual([]); + expect(await h.hasTable(SHARED)).toBe(false); + }, 60_000); +});