introduce start pahse for plugins service for parity with client side

src/core/server/index.ts

@@ -18,7 +18,7 @@
  */
 import { ElasticsearchServiceSetup } from './elasticsearch';
 import { HttpServiceSetup, HttpServiceStart } from './http';
-import { PluginsServiceSetup } from './plugins';
+import { PluginsServiceSetup, PluginsServiceStart } from './plugins';
 
 export { bootstrap } from './bootstrap';
 export { ConfigService } from './config';
@@ -47,6 +47,7 @@ export {
   PluginInitializerContext,
   PluginName,
   PluginSetupContext,
+  PluginStartContext,
 } from './plugins';
 
 /** @public */
@@ -58,6 +59,13 @@ export interface CoreSetup {
 
 export interface CoreStart {
   http: HttpServiceStart;
+  plugins: PluginsServiceStart;
 }
 
-export { HttpServiceSetup, HttpServiceStart, ElasticsearchServiceSetup, PluginsServiceSetup };
+export {
+  HttpServiceSetup,
+  HttpServiceStart,
+  ElasticsearchServiceSetup,
+  PluginsServiceSetup,
+  PluginsServiceStart,
+};

src/core/server/legacy/legacy_service.test.ts

@@ -35,7 +35,7 @@ import { ElasticsearchServiceSetup } from '../elasticsearch';
 import { HttpServiceStart } from '../http';
 import { loggingServiceMock } from '../logging/logging_service.mock';
 import { DiscoveredPlugin, DiscoveredPluginInternal } from '../plugins';
-import { PluginsServiceSetup } from '../plugins/plugins_service';
+import { PluginsServiceSetup, PluginsServiceStart } from '../plugins/plugins_service';
 import { LegacyPlatformProxy } from './legacy_platform_proxy';
 
 const MockKbnServer: jest.Mock<KbnServer> = KbnServer as any;
@@ -52,6 +52,7 @@ let setupDeps: {
 
 let startDeps: {
   http: HttpServiceStart;
+  plugins: PluginsServiceStart;
 };
 
 const logger = loggingServiceMock.create();
@@ -82,6 +83,9 @@ beforeEach(() => {
     http: {
       isListening: () => true,
     },
+    plugins: {
+      contracts: new Map(),
+    },
   };
 
   config$ = new BehaviorSubject<Config>(
@@ -317,6 +321,9 @@ describe('once LegacyService is set up without connection info', () => {
     http: {
       isListening: () => false,
     },
+    plugins: {
+      contracts: new Map(),
+    },
   };
   beforeEach(async () => {
     await legacyService.setup(setupDeps);
@@ -379,6 +386,9 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => {
       http: {
         isListening: () => false,
       },
+      plugins: {
+        contracts: new Map(),
+      },
     });
 
     expect(MockClusterManager.create.mock.calls).toMatchSnapshot(
@@ -407,6 +417,9 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => {
       http: {
         isListening: () => false,
       },
+      plugins: {
+        contracts: new Map(),
+      },
     });
 
     expect(MockClusterManager.create.mock.calls).toMatchSnapshot(

src/core/server/plugins/index.ts

@@ -17,7 +17,7 @@
  * under the License.
  */
 
-export { PluginsService, PluginsServiceSetup } from './plugins_service';
+export { PluginsService, PluginsServiceSetup, PluginsServiceStart } from './plugins_service';
 
 /** @internal */
 export { isNewPlatformPlugin } from './discovery';
@@ -29,4 +29,4 @@ export {
   PluginInitializer,
   PluginName,
 } from './plugin';
-export { PluginInitializerContext, PluginSetupContext } from './plugin_context';
+export { PluginInitializerContext, PluginSetupContext, PluginStartContext } from './plugin_context';

src/core/server/plugins/plugin.test.ts

@@ -179,6 +179,44 @@ test('`setup` initializes plugin and calls appropriate lifecycle hook', async ()
   expect(mockPluginInstance.setup).toHaveBeenCalledWith(setupContext, setupDependencies);
 });
 
+test('`start` fails if setup is not called first', async () => {
+  const manifest = createPluginManifest();
+  const plugin = new PluginWrapper(
+    'some-plugin-path',
+    manifest,
+    createPluginInitializerContext(coreContext, manifest)
+  );
+
+  await expect(plugin.start({} as any, {} as any)).rejects.toThrowErrorMatchingInlineSnapshot(
+    `"Plugin \\"some-plugin-id\\" can't be started since it isn't set up."`
+  );
+});
+
+test('`start` calls plugin.start with context and dependencies', async () => {
+  const manifest = createPluginManifest();
+  const plugin = new PluginWrapper(
+    'plugin-with-initializer-path',
+    manifest,
+    createPluginInitializerContext(coreContext, manifest)
+  );
+  const context = { any: 'thing' } as any;
+  const deps = { otherDep: 'value' };
+
+  const pluginStartContract = { contract: 'start-contract' };
+  const mockPluginInstance = {
+    setup: jest.fn(),
+    start: jest.fn().mockResolvedValue(pluginStartContract),
+  };
+  mockPluginInitializer.mockReturnValue(mockPluginInstance);
+
+  await plugin.setup({} as any, {} as any);
+
+  const startContract = await plugin.start(context, deps);
+
+  expect(startContract).toBe(pluginStartContract);
+  expect(mockPluginInstance.start).toHaveBeenCalledWith(context, deps);
+});
+
 test('`stop` fails if plugin is not set up', async () => {
   const manifest = createPluginManifest();
   const plugin = new PluginWrapper(

src/core/server/plugins/plugin.ts

@@ -21,7 +21,7 @@ import { join } from 'path';
 import typeDetect from 'type-detect';
 import { ConfigPath } from '../config';
 import { Logger } from '../logging';
-import { PluginInitializerContext, PluginSetupContext } from './plugin_context';
+import { PluginInitializerContext, PluginSetupContext, PluginStartContext } from './plugin_context';
 
 /**
  * Dedicated type for plugin name/id that is supposed to make Map/Set/Arrays
@@ -129,11 +129,14 @@ export interface DiscoveredPluginInternal extends DiscoveredPlugin {
  *
  * @public
  */
-export interface Plugin<TSetup, TPluginsSetup extends Record<PluginName, unknown> = {}> {
-  setup: (
-    pluginSetupContext: PluginSetupContext,
-    plugins: TPluginsSetup
-  ) => TSetup | Promise<TSetup>;
+export interface Plugin<
+  TSetup,
+  TStart,
+  TPluginsSetup extends Record<PluginName, unknown> = {},
+  TPluginsStart extends Record<PluginName, unknown> = {}
+> {
+  setup: (core: PluginSetupContext, plugins: TPluginsSetup) => TSetup | Promise<TSetup>;
+  start: (core: PluginStartContext, plugins: TPluginsStart) => TStart | Promise<TStart>;
   stop?: () => void;
 }
 
@@ -143,9 +146,12 @@ export interface Plugin<TSetup, TPluginsSetup extends Record<PluginName, unknown
  *
  * @public
  */
-export type PluginInitializer<TSetup, TPluginsSetup extends Record<PluginName, unknown> = {}> = (
-  coreContext: PluginInitializerContext
-) => Plugin<TSetup, TPluginsSetup>;
+export type PluginInitializer<
+  TSetup,
+  TStart,
+  TPluginsSetup extends Record<PluginName, unknown> = {},
+  TPluginsStart extends Record<PluginName, unknown> = {}
+> = (core: PluginInitializerContext) => Plugin<TSetup, TStart, TPluginsSetup, TPluginsStart>;
 
 /**
  * Lightweight wrapper around discovered plugin that is responsible for instantiating
@@ -155,7 +161,9 @@ export type PluginInitializer<TSetup, TPluginsSetup extends Record<PluginName, u
  */
 export class PluginWrapper<
   TSetup = unknown,
-  TPluginsSetup extends Record<PluginName, unknown> = Record<PluginName, unknown>
+  TStart = unknown,
+  TPluginsSetup extends Record<PluginName, unknown> = Record<PluginName, unknown>,
+  TPluginsStart extends Record<PluginName, unknown> = Record<PluginName, unknown>
 > {
   public readonly name: PluginManifest['id'];
   public readonly configPath: PluginManifest['configPath'];
@@ -166,7 +174,7 @@ export class PluginWrapper<
 
   private readonly log: Logger;
 
-  private instance?: Plugin<TSetup, TPluginsSetup>;
+  private instance?: Plugin<TSetup, TStart, TPluginsSetup, TPluginsStart>;
 
   constructor(
     public readonly path: string,
@@ -197,6 +205,21 @@ export class PluginWrapper<
     return await this.instance.setup(setupContext, plugins);
   }
 
+  /**
+   * Calls `start` function exposed by the initialized plugin.
+   * @param startContext Context that consists of various core services tailored specifically
+   * for the `start` lifecycle event.
+   * @param plugins The dictionary where the key is the dependency name and the value
+   * is the contract returned by the dependency's `start` function.
+   */
+  public async start(startContext: PluginStartContext, plugins: TPluginsStart) {
+    if (this.instance === undefined) {
+      throw new Error(`Plugin "${this.name}" can't be started since it isn't set up.`);
+    }
+
+    return await this.instance.start(startContext, plugins);
+  }
+
   /**
    * Calls optional `stop` function exposed by the plugin initializer.
    */
@@ -224,7 +247,7 @@ export class PluginWrapper<
     }
 
     const { plugin: initializer } = pluginDefinition as {
-      plugin: PluginInitializer<TSetup, TPluginsSetup>;
+      plugin: PluginInitializer<TSetup, TStart, TPluginsSetup, TPluginsStart>;
     };
     if (!initializer || typeof initializer !== 'function') {
       throw new Error(`Definition of plugin "${this.name}" should be a function (${this.path}).`);

src/core/server/plugins/plugin_context.ts

@@ -25,7 +25,7 @@ import { ClusterClient } from '../elasticsearch';
 import { HttpServiceSetup } from '../http';
 import { LoggerFactory } from '../logging';
 import { PluginWrapper, PluginManifest } from './plugin';
-import { PluginsServiceSetupDeps } from './plugins_service';
+import { PluginsServiceSetupDeps, PluginsServiceStartDeps } from './plugins_service';
 
 /**
  * Context that's available to plugins during initialization stage.
@@ -61,6 +61,13 @@ export interface PluginSetupContext {
   };
 }
 
+/**
+ * Context passed to the plugins `start` method.
+ *
+ * @public
+ */
+export interface PluginStartContext {} // eslint-disable-line @typescript-eslint/no-empty-interface
+
 /**
  * This returns a facade for `CoreContext` that will be exposed to the plugin initializer.
  * This facade should be safe to use across entire plugin lifespan.
@@ -144,3 +151,23 @@ export function createPluginSetupContext<TPlugin, TPluginDependencies>(
     },
   };
 }
+
+/**
+ * This returns a facade for `CoreContext` that will be exposed to the plugin `start` method.
+ * This facade should be safe to use only within `start` itself.
+ *
+ * This is called for each plugin when it starts, so each plugin gets its own
+ * version of these values.
+ *
+ * @param coreContext Kibana core context
+ * @param plugin The plugin we're building these values for.
+ * @param deps Dependencies that Plugins services gets during start.
+ * @internal
+ */
+export function createPluginStartContext<TPlugin, TPluginDependencies>(
+  coreContext: CoreContext,
+  deps: PluginsServiceStartDeps,
+  plugin: PluginWrapper<TPlugin, TPluginDependencies>
+): PluginStartContext {
+  return {};
+}

src/core/server/plugins/plugins_service.ts

@@ -38,14 +38,22 @@ export interface PluginsServiceSetup {
   };
 }
 
+/** @internal */
+export interface PluginsServiceStart {
+  contracts: Map<PluginName, unknown>;
+}
+
 /** @internal */
 export interface PluginsServiceSetupDeps {
   elasticsearch: ElasticsearchServiceSetup;
   http: HttpServiceSetup;
 }
 
 /** @internal */
-export class PluginsService implements CoreService<PluginsServiceSetup> {
+export interface PluginsServiceStartDeps {} // eslint-disable-line @typescript-eslint/no-empty-interface
+
+/** @internal */
+export class PluginsService implements CoreService<PluginsServiceSetup, PluginsServiceStart> {
   private readonly log: Logger;
   private readonly pluginsSystem: PluginsSystem;
 
@@ -80,7 +88,11 @@ export class PluginsService implements CoreService<PluginsServiceSetup> {
     };
   }
 
-  public async start() {}
+  public async start(deps: PluginsServiceStartDeps) {
+    this.log.debug('Plugins service starts plugins');
+    const contracts = await this.pluginsSystem.startPlugins(deps);
+    return { contracts };
+  }
 
   public async stop() {
     this.log.debug('Stopping plugins service');

src/core/server/plugins/plugins_system.test.mocks.ts

@@ -18,6 +18,8 @@
  */
 
 export const mockCreatePluginSetupContext = jest.fn();
+export const mockCreatePluginStartContext = jest.fn();
 jest.mock('./plugin_context', () => ({
   createPluginSetupContext: mockCreatePluginSetupContext,
+  createPluginStartContext: mockCreatePluginStartContext,
 }));