Skip to content

Commit 5db2a84

Browse files
committed
feat(storage): legacy file-value backfill — ADR-0104 D3 wave 2 (PR-6)
backfillFileReferences() converts the pre-reference forms a file/image/avatar/ video/audio field may hold — an inline metadata blob ({url, name, size, …}) or a bare URL string — into the reference form: an opaque sys_file id, owned by the record's field. What it will and will not convert: - A URL naming this platform's own resolver (…/storage/files/:id) already identifies a sys_file; the field is rewritten to the bare id, no bytes move. - A data: URI carries its bytes inline; they are uploaded, a sys_file is registered, and the field is rewritten to its id. - An external URL is REPORTED, NEVER CONVERTED. Re-hosting third-party content is a bandwidth, licensing and privacy decision that is not a migration's to make. ADR-0104 R7 retires these toward an explicit `url` field, which under AI authoring is the point: it stops "managed file" and "external link" being the same declaration. Dry run by default — nothing is written unless apply is set, and the dry-run report has the same shape as the applied one so the plan can be reviewed and diffed against what actually happened. Idempotent — a value already in reference form is recorded as already_id and left alone, so a partially completed run is safe to repeat. The backfill never writes the ref_* columns itself: it rewrites the record, and the claim hooks observe that write and record ownership. One claiming path, so there is nothing that can disagree with itself — asserted by a test that fails if the backfill ever starts touching sys_file directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SHpGw3GBA9aFpfwVArRWfd
1 parent 134df4f commit 5db2a84

4 files changed

Lines changed: 707 additions & 0 deletions

