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

use cache busting for KP bundles #64414

Merged
merged 29 commits into from
Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
0f88c86
convert into TS
mshustov Apr 24, 2020
5c2bc55
load plugin scripts in html body
mshustov Apr 24, 2020
3ab5f29
use buildNum as a unique Id for cache busting
mshustov Apr 24, 2020
f2c8d51
add tests for caching
mshustov Apr 27, 2020
9f46398
fix tests
mshustov Apr 27, 2020
5e5c145
remove the last TODO. url should be inlined with assetss server
mshustov Apr 27, 2020
03b1584
this logic handled by publicPathMap on the client
mshustov Apr 27, 2020
5082c6e
cache kbn-shared-deps as well
mshustov Apr 27, 2020
c0332e1
attempt to fix karma tests
mshustov Apr 27, 2020
3de245b
Merge branch 'master' into js-cachable-assets
mshustov Apr 27, 2020
9f45612
Merge branch 'master' of github.com:elastic/kibana into pr/64414
spalger Apr 27, 2020
c66b362
always run file through replace stream
spalger Apr 27, 2020
b1d7133
place buildHash at begining of path, include all static files
spalger Apr 27, 2020
db23f0b
Merge branch 'master' of github.com:elastic/kibana into pr/64414
spalger Apr 27, 2020
4ad5f1c
update bundles_route tests to inject buildNum everywhere
spalger Apr 27, 2020
73635fb
Merge branch 'master' of github.com:elastic/kibana into pr/64414
spalger Apr 27, 2020
0e6f64c
Merge branch 'master' of github.com:elastic/kibana into pr/64414
spalger Apr 27, 2020
b46fcd5
fix karma config to point to right prefix
spalger Apr 28, 2020
b6e7a63
use isDist naming throughout
spalger Apr 28, 2020
240553f
explain magic number with variables
spalger Apr 28, 2020
7c35408
restore replacePublicPath option from #64226
spalger Apr 28, 2020
0beb6ab
replace one more instance of replacePublicPath
spalger Apr 28, 2020
604ab26
use promisify instead of bluebird + non-null assertions
spalger Apr 28, 2020
50dc863
Merge branch 'master' of github.com:elastic/kibana into pr/64414
spalger Apr 28, 2020
94690da
remove one more magic number
spalger Apr 28, 2020
5c7c3f1
Merge branch 'master' into js-cachable-assets
elasticmachine Apr 28, 2020
6770a4a
Merge branch 'master' of github.com:elastic/kibana into pr/64414
spalger Apr 29, 2020
f06d044
Merge branch 'master' of github.com:elastic/kibana into pr/64414
spalger Apr 29, 2020
2943a61
Merge branch 'master' into js-cachable-assets
elasticmachine Apr 29, 2020
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