-
Notifications
You must be signed in to change notification settings - Fork 90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: payment method handler service (quoting) #1974
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
71483aa
feat: move ilp related items into /payment-method/ilp/
mkurapov c9ac007
Merge branch 'main' into mk/1967/payment-manager-service
mkurapov cf557c6
feat: adding payment method services
mkurapov b6d3ea7
chore: rename to PaymentMethodHandlerService
mkurapov 84ea26a
chore: revert quote service change for now
mkurapov 0f3800a
chore: use mockRatesApi helper function in rates service tests
mkurapov 04edc38
chore: some cleanup
mkurapov 40f4045
chore: renaming
mkurapov bdfb97b
chore: use object to resolve payment method services
mkurapov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,6 +47,8 @@ import { createFeeService } from './fee/service' | |
import { createAutoPeeringService } from './payment-method/ilp/auto-peering/service' | ||
import { createAutoPeeringRoutes } from './payment-method/ilp/auto-peering/routes' | ||
import axios from 'axios' | ||
import { createIlpPaymentService } from './payment-method/ilp/service' | ||
import { createPaymentMethodHandlerService } from './payment-method/handler/service' | ||
|
||
BigInt.prototype.toJSON = function () { | ||
return this.toString() | ||
|
@@ -432,6 +434,24 @@ export function initIocContainer( | |
}) | ||
}) | ||
|
||
container.singleton('ilpPaymentService', async (deps) => { | ||
return createIlpPaymentService({ | ||
logger: await deps.use('logger'), | ||
knex: await deps.use('knex'), | ||
config: await deps.use('config'), | ||
makeIlpPlugin: await deps.use('makeIlpPlugin'), | ||
ratesService: await deps.use('ratesService') | ||
}) | ||
}) | ||
|
||
container.singleton('paymentMethodHandlerService', async (deps) => { | ||
return createPaymentMethodHandlerService({ | ||
logger: await deps.use('logger'), | ||
knex: await deps.use('knex'), | ||
ilpPaymentService: await deps.use('ilpPaymentService') | ||
}) | ||
}) | ||
|
||
Comment on lines
+437
to
+454
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not used yet, but will be in the next PR |
||
return container | ||
} | ||
|
||
|
64 changes: 64 additions & 0 deletions
64
packages/backend/src/payment-method/handler/service.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
import { PaymentMethodHandlerService, StartQuoteOptions } from './service' | ||
import { initIocContainer } from '../../' | ||
import { createTestApp, TestContainer } from '../../tests/app' | ||
import { Config } from '../../config/app' | ||
import { IocContract } from '@adonisjs/fold' | ||
import { AppServices } from '../../app' | ||
import { createAsset } from '../../tests/asset' | ||
import { createPaymentPointer } from '../../tests/paymentPointer' | ||
|
||
import { createReceiver } from '../../tests/receiver' | ||
import { IlpPaymentService } from '../ilp/service' | ||
import { truncateTables } from '../../tests/tableManager' | ||
|
||
describe('PaymentMethodHandlerService', (): void => { | ||
let deps: IocContract<AppServices> | ||
let appContainer: TestContainer | ||
let paymentMethodHandlerService: PaymentMethodHandlerService | ||
let ilpPaymentService: IlpPaymentService | ||
|
||
beforeAll(async (): Promise<void> => { | ||
deps = initIocContainer(Config) | ||
appContainer = await createTestApp(deps) | ||
|
||
paymentMethodHandlerService = await deps.use('paymentMethodHandlerService') | ||
ilpPaymentService = await deps.use('ilpPaymentService') | ||
}) | ||
|
||
afterEach(async (): Promise<void> => { | ||
jest.restoreAllMocks() | ||
await truncateTables(appContainer.knex) | ||
}) | ||
|
||
afterAll(async (): Promise<void> => { | ||
await appContainer.shutdown() | ||
}) | ||
|
||
describe('getQuote', (): void => { | ||
test('calls ilpPaymentService for ILP payment type', async (): Promise<void> => { | ||
const asset = await createAsset(deps) | ||
const paymentPointer = await createPaymentPointer(deps, { | ||
assetId: asset.id | ||
}) | ||
|
||
const options: StartQuoteOptions = { | ||
paymentPointer, | ||
receiver: await createReceiver(deps, paymentPointer), | ||
debitAmount: { | ||
assetCode: 'USD', | ||
assetScale: 2, | ||
value: 100n | ||
} | ||
} | ||
|
||
const ilpPaymentServiceGetQuoteSpy = jest.spyOn( | ||
ilpPaymentService, | ||
'getQuote' | ||
) | ||
|
||
await paymentMethodHandlerService.getQuote('ILP', options) | ||
|
||
expect(ilpPaymentServiceGetQuoteSpy).toHaveBeenCalledWith(options) | ||
}) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
import { Amount } from '../../open_payments/amount' | ||
import { PaymentPointer } from '../../open_payments/payment_pointer/model' | ||
import { Receiver } from '../../open_payments/receiver/model' | ||
import { BaseService } from '../../shared/baseService' | ||
import { IlpPaymentService } from '../ilp/service' | ||
|
||
export interface StartQuoteOptions { | ||
paymentPointer: PaymentPointer | ||
debitAmount?: Amount | ||
receiveAmount?: Amount | ||
receiver: Receiver | ||
} | ||
|
||
export interface PaymentQuote { | ||
paymentPointer: PaymentPointer | ||
receiver: Receiver | ||
debitAmount: Amount | ||
receiveAmount: Amount | ||
additionalFields: Record<string, unknown> | ||
} | ||
|
||
export interface PaymentMethodService { | ||
getQuote(quoteOptions: StartQuoteOptions): Promise<PaymentQuote> | ||
} | ||
Comment on lines
+7
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interfaces that will need to be satisfied for all payment method handlers |
||
|
||
export type PaymentMethod = 'ILP' | ||
|
||
export interface PaymentMethodHandlerService { | ||
getQuote( | ||
method: PaymentMethod, | ||
quoteOptions: StartQuoteOptions | ||
): Promise<PaymentQuote> | ||
} | ||
|
||
interface ServiceDependencies extends BaseService { | ||
ilpPaymentService: IlpPaymentService | ||
} | ||
|
||
export async function createPaymentMethodHandlerService({ | ||
logger, | ||
knex, | ||
ilpPaymentService | ||
}: ServiceDependencies): Promise<PaymentMethodHandlerService> { | ||
const log = logger.child({ | ||
service: 'PaymentMethodHandlerService' | ||
}) | ||
const deps: ServiceDependencies = { | ||
logger: log, | ||
knex, | ||
ilpPaymentService | ||
} | ||
|
||
const paymentMethods: { [key in PaymentMethod]: PaymentMethodService } = { | ||
ILP: deps.ilpPaymentService | ||
} | ||
|
||
return { | ||
getQuote: (method, quoteOptions) => | ||
paymentMethods[method].getQuote(quoteOptions) | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand this is a minor part of integrating different payment methods into Rafiki, but I'm currently a bit confused. Wouldn't it be more appropriate to have this as factory that is instantiated at runtime based on the payment method?
What I mean by that:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To try and build on this idea, maybe it could take in an object whose keys are the names of supported payment methods, and the values are its services. So
getQuote
would then look like:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raducristianpopa the
PaymentMethodHandlerService
is our "factory" in this case already, but instead of a new instance of a class we're just returning the service/singleton for the specific payment method that extends fromPaymentMethodService
. I could renamePaymentMethodHandlerService
toPaymentMethodFactory
if that seems better?@njlie yeah I think I'll change it to the suggestion since the types make it sort of "safe" and prevent passing in an invalid payment method 👍