Adds session. Centralize catch.

Ed-Fi-Exchange-OSS · Oct 25, 2023 · 814d4bd · 814d4bd
1 parent fa6e4eb
commit 814d4bd
Show file tree

Hide file tree

Showing 9 changed files with 52 additions and 56 deletions.
diff --git a/Meadowlark-js/backends/meadowlark-mongodb-backend/src/repository/Db.ts b/Meadowlark-js/backends/meadowlark-mongodb-backend/src/repository/Db.ts
@@ -134,8 +134,9 @@ export const limitFive = (session: ClientSession): FindOptions => ({ limit: 5, s
 export async function writeLockDocuments(
   concurrencyCollection: Collection<ConcurrencyDocument>,
   concurrencyDocuments: ConcurrencyDocument[],
+  session: ClientSession,
 ): Promise<void> {
-  await concurrencyCollection.insertMany(concurrencyDocuments);
+  await concurrencyCollection.insertMany(concurrencyDocuments, { session });
 }
 
 /**
@@ -144,24 +145,28 @@ export async function writeLockDocuments(
 export async function removeDocumentLocks(
   concurrencyCollection: Collection<ConcurrencyDocument>,
   concurrencyDocuments: ConcurrencyDocument[],
+  session: ClientSession,
 ): Promise<void> {
   const meadowlarkIds: MeadowlarkId[] = concurrencyDocuments
     .map((document: ConcurrencyDocument) => document.meadowlarkId)
     .filter((meadowlarkId: MeadowlarkId | null) => meadowlarkId != null) as MeadowlarkId[];
   const documentUuids: DocumentUuid[] = concurrencyDocuments.map((document) => document.documentUuid);
 
-  await concurrencyCollection.deleteMany({
-    $and: [
-      {
-        meadowlarkId: {
-          $in: meadowlarkIds,
+  await concurrencyCollection.deleteMany(
+    {
+      $and: [
+        {
+          meadowlarkId: {
+            $in: meadowlarkIds,
+          },
         },
-      },
-      {
-        documentUuid: {
-          $in: documentUuids,
+        {
+          documentUuid: {
+            $in: documentUuids,
+          },
         },
-      },
-    ],
-  });
+      ],
+    },
+    { session },
+  );
 }
diff --git a/Meadowlark-js/backends/meadowlark-mongodb-backend/src/repository/Delete.ts b/Meadowlark-js/backends/meadowlark-mongodb-backend/src/repository/Delete.ts
@@ -109,22 +109,11 @@ export async function deleteDocumentByMeadowlarkIdTransaction(
     },
   ];
 
-  try {
-    await writeLockDocuments(concurrencyCollection, concurrencyDocuments);
-  } catch (e) {
-    // Codes 11000 and 11001 are both Duplicate Key Error
-    if (e.code === 11000 || e.code === 11001) {
-      return {
-        response: 'DELETE_FAILURE_WRITE_CONFLICT',
-        failureMessage: 'Write conflict due to concurrent access to this or related resources',
-      };
-    }
+  await writeLockDocuments(concurrencyCollection, concurrencyDocuments, session);
 
-    throw e;
-  }
   const { acknowledged, deletedCount } = await mongoCollection.deleteOne({ documentUuid }, { session });
 
-  await removeDocumentLocks(concurrencyCollection, concurrencyDocuments);
+  await removeDocumentLocks(concurrencyCollection, concurrencyDocuments, session);
 
   if (!acknowledged) {
     const msg =
@@ -180,20 +169,16 @@ export async function deleteDocumentByDocumentUuid(
     );
   } catch (e) {
     Logger.error(`${moduleName}.deleteDocumentByDocumentUuid`, deleteRequest.traceId, e);
+    await session.abortTransaction();
 
-    let response: DeleteResult = { response: 'UNKNOWN_FAILURE', failureMessage: e.message };
-
-    // Codes 11000 and 11001 are both Duplicate Key Error
-    if (e.code === 11000 || e.code === 11001) {
-      response = {
+    if (e.codeName === 'WriteConflict') {
+      return {
         response: 'DELETE_FAILURE_WRITE_CONFLICT',
         failureMessage: 'Write conflict due to concurrent access to this or related resources',
       };
     }
 
-    await session.abortTransaction();
-
-    return response;
+    return { response: 'UNKNOWN_FAILURE', failureMessage: e.message };
   } finally {
     await session.endSession();
   }

diff --git a/Meadowlark-js/backends/meadowlark-mongodb-backend/src/repository/Update.ts b/Meadowlark-js/backends/meadowlark-mongodb-backend/src/repository/Update.ts
@@ -164,7 +164,7 @@ async function updateAllowingIdentityChange(
   }));
   concurrencyDocuments.push({ meadowlarkId: updateRequest.meadowlarkId, documentUuid: updateRequest.documentUuid });
 
-  await writeLockDocuments(concurrencyCollection, concurrencyDocuments);
+  await writeLockDocuments(concurrencyCollection, concurrencyDocuments, session);
 
   // Optimize happy path by trying a replacement update, which will succeed if there is no identity change
   const tryUpdateByReplacementResult: UpdateResult | null = await tryUpdateByReplacement(
@@ -174,7 +174,7 @@ async function updateAllowingIdentityChange(
     session,
   );
 
-  await removeDocumentLocks(concurrencyCollection, concurrencyDocuments);
+  await removeDocumentLocks(concurrencyCollection, concurrencyDocuments, session);
 
   if (tryUpdateByReplacementResult != null) {
     return tryUpdateByReplacementResult;
@@ -262,7 +262,7 @@ async function updateDisallowingIdentityChange(
   }));
   concurrencyDocuments.push({ meadowlarkId: updateRequest.meadowlarkId, documentUuid: updateRequest.documentUuid });
 
-  await writeLockDocuments(concurrencyCollection, concurrencyDocuments);
+  await writeLockDocuments(concurrencyCollection, concurrencyDocuments, session);
 
   const tryUpdateByReplacementResult: UpdateResult | null = await tryUpdateByReplacement(
     document,
@@ -271,7 +271,7 @@ async function updateDisallowingIdentityChange(
     session,
   );
 
-  await removeDocumentLocks(concurrencyCollection, concurrencyDocuments);
+  await removeDocumentLocks(concurrencyCollection, concurrencyDocuments, session);
 
   if (tryUpdateByReplacementResult != null) return tryUpdateByReplacementResult;
 
@@ -426,8 +426,7 @@ export async function updateDocumentByDocumentUuid(
     Logger.error(`${moduleName}.updateDocumentByDocumentUuid`, traceId, e);
     await session.abortTransaction();
 
-    // Codes 11000 and 11001 are both Duplicate Key Error
-    if (e.code === 11000 || e.code === 11001) {
+    if (e.codeName === 'WriteConflict') {
       return {
         response: 'UPDATE_FAILURE_WRITE_CONFLICT',
         failureMessage: 'Write conflict due to concurrent access to this or related resources.',

diff --git a/Meadowlark-js/backends/meadowlark-mongodb-backend/src/repository/Upsert.ts b/Meadowlark-js/backends/meadowlark-mongodb-backend/src/repository/Upsert.ts
@@ -145,17 +145,18 @@ export async function upsertDocumentTransaction(
   }));
   concurrencyDocuments.push({ meadowlarkId, documentUuid });
 
-  await writeLockDocuments(concurrencyCollection, concurrencyDocuments);
   // Perform the document upsert
   Logger.debug(`${moduleName}.upsertDocumentTransaction Upserting document uuid ${documentUuid}`, traceId);
 
+  await writeLockDocuments(concurrencyCollection, concurrencyDocuments, session);
+
   const { acknowledged, upsertedCount, modifiedCount } = await mongoCollection.replaceOne(
     { _id: meadowlarkId },
     document,
     asUpsert(session),
   );
 
-  await removeDocumentLocks(concurrencyCollection, concurrencyDocuments);
+  await removeDocumentLocks(concurrencyCollection, concurrencyDocuments, session);
 
   if (!acknowledged) {
     const msg =
@@ -213,8 +214,7 @@ export async function upsertDocument(upsertRequest: UpsertRequest, client: Mongo
     Logger.error(`${moduleName}.upsertDocument`, upsertRequest.traceId, e);
     await session.abortTransaction();
 
-    // Codes 11000 and 11001 are both Duplicate Key Error
-    if (e.code === 11000 || e.code === 11001) {
+    if (e.codeName === 'WriteConflict') {
       return {
         response: 'UPSERT_FAILURE_WRITE_CONFLICT',
         failureMessage: 'Write conflict due to concurrent access to this or related resources',

diff --git a/Meadowlark-js/backends/meadowlark-mongodb-backend/test/integration/locking/Delete.test.ts b/Meadowlark-js/backends/meadowlark-mongodb-backend/test/integration/locking/Delete.test.ts
@@ -199,7 +199,7 @@ describe('given a delete concurrent with an insert referencing the to-be-deleted
     concurrencyDocumentsAcademicWeek.push({ meadowlarkId: academicWeekMeadowlarkId, documentUuid });
     concurrencyDocumentsAcademicWeek.push({ meadowlarkId: schoolMeadowlarkId, documentUuid: schoolDocument.documentUuid });
 
-    await writeLockDocuments(mongoConcurrencyCollection, concurrencyDocumentsAcademicWeek);
+    await writeLockDocuments(mongoConcurrencyCollection, concurrencyDocumentsAcademicWeek, upsertSession);
 
     // ----
     // End transaction to insert the AcademicWeek document
@@ -214,13 +214,15 @@ describe('given a delete concurrent with an insert referencing the to-be-deleted
 
     // Try deleting the School document - should fail thanks to AcademicWeek's read-for-write lock
     try {
-      await writeLockDocuments(mongoConcurrencyCollection, concurrencyDocumentsSchool);
+      await writeLockDocuments(mongoConcurrencyCollection, concurrencyDocumentsSchool, deleteSession);
 
       await mongoDocumentCollection.deleteOne({ _id: schoolMeadowlarkId }, { session: deleteSession });
     } catch (e) {
-      expect(e.message).toContain('E11000 duplicate key error collection');
+      expect(e.message).toContain(
+        'WriteConflict error: this operation conflicted with another operation. Please retry your operation or multi-document transaction.',
+      );
       expect(e.name).toBe('MongoBulkWriteError');
-      expect(e.code).toBe(11000);
+      expect(e.code).toBe(112);
     }
 
     // ----

diff --git a/Meadowlark-js/backends/meadowlark-mongodb-backend/test/integration/locking/Update.test.ts b/Meadowlark-js/backends/meadowlark-mongodb-backend/test/integration/locking/Update.test.ts
@@ -24,7 +24,7 @@ import {
   getDocumentCollection,
   getNewClient,
   onlyReturnId,
-  insertMeadowlarkIdOnConcurrencyCollection,
+  writeLockDocuments,
 } from '../../../src/repository/Db';
 import {
   validateReferences,
@@ -199,7 +199,7 @@ describe('given an upsert (update) concurrent with an insert referencing the to-
     concurrencyDocumentsAcademicWeek.push({ meadowlarkId: academicWeekMeadowlarkId, documentUuid });
     concurrencyDocumentsAcademicWeek.push({ meadowlarkId: schoolMeadowlarkId, documentUuid: schoolDocument.documentUuid });
 
-    await insertMeadowlarkIdOnConcurrencyCollection(mongoConcurrencyCollection, concurrencyDocumentsAcademicWeek);
+    await writeLockDocuments(mongoConcurrencyCollection, concurrencyDocumentsAcademicWeek, upsertSession);
 
     // ----
     // End transaction to insert the AcademicWeek document
@@ -214,15 +214,17 @@ describe('given an upsert (update) concurrent with an insert referencing the to-
 
     // Try updating the School document - should fail thanks to the conflict in concurrency collection
     try {
-      await insertMeadowlarkIdOnConcurrencyCollection(mongoConcurrencyCollection, concurrencyDocumentsSchool);
+      await writeLockDocuments(mongoConcurrencyCollection, concurrencyDocumentsSchool, updateSession);
 
       schoolDocument.edfiDoc.nameOfInstitution = 'A School 124';
 
       await mongoDocumentCollection.replaceOne({ _id: schoolMeadowlarkId }, schoolDocument, asUpsert(updateSession));
     } catch (e) {
-      expect(e.message).toContain('E11000 duplicate key error collection');
+      expect(e.message).toContain(
+        'WriteConflict error: this operation conflicted with another operation. Please retry your operation or multi-document transaction.',
+      );
       expect(e.name).toBe('MongoBulkWriteError');
-      expect(e.code).toBe(11000);
+      expect(e.code).toBe(112);
     }
 
     // ----

diff --git a/Meadowlark-js/backends/meadowlark-mongodb-backend/test/repository/Delete.test.ts b/Meadowlark-js/backends/meadowlark-mongodb-backend/test/repository/Delete.test.ts
@@ -8,7 +8,8 @@ describe('given a transaction on a resource', () => {
   let mongoClientMock = {};
   let deleteOneMock = jest.fn();
   const error = {
-    code: 11000,
+    code: 112,
+    codeName: 'WriteConflict',
   };
 
   beforeAll(() => {

diff --git a/Meadowlark-js/backends/meadowlark-mongodb-backend/test/repository/Update.test.ts b/Meadowlark-js/backends/meadowlark-mongodb-backend/test/repository/Update.test.ts
@@ -16,7 +16,8 @@ describe('given a transaction on a resource', () => {
   let mongoClientMock = {};
   let updateOneMock = jest.fn();
   const error = {
-    code: 11000,
+    code: 112,
+    codeName: 'WriteConflict',
   };
 
   beforeAll(() => {

diff --git a/Meadowlark-js/backends/meadowlark-mongodb-backend/test/repository/Upsert.test.ts b/Meadowlark-js/backends/meadowlark-mongodb-backend/test/repository/Upsert.test.ts
@@ -8,7 +8,8 @@ describe('given a transaction on a resource', () => {
   let mongoClientMock = {};
   let replaceOneMock = jest.fn();
   const error = {
-    code: 11000,
+    code: 112,
+    codeName: 'WriteConflict',
   };
 
   beforeAll(() => {