Improve performance of contentScopesCount via a request-scoped DataLoader

.changeset/user-content-scopes-count-dataloader.md

@@ -0,0 +1,7 @@
+---
+"@comet/cms-api": patch
+---
+
+Improve performance of the `contentScopesCount` field when querying the users list
+
+When resolving `contentScopesCount` for a list of users, the available content scopes were recomputed (and deduplicated) once per row. They are now resolved a single time per request through a request-scoped `DataLoader`, which noticeably speeds up the users list when many content scopes are configured.

packages/api/cms-api/src/user-permissions/user-content-scopes-loader.service.ts

@@ -0,0 +1,21 @@
+import { Injectable, Scope } from "@nestjs/common";
+import DataLoader from "dataloader";
+
+import { ContentScope } from "./interfaces/content-scope.interface";
+import { User } from "./interfaces/user";
+import { UserPermissionsService } from "./user-permissions.service";
+
+@Injectable({ scope: Scope.REQUEST })
+export class UserContentScopesLoaderService {
+    private dataLoader: DataLoader<User, ContentScope[], string>;
+
+    constructor(private readonly userPermissionsService: UserPermissionsService) {
+        this.dataLoader = new DataLoader<User, ContentScope[], string>((users) => this.userPermissionsService.getContentScopesForUsers([...users]), {
+            cacheKeyFn: (user) => user.id,
+        });
+    }
+
+    load(user: User): Promise<ContentScope[]> {
+        return this.dataLoader.load(user);
+    }
+}

packages/api/cms-api/src/user-permissions/user-permissions.module.ts

@@ -10,6 +10,7 @@ import { UserContentScopes } from "./entities/user-content-scopes.entity";
 import { UserPermission } from "./entities/user-permission.entity";
 import { UserResolver } from "./user.resolver";
 import { UserContentScopesResolver } from "./user-content-scopes.resolver";
+import { UserContentScopesLoaderService } from "./user-content-scopes-loader.service";
 import { UserPermissionResolver } from "./user-permission.resolver";
 import { ACCESS_CONTROL_SERVICE, USER_PERMISSIONS_OPTIONS, USER_PERMISSIONS_USER_SERVICE } from "./user-permissions.constants";
 import { UserPermissionsPublicService } from "./user-permissions.public.service";
