Skip to content

Commit

Permalink
filter uiPlugins dependencies to only include ui deps (#47879)
Browse files Browse the repository at this point in the history 
Signed-off-by: pgayvallet <pierre.gayvallet@elastic.co>
  • Loading branch information
@pgayvallet
pgayvallet authored Oct 16, 2019
1 parent 53e1c95 commit 47fef4e
Show file tree
Hide file tree
Showing 2 changed files with 130 additions and 105 deletions.
204 changes: 114 additions & 90 deletions src/core/server/plugins/plugins_system.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ function createPlugin(
required = [],
optional = [],
server = true,
}: { required?: string[]; optional?: string[]; server?: boolean } = {}
ui = true,
}: { required?: string[]; optional?: string[]; server?: boolean; ui?: boolean } = {}
) {
return new PluginWrapper({
path: 'some-path',
Expand All @@ -55,7 +56,7 @@ function createPlugin(
requiredPlugins: required,
optionalPlugins: optional,
server,
ui: true,
ui,
},
opaqueId: Symbol(id),
initializerContext: { logger } as any,
Expand Down Expand Up @@ -147,13 +148,13 @@ test('`setupPlugins` ignores missing optional dependency', async () => {
pluginsSystem.addPlugin(plugin);

expect([...(await pluginsSystem.setupPlugins(setupDeps))]).toMatchInlineSnapshot(`
Array [
Array [
"some-id",
"test",
],
]
`);
Array [
Array [
"some-id",
"test",
],
]
`);
});

test('correctly orders plugins and returns exposed values for "setup" and "start"', async () => {
Expand Down Expand Up @@ -221,29 +222,29 @@ test('correctly orders plugins and returns exposed values for "setup" and "start
);

expect([...(await pluginsSystem.setupPlugins(setupDeps))]).toMatchInlineSnapshot(`
Array [
Array [
"order-0",
"added-as-1",
],
Array [
"order-1",
"added-as-3",
],
Array [
"order-2",
"added-as-2",
],
Array [
"order-3",
"added-as-4",
],
Array [
"order-4",
"added-as-0",
],
]
`);
Array [
Array [
"order-0",
"added-as-1",
],
Array [
"order-1",
"added-as-3",
],
Array [
"order-2",
"added-as-2",
],
Array [
"order-3",
"added-as-4",
],
Array [
"order-4",
"added-as-0",
],
]
`);

for (const [plugin, deps] of plugins) {
expect(mockCreatePluginSetupContext).toHaveBeenCalledWith(coreContext, setupDeps, plugin);
Expand All @@ -253,29 +254,29 @@ Array [

const startDeps = {};
expect([...(await pluginsSystem.startPlugins(startDeps))]).toMatchInlineSnapshot(`
Array [
Array [
"order-0",
"started-as-1",
],
Array [
"order-1",
"started-as-3",
],
Array [
"order-2",
"started-as-2",
],
Array [
"order-3",
"started-as-4",
],
Array [
"order-4",
"started-as-0",
],
]
`);
Array [
Array [
"order-0",
"started-as-1",
],
Array [
"order-1",
"started-as-3",
],
Array [
"order-2",
"started-as-2",
],
Array [
"order-3",
"started-as-4",
],
Array [
"order-4",
"started-as-0",
],
]
`);

for (const [plugin, deps] of plugins) {
expect(mockCreatePluginStartContext).toHaveBeenCalledWith(coreContext, startDeps, plugin);
Expand All @@ -296,17 +297,17 @@ test('`setupPlugins` only setups plugins that have server side', async () => {
});

expect([...(await pluginsSystem.setupPlugins(setupDeps))]).toMatchInlineSnapshot(`
Array [
Array [
"order-1",
"added-as-2",
],
Array [
"order-0",
"added-as-0",
],
]
`);
Array [
Array [
"order-1",
"added-as-2",
],
Array [
"order-0",
"added-as-0",
],
]
`);

expect(mockCreatePluginSetupContext).toHaveBeenCalledWith(
coreContext,
Expand All @@ -327,11 +328,11 @@ Array [

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

test('`uiPlugins` returns ordered Maps of all plugin manifests', async () => {
Expand All @@ -354,14 +355,37 @@ test('`uiPlugins` returns ordered Maps of all plugin manifests', async () => {
});

expect([...pluginsSystem.uiPlugins().internal.keys()]).toMatchInlineSnapshot(`
Array [
"order-0",
"order-1",
"order-2",
"order-3",
"order-4",
]
`);
Array [
"order-0",
"order-1",
"order-2",
"order-3",
"order-4",
]
`);
});

test('`uiPlugins` returns only ui plugin dependencies', async () => {
const plugins = [
createPlugin('ui-plugin', {
required: ['req-ui', 'req-no-ui'],
optional: ['opt-ui', 'opt-no-ui'],
ui: true,
server: false,
}),
createPlugin('req-ui', { ui: true, server: false }),
createPlugin('req-no-ui', { ui: false, server: true }),
createPlugin('opt-ui', { ui: true, server: false }),
createPlugin('opt-no-ui', { ui: false, server: true }),
];

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

const plugin = pluginsSystem.uiPlugins().internal.get('ui-plugin')!;
expect(plugin.requiredPlugins).toEqual(['req-ui']);
expect(plugin.optionalPlugins).toEqual(['opt-ui']);
});

test('can start without plugins', async () => {
Expand All @@ -386,15 +410,15 @@ test('`startPlugins` only starts plugins that were setup', async () => {
await pluginsSystem.setupPlugins(setupDeps);
const result = await pluginsSystem.startPlugins({});
expect([...result]).toMatchInlineSnapshot(`
Array [
Array [
"order-1",
"started-as-2",
],
Array [
"order-0",
"started-as-0",
],
]
`);
Array [
Array [
"order-1",
"started-as-2",
],
Array [
"order-0",
"started-as-0",
],
]
`);
});
31 changes: 16 additions & 15 deletions src/core/server/plugins/plugins_system.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,22 +161,23 @@ export class PluginsSystem {
* Get a Map of all discovered UI plugins in topological order.
*/
public uiPlugins() {
const uiPluginNames = [...this.getTopologicallySortedPluginNames().keys()].filter(
pluginName => this.plugins.get(pluginName)!.includesUiPlugin
);
const internal = new Map<PluginName, DiscoveredPluginInternal>(
[...this.getTopologicallySortedPluginNames().keys()]
.filter(pluginName => this.plugins.get(pluginName)!.includesUiPlugin)
.map(pluginName => {
const plugin = this.plugins.get(pluginName)!;
return [
pluginName,
{
id: pluginName,
path: plugin.path,
configPath: plugin.manifest.configPath,
requiredPlugins: plugin.manifest.requiredPlugins,
optionalPlugins: plugin.manifest.optionalPlugins,
},
] as [PluginName, DiscoveredPluginInternal];
})
uiPluginNames.map(pluginName => {
const plugin = this.plugins.get(pluginName)!;
return [
pluginName,
{
id: pluginName,
path: plugin.path,
configPath: plugin.manifest.configPath,
requiredPlugins: plugin.manifest.requiredPlugins.filter(p => uiPluginNames.includes(p)),
optionalPlugins: plugin.manifest.optionalPlugins.filter(p => uiPluginNames.includes(p)),
},
] as [PluginName, DiscoveredPluginInternal];
})
);

const publicPlugins = new Map<PluginName, DiscoveredPlugin>(
Expand Down

0 comments on commit 47fef4e

Please sign in to comment.