From e8d506ae06a73efe3004ba4ef27ed613e8930ed0 Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Sun, 19 Apr 2026 15:28:00 +0200 Subject: [PATCH] fix: update one-to-many relation with array reassignment --- src/relation.ts | 172 ++++++++++++++++++++-------- tests/relations/one-to-many.test.ts | 30 +++++ 2 files changed, 155 insertions(+), 47 deletions(-) diff --git a/src/relation.ts b/src/relation.ts index 6666092..0470f7e 100644 --- a/src/relation.ts +++ b/src/relation.ts @@ -226,6 +226,7 @@ export abstract class Relation { const update = event.data if ( + update.path.length > path.length && path.every((key, index) => key === update.path[index]) && !isRecord(update.nextValue) ) { @@ -272,57 +273,114 @@ export abstract class Relation { (event) => { const update = event.data - if (isEqual(update.path, path) && isRecord(update.nextValue)) { - /** - * @note Listeners are attached per-record but fire for every owner update. - * Skip events whose target record's relation isn't this instance. - */ - if (update.prevRecord[kRelationMap].get(serializedPath) !== this) { - return - } + if (!isEqual(update.path, path)) { + return + } + + /** + * @note Listeners are attached per-record but fire for every owner update. + * Skip events whose target record's relation isn't this instance. + */ + if (update.prevRecord[kRelationMap].get(serializedPath) !== this) { + return + } + if (this instanceof One && isRecord(update.nextValue)) { event.preventDefault() // If the owner relation is "one-of", multiple foreign records cannot own this record. // Disassociate the old foreign records from pointing to the owner record. - if (this instanceof One) { - const oldForeignRecords = - this.foreignCollections.flatMap( - (foreignCollection) => { - return foreignCollection.findMany((q) => { - return q.where((record) => { - return this.foreignKeys.has(record[kPrimaryKey]) - }) + const oldForeignRecords = + this.foreignCollections.flatMap( + (foreignCollection) => { + return foreignCollection.findMany((q) => { + return q.where((record) => { + return this.foreignKeys.has(record[kPrimaryKey]) }) - }, - ) + }) + }, + ) - const foreignRelationsToDisassociate = oldForeignRecords.flatMap( - (record) => this.getRelationsToOwner(record), + const foreignRelationsToDisassociate = oldForeignRecords.flatMap( + (record) => this.getRelationsToOwner(record), + ) + + // Throw if attempting to disassociate unique relations. + if (this.options.unique) { + invariant.as( + RelationError.for( + RelationErrorCodes.FORBIDDEN_UNIQUE_UPDATE, + this.#createErrorDetails(), + ), + foreignRelationsToDisassociate.length === 0, + 'Failed to update a unique relation at "%s": the foreign record is already associated with another owner', + update.path.join('.'), ) + } - // Throw if attempting to disassociate unique relations. - if (this.options.unique) { - invariant.as( - RelationError.for( - RelationErrorCodes.FORBIDDEN_UNIQUE_UPDATE, - this.#createErrorDetails(), - ), - foreignRelationsToDisassociate.length === 0, - 'Failed to update a unique relation at "%s": the foreign record is already associated with another owner', - update.path.join('.'), - ) - } + for (const foreignRelation of foreignRelationsToDisassociate) { + foreignRelation.foreignKeys.delete(update.prevRecord[kPrimaryKey]) + } - for (const foreignRelation of foreignRelationsToDisassociate) { - foreignRelation.foreignKeys.delete(update.prevRecord[kPrimaryKey]) - } + // Check any other owners associated with the same foreign record. + // This is important since unique relations are not always two-way. + if (this.options.unique) { + const otherOwnersAssociatedWithForeignRecord = + this.#getOtherOwnerForRecords([update.nextValue]) + + invariant.as( + RelationError.for( + RelationErrorCodes.FORBIDDEN_UNIQUE_UPDATE, + this.#createErrorDetails(), + ), + otherOwnersAssociatedWithForeignRecord == null, + 'Failed to update a unique relation at "%s": the foreign record is already associated with another owner', + update.path.join('.'), + ) + } + + this.foreignKeys.clear() - // Check any other owners associated with the same foreign record. - // This is important since unique relations are not always two-way. - if (this.options.unique) { + // Associate the owner with a foreign record from the update data. + const foreignRecord = update.nextValue + this.foreignKeys.add(foreignRecord[kPrimaryKey]) + + for (const foreignRelation of this.getRelationsToOwner( + foreignRecord, + )) { + foreignRelation.foreignKeys.add(update.prevRecord[kPrimaryKey]) + } + } else if (this instanceof Many && Array.isArray(update.nextValue)) { + event.preventDefault() + + const nextForeignRecords: Array = [] + const nextForeignKeys = new Set() + + for (const foreignRecord of update.nextValue) { + invariant.as( + RelationError.for( + RelationErrorCodes.INVALID_FOREIGN_RECORD, + this.#createErrorDetails(), + ), + isRecord(foreignRecord) && foreignRecord[kPrimaryKey] != null, + 'Failed to update a relation at "%s": expected relational value to be a record with a primary key, got "%j"', + update.path.join('.'), + foreignRecord, + ) + + nextForeignRecords.push(foreignRecord) + nextForeignKeys.add(foreignRecord[kPrimaryKey]) + } + + // Unique check for newly-added foreign records. + if (this.options.unique) { + const addedForeignRecords = nextForeignRecords.filter( + (record) => !this.foreignKeys.has(record[kPrimaryKey]), + ) + + if (addedForeignRecords.length > 0) { const otherOwnersAssociatedWithForeignRecord = - this.#getOtherOwnerForRecords([update.nextValue]) + this.#getOtherOwnerForRecords(addedForeignRecords) invariant.as( RelationError.for( @@ -334,18 +392,38 @@ export abstract class Relation { update.path.join('.'), ) } + } - this.foreignKeys.clear() + // Disassociate inverse links for foreign records no longer referenced. + const removedForeignRecords = this.foreignCollections + .flatMap((foreignCollection) => { + return foreignCollection.findMany((q) => { + return q.where((record) => { + return this.foreignKeys.has(record[kPrimaryKey]) + }) + }) + }) + .filter((record) => !nextForeignKeys.has(record[kPrimaryKey])) + + for (const removedForeignRecord of removedForeignRecords) { + for (const foreignRelation of this.getRelationsToOwner( + removedForeignRecord, + )) { + foreignRelation.foreignKeys.delete(update.prevRecord[kPrimaryKey]) + } } - // Associate the owner with a foreign record from the update data. - const foreignRecord = update.nextValue - this.foreignKeys.add(foreignRecord[kPrimaryKey]) + this.foreignKeys.clear() + for (const foreignKey of nextForeignKeys) { + this.foreignKeys.add(foreignKey) + } - for (const foreignRelation of this.getRelationsToOwner( - foreignRecord, - )) { - foreignRelation.foreignKeys.add(update.prevRecord[kPrimaryKey]) + for (const foreignRecord of nextForeignRecords) { + for (const foreignRelation of this.getRelationsToOwner( + foreignRecord, + )) { + foreignRelation.foreignKeys.add(update.prevRecord[kPrimaryKey]) + } } } }, diff --git a/tests/relations/one-to-many.test.ts b/tests/relations/one-to-many.test.ts index 8525bf7..aea1c2b 100644 --- a/tests/relations/one-to-many.test.ts +++ b/tests/relations/one-to-many.test.ts @@ -200,6 +200,36 @@ it('updates a one-to-many relation when the referenced record is updated', async }) }) +it('updates a one-to-many relation when the relational property is reassigned to a new array', async () => { + const users = new Collection({ schema: userSchema }) + const posts = new Collection({ schema: postSchema }) + + users.defineRelations(({ many }) => ({ + posts: many(posts), + })) + + const firstPost = await posts.create({ title: 'First' }) + const user = await users.create({ id: 1, posts: [firstPost] }) + + const secondPost = await posts.create({ title: 'Second' }) + + const updatedUser = await users.update(user, { + data(draft) { + draft.posts = [...draft.posts, secondPost] + }, + }) + + expect(updatedUser).toEqual({ + id: 1, + posts: [{ title: 'First' }, { title: 'Second' }], + }) + + expect(users.findFirst((q) => q.where({ id: 1 }))).toEqual({ + id: 1, + posts: [{ title: 'First' }, { title: 'Second' }], + }) +}) + it('updates a one-to-many relation when the referenced record is dissociated', async () => { const users = new Collection({ schema: userSchema }) const posts = new Collection({ schema: postSchema })