diff --git a/packages/bindx/src/persistence/BatchPersister.ts b/packages/bindx/src/persistence/BatchPersister.ts index d2542ab..56b04f5 100644 --- a/packages/bindx/src/persistence/BatchPersister.ts +++ b/packages/bindx/src/persistence/BatchPersister.ts @@ -598,6 +598,11 @@ export class BatchPersister { for (const entity of entities) { let data: Record | null = null + // The parent's update carries this row's nested `delete`, so a standalone + // update would target a row that no longer exists (issue #91). The entity + // stays in the dirty set so a persisting interceptor can still veto it. + if (entity.changeType === 'update' && this.store.isPlannedForDeleteByParent(entity.entityId)) continue + if (entity.changeType === 'delete') { mutations.push({ entityType: entity.entityType, @@ -1186,9 +1191,39 @@ export class BatchPersister { ) if (outcome === 'conflict') addConflict(ownerKey, relationConflictMessage(change.entityType, change.entityId, change.fieldName)) } + this.purgeChildrenDeletedByParent(execution, keys) return conflicts } + /** + * Drops the entities whose rows a confirmed parent mutation deleted through a + * nested `delete`, the way a top-level delete already does — otherwise the + * snapshot lingers and the next save writes a row that is gone (issue #91). + */ + private purgeChildrenDeletedByParent(execution: PersistExecution, ownerKeys: ReadonlySet): void { + const targets = targetEntityTypesByRelation(execution) + const deleted = new Set() + const collect = (entityType: string, entityId: string, fieldName: string, childId: string): void => { + const targetType = targets.get(relationIdentityKey(entityType, entityId, fieldName)) + if (targetType) deleted.add(entityIdentityKey(targetType, childId)) + } + for (const change of execution.hasOneChanges) { + if (!ownerKeys.has(entityIdentityKey(change.entityType, change.entityId))) continue + if (change.transition.operation !== 'delete') continue + collect(change.entityType, change.entityId, change.fieldName, change.transition.targetId) + } + for (const change of execution.hasManyChanges) { + if (!ownerKeys.has(entityIdentityKey(change.entityType, change.entityId))) continue + for (const removal of change.removals) { + if (removal.type === 'delete') collect(change.entityType, change.entityId, change.fieldName, removal.itemId) + } + } + for (const key of deleted) { + const child = splitExecutionKey(key) + this.store.removeEntity(child.entityType, child.entityId) + } + } + private mapConfirmedIds( entity: ExecutionEntity, persistedId: string | undefined, @@ -1565,3 +1600,16 @@ function splitExecutionKey(key: string): { entityType: string; entityId: string entityId: separator < 0 ? key : key.slice(separator + 1), } } + +function relationIdentityKey(entityType: string, entityId: string, fieldName: string): string { + return `${entityIdentityKey(entityType, entityId)}:${fieldName}` +} + +/** Relation key → the entity type its children have, the only source of a nested child's type. */ +function targetEntityTypesByRelation(execution: PersistExecution): ReadonlyMap { + const targets = new Map() + for (const field of execution.relationFields) { + targets.set(relationIdentityKey(field.entityType, field.entityId, field.fieldName), field.targetEntityType) + } + return targets +} diff --git a/packages/bindx/src/store/HasManyStore.ts b/packages/bindx/src/store/HasManyStore.ts index 9058029..73dde2d 100644 --- a/packages/bindx/src/store/HasManyStore.ts +++ b/packages/bindx/src/store/HasManyStore.ts @@ -1,4 +1,5 @@ import { parentKeyFromOwnerPrefix, parentKeyFromRelationKey } from './relationKey.js' +import { PlannedDeleteIndex } from './PlannedDeleteIndex.js' import { RelationEdgeIndex } from './RelationEdgeIndex.js' type ReconciliationResult = 'applied' | 'conflict' @@ -117,6 +118,12 @@ export class HasManyStore { */ private readonly edges = new RelationEdgeIndex() + /** + * Children some relation plans to remove with `delete`, maintained by + * {@link writeHasMany} / {@link deleteHasMany}, like {@link edges}. + */ + private readonly plannedDeletes = new PlannedDeleteIndex() + private mutationVersion = 0 /** @@ -143,12 +150,14 @@ export class HasManyStore { * keeps the index correct without tracking the reverse direction itself. */ private writeHasMany(key: string, state: StoredHasManyState): void { - const oldLive = liveHasManyChildIds(this.hasManyStates.get(key)) + const previous = this.hasManyStates.get(key) + const oldLive = liveHasManyChildIds(previous) const newLive = liveHasManyChildIds(state) this.hasManyStates.set(key, state) const parentKey = parentKeyFromRelationKey(key) for (const id of newLive) if (!oldLive.has(id)) this.edges.addEdge(parentKey, id) for (const id of oldLive) if (!newLive.has(id)) this.edges.removeEdge(parentKey, id) + this.reconcilePlannedDeletes(plannedDeleteChildIds(previous), plannedDeleteChildIds(state)) this.mutationVersion++ } @@ -162,9 +171,21 @@ export class HasManyStore { const parentKey = parentKeyFromRelationKey(key) for (const id of liveHasManyChildIds(existing)) this.edges.removeEdge(parentKey, id) this.hasManyStates.delete(key) + this.reconcilePlannedDeletes(plannedDeleteChildIds(existing), new Set()) this.mutationVersion++ } + /** Applies one relation's planned-delete diff to the refcounted index. */ + private reconcilePlannedDeletes(previous: ReadonlySet, next: ReadonlySet): void { + for (const id of next) if (!previous.has(id)) this.plannedDeletes.retain(id) + for (const id of previous) if (!next.has(id)) this.plannedDeletes.release(id) + } + + /** Whether any has-many relation plans to remove {@link childId} with `delete`. */ + isPlannedForDelete(childId: string): boolean { + return this.plannedDeletes.has(childId) + } + /** * Gets or creates has-many list state. */ @@ -733,6 +754,7 @@ export class HasManyStore { clear(): void { this.hasManyStates.clear() this.edges.clear() + this.plannedDeletes.clear() this.mutationVersion++ } } @@ -754,6 +776,15 @@ function liveHasManyChildIds(state: StoredHasManyState | undefined): Set return live } +function plannedDeleteChildIds(state: StoredHasManyState | undefined): Set { + const ids = new Set() + if (!state) return ids + for (const [id, type] of state.plannedRemovals) { + if (type === 'delete') ids.add(id) + } + return ids +} + interface HasManyReconciliation { state: StoredHasManyState result: ReconciliationResult diff --git a/packages/bindx/src/store/HasOneStore.ts b/packages/bindx/src/store/HasOneStore.ts index f9cb94c..8625d46 100644 --- a/packages/bindx/src/store/HasOneStore.ts +++ b/packages/bindx/src/store/HasOneStore.ts @@ -1,6 +1,7 @@ import type { HasOneRelationState } from '../handles/types.js' import type { EntitySnapshot } from './snapshots.js' import { parentKeyFromOwnerPrefix, parentKeyFromRelationKey } from './relationKey.js' +import { PlannedDeleteIndex } from './PlannedDeleteIndex.js' import { RelationEdgeIndex } from './RelationEdgeIndex.js' /** @@ -48,6 +49,12 @@ export class HasOneStore { */ private readonly edges = new RelationEdgeIndex() + /** + * Targets this store plans to delete through their parent's mutation, maintained + * by {@link writeRelation} / {@link deleteRelation}, like {@link edges}. + */ + private readonly plannedDeletes = new PlannedDeleteIndex() + private mutationVersion = 0 /** @@ -73,7 +80,8 @@ export class HasOneStore { * tracking the reverse direction itself. */ private writeRelation(key: string, state: StoredRelationState): void { - const oldChild = liveHasOneChildId(this.relationStates.get(key)) + const previous = this.relationStates.get(key) + const oldChild = liveHasOneChildId(previous) const newChild = liveHasOneChildId(state) this.relationStates.set(key, state) if (oldChild !== newChild) { @@ -81,6 +89,7 @@ export class HasOneStore { if (oldChild !== null) this.edges.removeEdge(parentKey, oldChild) if (newChild !== null) this.edges.addEdge(parentKey, newChild) } + this.reconcilePlannedDelete(plannedDeleteChildId(previous), plannedDeleteChildId(state)) this.mutationVersion++ } @@ -94,9 +103,22 @@ export class HasOneStore { const child = liveHasOneChildId(existing) if (child !== null) this.edges.removeEdge(parentKeyFromRelationKey(key), child) this.relationStates.delete(key) + this.reconcilePlannedDelete(plannedDeleteChildId(existing), null) this.mutationVersion++ } + /** Applies one relation's planned-delete diff to the refcounted index. */ + private reconcilePlannedDelete(previous: string | null, next: string | null): void { + if (previous === next) return + if (previous !== null) this.plannedDeletes.release(previous) + if (next !== null) this.plannedDeletes.retain(next) + } + + /** Whether this store plans to delete {@link childId} through its parent. */ + isPlannedForDelete(childId: string): boolean { + return this.plannedDeletes.has(childId) + } + /** * Gets or creates relation state. */ @@ -376,6 +398,7 @@ export class HasOneStore { clear(): void { this.relationStates.clear() this.edges.clear() + this.plannedDeletes.clear() this.mutationVersion++ } } @@ -390,6 +413,16 @@ function liveHasOneChildId(state: StoredRelationState | undefined): string | nul return state.currentId !== null && state.state !== 'deleted' ? state.currentId : null } +/** + * The id the parent's `{ delete: true }` removes — `serverId`, not `currentId`, + * because that is the row MutationCollector deletes for a `deleted` relation, and a + * null serverId emits no delete at all. + */ +function plannedDeleteChildId(state: StoredRelationState | undefined): string | null { + if (!state || state.state !== 'deleted') return null + return state.serverId +} + interface HasOneReconciliation { state: StoredRelationState result: ReconciliationResult diff --git a/packages/bindx/src/store/PlannedDeleteIndex.ts b/packages/bindx/src/store/PlannedDeleteIndex.ts new file mode 100644 index 0000000..b199e3e --- /dev/null +++ b/packages/bindx/src/store/PlannedDeleteIndex.ts @@ -0,0 +1,33 @@ +/** + * Refcounted multiset of entity ids whose row a parent relation plans to delete + * through its own mutation ({@link HasManyStore} `plannedRemovals` of type `delete`, + * {@link HasOneStore} state `deleted`). + * + * Refcounted because more than one relation may plan the same child's deletion, so + * the id stays planned until the last of them drops it. Like {@link RelationEdgeIndex}, + * the index knows nothing about the rules: turning relation state into a planned-delete + * id is the owning sub-store's job, done once per write in its chokepoint by diffing + * the previous state against the next. + */ +export class PlannedDeleteIndex { + private readonly counts = new Map() + + retain(childId: string): void { + this.counts.set(childId, (this.counts.get(childId) ?? 0) + 1) + } + + release(childId: string): void { + const count = this.counts.get(childId) + if (count === undefined) return + if (count > 1) this.counts.set(childId, count - 1) + else this.counts.delete(childId) + } + + has(childId: string): boolean { + return this.counts.has(childId) + } + + clear(): void { + this.counts.clear() + } +} diff --git a/packages/bindx/src/store/RelationStore.ts b/packages/bindx/src/store/RelationStore.ts index 9864d7b..6dde4e4 100644 --- a/packages/bindx/src/store/RelationStore.ts +++ b/packages/bindx/src/store/RelationStore.ts @@ -158,6 +158,16 @@ export class RelationStore implements Rekeyable { return this.hasMany.getHasManyOrderedIds(key) } + /** + * Whether some relation plans to delete {@link childId} through its parent's + * mutation — a has-many `delete` removal or a has-one target marked `deleted`. + * Such a row goes away with the parent's update, so the child must not be + * written on its own (see issue #91). + */ + isPlannedForDeleteByParent(childId: string): boolean { + return this.hasOne.isPlannedForDelete(childId) || this.hasMany.isPlannedForDelete(childId) + } + // ==================== Reachability / Reverse Lookup ==================== /** diff --git a/packages/bindx/src/store/SnapshotStore.ts b/packages/bindx/src/store/SnapshotStore.ts index e7a1d18..118557b 100644 --- a/packages/bindx/src/store/SnapshotStore.ts +++ b/packages/bindx/src/store/SnapshotStore.ts @@ -577,6 +577,15 @@ export class SnapshotStore implements SnapshotVersionBumper, JournalTarget { return this.relations.getHasMany(key)?.plannedRemovals } + /** + * Whether some relation plans to delete {@link entityId} through its parent, i.e. + * the row goes away with the parent's update rather than with a mutation of its + * own (see issue #91). + */ + isPlannedForDeleteByParent(entityId: string): boolean { + return this.relations.isPlannedForDeleteByParent(entityId) + } + planHasManyConnection( parentType: string, parentId: string, diff --git a/tests/unit/persistence/deletedHasOneTargetStandaloneUpdate.test.ts b/tests/unit/persistence/deletedHasOneTargetStandaloneUpdate.test.ts new file mode 100644 index 0000000..e6918bf --- /dev/null +++ b/tests/unit/persistence/deletedHasOneTargetStandaloneUpdate.test.ts @@ -0,0 +1,132 @@ +// Regression test for https://github.com/contember/bindx/issues/91 — the has-one twin. +// +// A has-one target marked `deleted` is removed by the nested `{ delete: true }` in its +// parent's update, so a standalone `update` of that target would hit a row that no +// longer exists, exactly like the has-many case. +import { describe, test, expect, beforeEach, mock } from 'bun:test' +import { + SnapshotStore, + ActionDispatcher, + BatchPersister, + MutationCollector, + ContemberSchemaMutationAdapter, + type BackendAdapter, + type SchemaNames, +} from '@contember/bindx' + +const testSchema: SchemaNames = { + entities: { + Page: { + name: 'Page', + scalars: ['id', 'title'], + fields: { + id: { type: 'column' }, + title: { type: 'column' }, + cover: { type: 'one', entity: 'Block' }, + }, + }, + Block: { + name: 'Block', + scalars: ['id', 'title'], + fields: { + id: { type: 'column' }, + title: { type: 'column' }, + }, + }, + }, + enums: {}, +} + +interface PersistCall { + readonly entityType: string + readonly entityId: string + readonly changes: Record +} + +function createAdapter(calls: PersistCall[]): BackendAdapter { + return { + query: mock(() => Promise.resolve([])), + persist: mock((entityType: string, entityId: string, changes: Record) => { + calls.push({ entityType, entityId, changes }) + return Promise.resolve({ ok: true, data: { id: entityId } }) + }), + create: mock((_entityType: string, data: Record) => Promise.resolve({ ok: true, data: { id: 'created-1', ...data } })), + delete: mock(() => Promise.resolve({ ok: true })), + } +} + +describe('BatchPersister — has-one target marked deleted', () => { + let store: SnapshotStore + let dispatcher: ActionDispatcher + let calls: PersistCall[] + let persister: BatchPersister + + beforeEach(() => { + store = new SnapshotStore() + dispatcher = new ActionDispatcher(store) + calls = [] + persister = new BatchPersister(createAdapter(calls), store, dispatcher, { + mutationCollector: new MutationCollector(store, new ContemberSchemaMutationAdapter(testSchema)), + }) + + store.setEntityData('Page', 'page-1', { id: 'page-1', title: 'Page' }, true) + store.setEntityData('Block', 'block-1', { id: 'block-1', title: 'Block' }, true) + store.getOrCreateRelation('Page', 'page-1', 'cover', { + currentId: 'block-1', + serverId: 'block-1', + state: 'connected', + serverState: 'connected', + placeholderData: {}, + }) + }) + + test('should not emit a standalone update for a dirty target that is marked deleted', async () => { + store.setFieldValue('Block', 'block-1', ['title'], 'Edited') + store.setRelation('Page', 'page-1', 'cover', { state: 'deleted' }) + + const result = await persister.persistAll() + + expect(calls.filter(call => call.entityType === 'Block')).toEqual([]) + const pageCall = calls.find(call => call.entityType === 'Page') + expect(pageCall?.changes).toEqual({ cover: { delete: true } }) + expect(result.success).toBe(true) + }) + + test('should drop a target deleted through its parent from the store', async () => { + store.setFieldValue('Block', 'block-1', ['title'], 'Edited') + store.setRelation('Page', 'page-1', 'cover', { state: 'deleted' }) + + const result = await persister.persistAll() + + expect(result.success).toBe(true) + expect(store.getEntitySnapshot('Block', 'block-1')).toBeUndefined() + expect(store.getAllDirtyEntities()).toEqual([]) + }) + + test('should keep the standalone update of a target that is only disconnected', async () => { + store.setFieldValue('Block', 'block-1', ['title'], 'Edited') + store.setRelation('Page', 'page-1', 'cover', { state: 'disconnected', currentId: null }) + + const result = await persister.persistAll() + + // The row survives a disconnect, so its scalar edit still has to be written. + const blockCall = calls.find(call => call.entityType === 'Block') + expect(blockCall?.changes).toEqual({ title: 'Edited' }) + expect(store.getEntitySnapshot('Block', 'block-1')).toBeDefined() + expect(result.success).toBe(true) + }) + + test('should keep the target vetoable and suppress the parent-side delete', async () => { + store.setFieldValue('Block', 'block-1', ['title'], 'Edited') + store.setRelation('Page', 'page-1', 'cover', { state: 'deleted' }) + dispatcher.getEventEmitter().interceptEntity('entity:persisting', 'Block', 'block-1', () => ({ + action: 'cancel', + })) + + const result = await persister.persistAll() + + expect(result.skippedCount).toBe(1) + expect(calls).toEqual([]) + expect(store.getEntitySnapshot('Block', 'block-1')).toBeDefined() + }) +}) diff --git a/tests/unit/persistence/removedItemStandaloneUpdate.test.ts b/tests/unit/persistence/removedItemStandaloneUpdate.test.ts new file mode 100644 index 0000000..c464cd3 --- /dev/null +++ b/tests/unit/persistence/removedItemStandaloneUpdate.test.ts @@ -0,0 +1,208 @@ +// Regression test for https://github.com/contember/bindx/issues/91 +// +// An item that is dirty AND planned for removal (`delete`) from its parent's has-many +// must not get its own top-level `update` mutation: the parent's update carries the +// nested `delete`, so the standalone update targets a row that no longer exists. +// With the default sequential adapter (no `persistTransaction`, like ContemberAdapter) +// the parent runs first and the update then fails with NotFoundOrDenied. +import { describe, test, expect, beforeEach, mock } from 'bun:test' +import { + SnapshotStore, + ActionDispatcher, + BatchPersister, + MutationCollector, + ContemberSchemaMutationAdapter, + type BackendAdapter, + type SchemaNames, +} from '@contember/bindx' + +const testSchema: SchemaNames = { + entities: { + Article: { + name: 'Article', + scalars: ['id', 'title'], + fields: { + id: { type: 'column' }, + title: { type: 'column' }, + tags: { type: 'many', entity: 'Tag' }, + }, + }, + Tag: { + name: 'Tag', + scalars: ['id', 'name', 'order'], + fields: { + id: { type: 'column' }, + name: { type: 'column' }, + order: { type: 'column' }, + }, + }, + }, + enums: {}, +} + +type Call = { operation: 'update' | 'delete'; entityType: string; id: string; data?: Record } + +// Sequential adapter without `persistTransaction` — mirrors ContemberAdapter, which sends one +// request per mutation. The "server" tracks which Tag rows still exist so that an update of a +// row deleted earlier in the same persist fails the way the real API does. +function createSequentialAdapter(existingTags: string[]) { + const calls: Call[] = [] + const tags = new Set(existingTags) + const adapter: BackendAdapter = { + query: mock(() => Promise.resolve([])), + persist: mock((entityType: string, id: string, changes: Record) => { + calls.push({ operation: 'update', entityType, id, data: changes }) + if (entityType === 'Tag' && !tags.has(id)) { + return Promise.resolve({ ok: false, errorMessage: `Execution has failed:\nunknown field: NotFoundOrDenied (for input {"id":"${id}"})` }) + } + if (entityType === 'Article') { + const items = (changes['tags'] as Array> | undefined) ?? [] + for (const op of items) { + const del = op['delete'] as { id: string } | undefined + if (del) tags.delete(del.id) + } + } + return Promise.resolve({ ok: true }) + }), + create: mock((entityType: string, data: Record) => Promise.resolve({ ok: true, data: { id: 'new-id', ...data } })), + delete: mock((entityType: string, id: string) => { + calls.push({ operation: 'delete', entityType, id }) + tags.delete(id) + return Promise.resolve({ ok: true }) + }), + } + return { adapter, calls } +} + +describe('BatchPersister — has-many item planned for delete', () => { + let store: SnapshotStore + let dispatcher: ActionDispatcher + + beforeEach(() => { + store = new SnapshotStore() + dispatcher = new ActionDispatcher(store) + }) + + test('should not emit a standalone update for a dirty item that is planned for delete', async () => { + const { adapter, calls } = createSequentialAdapter(['tag-1', 'tag-2', 'tag-3']) + const schemaAdapter = new ContemberSchemaMutationAdapter(testSchema) + const mutationCollector = new MutationCollector(store, schemaAdapter) + const persister = new BatchPersister(adapter, store, dispatcher, { mutationCollector }) + + // Server state: an article with three ordered tags. + store.setEntityData('Article', 'a-1', { + id: 'a-1', + title: 'Article', + tags: [{ id: 'tag-1' }, { id: 'tag-2' }, { id: 'tag-3' }], + }, true) + store.setEntityData('Tag', 'tag-1', { id: 'tag-1', name: 'One', order: 10 }, true) + store.setEntityData('Tag', 'tag-2', { id: 'tag-2', name: 'Two', order: 20 }, true) + store.setEntityData('Tag', 'tag-3', { id: 'tag-3', name: 'Three', order: 30 }, true) + store.setHasManyServerIds('Article', 'a-1', 'tags', ['tag-1', 'tag-2', 'tag-3']) + + // The sortable-repeater sequence: removing tag-1 renumbers the survivors (tag-2 and + // tag-3 become dirty), then tag-2 is removed as well — it is now dirty AND removed. + store.planHasManyRemoval('Article', 'a-1', 'tags', 'tag-1', 'delete') + store.setFieldValue('Tag', 'tag-2', ['order'], 0) + store.setFieldValue('Tag', 'tag-3', ['order'], 1) + store.planHasManyRemoval('Article', 'a-1', 'tags', 'tag-2', 'delete') + + const result = await persister.persistAll() + + const tag2Updates = calls.filter(c => c.operation === 'update' && c.entityType === 'Tag' && c.id === 'tag-2') + expect(tag2Updates).toEqual([]) + + const articleUpdate = calls.find(c => c.operation === 'update' && c.entityType === 'Article') + expect(articleUpdate).toBeDefined() + const tagOps = (articleUpdate!.data!['tags'] as Array>) + expect(tagOps).toContainEqual(expect.objectContaining({ delete: { id: 'tag-1' } })) + expect(tagOps).toContainEqual(expect.objectContaining({ delete: { id: 'tag-2' } })) + + // The surviving tag keeps its reorder, and the whole save succeeds. + const tag3Update = calls.find(c => c.operation === 'update' && c.entityType === 'Tag' && c.id === 'tag-3') + expect(tag3Update?.data).toEqual({ order: 1 }) + expect(result.success).toBe(true) + expect(result.failedCount).toBe(0) + }) + + // Without this the save succeeds once, the next `commitAllRelations` clears the + // planned removal, the item turns dirty again and the following save updates a + // row the server has already dropped. + test('should drop an item deleted through its parent from the store', async () => { + const { adapter } = createSequentialAdapter(['tag-1', 'tag-2', 'tag-3']) + const persister = createPersister(store, dispatcher, adapter) + seedArticleWithTags(store) + + store.setFieldValue('Tag', 'tag-2', ['order'], 0) + store.planHasManyRemoval('Article', 'a-1', 'tags', 'tag-2', 'delete') + + const result = await persister.persistAll() + + expect(result.success).toBe(true) + expect(store.getEntitySnapshot('Tag', 'tag-2')).toBeUndefined() + expect(store.getAllDirtyEntities()).toEqual([]) + }) + + test('should keep the standalone update of an item removed with disconnect', async () => { + const { adapter, calls } = createSequentialAdapter(['tag-1', 'tag-2', 'tag-3']) + const persister = createPersister(store, dispatcher, adapter) + seedArticleWithTags(store) + + store.setFieldValue('Tag', 'tag-2', ['order'], 0) + store.planHasManyRemoval('Article', 'a-1', 'tags', 'tag-2', 'disconnect') + + const result = await persister.persistAll() + + // The row survives a disconnect, so its scalar edit still has to be written. + const tag2Update = calls.find(c => c.operation === 'update' && c.entityType === 'Tag' && c.id === 'tag-2') + expect(tag2Update?.data).toEqual({ order: 0 }) + const articleUpdate = calls.find(c => c.operation === 'update' && c.entityType === 'Article') + expect(tagOperations(articleUpdate)).toContainEqual(expect.objectContaining({ disconnect: { id: 'tag-2' } })) + expect(store.getEntitySnapshot('Tag', 'tag-2')).toBeDefined() + expect(result.success).toBe(true) + }) + + test('should leave a created-then-removed item out of the persist', async () => { + const { adapter, calls } = createSequentialAdapter(['tag-1', 'tag-2', 'tag-3']) + const persister = createPersister(store, dispatcher, adapter) + seedArticleWithTags(store) + + const newId = store.createEntity('Tag', { name: 'Four', order: 40 }) + store.addToHasMany('Article', 'a-1', 'tags', newId) + store.removeFromHasMany('Article', 'a-1', 'tags', newId, 'delete') + + // removeFromHasMany cancels the addition instead of planning a removal, so a + // never-persisted item never reaches the planned-delete index. + expect(store.isPlannedForDeleteByParent(newId)).toBe(false) + + const result = await persister.persistAll() + + expect(calls.filter(c => c.entityType === 'Tag')).toEqual([]) + const articleUpdate = calls.find(c => c.operation === 'update' && c.entityType === 'Article') + expect(tagOperations(articleUpdate)).toEqual([]) + expect(result.success).toBe(true) + }) +}) + +function createPersister(store: SnapshotStore, dispatcher: ActionDispatcher, adapter: BackendAdapter): BatchPersister { + const mutationCollector = new MutationCollector(store, new ContemberSchemaMutationAdapter(testSchema)) + return new BatchPersister(adapter, store, dispatcher, { mutationCollector }) +} + +/** Server state: an article with three ordered tags. */ +function seedArticleWithTags(store: SnapshotStore): void { + store.setEntityData('Article', 'a-1', { + id: 'a-1', + title: 'Article', + tags: [{ id: 'tag-1' }, { id: 'tag-2' }, { id: 'tag-3' }], + }, true) + store.setEntityData('Tag', 'tag-1', { id: 'tag-1', name: 'One', order: 10 }, true) + store.setEntityData('Tag', 'tag-2', { id: 'tag-2', name: 'Two', order: 20 }, true) + store.setEntityData('Tag', 'tag-3', { id: 'tag-3', name: 'Three', order: 30 }, true) + store.setHasManyServerIds('Article', 'a-1', 'tags', ['tag-1', 'tag-2', 'tag-3']) +} + +function tagOperations(call: Call | undefined): unknown[] { + const tags = call?.data?.['tags'] + return Array.isArray(tags) ? tags : [] +}