From e67d6498062d69ac483d6b0c9b1ae912267d9ac2 Mon Sep 17 00:00:00 2001 From: Josh Dover Date: Mon, 8 Jul 2019 17:22:53 -0500 Subject: [PATCH 1/2] Add ContextService --- ...n-public.contextcontainer.createcontext.md | 28 +++ .../kibana-plugin-public.contextcontainer.md | 25 +++ ...plugin-public.contextcontainer.register.md | 28 +++ .../kibana-plugin-public.contextprovider.md | 18 ++ .../core/public/kibana-plugin-public.md | 2 + src/core/public/context/context.test.ts | 178 ++++++++++++++++++ src/core/public/context/context.ts | 172 +++++++++++++++++ .../public/context/context_service.mock.ts | 41 ++++ src/core/public/context/context_service.ts | 62 ++++++ src/core/public/context/index.ts | 21 +++ src/core/public/core_system.test.mocks.ts | 7 + src/core/public/core_system.test.ts | 6 + src/core/public/core_system.ts | 5 + src/core/public/index.ts | 4 + src/core/public/legacy/legacy_service.test.ts | 3 + .../public/plugins/plugins_service.test.ts | 4 +- src/core/public/public.api.md | 15 ++ 17 files changed, 618 insertions(+), 1 deletion(-) create mode 100644 docs/development/core/public/kibana-plugin-public.contextcontainer.createcontext.md create mode 100644 docs/development/core/public/kibana-plugin-public.contextcontainer.md create mode 100644 docs/development/core/public/kibana-plugin-public.contextcontainer.register.md create mode 100644 docs/development/core/public/kibana-plugin-public.contextprovider.md create mode 100644 src/core/public/context/context.test.ts create mode 100644 src/core/public/context/context.ts create mode 100644 src/core/public/context/context_service.mock.ts create mode 100644 src/core/public/context/context_service.ts create mode 100644 src/core/public/context/index.ts diff --git a/docs/development/core/public/kibana-plugin-public.contextcontainer.createcontext.md b/docs/development/core/public/kibana-plugin-public.contextcontainer.createcontext.md new file mode 100644 index 00000000000000..062f720fd6b8f6 --- /dev/null +++ b/docs/development/core/public/kibana-plugin-public.contextcontainer.createcontext.md @@ -0,0 +1,28 @@ + + +[Home](./index.md) > [kibana-plugin-public](./kibana-plugin-public.md) > [ContextContainer](./kibana-plugin-public.contextcontainer.md) > [createContext](./kibana-plugin-public.contextcontainer.createcontext.md) + +## ContextContainer.createContext() method + +Create a new context. + +Signature: + +```typescript +createContext(plugin: PluginName, baseContext: Partial, ...contextArgs: TProviderParameters): Promise; +``` + +## Parameters + +| Parameter | Type | Description | +| --- | --- | --- | +| plugin | PluginName | The plugin the context will be exposed to. | +| baseContext | Partial<TContext> | The initial context for the given handler. | +| contextArgs | TProviderParameters | Additional parameters to call providers with. | + +Returns: + +`Promise` + +A Promise for the new context object. + diff --git a/docs/development/core/public/kibana-plugin-public.contextcontainer.md b/docs/development/core/public/kibana-plugin-public.contextcontainer.md new file mode 100644 index 00000000000000..1c71fa76c53479 --- /dev/null +++ b/docs/development/core/public/kibana-plugin-public.contextcontainer.md @@ -0,0 +1,25 @@ + + +[Home](./index.md) > [kibana-plugin-public](./kibana-plugin-public.md) > [ContextContainer](./kibana-plugin-public.contextcontainer.md) + +## ContextContainer interface + +An object that handles registration of context providers and building of new context objects. + +Signature: + +```typescript +export interface ContextContainer +``` + +## Methods + +| 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. | + +## Remarks + +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. + diff --git a/docs/development/core/public/kibana-plugin-public.contextcontainer.register.md b/docs/development/core/public/kibana-plugin-public.contextcontainer.register.md new file mode 100644 index 00000000000000..22e5411f43022f --- /dev/null +++ b/docs/development/core/public/kibana-plugin-public.contextcontainer.register.md @@ -0,0 +1,28 @@ + + +[Home](./index.md) > [kibana-plugin-public](./kibana-plugin-public.md) > [ContextContainer](./kibana-plugin-public.contextcontainer.md) > [register](./kibana-plugin-public.contextcontainer.register.md) + +## ContextContainer.register() method + +Register a new context provider. Throws an excpetion if more than one provider is registered for the same context key. + +Signature: + +```typescript +register(contextName: TContextName, provider: ContextProvider, plugin?: PluginName): this; +``` + +## Parameters + +| Parameter | Type | Description | +| --- | --- | --- | +| contextName | TContextName | The key of the object this provider supplies the value for. | +| provider | ContextProvider<TContext, TContextName, TProviderParameters> | A [ContextProvider](./kibana-plugin-public.contextprovider.md) to be called each time a new context is created. | +| plugin | PluginName | The plugin this provider is associated with. If undefined, provider gets access to all provided context keys. | + +Returns: + +`this` + +The `ContextContainer` for method chaining. + diff --git a/docs/development/core/public/kibana-plugin-public.contextprovider.md b/docs/development/core/public/kibana-plugin-public.contextprovider.md new file mode 100644 index 00000000000000..2e16e0568c90a4 --- /dev/null +++ b/docs/development/core/public/kibana-plugin-public.contextprovider.md @@ -0,0 +1,18 @@ + + +[Home](./index.md) > [kibana-plugin-public](./kibana-plugin-public.md) > [ContextProvider](./kibana-plugin-public.contextprovider.md) + +## ContextProvider type + +A function that returns a context value for a specific key of given context type. + +Signature: + +```typescript +export declare type ContextProvider = (context: Partial, ...rest: TProviderParameters) => Promise | TContext[TContextName]; +``` + +## Remarks + +This function will be called each time a new context is built for a handler invocation. + diff --git a/docs/development/core/public/kibana-plugin-public.md b/docs/development/core/public/kibana-plugin-public.md index 98b6a8703f5435..037730909dc945 100644 --- a/docs/development/core/public/kibana-plugin-public.md +++ b/docs/development/core/public/kibana-plugin-public.md @@ -34,6 +34,7 @@ The plugin integrates with the core system via lifecycle events: `setup` | [ChromeRecentlyAccessed](./kibana-plugin-public.chromerecentlyaccessed.md) | [APIs](./kibana-plugin-public.chromerecentlyaccessed.md) for recently accessed history. | | [ChromeRecentlyAccessedHistoryItem](./kibana-plugin-public.chromerecentlyaccessedhistoryitem.md) | | | [ChromeStart](./kibana-plugin-public.chromestart.md) | ChromeStart allows plugins to customize the global chrome header UI and enrich the UX with additional information about the current location of the browser. | +| [ContextContainer](./kibana-plugin-public.contextcontainer.md) | An object that handles registration of context providers and building of new context objects. | | [CoreSetup](./kibana-plugin-public.coresetup.md) | Core services exposed to the Plugin setup lifecycle | | [CoreStart](./kibana-plugin-public.corestart.md) | Core services exposed to the Plugin start lifecycle | | [DocLinksStart](./kibana-plugin-public.doclinksstart.md) | | @@ -58,6 +59,7 @@ The plugin integrates with the core system via lifecycle events: `setup` | --- | --- | | [ChromeHelpExtension](./kibana-plugin-public.chromehelpextension.md) | | | [ChromeNavLinkUpdateableFields](./kibana-plugin-public.chromenavlinkupdateablefields.md) | | +| [ContextProvider](./kibana-plugin-public.contextprovider.md) | A function that returns a context value for a specific key of given context type. | | [HttpSetup](./kibana-plugin-public.httpsetup.md) | | | [HttpStart](./kibana-plugin-public.httpstart.md) | | | [PluginInitializer](./kibana-plugin-public.plugininitializer.md) | The plugin export at the root of a plugin's public directory should conform to this interface. | diff --git a/src/core/public/context/context.test.ts b/src/core/public/context/context.test.ts new file mode 100644 index 00000000000000..a78a568a12aedd --- /dev/null +++ b/src/core/public/context/context.test.ts @@ -0,0 +1,178 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { PluginName } from '../../server'; +import { ContextContainerImplementation } from './context'; + +const plugins: ReadonlyMap = new Map([ + ['pluginA', []], + ['pluginB', ['pluginA']], + ['pluginC', ['pluginA', 'pluginB']], + ['pluginD', []], +]); + +interface MyContext { + core1: string; + core2: number; + ctxFromA: string; + ctxFromB: number; + ctxFromC: boolean; + ctxFromD: object; + baseCtx: number; +} + +describe('ContextContainer', () => { + it('does not allow the same context to be registered twice', () => { + const contextContainer = new ContextContainerImplementation(plugins); + contextContainer.register('ctxFromA', () => 'aString', 'pluginA'); + + expect(() => + contextContainer.register('ctxFromA', () => 'aString', 'pluginA') + ).toThrowErrorMatchingInlineSnapshot( + `"Context provider for ctxFromA has already been registered."` + ); + }); + + it('resolves dependencies', async () => { + expect.assertions(8); + const contextContainer = new ContextContainerImplementation(plugins); + + contextContainer + .register('core1', context => { + expect(context).toEqual({}); + return 'core'; + }) + .register( + 'ctxFromA', + context => { + expect(context).toEqual({}); + return 'aString'; + }, + 'pluginA' + ) + .register( + 'ctxFromB', + context => { + expect(context).toEqual({ ctxFromA: 'aString' }); + return 299; + }, + 'pluginB' + ) + .register( + 'ctxFromC', + context => { + expect(context).toEqual({ ctxFromA: 'aString', ctxFromB: 299 }); + return false; + }, + 'pluginC' + ) + .register( + 'ctxFromD', + context => { + expect(context).toEqual({}); + return {}; + }, + 'pluginD' + ); + + // Should have context from pluginC, its deps, and core + expect(await contextContainer.createContext('pluginC')).toEqual({ + core1: 'core', + ctxFromA: 'aString', + ctxFromB: 299, + ctxFromC: false, + }); + + // Should have context from pluginD, and core + expect(await contextContainer.createContext('pluginD')).toEqual({ + core1: 'core', + ctxFromD: {}, + }); + }); + + it('exposes all previously registerd context to Core providers', async () => { + expect.assertions(3); + const contextContainer = new ContextContainerImplementation(plugins); + + contextContainer + .register('core1', context => { + expect(context).toEqual({}); + return 'core'; + }) + .register('core2', context => { + expect(context).toEqual({ core1: 'core' }); + return 101; + }); + + // If no context is registered for pluginA, only core contexts should be exposed + expect(await contextContainer.createContext('pluginA')).toEqual({ + core1: 'core', + core2: 101, + }); + }); + + it('passes additional arguments to providers', async () => { + expect.assertions(5); + const contextContainer = new ContextContainerImplementation( + plugins + ); + + contextContainer + .register('core1', (context, str, num) => { + expect(str).toEqual('passed string'); + expect(num).toEqual(77); + return `core ${str}`; + }) + .register('ctxFromD', (context, str, num) => { + expect(str).toEqual('passed string'); + expect(num).toEqual(77); + return { + num: 77, + }; + }); + + expect(await contextContainer.createContext('pluginD', {}, 'passed string', 77)).toEqual({ + core1: 'core passed string', + ctxFromD: { + num: 77, + }, + }); + }); + + it('passes baseContext to context providers', async () => { + expect.assertions(3); + const contextContainer = new ContextContainerImplementation(plugins); + + contextContainer + .register('core1', context => { + expect(context.baseCtx).toEqual(1234); + return 'core'; + }) + .register('ctxFromD', context => { + expect(context.baseCtx).toEqual(1234); + return {}; + }); + + expect(await contextContainer.createContext('pluginD', { baseCtx: 1234 })).toEqual({ + core1: 'core', + baseCtx: 1234, + ctxFromD: {}, + }); + }); +}); diff --git a/src/core/public/context/context.ts b/src/core/public/context/context.ts new file mode 100644 index 00000000000000..28ee683966906a --- /dev/null +++ b/src/core/public/context/context.ts @@ -0,0 +1,172 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { flatten } from 'lodash'; +import { PluginName } from '../../server'; +import { pick } from '../../utils'; + +/** + * A function that returns a context value for a specific key of given context type. + * + * @remarks + * This function will be called each time a new context is built for a handler invocation. + * + * @param context - A partial context object containing only the keys for values provided by plugin dependencies + * @param rest - Additional parameters provided by the service owner of this context + * @returns The context value associated with this key. May also return a Promise. + * + * @public + */ +export type ContextProvider< + TContext extends {}, + TContextName extends keyof TContext, + TProviderParameters extends any[] = [] +> = ( + context: Partial, + ...rest: TProviderParameters +) => Promise | TContext[TContextName]; + +/** + * An object that handles registration of context providers and building of new context objects. + * + * @remarks + * 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 { + /** + * Register a new context provider. Throws an excpetion 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 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. + * @returns The `ContextContainer` for method chaining. + */ + register( + contextName: TContextName, + provider: ContextProvider, + plugin?: PluginName + ): this; + + /** + * Create a new context. + * + * @param plugin - The plugin the context will be exposed to. + * @param baseContext - The initial context for the given handler. + * @param contextArgs - Additional parameters to call providers with. + * @returns A Promise for the new context object. + */ + createContext( + plugin: PluginName, + baseContext: Partial, + ...contextArgs: TProviderParameters + ): Promise; +} + +/** @internal */ +export class ContextContainerImplementation< + TContext extends {}, + TProviderParameters extends any[] = [] +> implements ContextContainer { + /** + * Used to map contexts to their providers and associated plugin. In registration order which is tightly coupled to + * plugin load order. + */ + private readonly contextProviders = new Map< + keyof TContext, + { + provider: ContextProvider; + plugin?: PluginName; + } + >(); + /** Used to keep track of which plugins registered which contexts for dependency resolution. */ + private readonly contextNamesByPlugin = new Map>(); + + /** + * @param pluginDependencies - A map of plugins to an array of their dependencies. + */ + constructor(private readonly pluginDependencies: ReadonlyMap) {} + + public register( + contextName: TContextName, + provider: ContextProvider, + plugin?: PluginName + ): this { + if (this.contextProviders.has(contextName)) { + throw new Error(`Context provider for ${contextName} has already been registered.`); + } + + this.contextProviders.set(contextName, { provider, plugin }); + + if (plugin) { + this.contextNamesByPlugin.set(plugin, [ + ...(this.contextNamesByPlugin.get(plugin) || []), + contextName, + ]); + } + + return this; + } + + public async createContext( + pluginName: PluginName, + baseContext: Partial = {}, + ...contextArgs: TProviderParameters + ): Promise { + const contextNamesForPlugin = (plug: PluginName): Array => { + const pluginDeps = this.pluginDependencies.get(plug)!; + return 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), + ]); + + return [...this.contextProviders] + .filter( + ([contextName, { plugin }]) => + // Contexts we depend on OR provided by core + neededContexts.has(contextName) || plugin === undefined + ) + .reduce( + async (contextPromise, [contextName, { provider, plugin }]) => { + const resolvedContext = await contextPromise; + + // If the provider is not from a plugin, give access to the entire + // context built so far (this is only possible for providers registered + // by the service owner). + const exposedContext = plugin + ? pick(resolvedContext, contextNamesForPlugin(plugin)) + : resolvedContext; + + return { + ...resolvedContext, + [contextName]: await provider(exposedContext as Partial, ...contextArgs), + }; + }, + Promise.resolve(baseContext) as Promise + ); + } +} diff --git a/src/core/public/context/context_service.mock.ts b/src/core/public/context/context_service.mock.ts new file mode 100644 index 00000000000000..89ef5322fa6295 --- /dev/null +++ b/src/core/public/context/context_service.mock.ts @@ -0,0 +1,41 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { ContextStart, ContextService } from './context_service'; + +const createStartContractMock = () => { + const startContract: jest.Mocked = { + createContextContainer: jest.fn(), + }; + return startContract; +}; + +type ContextServiceContract = PublicMethodsOf; +const createMock = () => { + const mocked: jest.Mocked = { + start: jest.fn(), + }; + mocked.start.mockReturnValue(createStartContractMock()); + return mocked; +}; + +export const contextServiceMock = { + create: createMock, + createStartContract: createStartContractMock, +}; diff --git a/src/core/public/context/context_service.ts b/src/core/public/context/context_service.ts new file mode 100644 index 00000000000000..6d3df6e6aaee26 --- /dev/null +++ b/src/core/public/context/context_service.ts @@ -0,0 +1,62 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { InjectedMetadataStart } from '../injected_metadata'; +import { ContextContainer, ContextContainerImplementation } from './context'; +import { PluginName } from '../../server'; + +interface StartDeps { + injectedMetadata: InjectedMetadataStart; +} + +/** @internal */ +export class ContextService { + public start({ injectedMetadata }: StartDeps): ContextStart { + const plugins = injectedMetadata.getPlugins(); + const allPluginNames = new Set(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[]] + ) + ); + + return { + createContextContainer: () => + new ContextContainerImplementation(pluginDependencies), + }; + } +} + +/** @internal */ +export interface ContextStart { + /** + * Creates a new {@link ContextContainer} for a service owner. + */ + createContextContainer< + TContext extends {}, + TProviderParameters extends any[] = [] + >(): ContextContainer; +} diff --git a/src/core/public/context/index.ts b/src/core/public/context/index.ts new file mode 100644 index 00000000000000..c251e8f1120073 --- /dev/null +++ b/src/core/public/context/index.ts @@ -0,0 +1,21 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export { ContextService, ContextStart } from './context_service'; +export { ContextContainer, ContextProvider } from './context'; diff --git a/src/core/public/core_system.test.mocks.ts b/src/core/public/core_system.test.mocks.ts index 4a96214b3e5def..d2494badfacdb5 100644 --- a/src/core/public/core_system.test.mocks.ts +++ b/src/core/public/core_system.test.mocks.ts @@ -30,6 +30,7 @@ import { pluginsServiceMock } from './plugins/plugins_service.mock'; import { uiSettingsServiceMock } from './ui_settings/ui_settings_service.mock'; import { docLinksServiceMock } from './doc_links/doc_links_service.mock'; import { renderingServiceMock } from './rendering/rendering_service.mock'; +import { contextServiceMock } from './context/context_service.mock'; export const MockLegacyPlatformService = legacyPlatformServiceMock.create(); export const LegacyPlatformServiceConstructor = jest @@ -120,3 +121,9 @@ export const RenderingServiceConstructor = jest.fn().mockImplementation(() => Mo jest.doMock('./rendering', () => ({ RenderingService: RenderingServiceConstructor, })); + +export const MockContextService = contextServiceMock.create(); +export const ContextServiceConstructor = jest.fn().mockImplementation(() => MockContextService); +jest.doMock('./context', () => ({ + ContextService: ContextServiceConstructor, +})); diff --git a/src/core/public/core_system.test.ts b/src/core/public/core_system.test.ts index 044a40b2759936..ca26b6fe45dbad 100644 --- a/src/core/public/core_system.test.ts +++ b/src/core/public/core_system.test.ts @@ -41,6 +41,7 @@ import { MockDocLinksService, MockRenderingService, RenderingServiceConstructor, + MockContextService, } from './core_system.test.mocks'; import { CoreSystem } from './core_system'; @@ -216,6 +217,11 @@ describe('#start()', () => { expect(MockApplicationService.start).toHaveBeenCalledTimes(1); }); + it('calls context#start()', async () => { + await startCore(); + expect(MockContextService.start).toHaveBeenCalledTimes(1); + }); + it('calls docLinks#start()', async () => { await startCore(); expect(MockDocLinksService.start).toHaveBeenCalledTimes(1); diff --git a/src/core/public/core_system.ts b/src/core/public/core_system.ts index f3f466df8a78e4..eed3a25d09a01f 100644 --- a/src/core/public/core_system.ts +++ b/src/core/public/core_system.ts @@ -34,6 +34,7 @@ import { ApplicationService } from './application'; import { mapToObject } from '../utils/'; import { DocLinksService } from './doc_links'; import { RenderingService } from './rendering'; +import { ContextService } from './context'; interface Params { rootDomElement: HTMLElement; @@ -69,6 +70,7 @@ export class CoreSystem { private readonly application: ApplicationService; private readonly docLinks: DocLinksService; private readonly rendering: RenderingService; + private readonly context: ContextService; private readonly rootDomElement: HTMLElement; private fatalErrorsSetup: FatalErrorsSetup | null = null; @@ -103,6 +105,7 @@ export class CoreSystem { this.chrome = new ChromeService({ browserSupportsCsp }); this.docLinks = new DocLinksService(); this.rendering = new RenderingService(); + this.context = new ContextService(); const core: CoreContext = {}; this.plugins = new PluginsService(core); @@ -157,6 +160,7 @@ 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 i18n = await this.i18n.start(); const application = await this.application.start({ injectedMetadata }); @@ -190,6 +194,7 @@ export class CoreSystem { const core: InternalCoreStart = { application, chrome, + context, docLinks, http, i18n, diff --git a/src/core/public/index.ts b/src/core/public/index.ts index 2a88ebf86ab0c5..e56147e8e8f459 100644 --- a/src/core/public/index.ts +++ b/src/core/public/index.ts @@ -66,6 +66,7 @@ import { Plugin, PluginInitializer, PluginInitializerContext } from './plugins'; import { UiSettingsClient, UiSettingsState, UiSettingsClientContract } from './ui_settings'; import { ApplicationSetup, Capabilities, ApplicationStart } from './application'; import { DocLinksStart } from './doc_links'; +import { ContextContainer, ContextProvider, ContextStart } from './context'; export { CoreContext, CoreSystem } from './core_system'; export { RecursiveReadonly } from '../utils'; @@ -127,6 +128,7 @@ export interface InternalCoreSetup extends CoreSetup { /** @internal */ export interface InternalCoreStart extends CoreStart { application: ApplicationStart; + context: ContextStart; injectedMetadata: InjectedMetadataStart; } @@ -146,6 +148,8 @@ export { ChromeRecentlyAccessed, ChromeRecentlyAccessedHistoryItem, ChromeStart, + ContextContainer, + ContextProvider, DocLinksStart, ErrorToastOptions, FatalErrorInfo, diff --git a/src/core/public/legacy/legacy_service.test.ts b/src/core/public/legacy/legacy_service.test.ts index a514bf71540550..b3f4f9f440b388 100644 --- a/src/core/public/legacy/legacy_service.test.ts +++ b/src/core/public/legacy/legacy_service.test.ts @@ -58,6 +58,7 @@ import { uiSettingsServiceMock } from '../ui_settings/ui_settings_service.mock'; import { LegacyPlatformService } from './legacy_service'; import { applicationServiceMock } from '../application/application_service.mock'; import { docLinksServiceMock } from '../doc_links/doc_links_service.mock'; +import { contextServiceMock } from '../context/context_service.mock'; const applicationSetup = applicationServiceMock.createSetupContract(); const fatalErrorsSetup = fatalErrorsServiceMock.createSetupContract(); @@ -85,6 +86,7 @@ const defaultSetupDeps = { }; const applicationStart = applicationServiceMock.createStartContract(); +const contextStart = contextServiceMock.createStartContract(); const docLinksStart = docLinksServiceMock.createStartContract(); const httpStart = httpServiceMock.createStartContract(); const chromeStart = chromeServiceMock.createStartContract(); @@ -97,6 +99,7 @@ const uiSettingsStart = uiSettingsServiceMock.createStartContract(); const defaultStartDeps = { core: { application: applicationStart, + context: contextStart, docLinks: docLinksStart, http: httpStart, chrome: chromeStart, diff --git a/src/core/public/plugins/plugins_service.test.ts b/src/core/public/plugins/plugins_service.test.ts index 55e91bde27cb09..c491f0cf0c361e 100644 --- a/src/core/public/plugins/plugins_service.test.ts +++ b/src/core/public/plugins/plugins_service.test.ts @@ -43,6 +43,7 @@ import { injectedMetadataServiceMock } from '../injected_metadata/injected_metad import { httpServiceMock } from '../http/http_service.mock'; import { CoreSetup, CoreStart } from '..'; import { docLinksServiceMock } from '../doc_links/doc_links_service.mock'; +import { contextServiceMock } from '../context/context_service.mock'; export let mockPluginInitializers: Map; @@ -81,6 +82,7 @@ beforeEach(() => { mockSetupContext = omit(mockSetupDeps, 'application', 'injectedMetadata'); mockStartDeps = { application: applicationServiceMock.createStartContract(), + context: contextServiceMock.createStartContract(), docLinks: docLinksServiceMock.createStartContract(), http: httpServiceMock.createStartContract(), chrome: chromeServiceMock.createStartContract(), @@ -91,7 +93,7 @@ beforeEach(() => { uiSettings: uiSettingsServiceMock.createStartContract(), }; mockStartContext = { - ...omit(mockStartDeps, 'injectedMetadata'), + ...omit(mockStartDeps, 'context', 'injectedMetadata'), application: { capabilities: mockStartDeps.application.capabilities, }, diff --git a/src/core/public/public.api.md b/src/core/public/public.api.md index 55f2a252103211..8fbe8a817079af 100644 --- a/src/core/public/public.api.md +++ b/src/core/public/public.api.md @@ -166,6 +166,17 @@ export interface ChromeStart { setIsVisible(isVisible: boolean): void; } +// @public +export interface ContextContainer { + createContext(plugin: PluginName, baseContext: Partial, ...contextArgs: TProviderParameters): Promise; + // Warning: (ae-forgotten-export) The symbol "PluginName" needs to be exported by the entry point index.d.ts + // Warning: (ae-unresolved-link) The @link reference could not be resolved: The package "kibana" does not have an export "TContext" + register(contextName: TContextName, provider: ContextProvider, plugin?: PluginName): this; +} + +// @public +export type ContextProvider = (context: Partial, ...rest: TProviderParameters) => Promise | TContext[TContextName]; + // @internal (undocumented) export interface CoreContext { } @@ -414,6 +425,10 @@ export interface InternalCoreSetup extends CoreSetup { export interface InternalCoreStart extends CoreStart { // (undocumented) application: ApplicationStart; + // Warning: (ae-forgotten-export) The symbol "ContextStart" needs to be exported by the entry point index.d.ts + // + // (undocumented) + context: ContextStart; // Warning: (ae-forgotten-export) The symbol "InjectedMetadataStart" needs to be exported by the entry point index.d.ts // // (undocumented) From 4306f779d359608921f74c05dd89518ff2de8e7c Mon Sep 17 00:00:00 2001 From: Josh Dover Date: Mon, 15 Jul 2019 12:16:31 -0500 Subject: [PATCH 2/2] PR comments --- ...n-public.contextcontainer.createcontext.md | 4 +- .../kibana-plugin-public.contextcontainer.md | 4 +- ...plugin-public.contextcontainer.register.md | 8 +-- src/core/public/context/context.test.ts | 21 +++++-- src/core/public/context/context.ts | 49 +++++++++------ src/core/public/context/context_service.ts | 19 +----- src/core/public/core_system.ts | 7 ++- .../public/plugins/plugins_service.test.ts | 16 +++++ src/core/public/plugins/plugins_service.ts | 60 +++++++++++-------- src/core/public/public.api.md | 6 +- 10 files changed, 114 insertions(+), 80 deletions(-) diff --git a/docs/development/core/public/kibana-plugin-public.contextcontainer.createcontext.md b/docs/development/core/public/kibana-plugin-public.contextcontainer.createcontext.md index 062f720fd6b8f6..ea7c8dfd151069 100644 --- a/docs/development/core/public/kibana-plugin-public.contextcontainer.createcontext.md +++ b/docs/development/core/public/kibana-plugin-public.contextcontainer.createcontext.md @@ -9,14 +9,14 @@ Create a new context. Signature: ```typescript -createContext(plugin: PluginName, baseContext: Partial, ...contextArgs: TProviderParameters): Promise; +createContext(plugin: string, baseContext: Partial, ...contextArgs: TProviderParameters): Promise; ``` ## Parameters | Parameter | Type | Description | | --- | --- | --- | -| plugin | PluginName | The plugin the context will be exposed to. | +| plugin | string | The plugin the context will be exposed to. | | baseContext | Partial<TContext> | The initial context for the given handler. | | contextArgs | TProviderParameters | Additional parameters to call providers with. | diff --git a/docs/development/core/public/kibana-plugin-public.contextcontainer.md b/docs/development/core/public/kibana-plugin-public.contextcontainer.md index 1c71fa76c53479..ae93119be28e53 100644 --- a/docs/development/core/public/kibana-plugin-public.contextcontainer.md +++ b/docs/development/core/public/kibana-plugin-public.contextcontainer.md @@ -17,9 +17,11 @@ export interface ContextContainerSignature: ```typescript -register(contextName: TContextName, provider: ContextProvider, plugin?: PluginName): this; +register(contextName: TContextName, provider: ContextProvider, plugin?: string): this; ``` ## Parameters | Parameter | Type | Description | | --- | --- | --- | -| contextName | TContextName | The key of the object this provider supplies the value for. | +| contextName | TContextName | The key of the TContext object this provider supplies the value for. | | provider | ContextProvider<TContext, TContextName, TProviderParameters> | A [ContextProvider](./kibana-plugin-public.contextprovider.md) to be called each time a new context is created. | -| plugin | PluginName | The plugin this provider is associated with. If undefined, provider gets access to all provided context keys. | +| plugin | string | The plugin this provider is associated with. If undefined, provider gets access to all provided context keys. Only the service owner should be able to call with undefined. | Returns: diff --git a/src/core/public/context/context.test.ts b/src/core/public/context/context.test.ts index a78a568a12aedd..cdaaa0cd0c1730 100644 --- a/src/core/public/context/context.test.ts +++ 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(plugins); contextContainer @@ -61,7 +61,7 @@ describe('ContextContainer', () => { .register( 'ctxFromA', context => { - expect(context).toEqual({}); + expect(context).toEqual({ core1: 'core' }); return 'aString'; }, 'pluginA' @@ -69,7 +69,7 @@ describe('ContextContainer', () => { .register( 'ctxFromB', context => { - expect(context).toEqual({ ctxFromA: 'aString' }); + expect(context).toEqual({ core1: 'core', ctxFromA: 'aString' }); return 299; }, 'pluginB' @@ -77,7 +77,7 @@ describe('ContextContainer', () => { .register( 'ctxFromC', context => { - expect(context).toEqual({ ctxFromA: 'aString', ctxFromB: 299 }); + expect(context).toEqual({ core1: 'core', ctxFromA: 'aString', ctxFromB: 299 }); return false; }, 'pluginC' @@ -85,7 +85,7 @@ describe('ContextContainer', () => { .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(plugins); @@ -175,4 +175,13 @@ describe('ContextContainer', () => { ctxFromD: {}, }); }); + + it('throws error for unknown plugin', async () => { + const contextContainer = new ContextContainerImplementation(plugins); + await expect( + contextContainer.createContext('otherPlugin') + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"Cannot create context for unknown plugin: otherPlugin"` + ); + }); }); diff --git a/src/core/public/context/context.ts b/src/core/public/context/context.ts index 28ee683966906a..8a42181e90cb68 100644 --- a/src/core/public/context/context.ts +++ b/src/core/public/context/context.ts @@ -46,6 +46,11 @@ 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. * @@ -53,19 +58,19 @@ export type ContextProvider< */ export interface ContextContainer { /** - * 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( contextName: TContextName, provider: ContextProvider, - plugin?: PluginName + plugin?: string ): this; /** @@ -77,7 +82,7 @@ export interface ContextContainer, ...contextArgs: TProviderParameters ): Promise; @@ -133,23 +138,29 @@ export class ContextContainerImplementation< baseContext: Partial = {}, ...contextArgs: TProviderParameters ): Promise { - const contextNamesForPlugin = (plug: PluginName): Array => { - 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 => { + 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 { diff --git a/src/core/public/context/context_service.ts b/src/core/public/context/context_service.ts index 6d3df6e6aaee26..0c52e6601235a7 100644 --- a/src/core/public/context/context_service.ts +++ b/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; } /** @internal */ export class ContextService { - public start({ injectedMetadata }: StartDeps): ContextStart { - const plugins = injectedMetadata.getPlugins(); - const allPluginNames = new Set(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: () => new ContextContainerImplementation(pluginDependencies), diff --git a/src/core/public/core_system.ts b/src/core/public/core_system.ts index eed3a25d09a01f..3d4a153199bb68 100644 --- a/src/core/public/core_system.ts +++ b/src/core/public/core_system.ts @@ -73,6 +73,7 @@ export class CoreSystem { private readonly context: ContextService; private readonly rootDomElement: HTMLElement; + private pluginDependencies?: ReadonlyMap; 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 }); diff --git a/src/core/public/plugins/plugins_service.test.ts b/src/core/public/plugins/plugins_service.test.ts index c491f0cf0c361e..0f2c42cb034d53 100644 --- a/src/core/public/plugins/plugins_service.test.ts +++ b/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); diff --git a/src/core/public/plugins/plugins_service.ts b/src/core/public/plugins/plugins_service.ts index 03725a9d7f8839..ac3ef9870f51ba 100644 --- a/src/core/public/plugins/plugins_service.ts +++ b/src/core/public/plugins/plugins_service.ts @@ -35,11 +35,12 @@ export type PluginsServiceStartDeps = InternalCoreStart; /** @internal */ export interface PluginsServiceSetup { - contracts: Map; + contracts: ReadonlyMap; + pluginDependencies: ReadonlyMap; } /** @internal */ export interface PluginsServiceStart { - contracts: Map; + contracts: ReadonlyMap; } /** @@ -50,24 +51,15 @@ export interface PluginsServiceStart { */ export class PluginsService implements CoreService { /** Plugin wrappers in topological order. */ - private readonly plugins: Map< - PluginName, - PluginWrapper> - > = new Map(); + private readonly plugins = new Map>>(); + private readonly pluginDependencies = new Map(); + private readonly satupPlugins: PluginName[] = []; constructor(private readonly coreContext: CoreContext) {} public async setup(deps: PluginsServiceSetupDeps) { - // Construct plugin wrappers, depending on the topological order set by the server. - deps.injectedMetadata - .getPlugins() - .forEach(({ id, plugin }) => - this.plugins.set( - id, - new PluginWrapper(plugin, createPluginInitializerContext(deps, plugin)) - ) - ); + this.setPluginData(deps); // Load plugin bundles await this.loadPluginBundles(deps.http.basePath.prepend); @@ -75,12 +67,9 @@ export class PluginsService implements CoreService(); for (const [pluginName, plugin] of this.plugins.entries()) { - const pluginDeps = new Set([ - ...plugin.requiredPlugins, - ...plugin.optionalPlugins.filter(optPlugin => this.plugins.get(optPlugin)), - ]); + const pluginDeps = this.pluginDependencies.get(pluginName)!; - const pluginDepContracts = [...pluginDeps.keys()].reduce( + const pluginDepContracts = [...pluginDeps].reduce( (depContracts, dependencyName) => { // Only set if present. Could be absent if plugin does not have client-side code or is a // missing optional plugin. @@ -105,19 +94,16 @@ export class PluginsService implements CoreService(); for (const [pluginName, plugin] of this.plugins.entries()) { - const pluginDeps = new Set([ - ...plugin.requiredPlugins, - ...plugin.optionalPlugins.filter(optPlugin => this.plugins.get(optPlugin)), - ]); + const pluginDeps = this.pluginDependencies.get(pluginName)!; - const pluginDepContracts = [...pluginDeps.keys()].reduce( + const pluginDepContracts = [...pluginDeps].reduce( (depContracts, dependencyName) => { // Only set if present. Could be absent if plugin does not have client-side code or is a // missing optional plugin. @@ -154,4 +140,26 @@ export class PluginsService implements CoreService plugin.load(addBasePath))); } + + private setPluginData(deps: PluginsServiceSetupDeps) { + // Construct plugin wrappers, depending on the topological order set by the server. + deps.injectedMetadata + .getPlugins() + .forEach(({ id, plugin }) => + this.plugins.set( + id, + new PluginWrapper(plugin, createPluginInitializerContext(deps, plugin)) + ) + ); + + // Setup map of dependencies + const plugins = deps.injectedMetadata.getPlugins(); + const allPluginNames = new Set(plugins.map(p => p.id)); + plugins.forEach(({ id, plugin }) => + this.pluginDependencies.set(id, [ + ...plugin.requiredPlugins, + ...plugin.optionalPlugins.filter(optPlugin => allPluginNames.has(optPlugin)), + ]) + ); + } } diff --git a/src/core/public/public.api.md b/src/core/public/public.api.md index 8fbe8a817079af..722048cd9c0a01 100644 --- a/src/core/public/public.api.md +++ b/src/core/public/public.api.md @@ -168,10 +168,8 @@ export interface ChromeStart { // @public export interface ContextContainer { - createContext(plugin: PluginName, baseContext: Partial, ...contextArgs: TProviderParameters): Promise; - // Warning: (ae-forgotten-export) The symbol "PluginName" needs to be exported by the entry point index.d.ts - // Warning: (ae-unresolved-link) The @link reference could not be resolved: The package "kibana" does not have an export "TContext" - register(contextName: TContextName, provider: ContextProvider, plugin?: PluginName): this; + createContext(plugin: string, baseContext: Partial, ...contextArgs: TProviderParameters): Promise; + register(contextName: TContextName, provider: ContextProvider, plugin?: string): this; } // @public