useSchema everywhere for consistency. get rid of validateAll

src/core/server/config/config_service.mock.ts

@@ -22,17 +22,16 @@ import { ObjectToConfigAdapter } from './object_to_config_adapter';
 
 import { ConfigService } from './config_service';
 
-type ConfigSericeContract = PublicMethodsOf<ConfigService>;
+type ConfigServiceContract = PublicMethodsOf<ConfigService>;
 const createConfigServiceMock = () => {
-  const mocked: jest.Mocked<ConfigSericeContract> = {
+  const mocked: jest.Mocked<ConfigServiceContract> = {
     atPath: jest.fn(),
     getConfig$: jest.fn(),
     optionalAtPath: jest.fn(),
     getUsedPaths: jest.fn(),
     getUnusedPaths: jest.fn(),
     isEnabledAtPath: jest.fn(),
-    validateAll: jest.fn(),
-    setSchemaFor: jest.fn(),
+    setSchema: jest.fn(),
   };
   mocked.atPath.mockReturnValue(new BehaviorSubject({}));
   mocked.getConfig$.mockReturnValue(new BehaviorSubject(new ObjectToConfigAdapter({})));

src/core/server/config/config_service.test.ts

@@ -40,11 +40,10 @@ class ExampleClassWithStringSchema {
   constructor(readonly value: string) {}
 }
 
-const stringSchemaFor = (key: string): Array<[ConfigPath, Type<any>]> => [[key, schema.string()]];
-
 test('returns config at path as observable', async () => {
   const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 'foo' }));
-  const configService = new ConfigService(config$, defaultEnv, logger, stringSchemaFor('key'));
+  const configService = new ConfigService(config$, defaultEnv, logger);
+  configService.setSchema('key', schema.string());
 
   const configs = configService.atPath('key', ExampleClassWithStringSchema);
   const exampleConfig = await configs.pipe(first()).toPromise();
@@ -57,7 +56,8 @@ test('throws if config at path does not match schema', async () => {
 
   const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 123 }));
 
-  const configService = new ConfigService(config$, defaultEnv, logger, stringSchemaFor('key'));
+  const configService = new ConfigService(config$, defaultEnv, logger);
+  configService.setSchema('key', schema.string());
 
   const configs = configService.atPath('key', ExampleClassWithStringSchema);
 
