Skip to content

Commit

Permalink
move basePath functionality under namespace
Browse files Browse the repository at this point in the history
  • Loading branch information
mshustov committed May 27, 2019
1 parent 2517c97 commit 9599d32
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 59 deletions.
38 changes: 15 additions & 23 deletions src/core/server/http/http_server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);

Expand All @@ -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: '/',
};
Expand All @@ -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(
Expand Down
49 changes: 23 additions & 26 deletions src/core/server/http/http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -72,17 +77,10 @@ export interface HttpServerSetup {

export class HttpServer {
private server?: Server;
private registeredRouters = new Set<Router>();
private authRegistered = false;
private basePathCache = new WeakMap<
ReturnType<KibanaRequest['unstable_getIncomingMessage']>,
string
>();

private authState = new WeakMap<
ReturnType<KibanaRequest['unstable_getIncomingMessage']>,
unknown
>();
private readonly registeredRouters = new Set<Router>();
private readonly basePathCache = new WeakMap<ReturnType<typeof requestToKey>, string>();
private readonly authState = new WeakMap<ReturnType<typeof requestToKey>, unknown>();

constructor(private readonly log: Logger) {}

Expand All @@ -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 {
Expand All @@ -136,8 +132,10 @@ export class HttpServer {
fn: AuthenticationHandler<T>,
cookieOptions: SessionStorageCookieOptions<T>
) => 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),
Expand Down Expand Up @@ -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');
Expand All @@ -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
Expand Down
6 changes: 4 additions & 2 deletions src/core/server/http/http_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
7 changes: 4 additions & 3 deletions src/core/server/http/integration_tests/http_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -329,7 +330,7 @@ describe('http service', () => {
});
});

describe('#getBasePathFor()/#setBasePathFor()', () => {
describe('#basePath()', () => {
let root: ReturnType<typeof kbnTestServer.createRoot>;
beforeEach(async () => {
root = kbnTestServer.createRoot();
Expand All @@ -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();
});

Expand All @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions src/core/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
};
}

Expand Down
3 changes: 1 addition & 2 deletions src/core/server/plugins/plugin_context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,7 @@ export function createPluginSetupContext<TPlugin, TPluginDependencies>(
registerOnPreAuth: deps.http.registerOnPreAuth,
registerAuth: deps.http.registerAuth,
registerOnPostAuth: deps.http.registerOnPostAuth,
getBasePathFor: deps.http.getBasePathFor,
setBasePathFor: deps.http.setBasePathFor,
basePath: deps.http.basePath,
},
};
}
Expand Down
2 changes: 1 addition & 1 deletion src/legacy/server/http/setup_base_path_provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}

0 comments on commit 9599d32

Please sign in to comment.