Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .changeset/import-dryrun-required-validation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
'@objectstack/rest': patch
---

fix(rest): validate required fields in import dry-run to match the real insert

The bulk-import dry run (`POST /data/:object/import`, `dryRun:true`) only ran cell
coercion and reported every coercible CREATE row as ok — so a row missing a required
NOT-NULL field with no default was green-lit, then died on the real insert with
`NOT NULL constraint failed`. The ImportWizard shows the dry-run result, so it
promised imports that then failed.

Add a required-field pre-check to the shared import runner (CREATE rows only),
mirroring the engine's insert-time validation (`objectql/record-validator.ts` +
`applyFieldDefaults`): a required field is unsatisfied only when it has no value AND
no default; `system`/`readonly`/`autonumber` and the engine-owned lifecycle columns
are exempt. `ExportFieldMeta` gains `required`/`system`/`readonly`/`hasDefault`
(populated by `buildFieldMetaMap`). Applied to both dry-run and real paths so they
stay identical and a real insert returns a readable `<field> is required` instead of
a raw driver error; skipped when `runAutomations` is set (a beforeInsert hook may
populate the field).
20 changes: 20 additions & 0 deletions packages/rest/src/export-format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,19 @@ export interface ExportFieldMeta {
reference?: string;
/** Field on the referenced record to show as its label. */
displayField?: string;
// The following four drive the import path's required-field pre-check
// (import-runner.ts). They mirror the engine's insert-time validation
// (objectql record-validator.ts) so a dry run can predict a NOT NULL /
// required failure instead of green-lighting a row the real insert rejects.
// Unused by the export path (formatting only reads type/options/reference).
/** Field is required — a value (or default) must exist on insert. */
required?: boolean;
/** Engine-owned column the client never supplies (never required of import). */
system?: boolean;
/** Read-only column the client never supplies (never required of import). */
readonly?: boolean;
/** Field declares a `defaultValue` the engine applies on insert (satisfies required). */
hasDefault?: boolean;
}

/** Field types whose stored value points at another record. */
Expand Down Expand Up @@ -78,6 +91,13 @@ export function buildFieldMetaMap(schema: unknown): Map<string, ExportFieldMeta>
options: Array.isArray(f.options) ? f.options : undefined,
reference: typeof f.reference === 'string' ? f.reference : undefined,
displayField: typeof f.displayField === 'string' ? f.displayField : undefined,
required: f.required === true,
system: f.system === true,
readonly: f.readonly === true,
// Mirror the engine's `applyFieldDefaults` gate (`f.defaultValue == null`
// ⇒ no default): any non-null default — literal, expression object, or the
// `current_user` token — counts as satisfying a required field.
hasDefault: f.defaultValue != null,
});
}
return map;
Expand Down
47 changes: 47 additions & 0 deletions packages/rest/src/import-coerce.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,53 @@ export async function coerceFieldValue(
return { value: trim && typeof raw === 'string' ? raw.trim() : raw };
}

// ── required-field pre-check ───────────────────────────────────────

/**
* Engine-owned lifecycle columns the client never supplies. Mirrors
* `record-validator.ts`'s `SKIP_FIELDS`, so the import's required pre-check and
* the engine's insert-time required check agree on which fields the caller is
* responsible for.
*/
const REQUIRED_CHECK_SKIP = new Set<string>([
'id', 'created_at', 'created_by', 'updated_at', 'updated_by',
]);

function isBlankValue(v: unknown): boolean {
return v === undefined || v === null || (typeof v === 'string' && v.trim() === '');
}

