fix: validate other owners in unique one-way relations

Author: kettanaito

src/relation.ts

@@ -245,27 +245,42 @@ export abstract class Relation {
             },
           )
 
+          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(),
               ),
-              oldForeignRecords.length === 0,
+              foreignRelationsToDisassociate.length === 0,
               'Failed to update a unique relation at "%s": the foreign record is already associated with another owner',
               update.path.join('.'),
             )
           }
 
-          const foreignRelationsToDisassociate = oldForeignRecords.flatMap(
-            (record) => this.getRelationsToOwner(record),
-          )
-
           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.
+          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()
         }
 
@@ -316,24 +331,19 @@ export abstract class Relation {
         initialValue,
       )
 
-      const initialForeignEntries: Array<RecordType> = Array.prototype
+      const initialForeignRecords: Array<RecordType> = Array.prototype
         .concat([], get(initialValues, path))
         /**
          * @note If the initial value as an empty array, concatenating it above
          * results in [undefined]. Filter out undefined values.
          */
         .filter(Boolean)
 
-      logger.log('all foreign entries:', initialForeignEntries)
+      logger.log('all foreign entries:', initialForeignRecords)
 
-      /**
-       * @todo Check if:
-       * 1. Relation is unique;
-       * 2. Foreign entries already reference something!
-       * Then, throw.
-       */
       if (this.options.unique) {
-        const foreignRelations = initialForeignEntries.flatMap(
+        // Check if the foreign record isn't associated with another owner.
+        const foreignRelations = initialForeignRecords.flatMap(
           (foreignRecord) => {
             return this.getRelationsToOwner(foreignRecord)
           },
@@ -343,20 +353,32 @@ export abstract class Relation {
           (relation) => relation.foreignKeys.size === 0,
         )
 
-        const recordLabel = this instanceof Many ? 'records' : 'record'
-
         invariant.as(
           RelationError.for(
             RelationErrorCodes.FORBIDDEN_UNIQUE_CREATE,
             this.#createErrorDetails(),
           ),
           isUnique,
-          `Failed to create a unique relation at "%s": foreign ${recordLabel} already associated with another owner`,
-          this.path.join('.'),
+          `Failed to create a unique relation at "%s": foreign ${this instanceof Many ? 'records' : 'record'} already associated with another owner`,
+          serializedPath,
+        )
+
+        // Check if another owner isn't associated with the foreign record.
+        const otherOwnersAssociatedWithForeignRecord =
+          this.#getOtherOwnerForRecords(initialForeignRecords)
+
+        invariant.as(
+          RelationError.for(
+            RelationErrorCodes.FORBIDDEN_UNIQUE_CREATE,
+            this.#createErrorDetails(),
+          ),
+          otherOwnersAssociatedWithForeignRecord == null,
+          'Failed to create a unique relation at "%s": the foreign record is already associated with another owner',
+          serializedPath,
         )
       }
 
-      for (const foreignRecord of initialForeignEntries) {
+      for (const foreignRecord of initialForeignRecords) {
         const foreignKey = foreignRecord[kPrimaryKey]
 
         invariant.as(
@@ -448,6 +470,25 @@ export abstract class Relation {
     return result
   }
 
+  #getOtherOwnerForRecords(
+    foreignRecords: Array<RecordType>,
+  ): RecordType | undefined {
+    const serializedPath = this.path.join('.')
+
+    return this.ownerCollection.findFirst((q) => {
+      return q.where((otherOwner) => {
+        const otherOwnerRelations = otherOwner[kRelationMap]
+        const otherOwnerRelation = otherOwnerRelations.get(serializedPath)
+
+        // Forego any other relation comparisons since the same collection
+        // shares the relation definition at the same property path.
+        return foreignRecords.some((foreignRecord) => {
+          return otherOwnerRelation.foreignKeys.has(foreignRecord[kPrimaryKey])
+        })
+      })
+    })
+  }
+
   #createErrorDetails(): RelationErrorDetails {
     return {
       path: this.path,

tests/relations/one-to-one.test.ts

@@ -241,7 +241,7 @@ it('applies relation to records created before the relation is defined', async (
   })
 })
 
-it('supports unique one-to-one relations', async () => {
+it('supports creating unique one-to-one relations', async () => {
   const userSchema = z.object({ id: z.number() })
   const postSchema = z.object({
     title: z.string(),
@@ -274,7 +274,45 @@ it('supports unique one-to-one relations', async () => {
     .soft(posts.findFirst((q) => q.where({ author: { id: updatedUser!.id } })))
     .toEqual(post)
 
-  const anotherUser = await users.create({ id: 2 })
+  const anotherUser = await users.create({ id: 5 })
+  await expect(
+    posts.update(post, {
+      data(post) {
+        // This must not error since the provided foreign record (user)
+        // is not associated with any owners (posts).
+        post.author = anotherUser
+      },
+    }),
+    'Updates the unique relational property',
+  ).resolves.toEqual({
+    title: 'First',
+    author: anotherUser,
+  })
+})
+
+it('supports updating unique one-to-one relations', async () => {
+  const userSchema = z.object({ id: z.number() })
+  const postSchema = z.object({
+    title: z.string(),
+    author: userSchema,
+  })
+
+  const users = new Collection({ schema: userSchema })
+  const posts = new Collection({ schema: postSchema })
+
+  posts.defineRelations(({ one }) => ({
+    author: one(users, { unique: true }),
+  }))
+
+  const user = await users.create({ id: 1 })
+  const post = await posts.create({ title: 'First', author: user })
+
+  expect.soft(post.author).toEqual(user)
+  expect
+    .soft(posts.findFirst((q) => q.where({ author: { id: user.id } })))
+    .toEqual(post)
+
+  const anotherUser = await users.create({ id: 5 })
   await expect(
     posts.update(post, {
       data(post) {
@@ -307,6 +345,7 @@ it('errors when creating a unique relation with a foreign record that has alread
   const user = await users.create({ id: 1 })
   await posts.create({ title: 'First', author: user })
 
+  // Cannot create another post referencing the same `user` as the author.
   await expect(posts.create({ title: 'Second', author: user })).rejects.toThrow(
     new RelationError(
       `Failed to create a unique relation at "author": the foreign record is already associated with another owner`,