From 37aaca0da4ef3e4519ad898202e497852201b775 Mon Sep 17 00:00:00 2001 From: Philip Korsholm <88927411+pKorsholm@users.noreply.github.com> Date: Thu, 22 Dec 2022 13:20:57 +0100 Subject: [PATCH] feat(medusa): Extend Module Resolution configuration (#2864) **What** update module definitions in `ConfigModule` to support the following module configuration methods: - none: defaults are loaded - boolean `inventoryService: true|false`, if true the defaults are loaded, false throws if it's a required module, if the module is not required it's not loaded - string: `inventoryService: "..."`, treats the string as a path to the overriding module, (we dont handle the case where a string is given but the module is not overridable, we just load the default in that case) - `ConfigurableModuleDeclaration = { resolve?: string, options?: Record } `, like plugins, options can be used to pass configs to the main service of the module, if not defined no options are passed. Resolve is like string, if defined it's used to look for a custom module, otherwise the default is loaded. Testing: - Added unit tests for: - `loaders/modules.ts` - `loaders/module-definitions/index.ts` Fixes CORE-932 --- .../__mocks__/@modules/brokenloader.ts | 7 + .../src/loaders/__mocks__/@modules/default.ts | 3 + .../loaders/__mocks__/@modules/no-service.ts | 2 + .../src/loaders/__mocks__/medusa-telemetry.ts | 2 + .../__tests__/module-definitions.spec.ts | 144 +++++++++++ .../src/loaders/__tests__/module.spec.ts | 225 ++++++++++++++++++ packages/medusa/src/loaders/index.ts | 12 +- .../src/loaders/module-definitions/index.ts | 33 ++- packages/medusa/src/loaders/module.ts | 110 ++++++--- packages/medusa/src/types/global.ts | 10 +- packages/medusa/src/types/modules.ts | 2 +- 11 files changed, 509 insertions(+), 41 deletions(-) create mode 100644 packages/medusa/src/loaders/__mocks__/@modules/brokenloader.ts create mode 100644 packages/medusa/src/loaders/__mocks__/@modules/default.ts create mode 100644 packages/medusa/src/loaders/__mocks__/@modules/no-service.ts create mode 100644 packages/medusa/src/loaders/__mocks__/medusa-telemetry.ts create mode 100644 packages/medusa/src/loaders/__tests__/module-definitions.spec.ts create mode 100644 packages/medusa/src/loaders/__tests__/module.spec.ts diff --git a/packages/medusa/src/loaders/__mocks__/@modules/brokenloader.ts b/packages/medusa/src/loaders/__mocks__/@modules/brokenloader.ts new file mode 100644 index 0000000000..7fc8599ea6 --- /dev/null +++ b/packages/medusa/src/loaders/__mocks__/@modules/brokenloader.ts @@ -0,0 +1,7 @@ +const loader = ({}) => { + throw new Error("loader") +} + +export const service = class TestService {} +export const migrations = [] +export const loaders = [loader] diff --git a/packages/medusa/src/loaders/__mocks__/@modules/default.ts b/packages/medusa/src/loaders/__mocks__/@modules/default.ts new file mode 100644 index 0000000000..469cf6030c --- /dev/null +++ b/packages/medusa/src/loaders/__mocks__/@modules/default.ts @@ -0,0 +1,3 @@ +export const service = class TestService {} +export const migrations = [] +export const loaders = [] diff --git a/packages/medusa/src/loaders/__mocks__/@modules/no-service.ts b/packages/medusa/src/loaders/__mocks__/@modules/no-service.ts new file mode 100644 index 0000000000..9901daddec --- /dev/null +++ b/packages/medusa/src/loaders/__mocks__/@modules/no-service.ts @@ -0,0 +1,2 @@ +export const migrations = [] +export const loaders = [] diff --git a/packages/medusa/src/loaders/__mocks__/medusa-telemetry.ts b/packages/medusa/src/loaders/__mocks__/medusa-telemetry.ts new file mode 100644 index 0000000000..7b9171911c --- /dev/null +++ b/packages/medusa/src/loaders/__mocks__/medusa-telemetry.ts @@ -0,0 +1,2 @@ +export const trackInstallation = jest.fn() +export const trackFeatureFlag = jest.fn() diff --git a/packages/medusa/src/loaders/__tests__/module-definitions.spec.ts b/packages/medusa/src/loaders/__tests__/module-definitions.spec.ts new file mode 100644 index 0000000000..7739b8db96 --- /dev/null +++ b/packages/medusa/src/loaders/__tests__/module-definitions.spec.ts @@ -0,0 +1,144 @@ +// import resolveCwd from "resolve-cwd" +import { ConfigModule } from "../../types/global" +import ModuleDefinitionLoader from "../module-definitions" +import MODULE_DEFINITIONS from "../module-definitions/definitions" + +const RESOLVED_PACKAGE = "@medusajs/test-service-resolved" +jest.mock("resolve-cwd", () => jest.fn(() => RESOLVED_PACKAGE)) + +describe("module definitions loader", () => { + const defaultDefinition = { + key: "testService", + registrationName: "testService", + defaultPackage: "@medusajs/test-service", + label: "TestService", + isRequired: false, + canOverride: true, + } + + beforeEach(() => { + jest.resetModules() + jest.clearAllMocks() + + // Clear module definitions + MODULE_DEFINITIONS.splice(0, MODULE_DEFINITIONS.length) + }) + + it("Resolves module with default definition given empty config", () => { + MODULE_DEFINITIONS.push({ ...defaultDefinition }) + + const res = ModuleDefinitionLoader({ modules: {} } as ConfigModule) + + expect(res[defaultDefinition.key]).toEqual({ + resolutionPath: defaultDefinition.defaultPackage, + definition: defaultDefinition, + options: {}, + }) + }) + + describe("boolean config", () => { + it("Resolves module with no resolution path when given false", () => { + MODULE_DEFINITIONS.push({ ...defaultDefinition }) + + const res = ModuleDefinitionLoader({ + modules: { [defaultDefinition.key]: false }, + } as ConfigModule) + + expect(res[defaultDefinition.key]).toEqual({ + resolutionPath: undefined, + definition: defaultDefinition, + options: {}, + }) + }) + + it("Fails to resolve module with no resolution path when given false for a required module", () => { + expect.assertions(1) + MODULE_DEFINITIONS.push({ ...defaultDefinition, isRequired: true }) + + try { + ModuleDefinitionLoader({ + modules: { [defaultDefinition.key]: false }, + } as ConfigModule) + } catch (err) { + expect(err.message).toEqual( + `Module: ${defaultDefinition.label} is required` + ) + } + }) + }) + + describe("string config", () => { + it("Resolves module with default definition given empty config", () => { + MODULE_DEFINITIONS.push({ ...defaultDefinition }) + + const res = ModuleDefinitionLoader({ + modules: { + [defaultDefinition.key]: defaultDefinition.defaultPackage, + }, + } as ConfigModule) + + expect(res[defaultDefinition.key]).toEqual({ + resolutionPath: RESOLVED_PACKAGE, + definition: defaultDefinition, + options: {}, + }) + }) + }) + + describe("object config", () => { + it("Resolves resolution path and provides empty options when none are provided", () => { + MODULE_DEFINITIONS.push({ ...defaultDefinition }) + + const res = ModuleDefinitionLoader({ + modules: { + [defaultDefinition.key]: { + resolve: defaultDefinition.defaultPackage, + }, + }, + } as ConfigModule) + + expect(res[defaultDefinition.key]).toEqual({ + resolutionPath: RESOLVED_PACKAGE, + definition: defaultDefinition, + options: {}, + }) + }) + + it("Resolves default resolution path and provides options when only options are provided", () => { + MODULE_DEFINITIONS.push({ ...defaultDefinition }) + + const res = ModuleDefinitionLoader({ + modules: { + [defaultDefinition.key]: { + options: { test: 123 }, + }, + }, + } as unknown as ConfigModule) + + expect(res[defaultDefinition.key]).toEqual({ + resolutionPath: defaultDefinition.defaultPackage, + definition: defaultDefinition, + options: { test: 123 }, + }) + }) + + it("Resolves resolution path and provides options when only options are provided", () => { + MODULE_DEFINITIONS.push({ ...defaultDefinition }) + + const res = ModuleDefinitionLoader({ + modules: { + [defaultDefinition.key]: { + resolve: defaultDefinition.defaultPackage, + options: { test: 123 }, + }, + }, + } as unknown as ConfigModule) + + expect(res[defaultDefinition.key]).toEqual({ + resolutionPath: RESOLVED_PACKAGE, + definition: defaultDefinition, + options: { test: 123 }, + }) + }) + }) +}) diff --git a/packages/medusa/src/loaders/__tests__/module.spec.ts b/packages/medusa/src/loaders/__tests__/module.spec.ts new file mode 100644 index 0000000000..47fd8b49f7 --- /dev/null +++ b/packages/medusa/src/loaders/__tests__/module.spec.ts @@ -0,0 +1,225 @@ +import { + asFunction, + asValue, + AwilixContainer, + ClassOrFunctionReturning, + createContainer, + Resolver, +} from "awilix" +import { mkdirSync, rmSync, writeFileSync } from "fs" +import Logger from "../logger" +import { resolve } from "path" +import { + ConfigModule, + MedusaContainer, + ModuleResolution, +} from "../../types/global" +import registerModules from "../module" +import { trackInstallation } from "../__mocks__/medusa-telemetry" + +function asArray( + resolvers: (ClassOrFunctionReturning | Resolver)[] +): { resolve: (container: AwilixContainer) => unknown[] } { + return { + resolve: (container: AwilixContainer): unknown[] => + resolvers.map((resolver) => container.build(resolver)), + } +} + +const buildConfigModule = ( + configParts: Partial +): ConfigModule => { + return { + projectConfig: { + database_type: "sqlite", + database_logging: "all", + }, + featureFlags: {}, + modules: {}, + moduleResolutions: {}, + plugins: [], + ...configParts, + } +} + +const buildContainer = () => { + const container = createContainer() as MedusaContainer + + container.registerAdd = function ( + this: MedusaContainer, + name: string, + registration: typeof asFunction | typeof asValue + ): MedusaContainer { + const storeKey = name + "_STORE" + + if (this.registrations[storeKey] === undefined) { + this.register(storeKey, asValue([] as Resolver[])) + } + const store = this.resolve(storeKey) as ( + | ClassOrFunctionReturning + | Resolver + )[] + + if (this.registrations[name] === undefined) { + this.register(name, asArray(store)) + } + store.unshift(registration) + + return this + }.bind(container) + + return container +} +describe("modules loader", () => { + let container + + afterEach(() => { + jest.clearAllMocks() + }) + + beforeEach(() => { + container = buildContainer() + }) + + it("registers service as false in container when no resolution path is given", async () => { + const moduleResolutions: Record = { + testService: { + resolutionPath: undefined, + definition: { + registrationName: "testService", + key: "testService", + defaultPackage: "testService", + label: "TestService", + }, + }, + } + + const configModule = buildConfigModule({ + moduleResolutions, + }) + await registerModules({ container, configModule, logger: Logger }) + + const testService = container.resolve( + moduleResolutions.testService.definition.key + ) + expect(testService).toBe(false) + }) + + it("registers service ", async () => { + const moduleResolutions: Record = { + testService: { + resolutionPath: "@modules/default", + definition: { + registrationName: "testService", + key: "testService", + defaultPackage: "testService", + label: "TestService", + }, + }, + } + + const configModule = buildConfigModule({ + moduleResolutions, + }) + + await registerModules({ container, configModule, logger: Logger }) + + const testService = container.resolve( + moduleResolutions.testService.definition.key, + {} + ) + + expect(trackInstallation).toHaveBeenCalledWith( + { + module: moduleResolutions.testService.definition.key, + resolution: moduleResolutions.testService.resolutionPath, + }, + "module" + ) + expect(testService).toBeTruthy() + expect(typeof testService).toEqual("object") + }) + + it("runs defined loaders and logs error", async () => { + const moduleResolutions: Record = { + testService: { + resolutionPath: "@modules/brokenloader", + definition: { + registrationName: "testService", + key: "testService", + defaultPackage: "testService", + label: "TestService", + }, + }, + } + + const logger: typeof Logger = { + warn: jest.fn(), + } + + const configModule = buildConfigModule({ + moduleResolutions, + }) + + await registerModules({ container, configModule, logger }) + + expect(logger.warn).toHaveBeenCalledWith( + "Could not resolve module: TestService. Error: Loaders for module TestService failed: loader" + ) + }) + + it("logs error if no service is defined", async () => { + const moduleResolutions: Record = { + testService: { + resolutionPath: "@modules/no-service", + definition: { + registrationName: "testService", + key: "testService", + defaultPackage: "testService", + label: "TestService", + }, + }, + } + + const logger: typeof Logger = { + warn: jest.fn(), + } + + const configModule = buildConfigModule({ + moduleResolutions, + }) + + await registerModules({ container, configModule, logger }) + + expect(logger.warn).toHaveBeenCalledWith( + "Could not resolve module: TestService. Error: No service found in module. Make sure that your module exports a service." + ) + }) + + it("throws error if no service is defined and module is required", async () => { + expect.assertions(1) + const moduleResolutions: Record = { + testService: { + resolutionPath: "@modules/no-service", + definition: { + registrationName: "testService", + key: "testService", + defaultPackage: "testService", + label: "TestService", + isRequired: true, + }, + }, + } + + const configModule = buildConfigModule({ + moduleResolutions, + }) + try { + await registerModules({ container, configModule, logger: Logger }) + } catch (err) { + expect(err.message).toEqual( + "No service found in module. Make sure that your module exports a service." + ) + } + }) +}) diff --git a/packages/medusa/src/loaders/index.ts b/packages/medusa/src/loaders/index.ts index 6ae03b6431..4db2727c94 100644 --- a/packages/medusa/src/loaders/index.ts +++ b/packages/medusa/src/loaders/index.ts @@ -132,6 +132,12 @@ export default async ({ const stratAct = Logger.success(stratActivity, "Strategies initialized") || {} track("STRATEGIES_INIT_COMPLETED", { duration: stratAct.duration }) + const modulesActivity = Logger.activity(`Initializing modules${EOL}`) + track("MODULES_INIT_STARTED") + await moduleLoader({ container, configModule, logger: Logger }) + const modAct = Logger.success(modulesActivity, "Modules initialized") || {} + track("MODULES_INIT_COMPLETED", { duration: modAct.duration }) + const servicesActivity = Logger.activity(`Initializing services${EOL}`) track("SERVICES_INIT_STARTED") servicesLoader({ container, configModule, isTest }) @@ -191,12 +197,6 @@ export default async ({ Logger.success(searchActivity, "Indexing event emitted") || {} track("SEARCH_ENGINE_INDEXING_COMPLETED", { duration: searchAct.duration }) - const modulesActivity = Logger.activity(`Initializing modules${EOL}`) - track("MODULES_INIT_STARTED") - await moduleLoader({ container, configModule, logger: Logger }) - const modAct = Logger.success(modulesActivity, "Modules initialized") || {} - track("MODULES_INIT_COMPLETED", { duration: modAct.duration }) - return { container, dbConnection, app: expressApp } } diff --git a/packages/medusa/src/loaders/module-definitions/index.ts b/packages/medusa/src/loaders/module-definitions/index.ts index 49c59e96a0..93255a67bc 100644 --- a/packages/medusa/src/loaders/module-definitions/index.ts +++ b/packages/medusa/src/loaders/module-definitions/index.ts @@ -10,15 +10,42 @@ export default ({ modules }: ConfigModule) => { for (const definition of MODULE_DEFINITIONS) { let resolutionPath = definition.defaultPackage + const moduleConfiguration = projectModules[definition.key] + + if (typeof moduleConfiguration === "boolean") { + if (!moduleConfiguration && definition.isRequired) { + throw new Error(`Module: ${definition.label} is required`) + } + if (!moduleConfiguration) { + moduleResolutions[definition.key] = { + definition, + options: {}, + } + continue + } + } + // If user added a module and it's overridable, we resolve that instead - if (definition.canOverride && definition.key in projectModules) { - const mod = projectModules[definition.key] - resolutionPath = resolveCwd(mod) + if ( + definition.canOverride && + (typeof moduleConfiguration === "string" || + (typeof moduleConfiguration === "object" && + moduleConfiguration.resolve)) + ) { + resolutionPath = resolveCwd( + typeof moduleConfiguration === "string" + ? moduleConfiguration + : (moduleConfiguration.resolve as string) + ) } moduleResolutions[definition.key] = { resolutionPath, definition, + options: + typeof moduleConfiguration === "object" + ? moduleConfiguration.options ?? {} + : {}, } } diff --git a/packages/medusa/src/loaders/module.ts b/packages/medusa/src/loaders/module.ts index 44c87fe46f..333cada22f 100644 --- a/packages/medusa/src/loaders/module.ts +++ b/packages/medusa/src/loaders/module.ts @@ -11,6 +11,65 @@ type Options = { export const moduleHelper = new ModulesHelper() +const registerModule = async ( + container, + resolution, + configModule, + logger +): Promise<{ error?: Error } | void> => { + if (!resolution.resolutionPath) { + container.register({ + [resolution.definition.registrationName]: asValue(false), + }) + + return + } + + let loadedModule + try { + loadedModule = await import(resolution.resolutionPath!) + } catch (error) { + return { error } + } + + const moduleService = loadedModule?.service || null + + if (!moduleService) { + return { + error: new Error( + "No service found in module. Make sure that your module exports a service." + ), + } + } + + const moduleLoaders = loadedModule?.loaders || [] + try { + for (const loader of moduleLoaders) { + await loader({ container, configModule, logger }) + } + } catch (err) { + return { + error: new Error( + `Loaders for module ${resolution.definition.label} failed: ${err.message}` + ), + } + } + + container.register({ + [resolution.definition.registrationName]: asFunction( + (cradle) => new moduleService(cradle, resolution.options) + ).singleton(), + }) + + trackInstallation( + { + module: resolution.definition.key, + resolution: resolution.resolutionPath, + }, + "module" + ) +} + export default async ({ container, configModule, @@ -19,42 +78,35 @@ export default async ({ const moduleResolutions = configModule?.moduleResolutions ?? {} for (const resolution of Object.values(moduleResolutions)) { - try { - const loadedModule = await import(resolution.resolutionPath!) - - const moduleLoaders = loadedModule?.loaders || [] - for (const loader of moduleLoaders) { - await loader({ container, configModule, logger }) - } - - const moduleServices = loadedModule?.services || [] - - for (const service of moduleServices) { - container.register({ - [resolution.definition.registrationName]: asFunction( - (cradle) => new service(cradle, configModule) - ).singleton(), - }) - } - - const installation = { - module: resolution.definition.key, - resolution: resolution.resolutionPath, - } - - trackInstallation(installation, "module") - } catch (err) { + const registrationResult = await registerModule( + container, + resolution, + configModule, + logger + ) + if (registrationResult?.error) { + const { error } = registrationResult if (resolution.definition.isRequired) { - throw new Error( - `Could not resolve required module: ${resolution.definition.label}` + logger.warn( + `Could not resolve required module: ${resolution.definition.label}. Error: ${error.message}` ) + throw error } - logger.warn(`Couldn not resolve module: ${resolution.definition.label}`) + logger.warn( + `Could not resolve module: ${resolution.definition.label}. Error: ${error.message}` + ) } } - moduleHelper.setModules(moduleResolutions) + moduleHelper.setModules( + Object.entries(moduleResolutions).reduce((acc, [k, v]) => { + if (v.resolutionPath) { + acc[k] = v + } + return acc + }, {}) + ) container.register({ modulesHelper: asValue(moduleHelper), diff --git a/packages/medusa/src/types/global.ts b/packages/medusa/src/types/global.ts index ef8b35d6cb..7043efa4fb 100644 --- a/packages/medusa/src/types/global.ts +++ b/packages/medusa/src/types/global.ts @@ -38,8 +38,9 @@ export type Logger = _Logger & { } export type ModuleResolution = { - resolutionPath: string + resolutionPath?: string definition: ModuleDefinition + options?: Record } export type ModuleDefinition = { @@ -51,6 +52,11 @@ export type ModuleDefinition = { isRequired?: boolean } +export type ConfigurableModuleDeclaration = { + resolve?: string + options?: Record +} + export type ConfigModule = { projectConfig: { redis_url?: string @@ -71,7 +77,7 @@ export type ConfigModule = { admin_cors?: string } featureFlags: Record - modules?: Record + modules?: Record moduleResolutions?: Record plugins: ( | { diff --git a/packages/medusa/src/types/modules.ts b/packages/medusa/src/types/modules.ts index 2385f54f3b..dcca1cc063 100644 --- a/packages/medusa/src/types/modules.ts +++ b/packages/medusa/src/types/modules.ts @@ -1,4 +1,4 @@ export type ModulesResponse = { module: string - resolution: string + resolution?: string }[]