From 9599d328013b0ba734294959b90454eeeb9382f3 Mon Sep 17 00:00:00 2001 From: restrry Date: Mon, 27 May 2019 15:11:55 +0200 Subject: [PATCH] move basePath functionality under namespace --- src/core/server/http/http_server.test.ts | 38 ++++++-------- src/core/server/http/http_server.ts | 49 +++++++++---------- src/core/server/http/http_service.mock.ts | 6 ++- .../integration_tests/http_service.test.ts | 7 +-- src/core/server/index.ts | 3 +- src/core/server/plugins/plugin_context.ts | 3 +- .../server/http/setup_base_path_provider.js | 2 +- 7 files changed, 49 insertions(+), 59 deletions(-) diff --git a/src/core/server/http/http_server.test.ts b/src/core/server/http/http_server.test.ts index 0f1a85a336b048..aacd8f11c6929c 100644 --- a/src/core/server/http/http_server.test.ts +++ b/src/core/server/http/http_server.test.ts @@ -615,23 +615,19 @@ test('throws an error if starts without set up', async () => { ); }); -test('#getBasePathFor() returns base path associated with an incoming request', async () => { - const { - getBasePathFor, - setBasePathFor, - registerRouter, - server: innerServer, - registerOnPostAuth, - } = await server.setup(config); +test('#basePath.get() returns base path associated with an incoming request', async () => { + const { basePath, registerRouter, server: innerServer, registerOnPostAuth } = await server.setup( + config + ); const path = '/base-path'; registerOnPostAuth((req, t) => { - setBasePathFor(req, path); + basePath.set(req, path); return t.next(); }); const router = new Router('/'); - router.get({ path: '/', validate: false }, (req, res) => res.ok({ key: getBasePathFor(req) })); + router.get({ path: '/', validate: false }, (req, res) => res.ok({ key: basePath.get(req) })); registerRouter(router); await server.start(config); @@ -643,28 +639,24 @@ test('#getBasePathFor() returns base path associated with an incoming request', }); }); -test('#getBasePathFor() is based on server base path', async () => { +test('#basePath.get() is based on server base path', async () => { const configWithBasePath = { ...config, basePath: '/bar', }; - const { - getBasePathFor, - setBasePathFor, - registerRouter, - server: innerServer, - registerOnPostAuth, - } = await server.setup(configWithBasePath); + const { basePath, registerRouter, server: innerServer, registerOnPostAuth } = await server.setup( + configWithBasePath + ); const path = '/base-path'; registerOnPostAuth((req, t) => { - setBasePathFor(req, path); + basePath.set(req, path); return t.next(); }); const router = new Router('/'); router.get({ path: '/', validate: false }, async (req, res) => - res.ok({ key: getBasePathFor(req) }) + res.ok({ key: basePath.get(req) }) ); registerRouter(router); @@ -677,7 +669,7 @@ test('#getBasePathFor() is based on server base path', async () => { }); }); -test('#setBasePathFor() cannot be set twice for one request', async () => { +test('#basePath.set() cannot be set twice for one request', async () => { const incomingMessage = { url: '/', }; @@ -699,9 +691,9 @@ test('#setBasePathFor() cannot be set twice for one request', async () => { KibanaRequest: jest.fn(() => kibanaRequestFactory), })); - const { setBasePathFor } = await server.setup(config); + const { basePath } = await server.setup(config); - const setPath = () => setBasePathFor(kibanaRequestFactory.from(), '/path'); + const setPath = () => basePath.set(kibanaRequestFactory.from(), '/path'); setPath(); expect(setPath).toThrowErrorMatchingInlineSnapshot( diff --git a/src/core/server/http/http_server.ts b/src/core/server/http/http_server.ts index 1c7f18c750ec05..eb2ce1cc698606 100644 --- a/src/core/server/http/http_server.ts +++ b/src/core/server/http/http_server.ts @@ -38,6 +38,9 @@ enum AuthStatus { unknown = 'unknown', } +const requestToKey = (request: KibanaRequest | Request) => + request instanceof KibanaRequest ? request.unstable_getIncomingMessage() : request.raw.req; + export interface HttpServerSetup { server: Server; options: ServerOptions; @@ -57,8 +60,10 @@ export interface HttpServerSetup { */ registerOnPreAuth: (requestHandler: OnPreAuthHandler) => void; registerOnPostAuth: (requestHandler: OnPostAuthHandler) => void; - getBasePathFor: (request: KibanaRequest | Request) => string; - setBasePathFor: (request: KibanaRequest | Request, basePath: string) => void; + basePath: { + get: (request: KibanaRequest | Request) => string; + set: (request: KibanaRequest | Request, basePath: string) => void; + }; auth: { get: ( request: KibanaRequest | Request @@ -72,17 +77,10 @@ export interface HttpServerSetup { export class HttpServer { private server?: Server; - private registeredRouters = new Set(); private authRegistered = false; - private basePathCache = new WeakMap< - ReturnType, - string - >(); - - private authState = new WeakMap< - ReturnType, - unknown - >(); + private readonly registeredRouters = new Set(); + private readonly basePathCache = new WeakMap, string>(); + private readonly authState = new WeakMap, unknown>(); constructor(private readonly log: Logger) {} @@ -101,24 +99,22 @@ export class HttpServer { // passing hapi Request works for BWC. can be deleted once we remove legacy server. private getBasePathFor(config: HttpConfig, request: KibanaRequest | Request) { - const incomingMessage = - request instanceof KibanaRequest ? request.unstable_getIncomingMessage() : request.raw.req; + const key = requestToKey(request); - const requestScopePath = this.basePathCache.get(incomingMessage) || ''; + const requestScopePath = this.basePathCache.get(key) || ''; const serverBasePath = config.basePath || ''; return `${serverBasePath}${requestScopePath}`; } // should work only for KibanaRequest as soon as spaces migrate to NP private setBasePathFor(request: KibanaRequest | Request, basePath: string) { - const incomingMessage = - request instanceof KibanaRequest ? request.unstable_getIncomingMessage() : request.raw.req; - if (this.basePathCache.has(incomingMessage)) { + const key = requestToKey(request); + if (this.basePathCache.has(key)) { throw new Error( 'Request basePath was previously set. Setting multiple times is not supported.' ); } - this.basePathCache.set(incomingMessage, basePath); + this.basePathCache.set(key, basePath); } public setup(config: HttpConfig): HttpServerSetup { @@ -136,8 +132,10 @@ export class HttpServer { fn: AuthenticationHandler, cookieOptions: SessionStorageCookieOptions ) => this.registerAuth(fn, cookieOptions, config.basePath), - getBasePathFor: this.getBasePathFor.bind(this, config), - setBasePathFor: this.setBasePathFor.bind(this), + basePath: { + get: this.getBasePathFor.bind(this, config), + set: this.setBasePathFor.bind(this), + }, auth: { get: this.getAuthData.bind(this), isAuthenticated: this.isAuthenticated.bind(this), @@ -255,7 +253,7 @@ export class HttpServer { this.server.auth.scheme('login', () => ({ authenticate: adoptToHapiAuthFormat(fn, sessionStorage, (req, state) => { - this.authState.set(req.raw.req, state); + this.authState.set(requestToKey(req), state); }), })); this.server.auth.strategy('session', 'login'); @@ -267,11 +265,10 @@ export class HttpServer { this.server.auth.default('session'); } private getAuthData(request: KibanaRequest | Request) { - const incomingMessage = - request instanceof KibanaRequest ? request.unstable_getIncomingMessage() : request.raw.req; + const key = requestToKey(request); - const hasState = this.authState.has(incomingMessage); - const state = this.authState.get(incomingMessage); + const hasState = this.authState.has(key); + const state = this.authState.get(key); const status: AuthStatus = hasState ? AuthStatus.authenticated : this.authRegistered diff --git a/src/core/server/http/http_service.mock.ts b/src/core/server/http/http_service.mock.ts index 6d49ac3010ef02..6c5267a3abc4b1 100644 --- a/src/core/server/http/http_service.mock.ts +++ b/src/core/server/http/http_service.mock.ts @@ -27,10 +27,12 @@ const createSetupContractMock = () => { registerAuth: jest.fn(), registerOnPostAuth: jest.fn(), registerRouter: jest.fn(), - getBasePathFor: jest.fn(), - setBasePathFor: jest.fn(), // we can mock some hapi server method when we need it server: {} as Server, + basePath: { + get: jest.fn(), + set: jest.fn(), + }, auth: { get: jest.fn(), isAuthenticated: jest.fn(), diff --git a/src/core/server/http/integration_tests/http_service.test.ts b/src/core/server/http/integration_tests/http_service.test.ts index 93fe20a80e1209..9d5068d3a874f3 100644 --- a/src/core/server/http/integration_tests/http_service.test.ts +++ b/src/core/server/http/integration_tests/http_service.test.ts @@ -18,6 +18,7 @@ */ import request from 'request'; import Boom from 'boom'; +import { Request } from 'hapi'; import { AuthenticationHandler } from '../../../../core/server'; import { Router } from '../router'; @@ -329,7 +330,7 @@ describe('http service', () => { }); }); - describe('#getBasePathFor()/#setBasePathFor()', () => { + describe('#basePath()', () => { let root: ReturnType; beforeEach(async () => { root = kbnTestServer.createRoot(); @@ -340,7 +341,7 @@ describe('http service', () => { const reqBasePath = '/requests-specific-base-path'; const { http } = await root.setup(); http.registerOnPreAuth((req, t) => { - http.setBasePathFor(req, reqBasePath); + http.basePath.set(req, reqBasePath); return t.next(); }); @@ -351,7 +352,7 @@ describe('http service', () => { kbnServer.server.route({ method: 'GET', path: legacyUrl, - handler: kbnServer.newPlatform.setup.core.http.getBasePathFor, + handler: (req: Request) => kbnServer.newPlatform.setup.core.http.basePath.get(req), }); await kbnTestServer.request.get(root, legacyUrl).expect(200, reqBasePath); diff --git a/src/core/server/index.ts b/src/core/server/index.ts index e144c0f2568f8b..cee054cc9bc4c4 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -84,8 +84,7 @@ export interface CoreSetup { registerOnPreAuth: HttpServiceSetup['registerOnPreAuth']; registerAuth: HttpServiceSetup['registerAuth']; registerOnPostAuth: HttpServiceSetup['registerOnPostAuth']; - getBasePathFor: HttpServiceSetup['getBasePathFor']; - setBasePathFor: HttpServiceSetup['setBasePathFor']; + basePath: HttpServiceSetup['basePath']; }; } diff --git a/src/core/server/plugins/plugin_context.ts b/src/core/server/plugins/plugin_context.ts index b2a88bbd32760c..215f334bffab4d 100644 --- a/src/core/server/plugins/plugin_context.ts +++ b/src/core/server/plugins/plugin_context.ts @@ -120,8 +120,7 @@ export function createPluginSetupContext( registerOnPreAuth: deps.http.registerOnPreAuth, registerAuth: deps.http.registerAuth, registerOnPostAuth: deps.http.registerOnPostAuth, - getBasePathFor: deps.http.getBasePathFor, - setBasePathFor: deps.http.setBasePathFor, + basePath: deps.http.basePath, }, }; } diff --git a/src/legacy/server/http/setup_base_path_provider.js b/src/legacy/server/http/setup_base_path_provider.js index 8cf6cc1fde512d..10f79691870085 100644 --- a/src/legacy/server/http/setup_base_path_provider.js +++ b/src/legacy/server/http/setup_base_path_provider.js @@ -25,6 +25,6 @@ export function setupBasePathProvider(kbnServer) { kbnServer.server.decorate('request', 'getBasePath', function () { const request = this; - return kbnServer.newPlatform.setup.core.http.getBasePathFor(request); + return kbnServer.newPlatform.setup.core.http.basePath.get(request); }); }