fix(api-headless-cms): models and group loaders (#3643)

webiny · Oct 30, 2023 · 1f23ae3 · 1f23ae3
1 parent cf02851
commit 1f23ae3
Show file tree

Hide file tree

Showing 7 changed files with 98 additions and 55 deletions.
diff --git a/packages/api-headless-cms/__tests__/contentAPI/contentModel.crud.test.ts b/packages/api-headless-cms/__tests__/contentAPI/contentModel.crud.test.ts
@@ -971,6 +971,10 @@ describe("content model test", () => {
 
         const { listContentModelsQuery: listModels } = useGraphQLHandler({
             ...manageHandlerOpts,
+            identity: {
+                ...helpers.identity,
+                id: "identityWithSpecificModelPermissions"
+            },
             permissions: createPermissions({ models: [createdContentModels[0].modelId] })
         });
 

diff --git a/packages/api-headless-cms/src/crud/contentModel.crud.ts b/packages/api-headless-cms/src/crud/contentModel.crud.ts
@@ -104,10 +104,12 @@ export const createModelsCrud = (params: CreateModelsCrudParams): CmsModelContex
         })
     };
 
+    const listModelsCache = new Map<string, Promise<CmsModel[]>>();
     const clearModelsCache = (): void => {
         for (const loader of Object.values(loaders)) {
             loader.clearAll();
         }
+        listModelsCache.clear();
     };
 
     const managers = new Map<string, CmsModelManager>();
@@ -183,30 +185,49 @@ export const createModelsCrud = (params: CreateModelsCrudParams): CmsModelContex
 
     const modelsList = async (): Promise<CmsModel[]> => {
         const databaseModels: CmsModel[] = await loaders.listModels.load("listModels");
-
         const pluginsModels = getModelsAsPlugins();
-
         return databaseModels.concat(pluginsModels);
     };
-
+    /**
+     * The list models cache is a key -> Promise pair so it the listModels() can be called multiple times but executed only once.
+     */
     const listModels = async () => {
-        return context.benchmark.measure("headlessCms.crud.models.listModels", async () => {
-            const models = await modelsList();
-            return filterAsync(models, async model => {
-                const ownsModel = await modelsPermissions.ensure(
-                    { owns: model.createdBy },
-                    { throw: false }
-                );
-
-                if (!ownsModel) {
-                    return false;
-                }
+        /**
+         * Maybe we can cache based on permissions, not the identity id?
+         *
+         * TODO: @adrian please check if possible.
+         */
+        const key = JSON.stringify({
+            tenant: getTenant().id,
+            locale: getLocale().code,
+            identity: context.security.isAuthorizationEnabled() ? getIdentity()?.id : undefined
+        });
+        if (listModelsCache.has(key)) {
+            return listModelsCache.get(key) as Promise<CmsModel[]>;
+        }
+        const cachedModelList = async () => {
+            return context.benchmark.measure("headlessCms.crud.models.listModels", async () => {
+                const models = await modelsList();
+                return filterAsync(models, async model => {
+                    const ownsModel = await modelsPermissions.ensure(
+                        { owns: model.createdBy },
+                        { throw: false }
+                    );
+
+                    if (!ownsModel) {
+                        return false;
+                    }
 
-                return modelsPermissions.canAccessModel({
-                    model
+                    return modelsPermissions.canAccessModel({
+                        model
+                    });
                 });
             });
-        });
+        };
+
+        listModelsCache.set(key, cachedModelList());
+
+        return listModelsCache.get(key) as Promise<CmsModel[]>;
     };
 
     const getModel = async (modelId: string): Promise<CmsModel> => {
@@ -375,7 +396,7 @@ export const createModelsCrud = (params: CreateModelsCrudParams): CmsModelContex
                 model
             });
 
-            loaders.listModels.clearAll();
+            clearModelsCache();
 
             await updateManager(context, model);
 
@@ -507,7 +528,7 @@ export const createModelsCrud = (params: CreateModelsCrudParams): CmsModelContex
 
             await updateManager(context, resultModel);
 
-            loaders.listModels.clearAll();
+            clearModelsCache();
 
             await onModelAfterUpdate.publish({
                 input: {} as CmsModelUpdateInput,
@@ -595,7 +616,7 @@ export const createModelsCrud = (params: CreateModelsCrudParams): CmsModelContex
                 model
             });
 
-            loaders.listModels.clearAll();
+            clearModelsCache();
 
             await updateManager(context, model);
 
@@ -641,6 +662,8 @@ export const createModelsCrud = (params: CreateModelsCrudParams): CmsModelContex
                 );
             }
 
+            clearModelsCache();
+
             await onModelAfterDelete.publish({
                 model
             });

diff --git a/packages/api-headless-cms/src/crud/contentModelGroup.crud.ts b/packages/api-headless-cms/src/crud/contentModelGroup.crud.ts
@@ -4,7 +4,6 @@ import {
     CmsContext,
     CmsGroup,
     CmsGroupContext,
-    CmsGroupListParams,
     HeadlessCmsStorageOperations,
     OnGroupAfterCreateTopicParams,
     OnGroupAfterDeleteTopicParams,
@@ -76,6 +75,7 @@ export const createModelGroupsCrud = (params: CreateModelGroupsCrudParams): CmsG
         })
     };
 
