From 0228ad781bb4aea7ff79fcf9f72de66488339a23 Mon Sep 17 00:00:00 2001 From: Tim Sullivan Date: Tue, 16 Jun 2020 15:18:17 -0700 Subject: [PATCH] [Reporting/Server] register plugin routes synchronously (#68976) (#69224) * register routes synchronously * back out some refactoring * comment fix * fix tests * register route handler context provider * Add function level comments in core methods * fix tests * revert editor help * route context is the ReportingStart contract * Fix reporting job route tests * Fix generation tests Co-authored-by: Joel Griffith Co-authored-by: Elastic Machine # Conflicts: # x-pack/plugins/reporting/server/routes/generation.ts Co-authored-by: Elastic Machine --- x-pack/plugins/reporting/server/core.ts | 113 ++++++++++++------ .../csv/server/execute_job.test.ts | 25 ++-- .../export_types/csv/server/execute_job.ts | 2 +- .../server/lib/generate_csv_search.ts | 2 +- .../server/execute_job/index.test.ts | 36 +----- .../reporting/server/lib/jobs_query.ts | 12 +- .../plugins/reporting/server/plugin.test.ts | 3 +- x-pack/plugins/reporting/server/plugin.ts | 74 ++++++------ .../server/routes/generation.test.ts | 4 +- .../reporting/server/routes/generation.ts | 21 ++-- .../reporting/server/routes/jobs.test.ts | 4 +- .../plugins/reporting/server/routes/jobs.ts | 55 +++++++-- .../routes/lib/authorized_user_pre_routing.ts | 2 +- .../server/routes/lib/job_response_handler.ts | 21 ++-- .../create_mock_reportingplugin.ts | 81 +++++-------- .../usage/reporting_usage_collector.test.ts | 27 +++-- .../server/usage/reporting_usage_collector.ts | 14 +-- 17 files changed, 262 insertions(+), 234 deletions(-) diff --git a/x-pack/plugins/reporting/server/core.ts b/x-pack/plugins/reporting/server/core.ts index 94b138ffcae0b..9acd359fa0db4 100644 --- a/x-pack/plugins/reporting/server/core.ts +++ b/x-pack/plugins/reporting/server/core.ts @@ -5,7 +5,7 @@ */ import * as Rx from 'rxjs'; -import { first, map, mapTo } from 'rxjs/operators'; +import { first, map, take } from 'rxjs/operators'; import { BasePath, ElasticsearchServiceSetup, @@ -33,7 +33,8 @@ export interface ReportingInternalSetup { security?: SecurityPluginSetup; } -interface ReportingInternalStart { +export interface ReportingInternalStart { + browserDriverFactory: HeadlessChromiumDriverFactory; enqueueJob: EnqueueJobFn; esqueue: ESQueueInstance; savedObjects: SavedObjectsServiceStart; @@ -43,33 +44,83 @@ interface ReportingInternalStart { export class ReportingCore { private pluginSetupDeps?: ReportingInternalSetup; private pluginStartDeps?: ReportingInternalStart; - private browserDriverFactory?: HeadlessChromiumDriverFactory; - private readonly pluginSetup$ = new Rx.ReplaySubject(); - private readonly pluginStart$ = new Rx.ReplaySubject(); + private readonly pluginSetup$ = new Rx.ReplaySubject(); // observe async background setupDeps and config each are done + private readonly pluginStart$ = new Rx.ReplaySubject(); // observe async background startDeps private exportTypesRegistry = getExportTypesRegistry(); + private config?: ReportingConfig; - constructor(private config: ReportingConfig) {} + constructor() {} - public pluginSetup(reportingSetupDeps: ReportingInternalSetup) { - this.pluginSetupDeps = reportingSetupDeps; - this.pluginSetup$.next(reportingSetupDeps); + /* + * Register setupDeps + */ + public pluginSetup(setupDeps: ReportingInternalSetup) { + this.pluginSetup$.next(true); // trigger the observer + this.pluginSetupDeps = setupDeps; // cache } - public pluginStart(reportingStartDeps: ReportingInternalStart) { - this.pluginStart$.next(reportingStartDeps); + /* + * Register startDeps + */ + public pluginStart(startDeps: ReportingInternalStart) { + this.pluginStart$.next(startDeps); // trigger the observer + this.pluginStartDeps = startDeps; // cache } - public pluginHasStarted(): Promise { - return this.pluginStart$.pipe(first(), mapTo(true)).toPromise(); + /* + * Blocks the caller until setup is done + */ + public async pluginSetsUp(): Promise { + // use deps and config as a cached resolver + if (this.pluginSetupDeps && this.config) { + return true; + } + return await this.pluginSetup$.pipe(take(2)).toPromise(); // once for pluginSetupDeps (sync) and twice for config (async) } - public setBrowserDriverFactory(browserDriverFactory: HeadlessChromiumDriverFactory) { - this.browserDriverFactory = browserDriverFactory; + /* + * Blocks the caller until start is done + */ + public async pluginStartsUp(): Promise { + return await this.getPluginStartDeps().then(() => true); + } + + /* + * Synchronously checks if all async background setup and startup is completed + */ + public pluginIsStarted() { + return this.pluginSetupDeps != null && this.config != null && this.pluginStartDeps != null; } /* - * Internal module dependencies + * Allows config to be set in the background */ + public setConfig(config: ReportingConfig) { + this.config = config; + this.pluginSetup$.next(true); + } + + /* + * Gives synchronous access to the config + */ + public getConfig(): ReportingConfig { + if (!this.config) { + throw new Error('Config is not yet initialized'); + } + return this.config; + } + + /* + * Gives async access to the startDeps + */ + private async getPluginStartDeps() { + if (this.pluginStartDeps) { + return this.pluginStartDeps; + } + + return await this.pluginStart$.pipe(first()).toPromise(); + } + public getExportTypesRegistry() { return this.exportTypesRegistry; } @@ -92,18 +143,15 @@ export class ReportingCore { .toPromise(); } - public getConfig(): ReportingConfig { - return this.config; - } - - public getScreenshotsObservable(): ScreenshotsObservableFn { - const { browserDriverFactory } = this; - if (!browserDriverFactory) { - throw new Error(`"browserDriverFactory" dependency hasn't initialized yet`); - } - return screenshotsObservableFactory(this.config.get('capture'), browserDriverFactory); + public async getScreenshotsObservable(): Promise { + const config = this.getConfig(); + const { browserDriverFactory } = await this.getPluginStartDeps(); + return screenshotsObservableFactory(config.get('capture'), browserDriverFactory); } + /* + * Gives synchronous access to the setupDeps + */ public getPluginSetupDeps() { if (!this.pluginSetupDeps) { throw new Error(`"pluginSetupDeps" dependencies haven't initialized yet`); @@ -111,18 +159,7 @@ export class ReportingCore { return this.pluginSetupDeps; } - /* - * Outside dependencies - */ - - private async getPluginStartDeps() { - if (this.pluginStartDeps) { - return this.pluginStartDeps; - } - return await this.pluginStart$.pipe(first()).toPromise(); - } - - public async getElasticsearchService() { + public getElasticsearchService() { return this.getPluginSetupDeps().elasticsearch; } diff --git a/x-pack/plugins/reporting/server/export_types/csv/server/execute_job.test.ts b/x-pack/plugins/reporting/server/export_types/csv/server/execute_job.test.ts index 4ffbcf640e685..e7416b6fcf914 100644 --- a/x-pack/plugins/reporting/server/export_types/csv/server/execute_job.test.ts +++ b/x-pack/plugins/reporting/server/export_types/csv/server/execute_job.test.ts @@ -5,10 +5,16 @@ */ import nodeCrypto from '@elastic/node-crypto'; +import { IUiSettingsClient, ElasticsearchServiceSetup } from 'kibana/server'; // @ts-ignore import Puid from 'puid'; import sinon from 'sinon'; +import { ReportingConfig, ReportingCore } from '../../../'; import { fieldFormats, UI_SETTINGS } from '../../../../../../../src/plugins/data/server'; +import { + CSV_QUOTE_VALUES_SETTING, + CSV_SEPARATOR_SETTING, +} from '../../../../../../../src/plugins/share/server'; import { CancellationToken } from '../../../../common'; import { CSV_BOM_CHARS } from '../../../../common/constants'; import { LevelLogger } from '../../../lib'; @@ -16,10 +22,6 @@ import { setFieldFormats } from '../../../services'; import { createMockReportingCore } from '../../../test_helpers'; import { JobDocPayloadDiscoverCsv } from '../types'; import { executeJobFactory } from './execute_job'; -import { - CSV_SEPARATOR_SETTING, - CSV_QUOTE_VALUES_SETTING, -} from '../../../../../../../src/plugins/share/server'; const delay = (ms: number) => new Promise((resolve) => setTimeout(() => resolve(), ms)); @@ -48,8 +50,8 @@ describe('CSV Execute Job', function () { let clusterStub: any; let configGetStub: any; - let mockReportingConfig: any; - let mockReportingCore: any; + let mockReportingConfig: ReportingConfig; + let mockReportingCore: ReportingCore; let callAsCurrentUserStub: any; let cancellationToken: any; @@ -78,9 +80,11 @@ describe('CSV Execute Job', function () { mockReportingConfig = { get: configGetStub, kbnConfig: { get: configGetStub } }; mockReportingCore = await createMockReportingCore(mockReportingConfig); - mockReportingCore.getUiSettingsServiceFactory = () => Promise.resolve(mockUiSettingsClient); - mockReportingCore.getElasticsearchService = () => Promise.resolve(mockElasticsearch); - mockReportingCore.config = mockReportingConfig; + mockReportingCore.getUiSettingsServiceFactory = () => + Promise.resolve((mockUiSettingsClient as unknown) as IUiSettingsClient); + mockReportingCore.getElasticsearchService = () => + mockElasticsearch as ElasticsearchServiceSetup; + mockReportingCore.setConfig(mockReportingConfig); cancellationToken = new CancellationToken(); @@ -996,7 +1000,8 @@ describe('CSV Execute Job', function () { let maxSizeReached: boolean; beforeEach(async function () { - mockReportingCore.getUiSettingsServiceFactory = () => mockUiSettingsClient; + mockReportingCore.getUiSettingsServiceFactory = () => + Promise.resolve((mockUiSettingsClient as unknown) as IUiSettingsClient); configGetStub.withArgs('csv', 'maxSizeBytes').returns(18); callAsCurrentUserStub.onFirstCall().returns({ diff --git a/x-pack/plugins/reporting/server/export_types/csv/server/execute_job.ts b/x-pack/plugins/reporting/server/export_types/csv/server/execute_job.ts index 4b17cc669efe1..91a4db0469fb5 100644 --- a/x-pack/plugins/reporting/server/export_types/csv/server/execute_job.ts +++ b/x-pack/plugins/reporting/server/export_types/csv/server/execute_job.ts @@ -33,7 +33,7 @@ export const executeJobFactory: ExecuteJobFactory callAsCurrentUser(...params); const uiSettings = await getUiSettings(uiConfig); diff --git a/x-pack/plugins/reporting/server/export_types/printable_pdf/server/execute_job/index.test.ts b/x-pack/plugins/reporting/server/export_types/printable_pdf/server/execute_job/index.test.ts index 10f3ea4181247..2693e16e41f15 100644 --- a/x-pack/plugins/reporting/server/export_types/printable_pdf/server/execute_job/index.test.ts +++ b/x-pack/plugins/reporting/server/export_types/printable_pdf/server/execute_job/index.test.ts @@ -84,6 +84,7 @@ test(`passes browserTimezone to generatePdf`, async () => { await executeJob( 'pdfJobId', getJobDocPayload({ + title: 'PDF Params Timezone Test', relativeUrl: '/app/kibana#/something', browserTimezone, headers: encryptedHeaders, @@ -91,39 +92,8 @@ test(`passes browserTimezone to generatePdf`, async () => { cancellationToken ); - expect(generatePdfObservable.mock.calls).toMatchInlineSnapshot(` - Array [ - Array [ - LevelLogger { - "_logger": Object { - "get": [MockFunction], - }, - "_tags": Array [ - "printable_pdf", - "execute", - "pdfJobId", - ], - "warning": [Function], - }, - undefined, - Array [ - "http://localhost:5601/sbp/app/kibana#/something", - ], - "UTC", - Object { - "conditions": Object { - "basePath": "/sbp", - "hostname": "localhost", - "port": 5601, - "protocol": "http", - }, - "headers": Object {}, - }, - undefined, - false, - ], - ] - `); + const tzParam = generatePdfObservable.mock.calls[0][3]; + expect(tzParam).toBe('UTC'); }); test(`returns content_type of application/pdf`, async () => { diff --git a/x-pack/plugins/reporting/server/lib/jobs_query.ts b/x-pack/plugins/reporting/server/lib/jobs_query.ts index 8784d8ff35d25..f4670847260ee 100644 --- a/x-pack/plugins/reporting/server/lib/jobs_query.ts +++ b/x-pack/plugins/reporting/server/lib/jobs_query.ts @@ -6,10 +6,9 @@ import { i18n } from '@kbn/i18n'; import { errors as elasticsearchErrors } from 'elasticsearch'; -import { ElasticsearchServiceSetup } from 'kibana/server'; import { get } from 'lodash'; +import { ReportingCore } from '../'; import { AuthenticatedUser } from '../../../security/server'; -import { ReportingConfig } from '../'; import { JobSource } from '../types'; const esErrors = elasticsearchErrors as Record; @@ -42,11 +41,8 @@ interface CountAggResult { const getUsername = (user: AuthenticatedUser | null) => (user ? user.username : false); -export function jobsQueryFactory( - config: ReportingConfig, - elasticsearch: ElasticsearchServiceSetup -) { - const index = config.get('index'); +export function jobsQueryFactory(reportingCore: ReportingCore) { + const { elasticsearch } = reportingCore.getPluginSetupDeps(); const { callAsInternalUser } = elasticsearch.legacy.client; function execQuery(queryType: string, body: QueryBody) { @@ -60,6 +56,8 @@ export function jobsQueryFactory( }, }; + const config = reportingCore.getConfig(); + const index = config.get('index'); const query = { index: `${index}-*`, body: Object.assign(defaultBody[queryType] || {}, body), diff --git a/x-pack/plugins/reporting/server/plugin.test.ts b/x-pack/plugins/reporting/server/plugin.test.ts index b2bcd6b9c97ce..420fa8347cdeb 100644 --- a/x-pack/plugins/reporting/server/plugin.test.ts +++ b/x-pack/plugins/reporting/server/plugin.test.ts @@ -3,6 +3,7 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ + jest.mock('./browsers/install', () => ({ installBrowser: jest.fn().mockImplementation(() => ({ binaryPath$: { @@ -62,10 +63,10 @@ describe('Reporting Plugin', () => { }); it('logs setup issues', async () => { + initContext.config = null; const plugin = new ReportingPlugin(initContext); // @ts-ignore overloading error logger plugin.logger.error = jest.fn(); - coreSetup.elasticsearch = null; plugin.setup(coreSetup, pluginSetup); await sleep(5); diff --git a/x-pack/plugins/reporting/server/plugin.ts b/x-pack/plugins/reporting/server/plugin.ts index a3c89c7b8a8ce..693b0917603fc 100644 --- a/x-pack/plugins/reporting/server/plugin.ts +++ b/x-pack/plugins/reporting/server/plugin.ts @@ -4,7 +4,6 @@ * you may not use this file except in compliance with the Elastic License. */ -import * as Rx from 'rxjs'; import { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from 'src/core/server'; import { ReportingCore } from './'; import { initializeBrowserDriverFactory } from './browsers'; @@ -15,47 +14,57 @@ import { setFieldFormats } from './services'; import { ReportingSetup, ReportingSetupDeps, ReportingStart, ReportingStartDeps } from './types'; import { registerReportingUsageCollector } from './usage'; +declare module 'src/core/server' { + interface RequestHandlerContext { + reporting?: ReportingStart | null; + } +} + export class ReportingPlugin implements Plugin { private readonly initializerContext: PluginInitializerContext; private logger: LevelLogger; - private reportingCore?: ReportingCore; - - // Setup some observables for modules that need to await setup/start - public readonly setup$ = new Rx.Subject(); - public readonly start$ = new Rx.Subject(); + private reportingCore: ReportingCore; constructor(context: PluginInitializerContext) { this.logger = new LevelLogger(context.logger.get()); this.initializerContext = context; + this.reportingCore = new ReportingCore(); } public setup(core: CoreSetup, plugins: ReportingSetupDeps) { + // prevent throwing errors in route handlers about async deps not being initialized + core.http.registerRouteHandlerContext('reporting', () => { + if (this.reportingCore.pluginIsStarted()) { + return {}; // ReportingStart contract + } else { + return null; + } + }); + const { elasticsearch, http } = core; const { licensing, security } = plugins; - const { initializerContext: initContext } = this; + const { initializerContext: initContext, reportingCore } = this; + const router = http.createRouter(); const basePath = http.basePath.get; + reportingCore.pluginSetup({ + elasticsearch, + licensing, + basePath, + router, + security, + }); + + registerReportingUsageCollector(reportingCore, plugins); + registerRoutes(reportingCore, this.logger); + // async background setup (async () => { const config = await buildConfig(initContext, core, this.logger); - const reportingCore = new ReportingCore(config); - - reportingCore.pluginSetup({ - elasticsearch, - licensing, - basePath, - router, - security, - }); - - registerReportingUsageCollector(reportingCore, plugins); - registerRoutes(reportingCore, this.logger); - this.reportingCore = reportingCore; - + reportingCore.setConfig(config); this.logger.debug('Setup complete'); - this.setup$.next(true); })().catch((e) => { this.logger.error(`Error in Reporting setup, reporting may not function properly`); this.logger.error(e); @@ -68,20 +77,21 @@ export class ReportingPlugin // use data plugin for csv formats setFieldFormats(plugins.data.fieldFormats); - const { logger } = this; - const reportingCore = this.getReportingCore(); - const config = reportingCore.getConfig(); + const { logger, reportingCore } = this; const { elasticsearch } = reportingCore.getPluginSetupDeps(); // async background start (async () => { + await this.reportingCore.pluginSetsUp(); + const config = reportingCore.getConfig(); + const browserDriverFactory = await initializeBrowserDriverFactory(config, logger); - reportingCore.setBrowserDriverFactory(browserDriverFactory); - const esqueue = await createQueueFactory(reportingCore, logger); - const enqueueJob = enqueueJobFactory(reportingCore, logger); + const esqueue = await createQueueFactory(reportingCore, logger); // starts polling for pending jobs + const enqueueJob = enqueueJobFactory(reportingCore, logger); // called from generation routes reportingCore.pluginStart({ + browserDriverFactory, savedObjects: core.savedObjects, uiSettings: core.uiSettings, esqueue, @@ -92,7 +102,6 @@ export class ReportingPlugin runValidations(config, elasticsearch, browserDriverFactory, this.logger); this.logger.debug('Start complete'); - this.start$.next(true); })().catch((e) => { this.logger.error(`Error in Reporting start, reporting may not function properly`); this.logger.error(e); @@ -100,11 +109,4 @@ export class ReportingPlugin return {}; } - - public getReportingCore() { - if (!this.reportingCore) { - throw new Error('Setup is not ready'); - } - return this.reportingCore; - } } diff --git a/x-pack/plugins/reporting/server/routes/generation.test.ts b/x-pack/plugins/reporting/server/routes/generation.test.ts index f9b3e5446cfce..4474f2c95e1c3 100644 --- a/x-pack/plugins/reporting/server/routes/generation.test.ts +++ b/x-pack/plugins/reporting/server/routes/generation.test.ts @@ -18,6 +18,7 @@ import { of } from 'rxjs'; type setupServerReturn = UnwrapPromise>; describe('POST /api/reporting/generate', () => { + const reportingSymbol = Symbol('reporting'); let server: setupServerReturn['server']; let httpSetup: setupServerReturn['httpSetup']; let exportTypesRegistry: ExportTypesRegistry; @@ -47,7 +48,8 @@ describe('POST /api/reporting/generate', () => { } as unknown) as jest.Mocked; beforeEach(async () => { - ({ server, httpSetup } = await setupServer()); + ({ server, httpSetup } = await setupServer(reportingSymbol)); + httpSetup.registerRouteHandlerContext(reportingSymbol, 'reporting', () => ({})); const mockDeps = ({ elasticsearch: { legacy: { diff --git a/x-pack/plugins/reporting/server/routes/generation.ts b/x-pack/plugins/reporting/server/routes/generation.ts index ffcac9de6f4f1..69097c2080cdc 100644 --- a/x-pack/plugins/reporting/server/routes/generation.ts +++ b/x-pack/plugins/reporting/server/routes/generation.ts @@ -18,15 +18,21 @@ import { HandlerFunction } from './types'; const esErrors = elasticsearchErrors as Record; -export function registerJobGenerationRoutes(reporting: ReportingCore, logger: Logger) { +const getDownloadBaseUrl = (reporting: ReportingCore) => { const config = reporting.getConfig(); - const downloadBaseUrl = - config.kbnConfig.get('server', 'basePath') + `${API_BASE_URL}/jobs/download`; + return config.kbnConfig.get('server', 'basePath') + `${API_BASE_URL}/jobs/download`; +}; +export function registerJobGenerationRoutes(reporting: ReportingCore, logger: Logger) { /* * Generates enqueued job details to use in responses */ const handler: HandlerFunction = async (user, exportTypeId, jobParams, context, req, res) => { + // ensure the async dependencies are loaded + if (!context.reporting) { + return res.custom({ statusCode: 503, body: 'Not Available' }); + } + const licenseInfo = await reporting.getLicenseInfo(); const licenseResults = licenseInfo[exportTypeId]; @@ -43,6 +49,7 @@ export function registerJobGenerationRoutes(reporting: ReportingCore, logger: Lo // return the queue's job information const jobJson = job.toJSON(); + const downloadBaseUrl = getDownloadBaseUrl(reporting); return res.ok({ headers: { @@ -88,10 +95,6 @@ export function registerJobGenerationRoutes(reporting: ReportingCore, logger: Lo registerGenerateFromJobParams(reporting, handler, handleError); registerLegacy(reporting, handler, handleError, logger); // 7.x only - - // Register beta panel-action download-related API's - if (config.get('csv', 'enablePanelActionDownload')) { - registerGenerateCsvFromSavedObject(reporting, handler, handleError); - registerGenerateCsvFromSavedObjectImmediate(reporting, handleError, logger); - } + registerGenerateCsvFromSavedObject(reporting, handler, handleError); // FIXME: remove this https://github.com/elastic/kibana/issues/62986 + registerGenerateCsvFromSavedObjectImmediate(reporting, handleError, logger); } diff --git a/x-pack/plugins/reporting/server/routes/jobs.test.ts b/x-pack/plugins/reporting/server/routes/jobs.test.ts index 22d60d62d5fdb..35594474685b0 100644 --- a/x-pack/plugins/reporting/server/routes/jobs.test.ts +++ b/x-pack/plugins/reporting/server/routes/jobs.test.ts @@ -19,6 +19,7 @@ import { registerJobInfoRoutes } from './jobs'; type setupServerReturn = UnwrapPromise>; describe('GET /api/reporting/jobs/download', () => { + const reportingSymbol = Symbol('reporting'); let server: setupServerReturn['server']; let httpSetup: setupServerReturn['httpSetup']; let exportTypesRegistry: ExportTypesRegistry; @@ -39,7 +40,8 @@ describe('GET /api/reporting/jobs/download', () => { }; beforeEach(async () => { - ({ server, httpSetup } = await setupServer()); + ({ server, httpSetup } = await setupServer(reportingSymbol)); + httpSetup.registerRouteHandlerContext(reportingSymbol, 'reporting', () => ({})); core = await createMockReportingCore(config, ({ elasticsearch: { legacy: { client: { callAsInternalUser: jest.fn() } }, diff --git a/x-pack/plugins/reporting/server/routes/jobs.ts b/x-pack/plugins/reporting/server/routes/jobs.ts index 29cf55bc5c72e..90185f0736ed8 100644 --- a/x-pack/plugins/reporting/server/routes/jobs.ts +++ b/x-pack/plugins/reporting/server/routes/jobs.ts @@ -4,16 +4,16 @@ * you may not use this file except in compliance with the Elastic License. */ -import Boom from 'boom'; import { schema } from '@kbn/config-schema'; +import Boom from 'boom'; import { ReportingCore } from '../'; import { API_BASE_URL } from '../../common/constants'; import { jobsQueryFactory } from '../lib/jobs_query'; +import { authorizedUserPreRoutingFactory } from './lib/authorized_user_pre_routing'; import { deleteJobResponseHandlerFactory, downloadJobResponseHandlerFactory, } from './lib/job_response_handler'; -import { authorizedUserPreRoutingFactory } from './lib/authorized_user_pre_routing'; interface ListQuery { page: string; @@ -22,12 +22,14 @@ interface ListQuery { } const MAIN_ENTRY = `${API_BASE_URL}/jobs`; +const handleUnavailable = (res: any) => { + return res.custom({ statusCode: 503, body: 'Not Available' }); +}; + export function registerJobInfoRoutes(reporting: ReportingCore) { - const config = reporting.getConfig(); const setupDeps = reporting.getPluginSetupDeps(); const userHandler = authorizedUserPreRoutingFactory(reporting); - const { elasticsearch, router } = setupDeps; - const jobsQuery = jobsQueryFactory(config, elasticsearch); + const { router } = setupDeps; // list jobs in the queue, paginated router.get( @@ -36,6 +38,11 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { validate: false, }, userHandler(async (user, context, req, res) => { + // ensure the async dependencies are loaded + if (!context.reporting) { + return handleUnavailable(res); + } + const { management: { jobTypes = [] }, } = await reporting.getLicenseInfo(); @@ -47,6 +54,7 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { const page = parseInt(queryPage, 10) || 0; const size = Math.min(100, parseInt(querySize, 10) || 10); const jobIds = queryIds ? queryIds.split(',') : null; + const jobsQuery = jobsQueryFactory(reporting); const results = await jobsQuery.list(jobTypes, user, page, size, jobIds); return res.ok({ @@ -65,10 +73,16 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { validate: false, }, userHandler(async (user, context, req, res) => { + // ensure the async dependencies are loaded + if (!context.reporting) { + return handleUnavailable(res); + } + const { management: { jobTypes = [] }, } = await reporting.getLicenseInfo(); + const jobsQuery = jobsQueryFactory(reporting); const count = await jobsQuery.count(jobTypes, user); return res.ok({ @@ -91,11 +105,17 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { }, }, userHandler(async (user, context, req, res) => { + // ensure the async dependencies are loaded + if (!context.reporting) { + return handleUnavailable(res); + } + const { docId } = req.params as { docId: string }; const { management: { jobTypes = [] }, } = await reporting.getLicenseInfo(); + const jobsQuery = jobsQueryFactory(reporting); const result = await jobsQuery.get(user, docId, { includeContent: true }); if (!result) { @@ -130,11 +150,17 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { }, }, userHandler(async (user, context, req, res) => { + // ensure the async dependencies are loaded + if (!context.reporting) { + return res.custom({ statusCode: 503 }); + } + const { docId } = req.params as { docId: string }; const { management: { jobTypes = [] }, } = await reporting.getLicenseInfo(); + const jobsQuery = jobsQueryFactory(reporting); const result = await jobsQuery.get(user, docId); if (!result) { @@ -164,12 +190,7 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { ); // trigger a download of the output from a job - const exportTypesRegistry = reporting.getExportTypesRegistry(); - const downloadResponseHandler = downloadJobResponseHandlerFactory( - config, - elasticsearch, - exportTypesRegistry - ); + const downloadResponseHandler = downloadJobResponseHandlerFactory(reporting); router.get( { @@ -181,6 +202,11 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { }, }, userHandler(async (user, context, req, res) => { + // ensure the async dependencies are loaded + if (!context.reporting) { + return handleUnavailable(res); + } + const { docId } = req.params as { docId: string }; const { management: { jobTypes = [] }, @@ -191,7 +217,7 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { ); // allow a report to be deleted - const deleteResponseHandler = deleteJobResponseHandlerFactory(config, elasticsearch); + const deleteResponseHandler = deleteJobResponseHandlerFactory(reporting); router.delete( { path: `${MAIN_ENTRY}/delete/{docId}`, @@ -202,6 +228,11 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { }, }, userHandler(async (user, context, req, res) => { + // ensure the async dependencies are loaded + if (!context.reporting) { + return handleUnavailable(res); + } + const { docId } = req.params as { docId: string }; const { management: { jobTypes = [] }, diff --git a/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.ts b/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.ts index 2ad974c9dd8e1..2f5d4ebe1419a 100644 --- a/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.ts +++ b/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.ts @@ -19,7 +19,6 @@ export type RequestHandlerUser = RequestHandler extends (...a: infer U) => infer export const authorizedUserPreRoutingFactory = function authorizedUserPreRoutingFn( reporting: ReportingCore ) { - const config = reporting.getConfig(); const setupDeps = reporting.getPluginSetupDeps(); const getUser = getUserFactory(setupDeps.security); return (handler: RequestHandlerUser): RequestHandler => { @@ -36,6 +35,7 @@ export const authorizedUserPreRoutingFactory = function authorizedUserPreRouting if (user) { // check allowance with the configured set of roleas + "superuser" + const config = reporting.getConfig(); const allowedRoles = config.get('roles', 'allow') || []; const authorizedRoles = [superuserRole, ...allowedRoles]; diff --git a/x-pack/plugins/reporting/server/routes/lib/job_response_handler.ts b/x-pack/plugins/reporting/server/routes/lib/job_response_handler.ts index 1a2e10cf355a2..a8492481e6b13 100644 --- a/x-pack/plugins/reporting/server/routes/lib/job_response_handler.ts +++ b/x-pack/plugins/reporting/server/routes/lib/job_response_handler.ts @@ -4,11 +4,10 @@ * you may not use this file except in compliance with the Elastic License. */ -import { ElasticsearchServiceSetup, kibanaResponseFactory } from 'kibana/server'; +import { kibanaResponseFactory } from 'kibana/server'; +import { ReportingCore } from '../../'; import { AuthenticatedUser } from '../../../../security/server'; -import { ReportingConfig } from '../../'; import { WHITELISTED_JOB_CONTENT_TYPES } from '../../../common/constants'; -import { ExportTypesRegistry } from '../../lib/export_types_registry'; import { jobsQueryFactory } from '../../lib/jobs_query'; import { getDocumentPayloadFactory } from './get_document_payload'; @@ -20,12 +19,9 @@ interface JobResponseHandlerOpts { excludeContent?: boolean; } -export function downloadJobResponseHandlerFactory( - config: ReportingConfig, - elasticsearch: ElasticsearchServiceSetup, - exportTypesRegistry: ExportTypesRegistry -) { - const jobsQuery = jobsQueryFactory(config, elasticsearch); +export function downloadJobResponseHandlerFactory(reporting: ReportingCore) { + const jobsQuery = jobsQueryFactory(reporting); + const exportTypesRegistry = reporting.getExportTypesRegistry(); const getDocumentPayload = getDocumentPayloadFactory(exportTypesRegistry); return async function jobResponseHandler( @@ -69,11 +65,8 @@ export function downloadJobResponseHandlerFactory( }; } -export function deleteJobResponseHandlerFactory( - config: ReportingConfig, - elasticsearch: ElasticsearchServiceSetup -) { - const jobsQuery = jobsQueryFactory(config, elasticsearch); +export function deleteJobResponseHandlerFactory(reporting: ReportingCore) { + const jobsQuery = jobsQueryFactory(reporting); return async function deleteJobResponseHander( res: typeof kibanaResponseFactory, diff --git a/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts b/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts index 669381a92c522..579035a46f615 100644 --- a/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts +++ b/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts @@ -11,18 +11,15 @@ jest.mock('../lib/create_queue'); jest.mock('../lib/enqueue_job'); jest.mock('../lib/validate'); -import { of } from 'rxjs'; -import { first } from 'rxjs/operators'; -import { coreMock } from 'src/core/server/mocks'; +import * as Rx from 'rxjs'; import { ReportingConfig, ReportingCore } from '../'; import { chromium, HeadlessChromiumDriverFactory, initializeBrowserDriverFactory, } from '../browsers'; -import { ReportingInternalSetup } from '../core'; -import { ReportingPlugin } from '../plugin'; -import { ReportingSetupDeps, ReportingStartDeps } from '../types'; +import { ReportingInternalSetup, ReportingInternalStart } from '../core'; +import { ReportingStartDeps } from '../types'; (initializeBrowserDriverFactory as jest.Mock< Promise @@ -30,32 +27,30 @@ import { ReportingSetupDeps, ReportingStartDeps } from '../types'; (chromium as any).createDriverFactory.mockImplementation(() => ({})); -const createMockSetupDeps = (setupMock?: any): ReportingSetupDeps => { +const createMockPluginSetup = (setupMock?: any): ReportingInternalSetup => { return { + elasticsearch: setupMock.elasticsearch || { legacy: { client: {} } }, + basePath: setupMock.basePath, + router: setupMock.router, security: setupMock.security, - licensing: { - license$: of({ isAvailable: true, isActive: true, type: 'basic' }), - } as any, - usageCollection: { - makeUsageCollector: jest.fn(), - registerCollector: jest.fn(), - } as any, + licensing: { license$: Rx.of({ isAvailable: true, isActive: true, type: 'basic' }) } as any, + }; +}; + +const createMockPluginStart = (startMock?: any): ReportingInternalStart => { + return { + browserDriverFactory: startMock.browserDriverFactory, + enqueueJob: startMock.enqueueJob, + esqueue: startMock.esqueue, + savedObjects: startMock.savedObjects || { getScopedClient: jest.fn() }, + uiSettings: startMock.uiSettings || { asScopedToClient: () => ({ get: jest.fn() }) }, }; }; export const createMockConfigSchema = (overrides?: any) => ({ index: '.reporting', - kibanaServer: { - hostname: 'localhost', - port: '80', - }, - capture: { - browser: { - chromium: { - disableSandbox: true, - }, - }, - }, + kibanaServer: { hostname: 'localhost', port: '80' }, + capture: { browser: { chromium: { disableSandbox: true } } }, ...overrides, }); @@ -63,36 +58,20 @@ export const createMockStartDeps = (startMock?: any): ReportingStartDeps => ({ data: startMock.data, }); -const createMockReportingPlugin = async (config: ReportingConfig): Promise => { - const mockConfigSchema = createMockConfigSchema(config); - const plugin = new ReportingPlugin(coreMock.createPluginInitializerContext(mockConfigSchema)); - const setupMock = coreMock.createSetup(); - const coreStartMock = coreMock.createStart(); - const startMock = { - ...coreStartMock, - data: { fieldFormats: {} }, - }; - - plugin.setup(setupMock, createMockSetupDeps(setupMock)); - await plugin.setup$.pipe(first()).toPromise(); - plugin.start(startMock, createMockStartDeps(startMock)); - await plugin.start$.pipe(first()).toPromise(); - - return plugin; -}; - export const createMockReportingCore = async ( config: ReportingConfig, - setupDepsMock?: ReportingInternalSetup -): Promise => { + setupDepsMock: ReportingInternalSetup | undefined = createMockPluginSetup({}), + startDepsMock: ReportingInternalStart | undefined = createMockPluginStart({}) +) => { config = config || {}; - const plugin = await createMockReportingPlugin(config); - const core = plugin.getReportingCore(); + const core = new ReportingCore(); + + core.pluginSetup(setupDepsMock); + core.setConfig(config); + await core.pluginSetsUp(); - if (setupDepsMock) { - // @ts-ignore overwriting private properties - core.pluginSetupDeps = setupDepsMock; - } + core.pluginStart(startDepsMock); + await core.pluginStartsUp(); return core; }; diff --git a/x-pack/plugins/reporting/server/usage/reporting_usage_collector.test.ts b/x-pack/plugins/reporting/server/usage/reporting_usage_collector.test.ts index d5dccaca3042a..ed2abef2542de 100644 --- a/x-pack/plugins/reporting/server/usage/reporting_usage_collector.test.ts +++ b/x-pack/plugins/reporting/server/usage/reporting_usage_collector.test.ts @@ -7,7 +7,7 @@ import * as Rx from 'rxjs'; import sinon from 'sinon'; import { UsageCollectionSetup } from 'src/plugins/usage_collection/server'; -import { ReportingConfig } from '../'; +import { ReportingConfig, ReportingCore } from '../'; import { createMockReportingCore } from '../test_helpers'; import { getExportTypesRegistry } from '../lib/export_types_registry'; import { ReportingSetupDeps } from '../types'; @@ -62,8 +62,10 @@ const getResponseMock = (base = {}) => base; describe('license checks', () => { let mockConfig: ReportingConfig; + let mockCore: ReportingCore; beforeAll(async () => { mockConfig = getMockReportingConfig(); + mockCore = await createMockReportingCore(mockConfig); }); describe('with a basic license', () => { @@ -72,7 +74,7 @@ describe('license checks', () => { const plugins = getPluginsMock({ license: 'basic' }); const callClusterMock = jest.fn(() => Promise.resolve(getResponseMock())); const { fetch } = getReportingUsageCollector( - mockConfig, + mockCore, plugins.usageCollection, getLicenseMock('basic'), exportTypesRegistry, @@ -102,7 +104,7 @@ describe('license checks', () => { const plugins = getPluginsMock({ license: 'none' }); const callClusterMock = jest.fn(() => Promise.resolve(getResponseMock())); const { fetch } = getReportingUsageCollector( - mockConfig, + mockCore, plugins.usageCollection, getLicenseMock('none'), exportTypesRegistry, @@ -132,7 +134,7 @@ describe('license checks', () => { const plugins = getPluginsMock({ license: 'platinum' }); const callClusterMock = jest.fn(() => Promise.resolve(getResponseMock())); const { fetch } = getReportingUsageCollector( - mockConfig, + mockCore, plugins.usageCollection, getLicenseMock('platinum'), exportTypesRegistry, @@ -162,7 +164,7 @@ describe('license checks', () => { const plugins = getPluginsMock({ license: 'basic' }); const callClusterMock = jest.fn(() => Promise.resolve({})); const { fetch } = getReportingUsageCollector( - mockConfig, + mockCore, plugins.usageCollection, getLicenseMock('basic'), exportTypesRegistry, @@ -184,11 +186,16 @@ describe('license checks', () => { }); describe('data modeling', () => { + let mockConfig: ReportingConfig; + let mockCore: ReportingCore; + beforeAll(async () => { + mockConfig = getMockReportingConfig(); + mockCore = await createMockReportingCore(mockConfig); + }); test('with normal looking usage data', async () => { - const mockConfig = getMockReportingConfig(); const plugins = getPluginsMock(); const { fetch } = getReportingUsageCollector( - mockConfig, + mockCore, plugins.usageCollection, getLicenseMock(), exportTypesRegistry, @@ -238,10 +245,9 @@ describe('data modeling', () => { }); test('with sparse data', async () => { - const mockConfig = getMockReportingConfig(); const plugins = getPluginsMock(); const { fetch } = getReportingUsageCollector( - mockConfig, + mockCore, plugins.usageCollection, getLicenseMock(), exportTypesRegistry, @@ -291,10 +297,9 @@ describe('data modeling', () => { }); test('with empty data', async () => { - const mockConfig = getMockReportingConfig(); const plugins = getPluginsMock(); const { fetch } = getReportingUsageCollector( - mockConfig, + mockCore, plugins.usageCollection, getLicenseMock(), exportTypesRegistry, diff --git a/x-pack/plugins/reporting/server/usage/reporting_usage_collector.ts b/x-pack/plugins/reporting/server/usage/reporting_usage_collector.ts index d77d1b5396844..364f5187f056c 100644 --- a/x-pack/plugins/reporting/server/usage/reporting_usage_collector.ts +++ b/x-pack/plugins/reporting/server/usage/reporting_usage_collector.ts @@ -9,7 +9,6 @@ import { CallCluster } from 'src/legacy/core_plugins/elasticsearch'; import { UsageCollectionSetup } from 'src/plugins/usage_collection/server'; import { ReportingCore } from '../'; import { KIBANA_REPORTING_TYPE } from '../../common/constants'; -import { ReportingConfig } from '../../server'; import { ExportTypesRegistry } from '../lib/export_types_registry'; import { ReportingSetupDeps } from '../types'; import { GetLicense } from './'; @@ -23,7 +22,7 @@ const METATYPE = 'kibana_stats'; * @return {Object} kibana usage stats type collection object */ export function getReportingUsageCollector( - config: ReportingConfig, + reporting: ReportingCore, usageCollection: UsageCollectionSetup, getLicense: GetLicense, exportTypesRegistry: ExportTypesRegistry, @@ -31,8 +30,10 @@ export function getReportingUsageCollector( ) { return usageCollection.makeUsageCollector({ type: KIBANA_REPORTING_TYPE, - fetch: (callCluster: CallCluster) => - getReportingUsage(config, getLicense, callCluster, exportTypesRegistry), + fetch: (callCluster: CallCluster) => { + const config = reporting.getConfig(); + return getReportingUsage(config, getLicense, callCluster, exportTypesRegistry); + }, isReady, /* @@ -63,7 +64,6 @@ export function registerReportingUsageCollector( return; } - const config = reporting.getConfig(); const exportTypesRegistry = reporting.getExportTypesRegistry(); const getLicense = async () => { return await licensing.license$ @@ -78,10 +78,10 @@ export function registerReportingUsageCollector( ) .toPromise(); }; - const collectionIsReady = reporting.pluginHasStarted.bind(reporting); + const collectionIsReady = reporting.pluginStartsUp.bind(reporting); const collector = getReportingUsageCollector( - config, + reporting, usageCollection, getLicense, exportTypesRegistry,