From 83cc19a01324ca48ebd3af789ab16b6d4dce18e4 Mon Sep 17 00:00:00 2001 From: ChengLei Shao Date: Thu, 10 Nov 2022 21:28:33 +0800 Subject: [PATCH] fix: reference options sync (#1061) * fix: sync onDelete options between field and reverseField * feat: reference compatibility --- .../database/src/features/ReferencesMap.ts | 19 ++++---- packages/core/server/src/application.ts | 3 ++ .../src/__tests__/fields/reverseField.test.ts | 43 +++++++++++++++++++ .../hooks/afterCreateForForeignKeyField.ts | 6 +-- .../src/hooks/afterCreateForReverseField.ts | 1 + .../src/hooks/beforeCreateForReverseField.ts | 10 +++++ .../collection-manager/src/models/field.ts | 14 ++++++ .../plugins/collection-manager/src/server.ts | 9 +++- 8 files changed, 93 insertions(+), 12 deletions(-) diff --git a/packages/core/database/src/features/ReferencesMap.ts b/packages/core/database/src/features/ReferencesMap.ts index 44ff484e1..46eb949cd 100644 --- a/packages/core/database/src/features/ReferencesMap.ts +++ b/packages/core/database/src/features/ReferencesMap.ts @@ -10,22 +10,25 @@ class ReferencesMap { protected map: Map = new Map(); addReference(reference: Reference) { + if (!reference.onDelete) { + reference.onDelete = 'SET NULL'; + } + const existReference = this.existReference(reference); - if (existReference) { - if (reference.onDelete && existReference.onDelete !== reference.onDelete) { + if (existReference && existReference.onDelete !== reference.onDelete) { + if (reference.onDelete === 'SET NULL') { + // using existing reference + return; + } else if (existReference.onDelete === 'SET NULL') { + this.removeReference(existReference); + } else { throw new Error( `On Delete Conflict, exist reference ${JSON.stringify(existReference)}, new reference ${JSON.stringify( reference, )}`, ); } - - return; - } - - if (!reference.onDelete) { - reference.onDelete = 'SET NULL'; } this.map.set(reference.targetCollectionName, [...(this.map.get(reference.targetCollectionName) || []), reference]); diff --git a/packages/core/server/src/application.ts b/packages/core/server/src/application.ts index a29de2e66..4838996bb 100644 --- a/packages/core/server/src/application.ts +++ b/packages/core/server/src/application.ts @@ -106,6 +106,9 @@ export class ApplicationVersion { async get() { if (await this.app.db.collectionExistsInDb('applicationVersion')) { const model = await this.collection.model.findOne(); + if (!model) { + return null; + } return model.get('value') as any; } return null; diff --git a/packages/plugins/collection-manager/src/__tests__/fields/reverseField.test.ts b/packages/plugins/collection-manager/src/__tests__/fields/reverseField.test.ts index de185f84b..c1cd4571f 100644 --- a/packages/plugins/collection-manager/src/__tests__/fields/reverseField.test.ts +++ b/packages/plugins/collection-manager/src/__tests__/fields/reverseField.test.ts @@ -56,6 +56,49 @@ describe('reverseField options', () => { expect(json.foreignKey).toBe(json.reverseField.foreignKey); }); + it('should sync onDelete options for reverse field', async () => { + const field = await Field.repository.create({ + values: { + type: 'hasMany', + collectionName: 'tests', + target: 'targets', + onDelete: 'CASCADE', + reverseField: {}, + }, + }); + + const { reverseField } = field.toJSON(); + + expect(reverseField.onDelete).toBe('CASCADE'); + }); + + it('should update reverseField onDelete options', async () => { + const field = await Field.repository.create({ + values: { + type: 'hasMany', + collectionName: 'tests', + target: 'targets', + onDelete: 'CASCADE', + reverseField: {}, + }, + }); + + const { reverseField } = field.toJSON(); + + await Field.repository.update({ + filterByTk: reverseField.key, + values: { + onDelete: 'SET NULL', + }, + }); + + const mainField = await Field.repository.findOne({ + filterByTk: field.get('key'), + }); + + expect(mainField.get('onDelete')).toBe('SET NULL'); + }); + it('should update reverseField', async () => { const field = await Field.repository.create({ values: { diff --git a/packages/plugins/collection-manager/src/hooks/afterCreateForForeignKeyField.ts b/packages/plugins/collection-manager/src/hooks/afterCreateForForeignKeyField.ts index afe353a13..26ce7fd7e 100644 --- a/packages/plugins/collection-manager/src/hooks/afterCreateForForeignKeyField.ts +++ b/packages/plugins/collection-manager/src/hooks/afterCreateForForeignKeyField.ts @@ -81,9 +81,9 @@ export function afterCreateForForeignKeyField(db: Database) { await r.update({ filter: { collectionName, - options:{ - primaryKey: true - } + options: { + primaryKey: true, + }, }, values: { sort: 0, diff --git a/packages/plugins/collection-manager/src/hooks/afterCreateForReverseField.ts b/packages/plugins/collection-manager/src/hooks/afterCreateForReverseField.ts index 542e30302..61005a8fb 100644 --- a/packages/plugins/collection-manager/src/hooks/afterCreateForReverseField.ts +++ b/packages/plugins/collection-manager/src/hooks/afterCreateForReverseField.ts @@ -4,6 +4,7 @@ export function afterCreateForReverseField(db: Database) { return async (model, { transaction }) => { const Field = db.getCollection('fields'); const reverseKey = model.get('reverseKey'); + if (!reverseKey) { return; } diff --git a/packages/plugins/collection-manager/src/hooks/beforeCreateForReverseField.ts b/packages/plugins/collection-manager/src/hooks/beforeCreateForReverseField.ts index d5d3b6d25..cef3bb32f 100644 --- a/packages/plugins/collection-manager/src/hooks/beforeCreateForReverseField.ts +++ b/packages/plugins/collection-manager/src/hooks/beforeCreateForReverseField.ts @@ -4,19 +4,28 @@ export function beforeCreateForReverseField(db: Database) { return async (model, { transaction }) => { const Field = db.getCollection('fields'); const reverseKey = model.get('reverseKey'); + if (!reverseKey) { return; } + const reverse = await Field.model.findByPk(reverseKey, { transaction }); model.set('collectionName', reverse.get('target')); model.set('target', reverse.get('collectionName')); + + if (reverse.get('onDelete')) { + model.set('onDelete', reverse.get('onDelete')); + } + const reverseType = reverse.get('type') as any; + if (['hasMany', 'hasOne'].includes(reverseType)) { model.set('type', 'belongsTo'); model.set('targetKey', reverse.get('sourceKey')); model.set('foreignKey', reverse.get('foreignKey')); model.set('sourceKey', reverse.get('targetKey')); } + if (['belongsTo'].includes(reverseType)) { if (!model.get('type')) { model.set('type', 'hasMany'); @@ -25,6 +34,7 @@ export function beforeCreateForReverseField(db: Database) { model.set('foreignKey', reverse.get('foreignKey')); model.set('targetKey', reverse.get('sourceKey')); } + if (['belongsToMany'].includes(reverseType)) { model.set('type', 'belongsToMany'); model.set('through', reverse.get('through')); diff --git a/packages/plugins/collection-manager/src/models/field.ts b/packages/plugins/collection-manager/src/models/field.ts index 19abc4c05..6cd16f860 100644 --- a/packages/plugins/collection-manager/src/models/field.ts +++ b/packages/plugins/collection-manager/src/models/field.ts @@ -156,6 +156,20 @@ export class FieldModel extends MagicAttributeModel { ); } + async syncReferenceCheckOption(options: Transactionable) { + const reverseKey = this.get('reverseKey'); + if (!reverseKey) return; + + const reverseField = await this.db.getCollection('fields').repository.findOne({ + filterByTk: reverseKey, + transaction: options.transaction, + }); + + if (!reverseField) return; + reverseField.set('onDelete', this.get('onDelete')); + await reverseField.save({ hooks: false, transaction: options.transaction }); + } + protected getFieldCollection(): Collection | null { const collectionName = this.get('collectionName'); diff --git a/packages/plugins/collection-manager/src/server.ts b/packages/plugins/collection-manager/src/server.ts index 663fbfad6..efe74d1c6 100644 --- a/packages/plugins/collection-manager/src/server.ts +++ b/packages/plugins/collection-manager/src/server.ts @@ -13,7 +13,7 @@ import { beforeCreateForChildrenCollection, beforeCreateForReverseField, beforeDestroyForeignKey, - beforeInitOptions + beforeInitOptions, } from './hooks'; import { CollectionModel, FieldModel } from './models'; @@ -106,6 +106,13 @@ export class CollectionManagerPlugin extends Plugin { if (prevDefaultValue != currentDefaultValue) { await model.syncDefaultValue({ transaction, defaultValue: currentDefaultValue }); } + + const prevOnDelete = prevOptions['onDelete']; + const currentOnDelete = currentOptions['onDelete']; + + if (prevOnDelete != currentOnDelete) { + await model.syncReferenceCheckOption({ transaction }); + } }); this.app.db.on('fields.afterSaveWithAssociations', async (model: FieldModel, { context, transaction }) => {