From 814d4bd04fafb22bf82f68ef0f9b0b883d1f8d43 Mon Sep 17 00:00:00 2001 From: David Jimenez Barrantes Date: Wed, 25 Oct 2023 12:07:21 -0600 Subject: [PATCH] Adds session. Centralize catch. --- .../src/repository/Db.ts | 31 +++++++++++-------- .../src/repository/Delete.ts | 27 ++++------------ .../src/repository/Update.ts | 11 +++---- .../src/repository/Upsert.ts | 8 ++--- .../test/integration/locking/Delete.test.ts | 10 +++--- .../test/integration/locking/Update.test.ts | 12 ++++--- .../test/repository/Delete.test.ts | 3 +- .../test/repository/Update.test.ts | 3 +- .../test/repository/Upsert.test.ts | 3 +- 9 files changed, 52 insertions(+), 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 index 25c86a30..beab27f8 100644 --- 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, concurrencyDocuments: ConcurrencyDocument[], + session: ClientSession, ): Promise { - await concurrencyCollection.insertMany(concurrencyDocuments); + await concurrencyCollection.insertMany(concurrencyDocuments, { session }); } /** @@ -144,24 +145,28 @@ export async function writeLockDocuments( export async function removeDocumentLocks( concurrencyCollection: Collection, concurrencyDocuments: ConcurrencyDocument[], + session: ClientSession, ): Promise { 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 index e7eb8262..2eef4700 100644 --- 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 index 006ad5cd..5781f67e 100644 --- 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 index 9aeda960..6a93a0be 100644 --- 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 index bab9b431..befd24d6 100644 --- 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 index 26927bdc..3fe818e1 100644 --- 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 index 0d0dbef0..06e29957 100644 --- 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 index f932b172..32e0422c 100644 --- 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 index 552c1c69..b93e0d03 100644 --- 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(() => {