Skip to content

Commit bb639ae

Browse files
os-zhuangclaude
andcommitted
fix(rest,objectql): make single-item ?package= actually disambiguate (ADR-0048)
The prior commit threaded ?package= into the runtime dispatcher, but the live serving path (RestAPI + ObjectQL protocol) still dropped it for cacheable types: - rest-server: the cacheable single-item GET called getMetaItemCached (ETag keyed on type+name only) and never passed packageId, so a ?package= read on a doc/page/etc. was served package-blind. Now a ?package= read bypasses that cache and takes getMetaItem(type, name, packageId). - protocol.getMetaItem: forward packageId to the sys_metadata overlay query (package_id filter, prefer-local), to metadataService.get, and to registry.getItem. - metadata-facade: get() gained optional currentPackageId, forwarded to registry.getItem. Verified live: two packages (com.example.showcase, com.objectstack.studio) each shipping doc/showcase_index now resolve to their own item via ?package=<id>, and the doc viewer route /apps/:packageId/docs/:name renders the correct per-package content in the browser. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c895067 commit bb639ae

6 files changed

Lines changed: 145 additions & 41 deletions

File tree

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,30 @@
11
---
2+
"@objectstack/objectql": minor
3+
"@objectstack/rest": minor
24
"@objectstack/runtime": minor
35
"@objectstack/spec": minor
46
---
57

6-
feat(runtime): package-scoped single-item metadata resolution via `?package=` (ADR-0048)
8+
feat(metadata): package-scoped single-item resolution via `?package=` (ADR-0048)
79

8-
The REST single-item GET `/meta/:type/:name` now threads its `?package=` query
9-
into `getItem(type, name, currentPackageId)` (the package-scoped resolution
10-
added in the namespace-gate work). Previously only the *list* and the protocol
11-
path were package-aware; a single-item fetch was context-free.
10+
A single-item metadata GET (`/meta/:type/:name?package=<id>`) now resolves
11+
package-scoped (prefer-local): when two installed packages ship an item of the
12+
same `type`/`name`, the requester's own package wins. Previously only the *list*
13+
endpoint was package-aware; a single-item fetch was context-free, so a
14+
cross-package collision always resolved to whichever package registered first.
1215

13-
This lets a caller resolve one item scoped to a package — notably the doc
14-
viewer (`/apps/:packageId/docs/:name`), whose content lives only on the
15-
single-item endpoint. As a result, `doc` names no longer need a namespace
16-
prefix for uniqueness (the prefix becomes a recommended convention, like
17-
`page`/`dashboard`/`report`); `doc.zod` doc-comments updated accordingly.
16+
The fix threads `packageId` end-to-end:
17+
18+
- `@objectstack/rest` — the cacheable single-item path called `getMetaItemCached`
19+
(ETag keyed on type+name only) and dropped `?package=`. A `?package=` read now
20+
bypasses that cache and takes the disambiguating `getMetaItem(type, name,
21+
packageId)` path, so two same-named items never share one cache entry.
22+
- `@objectstack/objectql``protocol.getMetaItem` forwards `packageId` to the
23+
overlay query (`sys_metadata.package_id`), `MetadataFacade.get`, and
24+
`registry.getItem`; `MetadataFacade.get` gained an optional `currentPackageId`.
25+
- `@objectstack/runtime` — the parallel HTTP dispatcher threads `?package=` too.
26+
27+
This lets the doc viewer (`/apps/:packageId/docs/:name`) resolve one doc scoped
28+
to its app, so `doc` names no longer need a namespace prefix for uniqueness (the
29+
prefix becomes a recommended convention, like `page`/`dashboard`/`report`);
30+
`doc.zod` doc-comments updated accordingly.

packages/objectql/src/metadata-facade.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,16 @@ export class MetadataFacade {
4040
}
4141

