Skip to content

Commit 87d4da9

Browse files
committed
feat(rest): harden lookup/reference resolution (id fast-path + ambiguity)
Reference cells (name/email/id) now resolve more safely: - id fast-path: try an exact id match first, so a pasted record id is authoritative and never shadowed by a name/label collision. - ambiguity guard: when the first matching candidate field hits >1 record, stop and report a new 'reference_ambiguous' error instead of silently linking whichever row came back first ($top:2 detects the dup). - structured resolver contract: RefResolver may return { id, ambiguous, matchedField }; bare string|undefined from legacy resolvers still works (normalizeRefMatch). Tests: unit (structured result + ambiguous + not-found) and integration (duplicate-name ambiguity, id fast-path).
1 parent 73fad27 commit 87d4da9

4 files changed

Lines changed: 94 additions & 18 deletions

File tree

packages/rest/src/import-coerce.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,24 @@ describe('coerceRow', () => {
140140
expect(bad.errors[0]).toMatchObject({ field: 'owner', code: 'reference_not_found' });
141141
});
142142

143+
it('accepts a structured resolver result and flags ambiguous matches', async () => {
144+
const metaMap = meta({ owner: { type: 'lookup', reference: 'user', displayField: 'name' } });
145+
const resolveRef = async (_obj: string, display: string) => {
146+
if (display === '张三') return { id: 'u1', matchedField: 'name' };
147+
if (display === '李四') return { ambiguous: true, matchedField: 'name' };
148+
return {};
149+
};
150+
const ok = await coerceRow({ owner: '张三' }, metaMap, { resolveRef });
151+
expect(ok.data).toEqual({ owner: 'u1' });
152+
153+
const dup = await coerceRow({ owner: '李四' }, metaMap, { resolveRef });
154+
expect(dup.data.owner).toBeUndefined();
155+
expect(dup.errors[0]).toMatchObject({ field: 'owner', code: 'reference_ambiguous' });
156+
157+
const none = await coerceRow({ owner: '无名' }, metaMap, { resolveRef });
158+
expect(none.errors[0]).toMatchObject({ field: 'owner', code: 'reference_not_found' });
159+
});
160+
143161
it('reports coercion errors per field instead of throwing', async () => {
144162
const metaMap = meta({ n: { type: 'number' }, b: { type: 'boolean' } });
145163
const { data, errors } = await coerceRow({ n: 'abc', b: 'maybe' }, metaMap, {});

packages/rest/src/import-coerce.ts

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,17 +38,40 @@ const NUMBER_TYPES = new Set(['number', 'currency', 'percent', 'rating', 'slider
3838
/** Boolean field types (store a real boolean). */
3939
const BOOL_TYPES = new Set(['boolean', 'toggle']);
4040

41+
/**
42+
* Structured outcome of a reference lookup. `id` set → a single record matched.
43+
* `ambiguous` → the display value matched more than one record, so linking any
44+
* one of them would be a guess the importer refuses to make. `matchedField`
45+
* names the field the match came from (for diagnostics). An empty object means
46+
* nothing matched. A bare `string | undefined` is still accepted from legacy
47+
* resolvers and normalised to this shape.
48+
*/
49+
export interface RefMatch {
50+
id?: string;
51+
ambiguous?: boolean;
52+
matchedField?: string;
53+
}
54+
4155
/**
4256
* Resolve a reference field's display value (a name / email / id typed by the
43-
* user) to the referenced record's id. Returns `undefined` when nothing matches
44-
* so the caller can surface a per-row "not found" error. Implementations are
57+
* user) to the referenced record's id. Return `undefined` / `{}` when nothing
58+
* matches (caller surfaces "not found"), a bare id string / `{ id }` on a unique
59+
* hit, or `{ ambiguous: true }` when several records share the value. Legacy
60+
* resolvers that return `string | undefined` keep working. Implementations are
4561
* expected to cache — the same name shows up on many rows.
4662
*/
4763
export type RefResolver = (
4864
referenceObject: string,
4965
displayValue: string,
5066
meta: ExportFieldMeta,
51-
) => Promise<string | undefined>;
67+
) => Promise<string | undefined | RefMatch>;
68+
69+
/** Normalise a resolver result (legacy string or structured) to a RefMatch. */
70+
function normalizeRefMatch(result: string | undefined | RefMatch): RefMatch {
71+
if (result == null) return {};
72+
if (typeof result === 'string') return result ? { id: result } : {};
73+
return result;
74+
}
5275

5376
export interface CoerceContext {
5477
/** Trim leading/trailing whitespace from string-ish cells (default true). */
@@ -285,11 +308,14 @@ export async function coerceFieldValue(
285308
// no target object, store the raw value and let referential integrity be
286309
// enforced downstream.
287310
if (!ctx.resolveRef || !meta?.reference) return { value: display };
288-
const id = await ctx.resolveRef(meta.reference, display, meta);
289-
if (id === undefined) {
311+
const match = normalizeRefMatch(await ctx.resolveRef(meta.reference, display, meta));
312+
if (match.ambiguous) {
313+
return { error: { field, code: 'reference_ambiguous', message: `${field}: "${display}" matches more than one ${meta.reference} — use a unique value or the record id` } };
314+
}
315+
if (match.id === undefined) {
290316
return { error: { field, code: 'reference_not_found', message: `${field}: no ${meta.reference} matches "${display}"` } };
291317
}
292-
return { value: id };
318+
return { value: match.id };
293319
}
294320

295321
// Everything else (text, email, phone, json, html, file, …): pass through,

packages/rest/src/import-integration.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,32 @@ describe('import route — real engine + protocol integration', () => {
179179
expect(a.owner).toBe('u2');
180180
});
181181

182+
it('reports reference_ambiguous when a name matches more than one record', async () => {
183+
// Second 张三 makes the name non-unique; the importer must refuse to guess.
184+
await engine.insert('user', { id: 'u3', name: '张三', email: 'zhang2@x.com' });
185+
const res = await call(route, {
186+
format: 'json',
187+
rows: [
188+
{ id: 'a', title: 'x', owner: '张三' }, // ambiguous name
189+
{ id: 'b', title: 'y', owner: 'zhang2@x.com' }, // unique email → resolves
190+
],
191+
});
192+
expect(res._json.errors).toBe(1);
193+
expect(res._json.results.find((r: any) => !r.ok)).toMatchObject({ field: 'owner', code: 'reference_ambiguous' });
194+
const b = await engine.findOne('task', { where: { id: 'b' } });
195+
expect(b.owner).toBe('u3');
196+
});
197+
198+
it('accepts a pasted record id directly via the id fast-path', async () => {
199+
const res = await call(route, {
200+
format: 'json',
201+
rows: [{ id: 'c', title: 'z', owner: 'u1' }],
202+
});
203+
expect(res._json.errors).toBe(0);
204+
const c = await engine.findOne('task', { where: { id: 'c' } });
205+
expect(c.owner).toBe('u1');
206+
});
207+
182208
it('surfaces per-row coercion errors without aborting the batch', async () => {
183209
const res = await call(route, {
184210
format: 'json',

packages/rest/src/rest-server.ts

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
formatRowForJson,
1313
type ExportFieldMeta,
1414
} from './export-format.js';
15-
import { coerceRow, type RefResolver } from './import-coerce.js';
15+
import { coerceRow, type RefResolver, type RefMatch } from './import-coerce.js';
1616

1717
// Node-safe logger — avoids importing 'console' which is absent from ES2020 lib typings.
1818
const logError = (...args: unknown[]) => (globalThis as any).console?.error(...args);
@@ -3398,30 +3398,36 @@ export class RestServer {
33983398

33993399
// Reference resolver: name/email/id → referenced record id. Cached
34003400
// per (object, display) so a name repeated across rows costs one query.
3401-
const refCache = new Map<string, string | undefined>();
3401+
const refCache = new Map<string, RefMatch>();
34023402
const resolveRef: RefResolver = async (referenceObject, display, meta) => {
34033403
const cacheKey = `${referenceObject}::${display}`;
3404-
if (refCache.has(cacheKey)) return refCache.get(cacheKey);
3405-
// Try the configured display field first, then fall back to the
3406-
// usual human identifiers (name/email/id): users paste whichever
3407-
// uniquely names the record. De-dupe so displayField isn't retried.
3404+
const cached = refCache.get(cacheKey);
3405+
if (cached) return cached;
3406+
// Try an exact id first (authoritative + unique when the user pasted
3407+
// an id), then the configured display field, then the usual human
3408+
// identifiers. De-dupe so a field isn't queried twice. The first
3409+
// candidate field to match wins; if that field matches >1 record we
3410+
// stop and report ambiguity rather than silently linking the first.
34083411
const candidates = [...new Set([
3412+
'id',
34093413
...(meta.displayField ? [meta.displayField] : []),
3410-
'name', 'title', 'label', 'full_name', 'email', 'username', 'id',
3414+
'name', 'title', 'label', 'full_name', 'email', 'username',
34113415
])];
3412-
let id: string | undefined;
3416+
let match: RefMatch = {};
34133417
for (const f of candidates) {
34143418
try {
34153419
const r = await (p as any).findData({
34163420
...findArgsBase({ $filter: { [f]: display }, $top: 2 }),
34173421
object: referenceObject,
34183422
});
34193423
const recs = findRows(r);
3420-
if (recs.length > 0 && recs[0]?.id != null) { id = String(recs[0].id); break; }
3421-
} catch { /* try next candidate field */ }
3424+
if (recs.length === 0) continue;
3425+
if (recs.length > 1) { match = { ambiguous: true, matchedField: f }; break; }
3426+
if (recs[0]?.id != null) { match = { id: String(recs[0].id), matchedField: f }; break; }
3427+
} catch { /* field absent on target object — try the next candidate */ }
34223428
}
3423-
refCache.set(cacheKey, id);
3424-
return id;
3429+
refCache.set(cacheKey, match);
3430+
return match;
34253431
};
34263432

34273433
// Locate an existing record for update/upsert by matchFields. Returns

0 commit comments

Comments
 (0)