From f5cfded56a06c1a830a0e7290033c4c1941a9b35 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 12 Mar 2021 15:56:18 +0100 Subject: [PATCH] todo-backend: add router tests + tweaks Signed-off-by: Patrik Oldsberg --- .../todo-backend/src/service/router.test.ts | 131 +++++++++++++++++- plugins/todo-backend/src/service/router.ts | 13 +- 2 files changed, 137 insertions(+), 7 deletions(-) diff --git a/plugins/todo-backend/src/service/router.test.ts b/plugins/todo-backend/src/service/router.test.ts index 8fda603937..13574d9381 100644 --- a/plugins/todo-backend/src/service/router.test.ts +++ b/plugins/todo-backend/src/service/router.test.ts @@ -16,8 +16,14 @@ import express from 'express'; import request from 'supertest'; +import { errorHandler } from '@backstage/backend-common'; -import { createRouter } from './router'; +import { + createRouter, + parseFilterParam, + parseIntegerParam, + parseOrderByParam, +} from './router'; import { TodoService } from './types'; const mockListBody = { @@ -27,6 +33,18 @@ const mockListBody = { limit: 10, }; +function matchErrorResponse(status: number, name: string, message: string) { + return { + status, + body: expect.objectContaining({ + error: { name, message }, + response: { + statusCode: status, + }, + }), + }; +} + describe('createRouter', () => { let app: express.Express; const mockService: jest.Mocked = { @@ -37,7 +55,7 @@ describe('createRouter', () => { const router = await createRouter({ todoService: mockService, }); - app = express().use(router); + app = express().use(router).use(errorHandler()); }); beforeEach(() => { @@ -89,5 +107,114 @@ describe('createRouter', () => { limit: undefined, }); }); + + it('rejects invalid queries', async () => { + await expect( + request(app).get('/v1/todos?entity=k:n&entity=k:n'), + ).resolves.toMatchObject( + matchErrorResponse(400, 'InputError', 'entity query must be a string'), + ); + + await expect( + request(app).get('/v1/todos?entity=:n'), + ).resolves.toMatchObject( + matchErrorResponse( + 400, + 'InputError', + 'Invalid entity ref, TypeError: Entity reference ":n" was not on the form [:][/]', + ), + ); + + await expect( + request(app).get('/v1/todos?offset=1.5'), + ).resolves.toMatchObject( + matchErrorResponse( + 400, + 'InputError', + 'invalid offset query, not an integer', + ), + ); + + expect(mockService.listTodos).not.toHaveBeenCalled(); + }); + }); +}); + +describe('parseIntegerParam', () => { + it('should parse a param', () => { + expect(parseIntegerParam('1', 'ctx')).toBe(1); + }); + + it('should reject invalid params', () => { + expect(() => parseIntegerParam(['1'], 'ctx')).toThrow( + 'invalid ctx, must be a string', + ); + expect(() => parseIntegerParam('1.5', 'ctx')).toThrow( + 'invalid ctx, not an integer', + ); + expect(() => parseIntegerParam('foo', 'ctx')).toThrow( + 'invalid ctx, not an integer', + ); + expect(() => parseIntegerParam('1foo', 'ctx')).toThrow( + 'invalid ctx, not an integer', + ); + }); +}); + +describe('parseOrderByParam', () => { + it('should parse a param', () => { + expect(parseOrderByParam('a=asc', ['a'])).toEqual({ + field: 'a', + direction: 'asc', + }); + expect(parseOrderByParam('a=desc', ['a'])).toEqual({ + field: 'a', + direction: 'desc', + }); + }); + + it('should reject invalid params', () => { + expect(() => parseOrderByParam(['a=asc'], ['a'])).toThrow( + 'invalid orderBy query, must be a string', + ); + expect(() => parseOrderByParam('a=desc', ['b', 'c'])).toThrow( + 'invalid orderBy field, must be one of b, c', + ); + expect(() => parseOrderByParam('b=down', ['a'])).toThrow( + `invalid orderBy query, order direction must be 'asc' or 'desc'`, + ); + expect(() => parseOrderByParam('=asc', ['a'])).toThrow( + 'invalid orderBy query, field name is empty', + ); + }); +}); + +describe('parseFilterParam', () => { + it('should parse a param', () => { + expect(parseFilterParam('a=b', ['a'])).toEqual([ + { field: 'a', value: 'b' }, + ]); + expect(parseFilterParam(['a=b=c', 'c=d*'], ['a', 'c'])).toEqual([ + { field: 'a', value: 'b=c' }, + { field: 'c', value: 'd*' }, + ]); + }); + + it('should reject invalid params', () => { + expect(() => parseFilterParam({ a: 'a=b' }, ['a'])).toThrow( + 'invalid filter query, must be a string or list of strings', + ); + expect(() => parseFilterParam('a=b', ['b', 'c'])).toThrow( + 'invalid filter field, must be one of b, c', + ); + expect(() => parseFilterParam('b', ['a'])).toThrow( + `invalid filter query, must separate field from value using '='`, + ); + expect(() => parseFilterParam('=a', ['a'])).toThrow( + `invalid filter query, must separate field from value using '='`, + ); + expect(() => parseFilterParam('a=', ['a'])).toThrow( + 'invalid filter query, value may not be empty', + ); }); }); diff --git a/plugins/todo-backend/src/service/router.ts b/plugins/todo-backend/src/service/router.ts index d2eb7278f8..9c06e19a20 100644 --- a/plugins/todo-backend/src/service/router.ts +++ b/plugins/todo-backend/src/service/router.ts @@ -72,7 +72,10 @@ export async function createRouter( return router; } -function parseIntegerParam(str: unknown, ctx: string): number | undefined { +export function parseIntegerParam( + str: unknown, + ctx: string, +): number | undefined { if (str === undefined) { return undefined; } @@ -80,13 +83,13 @@ function parseIntegerParam(str: unknown, ctx: string): number | undefined { throw new InputError(`invalid ${ctx}, must be a string`); } const parsed = parseInt(str, 10); - if (!Number.isInteger(parsed)) { + if (!Number.isInteger(parsed) || String(parsed) !== str) { throw new InputError(`invalid ${ctx}, not an integer`); } return parsed; } -function parseOrderByParam( +export function parseOrderByParam( str: unknown, allowedFields: T, ): { field: T[number]; direction: 'asc' | 'desc' } | undefined { @@ -108,13 +111,13 @@ function parseOrderByParam( if (field && !allowedFields.includes(field)) { throw new InputError( - `invalid orderBy query, must be one of ${allowedFields.join(', ')}`, + `invalid orderBy field, must be one of ${allowedFields.join(', ')}`, ); } return { field, direction }; } -function parseFilterParam( +export function parseFilterParam( str: unknown, allowedFields: T, ): { field: T[number]; value: string }[] | undefined {