From c580cfd9b4138e90b7acd5bc9a9f73de22e751e2 Mon Sep 17 00:00:00 2001 From: Diljit Date: Mon, 21 Apr 2025 12:59:20 +0530 Subject: [PATCH] chore: add dsl version validation for app computation cache (#40301) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description This PR adds dsl version to the app computation cache. If there is a mismatch in dsl version the cache is updated with the new value. Change also includes error handling and reporting for cases when there are exceptions while fetching the cache. Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.All" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: fadb0c528b85b846799db2cbbd9da4d01834627f > Cypress dashboard. > Tags: `@tag.All` > Spec: >
Fri, 18 Apr 2025 11:07:11 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **New Features** - Added explicit DSL version tracking to application state and evaluation context for improved cache validation. - **Bug Fixes** - Improved error handling for cache operations, ensuring errors are logged and recorded without disrupting core functionality. - Enhanced cache validation to prevent usage of invalid or mismatched cache entries. - **Tests** - Expanded and updated test coverage for cache validity, error scenarios, and DSL version handling. - **Refactor** - Strengthened cache property validation and error propagation for more robust caching behavior. --- .../src/ce/selectors/entitiesSelector.ts | 4 + app/client/src/sagas/EvalErrorHandler.ts | 5 + app/client/src/sagas/EvaluationsSaga.test.ts | 11 +- app/client/src/sagas/EvaluationsSaga.ts | 3 + app/client/src/utils/DynamicBindingUtils.ts | 1 + .../Evaluation/__tests__/evaluation.test.ts | 1 + .../Evaluation/evalTreeWithChanges.test.ts | 1 + .../AppComputationCache.test.ts | 299 +++++++++++++++--- .../common/AppComputationCache/index.ts | 153 +++++---- .../common/AppComputationCache/types.ts | 10 + .../dataTreeEvaluator.test.ts | 3 + .../workers/common/DataTreeEvaluator/index.ts | 20 +- .../src/workers/common/DependencyMap/index.ts | 38 ++- 13 files changed, 438 insertions(+), 111 deletions(-) diff --git a/app/client/src/ce/selectors/entitiesSelector.ts b/app/client/src/ce/selectors/entitiesSelector.ts index d565cded45..c33a71ef95 100644 --- a/app/client/src/ce/selectors/entitiesSelector.ts +++ b/app/client/src/ce/selectors/entitiesSelector.ts @@ -1804,3 +1804,7 @@ export const getUpcomingPlugins = createSelector( (state: AppState) => state.entities.plugins.upcomingPlugins, (upcomingPlugins) => upcomingPlugins.list, ); + +export const getCurrentPageDSLVersion = (state: AppState) => { + return state.entities.canvasWidgets[0]?.version || null; +}; diff --git a/app/client/src/sagas/EvalErrorHandler.ts b/app/client/src/sagas/EvalErrorHandler.ts index 4a6093d39d..1664dff50d 100644 --- a/app/client/src/sagas/EvalErrorHandler.ts +++ b/app/client/src/sagas/EvalErrorHandler.ts @@ -327,6 +327,11 @@ export function* evalErrorHandler( }); break; } + case EvalErrorTypes.CACHE_ERROR: { + log.error(error); + captureException(error, { errorName: "CacheError" }); + break; + } default: { log.error(error); captureException(reconstructedError, { errorName: "UnknownEvalError" }); diff --git a/app/client/src/sagas/EvaluationsSaga.test.ts b/app/client/src/sagas/EvaluationsSaga.test.ts index 5aa1ca60bc..f37629c0f3 100644 --- a/app/client/src/sagas/EvaluationsSaga.test.ts +++ b/app/client/src/sagas/EvaluationsSaga.test.ts @@ -12,7 +12,10 @@ import { expectSaga, testSaga } from "redux-saga-test-plan"; import { EVAL_WORKER_ACTIONS } from "ee/workers/Evaluation/evalWorkerActions"; import { select } from "redux-saga/effects"; import { getMetaWidgets, getWidgets, getWidgetsMeta } from "./selectors"; -import { getAllActionValidationConfig } from "ee//selectors/entitiesSelector"; +import { + getAllActionValidationConfig, + getCurrentPageDSLVersion, +} from "ee//selectors/entitiesSelector"; import { getSelectedAppTheme } from "selectors/appThemingSelectors"; import { getAppMode } from "ee/selectors/applicationSelectors"; import * as log from "loglevel"; @@ -59,6 +62,7 @@ describe("evaluateTreeSaga", () => { select(getApplicationLastDeployedAt), new Date("11 September 2024").toISOString(), ], + [select(getCurrentPageDSLVersion), 1], ]) .call(evalWorker.request, EVAL_WORKER_ACTIONS.EVAL_TREE, { cacheProps: { @@ -67,6 +71,7 @@ describe("evaluateTreeSaga", () => { pageId: "pageId", appMode: false, timestamp: new Date("11 September 2024").toISOString(), + dslVersion: 1, }, unevalTree: unEvalAndConfigTree, widgetTypeConfigMap: undefined, @@ -105,6 +110,7 @@ describe("evaluateTreeSaga", () => { select(getApplicationLastDeployedAt), new Date("11 September 2024").toISOString(), ], + [select(getCurrentPageDSLVersion), 1], ]) .call(evalWorker.request, EVAL_WORKER_ACTIONS.EVAL_TREE, { cacheProps: { @@ -113,6 +119,7 @@ describe("evaluateTreeSaga", () => { pageId: "pageId", appMode: false, timestamp: new Date("11 September 2024").toISOString(), + dslVersion: 1, }, unevalTree: unEvalAndConfigTree, widgetTypeConfigMap: undefined, @@ -160,6 +167,7 @@ describe("evaluateTreeSaga", () => { select(getApplicationLastDeployedAt), new Date("11 September 2024").toISOString(), ], + [select(getCurrentPageDSLVersion), 1], ]) .call(evalWorker.request, EVAL_WORKER_ACTIONS.EVAL_TREE, { cacheProps: { @@ -168,6 +176,7 @@ describe("evaluateTreeSaga", () => { pageId: "pageId", appMode: false, timestamp: new Date("11 September 2024").toISOString(), + dslVersion: 1, }, unevalTree: unEvalAndConfigTree, widgetTypeConfigMap: undefined, diff --git a/app/client/src/sagas/EvaluationsSaga.ts b/app/client/src/sagas/EvaluationsSaga.ts index 82fec84a8d..626133a3cf 100644 --- a/app/client/src/sagas/EvaluationsSaga.ts +++ b/app/client/src/sagas/EvaluationsSaga.ts @@ -77,6 +77,7 @@ import { resetWidgetsMetaState, updateMetaState } from "actions/metaActions"; import { getAllActionValidationConfig, getAllJSActionsData, + getCurrentPageDSLVersion, } from "ee/selectors/entitiesSelector"; import type { WidgetEntityConfig } from "ee/entities/DataTree/types"; import type { @@ -269,6 +270,7 @@ export function* evaluateTreeSaga( const appMode: ReturnType = yield select(getAppMode); const widgetsMeta: ReturnType = yield select(getWidgetsMeta); + const dslVersion: number | null = yield select(getCurrentPageDSLVersion); const shouldRespondWithLogs = log.getLevel() === log.levels.DEBUG; @@ -279,6 +281,7 @@ export function* evaluateTreeSaga( pageId, timestamp: lastDeployedAt, instanceId, + dslVersion, }, unevalTree: unEvalAndConfigTree, widgetTypeConfigMap, diff --git a/app/client/src/utils/DynamicBindingUtils.ts b/app/client/src/utils/DynamicBindingUtils.ts index 19532a7b06..669bcf546b 100644 --- a/app/client/src/utils/DynamicBindingUtils.ts +++ b/app/client/src/utils/DynamicBindingUtils.ts @@ -161,6 +161,7 @@ export enum EvalErrorTypes { CLONE_ERROR = "CLONE_ERROR", SERIALIZATION_ERROR = "SERIALIZATION_ERROR", UPDATE_DATA_TREE_ERROR = "UPDATE_DATA_TREE_ERROR", + CACHE_ERROR = "CACHE_ERROR", } export interface EvalError { diff --git a/app/client/src/workers/Evaluation/__tests__/evaluation.test.ts b/app/client/src/workers/Evaluation/__tests__/evaluation.test.ts index ab4253aeff..f468750d39 100644 --- a/app/client/src/workers/Evaluation/__tests__/evaluation.test.ts +++ b/app/client/src/workers/Evaluation/__tests__/evaluation.test.ts @@ -588,6 +588,7 @@ describe("DataTreeEvaluator", () => { timestamp: "timestamp", appMode: APP_MODE.PUBLISHED, instanceId: "instanceId", + dslVersion: 1, }, ); evaluator.evalAndValidateFirstTree(); diff --git a/app/client/src/workers/Evaluation/evalTreeWithChanges.test.ts b/app/client/src/workers/Evaluation/evalTreeWithChanges.test.ts index 29ccef4f28..dee35e3218 100644 --- a/app/client/src/workers/Evaluation/evalTreeWithChanges.test.ts +++ b/app/client/src/workers/Evaluation/evalTreeWithChanges.test.ts @@ -200,6 +200,7 @@ describe("evaluateAndGenerateResponse", () => { timestamp: "timestamp", appMode: APP_MODE.PUBLISHED, instanceId: "instanceId", + dslVersion: 1, }, ); evaluator.evalAndValidateFirstTree(); diff --git a/app/client/src/workers/common/AppComputationCache/AppComputationCache.test.ts b/app/client/src/workers/common/AppComputationCache/AppComputationCache.test.ts index e3a837485c..223097799c 100644 --- a/app/client/src/workers/common/AppComputationCache/AppComputationCache.test.ts +++ b/app/client/src/workers/common/AppComputationCache/AppComputationCache.test.ts @@ -1,4 +1,8 @@ -import { EComputationCacheName, type ICacheProps } from "./types"; +import { + EComputationCacheName, + type ICacheProps, + type IValidatedCacheProps, +} from "./types"; import { APP_MODE } from "entities/App"; import localforage from "localforage"; import loglevel from "loglevel"; @@ -71,12 +75,13 @@ describe("AppComputationCache", () => { describe("generateCacheKey", () => { test("should generate the correct cache key", () => { - const cacheProps: ICacheProps = { + const cacheProps: IValidatedCacheProps = { appMode: APP_MODE.PUBLISHED, timestamp: new Date("11 September 2024").toISOString(), appId: "appId", instanceId: "instanceId", pageId: "pageId", + dslVersion: 1, }; const cacheName = EComputationCacheName.ALL_KEYS; @@ -102,14 +107,15 @@ describe("AppComputationCache", () => { appId: "appId", instanceId: "instanceId", pageId: "pageId", + dslVersion: 1, }; const cacheName = EComputationCacheName.ALL_KEYS; - const result = appComputationCache.isComputationCached({ + const result = appComputationCache.shouldComputationBeCached( cacheName, cacheProps, - }); + ); expect(result).toBe(false); }); @@ -121,14 +127,15 @@ describe("AppComputationCache", () => { appId: "appId", instanceId: "instanceId", pageId: "pageId", + dslVersion: 1, }; const cacheName = EComputationCacheName.ALL_KEYS; - const result = appComputationCache.isComputationCached({ + const result = appComputationCache.shouldComputationBeCached( cacheName, cacheProps, - }); + ); expect(result).toBe(true); }); @@ -139,14 +146,15 @@ describe("AppComputationCache", () => { appId: "appId", instanceId: "instanceId", pageId: "pageId", + dslVersion: 1, }; const cacheName = EComputationCacheName.ALL_KEYS; - const result = appComputationCache.isComputationCached({ + const result = appComputationCache.shouldComputationBeCached( cacheName, cacheProps, - }); + ); expect(result).toBe(false); }); @@ -158,14 +166,35 @@ describe("AppComputationCache", () => { appId: "appId", instanceId: "instanceId", pageId: "pageId", + dslVersion: 1, }; const cacheName = EComputationCacheName.ALL_KEYS; - const result = appComputationCache.isComputationCached({ + const result = appComputationCache.shouldComputationBeCached( cacheName, cacheProps, - }); + ); + + expect(result).toBe(false); + }); + + test("should return false if dslVersion is undefined", () => { + const cacheProps: ICacheProps = { + appMode: APP_MODE.PUBLISHED, + timestamp: new Date("11 September 2024").toISOString(), + appId: "appId", + instanceId: "instanceId", + pageId: "pageId", + dslVersion: null, + }; + + const cacheName = EComputationCacheName.ALL_KEYS; + + const result = appComputationCache.shouldComputationBeCached( + cacheName, + cacheProps, + ); expect(result).toBe(false); }); @@ -173,12 +202,13 @@ describe("AppComputationCache", () => { describe("getCachedComputationResult", () => { test("should call getItemMock and return null if cache miss", async () => { - const cacheProps: ICacheProps = { + const cacheProps: IValidatedCacheProps = { appMode: APP_MODE.PUBLISHED, timestamp: new Date("11 September 2024").toISOString(), appId: "appId", instanceId: "instanceId", pageId: "pageId", + dslVersion: 1, }; const cacheName = EComputationCacheName.ALL_KEYS; @@ -205,12 +235,13 @@ describe("AppComputationCache", () => { }); test("should call deleteInvalidCacheEntries on cache miss after 10 seconds", async () => { - const cacheProps: ICacheProps = { + const cacheProps: IValidatedCacheProps = { appMode: APP_MODE.PUBLISHED, timestamp: new Date("11 September 2024").toISOString(), appId: "appId", instanceId: "instanceId", pageId: "pageId", + dslVersion: 1, }; const cacheName = EComputationCacheName.ALL_KEYS; @@ -240,13 +271,14 @@ describe("AppComputationCache", () => { expect(keysMock).toHaveBeenCalledTimes(1); }); - test("should call getItemMock and return cached value if cache hit", async () => { - const cacheProps: ICacheProps = { + test("should call deleteInvalidCacheEntries on dsl version mismatch after 10 seconds", async () => { + const cacheProps: IValidatedCacheProps = { appMode: APP_MODE.PUBLISHED, timestamp: new Date("11 September 2024").toISOString(), appId: "appId", instanceId: "instanceId", pageId: "pageId", + dslVersion: 2, }; const cacheName = EComputationCacheName.ALL_KEYS; @@ -256,7 +288,43 @@ describe("AppComputationCache", () => { cacheProps, }); - getItemMock.mockResolvedValue({ value: "cachedValue" }); + getItemMock.mockResolvedValue({ value: "cachedValue", dslVersion: 1 }); + + const result = await appComputationCache.getCachedComputationResult({ + cacheName, + cacheProps, + }); + + expect(getItemMock).toHaveBeenCalledWith(cacheKey); + expect(result).toBe(null); + + jest.advanceTimersByTime(2500); + expect(keysMock).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(2500); + jest.runAllTimers(); + + expect(keysMock).toHaveBeenCalledTimes(1); + }); + + test("should call getItemMock and return cached value if cache hit", async () => { + const cacheProps: IValidatedCacheProps = { + appMode: APP_MODE.PUBLISHED, + timestamp: new Date("11 September 2024").toISOString(), + appId: "appId", + instanceId: "instanceId", + pageId: "pageId", + dslVersion: 1, + }; + + const cacheName = EComputationCacheName.ALL_KEYS; + + const cacheKey = appComputationCache.generateCacheKey({ + cacheName, + cacheProps, + }); + + getItemMock.mockResolvedValue({ value: "cachedValue", dslVersion: 1 }); const result = await appComputationCache.getCachedComputationResult({ cacheName, @@ -270,12 +338,13 @@ describe("AppComputationCache", () => { describe("cacheComputationResult", () => { test("should store computation result and call trackCacheUsage when shouldCache is true", async () => { - const cacheProps: ICacheProps = { + const cacheProps: IValidatedCacheProps = { appMode: APP_MODE.PUBLISHED, timestamp: new Date("11 September 2024").toISOString(), appId: "appId", instanceId: "instanceId", pageId: "pageId", + dslVersion: 1, }; const cacheName = EComputationCacheName.ALL_KEYS; @@ -300,6 +369,7 @@ describe("AppComputationCache", () => { expect(setItemMock).toHaveBeenCalledWith(cacheKey, { value: computationResult, + dslVersion: 1, }); expect(trackCacheUsageSpy).toHaveBeenCalledWith(cacheKey); @@ -313,6 +383,30 @@ describe("AppComputationCache", () => { appId: "appId", instanceId: "instanceId", pageId: "pageId", + dslVersion: 1, + }; + + const cacheName = EComputationCacheName.ALL_KEYS; + + const computationResult = "computedValue"; + + await appComputationCache.cacheComputationResult({ + cacheName, + cacheProps, + computationResult, + }); + + expect(setItemMock).not.toHaveBeenCalled(); + }); + + test("should not store computation result when dsl version is invalid", async () => { + const cacheProps: ICacheProps = { + appMode: APP_MODE.PUBLISHED, + timestamp: new Date("11 September 2024").toISOString(), + appId: "appId", + instanceId: "instanceId", + pageId: "pageId", + dslVersion: null, }; const cacheName = EComputationCacheName.ALL_KEYS; @@ -331,12 +425,13 @@ describe("AppComputationCache", () => { describe("fetchOrCompute", () => { test("should return cached result if available", async () => { - const cacheProps: ICacheProps = { + const cacheProps: IValidatedCacheProps = { appMode: APP_MODE.PUBLISHED, timestamp: new Date("11 September 2024").toISOString(), appId: "appId", instanceId: "instanceId", pageId: "pageId", + dslVersion: 1, }; const cacheName = EComputationCacheName.ALL_KEYS; @@ -346,7 +441,7 @@ describe("AppComputationCache", () => { cacheProps, }); - getItemMock.mockResolvedValue({ value: "cachedValue" }); + getItemMock.mockResolvedValue({ value: "cachedValue", dslVersion: 1 }); const computeFn = jest.fn(() => "computedValue"); @@ -364,12 +459,13 @@ describe("AppComputationCache", () => { test("should compute, cache, and return result if not in cache", async () => { getItemMock.mockResolvedValue(null); - const cacheProps: ICacheProps = { + const cacheProps: IValidatedCacheProps = { appMode: APP_MODE.PUBLISHED, timestamp: new Date("11 September 2024").toISOString(), appId: "appId", instanceId: "instanceId", pageId: "pageId", + dslVersion: 1, }; const cacheName = EComputationCacheName.ALL_KEYS; @@ -413,12 +509,13 @@ describe("AppComputationCache", () => { loglevel.setLevel("SILENT"); - const cacheProps: ICacheProps = { + const cacheProps: IValidatedCacheProps = { appMode: APP_MODE.PUBLISHED, timestamp: new Date("11 September 2024").toISOString(), appId: "appId", instanceId: "instanceId", pageId: "pageId", + dslVersion: 1, }; const cacheName = EComputationCacheName.ALL_KEYS; @@ -427,39 +524,54 @@ describe("AppComputationCache", () => { const computeFn = jest.fn(() => computationResult); - const cacheComputationResultSpy = jest.spyOn( - appComputationCache, - "cacheComputationResult", - ); + try { + await appComputationCache.fetchOrCompute({ + cacheName, + cacheProps, + computeFn, + }); + fail("Expected error to be thrown"); + } catch (error) { + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toContain("Cache access error"); + } - const result = await appComputationCache.fetchOrCompute({ - cacheName, - cacheProps, - computeFn, - }); - - expect(getItemMock).toHaveBeenCalled(); - expect(computeFn).toHaveBeenCalled(); - expect(cacheComputationResultSpy).toHaveBeenCalledWith({ - cacheName, - cacheProps, - computationResult, - }); - expect(result).toBe(computationResult); - - cacheComputationResultSpy.mockRestore(); loglevel.setLevel(defaultLogLevel); }); - }); - describe("deleteInvalidCacheEntries", () => { - test("should delete old cache entries", async () => { + test("should not cache result when dsl version is invalid", async () => { const cacheProps: ICacheProps = { appMode: APP_MODE.PUBLISHED, timestamp: new Date("11 September 2024").toISOString(), appId: "appId", instanceId: "instanceId", pageId: "pageId", + dslVersion: null, + }; + + const cacheName = EComputationCacheName.ALL_KEYS; + + const computationResult = "computedValue"; + + await appComputationCache.cacheComputationResult({ + cacheName, + cacheProps, + computationResult, + }); + + expect(setItemMock).not.toHaveBeenCalled(); + }); + }); + + describe("deleteInvalidCacheEntries", () => { + test("should delete old cache entries", async () => { + const cacheProps: IValidatedCacheProps = { + appMode: APP_MODE.PUBLISHED, + timestamp: new Date("11 September 2024").toISOString(), + appId: "appId", + instanceId: "instanceId", + pageId: "pageId", + dslVersion: 1, }; const currentTimestamp = new Date(cacheProps.timestamp).getTime(); @@ -516,4 +628,105 @@ describe("AppComputationCache", () => { }); }); }); + + describe("isAppModeValid", () => { + test("should return true for valid app modes", () => { + expect(appComputationCache.isAppModeValid(APP_MODE.PUBLISHED)).toBe(true); + expect(appComputationCache.isAppModeValid(APP_MODE.EDIT)).toBe(true); + }); + + test("should return false for invalid app modes", () => { + expect(appComputationCache.isAppModeValid(undefined)).toBe(false); + expect(appComputationCache.isAppModeValid(null)).toBe(false); + expect(appComputationCache.isAppModeValid("invalid")).toBe(false); + }); + }); + + describe("isTimestampValid", () => { + test("should return true for valid timestamps", () => { + const validTimestamp = new Date().toISOString(); + + expect(appComputationCache.isTimestampValid(validTimestamp)).toBe(true); + }); + + test("should return false for invalid timestamps", () => { + expect(appComputationCache.isTimestampValid(undefined)).toBe(false); + expect(appComputationCache.isTimestampValid(null)).toBe(false); + expect(appComputationCache.isTimestampValid("invalid")).toBe(false); + expect(appComputationCache.isTimestampValid("2024-01-01")).toBe(false); + expect(appComputationCache.isTimestampValid("2024-01-01T00")).toBe(false); + expect(appComputationCache.isTimestampValid("2024-01-01T00:00")).toBe( + false, + ); + expect(appComputationCache.isTimestampValid("2024-01-01T00:00:00")).toBe( + false, + ); + expect( + appComputationCache.isTimestampValid("2024-01-01T00:00:00.000"), + ).toBe(false); + }); + }); + + describe("isDSLVersionValid", () => { + test("should return true for valid dsl versions", () => { + expect(appComputationCache.isDSLVersionValid(1)).toBe(true); + expect(appComputationCache.isDSLVersionValid(90)).toBe(true); + }); + + test("should return false for invalid dsl versions", () => { + expect(appComputationCache.isDSLVersionValid(0)).toBe(false); + expect(appComputationCache.isDSLVersionValid(null)).toBe(false); + expect(appComputationCache.isDSLVersionValid("invalid")).toBe(false); + expect(appComputationCache.isDSLVersionValid(undefined)).toBe(false); + expect(appComputationCache.isDSLVersionValid(NaN)).toBe(false); + expect(appComputationCache.isDSLVersionValid(Infinity)).toBe(false); + expect(appComputationCache.isDSLVersionValid(-1)).toBe(false); + }); + }); + + describe("isCacheValid", () => { + const cacheProps: IValidatedCacheProps = { + appMode: APP_MODE.PUBLISHED, + timestamp: new Date().toISOString(), + appId: "appId", + instanceId: "instanceId", + pageId: "pageId", + dslVersion: 1, + }; + + test("should return true for valid cache", () => { + const cachedValue = { + value: "cachedValue", + dslVersion: 1, + }; + + expect(appComputationCache.isCacheValid(cachedValue, cacheProps)).toBe( + true, + ); + }); + + test("should return false null cache", () => { + expect(appComputationCache.isCacheValid(null, cacheProps)).toBe(false); + }); + + test("should return false if dsl version is not present", () => { + expect( + appComputationCache.isCacheValid( + { + value: "cachedValue", + }, + cacheProps, + ), + ).toBe(false); + }); + + test("should return false if dsl version mismatch", () => { + expect( + appComputationCache.isCacheValid( + { value: "cachedValue", dslVersion: 2 }, + cacheProps, + ), + ).toBe(false); + }); + }); }); diff --git a/app/client/src/workers/common/AppComputationCache/index.ts b/app/client/src/workers/common/AppComputationCache/index.ts index a7e70ad365..f872a6b6c8 100644 --- a/app/client/src/workers/common/AppComputationCache/index.ts +++ b/app/client/src/workers/common/AppComputationCache/index.ts @@ -2,11 +2,17 @@ import { APP_MODE } from "entities/App"; import localforage from "localforage"; import isNull from "lodash/isNull"; import loglevel from "loglevel"; -import { EComputationCacheName, type ICacheProps } from "./types"; +import { + EComputationCacheName, + type IValidatedCacheProps, + type ICacheProps, +} from "./types"; import debounce from "lodash/debounce"; +import { isFinite, isNumber, isString } from "lodash"; interface ICachedData { value: T; + dslVersion?: number; } interface ICacheLog { @@ -52,6 +58,25 @@ export class AppComputationCache { return AppComputationCache.instance; } + isAppModeValid(appMode: unknown) { + return appMode === APP_MODE.PUBLISHED || appMode === APP_MODE.EDIT; + } + + isTimestampValid(timestamp: unknown) { + const isoStringRegex = + /^(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2}):(\d{2})\.(\d{3})Z$/; + + if (isString(timestamp) && !!timestamp.trim()) { + return isoStringRegex.test(timestamp); + } + + return false; + } + + isDSLVersionValid(dslVersion: unknown) { + return isNumber(dslVersion) && isFinite(dslVersion) && dslVersion > 0; + } + debouncedDeleteInvalidCacheEntries = debounce( this.deleteInvalidCacheEntries, 5000, @@ -61,16 +86,17 @@ export class AppComputationCache { * Check if the computation result should be cached based on the app mode configuration * @returns - A boolean indicating whether the cache should be enabled for the given app mode */ - isComputationCached({ - cacheName, - cacheProps, - }: { - cacheName: EComputationCacheName; - cacheProps: ICacheProps; - }) { - const { appMode, timestamp } = cacheProps; + shouldComputationBeCached( + cacheName: EComputationCacheName, + cacheProps: ICacheProps, + ): cacheProps is IValidatedCacheProps { + const { appMode, dslVersion, timestamp } = cacheProps; - if (!appMode || !timestamp) { + if ( + !this.isAppModeValid(appMode) || + !this.isTimestampValid(timestamp) || + !this.isDSLVersionValid(dslVersion) + ) { return false; } @@ -81,6 +107,7 @@ export class AppComputationCache { * Checks if the value should be cached based on the app mode configuration and * caches the computation result if it should be cached. It also tracks the cache usage * @returns - A promise that resolves when the computation result is cached + * @throws - Logs an error if the computation result cannot be cached and throws the error */ async cacheComputationResult({ cacheName, @@ -91,31 +118,31 @@ export class AppComputationCache { cacheName: EComputationCacheName; computationResult: T; }) { - const shouldCache = this.isComputationCached({ - cacheName, - cacheProps, - }); - - if (!shouldCache) { - return; - } - - const cacheKey = this.generateCacheKey({ cacheProps, cacheName }); - try { + const isCacheable = this.shouldComputationBeCached(cacheName, cacheProps); + + if (!isCacheable) { + return; + } + + const cacheKey = this.generateCacheKey({ cacheProps, cacheName }); + await this.store.setItem>(cacheKey, { value: computationResult, + dslVersion: cacheProps.dslVersion, }); await this.trackCacheUsage(cacheKey); } catch (error) { - loglevel.debug("Error caching computation result:", error); + loglevel.error(error); + throw error; } } /** * Gets the cached computation result if it exists and is valid * @returns - A promise that resolves with the cached computation result or null if it does not exist + * @throws - Logs an error if the computation result cannot be fetched and throws the error */ async getCachedComputationResult({ cacheName, @@ -124,24 +151,21 @@ export class AppComputationCache { cacheProps: ICacheProps; cacheName: EComputationCacheName; }): Promise { - const shouldCache = this.isComputationCached({ - cacheName, - cacheProps, - }); - - if (!shouldCache) { - return null; - } - - const cacheKey = this.generateCacheKey({ - cacheProps, - cacheName, - }); - try { + const isCacheable = this.shouldComputationBeCached(cacheName, cacheProps); + + if (!isCacheable) { + return null; + } + + const cacheKey = this.generateCacheKey({ + cacheProps, + cacheName, + }); + const cached = await this.store.getItem>(cacheKey); - if (isNull(cached)) { + if (!this.isCacheValid(cached, cacheProps)) { // Cache miss // Delete invalid cache entries when thread is idle setTimeout(async () => { @@ -155,12 +179,30 @@ export class AppComputationCache { return cached.value; } catch (error) { - loglevel.error("Error getting cache result:", error); - - return null; + loglevel.error(error); + throw error; } } + /** + * Checks if the cached value is valid + * @returns - A boolean indicating whether the cached value is valid + */ + isCacheValid( + cachedValue: ICachedData | null, + cacheProps: IValidatedCacheProps, + ): cachedValue is ICachedData { + if (isNull(cachedValue)) { + return false; + } + + if (!cachedValue.dslVersion) { + return false; + } + + return cachedValue.dslVersion === cacheProps.dslVersion; + } + /** * Generates a cache key from the index parts * @returns - The generated cache key @@ -169,7 +211,7 @@ export class AppComputationCache { cacheName, cacheProps, }: { - cacheProps: ICacheProps; + cacheProps: IValidatedCacheProps; cacheName: EComputationCacheName; }) { const { appId, appMode, instanceId, pageId, timestamp } = cacheProps; @@ -201,16 +243,13 @@ export class AppComputationCache { computeFn: () => Promise | T; cacheName: EComputationCacheName; }) { - const shouldCache = this.isComputationCached({ - cacheName, - cacheProps, - }); - - if (!shouldCache) { - return computeFn(); - } - try { + const isCacheable = this.shouldComputationBeCached(cacheName, cacheProps); + + if (!isCacheable) { + return computeFn(); + } + const cachedResult = await this.getCachedComputationResult({ cacheProps, cacheName, @@ -230,10 +269,8 @@ export class AppComputationCache { return computationResult; } catch (error) { - loglevel.error("Error getting cache result:", error); - const fallbackResult = await computeFn(); - - return fallbackResult; + loglevel.error(error); + throw error; } } @@ -259,6 +296,7 @@ export class AppComputationCache { /** * Delete invalid cache entries * @returns - A promise that resolves when the invalid cache entries are deleted + * @throws - Logs an error if the invalid cache entries cannot be deleted */ async deleteInvalidCacheEntries(cacheProps: ICacheProps) { @@ -271,6 +309,10 @@ export class AppComputationCache { const keyParts = key.split(AppComputationCache.CACHE_KEY_DELIMITER); const cacheKeyTimestamp = parseInt(keyParts[4], 10); + if (!cacheProps.timestamp) { + return false; + } + return ( keyParts[0] === cacheProps.instanceId && keyParts[1] === cacheProps.appId && @@ -295,6 +337,9 @@ export class AppComputationCache { } } + /** + * Resets the singleton instance + */ static resetInstance() { AppComputationCache.instance = null; } diff --git a/app/client/src/workers/common/AppComputationCache/types.ts b/app/client/src/workers/common/AppComputationCache/types.ts index 5ae66f6103..42a0b36653 100644 --- a/app/client/src/workers/common/AppComputationCache/types.ts +++ b/app/client/src/workers/common/AppComputationCache/types.ts @@ -9,6 +9,16 @@ export interface ICacheProps { appId: string; pageId: string; appMode?: APP_MODE; + timestamp?: string; + instanceId: string; + dslVersion: number | null; +} + +export interface IValidatedCacheProps { + appId: string; + pageId: string; + appMode: APP_MODE; timestamp: string; instanceId: string; + dslVersion: number; } diff --git a/app/client/src/workers/common/DataTreeEvaluator/dataTreeEvaluator.test.ts b/app/client/src/workers/common/DataTreeEvaluator/dataTreeEvaluator.test.ts index 514fc3691f..030944958f 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/dataTreeEvaluator.test.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/dataTreeEvaluator.test.ts @@ -290,6 +290,7 @@ describe("DataTreeEvaluator", () => { timestamp: "timestamp", appMode: APP_MODE.PUBLISHED, instanceId: "instanceId", + dslVersion: 1, }, ); dataTreeEvaluator.evalAndValidateFirstTree(); @@ -391,6 +392,7 @@ describe("DataTreeEvaluator", () => { timestamp: "timestamp", appMode: APP_MODE.PUBLISHED, instanceId: "instanceId", + dslVersion: 1, }, ); dataTreeEvaluator.evalAndValidateFirstTree(); @@ -454,6 +456,7 @@ describe("DataTreeEvaluator", () => { timestamp: new Date().toISOString(), appMode: APP_MODE.PUBLISHED, instanceId: "instanceId", + dslVersion: 1, }, ); dataTreeEvaluator.evalAndValidateFirstTree(); diff --git a/app/client/src/workers/common/DataTreeEvaluator/index.ts b/app/client/src/workers/common/DataTreeEvaluator/index.ts index 99c2da53a7..bf55a03944 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/index.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/index.ts @@ -302,11 +302,21 @@ export default class DataTreeEvaluator { ); const allKeysGenerationStartTime = performance.now(); - this.allKeys = await appComputationCache.fetchOrCompute({ - cacheProps, - cacheName: EComputationCacheName.ALL_KEYS, - computeFn: () => getAllPaths(unEvalTreeWithStrigifiedJSFunctions), - }); + try { + this.allKeys = await appComputationCache.fetchOrCompute({ + cacheProps, + cacheName: EComputationCacheName.ALL_KEYS, + computeFn: () => getAllPaths(unEvalTreeWithStrigifiedJSFunctions), + }); + } catch (error) { + this.errors.push({ + type: EvalErrorTypes.CACHE_ERROR, + message: (error as Error).message, + stack: (error as Error).stack, + }); + + this.allKeys = getAllPaths(unEvalTreeWithStrigifiedJSFunctions); + } const allKeysGenerationEndTime = performance.now(); diff --git a/app/client/src/workers/common/DependencyMap/index.ts b/app/client/src/workers/common/DependencyMap/index.ts index bd0868c585..4674d5dc2f 100644 --- a/app/client/src/workers/common/DependencyMap/index.ts +++ b/app/client/src/workers/common/DependencyMap/index.ts @@ -12,7 +12,11 @@ import type { DataTreeEntityObject, } from "ee/entities/DataTree/types"; import type { ConfigTree, DataTree } from "entities/DataTree/dataTreeTypes"; -import { getEntityId, getEvalErrorPath } from "utils/DynamicBindingUtils"; +import { + EvalErrorTypes, + getEntityId, + getEvalErrorPath, +} from "utils/DynamicBindingUtils"; import { convertArrayToObject, extractInfoFromBindings } from "./utils"; import type DataTreeEvaluator from "workers/common/DataTreeEvaluator"; import { get, isEmpty, set } from "lodash"; @@ -59,13 +63,23 @@ export async function createDependencyMap( ); }); - const dependencyMapCache = - await appComputationCache.getCachedComputationResult< + let dependencyMapCache: Record | null = null; + + try { + dependencyMapCache = await appComputationCache.getCachedComputationResult< Record >({ cacheProps, cacheName: EComputationCacheName.DEPENDENCY_MAP, }); + } catch (error) { + dataTreeEvalRef.errors.push({ + type: EvalErrorTypes.CACHE_ERROR, + message: (error as Error).message, + stack: (error as Error).stack, + }); + dependencyMapCache = null; + } if (dependencyMapCache) { profileFn("createDependencyMap.addDependency", {}, webworkerSpans, () => { @@ -104,11 +118,19 @@ export async function createDependencyMap( DependencyMapUtils.makeParentsDependOnChildren(dependencyMap); if (shouldCache) { - await appComputationCache.cacheComputationResult({ - cacheProps, - cacheName: EComputationCacheName.DEPENDENCY_MAP, - computationResult: dependencyMap.dependencies, - }); + try { + await appComputationCache.cacheComputationResult({ + cacheProps, + cacheName: EComputationCacheName.DEPENDENCY_MAP, + computationResult: dependencyMap.dependencies, + }); + } catch (error) { + dataTreeEvalRef.errors.push({ + type: EvalErrorTypes.CACHE_ERROR, + message: (error as Error).message, + stack: (error as Error).stack, + }); + } } }