From 38b5974b902a5f44dd5cdcf6598b8e6c8ef2562c Mon Sep 17 00:00:00 2001
From: ChengLei Shao <chareice@live.com>
Date: Sun, 1 Jan 2023 23:14:48 +0800
Subject: [PATCH] fix: disable filterByTk options in destory method when
 collection has no primary key or has composite primary key (#1313)

* chore: tmp commit

* feat: filterByTk options with no primary key table

* chore: support filterTargetKey options

* chore: test
---
 .../src/__tests__/repository/destroy.test.ts  | 123 +++++++++++++++++-
 packages/core/database/src/repository.ts      |  26 ++++
 2 files changed, 148 insertions(+), 1 deletion(-)

diff --git a/packages/core/database/src/__tests__/repository/destroy.test.ts b/packages/core/database/src/__tests__/repository/destroy.test.ts
index e95d691b7..b67e4009d 100644
--- a/packages/core/database/src/__tests__/repository/destroy.test.ts
+++ b/packages/core/database/src/__tests__/repository/destroy.test.ts
@@ -13,6 +13,7 @@ describe('destroy with targetKey', function () {
 
   beforeEach(async () => {
     db = mockDatabase();
+
     User = db.collection({
       name: 'users',
       autoGenId: false,
@@ -76,7 +77,7 @@ describe('destroy with targetKey', function () {
 });
 
 describe('destroy', () => {
-  let db;
+  let db: Database;
   let User: Collection;
   let Post: Collection;
 
@@ -86,6 +87,10 @@ describe('destroy', () => {
 
   beforeEach(async () => {
     db = mockDatabase();
+    await db.clean({
+      drop: true,
+    });
+
     User = db.collection({
       name: 'users',
       fields: [
@@ -105,6 +110,122 @@ describe('destroy', () => {
     await db.sync();
   });
 
+  test('destroy records from no pk tables that filterTargetKey configured', async () => {
+    const Test = db.collection({
+      name: 'test',
+      timestamps: false,
+      autoGenId: false,
+      filterTargetKey: 'test',
+      fields: [
+        {
+          type: 'string',
+          name: 'test',
+        },
+      ],
+    });
+
+    await db.sync();
+
+    const t1 = await Test.repository.create({
+      values: {
+        test: 't1',
+      },
+    });
+
+    await Test.repository.create({
+      values: {
+        test: 't2',
+      },
+    });
+
+    await Test.repository.destroy({
+      filterByTk: 't2',
+    });
+
+    expect(await Test.repository.count()).toEqual(1);
+    expect((await Test.repository.findOne()).get('test')).toEqual('t1');
+  });
+
+  test('destroy records from tables without primary keys', async () => {
+    const Test = db.collection({
+      name: 'test',
+      timestamps: false,
+      autoGenId: false,
+      fields: [
+        {
+          type: 'string',
+          name: 'test',
+        },
+      ],
+    });
+
+    await db.sync();
+
+    const t1 = await Test.repository.create({
+      values: {
+        test: 't1',
+      },
+    });
+
+    await Test.repository.create({
+      values: {
+        test: 't2',
+      },
+    });
+
+    const destroy = async () => {
+      await Test.repository.destroy({
+        filterByTk: 111,
+      });
+    };
+
+    await expect(destroy()).rejects.toThrowError('filterByTk is not supported for collection that has no primary key');
+
+    await Test.repository.destroy({
+      filter: {
+        test: 't2',
+      },
+    });
+
+    expect(await Test.repository.count()).toEqual(1);
+    expect((await Test.repository.findOne()).get('test')).toEqual('t1');
+  });
+
+  test('destroy record has no primary key', async () => {
+    Post.addField('tags', {
+      type: 'belongsToMany',
+    });
+
+    const tags = db.collection({
+      name: 'tags',
+      fields: [
+        { type: 'belongsToMany', name: 'posts' },
+        { type: 'string', name: 'name' },
+      ],
+    });
+
+    await db.sync();
+
+    const post = await Post.repository.create({
+      values: {
+        title: 'p1',
+        tags: [{ name: 't1' }],
+      },
+    });
+
+    const throughCollection = db.getCollection(tags.getField('posts').options.through);
+
+    expect(await throughCollection.repository.count()).toEqual(1);
+
+    await throughCollection.repository.destroy({
+      filter: {
+        postId: post.get('id'),
+      },
+    });
+
+    expect(await throughCollection.repository.count()).toEqual(0);
+  });
+
   test('destroy with filter and filterByPk', async () => {
     const p1 = await Post.repository.create({
       values: {
diff --git a/packages/core/database/src/repository.ts b/packages/core/database/src/repository.ts
index c52a278ae..dc78545cc 100644
--- a/packages/core/database/src/repository.ts
+++ b/packages/core/database/src/repository.ts
@@ -532,6 +532,18 @@ export class Repository<TModelAttributes extends {} = any, TCreationAttributes e
         ? [options.filterByTk]
         : (options.filterByTk as TargetKey[] | undefined);
 
+    if (
+      this.collection.model.primaryKeyAttributes.length !== 1 &&
+      filterByTk &&
+      !lodash.get(this.collection.options, 'filterTargetKey')
+    ) {
+      if (this.collection.model.primaryKeyAttributes.length > 1) {
+        throw new Error(`filterByTk is not supported for composite primary key`);
+      } else {
+        throw new Error(`filterByTk is not supported for collection that has no primary key`);
+      }
+    }
+
     if (filterByTk && !options.filter) {
       return await this.model.destroy({
         ...options,
@@ -545,6 +557,20 @@ export class Repository<TModelAttributes extends {} = any, TCreationAttributes e
     }
 
     if (options.filter) {
+      if (
+        this.collection.model.primaryKeyAttributes.length !== 1 &&
+        !lodash.get(this.collection.options, 'filterTargetKey')
+      ) {
+        const queryOptions = {
+          ...this.buildQueryOptions(options),
+        };
+
+        return await this.model.destroy({
+          ...queryOptions,
+          transaction,
+        });
+      }
+
       let pks = (
         await this.find({
           filter: options.filter,