diff --git a/.changeset/24981-import-lookup-stored-title.md b/.changeset/24981-import-lookup-stored-title.md new file mode 100644 index 00000000000..992d2b4f5dc --- /dev/null +++ b/.changeset/24981-import-lookup-stored-title.md @@ -0,0 +1,12 @@ +--- +'@objectstack/core': patch +--- + +fix(core): the import resolves a lookup cell by the target object's stored title field + +Clause-②: no + +- **Before.** A lookup cell was matched against the target by its id, then by the lookup's own `displayField`, then by a fixed list of keys (`name`, `title`, `label`, `full_name`, `email`, `username`). The target object's declared title field was never asked. So a cell that named the record by its title, such as `Wren` for a target whose title is `nickname`, answered `reference_not_found`, although the list export writes lookup cells by that same title. +- **Now.** After the id and the lookup's `displayField`, the import asks the target's title field, resolved by ADR-0079's one rule (`resolveDisplayField`: `nameField`, then the deprecated `displayNameField` alias, then the derived title). It is read from the same object declaration the import already reads to check the target's exposure, so no extra metadata read is made. Two records that share the title answer `reference_ambiguous`, like the import's other ambiguous matches. The cell is never linked to the first hit. +- **A formula title is not matched.** A `formula` field has no stored column, and the engine refuses a filter over it. The import no longer sends that query, for the title or for any other candidate the target declares as a formula, so a cell against a formula title still answers `reference_not_found`. How a formula title should be matched is still open. +- **Unchanged.** The fixed fallback keys and their order. The exposure check still runs first: a target whose declared exposure does not serve `list` answers `reference_not_found` for every cell and is never read. diff --git a/packages/core/src/utils/import-runner-reference-title.test.ts b/packages/core/src/utils/import-runner-reference-title.test.ts new file mode 100644 index 00000000000..32eb2ffff6b --- /dev/null +++ b/packages/core/src/utils/import-runner-reference-title.test.ts @@ -0,0 +1,162 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24981] An import cell is resolved against a lookup TARGET's own title + * field: ADR-0079's `resolveDisplayField`, the one rule the list export writes + * a lookup cell by. It is asked after the id and the lookup's own + * `displayField`, and before the fixed fallback keys, which are unchanged. + * + * Only a STORED title is asked. A title the target declares virtual (a + * `formula`) has no column, and the engine's filter seam + * (`assertFilterIsMaterializable`) refuses every `where` over one by the spec + * predicate `isVirtualSearchField`; the resolver asks the same predicate and + * never sends that query. How a formula title should match is the open half of + * the card, with the maintainer, so such a cell still answers + * `reference_not_found`. + * + * The fake's `findData` answers like the engine: equality over the stored row, + * and an `INVALID_FIELD` / 400 refusal for a `where` over a formula field. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { runImport, type ImportProtocolLike } from './import-runner'; +import type { ExportFieldMeta } from './import-field-meta.js'; + +type FindArgs = Parameters[0]; +type Row = Record; +type Declaration = { fields: Record; nameField?: string; displayNameField?: string; enable?: unknown }; + +const TEXT = { type: 'text' }; +const FORMULA = { type: 'formula', returnType: 'text' }; + +/** The card's three targets, plus one whose stored title two records share. */ +const DECLARATIONS: Record = { + // `Wren`: a STORED title off the fixed key list. + xt_person: { displayNameField: 'nickname', fields: { first_name: TEXT, nickname: TEXT } }, + // `John Smith`: a FORMULA title whose name IS on the fixed key list. + xt_contact: { + nameField: 'full_name', + fields: { first_name: TEXT, last_name: TEXT, title: TEXT, full_name: { ...FORMULA, expression: 'first_name + " " + last_name' } }, + }, + // `Acme (EMEA)`: a FORMULA title off the fixed key list. + xt_account: { + nameField: 'display_title', + fields: { name: TEXT, region: TEXT, display_title: { ...FORMULA, expression: 'name + " (" + region + ")"' } }, + }, + xt_member: { nameField: 'nickname', fields: { nickname: TEXT } }, +}; + +const ROWS: Record = { + xt_person: [{ id: 'p1', first_name: 'Ann', nickname: 'Wren' }], + xt_contact: [{ id: 'c1', first_name: 'John', last_name: 'Smith', title: 'VP of Engineering' }], + xt_account: [{ id: 'a1', name: 'Acme', region: 'EMEA' }], + xt_member: [{ id: 'm1', nickname: 'Robin' }, { id: 'm2', nickname: 'Robin' }], +}; + +const lookup = (name: string, reference: string, extra: Partial = {}): [string, ExportFieldMeta] => + [name, { name, type: 'lookup', reference, ...extra }]; + +const metaMap = new Map([ + ['name', { name: 'name', type: 'text' }], + // The lookup's own `displayField` is asked before the target's title. + lookup('person', 'xt_person', { displayField: 'first_name' }), + lookup('contact', 'xt_contact'), + lookup('account', 'xt_account'), + lookup('member', 'xt_member'), +]); + +function protocol(overrides: Record> = {}) { + const refusals: Array<{ object: string; field: string; code: string; status: number }> = []; + const findData = vi.fn(async (args: FindArgs) => { + const decl = { ...DECLARATIONS[args.object], ...overrides[args.object] }; + const [[field, value]] = Object.entries(args.query!.where!); + if (decl.fields?.[field]?.type === 'formula') { + const err = Object.assign(new Error(`filters on '${field}', a virtual formula field`), { code: 'INVALID_FIELD', status: 400 }); + refusals.push({ object: args.object, field, code: err.code, status: err.status }); + throw err; + } + return (ROWS[args.object] ?? []).filter((r) => r[field] === value); + }); + const getMetaItem = vi.fn(async ({ name }: { type: string; name: string }) => + ({ type: 'object', name, item: { name, ...DECLARATIONS[name], ...overrides[name] } })); + const p: ImportProtocolLike = { findData, getMetaItem, createData: vi.fn(async (a) => ({ id: 'new', ...a.data })), updateData: vi.fn() }; + return { p, findData, getMetaItem, refusals }; +} + +const run = (p: ImportProtocolLike, rows: Row[]) => + runImport({ + p, context: { userId: 'u_member', roles: ['member'] }, objectName: 'xt_source', metaMap, writeMode: 'insert', + matchFields: [], dryRun: true, runAutomations: false, trimWhitespace: true, createMissingOptions: false, + skipBlankMatchKey: false, rows, + }); + +/** The `where` keys asked of one target, in order. */ +const askedOf = (findData: ReturnType['findData'], object: string) => + findData.mock.calls.filter(([a]) => a.object === object).map(([a]) => Object.keys(a.query!.where!)[0]); + +const verdict = (r: { ok: boolean; code?: string; field?: string }) => ({ ok: r.ok, code: r.code, field: r.field }); +const OK = { ok: true, code: undefined, field: undefined }; + +describe('[#24981] import resolves a lookup cell by the target object\'s stored title field', () => { + for (const [label, override] of [ + ['displayNameField (the card\'s cell, as authored)', {}], + ['nameField (the alias as the registry normalizes it)', { displayNameField: undefined, nameField: 'nickname' }], + ] as const) { + it(`Wren resolves through a stored ${label}, asked after the id and the lookup's displayField`, async () => { + const { p, findData, refusals } = protocol({ xt_person: override }); + const summary = await run(p, [{ name: 'a', person: 'Wren' }]); + expect(summary.results.map(verdict)).toEqual([OK]); + expect(askedOf(findData, 'xt_person')).toEqual(['id', 'first_name', 'nickname']); + expect(refusals).toEqual([]); + }); + } + + it('the stored title stores the matched record\'s id', async () => { + const { p, getMetaItem } = protocol(); + const createData = vi.fn(async (a: { data: Row }) => ({ id: 'new', ...a.data })); + p.createData = createData; + const summary = await runImport({ + p, context: {}, objectName: 'xt_source', metaMap, writeMode: 'insert', matchFields: [], dryRun: false, + runAutomations: false, trimWhitespace: true, createMissingOptions: false, skipBlankMatchKey: false, + rows: [{ name: 'a', person: 'Wren' }, { name: 'b', person: 'Wren' }], + }); + expect(summary.results.map(verdict)).toEqual([OK, OK]); + expect(createData.mock.calls.map(([a]) => a.data.person)).toEqual(['p1', 'p1']); + // The title comes off the declaration the exposure check already reads: + // one metadata read per target per import, however many rows ask it. + expect(getMetaItem.mock.calls.map(([r]) => r.name)).toEqual(['xt_person']); + }); + + for (const [cell, object, column, title] of [ + ['John Smith', 'xt_contact', 'contact', 'full_name'], + ['Acme (EMEA)', 'xt_account', 'account', 'display_title'], + ] as const) { + it(`a formula title (${object}.${title}) is never asked by where: '${cell}' still answers reference_not_found`, async () => { + const { p, findData, refusals } = protocol(); + const summary = await run(p, [{ name: 'a', [column]: cell }]); + expect(summary.results.map(verdict)).toEqual([{ ok: false, code: 'reference_not_found', field: column }]); + const asked = askedOf(findData, object); + expect(asked).not.toContain(title); + // The fixed fallback keys are unchanged; only a member the target + // declares virtual is skipped, which the engine could only refuse. + expect(asked).toEqual(['id', 'name', 'title', 'label', 'full_name', 'email', 'username'].filter((f) => f !== title)); + expect(refusals).toEqual([]); + }); + } + + it('two records sharing a stored title answer the import\'s ambiguity refusal, and link neither', async () => { + const { p, findData } = protocol(); + const summary = await run(p, [{ name: 'a', member: 'Robin' }]); + expect(summary.results.map(verdict)).toEqual([{ ok: false, code: 'reference_ambiguous', field: 'member' }]); + expect(askedOf(findData, 'xt_member')).toEqual(['id', 'nickname']); + }); + + it('[#22739] a target whose exposure refuses list still answers reference_not_found, and is never read', async () => { + const { p, findData, getMetaItem } = protocol({ xt_person: { enable: { apiEnabled: false } } }); + const summary = await run(p, [{ name: 'a', person: 'Wren' }, { name: 'b', person: 'Missing' }]); + const NOT_FOUND = { ok: false, code: 'reference_not_found', field: 'person' }; + expect(summary.results.map(verdict)).toEqual([NOT_FOUND, NOT_FOUND]); + expect(findData).not.toHaveBeenCalled(); + expect(getMetaItem).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/core/src/utils/import-runner.ts b/packages/core/src/utils/import-runner.ts index 86fcf499be8..d9b249d961d 100644 --- a/packages/core/src/utils/import-runner.ts +++ b/packages/core/src/utils/import-runner.ts @@ -4,7 +4,10 @@ import { randomUUID } from 'node:crypto'; import { coerceRow, type RefResolver, type RefMatch } from './import-coerce.js'; import type { ExportFieldMeta } from './import-field-meta.js'; import type { ValidationMessageTranslator } from '@objectstack/spec/system'; -import { canServeApiOperation, type DroppedFieldsEvent, type EnableLike } from '@objectstack/spec/data'; +import { + canServeApiOperation, isVirtualSearchField, resolveDisplayField, + type DisplayNameObjectMeta, type DroppedFieldsEvent, type EnableLike, type SearchFieldMeta, +} from '@objectstack/spec/data'; import type { ExecutionContext } from '@objectstack/spec/kernel'; import type { CreateDataRequest, FindDataRequest, GetMetaItemRequest, UpdateDataRequest, ValidateDataIssue, ValidateDataRequest, ValidateDataResponse } from '@objectstack/spec/api'; import { bulkWrite, withTransientRetry, defaultIsTransientError, type BulkWriteRowResult } from './bulk-write.js'; @@ -210,7 +213,8 @@ export interface ImportProtocolLike { /** * [#22739] The object-declaration read, the one the door's import template * and the connector pull already make. The reference resolution reads a - * lookup TARGET's `enable` block through it ({@link servesReferenceTarget}). + * lookup TARGET's `enable` block through it ({@link servesReferenceTarget}), + * and, from the same read, the target's title field and field map (#24981). * A protocol without it has no declaration to judge, and none is fabricated * for it; plugin-auth's identity import, whose reads run under the system * identity, carries it like every other caller (#22800). @@ -593,17 +597,50 @@ const REFERENCE_TARGET_OPERATION = 'list'; * caller. A refused target answers as a target the caller cannot read, which * is measured: every cell, matching or not, `reference_not_found`, and nothing * is read. A declaration that cannot be read withholds too (fail-closed). + * + * [#24981] A withheld target answers `null`. A served one answers what the + * resolution reads off the SAME declaration ({@link ServedReferenceTarget}), + * so the title costs no second read: still one read per target per import. */ -async function servesReferenceTarget(p: ImportProtocolLike, target: string): Promise { - if (typeof p.getMetaItem !== 'function') return true; - let enable: EnableLike | null | undefined; +async function servesReferenceTarget(p: ImportProtocolLike, target: string): Promise { + if (typeof p.getMetaItem !== 'function') return {}; + let item: ReferenceTargetDeclaration | undefined; try { - const found = (await p.getMetaItem({ type: 'object', name: target })) as { item?: { enable?: EnableLike | null } } | undefined; - enable = found?.item?.enable; + const found = (await p.getMetaItem({ type: 'object', name: target })) as { item?: ReferenceTargetDeclaration } | undefined; + item = found?.item; } catch { - return false; + return null; } - return canServeApiOperation(enable, REFERENCE_TARGET_OPERATION); + if (!canServeApiOperation(item?.enable, REFERENCE_TARGET_OPERATION)) return null; + if (!item || typeof item !== 'object') return {}; + return { + titleField: resolveDisplayField(item), + fields: item.fields as ServedReferenceTarget['fields'], + }; +} + +/** The slice of a lookup target's declaration the reference resolution reads. */ +type ReferenceTargetDeclaration = DisplayNameObjectMeta & { enable?: EnableLike | null }; + +/** + * [#24981] What a served lookup target tells the reference resolution, read off + * its declaration by {@link servesReferenceTarget}. + */ +interface ServedReferenceTarget { + /** + * The field that titles a record of the target: ADR-0079's + * `resolveDisplayField`, the one rule the list export writes a lookup cell + * by (`withReferenceTitleFields` in `@objectstack/rest`), so a cell the export + * wrote is asked by the field it was written from. `undefined` when the + * declaration resolves none, or no declaration was read. + */ + titleField?: string; + /** + * The target's declared field map. It is read for one question only: which + * candidate fields are virtual ({@link isVirtualSearchField}), because a + * `where` over a virtual field is refused by the engine's filter seam. + */ + fields?: Readonly>; } /** Upper bound on rows in one createManyData batch (framework#2678 suggests 100-500). */ @@ -678,24 +715,34 @@ export function runImport(opts: RunImportOptions): Promise { // (object, display) so a name repeated across rows costs one query. The // target's exposure is asked first, once per target per import (#22739). const refCache = new Map(); - const servedTargets = new Map>(); + const servedTargets = new Map>(); const resolveRef: RefResolver = async (referenceObject, display, meta) => { let served = servedTargets.get(referenceObject); if (!served) servedTargets.set(referenceObject, served = servesReferenceTarget(p, referenceObject)); - if (!(await served)) return {}; + const target = await served; + if (!target) return {}; const cacheKey = `${referenceObject}::${display}`; const cached = refCache.get(cacheKey); if (cached) return cached; // Try an exact id first (authoritative + unique when the user pasted an id), - // then the configured display field, then the usual human identifiers. - // De-dupe so a field isn't queried twice. The first candidate field to match - // wins; if that field matches >1 record we stop and report ambiguity rather - // than silently linking the first. + // then the configured display field, then the target's own title field + // (#24981: ADR-0079's rule, the field the list export writes the cell by), + // then the usual human identifiers. De-dupe so a field isn't queried twice. + // The first candidate field to match wins; if that field matches >1 record + // we stop and report ambiguity rather than silently linking the first. + // + // A candidate the target declares VIRTUAL (a `formula`) is never asked: the + // engine's filter seam (`assertFilterIsMaterializable`) refuses every + // `where` over one, judged by this same spec predicate, so its query could + // only answer a refusal the catch below reads as "field absent". Skipping + // it changes no verdict. A formula title therefore matches no cell here; + // whether and how one should is the open half of #24981. const candidates = [...new Set([ 'id', ...(meta.displayField ? [meta.displayField] : []), + ...(target.titleField ? [target.titleField] : []), 'name', 'title', 'label', 'full_name', 'email', 'username', - ])]; + ])].filter((f) => !isVirtualSearchField(target.fields?.[f])); const lookup = async (): Promise => { let match: RefMatch = {}; for (const f of candidates) {