From ebb6d2c62dd07cb427658bb80b7e7828b8c3e9be Mon Sep 17 00:00:00 2001 From: Chris Miaskowski Date: Fri, 19 Oct 2018 08:52:36 +0200 Subject: [PATCH] feat(router): throw exceptions instead return null --- .../http/src/router/__tests__/index.spec.ts | 51 ++++++++------- packages/http/src/router/errors.ts | 8 +++ packages/http/src/router/index.ts | 62 ++++++++++++++++--- packages/http/src/router/matchBaseUrl.ts | 2 +- 4 files changed, 93 insertions(+), 30 deletions(-) create mode 100644 packages/http/src/router/errors.ts diff --git a/packages/http/src/router/__tests__/index.spec.ts b/packages/http/src/router/__tests__/index.spec.ts index 3aefbc725..474e7cf83 100644 --- a/packages/http/src/router/__tests__/index.spec.ts +++ b/packages/http/src/router/__tests__/index.spec.ts @@ -1,5 +1,12 @@ import { IHttpOperation, IServer } from '@stoplight/types'; import { Chance } from 'chance'; +import { + NO_RESOURCE_PROVIDED_ERROR, + NO_SERVER_CONFIGURATION_PROVIDED_ERROR, + NONE_METHOD_MATCHED_ERROR, + NONE_PATH_MATCHED_ERROR, + NONE_SERVER_MATCHED_ERROR, +} from '../errors'; import { router } from '../index'; import { pickOneHttpMethod, pickSetOfHttpMethods, randomPath } from './utils'; @@ -17,8 +24,8 @@ function createResource(method: string, path: string, servers: IServer[] = []): describe('http router', () => { describe('route()', () => { - test('should return null if no resources given', async () => { - const resource = await router.route({ + test('should return null if no resources given', () => { + const resourcePromise = router.route({ resources: [], input: { method: pickOneHttpMethod(), @@ -29,29 +36,31 @@ describe('http router', () => { }, }); - expect(resource).toBeNull(); + return expect(resourcePromise).rejects.toBe(NO_RESOURCE_PROVIDED_ERROR); }); describe('given a resource', () => { - test('should not match if no server defined', async () => { - const resource = await router.route({ - resources: [createResource(pickOneHttpMethod(), randomPath())], + test('should not match if no server defined', () => { + const method = pickOneHttpMethod(); + const path = randomPath(); + const resourcePromise = router.route({ + resources: [createResource(method, path)], input: { - method: pickOneHttpMethod(), + method, url: { baseUrl: '', - path: '', + path, }, }, }); - expect(resource).toBeNull(); + return expect(resourcePromise).rejects.toBe(NO_SERVER_CONFIGURATION_PROVIDED_ERROR); }); - test('given a concrete matching server and unmatched methods should not match', async () => { + test('given a concrete matching server and unmatched methods should not match', () => { const url = chance.url(); const [resourceMethod, requestMethod] = pickSetOfHttpMethods(2); - const resource = await router.route({ + const resourcePromise = router.route({ resources: [ createResource(resourceMethod, randomPath(), [ { @@ -68,16 +77,16 @@ describe('http router', () => { }, }); - expect(resource).toBeNull(); + return expect(resourcePromise).rejects.toBe(NONE_METHOD_MATCHED_ERROR); }); describe('given matched methods', () => { const method = pickOneHttpMethod(); - test('given a concrete matching server unmatched path should not match', async () => { + test('given a concrete matching server unmatched path should not match', () => { const url = chance.url(); const path = randomPath({ trailingSlash: false }); - const resource = await router.route({ + const resourcePromise = router.route({ resources: [ createResource(method, path, [ { @@ -94,7 +103,7 @@ describe('http router', () => { }, }); - expect(resource).toBeNull(); + return expect(resourcePromise).rejects.toBe(NONE_PATH_MATCHED_ERROR); }); test('given a concrete matching server and matched concrete path should match', async () => { @@ -142,7 +151,7 @@ describe('http router', () => { expect(resource).toBe(expectedResource); }); - test('given a concrete matching server and unmatched templated path should not match', async () => { + test('given a concrete matching server and unmatched templated path should not match', () => { const url = chance.url(); const templatedPath = '/a/{x}/c'; const requestPath = '/a/y/b'; @@ -151,7 +160,7 @@ describe('http router', () => { url, }, ]); - const resource = await router.route({ + const resourcePromise = router.route({ resources: [expectedResource], input: { method, @@ -162,7 +171,7 @@ describe('http router', () => { }, }); - expect(resource).toBeNull(); + return expect(resourcePromise).rejects.toBe(NONE_PATH_MATCHED_ERROR); }); test('given a concrete servers and mixed paths should match concrete path', async () => { @@ -249,10 +258,10 @@ describe('http router', () => { expect(resource).toBe(resourceWithMatchingPath); }); - test('given empty baseUrl and concrete server it should not match', async () => { + test('given empty baseUrl and concrete server it should not match', () => { const path = randomPath({ includeTemplates: false }); const url = 'concrete.com'; - const resource = await router.route({ + const resourcePromise = router.route({ resources: [createResource(method, path, [{ url }])], input: { method, @@ -263,7 +272,7 @@ describe('http router', () => { }, }); - expect(resource).toBeNull(); + return expect(resourcePromise).rejects.toBe(NONE_SERVER_MATCHED_ERROR); }); test('given empty baseUrl and empty server url it should match', async () => { diff --git a/packages/http/src/router/errors.ts b/packages/http/src/router/errors.ts new file mode 100644 index 000000000..615371448 --- /dev/null +++ b/packages/http/src/router/errors.ts @@ -0,0 +1,8 @@ +export const ROUTE_DISAMBIGUATION_ERROR = new Error('Could not disambiguate the given route.'); +export const NO_RESOURCE_PROVIDED_ERROR = new Error('Route not resolved, no resource provided.'); +export const NONE_METHOD_MATCHED_ERROR = new Error('Route not resolved, none method matched.'); +export const NONE_PATH_MATCHED_ERROR = new Error('Route not resolved, none path matched.'); +export const NONE_SERVER_MATCHED_ERROR = new Error('Route not resolved, none server matched.'); +export const NO_SERVER_CONFIGURATION_PROVIDED_ERROR = new Error( + 'Route not resolved, no server configuration provided.' +); diff --git a/packages/http/src/router/index.ts b/packages/http/src/router/index.ts index 6d95e66e0..7d9768216 100644 --- a/packages/http/src/router/index.ts +++ b/packages/http/src/router/index.ts @@ -2,6 +2,14 @@ import { IRouter } from '@stoplight/prism-core'; import { IHttpOperation } from '@stoplight/types'; import { IHttpConfig, IHttpRequest } from '../types'; +import { + NO_RESOURCE_PROVIDED_ERROR, + NO_SERVER_CONFIGURATION_PROVIDED_ERROR, + NONE_METHOD_MATCHED_ERROR, + NONE_PATH_MATCHED_ERROR, + NONE_SERVER_MATCHED_ERROR, + ROUTE_DISAMBIGUATION_ERROR, +} from './errors'; import { matchBaseUrl } from './matchBaseUrl'; import { matchPath } from './matchPath'; import { IMatch, MatchType } from './types'; @@ -11,13 +19,27 @@ export const router: IRouter = { const matches = []; const { path: requestPath, baseUrl: requestBaseUrl } = input.url; + if (!resources.length) { + throw NO_RESOURCE_PROVIDED_ERROR; + } + + let noServerProvided: boolean = true; + let noneMethodMatched: boolean = true; + let nonePathMatched: boolean = true; + let noneServerMatched: boolean = true; + for (const resource of resources) { if (!matchByMethod(input, resource)) continue; + noneMethodMatched = false; const pathMatch = matchPath(requestPath, resource.path); const serverMatches = []; const { servers = [] } = resource; + + if (servers.length) noServerProvided = false; + if (pathMatch !== MatchType.NOMATCH) nonePathMatched = false; + for (const server of servers) { const tempServerMatch = matchBaseUrl(server, requestBaseUrl); if (tempServerMatch !== MatchType.NOMATCH) { @@ -27,15 +49,34 @@ export const router: IRouter = { const serverMatch = disambiguateServers(serverMatches); - if (serverMatch && pathMatch !== MatchType.NOMATCH) { - matches.push({ - pathMatch, - serverMatch, - resource, - }); + if (serverMatch) { + noneServerMatched = false; + if (pathMatch !== MatchType.NOMATCH) { + matches.push({ + pathMatch, + serverMatch, + resource, + }); + } } } + if (noneMethodMatched) { + throw NONE_METHOD_MATCHED_ERROR; + } + + if (noServerProvided) { + throw NO_SERVER_CONFIGURATION_PROVIDED_ERROR; + } + + if (nonePathMatched) { + throw NONE_PATH_MATCHED_ERROR; + } + + if (noneServerMatched) { + throw NONE_SERVER_MATCHED_ERROR; + } + return disambiguateMatches(matches); }, }; @@ -44,7 +85,7 @@ function matchByMethod(request: IHttpRequest, operation: IHttpOperation): boolea return operation.method.toLowerCase() === request.method.toLowerCase(); } -function disambiguateMatches(matches: IMatch[]): null | IHttpOperation { +function disambiguateMatches(matches: IMatch[]): IHttpOperation { const matchResult = // prefer concrete server and concrete path matches.find(match => areServerAndPath(match, MatchType.CONCRETE, MatchType.CONCRETE)) || @@ -54,7 +95,12 @@ function disambiguateMatches(matches: IMatch[]): null | IHttpOperation { matches.find(match => areServerAndPath(match, MatchType.CONCRETE, MatchType.TEMPLATED)) || // then fallback to first matches[0]; - return matchResult ? matchResult.resource : null; + + if (!matchResult) { + throw ROUTE_DISAMBIGUATION_ERROR; + } + + return matchResult.resource; } function areServerAndPath(match: IMatch, serverType: MatchType, pathType: MatchType) { diff --git a/packages/http/src/router/matchBaseUrl.ts b/packages/http/src/router/matchBaseUrl.ts index 41e014fca..430dbae1f 100644 --- a/packages/http/src/router/matchBaseUrl.ts +++ b/packages/http/src/router/matchBaseUrl.ts @@ -20,7 +20,7 @@ export function convertTemplateToRegExp( ) { const regexp = !variables ? urlTemplate - : urlTemplate.replace(variableRegexp, (match, variableName) => { + : urlTemplate.replace(variableRegexp, (_match, variableName) => { const variable = variables[variableName]; if (!variable) { throw new Error(`Variable '${variableName}' is not defined, cannot parse input.`);