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] Consolidate downloading plugin bundles to bootstrap script (#64685) #64725

Merged
merged 1 commit into from
Apr 29, 2020
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
6 changes: 3 additions & 3 deletions src/core/public/plugins/plugin.test.mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ export const mockPlugin = {
};
export const mockInitializer = jest.fn(() => mockPlugin);

export const mockPluginLoader = jest.fn().mockResolvedValue(mockInitializer);
export const mockPluginReader = jest.fn(() => mockInitializer);

jest.mock('./plugin_loader', () => ({
loadPluginBundle: mockPluginLoader,
jest.mock('./plugin_reader', () => ({
read: mockPluginReader,
}));
47 changes: 14 additions & 33 deletions src/core/public/plugins/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { mockInitializer, mockPlugin, mockPluginLoader } from './plugin.test.mocks';
import { mockInitializer, mockPlugin, mockPluginReader } from './plugin.test.mocks';

import { DiscoveredPlugin } from '../../server';
import { coreMock } from '../mocks';
Expand All @@ -38,67 +38,49 @@ function createManifest(
let plugin: PluginWrapper<unknown, Record<string, unknown>>;
const opaqueId = Symbol();
const initializerContext = coreMock.createPluginInitializerContext();
const addBasePath = (path: string) => path;

beforeEach(() => {
mockPluginLoader.mockClear();
mockPluginReader.mockClear();
mockPlugin.setup.mockClear();
mockPlugin.start.mockClear();
mockPlugin.stop.mockClear();
plugin = new PluginWrapper(createManifest('plugin-a'), opaqueId, initializerContext);
});

describe('PluginWrapper', () => {
test('`load` calls loadPluginBundle', () => {
plugin.load(addBasePath);
expect(mockPluginLoader).toHaveBeenCalledWith(addBasePath, 'plugin-a');
});

test('`setup` fails if load is not called first', async () => {
await expect(plugin.setup({} as any, {} as any)).rejects.toThrowErrorMatchingInlineSnapshot(
`"Plugin \\"plugin-a\\" can't be setup since its bundle isn't loaded."`
);
});

test('`setup` fails if plugin.setup is not a function', async () => {
mockInitializer.mockReturnValueOnce({ start: jest.fn() } as any);
await plugin.load(addBasePath);
await expect(plugin.setup({} as any, {} as any)).rejects.toThrowErrorMatchingInlineSnapshot(
`"Instance of plugin \\"plugin-a\\" does not define \\"setup\\" function."`
);
});

test('`setup` fails if plugin.start is not a function', async () => {
mockInitializer.mockReturnValueOnce({ setup: jest.fn() } as any);
await plugin.load(addBasePath);
await expect(plugin.setup({} as any, {} as any)).rejects.toThrowErrorMatchingInlineSnapshot(
`"Instance of plugin \\"plugin-a\\" does not define \\"start\\" function."`
);
});

test('`setup` calls initializer with initializer context', async () => {
await plugin.load(addBasePath);
await plugin.setup({} as any, {} as any);
expect(mockInitializer).toHaveBeenCalledWith(initializerContext);
});

test('`setup` calls plugin.setup with context and dependencies', async () => {
await plugin.load(addBasePath);
const context = { any: 'thing' } as any;
const deps = { otherDep: 'value' };
await plugin.setup(context, deps);
expect(mockPlugin.setup).toHaveBeenCalledWith(context, deps);
});

test('`start` fails if setup is not called first', async () => {
await plugin.load(addBasePath);
await expect(plugin.start({} as any, {} as any)).rejects.toThrowErrorMatchingInlineSnapshot(
`"Plugin \\"plugin-a\\" can't be started since it isn't set up."`
);
});

test('`start` calls plugin.start with context and dependencies', async () => {
await plugin.load(addBasePath);
await plugin.setup({} as any, {} as any);
const context = { any: 'thing' } as any;
const deps = { otherDep: 'value' };
Expand All @@ -114,20 +96,21 @@ describe('PluginWrapper', () => {
};

let startDependenciesResolved = false;
mockPluginLoader.mockResolvedValueOnce(() => ({
setup: jest.fn(),
start: async () => {
// Add small delay to ensure startDependencies is not resolved until after the plugin instance's start resolves.
await new Promise(resolve => setTimeout(resolve, 10));
expect(startDependenciesResolved).toBe(false);
return pluginStartContract;
},
}));
await plugin.load(addBasePath);
mockPluginReader.mockReturnValueOnce(
jest.fn(() => ({
setup: jest.fn(),
start: jest.fn(async () => {
// Add small delay to ensure startDependencies is not resolved until after the plugin instance's start resolves.
await new Promise(resolve => setTimeout(resolve, 10));
expect(startDependenciesResolved).toBe(false);
return pluginStartContract;
}),
stop: jest.fn(),
}))
);
await plugin.setup({} as any, {} as any);
const context = { any: 'thing' } as any;
const deps = { otherDep: 'value' };

// Add promise callback prior to calling `start` to ensure calls in `setup` will not resolve before `start` is
// called.
const startDependenciesCheck = plugin.startDependencies.then(res => {
Expand All @@ -145,15 +128,13 @@ describe('PluginWrapper', () => {
});

test('`stop` calls plugin.stop', async () => {
await plugin.load(addBasePath);
await plugin.setup({} as any, {} as any);
await plugin.stop();
expect(mockPlugin.stop).toHaveBeenCalled();
});

test('`stop` does not fail if plugin.stop does not exist', async () => {
mockInitializer.mockReturnValueOnce({ setup: jest.fn(), start: jest.fn() } as any);
await plugin.load(addBasePath);
await plugin.setup({} as any, {} as any);
expect(() => plugin.stop()).not.toThrow();
});
Expand Down
28 changes: 9 additions & 19 deletions src/core/public/plugins/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { Subject } from 'rxjs';
import { first } from 'rxjs/operators';
import { DiscoveredPlugin, PluginOpaqueId } from '../../server';
import { PluginInitializerContext } from './plugin_context';
import { loadPluginBundle } from './plugin_loader';
import { read } from './plugin_reader';
import { CoreStart, CoreSetup } from '..';

/**
Expand Down Expand Up @@ -69,7 +69,6 @@ export class PluginWrapper<
public readonly configPath: DiscoveredPlugin['configPath'];
public readonly requiredPlugins: DiscoveredPlugin['requiredPlugins'];
public readonly optionalPlugins: DiscoveredPlugin['optionalPlugins'];
private initializer?: PluginInitializer<TSetup, TStart, TPluginsSetup, TPluginsStart>;
private instance?: Plugin<TSetup, TStart, TPluginsSetup, TPluginsStart>;

private readonly startDependencies$ = new Subject<[CoreStart, TPluginsStart, TStart]>();
Expand All @@ -86,18 +85,6 @@ export class PluginWrapper<
this.optionalPlugins = discoveredPlugin.optionalPlugins;
}

/**
* Loads the plugin's bundle into the browser. Should be called in parallel with all plugins
* using `Promise.all`. Must be called before `setup`.
* @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, TStart, TPluginsSetup, TPluginsStart>(
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
Expand Down Expand Up @@ -146,11 +133,14 @@ export class PluginWrapper<
}

private async createPluginInstance() {
if (this.initializer === undefined) {
throw new Error(`Plugin "${this.name}" can't be setup since its bundle isn't loaded.`);
}

const instance = this.initializer(this.initializerContext);
const initializer = read(this.name) as PluginInitializer<
TSetup,
TStart,
TPluginsSetup,
TPluginsStart
>;

const instance = initializer(this.initializerContext);

if (typeof instance.setup !== 'function') {
throw new Error(`Instance of plugin "${this.name}" does not define "setup" function.`);
Expand Down
33 changes: 0 additions & 33 deletions src/core/public/plugins/plugin_loader.mock.ts

This file was deleted.

125 changes: 0 additions & 125 deletions src/core/public/plugins/plugin_loader.test.ts

This file was deleted.

Loading