todo-backend: add router tests + tweaks

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

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<TodoService> = {
@@ -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 [<kind>:][<namespace>/]<name>',
+        ),
+      );
+
+      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',
+    );
   });
 });

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<T extends readonly string[]>(
+export function parseOrderByParam<T extends readonly string[]>(
   str: unknown,
   allowedFields: T,
 ): { field: T[number]; direction: 'asc' | 'desc' } | undefined {
@@ -108,13 +111,13 @@ function parseOrderByParam<T extends readonly string[]>(
 
   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<T extends readonly string[]>(
+export function parseFilterParam<T extends readonly string[]>(
   str: unknown,
   allowedFields: T,
 ): { field: T[number]; value: string }[] | undefined {