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/26684-import-dry-run-batch-preview.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
'@objectstack/core': patch
'@objectstack/objectql': patch
---

fix(core)!: the import dry run previews create rows in the commit's batches, so a batch-aware parent-summary rule refuses in the dry run the rows the commit refuses (#26684)

Clause-②: no (narrowing) — an import dry run (`POST /api/v1/data/:object/import` and the async import job with `dryRun: true`) answers invalid for a create row that a child rule reading a parent `summary` refuses once the earlier admitted rows of its batch are counted, which it answered valid before; and the insert preview (`validate` / `validateData`, mode `insert`) refuses a row whose caller-supplied reference names no row, which it answered valid before. Nothing is widened.
Metadata-protocol: none

<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type changes: the dry run now reports the verdicts the import's own write already gives on the same file. The refusals are the rule's own VALIDATION_FAILED, worded by its author, and the write's reference_not_found, so there is nothing for a migration to rewrite and no tombstone. -->

**BREAKING** accept-set narrowing at the import dry run (`@objectstack/core`) and the insert preview (`@objectstack/objectql`).

- **Before.** The dry run asked the engine about each create row alone, so after #25579 it answered both rows valid when a file held two primary contacts for one new supplier, while the import created one and refused the other. The insert preview judged a row's rules but not the write's reference check, so a row with a dangling reference previewed valid and was counted toward a later row's parent summary.
- **Now.** The dry run buffers its create rows as the import does and previews them through one multi-row `validate()` per import batch (200 rows, or a smaller `progressEvery`), in file order, so the engine counts the earlier rows of the batch exactly as on the write. Each row keeps its own verdict, message and position in the report. The insert preview runs the write's reference check after the rules, behind the same write-gate probe as its related reads, and counts a row only after it passes.
- **Unchanged.** Update rows are previewed one at a time, as before. A dry run writes nothing.
- **Not covered.** A dry run writes nothing between batches, so two rows that only conflict across a batch boundary both preview valid while the import refuses the second.
- **Who sees it.** An operator dry-running an import into an object whose child rule reads a parent summary: rows the import would refuse are now reported in the dry run.

**The remedy.** None needed: fix the rows the dry run now reports, as the import would have required.
5 changes: 5 additions & 0 deletions content/docs/data-modeling/fields.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,11 @@ refuse is not counted. This holds for `count`, and for `sum`, `min` and `max`
over number values. An `avg` summary, a `sum`/`min`/`max` meeting a value that
is not a number, and a summary whose `filter` reads a dotted path through
another reference keep reading the stored value, as they did before.
An import's **dry run** previews its create rows in the batches the import
writes them in (200 rows each) and refuses the rows the import refuses, with
the same message. It writes nothing, so a row in a later batch reads the
summary as stored before the import: two such rows that fall in different
batches of a dry run both preview valid, while the import refuses the second.
**Concurrent writes are not guaranteed (并发不保证):** two writes in flight at
the same time each read the summary they started from, so a guard like the one
above can still admit one row from each.
Expand Down
16 changes: 10 additions & 6 deletions packages/core/src/utils/import-runner-dropped-fields.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ const withDrops = (row: Record<string, unknown>) => {
return d.length > 0 ? { droppedFields: d } : {};
};

/** The submitted rows of a `validateData` call: one verdict is owed per row (#26684 previews a batch). */
const rowsOf = (args: ValidateArgs) =>
(Array.isArray(args.data) ? args.data : [args.data]) as Array<Record<string, unknown>>;

/** The row's key set — a clean row must not grow an empty `droppedFields`. */
const keysOf = (r: ImportRowResult | undefined) => Object.keys(r ?? {}).sort();

