add synchronous config access API (#88981) (#89542)

* add synchronous config accessor * add `config.get` to plugin context and add tsdoc * remove useless markAsHandled calls * fix mocks * update generated docs * fix unit tests * add sync accessor for legacy config
elastic · Jan 28, 2021 · c7b900b · c7b900b
1 parent 758fdeb
commit c7b900b
Show file tree

Hide file tree

Showing 17 changed files with 478 additions and 190 deletions.
diff --git a/...opment/core/server/kibana-plugin-core-server.plugininitializercontext.config.md b/...opment/core/server/kibana-plugin-core-server.plugininitializercontext.config.md
@@ -4,14 +4,17 @@
 
 ## PluginInitializerContext.config property
 
+Accessors for the plugin's configuration
+
 <b>Signature:</b>
 
 ```typescript
 config: {
         legacy: {
             globalConfig$: Observable<SharedGlobalConfig>;
+            get: () => SharedGlobalConfig;
         };
         create: <T = ConfigSchema>() => Observable<T>;
-        createIfExists: <T = ConfigSchema>() => Observable<T | undefined>;
+        get: <T = ConfigSchema>() => T;
     };
 ```
diff --git a/...opment/core/server/kibana-plugin-core-server.plugininitializercontext.logger.md b/...opment/core/server/kibana-plugin-core-server.plugininitializercontext.logger.md
@@ -4,8 +4,29 @@
 
 ## PluginInitializerContext.logger property
 
+ instance already bound to the plugin's logging context
+
 <b>Signature:</b>
 
 ```typescript
 logger: LoggerFactory;
 ```
+
+## Example
+
+
+```typescript
+// plugins/my-plugin/server/plugin.ts
+// "id: myPlugin" in `plugins/my-plugin/kibana.yaml`
+
+export class MyPlugin implements Plugin  {
+  constructor(private readonly initContext: PluginInitializerContext) {
+    this.logger = initContext.logger.get();
+    // `logger` context: `plugins.myPlugin`
+    this.mySubLogger = initContext.logger.get('sub'); // or this.logger.get('sub');
+    // `mySubLogger` context: `plugins.myPlugin.sub`
+  }
+}
+
+```
+
diff --git a/docs/development/core/server/kibana-plugin-core-server.plugininitializercontext.md b/docs/development/core/server/kibana-plugin-core-server.plugininitializercontext.md
@@ -16,8 +16,8 @@ export interface PluginInitializerContext<ConfigSchema = unknown>
 
 |  Property | Type | Description |
 |  --- | --- | --- |
-|  [config](./kibana-plugin-core-server.plugininitializercontext.config.md) | <code>{</code><br/><code>        legacy: {</code><br/><code>            globalConfig$: Observable&lt;SharedGlobalConfig&gt;;</code><br/><code>        };</code><br/><code>        create: &lt;T = ConfigSchema&gt;() =&gt; Observable&lt;T&gt;;</code><br/><code>        createIfExists: &lt;T = ConfigSchema&gt;() =&gt; Observable&lt;T &#124; undefined&gt;;</code><br/><code>    }</code> |  |
+|  [config](./kibana-plugin-core-server.plugininitializercontext.config.md) | <code>{</code><br/><code>        legacy: {</code><br/><code>            globalConfig$: Observable&lt;SharedGlobalConfig&gt;;</code><br/><code>            get: () =&gt; SharedGlobalConfig;</code><br/><code>        };</code><br/><code>        create: &lt;T = ConfigSchema&gt;() =&gt; Observable&lt;T&gt;;</code><br/><code>        get: &lt;T = ConfigSchema&gt;() =&gt; T;</code><br/><code>    }</code> | Accessors for the plugin's configuration |
 |  [env](./kibana-plugin-core-server.plugininitializercontext.env.md) | <code>{</code><br/><code>        mode: EnvironmentMode;</code><br/><code>        packageInfo: Readonly&lt;PackageInfo&gt;;</code><br/><code>        instanceUuid: string;</code><br/><code>    }</code> |  |
-|  [logger](./kibana-plugin-core-server.plugininitializercontext.logger.md) | <code>LoggerFactory</code> |  |
+|  [logger](./kibana-plugin-core-server.plugininitializercontext.logger.md) | <code>LoggerFactory</code> |  instance already bound to the plugin's logging context |
 |  [opaqueId](./kibana-plugin-core-server.plugininitializercontext.opaqueid.md) | <code>PluginOpaqueId</code> |  |
 
diff --git a/packages/kbn-config/src/config_service.mock.ts b/packages/kbn-config/src/config_service.mock.ts
@@ -17,8 +17,8 @@ const createConfigServiceMock = ({
 }: { atPath?: Record<string, any>; getConfig$?: Record<string, any> } = {}) => {
   const mocked: jest.Mocked<IConfigService> = {
     atPath: jest.fn(),
+    atPathSync: jest.fn(),
     getConfig$: jest.fn(),
-    optionalAtPath: jest.fn(),
     getUsedPaths: jest.fn(),
     getUnusedPaths: jest.fn(),
     isEnabledAtPath: jest.fn(),
@@ -27,6 +27,7 @@ const createConfigServiceMock = ({
     validate: jest.fn(),
   };
   mocked.atPath.mockReturnValue(new BehaviorSubject(atPath));
+  mocked.atPathSync.mockReturnValue(atPath);
   mocked.getConfig$.mockReturnValue(new BehaviorSubject(new ObjectToConfigAdapter(getConfig$)));
   mocked.getUsedPaths.mockResolvedValue([]);
   mocked.getUnusedPaths.mockResolvedValue([]);

diff --git a/packages/kbn-config/src/config_service.test.ts b/packages/kbn-config/src/config_service.test.ts
@@ -105,27 +105,6 @@ test('re-validate config when updated', async () => {
   `);
 });
 
-test("returns undefined if fetching optional config at a path that doesn't exist", async () => {
-  const rawConfig = getRawConfigProvider({});
-  const configService = new ConfigService(rawConfig, defaultEnv, logger);
-
-  const value$ = configService.optionalAtPath('unique-name');
-  const value = await value$.pipe(first()).toPromise();
-
-  expect(value).toBeUndefined();
-});
-
-test('returns observable config at optional path if it exists', async () => {
-  const rawConfig = getRawConfigProvider({ value: 'bar' });
-  const configService = new ConfigService(rawConfig, defaultEnv, logger);
-  await configService.setSchema('value', schema.string());
-
-  const value$ = configService.optionalAtPath('value');
-  const value: any = await value$.pipe(first()).toPromise();
-
-  expect(value).toBe('bar');
-});
-
 test("does not push new configs when reloading if config at path hasn't changed", async () => {
   const rawConfig$ = new BehaviorSubject<Record<string, any>>({ key: 'value' });
   const rawConfigProvider = rawConfigServiceMock.create({ rawConfig$ });
@@ -209,34 +188,38 @@ test('flags schema paths as handled when registering a schema', async () => {
 
 test('tracks unhandled paths', async () => {
   const initialConfig = {
-    bar: {
-      deep1: {
-        key: '123',
-      },
-      deep2: {
-        key: '321',
-      },
+    service: {
+      string: 'str',
+      number: 42,
     },
-    foo: 'value',
-    quux: {
-      deep1: {
-        key: 'hello',
-      },
-      deep2: {
-        key: 'world',
-      },
+    plugin: {
+      foo: 'bar',
+    },
+    unknown: {
+      hello: 'dolly',
+      number: 9000,
     },
   };
 
   const rawConfigProvider = rawConfigServiceMock.create({ rawConfig: initialConfig });
   const configService = new ConfigService(rawConfigProvider, defaultEnv, logger);
-
-  configService.atPath('foo');
-  configService.atPath(['bar', 'deep2']);
+  await configService.setSchema(
+    'service',
+    schema.object({
+      string: schema.string(),
+      number: schema.number(),
+    })
+  );
+  await configService.setSchema(
+    'plugin',
+    schema.object({
+      foo: schema.string(),
+    })
+  );
 
   const unused = await configService.getUnusedPaths();
 
-  expect(unused).toEqual(['bar.deep1.key', 'quux.deep1.key', 'quux.deep2.key']);
+  expect(unused).toEqual(['unknown.hello', 'unknown.number']);
 });
 
 test('correctly passes context', async () => {
@@ -339,22 +322,18 @@ test('does not throw if schema does not define "enabled" schema', async () => {
 
   const rawConfigProvider = rawConfigServiceMock.create({ rawConfig: initialConfig });
   const configService = new ConfigService(rawConfigProvider, defaultEnv, logger);
-  await expect(
+  expect(
     configService.setSchema(
       'pid',
       schema.object({
         file: schema.string(),
       })
     )
-  ).resolves.toBeUndefined();
+  ).toBeUndefined();
 
   const value$ = configService.atPath('pid');
   const value: any = await value$.pipe(first()).toPromise();
   expect(value.enabled).toBe(undefined);
-
-  const valueOptional$ = configService.optionalAtPath('pid');
-  const valueOptional: any = await valueOptional$.pipe(first()).toPromise();
-  expect(valueOptional.enabled).toBe(undefined);
 });
 
 test('treats config as enabled if config path is not present in config', async () => {
@@ -457,3 +436,44 @@ test('logs deprecation warning during validation', async () => {
     ]
   `);
 });
+
+describe('atPathSync', () => {
+  test('returns the value at path', async () => {
+    const rawConfig = getRawConfigProvider({ key: 'foo' });
+    const configService = new ConfigService(rawConfig, defaultEnv, logger);
+    const stringSchema = schema.string();
+    await configService.setSchema('key', stringSchema);
+
+    await configService.validate();
+
+    const value = configService.atPathSync('key');
+    expect(value).toBe('foo');
+  });
+
+  test('throws if called before `validate`', async () => {
+    const rawConfig = getRawConfigProvider({ key: 'foo' });
+    const configService = new ConfigService(rawConfig, defaultEnv, logger);
+    const stringSchema = schema.string();
+    await configService.setSchema('key', stringSchema);
+
+    expect(() => configService.atPathSync('key')).toThrowErrorMatchingInlineSnapshot(
+      `"\`atPathSync\` called before config was validated"`
+    );
+  });
+
+  test('returns the last config value', async () => {
+    const rawConfig$ = new BehaviorSubject<Record<string, any>>({ key: 'value' });
+    const rawConfigProvider = rawConfigServiceMock.create({ rawConfig$ });
+
+    const configService = new ConfigService(rawConfigProvider, defaultEnv, logger);
+    await configService.setSchema('key', schema.string());
+
+    await configService.validate();
+
+    expect(configService.atPathSync('key')).toEqual('value');
+
+    rawConfig$.next({ key: 'new-value' });
+
+    expect(configService.atPathSync('key')).toEqual('new-value');
+  });
+});
diff --git a/packages/kbn-config/src/config_service.ts b/packages/kbn-config/src/config_service.ts
@@ -10,7 +10,7 @@ import type { PublicMethodsOf } from '@kbn/utility-types';
 import { Type } from '@kbn/config-schema';
 import { isEqual } from 'lodash';
 import { BehaviorSubject, combineLatest, Observable } from 'rxjs';
-import { distinctUntilChanged, first, map, shareReplay, take } from 'rxjs/operators';
+import { distinctUntilChanged, first, map, shareReplay, take, tap } from 'rxjs/operators';
 import { Logger, LoggerFactory } from '@kbn/logging';
 
 import { Config, ConfigPath, Env } from '.';
@@ -32,13 +32,15 @@ export class ConfigService {
   private readonly log: Logger;
   private readonly deprecationLog: Logger;
 
+  private validated = false;
   private readonly config$: Observable<Config>;
+  private lastConfig?: Config;
 
   /**
    * Whenever a config if read at a path, we mark that path as 'handled'. We can
    * then list all unhandled config paths when the startup process is completed.
    */
-  private readonly handledPaths: ConfigPath[] = [];
+  private readonly handledPaths: Set<ConfigPath> = new Set();
   private readonly schemas = new Map<string, Type<unknown>>();
   private readonly deprecations = new BehaviorSubject<ConfigDeprecationWithContext[]>([]);
 
@@ -55,14 +57,17 @@ export class ConfigService {
         const migrated = applyDeprecations(rawConfig, deprecations);
         return new LegacyObjectToConfigAdapter(migrated);
       }),
+      tap((config) => {
+        this.lastConfig = config;
+      }),
       shareReplay(1)
     );
   }
 
   /**
    * Set config schema for a path and performs its validation
    */
-  public async setSchema(path: ConfigPath, schema: Type<unknown>) {
+  public setSchema(path: ConfigPath, schema: Type<unknown>) {
     const namespace = pathToString(path);
     if (this.schemas.has(namespace)) {
       throw new Error(`Validation schema for [${path}] was already registered.`);
@@ -94,43 +99,43 @@ export class ConfigService {
   public async validate() {
     const namespaces = [...this.schemas.keys()];
     for (let i = 0; i < namespaces.length; i++) {
-      await this.validateConfigAtPath(namespaces[i]).pipe(first()).toPromise();
+      await this.getValidatedConfigAtPath$(namespaces[i]).pipe(first()).toPromise();
     }
 
     await this.logDeprecation();
+    this.validated = true;
   }
 
   /**
    * Returns the full config object observable. This is not intended for
-   * "normal use", but for features that _need_ access to the full object.
+   * "normal use", but for internal features that _need_ access to the full object.
    */
   public getConfig$() {
     return this.config$;
   }
 
   /**
    * Reads the subset of the config at the specified `path` and validates it
-   * against the static `schema` on the given `ConfigClass`.
+   * against its registered schema.
    *
    * @param path - The path to the desired subset of the config.
    */
   public atPath<TSchema>(path: ConfigPath) {
-    return this.validateConfigAtPath(path) as Observable<TSchema>;
+    return this.getValidatedConfigAtPath$(path) as Observable<TSchema>;
   }
 
   /**
-   * Same as `atPath`, but returns `undefined` if there is no config at the
-   * specified path.
+   * Similar to {@link atPath}, but return the last emitted value synchronously instead of an
+   * observable.
    *
-   * {@link ConfigService.atPath}
+   * @param path - The path to the desired subset of the config.
    */
-  public optionalAtPath<TSchema>(path: ConfigPath) {
-    return this.getDistinctConfig(path).pipe(
-      map((config) => {
-        if (config === undefined) return undefined;
-        return this.validateAtPath(path, config) as TSchema;
-      })
-    );
+  public atPathSync<TSchema>(path: ConfigPath) {
+    if (!this.validated) {
+      throw new Error('`atPathSync` called before config was validated');
+    }
+    const configAtPath = this.lastConfig!.get(path);
+    return this.validateAtPath(path, configAtPath) as TSchema;
   }
 
   public async isEnabledAtPath(path: ConfigPath) {
@@ -144,10 +149,7 @@ export class ConfigService {
     const config = await this.config$.pipe(first()).toPromise();
 
     // if plugin hasn't got a config schema, we try to read "enabled" directly
-    const isEnabled =
-      validatedConfig && validatedConfig.enabled !== undefined
-        ? validatedConfig.enabled
-        : config.get(enabledPath);
+    const isEnabled = validatedConfig?.enabled ?? config.get(enabledPath);
 
     // not declared. consider that plugin is enabled by default
     if (isEnabled === undefined) {
@@ -170,15 +172,13 @@ export class ConfigService {
 
   public async getUnusedPaths() {
     const config = await this.config$.pipe(first()).toPromise();
-    const handledPaths = this.handledPaths.map(pathToString);
-
+    const handledPaths = [...this.handledPaths.values()].map(pathToString);
     return config.getFlattenedPaths().filter((path) => !isPathHandled(path, handledPaths));
   }
 
   public async getUsedPaths() {
     const config = await this.config$.pipe(first()).toPromise();
-    const handledPaths = this.handledPaths.map(pathToString);
-
+    const handledPaths = [...this.handledPaths.values()].map(pathToString);
     return config.getFlattenedPaths().filter((path) => isPathHandled(path, handledPaths));
   }
 
@@ -210,22 +210,17 @@ export class ConfigService {
     );
   }
 
-  private validateConfigAtPath(path: ConfigPath) {
-    return this.getDistinctConfig(path).pipe(map((config) => this.validateAtPath(path, config)));
-  }
-
-  private getDistinctConfig(path: ConfigPath) {
-    this.markAsHandled(path);
-
+  private getValidatedConfigAtPath$(path: ConfigPath) {
     return this.config$.pipe(
       map((config) => config.get(path)),
-      distinctUntilChanged(isEqual)
+      distinctUntilChanged(isEqual),
+      map((config) => this.validateAtPath(path, config))
     );
   }
 
   private markAsHandled(path: ConfigPath) {
     this.log.debug(`Marking config path as handled: ${path}`);
-    this.handledPaths.push(path);
+    this.handledPaths.add(path);
   }
 }