diff --git a/app/client/src/ce/workers/Evaluation/__tests__/dataTreeUtils.test.ts b/app/client/src/ce/workers/Evaluation/__tests__/dataTreeUtils.test.ts index 5ebf71f292..b27f4caee4 100644 --- a/app/client/src/ce/workers/Evaluation/__tests__/dataTreeUtils.test.ts +++ b/app/client/src/ce/workers/Evaluation/__tests__/dataTreeUtils.test.ts @@ -267,94 +267,5 @@ describe("7. Test util methods", () => { expect(evalProps).toEqual(initialEvalProps); }); }); - - describe("serialise", () => { - it("should clean out all functions in the generated state", () => { - const state = { - Table1: { - filteredTableData: smallDataSet, - selectedRows: [], - // eslint-disable-next-line @typescript-eslint/no-empty-function - someFn: () => {}, - pageSize: 0, - __evaluation__: { - evaluatedValues: {}, - }, - }, - } as any; - - const identicalEvalPathsPatches = { - "Table1.__evaluation__.evaluatedValues.['filteredTableData']": - "Table1.filteredTableData", - }; - const evalProps = { - Table1: { - __evaluation__: { - evaluatedValues: { - someProp: "abc", - // eslint-disable-next-line @typescript-eslint/no-empty-function - someEvalFn: () => {}, - }, - }, - }, - } as any; - const dataTree = makeEntityConfigsAsObjProperties(state, { - sanitizeDataTree: true, - evalProps, - identicalEvalPathsPatches, - }) as any; - const expectedState = produce(state, (draft: any) => { - draft.Table1.__evaluation__.evaluatedValues.someProp = "abc"; - delete draft.Table1.someFn; - draft.Table1.__evaluation__.evaluatedValues.filteredTableData = - smallDataSet; - }); - - expect(dataTree).toEqual(expectedState); - //function introduced by evalProps is cleaned out - expect( - dataTree.Table1.__evaluation__.evaluatedValues.someEvalFn, - ).toBeUndefined(); - }); - - it("should serialise bigInteger values", () => { - const someBigInt = BigInt(121221); - const state = { - Table1: { - pageSize: someBigInt, - __evaluation__: { - evaluatedValues: {}, - }, - }, - } as any; - - const identicalEvalPathsPatches = { - "Table1.__evaluation__.evaluatedValues.['pageSize']": - "Table1.pageSize", - }; - const evalProps = { - Table1: { - __evaluation__: { - evaluatedValues: { - someProp: someBigInt, - }, - }, - }, - } as any; - - const dataTree = makeEntityConfigsAsObjProperties(state, { - sanitizeDataTree: true, - evalProps, - identicalEvalPathsPatches, - }); - const expectedState = produce(state, (draft: any) => { - draft.Table1.pageSize = "121221"; - draft.Table1.__evaluation__.evaluatedValues.pageSize = "121221"; - draft.Table1.__evaluation__.evaluatedValues.someProp = "121221"; - }); - - expect(dataTree).toEqual(expectedState); - }); - }); }); }); diff --git a/app/client/src/ce/workers/Evaluation/dataTreeUtils.ts b/app/client/src/ce/workers/Evaluation/dataTreeUtils.ts index 3d84a84e57..1ffb4a20a1 100644 --- a/app/client/src/ce/workers/Evaluation/dataTreeUtils.ts +++ b/app/client/src/ce/workers/Evaluation/dataTreeUtils.ts @@ -1,7 +1,7 @@ import type { DataTree } from "entities/DataTree/dataTreeFactory"; +import { klona } from "klona/json"; import { get, set, unset } from "lodash"; import type { EvalProps } from "workers/common/DataTreeEvaluator"; -import { removeFunctionsAndSerialzeBigInt } from "@appsmith/workers/Evaluation/evaluationUtils"; /** * This method loops through each entity object of dataTree and sets the entity config from prototype as object properties. @@ -25,9 +25,7 @@ export function makeEntityConfigsAsObjProperties( const entity = dataTree[entityName]; newDataTree[entityName] = Object.assign({}, entity); } - const dataTreeToReturn = sanitizeDataTree - ? removeFunctionsAndSerialzeBigInt(newDataTree) - : newDataTree; + const dataTreeToReturn = sanitizeDataTree ? klona(newDataTree) : newDataTree; if (!evalProps) return dataTreeToReturn; @@ -58,9 +56,7 @@ export function makeEntityConfigsAsObjProperties( unset(evalProps, evalPath); }); - const sanitizedEvalProps = removeFunctionsAndSerialzeBigInt( - evalProps, - ) as EvalProps; + const sanitizedEvalProps = klona(evalProps) as EvalProps; Object.entries(alreadySanitisedDataSet).forEach(([path, val]) => { // add it to sanitised Eval props set(sanitizedEvalProps, path, val); diff --git a/app/client/src/ce/workers/Evaluation/evaluationUtils.ts b/app/client/src/ce/workers/Evaluation/evaluationUtils.ts index 81bf542a3d..891249ea07 100644 --- a/app/client/src/ce/workers/Evaluation/evaluationUtils.ts +++ b/app/client/src/ce/workers/Evaluation/evaluationUtils.ts @@ -418,10 +418,11 @@ export function isDataTreeEntity(entity: unknown) { return !!entity && typeof entity === "object" && "ENTITY_TYPE" in entity; } +export const serialiseToBigInt = (value: any) => + JSON.stringify(value, (_, v) => (typeof v === "bigint" ? v.toString() : v)); + export const removeFunctionsAndSerialzeBigInt = (value: any) => - JSON.parse( - JSON.stringify(value, (_, v) => (typeof v === "bigint" ? v.toString() : v)), - ); + JSON.parse(serialiseToBigInt(value)); // We need to remove functions from data tree to avoid any unexpected identifier while JSON parsing // Check issue https://github.com/appsmithorg/appsmith/issues/719 export const removeFunctions = (value: any) => { diff --git a/app/client/src/sagas/EvaluationSaga.utils.ts b/app/client/src/sagas/EvaluationSaga.utils.ts new file mode 100644 index 0000000000..2d6cc7c52f --- /dev/null +++ b/app/client/src/sagas/EvaluationSaga.utils.ts @@ -0,0 +1,40 @@ +import log from "loglevel"; +import type { DiffWithReferenceState } from "workers/Evaluation/helpers"; + +export const parseUpdatesAndDeleteUndefinedUpdates = ( + updates: string, +): DiffWithReferenceState[] => { + let parsedUpdates = []; + try { + //Parse updates from a string + parsedUpdates = JSON.parse(updates); + } catch (e) { + log.error("Failed to parse updates", e, updates); + return []; + } + //delete all undefined properties from the state + const { deleteUpdates, regularUpdates } = parsedUpdates.reduce( + (acc: any, curr: any) => { + const { kind, path, rhs } = curr; + + if (rhs === undefined) { + //ignore any new undefined updates to the state if the value is undefined + if (kind === "N") { + return acc; + } + //convert undefined updates to delete updates + if (kind === "E") { + acc.deleteUpdates.push({ kind: "D", path }); + return acc; + } + } + + acc.regularUpdates.push(curr); + return acc; + }, + { regularUpdates: [], deleteUpdates: [] }, + ); + + const consolidatedUpdates = [...regularUpdates, ...deleteUpdates]; + return consolidatedUpdates; +}; diff --git a/app/client/src/sagas/EvaluationsSaga.ts b/app/client/src/sagas/EvaluationsSaga.ts index c2dfbe698f..210ceff606 100644 --- a/app/client/src/sagas/EvaluationsSaga.ts +++ b/app/client/src/sagas/EvaluationsSaga.ts @@ -100,6 +100,7 @@ import { executeJSUpdates } from "actions/pluginActionActions"; import { setEvaluatedActionSelectorField } from "actions/actionSelectorActions"; import { waitForWidgetConfigBuild } from "./InitSagas"; import { logDynamicTriggerExecution } from "@appsmith/sagas/analyticsSaga"; +import { parseUpdatesAndDeleteUndefinedUpdates } from "./EvaluationSaga.utils"; const APPSMITH_CONFIGS = getAppsmithConfigs(); export const evalWorker = new GracefulWorkerService( new Worker( @@ -158,8 +159,8 @@ export function* updateDataTreeHandler( if (!isEmpty(staleMetaIds)) { yield put(resetWidgetsMetaState(staleMetaIds)); } - - yield put(setEvaluatedTree(updates)); + const parsedUpdates = parseUpdatesAndDeleteUndefinedUpdates(updates); + yield put(setEvaluatedTree(parsedUpdates)); ConfigTreeActions.setConfigTree(configTree); diff --git a/app/client/src/utils/DynamicBindingUtils.ts b/app/client/src/utils/DynamicBindingUtils.ts index a245eb34ad..2e881ea3e3 100644 --- a/app/client/src/utils/DynamicBindingUtils.ts +++ b/app/client/src/utils/DynamicBindingUtils.ts @@ -145,6 +145,7 @@ export enum EvalErrorTypes { PARSE_JS_ERROR = "PARSE_JS_ERROR", EXTRACT_DEPENDENCY_ERROR = "EXTRACT_DEPENDENCY_ERROR", CLONE_ERROR = "CLONE_ERROR", + SERIALIZATION_ERROR = "SERIALIZATION_ERROR", } export type EvalError = { diff --git a/app/client/src/workers/Evaluation/__tests__/generateOpimisedUpdates.test.ts b/app/client/src/workers/Evaluation/__tests__/generateOpimisedUpdates.test.ts index 4951f3dab0..5424a73699 100644 --- a/app/client/src/workers/Evaluation/__tests__/generateOpimisedUpdates.test.ts +++ b/app/client/src/workers/Evaluation/__tests__/generateOpimisedUpdates.test.ts @@ -1,6 +1,14 @@ +/* eslint-disable @typescript-eslint/no-empty-function */ +import { applyChange } from "deep-diff"; import produce from "immer"; import { range } from "lodash"; -import { generateOptimisedUpdates } from "../helpers"; +import { parseUpdatesAndDeleteUndefinedUpdates } from "sagas/EvaluationSaga.utils"; +import { EvalErrorTypes } from "utils/DynamicBindingUtils"; + +import { + generateOptimisedUpdates, + generateSerialisedUpdates, +} from "../helpers"; export const smallDataSet = [ { @@ -74,7 +82,7 @@ const oldState = { }, }; -describe("optimised diff updates", () => { +describe("generateOptimisedUpdates", () => { describe("regular diff", () => { test("should generate regular diff updates when a simple property changes in the widget property segment", () => { const newState = produce(oldState, (draft) => { @@ -246,3 +254,217 @@ describe("optimised diff updates", () => { }); }); }); + +//we are testing the flow of serialised updates generated from the worker thread and subsequently applied to the main thread state +describe("generateSerialisedUpdates and parseUpdatesAndDeleteUndefinedUpdates", () => { + it("should ignore undefined updates", () => { + const oldStateWithUndefinedValues = produce(oldState, (draft: any) => { + draft.Table1.pageSize = undefined; + }); + + const { serialisedUpdates } = generateSerialisedUpdates( + oldStateWithUndefinedValues, + //new state has the same undefined value + oldStateWithUndefinedValues, + {}, + ); + //no change hence empty array + expect(serialisedUpdates).toEqual("[]"); + }); + it("should generate a delete patch when a property is transformed to undefined", () => { + const oldStateWithUndefinedValues = produce(oldState, (draft: any) => { + draft.Table1.pageSize = undefined; + }); + + const { serialisedUpdates } = generateSerialisedUpdates( + oldState, + oldStateWithUndefinedValues, + {}, + ); + const parsedUpdates = + parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); + expect(parsedUpdates).toEqual([ + { + kind: "D", + path: ["Table1", "pageSize"], + }, + ]); + }); + it("should generate an error when there is a serialisation error", () => { + const oldStateWithUndefinedValues = produce(oldState, (draft: any) => { + //generate a cyclical object + draft.Table1.filteredTableData = draft.Table1; + }); + const { error, serialisedUpdates } = generateSerialisedUpdates( + oldState, + oldStateWithUndefinedValues, + {}, + ); + + expect(error?.type).toEqual(EvalErrorTypes.SERIALIZATION_ERROR); + //when a serialisation error occurs we should not return an error + expect(serialisedUpdates).toEqual("[]"); + }); + //when functions are serialised they become undefined and these updates should be deleted from the state + describe("clean out all functions in the generated state", () => { + it("should clean out new function properties added to the generated state", () => { + const newStateWithSomeFnProperty = produce(oldState, (draft: any) => { + draft.Table1.someFn = () => {}; + draft.Table1.__evaluation__.evaluatedValues.someEvalFn = () => {}; + }); + + const { serialisedUpdates } = generateSerialisedUpdates( + oldState, + newStateWithSomeFnProperty, + {}, + ); + + const parsedUpdates = + parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); + //should ignore all function updates + expect(parsedUpdates).toEqual([]); + + const parseAndApplyUpdatesToOldState = produce(oldState, (draft) => { + parsedUpdates.forEach((v: any) => { + applyChange(draft, undefined, v); + }); + }); + //no change in state + expect(parseAndApplyUpdatesToOldState).toEqual(oldState); + }); + + it("should delete properties which get updated to a function", () => { + const newStateWithSomeFnProperty = produce(oldState, (draft: any) => { + draft.Table1.pageSize = () => {}; + draft.Table1.__evaluation__.evaluatedValues.transientTableData = + () => {}; + }); + + const { serialisedUpdates } = generateSerialisedUpdates( + oldState, + newStateWithSomeFnProperty, + {}, + ); + + const parsedUpdates = + parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); + + expect(parsedUpdates).toEqual([ + { + kind: "D", + path: ["Table1", "pageSize"], + }, + { + kind: "D", + path: [ + "Table1", + "__evaluation__", + "evaluatedValues", + "transientTableData", + ], + }, + ]); + + const parseAndApplyUpdatesToOldState = produce(oldState, (draft) => { + parsedUpdates.forEach((v: any) => { + applyChange(draft, undefined, v); + }); + }); + const expectedState = produce(oldState, (draft: any) => { + delete draft.Table1.pageSize; + delete draft.Table1.__evaluation__.evaluatedValues.transientTableData; + }); + + expect(parseAndApplyUpdatesToOldState).toEqual(expectedState); + }); + it("should delete function properties which get updated to undefined", () => { + const oldStateWithSomeFnProperty = produce(oldState, (draft: any) => { + // eslint-disable-next-line @typescript-eslint/no-empty-function + draft.Table1.pageSize = () => {}; + draft.Table1.__evaluation__.evaluatedValues.transientTableData = + // eslint-disable-next-line @typescript-eslint/no-empty-function + () => {}; + }); + const newStateWithFnsTransformedToUndefined = produce( + oldState, + (draft: any) => { + draft.Table1.pageSize = undefined; + draft.Table1.__evaluation__.evaluatedValues.transientTableData = + undefined; + }, + ); + + const { serialisedUpdates } = generateSerialisedUpdates( + oldStateWithSomeFnProperty, + newStateWithFnsTransformedToUndefined, + {}, + ); + + const parsedUpdates = + parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); + + expect(parsedUpdates).toEqual([ + { + kind: "D", + path: ["Table1", "pageSize"], + }, + { + kind: "D", + path: [ + "Table1", + "__evaluation__", + "evaluatedValues", + "transientTableData", + ], + }, + ]); + + const parseAndApplyUpdatesToOldState = produce(oldState, (draft) => { + parsedUpdates.forEach((v: any) => { + applyChange(draft, undefined, v); + }); + }); + const expectedState = produce(oldState, (draft: any) => { + delete draft.Table1.pageSize; + delete draft.Table1.__evaluation__.evaluatedValues.transientTableData; + }); + + expect(parseAndApplyUpdatesToOldState).toEqual(expectedState); + }); + }); + + it("should serialise bigInteger values", () => { + const someBigInt = BigInt(121221); + const newStateWithBigInt = produce(oldState, (draft: any) => { + draft.Table1.pageSize = someBigInt; + }); + const { serialisedUpdates } = generateSerialisedUpdates( + oldState, + newStateWithBigInt, + {}, + ); + + const parsedUpdates = + parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); + + //should generate serialised bigInt update + expect(parsedUpdates).toEqual([ + { + kind: "E", + path: ["Table1", "pageSize"], + rhs: "121221", + }, + ]); + + const parseAndApplyUpdatesToOldState = produce(oldState, (draft) => { + parsedUpdates.forEach((v: any) => { + applyChange(draft, undefined, v); + }); + }); + const expectedState = produce(oldState, (draft: any) => { + draft.Table1.pageSize = "121221"; + }); + + expect(parseAndApplyUpdatesToOldState).toEqual(expectedState); + }); +}); diff --git a/app/client/src/workers/Evaluation/helpers.ts b/app/client/src/workers/Evaluation/helpers.ts index 52dcb2cb03..627e2747f1 100644 --- a/app/client/src/workers/Evaluation/helpers.ts +++ b/app/client/src/workers/Evaluation/helpers.ts @@ -1,9 +1,10 @@ +import { serialiseToBigInt } from "@appsmith/workers/Evaluation/evaluationUtils"; import type { Diff } from "deep-diff"; import { diff } from "deep-diff"; import type { DataTree } from "entities/DataTree/dataTreeFactory"; import equal from "fast-deep-equal"; -import produce from "immer"; -import { get, isNumber, isObject, set } from "lodash"; +import { get, isNumber, isObject } from "lodash"; +import { EvalErrorTypes } from "utils/DynamicBindingUtils"; export interface DiffReferenceState { kind: "referenceState"; @@ -200,6 +201,15 @@ const generateDiffUpdates = ( const lhs = get(oldDataTree, segmentedPath); + if (rhs === undefined) { + //if an undefined value is being set it should be a delete + if (lhs !== undefined) { + attachDirectly.push({ kind: "D", lhs, path: segmentedPath }); + } + // if the lhs is also undefined ignore diff on this node + return true; + } + const isLhsLarge = isLargeCollection(lhs); const isRhsLarge = isLargeCollection(rhs); if (!isLhsLarge && !isRhsLarge) { @@ -246,17 +256,40 @@ export const generateOptimisedUpdates = ( return updates; }; -export const decompressIdenticalEvalPaths = ( - dataTree: any, - identicalEvalPathsPatches: Record, -) => - produce(dataTree, (draft: any) => - Object.entries(identicalEvalPathsPatches || {}).forEach(([key, value]) => { - const referencePathValue = get(dataTree, value); - set(draft, key, referencePathValue); - }), +export const generateSerialisedUpdates = ( + prevState: any, + currentState: any, + identicalEvalPathsPatches: any, +): { + serialisedUpdates: string; + error?: { type: string; message: string }; +} => { + const updates = generateOptimisedUpdates( + prevState, + currentState, + identicalEvalPathsPatches, ); + //remove lhs from diff to reduce the size of diff upload, + //it is not necessary to send lhs and we can make the payload to transfer to the main thread smaller for quicker transfer + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const removedLhs = updates.map(({ lhs, ...rest }: any) => rest); + + try { + // serialise bigInt values and convert the updates to a string over here to minismise the cost of transfer + // to the main thread. In the main thread parse this object there. + return { serialisedUpdates: serialiseToBigInt(removedLhs) }; + } catch (error) { + return { + serialisedUpdates: "[]", + error: { + type: EvalErrorTypes.SERIALIZATION_ERROR, + message: (error as Error).message, + }, + }; + } +}; + export const generateOptimisedUpdatesAndSetPrevState = ( dataTree: any, dataTreeEvaluator: any, @@ -264,12 +297,15 @@ export const generateOptimisedUpdatesAndSetPrevState = ( const identicalEvalPathsPatches = dataTreeEvaluator?.getEvalPathsIdenticalToState(); - const updates = generateOptimisedUpdates( - dataTreeEvaluator?.getPrevState(), + const { error, serialisedUpdates } = generateSerialisedUpdates( + dataTreeEvaluator.getPrevState(), dataTree, identicalEvalPathsPatches, ); + if (error) { + dataTreeEvaluator.errors.push(error); + } dataTreeEvaluator?.setPrevState(dataTree); - return updates; + return serialisedUpdates; }; diff --git a/app/client/src/workers/Evaluation/types.ts b/app/client/src/workers/Evaluation/types.ts index ce4f2e7292..32bb6ed3da 100644 --- a/app/client/src/workers/Evaluation/types.ts +++ b/app/client/src/workers/Evaluation/types.ts @@ -18,7 +18,6 @@ import type { EvalMetaUpdates } from "@appsmith/workers/common/DataTreeEvaluator import type { WorkerRequest } from "@appsmith/workers/common/types"; import type { DataTreeDiff } from "@appsmith/workers/Evaluation/evaluationUtils"; import type { APP_MODE } from "entities/App"; -import type { DiffWithReferenceState } from "./helpers"; export type EvalWorkerSyncRequest = WorkerRequest; export type EvalWorkerASyncRequest = WorkerRequest< @@ -57,7 +56,7 @@ export interface EvalTreeResponseData { isNewWidgetAdded: boolean; undefinedEvalValuesMap: Record; jsVarsCreatedEvent?: { path: string; type: string }[]; - updates: DiffWithReferenceState[]; + updates: string; } export type JSVarMutatedEvents = Record;