From ae35514bd95ecc2e7c0189d6f21ca52dbf307475 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 24 Jul 2026 16:54:33 +0000 Subject: [PATCH] =?UTF-8?q?feat(objectql):=20resolve=20file-field=20id=20r?= =?UTF-8?q?eferences=20on=20read=20=E2=80=94=20ADR-0104=20D3=20wave=202=20?= =?UTF-8?q?(PR-2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The engine read path resolves a file/image/avatar/video/audio value stored as an opaque sys_file id into its expanded FileValueSchema form ({id,name,size,mimeType,url}; url derived from /files/:fileId, never stored), via one batched sys_file `id $in […]` read (no N+1), mirroring lookup-$expand. Dual-mode safe: inline-blob objects pass through; only an id-shaped token (uuid/nanoid, never url-shaped) is a reference — a https/​/api/​/data:/blob: value a file field legitimately holds is never looked up (regression-tested). Fires zero reads unless a file field holds an id token, and no-ops when sys_file is unregistered — so the always-on step is free for objects that have not adopted references. Tests: 5 new engine cases (resolution, one-batched-query/no-N+1, blob+url dual-mode passthrough, url/data-URI never queried, non-committed skip); objectql 1068 green; field-zoo dogfood roundtrip green (12.6s, no bogus sys_file queries). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SHpGw3GBA9aFpfwVArRWfd --- .../adr-0104-d3w2-pr2-file-read-resolution.md | 22 ++++ packages/objectql/src/engine.test.ts | 103 +++++++++++++++ packages/objectql/src/engine.ts | 123 +++++++++++++++++- 3 files changed, 246 insertions(+), 2 deletions(-) create mode 100644 .changeset/adr-0104-d3w2-pr2-file-read-resolution.md diff --git a/.changeset/adr-0104-d3w2-pr2-file-read-resolution.md b/.changeset/adr-0104-d3w2-pr2-file-read-resolution.md new file mode 100644 index 000000000..6ad245f62 --- /dev/null +++ b/.changeset/adr-0104-d3w2-pr2-file-read-resolution.md @@ -0,0 +1,22 @@ +--- +"@objectstack/objectql": minor +--- + +feat(objectql): resolve file-field id references on read — ADR-0104 D3 wave 2 (PR-2) + +The engine read path now resolves a `file`/`image`/`avatar`/`video`/`audio` +value stored as an opaque `sys_file` id string into its expanded +`FileValueSchema` form — `{ id, name, size, mimeType, url }`, with `url` derived +from the stable `/api/v1/storage/files/:fileId` resolver (never stored). One +batched `sys_file` `id $in […]` read per query (no N+1), mirroring the +lookup-`$expand` batch pattern. + +**Dual-mode safe.** An inline-blob value (an object) passes through unchanged, +and only an **opaque id token** (uuid/nanoid-shaped) is treated as a reference — +a URL-shaped value (`https://…`, `/api/…`, `data:…`, `blob:…`), which a file +field legitimately holds in the legacy world, is never looked up. The step +fires zero reads unless a file field actually holds an id token (the blob/URL +case is free), and it no-ops entirely when `sys_file` is not registered. + +This makes a stored `fileId` (surfaced by PR-1) actually usable on read, ahead +of the v17 cutover that narrows the stored form to an id. diff --git a/packages/objectql/src/engine.test.ts b/packages/objectql/src/engine.test.ts index d8c3fec42..d4610bbb7 100644 --- a/packages/objectql/src/engine.test.ts +++ b/packages/objectql/src/engine.test.ts @@ -1173,6 +1173,109 @@ describe('ObjectQL Engine', () => { }); }); + describe('Resolve File References (ADR-0104 D3)', () => { + beforeEach(async () => { + engine.registerDriver(mockDriver, true); + await engine.init(); + }); + + const withSchema = () => { + vi.mocked(SchemaRegistry.getObject).mockImplementation((name) => { + if (name === 'doc') return { + name: 'doc', + fields: { + title: { type: 'text' }, + cover: { type: 'image' }, + attachments: { type: 'file', multiple: true }, + }, + } as any; + if (name === 'sys_file') return { + name: 'sys_file', + fields: { + name: { type: 'text' }, size: { type: 'number' }, + mime_type: { type: 'text' }, status: { type: 'text' }, + }, + } as any; + return undefined; + }); + }; + + it('resolves a stored fileId to the expanded FileValueSchema (url derived), in ONE batched query', async () => { + withSchema(); + vi.mocked(mockDriver.find) + .mockResolvedValueOnce([ + { id: 'd1', title: 'Doc 1', cover: 'file_a' }, + { id: 'd2', title: 'Doc 2', cover: 'file_b' }, + ]) + .mockResolvedValueOnce([ + { id: 'file_a', name: 'a.png', size: 10, mime_type: 'image/png', status: 'committed' }, + { id: 'file_b', name: 'b.png', size: 20, mime_type: 'image/png', status: 'committed' }, + ]); + + const result = await engine.find('doc', {}); + + expect(result[0].cover).toEqual({ id: 'file_a', name: 'a.png', size: 10, mimeType: 'image/png', url: '/api/v1/storage/files/file_a' }); + expect(result[1].cover).toEqual({ id: 'file_b', name: 'b.png', size: 20, mimeType: 'image/png', url: '/api/v1/storage/files/file_b' }); + + // No N+1: exactly one sys_file read, batched via id $in over both ids. + expect(mockDriver.find).toHaveBeenCalledTimes(2); + expect(mockDriver.find).toHaveBeenLastCalledWith( + 'sys_file', + expect.objectContaining({ where: { id: { $in: ['file_a', 'file_b'] } } }), + expect.objectContaining({ context: { __expandRead: true } }), + ); + }); + + it('leaves inline-blob values and non-matching strings untouched (dual-mode)', async () => { + withSchema(); + vi.mocked(mockDriver.find) + .mockResolvedValueOnce([ + { id: 'd1', cover: { url: 'https://cdn/x.png', alt: 'x' }, attachments: ['file_a', 'https://cdn/ext.pdf'] }, + ]) + .mockResolvedValueOnce([ + { id: 'file_a', name: 'a.pdf', status: 'committed' }, + ]); + + const result = await engine.find('doc', {}); + + expect(result[0].cover).toEqual({ url: 'https://cdn/x.png', alt: 'x' }); // inline blob → untouched + expect(result[0].attachments[0]).toEqual({ id: 'file_a', name: 'a.pdf', url: '/api/v1/storage/files/file_a' }); + expect(result[0].attachments[1]).toBe('https://cdn/ext.pdf'); // external url → passthrough + }); + + it('does NOT query sys_file when no file field holds a string (blob-only → zero cost)', async () => { + withSchema(); + vi.mocked(mockDriver.find).mockResolvedValueOnce([ + { id: 'd1', cover: { url: 'https://cdn/x.png' } }, + ]); + await engine.find('doc', {}); + expect(mockDriver.find).toHaveBeenCalledTimes(1); // primary read only + }); + + it('does NOT fire a sys_file query for url-shaped values (external url, /api path, data: URI)', async () => { + // Regression: a file field legitimately holds a URL string in the + // legacy/dual-mode world; only an opaque id token is a reference. + withSchema(); + vi.mocked(mockDriver.find).mockResolvedValueOnce([ + { id: 'd1', cover: 'https://cdn/a.png', attachments: ['/api/v1/storage/files/x', 'data:image/svg+xml,%3Csvg%3E'] }, + ]); + const result = await engine.find('doc', {}); + // Primary read only — no sys_file lookup for url-shaped strings. + expect(mockDriver.find).toHaveBeenCalledTimes(1); + expect(result[0].cover).toBe('https://cdn/a.png'); + expect(result[0].attachments).toEqual(['/api/v1/storage/files/x', 'data:image/svg+xml,%3Csvg%3E']); + }); + + it('does not resolve a non-committed (pending/deleted) sys_file row', async () => { + withSchema(); + vi.mocked(mockDriver.find) + .mockResolvedValueOnce([{ id: 'd1', cover: 'file_p' }]) + .mockResolvedValueOnce([{ id: 'file_p', name: 'p.png', status: 'pending' }]); + const result = await engine.find('doc', {}); + expect(result[0].cover).toBe('file_p'); // left as an opaque id + }); + }); + describe('Expand Related Records', () => { beforeEach(async () => { engine.registerDriver(mockDriver, true); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index b5df12c50..88d6b35b1 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -12,7 +12,7 @@ import { type DroppedFieldsEvent } from '@objectstack/spec/data'; import type { WriteObservabilityOptions } from '@objectstack/spec/contracts'; -import { parseAutonumberFormat, renderAutonumber, missingFieldValues, isTenancyDisabled } from '@objectstack/spec/data'; +import { parseAutonumberFormat, renderAutonumber, missingFieldValues, isTenancyDisabled, FILE_REFERENCE_TYPES } from '@objectstack/spec/data'; import { ExecutionContext, ExecutionContextSchema } from '@objectstack/spec/kernel'; import { IDataDriver, IDataEngine, Logger, createLogger, withTransientRetry, type RetryOptions } from '@objectstack/core'; import { SummaryRecomputeError, type SummaryRecomputeFailure } from './summary-errors.js'; @@ -2120,6 +2120,112 @@ export class ObjectQL implements IDataEngine { return records; } + /** + * Whether a value is an opaque `sys_file` id token — the minted uuid/nanoid + * form (letters, digits, `_`, `-`, bounded length), and crucially NOT a URL: + * a `https://…` / `/api/…` / `data:…` / `blob:…` file value carries `:`, `/` + * or `.` and is left untouched (it is a legacy/external URL, not a reference). + */ + private static readonly FILE_ID_RE = /^[A-Za-z0-9_-]{1,64}$/; + + /** + * Resolve file-field id references to their expanded `FileValueSchema` form + * (ADR-0104 D3 wave 2). A `file`/`image`/`avatar`/`video`/`audio` value + * stored as an opaque `sys_file` id string is enriched, in place, to + * `{ id, name, size, mimeType, url }` — `url` derived from the stable + * `/files/:fileId` resolver, never stored. + * + * DUAL-MODE SAFE: an inline-blob value (an object) and a string that does + * NOT match a committed `sys_file` row (e.g. an external url) pass through + * unchanged, so a field may hold either form during the pre-v17 window. + * + * Batched: at most one `sys_file` `id $in […]` read per call (no N+1); and + * zero reads when no field holds a string value (the blob-only case), so the + * step is free for objects that have not adopted references. + */ + private async resolveFileReferences( + objectName: string, + records: any[], + execCtx?: ExecutionContext, + ): Promise { + if (!records || records.length === 0) return records; + const objectSchema = this._registry.getObject(objectName); + if (!objectSchema || !objectSchema.fields) return records; + // Nothing to resolve against if the file object is not even registered + // (e.g. the storage plugin is absent) — skip without a failing query. + if (!this._registry.getObject('sys_file')) return records; + + const fileFields = Object.entries(objectSchema.fields) + .filter(([, def]: [string, any]) => def && FILE_REFERENCE_TYPES.has(def.type)) + .map(([name]) => name); + if (fileFields.length === 0) return records; + + // Collect candidate ids. A file field legitimately holds either an inline + // blob (an object — already the rich form, skipped) OR, in the dual-mode / + // legacy world, a URL string (`https://…`, `/api/…`, `data:…`, `blob:…`). + // Only an OPAQUE id token — the minted uuid/nanoid form, never url-shaped — + // is a `sys_file` reference to resolve. Filtering out url-shaped strings is + // what keeps a seeded `data:`/CDN image value from firing a bogus lookup. + const candidateIds: string[] = []; + const addCandidate = (v: unknown) => { + if (typeof v === 'string' && ObjectQL.FILE_ID_RE.test(v)) candidateIds.push(v); + }; + for (const record of records) { + for (const fieldName of fileFields) { + const val = record[fieldName]; + if (val == null) continue; + if (Array.isArray(val)) { + for (const v of val) addCandidate(v); + } else { + addCandidate(val); + } + } + } + const uniqueIds = [...new Set(candidateIds)]; + if (uniqueIds.length === 0) return records; // blob-only / empty → zero cost + + // One batched sys_file read. `__expandRead` mirrors the lookup-expansion + // sub-read (a system-built marker, never from client input). Metadata only — + // byte-download authorization is enforced at the /files/:fileId resolver. + let fileRows: any[] = []; + try { + fileRows = (await this.find( + 'sys_file', + { where: { id: { $in: uniqueIds } }, context: { ...(execCtx ?? {}), __expandRead: true } as ExecutionContext }, + )) ?? []; + } catch { + return records; // sys_file unregistered / unreadable — leave ids as-is + } + + const fileMap = new Map(); + for (const row of fileRows) { + if (row?.id != null && row.status === 'committed') fileMap.set(String(row.id), row); + } + if (fileMap.size === 0) return records; + + const toValue = (row: any) => ({ + id: String(row.id), + ...(row.name != null ? { name: row.name } : {}), + ...(row.size != null ? { size: row.size } : {}), + ...(row.mime_type != null ? { mimeType: row.mime_type } : {}), + url: `/api/v1/storage/files/${row.id}`, + }); + + for (const record of records) { + for (const fieldName of fileFields) { + const val = record[fieldName]; + if (val == null) continue; + if (Array.isArray(val)) { + record[fieldName] = val.map((v: any) => + typeof v === 'string' && fileMap.has(v) ? toValue(fileMap.get(v)) : v); + } else if (typeof val === 'string' && fileMap.has(val)) { + record[fieldName] = toValue(fileMap.get(val)); + } + } + } + return records; + } + // ============================================ // Data Access Methods (IDataEngine Interface) // ============================================ @@ -2240,7 +2346,14 @@ export class ObjectQL implements IDataEngine { if (ast.expand && Object.keys(ast.expand).length > 0 && Array.isArray(result)) { result = await this.expandRelatedRecords(object, result, ast.expand, 0, opCtx.context); } - + + // Post-process: resolve file-field id references to their expanded + // FileValueSchema form (ADR-0104 D3). Always-on but free unless a file + // field holds an id string; dual-mode-safe (blobs pass through). + if (Array.isArray(result)) { + result = await this.resolveFileReferences(object, result, opCtx.context); + } + hookContext.event = 'afterFind'; hookContext.result = result; await this.triggerHooks('afterFind', hookContext); @@ -2324,6 +2437,12 @@ export class ObjectQL implements IDataEngine { result = expanded[0]; } + // Post-process: resolve file-field id references (ADR-0104 D3). + if (result != null) { + const resolved = await this.resolveFileReferences(objectName, [result], opCtx.context); + result = resolved[0]; + } + hookContext.event = 'afterFind'; hookContext.result = result; await this.triggerHooks('afterFind', hookContext);