refs 126901: limit number of trees and nodes to process

apps/app/src/features/external-user-group/server/service/external-user-group-sync.ts

@@ -1,6 +1,7 @@
 import { IUserHasId } from '~/interfaces/user';
 import ExternalAccount from '~/server/models/external-account';
 import { excludeTestIdsFromTargetIds } from '~/server/util/compare-objectId';
+import { batchProcessPromiseAll } from '~/utils/promise';
 
 import { configManager } from '../../../../server/service/config-manager';
 import { externalAccountService } from '../../../../server/service/external-account';
@@ -10,13 +11,18 @@ import {
 import ExternalUserGroup from '../models/external-user-group';
 import ExternalUserGroupRelation from '../models/external-user-group-relation';
 
+// When d = max depth of group trees
+// Max space complexity of syncExternalUserGroups will be:
+// O(MAX_TREES_TO_PROCESS * d * MAX_USERS_TO_PROCESS)
+const MAX_TREES_TO_PROCESS = 10;
+const MAX_USERS_TO_PROCESS = 100;
+
 abstract class ExternalUserGroupSyncService {
 
   groupProviderType: ExternalGroupProviderType; // name of external service that contains user group info (e.g: ldap, keycloak)
 
   authProviderType: string; // auth provider type (e.g: ldap, oidc)
 
-  // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
   constructor(groupProviderType: ExternalGroupProviderType, authProviderType: string) {
     this.groupProviderType = groupProviderType;
     this.authProviderType = authProviderType;
@@ -35,14 +41,16 @@ abstract class ExternalUserGroupSyncService {
     const syncNode = async(node: ExternalUserGroupTreeNode, parentId?: string) => {
       const externalUserGroup = await this.createUpdateExternalUserGroup(node, parentId);
       existingExternalUserGroupIds.push(externalUserGroup._id);
-      await Promise.all(node.childGroupNodes.map((childNode) => {
-        return syncNode(childNode, externalUserGroup._id);
-      }));
+      // Do not use Promise.all, because the number of promises processed can
+      // exponentially grow when group tree is enormous
+      for await (const childNode of node.childGroupNodes) {
+        await syncNode(childNode, externalUserGroup._id);
+      }
     };
 
-    await Promise.all(trees.map((root) => {
+    await batchProcessPromiseAll(trees, MAX_TREES_TO_PROCESS, (root) => {
       return syncNode(root);
-    }));
+    });
 
     const preserveDeletedLdapGroups: boolean = configManager?.getConfig('crowi', `external-user-group:${this.groupProviderType}:preserveDeletedGroups`);
     if (!preserveDeletedLdapGroups) {
@@ -63,23 +71,21 @@ abstract class ExternalUserGroupSyncService {
     const externalUserGroup = await ExternalUserGroup.findAndUpdateOrCreateGroup(
       node.name, node.id, this.groupProviderType, node.description, parentId,
     );
-    await Promise.all(node.userInfos.map((userInfo) => {
-      return (async() => {
-        const user = await this.getMemberUser(userInfo);
-
-        if (user != null) {
-          const userGroups = await ExternalUserGroup.findGroupsWithAncestorsRecursively(externalUserGroup);
-          const userGroupIds = userGroups.map(g => g._id);
-
-          // remove existing relations from list to create
-          const existingRelations = await ExternalUserGroupRelation.find({ relatedGroup: { $in: userGroupIds }, relatedUser: user._id });
-          const existingGroupIds = existingRelations.map(r => r.relatedGroup.toString());
-          const groupIdsToCreateRelation = excludeTestIdsFromTargetIds(userGroupIds, existingGroupIds);
-
-          await ExternalUserGroupRelation.createRelations(groupIdsToCreateRelation, user);
-        }
-      })();
-    }));
+    await batchProcessPromiseAll(node.userInfos, MAX_USERS_TO_PROCESS, async(userInfo) => {
+      const user = await this.getMemberUser(userInfo);
+
+      if (user != null) {
+        const userGroups = await ExternalUserGroup.findGroupsWithAncestorsRecursively(externalUserGroup);
+        const userGroupIds = userGroups.map(g => g._id);
+
+        // remove existing relations from list to create
+        const existingRelations = await ExternalUserGroupRelation.find({ relatedGroup: { $in: userGroupIds }, relatedUser: user._id });
+        const existingGroupIds = existingRelations.map(r => r.relatedGroup.toString());
+        const groupIdsToCreateRelation = excludeTestIdsFromTargetIds(userGroupIds, existingGroupIds);
+
+        await ExternalUserGroupRelation.createRelations(groupIdsToCreateRelation, user);
+      }
+    });
 
     return externalUserGroup;
   }

apps/app/src/features/external-user-group/server/service/ldap-user-group-sync.ts

@@ -1,6 +1,7 @@
 import { configManager } from '~/server/service/config-manager';
 import LdapService, { SearchResultEntry } from '~/server/service/ldap';
 import PassportService from '~/server/service/passport';
+import { batchProcessPromiseAll } from '~/utils/promise';
 
 import {
   ExternalGroupProviderType, ExternalUserGroupTreeNode, ExternalUserInfo, LdapGroupMembershipAttributeType,
@@ -8,6 +9,12 @@ import {
 
 import ExternalUserGroupSyncService from './external-user-group-sync';
 
+// When d = max depth of group trees
+// Max space complexity of generateExternalUserGroupTrees will be:
+// O(MAX_TREES_TO_PROCESS * d * MAX_USERS_TO_PROCESS)
+const MAX_TREES_TO_PROCESS = 10;
+const MAX_USERS_TO_PROCESS = 100;
+
 class LdapUserGroupSyncService extends ExternalUserGroupSyncService {
 
   passportService: PassportService;
@@ -54,17 +61,23 @@ class LdapUserGroupSyncService extends ExternalUserGroupSyncService {
       converted.push(entry.objectName);
 
       const userIds = getUserIdsFromGroupEntry(entry);
-      const userInfos = (await Promise.all(userIds.map((id) => {
+
+      const userInfos = (await batchProcessPromiseAll(userIds, MAX_USERS_TO_PROCESS, (id) => {
         return this.getUserInfo(id);
-      }))).filter((info): info is NonNullable<ExternalUserInfo> => info != null);
+      })).filter((info): info is NonNullable<ExternalUserInfo> => info != null);
       const name = this.ldapService.getStringValFromSearchResultEntry(entry, groupNameAttribute);
       const description = this.ldapService.getStringValFromSearchResultEntry(entry, groupDescriptionAttribute);
       const childGroupDNs = getChildGroupDnsFromGroupEntry(entry);
 
-      const childGroupNodes: ExternalUserGroupTreeNode[] = (await Promise.all(childGroupDNs.map((dn) => {
+      const childGroupNodesWithNull: (ExternalUserGroupTreeNode | null)[] = [];
+      // Do not use Promise.all, because the number of promises processed can
+      // exponentially grow when group tree is enormous
+      for await (const dn of childGroupDNs) {
         const childEntry = groupEntries.find(ge => ge.objectName === dn);
-        return childEntry != null ? convert(childEntry, converted) : null;
-      }))).filter((node): node is NonNullable<ExternalUserGroupTreeNode> => node != null);
+        childGroupNodesWithNull.push(childEntry != null ? await convert(childEntry, converted) : null);
+      }
+      const childGroupNodes: ExternalUserGroupTreeNode[] = childGroupNodesWithNull
+        .filter((node): node is NonNullable<ExternalUserGroupTreeNode> => node != null);
 
       return name != null ? {
         id: entry.objectName,
@@ -85,7 +98,8 @@ class LdapUserGroupSyncService extends ExternalUserGroupSyncService {
       return !allChildGroupDNs.has(entry.objectName);
     });
 
-    return (await Promise.all(rootEntries.map(entry => convert(entry, [])))).filter((node): node is NonNullable<ExternalUserGroupTreeNode> => node != null);
+    return (await batchProcessPromiseAll(rootEntries, MAX_TREES_TO_PROCESS, entry => convert(entry, [])))
+      .filter((node): node is NonNullable<ExternalUserGroupTreeNode> => node != null);
   }
 
   private async getUserInfo(userId: string): Promise<ExternalUserInfo | null> {

apps/app/src/utils/promise.ts

@@ -0,0 +1,28 @@
+/**
+ * Divide arrays into batches, and apply a given function to each batch by using Promise.all
+ * Use when memory consumption can be too large by using simple Promise.all
+ * @param items array to process
+ * @param limit batch size
+ * @param fn function to apply on each item
+ * @returns result of fn applied to each item
+ */
+export const batchProcessPromiseAll = async<I, O>(
+  items: Array<I>,
+  limit: number,
+  fn: (item: I) => Promise<O>,
+): Promise<O[]> => {
+  let results: O[] = [];
+  for (let start = 0; start < items.length; start += limit) {
+    const end = start + limit > items.length ? items.length : start + limit;
+
+    // eslint-disable-next-line no-await-in-loop
+    const slicedResults = await Promise.all(items.slice(start, end).map(fn));
+
+    results = [
+      ...results,
+      ...slicedResults,
+    ];
+  }
+
+  return results;
+};