4242
/**
43-
* Get a metadata item by type and name
43+
* Get a metadata item by type and name.
44+
*
45+
* `currentPackageId` (ADR-0048) opts into package-scoped resolution: when two
46+
* installed packages ship an item of the same `type`/`name`, the registry
47+
* prefers the one owned by `currentPackageId` (composite key
48+
* `${packageId}:${name}`) before falling back to first-match. Omit it for the
49+
* legacy context-free lookup.
4450
*/
45-
async get(type: string, name: string): Promise<any> {
46-
const item = this.registry.getItem(type, name) as any;
51+
async get(type: string, name: string, currentPackageId?: string): Promise<any> {
52+
const item = this.registry.getItem(type, name, currentPackageId) as any;
4753
return item?.content ?? item;
4854
}
4955

packages/objectql/src/protocol-meta.test.ts

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -628,7 +628,9 @@ describe('ObjectStackProtocolImplementation - Metadata Persistence', () => {
628628
const result = await protocolWithService.getMetaItem({ type: 'view', name: 'case' });
629629

630630
expect(result.item).toMatchObject(fresh);
631-
expect(metadataService.get).toHaveBeenCalledWith('view', 'case');
631+
// The third arg is the package-scoped resolution hint (ADR-0048),
632+
// undefined here since the request carries no packageId.
633+
expect(metadataService.get).toHaveBeenCalledWith('view', 'case', undefined);
632634
});
633635

634636
it('falls back to SchemaRegistry when MetadataService returns undefined', async () => {
@@ -651,6 +653,42 @@ describe('ObjectStackProtocolImplementation - Metadata Persistence', () => {
651653
expect(result.item).toMatchObject(fromRegistry);
652654
});
653655

656+
it('resolves a same-name collision to the requesting package (ADR-0048 prefer-local)', async () => {
657+
// Two installed packages each ship `doc/intro`; the registry stores
658+
// them under composite keys `${packageId}:intro`. A single-item GET
659+
// carrying `packageId` must resolve to that package's own item.
660+
registry.registerItem('doc', { name: 'intro', content: '# Studio' }, 'name' as any, 'com.objectstack.studio');
661+
registry.registerItem('doc', { name: 'intro', content: '# Setup' }, 'name' as any, 'com.objectstack.setup');
662+
663+
mockEngine.findOne.mockResolvedValue(null); // no sys_metadata overlay
664+
665+
const studio = await protocol.getMetaItem({ type: 'doc', name: 'intro', packageId: 'com.objectstack.studio' });
666+
const setup = await protocol.getMetaItem({ type: 'doc', name: 'intro', packageId: 'com.objectstack.setup' });
667+
668+
expect((studio.item as any).content).toBe('# Studio');
669+
expect((setup.item as any).content).toBe('# Setup');
670+
});
671+
672+
it('package-scoped overlay read prefers the row owned by the requesting package (ADR-0048)', async () => {
673+
// sys_metadata holds two `doc/intro` rows differing only by package_id.
674+
// The overlay lookup must filter on package_id when one is supplied.
675+
const rows: Record<string, any> = {
676+
'com.objectstack.studio': { type: 'doc', name: 'intro', state: 'active', package_id: 'com.objectstack.studio', metadata: { name: 'intro', content: '# Studio overlay' } },
677+
'com.objectstack.setup': { type: 'doc', name: 'intro', state: 'active', package_id: 'com.objectstack.setup', metadata: { name: 'intro', content: '# Setup overlay' } },
678+
};
679+
// findOne(collection, query) — the package-scoped query carries
680+
// `package_id`; the package-less fallback query must miss (null).
681+
mockEngine.findOne.mockImplementation(async (_collection: string, query: any) =>
682+
query?.where?.package_id ? (rows[query.where.package_id] ?? null) : null,
683+
);
684+
685+
const studio = await protocol.getMetaItem({ type: 'doc', name: 'intro', packageId: 'com.objectstack.studio' });
686+
const setup = await protocol.getMetaItem({ type: 'doc', name: 'intro', packageId: 'com.objectstack.setup' });
687+
688+
expect((studio.item as any).content).toBe('# Studio overlay');
689+
expect((setup.item as any).content).toBe('# Setup overlay');
690+
});
691+
654692
it('should parse metadata JSON string from DB record', async () => {
655693
const complexData = { name: 'complex', nested: { value: 42 } };
656694
mockEngine.findOne.mockResolvedValue({

packages/objectql/src/protocol.ts

Lines changed: 41 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,16 +1412,23 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
14121412
if (request.previewDrafts && readState !== 'draft') {
14131413
try {
14141414
const findDraft = async (oid: string | null): Promise<any | undefined> => {
1415-
const rec = await this.engine.findOne('sys_metadata', {
1416-
where: { type: request.type, name: request.name, state: 'draft', organization_id: oid },
1417-
});
1415+
// ADR-0048 prefer-local (parity with the active-read overlay below).
1416+
const lookup = async (t: string): Promise<any | undefined> => {
1417+
const base: Record<string, unknown> = {
1418+
type: t, name: request.name, state: 'draft', organization_id: oid,
1419+
};
1420+
if (request.packageId) {
1421+
const scoped = await this.engine.findOne('sys_metadata', {
1422+
where: { ...base, package_id: request.packageId },
1423+
});
1424+
if (scoped) return scoped;
1425+
}
1426+
return await this.engine.findOne('sys_metadata', { where: base });
1427+
};
1428+
const rec = await lookup(request.type);
14181429
if (rec) return rec;
14191430
const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type];
1420-
if (alt) {
1421-
return await this.engine.findOne('sys_metadata', {
1422-
where: { type: alt, name: request.name, state: 'draft', organization_id: oid },
1423-
});
1424-
}
1431+
if (alt) return await lookup(alt);
14251432
return undefined;
14261433
};
14271434
const draftRec = (orgId ? await findDraft(orgId) : undefined) ?? await findDraft(null);
@@ -1447,24 +1454,29 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
14471454
// through to the in-memory registry / MetadataService.
14481455
try {
14491456
const findOverlay = async (oid: string | null): Promise<any | undefined> => {
1450-
const where: Record<string, unknown> = {
1451-
type: request.type,
1452-
name: request.name,
1453-
state: readState,
1454-
organization_id: oid,
1455-
};
1456-
const rec = await this.engine.findOne('sys_metadata', { where });
1457-
if (rec) return rec;
1458-
const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type];
1459-
if (alt) {
1460-
const altWhere: Record<string, unknown> = {
1461-
type: alt,
1457+
// ADR-0048 prefer-local: when a package id is supplied and two
1458+
// installed packages ship the same type/name, prefer the row owned
1459+
// by that package before falling back to first-match (package-less
1460+
// query). This mirrors `SchemaRegistry.getItem(type, name, pkg)`.
1461+
const lookup = async (t: string): Promise<any | undefined> => {
1462+
const base: Record<string, unknown> = {
1463+
type: t,
14621464
name: request.name,
14631465
state: readState,
14641466
organization_id: oid,
14651467
};
1466-
return await this.engine.findOne('sys_metadata', { where: altWhere });
1467-
}
1468+
if (request.packageId) {
1469+
const scoped = await this.engine.findOne('sys_metadata', {
1470+
where: { ...base, package_id: request.packageId },
1471+
});
1472+
if (scoped) return scoped;
1473+
}
1474+
return await this.engine.findOne('sys_metadata', { where: base });
1475+
};
1476+
const rec = await lookup(request.type);
1477+
if (rec) return rec;
1478+
const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type];
1479+
if (alt) return await lookup(alt);
14681480
return undefined;
14691481
};
14701482
const record = (orgId ? await findOverlay(orgId) : undefined)
@@ -1517,13 +1529,16 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
15171529
const services = this.getServicesRegistry?.();
15181530
const metadataService = services?.get('metadata');
15191531
if (metadataService && typeof metadataService.get === 'function') {
1520-
const fromService = await metadataService.get(request.type, request.name);
1532+
// Thread the caller's package id (ADR-0048) so a single-item
1533+
// fetch is package-scoped: when two installed packages ship the
1534+
// same type/name, the facade prefers the requester's own item.
1535+
const fromService = await metadataService.get(request.type, request.name, request.packageId);
15211536
if (fromService !== undefined && fromService !== null) {
15221537
item = fromService;
15231538
} else {
15241539
const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type];
15251540
if (alt) {
1526-
const altFromService = await metadataService.get(alt, request.name);
1541+
const altFromService = await metadataService.get(alt, request.name, request.packageId);
15271542
if (altFromService !== undefined && altFromService !== null) {
15281543
item = altFromService;
15291544
}
@@ -1548,10 +1563,10 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
15481563
// `GET /api/v1/meta/object/<name>` 404 even though the object is
15491564
// registered and visible via the list endpoint.
15501565
if (item === undefined) {
1551-
item = this.engine.registry.getItem(request.type, request.name);
1566+
item = this.engine.registry.getItem(request.type, request.name, request.packageId);
15521567
if (item === undefined) {
15531568
const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type];
1554-
if (alt) item = this.engine.registry.getItem(alt, request.name);
1569+
if (alt) item = this.engine.registry.getItem(alt, request.name, request.packageId);
15551570
}
15561571
}
15571572

packages/rest/src/rest-server.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1943,7 +1943,17 @@ export class RestServer {
19431943
// the preview to the stale published world.
19441944
const previewDrafts = typeof req.query?.preview === 'string'
19451945
&& req.query.preview.toLowerCase() === 'draft';
1946-
if (metadata.enableCache && p.getMetaItemCached && !isAppType && !isDraftRead && !previewDrafts) {
1946+
// ADR-0048 — a `?package=` read is package-scoped
1947+
// (prefer-local). The cached path keys ETags on
1948+
// type+name only and does NOT thread `packageId` into
1949+
// `getMetaItemCached`, so two installed packages shipping
1950+
// the same type/name would share one cache entry and the
1951+
// scope hint would be silently dropped. Bypass the cache
1952+
// when a package scope is requested so the disambiguating
1953+
// `getMetaItem(type, name, packageId)` path runs.
1954+
const packageScoped = typeof req.query?.package === 'string'
1955+
&& req.query.package.length > 0;
1956+
if (metadata.enableCache && p.getMetaItemCached && !isAppType && !isDraftRead && !previewDrafts && !packageScoped) {
19471957
const cacheRequest = {
19481958
ifNoneMatch: req.headers['if-none-match'] as string,
19491959
ifModifiedSince: req.headers['if-modified-since'] as string,

packages/rest/src/rest.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,28 @@ describe('RestServer', () => {
558558
expect.objectContaining({ type: 'object', name: 'lead', previewDrafts: true }),
559559
);
560560
});
561+
562+
it('GET /meta/:type/:name?package= bypasses the cache and threads packageId (ADR-0048)', async () => {
563+
// A `?package=` read is package-scoped (prefer-local). The cached path
564+
// keys ETags on type+name only and drops packageId, so it must be skipped
565+
// when a package scope is requested — otherwise two installed packages
566+
// shipping the same type/name share one cache entry and the scope hint is
567+
// silently lost.
568+
protocol.getMetaItemCached = vi.fn();
569+
const rest = new RestServer(server as any, protocol as any);
570+
rest.registerRoutes();
571+
const route = getMetaRoute(rest, 'GET', '/api/v1/meta/:type/:name');
572+
expect(route).toBeDefined();
573+
574+
await route!.handler(
575+
{ params: { type: 'doc', name: 'intro' }, query: { package: 'com.objectstack.studio' }, headers: {} },
576+
mockRes(),
577+
);
578+
expect(protocol.getMetaItemCached).not.toHaveBeenCalled();
579+
expect(protocol.getMetaItem).toHaveBeenCalledWith(
580+
expect.objectContaining({ type: 'doc', name: 'intro', packageId: 'com.objectstack.studio' }),
581+
);
582+
});
561583
});
562584

563585
describe('findData handler expand/populate forwarding', () => {

0 commit comments

Comments
 (0)