Bug Fix for Transaction Read/Write Error (#401)

* added packages * sample implementation test * lint fix * testing base functionalities * lint fix * more testing * omfg i got it * clean up testing related variables and functions * lint fix * added comments * added concurrent tests * bumped version number
acmucsd · Feb 29, 2024 · 60cba34 · 60cba34
1 parent 0a6da54
commit 60cba34
Show file tree

Hide file tree

Showing 8 changed files with 248 additions and 15 deletions.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@acmucsd/membership-portal",
-  "version": "3.3.0",
+  "version": "3.4.0",
   "description": "REST API for ACM UCSD's membership portal.",
   "main": "index.d.ts",
   "files": [
@@ -39,6 +39,7 @@
   "dependencies": {
     "@sendgrid/mail": "^7.4.4",
     "amazon-s3-uri": "^0.1.1",
+    "async-retry": "^1.3.3",
     "aws-sdk": "^2.906.0",
     "bcrypt": "^5.0.0",
     "body-parser": "^1.19.0",
@@ -71,6 +72,7 @@
     "winston-daily-rotate-file": "^4.5.0"
   },
   "devDependencies": {
+    "@types/async-retry": "^1.4.8",
     "@types/bcrypt": "^3.0.0",
     "@types/body-parser": "^1.19.0",
     "@types/datadog-metrics": "^0.6.1",

diff --git a/repositories/index.ts b/repositories/index.ts
@@ -1,4 +1,5 @@
 import { EntityManager } from 'typeorm';
+import AsyncRetry = require('async-retry');
 import { UserRepository } from './UserRepository';
 import { FeedbackRepository } from './FeedbackRepository';
 import { AttendanceRepository, ExpressCheckinRepository } from './AttendanceRepository';
@@ -16,6 +17,9 @@ import { LeaderboardRepository } from './LeaderboardRepository';
 import { ResumeRepository } from './ResumeRepository';
 import { UserSocialMediaRepository } from './UserSocialMediaRepository';
 
+const HINT_FOR_RETRIABLE_TRANSACTIONS : string = 'The transaction might succeed if retried.';
+const AMOUNT_OF_RETRIES : number = 10;
+
 export default class Repositories {
   public static activity(transactionalEntityManager: EntityManager): ActivityRepository {
     return transactionalEntityManager.getCustomRepository(ActivityRepository);
@@ -93,11 +97,34 @@ export class TransactionsManager {
     this.transactionalEntityManager = transactionalEntityManager;
   }
 
+  // used async-retry library to handle automatic retries under transaction failure due to concurrent transactions
   public readOnly<T>(fn: (transactionalEntityManager: EntityManager) => Promise<T>): Promise<T> {
-    return this.transactionalEntityManager.transaction('REPEATABLE READ', fn);
+    return AsyncRetry(async (bail, attemptNum) => {
+      try {
+        const res = await this.transactionalEntityManager.transaction('REPEATABLE READ', fn);
+        return res;
+      } catch (e) {
+        if (e.hint !== HINT_FOR_RETRIABLE_TRANSACTIONS) bail(e);
+        else throw e;
+      }
+    },
+    {
+      retries: AMOUNT_OF_RETRIES,
+    });
   }
 
   public readWrite<T>(fn: (transactionalEntityManager: EntityManager) => Promise<T>): Promise<T> {
-    return this.transactionalEntityManager.transaction('SERIALIZABLE', fn);
+    return AsyncRetry(async (bail, attemptNum) => {
+      try {
+        const res = await this.transactionalEntityManager.transaction('SERIALIZABLE', fn);
+        return res;
+      } catch (e) {
+        if (e.hint !== HINT_FOR_RETRIABLE_TRANSACTIONS) bail(e);
+        else throw e;
+      }
+    },
+    {
+      retries: AMOUNT_OF_RETRIES,
+    });
   }
 }
diff --git a/tests/Seeds.ts b/tests/Seeds.ts
@@ -479,7 +479,6 @@ async function seed(): Promise<void> {
   });
   const MERCH_ITEM_1_PHOTO_1 = MerchFactory.fakePhoto({
     merchItem: MERCH_ITEM_1,
-    uploadedPhoto: 'https://www.fakepicture.com/',
     position: 1,
   });
   const MERCH_ITEM_1_PHOTO_2 = MerchFactory.fakePhoto({
@@ -542,7 +541,6 @@ async function seed(): Promise<void> {
   ];
   const MERCH_ITEM_2_PHOTO_0 = MerchFactory.fakePhoto({
     merchItem: MERCH_ITEM_2,
-    uploadedPhoto: 'https://www.fakepicture.com/',
     position: 0,
   });
   const MERCH_ITEM_2_PHOTO_1 = MerchFactory.fakePhoto({
@@ -591,7 +589,6 @@ async function seed(): Promise<void> {
   });
   const MERCH_ITEM_3_PHOTO_1 = MerchFactory.fakePhoto({
     merchItem: MERCH_ITEM_3,
-    uploadedPhoto: 'https://www.fakepicture.com/',
     position: 1,
   });
   const MERCH_ITEM_3_PHOTO_2 = MerchFactory.fakePhoto({
@@ -601,12 +598,10 @@ async function seed(): Promise<void> {
   });
   const MERCH_ITEM_3_PHOTO_3 = MerchFactory.fakePhoto({
     merchItem: MERCH_ITEM_3,
-    uploadedPhoto: 'https://www.fakepicture.com/',
     position: 3,
   });
   const MERCH_ITEM_3_PHOTO_4 = MerchFactory.fakePhoto({
     merchItem: MERCH_ITEM_3,
-    uploadedPhoto: 'https://www.fakepicture.com/',
     position: 4,
   });
   MERCH_ITEM_3.merchPhotos = [

diff --git a/tests/data/EventFactory.ts b/tests/data/EventFactory.ts
@@ -37,10 +37,10 @@ export class EventFactory {
       requiresStaff: FactoryUtils.getRandomBoolean(),
       staffPointBonus: EventFactory.randomPointValue(),
       committee: 'ACM',
-      cover: faker.internet.url(),
+      cover: FactoryUtils.getRandomImageUrl(),
       deleted: false,
       eventLink: faker.internet.url(),
-      thumbnail: faker.internet.url(),
+      thumbnail: FactoryUtils.getRandomImageUrl(),
     });
     return EventModel.merge(fake, substitute);
   }

diff --git a/tests/data/FactoryUtils.ts b/tests/data/FactoryUtils.ts
@@ -40,4 +40,8 @@ export default class FactoryUtils {
   public static randomHexString(): string {
     return faker.datatype.hexaDecimal(10);
   }
+
+  public static getRandomImageUrl(): string {
+    return `http://i.imgur.com/${faker.random.alphaNumeric(10)}.jpeg`;
+  }
 }
diff --git a/tests/data/MerchFactory.ts b/tests/data/MerchFactory.ts
@@ -72,7 +72,7 @@ export class MerchFactory {
     const fake = MerchandiseItemPhotoModel.create({
       uuid: uuid(),
       position: 0,
-      uploadedPhoto: 'https://www.fakepicture.com/',
+      uploadedPhoto: FactoryUtils.getRandomImageUrl(),
       uploadedAt: faker.date.recent(),
     });
     return MerchandiseItemPhotoModel.merge(fake, substitute);

diff --git a/tests/userSocialMedia.test.ts b/tests/userSocialMedia.test.ts
@@ -1,24 +1,26 @@
 import faker = require('faker');
+import { Connection } from 'typeorm';
 import { UserModel } from '../models/UserModel';
 import { SocialMediaType } from '../types';
 import { ControllerFactory } from './controllers';
 import { DatabaseConnection, PortalState, UserFactory } from './data';
 import { UserSocialMediaFactory } from './data/UserSocialMediaFactory';
+import { UserController } from '../api/controllers/UserController';
 
 beforeAll(async () => {
   await DatabaseConnection.connect();
 });
 
-beforeEach(async () => {
-  await DatabaseConnection.clear();
-});
-
 afterAll(async () => {
   await DatabaseConnection.clear();
   await DatabaseConnection.close();
 });
 
 describe('social media URL submission', () => {
+  beforeEach(async () => {
+    await DatabaseConnection.clear();
+  });
+
   test('properly persists on successful submission', async () => {
     const conn = await DatabaseConnection.get();
     let member = UserFactory.fake();
@@ -60,6 +62,10 @@ describe('social media URL submission', () => {
 });
 
 describe('social media URL update', () => {
+  beforeEach(async () => {
+    await DatabaseConnection.clear();
+  });
+
   test('is invalidated when updating social media URL of another user', async () => {
     const conn = await DatabaseConnection.get();
     let member = UserFactory.fake();
@@ -100,6 +106,10 @@ describe('social media URL update', () => {
 });
 
 describe('social media URL delete', () => {
+  beforeEach(async () => {
+    await DatabaseConnection.clear();
+  });
+
   test('is invalidated when deleting social media URL of another user', async () => {
     const conn = await DatabaseConnection.get();
     let member = UserFactory.fake();
@@ -119,3 +129,174 @@ describe('social media URL delete', () => {
     await expect(userController.deleteSocialMediaForUser(uuidParams, unauthorizedMember)).rejects.toThrow(errorMessage);
   });
 });
+
+describe('social media URL update', () => {
+  let conn : Connection;
+  let member : UserModel;
+  let userController : UserController;
+
+  let flag : boolean = false;
+  function waitForFlag(interval = 500, timeout = 5000) {
+    let acc = 0; // time accumulation
+    return new Promise((resolve, reject) => {
+      const i = setInterval(() => {
+        acc += interval;
+        if (flag) {
+          clearInterval(i);
+          resolve(flag);
+        }
+        if (acc > timeout) {
+          clearInterval(i);
+          reject();
+        }
+      }, interval);
+    });
+  }
+
+  // beforeAll does not behave properly when concurrent is being used:
+  // https://stackoverflow.com/questions/42633635/how-to-run-concurrent-tests-in-jest-with-multiple-tests-per-request
+  beforeAll(async () => {
+    await DatabaseConnection.clear();
+    conn = await DatabaseConnection.get();
+    member = UserFactory.fake();
+
+    const userSocialMedia0 = UserSocialMediaFactory.fake({ user: member, type: SocialMediaType.FACEBOOK });
+    const userSocialMedia1 = UserSocialMediaFactory.fake({ user: member, type: SocialMediaType.GITHUB });
+    const userSocialMedia2 = UserSocialMediaFactory.fake({ user: member, type: SocialMediaType.DEVPOST });
+    const userSocialMedia3 = UserSocialMediaFactory.fake({ user: member, type: SocialMediaType.EMAIL });
+    const userSocialMedia4 = UserSocialMediaFactory.fake({ user: member, type: SocialMediaType.INSTAGRAM });
+
+    await new PortalState()
+      .createUsers(member)
+      .createUserSocialMedia(
+        member, userSocialMedia0, userSocialMedia1, userSocialMedia2, userSocialMedia3, userSocialMedia4,
+      ).write();
+
+    userController = ControllerFactory.user(conn);
+
+    // refreshes member to have the userSocialMedia field
+    member = await conn.manager.findOne(UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] });
+
+    flag = true;
+  });
+
+  test.concurrent('concurrent updates properly persist on successful submission 0', async () => {
+    await waitForFlag();
+    const index = 0;
+    const uuidParams = { uuid: member.userSocialMedia[index].uuid };
+    const socialMediaParams = { socialMedia: { url: faker.internet.url() } };
+    await userController.updateSocialMediaForUser(uuidParams, socialMediaParams, member);
+
+    const expectedUserSocialMedia0 = {
+      url: socialMediaParams.socialMedia.url,
+      type: member.userSocialMedia[index].type,
+      uuid: uuidParams.uuid,
+    };
+    const updatedMember = await conn.manager.findOne(
+      UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] },
+    );
+
+    expect(updatedMember.userSocialMedia).toHaveLength(5);
+    const targetUserSocialMedia = updatedMember.userSocialMedia.find(
+      (socialMedia) => socialMedia.uuid === expectedUserSocialMedia0.uuid,
+    );
+    expect(targetUserSocialMedia.url).toEqual(expectedUserSocialMedia0.url);
+    expect(targetUserSocialMedia.type).toEqual(expectedUserSocialMedia0.type);
+  });
+
+  test.concurrent('concurrent updates properly persist on successful submission 1', async () => {
+    await waitForFlag();
+    const index = 1;
+    const uuidParams = { uuid: member.userSocialMedia[index].uuid };
+    const socialMediaParams = { socialMedia: { url: faker.internet.url() } };
+    await userController.updateSocialMediaForUser(uuidParams, socialMediaParams, member);
+
+    const expectedUserSocialMedia1 = {
+      url: socialMediaParams.socialMedia.url,
+      type: member.userSocialMedia[index].type,
+      uuid: uuidParams.uuid,
+    };
+    const updatedMember = await conn.manager.findOne(
+      UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] },
+    );
+
+    expect(updatedMember.userSocialMedia).toHaveLength(5);
+    const targetUserSocialMedia = updatedMember.userSocialMedia.find(
+      (socialMedia) => socialMedia.uuid === expectedUserSocialMedia1.uuid,
+    );
+    expect(targetUserSocialMedia.url).toEqual(expectedUserSocialMedia1.url);
+    expect(targetUserSocialMedia.type).toEqual(expectedUserSocialMedia1.type);
+  });
+
+  test.concurrent('concurrent updates properly persist on successful submission 2', async () => {
+    await waitForFlag();
+    const index = 2;
+    const uuidParams = { uuid: member.userSocialMedia[index].uuid };
+    const socialMediaParams = { socialMedia: { url: faker.internet.url() } };
+    await userController.updateSocialMediaForUser(uuidParams, socialMediaParams, member);
+
+    const expectedUserSocialMedia2 = {
+      url: socialMediaParams.socialMedia.url,
+      type: member.userSocialMedia[index].type,
+      uuid: uuidParams.uuid,
+    };
+    const updatedMember = await conn.manager.findOne(
+      UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] },
+    );
+
+    expect(updatedMember.userSocialMedia).toHaveLength(5);
+    const targetUserSocialMedia = updatedMember.userSocialMedia.find(
+      (socialMedia) => socialMedia.uuid === expectedUserSocialMedia2.uuid,
+    );
+    expect(targetUserSocialMedia.url).toEqual(expectedUserSocialMedia2.url);
+    expect(targetUserSocialMedia.type).toEqual(expectedUserSocialMedia2.type);
+  });
+
+  test.concurrent('concurrent updates properly persist on successful submission 3', async () => {
+    await waitForFlag();
+    const index = 3;
+    const uuidParams = { uuid: member.userSocialMedia[index].uuid };
+    const socialMediaParams = { socialMedia: { url: faker.internet.url() } };
+    await userController.updateSocialMediaForUser(uuidParams, socialMediaParams, member);
+
+    const expectedUserSocialMedia3 = {
+      url: socialMediaParams.socialMedia.url,
+      type: member.userSocialMedia[index].type,
+      uuid: uuidParams.uuid,
+    };
+    const updatedMember = await conn.manager.findOne(
+      UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] },
+    );
+
+    expect(updatedMember.userSocialMedia).toHaveLength(5);
+    const targetUserSocialMedia = updatedMember.userSocialMedia.find(
+      (socialMedia) => socialMedia.uuid === expectedUserSocialMedia3.uuid,
+    );
+    expect(targetUserSocialMedia.url).toEqual(expectedUserSocialMedia3.url);
+    expect(targetUserSocialMedia.type).toEqual(expectedUserSocialMedia3.type);
+  });
+
+  test.concurrent('concurrent updates properly persist on successful submission 4', async () => {
+    await waitForFlag();
+    const index = 4;
+    const uuidParams = { uuid: member.userSocialMedia[index].uuid };
+    const socialMediaParams = { socialMedia: { url: faker.internet.url() } };
+    await userController.updateSocialMediaForUser(uuidParams, socialMediaParams, member);
+
+    const expectedUserSocialMedia4 = {
+      url: socialMediaParams.socialMedia.url,
+      type: member.userSocialMedia[index].type,
+      uuid: uuidParams.uuid,
+    };
+    const updatedMember = await conn.manager.findOne(
+      UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] },
+    );
+
+    expect(updatedMember.userSocialMedia).toHaveLength(5);
+    const targetUserSocialMedia = updatedMember.userSocialMedia.find(
+      (socialMedia) => socialMedia.uuid === expectedUserSocialMedia4.uuid,
+    );
+    expect(targetUserSocialMedia.url).toEqual(expectedUserSocialMedia4.url);
+    expect(targetUserSocialMedia.type).toEqual(expectedUserSocialMedia4.type);
+  });
+});