Skip to content

Commit

Permalink
Use plugin version in its publicPath (#95945)
Browse files Browse the repository at this point in the history
* Use plugin version in its publicPath

* remove useless comment

* fix types

* update generated doc
  • Loading branch information
pgayvallet authored Apr 5, 2021
1 parent ea03eb1 commit 95e45dd
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { registerRouteForBundleMock } from './register_bundle_routes.test.mocks'

import { PackageInfo } from '@kbn/config';
import { httpServiceMock } from '../../http/http_service.mock';
import { UiPlugins } from '../../plugins';
import { InternalPluginInfo, UiPlugins } from '../../plugins';
import { registerBundleRoutes } from './register_bundle_routes';
import { FileHashCache } from './file_hash_cache';

Expand All @@ -29,9 +29,12 @@ const createUiPlugins = (...ids: string[]): UiPlugins => ({
internal: ids.reduce((map, id) => {
map.set(id, {
publicTargetDir: `/plugins/${id}/public-target-dir`,
publicAssetsDir: `/plugins/${id}/public-assets-dir`,
version: '8.0.0',
requiredBundles: [],
});
return map;
}, new Map()),
}, new Map<string, InternalPluginInfo>()),
});

describe('registerBundleRoutes', () => {
Expand Down Expand Up @@ -86,16 +89,16 @@ describe('registerBundleRoutes', () => {
fileHashCache: expect.any(FileHashCache),
isDist: true,
bundlesPath: '/plugins/plugin-a/public-target-dir',
publicPath: '/server-base-path/42/bundles/plugin/plugin-a/',
routePath: '/42/bundles/plugin/plugin-a/',
publicPath: '/server-base-path/42/bundles/plugin/plugin-a/8.0.0/',
routePath: '/42/bundles/plugin/plugin-a/8.0.0/',
});

expect(registerRouteForBundleMock).toHaveBeenCalledWith(router, {
fileHashCache: expect.any(FileHashCache),
isDist: true,
bundlesPath: '/plugins/plugin-b/public-target-dir',
publicPath: '/server-base-path/42/bundles/plugin/plugin-b/',
routePath: '/42/bundles/plugin/plugin-b/',
publicPath: '/server-base-path/42/bundles/plugin/plugin-b/8.0.0/',
routePath: '/42/bundles/plugin/plugin-b/8.0.0/',
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { registerRouteForBundle } from './bundles_route';
*/
export function registerBundleRoutes({
router,
serverBasePath, // serverBasePath
serverBasePath,
uiPlugins,
packageInfo,
}: {
Expand Down Expand Up @@ -57,10 +57,10 @@ export function registerBundleRoutes({
isDist,
});

[...uiPlugins.internal.entries()].forEach(([id, { publicTargetDir }]) => {
[...uiPlugins.internal.entries()].forEach(([id, { publicTargetDir, version }]) => {
registerRouteForBundle(router, {
publicPath: `${serverBasePath}/${buildNum}/bundles/plugin/${id}/`,
routePath: `/${buildNum}/bundles/plugin/${id}/`,
publicPath: `${serverBasePath}/${buildNum}/bundles/plugin/${id}/${version}/`,
routePath: `/${buildNum}/bundles/plugin/${id}/${version}/`,
bundlesPath: publicTargetDir,
fileHashCache,
isDist,
Expand Down
1 change: 1 addition & 0 deletions src/core/server/legacy/legacy_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ beforeEach(() => {
'plugin-id',
{
requiredBundles: [],
version: '8.0.0',
publicTargetDir: 'path/to/target/public',
publicAssetsDir: '/plugins/name/assets/',
},
Expand Down
14 changes: 11 additions & 3 deletions src/core/server/plugins/plugins_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -562,12 +562,12 @@ describe('PluginsService', () => {
plugin$: from([
createPlugin('plugin-1', {
path: 'path-1',
version: 'some-version',
version: 'version-1',
configPath: 'plugin1',
}),
createPlugin('plugin-2', {
path: 'path-2',
version: 'some-version',
version: 'version-2',
configPath: 'plugin2',
}),
]),
Expand All @@ -577,7 +577,7 @@ describe('PluginsService', () => {
});

describe('uiPlugins.internal', () => {
it('includes disabled plugins', async () => {
it('contains internal properties for plugins', async () => {
config$.next({ plugins: { initialize: true }, plugin1: { enabled: false } });
const { uiPlugins } = await pluginsService.discover({ environment: environmentSetup });
expect(uiPlugins.internal).toMatchInlineSnapshot(`
Expand All @@ -586,15 +586,23 @@ describe('PluginsService', () => {
"publicAssetsDir": <absolute path>/path-1/public/assets,
"publicTargetDir": <absolute path>/path-1/target/public,
"requiredBundles": Array [],
"version": "version-1",
},
"plugin-2" => Object {
"publicAssetsDir": <absolute path>/path-2/public/assets,
"publicTargetDir": <absolute path>/path-2/target/public,
"requiredBundles": Array [],
"version": "version-2",
},
}
`);
});

it('includes disabled plugins', async () => {
config$.next({ plugins: { initialize: true }, plugin1: { enabled: false } });
const { uiPlugins } = await pluginsService.discover({ environment: environmentSetup });
expect([...uiPlugins.internal.keys()].sort()).toEqual(['plugin-1', 'plugin-2']);
});
});

describe('plugin initialization', () => {
Expand Down
1 change: 1 addition & 0 deletions src/core/server/plugins/plugins_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
if (plugin.includesUiPlugin) {
this.uiPluginInternalInfo.set(plugin.name, {
requiredBundles: plugin.requiredBundles,
version: plugin.manifest.version,
publicTargetDir: Path.resolve(plugin.path, 'target/public'),
publicAssetsDir: Path.resolve(plugin.path, 'public/assets'),
});
Expand Down
1 change: 1 addition & 0 deletions src/core/server/plugins/plugins_system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { PluginsServiceSetupDeps, PluginsServiceStartDeps } from './plugins_serv
import { PluginDependencies } from '.';

const Sec = 1000;

/** @internal */
export class PluginsSystem {
private readonly plugins = new Map<PluginName, PluginWrapper>();
Expand Down
13 changes: 10 additions & 3 deletions src/core/server/plugins/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,12 +224,15 @@ export interface DiscoveredPlugin {
*/
export interface InternalPluginInfo {
/**
* Bundles that must be loaded for this plugoin
* Version of the plugin
*/
readonly version: string;
/**
* Bundles that must be loaded for this plugin
*/
readonly requiredBundles: readonly string[];
/**
* Path to the target/public directory of the plugin which should be
* served
* Path to the target/public directory of the plugin which should be served
*/
readonly publicTargetDir: string;
/**
Expand All @@ -250,7 +253,9 @@ export interface Plugin<
TPluginsStart extends object = object
> {
setup(core: CoreSetup, plugins: TPluginsSetup): TSetup;

start(core: CoreStart, plugins: TPluginsStart): TStart;

stop?(): void;
}

Expand All @@ -267,7 +272,9 @@ export interface AsyncPlugin<
TPluginsStart extends object = object
> {
setup(core: CoreSetup, plugins: TPluginsSetup): TSetup | Promise<TSetup>;

start(core: CoreStart, plugins: TPluginsStart): TStart | Promise<TStart>;

stop?(): void;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import { UiPlugins } from '../../plugins';
import { InternalPluginInfo, UiPlugins } from '../../plugins';
import { getPluginsBundlePaths } from './get_plugin_bundle_paths';

const createUiPlugins = (pluginDeps: Record<string, string[]>) => {
Expand All @@ -16,19 +16,26 @@ const createUiPlugins = (pluginDeps: Record<string, string[]>) => {
browserConfigs: new Map(),
};

Object.entries(pluginDeps).forEach(([pluginId, deps]) => {
const addPlugin = (pluginId: string, deps: string[]) => {
uiPlugins.internal.set(pluginId, {
requiredBundles: deps,
version: '8.0.0',
publicTargetDir: '',
publicAssetsDir: '',
} as any);
} as InternalPluginInfo);
uiPlugins.public.set(pluginId, {
id: pluginId,
configPath: 'config-path',
optionalPlugins: [],
requiredPlugins: [],
requiredBundles: deps,
});

deps.forEach((dep) => addPlugin(dep, []));
};

Object.entries(pluginDeps).forEach(([pluginId, deps]) => {
addPlugin(pluginId, deps);
});

return uiPlugins;
Expand Down Expand Up @@ -56,13 +63,13 @@ describe('getPluginsBundlePaths', () => {
});

expect(pluginBundlePaths.get('a')).toEqual({
bundlePath: '/regular-bundle-path/plugin/a/a.plugin.js',
publicPath: '/regular-bundle-path/plugin/a/',
bundlePath: '/regular-bundle-path/plugin/a/8.0.0/a.plugin.js',
publicPath: '/regular-bundle-path/plugin/a/8.0.0/',
});

expect(pluginBundlePaths.get('b')).toEqual({
bundlePath: '/regular-bundle-path/plugin/b/b.plugin.js',
publicPath: '/regular-bundle-path/plugin/b/',
bundlePath: '/regular-bundle-path/plugin/b/8.0.0/b.plugin.js',
publicPath: '/regular-bundle-path/plugin/b/8.0.0/',
});
});
});
10 changes: 8 additions & 2 deletions src/core/server/rendering/bootstrap/get_plugin_bundle_paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,15 @@ export const getPluginsBundlePaths = ({

while (pluginsToProcess.length > 0) {
const pluginId = pluginsToProcess.pop() as string;
const plugin = uiPlugins.internal.get(pluginId);
if (!plugin) {
continue;
}
const { version } = plugin;

pluginBundlePaths.set(pluginId, {
publicPath: `${regularBundlePath}/plugin/${pluginId}/`,
bundlePath: `${regularBundlePath}/plugin/${pluginId}/${pluginId}.plugin.js`,
publicPath: `${regularBundlePath}/plugin/${pluginId}/${version}/`,
bundlePath: `${regularBundlePath}/plugin/${pluginId}/${version}/${pluginId}.plugin.js`,
});

const pluginBundleIds = uiPlugins.internal.get(pluginId)?.requiredBundles ?? [];
Expand Down
8 changes: 4 additions & 4 deletions src/core/server/server.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -3259,9 +3259,9 @@ export const validBodyOutput: readonly ["data", "stream"];
//
// src/core/server/elasticsearch/client/types.ts:94:7 - (ae-forgotten-export) The symbol "Explanation" needs to be exported by the entry point index.d.ts
// src/core/server/http/router/response.ts:297:3 - (ae-forgotten-export) The symbol "KibanaResponse" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:286:3 - (ae-forgotten-export) The symbol "KibanaConfigType" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:286:3 - (ae-forgotten-export) The symbol "SharedGlobalConfigKeys" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:289:3 - (ae-forgotten-export) The symbol "SavedObjectsConfigType" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:394:5 - (ae-unresolved-link) The @link reference could not be resolved: The package "kibana" does not have an export "create"
// src/core/server/plugins/types.ts:293:3 - (ae-forgotten-export) The symbol "KibanaConfigType" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:293:3 - (ae-forgotten-export) The symbol "SharedGlobalConfigKeys" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:296:3 - (ae-forgotten-export) The symbol "SavedObjectsConfigType" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:401:5 - (ae-unresolved-link) The @link reference could not be resolved: The package "kibana" does not have an export "create"

```

0 comments on commit 95e45dd

Please sign in to comment.