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

663/mk/open payments validate #679

Merged
merged 40 commits into from
Oct 25, 2022
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
d3470c4
feat(open-payments-client): initial POC of open-payments-client
mkurapov Oct 12, 2022
7f968c3
feat(open-payments-client): cleanup
mkurapov Oct 12, 2022
79e7cd7
feat(open-payments): remove open-payments-client folder
mkurapov Oct 13, 2022
32e2a31
feat(open-payments): make open-payments folder and use script for typ…
mkurapov Oct 13, 2022
050ebef
feat(open-payments): fix rootDir
mkurapov Oct 13, 2022
9893029
feat(open-payments): downgrading generation lib, updating how script …
mkurapov Oct 14, 2022
c15fdbc
feat(open-payments): update default config usage
mkurapov Oct 14, 2022
130e310
feat(open-payments): update pnpm-lock.yaml
mkurapov Oct 14, 2022
e067011
feat(open-payments): update pnpm-lock.yaml
mkurapov Oct 14, 2022
dc28878
feat(open-payments): adding tests
mkurapov Oct 14, 2022
39b561b
feat(open-payments): update pnpm-lock.yaml
mkurapov Oct 14, 2022
9393e47
feat(open-payments): simplify test
mkurapov Oct 14, 2022
3c3a0b7
feat(open-payments): updating workflows
mkurapov Oct 14, 2022
120dcae
feat(open-payments): pin the open api spec to the most recent commit
mkurapov Oct 14, 2022
80d9856
feat(open-payments): adding openapi validation
mkurapov Oct 18, 2022
218f1f0
feat(open-payments): adding openapi validation
mkurapov Oct 18, 2022
d7c203c
feat(open-payments): adding tests
mkurapov Oct 18, 2022
dfc07cb
feat(open-payments): correcting test
mkurapov Oct 18, 2022
d59c2ea
Merge branch 'main' into 663/mk/open-payments-validate
mkurapov Oct 21, 2022
43335f6
feat(open-payments): use updated RS spec
mkurapov Oct 21, 2022
723de2f
Merge branch '663/mk/op-client-1' into 663/mk/open-payments-validate
mkurapov Oct 21, 2022
02b8f9f
Merge branch 'main' into 663/mk/open-payments-validate
mkurapov Oct 24, 2022
f8c1846
feat(open-payments): build open api package on build step
mkurapov Oct 24, 2022
f560c79
Merge branch 'main' into 663/mk/op-client-1
mkurapov Oct 24, 2022
e11ec65
feat(open-payments): building open-api package during workflow
mkurapov Oct 24, 2022
0dad54d
Revert "feat(open-payments): building open-api package during workflow"
mkurapov Oct 24, 2022
000fc9b
Merge branch '663/mk/op-client-1' into 663/mk/open-payments-validate
mkurapov Oct 24, 2022
25d2617
feat(open-payments): building open-api package during workflow
mkurapov Oct 24, 2022
f734dff
Merge branch 'main' into 663/mk/open-payments-validate
mkurapov Oct 24, 2022
4552188
feat(open-payments): update naming
mkurapov Oct 24, 2022
85db136
feat(open-payments): instantiate validator functions once
mkurapov Oct 25, 2022
dc4aa0f
chore(openapi): add docs for usage
mkurapov Oct 25, 2022
c927291
chore(openapi): update docs
mkurapov Oct 25, 2022
3ed946b
chore(openapi): prettify docs
mkurapov Oct 25, 2022
347075a
chore(openapi): update docs
mkurapov Oct 25, 2022
90bc6b0
feat(open-payments): adding logger as dependency
mkurapov Oct 25, 2022
3129f2a
feat(open-payments): updating logging & error logic
mkurapov Oct 25, 2022
05b6cfb
feat(open-payments): remove unnecessary imports
mkurapov Oct 25, 2022
f1d2412
Merge branch 'main' into 663/mk/open-payments-validate
mkurapov Oct 25, 2022
f6eb6a8
feat(open-payments): update error handling & test
mkurapov Oct 25, 2022
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
1 change: 1 addition & 0 deletions .github/workflows/lint_test_build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ jobs:
steps:
- uses: actions/checkout@v2
- uses: ./.github/workflows/rafiki/env-setup
- run: pnpm --filter openapi build
- run: pnpm --filter open-payments test

