From 2b5688a953f05407e1a9b66f78a2fd069f47135d Mon Sep 17 00:00:00 2001 From: Icebob Date: Sat, 25 Nov 2023 17:36:40 +0100 Subject: [PATCH] remove legacy event handler --- CHANGELOG.md | 35 ++++++ docs/MIGRATION_GUIDE_0.15.md | 35 ++++++ src/service.js | 25 +---- src/utils.d.ts | 4 - src/utils.js | 118 --------------------- test/integration/broker-transit.spec.js | 11 +- test/integration/service-mixins.spec.js | 69 ++++++------ test/integration/service.lifecycle.spec.js | 22 ++-- test/unit/service-broker.spec.js | 3 - test/unit/service.spec.js | 41 +------ test/unit/utils.spec.js | 16 --- 11 files changed, 124 insertions(+), 255 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee9a274c2..37856c58b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,41 @@ The reason is desribed in this issue: https://github.com/moleculerjs/moleculer/i If you use one of those, you should change it to one of these schemaless serializers: MsgPack, Notepack.io, JSON, JSONExt, CBOR +## Legacy event handler is removed + +The legacy event handler signature (`user.created(payload, sender, eventName)`) is removed. You should use the new `Context` based signature which was introduced in version 0.14. + +**Legacy event handler** + +```js +module.exports = { + name: "accounts", + events: { + "user.created"(payload, sender, eventName) { + // ... + } + } +}; +``` + +**Supported event handler** + +```js +module.exports = { + name: "accounts", + events: { + "user.created"(ctx) { + console.log("Payload:", ctx.params); + console.log("Sender:", ctx.nodeID); + console.log("We have also metadata:", ctx.meta); + console.log("The called event name:", ctx.eventName); + + // ... + } + } +}; +``` + ## Action streaming The built-in `Stream` sending has been rewritten. Now it accepts `params` besides the `Stream` instance. diff --git a/docs/MIGRATION_GUIDE_0.15.md b/docs/MIGRATION_GUIDE_0.15.md index a251c7281..3cf03b89b 100644 --- a/docs/MIGRATION_GUIDE_0.15.md +++ b/docs/MIGRATION_GUIDE_0.15.md @@ -12,6 +12,41 @@ This documentation leads you how you can migrate your project to be compatible w >} >``` +## Legacy event handler is removed + +The legacy event handler signature (`user.created(payload, sender, eventName)`) is removed. You should use the new `Context` based signature which was introduced in version 0.14. + +**Legacy event handler** + +```js +module.exports = { + name: "accounts", + events: { + "user.created"(payload, sender, eventName) { + // ... + } + } +}; +``` + +**Supported event handler** + +```js +module.exports = { + name: "accounts", + events: { + "user.created"(ctx) { + console.log("Payload:", ctx.params); + console.log("Sender:", ctx.nodeID); + console.log("We have also metadata:", ctx.meta); + console.log("The called event name:", ctx.eventName); + + // ... + } + } +}; +``` + ## New action streaming The built-in `Stream` sending has been rewritten. Now it accepts `params` besides the `Stream` instance. diff --git a/src/service.js b/src/service.js index 605c95858..13bdcec68 100644 --- a/src/service.js +++ b/src/service.js @@ -8,7 +8,7 @@ const _ = require("lodash"); const { ServiceSchemaError, MoleculerError } = require("./errors"); -const { isObject, isFunction, flatten, functionArguments, uniq } = require("./utils"); +const { isObject, isFunction, flatten, uniq } = require("./utils"); /** * Import types @@ -38,10 +38,6 @@ function wrapToArray(o) { return Array.isArray(o) ? o : [o]; } -function isNewSignature(args) { - return args.length > 0 && ["ctx", "context"].indexOf(args[0].toLowerCase()) !== -1; -} - /** * Service class * @@ -468,15 +464,10 @@ class Service { // New: handler(ctx) let handler; if (isFunction(event.handler)) { - const args = functionArguments(event.handler); handler = this.Promise.method(event.handler); - // @ts-ignore - handler.__newSignature = event.context === true || isNewSignature(args); } else if (Array.isArray(event.handler)) { handler = event.handler.map(h => { - const args = functionArguments(h); h = this.Promise.method(h); - h.__newSignature = event.context === true || isNewSignature(args); return h; }); } @@ -488,22 +479,12 @@ class Service { if (isFunction(handler)) { // Call single handler event.handler = function (ctx) { - return handler.apply( - self, - handler.__newSignature ? [ctx] : [ctx.params, ctx.nodeID, ctx.eventName, ctx] - ); + return handler.call(self, ctx); }; } else if (Array.isArray(handler)) { // Call multiple handler event.handler = function (ctx) { - return self.Promise.all( - handler.map(fn => - fn.apply( - self, - fn.__newSignature ? [ctx] : [ctx.params, ctx.nodeID, ctx.eventName, ctx] - ) - ) - ); + return self.Promise.all(handler.map(fn => fn.call(self, ctx))); }; } diff --git a/src/utils.d.ts b/src/utils.d.ts index 468933457..d77beaf80 100644 --- a/src/utils.d.ts +++ b/src/utils.d.ts @@ -39,7 +39,3 @@ export declare function makeDirs(path: string): void; export declare function parseByteString(value: string): number; export declare function uniq(arr: Array): Array; - -export declare function functionArguments(function_: Function): Array; - - diff --git a/src/utils.js b/src/utils.js index 421ea2757..cf06a333b 100644 --- a/src/utils.js +++ b/src/utils.js @@ -504,124 +504,6 @@ const utils = { return false; }, - /** - * Detects the argument names of a function. - * Credits: https://github.com/sindresorhus/fn-args - * - * @param {Function} function_ - * @returns {Array} - */ - functionArguments(function_) { - if (typeof function_ !== "function") { - throw new TypeError("Expected a function"); - } - - const commentRegex = /(\/\*([\s\S]*?)\*\/|([^:]|^)\/\/(.*)$)/gm; - const quotes = ["`", '"', "'"]; - - const functionSource = function_.toString().replace(commentRegex, ""); // Function with no comments - - let functionWithNoDefaults = ""; - let depth = 0; // () [] {} - let index = 0; - - // To remove default values we can not use regexp because finite automaton can not handle such - // things as (potential) infinity-nested blocks (), [], {} - - // Remove default values - for (; index < functionSource.length && functionSource.charAt(index) !== ")"; index += 1) { - // Exiting if an arrow occurs. Needed when arrow function without '()'. - if (functionSource.startsWith("=>", index)) { - functionWithNoDefaults = functionSource; - index = functionSource.length; - break; - } - - // If we found a default value - skip it - if (functionSource.charAt(index) === "=") { - for ( - ; - index < functionSource.length && - ((functionSource.charAt(index) !== "," && - functionSource.charAt(index) !== ")") || - depth !== 0); - index += 1 - ) { - // Skip all quotes - let wasQuote = false; - - for (const quote of quotes) { - if (functionSource.charAt(index) === quote) { - index += 1; - - for ( - ; - index < functionSource.length && - functionSource.charAt(index) !== quote; - - ) { - index += 1; - } - - wasQuote = true; - break; - } - } - - // If any quote type was skipped, start the cycle again - if (wasQuote) { - continue; - } - - switch ( - functionSource.charAt(index) // Keeps correct depths of all types of parenthesises - ) { - case "(": - case "[": - case "{": - depth += 1; - break; - case ")": - case "]": - case "}": - depth -= 1; - break; - default: - } - } - - if (functionSource.charAt(index) === ",") { - functionWithNoDefaults += ","; - } - - if (functionSource.charAt(index) === ")") { - // Quits from the cycle immediately - functionWithNoDefaults += ")"; - break; - } - } else { - functionWithNoDefaults += functionSource.charAt(index); - } - } - - if (index < functionSource.length && functionSource.charAt(index) === ")") { - functionWithNoDefaults += ")"; - } - - // The first part matches parens-less arrow functions - // The second part matches the rest - const regexFnArguments = /^(?:async)?([^=()]+)=|\(([^)]+)\)/; - - const match = regexFnArguments.exec(functionWithNoDefaults); - - return match - ? (match[1] || match[2]) - .split(",") - .map(x => x.trim()) - .filter(Boolean) - : []; - }, - /** * Creates a duplicate-free version of an array * diff --git a/test/integration/broker-transit.spec.js b/test/integration/broker-transit.spec.js index 49e6c15fc..3cb33b5a3 100644 --- a/test/integration/broker-transit.spec.js +++ b/test/integration/broker-transit.spec.js @@ -72,12 +72,11 @@ describe("Test RPC", () => { it("should emit & receive an event via transporter", () => { return b1.call("echo.emitter", { a: 5 }).then(() => { expect(eventHandler).toHaveBeenCalledTimes(1); - expect(eventHandler).toHaveBeenCalledWith( - { a: 5 }, - "node-2", - "emitter.hello.event", - expect.any(b1.ContextFactory) - ); + expect(eventHandler).toHaveBeenCalledWith(expect.any(b1.ContextFactory)); + let ctx = eventHandler.mock.calls[0][0]; + expect(ctx.params).toEqual({ a: 5 }); + expect(ctx.eventName).toBe("emitter.hello.event"); + expect(ctx.nodeID).toBe("node-2"); }); }); diff --git a/test/integration/service-mixins.spec.js b/test/integration/service-mixins.spec.js index 697778647..0a4b6b158 100644 --- a/test/integration/service-mixins.spec.js +++ b/test/integration/service-mixins.spec.js @@ -633,12 +633,11 @@ describe("Test Service mixins", () => { broker.broadcastLocal("carbon", payload); expect(mainSchema.events.carbon).toHaveBeenCalledTimes(1); - expect(mainSchema.events.carbon).toHaveBeenCalledWith( - payload, - broker.nodeID, - "carbon", - expect.any(broker.ContextFactory) - ); + expect(mainSchema.events.carbon).toHaveBeenCalledWith(expect.any(broker.ContextFactory)); + const ctx = mainSchema.events.carbon.mock.calls[0][0]; + expect(ctx.params).toEqual(payload); + expect(ctx.eventName).toBe("carbon"); + expect(ctx.nodeID).toBe(broker.nodeID); }); it("should call 'hydrogen' event handlers", () => { @@ -646,20 +645,18 @@ describe("Test Service mixins", () => { broker.broadcastLocal("hydrogen", payload); expect(mixin1L1.events.hydrogen).toHaveBeenCalledTimes(1); - expect(mixin1L1.events.hydrogen).toHaveBeenCalledWith( - payload, - broker.nodeID, - "hydrogen", - expect.any(broker.ContextFactory) - ); + expect(mixin1L1.events.hydrogen).toHaveBeenCalledWith(expect.any(broker.ContextFactory)); + let ctx = mixin1L1.events.hydrogen.mock.calls[0][0]; + expect(ctx.params).toEqual(payload); + expect(ctx.eventName).toBe("hydrogen"); + expect(ctx.nodeID).toBe(broker.nodeID); expect(mixin2L1.events.hydrogen).toHaveBeenCalledTimes(1); - expect(mixin2L1.events.hydrogen).toHaveBeenCalledWith( - payload, - broker.nodeID, - "hydrogen", - expect.any(broker.ContextFactory) - ); + expect(mixin2L1.events.hydrogen).toHaveBeenCalledWith(expect.any(broker.ContextFactory)); + ctx = mixin2L1.events.hydrogen.mock.calls[0][0]; + expect(ctx.params).toEqual(payload); + expect(ctx.eventName).toBe("hydrogen"); + expect(ctx.nodeID).toBe(broker.nodeID); }); it("should call 'nitrogen' event handlers without group", () => { @@ -668,19 +665,19 @@ describe("Test Service mixins", () => { expect(mixin1L1.events.nitrogen.handler).toHaveBeenCalledTimes(1); expect(mixin1L1.events.nitrogen.handler).toHaveBeenCalledWith( - payload, - broker.nodeID, - "nitrogen", expect.any(broker.ContextFactory) ); + let ctx = mixin1L1.events.nitrogen.handler.mock.calls[0][0]; + expect(ctx.params).toEqual(payload); + expect(ctx.eventName).toBe("nitrogen"); + expect(ctx.nodeID).toBe(broker.nodeID); expect(mainSchema.events.nitrogen).toHaveBeenCalledTimes(1); - expect(mainSchema.events.nitrogen).toHaveBeenCalledWith( - payload, - broker.nodeID, - "nitrogen", - expect.any(broker.ContextFactory) - ); + expect(mainSchema.events.nitrogen).toHaveBeenCalledWith(expect.any(broker.ContextFactory)); + ctx = mainSchema.events.nitrogen.mock.calls[0][0]; + expect(ctx.params).toEqual(payload); + expect(ctx.eventName).toBe("nitrogen"); + expect(ctx.nodeID).toBe(broker.nodeID); }); it("should call 'nitrogen' event handlers with group", () => { @@ -692,19 +689,19 @@ describe("Test Service mixins", () => { expect(mixin1L1.events.nitrogen.handler).toHaveBeenCalledTimes(1); expect(mixin1L1.events.nitrogen.handler).toHaveBeenCalledWith( - payload, - broker.nodeID, - "nitrogen", expect.any(broker.ContextFactory) ); + let ctx = mixin1L1.events.nitrogen.handler.mock.calls[0][0]; + expect(ctx.params).toEqual(payload); + expect(ctx.eventName).toBe("nitrogen"); + expect(ctx.nodeID).toBe(broker.nodeID); expect(mainSchema.events.nitrogen).toHaveBeenCalledTimes(1); - expect(mainSchema.events.nitrogen).toHaveBeenCalledWith( - payload, - broker.nodeID, - "nitrogen", - expect.any(broker.ContextFactory) - ); + expect(mainSchema.events.nitrogen).toHaveBeenCalledWith(expect.any(broker.ContextFactory)); + ctx = mainSchema.events.nitrogen.mock.calls[0][0]; + expect(ctx.params).toEqual(payload); + expect(ctx.eventName).toBe("nitrogen"); + expect(ctx.nodeID).toBe(broker.nodeID); }); it("should call 'nitrogen' event handlers with wrong group", () => { diff --git a/test/integration/service.lifecycle.spec.js b/test/integration/service.lifecycle.spec.js index 722fa0269..1c6da3422 100644 --- a/test/integration/service.lifecycle.spec.js +++ b/test/integration/service.lifecycle.spec.js @@ -40,12 +40,11 @@ describe("Test Service handlers", () => { it("should call event handler", () => { broker.broadcastLocal("user.created", { id: 1, name: "John" }); expect(eventHandler).toHaveBeenCalledTimes(1); - expect(eventHandler).toHaveBeenCalledWith( - { id: 1, name: "John" }, - "node-1", - "user.created", - expect.any(broker.ContextFactory) - ); + expect(eventHandler).toHaveBeenCalledWith(expect.any(broker.ContextFactory)); + const ctx = eventHandler.mock.calls[0][0]; + expect(ctx.params).toEqual({ id: 1, name: "John" }); + expect(ctx.eventName).toBe("user.created"); + expect(ctx.nodeID).toBe("node-1"); }); it("should call stop handler", () => { @@ -92,12 +91,11 @@ describe("Test Service handlers after broker.start", () => { it("should call event handler", () => { broker.broadcastLocal("user.created", { id: 1, name: "John" }); expect(eventHandler).toHaveBeenCalledTimes(1); - expect(eventHandler).toHaveBeenCalledWith( - { id: 1, name: "John" }, - "node-1", - "user.created", - expect.any(broker.ContextFactory) - ); + expect(eventHandler).toHaveBeenCalledWith(expect.any(broker.ContextFactory)); + const ctx = eventHandler.mock.calls[0][0]; + expect(ctx.params).toEqual({ id: 1, name: "John" }); + expect(ctx.eventName).toBe("user.created"); + expect(ctx.nodeID).toBe("node-1"); }); it("should call stop handler", () => { diff --git a/test/unit/service-broker.spec.js b/test/unit/service-broker.spec.js index eda79c782..8949b3fa9 100644 --- a/test/unit/service-broker.spec.js +++ b/test/unit/service-broker.spec.js @@ -44,9 +44,6 @@ jest.mock("../../src/utils", () => ({ polyfillPromise(p) { return polyfillPromise(p); }, - functionArguments() { - return ["ctx"]; - }, deprecate() {}, uniq(arr) { return [...new Set(arr)]; diff --git a/test/unit/service.spec.js b/test/unit/service.spec.js index d0ef3d170..a307f418c 100644 --- a/test/unit/service.spec.js +++ b/test/unit/service.spec.js @@ -1101,35 +1101,7 @@ describe("Test Service class", () => { return res.handler({}).then(res => expect(res).toEqual(["Hello1", "Hello2"])); }); - it("should call handler with legacy arguments", () => { - const handler = function (payload, nodeID, eventName, ctx) { - expect(this).toBe(svc); - return { payload, nodeID, eventName, ctx }; - }; - - const res = svc._createEvent( - { - handler - }, - "user.updated" - ); - - expect.assertions(5); - - const ctx = { - params: { a: 5 }, - nodeID: "node-100", - eventName: "user.removed" - }; - return res.handler(ctx).then(res => { - expect(res.payload).toEqual({ a: 5 }); - expect(res.nodeID).toEqual("node-100"); - expect(res.eventName).toEqual("user.removed"); - expect(res.ctx).toBe(ctx); - }); - }); - - it("should call handler with context", () => { + it("should call handlers", () => { const handler = function (ctx) { expect(this).toBe(svc); return { ctx }; @@ -1163,9 +1135,9 @@ describe("Test Service class", () => { return { ctx }; }; - const handler2 = function (payload, nodeID, eventName, ctx) { + const handler2 = function (ctx) { expect(this).toBe(svc); - return { payload, nodeID, eventName, ctx }; + return { ctx }; }; const res = svc._createEvent( @@ -1183,14 +1155,7 @@ describe("Test Service class", () => { eventName: "user.removed" }; return res.handler(ctx).then(([res1, res2]) => { - expect(res1.payload).toBeUndefined(); - expect(res1.nodeID).toBeUndefined(); - expect(res1.eventName).toBeUndefined(); expect(res1.ctx).toBe(ctx); - - expect(res2.payload).toEqual({ a: 5 }); - expect(res2.nodeID).toEqual("node-100"); - expect(res2.eventName).toEqual("user.removed"); expect(res2.ctx).toBe(ctx); }); }); diff --git a/test/unit/utils.spec.js b/test/unit/utils.spec.js index 3cfa0d2e5..c9ff3453f 100644 --- a/test/unit/utils.spec.js +++ b/test/unit/utils.spec.js @@ -794,22 +794,6 @@ describe("Test utils.polyfillPromise", () => { } }); -describe("Test utils.functionArguments", () => { - it("should detect the arguments of the Function", () => { - expect(utils.functionArguments(() => {})).toEqual([]); - expect(utils.functionArguments(function () {})).toEqual([]); - - expect(utils.functionArguments(alpha => {})).toEqual(["alpha"]); - expect(utils.functionArguments(function (alpha) {})).toEqual(["alpha"]); - - expect(utils.functionArguments((alpha, beta) => {})).toEqual(["alpha", "beta"]); - expect(utils.functionArguments(function (alpha, beta) {})).toEqual(["alpha", "beta"]); - - expect(utils.functionArguments(async (alpha, beta) => {})).toEqual(["alpha", "beta"]); - expect(utils.functionArguments(async function (alpha, beta) {})).toEqual(["alpha", "beta"]); - }); -}); - describe("Test utils.uniq", () => { it("should return duplicate-free version of an array", () => { expect(utils.uniq([1, 2, 3, 3])).toEqual([1, 2, 3]);