Skip to content

Commit 3234216

Browse files
committed
fix(rest): import resolveRef flushes pending creates and drops negative cache (#3148)
Batched CREATE rows buffer into pendingCreates and flush later, but resolveRef only queried the DB — so a later row referencing an earlier same-file CREATE always missed with reference_not_found and was dropped. refCache compounded it by caching the empty miss, so even after a progress-checkpoint flush landed the row, the name stayed pinned to the cached miss. resolveRef now, on a same-object miss with a non-empty buffer, flushes pending creates and retries the lookup once (the buffered rows are all earlier than the current one, so the flush is safe and idempotent). A bare miss is no longer cached — only a definitive id/ambiguous verdict is — so a name that misses early can still resolve once a later flush creates it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APnSDJneEDRh3Z51KGCLga
1 parent ac781b7 commit 3234216

2 files changed

Lines changed: 147 additions & 13 deletions

File tree

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* framework#3148: within a single import file, a later row must be able to
5+
* reference a record an earlier row created — even though CREATE rows are
6+
* buffered into a batched flush rather than written immediately. resolveRef
7+
* flushes the pending-create buffer on a same-object miss and retries, and a
8+
* bare miss is never negatively cached (which would pin it forever once a
9+
* later flush actually creates the row).
10+
*/
11+
12+
import { describe, it, expect, vi } from 'vitest';
13+
import { runImport, type ImportProtocolLike } from './import-runner';
14+
import type { ExportFieldMeta } from './export-format.js';
15+
16+
// `parent` is a lookup back to this same object (a category tree).
17+
const metaMap = new Map<string, ExportFieldMeta>([
18+
['name', { name: 'name', type: 'text' }],
19+
['parent', { name: 'parent', type: 'lookup', reference: 'showcase_category' }],
20+
]);
21+
22+
const baseOpts = {
23+
objectName: 'showcase_category',
24+
metaMap,
25+
writeMode: 'insert' as const,
26+
matchFields: [] as string[],
27+
dryRun: false,
28+
runAutomations: false,
29+
trimWhitespace: true,
30+
createMissingOptions: false,
31+
skipBlankMatchKey: false,
32+
};
33+
34+
/** Mock protocol backed by an in-memory store: createManyData writes, findData filters. */
35+
function makeProtocol(seed: Array<Record<string, any>> = []) {
36+
const store: Array<Record<string, any>> = [...seed];
37+
let idc = 0;
38+
const createManyData = vi.fn(async (args: { records: any[] }) => ({
39+
records: args.records.map((r) => {
40+
const rec = { id: `new-${++idc}`, ...r };
41+
store.push(rec);
42+
return rec;
43+
}),
44+
}));
45+
const findData = vi.fn(async (args: { query?: { $filter?: Record<string, any> } }) => {
46+
const filter = args.query?.$filter ?? {};
47+
return store.filter((row) => Object.entries(filter).every(([k, v]) => row[k] === v));
48+
});
49+
const p: ImportProtocolLike = { findData, createData: vi.fn(), updateData: vi.fn(), createManyData };
50+
return { p, store, createManyData };
51+
}
52+
53+
describe('runImport — same-file forward references (framework#3148)', () => {
54+
it('resolves a row that references a record an earlier buffered row created', async () => {
55+
// Existing-Parent is already in the DB; New-Parent is created by row 2 and
56+
// referenced by row 3 — while still sitting in the create buffer.
57+
const { p, store, createManyData } = makeProtocol([{ id: 'existing-1', name: 'Existing-Parent' }]);
58+
59+
const summary = await runImport({
60+
...baseOpts, p,
61+
rows: [
62+
{ name: 'Control-Child', parent: 'Existing-Parent' }, // references a pre-existing row
63+
{ name: 'New-Parent' },
64+
{ name: 'New-Child', parent: 'New-Parent' }, // references row 2 (still buffered)
65+
],
66+
});
67+
68+
expect(summary.errors).toBe(0);
69+
expect(summary.created).toBe(3);
70+
71+
const newParent = store.find((r) => r.name === 'New-Parent')!;
72+
const newChild = store.find((r) => r.name === 'New-Child')!;
73+
const controlChild = store.find((r) => r.name === 'Control-Child')!;
74+
expect(newChild.parent).toBe(newParent.id); // resolved to the real created id
75+
expect(controlChild.parent).toBe('existing-1'); // existing-reference behaviour unchanged
76+
77+
// The miss on 'New-Parent' forced an early flush mid-loop, so there are two
78+
// createManyData calls (the forced flush + the end-of-loop flush).
79+
expect(createManyData).toHaveBeenCalledTimes(2);
80+
});
81+
82+
it('does not negatively cache a miss: a name that misses early resolves once created (progressEvery=1)', async () => {
83+
const { p, store } = makeProtocol();
84+
85+
const summary = await runImport({
86+
...baseOpts, p, progressEvery: 1,
87+
rows: [
88+
{ name: 'Decoy', parent: 'New-Parent' }, // miss — New-Parent doesn't exist yet → fails
89+
{ name: 'New-Parent' }, // created; flushed after this row (progressEvery=1)
90+
{ name: 'Child', parent: 'New-Parent' }, // must resolve — the earlier miss must not be cached
91+
],
92+
});
93+
94+
// Row 1 legitimately fails (the target truly did not exist at that point).
95+
expect(summary.results[0]).toMatchObject({ ok: false, code: 'reference_not_found' });
96+
// Rows 2 and 3 succeed; the child links to the real parent.
97+
const newParent = store.find((r) => r.name === 'New-Parent')!;
98+
const child = store.find((r) => r.name === 'Child')!;
99+
expect(child.parent).toBe(newParent.id);
100+
expect(summary.created).toBe(2);
101+
});
102+
103+
it('a genuine miss (no such record, no pending create) still reports reference_not_found', async () => {
104+
const { p, createManyData } = makeProtocol();
105+
106+
const summary = await runImport({
107+
...baseOpts, p,
108+
rows: [{ name: 'Orphan', parent: 'Nobody' }],
109+
});
110+
111+
expect(summary.results[0]).toMatchObject({ ok: false, code: 'reference_not_found' });
112+
expect(summary.created).toBe(0);
113+
// Nothing to create → no batch flush ever ran.
114+
expect(createManyData).not.toHaveBeenCalled();
115+
});
116+
});

packages/rest/src/import-runner.ts

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -199,20 +199,38 @@ export function runImport(opts: RunImportOptions): Promise<ImportRunSummary> {
199199
...(meta.displayField ? [meta.displayField] : []),
200200
'name', 'title', 'label', 'full_name', 'email', 'username',
201201
])];
202-
let match: RefMatch = {};
203-
for (const f of candidates) {
204-
try {
205-
const r = await p.findData({
206-
...findArgsBase({ $filter: { [f]: display }, $top: 2 }),
207-
object: referenceObject,
208-
});
209-
const recs = findRows(r);
210-
if (recs.length === 0) continue;
211-
if (recs.length > 1) { match = { ambiguous: true, matchedField: f }; break; }
212-
if (recs[0]?.id != null) { match = { id: String(recs[0].id), matchedField: f }; break; }
213-
} catch { /* field absent on target object — try the next candidate */ }
202+
const lookup = async (): Promise<RefMatch> => {
203+
let match: RefMatch = {};
204+
for (const f of candidates) {
205+
try {
206+
const r = await p.findData({
207+
...findArgsBase({ $filter: { [f]: display }, $top: 2 }),
208+
object: referenceObject,
209+
});
210+
const recs = findRows(r);
211+
if (recs.length === 0) continue;
212+
if (recs.length > 1) { match = { ambiguous: true, matchedField: f }; break; }
213+
if (recs[0]?.id != null) { match = { id: String(recs[0].id), matchedField: f }; break; }
214+
} catch { /* field absent on target object — try the next candidate */ }
215+
}
216+
return match;
217+
};
218+
let match = await lookup();
219+
// A miss may just mean the referenced row is still buffered as a pending
220+
// create — the same-file "later row references an earlier CREATE" case that
221+
// the batched-create rework regressed. Flush the buffer and retry the
222+
// lookup once: the buffered rows are all EARLIER than this one (resolveRef
223+
// runs mid row-loop), so the flush is safe and, once drained, a no-op.
224+
// Only a reference to THIS object can be satisfied from the buffer, so we
225+
// don't flush for a miss on some other object (framework#3148).
226+
if (!match.id && !match.ambiguous && referenceObject === objectName && pendingCreates.length > 0) {
227+
await flushPendingCreates();
228+
match = await lookup();
214229
}
215-
refCache.set(cacheKey, match);
230+
// Cache only a definitive verdict. A bare miss ({}) is deliberately NOT
231+
// cached: the referenced row may be created by a later flush, and a
232+
// negative-cache entry would pin the miss forever (the pre-fix regression).
233+
if (match.id != null || match.ambiguous) refCache.set(cacheKey, match);
216234
return match;
217235
};
218236

0 commit comments

Comments
 (0)