+    const listGroupsCache = new Map<string, Promise<CmsGroup[]>>();
     const clearGroupsCache = (): void => {
         for (const loader of Object.values(dataLoaders)) {
             loader.clearAll();
@@ -124,15 +124,12 @@ export const createModelGroupsCrud = (params: CreateModelGroupsCrudParams): CmsG
         return group;
     };
 
-    const listGroupsViaDataLoader = async (params: CmsGroupListParams) => {
-        const { where } = params || {};
-
+    const listGroupsViaDataLoader = async () => {
         try {
             return await dataLoaders.listGroups.load("listGroups");
         } catch (ex) {
             throw new WebinyError(ex.message, ex.code || "LIST_ERROR", {
-                ...(ex.data || {}),
-                where
+                ...(ex.data || {})
             });
         }
     };
@@ -191,37 +188,52 @@ export const createModelGroupsCrud = (params: CreateModelGroupsCrudParams): CmsG
 
         return group;
     };
+
     const listGroups: CmsGroupContext["listGroups"] = async params => {
         const { where } = params || {};
 
         const { tenant, locale } = where || {};
 
         await modelGroupsPermissions.ensure({ rwd: "r" });
 
-        const response = await listGroupsViaDataLoader({
-            ...(params || {}),
-            where: {
-                ...(where || {}),
-                tenant: tenant || getTenant().id,
-                locale: locale || getLocale().code
-            }
+        /**
+         * Maybe we can cache based on permissions, not the identity id?
+         *
+         * TODO: @adrian please check if possible.
+         */
+        const key = JSON.stringify({
+            tenant: tenant || getTenant().id,
+            locale: locale || getLocale().code,
+            identity: context.security.isAuthorizationEnabled() ? getIdentity()?.id : undefined
         });
+        if (listGroupsCache.has(key)) {
+            return listGroupsCache.get(key) as Promise<CmsGroup[]>;
+        }
 
-        return filterAsync(response, async group => {
-            const ownsGroup = await modelGroupsPermissions.ensure(
-                { owns: group.createdBy },
-                { throw: false }
-            );
+        const cached = async () => {
+            const response = await listGroupsViaDataLoader();
 
-            if (!ownsGroup) {
-                return false;
-            }
+            return filterAsync(response, async group => {
+                const ownsGroup = await modelGroupsPermissions.ensure(
+                    { owns: group.createdBy },
+                    { throw: false }
+                );
 
-            return await modelGroupsPermissions.canAccessGroup({
-                group
+                if (!ownsGroup) {
+                    return false;
+                }
+
+                return await modelGroupsPermissions.canAccessGroup({
+                    group
+                });
             });
-        });
+        };
+
+        listGroupsCache.set(key, cached());
+
+        return listGroupsCache.get(key) as Promise<CmsGroup[]>;
     };
+
     const createGroup: CmsGroupContext["createGroup"] = async input => {
         await modelGroupsPermissions.ensure({ rwd: "w" });
 

diff --git a/packages/api-headless-cms/src/types.ts b/packages/api-headless-cms/src/types.ts
@@ -1120,7 +1120,6 @@ export interface CmsGroupListParams {
     where: {
         tenant: string;
         locale: string;
-        [key: string]: any;
     };
 }
 

diff --git a/packages/api-headless-cms/src/utils/filterAsync.ts b/packages/api-headless-cms/src/utils/filterAsync.ts
@@ -2,15 +2,15 @@ export const filterAsync = async <T = Record<string, any>>(
     items: T[],
     predicate: (param: T) => Promise<boolean>
 ): Promise<T[]> => {
-    const filteredItems = [];
+    const filteredItems = await Promise.all(
+        items.map(async item => {
+            const valid = await predicate(item);
+            if (!valid) {
+                return null;
+            }
+            return item;
+        })
+    );
 
-    for (let i = 0; i < items.length; i++) {
-        const item = items[i];
-        const valid = await predicate(item);
-        if (valid) {
-            filteredItems.push(item);
-        }
-    }
-
-    return filteredItems;
+    return filteredItems.filter(Boolean) as T[];
 };
diff --git a/packages/api-security/src/createSecurity.ts b/packages/api-security/src/createSecurity.ts
@@ -86,6 +86,9 @@ export const createSecurity = async (config: SecurityConfig): Promise<Security>
             authentication.setIdentity(identity);
             this.onIdentity.publish({ identity });
         },
+        isAuthorizationEnabled: () => {
+            return performAuthorization;
+        },
         async withoutAuthorization<T = any>(cb: () => Promise<T>): Promise<T> {
             const isAuthorizationEnabled = performAuthorization;
             performAuthorization = false;

diff --git a/packages/api-security/src/types.ts b/packages/api-security/src/types.ts
@@ -86,6 +86,8 @@ export interface Security<TIdentity = SecurityIdentity> extends Authentication<T
 
     getStorageOperations(): SecurityStorageOperations;
 
+    isAuthorizationEnabled(): boolean;
+
     withoutAuthorization<T = any>(cb: () => Promise<T>): Promise<T>;
 
     /**