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.

Sign up for GitHub

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

Jump to bottom

chore(backend): make toOpenPaymentType sync #1056

Merged
merged 5 commits into from
Feb 1, 2023
Merged

chore(backend): make toOpenPaymentType sync #1056

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c20355e
chore(backend): make toOpenPaymentType sync
mkurapov Jan 31, 2023
88c13a4
Merge branch '662/mk/use-open-payments-types' into 662/mk/use-open-pa…
mkurapov Feb 1, 2023
d3f2557
chore(backend): make quote.toOpenPaymentsType sync
mkurapov Feb 1, 2023
b858dc2
chore(backend): convert tests to sync
mkurapov Feb 1, 2023
06e23ea
chore(backend): update tests
mkurapov Feb 1, 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
41 changes: 19 additions & 22 deletions packages/backend/src/open_payments/payment/incoming/model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,17 +130,10 @@ export class IncomingPayment
this.receivedAmountValue = amount.value
}

public async getUrl(fetchedPaymentPointer?: PaymentPointer): Promise<string> {
const paymentPointer =
fetchedPaymentPointer || (await this.getPaymentPointer())

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

public async getPaymentPointer(): Promise<PaymentPointer> {
return this.paymentPointer ?? (await this.$relatedQuery('paymentPointer'))
}

public async onCredit({
totalReceived
}: OnCreditOptions): Promise<IncomingPayment> {
Expand Down Expand Up @@ -249,29 +242,33 @@ export class IncomingPayment
return payment
}

public async toOpenPaymentsType(): Promise<OpenPaymentsIncomingPayment>
public async toOpenPaymentsType(
public toOpenPaymentsType(
paymentPointer: PaymentPointer
): OpenPaymentsIncomingPayment
public toOpenPaymentsType(
paymentPointer: PaymentPointer,
ilpStreamConnection: Connection
): Promise<OpenPaymentsIncomingPaymentWithConnection>
public async toOpenPaymentsType(
): OpenPaymentsIncomingPaymentWithConnection
public toOpenPaymentsType(
paymentPointer: PaymentPointer,
ilpStreamConnection: string
): Promise<OpenPaymentsIncomingPaymentWithConnectionUrl>
public async toOpenPaymentsType(
): OpenPaymentsIncomingPaymentWithConnectionUrl
public toOpenPaymentsType(
paymentPointer: PaymentPointer,
ilpStreamConnection?: Connection | string
): Promise<
):
| OpenPaymentsIncomingPaymentWithConnection
| OpenPaymentsIncomingPaymentWithConnectionUrl
>
public async toOpenPaymentsType(

public toOpenPaymentsType(
paymentPointer: PaymentPointer,
ilpStreamConnection?: Connection | string
): Promise<
):
| OpenPaymentsIncomingPayment
| OpenPaymentsIncomingPaymentWithConnection
| OpenPaymentsIncomingPaymentWithConnectionUrl
> {
const paymentPointer = await this.getPaymentPointer()
| OpenPaymentsIncomingPaymentWithConnectionUrl {
const baseIncomingPayment: OpenPaymentsIncomingPayment = {
id: await this.getUrl(paymentPointer),
id: this.getUrl(paymentPointer),
paymentPointer: paymentPointer.url,
incomingAmount: this.incomingAmount
? serializeAmount(this.incomingAmount)
Expand Down
6 changes: 3 additions & 3 deletions packages/backend/src/open_payments/payment/incoming/routes.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ describe('Incoming Payment Routes', (): void => {
externalRef
}),
get: (ctx) => incomingPaymentRoutes.get(ctx),
getBody: async (incomingPayment, list) => {
getBody: (incomingPayment, list) => {
return {
id: await incomingPayment.getUrl(),
id: incomingPayment.getUrl(paymentPointer),
paymentPointer: paymentPointer.url,
completed: false,
incomingAmount:
Expand Down Expand Up @@ -265,7 +265,7 @@ describe('Incoming Payment Routes', (): void => {
await expect(incomingPaymentRoutes.complete(ctx)).resolves.toBeUndefined()
expect(ctx.response).toSatisfyApiSpec()
expect(ctx.body).toEqual({
id: await incomingPayment.getUrl(),
id: incomingPayment.getUrl(paymentPointer),
paymentPointer: paymentPointer.url,
incomingAmount: {
value: '123',
Expand Down
30 changes: 20 additions & 10 deletions packages/backend/src/open_payments/payment/incoming/routes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
IncomingPaymentWithConnection as OpenPaymentsIncomingPaymentWithConnection,
IncomingPaymentWithConnectionUrl as OpenPaymentsIncomingPaymentWithConnectionUrl
} from 'open-payments'
import { PaymentPointer } from '../../payment_pointer/model'

// Don't allow creating an incoming payment too far out. Incoming payments with no payments before they expire are cleaned up, since incoming payments creation is unauthenticated.
// TODO what is a good default value for this?
Expand Down Expand Up @@ -75,7 +76,11 @@ async function getIncomingPayment(
}
if (!incomingPayment) return ctx.throw(404)
const connection = deps.connectionService.get(incomingPayment)
ctx.body = await incomingPaymentToBody(incomingPayment, connection)
ctx.body = incomingPaymentToBody(
Copy link
Contributor

Choose a reason for hiding this comment

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

Could these *toBody calls be replaced with calling *.toOpenPaymentsType directly?

Copy link
Contributor

Choose a reason for hiding this comment

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

🤔
...or we do the *withPaymentPointer subresource type idea and make *toBody the limited places where we're doing the *.paymentPointer = ctx.paymentPointer assignment.

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 think I like it.

I bet this would even let us remove some .withGraphFetched['paymentPointer']s in subresource service queries.

It's a little more straightforward than the alternatives, and will indeed allow us to remove the withGraphFetched calls. I can do that as a separate follow-up PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

toBody is a little redundant, I agree, but the reason I like toBody is it just keeps it standard across all of the routes, and if it were ever to change, we'd just need to change a single place.

"*withPaymentPointer subresource type idea"
I'm still not convinced about this idea, since having the non-null asserted paymentPointer/having throws or undefined in the model itself might end up having someone call it and open up doors for bugs... thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't feeling too strongly about toBody or *withPaymentPointer.
The latter would let getUrl be a url getter but it'd probably only be called within toOpenPaymentType.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess url getter would also be used in tests...

ctx.paymentPointer,
incomingPayment,
connection
)
}

export type CreateBody = {
Expand Down Expand Up @@ -116,7 +121,11 @@ async function createIncomingPayment(

ctx.status = 201
const connection = deps.connectionService.get(incomingPaymentOrError)
ctx.body = await incomingPaymentToBody(incomingPaymentOrError, connection)
ctx.body = incomingPaymentToBody(
ctx.paymentPointer,
incomingPaymentOrError,
connection
)
}

async function completeIncomingPayment(
Expand All @@ -138,7 +147,7 @@ async function completeIncomingPayment(
errorToMessage[incomingPaymentOrError]
)
}
ctx.body = await incomingPaymentToBody(incomingPaymentOrError)
ctx.body = incomingPaymentToBody(ctx.paymentPointer, incomingPaymentOrError)
}

async function listIncomingPayments(
Expand All @@ -149,8 +158,9 @@ async function listIncomingPayments(
await listSubresource({
ctx,
getPaymentPointerPage: deps.incomingPaymentService.getPaymentPointerPage,
toBody: async (payment) =>
await incomingPaymentToBody(
toBody: (payment) =>
incomingPaymentToBody(
ctx.paymentPointer,
payment,
deps.connectionService.getUrl(payment)
)
Expand All @@ -159,13 +169,13 @@ async function listIncomingPayments(
ctx.throw(500, 'Error trying to list incoming payments')
}
}
async function incomingPaymentToBody(
function incomingPaymentToBody(
paymentPointer: PaymentPointer,
incomingPayment: IncomingPayment,
ilpStreamConnection?: Connection | string
): Promise<
):
| OpenPaymentsIncomingPayment
| OpenPaymentsIncomingPaymentWithConnection
| OpenPaymentsIncomingPaymentWithConnectionUrl
> {
return incomingPayment.toOpenPaymentsType(ilpStreamConnection)
| OpenPaymentsIncomingPaymentWithConnectionUrl {
return incomingPayment.toOpenPaymentsType(paymentPointer, ilpStreamConnection)
}
15 changes: 6 additions & 9 deletions packages/backend/src/open_payments/payment/outgoing/model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,7 @@ export class OutgoingPayment
return this.quote.assetId
}

public async getUrl(fetchedPaymentPointer?: PaymentPointer): Promise<string> {
const paymentPointer =
fetchedPaymentPointer || (await this.getPaymentPointer())

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

Expand Down Expand Up @@ -188,13 +185,13 @@ export class OutgoingPayment
return this.paymentPointer ?? (await this.$relatedQuery('paymentPointer'))
}

public async toOpenPaymentsType(): Promise<OpenPaymentsOutgoingPayment> {
const paymentPointer = await this.getPaymentPointer()

public toOpenPaymentsType(
paymentPointer: PaymentPointer
): OpenPaymentsOutgoingPayment {
return {
id: await this.getUrl(paymentPointer),
id: this.getUrl(paymentPointer),
paymentPointer: paymentPointer.url,
quoteId: (await this.quote?.getUrl()) ?? undefined,
quoteId: this.quote?.getUrl(paymentPointer) ?? undefined,
receiveAmount: serializeAmount(this.receiveAmount),
sendAmount: serializeAmount(this.sendAmount),
sentAmount: serializeAmount(this.sentAmount),
Expand Down
4 changes: 2 additions & 2 deletions packages/backend/src/open_payments/payment/outgoing/routes.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,12 @@ describe('Outgoing Payment Routes', (): void => {
return outgoingPayment
},
get: (ctx) => outgoingPaymentRoutes.get(ctx),
getBody: async (outgoingPayment) => {
getBody: (outgoingPayment) => {
return {
id: `${paymentPointer.url}/outgoing-payments/${outgoingPayment.id}`,
paymentPointer: paymentPointer.url,
receiver: outgoingPayment.receiver,
quoteId: await outgoingPayment.quote.getUrl(),
quoteId: outgoingPayment.quote.getUrl(paymentPointer),
sendAmount: serializeAmount(outgoingPayment.sendAmount),
sentAmount: serializeAmount(outgoingPayment.sentAmount),
receiveAmount: serializeAmount(outgoingPayment.receiveAmount),
Expand Down
14 changes: 8 additions & 6 deletions packages/backend/src/open_payments/payment/outgoing/routes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
AccessAction,
OutgoingPayment as OpenPaymentsOutgoingPayment
} from 'open-payments'
import { PaymentPointer } from '../../payment_pointer/model'

interface ServiceDependencies {
config: IAppConfig
Expand Down Expand Up @@ -52,7 +53,7 @@ async function getOutgoingPayment(
ctx.throw(500, 'Error trying to get outgoing payment')
}
if (!outgoingPayment) return ctx.throw(404)
ctx.body = await outgoingPaymentToBody(outgoingPayment)
ctx.body = outgoingPaymentToBody(ctx.paymentPointer, outgoingPayment)
}

export type CreateBody = {
Expand Down Expand Up @@ -86,7 +87,7 @@ async function createOutgoingPayment(
return ctx.throw(errorToCode[paymentOrErr], errorToMessage[paymentOrErr])
}
ctx.status = 201
ctx.body = await outgoingPaymentToBody(paymentOrErr)
ctx.body = outgoingPaymentToBody(ctx.paymentPointer, paymentOrErr)
}

async function listOutgoingPayments(
Expand All @@ -97,15 +98,16 @@ async function listOutgoingPayments(
await listSubresource({
ctx,
getPaymentPointerPage: deps.outgoingPaymentService.getPaymentPointerPage,
toBody: async (payment) => await outgoingPaymentToBody(payment)
toBody: (payment) => outgoingPaymentToBody(ctx.paymentPointer, payment)
})
} catch (_) {
ctx.throw(500, 'Error trying to list outgoing payments')
}
}

async function outgoingPaymentToBody(
function outgoingPaymentToBody(
paymentPointer: PaymentPointer,
outgoingPayment: OutgoingPayment
): Promise<OpenPaymentsOutgoingPayment> {
return outgoingPayment.toOpenPaymentsType()
): OpenPaymentsOutgoingPayment {
return outgoingPayment.toOpenPaymentsType(paymentPointer)
}
15 changes: 8 additions & 7 deletions packages/backend/src/open_payments/payment/outgoing/service.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,11 +245,10 @@ describe('OutgoingPaymentService', (): void => {

beforeEach(async (): Promise<void> => {
const { id: sendAssetId } = await createAsset(deps, asset)
paymentPointerId = (
await createPaymentPointer(deps, {
assetId: sendAssetId
})
).id
const paymentPointer = await createPaymentPointer(deps, {
assetId: sendAssetId
})
paymentPointerId = paymentPointer.id
const { id: destinationAssetId } = await createAsset(deps, destinationAsset)
receiverPaymentPointer = await createPaymentPointer(deps, {
assetId: destinationAssetId,
Expand All @@ -266,7 +265,7 @@ describe('OutgoingPaymentService', (): void => {
incomingPayment = await createIncomingPayment(deps, {
paymentPointerId: receiverPaymentPointer.id
})
receiver = await incomingPayment.getUrl()
receiver = incomingPayment.getUrl(receiverPaymentPointer)

amtDelivered = BigInt(0)
})
Expand Down Expand Up @@ -855,12 +854,14 @@ describe('OutgoingPaymentService', (): void => {
assetScale: receiverPaymentPointer.asset.scale
}
})

const fetchedReceiver = connectionService.getUrl(incomingPayment)
assert.ok(fetchedReceiver)
assert.ok(incomingPayment.paymentPointer)
const paymentId = await setup({
receiver: toConnection
? fetchedReceiver
: await incomingPayment.getUrl(),
: incomingPayment.getUrl(incomingPayment.paymentPointer),
receiveAmount
})

Expand Down
14 changes: 6 additions & 8 deletions packages/backend/src/open_payments/payment_pointer/model.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ type RouteTestsOptions<M> = Omit<
> & {
getPaymentPointer: () => Promise<PaymentPointer>
get: (ctx: ReadContext) => Promise<void>
getBody: (model: M, list?: boolean) => Promise<Record<string, unknown>>
getBody: (model: M, list?: boolean) => Record<string, unknown>
list?: (ctx: ListContext) => Promise<void>
urlPath: string
}
Expand Down Expand Up @@ -230,7 +230,7 @@ export const getRouteTests = <M extends PaymentPointerSubresource>({
expect(ctx.response).toSatisfyApiSpec()
}
expect(ctx.body).toEqual({
result: expectedMatch ? [await getBody(expectedMatch, true)] : [],
result: expectedMatch ? [getBody(expectedMatch, true)] : [],
pagination: {
hasPreviousPage: false,
hasNextPage: false,
Expand Down Expand Up @@ -261,7 +261,7 @@ export const getRouteTests = <M extends PaymentPointerSubresource>({
if (expectedMatch) {
await expect(get(ctx)).resolves.toBeUndefined()
expect(ctx.response).toSatisfyApiSpec()
expect(ctx.body).toEqual(await getBody(expectedMatch))
expect(ctx.body).toEqual(getBody(expectedMatch))
} else {
await expect(get(ctx)).rejects.toMatchObject({
status: 404,
Expand Down Expand Up @@ -319,11 +319,9 @@ export const getRouteTests = <M extends PaymentPointerSubresource>({
expect(ctx.response).toSatisfyApiSpec()
expect(ctx.body).toEqual({
pagination,
result: await Promise.all(
models
.slice(startIndex, endIndex + 1)
.map(async (model) => await getBody(model, true))
)
result: models
.slice(startIndex, endIndex + 1)
.map((model) => getBody(model, true))
})
}
)
Expand Down
6 changes: 2 additions & 4 deletions packages/backend/src/open_payments/payment_pointer/routes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@ export async function getPaymentPointer(
interface ListSubresourceOptions<M extends PaymentPointerSubresource> {
ctx: ListContext
getPaymentPointerPage: PaymentPointerSubresourceService<M>['getPaymentPointerPage']
toBody:
| ((model: M) => Record<string, unknown>)
| ((model: M) => Promise<Record<string, unknown>>)
toBody: (model: M) => Record<string, unknown>
}

export const listSubresource = async <M extends PaymentPointerSubresource>({
Expand All @@ -68,7 +66,7 @@ export const listSubresource = async <M extends PaymentPointerSubresource>({
)
const result = {
pagination: pageInfo,
result: await Promise.all(page.map(async (item: M) => await toBody(item)))
result: page.map((item: M) => toBody(item))
}
ctx.body = result
}
15 changes: 3 additions & 12 deletions packages/backend/src/open_payments/quote/model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,7 @@ export class Quote extends PaymentPointerSubresource {

private sendAmountValue!: bigint

public async getPaymentPointer(): Promise<PaymentPointer> {
return this.paymentPointer ?? (await this.$relatedQuery('paymentPointer'))
}

public async getUrl(fetchedPaymentPointer?: PaymentPointer): Promise<string> {
const paymentPointer =
fetchedPaymentPointer || (await this.getPaymentPointer())

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

Expand Down Expand Up @@ -164,11 +157,9 @@ export class Quote extends PaymentPointerSubresource {
}
}

public async toOpenPaymentsType(): Promise<OpenPaymentsQuote> {
const paymentPointer = await this.getPaymentPointer()

public toOpenPaymentsType(paymentPointer: PaymentPointer): OpenPaymentsQuote {
return {
id: await this.getUrl(paymentPointer),
id: this.getUrl(paymentPointer),
paymentPointer: paymentPointer.url,
receiveAmount: serializeAmount(this.receiveAmount),
sendAmount: serializeAmount(this.sendAmount),
Expand Down
2 changes: 1 addition & 1 deletion packages/backend/src/open_payments/quote/routes.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe('Quote Routes', (): void => {
client
}),
get: (ctx) => quoteRoutes.get(ctx),
getBody: async (quote) => {
getBody: (quote) => {
return {
id: `${paymentPointer.url}/quotes/${quote.id}`,
paymentPointer: paymentPointer.url,
Expand Down
Loading