PR comments

docs/development/core/public/kibana-plugin-public.contextcontainer.createcontext.md

@@ -9,14 +9,14 @@ Create a new context.
 <b>Signature:</b>
 
 ```typescript
-createContext(plugin: PluginName, baseContext: Partial<TContext>, ...contextArgs: TProviderParameters): Promise<TContext>;
+createContext(plugin: string, baseContext: Partial<TContext>, ...contextArgs: TProviderParameters): Promise<TContext>;
 ```
 
 ## Parameters
 
 |  Parameter | Type | Description |
 |  --- | --- | --- |
-|  plugin | <code>PluginName</code> | The plugin the context will be exposed to. |
+|  plugin | <code>string</code> | The plugin the context will be exposed to. |
 |  baseContext | <code>Partial&lt;TContext&gt;</code> | The initial context for the given handler. |
 |  contextArgs | <code>TProviderParameters</code> | Additional parameters to call providers with. |
 

docs/development/core/public/kibana-plugin-public.contextcontainer.md

@@ -17,9 +17,11 @@ export interface ContextContainer<TContext extends {}, TProviderParameters exten
 |  Method | Description |
 |  --- | --- |
 |  [createContext(plugin, baseContext, contextArgs)](./kibana-plugin-public.contextcontainer.createcontext.md) | Create a new context. |
-|  [register(contextName, provider, plugin)](./kibana-plugin-public.contextcontainer.register.md) | Register a new context provider. Throws an excpetion if more than one provider is registered for the same context key. |
+|  [register(contextName, provider, plugin)](./kibana-plugin-public.contextcontainer.register.md) | Register a new context provider. Throws an exception if more than one provider is registered for the same context key. |
 
 ## Remarks
 
+A `ContextContainer` can be used by any Core service or plugin (known as the "service owner") which wishes to expose APIs in a handler function. The container object will manage registering context providers and building a context object for a handler with all of the contexts that should be exposed to the handler's plugin. This is dependent on the dependencies that the handler's plugin declares.
+
 Contexts providers are executed in the order they were registered. Each provider gets access to context values provided by any plugins that it depends on.
 

docs/development/core/public/kibana-plugin-public.contextcontainer.register.md

@@ -4,21 +4,21 @@
 
 ## ContextContainer.register() method
 
-Register a new context provider. Throws an excpetion if more than one provider is registered for the same context key.
+Register a new context provider. Throws an exception if more than one provider is registered for the same context key.
 
 <b>Signature:</b>
 
 ```typescript
-register<TContextName extends keyof TContext>(contextName: TContextName, provider: ContextProvider<TContext, TContextName, TProviderParameters>, plugin?: PluginName): this;
+register<TContextName extends keyof TContext>(contextName: TContextName, provider: ContextProvider<TContext, TContextName, TProviderParameters>, plugin?: string): this;
 ```
 
 ## Parameters
 
 |  Parameter | Type | Description |
 |  --- | --- | --- |
-|  contextName | <code>TContextName</code> | The key of the  object this provider supplies the value for. |
+|  contextName | <code>TContextName</code> | The key of the <code>TContext</code> object this provider supplies the value for. |
 |  provider | <code>ContextProvider&lt;TContext, TContextName, TProviderParameters&gt;</code> | A [ContextProvider](./kibana-plugin-public.contextprovider.md) to be called each time a new context is created. |
-|  plugin | <code>PluginName</code> | The plugin this provider is associated with. If <code>undefined</code>, provider gets access to all provided context keys. |
+|  plugin | <code>string</code> | The plugin this provider is associated with. If <code>undefined</code>, provider gets access to all provided context keys. Only the service owner should be able to call with <code>undefined</code>. |
 
 <b>Returns:</b>
 

src/core/public/context/context.test.ts

