Use opaque ids for plugins

elastic · Jul 29, 2019 · 2a7261a · 2a7261a
1 parent f81d6fb
commit 2a7261a
Show file tree

Hide file tree

Showing 20 changed files with 242 additions and 169 deletions.
diff --git a/docs/development/core/public/kibana-plugin-public.contextsetup.md b/docs/development/core/public/kibana-plugin-public.contextsetup.md
@@ -26,45 +26,49 @@ Contexts providers are executed in the order they were registered. Each provider
 
 In order to configure a handler with context, you must call the [IContextContainer.createHandler()](./kibana-plugin-public.icontextcontainer.createhandler.md) function and use the returned handler which will automatically build a context object when called.
 
-When registering context or creating handlers, the \_calling plugin's id\_ must be provided. Note this should NOT be the context service owner, but the plugin that is actually registering the context or handler.
+When registering context or creating handlers, the \_calling plugin's opaque id\_ must be provided. This id is passed in via the plugin's initializer and can be accessed from the [PluginInitializerContext.opaqueId](./kibana-plugin-public.plugininitializercontext.opaqueid.md) Note this should NOT be the context service owner's id, but the plugin that is actually registering the context or handler.
 
 ```ts
-// GOOD
+// Correct
 class MyPlugin {
   private readonly handlers = new Map();
 
   setup(core) {
     this.contextContainer = core.context.createContextContainer();
     return {
-      registerContext(pluginId, contextName, provider) {
-        this.contextContainer.registerContext(pluginId, contextName, provider);
+      registerContext(pluginOpaqueId, contextName, provider) {
+        this.contextContainer.registerContext(pluginOpaqueId, contextName, provider);
       },
-      registerRoute(pluginId, path, handler) {
+      registerRoute(pluginOpaqueId, path, handler) {
         this.handlers.set(
           path,
-          this.contextContainer.createHandler(pluginId, handler)
+          this.contextContainer.createHandler(pluginOpaqueId, handler)
         );
       }
     }
   }
 }
 
-// BAD
+// Incorrect
 class MyPlugin {
   private readonly handlers = new Map();
 
+  constructor(private readonly initContext: PluginInitializerContext) {}
+
   setup(core) {
     this.contextContainer = core.context.createContextContainer();
     return {
-      registerContext(pluginId, contextName, provider) {
+      registerContext(contextName, provider) {
+        // BUG!
         // This would leak this context to all handlers rather that only plugins that depend on the calling plugin.
-        this.contextContainer.registerContext('my_plugin', contextName, provider);
+        this.contextContainer.registerContext(this.initContext.opaqueId, contextName, provider);
       },
-      registerRoute(pluginId, path, handler) {
+      registerRoute(path, handler) {
         this.handlers.set(
           path,
+          // BUG!
           // This handler will not receive any contexts provided by other dependencies of the calling plugin.
-          this.contextContainer.createHandler('my_plugin', handler)
+          this.contextContainer.createHandler(this.initContext.opaqueId, handler)
         );
       }
     }
@@ -100,7 +104,7 @@ class VizRenderingPlugin {
 
     return {
       registerContext: this.contextContainer.registerContext,
-      registerVizRenderer: (plugin: string, renderMethod: string, renderer: VizTypeRenderer) =>
+      registerVizRenderer: (plugin: PluginOpaqueId, renderMethod: string, renderer: VizTypeRenderer) =>
         this.vizRenderers.set(renderMethod, this.contextContainer.createHandler(plugin, renderer)),
     };
   }

diff --git a/...development/core/public/kibana-plugin-public.icontextcontainer.createhandler.md b/...development/core/public/kibana-plugin-public.icontextcontainer.createhandler.md
@@ -9,14 +9,14 @@ Create a new handler function pre-wired to context for the plugin.
 <b>Signature:</b>
 
 ```typescript
