Skip to content

Commit

Permalink
Fix DB transaction leaks. Update unit tests (#220)
Browse files Browse the repository at this point in the history
- Fixed DB transaction/connection leaks
- Updated unit tests
- Bumped version
  • Loading branch information
oderayi authored Jun 1, 2020
1 parent ea50235 commit 79ca56f
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 27 deletions.
59 changes: 46 additions & 13 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "quoting-service",
"description": "Quoting Service hosted by a scheme",
"license": "Apache-2.0",
"version": "10.3.1",
"version": "10.3.2",
"author": "ModusBox",
"contributors": [
"James Bush <james.bush@modusbox.com>",
Expand Down Expand Up @@ -65,7 +65,7 @@
"@mojaloop/central-services-shared": "10.2.1",
"@mojaloop/event-sdk": "9.5.2",
"@mojaloop/ml-number": "8.2.0",
"@mojaloop/sdk-standard-components": "10.2.4",
"@mojaloop/sdk-standard-components": "10.3.0",
"axios": "0.19.2",
"blipp": "4.0.1",
"eslint-config-standard": "14.1.1",
Expand All @@ -88,7 +88,7 @@
"jest-junit": "10.0.0",
"npm-audit-resolver": "2.2.0",
"npm-check-updates": "6.0.1",
"nyc": "15.0.1",
"nyc": "15.1.0",
"pre-commit": "1.2.2",
"proxyquire": "2.1.3",
"sinon": "9.0.2",
Expand Down
12 changes: 6 additions & 6 deletions src/model/quotes.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,6 @@ class QuotesModel {
await this.validateQuoteRequest(fspiopSource, fspiopDestination, quoteRequest)

if (!envConfig.simpleRoutingMode) {
// do everything in a db txn so we can rollback multiple operations if something goes wrong
txn = await this.db.newTransaction()

// check if this is a resend or an erroneous duplicate
const dupe = await this.checkDuplicateQuoteRequest(quoteRequest)

Expand All @@ -234,6 +231,9 @@ class QuotesModel {
handledRuleEvents.quoteRequest, span)
}

// do everything in a db txn so we can rollback multiple operations if something goes wrong
txn = await this.db.newTransaction()

// todo: validation

// if we get here we need to create a duplicate check row
Expand Down Expand Up @@ -475,9 +475,6 @@ class QuotesModel {
// accumulate enum ids
const refs = {}
if (!envConfig.simpleRoutingMode) {
// do everything in a transaction so we can rollback multiple operations if something goes wrong
txn = await this.db.newTransaction()

// check if this is a resend or an erroneous duplicate
const dupe = await this.checkDuplicateQuoteResponse(quoteId, quoteUpdateRequest)
this.writeLog(`Check duplicate for quoteId ${quoteId} update returned: ${util.inspect(dupe)}`)
Expand All @@ -495,6 +492,9 @@ class QuotesModel {
return this.handleQuoteUpdateResend(headers, quoteId, quoteUpdateRequest, span)
}

// do everything in a transaction so we can rollback multiple operations if something goes wrong
txn = await this.db.newTransaction()

// todo: validation

// create the quote response row in the db
Expand Down
10 changes: 5 additions & 5 deletions test/unit/model/quotes.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1397,9 +1397,9 @@ describe('QuotesModel', () => {
try {
await quotesModel.handleQuoteUpdate(mockData.headers, mockData.quoteId, mockData.quoteUpdate, mockSpan)
} catch (err) {
expect(quotesModel.db.newTransaction.mock.calls.length).toBe(1)
expect(quotesModel.db.newTransaction.mock.calls.length).toBe(0)
expect(quotesModel.checkDuplicateQuoteResponse).toBeCalledWith(mockData.quoteId, mockData.quoteUpdate)
expect(mockTransaction.rollback.mock.calls.length).toBe(1)
expect(mockTransaction.rollback.mock.calls.length).toBe(0)
expect(mockSpan.error.mock.calls[0][0]).toEqual(err)
expect(mockSpan.finish.mock.calls[0][0]).toEqual(err.message)
expect(err instanceof ErrorHandler.Factory.FSPIOPError).toBeTruthy()
Expand All @@ -1415,7 +1415,7 @@ describe('QuotesModel', () => {

const refs = await quotesModel.handleQuoteUpdate(mockData.headers, mockData.quoteId, mockData.quoteUpdate, mockSpan)

expect(quotesModel.db.newTransaction.mock.calls.length).toBe(1)
expect(quotesModel.db.newTransaction.mock.calls.length).toBe(0)
expect(quotesModel.checkDuplicateQuoteResponse).toBeCalledWith(mockData.quoteId, mockData.quoteUpdate)
const args = [mockData.headers, mockData.quoteId, mockData.quoteUpdate, mockSpan]
expect(quotesModel.handleQuoteUpdateResend).toBeCalledWith(...args)
Expand Down Expand Up @@ -1575,8 +1575,8 @@ describe('QuotesModel', () => {
.rejects
.toHaveProperty('apiErrorCode.code', ErrorHandler.Enums.FSPIOPErrorCodes.INTERNAL_SERVER_ERROR.code)

expect(quotesModel.db.newTransaction.mock.calls.length).toBe(1)
expect(mockTransaction.rollback.mock.calls.length).toBe(1)
expect(quotesModel.db.newTransaction.mock.calls.length).toBe(0)
expect(mockTransaction.rollback.mock.calls.length).toBe(0)
})
})
describe('forwardQuoteUpdate', () => {
Expand Down

0 comments on commit 79ca56f

Please sign in to comment.