fix: optimize builds service

Author: Przybylski Krzysztof

src/builds/builds.controller.spec.ts

@@ -78,7 +78,7 @@ describe('Builds Controller', () => {
     const result = await controller.create(createBuildDto);
 
     expect(result).toStrictEqual(new BuildDto(buildWithNumber));
-    expect(deleteOldBuilds).toHaveBeenCalledWith(project.id, project.maxBuildAllowed);
+    expect(deleteOldBuilds).toHaveBeenCalledWith(project);
   });
 
   it('should reuse build', async () => {

src/builds/builds.service.spec.ts

@@ -9,6 +9,7 @@ import { BuildDto } from './dto/build.dto';
 import { ProjectsService } from '../projects/projects.service';
 import { generateTestRun } from '../_data_';
 import { PrismaClientKnownRequestError } from '@prisma/client/runtime/library';
+import { StaticService } from '../static/static.service';
 
 jest.mock('./dto/build.dto');
 
@@ -23,11 +24,15 @@ const initService = async ({
   testRunDeleteMock = jest.fn(),
   testRunApproveMock = jest.fn(),
   testRunFindManyMock = jest.fn(),
+  testRunDeleteManyMock = jest.fn(),
   eventsBuildUpdatedMock = jest.fn(),
   eventsBuildCreatedMock = jest.fn(),
   eventBuildDeletedMock = jest.fn(),
   projectFindOneMock = jest.fn(),
   projectUpdateMock = jest.fn(),
+  getImageMock = jest.fn(),
+  saveImageMock = jest.fn(),
+  deleteImageMock = jest.fn(),
 }) => {
   const module: TestingModule = await Test.createTestingModule({
     providers: [
@@ -47,6 +52,10 @@ const initService = async ({
             upsert: buildUpsertMock,
             count: buildCountMock,
           },
+          testRun: {
+            findMany: testRunFindManyMock.mockResolvedValue([]),
+            deleteMany: testRunDeleteManyMock,
+          },
         },
       },
       {
@@ -71,6 +80,14 @@ const initService = async ({
           findOne: projectFindOneMock,
         },
       },
+      {
+        provide: StaticService,
+        useValue: {
+          getImage: getImageMock,
+          saveImage: saveImageMock,
+          deleteImage: deleteImageMock,
+        },
+      },
     ],
   }).compile();
 
@@ -142,6 +159,9 @@ describe('BuildsService', () => {
       skip: 20,
       orderBy: { createdAt: 'desc' },
       where: { projectId },
+      include: {
+        testRuns: true,
+      },
     });
     expect(result).toEqual({
       data: [buildDto],
@@ -155,12 +175,12 @@ describe('BuildsService', () => {
     const buildFindUniqueMock = jest.fn().mockResolvedValueOnce(build);
     const buildFindManyMock = jest.fn().mockImplementation(() => Promise.resolve(build));
     const buildDeleteMock = jest.fn().mockImplementation(() => Promise.resolve(build));
-    const testRunDeleteMock = jest.fn();
+    const testRunDeleteManyMock = jest.fn();
     const eventBuildDeletedMock = jest.fn();
     service = await initService({
       buildFindUniqueMock,
       buildDeleteMock,
-      testRunDeleteMock,
+      testRunDeleteManyMock,
       eventBuildDeletedMock,
       buildFindManyMock,
     });
@@ -173,7 +193,7 @@ describe('BuildsService', () => {
         testRuns: true,
       },
     });
-    expect(testRunDeleteMock).toHaveBeenCalledWith(build.testRuns[0].id);
+    expect(testRunDeleteManyMock).toHaveBeenCalledWith({ where: { buildId: { in: [build.id] } } });
     expect(eventBuildDeletedMock).toHaveBeenCalledWith(new BuildDto(build));
     expect(buildDeleteMock).toHaveBeenCalledWith({
       where: { id: build.id },

src/builds/builds.service.ts

@@ -21,16 +21,13 @@ export class BuildsService {
   ) {}
 
   async findOne(id: string): Promise<BuildDto> {
-    const [build, testRuns] = await Promise.all([
-      this.prismaService.build.findUnique({
-        where: { id },
-      }),
-      this.testRunsService.findMany(id),
-    ]);
-    return new BuildDto({
-      ...build,
-      testRuns,
+    const build = await this.prismaService.build.findUnique({
+      where: { id },
+      include: {
+        testRuns: true,
+      },
     });
+    return new BuildDto(build);
   }
 
   async findMany(projectId: string, take: number, skip: number): Promise<PaginatedBuildDto> {
@@ -41,11 +38,12 @@ export class BuildsService {
         take,
         skip,
         orderBy: { createdAt: 'desc' },
+        include: { testRuns: true },
       }),
     ]);
 
     return {
-      data: await Promise.all(buildList.map((build) => this.findOne(build.id))),
+      data: buildList.map((build) => new BuildDto(build)),
       total,
       take,
       skip,
@@ -76,7 +74,7 @@ export class BuildsService {
       return;
     }
 
-    await Promise.all(build.testRuns.map((testRun) => this.testRunsService.delete(testRun.id)));
+    await this.deleteBulkTestRuns([build.id]);
 
     try {
       await this.prismaService.build.delete({ where: { id } });
@@ -95,19 +93,7 @@ export class BuildsService {
     return build;
   }
 
-  async deleteOldBuilds(project: Project) {
-    this.logger.log('Going to delete old builds');
-
-    const keepBuilds = project.maxBuildAllowed <= 1 ? 1 : project.maxBuildAllowed - 1;
-
-    const buildsToDelete = await this.prismaService.build.findMany({
-      where: { projectId: { equals: project.id } },
-      orderBy: { createdAt: 'desc' },
-      skip: keepBuilds,
-    });
-
-    const buildIds = buildsToDelete.map((build) => build.id);
-
+  async deleteBulkTestRuns(buildIds: string[]) {
     const testRunsToDelete = await this.prismaService.testRun.findMany({ where: { buildId: { in: buildIds } } });
 
     await this.prismaService.testRun.deleteMany({ where: { buildId: { in: buildIds } } });
@@ -121,6 +107,22 @@ export class BuildsService {
       this.logger.log(`TestRun deleted ${testRun.id}`);
       this.eventsGateway.testRunDeleted(testRun);
     });
+  }
+
+  async deleteOldBuilds(project: Project) {
+    this.logger.log('Going to delete old builds');
+
+    const keepBuilds = project.maxBuildAllowed <= 1 ? 1 : project.maxBuildAllowed - 1;
+
+    const buildsToDelete = await this.prismaService.build.findMany({
+      where: { projectId: { equals: project.id } },
+      orderBy: { createdAt: 'desc' },
+      skip: keepBuilds,
+    });
+
+    const buildIds = buildsToDelete.map((build) => build.id);
+
+    await this.deleteBulkTestRuns(buildIds);
 
     await this.prismaService.build.deleteMany({ where: { id: { in: buildIds } } });
 

test/builds.e2e-spec.ts

@@ -12,6 +12,7 @@ import { TestRunsService } from '../src/test-runs/test-runs.service';
 import { BuildsController } from '../src/builds/builds.controller';
 import { TEST_PROJECT } from '../src/_data_';
 import uuidAPIKey from 'uuid-apikey';
+import { readFileSync } from 'fs';
 
 describe('Builds (e2e)', () => {
   let app: INestApplication;
@@ -65,8 +66,20 @@ describe('Builds (e2e)', () => {
           branchName: `branchName-${count}`,
           project: project.id,
         };
-        await requestWithApiKey(app, 'post', '/builds', user.apiKey).send(createBuildDto).expect(201);
-        await haveTestRunCreated(buildsService, testRunsService, project.id, `branchName-${count}`, image_v1);
+
+        const buildId = (await requestWithApiKey(app, 'post', '/builds', user.apiKey).send(createBuildDto).expect(201))
+          .body.id;
+
+        await testRunsService.postTestRun({
+          createTestRequestDto: {
+            projectId: project.id,
+            branchName: `branchName-${count}`,
+            buildId,
+            name: 'Image name',
+            merge: false,
+          },
+          imageBuffer: readFileSync(image_v1),
+        });
       };
 
       for (let i = 0; i < 6; i++) {