Expand All @@ -85,7 +89,7 @@ describe('[#20701] the dry run copies validateData\'s per-row drops onto the row
it('a create-resolved row carries the verdict\'s droppedFields verbatim; a clean row carries no key', async () => {
const validateData = vi.fn(async (args: ValidateArgs) => ({
object: OBJECT, mode: args.mode ?? 'insert', valid: true,
results: [{ valid: true, errors: [], warnings: [], ...withDrops(args.data as Record<string, unknown>) }],
results: rowsOf(args).map((row) => ({ valid: true, errors: [], warnings: [], ...withDrops(row) })),
posture: { valueShapeStrict: false, mediaValueShapeStrict: false },
}));
const p: ImportProtocolLike = { findData: vi.fn(async () => []), createData: vi.fn(), updateData: vi.fn(), validateData };
Expand Down Expand Up @@ -116,12 +120,12 @@ describe('[#20701] the dry run copies validateData\'s per-row drops onto the row
it('a row the verdict refuses carries no droppedFields, and an admitted warning rides beside the drops', async () => {
const warning = { field: 'name', code: 'invalid_format', message: 'admitted' };
const validateData = vi.fn(async (args: ValidateArgs) => {
const refused = (args.data as Record<string, unknown>).name === 'bad';
const results = rowsOf(args).map((row) => (row.name === 'bad'
? { valid: false, errors: [{ field: 'name', code: 'required', message: 'name is required' }], warnings: [], droppedFields: [COMPUTED] }
: { valid: true, errors: [], warnings: [warning], droppedFields: [COMPUTED] }));
return {
object: OBJECT, mode: 'insert' as const, valid: !refused,
results: [refused
? { valid: false, errors: [{ field: 'name', code: 'required', message: 'name is required' }], warnings: [], droppedFields: [COMPUTED] }
: { valid: true, errors: [], warnings: [warning], droppedFields: [COMPUTED] }],
object: OBJECT, mode: 'insert' as const, valid: results.every((r) => r.valid),
results,
posture: { valueShapeStrict: false, mediaValueShapeStrict: false },
};
});
Expand Down
149 changes: 149 additions & 0 deletions packages/core/src/utils/import-runner-dry-run-batch.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#26684] How the import dry run asks the engine for its verdicts: a create
* row is previewed with the rows of the batch the commit would write it in,
* in ONE `validateData` call, and an update row alone, as before.
*
* Driven against an `ImportProtocolLike` double that answers one verdict per
* submitted row (the `ValidateDataResponse` contract) and records each call.
* What the batch buys — a child rule reading a parent roll-up counting the
* earlier rows — is pinned end to end over a real engine in
* `packages/rest/src/import-dry-run-batch-parent-summary.test.ts`.
*
* Pinned here:
* - the batches are the commit's: file order, the commit's checkpoints
* (every `progressEvery` rows) and run length
* (`Math.min(progressEvery, MAX_CREATE_BATCH_SIZE)`, 200);
* - each row keeps its own report, at its own position, between update rows;
* - an update row is asked alone, in `update` mode, as before;
* - a batch the engine refuses outright is degraded as the commit degrades
* one: each row alone, then the survivors together again.
*/

import { describe, it, expect, vi } from 'vitest';
import { runImport, type ImportProtocolLike } from './import-runner';
import type { ExportFieldMeta } from './import-field-meta.js';

type ValidateArgs = Parameters<NonNullable<ImportProtocolLike['validateData']>>[0];

const OBJECT = 'contact';

const baseOpts = {
objectName: OBJECT,
metaMap: new Map<string, ExportFieldMeta>([['name', { name: 'name', type: 'text' }]]),
writeMode: 'insert' as const,
matchFields: [] as string[],
dryRun: true,
runAutomations: false,
trimWhitespace: true,
createMissingOptions: false,
skipBlankMatchKey: false,
};

const POSTURE = { valueShapeStrict: false, mediaValueShapeStrict: false };

/**
* One verdict per submitted row; a row named `bad*` is refused. A batch that
* holds a row named `boom` is refused outright, as the engine throws on an
* undeclared field.
*/
function validatingProtocol(findData: ImportProtocolLike['findData'] = vi.fn(async () => [])) {
const validateData = vi.fn(async (args: ValidateArgs) => {
const rows = (Array.isArray(args.data) ? args.data : [args.data]) as Array<Record<string, unknown>>;
if (rows.some((r) => r.name === 'boom')) throw Object.assign(new Error('boom is not a field'), { code: 'INVALID_FIELD' });
const results = rows.map((r) => String(r.name).startsWith('bad')
? { valid: false, errors: [{ field: 'name', code: 'rule_violation', message: `refused ${String(r.name)}` }], warnings: [] }
: { valid: true, errors: [], warnings: [] });
return { object: OBJECT, mode: args.mode ?? 'insert', valid: results.every((r) => r.valid), results, posture: POSTURE };
});
const p: ImportProtocolLike = { findData, createData: vi.fn(), updateData: vi.fn(), validateData };
return { p, validateData };
}

const names = (call: [ValidateArgs]) => {
const data = call[0].data;
return Array.isArray(data) ? data.map((r) => (r as Record<string, unknown>).name) : (data as Record<string, unknown>).name;
};

describe('[#26684] the dry run previews create rows in the commit\'s batches', () => {
it('the create rows of a small file are ONE insert-mode call, in file order, each row keeping its own verdict', async () => {
const { p, validateData } = validatingProtocol();
const s = await runImport({ ...baseOpts, p, rows: [{ name: 'a' }, { name: 'bad-b' }, { name: 'c' }] });

expect(validateData).toHaveBeenCalledTimes(1);
expect(validateData.mock.calls[0]![0]).toMatchObject({ object: OBJECT, mode: 'insert' });
expect(names(validateData.mock.calls[0] as [ValidateArgs])).toEqual(['a', 'bad-b', 'c']);
expect(s).toMatchObject({ ok: 2, errors: 1, created: 2 });
expect(s.results).toEqual([
{ row: 1, ok: true, action: 'created' },
{ row: 2, ok: false, action: 'failed', field: 'name', code: 'rule_violation', error: 'refused bad-b' },
{ row: 3, ok: true, action: 'created' },
]);
});

it('a file of 450 rows is previewed in the commit\'s runs: 200, 200, 50', async () => {
const { p, validateData } = validatingProtocol();
const rows = Array.from({ length: 450 }, (_, n) => ({ name: `r${n + 1}` }));
const s = await runImport({ ...baseOpts, p, rows });

expect(validateData.mock.calls.map((c) => (c[0].data as unknown[]).length)).toEqual([200, 200, 50]);
expect(names(validateData.mock.calls[1] as [ValidateArgs])).toEqual(rows.slice(200, 400).map((r) => r.name));
expect(s).toMatchObject({ ok: 450, created: 450 });
expect(s.results.map((r) => r.row)).toEqual(rows.map((_, n) => n + 1));
});

it('a caller progressEvery sets the checkpoints, and the run length follows it, as on the commit', async () => {
const { p, validateData } = validatingProtocol();
const rows = Array.from({ length: 7 }, (_, n) => ({ name: `r${n + 1}` }));
const progress: number[] = [];
await runImport({ ...baseOpts, p, rows, progressEvery: 3, onProgress: (pr) => { progress.push(pr.created); } });

expect(validateData.mock.calls.map((c) => names(c as [ValidateArgs]))).toEqual([
['r1', 'r2', 'r3'], ['r4', 'r5', 'r6'], ['r7'],
]);
// Each checkpoint reports the rows previewed so far, as the commit reports the rows written.
expect(progress).toEqual([3, 6, 7]);
});

it('an update row is asked alone, in update mode, and the create rows around it keep their batch', async () => {
const findData = vi.fn(async (args: any) => {
const where = JSON.stringify(args ?? {});
return where.includes('"u1"') ? [{ id: 'e1', name: 'u1' }] : [];
});
const { p, validateData } = validatingProtocol(findData as any);
const s = await runImport({
...baseOpts, p, writeMode: 'upsert', matchFields: ['name'],
rows: [{ name: 'a' }, { name: 'u1' }, { name: 'bad-c' }],
});

const update = validateData.mock.calls.find((c) => c[0].mode === 'update')!;
expect(update[0].data).toEqual({ name: 'u1', id: 'e1' });
const insert = validateData.mock.calls.filter((c) => c[0].mode !== 'update');
expect(insert.map((c) => names(c as [ValidateArgs]))).toEqual([['a', 'bad-c']]);
expect(s.results).toEqual([
{ row: 1, ok: true, action: 'created' },
{ row: 2, ok: true, action: 'updated', id: 'e1' },
{ row: 3, ok: false, action: 'failed', field: 'name', code: 'rule_violation', error: 'refused bad-c' },
]);
});

it('a batch the engine refuses outright is asked row by row, then the survivors together again', async () => {
const { p, validateData } = validatingProtocol();
const s = await runImport({ ...baseOpts, p, rows: [{ name: 'a' }, { name: 'boom' }, { name: 'c' }] });

expect(validateData.mock.calls.map((c) => names(c as [ValidateArgs]))).toEqual([
['a', 'boom', 'c'], ['a'], ['boom'], ['c'], ['a', 'c'],
]);
expect(s.results[0]).toEqual({ row: 1, ok: true, action: 'created' });
expect(s.results[1]).toMatchObject({ row: 2, ok: false, action: 'failed', code: 'INVALID_FIELD' });
expect(s.results[2]).toEqual({ row: 3, ok: true, action: 'created' });
});

it('no validate-only operation: every create row is reported on its coercion verdict, as before', async () => {
const p: ImportProtocolLike = { findData: vi.fn(async () => []), createData: vi.fn(), updateData: vi.fn() };
const s = await runImport({ ...baseOpts, p, rows: [{ name: 'a' }, { name: 'b' }] });
expect(s).toMatchObject({ ok: 2, created: 2, errors: 0 });
expect(p.createData).not.toHaveBeenCalled();
});
});
23 changes: 16 additions & 7 deletions packages/core/src/utils/import-runner-kept-options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,9 @@ function makeProvider(shape: 'inline' | 'createMany' | 'insertMany' | 'update' |
}
if (shape === 'validate') {
p.validateData = vi.fn(async (args: ValidateArgs) => {
calls.push({ kind: 'validateData', rows: [args.data as Record<string, unknown>], context: args.context });
return { results: [{ valid: true, errors: [] }] } as any;
const rows = (Array.isArray(args.data) ? args.data : [args.data]) as Array<Record<string, unknown>>;
calls.push({ kind: 'validateData', rows, context: args.context });
return { results: rows.map(() => ({ valid: true, errors: [] })) } as any;
});
}
return { p, calls };
Expand Down Expand Up @@ -135,14 +136,22 @@ describe('runImport — createMissingOptions hands the kept values to the write
expect(calls[0].context.keptOptionValues).toEqual({ priority: ['Bogus'] });
});

it('the dry run asks validateData with the same per-row context the write would carry', async () => {
it('the dry run asks validateData with the context the batch write would carry: the union of its rows\' kept values', async () => {
// [#26684] The dry run previews the create rows as the batch the commit
// writes them in, so its one call carries that batch call's context.
const { p, calls } = makeProvider('validate');
const summary = await runImport({ ...baseOpts, p, dryRun: true, rows: ROWS });
expect(summary.created).toBe(3);
expect(calls.map((c) => c.kind)).toEqual(['validateData', 'validateData', 'validateData']);
expect(calls[0].context.keptOptionValues).toEqual({ priority: ['Bogus'] });
expect(calls[1].context).not.toHaveProperty('keptOptionValues');
expect(calls[2].context.keptOptionValues).toEqual({ tags: ['Other'] });
expect(calls.map((c) => c.kind)).toEqual(['validateData']);
expect(calls[0].rows).toHaveLength(3);
expect(calls[0].context.keptOptionValues).toEqual({ priority: ['Bogus'], tags: ['Other'] });
});

it('a dry run whose rows kept nothing carries the import\'s write context unchanged', async () => {
const { p, calls } = makeProvider('validate');
await runImport({ ...baseOpts, p, dryRun: true, rows: [{ code: 'b', priority: 'High' }] });
expect(calls).toHaveLength(1);
expect(calls[0].context).not.toHaveProperty('keptOptionValues');
});

it('with the option off, an unmatched cell fails at coercion and no write carries the key', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ describe('[#22445] the dry run of a matched row names the stored row it would up
await runImport({ ...baseOpts, p, writeMode: 'upsert', dryRun: true, rows: [{ code: 'c8', title: 'fresh' }] });

expect(validateData.mock.calls[0]![0]).toMatchObject({ mode: 'insert' });
expect(validateData.mock.calls[0]![0].data).toEqual({ code: 'c8', title: 'fresh' });
// [#26684] A create row is previewed with the rows of its batch: an array.
expect(validateData.mock.calls[0]![0].data).toEqual([{ code: 'c8', title: 'fresh' }]);
});
});
Loading
Loading