From 7cac1bd5002b42d2d079480090fcc0522aeba1c2 Mon Sep 17 00:00:00 2001 From: Swain Molster Date: Fri, 10 Jun 2022 15:12:36 -0400 Subject: [PATCH 1/2] migrate to using koa service to perform assertions about axios client --- src/integration.axios.test.ts | 154 +++++++++++++++++++++------------- src/test-utils.ts | 34 ++++++++ 2 files changed, 129 insertions(+), 59 deletions(-) create mode 100644 src/test-utils.ts diff --git a/src/integration.axios.test.ts b/src/integration.axios.test.ts index 6b387dc..a92f915 100644 --- a/src/integration.axios.test.ts +++ b/src/integration.axios.test.ts @@ -1,10 +1,13 @@ import { writeFileSync } from 'fs'; import { format } from 'prettier'; +import Koa from 'koa'; +import bodyparser = require('koa-bodyparser'); import { OneSchemaDefinition } from '.'; import { generateAxiosClient, GenerateAxiosClientInput, } from './generate-axios-client'; +import { useServiceClient } from './test-utils'; const testGeneratedFile = (ext: string) => `${__dirname}/test-generated${ext}`; @@ -14,7 +17,37 @@ const generateAndFormat = (input: GenerateAxiosClientInput) => declaration: format(source.declaration, { parser: 'typescript' }), })); -const prepare = async () => { +const mockMiddleware = jest.fn(); + +/** + * We use an actual HTTP server to help with assertions. Why: this + * helps us protect against supply chain problems with Axios, and + * confirm end-behavior (rather than just assert "we passed the right + * parameters to Axios"). + */ +const context = useServiceClient({ + service: new Koa() + .use(bodyparser()) + .use((ctx, next) => { + ctx.status = 200; + // Just return data about the request, to be used for assertions. + ctx.body = { + method: ctx.method, + query: ctx.query, + path: ctx.path, + body: ctx.request.body, + }; + return next(); + }) + .use(mockMiddleware), +}); + +/** The "well-typed" client generated from the schema. */ +let client: any; + +beforeEach(async () => { + mockMiddleware.mockReset().mockImplementation((ctx, next) => next()); + const spec: OneSchemaDefinition = { Resources: {}, Endpoints: { @@ -60,6 +93,7 @@ const prepare = async () => { additionalProperties: false, properties: { filter: { type: 'string' }, + url: { type: 'string' }, nextPageToken: { type: 'string' }, pageSize: { type: 'string' }, }, @@ -92,122 +126,124 @@ const prepare = async () => { writeFileSync(testGeneratedFile('.d.ts'), output.declaration); const { Service } = await import(testGeneratedFile('.js')); - - const request = jest.fn(); - const client = new Service({ request }); - return { client, request }; -}; + client = new Service(context.client); +}); describe('integration tests', () => { test('compile + execute', async () => { - const { client, request } = await prepare(); - expect(client).toMatchObject({ getPosts: expect.any(Function), putPost: expect.any(Function), }); - await client.getPosts({ input: 'some-input' }); - expect(request).toHaveBeenCalledTimes(1); - expect(request).toHaveBeenNthCalledWith(1, { + const getResult = await client.getPosts({ input: 'some-input' }); + expect(mockMiddleware).toHaveBeenCalledTimes(1); + expect(getResult.data).toStrictEqual({ method: 'GET', - url: '/posts', - params: { + path: '/posts', + body: {}, + query: { input: 'some-input', }, }); - await client.putPost({ id: 'some-id', message: 'some-message' }); - expect(request).toHaveBeenCalledTimes(2); - expect(request).toHaveBeenNthCalledWith(2, { + const putResult = await client.putPost({ + id: 'some-id', + message: 'some-message', + }); + expect(mockMiddleware).toHaveBeenCalledTimes(2); + expect(putResult.data).toStrictEqual({ method: 'PUT', - url: '/posts/some-id', - data: { + path: '/posts/some-id', + body: { message: 'some-message', }, + query: {}, }); }); test('generated code URI-encodes path parameters', async () => { - const { client, request } = await prepare(); - - await client.putPost({ id: 'some,bogus,param', message: 'some-message' }); + const result = await client.putPost({ + id: 'some,bogus,param', + message: 'some-message', + }); - expect(request).toHaveBeenCalledTimes(1); - expect(request).toHaveBeenCalledWith({ + expect(mockMiddleware).toHaveBeenCalledTimes(1); + expect(result.data).toStrictEqual({ method: 'PUT', - url: '/posts/some%2Cbogus%2Cparam', - data: { + path: '/posts/some%2Cbogus%2Cparam', + body: { message: 'some-message', }, + query: {}, }); }); test('generated code URI-encodes query parameters', async () => { - const { client, request } = await prepare(); - - await client.listPosts({ filter: 'some/evil/string' }); + const result = await client.listPosts({ filter: 'some/evil/string' }); - expect(request).toHaveBeenCalledTimes(1); - expect(request).toHaveBeenCalledWith({ + expect(mockMiddleware).toHaveBeenCalledTimes(1); + expect(result.data).toStrictEqual({ method: 'GET', - url: '/posts/list', - params: { + path: '/posts/list', + body: {}, + query: { filter: 'some%2Fevil%2Fstring', }, }); }); test('generated code does not send undefined query parameters', async () => { - const { client, request } = await prepare(); - - await client.listPosts({ filter: undefined }); + const result = await client.listPosts({ filter: undefined }); - expect(request).toHaveBeenCalledTimes(1); - expect(request).toHaveBeenCalledWith({ + expect(mockMiddleware).toHaveBeenCalledTimes(1); + expect(result.data).toStrictEqual({ method: 'GET', - url: '/posts/list', - params: {}, + path: '/posts/list', + body: {}, + query: {}, }); }); test('pagination', async () => { - const { client, request } = await prepare(); + const requestSpy = jest.spyOn(client.client, 'request'); - request - .mockResolvedValueOnce({ - data: { - items: ['first', 'second'], - links: { - self: 'blah-blah', - next: '/posts/list?nextPageToken=firstpagetoken&pageSize=10&randomProperty=blah', - }, - }, + mockMiddleware + .mockImplementationOnce((ctx) => { + if (ctx.body) + ctx.body = { + items: ['first', 'second'], + links: { + self: 'blah-blah', + next: '/posts/list?nextPageToken=firstpagetoken&pageSize=10&randomProperty=blah', + }, + }; }) - .mockResolvedValueOnce({ - data: { + .mockImplementationOnce((ctx) => { + ctx.body = { items: ['third', 'fourth'], links: { self: 'blah-blah', next: '/posts/list?nextPageToken=secondpagetoken&pageSize=10&randomProperty=blah', }, - }, + }; }) - .mockResolvedValueOnce({ - data: { + .mockImplementationOnce((ctx) => { + ctx.body = { items: ['fifth'], links: { self: 'blah-blah', }, - }, + }; }); const result = await client.paginate(client.listPosts, { filter: 'something', }); - expect(request).toHaveBeenCalledTimes(3); - expect(request).toHaveBeenNthCalledWith(1, { + expect(mockMiddleware).toHaveBeenCalledTimes(3); + expect(requestSpy).toHaveBeenCalledTimes(3); + expect(requestSpy).toHaveBeenNthCalledWith(1, { method: 'GET', params: { filter: 'something', @@ -215,7 +251,7 @@ describe('integration tests', () => { url: '/posts/list', }); // After first requests, inherits default page size - expect(request).toHaveBeenNthCalledWith(2, { + expect(requestSpy).toHaveBeenNthCalledWith(2, { method: 'GET', params: { filter: 'something', @@ -224,7 +260,7 @@ describe('integration tests', () => { }, url: '/posts/list', }); - expect(request).toHaveBeenNthCalledWith(3, { + expect(requestSpy).toHaveBeenNthCalledWith(3, { method: 'GET', params: { filter: 'something', diff --git a/src/test-utils.ts b/src/test-utils.ts new file mode 100644 index 0000000..17ef8a5 --- /dev/null +++ b/src/test-utils.ts @@ -0,0 +1,34 @@ +import { Server } from 'http'; +import axios, { AxiosInstance } from 'axios'; +import Koa from 'koa'; + +export type UseSchemaClientOptions = { + service: Koa; +}; + +export type UseServiceClientContext = { + client: AxiosInstance; +}; + +export const useServiceClient = ({ service }: UseSchemaClientOptions) => { + const context: UseServiceClientContext = {} as any; + + let server: Server; + + beforeEach(() => { + server = service.listen(); + + context.client = axios.create({ + // In tests, we should always explicitly assert 200. + validateStatus: () => true, + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + baseURL: `http://127.0.0.1:${(server.address() as any).port}`, + }); + }); + + afterEach(() => { + server.close(); + }); + + return context; +}; From aa497499ad2bc76b18edde2145b234752c8b2d76 Mon Sep 17 00:00:00 2001 From: Swain Molster Date: Fri, 10 Jun 2022 15:16:48 -0400 Subject: [PATCH 2/2] fix: correct encoding behavior --- src/generate-axios-client.test.ts | 10 ++++------ src/generate-axios-client.ts | 10 ++++------ src/integration.axios.test.ts | 7 +++---- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/generate-axios-client.test.ts b/src/generate-axios-client.test.ts index 2d46569..a2b93b6 100644 --- a/src/generate-axios-client.test.ts +++ b/src/generate-axios-client.test.ts @@ -68,14 +68,12 @@ const substituteParams = (url, params) => url ); -const removePathParams = (url, params, encode) => +const removePathParams = (url, params) => Object.entries(params) .filter(([key, value]) => value !== undefined) .reduce( (accum, [name, value]) => - url.includes(":" + name) - ? accum - : { ...accum, [name]: encode ? encodeURIComponent(value) : value }, + url.includes(":" + name) ? accum : { ...accum, [name]: value }, {} ); @@ -97,7 +95,7 @@ class Client { return this.client.request({ ...config, method: "GET", - params: removePathParams("/posts", data, true), + params: removePathParams("/posts", data), url: substituteParams("/posts", data), }); } @@ -106,7 +104,7 @@ class Client { return this.client.request({ ...config, method: "PUT", - data: removePathParams("/posts/:id", data, false), + data: removePathParams("/posts/:id", data), url: substituteParams("/posts/:id", data), }); } diff --git a/src/generate-axios-client.ts b/src/generate-axios-client.ts index 89475cf..1fa3283 100644 --- a/src/generate-axios-client.ts +++ b/src/generate-axios-client.ts @@ -64,14 +64,12 @@ const substituteParams = (url, params) => url ); -const removePathParams = (url, params, encode) => +const removePathParams = (url, params) => Object.entries(params) .filter(([key, value]) => value !== undefined) .reduce( (accum, [name, value]) => - url.includes(':' + name) - ? accum - : { ...accum, [name]: encode ? encodeURIComponent(value) : value }, + url.includes(':' + name) ? accum : { ...accum, [name]: value }, {} ); @@ -101,8 +99,8 @@ class ${outputClass} { method: '${method}', ${ useQueryParams - ? `params: removePathParams('${url}', data, true),` - : `data: removePathParams('${url}', data, false),` + ? `params: removePathParams('${url}', data),` + : `data: removePathParams('${url}', data),` } url: substituteParams('${url}', data), }) diff --git a/src/integration.axios.test.ts b/src/integration.axios.test.ts index a92f915..d0b7dba 100644 --- a/src/integration.axios.test.ts +++ b/src/integration.axios.test.ts @@ -180,16 +180,15 @@ describe('integration tests', () => { }); test('generated code URI-encodes query parameters', async () => { - const result = await client.listPosts({ filter: 'some/evil/string' }); + const filter = 'some/evil/string'; + const result = await client.listPosts({ filter }); expect(mockMiddleware).toHaveBeenCalledTimes(1); expect(result.data).toStrictEqual({ method: 'GET', path: '/posts/list', body: {}, - query: { - filter: 'some%2Fevil%2Fstring', - }, + query: { filter }, }); });