Additional checks for Routing (#33)

* Handling of empty paths and trimming spaces.

* Prevent using slashes in Routing elements.

src/errors.ts

@@ -1,4 +1,7 @@
-export class DependsOnMethodError extends Error {
+export class RoutingError extends Error {
+}
+
+export class DependsOnMethodError extends RoutingError {
 }
 
 export class OpenAPIError extends Error {

src/index.ts

@@ -9,7 +9,7 @@ export { ResultHandler } from './result-handler';
 export { Routing, DependsOnMethod } from './routing';
 export { createServer, attachRouting } from './server';
 export { OpenAPI } from './open-api';
-export { OpenAPIError, DependsOnMethodError } from './errors';
+export { OpenAPIError, DependsOnMethodError, RoutingError } from './errors';
 
 import { z } from 'zod';
 import * as createHttpError from 'http-errors';

src/routing.ts

@@ -2,7 +2,7 @@ import {Express} from 'express';
 import {Logger} from 'winston';
 import {ConfigType} from './config-type';
 import {AbstractEndpoint, Endpoint} from './endpoint';
-import {DependsOnMethodError} from './errors';
+import {DependsOnMethodError, RoutingError} from './errors';
 import {Method} from './method';
 
 export class DependsOnMethod {
@@ -43,9 +43,15 @@ export interface Routing {
 type RoutingCycleCallback = (endpoint: AbstractEndpoint, fullPath: string, method: Method) => void;
 
 export const routingCycle = (routing: Routing, cb: RoutingCycleCallback, parentPath?: string) => {
-  Object.keys(routing).forEach((path) => {
-    const fullPath = `${parentPath || ''}/${path}`;
-    const element = routing[path];
+  Object.entries(routing).forEach(([path, element]) => {
+    path = path.trim();
+    if (path.match(/\//)) {
+      throw new RoutingError(
+        'Routing elements should not contain \'/\' character.\n' + 
+        `The error caused by ${parentPath ? `'${parentPath}' route that has a '${path}'` : `'${path}'`} entry.`
+      );
+    }
+    const fullPath = `${parentPath || ''}${path ? `/${path}` : ''}`;
     if (element instanceof AbstractEndpoint) {
       element.getMethods().forEach((method) => {
         cb(element, fullPath, method);

tests/unit/__snapshots__/routing.spec.ts.snap

@@ -14,3 +14,13 @@ new DependsOnMethod({
 });
 "
 `;
+
+exports[`Routing initRouting() Should throw an error in case of slashes in route 1`] = `
+"Routing elements should not contain '/' character.
+The error caused by '/v1' route that has a 'user/retrieve' entry."
+`;
+
+exports[`Routing initRouting() Should throw an error in case of slashes in route 2`] = `
+"Routing elements should not contain '/' character.
+The error caused by 'v1/user/retrieve' entry."
+`;

tests/unit/routing.spec.ts

@@ -120,6 +120,91 @@ describe('Routing', () => {
       expect(appMock.patch.mock.calls[0][0]).toBe('/v1/user');
     });
 
+    test('Should accept parameters', () => {
+      const handlerMock = jest.fn();
+      const configMock = {};
+      const endpointMock = new EndpointsFactory().build({
+        methods: ['get'],
+        input: z.object({}).nonstrict(),
+        output: z.object({}).nonstrict(),
+        handler: handlerMock
+      });
+      const routing: Routing = {
+        v1: {
+          user: {
+            ':id': endpointMock
+          }
+        }
+      };
+      initRouting({
+        app: appMock as Express,
+        logger: loggerMock as Logger,
+        config: configMock as ConfigType,
+        routing: routing
+      });
+      expect(appMock.get).toBeCalledTimes(1);
+      expect(appMock.get.mock.calls[0][0]).toBe('/v1/user/:id');
+    });
+
+    test('Should handle empty paths and trim spaces', () => {
+      const handlerMock = jest.fn();
+      const configMock = {};
+      const endpointMock = new EndpointsFactory().build({
+        methods: ['get'],
+        input: z.object({}).nonstrict(),
+        output: z.object({}).nonstrict(),
+        handler: handlerMock
+      });
+      const routing: Routing = {
+        v1: {
+          user: {
+            ':id': {
+              '': endpointMock,
+              ' download ': endpointMock
+            }
+          }
+        }
+      };
+      initRouting({
+        app: appMock as Express,
+        logger: loggerMock as Logger,
+        config: configMock as ConfigType,
+        routing: routing
+      });
+      expect(appMock.get).toBeCalledTimes(2);
+      expect(appMock.get.mock.calls[0][0]).toBe('/v1/user/:id');
+      expect(appMock.get.mock.calls[1][0]).toBe('/v1/user/:id/download');
+    });
+
+    test('Should throw an error in case of slashes in route', () => {
+      const handlerMock = jest.fn();
+      const configMock = {};
+      const endpointMock = new EndpointsFactory().build({
+        methods: ['get'],
+        input: z.object({}).nonstrict(),
+        output: z.object({}).nonstrict(),
+        handler: handlerMock
+      });
+      expect(() => initRouting({
+        app: appMock as Express,
+        logger: loggerMock as Logger,
+        config: configMock as ConfigType,
+        routing: {
+          v1: {
+            'user/retrieve': endpointMock
+          }
+        }
+      })).toThrowErrorMatchingSnapshot();
+      expect(() => initRouting({
+        app: appMock as Express,
+        logger: loggerMock as Logger,
+        config: configMock as ConfigType,
+        routing: {
+          'v1/user/retrieve': endpointMock
+        }
+      })).toThrowErrorMatchingSnapshot();
+    });
+
     test('Should execute endpoints with right arguments', async () => {
       const handlerMock = jest.fn().mockImplementationOnce(() => ({result: true}));
       const configMock = { cors: true };