build:
Expand Down
6 changes: 4 additions & 2 deletions packages/open-payments/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
"dist/**/*"
],
"scripts": {
"build": "pnpm clean && tsc --build tsconfig.json",
"build:deps": "pnpm --filter openapi build",
"build": "pnpm build:deps && pnpm clean && tsc --build tsconfig.json",
"clean": "rm -fr dist/",
"generate:types": "npx ts-node scripts/generate-types.ts",
"prepack": "pnpm build",
Expand All @@ -21,6 +22,7 @@
"typescript": "^4.3.0"
},
"dependencies": {
"axios": "^1.1.2"
"axios": "^1.1.2",
"openapi": "workspace:../openapi"
}
}
67 changes: 0 additions & 67 deletions packages/open-payments/src/client.ts

This file was deleted.

37 changes: 37 additions & 0 deletions packages/open-payments/src/client/ilp-stream-connection.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/* eslint-disable @typescript-eslint/no-empty-function */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure how important these tests in particular are, but I thought it was worth making sure we are adding the correct path for the response validator

import { getILPStreamConnection } from './ilp-stream-connection'
import { OpenAPI, HttpMethod, createOpenAPI } from 'openapi'
import { createAxiosInstance } from './requests'
import config from '../config'

jest.mock('./requests', () => ({
...jest.requireActual('./requests'),
get: jest.fn()
}))

describe('ilp-stream-connection', (): void => {
let openApi: OpenAPI

beforeAll(async () => {
openApi = await createOpenAPI(config.OPEN_PAYMENTS_OPEN_API_URL)
})

const axiosInstance = createAxiosInstance()

describe('getILPStreamConnection', (): void => {
test('calls createResponseValidator properly', async (): Promise<void> => {
const createResponseValidatorSpy = jest.spyOn(
openApi,
'createResponseValidator'
)

await getILPStreamConnection(axiosInstance, openApi, {
url: 'http://localhost:1000/incoming-payment'
})
expect(createResponseValidatorSpy).toHaveBeenCalledWith({
path: '/connections/{id}',
method: HttpMethod.GET
})
})
})
})
19 changes: 19 additions & 0 deletions packages/open-payments/src/client/ilp-stream-connection.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { AxiosInstance } from 'axios'
import { OpenAPI, HttpMethod } from 'openapi'
import { getPath, ILPStreamConnection } from '../types'
import { GetArgs, get } from './requests'

export const getILPStreamConnection = async (
axios: AxiosInstance,
openApi: OpenAPI,
args: GetArgs
): Promise<ILPStreamConnection> => {
return get(
axios,
args,
openApi.createResponseValidator({
path: getPath('/connections/{id}'),
method: HttpMethod.GET
})
)
}
37 changes: 37 additions & 0 deletions packages/open-payments/src/client/incoming-payment.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/* eslint-disable @typescript-eslint/no-empty-function */
import { getIncomingPayment } from './incoming-payment'
import { OpenAPI, HttpMethod, createOpenAPI } from 'openapi'
import { createAxiosInstance } from './requests'
import config from '../config'

jest.mock('./requests', () => ({
...jest.requireActual('./requests'),
get: jest.fn()
}))

describe('incoming-payment', (): void => {
let openApi: OpenAPI

beforeAll(async () => {
openApi = await createOpenAPI(config.OPEN_PAYMENTS_OPEN_API_URL)
})

const axiosInstance = createAxiosInstance()

describe('getIncomingPayment', (): void => {
test('calls createResponseValidator properly', async (): Promise<void> => {
const createResponseValidatorSpy = jest.spyOn(
openApi,
'createResponseValidator'
)

await getIncomingPayment(axiosInstance, openApi, {
url: 'http://localhost:1000/incoming-payment'
})
expect(createResponseValidatorSpy).toHaveBeenCalledWith({
path: '/incoming-payments/{id}',
method: HttpMethod.GET
})
})
})
})
19 changes: 19 additions & 0 deletions packages/open-payments/src/client/incoming-payment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { AxiosInstance } from 'axios'
import { OpenAPI, HttpMethod } from 'openapi'
import { IncomingPayment, getPath } from '../types'
import { GetArgs, get } from './requests'

