clean up setup deps made available from plugin.ts

x-pack/plugins/reporting/server/core.ts

@@ -26,6 +26,7 @@ import { DEFAULT_SPACE_ID } from '../../spaces/common/constants';
 import { SpacesPluginSetup } from '../../spaces/server';
 import { TaskManagerSetupContract, TaskManagerStartContract } from '../../task_manager/server';
 import { ReportingConfig, ReportingSetup } from './';
+import { initializeBrowserDriverFactory } from './browsers';
 import { HeadlessChromiumDriverFactory } from './browsers/chromium/driver_factory';
 import { ReportingConfigType } from './config';
 import { checkLicense, getExportTypesRegistry, LevelLogger } from './lib';
@@ -33,7 +34,7 @@ import { ReportingStore } from './lib/store';
 import { ExecuteReportTask, MonitorReportsTask, ReportTaskParams } from './lib/tasks';
 import { ReportingPluginRouter } from './types';
 
-export interface ReportingInternalSetup {
+export interface ReportingPluginSetupDeps {
   basePath: Pick<BasePath, 'set'>;
   router: ReportingPluginRouter;
   features: FeaturesPluginSetup;
@@ -45,9 +46,7 @@ export interface ReportingInternalSetup {
   logger: LevelLogger;
 }
 
-export interface ReportingInternalStart {
-  browserDriverFactory: HeadlessChromiumDriverFactory;
-  store: ReportingStore;
+export interface ReportingPluginStartDeps {
   savedObjects: SavedObjectsServiceStart;
   uiSettings: UiSettingsServiceStart;
   esClient: IClusterClient;
@@ -56,11 +55,16 @@ export interface ReportingInternalStart {
   logger: LevelLogger;
 }
 
+type ReportingInternalPluginStartDeps = ReportingPluginStartDeps & {
+  browserDriverFactory: HeadlessChromiumDriverFactory;
+  store: ReportingStore;
+};
+
 export class ReportingCore {
-  private pluginSetupDeps?: ReportingInternalSetup;
-  private pluginStartDeps?: ReportingInternalStart;
+  private pluginSetupDeps?: ReportingPluginSetupDeps;
+  private pluginStartDeps?: ReportingInternalPluginStartDeps;
   private readonly pluginSetup$ = new Rx.ReplaySubject<boolean>(); // observe async background setupDeps and config each are done
-  private readonly pluginStart$ = new Rx.ReplaySubject<ReportingInternalStart>(); // observe async background startDeps
+  private readonly pluginStart$ = new Rx.ReplaySubject<ReportingInternalPluginStartDeps>(); // observe async background startDeps
   private deprecatedAllowedRoles: string[] | false = false; // DEPRECATED. If `false`, the deprecated features have been disableed
   private exportTypesRegistry = getExportTypesRegistry();
   private executeTask: ExecuteReportTask;
@@ -86,7 +90,7 @@ export class ReportingCore {
   /*
    * Register setupDeps
    */
-  public pluginSetup(setupDeps: ReportingInternalSetup) {
+  public pluginSetup(setupDeps: ReportingPluginSetupDeps) {
     this.pluginSetup$.next(true); // trigger the observer
     this.pluginSetupDeps = setupDeps; // cache
 
@@ -100,10 +104,18 @@ export class ReportingCore {
   /*
    * Register startDeps
    */
-  public async pluginStart(startDeps: ReportingInternalStart) {
+  public async pluginStart(deps: ReportingPluginStartDeps) {
+    const browserDriverFactory = await initializeBrowserDriverFactory(this, this.logger);
+    const store = new ReportingStore(this, this.logger);
+
+    const startDeps = { browserDriverFactory, store, ...deps };
+
     this.pluginStart$.next(startDeps); // trigger the observer
     this.pluginStartDeps = startDeps; // cache
 
+    // must be called after start deps observer is fulfilled
+    await store.start();
+
     const { taskManager } = startDeps;
     const { executeTask, monitorTask } = this;
     // enable this instance to generate reports and to monitor for pending reports

x-pack/plugins/reporting/server/lib/enqueue_job.test.ts

@@ -8,7 +8,7 @@
 import { KibanaRequest } from 'src/core/server';
 import { ReportingCore } from '../';
 import { TaskManagerStartContract } from '../../../task_manager/server';
-import { ReportingInternalStart } from '../core';
+import { ReportingPluginStartDeps } from '../core';
 import {
   createMockConfigSchema,
   createMockLevelLogger,
@@ -59,7 +59,7 @@ describe('Enqueue Job', () => {
         ensureScheduled: jest.fn(),
         schedule: scheduleMock,
       } as unknown) as TaskManagerStartContract,
-    } as unknown) as ReportingInternalStart);
+    } as unknown) as ReportingPluginStartDeps);
   });
 
   it('returns a Report object', async () => {

x-pack/plugins/reporting/server/plugin.ts

@@ -8,10 +8,9 @@
 import type { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from 'src/core/server';
 import { PLUGIN_ID } from '../common/constants';
 import { ReportingCore } from './';
-import { initializeBrowserDriverFactory } from './browsers';
 import { buildConfig, registerUiSettings, ReportingConfigType } from './config';
 import { registerDeprecations } from './deprecations';
-import { LevelLogger, ReportingStore } from './lib';
+import { LevelLogger } from './lib';
 import { registerRoutes } from './routes';
 import { setFieldFormats } from './services';
 import type {
@@ -97,23 +96,15 @@ export class ReportingPlugin
     (async () => {
       await reportingCore.pluginSetsUp();
 
-      const browserDriverFactory = await initializeBrowserDriverFactory(reportingCore, this.logger);
-      const store = new ReportingStore(reportingCore, this.logger);
-
       await reportingCore.pluginStart({
-        browserDriverFactory,
         savedObjects: core.savedObjects,
         uiSettings: core.uiSettings,
-        store,
         esClient: core.elasticsearch.client,
         data: plugins.data,
         taskManager: plugins.taskManager,
         logger: this.logger,
       });
 
-      // Note: this must be called after ReportingCore.pluginStart
-      await store.start();
-
       this.logger.debug('Start complete');
     })().catch((e) => {
       this.logger.error(`Error in Reporting start, reporting may not function properly`);

x-pack/plugins/reporting/server/routes/jobs.test.ts

@@ -12,7 +12,7 @@ import { ElasticsearchClient } from 'kibana/server';
 import { setupServer } from 'src/core/server/test_utils';
 import supertest from 'supertest';
 import { ReportingCore } from '..';
-import { ReportingInternalSetup } from '../core';
+import { ReportingPluginSetupDeps } from '../core';
 import { ExportTypesRegistry } from '../lib/export_types_registry';
 import {
   createMockConfigSchema,
@@ -30,7 +30,7 @@ describe('GET /api/reporting/jobs/download', () => {
   let httpSetup: SetupServerReturn['httpSetup'];
   let exportTypesRegistry: ExportTypesRegistry;
   let core: ReportingCore;
-  let mockSetupDeps: ReportingInternalSetup;
+  let mockSetupDeps: ReportingPluginSetupDeps;
   let mockEsClient: DeeplyMockedKeys<ElasticsearchClient>;
 
   const getHits = (...sources: any) => {
@@ -128,7 +128,7 @@ describe('GET /api/reporting/jobs/download', () => {
           getCurrentUser: () => undefined,
         },
       },
-    } as unknown) as ReportingInternalSetup;
+    } as unknown) as ReportingPluginSetupDeps;
     registerJobInfoRoutes(core);
 
     await server.start();
@@ -328,7 +328,7 @@ describe('GET /api/reporting/jobs/download', () => {
             }),
           },
         },
-      } as unknown) as ReportingInternalSetup;
+      } as unknown) as ReportingPluginSetupDeps;
       registerJobInfoRoutes(core);
 
       await server.start();

x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.test.ts

@@ -8,7 +8,7 @@
 import { KibanaRequest, KibanaResponseFactory } from 'src/core/server';
 import { coreMock, httpServerMock } from 'src/core/server/mocks';
 import { ReportingCore } from '../../';
-import { ReportingInternalSetup } from '../../core';
+import { ReportingPluginSetupDeps } from '../../core';
 import { createMockConfigSchema, createMockReportingCore } from '../../test_helpers';
 import type { ReportingRequestHandlerContext } from '../../types';
 import { authorizedUserPreRouting } from './authorized_user_pre_routing';
@@ -45,7 +45,7 @@ describe('authorized_user_pre_routing', function () {
         // @ts-ignore
         ...mockCore.pluginSetupDeps,
         security: undefined, // disable security
-      } as unknown) as ReportingInternalSetup);
+      } as unknown) as ReportingPluginSetupDeps);
     const mockResponseFactory = httpServerMock.createResponseFactory() as KibanaResponseFactory;
 
     let handlerCalled = false;
