From 3585054dfcabf77677390af46ed7d10ec6de8e74 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 14:29:11 +0000 Subject: [PATCH 1/4] fix(core): the import dry run previews create rows in the commit's batches The dry run asked the engine about each create row alone, so a child rule reading a parent roll-up (#25579) answered valid for a row the commit refuses. Create rows are now buffered like the commit's and previewed through one multi-row validate() per commit batch, in file order; update rows keep their per-row preview. The engine's multi-row preview now admits an insert row after the write's reference check, as insert() does. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5krDpBpMjEHKqjJ6u8 --- .../utils/import-runner-dry-run-batch.test.ts | 149 ++++++++++ packages/core/src/utils/import-runner.ts | 163 +++++++++-- packages/objectql/src/engine.ts | 26 +- ...mport-dry-run-batch-parent-summary.test.ts | 273 ++++++++++++++++++ 4 files changed, 581 insertions(+), 30 deletions(-) create mode 100644 packages/core/src/utils/import-runner-dry-run-batch.test.ts create mode 100644 packages/rest/src/import-dry-run-batch-parent-summary.test.ts diff --git a/packages/core/src/utils/import-runner-dry-run-batch.test.ts b/packages/core/src/utils/import-runner-dry-run-batch.test.ts new file mode 100644 index 00000000000..85f1cc776d2 --- /dev/null +++ b/packages/core/src/utils/import-runner-dry-run-batch.test.ts @@ -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>[0]; + +const OBJECT = 'contact'; + +const baseOpts = { + objectName: OBJECT, + metaMap: new Map([['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>; + 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).name) : (data as Record).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(); + }); +}); diff --git a/packages/core/src/utils/import-runner.ts b/packages/core/src/utils/import-runner.ts index 4983d0e39a2..33062dc70fd 100644 --- a/packages/core/src/utils/import-runner.ts +++ b/packages/core/src/utils/import-runner.ts @@ -960,11 +960,11 @@ export function runImport(opts: RunImportOptions): Promise { * with no validation at all, which is the false all-clear this card exists * to close. */ - const previewVerdict = async ( - data: Record, + const previewVerdicts = async ( + data: Record | Array>, mode: 'insert' | 'update', rowCtx: typeof writeCtx, - ): Promise[number] | null> => { + ): Promise | null> => { if (typeof p.validateData !== 'function') return null; const res = await p.validateData({ object: objectName, data, mode, @@ -975,7 +975,96 @@ export function runImport(opts: RunImportOptions): Promise { context: rowCtx, ...(environmentId ? { environmentId } : {}), }); - return res?.results?.[0] ?? null; + return res?.results ?? []; + }; + const previewVerdict = async ( + data: Record, + mode: 'insert' | 'update', + rowCtx: typeof writeCtx, + ): Promise => { + const verdicts = await previewVerdicts(data, mode, rowCtx); + return verdicts === null ? null : (verdicts[0] ?? null); + }; + + /** + * [#26684] The dry run's verdicts for a run of CREATE rows, asked of the + * engine as ONE multi-row insert preview, in file order — the batch the + * commit's flush hands `insertManyData`. The engine's multi-row `validate()` + * judges each row with the earlier rows of the same call that its rules + * admitted (#25579: a child rule reading a parent roll-up counts them), so + * the dry run refuses the rows the commit refuses. ⛔ Nothing here re-reads + * or re-judges a rule: the accumulation is the engine's, as on the write. + * + * One context for the run, the union of its rows' kept option values, as + * the commit's batch call carries (`ctxForRows`). + * + * A call that throws, or answers a verdict count other than one per row, is + * degraded as the commit's flush degrades a failed batch: each row is asked + * alone, so a row the engine refuses outright (an undeclared field) carries + * its own refusal, as `insertManyData` culls it per row; the rows that + * survive alone are then asked again together, so they still see each + * other. + */ + type PreviewVerdict = NonNullable[number]; + type PreviewOutcome = { verdict: PreviewVerdict | null } | { thrown: unknown }; + const previewCreateRun = async (run: ReadonlyArray): Promise => { + const ask = async (part: ReadonlyArray): Promise => { + const verdicts = await previewVerdicts(part.map((b) => b.data), 'insert', writeCtxFor(part.map((b) => b.kept))); + if (verdicts === null) return part.map(() => ({ verdict: null })); + if (verdicts.length !== part.length) { + throw Object.assign( + new Error(`validateData returned ${verdicts.length} verdict(s) for ${part.length} row(s)`), + { code: 'ERR_BULK_RESULT_MISMATCH' }, + ); + } + return verdicts.map((verdict) => ({ verdict })); + }; + try { + return await ask(run); + } catch (batchError) { + if (run.length === 1) return [{ thrown: batchError }]; + const alone: PreviewOutcome[] = []; + for (const b of run) { + try { alone.push((await ask([b]))[0]!); } catch (e) { alone.push({ thrown: e }); } + } + const survivors = run.filter((_, k) => !('thrown' in alone[k]!)); + if (survivors.length < 2 || survivors.length === run.length) return alone; + const together = await ask(survivors).catch(() => null); + if (!together) return alone; + let s = 0; + return alone.map((o) => ('thrown' in o ? o : together[s++]!)); + } + }; + + /** + * One dry-run row's report from its engine verdict (#4633). `existing` is + * the matched record of an update-resolved row; a create-resolved row has + * none. `null` (no validate-only operation) reports the row on the coercion + * + resolution verdict the caller already has. + */ + const reportPreview = ( + i: number, rowNo: number, verdict: PreviewVerdict | null, existing?: Record, + ): void => { + if (verdict && !verdict.valid) { + errCount++; + const first = verdict.errors[0]; + results[i] = { + row: rowNo, ok: false, action: 'failed', + ...(first?.field ? { field: first.field } : {}), + code: first?.code ?? 'VALIDATION_FAILED', + error: composeValidationMessage(verdict.errors), + }; + return; + } + okCount++; + // A warn-first deployment ADMITS some findings; the row is ok + // because the write would store it, and the complaint rides + // along so "accepted for now" is visible rather than silent. + const admitted = verdict?.warnings?.length ? { warnings: verdict.warnings } : {}; + // [#20701] What the write would strip from this row, from the same verdict. + const drops = rowDrops(verdict?.droppedFields); + if (existing) { updated++; results[i] = { row: rowNo, ok: true, action: 'updated', id: String(existing.id ?? '') || undefined, ...admitted, ...drops }; } + else { created++; results[i] = { row: rowNo, ok: true, action: 'created', ...admitted, ...drops }; } }; /** @@ -1011,7 +1100,11 @@ export function runImport(opts: RunImportOptions): Promise { // batch call, so a bad row never forces the whole-batch degradation that // re-runs beforeInsert hooks on its siblings. const canPartialCreate = typeof p.insertManyData === 'function'; - const pendingCreates: Array<{ index: number; rowNo: number; data: Record; kept: Record }> = []; + type PendingCreate = { index: number; rowNo: number; data: Record; kept: Record }; + const pendingCreates: PendingCreate[] = []; + // [#26684] A dry run's CREATE rows, buffered as the commit buffers them and + // previewed at the same checkpoints in the same runs (`flushPendingPreviews`). + const pendingPreviews: PendingCreate[] = []; // bulkWrite is at-least-once: a retry (or a mismatch-driven degradation) may // re-run a create whose prior attempt already committed. Every buffered // CREATE row is therefore pre-assigned a client-generated id at flush time @@ -1219,6 +1312,33 @@ export function runImport(opts: RunImportOptions): Promise { } }; + /** + * [#26684] The dry run's counterpart of {@link flushPendingCreates}: the + * buffered CREATE rows previewed in the commit's runs — the same checkpoints + * (every `progressEvery` rows, and the end) and the same run length + * (`Math.min(progressEvery, MAX_CREATE_BATCH_SIZE)`, the commit's + * `bulkWrite` batch size) — so each run is the batch the commit writes. + * Each row still gets its own verdict, reported as before. + */ + const flushPendingPreviews = async (): Promise => { + if (pendingPreviews.length === 0) return; + const batch = pendingPreviews.splice(0, pendingPreviews.length); + const runLength = Math.min(progressEvery, MAX_CREATE_BATCH_SIZE); + for (let start = 0; start < batch.length; start += runLength) { + const run = batch.slice(start, start + runLength); + const outcomes = await previewCreateRun(run); + run.forEach((b, k) => { + const outcome = outcomes[k]!; + if ('thrown' in outcome) { + errCount++; + results[b.index] = toFailedResult(b.rowNo, outcome.thrown, objectName); + } else { + reportPreview(b.index, b.rowNo, outcome.verdict); + } + }); + } + }; + return (async () => { const sharingKey = await rowsSharingKey(); for (let i = 0; i < rows.length; i++) { @@ -1293,28 +1413,17 @@ export function runImport(opts: RunImportOptions): Promise { // patch, as the by-id update does. Nothing new crosses the // protocol: the id is the address every update door already // carries in the payload. - const verdict = willUpdate - ? await previewVerdict({ ...updatePatch(data, matchedKeySet), id: (existing as Record).id }, 'update', rowCtx) - : await previewVerdict(data, 'insert', rowCtx); - if (verdict && !verdict.valid) { - errCount++; - const first = verdict.errors[0]; - results[i] = { - row: rowNo, ok: false, action: 'failed', - ...(first?.field ? { field: first.field } : {}), - code: first?.code ?? 'VALIDATION_FAILED', - error: composeValidationMessage(verdict.errors), - }; + // + // [#26684] A create is not asked alone: it is buffered like the + // commit's create and previewed with the rows of its batch + // (`flushPendingPreviews`), so a rule that counts the earlier + // rows of the batch judges it as the commit's write does. An + // update is asked alone, as it is written alone. + if (willUpdate) { + const target = existing as Record; + reportPreview(i, rowNo, await previewVerdict({ ...updatePatch(data, matchedKeySet), id: target.id }, 'update', rowCtx), target); } else { - okCount++; - // A warn-first deployment ADMITS some findings; the row is ok - // because the write would store it, and the complaint rides - // along so "accepted for now" is visible rather than silent. - const admitted = verdict?.warnings?.length ? { warnings: verdict.warnings } : {}; - // [#20701] What the write would strip from this row, from the same verdict. - const drops = rowDrops(verdict?.droppedFields); - if (willUpdate) { updated++; results[i] = { row: rowNo, ok: true, action: 'updated', id: String((existing as any).id ?? '') || undefined, ...admitted, ...drops }; } - else { created++; results[i] = { row: rowNo, ok: true, action: 'created', ...admitted, ...drops }; } + pendingPreviews.push({ index: i, rowNo, data, kept: keptOptionValues }); } } else if (willUpdate) { const target = existing as Record; @@ -1366,6 +1475,7 @@ export function runImport(opts: RunImportOptions): Promise { if (processed % progressEvery === 0 || processed === rows.length) { // Flush before reporting/cancelling so counts and `processed` reflect // every row up to this checkpoint, not just decided-but-unwritten ones. + await flushPendingPreviews(); await flushPendingCreates(); if (onProgress) await onProgress(snapshot(processed)); } @@ -1377,6 +1487,7 @@ export function runImport(opts: RunImportOptions): Promise { } } + await flushPendingPreviews(); await flushPendingCreates(); const compacted = results.filter((r): r is ImportRowResult => r !== undefined); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 1f7cd1571e1..c458fd31052 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -14122,8 +14122,10 @@ export class ObjectQL implements IObjectQLEngine { * and is refused by the write. * * Nothing is written and no sequence is consumed. Reads do happen: the - * stored row above, the master-detail headers, and the related rows a - * traversing rule names (see the `previewRelatedForRow` block below). + * stored row above, the master-detail headers, the related rows a + * traversing rule names (see the `previewRelatedForRow` block below), and + * [#26684] on an insert, the write's reference check on each id the caller + * supplied, behind the same write-gate probe as the related rows. * Validation itself is in-process, which is what makes row-by-row dry run * of a large import affordable. */ @@ -14528,7 +14530,10 @@ export class ObjectQL implements IObjectQLEngine { ? createBatchSummaryAccumulator(this.getSummaryDescriptors(object), (schemaForValidation as any)?.fields) : undefined; - const results: NonNullable = rows.map((row, i) => { + // [#26684] Judged one row after another, as the write's validation loop + // judges them: a row's admission (the reference check below) is awaited + // before the next row reads what the earlier rows added. + const judgeRow = async (row: Record, i: number): Promise[number]> => { const warnings: ValidateDataIssue[] = []; // Warn-first admissions are the posture signal the caller came for, so // they are reported — into this row's own bucket, never into the @@ -14584,6 +14589,17 @@ export class ObjectQL implements IObjectQLEngine { related: batchSummaries ? batchSummaries.overlay(previewRelatedForRow(judgedViews[i])) : previewRelatedForRow(judgedViews[i]), permissions: previewPermissionsFor(row), })); + // [#26684] The insert's admit step, run as `insert()` runs it: after + // the rules, the reference check on the ids the caller supplied + // (`rawRows`, before the defaults — the write's `suppliedPerRow`), so + // a row the write refuses on a dangling reference is refused here + // with the same finding, and is not counted toward a later row's + // parent summary. ⛔ Behind the same write-gate probe as the related + // read above: the probe on a `lookup` target is elevated, so a caller + // the probe refuses learns nothing about which ids exist. + if (mode === 'insert' && mayWrite) { + await this.assertReferencesResolve(schemaForValidation, row, rawRows[i], options?.context, messages); + } } catch (e) { if (e instanceof ValidationError) { return { valid: false, errors: e.fields.map((f) => ({ ...f })), warnings }; @@ -14600,7 +14616,9 @@ export class ObjectQL implements IObjectQLEngine { // when nothing would be taken. const droppedFields = droppedFieldEvents(object, computedStrip.droppedPerRow[i]!, readonlyDroppedPerRow[i]!); return { valid: true, errors: [], warnings, ...(droppedFields.length > 0 ? { droppedFields } : {}) }; - }); + }; + const results: NonNullable = []; + for (let i = 0; i < rows.length; i++) results.push(await judgeRow(rows[i]!, i)); return { object, diff --git a/packages/rest/src/import-dry-run-batch-parent-summary.test.ts b/packages/rest/src/import-dry-run-batch-parent-summary.test.ts new file mode 100644 index 00000000000..a71ea97cddd --- /dev/null +++ b/packages/rest/src/import-dry-run-batch-parent-summary.test.ts @@ -0,0 +1,273 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#26684] The import DRY RUN answers what the import COMMIT answers on the + * same file, now that a bulk create counts the earlier rows of its batch when + * a child rule reads a parent `summary` (#25579, ruling 6377847361 letter C: + * 「批量等于逐条」). + * + * #25579's reproduction through the REAL import door + * (`POST /api/v1/data/:object/import`), a REAL ObjectQL engine and a REAL + * sqlite `:memory:` driver: a supplier carries `primary_contact_count`, a + * `count` roll-up of its contacts filtered on `is_primary: true`, and the + * contact's `script` rule refuses a primary contact when that count is already + * above zero. The commit creates one of two primary rows and refuses the + * other. The dry run used to ask the engine about each row ALONE, so both rows + * read the stored `0` and both answered valid. Now the dry run previews the + * file's create rows in the commit's own batches through the engine's + * multi-row `validate()`, and each row keeps its own verdict. + * + * Every acceptance case runs the SAME file twice, on two fresh stacks — once + * with `dryRun: true`, once committed — and pins that the per-row verdicts + * agree (row, ok, action, error). + * + * The seat's addendum (6442401477): the preview's admit step now runs the + * write's reference check, so a row the write refuses on a dangling reference + * is refused by the preview too, and is not counted toward a later row. + * + * ## Reverse verification — predicted before running + * + * Ask the engine about each create row alone again (the run length set to 1): + * the two-primary agreement case and the second-batch case go red, each + * dry-running both primary rows valid; the non-primary, update-mode and + * refused-row cases stay green. Drop the engine's reference check from the + * preview: the two dangling-reference cases go red. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { SysMetadataAuditObject, SysMetadataCommitObject, SysMetadataHistoryObject, SysMetadataObject } from '@objectstack/metadata-core'; +import { RestServer } from './rest-server'; + +const liveEngines: ObjectQL[] = []; +afterEach(async () => { + while (liveEngines.length) { + try { await liveEngines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +const REGION = { + name: 'xt_region', label: 'Region', systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + name: { name: 'name', type: 'text' as const, label: 'Name' }, + }, +}; + +const SUPPLIER = { + name: 'xt_supplier', label: 'Supplier', systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + name: { name: 'name', type: 'text' as const, label: 'Name' }, + primary_contact_count: { + name: 'primary_contact_count', type: 'summary' as const, label: 'Primary Contacts', + summaryOperations: { object: 'xt_supplier_contact', field: 'is_primary', function: 'count' as const, filter: { is_primary: true } }, + }, + }, +}; + +const CONTACT = { + name: 'xt_supplier_contact', label: 'Supplier Contact', systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + name: { name: 'name', type: 'text' as const, label: 'Name' }, + is_primary: { name: 'is_primary', type: 'boolean' as const, label: 'Primary' }, + supplier: { name: 'supplier', type: 'lookup' as const, label: 'Supplier', reference: 'xt_supplier' }, + // A reference no rule traverses: only the write's reference check reads it. + region: { name: 'region', type: 'lookup' as const, label: 'Region', reference: 'xt_region' }, + }, + validations: [ + { + name: 'one_primary_contact', type: 'script', severity: 'error', + message: 'A supplier has at most one primary contact.', + condition: 'record.is_primary == true && record.supplier.primary_contact_count > 0', + }, + { + // An unrelated refusal, so a row the rules refuse can sit in the batch. + name: 'no_placeholder_name', type: 'script', severity: 'error', + message: 'Placeholder names are not allowed.', + condition: "record.name == 'placeholder'", + }, + ], +}; + +function createMockServer() { + const noop = () => {}; + return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; +} + +function makeRes() { + const res: any = { + write: () => true, end: () => {}, + header: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: any) => { res._json = body; return res; }, + }; + return res; +} + +async function boot() { + const engine = new ObjectQL(); + liveEngines.push(engine); + engine.registerDriver(new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), true); + await engine.init(); + for (const o of [REGION, SUPPLIER, CONTACT]) engine.registry.registerObject(o as any); + await engine.syncSchemas(); + for (const o of [SysMetadataObject, SysMetadataHistoryObject, SysMetadataAuditObject, SysMetadataCommitObject]) { + if (!engine.registry.getObject(o.name)) engine.registry.registerObject(o as any); + } + const protocol = new ObjectStackProtocolImplementation(engine as any); + const rest = new RestServer(createMockServer() as any, protocol as any, { api: { requireAuth: false } } as any); + (rest as any).resolveExecCtx = async () => ({ userId: 'test-user' }); + rest.registerRoutes(); + const route = rest.getRoutes().find((r: any) => r.method === 'POST' && r.path === '/api/v1/data/:object/import'); + expect(route).toBeDefined(); + const importContacts = async (rows: Array>, extra: Record = {}) => { + const res = makeRes(); + await route!.handler({ params: { object: 'xt_supplier_contact' }, body: { format: 'json', rows, ...extra } } as any, res); + expect(res._status ?? 200, JSON.stringify(res._json)).toBe(200); + return res._json; + }; + // A NEW supplier: the insert seeds its count at 0. + await engine.insert('xt_supplier', { id: 's1', name: 'Acme' }); + await engine.insert('xt_region', { id: 'r1', name: 'North' }); + const contacts = async () => (await engine.find('xt_supplier_contact', { where: { supplier: 's1' } })) as Array>; + return { engine, importContacts, contacts }; +} + +/** A row's verdict, in the words both halves of the import report it. */ +const verdicts = (summary: { results: Array> }) => + summary.results.map((r) => ({ row: r.row, ok: r.ok, action: r.action, error: r.error })); + +/** + * The file dry-run on one fresh stack and committed on another; returns both + * reports after pinning that they agree row by row. + */ +async function dryRunAndCommit(rows: Array>, extra: Record = {}) { + const dry = await (await boot()).importContacts(rows, { ...extra, dryRun: true }); + const committed = await boot(); + const commit = await committed.importContacts(rows, extra); + expect(verdicts(dry)).toEqual(verdicts(commit)); + expect({ created: dry.created, updated: dry.updated, errors: dry.errors }) + .toEqual({ created: commit.created, updated: commit.updated, errors: commit.errors }); + return { dry, commit, committed }; +} + +describe('[#26684] the dry run agrees with the commit on a batch-aware parent-summary rule', () => { + it('two primary rows for a new supplier: the dry run refuses the second with the commit\'s message', async () => { + const { dry, committed } = await dryRunAndCommit([ + { id: 'k1', name: 'Zhang', supplier: 's1', is_primary: true }, + { id: 'k2', name: 'Li', supplier: 's1', is_primary: true }, + ]); + expect(dry).toMatchObject({ dryRun: true, created: 1, errors: 1 }); + expect(dry.results[0]).toMatchObject({ row: 1, ok: true, action: 'created' }); + expect(dry.results[1]).toMatchObject({ row: 2, ok: false, action: 'failed' }); + expect(dry.results[1].error).toContain('A supplier has at most one primary contact.'); + expect((await committed.contacts()).map((c) => c.id)).toEqual(['k1']); + }); + + it('the dry run writes nothing', async () => { + const { importContacts, contacts, engine } = await boot(); + await importContacts([ + { id: 'k1', name: 'Zhang', supplier: 's1', is_primary: true }, + { id: 'k2', name: 'Li', supplier: 's1', is_primary: true }, + ], { dryRun: true }); + expect(await contacts()).toEqual([]); + expect((await engine.findOne('xt_supplier', { where: { id: 's1' } }))?.primary_contact_count).toBe(0); + }); + + it('two NON-primary rows both answer valid in the dry run, as both land', async () => { + const { dry } = await dryRunAndCommit([ + { id: 'k1', name: 'Zhang', supplier: 's1', is_primary: false }, + { id: 'k2', name: 'Li', supplier: 's1', is_primary: false }, + ]); + expect(dry).toMatchObject({ created: 2, errors: 0 }); + }); + + it('a row the rules refuse does not raise the count a later row reads, in the dry run as in the commit', async () => { + const { dry } = await dryRunAndCommit([ + { id: 'k1', name: 'placeholder', supplier: 's1', is_primary: true }, + { id: 'k2', name: 'Li', supplier: 's1', is_primary: true }, + ]); + expect(dry.results[0].error).toContain('Placeholder names are not allowed.'); + expect(dry.results[1]).toMatchObject({ ok: true, action: 'created' }); + }); + + it('a file larger than one batch: two primary rows in the second batch agree with the commit', async () => { + // 200 rows is the commit's batch (`Math.min(progressEvery, MAX_CREATE_BATCH_SIZE)`). + const rows: Array> = []; + for (let n = 1; n <= 200; n++) rows.push({ id: `a${n}`, name: `Filler ${n}`, supplier: 's1', is_primary: false }); + rows.push({ id: 'k1', name: 'Zhang', supplier: 's1', is_primary: true }); + rows.push({ id: 'k2', name: 'Li', supplier: 's1', is_primary: true }); + rows.push({ id: 'a201', name: 'Filler 201', supplier: 's1', is_primary: false }); + const { dry } = await dryRunAndCommit(rows); + expect(dry).toMatchObject({ created: 202, errors: 1 }); + expect(dry.results[201]).toMatchObject({ row: 202, ok: false, action: 'failed' }); + expect(dry.results[201].error).toContain('A supplier has at most one primary contact.'); + }); +}); + +describe('[#26684] an update-mode dry run keeps its per-row preview', () => { + it('an update against a stored primary is refused, a rename is not, as on the commit', async () => { + const seed = async (b: Awaited>) => { + await b.engine.insert('xt_supplier_contact', { id: 'k1', name: 'Zhang', supplier: 's1', is_primary: true }); + await b.engine.insert('xt_supplier_contact', { id: 'k2', name: 'Li', supplier: 's1', is_primary: false }); + await b.engine.insert('xt_supplier_contact', { id: 'k3', name: 'Wang', supplier: 's1', is_primary: false }); + }; + const rows = [ + { name: 'Li', is_primary: true }, + { name: 'Wang', supplier: 's1' }, + ]; + const extra = { writeMode: 'update', matchFields: ['name'] }; + const dryStack = await boot(); + await seed(dryStack); + const dry = await dryStack.importContacts(rows, { ...extra, dryRun: true }); + const commitStack = await boot(); + await seed(commitStack); + const commit = await commitStack.importContacts(rows, extra); + expect(verdicts(dry)).toEqual(verdicts(commit)); + expect(dry.results[0]).toMatchObject({ ok: false, action: 'failed' }); + expect(dry.results[0].error).toContain('A supplier has at most one primary contact.'); + expect(dry.results[1]).toMatchObject({ ok: true, action: 'updated' }); + }); +}); + +describe('[#26684] the preview admits a row as the write admits it: after the reference check', () => { + const ROWS = [ + // Refused by the write's reference check: no xt_region row `nowhere`. + { id: 'k1', name: 'Zhang', supplier: 's1', region: 'nowhere', is_primary: true }, + { id: 'k2', name: 'Li', supplier: 's1', region: 'r1', is_primary: true }, + ]; + + it('a multi-row preview refuses the dangling row and does not count it, as `insertMany` does', async () => { + const write = await boot(); + const outcomes = await write.engine.insertMany('xt_supplier_contact', ROWS.map((r) => ({ ...r }))); + expect(outcomes.map((o) => o.ok)).toEqual([false, true]); + + const { engine } = await boot(); + const preview = await engine.validate('xt_supplier_contact', ROWS.map((r) => ({ ...r })), { mode: 'insert' }); + expect(preview.results!.map((r) => r.valid)).toEqual([false, true]); + expect(preview.results![0]!.errors.map((e) => [e.field, e.code])).toEqual([['region', 'reference_not_found']]); + const refused = outcomes[0]!; + if (refused.ok) throw new Error('expected the first row to be refused'); + expect((refused.error as { fields?: Array<{ field: string; code: string }> }).fields?.map((f) => [f.field, f.code])) + .toEqual([['region', 'reference_not_found']]); + }); + + it('a single-row preview refuses a dangling reference, as the single insert does', async () => { + const { engine } = await boot(); + const preview = await engine.validate('xt_supplier_contact', { ...ROWS[0] }, { mode: 'insert' }); + expect(preview.results![0]).toMatchObject({ valid: false }); + expect(preview.results![0]!.errors.map((e) => [e.field, e.code])).toEqual([['region', 'reference_not_found']]); + await expect(engine.insert('xt_supplier_contact', { ...ROWS[0] })).rejects.toMatchObject({ code: 'VALIDATION_FAILED' }); + }); + + it('a caller the write-gate probe refuses is told nothing about which ids exist', async () => { + const { engine } = await boot(); + engine.registerWriteGateProbe(async () => false); + const preview = await engine.validate('xt_supplier_contact', { ...ROWS[0], is_primary: false }, { mode: 'insert' }); + expect(preview.results![0]!.errors.map((e) => e.code)).not.toContain('reference_not_found'); + }); +}); From 20ad5de9a6c7346c2c2b8ef10bfef64505486d68 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 14:30:23 +0000 Subject: [PATCH 2/4] docs(data-modeling): the import dry run previews create rows in the import's batches Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5krDpBpMjEHKqjJ6u8 --- content/docs/data-modeling/fields.mdx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/content/docs/data-modeling/fields.mdx b/content/docs/data-modeling/fields.mdx index ccbfdf3ebc6..74a02ef2bd1 100644 --- a/content/docs/data-modeling/fields.mdx +++ b/content/docs/data-modeling/fields.mdx @@ -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. From 1b2324f64ed00ad3fb8d109eff591b08caee2828 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 14:34:20 +0000 Subject: [PATCH 3/4] chore(changeset): the import dry run previews create rows in the commit's batches Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5krDpBpMjEHKqjJ6u8 --- .../26684-import-dry-run-batch-preview.md | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 .changeset/26684-import-dry-run-batch-preview.md diff --git a/.changeset/26684-import-dry-run-batch-preview.md b/.changeset/26684-import-dry-run-batch-preview.md new file mode 100644 index 00000000000..6952344d0ce --- /dev/null +++ b/.changeset/26684-import-dry-run-batch-preview.md @@ -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 + + + +**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. From 80b0a99e12f72ea46d99072cfd3820ee2d04b03b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 14:42:56 +0000 Subject: [PATCH 4/4] test: the dry-run doubles answer one verdict per submitted row, and the header-read control counts the write's reference check Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5krDpBpMjEHKqjJ6u8 --- .../import-runner-dropped-fields.test.ts | 16 ++++++++----- .../utils/import-runner-kept-options.test.ts | 23 +++++++++++++------ ...port-runner-update-preview-address.test.ts | 3 ++- .../src/validate-preview-parent.test.ts | 14 +++++++++-- 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/packages/core/src/utils/import-runner-dropped-fields.test.ts b/packages/core/src/utils/import-runner-dropped-fields.test.ts index 905cc5fe01c..78aa1b9a375 100644 --- a/packages/core/src/utils/import-runner-dropped-fields.test.ts +++ b/packages/core/src/utils/import-runner-dropped-fields.test.ts @@ -71,6 +71,10 @@ const withDrops = (row: Record) => { 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>; + /** The row's key set — a clean row must not grow an empty `droppedFields`. */ const keysOf = (r: ImportRowResult | undefined) => Object.keys(r ?? {}).sort(); @@ -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) }], + 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 }; @@ -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).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 }, }; }); diff --git a/packages/core/src/utils/import-runner-kept-options.test.ts b/packages/core/src/utils/import-runner-kept-options.test.ts index 80e5df0b19a..c9b2be77318 100644 --- a/packages/core/src/utils/import-runner-kept-options.test.ts +++ b/packages/core/src/utils/import-runner-kept-options.test.ts @@ -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], context: args.context }); - return { results: [{ valid: true, errors: [] }] } as any; + const rows = (Array.isArray(args.data) ? args.data : [args.data]) as Array>; + calls.push({ kind: 'validateData', rows, context: args.context }); + return { results: rows.map(() => ({ valid: true, errors: [] })) } as any; }); } return { p, calls }; @@ -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 () => { diff --git a/packages/core/src/utils/import-runner-update-preview-address.test.ts b/packages/core/src/utils/import-runner-update-preview-address.test.ts index 7720992745e..3150b16608e 100644 --- a/packages/core/src/utils/import-runner-update-preview-address.test.ts +++ b/packages/core/src/utils/import-runner-update-preview-address.test.ts @@ -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' }]); }); }); diff --git a/packages/objectql/src/validate-preview-parent.test.ts b/packages/objectql/src/validate-preview-parent.test.ts index 7d8a410093a..20b011703c0 100644 --- a/packages/objectql/src/validate-preview-parent.test.ts +++ b/packages/objectql/src/validate-preview-parent.test.ts @@ -261,14 +261,24 @@ describe('#22474 — the preview binds the master-detail header the write binds' return count; } - it('insert and update previews of a plain line issue no read of the header object', async () => { + it('an update preview of a plain line issues no read of the header object', async () => { storeFor(PLAIN_LINE).set('p1', { id: 'p1', invoice: 'h_sent', quantity: 1 }); const count = countHeaderReads(engine); - expect(await preview(engine, PLAIN_LINE, { invoice: 'h_sent', quantity: 1 }, 'insert')).toEqual(ADMITTED); expect(await preview(engine, PLAIN_LINE, { quantity: 2, id: 'p1' }, 'update')).toEqual(ADMITTED); expect(count.reads).toBe(0); }); + it('an insert preview of a plain line reads the header only as the insert does', async () => { + // [#26684] The insert preview runs the write's reference check, which + // asks the header's read door whether `h_sent` exists: the read the + // insert itself makes. The header binding adds none. + const count = countHeaderReads(engine); + expect(await preview(engine, PLAIN_LINE, { invoice: 'h_sent', quantity: 1 }, 'insert')).toEqual(ADMITTED); + const previewReads = count.reads; + expect(await write(() => engine.insert(PLAIN_LINE, { invoice: 'h_sent', quantity: 1 }))).toEqual(ADMITTED); + expect(count.reads - previewReads).toBe(previewReads); + }); + it('positive control: the parent-scoped line does read it', async () => { const count = countHeaderReads(engine); await preview(engine, LINE, { invoice: 'h_draft', quantity: 1 }, 'insert');