diff --git a/docs/development/core/server/kibana-plugin-server.coresetup.getstartservices.md b/docs/development/core/server/kibana-plugin-server.coresetup.getstartservices.md new file mode 100644 index 0000000000000..b05d28899f9d2 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-server.coresetup.getstartservices.md @@ -0,0 +1,17 @@ + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [CoreSetup](./kibana-plugin-server.coresetup.md) > [getStartServices](./kibana-plugin-server.coresetup.getstartservices.md) + +## CoreSetup.getStartServices() method + +Allows plugins to get access to APIs available in start inside async handlers. Promise will not resolve until Core and plugin dependencies have completed `start`. This should only be used inside handlers registered during `setup` that will only be executed after `start` lifecycle. + +Signature: + +```typescript +getStartServices(): Promise<[CoreStart, TPluginsStart]>; +``` +Returns: + +`Promise<[CoreStart, TPluginsStart]>` + diff --git a/docs/development/core/server/kibana-plugin-server.coresetup.md b/docs/development/core/server/kibana-plugin-server.coresetup.md index 3f7f5b727ee80..c36d649837e8a 100644 --- a/docs/development/core/server/kibana-plugin-server.coresetup.md +++ b/docs/development/core/server/kibana-plugin-server.coresetup.md @@ -1,26 +1,32 @@ - - -[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [CoreSetup](./kibana-plugin-server.coresetup.md) - -## CoreSetup interface - -Context passed to the plugins `setup` method. - -Signature: - -```typescript -export interface CoreSetup -``` - -## Properties - -| Property | Type | Description | -| --- | --- | --- | -| [capabilities](./kibana-plugin-server.coresetup.capabilities.md) | CapabilitiesSetup | [CapabilitiesSetup](./kibana-plugin-server.capabilitiessetup.md) | -| [context](./kibana-plugin-server.coresetup.context.md) | ContextSetup | [ContextSetup](./kibana-plugin-server.contextsetup.md) | -| [elasticsearch](./kibana-plugin-server.coresetup.elasticsearch.md) | ElasticsearchServiceSetup | [ElasticsearchServiceSetup](./kibana-plugin-server.elasticsearchservicesetup.md) | -| [http](./kibana-plugin-server.coresetup.http.md) | HttpServiceSetup | [HttpServiceSetup](./kibana-plugin-server.httpservicesetup.md) | -| [savedObjects](./kibana-plugin-server.coresetup.savedobjects.md) | SavedObjectsServiceSetup | [SavedObjectsServiceSetup](./kibana-plugin-server.savedobjectsservicesetup.md) | -| [uiSettings](./kibana-plugin-server.coresetup.uisettings.md) | UiSettingsServiceSetup | [UiSettingsServiceSetup](./kibana-plugin-server.uisettingsservicesetup.md) | -| [uuid](./kibana-plugin-server.coresetup.uuid.md) | UuidServiceSetup | [UuidServiceSetup](./kibana-plugin-server.uuidservicesetup.md) | - + + +[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [CoreSetup](./kibana-plugin-server.coresetup.md) + +## CoreSetup interface + +Context passed to the plugins `setup` method. + +Signature: + +```typescript +export interface CoreSetup +``` + +## Properties + +| Property | Type | Description | +| --- | --- | --- | +| [capabilities](./kibana-plugin-server.coresetup.capabilities.md) | CapabilitiesSetup | [CapabilitiesSetup](./kibana-plugin-server.capabilitiessetup.md) | +| [context](./kibana-plugin-server.coresetup.context.md) | ContextSetup | [ContextSetup](./kibana-plugin-server.contextsetup.md) | +| [elasticsearch](./kibana-plugin-server.coresetup.elasticsearch.md) | ElasticsearchServiceSetup | [ElasticsearchServiceSetup](./kibana-plugin-server.elasticsearchservicesetup.md) | +| [http](./kibana-plugin-server.coresetup.http.md) | HttpServiceSetup | [HttpServiceSetup](./kibana-plugin-server.httpservicesetup.md) | +| [savedObjects](./kibana-plugin-server.coresetup.savedobjects.md) | SavedObjectsServiceSetup | [SavedObjectsServiceSetup](./kibana-plugin-server.savedobjectsservicesetup.md) | +| [uiSettings](./kibana-plugin-server.coresetup.uisettings.md) | UiSettingsServiceSetup | [UiSettingsServiceSetup](./kibana-plugin-server.uisettingsservicesetup.md) | +| [uuid](./kibana-plugin-server.coresetup.uuid.md) | UuidServiceSetup | [UuidServiceSetup](./kibana-plugin-server.uuidservicesetup.md) | + +## Methods + +| Method | Description | +| --- | --- | +| [getStartServices()](./kibana-plugin-server.coresetup.getstartservices.md) | Allows plugins to get access to APIs available in start inside async handlers. Promise will not resolve until Core and plugin dependencies have completed start. This should only be used inside handlers registered during setup that will only be executed after start lifecycle. | + diff --git a/docs/development/core/server/kibana-plugin-server.discoveredplugin.configpath.md b/docs/development/core/server/kibana-plugin-server.discoveredplugin.configpath.md index b0830b8d72238..4de20b9c6cccf 100644 --- a/docs/development/core/server/kibana-plugin-server.discoveredplugin.configpath.md +++ b/docs/development/core/server/kibana-plugin-server.discoveredplugin.configpath.md @@ -4,7 +4,7 @@ ## DiscoveredPlugin.configPath property -Root configuration path used by the plugin, defaults to "id". +Root configuration path used by the plugin, defaults to "id" in snake\_case format. Signature: diff --git a/docs/development/core/server/kibana-plugin-server.discoveredplugin.md b/docs/development/core/server/kibana-plugin-server.discoveredplugin.md index aadf9763b1604..ea13422458c7f 100644 --- a/docs/development/core/server/kibana-plugin-server.discoveredplugin.md +++ b/docs/development/core/server/kibana-plugin-server.discoveredplugin.md @@ -16,7 +16,7 @@ export interface DiscoveredPlugin | Property | Type | Description | | --- | --- | --- | -| [configPath](./kibana-plugin-server.discoveredplugin.configpath.md) | ConfigPath | Root configuration path used by the plugin, defaults to "id". | +| [configPath](./kibana-plugin-server.discoveredplugin.configpath.md) | ConfigPath | Root configuration path used by the plugin, defaults to "id" in snake\_case format. | | [id](./kibana-plugin-server.discoveredplugin.id.md) | PluginName | Identifier of the plugin. | | [optionalPlugins](./kibana-plugin-server.discoveredplugin.optionalplugins.md) | readonly PluginName[] | An optional list of the other plugins that if installed and enabled \*\*may be\*\* leveraged by this plugin for some additional functionality but otherwise are not required for this plugin to work properly. | | [requiredPlugins](./kibana-plugin-server.discoveredplugin.requiredplugins.md) | readonly PluginName[] | An optional list of the other plugins that \*\*must be\*\* installed and enabled for this plugin to function properly. | diff --git a/docs/development/core/server/kibana-plugin-server.pluginmanifest.configpath.md b/docs/development/core/server/kibana-plugin-server.pluginmanifest.configpath.md index 39c1eeda47e0e..6ffe396aa2ed1 100644 --- a/docs/development/core/server/kibana-plugin-server.pluginmanifest.configpath.md +++ b/docs/development/core/server/kibana-plugin-server.pluginmanifest.configpath.md @@ -4,10 +4,15 @@ ## PluginManifest.configPath property -Root [configuration path](./kibana-plugin-server.configpath.md) used by the plugin, defaults to "id". +Root [configuration path](./kibana-plugin-server.configpath.md) used by the plugin, defaults to "id" in snake\_case format. Signature: ```typescript readonly configPath: ConfigPath; ``` + +## Example + +id: myPlugin configPath: my\_plugin + diff --git a/docs/development/core/server/kibana-plugin-server.pluginmanifest.id.md b/docs/development/core/server/kibana-plugin-server.pluginmanifest.id.md index 44e61f11fa215..104046f3ce7d0 100644 --- a/docs/development/core/server/kibana-plugin-server.pluginmanifest.id.md +++ b/docs/development/core/server/kibana-plugin-server.pluginmanifest.id.md @@ -4,7 +4,7 @@ ## PluginManifest.id property -Identifier of the plugin. +Identifier of the plugin. Must be a string in camelCase. Part of a plugin public contract. Other plugins leverage it to access plugin API, navigate to the plugin, etc. Signature: diff --git a/docs/development/core/server/kibana-plugin-server.pluginmanifest.md b/docs/development/core/server/kibana-plugin-server.pluginmanifest.md index 9bb208a809b22..c39a702389fb3 100644 --- a/docs/development/core/server/kibana-plugin-server.pluginmanifest.md +++ b/docs/development/core/server/kibana-plugin-server.pluginmanifest.md @@ -20,8 +20,8 @@ Should never be used in code outside of Core but is exported for documentation p | Property | Type | Description | | --- | --- | --- | -| [configPath](./kibana-plugin-server.pluginmanifest.configpath.md) | ConfigPath | Root [configuration path](./kibana-plugin-server.configpath.md) used by the plugin, defaults to "id". | -| [id](./kibana-plugin-server.pluginmanifest.id.md) | PluginName | Identifier of the plugin. | +| [configPath](./kibana-plugin-server.pluginmanifest.configpath.md) | ConfigPath | Root [configuration path](./kibana-plugin-server.configpath.md) used by the plugin, defaults to "id" in snake\_case format. | +| [id](./kibana-plugin-server.pluginmanifest.id.md) | PluginName | Identifier of the plugin. Must be a string in camelCase. Part of a plugin public contract. Other plugins leverage it to access plugin API, navigate to the plugin, etc. | | [kibanaVersion](./kibana-plugin-server.pluginmanifest.kibanaversion.md) | string | The version of Kibana the plugin is compatible with, defaults to "version". | | [optionalPlugins](./kibana-plugin-server.pluginmanifest.optionalplugins.md) | readonly PluginName[] | An optional list of the other plugins that if installed and enabled \*\*may be\*\* leveraged by this plugin for some additional functionality but otherwise are not required for this plugin to work properly. | | [requiredPlugins](./kibana-plugin-server.pluginmanifest.requiredplugins.md) | readonly PluginName[] | An optional list of the other plugins that \*\*must be\*\* installed and enabled for this plugin to function properly. | diff --git a/src/core/CONVENTIONS.md b/src/core/CONVENTIONS.md index 61c5d5b076a44..dd83ab2daca82 100644 --- a/src/core/CONVENTIONS.md +++ b/src/core/CONVENTIONS.md @@ -38,6 +38,7 @@ my_plugin/    ├── index.ts    └── plugin.ts ``` +- [Manifest file](/docs/development/core/server/kibana-plugin-server.pluginmanifest.md) should be defined on top level. - Both `server` and `public` should have an `index.ts` and a `plugin.ts` file: - `index.ts` should only contain: - The `plugin` export diff --git a/src/core/public/application/__snapshots__/application_service.test.ts.snap b/src/core/public/application/__snapshots__/application_service.test.ts.snap new file mode 100644 index 0000000000000..376b320b64ea9 --- /dev/null +++ b/src/core/public/application/__snapshots__/application_service.test.ts.snap @@ -0,0 +1,84 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`#start() getComponent returns renderable JSX tree 1`] = ` + +`; diff --git a/src/core/public/application/application_service.test.ts b/src/core/public/application/application_service.test.ts index 4672a42c9eb06..54489fbd182b4 100644 --- a/src/core/public/application/application_service.test.ts +++ b/src/core/public/application/application_service.test.ts @@ -525,17 +525,7 @@ describe('#start()', () => { const { getComponent } = await service.start(startDeps); expect(() => shallow(createElement(getComponent))).not.toThrow(); - expect(getComponent()).toMatchInlineSnapshot(` - - `); + expect(getComponent()).toMatchSnapshot(); }); it('renders null when in legacy mode', async () => { diff --git a/src/core/public/application/application_service.tsx b/src/core/public/application/application_service.tsx index c69b96274aa95..4d714c8f9dad2 100644 --- a/src/core/public/application/application_service.tsx +++ b/src/core/public/application/application_service.tsx @@ -19,7 +19,7 @@ import React from 'react'; import { BehaviorSubject, Observable, Subject, Subscription } from 'rxjs'; -import { map, takeUntil } from 'rxjs/operators'; +import { map, shareReplay, takeUntil } from 'rxjs/operators'; import { createBrowserHistory, History } from 'history'; import { InjectedMetadataSetup } from '../injected_metadata'; @@ -256,6 +256,11 @@ export class ApplicationService { ) .subscribe(apps => applications$.next(apps)); + const applicationStatuses$ = applications$.pipe( + map(apps => new Map([...apps.entries()].map(([id, app]) => [id, app.status!]))), + shareReplay(1) + ); + return { applications$, capabilities, @@ -264,11 +269,6 @@ export class ApplicationService { getUrlForApp: (appId, { path }: { path?: string } = {}) => getAppUrl(availableMounters, appId, path), navigateToApp: async (appId, { path, state }: { path?: string; state?: any } = {}) => { - const app = applications$.value.get(appId); - if (app && app.status !== AppStatus.accessible) { - // should probably redirect to the error page instead - throw new Error(`Trying to navigate to an inaccessible application: ${appId}`); - } if (await this.shouldNavigate(overlays)) { this.appLeaveHandlers.delete(this.currentAppId$.value!); this.navigate!(getAppUrl(availableMounters, appId, path), state); @@ -283,6 +283,7 @@ export class ApplicationService { ); diff --git a/src/core/public/application/integration_tests/router.test.tsx b/src/core/public/application/integration_tests/router.test.tsx index cc71cf8722df4..4d83ab67810af 100644 --- a/src/core/public/application/integration_tests/router.test.tsx +++ b/src/core/public/application/integration_tests/router.test.tsx @@ -18,15 +18,18 @@ */ import React from 'react'; +import { BehaviorSubject } from 'rxjs'; import { createMemoryHistory, History, createHashHistory } from 'history'; import { AppRouter, AppNotFound } from '../ui'; import { EitherApp, MockedMounterMap, MockedMounterTuple } from '../test_types'; import { createRenderer, createAppMounter, createLegacyAppMounter } from './utils'; +import { AppStatus } from '../types'; describe('AppContainer', () => { let mounters: MockedMounterMap; let history: History; + let appStatuses$: BehaviorSubject>; let update: ReturnType; const navigate = (path: string) => { @@ -38,6 +41,17 @@ describe('AppContainer', () => { new Map([...mounters].map(([appId, { mounter }]) => [appId, mounter])); const setAppLeaveHandlerMock = () => undefined; + const mountersToAppStatus$ = () => { + return new BehaviorSubject( + new Map( + [...mounters.keys()].map(id => [ + id, + id.startsWith('disabled') ? AppStatus.inaccessible : AppStatus.accessible, + ]) + ) + ); + }; + beforeEach(() => { mounters = new Map([ createAppMounter('app1', 'App 1'), @@ -45,12 +59,16 @@ describe('AppContainer', () => { createAppMounter('app2', '
App 2
'), createLegacyAppMounter('baseApp:legacyApp2', jest.fn()), createAppMounter('app3', '
App 3
', '/custom/path'), + createAppMounter('disabledApp', '
Disabled app
'), + createLegacyAppMounter('disabledLegacyApp', jest.fn()), ] as Array>); history = createMemoryHistory(); + appStatuses$ = mountersToAppStatus$(); update = createRenderer( ); @@ -89,6 +107,7 @@ describe('AppContainer', () => { ); @@ -107,6 +126,7 @@ describe('AppContainer', () => { ); @@ -147,6 +167,7 @@ describe('AppContainer', () => { ); @@ -202,4 +223,16 @@ describe('AppContainer', () => { expect(dom?.exists(AppNotFound)).toBe(true); }); + + it('displays error page if app is inaccessible', async () => { + const dom = await navigate('/app/disabledApp'); + + expect(dom?.exists(AppNotFound)).toBe(true); + }); + + it('displays error page if legacy app is inaccessible', async () => { + const dom = await navigate('/app/disabledLegacyApp'); + + expect(dom?.exists(AppNotFound)).toBe(true); + }); }); diff --git a/src/core/public/application/ui/app_container.tsx b/src/core/public/application/ui/app_container.tsx index 8afd4d0ca0551..6a630608b2c20 100644 --- a/src/core/public/application/ui/app_container.tsx +++ b/src/core/public/application/ui/app_container.tsx @@ -26,12 +26,13 @@ import React, { MutableRefObject, } from 'react'; -import { AppUnmount, Mounter, AppLeaveHandler } from '../types'; +import { AppLeaveHandler, AppStatus, AppUnmount, Mounter } from '../types'; import { AppNotFound } from './app_not_found_screen'; interface Props { appId: string; mounter?: Mounter; + appStatus: AppStatus; setAppLeaveHandler: (appId: string, handler: AppLeaveHandler) => void; } @@ -39,10 +40,12 @@ export const AppContainer: FunctionComponent = ({ mounter, appId, setAppLeaveHandler, + appStatus, }: Props) => { const [appNotFound, setAppNotFound] = useState(false); const elementRef = useRef(null); const unmountRef: MutableRefObject = useRef(null); + // const appStatus = useObservable(appStatus$); useLayoutEffect(() => { const unmount = () => { @@ -52,7 +55,7 @@ export const AppContainer: FunctionComponent = ({ } }; const mount = async () => { - if (!mounter) { + if (!mounter || appStatus !== AppStatus.accessible) { return setAppNotFound(true); } @@ -71,7 +74,7 @@ export const AppContainer: FunctionComponent = ({ mount(); return unmount; - }, [appId, mounter, setAppLeaveHandler]); + }, [appId, appStatus, mounter, setAppLeaveHandler]); return ( diff --git a/src/core/public/application/ui/app_not_found_screen.tsx b/src/core/public/application/ui/app_not_found_screen.tsx index 73a999c5dbf16..0d651ee048096 100644 --- a/src/core/public/application/ui/app_not_found_screen.tsx +++ b/src/core/public/application/ui/app_not_found_screen.tsx @@ -22,7 +22,7 @@ import React from 'react'; import { FormattedMessage } from '@kbn/i18n/react'; export const AppNotFound = () => ( - + ; history: History; + appStatuses$: Observable>; setAppLeaveHandler: (appId: string, handler: AppLeaveHandler) => void; } @@ -34,45 +37,59 @@ interface Params { appId: string; } -export const AppRouter: FunctionComponent = ({ history, mounters, setAppLeaveHandler }) => ( - - - {[...mounters].flatMap(([appId, mounter]) => - // Remove /app paths from the routes as they will be handled by the - // "named" route parameter `:appId` below - mounter.appBasePath.startsWith('/app') - ? [] - : [ - ( - - )} - />, - ] - )} - ) => { - // Find the mounter including legacy mounters with subapps: - const [id, mounter] = mounters.has(appId) - ? [appId, mounters.get(appId)] - : [...mounters].filter(([key]) => key.split(':')[0] === appId)[0] ?? []; +export const AppRouter: FunctionComponent = ({ + history, + mounters, + setAppLeaveHandler, + appStatuses$, +}) => { + const appStatuses = useObservable(appStatuses$, new Map()); + return ( + + + {[...mounters].flatMap(([appId, mounter]) => + // Remove /app paths from the routes as they will be handled by the + // "named" route parameter `:appId` below + mounter.appBasePath.startsWith('/app') + ? [] + : [ + ( + + )} + />, + ] + )} + ) => { + // Find the mounter including legacy mounters with subapps: + const [id, mounter] = mounters.has(appId) + ? [appId, mounters.get(appId)] + : [...mounters].filter(([key]) => key.split(':')[0] === appId)[0] ?? []; - return ( - - ); - }} - /> - - -); + return ( + + ); + }} + /> + + + ); +}; diff --git a/src/core/server/config/integration_tests/config_deprecation.test.ts b/src/core/server/config/integration_tests/config_deprecation.test.ts index e85f8567bfc68..5bc3887f05f93 100644 --- a/src/core/server/config/integration_tests/config_deprecation.test.ts +++ b/src/core/server/config/integration_tests/config_deprecation.test.ts @@ -36,7 +36,13 @@ describe('configuration deprecations', () => { await root.setup(); const logs = loggingServiceMock.collect(mockLoggingService); - expect(logs.warn).toMatchInlineSnapshot(`Array []`); + const warnings = logs.warn.flatMap(i => i); + expect(warnings).not.toContain( + '"optimize.lazy" is deprecated and has been replaced by "optimize.watch"' + ); + expect(warnings).not.toContain( + '"optimize.lazyPort" is deprecated and has been replaced by "optimize.watchPort"' + ); }); it('should log deprecation warnings for core deprecations', async () => { @@ -50,15 +56,12 @@ describe('configuration deprecations', () => { await root.setup(); const logs = loggingServiceMock.collect(mockLoggingService); - expect(logs.warn).toMatchInlineSnapshot(` - Array [ - Array [ - "\\"optimize.lazy\\" is deprecated and has been replaced by \\"optimize.watch\\"", - ], - Array [ - "\\"optimize.lazyPort\\" is deprecated and has been replaced by \\"optimize.watchPort\\"", - ], - ] - `); + const warnings = logs.warn.flatMap(i => i); + expect(warnings).toContain( + '"optimize.lazy" is deprecated and has been replaced by "optimize.watch"' + ); + expect(warnings).toContain( + '"optimize.lazyPort" is deprecated and has been replaced by "optimize.watchPort"' + ); }); }); diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 50d291b173640..3f67b9a656bb7 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -283,7 +283,7 @@ export interface RequestHandlerContext { * * @public */ -export interface CoreSetup { +export interface CoreSetup { /** {@link CapabilitiesSetup} */ capabilities: CapabilitiesSetup; /** {@link ContextSetup} */ @@ -298,6 +298,13 @@ export interface CoreSetup { uiSettings: UiSettingsServiceSetup; /** {@link UuidServiceSetup} */ uuid: UuidServiceSetup; + /** + * Allows plugins to get access to APIs available in start inside async handlers. + * Promise will not resolve until Core and plugin dependencies have completed `start`. + * This should only be used inside handlers registered during `setup` that will only be executed + * after `start` lifecycle. + */ + getStartServices(): Promise<[CoreStart, TPluginsStart]>; } /** diff --git a/src/core/server/legacy/legacy_service.ts b/src/core/server/legacy/legacy_service.ts index ffcbf1662ee85..07cc933033054 100644 --- a/src/core/server/legacy/legacy_service.ts +++ b/src/core/server/legacy/legacy_service.ts @@ -256,6 +256,12 @@ export class LegacyService implements CoreService { startDeps: LegacyServiceStartDeps, legacyPlugins: LegacyPlugins ) { + const coreStart: CoreStart = { + capabilities: startDeps.core.capabilities, + savedObjects: { getScopedClient: startDeps.core.savedObjects.getScopedClient }, + uiSettings: { asScopedToClient: startDeps.core.uiSettings.asScopedToClient }, + }; + const coreSetup: CoreSetup = { capabilities: setupDeps.core.capabilities, context: setupDeps.core.context, @@ -291,11 +297,7 @@ export class LegacyService implements CoreService { uuid: { getInstanceUuid: setupDeps.core.uuid.getInstanceUuid, }, - }; - const coreStart: CoreStart = { - capabilities: startDeps.core.capabilities, - savedObjects: { getScopedClient: startDeps.core.savedObjects.getScopedClient }, - uiSettings: { asScopedToClient: startDeps.core.uiSettings.asScopedToClient }, + getStartServices: () => Promise.resolve([coreStart, startDeps.plugins]), }; // eslint-disable-next-line @typescript-eslint/no-var-requires diff --git a/src/core/server/mocks.ts b/src/core/server/mocks.ts index c7082d46313ae..c0a8973d98a54 100644 --- a/src/core/server/mocks.ts +++ b/src/core/server/mocks.ts @@ -86,6 +86,8 @@ function pluginInitializerContextMock(config: T = {} as T) { return mock; } +type CoreSetupMockType = MockedKeys & jest.Mocked>; + function createCoreSetupMock() { const httpService = httpServiceMock.createSetupContract(); const httpMock: jest.Mocked = { @@ -105,7 +107,7 @@ function createCoreSetupMock() { const uiSettingsMock = { register: uiSettingsServiceMock.createSetupContract().register, }; - const mock: MockedKeys = { + const mock: CoreSetupMockType = { capabilities: capabilitiesServiceMock.createSetupContract(), context: contextServiceMock.createSetupContract(), elasticsearch: elasticsearchServiceMock.createSetup(), @@ -113,6 +115,9 @@ function createCoreSetupMock() { savedObjects: savedObjectsServiceMock.createSetupContract(), uiSettings: uiSettingsMock, uuid: uuidServiceMock.createSetupContract(), + getStartServices: jest + .fn, object]>, []>() + .mockResolvedValue([createCoreStartMock(), {}]), }; return mock; diff --git a/src/core/server/plugins/discovery/is_camel_case.test.ts b/src/core/server/plugins/discovery/is_camel_case.test.ts new file mode 100644 index 0000000000000..eb8cb8f170dac --- /dev/null +++ b/src/core/server/plugins/discovery/is_camel_case.test.ts @@ -0,0 +1,42 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { isCamelCase } from './is_camel_case'; + +describe('isCamelCase', () => { + it('matches a string in camelCase', () => { + expect(isCamelCase('foo')).toBe(true); + expect(isCamelCase('foo1')).toBe(true); + expect(isCamelCase('fooBar')).toBe(true); + expect(isCamelCase('fooBarBaz')).toBe(true); + expect(isCamelCase('fooBAR')).toBe(true); + }); + + it('does not match strings in other cases', () => { + expect(isCamelCase('AAA')).toBe(false); + expect(isCamelCase('FooBar')).toBe(false); + expect(isCamelCase('3Foo')).toBe(false); + expect(isCamelCase('o_O')).toBe(false); + expect(isCamelCase('foo_bar')).toBe(false); + expect(isCamelCase('foo_')).toBe(false); + expect(isCamelCase('_fooBar')).toBe(false); + expect(isCamelCase('fooBar_')).toBe(false); + expect(isCamelCase('_fooBar_')).toBe(false); + }); +}); diff --git a/src/core/server/plugins/discovery/is_camel_case.ts b/src/core/server/plugins/discovery/is_camel_case.ts new file mode 100644 index 0000000000000..12069ec473f8d --- /dev/null +++ b/src/core/server/plugins/discovery/is_camel_case.ts @@ -0,0 +1,22 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +const camelCaseRegExp = /^[a-z]{1}([a-zA-Z0-9]{1,})$/; +export function isCamelCase(candidate: string) { + return camelCaseRegExp.test(candidate); +} diff --git a/src/core/server/plugins/discovery/plugin_manifest_parser.test.ts b/src/core/server/plugins/discovery/plugin_manifest_parser.test.ts index 17b1ac7b86045..979accb1f769e 100644 --- a/src/core/server/plugins/discovery/plugin_manifest_parser.test.ts +++ b/src/core/server/plugins/discovery/plugin_manifest_parser.test.ts @@ -20,10 +20,12 @@ import { PluginDiscoveryErrorType } from './plugin_discovery_error'; import { mockReadFile } from './plugin_manifest_parser.test.mocks'; +import { loggingServiceMock } from '../../logging/logging_service.mock'; import { resolve } from 'path'; import { parseManifest } from './plugin_manifest_parser'; +const logger = loggingServiceMock.createLogger(); const pluginPath = resolve('path', 'existent-dir'); const pluginManifestPath = resolve(pluginPath, 'kibana.json'); const packageInfo = { @@ -43,7 +45,7 @@ test('return error when manifest is empty', async () => { cb(null, Buffer.from('')); }); - await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({ + await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({ message: `Unexpected end of JSON input (invalid-manifest, ${pluginManifestPath})`, type: PluginDiscoveryErrorType.InvalidManifest, path: pluginManifestPath, @@ -55,7 +57,7 @@ test('return error when manifest content is null', async () => { cb(null, Buffer.from('null')); }); - await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({ + await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({ message: `Plugin manifest must contain a JSON encoded object. (invalid-manifest, ${pluginManifestPath})`, type: PluginDiscoveryErrorType.InvalidManifest, path: pluginManifestPath, @@ -67,7 +69,7 @@ test('return error when manifest content is not a valid JSON', async () => { cb(null, Buffer.from('not-json')); }); - await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({ + await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({ message: `Unexpected token o in JSON at position 1 (invalid-manifest, ${pluginManifestPath})`, type: PluginDiscoveryErrorType.InvalidManifest, path: pluginManifestPath, @@ -79,7 +81,7 @@ test('return error when plugin id is missing', async () => { cb(null, Buffer.from(JSON.stringify({ version: 'some-version' }))); }); - await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({ + await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({ message: `Plugin manifest must contain an "id" property. (invalid-manifest, ${pluginManifestPath})`, type: PluginDiscoveryErrorType.InvalidManifest, path: pluginManifestPath, @@ -91,20 +93,36 @@ test('return error when plugin id includes `.` characters', async () => { cb(null, Buffer.from(JSON.stringify({ id: 'some.name', version: 'some-version' }))); }); - await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({ + await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({ message: `Plugin "id" must not include \`.\` characters. (invalid-manifest, ${pluginManifestPath})`, type: PluginDiscoveryErrorType.InvalidManifest, path: pluginManifestPath, }); }); +test('logs warning if pluginId is not in camelCase format', async () => { + mockReadFile.mockImplementation((path, cb) => { + cb(null, Buffer.from(JSON.stringify({ id: 'some_name', version: 'kibana', server: true }))); + }); + + expect(loggingServiceMock.collect(logger).warn).toHaveLength(0); + await parseManifest(pluginPath, packageInfo, logger); + expect(loggingServiceMock.collect(logger).warn).toMatchInlineSnapshot(` + Array [ + Array [ + "Expect plugin \\"id\\" in camelCase, but found: some_name", + ], + ] + `); +}); + test('return error when plugin version is missing', async () => { mockReadFile.mockImplementation((path, cb) => { - cb(null, Buffer.from(JSON.stringify({ id: 'some-id' }))); + cb(null, Buffer.from(JSON.stringify({ id: 'someId' }))); }); - await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({ - message: `Plugin manifest for "some-id" must contain a "version" property. (invalid-manifest, ${pluginManifestPath})`, + await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({ + message: `Plugin manifest for "someId" must contain a "version" property. (invalid-manifest, ${pluginManifestPath})`, type: PluginDiscoveryErrorType.InvalidManifest, path: pluginManifestPath, }); @@ -112,11 +130,11 @@ test('return error when plugin version is missing', async () => { test('return error when plugin expected Kibana version is lower than actual version', async () => { mockReadFile.mockImplementation((path, cb) => { - cb(null, Buffer.from(JSON.stringify({ id: 'some-id', version: '6.4.2' }))); + cb(null, Buffer.from(JSON.stringify({ id: 'someId', version: '6.4.2' }))); }); - await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({ - message: `Plugin "some-id" is only compatible with Kibana version "6.4.2", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`, + await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({ + message: `Plugin "someId" is only compatible with Kibana version "6.4.2", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`, type: PluginDiscoveryErrorType.IncompatibleVersion, path: pluginManifestPath, }); @@ -126,12 +144,12 @@ test('return error when plugin expected Kibana version cannot be interpreted as mockReadFile.mockImplementation((path, cb) => { cb( null, - Buffer.from(JSON.stringify({ id: 'some-id', version: '1.0.0', kibanaVersion: 'non-sem-ver' })) + Buffer.from(JSON.stringify({ id: 'someId', version: '1.0.0', kibanaVersion: 'non-sem-ver' })) ); }); - await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({ - message: `Plugin "some-id" is only compatible with Kibana version "non-sem-ver", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`, + await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({ + message: `Plugin "someId" is only compatible with Kibana version "non-sem-ver", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`, type: PluginDiscoveryErrorType.IncompatibleVersion, path: pluginManifestPath, }); @@ -139,11 +157,11 @@ test('return error when plugin expected Kibana version cannot be interpreted as test('return error when plugin config path is not a string', async () => { mockReadFile.mockImplementation((path, cb) => { - cb(null, Buffer.from(JSON.stringify({ id: 'some-id', version: '7.0.0', configPath: 2 }))); + cb(null, Buffer.from(JSON.stringify({ id: 'someId', version: '7.0.0', configPath: 2 }))); }); - await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({ - message: `The "configPath" in plugin manifest for "some-id" should either be a string or an array of strings. (invalid-manifest, ${pluginManifestPath})`, + await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({ + message: `The "configPath" in plugin manifest for "someId" should either be a string or an array of strings. (invalid-manifest, ${pluginManifestPath})`, type: PluginDiscoveryErrorType.InvalidManifest, path: pluginManifestPath, }); @@ -153,12 +171,12 @@ test('return error when plugin config path is an array that contains non-string mockReadFile.mockImplementation((path, cb) => { cb( null, - Buffer.from(JSON.stringify({ id: 'some-id', version: '7.0.0', configPath: ['config', 2] })) + Buffer.from(JSON.stringify({ id: 'someId', version: '7.0.0', configPath: ['config', 2] })) ); }); - await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({ - message: `The "configPath" in plugin manifest for "some-id" should either be a string or an array of strings. (invalid-manifest, ${pluginManifestPath})`, + await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({ + message: `The "configPath" in plugin manifest for "someId" should either be a string or an array of strings. (invalid-manifest, ${pluginManifestPath})`, type: PluginDiscoveryErrorType.InvalidManifest, path: pluginManifestPath, }); @@ -166,11 +184,11 @@ test('return error when plugin config path is an array that contains non-string test('return error when plugin expected Kibana version is higher than actual version', async () => { mockReadFile.mockImplementation((path, cb) => { - cb(null, Buffer.from(JSON.stringify({ id: 'some-id', version: '7.0.1' }))); + cb(null, Buffer.from(JSON.stringify({ id: 'someId', version: '7.0.1' }))); }); - await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({ - message: `Plugin "some-id" is only compatible with Kibana version "7.0.1", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`, + await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({ + message: `Plugin "someId" is only compatible with Kibana version "7.0.1", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`, type: PluginDiscoveryErrorType.IncompatibleVersion, path: pluginManifestPath, }); @@ -178,11 +196,11 @@ test('return error when plugin expected Kibana version is higher than actual ver test('return error when both `server` and `ui` are set to `false` or missing', async () => { mockReadFile.mockImplementation((path, cb) => { - cb(null, Buffer.from(JSON.stringify({ id: 'some-id', version: '7.0.0' }))); + cb(null, Buffer.from(JSON.stringify({ id: 'someId', version: '7.0.0' }))); }); - await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({ - message: `Both "server" and "ui" are missing or set to "false" in plugin manifest for "some-id", but at least one of these must be set to "true". (invalid-manifest, ${pluginManifestPath})`, + await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({ + message: `Both "server" and "ui" are missing or set to "false" in plugin manifest for "someId", but at least one of these must be set to "true". (invalid-manifest, ${pluginManifestPath})`, type: PluginDiscoveryErrorType.InvalidManifest, path: pluginManifestPath, }); @@ -190,12 +208,12 @@ test('return error when both `server` and `ui` are set to `false` or missing', a mockReadFile.mockImplementation((path, cb) => { cb( null, - Buffer.from(JSON.stringify({ id: 'some-id', version: '7.0.0', server: false, ui: false })) + Buffer.from(JSON.stringify({ id: 'someId', version: '7.0.0', server: false, ui: false })) ); }); - await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({ - message: `Both "server" and "ui" are missing or set to "false" in plugin manifest for "some-id", but at least one of these must be set to "true". (invalid-manifest, ${pluginManifestPath})`, + await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({ + message: `Both "server" and "ui" are missing or set to "false" in plugin manifest for "someId", but at least one of these must be set to "true". (invalid-manifest, ${pluginManifestPath})`, type: PluginDiscoveryErrorType.InvalidManifest, path: pluginManifestPath, }); @@ -207,7 +225,7 @@ test('return error when manifest contains unrecognized properties', async () => null, Buffer.from( JSON.stringify({ - id: 'some-id', + id: 'someId', version: '7.0.0', server: true, unknownOne: 'one', @@ -217,21 +235,69 @@ test('return error when manifest contains unrecognized properties', async () => ); }); - await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({ - message: `Manifest for plugin "some-id" contains the following unrecognized properties: unknownOne,unknownTwo. (invalid-manifest, ${pluginManifestPath})`, + await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({ + message: `Manifest for plugin "someId" contains the following unrecognized properties: unknownOne,unknownTwo. (invalid-manifest, ${pluginManifestPath})`, type: PluginDiscoveryErrorType.InvalidManifest, path: pluginManifestPath, }); }); +describe('configPath', () => { + test('falls back to plugin id if not specified', async () => { + mockReadFile.mockImplementation((path, cb) => { + cb(null, Buffer.from(JSON.stringify({ id: 'plugin', version: '7.0.0', server: true }))); + }); + + const manifest = await parseManifest(pluginPath, packageInfo, logger); + expect(manifest.configPath).toBe(manifest.id); + }); + + test('falls back to plugin id in snakeCase format', async () => { + mockReadFile.mockImplementation((path, cb) => { + cb(null, Buffer.from(JSON.stringify({ id: 'SomeId', version: '7.0.0', server: true }))); + }); + + const manifest = await parseManifest(pluginPath, packageInfo, logger); + expect(manifest.configPath).toBe('some_id'); + }); + + test('not formated to snakeCase if defined explicitly as string', async () => { + mockReadFile.mockImplementation((path, cb) => { + cb( + null, + Buffer.from( + JSON.stringify({ id: 'someId', configPath: 'somePath', version: '7.0.0', server: true }) + ) + ); + }); + + const manifest = await parseManifest(pluginPath, packageInfo, logger); + expect(manifest.configPath).toBe('somePath'); + }); + + test('not formated to snakeCase if defined explicitly as an array of strings', async () => { + mockReadFile.mockImplementation((path, cb) => { + cb( + null, + Buffer.from( + JSON.stringify({ id: 'someId', configPath: ['somePath'], version: '7.0.0', server: true }) + ) + ); + }); + + const manifest = await parseManifest(pluginPath, packageInfo, logger); + expect(manifest.configPath).toEqual(['somePath']); + }); +}); + test('set defaults for all missing optional fields', async () => { mockReadFile.mockImplementation((path, cb) => { - cb(null, Buffer.from(JSON.stringify({ id: 'some-id', version: '7.0.0', server: true }))); + cb(null, Buffer.from(JSON.stringify({ id: 'someId', version: '7.0.0', server: true }))); }); - await expect(parseManifest(pluginPath, packageInfo)).resolves.toEqual({ - id: 'some-id', - configPath: 'some-id', + await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({ + id: 'someId', + configPath: 'some_id', version: '7.0.0', kibanaVersion: '7.0.0', optionalPlugins: [], @@ -247,7 +313,7 @@ test('return all set optional fields as they are in manifest', async () => { null, Buffer.from( JSON.stringify({ - id: 'some-id', + id: 'someId', configPath: ['some', 'path'], version: 'some-version', kibanaVersion: '7.0.0', @@ -259,8 +325,8 @@ test('return all set optional fields as they are in manifest', async () => { ); }); - await expect(parseManifest(pluginPath, packageInfo)).resolves.toEqual({ - id: 'some-id', + await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({ + id: 'someId', configPath: ['some', 'path'], version: 'some-version', kibanaVersion: '7.0.0', @@ -277,7 +343,7 @@ test('return manifest when plugin expected Kibana version matches actual version null, Buffer.from( JSON.stringify({ - id: 'some-id', + id: 'someId', configPath: 'some-path', version: 'some-version', kibanaVersion: '7.0.0-alpha2', @@ -288,8 +354,8 @@ test('return manifest when plugin expected Kibana version matches actual version ); }); - await expect(parseManifest(pluginPath, packageInfo)).resolves.toEqual({ - id: 'some-id', + await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({ + id: 'someId', configPath: 'some-path', version: 'some-version', kibanaVersion: '7.0.0-alpha2', @@ -306,7 +372,7 @@ test('return manifest when plugin expected Kibana version is `kibana`', async () null, Buffer.from( JSON.stringify({ - id: 'some-id', + id: 'someId', version: 'some-version', kibanaVersion: 'kibana', requiredPlugins: ['some-required-plugin'], @@ -317,9 +383,9 @@ test('return manifest when plugin expected Kibana version is `kibana`', async () ); }); - await expect(parseManifest(pluginPath, packageInfo)).resolves.toEqual({ - id: 'some-id', - configPath: 'some-id', + await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({ + id: 'someId', + configPath: 'some_id', version: 'some-version', kibanaVersion: 'kibana', optionalPlugins: [], diff --git a/src/core/server/plugins/discovery/plugin_manifest_parser.ts b/src/core/server/plugins/discovery/plugin_manifest_parser.ts index 93c993a0fa373..573109c9db35a 100644 --- a/src/core/server/plugins/discovery/plugin_manifest_parser.ts +++ b/src/core/server/plugins/discovery/plugin_manifest_parser.ts @@ -21,9 +21,12 @@ import { readFile, stat } from 'fs'; import { resolve } from 'path'; import { coerce } from 'semver'; import { promisify } from 'util'; +import { snakeCase } from 'lodash'; import { isConfigPath, PackageInfo } from '../../config'; +import { Logger } from '../../logging'; import { PluginManifest } from '../types'; import { PluginDiscoveryError } from './plugin_discovery_error'; +import { isCamelCase } from './is_camel_case'; const fsReadFileAsync = promisify(readFile); const fsStatAsync = promisify(stat); @@ -67,7 +70,7 @@ const KNOWN_MANIFEST_FIELDS = (() => { * @param packageInfo Kibana package info. * @internal */ -export async function parseManifest(pluginPath: string, packageInfo: PackageInfo) { +export async function parseManifest(pluginPath: string, packageInfo: PackageInfo, log: Logger) { const manifestPath = resolve(pluginPath, MANIFEST_FILE_NAME); let manifestContent; @@ -107,6 +110,10 @@ export async function parseManifest(pluginPath: string, packageInfo: PackageInfo ); } + if (!isCamelCase(manifest.id)) { + log.warn(`Expect plugin "id" in camelCase, but found: ${manifest.id}`); + } + if (!manifest.version || typeof manifest.version !== 'string') { throw PluginDiscoveryError.invalidManifest( manifestPath, @@ -161,7 +168,7 @@ export async function parseManifest(pluginPath: string, packageInfo: PackageInfo id: manifest.id, version: manifest.version, kibanaVersion: expectedKibanaVersion, - configPath: manifest.configPath || manifest.id, + configPath: manifest.configPath || snakeCase(manifest.id), requiredPlugins: Array.isArray(manifest.requiredPlugins) ? manifest.requiredPlugins : [], optionalPlugins: Array.isArray(manifest.optionalPlugins) ? manifest.optionalPlugins : [], ui: includesUiPlugin, diff --git a/src/core/server/plugins/discovery/plugins_discovery.ts b/src/core/server/plugins/discovery/plugins_discovery.ts index 79238afdf5c81..e7f82c9dc15ad 100644 --- a/src/core/server/plugins/discovery/plugins_discovery.ts +++ b/src/core/server/plugins/discovery/plugins_discovery.ts @@ -112,7 +112,7 @@ function processPluginSearchPaths$(pluginDirs: readonly string[], log: Logger) { * @param coreContext Kibana core context. */ function createPlugin$(path: string, log: Logger, coreContext: CoreContext) { - return from(parseManifest(path, coreContext.env.packageInfo)).pipe( + return from(parseManifest(path, coreContext.env.packageInfo, log)).pipe( map(manifest => { log.debug(`Successfully discovered plugin "${manifest.id}" at "${path}"`); const opaqueId = Symbol(manifest.id); diff --git a/src/core/server/plugins/integration_tests/plugins_service.test.mocks.ts b/src/core/server/plugins/integration_tests/plugins_service.test.mocks.ts new file mode 100644 index 0000000000000..d81a7eb5db4ae --- /dev/null +++ b/src/core/server/plugins/integration_tests/plugins_service.test.mocks.ts @@ -0,0 +1,27 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export const mockPackage = new Proxy( + { raw: { __dirname: '/tmp' } as any }, + { get: (obj, prop) => obj.raw[prop] } +); +jest.mock('../../../../core/server/utils/package_json', () => ({ pkg: mockPackage })); + +export const mockDiscover = jest.fn(); +jest.mock('../discovery/plugins_discovery', () => ({ discover: mockDiscover })); diff --git a/src/core/server/plugins/integration_tests/plugins_service.test.ts b/src/core/server/plugins/integration_tests/plugins_service.test.ts new file mode 100644 index 0000000000000..d5531478f03c5 --- /dev/null +++ b/src/core/server/plugins/integration_tests/plugins_service.test.ts @@ -0,0 +1,167 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { mockPackage, mockDiscover } from './plugins_service.test.mocks'; + +import { join } from 'path'; + +import { PluginsService } from '../plugins_service'; +import { ConfigPath, ConfigService, Env } from '../../config'; +import { getEnvOptions } from '../../config/__mocks__/env'; +import { BehaviorSubject, from } from 'rxjs'; +import { rawConfigServiceMock } from '../../config/raw_config_service.mock'; +import { config } from '../plugins_config'; +import { loggingServiceMock } from '../../logging/logging_service.mock'; +import { coreMock } from '../../mocks'; +import { Plugin } from '../types'; +import { PluginWrapper } from '../plugin'; + +describe('PluginsService', () => { + const logger = loggingServiceMock.create(); + let pluginsService: PluginsService; + + const createPlugin = ( + id: string, + { + path = id, + disabled = false, + version = 'some-version', + requiredPlugins = [], + optionalPlugins = [], + kibanaVersion = '7.0.0', + configPath = [path], + server = true, + ui = true, + }: { + path?: string; + disabled?: boolean; + version?: string; + requiredPlugins?: string[]; + optionalPlugins?: string[]; + kibanaVersion?: string; + configPath?: ConfigPath; + server?: boolean; + ui?: boolean; + } + ): PluginWrapper => { + return new PluginWrapper({ + path, + manifest: { + id, + version, + configPath: `${configPath}${disabled ? '-disabled' : ''}`, + kibanaVersion, + requiredPlugins, + optionalPlugins, + server, + ui, + }, + opaqueId: Symbol(id), + initializerContext: { logger } as any, + }); + }; + + beforeEach(async () => { + mockPackage.raw = { + branch: 'feature-v1', + version: 'v1', + build: { + distributable: true, + number: 100, + sha: 'feature-v1-build-sha', + }, + }; + + const env = Env.createDefault(getEnvOptions()); + const config$ = new BehaviorSubject>({ + plugins: { + initialize: true, + }, + }); + const rawConfigService = rawConfigServiceMock.create({ rawConfig$: config$ }); + const configService = new ConfigService(rawConfigService, env, logger); + await configService.setSchema(config.path, config.schema); + + pluginsService = new PluginsService({ + coreId: Symbol('core'), + env, + logger, + configService, + }); + }); + + it("properly resolves `getStartServices` in plugin's lifecycle", async () => { + expect.assertions(5); + + const pluginPath = 'plugin-path'; + + mockDiscover.mockReturnValue({ + error$: from([]), + plugin$: from([ + createPlugin('plugin-id', { + path: pluginPath, + configPath: 'path', + }), + ]), + }); + + let startDependenciesResolved = false; + let contextFromStart: any = null; + let contextFromStartService: any = null; + + const pluginInitializer = () => + ({ + setup: async (coreSetup, deps) => { + coreSetup.getStartServices().then(([core, plugins]) => { + startDependenciesResolved = true; + contextFromStartService = { core, plugins }; + }); + }, + start: async (core, plugins) => { + contextFromStart = { core, plugins }; + await new Promise(resolve => setTimeout(resolve, 10)); + expect(startDependenciesResolved).toBe(false); + }, + } as Plugin); + + jest.doMock( + join(pluginPath, 'server'), + () => ({ + plugin: pluginInitializer, + }), + { + virtual: true, + } + ); + + await pluginsService.discover(); + + const setupDeps = coreMock.createInternalSetup(); + await pluginsService.setup(setupDeps); + + expect(startDependenciesResolved).toBe(false); + + const startDeps = coreMock.createInternalStart(); + await pluginsService.start(startDeps); + + expect(startDependenciesResolved).toBe(true); + expect(contextFromStart!.core).toEqual(contextFromStartService!.core); + expect(contextFromStart!.plugins).toEqual(contextFromStartService!.plugins); + }); +}); diff --git a/src/core/server/plugins/plugin.test.ts b/src/core/server/plugins/plugin.test.ts index 10259b718577c..6875302f88a9d 100644 --- a/src/core/server/plugins/plugin.test.ts +++ b/src/core/server/plugins/plugin.test.ts @@ -237,6 +237,43 @@ test('`start` calls plugin.start with context and dependencies', async () => { expect(mockPluginInstance.start).toHaveBeenCalledWith(context, deps); }); +test("`start` resolves `startDependencies` Promise after plugin's start", async () => { + expect.assertions(2); + + const manifest = createPluginManifest(); + const opaqueId = Symbol(); + const plugin = new PluginWrapper({ + path: 'plugin-with-initializer-path', + manifest, + opaqueId, + initializerContext: createPluginInitializerContext(coreContext, opaqueId, manifest), + }); + const startContext = { any: 'thing' } as any; + const pluginDeps = { someDep: 'value' }; + + let startDependenciesResolved = false; + + const mockPluginInstance = { + setup: jest.fn(), + start: async () => { + // 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); + }, + }; + mockPluginInitializer.mockReturnValue(mockPluginInstance); + + await plugin.setup({} as any, {} as any); + + const startDependenciesCheck = plugin.startDependencies.then(resolvedStartDeps => { + startDependenciesResolved = true; + expect(resolvedStartDeps).toEqual([startContext, pluginDeps]); + }); + + await plugin.start(startContext, pluginDeps); + await startDependenciesCheck; +}); + test('`stop` fails if plugin is not set up', async () => { const manifest = createPluginManifest(); const opaqueId = Symbol(); diff --git a/src/core/server/plugins/plugin.ts b/src/core/server/plugins/plugin.ts index c0b484515ccce..d6c774f6fc41c 100644 --- a/src/core/server/plugins/plugin.ts +++ b/src/core/server/plugins/plugin.ts @@ -19,7 +19,8 @@ import { join } from 'path'; import typeDetect from 'type-detect'; - +import { Subject } from 'rxjs'; +import { first } from 'rxjs/operators'; import { Type } from '@kbn/config-schema'; import { Logger } from '../logging'; @@ -60,6 +61,9 @@ export class PluginWrapper< private instance?: Plugin; + private readonly startDependencies$ = new Subject<[CoreStart, TPluginsStart]>(); + public readonly startDependencies = this.startDependencies$.pipe(first()).toPromise(); + constructor( public readonly params: { readonly path: string; @@ -88,12 +92,12 @@ export class PluginWrapper< * @param plugins The dictionary where the key is the dependency name and the value * is the contract returned by the dependency's `setup` function. */ - public async setup(setupContext: CoreSetup, plugins: TPluginsSetup) { + public async setup(setupContext: CoreSetup, plugins: TPluginsSetup) { this.instance = this.createPluginInstance(); this.log.info('Setting up plugin'); - return await this.instance.setup(setupContext, plugins); + return this.instance.setup(setupContext, plugins); } /** @@ -108,7 +112,9 @@ export class PluginWrapper< throw new Error(`Plugin "${this.name}" can't be started since it isn't set up.`); } - return await this.instance.start(startContext, plugins); + const startContract = await this.instance.start(startContext, plugins); + this.startDependencies$.next([startContext, plugins]); + return startContract; } /** diff --git a/src/core/server/plugins/plugin_context.ts b/src/core/server/plugins/plugin_context.ts index 6d82a8d3ec6cf..f266172cb4bd9 100644 --- a/src/core/server/plugins/plugin_context.ts +++ b/src/core/server/plugins/plugin_context.ts @@ -176,6 +176,7 @@ export function createPluginSetupContext( uuid: { getInstanceUuid: deps.uuid.getInstanceUuid, }, + getStartServices: () => plugin.startDependencies, }; } diff --git a/src/core/server/plugins/types.ts b/src/core/server/plugins/types.ts index e717871912f46..a89e2f8c684e4 100644 --- a/src/core/server/plugins/types.ts +++ b/src/core/server/plugins/types.ts @@ -105,7 +105,8 @@ export type PluginOpaqueId = symbol; */ export interface PluginManifest { /** - * Identifier of the plugin. + * Identifier of the plugin. Must be a string in camelCase. Part of a plugin public contract. + * Other plugins leverage it to access plugin API, navigate to the plugin, etc. */ readonly id: PluginName; @@ -121,7 +122,11 @@ export interface PluginManifest { /** * Root {@link ConfigPath | configuration path} used by the plugin, defaults - * to "id". + * to "id" in snake_case format. + * + * @example + * id: myPlugin + * configPath: my_plugin */ readonly configPath: ConfigPath; @@ -162,7 +167,7 @@ export interface DiscoveredPlugin { readonly id: PluginName; /** - * Root configuration path used by the plugin, defaults to "id". + * Root configuration path used by the plugin, defaults to "id" in snake_case format. */ readonly configPath: ConfigPath; diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 6a58666716f42..6e41a4aefba30 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -552,13 +552,14 @@ export interface ContextSetup { export type CoreId = symbol; // @public -export interface CoreSetup { +export interface CoreSetup { // (undocumented) capabilities: CapabilitiesSetup; // (undocumented) context: ContextSetup; // (undocumented) elasticsearch: ElasticsearchServiceSetup; + getStartServices(): Promise<[CoreStart, TPluginsStart]>; // (undocumented) http: HttpServiceSetup; // (undocumented) @@ -2005,9 +2006,9 @@ export const validBodyOutput: readonly ["data", "stream"]; // src/core/server/legacy/types.ts:161:3 - (ae-forgotten-export) The symbol "LegacyAppSpec" needs to be exported by the entry point index.d.ts // src/core/server/legacy/types.ts:162:16 - (ae-forgotten-export) The symbol "LegacyPluginSpec" needs to be exported by the entry point index.d.ts // src/core/server/plugins/plugins_service.ts:43:5 - (ae-forgotten-export) The symbol "InternalPluginInfo" needs to be exported by the entry point index.d.ts -// src/core/server/plugins/types.ts:221:3 - (ae-forgotten-export) The symbol "KibanaConfigType" needs to be exported by the entry point index.d.ts -// src/core/server/plugins/types.ts:221:3 - (ae-forgotten-export) The symbol "SharedGlobalConfigKeys" needs to be exported by the entry point index.d.ts -// src/core/server/plugins/types.ts:222:3 - (ae-forgotten-export) The symbol "ElasticsearchConfigType" needs to be exported by the entry point index.d.ts -// src/core/server/plugins/types.ts:223:3 - (ae-forgotten-export) The symbol "PathConfigType" needs to be exported by the entry point index.d.ts +// src/core/server/plugins/types.ts:226:3 - (ae-forgotten-export) The symbol "KibanaConfigType" needs to be exported by the entry point index.d.ts +// src/core/server/plugins/types.ts:226:3 - (ae-forgotten-export) The symbol "SharedGlobalConfigKeys" needs to be exported by the entry point index.d.ts +// src/core/server/plugins/types.ts:227:3 - (ae-forgotten-export) The symbol "ElasticsearchConfigType" needs to be exported by the entry point index.d.ts +// src/core/server/plugins/types.ts:228:3 - (ae-forgotten-export) The symbol "PathConfigType" needs to be exported by the entry point index.d.ts ``` diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/components/table/table.test.tsx b/src/legacy/core_plugins/kibana/public/discover/np_ready/components/table/table.test.tsx index f816b33bcd0ae..386f405544a61 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/components/table/table.test.tsx +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/components/table/table.test.tsx @@ -26,50 +26,48 @@ import { IndexPattern, indexPatterns } from '../../../kibana_services'; jest.mock('ui/new_platform'); -// @ts-ignore const indexPattern = { - fields: { - getByName: (name: string) => { - const fields: { [name: string]: {} } = { - _index: { - name: '_index', - type: 'string', - scripted: false, - filterable: true, - }, - message: { - name: 'message', - type: 'string', - scripted: false, - filterable: false, - }, - extension: { - name: 'extension', - type: 'string', - scripted: false, - filterable: true, - }, - bytes: { - name: 'bytes', - type: 'number', - scripted: false, - filterable: true, - }, - scripted: { - name: 'scripted', - type: 'number', - scripted: true, - filterable: false, - }, - }; - return fields[name]; + fields: [ + { + name: '_index', + type: 'string', + scripted: false, + filterable: true, + }, + { + name: 'message', + type: 'string', + scripted: false, + filterable: false, + }, + { + name: 'extension', + type: 'string', + scripted: false, + filterable: true, }, - }, + { + name: 'bytes', + type: 'number', + scripted: false, + filterable: true, + }, + { + name: 'scripted', + type: 'number', + scripted: true, + filterable: false, + }, + ], metaFields: ['_index', '_score'], flattenHit: undefined, formatHit: jest.fn(hit => hit._source), } as IndexPattern; +indexPattern.fields.getByName = (name: string) => { + return indexPattern.fields.find(field => field.name === name); +}; + indexPattern.flattenHit = indexPatterns.flattenHitWrapper(indexPattern, indexPattern.metaFields); describe('DocViewTable at Discover', () => { diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/components/table/table.tsx b/src/legacy/core_plugins/kibana/public/discover/np_ready/components/table/table.tsx index 4bb2f83016c22..85689768eb88e 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/components/table/table.tsx +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/components/table/table.tsx @@ -17,6 +17,7 @@ * under the License. */ import React, { useState } from 'react'; +import { escapeRegExp } from 'lodash'; import { DocViewTableRow } from './table_row'; import { arrayContainsObjects, trimAngularSpan } from './table_helper'; import { DocViewRenderProps } from '../../doc_views/doc_views_types'; @@ -68,11 +69,57 @@ export function DocViewTable({ const displayNoMappingWarning = !mapping(field) && !displayUnderscoreWarning && !isArrayOfObjects; + // Discover doesn't flatten arrays of objects, so for documents with an `object` or `nested` field that + // contains an array, Discover will only detect the top level root field. We want to detect when those + // root fields are `nested` so that we can display the proper icon and label. However, those root + // `nested` fields are not a part of the index pattern. Their children are though, and contain nested path + // info. So to detect nested fields we look through the index pattern for nested children + // whose path begins with the current field. There are edge cases where + // this could incorrectly identify a plain `object` field as `nested`. Say we had the following document + // where `foo` is a plain object field and `bar` is a nested field. + // { + // "foo": [ + // { + // "bar": [ + // { + // "baz": "qux" + // } + // ] + // }, + // { + // "bar": [ + // { + // "baz": "qux" + // } + // ] + // } + // ] + // } + // + // The following code will search for `foo`, find it at the beginning of the path to the nested child field + // `foo.bar.baz` and incorrectly mark `foo` as nested. Any time we're searching for the name of a plain object + // field that happens to match a segment of a nested path, we'll get a false positive. + // We're aware of this issue and we'll have to live with + // it in the short term. The long term fix will be to add info about the `nested` and `object` root fields + // to the index pattern, but that has its own complications which you can read more about in the following + // issue: https://github.com/elastic/kibana/issues/54957 + const isNestedField = + !indexPattern.fields.find(patternField => patternField.name === field) && + !!indexPattern.fields.find(patternField => { + // We only want to match a full path segment + const nestedRootRegex = new RegExp(escapeRegExp(field) + '(\\.|$)'); + return nestedRootRegex.test(patternField.subType?.nested?.path ?? ''); + }); + const fieldType = isNestedField + ? 'nested' + : indexPattern.fields.find(patternField => patternField.name === field)?.type; + return ( )} - + {isCollapsible && ( diff --git a/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/edit_index_pattern.html b/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/edit_index_pattern.html index 4b3014fd28a51..625227be3c2d2 100644 --- a/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/edit_index_pattern.html +++ b/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/edit_index_pattern.html @@ -83,8 +83,8 @@