-createHandler(pluginId: string, handler: IContextHandler<TContext, THandlerReturn, THandlerParameters>): (...rest: THandlerParameters) => Promisify<THandlerReturn>;
+createHandler(pluginOpaqueId: PluginOpaqueId, handler: IContextHandler<TContext, THandlerReturn, THandlerParameters>): (...rest: THandlerParameters) => Promisify<THandlerReturn>;
 ```
 
 ## Parameters
 
 |  Parameter | Type | Description |
 |  --- | --- | --- |
-|  pluginId | <code>string</code> | The plugin ID for the plugin that registers this handler. |
+|  pluginOpaqueId | <code>PluginOpaqueId</code> | The plugin opaque ID for the plugin that registers this handler. |
 |  handler | <code>IContextHandler&lt;TContext, THandlerReturn, THandlerParameters&gt;</code> | Handler function to pass context object to. |
 
 <b>Returns:</b>

diff --git a/docs/development/core/public/kibana-plugin-public.icontextcontainer.md b/docs/development/core/public/kibana-plugin-public.icontextcontainer.md
@@ -16,8 +16,8 @@ export interface IContextContainer<TContext extends {}, THandlerReturn, THandler
 
 |  Method | Description |
 |  --- | --- |
-|  [createHandler(pluginId, handler)](./kibana-plugin-public.icontextcontainer.createhandler.md) | Create a new handler function pre-wired to context for the plugin. |
-|  [registerContext(pluginId, contextName, provider)](./kibana-plugin-public.icontextcontainer.registercontext.md) | Register a new context provider. |
+|  [createHandler(pluginOpaqueId, handler)](./kibana-plugin-public.icontextcontainer.createhandler.md) | Create a new handler function pre-wired to context for the plugin. |
+|  [registerContext(pluginOpaqueId, contextName, provider)](./kibana-plugin-public.icontextcontainer.registercontext.md) | Register a new context provider. |
 
 ## Remarks
 
@@ -27,45 +27,49 @@ Contexts providers are executed in the order they were registered. Each provider
 
 In order to configure a handler with context, you must call the [IContextContainer.createHandler()](./kibana-plugin-public.icontextcontainer.createhandler.md) function and use the returned handler which will automatically build a context object when called.
 
-When registering context or creating handlers, the \_calling plugin's id\_ must be provided. Note this should NOT be the context service owner, but the plugin that is actually registering the context or handler.
+When registering context or creating handlers, the \_calling plugin's opaque id\_ must be provided. This id is passed in via the plugin's initializer and can be accessed from the [PluginInitializerContext.opaqueId](./kibana-plugin-public.plugininitializercontext.opaqueid.md) Note this should NOT be the context service owner's id, but the plugin that is actually registering the context or handler.
 
 ```ts
