fix: passing params from JS to API/SQL query (#10826)

* Revert "fix: revert this.params solution (#10322)"

This reverts commit 2bcd73e41d.

* replace 'this.params' with 'executionParams'

* replace 'this?.params' also with 'executionParams'

* fix unit test lint errors

* added unit tests for params handling

* evaluateActionBindings unit test - add default value case

* comments update

* remove un-necessary `executionparams` assigment to `evalTree`
This commit is contained in:
Anand Srinivasan 2022-02-04 17:58:46 +05:30 committed by GitHub
parent 1bcf344154
commit c2a6e089db
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 199 additions and 33 deletions

View File

@ -118,7 +118,8 @@ export interface ExecuteErrorPayload extends ErrorActionPayload {
export const urlGroupsRegexExp = /^(https?:\/{2}\S+?)(\/[\s\S]*?)?(\?(?![^{]*})[\s\S]*)?$/;
export const EXECUTION_PARAM_KEY = "executionParams";
export const EXECUTION_PARAM_REFERENCE_REGEX = /this.params/g;
export const EXECUTION_PARAM_REFERENCE_REGEX = /this.params|this\?.params/g;
export const THIS_DOT_PARAMS_KEY = "params";
export const RESP_HEADER_DATATYPE = "X-APPSMITH-DATATYPE";
export const API_REQUEST_HEADERS: APIHeaders = {

View File

@ -0,0 +1,109 @@
import DataTreeEvaluator from "./DataTreeEvaluator";
describe("DataTreeEvaluator", () => {
let dataTreeEvaluator: DataTreeEvaluator;
beforeAll(() => {
dataTreeEvaluator = new DataTreeEvaluator({});
});
describe("evaluateActionBindings", () => {
it("handles this.params.property", () => {
const result = dataTreeEvaluator.evaluateActionBindings(
[
"(function() { return this.params.property })()",
"(() => { return this.params.property })()",
'this.params.property || "default value"',
'this.params.property1 || "default value"',
],
{
property: "my value",
},
);
expect(result).toStrictEqual([
"my value",
"my value",
"my value",
"default value",
]);
});
it("handles this?.params.property", () => {
const result = dataTreeEvaluator.evaluateActionBindings(
[
"(() => { return this?.params.property })()",
"(function() { return this?.params.property })()",
'this?.params.property || "default value"',
'this?.params.property1 || "default value"',
],
{
property: "my value",
},
);
expect(result).toStrictEqual([
"my value",
"my value",
"my value",
"default value",
]);
});
it("handles this?.params?.property", () => {
const result = dataTreeEvaluator.evaluateActionBindings(
[
"(() => { return this?.params?.property })()",
"(function() { return this?.params?.property })()",
'this?.params?.property || "default value"',
'this?.params?.property1 || "default value"',
],
{
property: "my value",
},
);
expect(result).toStrictEqual([
"my value",
"my value",
"my value",
"default value",
]);
});
it("handles executionParams.property", () => {
const result = dataTreeEvaluator.evaluateActionBindings(
[
"(function() { return executionParams.property })()",
"(() => { return executionParams.property })()",
'executionParams.property || "default value"',
'executionParams.property1 || "default value"',
],
{
property: "my value",
},
);
expect(result).toStrictEqual([
"my value",
"my value",
"my value",
"default value",
]);
});
it("handles executionParams?.property", () => {
const result = dataTreeEvaluator.evaluateActionBindings(
[
"(function() { return executionParams?.property })()",
"(() => { return executionParams?.property })()",
'executionParams?.property || "default value"',
'executionParams?.property1 || "default value"',
],
{
property: "my value",
},
);
expect(result).toStrictEqual([
"my value",
"my value",
"my value",
"default value",
]);
});
});
});

View File

@ -55,11 +55,13 @@ import toposort from "toposort";
import {
EXECUTION_PARAM_KEY,
EXECUTION_PARAM_REFERENCE_REGEX,
THIS_DOT_PARAMS_KEY,
} from "constants/AppsmithActionConstants/ActionConstants";
import { DATA_BIND_REGEX } from "constants/BindingsConstants";
import evaluateSync, {
createGlobalData,
EvalResult,
EvaluateContext,
EvaluationScriptType,
getScriptToEval,
evaluateAsync,
@ -609,12 +611,19 @@ export default class DataTreeEvaluator {
entity.bindingPaths[propertyPath] ||
EvaluationSubstitutionType.TEMPLATE;
const contextData: EvaluateContext = {};
if (isAction(entity)) {
contextData.thisContext = {
params: {},
};
}
try {
evalPropertyValue = this.getDynamicValue(
unEvalPropertyValue,
currentTree,
resolvedFunctions,
evaluationSubstitutionType,
contextData,
undefined,
fullPropertyPath,
);
@ -764,6 +773,7 @@ export default class DataTreeEvaluator {
data: DataTree,
resolvedFunctions: Record<string, any>,
evaluationSubstitutionType: EvaluationSubstitutionType,
contextData?: EvaluateContext,
callBackData?: Array<any>,
fullPropertyPath?: string,
) {
@ -792,6 +802,7 @@ export default class DataTreeEvaluator {
toBeSentForEval,
data,
resolvedFunctions,
contextData,
callBackData,
);
if (fullPropertyPath && result.errors.length) {
@ -864,10 +875,17 @@ export default class DataTreeEvaluator {
js: string,
data: DataTree,
resolvedFunctions: Record<string, any>,
contextData?: EvaluateContext,
callbackData?: Array<any>,
): EvalResult {
try {
return evaluateSync(js, data, resolvedFunctions, callbackData);
return evaluateSync(
js,
data,
resolvedFunctions,
contextData,
callbackData,
);
} catch (e) {
return {
result: undefined,
@ -1555,24 +1573,26 @@ export default class DataTreeEvaluator {
);
}
// Replace any reference of 'this.params' to 'executionParams' (backwards compatibility)
const bindingsForExecutionParams: string[] = bindings.map(
(binding: string) =>
binding.replace(EXECUTION_PARAM_REFERENCE_REGEX, EXECUTION_PARAM_KEY),
);
const dataTreeWithExecutionParams = Object.assign({}, this.evalTree, {
[EXECUTION_PARAM_KEY]: evaluatedExecutionParams,
});
return bindingsForExecutionParams.map((binding) =>
this.getDynamicValue(
`{{${binding}}}`,
dataTreeWithExecutionParams,
return bindings.map((binding) => {
// Replace any reference of 'this.params' to 'executionParams' (backwards compatibility)
// also helps with dealing with IIFE which are normal functions (not arrow)
// because normal functions won't retain 'this' context (when executed elsewhere)
const replacedBinding = binding.replace(
EXECUTION_PARAM_REFERENCE_REGEX,
EXECUTION_PARAM_KEY,
);
return this.getDynamicValue(
`{{${replacedBinding}}}`,
this.evalTree,
this.resolvedFunctions,
EvaluationSubstitutionType.TEMPLATE,
),
);
// params can be accessed via "this.params" or "executionParams"
{
thisContext: { [THIS_DOT_PARAMS_KEY]: evaluatedExecutionParams },
globalContext: { [EXECUTION_PARAM_KEY]: evaluatedExecutionParams },
},
);
});
}
clearErrors() {

View File

@ -30,6 +30,9 @@ describe("evaluateSync", () => {
triggerPaths: {},
validationPaths: {},
logBlackList: {},
overridingPropertyPaths: {},
privateWidgets: {},
propertyOverrideDependency: {},
};
const dataTree: DataTree = {
Input1: widget,
@ -75,7 +78,7 @@ describe("evaluateSync", () => {
const result = wrongJS
return result;
}
closedFunction()
closedFunction.call(THIS_CONTEXT)
`,
severity: "error",
originalBinding: "wrongJS",
@ -89,7 +92,7 @@ describe("evaluateSync", () => {
const result = wrongJS
return result;
}
closedFunction()
closedFunction.call(THIS_CONTEXT)
`,
severity: "error",
originalBinding: "wrongJS",
@ -108,7 +111,7 @@ describe("evaluateSync", () => {
const result = {}.map()
return result;
}
closedFunction()
closedFunction.call(THIS_CONTEXT)
`,
severity: "error",
originalBinding: "{}.map()",
@ -135,7 +138,7 @@ describe("evaluateSync", () => {
const result = setTimeout(() => {}, 100)
return result;
}
closedFunction()
closedFunction.call(THIS_CONTEXT)
`,
severity: "error",
originalBinding: "setTimeout(() => {}, 100)",
@ -151,7 +154,7 @@ describe("evaluateSync", () => {
it("evaluates functions with callback data", () => {
const js = "(arg1, arg2) => arg1.value + arg2";
const callbackData = [{ value: "test" }, "1"];
const response = evaluate(js, dataTree, {}, callbackData);
const response = evaluate(js, dataTree, {}, {}, callbackData);
expect(response.result).toBe("test1");
});
it("handles EXPRESSIONS with new lines", () => {
@ -165,22 +168,38 @@ describe("evaluateSync", () => {
});
it("handles TRIGGERS with new lines", () => {
let js = "\n";
let response = evaluate(js, dataTree, {}, undefined);
let response = evaluate(js, dataTree, {}, undefined, undefined);
expect(response.errors.length).toBe(0);
js = "\n\n\n";
response = evaluate(js, dataTree, {}, undefined);
response = evaluate(js, dataTree, {}, undefined, undefined);
expect(response.errors.length).toBe(0);
});
it("handles ANONYMOUS_FUNCTION with new lines", () => {
let js = "\n";
let response = evaluate(js, dataTree, {}, undefined);
let response = evaluate(js, dataTree, {}, undefined, undefined);
expect(response.errors.length).toBe(0);
js = "\n\n\n";
response = evaluate(js, dataTree, {}, undefined);
response = evaluate(js, dataTree, {}, undefined, undefined);
expect(response.errors.length).toBe(0);
});
it("has access to this context", () => {
const js = "this.contextVariable";
const thisContext = { contextVariable: "test" };
const response = evaluate(js, dataTree, {}, { thisContext });
expect(response.result).toBe("test");
// there should not be any error when accessing "this" variables
expect(response.errors).toHaveLength(0);
});
it("has access to additional global context", () => {
const js = "contextVariable";
const globalContext = { contextVariable: "test" };
const response = evaluate(js, dataTree, {}, { globalContext });
expect(response.result).toBe("test");
expect(response.errors).toHaveLength(0);
});
});
describe("evaluateAsync", () => {
@ -256,7 +275,7 @@ describe("isFunctionAsync", () => {
if (typeof testFunc === "string") {
testFunc = eval(testFunc);
}
const actual = isFunctionAsync(testFunc, {});
const actual = isFunctionAsync(testFunc, {}, {});
expect(actual).toBe(testCase.expected);
}
});

View File

@ -34,12 +34,12 @@ export const EvaluationScripts: Record<EvaluationScriptType, string> = {
const result = ${ScriptTemplate}
return result;
}
closedFunction()
closedFunction.call(THIS_CONTEXT)
`,
[EvaluationScriptType.ANONYMOUS_FUNCTION]: `
function callback (script) {
const userFunction = script;
const result = userFunction?.apply(self, ARGUMENTS);
const result = userFunction?.apply(THIS_CONTEXT, ARGUMENTS);
return result;
}
callback(${ScriptTemplate})
@ -49,7 +49,7 @@ export const EvaluationScripts: Record<EvaluationScriptType, string> = {
const result = await ${ScriptTemplate};
return result;
}
closedFunction();
closedFunction.call(THIS_CONTEXT);
`,
};
@ -102,6 +102,18 @@ export const createGlobalData = (
const GLOBAL_DATA: Record<string, any> = {};
///// Adding callback data
GLOBAL_DATA.ARGUMENTS = evalArguments;
//// Adding contextual data not part of data tree
GLOBAL_DATA.THIS_CONTEXT = {};
if (context) {
if (context.thisContext) {
GLOBAL_DATA.THIS_CONTEXT = context.thisContext;
}
if (context.globalContext) {
Object.entries(context.globalContext).forEach(([key, value]) => {
GLOBAL_DATA[key] = value;
});
}
}
//// Add internal functions to dataTree;
const dataTreeWithFunctions = enhanceDataTreeWithFunctions(
dataTree,
@ -134,9 +146,13 @@ export function sanitizeScript(js: string) {
}
/** Define a context just for this script
* thisContext will define it on the `this`
* globalContext will define it globally
* requestId is used for completing promises
*/
export type EvaluateContext = {
thisContext?: Record<string, any>;
globalContext?: Record<string, any>;
requestId?: string;
};
@ -172,6 +188,7 @@ export default function evaluateSync(
userScript: string,
dataTree: DataTree,
resolvedFunctions: Record<string, any>,
context?: EvaluateContext,
evalArguments?: Array<any>,
): EvalResult {
return (function() {
@ -181,7 +198,7 @@ export default function evaluateSync(
const GLOBAL_DATA: Record<string, any> = createGlobalData(
dataTree,
resolvedFunctions,
undefined,
context,
evalArguments,
);
GLOBAL_DATA.ALLOW_ASYNC = false;

View File

@ -799,7 +799,7 @@ export const VALIDATORS: Record<ValidationTypes, Validator> = {
};
if (config.params?.fnString && isString(config.params?.fnString)) {
try {
const { result } = evaluate(config.params.fnString, {}, {}, [
const { result } = evaluate(config.params.fnString, {}, {}, undefined, [
value,
props,
_,