From d59b90263d00f239c306ebedcbaaddf02b22da17 Mon Sep 17 00:00:00 2001 From: Kalin Krustev Date: Tue, 18 Jun 2024 05:08:54 +0000 Subject: [PATCH] fix: avoid 2 extra db requests --- src/domain/fx/cyril.js | 37 --------------- src/handlers/transfers/FxFulfilService.js | 8 ++-- src/models/fxTransfer/fxTransfer.js | 2 + src/models/fxTransfer/watchList.js | 6 --- test/fixtures.js | 1 + test/unit/domain/fx/cyril.test.js | 47 ------------------- .../transfers/fxFuflilHandler.test.js | 4 +- 7 files changed, 9 insertions(+), 96 deletions(-) diff --git a/src/domain/fx/cyril.js b/src/domain/fx/cyril.js index d66ff2f72..eebc8f4e4 100644 --- a/src/domain/fx/cyril.js +++ b/src/domain/fx/cyril.js @@ -117,42 +117,6 @@ const getParticipantAndCurrencyForFxTransferMessage = async (payload) => { } } -const processFxFulfilMessage = async (commitRequestId, payload) => { - const histTimerGetParticipantAndCurrencyForFxTransferMessage = Metrics.getHistogram( - 'fx_domain_cyril_processFxFulfilMessage', - 'fx_domain_cyril_processFxFulfilMessage - Metrics for fx cyril', - ['success'] - ).startTimer() - // Does this commitRequestId appear on the watch list? - const watchListRecord = await watchList.getItemInWatchListByCommitRequestId(commitRequestId) - if (!watchListRecord) { - throw new Error(`Commit request ID ${commitRequestId} not found in watch list`) - } - const fxTransferRecord = await fxTransfer.getAllDetailsByCommitRequestId(commitRequestId) - const { - initiatingFspParticipantCurrencyId, - initiatingFspParticipantId, - initiatingFspName, - counterPartyFspSourceParticipantCurrencyId, - counterPartyFspTargetParticipantCurrencyId, - counterPartyFspParticipantId, - counterPartyFspName - } = fxTransferRecord - - // TODO: May need to update the watchList record to indicate that the fxTransfer has been fulfilled - - histTimerGetParticipantAndCurrencyForFxTransferMessage({ success: true }) - return { - initiatingFspParticipantCurrencyId, - initiatingFspParticipantId, - initiatingFspName, - counterPartyFspSourceParticipantCurrencyId, - counterPartyFspTargetParticipantCurrencyId, - counterPartyFspParticipantId, - counterPartyFspName - } -} - const processFulfilMessage = async (transferId, payload, transfer) => { const histTimerGetParticipantAndCurrencyForFxTransferMessage = Metrics.getHistogram( 'fx_domain_cyril_processFulfilMessage', @@ -260,6 +224,5 @@ const processFulfilMessage = async (transferId, payload, transfer) => { module.exports = { getParticipantAndCurrencyForTransferMessage, getParticipantAndCurrencyForFxTransferMessage, - processFxFulfilMessage, processFulfilMessage } diff --git a/src/handlers/transfers/FxFulfilService.js b/src/handlers/transfers/FxFulfilService.js index e69a5fb39..d8e22a8dd 100644 --- a/src/handlers/transfers/FxFulfilService.js +++ b/src/handlers/transfers/FxFulfilService.js @@ -307,17 +307,19 @@ class FxFulfilService { async processFxFulfil({ transfer, payload, action }) { await this.FxTransferModel.fxTransfer.saveFxFulfilResponse(transfer.commitRequestId, payload, action) - const cyrilOutput = await this.cyril.processFxFulfilMessage(transfer.commitRequestId, payload) + if (!transfer.fxWatchListId) { + throw new Error(`Commit request ID ${transfer.commitRequestId} not found in watch list`) + } const eventDetail = { functionality: Type.POSITION, action } - this.log.info('handle fxFulfilResponse', { eventDetail, cyrilOutput }) + this.log.info('handle fxFulfilResponse', { eventDetail }) await this.kafkaProceed({ consumerCommit, eventDetail, - messageKey: cyrilOutput.counterPartyFspSourceParticipantCurrencyId.toString(), + messageKey: transfer.counterPartyFspSourceParticipantCurrencyId.toString(), topicNameOverride: this.Config.KAFKA_CONFIG.EVENT_TYPE_ACTION_TOPIC_MAP?.POSITION?.COMMIT }) return true diff --git a/src/models/fxTransfer/fxTransfer.js b/src/models/fxTransfer/fxTransfer.js index 3dac9fd01..c048c87ba 100644 --- a/src/models/fxTransfer/fxTransfer.js +++ b/src/models/fxTransfer/fxTransfer.js @@ -90,9 +90,11 @@ const getAllDetailsByCommitRequestId = async (commitRequestId) => { .leftJoin('fxTransferStateChange AS tsc', 'tsc.commitRequestId', 'fxTransfer.commitRequestId') .leftJoin('transferState AS ts', 'ts.transferStateId', 'tsc.transferStateId') .leftJoin('fxTransferFulfilment AS tf', 'tf.commitRequestId', 'fxTransfer.commitRequestId') + .leftJoin('fxWatchList AS wl', 'wl.commitRequestId', 'fxTransfer.commitRequestId') // .leftJoin('transferError as te', 'te.commitRequestId', 'transfer.commitRequestId') // currently transferError.transferId is PK ensuring one error per transferId .select( 'fxTransfer.*', + 'wl.fxWatchListId', 'pc1.participantCurrencyId AS initiatingFspParticipantCurrencyId', 'tp1.amount AS initiatingFspAmount', 'da.participantId AS initiatingFspParticipantId', diff --git a/src/models/fxTransfer/watchList.js b/src/models/fxTransfer/watchList.js index 88a66fd9c..0f4e8a05b 100644 --- a/src/models/fxTransfer/watchList.js +++ b/src/models/fxTransfer/watchList.js @@ -27,11 +27,6 @@ const Db = require('../../lib/db') const { TABLE_NAMES } = require('../../shared/constants') const { logger } = require('../../shared/logger') -const getItemInWatchListByCommitRequestId = async (commitRequestId) => { - logger.debug(`get item in watch list (commitRequestId=${commitRequestId})`) - return Db.from(TABLE_NAMES.fxWatchList).findOne({ commitRequestId }) -} - const getItemsInWatchListByDeterminingTransferId = async (determiningTransferId) => { logger.debug(`get item in watch list (determiningTransferId=${determiningTransferId})`) return Db.from(TABLE_NAMES.fxWatchList).find({ determiningTransferId }) @@ -43,7 +38,6 @@ const addToWatchList = async (record) => { } module.exports = { - getItemInWatchListByCommitRequestId, getItemsInWatchListByDeterminingTransferId, addToWatchList } diff --git a/test/fixtures.js b/test/fixtures.js index 409a4c613..f868b67ef 100644 --- a/test/fixtures.js +++ b/test/fixtures.js @@ -254,6 +254,7 @@ const fxtGetAllDetailsByCommitRequestIdDto = ({ } = fxTransferDto()) => ({ commitRequestId, determiningTransferId, + fxWatchListId: 100, sourceAmount: sourceAmount.amount, sourceCurrency: sourceAmount.currency, targetAmount: targetAmount.amount, diff --git a/test/unit/domain/fx/cyril.test.js b/test/unit/domain/fx/cyril.test.js index 0090b2d3b..365791722 100644 --- a/test/unit/domain/fx/cyril.test.js +++ b/test/unit/domain/fx/cyril.test.js @@ -178,53 +178,6 @@ Test('Cyril', cyrilTest => { getParticipantAndCurrencyForFxTransferMessageTest.end() }) - cyrilTest.test('processFxFulfilMessage should', processFxFulfilMessageTest => { - processFxFulfilMessageTest.test('throws error when commitRequestId not in watchlist', async (test) => { - try { - watchList.getItemInWatchListByCommitRequestId.returns(Promise.resolve(null)) - await Cyril.processFxFulfilMessage(fxPayload.commitRequestId) - test.ok(watchList.getItemInWatchListByCommitRequestId.calledWith(fxPayload.commitRequestId)) - test.fail('Error not thrown') - test.end() - } catch (e) { - test.pass('Error Thrown') - test.end() - } - }) - - processFxFulfilMessageTest.test('should return fxTransferRecord when commitRequestId is in watchlist', async (test) => { - try { - const fxTransferRecordDetails = { - initiatingFspParticipantCurrencyId: 1, - initiatingFspParticipantId: 1, - initiatingFspName: 'fx_dfsp1', - counterPartyFspSourceParticipantCurrencyId: 1, - counterPartyFspTargetParticipantCurrencyId: 2, - counterPartyFspParticipantId: 2, - counterPartyFspName: 'fx_dfsp2' - } - watchList.getItemInWatchListByCommitRequestId.returns(Promise.resolve({ - commitRequestId: fxPayload.commitRequestId, - determiningTransferId: fxPayload.determiningTransferId, - fxTransferTypeId: Enum.Fx.FxTransferType.PAYER_CONVERSION, - createdDate: new Date() - })) - fxTransfer.getAllDetailsByCommitRequestId.returns(Promise.resolve(fxTransferRecordDetails)) - const result = await Cyril.processFxFulfilMessage(fxPayload.commitRequestId) - test.ok(watchList.getItemInWatchListByCommitRequestId.calledWith(fxPayload.commitRequestId)) - test.ok(fxTransfer.getAllDetailsByCommitRequestId.calledWith(fxPayload.commitRequestId)) - test.deepEqual(result, fxTransferRecordDetails) - test.pass('Error not thrown') - test.end() - } catch (e) { - test.fail('Error Thrown') - test.end() - } - }) - - processFxFulfilMessageTest.end() - }) - cyrilTest.test('processFulfilMessage should', processFulfilMessageTest => { processFulfilMessageTest.test('return false if transferId is not in watchlist', async (test) => { try { diff --git a/test/unit/handlers/transfers/fxFuflilHandler.test.js b/test/unit/handlers/transfers/fxFuflilHandler.test.js index 2bffd3b68..3242cdefc 100644 --- a/test/unit/handlers/transfers/fxFuflilHandler.test.js +++ b/test/unit/handlers/transfers/fxFuflilHandler.test.js @@ -339,7 +339,7 @@ Test('FX Transfer Fulfil handler -->', fxFulfilTest => { fxFulfilTest.test('should process fxFulfil callback - just skip message if no commitRequestId in watchList', async (t) => { // todo: clarify this behaviuor const fxTransferDetails = fixtures.fxtGetAllDetailsByCommitRequestIdDto() - sandbox.stub(FxFulfilService.prototype, 'getFxTransferDetails').resolves(fxTransferDetails) + sandbox.stub(FxFulfilService.prototype, 'getFxTransferDetails').resolves({ ...fxTransferDetails, fxWatchListId: null }) sandbox.stub(FxFulfilService.prototype, 'validateHeaders').resolves() sandbox.stub(FxFulfilService.prototype, 'validateEventType').resolves() sandbox.stub(FxFulfilService.prototype, 'validateFulfilment').resolves() @@ -350,7 +350,6 @@ Test('FX Transfer Fulfil handler -->', fxFulfilTest => { hasDuplicateHash: false }) Validator.validateFulfilCondition.returns(true) - fxTransferModel.watchList.getItemInWatchListByCommitRequestId.resolves(null) const metadata = fixtures.fulfilMetadataDto({ action: Action.FX_COMMIT }) const kafkaMessage = fixtures.fxFulfilKafkaMessageDto({ metadata }) @@ -375,7 +374,6 @@ Test('FX Transfer Fulfil handler -->', fxFulfilTest => { }) Validator.validateFulfilCondition.returns(true) fxTransferModel.fxTransfer.getAllDetailsByCommitRequestId.resolves(fxTransferDetails) - fxTransferModel.watchList.getItemInWatchListByCommitRequestId.resolves(fixtures.watchListItemDto()) const action = Action.FX_COMMIT const metadata = fixtures.fulfilMetadataDto({ action })