Skip to content

Commit

Permalink
Core capabilities: improve the performances of resolveCapabilities (#…
Browse files Browse the repository at this point in the history
…170454)

## Summary

Fix #146881

Introduce the concept of "capability path" to Core's capabilities API,
and rely on it to perform various performance optimization during
capabilities resolving.

### API Changes

#### CapabilitiesSetup.registerSwitcher

A new mandatory `capabilityPath` option was added to the API signature. 

Plugins registering capability switchers must now define the path(s) of
capabilities the switcher will impact.

E.g a live example with the `ml` capabilities switcher that was only
mutating `ml.{something}` capabilities:

*Before:*

```ts
coreSetup.capabilities.registerSwitcher(getSwitcher(license$, logger, enabledFeatures));
```

*After:*

```ts
  coreSetup.capabilities.registerSwitcher(getSwitcher(license$, logger, enabledFeatures), {
    capabilityPath: 'ml.*',
  });
```

#### CapabilitiesStart.resolveCapabilities

The `resolveCapabilities` was also changed accordingly, forcing API
consumers to specify the path(s) of capabilities they're planning to
access.

E.g for the `ml` plugin's capabilities resolving

*Before:*

```ts
const capabilities = await this.capabilities.resolveCapabilities(request);
return capabilities.ml as MlCapabilities;
```

*After:*

```ts
const capabilities = await this.capabilities.resolveCapabilities(request, {
   capabilityPath: 'ml.*',
});
return capabilities.ml as MlCapabilities;
```

### Performance optimizations

Knowing which capability path(s) the switchers are impacting and which
capability path(s) the resolver wants to use allow us to optimize the
way we're chaining the resolvers during the `resolveCapabilities`
internal implementation:

#### 1. We only apply switchers that may impact the paths the resolver
requested

E.g when requesting the ml capabilities, we now only apply the `ml`,
`security` and `spaces` switchers.

#### 2. We can call non-intersecting switchers in parallel 

Before, all switchers were executed in sequence. With these changes, we
now group non-intersecting switchers to resolve them in parallel.

E.g the `ml` (`ml.*`) and `fileUpload` (`fileUpload.*`) can be executed
and applied in parallel because they're not touching the same set of
capabilities.

