Skip to content

Commit 6ef0eb0

Browse files
committed
fix(rest): apply inline import mapping entries' transform, defaultValue and required
Claude-Session: https://claude.ai/code/session_01CxBKycaiUpA1yGx1XyoGqX Co-authored-by: Claude <noreply@anthropic.com>
1 parent dc3e5b9 commit 6ef0eb0

2 files changed

Lines changed: 339 additions & 1 deletion

File tree

Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#25491] An inline `FieldMappingEntry[]` mapping's per-cell rules
5+
* (`transform`, `defaultValue`, `required`) are applied by the REAL
6+
* `POST /data/:object/import` route before the cells are coerced: they used to
7+
* be accepted and dropped, leaving only the rename. Driven through a real
8+
* ObjectQL engine + protocol on sqlite `:memory:`, as
9+
* `import-integration.test.ts` is.
10+
*/
11+
12+
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
13+
import { ObjectQL } from '@objectstack/objectql';
14+
import { SqlDriver } from '@objectstack/driver-sql';
15+
import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
16+
import { SysMetadataAuditObject, SysMetadataCommitObject, SysMetadataHistoryObject, SysMetadataObject } from '@objectstack/metadata-core';
17+
import { RestServer } from './rest-server';
18+
import { installDefaultOrganization } from './default-organization.testkit.js';
19+
20+
const SUPPLIER = {
21+
name: 'supplier', label: 'Supplier', systemFields: false,
22+
fields: {
23+
id: { name: 'id', type: 'text' as const, primaryKey: true },
24+
name: { name: 'name', type: 'text' as const, label: 'Name' },
25+
country: { name: 'country', type: 'text' as const, label: 'Country' },
26+
credit_code: { name: 'credit_code', type: 'text' as const, label: 'Credit code' },
27+
},
28+
validations: [{
29+
type: 'format' as const, name: 'credit_code_format', field: 'credit_code',
30+
regex: '^[0-9A-Z]{18}$', message: 'Credit code must be 18 digits or capital letters',
31+
}],
32+
};
33+
34+
const CONTACT = {
35+
name: 'contact', label: 'Contact', systemFields: false,
36+
fields: {
37+
id: { name: 'id', type: 'text' as const, primaryKey: true },
38+
supplier: { name: 'supplier', type: 'text' as const, label: 'Supplier' },
39+
email: { name: 'email', type: 'email' as const, label: 'Email' },
40+
name: { name: 'name', type: 'text' as const, label: 'Name' },
41+
},
42+
};
43+
44+
const liveEngines: ObjectQL[] = [];
45+
afterEach(async () => {
46+
while (liveEngines.length) {
47+
try { await liveEngines.pop()?.destroy(); } catch { /* noop */ }
48+
}
49+
});
50+
51+
function makeRes() {
52+
const res: any = {
53+
write: () => true, end: () => {},
54+
header: () => res,
55+
status: (code: number) => { res._status = code; return res; },
56+
json: (body: any) => { res._json = body; return res; },
57+
};
58+
return res;
59+
}
60+
61+
async function boot() {
62+
const engine = new ObjectQL();
63+
liveEngines.push(engine);
64+
engine.registerDriver(new SqlDriver({
65+
client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true,
66+
}), true);
67+
await engine.init();
68+
engine.registry.registerObject(SUPPLIER as any);
69+
engine.registry.registerObject(CONTACT as any);
70+
await engine.syncSchemas();
71+
await installDefaultOrganization(engine);
72+
for (const o of [SysMetadataObject, SysMetadataHistoryObject, SysMetadataAuditObject, SysMetadataCommitObject]) {
73+
if (!engine.registry.getObject(o.name)) engine.registry.registerObject(o as any);
74+
}
75+
const protocol = new ObjectStackProtocolImplementation(engine as any);
76+
const noop = () => {};
77+
const server = { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} };
78+
const rest = new RestServer(server as any, protocol as any, { api: { requireAuth: false } } as any);
79+
(rest as any).resolveExecCtx = async () => ({ userId: 'test-user' });
80+
rest.registerRoutes();
81+
const route = rest.getRoutes().find((r: any) => r.method === 'POST' && r.path === '/api/v1/data/:object/import');
82+
expect(route).toBeDefined();
83+
return { engine, route };
84+
}
85+
86+
describe('import route — inline FieldMappingEntry rules (#25491)', () => {
87+
let engine: any;
88+
let route: any;
89+
const call = async (object: string, body: any) => {
90+
const res = makeRes();
91+
await route.handler({ params: { object }, body } as any, res);
92+
return res;
93+
};
94+
95+
beforeEach(async () => {
96+
({ engine, route } = await boot());
97+
});
98+
99+
it('uppercase: a lowercase credit code passes the ^[0-9A-Z]{18}$ rule and is stored upper-cased (the card\'s reproduction)', async () => {
100+
const res = await call('supplier', {
101+
format: 'csv',
102+
csv: '公司全称,国家 / 地区,统一社会信用代码\n示例公司,中国,91000000ma0trial02\n',
103+
mapping: [
104+
{ sourceField: '公司全称', targetField: 'name' },
105+
{ sourceField: '国家 / 地区', targetField: 'country' },
106+
{ sourceField: '统一社会信用代码', targetField: 'credit_code', transform: 'uppercase' },
107+
],
108+
writeMode: 'upsert', matchFields: ['credit_code'],
109+
});
110+
expect(res._json).toMatchObject({ total: 1, ok: 1, errors: 0, created: 1 });
111+
const stored = await engine.find('supplier', {});
112+
expect(stored).toHaveLength(1);
113+
expect(stored[0]).toMatchObject({ name: '示例公司', country: '中国', credit_code: '91000000MA0TRIAL02' });
114+
});
115+
116+
it('lowercase: two spellings of one email merge into one contact under matchFields [supplier, email]', async () => {
117+
const mapping = [
118+
{ sourceField: 'Supplier', targetField: 'supplier' },
119+
{ sourceField: 'Email', targetField: 'email', transform: 'lowercase' },
120+
{ sourceField: 'Name', targetField: 'name' },
121+
];
122+
const first = await call('contact', {
123+
format: 'csv', csv: 'Supplier,Email,Name\ns1,ZHOU@X.EXAMPLE.COM,Zhou\n',
124+
mapping, writeMode: 'upsert', matchFields: ['supplier', 'email'],
125+
});
126+
expect(first._json).toMatchObject({ ok: 1, created: 1 });
127+
const second = await call('contact', {
128+
format: 'csv', csv: 'Supplier,Email,Name\ns1,zhou@x.example.com,Zhou Wei\n',
129+
mapping, writeMode: 'upsert', matchFields: ['supplier', 'email'],
130+
});
131+
expect(second._json).toMatchObject({ ok: 1, created: 0, updated: 1 });
132+
const stored = await engine.find('contact', {});
133+
expect(stored).toHaveLength(1);
134+
expect(stored[0]).toMatchObject({ supplier: 's1', email: 'zhou@x.example.com', name: 'Zhou Wei' });
135+
});
136+
137+
it('trim: strips the cell even when the request turns trimWhitespace off', async () => {
138+
const res = await call('supplier', {
139+
format: 'json', trimWhitespace: false,
140+
rows: [{ N: ' Acme ', C: ' CN ' }],
141+
mapping: [
142+
{ sourceField: 'N', targetField: 'name', transform: 'trim' },
143+
{ sourceField: 'C', targetField: 'country' },
144+
],
145+
});
146+
expect(res._json).toMatchObject({ ok: 1, created: 1 });
147+
const [stored] = await engine.find('supplier', {});
148+
expect(stored).toMatchObject({ name: 'Acme', country: ' CN ' });
149+
});
150+
151+
it('defaultValue: fills a blank cell and a missing column, leaves a filled cell alone', async () => {
152+
const res = await call('supplier', {
153+
format: 'csv', csv: 'Name,Country\nA,\nB,JP\n',
154+
mapping: [
155+
{ sourceField: 'Name', targetField: 'name' },
156+
{ sourceField: 'Country', targetField: 'country', defaultValue: 'CN' },
157+
{ sourceField: 'Code', targetField: 'credit_code', defaultValue: '000000000000000000' },
158+
],
159+
});
160+
expect(res._json).toMatchObject({ ok: 2, created: 2 });
161+
const stored = await engine.find('supplier', { orderBy: [{ field: 'name', order: 'asc' }] });
162+
expect(stored.map((r: any) => [r.name, r.country, r.credit_code])).toEqual([
163+
['A', 'CN', '000000000000000000'],
164+
['B', 'JP', '000000000000000000'],
165+
]);
166+
});
167+
168+
it('required: a blank required cell refuses the request before any row, on the dry run and the commit alike', async () => {
169+
const body = {
170+
format: 'csv', csv: 'Name,Country\nA,CN\nB,\n',
171+
mapping: [
172+
{ sourceField: 'Name', targetField: 'name' },
173+
{ sourceField: 'Country', targetField: 'country', required: true },
174+
],
175+
};
176+
const dry = await call('supplier', { ...body, dryRun: true });
177+
const commit = await call('supplier', body);
178+
for (const res of [dry, commit]) {
179+
expect(res._status).toBe(400);
180+
expect(res._json?.error?.code ?? res._json?.code).toBe('MISSING_REQUIRED_FIELD');
181+
expect(JSON.stringify(res._json)).toContain('row 2');
182+
}
183+
expect(await engine.find('supplier', {})).toHaveLength(0);
184+
});
185+
186+
it('required with a defaultValue is satisfied by the default', async () => {
187+
const res = await call('supplier', {
188+
format: 'csv', csv: 'Name,Country\nB,\n',
189+
mapping: [
190+
{ sourceField: 'Name', targetField: 'name' },
191+
{ sourceField: 'Country', targetField: 'country', required: true, defaultValue: 'CN' },
192+
],
193+
});
194+
expect(res._json).toMatchObject({ ok: 1, created: 1 });
195+
});
196+
197+
it('date_format is refused with 400 UNSUPPORTED_TRANSFORM naming it, and nothing is written', async () => {
198+
const res = await call('supplier', {
199+
format: 'csv', csv: 'Name\nA\n',
200+
mapping: [{ sourceField: 'Name', targetField: 'name', transform: 'date_format' }],
201+
});
202+
expect(res._status).toBe(400);
203+
expect(res._json?.error?.code ?? res._json?.code).toBe('UNSUPPORTED_TRANSFORM');
204+
expect(JSON.stringify(res._json)).toContain('date_format');
205+
expect(await engine.find('supplier', {})).toHaveLength(0);
206+
});
207+
208+
it('control: a plain { sourceField, targetField } array mapping is the rename it always was', async () => {
209+
const res = await call('supplier', {
210+
format: 'csv', csv: 'Name,Country\n Acme ,cn\n', trimWhitespace: false,
211+
mapping: [
212+
{ sourceField: 'Name', targetField: 'name' },
213+
{ sourceField: 'Country', targetField: 'country', transform: 'none' },
214+
],
215+
});
216+
expect(res._json).toMatchObject({ ok: 1, created: 1 });
217+
const [stored] = await engine.find('supplier', {});
218+
expect(stored).toMatchObject({ name: ' Acme ', country: 'cn' });
219+
});
220+
});

