Skip to content
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

662/mk/use open payments types #1024

Merged
merged 33 commits into from
Feb 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
c3ccd88
feat(open-payments): export the different IncomingPayment types
mkurapov Jan 24, 2023
3415f5f
feat(backend): use open-payment types for IncomingPayment
mkurapov Jan 24, 2023
4693a79
feat(backend): use open-payment types for Connection
mkurapov Jan 24, 2023
c633b30
feat(backend): use open-payment types for OutgoingPayment
mkurapov Jan 24, 2023
909462a
feat(backend): use open-payment types for Quote
mkurapov Jan 24, 2023
8feaacb
chore(backend): fix IncomingPayments.toOpenPaymentsType
mkurapov Jan 24, 2023
5300073
chore(open-payments): update mocks
mkurapov Jan 24, 2023
01a0ba0
chore(backend): formatting
mkurapov Jan 24, 2023
36d8b8e
chore(backend): add better typing for incomingPayment.toOpenPaymentsType
mkurapov Jan 25, 2023
d04f994
chore(backend): remove QuoteJSON
mkurapov Jan 25, 2023
379aee8
Merge branch 'main' into 662/mk/use-open-payments-types
mkurapov Jan 25, 2023
11fa542
chore(open-payments): fix mocks
mkurapov Jan 25, 2023
27cfdde
chore(backend): fix IncomingPayment types
mkurapov Jan 25, 2023
4efddb3
chore(open-payments): fix incoming payment types
mkurapov Jan 25, 2023
dc051ec
chore(backend): fix incoming payment method
mkurapov Jan 25, 2023
0fddf9e
chore(backend): make toOpenPaymentsType async
mkurapov Jan 28, 2023
ad57048
chore(backend): getUrl for OutgoingPayment
mkurapov Jan 28, 2023
d48dc0a
chore(backend): fix tests with async toOpenPaymentsType
mkurapov Jan 28, 2023
9302bad
chore(backend): return undefined instead of null
mkurapov Jan 28, 2023
5e5d4e8
chore(backend): add awaits where necessary
mkurapov Jan 28, 2023
25972d3
chore(open-payments): make getBody async
mkurapov Jan 28, 2023
4800c80
chore(backend): fix outgoingPayment model method
mkurapov Jan 28, 2023
cf95410
chore(backend): fix list tests
mkurapov Jan 28, 2023
9a967f1
chore(backend): update receiver tests
mkurapov Jan 28, 2023
e5f3c03
Merge branch 'main' into 662/mk/use-open-payments-types
mkurapov Feb 1, 2023
5733224
chore(backend): make toOpenPaymentType sync (#1056)
mkurapov Feb 1, 2023
28a34df
chore(backend): remove $formatJson where applicable
mkurapov Feb 2, 2023
5e95486
chore(backend): remove $formatJson where applicable
mkurapov Feb 2, 2023
b6d25c6
chore(backend): add test for incomingPayment model
mkurapov Feb 2, 2023
09a0c10
chore(backend): fix incomingPayment model test
mkurapov Feb 2, 2023
c079942
chore(open-payments): use suggestion
mkurapov Feb 2, 2023
c55e586
chore(backend): fix test
mkurapov Feb 2, 2023
fc311e2
chore(backend): use suggestion
mkurapov Feb 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions packages/backend/src/graphql/resolvers/receiver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import { AppServices } from '../../app'
import { initIocContainer } from '../..'
import { Config } from '../../config/app'
import { Amount, serializeAmount } from '../../open_payments/amount'
import { mockIncomingPayment, mockPaymentPointer } from 'open-payments'
import {
mockIncomingPaymentWithConnection,
mockPaymentPointer
} from 'open-payments'
import { CreateReceiverResponse } from '../generated/graphql'
import { ReceiverService } from '../../open_payments/receiver/service'
import { Receiver } from '../../open_payments/receiver/model'
Expand Down Expand Up @@ -49,7 +52,7 @@ describe('Receiver Resolver', (): void => {
incomingAmount
}): Promise<void> => {
const receiver = Receiver.fromIncomingPayment(
mockIncomingPayment({
mockIncomingPaymentWithConnection({
id: `${paymentPointer.id}/incoming-payments/${uuid()}`,
paymentPointer: paymentPointer.id,
description,
Expand Down
18 changes: 0 additions & 18 deletions packages/backend/src/open_payments/connection/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,6 @@ import { IlpAddress } from 'ilp-packet'
import { ILPStreamConnection } from 'open-payments'
import { IncomingPayment } from '../payment/incoming/model'

export type ConnectionJSON = {
id: string
ilpAddress: IlpAddress
sharedSecret: string
assetCode: string
assetScale: number
}

export abstract class ConnectionBase {
protected constructor(
public readonly ilpAddress: IlpAddress,
Expand Down Expand Up @@ -55,16 +47,6 @@ export class Connection extends ConnectionBase {
return `${this.openPaymentsUrl}/connections/${this.id}`
}

public toJSON(): ConnectionJSON {
return {
id: this.url,
ilpAddress: this.ilpAddress,
sharedSecret: base64url(this.sharedSecret),
assetCode: this.assetCode,
assetScale: this.assetScale
}
}

public toOpenPaymentsType(): ILPStreamConnection {
return {
id: this.url,
Expand Down
2 changes: 1 addition & 1 deletion packages/backend/src/open_payments/connection/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,5 @@ async function getConnection(

const connection = deps.connectionService.get(incomingPayment)
if (!connection) return ctx.throw(404)
ctx.body = connection.toJSON()
ctx.body = connection.toOpenPaymentsType()
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe('Connection Service', (): void => {
expect(connection.url).toEqual(
`${Config.openPaymentsUrl}/connections/${incomingPayment.connectionId}`
)
expect(connection.toJSON()).toEqual({
expect(connection.toOpenPaymentsType()).toEqual({
id: connection.url,
ilpAddress: connection.ilpAddress,
sharedSecret: base64url(connection.sharedSecret || ''),
Expand Down
127 changes: 127 additions & 0 deletions packages/backend/src/open_payments/payment/incoming/model.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import { IocContract } from '@adonisjs/fold'
import { createTestApp, TestContainer } from '../../../tests/app'
import { Config, IAppConfig } from '../../../config/app'
import { initIocContainer } from '../../..'
import { AppServices } from '../../../app'
import { createIncomingPayment } from '../../../tests/incomingPayment'
import { createPaymentPointer } from '../../../tests/paymentPointer'
import { truncateTables } from '../../../tests/tableManager'
import { Connection } from '../../connection/model'
import { serializeAmount } from '../../amount'
import { IlpAddress } from 'ilp-packet'
import { IncomingPayment } from './model'

describe('Incoming Payment Model', (): void => {
let deps: IocContract<AppServices>
let appContainer: TestContainer
let config: IAppConfig

beforeAll(async (): Promise<void> => {
deps = initIocContainer(Config)
appContainer = await createTestApp(deps)
config = await deps.use('config')
})

afterEach(async (): Promise<void> => {
jest.useRealTimers()
await truncateTables(appContainer.knex)
})

afterAll(async (): Promise<void> => {
await appContainer.shutdown()
})

describe('toOpenPaymentsType', () => {
test('returns incoming payment without connection provided', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if there was a single test.each to make clear that output is the same other than ilpStreamConnection?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking this, but having a test case where the connection var in the test.each case was either

const connection = `${config.openPaymentsUrl}/connections/${incomingPayment.connectionId}` 

or

const connection = Connection.fromPayment({
        payment: incomingPayment,
        openPaymentsUrl: config.openPaymentsUrl,
        credentials: {
          ilpAddress: 'test.ilp' as IlpAddress,
          sharedSecret: Buffer.from('')
        }
      })

didn't feel nice

const paymentPointer = await createPaymentPointer(deps)
const incomingPayment = await createIncomingPayment(deps, {
paymentPointerId: paymentPointer.id,
description: 'my payment'
})

expect(incomingPayment.toOpenPaymentsType(paymentPointer)).toEqual({
id: `${paymentPointer.url}${IncomingPayment.urlPath}/${incomingPayment.id}`,
paymentPointer: paymentPointer.url,
completed: incomingPayment.completed,
receivedAmount: serializeAmount(incomingPayment.receivedAmount),
incomingAmount: incomingPayment.incomingAmount
? serializeAmount(incomingPayment.incomingAmount)
: undefined,
expiresAt: incomingPayment.expiresAt.toISOString(),
description: incomingPayment.description ?? undefined,
externalRef: incomingPayment.externalRef ?? undefined,
updatedAt: incomingPayment.updatedAt.toISOString(),
createdAt: incomingPayment.createdAt.toISOString()
})
})

test('returns incoming payment with connection as string', async () => {
const paymentPointer = await createPaymentPointer(deps)
const incomingPayment = await createIncomingPayment(deps, {
paymentPointerId: paymentPointer.id,
description: 'my payment'
})

const connection = `${config.openPaymentsUrl}/connections/${incomingPayment.connectionId}`

expect(
incomingPayment.toOpenPaymentsType(paymentPointer, connection)
).toEqual({
id: `${paymentPointer.url}${IncomingPayment.urlPath}/${incomingPayment.id}`,
paymentPointer: paymentPointer.url,
completed: incomingPayment.completed,
receivedAmount: serializeAmount(incomingPayment.receivedAmount),
incomingAmount: incomingPayment.incomingAmount
? serializeAmount(incomingPayment.incomingAmount)
: undefined,
expiresAt: incomingPayment.expiresAt.toISOString(),
description: incomingPayment.description ?? undefined,
externalRef: incomingPayment.externalRef ?? undefined,
updatedAt: incomingPayment.updatedAt.toISOString(),
createdAt: incomingPayment.createdAt.toISOString(),
ilpStreamConnection: connection
})
})

test('returns incoming payment with connection as object', async () => {
const paymentPointer = await createPaymentPointer(deps)
const incomingPayment = await createIncomingPayment(deps, {
paymentPointerId: paymentPointer.id,
description: 'my payment'
})

const connection = Connection.fromPayment({
payment: incomingPayment,
openPaymentsUrl: config.openPaymentsUrl,
credentials: {
ilpAddress: 'test.ilp' as IlpAddress,
sharedSecret: Buffer.from('')
}
})

expect(
incomingPayment.toOpenPaymentsType(paymentPointer, connection)
).toEqual({
id: `${paymentPointer.url}${IncomingPayment.urlPath}/${incomingPayment.id}`,
paymentPointer: paymentPointer.url,
completed: incomingPayment.completed,
receivedAmount: serializeAmount(incomingPayment.receivedAmount),
incomingAmount: incomingPayment.incomingAmount
? serializeAmount(incomingPayment.incomingAmount)
: undefined,
expiresAt: incomingPayment.expiresAt.toISOString(),
description: incomingPayment.description ?? undefined,
externalRef: incomingPayment.externalRef ?? undefined,
updatedAt: incomingPayment.updatedAt.toISOString(),
createdAt: incomingPayment.createdAt.toISOString(),
ilpStreamConnection: {
id: `${config.openPaymentsUrl}/connections/${incomingPayment.connectionId}`,
ilpAddress: 'test.ilp',
sharedSecret: expect.any(String),
assetCode: incomingPayment.asset.code,
assetScale: incomingPayment.asset.scale
}
})
})
})
})
112 changes: 54 additions & 58 deletions packages/backend/src/open_payments/payment/incoming/model.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Model, ModelOptions, Pojo, QueryContext } from 'objection'
import { Model, ModelOptions, QueryContext } from 'objection'
import { v4 as uuid } from 'uuid'

import { Amount, AmountJSON, serializeAmount } from '../../amount'
import { Connection, ConnectionJSON } from '../../connection/model'
import { Connection } from '../../connection/model'
import {
PaymentPointer,
PaymentPointerSubresource
Expand All @@ -11,7 +11,11 @@ import { Asset } from '../../../asset/model'
import { LiquidityAccount, OnCreditOptions } from '../../../accounting/service'
import { ConnectorAccount } from '../../../connector/core/rafiki'
import { WebhookEvent } from '../../../webhook/model'
import { IncomingPayment as OpenPaymentsIncomingPayment } from 'open-payments'
import {
IncomingPayment as OpenPaymentsIncomingPayment,
IncomingPaymentWithConnection as OpenPaymentsIncomingPaymentWithConnection,
IncomingPaymentWithConnectionUrl as OpenPaymentsIncomingPaymentWithConnectionUrl
} from 'open-payments'

export enum IncomingPaymentEventType {
IncomingPaymentExpired = 'incoming_payment.expired',
Expand Down Expand Up @@ -80,7 +84,6 @@ export class IncomingPayment
}
}

public paymentPointer!: PaymentPointer
Copy link
Contributor Author

@mkurapov mkurapov Jan 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no non-null assertion since we can start removing withGraphFetched('paymentPointer') from the DB calls

public description?: string
public expiresAt!: Date
public state!: IncomingPaymentState
Expand Down Expand Up @@ -127,8 +130,8 @@ export class IncomingPayment
this.receivedAmountValue = amount.value
}

public get url(): string {
return `${this.paymentPointer.url}${IncomingPayment.urlPath}/${this.id}`
public getUrl(paymentPointer: PaymentPointer): string {
return `${paymentPointer.url}${IncomingPayment.urlPath}/${this.id}`
}

public async onCredit({
Expand Down Expand Up @@ -211,67 +214,60 @@ export class IncomingPayment
}
}

$formatJson(json: Pojo): Pojo {
json = super.$formatJson(json)
const payment: Pojo = {
id: json.id,
receivedAmount: {
...json.receivedAmount,
value: json.receivedAmount.value.toString()
},
completed: json.completed,
createdAt: json.createdAt,
updatedAt: json.updatedAt,
expiresAt: json.expiresAt.toISOString()
}
if (json.incomingAmount) {
payment.incomingAmount = {
...json.incomingAmount,
value: json.incomingAmount.value.toString()
}
}
if (json.description) {
payment.description = json.description
}
if (json.externalRef) {
payment.externalRef = json.externalRef
}
return payment
}

public toOpenPaymentsType({
ilpStreamConnection
}: {
public toOpenPaymentsType(
paymentPointer: PaymentPointer
): OpenPaymentsIncomingPayment
public toOpenPaymentsType(
paymentPointer: PaymentPointer,
ilpStreamConnection: Connection
}): OpenPaymentsIncomingPayment {
return {
id: this.url,
paymentPointer: this.paymentPointer.url,
): OpenPaymentsIncomingPaymentWithConnection
public toOpenPaymentsType(
paymentPointer: PaymentPointer,
ilpStreamConnection: string
): OpenPaymentsIncomingPaymentWithConnectionUrl
public toOpenPaymentsType(
paymentPointer: PaymentPointer,
ilpStreamConnection?: Connection | string
):
| OpenPaymentsIncomingPaymentWithConnection
| OpenPaymentsIncomingPaymentWithConnectionUrl

public toOpenPaymentsType(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we added some model.test.tss at least to test toOpenPaymentsType?
Or are existing response body checks in routes tests good enough?

paymentPointer: PaymentPointer,
ilpStreamConnection?: Connection | string
):
| OpenPaymentsIncomingPayment
| OpenPaymentsIncomingPaymentWithConnection
| OpenPaymentsIncomingPaymentWithConnectionUrl {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to suggest adding an abstract toOpenPaymentsType to PaymentPointerSubresource but this one for incoming payments would complicate it

export abstract class PaymentPointerSubresource extends BaseModel {

const baseIncomingPayment: OpenPaymentsIncomingPayment = {
id: this.getUrl(paymentPointer),
paymentPointer: paymentPointer.url,
incomingAmount: this.incomingAmount
? serializeAmount(this.incomingAmount)
: undefined,
receivedAmount: serializeAmount(this.receivedAmount),
completed: this.completed,
description: this.description ?? undefined,
externalRef: this.externalRef ?? undefined,
createdAt: this.createdAt.toISOString(),
updatedAt: this.updatedAt.toISOString(),
expiresAt: this.expiresAt.toISOString(),
expiresAt: this.expiresAt.toISOString()
}

if (!ilpStreamConnection) {
return baseIncomingPayment
}

if (typeof ilpStreamConnection === 'string') {
return {
...baseIncomingPayment,
ilpStreamConnection
}
}

return {
...baseIncomingPayment,
ilpStreamConnection: ilpStreamConnection.toOpenPaymentsType()
}
}
}

// TODO: disallow undefined
// https://github.com/interledger/rafiki/issues/594
export type IncomingPaymentJSON = {
id: string
paymentPointer: string
incomingAmount?: AmountJSON
receivedAmount: AmountJSON
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wilsonianb Keeping AmountJSON over OpenPaymentsAmount makes sense right? The Amount interface is a more generic thing we use across the code, not necessarily being an "open payments" concept

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I'm good with keeping amount values as strings in open-payments and leaving it to callers to (de)serialize.

completed: boolean
description?: string
externalRef?: string
createdAt: string
updatedAt: string
expiresAt?: string
ilpStreamConnection?: ConnectionJSON | string
}
Loading