From 7a3d35e46272fc003d25ef7df052fea69ab587e4 Mon Sep 17 00:00:00 2001 From: Karol Maciaszek Date: Wed, 10 Apr 2019 15:12:11 +0200 Subject: [PATCH] fix: SL-80 fixed router logic --- .../http/src/router/__tests__/index.spec.ts | 48 +++++++++++++++++-- packages/http/src/router/errors.ts | 3 ++ packages/http/src/router/index.ts | 34 ++++++++----- 3 files changed, 68 insertions(+), 17 deletions(-) diff --git a/packages/http/src/router/__tests__/index.spec.ts b/packages/http/src/router/__tests__/index.spec.ts index c9650622b..bb095589c 100644 --- a/packages/http/src/router/__tests__/index.spec.ts +++ b/packages/http/src/router/__tests__/index.spec.ts @@ -2,6 +2,7 @@ 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, @@ -25,7 +26,25 @@ function createResource(method: string, path: string, servers: IServer[]): IHttp describe('http router', () => { describe('route()', () => { - test('should return null if no resources given', () => { + test('should not match if no server defined', () => { + const method = pickOneHttpMethod(); + const path = randomPath(); + + return expect(() => + router.route({ + resources: [createResource(method, path, [])], + input: { + method, + url: { + baseUrl: 'http://some.url/', + path, + }, + }, + }) + ).toThrow(NO_SERVER_CONFIGURATION_PROVIDED_ERROR); + }); + + test('should not match if no resources given', () => { expect(() => router.route({ resources: [], @@ -305,7 +324,7 @@ describe('http router', () => { expect(resource).toBe(resourceWithConcreteMatch); }); - test('given concret servers should match by path', async () => { + test('given concrete servers should match by path', async () => { const matchingPath = '/a/b/c'; const nonMatchingPath = '/a/b/c/d'; const url = 'concrete.com'; @@ -326,7 +345,26 @@ describe('http router', () => { expect(resource).toBe(resourceWithMatchingPath); }); - test('given empty baseUrl and concrete server it should not match', () => { + test('given empty baseUrl and concrete server it should match', () => { + const path = randomPath({ includeTemplates: false }); + const url = 'concrete.com'; + const expectedResource = createResource(method, path, [{ url }]); + + return expect( + router.route({ + resources: [expectedResource], + input: { + method, + url: { + baseUrl: '', + path, + }, + }, + }) + ).toEqual(expectedResource); + }); + + test('given baseUrl and concrete server and non-existing request baseUrl it should not match', () => { const path = randomPath({ includeTemplates: false }); const url = 'concrete.com'; @@ -336,12 +374,12 @@ describe('http router', () => { input: { method, url: { - baseUrl: '', + baseUrl: 'solid.com', path, }, }, }) - ).toThrow(NONE_SERVER_MATCHED_ERROR); + ).toThrowError(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 index 9d820a343..82f2bc58b 100644 --- a/packages/http/src/router/errors.ts +++ b/packages/http/src/router/errors.ts @@ -2,3 +2,6 @@ export const NO_RESOURCE_PROVIDED_ERROR = new Error('Route not resolved, no reso 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 ca682c2cb..8bd8cf5c2 100644 --- a/packages/http/src/router/index.ts +++ b/packages/http/src/router/index.ts @@ -4,6 +4,7 @@ import { IHttpOperation, IServer } 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, @@ -16,31 +17,36 @@ export const router: IRouter = { route: ({ resources, input }) => { const matches = []; const { path: requestPath, baseUrl: requestBaseUrl } = input.url; - const ignoreServers: boolean = requestBaseUrl === undefined; + const serverValidationEnabled = !!requestBaseUrl; if (!resources.length) { throw NO_RESOURCE_PROVIDED_ERROR; } - let noneMethodMatched: boolean = true; - let nonePathMatched: boolean = true; - let noneServerMatched: boolean = true; + let anyMethodMatched = false; + let anyPathMatched = false; + let anyServerMatched = false; + let anyServerProvided = false; for (const resource of resources) { if (!matchByMethod(input, resource)) continue; - noneMethodMatched = false; + anyMethodMatched = true; const pathMatch = matchPath(requestPath, resource.path); - if (pathMatch !== MatchType.NOMATCH) nonePathMatched = false; + if (pathMatch !== MatchType.NOMATCH) anyPathMatched = true; const { servers = [] } = resource; let serverMatch: MatchType | null = null; - if (!ignoreServers && servers.length > 0) { + if (serverValidationEnabled) { + if (servers.length === 0) continue; + + anyServerProvided = true; serverMatch = matchServer(servers, requestBaseUrl as string); - if (serverMatch) noneServerMatched = false; + if (serverMatch) anyServerMatched = true; } else { - noneServerMatched = false; + anyServerMatched = true; + anyServerProvided = true; } if (pathMatch !== MatchType.NOMATCH) { @@ -52,15 +58,19 @@ export const router: IRouter = { } } - if (noneMethodMatched) { + if (!anyMethodMatched) { throw NONE_METHOD_MATCHED_ERROR; } - if (nonePathMatched) { + if (!anyPathMatched) { throw NONE_PATH_MATCHED_ERROR; } - if (noneServerMatched) { + if (!anyServerProvided) { + throw NO_SERVER_CONFIGURATION_PROVIDED_ERROR; + } + + if (!anyServerMatched) { throw NONE_SERVER_MATCHED_ERROR; }