Redirects GraphQL API performance improvements (#2632)

Large number of redirects can cause issues in real projects. Help on improving the situation by making the API a bit faster.(that alone won't be the solution). - Add fixtures that create 7000 redirects (for this the additional exports are needed) - Add a relatively simple performance optimization in redirects resolver: Call `this.pageTreeReadApi.preloadNodes(scope);` to load the whole page tree into memory, as most redirects will access it (when they are internal) - Local tests showed (when loading 100 redirects) ~250ms (before) vs ~100ms (after) (!) - (Demo only) Add an index on `Redirect.scope_domain` - No difference in Demo performance, but much needed in "real" multidomain applications - Unfortunately this won't be generated by MikroORM, as the redirect entity is in library only - And increase the maximum limit for paginated redirects query from 100 to 1000 - Not much improvement, but still ~4.8s (before) vs ~3.2s (after) (both with preloadNodes) --------- Co-authored-by: Johannes Obermair <[email protected]>
vivid-planet · Oct 17, 2024 · cc2a117 · cc2a117
1 parent 8f23414
commit cc2a117
Show file tree

Hide file tree

Showing 11 changed files with 110 additions and 10 deletions.
diff --git a/.changeset/orange-apples-flash.md b/.changeset/orange-apples-flash.md
@@ -0,0 +1,7 @@
+---
+"@comet/cms-api": patch
+---
+
+Redirects: Improve GraphQL API performance by preloading the page tree to speed up target page lookup
+
+Also, increase the maximum limit from 100 to 1000.
diff --git a/demo/api/src/db/fixtures/fixtures.console.ts b/demo/api/src/db/fixtures/fixtures.console.ts
@@ -20,6 +20,7 @@ import slugify from "slugify";
 import { FileUploadsFixtureService } from "./generators/file-uploads-fixture.service";
 import { generateLinks } from "./generators/links.generator";
 import { ManyImagesTestPageFixtureService } from "./generators/many-images-test-page-fixture.service";
+import { RedirectsFixtureService } from "./generators/redirects-fixture.service";
 
 export interface PageTreeNodesFixtures {
     home?: PageTreeNodeInterface;
@@ -49,6 +50,7 @@ export class FixturesConsole {
         @InjectRepository(Link) private readonly linksRepository: EntityRepository<Link>,
         private readonly manyImagesTestPageFixtureService: ManyImagesTestPageFixtureService,
         private readonly fileUploadsFixtureService: FileUploadsFixtureService,
+        private readonly redirectsFixtureService: RedirectsFixtureService,
         private readonly dependenciesService: DependenciesService,
     ) {}
 
@@ -110,6 +112,7 @@ export class FixturesConsole {
 
         node = await this.pageTreeService.createNode(
             {
+                id: "aaa585d3-eca1-47c9-8852-9370817b49ac",
                 name: "Sub",
                 slug: "sub",
                 parentId: node.id,
@@ -278,6 +281,8 @@ export class FixturesConsole {
 
         await this.fileUploadsFixtureService.generateFileUploads();
 
+        await this.redirectsFixtureService.generateRedirects();
+
         await this.dependenciesService.createViews();
 
         await this.orm.em.flush();

diff --git a/demo/api/src/db/fixtures/fixtures.module.ts b/demo/api/src/db/fixtures/fixtures.module.ts
@@ -9,10 +9,18 @@ import { ConsoleModule } from "nestjs-console";
 import { FileUploadsFixtureService } from "./generators/file-uploads-fixture.service";
 import { ImageFileFixtureService } from "./generators/image-file-fixture.service";
 import { ManyImagesTestPageFixtureService } from "./generators/many-images-test-page-fixture.service";
+import { RedirectsFixtureService } from "./generators/redirects-fixture.service";
 import { SvgImageFileFixtureService } from "./generators/svg-image-file-fixture.service";
 
 @Module({
     imports: [ConfigModule, ConsoleModule, PagesModule, LinksModule, DependenciesModule],
-    providers: [FixturesConsole, ManyImagesTestPageFixtureService, ImageFileFixtureService, SvgImageFileFixtureService, FileUploadsFixtureService],
+    providers: [
+        FixturesConsole,
+        ManyImagesTestPageFixtureService,
+        ImageFileFixtureService,
+        SvgImageFileFixtureService,
+        FileUploadsFixtureService,
+        RedirectsFixtureService,
+    ],
 })
 export class FixturesModule {}
diff --git a/demo/api/src/db/fixtures/generators/redirects-fixture.service.ts b/demo/api/src/db/fixtures/generators/redirects-fixture.service.ts
@@ -0,0 +1,43 @@
+import { RedirectGenerationType, RedirectInterface, REDIRECTS_LINK_BLOCK, RedirectsLinkBlock, RedirectSourceTypeValues } from "@comet/cms-api";
+import { InjectRepository } from "@mikro-orm/nestjs";
+import { EntityRepository } from "@mikro-orm/postgresql";
+import { Inject, Injectable } from "@nestjs/common";
+
+@Injectable()
+export class RedirectsFixtureService {
+    constructor(
+        @Inject(REDIRECTS_LINK_BLOCK) private readonly redirectsLinkBlock: RedirectsLinkBlock,
+        @InjectRepository("Redirect") private readonly repository: EntityRepository<RedirectInterface>,
+    ) {}
+
+    async generateRedirects(): Promise<void> {
+        console.log("Generating redirects...");
+
+        for (let i = 0; i < 7000; i++) {
+            this.repository.create({
+                generationType: RedirectGenerationType.manual,
+                source: `/redirect-${i}`,
+                target: this.redirectsLinkBlock
+                    .blockInputFactory({
+                        attachedBlocks: [
+                            {
+                                type: "internal",
+                                props: {
+                                    targetPageId: "aaa585d3-eca1-47c9-8852-9370817b49ac",
+                                },
+                            },
+                        ],
+                        activeType: "internal",
+                    })
+                    .transformToBlockData(),
+                active: true,
+                scope: {
+                    domain: "main",
+                    language: "en",
+                },
+                sourceType: RedirectSourceTypeValues.path,
+            });
+        }
+        await this.repository.getEntityManager().flush();
+    }
+}
diff --git a/demo/api/src/db/migrations/Migration20241015162102.ts b/demo/api/src/db/migrations/Migration20241015162102.ts
@@ -0,0 +1,12 @@
+import { Migration } from '@mikro-orm/migrations';
+
+export class Migration20241015162102 extends Migration {
+
+  async up(): Promise<void> {
+    this.addSql('CREATE INDEX "Redirect_scope_domain" on "Redirect" ("scope_domain")');
+  }
+
+  async down(): Promise<void> {
+    this.addSql('DROP INDEX "Redirect_scope_domain"');
+  }
+}
diff --git a/demo/api/src/redirects/dto/redirect-scope.ts b/demo/api/src/redirects/dto/redirect-scope.ts
@@ -1,4 +1,4 @@
-import { Embeddable, Property } from "@mikro-orm/core";
+import { Embeddable, Index, Property } from "@mikro-orm/core";
 import { Field, InputType, ObjectType } from "@nestjs/graphql";
 import { IsString } from "class-validator";
 
@@ -7,6 +7,7 @@ import { IsString } from "class-validator";
 @InputType("RedirectScopeInput") // name must not be changed in the app
 // @TODO: disguise @ObjectType("RedirectScope") and @InputType("RedirectScopeInput") decorators under a custom decorator: f.i. @RedirectScope
 export class RedirectScope {
+    @Index() // this does nothing, migration has to be created manually as the entity is in library
     @Property({ columnType: "text" })
     @Field()
     @IsString()

diff --git a/demo/site/src/middleware/redirects.ts b/demo/site/src/middleware/redirects.ts
@@ -36,7 +36,7 @@ const createInternalRedirects = async (): Promise<Map<string, Redirect>> => {
 
 async function* fetchApiRedirects(scope: GQLRedirectScope) {
     let offset = 0;
-    const limit = 100;
+    const limit = 1000;
 
     while (true) {
         const { paginatedRedirects } = await graphQLFetch<GQLRedirectsQuery, GQLRedirectsQueryVariables>(redirectsQuery, {
@@ -64,6 +64,9 @@ const createApiRedirects = async (scope: GQLRedirectScope): Promise<Map<string,
         return value.replace(/[:?]/g, "\\$&");
     }
 
+    // eslint-disable-next-line no-console
+    console.time("createApiRedirects");
+
     for await (const redirect of fetchApiRedirects(scope)) {
         let source: string | undefined;
         let destination: string | undefined;
@@ -110,6 +113,8 @@ const createApiRedirects = async (scope: GQLRedirectScope): Promise<Map<string,
             redirects.set(source, { destination, permanent: true });
         }
     }
+    // eslint-disable-next-line no-console
+    console.timeEnd("createApiRedirects");
     return redirects;
 };
 

diff --git a/packages/api/cms-api/src/index.ts b/packages/api/cms-api/src/index.ts
@@ -156,8 +156,10 @@ export { PageTreeNodeDocumentEntityScopeService } from "./page-tree/page-tree-no
 export { PageTreeReadApiService } from "./page-tree/page-tree-read-api.service";
 export { PageTreeNodeCategory, PageTreeNodeInterface, PageTreeNodeVisibility, ScopeInterface } from "./page-tree/types";
 export { PageExists, PageExistsConstraint } from "./page-tree/validators/page-exists.validator";
+export { RedirectInterface } from "./redirects/entities/redirect-entity.factory";
+export { REDIRECTS_LINK_BLOCK } from "./redirects/redirects.constants";
 export { RedirectGenerationType, RedirectSourceTypeValues } from "./redirects/redirects.enum";
-export { RedirectsModule } from "./redirects/redirects.module";
+export { RedirectsLinkBlock, RedirectsModule } from "./redirects/redirects.module";
 export { createRedirectsResolver } from "./redirects/redirects.resolver";
 export { RedirectsService } from "./redirects/redirects.service";
 export { IsValidRedirectSource, IsValidRedirectSourceConstraint } from "./redirects/validators/isValidRedirectSource";

diff --git a/packages/api/cms-api/src/redirects/dto/paginated-redirects-args.factory.ts b/packages/api/cms-api/src/redirects/dto/paginated-redirects-args.factory.ts
@@ -1,9 +1,8 @@
 import { Type } from "@nestjs/common";
-import { ArgsType, Field } from "@nestjs/graphql";
+import { ArgsType, Field, Int } from "@nestjs/graphql";
 import { Type as TransformerType } from "class-transformer";
-import { IsOptional, IsString, ValidateNested } from "class-validator";
+import { IsInt, IsOptional, IsString, Max, Min, ValidateNested } from "class-validator";
 
-import { OffsetBasedPaginationArgs } from "../../common/pagination/offset-based.args";
 import { RedirectScopeInterface } from "../types";
 import { EmptyRedirectScope } from "./empty-redirect-scope";
 import { RedirectSort } from "./redirect.sort";
@@ -21,7 +20,7 @@ export interface PaginatedRedirectsArgsInterface {
 export class PaginatedRedirectsArgsFactory {
     static create({ Scope }: { Scope: Type<RedirectScopeInterface> }): Type<PaginatedRedirectsArgsInterface> {
         @ArgsType()
-        class PaginatedRedirectsArgs extends OffsetBasedPaginationArgs implements PaginatedRedirectsArgsInterface {
+        class PaginatedRedirectsArgs implements PaginatedRedirectsArgsInterface {
             @Field(() => Scope, { defaultValue: Scope === EmptyRedirectScope ? {} : undefined })
             @TransformerType(() => Scope)
             @ValidateNested()
@@ -41,6 +40,16 @@ export class PaginatedRedirectsArgsFactory {
             @ValidateNested({ each: true })
             @TransformerType(() => RedirectSort)
             sort?: RedirectSort[];
+
+            @Field(() => Int, { defaultValue: 0 })
+            @IsInt()
+            @Min(0)
+            offset: number;
+
+            @Field(() => Int, { defaultValue: 25 })
+            @Min(1)
+            @Max(1000)
+            limit: number;
         }
         return PaginatedRedirectsArgs;
     }

diff --git a/packages/api/cms-api/src/redirects/redirects.module.ts b/packages/api/cms-api/src/redirects/redirects.module.ts
@@ -44,11 +44,13 @@ export class RedirectsModule {
             useValue: linkBlock,
         };
 
+        const mikroOrmModule = MikroOrmModule.forFeature([Redirect]);
+
         return {
             module: RedirectsModule,
-            imports: [MikroOrmModule.forFeature([Redirect])],
+            imports: [mikroOrmModule],
             providers: [RedirectsResolver, RedirectsDependenciesResolver, RedirectsService, linkBlockProvider, ImportRedirectsConsole],
-            exports: [RedirectsService],
+            exports: [RedirectsService, REDIRECTS_LINK_BLOCK, mikroOrmModule],
         };
     }
 }
diff --git a/packages/api/cms-api/src/redirects/redirects.resolver.ts b/packages/api/cms-api/src/redirects/redirects.resolver.ts
@@ -8,6 +8,7 @@ import { CometValidationException } from "../common/errors/validation.exception"
 import { PaginatedResponseFactory } from "../common/pagination/paginated-response.factory";
 import { DynamicDtoValidationPipe } from "../common/validation/dynamic-dto-validation.pipe";
 import { validateNotModified } from "../document/validateNotModified";
+import { PageTreeReadApiService } from "../page-tree/page-tree-read-api.service";
 import { AffectedEntity } from "../user-permissions/decorators/affected-entity.decorator";
 import { RequiredPermission } from "../user-permissions/decorators/required-permission.decorator";
 import { EmptyRedirectScope } from "./dto/empty-redirect-scope";
@@ -55,6 +56,7 @@ export function createRedirectsResolver({
         constructor(
             private readonly redirectService: RedirectsService,
             @InjectRepository("Redirect") private readonly repository: EntityRepository<RedirectInterface>,
+            private readonly pageTreeReadApi: PageTreeReadApiService,
         ) {}
 
         @Query(() => [Redirect], { deprecationReason: "Use paginatedRedirects instead. Will be removed in the next version." })
@@ -70,6 +72,8 @@ export function createRedirectsResolver({
                 options.orderBy = { [sortColumnName]: sortDirection };
             }
 
+            await this.pageTreeReadApi.preloadNodes(scope);
+
             return this.repository.find(where, options);
         }
 
@@ -91,6 +95,8 @@ export function createRedirectsResolver({
                 });
             }
 
+            await this.pageTreeReadApi.preloadNodes(scope);
+
             const [entities, totalCount] = await this.repository.findAndCount(where, options);
             return new PaginatedRedirects(entities, totalCount);
         }