From a1dc139cf488c8632fd7e06df41f9dca6255fc80 Mon Sep 17 00:00:00 2001 From: Junyi Date: Sat, 20 Aug 2022 23:23:13 +0800 Subject: [PATCH] feat(plugin-cm): add unique option for base fields (#745) * feat(plugin-cm): add unique option for base fields * refactor(plugin-cm): make sure unique constraint sync with field option * fix(plugin-cm): fix sqlite unique field sync * fix(plugin-cm): fix unique constraint sync logic * refactor(plugin-cm): remove unique property for select components * fix: previous * fix: test error Co-authored-by: chenos --- .../collection-manager/interfaces/checkbox.ts | 2 +- .../collection-manager/interfaces/email.ts | 3 +- .../collection-manager/interfaces/input.ts | 3 +- .../collection-manager/interfaces/integer.ts | 3 +- .../collection-manager/interfaces/number.ts | 5 +- .../collection-manager/interfaces/password.ts | 3 +- .../collection-manager/interfaces/percent.ts | 5 +- .../collection-manager/interfaces/phone.ts | 3 +- .../interfaces/properties/index.ts | 7 + packages/core/client/src/locale/zh_CN.ts | 1 + .../__tests__/magic-attribute-model.test.ts | 24 +++ packages/core/database/src/collection.ts | 16 +- packages/core/database/src/fields/field.ts | 2 +- .../database/src/magic-attribute-model.ts | 194 +++++++++++++++++- .../database/src/transaction-decorator.ts | 2 +- packages/plugins/acl/src/__tests__/prepare.ts | 2 + .../plugins/collection-manager/package.json | 6 + .../src/__tests__/action.test.ts | 9 +- .../__tests__/field-options/indexes.test.ts | 96 +++++++++ .../__tests__/http-api/collections.test.ts | 104 ++++------ .../collection-manager/src/__tests__/index.ts | 6 +- .../src/__tests__/through.test.ts | 9 +- .../collection-manager/src/models/field.ts | 56 ++++- .../plugins/collection-manager/src/server.ts | 29 ++- .../error-handler/src/error-handler.ts | 4 +- .../src/__tests__/server-hook-impl.test.ts | 2 + .../src/__tests__/server-hook.test.ts | 2 + 27 files changed, 477 insertions(+), 121 deletions(-) create mode 100644 packages/plugins/collection-manager/src/__tests__/field-options/indexes.test.ts diff --git a/packages/core/client/src/collection-manager/interfaces/checkbox.ts b/packages/core/client/src/collection-manager/interfaces/checkbox.ts index 3792a1794..f254fd813 100644 --- a/packages/core/client/src/collection-manager/interfaces/checkbox.ts +++ b/packages/core/client/src/collection-manager/interfaces/checkbox.ts @@ -17,7 +17,7 @@ export const checkbox: IField = { }, hasDefaultValue: true, properties: { - ...defaultProps, + ...defaultProps }, filterable: { operators: operators.boolean, diff --git a/packages/core/client/src/collection-manager/interfaces/email.ts b/packages/core/client/src/collection-manager/interfaces/email.ts index 4ae298ab4..05a21d33b 100644 --- a/packages/core/client/src/collection-manager/interfaces/email.ts +++ b/packages/core/client/src/collection-manager/interfaces/email.ts @@ -1,4 +1,4 @@ -import { defaultProps, operators } from './properties'; +import { defaultProps, operators, unique } from './properties'; import { IField } from './types'; export const email: IField = { @@ -21,6 +21,7 @@ export const email: IField = { hasDefaultValue: true, properties: { ...defaultProps, + unique, }, filterable: { operators: operators.string, diff --git a/packages/core/client/src/collection-manager/interfaces/input.ts b/packages/core/client/src/collection-manager/interfaces/input.ts index 42f80008b..d1d63307e 100644 --- a/packages/core/client/src/collection-manager/interfaces/input.ts +++ b/packages/core/client/src/collection-manager/interfaces/input.ts @@ -1,5 +1,5 @@ import { ISchema } from '@formily/react'; -import { defaultProps, operators } from './properties'; +import { defaultProps, operators, unique } from './properties'; import { IField } from './types'; import { i18n } from '../../i18n'; @@ -21,6 +21,7 @@ export const input: IField = { hasDefaultValue: true, properties: { ...defaultProps, + unique, }, filterable: { operators: operators.string, diff --git a/packages/core/client/src/collection-manager/interfaces/integer.ts b/packages/core/client/src/collection-manager/interfaces/integer.ts index 98c12beeb..b8b84c816 100644 --- a/packages/core/client/src/collection-manager/interfaces/integer.ts +++ b/packages/core/client/src/collection-manager/interfaces/integer.ts @@ -1,4 +1,4 @@ -import { defaultProps, operators } from './properties'; +import { defaultProps, operators, unique } from './properties'; import { IField } from './types'; import { i18n } from '../../i18n'; import { registerValidateFormats } from '@formily/core'; @@ -33,6 +33,7 @@ export const integer: IField = { hasDefaultValue: true, properties: { ...defaultProps, + unique, }, filterable: { operators: operators.number, diff --git a/packages/core/client/src/collection-manager/interfaces/number.ts b/packages/core/client/src/collection-manager/interfaces/number.ts index 99ea17623..fa9329514 100644 --- a/packages/core/client/src/collection-manager/interfaces/number.ts +++ b/packages/core/client/src/collection-manager/interfaces/number.ts @@ -1,6 +1,4 @@ -import { registerValidateRules } from '@formily/core'; -import { ISchema } from '@formily/react'; -import { defaultProps, operators } from './properties'; +import { defaultProps, operators, unique } from './properties'; import { IField } from './types'; import { i18n } from '../../i18n'; @@ -27,6 +25,7 @@ export const number: IField = { hasDefaultValue: true, properties: { ...defaultProps, + unique, 'uiSchema.x-component-props.step': { type: 'string', title: '{{t("Precision")}}', diff --git a/packages/core/client/src/collection-manager/interfaces/password.ts b/packages/core/client/src/collection-manager/interfaces/password.ts index d4b4a13fb..7ff1fc680 100644 --- a/packages/core/client/src/collection-manager/interfaces/password.ts +++ b/packages/core/client/src/collection-manager/interfaces/password.ts @@ -1,5 +1,5 @@ import { i18n } from '../../i18n'; -import { defaultProps } from './properties'; +import { defaultProps, unique } from './properties'; import { IField } from './types'; export const password: IField = { @@ -21,6 +21,7 @@ export const password: IField = { hasDefaultValue: true, properties: { ...defaultProps, + unique, }, validateSchema(fieldSchema) { return { diff --git a/packages/core/client/src/collection-manager/interfaces/percent.ts b/packages/core/client/src/collection-manager/interfaces/percent.ts index 4d483d691..b06a1d52e 100644 --- a/packages/core/client/src/collection-manager/interfaces/percent.ts +++ b/packages/core/client/src/collection-manager/interfaces/percent.ts @@ -1,5 +1,5 @@ import { ISchema } from '@formily/react'; -import { defaultProps, operators } from './properties'; +import { defaultProps, operators, unique } from './properties'; import { IField } from './types'; import { i18n } from '../../i18n'; import { registerValidateFormats, registerValidateRules, registerValidateLocale } from '@formily/core'; @@ -25,7 +25,7 @@ registerValidateRules({ } } } - + return true; }, @@ -71,6 +71,7 @@ export const percent: IField = { hasDefaultValue: true, properties: { ...defaultProps, + unique, 'uiSchema.x-component-props.step': { type: 'string', title: '{{t("Precision")}}', diff --git a/packages/core/client/src/collection-manager/interfaces/phone.ts b/packages/core/client/src/collection-manager/interfaces/phone.ts index 6de65d589..24055ca8c 100644 --- a/packages/core/client/src/collection-manager/interfaces/phone.ts +++ b/packages/core/client/src/collection-manager/interfaces/phone.ts @@ -1,4 +1,4 @@ -import { defaultProps, operators } from './properties'; +import { defaultProps, operators, unique } from './properties'; import { IField } from './types'; export const phone: IField = { @@ -24,6 +24,7 @@ export const phone: IField = { hasDefaultValue: true, properties: { ...defaultProps, + unique, }, filterable: { operators: operators.string, diff --git a/packages/core/client/src/collection-manager/interfaces/properties/index.ts b/packages/core/client/src/collection-manager/interfaces/properties/index.ts index e1fd1ba94..86443c2c1 100644 --- a/packages/core/client/src/collection-manager/interfaces/properties/index.ts +++ b/packages/core/client/src/collection-manager/interfaces/properties/index.ts @@ -30,6 +30,13 @@ export const type: ISchema = { ], }; +export const unique = { + type: 'boolean', + 'x-content': '{{t("Unique")}}', + 'x-decorator': 'FormItem', + 'x-component': 'Checkbox', +}; + export const relationshipType: ISchema = { type: 'string', title: '{{t("Relationship type")}}', diff --git a/packages/core/client/src/locale/zh_CN.ts b/packages/core/client/src/locale/zh_CN.ts index 62920fe93..c9739798d 100644 --- a/packages/core/client/src/locale/zh_CN.ts +++ b/packages/core/client/src/locale/zh_CN.ts @@ -234,6 +234,7 @@ export default { "Custom column name": "自定义列名称", "Edit description": "编辑描述", "Required": "必填", + "Unique": "不允许重复", "Label field": "标签字段", "Default is the ID field": "默认为 ID 字段", "Set default sorting rules": "设置排序规则", diff --git a/packages/core/database/src/__tests__/magic-attribute-model.test.ts b/packages/core/database/src/__tests__/magic-attribute-model.test.ts index b2869d9fd..e4a6d5e69 100644 --- a/packages/core/database/src/__tests__/magic-attribute-model.test.ts +++ b/packages/core/database/src/__tests__/magic-attribute-model.test.ts @@ -12,6 +12,30 @@ describe('magic-attribute-model', () => { await db.close(); }); + it('case 0', async () => { + db.registerModels({ MagicAttributeModel }); + + const Test = db.collection({ + name: 'tests', + model: 'MagicAttributeModel', + fields: [ + { type: 'string', name: 'title' }, + { type: 'json', name: 'options' }, + ], + }); + + await db.sync(); + + const test = await Test.model.create({ + title: 'aa', + 'x-component-props': { key1: 'val1', arr1: [1, 2, 3], arr2: [4, 5] }, + }); + + test.set('x-component-props', { arr2: [1, 2, 3] }); + + expect(test.previous('options')['x-component-props']['arr2']).toEqual([4, 5]); + }); + it('case 1', async () => { db.registerModels({ MagicAttributeModel }); diff --git a/packages/core/database/src/collection.ts b/packages/core/database/src/collection.ts index 08ca7a397..03e5f3353 100644 --- a/packages/core/database/src/collection.ts +++ b/packages/core/database/src/collection.ts @@ -134,10 +134,8 @@ export class Collection< } private bindFieldEventListener() { - this.on('field.afterAdd', (field: Field) => { - field.bind(); - }); - this.on('field.afterRemove', (field) => field.unbind()); + this.on('field.afterAdd', (field: Field) => field.bind()); + this.on('field.afterRemove', (field: Field) => field.unbind()); } forEachField(callback: (field: Field) => void) { @@ -285,7 +283,7 @@ export class Collection< this.setField(options.name || name, options); } - addIndex(index: any) { + addIndex(index: string | string[] | { fields: string[], unique?: boolean,[key: string]: any }) { if (!index) { return; } @@ -347,22 +345,22 @@ export class Collection< } async sync(syncOptions?: SyncOptions) { - const modelNames = [this.model.name]; + const modelNames = new Set([this.model.name]); const associations = this.model.associations; for (const associationKey in associations) { const association = associations[associationKey]; - modelNames.push(association.target.name); + modelNames.add(association.target.name); if ((association).through) { - modelNames.push((association).through.model.name); + modelNames.add((association).through.model.name); } } const models: ModelCtor[] = []; // @ts-ignore this.context.database.sequelize.modelManager.forEachModel((model) => { - if (modelNames.includes(model.name)) { + if (modelNames.has(model.name)) { models.push(model); } }); diff --git a/packages/core/database/src/fields/field.ts b/packages/core/database/src/fields/field.ts index 32d23ec99..029a19e74 100644 --- a/packages/core/database/src/fields/field.ts +++ b/packages/core/database/src/fields/field.ts @@ -179,7 +179,7 @@ export abstract class Field { unbind() { const { model } = this.context.collection; model.removeAttribute(this.name); - if (this.options.index) { + if (this.options.index || this.options.unique) { this.context.collection.removeIndex([this.name]); } } diff --git a/packages/core/database/src/magic-attribute-model.ts b/packages/core/database/src/magic-attribute-model.ts index 958d7fb9a..0577e7adc 100644 --- a/packages/core/database/src/magic-attribute-model.ts +++ b/packages/core/database/src/magic-attribute-model.ts @@ -1,7 +1,9 @@ import { merge } from '@nocobase/utils'; import _ from 'lodash'; +import { Utils } from 'sequelize'; import Database from './database'; import { Model } from './model'; +const Dottie = require('dottie'); export class MagicAttributeModel extends Model { get magicAttribute() { @@ -14,25 +16,205 @@ export class MagicAttributeModel extends Model { if (typeof key === 'string') { const [column] = key.split('.'); if ((this.constructor as any).hasAlias(column)) { - return super.set(key, value, options); + return this.setV1(key, value, options); } if ((this.constructor as any).rawAttributes[column]) { - return super.set(key, value, options); + return this.setV1(key, value, options); } if (_.isPlainObject(value)) { const opts = super.get(this.magicAttribute) || {}; - return super.set(`${this.magicAttribute}.${key}`, merge(opts?.[key], value), options); + return this.setV1(`${this.magicAttribute}.${key}`, merge(opts?.[key], value), options); } - return super.set(`${this.magicAttribute}.${key}`, value, options); + return this.setV1(`${this.magicAttribute}.${key}`, value, options); } else { if (!key) { return; } Object.keys(key).forEach((k) => { - this.set(k, key[k], options); + this.setV1(k, key[k], options); }); } - return super.set(key, value, options); + return this.setV1(key, value, options); + } + + setV1(key?: any, value?: any, options?: any) { + let values; + let originalValue; + + if (typeof key === 'object' && key !== null) { + values = key; + options = value || {}; + + if (options.reset) { + // @ts-ignore + this.dataValues = {}; + for (const key in values) { + this.changed(key, false); + } + } + + // If raw, and we're not dealing with includes or special attributes, just set it straight on the dataValues object + // @ts-ignore + if ( + options.raw && + // @ts-ignore + !(this._options && this._options.include) && + !(options && options.attributes) && + // @ts-ignore + !this.constructor._hasDateAttributes && + // @ts-ignore + !this.constructor._hasBooleanAttributes + ) { + // @ts-ignore + if (Object.keys(this.dataValues).length) { + // @ts-ignore + Object.assign(this.dataValues, values); + } else { + // @ts-ignore + this.dataValues = values; + } + // If raw, .changed() shouldn't be true + // @ts-ignore + this._previousDataValues = { ...this.dataValues }; + } else { + // Loop and call set + if (options.attributes) { + const setKeys = (data) => { + for (const k of data) { + if (values[k] === undefined) { + continue; + } + this.set(k, values[k], options); + } + }; + setKeys(options.attributes); + // @ts-ignore + if (this.constructor._hasVirtualAttributes) { + // @ts-ignore + setKeys(this.constructor._virtualAttributes); + } + // @ts-ignore + if (this._options.includeNames) { + // @ts-ignore + setKeys(this._options.includeNames); + } + } else { + for (const key in values) { + this.set(key, values[key], options); + } + } + + if (options.raw) { + // If raw, .changed() shouldn't be true + // @ts-ignore + this._previousDataValues = { ...this.dataValues }; + } + } + return this; + } + if (!options) options = {}; + if (!options.raw) { + // @ts-ignore + originalValue = this.dataValues[key]; + } + + // If not raw, and there's a custom setter + // @ts-ignore + if (!options.raw && this._customSetters[key]) { + // @ts-ignore + this._customSetters[key].call(this, value, key); + // custom setter should have changed value, get that changed value + // TODO: v5 make setters return new value instead of changing internal store + // @ts-ignore + const newValue = this.dataValues[key]; + if (!_.isEqual(newValue, originalValue)) { + // @ts-ignore + this._previousDataValues[key] = originalValue; + this.changed(key, true); + } + } else { + // Check if we have included models, and if this key matches the include model names/aliases + // @ts-ignore + if (this._options && this._options.include && this._options.includeNames.includes(key)) { + // Pass it on to the include handler + // @ts-ignore + this._setInclude(key, value, options); + return this; + } + // Bunch of stuff we won't do when it's raw + if (!options.raw) { + // If attribute is not in model definition, return + // @ts-ignore + if (!this._isAttribute(key)) { + // @ts-ignore + if (key.includes('.') && this.constructor._jsonAttributes.has(key.split('.')[0])) { + // @ts-ignore + const previousNestedValue = Dottie.get(this.dataValues, key); + if (!_.isEqual(previousNestedValue, value)) { + // @ts-ignore + this._previousDataValues = _.cloneDeep(this._previousDataValues); + // @ts-ignore + Dottie.set(this.dataValues, key, value); + this.changed(key.split('.')[0], true); + } + } + return this; + } + + // If attempting to set primary key and primary key is already defined, return + // @ts-ignore + if (this.constructor._hasPrimaryKeys && originalValue && this.constructor._isPrimaryKey(key)) { + return this; + } + + // If attempting to set read only attributes, return + // @ts-ignore + if ( + !this.isNewRecord && + // @ts-ignore + this.constructor._hasReadOnlyAttributes && + // @ts-ignore + this.constructor._readOnlyAttributes.has(key) + ) { + return this; + } + } + + // If there's a data type sanitizer + if ( + !(value instanceof Utils.SequelizeMethod) && + // @ts-ignore + Object.prototype.hasOwnProperty.call(this.constructor._dataTypeSanitizers, key) + ) { + // @ts-ignore + value = this.constructor._dataTypeSanitizers[key].call(this, value, options); + } + + // Set when the value has changed and not raw + if ( + !options.raw && + // True when sequelize method + (value instanceof Utils.SequelizeMethod || + // Check for data type type comparators + // @ts-ignore + (!(value instanceof Utils.SequelizeMethod) && + // @ts-ignore + this.constructor._dataTypeChanges[key] && + // @ts-ignore + this.constructor._dataTypeChanges[key].call(this, value, originalValue, options)) || // Check default + // @ts-ignore + (!this.constructor._dataTypeChanges[key] && !_.isEqual(value, originalValue))) + ) { + // @ts-ignore + this._previousDataValues[key] = originalValue; + this.changed(key, true); + } + + // set data value + // @ts-ignore + this.dataValues[key] = value; + } + return this; } get(key?: any, value?: any): any { diff --git a/packages/core/database/src/transaction-decorator.ts b/packages/core/database/src/transaction-decorator.ts index fba841468..44be03bf0 100644 --- a/packages/core/database/src/transaction-decorator.ts +++ b/packages/core/database/src/transaction-decorator.ts @@ -43,7 +43,7 @@ export function transactionWrapperBuilder(transactionGenerator) { return results; } catch (err) { - console.error({ err }); + // console.error({ err }); await transaction.rollback(); throw err; } diff --git a/packages/plugins/acl/src/__tests__/prepare.ts b/packages/plugins/acl/src/__tests__/prepare.ts index 933f3237a..7c83d93d0 100644 --- a/packages/plugins/acl/src/__tests__/prepare.ts +++ b/packages/plugins/acl/src/__tests__/prepare.ts @@ -1,4 +1,5 @@ import PluginUsers from '@nocobase/plugin-users'; +import PluginErrorHandler from '@nocobase/plugin-error-handler'; import PluginCollectionManager from '@nocobase/plugin-collection-manager'; import PluginUiSchema from '@nocobase/plugin-ui-schema-storage'; import { mockServer } from '@nocobase/test'; @@ -15,6 +16,7 @@ export async function prepareApp() { app.plugin(PluginUsers); app.plugin(PluginUiSchema); + app.plugin(PluginErrorHandler); app.plugin(PluginCollectionManager); app.plugin(PluginACL); diff --git a/packages/plugins/collection-manager/package.json b/packages/plugins/collection-manager/package.json index 1525bfc93..efc25c7d8 100644 --- a/packages/plugins/collection-manager/package.json +++ b/packages/plugins/collection-manager/package.json @@ -9,6 +9,12 @@ "url": "http://www.apache.org/licenses/LICENSE-2.0" } ], + "dependencies": { + "@nocobase/database": "0.7.4-alpha.4", + "@nocobase/plugin-error-handler": "0.7.4-alpha.4", + "@nocobase/server": "0.7.4-alpha.4", + "sequelize": "^6.9.0" + }, "devDependencies": { "@nocobase/test": "0.7.4-alpha.7" }, diff --git a/packages/plugins/collection-manager/src/__tests__/action.test.ts b/packages/plugins/collection-manager/src/__tests__/action.test.ts index fc70d19e2..dd5e64245 100644 --- a/packages/plugins/collection-manager/src/__tests__/action.test.ts +++ b/packages/plugins/collection-manager/src/__tests__/action.test.ts @@ -2,19 +2,16 @@ import { Database } from '@nocobase/database'; import { mockServer, MockServer } from '@nocobase/test'; import CollectionManagerPlugin from '@nocobase/plugin-collection-manager'; import { UiSchemaStoragePlugin } from '@nocobase/plugin-ui-schema-storage'; +import { createApp } from '.'; describe('action test', () => { let db: Database; let app: MockServer; beforeEach(async () => { - app = mockServer(); - app.plugin(CollectionManagerPlugin); - app.plugin(UiSchemaStoragePlugin); - + app = await createApp(); + await app.install({ clean: true }); db = app.db; - await db.clean({ drop: true }); - await app.loadAndInstall(); }); afterEach(async () => { diff --git a/packages/plugins/collection-manager/src/__tests__/field-options/indexes.test.ts b/packages/plugins/collection-manager/src/__tests__/field-options/indexes.test.ts new file mode 100644 index 000000000..3b4a35476 --- /dev/null +++ b/packages/plugins/collection-manager/src/__tests__/field-options/indexes.test.ts @@ -0,0 +1,96 @@ +import { MockServer } from '@nocobase/test'; +import { createApp } from '..'; + +describe('field indexes', () => { + let app: MockServer; + let agent; + + beforeEach(async () => { + app = await createApp(); + await app.install({ clean: true }); + await app.start(); + agent = app.agent(); + await agent + .resource('collections') + .create({ + values: { + name: 'test1', + }, + }); + }); + + afterEach(async () => { + await app.destroy(); + }); + + it('field value cannot be duplicated with unique index', async () => { + const tableName = 'test1'; + // create an field with unique constraint + const field = await agent + .resource('collections.fields', tableName) + .create({ + values: { + name: 'title', + type: 'string', + unique: true + }, + }); + + // create a record + const response1 = await agent.resource(tableName).create({ + values: { + title: 't1' + } + }); + expect(response1.status).toBe(200); + expect(response1.body.data.title).toBe('t1'); + + // create another record with the same value on unique field should fail + const response2 = await agent.resource(tableName).create({ + values: { + title: 't1' + } + }); + expect(response2.status).toBe(400); + + // update field to remove unique constraint + await agent.resource('fields').update({ + filterByTk: field.id, + values: { + unique: false + } + }); + + // create another record with the same value on unique field should be ok + const response3 = await agent.resource(tableName).create({ + values: { + title: 't1' + } + }); + expect(response3.status).toBe(200); + expect(response3.body.data.title).toBe('t1'); + + // update field to add unique constraint should fail because of duplicated records + const response4 = await agent.resource('fields').update({ + filterByTk: field.id, + values: { + unique: true + } + }); + expect(response4.status).toBe(400); + + // remove a duplicated record + await agent.resource(tableName).destroy({ + filterByTk: response3.body.data.id + }); + + // update field to add unique constraint should be ok + const response6 = await agent.resource('fields').update({ + filterByTk: field.id, + values: { + unique: true + } + }); + expect(response6.status).toBe(200); + }); +}); diff --git a/packages/plugins/collection-manager/src/__tests__/http-api/collections.test.ts b/packages/plugins/collection-manager/src/__tests__/http-api/collections.test.ts index 9bc87cdfd..6b3255723 100644 --- a/packages/plugins/collection-manager/src/__tests__/http-api/collections.test.ts +++ b/packages/plugins/collection-manager/src/__tests__/http-api/collections.test.ts @@ -3,13 +3,14 @@ import { createApp } from '..'; describe('collections repository', () => { let app: MockServer; + let agent; beforeEach(async () => { app = await createApp(); + agent = app.agent(); await app.install({ clean: true }); await app.start(); - await app - .agent() + await agent .resource('collections') .create({ values: { @@ -22,8 +23,7 @@ describe('collections repository', () => { ], }, }); - await app - .agent() + await agent .resource('collections') .create({ values: { @@ -36,8 +36,7 @@ describe('collections repository', () => { ], }, }); - await app - .agent() + await agent .resource('collections.fields', 'tags') .create({ values: { @@ -46,8 +45,7 @@ describe('collections repository', () => { type: 'belongsToMany', }, }); - await app - .agent() + await agent .resource('collections') .create({ values: { @@ -60,8 +58,7 @@ describe('collections repository', () => { ], }, }); - await app - .agent() + await agent .resource('collections') .create({ values: { @@ -100,24 +97,21 @@ describe('collections repository', () => { it('case 2', async () => { const response = await app.agent().resource('posts').create(); const postId = response.body.data.id; - await app - .agent() + await agent .resource('posts.comments', postId) .create({ values: { title: 'comment 1', }, }); - await app - .agent() + await agent .resource('posts.comments', postId) .create({ values: { title: 'comment 2', }, }); - const response2 = await app - .agent() + const response2 = await agent .resource('posts') .list({ filter: { @@ -130,16 +124,14 @@ describe('collections repository', () => { it('case 3', async () => { const response = await app.agent().resource('posts').create(); const postId = response.body.data.id; - await app - .agent() + await agent .resource('posts.comments', postId) .create({ values: { title: 'comment 1', }, }); - const response2 = await app - .agent() + const response2 = await agent .resource('posts') .list({ filter: { @@ -152,16 +144,14 @@ describe('collections repository', () => { it('case 4', async () => { const response = await app.agent().resource('posts').create(); const postId = response.body.data.id; - await app - .agent() + await agent .resource('posts.comments', postId) .create({ values: { title: 'comment 1', }, }); - const response2 = await app - .agent() + const response2 = await agent .resource('posts') .list({ filter: { @@ -186,8 +176,7 @@ describe('collections repository', () => { }); it('case 6', async () => { - const response = await app - .agent() + const response = await agent .resource('posts') .create({ values: { @@ -205,8 +194,7 @@ describe('collections repository', () => { }); it('case 7', async () => { - const response = await app - .agent() + const response = await agent .resource('posts') .create({ values: { @@ -219,8 +207,7 @@ describe('collections repository', () => { }, }); const postId = response.body.data.id; - const response1 = await app - .agent() + const response1 = await agent .resource('posts.tags', postId) .list({ filter: { @@ -231,8 +218,7 @@ describe('collections repository', () => { }); it('case 8', async () => { - const response = await app - .agent() + const response = await agent .resource('posts') .create({ values: { @@ -248,8 +234,7 @@ describe('collections repository', () => { }, }); const postId = response.body.data.id; - const response1 = await app - .agent() + const response1 = await agent .resource('posts.tags', postId) .list({ filter: { @@ -260,8 +245,7 @@ describe('collections repository', () => { }); it('case 9', async () => { - const response = await app - .agent() + const response = await agent .resource('posts') .create({ values: { @@ -277,8 +261,7 @@ describe('collections repository', () => { }, }); const postId = response.body.data.id; - const response1 = await app - .agent() + const response1 = await agent .resource('posts.tags', postId) .list({ filter: { @@ -289,8 +272,7 @@ describe('collections repository', () => { }); it('case 10', async () => { - const response = await app - .agent() + const response = await agent .resource('posts') .create({ values: { @@ -306,8 +288,7 @@ describe('collections repository', () => { }, }); const postId = response.body.data.id; - const response1 = await app - .agent() + const response1 = await agent .resource('posts.tags', postId) .list({ appends: ['foos'], @@ -322,16 +303,14 @@ describe('collections repository', () => { it('case 11', async () => { const response = await app.agent().resource('posts').create(); const postId = response.body.data.id; - await app - .agent() + await agent .resource('posts.comments', postId) .create({ values: { title: 'comment 1', }, }); - await app - .agent() + await agent .resource('posts.comments', postId) .create({ values: { @@ -340,24 +319,21 @@ describe('collections repository', () => { }); const response2 = await app.agent().resource('posts').create(); const postId2 = response2.body.data.id; - await app - .agent() + await agent .resource('posts.comments', postId2) .create({ values: { title: 'comment 2', }, }); - await app - .agent() + await agent .resource('posts.comments', postId2) .create({ values: { title: 'comment 2', }, }); - const response3 = await app - .agent() + const response3 = await agent .resource('posts') .list({ filter: { @@ -377,32 +353,28 @@ describe('collections repository', () => { it('case 12', async () => { const response = await app.agent().resource('posts').create(); const postId = response.body.data.id; - await app - .agent() + await agent .resource('posts.comments', postId) .create({ values: { title: 'comment 1', }, }); - await app - .agent() + await agent .resource('posts.comments', postId) .create({ values: { title: 'comment 2', }, }); - await app - .agent() + await agent .resource('posts.comments', postId) .create({ values: { title: 'comment 3', }, }); - const response2 = await app - .agent() + const response2 = await agent .resource('posts.comments', postId) .list({ filter: { @@ -424,24 +396,21 @@ describe('collections repository', () => { const tag1 = await tagRepository.create({ values: { title: 'tag1' } }); const tag2 = await tagRepository.create({ values: { title: 'tag2' } }); const tag3 = await tagRepository.create({ values: { title: 'tag3' } }); - await app - .agent() + await agent .resource('posts') .create({ values: { tags: [tag1.get('id'), tag3.get('id')], }, }); - await app - .agent() + await agent .resource('posts') .create({ values: { tags: [tag2.get('id')], }, }); - await app - .agent() + await agent .resource('posts') .create({ values: { @@ -449,8 +418,7 @@ describe('collections repository', () => { }, }); - const response1 = await app - .agent() + const response1 = await agent .resource('posts') .list({ filter: { diff --git a/packages/plugins/collection-manager/src/__tests__/index.ts b/packages/plugins/collection-manager/src/__tests__/index.ts index 51fa038cb..9926197ec 100644 --- a/packages/plugins/collection-manager/src/__tests__/index.ts +++ b/packages/plugins/collection-manager/src/__tests__/index.ts @@ -1,7 +1,8 @@ +import PluginErrorHandler from '@nocobase/plugin-error-handler'; import PluginUiSchema from '@nocobase/plugin-ui-schema-storage'; import { mockServer } from '@nocobase/test'; import lodash from 'lodash'; -import CollectionManagerPlugin from '../'; +import Plugin from '../'; export async function createApp(options = {}) { const app = mockServer(); @@ -10,7 +11,8 @@ export async function createApp(options = {}) { await app.cleanDb(); } - app.plugin(CollectionManagerPlugin); + app.plugin(PluginErrorHandler); + app.plugin(Plugin); app.plugin(PluginUiSchema); await app.load(); diff --git a/packages/plugins/collection-manager/src/__tests__/through.test.ts b/packages/plugins/collection-manager/src/__tests__/through.test.ts index 3df917ade..54aded2b5 100644 --- a/packages/plugins/collection-manager/src/__tests__/through.test.ts +++ b/packages/plugins/collection-manager/src/__tests__/through.test.ts @@ -1,5 +1,6 @@ +import PluginErrorHandler from '@nocobase/plugin-error-handler'; import { mockServer } from '@nocobase/test'; -import CollectionManagerPlugin from '../server'; +import Plugin from '../server'; describe('collections repository', () => { it('case 1', async () => { @@ -9,7 +10,8 @@ describe('collections repository', () => { }, }); await app1.cleanDb(); - app1.plugin(CollectionManagerPlugin); + app1.plugin(PluginErrorHandler); + app1.plugin(Plugin); await app1.load(); await app1.install({ clean: true }); await app1.start(); @@ -119,7 +121,8 @@ describe('collections repository', () => { tablePrefix: 'through_', }, }); - app2.plugin(CollectionManagerPlugin); + app2.plugin(PluginErrorHandler); + app2.plugin(Plugin); await app2.load(); await app2.start(); diff --git a/packages/plugins/collection-manager/src/models/field.ts b/packages/plugins/collection-manager/src/models/field.ts index 121803904..38c2333c1 100644 --- a/packages/plugins/collection-manager/src/models/field.ts +++ b/packages/plugins/collection-manager/src/models/field.ts @@ -1,11 +1,53 @@ -import Database, { MagicAttributeModel } from '@nocobase/database'; -import { SyncOptions, Transactionable } from 'sequelize'; +import Database, { Field, MagicAttributeModel } from '@nocobase/database'; +import { SyncOptions, Transactionable, UniqueConstraintError, Utils } from 'sequelize'; interface LoadOptions extends Transactionable { // TODO skipExist?: boolean; } +interface MigrateOptions extends SyncOptions, Transactionable {} + +async function migrate(field: Field, options: MigrateOptions): Promise { + const { unique } = field.options; + const { model } = field.collection; + const ukName = `${model.tableName}_${field.name}_uk`; + const queryInterface = model.sequelize.getQueryInterface(); + const fieldAttribute = model.rawAttributes[field.name]; + // @ts-ignore + const existedConstraints = await queryInterface.showConstraint(model.tableName, ukName, { transaction: options.transaction }) as any[]; + const constraintBefore = existedConstraints.find(item => item.constraintName === ukName); + if (typeof fieldAttribute?.unique !== 'undefined') { + if (constraintBefore && !unique) { + await queryInterface.removeConstraint(model.tableName, ukName, { transaction: options.transaction }); + } + fieldAttribute.unique = Boolean(constraintBefore); + } + + await field.sync(options); + + if (!constraintBefore && unique) { + await queryInterface.addConstraint(model.tableName, { + type: 'unique', + fields: [field.name], + name: ukName, + transaction: options.transaction + }); + } + if (typeof fieldAttribute?.unique !== 'undefined') { + fieldAttribute.unique = unique; + } + + // @ts-ignore + const updatedConstraints = await queryInterface.showConstraint(model.tableName, ukName, { transaction: options.transaction }) as any[]; + const indexAfter = updatedConstraints.find(item => item.constraintName === ukName); + if (unique && !indexAfter) { + throw new UniqueConstraintError({ + fields: { [field.name]: undefined } + }); + } +} + export class FieldModel extends MagicAttributeModel { get db(): Database { return (this.constructor).database; @@ -28,21 +70,21 @@ export class FieldModel extends MagicAttributeModel { const uiSchema = await UISchema.findByPk(options.uiSchemaUid, { transaction: loadOptions.transaction, }); - return collection.setField(name, { ...options, uiSchema: uiSchema.get() }); - } else { - return collection.setField(name, options); + Object.assign(options, { uiSchema: uiSchema.get() }); } + return collection.setField(name, options); } - async migrate(options?: SyncOptions & Transactionable) { + async migrate(options: MigrateOptions = {}) { const field = await this.load({ transaction: options.transaction, }); if (!field) { return; } + // const migrator = Dialects[field.database.options.dialect] ?? migrate; try { - await field.sync(options); + await migrate(field, options); } catch (error) { // field sync failed, delete from memory field.remove(); diff --git a/packages/plugins/collection-manager/src/server.ts b/packages/plugins/collection-manager/src/server.ts index 7f88019ed..96d5e7855 100644 --- a/packages/plugins/collection-manager/src/server.ts +++ b/packages/plugins/collection-manager/src/server.ts @@ -1,6 +1,11 @@ -import { Plugin } from '@nocobase/server'; -import lodash from 'lodash'; import path from 'path'; + +import lodash from 'lodash'; +import { UniqueConstraintError } from 'sequelize'; + +import PluginErrorHandler from '@nocobase/plugin-error-handler'; +import { Plugin } from '@nocobase/server'; + import { CollectionRepository } from '.'; import { afterCreateForReverseField, @@ -64,19 +69,23 @@ export class CollectionManagerPlugin extends Plugin { } }); - this.app.db.on('fields.afterCreate', async (model, { context, transaction }) => { + this.app.db.on('fields.afterCreate', async (model: FieldModel, { context, transaction }) => { if (context) { await model.migrate({ transaction }); } }); - this.app.db.on('fields.afterUpdateWithAssociations', async (model, { context, transaction }) => { + this.app.db.on('fields.afterUpdate', async (model: FieldModel, { context, transaction }) => { if (context) { - await model.load({ transaction }); + const prev = model.previous('options')?.unique; + const next = model.get('options')?.unique; + if (lodash.isBoolean(prev) && lodash.isBoolean(next) && prev !== next) { + await model.migrate({ transaction }); + } } }); - this.app.db.on('fields.afterCreateWithAssociations', async (model, { context, transaction }) => { + this.app.db.on('fields.afterSaveWithAssociations', async (model, { context, transaction }) => { if (context) { await model.load({ transaction }); } @@ -168,6 +177,14 @@ export class CollectionManagerPlugin extends Plugin { await this.app.db.import({ directory: path.resolve(__dirname, './collections'), }); + + const errorHandlerPlugin = this.app.getPlugin('@nocobase/plugin-error-handler'); + errorHandlerPlugin.errorHandler.register( + (err) => err instanceof UniqueConstraintError, + (err, ctx) => { + return ctx.throw(400, ctx.t(`The value of ${Object.keys(err.fields)} field duplicated`)); + }, + ); } getName(): string { diff --git a/packages/plugins/error-handler/src/error-handler.ts b/packages/plugins/error-handler/src/error-handler.ts index f31cfd16a..ce75dc925 100644 --- a/packages/plugins/error-handler/src/error-handler.ts +++ b/packages/plugins/error-handler/src/error-handler.ts @@ -18,6 +18,9 @@ export class ErrorHandler { }, ], }; + if (ctx.status === 500) { + console.error(err); + } } middleware() { @@ -27,7 +30,6 @@ export class ErrorHandler { try { await next(); } catch (err) { - console.error(err); for (const handler of self.handlers) { if (handler.guard(err)) { return handler.render(err, ctx); diff --git a/packages/plugins/ui-schema-storage/src/__tests__/server-hook-impl.test.ts b/packages/plugins/ui-schema-storage/src/__tests__/server-hook-impl.test.ts index db93e4561..c25cd9ba1 100644 --- a/packages/plugins/ui-schema-storage/src/__tests__/server-hook-impl.test.ts +++ b/packages/plugins/ui-schema-storage/src/__tests__/server-hook-impl.test.ts @@ -1,6 +1,7 @@ import { BelongsToManyRepository, Database } from '@nocobase/database'; import PluginUsers from '@nocobase/plugin-users'; import PluginACL from '@nocobase/plugin-acl'; +import PluginErrorHandler from '@nocobase/plugin-error-handler'; import PluginCollectionManager from '@nocobase/plugin-collection-manager'; import UiSchemaStoragePlugin, { UiSchemaRepository } from '@nocobase/plugin-ui-schema-storage'; import { mockServer, MockServer } from '@nocobase/test'; @@ -24,6 +25,7 @@ describe('server hooks', () => { db = app.db; app.plugin(UiSchemaStoragePlugin); + app.plugin(PluginErrorHandler); app.plugin(PluginCollectionManager); app.plugin(PluginUsers); app.plugin(PluginACL); diff --git a/packages/plugins/ui-schema-storage/src/__tests__/server-hook.test.ts b/packages/plugins/ui-schema-storage/src/__tests__/server-hook.test.ts index f770210a3..3f019aed9 100644 --- a/packages/plugins/ui-schema-storage/src/__tests__/server-hook.test.ts +++ b/packages/plugins/ui-schema-storage/src/__tests__/server-hook.test.ts @@ -1,4 +1,5 @@ import { Database } from '@nocobase/database'; +import PluginErrorHandler from '@nocobase/plugin-error-handler'; import PluginCollectionManager from '@nocobase/plugin-collection-manager'; import UiSchemaStoragePlugin, { UiSchemaRepository } from '@nocobase/plugin-ui-schema-storage'; import { mockServer, MockServer } from '@nocobase/test'; @@ -62,6 +63,7 @@ describe('server hooks', () => { db = app.db; app.plugin(UiSchemaStoragePlugin); + app.plugin(PluginErrorHandler); app.plugin(PluginCollectionManager); await app.loadAndInstall();