Skip to content

Commit

Permalink
[HTTP] Move plugins static dir registration to CoreApp service (#17…
Browse files Browse the repository at this point in the history 
…7571)

## Summary

Close #176201
  • Loading branch information
@jloleysens
jloleysens authored Feb 22, 2024
1 parent 4e30d4b commit 162426c
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 78 deletions.
128 changes: 124 additions & 4 deletions packages/core/apps/core-apps-server-internal/src/core_app.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,73 @@ describe('CoreApp', () => {
bypassErrorFormat: true,
});
});

it('registers expected static dirs if there are public plugins', () => {
prebootUIPlugins.internal.set('some-plugin', {
publicAssetsDir: '/foo',
publicTargetDir: '/bar',
requiredBundles: [],
version: '1.0.0',
});
prebootUIPlugins.public.set('some-plugin-2', {
type: PluginType.preboot,
configPath: 'some-plugin-2',
id: 'some-plugin-2',
optionalPlugins: [],
requiredBundles: [],
requiredPlugins: [],
runtimePluginDependencies: [],
});
prebootUIPlugins.internal.set('some-plugin-2', {
publicAssetsDir: '/foo',
publicTargetDir: '/bar',
requiredBundles: [],
version: '1.0.0',
});

internalCorePreboot.http.staticAssets.prependServerPath.mockReturnValue(
'/static-assets-path'
);
internalCorePreboot.http.staticAssets.getPluginServerPath.mockImplementation(
(name: string, path: string) => `/static-assets-path/${name}/${path}`
);
coreApp.preboot(internalCorePreboot, prebootUIPlugins);

expect(internalCorePreboot.http.registerStaticDir).toHaveBeenCalledTimes(
// Twice for all UI plugins + core's UI asset routes
prebootUIPlugins.public.size * 2 + 2
);
expect(internalCorePreboot.http.registerStaticDir).toHaveBeenCalledWith(
'/static-assets-path',
expect.any(String)
);
expect(internalCorePreboot.http.registerStaticDir).toHaveBeenCalledWith(
'/ui/{path*}',
expect.any(String)
);
expect(internalCorePreboot.http.registerStaticDir).toHaveBeenCalledWith(
'/static-assets-path/some-plugin/{path*}',
expect.any(String)
);
expect(internalCorePreboot.http.registerStaticDir).toHaveBeenCalledWith(
'/plugins/some-plugin/assets/{path*}', // legacy
expect.any(String)
);
expect(internalCorePreboot.http.registerStaticDir).toHaveBeenCalledWith(
'/static-assets-path/some-plugin-2/{path*}',
expect.any(String)
);
expect(internalCorePreboot.http.registerStaticDir).toHaveBeenCalledWith(
'/plugins/some-plugin-2/assets/{path*}', // legacy
expect.any(String)
);
});

it('does not register any static dirs if there are no public plugins', () => {
prebootUIPlugins = emptyPlugins();
coreApp.preboot(internalCorePreboot, prebootUIPlugins);
expect(internalCorePreboot.http.registerStaticDir).toHaveBeenCalledTimes(0);
});
});

describe('`/app/{id}/{any*}` route', () => {
Expand Down Expand Up @@ -249,19 +316,72 @@ describe('CoreApp', () => {
});
});

