From b433c80d3acc96a2069821aa362c35e5ffd7c65c Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 22 Jun 2026 18:33:33 +0200 Subject: [PATCH 01/19] perf(bindx): memoize reachability walk via sub-store mutation counters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit computeReachableCreated() is an O(E+R) walk run on every dirty check and every post-persist sweep, previously recomputed from scratch each time. Each sub-store the walk reads (entity snapshots, meta, relations, roots) now exposes a monotonic getMutationVersion() bumped only when graph-relevant state changes — the entity key set / id index, existsOnServer / isPersisting, the root set, and relation edges. Pure value edits (setFieldValue/updateFields/...) and the per-render no-op getOrCreate* calls do not bump any counter, so the hot path stays warm. ReachabilityAnalyzer caches its result keyed on the sum of those counters. The sum is strictly increasing, so an unchanged sum proves nothing relevant changed and the cached set is returned without re-walking. All RelationStore map writes are routed through writeRelation/writeHasMany helpers so the bump cannot be missed; EntitySnapshotStore and EntityMetaStore bump selectively to avoid invalidating on value-only edits; RootRegistry bumps only on an actual change so the per-render registerParentChild -> unregister no-op does not thrash the cache. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- packages/bindx/src/store/EntityMetaStore.ts | 39 +++++++-- .../bindx/src/store/EntitySnapshotStore.ts | 18 ++++ .../bindx/src/store/ReachabilityAnalyzer.ts | 37 ++++++++ packages/bindx/src/store/RelationStore.ts | 84 ++++++++++++------- packages/bindx/src/store/RootRegistry.ts | 24 +++++- 5 files changed, 167 insertions(+), 35 deletions(-) diff --git a/packages/bindx/src/store/EntityMetaStore.ts b/packages/bindx/src/store/EntityMetaStore.ts index f2de0bf..297d535 100644 --- a/packages/bindx/src/store/EntityMetaStore.ts +++ b/packages/bindx/src/store/EntityMetaStore.ts @@ -39,6 +39,18 @@ export class EntityMetaStore { /** Mapping from temp ID to persisted ID (keyed by "entityType:tempId") */ private readonly tempToPersistedId = new Map() + /** + * Monotonic counter bumped when reachability-relevant metadata changes — + * `existsOnServer` and `isPersisting` (which seed the reachability roots) plus + * entity removal/rekey. Load state and deletion scheduling do NOT bump it. + * Used by {@link ReachabilityAnalyzer} to memoize its walk. + */ + private mutationVersion = 0 + + getMutationVersion(): number { + return this.mutationVersion + } + // ==================== Load State ==================== getLoadState(key: string): EntityLoadState | undefined { @@ -60,8 +72,15 @@ export class EntityMetaStore { } setExistsOnServer(key: string, existsOnServer: boolean): void { - const existing = this.entityMetas.get(key) ?? { existsOnServer: false, isScheduledForDeletion: false } - this.entityMetas.set(key, { ...existing, existsOnServer }) + const existing = this.entityMetas.get(key) + if (existing && existing.existsOnServer === existsOnServer) { + return + } + this.entityMetas.set(key, { + existsOnServer, + isScheduledForDeletion: existing?.isScheduledForDeletion ?? false, + }) + this.mutationVersion++ } existsOnServer(key: string): boolean { @@ -90,9 +109,12 @@ export class EntityMetaStore { setPersisting(key: string, isPersisting: boolean): void { if (isPersisting) { - this.persistingEntities.add(key) - } else { - this.persistingEntities.delete(key) + if (!this.persistingEntities.has(key)) { + this.persistingEntities.add(key) + this.mutationVersion++ + } + } else if (this.persistingEntities.delete(key)) { + this.mutationVersion++ } } @@ -123,6 +145,7 @@ export class EntityMetaStore { this.entityMetas.delete(key) this.persistingEntities.delete(key) this.tempToPersistedId.delete(key) + this.mutationVersion++ } /** @@ -152,6 +175,8 @@ export class EntityMetaStore { this.tempToPersistedId.delete(oldKey) this.tempToPersistedId.set(newKey, persistedId) } + + this.mutationVersion++ } // ==================== Bulk Operations ==================== @@ -168,9 +193,12 @@ export class EntityMetaStore { } importMetas(metas: Map): void { + let imported = false for (const [key, meta] of metas) { this.entityMetas.set(key, { ...meta }) + imported = true } + if (imported) this.mutationVersion++ } clear(): void { @@ -178,5 +206,6 @@ export class EntityMetaStore { this.entityMetas.clear() this.persistingEntities.clear() this.tempToPersistedId.clear() + this.mutationVersion++ } } diff --git a/packages/bindx/src/store/EntitySnapshotStore.ts b/packages/bindx/src/store/EntitySnapshotStore.ts index 4d0fc36..3292e5f 100644 --- a/packages/bindx/src/store/EntitySnapshotStore.ts +++ b/packages/bindx/src/store/EntitySnapshotStore.ts @@ -23,6 +23,19 @@ export class EntitySnapshotStore { */ private readonly idIndex = new Map() + /** + * Monotonic counter bumped only when the set of keys or the id→key index + * changes (new key, removal, rekey, bulk import, clear). Pure value edits + * (setFieldValue/updateFields/commit/reset/...) do NOT bump it — they cannot + * change reachability — so the per-keystroke edit path keeps the + * {@link ReachabilityAnalyzer} cache warm. + */ + private mutationVersion = 0 + + getMutationVersion(): number { + return this.mutationVersion + } + get(key: string): EntitySnapshot | undefined { return this.snapshots.get(key) } @@ -62,6 +75,7 @@ export class EntitySnapshotStore { this.snapshots.set(key, newSnapshot) this.idIndex.set(id, key) + if (!existing) this.mutationVersion++ return newSnapshot } @@ -208,6 +222,7 @@ export class EntitySnapshotStore { // that survivor as id-unresolvable. if (existing && this.idIndex.get(existing.id) === key) { this.idIndex.delete(existing.id) + this.mutationVersion++ } this.snapshots.delete(key) } @@ -284,6 +299,7 @@ export class EntitySnapshotStore { this.idIndex.set(snapshot.id, key) keys.add(key) } + if (keys.size > 0) this.mutationVersion++ return keys } @@ -310,6 +326,7 @@ export class EntitySnapshotStore { this.snapshots.set(newKey, newSnapshot) this.idIndex.delete(snapshot.id) this.idIndex.set(newId, newKey) + this.mutationVersion++ } keys(): IterableIterator { @@ -329,6 +346,7 @@ export class EntitySnapshotStore { clear(): void { this.snapshots.clear() this.idIndex.clear() + this.mutationVersion++ } } diff --git a/packages/bindx/src/store/ReachabilityAnalyzer.ts b/packages/bindx/src/store/ReachabilityAnalyzer.ts index 10e211d..5ed9460 100644 --- a/packages/bindx/src/store/ReachabilityAnalyzer.ts +++ b/packages/bindx/src/store/ReachabilityAnalyzer.ts @@ -18,6 +18,14 @@ import type { RootRegistry } from './RootRegistry.js' * The graph is read from {@link RelationStore} (the source of truth for live * relation membership), never from the subscription parent-child registry, * which is append-mostly for notifications and does not reflect disconnects. + * + * The walk is O(E+R) and runs on every dirty check and post-persist sweep, so + * its result is memoized. Each sub-store the walk reads exposes a monotonic + * `getMutationVersion()` that bumps only when graph-relevant state changes + * (entity key set, `existsOnServer`/`isPersisting`, roots, relation edges). + * Their sum is a cache key: it is strictly increasing, so an unchanged sum + * proves nothing relevant changed and the cached set can be returned. Pure + * field edits do not bump any counter, keeping the cache warm on the hot path. */ export class ReachabilityAnalyzer { constructor( @@ -27,11 +35,40 @@ export class ReachabilityAnalyzer { private readonly roots: RootRegistry, ) {} + private cache: { version: number; result: Set } | null = null + /** * Returns the set of entity keys ("entityType:id") for created * (never-persisted) entities reachable from a root through live relations. + * + * The returned set is the cached instance and is owned by the analyzer — + * callers must treat it as read-only (the current consumers only call + * `.has(...)` on it). */ computeReachableCreated(): Set { + const version = this.graphVersion() + const cached = this.cache + if (cached !== null && cached.version === version) { + return cached.result + } + + const result = this.walk() + this.cache = { version, result } + return result + } + + /** + * Sum of the graph-relevant mutation counters across the sub-stores the walk + * reads. Monotonic, so an unchanged value means no relevant mutation happened. + */ + private graphVersion(): number { + return this.entitySnapshots.getMutationVersion() + + this.meta.getMutationVersion() + + this.relations.getMutationVersion() + + this.roots.getMutationVersion() + } + + private walk(): Set { const reachableCreated = new Set() const visited = new Set() const stack: string[] = [] diff --git a/packages/bindx/src/store/RelationStore.ts b/packages/bindx/src/store/RelationStore.ts index 617b21b..24ff870 100644 --- a/packages/bindx/src/store/RelationStore.ts +++ b/packages/bindx/src/store/RelationStore.ts @@ -56,6 +56,29 @@ export class RelationStore { /** Has-many list states keyed by "parentType:parentId:fieldName" */ private readonly hasManyStates = new Map() + /** + * Monotonic counter bumped on every actual write to relation/has-many state. + * Used by {@link ReachabilityAnalyzer} to memoize its walk. All writes funnel + * through {@link writeRelation}/{@link writeHasMany} (plus the delete/clear + * paths), so a read-only `getOrCreate*` call on the per-render materialize + * path — which does not write when the entry already matches — never bumps it. + */ + private mutationVersion = 0 + + getMutationVersion(): number { + return this.mutationVersion + } + + private writeRelation(key: string, state: StoredRelationState): void { + this.relationStates.set(key, state) + this.mutationVersion++ + } + + private writeHasMany(key: string, state: StoredHasManyState): void { + this.hasManyStates.set(key, state) + this.mutationVersion++ + } + // ==================== Has-One Relations ==================== /** @@ -66,7 +89,7 @@ export class RelationStore { initial: Omit, ): StoredRelationState { if (!this.relationStates.has(key)) { - this.relationStates.set(key, { ...initial, version: 0 }) + this.writeRelation(key, { ...initial, version: 0 }) } return this.relationStates.get(key)! @@ -103,7 +126,7 @@ export class RelationStore { } } - this.relationStates.set(key, { + this.writeRelation(key, { currentId: 'currentId' in updates ? updates.currentId! : serverId, serverId, state: 'state' in updates ? updates.state! : serverState, @@ -112,7 +135,7 @@ export class RelationStore { version: 0, }) } else { - this.relationStates.set(key, { + this.writeRelation(key, { ...existing, ...updates, version: existing.version + 1, @@ -127,7 +150,7 @@ export class RelationStore { const existing = this.relationStates.get(key) if (!existing) return - this.relationStates.set(key, { + this.writeRelation(key, { ...existing, serverId: existing.currentId, serverState: existing.state === 'creating' ? 'connected' : existing.state, @@ -143,7 +166,7 @@ export class RelationStore { const existing = this.relationStates.get(key) if (!existing) return - this.relationStates.set(key, { + this.writeRelation(key, { ...existing, currentId: existing.serverId, state: existing.serverState, @@ -160,7 +183,7 @@ export class RelationStore { getOrCreateHasMany(key: string, serverIds?: string[]): StoredHasManyState { const existing = this.hasManyStates.get(key) if (!existing) { - this.hasManyStates.set(key, { + this.writeHasMany(key, { serverIds: new Set(serverIds ?? []), orderedIds: null, plannedRemovals: new Map(), @@ -171,7 +194,7 @@ export class RelationStore { } else if (serverIds !== undefined) { const newServerIds = new Set(serverIds) if (!setsEqual(existing.serverIds, newServerIds)) { - this.hasManyStates.set(key, { + this.writeHasMany(key, { ...existing, serverIds: newServerIds, orderedIds: null, @@ -197,7 +220,7 @@ export class RelationStore { const existing = this.hasManyStates.get(key) if (!existing) { - this.hasManyStates.set(key, { + this.writeHasMany(key, { serverIds: new Set(serverIds), orderedIds: null, plannedRemovals: new Map(), @@ -206,7 +229,7 @@ export class RelationStore { version: 0, }) } else { - this.hasManyStates.set(key, { + this.writeHasMany(key, { ...existing, serverIds: new Set(serverIds), orderedIds: null, @@ -222,7 +245,7 @@ export class RelationStore { const existing = this.hasManyStates.get(key) if (!existing) { - this.hasManyStates.set(key, { + this.writeHasMany(key, { serverIds: new Set(), orderedIds: null, plannedRemovals: new Map([[itemId, type]]), @@ -241,7 +264,7 @@ export class RelationStore { } const newCreatedEntities = new Set(existing.createdEntities) newCreatedEntities.delete(itemId) - this.hasManyStates.set(key, { + this.writeHasMany(key, { ...existing, orderedIds: newOrderedIds, plannedRemovals: newPlannedRemovals, @@ -259,7 +282,7 @@ export class RelationStore { const existing = this.hasManyStates.get(key) if (!existing) { - this.hasManyStates.set(key, { + this.writeHasMany(key, { serverIds: new Set(), orderedIds: null, plannedRemovals: new Map(), @@ -276,7 +299,7 @@ export class RelationStore { if (newOrderedIds !== null && !newOrderedIds.includes(itemId)) { newOrderedIds = [...newOrderedIds, itemId] } - this.hasManyStates.set(key, { + this.writeHasMany(key, { ...existing, orderedIds: newOrderedIds, plannedConnections: newPlannedConnections, @@ -292,7 +315,7 @@ export class RelationStore { commitHasMany(key: string, newServerIds: string[]): void { const existing = this.hasManyStates.get(key) - this.hasManyStates.set(key, { + this.writeHasMany(key, { serverIds: new Set(newServerIds), orderedIds: null, plannedRemovals: new Map(), @@ -309,7 +332,7 @@ export class RelationStore { const existing = this.hasManyStates.get(key) if (!existing) return - this.hasManyStates.set(key, { + this.writeHasMany(key, { serverIds: existing.serverIds, orderedIds: null, plannedRemovals: new Map(), @@ -327,7 +350,7 @@ export class RelationStore { const existing = this.hasManyStates.get(key) if (!existing) { - this.hasManyStates.set(key, { + this.writeHasMany(key, { serverIds: new Set(), orderedIds: [itemId], plannedRemovals: new Map(), @@ -342,7 +365,7 @@ export class RelationStore { newCreatedEntities.add(itemId) const currentOrderedIds = existing.orderedIds ?? computeDefaultOrderedIds(existing) const newOrderedIds = [...currentOrderedIds, itemId] - this.hasManyStates.set(key, { + this.writeHasMany(key, { ...existing, orderedIds: newOrderedIds, plannedConnections: newPlannedConnections, @@ -361,7 +384,7 @@ export class RelationStore { const existing = this.hasManyStates.get(key) if (!existing) { - this.hasManyStates.set(key, { + this.writeHasMany(key, { serverIds: new Set(), orderedIds: [itemId], plannedRemovals: new Map(), @@ -374,7 +397,7 @@ export class RelationStore { newPlannedConnections.add(itemId) const currentOrderedIds = existing.orderedIds ?? computeDefaultOrderedIds(existing) const newOrderedIds = [...currentOrderedIds, itemId] - this.hasManyStates.set(key, { + this.writeHasMany(key, { ...existing, orderedIds: newOrderedIds, plannedConnections: newPlannedConnections, @@ -425,7 +448,7 @@ export class RelationStore { } } - this.hasManyStates.set(key, newState) + this.writeHasMany(key, newState) return true } else { this.planHasManyRemoval(key, itemId, removalType) @@ -477,16 +500,20 @@ export class RelationStore { * stale relation state behind. */ removeOwnedRelations(keyPrefix: string): void { + let changed = false for (const key of [...this.relationStates.keys()]) { if (key.startsWith(keyPrefix)) { this.relationStates.delete(key) + changed = true } } for (const key of [...this.hasManyStates.keys()]) { if (key.startsWith(keyPrefix)) { this.hasManyStates.delete(key) + changed = true } } + if (changed) this.mutationVersion++ } /** @@ -507,7 +534,7 @@ export class RelationStore { if (movedItem === undefined) return newOrderedIds.splice(toIndex, 0, movedItem) - this.hasManyStates.set(key, { + this.writeHasMany(key, { ...existing, orderedIds: newOrderedIds, version: existing.version + 1, @@ -533,7 +560,7 @@ export class RelationStore { * Used in pessimistic mode after successful server confirmation. */ restoreHasManyState(key: string, state: StoredHasManyState): void { - this.hasManyStates.set(key, { + this.writeHasMany(key, { serverIds: new Set(state.serverIds), orderedIds: state.orderedIds ? [...state.orderedIds] : null, plannedRemovals: new Map(state.plannedRemovals), @@ -699,7 +726,7 @@ export class RelationStore { importRelationStates(states: Map): string[] { const keys: string[] = [] for (const [key, state] of states) { - this.relationStates.set(key, { + this.writeRelation(key, { ...state, placeholderData: { ...state.placeholderData }, }) @@ -715,7 +742,7 @@ export class RelationStore { importHasManyStates(states: Map): string[] { const keys: string[] = [] for (const [key, state] of states) { - this.hasManyStates.set(key, { + this.writeHasMany(key, { serverIds: new Set(state.serverIds), orderedIds: state.orderedIds ? [...state.orderedIds] : null, plannedRemovals: new Map(state.plannedRemovals), @@ -743,7 +770,7 @@ export class RelationStore { if (serverId === oldId) { serverId = newId; changed = true } if (changed) { - this.relationStates.set(key, { ...state, currentId, serverId, version: state.version + 1 }) + this.writeRelation(key, { ...state, currentId, serverId, version: state.version + 1 }) } } @@ -795,7 +822,7 @@ export class RelationStore { } if (changed) { - this.hasManyStates.set(key, { + this.writeHasMany(key, { serverIds, orderedIds, plannedRemovals, @@ -819,7 +846,7 @@ export class RelationStore { } for (const [oldKey, value] of toMoveRelations) { this.relationStates.delete(oldKey) - this.relationStates.set(newKeyPrefix + oldKey.slice(oldKeyPrefix.length), value) + this.writeRelation(newKeyPrefix + oldKey.slice(oldKeyPrefix.length), value) } const toMoveHasMany: [string, StoredHasManyState][] = [] @@ -830,7 +857,7 @@ export class RelationStore { } for (const [oldKey, value] of toMoveHasMany) { this.hasManyStates.delete(oldKey) - this.hasManyStates.set(newKeyPrefix + oldKey.slice(oldKeyPrefix.length), value) + this.writeHasMany(newKeyPrefix + oldKey.slice(oldKeyPrefix.length), value) } } @@ -840,6 +867,7 @@ export class RelationStore { clear(): void { this.relationStates.clear() this.hasManyStates.clear() + this.mutationVersion++ } } diff --git a/packages/bindx/src/store/RootRegistry.ts b/packages/bindx/src/store/RootRegistry.ts index 01466e0..466fea0 100644 --- a/packages/bindx/src/store/RootRegistry.ts +++ b/packages/bindx/src/store/RootRegistry.ts @@ -16,12 +16,26 @@ export class RootRegistry { private readonly roots = new Set() + /** + * Monotonic counter bumped whenever the root set actually changes. Used by + * {@link ReachabilityAnalyzer} to memoize the reachability walk. Bumps happen + * only on a real change so the per-render `registerParentChild` → `unregister` + * call (almost always a no-op for an already-anchored child) does not + * needlessly invalidate the cache. + */ + private mutationVersion = 0 + register(key: string): void { - this.roots.add(key) + if (!this.roots.has(key)) { + this.roots.add(key) + this.mutationVersion++ + } } unregister(key: string): void { - this.roots.delete(key) + if (this.roots.delete(key)) { + this.mutationVersion++ + } } keys(): IterableIterator { @@ -35,10 +49,16 @@ export class RootRegistry { rekey(oldKey: string, newKey: string): void { if (this.roots.delete(oldKey)) { this.roots.add(newKey) + this.mutationVersion++ } } clear(): void { this.roots.clear() + this.mutationVersion++ + } + + getMutationVersion(): number { + return this.mutationVersion } } From 80016d43ea2908fb39ed4c8c2da21d03197fa2d9 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 22 Jun 2026 18:33:38 +0200 Subject: [PATCH 02/19] test(bindx): cover reachability memoization cache hit/invalidation White-box tests drive ReachabilityAnalyzer directly and spy on getLiveChildIds to assert the cache hits when nothing changes (incl. across pure field edits) and misses on each graph-affecting mutation type. A black-box test proves SnapshotStore propagates counter bumps end-to-end through getAllDirtyEntities (no stale create set). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- .../store/reachabilityMemoization.test.ts | 186 ++++++++++++++++++ 1 file changed, 186 insertions(+) create mode 100644 tests/unit/store/reachabilityMemoization.test.ts diff --git a/tests/unit/store/reachabilityMemoization.test.ts b/tests/unit/store/reachabilityMemoization.test.ts new file mode 100644 index 0000000..e5d42df --- /dev/null +++ b/tests/unit/store/reachabilityMemoization.test.ts @@ -0,0 +1,186 @@ +// Memoization of the reachability walk (PR 1 of the store/persistence debt program). +// +// computeReachableCreated() is an O(E+R) walk run on every dirty check and every +// post-persist sweep. It is memoized behind a cache key that sums monotonic +// mutation counters from the four sub-stores it reads (entity snapshots, meta, +// relations, roots). The cache must: +// - HIT when nothing graph-relevant changed (incl. across pure field edits), and +// - MISS (recompute, no stale result) on any graph-affecting mutation. +// +// The white-box tests drive ReachabilityAnalyzer directly and spy on +// RelationStore.getLiveChildIds (called once per visited node) to observe whether +// a walk actually happened. The black-box test proves the SnapshotStore wiring +// propagates counter bumps end-to-end through getAllDirtyEntities. +import { describe, test, expect } from 'bun:test' +import { SnapshotStore } from '@contember/bindx' +import { ReachabilityAnalyzer } from '../../../packages/bindx/src/store/ReachabilityAnalyzer.js' +import { EntitySnapshotStore } from '../../../packages/bindx/src/store/EntitySnapshotStore.js' +import { EntityMetaStore } from '../../../packages/bindx/src/store/EntityMetaStore.js' +import { RelationStore } from '../../../packages/bindx/src/store/RelationStore.js' +import { RootRegistry } from '../../../packages/bindx/src/store/RootRegistry.js' + +interface Harness { + entitySnapshots: EntitySnapshotStore + meta: EntityMetaStore + relations: RelationStore + roots: RootRegistry + analyzer: ReachabilityAnalyzer + walkCount: () => number +} + +function createHarness(): Harness { + const entitySnapshots = new EntitySnapshotStore() + const meta = new EntityMetaStore() + const relations = new RelationStore() + const roots = new RootRegistry() + + // Spy: getLiveChildIds is invoked once per node the walk visits, so a stable + // count across a call proves the cache served it without re-walking. + let calls = 0 + const original = relations.getLiveChildIds.bind(relations) + relations.getLiveChildIds = (keyPrefix: string): string[] => { + calls++ + return original(keyPrefix) + } + + const analyzer = new ReachabilityAnalyzer(entitySnapshots, meta, relations, roots) + return { entitySnapshots, meta, relations, roots, analyzer, walkCount: () => calls } +} + +// Server Article a1 with one created (never-persisted) Comment child c1 connected +// through its has-many. c1 is reachable from the server root, so it is a create. +function seedServerParentWithCreatedChild(h: Harness): void { + h.entitySnapshots.setData('Article:a1', 'a1', 'Article', { id: 'a1' }, true) + h.meta.setExistsOnServer('Article:a1', true) + h.entitySnapshots.setData('Comment:c1', 'c1', 'Comment', { id: 'c1' }, false) + h.relations.addToHasMany('Article:a1:comments', 'c1') +} + +const sortedKeys = (set: Set): string[] => [...set].sort() + +describe('reachability memoization', () => { + test('recomputes once and returns the cached set while nothing changes', () => { + const h = createHarness() + seedServerParentWithCreatedChild(h) + + const first = h.analyzer.computeReachableCreated() + expect(sortedKeys(first)).toEqual(['Comment:c1']) + const callsAfterFirst = h.walkCount() + expect(callsAfterFirst).toBeGreaterThan(0) + + const second = h.analyzer.computeReachableCreated() + expect(h.walkCount()).toBe(callsAfterFirst) // cache hit — no re-walk + expect(second).toBe(first) // same cached instance + }) + + test('a pure field edit does NOT invalidate the cache', () => { + const h = createHarness() + seedServerParentWithCreatedChild(h) + h.analyzer.computeReachableCreated() + const calls = h.walkCount() + + // Value-only edits change snapshot data/version but not the key set or any + // relation edge, so they must not bump any reachability counter. + h.entitySnapshots.setFieldValue('Article:a1', ['title'], 'Edited') + h.entitySnapshots.updateFields('Comment:c1', { text: 'changed' }) + + h.analyzer.computeReachableCreated() + expect(h.walkCount()).toBe(calls) // still cached + }) + + test('adding another created child invalidates the cache and is reflected', () => { + const h = createHarness() + seedServerParentWithCreatedChild(h) + h.analyzer.computeReachableCreated() + const calls = h.walkCount() + + h.entitySnapshots.setData('Comment:c2', 'c2', 'Comment', { id: 'c2' }, false) + h.relations.addToHasMany('Article:a1:comments', 'c2') + + const result = h.analyzer.computeReachableCreated() + expect(h.walkCount()).toBeGreaterThan(calls) // recomputed + expect(sortedKeys(result)).toEqual(['Comment:c1', 'Comment:c2']) + }) + + test('removing a created child invalidates the cache and drops it', () => { + const h = createHarness() + seedServerParentWithCreatedChild(h) + expect(sortedKeys(h.analyzer.computeReachableCreated())).toEqual(['Comment:c1']) + const calls = h.walkCount() + + h.relations.removeFromHasMany('Article:a1:comments', 'c1', 'disconnect') + + const result = h.analyzer.computeReachableCreated() + expect(h.walkCount()).toBeGreaterThan(calls) + expect(sortedKeys(result)).toEqual([]) + }) + + test('flipping existsOnServer invalidates the cache', () => { + const h = createHarness() + // A top-level created entity (root) — reported as a create until it exists. + h.entitySnapshots.setData('Draft:d1', 'd1', 'Draft', { id: 'd1' }, false) + h.roots.register('Draft:d1') + expect(sortedKeys(h.analyzer.computeReachableCreated())).toEqual(['Draft:d1']) + + h.meta.setExistsOnServer('Draft:d1', true) + + // Flipping the last created entity to a server entity invalidates the cache; the + // recompute then takes the no-created-snapshot fast path (no node walk), so — as + // in the root (un)register case — invalidation is proven by the result alone: a + // stale cache would still report the old create instead of the empty set. + expect(sortedKeys(h.analyzer.computeReachableCreated())).toEqual([]) // now a server entity, not a create + }) + + test('toggling persisting invalidates the cache', () => { + const h = createHarness() + // A created entity reachable from nothing: not a create on its own, but it + // becomes live while it is persisting (the in-flight seed). + h.entitySnapshots.setData('Comment:x1', 'x1', 'Comment', { id: 'x1' }, false) + expect(sortedKeys(h.analyzer.computeReachableCreated())).toEqual([]) + const calls = h.walkCount() + + h.meta.setPersisting('Comment:x1', true) + + const result = h.analyzer.computeReachableCreated() + expect(h.walkCount()).toBeGreaterThan(calls) + expect(sortedKeys(result)).toEqual(['Comment:x1']) + }) + + test('registering and unregistering a root invalidates the cache', () => { + const h = createHarness() + h.entitySnapshots.setData('Draft:d2', 'd2', 'Draft', { id: 'd2' }, false) + expect(sortedKeys(h.analyzer.computeReachableCreated())).toEqual([]) + + h.roots.register('Draft:d2') + expect(sortedKeys(h.analyzer.computeReachableCreated())).toEqual(['Draft:d2']) + + // A no-op unregister (key absent) must NOT change the result and stays correct. + h.roots.unregister('Draft:absent') + expect(sortedKeys(h.analyzer.computeReachableCreated())).toEqual(['Draft:d2']) + + h.roots.unregister('Draft:d2') + expect(sortedKeys(h.analyzer.computeReachableCreated())).toEqual([]) + }) + + test('end-to-end: SnapshotStore propagates bumps through getAllDirtyEntities', () => { + const store = new SnapshotStore() + store.setEntityData('Article', 'a1', { id: 'a1', title: 'T' }, true) + const cId = store.createEntity('Comment', { text: 'c' }) + store.addToHasMany('Article', 'a1', 'comments', cId) + store.registerParentChild('Article', 'a1', 'Comment', cId) + + const createIds = (): string[] => + store + .getAllDirtyEntities() + .filter(e => e.changeType === 'create') + .map(e => e.entityId) + + expect(createIds()).toEqual([cId]) // populates the cache + // A field edit must not stale the create set. + store.setFieldValue('Article', 'a1', ['title'], 'Edited') + expect(createIds()).toEqual([cId]) + // A graph change must be reflected, not served from a stale cache. + store.removeFromHasMany('Article', 'a1', 'comments', cId, 'disconnect') + expect(createIds()).toEqual([]) + }) +}) From f3a19390f75f3d0f3253908ed89219ed6a744a78 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 22 Jun 2026 18:47:36 +0200 Subject: [PATCH 03/19] refactor(bindx): centralize temp-id rekey in RekeyOrchestrator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The temp→persisted rekey was a 9-store fan-out hand-written inline in SnapshotStore.mapTempIdToPersistedId, and the same temp→persisted fact was stored twice in two formats: SnapshotStore.rekeyedEntities ("Type:tempId" → "Type:persistedId") and EntityMetaStore.tempToPersistedId ("Type:key" → persistedId). Introduce RekeyOrchestrator as the single owner of temp→persisted identity. Both key resolution (resolveKey/resolveId) and persisted-id queries (getPersistedId/isNewEntity) now derive from its one map, so the two duplicate maps are gone. EntityMetaStore sheds tempToPersistedId and its getPersistedId/isNewEntity/mapTempIdToPersistedId API; the exists-on-server flip folds into its rekey(). Each participating sub-store implements a uniform Rekeyable.rekey(ctx) interface, and the orchestrator drives them in one explicit, documented order (previously load-bearing but implicit in the inline sequence). SubscriptionManager keeps its own closure-redirect chain, which tracks relation-key prefixes and stale unsubscribe closures rather than entity identity. clear() now also clears the redirect map (previously leaked). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- packages/bindx/src/store/EntityMetaStore.ts | 62 +++------- .../bindx/src/store/EntitySnapshotStore.ts | 22 ++-- packages/bindx/src/store/ErrorStore.ts | 11 +- packages/bindx/src/store/RekeyOrchestrator.ts | 112 ++++++++++++++++++ packages/bindx/src/store/RelationStore.ts | 13 +- packages/bindx/src/store/RootRegistry.ts | 14 ++- packages/bindx/src/store/SnapshotStore.ts | 76 ++++-------- .../bindx/src/store/SubscriptionManager.ts | 7 +- packages/bindx/src/store/TouchedStore.ts | 7 +- 9 files changed, 202 insertions(+), 122 deletions(-) create mode 100644 packages/bindx/src/store/RekeyOrchestrator.ts diff --git a/packages/bindx/src/store/EntityMetaStore.ts b/packages/bindx/src/store/EntityMetaStore.ts index 297d535..a4b940f 100644 --- a/packages/bindx/src/store/EntityMetaStore.ts +++ b/packages/bindx/src/store/EntityMetaStore.ts @@ -1,6 +1,6 @@ import type { LoadStatus } from './snapshots.js' -import { isPersistedId, isPlaceholderId } from './entityId.js' import type { FieldError } from '../errors/types.js' +import type { RekeyContext, Rekeyable } from './RekeyOrchestrator.js' /** * Entity load state tracking. @@ -26,7 +26,7 @@ export interface EntityMeta { * Keys are pre-computed composite strings (e.g., "entityType:id"). * Follows the same pattern as ErrorStore and RelationStore. */ -export class EntityMetaStore { +export class EntityMetaStore implements Rekeyable { /** Load states keyed by "entityType:id" */ private readonly loadStates = new Map() @@ -36,9 +36,6 @@ export class EntityMetaStore { /** Persisting status keyed by "entityType:id" */ private readonly persistingEntities = new Set() - /** Mapping from temp ID to persisted ID (keyed by "entityType:tempId") */ - private readonly tempToPersistedId = new Map() - /** * Monotonic counter bumped when reachability-relevant metadata changes — * `existsOnServer` and `isPersisting` (which seed the reachability roots) plus @@ -118,65 +115,41 @@ export class EntityMetaStore { } } - // ==================== Temp ID Mapping ==================== - - mapTempIdToPersistedId(key: string, persistedId: string): void { - this.tempToPersistedId.set(key, persistedId) - this.setExistsOnServer(key, true) - } - - getPersistedId(key: string, id: string): string | null { - if (isPlaceholderId(id)) return null - if (isPersistedId(id)) return id - return this.tempToPersistedId.get(key) ?? null - } - - isNewEntity(key: string, id: string): boolean { - if (isPlaceholderId(id)) return true - if (isPersistedId(id)) return false - return !this.tempToPersistedId.has(key) - } - /** - * Removes all metadata for an entity (load state, meta, persisting, temp mapping). + * Removes all metadata for an entity (load state, meta, persisting). */ remove(key: string): void { this.loadStates.delete(key) this.entityMetas.delete(key) this.persistingEntities.delete(key) - this.tempToPersistedId.delete(key) this.mutationVersion++ } /** - * Moves all metadata from oldKey to newKey. + * Moves all metadata from the temp key to the persisted key. The entity has + * just been confirmed by the server, so it is also marked as existing there + * (this replaces the former separate mapTempIdToPersistedId step). */ - rekey(oldKey: string, newKey: string): void { - const meta = this.entityMetas.get(oldKey) + rekey(ctx: RekeyContext): void { + const meta = this.entityMetas.get(ctx.oldKey) if (meta) { - this.entityMetas.delete(oldKey) - this.entityMetas.set(newKey, meta) + this.entityMetas.delete(ctx.oldKey) + this.entityMetas.set(ctx.newKey, meta) } - const loadState = this.loadStates.get(oldKey) + const loadState = this.loadStates.get(ctx.oldKey) if (loadState) { - this.loadStates.delete(oldKey) - this.loadStates.set(newKey, loadState) - } - - if (this.persistingEntities.has(oldKey)) { - this.persistingEntities.delete(oldKey) - this.persistingEntities.add(newKey) + this.loadStates.delete(ctx.oldKey) + this.loadStates.set(ctx.newKey, loadState) } - // Move temp ID mapping - const persistedId = this.tempToPersistedId.get(oldKey) - if (persistedId !== undefined) { - this.tempToPersistedId.delete(oldKey) - this.tempToPersistedId.set(newKey, persistedId) + if (this.persistingEntities.has(ctx.oldKey)) { + this.persistingEntities.delete(ctx.oldKey) + this.persistingEntities.add(ctx.newKey) } this.mutationVersion++ + this.setExistsOnServer(ctx.newKey, true) } // ==================== Bulk Operations ==================== @@ -205,7 +178,6 @@ export class EntityMetaStore { this.loadStates.clear() this.entityMetas.clear() this.persistingEntities.clear() - this.tempToPersistedId.clear() this.mutationVersion++ } } diff --git a/packages/bindx/src/store/EntitySnapshotStore.ts b/packages/bindx/src/store/EntitySnapshotStore.ts index 3292e5f..71f4a8d 100644 --- a/packages/bindx/src/store/EntitySnapshotStore.ts +++ b/packages/bindx/src/store/EntitySnapshotStore.ts @@ -2,6 +2,7 @@ import { createEntitySnapshot, type EntitySnapshot, } from './snapshots.js' +import type { RekeyContext, Rekeyable } from './RekeyOrchestrator.js' /** * Manages entity snapshots — core CRUD for immutable entity data. @@ -12,7 +13,7 @@ import { * Follows the same sub-store pattern as ErrorStore, RelationStore, etc. * SnapshotStore delegates entity snapshot operations here. */ -export class EntitySnapshotStore { +export class EntitySnapshotStore implements Rekeyable { private readonly snapshots = new Map() /** @@ -304,28 +305,29 @@ export class EntitySnapshotStore { } /** - * Moves a snapshot from oldKey to newKey, updating the id in the snapshot data. + * Moves a snapshot from the temp key to the persisted key, updating the id in + * the snapshot data. */ - rekey(oldKey: string, newKey: string, newId: string): void { - const snapshot = this.snapshots.get(oldKey) + rekey(ctx: RekeyContext): void { + const snapshot = this.snapshots.get(ctx.oldKey) if (!snapshot) return // Update id field in data and serverData - const data = { ...snapshot.data as Record, id: newId } - const serverData = { ...snapshot.serverData as Record, id: newId } + const data = { ...snapshot.data as Record, id: ctx.newId } + const serverData = { ...snapshot.serverData as Record, id: ctx.newId } const newSnapshot = createEntitySnapshot( - newId, + ctx.newId, snapshot.entityType, data, serverData, snapshot.version + 1, ) - this.snapshots.delete(oldKey) - this.snapshots.set(newKey, newSnapshot) + this.snapshots.delete(ctx.oldKey) + this.snapshots.set(ctx.newKey, newSnapshot) this.idIndex.delete(snapshot.id) - this.idIndex.set(newId, newKey) + this.idIndex.set(ctx.newId, ctx.newKey) this.mutationVersion++ } diff --git a/packages/bindx/src/store/ErrorStore.ts b/packages/bindx/src/store/ErrorStore.ts index 4e783d4..ca8759a 100644 --- a/packages/bindx/src/store/ErrorStore.ts +++ b/packages/bindx/src/store/ErrorStore.ts @@ -1,5 +1,6 @@ import type { ErrorState, FieldError } from '../errors/types.js' import { filterStickyErrors } from '../errors/types.js' +import type { RekeyContext, Rekeyable } from './RekeyOrchestrator.js' /** * Manages error state for fields, entities, and relations. @@ -9,7 +10,7 @@ import { filterStickyErrors } from '../errors/types.js' * - Entity errors: "entityType:id" * - Relation errors: "entityType:id:relationName" */ -export class ErrorStore { +export class ErrorStore implements Rekeyable { /** Field errors keyed by "entityType:id:fieldName" */ private readonly fieldErrors = new Map() @@ -252,10 +253,10 @@ export class ErrorStore { /** * Rekeys all errors from oldKeyPrefix to newKeyPrefix. */ - rekey(oldEntityKey: string, newEntityKey: string, oldKeyPrefix: string, newKeyPrefix: string): void { - this.rekeyMap(this.entityErrors, oldEntityKey, newEntityKey) - this.rekeyByPrefix(this.fieldErrors, oldKeyPrefix, newKeyPrefix) - this.rekeyByPrefix(this.relationErrors, oldKeyPrefix, newKeyPrefix) + rekey(ctx: RekeyContext): void { + this.rekeyMap(this.entityErrors, ctx.oldKey, ctx.newKey) + this.rekeyByPrefix(this.fieldErrors, ctx.oldKeyPrefix, ctx.newKeyPrefix) + this.rekeyByPrefix(this.relationErrors, ctx.oldKeyPrefix, ctx.newKeyPrefix) } private rekeyMap(map: Map, oldKey: string, newKey: string): void { diff --git a/packages/bindx/src/store/RekeyOrchestrator.ts b/packages/bindx/src/store/RekeyOrchestrator.ts new file mode 100644 index 0000000..8789550 --- /dev/null +++ b/packages/bindx/src/store/RekeyOrchestrator.ts @@ -0,0 +1,112 @@ +import { isPersistedId, isPlaceholderId } from './entityId.js' + +/** + * Describes a single temp→persisted rekey in every shape the participating + * sub-stores need (full key, owner prefix, bare id). + */ +export interface RekeyContext { + /** Old composite key, "entityType:tempId". */ + readonly oldKey: string + /** New composite key, "entityType:persistedId". */ + readonly newKey: string + /** Old owner prefix, "entityType:tempId:". */ + readonly oldKeyPrefix: string + /** New owner prefix, "entityType:persistedId:". */ + readonly newKeyPrefix: string + /** The temp id being replaced. */ + readonly oldId: string + /** The server-assigned id. */ + readonly newId: string +} + +/** + * A store that owns per-entity state keyed (directly or by prefix) on an entity + * id and can migrate it from a temp id to its persisted id. + */ +export interface Rekeyable { + rekey(ctx: RekeyContext): void +} + +/** + * Single source of temp→persisted identity, and the one place the rekey fan-out + * is sequenced. + * + * Owns the only id-redirect map: "entityType:tempId" → persistedId. Both key + * resolution ({@link resolveKey}/{@link resolveId}) and persisted-id queries + * ({@link getPersistedId}/{@link isNewEntity}) derive from it, so there is no + * second copy to keep in sync — the former `SnapshotStore.rekeyedEntities` and + * `EntityMetaStore.tempToPersistedId` are both gone. `SubscriptionManager` keeps + * its own closure-redirect chain, which tracks relation-key prefixes and stale + * unsubscribe closures rather than entity identity, so it stays internal there. + */ +export class RekeyOrchestrator { + /** "entityType:tempId" → persistedId. The single identity-redirect map. */ + private readonly tempToPersisted = new Map() + + /** + * @param participants the sub-stores to migrate, in the exact order + * {@link rekey} must visit them (the order is load-bearing — see rekey()). + */ + constructor(private readonly participants: readonly Rekeyable[]) {} + + /** Resolves an entity key, following a temp→persisted redirect if present. */ + resolveKey(entityType: string, id: string): string { + const persisted = this.tempToPersisted.get(`${entityType}:${id}`) + return persisted !== undefined ? `${entityType}:${persisted}` : `${entityType}:${id}` + } + + /** Resolves an entity id to its persisted id if it has been rekeyed. */ + resolveId(entityType: string, id: string): string { + return this.tempToPersisted.get(`${entityType}:${id}`) ?? id + } + + /** The persisted id for an entity, or null if it has none yet. */ + getPersistedId(entityType: string, id: string): string | null { + if (isPlaceholderId(id)) return null + if (isPersistedId(id)) return id + return this.tempToPersisted.get(`${entityType}:${id}`) ?? null + } + + /** Whether an entity has never been persisted (no server-assigned id yet). */ + isNewEntity(entityType: string, id: string): boolean { + if (isPlaceholderId(id)) return true + if (isPersistedId(id)) return false + return !this.tempToPersisted.has(`${entityType}:${id}`) + } + + /** + * Migrates an entity from its temp id to its server-assigned id across every + * participating store. + * + * Ordering contract (do not reorder — each step relies on the previous): + * 0. register the redirect, so any resolveKey/resolveId during the fan-out + * already sees the persisted key; + * 1. roots — keep the root registry aligned; + * 2. entity snapshot — move data, rewrite the id field and id index; + * 3. meta — move metadata/load/persisting, then mark exists-on-server; + * 4. subscriptions — move entity + relation subscribers and parent links; + * 5. relations — rekey owned relation keys, then replace value references; + * 6. errors / touched / propagation — move the remaining per-entity state. + * The caller performs the final notification on the new key. + */ + rekey(entityType: string, tempId: string, persistedId: string): void { + const ctx: RekeyContext = { + oldKey: `${entityType}:${tempId}`, + newKey: `${entityType}:${persistedId}`, + oldKeyPrefix: `${entityType}:${tempId}:`, + newKeyPrefix: `${entityType}:${persistedId}:`, + oldId: tempId, + newId: persistedId, + } + + this.tempToPersisted.set(ctx.oldKey, persistedId) + + for (const participant of this.participants) { + participant.rekey(ctx) + } + } + + clear(): void { + this.tempToPersisted.clear() + } +} diff --git a/packages/bindx/src/store/RelationStore.ts b/packages/bindx/src/store/RelationStore.ts index 24ff870..6df9217 100644 --- a/packages/bindx/src/store/RelationStore.ts +++ b/packages/bindx/src/store/RelationStore.ts @@ -1,5 +1,6 @@ import type { HasOneRelationState } from '../handles/types.js' import type { EntitySnapshot } from './snapshots.js' +import type { RekeyContext, Rekeyable } from './RekeyOrchestrator.js' function setsEqual(a: Set, b: Set): boolean { if (a.size !== b.size) return false @@ -49,7 +50,7 @@ export interface StoredRelationState { * Relation keys use the format "parentType:parentId:fieldName". * Notification is handled by callers via callback returns. */ -export class RelationStore { +export class RelationStore implements Rekeyable { /** Relation states keyed by "parentType:parentId:fieldName" */ private readonly relationStates = new Map() @@ -834,6 +835,16 @@ export class RelationStore { } } + /** + * Migrates an entity's relation state for a temp→persisted rekey: first the + * relation/has-many keys it owns (the parent id in the key), then every value + * reference to its id from other entities' relations. + */ + rekey(ctx: RekeyContext): void { + this.rekeyOwner(ctx.oldKeyPrefix, ctx.newKeyPrefix) + this.replaceEntityId(ctx.oldId, ctx.newId) + } + /** * Rekeys relation/hasMany entries owned by an entity (changes the parent ID in the key). */ diff --git a/packages/bindx/src/store/RootRegistry.ts b/packages/bindx/src/store/RootRegistry.ts index 466fea0..5aee96e 100644 --- a/packages/bindx/src/store/RootRegistry.ts +++ b/packages/bindx/src/store/RootRegistry.ts @@ -13,7 +13,9 @@ * * Keys use the same composite format as the rest of the store: "entityType:id". */ -export class RootRegistry { +import type { RekeyContext, Rekeyable } from './RekeyOrchestrator.js' + +export class RootRegistry implements Rekeyable { private readonly roots = new Set() /** @@ -43,12 +45,12 @@ export class RootRegistry { } /** - * Moves a root entry from oldKey to newKey (used after persist rekeys a temp - * id to a server-assigned id). + * Moves a root entry from the temp key to the persisted key (used after + * persist rekeys a temp id to a server-assigned id). */ - rekey(oldKey: string, newKey: string): void { - if (this.roots.delete(oldKey)) { - this.roots.add(newKey) + rekey(ctx: RekeyContext): void { + if (this.roots.delete(ctx.oldKey)) { + this.roots.add(ctx.newKey) this.mutationVersion++ } } diff --git a/packages/bindx/src/store/SnapshotStore.ts b/packages/bindx/src/store/SnapshotStore.ts index 19e9eae..decb0db 100644 --- a/packages/bindx/src/store/SnapshotStore.ts +++ b/packages/bindx/src/store/SnapshotStore.ts @@ -15,6 +15,7 @@ import { DirtyTracker } from './DirtyTracker.js' import { EntitySnapshotStore } from './EntitySnapshotStore.js' import { RootRegistry } from './RootRegistry.js' import { ReachabilityAnalyzer } from './ReachabilityAnalyzer.js' +import { RekeyOrchestrator } from './RekeyOrchestrator.js' export type { HasManyRemovalType, StoredHasManyState, StoredRelationState } from './RelationStore.js' export type { EntityMeta } from './EntityMetaStore.js' @@ -49,6 +50,7 @@ export class SnapshotStore implements SnapshotVersionBumper { private readonly roots = new RootRegistry() private readonly reachability: ReachabilityAnalyzer private readonly dirtyTracker: DirtyTracker + private readonly rekeyOrchestrator: RekeyOrchestrator /** * Tracks the last embedded data reference propagated from parent to child. @@ -61,16 +63,25 @@ export class SnapshotStore implements SnapshotVersionBumper { constructor() { this.reachability = new ReachabilityAnalyzer(this.entitySnapshots, this.meta, this.relations, this.roots) this.dirtyTracker = new DirtyTracker(this.entitySnapshots, this.meta, this.relations, this.reachability) + // The participants are visited in this exact order on every rekey — see the + // ordering contract in RekeyOrchestrator.rekey(). Propagation tracking lives + // on this store, so it joins as a small inline adapter. + this.rekeyOrchestrator = new RekeyOrchestrator([ + this.roots, + this.entitySnapshots, + this.meta, + this.subscriptions, + this.relations, + this.errors, + this.touched, + { rekey: ctx => this.rekeyPropagatedData(ctx.oldKeyPrefix, ctx.newKeyPrefix) }, + ]) } // ==================== Key Generation ==================== - /** Maps temp ID entity keys to their persisted ID entity keys for transparent resolution */ - private readonly rekeyedEntities = new Map() - private getEntityKey(entityType: string, id: string): string { - const key = `${entityType}:${id}` - return this.rekeyedEntities.get(key) ?? key + return this.rekeyOrchestrator.resolveKey(entityType, id) } private getRelationKey(parentType: string, parentId: string, fieldName: string): string { @@ -82,13 +93,7 @@ export class SnapshotStore implements SnapshotVersionBumper { * Resolves an ID to its persisted ID if it has been rekeyed. */ private resolveId(entityType: string, id: string): string { - const key = `${entityType}:${id}` - const rekeyed = this.rekeyedEntities.get(key) - if (rekeyed) { - // Extract the ID from the rekeyed key (format: "EntityType:id") - return rekeyed.slice(entityType.length + 1) - } - return id + return this.rekeyOrchestrator.resolveId(entityType, id) } // ==================== SnapshotVersionBumper ==================== @@ -367,52 +372,20 @@ export class SnapshotStore implements SnapshotVersionBumper { } mapTempIdToPersistedId(entityType: string, tempId: string, persistedId: string): void { - // Use raw keys (bypass resolveId) since we're the ones creating the mapping - const oldKey = `${entityType}:${tempId}` - const newKey = `${entityType}:${persistedId}` - const oldKeyPrefix = `${entityType}:${tempId}:` - const newKeyPrefix = `${entityType}:${persistedId}:` - - // Register redirect so future lookups by temp ID resolve to persisted key - this.rekeyedEntities.set(oldKey, newKey) - - // Keep root registration consistent across the rekey (the persisted entity - // is now a server root via existsOnServer, but keep the registry aligned). - this.roots.rekey(oldKey, newKey) - - // Rekey entity snapshot (moves data, updates id field) - this.entitySnapshots.rekey(oldKey, newKey, persistedId) - - // Rekey metadata FIRST, then set persisted mapping (which sets existsOnServer) - this.meta.rekey(oldKey, newKey) - this.meta.mapTempIdToPersistedId(newKey, persistedId) + // The orchestrator owns the temp→persisted redirect and drives the rekey + // fan-out across every sub-store in its documented order. + this.rekeyOrchestrator.rekey(entityType, tempId, persistedId) - // Rekey subscriptions, parent-child relationships, and relation subscribers - this.subscriptions.rekey(oldKey, newKey, oldKeyPrefix, newKeyPrefix) - - // Rekey relations/hasMany owned by this entity (parent key changes) - this.relations.rekeyOwner(oldKeyPrefix, newKeyPrefix) - - // Replace tempId with persistedId in all relation/hasMany VALUE references - this.relations.replaceEntityId(tempId, persistedId) - - // Rekey errors, touched state, and propagation tracking - this.errors.rekey(oldKey, newKey, oldKeyPrefix, newKeyPrefix) - this.touched.rekey(oldKeyPrefix, newKeyPrefix) - this.rekeyPropagatedData(oldKeyPrefix, newKeyPrefix) - - // Notify on the NEW key so React picks up the change - this.notifyEntitySubscribers(newKey) + // Notify on the NEW key so React picks up the change. + this.notifyEntitySubscribers(`${entityType}:${persistedId}`) } getPersistedId(entityType: string, id: string): string | null { - const key = this.getEntityKey(entityType, id) - return this.meta.getPersistedId(key, id) + return this.rekeyOrchestrator.getPersistedId(entityType, id) } isNewEntity(entityType: string, id: string): boolean { - const key = this.getEntityKey(entityType, id) - return this.meta.isNewEntity(key, id) + return this.rekeyOrchestrator.isNewEntity(entityType, id) } // ==================== Has-Many State (delegated to RelationStore) ==================== @@ -953,6 +926,7 @@ export class SnapshotStore implements SnapshotVersionBumper { this.errors.clear() this.touched.clear() this.roots.clear() + this.rekeyOrchestrator.clear() this.lastPropagatedData.clear() this.subscriptions.notify() diff --git a/packages/bindx/src/store/SubscriptionManager.ts b/packages/bindx/src/store/SubscriptionManager.ts index a736dea..b4ef7c1 100644 --- a/packages/bindx/src/store/SubscriptionManager.ts +++ b/packages/bindx/src/store/SubscriptionManager.ts @@ -1,3 +1,5 @@ +import type { RekeyContext, Rekeyable } from './RekeyOrchestrator.js' + type Subscriber = () => void /** @@ -18,7 +20,7 @@ export interface SnapshotVersionBumper { * - Parent-child change propagation * - Global version tracking for change detection */ -export class SubscriptionManager { +export class SubscriptionManager implements Rekeyable { /** Subscribers per entity key */ private readonly entitySubscribers = new Map>() @@ -265,7 +267,8 @@ export class SubscriptionManager { * Also rekeys relation subscribers under oldKeyPrefix to newKeyPrefix. * Registers redirects so unsubscribe closures can find migrated callbacks. */ - rekey(oldKey: string, newKey: string, oldKeyPrefix: string, newKeyPrefix: string): void { + rekey(ctx: RekeyContext): void { + const { oldKey, newKey, oldKeyPrefix, newKeyPrefix } = ctx // Register redirect for entity key (update existing chains first) for (const [fromKey, toKey] of this.rekeyedKeys) { if (toKey === oldKey) { diff --git a/packages/bindx/src/store/TouchedStore.ts b/packages/bindx/src/store/TouchedStore.ts index 305910f..b7bea34 100644 --- a/packages/bindx/src/store/TouchedStore.ts +++ b/packages/bindx/src/store/TouchedStore.ts @@ -1,3 +1,5 @@ +import type { RekeyContext, Rekeyable } from './RekeyOrchestrator.js' + /** * Manages touched state for entity fields. * @@ -6,7 +8,7 @@ * * Keys are pre-computed composite strings (e.g., "entityType:id:fieldName"). */ -export class TouchedStore { +export class TouchedStore implements Rekeyable { /** Touched state keyed by "entityType:id:fieldName" */ private readonly touchedFields = new Map() @@ -39,7 +41,8 @@ export class TouchedStore { /** * Rekeys all touched fields from oldKeyPrefix to newKeyPrefix. */ - rekey(oldKeyPrefix: string, newKeyPrefix: string): void { + rekey(ctx: RekeyContext): void { + const { oldKeyPrefix, newKeyPrefix } = ctx const toMove: [string, boolean][] = [] for (const [key, value] of this.touchedFields) { if (key.startsWith(oldKeyPrefix)) { From 010c7b72c0034e4631fa4cd1d5d2c979c68999a8 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 22 Jun 2026 18:47:41 +0200 Subject: [PATCH 04/19] test(bindx): cover RekeyOrchestrator resolution and ordering contract Pins resolveKey/resolveId/getPersistedId/isNewEntity across placeholder, temp, and persisted ids; asserts rekey visits every participant exactly once in order with a fully-derived context; and an end-to-end check that SnapshotStore resolves a created entity after persist via the orchestrator. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- tests/unit/store/rekeyOrchestrator.test.ts | 107 +++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 tests/unit/store/rekeyOrchestrator.test.ts diff --git a/tests/unit/store/rekeyOrchestrator.test.ts b/tests/unit/store/rekeyOrchestrator.test.ts new file mode 100644 index 0000000..4d4fe57 --- /dev/null +++ b/tests/unit/store/rekeyOrchestrator.test.ts @@ -0,0 +1,107 @@ +// RekeyOrchestrator (PR 2 of the store/persistence debt program). +// +// The orchestrator is the single owner of temp→persisted identity and the one +// place the rekey fan-out is sequenced. These tests pin its resolution logic +// (resolveKey/resolveId/getPersistedId/isNewEntity, formerly split between +// SnapshotStore.rekeyedEntities and EntityMetaStore.tempToPersistedId) and its +// ordering contract (each participant visited exactly once, in order, with a +// fully-derived context). The end-to-end fan-out across the real sub-stores is +// covered by tests/subscriptionRekey.test.ts. +import { describe, test, expect } from 'bun:test' +import { SnapshotStore } from '@contember/bindx' +import { RekeyOrchestrator, type RekeyContext, type Rekeyable } from '../../../packages/bindx/src/store/RekeyOrchestrator.js' + +const TEMP = '__temp_1' +const PLACEHOLDER = '__placeholder_1' +const SERVER = 'srv-1' + +describe('RekeyOrchestrator', () => { + test('resolveKey/resolveId redirect a temp id to its persisted id after rekey', () => { + const o = new RekeyOrchestrator([]) + + // Before rekey: identity. + expect(o.resolveKey('Article', TEMP)).toBe(`Article:${TEMP}`) + expect(o.resolveId('Article', TEMP)).toBe(TEMP) + + o.rekey('Article', TEMP, SERVER) + + expect(o.resolveKey('Article', TEMP)).toBe(`Article:${SERVER}`) + expect(o.resolveId('Article', TEMP)).toBe(SERVER) + // A persisted id (or an unrelated type) is never redirected. + expect(o.resolveKey('Article', SERVER)).toBe(`Article:${SERVER}`) + expect(o.resolveKey('Comment', TEMP)).toBe(`Comment:${TEMP}`) + }) + + test('getPersistedId/isNewEntity reflect placeholder, temp, and persisted ids', () => { + const o = new RekeyOrchestrator([]) + + // Placeholder is always "new" and has no persisted id. + expect(o.getPersistedId('Article', PLACEHOLDER)).toBeNull() + expect(o.isNewEntity('Article', PLACEHOLDER)).toBe(true) + + // A persisted-looking id is itself the persisted id and is not new. + expect(o.getPersistedId('Article', SERVER)).toBe(SERVER) + expect(o.isNewEntity('Article', SERVER)).toBe(false) + + // A temp id has no persisted id until it is rekeyed. + expect(o.getPersistedId('Article', TEMP)).toBeNull() + expect(o.isNewEntity('Article', TEMP)).toBe(true) + + o.rekey('Article', TEMP, SERVER) + + expect(o.getPersistedId('Article', TEMP)).toBe(SERVER) + expect(o.isNewEntity('Article', TEMP)).toBe(false) + }) + + test('rekey visits every participant exactly once, in order', () => { + const calls: string[] = [] + const spy = (name: string): Rekeyable => ({ rekey: () => calls.push(name) }) + + const o = new RekeyOrchestrator([spy('a'), spy('b'), spy('c')]) + o.rekey('Article', TEMP, SERVER) + + expect(calls).toEqual(['a', 'b', 'c']) + }) + + test('rekey passes a fully-derived context to participants', () => { + let captured: RekeyContext | undefined + const o = new RekeyOrchestrator([{ rekey: ctx => { captured = ctx } }]) + + o.rekey('Article', TEMP, SERVER) + + expect(captured).toEqual({ + oldKey: `Article:${TEMP}`, + newKey: `Article:${SERVER}`, + oldKeyPrefix: `Article:${TEMP}:`, + newKeyPrefix: `Article:${SERVER}:`, + oldId: TEMP, + newId: SERVER, + }) + }) + + test('clear forgets all redirects', () => { + const o = new RekeyOrchestrator([]) + o.rekey('Article', TEMP, SERVER) + expect(o.resolveId('Article', TEMP)).toBe(SERVER) + + o.clear() + expect(o.resolveId('Article', TEMP)).toBe(TEMP) + expect(o.getPersistedId('Article', TEMP)).toBeNull() + }) + + test('end-to-end: SnapshotStore resolves a created entity after persist via the orchestrator', () => { + const store = new SnapshotStore() + const tempId = store.createEntity('Article', { title: 'Draft' }) + expect(store.isNewEntity('Article', tempId)).toBe(true) + expect(store.getPersistedId('Article', tempId)).toBeNull() + + store.mapTempIdToPersistedId('Article', tempId, 'server-99') + + // The handle still references the temp id; lookups transparently resolve. + expect(store.getPersistedId('Article', tempId)).toBe('server-99') + expect(store.isNewEntity('Article', tempId)).toBe(false) + expect(store.existsOnServer('Article', tempId)).toBe(true) + // The snapshot moved to the persisted key (id field rewritten). + expect(store.getEntitySnapshot('Article', 'server-99')?.data).toMatchObject({ id: 'server-99', title: 'Draft' }) + }) +}) From ea009e01c071d9b0b743e095fcd11479d9da8656 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 22 Jun 2026 18:54:49 +0200 Subject: [PATCH 05/19] feat(bindx): add pessimistic presentation primitive (inert) Introduces the read primitive that will let handles present the server baseline during a pessimistic persist WITHOUT the current mutate-store- to-server-then-restore dance. Inert for now: no consumer reads it yet (wiring + removal of mutate-restore lands in the next PR). - EntityMetaStore tracks a pessimisticInFlight set (a subset of the persisting set), set/cleared in the same setPersisting transition so the two cannot drift; it does not bump the reachability mutation counter since the flag drives presentation only. - SnapshotStore.getPresentationSnapshot returns the canonical snapshot except while pessimistically in-flight, when it returns a frozen server-baseline view (data === serverData) built without mutating the store. Non-pessimistic entities are returned verbatim, so optimistic and not-persisting share one path. - The SET_PERSISTING action carries an optional pessimistic flag; BatchPersister sets it when updateMode is pessimistic. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- packages/bindx/src/core/ActionDispatcher.ts | 1 + packages/bindx/src/core/actions.ts | 9 ++++- .../bindx/src/persistence/BatchPersister.ts | 5 +-- packages/bindx/src/store/EntityMetaStore.ts | 35 ++++++++++++++++-- packages/bindx/src/store/SnapshotStore.ts | 36 +++++++++++++++++-- 5 files changed, 78 insertions(+), 8 deletions(-) diff --git a/packages/bindx/src/core/ActionDispatcher.ts b/packages/bindx/src/core/ActionDispatcher.ts index bd66eda..6750671 100644 --- a/packages/bindx/src/core/ActionDispatcher.ts +++ b/packages/bindx/src/core/ActionDispatcher.ts @@ -373,6 +373,7 @@ export class ActionDispatcher { action.entityType, action.entityId, action.isPersisting, + action.pessimistic ?? false, ) break diff --git a/packages/bindx/src/core/actions.ts b/packages/bindx/src/core/actions.ts index 4042d84..f3c31a1 100644 --- a/packages/bindx/src/core/actions.ts +++ b/packages/bindx/src/core/actions.ts @@ -202,6 +202,12 @@ export interface SetPersistingAction { readonly entityType: string readonly entityId: string readonly isPersisting: boolean + /** + * Whether this persist is pessimistic. While a pessimistic persist is + * in-flight, handles present the server baseline instead of the (still dirty) + * local data. Ignored when isPersisting is false. + */ + readonly pessimistic?: boolean } // ==================== Error Actions ==================== @@ -428,8 +434,9 @@ export function setPersisting( entityType: string, entityId: string, isPersisting: boolean, + pessimistic: boolean = false, ): SetPersistingAction { - return { type: 'SET_PERSISTING', entityType, entityId, isPersisting } + return { type: 'SET_PERSISTING', entityType, entityId, isPersisting, pessimistic } } /** diff --git a/packages/bindx/src/persistence/BatchPersister.ts b/packages/bindx/src/persistence/BatchPersister.ts index f0e3339..a173a22 100644 --- a/packages/bindx/src/persistence/BatchPersister.ts +++ b/packages/bindx/src/persistence/BatchPersister.ts @@ -206,9 +206,10 @@ export class BatchPersister { // Mark all as in-flight this.changeRegistry.markInFlight(sortedEntities) - // Set persisting state for all entities + // Set persisting state for all entities. In pessimistic mode the flag also + // marks the entity for server-baseline presentation while in-flight. for (const entity of sortedEntities) { - this.dispatcher.dispatch(setPersisting(entity.entityType, entity.entityId, true)) + this.dispatcher.dispatch(setPersisting(entity.entityType, entity.entityId, true, updateMode === 'pessimistic')) this.dispatcher.dispatch(clearAllServerErrors(entity.entityType, entity.entityId)) } diff --git a/packages/bindx/src/store/EntityMetaStore.ts b/packages/bindx/src/store/EntityMetaStore.ts index a4b940f..e8e23a1 100644 --- a/packages/bindx/src/store/EntityMetaStore.ts +++ b/packages/bindx/src/store/EntityMetaStore.ts @@ -36,6 +36,14 @@ export class EntityMetaStore implements Rekeyable { /** Persisting status keyed by "entityType:id" */ private readonly persistingEntities = new Set() + /** + * Entities whose in-flight persist is pessimistic, keyed by "entityType:id". + * A subset of {@link persistingEntities}. While present, the entity is + * presented at its server baseline (see SnapshotStore.getPresentationSnapshot) + * even though its canonical data stays dirty. + */ + private readonly pessimisticInFlight = new Set() + /** * Monotonic counter bumped when reachability-relevant metadata changes — * `existsOnServer` and `isPersisting` (which seed the reachability roots) plus @@ -104,17 +112,31 @@ export class EntityMetaStore implements Rekeyable { return this.persistingEntities.has(key) } - setPersisting(key: string, isPersisting: boolean): void { + setPersisting(key: string, isPersisting: boolean, pessimistic: boolean = false): void { if (isPersisting) { if (!this.persistingEntities.has(key)) { this.persistingEntities.add(key) this.mutationVersion++ } - } else if (this.persistingEntities.delete(key)) { - this.mutationVersion++ + // The pessimistic flag drives presentation only, not reachability, so it + // deliberately does not bump mutationVersion. + if (pessimistic) { + this.pessimisticInFlight.add(key) + } else { + this.pessimisticInFlight.delete(key) + } + } else { + if (this.persistingEntities.delete(key)) { + this.mutationVersion++ + } + this.pessimisticInFlight.delete(key) } } + isPessimisticInFlight(key: string): boolean { + return this.pessimisticInFlight.has(key) + } + /** * Removes all metadata for an entity (load state, meta, persisting). */ @@ -122,6 +144,7 @@ export class EntityMetaStore implements Rekeyable { this.loadStates.delete(key) this.entityMetas.delete(key) this.persistingEntities.delete(key) + this.pessimisticInFlight.delete(key) this.mutationVersion++ } @@ -148,6 +171,11 @@ export class EntityMetaStore implements Rekeyable { this.persistingEntities.add(ctx.newKey) } + if (this.pessimisticInFlight.has(ctx.oldKey)) { + this.pessimisticInFlight.delete(ctx.oldKey) + this.pessimisticInFlight.add(ctx.newKey) + } + this.mutationVersion++ this.setExistsOnServer(ctx.newKey, true) } @@ -178,6 +206,7 @@ export class EntityMetaStore implements Rekeyable { this.loadStates.clear() this.entityMetas.clear() this.persistingEntities.clear() + this.pessimisticInFlight.clear() this.mutationVersion++ } } diff --git a/packages/bindx/src/store/SnapshotStore.ts b/packages/bindx/src/store/SnapshotStore.ts index decb0db..e882e72 100644 --- a/packages/bindx/src/store/SnapshotStore.ts +++ b/packages/bindx/src/store/SnapshotStore.ts @@ -1,4 +1,5 @@ import type { EntitySnapshot, LoadStatus } from './snapshots.js' +import { createEntitySnapshot } from './snapshots.js' import type { FieldError } from '../errors/types.js' import { SubscriptionManager, type SnapshotVersionBumper } from './SubscriptionManager.js' import { ErrorStore } from './ErrorStore.js' @@ -583,12 +584,43 @@ export class SnapshotStore implements SnapshotVersionBumper { return this.meta.isPersisting(key) } - setPersisting(entityType: string, id: string, isPersisting: boolean): void { + setPersisting(entityType: string, id: string, isPersisting: boolean, pessimistic: boolean = false): void { const key = this.getEntityKey(entityType, id) - this.meta.setPersisting(key, isPersisting) + this.meta.setPersisting(key, isPersisting, pessimistic) this.notifyEntitySubscribers(key) } + /** + * Returns the snapshot a consumer should DISPLAY for an entity. + * + * This equals the canonical {@link getEntitySnapshot} except while the entity + * is pessimistically in-flight, when it returns the server baseline (data === + * serverData) WITHOUT mutating the store. The canonical snapshot stays dirty, + * so dirty tracking, mutation building, and retry are unaffected — only the + * presented value is the pre-persist server view. + * + * Inert until consumers route their display reads through it (see PR 4); a + * non-pessimistic entity is returned verbatim, so optimistic mode and the + * not-persisting case share this one path. + */ + getPresentationSnapshot(entityType: string, id: string): EntitySnapshot | undefined { + const snapshot = this.getEntitySnapshot(entityType, id) + if (!snapshot) return undefined + + const key = this.getEntityKey(entityType, id) + if (!this.meta.isPessimisticInFlight(key)) { + return snapshot + } + + return createEntitySnapshot( + snapshot.id, + snapshot.entityType, + snapshot.serverData, + snapshot.serverData, + snapshot.version, + ) + } + // ==================== Error State (delegated to ErrorStore) ==================== getFieldErrors(entityType: string, id: string, fieldName: string): readonly FieldError[] { From 1fe2a66ba2409037e3f6280db4692af0f7c8dd9c Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 22 Jun 2026 18:54:49 +0200 Subject: [PATCH 06/19] test(bindx): cover the pessimistic presentation primitive Asserts presentation equals canonical without the flag, equals the server baseline while pessimistically in-flight (canonical staying dirty, still reported as an update), tracks optimistic vs pessimistic, restores on clear, and that the baseline view is frozen and not aliased. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- tests/unit/store/presentationFlag.test.ts | 76 +++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 tests/unit/store/presentationFlag.test.ts diff --git a/tests/unit/store/presentationFlag.test.ts b/tests/unit/store/presentationFlag.test.ts new file mode 100644 index 0000000..8a4947b --- /dev/null +++ b/tests/unit/store/presentationFlag.test.ts @@ -0,0 +1,76 @@ +// Pessimistic presentation primitive (PR 3 / C1 of the store/persistence debt program). +// +// getPresentationSnapshot returns the snapshot a consumer should DISPLAY: the +// canonical snapshot, except while an entity is pessimistically in-flight, when +// it returns the server baseline (data === serverData) WITHOUT mutating the +// store. The canonical snapshot stays dirty, so dirty tracking and retry are +// unaffected. This primitive is inert until PR 4 routes display reads through it. +import { describe, test, expect, beforeEach } from 'bun:test' +import { SnapshotStore } from '@contember/bindx' + +interface Article { + id: string + title: string +} + +describe('pessimistic presentation flag', () => { + let store: SnapshotStore + + beforeEach(() => { + store = new SnapshotStore() + // Server baseline "Server", with a local dirty edit "Local edit". + store.setEntityData('Article', 'a1', { id: 'a1', title: 'Server' }, true) + store.setFieldValue('Article', 'a1', ['title'], 'Local edit') + }) + + const presentedTitle = (): string | undefined => + store.getPresentationSnapshot
('Article', 'a1')?.data.title + const canonicalTitle = (): string | undefined => + store.getEntitySnapshot
('Article', 'a1')?.data.title + + test('without the flag, presentation equals the canonical snapshot', () => { + expect(presentedTitle()).toBe('Local edit') + expect(store.getPresentationSnapshot('Article', 'a1')).toBe(store.getEntitySnapshot('Article', 'a1')) + }) + + test('while pessimistically in-flight, presentation is the server baseline', () => { + store.setPersisting('Article', 'a1', true, true) + + expect(presentedTitle()).toBe('Server') + const presented = store.getPresentationSnapshot
('Article', 'a1') + expect(presented?.data).toEqual(presented?.serverData) + }) + + test('the canonical snapshot stays dirty during pessimistic in-flight', () => { + store.setPersisting('Article', 'a1', true, true) + + // Canonical data untouched — no mutate-restore. + expect(canonicalTitle()).toBe('Local edit') + // Still reported as a dirty update. + expect(store.getAllDirtyEntities()).toContainEqual({ + entityType: 'Article', + entityId: 'a1', + changeType: 'update', + }) + }) + + test('optimistic in-flight presents the live data, not the baseline', () => { + store.setPersisting('Article', 'a1', true, false) + expect(presentedTitle()).toBe('Local edit') + }) + + test('clearing the persisting flag restores canonical presentation', () => { + store.setPersisting('Article', 'a1', true, true) + expect(presentedTitle()).toBe('Server') + + store.setPersisting('Article', 'a1', false) + expect(presentedTitle()).toBe('Local edit') + }) + + test('the presented baseline snapshot is frozen and does not alias the stored one', () => { + store.setPersisting('Article', 'a1', true, true) + const presented = store.getPresentationSnapshot
('Article', 'a1') + expect(Object.isFrozen(presented)).toBe(true) + expect(presented).not.toBe(store.getEntitySnapshot('Article', 'a1')) + }) +}) From 241eb4a8d7125e3ad0607135da10a5b6de134659 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 22 Jun 2026 19:03:36 +0200 Subject: [PATCH 07/19] refactor(bindx): route handle display reads through presentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Field and entity value display now read getPresentationSnapshot, so a pessimistically in-flight entity shows the server baseline. Dirty tracking keeps reading the canonical snapshot, so a field stays correctly dirty while its display shows the server value — the crucial split that lets the next commit delete the mutate-store-to-server-then-restore dance. - BaseHandle: getEntityData stays canonical (also used by has-many materialization); new getPresentationData for display. - FieldHandle.value -> presentation; isDirty compares canonical vs server. - EntityHandle.data -> presentation; getSnapshot/isDirty/serverData stay canonical. Scope note: relation display (has-one/has-many) still presents canonical state during a pessimistic in-flight window; presenting relations at the server baseline is a follow-up, cleaner once the has-one snapshot fallback is removed (keystone PR). Final post-persist states are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- packages/bindx/src/handles/BaseHandle.ts | 14 +++++++++- packages/bindx/src/handles/EntityHandle.ts | 7 +++-- packages/bindx/src/handles/FieldHandle.ts | 13 ++++++--- tests/unit/handles/fieldHandle.test.ts | 32 ++++++++++++++++++++++ 4 files changed, 59 insertions(+), 7 deletions(-) diff --git a/packages/bindx/src/handles/BaseHandle.ts b/packages/bindx/src/handles/BaseHandle.ts index 6dbd5ae..f62bea6 100644 --- a/packages/bindx/src/handles/BaseHandle.ts +++ b/packages/bindx/src/handles/BaseHandle.ts @@ -83,13 +83,25 @@ export abstract class EntityRelatedHandle extends BaseHandle { } /** - * Get entity data. + * Get the CANONICAL entity data — the live, possibly-dirty values. Use this + * for dirty tracking and any logic that must reflect the user's edits, even + * while a pessimistic persist is in-flight. For values to DISPLAY, use + * {@link getPresentationData}. */ protected getEntityData(): Record | undefined { const snapshot = this.store.getEntitySnapshot(this.entityType, this.entityId) return snapshot?.data as Record | undefined } + /** + * Get the entity data a consumer should DISPLAY. Equals {@link getEntityData} + * except while the entity is pessimistically in-flight, when it returns the + * server baseline (the canonical data stays dirty underneath). + */ + protected getPresentationData(): Record | undefined { + return this.store.getPresentationSnapshot>(this.entityType, this.entityId)?.data + } + /** * Get entity server data. */ diff --git a/packages/bindx/src/handles/EntityHandle.ts b/packages/bindx/src/handles/EntityHandle.ts index d5db13a..6eff44f 100644 --- a/packages/bindx/src/handles/EntityHandle.ts +++ b/packages/bindx/src/handles/EntityHandle.ts @@ -140,11 +140,14 @@ export class EntityHandle extends Enti } /** - * Gets the current entity data. + * Gets the current entity data to DISPLAY. * Returns selected fields subset as specified by TSelected type parameter. + * While a pessimistic persist is in-flight this is the server baseline; the + * canonical snapshot (see {@link getSnapshot}, used for dirty tracking) stays + * dirty underneath. */ get data(): TSelected | null { - const snapshot = this.getSnapshot() + const snapshot = this.store.getPresentationSnapshot(this.entityType, this.entityId) return (snapshot?.data ?? null) as TSelected | null } diff --git a/packages/bindx/src/handles/FieldHandle.ts b/packages/bindx/src/handles/FieldHandle.ts index d2cdcde..63192e3 100644 --- a/packages/bindx/src/handles/FieldHandle.ts +++ b/packages/bindx/src/handles/FieldHandle.ts @@ -87,10 +87,11 @@ export class FieldHandle extends EntityRelatedHandle { } /** - * Gets the current field value. + * Gets the current field value to DISPLAY. While a pessimistic persist is + * in-flight this is the server baseline; otherwise it is the live value. */ get value(): T | null { - const data = this.getEntityData() + const data = this.getPresentationData() if (!data) return null return getNestedValue(data, this.fieldPath) as T | null } @@ -105,10 +106,14 @@ export class FieldHandle extends EntityRelatedHandle { } /** - * Checks if the field has been modified. + * Checks if the field has been modified. Compares the CANONICAL value against + * the server value — never the presented value, so a field stays correctly + * dirty even while its display shows the server baseline mid-pessimistic-persist. */ get isDirty(): boolean { - return !deepEqual(this.value, this.serverValue) + const data = this.getEntityData() + const value = data ? (getNestedValue(data, this.fieldPath) as T | null) : null + return !deepEqual(value, this.serverValue) } /** diff --git a/tests/unit/handles/fieldHandle.test.ts b/tests/unit/handles/fieldHandle.test.ts index 3dd68e0..284f5c3 100644 --- a/tests/unit/handles/fieldHandle.test.ts +++ b/tests/unit/handles/fieldHandle.test.ts @@ -55,6 +55,38 @@ describe('FieldHandle', () => { }) }) + // ==================== Pessimistic Presentation ==================== + + describe('Pessimistic Presentation', () => { + test('value shows the server baseline while pessimistically in-flight, but stays dirty', () => { + store.setEntityData('Article', 'a-1', { id: 'a-1', title: 'Server' }, true) + store.setFieldValue('Article', 'a-1', ['title'], 'Local edit') + const handle = createFieldHandle(['title']) + + expect(handle.value).toBe('Local edit') + expect(handle.isDirty).toBe(true) + + store.setPersisting('Article', 'a-1', true, true) + // Display flips to the server baseline; dirty state (canonical) is unchanged. + expect(handle.value).toBe('Server') + expect(handle.serverValue).toBe('Server') + expect(handle.isDirty).toBe(true) + + store.setPersisting('Article', 'a-1', false) + expect(handle.value).toBe('Local edit') + expect(handle.isDirty).toBe(true) + }) + + test('optimistic in-flight keeps showing the live value', () => { + store.setEntityData('Article', 'a-1', { id: 'a-1', title: 'Server' }, true) + store.setFieldValue('Article', 'a-1', ['title'], 'Local edit') + const handle = createFieldHandle(['title']) + + store.setPersisting('Article', 'a-1', true, false) + expect(handle.value).toBe('Local edit') + }) + }) + // ==================== Dirty State ==================== describe('Dirty State', () => { From 311d56c189319b506a78aaa0ca5f68dbfe15f043 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 22 Jun 2026 19:08:49 +0200 Subject: [PATCH 08/19] refactor(bindx): delete pessimistic mutate-restore dance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With handle display reads now routed through getPresentationSnapshot, the store no longer needs to be reset to the server view during a pessimistic persist and restored afterward. Remove the whole capture/reset/restore machinery: - drop captureEntityStates, restoreEntityState and the CapturedEntityState type, and the per-update resetEntity + resetAllRelations block; - success now commits the (still-dirty) canonical state as the new server baseline via the same path optimistic mode always used; - failure leaves the entity dirty with no action — edits and created entities survive for a retry (P2) by construction, since the store was never mutated; - remove the now-dead store methods getAllRelationsForEntity, getAllHasManyForEntity and restoreHasManyState. The full pessimistic suite (commit-on-success, P2 preserve-edits and preserve-create on failure, isPersisting timing, batch) passes unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- .../bindx/src/persistence/BatchPersister.ts | 176 +++--------------- packages/bindx/src/store/RelationStore.ts | 48 ----- packages/bindx/src/store/SnapshotStore.ts | 22 --- 3 files changed, 23 insertions(+), 223 deletions(-) diff --git a/packages/bindx/src/persistence/BatchPersister.ts b/packages/bindx/src/persistence/BatchPersister.ts index a173a22..65ffb09 100644 --- a/packages/bindx/src/persistence/BatchPersister.ts +++ b/packages/bindx/src/persistence/BatchPersister.ts @@ -25,19 +25,6 @@ import type { EntitySnapshot } from '../store/snapshots.js' import type { StoredHasManyState, StoredRelationState } from '../store/SnapshotStore.js' import { deepEqual } from '../utils/deepEqual.js' -/** - * Captured state for pessimistic mode restoration. - * Contains all the data needed to restore an entity's dirty state after - * temporarily resetting it to server state during pessimistic persistence. - */ -interface CapturedEntityState { - entityType: string - entityId: string - snapshot: EntitySnapshot | undefined - relations: Map - hasManyStates: Map -} - /** * Options for BatchPersister */ @@ -216,17 +203,12 @@ export class BatchPersister { // Block undo during persist this.undoManager?.block() - // For pessimistic mode, capture dirty state before building mutations - // This allows us to restore and commit the state after server confirmation - let capturedStates: CapturedEntityState[] | null = null - if (updateMode === 'pessimistic') { - capturedStates = this.captureEntityStates(sortedEntities) - } - let result: PersistenceResult | undefined try { - // Build mutations BEFORE resetting state (for pessimistic mode) - // The mutations need to contain the dirty data to send to server + // Build mutations from the (dirty) canonical state. The store is never + // mutated to the server view — pessimistic mode presents the server + // baseline via getPresentationSnapshot instead — so there is nothing to + // capture or restore. const mutations = this.buildMutations(sortedEntities, scope) if (mutations.length === 0) { @@ -241,23 +223,11 @@ export class BatchPersister { return result } - // For pessimistic mode, reset entities to server state after capturing - // This makes the UI show the "old" (server) data while persisting - if (updateMode === 'pessimistic') { - for (const entity of sortedEntities) { - // Only reset updates, not creates (creates don't have server state) - if (entity.changeType === 'update') { - this.dispatcher.dispatch(resetEntity(entity.entityType, entity.entityId)) - this.store.resetAllRelations(entity.entityType, entity.entityId) - } - } - } - // Execute transaction const transactionResult = await this.executeTransaction(mutations, options?.signal) - // Process results with captured state for pessimistic mode - result = this.processTransactionResult(sortedEntities, transactionResult, scope, options, capturedStates) + // Assigned to `result` so the finally block can gate the sweep on full success. + result = this.processTransactionResult(sortedEntities, transactionResult, scope, options) return result } finally { @@ -273,9 +243,9 @@ export class BatchPersister { this.undoManager?.unblock() // Reclaim memory: drop snapshots of any created entities orphaned during - // editing. Only after a FULLY successful persist — on failure the captured - // creates/edits were restored for retry (see processTransactionResult), and - // sweeping then could reclaim a create the user still intends to save. + // editing. Only after a FULLY successful persist — on a failed or partial + // persist the user's creates and edits are left intact and dirty for a retry, + // and sweeping then could reclaim a create the user still intends to save. if (result?.success) { this.store.sweepUnreachableCreated() } @@ -675,13 +645,11 @@ export class BatchPersister { transactionResult: TransactionResult, scope: PersistScope, options?: BatchPersistOptions, - capturedStates?: CapturedEntityState[] | null, ): PersistenceResult { const results: EntityPersistResult[] = [] let successCount = 0 let failedCount = 0 const rollbackOnError = options?.rollbackOnError ?? false - const isPessimistic = capturedStates != null if (transactionResult.ok) { // All succeeded - commit all @@ -692,24 +660,16 @@ export class BatchPersister { ) if (entity) { - // For pessimistic mode, restore captured state and commit - if (isPessimistic && capturedStates) { - const capturedState = capturedStates.find( - s => s.entityType === entity.entityType && s.entityId === entity.entityId, - ) - if (capturedState) { - this.restoreEntityState(capturedState, true) - } + // The store was never mutated to the server view, so a successful + // persist just commits the dirty state as the new server baseline — + // the same path optimistic mode always took. + if (scope.type === 'fields' && scope.entityType === entity.entityType && scope.entityId === entity.entityId) { + // Partial commit for field scope + this.store.commitFields(entity.entityType, entity.entityId, [...scope.fields]) } else { - // Commit based on scope - if (scope.type === 'fields' && scope.entityType === entity.entityType && scope.entityId === entity.entityId) { - // Partial commit for field scope - this.store.commitFields(entity.entityType, entity.entityId, [...scope.fields]) - } else { - // Full commit - this.dispatcher.dispatch(commitEntity(entity.entityType, entity.entityId)) - this.store.commitAllRelations(entity.entityType, entity.entityId) - } + // Full commit + this.dispatcher.dispatch(commitEntity(entity.entityType, entity.entityId)) + this.store.commitAllRelations(entity.entityType, entity.entityId) } // Map temp ID if create @@ -759,10 +719,11 @@ export class BatchPersister { mutationResult.errorMessage, ) - // Optimistic mode rolls a FAILED entity back to server state only when - // rollbackOnError is set. Pessimistic restore is handled below for ALL - // captured entities, not just the failed ones. - if (!isPessimistic && rollbackOnError) { + // The store was never mutated to the server view, so on failure the + // entity's edits and creates are already intact and dirty — they + // simply survive for a retry (P2), no restore needed. Rollback to + // server state happens only when rollbackOnError is set. + if (rollbackOnError) { this.rollbackEntity(entity) } } @@ -786,20 +747,6 @@ export class BatchPersister { } } } - - // Pessimistic mode: the transaction failed, so from the user's point of view - // nothing was committed. Undo the pre-transaction server-view reset for EVERY - // captured entity (not only the ones whose individual mutation failed) and keep - // the restored data dirty (commit=false) so the user can retry. Restoring even - // the entities whose mutation happened to succeed in the non-atomic sequential - // fallback is what re-establishes their relation edges to inline-created - // children — keeping those creates reachable so the post-persist sweep does not - // reclaim a child the user still intends to save (P2). - if (isPessimistic && capturedStates) { - for (const capturedState of capturedStates) { - this.restoreEntityState(capturedState, false) - } - } } return { @@ -811,83 +758,6 @@ export class BatchPersister { } } - /** - * Captures the current state of entities for pessimistic mode. - * This allows us to restore the dirty state after a successful persist. - */ - private captureEntityStates(entities: DirtyEntity[]): CapturedEntityState[] { - return entities.map(entity => { - const snapshot = this.store.getEntitySnapshot(entity.entityType, entity.entityId) - const relations = this.store.getAllRelationsForEntity(entity.entityType, entity.entityId) - const hasManyStates = this.store.getAllHasManyForEntity(entity.entityType, entity.entityId) - - return { - entityType: entity.entityType, - entityId: entity.entityId, - snapshot, - relations, - hasManyStates, - } - }) - } - - /** - * Restores a captured entity state (pessimistic mode), optionally committing it. - * - * - commit=true (server confirmed): restore the dirty data and commit it as the - * new server baseline. - * - commit=false (server rejected): restore the dirty data but leave it dirty, so - * the user's in-flight edits and creates survive for a retry (P2) instead of - * being stuck at the server view the pessimistic reset left behind. - */ - private restoreEntityState(capturedState: CapturedEntityState, commit: boolean): void { - if (capturedState.snapshot) { - this.store.setEntityData( - capturedState.entityType, - capturedState.entityId, - capturedState.snapshot.data as Record, - commit, // server data only when confirmed; otherwise keep it as a dirty edit - ) - } - - for (const [relationKey, relationState] of capturedState.relations) { - const fieldName = relationKey.split(':')[2] - if (fieldName) { - this.store.setRelation(capturedState.entityType, capturedState.entityId, fieldName, { - currentId: relationState.currentId, - state: relationState.state, - // Restore placeholderData too: a has-one in the 'creating' state carries - // its inline create payload here, and getDirtyRelations treats a - // non-empty placeholderData as a dirty signal. Dropping it on a - // commit=false retry would lose the inline data and silently mark the - // relation clean. (For commit=true it is cleared by commitRelation below.) - placeholderData: relationState.placeholderData, - }) - if (commit) { - this.store.commitRelation(capturedState.entityType, capturedState.entityId, fieldName) - } - } - } - - for (const [hasManyKey, hasManyState] of capturedState.hasManyStates) { - const fieldName = hasManyKey.split(':')[2] - if (fieldName) { - this.store.restoreHasManyState(capturedState.entityType, capturedState.entityId, fieldName, hasManyState) - if (commit) { - // Compute new server IDs: serverIds - removals + connections - const newServerIds = new Set(hasManyState.serverIds) - for (const removedId of hasManyState.plannedRemovals.keys()) { - newServerIds.delete(removedId) - } - for (const connectedId of hasManyState.plannedConnections) { - newServerIds.add(connectedId) - } - this.store.commitHasMany(capturedState.entityType, capturedState.entityId, fieldName, Array.from(newServerIds)) - } - } - } - } - /** * Rolls back an entity's optimistic changes to server state. * Handles all mutation types: create, update, and delete. diff --git a/packages/bindx/src/store/RelationStore.ts b/packages/bindx/src/store/RelationStore.ts index 6df9217..b912b55 100644 --- a/packages/bindx/src/store/RelationStore.ts +++ b/packages/bindx/src/store/RelationStore.ts @@ -556,21 +556,6 @@ export class RelationStore implements Rekeyable { return computeDefaultOrderedIds(existing) } - /** - * Restores a has-many state from a captured snapshot. - * Used in pessimistic mode after successful server confirmation. - */ - restoreHasManyState(key: string, state: StoredHasManyState): void { - this.writeHasMany(key, { - serverIds: new Set(state.serverIds), - orderedIds: state.orderedIds ? [...state.orderedIds] : null, - plannedRemovals: new Map(state.plannedRemovals), - plannedConnections: new Set(state.plannedConnections), - createdEntities: new Set(state.createdEntities), - version: state.version + 1, - }) - } - // ==================== Bulk Operations ==================== /** @@ -614,39 +599,6 @@ export class RelationStore implements Rekeyable { } } - /** - * Gets all relation states for an entity. - */ - getAllRelationsForEntity(keyPrefix: string): Map { - const result = new Map() - for (const [key, state] of this.relationStates) { - if (key.startsWith(keyPrefix)) { - result.set(key, { ...state }) - } - } - return result - } - - /** - * Gets all has-many states for an entity. - */ - getAllHasManyForEntity(keyPrefix: string): Map { - const result = new Map() - for (const [key, state] of this.hasManyStates) { - if (key.startsWith(keyPrefix)) { - result.set(key, { - serverIds: new Set(state.serverIds), - orderedIds: state.orderedIds ? [...state.orderedIds] : null, - plannedRemovals: new Map(state.plannedRemovals), - plannedConnections: new Set(state.plannedConnections), - createdEntities: new Set(state.createdEntities), - version: state.version, - }) - } - } - return result - } - // ==================== Dirty Tracking ==================== /** diff --git a/packages/bindx/src/store/SnapshotStore.ts b/packages/bindx/src/store/SnapshotStore.ts index e882e72..7727aad 100644 --- a/packages/bindx/src/store/SnapshotStore.ts +++ b/packages/bindx/src/store/SnapshotStore.ts @@ -792,28 +792,6 @@ export class SnapshotStore implements SnapshotVersionBumper { this.relations.resetAllRelations(keyPrefix) } - getAllRelationsForEntity(entityType: string, entityId: string): Map { - const keyPrefix = `${entityType}:${entityId}:` - return this.relations.getAllRelationsForEntity(keyPrefix) - } - - getAllHasManyForEntity(entityType: string, entityId: string): Map { - const keyPrefix = `${entityType}:${entityId}:` - return this.relations.getAllHasManyForEntity(keyPrefix) - } - - restoreHasManyState( - parentType: string, - parentId: string, - fieldName: string, - state: StoredHasManyState, - alias?: string, - ): void { - const key = this.getRelationKey(parentType, parentId, alias ?? fieldName) - this.relations.restoreHasManyState(key, state) - this.notifyRelationSubscribers(key) - } - // ==================== Subscriptions (delegated to SubscriptionManager) ==================== subscribeToEntity(entityType: string, id: string, callback: Subscriber): () => void { From feb12f6a92b53b1cf68712e8867a83db7396f737 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 22 Jun 2026 19:13:24 +0200 Subject: [PATCH 09/19] test(bindx): pin parent-notification behavior before childToParents rework MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subscriber call-count harness covering has-one/has-many parent re-render on child change, the diamond (shared child notifies both parents), rekey preserving subscriptions, and the known append-only childToParents leak (disconnect still notifies the former parent) — the regression oracle for the upcoming reverse-index notification rework. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- .../store/notificationPropagation.test.ts | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 tests/unit/store/notificationPropagation.test.ts diff --git a/tests/unit/store/notificationPropagation.test.ts b/tests/unit/store/notificationPropagation.test.ts new file mode 100644 index 0000000..edd6109 --- /dev/null +++ b/tests/unit/store/notificationPropagation.test.ts @@ -0,0 +1,151 @@ +import { describe, test, expect, beforeEach } from 'bun:test' +import { SnapshotStore } from '@contember/bindx' +import { createTestStore, createMockSubscriber } from '../shared/unitTestHelpers.js' + +/** + * Pins the CURRENT parent re-render / subscription-notification behavior with + * subscriber call-count assertions, BEFORE the notification machinery is rewired. + * + * A later PR replaces the append-only `childToParents` registry in + * `SubscriptionManager` with a reverse index derived from relation edges. This + * harness is the regression oracle: every assertion encodes today's behavior so + * the rework can prove it introduced no re-render regression. + * + * Mechanics worth keeping in mind while reading these tests: + * - `setRelation` / `addToHasMany` notify the relation's own subscribers and the + * relation OWNER's entity subscribers — they do NOT walk `childToParents`. + * - `setFieldValue` on the child entity calls `notifyEntitySubscribers`, which + * walks `childToParents` UP the tree and bumps each ancestor's snapshot version. + * So the child-field mutation is what exercises parent propagation here. + */ +describe('Notification propagation', () => { + let store: SnapshotStore + + beforeEach(() => { + store = createTestStore() + }) + + test('has-one parent re-renders when child field changes', () => { + // Server parent A with a child B connected via has-one. + store.setEntityData('Author', 'author-1', { id: 'author-1', name: 'Alice' }, true) + store.setEntityData('Article', 'article-1', { id: 'article-1', title: 'Draft' }, true) + + store.getOrCreateRelation('Author', 'author-1', 'featuredArticle', { + currentId: null, + serverId: null, + state: 'disconnected', + serverState: 'disconnected', + placeholderData: {}, + }) + store.setRelation('Author', 'author-1', 'featuredArticle', { + currentId: 'article-1', + state: 'connected', + }) + store.registerParentChild('Author', 'author-1', 'Article', 'article-1') + + const parent = createMockSubscriber() + store.subscribeToEntity('Author', 'author-1', parent.fn) + parent.reset() + + // Mutate the child. + store.setFieldValue('Article', 'article-1', ['title'], 'Updated') + + // The parent's subscriber must fire via child→parent propagation. + expect(parent.callCount()).toBe(1) + }) + + test('has-many parent re-renders when an item changes', () => { + // Server parent A with a child item B in a has-many. + store.setEntityData('Author', 'author-1', { id: 'author-1', name: 'Alice' }, true) + store.setEntityData('Article', 'article-1', { id: 'article-1', title: 'Draft' }, true) + + store.getOrCreateHasMany('Author', 'author-1', 'articles', []) + store.addToHasMany('Author', 'author-1', 'articles', 'article-1') + store.registerParentChild('Author', 'author-1', 'Article', 'article-1') + + const parent = createMockSubscriber() + store.subscribeToEntity('Author', 'author-1', parent.fn) + parent.reset() + + // Mutate the child item. + store.setFieldValue('Article', 'article-1', ['title'], 'Updated') + + expect(parent.callCount()).toBe(1) + }) + + test('disconnect still notifies the former parent (current append-only behavior)', () => { + // Server parent A with a child B connected via has-one. + store.setEntityData('Author', 'author-1', { id: 'author-1', name: 'Alice' }, true) + store.setEntityData('Article', 'article-1', { id: 'article-1', title: 'Draft' }, true) + + store.getOrCreateRelation('Author', 'author-1', 'featuredArticle', { + currentId: null, + serverId: null, + state: 'disconnected', + serverState: 'disconnected', + placeholderData: {}, + }) + store.setRelation('Author', 'author-1', 'featuredArticle', { + currentId: 'article-1', + state: 'connected', + }) + store.registerParentChild('Author', 'author-1', 'Article', 'article-1') + + // "Disconnect" the child: clear the relation. Note that a relation disconnect + // does NOT call unregisterParentChild, so the parent link survives. + store.setRelation('Author', 'author-1', 'featuredArticle', { + currentId: null, + state: 'disconnected', + }) + + const parent = createMockSubscriber() + store.subscribeToEntity('Author', 'author-1', parent.fn) + parent.reset() + + // Mutate the now-disconnected child. + store.setFieldValue('Article', 'article-1', ['title'], 'Updated') + + // LEAK: PR 7 will make a disconnected child stop notifying its former parent; + // this assertion will flip then. + expect(parent.callCount()).toBe(1) + }) + + test('diamond: a shared child notifies both parents', () => { + // Child B connected to TWO parents A1 and A2. + store.setEntityData('Author', 'author-1', { id: 'author-1', name: 'Alice' }, true) + store.setEntityData('Author', 'author-2', { id: 'author-2', name: 'Bob' }, true) + store.setEntityData('Article', 'article-1', { id: 'article-1', title: 'Draft' }, true) + + store.registerParentChild('Author', 'author-1', 'Article', 'article-1') + store.registerParentChild('Author', 'author-2', 'Article', 'article-1') + + const parentOne = createMockSubscriber() + const parentTwo = createMockSubscriber() + store.subscribeToEntity('Author', 'author-1', parentOne.fn) + store.subscribeToEntity('Author', 'author-2', parentTwo.fn) + parentOne.reset() + parentTwo.reset() + + // Mutate the shared child. + store.setFieldValue('Article', 'article-1', ['title'], 'Updated') + + expect(parentOne.callCount()).toBe(1) + expect(parentTwo.callCount()).toBe(1) + }) + + test('rekey preserves subscriptions', () => { + // Subscribe to a created (temp-id) entity, rekey it, then mutate via the + // persisted id and confirm the original subscription still fires. + const tempId = store.createEntity('Article', { title: 'Draft' }) + + const subscriber = createMockSubscriber() + store.subscribeToEntity('Article', tempId, subscriber.fn) + + store.mapTempIdToPersistedId('Article', tempId, 'article-persisted') + subscriber.reset() + + store.setFieldValue('Article', 'article-persisted', ['title'], 'Updated') + + expect(subscriber.callCount()).toBe(1) + }) +}) From 53f23e21787dd6db0b5f27ff103f3991d66c1fe9 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 22 Jun 2026 19:29:23 +0200 Subject: [PATCH 10/19] refactor(bindx): make RelationStore authoritative for has-one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Has-one relation state had two sources of truth: the RelationStore entry AND the related object embedded in the parent's snapshot data. HasOneHandle read from the store when an entry existed, else fell back to the embedded snapshot — invisible to reachability-based create detection, which reads membership exclusively from RelationStore. Mirror the has-many path (HasManyListHandle.materializeEmbeddedItems): add an idempotent ensureEntry() that materializes the relation entry from the parent's embedded data on every relation-state read (relatedId/state/isDirty), then drop the snapshot fallback so RelationStore is the single source of truth. - Initial materialization uses the non-notifying getOrCreateRelation, deriving the server baseline from the parent's serverData so a freshly loaded relation is not dirty (currentId === serverId, state === serverState). - A parent re-fetch (embedded reference changed) advances the server baseline only when the relation is not locally dirty, via hasEmbeddedDataChanged — child-snapshot propagation keeps sole ownership of the propagation slot in ensureRelatedEntitySnapshot, so the two paths never double-consume it. - Local connect/disconnect, placeholders, and creating entries are never clobbered (detected as locally-dirty / id-mismatch). A created child connected via a has-one now appears in reachability and getAllDirtyEntities() as a create with no explicit setRelation, with no analyzer change. Tests cover loaded-not-dirty materialization, the dropped fallback, and the created-child-as-create case. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- packages/bindx/src/handles/HasOneHandle.ts | 140 ++++++++++++++++----- tests/unit/handles/hasOneHandle.test.ts | 123 ++++++++++++++++++ 2 files changed, 233 insertions(+), 30 deletions(-) diff --git a/packages/bindx/src/handles/HasOneHandle.ts b/packages/bindx/src/handles/HasOneHandle.ts index 35faaec..a02e658 100644 --- a/packages/bindx/src/handles/HasOneHandle.ts +++ b/packages/bindx/src/handles/HasOneHandle.ts @@ -1,6 +1,7 @@ import { EntityRelatedHandle, embeddedDataMatchesSnapshot } from './BaseHandle.js' import type { ActionDispatcher } from '../core/ActionDispatcher.js' import type { SnapshotStore } from '../store/SnapshotStore.js' +import type { StoredRelationState } from '../store/RelationStore.js' import type { SchemaRegistry } from '../schema/SchemaRegistry.js' import type { SelectionMeta } from '../selection/types.js' import { @@ -17,6 +18,7 @@ import { type Unsubscribe, type EntityAccessor, type HasOneAccessor, + type HasOneRelationState, } from './types.js' import { createClientError, type ErrorInput, type FieldError } from '../errors/types.js' import type { @@ -137,51 +139,122 @@ export class HasOneHandle /** * Gets the relation state. - * Falls back to snapshot data when no explicit relation state exists, - * so server-loaded has-one relations report 'connected' without - * requiring a prior RelationStore entry. + * Materializes the RelationStore entry from embedded snapshot data first, so a + * server-loaded has-one reports 'connected' without a prior explicit entry. */ - get state(): 'connected' | 'disconnected' | 'deleted' | 'creating' { + get state(): HasOneRelationState { + this.ensureEntry() const relation = this.store.getRelation( this.entityType, this.entityId, this.fieldName, ) - if (relation) { - return relation.state - } - // No explicit relation state — check if snapshot has embedded data - if (this.relatedId !== null) { - return 'connected' - } - return 'disconnected' + return relation?.state ?? 'disconnected' } /** - * Gets the current related entity ID. - * Falls back to entity data if relation state is not initialized. + * Gets the current related entity ID — read exclusively from the RelationStore + * after materializing the entry from embedded snapshot data. */ get relatedId(): string | null { - // First check relation state (for manual changes like connect/disconnect) + this.ensureEntry() const relation = this.store.getRelation( this.entityType, this.entityId, this.fieldName, ) - if (relation) { - return relation.currentId + return relation?.currentId ?? null + } + + /** + * Materializes this has-one's RelationStore entry from the parent's embedded + * snapshot data, so the store is the single source of truth (symmetric with + * {@link HasManyListHandle.materializeEmbeddedItems}). + * + * Only the relation entry is touched here — child-snapshot propagation stays in + * {@link ensureRelatedEntitySnapshot}, which owns the per-relation propagation + * slot so the two paths never double-consume it. + * + * - No entry yet + the parent embeds a related object with an id → create a + * `connected` entry (non-notifying) whose server baseline comes from the + * parent's serverData, so a freshly loaded relation is not dirty + * (currentId === serverId, state === serverState). + * - Existing entry + parent re-fetch (embedded reference changed) that is NOT + * locally dirty → advance the server baseline to the new related id. + * - A local connect/disconnect, a placeholder, or a `creating` entry is left + * untouched — it is detected as a locally-dirty relation. + * - No embedded data and no entry → the relation stays unmaterialized (null). + */ + private ensureEntry(): void { + const existing = this.store.getRelation(this.entityType, this.entityId, this.fieldName) + const embeddedId = this.readEmbeddedRelatedId() + + if (!existing) { + if (embeddedId === null) return + const serverId = this.readServerRelatedId() + this.store.getOrCreateRelation(this.entityType, this.entityId, this.fieldName, { + currentId: embeddedId, + serverId, + state: 'connected', + serverState: serverId !== null ? 'connected' : 'disconnected', + placeholderData: {}, + }) + return } - // Fallback to entity snapshot data (for server-loaded data) - const parentSnapshot = this.store.getEntitySnapshot(this.entityType, this.entityId) - if (parentSnapshot?.data) { - const relatedData = (parentSnapshot.data as Record)[this.fieldName] - if (relatedData && typeof relatedData === 'object' && 'id' in relatedData) { - return (relatedData as { id: string }).id - } + this.advanceServerBaselineOnRefetch(existing, embeddedId) + } + + /** + * On a parent re-fetch (embedded reference changed) advances the relation's + * server baseline to the embedded related id, but only when the relation is + * not locally dirty — a local connect/disconnect/create must survive a re-fetch. + * + * Does NOT consume the propagation slot — {@link ensureRelatedEntitySnapshot} + * owns it. The same reference-change signal drives both, so both run within the + * one render that observes a new parent reference. + */ + private advanceServerBaselineOnRefetch( + existing: StoredRelationState, + embeddedId: string | null, + ): void { + if (embeddedId === null || existing.serverId === embeddedId) return + if (this.isLocallyDirty(existing)) return + + const embeddedData = this.readEmbeddedRelatedData() + if (!this.store.hasEmbeddedDataChanged(this.entityType, this.entityId, this.fieldName, embeddedData)) { + return } - return null + this.store.setRelation(this.entityType, this.entityId, this.fieldName, { + currentId: embeddedId, + serverId: embeddedId, + state: 'connected', + serverState: 'connected', + }) + } + + private isLocallyDirty(relation: StoredRelationState): boolean { + return ( + relation.currentId !== relation.serverId || + relation.state !== relation.serverState || + Object.keys(relation.placeholderData).length > 0 + ) + } + + /** Reads the embedded related object from the parent's canonical current data. */ + private readEmbeddedRelatedData(): unknown { + return this.getEntityData()?.[this.fieldName] + } + + /** Extracts the related id from the parent's embedded current data, or null. */ + private readEmbeddedRelatedId(): string | null { + return extractRelatedId(this.readEmbeddedRelatedData()) + } + + /** Extracts the related id from the parent's embedded server data, or null. */ + private readServerRelatedId(): string | null { + return extractRelatedId(this.getServerData()?.[this.fieldName]) } /** @@ -354,6 +427,7 @@ export class HasOneHandle * Checks if the relation is dirty. */ get isDirty(): boolean { + this.ensureEntry() const relation = this.store.getRelation( this.entityType, this.entityId, @@ -361,11 +435,7 @@ export class HasOneHandle ) if (!relation) return false - return ( - relation.currentId !== relation.serverId || - relation.state !== relation.serverState || - Object.keys(relation.placeholderData).length > 0 - ) + return this.isLocallyDirty(relation) } /** @@ -593,3 +663,13 @@ export class HasOneHandle } } + +/** + * Extracts the related entity id from an embedded has-one object, or null when + * the value is absent / not an object with a string id. + */ +function extractRelatedId(embedded: unknown): string | null { + if (!embedded || typeof embedded !== 'object' || !('id' in embedded)) return null + const id = embedded.id + return typeof id === 'string' ? id : null +} diff --git a/tests/unit/handles/hasOneHandle.test.ts b/tests/unit/handles/hasOneHandle.test.ts index feb0da2..686f358 100644 --- a/tests/unit/handles/hasOneHandle.test.ts +++ b/tests/unit/handles/hasOneHandle.test.ts @@ -676,4 +676,127 @@ describe('HasOneHandle', () => { expect(handle.__entityName).toBe('Author') }) }) + + // ==================== Eager Materialization (PR 6) ==================== + // + // RelationStore is the single source of truth for has-one: reading relatedId/ + // state materializes the entry from embedded snapshot data, and there is no + // longer a snapshot fallback in the getters. + + describe('Eager materialization', () => { + test('a loaded has-one materializes a RelationStore entry and is not dirty', () => { + store.setEntityData('Article', 'a-1', { + id: 'a-1', + title: 'Test', + author: { id: 'auth-1', name: 'John' }, + }, true) + + // No explicit setRelation — the entry only lives in embedded snapshot data. + expect(store.getRelation('Article', 'a-1', 'author')).toBeUndefined() + + const handle = createHasOneHandleRaw() + + // Reading state materializes the entry. + expect(handle.state).toBe('connected') + expect(handle.relatedId).toBe('auth-1') + + const relation = store.getRelation('Article', 'a-1', 'author') + expect(relation).toBeDefined() + expect(relation!.currentId).toBe('auth-1') + expect(relation!.serverId).toBe('auth-1') + expect(relation!.state).toBe('connected') + expect(relation!.serverState).toBe('connected') + + // A freshly materialized loaded relation must NOT be dirty. + expect(handle.isDirty).toBe(false) + }) + + test('relatedId reads purely from RelationStore (no snapshot fallback)', () => { + store.setEntityData('Article', 'a-1', { + id: 'a-1', + title: 'Test', + author: { id: 'auth-1', name: 'John' }, + }, true) + + const handle = createHasOneHandleRaw() + + // First read materializes the entry. + expect(handle.relatedId).toBe('auth-1') + + // Disconnect at the relation level only — the parent's embedded data is + // left untouched (still { id: 'auth-1', ... }). A snapshot fallback would + // resurrect 'auth-1'; reading purely from the store must report null. + store.setRelation('Article', 'a-1', 'author', { currentId: null, state: 'disconnected' }) + + const embedded = (store.getEntitySnapshot('Article', 'a-1')!.data as Record)['author'] + expect(embedded).toEqual({ id: 'auth-1', name: 'John' }) + + const handle2 = createHasOneHandleRaw() + expect(handle2.relatedId).toBeNull() + expect(handle2.state).toBe('disconnected') + }) + + test('disconnected (no embedded data, no entry) leaves the relation unmaterialized', () => { + store.setEntityData('Article', 'a-1', { id: 'a-1', title: 'Test' }, true) + + const handle = createHasOneHandleRaw() + expect(handle.relatedId).toBeNull() + expect(handle.state).toBe('disconnected') + + // No spurious entry created for an empty relation. + expect(store.getRelation('Article', 'a-1', 'author')).toBeUndefined() + }) + + test('materialization does not clobber a local connect', () => { + store.setEntityData('Article', 'a-1', { + id: 'a-1', + title: 'Test', + author: { id: 'auth-1', name: 'John' }, + }, true) + // Local connect to a different author than the embedded one. + store.setRelation('Article', 'a-1', 'author', { currentId: 'auth-2', state: 'connected' }) + + const handle = createHasOneHandleRaw() + + // The local connect survives — embedded 'auth-1' must not win. + expect(handle.relatedId).toBe('auth-2') + expect(handle.isDirty).toBe(true) + }) + }) + + // ==================== Reachability / dirty for created has-one child ==================== + + describe('Created has-one child via embedded data', () => { + test('a created child connected via has-one appears as a create without explicit setRelation', () => { + // Server parent. + store.setEntityData('Article', 'a-1', { id: 'a-1', title: 'Test' }, true) + + // User creates a new author (auto-rooted as a top-level create). + const childId = store.createEntity('Author', { name: 'Draft Author' }) + + // The connection lives ONLY in the parent's embedded current data — + // no store.setRelation() call. + store.setFieldValue('Article', 'a-1', ['author'], { id: childId, name: 'Draft Author' }) + + // Detach the auto-root so the child is reachable ONLY through the has-one + // edge once it is materialized (proves materialization drives reachability). + store.registerParentChild('Article', 'a-1', 'Author', childId) + store.unregisterRootEntity('Author', childId) + + // Before any handle read, the relation entry does not exist yet, so the + // child is not reachable through it. + expect(store.getRelation('Article', 'a-1', 'author')).toBeUndefined() + const beforeCreates = store.getAllDirtyEntities().filter(e => e.changeType === 'create') + expect(beforeCreates.map(e => e.entityId)).not.toContain(childId) + + // A handle read materializes the relation entry. + const handle = createHasOneHandleRaw() + expect(handle.relatedId).toBe(childId) + + // Now the created child is reachable via the materialized has-one edge and + // reported as a create — no explicit setRelation was ever called. + const creates = store.getAllDirtyEntities().filter(e => e.changeType === 'create') + expect(creates.map(e => e.entityId)).toContain(childId) + }) + }) }) From 6ec3260227e4d19a68fdb5d85c2cd79c37944a99 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 22 Jun 2026 20:59:25 +0200 Subject: [PATCH 11/19] refactor(bindx): derive parent notification from relation edges; remove childToParents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parent re-render propagation used a separate append-only childToParents registry in SubscriptionManager, populated on every render via registerParentChild and never cleaned on disconnect (a leak). RelationStore is now the single source of truth for relation membership, so derive 'which parents reference this child' from its live edges instead. - Add RelationStore.getParentKeysForChild(childId): an on-demand scan of the live has-one/has-many edges, with liveness matching getLiveChildIds exactly. Chosen over an incremental reverse index: correct by construction (reads the same maps as the forward query), so nothing can drift on disconnect/rekey/clear. - Inject it into SubscriptionManager via a small ParentKeyLookup interface, mirroring SnapshotVersionBumper. notifyEntitySubscribers derives parents from it, preserving the recursion + cycle guard and the parent snapshot bump. - Remove childToParents, registerParentChild/unregisterParentChild bodies, and their migration in rekey(). SnapshotStore.registerParentChild keeps only the load-bearing roots.unregister(childKey) side effect; unregisterParentChild is deleted. Render-time callers (handles, ActionDispatcher) are unchanged and now trigger only the root-unregister half — the relation edge they already set up is the notification source. A disconnected child no longer notifies its former parent (the leak is gone). Tests: flip the harness disconnect scenario to the no-leak behavior; update the notification harness diamond and the rekey/snapshotStore/actionDispatcher tests to establish real relation edges (the new notification source) instead of bare registerParentChild calls. Add getParentKeysForChild.test.ts with a randomized cross-check proving the reverse query always agrees with getLiveChildIds. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- packages/bindx/src/store/RelationStore.ts | 55 +++++++ packages/bindx/src/store/SnapshotStore.ts | 25 +-- .../bindx/src/store/SubscriptionManager.ts | 90 ++++++----- tests/subscriptionRekey.test.ts | 29 +++- tests/unit/core/actionDispatcher.test.ts | 14 +- .../unit/store/getParentKeysForChild.test.ts | 142 ++++++++++++++++++ .../store/notificationPropagation.test.ts | 21 ++- tests/unit/store/snapshotStore.test.ts | 11 +- 8 files changed, 311 insertions(+), 76 deletions(-) create mode 100644 tests/unit/store/getParentKeysForChild.test.ts diff --git a/packages/bindx/src/store/RelationStore.ts b/packages/bindx/src/store/RelationStore.ts index b912b55..870324a 100644 --- a/packages/bindx/src/store/RelationStore.ts +++ b/packages/bindx/src/store/RelationStore.ts @@ -495,6 +495,50 @@ export class RelationStore implements Rekeyable { return Array.from(ids) } + /** + * Collects the composite keys ("parentType:parentId") of every entity that + * currently has a LIVE relation edge pointing at {@link childId}. This is the + * reverse of {@link getLiveChildIds}: instead of "which children does this + * parent reach", it answers "which parents reference this child". + * + * It is the single source of truth for parent re-render notification — when a + * child entity's own field changes, every parent returned here is notified so + * its rendered view of the child updates. + * + * Liveness matches {@link getLiveChildIds} EXACTLY so the two never disagree: + * - has-one: currentId === childId, when the relation is not 'deleted' + * (a disconnected relation has a null currentId and is skipped); + * - has-many: childId ∈ (serverIds ∪ plannedConnections ∪ createdEntities) + * and childId ∉ plannedRemovals. + * + * Implemented as an on-demand scan of the live edges rather than an + * incrementally-maintained reverse index: it is correct by construction (it + * reads the same maps the forward query reads), so there is no second + * structure that could drift out of sync on disconnect/remove/rekey/clear. + */ + getParentKeysForChild(childId: string): Set { + const parents = new Set() + + for (const [key, state] of this.relationStates) { + if (state.currentId === childId && state.state !== 'deleted') { + parents.add(parentKeyFromRelationKey(key)) + } + } + + for (const [key, state] of this.hasManyStates) { + if (state.plannedRemovals.has(childId)) continue + if ( + state.serverIds.has(childId) || + state.plannedConnections.has(childId) || + state.createdEntities.has(childId) + ) { + parents.add(parentKeyFromRelationKey(key)) + } + } + + return parents + } + /** * Removes all relation and has-many state owned by an entity (keys under the * given owner prefix). Called by removeEntity so a removed entity leaves no @@ -836,6 +880,17 @@ export class RelationStore implements Rekeyable { // ==================== Helper Functions ==================== +/** + * Derives the parent composite key ("parentType:parentId") from a relation key + * ("parentType:parentId:fieldName") by dropping the trailing field segment. + * Entity ids and field names never contain ':', so the parent key is everything + * before the last separator. + */ +function parentKeyFromRelationKey(relationKey: string): string { + const lastSeparator = relationKey.lastIndexOf(':') + return relationKey.slice(0, lastSeparator) +} + /** * Computes the default ordered IDs for a has-many relation. * Order is: serverIds (minus removals) + plannedConnections diff --git a/packages/bindx/src/store/SnapshotStore.ts b/packages/bindx/src/store/SnapshotStore.ts index 7727aad..1353201 100644 --- a/packages/bindx/src/store/SnapshotStore.ts +++ b/packages/bindx/src/store/SnapshotStore.ts @@ -64,6 +64,10 @@ export class SnapshotStore implements SnapshotVersionBumper { constructor() { this.reachability = new ReachabilityAnalyzer(this.entitySnapshots, this.meta, this.relations, this.roots) this.dirtyTracker = new DirtyTracker(this.entitySnapshots, this.meta, this.relations, this.reachability) + // Parent re-render propagation is derived from the relation store's live + // edges — the single source of truth for relation membership — rather than + // a separate parent-child registry. + this.subscriptions.setParentKeyLookup(this.relations) // The participants are visited in this exact order on every rekey — see the // ordering contract in RekeyOrchestrator.rekey(). Propagation tracking lives // on this store, so it joins as a small inline adapter. @@ -823,21 +827,22 @@ export class SnapshotStore implements SnapshotVersionBumper { // ==================== Parent-Child Relationships ==================== + /** + * Anchors a child under a parent relation. Parent re-render notification is now + * DERIVED from the relation store's live edges (see + * {@link SubscriptionManager.getParentKeys}), so the only remaining effect is + * the reachability one: a child anchored by a parent relation is no longer a + * top-level root; its reachability flows through the parent. (No-op for server + * children that were never roots.) + * + * Callers (handles, the action dispatcher) keep calling this on connect so the + * root-unregister stays centralized in one place. + */ registerParentChild(parentType: string, parentId: string, childType: string, childId: string): void { - const parentKey = this.getEntityKey(parentType, parentId) const childKey = this.getEntityKey(childType, childId) - this.subscriptions.registerParentChild(parentKey, childKey) - // A child anchored by a parent relation is no longer a top-level root; its - // reachability now flows through the parent. (No-op for server children.) this.roots.unregister(childKey) } - unregisterParentChild(parentType: string, parentId: string, childType: string, childId: string): void { - const parentKey = this.getEntityKey(parentType, parentId) - const childKey = this.getEntityKey(childType, childId) - this.subscriptions.unregisterParentChild(parentKey, childKey) - } - // ==================== Partial Snapshot Export/Import ==================== exportPartialSnapshot(keys: { diff --git a/packages/bindx/src/store/SubscriptionManager.ts b/packages/bindx/src/store/SubscriptionManager.ts index b4ef7c1..a44780b 100644 --- a/packages/bindx/src/store/SubscriptionManager.ts +++ b/packages/bindx/src/store/SubscriptionManager.ts @@ -10,6 +10,16 @@ export interface SnapshotVersionBumper { bumpEntitySnapshotVersion(key: string): void } +/** + * Resolves the parent entity keys that currently have a LIVE relation edge to a + * child, so a child-field change can propagate up to its parents. Implemented by + * the relation store (the single source of truth for relation membership); this + * interface keeps {@link SubscriptionManager} decoupled from its concrete type. + */ +export interface ParentKeyLookup { + getParentKeysForChild(childId: string): Set +} + /** * Manages subscriptions for entity and relation changes. * @@ -17,7 +27,7 @@ export interface SnapshotVersionBumper { * - Entity-level subscriptions * - Relation-level subscriptions * - Global subscriptions (any change) - * - Parent-child change propagation + * - Parent-child change propagation (derived from live relation edges) * - Global version tracking for change detection */ export class SubscriptionManager implements Rekeyable { @@ -30,15 +40,19 @@ export class SubscriptionManager implements Rekeyable { /** Global subscribers (notified on any change) */ private readonly globalSubscribers = new Set() - /** Parent-child relationships: childKey -> Set of parentKeys */ - private readonly childToParents = new Map>() - /** Maps old keys → new keys after rekey, so unsubscribe closures can find migrated callbacks */ private readonly rekeyedKeys = new Map() /** Global version number for change detection */ private globalVersion = 0 + /** + * Resolves a child's parents from live relation edges. Injected after + * construction (the relation store and this manager are siblings under + * SnapshotStore) via {@link setParentKeyLookup}. + */ + private parentKeyLookup: ParentKeyLookup | undefined + /** * Resolves a key through the rekey redirect chain. * The rekey() method collapses chains (A→C instead of A→B→C), so this should @@ -121,29 +135,25 @@ export class SubscriptionManager implements Rekeyable { // ==================== Parent-Child Relationships ==================== /** - * Registers a parent-child relationship for change propagation. - * When the child entity changes, parent entity subscribers will be notified. + * Wires the live-edge parent lookup. Parent re-render propagation is derived + * from the relation store's edges rather than a separate registry, so there is + * one source of truth for the parent-child graph. */ - registerParentChild(parentKey: string, childKey: string): void { - let parents = this.childToParents.get(childKey) - if (!parents) { - parents = new Set() - this.childToParents.set(childKey, parents) - } - parents.add(parentKey) + setParentKeyLookup(lookup: ParentKeyLookup): void { + this.parentKeyLookup = lookup } /** - * Unregisters a parent-child relationship. + * Derives the parent entity keys for a child entity key by reading the live + * relation edges via the injected lookup. The child's bare id is the part of + * the key after the first ':' ("entityType:id"). */ - unregisterParentChild(parentKey: string, childKey: string): void { - const parents = this.childToParents.get(childKey) - if (parents) { - parents.delete(parentKey) - if (parents.size === 0) { - this.childToParents.delete(childKey) - } - } + private getParentKeys(childKey: string): Set { + if (!this.parentKeyLookup) return new Set() + const separator = childKey.indexOf(':') + if (separator === -1) return new Set() + const childId = childKey.slice(separator + 1) + return this.parentKeyLookup.getParentKeysForChild(childId) } // ==================== Notification ==================== @@ -171,14 +181,14 @@ export class SubscriptionManager implements Rekeyable { } } - // Notify parent entity subscribers (propagate change up the tree) - const parents = this.childToParents.get(key) - if (parents) { - for (const parentKey of parents) { - // Bump parent snapshot version so useSyncExternalStore detects a change - bumper.bumpEntitySnapshotVersion(parentKey) - this.notifyEntitySubscribers(parentKey, bumper, notifiedKeys) - } + // Notify parent entity subscribers (propagate change up the tree). + // Parents are derived from the relation store's LIVE edges, so a + // disconnected child no longer reaches its former parent. + const parents = this.getParentKeys(key) + for (const parentKey of parents) { + // Bump parent snapshot version so useSyncExternalStore detects a change + bumper.bumpEntitySnapshotVersion(parentKey) + this.notifyEntitySubscribers(parentKey, bumper, notifiedKeys) } // Notify global subscribers (only once, not for each parent) @@ -263,9 +273,12 @@ export class SubscriptionManager implements Rekeyable { } /** - * Moves subscriptions and parent-child relationships from oldKey to newKey. + * Moves entity and relation subscriptions from oldKey to newKey. * Also rekeys relation subscribers under oldKeyPrefix to newKeyPrefix. * Registers redirects so unsubscribe closures can find migrated callbacks. + * + * Parent-child links are NOT migrated here — they are derived from the + * relation store's live edges, which migrate their own id references on rekey. */ rekey(ctx: RekeyContext): void { const { oldKey, newKey, oldKeyPrefix, newKeyPrefix } = ctx @@ -305,20 +318,5 @@ export class SubscriptionManager implements Rekeyable { this.relationSubscribers.delete(oldRelKey) this.relationSubscribers.set(newRelKey, subs) } - - // Move parent-child: update child→parent mappings - const parents = this.childToParents.get(oldKey) - if (parents) { - this.childToParents.delete(oldKey) - this.childToParents.set(newKey, parents) - } - - // Update parent-child: replace oldKey in any parent sets that reference it - for (const parentSet of this.childToParents.values()) { - if (parentSet.has(oldKey)) { - parentSet.delete(oldKey) - parentSet.add(newKey) - } - } } } diff --git a/tests/subscriptionRekey.test.ts b/tests/subscriptionRekey.test.ts index 60baadb..d0d747d 100644 --- a/tests/subscriptionRekey.test.ts +++ b/tests/subscriptionRekey.test.ts @@ -213,7 +213,8 @@ describe('Subscription migration after mapTempIdToPersistedId', () => { const parentId = store.createEntity('Author', { name: 'John' }) const childId = store.createEntity('Article', { title: 'Draft' }) - // Register parent-child before mapping + // Establish a live relation edge — parent notification is derived from it. + store.setRelation('Author', parentId, 'featuredArticle', { currentId: childId, state: 'connected' }) store.registerParentChild('Author', parentId, 'Article', childId) const parentCallback = mock(() => {}) @@ -243,7 +244,11 @@ describe('Subscription migration after mapTempIdToPersistedId', () => { const roundId = store.createEntity('Round', { roundNumber: 1 }) const reviewId = store.createEntity('Review', { comment: 'initial' }) - // Register parent-child chain (simulates what handles do during render) + // Establish live relation edges down the chain — parent notification is + // derived from these. registerParentChild also un-roots each anchored child. + store.setRelation('Program', 'prog-1', 'approval', { currentId: approvalId, state: 'connected' }) + store.setRelation('Approval', approvalId, 'round', { currentId: roundId, state: 'connected' }) + store.setRelation('Round', roundId, 'review', { currentId: reviewId, state: 'connected' }) store.registerParentChild('Program', 'prog-1', 'Approval', approvalId) store.registerParentChild('Approval', approvalId, 'Round', roundId) store.registerParentChild('Round', roundId, 'Review', reviewId) @@ -275,6 +280,9 @@ describe('Subscription migration after mapTempIdToPersistedId', () => { const roundId = store.createEntity('Round', { roundNumber: 1 }) const reviewId = store.createEntity('Review', { comment: 'initial' }) + store.setRelation('Program', 'prog-1', 'approval', { currentId: approvalId, state: 'connected' }) + store.setRelation('Approval', approvalId, 'round', { currentId: roundId, state: 'connected' }) + store.setRelation('Round', roundId, 'review', { currentId: reviewId, state: 'connected' }) store.registerParentChild('Program', 'prog-1', 'Approval', approvalId) store.registerParentChild('Approval', approvalId, 'Round', roundId) store.registerParentChild('Round', roundId, 'Review', reviewId) @@ -307,7 +315,9 @@ describe('Subscription migration after mapTempIdToPersistedId', () => { const approvalId = store.createEntity('Approval', { status: 'pending' }) const roundId = store.createEntity('Round', { roundNumber: 1 }) - // Initial parent-child registration + // Initial relation edges + parent-child registration + store.setRelation('Program', 'prog-1', 'approval', { currentId: approvalId, state: 'connected' }) + store.setRelation('Approval', approvalId, 'round', { currentId: roundId, state: 'connected' }) store.registerParentChild('Program', 'prog-1', 'Approval', approvalId) store.registerParentChild('Approval', approvalId, 'Round', roundId) @@ -338,12 +348,15 @@ describe('Subscription migration after mapTempIdToPersistedId', () => { const roundId = store.createEntity('Round', { roundNumber: 1 }) const reviewId = store.createEntity('Review', { comment: 'init' }) - // Rekey WITHOUT any prior registerParentChild + // Rekey WITHOUT any prior relation edge / registerParentChild store.mapTempIdToPersistedId('Round', roundId, 'uuid-round') store.mapTempIdToPersistedId('Review', reviewId, 'uuid-review') - // Now handles register parent-child (during render AFTER persist) - // using temp IDs — getEntityKey resolves them + // Now handles establish the relation edges + register parent-child (during + // render AFTER persist) using temp IDs — getRelationKey/getEntityKey resolve + // them to the persisted keys/ids, so notification follows the live edges. + store.setRelation('Program', 'prog-1', 'round', { currentId: 'uuid-round', state: 'connected' }) + store.setRelation('Round', roundId, 'review', { currentId: 'uuid-review', state: 'connected' }) store.registerParentChild('Program', 'prog-1', 'Round', roundId) store.registerParentChild('Round', roundId, 'Review', reviewId) @@ -366,6 +379,10 @@ describe('Subscription migration after mapTempIdToPersistedId', () => { const parentId = store.createEntity('Parent', { data: 1 }) const childId = store.createEntity('Child', { data: 2 }) + // Live relation edges drive notification; replaceEntityId migrates the stored + // currentId references when each end is rekeyed, in either order. + store.setRelation('Root', 'root-1', 'child', { currentId: parentId, state: 'connected' }) + store.setRelation('Parent', parentId, 'child', { currentId: childId, state: 'connected' }) store.registerParentChild('Root', 'root-1', 'Parent', parentId) store.registerParentChild('Parent', parentId, 'Child', childId) diff --git a/tests/unit/core/actionDispatcher.test.ts b/tests/unit/core/actionDispatcher.test.ts index d1a8adb..30fc58e 100644 --- a/tests/unit/core/actionDispatcher.test.ts +++ b/tests/unit/core/actionDispatcher.test.ts @@ -704,20 +704,28 @@ describe('ActionDispatcher', () => { expect(parentCallback).toHaveBeenCalled() }) - test('store.addToHasMany alone does NOT register parent-child', () => { + test('store.addToHasMany alone propagates: the live edge is the notification source', () => { store.setEntityData('Article', 'a-1', { id: 'a-1', title: 'Test' }, true) store.getOrCreateHasMany('Article', 'a-1', 'tags') store.setEntityData('Tag', 't-1', { id: 't-1', name: 'JS' }, true) - // Use low-level store API directly (no dispatcher) + // Use low-level store API directly (no dispatcher). The has-many edge IS the + // single source of truth for parent notification — there is no separate + // parent-child registry to populate. store.addToHasMany('Article', 'a-1', 'tags', 't-1') const parentCallback = mock(() => {}) store.subscribeToEntity('Article', 'a-1', parentCallback) parentCallback.mockClear() - // Modifying the child should NOT propagate — no parent-child registered + // Modifying the child propagates to the parent via the live relation edge. store.setFieldValue('Tag', 't-1', ['name'], 'TypeScript') + expect(parentCallback).toHaveBeenCalled() + + // Removing the edge severs propagation. + store.removeFromHasMany('Article', 'a-1', 'tags', 't-1', 'disconnect') + parentCallback.mockClear() + store.setFieldValue('Tag', 't-1', ['name'], 'JavaScript') expect(parentCallback).not.toHaveBeenCalled() }) }) diff --git a/tests/unit/store/getParentKeysForChild.test.ts b/tests/unit/store/getParentKeysForChild.test.ts new file mode 100644 index 0000000..8debbb1 --- /dev/null +++ b/tests/unit/store/getParentKeysForChild.test.ts @@ -0,0 +1,142 @@ +import { describe, test, expect } from 'bun:test' +import { RelationStore } from '../../../packages/bindx/src/store/RelationStore.js' + +/** + * Behavioral + cross-check tests for {@link RelationStore.getParentKeysForChild}, + * the reverse query that powers parent re-render notification after the + * append-only `childToParents` registry was removed. + * + * The cross-check proves the reverse query agrees with the forward query + * ({@link RelationStore.getLiveChildIds}): a child appears under a parent iff that + * parent's live children include the child. Since both read the same maps with + * the same liveness rules, they must never disagree — the randomized sequence is + * the safety net guarding that invariant. + */ +describe('RelationStore.getParentKeysForChild', () => { + test('has-one connect returns the parent; disconnect removes it', () => { + const relations = new RelationStore() + + relations.setRelation( + 'Author:a1:featured', + { currentId: 'art1', state: 'connected' }, + undefined, + 'featured', + ) + + expect(relations.getParentKeysForChild('art1')).toEqual(new Set(['Author:a1'])) + + relations.setRelation( + 'Author:a1:featured', + { currentId: null, state: 'disconnected' }, + undefined, + 'featured', + ) + + expect(relations.getParentKeysForChild('art1')).toEqual(new Set()) + }) + + test('has-one in deleted state does not anchor its target', () => { + const relations = new RelationStore() + + relations.setRelation( + 'Author:a1:featured', + { currentId: 'art1', state: 'deleted' }, + undefined, + 'featured', + ) + + expect(relations.getParentKeysForChild('art1')).toEqual(new Set()) + }) + + test('has-many add returns the parent; remove (disconnect) removes it', () => { + const relations = new RelationStore() + + relations.addToHasMany('Author:a1:articles', 'art1') + expect(relations.getParentKeysForChild('art1')).toEqual(new Set(['Author:a1'])) + + relations.removeFromHasMany('Author:a1:articles', 'art1', 'disconnect') + expect(relations.getParentKeysForChild('art1')).toEqual(new Set()) + }) + + test('has-many server item is a live parent until removed', () => { + const relations = new RelationStore() + + relations.setHasManyServerIds('Author:a1:articles', ['art1', 'art2']) + expect(relations.getParentKeysForChild('art1')).toEqual(new Set(['Author:a1'])) + + relations.planHasManyRemoval('Author:a1:articles', 'art1', 'delete') + expect(relations.getParentKeysForChild('art1')).toEqual(new Set()) + }) + + test('diamond: a shared child returns both parents', () => { + const relations = new RelationStore() + + relations.setRelation( + 'Author:a1:featured', + { currentId: 'art1', state: 'connected' }, + undefined, + 'featured', + ) + relations.addToHasMany('Tag:t1:articles', 'art1') + + expect(relations.getParentKeysForChild('art1')).toEqual(new Set(['Author:a1', 'Tag:t1'])) + }) + + test('cross-check: reverse query agrees with getLiveChildIds over a randomized sequence', () => { + const relations = new RelationStore() + const parents = ['Author:a1', 'Author:a2', 'Tag:t1', 'Tag:t2'] + const children = ['art1', 'art2', 'art3', 'art4'] + + // Deterministic pseudo-random sequence (no flakiness, full reproducibility). + let seed = 0x1234abcd + const rand = (n: number): number => { + seed = (seed * 1103515245 + 12345) & 0x7fffffff + return seed % n + } + + const pick = (list: readonly T[]): T => list[rand(list.length)]! + + for (let step = 0; step < 400; step++) { + const parent = pick(parents) + const child = pick(children) + const hasOneKey = `${parent}:featured` + const hasManyKey = `${parent}:articles` + + switch (rand(7)) { + case 0: + relations.setRelation(hasOneKey, { currentId: child, state: 'connected' }, undefined, 'featured') + break + case 1: + relations.setRelation(hasOneKey, { currentId: null, state: 'disconnected' }, undefined, 'featured') + break + case 2: + relations.setRelation(hasOneKey, { currentId: child, state: 'deleted' }, undefined, 'featured') + break + case 3: + relations.addToHasMany(hasManyKey, child) + break + case 4: + relations.planHasManyConnection(hasManyKey, child) + break + case 5: + relations.removeFromHasMany(hasManyKey, child, 'disconnect') + break + case 6: + relations.setHasManyServerIds(hasManyKey, [child]) + break + } + + // Forward-derived expectation: a parent should appear for `child` iff + // `child` is among that parent's live children. + for (const c of children) { + const expected = new Set() + for (const p of parents) { + if (relations.getLiveChildIds(`${p}:`).includes(c)) { + expected.add(p) + } + } + expect(relations.getParentKeysForChild(c)).toEqual(expected) + } + } + }) +}) diff --git a/tests/unit/store/notificationPropagation.test.ts b/tests/unit/store/notificationPropagation.test.ts index edd6109..e4e2b2d 100644 --- a/tests/unit/store/notificationPropagation.test.ts +++ b/tests/unit/store/notificationPropagation.test.ts @@ -73,7 +73,7 @@ describe('Notification propagation', () => { expect(parent.callCount()).toBe(1) }) - test('disconnect still notifies the former parent (current append-only behavior)', () => { + test('disconnect stops notifying the former parent', () => { // Server parent A with a child B connected via has-one. store.setEntityData('Author', 'author-1', { id: 'author-1', name: 'Alice' }, true) store.setEntityData('Article', 'article-1', { id: 'article-1', title: 'Draft' }, true) @@ -91,8 +91,8 @@ describe('Notification propagation', () => { }) store.registerParentChild('Author', 'author-1', 'Article', 'article-1') - // "Disconnect" the child: clear the relation. Note that a relation disconnect - // does NOT call unregisterParentChild, so the parent link survives. + // "Disconnect" the child: clear the relation edge. Parent notification is now + // derived from the live relation edge, so clearing it severs the link. store.setRelation('Author', 'author-1', 'featuredArticle', { currentId: null, state: 'disconnected', @@ -105,17 +105,24 @@ describe('Notification propagation', () => { // Mutate the now-disconnected child. store.setFieldValue('Article', 'article-1', ['title'], 'Updated') - // LEAK: PR 7 will make a disconnected child stop notifying its former parent; - // this assertion will flip then. - expect(parent.callCount()).toBe(1) + // The former parent must NOT be notified — there is no live edge to it. + expect(parent.callCount()).toBe(0) }) test('diamond: a shared child notifies both parents', () => { - // Child B connected to TWO parents A1 and A2. + // Child B connected to TWO parents A1 and A2 via live relation edges. store.setEntityData('Author', 'author-1', { id: 'author-1', name: 'Alice' }, true) store.setEntityData('Author', 'author-2', { id: 'author-2', name: 'Bob' }, true) store.setEntityData('Article', 'article-1', { id: 'article-1', title: 'Draft' }, true) + store.setRelation('Author', 'author-1', 'featuredArticle', { + currentId: 'article-1', + state: 'connected', + }) + store.setRelation('Author', 'author-2', 'featuredArticle', { + currentId: 'article-1', + state: 'connected', + }) store.registerParentChild('Author', 'author-1', 'Article', 'article-1') store.registerParentChild('Author', 'author-2', 'Article', 'article-1') diff --git a/tests/unit/store/snapshotStore.test.ts b/tests/unit/store/snapshotStore.test.ts index 2cd7cd4..a334f8d 100644 --- a/tests/unit/store/snapshotStore.test.ts +++ b/tests/unit/store/snapshotStore.test.ts @@ -1109,6 +1109,8 @@ describe('SnapshotStore', () => { store.setEntityData('Article', 'a-1', { id: 'a-1', title: 'Test' }, true) store.setEntityData('Author', 'auth-1', { id: 'auth-1', name: 'John' }, true) + // Parent notification is derived from the live relation edge. + store.setRelation('Article', 'a-1', 'author', { currentId: 'auth-1', state: 'connected' }) store.registerParentChild('Article', 'a-1', 'Author', 'auth-1') store.subscribeToEntity('Article', 'a-1', subscriber.fn) @@ -1117,18 +1119,19 @@ describe('SnapshotStore', () => { expect(subscriber.callCount()).toBeGreaterThan(0) }) - test('should unregister parent-child relationship', () => { + test('should stop propagating after the relation edge is disconnected', () => { const subscriber = createMockSubscriber() store.setEntityData('Article', 'a-1', { id: 'a-1', title: 'Test' }, true) store.setEntityData('Author', 'auth-1', { id: 'auth-1', name: 'John' }, true) - store.registerParentChild('Article', 'a-1', 'Author', 'auth-1') - store.unregisterParentChild('Article', 'a-1', 'Author', 'auth-1') + store.setRelation('Article', 'a-1', 'author', { currentId: 'auth-1', state: 'connected' }) + // Disconnecting the edge severs parent notification (no separate registry). + store.setRelation('Article', 'a-1', 'author', { currentId: null, state: 'disconnected' }) store.subscribeToEntity('Article', 'a-1', subscriber.fn) store.setFieldValue('Author', 'auth-1', ['name'], 'Jane') - // Parent should not be notified after unregistering + // Parent should not be notified — there is no live edge to it. expect(subscriber.callCount()).toBe(0) }) }) From b20584113edcad5b16decbaf36fca23315f518aa Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 22 Jun 2026 21:10:08 +0200 Subject: [PATCH 12/19] refactor(bindx): collapse has-many planned-addition sets into one map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace StoredHasManyState's plannedConnections: Set and createdEntities: Set with a single plannedAdditions: Map. The map keys are exactly the old plannedConnections; keys whose value is 'created' are exactly the old createdEntities, making the "createdEntities ⊆ plannedConnections" invariant structural and removing one mutable field (5→4). add() records 'created', connect()/embedded-connect record 'connected' without downgrading an existing 'created' entry. removeFromHasMany branches on get(id) === 'created'. getLiveChildIds, getParentKeysForChild, dirty tracking, commit, rekey, export/import and computeDefaultOrderedIds updated to the new field. SnapshotStore accessors keep their Set/boolean return types so external callers are unaffected; MutationCollector emits create vs connect from the kind. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- .../bindx/src/handles/HasManyListHandle.ts | 3 +- .../src/persistence/MutationCollector.ts | 25 ++- packages/bindx/src/store/RelationStore.ts | 154 ++++++++---------- packages/bindx/src/store/SnapshotStore.ts | 14 +- tests/unit/core/actionDispatcher.test.ts | 2 +- tests/unit/handles/hasManyAlias.test.ts | 6 +- tests/unit/handles/hasManyHandle.test.ts | 20 +-- tests/unit/persistence/rollback.test.ts | 4 +- tests/unit/store/snapshotStore.test.ts | 28 ++-- 9 files changed, 121 insertions(+), 135 deletions(-) diff --git a/packages/bindx/src/handles/HasManyListHandle.ts b/packages/bindx/src/handles/HasManyListHandle.ts index 6ab29ce..ae03a7f 100644 --- a/packages/bindx/src/handles/HasManyListHandle.ts +++ b/packages/bindx/src/handles/HasManyListHandle.ts @@ -322,8 +322,7 @@ export class HasManyListHandle 0 || - state.plannedConnections.size > 0 || - state.createdEntities.size > 0 || + state.plannedAdditions.size > 0 || state.orderedIds !== null ) } diff --git a/packages/bindx/src/persistence/MutationCollector.ts b/packages/bindx/src/persistence/MutationCollector.ts index 5d04bc5..bac4b47 100644 --- a/packages/bindx/src/persistence/MutationCollector.ts +++ b/packages/bindx/src/persistence/MutationCollector.ts @@ -472,22 +472,19 @@ export class MutationCollector implements MutationDataCollector { } } - // Created entities -> create (using collectCreateData for full recursive collection) - if (targetType) { - for (const tempId of hasManyState.createdEntities) { - this._nestedEntityIds.add(tempId) - this._nestedEntityTypes.set(tempId, targetType) - const createData = this.collectCreateData(targetType, tempId) - operations.push({ create: createData ?? {}, alias: tempId }) + // Planned additions -> create (newly created) or connect (existing persisted) + for (const [additionId, kind] of hasManyState.plannedAdditions) { + if (kind === 'created') { + if (!targetType) continue + this._nestedEntityIds.add(additionId) + this._nestedEntityTypes.set(additionId, targetType) + const createData = this.collectCreateData(targetType, additionId) + operations.push({ create: createData ?? {}, alias: additionId }) + } else { + operations.push({ connect: { id: additionId }, alias: additionId }) } } - // Planned connections (minus created entities) -> connect - for (const connectedId of hasManyState.plannedConnections) { - if (hasManyState.createdEntities.has(connectedId)) continue - operations.push({ connect: { id: connectedId }, alias: connectedId }) - } - // Server items that aren't removed -> check for updates via entity snapshots if (targetType) { for (const itemId of hasManyState.serverIds) { @@ -723,7 +720,7 @@ export class MutationCollector implements MutationDataCollector { // Skip if store already has managed entities for this relation const existing = this.store.getHasMany(entityType, entityId, fieldName) - if (existing && (existing.createdEntities.size > 0 || existing.plannedConnections.size > 0)) { + if (existing && existing.plannedAdditions.size > 0) { return } diff --git a/packages/bindx/src/store/RelationStore.ts b/packages/bindx/src/store/RelationStore.ts index 870324a..a03de25 100644 --- a/packages/bindx/src/store/RelationStore.ts +++ b/packages/bindx/src/store/RelationStore.ts @@ -15,20 +15,31 @@ function setsEqual(a: Set, b: Set): boolean { */ export type HasManyRemovalType = 'disconnect' | 'delete' +/** + * Kind of a planned has-many addition: + * - 'created': a newly created entity (via add()) + * - 'connected': an existing persisted entity being connected (via connect()) + */ +export type HasManyAdditionKind = 'created' | 'connected' + /** * Has-many list state stored in SnapshotStore */ export interface StoredHasManyState { /** IDs of items from server */ serverIds: Set - /** Explicit ordered list of item IDs, null means use default order (serverIds + plannedConnections) */ + /** Explicit ordered list of item IDs, null means use default order (serverIds + plannedAdditions) */ orderedIds: string[] | null /** Planned removals (disconnect or delete) keyed by entity ID */ plannedRemovals: Map - /** Planned connections (IDs to add to the list) */ - plannedConnections: Set - /** Entity IDs created via add() - tracked for proper remove() semantics and mutation generation */ - createdEntities: Set + /** + * Planned additions (IDs to add to the list) keyed by entity ID, with the + * value distinguishing newly CREATED entities (add()) from existing PERSISTED + * entities being CONNECTED (connect()). The keys are exactly the connections; + * the keys whose value is 'created' are exactly the created entities, so the + * "created ⊆ connections" invariant is structural. + */ + plannedAdditions: Map version: number } @@ -188,8 +199,7 @@ export class RelationStore implements Rekeyable { serverIds: new Set(serverIds ?? []), orderedIds: null, plannedRemovals: new Map(), - plannedConnections: new Set(), - createdEntities: new Set(), + plannedAdditions: new Map(), version: 0, }) } else if (serverIds !== undefined) { @@ -225,8 +235,7 @@ export class RelationStore implements Rekeyable { serverIds: new Set(serverIds), orderedIds: null, plannedRemovals: new Map(), - plannedConnections: new Set(), - createdEntities: new Set(), + plannedAdditions: new Map(), version: 0, }) } else { @@ -250,27 +259,23 @@ export class RelationStore implements Rekeyable { serverIds: new Set(), orderedIds: null, plannedRemovals: new Map([[itemId, type]]), - plannedConnections: new Set(), - createdEntities: new Set(), + plannedAdditions: new Map(), version: 0, }) } else { const newPlannedRemovals = new Map(existing.plannedRemovals) newPlannedRemovals.set(itemId, type) - const newPlannedConnections = new Set(existing.plannedConnections) - newPlannedConnections.delete(itemId) + const newPlannedAdditions = new Map(existing.plannedAdditions) + newPlannedAdditions.delete(itemId) let newOrderedIds = existing.orderedIds if (newOrderedIds !== null) { newOrderedIds = newOrderedIds.filter(id => id !== itemId) } - const newCreatedEntities = new Set(existing.createdEntities) - newCreatedEntities.delete(itemId) this.writeHasMany(key, { ...existing, orderedIds: newOrderedIds, plannedRemovals: newPlannedRemovals, - plannedConnections: newPlannedConnections, - createdEntities: newCreatedEntities, + plannedAdditions: newPlannedAdditions, version: existing.version + 1, }) } @@ -287,13 +292,15 @@ export class RelationStore implements Rekeyable { serverIds: new Set(), orderedIds: null, plannedRemovals: new Map(), - plannedConnections: new Set([itemId]), - createdEntities: new Set(), + plannedAdditions: new Map([[itemId, 'connected']]), version: 0, }) } else { - const newPlannedConnections = new Set(existing.plannedConnections) - newPlannedConnections.add(itemId) + const newPlannedAdditions = new Map(existing.plannedAdditions) + // Do not downgrade an existing 'created' addition to 'connected'. + if (newPlannedAdditions.get(itemId) !== 'created') { + newPlannedAdditions.set(itemId, 'connected') + } const newPlannedRemovals = new Map(existing.plannedRemovals) newPlannedRemovals.delete(itemId) let newOrderedIds = existing.orderedIds @@ -303,7 +310,7 @@ export class RelationStore implements Rekeyable { this.writeHasMany(key, { ...existing, orderedIds: newOrderedIds, - plannedConnections: newPlannedConnections, + plannedAdditions: newPlannedAdditions, plannedRemovals: newPlannedRemovals, version: existing.version + 1, }) @@ -320,8 +327,7 @@ export class RelationStore implements Rekeyable { serverIds: new Set(newServerIds), orderedIds: null, plannedRemovals: new Map(), - plannedConnections: new Set(), - createdEntities: new Set(), + plannedAdditions: new Map(), version: (existing?.version ?? 0) + 1, }) } @@ -337,8 +343,7 @@ export class RelationStore implements Rekeyable { serverIds: existing.serverIds, orderedIds: null, plannedRemovals: new Map(), - plannedConnections: new Set(), - createdEntities: new Set(), + plannedAdditions: new Map(), version: existing.version + 1, }) } @@ -355,22 +360,18 @@ export class RelationStore implements Rekeyable { serverIds: new Set(), orderedIds: [itemId], plannedRemovals: new Map(), - plannedConnections: new Set([itemId]), - createdEntities: new Set([itemId]), + plannedAdditions: new Map([[itemId, 'created']]), version: 0, }) } else { - const newPlannedConnections = new Set(existing.plannedConnections) - newPlannedConnections.add(itemId) - const newCreatedEntities = new Set(existing.createdEntities) - newCreatedEntities.add(itemId) + const newPlannedAdditions = new Map(existing.plannedAdditions) + newPlannedAdditions.set(itemId, 'created') const currentOrderedIds = existing.orderedIds ?? computeDefaultOrderedIds(existing) const newOrderedIds = [...currentOrderedIds, itemId] this.writeHasMany(key, { ...existing, orderedIds: newOrderedIds, - plannedConnections: newPlannedConnections, - createdEntities: newCreatedEntities, + plannedAdditions: newPlannedAdditions, version: existing.version + 1, }) } @@ -378,8 +379,8 @@ export class RelationStore implements Rekeyable { /** * Connects an existing (persisted) entity to a has-many relation. - * Unlike addToHasMany, does NOT add to createdEntities — used for - * materializing embedded connect references to existing entities. + * Unlike addToHasMany, records the addition as 'connected' (not 'created') — + * used for materializing embedded connect references to existing entities. */ connectExistingToHasMany(key: string, itemId: string): void { const existing = this.hasManyStates.get(key) @@ -389,19 +390,21 @@ export class RelationStore implements Rekeyable { serverIds: new Set(), orderedIds: [itemId], plannedRemovals: new Map(), - plannedConnections: new Set([itemId]), - createdEntities: new Set(), + plannedAdditions: new Map([[itemId, 'connected']]), version: 0, }) } else { - const newPlannedConnections = new Set(existing.plannedConnections) - newPlannedConnections.add(itemId) + const newPlannedAdditions = new Map(existing.plannedAdditions) + // Do not downgrade an existing 'created' addition to 'connected'. + if (newPlannedAdditions.get(itemId) !== 'created') { + newPlannedAdditions.set(itemId, 'connected') + } const currentOrderedIds = existing.orderedIds ?? computeDefaultOrderedIds(existing) const newOrderedIds = [...currentOrderedIds, itemId] this.writeHasMany(key, { ...existing, orderedIds: newOrderedIds, - plannedConnections: newPlannedConnections, + plannedAdditions: newPlannedAdditions, version: existing.version + 1, }) } @@ -417,13 +420,11 @@ export class RelationStore implements Rekeyable { const existing = this.hasManyStates.get(key) if (!existing) return false - const isCreatedEntity = existing.createdEntities.has(itemId) + const isCreatedEntity = existing.plannedAdditions.get(itemId) === 'created' if (isCreatedEntity) { - const newPlannedConnections = new Set(existing.plannedConnections) - newPlannedConnections.delete(itemId) - const newCreatedEntities = new Set(existing.createdEntities) - newCreatedEntities.delete(itemId) + const newPlannedAdditions = new Map(existing.plannedAdditions) + newPlannedAdditions.delete(itemId) let newOrderedIds = existing.orderedIds if (newOrderedIds !== null) { newOrderedIds = newOrderedIds.filter(id => id !== itemId) @@ -432,14 +433,12 @@ export class RelationStore implements Rekeyable { const newState: StoredHasManyState = { ...existing, orderedIds: newOrderedIds, - plannedConnections: newPlannedConnections, - createdEntities: newCreatedEntities, + plannedAdditions: newPlannedAdditions, version: existing.version + 1, } if ( - newPlannedConnections.size === 0 && - newCreatedEntities.size === 0 && + newPlannedAdditions.size === 0 && existing.plannedRemovals.size === 0 && newOrderedIds !== null ) { @@ -466,8 +465,8 @@ export class RelationStore implements Rekeyable { * - has-one: currentId, when the relation is not disconnected/deleted * (a disconnected relation has a null currentId; a 'deleted' relation is * removing its target, so the target is not anchored by it). - * - has-many: effective members = (serverIds ∪ plannedConnections ∪ - * createdEntities) minus plannedRemovals. + * - has-many: effective members = (serverIds ∪ plannedAdditions.keys()) + * minus plannedRemovals. */ getLiveChildIds(keyPrefix: string): string[] { const ids = new Set() @@ -484,10 +483,7 @@ export class RelationStore implements Rekeyable { for (const id of state.serverIds) { if (!state.plannedRemovals.has(id)) ids.add(id) } - for (const id of state.plannedConnections) { - if (!state.plannedRemovals.has(id)) ids.add(id) - } - for (const id of state.createdEntities) { + for (const id of state.plannedAdditions.keys()) { if (!state.plannedRemovals.has(id)) ids.add(id) } } @@ -508,7 +504,7 @@ export class RelationStore implements Rekeyable { * Liveness matches {@link getLiveChildIds} EXACTLY so the two never disagree: * - has-one: currentId === childId, when the relation is not 'deleted' * (a disconnected relation has a null currentId and is skipped); - * - has-many: childId ∈ (serverIds ∪ plannedConnections ∪ createdEntities) + * - has-many: childId ∈ (serverIds ∪ plannedAdditions.keys()) * and childId ∉ plannedRemovals. * * Implemented as an on-demand scan of the live edges rather than an @@ -527,11 +523,7 @@ export class RelationStore implements Rekeyable { for (const [key, state] of this.hasManyStates) { if (state.plannedRemovals.has(childId)) continue - if ( - state.serverIds.has(childId) || - state.plannedConnections.has(childId) || - state.createdEntities.has(childId) - ) { + if (state.serverIds.has(childId) || state.plannedAdditions.has(childId)) { parents.add(parentKeyFromRelationKey(key)) } } @@ -618,7 +610,7 @@ export class RelationStore implements Rekeyable { for (const removedId of state.plannedRemovals.keys()) { newServerIds.delete(removedId) } - for (const connectedId of state.plannedConnections) { + for (const connectedId of state.plannedAdditions.keys()) { newServerIds.add(connectedId) } this.commitHasMany(key, Array.from(newServerIds)) @@ -668,7 +660,7 @@ export class RelationStore implements Rekeyable { if (!key.startsWith(keyPrefix)) continue const fieldName = key.slice(keyPrefix.length) - if (state.plannedRemovals.size > 0 || state.plannedConnections.size > 0) { + if (state.plannedRemovals.size > 0 || state.plannedAdditions.size > 0) { dirtyRelations.push(fieldName) } } @@ -707,8 +699,7 @@ export class RelationStore implements Rekeyable { serverIds: new Set(state.serverIds), orderedIds: state.orderedIds ? [...state.orderedIds] : null, plannedRemovals: new Map(state.plannedRemovals), - plannedConnections: new Set(state.plannedConnections), - createdEntities: new Set(state.createdEntities), + plannedAdditions: new Map(state.plannedAdditions), version: state.version, }) } @@ -743,8 +734,7 @@ export class RelationStore implements Rekeyable { serverIds: new Set(state.serverIds), orderedIds: state.orderedIds ? [...state.orderedIds] : null, plannedRemovals: new Map(state.plannedRemovals), - plannedConnections: new Set(state.plannedConnections), - createdEntities: new Set(state.createdEntities), + plannedAdditions: new Map(state.plannedAdditions), version: state.version + 1, }) keys.push(key) @@ -771,7 +761,7 @@ export class RelationStore implements Rekeyable { } } - // Replace in has-many states: serverIds, orderedIds, plannedConnections, createdEntities, plannedRemovals + // Replace in has-many states: serverIds, orderedIds, plannedAdditions, plannedRemovals for (const [key, state] of this.hasManyStates) { let changed = false @@ -793,19 +783,12 @@ export class RelationStore implements Rekeyable { } } - let plannedConnections = state.plannedConnections - if (plannedConnections.has(oldId)) { - plannedConnections = new Set(plannedConnections) - plannedConnections.delete(oldId) - plannedConnections.add(newId) - changed = true - } - - let createdEntities = state.createdEntities - if (createdEntities.has(oldId)) { - createdEntities = new Set(createdEntities) - createdEntities.delete(oldId) - createdEntities.add(newId) + let plannedAdditions = state.plannedAdditions + const additionKind = plannedAdditions.get(oldId) + if (additionKind !== undefined) { + plannedAdditions = new Map(plannedAdditions) + plannedAdditions.delete(oldId) + plannedAdditions.set(newId, additionKind) changed = true } @@ -823,8 +806,7 @@ export class RelationStore implements Rekeyable { serverIds, orderedIds, plannedRemovals, - plannedConnections, - createdEntities, + plannedAdditions, version: state.version + 1, }) } @@ -893,7 +875,7 @@ function parentKeyFromRelationKey(relationKey: string): string { /** * Computes the default ordered IDs for a has-many relation. - * Order is: serverIds (minus removals) + plannedConnections + * Order is: serverIds (minus removals) + plannedAdditions */ export function computeDefaultOrderedIds(state: StoredHasManyState): string[] { const result: string[] = [] @@ -904,7 +886,7 @@ export function computeDefaultOrderedIds(state: StoredHasManyState): string[] { } } - for (const id of state.plannedConnections) { + for (const id of state.plannedAdditions.keys()) { if (!result.includes(id)) { result.push(id) } diff --git a/packages/bindx/src/store/SnapshotStore.ts b/packages/bindx/src/store/SnapshotStore.ts index 1353201..5c157dd 100644 --- a/packages/bindx/src/store/SnapshotStore.ts +++ b/packages/bindx/src/store/SnapshotStore.ts @@ -470,7 +470,9 @@ export class SnapshotStore implements SnapshotVersionBumper { alias?: string, ): Set | undefined { const key = this.getRelationKey(parentType, parentId, alias ?? fieldName) - return this.relations.getHasMany(key)?.plannedConnections + const state = this.relations.getHasMany(key) + if (!state) return undefined + return new Set(state.plannedAdditions.keys()) } commitHasMany( @@ -568,7 +570,7 @@ export class SnapshotStore implements SnapshotVersionBumper { ): boolean { const key = this.getRelationKey(parentType, parentId, alias ?? fieldName) const state = this.relations.getHasMany(key) - return state?.createdEntities.has(itemId) ?? false + return state?.plannedAdditions.get(itemId) === 'created' } getHasManyCreatedEntities( @@ -578,7 +580,13 @@ export class SnapshotStore implements SnapshotVersionBumper { alias?: string, ): Set | undefined { const key = this.getRelationKey(parentType, parentId, alias ?? fieldName) - return this.relations.getHasMany(key)?.createdEntities + const state = this.relations.getHasMany(key) + if (!state) return undefined + const created = new Set() + for (const [id, kind] of state.plannedAdditions) { + if (kind === 'created') created.add(id) + } + return created } // ==================== Persisting State (delegated to EntityMetaStore) ==================== diff --git a/tests/unit/core/actionDispatcher.test.ts b/tests/unit/core/actionDispatcher.test.ts index 30fc58e..9bd9b0e 100644 --- a/tests/unit/core/actionDispatcher.test.ts +++ b/tests/unit/core/actionDispatcher.test.ts @@ -237,7 +237,7 @@ describe('ActionDispatcher', () => { }) const state = store.getHasMany('Article', 'a-1', 'tags') - expect(state?.plannedConnections.has('t-1')).toBe(true) + expect(state?.plannedAdditions.has('t-1')).toBe(true) }) test('ADD_TO_LIST should create entity and add to has-many', () => { diff --git a/tests/unit/handles/hasManyAlias.test.ts b/tests/unit/handles/hasManyAlias.test.ts index 625f646..36c4b50 100644 --- a/tests/unit/handles/hasManyAlias.test.ts +++ b/tests/unit/handles/hasManyAlias.test.ts @@ -262,8 +262,8 @@ describe('HasMany with Alias Support', () => { const state1 = store.getHasMany('Article', 'a-1', 'tags', alias1) const state2 = store.getHasMany('Article', 'a-1', 'tags', alias2) - expect(state1?.plannedConnections.size).toBe(0) - expect(state2?.plannedConnections.size).toBe(1) + expect(state1?.plannedAdditions.size).toBe(0) + expect(state2?.plannedAdditions.size).toBe(1) }) }) @@ -309,7 +309,7 @@ describe('HasMany with Alias Support', () => { // Should be tracked under the alias const state = store.getHasMany('Article', 'a-1', 'tags', alias) - expect(state?.createdEntities.has(tempId)).toBe(true) + expect(state?.plannedAdditions.get(tempId) === 'created').toBe(true) }) test('should remove items from the correct alias', () => { diff --git a/tests/unit/handles/hasManyHandle.test.ts b/tests/unit/handles/hasManyHandle.test.ts index 565df1e..2c18e92 100644 --- a/tests/unit/handles/hasManyHandle.test.ts +++ b/tests/unit/handles/hasManyHandle.test.ts @@ -359,8 +359,8 @@ describe('HasManyListHandle', () => { handle.remove(tempId) const state = store.getHasMany('Article', 'a-1', 'tags') - expect(state?.createdEntities.has(tempId)).toBe(false) - expect(state?.plannedConnections.has(tempId)).toBe(false) + expect(state?.plannedAdditions.get(tempId) === 'created').toBe(false) + expect(state?.plannedAdditions.has(tempId)).toBe(false) }) test('should remove server entity — manyHasMany uses disconnect', () => { @@ -551,8 +551,8 @@ describe('HasManyListHandle', () => { handle.remove(tempId) const state = store.getHasMany('Article', 'a-1', 'comments') - expect(state?.createdEntities.has(tempId)).toBe(false) - expect(state?.plannedConnections.has(tempId)).toBe(false) + expect(state?.plannedAdditions.get(tempId) === 'created').toBe(false) + expect(state?.plannedAdditions.has(tempId)).toBe(false) // No planned removal — the add was cancelled, not a delete expect(state?.plannedRemovals.has(tempId)).toBe(false) }) @@ -592,8 +592,8 @@ describe('HasManyListHandle', () => { const state = store.getHasMany('Article', 'a-1', 'tags') expect(state?.plannedRemovals.has(tempId)).toBe(false) - expect(state?.plannedConnections.has(tempId)).toBe(false) - expect(state?.createdEntities.has(tempId)).toBe(false) + expect(state?.plannedAdditions.has(tempId)).toBe(false) + expect(state?.plannedAdditions.get(tempId) === 'created').toBe(false) expect(handle.items.length).toBe(0) }) @@ -612,8 +612,8 @@ describe('HasManyListHandle', () => { const state = store.getHasMany('Article', 'a-1', 'tags') expect(state?.plannedRemovals.has(tempId)).toBe(false) - expect(state?.plannedConnections.has(tempId)).toBe(false) - expect(state?.createdEntities.has(tempId)).toBe(false) + expect(state?.plannedAdditions.has(tempId)).toBe(false) + expect(state?.plannedAdditions.get(tempId) === 'created').toBe(false) expect(handle.items.length).toBe(0) }) }) @@ -812,7 +812,7 @@ describe('HasManyListHandle', () => { const state = store.getHasMany('Article', 'a-1', 'tags') expect(state?.plannedRemovals.size).toBe(0) - expect(state?.plannedConnections.size).toBe(0) + expect(state?.plannedAdditions.size).toBe(0) }) }) @@ -1016,7 +1016,7 @@ describe('HasManyListHandle', () => { handle.connect('t-1') const state = store.getHasMany('Article', 'a-1', 'tags') - expect(state?.plannedConnections.has('t-1')).toBe(false) + expect(state?.plannedAdditions.has('t-1')).toBe(false) }) test('interceptor can cancel disconnect()', () => { diff --git a/tests/unit/persistence/rollback.test.ts b/tests/unit/persistence/rollback.test.ts index a5e3798..9ecc614 100644 --- a/tests/unit/persistence/rollback.test.ts +++ b/tests/unit/persistence/rollback.test.ts @@ -225,7 +225,7 @@ describe('BatchPersister rollback', () => { // Verify local changes const beforeHasMany = store.getHasMany('Article', 'a-1', 'tags') - expect(beforeHasMany?.plannedConnections.has('tag-3')).toBe(true) + expect(beforeHasMany?.plannedAdditions.has('tag-3')).toBe(true) expect(beforeHasMany?.plannedRemovals.has('tag-1')).toBe(true) // Persist with rollback @@ -239,7 +239,7 @@ describe('BatchPersister rollback', () => { // Has-many should be back to server state const afterHasMany = store.getHasMany('Article', 'a-1', 'tags') - expect(afterHasMany?.plannedConnections.size).toBe(0) + expect(afterHasMany?.plannedAdditions.size).toBe(0) expect(afterHasMany?.plannedRemovals.size).toBe(0) }) }) diff --git a/tests/unit/store/snapshotStore.test.ts b/tests/unit/store/snapshotStore.test.ts index a334f8d..283e9e9 100644 --- a/tests/unit/store/snapshotStore.test.ts +++ b/tests/unit/store/snapshotStore.test.ts @@ -353,7 +353,7 @@ describe('SnapshotStore', () => { expect(state.serverIds).toEqual(new Set(['t-1', 't-2'])) expect(state.plannedRemovals.size).toBe(0) - expect(state.plannedConnections.size).toBe(0) + expect(state.plannedAdditions.size).toBe(0) }) test('should update serverIds when called with new values', () => { @@ -412,18 +412,18 @@ describe('SnapshotStore', () => { store.planHasManyRemoval('Article', 'a-1', 'tags', 't-1', 'disconnect') const state = store.getHasMany('Article', 'a-1', 'tags') - expect(state?.plannedConnections.has('t-1')).toBe(false) + expect(state?.plannedAdditions.has('t-1')).toBe(false) }) test('should clear createdEntities when planning removal', () => { store.getOrCreateHasMany('Article', 'a-1', 'tags') store.addToHasMany('Article', 'a-1', 'tags', 't-new') - expect(store.getHasMany('Article', 'a-1', 'tags')?.createdEntities.has('t-new')).toBe(true) + expect(store.getHasMany('Article', 'a-1', 'tags')?.plannedAdditions.get('t-new') === 'created').toBe(true) store.planHasManyRemoval('Article', 'a-1', 'tags', 't-new', 'delete') const state = store.getHasMany('Article', 'a-1', 'tags') - expect(state?.createdEntities.has('t-new')).toBe(false) + expect(state?.plannedAdditions.get('t-new') === 'created').toBe(false) }) test('should remove item from orderedIds when planning removal', () => { @@ -445,7 +445,7 @@ describe('SnapshotStore', () => { store.planHasManyConnection('Article', 'a-1', 'tags', 't-new') const state = store.getHasMany('Article', 'a-1', 'tags') - expect(state?.plannedConnections.has('t-new')).toBe(true) + expect(state?.plannedAdditions.has('t-new')).toBe(true) }) test('should cancel planned disconnect when planning connection', () => { @@ -464,7 +464,7 @@ describe('SnapshotStore', () => { const state = store.getHasMany('Article', 'a-1', 'tags') expect(state?.plannedRemovals.has('t-1')).toBe(false) - expect(state?.plannedConnections.has('t-1')).toBe(true) + expect(state?.plannedAdditions.has('t-1')).toBe(true) }) }) @@ -474,7 +474,7 @@ describe('SnapshotStore', () => { store.addToHasMany('Article', 'a-1', 'tags', 't-new') const state = store.getHasMany('Article', 'a-1', 'tags') - expect(state?.plannedConnections.has('t-new')).toBe(true) + expect(state?.plannedAdditions.has('t-new')).toBe(true) }) test('should track item in createdEntities', () => { @@ -482,7 +482,7 @@ describe('SnapshotStore', () => { store.addToHasMany('Article', 'a-1', 'tags', 't-new') const state = store.getHasMany('Article', 'a-1', 'tags') - expect(state?.createdEntities.has('t-new')).toBe(true) + expect(state?.plannedAdditions.get('t-new') === 'created').toBe(true) }) test('should add to orderedIds', () => { @@ -501,8 +501,8 @@ describe('SnapshotStore', () => { store.removeFromHasMany('Article', 'a-1', 'tags', 't-new', 'disconnect') const state = store.getHasMany('Article', 'a-1', 'tags') - expect(state?.plannedConnections.has('t-new')).toBe(false) - expect(state?.createdEntities.has('t-new')).toBe(false) + expect(state?.plannedAdditions.has('t-new')).toBe(false) + expect(state?.plannedAdditions.get('t-new') === 'created').toBe(false) }) test('should plan disconnect for server entity', () => { @@ -528,8 +528,8 @@ describe('SnapshotStore', () => { const state = store.getHasMany('Article', 'a-1', 'tags') // Created entities should be cancelled, not planned for deletion - expect(state?.plannedConnections.has('t-new')).toBe(false) - expect(state?.createdEntities.has('t-new')).toBe(false) + expect(state?.plannedAdditions.has('t-new')).toBe(false) + expect(state?.plannedAdditions.get('t-new') === 'created').toBe(false) expect(state?.plannedRemovals.has('t-new')).toBe(false) }) }) @@ -570,7 +570,7 @@ describe('SnapshotStore', () => { const state = store.getHasMany('Article', 'a-1', 'tags') expect(state?.serverIds).toEqual(new Set(['t-1', 't-2'])) - expect(state?.plannedConnections.size).toBe(0) + expect(state?.plannedAdditions.size).toBe(0) }) }) @@ -583,7 +583,7 @@ describe('SnapshotStore', () => { const state = store.getHasMany('Article', 'a-1', 'tags') expect(state?.plannedRemovals.size).toBe(0) - expect(state?.plannedConnections.size).toBe(0) + expect(state?.plannedAdditions.size).toBe(0) expect(state?.orderedIds).toBeNull() }) }) From b667a187bcef364239f5423c2c822cf9ac5f778c Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 22 Jun 2026 21:18:19 +0200 Subject: [PATCH 13/19] refactor(bindx): decompose RelationStore into composed HasOneStore + HasManyStore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split the ~900-line RelationStore into two focused collaborator classes behind an unchanged public API: - HasOneStore owns has-one relation state and its own mutation-version counter. - HasManyStore owns has-many list state, computeDefaultOrderedIds, and its own mutation-version counter. - relationKey.ts holds the shared parentKeyFromRelationKey helper. - RelationStore is now a thin facade composing both: it sums the two mutation versions, unions getLiveChildIds/getParentKeysForChild/getDirtyRelations, and fans rekey/replaceEntityId/removeOwnedRelations/commit/reset/clear out to both sub-stores. Public API and all import paths (StoredRelationState, StoredHasManyState, HasManyRemovalType, HasManyAdditionKind, computeDefaultOrderedIds re-exported from RelationStore.js) are preserved; consumers see no change. Behavior is identical — pure structural move. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- packages/bindx/src/store/HasManyStore.ts | 609 ++++++++++++++++ packages/bindx/src/store/HasOneStore.ts | 289 ++++++++ packages/bindx/src/store/RelationStore.ts | 825 +++------------------- packages/bindx/src/store/relationKey.ts | 10 + 4 files changed, 990 insertions(+), 743 deletions(-) create mode 100644 packages/bindx/src/store/HasManyStore.ts create mode 100644 packages/bindx/src/store/HasOneStore.ts create mode 100644 packages/bindx/src/store/relationKey.ts diff --git a/packages/bindx/src/store/HasManyStore.ts b/packages/bindx/src/store/HasManyStore.ts new file mode 100644 index 0000000..d5570b7 --- /dev/null +++ b/packages/bindx/src/store/HasManyStore.ts @@ -0,0 +1,609 @@ +import { parentKeyFromRelationKey } from './relationKey.js' + +function setsEqual(a: Set, b: Set): boolean { + if (a.size !== b.size) return false + for (const item of a) { + if (!b.has(item)) return false + } + return true +} + +function arraysEqual(a: string[], b: string[]): boolean { + if (a.length !== b.length) return false + for (let i = 0; i < a.length; i++) { + if (a[i] !== b[i]) return false + } + return true +} + +/** + * Removal type for has-many items + */ +export type HasManyRemovalType = 'disconnect' | 'delete' + +/** + * Kind of a planned has-many addition: + * - 'created': a newly created entity (via add()) + * - 'connected': an existing persisted entity being connected (via connect()) + */ +export type HasManyAdditionKind = 'created' | 'connected' + +/** + * Has-many list state stored in SnapshotStore + */ +export interface StoredHasManyState { + /** IDs of items from server */ + serverIds: Set + /** Explicit ordered list of item IDs, null means use default order (serverIds + plannedAdditions) */ + orderedIds: string[] | null + /** Planned removals (disconnect or delete) keyed by entity ID */ + plannedRemovals: Map + /** + * Planned additions (IDs to add to the list) keyed by entity ID, with the + * value distinguishing newly CREATED entities (add()) from existing PERSISTED + * entities being CONNECTED (connect()). The keys are exactly the connections; + * the keys whose value is 'created' are exactly the created entities, so the + * "created ⊆ connections" invariant is structural. + */ + plannedAdditions: Map + version: number +} + +/** + * Computes the default ordered IDs for a has-many relation. + * Order is: serverIds (minus removals) + plannedAdditions + */ +export function computeDefaultOrderedIds(state: StoredHasManyState): string[] { + const result: string[] = [] + + for (const id of state.serverIds) { + if (!state.plannedRemovals.has(id)) { + result.push(id) + } + } + + for (const id of state.plannedAdditions.keys()) { + if (!result.includes(id)) { + result.push(id) + } + } + + return result +} + +/** + * Owns has-many list state ("parentType:parentId:fieldName" → {@link StoredHasManyState}). + * + * Has its own monotonic {@link mutationVersion} bumped on every actual write + * (funnelled through {@link writeHasMany} plus the delete/clear paths); the + * facade sums this with the has-one counter for {@link ReachabilityAnalyzer}. + */ +export class HasManyStore { + /** Has-many list states keyed by "parentType:parentId:fieldName" */ + private readonly hasManyStates = new Map() + + private mutationVersion = 0 + + getMutationVersion(): number { + return this.mutationVersion + } + + private writeHasMany(key: string, state: StoredHasManyState): void { + this.hasManyStates.set(key, state) + this.mutationVersion++ + } + + /** + * Gets or creates has-many list state. + */ + getOrCreateHasMany(key: string, serverIds?: string[]): StoredHasManyState { + const existing = this.hasManyStates.get(key) + if (!existing) { + this.writeHasMany(key, { + serverIds: new Set(serverIds ?? []), + orderedIds: null, + plannedRemovals: new Map(), + plannedAdditions: new Map(), + version: 0, + }) + } else if (serverIds !== undefined) { + const newServerIds = new Set(serverIds) + if (!setsEqual(existing.serverIds, newServerIds)) { + this.writeHasMany(key, { + ...existing, + serverIds: newServerIds, + orderedIds: null, + version: existing.version + 1, + }) + } + } + + return this.hasManyStates.get(key)! + } + + /** + * Gets has-many list state. + */ + getHasMany(key: string): StoredHasManyState | undefined { + return this.hasManyStates.get(key) + } + + /** + * Sets server IDs for a has-many relation. + */ + setHasManyServerIds(key: string, serverIds: string[]): void { + const existing = this.hasManyStates.get(key) + + if (!existing) { + this.writeHasMany(key, { + serverIds: new Set(serverIds), + orderedIds: null, + plannedRemovals: new Map(), + plannedAdditions: new Map(), + version: 0, + }) + } else { + this.writeHasMany(key, { + ...existing, + serverIds: new Set(serverIds), + orderedIds: null, + version: existing.version + 1, + }) + } + } + + /** + * Plans a removal for a has-many item. + */ + planHasManyRemoval(key: string, itemId: string, type: HasManyRemovalType): void { + const existing = this.hasManyStates.get(key) + + if (!existing) { + this.writeHasMany(key, { + serverIds: new Set(), + orderedIds: null, + plannedRemovals: new Map([[itemId, type]]), + plannedAdditions: new Map(), + version: 0, + }) + } else { + const newPlannedRemovals = new Map(existing.plannedRemovals) + newPlannedRemovals.set(itemId, type) + const newPlannedAdditions = new Map(existing.plannedAdditions) + newPlannedAdditions.delete(itemId) + let newOrderedIds = existing.orderedIds + if (newOrderedIds !== null) { + newOrderedIds = newOrderedIds.filter(id => id !== itemId) + } + this.writeHasMany(key, { + ...existing, + orderedIds: newOrderedIds, + plannedRemovals: newPlannedRemovals, + plannedAdditions: newPlannedAdditions, + version: existing.version + 1, + }) + } + } + + /** + * Plans a connection for a has-many item. + */ + planHasManyConnection(key: string, itemId: string): void { + const existing = this.hasManyStates.get(key) + + if (!existing) { + this.writeHasMany(key, { + serverIds: new Set(), + orderedIds: null, + plannedRemovals: new Map(), + plannedAdditions: new Map([[itemId, 'connected']]), + version: 0, + }) + } else { + const newPlannedAdditions = new Map(existing.plannedAdditions) + // Do not downgrade an existing 'created' addition to 'connected'. + if (newPlannedAdditions.get(itemId) !== 'created') { + newPlannedAdditions.set(itemId, 'connected') + } + const newPlannedRemovals = new Map(existing.plannedRemovals) + newPlannedRemovals.delete(itemId) + let newOrderedIds = existing.orderedIds + if (newOrderedIds !== null && !newOrderedIds.includes(itemId)) { + newOrderedIds = [...newOrderedIds, itemId] + } + this.writeHasMany(key, { + ...existing, + orderedIds: newOrderedIds, + plannedAdditions: newPlannedAdditions, + plannedRemovals: newPlannedRemovals, + version: existing.version + 1, + }) + } + } + + /** + * Commits has-many state after successful persist. + */ + commitHasMany(key: string, newServerIds: string[]): void { + const existing = this.hasManyStates.get(key) + + this.writeHasMany(key, { + serverIds: new Set(newServerIds), + orderedIds: null, + plannedRemovals: new Map(), + plannedAdditions: new Map(), + version: (existing?.version ?? 0) + 1, + }) + } + + /** + * Resets has-many state to server state (clears planned operations). + */ + resetHasMany(key: string): void { + const existing = this.hasManyStates.get(key) + if (!existing) return + + this.writeHasMany(key, { + serverIds: existing.serverIds, + orderedIds: null, + plannedRemovals: new Map(), + plannedAdditions: new Map(), + version: existing.version + 1, + }) + } + + /** + * Adds a newly created entity to a has-many relation. + * Used by HasManyListHandle.add() for inline entity creation. + */ + addToHasMany(key: string, itemId: string): void { + const existing = this.hasManyStates.get(key) + + if (!existing) { + this.writeHasMany(key, { + serverIds: new Set(), + orderedIds: [itemId], + plannedRemovals: new Map(), + plannedAdditions: new Map([[itemId, 'created']]), + version: 0, + }) + } else { + const newPlannedAdditions = new Map(existing.plannedAdditions) + newPlannedAdditions.set(itemId, 'created') + const currentOrderedIds = existing.orderedIds ?? computeDefaultOrderedIds(existing) + const newOrderedIds = [...currentOrderedIds, itemId] + this.writeHasMany(key, { + ...existing, + orderedIds: newOrderedIds, + plannedAdditions: newPlannedAdditions, + version: existing.version + 1, + }) + } + } + + /** + * Connects an existing (persisted) entity to a has-many relation. + * Unlike addToHasMany, records the addition as 'connected' (not 'created') — + * used for materializing embedded connect references to existing entities. + */ + connectExistingToHasMany(key: string, itemId: string): void { + const existing = this.hasManyStates.get(key) + + if (!existing) { + this.writeHasMany(key, { + serverIds: new Set(), + orderedIds: [itemId], + plannedRemovals: new Map(), + plannedAdditions: new Map([[itemId, 'connected']]), + version: 0, + }) + } else { + const newPlannedAdditions = new Map(existing.plannedAdditions) + // Do not downgrade an existing 'created' addition to 'connected'. + if (newPlannedAdditions.get(itemId) !== 'created') { + newPlannedAdditions.set(itemId, 'connected') + } + const currentOrderedIds = existing.orderedIds ?? computeDefaultOrderedIds(existing) + const newOrderedIds = [...currentOrderedIds, itemId] + this.writeHasMany(key, { + ...existing, + orderedIds: newOrderedIds, + plannedAdditions: newPlannedAdditions, + version: existing.version + 1, + }) + } + } + + /** + * Removes an entity from a has-many relation. + * For newly created entities (via add()), cancels the connection. + * For existing server entities, plans the specified removal type. + * Returns true if the state changed (caller should notify), false if it was a no-op. + */ + removeFromHasMany(key: string, itemId: string, removalType: HasManyRemovalType): boolean { + const existing = this.hasManyStates.get(key) + if (!existing) return false + + const isCreatedEntity = existing.plannedAdditions.get(itemId) === 'created' + + if (isCreatedEntity) { + const newPlannedAdditions = new Map(existing.plannedAdditions) + newPlannedAdditions.delete(itemId) + let newOrderedIds = existing.orderedIds + if (newOrderedIds !== null) { + newOrderedIds = newOrderedIds.filter(id => id !== itemId) + } + + const newState: StoredHasManyState = { + ...existing, + orderedIds: newOrderedIds, + plannedAdditions: newPlannedAdditions, + version: existing.version + 1, + } + + if ( + newPlannedAdditions.size === 0 && + existing.plannedRemovals.size === 0 && + newOrderedIds !== null + ) { + const defaultOrder = computeDefaultOrderedIds(newState) + if (arraysEqual(newOrderedIds, defaultOrder)) { + newState.orderedIds = null + } + } + + this.writeHasMany(key, newState) + return true + } else { + this.planHasManyRemoval(key, itemId, removalType) + return true + } + } + + /** + * Moves an item within a has-many relation from one index to another. + */ + moveInHasMany(key: string, fromIndex: number, toIndex: number): void { + const existing = this.hasManyStates.get(key) + if (!existing) return + + const currentOrderedIds = existing.orderedIds ?? computeDefaultOrderedIds(existing) + + if (fromIndex < 0 || fromIndex >= currentOrderedIds.length) return + if (toIndex < 0 || toIndex >= currentOrderedIds.length) return + if (fromIndex === toIndex) return + + const newOrderedIds = [...currentOrderedIds] + const movedItem = newOrderedIds.splice(fromIndex, 1)[0] + if (movedItem === undefined) return + newOrderedIds.splice(toIndex, 0, movedItem) + + this.writeHasMany(key, { + ...existing, + orderedIds: newOrderedIds, + version: existing.version + 1, + }) + } + + /** + * Gets the ordered list of item IDs for a has-many relation. + */ + getHasManyOrderedIds(key: string): string[] { + const existing = this.hasManyStates.get(key) + if (!existing) return [] + + if (existing.orderedIds !== null) { + return existing.orderedIds + } + + return computeDefaultOrderedIds(existing) + } + + /** + * Collects the ids of child entities reachable through LIVE has-many edges + * (key prefix "parentType:parentId:"): + * effective members = (serverIds ∪ plannedAdditions.keys()) minus plannedRemovals. + */ + collectLiveChildIds(keyPrefix: string, ids: Set): void { + for (const [key, state] of this.hasManyStates) { + if (!key.startsWith(keyPrefix)) continue + for (const id of state.serverIds) { + if (!state.plannedRemovals.has(id)) ids.add(id) + } + for (const id of state.plannedAdditions.keys()) { + if (!state.plannedRemovals.has(id)) ids.add(id) + } + } + } + + /** + * Adds the composite parent keys of every LIVE has-many edge containing + * {@link childId}: childId ∈ (serverIds ∪ plannedAdditions.keys()) and + * childId ∉ plannedRemovals. + */ + collectParentKeysForChild(childId: string, parents: Set): void { + for (const [key, state] of this.hasManyStates) { + if (state.plannedRemovals.has(childId)) continue + if (state.serverIds.has(childId) || state.plannedAdditions.has(childId)) { + parents.add(parentKeyFromRelationKey(key)) + } + } + } + + /** + * Removes all has-many state owned by an entity (keys under the given owner + * prefix). Bumps the mutation version once if anything was removed. + */ + removeOwnedRelations(keyPrefix: string): void { + let changed = false + for (const key of [...this.hasManyStates.keys()]) { + if (key.startsWith(keyPrefix)) { + this.hasManyStates.delete(key) + changed = true + } + } + if (changed) this.mutationVersion++ + } + + /** + * Commits all has-many relations for an entity. + */ + commitAllRelations(keyPrefix: string): void { + for (const [key, state] of this.hasManyStates) { + if (key.startsWith(keyPrefix)) { + const newServerIds = new Set(state.serverIds) + for (const removedId of state.plannedRemovals.keys()) { + newServerIds.delete(removedId) + } + for (const connectedId of state.plannedAdditions.keys()) { + newServerIds.add(connectedId) + } + this.commitHasMany(key, Array.from(newServerIds)) + } + } + } + + /** + * Resets all has-many relations for an entity to server state. + */ + resetAllRelations(keyPrefix: string): void { + for (const key of this.hasManyStates.keys()) { + if (key.startsWith(keyPrefix)) { + this.resetHasMany(key) + } + } + } + + /** + * Collects the field names of dirty has-many relations for an entity. + */ + collectDirtyRelations(keyPrefix: string, dirtyRelations: string[]): void { + for (const [key, state] of this.hasManyStates) { + if (!key.startsWith(keyPrefix)) continue + const fieldName = key.slice(keyPrefix.length) + + if (state.plannedRemovals.size > 0 || state.plannedAdditions.size > 0) { + dirtyRelations.push(fieldName) + } + } + } + + /** + * Exports has-many states for given keys. + */ + exportHasManyStates(keys: string[]): Map { + const result = new Map() + for (const key of keys) { + const state = this.hasManyStates.get(key) + if (state) { + result.set(key, { + serverIds: new Set(state.serverIds), + orderedIds: state.orderedIds ? [...state.orderedIds] : null, + plannedRemovals: new Map(state.plannedRemovals), + plannedAdditions: new Map(state.plannedAdditions), + version: state.version, + }) + } + } + return result + } + + /** + * Imports has-many states from a snapshot. + * Returns the keys that were imported for notification. + */ + importHasManyStates(states: Map): string[] { + const keys: string[] = [] + for (const [key, state] of states) { + this.writeHasMany(key, { + serverIds: new Set(state.serverIds), + orderedIds: state.orderedIds ? [...state.orderedIds] : null, + plannedRemovals: new Map(state.plannedRemovals), + plannedAdditions: new Map(state.plannedAdditions), + version: state.version + 1, + }) + keys.push(key) + } + return keys + } + + /** + * Replaces all occurrences of oldId with newId across has-many states + * (serverIds, orderedIds, plannedAdditions, plannedRemovals). + */ + replaceEntityId(oldId: string, newId: string): void { + for (const [key, state] of this.hasManyStates) { + let changed = false + + let serverIds = state.serverIds + if (serverIds.has(oldId)) { + serverIds = new Set(serverIds) + serverIds.delete(oldId) + serverIds.add(newId) + changed = true + } + + let orderedIds = state.orderedIds + if (orderedIds) { + const idx = orderedIds.indexOf(oldId) + if (idx !== -1) { + orderedIds = [...orderedIds] + orderedIds[idx] = newId + changed = true + } + } + + let plannedAdditions = state.plannedAdditions + const additionKind = plannedAdditions.get(oldId) + if (additionKind !== undefined) { + plannedAdditions = new Map(plannedAdditions) + plannedAdditions.delete(oldId) + plannedAdditions.set(newId, additionKind) + changed = true + } + + let plannedRemovals = state.plannedRemovals + if (plannedRemovals.has(oldId)) { + const removalType = plannedRemovals.get(oldId)! + plannedRemovals = new Map(plannedRemovals) + plannedRemovals.delete(oldId) + plannedRemovals.set(newId, removalType) + changed = true + } + + if (changed) { + this.writeHasMany(key, { + serverIds, + orderedIds, + plannedRemovals, + plannedAdditions, + version: state.version + 1, + }) + } + } + } + + /** + * Rekeys has-many entries owned by an entity (changes the parent ID in the key). + */ + rekeyOwner(oldKeyPrefix: string, newKeyPrefix: string): void { + const toMove: [string, StoredHasManyState][] = [] + for (const [key, value] of this.hasManyStates) { + if (key.startsWith(oldKeyPrefix)) { + toMove.push([key, value]) + } + } + for (const [oldKey, value] of toMove) { + this.hasManyStates.delete(oldKey) + this.writeHasMany(newKeyPrefix + oldKey.slice(oldKeyPrefix.length), value) + } + } + + /** + * Clears all has-many relation data. + */ + clear(): void { + this.hasManyStates.clear() + this.mutationVersion++ + } +} diff --git a/packages/bindx/src/store/HasOneStore.ts b/packages/bindx/src/store/HasOneStore.ts new file mode 100644 index 0000000..97d5649 --- /dev/null +++ b/packages/bindx/src/store/HasOneStore.ts @@ -0,0 +1,289 @@ +import type { HasOneRelationState } from '../handles/types.js' +import type { EntitySnapshot } from './snapshots.js' +import { parentKeyFromRelationKey } from './relationKey.js' + +/** + * Relation state stored in SnapshotStore + */ +export interface StoredRelationState { + currentId: string | null + serverId: string | null + state: HasOneRelationState + serverState: HasOneRelationState + placeholderData: Record + version: number +} + +/** + * Owns has-one relation state ("parentType:parentId:fieldName" → {@link StoredRelationState}). + * + * Has its own monotonic {@link mutationVersion} bumped on every actual write + * (funnelled through {@link writeRelation} plus the delete/clear paths); the + * facade sums this with the has-many counter for {@link ReachabilityAnalyzer}. + */ +export class HasOneStore { + /** Relation states keyed by "parentType:parentId:fieldName" */ + private readonly relationStates = new Map() + + private mutationVersion = 0 + + getMutationVersion(): number { + return this.mutationVersion + } + + private writeRelation(key: string, state: StoredRelationState): void { + this.relationStates.set(key, state) + this.mutationVersion++ + } + + /** + * Gets or creates relation state. + */ + getOrCreateRelation( + key: string, + initial: Omit, + ): StoredRelationState { + if (!this.relationStates.has(key)) { + this.writeRelation(key, { ...initial, version: 0 }) + } + + return this.relationStates.get(key)! + } + + /** + * Gets relation state. + */ + getRelation(key: string): StoredRelationState | undefined { + return this.relationStates.get(key) + } + + /** + * Updates relation state. + * If the relation state doesn't exist, creates it using entity snapshot for server data. + */ + setRelation( + key: string, + updates: Partial>, + entitySnapshot: EntitySnapshot | undefined, + fieldName: string, + ): void { + const existing = this.relationStates.get(key) + + if (!existing) { + let serverId: string | null = null + let serverState: HasOneRelationState = 'disconnected' + + if (entitySnapshot?.serverData) { + const relatedData = (entitySnapshot.serverData as Record)[fieldName] + if (relatedData && typeof relatedData === 'object' && 'id' in relatedData) { + serverId = (relatedData as { id: string }).id + serverState = 'connected' + } + } + + this.writeRelation(key, { + currentId: 'currentId' in updates ? updates.currentId! : serverId, + serverId, + state: 'state' in updates ? updates.state! : serverState, + serverState, + placeholderData: updates.placeholderData ?? {}, + version: 0, + }) + } else { + this.writeRelation(key, { + ...existing, + ...updates, + version: existing.version + 1, + }) + } + } + + /** + * Commits relation state (server = current). + */ + commitRelation(key: string): void { + const existing = this.relationStates.get(key) + if (!existing) return + + this.writeRelation(key, { + ...existing, + serverId: existing.currentId, + serverState: existing.state === 'creating' ? 'connected' : existing.state, + placeholderData: {}, + version: existing.version + 1, + }) + } + + /** + * Resets relation to server state. + */ + resetRelation(key: string): void { + const existing = this.relationStates.get(key) + if (!existing) return + + this.writeRelation(key, { + ...existing, + currentId: existing.serverId, + state: existing.serverState, + placeholderData: {}, + version: existing.version + 1, + }) + } + + /** + * Collects the ids of child entities reachable through LIVE has-one edges + * (key prefix "parentType:parentId:"): currentId, when the relation is not + * disconnected/deleted. + */ + collectLiveChildIds(keyPrefix: string, ids: Set): void { + for (const [key, state] of this.relationStates) { + if (!key.startsWith(keyPrefix)) continue + if (state.currentId !== null && state.state !== 'deleted') { + ids.add(state.currentId) + } + } + } + + /** + * Adds the composite parent keys of every LIVE has-one edge pointing at + * {@link childId} (currentId === childId, when the relation is not 'deleted'). + */ + collectParentKeysForChild(childId: string, parents: Set): void { + for (const [key, state] of this.relationStates) { + if (state.currentId === childId && state.state !== 'deleted') { + parents.add(parentKeyFromRelationKey(key)) + } + } + } + + /** + * Removes all has-one state owned by an entity (keys under the given owner + * prefix). Bumps the mutation version once if anything was removed. + */ + removeOwnedRelations(keyPrefix: string): void { + let changed = false + for (const key of [...this.relationStates.keys()]) { + if (key.startsWith(keyPrefix)) { + this.relationStates.delete(key) + changed = true + } + } + if (changed) this.mutationVersion++ + } + + /** + * Commits all has-one relations for an entity. + */ + commitAllRelations(keyPrefix: string): void { + for (const key of this.relationStates.keys()) { + if (key.startsWith(keyPrefix)) { + this.commitRelation(key) + } + } + } + + /** + * Resets all has-one relations for an entity to server state. + */ + resetAllRelations(keyPrefix: string): void { + for (const key of this.relationStates.keys()) { + if (key.startsWith(keyPrefix)) { + this.resetRelation(key) + } + } + } + + /** + * Collects the field names of dirty has-one relations for an entity. + */ + collectDirtyRelations(keyPrefix: string, dirtyRelations: string[]): void { + for (const [key, state] of this.relationStates) { + if (!key.startsWith(keyPrefix)) continue + const fieldName = key.slice(keyPrefix.length) + + if ( + state.currentId !== state.serverId || + state.state !== state.serverState || + Object.keys(state.placeholderData).length > 0 + ) { + dirtyRelations.push(fieldName) + } + } + } + + /** + * Exports relation states for given keys. + */ + exportRelationStates(keys: string[]): Map { + const result = new Map() + for (const key of keys) { + const state = this.relationStates.get(key) + if (state) { + result.set(key, { + ...state, + placeholderData: { ...state.placeholderData }, + }) + } + } + return result + } + + /** + * Imports relation states from a snapshot. + * Returns the keys that were imported for notification. + */ + importRelationStates(states: Map): string[] { + const keys: string[] = [] + for (const [key, state] of states) { + this.writeRelation(key, { + ...state, + placeholderData: { ...state.placeholderData }, + }) + keys.push(key) + } + return keys + } + + /** + * Replaces all occurrences of oldId with newId across has-one relation states + * (currentId, serverId). + */ + replaceEntityId(oldId: string, newId: string): void { + for (const [key, state] of this.relationStates) { + let changed = false + let currentId = state.currentId + let serverId = state.serverId + + if (currentId === oldId) { currentId = newId; changed = true } + if (serverId === oldId) { serverId = newId; changed = true } + + if (changed) { + this.writeRelation(key, { ...state, currentId, serverId, version: state.version + 1 }) + } + } + } + + /** + * Rekeys has-one entries owned by an entity (changes the parent ID in the key). + */ + rekeyOwner(oldKeyPrefix: string, newKeyPrefix: string): void { + const toMove: [string, StoredRelationState][] = [] + for (const [key, value] of this.relationStates) { + if (key.startsWith(oldKeyPrefix)) { + toMove.push([key, value]) + } + } + for (const [oldKey, value] of toMove) { + this.relationStates.delete(oldKey) + this.writeRelation(newKeyPrefix + oldKey.slice(oldKeyPrefix.length), value) + } + } + + /** + * Clears all has-one relation data. + */ + clear(): void { + this.relationStates.clear() + this.mutationVersion++ + } +} diff --git a/packages/bindx/src/store/RelationStore.ts b/packages/bindx/src/store/RelationStore.ts index a03de25..43e10b0 100644 --- a/packages/bindx/src/store/RelationStore.ts +++ b/packages/bindx/src/store/RelationStore.ts @@ -1,816 +1,218 @@ -import type { HasOneRelationState } from '../handles/types.js' import type { EntitySnapshot } from './snapshots.js' import type { RekeyContext, Rekeyable } from './RekeyOrchestrator.js' - -function setsEqual(a: Set, b: Set): boolean { - if (a.size !== b.size) return false - for (const item of a) { - if (!b.has(item)) return false - } - return true -} - -/** - * Removal type for has-many items - */ -export type HasManyRemovalType = 'disconnect' | 'delete' - -/** - * Kind of a planned has-many addition: - * - 'created': a newly created entity (via add()) - * - 'connected': an existing persisted entity being connected (via connect()) - */ -export type HasManyAdditionKind = 'created' | 'connected' - -/** - * Has-many list state stored in SnapshotStore - */ -export interface StoredHasManyState { - /** IDs of items from server */ - serverIds: Set - /** Explicit ordered list of item IDs, null means use default order (serverIds + plannedAdditions) */ - orderedIds: string[] | null - /** Planned removals (disconnect or delete) keyed by entity ID */ - plannedRemovals: Map - /** - * Planned additions (IDs to add to the list) keyed by entity ID, with the - * value distinguishing newly CREATED entities (add()) from existing PERSISTED - * entities being CONNECTED (connect()). The keys are exactly the connections; - * the keys whose value is 'created' are exactly the created entities, so the - * "created ⊆ connections" invariant is structural. - */ - plannedAdditions: Map - version: number -} - -/** - * Relation state stored in SnapshotStore - */ -export interface StoredRelationState { - currentId: string | null - serverId: string | null - state: HasOneRelationState - serverState: HasOneRelationState - placeholderData: Record - version: number -} +import { HasOneStore, type StoredRelationState } from './HasOneStore.js' +import { + HasManyStore, + computeDefaultOrderedIds, + type HasManyAdditionKind, + type HasManyRemovalType, + type StoredHasManyState, +} from './HasManyStore.js' + +// Re-exported so existing imports from './RelationStore.js' keep resolving. +export type { StoredRelationState } from './HasOneStore.js' +export type { HasManyAdditionKind, HasManyRemovalType, StoredHasManyState } from './HasManyStore.js' +export { computeDefaultOrderedIds } from './HasManyStore.js' /** * Manages has-one and has-many relation state. * * Relation keys use the format "parentType:parentId:fieldName". * Notification is handled by callers via callback returns. + * + * Thin facade composing a {@link HasOneStore} and a {@link HasManyStore}: each + * sub-store owns its own state map and write helper, and the facade merges the + * cross-cutting queries (mutation version sum; live-child/parent-key unions; + * dirty union) and fans out the cross-cutting mutations (rekey, removeOwned, + * clear) to both. */ export class RelationStore implements Rekeyable { - /** Relation states keyed by "parentType:parentId:fieldName" */ - private readonly relationStates = new Map() - - /** Has-many list states keyed by "parentType:parentId:fieldName" */ - private readonly hasManyStates = new Map() + private readonly hasOne = new HasOneStore() + private readonly hasMany = new HasManyStore() /** - * Monotonic counter bumped on every actual write to relation/has-many state. - * Used by {@link ReachabilityAnalyzer} to memoize its walk. All writes funnel - * through {@link writeRelation}/{@link writeHasMany} (plus the delete/clear - * paths), so a read-only `getOrCreate*` call on the per-render materialize - * path — which does not write when the entry already matches — never bumps it. + * Increases whenever EITHER sub-store mutates. Used by {@link ReachabilityAnalyzer} + * to memoize its walk; only monotonic-increase-on-mutation matters, so the sum + * of the two per-store counters is sufficient. */ - private mutationVersion = 0 - getMutationVersion(): number { - return this.mutationVersion - } - - private writeRelation(key: string, state: StoredRelationState): void { - this.relationStates.set(key, state) - this.mutationVersion++ - } - - private writeHasMany(key: string, state: StoredHasManyState): void { - this.hasManyStates.set(key, state) - this.mutationVersion++ + return this.hasOne.getMutationVersion() + this.hasMany.getMutationVersion() } // ==================== Has-One Relations ==================== - /** - * Gets or creates relation state. - */ getOrCreateRelation( key: string, initial: Omit, ): StoredRelationState { - if (!this.relationStates.has(key)) { - this.writeRelation(key, { ...initial, version: 0 }) - } - - return this.relationStates.get(key)! + return this.hasOne.getOrCreateRelation(key, initial) } - /** - * Gets relation state. - */ getRelation(key: string): StoredRelationState | undefined { - return this.relationStates.get(key) + return this.hasOne.getRelation(key) } - /** - * Updates relation state. - * If the relation state doesn't exist, creates it using entity snapshot for server data. - */ setRelation( key: string, updates: Partial>, entitySnapshot: EntitySnapshot | undefined, fieldName: string, ): void { - const existing = this.relationStates.get(key) - - if (!existing) { - let serverId: string | null = null - let serverState: HasOneRelationState = 'disconnected' - - if (entitySnapshot?.serverData) { - const relatedData = (entitySnapshot.serverData as Record)[fieldName] - if (relatedData && typeof relatedData === 'object' && 'id' in relatedData) { - serverId = (relatedData as { id: string }).id - serverState = 'connected' - } - } - - this.writeRelation(key, { - currentId: 'currentId' in updates ? updates.currentId! : serverId, - serverId, - state: 'state' in updates ? updates.state! : serverState, - serverState, - placeholderData: updates.placeholderData ?? {}, - version: 0, - }) - } else { - this.writeRelation(key, { - ...existing, - ...updates, - version: existing.version + 1, - }) - } + this.hasOne.setRelation(key, updates, entitySnapshot, fieldName) } - /** - * Commits relation state (server = current). - */ commitRelation(key: string): void { - const existing = this.relationStates.get(key) - if (!existing) return - - this.writeRelation(key, { - ...existing, - serverId: existing.currentId, - serverState: existing.state === 'creating' ? 'connected' : existing.state, - placeholderData: {}, - version: existing.version + 1, - }) + this.hasOne.commitRelation(key) } - /** - * Resets relation to server state. - */ resetRelation(key: string): void { - const existing = this.relationStates.get(key) - if (!existing) return - - this.writeRelation(key, { - ...existing, - currentId: existing.serverId, - state: existing.serverState, - placeholderData: {}, - version: existing.version + 1, - }) + this.hasOne.resetRelation(key) } // ==================== Has-Many Relations ==================== - /** - * Gets or creates has-many list state. - */ getOrCreateHasMany(key: string, serverIds?: string[]): StoredHasManyState { - const existing = this.hasManyStates.get(key) - if (!existing) { - this.writeHasMany(key, { - serverIds: new Set(serverIds ?? []), - orderedIds: null, - plannedRemovals: new Map(), - plannedAdditions: new Map(), - version: 0, - }) - } else if (serverIds !== undefined) { - const newServerIds = new Set(serverIds) - if (!setsEqual(existing.serverIds, newServerIds)) { - this.writeHasMany(key, { - ...existing, - serverIds: newServerIds, - orderedIds: null, - version: existing.version + 1, - }) - } - } - - return this.hasManyStates.get(key)! + return this.hasMany.getOrCreateHasMany(key, serverIds) } - /** - * Gets has-many list state. - */ getHasMany(key: string): StoredHasManyState | undefined { - return this.hasManyStates.get(key) + return this.hasMany.getHasMany(key) } - /** - * Sets server IDs for a has-many relation. - */ setHasManyServerIds(key: string, serverIds: string[]): void { - const existing = this.hasManyStates.get(key) - - if (!existing) { - this.writeHasMany(key, { - serverIds: new Set(serverIds), - orderedIds: null, - plannedRemovals: new Map(), - plannedAdditions: new Map(), - version: 0, - }) - } else { - this.writeHasMany(key, { - ...existing, - serverIds: new Set(serverIds), - orderedIds: null, - version: existing.version + 1, - }) - } + this.hasMany.setHasManyServerIds(key, serverIds) } - /** - * Plans a removal for a has-many item. - */ planHasManyRemoval(key: string, itemId: string, type: HasManyRemovalType): void { - const existing = this.hasManyStates.get(key) - - if (!existing) { - this.writeHasMany(key, { - serverIds: new Set(), - orderedIds: null, - plannedRemovals: new Map([[itemId, type]]), - plannedAdditions: new Map(), - version: 0, - }) - } else { - const newPlannedRemovals = new Map(existing.plannedRemovals) - newPlannedRemovals.set(itemId, type) - const newPlannedAdditions = new Map(existing.plannedAdditions) - newPlannedAdditions.delete(itemId) - let newOrderedIds = existing.orderedIds - if (newOrderedIds !== null) { - newOrderedIds = newOrderedIds.filter(id => id !== itemId) - } - this.writeHasMany(key, { - ...existing, - orderedIds: newOrderedIds, - plannedRemovals: newPlannedRemovals, - plannedAdditions: newPlannedAdditions, - version: existing.version + 1, - }) - } + this.hasMany.planHasManyRemoval(key, itemId, type) } - /** - * Plans a connection for a has-many item. - */ planHasManyConnection(key: string, itemId: string): void { - const existing = this.hasManyStates.get(key) - - if (!existing) { - this.writeHasMany(key, { - serverIds: new Set(), - orderedIds: null, - plannedRemovals: new Map(), - plannedAdditions: new Map([[itemId, 'connected']]), - version: 0, - }) - } else { - const newPlannedAdditions = new Map(existing.plannedAdditions) - // Do not downgrade an existing 'created' addition to 'connected'. - if (newPlannedAdditions.get(itemId) !== 'created') { - newPlannedAdditions.set(itemId, 'connected') - } - const newPlannedRemovals = new Map(existing.plannedRemovals) - newPlannedRemovals.delete(itemId) - let newOrderedIds = existing.orderedIds - if (newOrderedIds !== null && !newOrderedIds.includes(itemId)) { - newOrderedIds = [...newOrderedIds, itemId] - } - this.writeHasMany(key, { - ...existing, - orderedIds: newOrderedIds, - plannedAdditions: newPlannedAdditions, - plannedRemovals: newPlannedRemovals, - version: existing.version + 1, - }) - } + this.hasMany.planHasManyConnection(key, itemId) } - /** - * Commits has-many state after successful persist. - */ commitHasMany(key: string, newServerIds: string[]): void { - const existing = this.hasManyStates.get(key) - - this.writeHasMany(key, { - serverIds: new Set(newServerIds), - orderedIds: null, - plannedRemovals: new Map(), - plannedAdditions: new Map(), - version: (existing?.version ?? 0) + 1, - }) + this.hasMany.commitHasMany(key, newServerIds) } - /** - * Resets has-many state to server state (clears planned operations). - */ resetHasMany(key: string): void { - const existing = this.hasManyStates.get(key) - if (!existing) return - - this.writeHasMany(key, { - serverIds: existing.serverIds, - orderedIds: null, - plannedRemovals: new Map(), - plannedAdditions: new Map(), - version: existing.version + 1, - }) + this.hasMany.resetHasMany(key) } - /** - * Adds a newly created entity to a has-many relation. - * Used by HasManyListHandle.add() for inline entity creation. - */ addToHasMany(key: string, itemId: string): void { - const existing = this.hasManyStates.get(key) - - if (!existing) { - this.writeHasMany(key, { - serverIds: new Set(), - orderedIds: [itemId], - plannedRemovals: new Map(), - plannedAdditions: new Map([[itemId, 'created']]), - version: 0, - }) - } else { - const newPlannedAdditions = new Map(existing.plannedAdditions) - newPlannedAdditions.set(itemId, 'created') - const currentOrderedIds = existing.orderedIds ?? computeDefaultOrderedIds(existing) - const newOrderedIds = [...currentOrderedIds, itemId] - this.writeHasMany(key, { - ...existing, - orderedIds: newOrderedIds, - plannedAdditions: newPlannedAdditions, - version: existing.version + 1, - }) - } + this.hasMany.addToHasMany(key, itemId) } - /** - * Connects an existing (persisted) entity to a has-many relation. - * Unlike addToHasMany, records the addition as 'connected' (not 'created') — - * used for materializing embedded connect references to existing entities. - */ connectExistingToHasMany(key: string, itemId: string): void { - const existing = this.hasManyStates.get(key) - - if (!existing) { - this.writeHasMany(key, { - serverIds: new Set(), - orderedIds: [itemId], - plannedRemovals: new Map(), - plannedAdditions: new Map([[itemId, 'connected']]), - version: 0, - }) - } else { - const newPlannedAdditions = new Map(existing.plannedAdditions) - // Do not downgrade an existing 'created' addition to 'connected'. - if (newPlannedAdditions.get(itemId) !== 'created') { - newPlannedAdditions.set(itemId, 'connected') - } - const currentOrderedIds = existing.orderedIds ?? computeDefaultOrderedIds(existing) - const newOrderedIds = [...currentOrderedIds, itemId] - this.writeHasMany(key, { - ...existing, - orderedIds: newOrderedIds, - plannedAdditions: newPlannedAdditions, - version: existing.version + 1, - }) - } + this.hasMany.connectExistingToHasMany(key, itemId) } - /** - * Removes an entity from a has-many relation. - * For newly created entities (via add()), cancels the connection. - * For existing server entities, plans the specified removal type. - * Returns true if the state changed (caller should notify), false if it was a no-op. - */ removeFromHasMany(key: string, itemId: string, removalType: HasManyRemovalType): boolean { - const existing = this.hasManyStates.get(key) - if (!existing) return false - - const isCreatedEntity = existing.plannedAdditions.get(itemId) === 'created' - - if (isCreatedEntity) { - const newPlannedAdditions = new Map(existing.plannedAdditions) - newPlannedAdditions.delete(itemId) - let newOrderedIds = existing.orderedIds - if (newOrderedIds !== null) { - newOrderedIds = newOrderedIds.filter(id => id !== itemId) - } - - const newState: StoredHasManyState = { - ...existing, - orderedIds: newOrderedIds, - plannedAdditions: newPlannedAdditions, - version: existing.version + 1, - } - - if ( - newPlannedAdditions.size === 0 && - existing.plannedRemovals.size === 0 && - newOrderedIds !== null - ) { - const defaultOrder = computeDefaultOrderedIds(newState) - if (arraysEqual(newOrderedIds, defaultOrder)) { - newState.orderedIds = null - } - } - - this.writeHasMany(key, newState) - return true - } else { - this.planHasManyRemoval(key, itemId, removalType) - return true - } + return this.hasMany.removeFromHasMany(key, itemId, removalType) } + moveInHasMany(key: string, fromIndex: number, toIndex: number): void { + this.hasMany.moveInHasMany(key, fromIndex, toIndex) + } + + getHasManyOrderedIds(key: string): string[] { + return this.hasMany.getHasManyOrderedIds(key) + } + + // ==================== Reachability / Reverse Lookup ==================== + /** * Collects the ids of child entities currently reachable through an entity's - * LIVE relations (key prefix "parentType:parentId:"). Used by reachability-based - * create detection to walk the relation graph from roots. - * - * Live edges are: - * - has-one: currentId, when the relation is not disconnected/deleted - * (a disconnected relation has a null currentId; a 'deleted' relation is - * removing its target, so the target is not anchored by it). - * - has-many: effective members = (serverIds ∪ plannedAdditions.keys()) - * minus plannedRemovals. + * LIVE relations (key prefix "parentType:parentId:"). Unions the live has-one + * edges with the live has-many members. */ getLiveChildIds(keyPrefix: string): string[] { const ids = new Set() - - for (const [key, state] of this.relationStates) { - if (!key.startsWith(keyPrefix)) continue - if (state.currentId !== null && state.state !== 'deleted') { - ids.add(state.currentId) - } - } - - for (const [key, state] of this.hasManyStates) { - if (!key.startsWith(keyPrefix)) continue - for (const id of state.serverIds) { - if (!state.plannedRemovals.has(id)) ids.add(id) - } - for (const id of state.plannedAdditions.keys()) { - if (!state.plannedRemovals.has(id)) ids.add(id) - } - } - + this.hasOne.collectLiveChildIds(keyPrefix, ids) + this.hasMany.collectLiveChildIds(keyPrefix, ids) return Array.from(ids) } /** * Collects the composite keys ("parentType:parentId") of every entity that - * currently has a LIVE relation edge pointing at {@link childId}. This is the - * reverse of {@link getLiveChildIds}: instead of "which children does this - * parent reach", it answers "which parents reference this child". - * - * It is the single source of truth for parent re-render notification — when a - * child entity's own field changes, every parent returned here is notified so - * its rendered view of the child updates. - * - * Liveness matches {@link getLiveChildIds} EXACTLY so the two never disagree: - * - has-one: currentId === childId, when the relation is not 'deleted' - * (a disconnected relation has a null currentId and is skipped); - * - has-many: childId ∈ (serverIds ∪ plannedAdditions.keys()) - * and childId ∉ plannedRemovals. - * - * Implemented as an on-demand scan of the live edges rather than an - * incrementally-maintained reverse index: it is correct by construction (it - * reads the same maps the forward query reads), so there is no second - * structure that could drift out of sync on disconnect/remove/rekey/clear. + * currently has a LIVE relation edge pointing at {@link childId} — the reverse + * of {@link getLiveChildIds}. Unions both sub-stores' results, scanning the + * same live edges the forward query reads so the two never disagree. */ getParentKeysForChild(childId: string): Set { const parents = new Set() - - for (const [key, state] of this.relationStates) { - if (state.currentId === childId && state.state !== 'deleted') { - parents.add(parentKeyFromRelationKey(key)) - } - } - - for (const [key, state] of this.hasManyStates) { - if (state.plannedRemovals.has(childId)) continue - if (state.serverIds.has(childId) || state.plannedAdditions.has(childId)) { - parents.add(parentKeyFromRelationKey(key)) - } - } - + this.hasOne.collectParentKeysForChild(childId, parents) + this.hasMany.collectParentKeysForChild(childId, parents) return parents } + // ==================== Bulk Operations ==================== + /** * Removes all relation and has-many state owned by an entity (keys under the * given owner prefix). Called by removeEntity so a removed entity leaves no * stale relation state behind. */ removeOwnedRelations(keyPrefix: string): void { - let changed = false - for (const key of [...this.relationStates.keys()]) { - if (key.startsWith(keyPrefix)) { - this.relationStates.delete(key) - changed = true - } - } - for (const key of [...this.hasManyStates.keys()]) { - if (key.startsWith(keyPrefix)) { - this.hasManyStates.delete(key) - changed = true - } - } - if (changed) this.mutationVersion++ + this.hasOne.removeOwnedRelations(keyPrefix) + this.hasMany.removeOwnedRelations(keyPrefix) } - /** - * Moves an item within a has-many relation from one index to another. - */ - moveInHasMany(key: string, fromIndex: number, toIndex: number): void { - const existing = this.hasManyStates.get(key) - if (!existing) return - - const currentOrderedIds = existing.orderedIds ?? computeDefaultOrderedIds(existing) - - if (fromIndex < 0 || fromIndex >= currentOrderedIds.length) return - if (toIndex < 0 || toIndex >= currentOrderedIds.length) return - if (fromIndex === toIndex) return - - const newOrderedIds = [...currentOrderedIds] - const movedItem = newOrderedIds.splice(fromIndex, 1)[0] - if (movedItem === undefined) return - newOrderedIds.splice(toIndex, 0, movedItem) - - this.writeHasMany(key, { - ...existing, - orderedIds: newOrderedIds, - version: existing.version + 1, - }) - } - - /** - * Gets the ordered list of item IDs for a has-many relation. - */ - getHasManyOrderedIds(key: string): string[] { - const existing = this.hasManyStates.get(key) - if (!existing) return [] - - if (existing.orderedIds !== null) { - return existing.orderedIds - } - - return computeDefaultOrderedIds(existing) - } - - // ==================== Bulk Operations ==================== - /** * Commits all relations (hasOne and hasMany) for an entity. */ commitAllRelations(keyPrefix: string): void { - for (const key of this.relationStates.keys()) { - if (key.startsWith(keyPrefix)) { - this.commitRelation(key) - } - } - - for (const [key, state] of this.hasManyStates) { - if (key.startsWith(keyPrefix)) { - const newServerIds = new Set(state.serverIds) - for (const removedId of state.plannedRemovals.keys()) { - newServerIds.delete(removedId) - } - for (const connectedId of state.plannedAdditions.keys()) { - newServerIds.add(connectedId) - } - this.commitHasMany(key, Array.from(newServerIds)) - } - } + this.hasOne.commitAllRelations(keyPrefix) + this.hasMany.commitAllRelations(keyPrefix) } /** * Resets all relations (hasOne and hasMany) for an entity to server state. */ resetAllRelations(keyPrefix: string): void { - for (const key of this.relationStates.keys()) { - if (key.startsWith(keyPrefix)) { - this.resetRelation(key) - } - } - - for (const key of this.hasManyStates.keys()) { - if (key.startsWith(keyPrefix)) { - this.resetHasMany(key) - } - } + this.hasOne.resetAllRelations(keyPrefix) + this.hasMany.resetAllRelations(keyPrefix) } // ==================== Dirty Tracking ==================== /** - * Gets the list of dirty relations for an entity. + * Gets the list of dirty relations (has-one and has-many) for an entity. */ getDirtyRelations(keyPrefix: string): string[] { const dirtyRelations: string[] = [] - - for (const [key, state] of this.relationStates) { - if (!key.startsWith(keyPrefix)) continue - const fieldName = key.slice(keyPrefix.length) - - if ( - state.currentId !== state.serverId || - state.state !== state.serverState || - Object.keys(state.placeholderData).length > 0 - ) { - dirtyRelations.push(fieldName) - } - } - - for (const [key, state] of this.hasManyStates) { - if (!key.startsWith(keyPrefix)) continue - const fieldName = key.slice(keyPrefix.length) - - if (state.plannedRemovals.size > 0 || state.plannedAdditions.size > 0) { - dirtyRelations.push(fieldName) - } - } - + this.hasOne.collectDirtyRelations(keyPrefix, dirtyRelations) + this.hasMany.collectDirtyRelations(keyPrefix, dirtyRelations) return dirtyRelations } // ==================== Export/Import ==================== - /** - * Exports relation states for given keys. - */ exportRelationStates(keys: string[]): Map { - const result = new Map() - for (const key of keys) { - const state = this.relationStates.get(key) - if (state) { - result.set(key, { - ...state, - placeholderData: { ...state.placeholderData }, - }) - } - } - return result + return this.hasOne.exportRelationStates(keys) } - /** - * Exports has-many states for given keys. - */ exportHasManyStates(keys: string[]): Map { - const result = new Map() - for (const key of keys) { - const state = this.hasManyStates.get(key) - if (state) { - result.set(key, { - serverIds: new Set(state.serverIds), - orderedIds: state.orderedIds ? [...state.orderedIds] : null, - plannedRemovals: new Map(state.plannedRemovals), - plannedAdditions: new Map(state.plannedAdditions), - version: state.version, - }) - } - } - return result + return this.hasMany.exportHasManyStates(keys) } - /** - * Imports relation states from a snapshot. - * Returns the keys that were imported for notification. - */ importRelationStates(states: Map): string[] { - const keys: string[] = [] - for (const [key, state] of states) { - this.writeRelation(key, { - ...state, - placeholderData: { ...state.placeholderData }, - }) - keys.push(key) - } - return keys + return this.hasOne.importRelationStates(states) } - /** - * Imports has-many states from a snapshot. - * Returns the keys that were imported for notification. - */ importHasManyStates(states: Map): string[] { - const keys: string[] = [] - for (const [key, state] of states) { - this.writeHasMany(key, { - serverIds: new Set(state.serverIds), - orderedIds: state.orderedIds ? [...state.orderedIds] : null, - plannedRemovals: new Map(state.plannedRemovals), - plannedAdditions: new Map(state.plannedAdditions), - version: state.version + 1, - }) - keys.push(key) - } - return keys + return this.hasMany.importHasManyStates(states) } + // ==================== Rekey ==================== + /** * Replaces all occurrences of oldId with newId across relation and hasMany states. * Used after persist to rekey temp IDs to server-assigned IDs. */ replaceEntityId(oldId: string, newId: string): void { - // Replace in has-one relation states: currentId, serverId - for (const [key, state] of this.relationStates) { - let changed = false - let currentId = state.currentId - let serverId = state.serverId - - if (currentId === oldId) { currentId = newId; changed = true } - if (serverId === oldId) { serverId = newId; changed = true } - - if (changed) { - this.writeRelation(key, { ...state, currentId, serverId, version: state.version + 1 }) - } - } - - // Replace in has-many states: serverIds, orderedIds, plannedAdditions, plannedRemovals - for (const [key, state] of this.hasManyStates) { - let changed = false - - let serverIds = state.serverIds - if (serverIds.has(oldId)) { - serverIds = new Set(serverIds) - serverIds.delete(oldId) - serverIds.add(newId) - changed = true - } - - let orderedIds = state.orderedIds - if (orderedIds) { - const idx = orderedIds.indexOf(oldId) - if (idx !== -1) { - orderedIds = [...orderedIds] - orderedIds[idx] = newId - changed = true - } - } - - let plannedAdditions = state.plannedAdditions - const additionKind = plannedAdditions.get(oldId) - if (additionKind !== undefined) { - plannedAdditions = new Map(plannedAdditions) - plannedAdditions.delete(oldId) - plannedAdditions.set(newId, additionKind) - changed = true - } - - let plannedRemovals = state.plannedRemovals - if (plannedRemovals.has(oldId)) { - const removalType = plannedRemovals.get(oldId)! - plannedRemovals = new Map(plannedRemovals) - plannedRemovals.delete(oldId) - plannedRemovals.set(newId, removalType) - changed = true - } - - if (changed) { - this.writeHasMany(key, { - serverIds, - orderedIds, - plannedRemovals, - plannedAdditions, - version: state.version + 1, - }) - } - } + this.hasOne.replaceEntityId(oldId, newId) + this.hasMany.replaceEntityId(oldId, newId) } /** @@ -827,78 +229,15 @@ export class RelationStore implements Rekeyable { * Rekeys relation/hasMany entries owned by an entity (changes the parent ID in the key). */ rekeyOwner(oldKeyPrefix: string, newKeyPrefix: string): void { - const toMoveRelations: [string, StoredRelationState][] = [] - for (const [key, value] of this.relationStates) { - if (key.startsWith(oldKeyPrefix)) { - toMoveRelations.push([key, value]) - } - } - for (const [oldKey, value] of toMoveRelations) { - this.relationStates.delete(oldKey) - this.writeRelation(newKeyPrefix + oldKey.slice(oldKeyPrefix.length), value) - } - - const toMoveHasMany: [string, StoredHasManyState][] = [] - for (const [key, value] of this.hasManyStates) { - if (key.startsWith(oldKeyPrefix)) { - toMoveHasMany.push([key, value]) - } - } - for (const [oldKey, value] of toMoveHasMany) { - this.hasManyStates.delete(oldKey) - this.writeHasMany(newKeyPrefix + oldKey.slice(oldKeyPrefix.length), value) - } + this.hasOne.rekeyOwner(oldKeyPrefix, newKeyPrefix) + this.hasMany.rekeyOwner(oldKeyPrefix, newKeyPrefix) } /** * Clears all relation data. */ clear(): void { - this.relationStates.clear() - this.hasManyStates.clear() - this.mutationVersion++ - } -} - -// ==================== Helper Functions ==================== - -/** - * Derives the parent composite key ("parentType:parentId") from a relation key - * ("parentType:parentId:fieldName") by dropping the trailing field segment. - * Entity ids and field names never contain ':', so the parent key is everything - * before the last separator. - */ -function parentKeyFromRelationKey(relationKey: string): string { - const lastSeparator = relationKey.lastIndexOf(':') - return relationKey.slice(0, lastSeparator) -} - -/** - * Computes the default ordered IDs for a has-many relation. - * Order is: serverIds (minus removals) + plannedAdditions - */ -export function computeDefaultOrderedIds(state: StoredHasManyState): string[] { - const result: string[] = [] - - for (const id of state.serverIds) { - if (!state.plannedRemovals.has(id)) { - result.push(id) - } - } - - for (const id of state.plannedAdditions.keys()) { - if (!result.includes(id)) { - result.push(id) - } - } - - return result -} - -function arraysEqual(a: string[], b: string[]): boolean { - if (a.length !== b.length) return false - for (let i = 0; i < a.length; i++) { - if (a[i] !== b[i]) return false + this.hasOne.clear() + this.hasMany.clear() } - return true } diff --git a/packages/bindx/src/store/relationKey.ts b/packages/bindx/src/store/relationKey.ts new file mode 100644 index 0000000..6b6973f --- /dev/null +++ b/packages/bindx/src/store/relationKey.ts @@ -0,0 +1,10 @@ +/** + * Derives the parent composite key ("parentType:parentId") from a relation key + * ("parentType:parentId:fieldName") by dropping the trailing field segment. + * Entity ids and field names never contain ':', so the parent key is everything + * before the last separator. + */ +export function parentKeyFromRelationKey(relationKey: string): string { + const lastSeparator = relationKey.lastIndexOf(':') + return relationKey.slice(0, lastSeparator) +} From d1b837a7d0ab0388d4a89d772f59137c1af74347 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 23 Jun 2026 13:39:17 +0200 Subject: [PATCH 14/19] fix(bindx): make has-one refetch baseline advance non-notifying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ensureEntry()/advanceServerBaselineOnRefetch runs on every has-one read, including during React render. Its refetch branch wrote the new server baseline via the notifying setRelation, calling subscribers mid-render and violating the external-store contract. Add a skipNotify path to SnapshotStore.setRelation and use it here — the parent re-fetch that produced the new embedded reference already notified subscribers. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- packages/bindx/src/handles/HasOneHandle.ts | 9 ++- packages/bindx/src/store/SnapshotStore.ts | 8 +- tests/unit/handles/hasOneHandle.test.ts | 93 ++++++++++++++++++++++ 3 files changed, 108 insertions(+), 2 deletions(-) diff --git a/packages/bindx/src/handles/HasOneHandle.ts b/packages/bindx/src/handles/HasOneHandle.ts index a02e658..a379855 100644 --- a/packages/bindx/src/handles/HasOneHandle.ts +++ b/packages/bindx/src/handles/HasOneHandle.ts @@ -213,6 +213,13 @@ export class HasOneHandle * Does NOT consume the propagation slot — {@link ensureRelatedEntitySnapshot} * owns it. The same reference-change signal drives both, so both run within the * one render that observes a new parent reference. + * + * The baseline write is NON-NOTIFYING: it runs during a render-phase read, and + * the parent re-fetch that produced the new embedded reference already notified + * subscribers. Notifying again here would mutate the store and synchronously call + * subscribers mid-render, violating the external-store contract (cf. + * {@link ensureRelatedEntitySnapshot}, which refreshes server data with skipNotify + * for the same reason). */ private advanceServerBaselineOnRefetch( existing: StoredRelationState, @@ -231,7 +238,7 @@ export class HasOneHandle serverId: embeddedId, state: 'connected', serverState: 'connected', - }) + }, true) } private isLocallyDirty(relation: StoredRelationState): boolean { diff --git a/packages/bindx/src/store/SnapshotStore.ts b/packages/bindx/src/store/SnapshotStore.ts index 5c157dd..72ea4a9 100644 --- a/packages/bindx/src/store/SnapshotStore.ts +++ b/packages/bindx/src/store/SnapshotStore.ts @@ -774,12 +774,18 @@ export class SnapshotStore implements SnapshotVersionBumper { parentId: string, fieldName: string, updates: Partial>, + skipNotify: boolean = false, ): void { const key = this.getRelationKey(parentType, parentId, fieldName) const entityKey = this.getEntityKey(parentType, parentId) const entitySnapshot = this.entitySnapshots.get(entityKey) this.relations.setRelation(key, updates, entitySnapshot, fieldName) - this.notifyRelationSubscribers(key) + // skipNotify is for writes that happen during a render-phase read (e.g. + // HasOneHandle materialization) where the change that triggered them already + // notified subscribers — notifying again would call subscribers mid-render. + if (!skipNotify) { + this.notifyRelationSubscribers(key) + } } commitRelation(parentType: string, parentId: string, fieldName: string): void { diff --git a/tests/unit/handles/hasOneHandle.test.ts b/tests/unit/handles/hasOneHandle.test.ts index 686f358..012e550 100644 --- a/tests/unit/handles/hasOneHandle.test.ts +++ b/tests/unit/handles/hasOneHandle.test.ts @@ -764,6 +764,99 @@ describe('HasOneHandle', () => { }) }) + // ==================== Server-baseline advance on re-fetch ==================== + // + // On a parent re-fetch whose embedded related id changed, a non-dirty has-one + // must advance its server baseline to the new id (and stay clean), while a + // locally-dirty relation must survive the re-fetch. The advance runs during a + // render-phase read and must NOT notify subscribers (the re-fetch already did). + + describe('Server-baseline advance on re-fetch', () => { + test('a re-fetch that changes the related id advances the baseline and stays clean', () => { + store.setEntityData('Article', 'a-1', { + id: 'a-1', + title: 'Test', + author: { id: 'auth-1', name: 'John' }, + }, true) + + const handle = createHasOneHandleRaw() + expect(handle.relatedId).toBe('auth-1') + expect(handle.isDirty).toBe(false) + + // Parent re-fetched: a NEW embedded author reference with a different id. + store.setEntityData('Article', 'a-1', { + id: 'a-1', + title: 'Test', + author: { id: 'auth-2', name: 'Jane' }, + }, true) + + // Reading advances the server baseline to the new related id; still clean. + expect(handle.relatedId).toBe('auth-2') + expect(handle.isDirty).toBe(false) + + const relation = store.getRelation('Article', 'a-1', 'author') + expect(relation?.currentId).toBe('auth-2') + expect(relation?.serverId).toBe('auth-2') + expect(relation?.state).toBe('connected') + expect(relation?.serverState).toBe('connected') + }) + + test('a locally-connected relation survives a re-fetch that changes the embedded id', () => { + store.setEntityData('Article', 'a-1', { + id: 'a-1', + title: 'Test', + author: { id: 'auth-1', name: 'John' }, + }, true) + const handle = createHasOneHandleRaw() + expect(handle.relatedId).toBe('auth-1') + + // Local connect to a different author → dirty. + store.setRelation('Article', 'a-1', 'author', { currentId: 'auth-2', state: 'connected' }) + expect(handle.relatedId).toBe('auth-2') + expect(handle.isDirty).toBe(true) + + // Parent re-fetched at yet another author — the local dirty connect wins. + store.setEntityData('Article', 'a-1', { + id: 'a-1', + title: 'Test', + author: { id: 'auth-3', name: 'Bob' }, + }, true) + + expect(handle.relatedId).toBe('auth-2') + expect(handle.isDirty).toBe(true) + }) + + test('the baseline advance does not notify subscribers during a render-phase read', () => { + store.setEntityData('Article', 'a-1', { + id: 'a-1', + title: 'Test', + author: { id: 'auth-1', name: 'John' }, + }, true) + const handle = createHasOneHandleRaw() + expect(handle.relatedId).toBe('auth-1') + + // Re-fetch with a new related id, setting up the advance on the next read. + store.setEntityData('Article', 'a-1', { + id: 'a-1', + title: 'Test', + author: { id: 'auth-2', name: 'Jane' }, + }, true) + + // Subscribe AFTER the re-fetch notification: any callback fired now would be + // the illegal mid-read notification from the baseline advance (CORR-6). + const entityCb = mock(() => {}) + const relationCb = mock(() => {}) + store.subscribeToEntity('Article', 'a-1', entityCb) + store.subscribeToRelation('Article', 'a-1', 'author', relationCb) + + // This read triggers advanceServerBaselineOnRefetch. + expect(handle.relatedId).toBe('auth-2') + + expect(entityCb).not.toHaveBeenCalled() + expect(relationCb).not.toHaveBeenCalled() + }) + }) + // ==================== Reachability / dirty for created has-one child ==================== describe('Created has-one child via embedded data', () => { From 3827de08da8932fefa329bb35996414475307c2d Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 23 Jun 2026 13:39:29 +0200 Subject: [PATCH 15/19] fix(bindx): guard connectExistingToHasMany against duplicate ordered id connectExistingToHasMany appended to orderedIds unconditionally, so re-materializing the same embedded connect reference surfaced the list item twice. Mirror planHasManyConnection: only touch an explicit order and guard with !includes. Also pin the load-bearing no-downgrade invariant (a connect after a create on the same id must keep emitting a create) on both the planHasManyConnection and connectExistingToHasMany paths. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- packages/bindx/src/store/HasManyStore.ts | 11 +++- tests/unit/store/hasManyAdditionKind.test.ts | 68 ++++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 tests/unit/store/hasManyAdditionKind.test.ts diff --git a/packages/bindx/src/store/HasManyStore.ts b/packages/bindx/src/store/HasManyStore.ts index d5570b7..224c9b8 100644 --- a/packages/bindx/src/store/HasManyStore.ts +++ b/packages/bindx/src/store/HasManyStore.ts @@ -303,8 +303,15 @@ export class HasManyStore { if (newPlannedAdditions.get(itemId) !== 'created') { newPlannedAdditions.set(itemId, 'connected') } - const currentOrderedIds = existing.orderedIds ?? computeDefaultOrderedIds(existing) - const newOrderedIds = [...currentOrderedIds, itemId] + // Only touch an explicit order — the default order already derives from + // plannedAdditions (see computeDefaultOrderedIds). Guard against + // re-appending an id that is already listed: this path re-runs whenever an + // embedded connect reference is re-materialized, and an unconditional + // append would surface the same item twice (mirrors planHasManyConnection). + let newOrderedIds = existing.orderedIds + if (newOrderedIds !== null && !newOrderedIds.includes(itemId)) { + newOrderedIds = [...newOrderedIds, itemId] + } this.writeHasMany(key, { ...existing, orderedIds: newOrderedIds, diff --git a/tests/unit/store/hasManyAdditionKind.test.ts b/tests/unit/store/hasManyAdditionKind.test.ts new file mode 100644 index 0000000..319c29d --- /dev/null +++ b/tests/unit/store/hasManyAdditionKind.test.ts @@ -0,0 +1,68 @@ +import { describe, test, expect } from 'bun:test' +import { RelationStore } from '../../../packages/bindx/src/store/RelationStore.js' + +/** + * Pins the has-many planned-addition KIND invariant after the two-set model + * (plannedConnections + createdEntities) collapsed into a single + * Map in HasManyStore. + * + * The load-bearing rule is "no downgrade": once an id is recorded as 'created' + * (a newly created, never-persisted entity), a later connect MUST NOT rewrite it + * to 'connected'. MutationCollector reads the kind verbatim — 'created' → emit a + * create, 'connected' → emit a connect — so a downgrade would emit a connect to a + * temp id the server never creates: a silently dropped write (data loss). Both the + * planHasManyConnection and connectExistingToHasMany paths carry the guard, so + * both are pinned here. + */ +describe('HasMany planned-addition kind', () => { + const KEY = 'Article:a1:tags' + + describe('no downgrade: created stays created', () => { + test('planHasManyConnection does not downgrade a created addition', () => { + const relations = new RelationStore() + relations.addToHasMany(KEY, 'temp-1') + expect(relations.getHasMany(KEY)?.plannedAdditions.get('temp-1')).toBe('created') + + // connect() after add() on the SAME id must keep it a create. + relations.planHasManyConnection(KEY, 'temp-1') + expect(relations.getHasMany(KEY)?.plannedAdditions.get('temp-1')).toBe('created') + }) + + test('connectExistingToHasMany does not downgrade a created addition', () => { + const relations = new RelationStore() + relations.addToHasMany(KEY, 'temp-1') + expect(relations.getHasMany(KEY)?.plannedAdditions.get('temp-1')).toBe('created') + + // The embedded-connect materialization path must not downgrade either. + relations.connectExistingToHasMany(KEY, 'temp-1') + expect(relations.getHasMany(KEY)?.plannedAdditions.get('temp-1')).toBe('created') + }) + + test('a genuine connect of a never-added id is recorded as connected', () => { + const relations = new RelationStore() + relations.connectExistingToHasMany(KEY, 'persisted-1') + expect(relations.getHasMany(KEY)?.plannedAdditions.get('persisted-1')).toBe('connected') + }) + }) + + describe('connectExistingToHasMany ordered-id dedup', () => { + test('re-connecting the same id does not duplicate it in the ordered list', () => { + const relations = new RelationStore() + // The same embedded connect reference can be materialized more than once. + relations.connectExistingToHasMany(KEY, 'persisted-1') + relations.connectExistingToHasMany(KEY, 'persisted-1') + + expect(relations.getHasManyOrderedIds(KEY)).toEqual(['persisted-1']) + expect(relations.getHasMany(KEY)?.plannedAdditions.get('persisted-1')).toBe('connected') + }) + + test('connecting an id already present as a server member does not duplicate it', () => { + const relations = new RelationStore() + relations.setHasManyServerIds(KEY, ['persisted-1', 'persisted-2']) + + relations.connectExistingToHasMany(KEY, 'persisted-1') + + expect(relations.getHasManyOrderedIds(KEY)).toEqual(['persisted-1', 'persisted-2']) + }) + }) +}) From 74c501b0b52dfb463134e9ef94dcf45f849e1ab1 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 23 Jun 2026 13:39:34 +0200 Subject: [PATCH 16/19] test(bindx): pin reachability cache invalidation for has-one edges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The memoization suite covered only has-many edge changes. Add has-one setRelation connect/disconnect cases and a direct getMutationVersion test proving the facade counter sums both sub-stores — so dropping the has-one term (a stale-cache create-detection bug) would be caught. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- .../store/reachabilityMemoization.test.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/unit/store/reachabilityMemoization.test.ts b/tests/unit/store/reachabilityMemoization.test.ts index e5d42df..2d7bec1 100644 --- a/tests/unit/store/reachabilityMemoization.test.ts +++ b/tests/unit/store/reachabilityMemoization.test.ts @@ -115,6 +115,56 @@ describe('reachability memoization', () => { expect(sortedKeys(result)).toEqual([]) }) + // Has-one edge variants of the add/remove tests above. getMutationVersion() + // SUMS both sub-store counters, so a has-one setRelation (connect/disconnect) + // must invalidate the cache just like a has-many edge change. If the has-one + // term were dropped from the sum, a created child connected through a has-one + // would be served stale — a dropped or phantom create. + test('connecting a created child via has-one invalidates the cache and is reflected', () => { + const h = createHarness() + h.entitySnapshots.setData('Article:a1', 'a1', 'Article', { id: 'a1' }, true) + h.meta.setExistsOnServer('Article:a1', true) + // A created (never-persisted) author exists but is not yet reachable. + h.entitySnapshots.setData('Author:au1', 'au1', 'Author', { id: 'au1' }, false) + expect(sortedKeys(h.analyzer.computeReachableCreated())).toEqual([]) + const calls = h.walkCount() + + h.relations.setRelation('Article:a1:author', { currentId: 'au1', state: 'connected' }, undefined, 'author') + + const result = h.analyzer.computeReachableCreated() + expect(h.walkCount()).toBeGreaterThan(calls) // recomputed, not served stale + expect(sortedKeys(result)).toEqual(['Author:au1']) + }) + + test('disconnecting a created child via has-one invalidates the cache and drops it', () => { + const h = createHarness() + h.entitySnapshots.setData('Article:a1', 'a1', 'Article', { id: 'a1' }, true) + h.meta.setExistsOnServer('Article:a1', true) + h.entitySnapshots.setData('Author:au1', 'au1', 'Author', { id: 'au1' }, false) + h.relations.setRelation('Article:a1:author', { currentId: 'au1', state: 'connected' }, undefined, 'author') + expect(sortedKeys(h.analyzer.computeReachableCreated())).toEqual(['Author:au1']) + const calls = h.walkCount() + + h.relations.setRelation('Article:a1:author', { currentId: null, state: 'disconnected' }, undefined, 'author') + + const result = h.analyzer.computeReachableCreated() + expect(h.walkCount()).toBeGreaterThan(calls) + expect(sortedKeys(result)).toEqual([]) + }) + + test('RelationStore.getMutationVersion sums has-one and has-many writes', () => { + const relations = new RelationStore() + const v0 = relations.getMutationVersion() + + relations.setRelation('Article:a1:author', { currentId: 'au1', state: 'connected' }, undefined, 'author') + const v1 = relations.getMutationVersion() + expect(v1).toBeGreaterThan(v0) // has-one write counted + + relations.addToHasMany('Article:a1:comments', 'c1') + const v2 = relations.getMutationVersion() + expect(v2).toBeGreaterThan(v1) // has-many write also counted (the sum) + }) + test('flipping existsOnServer invalidates the cache', () => { const h = createHarness() // A top-level created entity (root) — reported as a create until it exists. From 544ba207c3671d2c44a3c77bea6d0592dfa60ea8 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 23 Jun 2026 13:39:43 +0200 Subject: [PATCH 17/19] docs(bindx): document bare-id contract of getParentKeysForChild The reverse parent lookup matches on a bare child id, deliberately relying on the store-wide global-id-uniqueness invariant (the same one behind EntitySnapshotStore idIndex/keyForId and the forward reachability walk). Document this on getParentKeysForChild and getParentKeys, and pin it with a test so a future type-aware change trips deliberately. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- packages/bindx/src/store/RelationStore.ts | 8 ++++++++ packages/bindx/src/store/SubscriptionManager.ts | 4 ++++ tests/unit/store/getParentKeysForChild.test.ts | 16 ++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/packages/bindx/src/store/RelationStore.ts b/packages/bindx/src/store/RelationStore.ts index 43e10b0..de56d45 100644 --- a/packages/bindx/src/store/RelationStore.ts +++ b/packages/bindx/src/store/RelationStore.ts @@ -138,6 +138,14 @@ export class RelationStore implements Rekeyable { * currently has a LIVE relation edge pointing at {@link childId} — the reverse * of {@link getLiveChildIds}. Unions both sub-stores' results, scanning the * same live edges the forward query reads so the two never disagree. + * + * {@link childId} is a BARE entity id (no type): relation state records child ids + * without their type, so the match is by id alone. This deliberately relies on + * the store-wide invariant that entity ids are globally unique across types + * (server UUIDs / minted temp ids — the same invariant behind + * EntitySnapshotStore's idIndex / keyForId), so a bare id resolves to exactly one + * entity. The forward reachability walk matches on the same bare ids, so making + * this type-aware in isolation would only diverge the two halves of one query. */ getParentKeysForChild(childId: string): Set { const parents = new Set() diff --git a/packages/bindx/src/store/SubscriptionManager.ts b/packages/bindx/src/store/SubscriptionManager.ts index a44780b..1e937b1 100644 --- a/packages/bindx/src/store/SubscriptionManager.ts +++ b/packages/bindx/src/store/SubscriptionManager.ts @@ -147,6 +147,10 @@ export class SubscriptionManager implements Rekeyable { * Derives the parent entity keys for a child entity key by reading the live * relation edges via the injected lookup. The child's bare id is the part of * the key after the first ':' ("entityType:id"). + * + * The child's TYPE is intentionally dropped — {@link ParentKeyLookup} matches on + * the bare id, relying on the store-wide global-id-uniqueness invariant (see + * {@link RelationStore.getParentKeysForChild}). */ private getParentKeys(childKey: string): Set { if (!this.parentKeyLookup) return new Set() diff --git a/tests/unit/store/getParentKeysForChild.test.ts b/tests/unit/store/getParentKeysForChild.test.ts index 8debbb1..968fa1f 100644 --- a/tests/unit/store/getParentKeysForChild.test.ts +++ b/tests/unit/store/getParentKeysForChild.test.ts @@ -82,6 +82,22 @@ describe('RelationStore.getParentKeysForChild', () => { expect(relations.getParentKeysForChild('art1')).toEqual(new Set(['Author:a1', 'Tag:t1'])) }) + test('matches purely on the bare child id (global-id-uniqueness invariant)', () => { + const relations = new RelationStore() + + // The reverse lookup receives a BARE id (no entity type) and matches by id + // alone, deliberately relying on the store-wide invariant that ids are unique + // across types. A single bare id therefore resolves to every parent that + // anchors it, regardless of those parents' types. This pins the bare-id + // contract: making the lookup type-aware in isolation would change this and + // must be a deliberate decision (it would also have to move in lockstep with + // the forward reachability walk, which matches on the same bare ids). + relations.setRelation('Author:a1:featured', { currentId: 'shared', state: 'connected' }, undefined, 'featured') + relations.addToHasMany('Tag:t1:articles', 'shared') + + expect(relations.getParentKeysForChild('shared')).toEqual(new Set(['Author:a1', 'Tag:t1'])) + }) + test('cross-check: reverse query agrees with getLiveChildIds over a randomized sequence', () => { const relations = new RelationStore() const parents = ['Author:a1', 'Author:a2', 'Tag:t1', 'Tag:t2'] From 8ec75827efa32dc74c32211b395f95b135ec7a93 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 23 Jun 2026 14:34:33 +0200 Subject: [PATCH 18/19] perf(bindx): back relation queries with a bidirectional edge index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getParentKeysForChild and getLiveChildIds scanned every relation entry, so parent-notification and the reachability walk were O(total relations) per call — a per-edit cost that scaled with store size (benchmarked ~550 µs/edit at 4000 rows vs ~3 µs before the childToParents map was removed). Introduce RelationEdgeIndex: a bidirectional, reference-counted index of live parent<->child edges that each sub-store maintains through its single write/delete chokepoint by diffing the previous against the next live set. Both queries become O(degree) and the two directions stay consistent by construction (the failure mode of the old append-only childToParents map). Liveness is now a single predicate per sub-store, consumed by the chokepoint, instead of being duplicated across the forward and reverse scans. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- packages/bindx/src/store/HasManyStore.ts | 87 +++++++++++++------ packages/bindx/src/store/HasOneStore.ts | 77 +++++++++++----- packages/bindx/src/store/RelationEdgeIndex.ts | 79 +++++++++++++++++ packages/bindx/src/store/relationKey.ts | 10 +++ 4 files changed, 206 insertions(+), 47 deletions(-) create mode 100644 packages/bindx/src/store/RelationEdgeIndex.ts diff --git a/packages/bindx/src/store/HasManyStore.ts b/packages/bindx/src/store/HasManyStore.ts index 224c9b8..aea0b36 100644 --- a/packages/bindx/src/store/HasManyStore.ts +++ b/packages/bindx/src/store/HasManyStore.ts @@ -1,4 +1,5 @@ -import { parentKeyFromRelationKey } from './relationKey.js' +import { parentKeyFromOwnerPrefix, parentKeyFromRelationKey } from './relationKey.js' +import { RelationEdgeIndex } from './RelationEdgeIndex.js' function setsEqual(a: Set, b: Set): boolean { if (a.size !== b.size) return false @@ -82,14 +83,45 @@ export class HasManyStore { /** Has-many list states keyed by "parentType:parentId:fieldName" */ private readonly hasManyStates = new Map() + /** + * Bidirectional live-edge index, maintained by {@link writeHasMany} / + * {@link deleteHasMany} so the parent↔child queries are O(degree) and the two + * directions stay consistent by construction. + */ + private readonly edges = new RelationEdgeIndex() + private mutationVersion = 0 getMutationVersion(): number { return this.mutationVersion } + /** + * The single write chokepoint. Reconciles the edge index by diffing the live + * members of the previous state against the next, so every state-changing path + * (server ids / planned add/remove / move / import / replaceEntityId / ...) + * 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 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.mutationVersion++ + } + + /** + * The single delete chokepoint — removes an entry and its live edges. Used by + * the bulk remove and rekey-owner paths so they don't leak edges. + */ + private deleteHasMany(key: string): void { + const existing = this.hasManyStates.get(key) + if (!existing) return + const parentKey = parentKeyFromRelationKey(key) + for (const id of liveHasManyChildIds(existing)) this.edges.removeEdge(parentKey, id) + this.hasManyStates.delete(key) this.mutationVersion++ } @@ -408,48 +440,31 @@ export class HasManyStore { /** * Collects the ids of child entities reachable through LIVE has-many edges - * (key prefix "parentType:parentId:"): - * effective members = (serverIds ∪ plannedAdditions.keys()) minus plannedRemovals. + * (key prefix "parentType:parentId:") — an O(degree) read of the edge index. */ collectLiveChildIds(keyPrefix: string, ids: Set): void { - for (const [key, state] of this.hasManyStates) { - if (!key.startsWith(keyPrefix)) continue - for (const id of state.serverIds) { - if (!state.plannedRemovals.has(id)) ids.add(id) - } - for (const id of state.plannedAdditions.keys()) { - if (!state.plannedRemovals.has(id)) ids.add(id) - } - } + this.edges.collectChildren(parentKeyFromOwnerPrefix(keyPrefix), ids) } /** * Adds the composite parent keys of every LIVE has-many edge containing - * {@link childId}: childId ∈ (serverIds ∪ plannedAdditions.keys()) and - * childId ∉ plannedRemovals. + * {@link childId} — an O(degree) read of the edge index, the exact reverse of + * {@link collectLiveChildIds}. */ collectParentKeysForChild(childId: string, parents: Set): void { - for (const [key, state] of this.hasManyStates) { - if (state.plannedRemovals.has(childId)) continue - if (state.serverIds.has(childId) || state.plannedAdditions.has(childId)) { - parents.add(parentKeyFromRelationKey(key)) - } - } + this.edges.collectParents(childId, parents) } /** * Removes all has-many state owned by an entity (keys under the given owner - * prefix). Bumps the mutation version once if anything was removed. + * prefix), dropping each entry's edges through {@link deleteHasMany}. */ removeOwnedRelations(keyPrefix: string): void { - let changed = false for (const key of [...this.hasManyStates.keys()]) { if (key.startsWith(keyPrefix)) { - this.hasManyStates.delete(key) - changed = true + this.deleteHasMany(key) } } - if (changed) this.mutationVersion++ } /** @@ -592,6 +607,8 @@ export class HasManyStore { /** * Rekeys has-many entries owned by an entity (changes the parent ID in the key). + * Routing through {@link deleteHasMany} + {@link writeHasMany} migrates the edge + * index from the old parent key to the new one for free. */ rekeyOwner(oldKeyPrefix: string, newKeyPrefix: string): void { const toMove: [string, StoredHasManyState][] = [] @@ -601,7 +618,7 @@ export class HasManyStore { } } for (const [oldKey, value] of toMove) { - this.hasManyStates.delete(oldKey) + this.deleteHasMany(oldKey) this.writeHasMany(newKeyPrefix + oldKey.slice(oldKeyPrefix.length), value) } } @@ -611,6 +628,24 @@ export class HasManyStore { */ clear(): void { this.hasManyStates.clear() + this.edges.clear() this.mutationVersion++ } } + +/** + * The single liveness predicate for has-many membership: effective members are + * (serverIds ∪ plannedAdditions) minus plannedRemovals. Defined once and consumed + * by the write chokepoint so the forward/reverse index can never drift from it. + */ +function liveHasManyChildIds(state: StoredHasManyState | undefined): Set { + const live = new Set() + if (!state) return live + for (const id of state.serverIds) { + if (!state.plannedRemovals.has(id)) live.add(id) + } + for (const id of state.plannedAdditions.keys()) { + if (!state.plannedRemovals.has(id)) live.add(id) + } + return live +} diff --git a/packages/bindx/src/store/HasOneStore.ts b/packages/bindx/src/store/HasOneStore.ts index 97d5649..574cab1 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 { parentKeyFromRelationKey } from './relationKey.js' +import { parentKeyFromOwnerPrefix, parentKeyFromRelationKey } from './relationKey.js' +import { RelationEdgeIndex } from './RelationEdgeIndex.js' /** * Relation state stored in SnapshotStore @@ -25,14 +26,47 @@ export class HasOneStore { /** Relation states keyed by "parentType:parentId:fieldName" */ private readonly relationStates = new Map() + /** + * Bidirectional live-edge index, maintained by {@link writeRelation} / + * {@link deleteRelation} so the parent↔child queries are O(degree) and the two + * directions stay consistent by construction. + */ + private readonly edges = new RelationEdgeIndex() + private mutationVersion = 0 getMutationVersion(): number { return this.mutationVersion } + /** + * The single write chokepoint. Reconciles the edge index by diffing the live + * child of the previous state against the next, so every state-changing path + * (set/commit/reset/import/replaceEntityId/...) keeps the index correct without + * tracking the reverse direction itself. + */ private writeRelation(key: string, state: StoredRelationState): void { + const oldChild = liveHasOneChildId(this.relationStates.get(key)) + const newChild = liveHasOneChildId(state) this.relationStates.set(key, state) + if (oldChild !== newChild) { + const parentKey = parentKeyFromRelationKey(key) + if (oldChild !== null) this.edges.removeEdge(parentKey, oldChild) + if (newChild !== null) this.edges.addEdge(parentKey, newChild) + } + this.mutationVersion++ + } + + /** + * The single delete chokepoint — removes an entry and its live edge. Used by + * the bulk remove and rekey-owner paths so they don't leak edges. + */ + private deleteRelation(key: string): void { + const existing = this.relationStates.get(key) + if (!existing) return + const child = liveHasOneChildId(existing) + if (child !== null) this.edges.removeEdge(parentKeyFromRelationKey(key), child) + this.relationStates.delete(key) this.mutationVersion++ } @@ -132,43 +166,31 @@ export class HasOneStore { /** * Collects the ids of child entities reachable through LIVE has-one edges - * (key prefix "parentType:parentId:"): currentId, when the relation is not - * disconnected/deleted. + * (key prefix "parentType:parentId:") — an O(degree) read of the edge index. */ collectLiveChildIds(keyPrefix: string, ids: Set): void { - for (const [key, state] of this.relationStates) { - if (!key.startsWith(keyPrefix)) continue - if (state.currentId !== null && state.state !== 'deleted') { - ids.add(state.currentId) - } - } + this.edges.collectChildren(parentKeyFromOwnerPrefix(keyPrefix), ids) } /** * Adds the composite parent keys of every LIVE has-one edge pointing at - * {@link childId} (currentId === childId, when the relation is not 'deleted'). + * {@link childId} — an O(degree) read of the edge index, the exact reverse of + * {@link collectLiveChildIds}. */ collectParentKeysForChild(childId: string, parents: Set): void { - for (const [key, state] of this.relationStates) { - if (state.currentId === childId && state.state !== 'deleted') { - parents.add(parentKeyFromRelationKey(key)) - } - } + this.edges.collectParents(childId, parents) } /** * Removes all has-one state owned by an entity (keys under the given owner - * prefix). Bumps the mutation version once if anything was removed. + * prefix), dropping each entry's edge through {@link deleteRelation}. */ removeOwnedRelations(keyPrefix: string): void { - let changed = false for (const key of [...this.relationStates.keys()]) { if (key.startsWith(keyPrefix)) { - this.relationStates.delete(key) - changed = true + this.deleteRelation(key) } } - if (changed) this.mutationVersion++ } /** @@ -265,6 +287,8 @@ export class HasOneStore { /** * Rekeys has-one entries owned by an entity (changes the parent ID in the key). + * Routing through {@link deleteRelation} + {@link writeRelation} migrates the + * edge index from the old parent key to the new one for free. */ rekeyOwner(oldKeyPrefix: string, newKeyPrefix: string): void { const toMove: [string, StoredRelationState][] = [] @@ -274,7 +298,7 @@ export class HasOneStore { } } for (const [oldKey, value] of toMove) { - this.relationStates.delete(oldKey) + this.deleteRelation(oldKey) this.writeRelation(newKeyPrefix + oldKey.slice(oldKeyPrefix.length), value) } } @@ -284,6 +308,17 @@ export class HasOneStore { */ clear(): void { this.relationStates.clear() + this.edges.clear() this.mutationVersion++ } } + +/** + * The single liveness predicate for a has-one edge: the related id is live when + * the relation is connected to it and not deleted. Defined once and consumed by + * the write chokepoint so the forward/reverse index can never drift from it. + */ +function liveHasOneChildId(state: StoredRelationState | undefined): string | null { + if (!state) return null + return state.currentId !== null && state.state !== 'deleted' ? state.currentId : null +} diff --git a/packages/bindx/src/store/RelationEdgeIndex.ts b/packages/bindx/src/store/RelationEdgeIndex.ts new file mode 100644 index 0000000..43ddf73 --- /dev/null +++ b/packages/bindx/src/store/RelationEdgeIndex.ts @@ -0,0 +1,79 @@ +/** + * Bidirectional index of LIVE relation edges between a parent entity + * ("parentType:parentId") and a child entity id (a bare id). + * + * Both directions are updated together by {@link addEdge}/{@link removeEdge}, so + * the forward query ({@link collectChildren}) and the reverse query + * ({@link collectParents}) can never disagree — that was the failure mode of the + * old append-only `childToParents` map, which was updated on connect but not on + * disconnect. Both queries are O(degree), not O(total edges). + * + * Edges are reference-counted: one parent entity may reach the same child through + * more than one relation field (e.g. `author` and `coauthor`, or two has-many + * fields), so an edge survives until the last field that contributes it drops it. + * + * The index stores only the LIVE edge set; turning relation state (server ids, + * planned additions/removals, has-one state) into that set is the owning + * sub-store's job, done once per write by diffing the previous against the next + * live set (see {@link HasOneStore}/{@link HasManyStore}). The index itself knows + * nothing about liveness rules. + */ +export class RelationEdgeIndex { + /** parentKey ("parentType:parentId") → childId → refcount */ + private readonly forward = new Map>() + /** childId → parentKey → refcount */ + private readonly reverse = new Map>() + + addEdge(parentKey: string, childId: string): void { + increment(this.forward, parentKey, childId) + increment(this.reverse, childId, parentKey) + } + + removeEdge(parentKey: string, childId: string): void { + decrement(this.forward, parentKey, childId) + decrement(this.reverse, childId, parentKey) + } + + /** Adds the live child ids of {@link parentKey} to {@link out}. */ + collectChildren(parentKey: string, out: Set): void { + const children = this.forward.get(parentKey) + if (children) { + for (const childId of children.keys()) out.add(childId) + } + } + + /** Adds the parent keys that hold a live edge to {@link childId} to {@link out}. */ + collectParents(childId: string, out: Set): void { + const parents = this.reverse.get(childId) + if (parents) { + for (const parentKey of parents.keys()) out.add(parentKey) + } + } + + clear(): void { + this.forward.clear() + this.reverse.clear() + } +} + +function increment(map: Map>, outer: string, inner: string): void { + let counts = map.get(outer) + if (!counts) { + counts = new Map() + map.set(outer, counts) + } + counts.set(inner, (counts.get(inner) ?? 0) + 1) +} + +function decrement(map: Map>, outer: string, inner: string): void { + const counts = map.get(outer) + if (!counts) return + const count = counts.get(inner) + if (count === undefined) return + if (count > 1) { + counts.set(inner, count - 1) + return + } + counts.delete(inner) + if (counts.size === 0) map.delete(outer) +} diff --git a/packages/bindx/src/store/relationKey.ts b/packages/bindx/src/store/relationKey.ts index 6b6973f..f3c2cd5 100644 --- a/packages/bindx/src/store/relationKey.ts +++ b/packages/bindx/src/store/relationKey.ts @@ -8,3 +8,13 @@ export function parentKeyFromRelationKey(relationKey: string): string { const lastSeparator = relationKey.lastIndexOf(':') return relationKey.slice(0, lastSeparator) } + +/** + * Derives the parent composite key ("parentType:parentId") from an owner key + * prefix ("parentType:parentId:") by dropping the trailing separator. Callers + * pass the owner prefix used for relation-key lookups; this maps it to the key + * the {@link RelationEdgeIndex} stores edges under. + */ +export function parentKeyFromOwnerPrefix(ownerPrefix: string): string { + return ownerPrefix.endsWith(':') ? ownerPrefix.slice(0, -1) : ownerPrefix +} From 0c662bbe84a1a3751f073ddca5efd95f426c4faa Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 23 Jun 2026 14:34:38 +0200 Subject: [PATCH 19/19] test(bindx): cover the relation edge index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pin the reference-counting and migration cases the bidirectional index introduces — same parent reaching a child through several fields, id replacement, owner rekey, bulk removal, commit/reset — alongside the existing randomized forward/reverse cross-check. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01PkeRcE9offiYmcy3c7fiuj --- tests/unit/store/relationEdgeIndex.test.ts | 138 +++++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 tests/unit/store/relationEdgeIndex.test.ts diff --git a/tests/unit/store/relationEdgeIndex.test.ts b/tests/unit/store/relationEdgeIndex.test.ts new file mode 100644 index 0000000..37c4c8e --- /dev/null +++ b/tests/unit/store/relationEdgeIndex.test.ts @@ -0,0 +1,138 @@ +import { describe, test, expect } from 'bun:test' +import { RelationStore } from '../../../packages/bindx/src/store/RelationStore.js' +import { RelationEdgeIndex } from '../../../packages/bindx/src/store/RelationEdgeIndex.js' + +/** + * Edge cases introduced by the bidirectional {@link RelationEdgeIndex} that backs + * getLiveChildIds / getParentKeysForChild. The randomized cross-check in + * getParentKeysForChild.test.ts proves forward/reverse agreement over long + * sequences; these tests pin the specific reference-counting and migration cases + * (the same parent reaching a child through several fields, id replacement, owner + * rekey, bulk removal) where a naive reverse map would drift. + */ +describe('RelationEdgeIndex (unit)', () => { + test('reference-counts an edge contributed by multiple fields', () => { + const index = new RelationEdgeIndex() + index.addEdge('Author:a1', 'art1') + index.addEdge('Author:a1', 'art1') // second field on the same parent + + const parents1 = new Set() + index.collectParents('art1', parents1) + expect(parents1).toEqual(new Set(['Author:a1'])) + + index.removeEdge('Author:a1', 'art1') // one field drops it — still live + const parents2 = new Set() + index.collectParents('art1', parents2) + expect(parents2).toEqual(new Set(['Author:a1'])) + + index.removeEdge('Author:a1', 'art1') // last field drops it — gone + const parents3 = new Set() + index.collectParents('art1', parents3) + expect(parents3).toEqual(new Set()) + + // Forward direction is symmetric. + const children = new Set() + index.collectChildren('Author:a1', children) + expect(children).toEqual(new Set()) + }) + + test('over-decrement is a safe no-op', () => { + const index = new RelationEdgeIndex() + index.addEdge('P', 'c') + index.removeEdge('P', 'c') + index.removeEdge('P', 'c') // already gone — must not throw or go negative + const parents = new Set() + index.collectParents('c', parents) + expect(parents).toEqual(new Set()) + }) +}) + +describe('RelationStore edge index integration', () => { + test('two has-one fields to the same child keep the parent until both drop', () => { + const relations = new RelationStore() + relations.setRelation('Author:a1:featured', { currentId: 'art1', state: 'connected' }, undefined, 'featured') + relations.setRelation('Author:a1:secondary', { currentId: 'art1', state: 'connected' }, undefined, 'secondary') + + expect(relations.getParentKeysForChild('art1')).toEqual(new Set(['Author:a1'])) + + relations.setRelation('Author:a1:featured', { currentId: null, state: 'disconnected' }, undefined, 'featured') + expect(relations.getParentKeysForChild('art1')).toEqual(new Set(['Author:a1'])) // secondary still holds it + + relations.setRelation('Author:a1:secondary', { currentId: null, state: 'disconnected' }, undefined, 'secondary') + expect(relations.getParentKeysForChild('art1')).toEqual(new Set()) + }) + + test('a child reached via has-one AND has-many of one parent survives dropping either', () => { + const relations = new RelationStore() + relations.setRelation('Author:a1:featured', { currentId: 'art1', state: 'connected' }, undefined, 'featured') + relations.addToHasMany('Author:a1:articles', 'art1') + + expect(relations.getParentKeysForChild('art1')).toEqual(new Set(['Author:a1'])) + + relations.removeFromHasMany('Author:a1:articles', 'art1', 'disconnect') + expect(relations.getParentKeysForChild('art1')).toEqual(new Set(['Author:a1'])) // has-one still holds + + relations.setRelation('Author:a1:featured', { currentId: null, state: 'disconnected' }, undefined, 'featured') + expect(relations.getParentKeysForChild('art1')).toEqual(new Set()) + }) + + test('replaceEntityId migrates the child id in both directions', () => { + const relations = new RelationStore() + relations.addToHasMany('Author:a1:articles', 'old1') + relations.setRelation('Author:a1:featured', { currentId: 'old1', state: 'connected' }, undefined, 'featured') + + expect(relations.getParentKeysForChild('old1')).toEqual(new Set(['Author:a1'])) + expect(relations.getLiveChildIds('Author:a1:')).toContain('old1') + + relations.replaceEntityId('old1', 'new1') + + expect(relations.getParentKeysForChild('old1')).toEqual(new Set()) + expect(relations.getParentKeysForChild('new1')).toEqual(new Set(['Author:a1'])) + const live = relations.getLiveChildIds('Author:a1:') + expect(live).toContain('new1') + expect(live).not.toContain('old1') + }) + + test('rekeyOwner migrates the parent key in both directions', () => { + const relations = new RelationStore() + relations.addToHasMany('Author:a1:articles', 'art1') + relations.setRelation('Author:a1:featured', { currentId: 'art2', state: 'connected' }, undefined, 'featured') + + relations.rekeyOwner('Author:a1:', 'Author:p1:') + + expect(relations.getParentKeysForChild('art1')).toEqual(new Set(['Author:p1'])) + expect(relations.getParentKeysForChild('art2')).toEqual(new Set(['Author:p1'])) + expect(relations.getLiveChildIds('Author:a1:')).toEqual([]) + expect(new Set(relations.getLiveChildIds('Author:p1:'))).toEqual(new Set(['art1', 'art2'])) + }) + + test('removeOwnedRelations drops all of an owner edges', () => { + const relations = new RelationStore() + relations.addToHasMany('Author:a1:articles', 'art1') + relations.setRelation('Author:a1:featured', { currentId: 'art2', state: 'connected' }, undefined, 'featured') + + relations.removeOwnedRelations('Author:a1:') + + expect(relations.getParentKeysForChild('art1')).toEqual(new Set()) + expect(relations.getParentKeysForChild('art2')).toEqual(new Set()) + expect(relations.getLiveChildIds('Author:a1:')).toEqual([]) + }) + + test('commit and reset keep the index consistent with membership', () => { + const relations = new RelationStore() + relations.setHasManyServerIds('Author:a1:articles', ['s1', 's2']) + relations.addToHasMany('Author:a1:articles', 'c1') + expect(new Set(relations.getLiveChildIds('Author:a1:'))).toEqual(new Set(['s1', 's2', 'c1'])) + + // commit folds plannedAdditions into serverIds — live membership unchanged. + relations.commitHasMany('Author:a1:articles', ['s1', 's2', 'c1']) + expect(new Set(relations.getLiveChildIds('Author:a1:'))).toEqual(new Set(['s1', 's2', 'c1'])) + expect(relations.getParentKeysForChild('c1')).toEqual(new Set(['Author:a1'])) + + // plan a removal then reset — reset restores the full server membership. + relations.planHasManyRemoval('Author:a1:articles', 's1', 'disconnect') + expect(relations.getParentKeysForChild('s1')).toEqual(new Set()) // removed → not live + relations.resetHasMany('Author:a1:articles') + expect(relations.getParentKeysForChild('s1')).toEqual(new Set(['Author:a1'])) + }) +})