From df4f32202496604e746113389643dd470e9c5aa2 Mon Sep 17 00:00:00 2001 From: Ajai Shankar Date: Fri, 10 Feb 2023 21:55:05 -0600 Subject: [PATCH 1/4] feat(eval): compiled and cached expressions --- packages/bruno-js/package.json | 3 ++ packages/bruno-js/src/utils.js | 53 +++++++++++++++++++-- packages/bruno-js/tests/utils.spec.js | 68 +++++++++++++++++++++++++++ 3 files changed, 119 insertions(+), 5 deletions(-) create mode 100644 packages/bruno-js/tests/utils.spec.js diff --git a/packages/bruno-js/package.json b/packages/bruno-js/package.json index 95c16e8f..2a907eee 100644 --- a/packages/bruno-js/package.json +++ b/packages/bruno-js/package.json @@ -9,6 +9,9 @@ "peerDependencies": { "vm2": "^3.9.13" }, + "scripts": { + "test": "jest --testPathIgnorePatterns test.js" + }, "dependencies": { "atob": "^2.1.2", "ajv": "^8.12.0", diff --git a/packages/bruno-js/src/utils.js b/packages/bruno-js/src/utils.js index 0f456325..6b508d35 100644 --- a/packages/bruno-js/src/utils.js +++ b/packages/bruno-js/src/utils.js @@ -1,15 +1,57 @@ const jsonQuery = require('json-query'); +const JS_KEYWORDS = ` + break case catch class const continue debugger default delete do + else export extends false finally for function if import in instanceof + new null return super switch this throw true try typeof var void while with + undefined let static yield arguments of +`.split(/\s+/).filter(word => word.length > 0); + +/** + * Creates a function from a Javascript expression + * + * When the function is called, the variables used in this expression are picked up from the context + * + * ```js + * res.data.pets.map(pet => pet.name.toUpperCase()) + * + * function(context) { + * const { res, pet } = context; + * return res.data.pets.map(pet => pet.name.toUpperCase()) + * } + * ``` + */ +const compileJsExpression = (expr) => { + // get all dotted identifiers (foo, bar.baz, .baz) + const matches = expr.match(/([\w\.$]+)/g) ?? []; + + // get valid js identifiers (foo, bar) + const names = matches + .filter(match => /^[a-zA-Z$_]/.test(match)) + .map(match => match.split('.')[0]); + + // exclude js keywords and get unique vars + const vars = new Set(names.filter(name => !JS_KEYWORDS.includes(name))); + const spread = [...vars].join(", "); + const body = `const { ${spread} } = context; return ${expr}`; + return new Function("context", body); +}; + +const internalExpressionCache = new Map(); + const evaluateJsExpression = (expression, context) => { - const fn = new Function(...Object.keys(context), `return ${expression}`); - return fn(...Object.values(context)); + let fn = internalExpressionCache.get(expression); + if (fn == null) { + internalExpressionCache.set(expression, fn = compileJsExpression(expression)); + } + return fn(context); }; const createResponseParser = (response = {}) => { - const res = (expr) => { + const res = (expr) => { const output = jsonQuery(expr, { data: response.data }); return output ? output.value : null; - } + }; res.status = response.status; res.statusText = response.statusText; @@ -21,5 +63,6 @@ const createResponseParser = (response = {}) => { module.exports = { evaluateJsExpression, - createResponseParser + createResponseParser, + internalExpressionCache }; \ No newline at end of file diff --git a/packages/bruno-js/tests/utils.spec.js b/packages/bruno-js/tests/utils.spec.js new file mode 100644 index 00000000..caf5b751 --- /dev/null +++ b/packages/bruno-js/tests/utils.spec.js @@ -0,0 +1,68 @@ +const { evaluateJsExpression, internalExpressionCache: cache } = require("../src/utils"); + +describe("utils", () => { + describe("expression evaluation", () => { + const context = { + res: { + data: { pets: ["bruno", "max"] } + } + }; + + afterEach(() => cache.clear()); + + it("should evaluate expression", () => { + let result; + + result = evaluateJsExpression("res.data.pets", context); + expect(result).toEqual(["bruno", "max"]); + + result = evaluateJsExpression("res.data.pets[0].toUpperCase()", context); + expect(result).toEqual("BRUNO"); + }); + + it("should cache expression", () => { + expect(cache.size).toBe(0); + evaluateJsExpression("res.data.pets", context); + expect(cache.size).toBe(1); + }); + + it("should identify top level variables", () => { + const expr = "res.data.pets[0].toUpperCase()"; + evaluateJsExpression(expr, context); + expect(cache.get(expr).toString()).toContain("const { res } = context;"); + }); + + it("should not duplicate variables", () => { + const expr = "res.data.pets[0] + res.data.pets[1]"; + evaluateJsExpression(expr, context); + expect(cache.get(expr).toString()).toContain("const { res } = context;"); + }); + + it("should exclude js keywords like true false from vars", () => { + const expr = "res.data.pets.length > 0 ? true : false"; + evaluateJsExpression(expr, context); + expect(cache.get(expr).toString()).toContain("const { res } = context;"); + }); + + it("should exclude numbers from vars", () => { + const expr = "res.data.pets.length + 10"; + evaluateJsExpression(expr, context); + expect(cache.get(expr).toString()).toContain("const { res } = context;"); + }); + + it("should pick variables from complex expressions", () => { + const expr = "res.data.pets.map(pet => pet.length)"; + const result = evaluateJsExpression(expr, context); + expect(result).toEqual([5, 3]); + expect(cache.get(expr).toString()).toContain("const { res, pet } = context;"); + }); + + it("should be ok picking extra vars from strings", () => { + const expr = "'hello' + ' ' + res.data.pets[0]"; + const result = evaluateJsExpression(expr, context); + expect(result).toBe("hello bruno"); + // extra var hello is harmless + expect(cache.get(expr).toString()).toContain("const { hello, res } = context;"); + }); + }); +}); \ No newline at end of file From a4f757ee87f01defbcb88f6b01c1ae5d7ab985b0 Mon Sep 17 00:00:00 2001 From: Ajai Shankar Date: Fri, 10 Feb 2023 22:24:28 -0600 Subject: [PATCH 2/4] minor: clear expression cache before and after test --- packages/bruno-js/tests/utils.spec.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/bruno-js/tests/utils.spec.js b/packages/bruno-js/tests/utils.spec.js index caf5b751..e32ffb1d 100644 --- a/packages/bruno-js/tests/utils.spec.js +++ b/packages/bruno-js/tests/utils.spec.js @@ -8,6 +8,7 @@ describe("utils", () => { } }; + beforeEach(() => cache.clear()); afterEach(() => cache.clear()); it("should evaluate expression", () => { @@ -65,4 +66,4 @@ describe("utils", () => { expect(cache.get(expr).toString()).toContain("const { hello, res } = context;"); }); }); -}); \ No newline at end of file +}); From 429ca4093c45a325e4af41c2dd8c0486e73cfe1d Mon Sep 17 00:00:00 2001 From: Ajai Shankar Date: Fri, 10 Feb 2023 23:34:46 -0600 Subject: [PATCH 3/4] test: expression cache --- packages/bruno-js/tests/utils.spec.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/bruno-js/tests/utils.spec.js b/packages/bruno-js/tests/utils.spec.js index e32ffb1d..36a51c94 100644 --- a/packages/bruno-js/tests/utils.spec.js +++ b/packages/bruno-js/tests/utils.spec.js @@ -27,6 +27,20 @@ describe("utils", () => { expect(cache.size).toBe(1); }); + it("should use cached expression", () => { + const expr = "res.data.pets"; + + evaluateJsExpression(expr, context); + + const fn = cache.get(expr); + expect(fn).toBeDefined(); + + evaluateJsExpression(expr, context); + + // cache should not be overwritten + expect(cache.get(expr)).toBe(fn); + }); + it("should identify top level variables", () => { const expr = "res.data.pets[0].toUpperCase()"; evaluateJsExpression(expr, context); From 3d22f77226f3ef941c302b9d8e9be75a444560b6 Mon Sep 17 00:00:00 2001 From: Ajai Shankar Date: Sat, 11 Feb 2023 08:57:27 -0600 Subject: [PATCH 4/4] feat(eval): handle globals --- packages/bruno-js/src/utils.js | 26 +++++++++++----- packages/bruno-js/tests/utils.spec.js | 44 +++++++++++++++++++++++---- 2 files changed, 57 insertions(+), 13 deletions(-) diff --git a/packages/bruno-js/src/utils.js b/packages/bruno-js/src/utils.js index 6b508d35..6bc73da8 100644 --- a/packages/bruno-js/src/utils.js +++ b/packages/bruno-js/src/utils.js @@ -26,14 +26,26 @@ const compileJsExpression = (expr) => { const matches = expr.match(/([\w\.$]+)/g) ?? []; // get valid js identifiers (foo, bar) - const names = matches - .filter(match => /^[a-zA-Z$_]/.test(match)) - .map(match => match.split('.')[0]); + const vars = new Set( + matches + .filter(match => /^[a-zA-Z$_]/.test(match)) // starts with valid js identifier (foo.bar) + .map(match => match.split('.')[0]) // top level identifier (foo) + .filter(name => !JS_KEYWORDS.includes(name)) // exclude js keywords + ); + + // globals such as Math + const globals = [...vars].filter(name => name in globalThis); + + const code = { + vars: [...vars].join(", "), + // pick global from context or globalThis + globals: globals + .map(name => ` ${name} = ${name} ?? globalThis.${name};`) + .join('') + }; + + const body = `let { ${code.vars} } = context; ${code.globals}; return ${expr}`; - // exclude js keywords and get unique vars - const vars = new Set(names.filter(name => !JS_KEYWORDS.includes(name))); - const spread = [...vars].join(", "); - const body = `const { ${spread} } = context; return ${expr}`; return new Function("context", body); }; diff --git a/packages/bruno-js/tests/utils.spec.js b/packages/bruno-js/tests/utils.spec.js index 36a51c94..6d521240 100644 --- a/packages/bruno-js/tests/utils.spec.js +++ b/packages/bruno-js/tests/utils.spec.js @@ -44,32 +44,32 @@ describe("utils", () => { it("should identify top level variables", () => { const expr = "res.data.pets[0].toUpperCase()"; evaluateJsExpression(expr, context); - expect(cache.get(expr).toString()).toContain("const { res } = context;"); + expect(cache.get(expr).toString()).toContain("let { res } = context;"); }); it("should not duplicate variables", () => { const expr = "res.data.pets[0] + res.data.pets[1]"; evaluateJsExpression(expr, context); - expect(cache.get(expr).toString()).toContain("const { res } = context;"); + expect(cache.get(expr).toString()).toContain("let { res } = context;"); }); it("should exclude js keywords like true false from vars", () => { const expr = "res.data.pets.length > 0 ? true : false"; evaluateJsExpression(expr, context); - expect(cache.get(expr).toString()).toContain("const { res } = context;"); + expect(cache.get(expr).toString()).toContain("let { res } = context;"); }); it("should exclude numbers from vars", () => { const expr = "res.data.pets.length + 10"; evaluateJsExpression(expr, context); - expect(cache.get(expr).toString()).toContain("const { res } = context;"); + expect(cache.get(expr).toString()).toContain("let { res } = context;"); }); it("should pick variables from complex expressions", () => { const expr = "res.data.pets.map(pet => pet.length)"; const result = evaluateJsExpression(expr, context); expect(result).toEqual([5, 3]); - expect(cache.get(expr).toString()).toContain("const { res, pet } = context;"); + expect(cache.get(expr).toString()).toContain("let { res, pet } = context;"); }); it("should be ok picking extra vars from strings", () => { @@ -77,7 +77,39 @@ describe("utils", () => { const result = evaluateJsExpression(expr, context); expect(result).toBe("hello bruno"); // extra var hello is harmless - expect(cache.get(expr).toString()).toContain("const { hello, res } = context;"); + expect(cache.get(expr).toString()).toContain("let { hello, res } = context;"); + }); + + it("should evaluate expressions referencing globals", () => { + const startTime = new Date("2022-02-01").getTime(); + const currentTime = new Date("2022-02-02").getTime(); + + jest.useFakeTimers({ now: currentTime }); + + const expr = "Math.max(Date.now(), startTime)"; + const result = evaluateJsExpression(expr, { startTime }); + + expect(result).toBe(currentTime); + + expect(cache.get(expr).toString()).toContain("Math = Math ?? globalThis.Math;"); + expect(cache.get(expr).toString()).toContain("Date = Date ?? globalThis.Date;"); + }); + + it("should use global overridden in context", () => { + const startTime = new Date("2022-02-01").getTime(); + const currentTime = new Date("2022-02-02").getTime(); + + jest.useFakeTimers({ now: currentTime }); + + const context = { + Date: { now: () => new Date("2022-01-31").getTime() }, + startTime + }; + + const expr = "Math.max(Date.now(), startTime)"; + const result = evaluateJsExpression(expr, context); + + expect(result).toBe(startTime); }); }); });