export const getIncomingPayment = async (
axios: AxiosInstance,
openApi: OpenAPI,
args: GetArgs
): Promise<IncomingPayment> => {
return get(
axios,
args,
openApi.createResponseValidator({
Copy link
Contributor

Choose a reason for hiding this comment

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

The response validator library uses Ajv, which recommends instantiating once at initialization (vs once per request here)
https://github.com/kogosoftwarellc/open-api/blob/9b58193f17f69a3f6b018a17c79a8c938b7628a2/packages/openapi-response-validator/index.ts#L79-L85
https://ajv.js.org/guide/managing-schemas.html#compiling-during-initialization
(We should probably document this in our openapi library...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, updated to create the validators just once. Will also update the docs.

path: getPath('/incoming-payments/{id}'),
method: HttpMethod.GET
})
)
}
36 changes: 36 additions & 0 deletions packages/open-payments/src/client/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { createOpenAPI } from 'openapi'

import { ILPStreamConnection, IncomingPayment } from '../types'
import config from '../config'
import { getIncomingPayment } from './incoming-payment'
import { getILPStreamConnection } from './ilp-stream-connection'
import { createAxiosInstance, GetArgs } from './requests'

export interface CreateOpenPaymentClientArgs {
timeout?: number
}

export interface OpenPaymentsClient {
incomingPayment: {
get(args: GetArgs): Promise<IncomingPayment>
}
ilpStreamConnection: {
get(args: GetArgs): Promise<ILPStreamConnection>
}
}

export const createClient = async (
args?: CreateOpenPaymentClientArgs
): Promise<OpenPaymentsClient> => {
const axios = createAxiosInstance(args)
const openApi = await createOpenAPI(config.OPEN_PAYMENTS_OPEN_API_URL)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

using the same version to validate the Open API as was used for generating the types in the first place


return {
incomingPayment: {
get: (args: GetArgs) => getIncomingPayment(axios, openApi, args)
},
ilpStreamConnection: {
get: (args: GetArgs) => getILPStreamConnection(axios, openApi, args)
}
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { createAxiosInstance, get } from './client'
/* eslint-disable @typescript-eslint/no-empty-function */
import { createAxiosInstance, get } from './requests'
import nock from 'nock'

describe('open-payments', (): void => {
describe('requests', (): void => {
jest.spyOn(console, 'log').mockImplementation(() => {})

describe('createAxiosInstance', (): void => {
test('sets timeout properly', async (): Promise<void> => {
expect(createAxiosInstance({ timeout: 1000 }).defaults.timeout).toBe(1000)
Expand All @@ -16,22 +19,24 @@ describe('open-payments', (): void => {
describe('get', (): void => {
const axiosInstance = createAxiosInstance()
const baseUrl = 'http://localhost:1000'
const successfulValidator = (data: unknown): data is unknown => true
const failedValidator = (data: unknown): data is unknown => false

beforeEach(() => {
beforeAll(() => {
jest.spyOn(axiosInstance, 'get')
})

test('sets headers properly if accessToken provided', async (): Promise<void> => {
nock(baseUrl)
.get('/incoming-payment')
.reply(200, () => ({
validReceiver: 0
}))
nock(baseUrl).get('/incoming-payment').reply(200)

await get(axiosInstance, {
url: `${baseUrl}/incoming-payment`,
accessToken: 'accessToken'
})
await get(
axiosInstance,
{
url: `${baseUrl}/incoming-payment`,
accessToken: 'accessToken'

Check failure

Code scanning / CodeQL

Hard-coded credentials

The hard-coded value "accessToken" is used as [authorization header](1).
},
successfulValidator
)

expect(axiosInstance.get).toHaveBeenCalledWith(
`${baseUrl}/incoming-payment`,
Expand All @@ -46,15 +51,15 @@ describe('open-payments', (): void => {
})

test('sets headers properly if accessToken is not provided', async (): Promise<void> => {
nock(baseUrl)
.get('/incoming-payment')
.reply(200, () => ({
validReceiver: 0
}))
nock(baseUrl).get('/incoming-payment').reply(200)

await get(axiosInstance, {
url: `${baseUrl}/incoming-payment`
})
await get(
axiosInstance,
{
url: `${baseUrl}/incoming-payment`
},
successfulValidator
)

expect(axiosInstance.get).toHaveBeenCalledWith(
`${baseUrl}/incoming-payment`,
Expand All @@ -63,5 +68,19 @@ describe('open-payments', (): void => {
}
)
})

test('throws if response validator function fails', async (): Promise<void> => {
nock(baseUrl).get('/incoming-payment').reply(200)

await expect(
get(
axiosInstance,
{
url: `${baseUrl}/incoming-payment`
},
failedValidator
)
).rejects.toThrow('Failed to validate OpenApi response')
})
})
})
Loading