From c7300d1100605e9b577d0fe7cf857f953941ba0c Mon Sep 17 00:00:00 2001 From: Junyi Date: Thu, 20 Oct 2022 15:45:00 +0800 Subject: [PATCH] fix(plugin-workflow): avoid revision with ghost nodes (#941) --- .../src/server/__tests__/Plugin.test.ts | 38 +++++++++++++++++++ .../workflow/src/server/actions/workflows.ts | 23 ++++++++--- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/packages/plugins/workflow/src/server/__tests__/Plugin.test.ts b/packages/plugins/workflow/src/server/__tests__/Plugin.test.ts index cc4d82fd3..590490ecc 100644 --- a/packages/plugins/workflow/src/server/__tests__/Plugin.test.ts +++ b/packages/plugins/workflow/src/server/__tests__/Plugin.test.ts @@ -285,5 +285,43 @@ describe('workflow > Plugin', () => { const [echo, calculation] = await execution.getJobs({ order: [['id', 'ASC']] }); expect(calculation.result).toBe(2); }); + + it('revision with using of deleted nodes', async () => { + const w1 = await WorkflowModel.create({ + enabled: true, + type: 'collection', + config: { + mode: 1, + collection: 'posts' + } + }); + + const n2 = await w1.createNode({ + type: 'calculation', + config: { + calculation: { + calculator: 'add', + operands: [ + { + type: '$jobsMapByNodeId', + options: { + nodeId: 0, + path: 'data.read' + } + }, + { + value: `{{$jobsMapByNodeId.0.data.read}}` + } + ] + } + }, + }); + + const { status } = await agent.resource(`workflows`).revision({ + filterByTk: w1.id + }); + + expect(status).toBe(400); + }); }); }); diff --git a/packages/plugins/workflow/src/server/actions/workflows.ts b/packages/plugins/workflow/src/server/actions/workflows.ts index cf17cd236..447f73df4 100644 --- a/packages/plugins/workflow/src/server/actions/workflows.ts +++ b/packages/plugins/workflow/src/server/actions/workflows.ts @@ -31,11 +31,16 @@ function migrateConfig(config, oldToNew) { case 'array': return value.map(item => migrate(item)); case 'string': - return value - .replace( - /(\{\{\$jobsMapByNodeId\.)([\w-]+)/, - (_, prefix, id) => `${prefix}${oldToNew.get(Number.parseInt(id, 10)).id}` - ); + const matcher = value.match(/(\{\{\$jobsMapByNodeId\.)([\w-]+)/); + if (!matcher) { + return value; + } + const oldNodeId = Number.parseInt(matcher[2], 10); + const newNode = oldToNew.get(oldNodeId); + if (!newNode) { + throw new Error('node configurated for result is not existed'); + } + return value.replace(matcher[0], `{{$jobsMapByNodeId.${newNode.id}`); default: return value; } @@ -91,11 +96,17 @@ export async function revision(context: Context, next) { const oldNode = originalNodesMap.get(oldId); const newUpstream = oldNode.upstreamId ? oldToNew.get(oldNode.upstreamId) : null; const newDownstream = oldNode.downstreamId ? oldToNew.get(oldNode.downstreamId) : null; + let migratedConfig; + try { + migratedConfig = migrateConfig(oldNode.config, oldToNew); + } catch (err) { + return context.throw(400, err.message); + } await newNode.update({ upstreamId: newUpstream?.id ?? null, downstreamId: newDownstream?.id ?? null, - config: migrateConfig(oldNode.config, oldToNew) + config: migratedConfig }, { transaction }); }