-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implements getStartServices
on server-side
#55156
Changes from all commits
4909a82
2d60b09
dbe6b50
7e02978
b3f8875
ea95905
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
<!-- Do not edit this file. It is automatically generated by API Documenter. --> | ||
|
||
[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. | ||
|
||
<b>Signature:</b> | ||
|
||
```typescript | ||
getStartServices(): Promise<[CoreStart, TPluginsStart]>; | ||
``` | ||
<b>Returns:</b> | ||
|
||
`Promise<[CoreStart, TPluginsStart]>` | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,26 +1,32 @@ | ||
<!-- Do not edit this file. It is automatically generated by API Documenter. --> | ||
|
||
[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. | ||
|
||
<b>Signature:</b> | ||
|
||
```typescript | ||
export interface CoreSetup | ||
``` | ||
|
||
## Properties | ||
|
||
| Property | Type | Description | | ||
| --- | --- | --- | | ||
| [capabilities](./kibana-plugin-server.coresetup.capabilities.md) | <code>CapabilitiesSetup</code> | [CapabilitiesSetup](./kibana-plugin-server.capabilitiessetup.md) | | ||
| [context](./kibana-plugin-server.coresetup.context.md) | <code>ContextSetup</code> | [ContextSetup](./kibana-plugin-server.contextsetup.md) | | ||
| [elasticsearch](./kibana-plugin-server.coresetup.elasticsearch.md) | <code>ElasticsearchServiceSetup</code> | [ElasticsearchServiceSetup](./kibana-plugin-server.elasticsearchservicesetup.md) | | ||
| [http](./kibana-plugin-server.coresetup.http.md) | <code>HttpServiceSetup</code> | [HttpServiceSetup](./kibana-plugin-server.httpservicesetup.md) | | ||
| [savedObjects](./kibana-plugin-server.coresetup.savedobjects.md) | <code>SavedObjectsServiceSetup</code> | [SavedObjectsServiceSetup](./kibana-plugin-server.savedobjectsservicesetup.md) | | ||
| [uiSettings](./kibana-plugin-server.coresetup.uisettings.md) | <code>UiSettingsServiceSetup</code> | [UiSettingsServiceSetup](./kibana-plugin-server.uisettingsservicesetup.md) | | ||
| [uuid](./kibana-plugin-server.coresetup.uuid.md) | <code>UuidServiceSetup</code> | [UuidServiceSetup](./kibana-plugin-server.uuidservicesetup.md) | | ||
|
||
<!-- Do not edit this file. It is automatically generated by API Documenter. --> | ||
|
||
[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. | ||
|
||
<b>Signature:</b> | ||
|
||
```typescript | ||
export interface CoreSetup<TPluginsStart extends object = object> | ||
``` | ||
|
||
## Properties | ||
|
||
| Property | Type | Description | | ||
| --- | --- | --- | | ||
| [capabilities](./kibana-plugin-server.coresetup.capabilities.md) | <code>CapabilitiesSetup</code> | [CapabilitiesSetup](./kibana-plugin-server.capabilitiessetup.md) | | ||
| [context](./kibana-plugin-server.coresetup.context.md) | <code>ContextSetup</code> | [ContextSetup](./kibana-plugin-server.contextsetup.md) | | ||
| [elasticsearch](./kibana-plugin-server.coresetup.elasticsearch.md) | <code>ElasticsearchServiceSetup</code> | [ElasticsearchServiceSetup](./kibana-plugin-server.elasticsearchservicesetup.md) | | ||
| [http](./kibana-plugin-server.coresetup.http.md) | <code>HttpServiceSetup</code> | [HttpServiceSetup](./kibana-plugin-server.httpservicesetup.md) | | ||
| [savedObjects](./kibana-plugin-server.coresetup.savedobjects.md) | <code>SavedObjectsServiceSetup</code> | [SavedObjectsServiceSetup](./kibana-plugin-server.savedobjectsservicesetup.md) | | ||
| [uiSettings](./kibana-plugin-server.coresetup.uisettings.md) | <code>UiSettingsServiceSetup</code> | [UiSettingsServiceSetup](./kibana-plugin-server.uisettingsservicesetup.md) | | ||
| [uuid](./kibana-plugin-server.coresetup.uuid.md) | <code>UuidServiceSetup</code> | [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 <code>start</code>. This should only be used inside handlers registered during <code>setup</code> that will only be executed after <code>start</code> lifecycle. | | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 })); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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'; | ||
Comment on lines
+20
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are very similar to the service's unit test. However the difference is that in this file, we do not mock the underlying plugin system, and call the service's |
||
|
||
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<Record<string, any>>({ | ||
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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be worth verifying that the resolved dependencies are the objects you expect them to be. |
||
expect(contextFromStart!.core).toEqual(contextFromStartService!.core); | ||
expect(contextFromStart!.plugins).toEqual(contextFromStartService!.plugins); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a hard time adding
getStartServices
to theCoreSetup
mock.MockedKeys
does not properly handles functions and resulted in the mock not being compatible with the concrete implementation. Usingjest.Mocked
orDeeplyMockedKeys
both resulted on errors on tests that were overriding read-only properties (that seems to be preserved by both but not byMockedKeys
), such askibana/x-pack/plugins/licensing/server/plugin.test.ts
Lines 54 to 55 in 56041f0
So I had to compose this custom type instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds like something we will face in the future again when marking more API points as
readonly
. We need to define a common pattern for configuring mocks at any deep level. Re-writing a property is the most obvious way, but create this incompatibility. Options:coreMock.createSetup({ elasticsearch: { dataClient: ... } });
That would quick grow with the number of nested objects.coreMock.get().withElasticsearchDataClient(...).withSomethingElse().build()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After a quick look, this is not the first time this occurs (the exemple is actually the root cause if this specific issue)
kibana/src/core/server/elasticsearch/elasticsearch_service.mock.ts
Lines 46 to 51 in 2d62ff2
We are also inconsistent in the types we actually return from mocks. Some are explicitly mock instance, some are not
kibana/src/core/server/mocks.ts
Lines 121 to 122 in ea9a7b8
vs
kibana/src/core/server/mocks.ts
Lines 131 to 132 in ea9a7b8
It does. however the (manually coded) factory pattern present the same (if not bigger) problem in term of maintenance. I'm not the biggest TS expert, but can't we have an utility type that would remove readonly from the mock definition while preserving compatibility with the concrete type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum, I was trying things and was not understanding why I couldnt override
core.elasticsearch.dataClient
from inside core... And I just saw that theMockedKeys
type is not the same betweensrc
andx-pack
... This explain why this is possible in the licensing plugin tests:kibana/typings/index.d.ts
Line 32 in eb03fd8
kibana/x-pack/typings/index.d.ts
Line 30 in 643f15c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is outside of the scope of this PR. Created #55189 to continue the discussion