diff --git a/app/client/src/ce/selectors/entitiesSelector.ts b/app/client/src/ce/selectors/entitiesSelector.ts index d565cded4504..c33a71ef9562 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 4a6093d39d59..1664dff50d72 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 5aa1ca60bc70..f37629c0f316 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 82fec84a8d0e..626133a3cf7d 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 19532a7b0689..669bcf546b20 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 ab4253aeffc6..f468750d3911 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 29ccef4f284c..dee35e321843 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 e3a837485c9c..223097799ca2 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,50 @@ describe("AppComputationCache", () => { expect(keysMock).toHaveBeenCalledTimes(1); }); + 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; + + const cacheKey = appComputationCache.generateCacheKey({ + cacheName, + cacheProps, + }); + + 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: 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; @@ -256,7 +324,7 @@ describe("AppComputationCache", () => { cacheProps, }); - getItemMock.mockResolvedValue({ value: "cachedValue" }); + 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, - }); + loglevel.setLevel(defaultLogLevel); + }); - expect(getItemMock).toHaveBeenCalled(); - expect(computeFn).toHaveBeenCalled(); - expect(cacheComputationResultSpy).toHaveBeenCalledWith({ + 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(result).toBe(computationResult); - cacheComputationResultSpy.mockRestore(); - loglevel.setLevel(defaultLogLevel); + expect(setItemMock).not.toHaveBeenCalled(); }); }); describe("deleteInvalidCacheEntries", () => { test("should delete old cache entries", 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 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 a7e70ad36512..f872a6b6c8d5 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; - - if (!appMode || !timestamp) { + shouldComputationBeCached( + cacheName: EComputationCacheName, + cacheProps: ICacheProps, + ): cacheProps is IValidatedCacheProps { + const { appMode, dslVersion, timestamp } = cacheProps; + + 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, - }); + try { + const isCacheable = this.shouldComputationBeCached(cacheName, cacheProps); - if (!shouldCache) { - return; - } + if (!isCacheable) { + return; + } - const cacheKey = this.generateCacheKey({ cacheProps, cacheName }); + const cacheKey = this.generateCacheKey({ cacheProps, cacheName }); - try { 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, - }); + try { + const isCacheable = this.shouldComputationBeCached(cacheName, cacheProps); - if (!shouldCache) { - return null; - } + if (!isCacheable) { + return null; + } - const cacheKey = this.generateCacheKey({ - cacheProps, - cacheName, - }); + const cacheKey = this.generateCacheKey({ + cacheProps, + cacheName, + }); - try { 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,10 +179,28 @@ export class AppComputationCache { return cached.value; } catch (error) { - loglevel.error("Error getting cache result:", error); + loglevel.error(error); + throw error; + } + } - return null; + /** + * 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; } /** @@ -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, - }); + try { + const isCacheable = this.shouldComputationBeCached(cacheName, cacheProps); - if (!shouldCache) { - return computeFn(); - } + if (!isCacheable) { + return computeFn(); + } - try { 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 5ae66f61039b..42a0b366537b 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 514fc3691f91..030944958ff8 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 99c2da53a7ea..bf55a03944f5 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 bd0868c58523..4674d5dc2fdf 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, + }); + } } }