Skip to content

Commit 551f899

Browse files
huangyiireneclaude
andauthored
fix(objectql): uninstalling a package removes the non-object metadata it shipped (#7221) (#7961)
`MetadataFacade.unregisterPackage` and `SchemaRegistry.uninstallPackage` both reached only `objectContributors`, so every non-object item a package shipped — `page`, `view`, `flow`, `app`, `api` … — stayed registered and fully resolvable after the package was gone, and the facade additionally orphaned the generic-map half of the package's objects. Measured before writing: after `uninstallPackage('crm')` the package record was gone while `getItem('page', 'home')` kept serving the uninstalled package's page and `metadata.get('flow')` still held `crm:onboard`. Both callers share the gap, which is why the new verb sits on `SchemaRegistry` rather than privately on the facade — a facade-local scan would have been a second expression of the same package-ownership rule and would have left the registry-direct caller half-done. `unregisterItemsByPackage(packageId)` is scoped to composite `${packageId}:${name}` keys, the exact inverse of `registerItem`'s construction. Bare-key ADR-0005 runtime/DB overlays are deliberately kept — they are tenant-authored and an uninstall does not delete tenant data — and the orphan that leaves behind is made loud rather than silently deleted or silently kept, following ADR-0029 D9.5. What nothing yet does with that report is filed as #7951. In both callers the sweep runs after the object verb, which can refuse on ADR-0029 extenders, so a refused uninstall removes nothing at all. Claude-Session: https://claude.ai/code/session_01BtbjfNQKy7DXqNF7Y26vmP Co-authored-by: Claude <noreply@anthropic.com>
1 parent 627e65a commit 551f899

4 files changed

Lines changed: 514 additions & 1 deletion

File tree

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
fix(objectql): uninstalling a package now removes the non-object metadata it shipped (#7221)
6+
7+
"Unregister all metadata from a package" reached only `objectContributors`, so
8+
every non-object item a package shipped — its `page`, `view`, `flow`, `app`,
9+
`api` … — stayed registered and fully resolvable after the package was gone.
10+
Not a stale-cache nuisance: an uninstall that leaves the package's UI and API
11+
metadata installed.
12+
13+
A package writes into two stores. `SchemaRegistry.unregisterObjectsByPackage`
14+
walks the contributor list; everything else lives in the generic `metadata` map
15+
under the composite `${packageId}:${name}` key `registerItem` builds, and no
16+
verb removed those. Measured on the real registry: after
17+
`uninstallPackage('crm')` the package record was gone while
18+
`getItem('page', 'home')` kept serving the uninstalled package's page and
19+
`metadata.get('flow')` still held `crm:onboard`, for the life of the process.
20+
The same call through `MetadataFacade.unregisterPackage` additionally left the
21+
generic-map half of the package's objects behind as a genuine orphan.
22+
23+
`SchemaRegistry.unregisterItemsByPackage(packageId)` is the missing verb, and it
24+
sits on the registry rather than privately on the facade because **both** callers
25+
were measured to have the gap — `uninstallPackage` is registry-direct and shares
26+
it exactly. A private copy in the facade would have been a second expression of
27+
the same package-ownership rule, and would have left every registry-direct
28+
uninstall still half-done. Membership is the exact inverse of the construction in
29+
`registerItem`, so a discriminated type's whole i18n bundle leaves with the
30+
package that shipped it, and a scoped package id (`@acme/crm`) is handled by the
31+
same relation.
32+
33+
**Tenant overlays are deliberately kept.** A bare-key entry is the ADR-0005
34+
runtime/DB overlay slot — a tenant's own customization, carrying no package
35+
provenance and with no separate contributor list holding a durable copy. An
36+
uninstall that deleted it would take tenant-authored data along with the package
37+
it merely overlaid, so the sweep is scoped to composite keys only. The
38+
consequence — an overlay that now layers over nothing — is made **loud** rather
39+
than silently deleted or silently kept, the same house pattern as ADR-0029
40+
D9.5's orphan-overlay violation: the verb warns naming every orphan it left and
41+
returns them as `orphanedOverlays` for a caller that wants to act. What nothing
42+
yet does with that report is filed separately as #7951.
43+
44+
This is deliberately **not** the object-side D9.7 rule ("an overlay layer leaves
45+
with the base it layers over"), which is safe only because an object overlay
46+
layer is a runtime projection of a `sys_metadata` row the removal does not touch.
47+
48+
Ordering: in both callers the item sweep runs after the object verb, because that
49+
one can refuse (ADR-0029 extenders) — a refused uninstall removes nothing at all.
50+
51+
Unaffected: another package's same-named items (including a package id that is a
52+
string prefix of another), runtime-authored items with no package, and the
53+
persisted `sys_metadata` rows — a distinct mechanism this change does not reach
54+
into.

packages/objectql/src/metadata-facade.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,10 +258,33 @@ export class MetadataFacade {
258258
}
259259

260260
/**
261-
* Unregister all metadata from a package
261+
* Unregister all metadata from a package.
262+
*
263+
* [#7221] Both stores, for the same reason {@link register} and
264+
* {@link unregister} reach both: `unregisterObjectsByPackage` walks
265+
* `objectContributors` alone, so this verb — whose `IMetadataService`
266+
* contract reads "Unregister all metadata items from a specific package" —
267+
* used to leave every non-object item the package shipped (`page`, `view`,
268+
* `flow`, `app`, `api` …) fully resolvable through this class's own `get`,
269+
* `list`, `listNames` and `exists`, plus the generic-map half of its
270+
* objects, which {@link registerObjectBothPlaces} writes. A half-uninstall,
271+
* silently.
272+
*
273+
* `SchemaRegistry.unregisterItemsByPackage` is the registry-side verb rather
274+
* than a scan private to this class, because `SchemaRegistry.uninstallPackage`
275+
* was measured to have the identical gap — a second copy of the
276+
* package-ownership rule here would be the #6808 drift, and would have left
277+
* the registry-direct caller half-done. It deliberately keeps bare-key
278+
* ADR-0005 runtime/DB overlays and warns about the ones it orphans; see its
279+
* header for why that is loudness rather than a silent delete.
280+
*
281+
* Ordering mirrors {@link unregister}: the object verb runs first because it
282+
* is the half that can refuse (ADR-0029 extenders), so a refusal removes
283+
* nothing at all rather than taking the generic half with it.
262284
*/
263285
async unregisterPackage(packageName: string): Promise<void> {
264286
this.registry.unregisterObjectsByPackage(packageName);
287+
this.registry.unregisterItemsByPackage(packageName);
265288
}
266289

267290
/**

0 commit comments

Comments
 (0)