it('registers SHA-scoped and non-SHA-scoped UI bundle routes', async () => {
it('registers expected static dirs if there are public plugins', async () => {
const uiPlugins = emptyPlugins();
internalCoreSetup.http.staticAssets.prependServerPath.mockReturnValue('/some-path');
uiPlugins.public.set('some-plugin', {
type: PluginType.preboot,
configPath: 'some-plugin',
id: 'some-plugin',
optionalPlugins: [],
requiredBundles: [],
requiredPlugins: [],
runtimePluginDependencies: [],
});
uiPlugins.internal.set('some-plugin', {
publicAssetsDir: '/foo',
publicTargetDir: '/bar',
requiredBundles: [],
version: '1.0.0',
});
uiPlugins.public.set('some-plugin-2', {
type: PluginType.preboot,
configPath: 'some-plugin-2',
id: 'some-plugin-2',
optionalPlugins: [],
requiredBundles: [],
requiredPlugins: [],
runtimePluginDependencies: [],
});
uiPlugins.internal.set('some-plugin-2', {
publicAssetsDir: '/foo',
publicTargetDir: '/bar',
requiredBundles: [],
version: '1.0.0',
});

internalCoreSetup.http.staticAssets.prependServerPath.mockReturnValue('/static-assets-path');
internalCoreSetup.http.staticAssets.getPluginServerPath.mockImplementation(
(name: string, path: string) => `/static-assets-path/${name}/${path}`
);
await coreApp.setup(internalCoreSetup, uiPlugins);

expect(internalCoreSetup.http.registerStaticDir).toHaveBeenCalledTimes(2);
expect(internalCoreSetup.http.registerStaticDir).toHaveBeenCalledTimes(
// Twice for all UI plugins + core's UI asset routes
uiPlugins.public.size * 2 + 2
);
expect(internalCoreSetup.http.registerStaticDir).toHaveBeenCalledWith(
'/some-path',
'/static-assets-path',
expect.any(String)
);
expect(internalCoreSetup.http.registerStaticDir).toHaveBeenCalledWith(
'/ui/{path*}',
expect.any(String)
);
expect(internalCoreSetup.http.registerStaticDir).toHaveBeenCalledWith(
'/static-assets-path/some-plugin/{path*}',
expect.any(String)
);
expect(internalCoreSetup.http.registerStaticDir).toHaveBeenCalledWith(
'/plugins/some-plugin/assets/{path*}', // legacy
expect.any(String)
);
expect(internalCoreSetup.http.registerStaticDir).toHaveBeenCalledWith(
'/static-assets-path/some-plugin-2/{path*}',
expect.any(String)
);
expect(internalCoreSetup.http.registerStaticDir).toHaveBeenCalledWith(
'/plugins/some-plugin-2/assets/{path*}', // legacy
expect.any(String)
);
});
});
21 changes: 18 additions & 3 deletions packages/core/apps/core-apps-server-internal/src/core_app.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,15 @@ export class CoreAppsService {
// We register app-serving routes only if there are `preboot` plugins that may need them.
if (uiPlugins.public.size > 0) {
this.registerPrebootDefaultRoutes(corePreboot, uiPlugins);
this.registerStaticDirs(corePreboot);
this.registerStaticDirs(corePreboot, uiPlugins);
}
}

async setup(coreSetup: InternalCoreSetup, uiPlugins: UiPlugins) {
this.logger.debug('Setting up core app.');
const config = await firstValueFrom(this.config$);
this.registerDefaultRoutes(coreSetup, uiPlugins, config);
this.registerStaticDirs(coreSetup);
this.registerStaticDirs(coreSetup, uiPlugins);
}

