From e47461d95caecf3a447ee9fa0b0950340b93f282 Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Wed, 18 Oct 2023 11:47:22 +0200 Subject: [PATCH] chore(orchestration): prevent throwing on already defined workflow (#5337) **What** At the moment, when importing something from medusa entry point, if two medusa packages are installed because of a plugin, the evaluation of the import can end up throwing that a workflow is already defined. To prevent that from happening, we can just not throw if it is already defined and just return prematurely and make it idempotent. --- .changeset/strong-dragons-flow.md | 6 +++ .../src/__tests__/workflow/local-workflow.ts | 46 +++++++++++++++++++ .../src/workflow/workflow-manager.ts | 14 ++++-- .../common/__tests__/deep-equal-obj.spec.ts | 33 +++++++++++++ packages/utils/src/common/deep-equal-obj.ts | 24 ++++++++++ packages/utils/src/common/index.ts | 1 + 6 files changed, 120 insertions(+), 4 deletions(-) create mode 100644 .changeset/strong-dragons-flow.md create mode 100644 packages/utils/src/common/__tests__/deep-equal-obj.spec.ts create mode 100644 packages/utils/src/common/deep-equal-obj.ts diff --git a/.changeset/strong-dragons-flow.md b/.changeset/strong-dragons-flow.md new file mode 100644 index 0000000000..083f49d5c9 --- /dev/null +++ b/.changeset/strong-dragons-flow.md @@ -0,0 +1,6 @@ +--- +"@medusajs/orchestration": patch +"@medusajs/utilsgst": patch +--- + +chore(orchestration): prevent throwing on already defined workflow diff --git a/packages/orchestration/src/__tests__/workflow/local-workflow.ts b/packages/orchestration/src/__tests__/workflow/local-workflow.ts index a5c07c2882..6060a37515 100644 --- a/packages/orchestration/src/__tests__/workflow/local-workflow.ts +++ b/packages/orchestration/src/__tests__/workflow/local-workflow.ts @@ -80,6 +80,52 @@ describe("WorkflowManager", () => { expect(wf).toEqual(["create-product", "broken-delivery", "deliver-product"]) }) + it("should throw when registering a workflow with an existing id", () => { + let err + try { + WorkflowManager.register( + "create-product", + { + action: "foo", + next: { + action: "bar", + next: { + action: "xor", + }, + }, + }, + handlers + ) + } catch (e) { + err = e + } + + expect(err).toBeDefined() + expect(err.message).toBe( + `Workflow with id "create-product" and step definition already exists.` + ) + }) + + it("should not throw when registering a workflow with an existing id but identical definition", () => { + let err + try { + WorkflowManager.register( + "create-product", + { + action: "foo", + next: { + action: "bar", + }, + }, + handlers + ) + } catch (e) { + err = e + } + + expect(err).not.toBeDefined() + }) + it("should begin a transaction and returns its final state", async () => { const flow = new LocalWorkflow("create-product", container) const transaction = await flow.run("t-id", { diff --git a/packages/orchestration/src/workflow/workflow-manager.ts b/packages/orchestration/src/workflow/workflow-manager.ts index 7cf49251e8..e46f300f5b 100644 --- a/packages/orchestration/src/workflow/workflow-manager.ts +++ b/packages/orchestration/src/workflow/workflow-manager.ts @@ -75,12 +75,18 @@ export class WorkflowManager { requiredModules?: Set, optionalModules?: Set ) { - if (WorkflowManager.workflows.has(workflowId)) { - throw new Error(`Workflow with id "${workflowId}" is already defined.`) - } - const finalFlow = flow instanceof OrchestratorBuilder ? flow.build() : flow + if (WorkflowManager.workflows.has(workflowId)) { + const areStepsEqual = + JSON.stringify(finalFlow) === + JSON.stringify(WorkflowManager.workflows.get(workflowId)!.flow_) + + if (!areStepsEqual) { + throw new Error(`Workflow with id "${workflowId}" and step definition already exists.`) + } + } + WorkflowManager.workflows.set(workflowId, { id: workflowId, flow_: finalFlow, diff --git a/packages/utils/src/common/__tests__/deep-equal-obj.spec.ts b/packages/utils/src/common/__tests__/deep-equal-obj.spec.ts new file mode 100644 index 0000000000..69e6c3337f --- /dev/null +++ b/packages/utils/src/common/__tests__/deep-equal-obj.spec.ts @@ -0,0 +1,33 @@ +import { deepEqualObj } from "../deep-equal-obj" + +describe("deepEqualObj", function () { + it("should return true if objects are equal", function () { + const object1 = { + foo: "bar", + bar: "foo", + xar: { foo: "bar", wor: { bar: "foo", ror: ["test", "test1"] } }, + } + const object2 = { + foo: "bar", + bar: "foo", + xar: { foo: "bar", wor: { bar: "foo", ror: ["test", "test1"] } }, + } + + expect(deepEqualObj(object1, object2)).toBe(true) + }) + + it("should return false if objects are not equal", function () { + const object1 = { + foo: "bar", + bar: "foo", + xar: { foo: "bar", wor: { bar: "foo", ror: ["test", "test1"] } }, + } + const object2 = { + foo: "bar", + bar: "foo", + xar: { foo: "bar", wor: { bar: "foo", ror: ["test", "test1_"] } }, + } + + expect(deepEqualObj(object1, object2)).toBe(false) + }) +}) diff --git a/packages/utils/src/common/deep-equal-obj.ts b/packages/utils/src/common/deep-equal-obj.ts new file mode 100644 index 0000000000..495b683cde --- /dev/null +++ b/packages/utils/src/common/deep-equal-obj.ts @@ -0,0 +1,24 @@ +export function deepEqualObj(obj1: object, obj2: object): boolean { + if (typeof obj1 !== typeof obj2) { + return false + } + + if (typeof obj1 !== "object" || obj1 === null) { + return obj1 === obj2 + } + + const obj1Keys = Object.keys(obj1) + const obj2Keys = Object.keys(obj2) + + if (obj1Keys.length !== obj2Keys.length) { + return false + } + + for (const key of obj1Keys) { + if (!obj2Keys.includes(key) || !deepEqualObj(obj1[key], obj2[key])) { + return false + } + } + + return true +} diff --git a/packages/utils/src/common/index.ts b/packages/utils/src/common/index.ts index 7d6237e5bb..af925d5d85 100644 --- a/packages/utils/src/common/index.ts +++ b/packages/utils/src/common/index.ts @@ -29,3 +29,4 @@ export * from "./to-pascal-case" export * from "./transaction" export * from "./upper-case-first" export * from "./wrap-handler" +export * from "./deep-equal-obj"