-// GOOD
+// Correct
 class MyPlugin {
   private readonly handlers = new Map();
 
   setup(core) {
     this.contextContainer = core.context.createContextContainer();
     return {
-      registerContext(pluginId, contextName, provider) {
-        this.contextContainer.registerContext(pluginId, contextName, provider);
+      registerContext(pluginOpaqueId, contextName, provider) {
+        this.contextContainer.registerContext(pluginOpaqueId, contextName, provider);
       },
-      registerRoute(pluginId, path, handler) {
+      registerRoute(pluginOpaqueId, path, handler) {
         this.handlers.set(
           path,
-          this.contextContainer.createHandler(pluginId, handler)
+          this.contextContainer.createHandler(pluginOpaqueId, handler)
         );
       }
     }
   }
 }
 
-// BAD
+// Incorrect
 class MyPlugin {
   private readonly handlers = new Map();
 
+  constructor(private readonly initContext: PluginInitializerContext) {}
+
   setup(core) {
     this.contextContainer = core.context.createContextContainer();
     return {
-      registerContext(pluginId, contextName, provider) {
+      registerContext(contextName, provider) {
+        // BUG!
         // This would leak this context to all handlers rather that only plugins that depend on the calling plugin.
-        this.contextContainer.registerContext('my_plugin', contextName, provider);
+        this.contextContainer.registerContext(this.initContext.opaqueId, contextName, provider);
       },
-      registerRoute(pluginId, path, handler) {
+      registerRoute(path, handler) {
         this.handlers.set(
           path,
+          // BUG!
           // This handler will not receive any contexts provided by other dependencies of the calling plugin.
-          this.contextContainer.createHandler('my_plugin', handler)
+          this.contextContainer.createHandler(this.initContext.opaqueId, handler)
         );
       }
     }

diff --git a/...velopment/core/public/kibana-plugin-public.icontextcontainer.registercontext.md b/...velopment/core/public/kibana-plugin-public.icontextcontainer.registercontext.md
@@ -9,14 +9,14 @@ Register a new context provider.
 <b>Signature:</b>
 
 ```typescript
-registerContext<TContextName extends keyof TContext>(pluginId: string, contextName: TContextName, provider: IContextProvider<TContext, TContextName, THandlerParameters>): this;
+registerContext<TContextName extends keyof TContext>(pluginOpaqueId: PluginOpaqueId, contextName: TContextName, provider: IContextProvider<TContext, TContextName, THandlerParameters>): this;
 ```
 
 ## Parameters
 
 |  Parameter | Type | Description |
 |  --- | --- | --- |
-|  pluginId | <code>string</code> | The plugin ID for the plugin that registers this context. |
+|  pluginOpaqueId | <code>PluginOpaqueId</code> | The plugin opaque ID for the plugin that registers this context. |
 |  contextName | <code>TContextName</code> | The key of the <code>TContext</code> object this provider supplies the value for. |
 |  provider | <code>IContextProvider&lt;TContext, TContextName, THandlerParameters&gt;</code> | A [IContextProvider](./kibana-plugin-public.icontextprovider.md) to be called each time a new context is created. |
 

diff --git a/docs/development/core/public/kibana-plugin-public.plugininitializercontext.md b/docs/development/core/public/kibana-plugin-public.plugininitializercontext.md
@@ -11,3 +11,10 @@ The available core services passed to a `PluginInitializer`
 ```typescript
 export interface PluginInitializerContext 
 ```
+
+## Properties
+
+|  Property | Type | Description |
+|  --- | --- | --- |
+|  [opaqueId](./kibana-plugin-public.plugininitializercontext.opaqueid.md) | <code>PluginOpaqueId</code> | A symbol used to identify this plugin in the system. Needed when registering handlers or context providers. |
+
diff --git a/...velopment/core/public/kibana-plugin-public.plugininitializercontext.opaqueid.md b/...velopment/core/public/kibana-plugin-public.plugininitializercontext.opaqueid.md
@@ -0,0 +1,13 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-public](./kibana-plugin-public.md) &gt; [PluginInitializerContext](./kibana-plugin-public.plugininitializercontext.md) &gt; [opaqueId](./kibana-plugin-public.plugininitializercontext.opaqueid.md)
+
+## PluginInitializerContext.opaqueId property
+
+A symbol used to identify this plugin in the system. Needed when registering handlers or context providers.
+
+<b>Signature:</b>
+
+```typescript
+readonly opaqueId: PluginOpaqueId;
+```
diff --git a/src/core/public/context/context.test.ts b/src/core/public/context/context.test.ts
@@ -17,14 +17,18 @@
  * under the License.
  */
 
-import { PluginName } from '../../server';
 import { ContextContainer } from './context';
-
-const plugins: ReadonlyMap<PluginName, PluginName[]> = new Map([
-  ['pluginA', []],
-  ['pluginB', ['pluginA']],
-  ['pluginC', ['pluginA', 'pluginB']],
-  ['pluginD', []],
+import { PluginOpaqueId } from '../plugins';
+
+const pluginA = Symbol('pluginA');
+const pluginB = Symbol('pluginB');
+const pluginC = Symbol('pluginC');
+const pluginD = Symbol('pluginD');
+const plugins: ReadonlyMap<PluginOpaqueId, PluginOpaqueId[]> = new Map([
+  [pluginA, []],
+  [pluginB, [pluginA]],
+  [pluginC, [pluginA, pluginB]],
+  [pluginD, []],
 ]);
 
 interface MyContext {
@@ -34,7 +38,6 @@ interface MyContext {
   ctxFromB: number;
   ctxFromC: boolean;
   ctxFromD: object;
-  baseCtx: number;
 }
 
 const coreId = Symbol();
@@ -51,6 +54,17 @@ describe('ContextContainer', () => {
     );
   });
 
+  describe('registerContext', () => {
+    it('throws error if called with an unknown symbol', async () => {
+      const contextContainer = new ContextContainer<MyContext, string>(plugins, coreId);
+      await expect(() =>
+        contextContainer.registerContext(Symbol('unknown'), 'ctxFromA', jest.fn())
+      ).toThrowErrorMatchingInlineSnapshot(
+        `"Cannot register context for unknown plugin: Symbol(unknown)"`
+      );
+    });
+  });
+
   describe('context building', () => {
     it('resolves dependencies', async () => {
       const contextContainer = new ContextContainer<MyContext, string>(plugins, coreId);
@@ -60,28 +74,28 @@ describe('ContextContainer', () => {
         return 'core';
       });
 
-      contextContainer.registerContext('pluginA', 'ctxFromA', context => {
+      contextContainer.registerContext(pluginA, 'ctxFromA', context => {
         expect(context).toEqual({ core1: 'core' });
         return 'aString';
       });
-      contextContainer.registerContext('pluginB', 'ctxFromB', context => {
+      contextContainer.registerContext(pluginB, 'ctxFromB', context => {
         expect(context).toEqual({ core1: 'core', ctxFromA: 'aString' });
         return 299;
       });
-      contextContainer.registerContext('pluginC', 'ctxFromC', context => {
+      contextContainer.registerContext(pluginC, 'ctxFromC', context => {
         expect(context).toEqual({ core1: 'core', ctxFromA: 'aString', ctxFromB: 299 });
         return false;
       });
-      contextContainer.registerContext('pluginD', 'ctxFromD', context => {
+      contextContainer.registerContext(pluginD, 'ctxFromD', context => {
         expect(context).toEqual({ core1: 'core' });
         return {};
       });
 
       const rawHandler1 = jest.fn<string, []>(() => 'handler1');
-      const handler1 = contextContainer.createHandler('pluginC', rawHandler1);
+      const handler1 = contextContainer.createHandler(pluginC, rawHandler1);
 
       const rawHandler2 = jest.fn<string, []>(() => 'handler2');
-      const handler2 = contextContainer.createHandler('pluginD', rawHandler2);
+      const handler2 = contextContainer.createHandler(pluginD, rawHandler2);
 
       await handler1();
       await handler2();
@@ -116,7 +130,7 @@ describe('ContextContainer', () => {
         });
 
       const rawHandler1 = jest.fn<string, []>(() => 'handler1');
-      const handler1 = contextContainer.createHandler('pluginA', rawHandler1);
+      const handler1 = contextContainer.createHandler(pluginA, rawHandler1);
 
       expect(await handler1()).toEqual('handler1');
 
@@ -132,7 +146,7 @@ describe('ContextContainer', () => {
 
       contextContainer
         .registerContext(coreId, 'core1', context => 'core')
-        .registerContext('pluginA', 'ctxFromA', context => 'aString');
+        .registerContext(pluginA, 'ctxFromA', context => 'aString');
 
       const rawHandler1 = jest.fn<string, []>(() => 'handler1');
       const handler1 = contextContainer.createHandler(coreId, rawHandler1);
@@ -157,7 +171,7 @@ describe('ContextContainer', () => {
         return `core ${str}`;
       });
 
-      contextContainer.registerContext('pluginD', 'ctxFromD', (context, str, num) => {
+      contextContainer.registerContext(pluginD, 'ctxFromD', (context, str, num) => {
         expect(str).toEqual('passed string');
         expect(num).toEqual(77);
         return {
@@ -166,7 +180,7 @@ describe('ContextContainer', () => {
       });
 
       const rawHandler1 = jest.fn<string, [MyContext, string, number]>(() => 'handler1');
-      const handler1 = contextContainer.createHandler('pluginD', rawHandler1);
+      const handler1 = contextContainer.createHandler(pluginD, rawHandler1);
 
       expect(await handler1('passed string', 77)).toEqual('handler1');
 
@@ -184,18 +198,20 @@ describe('ContextContainer', () => {
   });
 
   describe('createHandler', () => {
-    it('throws error if called with a different symbol', async () => {
+    it('throws error if called with an unknown symbol', async () => {
       const contextContainer = new ContextContainer<MyContext, string>(plugins, coreId);
       await expect(() =>
-        contextContainer.createHandler(Symbol(), jest.fn())
-      ).toThrowErrorMatchingInlineSnapshot(`"Symbol only allowed for core services"`);
+        contextContainer.createHandler(Symbol('unknown'), jest.fn())
+      ).toThrowErrorMatchingInlineSnapshot(
+        `"Cannot create handler for unknown plugin: Symbol(unknown)"`
+      );
     });
 
     it('returns value from original handler', async () => {
       const contextContainer = new ContextContainer<MyContext, string>(plugins, coreId);
 
       const rawHandler1 = jest.fn(() => 'handler1');
-      const handler1 = contextContainer.createHandler('pluginA', rawHandler1);
+      const handler1 = contextContainer.createHandler(pluginA, rawHandler1);
 
       expect(await handler1()).toEqual('handler1');
     });
@@ -207,7 +223,7 @@ describe('ContextContainer', () => {
       );
 
       const rawHandler1 = jest.fn<string, [MyContext, string, number]>(() => 'handler1');
-      const handler1 = contextContainer.createHandler('pluginA', rawHandler1);
+      const handler1 = contextContainer.createHandler(pluginA, rawHandler1);
 
       await handler1('passed string', 77);
       expect(rawHandler1).toHaveBeenCalledWith({}, 'passed string', 77);