@@ -32,6 +33,7 @@ import {
         UserResolver,
         UserPermissionResolver,
         UserContentScopesResolver,
+        UserContentScopesLoaderService,
         ContentScopeService,
         {
             provide: APP_GUARD,

packages/api/cms-api/src/user-permissions/user-permissions.service.spec.ts

@@ -0,0 +1,60 @@
+import type { DiscoveryService } from "@golevelup/nestjs-discovery";
+import type { EntityRepository } from "@mikro-orm/postgresql";
+import { describe, expect, it, vi } from "vitest";
+
+import type { UserContentScopes } from "./entities/user-content-scopes.entity";
+import type { UserPermission } from "./entities/user-permission.entity";
+import type { ContentScope } from "./interfaces/content-scope.interface";
+import type { User } from "./interfaces/user";
+import { UserPermissionsService } from "./user-permissions.service";
+import { type AccessControlServiceInterface, UserPermissions, type UserPermissionsOptions } from "./user-permissions.types";
+
+function createService({
+    availableContentScopes,
+    getContentScopesForUser,
+}: {
+    availableContentScopes: ContentScope[];
+    getContentScopesForUser?: AccessControlServiceInterface["getContentScopesForUser"];
+}) {
+    const options: UserPermissionsOptions = { availableContentScopes };
+    const accessControlService: AccessControlServiceInterface = { isAllowed: () => true, getContentScopesForUser };
+    const contentScopeRepository = { findOne: vi.fn().mockResolvedValue(null) } as unknown as EntityRepository<UserContentScopes>;
+    const permissionRepository = {} as EntityRepository<UserPermission>;
+
+    return new UserPermissionsService(options, undefined, accessControlService, permissionRepository, contentScopeRepository, {} as DiscoveryService);
+}
+
+describe("UserPermissionsService", () => {
+    const scopeA: ContentScope = { domain: "main", language: "en" };
+    const scopeB: ContentScope = { domain: "secondary", language: "de" };
+
+    const userA: User = { id: "a", name: "User A", email: "a@example.com" };
+    const userB: User = { id: "b", name: "User B", email: "b@example.com" };
+
+    describe("getContentScopesForUsers", () => {
+        it("computes the available content scopes only once for all users", async () => {
+            const service = createService({
+                availableContentScopes: [scopeA, scopeB],
+                getContentScopesForUser: () => UserPermissions.allContentScopes,
+            });
+            const getAvailableContentScopes = vi.spyOn(service, "getAvailableContentScopes");
+
+            const result = await service.getContentScopesForUsers([userA, userB]);
+
+            expect(getAvailableContentScopes).toHaveBeenCalledTimes(1);
+            expect(result).toEqual([
+                [scopeA, scopeB],
+                [scopeA, scopeB],
+            ]);
+        });
+
+        it("returns each user's content scopes in the order of the passed users", async () => {
+            const service = createService({
+                availableContentScopes: [scopeA, scopeB],
+                getContentScopesForUser: (user) => (user.id === "a" ? [scopeA] : [scopeB]),
+            });
+
+            await expect(service.getContentScopesForUsers([userA, userB])).resolves.toEqual([[scopeA], [scopeB]]);
+        });
+    });
+});

packages/api/cms-api/src/user-permissions/user-permissions.service.ts

@@ -206,8 +206,28 @@ export class UserPermissionsService {
     }
 
     async getContentScopes(user: User, includeContentScopesManual = true): Promise<ContentScope[]> {
-        const contentScopes: ContentScope[] = [];
         const availableContentScopes = (await this.getAvailableContentScopes()).map((cs) => cs.scope);
+        return this.filterContentScopesForUser({ user, availableContentScopes, includeContentScopesManual });
+    }
+
+    // Resolves the content scopes for many users while computing the (shared) available content scopes only once.
+    // Used by the request-scoped UserContentScopesLoaderService so the contentScopesCount field resolver doesn't
+    // recompute the full available-scope list once per returned user.
+    async getContentScopesForUsers(users: User[], includeContentScopesManual = true): Promise<ContentScope[][]> {
+        const availableContentScopes = (await this.getAvailableContentScopes()).map((cs) => cs.scope);
+        return Promise.all(users.map((user) => this.filterContentScopesForUser({ user, availableContentScopes, includeContentScopesManual })));
+    }
+
+    private async filterContentScopesForUser({
+        user,
+        availableContentScopes,
+        includeContentScopesManual,
+    }: {
+        user: User;
+        availableContentScopes: ContentScope[];
+        includeContentScopesManual: boolean;
+    }): Promise<ContentScope[]> {
+        const contentScopes: ContentScope[] = [];
 
         if (this.accessControlService.getContentScopesForUser) {
             const userContentScopes = await this.accessControlService.getContentScopesForUser(user);

packages/api/cms-api/src/user-permissions/user.resolver.ts

@@ -9,6 +9,7 @@ import { CurrentUser } from "./dto/current-user";
 import { FindUsersArgs, PermissionFilter } from "./dto/paginated-user-list";
 import { UserPermissionsUser } from "./dto/user";
 import { User } from "./interfaces/user";
+import { UserContentScopesLoaderService } from "./user-content-scopes-loader.service";
 import { UserPermissionsService } from "./user-permissions.service";
 
 @ObjectType()
@@ -19,7 +20,10 @@ class UserPermissionPaginatedUserList extends PaginatedResponseFactory.create(Us
 export class UserResolver {
     private readonly logger = new Logger(UserResolver.name);
 
-    constructor(private readonly userService: UserPermissionsService) {}
+    constructor(
+        private readonly userService: UserPermissionsService,
+        private readonly userContentScopesLoader: UserContentScopesLoaderService,
+    ) {}
 
     @Query(() => UserPermissionsUser)
     async userPermissionsUserById(@Args("id", { type: () => String }) id: string): Promise<UserPermissionsUser> {
@@ -115,7 +119,7 @@ export class UserResolver {
 
     @ResolveField(() => Int)
     async contentScopesCount(@Parent() user: UserPermissionsUser): Promise<number> {
-        return (await this.userService.getContentScopes(user)).length;
+        return (await this.userContentScopesLoader.load(user)).length;
     }
 
     @ResolveField(() => Boolean)