private registerPrebootDefaultRoutes(corePreboot: InternalCorePreboot, uiPlugins: UiPlugins) {
Expand Down Expand Up @@ -271,7 +271,7 @@ export class CoreAppsService {

// After the package is built and bootstrap extracts files to bazel-bin,
// assets are exposed at the root of the package and in the package's node_modules dir
private registerStaticDirs(core: InternalCoreSetup | InternalCorePreboot) {
private registerStaticDirs(core: InternalCoreSetup | InternalCorePreboot, uiPlugins: UiPlugins) {
/**
* Serve UI from sha-scoped and not-sha-scoped paths to allow time for plugin code to migrate
* Eventually we only want to serve from the sha scoped path
Expand All @@ -282,5 +282,20 @@ export class CoreAppsService {
fromRoot('node_modules/@kbn/core-apps-server-internal/assets')
);
});

for (const [pluginName] of uiPlugins.public) {
const pluginInfo = uiPlugins.internal.get(pluginName);
if (!pluginInfo) continue; // assuming we will never hit this case; a public entry should have internal info registered
/**
* Serve UI from sha-scoped and not-sha-scoped paths to allow time for plugin code to migrate
* Eventually we only want to serve from the sha scoped path
*/
[
core.http.staticAssets.getPluginServerPath(pluginName, '{path*}'),
`/plugins/${pluginName}/assets/{path*}`,
].forEach((path) => {
core.http.registerStaticDir(path, pluginInfo.publicAssetsDir);
});
}
}
}
51 changes: 0 additions & 51 deletions packages/core/plugins/core-plugins-server-internal/src/plugins_service.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1301,31 +1301,6 @@ describe('PluginsService', () => {
expect(standardMockPluginSystem.setupPlugins).not.toHaveBeenCalled();
});

it('#preboot registers expected static dirs', async () => {
prebootDeps.http.staticAssets.getPluginServerPath.mockImplementation(
(pluginName: string) => `/static-assets/${pluginName}`
);
await pluginsService.discover({ environment: environmentPreboot, node: nodePreboot });
await pluginsService.preboot(prebootDeps);
expect(prebootDeps.http.registerStaticDir).toHaveBeenCalledTimes(prebootPlugins.length * 2);
expect(prebootDeps.http.registerStaticDir).toHaveBeenCalledWith(
'/static-assets/plugin-1-preboot',
expect.any(String)
);
expect(prebootDeps.http.registerStaticDir).toHaveBeenCalledWith(
'/plugins/plugin-1-preboot/assets/{path*}',
expect.any(String)
);
expect(prebootDeps.http.registerStaticDir).toHaveBeenCalledWith(
'/static-assets/plugin-2-preboot',
expect.any(String)
);
expect(prebootDeps.http.registerStaticDir).toHaveBeenCalledWith(
'/plugins/plugin-2-preboot/assets/{path*}',
expect.any(String)
);
});

it('#setup does initialize `standard` plugins if plugins.initialize is true', async () => {
config$.next({ plugins: { initialize: true } });
await pluginsService.discover({ environment: environmentPreboot, node: nodePreboot });
Expand All @@ -1346,32 +1321,6 @@ describe('PluginsService', () => {
expect(prebootMockPluginSystem.setupPlugins).not.toHaveBeenCalled();
expect(initialized).toBe(false);
});

it('#setup registers expected static dirs', async () => {
await pluginsService.discover({ environment: environmentPreboot, node: nodePreboot });
await pluginsService.preboot(prebootDeps);
setupDeps.http.staticAssets.getPluginServerPath.mockImplementation(
(pluginName: string) => `/static-assets/${pluginName}`
);
await pluginsService.setup(setupDeps);
expect(setupDeps.http.registerStaticDir).toHaveBeenCalledTimes(standardPlugins.length * 2);
expect(setupDeps.http.registerStaticDir).toHaveBeenCalledWith(
'/static-assets/plugin-1-standard',
expect.any(String)
);
expect(setupDeps.http.registerStaticDir).toHaveBeenCalledWith(
'/plugins/plugin-1-standard/assets/{path*}',
expect.any(String)
);
expect(setupDeps.http.registerStaticDir).toHaveBeenCalledWith(
'/static-assets/plugin-2-standard',
expect.any(String)
);
expect(setupDeps.http.registerStaticDir).toHaveBeenCalledWith(
'/plugins/plugin-2-standard/assets/{path*}',
expect.any(String)
);
});
});

describe('#getExposedPluginConfigsToUsage', () => {
Expand Down
20 changes: 0 additions & 20 deletions packages/core/plugins/core-plugins-server-internal/src/plugins_service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ export class PluginsService
const config = await firstValueFrom(this.config$);
if (config.initialize) {
await this.prebootPluginsSystem.setupPlugins(deps);
this.registerPluginStaticDirs(deps, this.prebootUiPluginInternalInfo);
} else {
this.log.info(
'Skipping `setup` for `preboot` plugins since plugin initialization is disabled.'
Expand All @@ -166,7 +165,6 @@ export class PluginsService
let contracts = new Map<PluginName, unknown>();
if (config.initialize) {
contracts = await this.standardPluginsSystem.setupPlugins(deps);
this.registerPluginStaticDirs(deps, this.standardUiPluginInternalInfo);
} else {
this.log.info(
'Skipping `setup` for `standard` plugins since plugin initialization is disabled.'
Expand Down Expand Up @@ -442,22 +440,4 @@ export class PluginsService
missingOrIncompatibleDependencies,
};
}

private registerPluginStaticDirs(
deps: PluginsServiceSetupDeps | PluginsServicePrebootSetupDeps,
uiPluginInternalInfo: Map<PluginName, InternalPluginInfo>
) {
for (const [pluginName, pluginInfo] of uiPluginInternalInfo) {
/**
* Serve UI from sha-scoped and not-sha-scoped paths to allow time for plugin code to migrate
* Eventually we only want to serve from the sha scoped path
*/
[
deps.http.staticAssets.getPluginServerPath(pluginName, '{path*}'),
`/plugins/${pluginName}/assets/{path*}`,
].forEach((path) => {
deps.http.registerStaticDir(path, pluginInfo.publicAssetsDir);
});
}
}
}

0 comments on commit 162426c

Please sign in to comment.