/**
* The first required field a would-be CREATE leaves unsatisfied, or `null` when
* every required field has either a mapped value or a schema default.
*
* This mirrors the engine's insert-time required check (objectql
* `record-validator.ts` + `applyFieldDefaults`) so the import's dry run predicts
* the SAME verdict the real insert produces: a required (⇒ NOT NULL) field with
* no default and no value fails both. Without it, dry run only reports coercion
* errors and green-lights a row that then dies on `NOT NULL constraint failed`.
*
* Matches the engine's exemptions exactly — `system`/`readonly` columns, the
* runtime-generated `autonumber`, a field carrying a `defaultValue`, and the
* engine-owned lifecycle columns are never required of the importer. Applies to
* CREATE only; an UPDATE touches just the supplied fields, so callers gate on
* "will create" before calling.
*/
export function firstMissingRequiredField(
data: Record<string, unknown>,
metaMap: Map<string, ExportFieldMeta>,
): string | null {
for (const meta of metaMap.values()) {
if (!meta.required) continue;
if (meta.system || meta.readonly) continue;
if (meta.hasDefault) continue;
if (meta.type === 'autonumber') continue;
if (REQUIRED_CHECK_SKIP.has(meta.name)) continue;
if (isBlankValue(data[meta.name])) return meta.name;
}
return null;
}

