From ce28c680186004212fd8813c63806384f2b43367 Mon Sep 17 00:00:00 2001 From: Junyi Date: Tue, 1 Apr 2025 08:34:06 +0800 Subject: [PATCH 01/10] refactor(plugin-workflow): improve code of schedule trigger (#6589) --- .../client/triggers/schedule/RepeatField.tsx | 12 ++- .../plugin-workflow/src/server/Plugin.ts | 5 ++ .../DateFieldScheduleTrigger.ts | 76 ++++++++++--------- .../ScheduleTrigger/StaticScheduleTrigger.ts | 59 +++++++------- 4 files changed, 83 insertions(+), 69 deletions(-) diff --git a/packages/plugins/@nocobase/plugin-workflow/src/client/triggers/schedule/RepeatField.tsx b/packages/plugins/@nocobase/plugin-workflow/src/client/triggers/schedule/RepeatField.tsx index 206f8b273a..9024a5a614 100644 --- a/packages/plugins/@nocobase/plugin-workflow/src/client/triggers/schedule/RepeatField.tsx +++ b/packages/plugins/@nocobase/plugin-workflow/src/client/triggers/schedule/RepeatField.tsx @@ -42,7 +42,7 @@ function getRepeatTypeValue(v) { return 'none'; } -function CommonRepeatField({ value, onChange }) { +function CommonRepeatField({ value, onChange, disabled }) { const { t } = useWorkflowTranslation(); const option = getNumberOption(value); @@ -59,11 +59,12 @@ function CommonRepeatField({ value, onChange }) { addonBefore={t('Every')} addonAfter={t(option.unitText)} className="auto-width" + disabled={disabled} /> ); } -export function RepeatField({ value = null, onChange }) { +export function RepeatField({ value = null, onChange, disabled }) { const { t } = useWorkflowTranslation(); const typeValue = getRepeatTypeValue(value); const onTypeChange = useCallback( @@ -114,20 +115,23 @@ export function RepeatField({ value = null, onChange }) { } `} > - {RepeatOptions.map((item) => ( {t(item.text)} ))} - {typeof typeValue === 'number' ? : null} + {typeof typeValue === 'number' ? ( + + ) : null} {typeValue === 'cron' ? ( onChange(`0 ${v}`)} clearButton={false} locale={window['cronLocale']} + disabled={disabled} /> ) : null} diff --git a/packages/plugins/@nocobase/plugin-workflow/src/server/Plugin.ts b/packages/plugins/@nocobase/plugin-workflow/src/server/Plugin.ts index 794b1dc158..998ddc42a9 100644 --- a/packages/plugins/@nocobase/plugin-workflow/src/server/Plugin.ts +++ b/packages/plugins/@nocobase/plugin-workflow/src/server/Plugin.ts @@ -366,11 +366,16 @@ export default class PluginWorkflowServer extends Plugin { const prev = workflow.previous(); if (prev.config) { trigger.off({ ...workflow.get(), ...prev }); + this.getLogger(workflow.id).info(`toggle OFF workflow ${workflow.id} based on configuration before updated`); } trigger.on(workflow); + this.getLogger(workflow.id).info(`toggle ON workflow ${workflow.id}`); + this.enabledCache.set(workflow.id, workflow); } else { trigger.off(workflow); + this.getLogger(workflow.id).info(`toggle OFF workflow ${workflow.id}`); + this.enabledCache.delete(workflow.id); } if (!silent) { diff --git a/packages/plugins/@nocobase/plugin-workflow/src/server/triggers/ScheduleTrigger/DateFieldScheduleTrigger.ts b/packages/plugins/@nocobase/plugin-workflow/src/server/triggers/ScheduleTrigger/DateFieldScheduleTrigger.ts index c720e13eca..5a62986a72 100644 --- a/packages/plugins/@nocobase/plugin-workflow/src/server/triggers/ScheduleTrigger/DateFieldScheduleTrigger.ts +++ b/packages/plugins/@nocobase/plugin-workflow/src/server/triggers/ScheduleTrigger/DateFieldScheduleTrigger.ts @@ -104,50 +104,54 @@ export default class DateFieldScheduleTrigger { // caching workflows in range, default to 5min cacheCycle = 300_000; + onAfterStart = () => { + if (this.timer) { + return; + } + + this.timer = setInterval(() => this.reload(), this.cacheCycle); + + this.reload(); + }; + + onBeforeStop = () => { + if (this.timer) { + clearInterval(this.timer); + } + + for (const [key, timer] of this.cache.entries()) { + clearTimeout(timer); + this.cache.delete(key); + } + }; + constructor(public workflow: Plugin) { - workflow.app.on('afterStart', async () => { - if (this.timer) { - return; - } - - this.timer = setInterval(() => this.reload(), this.cacheCycle); - - this.reload(); - }); - - workflow.app.on('beforeStop', () => { - if (this.timer) { - clearInterval(this.timer); - } - - for (const [key, timer] of this.cache.entries()) { - clearTimeout(timer); - this.cache.delete(key); - } - }); + workflow.app.on('afterStart', this.onAfterStart); + workflow.app.on('beforeStop', this.onBeforeStop); } - async reload() { + reload() { + for (const [key, timer] of this.cache.entries()) { + clearTimeout(timer); + this.cache.delete(key); + } + const workflows = Array.from(this.workflow.enabledCache.values()).filter( (item) => item.type === 'schedule' && item.config.mode === SCHEDULE_MODE.DATE_FIELD, ); - // NOTE: clear cached jobs in last cycle - this.cache = new Map(); - - this.inspect(workflows); + workflows.forEach((workflow) => { + this.inspect(workflow); + }); } - inspect(workflows: WorkflowModel[]) { + async inspect(workflow: WorkflowModel) { const now = new Date(); - - workflows.forEach(async (workflow) => { - const records = await this.loadRecordsToSchedule(workflow, now); - this.workflow.getLogger(workflow.id).info(`[Schedule on date field] ${records.length} records to schedule`); - records.forEach((record) => { - const nextTime = this.getRecordNextTime(workflow, record); - this.schedule(workflow, record, nextTime, Boolean(nextTime)); - }); + const records = await this.loadRecordsToSchedule(workflow, now); + this.workflow.getLogger(workflow.id).info(`[Schedule on date field] ${records.length} records to schedule`); + records.forEach((record) => { + const nextTime = this.getRecordNextTime(workflow, record); + this.schedule(workflow, record, nextTime, Boolean(nextTime)); }); } @@ -233,8 +237,6 @@ export default class DateFieldScheduleTrigger { [Op.gte]: new Date(endTimestamp), }, }); - } else { - this.workflow.getLogger(id).warn(`[Schedule on date field] "endsOn.field" is not configured`); } } } @@ -367,7 +369,7 @@ export default class DateFieldScheduleTrigger { } on(workflow: WorkflowModel) { - this.inspect([workflow]); + this.inspect(workflow); const { collection } = workflow.config; const [dataSourceName, collectionName] = parseCollectionName(collection); diff --git a/packages/plugins/@nocobase/plugin-workflow/src/server/triggers/ScheduleTrigger/StaticScheduleTrigger.ts b/packages/plugins/@nocobase/plugin-workflow/src/server/triggers/ScheduleTrigger/StaticScheduleTrigger.ts index 9552ceae22..988c4f545b 100644 --- a/packages/plugins/@nocobase/plugin-workflow/src/server/triggers/ScheduleTrigger/StaticScheduleTrigger.ts +++ b/packages/plugins/@nocobase/plugin-workflow/src/server/triggers/ScheduleTrigger/StaticScheduleTrigger.ts @@ -18,36 +18,39 @@ const MAX_SAFE_INTERVAL = 2147483647; export default class StaticScheduleTrigger { private timers: Map = new Map(); - constructor(public workflow: Plugin) { - workflow.app.on('afterStart', async () => { - const workflows = Array.from(this.workflow.enabledCache.values()).filter( - (item) => item.type === 'schedule' && item.config.mode === SCHEDULE_MODE.STATIC, - ); - - this.inspect(workflows); - }); - - workflow.app.on('beforeStop', () => { - for (const timer of this.timers.values()) { - clearInterval(timer); - } - }); - } - - inspect(workflows: WorkflowModel[]) { - const now = new Date(); + onAfterStart = () => { + const workflows = Array.from(this.workflow.enabledCache.values()).filter( + (item) => item.type === 'schedule' && item.config.mode === SCHEDULE_MODE.STATIC, + ); workflows.forEach((workflow) => { - const nextTime = this.getNextTime(workflow, now); - if (nextTime) { - this.workflow - .getLogger(workflow.id) - .info(`caching scheduled workflow will run at: ${new Date(nextTime).toISOString()}`); - } else { - this.workflow.getLogger(workflow.id).info('workflow will not be scheduled'); - } - this.schedule(workflow, nextTime, nextTime >= now.getTime()); + this.inspect(workflow); }); + }; + + onBeforeStop = () => { + for (const timer of this.timers.values()) { + clearInterval(timer); + } + }; + + constructor(public workflow: Plugin) { + workflow.app.on('afterStart', this.onAfterStart); + workflow.app.on('beforeStop', this.onBeforeStop); + } + + inspect(workflow: WorkflowModel) { + const now = new Date(); + + const nextTime = this.getNextTime(workflow, now); + if (nextTime) { + this.workflow + .getLogger(workflow.id) + .info(`caching scheduled workflow will run at: ${new Date(nextTime).toISOString()}`); + } else { + this.workflow.getLogger(workflow.id).info('workflow will not be scheduled'); + } + this.schedule(workflow, nextTime, nextTime >= now.getTime()); } getNextTime({ config, allExecuted }: WorkflowModel, currentDate: Date, nextSecond = false) { @@ -130,7 +133,7 @@ export default class StaticScheduleTrigger { } on(workflow) { - this.inspect([workflow]); + this.inspect(workflow); } off(workflow) { From 04e943a0b2febf3b6c02a011c921f0484b20db24 Mon Sep 17 00:00:00 2001 From: YANG QIA <2013xile@gmail.com> Date: Tue, 1 Apr 2025 11:00:59 +0800 Subject: [PATCH 02/10] fix(m2m-array): error of filtering by fields in an association collection with a m2m array field (#6596) * fix(m2m-array): error of filtering by fields in an association collection with a m2m array field * test: add test * feat: generateJoinOnForJSONArray --- .../belongs-to-array-repository.ts | 13 ++- .../src/eager-loading/eager-loading-tree.ts | 37 ++++--- .../query-interface/mysql-query-interface.ts | 4 + .../postgres-query-interface.ts | 4 + .../src/query-interface/query-interface.ts | 5 + .../query-interface/sqlite-query-interface.ts | 4 + .../src/server/__tests__/issues.test.ts | 96 +++++++++++++++++++ .../__tests__/m2m-array-bigint-api.test.ts | 19 +--- .../__tests__/m2m-array-string-api.test.ts | 19 +--- 9 files changed, 153 insertions(+), 48 deletions(-) diff --git a/packages/core/database/src/belongs-to-array/belongs-to-array-repository.ts b/packages/core/database/src/belongs-to-array/belongs-to-array-repository.ts index 375e65829e..254baf3aa0 100644 --- a/packages/core/database/src/belongs-to-array/belongs-to-array-repository.ts +++ b/packages/core/database/src/belongs-to-array/belongs-to-array-repository.ts @@ -54,19 +54,18 @@ export class BelongsToArrayAssociation { return this.db.getModel(this.targetName); } - generateInclude() { - if (this.db.sequelize.getDialect() !== 'postgres') { - throw new Error('Filtering by many to many (array) associations is only supported on postgres'); - } + generateInclude(parentAs?: string) { const targetCollection = this.db.getCollection(this.targetName); const targetField = targetCollection.getField(this.targetKey); const sourceCollection = this.db.getCollection(this.source.name); const foreignField = sourceCollection.getField(this.foreignKey); const queryInterface = this.db.sequelize.getQueryInterface(); - const left = queryInterface.quoteIdentifiers(`${this.as}.${targetField.columnName()}`); - const right = queryInterface.quoteIdentifiers(`${this.source.collection.name}.${foreignField.columnName()}`); + const asLeft = parentAs ? `${parentAs}->${this.as}` : this.as; + const asRight = parentAs || this.source.collection.name; + const left = queryInterface.quoteIdentifiers(`${asLeft}.${targetField.columnName()}`); + const right = queryInterface.quoteIdentifiers(`${asRight}.${foreignField.columnName()}`); return { - on: this.db.sequelize.literal(`${left}=any(${right})`), + on: this.db.queryInterface.generateJoinOnForJSONArray(left, right), }; } diff --git a/packages/core/database/src/eager-loading/eager-loading-tree.ts b/packages/core/database/src/eager-loading/eager-loading-tree.ts index d0d9fb2eeb..c450eb816f 100644 --- a/packages/core/database/src/eager-loading/eager-loading-tree.ts +++ b/packages/core/database/src/eager-loading/eager-loading-tree.ts @@ -82,6 +82,31 @@ const queryParentSQL = (options: { SELECT ${q(targetKeyField)} AS ${q(targetKey)}, ${q(foreignKeyField)} AS ${q(foreignKey)} FROM cte`; }; +const processIncludes = (includes: any[], model: any, parentAs = '') => { + includes.forEach((include: { association: string; include?: any[] }, index: number) => { + // Process current level + const association = model.associations[include.association]; + if (association?.generateInclude) { + includes[index] = { + ...include, + ...association.generateInclude(parentAs), + }; + } + + // Recursively process nested includes if they exist + if (include.include && Array.isArray(include.include) && include.include.length > 0) { + // Get the associated model for the next level + const nextModel = association?.target; + if (!nextModel) { + return; + } + processIncludes(include.include, nextModel, parentAs ? `${parentAs}->${association.as}` : association.as); + } + }); + + return includes; +}; + export class EagerLoadingTree { public root: EagerLoadingNode; db: Database; @@ -252,16 +277,6 @@ export class EagerLoadingTree { throw new Error(`Model ${node.model.name} does not have primary key`); } - includeForFilter.forEach((include: { association: string }, index: number) => { - const association = node.model.associations[include.association]; - if (association?.associationType == 'BelongsToArray') { - includeForFilter[index] = { - ...include, - ...association.generateInclude(), - }; - } - }); - // find all ids const ids = ( await node.model.findAll({ @@ -270,7 +285,7 @@ export class EagerLoadingTree { attributes: [primaryKeyField], group: `${node.model.name}.${primaryKeyField}`, transaction, - include: includeForFilter, + include: processIncludes(includeForFilter, node.model), } as any) ).map((row) => { return { row, pk: row[primaryKeyField] }; diff --git a/packages/core/database/src/query-interface/mysql-query-interface.ts b/packages/core/database/src/query-interface/mysql-query-interface.ts index fff3afa528..62c2c7febd 100644 --- a/packages/core/database/src/query-interface/mysql-query-interface.ts +++ b/packages/core/database/src/query-interface/mysql-query-interface.ts @@ -141,4 +141,8 @@ export default class MysqlQueryInterface extends QueryInterface { await this.db.sequelize.query(sql, { transaction }); } } + + public generateJoinOnForJSONArray(left: string, right: string) { + return this.db.sequelize.literal(`JSON_CONTAINS(${right}, JSON_ARRAY(${left}))`); + } } diff --git a/packages/core/database/src/query-interface/postgres-query-interface.ts b/packages/core/database/src/query-interface/postgres-query-interface.ts index 2d251ddf51..0b38c7c098 100644 --- a/packages/core/database/src/query-interface/postgres-query-interface.ts +++ b/packages/core/database/src/query-interface/postgres-query-interface.ts @@ -232,4 +232,8 @@ $BODY$ return res[0]['show_create_table']; } + + public generateJoinOnForJSONArray(left: string, right: string) { + return this.db.sequelize.literal(`${left}=any(${right})`); + } } diff --git a/packages/core/database/src/query-interface/query-interface.ts b/packages/core/database/src/query-interface/query-interface.ts index c7d484ccb7..9d8ffe349a 100644 --- a/packages/core/database/src/query-interface/query-interface.ts +++ b/packages/core/database/src/query-interface/query-interface.ts @@ -83,4 +83,9 @@ export default abstract class QueryInterface { // @ts-ignore return this.db.sequelize.getQueryInterface().queryGenerator.quoteIdentifier(identifier); } + + public generateJoinOnForJSONArray(left: string, right: string) { + const dialect = this.db.sequelize.getDialect(); + throw new Error(`Filtering by many to many (array) associations is not supported on ${dialect}`); + } } diff --git a/packages/core/database/src/query-interface/sqlite-query-interface.ts b/packages/core/database/src/query-interface/sqlite-query-interface.ts index 2f6b93c877..ec77652c56 100644 --- a/packages/core/database/src/query-interface/sqlite-query-interface.ts +++ b/packages/core/database/src/query-interface/sqlite-query-interface.ts @@ -146,4 +146,8 @@ export default class SqliteQueryInterface extends QueryInterface { WHERE name = '${tableName}';`; await this.db.sequelize.query(sql, { transaction }); } + + public generateJoinOnForJSONArray(left: string, right: string) { + return this.db.sequelize.literal(`${left} in (SELECT value from json_each(${right}))`); + } } diff --git a/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/issues.test.ts b/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/issues.test.ts index 536d72d1a5..94b2f559b3 100644 --- a/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/issues.test.ts +++ b/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/issues.test.ts @@ -324,4 +324,100 @@ describe('issues', () => { } expect(res.status).toBe(200); }); + + test('filtering by fields of a relation collection with m2m array field', async () => { + await db.getRepository('collections').create({ + values: { + name: 'tags', + fields: [ + { + name: 'id', + type: 'bigInt', + autoIncrement: true, + primaryKey: true, + allowNull: false, + }, + { + name: 'title', + type: 'string', + }, + ], + }, + }); + await db.getRepository('collections').create({ + values: { + name: 'users', + fields: [ + { + name: 'id', + type: 'bigInt', + autoIncrement: true, + primaryKey: true, + allowNull: false, + }, + { + name: 'username', + type: 'string', + }, + { + name: 'tags', + type: 'belongsToArray', + foreignKey: 'tag_ids', + target: 'tags', + targetKey: 'id', + }, + ], + }, + }); + await db.getRepository('collections').create({ + values: { + name: 'projects', + fields: [ + { + name: 'id', + type: 'bigInt', + autoIncrement: true, + primaryKey: true, + allowNull: false, + }, + { + name: 'title', + type: 'string', + }, + { + name: 'users', + type: 'belongsTo', + foreignKey: 'user_id', + target: 'users', + }, + ], + }, + }); + // @ts-ignore + await db.getRepository('collections').load(); + await db.sync(); + await db.getRepository('tags').create({ + values: [{ title: 'a' }, { title: 'b' }, { title: 'c' }], + }); + await db.getRepository('users').create({ + values: { id: 1, username: 'a' }, + }); + await db.getRepository('projects').create({ + values: { id: 1, title: 'p1', user_id: 1 }, + }); + await expect( + db.getRepository('projects').findOne({ + appends: ['users', 'users.tags'], + filter: { + $and: [ + { + users: { + username: 'a', + }, + }, + ], + }, + }), + ).resolves.toBeTruthy(); + }); }); diff --git a/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/m2m-array-bigint-api.test.ts b/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/m2m-array-bigint-api.test.ts index e20b709e05..778c196582 100644 --- a/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/m2m-array-bigint-api.test.ts +++ b/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/m2m-array-bigint-api.test.ts @@ -207,15 +207,8 @@ describe('m2m array api, bigInt targetKey', () => { }, }, }); - if (db.sequelize.getDialect() === 'postgres') { - const res = await search; - expect(res.length).toBe(1); - } else { - expect(search).rejects.toThrowError(); - } - if (db.sequelize.getDialect() !== 'postgres') { - return; - } + const res1 = await search; + expect(res1.length).toBe(1); const search2 = db.getRepository('users').find({ filter: { 'tags.title': { @@ -223,12 +216,8 @@ describe('m2m array api, bigInt targetKey', () => { }, }, }); - if (db.sequelize.getDialect() === 'postgres') { - const res = await search2; - expect(res.length).toBe(2); - } else { - expect(search2).rejects.toThrowError(); - } + const res2 = await search2; + expect(res2.length).toBe(2); }); it('should create with belongsToArray', async () => { diff --git a/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/m2m-array-string-api.test.ts b/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/m2m-array-string-api.test.ts index 3d1f1d8265..2c0cf26e4b 100644 --- a/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/m2m-array-string-api.test.ts +++ b/packages/plugins/@nocobase/plugin-field-m2m-array/src/server/__tests__/m2m-array-string-api.test.ts @@ -186,15 +186,8 @@ describe('m2m array api, string targetKey', () => { }, }, }); - if (db.sequelize.getDialect() === 'postgres') { - const res = await search; - expect(res.length).toBe(1); - } else { - expect(search).rejects.toThrowError(); - } - if (db.sequelize.getDialect() !== 'postgres') { - return; - } + const res1 = await search; + expect(res1.length).toBe(1); const search2 = db.getRepository('users').find({ filter: { 'tags.title': { @@ -202,12 +195,8 @@ describe('m2m array api, string targetKey', () => { }, }, }); - if (db.sequelize.getDialect() === 'postgres') { - const res = await search2; - expect(res.length).toBe(2); - } else { - expect(search2).rejects.toThrowError(); - } + const res2 = await search2; + expect(res2.length).toBe(2); }); it('should create with belongsToArray', async () => { From 69fac1120446ac554d4b94bb2f1923b0bdbf527f Mon Sep 17 00:00:00 2001 From: Katherine Date: Tue, 1 Apr 2025 11:38:15 +0800 Subject: [PATCH 03/10] fix: oneToMa ny e2e test (#6601) --- .../antd/association-field/AssociationSelect.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/client/src/schema-component/antd/association-field/AssociationSelect.tsx b/packages/core/client/src/schema-component/antd/association-field/AssociationSelect.tsx index 7858174d37..ed9969a523 100644 --- a/packages/core/client/src/schema-component/antd/association-field/AssociationSelect.tsx +++ b/packages/core/client/src/schema-component/antd/association-field/AssociationSelect.tsx @@ -33,7 +33,7 @@ import { RemoteSelect, RemoteSelectProps } from '../remote-select'; import useServiceOptions, { useAssociationFieldContext } from './hooks'; const removeIfKeyEmpty = (obj, filterTargetKey) => { - if (!obj || typeof obj !== 'object' || !filterTargetKey) return obj; + if (!obj || typeof obj !== 'object' || !filterTargetKey || Array.isArray(obj)) return obj; return !obj[filterTargetKey] ? null : obj; }; From 4c3255d455bfce66b398da4b413f17a13b2cae89 Mon Sep 17 00:00:00 2001 From: Junyi Date: Tue, 1 Apr 2025 13:25:08 +0800 Subject: [PATCH 04/10] feat(database): add trim option for string field (#6565) * feat(database): add trim option for string field * refactor(database): change to setDataValue instead of hooks * fix(database): fix test case of view collection --- .../src/collection-manager/interfaces/input.ts | 6 ++++++ packages/core/client/src/locale/zh-CN.json | 1 + .../src/__tests__/fields/string-field.test.ts | 14 ++++++++++++++ packages/core/database/src/fields/string-field.ts | 13 ++++++++++++- 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/core/client/src/collection-manager/interfaces/input.ts b/packages/core/client/src/collection-manager/interfaces/input.ts index ac5897adb7..40d131e5be 100644 --- a/packages/core/client/src/collection-manager/interfaces/input.ts +++ b/packages/core/client/src/collection-manager/interfaces/input.ts @@ -62,6 +62,12 @@ export class InputFieldInterface extends CollectionFieldInterface { hasDefaultValue = true; properties = { ...defaultProps, + trim: { + type: 'boolean', + 'x-content': '{{t("Automatically remove heading and tailing spaces")}}', + 'x-decorator': 'FormItem', + 'x-component': 'Checkbox', + }, layout: { type: 'void', title: '{{t("Index")}}', diff --git a/packages/core/client/src/locale/zh-CN.json b/packages/core/client/src/locale/zh-CN.json index ee7937d066..6c245aecf1 100644 --- a/packages/core/client/src/locale/zh-CN.json +++ b/packages/core/client/src/locale/zh-CN.json @@ -258,6 +258,7 @@ "Parent collection fields": "父表字段", "Basic": "基本类型", "Single line text": "单行文本", + "Automatically remove heading and tailing spaces": "自动去除首尾空白字符", "Long text": "多行文本", "Phone": "手机号码", "Email": "电子邮箱", diff --git a/packages/core/database/src/__tests__/fields/string-field.test.ts b/packages/core/database/src/__tests__/fields/string-field.test.ts index ee4f3e631d..ff095bc862 100644 --- a/packages/core/database/src/__tests__/fields/string-field.test.ts +++ b/packages/core/database/src/__tests__/fields/string-field.test.ts @@ -105,4 +105,18 @@ describe('string field', () => { name2: 'n2111', }); }); + + it('trim', async () => { + const collection = db.collection({ + name: 'tests', + fields: [{ type: 'string', name: 'name', trim: true }], + }); + await db.sync(); + const model = await collection.model.create({ + name: ' n1\n ', + }); + expect(model.toJSON()).toMatchObject({ + name: 'n1', + }); + }); }); diff --git a/packages/core/database/src/fields/string-field.ts b/packages/core/database/src/fields/string-field.ts index 50431f0565..627768beeb 100644 --- a/packages/core/database/src/fields/string-field.ts +++ b/packages/core/database/src/fields/string-field.ts @@ -8,7 +8,7 @@ */ import { DataTypes } from 'sequelize'; -import { BaseColumnFieldOptions, Field } from './field'; +import { BaseColumnFieldOptions, Field, FieldContext } from './field'; export class StringField extends Field { get dataType() { @@ -18,9 +18,20 @@ export class StringField extends Field { return DataTypes.STRING; } + + additionalSequelizeOptions() { + const { name, trim } = this.options; + + return { + set(value) { + this.setDataValue(name, trim ? value?.trim() : value); + }, + }; + } } export interface StringFieldOptions extends BaseColumnFieldOptions { type: 'string'; length?: number; + trim?: boolean; } From bd87d74ce1c81c0dea7b0cf1a51a4441171e78b7 Mon Sep 17 00:00:00 2001 From: aaaaaajie Date: Tue, 1 Apr 2025 17:38:06 +0800 Subject: [PATCH 05/10] refactor: enhance role merging logic: prioritize strategy actions over existing actions --- packages/core/acl/src/utils/acl-role.ts | 62 ++++++++ .../src/server/__tests__/union-role.test.ts | 143 ++++++++++++++++++ 2 files changed, 205 insertions(+) diff --git a/packages/core/acl/src/utils/acl-role.ts b/packages/core/acl/src/utils/acl-role.ts index 406ce36dec..5d3515f1ef 100644 --- a/packages/core/acl/src/utils/acl-role.ts +++ b/packages/core/acl/src/utils/acl-role.ts @@ -30,9 +30,71 @@ export function mergeRole(roles: ACLRole[]) { } } result.snippets = mergeRoleSnippets(allSnippets); + adjustActionByStrategy(roles, result); return result; } +/** + * When merging permissions from multiple roles, if strategy.actions allows certain actions, then those actions have higher priority. + * For example, [ + * { + * actions: { + * 'users:view': {...}, + * 'users:create': {...} + * }, + * strategy: { + * actions: ['view'] + * } + * }] + * finally result: [{ + * actions: { + * 'users:create': {...}, + * }, + * { + * strategy: { + * actions: ['view'] + * }] + **/ +function adjustActionByStrategy( + roles, + result: { actions?: Record; strategy?: { actions?: string[] } }, +) { + const { actions, strategy } = result; + if (_.isEmpty(actions) || _.isEmpty(strategy?.actions)) { + return; + } + const adjustActions = calcAdjustActions(roles); + if (!adjustActions.length) { + return; + } + for (const key of Object.keys(actions)) { + const needRemove = adjustActions.includes(key.split(':')[1]); + if (needRemove) { + delete actions[key]; + } + } +} + +function calcAdjustActions(roles) { + const adjustActionSet = new Set(); + for (const role of roles) { + const r = role.toJSON(); + if (_.isEmpty(r.actions) && r.strategy?.actions?.length) { + r.strategy.actions.forEach((x) => adjustActionSet.add(x)); + continue; + } + if (!_.isEmpty(r.actions) && r.strategy?.actions?.length) { + for (const action of r.strategy.actions) { + const exist = Object.keys(r.actions).some((key) => key.split(':')[1] === action); + if (!exist) { + adjustActionSet.add(action); + } + } + } + } + return [...adjustActionSet]; +} + function mergeRoleNames(sourceRoleNames, newRoleName) { return newRoleName ? sourceRoleNames.concat(newRoleName) : sourceRoleNames; } diff --git a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/union-role.test.ts b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/union-role.test.ts index 53758c0d25..162106b017 100644 --- a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/union-role.test.ts +++ b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/union-role.test.ts @@ -530,4 +530,147 @@ describe('union role: full permissions', async () => { expect(createRoleResponse.statusCode).toBe(200); expect(createRoleResponse.body.data.role).not.toBe(UNION_ROLE_KEY); }); + + it('should general action permissions override specific resource permissions when using union role #1924', async () => { + const rootAgent = await app.agent().login(rootUser); + await rootAgent + .post(`/dataSources/main/roles:update`) + .query({ + filterByTk: role1.name, + }) + .send({ + roleName: role1.name, + strategy: { + actions: ['view'], + }, + dataSourceKey: 'main', + }); + + const ownDataSourceScopeRole = await db.getRepository('dataSourcesRolesResourcesScopes').findOne({ + where: { + key: 'own', + dataSourceKey: 'main', + }, + }); + const scopeFields = ['id', 'createdBy', 'createdById']; + const dataSourceResourcesResponse = await rootAgent + .post(`/roles/${role2.name}/dataSourceResources:create`) + .query({ + filterByTk: 'users', + filter: { + dataSourceKey: 'main', + name: 'users', + }, + }) + .send({ + usingActionsConfig: true, + actions: [ + { + name: 'view', + fields: scopeFields, + scope: { + id: ownDataSourceScopeRole.id, + createdAt: '2025-02-19T08:57:17.385Z', + updatedAt: '2025-02-19T08:57:17.385Z', + key: 'own', + dataSourceKey: 'main', + name: '{{t("Own records")}}', + resourceName: null, + scope: { + createdById: '{{ ctx.state.currentUser.id }}', + }, + }, + }, + ], + name: 'users', + dataSourceKey: 'main', + }); + expect(dataSourceResourcesResponse.statusCode).toBe(200); + + agent = await app.agent().login(user, UNION_ROLE_KEY); + const rolesResponse = await agent.resource('roles').check(); + expect(rolesResponse.status).toBe(200); + expect(rolesResponse.body.data.actions).toStrictEqual({}); + }); + + it('should verify actions configuration for union role with specific scopes', async () => { + const rootAgent = await app.agent().login(rootUser); + await rootAgent + .post(`/dataSources/main/roles:update`) + .query({ + filterByTk: role1.name, + }) + .send({ + roleName: role1.name, + strategy: { + actions: ['view', 'create:own1'], + }, + dataSourceKey: 'main', + }); + + const ownDataSourceScopeRole = await db.getRepository('dataSourcesRolesResourcesScopes').findOne({ + where: { + key: 'own', + dataSourceKey: 'main', + }, + }); + const scopeFields = ['id', 'createdBy', 'createdById']; + const dataSourceResourcesResponse = await rootAgent + .post(`/roles/${role2.name}/dataSourceResources:create`) + .query({ + filterByTk: 'users', + filter: { + dataSourceKey: 'main', + name: 'users', + }, + }) + .send({ + usingActionsConfig: true, + actions: [ + { + name: 'view', + fields: scopeFields, + scope: { + id: ownDataSourceScopeRole.id, + createdAt: '2025-02-19T08:57:17.385Z', + updatedAt: '2025-02-19T08:57:17.385Z', + key: 'own', + dataSourceKey: 'main', + name: '{{t("Own records")}}', + resourceName: null, + scope: { + createdById: '{{ ctx.state.currentUser.id }}', + }, + }, + }, + { + name: 'create', + fields: scopeFields, + scope: { + id: ownDataSourceScopeRole.id, + createdAt: '2025-02-19T08:57:17.385Z', + updatedAt: '2025-02-19T08:57:17.385Z', + key: 'own', + dataSourceKey: 'main', + name: '{{t("Own records")}}', + resourceName: null, + scope: { + createdById: '{{ ctx.state.currentUser.id }}', + }, + }, + }, + ], + name: 'users', + dataSourceKey: 'main', + }); + expect(dataSourceResourcesResponse.statusCode).toBe(200); + + agent = await app.agent().login(user, UNION_ROLE_KEY); + const rolesResponse = await agent.resource('roles').check(); + expect(rolesResponse.status).toBe(200); + expect(rolesResponse.body.data.actions).toHaveProperty('users:create'); + expect(rolesResponse.body.data.actions).not.toHaveProperty('users:view'); + expect(rolesResponse.body.data.actions['users:create']).toHaveProperty('filter'); + expect(rolesResponse.body.data.actions['users:create']).toHaveProperty('whitelist'); + }); }); From 9ef9dd4c15d5b57e59c6e622c63ee50366e288d5 Mon Sep 17 00:00:00 2001 From: aaaaaajie Date: Tue, 1 Apr 2025 20:36:53 +0800 Subject: [PATCH 06/10] refactor: union role acl --- packages/core/acl/src/utils/acl-role.ts | 45 +++++-------------- .../src/server/__tests__/union-role.test.ts | 8 ++-- 2 files changed, 15 insertions(+), 38 deletions(-) diff --git a/packages/core/acl/src/utils/acl-role.ts b/packages/core/acl/src/utils/acl-role.ts index 5d3515f1ef..4fe526577b 100644 --- a/packages/core/acl/src/utils/acl-role.ts +++ b/packages/core/acl/src/utils/acl-role.ts @@ -30,7 +30,7 @@ export function mergeRole(roles: ACLRole[]) { } } result.snippets = mergeRoleSnippets(allSnippets); - adjustActionByStrategy(roles, result); + adjustActionByStrategy(result); return result; } @@ -55,44 +55,19 @@ export function mergeRole(roles: ACLRole[]) { * actions: ['view'] * }] **/ -function adjustActionByStrategy( - roles, - result: { actions?: Record; strategy?: { actions?: string[] } }, -) { +function adjustActionByStrategy(result: { + actions?: Record; + strategy?: { actions?: string[] }; + resources?: string[]; +}) { const { actions, strategy } = result; - if (_.isEmpty(actions) || _.isEmpty(strategy?.actions)) { - return; - } - const adjustActions = calcAdjustActions(roles); - if (!adjustActions.length) { - return; - } - for (const key of Object.keys(actions)) { - const needRemove = adjustActions.includes(key.split(':')[1]); - if (needRemove) { - delete actions[key]; - } - } -} - -function calcAdjustActions(roles) { - const adjustActionSet = new Set(); - for (const role of roles) { - const r = role.toJSON(); - if (_.isEmpty(r.actions) && r.strategy?.actions?.length) { - r.strategy.actions.forEach((x) => adjustActionSet.add(x)); - continue; - } - if (!_.isEmpty(r.actions) && r.strategy?.actions?.length) { - for (const action of r.strategy.actions) { - const exist = Object.keys(r.actions).some((key) => key.split(':')[1] === action); - if (!exist) { - adjustActionSet.add(action); - } + if (!_.isEmpty(actions) && !_.isEmpty(strategy?.actions) && !_.isEmpty(result.resources)) { + for (const resource of result.resources) { + for (const action of strategy.actions) { + actions[`${resource}:${action}`] = {}; } } } - return [...adjustActionSet]; } function mergeRoleNames(sourceRoleNames, newRoleName) { diff --git a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/union-role.test.ts b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/union-role.test.ts index 162106b017..7095e672bb 100644 --- a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/union-role.test.ts +++ b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/union-role.test.ts @@ -590,7 +590,7 @@ describe('union role: full permissions', async () => { agent = await app.agent().login(user, UNION_ROLE_KEY); const rolesResponse = await agent.resource('roles').check(); expect(rolesResponse.status).toBe(200); - expect(rolesResponse.body.data.actions).toStrictEqual({}); + expect(rolesResponse.body.data.actions['users:view']).toStrictEqual({}); }); it('should verify actions configuration for union role with specific scopes', async () => { @@ -603,7 +603,7 @@ describe('union role: full permissions', async () => { .send({ roleName: role1.name, strategy: { - actions: ['view', 'create:own1'], + actions: ['view', 'create:own', 'update'], }, dataSourceKey: 'main', }); @@ -669,8 +669,10 @@ describe('union role: full permissions', async () => { const rolesResponse = await agent.resource('roles').check(); expect(rolesResponse.status).toBe(200); expect(rolesResponse.body.data.actions).toHaveProperty('users:create'); - expect(rolesResponse.body.data.actions).not.toHaveProperty('users:view'); + expect(rolesResponse.body.data.actions).toHaveProperty('users:view'); + expect(rolesResponse.body.data.actions['users:view']).toStrictEqual({}); expect(rolesResponse.body.data.actions['users:create']).toHaveProperty('filter'); expect(rolesResponse.body.data.actions['users:create']).toHaveProperty('whitelist'); + expect(rolesResponse.body.data.actions['users:update']).toStrictEqual({}); }); }); From 18b8f9bc0ef5080cdcbbb42c2490c710ebd0d5c2 Mon Sep 17 00:00:00 2001 From: aaaaaajie Date: Tue, 1 Apr 2025 21:13:28 +0800 Subject: [PATCH 07/10] refactor: union role acl --- packages/core/acl/src/utils/acl-role.ts | 32 +++++++++++++++++++------ 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/packages/core/acl/src/utils/acl-role.ts b/packages/core/acl/src/utils/acl-role.ts index 4fe526577b..b9d807268f 100644 --- a/packages/core/acl/src/utils/acl-role.ts +++ b/packages/core/acl/src/utils/acl-role.ts @@ -30,7 +30,7 @@ export function mergeRole(roles: ACLRole[]) { } } result.snippets = mergeRoleSnippets(allSnippets); - adjustActionByStrategy(result); + adjustActionByStrategy(roles, result); return result; } @@ -55,21 +55,39 @@ export function mergeRole(roles: ACLRole[]) { * actions: ['view'] * }] **/ -function adjustActionByStrategy(result: { - actions?: Record; - strategy?: { actions?: string[] }; - resources?: string[]; -}) { +function adjustActionByStrategy( + roles, + result: { + actions?: Record; + strategy?: { actions?: string[] }; + resources?: string[]; + }, +) { const { actions, strategy } = result; + const actionSet = getAdjustActions(roles); if (!_.isEmpty(actions) && !_.isEmpty(strategy?.actions) && !_.isEmpty(result.resources)) { for (const resource of result.resources) { for (const action of strategy.actions) { - actions[`${resource}:${action}`] = {}; + if (actionSet.has(action)) { + actions[`${resource}:${action}`] = {}; + } } } } } +function getAdjustActions(roles: ACLRole[]) { + const actionSet = new Set(); + for (const role of roles) { + const jsonRole = role.toJSON(); + // Within the same role, actions have higher priority than strategy.actions. + if (!_.isEmpty(jsonRole.strategy?.['actions']) && _.isEmpty(jsonRole.actions)) { + jsonRole.strategy['actions'].forEach((x) => actionSet.add(x)); + } + } + return actionSet; +} + function mergeRoleNames(sourceRoleNames, newRoleName) { return newRoleName ? sourceRoleNames.concat(newRoleName) : sourceRoleNames; } From fcfeffe5041e1e8439051a26a8b2179c07ad19e6 Mon Sep 17 00:00:00 2001 From: aaaaaajie Date: Tue, 1 Apr 2025 23:49:01 +0800 Subject: [PATCH 08/10] refactor: union role acl --- packages/core/acl/src/utils/acl-role.ts | 2 +- .../plugin-acl/src/server/__tests__/union-role.test.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/acl/src/utils/acl-role.ts b/packages/core/acl/src/utils/acl-role.ts index b9d807268f..fac818d616 100644 --- a/packages/core/acl/src/utils/acl-role.ts +++ b/packages/core/acl/src/utils/acl-role.ts @@ -82,7 +82,7 @@ function getAdjustActions(roles: ACLRole[]) { const jsonRole = role.toJSON(); // Within the same role, actions have higher priority than strategy.actions. if (!_.isEmpty(jsonRole.strategy?.['actions']) && _.isEmpty(jsonRole.actions)) { - jsonRole.strategy['actions'].forEach((x) => actionSet.add(x)); + jsonRole.strategy['actions'].forEach((x) => !x.includes('own') && actionSet.add(x)); } } return actionSet; diff --git a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/union-role.test.ts b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/union-role.test.ts index 7095e672bb..33a19f9cf5 100644 --- a/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/union-role.test.ts +++ b/packages/plugins/@nocobase/plugin-acl/src/server/__tests__/union-role.test.ts @@ -671,6 +671,7 @@ describe('union role: full permissions', async () => { expect(rolesResponse.body.data.actions).toHaveProperty('users:create'); expect(rolesResponse.body.data.actions).toHaveProperty('users:view'); expect(rolesResponse.body.data.actions['users:view']).toStrictEqual({}); + expect(rolesResponse.body.data.actions).not.toHaveProperty('users:create:own'); expect(rolesResponse.body.data.actions['users:create']).toHaveProperty('filter'); expect(rolesResponse.body.data.actions['users:create']).toHaveProperty('whitelist'); expect(rolesResponse.body.data.actions['users:update']).toStrictEqual({}); From 5046d68c8e116ffd07569d2fbfbfb3cd642fcd25 Mon Sep 17 00:00:00 2001 From: chenos Date: Wed, 2 Apr 2025 12:48:00 +0800 Subject: [PATCH 09/10] fix: assign getKeys --- .../core/utils/src/__tests__/assign.test.ts | 28 +++++++++++++++++++ packages/core/utils/src/assign.ts | 6 ++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/packages/core/utils/src/__tests__/assign.test.ts b/packages/core/utils/src/__tests__/assign.test.ts index b40c90150b..7fa57b6528 100644 --- a/packages/core/utils/src/__tests__/assign.test.ts +++ b/packages/core/utils/src/__tests__/assign.test.ts @@ -315,6 +315,34 @@ describe('merge strategy', () => { key1: 'val1 + val2', }); }); + it('case 2', () => { + const obj = assign( + { + filter: { a: 'a2' }, + }, + {}, + { + filter: () => '123', + }, + ); + expect(obj).toMatchObject({ + filter: '123', + }); + }); + it('case 3', () => { + const obj = assign( + {}, + { + filter: { a: 'a2' }, + }, + { + filter: () => '123', + }, + ); + expect(obj).toMatchObject({ + filter: '123', + }); + }); }); describe('merge', () => { diff --git a/packages/core/utils/src/assign.ts b/packages/core/utils/src/assign.ts index 306da77cba..932cbf5e3c 100644 --- a/packages/core/utils/src/assign.ts +++ b/packages/core/utils/src/assign.ts @@ -8,7 +8,7 @@ */ import deepmerge from 'deepmerge'; -import lodash from 'lodash'; +import _ from 'lodash'; import { isPlainObject } from './common'; type MergeStrategyType = 'merge' | 'deepMerge' | 'overwrite' | 'andMerge' | 'orMerge' | 'intersect' | 'union'; @@ -88,7 +88,7 @@ mergeStrategies.set('union', (x, y) => { if (typeof y === 'string') { y = y.split(','); } - return lodash.uniq((x || []).concat(y || [])).filter(Boolean); + return _.uniq((x || []).concat(y || [])).filter(Boolean); }); mergeStrategies.set('intersect', (x, y) => @@ -110,7 +110,7 @@ mergeStrategies.set('intersect', (x, y) => ); export function assign(target: any, source: any, strategies: MergeStrategies = {}) { - getKeys(source).forEach((sourceKey) => { + _.uniq([...getKeys(source), ...getKeys(target)]).forEach((sourceKey) => { const strategy = strategies[sourceKey]; let func = mergeStrategies.get('deepMerge'); if (typeof strategy === 'function') { From ab95c13a73299f8e59665a5ed1614d4272b41f02 Mon Sep 17 00:00:00 2001 From: chenos Date: Wed, 2 Apr 2025 14:46:41 +0800 Subject: [PATCH 10/10] fix: assign --- .../core/utils/src/__tests__/assign.test.ts | 28 +++++++++++++++++++ packages/core/utils/src/assign.ts | 13 +++++++-- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/packages/core/utils/src/__tests__/assign.test.ts b/packages/core/utils/src/__tests__/assign.test.ts index 7fa57b6528..49b2333656 100644 --- a/packages/core/utils/src/__tests__/assign.test.ts +++ b/packages/core/utils/src/__tests__/assign.test.ts @@ -444,4 +444,32 @@ describe('merge strategy', () => { }); }); }); + + describe('source is empty', () => { + it('case 1', () => { + const obj = assign( + { + resourceName: 'uiSchemas', + resourceIndex: 'n0jylid5rqa', + actionName: 'getJsonSchema', + values: {}, + }, + {}, + { + filter: 'andMerge', + fields: 'intersect', + except: 'union', + whitelist: 'intersect', + blacklist: 'intersect', + sort: 'overwrite', + }, + ); + expect(obj).toMatchObject({ + resourceName: 'uiSchemas', + resourceIndex: 'n0jylid5rqa', + actionName: 'getJsonSchema', + values: {}, + }); + }); + }); }); diff --git a/packages/core/utils/src/assign.ts b/packages/core/utils/src/assign.ts index 932cbf5e3c..44a368a0ff 100644 --- a/packages/core/utils/src/assign.ts +++ b/packages/core/utils/src/assign.ts @@ -110,15 +110,22 @@ mergeStrategies.set('intersect', (x, y) => ); export function assign(target: any, source: any, strategies: MergeStrategies = {}) { - _.uniq([...getKeys(source), ...getKeys(target)]).forEach((sourceKey) => { + const sourceKeys = getKeys(source); + const targetKeys = getKeys(target); + _.uniq([...sourceKeys, ...targetKeys]).forEach((sourceKey) => { const strategy = strategies[sourceKey]; - let func = mergeStrategies.get('deepMerge'); + let func: any; if (typeof strategy === 'function') { func = strategy; } else if (typeof strategy === 'string' && mergeStrategies.has(strategy as any)) { func = mergeStrategies.get(strategy as any); } - target[sourceKey] = func(target[sourceKey], source[sourceKey]); + if (func) { + target[sourceKey] = func(target[sourceKey], source[sourceKey]); + } else if (sourceKeys.includes(sourceKey)) { + const func = mergeStrategies.get('deepMerge'); + target[sourceKey] = func(target[sourceKey], source[sourceKey]); + } }); return target; }