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

[7.x] Inject UI plugins into injectedMetadata (#31840) #33574

Merged
merged 1 commit into from
Mar 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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