diff --git a/.changeset/fix-mutation-reconciliation.md b/.changeset/fix-mutation-reconciliation.md new file mode 100644 index 000000000..bdf150a59 --- /dev/null +++ b/.changeset/fix-mutation-reconciliation.md @@ -0,0 +1,5 @@ +--- +'@tanstack/db': patch +--- + +Fix same-key delete-then-insert reduction, preserve whole-row replacements through local adapters, and publish immutable previous values for live-object and replacement-object sync updates. Same-reference live rows still require an immutable provider `previousValue`; stale or partial values on that reused reference remain unsupported. diff --git a/packages/db/src/collection/changes.ts b/packages/db/src/collection/changes.ts index 962c7fff6..ad1487424 100644 --- a/packages/db/src/collection/changes.ts +++ b/packages/db/src/collection/changes.ts @@ -124,7 +124,13 @@ export class CollectionChangesManager< // Skip batching for user actions (forceEmit=true) to keep UI responsive if (this.shouldBatchEvents && !forceEmit) { // Add events to the batch - this.batchedEvents.push(...changes) + this.batchedEvents.push( + ...changes.map((change) => + change.type === `delete` + ? this.enrichChangeWithVirtualProps(change) + : change, + ), + ) return } @@ -146,7 +152,11 @@ export class CollectionChangesManager< combined.set( change.key, pending?.type === `delete` && change.type === `insert` - ? { ...change, type: `update`, previousValue: pending.value } + ? { + ...change, + type: `update`, + previousValue: pending.value, + } : change, ) } diff --git a/packages/db/src/collection/state.ts b/packages/db/src/collection/state.ts index c092f9307..9afe913ac 100644 --- a/packages/db/src/collection/state.ts +++ b/packages/db/src/collection/state.ts @@ -1016,11 +1016,17 @@ export class CollectionStateManager< // First collect all keys that will be affected by sync operations const changedKeys = new Set() const syncedInsertedOrUpdatedKeys = new Set() + const firstSyncOperations = new Map< + TKey, + OptimisticChangeMessage + >() for (const transaction of committedSyncedTransactions) { for (const operation of transaction.operations) { - changedKeys.add(operation.key as TKey) - if (operation.type !== `delete`) - syncedInsertedOrUpdatedKeys.add(operation.key as TKey) + const key = operation.key as TKey + changedKeys.add(key) + if (!firstSyncOperations.has(key)) + firstSyncOperations.set(key, operation) + if (operation.type !== `delete`) syncedInsertedOrUpdatedKeys.add(key) } for (const [key] of transaction.rowMetadataWrites) { changedKeys.add(key) @@ -1148,6 +1154,11 @@ export class CollectionStateManager< : 'remote' if (origin === `local`) localKeys.add(key) + // A sync source may reuse a live-reading row object, making an + // enriched snapshot cached for an earlier publication stale. + if (operation.type !== `delete`) + this.virtualPropsCache.delete(operation.value) + // Update synced data switch (operation.type) { case `insert`: @@ -1381,7 +1392,21 @@ export class CollectionStateManager< // Now check what actually changed in the final visible state for (const key of changedKeys) { - const previousVisibleValue = currentVisibleState.get(key) + const firstSyncOperation = firstSyncOperations.get(key) + // A live-reading source can change a reused row before this commit + // captures it. Later writes must not substitute an intermediate value. + const syncPreviousValue = + firstSyncOperation?.type === `update` && + currentVisibleState.get(key) === firstSyncOperation.value + ? firstSyncOperation.previousValue + : undefined + const previousVisibleValue = + !hasTruncateSync && + !previousOptimisticUpserts.has(key) && + !previousOptimisticDeletes.has(key) && + syncPreviousValue !== undefined + ? syncPreviousValue + : currentVisibleState.get(key) const newVisibleValue = this.get(key) // This returns the new derived state const previousVirtualProps = this.preSyncVirtualState.get(key) ?? diff --git a/packages/db/src/local-only.ts b/packages/db/src/local-only.ts index 911b0cb92..9de92f4d6 100644 --- a/packages/db/src/local-only.ts +++ b/packages/db/src/local-only.ts @@ -314,6 +314,7 @@ function createLocalOnlySync( let collection: Collection | null = null const sync: SyncConfig = { + rowUpdateMode: `full`, /** * Sync function that captures sync parameters and applies initial data * @param params - Sync parameters containing begin, write, and commit functions diff --git a/packages/db/src/local-storage.ts b/packages/db/src/local-storage.ts index be8e326a7..a07f4e527 100644 --- a/packages/db/src/local-storage.ts +++ b/packages/db/src/local-storage.ts @@ -737,6 +737,7 @@ function createLocalStorageSync( manualTrigger?: () => void collection: any } = { + rowUpdateMode: `full`, sync: (params: Parameters[`sync`]>[0]) => { const { begin, write, commit, markReady } = params diff --git a/packages/db/src/transactions.ts b/packages/db/src/transactions.ts index 738ec3ced..2673839c8 100644 --- a/packages/db/src/transactions.ts +++ b/packages/db/src/transactions.ts @@ -1,4 +1,5 @@ import { createDeferred } from './deferred' +import { deepEquals } from './utils' import { safeRandomUUID } from './utils/uuid' import { normalizeError } from './utils/error.js' import './duplicate-instance-check' @@ -159,9 +160,11 @@ function getTransactionAmbientScope(transaction: object): TransactionScope { * - (update, update) → update (replace with latest, union changes) * - (delete, delete) → delete (replace with latest) * - (insert, insert) → insert (replace with latest) + * - (delete, insert) → insert without an authoritative row, null if restoring + * the authoritative row, otherwise update * - * Note: (delete, update) and (delete, insert) should never occur as the collection - * layer prevents operations on deleted items within the same transaction. + * Note: (delete, update) should never occur as the collection layer prevents + * update operations on deleted items within the same transaction. * * @param existing - The existing mutation in the transaction * @param incoming - The new mutation being applied @@ -199,7 +202,8 @@ function mergePendingMutations( return null case `update-delete`: - // Delete after update: delete dominates + case `delete-delete`: + // Delete dominates an update or earlier delete. return incoming case `update-update`: { @@ -216,11 +220,39 @@ function mergePendingMutations( } } - case `delete-delete`: case `insert-insert`: // Same type: replace with latest return incoming + case `delete-insert`: { + const original = existing.collection._state.syncedData.get(existing.key) + if (original === undefined) return incoming + if (deepEquals(original, incoming.modified)) { + return null + } + + const modified = incoming.modified + const keys = new Set([...Object.keys(original), ...Object.keys(modified)]) + const changes: Partial = {} + for (const key of keys) { + if ( + Object.hasOwn(original, key) !== Object.hasOwn(modified, key) || + !deepEquals(original[key as keyof T], modified[key as keyof T]) + ) { + changes[key as keyof T] = modified[key as keyof T] + } + } + + return { + ...incoming, + type: `update`, + original, + changes, + metadata: incoming.metadata ?? existing.metadata, + syncMetadata: { ...existing.syncMetadata, ...incoming.syncMetadata }, + } + } + default: { // Exhaustiveness check const _exhaustive: never = `${existing.type}-${incoming.type}` as never @@ -450,6 +482,7 @@ class Transaction> { * - **insert + delete** → removed (mutations cancel each other out) * - **update + delete** → delete (delete dominates) * - **update + update** → update (union changes, keep first original) + * - **delete + insert** → removed if restored, otherwise update * - **same type** → replace with latest * * This merging reduces over-the-wire churn and keeps the optimistic local view diff --git a/packages/db/tests/collection-state-retention-oracle.property.test.ts b/packages/db/tests/collection-state-retention-oracle.property.test.ts index 2034bba97..8d7832738 100644 --- a/packages/db/tests/collection-state-retention-oracle.property.test.ts +++ b/packages/db/tests/collection-state-retention-oracle.property.test.ts @@ -2,10 +2,12 @@ import { fc, test as fcTest } from '@fast-check/vitest' import { expect, it } from 'vitest' import { createCollection } from '../src/collection/index.js' import { DuplicateKeySyncError } from '../src/errors.js' +import { BTreeIndex } from '../src/indexes/btree-index.js' import { createTransaction } from '../src/transactions.js' import { oraclePropertyOptions, oracleRuns } from './oracle-config.js' import { runOptimisticHistory } from './optimistic-history-oracle.js' import type { OptimisticStep } from './optimistic-history-oracle.js' +import type { CollectionChangesManager } from '../src/collection/changes.js' import type { Collection } from '../src/collection/index.js' import type { SyncConfig, TransactionState } from '../src/types.js' @@ -726,6 +728,751 @@ it(`publishes a virtual-state update when a restarted optimistic row is confirme } }) +type LivePreviousRow = { + id: number + value: number | null | undefined +} + +function observeValuePublications( + collection: Collection, + includeInitialState = false, +) { + const publications: Array< + Array<{ + type: string + key: string | number + value: LivePreviousRow[`value`] + previousValue: LivePreviousRow[`value`] + }> + > = [] + const subscription = collection.subscribeChanges( + (changes) => + publications.push( + changes.map((change) => ({ + type: change.type, + key: change.key, + value: change.value.value, + previousValue: change.previousValue?.value, + })), + ), + { includeInitialState }, + ) + return { publications, subscription } +} + +async function runImmutablePreviousValuePublication( + initial: LivePreviousRow[`value`], + updates: ReadonlyArray, +) { + let sync!: Parameters[`sync`]>[0] + let liveValue = initial + let writes = 0 + const liveRow = { + id: 1, + get value() { + return liveValue + }, + } + const collection = createCollection({ + getKey: (row) => row.id, + startSync: true, + sync: { + rowUpdateMode: `full`, + sync: (actions) => { + sync = actions + actions.begin() + actions.write({ type: `insert`, value: liveRow }) + actions.write({ type: `insert`, value: { id: 2, value: 10 } }) + actions.commit() + actions.markReady() + }, + }, + }) + const { publications, subscription } = observeValuePublications(collection) + try { + sync.begin() + let previousValue = initial + for (const value of updates) { + liveValue = value + sync.write({ + type: `update`, + value: liveRow, + previousValue: { id: 1, value: previousValue }, + }) + writes++ + previousValue = value + } + sync.write({ + type: `update`, + value: { id: 2, value: 11 }, + previousValue: { id: 2, value: 10 }, + }) + writes++ + expect(sync.commit(), `live-value sync commit reached`).toBe(true) + expect(writes, `all live-value writes reached`).toBe(updates.length + 1) + expect(publications).toStrictEqual([ + [ + { + type: `update`, + key: 1, + value: updates.at(-1), + previousValue: initial, + }, + { + type: `update`, + key: 2, + value: 11, + previousValue: 10, + }, + ], + ]) + } finally { + subscription.unsubscribe() + await collection.cleanup() + } +} + +it.each([ + { initial: 0, updates: [1] }, + { initial: 0, updates: [1, 2] }, + { initial: null, updates: [1] }, + { initial: undefined, updates: [1] }, +] satisfies Array<{ + initial: LivePreviousRow[`value`] + updates: Array +}>)( + `publishes a live value from immutable previous state: %j`, + async ({ initial, updates }) => { + await runImmutablePreviousValuePublication(initial, updates) + }, +) + +it(`keeps the first queued before-image when metadata reserves the key`, async () => { + let sync!: Parameters[`sync`]>[0] + let liveValue: LivePreviousRow[`value`] = 0 + const liveRow = { + id: 1, + get value() { + return liveValue + }, + } + const collection = createCollection({ + getKey: (row) => row.id, + startSync: true, + sync: { + rowUpdateMode: `full`, + sync: (actions) => { + sync = actions + actions.begin() + actions.write({ type: `insert`, value: liveRow }) + actions.write({ type: `insert`, value: { id: 2, value: 0 } }) + actions.commit() + actions.markReady() + }, + }, + }) + let release!: () => void + const hold = new Promise((resolve) => { + release = resolve + }) + const blocker = createTransaction({ + autoCommit: false, + mutationFn: () => hold, + }) + await collection.stateWhenReady() + const { publications, subscription } = observeValuePublications(collection) + const receipts: Array> = [] + let blockerCommit: Promise | undefined + + const commitQueuedSync = () => { + const receipt = sync.commit() + expect(receipt, `persisting work queues sync`).not.toBe(true) + if (receipt === true) throw new Error(`sync was not queued`) + void receipt.catch(() => undefined) + receipts.push(receipt) + } + + try { + blocker.mutate(() => + collection.update(2, (draft) => { + draft.value = 20 + }), + ) + publications.length = 0 + blockerCommit = blocker.commit() + await Promise.resolve() + + sync.begin() + sync.metadata!.row.set(1, { phase: `metadata-first` }) + commitQueuedSync() + + liveValue = 1 + sync.begin() + sync.write({ + type: `update`, + value: liveRow, + previousValue: { id: 1, value: 0 }, + }) + commitQueuedSync() + + liveValue = 2 + sync.begin() + sync.write({ + type: `update`, + value: liveRow, + previousValue: { id: 1, value: 1 }, + }) + commitQueuedSync() + + release() + await blockerCommit + await Promise.all(receipts) + + expect( + publications.flat().filter(({ key }) => key === 1), + `the queued drain publishes one complete key transition`, + ).toStrictEqual([{ type: `update`, key: 1, value: 2, previousValue: 0 }]) + expect(collection._state.syncedMetadata.get(1)).toStrictEqual({ + phase: `metadata-first`, + }) + } finally { + release() + if (blocker.state === `pending` || blocker.state === `persisting`) + blocker.rollback() + await blockerCommit?.catch(() => undefined) + subscription.unsubscribe() + await collection.cleanup() + await Promise.allSettled(receipts) + } +}) + +it(`snapshots a buffered delete before its row object is reused`, async () => { + const reusedRow: LivePreviousRow = { id: 1, value: 0 } + const collection = createCollection({ + getKey: (row) => row.id, + startSync: true, + sync: { + sync: (actions) => { + actions.begin() + actions.write({ type: `insert`, value: reusedRow }) + actions.commit() + actions.markReady() + }, + }, + }) + const publications: Array< + Array<{ + type: string + value: LivePreviousRow[`value`] + previousValue: LivePreviousRow[`value`] + previousSynced: boolean | undefined + previousOrigin: string | undefined + }> + > = [] + const subscription = collection.subscribeChanges( + (changes) => + publications.push( + changes.map((change) => { + const previous = change.previousValue as + | (LivePreviousRow & { $synced: boolean; $origin: string }) + | undefined + return { + type: change.type, + value: change.value.value, + previousValue: previous?.value, + previousSynced: previous?.$synced, + previousOrigin: previous?.$origin, + } + }), + ), + { includeInitialState: true }, + ) + const changes = ( + collection as unknown as { + _changes: CollectionChangesManager + } + )._changes + + try { + await collection.stateWhenReady() + publications.length = 0 + changes.shouldBatchEvents = true + changes.emitEvents([{ type: `delete`, key: 1, value: reusedRow }]) + expect(publications, `delete remains buffered`).toStrictEqual([]) + + reusedRow.value = 1 + collection._state.optimisticUpserts.set(1, reusedRow) + changes.emitEvents( + [{ type: `insert`, key: 1, value: { ...reusedRow } }], + true, + ) + expect(publications).toStrictEqual([ + [ + { + type: `update`, + value: 1, + previousValue: 0, + previousSynced: true, + previousOrigin: `remote`, + }, + ], + ]) + } finally { + subscription.unsubscribe() + await collection.cleanup() + } +}) + +it(`uses the preserved visible row when rollback releases a queued sync`, async () => { + let sync!: Parameters[`sync`]>[0] + let rejectPersistence!: (error: Error) => void + const persistenceGate = new Promise((_resolve, reject) => { + rejectPersistence = reject + }) + const collection = createCollection({ + getKey: (row) => row.id, + startSync: true, + sync: { + rowUpdateMode: `full`, + sync: (actions) => { + sync = actions + actions.begin() + actions.write({ type: `insert`, value: { id: 1, value: 0 } }) + actions.commit() + actions.markReady() + }, + }, + }) + const transaction = createTransaction({ + autoCommit: false, + mutationFn: () => persistenceGate, + }) + const publications = observeValuePublications(collection) + + try { + await collection.stateWhenReady() + const index = collection.createIndex((row) => row.value, { + indexType: BTreeIndex, + }) + transaction.mutate(() => + collection.update(1, (draft) => { + draft.value = 1 + }), + ) + publications.publications.length = 0 + const commit = transaction.commit() + await Promise.resolve() + + sync.begin() + sync.write({ + type: `update`, + value: { id: 1, value: 2 }, + previousValue: { id: 1, value: 0 }, + }) + const receipt = sync.commit() + expect(receipt).not.toBe(true) + + const failure = new Error(`queued mutation failed`) + rejectPersistence(failure) + await expect(commit).rejects.toBe(failure) + if (receipt !== true) await receipt + + expect(collection.get(1)?.value).toBe(2) + expect(publications.publications).toStrictEqual([ + [{ type: `update`, key: 1, value: 2, previousValue: 1 }], + ]) + expect(index.lookup(`eq`, 0)).toEqual(new Set()) + expect(index.lookup(`eq`, 1)).toEqual(new Set()) + expect(index.lookup(`eq`, 2)).toEqual(new Set([1])) + } finally { + publications.subscription.unsubscribe() + if (transaction.state === `pending` || transaction.state === `persisting`) + transaction.rollback() + await collection.cleanup() + } +}) + +it(`publishes a provider update for an unseen key as an insert`, async () => { + let sync!: Parameters[`sync`]>[0] + const collection = createCollection({ + getKey: (row) => row.id, + startSync: true, + sync: { + rowUpdateMode: `full`, + sync: (actions) => { + sync = actions + actions.markReady() + }, + }, + }) + const publications = observeValuePublications(collection, true) + + try { + await collection.stateWhenReady() + sync.begin() + sync.write({ + type: `update`, + value: { id: 1, value: 1 }, + previousValue: { id: 1, value: 0 }, + }) + expect(sync.commit()).toBe(true) + expect(publications.publications).toStrictEqual([ + [], + [{ type: `insert`, key: 1, value: 1, previousValue: undefined }], + ]) + } finally { + publications.subscription.unsubscribe() + await collection.cleanup() + } +}) + +it(`suppresses a replacement-object redelivery with a stale before-image`, async () => { + let sync!: Parameters[`sync`]>[0] + const collection = createCollection({ + getKey: (row) => row.id, + startSync: true, + sync: { + rowUpdateMode: `full`, + sync: (actions) => { + sync = actions + actions.begin() + actions.write({ type: `insert`, value: { id: 1, value: 1 } }) + actions.commit() + actions.markReady() + }, + }, + }) + const publications = observeValuePublications(collection) + + try { + await collection.stateWhenReady() + sync.begin() + sync.write({ + type: `update`, + value: { id: 1, value: 1 }, + previousValue: { id: 1, value: 0 }, + }) + expect(sync.commit()).toBe(true) + expect(publications.publications).toStrictEqual([]) + } finally { + publications.subscription.unsubscribe() + await collection.cleanup() + } +}) + +it(`suppresses a replacement object's partial before-image without index drift`, async () => { + let sync!: Parameters[`sync`]>[0] + const collection = createCollection({ + getKey: (row) => row.id, + startSync: true, + sync: { + rowUpdateMode: `full`, + sync: (actions) => { + sync = actions + actions.begin() + actions.write({ type: `insert`, value: { id: 1, value: 1 } }) + actions.commit() + actions.markReady() + }, + }, + }) + const publications = observeValuePublications(collection) + + try { + await collection.stateWhenReady() + const index = collection.createIndex((row) => row.value, { + indexType: BTreeIndex, + }) + sync.begin() + sync.write({ + type: `update`, + value: { id: 1, value: 1 }, + previousValue: { id: 1 } as LivePreviousRow, + }) + expect(sync.commit()).toBe(true) + + expect( + { + publications: publications.publications, + indexed: index.lookup(`eq`, 1), + rangeDomains: ( + index as unknown as { + rangeValueDomains: Map + } + ).rangeValueDomains, + }, + `partial before-image observation`, + ).toStrictEqual({ + publications: [], + indexed: new Set([1]), + rangeDomains: new Map([[`number`, 1]]), + }) + } finally { + publications.subscription.unsubscribe() + await collection.cleanup() + } +}) + +it(`retains the pre-batch value when a later update supplies an intermediate snapshot`, async () => { + let sync!: Parameters[`sync`]>[0] + const collection = createCollection({ + getKey: (row) => row.id, + startSync: true, + sync: { + rowUpdateMode: `full`, + sync: (actions) => { + sync = actions + actions.begin() + actions.write({ type: `insert`, value: { id: 1, value: 0 } }) + actions.commit() + actions.markReady() + }, + }, + }) + const { publications, subscription } = observeValuePublications(collection) + try { + await collection.stateWhenReady() + sync.begin() + sync.write({ type: `update`, value: { id: 1, value: 1 } }) + sync.write({ + type: `update`, + value: { id: 1, value: 2 }, + previousValue: { id: 1, value: 1 }, + }) + expect(sync.commit(), `repeated update batch applies`).toBe(true) + expect(publications).toStrictEqual([ + [{ type: `update`, key: 1, value: 2, previousValue: 0 }], + ]) + } finally { + subscription.unsubscribe() + await collection.cleanup() + } +}) + +it(`publishes an insert when only its following update supplies a previous value`, async () => { + let sync!: Parameters[`sync`]>[0] + const collection = createCollection({ + getKey: (row) => row.id, + startSync: true, + sync: { + rowUpdateMode: `full`, + sync: (actions) => { + sync = actions + actions.markReady() + }, + }, + }) + const { publications, subscription } = observeValuePublications(collection) + try { + await collection.stateWhenReady() + sync.begin() + sync.write({ type: `insert`, value: { id: 1, value: 1 } }) + sync.write({ + type: `update`, + value: { id: 1, value: 2 }, + previousValue: { id: 1, value: 1 }, + }) + expect(sync.commit(), `insert then update batch applies`).toBe(true) + expect(publications).toStrictEqual([ + [{ type: `insert`, key: 1, value: 2, previousValue: undefined }], + ]) + } finally { + subscription.unsubscribe() + await collection.cleanup() + } +}) + +it(`publishes a truncate rebuild as an insert despite an update snapshot`, async () => { + let sync!: Parameters[`sync`]>[0] + const collection = createCollection({ + getKey: (row) => row.id, + startSync: true, + sync: { + rowUpdateMode: `full`, + sync: (actions) => { + sync = actions + actions.begin() + actions.write({ type: `insert`, value: { id: 1, value: 0 } }) + actions.commit() + actions.markReady() + }, + }, + }) + const { publications, subscription } = observeValuePublications(collection) + try { + await collection.stateWhenReady() + sync.begin() + sync.truncate() + sync.write({ + type: `update`, + value: { id: 1, value: 1 }, + previousValue: { id: 1, value: 0 }, + }) + expect(sync.commit(), `truncate rebuild applies`).toBe(true) + expect(publications).toStrictEqual([ + [ + { type: `delete`, key: 1, value: 0, previousValue: undefined }, + { type: `insert`, key: 1, value: 1, previousValue: undefined }, + ], + ]) + } finally { + subscription.unsubscribe() + await collection.cleanup() + } +}) + +it(`does not publish an authoritative update hidden by an optimistic overlay`, async () => { + let sync!: Parameters[`sync`]>[0] + let release!: () => void + const hold = new Promise((resolve) => { + release = resolve + }) + const collection = createCollection({ + getKey: (row) => row.id, + startSync: true, + sync: { + rowUpdateMode: `full`, + sync: (actions) => { + sync = actions + actions.begin() + actions.write({ type: `insert`, value: { id: 1, value: 0 } }) + actions.commit() + actions.markReady() + }, + }, + }) + const transaction = createTransaction({ + autoCommit: false, + mutationFn: () => hold, + }) + let commit: Promise | undefined + const { publications, subscription } = observeValuePublications(collection) + try { + await collection.stateWhenReady() + transaction.mutate(() => + collection.update(1, (draft) => { + draft.value = 100 + }), + ) + publications.length = 0 + commit = transaction.commit() + await Promise.resolve() + + sync.begin({ immediate: true }) + sync.write({ + type: `update`, + value: { id: 1, value: 1 }, + previousValue: { id: 1, value: 0 }, + }) + expect(sync.commit(), `hidden authoritative update applies`).toBe(true) + expect(collection.get(1)?.value, `optimistic overlay remains visible`).toBe( + 100, + ) + expect(publications, `hidden update is not published`).toStrictEqual([]) + } finally { + subscription.unsubscribe() + release() + await commit + await collection.cleanup() + } +}) + +it(`does not carry previous-value state across a failed sync session`, async () => { + let sync!: Parameters[`sync`]>[0] + let session = 0 + let liveValue: LivePreviousRow[`value`] = 0 + const failure = new Error(`live value read failed`) + const liveRow = { + id: 1, + get value() { + return liveValue + }, + } + const collection = createCollection({ + getKey: (row) => row.id, + startSync: true, + sync: { + rowUpdateMode: `full`, + sync: (actions) => { + sync = actions + session++ + actions.begin() + if (session === 1) { + actions.write({ type: `insert`, value: liveRow }) + actions.write({ type: `insert`, value: { id: 2, value: 0 } }) + } else { + actions.write({ type: `insert`, value: { id: 1, value: 10 } }) + } + actions.commit() + actions.markReady() + }, + }, + }) + let subscription: ReturnType | undefined + try { + liveValue = 1 + sync.begin() + sync.write({ + type: `update`, + value: liveRow, + previousValue: { id: 1, value: 0 }, + }) + sync.write({ + type: `update`, + value: { + id: 2, + get value(): number { + throw failure + }, + }, + previousValue: { id: 2, value: 0 }, + }) + let thrown: unknown + try { + sync.commit() + } catch (error) { + thrown = error + } + expect(thrown, `the poisoned batch reached value comparison`).toBe(failure) + + const optimisticFailure = new Error(`optimistic mutation failed`) + const optimistic = createTransaction({ + autoCommit: false, + mutationFn: () => { + throw optimisticFailure + }, + }) + const persistence = optimistic.isPersisted.promise.catch((error) => error) + optimistic.mutate(() => collection.insert({ id: 3, value: 30 })) + expect(collection.has(3), `failed-sync optimistic overlay appears`).toBe( + true, + ) + await expect(optimistic.commit()).rejects.toBe(optimisticFailure) + expect(await persistence, `failed optimistic receipt`).toBe( + optimisticFailure, + ) + expect( + collection.has(3), + `failed-sync optimistic overlay rolls back before restart`, + ).toBe(false) + + await collection.cleanup() + collection.startSyncImmediate() + expect(session, `replacement sync session started`).toBe(2) + const observation = observeValuePublications(collection) + subscription = observation.subscription + sync.begin() + sync.write({ type: `update`, value: { id: 1, value: 11 } }) + expect(sync.commit(), `clean replacement batch applies`).toBe(true) + expect(observation.publications).toStrictEqual([ + [{ type: `update`, key: 1, value: 11, previousValue: 10 }], + ]) + } finally { + subscription?.unsubscribe() + await collection.cleanup() + } +}) + fcTest.prop( [fc.array(retentionActionArbitrary, { minLength: 1, maxLength: 20 })], oraclePropertyOptions(100, `collection-state.retention`), @@ -1038,6 +1785,18 @@ it.each([true, false])( ) }, ) +it(`publishes prior optimistic ownership when rollback reveals an identical authoritative row`, async () => { + await runOptimisticHistory( + [{ id: 3, a: 0, b: 0, c: 0 }], + [ + { type: `delete`, key: 3, optimistic: true }, + { type: `edit`, key: 3, fields: { c: 0 }, optimistic: true }, + { type: `sync`, rows: [], truncate: false, immediate: false, copies: 1 }, + { type: `settle`, slot: 0, success: true, cascade: false }, + { type: `settle`, slot: 0, success: false, cascade: false }, + ], + ) +}) fcTest.prop([optimisticHistory], { numRuns: oracleRuns(60), seed: 86103 })( `matches optimistic ownership and publication histories with a fixed seed`, async ({ initial, steps }) => { diff --git a/packages/db/tests/local-only.test.ts b/packages/db/tests/local-only.test.ts index acdb19b71..71132a1d1 100644 --- a/packages/db/tests/local-only.test.ts +++ b/packages/db/tests/local-only.test.ts @@ -749,5 +749,50 @@ describe(`LocalOnly Collection`, () => { // Item should be rolled back expect(collection.has(700)).toBe(false) }) + + it(`keeps fields omitted by a same-key replacement absent`, async () => { + const replacementCollection = createCollection< + TestItem, + number, + LocalOnlyCollectionUtils + >( + localOnlyCollectionOptions({ + id: `local-only-replacement`, + getKey: (item: TestItem) => item.id, + initialData: [ + { id: 1, name: `replacement`, completed: true, number: 1 }, + ], + }), + ) + const transaction = createTransaction({ + autoCommit: false, + mutationFn: ({ transaction: committed }: any) => + Promise.resolve( + replacementCollection.utils.acceptMutations(committed), + ), + }) + + try { + await replacementCollection.stateWhenReady() + transaction.mutate(() => { + replacementCollection.delete(1) + replacementCollection.insert({ id: 1, name: `replacement` }) + }) + await transaction.commit() + + expect(stripVirtualProps(replacementCollection.get(1))).toStrictEqual({ + id: 1, + name: `replacement`, + }) + } finally { + if ( + transaction.state === `pending` || + transaction.state === `persisting` + ) + transaction.rollback() + await transaction.isPersisted.promise.catch(() => undefined) + await replacementCollection.cleanup() + } + }) }) }) diff --git a/packages/db/tests/local-storage.test.ts b/packages/db/tests/local-storage.test.ts index 8a60f8a84..1f407f27d 100644 --- a/packages/db/tests/local-storage.test.ts +++ b/packages/db/tests/local-storage.test.ts @@ -1016,6 +1016,70 @@ describe(`localStorage collection`, () => { subscription.unsubscribe() }) + it(`keeps replacement fields absent in memory, storage, and reload`, async () => { + type ReplacementRow = { + id: string + title: string + removed?: string + } + const createReplacementCollection = () => + createCollection( + localStorageCollectionOptions({ + storageKey: `replacement-rows`, + storage: mockStorage, + storageEventApi: mockStorageEventApi, + getKey: (row) => row.id, + }), + ) + const collection = createReplacementCollection() + let reopened: typeof collection | undefined + const transaction = createTransaction({ + autoCommit: false, + mutationFn: ({ transaction: committed }: any) => + Promise.resolve(collection.utils.acceptMutations(committed)), + }) + + try { + await collection.preload() + const insert = collection.insert({ + id: `row`, + title: `replacement`, + removed: `old`, + }) + await insert.isPersisted.promise + + transaction.mutate(() => { + collection.delete(`row`) + collection.insert({ id: `row`, title: `replacement` }) + }) + await transaction.commit() + + const memoryRow = collection.get(`row`)! + const stored = JSON.parse( + mockStorage.getItem(`replacement-rows`)!, + ) as Record + await collection.cleanup() + reopened = createReplacementCollection() + await reopened.preload() + const reloadedRow = reopened.get(`row`)! + + expect({ + memory: Object.hasOwn(memoryRow, `removed`), + storage: Object.hasOwn(stored[`s:row`]!.data, `removed`), + reload: Object.hasOwn(reloadedRow, `removed`), + }).toStrictEqual({ memory: false, storage: false, reload: false }) + } finally { + if ( + transaction.state === `pending` || + transaction.state === `persisting` + ) + transaction.rollback() + await transaction.isPersisted.promise.catch(() => undefined) + await reopened?.cleanup() + await collection.cleanup() + } + }) + it(`should only accept mutations for the specific collection`, async () => { const collection1 = createCollection( localStorageCollectionOptions({ diff --git a/packages/db/tests/optimistic-transaction-oracle.property.test.ts b/packages/db/tests/optimistic-transaction-oracle.property.test.ts index f66bc76c0..0990d0133 100644 --- a/packages/db/tests/optimistic-transaction-oracle.property.test.ts +++ b/packages/db/tests/optimistic-transaction-oracle.property.test.ts @@ -11,6 +11,7 @@ import { withHistoryCleanup, } from './optimistic-history-oracle.js' import { oraclePropertyOptions, oracleRuns } from './oracle-config.js' +import type { Collection } from '../src/collection/index.js' import type { ChangeMessage, SyncConfig } from '../src/types.js' type Row = { id: number; value: number; note: string } @@ -230,12 +231,444 @@ describe(`Same-key transaction laws`, () => { ) }) +type ReplacementRow = { + id: number + value: number | null | undefined + note?: string | null + added?: string + nested?: { count: number } +} + +type ReplacementMutation = { + type: Operation + original: object + modified: object + changes: object + metadata: unknown + syncMetadata: Record +} + +async function runDeleteInsertReplacement( + replacements: ReadonlyArray, + expected: ReplacementMutation | undefined, + original: ReplacementRow = { id: 1, value: 0, note: `original` }, + author?: (collection: Collection) => void, +) { + const collection = createCollection({ + getKey: (row) => row.id, + sync: { + getSyncMetadata: () => ({ insert: true, shared: `insert` }), + sync: (actions) => { + actions.begin() + actions.write({ type: `insert`, value: original }) + actions.metadata?.row.set(1, { delete: true, shared: `delete` }) + actions.commit() + actions.markReady() + }, + }, + }) + let calls = 0 + let request: Array = [] + const transaction = createTransaction({ + autoCommit: false, + mutationFn: ({ transaction: persisted }) => { + calls++ + request = persisted.mutations.map((mutation) => ({ + type: mutation.type, + original: { ...mutation.original }, + modified: { ...mutation.modified }, + changes: { ...mutation.changes }, + metadata: mutation.metadata, + syncMetadata: { ...mutation.syncMetadata }, + })) + return Promise.resolve() + }, + }) + const settlement = transaction.isPersisted.promise.catch(() => undefined) + + try { + await collection.preload() + transaction.mutate(() => { + if (author) { + author(collection) + return + } + for (const [index, replacement] of replacements.entries()) { + collection.delete(1, { + metadata: { operation: `delete`, index }, + }) + collection.insert(replacement, { + metadata: { operation: `insert`, index }, + }) + } + }) + + await transaction.commit() + expect(request, `net delete then insert request`).toStrictEqual( + expected === undefined ? [] : [expected], + ) + expect(calls, `persistence runs only for a net mutation`).toBe( + expected === undefined ? 0 : 1, + ) + expect( + [...collection.values()].map(userRow), + `manual settlement releases the optimistic replacement`, + ).toStrictEqual([original]) + return request + } finally { + if (transaction.state === `pending` || transaction.state === `persisting`) { + transaction.rollback() + } + await settlement + await collection.cleanup() + } +} + +describe(`Delete then insert transaction laws`, () => { + it(`cancels an exact same-key restoration`, async () => { + await runDeleteInsertReplacement( + [{ id: 1, value: 0, note: `original` }], + undefined, + ) + }) + + it.each([ + { + name: `removed field`, + replacement: { id: 1, value: 0 }, + changes: { note: undefined }, + }, + { + name: `null field`, + replacement: { id: 1, value: null, note: null }, + changes: { value: null, note: null }, + }, + { + name: `undefined field`, + replacement: { id: 1, value: undefined, note: `original` }, + changes: { value: undefined }, + }, + { + name: `added field`, + replacement: { id: 1, value: 0, note: `original`, added: `new` }, + changes: { added: `new` }, + }, + ] satisfies Array<{ + name: string + replacement: ReplacementRow + changes: object + }>)( + `converts a $name to one symmetric update`, + async ({ replacement, changes }) => { + await runDeleteInsertReplacement([replacement], { + type: `update`, + original: { id: 1, value: 0, note: `original` }, + modified: replacement, + changes, + metadata: { operation: `insert`, index: 0 }, + syncMetadata: { + delete: true, + shared: `insert`, + insert: true, + }, + }) + }, + ) + + it(`reduces repeated delete and insert pairs to the final net update`, async () => { + const first = { id: 1, value: 1, note: `first` } + const final = { id: 1, value: 2, added: `final` } + await runDeleteInsertReplacement([first, final], { + type: `update`, + original: { id: 1, value: 0, note: `original` }, + modified: final, + changes: { value: 2, note: undefined, added: `final` }, + metadata: { operation: `insert`, index: 1 }, + syncMetadata: { + delete: true, + shared: `insert`, + insert: true, + }, + }) + }) + + it(`preserves the first preimage through duplicate deletes before replacement`, async () => { + const original = { id: 1, value: 0, note: `original` } + const replacement = { id: 1, value: 2, note: `original` } + await runDeleteInsertReplacement( + [], + { + type: `update`, + original, + modified: replacement, + changes: { value: 2 }, + metadata: { operation: `insert` }, + syncMetadata: { + delete: true, + shared: `insert`, + insert: true, + }, + }, + original, + (collection) => { + collection.update(1, (draft) => { + draft.value = 1 + }) + collection.delete([1, 1]) + collection.insert(replacement, { + metadata: { operation: `insert` }, + }) + }, + ) + }) + + it(`compares nested replacement values structurally`, async () => { + const original = { + id: 1, + value: 0, + note: `original`, + nested: { count: 1 }, + } + await runDeleteInsertReplacement( + [{ ...original, nested: { count: 1 } }], + undefined, + original, + ) + await runDeleteInsertReplacement( + [{ ...original, nested: { count: 2 } }], + { + type: `update`, + original, + modified: { ...original, nested: { count: 2 } }, + changes: { nested: { count: 2 } }, + metadata: { operation: `insert`, index: 0 }, + syncMetadata: { + delete: true, + shared: `insert`, + insert: true, + }, + }, + original, + ) + }) + + it(`delivers a replacement that differs only by an enumerable symbol`, async () => { + const field = Symbol(`replacement field`) + const original = { id: 1, value: 0, [field]: 0 } + const replacement = { id: 1, value: 0, [field]: 1 } + const request = await runDeleteInsertReplacement( + [replacement], + { + type: `update`, + original, + modified: replacement, + changes: {}, + metadata: { operation: `insert`, index: 0 }, + syncMetadata: { + delete: true, + shared: `insert`, + insert: true, + }, + }, + original, + ) + + expect(Reflect.ownKeys(request[0]!.changes)).toStrictEqual([]) + expect(Reflect.get(request[0]!.modified, field)).toBe(1) + }) + + it(`uses own-string key order for a symmetric replacement diff`, async () => { + const original = { + 10: `remove numeric`, + 2: `stable numeric`, + id: 1, + value: 0, + before: `remove string`, + stable: `same`, + } + const replacement = { + 1: `add numeric`, + 2: `stable numeric`, + id: 1, + value: 0, + stable: `same`, + after: `add string`, + } + const request = await runDeleteInsertReplacement( + [replacement], + { + type: `update`, + original, + modified: replacement, + changes: { + 1: `add numeric`, + 10: undefined, + before: undefined, + after: `add string`, + }, + metadata: { operation: `insert`, index: 0 }, + syncMetadata: { + delete: true, + shared: `insert`, + insert: true, + }, + }, + original, + ) + + expect(Object.keys(request[0]!.changes)).toStrictEqual([ + `1`, + `10`, + `before`, + `after`, + ]) + }) + + it(`delivers a replacement after duplicate deletes of another overlay`, async () => { + const original = { id: 1, value: 0, note: `original` } + const overlaid = { id: 1, value: 1, note: `original` } + const collection = createCollection({ + getKey: (row) => row.id, + sync: { + sync: (actions) => { + actions.begin() + actions.write({ type: `insert`, value: original }) + actions.commit() + actions.markReady() + }, + }, + }) + let rejectOverlay!: (error: Error) => void + const overlayGate = new Promise((_resolve, reject) => { + rejectOverlay = reject + }) + const overlay = createTransaction({ + autoCommit: false, + mutationFn: () => overlayGate, + }) + let deliveries = 0 + let delivered: ReplacementRow | undefined + const replacement = createTransaction({ + autoCommit: false, + mutationFn: ({ transaction }) => { + deliveries++ + delivered = transaction.mutations[0].modified + return Promise.resolve() + }, + }) + const overlaySettlement = overlay.isPersisted.promise.catch(() => undefined) + const replacementSettlement = replacement.isPersisted.promise.catch( + () => undefined, + ) + + try { + await collection.preload() + overlay.mutate(() => + collection.update(1, (draft) => { + draft.value = 1 + }), + ) + const overlayCommit = overlay.commit().catch(() => undefined) + await Promise.resolve() + + replacement.mutate(() => { + collection.delete([1, 1]) + collection.insert(overlaid) + }) + await replacement.commit() + + expect(deliveries, `replacement persistence delivery count`).toBe(1) + expect(delivered, `replacement server payload`).toStrictEqual(overlaid) + + rejectOverlay(new Error(`earlier overlay failed`)) + await overlayCommit + } finally { + if (overlay.state === `pending` || overlay.state === `persisting`) + overlay.rollback() + if (replacement.state === `pending` || replacement.state === `persisting`) + replacement.rollback() + await Promise.all([overlaySettlement, replacementSettlement]) + await collection.cleanup() + } + }) + + it(`delivers a replacement equal only to another pending insert`, async () => { + const inserted = { id: 1, value: 1, note: `inserted` } + const collection = createCollection({ + getKey: (row) => row.id, + sync: { + sync: ({ markReady }) => markReady(), + }, + }) + let rejectInsert!: (error: Error) => void + const insertGate = new Promise((_resolve, reject) => { + rejectInsert = reject + }) + const pendingInsert = createTransaction({ + autoCommit: false, + mutationFn: () => insertGate, + }) + let deliveries = 0 + let delivered: { type: string; modified: ReplacementRow } | undefined + const replacement = createTransaction({ + autoCommit: false, + mutationFn: ({ transaction }) => { + deliveries++ + const mutation = transaction.mutations[0] + delivered = { + type: mutation.type, + modified: mutation.modified, + } + return Promise.resolve() + }, + }) + const insertSettlement = pendingInsert.isPersisted.promise.catch( + () => undefined, + ) + const replacementSettlement = replacement.isPersisted.promise.catch( + () => undefined, + ) + + try { + await collection.preload() + pendingInsert.mutate(() => collection.insert(inserted)) + const insertCommit = pendingInsert.commit().catch(() => undefined) + await Promise.resolve() + + replacement.mutate(() => { + collection.delete(1) + collection.insert(inserted) + }) + await replacement.commit() + + expect(deliveries, `replacement persistence delivery count`).toBe(1) + expect(delivered, `replacement server mutation`).toStrictEqual({ + type: `insert`, + modified: inserted, + }) + + rejectInsert(new Error(`earlier insert failed`)) + await insertCommit + } finally { + if ( + pendingInsert.state === `pending` || + pendingInsert.state === `persisting` + ) + pendingInsert.rollback() + if (replacement.state === `pending` || replacement.state === `persisting`) + replacement.rollback() + await Promise.all([insertSettlement, replacementSettlement]) + await collection.cleanup() + } + }) +}) + // Observe user fields without discarding unexpected fields or undefined keys. -function userRow(value: Row): Row { - const copy: Record = { ...value } +function userRow(value: T): T { + const copy = { ...value } as Record for (const field of [`$synced`, `$origin`, `$key`, `$collectionId`]) delete copy[field] - return copy as Row + return copy as T } const ordered = (rows: Iterable) => [...rows].map(userRow).sort((a, b) => a.id - b.id)