From 95e45ddebc6e86bb3d63f04bd7c4e56ad8ef55bb Mon Sep 17 00:00:00 2001 From: Pierre Gayvallet Date: Mon, 5 Apr 2021 18:00:13 +0200 Subject: [PATCH] Use plugin version in its publicPath (#95945) * Use plugin version in its publicPath * remove useless comment * fix types * update generated doc --- .../register_bundle_routes.test.ts | 15 +++++++------ .../bundle_routes/register_bundle_routes.ts | 8 +++---- src/core/server/legacy/legacy_service.test.ts | 1 + .../server/plugins/plugins_service.test.ts | 14 ++++++++++--- src/core/server/plugins/plugins_service.ts | 1 + src/core/server/plugins/plugins_system.ts | 1 + src/core/server/plugins/types.ts | 13 +++++++++--- .../bootstrap/get_plugin_bundle_paths.test.ts | 21 ++++++++++++------- .../bootstrap/get_plugin_bundle_paths.ts | 10 +++++++-- src/core/server/server.api.md | 8 +++---- 10 files changed, 63 insertions(+), 29 deletions(-) diff --git a/src/core/server/core_app/bundle_routes/register_bundle_routes.test.ts b/src/core/server/core_app/bundle_routes/register_bundle_routes.test.ts index d51c36914695..830f4a9a9436 100644 --- a/src/core/server/core_app/bundle_routes/register_bundle_routes.test.ts +++ b/src/core/server/core_app/bundle_routes/register_bundle_routes.test.ts @@ -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'; @@ -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()), }); describe('registerBundleRoutes', () => { @@ -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/', }); }); }); diff --git a/src/core/server/core_app/bundle_routes/register_bundle_routes.ts b/src/core/server/core_app/bundle_routes/register_bundle_routes.ts index ee54f8ef3462..df46753747f5 100644 --- a/src/core/server/core_app/bundle_routes/register_bundle_routes.ts +++ b/src/core/server/core_app/bundle_routes/register_bundle_routes.ts @@ -27,7 +27,7 @@ import { registerRouteForBundle } from './bundles_route'; */ export function registerBundleRoutes({ router, - serverBasePath, // serverBasePath + serverBasePath, uiPlugins, packageInfo, }: { @@ -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, diff --git a/src/core/server/legacy/legacy_service.test.ts b/src/core/server/legacy/legacy_service.test.ts index d0a02b985996..67b5393f0b83 100644 --- a/src/core/server/legacy/legacy_service.test.ts +++ b/src/core/server/legacy/legacy_service.test.ts @@ -91,6 +91,7 @@ beforeEach(() => { 'plugin-id', { requiredBundles: [], + version: '8.0.0', publicTargetDir: 'path/to/target/public', publicAssetsDir: '/plugins/name/assets/', }, diff --git a/src/core/server/plugins/plugins_service.test.ts b/src/core/server/plugins/plugins_service.test.ts index 2d54648d2295..6bf7a1fadb4d 100644 --- a/src/core/server/plugins/plugins_service.test.ts +++ b/src/core/server/plugins/plugins_service.test.ts @@ -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', }), ]), @@ -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(` @@ -586,15 +586,23 @@ describe('PluginsService', () => { "publicAssetsDir": /path-1/public/assets, "publicTargetDir": /path-1/target/public, "requiredBundles": Array [], + "version": "version-1", }, "plugin-2" => Object { "publicAssetsDir": /path-2/public/assets, "publicTargetDir": /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', () => { diff --git a/src/core/server/plugins/plugins_service.ts b/src/core/server/plugins/plugins_service.ts index 8b33e2cf4cc6..09be40ecaf2a 100644 --- a/src/core/server/plugins/plugins_service.ts +++ b/src/core/server/plugins/plugins_service.ts @@ -222,6 +222,7 @@ export class PluginsService implements CoreService(); diff --git a/src/core/server/plugins/types.ts b/src/core/server/plugins/types.ts index a6086bd6f17e..3a01049c5e1f 100644 --- a/src/core/server/plugins/types.ts +++ b/src/core/server/plugins/types.ts @@ -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; /** @@ -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; } @@ -267,7 +272,9 @@ export interface AsyncPlugin< TPluginsStart extends object = object > { setup(core: CoreSetup, plugins: TPluginsSetup): TSetup | Promise; + start(core: CoreStart, plugins: TPluginsStart): TStart | Promise; + stop?(): void; } diff --git a/src/core/server/rendering/bootstrap/get_plugin_bundle_paths.test.ts b/src/core/server/rendering/bootstrap/get_plugin_bundle_paths.test.ts index ea3843884df3..0abd8fd5a005 100644 --- a/src/core/server/rendering/bootstrap/get_plugin_bundle_paths.test.ts +++ b/src/core/server/rendering/bootstrap/get_plugin_bundle_paths.test.ts @@ -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) => { @@ -16,12 +16,13 @@ const createUiPlugins = (pluginDeps: Record) => { 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', @@ -29,6 +30,12 @@ const createUiPlugins = (pluginDeps: Record) => { requiredPlugins: [], requiredBundles: deps, }); + + deps.forEach((dep) => addPlugin(dep, [])); + }; + + Object.entries(pluginDeps).forEach(([pluginId, deps]) => { + addPlugin(pluginId, deps); }); return uiPlugins; @@ -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/', }); }); }); diff --git a/src/core/server/rendering/bootstrap/get_plugin_bundle_paths.ts b/src/core/server/rendering/bootstrap/get_plugin_bundle_paths.ts index c8291b2720a9..86ffdcf835f7 100644 --- a/src/core/server/rendering/bootstrap/get_plugin_bundle_paths.ts +++ b/src/core/server/rendering/bootstrap/get_plugin_bundle_paths.ts @@ -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 ?? []; diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index de96c5ccfb81..fb5fe3efd3e0 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -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" ```