From 718d962cd135cd150495398efde4e0c72271c4a9 Mon Sep 17 00:00:00 2001 From: Blair Currey <12960453+BlairCurrey@users.noreply.github.com> Date: Fri, 20 Oct 2023 13:54:13 -0400 Subject: [PATCH] fix(backend): return empty methods for complete or expired incomingPayment --- .../payment/incoming/model.test.ts | 50 +++++++++++++++++-- .../open_payments/payment/incoming/model.ts | 48 +++++++----------- .../payment/incoming/routes.test.ts | 32 +++++++++++- .../open_payments/payment/incoming/routes.ts | 32 +++--------- .../src/open_payments/receiver/model.test.ts | 32 +++++++----- .../src/open_payments/receiver/model.ts | 8 +-- .../src/open_payments/receiver/service.ts | 7 ++- packages/backend/src/tests/outgoingPayment.ts | 5 +- packages/backend/src/tests/receiver.ts | 2 +- 9 files changed, 135 insertions(+), 81 deletions(-) diff --git a/packages/backend/src/open_payments/payment/incoming/model.test.ts b/packages/backend/src/open_payments/payment/incoming/model.test.ts index 56f237fd72..036803207e 100644 --- a/packages/backend/src/open_payments/payment/incoming/model.test.ts +++ b/packages/backend/src/open_payments/payment/incoming/model.test.ts @@ -9,7 +9,7 @@ import { truncateTables } from '../../../tests/tableManager' import { IlpStreamCredentials } from '../../../payment-method/ilp/stream-credentials/service' import { serializeAmount } from '../../amount' import { IlpAddress } from 'ilp-packet' -import { IncomingPayment } from './model' +import { IncomingPayment, IncomingPaymentState } from './model' describe('Incoming Payment Model', (): void => { let deps: IocContract @@ -30,7 +30,7 @@ describe('Incoming Payment Model', (): void => { }) describe('toOpenPaymentsType', () => { - test('returns incoming payment without stream credentials provided', async () => { + test('returns incoming payment', async () => { const walletAddress = await createWalletAddress(deps) const incomingPayment = await createIncomingPayment(deps, { walletAddressId: walletAddress.id, @@ -51,8 +51,10 @@ describe('Incoming Payment Model', (): void => { createdAt: incomingPayment.createdAt.toISOString() }) }) + }) - test('returns incoming payment with stream credentials as object', async () => { + describe('toOpenPaymentsTypeWithMethods', () => { + test('returns incoming payment with payment methods', async () => { const walletAddress = await createWalletAddress(deps) const incomingPayment = await createIncomingPayment(deps, { walletAddressId: walletAddress.id, @@ -65,7 +67,10 @@ describe('Incoming Payment Model', (): void => { } expect( - incomingPayment.toOpenPaymentsType(walletAddress, streamCredentials) + incomingPayment.toOpenPaymentsTypeWithMethods( + walletAddress, + streamCredentials + ) ).toEqual({ id: `${walletAddress.url}${IncomingPayment.urlPath}/${incomingPayment.id}`, walletAddress: walletAddress.url, @@ -87,5 +92,42 @@ describe('Incoming Payment Model', (): void => { ] }) }) + + test.each([IncomingPaymentState.Completed, IncomingPaymentState.Expired])( + 'returns incoming payment with empty methods if payment state is %s', + async (paymentState): Promise => { + const walletAddress = await createWalletAddress(deps) + const incomingPayment = await createIncomingPayment(deps, { + walletAddressId: walletAddress.id, + metadata: { description: 'my payment' } + }) + incomingPayment.state = paymentState + + const streamCredentials: IlpStreamCredentials = { + ilpAddress: 'test.ilp' as IlpAddress, + sharedSecret: Buffer.from('') + } + + expect( + incomingPayment.toOpenPaymentsTypeWithMethods( + walletAddress, + streamCredentials + ) + ).toEqual({ + id: `${walletAddress.url}${IncomingPayment.urlPath}/${incomingPayment.id}`, + walletAddress: walletAddress.url, + completed: incomingPayment.completed, + receivedAmount: serializeAmount(incomingPayment.receivedAmount), + incomingAmount: incomingPayment.incomingAmount + ? serializeAmount(incomingPayment.incomingAmount) + : undefined, + expiresAt: incomingPayment.expiresAt.toISOString(), + metadata: incomingPayment.metadata ?? undefined, + updatedAt: incomingPayment.updatedAt.toISOString(), + createdAt: incomingPayment.createdAt.toISOString(), + methods: [] + }) + } + ) }) }) diff --git a/packages/backend/src/open_payments/payment/incoming/model.ts b/packages/backend/src/open_payments/payment/incoming/model.ts index e11566ebc4..6fb0fcede0 100644 --- a/packages/backend/src/open_payments/payment/incoming/model.ts +++ b/packages/backend/src/open_payments/payment/incoming/model.ts @@ -200,23 +200,8 @@ export class IncomingPayment public toOpenPaymentsType( walletAddress: WalletAddress - ): OpenPaymentsIncomingPayment - public toOpenPaymentsType( - walletAddress: WalletAddress, - ilpStreamCredentials: IlpStreamCredentials - ): OpenPaymentsIncomingPaymentWithPaymentMethod - public toOpenPaymentsType( - walletAddress: WalletAddress, - ilpStreamCredentials?: IlpStreamCredentials - ): OpenPaymentsIncomingPayment | OpenPaymentsIncomingPaymentWithPaymentMethod - - public toOpenPaymentsType( - walletAddress: WalletAddress, - ilpStreamCredentials?: IlpStreamCredentials - ): - | OpenPaymentsIncomingPayment - | OpenPaymentsIncomingPaymentWithPaymentMethod { - const baseIncomingPayment: OpenPaymentsIncomingPayment = { + ): OpenPaymentsIncomingPayment { + return { id: this.getUrl(walletAddress), walletAddress: walletAddress.url, incomingAmount: this.incomingAmount @@ -229,21 +214,24 @@ export class IncomingPayment updatedAt: this.updatedAt.toISOString(), expiresAt: this.expiresAt.toISOString() } + } - if (ilpStreamCredentials) { - return { - ...baseIncomingPayment, - methods: [ - { - type: 'ilp', - ilpAddress: ilpStreamCredentials.ilpAddress, - sharedSecret: base64url(ilpStreamCredentials.sharedSecret) - } - ] - } + public toOpenPaymentsTypeWithMethods( + walletAddress: WalletAddress, + ilpStreamCredentials: IlpStreamCredentials + ): OpenPaymentsIncomingPaymentWithPaymentMethod { + return { + ...this.toOpenPaymentsType(walletAddress), + methods: this.isExpiredOrComplete() + ? [] + : [ + { + type: 'ilp', + ilpAddress: ilpStreamCredentials.ilpAddress, + sharedSecret: base64url(ilpStreamCredentials.sharedSecret) + } + ] } - - return baseIncomingPayment } public toPublicOpenPaymentsType(): Pick< diff --git a/packages/backend/src/open_payments/payment/incoming/routes.test.ts b/packages/backend/src/open_payments/payment/incoming/routes.test.ts index 0fc8e07513..61f9d6fa25 100644 --- a/packages/backend/src/open_payments/payment/incoming/routes.test.ts +++ b/packages/backend/src/open_payments/payment/incoming/routes.test.ts @@ -15,7 +15,7 @@ import { ListContext } from '../../../app' import { truncateTables } from '../../../tests/tableManager' -import { IncomingPayment } from './model' +import { IncomingPayment, IncomingPaymentState } from './model' import { IncomingPaymentRoutes, CreateBody, @@ -143,6 +143,36 @@ describe('Incoming Payment Routes', (): void => { }) }) + describe('get', (): void => { + test.each([IncomingPaymentState.Completed, IncomingPaymentState.Expired])( + 'returns incoming payment with empty methods if payment state is %s', + async (paymentState): Promise => { + const walletAddress = await createWalletAddress(deps) + const incomingPayment = await createIncomingPayment(deps, { + walletAddressId: walletAddress.id + }) + await incomingPayment.$query().update({ state: paymentState }) + + const ctx = setup({ + reqOpts: { + headers: { Accept: 'application/json' }, + method: 'GET', + url: `/incoming-payments/${incomingPayment.id}` + }, + params: { + id: incomingPayment.id + }, + walletAddress + }) + + await expect(incomingPaymentRoutes.get(ctx)).resolves.toBeUndefined() + + expect(ctx.response).toSatisfyApiSpec() + expect(ctx.body).toMatchObject({ methods: [] }) + } + ) + }) + describe('create', (): void => { let amount: AmountJSON diff --git a/packages/backend/src/open_payments/payment/incoming/routes.ts b/packages/backend/src/open_payments/payment/incoming/routes.ts index 0028b473c3..82bd19ebb3 100644 --- a/packages/backend/src/open_payments/payment/incoming/routes.ts +++ b/packages/backend/src/open_payments/payment/incoming/routes.ts @@ -18,16 +18,8 @@ import { } from './errors' import { AmountJSON, parseAmount } from '../../amount' import { listSubresource } from '../../wallet_address/routes' -import { - IlpStreamCredentials, - StreamCredentialsService -} from '../../../payment-method/ilp/stream-credentials/service' -import { - AccessAction, - IncomingPayment as OpenPaymentsIncomingPayment, - IncomingPaymentWithPaymentMethods -} from '@interledger/open-payments' -import { WalletAddress } from '../../wallet_address/model' +import { StreamCredentialsService } from '../../../payment-method/ilp/stream-credentials/service' +import { AccessAction } from '@interledger/open-payments' interface ServiceDependencies { config: IAppConfig @@ -110,9 +102,8 @@ async function getIncomingPaymentPrivate( const streamCredentials = deps.streamCredentialsService.get(incomingPayment) - ctx.body = incomingPaymentToBody( + ctx.body = incomingPayment.toOpenPaymentsTypeWithMethods( incomingPayment.walletAddress, - incomingPayment, streamCredentials ) } @@ -158,9 +149,8 @@ async function createIncomingPayment( const streamCredentials = deps.streamCredentialsService.get( incomingPaymentOrError ) - ctx.body = incomingPaymentToBody( + ctx.body = incomingPaymentOrError.toOpenPaymentsTypeWithMethods( incomingPaymentOrError.walletAddress, - incomingPaymentOrError, streamCredentials ) } @@ -189,9 +179,8 @@ async function completeIncomingPayment( ctx.throw(404) } - ctx.body = incomingPaymentToBody( - incomingPaymentOrError.walletAddress, - incomingPaymentOrError + ctx.body = incomingPaymentOrError.toOpenPaymentsType( + incomingPaymentOrError.walletAddress ) } @@ -203,7 +192,7 @@ async function listIncomingPayments( await listSubresource({ ctx, getWalletAddressPage: deps.incomingPaymentService.getWalletAddressPage, - toBody: (payment) => incomingPaymentToBody(ctx.walletAddress, payment) + toBody: (payment) => payment.toOpenPaymentsType(ctx.walletAddress) }) } catch (err) { if (err instanceof Koa.HttpError) { @@ -212,10 +201,3 @@ async function listIncomingPayments( ctx.throw(500, 'Error trying to list incoming payments') } } -function incomingPaymentToBody( - walletAddress: WalletAddress, - incomingPayment: IncomingPayment, - ilpStreamCredentials?: IlpStreamCredentials -): OpenPaymentsIncomingPayment | IncomingPaymentWithPaymentMethods { - return incomingPayment.toOpenPaymentsType(walletAddress, ilpStreamCredentials) -} diff --git a/packages/backend/src/open_payments/receiver/model.test.ts b/packages/backend/src/open_payments/receiver/model.test.ts index fd0d6dc2a9..c062e1370c 100644 --- a/packages/backend/src/open_payments/receiver/model.test.ts +++ b/packages/backend/src/open_payments/receiver/model.test.ts @@ -47,7 +47,10 @@ describe('Receiver Model', (): void => { assert(streamCredentials) const receiver = new Receiver( - incomingPayment.toOpenPaymentsType(walletAddress, streamCredentials) + incomingPayment.toOpenPaymentsTypeWithMethods( + walletAddress, + streamCredentials + ) ) expect(receiver).toEqual({ @@ -87,10 +90,11 @@ describe('Receiver Model', (): void => { sharedSecret: Buffer.from('') } - const openPaymentsIncomingPayment = incomingPayment.toOpenPaymentsType( - walletAddress, - streamCredentials - ) + const openPaymentsIncomingPayment = + incomingPayment.toOpenPaymentsTypeWithMethods( + walletAddress, + streamCredentials + ) expect(() => new Receiver(openPaymentsIncomingPayment)).toThrow( 'Cannot create receiver from completed incoming payment' @@ -106,10 +110,11 @@ describe('Receiver Model', (): void => { incomingPayment.expiresAt = new Date(Date.now() - 1) const streamCredentials = streamCredentialsService.get(incomingPayment) assert(streamCredentials) - const openPaymentsIncomingPayment = incomingPayment.toOpenPaymentsType( - walletAddress, - streamCredentials - ) + const openPaymentsIncomingPayment = + incomingPayment.toOpenPaymentsTypeWithMethods( + walletAddress, + streamCredentials + ) expect(() => new Receiver(openPaymentsIncomingPayment)).toThrow( 'Cannot create receiver from expired incoming payment' @@ -126,10 +131,11 @@ describe('Receiver Model', (): void => { assert(streamCredentials) ;(streamCredentials.ilpAddress as string) = 'not base 64 encoded' - const openPaymentsIncomingPayment = incomingPayment.toOpenPaymentsType( - walletAddress, - streamCredentials - ) + const openPaymentsIncomingPayment = + incomingPayment.toOpenPaymentsTypeWithMethods( + walletAddress, + streamCredentials + ) expect(() => new Receiver(openPaymentsIncomingPayment)).toThrow( 'Invalid ILP address on ilp payment method' diff --git a/packages/backend/src/open_payments/receiver/model.ts b/packages/backend/src/open_payments/receiver/model.ts index 9de55d5ceb..bc694ffc7e 100644 --- a/packages/backend/src/open_payments/receiver/model.ts +++ b/packages/backend/src/open_payments/receiver/model.ts @@ -31,14 +31,14 @@ export class Receiver { public readonly incomingPayment: ReceiverIncomingPayment constructor(incomingPayment: OpenPaymentsIncomingPaymentWithPaymentMethod) { - if (!incomingPayment.methods.length) { - throw new Error('Missing payment method(s) on incoming payment') - } - if (incomingPayment.completed) { throw new Error('Cannot create receiver from completed incoming payment') } + if (!incomingPayment.methods.length) { + throw new Error('Missing payment method(s) on incoming payment') + } + const expiresAt = incomingPayment.expiresAt ? new Date(incomingPayment.expiresAt) : undefined diff --git a/packages/backend/src/open_payments/receiver/service.ts b/packages/backend/src/open_payments/receiver/service.ts index 47e2f049d0..e461fd6ae1 100644 --- a/packages/backend/src/open_payments/receiver/service.ts +++ b/packages/backend/src/open_payments/receiver/service.ts @@ -135,7 +135,7 @@ async function createLocalIncomingPayment( incomingPaymentOrError ) - return incomingPaymentOrError.toOpenPaymentsType( + return incomingPaymentOrError.toOpenPaymentsTypeWithMethods( walletAddress, streamCredentials ) @@ -227,7 +227,10 @@ async function getLocalIncomingPayment({ const streamCredentials = deps.streamCredentialsService.get(incomingPayment) - return incomingPayment.toOpenPaymentsType(walletAddress, streamCredentials) + return incomingPayment.toOpenPaymentsTypeWithMethods( + walletAddress, + streamCredentials + ) } async function getIncomingPaymentGrant( diff --git a/packages/backend/src/tests/outgoingPayment.ts b/packages/backend/src/tests/outgoingPayment.ts index 4da04fe8cf..3a84d23e67 100644 --- a/packages/backend/src/tests/outgoingPayment.ts +++ b/packages/backend/src/tests/outgoingPayment.ts @@ -45,7 +45,10 @@ export async function createOutgoingPayment( .spyOn(receiverService, 'get') .mockResolvedValueOnce( new Receiver( - incomingPayment.toOpenPaymentsType(walletAddress, streamCredentials) + incomingPayment.toOpenPaymentsTypeWithMethods( + walletAddress, + streamCredentials + ) ) ) } diff --git a/packages/backend/src/tests/receiver.ts b/packages/backend/src/tests/receiver.ts index ed04051974..3cd6b7a8c8 100644 --- a/packages/backend/src/tests/receiver.ts +++ b/packages/backend/src/tests/receiver.ts @@ -19,7 +19,7 @@ export async function createReceiver( const streamCredentialsService = await deps.use('streamCredentialsService') return new Receiver( - incomingPayment.toOpenPaymentsType( + incomingPayment.toOpenPaymentsTypeWithMethods( walletAddress, streamCredentialsService.get(incomingPayment)! )