Skip to content

Commit

Permalink
[new-platform] Improve naming and consistency in Plugin types (#34725)
Browse files Browse the repository at this point in the history
* Improve consistency in Plugin types

* Use #has() instead of undefined check

* Rename parameter

* Update core docs

* Internal updates
  • Loading branch information
joshdover authored Apr 10, 2019
1 parent 2a6c7b4 commit b006361
Show file tree
Hide file tree
Showing 23 changed files with 204 additions and 109 deletions.
4 changes: 2 additions & 2 deletions docs/development/core/public/kibana-plugin-public.plugin.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ The interface that should be returned by a `PluginInitializer`<!-- -->.
<b>Signature:</b>

```typescript
export interface Plugin<TSetup, TDependencies extends Record<string, unknown> =
export interface Plugin<TSetup, TPluginsSetup extends Record<string, unknown> =
```
## Properties
| Property | Type | Description |
| --- | --- | --- |
| [setup](./kibana-plugin-public.plugin.setup.md) | <code>(core: PluginSetupContext, dependencies: TDependencies) =&gt; TSetup &#124; Promise&lt;TSetup&gt;</code> | |
| [setup](./kibana-plugin-public.plugin.setup.md) | <code>(core: PluginSetupContext, plugins: TPluginsSetup) =&gt; TSetup &#124; Promise&lt;TSetup&gt;</code> | |
| [stop](./kibana-plugin-public.plugin.stop.md) | <code>() =&gt; void</code> | |
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
<b>Signature:</b>

```typescript
setup: (core: PluginSetupContext, dependencies: TDependencies) => TSetup | Promise<TSetup>;
setup: (core: PluginSetupContext, plugins: TPluginsSetup) => TSetup | Promise<TSetup>;
```
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ The `plugin` export at the root of a plugin's `public` directory should conform
<b>Signature:</b>

```typescript
export declare type PluginInitializer<TSetup, TDependencies extends Record<string, unknown> = {}> = (core: PluginInitializerContext) => Plugin<TSetup, TDependencies>;
export declare type PluginInitializer<TSetup, TPluginsSetup extends Record<string, unknown> = {}> = (core: PluginInitializerContext) => Plugin<TSetup, TPluginsSetup>;
```
2 changes: 2 additions & 0 deletions docs/development/core/server/kibana-plugin-server.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
| [Logger](./kibana-plugin-server.logger.md) | Logger exposes all the necessary methods to log any type of information and this is the interface used by the logging consumers including plugins. |
| [LoggerFactory](./kibana-plugin-server.loggerfactory.md) | The single purpose of <code>LoggerFactory</code> interface is to define a way to retrieve a context-based logger instance. |
| [LogMeta](./kibana-plugin-server.logmeta.md) | Contextual metadata |
| [Plugin](./kibana-plugin-server.plugin.md) | The interface that should be returned by a <code>PluginInitializer</code>. |
| [PluginInitializerContext](./kibana-plugin-server.plugininitializercontext.md) | Context that's available to plugins during initialization stage. |
| [PluginSetupContext](./kibana-plugin-server.pluginsetupcontext.md) | Context passed to the plugins <code>setup</code> method. |

Expand All @@ -30,5 +31,6 @@
| [APICaller](./kibana-plugin-server.apicaller.md) | |
| [ElasticsearchClientConfig](./kibana-plugin-server.elasticsearchclientconfig.md) | |
| [Headers](./kibana-plugin-server.headers.md) | |
| [PluginInitializer](./kibana-plugin-server.plugininitializer.md) | The <code>plugin</code> export at the root of a plugin's <code>server</code> directory should conform to this interface. |
| [PluginName](./kibana-plugin-server.pluginname.md) | Dedicated type for plugin name/id that is supposed to make Map/Set/Arrays that use it as a key or value more obvious. |

19 changes: 19 additions & 0 deletions docs/development/core/server/kibana-plugin-server.plugin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
[Home](./index) &gt; [kibana-plugin-server](./kibana-plugin-server.md) &gt; [Plugin](./kibana-plugin-server.plugin.md)

## Plugin interface

The interface that should be returned by a `PluginInitializer`<!-- -->.

<b>Signature:</b>

```typescript
export interface Plugin<TSetup, TPluginsSetup extends Record<PluginName, unknown> =
```
## Properties
| Property | Type | Description |
| --- | --- | --- |
| [setup](./kibana-plugin-server.plugin.setup.md) | <code>(pluginSetupContext: PluginSetupContext, plugins: TPluginsSetup) =&gt; TSetup &#124; Promise&lt;TSetup&gt;</code> | |
| [stop](./kibana-plugin-server.plugin.stop.md) | <code>() =&gt; void</code> | |
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[Home](./index) &gt; [kibana-plugin-server](./kibana-plugin-server.md) &gt; [Plugin](./kibana-plugin-server.plugin.md) &gt; [setup](./kibana-plugin-server.plugin.setup.md)

## Plugin.setup property

<b>Signature:</b>

```typescript
setup: (pluginSetupContext: PluginSetupContext, plugins: TPluginsSetup) => TSetup | Promise<TSetup>;
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[Home](./index) &gt; [kibana-plugin-server](./kibana-plugin-server.md) &gt; [Plugin](./kibana-plugin-server.plugin.md) &gt; [stop](./kibana-plugin-server.plugin.stop.md)

## Plugin.stop property

<b>Signature:</b>

```typescript
stop?: () => void;
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[Home](./index) &gt; [kibana-plugin-server](./kibana-plugin-server.md) &gt; [PluginInitializer](./kibana-plugin-server.plugininitializer.md)

## PluginInitializer type

The `plugin` export at the root of a plugin's `server` directory should conform to this interface.

<b>Signature:</b>

```typescript
export declare type PluginInitializer<TSetup, TPluginsSetup extends Record<PluginName, unknown> = {}> = (coreContext: PluginInitializerContext) => Plugin<TSetup, TPluginsSetup>;
```
6 changes: 3 additions & 3 deletions src/core/public/kibana.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -176,15 +176,15 @@ export interface OverlaySetup {
}

// @public
export interface Plugin<TSetup, TDependencies extends Record<string, unknown> = {}> {
export interface Plugin<TSetup, TPluginsSetup extends Record<string, unknown> = {}> {
// (undocumented)
setup: (core: PluginSetupContext, dependencies: TDependencies) => TSetup | Promise<TSetup>;
setup: (core: PluginSetupContext, plugins: TPluginsSetup) => TSetup | Promise<TSetup>;
// (undocumented)
stop?: () => void;
}

// @public
export type PluginInitializer<TSetup, TDependencies extends Record<string, unknown> = {}> = (core: PluginInitializerContext) => Plugin<TSetup, TDependencies>;
export type PluginInitializer<TSetup, TPluginsSetup extends Record<string, unknown> = {}> = (core: PluginInitializerContext) => Plugin<TSetup, TPluginsSetup>;

// @public
export interface PluginInitializerContext {
Expand Down
30 changes: 15 additions & 15 deletions src/core/public/plugins/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ import { loadPluginBundle } from './plugin_loader';
*
* @public
*/
export interface Plugin<TSetup, TDependencies extends Record<string, unknown> = {}> {
setup: (core: PluginSetupContext, dependencies: TDependencies) => TSetup | Promise<TSetup>;
export interface Plugin<TSetup, TPluginsSetup extends Record<string, unknown> = {}> {
setup: (core: PluginSetupContext, plugins: TPluginsSetup) => TSetup | Promise<TSetup>;
stop?: () => void;
}

Expand All @@ -37,9 +37,9 @@ export interface Plugin<TSetup, TDependencies extends Record<string, unknown> =
*
* @public
*/
export type PluginInitializer<TSetup, TDependencies extends Record<string, unknown> = {}> = (
export type PluginInitializer<TSetup, TPluginsSetup extends Record<string, unknown> = {}> = (
core: PluginInitializerContext
) => Plugin<TSetup, TDependencies>;
) => Plugin<TSetup, TPluginsSetup>;

/**
* Lightweight wrapper around discovered plugin that is responsible for instantiating
Expand All @@ -49,23 +49,23 @@ export type PluginInitializer<TSetup, TDependencies extends Record<string, unkno
*/
export class PluginWrapper<
TSetup = unknown,
TDependenciesSetup extends Record<PluginName, unknown> = Record<PluginName, unknown>
TPluginsSetup extends Record<PluginName, unknown> = Record<PluginName, unknown>
> {
public readonly name: DiscoveredPlugin['id'];
public readonly configPath: DiscoveredPlugin['configPath'];
public readonly requiredDependencies: DiscoveredPlugin['requiredPlugins'];
public readonly optionalDependencies: DiscoveredPlugin['optionalPlugins'];
private initializer?: PluginInitializer<TSetup, TDependenciesSetup>;
private instance?: Plugin<TSetup, TDependenciesSetup>;
public readonly requiredPlugins: DiscoveredPlugin['requiredPlugins'];
public readonly optionalPlugins: DiscoveredPlugin['optionalPlugins'];
private initializer?: PluginInitializer<TSetup, TPluginsSetup>;
private instance?: Plugin<TSetup, TPluginsSetup>;

constructor(
readonly discoveredPlugin: DiscoveredPlugin,
private readonly initializerContext: PluginInitializerContext
) {
this.name = discoveredPlugin.id;
this.configPath = discoveredPlugin.configPath;
this.requiredDependencies = discoveredPlugin.requiredPlugins;
this.optionalDependencies = discoveredPlugin.optionalPlugins;
this.requiredPlugins = discoveredPlugin.requiredPlugins;
this.optionalPlugins = discoveredPlugin.optionalPlugins;
}

/**
Expand All @@ -74,20 +74,20 @@ export class PluginWrapper<
* @param addBasePath Function that adds the base path to a string for plugin bundle path.
*/
public async load(addBasePath: (path: string) => string) {
this.initializer = await loadPluginBundle<TSetup, TDependenciesSetup>(addBasePath, this.name);
this.initializer = await loadPluginBundle<TSetup, TPluginsSetup>(addBasePath, this.name);
}

/**
* Instantiates plugin and calls `setup` function exposed by the plugin initializer.
* @param setupContext Context that consists of various core services tailored specifically
* for the `setup` lifecycle event.
* @param dependencies The dictionary where the key is the dependency name and the value
* @param plugins The dictionary where the key is the dependency name and the value
* is the contract returned by the dependency's `setup` function.
*/
public async setup(setupContext: PluginSetupContext, dependencies: TDependenciesSetup) {
public async setup(setupContext: PluginSetupContext, plugins: TPluginsSetup) {
this.instance = await this.createPluginInstance();

return await this.instance.setup(setupContext, dependencies);
return await this.instance.setup(setupContext, plugins);
}

/**
Expand Down
25 changes: 13 additions & 12 deletions src/core/public/plugins/plugins_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,34 +62,35 @@ export class PluginsService implements CoreService<PluginsServiceSetup> {
// Load plugin bundles
await this.loadPluginBundles(deps.basePath.addToPath);

// Setup each plugin with correct dependencies
// Setup each plugin with required and optional plugin contracts
const contracts = new Map<string, unknown>();
for (const [pluginName, plugin] of this.plugins.entries()) {
const dependencies = new Set([
...plugin.requiredDependencies,
...plugin.optionalDependencies.filter(optPlugin => this.plugins.get(optPlugin)),
const pluginDeps = new Set([
...plugin.requiredPlugins,
...plugin.optionalPlugins.filter(optPlugin => this.plugins.get(optPlugin)),
]);

const dependencyContracts = [...dependencies.keys()].reduce(
(depContracts, dependency) => {
const pluginDepContracts = [...pluginDeps.keys()].reduce(
(depContracts, dependencyName) => {
// Only set if present. Could be absent if plugin does not have client-side code or is a
// missing optional dependency.
if (contracts.get(dependency) !== undefined) {
depContracts[dependency] = contracts.get(dependency);
// missing optional plugin.
if (contracts.has(dependencyName)) {
depContracts[dependencyName] = contracts.get(dependencyName);
}

return depContracts;
},
{} as { [dep: string]: unknown }
{} as Record<PluginName, unknown>
);

contracts.set(
pluginName,
await plugin.setup(
createPluginSetupContext(this.coreContext, deps, plugin),
dependencyContracts
pluginDepContracts
)
);

this.satupPlugins.push(pluginName);
}

Expand All @@ -98,7 +99,7 @@ export class PluginsService implements CoreService<PluginsServiceSetup> {
}

public async stop() {
// Stop plugins in reverse dependency order.
// Stop plugins in reverse topological order.
for (const pluginName of this.satupPlugins.reverse()) {
this.plugins.get(pluginName)!.stop();
}
Expand Down
3 changes: 3 additions & 0 deletions src/core/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,11 @@ export {
APICaller,
} from './elasticsearch';
export { Logger, LoggerFactory, LogMeta, LogRecord, LogLevel } from './logging';

export {
DiscoveredPlugin,
Plugin,
PluginInitializer,
PluginInitializerContext,
PluginName,
PluginSetupContext,
Expand Down
11 changes: 11 additions & 0 deletions src/core/server/kibana.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,17 @@ export interface LogRecord {
timestamp: Date;
}

// @public
export interface Plugin<TSetup, TPluginsSetup extends Record<PluginName, unknown> = {}> {
// (undocumented)
setup: (pluginSetupContext: PluginSetupContext, plugins: TPluginsSetup) => TSetup | Promise<TSetup>;
// (undocumented)
stop?: () => void;
}

// @public
export type PluginInitializer<TSetup, TPluginsSetup extends Record<PluginName, unknown> = {}> = (coreContext: PluginInitializerContext) => Plugin<TSetup, TPluginsSetup>;

// @public
export interface PluginInitializerContext {
// (undocumented)
Expand Down
8 changes: 4 additions & 4 deletions src/core/server/plugins/discovery/plugin_discovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { first, map, toArray } from 'rxjs/operators';
import { Config, ConfigService, Env, ObjectToConfigAdapter } from '../../config';
import { getEnvOptions } from '../../config/__mocks__/env';
import { loggingServiceMock } from '../../logging/logging_service.mock';
import { Plugin } from '../plugin';
import { PluginWrapper } from '../plugin';
import { PluginsConfig } from '../plugins_config';
import { discover } from './plugins_discovery';

Expand Down Expand Up @@ -142,10 +142,10 @@ test('properly iterates through plugin search locations', async () => {
TEST_EXTRA_PLUGIN_PATH,
]) {
const discoveredPlugin = plugins.find(plugin => plugin.path === path)!;
expect(discoveredPlugin).toBeInstanceOf(Plugin);
expect(discoveredPlugin).toBeInstanceOf(PluginWrapper);
expect(discoveredPlugin.configPath).toEqual(['core', 'config']);
expect(discoveredPlugin.requiredDependencies).toEqual(['a', 'b']);
expect(discoveredPlugin.optionalDependencies).toEqual(['c', 'd']);
expect(discoveredPlugin.requiredPlugins).toEqual(['a', 'b']);
expect(discoveredPlugin.optionalPlugins).toEqual(['c', 'd']);
}

await expect(
Expand Down
14 changes: 10 additions & 4 deletions src/core/server/plugins/discovery/plugins_discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { bindNodeCallback, from, merge } from 'rxjs';
import { catchError, filter, map, mergeMap, shareReplay } from 'rxjs/operators';
import { CoreContext } from '../../core_context';
import { Logger } from '../../logging';
import { Plugin } from '../plugin';
import { PluginWrapper } from '../plugin';
import { createPluginInitializerContext } from '../plugin_context';
import { PluginsConfig } from '../plugins_config';
import { PluginDiscoveryError } from './plugin_discovery_error';
Expand Down Expand Up @@ -67,9 +67,11 @@ export function discover(config: PluginsConfig, coreContext: CoreContext) {
);

return {
plugin$: discoveryResults$.pipe(filter((entry): entry is Plugin => entry instanceof Plugin)),
plugin$: discoveryResults$.pipe(
filter((entry): entry is PluginWrapper => entry instanceof PluginWrapper)
),
error$: discoveryResults$.pipe(
filter((entry): entry is PluginDiscoveryError => !(entry instanceof Plugin))
filter((entry): entry is PluginDiscoveryError => !(entry instanceof PluginWrapper))
),
};
}
Expand Down Expand Up @@ -115,7 +117,11 @@ function createPlugin$(path: string, log: Logger, coreContext: CoreContext) {
return from(parseManifest(path, coreContext.env.packageInfo)).pipe(
map(manifest => {
log.debug(`Successfully discovered plugin "${manifest.id}" at "${path}"`);
return new Plugin(path, manifest, createPluginInitializerContext(coreContext, manifest));
return new PluginWrapper(
path,
manifest,
createPluginInitializerContext(coreContext, manifest)
);
}),
catchError(err => [err])
);
Expand Down
8 changes: 7 additions & 1 deletion src/core/server/plugins/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,11 @@ export { PluginsService, PluginsServiceSetup } from './plugins_service';
/** @internal */
export { isNewPlatformPlugin } from './discovery';
/** @internal */
export { DiscoveredPlugin, DiscoveredPluginInternal, PluginName } from './plugin';
export {
DiscoveredPlugin,
DiscoveredPluginInternal,
Plugin,
PluginInitializer,
PluginName,
} from './plugin';
export { PluginInitializerContext, PluginSetupContext } from './plugin_context';
Loading

0 comments on commit b006361

Please sign in to comment.