diff --git a/v3/package-lock.json b/v3/package-lock.json index 420461ac3f..a6f2724230 100644 --- a/v3/package-lock.json +++ b/v3/package-lock.json @@ -25,7 +25,7 @@ "iframe-phone": "^1.3.1", "leaflet": "^1.9.4", "lodash": "^4.17.21", - "mathjs": "12.3.1", + "mathjs": "12.4.3", "mime": "^4.0.4", "mobx": "^6.13.0", "mobx-react-lite": "^4.0.7", @@ -18339,11 +18339,11 @@ } }, "node_modules/mathjs": { - "version": "12.3.1", - "resolved": "https://registry.npmjs.org/mathjs/-/mathjs-12.3.1.tgz", - "integrity": "sha512-Z8G8Cunz97rV32eghW/twFFSY/dmU2eswnh6cAkcQMbbEUUpaweF0li435GkZjRS+G52VYotN+vLx3VTYBf5WA==", + "version": "12.4.3", + "resolved": "https://registry.npmjs.org/mathjs/-/mathjs-12.4.3.tgz", + "integrity": "sha512-oHdGPDbp7gO873xxG90RLq36IuicuKvbpr/bBG5g9c8Obm/VsKVrK9uoRZZHUodohzlnmCEqfDzbR3LH6m+aAQ==", "dependencies": { - "@babel/runtime": "^7.23.9", + "@babel/runtime": "^7.24.4", "complex.js": "^2.1.1", "decimal.js": "^10.4.3", "escape-latex": "^1.2.0", @@ -38190,11 +38190,11 @@ } }, "mathjs": { - "version": "12.3.1", - "resolved": "https://registry.npmjs.org/mathjs/-/mathjs-12.3.1.tgz", - "integrity": "sha512-Z8G8Cunz97rV32eghW/twFFSY/dmU2eswnh6cAkcQMbbEUUpaweF0li435GkZjRS+G52VYotN+vLx3VTYBf5WA==", + "version": "12.4.3", + "resolved": "https://registry.npmjs.org/mathjs/-/mathjs-12.4.3.tgz", + "integrity": "sha512-oHdGPDbp7gO873xxG90RLq36IuicuKvbpr/bBG5g9c8Obm/VsKVrK9uoRZZHUodohzlnmCEqfDzbR3LH6m+aAQ==", "requires": { - "@babel/runtime": "^7.23.9", + "@babel/runtime": "^7.24.4", "complex.js": "^2.1.1", "decimal.js": "^10.4.3", "escape-latex": "^1.2.0", diff --git a/v3/package.json b/v3/package.json index f75179560d..57154beb46 100644 --- a/v3/package.json +++ b/v3/package.json @@ -191,7 +191,7 @@ "iframe-phone": "^1.3.1", "leaflet": "^1.9.4", "lodash": "^4.17.21", - "mathjs": "12.3.1", + "mathjs": "12.4.3", "mime": "^4.0.4", "mobx": "^6.13.0", "mobx-react-lite": "^4.0.7", diff --git a/v3/src/models/formula/formula-mathjs-scope.ts b/v3/src/models/formula/formula-mathjs-scope.ts index 44f2bffad8..c8ad54cb5f 100644 --- a/v3/src/models/formula/formula-mathjs-scope.ts +++ b/v3/src/models/formula/formula-mathjs-scope.ts @@ -129,14 +129,6 @@ export class FormulaMathJsScope { throw new Error("It's not allowed to clear values in the formula scope.") } - // MathJS requests a separate subscope for every parsed and evaluated function call. - createSubScope () { - // In regular MathJS use case, a subscope is used to create a new scope for a function call. It needs to ensure - // that variables from subscope cannot overwrite variables from the parent scope. However, since we don't allow - // any variables to be set in the formula scope, we can reuse the same scope instance. - return this - } - // --- Custom functions used by our formulas or formula manager -- setExtraScope(extraScope?: Map) { diff --git a/v3/src/models/formula/formula-types.ts b/v3/src/models/formula/formula-types.ts index a9487686aa..d19217d78b 100644 --- a/v3/src/models/formula/formula-types.ts +++ b/v3/src/models/formula/formula-types.ts @@ -41,7 +41,14 @@ export type EvaluateFunc = (...args: FValue[]) => FValue export type EvaluateFuncWithAggregateContextSupport = (...args: (FValueOrArray)[]) => FValueOrArray -export type EvaluateRawFunc = (args: MathNode[], mathjs: any, scope: FormulaMathJsScope) => FValueOrArray +// MathJS v12.3.2 introduced the concept of PartitionedMap, replacing the previous concepts of scopes and sub-scopes. +// The parent scope is still available as the `a` property of the PartitionedMap. +// For more information, see: https://github.com/josdejong/mathjs/pull/3150 +export type MathJSPartitionedMap = { a: CurrentScope, b: Map} + +export type CurrentScope = MathJSPartitionedMap | FormulaMathJsScope + +export type EvaluateRawFunc = (args: MathNode[], mathjs: any, currentScope: CurrentScope) => FValueOrArray export type CaseList = ICase[] | "ALL_CASES" diff --git a/v3/src/models/formula/functions/aggregate-functions.test.ts b/v3/src/models/formula/functions/aggregate-functions.test.ts index 52f64cc5fa..eef56aa868 100644 --- a/v3/src/models/formula/functions/aggregate-functions.test.ts +++ b/v3/src/models/formula/functions/aggregate-functions.test.ts @@ -151,25 +151,26 @@ describe("cachedAggregateFnFactory", () => { setCached: (key: string, val: any) => cache.set(key, val), getCached: (key: string) => cache.get(key) } as any as FormulaMathJsScope + const currentScope = { a: scope, b: new Map() } - expect(cachedFn([ parse("1"), parse("2") ], null, scope)).toEqual(123) + expect(cachedFn([ parse("1"), parse("2") ], null, currentScope)).toEqual(123) expect(fn).toHaveBeenCalledTimes(1) - expect(cachedFn([ parse("1"), parse("2") ], null, scope)).toEqual(123) + expect(cachedFn([ parse("1"), parse("2") ], null, currentScope)).toEqual(123) expect(fn).toHaveBeenCalledTimes(1) // Same arguments as in the previous call, cache should be used. - expect(cachedFn([ parse("1"), parse("2"), parse("3") ], null, scope)).toEqual(123) + expect(cachedFn([ parse("1"), parse("2"), parse("3") ], null, currentScope)).toEqual(123) expect(fn).toHaveBeenCalledTimes(2) // Different arguments, so cache should not be used. - expect(cachedFn([ parse("1"), parse("2"), parse("3") ], null, scope)).toEqual(123) + expect(cachedFn([ parse("1"), parse("2"), parse("3") ], null, currentScope)).toEqual(123) expect(fn).toHaveBeenCalledTimes(2) // Same arguments as in the previous call, cache should be used. // Update scope.getCaseAggregateGroupId ;(scope.getCaseAggregateGroupId as jest.Mock).mockImplementation(() => "newTestGroup") - expect(cachedFn([ parse("1"), parse("2"), parse("3") ], null, scope)).toEqual(123) + expect(cachedFn([ parse("1"), parse("2"), parse("3") ], null, currentScope)).toEqual(123) expect(fn).toHaveBeenCalledTimes(3) // New caseAggregateGroupId, so cache should not be used. - expect(cachedFn([ parse("1"), parse("2"), parse("3") ], null, scope)).toEqual(123) + expect(cachedFn([ parse("1"), parse("2"), parse("3") ], null, currentScope)).toEqual(123) expect(fn).toHaveBeenCalledTimes(3) // Same arguments and caseAggregateGroupId, cache should be used. }) }) diff --git a/v3/src/models/formula/functions/aggregate-functions.ts b/v3/src/models/formula/functions/aggregate-functions.ts index 9b67752490..111f9ca6a4 100644 --- a/v3/src/models/formula/functions/aggregate-functions.ts +++ b/v3/src/models/formula/functions/aggregate-functions.ts @@ -1,18 +1,18 @@ import { MathNode, mad, max, mean, median, min, sum } from "mathjs" -import { FormulaMathJsScope } from "../formula-mathjs-scope" -import { FValue, FValueOrArray } from "../formula-types" -import { UNDEF_RESULT, evaluateNode, isNumber, isValueTruthy } from "./function-utils" +import { CurrentScope, FValue, FValueOrArray } from "../formula-types" +import { UNDEF_RESULT, evaluateNode, getRootScope, isNumber, isValueTruthy } from "./function-utils" // Almost every aggregate function can be cached in the same way. export const cachedAggregateFnFactory = -(fnName: string, fn: (args: MathNode[], mathjs: any, scope: FormulaMathJsScope) => FValueOrArray) => { - return (args: MathNode[], mathjs: any, scope: FormulaMathJsScope) => { +(fnName: string, fn: (args: MathNode[], mathjs: any, currentScope: CurrentScope) => FValueOrArray) => { + return (args: MathNode[], mathjs: any, currentScope: CurrentScope) => { + const scope = getRootScope(currentScope) const cacheKey = `${fnName}(${args.toString()})-${scope.getCaseAggregateGroupId()}` const cachedValue = scope.getCached(cacheKey) if (cachedValue !== undefined) { return cachedValue } - const result = fn(args, mathjs, scope) + const result = fn(args, mathjs, currentScope) scope.setCached(cacheKey, result) return result } @@ -21,7 +21,8 @@ export const cachedAggregateFnFactory = // Note that aggregate functions like mean, max, min, etc., all have exactly the same signature and implementation. // The only difference is the final math operation applies to the expression results. const aggregateFnWithFilterFactory = (fn: (values: number[]) => FValue) => { - return (args: MathNode[], mathjs: any, scope: FormulaMathJsScope) => { + return (args: MathNode[], mathjs: any, currentScope: CurrentScope) => { + const scope = getRootScope(currentScope) const [ expression, filter ] = args let expressionValues = evaluateNode(expression, scope) if (!Array.isArray(expressionValues)) { @@ -96,7 +97,8 @@ export const aggregateFunctions = { // arguments, the default caching method would calculate incorrect cache key. Hence, caching is implemented directly // in the function body. cachedEvaluateFactory: undefined, - evaluateRaw: (args: MathNode[], mathjs: any, scope: FormulaMathJsScope) => { + evaluateRaw: (args: MathNode[], mathjs: any, currentScope: CurrentScope) => { + const scope = getRootScope(currentScope) const [expression, filter] = args if (!expression) { // Special case: count() without arguments returns number of children cases. Note that this cannot be cached diff --git a/v3/src/models/formula/functions/function-utils.test.ts b/v3/src/models/formula/functions/function-utils.test.ts index b21779a826..c17d2113bf 100644 --- a/v3/src/models/formula/functions/function-utils.test.ts +++ b/v3/src/models/formula/functions/function-utils.test.ts @@ -1,5 +1,7 @@ -import { isValueTruthy, equal, evaluateNode } from "./function-utils" +import { isValueTruthy, equal, evaluateNode, isPartitionedMap, isMap, getRootScope } from "./function-utils" import { parse } from "mathjs" +import { CurrentScope } from "../formula-types" +import { FormulaMathJsScope } from "../formula-mathjs-scope" describe("isValueTruthy", () => { it("should return true for truthy values", () => { @@ -42,3 +44,39 @@ describe("evaluateNode", () => { expect(evaluateNode(node)).toBe(3) }) }) + +describe("isPartitionedMap", () => { + it("should return true for partitioned map", () => { + const map = { a: {}, b: new Map() } + expect(isPartitionedMap(map)).toBe(true) + }) + + it("should return false for non-partitioned map", () => { + const map = new Map() + expect(isPartitionedMap(map)).toBe(false) + }) +}) + +describe("isMap", () => { + it("should return true for map", () => { + const map = new Map() + expect(isMap(map)).toBe(true) + }) + + it("should return false for non-map", () => { + const map = {} + expect(isMap(map)).toBe(false) + }) +}) + +describe("getRootScope", () => { + it("should return root scope for partitioned map", () => { + const scope = { a: {}, b: new Map() } + expect(getRootScope(scope as any as CurrentScope)).toBe(scope.a) + }) + + it("should return scope for non-partitioned map", () => { + const scope = {} as any as FormulaMathJsScope + expect(getRootScope(scope)).toBe(scope) + }) +}) diff --git a/v3/src/models/formula/functions/function-utils.ts b/v3/src/models/formula/functions/function-utils.ts index aa4a811fbe..cd72b438e8 100644 --- a/v3/src/models/formula/functions/function-utils.ts +++ b/v3/src/models/formula/functions/function-utils.ts @@ -1,6 +1,7 @@ import { MathNode } from "mathjs" import { FormulaMathJsScope } from "../formula-mathjs-scope" import { isValueNonEmpty } from "../../../utilities/math-utils" +import { CurrentScope, MathJSPartitionedMap } from "../formula-types" export { isNumber, isValueNonEmpty } from "../../../utilities/math-utils" export const UNDEF_RESULT = "" @@ -33,3 +34,23 @@ equal = (a: any, b: any): boolean => { export const evaluateNode = (node: MathNode, scope?: FormulaMathJsScope) => { return node.compile().evaluate(scope) } + +// This function is used to get the root scope (an instance of our FormulaMathJSScope) within custom MathJS functions. +// When the formula expression is executed, the initially passed scope can be wrapped in MathJS's PartitionedMap, which +// is used to store temporary values. This function retrieves the root scope from the PartitionedMap. +// This approach has been recommended by the MathJS maintainer. For more details, see: +// https://github.com/josdejong/mathjs/pull/3150#issuecomment-2248101774 +export const getRootScope = (currentScope: CurrentScope): FormulaMathJsScope => { + return isPartitionedMap(currentScope) ? getRootScope(currentScope.a) : currentScope +} + +export const isPartitionedMap = (map: any): map is MathJSPartitionedMap => { + // We could also check isMap(map.a), but that makes tests and mocking more difficult. + // The current check is probably specific enough to be safe. + return map && typeof map.a === "object" && isMap(map.b) +} + +export const isMap = (map: any) => { + const requiredMapMethods = ['get', 'set', 'has', 'delete', 'entries'] + return map && typeof map === 'object' && requiredMapMethods.every(method => typeof map[method] === 'function') +} diff --git a/v3/src/models/formula/functions/lookup-functions.ts b/v3/src/models/formula/functions/lookup-functions.ts index 1732f47f70..3bbdb5f5c9 100644 --- a/v3/src/models/formula/functions/lookup-functions.ts +++ b/v3/src/models/formula/functions/lookup-functions.ts @@ -1,8 +1,7 @@ import { ConstantNode, MathNode } from "mathjs" -import { FormulaMathJsScope } from "../formula-mathjs-scope" -import { DisplayNameMap, FValue, ILookupDependency } from "../formula-types" +import { CurrentScope, DisplayNameMap, FValue, ILookupDependency } from "../formula-types" import { rmCanonicalPrefix } from "../utils/name-mapping-utils" -import { UNDEF_RESULT, equal, evaluateNode } from "./function-utils" +import { UNDEF_RESULT, equal, evaluateNode, getRootScope } from "./function-utils" import { isConstantStringNode } from "../utils/mathjs-utils" import { t } from "../../../utilities/translation/translate" import type { IDataSet } from "../../data/data-set" @@ -32,7 +31,8 @@ export const lookupFunctions = { attrNameArg.value = displayNameMap.dataSet[dataSetName]?.attribute[attrName] || attrName } }, - evaluateRaw: (args: MathNode[], mathjs: any, scope: FormulaMathJsScope) => { + evaluateRaw: (args: MathNode[], mathjs: any, currentScope: CurrentScope) => { + const scope = getRootScope(currentScope) const functionName = "lookupByIndex" const numOfReqArgs = lookupFunctions.lookupByIndex.numOfRequiredArguments if (args.length !== numOfReqArgs) { @@ -91,7 +91,8 @@ export const lookupFunctions = { keyAttrNameArg.value = displayNameMap.dataSet[dataSetName]?.attribute[keyAttrName] || keyAttrName } }, - evaluateRaw: (args: MathNode[], mathjs: any, scope: FormulaMathJsScope) => { + evaluateRaw: (args: MathNode[], mathjs: any, currentScope: CurrentScope) => { + const scope = getRootScope(currentScope) const functionName = "lookupByKey" const numOfReqArgs = lookupFunctions.lookupByKey.numOfRequiredArguments if (args.length !== numOfReqArgs) { diff --git a/v3/src/models/formula/functions/math.test.ts b/v3/src/models/formula/functions/math.test.ts index 8515256564..0b6a43d16f 100644 --- a/v3/src/models/formula/functions/math.test.ts +++ b/v3/src/models/formula/functions/math.test.ts @@ -1,9 +1,8 @@ import { MathNode, SymbolNode, parse } from "mathjs" -import { FormulaMathJsScope } from "../formula-mathjs-scope" import { evaluateRawWithAggregateContext, evaluateRawWithDefaultArg, evaluateToEvaluateRaw, evaluateWithAggregateContextSupport } from "./math" -import { FValue, FValueOrArray } from "../formula-types" +import { FValue, FValueOrArray, MathJSPartitionedMap } from "../formula-types" describe("evaluateRawWithAggregateContext", () => { it("should call provided function within withAggregateContext", () => { @@ -17,11 +16,12 @@ describe("evaluateRawWithAggregateContext", () => { scope.aggregateContext = false } } + const currentScope = { a: scope, b: new Map() } as any as MathJSPartitionedMap let result = false const mockFn = jest.fn(() => { result = scope.aggregateContext; return "" }) - evaluateRawWithAggregateContext(mockFn)(args, mathjs, scope as any as FormulaMathJsScope) - expect(mockFn).toHaveBeenCalledWith(args, mathjs, scope) + evaluateRawWithAggregateContext(mockFn)(args, mathjs, currentScope) + expect(mockFn).toHaveBeenCalledWith(args, mathjs, currentScope) expect(result).toBeTruthy() }) }) @@ -33,11 +33,12 @@ describe("evaluateRawWithDefaultArg", () => { const scope = { defaultArgumentNode: { name: "default" } } + const currentScope = { a: scope, b: new Map() } as any as MathJSPartitionedMap const mockFn = jest.fn((_args: MathNode[]) => (_args[0] as SymbolNode).name) const numOfReqArgs = 1 - const res = evaluateRawWithDefaultArg(mockFn, numOfReqArgs)(args, mathjs, scope as any as FormulaMathJsScope) - expect(mockFn).toHaveBeenCalledWith([scope.defaultArgumentNode], mathjs, scope) + const res = evaluateRawWithDefaultArg(mockFn, numOfReqArgs)(args, mathjs, currentScope) + expect(mockFn).toHaveBeenCalledWith([scope.defaultArgumentNode], mathjs, currentScope) expect(res).toEqual("default") }) @@ -47,11 +48,12 @@ describe("evaluateRawWithDefaultArg", () => { const scope = { defaultArgumentNode: { name: "default" } } + const currentScope = { a: scope, b: new Map() } as any as MathJSPartitionedMap const mockFn = jest.fn((_args: MathNode[]) => (_args[0] as SymbolNode).name) const res = - evaluateRawWithDefaultArg(mockFn, 1)(args as any as MathNode[], mathjs, scope as any as FormulaMathJsScope) - expect(mockFn).toHaveBeenCalledWith(args, mathjs, scope) + evaluateRawWithDefaultArg(mockFn, 1)(args as any as MathNode[], mathjs, currentScope) + expect(mockFn).toHaveBeenCalledWith(args, mathjs, currentScope) expect(res).toEqual("provided") }) }) @@ -61,9 +63,10 @@ describe("evaluateToEvaluateRaw", () => { const args = [ parse("1"), parse("2") ] const mathjs = {} const scope = {} + const currentScope = { a: scope, b: new Map() } as any as MathJSPartitionedMap const mockFn = jest.fn((a: FValueOrArray, b: FValueOrArray) => Number(a) + Number(b)) - const res = evaluateToEvaluateRaw(mockFn)(args as any as MathNode[], mathjs, scope as any as FormulaMathJsScope) + const res = evaluateToEvaluateRaw(mockFn)(args as any as MathNode[], mathjs, currentScope) expect(mockFn).toHaveBeenCalledWith(1, 2) expect(res).toEqual(3) }) diff --git a/v3/src/models/formula/functions/math.ts b/v3/src/models/formula/functions/math.ts index b52f2429fe..d3917491c6 100644 --- a/v3/src/models/formula/functions/math.ts +++ b/v3/src/models/formula/functions/math.ts @@ -1,10 +1,9 @@ import { create, all, MathNode } from 'mathjs' -import { FormulaMathJsScope } from '../formula-mathjs-scope' import { - CODAPMathjsFunctionRegistry, EvaluateFunc, EvaluateFuncWithAggregateContextSupport, EvaluateRawFunc, FValue, - FValueOrArray + CODAPMathjsFunctionRegistry, CurrentScope, EvaluateFunc, EvaluateFuncWithAggregateContextSupport, EvaluateRawFunc, + FValue, FValueOrArray } from '../formula-types' -import { evaluateNode } from './function-utils' +import { evaluateNode, getRootScope } from './function-utils' import { arithmeticFunctions } from './arithmetic-functions' import { dateFunctions } from './date-functions' import { stringFunctions } from './string-functions' @@ -18,23 +17,26 @@ export const math = create(all) // Each aggregate function needs to be evaluated with `withAggregateContext` method. export const evaluateRawWithAggregateContext = (fn: EvaluateRawFunc): EvaluateRawFunc => { - return (args: MathNode[], mathjs: any, scope: FormulaMathJsScope) => { + return (args: MathNode[], mathjs: any, currentScope: CurrentScope) => { + const scope = getRootScope(currentScope) // withAggregateContext returns result of the callback function - return scope.withAggregateContext(() => fn(args, mathjs, scope)) + return scope.withAggregateContext(() => fn(args, mathjs, currentScope)) } } export const evaluateRawWithDefaultArg = (fn: EvaluateRawFunc, numOfRequiredArguments: number): EvaluateRawFunc => { - return (args: MathNode[], mathjs: any, scope: FormulaMathJsScope) => { + return (args: MathNode[], mathjs: any, currentScope: CurrentScope) => { + const scope = getRootScope(currentScope) if (scope.defaultArgumentNode && args.length < numOfRequiredArguments) { - return fn([...args, scope.defaultArgumentNode], mathjs, scope) + return fn([...args, scope.defaultArgumentNode], mathjs, currentScope) } - return fn(args, mathjs, scope) + return fn(args, mathjs, currentScope) } } export const evaluateToEvaluateRaw = (fn: EvaluateFuncWithAggregateContextSupport): EvaluateRawFunc => { - return (args: MathNode[], mathjs: any, scope: FormulaMathJsScope) => { + return (args: MathNode[], mathjs: any, currentScope: CurrentScope) => { + const scope = getRootScope(currentScope) return fn(...(args.map(arg => evaluateNode(arg, scope)))) } } diff --git a/v3/src/models/formula/functions/semi-aggregate-functions.ts b/v3/src/models/formula/functions/semi-aggregate-functions.ts index 774d013648..a37db4dde4 100644 --- a/v3/src/models/formula/functions/semi-aggregate-functions.ts +++ b/v3/src/models/formula/functions/semi-aggregate-functions.ts @@ -1,19 +1,19 @@ import { MathNode } from "mathjs" -import { FormulaMathJsScope } from "../formula-mathjs-scope" -import { FValue } from "../formula-types" -import { UNDEF_RESULT, evaluateNode, isValueTruthy } from "./function-utils" +import { CurrentScope, FValue } from "../formula-types" +import { UNDEF_RESULT, evaluateNode, getRootScope, isValueTruthy } from "./function-utils" export const semiAggregateFunctions = { next: { numOfRequiredArguments: 1, // expression and filter are evaluated as aggregate symbols, defaultValue is not - it depends on case index isSemiAggregate: [true, false, true], - evaluateRaw: (args: MathNode[], mathjs: any, scope: FormulaMathJsScope) => { + evaluateRaw: (args: MathNode[], mathjs: any, currentScope: CurrentScope) => { interface ICachedData { result?: FValue resultCasePointer: number } + const scope = getRootScope(currentScope) const caseGroupId = scope.getCaseGroupId() const cacheKey = `next(${args.toString()})-${caseGroupId}` const [ expression, defaultValue, filter ] = args @@ -75,7 +75,8 @@ export const semiAggregateFunctions = { selfReferenceAllowed: true, // expression and filter are evaluated as aggregate symbols, defaultValue is not - it depends on case index isSemiAggregate: [true, false, true], - evaluateRaw: (args: MathNode[], mathjs: any, scope: FormulaMathJsScope) => { + evaluateRaw: (args: MathNode[], mathjs: any, currentScope: CurrentScope) => { + const scope = getRootScope(currentScope) const [ expression, defaultValue, filter ] = args const caseGroupId = scope.getCaseGroupId()