-
Notifications
You must be signed in to change notification settings - Fork 4.7k
chore: Prefetch module apis in service worker #34003
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
876c606
59e2a3c
200295e
d65d297
5af3a7b
88901d0
631ee97
3b045f0
700da97
787bc5a
ddd578e
fda023a
5619d00
4b25d98
c23df88
785197e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,222 @@ | ||||
| /* eslint-disable no-console */ | ||||
| import { | ||||
| matchBuilderPath, | ||||
| matchViewerPath, | ||||
| } from "@appsmith/constants/routes/appRoutes"; | ||||
| import { Mutex } from "async-mutex"; | ||||
| import { APP_MODE } from "entities/App"; | ||||
|
|
||||
| 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) => { | ||||
| 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<string, Mutex>(); | ||||
|
|
||||
| constructor() {} | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove the unnecessary constructor to simplify the class definition. - constructor() {}Committable suggestion
Suggested change
ToolsBiome
|
||||
|
|
||||
| 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) => { | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we could consider moving mutex functions into a separate file to improve readability.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and have separate jest test cases for the mutex file
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The mutex map is a state specific to this class. The methods |
||||
| 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); | ||||
|
dvj1988 marked this conversation as resolved.
|
||||
| } | ||||
| } | ||||
|
|
||||
| async getCachedResponse(request: Request) { | ||||
| // Wait for the lock to be released | ||||
| await this.waitForUnlock(request); | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. waitForUnlock returns a null value in some cases. Is it needed to handle that scenario ?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if there are no prefetched requests for a certain request, then the |
||||
| 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; | ||||
| } | ||||
| } | ||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +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; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,9 +7,12 @@ import { | |
| StaleWhileRevalidate, | ||
| } from "workbox-strategies"; | ||
| import { | ||
| getPrefetchConsolidatedApiRequest, | ||
| ConsolidatedApiCacheStrategy, | ||
| } from "utils/serviceWorkerUtils"; | ||
| getApplicationParamsFromUrl, | ||
| getConsolidatedApiPrefetchRequest, | ||
| getPrefetchModuleApiRequests, | ||
| PrefetchApiCacheStrategy, | ||
| } from "@appsmith/utils/serviceWorkerUtils"; | ||
|
dvj1988 marked this conversation as resolved.
|
||
| 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 = | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this logic be extracted into a generic functions that returns an array of all prefetch requests and fetches them ? Consolidated apis and prefetch apis. |
||
| getPrefetchModuleApiRequests(applicationParams); | ||
|
|
||
| moduleApiPrefetchRequests.forEach((prefetchRequest) => { | ||
| prefetchApiCacheStrategy | ||
| .cacheConsolidatedApi(prefetchRequest) | ||
| .catch(() => { | ||
| // Silently fail | ||
| }); | ||
| }); | ||
|
dvj1988 marked this conversation as resolved.
|
||
| } | ||
|
|
||
| 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) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: This can be renamed to PrefetchApiCachingService
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or PrefetchApiService