diff --git a/.changeset/adr-0048-meta-getitem-package.md b/.changeset/adr-0048-meta-getitem-package.md new file mode 100644 index 0000000000..c77275c7e1 --- /dev/null +++ b/.changeset/adr-0048-meta-getitem-package.md @@ -0,0 +1,30 @@ +--- +"@objectstack/objectql": minor +"@objectstack/rest": minor +"@objectstack/runtime": minor +"@objectstack/spec": minor +--- + +feat(metadata): package-scoped single-item resolution via `?package=` (ADR-0048) + +A single-item metadata GET (`/meta/:type/:name?package=`) now resolves +package-scoped (prefer-local): when two installed packages ship an item of the +same `type`/`name`, the requester's own package wins. Previously only the *list* +endpoint was package-aware; a single-item fetch was context-free, so a +cross-package collision always resolved to whichever package registered first. + +The fix threads `packageId` end-to-end: + +- `@objectstack/rest` — the cacheable single-item path called `getMetaItemCached` + (ETag keyed on type+name only) and dropped `?package=`. A `?package=` read now + bypasses that cache and takes the disambiguating `getMetaItem(type, name, + packageId)` path, so two same-named items never share one cache entry. +- `@objectstack/objectql` — `protocol.getMetaItem` forwards `packageId` to the + overlay query (`sys_metadata.package_id`), `MetadataFacade.get`, and + `registry.getItem`; `MetadataFacade.get` gained an optional `currentPackageId`. +- `@objectstack/runtime` — the parallel HTTP dispatcher threads `?package=` too. + +This lets the doc viewer (`/apps/:packageId/docs/:name`) resolve one doc scoped +to its app, so `doc` names no longer need a namespace prefix for uniqueness (the +prefix becomes a recommended convention, like `page`/`dashboard`/`report`); +`doc.zod` doc-comments updated accordingly. diff --git a/packages/objectql/src/metadata-facade.ts b/packages/objectql/src/metadata-facade.ts index 61abb1420d..18f25129f0 100644 --- a/packages/objectql/src/metadata-facade.ts +++ b/packages/objectql/src/metadata-facade.ts @@ -40,10 +40,16 @@ export class MetadataFacade { } /** - * Get a metadata item by type and name + * Get a metadata item by type and name. + * + * `currentPackageId` (ADR-0048) opts into package-scoped resolution: when two + * installed packages ship an item of the same `type`/`name`, the registry + * prefers the one owned by `currentPackageId` (composite key + * `${packageId}:${name}`) before falling back to first-match. Omit it for the + * legacy context-free lookup. */ - async get(type: string, name: string): Promise { - const item = this.registry.getItem(type, name) as any; + async get(type: string, name: string, currentPackageId?: string): Promise { + const item = this.registry.getItem(type, name, currentPackageId) as any; return item?.content ?? item; } diff --git a/packages/objectql/src/protocol-meta.test.ts b/packages/objectql/src/protocol-meta.test.ts index 258b7373f9..49224cc3fa 100644 --- a/packages/objectql/src/protocol-meta.test.ts +++ b/packages/objectql/src/protocol-meta.test.ts @@ -628,7 +628,9 @@ describe('ObjectStackProtocolImplementation - Metadata Persistence', () => { const result = await protocolWithService.getMetaItem({ type: 'view', name: 'case' }); expect(result.item).toMatchObject(fresh); - expect(metadataService.get).toHaveBeenCalledWith('view', 'case'); + // The third arg is the package-scoped resolution hint (ADR-0048), + // undefined here since the request carries no packageId. + expect(metadataService.get).toHaveBeenCalledWith('view', 'case', undefined); }); it('falls back to SchemaRegistry when MetadataService returns undefined', async () => { @@ -651,6 +653,42 @@ describe('ObjectStackProtocolImplementation - Metadata Persistence', () => { expect(result.item).toMatchObject(fromRegistry); }); + it('resolves a same-name collision to the requesting package (ADR-0048 prefer-local)', async () => { + // Two installed packages each ship `doc/intro`; the registry stores + // them under composite keys `${packageId}:intro`. A single-item GET + // carrying `packageId` must resolve to that package's own item. + registry.registerItem('doc', { name: 'intro', content: '# Studio' }, 'name' as any, 'com.objectstack.studio'); + registry.registerItem('doc', { name: 'intro', content: '# Setup' }, 'name' as any, 'com.objectstack.setup'); + + mockEngine.findOne.mockResolvedValue(null); // no sys_metadata overlay + + const studio = await protocol.getMetaItem({ type: 'doc', name: 'intro', packageId: 'com.objectstack.studio' }); + const setup = await protocol.getMetaItem({ type: 'doc', name: 'intro', packageId: 'com.objectstack.setup' }); + + expect((studio.item as any).content).toBe('# Studio'); + expect((setup.item as any).content).toBe('# Setup'); + }); + + it('package-scoped overlay read prefers the row owned by the requesting package (ADR-0048)', async () => { + // sys_metadata holds two `doc/intro` rows differing only by package_id. + // The overlay lookup must filter on package_id when one is supplied. + const rows: Record = { + 'com.objectstack.studio': { type: 'doc', name: 'intro', state: 'active', package_id: 'com.objectstack.studio', metadata: { name: 'intro', content: '# Studio overlay' } }, + 'com.objectstack.setup': { type: 'doc', name: 'intro', state: 'active', package_id: 'com.objectstack.setup', metadata: { name: 'intro', content: '# Setup overlay' } }, + }; + // findOne(collection, query) — the package-scoped query carries + // `package_id`; the package-less fallback query must miss (null). + mockEngine.findOne.mockImplementation(async (_collection: string, query: any) => + query?.where?.package_id ? (rows[query.where.package_id] ?? null) : null, + ); + + const studio = await protocol.getMetaItem({ type: 'doc', name: 'intro', packageId: 'com.objectstack.studio' }); + const setup = await protocol.getMetaItem({ type: 'doc', name: 'intro', packageId: 'com.objectstack.setup' }); + + expect((studio.item as any).content).toBe('# Studio overlay'); + expect((setup.item as any).content).toBe('# Setup overlay'); + }); + it('should parse metadata JSON string from DB record', async () => { const complexData = { name: 'complex', nested: { value: 42 } }; mockEngine.findOne.mockResolvedValue({ diff --git a/packages/objectql/src/protocol.ts b/packages/objectql/src/protocol.ts index 282c95b04e..07caf2a9d4 100644 --- a/packages/objectql/src/protocol.ts +++ b/packages/objectql/src/protocol.ts @@ -1412,16 +1412,23 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { if (request.previewDrafts && readState !== 'draft') { try { const findDraft = async (oid: string | null): Promise => { - const rec = await this.engine.findOne('sys_metadata', { - where: { type: request.type, name: request.name, state: 'draft', organization_id: oid }, - }); + // ADR-0048 prefer-local (parity with the active-read overlay below). + const lookup = async (t: string): Promise => { + const base: Record = { + type: t, name: request.name, state: 'draft', organization_id: oid, + }; + if (request.packageId) { + const scoped = await this.engine.findOne('sys_metadata', { + where: { ...base, package_id: request.packageId }, + }); + if (scoped) return scoped; + } + return await this.engine.findOne('sys_metadata', { where: base }); + }; + const rec = await lookup(request.type); if (rec) return rec; const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type]; - if (alt) { - return await this.engine.findOne('sys_metadata', { - where: { type: alt, name: request.name, state: 'draft', organization_id: oid }, - }); - } + if (alt) return await lookup(alt); return undefined; }; const draftRec = (orgId ? await findDraft(orgId) : undefined) ?? await findDraft(null); @@ -1447,24 +1454,29 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { // through to the in-memory registry / MetadataService. try { const findOverlay = async (oid: string | null): Promise => { - const where: Record = { - type: request.type, - name: request.name, - state: readState, - organization_id: oid, - }; - const rec = await this.engine.findOne('sys_metadata', { where }); - if (rec) return rec; - const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type]; - if (alt) { - const altWhere: Record = { - type: alt, + // ADR-0048 prefer-local: when a package id is supplied and two + // installed packages ship the same type/name, prefer the row owned + // by that package before falling back to first-match (package-less + // query). This mirrors `SchemaRegistry.getItem(type, name, pkg)`. + const lookup = async (t: string): Promise => { + const base: Record = { + type: t, name: request.name, state: readState, organization_id: oid, }; - return await this.engine.findOne('sys_metadata', { where: altWhere }); - } + if (request.packageId) { + const scoped = await this.engine.findOne('sys_metadata', { + where: { ...base, package_id: request.packageId }, + }); + if (scoped) return scoped; + } + return await this.engine.findOne('sys_metadata', { where: base }); + }; + const rec = await lookup(request.type); + if (rec) return rec; + const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type]; + if (alt) return await lookup(alt); return undefined; }; const record = (orgId ? await findOverlay(orgId) : undefined) @@ -1517,13 +1529,16 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { const services = this.getServicesRegistry?.(); const metadataService = services?.get('metadata'); if (metadataService && typeof metadataService.get === 'function') { - const fromService = await metadataService.get(request.type, request.name); + // Thread the caller's package id (ADR-0048) so a single-item + // fetch is package-scoped: when two installed packages ship the + // same type/name, the facade prefers the requester's own item. + const fromService = await metadataService.get(request.type, request.name, request.packageId); if (fromService !== undefined && fromService !== null) { item = fromService; } else { const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type]; if (alt) { - const altFromService = await metadataService.get(alt, request.name); + const altFromService = await metadataService.get(alt, request.name, request.packageId); if (altFromService !== undefined && altFromService !== null) { item = altFromService; } @@ -1548,10 +1563,10 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { // `GET /api/v1/meta/object/` 404 even though the object is // registered and visible via the list endpoint. if (item === undefined) { - item = this.engine.registry.getItem(request.type, request.name); + item = this.engine.registry.getItem(request.type, request.name, request.packageId); if (item === undefined) { const alt = PLURAL_TO_SINGULAR[request.type] ?? SINGULAR_TO_PLURAL[request.type]; - if (alt) item = this.engine.registry.getItem(alt, request.name); + if (alt) item = this.engine.registry.getItem(alt, request.name, request.packageId); } } diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index bc3cdd8ce0..c2eb1b6d41 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -1943,7 +1943,17 @@ export class RestServer { // the preview to the stale published world. const previewDrafts = typeof req.query?.preview === 'string' && req.query.preview.toLowerCase() === 'draft'; - if (metadata.enableCache && p.getMetaItemCached && !isAppType && !isDraftRead && !previewDrafts) { + // ADR-0048 — a `?package=` read is package-scoped + // (prefer-local). The cached path keys ETags on + // type+name only and does NOT thread `packageId` into + // `getMetaItemCached`, so two installed packages shipping + // the same type/name would share one cache entry and the + // scope hint would be silently dropped. Bypass the cache + // when a package scope is requested so the disambiguating + // `getMetaItem(type, name, packageId)` path runs. + const packageScoped = typeof req.query?.package === 'string' + && req.query.package.length > 0; + if (metadata.enableCache && p.getMetaItemCached && !isAppType && !isDraftRead && !previewDrafts && !packageScoped) { const cacheRequest = { ifNoneMatch: req.headers['if-none-match'] as string, ifModifiedSince: req.headers['if-modified-since'] as string, diff --git a/packages/rest/src/rest.test.ts b/packages/rest/src/rest.test.ts index e5279e48bc..1e8f4fdc55 100644 --- a/packages/rest/src/rest.test.ts +++ b/packages/rest/src/rest.test.ts @@ -558,6 +558,28 @@ describe('RestServer', () => { expect.objectContaining({ type: 'object', name: 'lead', previewDrafts: true }), ); }); + + it('GET /meta/:type/:name?package= bypasses the cache and threads packageId (ADR-0048)', async () => { + // A `?package=` read is package-scoped (prefer-local). The cached path + // keys ETags on type+name only and drops packageId, so it must be skipped + // when a package scope is requested — otherwise two installed packages + // shipping the same type/name share one cache entry and the scope hint is + // silently lost. + protocol.getMetaItemCached = vi.fn(); + const rest = new RestServer(server as any, protocol as any); + rest.registerRoutes(); + const route = getMetaRoute(rest, 'GET', '/api/v1/meta/:type/:name'); + expect(route).toBeDefined(); + + await route!.handler( + { params: { type: 'doc', name: 'intro' }, query: { package: 'com.objectstack.studio' }, headers: {} }, + mockRes(), + ); + expect(protocol.getMetaItemCached).not.toHaveBeenCalled(); + expect(protocol.getMetaItem).toHaveBeenCalledWith( + expect.objectContaining({ type: 'doc', name: 'intro', packageId: 'com.objectstack.studio' }), + ); + }); }); describe('findData handler expand/populate forwarding', () => { diff --git a/packages/runtime/src/http-dispatcher.ts b/packages/runtime/src/http-dispatcher.ts index 83548f8415..f93054b11e 100644 --- a/packages/runtime/src/http-dispatcher.ts +++ b/packages/runtime/src/http-dispatcher.ts @@ -1173,7 +1173,9 @@ export class HttpDispatcher { const metaSvc = await this.resolveService('metadata', _context.environmentId); if (metaSvc && typeof (metaSvc as any).getItem === 'function') { try { - const data = await (metaSvc as any).getItem(singularType, name); + // ADR-0048 — thread `?package=` so single-item resolution is + // package-scoped (prefer-local), matching list resolution. + const data = await (metaSvc as any).getItem(singularType, name, packageId); if (data) return { handled: true, response: this.success(data) }; } catch { /* not found */ } } diff --git a/packages/spec/src/system/doc.zod.ts b/packages/spec/src/system/doc.zod.ts index 2a29caf5f0..88a004fe64 100644 --- a/packages/spec/src/system/doc.zod.ts +++ b/packages/spec/src/system/doc.zod.ts @@ -12,12 +12,13 @@ import { lazySchema } from '../shared/lazy-schema'; * build time; TS-first stacks may also declare items inline via * `defineStack({ docs: [...] })`. * - * Identity model: `name` = filename stem and MUST carry the package - * namespace prefix (`crm_lead_guide.md` → `crm_lead_guide`). Unlike object - * names (validated in the kernel because they map to physical table - * names), doc-name prefixing is a *logical* uniqueness requirement — the - * metadata registry key carries no package coordinate — so it is enforced - * by build/publish lint, not by this schema. + * Identity model: `name` = filename stem (lowercase snake_case). A namespace + * prefix (`crm_lead_guide`) is a *recommended convention*, no longer required: + * per ADR-0048, single-doc resolution is package-scoped (`getItem('doc', name, + * packageId)` via `?package=` on the detail route), so two packages may ship a + * doc with the same bare name and each resolves within its own package — just + * like `page`/`dashboard`/`report`. The prefix stays useful for readable, + * globally-unique filenames but is not load-bearing for uniqueness. * * Docs are inert data: the kernel registers them without parsing * `content`, and they participate in no runtime behavior. Renderers @@ -26,13 +27,14 @@ import { lazySchema } from '../shared/lazy-schema'; */ export const DocSchema = lazySchema(() => z.object({ /** - * Unique doc name; equals the source filename stem. Namespace-prefixed - * (e.g. `crm_lead_guide`) — see ADR-0046 §3.2 for the enforcement - * posture (build/publish lint, not kernel rejection). + * Doc name; equals the source filename stem. Lowercase snake_case. A + * namespace prefix (e.g. `crm_lead_guide`) is recommended for readable, + * globally-unique filenames but NOT required — single-doc resolution is + * package-scoped (ADR-0048), so bare names are unique within their package. */ name: z.string() .regex(/^[a-z][a-z0-9_]*$/, 'name must be lowercase snake_case') - .describe('Unique doc name (= filename stem, namespace-prefixed snake_case)'), + .describe('Doc name (= filename stem, snake_case; namespace prefix recommended, not required)'), /** * Display title. The CLI derives it from frontmatter `title:` or the