Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inject UI plugins into injectedMetadata #31840

Merged
merged 13 commits into from
Mar 20, 2019
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* under the License.
*/

import { DiscoveredPlugin } from 'src/core/server';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a TypeScript feature to allow imports from the root of the project like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah I think my editor did that automatically, I'll make it relative.

import { InjectedMetadataService } from './injected_metadata_service';

describe('#getKibanaVersion', () => {
Expand Down Expand Up @@ -76,6 +77,43 @@ describe('start.getCspConfig()', () => {
});
});

describe('start.getPlugins()', () => {
it('returns injectedMetadata.uiPlugins', () => {
const injectedMetadata = new InjectedMetadataService({
injectedMetadata: {
uiPlugins: [{ id: 'plugin-1', plugin: {} }, { id: 'plugin-2', plugin: {} }],
},
} as any);

const plugins = injectedMetadata.start().getPlugins();
expect(plugins).toEqual([{ id: 'plugin-1', plugin: {} }, { id: 'plugin-2', plugin: {} }]);
});

it('returns frozen version of uiPlugins', () => {
const injectedMetadata = new InjectedMetadataService({
injectedMetadata: {
uiPlugins: [{ id: 'plugin-1', plugin: {} }, { id: 'plugin-2', plugin: {} }],
},
} as any);

const plugins = injectedMetadata.start().getPlugins();
expect(() => {
plugins.pop();
}).toThrowError();
expect(() => {
plugins.push({ id: 'new-plugin', plugin: {} as DiscoveredPlugin });
}).toThrowError();
expect(() => {
// @ts-ignore TS knows this shouldn't be possible
plugins[0].name = 'changed';
}).toThrowError();
expect(() => {
// @ts-ignore TS knows this shouldn't be possible
plugins[0].newProp = 'changed';
}).toThrowError();
});
});