@@ -50,7 +50,7 @@ describe('ContextContainer', () => {
   });
 
   it('resolves dependencies', async () => {
-    expect.assertions(8);
+    // expect.assertions(3);
     const contextContainer = new ContextContainerImplementation<MyContext>(plugins);
 
     contextContainer
@@ -61,31 +61,31 @@ describe('ContextContainer', () => {
       .register(
         'ctxFromA',
         context => {
-          expect(context).toEqual({});
+          expect(context).toEqual({ core1: 'core' });
           return 'aString';
         },
         'pluginA'
       )
       .register(
         'ctxFromB',
         context => {
-          expect(context).toEqual({ ctxFromA: 'aString' });
+          expect(context).toEqual({ core1: 'core', ctxFromA: 'aString' });
           return 299;
         },
         'pluginB'
       )
       .register(
         'ctxFromC',
         context => {
-          expect(context).toEqual({ ctxFromA: 'aString', ctxFromB: 299 });
+          expect(context).toEqual({ core1: 'core', ctxFromA: 'aString', ctxFromB: 299 });
           return false;
         },
         'pluginC'
       )
       .register(
         'ctxFromD',
         context => {
-          expect(context).toEqual({});
+          expect(context).toEqual({ core1: 'core' });
           return {};
         },
         'pluginD'
@@ -106,7 +106,7 @@ describe('ContextContainer', () => {
     });
   });
 
-  it('exposes all previously registerd context to Core providers', async () => {
+  it('exposes all previously registered context to Core providers', async () => {
     expect.assertions(3);
     const contextContainer = new ContextContainerImplementation<MyContext>(plugins);
 
@@ -175,4 +175,13 @@ describe('ContextContainer', () => {
       ctxFromD: {},
     });
   });
+
+  it('throws error for unknown plugin', async () => {
+    const contextContainer = new ContextContainerImplementation<MyContext>(plugins);
+    await expect(
+      contextContainer.createContext('otherPlugin')
+    ).rejects.toThrowErrorMatchingInlineSnapshot(
+      `"Cannot create context for unknown plugin: otherPlugin"`
+    );
+  });
 });

src/core/public/context/context.ts

@@ -46,26 +46,31 @@ export type ContextProvider<
  * An object that handles registration of context providers and building of new context objects.
  *
  * @remarks
+ * A `ContextContainer` can be used by any Core service or plugin (known as the "service owner") which wishes to expose
+ * APIs in a handler function. The container object will manage registering context providers and building a context
+ * object for a handler with all of the contexts that should be exposed to the handler's plugin. This is dependent on
+ * the dependencies that the handler's plugin declares.
+ *
  * Contexts providers are executed in the order they were registered. Each provider gets access to context values
  * provided by any plugins that it depends on.
  *
  * @public
  */
 export interface ContextContainer<TContext extends {}, TProviderParameters extends any[] = []> {
   /**
-   * Register a new context provider. Throws an excpetion if more than one provider is registered for the same context
+   * Register a new context provider. Throws an exception if more than one provider is registered for the same context
    * key.
    *
-   * @param contextName - The key of the {@link TContext} object this provider supplies the value for.
+   * @param contextName - The key of the `TContext` object this provider supplies the value for.
    * @param provider - A {@link ContextProvider} to be called each time a new context is created.
    * @param plugin - The plugin this provider is associated with. If `undefined`, provider gets access to all provided
-   *                 context keys.
+   *                 context keys. Only the service owner should be able to call with `undefined`.
    * @returns The `ContextContainer` for method chaining.
    */
   register<TContextName extends keyof TContext>(
     contextName: TContextName,
     provider: ContextProvider<TContext, TContextName, TProviderParameters>,
-    plugin?: PluginName
+    plugin?: string
   ): this;
 
   /**
@@ -77,7 +82,7 @@ export interface ContextContainer<TContext extends {}, TProviderParameters exten
    * @returns A Promise for the new context object.
    */
   createContext(
-    plugin: PluginName,
+    plugin: string,
     baseContext: Partial<TContext>,
     ...contextArgs: TProviderParameters
   ): Promise<TContext>;
@@ -133,23 +138,29 @@ export class ContextContainerImplementation<
     baseContext: Partial<TContext> = {},
     ...contextArgs: TProviderParameters
   ): Promise<TContext> {
-    const contextNamesForPlugin = (plug: PluginName): Array<keyof TContext> => {
-      const pluginDeps = this.pluginDependencies.get(plug)!;
-      return flatten(pluginDeps.map(p => this.contextNamesByPlugin.get(p)!));
+    const ownerContextNames = [...this.contextProviders]
+      .filter(([name, { plugin }]) => plugin === undefined)
+      .map(([name]) => name);
+    const contextNamesForPlugin = (plug: PluginName): Set<keyof TContext> => {
+      const pluginDeps = this.pluginDependencies.get(plug);
+      if (!pluginDeps) {
+        throw new Error(`Cannot create context for unknown plugin: ${pluginName}`);
+      }
+
+      return new Set([
+        // Owner contexts
+        ...ownerContextNames,
+        // Contexts calling plugin created
+        ...(this.contextNamesByPlugin.get(pluginName) || []),
+        // Contexts calling plugin's dependencies created
+        ...flatten(pluginDeps.map(p => this.contextNamesByPlugin.get(p) || [])),
+      ]);
     };
 
-    // Set of all contexts depended on by this plugin
-    const neededContexts = new Set([
-      ...(this.contextNamesByPlugin.get(pluginName) || []),
-      ...contextNamesForPlugin(pluginName),
-    ]);
+    const contextsToBuild = contextNamesForPlugin(pluginName);
 
     return [...this.contextProviders]
-      .filter(
-        ([contextName, { plugin }]) =>
-          // Contexts we depend on OR provided by core
-          neededContexts.has(contextName) || plugin === undefined
-      )
+      .filter(([contextName]) => contextsToBuild.has(contextName))
       .reduce(
         async (contextPromise, [contextName, { provider, plugin }]) => {
           const resolvedContext = await contextPromise;
@@ -158,7 +169,7 @@ export class ContextContainerImplementation<
           // context built so far (this is only possible for providers registered
           // by the service owner).
           const exposedContext = plugin
-            ? pick(resolvedContext, contextNamesForPlugin(plugin))
+            ? pick(resolvedContext, [...contextNamesForPlugin(plugin)])
             : resolvedContext;
 
           return {

src/core/public/context/context_service.ts

@@ -19,30 +19,15 @@
 
 import { InjectedMetadataStart } from '../injected_metadata';
 import { ContextContainer, ContextContainerImplementation } from './context';
-import { PluginName } from '../../server';
 
 interface StartDeps {
   injectedMetadata: InjectedMetadataStart;
+  pluginDependencies: ReadonlyMap<string, string[]>;
 }
 
 /** @internal */
 export class ContextService {
-  public start({ injectedMetadata }: StartDeps): ContextStart {
-    const plugins = injectedMetadata.getPlugins();
-    const allPluginNames = new Set<PluginName>(plugins.map(p => p.id));
-    const pluginDependencies = new Map(
-      plugins.map(
-        p =>
-          [
-            p.id,
-            [
-              ...p.plugin.requiredPlugins,
-              ...p.plugin.optionalPlugins.filter(optPlugin => allPluginNames.has(optPlugin)),
-            ],
-          ] as [PluginName, PluginName[]]
-      )
-    );
-
+  public start({ injectedMetadata, pluginDependencies }: StartDeps): ContextStart {
     return {
       createContextContainer: <T extends {}, U extends any[] = []>() =>
         new ContextContainerImplementation<T, U>(pluginDependencies),

src/core/public/core_system.ts

@@ -73,6 +73,7 @@ export class CoreSystem {
   private readonly context: ContextService;
 
   private readonly rootDomElement: HTMLElement;
+  private pluginDependencies?: ReadonlyMap<string, string[]>;
   private fatalErrorsSetup: FatalErrorsSetup | null = null;
 
   constructor(params: Params) {
@@ -141,6 +142,7 @@ export class CoreSystem {
 
       // Services that do not expose contracts at setup
       const plugins = await this.plugins.setup(core);
+      this.pluginDependencies = plugins.pluginDependencies;
       await this.legacyPlatform.setup({ core, plugins: mapToObject(plugins.contracts) });
 
       return { fatalErrors: this.fatalErrorsSetup };
@@ -160,7 +162,10 @@ export class CoreSystem {
       const injectedMetadata = await this.injectedMetadata.start();
       const docLinks = await this.docLinks.start({ injectedMetadata });
       const http = await this.http.start({ injectedMetadata, fatalErrors: this.fatalErrorsSetup });
-      const context = this.context.start({ injectedMetadata });
+      const context = this.context.start({
+        injectedMetadata,
+        pluginDependencies: this.pluginDependencies!,
+      });
       const i18n = await this.i18n.start();
       const application = await this.application.start({ injectedMetadata });
 

src/core/public/plugins/plugins_service.test.ts

@@ -167,6 +167,22 @@ test('`PluginsService.setup` fails if any plugin instance does not have a setup
   );
 });
 
+test('`PluginsService.setup` exposes plugin dependency map', async () => {
+  const pluginsService = new PluginsService(mockCoreContext);
+  const { pluginDependencies } = await pluginsService.setup(mockSetupDeps);
+  expect(pluginDependencies).toMatchInlineSnapshot(`
+    Map {
+      "pluginA" => Array [],
+      "pluginB" => Array [
+        "pluginA",
+      ],
+      "pluginC" => Array [
+        "pluginA",
+      ],
+    }
+  `);
+});
+
 test('`PluginsService.setup` calls loadPluginBundles with http and plugins', async () => {
   const pluginsService = new PluginsService(mockCoreContext);
   await pluginsService.setup(mockSetupDeps);