File tree

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
"@objectstack/service-storage": minor
3+
---
4+
5+
feat(storage): legacy file-value backfill — ADR-0104 D3 wave 2 (PR-6)
6+
7+
`backfillFileReferences()` converts the pre-reference forms a `file`/`image`/
8+
`avatar`/`video`/`audio` field may hold — an inline metadata blob
9+
(`{url, name, size, …}`) or a bare URL string — into the reference form: an
10+
opaque `sys_file` id, owned by the record's field.
11+
12+
What it will and will not convert:
13+
14+
- **A URL naming this platform's own resolver** (`…/storage/files/:id`) already
15+
identifies a `sys_file`; the field is rewritten to the bare id and no bytes
16+
move.
17+
- **A `data:` URI** carries its bytes inline; they are uploaded, a `sys_file` is
18+
registered, and the field is rewritten to its id.
19+
- **An external URL** is reported, never converted. Re-hosting third-party
20+
content is a bandwidth, licensing and privacy decision that is not a
21+
migration's to make — ADR-0104 R7 retires these toward an explicit `url`
22+
field, which under AI authoring is the point: it stops "managed file" and
23+
"external link" being the same declaration.
24+
25+
**Dry run by default** — nothing is written unless `apply` is set, and the
26+
dry-run report has the same shape as the applied one so the plan can be reviewed
27+
and diffed. **Idempotent** — a value already in reference form is recorded and
28+
left alone, so a partially-completed run is safe to repeat.
29+
30+
The backfill never writes the ownership columns itself: it rewrites the record,
31+
and the claim hooks observe that write and record ownership. One claiming path,
32+
so there is nothing that can disagree with itself. Run
33+
`verifyFileReferences()` afterwards to confirm the two agree — that
34+
reconciliation is the gate the irreversible collection change must pass.
Lines changed: 251 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,251 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect, vi } from 'vitest';
4+
import {
5+
backfillFileReferences,
6+
formatBackfillReport,
7+
type BackfillEngine,
8+
} from './backfill-file-references.js';
9+
10+
const silentLogger = () => ({ info: vi.fn(), warn: vi.fn(), debug: vi.fn() });
11+
12+
const REGISTRY: Record<string, any> = {
13+
sys_file: { fields: { id: { type: 'text' } } },
14+
product: {
15+
fields: {
16+
id: { type: 'text' },
17+
name: { type: 'text' },
18+
image: { type: 'image' },
19+
gallery: { type: 'image', multiple: true },
20+
},
21+
},
22+
tag: { fields: { id: { type: 'text' }, label: { type: 'text' } } },
23+
};
24+
25+
function fakeEngine(tables: Record<string, Array<Record<string, unknown>>>) {
26+
const calls: Array<{ op: string; object: string; arg: any }> = [];
27+
const engine: BackfillEngine & { tables: typeof tables; calls: typeof calls } = {
28+
getObject: (name) => REGISTRY[name],
29+
getConfigs: () => REGISTRY,
30+
async find(object, options: any) {
31+
const rows = tables[object] ?? [];
32+
const start = typeof options?.offset === 'number' ? options.offset : 0;
33+
const end = typeof options?.limit === 'number' ? start + options.limit : undefined;
34+
return rows.slice(start, end);
35+
},
36+
async insert(object, data: any) {
37+
calls.push({ op: 'insert', object, arg: data });
38+
(tables[object] ??= []).push({ ...data });
39+
return data;
40+
},
41+
async update(object, data: any) {
42+
calls.push({ op: 'update', object, arg: data });
43+
const row = (tables[object] ?? []).find((r) => String(r.id) === String(data.id));
44+
if (row) Object.assign(row, data);
45+
return row;
46+
},
47+
tables,
48+
calls,
49+
};
50+
return engine;
51+
}
52+
53+
const fakeStorage = () =>
54+
({
55+
upload: vi.fn(async () => {}),
56+
download: vi.fn(async () => Buffer.from('x')),
57+
delete: vi.fn(async () => {}),
58+
exists: vi.fn(async () => true),
59+
getInfo: vi.fn(async () => ({ key: 'k', size: 1, contentType: 'text/plain', lastModified: new Date() })),
60+
}) as any;
61+
62+
const run = (engine: any, opts: any = {}, storage: any = fakeStorage()) =>
63+
backfillFileReferences(engine, () => storage, silentLogger(), opts);
64+
65+
describe('backfillFileReferences (ADR-0104 D3 wave 2)', () => {
66+
it('rewrites a URL that already names a sys_file to the bare id, moving no bytes', async () => {
67+
const engine = fakeEngine({
68+
product: [{ id: 'p1', image: 'https://app.example.com/api/v1/storage/files/file_a' }],
69+
sys_file: [],
70+
});
71+
const storage = fakeStorage();
72+
73+
const report = await run(engine, { apply: true }, storage);
74+
75+
expect(engine.tables.product[0].image).toBe('file_a');
76+
expect(report.converted).toBe(1);
77+
expect(storage.upload).not.toHaveBeenCalled();
78+
expect(report.actions[0].kind).toBe('resolved_local_url');
79+
});
80+
81+
it('resolves the same URL when it is wrapped in a legacy inline blob', async () => {
82+
const engine = fakeEngine({
83+
product: [
84+
{ id: 'p1', image: { url: '/api/v1/storage/files/file_b', name: 'b.png', size: 12 } },
85+
],
86+
sys_file: [],
87+
});
88+
89+
await run(engine, { apply: true });
90+
91+
expect(engine.tables.product[0].image).toBe('file_b');
92+
});
93+
94+
it('uploads inline data: bytes and rewrites the field to the new id', async () => {
95+
const engine = fakeEngine({
96+
product: [{ id: 'p1', image: { url: 'data:image/png;base64,aGVsbG8=', name: 'hi.png' } }],
97+
sys_file: [],
98+
});
99+
const storage = fakeStorage();
100+
101+
const report = await run(engine, { apply: true }, storage);
102+
103+
expect(storage.upload).toHaveBeenCalledTimes(1);
104+
const created = engine.tables.sys_file[0];
105+
expect(created).toMatchObject({ name: 'hi.png', mime_type: 'image/png', status: 'committed' });
106+
expect(created.size).toBe(5); // "hello"
107+
expect(engine.tables.product[0].image).toBe(created.id);
108+
expect(report.converted).toBe(1);
109+
});
110+
111+
/**
112+
* Converting an external URL would mean fetching third-party content and
113+
* silently re-hosting it — a bandwidth/licensing/privacy decision that is not
114+
* a migration's to make.
115+
*/
116+
it('reports an external URL and never re-hosts it', async () => {
117+
const engine = fakeEngine({
118+
product: [{ id: 'p1', image: 'https://cdn.example.com/logo.png' }],
119+
sys_file: [],
120+
});
121+
const storage = fakeStorage();
122+
123+
const report = await run(engine, { apply: true }, storage);
124+
125+
expect(engine.tables.product[0].image).toBe('https://cdn.example.com/logo.png');
126+
expect(storage.upload).not.toHaveBeenCalled();
127+
expect(report.externalUrls).toBe(1);
128+
expect(report.converted).toBe(0);
129+
expect(formatBackfillReport(report)).toContain('NOT re-hosted');
130+
});
131+
132+
it('handles a multiple:true field element-wise', async () => {
133+
const engine = fakeEngine({
134+
product: [
135+
{
136+
id: 'p1',
137+
gallery: [
138+
'/api/v1/storage/files/file_a',
139+
'https://cdn.example.com/x.png',
140+
'file_already',
141+
],
142+
},
143+
],
144+
sys_file: [],
145+
});
146+
147+
const report = await run(engine, { apply: true });
148+
149+
expect(engine.tables.product[0].gallery).toEqual([
150+
'file_a',
151+
'https://cdn.example.com/x.png',
152+
'file_already',
153+
]);
154+
expect(report.converted).toBe(1);
155+
expect(report.externalUrls).toBe(1);
156+
expect(report.alreadyReferences).toBe(1);
157+
});
158+
159+
// ── Dry run ───────────────────────────────────────────────────────
160+
it('writes nothing on a dry run but reports the same plan', async () => {
161+
const engine = fakeEngine({
162+
product: [
163+
{ id: 'p1', image: '/api/v1/storage/files/file_a' },
164+
{ id: 'p2', image: 'data:text/plain;base64,aGk=' },
165+
],
166+
sys_file: [],
167+
});
168+
const storage = fakeStorage();
169+
170+
const report = await run(engine, {}, storage);
171+
172+
expect(report.applied).toBe(false);
173+
expect(report.converted).toBe(2);
174+
expect(engine.calls).toHaveLength(0);
175+
expect(engine.tables.product[0].image).toBe('/api/v1/storage/files/file_a');
176+
expect(storage.upload).not.toHaveBeenCalled();
177+
expect(formatBackfillReport(report)).toContain('DRY RUN');
178+
});
179+
180+
// ── Idempotence / safety ──────────────────────────────────────────
181+
it('is idempotent — a second run converts nothing', async () => {
182+
const engine = fakeEngine({
183+
product: [{ id: 'p1', image: '/api/v1/storage/files/file_a' }],
184+
sys_file: [],
185+
});
186+
187+
await run(engine, { apply: true });
188+
const second = await run(engine, { apply: true });
189+
190+
expect(second.converted).toBe(0);
191+
expect(second.alreadyReferences).toBe(1);
192+
});
193+
194+
it('never writes the ref_* columns itself — claiming stays on one path', async () => {
195+
const engine = fakeEngine({
196+
product: [{ id: 'p1', image: '/api/v1/storage/files/file_a' }],
197+
sys_file: [],
198+
});
199+
200+
await run(engine, { apply: true });
201+
202+
// The record write is what the claim hooks observe; the backfill must not
203+
// record ownership behind their back, or there would be two claiming paths
204+
// that could disagree.
205+
for (const c of engine.calls) {
206+
expect(c.arg).not.toHaveProperty('ref_object');
207+
expect(c.arg).not.toHaveProperty('ref_id');
208+
expect(c.arg).not.toHaveProperty('ref_field');
209+
}
210+
expect(engine.calls.filter((c) => c.object === 'sys_file')).toHaveLength(0);
211+
});
212+
213+
it('leaves data: values alone when no storage service is available', async () => {
214+
const engine = fakeEngine({
215+
product: [{ id: 'p1', image: 'data:text/plain;base64,aGk=' }],
216+
sys_file: [],
217+
});
218+
219+
const report = await backfillFileReferences(engine, () => null, silentLogger(), { apply: true });
220+
221+
expect(report.unresolvable).toBe(1);
222+
expect(report.converted).toBe(0);
223+
expect(engine.tables.product[0].image).toBe('data:text/plain;base64,aGk=');
224+
});
225+
226+
it('skips objects with no file-class fields', async () => {
227+
const engine = fakeEngine({ tag: [{ id: 't1', label: 'x' }], product: [], sys_file: [] });
228+
229+
const report = await run(engine, { apply: true });
230+
231+
expect(report.scannedObjects).toEqual(['product']);
232+
});
233+
234+
it('pages through large objects and flags truncation at the bound', async () => {
235+
const many = Array.from({ length: 500 }, (_, i) => ({
236+
id: `p${i}`,
237+
image: `/api/v1/storage/files/file_${i}`,
238+
}));
239+
const engine = fakeEngine({ product: many, sys_file: [] });
240+
241+
const full = await run(engine, {});
242+
expect(full.scannedRecords).toBe(500);
243+
expect(full.truncated).toBe(false);
244+
245+
const bounded = await run(fakeEngine({ product: many, sys_file: [] }), {
246+
maxRecordsPerObject: 200,
247+
});
248+
expect(bounded.truncated).toBe(true);
249+
expect(formatBackfillReport(bounded)).toContain('truncated');
250+
});
251+
});

0 commit comments

Comments
 (0)