From 3556ddc73044b5541985486e8f625301feeb74ac Mon Sep 17 00:00:00 2001 From: ChengLei Shao Date: Tue, 22 Nov 2022 16:19:48 +0800 Subject: [PATCH] fix: reference check after remove collection (#1123) --- packages/core/database/src/collection.ts | 2 + packages/core/database/src/database.ts | 2 + .../database/src/features/ReferencesMap.ts | 14 +++- .../features/referential-integrity-check.ts | 1 + .../database/src/fields/belongs-to-field.ts | 6 +- .../reference-integerity-check.test.ts | 70 +++++++++++++++++++ 6 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 packages/plugins/collection-manager/src/__tests__/associations/reference-integerity-check.test.ts diff --git a/packages/core/database/src/collection.ts b/packages/core/database/src/collection.ts index 7fe099630..740923f96 100644 --- a/packages/core/database/src/collection.ts +++ b/packages/core/database/src/collection.ts @@ -260,7 +260,9 @@ export class Collection< if (!this.fields.has(name)) { return; } + const field = this.fields.get(name); + const bool = this.fields.delete(name); if (bool) { this.emit('field.afterRemove', field); diff --git a/packages/core/database/src/database.ts b/packages/core/database/src/database.ts index 113a49000..d2f7d4bdb 100644 --- a/packages/core/database/src/database.ts +++ b/packages/core/database/src/database.ts @@ -356,6 +356,8 @@ export class Database extends EventEmitter implements AsyncEmitter { const collection = this.collections.get(name); this.emit('beforeRemoveCollection', collection); + collection.resetFields(); + const result = this.collections.delete(name); this.sequelize.modelManager.removeModel(collection.model); diff --git a/packages/core/database/src/features/ReferencesMap.ts b/packages/core/database/src/features/ReferencesMap.ts index 46eb949cd..654201f21 100644 --- a/packages/core/database/src/features/ReferencesMap.ts +++ b/packages/core/database/src/features/ReferencesMap.ts @@ -1,3 +1,5 @@ +import lodash from 'lodash'; + export interface Reference { sourceCollectionName: string; sourceField: string; @@ -21,7 +23,7 @@ class ReferencesMap { // using existing reference return; } else if (existReference.onDelete === 'SET NULL') { - this.removeReference(existReference); + existReference.onDelete = reference.onDelete; } else { throw new Error( `On Delete Conflict, exist reference ${JSON.stringify(existReference)}, new reference ${JSON.stringify( @@ -31,7 +33,12 @@ class ReferencesMap { } } - this.map.set(reference.targetCollectionName, [...(this.map.get(reference.targetCollectionName) || []), reference]); + if (!existReference) { + this.map.set(reference.targetCollectionName, [ + ...(this.map.get(reference.targetCollectionName) || []), + reference, + ]); + } } getReferences(collectionName) { @@ -55,7 +62,8 @@ class ReferencesMap { if (!references) { return; } - const keys = Object.keys(reference); + + const keys = ['sourceCollectionName', 'sourceField', 'targetField', 'targetCollectionName']; this.map.set( reference.targetCollectionName, diff --git a/packages/core/database/src/features/referential-integrity-check.ts b/packages/core/database/src/features/referential-integrity-check.ts index bffc628af..3f6de630d 100644 --- a/packages/core/database/src/features/referential-integrity-check.ts +++ b/packages/core/database/src/features/referential-integrity-check.ts @@ -27,6 +27,7 @@ export async function referentialIntegrityCheck(options: ReferentialIntegrityChe const filter = { [sourceField]: referencedInstance[targetField], }; + const referencingExists = await sourceRepository.count({ filter, transaction, diff --git a/packages/core/database/src/fields/belongs-to-field.ts b/packages/core/database/src/fields/belongs-to-field.ts index 3c95eadc7..c28bd90af 100644 --- a/packages/core/database/src/fields/belongs-to-field.ts +++ b/packages/core/database/src/fields/belongs-to-field.ts @@ -83,15 +83,19 @@ export class BelongsToField extends RelationField { const tcoll = database.collections.get(this.target); const foreignKey = this.options.foreignKey; const field1 = collection.getField(foreignKey); + const field2 = tcoll.findField((field) => { return field.type === 'hasMany' && field.foreignKey === foreignKey; }); + if (!field1 && !field2) { collection.model.removeAttribute(foreignKey); } const association = collection.model.associations[this.name]; - this.database.referenceMap.removeReference(this.reference(association)); + const reference = this.reference(association); + + this.database.referenceMap.removeReference(reference); this.clearAccessors(); // 删掉 model 的关联字段 diff --git a/packages/plugins/collection-manager/src/__tests__/associations/reference-integerity-check.test.ts b/packages/plugins/collection-manager/src/__tests__/associations/reference-integerity-check.test.ts new file mode 100644 index 000000000..865c8db6f --- /dev/null +++ b/packages/plugins/collection-manager/src/__tests__/associations/reference-integerity-check.test.ts @@ -0,0 +1,70 @@ +import Database, { Collection as DBCollection } from '@nocobase/database'; +import { MockServer, mockServer } from '@nocobase/test'; + +describe('reference integrity check', () => { + let db: Database; + let app: MockServer; + beforeEach(async () => { + app = mockServer(); + db = app.db; + }); + + afterEach(async () => { + await app.destroy(); + }); + + it('should clean reference after collection destroy', async () => { + const users = db.collection({ + name: 'users', + fields: [ + { + type: 'string', + name: 'name', + }, + { + type: 'hasOne', + name: 'profile', + onDelete: 'CASCADE', + }, + ], + }); + + const profiles = db.collection({ + name: 'profiles', + fields: [ + { + type: 'integer', + name: 'age', + }, + { + type: 'belongsTo', + name: 'user', + onDelete: 'CASCADE', + }, + ], + }); + + await db.sync(); + + expect(db.referenceMap.getReferences('users').length).toEqual(1); + + await users.repository.create({ + values: { + name: 'foo', + profile: { + age: 18, + }, + }, + }); + + db.removeCollection('profiles'); + + expect(db.referenceMap.getReferences('users').length).toEqual(0); + + await users.repository.destroy({ + filter: { + name: 'foo', + }, + }); + }); +});