diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ad1225..eec6929 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## 1.5.1 + +### Fixes + +- **Undo of a JSON cell edit no longer clobbers concurrent changes to other keys.** When a JSON cell was edited as a merge patch, undo previously restored the whole captured document, which could overwrite a concurrent change to a different key of the same cell. Undo now applies an inverse merge patch that touches only the keys the original edit changed (web and desktop): a key the edit added is removed, a key it changed is restored to its prior value. Non-JSON edits and edits whose prior value was not a JSON object keep the previous full-value undo. +- **JSON merge patches treat arrays as atomic values** (RFC 7396), aligning the JavaScript merge-patch fallback with SQLite's native `json_patch()` semantics. + ## 1.5.0 ### Features diff --git a/package.json b/package.json index 2723ab7..a0a01b4 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "name": "sqlite-explorer", "displayName": "SQLite Explorer", "description": "A powerful SQLite database viewer and editor for VS Code", - "version": "1.5.0", + "version": "1.5.1", "publisher": "zknpr", "license": "MIT", "repository": { diff --git a/src/core/engine/wasm/WasmDatabaseEngine.ts b/src/core/engine/wasm/WasmDatabaseEngine.ts index a485a8e..f0d6961 100644 --- a/src/core/engine/wasm/WasmDatabaseEngine.ts +++ b/src/core/engine/wasm/WasmDatabaseEngine.ts @@ -24,7 +24,7 @@ import type { } from '../../types'; import { escapeIdentifier, validateSqlType, validateRowId, validateRowIds } from '../../sql-utils'; import { buildSelectQuery, buildCountQuery } from '../../query-builder'; -import { applyMergePatch } from '../../json-utils'; +import { applyMergePatch, tryCreateInverseMergePatch } from '../../json-utils'; import { getNodeFs } from '../../platform/fs'; // ============================================================================ @@ -282,21 +282,58 @@ export class WasmDatabaseEngine implements DatabaseOperations { } private async undoCellUpdate(targetTable: string, mod: ModificationEntry): Promise { - const { affectedCells, targetRowId, targetColumn, priorValue } = mod; + const { affectedCells, targetRowId, targetColumn, priorValue, newValue, operation } = mod; if (affectedCells) { // Batch undo - const updates = affectedCells.map(cell => ({ - rowId: cell.rowId, - column: cell.columnName, - value: cell.priorValue ?? null - })); - await this.updateCellBatch(targetTable, updates); + const updates = affectedCells.map(cell => + this.createUndoCellUpdate(cell.rowId, cell.columnName, cell.priorValue, cell.newValue, cell.operation) + ); + if (updates.some(update => update.operation === 'json_patch')) { + for (const update of updates) { + await this.applyUndoCellUpdate(targetTable, update); + } + } else { + await this.updateCellBatch(targetTable, updates); + } } else if (targetRowId !== undefined && targetColumn) { // Single cell undo - await this.updateCell(targetTable, targetRowId, targetColumn, priorValue ?? null); + const update = this.createUndoCellUpdate(targetRowId, targetColumn, priorValue, newValue, operation); + await this.applyUndoCellUpdate(targetTable, update); + } + } + + private async applyUndoCellUpdate(targetTable: string, update: CellUpdate): Promise { + if (update.operation === 'json_patch') { + try { + await this.updateCell(targetTable, update.rowId, update.column, null, update.value as string); + } catch { + // The current cell can become non-JSON after the tracked edit; the recorded prior value is the safe full-cell fallback. + await this.updateCell(targetTable, update.rowId, update.column, update.originalValue ?? null); + } + } else { + await this.updateCell(targetTable, update.rowId, update.column, update.value); } } + private createUndoCellUpdate( + rowId: RecordId, + column: string, + priorValue: CellValue | undefined, + newValue: CellValue | undefined, + operation: ModificationEntry['operation'] + ): CellUpdate { + if (operation === 'json_patch') { + const inversePatch = tryCreateInverseMergePatch(newValue, priorValue); + if (inversePatch !== undefined) { + // The inverse merge patch touches only keys from the original forward patch. + return { rowId, column, value: inversePatch, originalValue: priorValue ?? null, operation: 'json_patch' }; + } + } + + // Non-patch edits and unparseable/non-object priors retain legacy value replacement undo. + return { rowId, column, value: priorValue ?? null, originalValue: priorValue ?? null, operation: 'set' }; + } + private async undoRowInsert(targetTable: string, mod: ModificationEntry): Promise { const { targetRowId } = mod; // Undo insert = delete row diff --git a/src/core/json-utils.ts b/src/core/json-utils.ts index b386c34..9230694 100644 --- a/src/core/json-utils.ts +++ b/src/core/json-utils.ts @@ -9,6 +9,12 @@ const MAX_DEPTH = 1000; +class UnfaithfulInverseMergePatchError extends Error { + constructor() { + super('JSON inverse merge patch would not faithfully restore the prior value'); + } +} + export function generateMergePatch(original: unknown, modified: unknown, depth = 0): unknown { if (depth > MAX_DEPTH) { throw new Error('JSON merge patch depth limit exceeded'); @@ -108,6 +114,104 @@ export function applyMergePatch(target: unknown, patch: unknown, depth = 0): unk return targetObj; } +/** + * Build a JSON Merge Patch that reverses the keys touched by a forward patch. + * + * The inverse is intentionally restricted to the forward patch key structure so + * undo can restore the edited keys without replacing concurrent sibling edits. + */ +export function invertMergePatch(forwardPatch: unknown, prior: unknown): unknown { + return invertMergePatchAtDepth(forwardPatch, prior, 0); +} + +/** + * Convert recorded cell values into a serialized inverse merge patch. + * + * Undo uses this only when both recorded values are JSON strings and the prior + * document is an object, matching SQLite json_patch object-merge semantics. + */ +export function tryCreateInverseMergePatch(forwardPatchValue: unknown, priorValue: unknown): string | undefined { + if (typeof forwardPatchValue !== 'string' || typeof priorValue !== 'string') { + return undefined; + } + + const forwardPatch = parseJsonValue(forwardPatchValue); + const prior = parseJsonValue(priorValue); + if (!forwardPatch.ok || !prior.ok || !isObject(forwardPatch.value) || !isObject(prior.value)) { + return undefined; + } + + try { + return JSON.stringify(invertMergePatch(forwardPatch.value, prior.value)); + } catch (err) { + if (err instanceof UnfaithfulInverseMergePatchError) { + return undefined; + } + throw err; + } +} + +function invertMergePatchAtDepth(forwardPatch: unknown, prior: unknown, depth: number): unknown { + if (depth > MAX_DEPTH) { + throw new Error('JSON invert merge patch depth limit exceeded'); + } + + if (!isObject(forwardPatch)) { + return prior === undefined ? null : prior; + } + + const inverse: Record = {}; + const priorObj = isObject(prior) ? prior : {}; + + for (const key of Object.keys(forwardPatch)) { + const priorHas = Object.prototype.hasOwnProperty.call(priorObj, key); + const priorVal = priorHas ? priorObj[key] : undefined; + const forwardVal = forwardPatch[key]; + + if (isObject(forwardVal)) { + if (priorHas && isObject(priorVal)) { + inverse[key] = invertMergePatchAtDepth(forwardVal, priorVal, depth + 1); + } else if (priorHas && priorVal === null) { + throw new UnfaithfulInverseMergePatchError(); + } else if (priorHas) { + // A forward object patch replaced a scalar or array prior, so the inverse must restore that whole value. + inverse[key] = priorVal; + } else { + // Recurse into added objects so undo deletes only the leaves touched by the forward patch. + inverse[key] = invertMergePatchAtDepth(forwardVal, {}, depth + 1); + } + } else if (forwardVal === null) { + inverse[key] = createInverseLeafValue(priorHas, priorVal); + } else { + inverse[key] = createInverseLeafValue(priorHas, priorVal); + } + } + + return inverse; +} + +function createInverseLeafValue(priorHas: boolean, priorVal: unknown): unknown { + if (!priorHas) { + // RFC 7396 null deletes a key, which is faithful when the forward patch added that key. + return null; + } + + if (priorVal === null) { + // RFC 7396 null would delete this existing key instead of restoring its explicit JSON null value. + throw new UnfaithfulInverseMergePatchError(); + } + + return priorVal; +} + +function parseJsonValue(value: string): { ok: true; value: unknown } | { ok: false } { + try { + return { ok: true, value: JSON.parse(value) }; + } catch { + return { ok: false }; + } +} + function isObject(val: unknown): val is Record { - return val !== null && typeof val === 'object'; + return val !== null && typeof val === 'object' && !Array.isArray(val); } diff --git a/src/nativeWorker.ts b/src/nativeWorker.ts index cedaa20..690797f 100644 --- a/src/nativeWorker.ts +++ b/src/nativeWorker.ts @@ -38,6 +38,7 @@ import type { } from './core/types'; import { escapeIdentifier, validateSqlType, validateRowId, validateRowIds } from './core/sql-utils'; import { buildSelectQuery, buildCountQuery } from './core/query-builder'; +import { tryCreateInverseMergePatch } from './core/json-utils'; // ============================================================================ // Utility Functions @@ -90,6 +91,27 @@ const INIT_TIMEOUT = 10000; /** Timeout for individual queries (ms) */ const QUERY_TIMEOUT = 30000; +// Keep this paired with WasmDatabaseEngine.undoCellUpdate so web and native +// undo interpret ModificationEntry cell_update fields the same way. +function createUndoCellUpdate( + rowId: RecordId, + column: string, + priorValue: CellValue | undefined, + newValue: CellValue | undefined, + operation: ModificationEntry['operation'] +): CellUpdate { + if (operation === 'json_patch') { + const inversePatch = tryCreateInverseMergePatch(newValue, priorValue); + if (inversePatch !== undefined) { + // The inverse merge patch restores only keys touched by the forward patch. + return { rowId, column, value: inversePatch, originalValue: priorValue ?? null, operation: 'json_patch' }; + } + } + + // Non-patch edits and unparseable/non-object priors retain legacy value replacement undo. + return { rowId, column, value: priorValue ?? null, originalValue: priorValue ?? null, operation: 'set' }; +} + // ============================================================================ // Platform Detection // ============================================================================ @@ -535,28 +557,39 @@ export async function createNativeDatabaseConnection( * Undo a modification by executing the inverse SQL. */ undoModification: async (mod: ModificationEntry) => { - const { modificationType, targetTable, targetRowId, targetColumn, priorValue, affectedCells, deletedRows, columnDef, deletedColumns } = mod; + const { modificationType, targetTable, targetRowId, targetColumn, priorValue, newValue, operation, affectedCells, deletedRows, columnDef, deletedColumns } = mod; if (!targetTable) return; + const applyUndoCellUpdate = async (update: CellUpdate): Promise => { + if (update.operation === 'json_patch') { + try { + await operationsFacade.updateCell(targetTable, update.rowId, update.column, null, update.value as string); + } catch { + // A json_patch undo can fail if another edit replaced the current cell with non-JSON text. + await operationsFacade.updateCell(targetTable, update.rowId, update.column, update.originalValue ?? null); + } + } else { + await operationsFacade.updateCell(targetTable, update.rowId, update.column, update.value); + } + }; + switch (modificationType) { case 'cell_update': if (affectedCells) { // Batch undo - const updates = affectedCells.map(c => ({ - rowId: c.rowId, - column: c.columnName, - value: c.priorValue - } as CellUpdate)); - await worker.call('execBatch', [ - updates.map(u => ({ - sql: `UPDATE ${escapeIdentifier(targetTable)} SET ${escapeIdentifier(u.column)} = ? WHERE rowid = ?`, - params: [u.value, Number(u.rowId)] - })) - ]); + const updates = affectedCells.map(c => + createUndoCellUpdate(c.rowId, c.columnName, c.priorValue, c.newValue, c.operation) + ); + if (updates.some(update => update.operation === 'json_patch')) { + for (const update of updates) { + await applyUndoCellUpdate(update); + } + } else { + await operationsFacade.updateCellBatch(targetTable, updates); + } } else if (targetRowId !== undefined && targetColumn) { - const rowIdNum = Number(targetRowId); - const sql = `UPDATE ${escapeIdentifier(targetTable)} SET ${escapeIdentifier(targetColumn)} = ? WHERE rowid = ?`; - await worker.call('run', [sql, [priorValue, rowIdNum]]); + const update = createUndoCellUpdate(targetRowId, targetColumn, priorValue, newValue, operation); + await applyUndoCellUpdate(update); } break; diff --git a/tests/unit/json_utils.test.ts b/tests/unit/json_utils.test.ts index 78d2d59..e4dfcbc 100644 --- a/tests/unit/json_utils.test.ts +++ b/tests/unit/json_utils.test.ts @@ -1,7 +1,7 @@ import { describe, it } from 'node:test'; import assert from 'node:assert'; -import { generateMergePatch, applyMergePatch } from '../../src/core/json-utils'; +import { generateMergePatch, applyMergePatch, invertMergePatch, tryCreateInverseMergePatch } from '../../src/core/json-utils'; describe('JSON Merge Patch (RFC 7396)', () => { describe('generateMergePatch', () => { @@ -103,4 +103,96 @@ describe('JSON Merge Patch (RFC 7396)', () => { assert.deepStrictEqual(result, { a: { x: 1, y: 2 } }); }); }); + + describe('invertMergePatch', () => { + it('should delete keys added by the forward patch', () => { + const prior = { a: 1 }; + const forward = { b: 2 }; + + // The inverse uses null for keys that did not exist in the prior document. + assert.deepStrictEqual(invertMergePatch(forward, prior), { b: null }); + }); + + it('should restore changed scalar values from the prior document', () => { + const prior = { status: 'draft', count: 1 }; + const forward = { status: 'published' }; + + assert.deepStrictEqual(invertMergePatch(forward, prior), { status: 'draft' }); + }); + + it('should recurse into nested object patches', () => { + const prior = { meta: { reviewed: false, owner: 'ada' } }; + const forward = { meta: { reviewed: true } }; + + assert.deepStrictEqual(invertMergePatch(forward, prior), { meta: { reviewed: false } }); + }); + + it('should restore values removed by a forward delete patch', () => { + const prior = { a: 1, retired: 'keep' }; + const forward = { retired: null }; + + assert.deepStrictEqual(invertMergePatch(forward, prior), { retired: 'keep' }); + }); + + it('should round-trip the touched keys while preserving concurrent sibling changes', () => { + const prior = { status: 'draft', meta: { reviewed: false, owner: 'ada' } }; + const forward = { status: 'published', meta: { reviewed: true }, added: 'new' }; + const inverse = invertMergePatch(forward, prior); + const afterForward = applyMergePatch(prior, forward); + + assert.deepStrictEqual(applyMergePatch(afterForward, inverse), prior); + + // A sibling key added after the forward patch is outside the inverse key structure. + const withConcurrentSibling = applyMergePatch(afterForward, { + concurrent: 'survives', + meta: { note: 'keep' } + }); + + assert.deepStrictEqual(applyMergePatch(withConcurrentSibling, inverse), { + status: 'draft', + meta: { reviewed: false, owner: 'ada', note: 'keep' }, + concurrent: 'survives' + }); + }); + }); + + describe('tryCreateInverseMergePatch', () => { + it('should reject scalar and array forward patches so undo can value-replace the cell', () => { + const prior = JSON.stringify({ a: 1 }); + + // RFC 7396 scalar and array patches replace the whole document, so a key-limited inverse is not faithful. + assert.strictEqual(tryCreateInverseMergePatch('5', prior), undefined); + assert.strictEqual(tryCreateInverseMergePatch('[1,2]', prior), undefined); + }); + + it('should reject inverse patches that would restore an explicit null leaf', () => { + const prior = JSON.stringify({ a: null, b: 1 }); + const forward = JSON.stringify({ a: 2 }); + + // Emitting { a: null } would delete a instead of restoring an explicit JSON null. + assert.strictEqual(tryCreateInverseMergePatch(forward, prior), undefined); + }); + + it('should still emit null when deleting a key added by the forward patch', () => { + const prior = JSON.stringify({ b: 1 }); + const forward = JSON.stringify({ a: 2 }); + + assert.deepStrictEqual( + JSON.parse(tryCreateInverseMergePatch(forward, prior)!), + { a: null } + ); + }); + + it('should recurse into nested additions so concurrent nested siblings survive undo', () => { + const prior = JSON.stringify({}); + const forward = JSON.stringify({ meta: { reviewed: true } }); + const inverse = JSON.parse(tryCreateInverseMergePatch(forward, prior)!); + + assert.deepStrictEqual(inverse, { meta: { reviewed: null } }); + assert.deepStrictEqual( + applyMergePatch({ meta: { reviewed: true, note: 'keep' } }, inverse), + { meta: { note: 'keep' } } + ); + }); + }); }); diff --git a/tests/unit/nativeWorker.test.ts b/tests/unit/nativeWorker.test.ts index 13d3c03..33880e8 100644 --- a/tests/unit/nativeWorker.test.ts +++ b/tests/unit/nativeWorker.test.ts @@ -7,6 +7,7 @@ import * as v8 from 'node:v8'; import { EventEmitter } from 'node:events'; import * as vscode from 'vscode'; import { isNativeAvailable, NativeWorkerProcess } from '../../src/nativeWorker'; +import { applyMergePatch } from '../../src/core/json-utils'; import type { DatabaseOperations } from '../../src/core/types'; interface RecordedNativeCall { @@ -15,6 +16,9 @@ interface RecordedNativeCall { args: unknown[]; } +type NativeCallResponse = { result: unknown } | { error: string }; +type NativeCallResponder = (call: RecordedNativeCall) => NativeCallResponse; + function encodeNativeMessage(message: unknown): Buffer { // Native worker messages are length-prefixed V8 payloads, matching NativeWorkerProcess.writeMessage. const payload = v8.serialize(message); @@ -23,7 +27,10 @@ function encodeNativeMessage(message: unknown): Buffer { return Buffer.concat([header, payload]); } -function createRecordingNativeProcess(recordedCalls: RecordedNativeCall[]) { +function createRecordingNativeProcess( + recordedCalls: RecordedNativeCall[], + respondToCall: NativeCallResponder = () => ({ result: { changes: 1, lastInsertRowId: 1 } }) +) { const mockProcess = new EventEmitter() as any; mockProcess.stdout = new EventEmitter(); mockProcess.stderr = new EventEmitter(); @@ -49,9 +56,8 @@ function createRecordingNativeProcess(recordedCalls: RecordedNativeCall[]) { const call = v8.deserialize(payload) as RecordedNativeCall; recordedCalls.push(call); - // Every operation under test only needs a successful native response. queueMicrotask(() => { - emitMessage({ id: call.id, result: { changes: 1, lastInsertRowId: 1 } }); + emitMessage({ id: call.id, ...respondToCall(call) }); }); } }; @@ -197,13 +203,13 @@ describe('createNativeDatabaseConnection', () => { mock.restoreAll(); }); - async function createRecordingConnection(): Promise<{ + async function createRecordingConnection(respondToCall?: NativeCallResponder): Promise<{ databaseOps: DatabaseOperations; calls: RecordedNativeCall[]; dispose: () => void; }> { const calls: RecordedNativeCall[] = []; - mock.method(child_process, 'spawn', () => createRecordingNativeProcess(calls)); + mock.method(child_process, 'spawn', () => createRecordingNativeProcess(calls, respondToCall)); const { createNativeDatabaseConnection } = require('../../src/nativeWorker'); const bundle = await createNativeDatabaseConnection({ fsPath: tempDir } as any); @@ -365,6 +371,304 @@ describe('createNativeDatabaseConnection', () => { } }); + it('undoes single json_patch cells through an inverse patch primitive', async () => { + const connection = await createRecordingConnection(); + + try { + const priorValue = JSON.stringify({ status: 'draft', owner: 'ada' }); + const forwardPatch = JSON.stringify({ status: 'published' }); + const concurrentPatch = JSON.stringify({ reviewer: 'grace' }); + connection.calls.length = 0; + + // Record the same operation order as the real engine: tracked patch, concurrent patch, undo. + await connection.databaseOps.updateCell('docs', 7, 'payload', null, forwardPatch); + await connection.databaseOps.updateCell('docs', 7, 'payload', null, concurrentPatch); + await connection.databaseOps.undoModification({ + modificationType: 'cell_update', + description: 'Undo patch payload', + targetTable: 'docs', + targetRowId: 7, + targetColumn: 'payload', + priorValue, + newValue: forwardPatch, + operation: 'json_patch' + }); + + assert.strictEqual(connection.calls.length, 3); + const undoCall = connection.calls[2]; + assert.strictEqual(undoCall.method, 'run'); + + const [sql, params] = undoCall.args as [string, unknown[]]; + assert.strictEqual( + sql, + `UPDATE "docs" SET "payload" = json_patch(COALESCE("payload", '{}'), ?) WHERE rowid = ?` + ); + + const inversePatch = JSON.parse(params[0] as string); + const currentValue = applyMergePatch( + applyMergePatch(JSON.parse(priorValue), JSON.parse(forwardPatch)), + JSON.parse(concurrentPatch) + ); + assert.deepStrictEqual(applyMergePatch(currentValue, inversePatch), { + status: 'draft', + owner: 'ada', + reviewer: 'grace' + }); + assert.deepStrictEqual(params, [JSON.stringify({ status: 'draft' }), 7]); + } finally { + connection.dispose(); + } + }); + + it('undoes scalar forward json_patch cells through value replacement', async () => { + const connection = await createRecordingConnection(); + + try { + const priorValue = JSON.stringify({ a: 1 }); + connection.calls.length = 0; + + await connection.databaseOps.undoModification({ + modificationType: 'cell_update', + description: 'Undo scalar patch payload', + targetTable: 'docs', + targetRowId: 7, + targetColumn: 'payload', + priorValue, + newValue: '5', + operation: 'json_patch' + }); + + assert.strictEqual(connection.calls.length, 1); + const [sql, params] = connection.calls[0].args as [string, unknown[]]; + assert.strictEqual(sql, `UPDATE "docs" SET "payload" = ? WHERE rowid = ?`); + assert.deepStrictEqual(params, [priorValue, 7]); + } finally { + connection.dispose(); + } + }); + + it('undoes prior explicit null JSON leaves through value replacement', async () => { + const connection = await createRecordingConnection(); + + try { + const priorValue = JSON.stringify({ a: null, b: 1 }); + const forwardPatch = JSON.stringify({ a: 2 }); + connection.calls.length = 0; + + await connection.databaseOps.undoModification({ + modificationType: 'cell_update', + description: 'Undo explicit null leaf', + targetTable: 'docs', + targetRowId: 9, + targetColumn: 'payload', + priorValue, + newValue: forwardPatch, + operation: 'json_patch' + }); + + assert.strictEqual(connection.calls.length, 1); + const [sql, params] = connection.calls[0].args as [string, unknown[]]; + assert.strictEqual(sql, `UPDATE "docs" SET "payload" = ? WHERE rowid = ?`); + assert.deepStrictEqual(params, [priorValue, 9]); + } finally { + connection.dispose(); + } + }); + + it('undoes nested json_patch additions without deleting concurrent nested siblings', async () => { + const connection = await createRecordingConnection(); + + try { + const priorValue = JSON.stringify({}); + const forwardPatch = JSON.stringify({ meta: { reviewed: true } }); + const concurrentPatch = JSON.stringify({ meta: { note: 'keep' } }); + connection.calls.length = 0; + + await connection.databaseOps.undoModification({ + modificationType: 'cell_update', + description: 'Undo nested addition', + targetTable: 'docs', + targetRowId: 10, + targetColumn: 'payload', + priorValue, + newValue: forwardPatch, + operation: 'json_patch' + }); + + assert.strictEqual(connection.calls.length, 1); + const [sql, params] = connection.calls[0].args as [string, unknown[]]; + assert.strictEqual( + sql, + `UPDATE "docs" SET "payload" = json_patch(COALESCE("payload", '{}'), ?) WHERE rowid = ?` + ); + + const inversePatch = JSON.parse(params[0] as string); + const currentValue = applyMergePatch( + applyMergePatch(JSON.parse(priorValue), JSON.parse(forwardPatch)), + JSON.parse(concurrentPatch) + ); + assert.deepStrictEqual(inversePatch, { meta: { reviewed: null } }); + assert.deepStrictEqual(applyMergePatch(currentValue, inversePatch), { + meta: { note: 'keep' } + }); + assert.deepStrictEqual(params, [JSON.stringify({ meta: { reviewed: null } }), 10]); + } finally { + connection.dispose(); + } + }); + + it('undoes batch json_patch cells through per-cell inverse patch operations', async () => { + const connection = await createRecordingConnection(); + + try { + const rowOnePrior = JSON.stringify({ count: 1, stable: 'one' }); + const rowTwoPrior = JSON.stringify({ count: 10, stable: 'two' }); + const rowOnePatch = JSON.stringify({ count: 2 }); + const rowTwoPatch = JSON.stringify({ count: 11 }); + connection.calls.length = 0; + + await connection.databaseOps.undoModification({ + modificationType: 'cell_update', + description: 'Undo batch patch payloads', + targetTable: 'docs', + affectedCells: [ + { rowId: 3, columnName: 'payload', priorValue: rowOnePrior, newValue: rowOnePatch, operation: 'json_patch' }, + { rowId: 4, columnName: 'payload', priorValue: rowTwoPrior, newValue: rowTwoPatch, operation: 'json_patch' }, + { rowId: 5, columnName: 'payload', priorValue: null, newValue: JSON.stringify({ added: true }), operation: 'json_patch' }, + { rowId: 6, columnName: 'title', priorValue: 'Original title', newValue: 'Changed title' } + ] + }); + + assert.strictEqual(connection.calls.length, 4); + assert.ok(connection.calls.every(call => call.method === 'run')); + assert.strictEqual( + connection.calls[0].args[0], + `UPDATE "docs" SET "payload" = json_patch(COALESCE("payload", '{}'), ?) WHERE rowid = ?` + ); + assert.deepStrictEqual(connection.calls[0].args[1], [JSON.stringify({ count: 1 }), 3]); + assert.strictEqual( + connection.calls[1].args[0], + `UPDATE "docs" SET "payload" = json_patch(COALESCE("payload", '{}'), ?) WHERE rowid = ?` + ); + assert.deepStrictEqual(connection.calls[1].args[1], [JSON.stringify({ count: 10 }), 4]); + assert.strictEqual(connection.calls[2].args[0], `UPDATE "docs" SET "payload" = ? WHERE rowid = ?`); + assert.deepStrictEqual(connection.calls[2].args[1], [null, 5]); + assert.strictEqual(connection.calls[3].args[0], `UPDATE "docs" SET "title" = ? WHERE rowid = ?`); + assert.deepStrictEqual(connection.calls[3].args[1], ['Original title', 6]); + } finally { + connection.dispose(); + } + }); + + it('falls back to value replacement when single-cell json_patch undo reports malformed JSON', async () => { + let failedPatch = false; + const connection = await createRecordingConnection((call) => { + const [sql] = call.args as [string]; + if (!failedPatch && call.method === 'run' && sql.includes('json_patch')) { + failedPatch = true; + return { error: 'malformed JSON' }; + } + return { result: { changes: 1, lastInsertRowId: 1 } }; + }); + + try { + const priorValue = JSON.stringify({ status: 'draft', owner: 'ada' }); + const forwardPatch = JSON.stringify({ status: 'published' }); + connection.calls.length = 0; + + await connection.databaseOps.undoModification({ + modificationType: 'cell_update', + description: 'Undo malformed JSON text', + targetTable: 'docs', + targetRowId: 11, + targetColumn: 'payload', + priorValue, + newValue: forwardPatch, + operation: 'json_patch' + }); + + assert.strictEqual(connection.calls.length, 2); + assert.strictEqual( + connection.calls[0].args[0], + `UPDATE "docs" SET "payload" = json_patch(COALESCE("payload", '{}'), ?) WHERE rowid = ?` + ); + assert.strictEqual(connection.calls[1].args[0], `UPDATE "docs" SET "payload" = ? WHERE rowid = ?`); + assert.deepStrictEqual(connection.calls[1].args[1], [priorValue, 11]); + } finally { + connection.dispose(); + } + }); + + it('falls back per cell when batch json_patch undo reports malformed JSON', async () => { + let failedPatch = false; + const connection = await createRecordingConnection((call) => { + const [sql, params] = call.args as [string, unknown[]]; + if (!failedPatch && call.method === 'run' && sql.includes('json_patch') && params[1] === 12) { + failedPatch = true; + return { error: 'malformed JSON' }; + } + return { result: { changes: 1, lastInsertRowId: 1 } }; + }); + + try { + const rowOnePrior = JSON.stringify({ status: 'draft', stable: 'one' }); + const rowTwoPrior = JSON.stringify({ status: 'draft', stable: 'two' }); + const forwardPatch = JSON.stringify({ status: 'published' }); + connection.calls.length = 0; + + await connection.databaseOps.undoModification({ + modificationType: 'cell_update', + description: 'Undo malformed batch JSON text', + targetTable: 'docs', + affectedCells: [ + { rowId: 12, columnName: 'payload', priorValue: rowOnePrior, newValue: forwardPatch, operation: 'json_patch' }, + { rowId: 13, columnName: 'payload', priorValue: rowTwoPrior, newValue: forwardPatch, operation: 'json_patch' } + ] + }); + + assert.strictEqual(connection.calls.length, 3); + assert.strictEqual( + connection.calls[0].args[0], + `UPDATE "docs" SET "payload" = json_patch(COALESCE("payload", '{}'), ?) WHERE rowid = ?` + ); + assert.strictEqual(connection.calls[1].args[0], `UPDATE "docs" SET "payload" = ? WHERE rowid = ?`); + assert.deepStrictEqual(connection.calls[1].args[1], [rowOnePrior, 12]); + assert.strictEqual( + connection.calls[2].args[0], + `UPDATE "docs" SET "payload" = json_patch(COALESCE("payload", '{}'), ?) WHERE rowid = ?` + ); + assert.deepStrictEqual(connection.calls[2].args[1], [JSON.stringify({ status: 'draft' }), 13]); + } finally { + connection.dispose(); + } + }); + + it('undoes json_patch cells with NULL prior values by restoring NULL', async () => { + const connection = await createRecordingConnection(); + + try { + connection.calls.length = 0; + + await connection.databaseOps.undoModification({ + modificationType: 'cell_update', + description: 'Undo NULL prior patch', + targetTable: 'docs', + targetRowId: 8, + targetColumn: 'payload', + priorValue: null, + newValue: JSON.stringify({ added: true }), + operation: 'json_patch' + }); + + assert.strictEqual(connection.calls.length, 1); + const [sql, params] = connection.calls[0].args as [string, unknown[]]; + assert.strictEqual(sql, `UPDATE "docs" SET "payload" = ? WHERE rowid = ?`); + assert.deepStrictEqual(params, [null, 8]); + } finally { + connection.dispose(); + } + }); + it('replays column_drop redo by dropping recorded dependent indexes first', async () => { const connection = await createRecordingConnection(); diff --git a/tests/unit/sqlite_db.test.ts b/tests/unit/sqlite_db.test.ts index 944b9f3..a67f728 100644 --- a/tests/unit/sqlite_db.test.ts +++ b/tests/unit/sqlite_db.test.ts @@ -94,6 +94,243 @@ describe('WasmDatabaseEngine', () => { assert.deepStrictEqual(JSON.parse(rows[1][2] as string), { y: 20, z: 30 }); }); + it('should undo single JSON patch edits without clobbering concurrent sibling keys', async () => { + await engine.executeQuery("DELETE FROM users"); + + const priorValue = JSON.stringify({ status: 'draft', owner: 'ada' }); + const forwardPatch = JSON.stringify({ status: 'published' }); + const concurrentPatch = JSON.stringify({ reviewer: 'grace' }); + await engine.insertRow('users', { id: 1, name: 'Patch Undo', age: 41, data: priorValue }); + + // The first edit is the tracked modification; the second edit happens before undo. + await engine.updateCell('users', 1, 'data', null, forwardPatch); + await engine.updateCell('users', 1, 'data', null, concurrentPatch); + + await engine.undoModification({ + modificationType: 'cell_update', + description: 'Undo JSON patch', + targetTable: 'users', + targetRowId: 1, + targetColumn: 'data', + priorValue, + newValue: forwardPatch, + operation: 'json_patch' + }); + + const result = await engine.executeQuery("SELECT data FROM users WHERE id = 1"); + assert.deepStrictEqual(JSON.parse(result[0].rows[0][0] as string), { + status: 'draft', + owner: 'ada', + reviewer: 'grace' + }); + }); + + it('should undo batch JSON patch edits with per-cell inverse patches', async () => { + await engine.executeQuery("DELETE FROM users"); + + const rowOnePrior = JSON.stringify({ count: 1, stable: 'one' }); + const rowTwoPrior = JSON.stringify({ count: 10, stable: 'two' }); + const rowOnePatch = JSON.stringify({ count: 2 }); + const rowTwoPatch = JSON.stringify({ count: 11 }); + await engine.insertRow('users', { id: 1, name: 'Batch One', age: 20, data: rowOnePrior }); + await engine.insertRow('users', { id: 2, name: 'Batch Two', age: 21, data: rowTwoPrior }); + + // Both cells receive tracked patches, then independent sibling patches before undo. + await engine.updateCellBatch('users', [ + { rowId: 1, column: 'data', value: rowOnePatch, operation: 'json_patch' }, + { rowId: 2, column: 'data', value: rowTwoPatch, operation: 'json_patch' } + ]); + await engine.updateCellBatch('users', [ + { rowId: 1, column: 'data', value: JSON.stringify({ concurrent: 'row-one' }), operation: 'json_patch' }, + { rowId: 2, column: 'data', value: JSON.stringify({ concurrent: 'row-two' }), operation: 'json_patch' } + ]); + + await engine.undoModification({ + modificationType: 'cell_update', + description: 'Undo batch JSON patch', + targetTable: 'users', + affectedCells: [ + { rowId: 1, columnName: 'data', priorValue: rowOnePrior, newValue: rowOnePatch, operation: 'json_patch' }, + { rowId: 2, columnName: 'data', priorValue: rowTwoPrior, newValue: rowTwoPatch, operation: 'json_patch' } + ] + }); + + const result = await engine.executeQuery("SELECT id, data FROM users ORDER BY id"); + assert.deepStrictEqual(JSON.parse(result[0].rows[0][1] as string), { + count: 1, + stable: 'one', + concurrent: 'row-one' + }); + assert.deepStrictEqual(JSON.parse(result[0].rows[1][1] as string), { + count: 10, + stable: 'two', + concurrent: 'row-two' + }); + }); + + it('should restore NULL when undoing a JSON patch edit whose prior value was NULL', async () => { + await engine.executeQuery("DELETE FROM users"); + + const forwardPatch = JSON.stringify({ added: 'value' }); + await engine.insertRow('users', { id: 1, name: 'Null Prior', age: 42, data: null }); + await engine.updateCell('users', 1, 'data', null, forwardPatch); + + await engine.undoModification({ + modificationType: 'cell_update', + description: 'Undo JSON patch on NULL', + targetTable: 'users', + targetRowId: 1, + targetColumn: 'data', + priorValue: null, + newValue: forwardPatch, + operation: 'json_patch' + }); + + const result = await engine.executeQuery("SELECT data FROM users WHERE id = 1"); + assert.strictEqual(result[0].rows[0][0], null); + }); + + it('should value-replace undo when the forward JSON patch was a scalar document replacement', async () => { + await engine.executeQuery("DELETE FROM users"); + + const priorValue = JSON.stringify({ a: 1 }); + const forwardPatch = '5'; + await engine.insertRow('users', { id: 1, name: 'Scalar Patch', age: 43, data: priorValue }); + await engine.updateCell('users', 1, 'data', null, forwardPatch); + + // A later write makes a json_patch inverse observable: merge undo would preserve extra, value-replace will not. + await engine.updateCell('users', 1, 'data', JSON.stringify({ extra: 'concurrent' })); + + await engine.undoModification({ + modificationType: 'cell_update', + description: 'Undo scalar JSON patch', + targetTable: 'users', + targetRowId: 1, + targetColumn: 'data', + priorValue, + newValue: forwardPatch, + operation: 'json_patch' + }); + + const result = await engine.executeQuery("SELECT data FROM users WHERE id = 1"); + assert.deepStrictEqual(JSON.parse(result[0].rows[0][0] as string), { a: 1 }); + }); + + it('should value-replace undo when restoring a prior explicit null JSON leaf', async () => { + await engine.executeQuery("DELETE FROM users"); + + const priorValue = JSON.stringify({ a: null, b: 1 }); + const forwardPatch = JSON.stringify({ a: 2 }); + await engine.insertRow('users', { id: 1, name: 'Explicit Null Leaf', age: 44, data: priorValue }); + await engine.updateCell('users', 1, 'data', null, forwardPatch); + + await engine.undoModification({ + modificationType: 'cell_update', + description: 'Undo explicit null JSON leaf', + targetTable: 'users', + targetRowId: 1, + targetColumn: 'data', + priorValue, + newValue: forwardPatch, + operation: 'json_patch' + }); + + const result = await engine.executeQuery("SELECT data FROM users WHERE id = 1"); + assert.deepStrictEqual(JSON.parse(result[0].rows[0][0] as string), { a: null, b: 1 }); + assert.ok(Object.prototype.hasOwnProperty.call(JSON.parse(result[0].rows[0][0] as string), 'a')); + }); + + it('should undo nested JSON patch additions without deleting concurrent nested siblings', async () => { + await engine.executeQuery("DELETE FROM users"); + + const priorValue = JSON.stringify({}); + const forwardPatch = JSON.stringify({ meta: { reviewed: true } }); + const concurrentPatch = JSON.stringify({ meta: { note: 'keep' } }); + await engine.insertRow('users', { id: 1, name: 'Nested Add', age: 45, data: priorValue }); + await engine.updateCell('users', 1, 'data', null, forwardPatch); + await engine.updateCell('users', 1, 'data', null, concurrentPatch); + + await engine.undoModification({ + modificationType: 'cell_update', + description: 'Undo nested JSON addition', + targetTable: 'users', + targetRowId: 1, + targetColumn: 'data', + priorValue, + newValue: forwardPatch, + operation: 'json_patch' + }); + + const result = await engine.executeQuery("SELECT data FROM users WHERE id = 1"); + assert.deepStrictEqual(JSON.parse(result[0].rows[0][0] as string), { + meta: { note: 'keep' } + }); + }); + + it('should fall back to value replacement when single-cell JSON patch undo sees non-JSON text', async () => { + await engine.executeQuery("DELETE FROM users"); + + const priorValue = JSON.stringify({ status: 'draft', owner: 'ada' }); + const forwardPatch = JSON.stringify({ status: 'published' }); + await engine.insertRow('users', { id: 1, name: 'Malformed Single', age: 46, data: priorValue }); + await engine.updateCell('users', 1, 'data', null, forwardPatch); + await engine.updateCell('users', 1, 'data', 'plain text'); + + await engine.undoModification({ + modificationType: 'cell_update', + description: 'Undo malformed single JSON patch', + targetTable: 'users', + targetRowId: 1, + targetColumn: 'data', + priorValue, + newValue: forwardPatch, + operation: 'json_patch' + }); + + const result = await engine.executeQuery("SELECT data FROM users WHERE id = 1"); + assert.deepStrictEqual(JSON.parse(result[0].rows[0][0] as string), { + status: 'draft', + owner: 'ada' + }); + }); + + it('should fall back per cell when batch JSON patch undo sees non-JSON text', async () => { + await engine.executeQuery("DELETE FROM users"); + + const rowOnePrior = JSON.stringify({ status: 'draft', stable: 'one' }); + const rowTwoPrior = JSON.stringify({ status: 'draft', stable: 'two' }); + const forwardPatch = JSON.stringify({ status: 'published' }); + await engine.insertRow('users', { id: 1, name: 'Malformed Batch One', age: 47, data: rowOnePrior }); + await engine.insertRow('users', { id: 2, name: 'Malformed Batch Two', age: 48, data: rowTwoPrior }); + await engine.updateCellBatch('users', [ + { rowId: 1, column: 'data', value: forwardPatch, operation: 'json_patch' }, + { rowId: 2, column: 'data', value: forwardPatch, operation: 'json_patch' } + ]); + await engine.updateCell('users', 1, 'data', 'plain text'); + await engine.updateCell('users', 2, 'data', null, JSON.stringify({ concurrent: 'survives' })); + + await engine.undoModification({ + modificationType: 'cell_update', + description: 'Undo malformed batch JSON patch', + targetTable: 'users', + affectedCells: [ + { rowId: 1, columnName: 'data', priorValue: rowOnePrior, newValue: forwardPatch, operation: 'json_patch' }, + { rowId: 2, columnName: 'data', priorValue: rowTwoPrior, newValue: forwardPatch, operation: 'json_patch' } + ] + }); + + const result = await engine.executeQuery("SELECT id, data FROM users ORDER BY id"); + assert.deepStrictEqual(JSON.parse(result[0].rows[0][1] as string), { + status: 'draft', + stable: 'one' + }); + assert.deepStrictEqual(JSON.parse(result[0].rows[1][1] as string), { + status: 'draft', + stable: 'two', + concurrent: 'survives' + }); + }); + it('should handle empty updates gracefully', async () => { await engine.updateCellBatch('users', []); assert.ok(true);