@@ -68,7 +68,7 @@ describe('authorized_user_pre_routing', function () {
             isEnabled: () => false,
           },
         }, // disable security
-      } as unknown) as ReportingInternalSetup);
+      } as unknown) as ReportingPluginSetupDeps);
     const mockResponseFactory = httpServerMock.createResponseFactory() as KibanaResponseFactory;
 
     let handlerCalled = false;
@@ -90,7 +90,7 @@ describe('authorized_user_pre_routing', function () {
           license: { isEnabled: () => true },
           authc: { getCurrentUser: () => null },
         },
-      } as unknown) as ReportingInternalSetup);
+      } as unknown) as ReportingPluginSetupDeps);
     const mockHandler = () => {
       throw new Error('Handler callback should not be called');
     };
@@ -122,7 +122,7 @@ describe('authorized_user_pre_routing', function () {
             license: { isEnabled: () => true },
             authc: { getCurrentUser: () => ({ username: 'friendlyuser', roles: ['cowboy'] }) },
           },
-        } as unknown) as ReportingInternalSetup);
+        } as unknown) as ReportingPluginSetupDeps);
       const mockResponseFactory = getMockResponseFactory();
 
       const mockHandler = () => {
@@ -148,7 +148,7 @@ describe('authorized_user_pre_routing', function () {
               getCurrentUser: () => ({ username: 'friendlyuser', roles: ['reporting_user'] }),
             },
           },
-        } as unknown) as ReportingInternalSetup);
+        } as unknown) as ReportingPluginSetupDeps);
       const mockResponseFactory = getMockResponseFactory();
 
       authorizedUserPreRouting(mockCore, (user) => {

x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts

@@ -23,7 +23,7 @@ import {
   initializeBrowserDriverFactory,
 } from '../browsers';
 import { ReportingConfigType } from '../config';
-import { ReportingInternalSetup, ReportingInternalStart } from '../core';
+import { ReportingPluginSetupDeps, ReportingPluginStartDeps } from '../core';
 import { ReportingStore } from '../lib';
 import { setFieldFormats } from '../services';
 import { createMockLevelLogger } from './create_mock_levellogger';
@@ -34,7 +34,7 @@ import { createMockLevelLogger } from './create_mock_levellogger';
 
 (chromium as any).createDriverFactory.mockImplementation(() => ({}));
 
-export const createMockPluginSetup = (setupMock?: any): ReportingInternalSetup => {
+export const createMockPluginSetup = (setupMock?: any): ReportingPluginSetupDeps => {
   return {
     features: featuresPluginMock.createSetup(),
     basePath: { set: jest.fn() },
@@ -54,7 +54,7 @@ const createMockReportingStore = () => ({} as ReportingStore);
 export const createMockPluginStart = (
   mockReportingCore: ReportingCore | undefined,
   startMock?: any
-): ReportingInternalStart => {
+): ReportingPluginStartDeps => {
   const store = mockReportingCore
     ? new ReportingStore(mockReportingCore, logger)
     : createMockReportingStore();
@@ -138,8 +138,8 @@ export const createMockConfig = (
 
 export const createMockReportingCore = async (
   config: ReportingConfigType,
-  setupDepsMock: ReportingInternalSetup | undefined = undefined,
-  startDepsMock: ReportingInternalStart | undefined = undefined
+  setupDepsMock: ReportingPluginSetupDeps | undefined = undefined,
+  startDepsMock: ReportingPluginStartDeps | undefined = undefined
 ) => {
   const mockReportingCore = ({
     getConfig: () => createMockConfig(config),