fix(426): Concurrency issue with multiple requests to create build (#251

) * fix(426): Concurrency issue with multiple requests to create build closes Visual-Regression-Tracker/Visual-Regression-Tracker#426
Visual-Regression-Tracker · Oct 11, 2023 · 0e96415 · 0e96415
1 parent 531a2c1
commit 0e96415
Show file tree

Hide file tree

Showing 3 changed files with 150 additions and 14 deletions.
diff --git a/src/builds/builds.service.spec.ts b/src/builds/builds.service.spec.ts
@@ -8,6 +8,7 @@ import { mocked, MockedObject } from 'jest-mock';
 import { BuildDto } from './dto/build.dto';
 import { ProjectsService } from '../projects/projects.service';
 import { generateTestRun } from '../_data_';
+import { PrismaClientKnownRequestError } from '@prisma/client/runtime/library';
 
 jest.mock('./dto/build.dto');
 
@@ -221,4 +222,106 @@ describe('BuildsService', () => {
     });
     expect(testRunApproveMock).toHaveBeenCalledWith(build.testRuns[0].id, true);
   });
+
+  describe('findOsCreate', () => {
+    it('create without ciBuildId', async () => {
+      const buildUpsertMock = jest.fn().mockResolvedValueOnce(build);
+
+      service = await initService({ buildUpsertMock });
+      service.incrementBuildNumber = jest.fn().mockResolvedValueOnce(build);
+
+      await service.findOrCreate({
+        projectId: '111',
+        branchName: 'develop',
+      });
+
+      expect(buildUpsertMock).toHaveBeenCalledWith({
+        where: {
+          id: '111',
+        },
+        create: {
+          branchName: 'develop',
+          isRunning: true,
+          project: {
+            connect: {
+              id: '111',
+            },
+          },
+        },
+        update: {
+          isRunning: true,
+        },
+      });
+    });
+
+    it('create with ciBuildId', async () => {
+      const buildUpsertMock = jest.fn().mockResolvedValueOnce(build);
+      service = await initService({ buildUpsertMock });
+      service.incrementBuildNumber = jest.fn().mockResolvedValueOnce(build);
+
+      await service.findOrCreate({
+        projectId: '111',
+        branchName: 'develop',
+        ciBuildId: '222',
+      });
+
+      expect(buildUpsertMock).toHaveBeenCalledWith({
+        where: {
+          projectId_ciBuildId: {
+            projectId: '111',
+            ciBuildId: '222',
+          },
+        },
+        create: {
+          branchName: 'develop',
+          ciBuildId: '222',
+          isRunning: true,
+          project: {
+            connect: {
+              id: '111',
+            },
+          },
+        },
+        update: {
+          isRunning: true,
+        },
+      });
+    });
+
+    it('create with retry', async () => {
+      const buildUpsertMock = jest
+        .fn()
+        .mockRejectedValueOnce(new PrismaClientKnownRequestError('mock error', { code: 'P2002', clientVersion: '5' }));
+      const buildUpdateMock = jest.fn().mockResolvedValueOnce(build);
+      service = await initService({ buildUpsertMock, buildUpdateMock });
+      service.incrementBuildNumber = jest.fn().mockResolvedValueOnce(build);
+
+      const result = await service.findOrCreate({
+        projectId: '111',
+        branchName: 'develop',
+      });
+
+      expect(result).toEqual(build);
+    });
+
+    it('update already created', async () => {
+      const buildUpsertMock = jest.fn().mockResolvedValueOnce({
+        ...build,
+        number: 100,
+      });
+      service = await initService({ buildUpsertMock });
+      service.incrementBuildNumber = jest.fn();
+
+      const result = await service.findOrCreate({
+        projectId: '111',
+        branchName: 'develop',
+      });
+
+      expect(service.incrementBuildNumber).toHaveBeenCalledTimes(0);
+      expect(result).toEqual({
+        ...build,
+        number: 100,
+      });
+    });
+  });
 });
diff --git a/src/builds/builds.service.ts b/src/builds/builds.service.ts
@@ -133,22 +133,37 @@ export class BuildsService {
         }
       : { id: projectId };
 
-    let build = await this.prismaService.build.upsert({
-      where,
-      create: {
-        ciBuildId,
-        branchName,
-        isRunning: true,
-        project: {
-          connect: {
-            id: projectId,
+    let build: Build;
+    try {
+      build = await this.prismaService.build.upsert({
+        where,
+        create: {
+          ciBuildId,
+          branchName,
+          isRunning: true,
+          project: {
+            connect: {
+              id: projectId,
+            },
           },
         },
-      },
-      update: {
-        isRunning: true,
-      },
-    });
+        update: {
+          isRunning: true,
+        },
+      });
+    } catch (e) {
+      if (e instanceof Prisma.PrismaClientKnownRequestError) {
+        if (e.code === 'P2002') {
+          // cuncurent upsert workaround https://www.prisma.io/docs/reference/api-reference/prisma-client-reference#unique-key-constraint-errors-on-upserts
+          build = await this.prismaService.build.update({
+            where,
+            data: {
+              isRunning: true,
+            },
+          });
+        }
+      }
+    }
 
     // assigne build number
     if (!build.number) {

diff --git a/test/builds.e2e-spec.ts b/test/builds.e2e-spec.ts
@@ -151,6 +151,24 @@ describe('Builds (e2e)', () => {
         });
     });
 
+    it('201 cuncurrent', async () => {
+      const createBuildDto: CreateBuildDto = {
+        ciBuildId: 'ciBuildId',
+        branchName: 'branchName',
+        project: project.name,
+      };
+
+      const builds = await Promise.all([
+        buildsController.create(createBuildDto),
+        buildsController.create(createBuildDto),
+        buildsController.create(createBuildDto),
+        buildsController.create(createBuildDto),
+        buildsController.create(createBuildDto),
+      ]);
+
+      expect(new Set(builds.map((build) => build.id)).size).toBe(1);
+    });
+
     it('404', () => {
       const createBuildDto: CreateBuildDto = {
         branchName: 'branchName',