‎packages/rest/src/import-prepare.ts‎

Lines changed: 119 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
resolveNamedMapping, applyMappingToRows, refuseUnknownMappingTargets, type MappingArtifactLike,
1818
} from './import-mapping.js';
1919
import { asXlsxLoadInput, loadExcelJs } from './xlsx-module.js';
20+
import { isBlank } from '@objectstack/core';
2021

2122
/**
2223
* Minimal RFC-4180-style CSV parser used by the bulk-import endpoint
@@ -194,6 +195,112 @@ export async function parseXlsxToRows(
194195
return out;
195196
}
196197

198+
/**
199+
* [#25491] The per-cell rules of one inline `FieldMappingEntry`
200+
* (`ImportRequestSchema.mapping` in its array form), keyed by the entry's
201+
* target: the column has already been renamed to it when the rules run.
202+
*/
203+
interface InlineEntryRule {
204+
sourceField: string;
205+
targetField: string;
206+
transform: InlineTransform;
207+
hasDefault: boolean;
208+
defaultValue?: unknown;
209+
required: boolean;
210+
}
211+
212+
/**
213+
* The transforms an inline mapping entry may declare that this path applies,
214+
* each to a string cell (any other value passes through). `none` and `lookup`
215+
* copy the cell: a `lookup` cell is resolved to a record id by the import's
216+
* reference resolution downstream, as the named mapping's `lookup` is.
217+
*/
218+
const INLINE_TRANSFORMS = {
219+
none: (s: string) => s,
220+
lookup: (s: string) => s,
221+
trim: (s: string) => s.trim(),
222+
uppercase: (s: string) => s.toUpperCase(),
223+
lowercase: (s: string) => s.toLowerCase(),
224+
} as const;
225+
type InlineTransform = keyof typeof INLINE_TRANSFORMS;
226+
227+
type ImportRefusal = { ok: false; status: number; code: string; error: string };
228+
229+
/**
230+
* Read one inline entry's rules, refusing a transform this path does not
231+
* apply instead of running the import without it. `date_format` is declared
232+
* by the spec, but an entry carries no format for it to apply. Returns no
233+
* rule for a plain rename (no transform, no default, not required), which
234+
* leaves the row exactly as the rename built it.
235+
*/
236+
function readInlineEntryRule(
237+
e: { sourceField: string; targetField: string; transform?: unknown; defaultValue?: unknown; required?: unknown },
238+
): { ok: true; rule?: InlineEntryRule } | ImportRefusal {
239+
const transform = e.transform === undefined ? 'none' : e.transform;
240+
if (typeof transform !== 'string' || !Object.prototype.hasOwnProperty.call(INLINE_TRANSFORMS, transform)) {
241+
const named = typeof transform === 'string' ? `"${transform}"` : String(transform);
242+
const why = transform === 'date_format'
243+
? 'a mapping entry carries no date format for it to apply; a cell mapped to a date or datetime field is already parsed as a date, so drop the transform'
244+
: `the transforms a mapping entry may declare are ${Object.keys(INLINE_TRANSFORMS).join(', ')}`;
245+
return {
246+
ok: false, status: 400, code: 'UNSUPPORTED_TRANSFORM',
247+
error: `Mapping entry "${e.sourceField}" to "${e.targetField}" declares transform ${named}, which the import path does not apply, `
248+
+ `so the import is refused rather than run with that transform skipped: ${why}.`,
249+
};
250+
}
251+
const hasDefault = e.defaultValue !== undefined;
252+
const required = e.required === true;
253+
if (transform === 'none' && !hasDefault && !required) return { ok: true };
254+
return {
255+
ok: true,
256+
rule: {
257+
sourceField: e.sourceField, targetField: e.targetField,
258+
transform: transform as InlineTransform, hasDefault, defaultValue: e.defaultValue, required,
259+
},
260+
};
261+
}
262+
263+
/**
264+
* Apply each inline entry's rules to every row, in place, before the cells
265+
* are coerced: the transform to a string cell, then `defaultValue` to a cell
266+
* that is blank (what cell coercion calls blank: absent, empty, whitespace or
267+
* one of the request's `nullValues`), then `required` to what is left. A
268+
* required cell still blank refuses the whole request before any row is
269+
* written, on the dry run and the commit alike, naming the column and the
270+
* rows (numbered as the row report numbers them).
271+
*/
272+
function applyInlineEntryRules(
273+
rows: Array<Record<string, any>>,
274+
rules: InlineEntryRule[],
275+
nullValues: string[] | undefined,
276+
): ImportRefusal | undefined {
277+
const misses = new Map<InlineEntryRule, number[]>();
278+
rows.forEach((row, i) => {
279+
for (const rule of rules) {
280+
let value = row[rule.targetField];
281+
if (typeof value === 'string') value = INLINE_TRANSFORMS[rule.transform](value);
282+
if (rule.hasDefault && isBlank(value, nullValues)) value = rule.defaultValue;
283+
if (value !== undefined) row[rule.targetField] = value;
284+
if (rule.required && isBlank(value, nullValues)) {
285+
const at = misses.get(rule) ?? [];
286+
at.push(i + 1);
287+
misses.set(rule, at);
288+
}
289+
}
290+
});
291+
if (misses.size === 0) return undefined;
292+
const listed = [...misses].map(([rule, at]) => {
293+
const shown = at.length > 10 ? `${at.slice(0, 10).join(', ')} and ${at.length - 10} more` : at.join(', ');
294+
return `"${rule.sourceField}" to "${rule.targetField}" (row${at.length === 1 ? '' : 's'} ${shown})`;
295+
});
296+
return {
297+
ok: false, status: 400, code: 'MISSING_REQUIRED_FIELD',
298+
error: `The mapping marks ${listed.length === 1 ? 'a column' : 'columns'} required that some rows leave blank: ${listed.join('; ')}. `
299+
+ 'The import is refused before any row, on the dry run and the commit alike. '
300+
+ 'Fill those cells, give the entry a defaultValue, or drop required from it.',
301+
};
302+
}
303+
197304
/** Everything the import runner needs, parsed & validated from a request body. */
198305
export interface PreparedImport {
199306
rows: Array<Record<string, any>>;
@@ -453,12 +560,18 @@ export async function prepareImportRequest(
453560
}
454561

455562
// Normalize `mapping` to a `{ sourceColumn: targetField }` record. Accepts
456-
// either that compact form or a FieldMappingEntry[].
563+
// either that compact form or a FieldMappingEntry[]. An entry's own
564+
// per-cell rules (`transform` / `defaultValue` / `required`) are kept
565+
// beside the rename and applied once the rows are built (#25491).
457566
const mapping: Record<string, string> = {};
567+
const entryRules: InlineEntryRule[] = [];
458568
if (Array.isArray(body?.mapping)) {
459569
for (const e of body.mapping) {
460570
if (e && typeof e.sourceField === 'string' && typeof e.targetField === 'string') {
461571
mapping[e.sourceField] = e.targetField;
572+
const read = readInlineEntryRule(e);
573+
if (!read.ok) return read;
574+
if (read.rule) entryRules.push(read.rule);
462575
}
463576
}
464577
} else if (body?.mapping && typeof body.mapping === 'object') {
@@ -499,6 +612,11 @@ export async function prepareImportRequest(
499612
return { ok: false, status: 413, code: 'PAYLOAD_TOO_LARGE', error: `Import limit is ${maxRows} rows per request (got ${rows.length}).` };
500613
}
501614

615+
if (entryRules.length > 0) {
616+
const refused = applyInlineEntryRules(rows, entryRules, nullValues);
617+
if (refused) return refused;
618+
}
619+
502620
// Resolve the object's field metadata so cells coerce to storage values
503621
// (booleans, numbers, dates→ISO, select label→code) and lookup names resolve
504622
// to record ids. Best-effort: a failed lookup leaves `metaMap` empty and

0 commit comments

Comments
 (0)