describe('start.getLegacyMetadata()', () => {
it('returns injectedMetadata.legacyMetadata', () => {
const injectedMetadata = new InjectedMetadataService({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/

import { get } from 'lodash';
import { DiscoveredPlugin, PluginName } from 'src/core/server';
import { UiSettingsState } from '../ui_settings';
import { deepFreeze } from './deep_freeze';

Expand All @@ -32,6 +33,10 @@ export interface InjectedMetadataParams {
vars: {
[key: string]: unknown;
};
uiPlugins: Array<{
id: PluginName;
plugin: DiscoveredPlugin;
}>;
legacyMetadata: {
app: unknown;
translations: unknown;
Expand Down Expand Up @@ -77,6 +82,10 @@ export class InjectedMetadataService {
return this.state.csp;
},

getPlugins: () => {
return this.state.uiPlugins;
},

getLegacyMetadata: () => {
return this.state.legacyMetadata;
},
Expand Down
1 change: 1 addition & 0 deletions src/core/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import { PluginsModule } from './plugins';

export { bootstrap } from './bootstrap';
export { DiscoveredPlugin, PluginName } from './plugins';

import { first } from 'rxjs/operators';
import { ConfigService, Env } from './config';
Expand Down
10 changes: 7 additions & 3 deletions src/core/server/legacy/legacy_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { getEnvOptions } from '../config/__mocks__/env';
import { configServiceMock } from '../config/config_service.mock';
import { ElasticsearchServiceStart } from '../elasticsearch';
import { loggingServiceMock } from '../logging/logging_service.mock';
import { DiscoveredPlugin } from '../plugins/plugin';
import { PluginsServiceStart } from '../plugins/plugins_service';
import { LegacyPlatformProxy } from './legacy_platform_proxy';

Expand Down Expand Up @@ -62,7 +63,10 @@ beforeEach(() => {
server: { listener: { addListener: jest.fn() }, route: jest.fn() },
options: { someOption: 'foo', someAnotherOption: 'bar' },
},
plugins: new Map([['plugin-id', 'plugin-value']]),
plugins: {
contracts: new Map([['plugin-id', 'plugin-value']]),
uiPlugins: new Map([['plugin-id', {} as DiscoveredPlugin]]),
},
};

config$ = new BehaviorSubject<Config>(
Expand Down Expand Up @@ -341,7 +345,7 @@ describe('once LegacyService is started in `devClusterMaster` mode', () => {

await devClusterLegacyService.start({
elasticsearch: startDeps.elasticsearch,
plugins: new Map(),
plugins: { contracts: new Map(), uiPlugins: new Map() },
});

expect(MockClusterManager.create.mock.calls).toMatchSnapshot(
Expand All @@ -363,7 +367,7 @@ describe('once LegacyService is started in `devClusterMaster` mode', () => {

await devClusterLegacyService.start({
elasticsearch: startDeps.elasticsearch,
plugins: new Map(),
plugins: { contracts: new Map(), uiPlugins: new Map() },
});

expect(MockClusterManager.create.mock.calls).toMatchSnapshot(
Expand Down
4 changes: 3 additions & 1 deletion src/core/server/plugins/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ import { PluginsService } from './plugins_service';

/** @internal */
export { isNewPlatformPlugin } from './discovery';
export { PluginInitializerContext, PluginStartContext } from './plugin_context';
/** @internal */
export { DiscoveredPlugin } from './plugin';
export { PluginName } from './plugin';
export { PluginInitializerContext, PluginStartContext } from './plugin_context';

/** @internal */
export class PluginsModule {
Expand Down
1 change: 1 addition & 0 deletions src/core/server/plugins/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { getEnvOptions } from '../config/__mocks__/env';
import { CoreContext } from '../core_context';
import { elasticsearchServiceMock } from '../elasticsearch/elasticsearch_service.mock';
import { loggingServiceMock } from '../logging/logging_service.mock';

import { Plugin, PluginManifest } from './plugin';
import { createPluginInitializerContext, createPluginStartContext } from './plugin_context';

Expand Down
37 changes: 36 additions & 1 deletion src/core/server/plugins/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,41 @@ export interface PluginManifest {
readonly server: boolean;
}

/**
* Small container object used to expose information about discovered plugins that may
* or may not have been started.
* @internal
*/
export interface DiscoveredPlugin {
/**
* Identifier of the plugin.
*/
readonly id: PluginName;

/**
* Path on the filesystem where plugin was loaded from.
*/
readonly path: string;

/**
* Root configuration path used by the plugin, defaults to "id".
*/
readonly configPath: ConfigPath;

/**
* An optional list of the other plugins that **must be** installed and enabled
* for this plugin to function properly.
*/
readonly requiredPlugins: ReadonlyArray<PluginName>;

/**
* An optional list of the other plugins that if installed and enabled **may be**
* leveraged by this plugin for some additional functionality but otherwise are
* not required for this plugin to work properly.
*/
readonly optionalPlugins: ReadonlyArray<PluginName>;
}

type PluginInitializer<TExposed, TDependencies extends Record<PluginName, unknown>> = (
coreContext: PluginInitializerContext
) => {
Expand Down Expand Up @@ -109,7 +144,7 @@ export class Plugin<

constructor(
public readonly path: string,
private readonly manifest: PluginManifest,
public readonly manifest: PluginManifest,
private readonly initializerContext: PluginInitializerContext
) {
this.log = initializerContext.logger.get();
Expand Down
15 changes: 11 additions & 4 deletions src/core/server/plugins/plugins_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ test('`start` properly detects plugins that should be disabled.', async () => {
.mockImplementation(path => Promise.resolve(!path.includes('disabled')));

mockPluginSystem.startPlugins.mockResolvedValue(new Map());
mockPluginSystem.uiPlugins.mockReturnValue(new Map());

mockDiscover.mockReturnValue({
error$: from([]),
Expand Down Expand Up @@ -224,7 +225,10 @@ test('`start` properly detects plugins that should be disabled.', async () => {
]),
});

expect(await pluginsService.start(startDeps)).toBeInstanceOf(Map);
const start = await pluginsService.start(startDeps);

expect(start.contracts).toBeInstanceOf(Map);
expect(start.uiPlugins).toBeInstanceOf(Map);
expect(mockPluginSystem.addPlugin).not.toHaveBeenCalled();
expect(mockPluginSystem.startPlugins).toHaveBeenCalledTimes(1);
expect(mockPluginSystem.startPlugins).toHaveBeenCalledWith(startDeps);
Expand Down Expand Up @@ -287,12 +291,15 @@ test('`start` properly invokes `discover` and ignores non-critical errors.', asy
plugin$: from([firstPlugin, secondPlugin]),
});

const pluginsStart = new Map();
mockPluginSystem.startPlugins.mockResolvedValue(pluginsStart);
const contracts = new Map();
const discoveredPlugins = new Map();
mockPluginSystem.startPlugins.mockResolvedValue(contracts);
mockPluginSystem.uiPlugins.mockReturnValue(discoveredPlugins);

const start = await pluginsService.start(startDeps);

expect(start).toBe(pluginsStart);
expect(start.contracts).toBe(contracts);
expect(start.uiPlugins).toBe(discoveredPlugins);
expect(mockPluginSystem.addPlugin).toHaveBeenCalledTimes(2);
expect(mockPluginSystem.addPlugin).toHaveBeenCalledWith(firstPlugin);
expect(mockPluginSystem.addPlugin).toHaveBeenCalledWith(secondPlugin);
Expand Down
17 changes: 13 additions & 4 deletions src/core/server/plugins/plugins_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,15 @@ import { CoreContext } from '../core_context';
import { ElasticsearchServiceStart } from '../elasticsearch';
import { Logger } from '../logging';
import { discover, PluginDiscoveryError, PluginDiscoveryErrorType } from './discovery';
import { Plugin, PluginName } from './plugin';
import { DiscoveredPlugin, Plugin, PluginName } from './plugin';
import { PluginsConfig } from './plugins_config';
import { PluginsSystem } from './plugins_system';

/** @internal */
export type PluginsServiceStart = Map<PluginName, unknown>;
export interface PluginsServiceStart {
contracts: Map<PluginName, unknown>;
uiPlugins: Map<PluginName, DiscoveredPlugin>;
}

/** @internal */
export interface PluginsServiceStartDeps {
Expand Down Expand Up @@ -60,10 +63,16 @@ export class PluginsService implements CoreService<PluginsServiceStart> {

if (!config.initialize || this.coreContext.env.isDevClusterMaster) {
this.log.info('Plugin initialization disabled.');
return new Map();
return {
contracts: new Map(),
uiPlugins: this.pluginsSystem.uiPlugins(),
joshdover marked this conversation as resolved.
Show resolved Hide resolved
};
}

return await this.pluginsSystem.startPlugins(deps);
return {
contracts: await this.pluginsSystem.startPlugins(deps),
uiPlugins: this.pluginsSystem.uiPlugins(),
};
}

public async stop() {
Expand Down
76 changes: 76 additions & 0 deletions src/core/server/plugins/plugins_system.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,3 +232,79 @@ Array [
expect(secondPluginNotToRun.start).not.toHaveBeenCalled();
expect(thirdPluginToRun.start).toHaveBeenCalledTimes(1);
});

test('`uiPlugins` returns empty Map before plugins are added', async () => {
expect(pluginsSystem.uiPlugins()).toMatchInlineSnapshot(`Map {}`);
});

test('`uiPlugins` returns ordered Map of all plugin manifests', async () => {
const plugins = new Map([
[createPlugin('order-4', { required: ['order-2'] }), { 'order-2': 'added-as-2' }],
[createPlugin('order-0'), {}],
[
createPlugin('order-2', { required: ['order-1'], optional: ['order-0'] }),
{ 'order-1': 'added-as-3', 'order-0': 'added-as-1' },
],
[createPlugin('order-1', { required: ['order-0'] }), { 'order-0': 'added-as-1' }],
[
createPlugin('order-3', { required: ['order-2'], optional: ['missing-dep'] }),
{ 'order-2': 'added-as-2' },
],
] as Array<[Plugin, Record<PluginName, unknown>]>);

[...plugins.keys()].forEach(plugin => {
pluginsSystem.addPlugin(plugin);
});

expect(pluginsSystem.uiPlugins()).toMatchInlineSnapshot(`
Map {
"order-0" => Object {
"configPath": "path",
"id": "order-0",
"optionalPlugins": Array [],
"path": "some-path",
"requiredPlugins": Array [],
},
"order-1" => Object {
"configPath": "path",
"id": "order-1",
"optionalPlugins": Array [],
"path": "some-path",
"requiredPlugins": Array [
"order-0",
],
},
"order-2" => Object {
"configPath": "path",
"id": "order-2",
"optionalPlugins": Array [
"order-0",
],
"path": "some-path",
"requiredPlugins": Array [
"order-1",
],
},
"order-3" => Object {
"configPath": "path",
"id": "order-3",
"optionalPlugins": Array [
"missing-dep",
],
"path": "some-path",
"requiredPlugins": Array [
"order-2",
],
},
"order-4" => Object {
"configPath": "path",
"id": "order-4",
"optionalPlugins": Array [],
"path": "some-path",
"requiredPlugins": Array [
"order-2",
],
},
}
`);
});
Loading