Skip to content

Commit

Permalink
Inject UI plugins into injectedMetadata (elastic#31840)
Browse files Browse the repository at this point in the history
This modifies the PluginService to expose a Map of UI plugins and their manifests, in topological order by their dependencies. That data is then exposed to injectedMetadata via the ui_render_mixin in the legacy platform, and read back out via the new platform's client-side InjectedMetadataService, preserving the topological order.
  • Loading branch information
joshdover committed Mar 20, 2019
1 parent 5c6f594 commit 1bc205f
Show file tree
Hide file tree
Showing 14 changed files with 225 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const createStartContractMock = () => {
getKibanaVersion: jest.fn(),
getCspConfig: jest.fn(),
getLegacyMetadata: jest.fn(),
getPlugins: jest.fn(),
getInjectedVar: jest.fn(),
getInjectedVars: jest.fn(),
};
Expand All @@ -35,6 +36,7 @@ const createStartContractMock = () => {
user: { legacyInjectedUiSettingUserValues: true },
},
} as any);
startContract.getPlugins.mockReturnValue([]);
return startContract;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* under the License.
*/

import { DiscoveredPlugin } from '../../server';
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 '../../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 @@ -79,6 +84,10 @@ export class InjectedMetadataService {
return this.state.csp;
},

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

getLegacyMetadata: () => {
return this.state.legacyMetadata;
},
Expand Down
7 changes: 6 additions & 1 deletion src/core/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,9 @@
export { bootstrap } from './bootstrap';
export { CallAPIOptions, ClusterClient } from './elasticsearch';
export { Logger, LoggerFactory } from './logging';
export { PluginInitializerContext, PluginName, PluginStartContext } from './plugins';
export {
DiscoveredPlugin,
PluginInitializerContext,
PluginName,
PluginStartContext,
} from './plugins';
13 changes: 10 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, DiscoveredPluginInternal } from '../plugins';
import { PluginsServiceStart } from '../plugins/plugins_service';
import { LegacyPlatformProxy } from './legacy_platform_proxy';

Expand Down Expand Up @@ -62,7 +63,13 @@ 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: {
public: new Map([['plugin-id', {} as DiscoveredPlugin]]),
internal: new Map([['plugin-id', {} as DiscoveredPluginInternal]]),
},
},
};

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

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

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

await devClusterLegacyService.start({
elasticsearch: startDeps.elasticsearch,
plugins: new Map(),
plugins: { contracts: new Map(), uiPlugins: { public: new Map(), internal: 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, DiscoveredPluginInternal } 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
44 changes: 43 additions & 1 deletion src/core/server/plugins/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,48 @@ 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;

/**
* 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>;
}

/**
* An extended `DiscoveredPlugin` that exposes more sensitive information. Should never
* be exposed to client-side code.
* @internal
*/
export interface DiscoveredPluginInternal extends DiscoveredPlugin {
/**
* Path on the filesystem where plugin was loaded from.
*/
readonly path: string;
}

type PluginInitializer<TExposed, TDependencies extends Record<PluginName, unknown>> = (
coreContext: PluginInitializerContext
) => {
Expand Down Expand Up @@ -109,7 +151,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
16 changes: 12 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({ public: new Map(), internal: new Map() });

mockDiscover.mockReturnValue({
error$: from([]),
Expand Down Expand Up @@ -224,7 +225,11 @@ 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.public).toBeInstanceOf(Map);
expect(start.uiPlugins.internal).toBeInstanceOf(Map);
expect(mockPluginSystem.addPlugin).not.toHaveBeenCalled();
expect(mockPluginSystem.startPlugins).toHaveBeenCalledTimes(1);
expect(mockPluginSystem.startPlugins).toHaveBeenCalledWith(startDeps);
Expand Down Expand Up @@ -287,12 +292,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 = { public: new Map(), internal: 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
20 changes: 16 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,18 @@ 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, DiscoveredPluginInternal, 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: {
public: Map<PluginName, DiscoveredPlugin>;
internal: Map<PluginName, DiscoveredPluginInternal>;
};
}

/** @internal */
export interface PluginsServiceStartDeps {
Expand Down Expand Up @@ -60,10 +66,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(),
};
}

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

public async stop() {
Expand Down
39 changes: 39 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,42 @@ Array [
expect(secondPluginNotToRun.start).not.toHaveBeenCalled();
expect(thirdPluginToRun.start).toHaveBeenCalledTimes(1);
});

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

test('`uiPlugins` returns ordered Maps 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().internal.keys()]).toMatchInlineSnapshot(`
Array [
"order-0",
"order-1",
"order-2",
"order-3",
"order-4",
]
`);
});
Loading

0 comments on commit 1bc205f

Please sign in to comment.