From 41df24e2dc9cf526d9d0f71fef1bce98506cd82b Mon Sep 17 00:00:00 2001 From: "Carlos R. L. Rodrigues" <37986729+carlos-r-l-rodrigues@users.noreply.github.com> Date: Tue, 4 Jun 2024 07:18:40 -0300 Subject: [PATCH] chore(workflows-sdk): check exported workflow (#7592) --- .../src/__tests__/workflow/local-workflow.ts | 40 ++------------- .../src/workflow/workflow-manager.ts | 12 +++-- .../src/helper/__tests__/compose.ts | 50 +++++++++++++++++++ .../core/workflows-sdk/src/medusa-workflow.ts | 2 +- .../src/utils/composer/create-workflow.ts | 16 +++--- 5 files changed, 71 insertions(+), 49 deletions(-) diff --git a/packages/core/orchestration/src/__tests__/workflow/local-workflow.ts b/packages/core/orchestration/src/__tests__/workflow/local-workflow.ts index 7a0810e883..2bfbeaeecc 100644 --- a/packages/core/orchestration/src/__tests__/workflow/local-workflow.ts +++ b/packages/core/orchestration/src/__tests__/workflow/local-workflow.ts @@ -80,9 +80,8 @@ describe("WorkflowManager", () => { expect(wf).toEqual(["create-product", "broken-delivery", "deliver-product"]) }) - it("should NOT throw when registering a workflow with an existing id in Medusa V1", () => { - let err - try { + it("should throw when registering a workflow with an existing id and different definition", () => { + const exec = () => WorkflowManager.register( "create-product", { @@ -96,38 +95,8 @@ describe("WorkflowManager", () => { }, handlers ) - } catch (e) { - err = e - } - expect(err).toBeUndefined() - }) - - it("should throw when registering a workflow with an existing id in Medusa V2", () => { - let err - const env = process.env.MEDUSA_FF_MEDUSA_V2 - process.env.MEDUSA_FF_MEDUSA_V2 = "true" - try { - WorkflowManager.register( - "create-product", - { - action: "foo", - next: { - action: "bar", - next: { - action: "xor", - }, - }, - }, - handlers - ) - } catch (e) { - err = e - } - process.env.MEDUSA_FF_MEDUSA_V2 = env - - expect(err).toBeDefined() - expect(err.message).toBe( + expect(exec).toThrowError( `Workflow with id "create-product" and step definition already exists.` ) }) @@ -135,8 +104,6 @@ describe("WorkflowManager", () => { it("should not throw when registering a workflow with an existing id but identical definition", () => { let err - const env = process.env.MEDUSA_FF_MEDUSA_V2 - process.env.MEDUSA_FF_MEDUSA_V2 = "true" try { WorkflowManager.register( "create-product", @@ -151,7 +118,6 @@ describe("WorkflowManager", () => { } catch (e) { err = e } - process.env.MEDUSA_FF_MEDUSA_V2 = env expect(err).not.toBeDefined() }) diff --git a/packages/core/orchestration/src/workflow/workflow-manager.ts b/packages/core/orchestration/src/workflow/workflow-manager.ts index 5b5e103022..69af1b1bad 100644 --- a/packages/core/orchestration/src/workflow/workflow-manager.ts +++ b/packages/core/orchestration/src/workflow/workflow-manager.ts @@ -77,6 +77,10 @@ export class WorkflowManager { return new OrchestratorBuilder(workflow.flow_) } + static getEmptyTransactionDefinition(): OrchestratorBuilder { + return new OrchestratorBuilder() + } + static register( workflowId: string, flow: TransactionStepsDefinition | OrchestratorBuilder | undefined, @@ -101,11 +105,9 @@ export class WorkflowManager { : true if (!areStepsEqual) { - if (process.env.MEDUSA_FF_MEDUSA_V2 == "true") { - throw new Error( - `Workflow with id "${workflowId}" and step definition already exists.` - ) - } + throw new Error( + `Workflow with id "${workflowId}" and step definition already exists.` + ) } } diff --git a/packages/core/workflows-sdk/src/helper/__tests__/compose.ts b/packages/core/workflows-sdk/src/helper/__tests__/compose.ts index 4aa510e8fa..a5f2e49baf 100644 --- a/packages/core/workflows-sdk/src/helper/__tests__/compose.ts +++ b/packages/core/workflows-sdk/src/helper/__tests__/compose.ts @@ -1,3 +1,4 @@ +import { WorkflowManager } from "@medusajs/orchestration" import { promiseAll } from "@medusajs/utils" import { createStep, @@ -14,6 +15,7 @@ jest.setTimeout(30000) const afterEach_ = () => { jest.clearAllMocks() MedusaWorkflow.workflows = {} + WorkflowManager.unregisterAll() } describe("Workflow composer", function () { @@ -1972,4 +1974,52 @@ describe("Workflow composer", function () { expect(mockStep1Fn.mock.calls[0][0]).toEqual(workflowInput) expect(mockCompensateSte1.mock.calls[0][0]).toEqual({ data: "data" }) }) + + it("should throw if the same workflow is defined using different steps", async () => { + const step1 = createStep("step1", () => { + return new StepResponse({ + obj: { + nested: "nested", + }, + }) + }) + + const step2 = createStep("step2", () => { + return new StepResponse({ + obj: "returned from 2**", + }) + }) + + const step3 = createStep("step3", () => { + return new StepResponse({ + obj: "returned from 3**", + }) + }) + + createWorkflow("workflow1", function () { + const { obj } = step1() + const s2 = step2() + + return [{ step1_nested_obj: obj.nested }, s2] + }) + + expect(() => + createWorkflow("workflow1", function () { + const { obj } = step1() + const s2 = step2() + step3() + + return [{ step1_nested_obj: obj.nested }, s2] + }) + ).toThrowError( + `Workflow with id "workflow1" and step definition already exists.` + ) + + createWorkflow("workflow1", function () { + const { obj } = step1() + const s2 = step2() + + return [{ step1_nested_obj: obj.nested }, s2] + }) + }) }) diff --git a/packages/core/workflows-sdk/src/medusa-workflow.ts b/packages/core/workflows-sdk/src/medusa-workflow.ts index baea7536c7..a739779dda 100644 --- a/packages/core/workflows-sdk/src/medusa-workflow.ts +++ b/packages/core/workflows-sdk/src/medusa-workflow.ts @@ -16,7 +16,7 @@ export class MedusaWorkflow { static registerWorkflow(workflowId, exportedWorkflow) { if (workflowId in MedusaWorkflow.workflows) { - throw new Error(`Workflow with id ${workflowId} already registered.`) + return } MedusaWorkflow.workflows[workflowId] = exportedWorkflow diff --git a/packages/core/workflows-sdk/src/utils/composer/create-workflow.ts b/packages/core/workflows-sdk/src/utils/composer/create-workflow.ts index ac4d772990..5d706e1cad 100644 --- a/packages/core/workflows-sdk/src/utils/composer/create-workflow.ts +++ b/packages/core/workflows-sdk/src/utils/composer/create-workflow.ts @@ -102,15 +102,15 @@ export function createWorkflow< const handlers: WorkflowHandler = new Map() - if (WorkflowManager.getWorkflow(name)) { - WorkflowManager.unregister(name) + let newWorkflow = false + if (!WorkflowManager.getWorkflow(name)) { + newWorkflow = true + WorkflowManager.register(name, undefined, handlers, options) } - WorkflowManager.register(name, undefined, handlers, options) - const context: CreateWorkflowComposerContext = { workflowId: name, - flow: WorkflowManager.getTransactionDefinition(name), + flow: WorkflowManager.getEmptyTransactionDefinition(), handlers, hooks_: [], hooksCallback_: {}, @@ -141,7 +141,11 @@ export function createWorkflow< delete global[OrchestrationUtils.SymbolMedusaWorkflowComposerContext] - WorkflowManager.update(name, context.flow, handlers, options) + if (newWorkflow) { + WorkflowManager.update(name, context.flow, handlers, options) + } else { + WorkflowManager.register(name, context.flow, handlers, options) + } const workflow = exportWorkflow( name,