From ea4abdeeddc74c458ced06cea88779be9dfd0b4e Mon Sep 17 00:00:00 2001 From: Georgi Georgiev Date: Wed, 3 Apr 2019 17:45:53 +0300 Subject: [PATCH] Bugfix/714 Settlement Transfer Fulfilment Duplicate Check (#68) * Fixed transferFulfilment issue and settlement ABORT failure. Unit tests fail * swagger.json update and unit tests fix * Fix coverage * Bump up version for next release * Updated package-lock * reset cache * Fix ml-api-adapter start failure --- .istanbul.yml | 5 +- package-lock.json | 2 +- package.json | 4 +- src/handlers/settlements/{id}.js | 3 +- src/interface/swagger.json | 4 ++ src/models/index.js | 3 + src/models/lib/enums.js | 17 +++++ src/models/settlement/facade.js | 24 +++++-- test/integration-config-mlapiadapter.json | 5 ++ test/integration/settlementTransfer.test.js | 1 - test/unit/handlers/settlements/{id}.test.js | 2 + test/unit/models/index.test.js | 1 - test/unit/models/lib/enums.test.js | 46 +++++++++++++ test/unit/models/settlement/facade.test.js | 74 ++++++++++++--------- 14 files changed, 147 insertions(+), 44 deletions(-) diff --git a/.istanbul.yml b/.istanbul.yml index ae211024..20700e16 100644 --- a/.istanbul.yml +++ b/.istanbul.yml @@ -1,6 +1,9 @@ instrumentation: include-all-sources: true - excludes: ['**/test/**'] + excludes: [ + '**/test/**, + src/models/index.js' + ] check: global: statements: 90 diff --git a/package-lock.json b/package-lock.json index db1a3afe..62875af8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "central-settlement", - "version": "5.3.0", + "version": "5.5.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index c3936bc1..c377ecd4 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "central-settlement", - "description": "Central settlements hosted by a scheme to record and make settlements.", - "version": "5.3.0", + "description": "Central settlements hosted by a scheme to record and make settlements", + "version": "5.5.0", "license": "Apache-2.0", "private": true, "author": "ModusBox", diff --git a/src/handlers/settlements/{id}.js b/src/handlers/settlements/{id}.js index 95f6579c..41eaaf3b 100644 --- a/src/handlers/settlements/{id}.js +++ b/src/handlers/settlements/{id}.js @@ -88,7 +88,8 @@ module.exports = { settlementStates: await request.server.methods.enums('settlementStates'), settlementWindowStates: await request.server.methods.enums('settlementWindowStates'), transferParticipantRoleTypes: await request.server.methods.enums('transferParticipantRoleTypes'), - transferStates: await request.server.methods.enums('transferStates') + transferStates: await request.server.methods.enums('transferStates'), + transferStateEnums: await request.server.methods.enums('transferStateEnums') } if (p.participants) { return await Settlements.putById(settlementId, request.payload, Enums) diff --git a/src/interface/swagger.json b/src/interface/swagger.json index ff294a56..f6245bf1 100644 --- a/src/interface/swagger.json +++ b/src/interface/swagger.json @@ -273,6 +273,10 @@ "type": "string", "enum": [ "PENDING_SETTLEMENT", + "PS_TRANSFERS_RECORDED", + "PS_TRANSFERS_RESERVED", + "PS_TRANSFERS_COMMITTED", + "SETTLING", "SETTLED", "ABORTED" ], diff --git a/src/models/index.js b/src/models/index.js index 0e4b2d68..b2b4c370 100644 --- a/src/models/index.js +++ b/src/models/index.js @@ -16,6 +16,8 @@ their names indented and be marked with a '-'. Email address can be added optionally within square brackets . * Gates Foundation + - Name Surname + * Georgi Georgiev * Valentin Genev * Deon Botha @@ -24,5 +26,6 @@ -------------- ******/ +'use strict' module.exports = require('@mojaloop/central-services-database').Db diff --git a/src/models/lib/enums.js b/src/models/lib/enums.js index 893461cd..6835c9fd 100644 --- a/src/models/lib/enums.js +++ b/src/models/lib/enums.js @@ -78,6 +78,23 @@ module.exports = { throw err } }, + transferStateEnums: async function () { + try { + let transferStateEnum = {} + let transferStateEnumsList = await Db.transferState.find({}) + if (transferStateEnumsList) { + for (let state of transferStateEnumsList) { + // apply distinct even though final result would contain distinct values + if (!transferStateEnum[`${state.enumeration}`]) { + transferStateEnum[`${state.enumeration}`] = state.enumeration + } + } + return transferStateEnum + } + } catch (err) { + throw err + } + }, ledgerAccountTypes: async function () { try { let ledgerAccountTypeEnum = {} diff --git a/src/models/settlement/facade.js b/src/models/settlement/facade.js index 5fe04549..33adef09 100644 --- a/src/models/settlement/facade.js +++ b/src/models/settlement/facade.js @@ -389,13 +389,15 @@ const settlementTransfersAbort = async function (settlementId, transactionTimest this.on('spcsc.settlementParticipantCurrencyId', 'spc.settlementParticipantCurrencyId') .andOn('spcsc.settlementStateId', knex.raw('?', [enums.settlementStates.ABORTED])) }) - .leftJoin('transferStateChange AS tsc1', function () { - this.on('tsc1.transferId', 'spc.settlementTransferId') - .andOn('tsc1.transferStateId', knex.raw('?', [enums.transferStates.RESERVED])) + .leftJoin('transferStateChange AS tsc1', 'tsc1.transferId', 'spc.settlementTransferId') + .leftJoin('transferState AS ts1', function () { + this.on('ts1.transferStateId', 'tsc1.transferStateId') + .andOn('ts1.enumeration', knex.raw('?', [enums.transferStateEnums.RESERVED])) }) - .leftJoin('transferStateChange AS tsc2', function () { - this.on('tsc2.transferId', 'spc.settlementTransferId') - .andOn('tsc2.transferStateId', knex.raw('?', [enums.transferStates.ABORTED])) + .leftJoin('transferStateChange AS tsc2', 'tsc2.transferId', 'spc.settlementTransferId') + .leftJoin('transferState AS ts2', function () { + this.on('ts2.transferStateId', 'tsc2.transferStateId') + .andOn('ts2.enumeration', knex.raw('?', [enums.transferStateEnums.ABORTED])) }) .join('transferParticipant AS tp1', function () { this.on('tp1.transferId', 'spc.settlementTransferId') @@ -575,9 +577,17 @@ const settlementTransfersCommit = async function (settlementId, transactionTimes for (let { transferId, ledgerEntryTypeId, dfspAccountId, dfspAmount, hubAccountId, hubAmount, dfspName, currencyId } of settlementTransferList) { // Persist transfer fulfilment and transfer state change + const transferFulfilmentId = Uuid() + await knex('transferFulfilmentDuplicateCheck') + .insert({ + transferFulfilmentId, + transferId + }) + .transacting(trx) + await knex('transferFulfilment') .insert({ - transferFulfilmentId: Uuid(), + transferFulfilmentId, transferId, ilpFulfilment: 0, completedDate: transactionTimestamp, diff --git a/test/integration-config-mlapiadapter.json b/test/integration-config-mlapiadapter.json index b7b03986..1d355bcc 100644 --- a/test/integration-config-mlapiadapter.json +++ b/test/integration-config-mlapiadapter.json @@ -6,6 +6,11 @@ "expiresIn": 180000, "generateTimeout": 30000 }, + "ENDPOINT_SECURITY":{ + "TLS": { + "rejectUnauthorized": true + } + }, "AMOUNT": { "PRECISION": 10, "SCALE": 2 diff --git a/test/integration/settlementTransfer.test.js b/test/integration/settlementTransfer.test.js index 4df0384c..44aa6cab 100644 --- a/test/integration/settlementTransfer.test.js +++ b/test/integration/settlementTransfer.test.js @@ -21,7 +21,6 @@ * Georgi Georgiev -------------- ******/ - 'use strict' const Test = require('tapes')(require('tape')) diff --git a/test/unit/handlers/settlements/{id}.test.js b/test/unit/handlers/settlements/{id}.test.js index e8f29ff7..147012f6 100644 --- a/test/unit/handlers/settlements/{id}.test.js +++ b/test/unit/handlers/settlements/{id}.test.js @@ -160,6 +160,7 @@ Test('/settlements/{id}', async (settlementTest) => { sandbox.stub(Enums, 'settlementWindowStates').returns({}) sandbox.stub(Enums, 'transferParticipantRoleTypes').returns({}) sandbox.stub(Enums, 'transferStates').returns({}) + sandbox.stub(Enums, 'transferStateEnums').returns({}) sandbox.stub(settlement, 'putById').returns({}) try { const requests = new Promise((resolve, reject) => { @@ -218,6 +219,7 @@ Test('/settlements/{id}', async (settlementTest) => { sandbox.stub(Enums, 'settlementWindowStates').returns({}) sandbox.stub(Enums, 'transferParticipantRoleTypes').returns({}) sandbox.stub(Enums, 'transferStates').returns({}) + sandbox.stub(Enums, 'transferStateEnums').returns({}) sandbox.stub(settlement, 'abortById').returns({}) try { const requests = new Promise((resolve, reject) => { diff --git a/test/unit/models/index.test.js b/test/unit/models/index.test.js index 23ed1bf1..01753586 100644 --- a/test/unit/models/index.test.js +++ b/test/unit/models/index.test.js @@ -21,7 +21,6 @@ * Georgi Georgiev -------------- ******/ - 'use strict' const Test = require('tapes')(require('tape')) diff --git a/test/unit/models/lib/enums.test.js b/test/unit/models/lib/enums.test.js index 09ac6751..b527167c 100644 --- a/test/unit/models/lib/enums.test.js +++ b/test/unit/models/lib/enums.test.js @@ -180,6 +180,52 @@ Test('Enums', async (enumsTest) => { } }) + await enumsTest.test('transferStateEnums should', async transferStateEnumsTest => { + try { + await transferStateEnumsTest.test('return', async test => { + try { + const states = [ + { enumeration: 'RECEIVED' }, + { enumeration: 'RESERVED' }, + { enumeration: 'COMMITTED' }, + { enumeration: 'ABORTED' }, + { enumeration: 'ABORTED' } + ] + Db.transferState = { find: sandbox.stub().returns(states) } + let transferStateEnumsEnum = await Enums.transferStateEnums() + test.equal(Object.keys(transferStateEnumsEnum).length, states.length - 1, 'transfer states enum') + + Db.transferState.find = sandbox.stub().returns(undefined) + transferStateEnumsEnum = await Enums.transferStateEnums() + test.notOk(transferStateEnumsEnum, 'undefined when no record is returned') + test.end() + } catch (err) { + Logger.error(`transferStateEnums failed with error - ${err}`) + test.fail() + test.end() + } + }) + + await transferStateEnumsTest.test('throw error if database is unavailable', async test => { + try { + Db.transferState = { find: sandbox.stub().throws(new Error('Database unavailable')) } + await Enums.transferStateEnums() + test.fail('Error not thrown!') + test.end() + } catch (err) { + Logger.error(`transferStateEnums failed with error - ${err}`) + test.pass('Error thrown') + test.end() + } + }) + await transferStateEnumsTest.end() + } catch (err) { + Logger.error(`enumsTest failed with error - ${err}`) + transferStateEnumsTest.fail() + transferStateEnumsTest.end() + } + }) + await enumsTest.test('ledgerAccountTypes should', async ledgerAccountTypesTest => { try { await ledgerAccountTypesTest.test('return', async test => { diff --git a/test/unit/models/settlement/facade.test.js b/test/unit/models/settlement/facade.test.js index 08acdafe..120f4977 100644 --- a/test/unit/models/settlement/facade.test.js +++ b/test/unit/models/settlement/facade.test.js @@ -183,6 +183,10 @@ Test('Settlement facade', async (settlementFacadeTest) => { RESERVED: 'RESERVED', COMMITTED: 'COMMITTED' }, + transferStateEnums: { + ABORTED: 'ABORTED', + RESERVED: 'RESERVED' + }, transferParticipantRoleTypes: { PAYER_DFSP: 'PAYER_DFSP', PAYEE_DFSP: 'PAYEE_DFSP', @@ -1535,26 +1539,30 @@ Test('Settlement facade', async (settlementFacadeTest) => { andOn: sandbox.stub() }) let join1Stub = sandbox.stub().callsArgOn(1, context) - let join2Stub = sandbox.stub().callsArgOn(1, context) let leftJoin1Stub = sandbox.stub().callsArgOn(1, context) + let leftJoin2Stub = sandbox.stub().callsArgOn(1, context) + let join2Stub = sandbox.stub().callsArgOn(1, context) let join3Stub = sandbox.stub().callsArgOn(1, context) - let join4Stub = sandbox.stub().callsArgOn(1, context) knexStub.returns({ join: join1Stub.returns({ - leftJoin: join2Stub.returns({ + leftJoin: sandbox.stub().returns({ leftJoin: leftJoin1Stub.returns({ - join: join3Stub.returns({ - join: sandbox.stub().returns({ - join: sandbox.stub().returns({ - join: join4Stub.returns({ - select: sandbox.stub().returns({ - where: sandbox.stub().returns({ - whereNull: sandbox.stub().returns({ - transacting: sandbox.stub().returns( - Promise.resolve( - stubData['settlementTransfersAbort'].settlementTransferList - ) - ) + leftJoin: sandbox.stub().returns({ + leftJoin: leftJoin2Stub.returns({ + join: join2Stub.returns({ + join: sandbox.stub().returns({ + join: sandbox.stub().returns({ + join: join3Stub.returns({ + select: sandbox.stub().returns({ + where: sandbox.stub().returns({ + whereNull: sandbox.stub().returns({ + transacting: sandbox.stub().returns( + Promise.resolve( + stubData['settlementTransfersAbort'].settlementTransferList + ) + ) + }) + }) }) }) }) @@ -1562,6 +1570,7 @@ Test('Settlement facade', async (settlementFacadeTest) => { }) }) }) + }) }) }), @@ -1768,26 +1777,30 @@ Test('Settlement facade', async (settlementFacadeTest) => { andOn: sandbox.stub() }) let join1Stub = sandbox.stub().callsArgOn(1, context) - let join2Stub = sandbox.stub().callsArgOn(1, context) let leftJoin1Stub = sandbox.stub().callsArgOn(1, context) + let leftJoin2Stub = sandbox.stub().callsArgOn(1, context) + let join2Stub = sandbox.stub().callsArgOn(1, context) let join3Stub = sandbox.stub().callsArgOn(1, context) - let join4Stub = sandbox.stub().callsArgOn(1, context) knexStub.returns({ join: join1Stub.returns({ - leftJoin: join2Stub.returns({ + leftJoin: sandbox.stub().returns({ leftJoin: leftJoin1Stub.returns({ - join: join3Stub.returns({ - join: sandbox.stub().returns({ - join: sandbox.stub().returns({ - join: join4Stub.returns({ - select: sandbox.stub().returns({ - where: sandbox.stub().returns({ - whereNull: sandbox.stub().returns({ - transacting: sandbox.stub().returns( - Promise.resolve( - stubData['settlementTransfersAbort'].settlementTransferList - ) - ) + leftJoin: sandbox.stub().returns({ + leftJoin: leftJoin2Stub.returns({ + join: join2Stub.returns({ + join: sandbox.stub().returns({ + join: sandbox.stub().returns({ + join: join3Stub.returns({ + select: sandbox.stub().returns({ + where: sandbox.stub().returns({ + whereNull: sandbox.stub().returns({ + transacting: sandbox.stub().returns( + Promise.resolve( + stubData['settlementTransfersAbort'].settlementTransferList + ) + ) + }) + }) }) }) }) @@ -1795,6 +1808,7 @@ Test('Settlement facade', async (settlementFacadeTest) => { }) }) }) + }) }) }),