From dbdd587b718f4192d58c631edb83be0818c60b63 Mon Sep 17 00:00:00 2001 From: ChengLei Shao Date: Wed, 27 Sep 2023 22:03:04 +0800 Subject: [PATCH] chore(database): clean invalid associations in collection model when set field failed (#2720) * chore(database): clean invalid associations in collection model when set field failed * fix: import * chore: test --- .../fields/belongs-to-many-field.test.ts | 52 ++++++++++++++ packages/core/database/src/collection.ts | 35 +++++++++ .../src/fields/belongs-to-many-field.ts | 6 +- .../__tests__/fields/belongs-to-many.test.ts | 72 +++++++++++++++++++ 4 files changed, 163 insertions(+), 2 deletions(-) diff --git a/packages/core/database/src/__tests__/fields/belongs-to-many-field.test.ts b/packages/core/database/src/__tests__/fields/belongs-to-many-field.test.ts index 434e2eff9..f06dd6141 100644 --- a/packages/core/database/src/__tests__/fields/belongs-to-many-field.test.ts +++ b/packages/core/database/src/__tests__/fields/belongs-to-many-field.test.ts @@ -13,6 +13,58 @@ describe('belongs to many field', () => { await db.close(); }); + it('should define belongs to many when change alias name', async () => { + const Through = db.collection({ + name: 't1', + fields: [ + { + type: 'bigInt', + name: 'id', + primaryKey: true, + }, + ], + }); + + const A = db.collection({ + name: 'a', + }); + + const B = db.collection({ + name: 'b', + }); + + await db.sync(); + + let error; + + expect(Object.keys(A.model.associations).length).toEqual(0); + try { + A.setField('t1', { + type: 'belongsToMany', + through: 't1', + target: 't1', + }); + } catch (e) { + error = e; + } + + expect(error).toBeDefined(); + + expect(Object.keys(A.model.associations).length).toEqual(0); + let error2; + try { + A.setField('xxx', { + type: 'belongsToMany', + through: 't1', + target: 't1', + }); + } catch (e) { + error2 = e; + } + + expect(error2).not.toBeDefined(); + }); + it('should define belongs to many relation through exists pivot collection', async () => { const PostTag = db.collection({ name: 'postsTags', diff --git a/packages/core/database/src/collection.ts b/packages/core/database/src/collection.ts index 5afcb6256..5c4a2a0a6 100644 --- a/packages/core/database/src/collection.ts +++ b/packages/core/database/src/collection.ts @@ -22,6 +22,40 @@ export type CollectionSortable = string | boolean | { name?: string; scopeKey?: type dumpable = 'required' | 'optional' | 'skip'; +function EnsureAtomicity(target: any, propertyKey: string, descriptor: PropertyDescriptor) { + const originalMethod = descriptor.value; + + descriptor.value = function (...args: any[]) { + const model = this.model; + const beforeAssociationKeys = Object.keys(model.associations); + const beforeRawAttributes = Object.keys(model.rawAttributes); + + try { + return originalMethod.apply(this, args); + } catch (error) { + // remove associations created in this method + const afterAssociationKeys = Object.keys(model.associations); + const createdAssociationKeys = lodash.difference(afterAssociationKeys, beforeAssociationKeys); + for (const key of createdAssociationKeys) { + delete this.model.associations[key]; + } + + const afterRawAttributes = Object.keys(model.rawAttributes); + const createdRawAttributes = lodash.difference(afterRawAttributes, beforeRawAttributes); + console.log({ + beforeRawAttributes, + afterRawAttributes, + }); + for (const key of createdRawAttributes) { + delete this.model.rawAttributes[key]; + } + throw error; + } + }; + + return descriptor; +} + export interface CollectionOptions extends Omit { name: string; namespace?: string; @@ -251,6 +285,7 @@ export class Collection< } } + @EnsureAtomicity setField(name: string, options: FieldOptions): Field { checkIdentifier(name); this.checkFieldType(name, options); diff --git a/packages/core/database/src/fields/belongs-to-many-field.ts b/packages/core/database/src/fields/belongs-to-many-field.ts index adae1630d..bd38d5ef4 100644 --- a/packages/core/database/src/fields/belongs-to-many-field.ts +++ b/packages/core/database/src/fields/belongs-to-many-field.ts @@ -85,12 +85,14 @@ export class BelongsToManyField extends RelationField { this.options.onDelete = 'CASCADE'; } - const association = collection.model.belongsToMany(Target, { + const belongsToManyOptions = { constraints: false, ...omit(this.options, ['name', 'type', 'target']), as: this.name, through: Through.model, - }); + }; + + const association = collection.model.belongsToMany(Target, belongsToManyOptions); // 建立关系之后从 pending 列表中删除 database.removePendingField(this); diff --git a/packages/plugins/@nocobase/plugin-collection-manager/src/server/__tests__/fields/belongs-to-many.test.ts b/packages/plugins/@nocobase/plugin-collection-manager/src/server/__tests__/fields/belongs-to-many.test.ts index 5102165e6..ead47cb27 100644 --- a/packages/plugins/@nocobase/plugin-collection-manager/src/server/__tests__/fields/belongs-to-many.test.ts +++ b/packages/plugins/@nocobase/plugin-collection-manager/src/server/__tests__/fields/belongs-to-many.test.ts @@ -34,6 +34,78 @@ describe('belongsToMany', () => { await app.destroy(); }); + it('should define belongs to many when change alias name', async () => { + await Collection.repository.create({ + values: { + name: 'a', + fields: [{ type: 'bigInt', name: 'id', primaryKey: true, autoIncrement: true }], + }, + context: {}, + }); + + await Collection.repository.create({ + values: { + name: 'b', + fields: [{ type: 'bigInt', name: 'id', primaryKey: true, autoIncrement: true }], + }, + context: {}, + }); + + await Collection.repository.create({ + values: { + name: 't1', + fields: [{ type: 'bigInt', name: 'id', primaryKey: true, autoIncrement: true }], + }, + context: {}, + }); + + const Through = db.getCollection('t1'); + + let error; + try { + await Field.repository.create({ + values: { + name: 't1', + type: 'belongsToMany', + target: 'b', + through: 't1', + collectionName: 'a', + sourceKey: 'id', + targetKey: 'id', + foreignKey: 'a_id', + otherKey: 'b_id', + }, + context: {}, + }); + } catch (e) { + error = e; + } + + expect(error).toBeDefined(); + + let error2; + try { + await Field.repository.create({ + values: { + name: 'xxx', + type: 'belongsToMany', + target: 'b', + through: 't1', + collectionName: 'a', + sourceKey: 'id', + targetKey: 'id', + foreignKey: 'a_id', + otherKey: 'b_id', + }, + context: {}, + }); + } catch (e) { + error2 = e; + } + + expect(error2).not.toBeDefined(); + }); + it('should create belongsToMany field', async () => { await Field.repository.create({ values: {