move plugin discovery to plugin service pre-setup stage.

Set validation schemes in ConfigService.preSetup stage.

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

@@ -25,13 +25,13 @@ import { ConfigService } from './config_service';
 type ConfigSericeContract = PublicMethodsOf<ConfigService>;
 const createConfigServiceMock = () => {
   const mocked: jest.Mocked<ConfigSericeContract> = {
-    validateAll: jest.fn(),
     atPath: jest.fn(),
     getConfig$: jest.fn(),
     optionalAtPath: jest.fn(),
     getUsedPaths: jest.fn(),
     getUnusedPaths: jest.fn(),
     isEnabledAtPath: jest.fn(),
+    preSetup: 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,12 @@ class ExampleClassWithStringSchema {
   constructor(readonly value: string) {}
 }
 
-const stringSchemaFor = (key: string) => new Map([[key, ExampleClassWithStringSchema.schema]]);
+const stringSchemaFor = (key: string) => new Map([[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.preSetup(stringSchemaFor('key'));
 
   const configs = configService.atPath('key', ExampleClassWithStringSchema);
   const exampleConfig = await configs.pipe(first()).toPromise();
@@ -57,7 +58,9 @@ 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.preSetup(stringSchemaFor('key'));
+
   const configs = configService.atPath('key', ExampleClassWithStringSchema);
 
   try {
@@ -69,12 +72,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.preSetup(stringSchemaFor('unique-name'));
 
   const configs = configService.optionalAtPath('unique-name', ExampleClassWithStringSchema);
   const exampleConfig = await configs.pipe(first()).toPromise();
@@ -84,7 +83,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.preSetup(stringSchemaFor('value'));
 
   const configs = configService.optionalAtPath('value', ExampleClassWithStringSchema);
   const exampleConfig: any = await configs.pipe(first()).toPromise();
@@ -95,7 +95,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.preSetup(stringSchemaFor('key'));
 
   const valuesReceived: any[] = [];
   configService.atPath('key', ExampleClassWithStringSchema).subscribe(config => {
@@ -109,7 +110,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.preSetup(stringSchemaFor('key'));
 
   const valuesReceived: any[] = [];
   configService.atPath('key', ExampleClassWithStringSchema).subscribe(config => {
@@ -121,21 +123,29 @@ test('pushes new config when reloading and config at path has changed', async ()
   expect(valuesReceived).toEqual(['value', 'new value']);
 });
 
-test("throws error if 'schema' is not defined for a key", async () => {
-  expect.assertions(1);
-
+test('throws error if reads config before schema is set', async () => {
   class ExampleClass {}
 
   const config$ = new BehaviorSubject(new ObjectToConfigAdapter({ key: 'value' }));
-  const configService = new ConfigService(config$, defaultEnv, logger, new Map());
+  const configService = new ConfigService(config$, defaultEnv, logger);
+  const configs = configService.atPath('key', ExampleClass as any);
 
+  expect(configs.pipe(first()).toPromise()).rejects.toMatchInlineSnapshot(
+    `[Error: No validation schema has been defined]`
+  );
+});
+
+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);
+  configService.preSetup(stringSchemaFor('no-key'));
   const configs = configService.atPath('key', ExampleClass as any);
 
-  try {
-    await configs.pipe(first()).toPromise();
-  } catch (e) {
-    expect(e).toMatchSnapshot();
-  }
+  expect(configs.pipe(first()).toPromise()).rejects.toMatchInlineSnapshot(
+    `[Error: No config validator defined for key]`
+  );
 });
 
 test('tracks unhandled paths', async () => {
@@ -160,7 +170,7 @@ test('tracks unhandled paths', async () => {
   };
 
   const config$ = new BehaviorSubject(new ObjectToConfigAdapter(initialConfig));
-  const configService = new ConfigService(config$, defaultEnv, logger, new Map());
+  const configService = new ConfigService(config$, defaultEnv, logger);
 
   configService.atPath('foo', createClassWithSchema(schema.string()));
   configService.atPath(
@@ -207,12 +217,8 @@ test('correctly passes context', async () => {
       defaultValue: schema.contextRef('version'),
     }),
   });
-  const configService = new ConfigService(
-    config$,
-    env,
-    logger,
-    new Map([['foo', schemaDefinition]])
-  );
+  const configService = new ConfigService(config$, env, logger);
+  configService.preSetup(new Map([['foo', schemaDefinition]]));
 
   const configs = configService.atPath('foo', createClassWithSchema(schemaDefinition));
 
@@ -228,7 +234,7 @@ test('handles enabled path, but only marks the enabled path as used', async () =
   };
 
   const config$ = new BehaviorSubject(new ObjectToConfigAdapter(initialConfig));
-  const configService = new ConfigService(config$, defaultEnv, logger, new Map());
+  const configService = new ConfigService(config$, defaultEnv, logger);
 
   const isEnabled = await configService.isEnabledAtPath('pid');
   expect(isEnabled).toBe(true);
@@ -246,7 +252,7 @@ test('handles enabled path when path is array', async () => {
   };
 
   const config$ = new BehaviorSubject(new ObjectToConfigAdapter(initialConfig));
-  const configService = new ConfigService(config$, defaultEnv, logger, new Map());
+  const configService = new ConfigService(config$, defaultEnv, logger);
 
   const isEnabled = await configService.isEnabledAtPath(['pid']);
   expect(isEnabled).toBe(true);
@@ -264,7 +270,7 @@ test('handles disabled path and marks config as used', async () => {
   };
 
   const config$ = new BehaviorSubject(new ObjectToConfigAdapter(initialConfig));
-  const configService = new ConfigService(config$, defaultEnv, logger, new Map());
+  const configService = new ConfigService(config$, defaultEnv, logger);
 
   const isEnabled = await configService.isEnabledAtPath('pid');
   expect(isEnabled).toBe(false);
@@ -277,7 +283,7 @@ test('treats config as enabled if config path is not present in config', async (
   const initialConfig = {};
 
   const config$ = new BehaviorSubject(new ObjectToConfigAdapter(initialConfig));
-  const configService = new ConfigService(config$, defaultEnv, logger, new Map());
+  const configService = new ConfigService(config$, defaultEnv, logger);
 
   const isEnabled = await configService.isEnabledAtPath('pid');
   expect(isEnabled).toBe(true);

src/core/server/config/config_service.ts

@@ -34,17 +34,24 @@ 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 schemas?: Map<string, Type<any>>;
 
   constructor(
     private readonly config$: Observable<Config>,
     private readonly env: Env,
-    logger: LoggerFactory,
-    schemas: Map<ConfigPath, Type<any>> = new Map()
+    logger: LoggerFactory
   ) {
     this.log = logger.get('config');
+  }
+
+  public async preSetup(schemas: Map<ConfigPath, Type<any>>) {
+    this.schemas = new Map();
     for (const [path, schema] of schemas) {
-      this.schemas.set(pathToString(path), schema);
+      const namespace = pathToString(path);
+      this.schemas.set(namespace, schema);
+      await this.getValidatedConfig(namespace)
+        .pipe(first())
+        .toPromise();
     }
   }
 
@@ -125,15 +132,10 @@ export class ConfigService {
     return config.getFlattenedPaths().filter(path => isPathHandled(path, handledPaths));
   }
 
-  public async validateAll() {
-    for (const namespace of this.schemas.keys()) {
-      await this.getValidatedConfig(namespace)
-        .pipe(first())
-        .toPromise();
-    }
-  }
-
   private validateConfig(path: ConfigPath, config: Record<string, any>) {
+    if (!this.schemas) {
+      throw new Error('No validation schema has been defined');
+    }
     const namespace = pathToString(path);
     const schema = this.schemas.get(namespace);
     if (!schema) {

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

@@ -30,18 +30,21 @@ import { getEnvOptions } from '../config/__mocks__/env';
 const logger = loggingServiceMock.create();
 const env = Env.createDefault(getEnvOptions());
 
-const createConfigService = (value: Partial<HttpConfigType> = {}) =>
-  new ConfigService(
+const createConfigService = (value: Partial<HttpConfigType> = {}) => {
+  const configService = new ConfigService(
     new BehaviorSubject<Config>(
       new ObjectToConfigAdapter({
         server: value,
       })
     ),
     env,
-    logger,
-    new Map([['server', configDefinition.schema]])
+    logger
   );
 
+  configService.preSetup(new Map([['server', configDefinition.schema]]))
+  return configService;
+};
+
 afterEach(() => {
   jest.clearAllMocks();
 });

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

@@ -23,7 +23,8 @@ jest.doMock('./http/http_service', () => ({
   HttpService: jest.fn(() => httpService),
 }));
 
-export const mockPluginsService = { setup: jest.fn(), start: jest.fn(), stop: jest.fn() };
+import { pluginServiceMock } from './plugins/plugins_service.mock';
+export const mockPluginsService = pluginServiceMock.create();
 jest.doMock('./plugins/plugins_service', () => ({
   PluginsService: jest.fn(() => mockPluginsService),
 }));
@@ -38,3 +39,9 @@ export const mockLegacyService = { setup: jest.fn(), start: jest.fn(), stop: jes
 jest.mock('./legacy/legacy_service', () => ({
   LegacyService: jest.fn(() => mockLegacyService),
 }));
+
+import { configServiceMock } from './config/config_service.mock';
+export const configService = configServiceMock.create();
+jest.doMock('./config/config_service', () => ({
+  ConfigService: jest.fn(() => configService),
+}));

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

@@ -0,0 +1,20 @@
+/*
+ * 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 mockDiscover = jest.fn();
+jest.mock('./plugins_discovery', () => ({ discover: mockDiscover }));

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

@@ -0,0 +1,41 @@
+/*
+ * 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 { PluginsService } from './plugins_service';
+
+type ServiceContract = PublicMethodsOf<PluginsService>;
+const createServiceMock = () => {
+  const mocked: jest.Mocked<ServiceContract> = {
+    preSetup: jest.fn(),
+    setup: jest.fn(),
+    start: jest.fn(),
+    stop: jest.fn(),
+  };
+  mocked.preSetup.mockResolvedValue({
+    pluginDefinitions: [],
+    errors: [],
+    searchPaths: [],
+    devPluginPaths: [],
+  });
+  return mocked;
+};
+
+export const pluginServiceMock = {
+  create: createServiceMock,
+};

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

@@ -20,7 +20,4 @@
 export const mockPackage = new Proxy({ raw: {} as any }, { get: (obj, prop) => obj.raw[prop] });
 jest.mock('../../../legacy/utils/package_json', () => ({ pkg: mockPackage }));
 
-export const mockDiscover = jest.fn();
-jest.mock('./discovery/plugins_discovery', () => ({ discover: mockDiscover }));
-
 jest.mock('./plugins_system');