Skip to content

Commit ae35514

Browse files
committed
feat(objectql): resolve file-field id references on read — ADR-0104 D3 wave 2 (PR-2)
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SHpGw3GBA9aFpfwVArRWfd
1 parent 37b1346 commit ae35514

3 files changed

Lines changed: 246 additions & 2 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
"@objectstack/objectql": minor
3+
---
4+
5+
feat(objectql): resolve file-field id references on read — ADR-0104 D3 wave 2 (PR-2)
6+
7+
The engine read path now resolves a `file`/`image`/`avatar`/`video`/`audio`
8+
value stored as an opaque `sys_file` id string into its expanded
9+
`FileValueSchema` form — `{ id, name, size, mimeType, url }`, with `url` derived
10+
from the stable `/api/v1/storage/files/:fileId` resolver (never stored). One
11+
batched `sys_file` `id $in […]` read per query (no N+1), mirroring the
12+
lookup-`$expand` batch pattern.
13+
14+
**Dual-mode safe.** An inline-blob value (an object) passes through unchanged,
15+
and only an **opaque id token** (uuid/nanoid-shaped) is treated as a reference —
16+
a URL-shaped value (`https://…`, `/api/…`, `data:…`, `blob:…`), which a file
17+
field legitimately holds in the legacy world, is never looked up. The step
18+
fires zero reads unless a file field actually holds an id token (the blob/URL
19+
case is free), and it no-ops entirely when `sys_file` is not registered.
20+
21+
This makes a stored `fileId` (surfaced by PR-1) actually usable on read, ahead
22+
of the v17 cutover that narrows the stored form to an id.

packages/objectql/src/engine.test.ts

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1173,6 +1173,109 @@ describe('ObjectQL Engine', () => {
11731173
});
11741174
});
11751175

