From 876c606d315d6e45529177c3ae77baa03104f138 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Thu, 6 Jun 2024 05:13:59 +0530 Subject: [PATCH 01/15] Convert service worker to ts --- app/client/craco.dev.config.js | 2 +- app/client/src/ce/utils/serviceWorkerUtils.ts | 221 ++++++++++++++++++ app/client/src/ee/utils/serviceWorkerUtils.ts | 1 + .../{serviceWorker.js => serviceWorker.ts} | 68 ++++-- app/client/src/utils/serviceWorkerUtils.js | 177 -------------- 5 files changed, 267 insertions(+), 202 deletions(-) create mode 100644 app/client/src/ce/utils/serviceWorkerUtils.ts create mode 100644 app/client/src/ee/utils/serviceWorkerUtils.ts rename app/client/src/{serviceWorker.js => serviceWorker.ts} (59%) delete mode 100644 app/client/src/utils/serviceWorkerUtils.js diff --git a/app/client/craco.dev.config.js b/app/client/craco.dev.config.js index 8fcf53ac7f99..9834c2b38a36 100644 --- a/app/client/craco.dev.config.js +++ b/app/client/craco.dev.config.js @@ -24,7 +24,7 @@ module.exports = merge(common, { webpack: { plugins: [ new WorkboxPlugin.InjectManifest({ - swSrc: "./src/serviceWorker.js", + swSrc: "./src/serviceWorker.ts", mode: "development", swDest: "./pageService.js", exclude: [ diff --git a/app/client/src/ce/utils/serviceWorkerUtils.ts b/app/client/src/ce/utils/serviceWorkerUtils.ts new file mode 100644 index 000000000000..8516da7ce53c --- /dev/null +++ b/app/client/src/ce/utils/serviceWorkerUtils.ts @@ -0,0 +1,221 @@ +/* eslint-disable no-console */ +import { + matchBuilderPath, + matchViewerPath, +} from "@appsmith/constants/routes/appRoutes"; +import { Mutex } from "async-mutex"; +import { APP_MODE } from "entities/App"; + +interface TApplicationParams { + origin: string; + pageId?: string; + applicationId?: string; + branchName: string; + appMode: APP_MODE; +} + +type TApplicationParamsOrNull = TApplicationParams | null; + +/** + * returns the value in the query string for a key + */ +export const getSearchQuery = (search = "", key: string) => { + const params = new URLSearchParams(search); + return decodeURIComponent(params.get(key) || ""); +}; +export const getApplicationParamsFromUrl = ( + url: URL, +): TApplicationParamsOrNull => { + if (!url) { + return null; + } + + const branchName = getSearchQuery(url.search, "branch"); + + const matchedBuilder: { pageId?: string; applicationId?: string } = + matchBuilderPath(url.pathname, { + end: false, + }); + const matchedViewer: { pageId?: string; applicationId?: string } = + matchViewerPath(url.pathname); + + if (matchedBuilder) { + return { + origin: url.origin, + pageId: matchedBuilder.pageId, + applicationId: matchedBuilder.applicationId, + branchName, + appMode: APP_MODE.EDIT, + }; + } + + if (matchedViewer) { + return { + origin: url.origin, + pageId: matchedViewer.pageId, + applicationId: matchedViewer.applicationId, + branchName, + appMode: APP_MODE.PUBLISHED, + }; + } + + return null; +}; + +/** + * Function to get the prefetch request for consolidated api + */ +export const getConsolidatedApiPrefetchRequest = ( + applicationProps: TApplicationParams, +) => { + const { applicationId, appMode, branchName, origin, pageId } = + applicationProps; + + const headers = new Headers(); + const searchParams = new URLSearchParams(); + + if (!pageId) { + return null; + } + + searchParams.append("defaultPageId", pageId); + + if (applicationId) { + searchParams.append("applicationId", applicationId); + } + + // Add the branch name to the headers + if (branchName) { + headers.append("Branchname", branchName); + } + + // If the URL matches the builder path + if (appMode === APP_MODE.EDIT) { + const requestUrl = `${origin}/api/v1/consolidated-api/edit?${searchParams.toString()}`; + const request = new Request(requestUrl, { method: "GET", headers }); + return request; + } + + // If the URL matches the viewer path + if (appMode === APP_MODE.PUBLISHED) { + const requestUrl = `${origin}/api/v1/consolidated-api/view?${searchParams.toString()}`; + const request = new Request(requestUrl, { method: "GET", headers }); + return request; + } + + return null; +}; + +/** + * Function to get the prefetch request for consolidated api + */ +export const getPrefetchModuleApiRequests = ( + // eslint-disable-next-line @typescript-eslint/no-unused-vars + applicationProps: TApplicationParams, +): Request[] => { + return []; +}; + +/** + * Cache strategy for Appsmith API + */ +export class PrefetchApiCacheStrategy { + cacheName = "prefetch-cache-v1"; + cacheMaxAge = 2 * 60 * 1000; // 2 minutes in milliseconds + // Mutex to lock the fetch and cache operation + prefetchFetchMutexMap = new Map(); + + constructor() {} + + getRequestKey = (request: Request) => { + const headersKey = Array.from(request.headers.entries()) + .map(([key, value]) => `${key}:${value}`) + .join(","); + return `${request.method}:${request.url}:${headersKey}`; + }; + + aqcuireFetchMutex = async (request: Request) => { + const requestKey = this.getRequestKey(request); + let mutex = this.prefetchFetchMutexMap.get(requestKey); + + if (!mutex) { + mutex = new Mutex(); + this.prefetchFetchMutexMap.set(requestKey, mutex); + } + + return mutex.acquire(); + }; + + waitForUnlock = async (request: Request) => { + const requestKey = this.getRequestKey(request); + const mutex = this.prefetchFetchMutexMap.get(requestKey); + + if (mutex) { + return mutex.waitForUnlock(); + } + }; + + releaseFetchMutex = (request: Request) => { + const requestKey = this.getRequestKey(request); + const mutex = this.prefetchFetchMutexMap.get(requestKey); + + if (mutex) { + mutex.release(); + } + }; + + /** + * Function to fetch and cache the consolidated API + * @param {Request} request + * @returns + */ + async cacheConsolidatedApi(request: Request) { + // Acquire the lock + await this.aqcuireFetchMutex(request); + const prefetchApiCache = await caches.open(this.cacheName); + try { + const response = await fetch(request); + + if (response.ok) { + // Clone the response as the response can be consumed only once + const clonedResponse = response.clone(); + // Put the response in the cache + await prefetchApiCache.put(request, clonedResponse); + } + } catch (error) { + // Delete the existing cache if the fetch fails + await prefetchApiCache.delete(request); + } finally { + // Release the lock + this.releaseFetchMutex(request); + } + } + + async getCachedResponse(request: Request) { + // Wait for the lock to be released + await this.waitForUnlock(request); + const prefetchApiCache = await caches.open(this.cacheName); + // Check if the response is already in cache + const cachedResponse = await prefetchApiCache.match(request); + + if (cachedResponse) { + const dateHeader = cachedResponse.headers.get("date"); + const cachedTime = dateHeader ? new Date(dateHeader).getTime() : 0; + const currentTime = Date.now(); + + const isCacheValid = currentTime - cachedTime < this.cacheMaxAge; + + if (isCacheValid) { + // Delete the cache as this is a one-time cache + await prefetchApiCache.delete(request); + // Return the cached response + return cachedResponse; + } + + // If the cache is not valid, delete the cache + await prefetchApiCache.delete(request); + } + + return null; + } +} diff --git a/app/client/src/ee/utils/serviceWorkerUtils.ts b/app/client/src/ee/utils/serviceWorkerUtils.ts new file mode 100644 index 000000000000..96fdd24b8bc7 --- /dev/null +++ b/app/client/src/ee/utils/serviceWorkerUtils.ts @@ -0,0 +1 @@ +export * from "ce/utils/serviceWorkerUtils"; diff --git a/app/client/src/serviceWorker.js b/app/client/src/serviceWorker.ts similarity index 59% rename from app/client/src/serviceWorker.js rename to app/client/src/serviceWorker.ts index d97b6cc6cfd1..0585343adec0 100644 --- a/app/client/src/serviceWorker.js +++ b/app/client/src/serviceWorker.ts @@ -7,9 +7,12 @@ import { StaleWhileRevalidate, } from "workbox-strategies"; import { - getPrefetchConsolidatedApiRequest, - ConsolidatedApiCacheStrategy, -} from "utils/serviceWorkerUtils"; + getApplicationParamsFromUrl, + getConsolidatedApiPrefetchRequest, + getPrefetchModuleApiRequests, + PrefetchApiCacheStrategy, +} from "@appsmith/utils/serviceWorkerUtils"; +import type { RouteHandlerCallback } from "workbox-core/types"; setCacheNameDetails({ prefix: "appsmith", @@ -30,15 +33,16 @@ const regexMap = { /* eslint-disable no-restricted-globals */ // Note: if you need to filter out some files from precaching, + // do that in craco.build.config.js → workbox webpack plugin options -const toPrecache = self.__WB_MANIFEST; +const toPrecache = (self as any).__WB_MANIFEST; precacheAndRoute(toPrecache); -self.__WB_DISABLE_DEV_DEBUG_LOGS = false; +self.__WB_DISABLE_DEV_LOGS = false; skipWaiting(); clientsClaim(); -const consolidatedApiCacheStrategy = new ConsolidatedApiCacheStrategy(); +const prefetchApiCacheStrategy = new PrefetchApiCacheStrategy(); /** * @@ -47,16 +51,35 @@ const consolidatedApiCacheStrategy = new ConsolidatedApiCacheStrategy(); * @param {URL} url * @returns */ -const handleFetchHtml = async (event, request, url) => { - // Get the prefetch consolidated api request if the url matches the builder or viewer path - const prefetchConsolidatedApiRequest = getPrefetchConsolidatedApiRequest(url); - - if (prefetchConsolidatedApiRequest) { - consolidatedApiCacheStrategy - .cacheConsolidatedApi(prefetchConsolidatedApiRequest) - .catch(() => { - // Silently fail - }); +const handleFetchHtml: RouteHandlerCallback = async ({ + event, + request, + url, +}) => { + const applicationParams = getApplicationParamsFromUrl(url); + + if (applicationParams) { + const consolidatedApiPrefetchRequest = + getConsolidatedApiPrefetchRequest(applicationParams); + + if (consolidatedApiPrefetchRequest) { + prefetchApiCacheStrategy + .cacheConsolidatedApi(consolidatedApiPrefetchRequest) + .catch(() => { + // Silently fail + }); + } + + const moduleApiPrefetchRequests = + getPrefetchModuleApiRequests(applicationParams); + + moduleApiPrefetchRequests.forEach((prefetchRequest) => { + prefetchApiCacheStrategy + .cacheConsolidatedApi(prefetchRequest) + .catch(() => { + // Silently fail + }); + }); } const networkHandler = new NetworkOnly(); @@ -81,21 +104,18 @@ registerRoute(({ url }) => { }, new StaleWhileRevalidate()); registerRoute( - new Route( - ({ request, sameOrigin }) => { - return sameOrigin && request.destination === "document"; - }, - async ({ event, request, url }) => handleFetchHtml(event, request, url), - ), + new Route(({ request, sameOrigin }) => { + return sameOrigin && request.destination === "document"; + }, handleFetchHtml), ); // Route for fetching the API directly registerRoute( - new RegExp("/api/v1/consolidated-api/"), + new RegExp("/api/v1/(consolidated-api|moduleInstances)"), async ({ event, request }) => { // Check for cached response const cachedResponse = - await consolidatedApiCacheStrategy.getCachedResponse(request); + await prefetchApiCacheStrategy.getCachedResponse(request); // If the response is cached, return the response if (cachedResponse) { diff --git a/app/client/src/utils/serviceWorkerUtils.js b/app/client/src/utils/serviceWorkerUtils.js deleted file mode 100644 index f0d1464d22bb..000000000000 --- a/app/client/src/utils/serviceWorkerUtils.js +++ /dev/null @@ -1,177 +0,0 @@ -/* eslint-disable no-console */ -import { match } from "path-to-regexp"; -import { Mutex } from "async-mutex"; - -export const BUILDER_PATH = `/app/:applicationSlug/:pageSlug(.*\-):pageId/edit`; -export const BUILDER_CUSTOM_PATH = `/app/:customSlug(.*\-):pageId/edit`; -export const VIEWER_PATH = `/app/:applicationSlug/:pageSlug(.*\-):pageId`; -export const VIEWER_CUSTOM_PATH = `/app/:customSlug(.*\-):pageId`; -export const BUILDER_PATH_DEPRECATED = `/applications/:applicationId/pages/:pageId/edit`; -export const VIEWER_PATH_DEPRECATED = `/applications/:applicationId/pages/:pageId`; - -/** - * Function to get the search query from the URL - * @param {string} search - * @param {string} key - * @returns {string | null} - */ -export const getSearchQuery = (search = "", key) => { - const params = new URLSearchParams(search); - return decodeURIComponent(params.get(key) || ""); -}; - -/** - * Function to match the path with the builder path - * @param {string} pathName - * @param {Object} options - * @param {boolean} options.end - * @returns {Match | boolean} - */ -export const matchBuilderPath = (pathName, options) => - match(BUILDER_PATH, options)(pathName) || - match(BUILDER_PATH_DEPRECATED, options)(pathName) || - match(BUILDER_CUSTOM_PATH, options)(pathName); - -/** - * Function to match the path with the viewer path - * @param {string} pathName - * @returns {Match | boolean} - */ -export const matchViewerPath = (pathName) => - match(VIEWER_PATH)(pathName) || - match(VIEWER_PATH_DEPRECATED)(pathName) || - match(VIEWER_CUSTOM_PATH)(pathName); - -/** - * Function to get the consolidated API search params - * @param {Match} params - * @returns - */ -export const getConsolidatedAPISearchParams = (params = {}) => { - if (!params || !params?.pageId) { - return ""; - } - - const { applicationId, pageId } = params; - const searchParams = new URLSearchParams(); - - searchParams.append("defaultPageId", pageId); - - if (applicationId) { - searchParams.append("applicationId", applicationId); - } - - return searchParams.toString(); -}; - -/** - * Function to get the prefetch request for consolidated api - * @param {URL} url - * @returns {Request | null} - */ -export const getPrefetchConsolidatedApiRequest = (url) => { - if (!url) { - return null; - } - - // Match the URL with the builder and viewer paths - const matchedBuilder = matchBuilderPath(url.pathname, { end: false }); - const matchedViewer = matchViewerPath(url.pathname, { end: false }); - - // Get the branch name from the search query - const branchName = getSearchQuery(url.search, "branch"); - - let headers = new Headers(); - - // Add the branch name to the headers - if (branchName) { - headers.append("Branchname", branchName); - } - - // If the URL matches the builder path - if (matchedBuilder && matchedBuilder.params?.pageId) { - const searchParams = getConsolidatedAPISearchParams(matchedBuilder.params); - const requestUrl = `${url.origin}/api/v1/consolidated-api/edit?${searchParams}`; - const request = new Request(requestUrl, { method: "GET", headers }); - return request; - } - - // If the URL matches the viewer path - if (matchedViewer && matchedViewer.params?.pageId) { - const searchParams = getConsolidatedAPISearchParams(matchedViewer.params); - const requestUrl = `${url.origin}/api/v1/consolidated-api/view?${searchParams}`; - const request = new Request(requestUrl, { method: "GET", headers }); - return request; - } - - // Return null if the URL does not match the builder or viewer path - return null; -}; - -/** - * Cache strategy for Appsmith API - */ -export class ConsolidatedApiCacheStrategy { - cacheName = "prefetch-cache-v1"; - cacheMaxAge = 2 * 60 * 1000; // 2 minutes in milliseconds - - constructor() { - // Mutex to lock the fetch and cache operation - this.consolidatedApiFetchmutex = new Mutex(); - } - - /** - * Function to fetch and cache the consolidated API - * @param {Request} request - * @returns - */ - async cacheConsolidatedApi(request) { - // Acquire the lock - await this.consolidatedApiFetchmutex.acquire(); - const prefetchApiCache = await caches.open(this.cacheName); - try { - const response = await fetch(request); - - if (response.ok) { - // Clone the response as the response can be consumed only once - const clonedResponse = response.clone(); - // Put the response in the cache - await prefetchApiCache.put(request, clonedResponse); - } - } catch (error) { - // Delete the existing cache if the fetch fails - await prefetchApiCache.delete(request); - } finally { - // Release the lock - this.consolidatedApiFetchmutex.release(); - } - } - - async getCachedResponse(request) { - // Wait for the lock to be released - await this.consolidatedApiFetchmutex.waitForUnlock(); - const prefetchApiCache = await caches.open(this.cacheName); - // Check if the response is already in cache - const cachedResponse = await prefetchApiCache.match(request); - - if (cachedResponse) { - const dateHeader = cachedResponse.headers.get("date"); - const cachedTime = new Date(dateHeader).getTime(); - const currentTime = Date.now(); - - const isCacheValid = currentTime - cachedTime < this.cacheMaxAge; - - if (isCacheValid) { - // Delete the cache as this is a one-time cache - await prefetchApiCache.delete(request); - // Return the cached response - return cachedResponse; - } - - // If the cache is not valid, delete the cache - await prefetchApiCache.delete(request); - } - - return null; - } -} From 59e2a3cc164835517e0b211e3f007b3491e1ca6e Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Thu, 6 Jun 2024 05:25:28 +0530 Subject: [PATCH 02/15] Update ee import path --- app/client/src/ce/utils/serviceWorkerUtils.ts | 3 ++- app/client/src/ee/utils/serviceWorkerUtils.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/client/src/ce/utils/serviceWorkerUtils.ts b/app/client/src/ce/utils/serviceWorkerUtils.ts index 8516da7ce53c..ae0da7ad0761 100644 --- a/app/client/src/ce/utils/serviceWorkerUtils.ts +++ b/app/client/src/ce/utils/serviceWorkerUtils.ts @@ -19,10 +19,11 @@ type TApplicationParamsOrNull = TApplicationParams | null; /** * returns the value in the query string for a key */ -export const getSearchQuery = (search = "", key: string) => { +const getSearchQuery = (search = "", key: string) => { const params = new URLSearchParams(search); return decodeURIComponent(params.get(key) || ""); }; + export const getApplicationParamsFromUrl = ( url: URL, ): TApplicationParamsOrNull => { diff --git a/app/client/src/ee/utils/serviceWorkerUtils.ts b/app/client/src/ee/utils/serviceWorkerUtils.ts index 96fdd24b8bc7..3906e73eeba7 100644 --- a/app/client/src/ee/utils/serviceWorkerUtils.ts +++ b/app/client/src/ee/utils/serviceWorkerUtils.ts @@ -1 +1 @@ -export * from "ce/utils/serviceWorkerUtils"; +export * from "../../ce/utils/serviceWorkerUtils"; From 200295ef16db7727f430a719a07a82360f508ed7 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Thu, 6 Jun 2024 05:41:30 +0530 Subject: [PATCH 03/15] Export TApplicationParams from ce --- app/client/src/ce/utils/serviceWorkerUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/client/src/ce/utils/serviceWorkerUtils.ts b/app/client/src/ce/utils/serviceWorkerUtils.ts index ae0da7ad0761..b70b7af5ef29 100644 --- a/app/client/src/ce/utils/serviceWorkerUtils.ts +++ b/app/client/src/ce/utils/serviceWorkerUtils.ts @@ -6,7 +6,7 @@ import { import { Mutex } from "async-mutex"; import { APP_MODE } from "entities/App"; -interface TApplicationParams { +export interface TApplicationParams { origin: string; pageId?: string; applicationId?: string; From d65d2978dee9c7c74f67c5233f48dbefd32e2a32 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Thu, 6 Jun 2024 05:57:10 +0530 Subject: [PATCH 04/15] Change service worker source path --- app/client/craco.build.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/client/craco.build.config.js b/app/client/craco.build.config.js index dd3966785152..37765b6f363d 100644 --- a/app/client/craco.build.config.js +++ b/app/client/craco.build.config.js @@ -13,7 +13,7 @@ const plugins = []; plugins.push( new WorkboxPlugin.InjectManifest({ - swSrc: "./src/serviceWorker.js", + swSrc: "./src/serviceWorker.ts", mode: "development", swDest: "./pageService.js", maximumFileSizeToCacheInBytes: 11 * 1024 * 1024, From 5af3a7b27458f61d43ea354afd1064552e464f4e Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Thu, 6 Jun 2024 10:17:45 +0530 Subject: [PATCH 05/15] EE changes in service worker utils --- app/client/src/ee/utils/serviceWorkerUtils.ts | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/app/client/src/ee/utils/serviceWorkerUtils.ts b/app/client/src/ee/utils/serviceWorkerUtils.ts index 3906e73eeba7..dfe8905ed69d 100644 --- a/app/client/src/ee/utils/serviceWorkerUtils.ts +++ b/app/client/src/ee/utils/serviceWorkerUtils.ts @@ -1 +1,44 @@ export * from "../../ce/utils/serviceWorkerUtils"; + +import { APP_MODE } from "entities/App"; +import type { TApplicationParams } from "../../ce/utils/serviceWorkerUtils"; + +export const getPrefetchModuleApiRequests = ( + applicationProps: TApplicationParams, +): Request[] => { + const prefetchRequests: Request[] = []; + const { appMode, branchName, origin, pageId } = applicationProps; + + if (!pageId) { + return prefetchRequests; + } + + const searchParams = new URLSearchParams(); + + searchParams.append("contextId", pageId); + searchParams.append("contextType", "PAGE"); + searchParams.append("viewMode", (appMode === APP_MODE.PUBLISHED).toString()); + + const headers = new Headers(); + + if (branchName) { + headers.append("Branchname", branchName); + } + + const moduleInstanceUrl = `${origin}/api/v1/moduleInstances?${searchParams.toString()}`; + const moduleInstanceRequest = new Request(moduleInstanceUrl, { + method: "GET", + headers, + }); + prefetchRequests.push(moduleInstanceRequest); + + // Add module entities api + const moduleEntitiesUrl = `${origin}/api/v1/moduleInstances/entities?${searchParams.toString()}`; + const moduleEntitiesRequest = new Request(moduleEntitiesUrl, { + method: "GET", + headers, + }); + prefetchRequests.push(moduleEntitiesRequest); + + return prefetchRequests; +}; From 88901d074d5ee300e248fe0cc7873c631edb61ad Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Thu, 6 Jun 2024 10:30:24 +0530 Subject: [PATCH 06/15] rm jest test cases --- .../src/utils/serviceWorkerUtils.test.js | 383 ------------------ 1 file changed, 383 deletions(-) delete mode 100644 app/client/src/utils/serviceWorkerUtils.test.js diff --git a/app/client/src/utils/serviceWorkerUtils.test.js b/app/client/src/utils/serviceWorkerUtils.test.js deleted file mode 100644 index 04b03cdbfaf1..000000000000 --- a/app/client/src/utils/serviceWorkerUtils.test.js +++ /dev/null @@ -1,383 +0,0 @@ -import { - getSearchQuery, - getConsolidatedAPISearchParams, - getPrefetchConsolidatedApiRequest, - ConsolidatedApiCacheStrategy, - matchBuilderPath, - matchViewerPath, -} from "./serviceWorkerUtils"; -import { Headers, Request, Response } from "node-fetch"; - -global.fetch = jest.fn(); -global.caches = { - open: jest.fn().mockResolvedValue({ - match: jest.fn(), - put: jest.fn(), - delete: jest.fn(), - }), -}; - -describe("serviceWorkerUtils", () => { - describe("getSearchQuery", () => { - it("should return the search query from the URL", () => { - const search = "?key=value"; - const key = "key"; - const result = getSearchQuery(search, key); - expect(result).toEqual("value"); - }); - - it("should return an empty string if the key is not present in the URL", () => { - const search = "?key=value"; - const key = "invalid"; - const result = getSearchQuery(search, key); - expect(result).toEqual(""); - }); - - it("should return an empty string if the search query is empty", () => { - const search = ""; - const key = "key"; - const result = getSearchQuery(search, key); - expect(result).toEqual(""); - }); - - it("should return an empty string if the search query is null", () => { - const search = null; - const key = "key"; - const result = getSearchQuery(search, key); - expect(result).toEqual(""); - }); - }); - - describe("getConsolidatedAPISearchParams", () => { - it("should return the consolidated API search params", () => { - const params = { - applicationId: "appId", - pageId: "pageId", - }; - const result = getConsolidatedAPISearchParams(params); - expect(result).toEqual("defaultPageId=pageId&applicationId=appId"); - }); - - it("should return empty string search params with only the applicationId", () => { - const params = { - applicationId: "appId", - }; - const result = getConsolidatedAPISearchParams(params); - expect(result).toEqual(""); - }); - - it("should return the consolidated API search params with only the pageId", () => { - const params = { - pageId: "pageId", - }; - const result = getConsolidatedAPISearchParams(params); - expect(result).toEqual("defaultPageId=pageId"); - }); - - it("should return an empty string if the params are empty", () => { - const result = getConsolidatedAPISearchParams(); - expect(result).toEqual(""); - }); - - it("should return an empty string if the params are null", () => { - const result = getConsolidatedAPISearchParams(null); - expect(result).toEqual(""); - }); - - it("should return an empty string if the params are undefined", () => { - const result = getConsolidatedAPISearchParams(undefined); - expect(result).toEqual(""); - }); - }); - - describe("getPrefetchConsolidatedApiRequest", () => { - beforeAll(() => { - global.Request = Request; - global.Headers = Headers; - }); - - beforeEach(() => { - jest.clearAllMocks(); - }); - - it("should return null if url is not provided", () => { - expect(getPrefetchConsolidatedApiRequest(null)).toBeNull(); - }); - - it("should return null if url does not match any paths", () => { - const url = new URL("https://app.appsmith.com/unknown/path"); - expect(getPrefetchConsolidatedApiRequest(url)).toBeNull(); - }); - - it("should return a request for builder path", async () => { - const url = new URL( - "http://example.com/app/test-slug/test-page-123/edit?branch=test-branch", - ); - - const result = getPrefetchConsolidatedApiRequest(url); - const expectedResult = new Request( - "http://example.com/api/v1/consolidated-api/edit?defaultPageId=123", - { - method: "GET", - headers: { - BranchName: "test-branch", - }, - }, - ); - - expect(result).toEqual(expectedResult); - }); - - it("should return a request for viewer path", () => { - const url = new URL( - "https://app.appsmith.com/app/test-slug/test-page-123?branch=test-branch", - ); - - const expectedResult = new Request( - "https://app.appsmith.com/api/v1/consolidated-api/view?defaultPageId=123", - { - method: "GET", - headers: { - BranchName: "test-branch", - }, - }, - ); - - const result = getPrefetchConsolidatedApiRequest(url); - - expect(result).toEqual(expectedResult); - }); - - it("should return a request without branch name in headers", () => { - const url = new URL( - "https://app.appsmith.com/app/test-slug/test-page-123/edit", - ); - - const expectedResult = new Request( - "https://app.appsmith.com/api/v1/consolidated-api/edit?defaultPageId=123", - { - method: "GET", - headers: {}, - }, - ); - - const result = getPrefetchConsolidatedApiRequest(url); - - expect(result).toEqual(expectedResult); - }); - }); - - describe("ConsolidatedApiCacheStrategy", () => { - let strategy; - let mockCache; - - beforeAll(() => { - global.Request = Request; - global.Headers = Headers; - global.Response = Response; - }); - - beforeEach(async () => { - // Reset all mocks - jest.clearAllMocks(); - - mockCache = { - match: jest.fn(), - delete: jest.fn(), - put: jest.fn(), - }; - - global.caches = { - open: jest.fn().mockResolvedValue(mockCache), - }; - - strategy = new ConsolidatedApiCacheStrategy(); - }); - - it("should cache the API response", async () => { - const mockRequest = new Request("https://example.com/api"); - const mockResponse = new Response("mock data", { - status: 200, - headers: { date: new Date().toUTCString() }, - }); - - fetch.mockResolvedValue(mockResponse); - - await strategy.cacheConsolidatedApi(mockRequest); - - expect(fetch).toHaveBeenCalledWith(mockRequest); - expect(mockCache.put).toHaveBeenCalledWith( - mockRequest, - expect.any(Response), - ); - }); - - it("should handle errors when fetching the API response", async () => { - const mockRequest = new Request("https://example.com/api"); - - fetch.mockRejectedValue(new Error("Network error")); - - await strategy.cacheConsolidatedApi(mockRequest); - - expect(fetch).toHaveBeenCalledWith(mockRequest); - expect(mockCache.delete).toHaveBeenCalledWith(mockRequest); - }); - - it("should return a valid cached response", async () => { - const mockRequest = new Request("https://example.com/api"); - const mockResponse = new Response("mock data", { - status: 200, - headers: { date: new Date().toUTCString() }, - }); - - mockCache.match.mockResolvedValue(mockResponse); - - const cachedResponse = await strategy.getCachedResponse(mockRequest); - - expect(mockCache.match).toHaveBeenCalledWith(mockRequest); - expect(cachedResponse).toEqual(mockResponse); - }); - - it("should return null for an invalid cached response", async () => { - const mockRequest = new Request("https://example.com/api"); - const mockResponse = new Response("mock data", { - status: 200, - headers: { - date: new Date(Date.now() - (2 * 60 * 1000 + 1)).toUTCString(), - }, // 2 minutes 1 second old cache - }); - - mockCache.match.mockResolvedValue(mockResponse); - - const cachedResponse = await strategy.getCachedResponse(mockRequest); - - expect(mockCache.match).toHaveBeenCalledWith(mockRequest); - expect(mockCache.delete).toHaveBeenCalledWith(mockRequest); - expect(cachedResponse).toBeNull(); - }); - - it("should return null if no cached response is found", async () => { - const mockRequest = new Request("https://example.com/api"); - - mockCache.match.mockResolvedValue(null); - - const cachedResponse = await strategy.getCachedResponse(mockRequest); - - expect(mockCache.match).toHaveBeenCalledWith(mockRequest); - expect(cachedResponse).toBeNull(); - }); - }); - - describe("matchBuilderPath", () => { - it("should match the standard builder path", () => { - const pathName = "/app/applicationSlug/pageSlug-123/edit"; - const options = { end: false }; - const result = matchBuilderPath(pathName, options); - - expect(result).toBeTruthy(); - expect(result.params).toHaveProperty("applicationSlug"); - expect(result.params).toHaveProperty("pageSlug"); - expect(result.params).toHaveProperty("pageId", "123"); - }); - - it("should match the standard builder path for alphanumeric pageId", () => { - const pathName = - "/app/applicationSlug/pageSlug-6616733a6e70274710f21a07/edit"; - const options = { end: false }; - const result = matchBuilderPath(pathName, options); - - expect(result).toBeTruthy(); - expect(result.params).toHaveProperty("applicationSlug"); - expect(result.params).toHaveProperty("pageSlug"); - expect(result.params).toHaveProperty( - "pageId", - "6616733a6e70274710f21a07", - ); - }); - - it("should match the custom builder path", () => { - const pathName = "/app/customSlug-custom-456/edit"; - const options = { end: false }; - const result = matchBuilderPath(pathName, options); - - expect(result).toBeTruthy(); - expect(result.params).toHaveProperty("customSlug"); - expect(result.params).toHaveProperty("pageId", "456"); - }); - - it("should match the deprecated builder path", () => { - const pathName = "/applications/appId123/pages/456/edit"; - const options = { end: false }; - const result = matchBuilderPath(pathName, options); - - expect(result).toBeTruthy(); - expect(result.params).toHaveProperty("applicationId", "appId123"); - expect(result.params).toHaveProperty("pageId", "456"); - }); - - it("should not match incorrect builder path", () => { - const pathName = "/app/applicationSlug/nonMatching-123"; - const options = { end: false }; - const result = matchBuilderPath(pathName, options); - - expect(result).toBeFalsy(); - }); - - it("should not match when no pageId is present", () => { - const pathName = "/app/applicationSlug/pageSlug-edit"; - const options = { end: false }; - const result = matchBuilderPath(pathName, options); - - expect(result).toBeFalsy(); - }); - - it("should match when the path is edit widgets", () => { - const pathName = - "/app/applicationSlug/pageSlug-123/edit/widgets/t36hb2zukr"; - const options = { end: false }; - const result = matchBuilderPath(pathName, options); - - expect(result).toBeTruthy(); - expect(result.params).toHaveProperty("applicationSlug"); - expect(result.params).toHaveProperty("pageSlug"); - expect(result.params).toHaveProperty("pageId", "123"); - }); - }); - - describe("matchViewerPath", () => { - it("should match the standard viewer path", () => { - const pathName = "/app/applicationSlug/pageSlug-123"; - const result = matchViewerPath(pathName); - - expect(result).toBeTruthy(); - expect(result.params).toHaveProperty("applicationSlug"); - expect(result.params).toHaveProperty("pageSlug"); - expect(result.params).toHaveProperty("pageId", "123"); - }); - - it("should match the custom viewer path", () => { - const pathName = "/app/customSlug-custom-456"; - const result = matchViewerPath(pathName); - - expect(result).toBeTruthy(); - expect(result.params).toHaveProperty("customSlug"); - expect(result.params).toHaveProperty("pageId", "456"); - }); - - it("should match the deprecated viewer path", () => { - const pathName = "/applications/appId123/pages/456"; - const result = matchViewerPath(pathName); - - expect(result).toBeTruthy(); - expect(result.params).toHaveProperty("applicationId", "appId123"); - expect(result.params).toHaveProperty("pageId", "456"); - }); - - it("should not match when no pageId is present", () => { - const pathName = "/app/applicationSlug/pageSlug"; - const result = matchViewerPath(pathName); - - expect(result).toBeFalsy(); - }); - }); -}); From 631ee9711156ad38c1758521c06be95b7a3efae5 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Fri, 7 Jun 2024 08:12:07 +0530 Subject: [PATCH 07/15] Remove imports from constants/routes/appRoutes --- app/client/src/ce/utils/serviceWorkerUtils.ts | 53 +++++++++++++------ 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/app/client/src/ce/utils/serviceWorkerUtils.ts b/app/client/src/ce/utils/serviceWorkerUtils.ts index b70b7af5ef29..52fc375d73be 100644 --- a/app/client/src/ce/utils/serviceWorkerUtils.ts +++ b/app/client/src/ce/utils/serviceWorkerUtils.ts @@ -1,10 +1,35 @@ -/* eslint-disable no-console */ -import { - matchBuilderPath, - matchViewerPath, -} from "@appsmith/constants/routes/appRoutes"; import { Mutex } from "async-mutex"; import { APP_MODE } from "entities/App"; +import type { Match, TokensToRegexpOptions } from "path-to-regexp"; +import { match } from "path-to-regexp"; + +export const BUILDER_PATH = `/app/:applicationSlug/:pageSlug(.*\-):pageId/edit`; +export const BUILDER_CUSTOM_PATH = `/app/:customSlug(.*\-):pageId/edit`; +export const VIEWER_PATH = `/app/:applicationSlug/:pageSlug(.*\-):pageId`; +export const VIEWER_CUSTOM_PATH = `/app/:customSlug(.*\-):pageId`; +export const BUILDER_PATH_DEPRECATED = `/applications/:applicationId/pages/:pageId/edit`; +export const VIEWER_PATH_DEPRECATED = `/applications/:applicationId/pages/:pageId`; + +interface TMatchResult { + pageId?: string; + applicationId?: string; +} + +export const matchBuilderPath = ( + pathName: string, + options: TokensToRegexpOptions, +) => + match(BUILDER_PATH, options)(pathName) || + match(BUILDER_PATH_DEPRECATED, options)(pathName) || + match(BUILDER_CUSTOM_PATH, options)(pathName); + +/** + * Function to match the path with the viewer path + */ +export const matchViewerPath = (pathName: string) => + match(VIEWER_PATH)(pathName) || + match(VIEWER_PATH_DEPRECATED)(pathName) || + match(VIEWER_CUSTOM_PATH)(pathName); export interface TApplicationParams { origin: string; @@ -33,18 +58,16 @@ export const getApplicationParamsFromUrl = ( const branchName = getSearchQuery(url.search, "branch"); - const matchedBuilder: { pageId?: string; applicationId?: string } = - matchBuilderPath(url.pathname, { - end: false, - }); - const matchedViewer: { pageId?: string; applicationId?: string } = - matchViewerPath(url.pathname); + const matchedBuilder: Match = matchBuilderPath(url.pathname, { + end: false, + }); + const matchedViewer: Match = matchViewerPath(url.pathname); if (matchedBuilder) { return { origin: url.origin, - pageId: matchedBuilder.pageId, - applicationId: matchedBuilder.applicationId, + pageId: matchedBuilder.params.pageId, + applicationId: matchedBuilder.params.applicationId, branchName, appMode: APP_MODE.EDIT, }; @@ -53,8 +76,8 @@ export const getApplicationParamsFromUrl = ( if (matchedViewer) { return { origin: url.origin, - pageId: matchedViewer.pageId, - applicationId: matchedViewer.applicationId, + pageId: matchedViewer.params.pageId, + applicationId: matchedViewer.params.applicationId, branchName, appMode: APP_MODE.PUBLISHED, }; From 3b045f0718d21ab7b409536180d8c8f259a1d2c2 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Fri, 7 Jun 2024 10:25:02 +0530 Subject: [PATCH 08/15] Remove ee code --- app/client/src/ee/utils/serviceWorkerUtils.ts | 43 ------------------- 1 file changed, 43 deletions(-) diff --git a/app/client/src/ee/utils/serviceWorkerUtils.ts b/app/client/src/ee/utils/serviceWorkerUtils.ts index dfe8905ed69d..3906e73eeba7 100644 --- a/app/client/src/ee/utils/serviceWorkerUtils.ts +++ b/app/client/src/ee/utils/serviceWorkerUtils.ts @@ -1,44 +1 @@ export * from "../../ce/utils/serviceWorkerUtils"; - -import { APP_MODE } from "entities/App"; -import type { TApplicationParams } from "../../ce/utils/serviceWorkerUtils"; - -export const getPrefetchModuleApiRequests = ( - applicationProps: TApplicationParams, -): Request[] => { - const prefetchRequests: Request[] = []; - const { appMode, branchName, origin, pageId } = applicationProps; - - if (!pageId) { - return prefetchRequests; - } - - const searchParams = new URLSearchParams(); - - searchParams.append("contextId", pageId); - searchParams.append("contextType", "PAGE"); - searchParams.append("viewMode", (appMode === APP_MODE.PUBLISHED).toString()); - - const headers = new Headers(); - - if (branchName) { - headers.append("Branchname", branchName); - } - - const moduleInstanceUrl = `${origin}/api/v1/moduleInstances?${searchParams.toString()}`; - const moduleInstanceRequest = new Request(moduleInstanceUrl, { - method: "GET", - headers, - }); - prefetchRequests.push(moduleInstanceRequest); - - // Add module entities api - const moduleEntitiesUrl = `${origin}/api/v1/moduleInstances/entities?${searchParams.toString()}`; - const moduleEntitiesRequest = new Request(moduleEntitiesUrl, { - method: "GET", - headers, - }); - prefetchRequests.push(moduleEntitiesRequest); - - return prefetchRequests; -}; From 700da976ef5391fa633026b12ea8b3c6598e53e5 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Fri, 7 Jun 2024 12:38:26 +0530 Subject: [PATCH 09/15] Refactor --- app/client/package.json | 1 + app/client/src/ce/utils/serviceWorkerUtils.ts | 45 +++++++++++-------- app/client/src/serviceWorker.ts | 29 ++++++------ app/client/yarn.lock | 11 +++++ 4 files changed, 53 insertions(+), 33 deletions(-) diff --git a/app/client/package.json b/app/client/package.json index 49d6eace1ff6..bd18d3c6fbe4 100644 --- a/app/client/package.json +++ b/app/client/package.json @@ -270,6 +270,7 @@ "@types/moment-timezone": "^0.5.10", "@types/nanoid": "^2.0.0", "@types/node": "^10.12.18", + "@types/node-fetch": "^2.6.11", "@types/node-forge": "^0.10.0", "@types/object-hash": "^2.2.1", "@types/pg": "^8.10.2", diff --git a/app/client/src/ce/utils/serviceWorkerUtils.ts b/app/client/src/ce/utils/serviceWorkerUtils.ts index 52fc375d73be..b33f0101e5b1 100644 --- a/app/client/src/ce/utils/serviceWorkerUtils.ts +++ b/app/client/src/ce/utils/serviceWorkerUtils.ts @@ -15,6 +15,21 @@ interface TMatchResult { applicationId?: string; } +export interface TApplicationParams { + origin: string; + pageId?: string; + applicationId?: string; + branchName: string; + appMode: APP_MODE; +} + +type TApplicationParamsOrNull = TApplicationParams | null; + +export const cachedApiUrlRegex = new RegExp("/api/v1/consolidated-api/"); + +/** + * Function to match the path with the builder path + */ export const matchBuilderPath = ( pathName: string, options: TokensToRegexpOptions, @@ -31,20 +46,10 @@ export const matchViewerPath = (pathName: string) => match(VIEWER_PATH_DEPRECATED)(pathName) || match(VIEWER_CUSTOM_PATH)(pathName); -export interface TApplicationParams { - origin: string; - pageId?: string; - applicationId?: string; - branchName: string; - appMode: APP_MODE; -} - -type TApplicationParamsOrNull = TApplicationParams | null; - /** * returns the value in the query string for a key */ -const getSearchQuery = (search = "", key: string) => { +export const getSearchQuery = (search = "", key: string) => { const params = new URLSearchParams(search); return decodeURIComponent(params.get(key) || ""); }; @@ -52,10 +57,7 @@ const getSearchQuery = (search = "", key: string) => { export const getApplicationParamsFromUrl = ( url: URL, ): TApplicationParamsOrNull => { - if (!url) { - return null; - } - + // Get the branch name from the query string const branchName = getSearchQuery(url.search, "branch"); const matchedBuilder: Match = matchBuilderPath(url.pathname, { @@ -131,7 +133,7 @@ export const getConsolidatedApiPrefetchRequest = ( }; /** - * Function to get the prefetch request for consolidated api + * Function to get the prefetch request for module apis */ export const getPrefetchModuleApiRequests = ( // eslint-disable-next-line @typescript-eslint/no-unused-vars @@ -151,6 +153,7 @@ export class PrefetchApiCacheStrategy { constructor() {} + // Function to get the request key getRequestKey = (request: Request) => { const headersKey = Array.from(request.headers.entries()) .map(([key, value]) => `${key}:${value}`) @@ -158,6 +161,7 @@ export class PrefetchApiCacheStrategy { return `${request.method}:${request.url}:${headersKey}`; }; + // Function to acquire the fetch mutex for a request aqcuireFetchMutex = async (request: Request) => { const requestKey = this.getRequestKey(request); let mutex = this.prefetchFetchMutexMap.get(requestKey); @@ -170,6 +174,7 @@ export class PrefetchApiCacheStrategy { return mutex.acquire(); }; + // Function to wait for the lock to be released for a request waitForUnlock = async (request: Request) => { const requestKey = this.getRequestKey(request); const mutex = this.prefetchFetchMutexMap.get(requestKey); @@ -179,6 +184,7 @@ export class PrefetchApiCacheStrategy { } }; + // Function to release the fetch mutex for a request releaseFetchMutex = (request: Request) => { const requestKey = this.getRequestKey(request); const mutex = this.prefetchFetchMutexMap.get(requestKey); @@ -190,10 +196,8 @@ export class PrefetchApiCacheStrategy { /** * Function to fetch and cache the consolidated API - * @param {Request} request - * @returns */ - async cacheConsolidatedApi(request: Request) { + async cacheApi(request: Request) { // Acquire the lock await this.aqcuireFetchMutex(request); const prefetchApiCache = await caches.open(this.cacheName); @@ -215,6 +219,9 @@ export class PrefetchApiCacheStrategy { } } + /** + * Function to get the cached response for the request + */ async getCachedResponse(request: Request) { // Wait for the lock to be released await this.waitForUnlock(request); diff --git a/app/client/src/serviceWorker.ts b/app/client/src/serviceWorker.ts index 0585343adec0..4b43d91b38b2 100644 --- a/app/client/src/serviceWorker.ts +++ b/app/client/src/serviceWorker.ts @@ -7,6 +7,7 @@ import { StaleWhileRevalidate, } from "workbox-strategies"; import { + cachedApiUrlRegex, getApplicationParamsFromUrl, getConsolidatedApiPrefetchRequest, getPrefetchModuleApiRequests, @@ -45,40 +46,39 @@ clientsClaim(); const prefetchApiCacheStrategy = new PrefetchApiCacheStrategy(); /** - * - * @param {ExtendableEvent} event - * @param {Request} request - * @param {URL} url - * @returns + * Route handler callback for HTML pages. + * This callback is responsible for prefetching the API requests for the application page. */ -const handleFetchHtml: RouteHandlerCallback = async ({ +const htmlRouteHandlerCallback: RouteHandlerCallback = async ({ event, request, url, }) => { + // Extract application params from the URL const applicationParams = getApplicationParamsFromUrl(url); + // If application params are present, prefetch the API requests for the application if (applicationParams) { + // Prefetch the consolidated API request const consolidatedApiPrefetchRequest = getConsolidatedApiPrefetchRequest(applicationParams); if (consolidatedApiPrefetchRequest) { prefetchApiCacheStrategy - .cacheConsolidatedApi(consolidatedApiPrefetchRequest) + .cacheApi(consolidatedApiPrefetchRequest) .catch(() => { // Silently fail }); } + // Prefetch the module API requests const moduleApiPrefetchRequests = getPrefetchModuleApiRequests(applicationParams); moduleApiPrefetchRequests.forEach((prefetchRequest) => { - prefetchApiCacheStrategy - .cacheConsolidatedApi(prefetchRequest) - .catch(() => { - // Silently fail - }); + prefetchApiCacheStrategy.cacheApi(prefetchRequest).catch(() => { + // Silently fail + }); }); } @@ -106,12 +106,13 @@ registerRoute(({ url }) => { registerRoute( new Route(({ request, sameOrigin }) => { return sameOrigin && request.destination === "document"; - }, handleFetchHtml), + }, htmlRouteHandlerCallback), ); // Route for fetching the API directly registerRoute( - new RegExp("/api/v1/(consolidated-api|moduleInstances)"), + // Intercept requests to the consolidated API and module instances API + cachedApiUrlRegex, async ({ event, request }) => { // Check for cached response const cachedResponse = diff --git a/app/client/yarn.lock b/app/client/yarn.lock index fd748aabb708..fd449c352479 100644 --- a/app/client/yarn.lock +++ b/app/client/yarn.lock @@ -11038,6 +11038,16 @@ __metadata: languageName: node linkType: hard +"@types/node-fetch@npm:^2.6.11": + version: 2.6.11 + resolution: "@types/node-fetch@npm:2.6.11" + dependencies: + "@types/node": "*" + form-data: ^4.0.0 + checksum: 180e4d44c432839bdf8a25251ef8c47d51e37355ddd78c64695225de8bc5dc2b50b7bb855956d471c026bb84bd7295688a0960085e7158cbbba803053492568b + languageName: node + linkType: hard + "@types/node-forge@npm:^0.10.0": version: 0.10.0 resolution: "@types/node-forge@npm:0.10.0" @@ -13131,6 +13141,7 @@ __metadata: "@types/moment-timezone": ^0.5.10 "@types/nanoid": ^2.0.0 "@types/node": ^10.12.18 + "@types/node-fetch": ^2.6.11 "@types/node-forge": ^0.10.0 "@types/object-hash": ^2.2.1 "@types/pg": ^8.10.2 From 787bc5aae8650e2ec59d22d756b09f00cc4bef16 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Sat, 8 Jun 2024 16:19:22 +0530 Subject: [PATCH 10/15] Add jest test cases --- .../src/ce/utils/serviceWorkerUtils.test.ts | 613 ++++++++++++++++++ app/client/src/ce/utils/serviceWorkerUtils.ts | 32 +- 2 files changed, 629 insertions(+), 16 deletions(-) create mode 100644 app/client/src/ce/utils/serviceWorkerUtils.test.ts diff --git a/app/client/src/ce/utils/serviceWorkerUtils.test.ts b/app/client/src/ce/utils/serviceWorkerUtils.test.ts new file mode 100644 index 000000000000..743b069e2eb6 --- /dev/null +++ b/app/client/src/ce/utils/serviceWorkerUtils.test.ts @@ -0,0 +1,613 @@ +import { APP_MODE } from "entities/App"; +import type { TApplicationParams } from "./serviceWorkerUtils"; +import { + matchBuilderPath, + matchViewerPath, + getSearchQuery, + getConsolidatedApiPrefetchRequest, + getApplicationParamsFromUrl, + getPrefetchModuleApiRequests, + PrefetchApiCacheStrategy, +} from "./serviceWorkerUtils"; +import { Mutex } from "async-mutex"; +import { Request as NFRequest, Response as NFResponse } from "node-fetch"; + +(global as any).fetch = jest.fn() as jest.Mock; +(global as any).caches = { + open: jest.fn(), + delete: jest.fn(), +}; + +describe("serviceWorkerUtils", () => { + describe("matchBuilderPath", () => { + it("should match the standard builder path", () => { + const pathName = "/app/applicationSlug/pageSlug-123/edit"; + const options = { end: false }; + const result = matchBuilderPath(pathName, options); + + expect(result).toBeTruthy(); + if (result) { + expect(result.params).toHaveProperty("applicationSlug"); + expect(result.params).toHaveProperty("pageSlug"); + expect(result.params).toHaveProperty("pageId", "123"); + } else { + fail("Expected result to be truthy"); + } + }); + + it("should match the standard builder path for alphanumeric pageId", () => { + const pathName = + "/app/applicationSlug/pageSlug-6616733a6e70274710f21a07/edit"; + const options = { end: false }; + const result = matchBuilderPath(pathName, options); + + expect(result).toBeTruthy(); + if (result) { + expect(result.params).toHaveProperty("applicationSlug"); + expect(result.params).toHaveProperty("pageSlug"); + expect(result.params).toHaveProperty( + "pageId", + "6616733a6e70274710f21a07", + ); + } else { + fail("Expected result to be truthy"); + } + }); + + it("should match the custom builder path", () => { + const pathName = "/app/customSlug-custom-456/edit"; + const options = { end: false }; + const result = matchBuilderPath(pathName, options); + + expect(result).toBeTruthy(); + if (result) { + expect(result.params).toHaveProperty("customSlug"); + expect(result.params).toHaveProperty("pageId", "456"); + } else { + fail("Expected result to be truthy"); + } + }); + + it("should match the deprecated builder path", () => { + const pathName = "/applications/appId123/pages/456/edit"; + const options = { end: false }; + const result = matchBuilderPath(pathName, options); + + expect(result).toBeTruthy(); + if (result) { + expect(result.params).toHaveProperty("applicationId", "appId123"); + expect(result.params).toHaveProperty("pageId", "456"); + } else { + fail("Expected result to be truthy"); + } + }); + + it("should not match incorrect builder path", () => { + const pathName = "/app/applicationSlug/nonMatching-123"; + const options = { end: false }; + const result = matchBuilderPath(pathName, options); + + expect(result).toBeFalsy(); + }); + + it("should not match when no pageId is present", () => { + const pathName = "/app/applicationSlug/pageSlug-edit"; + const options = { end: false }; + const result = matchBuilderPath(pathName, options); + + expect(result).toBeFalsy(); + }); + + it("should match when the path is edit widgets", () => { + const pathName = + "/app/applicationSlug/pageSlug-123/edit/widgets/t36hb2zukr"; + const options = { end: false }; + const result = matchBuilderPath(pathName, options); + + if (result) { + expect(result.params).toHaveProperty("applicationSlug"); + expect(result.params).toHaveProperty("pageSlug"); + expect(result.params).toHaveProperty("pageId", "123"); + } else { + fail("Expected result to be truthy"); + } + }); + }); + + describe("matchViewerPath", () => { + it("should match the standard viewer path", () => { + const pathName = "/app/applicationSlug/pageSlug-123"; + const result = matchViewerPath(pathName); + + expect(result).toBeTruthy(); + if (result) { + expect(result.params).toHaveProperty("applicationSlug"); + expect(result.params).toHaveProperty("pageSlug"); + expect(result.params).toHaveProperty("pageId", "123"); + } else { + fail("Expected result to be truthy"); + } + }); + + it("should match the custom viewer path", () => { + const pathName = "/app/customSlug-custom-456"; + const result = matchViewerPath(pathName); + + expect(result).toBeTruthy(); + if (result) { + expect(result.params).toHaveProperty("customSlug"); + expect(result.params).toHaveProperty("pageId", "456"); + } else { + fail("Expected result to be truthy"); + } + }); + + it("should match the deprecated viewer path", () => { + const pathName = "/applications/appId123/pages/456"; + const result = matchViewerPath(pathName); + + expect(result).toBeTruthy(); + if (result) { + expect(result.params).toHaveProperty("applicationId", "appId123"); + expect(result.params).toHaveProperty("pageId", "456"); + } else { + fail("Expected result to be truthy"); + } + }); + + it("should not match when no pageId is present", () => { + const pathName = "/app/applicationSlug/pageSlug"; + const result = matchViewerPath(pathName); + + expect(result).toBeFalsy(); + }); + }); + + describe("getSearchQuery", () => { + it("should return the search query from the URL", () => { + const search = "?key=value"; + const key = "key"; + const result = getSearchQuery(search, key); + expect(result).toEqual("value"); + }); + + it("should return an empty string if the key is not present in the URL", () => { + const search = "?key=value"; + const key = "invalid"; + const result = getSearchQuery(search, key); + expect(result).toEqual(""); + }); + + it("should return an empty string if the search query is empty", () => { + const search = ""; + const key = "key"; + const result = getSearchQuery(search, key); + expect(result).toEqual(""); + }); + }); + + describe("getApplicationParamsFromUrl", () => { + it("should parse URL and return correct params for builder path", () => { + const url = new URL( + "https://app.appsmith.com/app/my-app/page-123/edit?branch=main", + ); + const expectedParams: TApplicationParams = { + origin: "https://app.appsmith.com", + pageId: "123", + applicationId: undefined, + branchName: "main", + appMode: APP_MODE.EDIT, + }; + + expect(getApplicationParamsFromUrl(url)).toEqual(expectedParams); + }); + + it("should parse URL and return correct params for viewer path", () => { + const url = new URL( + "https://app.appsmith.com/app/my-app/page-123?branch=main", + ); + const expectedParams: TApplicationParams = { + origin: "https://app.appsmith.com", + pageId: "123", + applicationId: undefined, + branchName: "main", + appMode: APP_MODE.PUBLISHED, + }; + + expect(getApplicationParamsFromUrl(url)).toEqual(expectedParams); + }); + + it("should return null if the path does not match any pattern", () => { + const url = new URL("https://app.appsmith.com/invalid/path?branch=main"); + expect(getApplicationParamsFromUrl(url)).toBeNull(); + }); + + it("should parse deprecated builder path and return correct params", () => { + const url = new URL( + "https://app.appsmith.com/applications/app-123/pages/page-123/edit?branch=main", + ); + const expectedParams: TApplicationParams = { + origin: "https://app.appsmith.com", + pageId: "page-123", + applicationId: "app-123", + branchName: "main", + appMode: APP_MODE.EDIT, + }; + + expect(getApplicationParamsFromUrl(url)).toEqual(expectedParams); + }); + + it("should parse deprecated viewer path and return correct params", () => { + const url = new URL( + "https://app.appsmith.com/applications/app-123/pages/page-123?branch=main", + ); + const expectedParams: TApplicationParams = { + origin: "https://app.appsmith.com", + pageId: "page-123", + applicationId: "app-123", + branchName: "main", + appMode: APP_MODE.PUBLISHED, + }; + + expect(getApplicationParamsFromUrl(url)).toEqual(expectedParams); + }); + + it("should parse custom builder path and return correct params", () => { + const url = new URL( + "https://app.appsmith.com/app/custom-app-123/edit?branch=main", + ); + const expectedParams: TApplicationParams = { + origin: "https://app.appsmith.com", + pageId: "123", + applicationId: undefined, + branchName: "main", + appMode: APP_MODE.EDIT, + }; + + expect(getApplicationParamsFromUrl(url)).toEqual(expectedParams); + }); + + it("should parse custom viewer path and return correct params", () => { + const url = new URL( + "https://app.appsmith.com/app/custom-app-123?branch=main", + ); + const expectedParams: TApplicationParams = { + origin: "https://app.appsmith.com", + pageId: "123", + applicationId: undefined, + branchName: "main", + appMode: APP_MODE.PUBLISHED, + }; + + expect(getApplicationParamsFromUrl(url)).toEqual(expectedParams); + }); + + it("should parse URL and return params with empty branch name if branch query param is not present", () => { + const url = new URL("https://app.appsmith.com/app/my-app/page-123/edit"); + const expectedParams: TApplicationParams = { + origin: "https://app.appsmith.com", + pageId: "123", + applicationId: undefined, + branchName: "", + appMode: APP_MODE.EDIT, + }; + + expect(getApplicationParamsFromUrl(url)).toEqual(expectedParams); + }); + }); + + describe("getConsolidatedApiPrefetchRequest", () => { + beforeAll(() => { + (global as any).Request = NFRequest; + }); + + it("should return null if pageId is not provided", () => { + const params: TApplicationParams = { + origin: "https://app.appsmith.com", + branchName: "main", + appMode: APP_MODE.EDIT, + }; + + expect(getConsolidatedApiPrefetchRequest(params)).toBeNull(); + }); + + it("should create request for EDIT mode with applicationId", () => { + const params: TApplicationParams = { + origin: "https://app.appsmith.com", + pageId: "page123", + applicationId: "app123", + branchName: "main", + appMode: APP_MODE.EDIT, + }; + + const request = getConsolidatedApiPrefetchRequest(params); + expect(request).toBeInstanceOf(Request); + expect(request?.url).toBe( + "https://app.appsmith.com/api/v1/consolidated-api/edit?defaultPageId=page123&applicationId=app123", + ); + expect(request?.method).toBe("GET"); + expect(request?.headers.get("Branchname")).toBe("main"); + }); + + it("should create request for PUBLISHED mode with applicationId", () => { + const params: TApplicationParams = { + origin: "https://app.appsmith.com", + pageId: "page123", + applicationId: "app123", + branchName: "main", + appMode: APP_MODE.PUBLISHED, + }; + + const request = getConsolidatedApiPrefetchRequest(params); + expect(request).toBeInstanceOf(Request); + expect(request?.url).toBe( + "https://app.appsmith.com/api/v1/consolidated-api/view?defaultPageId=page123&applicationId=app123", + ); + expect(request?.method).toBe("GET"); + expect(request?.headers.get("Branchname")).toBe("main"); + }); + + it("should create request for EDIT mode without applicationId", () => { + const params: TApplicationParams = { + origin: "https://app.appsmith.com", + pageId: "page123", + branchName: "main", + appMode: APP_MODE.EDIT, + }; + + const request = getConsolidatedApiPrefetchRequest(params); + expect(request).toBeInstanceOf(Request); + expect(request?.url).toBe( + "https://app.appsmith.com/api/v1/consolidated-api/edit?defaultPageId=page123", + ); + expect(request?.method).toBe("GET"); + expect(request?.headers.get("Branchname")).toBe("main"); + }); + + it("should create request for PUBLISHED mode without applicationId", () => { + const params: TApplicationParams = { + origin: "https://app.appsmith.com", + pageId: "page123", + branchName: "main", + appMode: APP_MODE.PUBLISHED, + }; + + const request = getConsolidatedApiPrefetchRequest(params); + expect(request).toBeInstanceOf(Request); + expect(request?.url).toBe( + "https://app.appsmith.com/api/v1/consolidated-api/view?defaultPageId=page123", + ); + expect(request?.method).toBe("GET"); + expect(request?.headers.get("Branchname")).toBe("main"); + }); + + it("should return null for an unknown app mode", () => { + const params: TApplicationParams = { + origin: "https://app.appsmith.com", + pageId: "page123", + branchName: "main", + appMode: "UNKNOWN" as APP_MODE, + }; + + expect(getConsolidatedApiPrefetchRequest(params)).toBeNull(); + }); + }); + + describe("getPrefetchModuleApiRequests", () => { + it("should return empty array", () => { + const params: TApplicationParams = { + origin: "https://app.appsmith.com", + branchName: "main", + appMode: APP_MODE.EDIT, + pageId: "page123", + applicationId: "app123", + }; + const requests = getPrefetchModuleApiRequests(params); + expect(requests).toHaveLength(0); + }); + }); + + describe("PrefetchApiCacheStrategy", () => { + let cacheStrategy: PrefetchApiCacheStrategy; + let mockCache: any; + + beforeEach(() => { + cacheStrategy = new PrefetchApiCacheStrategy(); + mockCache = { + put: jest.fn(), + match: jest.fn(), + delete: jest.fn(), + }; + (global as any).caches.open.mockResolvedValue(mockCache); + (global as any).Request = NFRequest; + (global as any).Response = NFResponse; + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe("getRequestKey", () => { + it("should return the correct request key", () => { + const request = new Request("https://app.appsmith.com", { + method: "GET", + }); + request.headers.append("branchname", "main"); + const key = cacheStrategy.getRequestKey(request); + expect(key).toBe("GET:https://app.appsmith.com/:branchname:main"); + }); + }); + + describe("aqcuireFetchMutex", () => { + it("should acquire a new mutex if not present", async () => { + const request = new Request("https://app.appsmith.com", { + method: "GET", + }); + const acquireSpy = jest.spyOn(Mutex.prototype, "acquire"); + await cacheStrategy.aqcuireFetchMutex(request); + expect(acquireSpy).toHaveBeenCalled(); + }); + + it("should reuse existing mutex if present", async () => { + const request = new Request("https://app.appsmith.com", { + method: "GET", + }); + const mutex = new Mutex(); + cacheStrategy.prefetchFetchMutexMap.set( + cacheStrategy.getRequestKey(request), + mutex, + ); + const acquireSpy = jest.spyOn(mutex, "acquire"); + await cacheStrategy.aqcuireFetchMutex(request); + expect(acquireSpy).toHaveBeenCalled(); + }); + }); + + describe("waitForUnlock", () => { + it("should wait for the mutex to unlock if it exists", async () => { + const request = new Request("https://app.appsmith.com", { + method: "GET", + }); + const mutex = new Mutex(); + cacheStrategy.prefetchFetchMutexMap.set( + cacheStrategy.getRequestKey(request), + mutex, + ); + const waitForUnlockSpy = jest.spyOn(mutex, "waitForUnlock"); + await cacheStrategy.waitForUnlock(request); + expect(waitForUnlockSpy).toHaveBeenCalled(); + }); + + it("should do nothing if the mutex does not exist", async () => { + const request = new Request("https://app.appsmith.com", { + method: "GET", + }); + await expect( + cacheStrategy.waitForUnlock(request), + ).resolves.not.toThrow(); + }); + }); + + describe("releaseFetchMutex", () => { + it("should release the mutex if it exists", () => { + const request = new Request("https://app.appsmith.com", { + method: "GET", + }); + const mutex = new Mutex(); + cacheStrategy.prefetchFetchMutexMap.set( + cacheStrategy.getRequestKey(request), + mutex, + ); + const releaseSpy = jest.spyOn(mutex, "release"); + cacheStrategy.releaseFetchMutex(request); + expect(releaseSpy).toHaveBeenCalled(); + }); + + it("should do nothing if the mutex does not exist", () => { + const request = new Request("https://app.appsmith.com", { + method: "GET", + }); + expect(() => cacheStrategy.releaseFetchMutex(request)).not.toThrow(); + }); + }); + + describe("cacheApi", () => { + it("should acquire the mutex, fetch the request, cache the response, and release the mutex", async () => { + const request = new Request("https://app.appsmith.com/sdfsdf", { + method: "GET", + }); + const response = new Response("Test response", { + status: 200, + statusText: "OK", + }); + + (global as any).fetch.mockResolvedValue(response); + + const acquireSpy = jest.spyOn(Mutex.prototype, "acquire"); + const releaseSpy = jest.spyOn(Mutex.prototype, "release"); + + await cacheStrategy.cacheApi(request); + + expect(acquireSpy).toHaveBeenCalled(); + expect((global as any).fetch).toHaveBeenCalledWith(request); + expect(mockCache.put).toHaveBeenCalledWith( + request, + expect.objectContaining({ + status: 200, + statusText: "OK", + }), + ); + expect(releaseSpy).toHaveBeenCalled(); + }); + + it("should delete the cache and release the mutex if fetch fails", async () => { + const request = new Request("https://app.appsmith.com", { + method: "GET", + }); + (global as any).fetch.mockRejectedValue(new Error("Fetch error")); + + const acquireSpy = jest.spyOn(Mutex.prototype, "acquire"); + const releaseSpy = jest.spyOn(Mutex.prototype, "release"); + + await cacheStrategy.cacheApi(request); + + expect(acquireSpy).toHaveBeenCalled(); + expect(mockCache.delete).toHaveBeenCalledWith(request); + expect(releaseSpy).toHaveBeenCalled(); + }); + }); + + describe("getCachedResponse", () => { + it("should wait for the mutex to unlock, get the cached response if valid, and delete it afterwards", async () => { + const request = new Request("https://app.appsmith.com", { + method: "GET", + }); + const response = new Response("test response", { + headers: { date: new Date(Date.now() - 1000).toUTCString() }, + }); + mockCache.match.mockResolvedValue(response); + const mutex = new Mutex(); + cacheStrategy.prefetchFetchMutexMap.set( + cacheStrategy.getRequestKey(request), + mutex, + ); + const waitForUnlockSpy = jest.spyOn(Mutex.prototype, "waitForUnlock"); + + const cachedResponse = await cacheStrategy.getCachedResponse(request); + + expect(waitForUnlockSpy).toHaveBeenCalled(); + expect(mockCache.match).toHaveBeenCalledWith(request); + expect(mockCache.delete).toHaveBeenCalledWith(request); + expect(cachedResponse).toBe(response); + }); + + it("should return null if the cache is invalid", async () => { + const request = new Request("https://app.appsmith.com", { + method: "GET", + }); + const response = new Response("test response", { + headers: { date: new Date(Date.now() - 3 * 60 * 1000).toUTCString() }, + }); + mockCache.match.mockResolvedValue(response); + + const cachedResponse = await cacheStrategy.getCachedResponse(request); + + expect(mockCache.match).toHaveBeenCalledWith(request); + expect(mockCache.delete).toHaveBeenCalledWith(request); + expect(cachedResponse).toBeNull(); + }); + + it("should return null if there is no cached response", async () => { + const request = new Request("https://app.appsmith.com", { + method: "GET", + }); + mockCache.match.mockResolvedValue(null); + + const cachedResponse = await cacheStrategy.getCachedResponse(request); + + expect(mockCache.match).toHaveBeenCalledWith(request); + expect(cachedResponse).toBeNull(); + }); + }); + }); +}); diff --git a/app/client/src/ce/utils/serviceWorkerUtils.ts b/app/client/src/ce/utils/serviceWorkerUtils.ts index b33f0101e5b1..dbab00c5a563 100644 --- a/app/client/src/ce/utils/serviceWorkerUtils.ts +++ b/app/client/src/ce/utils/serviceWorkerUtils.ts @@ -3,12 +3,12 @@ import { APP_MODE } from "entities/App"; import type { Match, TokensToRegexpOptions } from "path-to-regexp"; import { match } from "path-to-regexp"; -export const BUILDER_PATH = `/app/:applicationSlug/:pageSlug(.*\-):pageId/edit`; -export const BUILDER_CUSTOM_PATH = `/app/:customSlug(.*\-):pageId/edit`; -export const VIEWER_PATH = `/app/:applicationSlug/:pageSlug(.*\-):pageId`; -export const VIEWER_CUSTOM_PATH = `/app/:customSlug(.*\-):pageId`; -export const BUILDER_PATH_DEPRECATED = `/applications/:applicationId/pages/:pageId/edit`; -export const VIEWER_PATH_DEPRECATED = `/applications/:applicationId/pages/:pageId`; +const BUILDER_PATH = `/app/:applicationSlug/:pageSlug(.*\-):pageId/edit`; +const BUILDER_CUSTOM_PATH = `/app/:customSlug(.*\-):pageId/edit`; +const VIEWER_PATH = `/app/:applicationSlug/:pageSlug(.*\-):pageId`; +const VIEWER_CUSTOM_PATH = `/app/:customSlug(.*\-):pageId`; +const BUILDER_PATH_DEPRECATED = `/applications/:applicationId/pages/:pageId/edit`; +const VIEWER_PATH_DEPRECATED = `/applications/:applicationId/pages/:pageId`; interface TMatchResult { pageId?: string; @@ -226,27 +226,27 @@ export class PrefetchApiCacheStrategy { // Wait for the lock to be released await this.waitForUnlock(request); const prefetchApiCache = await caches.open(this.cacheName); - // Check if the response is already in cache - const cachedResponse = await prefetchApiCache.match(request); + // Fetch the cached response for the request + // if it is a miss, assign null to the cachedResponse + let cachedResponse: Response | null = + (await prefetchApiCache.match(request)) || null; if (cachedResponse) { const dateHeader = cachedResponse.headers.get("date"); const cachedTime = dateHeader ? new Date(dateHeader).getTime() : 0; const currentTime = Date.now(); - + // Check if the cache is valid const isCacheValid = currentTime - cachedTime < this.cacheMaxAge; - if (isCacheValid) { - // Delete the cache as this is a one-time cache - await prefetchApiCache.delete(request); - // Return the cached response - return cachedResponse; + // If the cache is not valid, assign null to the cachedResponse + if (!isCacheValid) { + cachedResponse = null; } - // If the cache is not valid, delete the cache + // Delete the cache as this is a one time read cache await prefetchApiCache.delete(request); } - return null; + return cachedResponse; } } From ddd578e8eade60d08f334797d05aa1ee1d6c37d4 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Sat, 8 Jun 2024 16:44:29 +0530 Subject: [PATCH 11/15] Get consolidated api base url from ConsolidatedPageLoadApi class --- app/client/src/api/ConsolidatedPageLoadApi.tsx | 8 +++++--- app/client/src/ce/utils/serviceWorkerUtils.test.ts | 9 +++++---- app/client/src/ce/utils/serviceWorkerUtils.ts | 5 +++-- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/app/client/src/api/ConsolidatedPageLoadApi.tsx b/app/client/src/api/ConsolidatedPageLoadApi.tsx index ac4902e4058b..473b9a093015 100644 --- a/app/client/src/api/ConsolidatedPageLoadApi.tsx +++ b/app/client/src/api/ConsolidatedPageLoadApi.tsx @@ -5,19 +5,21 @@ import type { ApiResponse } from "api/ApiResponses"; import type { InitConsolidatedApi } from "sagas/InitSagas"; class ConsolidatedPageLoadApi extends Api { - static url = "/v1/consolidated-api"; + static url = "v1/consolidated-api"; + static consolidatedApiViewUrl = `${ConsolidatedPageLoadApi.url}/view`; + static consolidatedApiEditUrl = `${ConsolidatedPageLoadApi.url}/edit`; static async getConsolidatedPageLoadDataView(params: { applicationId?: string; defaultPageId?: string; }): Promise>> { - return Api.get(ConsolidatedPageLoadApi.url + "/view", params); + return Api.get(ConsolidatedPageLoadApi.consolidatedApiViewUrl, params); } static async getConsolidatedPageLoadDataEdit(params: { applicationId?: string; defaultPageId?: string; }): Promise>> { - return Api.get(ConsolidatedPageLoadApi.url + "/edit", params); + return Api.get(ConsolidatedPageLoadApi.consolidatedApiEditUrl, params); } } diff --git a/app/client/src/ce/utils/serviceWorkerUtils.test.ts b/app/client/src/ce/utils/serviceWorkerUtils.test.ts index 743b069e2eb6..5bd03ace0d3a 100644 --- a/app/client/src/ce/utils/serviceWorkerUtils.test.ts +++ b/app/client/src/ce/utils/serviceWorkerUtils.test.ts @@ -11,6 +11,7 @@ import { } from "./serviceWorkerUtils"; import { Mutex } from "async-mutex"; import { Request as NFRequest, Response as NFResponse } from "node-fetch"; +import ConsolidatedPageLoadApi from "api/ConsolidatedPageLoadApi"; (global as any).fetch = jest.fn() as jest.Mock; (global as any).caches = { @@ -323,7 +324,7 @@ describe("serviceWorkerUtils", () => { const request = getConsolidatedApiPrefetchRequest(params); expect(request).toBeInstanceOf(Request); expect(request?.url).toBe( - "https://app.appsmith.com/api/v1/consolidated-api/edit?defaultPageId=page123&applicationId=app123", + `https://app.appsmith.com/api/${ConsolidatedPageLoadApi.consolidatedApiEditUrl}?defaultPageId=page123&applicationId=app123`, ); expect(request?.method).toBe("GET"); expect(request?.headers.get("Branchname")).toBe("main"); @@ -341,7 +342,7 @@ describe("serviceWorkerUtils", () => { const request = getConsolidatedApiPrefetchRequest(params); expect(request).toBeInstanceOf(Request); expect(request?.url).toBe( - "https://app.appsmith.com/api/v1/consolidated-api/view?defaultPageId=page123&applicationId=app123", + `https://app.appsmith.com/api/${ConsolidatedPageLoadApi.consolidatedApiViewUrl}?defaultPageId=page123&applicationId=app123`, ); expect(request?.method).toBe("GET"); expect(request?.headers.get("Branchname")).toBe("main"); @@ -358,7 +359,7 @@ describe("serviceWorkerUtils", () => { const request = getConsolidatedApiPrefetchRequest(params); expect(request).toBeInstanceOf(Request); expect(request?.url).toBe( - "https://app.appsmith.com/api/v1/consolidated-api/edit?defaultPageId=page123", + `https://app.appsmith.com/api/${ConsolidatedPageLoadApi.consolidatedApiEditUrl}?defaultPageId=page123`, ); expect(request?.method).toBe("GET"); expect(request?.headers.get("Branchname")).toBe("main"); @@ -375,7 +376,7 @@ describe("serviceWorkerUtils", () => { const request = getConsolidatedApiPrefetchRequest(params); expect(request).toBeInstanceOf(Request); expect(request?.url).toBe( - "https://app.appsmith.com/api/v1/consolidated-api/view?defaultPageId=page123", + `https://app.appsmith.com/api/${ConsolidatedPageLoadApi.consolidatedApiViewUrl}?defaultPageId=page123`, ); expect(request?.method).toBe("GET"); expect(request?.headers.get("Branchname")).toBe("main"); diff --git a/app/client/src/ce/utils/serviceWorkerUtils.ts b/app/client/src/ce/utils/serviceWorkerUtils.ts index dbab00c5a563..2734c4d2bc6d 100644 --- a/app/client/src/ce/utils/serviceWorkerUtils.ts +++ b/app/client/src/ce/utils/serviceWorkerUtils.ts @@ -1,3 +1,4 @@ +import ConsolidatedPageLoadApi from "api/ConsolidatedPageLoadApi"; import { Mutex } from "async-mutex"; import { APP_MODE } from "entities/App"; import type { Match, TokensToRegexpOptions } from "path-to-regexp"; @@ -117,14 +118,14 @@ export const getConsolidatedApiPrefetchRequest = ( // If the URL matches the builder path if (appMode === APP_MODE.EDIT) { - const requestUrl = `${origin}/api/v1/consolidated-api/edit?${searchParams.toString()}`; + const requestUrl = `${origin}/api/${ConsolidatedPageLoadApi.consolidatedApiEditUrl}?${searchParams.toString()}`; const request = new Request(requestUrl, { method: "GET", headers }); return request; } // If the URL matches the viewer path if (appMode === APP_MODE.PUBLISHED) { - const requestUrl = `${origin}/api/v1/consolidated-api/view?${searchParams.toString()}`; + const requestUrl = `${origin}/api/${ConsolidatedPageLoadApi.consolidatedApiViewUrl}?${searchParams.toString()}`; const request = new Request(requestUrl, { method: "GET", headers }); return request; } From fda023a348704bdb7f09d12bd1ac164f28700874 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Mon, 10 Jun 2024 14:39:04 +0530 Subject: [PATCH 12/15] PR review comments --- .../src/ce/utils/serviceWorkerUtils.test.ts | 53 ++++++++++--------- app/client/src/ce/utils/serviceWorkerUtils.ts | 19 ++++--- app/client/src/serviceWorker.ts | 30 +++-------- 3 files changed, 49 insertions(+), 53 deletions(-) diff --git a/app/client/src/ce/utils/serviceWorkerUtils.test.ts b/app/client/src/ce/utils/serviceWorkerUtils.test.ts index 5bd03ace0d3a..2dbefc7e4373 100644 --- a/app/client/src/ce/utils/serviceWorkerUtils.test.ts +++ b/app/client/src/ce/utils/serviceWorkerUtils.test.ts @@ -7,7 +7,7 @@ import { getConsolidatedApiPrefetchRequest, getApplicationParamsFromUrl, getPrefetchModuleApiRequests, - PrefetchApiCacheStrategy, + PrefetchApiService, } from "./serviceWorkerUtils"; import { Mutex } from "async-mutex"; import { Request as NFRequest, Response as NFResponse } from "node-fetch"; @@ -408,12 +408,12 @@ describe("serviceWorkerUtils", () => { }); }); - describe("PrefetchApiCacheStrategy", () => { - let cacheStrategy: PrefetchApiCacheStrategy; + describe("PrefetchApiService", () => { + let prefetchApiService: PrefetchApiService; let mockCache: any; beforeEach(() => { - cacheStrategy = new PrefetchApiCacheStrategy(); + prefetchApiService = new PrefetchApiService(); mockCache = { put: jest.fn(), match: jest.fn(), @@ -434,7 +434,7 @@ describe("serviceWorkerUtils", () => { method: "GET", }); request.headers.append("branchname", "main"); - const key = cacheStrategy.getRequestKey(request); + const key = prefetchApiService.getRequestKey(request); expect(key).toBe("GET:https://app.appsmith.com/:branchname:main"); }); }); @@ -445,7 +445,7 @@ describe("serviceWorkerUtils", () => { method: "GET", }); const acquireSpy = jest.spyOn(Mutex.prototype, "acquire"); - await cacheStrategy.aqcuireFetchMutex(request); + await prefetchApiService.aqcuireFetchMutex(request); expect(acquireSpy).toHaveBeenCalled(); }); @@ -454,12 +454,12 @@ describe("serviceWorkerUtils", () => { method: "GET", }); const mutex = new Mutex(); - cacheStrategy.prefetchFetchMutexMap.set( - cacheStrategy.getRequestKey(request), + prefetchApiService.prefetchFetchMutexMap.set( + prefetchApiService.getRequestKey(request), mutex, ); const acquireSpy = jest.spyOn(mutex, "acquire"); - await cacheStrategy.aqcuireFetchMutex(request); + await prefetchApiService.aqcuireFetchMutex(request); expect(acquireSpy).toHaveBeenCalled(); }); }); @@ -470,12 +470,12 @@ describe("serviceWorkerUtils", () => { method: "GET", }); const mutex = new Mutex(); - cacheStrategy.prefetchFetchMutexMap.set( - cacheStrategy.getRequestKey(request), + prefetchApiService.prefetchFetchMutexMap.set( + prefetchApiService.getRequestKey(request), mutex, ); const waitForUnlockSpy = jest.spyOn(mutex, "waitForUnlock"); - await cacheStrategy.waitForUnlock(request); + await prefetchApiService.waitForUnlock(request); expect(waitForUnlockSpy).toHaveBeenCalled(); }); @@ -484,7 +484,7 @@ describe("serviceWorkerUtils", () => { method: "GET", }); await expect( - cacheStrategy.waitForUnlock(request), + prefetchApiService.waitForUnlock(request), ).resolves.not.toThrow(); }); }); @@ -495,12 +495,12 @@ describe("serviceWorkerUtils", () => { method: "GET", }); const mutex = new Mutex(); - cacheStrategy.prefetchFetchMutexMap.set( - cacheStrategy.getRequestKey(request), + prefetchApiService.prefetchFetchMutexMap.set( + prefetchApiService.getRequestKey(request), mutex, ); const releaseSpy = jest.spyOn(mutex, "release"); - cacheStrategy.releaseFetchMutex(request); + prefetchApiService.releaseFetchMutex(request); expect(releaseSpy).toHaveBeenCalled(); }); @@ -508,7 +508,9 @@ describe("serviceWorkerUtils", () => { const request = new Request("https://app.appsmith.com", { method: "GET", }); - expect(() => cacheStrategy.releaseFetchMutex(request)).not.toThrow(); + expect(() => + prefetchApiService.releaseFetchMutex(request), + ).not.toThrow(); }); }); @@ -527,7 +529,7 @@ describe("serviceWorkerUtils", () => { const acquireSpy = jest.spyOn(Mutex.prototype, "acquire"); const releaseSpy = jest.spyOn(Mutex.prototype, "release"); - await cacheStrategy.cacheApi(request); + await prefetchApiService.cacheApi(request); expect(acquireSpy).toHaveBeenCalled(); expect((global as any).fetch).toHaveBeenCalledWith(request); @@ -550,7 +552,7 @@ describe("serviceWorkerUtils", () => { const acquireSpy = jest.spyOn(Mutex.prototype, "acquire"); const releaseSpy = jest.spyOn(Mutex.prototype, "release"); - await cacheStrategy.cacheApi(request); + await prefetchApiService.cacheApi(request); expect(acquireSpy).toHaveBeenCalled(); expect(mockCache.delete).toHaveBeenCalledWith(request); @@ -568,13 +570,14 @@ describe("serviceWorkerUtils", () => { }); mockCache.match.mockResolvedValue(response); const mutex = new Mutex(); - cacheStrategy.prefetchFetchMutexMap.set( - cacheStrategy.getRequestKey(request), + prefetchApiService.prefetchFetchMutexMap.set( + prefetchApiService.getRequestKey(request), mutex, ); const waitForUnlockSpy = jest.spyOn(Mutex.prototype, "waitForUnlock"); - const cachedResponse = await cacheStrategy.getCachedResponse(request); + const cachedResponse = + await prefetchApiService.getCachedResponse(request); expect(waitForUnlockSpy).toHaveBeenCalled(); expect(mockCache.match).toHaveBeenCalledWith(request); @@ -591,7 +594,8 @@ describe("serviceWorkerUtils", () => { }); mockCache.match.mockResolvedValue(response); - const cachedResponse = await cacheStrategy.getCachedResponse(request); + const cachedResponse = + await prefetchApiService.getCachedResponse(request); expect(mockCache.match).toHaveBeenCalledWith(request); expect(mockCache.delete).toHaveBeenCalledWith(request); @@ -604,7 +608,8 @@ describe("serviceWorkerUtils", () => { }); mockCache.match.mockResolvedValue(null); - const cachedResponse = await cacheStrategy.getCachedResponse(request); + const cachedResponse = + await prefetchApiService.getCachedResponse(request); expect(mockCache.match).toHaveBeenCalledWith(request); expect(cachedResponse).toBeNull(); diff --git a/app/client/src/ce/utils/serviceWorkerUtils.ts b/app/client/src/ce/utils/serviceWorkerUtils.ts index 2734c4d2bc6d..2787a9f82a9b 100644 --- a/app/client/src/ce/utils/serviceWorkerUtils.ts +++ b/app/client/src/ce/utils/serviceWorkerUtils.ts @@ -134,19 +134,26 @@ export const getConsolidatedApiPrefetchRequest = ( }; /** - * Function to get the prefetch request for module apis + * Function to get the prefetch requests for an application */ -export const getPrefetchModuleApiRequests = ( - // eslint-disable-next-line @typescript-eslint/no-unused-vars - applicationProps: TApplicationParams, +export const getPrefetchRequests = ( + applicationParams: TApplicationParams, ): Request[] => { - return []; + const prefetchRequests: Request[] = []; + const consolidatedApiPrefetchRequest = + getConsolidatedApiPrefetchRequest(applicationParams); + + if (consolidatedApiPrefetchRequest) { + prefetchRequests.push(consolidatedApiPrefetchRequest); + } + + return prefetchRequests; }; /** * Cache strategy for Appsmith API */ -export class PrefetchApiCacheStrategy { +export class PrefetchApiService { cacheName = "prefetch-cache-v1"; cacheMaxAge = 2 * 60 * 1000; // 2 minutes in milliseconds // Mutex to lock the fetch and cache operation diff --git a/app/client/src/serviceWorker.ts b/app/client/src/serviceWorker.ts index 4b43d91b38b2..419cf3fa4266 100644 --- a/app/client/src/serviceWorker.ts +++ b/app/client/src/serviceWorker.ts @@ -9,9 +9,8 @@ import { import { cachedApiUrlRegex, getApplicationParamsFromUrl, - getConsolidatedApiPrefetchRequest, - getPrefetchModuleApiRequests, - PrefetchApiCacheStrategy, + getPrefetchRequests, + PrefetchApiService, } from "@appsmith/utils/serviceWorkerUtils"; import type { RouteHandlerCallback } from "workbox-core/types"; @@ -43,7 +42,7 @@ self.__WB_DISABLE_DEV_LOGS = false; skipWaiting(); clientsClaim(); -const prefetchApiCacheStrategy = new PrefetchApiCacheStrategy(); +const prefetchApiService = new PrefetchApiService(); /** * Route handler callback for HTML pages. @@ -59,24 +58,10 @@ const htmlRouteHandlerCallback: RouteHandlerCallback = async ({ // If application params are present, prefetch the API requests for the application if (applicationParams) { - // Prefetch the consolidated API request - const consolidatedApiPrefetchRequest = - getConsolidatedApiPrefetchRequest(applicationParams); - - if (consolidatedApiPrefetchRequest) { - prefetchApiCacheStrategy - .cacheApi(consolidatedApiPrefetchRequest) - .catch(() => { - // Silently fail - }); - } - - // Prefetch the module API requests - const moduleApiPrefetchRequests = - getPrefetchModuleApiRequests(applicationParams); + const prefetchRequests = getPrefetchRequests(applicationParams); - moduleApiPrefetchRequests.forEach((prefetchRequest) => { - prefetchApiCacheStrategy.cacheApi(prefetchRequest).catch(() => { + prefetchRequests.forEach((prefetchRequest) => { + prefetchApiService.cacheApi(prefetchRequest).catch(() => { // Silently fail }); }); @@ -115,8 +100,7 @@ registerRoute( cachedApiUrlRegex, async ({ event, request }) => { // Check for cached response - const cachedResponse = - await prefetchApiCacheStrategy.getCachedResponse(request); + const cachedResponse = await prefetchApiService.getCachedResponse(request); // If the response is cached, return the response if (cachedResponse) { From 5619d0048707c8f385382ff42f5867a743f40f85 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Mon, 10 Jun 2024 14:46:44 +0530 Subject: [PATCH 13/15] Change the comment --- app/client/src/ce/utils/serviceWorkerUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/client/src/ce/utils/serviceWorkerUtils.ts b/app/client/src/ce/utils/serviceWorkerUtils.ts index 2787a9f82a9b..039dad2350b4 100644 --- a/app/client/src/ce/utils/serviceWorkerUtils.ts +++ b/app/client/src/ce/utils/serviceWorkerUtils.ts @@ -151,7 +151,7 @@ export const getPrefetchRequests = ( }; /** - * Cache strategy for Appsmith API + * Service to fetch and cache prefetch requests */ export class PrefetchApiService { cacheName = "prefetch-cache-v1"; From 4b25d9884c95384495ceb3947a4fe4c5a06f991a Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Tue, 11 Jun 2024 09:20:06 +0530 Subject: [PATCH 14/15] Update test cases --- .../src/ce/utils/serviceWorkerUtils.test.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/app/client/src/ce/utils/serviceWorkerUtils.test.ts b/app/client/src/ce/utils/serviceWorkerUtils.test.ts index 2dbefc7e4373..7b721b3093a7 100644 --- a/app/client/src/ce/utils/serviceWorkerUtils.test.ts +++ b/app/client/src/ce/utils/serviceWorkerUtils.test.ts @@ -6,7 +6,7 @@ import { getSearchQuery, getConsolidatedApiPrefetchRequest, getApplicationParamsFromUrl, - getPrefetchModuleApiRequests, + getPrefetchRequests, PrefetchApiService, } from "./serviceWorkerUtils"; import { Mutex } from "async-mutex"; @@ -394,17 +394,23 @@ describe("serviceWorkerUtils", () => { }); }); - describe("getPrefetchModuleApiRequests", () => { - it("should return empty array", () => { + describe("getPrefetchRequests", () => { + it("should return prefetch requests with consolidated api request", () => { const params: TApplicationParams = { origin: "https://app.appsmith.com", branchName: "main", appMode: APP_MODE.EDIT, pageId: "page123", - applicationId: "app123", }; - const requests = getPrefetchModuleApiRequests(params); - expect(requests).toHaveLength(0); + const requests = getPrefetchRequests(params); + expect(requests).toHaveLength(1); + const [consolidatedAPIRequest] = requests; + expect(consolidatedAPIRequest).toBeInstanceOf(Request); + expect(consolidatedAPIRequest?.url).toBe( + `https://app.appsmith.com/api/${ConsolidatedPageLoadApi.consolidatedApiEditUrl}?defaultPageId=page123`, + ); + expect(consolidatedAPIRequest?.method).toBe("GET"); + expect(consolidatedAPIRequest?.headers.get("Branchname")).toBe("main"); }); }); From c23df88f8051f4f60bd341e1c569ebcd7fd365e3 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Wed, 12 Jun 2024 10:22:26 +0530 Subject: [PATCH 15/15] Remove import from consolidated api --- app/client/src/ce/utils/serviceWorkerUtils.test.ts | 11 +++++------ app/client/src/ce/utils/serviceWorkerUtils.ts | 5 ++--- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/app/client/src/ce/utils/serviceWorkerUtils.test.ts b/app/client/src/ce/utils/serviceWorkerUtils.test.ts index 7b721b3093a7..11514036c46d 100644 --- a/app/client/src/ce/utils/serviceWorkerUtils.test.ts +++ b/app/client/src/ce/utils/serviceWorkerUtils.test.ts @@ -11,7 +11,6 @@ import { } from "./serviceWorkerUtils"; import { Mutex } from "async-mutex"; import { Request as NFRequest, Response as NFResponse } from "node-fetch"; -import ConsolidatedPageLoadApi from "api/ConsolidatedPageLoadApi"; (global as any).fetch = jest.fn() as jest.Mock; (global as any).caches = { @@ -324,7 +323,7 @@ describe("serviceWorkerUtils", () => { const request = getConsolidatedApiPrefetchRequest(params); expect(request).toBeInstanceOf(Request); expect(request?.url).toBe( - `https://app.appsmith.com/api/${ConsolidatedPageLoadApi.consolidatedApiEditUrl}?defaultPageId=page123&applicationId=app123`, + `https://app.appsmith.com/api/v1/consolidated-api/edit?defaultPageId=page123&applicationId=app123`, ); expect(request?.method).toBe("GET"); expect(request?.headers.get("Branchname")).toBe("main"); @@ -342,7 +341,7 @@ describe("serviceWorkerUtils", () => { const request = getConsolidatedApiPrefetchRequest(params); expect(request).toBeInstanceOf(Request); expect(request?.url).toBe( - `https://app.appsmith.com/api/${ConsolidatedPageLoadApi.consolidatedApiViewUrl}?defaultPageId=page123&applicationId=app123`, + `https://app.appsmith.com/api/v1/consolidated-api/view?defaultPageId=page123&applicationId=app123`, ); expect(request?.method).toBe("GET"); expect(request?.headers.get("Branchname")).toBe("main"); @@ -359,7 +358,7 @@ describe("serviceWorkerUtils", () => { const request = getConsolidatedApiPrefetchRequest(params); expect(request).toBeInstanceOf(Request); expect(request?.url).toBe( - `https://app.appsmith.com/api/${ConsolidatedPageLoadApi.consolidatedApiEditUrl}?defaultPageId=page123`, + `https://app.appsmith.com/api/v1/consolidated-api/edit?defaultPageId=page123`, ); expect(request?.method).toBe("GET"); expect(request?.headers.get("Branchname")).toBe("main"); @@ -376,7 +375,7 @@ describe("serviceWorkerUtils", () => { const request = getConsolidatedApiPrefetchRequest(params); expect(request).toBeInstanceOf(Request); expect(request?.url).toBe( - `https://app.appsmith.com/api/${ConsolidatedPageLoadApi.consolidatedApiViewUrl}?defaultPageId=page123`, + `https://app.appsmith.com/api/v1/consolidated-api/view?defaultPageId=page123`, ); expect(request?.method).toBe("GET"); expect(request?.headers.get("Branchname")).toBe("main"); @@ -407,7 +406,7 @@ describe("serviceWorkerUtils", () => { const [consolidatedAPIRequest] = requests; expect(consolidatedAPIRequest).toBeInstanceOf(Request); expect(consolidatedAPIRequest?.url).toBe( - `https://app.appsmith.com/api/${ConsolidatedPageLoadApi.consolidatedApiEditUrl}?defaultPageId=page123`, + `https://app.appsmith.com/api/v1/consolidated-api/edit?defaultPageId=page123`, ); expect(consolidatedAPIRequest?.method).toBe("GET"); expect(consolidatedAPIRequest?.headers.get("Branchname")).toBe("main"); diff --git a/app/client/src/ce/utils/serviceWorkerUtils.ts b/app/client/src/ce/utils/serviceWorkerUtils.ts index 039dad2350b4..4fc8ba81541a 100644 --- a/app/client/src/ce/utils/serviceWorkerUtils.ts +++ b/app/client/src/ce/utils/serviceWorkerUtils.ts @@ -1,4 +1,3 @@ -import ConsolidatedPageLoadApi from "api/ConsolidatedPageLoadApi"; import { Mutex } from "async-mutex"; import { APP_MODE } from "entities/App"; import type { Match, TokensToRegexpOptions } from "path-to-regexp"; @@ -118,14 +117,14 @@ export const getConsolidatedApiPrefetchRequest = ( // If the URL matches the builder path if (appMode === APP_MODE.EDIT) { - const requestUrl = `${origin}/api/${ConsolidatedPageLoadApi.consolidatedApiEditUrl}?${searchParams.toString()}`; + const requestUrl = `${origin}/api/${"v1/consolidated-api/edit"}?${searchParams.toString()}`; const request = new Request(requestUrl, { method: "GET", headers }); return request; } // If the URL matches the viewer path if (appMode === APP_MODE.PUBLISHED) { - const requestUrl = `${origin}/api/${ConsolidatedPageLoadApi.consolidatedApiViewUrl}?${searchParams.toString()}`; + const requestUrl = `${origin}/api/v1/consolidated-api/view?${searchParams.toString()}`; const request = new Request(requestUrl, { method: "GET", headers }); return request; }