fix(bindx): stop nested collection recursing through relation cycles

packages/bindx/src/persistence/MutationCollector.ts

@@ -102,6 +102,14 @@ export class MutationCollector implements MutationDataCollector {
 	private readonly _relationFields = new Map<string, CollectedRelationField>()
 	private readonly _nestedCreates: CollectedNestedCreate[] = []
 	private readonly _nestedUpdates: CollectedNestedUpdate[] = []
+	/**
+	 * Entities whose data collection is still on the stack. A schema may point two
+	 * hasOne relations back at each other (`page.publishedRevision` ↔ `revision.page`),
+	 * and once both sides are in the store the nested-update walk would recurse until
+	 * the stack blows. Re-entering an entity means an ancestor frame already emits its
+	 * changes, so the inner edge contributes nothing (see issue #88).
+	 */
+	private readonly _collecting = new Set<string>()
 
 	constructor(
 		private readonly store: SnapshotStore,
@@ -133,6 +141,7 @@ export class MutationCollector implements MutationDataCollector {
 		this._relationFields.clear()
 		this._nestedCreates.length = 0
 		this._nestedUpdates.length = 0
+		this._collecting.clear()
 	}
 
 	setExcludedEntityKeys(keys: ReadonlySet<string>): void {
@@ -313,21 +322,30 @@ export class MutationCollector implements MutationDataCollector {
 			throw new Error(`Entity type '${entityType}' not found in schema`)
 		}
 
-		const mutation: Record<string, unknown> = {}
+		const key = this.entityKey(entityType, entityId)
+		if (this._collecting.has(key)) {
+			return null
+		}
+		this._collecting.add(key)
+		try {
+			const mutation: Record<string, unknown> = {}
 
-		// Collect scalar field changes
-		this.collectScalarChanges(entityType, snapshot, mutation)
+			// Collect scalar field changes
+			this.collectScalarChanges(entityType, snapshot, mutation)
 
-		// Materialize placeholder-backed creating-state hasOne relations before
-		// collecting, so the collection phase can remain a pure read over the
-		// store. Embedded data is not materialized here — for existing
-		// (server-side) parents, hasMany state is already authoritative.
-		this.materializeForUpdate(entityType, entityId)
+			// Materialize placeholder-backed creating-state hasOne relations before
+			// collecting, so the collection phase can remain a pure read over the
+			// store. Embedded data is not materialized here — for existing
+			// (server-side) parents, hasMany state is already authoritative.
+			this.materializeForUpdate(entityType, entityId)
 
-		// Collect relation changes
-		this.collectRelationChanges(entityType, entityId, mutation)
+			// Collect relation changes
+			this.collectRelationChanges(entityType, entityId, mutation)
 
-		return Object.keys(mutation).length > 0 ? mutation : null
+			return Object.keys(mutation).length > 0 ? mutation : null
+		} finally {
+			this._collecting.delete(key)
+		}
 	}
 
 	/**
@@ -347,6 +365,23 @@ export class MutationCollector implements MutationDataCollector {
 			throw new Error(`Entity type '${entityType}' not found in schema`)
 		}
 
+		const key = this.entityKey(entityType, entityId)
+		if (this._collecting.has(key)) {
+			return null
+		}
+		this._collecting.add(key)
+		try {
+			return this.buildCreateData(entityType, entityId, snapshot)
+		} finally {
+			this._collecting.delete(key)
+		}
+	}
+
+	private buildCreateData(
+		entityType: string,
+		entityId: string,
+		snapshot: EntitySnapshot,
+	): Record<string, unknown> | null {
 		const data = snapshot.data as Record<string, unknown>
 		const createData: Record<string, unknown> = {}
 

tests/unit/persistence/relationCycleCollection.test.ts

@@ -0,0 +1,144 @@
+import { beforeEach, describe, expect, test } from 'bun:test'
+import {
+	ActionDispatcher,
+	BatchPersister,
+	ContemberSchemaMutationAdapter,
+	MutationCollector,
+	SnapshotStore,
+	type BackendAdapter,
+	type SchemaNames,
+} from '@contember/bindx'
+
+/**
+ * `Page.publishedRevision` and `Revision.page` point back at each other, which is an
+ * ordinary shape for a schema that keeps a pointer to the currently live version.
+ */
+const schema: SchemaNames = {
+	entities: {
+		Page: {
+			name: 'Page',
+			scalars: ['id', 'title'],
+			fields: {
+				id: { type: 'column' },
+				title: { type: 'column' },
+				publishedRevision: { type: 'one', entity: 'Revision' },
+			},
+		},
+		Revision: {
+			name: 'Revision',
+			scalars: ['id', 'name'],
+			fields: {
+				id: { type: 'column' },
+				name: { type: 'column' },
+				page: { type: 'one', entity: 'Page' },
+			},
+		},
+	},
+	enums: {},
+}
+
+interface PersistCall {
+	readonly entityType: string
+	readonly entityId: string
+	readonly changes: Record<string, unknown>
+}
+
+function createAdapter(calls: PersistCall[]): BackendAdapter {
+	return {
+		query: () => Promise.resolve([]),
+		persist: (entityType, entityId, changes) => {
+			calls.push({ entityType, entityId, changes })
+			return Promise.resolve({ ok: true, data: { id: entityId } })
+		},
+		create: (_entityType, data) => Promise.resolve({ ok: true, data: { id: 'created-1', ...data } }),
+		delete: () => Promise.resolve({ ok: true }),
+	}
+}
+
+function connect(store: SnapshotStore, entityType: string, entityId: string, fieldName: string, targetId: string): void {
+	store.getOrCreateRelation(entityType, entityId, fieldName, {
+		currentId: targetId,
+		serverId: targetId,
+		state: 'connected',
+		serverState: 'connected',
+		placeholderData: {},
+	})
+}
+
+/**
+ * Collecting an update walks unchanged hasOne targets looking for nested changes. With a
+ * relation cycle in the schema that walk revisits an entity already on the stack, and
+ * without a guard it recurses until `RangeError: Maximum call stack size exceeded` — the
+ * persist then rejects, so nothing reaches the server and the store stays dirty.
+ */
+describe('relation cycles during collection', () => {
+	let store: SnapshotStore
+	let dispatcher: ActionDispatcher
+	let calls: PersistCall[]
+	let persister: BatchPersister
+
+	beforeEach(() => {
+		store = new SnapshotStore()
+		dispatcher = new ActionDispatcher(store)
+		calls = []
+		persister = new BatchPersister(createAdapter(calls), store, dispatcher, {
+			mutationCollector: new MutationCollector(store, new ContemberSchemaMutationAdapter(schema)),
+		})
+
+		store.setEntityData('Page', 'page-1', { id: 'page-1', title: 'Page' }, true)
+		store.setEntityData('Revision', 'rev-draft', { id: 'rev-draft', name: 'Draft' }, true)
+		store.setEntityData('Revision', 'rev-published', { id: 'rev-published', name: 'Published' }, true)
+
+		connect(store, 'Revision', 'rev-draft', 'page', 'page-1')
+		connect(store, 'Page', 'page-1', 'publishedRevision', 'rev-published')
+		connect(store, 'Revision', 'rev-published', 'page', 'page-1')
+	})
+
+	test('an update through a hasOne cycle persists instead of overflowing the stack', async () => {
+		store.setFieldValue('Revision', 'rev-draft', ['name'], 'Renamed draft')
+
+		const result = await persister.persistAll()
+
+		expect(result.success).toBe(true)
+		expect(calls).toHaveLength(1)
+		expect(calls[0]?.entityType).toBe('Revision')
+		expect(calls[0]?.entityId).toBe('rev-draft')
+		expect(calls[0]?.changes).toEqual({ name: 'Renamed draft' })
+	})
+
+	test('a real change on the far side of the cycle still travels with its parent', async () => {
+		store.setFieldValue('Revision', 'rev-draft', ['name'], 'Renamed draft')
+		store.setFieldValue('Page', 'page-1', ['title'], 'Renamed page')
+
+		const result = await persister.persistAll()
+
+		expect(result.success).toBe(true)
+		const revisionCall = calls.find(call => call.entityId === 'rev-draft')
+		const pageCall = calls.find(call => call.entityId === 'page-1')
+		expect(revisionCall?.changes).toEqual({ name: 'Renamed draft' })
+		expect(pageCall?.changes).toEqual({ title: 'Renamed page' })
+	})
+
+	test('a create whose nested target points back at it does not recurse forever', async () => {
+		const pageId = store.createEntity('Page', { title: 'New page' })
+		const revisionId = store.createEntity('Revision', { name: 'New revision' })
+		store.getOrCreateRelation('Page', pageId, 'publishedRevision', {
+			currentId: revisionId,
+			serverId: null,
+			state: 'connected',
+			serverState: 'disconnected',
+			placeholderData: {},
+		})
+		store.getOrCreateRelation('Revision', revisionId, 'page', {
+			currentId: pageId,
+			serverId: null,
+			state: 'connected',
+			serverState: 'disconnected',
+			placeholderData: {},
+		})
+
+		const result = await persister.persistAll()
+
+		expect(result.success).toBe(true)
+	})
+})