---------

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
pgayvallet and kibanamachine authored Nov 9, 2023
1 parent 7ba20f6 commit 8b7c677
Show file tree
Hide file tree
Showing 34 changed files with 1,249 additions and 303 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import { mockCoreContext } from '@kbn/core-base-server-mocks';
import { mockRouter, RouterMock } from '@kbn/core-http-router-server-mocks';
import type { KibanaRequest } from '@kbn/core-http-server';
import {
httpServiceMock,
InternalHttpServicePrebootMock,
Expand All @@ -16,6 +17,8 @@ import {
import type { CapabilitiesSetup } from '@kbn/core-capabilities-server';
import { CapabilitiesService } from './capabilities_service';

const fakeRequest = {} as KibanaRequest;

describe('CapabilitiesService', () => {
let http: InternalHttpServiceSetupMock;
let service: CapabilitiesService;
Expand Down Expand Up @@ -71,7 +74,8 @@ describe('CapabilitiesService', () => {
catalogue: { myPlugin: true },
}));
const start = service.start();
expect(await start.resolveCapabilities({} as any)).toMatchInlineSnapshot(`
expect(await start.resolveCapabilities(fakeRequest, { capabilityPath: '*' }))
.toMatchInlineSnapshot(`
Object {
"catalogue": Object {
"myPlugin": true,
Expand Down Expand Up @@ -100,7 +104,8 @@ describe('CapabilitiesService', () => {
},
}));
const start = service.start();
expect(await start.resolveCapabilities({} as any)).toMatchInlineSnapshot(`
expect(await start.resolveCapabilities(fakeRequest, { capabilityPath: '*' }))
.toMatchInlineSnapshot(`
Object {
"catalogue": Object {
"A": true,
Expand All @@ -123,17 +128,21 @@ describe('CapabilitiesService', () => {
setup.registerProvider(() => ({
catalogue: { a: true, b: true, c: true },
}));
setup.registerSwitcher((req, capabilities) => {
return {
...capabilities,
catalogue: {
...capabilities.catalogue,
b: false,
},
};
});
setup.registerSwitcher(
(req, capabilities) => {
return {
...capabilities,
catalogue: {
...capabilities.catalogue,
b: false,
},
};
},
{ capabilityPath: '*' }
);
const start = service.start();
expect(await start.resolveCapabilities({} as any)).toMatchInlineSnapshot(`
expect(await start.resolveCapabilities(fakeRequest, { capabilityPath: '*' }))
.toMatchInlineSnapshot(`
Object {
"catalogue": Object {
"a": true,
Expand Down Expand Up @@ -162,29 +171,39 @@ describe('CapabilitiesService', () => {
c: true,
},
}));
setup.registerSwitcher((req, capabilities) => {
return {
catalogue: {
b: false,
},
};
});

setup.registerSwitcher((req, capabilities) => {
return {
navLinks: { c: false },
};
});
setup.registerSwitcher((req, capabilities) => {
return {
customSection: {
c: false,
},
};
});
setup.registerSwitcher(
(req, capabilities) => {
return {
catalogue: {
b: false,
},
};
},
{ capabilityPath: '*' }
);

setup.registerSwitcher(
(req, capabilities) => {
return {
navLinks: { c: false },
};
},
{ capabilityPath: '*' }
);
setup.registerSwitcher(
(req, capabilities) => {
return {
customSection: {
c: false,
},
};
},
{ capabilityPath: '*' }
);

const start = service.start();
expect(await start.resolveCapabilities({} as any)).toMatchInlineSnapshot(`
expect(await start.resolveCapabilities(fakeRequest, { capabilityPath: '*' }))
.toMatchInlineSnapshot(`
Object {
"catalogue": Object {
"a": true,
Expand All @@ -206,12 +225,15 @@ describe('CapabilitiesService', () => {

it('allows to indicate that default capabilities should be returned', async () => {
setup.registerProvider(() => ({ customSection: { isDefault: true } }));
setup.registerSwitcher((req, capabilities, useDefaultCapabilities) =>
useDefaultCapabilities ? capabilities : { customSection: { isDefault: false } }
setup.registerSwitcher(
(req, capabilities, useDefaultCapabilities) =>
useDefaultCapabilities ? capabilities : { customSection: { isDefault: false } },
{ capabilityPath: '*' }
);

const start = service.start();
expect(await start.resolveCapabilities({} as any)).toMatchInlineSnapshot(`
expect(await start.resolveCapabilities(fakeRequest, { capabilityPath: '*' }))
.toMatchInlineSnapshot(`
Object {
"catalogue": Object {},
"customSection": Object {
Expand All @@ -221,8 +243,12 @@ describe('CapabilitiesService', () => {
"navLinks": Object {},
}
`);
expect(await start.resolveCapabilities({} as any, { useDefaultCapabilities: false }))
.toMatchInlineSnapshot(`
expect(
await start.resolveCapabilities({} as any, {
useDefaultCapabilities: false,
capabilityPath: '*',
})
).toMatchInlineSnapshot(`
Object {
"catalogue": Object {},
"customSection": Object {
Expand All @@ -232,8 +258,12 @@ describe('CapabilitiesService', () => {
"navLinks": Object {},
}
`);
expect(await start.resolveCapabilities({} as any, { useDefaultCapabilities: true }))
.toMatchInlineSnapshot(`
expect(
await start.resolveCapabilities({} as any, {
useDefaultCapabilities: true,
capabilityPath: '*',
})
).toMatchInlineSnapshot(`
Object {
"catalogue": Object {},
"customSection": Object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ import type {
CapabilitiesSwitcher,
CapabilitiesStart,
CapabilitiesSetup,
CapabilitiesSwitcherOptions,
} from '@kbn/core-capabilities-server';
import type { SwitcherWithOptions } from './types';
import { mergeCapabilities } from './merge_capabilities';
import { getCapabilitiesResolver, CapabilitiesResolver } from './resolve_capabilities';
import { registerRoutes } from './routes';
Expand All @@ -41,8 +43,9 @@ const defaultCapabilities: Capabilities = {
export class CapabilitiesService {
private readonly logger: Logger;
private readonly capabilitiesProviders: CapabilitiesProvider[] = [];
private readonly capabilitiesSwitchers: CapabilitiesSwitcher[] = [];
private readonly capabilitiesSwitchers: SwitcherWithOptions[] = [];
private readonly resolveCapabilities: CapabilitiesResolver;
private started = false;

constructor(core: CoreContext) {
this.logger = core.logger.get('capabilities-service');
Expand Down Expand Up @@ -73,18 +76,38 @@ export class CapabilitiesService {

return {
registerProvider: (provider: CapabilitiesProvider) => {
if (this.started) {
throw new Error('registerProvider cannot be called after #start');
}
this.capabilitiesProviders.push(provider);
},
registerSwitcher: (switcher: CapabilitiesSwitcher) => {
this.capabilitiesSwitchers.push(switcher);
registerSwitcher: (switcher: CapabilitiesSwitcher, options: CapabilitiesSwitcherOptions) => {
if (this.started) {
throw new Error('registerSwitcher cannot be called after #start');
}
this.capabilitiesSwitchers.push({
switcher,
capabilityPath: Array.isArray(options.capabilityPath)
? options.capabilityPath
: [options.capabilityPath],
});
},
};
}

public start(): CapabilitiesStart {
this.started = true;

return {
resolveCapabilities: (request, options) =>
this.resolveCapabilities(request, [], options?.useDefaultCapabilities ?? false),
this.resolveCapabilities({
request,
capabilityPath: Array.isArray(options.capabilityPath)
? options.capabilityPath
: [options.capabilityPath],
useDefaultCapabilities: options.useDefaultCapabilities ?? false,
applications: [],
}),
};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

const actualHelpers = jest.requireActual('./resolve_helpers');

export const splitIntoBucketsMock = jest.fn().mockImplementation(actualHelpers.splitIntoBuckets);

jest.doMock('./resolve_helpers', () => {
return {
...actualHelpers,
splitIntoBuckets: splitIntoBucketsMock,
};
});
Loading

0 comments on commit 8b7c677

Please sign in to comment.