/**
* Coerce a whole raw row into a storage-ready record. Unknown columns (no
* matching field metadata) pass through untouched so ad-hoc / schemaless
Expand Down
91 changes: 91 additions & 0 deletions packages/rest/src/import-integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,31 @@ const TASK = {
},
};

// Mirrors an AI-built object with required fields and NO default (framework
// import dry-run fidelity): `member_name` (required text) and `status` (required
// select, no default) must be present on create; `tier` is required but carries
// a default, so the engine fills it and the importer must NOT demand it.
const MEMBER = {
name: 'member', label: 'Member', systemFields: false,
fields: {
id: { name: 'id', type: 'text' as const, primaryKey: true },
member_name: { name: 'member_name', type: 'text' as const, label: 'Name', required: true },
status: {
name: 'status', type: 'select' as const, label: 'Status', required: true,
options: [
{ label: 'Active', value: 'active' },
{ label: 'Frozen', value: 'frozen' },
{ label: 'Lost Contact', value: 'lost_contact' },
{ label: 'Archived', value: 'archived' },
],
},
tier: {
name: 'tier', type: 'select' as const, label: 'Tier', required: true, defaultValue: 'standard',
options: [{ label: 'Standard', value: 'standard' }, { label: 'Gold', value: 'gold' }],
},
},
};

function createMockServer() {
const noop = () => {};
return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} };
Expand All @@ -119,6 +144,7 @@ async function boot() {
await engine.init();
engine.registry.registerObject(USER as any);
engine.registry.registerObject(TASK as any);
engine.registry.registerObject(MEMBER as any);
await engine.insert('user', { id: 'u1', name: '张三', email: 'zhang@x.com' });
await engine.insert('user', { id: 'u2', name: '李四', email: 'li@x.com' });

Expand Down Expand Up @@ -310,6 +336,71 @@ describe('import route — real engine + protocol integration', () => {
});
});

// ---------------------------------------------------------------------------
// Required-field dry-run fidelity — the dry run must predict the real insert's
// NOT NULL / required failures instead of green-lighting a row the insert
// rejects. Mirrors the live mx1n_member case (required `status` select, no
// default): dryRun said ok, the real insert died on a NOT NULL constraint.
// ---------------------------------------------------------------------------
describe('import route — required-field dry-run fidelity', () => {
let route: any;
let engine: any;
beforeEach(async () => { ({ route, engine } = await boot()); });

const imp = (body: any) => {
const res = makeRes();
return route.handler({ params: { object: 'member' }, body } as any, res).then(() => res);
};

it('dry run fails a create row missing a required no-default field — and the real insert agrees', async () => {
const rows = [
{ id: 'm1', member_name: 'Alice' }, // status missing → must fail
{ id: 'm2', member_name: 'Bob', status: 'active' }, // complete → ok
];
// Dry run: no longer reports success for the row the insert will reject.
const dry = await imp({ format: 'json', dryRun: true, rows });
expect(dry._json).toMatchObject({ dryRun: true, total: 2, ok: 1, errors: 1 });
expect(dry._json.results.find((r: any) => !r.ok)).toMatchObject({ row: 1, field: 'status', code: 'required' });
expect(await engine.findOne('member', { where: { id: 'm2' } })).toBeNull(); // dry run never writes

// Real insert: SAME verdict (parity), and a readable `status is required`
// instead of a raw `NOT NULL constraint failed: member.status`.
const real = await imp({ format: 'json', rows });
expect(real._json).toMatchObject({ total: 2, ok: 1, errors: 1, created: 1 });
expect(real._json.results.find((r: any) => !r.ok)).toMatchObject({ field: 'status', code: 'required', error: 'status is required' });
expect((await engine.findOne('member', { where: { id: 'm2' } }))?.status).toBe('active');
expect(await engine.findOne('member', { where: { id: 'm1' } })).toBeNull();
});

it('a required field with a schema default is satisfied without being mapped', async () => {
// `tier` is required but defaulted — the importer must not demand it; the
// engine fills 'standard'. Only member_name + status are supplied.
const res = await imp({ format: 'json', rows: [{ id: 'm3', member_name: 'Cara', status: 'frozen' }] });
expect(res._json).toMatchObject({ ok: 1, errors: 0, created: 1 });
expect(await engine.findOne('member', { where: { id: 'm3' } }))
.toMatchObject({ member_name: 'Cara', status: 'frozen', tier: 'standard' });
});

it('flags a required text field too (not just selects); a blank cell counts as missing', async () => {
const res = await imp({ format: 'json', dryRun: true, rows: [
{ id: 'm4', status: 'active' }, // member_name missing
{ id: 'm5', member_name: ' ', status: 'active' }, // member_name blank
] });
expect(res._json).toMatchObject({ ok: 0, errors: 2 });
for (const r of res._json.results) expect(r).toMatchObject({ field: 'member_name', code: 'required' });
});

it('required check does not apply to update-mode rows (only the touched fields matter)', async () => {
await engine.insert('member', { id: 'm6', member_name: 'Dan', status: 'active', tier: 'gold' });
// writeMode:update on an existing match, touching only member_name — status
// is not supplied but the record already has it, so this must NOT fail.
const res = await imp({ format: 'json', writeMode: 'update', matchFields: ['id'],
rows: [{ id: 'm6', member_name: 'Daniel' }] });
expect(res._json).toMatchObject({ ok: 1, errors: 0, updated: 1 });
expect((await engine.findOne('member', { where: { id: 'm6' } }))?.member_name).toBe('Daniel');
});
});

// ---------------------------------------------------------------------------
// Named mapping artifacts (#2611) — `mappingName` resolves a registered
// `mapping` item and applies its fieldMapping pipeline before coercion.
Expand Down
19 changes: 17 additions & 2 deletions packages/rest/src/import-runner.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

import { coerceRow, type RefResolver, type RefMatch } from './import-coerce.js';
import { coerceRow, firstMissingRequiredField, type RefResolver, type RefMatch } from './import-coerce.js';
import type { ExportFieldMeta } from './export-format.js';
import { bulkWrite, withTransientRetry, type BulkWriteRowResult } from '@objectstack/core';

Expand Down Expand Up @@ -310,7 +310,22 @@ export function runImport(opts: RunImportOptions): Promise<ImportRunSummary> {
const willUpdate = existing && typeof existing === 'object';
const willCreate = !willUpdate && (writeMode === 'insert' || writeMode === 'upsert');

if (!willUpdate && !willCreate) {
// Required-field pre-check (CREATE only). Give dry run the same
// verdict the real insert produces — a required (⇒ NOT NULL) field
// with no default and no value fails both — instead of reporting
// success for a row that then dies on `NOT NULL constraint failed`.
// Shared by both paths so they stay identical (and a real insert
// gets a readable `<field> is required` instead of a raw driver
// error). Skipped when automations run: a beforeInsert hook may
// populate a required field, so we defer to the engine's own
// validation rather than false-reject here.
const requiredMiss =
willCreate && !runAutomations ? firstMissingRequiredField(data, metaMap) : null;

if (requiredMiss) {
errCount++;
results[i] = { row: rowNo, ok: false, action: 'failed', field: requiredMiss, code: 'required', error: `${requiredMiss} is required` };
} else if (!willUpdate && !willCreate) {
// update mode, no match → skip.
skipped++;
results[i] = { row: rowNo, ok: true, action: 'skipped', code: 'NO_MATCH' };
Expand Down