From 8eef541d050f758a1902a2515fe1545d424935eb Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Thu, 11 Jun 2020 14:07:48 -0700 Subject: [PATCH 01/11] register routes synchronously --- x-pack/plugins/reporting/server/core.ts | 108 +++++++++--------- .../csv/server/execute_job.test.ts | 25 ++-- .../export_types/csv/server/execute_job.ts | 2 +- .../server/lib/generate_csv_search.ts | 2 +- .../reporting/server/lib/jobs_query.ts | 12 +- .../plugins/reporting/server/plugin.test.ts | 1 + x-pack/plugins/reporting/server/plugin.ts | 59 ++++------ .../reporting/server/routes/generation.ts | 16 ++- .../plugins/reporting/server/routes/jobs.ts | 29 +++-- .../routes/lib/authorized_user_pre_routing.ts | 2 +- .../server/routes/lib/job_response_handler.ts | 21 ++-- .../create_mock_reportingplugin.ts | 80 +++++-------- .../usage/reporting_usage_collector.test.ts | 27 +++-- .../server/usage/reporting_usage_collector.ts | 14 +-- 14 files changed, 179 insertions(+), 219 deletions(-) diff --git a/x-pack/plugins/reporting/server/core.ts b/x-pack/plugins/reporting/server/core.ts index 94b138ffcae0b..a9154cefb5fb7 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,65 +44,36 @@ 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(); // for pluginHasSetup + private readonly pluginStart$ = new Rx.ReplaySubject(); // for getPluginStartDeps private exportTypesRegistry = getExportTypesRegistry(); + private config?: ReportingConfig; - constructor(private config: ReportingConfig) {} + constructor() {} public pluginSetup(reportingSetupDeps: ReportingInternalSetup) { this.pluginSetupDeps = reportingSetupDeps; - this.pluginSetup$.next(reportingSetupDeps); + this.pluginSetup$.next(true); } public pluginStart(reportingStartDeps: ReportingInternalStart) { this.pluginStart$.next(reportingStartDeps); } - public pluginHasStarted(): Promise { - return this.pluginStart$.pipe(first(), mapTo(true)).toPromise(); - } - - public setBrowserDriverFactory(browserDriverFactory: HeadlessChromiumDriverFactory) { - this.browserDriverFactory = browserDriverFactory; + public setConfig(config: ReportingConfig) { + this.config = config; + this.pluginSetup$.next(true); } /* - * Internal module dependencies + * High-level module dependencies */ - public getExportTypesRegistry() { - return this.exportTypesRegistry; - } - - public async getEsqueue() { - return (await this.getPluginStartDeps()).esqueue; - } - - public async getEnqueueJob() { - return (await this.getPluginStartDeps()).enqueueJob; - } - - public async getLicenseInfo() { - const { licensing } = this.getPluginSetupDeps(); - return await licensing.license$ - .pipe( - map((license) => checkLicense(this.getExportTypesRegistry(), license)), - first() - ) - .toPromise(); - } public getConfig(): ReportingConfig { - return this.config; - } - - public getScreenshotsObservable(): ScreenshotsObservableFn { - const { browserDriverFactory } = this; - if (!browserDriverFactory) { - throw new Error(`"browserDriverFactory" dependency hasn't initialized yet`); + if (!this.config) { + throw new Error('Config is not yet initialized'); } - return screenshotsObservableFactory(this.config.get('capture'), browserDriverFactory); + return this.config; } public getPluginSetupDeps() { @@ -111,10 +83,6 @@ export class ReportingCore { return this.pluginSetupDeps; } - /* - * Outside dependencies - */ - private async getPluginStartDeps() { if (this.pluginStartDeps) { return this.pluginStartDeps; @@ -122,8 +90,24 @@ export class ReportingCore { return await this.pluginStart$.pipe(first()).toPromise(); } - public async getElasticsearchService() { - return this.getPluginSetupDeps().elasticsearch; + public async pluginIsSetup(): 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 async pluginIsStarted(): Promise { + return await this.getPluginStartDeps().then(() => true); + } + + /* + * Downstream dependencies + */ + + public getExportTypesRegistry() { + return this.exportTypesRegistry; } public async getSavedObjectsClient(fakeRequest: KibanaRequest) { @@ -131,9 +115,27 @@ export class ReportingCore { return savedObjects.getScopedClient(fakeRequest) as SavedObjectsClientContract; } - public async getUiSettingsServiceFactory(savedObjectsClient: SavedObjectsClientContract) { - const { uiSettings: uiSettingsService } = await this.getPluginStartDeps(); - const scopedUiSettingsService = uiSettingsService.asScopedToClient(savedObjectsClient); - return scopedUiSettingsService; + public async getEsqueue() { + return (await this.getPluginStartDeps()).esqueue; + } + + public async getEnqueueJob() { + return (await this.getPluginStartDeps()).enqueueJob; + } + + public async getLicenseInfo() { + const { licensing } = this.getPluginSetupDeps(); + return await licensing.license$ + .pipe( + map((license) => checkLicense(this.getExportTypesRegistry(), license)), + first() + ) + .toPromise(); + } + + public async getScreenshotsObservable(): Promise { + const config = this.getConfig(); + const { browserDriverFactory } = await this.getPluginStartDeps(); + return screenshotsObservableFactory(config.get('capture'), browserDriverFactory); } } 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 ddcf94079ade4..4ce448e953bd1 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(); @@ -995,7 +999,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/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..b1e6931bf59e7 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$: { diff --git a/x-pack/plugins/reporting/server/plugin.ts b/x-pack/plugins/reporting/server/plugin.ts index a3c89c7b8a8ce..a65c8ebfd95a8 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'; @@ -19,43 +18,38 @@ 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) { 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 +62,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.pluginIsSetup(); + 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 +87,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 +94,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.ts b/x-pack/plugins/reporting/server/routes/generation.ts index f2e616c0803a7..77498ae5b9e13 100644 --- a/x-pack/plugins/reporting/server/routes/generation.ts +++ b/x-pack/plugins/reporting/server/routes/generation.ts @@ -17,11 +17,12 @@ 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 */ @@ -42,6 +43,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: { @@ -86,10 +88,6 @@ export function registerJobGenerationRoutes(reporting: ReportingCore, logger: Lo } registerGenerateFromJobParams(reporting, handler, handleError); - - // 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.ts b/x-pack/plugins/reporting/server/routes/jobs.ts index 29cf55bc5c72e..fd8e0dc9c6bb0 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; @@ -23,11 +23,9 @@ interface ListQuery { const MAIN_ENTRY = `${API_BASE_URL}/jobs`; 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( @@ -47,7 +45,8 @@ 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 results = await jobsQuery.list(jobTypes, user, page, size, jobIds); + const queryProvider = jobsQueryFactory(reporting); + const results = await queryProvider.list(jobTypes, user, page, size, jobIds); return res.ok({ body: results, @@ -69,7 +68,8 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { management: { jobTypes = [] }, } = await reporting.getLicenseInfo(); - const count = await jobsQuery.count(jobTypes, user); + const queryProvider = jobsQueryFactory(reporting); + const count = await queryProvider.count(jobTypes, user); return res.ok({ body: count.toString(), @@ -96,7 +96,8 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { management: { jobTypes = [] }, } = await reporting.getLicenseInfo(); - const result = await jobsQuery.get(user, docId, { includeContent: true }); + const queryProvider = jobsQueryFactory(reporting); + const result = await queryProvider.get(user, docId, { includeContent: true }); if (!result) { throw Boom.notFound(); @@ -135,7 +136,8 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { management: { jobTypes = [] }, } = await reporting.getLicenseInfo(); - const result = await jobsQuery.get(user, docId); + const queryProvider = jobsQueryFactory(reporting); + const result = await queryProvider.get(user, docId); if (!result) { throw Boom.notFound(); @@ -164,12 +166,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( { @@ -191,7 +188,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}`, 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..b0af426aa688d 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,15 @@ 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(); - - if (setupDepsMock) { - // @ts-ignore overwriting private properties - core.pluginSetupDeps = setupDepsMock; - } - + const core = new ReportingCore(); + core.pluginSetup(setupDepsMock); + core.setConfig(config); + core.pluginStart(startDepsMock); 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..b3d482491f037 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.pluginIsStarted.bind(reporting); const collector = getReportingUsageCollector( - config, + reporting, usageCollection, getLicense, exportTypesRegistry, From 26abb7d2c2785f351d30b1ae8f4cd591740af385 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Thu, 11 Jun 2020 20:07:12 -0700 Subject: [PATCH 02/11] back out some refactoring --- x-pack/plugins/reporting/server/core.ts | 80 ++++++++++--------- .../plugins/reporting/server/routes/jobs.ts | 16 ++-- .../server/usage/reporting_usage_collector.ts | 2 +- 3 files changed, 50 insertions(+), 48 deletions(-) diff --git a/x-pack/plugins/reporting/server/core.ts b/x-pack/plugins/reporting/server/core.ts index a9154cefb5fb7..6059bed9c6798 100644 --- a/x-pack/plugins/reporting/server/core.ts +++ b/x-pack/plugins/reporting/server/core.ts @@ -60,36 +60,6 @@ export class ReportingCore { this.pluginStart$.next(reportingStartDeps); } - public setConfig(config: ReportingConfig) { - this.config = config; - this.pluginSetup$.next(true); - } - - /* - * High-level module dependencies - */ - - public getConfig(): ReportingConfig { - if (!this.config) { - throw new Error('Config is not yet initialized'); - } - return this.config; - } - - public getPluginSetupDeps() { - if (!this.pluginSetupDeps) { - throw new Error(`"pluginSetupDeps" dependencies haven't initialized yet`); - } - return this.pluginSetupDeps; - } - - private async getPluginStartDeps() { - if (this.pluginStartDeps) { - return this.pluginStartDeps; - } - return await this.pluginStart$.pipe(first()).toPromise(); - } - public async pluginIsSetup(): Promise { // use deps and config as a cached resolver if (this.pluginSetupDeps && this.config) { @@ -98,23 +68,19 @@ export class ReportingCore { return await this.pluginSetup$.pipe(take(2)).toPromise(); // once for pluginSetupDeps (sync) and twice for config (async) } - public async pluginIsStarted(): Promise { + public async pluginHasStarted(): Promise { return await this.getPluginStartDeps().then(() => true); } - /* - * Downstream dependencies - */ + public setConfig(config: ReportingConfig) { + this.config = config; + this.pluginSetup$.next(true); + } public getExportTypesRegistry() { return this.exportTypesRegistry; } - public async getSavedObjectsClient(fakeRequest: KibanaRequest) { - const { savedObjects } = await this.getPluginStartDeps(); - return savedObjects.getScopedClient(fakeRequest) as SavedObjectsClientContract; - } - public async getEsqueue() { return (await this.getPluginStartDeps()).esqueue; } @@ -133,9 +99,45 @@ export class ReportingCore { .toPromise(); } + public getConfig(): ReportingConfig { + if (!this.config) { + throw new Error('Config is not yet initialized'); + } + return this.config; + } + public async getScreenshotsObservable(): Promise { const config = this.getConfig(); const { browserDriverFactory } = await this.getPluginStartDeps(); return screenshotsObservableFactory(config.get('capture'), browserDriverFactory); } + + public getPluginSetupDeps() { + if (!this.pluginSetupDeps) { + throw new Error(`"pluginSetupDeps" dependencies haven't initialized yet`); + } + return this.pluginSetupDeps; + } + + private async getPluginStartDeps() { + if (this.pluginStartDeps) { + return this.pluginStartDeps; + } + return await this.pluginStart$.pipe(first()).toPromise(); + } + + public getElasticsearchService() { + return this.getPluginSetupDeps().elasticsearch; + } + + public async getSavedObjectsClient(fakeRequest: KibanaRequest) { + const { savedObjects } = await this.getPluginStartDeps(); + return savedObjects.getScopedClient(fakeRequest) as SavedObjectsClientContract; + } + + public async getUiSettingsServiceFactory(savedObjectsClient: SavedObjectsClientContract) { + const { uiSettings: uiSettingsService } = await this.getPluginStartDeps(); + const scopedUiSettingsService = uiSettingsService.asScopedToClient(savedObjectsClient); + return scopedUiSettingsService; + } } diff --git a/x-pack/plugins/reporting/server/routes/jobs.ts b/x-pack/plugins/reporting/server/routes/jobs.ts index fd8e0dc9c6bb0..582166b033f66 100644 --- a/x-pack/plugins/reporting/server/routes/jobs.ts +++ b/x-pack/plugins/reporting/server/routes/jobs.ts @@ -45,8 +45,8 @@ 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 queryProvider = jobsQueryFactory(reporting); - const results = await queryProvider.list(jobTypes, user, page, size, jobIds); + const jobsQuery = jobsQueryFactory(reporting); + const results = await jobsQuery.list(jobTypes, user, page, size, jobIds); return res.ok({ body: results, @@ -68,8 +68,8 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { management: { jobTypes = [] }, } = await reporting.getLicenseInfo(); - const queryProvider = jobsQueryFactory(reporting); - const count = await queryProvider.count(jobTypes, user); + const jobsQuery = jobsQueryFactory(reporting); + const count = await jobsQuery.count(jobTypes, user); return res.ok({ body: count.toString(), @@ -96,8 +96,8 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { management: { jobTypes = [] }, } = await reporting.getLicenseInfo(); - const queryProvider = jobsQueryFactory(reporting); - const result = await queryProvider.get(user, docId, { includeContent: true }); + const jobsQuery = jobsQueryFactory(reporting); + const result = await jobsQuery.get(user, docId, { includeContent: true }); if (!result) { throw Boom.notFound(); @@ -136,8 +136,8 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { management: { jobTypes = [] }, } = await reporting.getLicenseInfo(); - const queryProvider = jobsQueryFactory(reporting); - const result = await queryProvider.get(user, docId); + const jobsQuery = jobsQueryFactory(reporting); + const result = await jobsQuery.get(user, docId); if (!result) { throw Boom.notFound(); 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 b3d482491f037..8c73e8119ac61 100644 --- a/x-pack/plugins/reporting/server/usage/reporting_usage_collector.ts +++ b/x-pack/plugins/reporting/server/usage/reporting_usage_collector.ts @@ -78,7 +78,7 @@ export function registerReportingUsageCollector( ) .toPromise(); }; - const collectionIsReady = reporting.pluginIsStarted.bind(reporting); + const collectionIsReady = reporting.pluginHasStarted.bind(reporting); const collector = getReportingUsageCollector( reporting, From 8f381a797fe7b35e3e57c9e44647484e98dac096 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Fri, 12 Jun 2020 10:00:23 -0700 Subject: [PATCH 03/11] comment fix --- x-pack/plugins/reporting/server/core.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/reporting/server/core.ts b/x-pack/plugins/reporting/server/core.ts index 6059bed9c6798..892fecf9d30d3 100644 --- a/x-pack/plugins/reporting/server/core.ts +++ b/x-pack/plugins/reporting/server/core.ts @@ -44,7 +44,7 @@ export interface ReportingInternalStart { export class ReportingCore { private pluginSetupDeps?: ReportingInternalSetup; private pluginStartDeps?: ReportingInternalStart; - private readonly pluginSetup$ = new Rx.ReplaySubject(); // for pluginHasSetup + private readonly pluginSetup$ = new Rx.ReplaySubject(); // for pluginIsSetup private readonly pluginStart$ = new Rx.ReplaySubject(); // for getPluginStartDeps private exportTypesRegistry = getExportTypesRegistry(); private config?: ReportingConfig; From ed011c095ea71b439df5cf4dab226854eded4ddd Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Fri, 12 Jun 2020 10:57:45 -0700 Subject: [PATCH 04/11] fix tests --- .../server/execute_job/index.test.ts | 36 ++----------------- .../plugins/reporting/server/plugin.test.ts | 6 ++-- 2 files changed, 6 insertions(+), 36 deletions(-) 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 b8e1e5eebd9e7..2f4ca47cf739e 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/plugin.test.ts b/x-pack/plugins/reporting/server/plugin.test.ts index b1e6931bf59e7..b2eeac96e1129 100644 --- a/x-pack/plugins/reporting/server/plugin.test.ts +++ b/x-pack/plugins/reporting/server/plugin.test.ts @@ -31,8 +31,8 @@ describe('Reporting Plugin', () => { beforeEach(async () => { configSchema = createMockConfigSchema(); initContext = coreMock.createPluginInitializerContext(configSchema); - coreSetup = await coreMock.createSetup(configSchema); - coreStart = await coreMock.createStart(); + coreSetup = coreMock.createSetup(configSchema); + coreStart = coreMock.createStart(); pluginSetup = ({ licensing: {}, usageCollection: { @@ -63,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); From 1136503a4a41801dfab7672f6cb71d3a20715987 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Fri, 12 Jun 2020 15:30:58 -0700 Subject: [PATCH 05/11] register route handler context provider --- x-pack/plugins/reporting/server/core.ts | 8 +++-- x-pack/plugins/reporting/server/plugin.ts | 16 ++++++++- .../reporting/server/routes/generation.ts | 5 +++ .../plugins/reporting/server/routes/jobs.ts | 34 +++++++++++++++++++ .../server/usage/reporting_usage_collector.ts | 2 +- 5 files changed, 61 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/reporting/server/core.ts b/x-pack/plugins/reporting/server/core.ts index 892fecf9d30d3..e032bb01497ec 100644 --- a/x-pack/plugins/reporting/server/core.ts +++ b/x-pack/plugins/reporting/server/core.ts @@ -60,7 +60,7 @@ export class ReportingCore { this.pluginStart$.next(reportingStartDeps); } - public async pluginIsSetup(): Promise { + public async pluginSetsUp(): Promise { // use deps and config as a cached resolver if (this.pluginSetupDeps && this.config) { return true; @@ -68,10 +68,14 @@ export class ReportingCore { return await this.pluginSetup$.pipe(take(2)).toPromise(); // once for pluginSetupDeps (sync) and twice for config (async) } - public async pluginHasStarted(): Promise { + public async pluginStartsUp(): Promise { return await this.getPluginStartDeps().then(() => true); } + public pluginIsStarted() { + return this.pluginStartDeps !== undefined; + } + public setConfig(config: ReportingConfig) { this.config = config; this.pluginSetup$.next(true); diff --git a/x-pack/plugins/reporting/server/plugin.ts b/x-pack/plugins/reporting/server/plugin.ts index a65c8ebfd95a8..c44c97541f23e 100644 --- a/x-pack/plugins/reporting/server/plugin.ts +++ b/x-pack/plugins/reporting/server/plugin.ts @@ -14,6 +14,12 @@ import { setFieldFormats } from './services'; import { ReportingSetup, ReportingSetupDeps, ReportingStart, ReportingStartDeps } from './types'; import { registerReportingUsageCollector } from './usage'; +declare module 'src/core/server' { + interface RequestHandlerContext { + reporting?: ReportingCore | null; + } +} + export class ReportingPlugin implements Plugin { private readonly initializerContext: PluginInitializerContext; @@ -27,6 +33,14 @@ export class ReportingPlugin } public setup(core: CoreSetup, plugins: ReportingSetupDeps) { + core.http.registerRouteHandlerContext('reporting', () => { + if (this.reportingCore.pluginIsStarted()) { + return this.reportingCore; + } else { + return null; + } + }); + const { elasticsearch, http } = core; const { licensing, security } = plugins; const { initializerContext: initContext, reportingCore } = this; @@ -67,7 +81,7 @@ export class ReportingPlugin // async background start (async () => { - await this.reportingCore.pluginIsSetup(); + await this.reportingCore.pluginSetsUp(); const config = reportingCore.getConfig(); const browserDriverFactory = await initializeBrowserDriverFactory(config, logger); diff --git a/x-pack/plugins/reporting/server/routes/generation.ts b/x-pack/plugins/reporting/server/routes/generation.ts index 77498ae5b9e13..b4c81e698ce71 100644 --- a/x-pack/plugins/reporting/server/routes/generation.ts +++ b/x-pack/plugins/reporting/server/routes/generation.ts @@ -27,6 +27,11 @@ export function registerJobGenerationRoutes(reporting: ReportingCore, logger: Lo * 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]; diff --git a/x-pack/plugins/reporting/server/routes/jobs.ts b/x-pack/plugins/reporting/server/routes/jobs.ts index 582166b033f66..90185f0736ed8 100644 --- a/x-pack/plugins/reporting/server/routes/jobs.ts +++ b/x-pack/plugins/reporting/server/routes/jobs.ts @@ -22,6 +22,10 @@ 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 setupDeps = reporting.getPluginSetupDeps(); const userHandler = authorizedUserPreRoutingFactory(reporting); @@ -34,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(); @@ -64,6 +73,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(); @@ -91,6 +105,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 = [] }, @@ -131,6 +150,11 @@ 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 = [] }, @@ -178,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 = [] }, @@ -199,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/usage/reporting_usage_collector.ts b/x-pack/plugins/reporting/server/usage/reporting_usage_collector.ts index 8c73e8119ac61..364f5187f056c 100644 --- a/x-pack/plugins/reporting/server/usage/reporting_usage_collector.ts +++ b/x-pack/plugins/reporting/server/usage/reporting_usage_collector.ts @@ -78,7 +78,7 @@ export function registerReportingUsageCollector( ) .toPromise(); }; - const collectionIsReady = reporting.pluginHasStarted.bind(reporting); + const collectionIsReady = reporting.pluginStartsUp.bind(reporting); const collector = getReportingUsageCollector( reporting, From 7d75238d527bc474d65b24374ab098fa6f4f67b4 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Fri, 12 Jun 2020 16:14:41 -0700 Subject: [PATCH 06/11] Add function level comments in core methods --- x-pack/plugins/reporting/server/core.ts | 73 +++++++++++++++++-------- 1 file changed, 51 insertions(+), 22 deletions(-) diff --git a/x-pack/plugins/reporting/server/core.ts b/x-pack/plugins/reporting/server/core.ts index e032bb01497ec..9acd359fa0db4 100644 --- a/x-pack/plugins/reporting/server/core.ts +++ b/x-pack/plugins/reporting/server/core.ts @@ -44,22 +44,32 @@ export interface ReportingInternalStart { export class ReportingCore { private pluginSetupDeps?: ReportingInternalSetup; private pluginStartDeps?: ReportingInternalStart; - private readonly pluginSetup$ = new Rx.ReplaySubject(); // for pluginIsSetup - private readonly pluginStart$ = new Rx.ReplaySubject(); // for getPluginStartDeps + 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() {} - public pluginSetup(reportingSetupDeps: ReportingInternalSetup) { - this.pluginSetupDeps = reportingSetupDeps; - this.pluginSetup$.next(true); + /* + * 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 } + /* + * Blocks the caller until setup is done + */ public async pluginSetsUp(): Promise { // use deps and config as a cached resolver if (this.pluginSetupDeps && this.config) { @@ -68,19 +78,49 @@ export class ReportingCore { return await this.pluginSetup$.pipe(take(2)).toPromise(); // once for pluginSetupDeps (sync) and twice for config (async) } + /* + * 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.pluginStartDeps !== undefined; + return this.pluginSetupDeps != null && this.config != null && this.pluginStartDeps != null; } + /* + * 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; } @@ -103,19 +143,15 @@ export class ReportingCore { .toPromise(); } - public getConfig(): ReportingConfig { - if (!this.config) { - throw new Error('Config is not yet initialized'); - } - return this.config; - } - 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`); @@ -123,13 +159,6 @@ export class ReportingCore { return this.pluginSetupDeps; } - private async getPluginStartDeps() { - if (this.pluginStartDeps) { - return this.pluginStartDeps; - } - return await this.pluginStart$.pipe(first()).toPromise(); - } - public getElasticsearchService() { return this.getPluginSetupDeps().elasticsearch; } From 30d12fe5cd6c0639fd927f14ca74117c33caf75e Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Fri, 12 Jun 2020 16:26:28 -0700 Subject: [PATCH 07/11] fix tests --- .../server/test_helpers/create_mock_reportingplugin.ts | 5 +++++ 1 file changed, 5 insertions(+) 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 b0af426aa688d..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 @@ -65,8 +65,13 @@ export const createMockReportingCore = async ( ) => { config = config || {}; const core = new ReportingCore(); + core.pluginSetup(setupDepsMock); core.setConfig(config); + await core.pluginSetsUp(); + core.pluginStart(startDepsMock); + await core.pluginStartsUp(); + return core; }; From 3142b18d27ec250e446f6c6413a61adafa5847be Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Fri, 12 Jun 2020 16:54:27 -0700 Subject: [PATCH 08/11] revert editor help --- x-pack/plugins/reporting/server/plugin.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/reporting/server/plugin.test.ts b/x-pack/plugins/reporting/server/plugin.test.ts index b2eeac96e1129..420fa8347cdeb 100644 --- a/x-pack/plugins/reporting/server/plugin.test.ts +++ b/x-pack/plugins/reporting/server/plugin.test.ts @@ -31,8 +31,8 @@ describe('Reporting Plugin', () => { beforeEach(async () => { configSchema = createMockConfigSchema(); initContext = coreMock.createPluginInitializerContext(configSchema); - coreSetup = coreMock.createSetup(configSchema); - coreStart = coreMock.createStart(); + coreSetup = await coreMock.createSetup(configSchema); + coreStart = await coreMock.createStart(); pluginSetup = ({ licensing: {}, usageCollection: { From a8ff18ca0edad510a24ddf1f1fb8473e337634a7 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Fri, 12 Jun 2020 16:55:04 -0700 Subject: [PATCH 09/11] route context is the ReportingStart contract --- x-pack/plugins/reporting/server/plugin.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/reporting/server/plugin.ts b/x-pack/plugins/reporting/server/plugin.ts index c44c97541f23e..693b0917603fc 100644 --- a/x-pack/plugins/reporting/server/plugin.ts +++ b/x-pack/plugins/reporting/server/plugin.ts @@ -16,7 +16,7 @@ import { registerReportingUsageCollector } from './usage'; declare module 'src/core/server' { interface RequestHandlerContext { - reporting?: ReportingCore | null; + reporting?: ReportingStart | null; } } @@ -33,9 +33,10 @@ export class ReportingPlugin } 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 this.reportingCore; + return {}; // ReportingStart contract } else { return null; } From 1e078fc904f0abe72631642074be110988d88a0f Mon Sep 17 00:00:00 2001 From: Joel Griffith Date: Mon, 15 Jun 2020 10:25:13 -0700 Subject: [PATCH 10/11] Fix reporting job route tests --- x-pack/plugins/reporting/server/routes/jobs.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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() } }, From a7bae6880ddfc320f221afbce69903e06aab7e3f Mon Sep 17 00:00:00 2001 From: Joel Griffith Date: Mon, 15 Jun 2020 10:31:17 -0700 Subject: [PATCH 11/11] Fix generation tests --- x-pack/plugins/reporting/server/routes/generation.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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: {