@@ -70,12 +70,8 @@ test('throws if config at path does not match schema', async () => {
 
 test("returns undefined if fetching optional config at a path that doesn't exist", async () => {
   const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ foo: 'bar' }));
-  const configService = new ConfigService(
-    config$,
-    defaultEnv,
-    logger,
-    stringSchemaFor('unique-name')
-  );
+  const configService = new ConfigService(config$, defaultEnv, logger);
+  configService.setSchema('unique-name', schema.string());
 
   const configs = configService.optionalAtPath('unique-name', ExampleClassWithStringSchema);
   const exampleConfig = await configs.pipe(first()).toPromise();
@@ -85,7 +81,8 @@ test("returns undefined if fetching optional config at a path that doesn't exist
 
 test('returns observable config at optional path if it exists', async () => {
   const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ value: 'bar' }));
-  const configService = new ConfigService(config$, defaultEnv, logger, stringSchemaFor('value'));
+  const configService = new ConfigService(config$, defaultEnv, logger);
+  configService.setSchema('value', schema.string());
 
   const configs = configService.optionalAtPath('value', ExampleClassWithStringSchema);
   const exampleConfig: any = await configs.pipe(first()).toPromise();
@@ -96,7 +93,8 @@ test('returns observable config at optional path if it exists', async () => {
 
 test("does not push new configs when reloading if config at path hasn't changed", async () => {
   const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 'value' }));
-  const configService = new ConfigService(config$, defaultEnv, logger, stringSchemaFor('key'));
+  const configService = new ConfigService(config$, defaultEnv, logger);
+  configService.setSchema('key', schema.string());
 
   const valuesReceived: any[] = [];
   configService.atPath('key', ExampleClassWithStringSchema).subscribe(config => {
@@ -110,7 +108,8 @@ test("does not push new configs when reloading if config at path hasn't changed"
 
 test('pushes new config when reloading and config at path has changed', async () => {
   const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 'value' }));
-  const configService = new ConfigService(config$, defaultEnv, logger, stringSchemaFor('key'));
+  const configService = new ConfigService(config$, defaultEnv, logger);
+  configService.setSchema('key', schema.string());
 
   const valuesReceived: any[] = [];
   configService.atPath('key', ExampleClassWithStringSchema).subscribe(config => {
@@ -126,7 +125,8 @@ test("throws error if 'schema' is not defined for a key", async () => {
   class ExampleClass {}
 
   const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 'value' }));
-  const configService = new ConfigService(config$, defaultEnv, logger, stringSchemaFor('no-key'));
+  const configService = new ConfigService(config$, defaultEnv, logger);
+  configService.setSchema('no-key', schema.string());
 
   const configs = configService.atPath('key', ExampleClass as any);
 
@@ -135,6 +135,16 @@ test("throws error if 'schema' is not defined for a key", async () => {
   );
 });
 
+test("throws error if 'setSchema' called several times for the same key", async () => {
+  const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 'value' }));
+  const configService = new ConfigService(config$, defaultEnv, logger);
+  const addSchema = async () => await configService.setSchema('key', schema.string());
+  await addSchema();
+  expect(addSchema()).rejects.toMatchInlineSnapshot(
+    `[Error: Validation schema for key was already registered.]`
+  );
+});
+
 test('tracks unhandled paths', async () => {
   const initialConfig = {
     bar: {
@@ -204,8 +214,8 @@ test('correctly passes context', async () => {
       defaultValue: schema.contextRef('version'),
     }),
   });
-  const configService = new ConfigService(config$, env, logger, [['foo', schemaDefinition]]);
-
+  const configService = new ConfigService(config$, env, logger);
+  configService.setSchema('foo', schemaDefinition);
   const configs = configService.atPath('foo', createClassWithSchema(schemaDefinition));
 
   expect(await configs.pipe(first()).toPromise()).toMatchSnapshot();

src/core/server/config/config_service.ts

@@ -34,41 +34,32 @@ export class ConfigService {
    * then list all unhandled config paths when the startup process is completed.
    */
   private readonly handledPaths: ConfigPath[] = [];
-  private readonly schemas = new Map<string, Type<any>>();
+  private readonly schemas = new Map<string, Type<unknown>>();
 
   constructor(
     private readonly config$: Observable<Config>,
     private readonly env: Env,
-    logger: LoggerFactory,
-    coreServiceSchemas: Array<[ConfigPath, Type<any>]> = []
+    logger: LoggerFactory
   ) {
     this.log = logger.get('config');
-    for (const [path, schema] of coreServiceSchemas) {
-      this.setSchema(path, schema);
-    }
   }
 
   /**
    * Set config schema for a path and performs its validation
    */
-  public async setSchemaFor(path: ConfigPath, schema: Type<any>) {
-    this.setSchema(path, schema);
+  public async 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.`);
+    }
+
+    this.schemas.set(namespace, schema);
+
     await this.validateConfig(path)
       .pipe(first())
       .toPromise();
   }
 
-  /**
-   * Performs validation for all known validation schemas
-   */
-  public async validateAll() {
-    for (const namespace of this.schemas.keys()) {
-      await this.validateConfig(namespace)
-        .pipe(first())
-        .toPromise();
-    }
-  }
-
   /**
    * Returns the full config object observable. This is not intended for
    * "normal use", but for features that _need_ access to the full object.
@@ -85,7 +76,7 @@ export class ConfigService {
    * @param ConfigClass - A class (not an instance of a class) that contains a
    * static `schema` that we validate the config at the given `path` against.
    */
-  public atPath<TSchema extends Type<any>, TConfig>(
+  public atPath<TSchema extends Type<unknown>, TConfig>(
     path: ConfigPath,
     ConfigClass: ConfigWithSchema<TSchema, TConfig>
   ) {
@@ -165,8 +156,8 @@ export class ConfigService {
     );
   }
 
-  private createConfig<TSchema extends Type<any>, TConfig>(
-    validatedConfig: Record<string, any>,
+  private createConfig<TSchema extends Type<unknown>, TConfig>(
+    validatedConfig: unknown,
     ConfigClass: ConfigWithSchema<TSchema, TConfig>
   ) {
     return new ConfigClass(validatedConfig, this.env);
@@ -189,15 +180,6 @@ export class ConfigService {
     this.log.debug(`Marking config path as handled: ${path}`);
     this.handledPaths.push(path);
   }
-
-  /**
-   * Defines a validation schema for an appropriate path in config object.
-   * @internal
-   */
-  private setSchema(path: ConfigPath, schema: Type<any>) {
-    const namespace = pathToString(path);
-    this.schemas.set(namespace, schema);
-  }
 }
 
 const createPluginEnabledPath = (configPath: string | string[]) => {

src/core/server/http/http_service.test.ts

@@ -38,10 +38,9 @@ const createConfigService = (value: Partial<HttpConfigType> = {}) => {
       })
     ),
     env,
-    logger,
-    [[configDefinition.configPath, configDefinition.schema]]
+    logger
   );
-
+  configService.setSchema(configDefinition.configPath, configDefinition.schema);
   return configService;
 };
 

src/core/server/plugins/discovery/plugins_discovery.test.ts

@@ -144,9 +144,9 @@ test('properly iterates through plugin search locations', async () => {
       new ObjectToConfigAdapter({ plugins: { paths: [TEST_EXTRA_PLUGIN_PATH] } })
     ),
     env,
-    logger,
-    [[configDefinition.configPath, configDefinition.schema]]
+    logger
   );
+  configService.setSchema(configDefinition.configPath, configDefinition.schema);
 
   const pluginsConfig = await configService
     .atPath('plugins', PluginsConfig)

src/core/server/plugins/plugins_service.test.ts

@@ -61,9 +61,9 @@ beforeEach(() => {
   configService = new ConfigService(
     new BehaviorSubject<Config>(new ObjectToConfigAdapter({ plugins: { initialize: true } })),
     env,
-    logger,
-    [[configDefinition.configPath, configDefinition.schema]]
+    logger
   );
+  configService.setSchema(configDefinition.configPath, configDefinition.schema);
   pluginsService = new PluginsService({ env, logger, configService });
 
   [mockPluginSystem] = MockPluginsSystem.mock.instances as any;

src/core/server/plugins/plugins_service.ts

@@ -132,7 +132,7 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
         mergeMap(async plugin => {
           const isEnabled = await this.coreContext.configService.isEnabledAtPath(plugin.configPath);
           if (plugin.schema) {
-            await this.coreContext.configService.setSchemaFor(plugin.configPath, plugin.schema);
+            await this.coreContext.configService.setSchema(plugin.configPath, plugin.schema);
           }
 
           if (pluginEnableStatuses.has(plugin.name)) {

src/core/server/root/index.test.mocks.ts

@@ -29,5 +29,5 @@ jest.doMock('../config/config_service', () => ({
   ConfigService: jest.fn(() => configService),
 }));
 
-export const mockServer = { setup: jest.fn(), stop: jest.fn(), configService };
+export const mockServer = { preSetup: jest.fn(), setup: jest.fn(), stop: jest.fn(), configService };
 jest.mock('../server', () => ({ Server: jest.fn(() => mockServer) }));

src/core/server/root/index.ts

@@ -47,8 +47,9 @@ export class Root {
 
   public async setup() {
     try {
-      this.log.debug('setting up root');
+      await this.server.preSetup();
       await this.setupLogging();
+      this.log.debug('setting up root');
       return await this.server.setup();
     } catch (e) {
       await this.shutdown(e);

src/core/server/server.test.ts

@@ -103,11 +103,11 @@ test('stops services on "stop"', async () => {
 });
 
 test(`doesn't setup core services if config validation fails`, async () => {
-  configService.validateAll.mockImplementation(() => {
+  configService.setSchema.mockImplementation(() => {
     throw new Error('invalid config');
   });
   const server = new Server(config$, env, logger);
-  await expect(server.setup()).rejects.toThrowErrorMatchingInlineSnapshot(`"invalid config"`);
+  await expect(server.preSetup()).rejects.toThrowErrorMatchingInlineSnapshot(`"invalid config"`);
   expect(httpService.setup).not.toHaveBeenCalled();
   expect(elasticsearchService.setup).not.toHaveBeenCalled();
   expect(mockPluginsService.setup).not.toHaveBeenCalled();

src/core/server/server.ts

@@ -31,14 +31,6 @@ import { configDefinition as httpConfigDefinition } from './http';
 import { configDefinition as loggingConfigDefinition } from './logging';
 import { configDefinition as devConfigDefinition } from './dev';
 
-const schemas: Array<[ConfigPath, Type<any>]> = [
-  [elasticsearchConfigDefinition.configPath, elasticsearchConfigDefinition.schema],
-  [loggingConfigDefinition.configPath, loggingConfigDefinition.schema],
-  [httpConfigDefinition.configPath, httpConfigDefinition.schema],
-  [pluginsConfigDefinition.configPath, pluginsConfigDefinition.schema],
-  [devConfigDefinition.configPath, devConfigDefinition.schema],
-];
-
 export class Server {
   public readonly configService: ConfigService;
   private readonly elasticsearch: ElasticsearchService;
@@ -53,7 +45,7 @@ export class Server {
     private readonly logger: LoggerFactory
   ) {
     this.log = this.logger.get('server');
-    this.configService = new ConfigService(config$, env, logger, schemas);
+    this.configService = new ConfigService(config$, env, logger);
 
     const core = { configService: this.configService, env, logger };
     this.http = new HttpService(core);
@@ -62,8 +54,11 @@ export class Server {
     this.elasticsearch = new ElasticsearchService(core);
   }
 
+  public async preSetup() {
+    await this.setupConfigSchemas();
+  }
+
   public async setup() {
-    await this.configService.validateAll();
     this.log.debug('setting up server');
 
     const httpSetup = await this.http.setup();
@@ -115,4 +110,18 @@ export class Server {
     router.get({ path: '/', validate: false }, async (req, res) => res.ok({ version: '0.0.1' }));
     httpSetup.registerRouter(router);
   }
+
+  private async setupConfigSchemas() {
+    const schemas: Array<[ConfigPath, Type<unknown>]> = [
+      [elasticsearchConfigDefinition.configPath, elasticsearchConfigDefinition.schema],
+      [loggingConfigDefinition.configPath, loggingConfigDefinition.schema],
+      [httpConfigDefinition.configPath, httpConfigDefinition.schema],
+      [pluginsConfigDefinition.configPath, pluginsConfigDefinition.schema],
+      [devConfigDefinition.configPath, devConfigDefinition.schema],
+    ];
+
+    for (const [path, schema] of schemas) {
+      await this.configService.setSchema(path, schema);
+    }
+  }
 }