From 4eef5fbce23c706dd804d7165e9111ba88fe4575 Mon Sep 17 00:00:00 2001 From: restrry Date: Thu, 18 Apr 2019 11:57:53 +0200 Subject: [PATCH 01/20] introduce start phase. setup is bloated with start functionality --- src/core/public/plugins/plugins_service.ts | 2 + src/core/server/bootstrap.ts | 1 + .../elasticsearch/elasticsearch_service.ts | 2 + src/core/server/http/http_server.ts | 36 +++++++---- src/core/server/http/http_service.ts | 29 ++++----- src/core/server/http/index.ts | 2 +- src/core/server/index.ts | 8 ++- src/core/server/legacy/index.ts | 2 +- src/core/server/legacy/legacy_service.ts | 29 +++++---- src/core/server/plugins/plugins_service.ts | 2 + src/core/server/root/index.ts | 11 ++++ src/core/server/server.ts | 59 ++++++++++++------- src/core/types/core_service.ts | 3 +- src/legacy/server/kbn_server.d.ts | 8 ++- src/legacy/server/kbn_server.js | 13 ++-- 15 files changed, 134 insertions(+), 73 deletions(-) diff --git a/src/core/public/plugins/plugins_service.ts b/src/core/public/plugins/plugins_service.ts index 0a157067aef55..1ec0e6f75a76d 100644 --- a/src/core/public/plugins/plugins_service.ts +++ b/src/core/public/plugins/plugins_service.ts @@ -98,6 +98,8 @@ export class PluginsService implements CoreService { return { contracts }; } + public async start() {} + public async stop() { // Stop plugins in reverse topological order. for (const pluginName of this.satupPlugins.reverse()) { diff --git a/src/core/server/bootstrap.ts b/src/core/server/bootstrap.ts index 25ac744790e79..94df667788760 100644 --- a/src/core/server/bootstrap.ts +++ b/src/core/server/bootstrap.ts @@ -84,6 +84,7 @@ export async function bootstrap({ try { await root.setup(); + await root.start(); } catch (err) { await shutdown(err); } diff --git a/src/core/server/elasticsearch/elasticsearch_service.ts b/src/core/server/elasticsearch/elasticsearch_service.ts index cd0af14b3cf37..4ede7d6af1453 100644 --- a/src/core/server/elasticsearch/elasticsearch_service.ts +++ b/src/core/server/elasticsearch/elasticsearch_service.ts @@ -106,6 +106,8 @@ export class ElasticsearchService implements CoreService void; /** * Define custom authentication and/or authorization mechanism for incoming requests. * Applied to all resources by default. Only one AuthenticationHandler can be registered. @@ -50,6 +50,10 @@ export interface HttpServerInfo { registerOnRequest: (requestHandler: OnRequestHandler) => void; } +export interface HttpServerInfo { + server: Server; +} + export class HttpServer { private server?: Server; private registeredRouters = new Set(); @@ -66,15 +70,31 @@ export class HttpServer { throw new Error('Routers can be registered only when HTTP server is stopped.'); } + this.log.debug(`registering route handler for [${router.path}]`); this.registeredRouters.add(router); } - public async start(config: HttpConfig): Promise { - this.log.debug('starting http server'); - + public setup(config: HttpConfig): HttpServerSetup { const serverOptions = getServerOptions(config); this.server = createServer(serverOptions); + return { + options: serverOptions, + registerRouter: this.registerRouter.bind(this), + registerOnRequest: this.registerOnRequest.bind(this), + registerAuth: ( + fn: AuthenticationHandler, + cookieOptions: SessionStorageCookieOptions + ) => this.registerAuth(fn, cookieOptions, config.basePath), + }; + } + + public async start(config: HttpConfig): Promise { + if (this.server === undefined) { + throw new Error('server is not setup up yet'); + } + this.log.debug('starting http server'); + this.setupBasePathRewrite(this.server, config); for (const router of this.registeredRouters) { @@ -100,12 +120,6 @@ export class HttpServer { // needed anymore we shouldn't return anything from this method. return { server: this.server, - options: serverOptions, - registerOnRequest: this.registerOnRequest.bind(this), - registerAuth: ( - fn: AuthenticationHandler, - cookieOptions: SessionStorageCookieOptions - ) => this.registerAuth(fn, cookieOptions, config.basePath), }; } diff --git a/src/core/server/http/http_service.ts b/src/core/server/http/http_service.ts index c136a7b20b361..ae1aabef3ea37 100644 --- a/src/core/server/http/http_service.ts +++ b/src/core/server/http/http_service.ts @@ -23,15 +23,15 @@ import { first } from 'rxjs/operators'; import { CoreService } from '../../types'; import { Logger, LoggerFactory } from '../logging'; import { HttpConfig } from './http_config'; -import { HttpServer, HttpServerInfo } from './http_server'; +import { HttpServer, HttpServerSetup, HttpServerInfo } from './http_server'; import { HttpsRedirectServer } from './https_redirect_server'; -import { Router } from './router'; /** @public */ -export type HttpServiceSetup = HttpServerInfo; +export type HttpServiceSetup = HttpServerSetup; +export type HttpServiceStart = HttpServerInfo; /** @internal */ -export class HttpService implements CoreService { +export class HttpService implements CoreService { private readonly httpServer: HttpServer; private readonly httpsRedirectServer: HttpsRedirectServer; private configSubscription?: Subscription; @@ -58,6 +58,12 @@ export class HttpService implements CoreService { const config = await this.config$.pipe(first()).toPromise(); + return this.httpServer.setup(config); + } + + public async start() { + const config = await this.config$.pipe(first()).toPromise(); + // If a redirect port is specified, we start an HTTP server at this port and // redirect all requests to the SSL port. if (config.ssl.enabled && config.ssl.redirectHttpFromPort !== undefined) { @@ -81,19 +87,4 @@ export class HttpService implements CoreService { await this.httpServer.stop(); await this.httpsRedirectServer.stop(); } - - public registerRouter(router: Router): void { - if (this.httpServer.isListening()) { - // If the server is already running we can't make any config changes - // to it, so we warn and don't allow the config to pass through. - // TODO Should we throw instead? - this.log.error( - `Received new router [${router.path}] after server was started. ` + - 'Router will **not** be applied.' - ); - } else { - this.log.debug(`registering route handler for [${router.path}]`); - this.httpServer.registerRouter(router); - } - } } diff --git a/src/core/server/http/index.ts b/src/core/server/http/index.ts index 9457a3fad3c3c..d0595c7415e05 100644 --- a/src/core/server/http/index.ts +++ b/src/core/server/http/index.ts @@ -18,7 +18,7 @@ */ export { HttpConfig } from './http_config'; -export { HttpService, HttpServiceSetup } from './http_service'; +export { HttpService, HttpServiceSetup, HttpServiceStart } from './http_service'; export { Router, KibanaRequest } from './router'; export { HttpServerInfo } from './http_server'; export { BasePathProxyServer } from './base_path_proxy_server'; diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 535b4f4749aa3..1aa602e32c384 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -17,7 +17,7 @@ * under the License. */ import { ElasticsearchServiceSetup } from './elasticsearch'; -import { HttpServiceSetup } from './http'; +import { HttpServiceSetup, HttpServiceStart } from './http'; import { PluginsServiceSetup } from './plugins'; export { bootstrap } from './bootstrap'; @@ -56,4 +56,8 @@ export interface CoreSetup { plugins: PluginsServiceSetup; } -export { HttpServiceSetup, ElasticsearchServiceSetup, PluginsServiceSetup }; +export interface CoreStart { + http: HttpServiceStart; +} + +export { HttpServiceSetup, HttpServiceStart, ElasticsearchServiceSetup, PluginsServiceSetup }; diff --git a/src/core/server/legacy/index.ts b/src/core/server/legacy/index.ts index bb965c0272d14..74c10a83ba7d1 100644 --- a/src/core/server/legacy/index.ts +++ b/src/core/server/legacy/index.ts @@ -20,4 +20,4 @@ /** @internal */ export { LegacyObjectToConfigAdapter } from './config/legacy_object_to_config_adapter'; /** @internal */ -export { LegacyService } from './legacy_service'; +export { LegacyService, SetupDeps } from './legacy_service'; diff --git a/src/core/server/legacy/legacy_service.ts b/src/core/server/legacy/legacy_service.ts index ffbb0f1edf711..46f7dd66787a9 100644 --- a/src/core/server/legacy/legacy_service.ts +++ b/src/core/server/legacy/legacy_service.ts @@ -25,7 +25,7 @@ import { Config } from '../config'; import { CoreContext } from '../core_context'; import { DevConfig } from '../dev'; import { ElasticsearchServiceSetup } from '../elasticsearch'; -import { BasePathProxyServer, HttpConfig, HttpServiceSetup } from '../http'; +import { BasePathProxyServer, HttpConfig, HttpServiceSetup, HttpServiceStart } from '../http'; import { Logger } from '../logging'; import { PluginsServiceSetup } from '../plugins/plugins_service'; import { LegacyPlatformProxy } from './legacy_platform_proxy'; @@ -37,12 +37,16 @@ interface LegacyKbnServer { close: () => Promise; } -interface SetupDeps { +export interface SetupDeps { elasticsearch: ElasticsearchServiceSetup; - http?: HttpServiceSetup; + http: HttpServiceSetup; plugins: PluginsServiceSetup; } +export interface StartDeps { + http?: HttpServiceStart; +} + function getLegacyRawConfig(config: Config) { const rawConfig = config.toRaw(); @@ -64,8 +68,8 @@ export class LegacyService implements CoreService { constructor(private readonly coreContext: CoreContext) { this.log = coreContext.logger.get('legacy-service'); } - - public async setup(deps: SetupDeps) { + public async setup() {} + public async start(setupDeps: SetupDeps, startDeps: StartDeps) { this.log.debug('setting up legacy service'); const update$ = this.coreContext.configService.getConfig$().pipe( @@ -89,7 +93,7 @@ export class LegacyService implements CoreService { await this.createClusterManager(config); return; } - return await this.createKbnServer(config, deps); + return await this.createKbnServer(config, setupDeps, startDeps); }) ) .toPromise(); @@ -130,7 +134,7 @@ export class LegacyService implements CoreService { ); } - private async createKbnServer(config: Config, { elasticsearch, http, plugins }: SetupDeps) { + private async createKbnServer(config: Config, setupDeps: SetupDeps, startDeps: StartDeps) { // eslint-disable-next-line @typescript-eslint/no-var-requires const KbnServer = require('../../../legacy/server/kbn_server'); const kbnServer: LegacyKbnServer = new KbnServer(getLegacyRawConfig(config), { @@ -140,16 +144,15 @@ export class LegacyService implements CoreService { // managed by ClusterManager or optimizer) then we won't have that info, // so we can't start "legacy" server either. serverOptions: - http !== undefined + startDeps.http !== undefined ? { - ...http.options, - listener: this.setupProxyListener(http.server), + ...setupDeps.http.options, + listener: this.setupProxyListener(startDeps.http.server), } : { autoListen: false }, handledConfigPaths: await this.coreContext.configService.getUsedPaths(), - http, - elasticsearch, - plugins, + setupDeps, + startDeps, }); // The kbnWorkerType check is necessary to prevent the repl diff --git a/src/core/server/plugins/plugins_service.ts b/src/core/server/plugins/plugins_service.ts index 5be42727159dc..df639c8de7bd5 100644 --- a/src/core/server/plugins/plugins_service.ts +++ b/src/core/server/plugins/plugins_service.ts @@ -80,6 +80,8 @@ export class PluginsService implements CoreService { }; } + public async start() {} + public async stop() { this.log.debug('Stopping plugins service'); await this.pluginsSystem.stopPlugins(); diff --git a/src/core/server/root/index.ts b/src/core/server/root/index.ts index 00c77550326ef..dabc0c22213cc 100644 --- a/src/core/server/root/index.ts +++ b/src/core/server/root/index.ts @@ -60,6 +60,17 @@ export class Root { } } + public async start() { + this.log.debug('starting root'); + + try { + await this.server.start(); + } catch (e) { + await this.shutdown(e); + throw e; + } + } + public async shutdown(reason?: any) { this.log.debug('shutting root down'); diff --git a/src/core/server/server.ts b/src/core/server/server.ts index 08ab584f7d47f..49ca3e2051d6d 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -20,8 +20,8 @@ import { first } from 'rxjs/operators'; import { ConfigService, Env } from './config'; import { ElasticsearchService } from './elasticsearch'; -import { HttpConfig, HttpService, HttpServiceSetup, Router } from './http'; -import { LegacyService } from './legacy'; +import { HttpConfig, HttpService, HttpServiceSetup, HttpServiceStart, Router } from './http'; +import { LegacyService, SetupDeps } from './legacy'; import { Logger, LoggerFactory } from './logging'; import { PluginsService } from './plugins'; @@ -32,6 +32,8 @@ export class Server { private readonly legacy: LegacyService; private readonly log: Logger; + private setupDeps?: SetupDeps; + constructor( private readonly configService: ConfigService, logger: LoggerFactory, @@ -40,9 +42,6 @@ export class Server { this.log = logger.get('server'); this.http = new HttpService(configService.atPath('server', HttpConfig), logger); - const router = new Router('/core'); - router.get({ path: '/', validate: false }, async (req, res) => res.ok({ version: '0.0.1' })); - this.http.registerRouter(router); const core = { env, configService, logger }; this.plugins = new PluginsService(core); @@ -53,31 +52,45 @@ export class Server { public async setup() { this.log.debug('setting up server'); - // We shouldn't set up http service in two cases: - // 1. If `server.autoListen` is explicitly set to `false`. - // 2. When the process is run as dev cluster master in which case cluster manager - // will fork a dedicated process where http service will be set up instead. - let httpSetup: HttpServiceSetup | undefined; - const httpConfig = await this.configService - .atPath('server', HttpConfig) - .pipe(first()) - .toPromise(); - if (!this.env.isDevClusterMaster && httpConfig.autoListen) { - httpSetup = await this.http.setup(); - } + const httpSetup = await this.http.setup(); + this.registerDefaultRoute(httpSetup); const elasticsearchServiceSetup = await this.elasticsearch.setup(); - const pluginsSetup = await this.plugins.setup({ elasticsearch: elasticsearchServiceSetup, http: httpSetup, }); - await this.legacy.setup({ + this.setupDeps = { elasticsearch: elasticsearchServiceSetup, http: httpSetup, plugins: pluginsSetup, - }); + }; + return this.setupDeps; + } + + public async start() { + const httpConfig = await this.configService + .atPath('server', HttpConfig) + .pipe(first()) + .toPromise(); + + let httpStart: HttpServiceStart | undefined; + // We shouldn't set up http service in two cases: + // 1. If `server.autoListen` is explicitly set to `false`. + // 2. When the process is run as dev cluster master in which case cluster manager + // will fork a dedicated process where http service will be set up instead. + if (!this.env.isDevClusterMaster && httpConfig.autoListen) { + httpStart = await this.http.start(); + } + + const startDeps = { + http: httpStart, + }; + + await this.legacy.start(this.setupDeps!, startDeps); + + return startDeps; } public async stop() { @@ -88,4 +101,10 @@ export class Server { await this.elasticsearch.stop(); await this.http.stop(); } + + private registerDefaultRoute(httpSetup: HttpServiceSetup) { + const router = new Router('/core'); + router.get({ path: '/', validate: false }, async (req, res) => res.ok({ version: '0.0.1' })); + httpSetup.registerRouter(router); + } } diff --git a/src/core/types/core_service.ts b/src/core/types/core_service.ts index 44f9e4e36b813..b99c419b796ba 100644 --- a/src/core/types/core_service.ts +++ b/src/core/types/core_service.ts @@ -18,7 +18,8 @@ */ /** @internal */ -export interface CoreService { +export interface CoreService { setup(...params: any[]): Promise; + start(...params: any[]): Promise; stop(): Promise; } diff --git a/src/legacy/server/kbn_server.d.ts b/src/legacy/server/kbn_server.d.ts index 2a05ec166d52d..978c41f62c02a 100644 --- a/src/legacy/server/kbn_server.d.ts +++ b/src/legacy/server/kbn_server.d.ts @@ -22,6 +22,7 @@ import { ResponseObject, Server } from 'hapi'; import { ElasticsearchServiceSetup, HttpServiceSetup, + HttpServiceStart, ConfigService, PluginsServiceSetup, } from '../../core/server'; @@ -77,10 +78,15 @@ export default class KbnServer { setup: { core: { elasticsearch: ElasticsearchServiceSetup; - http?: HttpServiceSetup; + http: HttpServiceSetup; }; plugins: PluginsServiceSetup; }; + start: { + core: { + http?: HttpServiceStart; + }; + }; stop: null; params: { serverOptions: ElasticsearchServiceSetup; diff --git a/src/legacy/server/kbn_server.js b/src/legacy/server/kbn_server.js index c81e7382e1bb7..e467088542ffe 100644 --- a/src/legacy/server/kbn_server.js +++ b/src/legacy/server/kbn_server.js @@ -54,14 +54,19 @@ export default class KbnServer { this.rootDir = rootDir; this.settings = settings || {}; - const { plugins, http, elasticsearch, serverOptions, handledConfigPaths } = core; + const { setupDeps, startDeps, serverOptions, handledConfigPaths } = core; this.newPlatform = { setup: { core: { - elasticsearch, - http, + elasticsearch: setupDeps.elasticsearch, + http: setupDeps.http, + }, + plugins: setupDeps.plugins, + }, + start: { + core: { + http: startDeps.http, }, - plugins, }, stop: null, params: { From f116a293803b2dcd091fdee4b3fd54fb3d733588 Mon Sep 17 00:00:00 2001 From: restrry Date: Thu, 18 Apr 2019 11:58:52 +0200 Subject: [PATCH 02/20] fix amp typings: server is part of start contract now --- src/core/server/http/http_server.ts | 2 +- x-pack/plugins/apm/index.ts | 4 ++-- .../server/lib/apm_telemetry/make_apm_usage_collector.ts | 8 ++++---- x-pack/plugins/apm/server/new-platform/plugin.ts | 8 ++++---- .../apm/server/routes/__test__/routeFailures.test.ts | 6 +++--- x-pack/plugins/apm/server/routes/errors.ts | 4 ++-- x-pack/plugins/apm/server/routes/metrics.ts | 4 ++-- x-pack/plugins/apm/server/routes/services.ts | 4 ++-- x-pack/plugins/apm/server/routes/traces.ts | 4 ++-- x-pack/plugins/apm/server/routes/transaction_groups.ts | 4 ++-- 10 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/core/server/http/http_server.ts b/src/core/server/http/http_server.ts index 74cfe87d8fed4..e9f301ca18256 100644 --- a/src/core/server/http/http_server.ts +++ b/src/core/server/http/http_server.ts @@ -65,7 +65,7 @@ export class HttpServer { return this.server !== undefined && this.server.listener.listening; } - public registerRouter(router: Router) { + private registerRouter(router: Router) { if (this.isListening()) { throw new Error('Routers can be registered only when HTTP server is stopped.'); } diff --git a/x-pack/plugins/apm/index.ts b/x-pack/plugins/apm/index.ts index a97c63a51bf1e..9a06bfb5fbf96 100644 --- a/x-pack/plugins/apm/index.ts +++ b/x-pack/plugins/apm/index.ts @@ -7,7 +7,7 @@ import { i18n } from '@kbn/i18n'; import { Server } from 'hapi'; import { resolve } from 'path'; -import { CoreSetup, PluginInitializerContext } from 'src/core/server/index.js'; +import { CoreStart, PluginInitializerContext } from 'src/core/server/index.js'; import mappings from './mappings.json'; import { plugin } from './server/new-platform/index'; @@ -106,7 +106,7 @@ export function apm(kibana: any) { http: { server } - } as CoreSetup; + } as CoreStart; plugin(initializerContext).setup(core); } }); diff --git a/x-pack/plugins/apm/server/lib/apm_telemetry/make_apm_usage_collector.ts b/x-pack/plugins/apm/server/lib/apm_telemetry/make_apm_usage_collector.ts index 8f03bd0b5fbb9..a182ef0c8f17c 100644 --- a/x-pack/plugins/apm/server/lib/apm_telemetry/make_apm_usage_collector.ts +++ b/x-pack/plugins/apm/server/lib/apm_telemetry/make_apm_usage_collector.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { CoreSetup } from 'src/core/server'; +import { CoreStart } from 'src/core/server'; import { APM_TELEMETRY_DOC_ID, createApmTelementry, @@ -12,8 +12,8 @@ import { } from './apm_telemetry'; // TODO this type should be defined by the platform -export interface CoreSetupWithUsageCollector extends CoreSetup { - http: CoreSetup['http'] & { +export interface CoreStartWithUsageCollector extends CoreStart { + http: CoreStart['http'] & { server: { usage: { collectorSet: { @@ -25,7 +25,7 @@ export interface CoreSetupWithUsageCollector extends CoreSetup { }; } -export function makeApmUsageCollector(core: CoreSetupWithUsageCollector) { +export function makeApmUsageCollector(core: CoreStartWithUsageCollector) { const { server } = core.http; const apmUsageCollector = server.usage.collectorSet.makeUsageCollector({ diff --git a/x-pack/plugins/apm/server/new-platform/plugin.ts b/x-pack/plugins/apm/server/new-platform/plugin.ts index 4ad9697033916..7eb23509f0cf2 100644 --- a/x-pack/plugins/apm/server/new-platform/plugin.ts +++ b/x-pack/plugins/apm/server/new-platform/plugin.ts @@ -4,9 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -import { CoreSetup } from 'src/core/server'; +import { CoreStart } from 'src/core/server'; import { makeApmUsageCollector } from '../lib/apm_telemetry'; -import { CoreSetupWithUsageCollector } from '../lib/apm_telemetry/make_apm_usage_collector'; +import { CoreStartWithUsageCollector } from '../lib/apm_telemetry/make_apm_usage_collector'; import { initErrorsApi } from '../routes/errors'; import { initMetricsApi } from '../routes/metrics'; import { initServicesApi } from '../routes/services'; @@ -14,12 +14,12 @@ import { initTracesApi } from '../routes/traces'; import { initTransactionGroupsApi } from '../routes/transaction_groups'; export class Plugin { - public setup(core: CoreSetup) { + public setup(core: CoreStart) { initTransactionGroupsApi(core); initTracesApi(core); initServicesApi(core); initErrorsApi(core); initMetricsApi(core); - makeApmUsageCollector(core as CoreSetupWithUsageCollector); + makeApmUsageCollector(core as CoreStartWithUsageCollector); } } diff --git a/x-pack/plugins/apm/server/routes/__test__/routeFailures.test.ts b/x-pack/plugins/apm/server/routes/__test__/routeFailures.test.ts index 4db63a4ce5886..ace1b75ba88e2 100644 --- a/x-pack/plugins/apm/server/routes/__test__/routeFailures.test.ts +++ b/x-pack/plugins/apm/server/routes/__test__/routeFailures.test.ts @@ -5,7 +5,7 @@ */ import { flatten } from 'lodash'; -import { CoreSetup } from 'src/core/server'; +import { CoreStart } from 'src/core/server'; import { initErrorsApi } from '../errors'; import { initServicesApi } from '../services'; import { initTracesApi } from '../traces'; @@ -13,13 +13,13 @@ import { initTracesApi } from '../traces'; describe('route handlers should fail with a Boom error', () => { let consoleErrorSpy: any; - async function testRouteFailures(init: (core: CoreSetup) => void) { + async function testRouteFailures(init: (core: CoreStart) => void) { const mockServer = { route: jest.fn() }; const mockCore = ({ http: { server: mockServer } - } as unknown) as CoreSetup; + } as unknown) as CoreStart; init(mockCore); expect(mockServer.route).toHaveBeenCalled(); diff --git a/x-pack/plugins/apm/server/routes/errors.ts b/x-pack/plugins/apm/server/routes/errors.ts index 3bd7be240eb4c..45c7193e471bb 100644 --- a/x-pack/plugins/apm/server/routes/errors.ts +++ b/x-pack/plugins/apm/server/routes/errors.ts @@ -7,7 +7,7 @@ import Boom from 'boom'; import Joi from 'joi'; import { Legacy } from 'kibana'; -import { CoreSetup } from 'src/core/server'; +import { CoreStart } from 'src/core/server'; import { getDistribution } from '../lib/errors/distribution/get_distribution'; import { getErrorGroup } from '../lib/errors/get_error_group'; import { getErrorGroups } from '../lib/errors/get_error_groups'; @@ -21,7 +21,7 @@ const defaultErrorHandler = (err: Error) => { throw Boom.boomify(err, { statusCode: 400 }); }; -export function initErrorsApi(core: CoreSetup) { +export function initErrorsApi(core: CoreStart) { const { server } = core.http; server.route({ method: 'GET', diff --git a/x-pack/plugins/apm/server/routes/metrics.ts b/x-pack/plugins/apm/server/routes/metrics.ts index 997ff9b803c09..e68d0100858a3 100644 --- a/x-pack/plugins/apm/server/routes/metrics.ts +++ b/x-pack/plugins/apm/server/routes/metrics.ts @@ -5,7 +5,7 @@ */ import Boom from 'boom'; -import { CoreSetup } from 'src/core/server'; +import { CoreStart } from 'src/core/server'; import { withDefaultValidators } from '../lib/helpers/input_validation'; import { setupRequest } from '../lib/helpers/setup_request'; import { getAllMetricsChartData } from '../lib/metrics/get_all_metrics_chart_data'; @@ -16,7 +16,7 @@ const defaultErrorHandler = (err: Error) => { throw Boom.boomify(err, { statusCode: 400 }); }; -export function initMetricsApi(core: CoreSetup) { +export function initMetricsApi(core: CoreStart) { const { server } = core.http; server.route({ method: 'GET', diff --git a/x-pack/plugins/apm/server/routes/services.ts b/x-pack/plugins/apm/server/routes/services.ts index 0bbc1b8be894b..2ed891626c97c 100644 --- a/x-pack/plugins/apm/server/routes/services.ts +++ b/x-pack/plugins/apm/server/routes/services.ts @@ -5,7 +5,7 @@ */ import Boom from 'boom'; -import { CoreSetup } from 'src/core/server'; +import { CoreStart } from 'src/core/server'; import { AgentName } from '../../typings/es_schemas/ui/fields/Agent'; import { createApmTelementry, storeApmTelemetry } from '../lib/apm_telemetry'; import { withDefaultValidators } from '../lib/helpers/input_validation'; @@ -20,7 +20,7 @@ const defaultErrorHandler = (err: Error) => { throw Boom.boomify(err, { statusCode: 400 }); }; -export function initServicesApi(core: CoreSetup) { +export function initServicesApi(core: CoreStart) { const { server } = core.http; server.route({ method: 'GET', diff --git a/x-pack/plugins/apm/server/routes/traces.ts b/x-pack/plugins/apm/server/routes/traces.ts index 5bc996b1b71cf..4f7bef95fc64b 100644 --- a/x-pack/plugins/apm/server/routes/traces.ts +++ b/x-pack/plugins/apm/server/routes/traces.ts @@ -6,7 +6,7 @@ import Boom from 'boom'; -import { CoreSetup } from 'src/core/server'; +import { CoreStart } from 'src/core/server'; import { withDefaultValidators } from '../lib/helpers/input_validation'; import { setupRequest } from '../lib/helpers/setup_request'; import { getTopTraces } from '../lib/traces/get_top_traces'; @@ -19,7 +19,7 @@ const defaultErrorHandler = (err: Error) => { throw Boom.boomify(err, { statusCode: 400 }); }; -export function initTracesApi(core: CoreSetup) { +export function initTracesApi(core: CoreStart) { const { server } = core.http; // Get trace list diff --git a/x-pack/plugins/apm/server/routes/transaction_groups.ts b/x-pack/plugins/apm/server/routes/transaction_groups.ts index 5c93edf5bd867..8614866d4360a 100644 --- a/x-pack/plugins/apm/server/routes/transaction_groups.ts +++ b/x-pack/plugins/apm/server/routes/transaction_groups.ts @@ -6,7 +6,7 @@ import Boom from 'boom'; import Joi from 'joi'; -import { CoreSetup } from 'src/core/server'; +import { CoreStart } from 'src/core/server'; import { withDefaultValidators } from '../lib/helpers/input_validation'; import { setupRequest } from '../lib/helpers/setup_request'; import { getChartsData } from '../lib/transactions/charts'; @@ -19,7 +19,7 @@ const defaultErrorHandler = (err: Error) => { throw Boom.boomify(err, { statusCode: 400 }); }; -export function initTransactionGroupsApi(core: CoreSetup) { +export function initTransactionGroupsApi(core: CoreStart) { const { server } = core.http; server.route({ From faad24427e81b526a00dbda0b7168efbf36eb5ac Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Thu, 18 Apr 2019 15:53:36 +0200 Subject: [PATCH 03/20] update mock files --- src/core/public/plugins/plugins_service.mock.ts | 1 + .../elasticsearch/elasticsearch_service.mock.ts | 1 + src/core/server/http/http_service.mock.ts | 14 +++++++++++--- src/core/server/index.test.mocks.ts | 4 ++-- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/core/public/plugins/plugins_service.mock.ts b/src/core/public/plugins/plugins_service.mock.ts index d4c6ffb12aaef..398ae6c9adcb8 100644 --- a/src/core/public/plugins/plugins_service.mock.ts +++ b/src/core/public/plugins/plugins_service.mock.ts @@ -31,6 +31,7 @@ type PluginsServiceContract = PublicMethodsOf; const createMock = () => { const mocked: jest.Mocked = { setup: jest.fn(), + start: jest.fn(), stop: jest.fn(), }; diff --git a/src/core/server/elasticsearch/elasticsearch_service.mock.ts b/src/core/server/elasticsearch/elasticsearch_service.mock.ts index 57157adc2fac5..672331199ffa0 100644 --- a/src/core/server/elasticsearch/elasticsearch_service.mock.ts +++ b/src/core/server/elasticsearch/elasticsearch_service.mock.ts @@ -39,6 +39,7 @@ type ElasticsearchServiceContract = PublicMethodsOf; const createMock = () => { const mocked: jest.Mocked = { setup: jest.fn(), + start: jest.fn(), stop: jest.fn(), }; mocked.setup.mockResolvedValue(createSetupContractMock()); diff --git a/src/core/server/http/http_service.mock.ts b/src/core/server/http/http_service.mock.ts index d8bab493a2ed9..c4ca79429dc12 100644 --- a/src/core/server/http/http_service.mock.ts +++ b/src/core/server/http/http_service.mock.ts @@ -22,23 +22,31 @@ import { HttpService } from './http_service'; const createSetupContractMock = () => { const setupContract = { - // we can mock some hapi server method when we need it - server: {} as Server, options: {} as ServerOptions, registerAuth: jest.fn(), registerOnRequest: jest.fn(), + registerRouter: jest.fn(), }; return setupContract; }; +const createStartContractMock = () => { + const startContract = { + // we can mock some hapi server method when we need it + server: {} as Server, + }; + return startContract; +}; + type HttpServiceContract = PublicMethodsOf; const createHttpServiceMock = () => { const mocked: jest.Mocked = { setup: jest.fn(), + start: jest.fn(), stop: jest.fn(), - registerRouter: jest.fn(), }; mocked.setup.mockResolvedValue(createSetupContractMock()); + mocked.start.mockResolvedValue(createStartContractMock()); return mocked; }; diff --git a/src/core/server/index.test.mocks.ts b/src/core/server/index.test.mocks.ts index c25772ea46de4..a3bcaa56da975 100644 --- a/src/core/server/index.test.mocks.ts +++ b/src/core/server/index.test.mocks.ts @@ -23,7 +23,7 @@ jest.doMock('./http/http_service', () => ({ HttpService: jest.fn(() => httpService), })); -export const mockPluginsService = { setup: jest.fn(), stop: jest.fn() }; +export const mockPluginsService = { setup: jest.fn(), start: jest.fn(), stop: jest.fn() }; jest.doMock('./plugins/plugins_service', () => ({ PluginsService: jest.fn(() => mockPluginsService), })); @@ -34,7 +34,7 @@ jest.doMock('./elasticsearch/elasticsearch_service', () => ({ ElasticsearchService: jest.fn(() => elasticsearchService), })); -export const mockLegacyService = { setup: jest.fn(), stop: jest.fn() }; +export const mockLegacyService = { setup: jest.fn(), start: jest.fn(), stop: jest.fn() }; jest.mock('./legacy/legacy_service', () => ({ LegacyService: jest.fn(() => mockLegacyService), })); From a037fe8c96d4d9d2e9338c148a9fff5105153db7 Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Thu, 18 Apr 2019 15:58:43 +0200 Subject: [PATCH 04/20] root.start(): necessary to run test server --- .../server/http/integration_tests/max_payload_size.test.js | 1 + src/legacy/server/http/integration_tests/version_check.test.js | 1 + src/legacy/server/http/integration_tests/xsrf.test.js | 1 + 3 files changed, 3 insertions(+) diff --git a/src/legacy/server/http/integration_tests/max_payload_size.test.js b/src/legacy/server/http/integration_tests/max_payload_size.test.js index fb6b911055a98..53f77d4df74f5 100644 --- a/src/legacy/server/http/integration_tests/max_payload_size.test.js +++ b/src/legacy/server/http/integration_tests/max_payload_size.test.js @@ -24,6 +24,7 @@ beforeAll(async () => { root = kbnTestServer.createRoot({ server: { maxPayloadBytes: 100 } }); await root.setup(); + await root.start(); kbnTestServer.getKbnServer(root).server.route({ path: '/payload_size_check/test/route', diff --git a/src/legacy/server/http/integration_tests/version_check.test.js b/src/legacy/server/http/integration_tests/version_check.test.js index 2594ce66644de..96a78bfe871d2 100644 --- a/src/legacy/server/http/integration_tests/version_check.test.js +++ b/src/legacy/server/http/integration_tests/version_check.test.js @@ -31,6 +31,7 @@ describe('version_check request filter', function () { root = kbnTestServer.createRoot(); await root.setup(); + await root.start(); kbnTestServer.getKbnServer(root).server.route({ path: '/version_check/test/route', diff --git a/src/legacy/server/http/integration_tests/xsrf.test.js b/src/legacy/server/http/integration_tests/xsrf.test.js index a2531ca3027bc..562a94e198631 100644 --- a/src/legacy/server/http/integration_tests/xsrf.test.js +++ b/src/legacy/server/http/integration_tests/xsrf.test.js @@ -39,6 +39,7 @@ describe('xsrf request filter', () => { }); await root.setup(); + await root.start(); const kbnServer = kbnTestServer.getKbnServer(root); kbnServer.server.route({ From 25aa5a56abebb514077aeabbf9cd00430877dd04 Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Thu, 18 Apr 2019 16:02:13 +0200 Subject: [PATCH 05/20] expose setup&start server api to simplify testing --- .../http/integration_tests/http_service.test.ts | 13 +++++++------ src/core/server/root/index.ts | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) 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 199e103a3ed92..ac2e5ebbe5cbe 100644 --- a/src/core/server/http/integration_tests/http_service.test.ts +++ b/src/core/server/http/integration_tests/http_service.test.ts @@ -43,10 +43,10 @@ describe('http service', () => { router.get({ path: authUrl.auth, validate: false }, async (req, res) => res.ok({ content: 'ok' }) ); - // TODO fix me when registerRouter is available before HTTP server is run - (root as any).server.http.registerRouter(router); - await root.setup(); + const { http } = await root.setup(); + http.registerRouter(router); + await root.start(); }, 30000); afterAll(async () => await root.shutdown()); @@ -129,10 +129,11 @@ describe('http service', () => { [onReqUrl.root, onReqUrl.independentReq].forEach(url => router.get({ path: url, validate: false }, async (req, res) => res.ok({ content: 'ok' })) ); - // TODO fix me when registerRouter is available before HTTP server is run - (root as any).server.http.registerRouter(router); - await root.setup(); + const { http } = await root.setup(); + http.registerRouter(router); + + await root.start(); }, 30000); afterAll(async () => await root.shutdown()); diff --git a/src/core/server/root/index.ts b/src/core/server/root/index.ts index dabc0c22213cc..d3125543f4189 100644 --- a/src/core/server/root/index.ts +++ b/src/core/server/root/index.ts @@ -53,7 +53,7 @@ export class Root { try { await this.setupLogging(); - await this.server.setup(); + return await this.server.setup(); } catch (e) { await this.shutdown(e); throw e; @@ -64,7 +64,7 @@ export class Root { this.log.debug('starting root'); try { - await this.server.start(); + return await this.server.start(); } catch (e) { await this.shutdown(e); throw e; From 7ffd2aac3baa2b7f153dc9356e2aeeefa5492fe8 Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Thu, 18 Apr 2019 16:13:20 +0200 Subject: [PATCH 06/20] move tests to the new API --- src/core/server/http/http_server.test.ts | 60 ++++++++----- src/core/server/http/http_service.test.ts | 45 ++++------ src/core/server/legacy/legacy_service.test.ts | 89 +++++++++++-------- src/core/server/server.test.ts | 47 ++++++---- 4 files changed, 140 insertions(+), 101 deletions(-) diff --git a/src/core/server/http/http_server.test.ts b/src/core/server/http/http_server.test.ts index b8b740796a932..4ccc5b733cf49 100644 --- a/src/core/server/http/http_server.test.ts +++ b/src/core/server/http/http_server.test.ts @@ -56,6 +56,7 @@ afterEach(async () => { test('listening after started', async () => { expect(server.isListening()).toBe(false); + await server.setup(config); await server.start(config); expect(server.isListening()).toBe(true); @@ -68,8 +69,8 @@ test('200 OK with body', async () => { return res.ok({ key: 'value' }); }); - server.registerRouter(router); - + const { registerRouter } = await server.setup(config); + registerRouter(router); const { server: innerServer } = await server.start(config); await supertest(innerServer.listener) @@ -87,7 +88,8 @@ test('202 Accepted with body', async () => { return res.accepted({ location: 'somewhere' }); }); - server.registerRouter(router); + const { registerRouter } = await server.setup(config); + registerRouter(router); const { server: innerServer } = await server.start(config); @@ -106,7 +108,8 @@ test('204 No content', async () => { return res.noContent(); }); - server.registerRouter(router); + const { registerRouter } = await server.setup(config); + registerRouter(router); const { server: innerServer } = await server.start(config); @@ -127,7 +130,8 @@ test('400 Bad request with error', async () => { return res.badRequest(err); }); - server.registerRouter(router); + const { registerRouter } = await server.setup(config); + registerRouter(router); const { server: innerServer } = await server.start(config); @@ -156,7 +160,8 @@ test('valid params', async () => { } ); - server.registerRouter(router); + const { registerRouter } = await server.setup(config); + registerRouter(router); const { server: innerServer } = await server.start(config); @@ -185,7 +190,8 @@ test('invalid params', async () => { } ); - server.registerRouter(router); + const { registerRouter } = await server.setup(config); + registerRouter(router); const { server: innerServer } = await server.start(config); @@ -217,7 +223,8 @@ test('valid query', async () => { } ); - server.registerRouter(router); + const { registerRouter } = await server.setup(config); + registerRouter(router); const { server: innerServer } = await server.start(config); @@ -246,7 +253,8 @@ test('invalid query', async () => { } ); - server.registerRouter(router); + const { registerRouter } = await server.setup(config); + registerRouter(router); const { server: innerServer } = await server.start(config); @@ -278,7 +286,8 @@ test('valid body', async () => { } ); - server.registerRouter(router); + const { registerRouter } = await server.setup(config); + registerRouter(router); const { server: innerServer } = await server.start(config); @@ -311,7 +320,8 @@ test('invalid body', async () => { } ); - server.registerRouter(router); + const { registerRouter } = await server.setup(config); + registerRouter(router); const { server: innerServer } = await server.start(config); @@ -343,7 +353,8 @@ test('handles putting', async () => { } ); - server.registerRouter(router); + const { registerRouter } = await server.setup(config); + registerRouter(router); const { server: innerServer } = await server.start(config); @@ -373,7 +384,8 @@ test('handles deleting', async () => { } ); - server.registerRouter(router); + const { registerRouter } = await server.setup(config); + registerRouter(router); const { server: innerServer } = await server.start(config); @@ -398,7 +410,8 @@ test('filtered headers', async () => { return res.noContent(); }); - server.registerRouter(router); + const { registerRouter } = await server.setup(config); + registerRouter(router); const { server: innerServer } = await server.start(config); @@ -430,7 +443,8 @@ describe('with `basepath: /bar` and `rewriteBasePath: false`', () => { res.ok({ key: 'value:/foo' }) ); - server.registerRouter(router); + const { registerRouter } = await server.setup(config); + registerRouter(router); const { server: innerServer } = await server.start(configWithBasePath); innerServerListener = innerServer.listener; @@ -490,7 +504,8 @@ describe('with `basepath: /bar` and `rewriteBasePath: true`', () => { res.ok({ key: 'value:/foo' }) ); - server.registerRouter(router); + const { registerRouter } = await server.setup(config); + registerRouter(router); const { server: innerServer } = await server.start(configWithBasePath); innerServerListener = innerServer.listener; @@ -555,17 +570,20 @@ describe('with defined `redirectHttpFromPort`', () => { const router = new Router('/'); router.get({ path: '/', validate: false }, async (req, res) => res.ok({ key: 'value:/' })); - server.registerRouter(router); + const { registerRouter } = await server.setup(config); + registerRouter(router); await server.start(configWithSSL); }); }); test('returns server and connection options on start', async () => { - const { server: innerServer, options } = await server.start({ + const configWithPort = { ...config, port: 12345, - }); + }; + const { options } = await server.setup(configWithPort); + const { server: innerServer } = await server.start(configWithPort); expect(innerServer).toBeDefined(); expect(innerServer).toBe((server as any).server); @@ -573,7 +591,7 @@ test('returns server and connection options on start', async () => { }); test('registers auth request interceptor only once', async () => { - const { registerAuth } = await server.start(config); + const { registerAuth } = await server.setup(config); const doRegister = () => registerAuth(() => null as any, { encryptionKey: 'any_password', @@ -584,7 +602,7 @@ test('registers auth request interceptor only once', async () => { }); test('registers onRequest interceptor several times', async () => { - const { registerOnRequest } = await server.start(config); + const { registerOnRequest } = await server.setup(config); const doRegister = () => registerOnRequest(() => null as any); doRegister(); diff --git a/src/core/server/http/http_service.test.ts b/src/core/server/http/http_service.test.ts index 6c38162a0b7f9..6d4f7ddb4bd53 100644 --- a/src/core/server/http/http_service.test.ts +++ b/src/core/server/http/http_service.test.ts @@ -41,6 +41,7 @@ test('creates and sets up http server', async () => { const httpServer = { isListening: () => false, + setup: jest.fn(), start: jest.fn(), stop: noop, }; @@ -49,10 +50,14 @@ test('creates and sets up http server', async () => { const service = new HttpService(config$.asObservable(), logger); expect(mockHttpServer.mock.instances.length).toBe(1); + + expect(httpServer.setup).not.toHaveBeenCalled(); expect(httpServer.start).not.toHaveBeenCalled(); await service.setup(); + expect(httpServer.setup).toHaveBeenCalledTimes(1); + await service.start(); expect(httpServer.start).toHaveBeenCalledTimes(1); }); @@ -63,6 +68,7 @@ test('logs error if already set up', async () => { const httpServer = { isListening: () => true, + setup: jest.fn(), start: noop, stop: noop, }; @@ -82,6 +88,7 @@ test('stops http server', async () => { const httpServer = { isListening: () => false, + setup: noop, start: noop, stop: jest.fn(), }; @@ -98,14 +105,14 @@ test('stops http server', async () => { expect(httpServer.stop).toHaveBeenCalledTimes(1); }); -test('register route handler', () => { +test('register route handler', async () => { const config = {} as HttpConfig; - const config$ = new BehaviorSubject(config); + const registerRouterMock = jest.fn(); const httpServer = { isListening: () => false, - registerRouter: jest.fn(), + setup: () => ({ registerRouter: registerRouterMock }), start: noop, stop: noop, }; @@ -114,33 +121,11 @@ test('register route handler', () => { const service = new HttpService(config$.asObservable(), logger); const router = new Router('/foo'); - service.registerRouter(router); - - expect(httpServer.registerRouter).toHaveBeenCalledTimes(1); - expect(httpServer.registerRouter).toHaveBeenLastCalledWith(router); - expect(loggingServiceMock.collect(logger)).toMatchSnapshot(); -}); + const { registerRouter } = await service.setup(); + registerRouter(router); -test('throws if registering route handler after http server is set up', () => { - const config = {} as HttpConfig; - - const config$ = new BehaviorSubject(config); - - const httpServer = { - isListening: () => true, - registerRouter: jest.fn(), - start: noop, - stop: noop, - }; - mockHttpServer.mockImplementation(() => httpServer); - - const service = new HttpService(config$.asObservable(), logger); - - const router = new Router('/foo'); - service.registerRouter(router); - - expect(httpServer.registerRouter).toHaveBeenCalledTimes(0); - expect(loggingServiceMock.collect(logger)).toMatchSnapshot(); + expect(registerRouterMock).toHaveBeenCalledTimes(1); + expect(registerRouterMock).toHaveBeenLastCalledWith(router); }); test('returns http server contract on setup', async () => { @@ -157,5 +142,5 @@ test('returns http server contract on setup', async () => { const service = new HttpService(new BehaviorSubject({ ssl: {} } as HttpConfig), logger); - expect(await service.setup()).toBe(httpServer); + expect(await service.start()).toBe(httpServer); }); diff --git a/src/core/server/legacy/legacy_service.test.ts b/src/core/server/legacy/legacy_service.test.ts index e62177d6756e3..9950c21276c35 100644 --- a/src/core/server/legacy/legacy_service.test.ts +++ b/src/core/server/legacy/legacy_service.test.ts @@ -48,6 +48,10 @@ let setupDeps: { http: any; plugins: PluginsServiceSetup; }; + +let startDeps: { + http: any; +}; const logger = loggingServiceMock.create(); let configService: ReturnType; @@ -60,7 +64,6 @@ beforeEach(() => { setupDeps = { elasticsearch: { legacy: {} } as any, http: { - server: { listener: { addListener: jest.fn() }, route: jest.fn() }, options: { someOption: 'foo', someAnotherOption: 'bar' }, }, plugins: { @@ -72,6 +75,12 @@ beforeEach(() => { }, }; + startDeps = { + http: { + server: { listener: { addListener: jest.fn() }, route: jest.fn() }, + }, + }; + config$ = new BehaviorSubject( new ObjectToConfigAdapter({ elasticsearch: { hosts: ['http://127.0.0.1'] }, @@ -91,9 +100,9 @@ afterEach(() => { describe('once LegacyService is set up with connection info', () => { test('register proxy route.', async () => { - await legacyService.setup(setupDeps); + await legacyService.start(setupDeps, startDeps); - expect(setupDeps.http.server.route.mock.calls).toMatchSnapshot('proxy route options'); + expect(startDeps.http.server.route.mock.calls).toMatchSnapshot('proxy route options'); }); test('proxy route responds with `503` if `kbnServer` is not ready yet.', async () => { @@ -107,7 +116,7 @@ describe('once LegacyService is set up with connection info', () => { // Wait until listen is called and proxy route is registered, but don't allow // listen to complete and make kbnServer available. - const legacySetupPromise = legacyService.setup(setupDeps); + const legacySetupPromise = legacyService.start(setupDeps, startDeps); await kbnServerListen$.pipe(first()).toPromise(); const mockResponse: any = { @@ -120,7 +129,7 @@ describe('once LegacyService is set up with connection info', () => { }; const mockRequest = { raw: { req: { a: 1 }, res: { b: 2 } } }; - const [[{ handler }]] = setupDeps.http.server.route.mock.calls; + const [[{ handler }]] = startDeps.http.server.route.mock.calls; const response503 = await handler(mockRequest, mockResponseToolkit); expect(response503).toBe(mockResponse); @@ -155,21 +164,20 @@ describe('once LegacyService is set up with connection info', () => { test('creates legacy kbnServer and calls `listen`.', async () => { configService.atPath.mockReturnValue(new BehaviorSubject({ autoListen: true })); - await legacyService.setup(setupDeps); + await legacyService.start(setupDeps, startDeps); expect(MockKbnServer).toHaveBeenCalledTimes(1); expect(MockKbnServer).toHaveBeenCalledWith( { server: { autoListen: true } }, { - elasticsearch: setupDeps.elasticsearch, - http: setupDeps.http, + setupDeps, + startDeps, serverOptions: { listener: expect.any(LegacyPlatformProxy), someAnotherOption: 'bar', someOption: 'foo', }, handledConfigPaths: ['foo.bar'], - plugins: setupDeps.plugins, } ); @@ -181,21 +189,20 @@ describe('once LegacyService is set up with connection info', () => { test('creates legacy kbnServer but does not call `listen` if `autoListen: false`.', async () => { configService.atPath.mockReturnValue(new BehaviorSubject({ autoListen: false })); - await legacyService.setup(setupDeps); + await legacyService.start(setupDeps, startDeps); expect(MockKbnServer).toHaveBeenCalledTimes(1); expect(MockKbnServer).toHaveBeenCalledWith( { server: { autoListen: true } }, { - elasticsearch: setupDeps.elasticsearch, - http: setupDeps.http, + setupDeps, + startDeps, serverOptions: { listener: expect.any(LegacyPlatformProxy), someAnotherOption: 'bar', someOption: 'foo', }, handledConfigPaths: ['foo.bar'], - plugins: setupDeps.plugins, } ); @@ -209,7 +216,7 @@ describe('once LegacyService is set up with connection info', () => { configService.atPath.mockReturnValue(new BehaviorSubject({ autoListen: true })); MockKbnServer.prototype.listen.mockRejectedValue(new Error('something failed')); - await expect(legacyService.setup(setupDeps)).rejects.toThrowErrorMatchingSnapshot(); + await expect(legacyService.start(setupDeps, startDeps)).rejects.toThrowErrorMatchingSnapshot(); const [mockKbnServer] = MockKbnServer.mock.instances; expect(mockKbnServer.listen).toHaveBeenCalled(); @@ -219,14 +226,14 @@ describe('once LegacyService is set up with connection info', () => { test('throws if fails to retrieve initial config.', async () => { configService.getConfig$.mockReturnValue(throwError(new Error('something failed'))); - await expect(legacyService.setup(setupDeps)).rejects.toThrowErrorMatchingSnapshot(); + await expect(legacyService.start(setupDeps, startDeps)).rejects.toThrowErrorMatchingSnapshot(); expect(MockKbnServer).not.toHaveBeenCalled(); expect(MockClusterManager).not.toHaveBeenCalled(); }); test('reconfigures logging configuration if new config is received.', async () => { - await legacyService.setup(setupDeps); + await legacyService.start(setupDeps, startDeps); const [mockKbnServer] = MockKbnServer.mock.instances as Array>; expect(mockKbnServer.applyLoggingConfiguration).not.toHaveBeenCalled(); @@ -239,7 +246,7 @@ describe('once LegacyService is set up with connection info', () => { }); test('logs error if re-configuring fails.', async () => { - await legacyService.setup(setupDeps); + await legacyService.start(setupDeps, startDeps); const [mockKbnServer] = MockKbnServer.mock.instances as Array>; expect(mockKbnServer.applyLoggingConfiguration).not.toHaveBeenCalled(); @@ -256,7 +263,7 @@ describe('once LegacyService is set up with connection info', () => { }); test('logs error if config service fails.', async () => { - await legacyService.setup(setupDeps); + await legacyService.start(setupDeps, startDeps); const [mockKbnServer] = MockKbnServer.mock.instances; expect(mockKbnServer.applyLoggingConfiguration).not.toHaveBeenCalled(); @@ -273,9 +280,9 @@ describe('once LegacyService is set up with connection info', () => { const mockResponseToolkit = { response: jest.fn(), abandon: Symbol('abandon') }; const mockRequest = { raw: { req: { a: 1 }, res: { b: 2 } } }; - await legacyService.setup(setupDeps); + await legacyService.start(setupDeps, startDeps); - const [[{ handler }]] = setupDeps.http.server.route.mock.calls; + const [[{ handler }]] = startDeps.http.server.route.mock.calls; const response = await handler(mockRequest, mockResponseToolkit); expect(response).toBe(mockResponseToolkit.abandon); @@ -293,23 +300,23 @@ describe('once LegacyService is set up with connection info', () => { }); describe('once LegacyService is set up without connection info', () => { + const disabledHttpStartDeps = { + http: undefined, + }; beforeEach(async () => { - await legacyService.setup({ - elasticsearch: setupDeps.elasticsearch, - plugins: setupDeps.plugins, - }); + await legacyService.start(setupDeps, disabledHttpStartDeps); }); test('creates legacy kbnServer with `autoListen: false`.', () => { - expect(setupDeps.http.server.route).not.toHaveBeenCalled(); + expect(startDeps.http.server.route).not.toHaveBeenCalled(); expect(MockKbnServer).toHaveBeenCalledTimes(1); expect(MockKbnServer).toHaveBeenCalledWith( { server: { autoListen: true } }, { - elasticsearch: setupDeps.elasticsearch, + setupDeps, + startDeps: disabledHttpStartDeps, serverOptions: { autoListen: false }, handledConfigPaths: ['foo.bar'], - plugins: setupDeps.plugins, } ); }); @@ -347,10 +354,16 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => { configService: configService as any, }); - await devClusterLegacyService.setup({ - elasticsearch: setupDeps.elasticsearch, - plugins: { contracts: new Map(), uiPlugins: { public: new Map(), internal: new Map() } }, - }); + await devClusterLegacyService.start( + { + elasticsearch: setupDeps.elasticsearch, + plugins: { contracts: new Map(), uiPlugins: { public: new Map(), internal: new Map() } }, + http: setupDeps.http, + }, + { + http: undefined, + } + ); expect(MockClusterManager.create.mock.calls).toMatchSnapshot( 'cluster manager without base path proxy' @@ -369,10 +382,16 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => { configService: configService as any, }); - await devClusterLegacyService.setup({ - elasticsearch: setupDeps.elasticsearch, - plugins: { contracts: new Map(), uiPlugins: { public: new Map(), internal: new Map() } }, - }); + await devClusterLegacyService.start( + { + elasticsearch: setupDeps.elasticsearch, + plugins: { contracts: new Map(), uiPlugins: { public: new Map(), internal: new Map() } }, + http: setupDeps.http, + }, + { + http: undefined, + } + ); expect(MockClusterManager.create.mock.calls).toMatchSnapshot( 'cluster manager with base path proxy' diff --git a/src/core/server/server.test.ts b/src/core/server/server.test.ts index cbfdbbc3bc00d..f97a12caa6326 100644 --- a/src/core/server/server.test.ts +++ b/src/core/server/server.test.ts @@ -44,13 +44,15 @@ afterEach(() => { jest.clearAllMocks(); configService.atPath.mockReset(); - httpService.setup.mockReset(); + httpService.setup.mockClear(); + httpService.start.mockClear(); httpService.stop.mockReset(); elasticsearchService.setup.mockReset(); elasticsearchService.stop.mockReset(); mockPluginsService.setup.mockReset(); mockPluginsService.stop.mockReset(); mockLegacyService.setup.mockReset(); + mockLegacyService.start.mockReset(); mockLegacyService.stop.mockReset(); }); @@ -63,52 +65,67 @@ test('sets up services on "setup"', async () => { expect(httpService.setup).not.toHaveBeenCalled(); expect(elasticsearchService.setup).not.toHaveBeenCalled(); expect(mockPluginsService.setup).not.toHaveBeenCalled(); - expect(mockLegacyService.setup).not.toHaveBeenCalled(); + expect(mockLegacyService.start).not.toHaveBeenCalled(); await server.setup(); expect(httpService.setup).toHaveBeenCalledTimes(1); expect(elasticsearchService.setup).toHaveBeenCalledTimes(1); expect(mockPluginsService.setup).toHaveBeenCalledTimes(1); - expect(mockLegacyService.setup).toHaveBeenCalledTimes(1); +}); + +test('runs services on "start"', async () => { + const mockPluginsServiceSetup = new Map([['some-plugin', 'some-value']]); + mockPluginsService.setup.mockReturnValue(Promise.resolve(mockPluginsServiceSetup)); + + const server = new Server(configService as any, logger, env); + + expect(httpService.setup).not.toHaveBeenCalled(); + expect(mockLegacyService.start).not.toHaveBeenCalled(); + + await server.setup(); + await server.start(); + + expect(httpService.start).toHaveBeenCalledTimes(1); + expect(mockLegacyService.start).toHaveBeenCalledTimes(1); }); test('does not fail on "setup" if there are unused paths detected', async () => { configService.getUnusedPaths.mockResolvedValue(['some.path', 'another.path']); const server = new Server(configService as any, logger, env); - await expect(server.setup()).resolves.toBeUndefined(); + await expect(server.setup()).resolves.toBeDefined(); expect(loggingServiceMock.collect(logger)).toMatchSnapshot('unused paths logs'); }); -test('does not setup http service is `autoListen:false`', async () => { +test('does not start http service is `autoListen:false`', async () => { configService.atPath.mockReturnValue(new BehaviorSubject({ autoListen: false })); const server = new Server(configService as any, logger, env); - - expect(mockLegacyService.setup).not.toHaveBeenCalled(); + expect(mockLegacyService.start).not.toHaveBeenCalled(); + expect(httpService.start).not.toHaveBeenCalled(); await server.setup(); + await server.start(); - expect(httpService.setup).not.toHaveBeenCalled(); - expect(mockLegacyService.setup).toHaveBeenCalledTimes(1); - expect(mockLegacyService.setup).toHaveBeenCalledWith({}); + expect(httpService.start).not.toHaveBeenCalled(); + expect(mockLegacyService.start).toHaveBeenCalledTimes(1); }); -test('does not setup http service if process is dev cluster master', async () => { +test('does not start http service if process is dev cluster master', async () => { const server = new Server( configService as any, logger, new Env('.', getEnvOptions({ isDevClusterMaster: true })) ); - expect(mockLegacyService.setup).not.toHaveBeenCalled(); + expect(mockLegacyService.start).not.toHaveBeenCalled(); await server.setup(); + await server.start(); - expect(httpService.setup).not.toHaveBeenCalled(); - expect(mockLegacyService.setup).toHaveBeenCalledTimes(1); - expect(mockLegacyService.setup).toHaveBeenCalledWith({}); + expect(httpService.start).not.toHaveBeenCalled(); + expect(mockLegacyService.start).toHaveBeenCalledTimes(1); }); test('stops services on "stop"', async () => { From fad9a06faa4f04d95fc92f0f55b6b6f7a3a3d426 Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Thu, 18 Apr 2019 16:54:12 +0200 Subject: [PATCH 07/20] test servers also should call root.start() --- .../server/config/__tests__/fixtures/run_kbn_server_startup.js | 1 + src/legacy/ui/__tests__/ui_exports_replace_injected_vars.js | 1 + src/test_utils/kbn_server.ts | 1 + 3 files changed, 3 insertions(+) diff --git a/src/legacy/server/config/__tests__/fixtures/run_kbn_server_startup.js b/src/legacy/server/config/__tests__/fixtures/run_kbn_server_startup.js index 2992722ebd374..3eaf18be609d4 100644 --- a/src/legacy/server/config/__tests__/fixtures/run_kbn_server_startup.js +++ b/src/legacy/server/config/__tests__/fixtures/run_kbn_server_startup.js @@ -27,6 +27,7 @@ import { createRoot } from '../../../../../test_utils/kbn_server'; // to allow the process to exit naturally try { await root.setup(); + await root.start(); } finally { await root.shutdown(); } diff --git a/src/legacy/ui/__tests__/ui_exports_replace_injected_vars.js b/src/legacy/ui/__tests__/ui_exports_replace_injected_vars.js index 9243a715625d8..20ff1d5bcfe0a 100644 --- a/src/legacy/ui/__tests__/ui_exports_replace_injected_vars.js +++ b/src/legacy/ui/__tests__/ui_exports_replace_injected_vars.js @@ -61,6 +61,7 @@ describe('UiExports', function () { }); await root.setup(); + await root.start(); kbnServer = getKbnServer(root); diff --git a/src/test_utils/kbn_server.ts b/src/test_utils/kbn_server.ts index 330c058ed39ae..2d45ca3b51d89 100644 --- a/src/test_utils/kbn_server.ts +++ b/src/test_utils/kbn_server.ts @@ -236,6 +236,7 @@ export async function startTestServers({ const root = createRootWithCorePlugins(kbnSettings); await root.setup(); + await root.start(); const kbnServer = getKbnServer(root); await kbnServer.server.plugins.elasticsearch.waitUntilReady(); From cfddce2ca3459ef26ad5b6982b0a572a40c9e065 Mon Sep 17 00:00:00 2001 From: restrry Date: Thu, 18 Apr 2019 17:29:06 +0200 Subject: [PATCH 08/20] update docs --- ...bana-plugin-server.authenticationhandler.md | 2 ++ ...-plugin-server.authtoolkit.authenticated.md | 2 ++ .../server/kibana-plugin-server.authtoolkit.md | 2 ++ ...ana-plugin-server.authtoolkit.redirected.md | 2 ++ ...ibana-plugin-server.authtoolkit.rejected.md | 2 ++ .../kibana-plugin-server.corestart.http.md | 11 +++++++++++ .../server/kibana-plugin-server.corestart.md | 18 ++++++++++++++++++ .../kibana-plugin-server.httpservicesetup.md | 4 +++- .../kibana-plugin-server.httpservicestart.md | 11 +++++++++++ .../kibana-plugin-server.kibanarequest.body.md | 2 ++ .../kibana-plugin-server.kibanarequest.from.md | 2 ++ ...-server.kibanarequest.getfilteredheaders.md | 2 ++ ...bana-plugin-server.kibanarequest.headers.md | 2 ++ .../kibana-plugin-server.kibanarequest.md | 2 ++ ...ibana-plugin-server.kibanarequest.params.md | 2 ++ .../kibana-plugin-server.kibanarequest.path.md | 2 ++ ...kibana-plugin-server.kibanarequest.query.md | 2 ++ .../core/server/kibana-plugin-server.md | 2 ++ .../kibana-plugin-server.onrequesthandler.md | 2 ++ .../kibana-plugin-server.onrequesttoolkit.md | 2 ++ ...bana-plugin-server.onrequesttoolkit.next.md | 2 ++ ...lugin-server.onrequesttoolkit.redirected.md | 2 ++ ...-plugin-server.onrequesttoolkit.rejected.md | 2 ++ ...na-plugin-server.pluginsetupcontext.http.md | 2 ++ .../kibana-plugin-server.router.delete.md | 2 ++ .../server/kibana-plugin-server.router.get.md | 2 ++ .../kibana-plugin-server.router.getroutes.md | 2 ++ .../core/server/kibana-plugin-server.router.md | 2 ++ .../server/kibana-plugin-server.router.path.md | 2 ++ .../server/kibana-plugin-server.router.post.md | 2 ++ .../server/kibana-plugin-server.router.put.md | 2 ++ .../kibana-plugin-server.router.routes.md | 2 ++ src/core/server/server.api.md | 11 ++++++++++- 33 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 docs/development/core/server/kibana-plugin-server.corestart.http.md create mode 100644 docs/development/core/server/kibana-plugin-server.corestart.md create mode 100644 docs/development/core/server/kibana-plugin-server.httpservicestart.md diff --git a/docs/development/core/server/kibana-plugin-server.authenticationhandler.md b/docs/development/core/server/kibana-plugin-server.authenticationhandler.md index 3f087489c1376..cb09dc0cd6051 100644 --- a/docs/development/core/server/kibana-plugin-server.authenticationhandler.md +++ b/docs/development/core/server/kibana-plugin-server.authenticationhandler.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [AuthenticationHandler](./kibana-plugin-server.authenticationhandler.md) ## AuthenticationHandler type diff --git a/docs/development/core/server/kibana-plugin-server.authtoolkit.authenticated.md b/docs/development/core/server/kibana-plugin-server.authtoolkit.authenticated.md index cec5cf405924c..d0f1e07c47484 100644 --- a/docs/development/core/server/kibana-plugin-server.authtoolkit.authenticated.md +++ b/docs/development/core/server/kibana-plugin-server.authtoolkit.authenticated.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [AuthToolkit](./kibana-plugin-server.authtoolkit.md) > [authenticated](./kibana-plugin-server.authtoolkit.authenticated.md) ## AuthToolkit.authenticated property diff --git a/docs/development/core/server/kibana-plugin-server.authtoolkit.md b/docs/development/core/server/kibana-plugin-server.authtoolkit.md index 60590ee448e57..d7e2e39b44d4c 100644 --- a/docs/development/core/server/kibana-plugin-server.authtoolkit.md +++ b/docs/development/core/server/kibana-plugin-server.authtoolkit.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [AuthToolkit](./kibana-plugin-server.authtoolkit.md) ## AuthToolkit interface diff --git a/docs/development/core/server/kibana-plugin-server.authtoolkit.redirected.md b/docs/development/core/server/kibana-plugin-server.authtoolkit.redirected.md index 1ebe9a8549ff3..3eb331e6c9302 100644 --- a/docs/development/core/server/kibana-plugin-server.authtoolkit.redirected.md +++ b/docs/development/core/server/kibana-plugin-server.authtoolkit.redirected.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [AuthToolkit](./kibana-plugin-server.authtoolkit.md) > [redirected](./kibana-plugin-server.authtoolkit.redirected.md) ## AuthToolkit.redirected property diff --git a/docs/development/core/server/kibana-plugin-server.authtoolkit.rejected.md b/docs/development/core/server/kibana-plugin-server.authtoolkit.rejected.md index dffa66531c37d..949cd5dd2adc2 100644 --- a/docs/development/core/server/kibana-plugin-server.authtoolkit.rejected.md +++ b/docs/development/core/server/kibana-plugin-server.authtoolkit.rejected.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [AuthToolkit](./kibana-plugin-server.authtoolkit.md) > [rejected](./kibana-plugin-server.authtoolkit.rejected.md) ## AuthToolkit.rejected property diff --git a/docs/development/core/server/kibana-plugin-server.corestart.http.md b/docs/development/core/server/kibana-plugin-server.corestart.http.md new file mode 100644 index 0000000000000..a12fcbaab27cc --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.corestart.http.md @@ -0,0 +1,11 @@ + + +[Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [CoreStart](./kibana-plugin-server.corestart.md) > [http](./kibana-plugin-server.corestart.http.md) + +## CoreStart.http property + +Signature: + +```typescript +http: HttpServiceStart; +``` diff --git a/docs/development/core/server/kibana-plugin-server.corestart.md b/docs/development/core/server/kibana-plugin-server.corestart.md new file mode 100644 index 0000000000000..13b86f480e6f3 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.corestart.md @@ -0,0 +1,18 @@ + + +[Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [CoreStart](./kibana-plugin-server.corestart.md) + +## CoreStart interface + +Signature: + +```typescript +export interface CoreStart +``` + +## Properties + +| Property | Type | Description | +| --- | --- | --- | +| [http](./kibana-plugin-server.corestart.http.md) | HttpServiceStart | | + diff --git a/docs/development/core/server/kibana-plugin-server.httpservicesetup.md b/docs/development/core/server/kibana-plugin-server.httpservicesetup.md index a5517f26bdf96..36335c0d5f4cd 100644 --- a/docs/development/core/server/kibana-plugin-server.httpservicesetup.md +++ b/docs/development/core/server/kibana-plugin-server.httpservicesetup.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [HttpServiceSetup](./kibana-plugin-server.httpservicesetup.md) ## HttpServiceSetup type @@ -6,5 +8,5 @@ Signature: ```typescript -export declare type HttpServiceSetup = HttpServerInfo; +export declare type HttpServiceSetup = HttpServerSetup; ``` diff --git a/docs/development/core/server/kibana-plugin-server.httpservicestart.md b/docs/development/core/server/kibana-plugin-server.httpservicestart.md new file mode 100644 index 0000000000000..c3b1b71d7143a --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.httpservicestart.md @@ -0,0 +1,11 @@ + + +[Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [HttpServiceStart](./kibana-plugin-server.httpservicestart.md) + +## HttpServiceStart type + +Signature: + +```typescript +export declare type HttpServiceStart = HttpServerInfo; +``` diff --git a/docs/development/core/server/kibana-plugin-server.kibanarequest.body.md b/docs/development/core/server/kibana-plugin-server.kibanarequest.body.md index e08f84ea44bba..60ef0a023702a 100644 --- a/docs/development/core/server/kibana-plugin-server.kibanarequest.body.md +++ b/docs/development/core/server/kibana-plugin-server.kibanarequest.body.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [KibanaRequest](./kibana-plugin-server.kibanarequest.md) > [body](./kibana-plugin-server.kibanarequest.body.md) ## KibanaRequest.body property diff --git a/docs/development/core/server/kibana-plugin-server.kibanarequest.from.md b/docs/development/core/server/kibana-plugin-server.kibanarequest.from.md index 7d762642bd99f..cdf7ea1ebfe43 100644 --- a/docs/development/core/server/kibana-plugin-server.kibanarequest.from.md +++ b/docs/development/core/server/kibana-plugin-server.kibanarequest.from.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [KibanaRequest](./kibana-plugin-server.kibanarequest.md) > [from](./kibana-plugin-server.kibanarequest.from.md) ## KibanaRequest.from() method diff --git a/docs/development/core/server/kibana-plugin-server.kibanarequest.getfilteredheaders.md b/docs/development/core/server/kibana-plugin-server.kibanarequest.getfilteredheaders.md index defa9b739586f..c17a416fed2d6 100644 --- a/docs/development/core/server/kibana-plugin-server.kibanarequest.getfilteredheaders.md +++ b/docs/development/core/server/kibana-plugin-server.kibanarequest.getfilteredheaders.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [KibanaRequest](./kibana-plugin-server.kibanarequest.md) > [getFilteredHeaders](./kibana-plugin-server.kibanarequest.getfilteredheaders.md) ## KibanaRequest.getFilteredHeaders() method diff --git a/docs/development/core/server/kibana-plugin-server.kibanarequest.headers.md b/docs/development/core/server/kibana-plugin-server.kibanarequest.headers.md index f920942db4a67..f5c38cc752ab3 100644 --- a/docs/development/core/server/kibana-plugin-server.kibanarequest.headers.md +++ b/docs/development/core/server/kibana-plugin-server.kibanarequest.headers.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [KibanaRequest](./kibana-plugin-server.kibanarequest.md) > [headers](./kibana-plugin-server.kibanarequest.headers.md) ## KibanaRequest.headers property diff --git a/docs/development/core/server/kibana-plugin-server.kibanarequest.md b/docs/development/core/server/kibana-plugin-server.kibanarequest.md index 835e34c9e602a..d423cfb5f4dea 100644 --- a/docs/development/core/server/kibana-plugin-server.kibanarequest.md +++ b/docs/development/core/server/kibana-plugin-server.kibanarequest.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [KibanaRequest](./kibana-plugin-server.kibanarequest.md) ## KibanaRequest class diff --git a/docs/development/core/server/kibana-plugin-server.kibanarequest.params.md b/docs/development/core/server/kibana-plugin-server.kibanarequest.params.md index c8b57329aac0d..ef26aac2e6904 100644 --- a/docs/development/core/server/kibana-plugin-server.kibanarequest.params.md +++ b/docs/development/core/server/kibana-plugin-server.kibanarequest.params.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [KibanaRequest](./kibana-plugin-server.kibanarequest.md) > [params](./kibana-plugin-server.kibanarequest.params.md) ## KibanaRequest.params property diff --git a/docs/development/core/server/kibana-plugin-server.kibanarequest.path.md b/docs/development/core/server/kibana-plugin-server.kibanarequest.path.md index 269fcfd4e4937..ebf6fb28b98ea 100644 --- a/docs/development/core/server/kibana-plugin-server.kibanarequest.path.md +++ b/docs/development/core/server/kibana-plugin-server.kibanarequest.path.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [KibanaRequest](./kibana-plugin-server.kibanarequest.md) > [path](./kibana-plugin-server.kibanarequest.path.md) ## KibanaRequest.path property diff --git a/docs/development/core/server/kibana-plugin-server.kibanarequest.query.md b/docs/development/core/server/kibana-plugin-server.kibanarequest.query.md index 17cf96f3c3a93..69fb3faed84a2 100644 --- a/docs/development/core/server/kibana-plugin-server.kibanarequest.query.md +++ b/docs/development/core/server/kibana-plugin-server.kibanarequest.query.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [KibanaRequest](./kibana-plugin-server.kibanarequest.md) > [query](./kibana-plugin-server.kibanarequest.query.md) ## KibanaRequest.query property diff --git a/docs/development/core/server/kibana-plugin-server.md b/docs/development/core/server/kibana-plugin-server.md index afe9d731aa338..6f9da5680616c 100644 --- a/docs/development/core/server/kibana-plugin-server.md +++ b/docs/development/core/server/kibana-plugin-server.md @@ -21,6 +21,7 @@ | [AuthToolkit](./kibana-plugin-server.authtoolkit.md) | A tool set defining an outcome of Auth interceptor for incoming request. | | [CallAPIOptions](./kibana-plugin-server.callapioptions.md) | The set of options that defines how API call should be made and result be processed. | | [CoreSetup](./kibana-plugin-server.coresetup.md) | | +| [CoreStart](./kibana-plugin-server.corestart.md) | | | [ElasticsearchServiceSetup](./kibana-plugin-server.elasticsearchservicesetup.md) | | | [Logger](./kibana-plugin-server.logger.md) | Logger exposes all the necessary methods to log any type of information and this is the interface used by the logging consumers including plugins. | | [LoggerFactory](./kibana-plugin-server.loggerfactory.md) | The single purpose of LoggerFactory interface is to define a way to retrieve a context-based logger instance. | @@ -39,6 +40,7 @@ | [ElasticsearchClientConfig](./kibana-plugin-server.elasticsearchclientconfig.md) | | | [Headers](./kibana-plugin-server.headers.md) | | | [HttpServiceSetup](./kibana-plugin-server.httpservicesetup.md) | | +| [HttpServiceStart](./kibana-plugin-server.httpservicestart.md) | | | [OnRequestHandler](./kibana-plugin-server.onrequesthandler.md) | | | [PluginInitializer](./kibana-plugin-server.plugininitializer.md) | The plugin export at the root of a plugin's server directory should conform to this interface. | | [PluginName](./kibana-plugin-server.pluginname.md) | Dedicated type for plugin name/id that is supposed to make Map/Set/Arrays that use it as a key or value more obvious. | diff --git a/docs/development/core/server/kibana-plugin-server.onrequesthandler.md b/docs/development/core/server/kibana-plugin-server.onrequesthandler.md index 5f093fef4eb20..5d90e399db676 100644 --- a/docs/development/core/server/kibana-plugin-server.onrequesthandler.md +++ b/docs/development/core/server/kibana-plugin-server.onrequesthandler.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [OnRequestHandler](./kibana-plugin-server.onrequesthandler.md) ## OnRequestHandler type diff --git a/docs/development/core/server/kibana-plugin-server.onrequesttoolkit.md b/docs/development/core/server/kibana-plugin-server.onrequesttoolkit.md index 5fdbebbf37263..cd88ef8fc5dda 100644 --- a/docs/development/core/server/kibana-plugin-server.onrequesttoolkit.md +++ b/docs/development/core/server/kibana-plugin-server.onrequesttoolkit.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [OnRequestToolkit](./kibana-plugin-server.onrequesttoolkit.md) ## OnRequestToolkit interface diff --git a/docs/development/core/server/kibana-plugin-server.onrequesttoolkit.next.md b/docs/development/core/server/kibana-plugin-server.onrequesttoolkit.next.md index 4a6aef813a566..976e3b1a2db87 100644 --- a/docs/development/core/server/kibana-plugin-server.onrequesttoolkit.next.md +++ b/docs/development/core/server/kibana-plugin-server.onrequesttoolkit.next.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [OnRequestToolkit](./kibana-plugin-server.onrequesttoolkit.md) > [next](./kibana-plugin-server.onrequesttoolkit.next.md) ## OnRequestToolkit.next property diff --git a/docs/development/core/server/kibana-plugin-server.onrequesttoolkit.redirected.md b/docs/development/core/server/kibana-plugin-server.onrequesttoolkit.redirected.md index d2968d7b9c497..311398845bd59 100644 --- a/docs/development/core/server/kibana-plugin-server.onrequesttoolkit.redirected.md +++ b/docs/development/core/server/kibana-plugin-server.onrequesttoolkit.redirected.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [OnRequestToolkit](./kibana-plugin-server.onrequesttoolkit.md) > [redirected](./kibana-plugin-server.onrequesttoolkit.redirected.md) ## OnRequestToolkit.redirected property diff --git a/docs/development/core/server/kibana-plugin-server.onrequesttoolkit.rejected.md b/docs/development/core/server/kibana-plugin-server.onrequesttoolkit.rejected.md index c89c6ae5465d7..447d9b3fb9be5 100644 --- a/docs/development/core/server/kibana-plugin-server.onrequesttoolkit.rejected.md +++ b/docs/development/core/server/kibana-plugin-server.onrequesttoolkit.rejected.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [OnRequestToolkit](./kibana-plugin-server.onrequesttoolkit.md) > [rejected](./kibana-plugin-server.onrequesttoolkit.rejected.md) ## OnRequestToolkit.rejected property diff --git a/docs/development/core/server/kibana-plugin-server.pluginsetupcontext.http.md b/docs/development/core/server/kibana-plugin-server.pluginsetupcontext.http.md index d66356117ce24..65acef7b54774 100644 --- a/docs/development/core/server/kibana-plugin-server.pluginsetupcontext.http.md +++ b/docs/development/core/server/kibana-plugin-server.pluginsetupcontext.http.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [PluginSetupContext](./kibana-plugin-server.pluginsetupcontext.md) > [http](./kibana-plugin-server.pluginsetupcontext.http.md) ## PluginSetupContext.http property diff --git a/docs/development/core/server/kibana-plugin-server.router.delete.md b/docs/development/core/server/kibana-plugin-server.router.delete.md index 40457d0645ddb..b98a2c24c090c 100644 --- a/docs/development/core/server/kibana-plugin-server.router.delete.md +++ b/docs/development/core/server/kibana-plugin-server.router.delete.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [Router](./kibana-plugin-server.router.md) > [delete](./kibana-plugin-server.router.delete.md) ## Router.delete() method diff --git a/docs/development/core/server/kibana-plugin-server.router.get.md b/docs/development/core/server/kibana-plugin-server.router.get.md index 8ab429d411351..b9c16eed73923 100644 --- a/docs/development/core/server/kibana-plugin-server.router.get.md +++ b/docs/development/core/server/kibana-plugin-server.router.get.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [Router](./kibana-plugin-server.router.md) > [get](./kibana-plugin-server.router.get.md) ## Router.get() method diff --git a/docs/development/core/server/kibana-plugin-server.router.getroutes.md b/docs/development/core/server/kibana-plugin-server.router.getroutes.md index eb48ca46d7282..4599728a620ca 100644 --- a/docs/development/core/server/kibana-plugin-server.router.getroutes.md +++ b/docs/development/core/server/kibana-plugin-server.router.getroutes.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [Router](./kibana-plugin-server.router.md) > [getRoutes](./kibana-plugin-server.router.getroutes.md) ## Router.getRoutes() method diff --git a/docs/development/core/server/kibana-plugin-server.router.md b/docs/development/core/server/kibana-plugin-server.router.md index 8fc634b6a592b..f30826c7413b1 100644 --- a/docs/development/core/server/kibana-plugin-server.router.md +++ b/docs/development/core/server/kibana-plugin-server.router.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [Router](./kibana-plugin-server.router.md) ## Router class diff --git a/docs/development/core/server/kibana-plugin-server.router.path.md b/docs/development/core/server/kibana-plugin-server.router.path.md index 4344dcf4560e3..050047593a8cf 100644 --- a/docs/development/core/server/kibana-plugin-server.router.path.md +++ b/docs/development/core/server/kibana-plugin-server.router.path.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [Router](./kibana-plugin-server.router.md) > [path](./kibana-plugin-server.router.path.md) ## Router.path property diff --git a/docs/development/core/server/kibana-plugin-server.router.post.md b/docs/development/core/server/kibana-plugin-server.router.post.md index 929af38f0c662..d815b58c46029 100644 --- a/docs/development/core/server/kibana-plugin-server.router.post.md +++ b/docs/development/core/server/kibana-plugin-server.router.post.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [Router](./kibana-plugin-server.router.md) > [post](./kibana-plugin-server.router.post.md) ## Router.post() method diff --git a/docs/development/core/server/kibana-plugin-server.router.put.md b/docs/development/core/server/kibana-plugin-server.router.put.md index 902104883262e..ebc62bb70fec0 100644 --- a/docs/development/core/server/kibana-plugin-server.router.put.md +++ b/docs/development/core/server/kibana-plugin-server.router.put.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [Router](./kibana-plugin-server.router.md) > [put](./kibana-plugin-server.router.put.md) ## Router.put() method diff --git a/docs/development/core/server/kibana-plugin-server.router.routes.md b/docs/development/core/server/kibana-plugin-server.router.routes.md index a879bbc733b0c..2cccf62cdda78 100644 --- a/docs/development/core/server/kibana-plugin-server.router.routes.md +++ b/docs/development/core/server/kibana-plugin-server.router.routes.md @@ -1,3 +1,5 @@ + + [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [Router](./kibana-plugin-server.router.md) > [routes](./kibana-plugin-server.router.routes.md) ## Router.routes property diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 96c68685d3bd3..9c20bde8218af 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -74,6 +74,12 @@ export interface CoreSetup { plugins: PluginsServiceSetup; } +// @public (undocumented) +export interface CoreStart { + // (undocumented) + http: HttpServiceStart; +} + // @internal export interface DiscoveredPlugin { readonly configPath: ConfigPath; @@ -108,7 +114,10 @@ export interface ElasticsearchServiceSetup { export type Headers = Record; // @public (undocumented) -export type HttpServiceSetup = HttpServerInfo; +export type HttpServiceSetup = HttpServerSetup; + +// @public (undocumented) +export type HttpServiceStart = HttpServerInfo; // @public (undocumented) export class KibanaRequest { From 406ae803fbdfdf7ee239882489af0bd1822c7287 Mon Sep 17 00:00:00 2001 From: restrry Date: Thu, 18 Apr 2019 17:31:57 +0200 Subject: [PATCH 09/20] update snapshots: this functionality is tested in http server --- .../__snapshots__/http_service.test.ts.snap | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/src/core/server/http/__snapshots__/http_service.test.ts.snap b/src/core/server/http/__snapshots__/http_service.test.ts.snap index 601f5cdca956b..97b6b7de01f4a 100644 --- a/src/core/server/http/__snapshots__/http_service.test.ts.snap +++ b/src/core/server/http/__snapshots__/http_service.test.ts.snap @@ -15,35 +15,3 @@ Object { ], } `; - -exports[`register route handler 1`] = ` -Object { - "debug": Array [ - Array [ - "registering route handler for [/foo]", - ], - ], - "error": Array [], - "fatal": Array [], - "info": Array [], - "log": Array [], - "trace": Array [], - "warn": Array [], -} -`; - -exports[`throws if registering route handler after http server is set up 1`] = ` -Object { - "debug": Array [], - "error": Array [ - Array [ - "Received new router [/foo] after server was started. Router will **not** be applied.", - ], - ], - "fatal": Array [], - "info": Array [], - "log": Array [], - "trace": Array [], - "warn": Array [], -} -`; From bfd7b0ba6b90f1d468aaff111781ed89a0574ad2 Mon Sep 17 00:00:00 2001 From: restrry Date: Wed, 24 Apr 2019 09:38:23 +0300 Subject: [PATCH 10/20] split setup/start phases --- src/core/server/index.ts | 2 +- src/core/server/legacy/index.ts | 2 +- src/core/server/legacy/legacy_service.test.ts | 69 ++++++++++--------- src/core/server/legacy/legacy_service.ts | 29 ++++---- src/core/server/server.ts | 13 ++-- x-pack/plugins/apm/index.ts | 11 ++- .../apm_telemetry/make_apm_usage_collector.ts | 6 +- .../plugins/apm/server/new-platform/plugin.ts | 4 +- .../routes/__test__/routeFailures.test.ts | 6 +- x-pack/plugins/apm/server/routes/errors.ts | 4 +- x-pack/plugins/apm/server/routes/metrics.ts | 5 +- x-pack/plugins/apm/server/routes/services.ts | 4 +- x-pack/plugins/apm/server/routes/traces.ts | 4 +- .../apm/server/routes/transaction_groups.ts | 4 +- 14 files changed, 87 insertions(+), 76 deletions(-) diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 1aa602e32c384..0870ef76633c9 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -57,7 +57,7 @@ export interface CoreSetup { } export interface CoreStart { - http: HttpServiceStart; + http?: HttpServiceStart; } export { HttpServiceSetup, HttpServiceStart, ElasticsearchServiceSetup, PluginsServiceSetup }; diff --git a/src/core/server/legacy/index.ts b/src/core/server/legacy/index.ts index 74c10a83ba7d1..bb965c0272d14 100644 --- a/src/core/server/legacy/index.ts +++ b/src/core/server/legacy/index.ts @@ -20,4 +20,4 @@ /** @internal */ export { LegacyObjectToConfigAdapter } from './config/legacy_object_to_config_adapter'; /** @internal */ -export { LegacyService, SetupDeps } from './legacy_service'; +export { LegacyService } from './legacy_service'; diff --git a/src/core/server/legacy/legacy_service.test.ts b/src/core/server/legacy/legacy_service.test.ts index 9950c21276c35..549857e731085 100644 --- a/src/core/server/legacy/legacy_service.test.ts +++ b/src/core/server/legacy/legacy_service.test.ts @@ -100,7 +100,8 @@ afterEach(() => { describe('once LegacyService is set up with connection info', () => { test('register proxy route.', async () => { - await legacyService.start(setupDeps, startDeps); + await legacyService.setup(setupDeps); + await legacyService.start(startDeps); expect(startDeps.http.server.route.mock.calls).toMatchSnapshot('proxy route options'); }); @@ -116,7 +117,8 @@ describe('once LegacyService is set up with connection info', () => { // Wait until listen is called and proxy route is registered, but don't allow // listen to complete and make kbnServer available. - const legacySetupPromise = legacyService.start(setupDeps, startDeps); + await legacyService.setup(setupDeps); + const legacySetupPromise = legacyService.start(startDeps); await kbnServerListen$.pipe(first()).toPromise(); const mockResponse: any = { @@ -164,7 +166,8 @@ describe('once LegacyService is set up with connection info', () => { test('creates legacy kbnServer and calls `listen`.', async () => { configService.atPath.mockReturnValue(new BehaviorSubject({ autoListen: true })); - await legacyService.start(setupDeps, startDeps); + await legacyService.setup(setupDeps); + await legacyService.start(startDeps); expect(MockKbnServer).toHaveBeenCalledTimes(1); expect(MockKbnServer).toHaveBeenCalledWith( @@ -189,7 +192,8 @@ describe('once LegacyService is set up with connection info', () => { test('creates legacy kbnServer but does not call `listen` if `autoListen: false`.', async () => { configService.atPath.mockReturnValue(new BehaviorSubject({ autoListen: false })); - await legacyService.start(setupDeps, startDeps); + await legacyService.setup(setupDeps); + await legacyService.start(startDeps); expect(MockKbnServer).toHaveBeenCalledTimes(1); expect(MockKbnServer).toHaveBeenCalledWith( @@ -216,7 +220,8 @@ describe('once LegacyService is set up with connection info', () => { configService.atPath.mockReturnValue(new BehaviorSubject({ autoListen: true })); MockKbnServer.prototype.listen.mockRejectedValue(new Error('something failed')); - await expect(legacyService.start(setupDeps, startDeps)).rejects.toThrowErrorMatchingSnapshot(); + await legacyService.setup(setupDeps); + await expect(legacyService.start(startDeps)).rejects.toThrowErrorMatchingSnapshot(); const [mockKbnServer] = MockKbnServer.mock.instances; expect(mockKbnServer.listen).toHaveBeenCalled(); @@ -226,14 +231,16 @@ describe('once LegacyService is set up with connection info', () => { test('throws if fails to retrieve initial config.', async () => { configService.getConfig$.mockReturnValue(throwError(new Error('something failed'))); - await expect(legacyService.start(setupDeps, startDeps)).rejects.toThrowErrorMatchingSnapshot(); + await legacyService.setup(setupDeps); + await expect(legacyService.start(startDeps)).rejects.toThrowErrorMatchingSnapshot(); expect(MockKbnServer).not.toHaveBeenCalled(); expect(MockClusterManager).not.toHaveBeenCalled(); }); test('reconfigures logging configuration if new config is received.', async () => { - await legacyService.start(setupDeps, startDeps); + await legacyService.setup(setupDeps); + await legacyService.start(startDeps); const [mockKbnServer] = MockKbnServer.mock.instances as Array>; expect(mockKbnServer.applyLoggingConfiguration).not.toHaveBeenCalled(); @@ -246,7 +253,8 @@ describe('once LegacyService is set up with connection info', () => { }); test('logs error if re-configuring fails.', async () => { - await legacyService.start(setupDeps, startDeps); + await legacyService.setup(setupDeps); + await legacyService.start(startDeps); const [mockKbnServer] = MockKbnServer.mock.instances as Array>; expect(mockKbnServer.applyLoggingConfiguration).not.toHaveBeenCalled(); @@ -263,7 +271,8 @@ describe('once LegacyService is set up with connection info', () => { }); test('logs error if config service fails.', async () => { - await legacyService.start(setupDeps, startDeps); + await legacyService.setup(setupDeps); + await legacyService.start(startDeps); const [mockKbnServer] = MockKbnServer.mock.instances; expect(mockKbnServer.applyLoggingConfiguration).not.toHaveBeenCalled(); @@ -280,7 +289,8 @@ describe('once LegacyService is set up with connection info', () => { const mockResponseToolkit = { response: jest.fn(), abandon: Symbol('abandon') }; const mockRequest = { raw: { req: { a: 1 }, res: { b: 2 } } }; - await legacyService.start(setupDeps, startDeps); + await legacyService.setup(setupDeps); + await legacyService.start(startDeps); const [[{ handler }]] = startDeps.http.server.route.mock.calls; const response = await handler(mockRequest, mockResponseToolkit); @@ -304,7 +314,8 @@ describe('once LegacyService is set up without connection info', () => { http: undefined, }; beforeEach(async () => { - await legacyService.start(setupDeps, disabledHttpStartDeps); + await legacyService.setup(setupDeps); + await legacyService.start(disabledHttpStartDeps); }); test('creates legacy kbnServer with `autoListen: false`.', () => { @@ -354,16 +365,14 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => { configService: configService as any, }); - await devClusterLegacyService.start( - { - elasticsearch: setupDeps.elasticsearch, - plugins: { contracts: new Map(), uiPlugins: { public: new Map(), internal: new Map() } }, - http: setupDeps.http, - }, - { - http: undefined, - } - ); + await devClusterLegacyService.setup({ + elasticsearch: setupDeps.elasticsearch, + plugins: { contracts: new Map(), uiPlugins: { public: new Map(), internal: new Map() } }, + http: setupDeps.http, + }); + await devClusterLegacyService.start({ + http: undefined, + }); expect(MockClusterManager.create.mock.calls).toMatchSnapshot( 'cluster manager without base path proxy' @@ -382,16 +391,14 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => { configService: configService as any, }); - await devClusterLegacyService.start( - { - elasticsearch: setupDeps.elasticsearch, - plugins: { contracts: new Map(), uiPlugins: { public: new Map(), internal: new Map() } }, - http: setupDeps.http, - }, - { - http: undefined, - } - ); + await devClusterLegacyService.setup({ + elasticsearch: setupDeps.elasticsearch, + plugins: { contracts: new Map(), uiPlugins: { public: new Map(), internal: new Map() } }, + http: setupDeps.http, + }); + await devClusterLegacyService.start({ + http: undefined, + }); expect(MockClusterManager.create.mock.calls).toMatchSnapshot( 'cluster manager with base path proxy' diff --git a/src/core/server/legacy/legacy_service.ts b/src/core/server/legacy/legacy_service.ts index 46f7dd66787a9..3503a1471b6d6 100644 --- a/src/core/server/legacy/legacy_service.ts +++ b/src/core/server/legacy/legacy_service.ts @@ -21,13 +21,12 @@ import { Server as HapiServer } from 'hapi'; import { combineLatest, ConnectableObservable, EMPTY, Subscription } from 'rxjs'; import { first, map, mergeMap, publishReplay, tap } from 'rxjs/operators'; import { CoreService } from '../../types'; +import { CoreSetup, CoreStart } from '../../server'; import { Config } from '../config'; import { CoreContext } from '../core_context'; import { DevConfig } from '../dev'; -import { ElasticsearchServiceSetup } from '../elasticsearch'; -import { BasePathProxyServer, HttpConfig, HttpServiceSetup, HttpServiceStart } from '../http'; +import { BasePathProxyServer, HttpConfig } from '../http'; import { Logger } from '../logging'; -import { PluginsServiceSetup } from '../plugins/plugins_service'; import { LegacyPlatformProxy } from './legacy_platform_proxy'; interface LegacyKbnServer { @@ -37,16 +36,6 @@ interface LegacyKbnServer { close: () => Promise; } -export interface SetupDeps { - elasticsearch: ElasticsearchServiceSetup; - http: HttpServiceSetup; - plugins: PluginsServiceSetup; -} - -export interface StartDeps { - http?: HttpServiceStart; -} - function getLegacyRawConfig(config: Config) { const rawConfig = config.toRaw(); @@ -64,12 +53,15 @@ export class LegacyService implements CoreService { private readonly log: Logger; private kbnServer?: LegacyKbnServer; private configSubscription?: Subscription; + private setupDeps?: CoreSetup; constructor(private readonly coreContext: CoreContext) { this.log = coreContext.logger.get('legacy-service'); } - public async setup() {} - public async start(setupDeps: SetupDeps, startDeps: StartDeps) { + public async setup(setupDeps: CoreSetup) { + this.setupDeps = setupDeps; + } + public async start(startDeps: CoreStart) { this.log.debug('setting up legacy service'); const update$ = this.coreContext.configService.getConfig$().pipe( @@ -93,7 +85,10 @@ export class LegacyService implements CoreService { await this.createClusterManager(config); return; } - return await this.createKbnServer(config, setupDeps, startDeps); + if (!this.setupDeps) { + throw new Error('Core setup contract is not defined'); + } + return await this.createKbnServer(config, this.setupDeps, startDeps); }) ) .toPromise(); @@ -134,7 +129,7 @@ export class LegacyService implements CoreService { ); } - private async createKbnServer(config: Config, setupDeps: SetupDeps, startDeps: StartDeps) { + private async createKbnServer(config: Config, setupDeps: CoreSetup, startDeps: CoreStart) { // eslint-disable-next-line @typescript-eslint/no-var-requires const KbnServer = require('../../../legacy/server/kbn_server'); const kbnServer: LegacyKbnServer = new KbnServer(getLegacyRawConfig(config), { diff --git a/src/core/server/server.ts b/src/core/server/server.ts index 49ca3e2051d6d..6d2b2c4356f22 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -21,7 +21,7 @@ import { first } from 'rxjs/operators'; import { ConfigService, Env } from './config'; import { ElasticsearchService } from './elasticsearch'; import { HttpConfig, HttpService, HttpServiceSetup, HttpServiceStart, Router } from './http'; -import { LegacyService, SetupDeps } from './legacy'; +import { LegacyService } from './legacy'; import { Logger, LoggerFactory } from './logging'; import { PluginsService } from './plugins'; @@ -32,8 +32,6 @@ export class Server { private readonly legacy: LegacyService; private readonly log: Logger; - private setupDeps?: SetupDeps; - constructor( private readonly configService: ConfigService, logger: LoggerFactory, @@ -61,12 +59,15 @@ export class Server { http: httpSetup, }); - this.setupDeps = { + const coreSetup = { elasticsearch: elasticsearchServiceSetup, http: httpSetup, plugins: pluginsSetup, }; - return this.setupDeps; + + this.legacy.setup(coreSetup); + + return coreSetup; } public async start() { @@ -88,7 +89,7 @@ export class Server { http: httpStart, }; - await this.legacy.start(this.setupDeps!, startDeps); + await this.legacy.start(startDeps); return startDeps; } diff --git a/x-pack/plugins/apm/index.ts b/x-pack/plugins/apm/index.ts index 9a06bfb5fbf96..2d1857fac161b 100644 --- a/x-pack/plugins/apm/index.ts +++ b/x-pack/plugins/apm/index.ts @@ -7,10 +7,17 @@ import { i18n } from '@kbn/i18n'; import { Server } from 'hapi'; import { resolve } from 'path'; -import { CoreStart, PluginInitializerContext } from 'src/core/server/index.js'; +import { + PluginInitializerContext, + HttpServiceStart +} from 'src/core/server/index.js'; import mappings from './mappings.json'; import { plugin } from './server/new-platform/index'; +export interface SetupDeps { + http: HttpServiceStart; +} + // TODO: get proper types export function apm(kibana: any) { return new kibana.Plugin({ @@ -106,7 +113,7 @@ export function apm(kibana: any) { http: { server } - } as CoreStart; + } as SetupDeps; plugin(initializerContext).setup(core); } }); diff --git a/x-pack/plugins/apm/server/lib/apm_telemetry/make_apm_usage_collector.ts b/x-pack/plugins/apm/server/lib/apm_telemetry/make_apm_usage_collector.ts index a182ef0c8f17c..a1148e97eb19e 100644 --- a/x-pack/plugins/apm/server/lib/apm_telemetry/make_apm_usage_collector.ts +++ b/x-pack/plugins/apm/server/lib/apm_telemetry/make_apm_usage_collector.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { CoreStart } from 'src/core/server'; +import { SetupDeps } from '../../..'; import { APM_TELEMETRY_DOC_ID, createApmTelementry, @@ -12,8 +12,8 @@ import { } from './apm_telemetry'; // TODO this type should be defined by the platform -export interface CoreStartWithUsageCollector extends CoreStart { - http: CoreStart['http'] & { +export interface CoreStartWithUsageCollector extends SetupDeps { + http: SetupDeps['http'] & { server: { usage: { collectorSet: { diff --git a/x-pack/plugins/apm/server/new-platform/plugin.ts b/x-pack/plugins/apm/server/new-platform/plugin.ts index 7eb23509f0cf2..e6d9fc306121f 100644 --- a/x-pack/plugins/apm/server/new-platform/plugin.ts +++ b/x-pack/plugins/apm/server/new-platform/plugin.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { CoreStart } from 'src/core/server'; +import { SetupDeps } from '../..'; import { makeApmUsageCollector } from '../lib/apm_telemetry'; import { CoreStartWithUsageCollector } from '../lib/apm_telemetry/make_apm_usage_collector'; import { initErrorsApi } from '../routes/errors'; @@ -14,7 +14,7 @@ import { initTracesApi } from '../routes/traces'; import { initTransactionGroupsApi } from '../routes/transaction_groups'; export class Plugin { - public setup(core: CoreStart) { + public setup(core: SetupDeps) { initTransactionGroupsApi(core); initTracesApi(core); initServicesApi(core); diff --git a/x-pack/plugins/apm/server/routes/__test__/routeFailures.test.ts b/x-pack/plugins/apm/server/routes/__test__/routeFailures.test.ts index ace1b75ba88e2..0c17378f1345b 100644 --- a/x-pack/plugins/apm/server/routes/__test__/routeFailures.test.ts +++ b/x-pack/plugins/apm/server/routes/__test__/routeFailures.test.ts @@ -5,7 +5,7 @@ */ import { flatten } from 'lodash'; -import { CoreStart } from 'src/core/server'; +import { SetupDeps } from '../../..'; import { initErrorsApi } from '../errors'; import { initServicesApi } from '../services'; import { initTracesApi } from '../traces'; @@ -13,13 +13,13 @@ import { initTracesApi } from '../traces'; describe('route handlers should fail with a Boom error', () => { let consoleErrorSpy: any; - async function testRouteFailures(init: (core: CoreStart) => void) { + async function testRouteFailures(init: (core: SetupDeps) => void) { const mockServer = { route: jest.fn() }; const mockCore = ({ http: { server: mockServer } - } as unknown) as CoreStart; + } as unknown) as SetupDeps; init(mockCore); expect(mockServer.route).toHaveBeenCalled(); diff --git a/x-pack/plugins/apm/server/routes/errors.ts b/x-pack/plugins/apm/server/routes/errors.ts index 45c7193e471bb..c327ad749698a 100644 --- a/x-pack/plugins/apm/server/routes/errors.ts +++ b/x-pack/plugins/apm/server/routes/errors.ts @@ -7,7 +7,7 @@ import Boom from 'boom'; import Joi from 'joi'; import { Legacy } from 'kibana'; -import { CoreStart } from 'src/core/server'; +import { SetupDeps } from '../..'; import { getDistribution } from '../lib/errors/distribution/get_distribution'; import { getErrorGroup } from '../lib/errors/get_error_group'; import { getErrorGroups } from '../lib/errors/get_error_groups'; @@ -21,7 +21,7 @@ const defaultErrorHandler = (err: Error) => { throw Boom.boomify(err, { statusCode: 400 }); }; -export function initErrorsApi(core: CoreStart) { +export function initErrorsApi(core: SetupDeps) { const { server } = core.http; server.route({ method: 'GET', diff --git a/x-pack/plugins/apm/server/routes/metrics.ts b/x-pack/plugins/apm/server/routes/metrics.ts index e68d0100858a3..cd821df76cedd 100644 --- a/x-pack/plugins/apm/server/routes/metrics.ts +++ b/x-pack/plugins/apm/server/routes/metrics.ts @@ -5,7 +5,7 @@ */ import Boom from 'boom'; -import { CoreStart } from 'src/core/server'; +import { SetupDeps } from '../..'; import { withDefaultValidators } from '../lib/helpers/input_validation'; import { setupRequest } from '../lib/helpers/setup_request'; import { getAllMetricsChartData } from '../lib/metrics/get_all_metrics_chart_data'; @@ -16,7 +16,8 @@ const defaultErrorHandler = (err: Error) => { throw Boom.boomify(err, { statusCode: 400 }); }; -export function initMetricsApi(core: CoreStart) { +export function initMetricsApi(core: SetupDeps) { + if (!core.http) return; const { server } = core.http; server.route({ method: 'GET', diff --git a/x-pack/plugins/apm/server/routes/services.ts b/x-pack/plugins/apm/server/routes/services.ts index 2ed891626c97c..3a32817b1152e 100644 --- a/x-pack/plugins/apm/server/routes/services.ts +++ b/x-pack/plugins/apm/server/routes/services.ts @@ -5,7 +5,7 @@ */ import Boom from 'boom'; -import { CoreStart } from 'src/core/server'; +import { SetupDeps } from '../..'; import { AgentName } from '../../typings/es_schemas/ui/fields/Agent'; import { createApmTelementry, storeApmTelemetry } from '../lib/apm_telemetry'; import { withDefaultValidators } from '../lib/helpers/input_validation'; @@ -20,7 +20,7 @@ const defaultErrorHandler = (err: Error) => { throw Boom.boomify(err, { statusCode: 400 }); }; -export function initServicesApi(core: CoreStart) { +export function initServicesApi(core: SetupDeps) { const { server } = core.http; server.route({ method: 'GET', diff --git a/x-pack/plugins/apm/server/routes/traces.ts b/x-pack/plugins/apm/server/routes/traces.ts index 4f7bef95fc64b..8f61486287ed0 100644 --- a/x-pack/plugins/apm/server/routes/traces.ts +++ b/x-pack/plugins/apm/server/routes/traces.ts @@ -6,7 +6,7 @@ import Boom from 'boom'; -import { CoreStart } from 'src/core/server'; +import { SetupDeps } from '../..'; import { withDefaultValidators } from '../lib/helpers/input_validation'; import { setupRequest } from '../lib/helpers/setup_request'; import { getTopTraces } from '../lib/traces/get_top_traces'; @@ -19,7 +19,7 @@ const defaultErrorHandler = (err: Error) => { throw Boom.boomify(err, { statusCode: 400 }); }; -export function initTracesApi(core: CoreStart) { +export function initTracesApi(core: SetupDeps) { const { server } = core.http; // Get trace list diff --git a/x-pack/plugins/apm/server/routes/transaction_groups.ts b/x-pack/plugins/apm/server/routes/transaction_groups.ts index 8614866d4360a..2ee942b008bb3 100644 --- a/x-pack/plugins/apm/server/routes/transaction_groups.ts +++ b/x-pack/plugins/apm/server/routes/transaction_groups.ts @@ -6,7 +6,7 @@ import Boom from 'boom'; import Joi from 'joi'; -import { CoreStart } from 'src/core/server'; +import { SetupDeps } from '../..'; import { withDefaultValidators } from '../lib/helpers/input_validation'; import { setupRequest } from '../lib/helpers/setup_request'; import { getChartsData } from '../lib/transactions/charts'; @@ -19,7 +19,7 @@ const defaultErrorHandler = (err: Error) => { throw Boom.boomify(err, { statusCode: 400 }); }; -export function initTransactionGroupsApi(core: CoreStart) { +export function initTransactionGroupsApi(core: SetupDeps) { const { server } = core.http; server.route({ From f504d009c5948f8e68ba883903ee1f5b88c22a40 Mon Sep 17 00:00:00 2001 From: restrry Date: Wed, 24 Apr 2019 09:56:23 +0300 Subject: [PATCH 11/20] update docs --- .../core/server/kibana-plugin-server.corestart.http.md | 2 +- src/core/server/server.api.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/development/core/server/kibana-plugin-server.corestart.http.md b/docs/development/core/server/kibana-plugin-server.corestart.http.md index a12fcbaab27cc..51ab1e5dbe303 100644 --- a/docs/development/core/server/kibana-plugin-server.corestart.http.md +++ b/docs/development/core/server/kibana-plugin-server.corestart.http.md @@ -7,5 +7,5 @@ Signature: ```typescript -http: HttpServiceStart; +http?: HttpServiceStart; ``` diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 9c20bde8218af..00e32ba77e3c6 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -77,7 +77,7 @@ export interface CoreSetup { // @public (undocumented) export interface CoreStart { // (undocumented) - http: HttpServiceStart; + http?: HttpServiceStart; } // @internal From ea34a0e56f938814c885b5cad80442e791917347 Mon Sep 17 00:00:00 2001 From: restrry Date: Thu, 25 Apr 2019 11:01:55 +0300 Subject: [PATCH 12/20] expose http server if it not started to get rid of Optional type and make it Require --- src/core/server/http/http_server.ts | 18 +++------ src/core/server/http/http_service.ts | 44 ++++++++++++++-------- src/core/server/http/index.ts | 1 - src/core/server/index.ts | 2 +- src/core/server/legacy/legacy_service.ts | 13 +++---- src/core/server/plugins/plugin_context.ts | 19 ++-------- src/core/server/plugins/plugins_service.ts | 2 +- src/core/server/server.ts | 29 +++----------- src/legacy/server/kbn_server.d.ts | 2 +- 9 files changed, 53 insertions(+), 77 deletions(-) diff --git a/src/core/server/http/http_server.ts b/src/core/server/http/http_server.ts index e9f301ca18256..d1c1b8fd688eb 100644 --- a/src/core/server/http/http_server.ts +++ b/src/core/server/http/http_server.ts @@ -32,6 +32,7 @@ import { } from './cookie_session_storage'; export interface HttpServerSetup { + server: Server; options: ServerOptions; registerRouter: (router: Router) => void; /** @@ -50,10 +51,6 @@ export interface HttpServerSetup { registerOnRequest: (requestHandler: OnRequestHandler) => void; } -export interface HttpServerInfo { - server: Server; -} - export class HttpServer { private server?: Server; private registeredRouters = new Set(); @@ -86,10 +83,14 @@ export class HttpServer { fn: AuthenticationHandler, cookieOptions: SessionStorageCookieOptions ) => this.registerAuth(fn, cookieOptions, config.basePath), + // Return server instance with the connection options so that we can properly + // bridge core and the "legacy" Kibana internally. Once this bridge isn't + // needed anymore we shouldn't return the instance from this method. + server: this.server, }; } - public async start(config: HttpConfig): Promise { + public async start(config: HttpConfig) { if (this.server === undefined) { throw new Error('server is not setup up yet'); } @@ -114,13 +115,6 @@ export class HttpServer { config.rewriteBasePath ? config.basePath : '' }` ); - - // Return server instance with the connection options so that we can properly - // bridge core and the "legacy" Kibana internally. Once this bridge isn't - // needed anymore we shouldn't return anything from this method. - return { - server: this.server, - }; } public async stop() { diff --git a/src/core/server/http/http_service.ts b/src/core/server/http/http_service.ts index ae1aabef3ea37..005488b81689c 100644 --- a/src/core/server/http/http_service.ts +++ b/src/core/server/http/http_service.ts @@ -21,28 +21,35 @@ import { Observable, Subscription } from 'rxjs'; import { first } from 'rxjs/operators'; import { CoreService } from '../../types'; -import { Logger, LoggerFactory } from '../logging'; +import { Logger } from '../logging'; +import { CoreContext } from '../core_context'; import { HttpConfig } from './http_config'; -import { HttpServer, HttpServerSetup, HttpServerInfo } from './http_server'; +import { HttpServer, HttpServerSetup } from './http_server'; import { HttpsRedirectServer } from './https_redirect_server'; /** @public */ export type HttpServiceSetup = HttpServerSetup; -export type HttpServiceStart = HttpServerInfo; +export interface HttpServiceStart { + isListening: () => boolean; +} /** @internal */ export class HttpService implements CoreService { private readonly httpServer: HttpServer; private readonly httpsRedirectServer: HttpsRedirectServer; + private readonly config$: Observable; private configSubscription?: Subscription; private readonly log: Logger; - constructor(private readonly config$: Observable, logger: LoggerFactory) { - this.log = logger.get('http'); + constructor(private readonly coreContext: CoreContext) { + this.log = coreContext.logger.get('http'); + this.config$ = this.coreContext.configService.atPath('server', HttpConfig); - this.httpServer = new HttpServer(logger.get('http', 'server')); - this.httpsRedirectServer = new HttpsRedirectServer(logger.get('http', 'redirect', 'server')); + this.httpServer = new HttpServer(coreContext.logger.get('http', 'server')); + this.httpsRedirectServer = new HttpsRedirectServer( + coreContext.logger.get('http', 'redirect', 'server') + ); } public async setup() { @@ -64,16 +71,23 @@ export class HttpService implements CoreService this.httpServer.isListening(), + }; } public async stop() { diff --git a/src/core/server/http/index.ts b/src/core/server/http/index.ts index d0595c7415e05..633fcd9885212 100644 --- a/src/core/server/http/index.ts +++ b/src/core/server/http/index.ts @@ -20,7 +20,6 @@ export { HttpConfig } from './http_config'; export { HttpService, HttpServiceSetup, HttpServiceStart } from './http_service'; export { Router, KibanaRequest } from './router'; -export { HttpServerInfo } from './http_server'; export { BasePathProxyServer } from './base_path_proxy_server'; export { AuthenticationHandler, AuthToolkit } from './lifecycle/auth'; export { OnRequestHandler, OnRequestToolkit } from './lifecycle/on_request'; diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 0870ef76633c9..1aa602e32c384 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -57,7 +57,7 @@ export interface CoreSetup { } export interface CoreStart { - http?: HttpServiceStart; + http: HttpServiceStart; } export { HttpServiceSetup, HttpServiceStart, ElasticsearchServiceSetup, PluginsServiceSetup }; diff --git a/src/core/server/legacy/legacy_service.ts b/src/core/server/legacy/legacy_service.ts index 3503a1471b6d6..c0b02a0f11def 100644 --- a/src/core/server/legacy/legacy_service.ts +++ b/src/core/server/legacy/legacy_service.ts @@ -138,13 +138,12 @@ export class LegacyService implements CoreService { // bridge with the "legacy" Kibana. If server isn't run (e.g. if process is // managed by ClusterManager or optimizer) then we won't have that info, // so we can't start "legacy" server either. - serverOptions: - startDeps.http !== undefined - ? { - ...setupDeps.http.options, - listener: this.setupProxyListener(startDeps.http.server), - } - : { autoListen: false }, + serverOptions: startDeps.http.isListening() + ? { + ...setupDeps.http.options, + listener: this.setupProxyListener(setupDeps.http.server), + } + : { autoListen: false }, handledConfigPaths: await this.coreContext.configService.getUsedPaths(), setupDeps, startDeps, diff --git a/src/core/server/plugins/plugin_context.ts b/src/core/server/plugins/plugin_context.ts index 98e02080e045b..1de59ddaef8a3 100644 --- a/src/core/server/plugins/plugin_context.ts +++ b/src/core/server/plugins/plugin_context.ts @@ -114,12 +114,6 @@ export function createPluginInitializerContext( }; } -// Added to improve http typings as make { http: Required } -// Http service is disabled, when Kibana runs in optimizer mode or as dev cluster managed by cluster master. -// In theory no plugins shouldn try to access http dependency in this case. -function preventAccess() { - throw new Error('Cannot use http contract when http server not started'); -} /** * This returns a facade for `CoreContext` that will be exposed to the plugin `setup` method. * This facade should be safe to use only within `setup` itself. @@ -144,14 +138,9 @@ export function createPluginSetupContext( adminClient$: deps.elasticsearch.adminClient$, dataClient$: deps.elasticsearch.dataClient$, }, - http: deps.http - ? { - registerAuth: deps.http.registerAuth, - registerOnRequest: deps.http.registerOnRequest, - } - : { - registerAuth: preventAccess, - registerOnRequest: preventAccess, - }, + http: { + registerAuth: deps.http.registerAuth, + registerOnRequest: deps.http.registerOnRequest, + }, }; } diff --git a/src/core/server/plugins/plugins_service.ts b/src/core/server/plugins/plugins_service.ts index df639c8de7bd5..0caba3477bf24 100644 --- a/src/core/server/plugins/plugins_service.ts +++ b/src/core/server/plugins/plugins_service.ts @@ -41,7 +41,7 @@ export interface PluginsServiceSetup { /** @internal */ export interface PluginsServiceSetupDeps { elasticsearch: ElasticsearchServiceSetup; - http?: HttpServiceSetup; + http: HttpServiceSetup; } /** @internal */ diff --git a/src/core/server/server.ts b/src/core/server/server.ts index 6d2b2c4356f22..7cef2f3beba93 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -17,10 +17,9 @@ * under the License. */ -import { first } from 'rxjs/operators'; import { ConfigService, Env } from './config'; import { ElasticsearchService } from './elasticsearch'; -import { HttpConfig, HttpService, HttpServiceSetup, HttpServiceStart, Router } from './http'; +import { HttpService, HttpServiceSetup, Router } from './http'; import { LegacyService } from './legacy'; import { Logger, LoggerFactory } from './logging'; import { PluginsService } from './plugins'; @@ -32,16 +31,10 @@ export class Server { private readonly legacy: LegacyService; private readonly log: Logger; - constructor( - private readonly configService: ConfigService, - logger: LoggerFactory, - private readonly env: Env - ) { - this.log = logger.get('server'); - - this.http = new HttpService(configService.atPath('server', HttpConfig), logger); - + constructor(configService: ConfigService, logger: LoggerFactory, env: Env) { const core = { env, configService, logger }; + this.log = logger.get('server'); + this.http = new HttpService(core); this.plugins = new PluginsService(core); this.legacy = new LegacyService(core); this.elasticsearch = new ElasticsearchService(core); @@ -71,19 +64,7 @@ export class Server { } public async start() { - const httpConfig = await this.configService - .atPath('server', HttpConfig) - .pipe(first()) - .toPromise(); - - let httpStart: HttpServiceStart | undefined; - // We shouldn't set up http service in two cases: - // 1. If `server.autoListen` is explicitly set to `false`. - // 2. When the process is run as dev cluster master in which case cluster manager - // will fork a dedicated process where http service will be set up instead. - if (!this.env.isDevClusterMaster && httpConfig.autoListen) { - httpStart = await this.http.start(); - } + const httpStart = await this.http.start(); const startDeps = { http: httpStart, diff --git a/src/legacy/server/kbn_server.d.ts b/src/legacy/server/kbn_server.d.ts index 978c41f62c02a..93573bf4f342c 100644 --- a/src/legacy/server/kbn_server.d.ts +++ b/src/legacy/server/kbn_server.d.ts @@ -84,7 +84,7 @@ export default class KbnServer { }; start: { core: { - http?: HttpServiceStart; + http: HttpServiceStart; }; }; stop: null; From 3d245f5aa13ac34be6992e3c7023e54a40213cba Mon Sep 17 00:00:00 2001 From: restrry Date: Thu, 25 Apr 2019 11:03:49 +0300 Subject: [PATCH 13/20] adopt test to exposed Http server via SetupContract --- .../__snapshots__/http_service.test.ts.snap | 16 +-- src/core/server/http/http_config.ts | 2 +- src/core/server/http/http_server.test.ts | 63 ++++++----- src/core/server/http/http_service.mock.ts | 6 +- src/core/server/http/http_service.test.ts | 101 ++++++++++++++---- src/core/server/legacy/legacy_service.test.ts | 27 +++-- src/core/server/plugins/plugin.test.ts | 6 +- .../server/plugins/plugins_service.test.ts | 6 +- .../server/plugins/plugins_system.test.ts | 6 +- src/core/server/server.test.ts | 30 ------ x-pack/plugins/apm/index.ts | 11 +- .../apm_telemetry/make_apm_usage_collector.ts | 6 +- .../apm/server/lib/index_pattern/index.ts | 4 +- .../plugins/apm/server/new-platform/plugin.ts | 4 +- .../routes/__test__/routeFailures.test.ts | 6 +- x-pack/plugins/apm/server/routes/errors.ts | 4 +- x-pack/plugins/apm/server/routes/metrics.ts | 5 +- x-pack/plugins/apm/server/routes/services.ts | 4 +- x-pack/plugins/apm/server/routes/traces.ts | 4 +- .../apm/server/routes/transaction_groups.ts | 4 +- 20 files changed, 173 insertions(+), 142 deletions(-) diff --git a/src/core/server/http/__snapshots__/http_service.test.ts.snap b/src/core/server/http/__snapshots__/http_service.test.ts.snap index 97b6b7de01f4a..2ff4fa987c7e4 100644 --- a/src/core/server/http/__snapshots__/http_service.test.ts.snap +++ b/src/core/server/http/__snapshots__/http_service.test.ts.snap @@ -1,17 +1,9 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`logs error if already set up 1`] = ` -Object { - "debug": Array [], - "error": Array [], - "fatal": Array [], - "info": Array [], - "log": Array [], - "trace": Array [], - "warn": Array [ - Array [ - "Received new HTTP config after server was started. Config will **not** be applied.", - ], +Array [ + Array [ + "Received new HTTP config after server was started. Config will **not** be applied.", ], -} +] `; diff --git a/src/core/server/http/http_config.ts b/src/core/server/http/http_config.ts index e681d9634abb5..ebf3bd8023f1a 100644 --- a/src/core/server/http/http_config.ts +++ b/src/core/server/http/http_config.ts @@ -83,7 +83,7 @@ const createHttpSchema = schema.object( } ); -type HttpConfigType = TypeOf; +export type HttpConfigType = TypeOf; export class HttpConfig { /** diff --git a/src/core/server/http/http_server.test.ts b/src/core/server/http/http_server.test.ts index 4ccc5b733cf49..5e7abc0224d4d 100644 --- a/src/core/server/http/http_server.test.ts +++ b/src/core/server/http/http_server.test.ts @@ -69,9 +69,9 @@ test('200 OK with body', async () => { return res.ok({ key: 'value' }); }); - const { registerRouter } = await server.setup(config); + const { registerRouter, server: innerServer } = await server.setup(config); registerRouter(router); - const { server: innerServer } = await server.start(config); + await server.start(config); await supertest(innerServer.listener) .get('/foo/') @@ -88,10 +88,10 @@ test('202 Accepted with body', async () => { return res.accepted({ location: 'somewhere' }); }); - const { registerRouter } = await server.setup(config); + const { registerRouter, server: innerServer } = await server.setup(config); registerRouter(router); - const { server: innerServer } = await server.start(config); + await server.start(config); await supertest(innerServer.listener) .get('/foo/') @@ -108,10 +108,10 @@ test('204 No content', async () => { return res.noContent(); }); - const { registerRouter } = await server.setup(config); + const { registerRouter, server: innerServer } = await server.setup(config); registerRouter(router); - const { server: innerServer } = await server.start(config); + await server.start(config); await supertest(innerServer.listener) .get('/foo/') @@ -130,10 +130,10 @@ test('400 Bad request with error', async () => { return res.badRequest(err); }); - const { registerRouter } = await server.setup(config); + const { registerRouter, server: innerServer } = await server.setup(config); registerRouter(router); - const { server: innerServer } = await server.start(config); + await server.start(config); await supertest(innerServer.listener) .get('/foo/') @@ -160,10 +160,10 @@ test('valid params', async () => { } ); - const { registerRouter } = await server.setup(config); + const { registerRouter, server: innerServer } = await server.setup(config); registerRouter(router); - const { server: innerServer } = await server.start(config); + await server.start(config); await supertest(innerServer.listener) .get('/foo/some-string') @@ -190,10 +190,10 @@ test('invalid params', async () => { } ); - const { registerRouter } = await server.setup(config); + const { registerRouter, server: innerServer } = await server.setup(config); registerRouter(router); - const { server: innerServer } = await server.start(config); + await server.start(config); await supertest(innerServer.listener) .get('/foo/some-string') @@ -223,10 +223,10 @@ test('valid query', async () => { } ); - const { registerRouter } = await server.setup(config); + const { registerRouter, server: innerServer } = await server.setup(config); registerRouter(router); - const { server: innerServer } = await server.start(config); + await server.start(config); await supertest(innerServer.listener) .get('/foo/?bar=test&quux=123') @@ -253,10 +253,10 @@ test('invalid query', async () => { } ); - const { registerRouter } = await server.setup(config); + const { registerRouter, server: innerServer } = await server.setup(config); registerRouter(router); - const { server: innerServer } = await server.start(config); + await server.start(config); await supertest(innerServer.listener) .get('/foo/?bar=test') @@ -286,10 +286,10 @@ test('valid body', async () => { } ); - const { registerRouter } = await server.setup(config); + const { registerRouter, server: innerServer } = await server.setup(config); registerRouter(router); - const { server: innerServer } = await server.start(config); + await server.start(config); await supertest(innerServer.listener) .post('/foo/') @@ -320,10 +320,10 @@ test('invalid body', async () => { } ); - const { registerRouter } = await server.setup(config); + const { registerRouter, server: innerServer } = await server.setup(config); registerRouter(router); - const { server: innerServer } = await server.start(config); + await server.start(config); await supertest(innerServer.listener) .post('/foo/') @@ -353,10 +353,10 @@ test('handles putting', async () => { } ); - const { registerRouter } = await server.setup(config); + const { registerRouter, server: innerServer } = await server.setup(config); registerRouter(router); - const { server: innerServer } = await server.start(config); + await server.start(config); await supertest(innerServer.listener) .put('/foo/') @@ -384,10 +384,10 @@ test('handles deleting', async () => { } ); - const { registerRouter } = await server.setup(config); + const { registerRouter, server: innerServer } = await server.setup(config); registerRouter(router); - const { server: innerServer } = await server.start(config); + await server.start(config); await supertest(innerServer.listener) .delete('/foo/3') @@ -410,10 +410,10 @@ test('filtered headers', async () => { return res.noContent(); }); - const { registerRouter } = await server.setup(config); + const { registerRouter, server: innerServer } = await server.setup(config); registerRouter(router); - const { server: innerServer } = await server.start(config); + await server.start(config); await supertest(innerServer.listener) .get('/foo/?bar=quux') @@ -443,10 +443,10 @@ describe('with `basepath: /bar` and `rewriteBasePath: false`', () => { res.ok({ key: 'value:/foo' }) ); - const { registerRouter } = await server.setup(config); + const { registerRouter, server: innerServer } = await server.setup(config); registerRouter(router); - const { server: innerServer } = await server.start(configWithBasePath); + await server.start(configWithBasePath); innerServerListener = innerServer.listener; }); @@ -504,10 +504,10 @@ describe('with `basepath: /bar` and `rewriteBasePath: true`', () => { res.ok({ key: 'value:/foo' }) ); - const { registerRouter } = await server.setup(config); + const { registerRouter, server: innerServer } = await server.setup(config); registerRouter(router); - const { server: innerServer } = await server.start(configWithBasePath); + await server.start(configWithBasePath); innerServerListener = innerServer.listener; }); @@ -582,8 +582,7 @@ test('returns server and connection options on start', async () => { ...config, port: 12345, }; - const { options } = await server.setup(configWithPort); - const { server: innerServer } = await server.start(configWithPort); + const { options, server: innerServer } = await server.setup(configWithPort); expect(innerServer).toBeDefined(); expect(innerServer).toBe((server as any).server); diff --git a/src/core/server/http/http_service.mock.ts b/src/core/server/http/http_service.mock.ts index c4ca79429dc12..fe9b8a082ace5 100644 --- a/src/core/server/http/http_service.mock.ts +++ b/src/core/server/http/http_service.mock.ts @@ -26,15 +26,17 @@ const createSetupContractMock = () => { registerAuth: jest.fn(), registerOnRequest: jest.fn(), registerRouter: jest.fn(), + // we can mock some hapi server method when we need it + server: {} as Server, }; return setupContract; }; const createStartContractMock = () => { const startContract = { - // we can mock some hapi server method when we need it - server: {} as Server, + isListening: jest.fn(), }; + startContract.isListening.mockReturnValue(true); return startContract; }; diff --git a/src/core/server/http/http_service.test.ts b/src/core/server/http/http_service.test.ts index 6d4f7ddb4bd53..dd66a050e3302 100644 --- a/src/core/server/http/http_service.test.ts +++ b/src/core/server/http/http_service.test.ts @@ -21,23 +21,35 @@ import { mockHttpServer } from './http_service.test.mocks'; import { noop } from 'lodash'; import { BehaviorSubject } from 'rxjs'; -import { HttpConfig, HttpService, Router } from '.'; +import { HttpService, Router } from '.'; +import { HttpConfigType } from './http_config'; +import { Config, ConfigService, Env, ObjectToConfigAdapter } from '../config'; import { loggingServiceMock } from '../logging/logging_service.mock'; +import { getEnvOptions } from '../config/__mocks__/env'; const logger = loggingServiceMock.create(); +const env = Env.createDefault(getEnvOptions()); + +const createConfigService = (value: Partial = {}) => + new ConfigService( + new BehaviorSubject( + new ObjectToConfigAdapter({ + http: value, + }) + ), + env, + logger + ); afterEach(() => { jest.clearAllMocks(); }); test('creates and sets up http server', async () => { - const config = { + const configService = createConfigService({ host: 'example.org', port: 1234, - ssl: {}, - } as HttpConfig; - - const config$ = new BehaviorSubject(config); + }); const httpServer = { isListening: () => false, @@ -47,24 +59,22 @@ test('creates and sets up http server', async () => { }; mockHttpServer.mockImplementation(() => httpServer); - const service = new HttpService(config$.asObservable(), logger); + const service = new HttpService({ configService, env, logger }); expect(mockHttpServer.mock.instances.length).toBe(1); expect(httpServer.setup).not.toHaveBeenCalled(); - expect(httpServer.start).not.toHaveBeenCalled(); await service.setup(); expect(httpServer.setup).toHaveBeenCalledTimes(1); + expect(httpServer.start).not.toHaveBeenCalled(); await service.start(); expect(httpServer.start).toHaveBeenCalledTimes(1); }); test('logs error if already set up', async () => { - const config = { ssl: {} } as HttpConfig; - - const config$ = new BehaviorSubject(config); + const configService = createConfigService(); const httpServer = { isListening: () => true, @@ -74,17 +84,15 @@ test('logs error if already set up', async () => { }; mockHttpServer.mockImplementation(() => httpServer); - const service = new HttpService(config$.asObservable(), logger); + const service = new HttpService({ configService, env, logger }); await service.setup(); - expect(loggingServiceMock.collect(logger)).toMatchSnapshot(); + expect(loggingServiceMock.collect(logger).warn).toMatchSnapshot(); }); test('stops http server', async () => { - const config = { ssl: {} } as HttpConfig; - - const config$ = new BehaviorSubject(config); + const configService = createConfigService(); const httpServer = { isListening: () => false, @@ -94,9 +102,10 @@ test('stops http server', async () => { }; mockHttpServer.mockImplementation(() => httpServer); - const service = new HttpService(config$.asObservable(), logger); + const service = new HttpService({ configService, env, logger }); await service.setup(); + await service.start(); expect(httpServer.stop).toHaveBeenCalledTimes(0); @@ -106,8 +115,7 @@ test('stops http server', async () => { }); test('register route handler', async () => { - const config = {} as HttpConfig; - const config$ = new BehaviorSubject(config); + const configService = createConfigService({}); const registerRouterMock = jest.fn(); const httpServer = { @@ -118,7 +126,7 @@ test('register route handler', async () => { }; mockHttpServer.mockImplementation(() => httpServer); - const service = new HttpService(config$.asObservable(), logger); + const service = new HttpService({ configService, env, logger }); const router = new Router('/foo'); const { registerRouter } = await service.setup(); @@ -129,6 +137,7 @@ test('register route handler', async () => { }); test('returns http server contract on setup', async () => { + const configService = createConfigService(); const httpServer = { server: {}, options: { someOption: true }, @@ -136,11 +145,57 @@ test('returns http server contract on setup', async () => { mockHttpServer.mockImplementation(() => ({ isListening: () => false, - start: jest.fn().mockReturnValue(httpServer), + setup: jest.fn().mockReturnValue(httpServer), stop: noop, })); - const service = new HttpService(new BehaviorSubject({ ssl: {} } as HttpConfig), logger); + const service = new HttpService({ configService, env, logger }); + + expect(await service.setup()).toBe(httpServer); +}); + +test('does not start http server if process is dev cluster master', async () => { + const configService = createConfigService({}); + const httpServer = { + isListening: () => false, + setup: noop, + start: jest.fn(), + stop: noop, + }; + mockHttpServer.mockImplementation(() => httpServer); + + const service = new HttpService({ + configService, + env: new Env('.', getEnvOptions({ isDevClusterMaster: true })), + logger, + }); + + await service.setup(); + await service.start(); + + expect(httpServer.start).not.toHaveBeenCalled(); +}); + +test('does not start http server if configured with `autoListen:false`', async () => { + const configService = createConfigService({ + autoListen: false, + }); + const httpServer = { + isListening: () => false, + setup: noop, + start: jest.fn(), + stop: noop, + }; + mockHttpServer.mockImplementation(() => httpServer); + + const service = new HttpService({ + configService, + env: new Env('.', getEnvOptions({ isDevClusterMaster: true })), + logger, + }); - expect(await service.start()).toBe(httpServer); + await service.setup(); + await service.start(); + + expect(httpServer.start).not.toHaveBeenCalled(); }); diff --git a/src/core/server/legacy/legacy_service.test.ts b/src/core/server/legacy/legacy_service.test.ts index 549857e731085..fbcf22e15eb70 100644 --- a/src/core/server/legacy/legacy_service.test.ts +++ b/src/core/server/legacy/legacy_service.test.ts @@ -32,6 +32,7 @@ import { Config, Env, ObjectToConfigAdapter } from '../config'; import { getEnvOptions } from '../config/__mocks__/env'; import { configServiceMock } from '../config/config_service.mock'; import { ElasticsearchServiceSetup } from '../elasticsearch'; +import { HttpServiceStart } from '../http'; import { loggingServiceMock } from '../logging/logging_service.mock'; import { DiscoveredPlugin, DiscoveredPluginInternal } from '../plugins'; import { PluginsServiceSetup } from '../plugins/plugins_service'; @@ -50,8 +51,9 @@ let setupDeps: { }; let startDeps: { - http: any; + http: HttpServiceStart; }; + const logger = loggingServiceMock.create(); let configService: ReturnType; @@ -65,6 +67,7 @@ beforeEach(() => { elasticsearch: { legacy: {} } as any, http: { options: { someOption: 'foo', someAnotherOption: 'bar' }, + server: { listener: { addListener: jest.fn() }, route: jest.fn() }, }, plugins: { contracts: new Map([['plugin-id', 'plugin-value']]), @@ -77,7 +80,7 @@ beforeEach(() => { startDeps = { http: { - server: { listener: { addListener: jest.fn() }, route: jest.fn() }, + isListening: () => true, }, }; @@ -103,7 +106,7 @@ describe('once LegacyService is set up with connection info', () => { await legacyService.setup(setupDeps); await legacyService.start(startDeps); - expect(startDeps.http.server.route.mock.calls).toMatchSnapshot('proxy route options'); + expect(setupDeps.http.server.route.mock.calls).toMatchSnapshot('proxy route options'); }); test('proxy route responds with `503` if `kbnServer` is not ready yet.', async () => { @@ -131,7 +134,7 @@ describe('once LegacyService is set up with connection info', () => { }; const mockRequest = { raw: { req: { a: 1 }, res: { b: 2 } } }; - const [[{ handler }]] = startDeps.http.server.route.mock.calls; + const [[{ handler }]] = setupDeps.http.server.route.mock.calls; const response503 = await handler(mockRequest, mockResponseToolkit); expect(response503).toBe(mockResponse); @@ -292,7 +295,7 @@ describe('once LegacyService is set up with connection info', () => { await legacyService.setup(setupDeps); await legacyService.start(startDeps); - const [[{ handler }]] = startDeps.http.server.route.mock.calls; + const [[{ handler }]] = setupDeps.http.server.route.mock.calls; const response = await handler(mockRequest, mockResponseToolkit); expect(response).toBe(mockResponseToolkit.abandon); @@ -311,7 +314,9 @@ describe('once LegacyService is set up with connection info', () => { describe('once LegacyService is set up without connection info', () => { const disabledHttpStartDeps = { - http: undefined, + http: { + isListening: () => false, + }, }; beforeEach(async () => { await legacyService.setup(setupDeps); @@ -319,7 +324,7 @@ describe('once LegacyService is set up without connection info', () => { }); test('creates legacy kbnServer with `autoListen: false`.', () => { - expect(startDeps.http.server.route).not.toHaveBeenCalled(); + expect(setupDeps.http.server.route).not.toHaveBeenCalled(); expect(MockKbnServer).toHaveBeenCalledTimes(1); expect(MockKbnServer).toHaveBeenCalledWith( { server: { autoListen: true } }, @@ -371,7 +376,9 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => { http: setupDeps.http, }); await devClusterLegacyService.start({ - http: undefined, + http: { + isListening: () => false, + }, }); expect(MockClusterManager.create.mock.calls).toMatchSnapshot( @@ -397,7 +404,9 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => { http: setupDeps.http, }); await devClusterLegacyService.start({ - http: undefined, + http: { + isListening: () => false, + }, }); expect(MockClusterManager.create.mock.calls).toMatchSnapshot( diff --git a/src/core/server/plugins/plugin.test.ts b/src/core/server/plugins/plugin.test.ts index 7acc0d2e76a41..dfeb3f0e5ed40 100644 --- a/src/core/server/plugins/plugin.test.ts +++ b/src/core/server/plugins/plugin.test.ts @@ -23,6 +23,7 @@ import { Config, ConfigService, Env, ObjectToConfigAdapter } from '../config'; import { getEnvOptions } from '../config/__mocks__/env'; import { CoreContext } from '../core_context'; import { elasticsearchServiceMock } from '../elasticsearch/elasticsearch_service.mock'; +import { httpServiceMock } from '../http/http_service.mock'; import { loggingServiceMock } from '../logging/logging_service.mock'; import { PluginWrapper, PluginManifest } from './plugin'; @@ -59,7 +60,10 @@ function createPluginManifest(manifestProps: Partial = {}): Plug let configService: ConfigService; let env: Env; let coreContext: CoreContext; -const setupDeps = { elasticsearch: elasticsearchServiceMock.createSetupContract() }; +const setupDeps = { + elasticsearch: elasticsearchServiceMock.createSetupContract(), + http: httpServiceMock.createSetupContract(), +}; beforeEach(() => { env = Env.createDefault(getEnvOptions()); diff --git a/src/core/server/plugins/plugins_service.test.ts b/src/core/server/plugins/plugins_service.test.ts index 305d75a5dcc7a..774639758738c 100644 --- a/src/core/server/plugins/plugins_service.test.ts +++ b/src/core/server/plugins/plugins_service.test.ts @@ -25,6 +25,7 @@ import { BehaviorSubject, from } from 'rxjs'; import { Config, ConfigService, Env, ObjectToConfigAdapter } from '../config'; import { getEnvOptions } from '../config/__mocks__/env'; import { elasticsearchServiceMock } from '../elasticsearch/elasticsearch_service.mock'; +import { httpServiceMock } from '../http/http_service.mock'; import { loggingServiceMock } from '../logging/logging_service.mock'; import { PluginDiscoveryError } from './discovery'; import { PluginWrapper } from './plugin'; @@ -37,7 +38,10 @@ let pluginsService: PluginsService; let configService: ConfigService; let env: Env; let mockPluginSystem: jest.Mocked; -const setupDeps = { elasticsearch: elasticsearchServiceMock.createSetupContract() }; +const setupDeps = { + elasticsearch: elasticsearchServiceMock.createSetupContract(), + http: httpServiceMock.createSetupContract(), +}; const logger = loggingServiceMock.create(); beforeEach(() => { mockPackage.raw = { diff --git a/src/core/server/plugins/plugins_system.test.ts b/src/core/server/plugins/plugins_system.test.ts index 3965e9a8d21b9..6cca961e70e91 100644 --- a/src/core/server/plugins/plugins_system.test.ts +++ b/src/core/server/plugins/plugins_system.test.ts @@ -24,6 +24,7 @@ import { Config, ConfigService, Env, ObjectToConfigAdapter } from '../config'; import { getEnvOptions } from '../config/__mocks__/env'; import { CoreContext } from '../core_context'; import { elasticsearchServiceMock } from '../elasticsearch/elasticsearch_service.mock'; +import { httpServiceMock } from '../http/http_service.mock'; import { loggingServiceMock } from '../logging/logging_service.mock'; import { PluginWrapper, PluginName } from './plugin'; import { PluginsSystem } from './plugins_system'; @@ -57,7 +58,10 @@ let pluginsSystem: PluginsSystem; let configService: ConfigService; let env: Env; let coreContext: CoreContext; -const setupDeps = { elasticsearch: elasticsearchServiceMock.createSetupContract() }; +const setupDeps = { + elasticsearch: elasticsearchServiceMock.createSetupContract(), + http: httpServiceMock.createSetupContract(), +}; beforeEach(() => { env = Env.createDefault(getEnvOptions()); diff --git a/src/core/server/server.test.ts b/src/core/server/server.test.ts index f97a12caa6326..6b673b7a68089 100644 --- a/src/core/server/server.test.ts +++ b/src/core/server/server.test.ts @@ -98,36 +98,6 @@ test('does not fail on "setup" if there are unused paths detected', async () => expect(loggingServiceMock.collect(logger)).toMatchSnapshot('unused paths logs'); }); -test('does not start http service is `autoListen:false`', async () => { - configService.atPath.mockReturnValue(new BehaviorSubject({ autoListen: false })); - - const server = new Server(configService as any, logger, env); - expect(mockLegacyService.start).not.toHaveBeenCalled(); - expect(httpService.start).not.toHaveBeenCalled(); - - await server.setup(); - await server.start(); - - expect(httpService.start).not.toHaveBeenCalled(); - expect(mockLegacyService.start).toHaveBeenCalledTimes(1); -}); - -test('does not start http service if process is dev cluster master', async () => { - const server = new Server( - configService as any, - logger, - new Env('.', getEnvOptions({ isDevClusterMaster: true })) - ); - - expect(mockLegacyService.start).not.toHaveBeenCalled(); - - await server.setup(); - await server.start(); - - expect(httpService.start).not.toHaveBeenCalled(); - expect(mockLegacyService.start).toHaveBeenCalledTimes(1); -}); - test('stops services on "stop"', async () => { const server = new Server(configService as any, logger, env); diff --git a/x-pack/plugins/apm/index.ts b/x-pack/plugins/apm/index.ts index 2d1857fac161b..a2b0b94e2fd70 100644 --- a/x-pack/plugins/apm/index.ts +++ b/x-pack/plugins/apm/index.ts @@ -7,17 +7,10 @@ import { i18n } from '@kbn/i18n'; import { Server } from 'hapi'; import { resolve } from 'path'; -import { - PluginInitializerContext, - HttpServiceStart -} from 'src/core/server/index.js'; +import { PluginInitializerContext, CoreSetup } from 'src/core/server/index.js'; import mappings from './mappings.json'; import { plugin } from './server/new-platform/index'; -export interface SetupDeps { - http: HttpServiceStart; -} - // TODO: get proper types export function apm(kibana: any) { return new kibana.Plugin({ @@ -113,7 +106,7 @@ export function apm(kibana: any) { http: { server } - } as SetupDeps; + } as CoreSetup; plugin(initializerContext).setup(core); } }); diff --git a/x-pack/plugins/apm/server/lib/apm_telemetry/make_apm_usage_collector.ts b/x-pack/plugins/apm/server/lib/apm_telemetry/make_apm_usage_collector.ts index 44c93b65079e3..6ea2bdb1eb944 100644 --- a/x-pack/plugins/apm/server/lib/apm_telemetry/make_apm_usage_collector.ts +++ b/x-pack/plugins/apm/server/lib/apm_telemetry/make_apm_usage_collector.ts @@ -4,12 +4,12 @@ * you may not use this file except in compliance with the Elastic License. */ -import { SetupDeps } from '../../..'; +import { CoreSetup } from 'src/core/server'; import { getSavedObjectsClient } from '../helpers/saved_objects_client'; import { APM_TELEMETRY_DOC_ID, createApmTelementry } from './apm_telemetry'; -export interface CoreSetupWithUsageCollector extends SetupDeps { - http: SetupDeps['http'] & { +export interface CoreSetupWithUsageCollector extends CoreSetup { + http: CoreSetup['http'] & { server: { usage: { collectorSet: { diff --git a/x-pack/plugins/apm/server/lib/index_pattern/index.ts b/x-pack/plugins/apm/server/lib/index_pattern/index.ts index 4921c31f11c79..69aad1bb3ca4f 100644 --- a/x-pack/plugins/apm/server/lib/index_pattern/index.ts +++ b/x-pack/plugins/apm/server/lib/index_pattern/index.ts @@ -3,11 +3,11 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { SetupDeps } from '../../..'; +import { CoreSetup } from 'src/core/server'; import { getSavedObjectsClient } from '../helpers/saved_objects_client'; import indexPattern from '../../../../../../src/legacy/core_plugins/kibana/server/tutorials/apm/index_pattern.json'; -export async function ensureIndexPatternExists(core: SetupDeps) { +export async function ensureIndexPatternExists(core: CoreSetup) { const { server } = core.http; const config = server.config(); const apmIndexPatternTitle = config.get('apm_oss.indexPattern'); diff --git a/x-pack/plugins/apm/server/new-platform/plugin.ts b/x-pack/plugins/apm/server/new-platform/plugin.ts index 646f00f44c201..908988d848758 100644 --- a/x-pack/plugins/apm/server/new-platform/plugin.ts +++ b/x-pack/plugins/apm/server/new-platform/plugin.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { SetupDeps } from '../..'; +import { CoreSetup } from 'src/core/server'; import { makeApmUsageCollector } from '../lib/apm_telemetry'; import { ensureIndexPatternExists } from '../lib/index_pattern'; import { CoreSetupWithUsageCollector } from '../lib/apm_telemetry/make_apm_usage_collector'; @@ -15,7 +15,7 @@ import { initTracesApi } from '../routes/traces'; import { initTransactionGroupsApi } from '../routes/transaction_groups'; export class Plugin { - public setup(core: SetupDeps) { + public setup(core: CoreSetup) { initTransactionGroupsApi(core); initTracesApi(core); initServicesApi(core); diff --git a/x-pack/plugins/apm/server/routes/__test__/routeFailures.test.ts b/x-pack/plugins/apm/server/routes/__test__/routeFailures.test.ts index 0c17378f1345b..4db63a4ce5886 100644 --- a/x-pack/plugins/apm/server/routes/__test__/routeFailures.test.ts +++ b/x-pack/plugins/apm/server/routes/__test__/routeFailures.test.ts @@ -5,7 +5,7 @@ */ import { flatten } from 'lodash'; -import { SetupDeps } from '../../..'; +import { CoreSetup } from 'src/core/server'; import { initErrorsApi } from '../errors'; import { initServicesApi } from '../services'; import { initTracesApi } from '../traces'; @@ -13,13 +13,13 @@ import { initTracesApi } from '../traces'; describe('route handlers should fail with a Boom error', () => { let consoleErrorSpy: any; - async function testRouteFailures(init: (core: SetupDeps) => void) { + async function testRouteFailures(init: (core: CoreSetup) => void) { const mockServer = { route: jest.fn() }; const mockCore = ({ http: { server: mockServer } - } as unknown) as SetupDeps; + } as unknown) as CoreSetup; init(mockCore); expect(mockServer.route).toHaveBeenCalled(); diff --git a/x-pack/plugins/apm/server/routes/errors.ts b/x-pack/plugins/apm/server/routes/errors.ts index c327ad749698a..3bd7be240eb4c 100644 --- a/x-pack/plugins/apm/server/routes/errors.ts +++ b/x-pack/plugins/apm/server/routes/errors.ts @@ -7,7 +7,7 @@ import Boom from 'boom'; import Joi from 'joi'; import { Legacy } from 'kibana'; -import { SetupDeps } from '../..'; +import { CoreSetup } from 'src/core/server'; import { getDistribution } from '../lib/errors/distribution/get_distribution'; import { getErrorGroup } from '../lib/errors/get_error_group'; import { getErrorGroups } from '../lib/errors/get_error_groups'; @@ -21,7 +21,7 @@ const defaultErrorHandler = (err: Error) => { throw Boom.boomify(err, { statusCode: 400 }); }; -export function initErrorsApi(core: SetupDeps) { +export function initErrorsApi(core: CoreSetup) { const { server } = core.http; server.route({ method: 'GET', diff --git a/x-pack/plugins/apm/server/routes/metrics.ts b/x-pack/plugins/apm/server/routes/metrics.ts index cd821df76cedd..997ff9b803c09 100644 --- a/x-pack/plugins/apm/server/routes/metrics.ts +++ b/x-pack/plugins/apm/server/routes/metrics.ts @@ -5,7 +5,7 @@ */ import Boom from 'boom'; -import { SetupDeps } from '../..'; +import { CoreSetup } from 'src/core/server'; import { withDefaultValidators } from '../lib/helpers/input_validation'; import { setupRequest } from '../lib/helpers/setup_request'; import { getAllMetricsChartData } from '../lib/metrics/get_all_metrics_chart_data'; @@ -16,8 +16,7 @@ const defaultErrorHandler = (err: Error) => { throw Boom.boomify(err, { statusCode: 400 }); }; -export function initMetricsApi(core: SetupDeps) { - if (!core.http) return; +export function initMetricsApi(core: CoreSetup) { const { server } = core.http; server.route({ method: 'GET', diff --git a/x-pack/plugins/apm/server/routes/services.ts b/x-pack/plugins/apm/server/routes/services.ts index 3a32817b1152e..0bbc1b8be894b 100644 --- a/x-pack/plugins/apm/server/routes/services.ts +++ b/x-pack/plugins/apm/server/routes/services.ts @@ -5,7 +5,7 @@ */ import Boom from 'boom'; -import { SetupDeps } from '../..'; +import { CoreSetup } from 'src/core/server'; import { AgentName } from '../../typings/es_schemas/ui/fields/Agent'; import { createApmTelementry, storeApmTelemetry } from '../lib/apm_telemetry'; import { withDefaultValidators } from '../lib/helpers/input_validation'; @@ -20,7 +20,7 @@ const defaultErrorHandler = (err: Error) => { throw Boom.boomify(err, { statusCode: 400 }); }; -export function initServicesApi(core: SetupDeps) { +export function initServicesApi(core: CoreSetup) { const { server } = core.http; server.route({ method: 'GET', diff --git a/x-pack/plugins/apm/server/routes/traces.ts b/x-pack/plugins/apm/server/routes/traces.ts index 8f61486287ed0..5bc996b1b71cf 100644 --- a/x-pack/plugins/apm/server/routes/traces.ts +++ b/x-pack/plugins/apm/server/routes/traces.ts @@ -6,7 +6,7 @@ import Boom from 'boom'; -import { SetupDeps } from '../..'; +import { CoreSetup } from 'src/core/server'; import { withDefaultValidators } from '../lib/helpers/input_validation'; import { setupRequest } from '../lib/helpers/setup_request'; import { getTopTraces } from '../lib/traces/get_top_traces'; @@ -19,7 +19,7 @@ const defaultErrorHandler = (err: Error) => { throw Boom.boomify(err, { statusCode: 400 }); }; -export function initTracesApi(core: SetupDeps) { +export function initTracesApi(core: CoreSetup) { const { server } = core.http; // Get trace list diff --git a/x-pack/plugins/apm/server/routes/transaction_groups.ts b/x-pack/plugins/apm/server/routes/transaction_groups.ts index 2ee942b008bb3..5c93edf5bd867 100644 --- a/x-pack/plugins/apm/server/routes/transaction_groups.ts +++ b/x-pack/plugins/apm/server/routes/transaction_groups.ts @@ -6,7 +6,7 @@ import Boom from 'boom'; import Joi from 'joi'; -import { SetupDeps } from '../..'; +import { CoreSetup } from 'src/core/server'; import { withDefaultValidators } from '../lib/helpers/input_validation'; import { setupRequest } from '../lib/helpers/setup_request'; import { getChartsData } from '../lib/transactions/charts'; @@ -19,7 +19,7 @@ const defaultErrorHandler = (err: Error) => { throw Boom.boomify(err, { statusCode: 400 }); }; -export function initTransactionGroupsApi(core: SetupDeps) { +export function initTransactionGroupsApi(core: CoreSetup) { const { server } = core.http; server.route({ From 05e03335fa03b52c09d1857a489ec251ac38965f Mon Sep 17 00:00:00 2001 From: restrry Date: Thu, 25 Apr 2019 11:10:22 +0300 Subject: [PATCH 14/20] udpate docs --- .../server/kibana-plugin-server.corestart.http.md | 2 +- ...na-plugin-server.httpservicestart.islistening.md | 13 +++++++++++++ .../server/kibana-plugin-server.httpservicestart.md | 12 ++++++++++-- .../development/core/server/kibana-plugin-server.md | 2 +- src/core/server/http/http_service.ts | 2 ++ src/core/server/server.api.md | 6 ++++-- 6 files changed, 31 insertions(+), 6 deletions(-) create mode 100644 docs/development/core/server/kibana-plugin-server.httpservicestart.islistening.md diff --git a/docs/development/core/server/kibana-plugin-server.corestart.http.md b/docs/development/core/server/kibana-plugin-server.corestart.http.md index 51ab1e5dbe303..a12fcbaab27cc 100644 --- a/docs/development/core/server/kibana-plugin-server.corestart.http.md +++ b/docs/development/core/server/kibana-plugin-server.corestart.http.md @@ -7,5 +7,5 @@ Signature: ```typescript -http?: HttpServiceStart; +http: HttpServiceStart; ``` diff --git a/docs/development/core/server/kibana-plugin-server.httpservicestart.islistening.md b/docs/development/core/server/kibana-plugin-server.httpservicestart.islistening.md new file mode 100644 index 0000000000000..5d27db32fc946 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.httpservicestart.islistening.md @@ -0,0 +1,13 @@ + + +[Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [HttpServiceStart](./kibana-plugin-server.httpservicestart.md) > [isListening](./kibana-plugin-server.httpservicestart.islistening.md) + +## HttpServiceStart.isListening property + +Indicates if http server is listening on a port + +Signature: + +```typescript +isListening: () => boolean; +``` diff --git a/docs/development/core/server/kibana-plugin-server.httpservicestart.md b/docs/development/core/server/kibana-plugin-server.httpservicestart.md index c3b1b71d7143a..33e43061bdedf 100644 --- a/docs/development/core/server/kibana-plugin-server.httpservicestart.md +++ b/docs/development/core/server/kibana-plugin-server.httpservicestart.md @@ -2,10 +2,18 @@ [Home](./index) > [kibana-plugin-server](./kibana-plugin-server.md) > [HttpServiceStart](./kibana-plugin-server.httpservicestart.md) -## HttpServiceStart type +## HttpServiceStart interface + Signature: ```typescript -export declare type HttpServiceStart = HttpServerInfo; +export interface HttpServiceStart ``` + +## Properties + +| Property | Type | Description | +| --- | --- | --- | +| [isListening](./kibana-plugin-server.httpservicestart.islistening.md) | () => boolean | Indicates if http server is listening on a port | + diff --git a/docs/development/core/server/kibana-plugin-server.md b/docs/development/core/server/kibana-plugin-server.md index 6f9da5680616c..3d0f4613ed061 100644 --- a/docs/development/core/server/kibana-plugin-server.md +++ b/docs/development/core/server/kibana-plugin-server.md @@ -23,6 +23,7 @@ | [CoreSetup](./kibana-plugin-server.coresetup.md) | | | [CoreStart](./kibana-plugin-server.corestart.md) | | | [ElasticsearchServiceSetup](./kibana-plugin-server.elasticsearchservicesetup.md) | | +| [HttpServiceStart](./kibana-plugin-server.httpservicestart.md) | | | [Logger](./kibana-plugin-server.logger.md) | Logger exposes all the necessary methods to log any type of information and this is the interface used by the logging consumers including plugins. | | [LoggerFactory](./kibana-plugin-server.loggerfactory.md) | The single purpose of LoggerFactory interface is to define a way to retrieve a context-based logger instance. | | [LogMeta](./kibana-plugin-server.logmeta.md) | Contextual metadata | @@ -40,7 +41,6 @@ | [ElasticsearchClientConfig](./kibana-plugin-server.elasticsearchclientconfig.md) | | | [Headers](./kibana-plugin-server.headers.md) | | | [HttpServiceSetup](./kibana-plugin-server.httpservicesetup.md) | | -| [HttpServiceStart](./kibana-plugin-server.httpservicestart.md) | | | [OnRequestHandler](./kibana-plugin-server.onrequesthandler.md) | | | [PluginInitializer](./kibana-plugin-server.plugininitializer.md) | The plugin export at the root of a plugin's server directory should conform to this interface. | | [PluginName](./kibana-plugin-server.pluginname.md) | Dedicated type for plugin name/id that is supposed to make Map/Set/Arrays that use it as a key or value more obvious. | diff --git a/src/core/server/http/http_service.ts b/src/core/server/http/http_service.ts index 005488b81689c..ea172b881f546 100644 --- a/src/core/server/http/http_service.ts +++ b/src/core/server/http/http_service.ts @@ -29,7 +29,9 @@ import { HttpsRedirectServer } from './https_redirect_server'; /** @public */ export type HttpServiceSetup = HttpServerSetup; +/** @public */ export interface HttpServiceStart { + /** Indicates if http server is listening on a port */ isListening: () => boolean; } diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 00e32ba77e3c6..4c1ae67bba74e 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -77,7 +77,7 @@ export interface CoreSetup { // @public (undocumented) export interface CoreStart { // (undocumented) - http?: HttpServiceStart; + http: HttpServiceStart; } // @internal @@ -117,7 +117,9 @@ export type Headers = Record; export type HttpServiceSetup = HttpServerSetup; // @public (undocumented) -export type HttpServiceStart = HttpServerInfo; +export interface HttpServiceStart { + isListening: () => boolean; +} // @public (undocumented) export class KibanaRequest { From d46e7a8e005a0973c4b48a0a7023996e92438aef Mon Sep 17 00:00:00 2001 From: restrry Date: Thu, 25 Apr 2019 11:14:18 +0300 Subject: [PATCH 15/20] cleanup apm changees --- x-pack/plugins/apm/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/apm/index.ts b/x-pack/plugins/apm/index.ts index a2b0b94e2fd70..a97c63a51bf1e 100644 --- a/x-pack/plugins/apm/index.ts +++ b/x-pack/plugins/apm/index.ts @@ -7,7 +7,7 @@ import { i18n } from '@kbn/i18n'; import { Server } from 'hapi'; import { resolve } from 'path'; -import { PluginInitializerContext, CoreSetup } from 'src/core/server/index.js'; +import { CoreSetup, PluginInitializerContext } from 'src/core/server/index.js'; import mappings from './mappings.json'; import { plugin } from './server/new-platform/index'; From 255d3c3abc462f32d16c8a3cb584135395ad214a Mon Sep 17 00:00:00 2001 From: restrry Date: Thu, 25 Apr 2019 12:37:10 +0300 Subject: [PATCH 16/20] check legacy service setup before start --- .../legacy/__snapshots__/legacy_service.test.ts.snap | 2 +- src/core/server/legacy/legacy_service.test.ts | 6 ++++++ src/core/server/legacy/legacy_service.ts | 11 ++++++----- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/core/server/legacy/__snapshots__/legacy_service.test.ts.snap b/src/core/server/legacy/__snapshots__/legacy_service.test.ts.snap index 76d53b4b1cc5d..82192eda44a80 100644 --- a/src/core/server/legacy/__snapshots__/legacy_service.test.ts.snap +++ b/src/core/server/legacy/__snapshots__/legacy_service.test.ts.snap @@ -36,7 +36,7 @@ Array [ "debug": [MockFunction] { "calls": Array [ Array [ - "setting up legacy service", + "starting legacy service", ], ], "results": Array [ diff --git a/src/core/server/legacy/legacy_service.test.ts b/src/core/server/legacy/legacy_service.test.ts index fbcf22e15eb70..da5499080eee1 100644 --- a/src/core/server/legacy/legacy_service.test.ts +++ b/src/core/server/legacy/legacy_service.test.ts @@ -414,3 +414,9 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => { ); }); }); + +test('Cannot start without setup phase', async () => { + await expect(legacyService.start(startDeps)).rejects.toThrowErrorMatchingInlineSnapshot( + `"Legacy service is not setup yet."` + ); +}); diff --git a/src/core/server/legacy/legacy_service.ts b/src/core/server/legacy/legacy_service.ts index c0b02a0f11def..63dd9823f2522 100644 --- a/src/core/server/legacy/legacy_service.ts +++ b/src/core/server/legacy/legacy_service.ts @@ -62,7 +62,11 @@ export class LegacyService implements CoreService { this.setupDeps = setupDeps; } public async start(startDeps: CoreStart) { - this.log.debug('setting up legacy service'); + const { setupDeps } = this; + if (!setupDeps) { + throw new Error('Legacy service is not setup yet.'); + } + this.log.debug('starting legacy service'); const update$ = this.coreContext.configService.getConfig$().pipe( tap(config => { @@ -85,10 +89,7 @@ export class LegacyService implements CoreService { await this.createClusterManager(config); return; } - if (!this.setupDeps) { - throw new Error('Core setup contract is not defined'); - } - return await this.createKbnServer(config, this.setupDeps, startDeps); + return await this.createKbnServer(config, setupDeps, startDeps); }) ) .toPromise(); From 8880af142881ac21a1859ba0bfd9ccacff5d8656 Mon Sep 17 00:00:00 2001 From: restrry Date: Thu, 25 Apr 2019 13:01:38 +0300 Subject: [PATCH 17/20] check http server setup before start --- src/core/server/http/http_server.test.ts | 6 ++++++ src/core/server/http/http_server.ts | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/core/server/http/http_server.test.ts b/src/core/server/http/http_server.test.ts index 5e7abc0224d4d..75547074bedb6 100644 --- a/src/core/server/http/http_server.test.ts +++ b/src/core/server/http/http_server.test.ts @@ -607,3 +607,9 @@ test('registers onRequest interceptor several times', async () => { doRegister(); expect(doRegister).not.toThrowError(); }); + +test('throws an error if starts without set up', async () => { + await expect(server.start(config)).rejects.toThrowErrorMatchingInlineSnapshot( + `"Http server is not setup up yet"` + ); +}); diff --git a/src/core/server/http/http_server.ts b/src/core/server/http/http_server.ts index d1c1b8fd688eb..6e7d5f8db4842 100644 --- a/src/core/server/http/http_server.ts +++ b/src/core/server/http/http_server.ts @@ -92,7 +92,7 @@ export class HttpServer { public async start(config: HttpConfig) { if (this.server === undefined) { - throw new Error('server is not setup up yet'); + throw new Error('Http server is not setup up yet'); } this.log.debug('starting http server'); From 02ccc544d3873df90491e5be68d6ee62cbd28471 Mon Sep 17 00:00:00 2001 From: restrry Date: Tue, 30 Apr 2019 09:07:00 +0200 Subject: [PATCH 18/20] restrict server options mutation; unify Promise interface for setup --- src/core/server/http/http_server.ts | 6 ++-- src/core/server/http/lib/deep_freeze.ts | 42 +++++++++++++++++++++++++ src/core/server/server.ts | 2 +- 3 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 src/core/server/http/lib/deep_freeze.ts diff --git a/src/core/server/http/http_server.ts b/src/core/server/http/http_server.ts index 6e7d5f8db4842..a3c1d9f7346cb 100644 --- a/src/core/server/http/http_server.ts +++ b/src/core/server/http/http_server.ts @@ -26,6 +26,8 @@ import { createServer, getServerOptions } from './http_tools'; import { adoptToHapiAuthFormat, AuthenticationHandler } from './lifecycle/auth'; import { adoptToHapiOnRequestFormat, OnRequestHandler } from './lifecycle/on_request'; import { Router } from './router'; +import { deepFreeze, RecursiveReadonly } from './lib/deep_freeze'; + import { SessionStorageCookieOptions, createCookieSessionStorageFactory, @@ -33,7 +35,7 @@ import { export interface HttpServerSetup { server: Server; - options: ServerOptions; + options: RecursiveReadonly; registerRouter: (router: Router) => void; /** * Define custom authentication and/or authorization mechanism for incoming requests. @@ -76,7 +78,7 @@ export class HttpServer { this.server = createServer(serverOptions); return { - options: serverOptions, + options: deepFreeze(serverOptions), registerRouter: this.registerRouter.bind(this), registerOnRequest: this.registerOnRequest.bind(this), registerAuth: ( diff --git a/src/core/server/http/lib/deep_freeze.ts b/src/core/server/http/lib/deep_freeze.ts new file mode 100644 index 0000000000000..86996fefce62c --- /dev/null +++ b/src/core/server/http/lib/deep_freeze.ts @@ -0,0 +1,42 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +type Freezable = { [k: string]: any } | any[]; + +// if we define this inside RecursiveReadonly TypeScript complains +// eslint-disable-next-line @typescript-eslint/no-empty-interface +interface RecursiveReadonlyArray extends Array> {} + +export type RecursiveReadonly = T extends any[] + ? RecursiveReadonlyArray + : T extends object + ? Readonly<{ [K in keyof T]: RecursiveReadonly }> + : T; + +export function deepFreeze(object: T) { + // for any properties that reference an object, makes sure that object is + // recursively frozen as well + for (const value of Object.values(object)) { + if (value !== null && typeof value === 'object') { + deepFreeze(value); + } + } + + return Object.freeze(object) as RecursiveReadonly; +} diff --git a/src/core/server/server.ts b/src/core/server/server.ts index 7cef2f3beba93..04255d61b4986 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -58,7 +58,7 @@ export class Server { plugins: pluginsSetup, }; - this.legacy.setup(coreSetup); + await this.legacy.setup(coreSetup); return coreSetup; } From c04fdd2e26f483b3e612b854cc1c15a000a9bd13 Mon Sep 17 00:00:00 2001 From: restrry Date: Tue, 30 Apr 2019 11:52:20 +0200 Subject: [PATCH 19/20] introduce start pahse for plugins service for parity with client side --- src/core/server/index.ts | 12 +- src/core/server/legacy/legacy_service.test.ts | 15 ++- src/core/server/plugins/index.ts | 4 +- src/core/server/plugins/plugin.test.ts | 38 ++++++ src/core/server/plugins/plugin.ts | 47 +++++-- src/core/server/plugins/plugin_context.ts | 29 +++- src/core/server/plugins/plugins_service.ts | 16 ++- .../plugins/plugins_system.test.mocks.ts | 2 + .../server/plugins/plugins_system.test.ts | 125 ++++++++++++++++-- src/core/server/plugins/plugins_system.ts | 45 ++++++- src/core/server/server.ts | 2 + src/legacy/server/kbn_server.d.ts | 2 + src/legacy/server/kbn_server.js | 1 + src/plugins/testbed/server/index.ts | 22 ++- 14 files changed, 325 insertions(+), 35 deletions(-) diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 1aa602e32c384..448433b5420a9 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -18,7 +18,7 @@ */ import { ElasticsearchServiceSetup } from './elasticsearch'; import { HttpServiceSetup, HttpServiceStart } from './http'; -import { PluginsServiceSetup } from './plugins'; +import { PluginsServiceSetup, PluginsServiceStart } from './plugins'; export { bootstrap } from './bootstrap'; export { ConfigService } from './config'; @@ -47,6 +47,7 @@ export { PluginInitializerContext, PluginName, PluginSetupContext, + PluginStartContext, } from './plugins'; /** @public */ @@ -58,6 +59,13 @@ export interface CoreSetup { export interface CoreStart { http: HttpServiceStart; + plugins: PluginsServiceStart; } -export { HttpServiceSetup, HttpServiceStart, ElasticsearchServiceSetup, PluginsServiceSetup }; +export { + HttpServiceSetup, + HttpServiceStart, + ElasticsearchServiceSetup, + PluginsServiceSetup, + PluginsServiceStart, +}; diff --git a/src/core/server/legacy/legacy_service.test.ts b/src/core/server/legacy/legacy_service.test.ts index da5499080eee1..dd2883e6e48fb 100644 --- a/src/core/server/legacy/legacy_service.test.ts +++ b/src/core/server/legacy/legacy_service.test.ts @@ -35,7 +35,7 @@ import { ElasticsearchServiceSetup } from '../elasticsearch'; import { HttpServiceStart } from '../http'; import { loggingServiceMock } from '../logging/logging_service.mock'; import { DiscoveredPlugin, DiscoveredPluginInternal } from '../plugins'; -import { PluginsServiceSetup } from '../plugins/plugins_service'; +import { PluginsServiceSetup, PluginsServiceStart } from '../plugins/plugins_service'; import { LegacyPlatformProxy } from './legacy_platform_proxy'; const MockKbnServer: jest.Mock = KbnServer as any; @@ -52,6 +52,7 @@ let setupDeps: { let startDeps: { http: HttpServiceStart; + plugins: PluginsServiceStart; }; const logger = loggingServiceMock.create(); @@ -82,6 +83,9 @@ beforeEach(() => { http: { isListening: () => true, }, + plugins: { + contracts: new Map(), + }, }; config$ = new BehaviorSubject( @@ -317,6 +321,9 @@ describe('once LegacyService is set up without connection info', () => { http: { isListening: () => false, }, + plugins: { + contracts: new Map(), + }, }; beforeEach(async () => { await legacyService.setup(setupDeps); @@ -379,6 +386,9 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => { http: { isListening: () => false, }, + plugins: { + contracts: new Map(), + }, }); expect(MockClusterManager.create.mock.calls).toMatchSnapshot( @@ -407,6 +417,9 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => { http: { isListening: () => false, }, + plugins: { + contracts: new Map(), + }, }); expect(MockClusterManager.create.mock.calls).toMatchSnapshot( diff --git a/src/core/server/plugins/index.ts b/src/core/server/plugins/index.ts index 8469e21783f0f..6faef56a43ce7 100644 --- a/src/core/server/plugins/index.ts +++ b/src/core/server/plugins/index.ts @@ -17,7 +17,7 @@ * under the License. */ -export { PluginsService, PluginsServiceSetup } from './plugins_service'; +export { PluginsService, PluginsServiceSetup, PluginsServiceStart } from './plugins_service'; /** @internal */ export { isNewPlatformPlugin } from './discovery'; @@ -29,4 +29,4 @@ export { PluginInitializer, PluginName, } from './plugin'; -export { PluginInitializerContext, PluginSetupContext } from './plugin_context'; +export { PluginInitializerContext, PluginSetupContext, PluginStartContext } from './plugin_context'; diff --git a/src/core/server/plugins/plugin.test.ts b/src/core/server/plugins/plugin.test.ts index dfeb3f0e5ed40..cb4203244e86d 100644 --- a/src/core/server/plugins/plugin.test.ts +++ b/src/core/server/plugins/plugin.test.ts @@ -179,6 +179,44 @@ test('`setup` initializes plugin and calls appropriate lifecycle hook', async () expect(mockPluginInstance.setup).toHaveBeenCalledWith(setupContext, setupDependencies); }); +test('`start` fails if setup is not called first', async () => { + const manifest = createPluginManifest(); + const plugin = new PluginWrapper( + 'some-plugin-path', + manifest, + createPluginInitializerContext(coreContext, manifest) + ); + + await expect(plugin.start({} as any, {} as any)).rejects.toThrowErrorMatchingInlineSnapshot( + `"Plugin \\"some-plugin-id\\" can't be started since it isn't set up."` + ); +}); + +test('`start` calls plugin.start with context and dependencies', async () => { + const manifest = createPluginManifest(); + const plugin = new PluginWrapper( + 'plugin-with-initializer-path', + manifest, + createPluginInitializerContext(coreContext, manifest) + ); + const context = { any: 'thing' } as any; + const deps = { otherDep: 'value' }; + + const pluginStartContract = { contract: 'start-contract' }; + const mockPluginInstance = { + setup: jest.fn(), + start: jest.fn().mockResolvedValue(pluginStartContract), + }; + mockPluginInitializer.mockReturnValue(mockPluginInstance); + + await plugin.setup({} as any, {} as any); + + const startContract = await plugin.start(context, deps); + + expect(startContract).toBe(pluginStartContract); + expect(mockPluginInstance.start).toHaveBeenCalledWith(context, deps); +}); + test('`stop` fails if plugin is not set up', async () => { const manifest = createPluginManifest(); const plugin = new PluginWrapper( diff --git a/src/core/server/plugins/plugin.ts b/src/core/server/plugins/plugin.ts index 5e23fb3b8b30e..72981005968f3 100644 --- a/src/core/server/plugins/plugin.ts +++ b/src/core/server/plugins/plugin.ts @@ -21,7 +21,7 @@ import { join } from 'path'; import typeDetect from 'type-detect'; import { ConfigPath } from '../config'; import { Logger } from '../logging'; -import { PluginInitializerContext, PluginSetupContext } from './plugin_context'; +import { PluginInitializerContext, PluginSetupContext, PluginStartContext } from './plugin_context'; /** * Dedicated type for plugin name/id that is supposed to make Map/Set/Arrays @@ -129,11 +129,14 @@ export interface DiscoveredPluginInternal extends DiscoveredPlugin { * * @public */ -export interface Plugin = {}> { - setup: ( - pluginSetupContext: PluginSetupContext, - plugins: TPluginsSetup - ) => TSetup | Promise; +export interface Plugin< + TSetup, + TStart, + TPluginsSetup extends Record = {}, + TPluginsStart extends Record = {} +> { + setup: (core: PluginSetupContext, plugins: TPluginsSetup) => TSetup | Promise; + start: (core: PluginStartContext, plugins: TPluginsStart) => TStart | Promise; stop?: () => void; } @@ -143,9 +146,12 @@ export interface Plugin = {}> = ( - coreContext: PluginInitializerContext -) => Plugin; +export type PluginInitializer< + TSetup, + TStart, + TPluginsSetup extends Record = {}, + TPluginsStart extends Record = {} +> = (core: PluginInitializerContext) => Plugin; /** * Lightweight wrapper around discovered plugin that is responsible for instantiating @@ -155,7 +161,9 @@ export type PluginInitializer = Record + TStart = unknown, + TPluginsSetup extends Record = Record, + TPluginsStart extends Record = Record > { public readonly name: PluginManifest['id']; public readonly configPath: PluginManifest['configPath']; @@ -166,7 +174,7 @@ export class PluginWrapper< private readonly log: Logger; - private instance?: Plugin; + private instance?: Plugin; constructor( public readonly path: string, @@ -197,6 +205,21 @@ export class PluginWrapper< return await this.instance.setup(setupContext, plugins); } + /** + * Calls `start` function exposed by the initialized plugin. + * @param startContext Context that consists of various core services tailored specifically + * for the `start` lifecycle event. + * @param plugins The dictionary where the key is the dependency name and the value + * is the contract returned by the dependency's `start` function. + */ + public async start(startContext: PluginStartContext, plugins: TPluginsStart) { + if (this.instance === undefined) { + throw new Error(`Plugin "${this.name}" can't be started since it isn't set up.`); + } + + return await this.instance.start(startContext, plugins); + } + /** * Calls optional `stop` function exposed by the plugin initializer. */ @@ -224,7 +247,7 @@ export class PluginWrapper< } const { plugin: initializer } = pluginDefinition as { - plugin: PluginInitializer; + plugin: PluginInitializer; }; if (!initializer || typeof initializer !== 'function') { throw new Error(`Definition of plugin "${this.name}" should be a function (${this.path}).`); diff --git a/src/core/server/plugins/plugin_context.ts b/src/core/server/plugins/plugin_context.ts index 1de59ddaef8a3..b738e192896dd 100644 --- a/src/core/server/plugins/plugin_context.ts +++ b/src/core/server/plugins/plugin_context.ts @@ -25,7 +25,7 @@ import { ClusterClient } from '../elasticsearch'; import { HttpServiceSetup } from '../http'; import { LoggerFactory } from '../logging'; import { PluginWrapper, PluginManifest } from './plugin'; -import { PluginsServiceSetupDeps } from './plugins_service'; +import { PluginsServiceSetupDeps, PluginsServiceStartDeps } from './plugins_service'; /** * Context that's available to plugins during initialization stage. @@ -61,6 +61,13 @@ export interface PluginSetupContext { }; } +/** + * Context passed to the plugins `start` method. + * + * @public + */ +export interface PluginStartContext {} // eslint-disable-line @typescript-eslint/no-empty-interface + /** * This returns a facade for `CoreContext` that will be exposed to the plugin initializer. * This facade should be safe to use across entire plugin lifespan. @@ -144,3 +151,23 @@ export function createPluginSetupContext( }, }; } + +/** + * This returns a facade for `CoreContext` that will be exposed to the plugin `start` method. + * This facade should be safe to use only within `start` itself. + * + * This is called for each plugin when it starts, so each plugin gets its own + * version of these values. + * + * @param coreContext Kibana core context + * @param plugin The plugin we're building these values for. + * @param deps Dependencies that Plugins services gets during start. + * @internal + */ +export function createPluginStartContext( + coreContext: CoreContext, + deps: PluginsServiceStartDeps, + plugin: PluginWrapper +): PluginStartContext { + return {}; +} diff --git a/src/core/server/plugins/plugins_service.ts b/src/core/server/plugins/plugins_service.ts index 0caba3477bf24..f243754db8415 100644 --- a/src/core/server/plugins/plugins_service.ts +++ b/src/core/server/plugins/plugins_service.ts @@ -38,6 +38,11 @@ export interface PluginsServiceSetup { }; } +/** @internal */ +export interface PluginsServiceStart { + contracts: Map; +} + /** @internal */ export interface PluginsServiceSetupDeps { elasticsearch: ElasticsearchServiceSetup; @@ -45,7 +50,10 @@ export interface PluginsServiceSetupDeps { } /** @internal */ -export class PluginsService implements CoreService { +export interface PluginsServiceStartDeps {} // eslint-disable-line @typescript-eslint/no-empty-interface + +/** @internal */ +export class PluginsService implements CoreService { private readonly log: Logger; private readonly pluginsSystem: PluginsSystem; @@ -80,7 +88,11 @@ export class PluginsService implements CoreService { }; } - public async start() {} + public async start(deps: PluginsServiceStartDeps) { + this.log.debug('Plugins service starts plugins'); + const contracts = await this.pluginsSystem.startPlugins(deps); + return { contracts }; + } public async stop() { this.log.debug('Stopping plugins service'); diff --git a/src/core/server/plugins/plugins_system.test.mocks.ts b/src/core/server/plugins/plugins_system.test.mocks.ts index 9bcbba892fa2a..647f0ed8002af 100644 --- a/src/core/server/plugins/plugins_system.test.mocks.ts +++ b/src/core/server/plugins/plugins_system.test.mocks.ts @@ -18,6 +18,8 @@ */ export const mockCreatePluginSetupContext = jest.fn(); +export const mockCreatePluginStartContext = jest.fn(); jest.mock('./plugin_context', () => ({ createPluginSetupContext: mockCreatePluginSetupContext, + createPluginStartContext: mockCreatePluginStartContext, })); diff --git a/src/core/server/plugins/plugins_system.test.ts b/src/core/server/plugins/plugins_system.test.ts index 6cca961e70e91..8dd36aba3d822 100644 --- a/src/core/server/plugins/plugins_system.test.ts +++ b/src/core/server/plugins/plugins_system.test.ts @@ -17,7 +17,10 @@ * under the License. */ -import { mockCreatePluginSetupContext } from './plugins_system.test.mocks'; +import { + mockCreatePluginSetupContext, + mockCreatePluginStartContext, +} from './plugins_system.test.mocks'; import { BehaviorSubject } from 'rxjs'; import { Config, ConfigService, Env, ObjectToConfigAdapter } from '../config'; @@ -105,7 +108,7 @@ test('`setupPlugins` throws if plugins have circular required dependency', async ); }); -test('`setupPlugins` throws if plugins have circular optional dependency', async () => { +test('`setupPlugins` && `startPlugins` throws if plugins have circular optional dependency', async () => { pluginsSystem.addPlugin(createPlugin('no-dep')); pluginsSystem.addPlugin(createPlugin('depends-on-1', { optional: ['depends-on-2'] })); pluginsSystem.addPlugin(createPlugin('depends-on-2', { optional: ['depends-on-1'] })); @@ -131,27 +134,58 @@ Array [ `); }); -test('`setupPlugins` correctly orders plugins and returns exposed values', async () => { +test('correctly orders plugins and returns exposed values for "setup" and "start"', async () => { + interface Contracts { + setup: Record; + start: Record; + } const plugins = new Map([ - [createPlugin('order-4', { required: ['order-2'] }), { 'order-2': 'added-as-2' }], - [createPlugin('order-0'), {}], + [ + createPlugin('order-4', { required: ['order-2'] }), + { + setup: { 'order-2': 'added-as-2' }, + start: { 'order-2': 'started-as-2' }, + }, + ], + [ + createPlugin('order-0'), + { + setup: {}, + start: {}, + }, + ], [ createPlugin('order-2', { required: ['order-1'], optional: ['order-0'] }), - { 'order-1': 'added-as-3', 'order-0': 'added-as-1' }, + { + setup: { 'order-1': 'added-as-3', 'order-0': 'added-as-1' }, + start: { 'order-1': 'started-as-3', 'order-0': 'started-as-1' }, + }, + ], + [ + createPlugin('order-1', { required: ['order-0'] }), + { + setup: { 'order-0': 'added-as-1' }, + start: { 'order-0': 'started-as-1' }, + }, ], - [createPlugin('order-1', { required: ['order-0'] }), { 'order-0': 'added-as-1' }], [ createPlugin('order-3', { required: ['order-2'], optional: ['missing-dep'] }), - { 'order-2': 'added-as-2' }, + { + setup: { 'order-2': 'added-as-2' }, + start: { 'order-2': 'started-as-2' }, + }, ], - ] as Array<[PluginWrapper, Record]>); + ] as Array<[PluginWrapper, Contracts]>); const setupContextMap = new Map(); + const startContextMap = new Map(); [...plugins.keys()].forEach((plugin, index) => { jest.spyOn(plugin, 'setup').mockResolvedValue(`added-as-${index}`); + jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`); setupContextMap.set(plugin.name, `setup-for-${plugin.name}`); + startContextMap.set(plugin.name, `start-for-${plugin.name}`); pluginsSystem.addPlugin(plugin); }); @@ -160,6 +194,10 @@ test('`setupPlugins` correctly orders plugins and returns exposed values', async setupContextMap.get(plugin.name) ); + mockCreatePluginStartContext.mockImplementation((context, deps, plugin) => + startContextMap.get(plugin.name) + ); + expect([...(await pluginsSystem.setupPlugins(setupDeps))]).toMatchInlineSnapshot(` Array [ Array [ @@ -188,7 +226,39 @@ Array [ for (const [plugin, deps] of plugins) { expect(mockCreatePluginSetupContext).toHaveBeenCalledWith(coreContext, setupDeps, plugin); expect(plugin.setup).toHaveBeenCalledTimes(1); - expect(plugin.setup).toHaveBeenCalledWith(setupContextMap.get(plugin.name), deps); + expect(plugin.setup).toHaveBeenCalledWith(setupContextMap.get(plugin.name), deps.setup); + } + + const startDeps = {}; + expect([...(await pluginsSystem.startPlugins(startDeps))]).toMatchInlineSnapshot(` +Array [ + Array [ + "order-0", + "started-as-1", + ], + Array [ + "order-1", + "started-as-3", + ], + Array [ + "order-2", + "started-as-2", + ], + Array [ + "order-3", + "started-as-4", + ], + Array [ + "order-4", + "started-as-0", + ], +] +`); + + for (const [plugin, deps] of plugins) { + expect(mockCreatePluginStartContext).toHaveBeenCalledWith(coreContext, startDeps, plugin); + expect(plugin.start).toHaveBeenCalledTimes(1); + expect(plugin.start).toHaveBeenCalledWith(startContextMap.get(plugin.name), deps.start); } }); @@ -271,3 +341,38 @@ Array [ ] `); }); + +test('can start without plugins', async () => { + await pluginsSystem.setupPlugins(setupDeps); + const pluginsStart = await pluginsSystem.startPlugins({}); + + expect(pluginsStart).toBeInstanceOf(Map); + expect(pluginsStart.size).toBe(0); +}); + +test('`startPlugins` only starts plugins that were setup', async () => { + const firstPluginToRun = createPlugin('order-0'); + const secondPluginNotToRun = createPlugin('order-not-run', { server: false }); + const thirdPluginToRun = createPlugin('order-1'); + + [firstPluginToRun, secondPluginNotToRun, thirdPluginToRun].forEach((plugin, index) => { + jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`); + jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`); + + pluginsSystem.addPlugin(plugin); + }); + await pluginsSystem.setupPlugins(setupDeps); + const result = await pluginsSystem.startPlugins({}); + expect([...result]).toMatchInlineSnapshot(` +Array [ + Array [ + "order-1", + "started-as-2", + ], + Array [ + "order-0", + "started-as-0", + ], +] +`); +}); diff --git a/src/core/server/plugins/plugins_system.ts b/src/core/server/plugins/plugins_system.ts index eeb3da0a83862..37eab8226af72 100644 --- a/src/core/server/plugins/plugins_system.ts +++ b/src/core/server/plugins/plugins_system.ts @@ -22,8 +22,8 @@ import { pick } from 'lodash'; import { CoreContext } from '../core_context'; import { Logger } from '../logging'; import { DiscoveredPlugin, DiscoveredPluginInternal, PluginWrapper, PluginName } from './plugin'; -import { createPluginSetupContext } from './plugin_context'; -import { PluginsServiceSetupDeps } from './plugins_service'; +import { createPluginSetupContext, createPluginStartContext } from './plugin_context'; +import { PluginsServiceSetupDeps, PluginsServiceStartDeps } from './plugins_service'; /** @internal */ export class PluginsSystem { @@ -56,8 +56,8 @@ export class PluginsSystem { } this.log.debug(`Setting up plugin "${pluginName}"...`); - - const pluginDepContracts = [...plugin.requiredPlugins, ...plugin.optionalPlugins].reduce( + const pluginDeps = new Set([...plugin.requiredPlugins, ...plugin.optionalPlugins]); + const pluginDepContracts = Array.from(pluginDeps).reduce( (depContracts, dependencyName) => { // Only set if present. Could be absent if plugin does not have server-side code or is a // missing optional dependency. @@ -84,6 +84,43 @@ export class PluginsSystem { return contracts; } + public async startPlugins(deps: PluginsServiceStartDeps) { + const contracts = new Map(); + if (this.satupPlugins.length === 0) { + return contracts; + } + + this.log.info(`Starting [${this.satupPlugins.length}] plugins: [${[...this.satupPlugins]}]`); + + for (const pluginName of this.satupPlugins) { + this.log.debug(`Starting plugin "${pluginName}"...`); + const plugin = this.plugins.get(pluginName)!; + const pluginDeps = new Set([...plugin.requiredPlugins, ...plugin.optionalPlugins]); + const pluginDepContracts = Array.from(pluginDeps).reduce( + (depContracts, dependencyName) => { + // Only set if present. Could be absent if plugin does not have server-side code or is a + // missing optional dependency. + if (contracts.has(dependencyName)) { + depContracts[dependencyName] = contracts.get(dependencyName); + } + + return depContracts; + }, + {} as Record + ); + + contracts.set( + pluginName, + await plugin.start( + createPluginStartContext(this.coreContext, deps, plugin), + pluginDepContracts + ) + ); + } + + return contracts; + } + public async stopPlugins() { if (this.plugins.size === 0 || this.satupPlugins.length === 0) { return; diff --git a/src/core/server/server.ts b/src/core/server/server.ts index 04255d61b4986..4d85c79b13e06 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -65,9 +65,11 @@ export class Server { public async start() { const httpStart = await this.http.start(); + const plugins = await this.plugins.start({}); const startDeps = { http: httpStart, + plugins, }; await this.legacy.start(startDeps); diff --git a/src/legacy/server/kbn_server.d.ts b/src/legacy/server/kbn_server.d.ts index 93573bf4f342c..8212e5e9d3d33 100644 --- a/src/legacy/server/kbn_server.d.ts +++ b/src/legacy/server/kbn_server.d.ts @@ -25,6 +25,7 @@ import { HttpServiceStart, ConfigService, PluginsServiceSetup, + PluginsServiceStart, } from '../../core/server'; import { ApmOssPlugin } from '../core_plugins/apm_oss'; import { CallClusterWithRequest, ElasticsearchPlugin } from '../core_plugins/elasticsearch'; @@ -86,6 +87,7 @@ export default class KbnServer { core: { http: HttpServiceStart; }; + plugins: PluginsServiceStart; }; stop: null; params: { diff --git a/src/legacy/server/kbn_server.js b/src/legacy/server/kbn_server.js index e467088542ffe..7e6d70b3f9a1b 100644 --- a/src/legacy/server/kbn_server.js +++ b/src/legacy/server/kbn_server.js @@ -67,6 +67,7 @@ export default class KbnServer { core: { http: startDeps.http, }, + plugins: startDeps.plugins, }, stop: null, params: { diff --git a/src/plugins/testbed/server/index.ts b/src/plugins/testbed/server/index.ts index 6470251f057d9..e430488dd268b 100644 --- a/src/plugins/testbed/server/index.ts +++ b/src/plugins/testbed/server/index.ts @@ -19,7 +19,13 @@ import { map, mergeMap } from 'rxjs/operators'; -import { Logger, PluginInitializerContext, PluginName, PluginSetupContext } from 'kibana/server'; +import { + Logger, + PluginInitializerContext, + PluginName, + PluginSetupContext, + PluginStartContext, +} from 'kibana/server'; import { TestBedConfig } from './config'; class Plugin { @@ -49,6 +55,20 @@ class Plugin { }; } + public start(startContext: PluginStartContext, deps: Record) { + this.log.debug( + `Starting up TestBed testbed with core contract [${Object.keys( + startContext + )}] and deps [${Object.keys(deps)}]` + ); + + return { + getStartContext() { + return startContext; + }, + }; + } + public stop() { this.log.debug(`Stopping TestBed`); } From 37de718354d641ed22f4b13395e7fc39ebd4d7ae Mon Sep 17 00:00:00 2001 From: restrry Date: Tue, 30 Apr 2019 11:57:14 +0200 Subject: [PATCH 20/20] Revert "introduce start pahse for plugins service for parity with client side" This reverts commit c04fdd2e26f483b3e612b854cc1c15a000a9bd13. --- src/core/server/index.ts | 12 +- src/core/server/legacy/legacy_service.test.ts | 15 +-- src/core/server/plugins/index.ts | 4 +- src/core/server/plugins/plugin.test.ts | 38 ------ src/core/server/plugins/plugin.ts | 47 ++----- src/core/server/plugins/plugin_context.ts | 29 +--- src/core/server/plugins/plugins_service.ts | 16 +-- .../plugins/plugins_system.test.mocks.ts | 2 - .../server/plugins/plugins_system.test.ts | 125 ++---------------- src/core/server/plugins/plugins_system.ts | 45 +------ src/core/server/server.ts | 2 - src/legacy/server/kbn_server.d.ts | 2 - src/legacy/server/kbn_server.js | 1 - src/plugins/testbed/server/index.ts | 22 +-- 14 files changed, 35 insertions(+), 325 deletions(-) diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 448433b5420a9..1aa602e32c384 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -18,7 +18,7 @@ */ import { ElasticsearchServiceSetup } from './elasticsearch'; import { HttpServiceSetup, HttpServiceStart } from './http'; -import { PluginsServiceSetup, PluginsServiceStart } from './plugins'; +import { PluginsServiceSetup } from './plugins'; export { bootstrap } from './bootstrap'; export { ConfigService } from './config'; @@ -47,7 +47,6 @@ export { PluginInitializerContext, PluginName, PluginSetupContext, - PluginStartContext, } from './plugins'; /** @public */ @@ -59,13 +58,6 @@ export interface CoreSetup { export interface CoreStart { http: HttpServiceStart; - plugins: PluginsServiceStart; } -export { - HttpServiceSetup, - HttpServiceStart, - ElasticsearchServiceSetup, - PluginsServiceSetup, - PluginsServiceStart, -}; +export { HttpServiceSetup, HttpServiceStart, ElasticsearchServiceSetup, PluginsServiceSetup }; diff --git a/src/core/server/legacy/legacy_service.test.ts b/src/core/server/legacy/legacy_service.test.ts index dd2883e6e48fb..da5499080eee1 100644 --- a/src/core/server/legacy/legacy_service.test.ts +++ b/src/core/server/legacy/legacy_service.test.ts @@ -35,7 +35,7 @@ import { ElasticsearchServiceSetup } from '../elasticsearch'; import { HttpServiceStart } from '../http'; import { loggingServiceMock } from '../logging/logging_service.mock'; import { DiscoveredPlugin, DiscoveredPluginInternal } from '../plugins'; -import { PluginsServiceSetup, PluginsServiceStart } from '../plugins/plugins_service'; +import { PluginsServiceSetup } from '../plugins/plugins_service'; import { LegacyPlatformProxy } from './legacy_platform_proxy'; const MockKbnServer: jest.Mock = KbnServer as any; @@ -52,7 +52,6 @@ let setupDeps: { let startDeps: { http: HttpServiceStart; - plugins: PluginsServiceStart; }; const logger = loggingServiceMock.create(); @@ -83,9 +82,6 @@ beforeEach(() => { http: { isListening: () => true, }, - plugins: { - contracts: new Map(), - }, }; config$ = new BehaviorSubject( @@ -321,9 +317,6 @@ describe('once LegacyService is set up without connection info', () => { http: { isListening: () => false, }, - plugins: { - contracts: new Map(), - }, }; beforeEach(async () => { await legacyService.setup(setupDeps); @@ -386,9 +379,6 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => { http: { isListening: () => false, }, - plugins: { - contracts: new Map(), - }, }); expect(MockClusterManager.create.mock.calls).toMatchSnapshot( @@ -417,9 +407,6 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => { http: { isListening: () => false, }, - plugins: { - contracts: new Map(), - }, }); expect(MockClusterManager.create.mock.calls).toMatchSnapshot( diff --git a/src/core/server/plugins/index.ts b/src/core/server/plugins/index.ts index 6faef56a43ce7..8469e21783f0f 100644 --- a/src/core/server/plugins/index.ts +++ b/src/core/server/plugins/index.ts @@ -17,7 +17,7 @@ * under the License. */ -export { PluginsService, PluginsServiceSetup, PluginsServiceStart } from './plugins_service'; +export { PluginsService, PluginsServiceSetup } from './plugins_service'; /** @internal */ export { isNewPlatformPlugin } from './discovery'; @@ -29,4 +29,4 @@ export { PluginInitializer, PluginName, } from './plugin'; -export { PluginInitializerContext, PluginSetupContext, PluginStartContext } from './plugin_context'; +export { PluginInitializerContext, PluginSetupContext } from './plugin_context'; diff --git a/src/core/server/plugins/plugin.test.ts b/src/core/server/plugins/plugin.test.ts index cb4203244e86d..dfeb3f0e5ed40 100644 --- a/src/core/server/plugins/plugin.test.ts +++ b/src/core/server/plugins/plugin.test.ts @@ -179,44 +179,6 @@ test('`setup` initializes plugin and calls appropriate lifecycle hook', async () expect(mockPluginInstance.setup).toHaveBeenCalledWith(setupContext, setupDependencies); }); -test('`start` fails if setup is not called first', async () => { - const manifest = createPluginManifest(); - const plugin = new PluginWrapper( - 'some-plugin-path', - manifest, - createPluginInitializerContext(coreContext, manifest) - ); - - await expect(plugin.start({} as any, {} as any)).rejects.toThrowErrorMatchingInlineSnapshot( - `"Plugin \\"some-plugin-id\\" can't be started since it isn't set up."` - ); -}); - -test('`start` calls plugin.start with context and dependencies', async () => { - const manifest = createPluginManifest(); - const plugin = new PluginWrapper( - 'plugin-with-initializer-path', - manifest, - createPluginInitializerContext(coreContext, manifest) - ); - const context = { any: 'thing' } as any; - const deps = { otherDep: 'value' }; - - const pluginStartContract = { contract: 'start-contract' }; - const mockPluginInstance = { - setup: jest.fn(), - start: jest.fn().mockResolvedValue(pluginStartContract), - }; - mockPluginInitializer.mockReturnValue(mockPluginInstance); - - await plugin.setup({} as any, {} as any); - - const startContract = await plugin.start(context, deps); - - expect(startContract).toBe(pluginStartContract); - expect(mockPluginInstance.start).toHaveBeenCalledWith(context, deps); -}); - test('`stop` fails if plugin is not set up', async () => { const manifest = createPluginManifest(); const plugin = new PluginWrapper( diff --git a/src/core/server/plugins/plugin.ts b/src/core/server/plugins/plugin.ts index 72981005968f3..5e23fb3b8b30e 100644 --- a/src/core/server/plugins/plugin.ts +++ b/src/core/server/plugins/plugin.ts @@ -21,7 +21,7 @@ import { join } from 'path'; import typeDetect from 'type-detect'; import { ConfigPath } from '../config'; import { Logger } from '../logging'; -import { PluginInitializerContext, PluginSetupContext, PluginStartContext } from './plugin_context'; +import { PluginInitializerContext, PluginSetupContext } from './plugin_context'; /** * Dedicated type for plugin name/id that is supposed to make Map/Set/Arrays @@ -129,14 +129,11 @@ export interface DiscoveredPluginInternal extends DiscoveredPlugin { * * @public */ -export interface Plugin< - TSetup, - TStart, - TPluginsSetup extends Record = {}, - TPluginsStart extends Record = {} -> { - setup: (core: PluginSetupContext, plugins: TPluginsSetup) => TSetup | Promise; - start: (core: PluginStartContext, plugins: TPluginsStart) => TStart | Promise; +export interface Plugin = {}> { + setup: ( + pluginSetupContext: PluginSetupContext, + plugins: TPluginsSetup + ) => TSetup | Promise; stop?: () => void; } @@ -146,12 +143,9 @@ export interface Plugin< * * @public */ -export type PluginInitializer< - TSetup, - TStart, - TPluginsSetup extends Record = {}, - TPluginsStart extends Record = {} -> = (core: PluginInitializerContext) => Plugin; +export type PluginInitializer = {}> = ( + coreContext: PluginInitializerContext +) => Plugin; /** * Lightweight wrapper around discovered plugin that is responsible for instantiating @@ -161,9 +155,7 @@ export type PluginInitializer< */ export class PluginWrapper< TSetup = unknown, - TStart = unknown, - TPluginsSetup extends Record = Record, - TPluginsStart extends Record = Record + TPluginsSetup extends Record = Record > { public readonly name: PluginManifest['id']; public readonly configPath: PluginManifest['configPath']; @@ -174,7 +166,7 @@ export class PluginWrapper< private readonly log: Logger; - private instance?: Plugin; + private instance?: Plugin; constructor( public readonly path: string, @@ -205,21 +197,6 @@ export class PluginWrapper< return await this.instance.setup(setupContext, plugins); } - /** - * Calls `start` function exposed by the initialized plugin. - * @param startContext Context that consists of various core services tailored specifically - * for the `start` lifecycle event. - * @param plugins The dictionary where the key is the dependency name and the value - * is the contract returned by the dependency's `start` function. - */ - public async start(startContext: PluginStartContext, plugins: TPluginsStart) { - if (this.instance === undefined) { - throw new Error(`Plugin "${this.name}" can't be started since it isn't set up.`); - } - - return await this.instance.start(startContext, plugins); - } - /** * Calls optional `stop` function exposed by the plugin initializer. */ @@ -247,7 +224,7 @@ export class PluginWrapper< } const { plugin: initializer } = pluginDefinition as { - plugin: PluginInitializer; + plugin: PluginInitializer; }; if (!initializer || typeof initializer !== 'function') { throw new Error(`Definition of plugin "${this.name}" should be a function (${this.path}).`); diff --git a/src/core/server/plugins/plugin_context.ts b/src/core/server/plugins/plugin_context.ts index b738e192896dd..1de59ddaef8a3 100644 --- a/src/core/server/plugins/plugin_context.ts +++ b/src/core/server/plugins/plugin_context.ts @@ -25,7 +25,7 @@ import { ClusterClient } from '../elasticsearch'; import { HttpServiceSetup } from '../http'; import { LoggerFactory } from '../logging'; import { PluginWrapper, PluginManifest } from './plugin'; -import { PluginsServiceSetupDeps, PluginsServiceStartDeps } from './plugins_service'; +import { PluginsServiceSetupDeps } from './plugins_service'; /** * Context that's available to plugins during initialization stage. @@ -61,13 +61,6 @@ export interface PluginSetupContext { }; } -/** - * Context passed to the plugins `start` method. - * - * @public - */ -export interface PluginStartContext {} // eslint-disable-line @typescript-eslint/no-empty-interface - /** * This returns a facade for `CoreContext` that will be exposed to the plugin initializer. * This facade should be safe to use across entire plugin lifespan. @@ -151,23 +144,3 @@ export function createPluginSetupContext( }, }; } - -/** - * This returns a facade for `CoreContext` that will be exposed to the plugin `start` method. - * This facade should be safe to use only within `start` itself. - * - * This is called for each plugin when it starts, so each plugin gets its own - * version of these values. - * - * @param coreContext Kibana core context - * @param plugin The plugin we're building these values for. - * @param deps Dependencies that Plugins services gets during start. - * @internal - */ -export function createPluginStartContext( - coreContext: CoreContext, - deps: PluginsServiceStartDeps, - plugin: PluginWrapper -): PluginStartContext { - return {}; -} diff --git a/src/core/server/plugins/plugins_service.ts b/src/core/server/plugins/plugins_service.ts index f243754db8415..0caba3477bf24 100644 --- a/src/core/server/plugins/plugins_service.ts +++ b/src/core/server/plugins/plugins_service.ts @@ -38,11 +38,6 @@ export interface PluginsServiceSetup { }; } -/** @internal */ -export interface PluginsServiceStart { - contracts: Map; -} - /** @internal */ export interface PluginsServiceSetupDeps { elasticsearch: ElasticsearchServiceSetup; @@ -50,10 +45,7 @@ export interface PluginsServiceSetupDeps { } /** @internal */ -export interface PluginsServiceStartDeps {} // eslint-disable-line @typescript-eslint/no-empty-interface - -/** @internal */ -export class PluginsService implements CoreService { +export class PluginsService implements CoreService { private readonly log: Logger; private readonly pluginsSystem: PluginsSystem; @@ -88,11 +80,7 @@ export class PluginsService implements CoreService ({ createPluginSetupContext: mockCreatePluginSetupContext, - createPluginStartContext: mockCreatePluginStartContext, })); diff --git a/src/core/server/plugins/plugins_system.test.ts b/src/core/server/plugins/plugins_system.test.ts index 8dd36aba3d822..6cca961e70e91 100644 --- a/src/core/server/plugins/plugins_system.test.ts +++ b/src/core/server/plugins/plugins_system.test.ts @@ -17,10 +17,7 @@ * under the License. */ -import { - mockCreatePluginSetupContext, - mockCreatePluginStartContext, -} from './plugins_system.test.mocks'; +import { mockCreatePluginSetupContext } from './plugins_system.test.mocks'; import { BehaviorSubject } from 'rxjs'; import { Config, ConfigService, Env, ObjectToConfigAdapter } from '../config'; @@ -108,7 +105,7 @@ test('`setupPlugins` throws if plugins have circular required dependency', async ); }); -test('`setupPlugins` && `startPlugins` throws if plugins have circular optional dependency', async () => { +test('`setupPlugins` throws if plugins have circular optional dependency', async () => { pluginsSystem.addPlugin(createPlugin('no-dep')); pluginsSystem.addPlugin(createPlugin('depends-on-1', { optional: ['depends-on-2'] })); pluginsSystem.addPlugin(createPlugin('depends-on-2', { optional: ['depends-on-1'] })); @@ -134,58 +131,27 @@ Array [ `); }); -test('correctly orders plugins and returns exposed values for "setup" and "start"', async () => { - interface Contracts { - setup: Record; - start: Record; - } +test('`setupPlugins` correctly orders plugins and returns exposed values', async () => { const plugins = new Map([ - [ - createPlugin('order-4', { required: ['order-2'] }), - { - setup: { 'order-2': 'added-as-2' }, - start: { 'order-2': 'started-as-2' }, - }, - ], - [ - createPlugin('order-0'), - { - setup: {}, - start: {}, - }, - ], + [createPlugin('order-4', { required: ['order-2'] }), { 'order-2': 'added-as-2' }], + [createPlugin('order-0'), {}], [ createPlugin('order-2', { required: ['order-1'], optional: ['order-0'] }), - { - setup: { 'order-1': 'added-as-3', 'order-0': 'added-as-1' }, - start: { 'order-1': 'started-as-3', 'order-0': 'started-as-1' }, - }, - ], - [ - createPlugin('order-1', { required: ['order-0'] }), - { - setup: { 'order-0': 'added-as-1' }, - start: { 'order-0': 'started-as-1' }, - }, + { 'order-1': 'added-as-3', 'order-0': 'added-as-1' }, ], + [createPlugin('order-1', { required: ['order-0'] }), { 'order-0': 'added-as-1' }], [ createPlugin('order-3', { required: ['order-2'], optional: ['missing-dep'] }), - { - setup: { 'order-2': 'added-as-2' }, - start: { 'order-2': 'started-as-2' }, - }, + { 'order-2': 'added-as-2' }, ], - ] as Array<[PluginWrapper, Contracts]>); + ] as Array<[PluginWrapper, Record]>); const setupContextMap = new Map(); - const startContextMap = new Map(); [...plugins.keys()].forEach((plugin, index) => { jest.spyOn(plugin, 'setup').mockResolvedValue(`added-as-${index}`); - jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`); setupContextMap.set(plugin.name, `setup-for-${plugin.name}`); - startContextMap.set(plugin.name, `start-for-${plugin.name}`); pluginsSystem.addPlugin(plugin); }); @@ -194,10 +160,6 @@ test('correctly orders plugins and returns exposed values for "setup" and "start setupContextMap.get(plugin.name) ); - mockCreatePluginStartContext.mockImplementation((context, deps, plugin) => - startContextMap.get(plugin.name) - ); - expect([...(await pluginsSystem.setupPlugins(setupDeps))]).toMatchInlineSnapshot(` Array [ Array [ @@ -226,39 +188,7 @@ Array [ for (const [plugin, deps] of plugins) { expect(mockCreatePluginSetupContext).toHaveBeenCalledWith(coreContext, setupDeps, plugin); expect(plugin.setup).toHaveBeenCalledTimes(1); - expect(plugin.setup).toHaveBeenCalledWith(setupContextMap.get(plugin.name), deps.setup); - } - - const startDeps = {}; - expect([...(await pluginsSystem.startPlugins(startDeps))]).toMatchInlineSnapshot(` -Array [ - Array [ - "order-0", - "started-as-1", - ], - Array [ - "order-1", - "started-as-3", - ], - Array [ - "order-2", - "started-as-2", - ], - Array [ - "order-3", - "started-as-4", - ], - Array [ - "order-4", - "started-as-0", - ], -] -`); - - for (const [plugin, deps] of plugins) { - expect(mockCreatePluginStartContext).toHaveBeenCalledWith(coreContext, startDeps, plugin); - expect(plugin.start).toHaveBeenCalledTimes(1); - expect(plugin.start).toHaveBeenCalledWith(startContextMap.get(plugin.name), deps.start); + expect(plugin.setup).toHaveBeenCalledWith(setupContextMap.get(plugin.name), deps); } }); @@ -341,38 +271,3 @@ Array [ ] `); }); - -test('can start without plugins', async () => { - await pluginsSystem.setupPlugins(setupDeps); - const pluginsStart = await pluginsSystem.startPlugins({}); - - expect(pluginsStart).toBeInstanceOf(Map); - expect(pluginsStart.size).toBe(0); -}); - -test('`startPlugins` only starts plugins that were setup', async () => { - const firstPluginToRun = createPlugin('order-0'); - const secondPluginNotToRun = createPlugin('order-not-run', { server: false }); - const thirdPluginToRun = createPlugin('order-1'); - - [firstPluginToRun, secondPluginNotToRun, thirdPluginToRun].forEach((plugin, index) => { - jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`); - jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`); - - pluginsSystem.addPlugin(plugin); - }); - await pluginsSystem.setupPlugins(setupDeps); - const result = await pluginsSystem.startPlugins({}); - expect([...result]).toMatchInlineSnapshot(` -Array [ - Array [ - "order-1", - "started-as-2", - ], - Array [ - "order-0", - "started-as-0", - ], -] -`); -}); diff --git a/src/core/server/plugins/plugins_system.ts b/src/core/server/plugins/plugins_system.ts index 37eab8226af72..eeb3da0a83862 100644 --- a/src/core/server/plugins/plugins_system.ts +++ b/src/core/server/plugins/plugins_system.ts @@ -22,8 +22,8 @@ import { pick } from 'lodash'; import { CoreContext } from '../core_context'; import { Logger } from '../logging'; import { DiscoveredPlugin, DiscoveredPluginInternal, PluginWrapper, PluginName } from './plugin'; -import { createPluginSetupContext, createPluginStartContext } from './plugin_context'; -import { PluginsServiceSetupDeps, PluginsServiceStartDeps } from './plugins_service'; +import { createPluginSetupContext } from './plugin_context'; +import { PluginsServiceSetupDeps } from './plugins_service'; /** @internal */ export class PluginsSystem { @@ -56,8 +56,8 @@ export class PluginsSystem { } this.log.debug(`Setting up plugin "${pluginName}"...`); - const pluginDeps = new Set([...plugin.requiredPlugins, ...plugin.optionalPlugins]); - const pluginDepContracts = Array.from(pluginDeps).reduce( + + const pluginDepContracts = [...plugin.requiredPlugins, ...plugin.optionalPlugins].reduce( (depContracts, dependencyName) => { // Only set if present. Could be absent if plugin does not have server-side code or is a // missing optional dependency. @@ -84,43 +84,6 @@ export class PluginsSystem { return contracts; } - public async startPlugins(deps: PluginsServiceStartDeps) { - const contracts = new Map(); - if (this.satupPlugins.length === 0) { - return contracts; - } - - this.log.info(`Starting [${this.satupPlugins.length}] plugins: [${[...this.satupPlugins]}]`); - - for (const pluginName of this.satupPlugins) { - this.log.debug(`Starting plugin "${pluginName}"...`); - const plugin = this.plugins.get(pluginName)!; - const pluginDeps = new Set([...plugin.requiredPlugins, ...plugin.optionalPlugins]); - const pluginDepContracts = Array.from(pluginDeps).reduce( - (depContracts, dependencyName) => { - // Only set if present. Could be absent if plugin does not have server-side code or is a - // missing optional dependency. - if (contracts.has(dependencyName)) { - depContracts[dependencyName] = contracts.get(dependencyName); - } - - return depContracts; - }, - {} as Record - ); - - contracts.set( - pluginName, - await plugin.start( - createPluginStartContext(this.coreContext, deps, plugin), - pluginDepContracts - ) - ); - } - - return contracts; - } - public async stopPlugins() { if (this.plugins.size === 0 || this.satupPlugins.length === 0) { return; diff --git a/src/core/server/server.ts b/src/core/server/server.ts index 4d85c79b13e06..04255d61b4986 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -65,11 +65,9 @@ export class Server { public async start() { const httpStart = await this.http.start(); - const plugins = await this.plugins.start({}); const startDeps = { http: httpStart, - plugins, }; await this.legacy.start(startDeps); diff --git a/src/legacy/server/kbn_server.d.ts b/src/legacy/server/kbn_server.d.ts index 8212e5e9d3d33..93573bf4f342c 100644 --- a/src/legacy/server/kbn_server.d.ts +++ b/src/legacy/server/kbn_server.d.ts @@ -25,7 +25,6 @@ import { HttpServiceStart, ConfigService, PluginsServiceSetup, - PluginsServiceStart, } from '../../core/server'; import { ApmOssPlugin } from '../core_plugins/apm_oss'; import { CallClusterWithRequest, ElasticsearchPlugin } from '../core_plugins/elasticsearch'; @@ -87,7 +86,6 @@ export default class KbnServer { core: { http: HttpServiceStart; }; - plugins: PluginsServiceStart; }; stop: null; params: { diff --git a/src/legacy/server/kbn_server.js b/src/legacy/server/kbn_server.js index 7e6d70b3f9a1b..e467088542ffe 100644 --- a/src/legacy/server/kbn_server.js +++ b/src/legacy/server/kbn_server.js @@ -67,7 +67,6 @@ export default class KbnServer { core: { http: startDeps.http, }, - plugins: startDeps.plugins, }, stop: null, params: { diff --git a/src/plugins/testbed/server/index.ts b/src/plugins/testbed/server/index.ts index e430488dd268b..6470251f057d9 100644 --- a/src/plugins/testbed/server/index.ts +++ b/src/plugins/testbed/server/index.ts @@ -19,13 +19,7 @@ import { map, mergeMap } from 'rxjs/operators'; -import { - Logger, - PluginInitializerContext, - PluginName, - PluginSetupContext, - PluginStartContext, -} from 'kibana/server'; +import { Logger, PluginInitializerContext, PluginName, PluginSetupContext } from 'kibana/server'; import { TestBedConfig } from './config'; class Plugin { @@ -55,20 +49,6 @@ class Plugin { }; } - public start(startContext: PluginStartContext, deps: Record) { - this.log.debug( - `Starting up TestBed testbed with core contract [${Object.keys( - startContext - )}] and deps [${Object.keys(deps)}]` - ); - - return { - getStartContext() { - return startContext; - }, - }; - } - public stop() { this.log.debug(`Stopping TestBed`); }