1176+
describe('Resolve File References (ADR-0104 D3)', () => {
1177+
beforeEach(async () => {
1178+
engine.registerDriver(mockDriver, true);
1179+
await engine.init();
1180+
});
1181+
1182+
const withSchema = () => {
1183+
vi.mocked(SchemaRegistry.getObject).mockImplementation((name) => {
1184+
if (name === 'doc') return {
1185+
name: 'doc',
1186+
fields: {
1187+
title: { type: 'text' },
1188+
cover: { type: 'image' },
1189+
attachments: { type: 'file', multiple: true },
1190+
},
1191+
} as any;
1192+
if (name === 'sys_file') return {
1193+
name: 'sys_file',
1194+
fields: {
1195+
name: { type: 'text' }, size: { type: 'number' },
1196+
mime_type: { type: 'text' }, status: { type: 'text' },
1197+
},
1198+
} as any;
1199+
return undefined;
1200+
});
1201+
};
1202+
1203+
it('resolves a stored fileId to the expanded FileValueSchema (url derived), in ONE batched query', async () => {
1204+
withSchema();
1205+
vi.mocked(mockDriver.find)
1206+
.mockResolvedValueOnce([
1207+
{ id: 'd1', title: 'Doc 1', cover: 'file_a' },
1208+
{ id: 'd2', title: 'Doc 2', cover: 'file_b' },
1209+
])
1210+
.mockResolvedValueOnce([
1211+
{ id: 'file_a', name: 'a.png', size: 10, mime_type: 'image/png', status: 'committed' },
1212+
{ id: 'file_b', name: 'b.png', size: 20, mime_type: 'image/png', status: 'committed' },
1213+
]);
1214+
1215+
const result = await engine.find('doc', {});
1216+
1217+
expect(result[0].cover).toEqual({ id: 'file_a', name: 'a.png', size: 10, mimeType: 'image/png', url: '/api/v1/storage/files/file_a' });
1218+
expect(result[1].cover).toEqual({ id: 'file_b', name: 'b.png', size: 20, mimeType: 'image/png', url: '/api/v1/storage/files/file_b' });
1219+
1220+
// No N+1: exactly one sys_file read, batched via id $in over both ids.
1221+
expect(mockDriver.find).toHaveBeenCalledTimes(2);
1222+
expect(mockDriver.find).toHaveBeenLastCalledWith(
1223+
'sys_file',
1224+
expect.objectContaining({ where: { id: { $in: ['file_a', 'file_b'] } } }),
1225+
expect.objectContaining({ context: { __expandRead: true } }),
1226+
);
1227+
});
1228+
1229+
it('leaves inline-blob values and non-matching strings untouched (dual-mode)', async () => {
1230+
withSchema();
1231+
vi.mocked(mockDriver.find)
1232+
.mockResolvedValueOnce([
1233+
{ id: 'd1', cover: { url: 'https://cdn/x.png', alt: 'x' }, attachments: ['file_a', 'https://cdn/ext.pdf'] },
1234+
])
1235+
.mockResolvedValueOnce([
1236+
{ id: 'file_a', name: 'a.pdf', status: 'committed' },
1237+
]);
1238+
1239+
const result = await engine.find('doc', {});
1240+
1241+
expect(result[0].cover).toEqual({ url: 'https://cdn/x.png', alt: 'x' }); // inline blob → untouched
1242+
expect(result[0].attachments[0]).toEqual({ id: 'file_a', name: 'a.pdf', url: '/api/v1/storage/files/file_a' });
1243+
expect(result[0].attachments[1]).toBe('https://cdn/ext.pdf'); // external url → passthrough
1244+
});
1245+
1246+
it('does NOT query sys_file when no file field holds a string (blob-only → zero cost)', async () => {
1247+
withSchema();
1248+
vi.mocked(mockDriver.find).mockResolvedValueOnce([
1249+
{ id: 'd1', cover: { url: 'https://cdn/x.png' } },
1250+
]);
1251+
await engine.find('doc', {});
1252+
expect(mockDriver.find).toHaveBeenCalledTimes(1); // primary read only
1253+
});
1254+
1255+
it('does NOT fire a sys_file query for url-shaped values (external url, /api path, data: URI)', async () => {
1256+
// Regression: a file field legitimately holds a URL string in the
1257+
// legacy/dual-mode world; only an opaque id token is a reference.
1258+
withSchema();
1259+
vi.mocked(mockDriver.find).mockResolvedValueOnce([
1260+
{ id: 'd1', cover: 'https://cdn/a.png', attachments: ['/api/v1/storage/files/x', 'data:image/svg+xml,%3Csvg%3E'] },
1261+
]);
1262+
const result = await engine.find('doc', {});
1263+
// Primary read only — no sys_file lookup for url-shaped strings.
1264+
expect(mockDriver.find).toHaveBeenCalledTimes(1);
1265+
expect(result[0].cover).toBe('https://cdn/a.png');
1266+
expect(result[0].attachments).toEqual(['/api/v1/storage/files/x', 'data:image/svg+xml,%3Csvg%3E']);
1267+
});
1268+
1269+
it('does not resolve a non-committed (pending/deleted) sys_file row', async () => {
1270+
withSchema();
1271+
vi.mocked(mockDriver.find)
1272+
.mockResolvedValueOnce([{ id: 'd1', cover: 'file_p' }])
1273+
.mockResolvedValueOnce([{ id: 'file_p', name: 'p.png', status: 'pending' }]);
1274+
const result = await engine.find('doc', {});
1275+
expect(result[0].cover).toBe('file_p'); // left as an opaque id
1276+
});
1277+
});
1278+
11761279
describe('Expand Related Records', () => {
11771280
beforeEach(async () => {
11781281
engine.registerDriver(mockDriver, true);

packages/objectql/src/engine.ts

Lines changed: 121 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
type DroppedFieldsEvent
1313
} from '@objectstack/spec/data';
1414
import type { WriteObservabilityOptions } from '@objectstack/spec/contracts';
15-
import { parseAutonumberFormat, renderAutonumber, missingFieldValues, isTenancyDisabled } from '@objectstack/spec/data';
15+
import { parseAutonumberFormat, renderAutonumber, missingFieldValues, isTenancyDisabled, FILE_REFERENCE_TYPES } from '@objectstack/spec/data';
1616
import { ExecutionContext, ExecutionContextSchema } from '@objectstack/spec/kernel';
1717
import { IDataDriver, IDataEngine, Logger, createLogger, withTransientRetry, type RetryOptions } from '@objectstack/core';
1818
import { SummaryRecomputeError, type SummaryRecomputeFailure } from './summary-errors.js';
@@ -2120,6 +2120,112 @@ export class ObjectQL implements IDataEngine {
21202120
return records;
21212121
}
21222122

2123+
/**
2124+
* Whether a value is an opaque `sys_file` id token — the minted uuid/nanoid
2125+
* form (letters, digits, `_`, `-`, bounded length), and crucially NOT a URL:
2126+
* a `https://…` / `/api/…` / `data:…` / `blob:…` file value carries `:`, `/`
2127+
* or `.` and is left untouched (it is a legacy/external URL, not a reference).
2128+
*/
2129+
private static readonly FILE_ID_RE = /^[A-Za-z0-9_-]{1,64}$/;
2130+
2131+
/**
2132+
* Resolve file-field id references to their expanded `FileValueSchema` form
2133+
* (ADR-0104 D3 wave 2). A `file`/`image`/`avatar`/`video`/`audio` value
2134+
* stored as an opaque `sys_file` id string is enriched, in place, to
2135+
* `{ id, name, size, mimeType, url }` — `url` derived from the stable
2136+
* `/files/:fileId` resolver, never stored.
2137+
*
2138+
* DUAL-MODE SAFE: an inline-blob value (an object) and a string that does
2139+
* NOT match a committed `sys_file` row (e.g. an external url) pass through
2140+
* unchanged, so a field may hold either form during the pre-v17 window.
2141+
*
2142+
* Batched: at most one `sys_file` `id $in […]` read per call (no N+1); and
2143+
* zero reads when no field holds a string value (the blob-only case), so the
2144+
* step is free for objects that have not adopted references.
2145+
*/
2146+
private async resolveFileReferences(
2147+
objectName: string,
2148+
records: any[],
2149+
execCtx?: ExecutionContext,
2150+
): Promise<any[]> {
2151+
if (!records || records.length === 0) return records;
2152+
const objectSchema = this._registry.getObject(objectName);
2153+
if (!objectSchema || !objectSchema.fields) return records;
2154+
// Nothing to resolve against if the file object is not even registered
2155+
// (e.g. the storage plugin is absent) — skip without a failing query.
2156+
if (!this._registry.getObject('sys_file')) return records;
2157+
2158+
const fileFields = Object.entries(objectSchema.fields)
2159+
.filter(([, def]: [string, any]) => def && FILE_REFERENCE_TYPES.has(def.type))
2160+
.map(([name]) => name);
2161+
if (fileFields.length === 0) return records;
2162+
2163+
// Collect candidate ids. A file field legitimately holds either an inline
2164+
// blob (an object — already the rich form, skipped) OR, in the dual-mode /
2165+
// legacy world, a URL string (`https://…`, `/api/…`, `data:…`, `blob:…`).
2166+
// Only an OPAQUE id token — the minted uuid/nanoid form, never url-shaped —
2167+
// is a `sys_file` reference to resolve. Filtering out url-shaped strings is
2168+
// what keeps a seeded `data:`/CDN image value from firing a bogus lookup.
2169+
const candidateIds: string[] = [];
2170+
const addCandidate = (v: unknown) => {
2171+
if (typeof v === 'string' && ObjectQL.FILE_ID_RE.test(v)) candidateIds.push(v);
2172+
};
2173+
for (const record of records) {
2174+
for (const fieldName of fileFields) {
2175+
const val = record[fieldName];
2176+
if (val == null) continue;
2177+
if (Array.isArray(val)) {
2178+
for (const v of val) addCandidate(v);
2179+
} else {
2180+
addCandidate(val);
2181+
}
2182+
}
2183+
}
2184+
const uniqueIds = [...new Set(candidateIds)];
2185+
if (uniqueIds.length === 0) return records; // blob-only / empty → zero cost
2186+
2187+
// One batched sys_file read. `__expandRead` mirrors the lookup-expansion
2188+
// sub-read (a system-built marker, never from client input). Metadata only —
2189+
// byte-download authorization is enforced at the /files/:fileId resolver.
2190+
let fileRows: any[] = [];
2191+
try {
2192+
fileRows = (await this.find(
2193+
'sys_file',
2194+
{ where: { id: { $in: uniqueIds } }, context: { ...(execCtx ?? {}), __expandRead: true } as ExecutionContext },
2195+
)) ?? [];
2196+
} catch {
2197+
return records; // sys_file unregistered / unreadable — leave ids as-is
2198+
}
2199+
2200+
const fileMap = new Map<string, any>();
2201+
for (const row of fileRows) {
2202+
if (row?.id != null && row.status === 'committed') fileMap.set(String(row.id), row);
2203+
}
2204+
if (fileMap.size === 0) return records;
2205+
2206+
const toValue = (row: any) => ({
2207+
id: String(row.id),
2208+
...(row.name != null ? { name: row.name } : {}),
2209+
...(row.size != null ? { size: row.size } : {}),
2210+
...(row.mime_type != null ? { mimeType: row.mime_type } : {}),
2211+
url: `/api/v1/storage/files/${row.id}`,
2212+
});
2213+
2214+
for (const record of records) {
2215+
for (const fieldName of fileFields) {
2216+
const val = record[fieldName];
2217+
if (val == null) continue;
2218+
if (Array.isArray(val)) {
2219+
record[fieldName] = val.map((v: any) =>
2220+
typeof v === 'string' && fileMap.has(v) ? toValue(fileMap.get(v)) : v);
2221+
} else if (typeof val === 'string' && fileMap.has(val)) {
2222+
record[fieldName] = toValue(fileMap.get(val));
2223+
}
2224+
}
2225+
}
2226+
return records;
2227+
}
2228+
21232229
// ============================================
21242230
// Data Access Methods (IDataEngine Interface)
21252231
// ============================================
@@ -2240,7 +2346,14 @@ export class ObjectQL implements IDataEngine {
22402346
if (ast.expand && Object.keys(ast.expand).length > 0 && Array.isArray(result)) {
22412347
result = await this.expandRelatedRecords(object, result, ast.expand, 0, opCtx.context);
22422348
}
2243-
2349+
2350+
// Post-process: resolve file-field id references to their expanded
2351+
// FileValueSchema form (ADR-0104 D3). Always-on but free unless a file
2352+
// field holds an id string; dual-mode-safe (blobs pass through).
2353+
if (Array.isArray(result)) {
2354+
result = await this.resolveFileReferences(object, result, opCtx.context);
2355+
}
2356+
22442357
hookContext.event = 'afterFind';
22452358
hookContext.result = result;
22462359
await this.triggerHooks('afterFind', hookContext);
@@ -2324,6 +2437,12 @@ export class ObjectQL implements IDataEngine {
23242437
result = expanded[0];
23252438
}
23262439

2440+
// Post-process: resolve file-field id references (ADR-0104 D3).
2441+
if (result != null) {
2442+
const resolved = await this.resolveFileReferences(objectName, [result], opCtx.context);
2443+
result = resolved[0];
2444+
}
2445+
23272446
hookContext.event = 'afterFind';
23282447
hookContext.result = result;
23292448
await this.triggerHooks('afterFind', hookContext);

0 commit comments

Comments
 (0)