move plugin schema registration in plugins plugins service

plugins system is not setup when kibana is run in optimizer mode, so config keys aren't marked as applied.
elastic · May 9, 2019 · 2a01ed7 · 2a01ed7
1 parent 12d9947
commit 2a01ed7
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 22 deletions.
diff --git a/src/core/server/plugins/plugins_service.test.ts b/src/core/server/plugins/plugins_service.test.ts
@@ -19,8 +19,9 @@
 
 import { mockDiscover, mockPackage } from './plugins_service.test.mocks';
 
-import { resolve } from 'path';
+import { resolve, join } from 'path';
 import { BehaviorSubject, from } from 'rxjs';
+import { schema } from '@kbn/config-schema';
 
 import { Config, ConfigService, Env, ObjectToConfigAdapter } from '../config';
 import { getEnvOptions } from '../config/__mocks__/env';
@@ -44,6 +45,13 @@ const setupDeps = {
   http: httpServiceMock.createSetupContract(),
 };
 const logger = loggingServiceMock.create();
+
+['path-1', 'path-2', 'path-3', 'path-4', 'path-5'].forEach(path => {
+  jest.doMock(join(path, 'server'), () => ({}), {
+    virtual: true,
+  });
+});
+
 beforeEach(() => {
   mockPackage.raw = {
     branch: 'feature-v1',
@@ -328,3 +336,40 @@ test('`stop` stops plugins system', async () => {
   await pluginsService.stop();
   expect(mockPluginSystem.stopPlugins).toHaveBeenCalledTimes(1);
 });
+
+test('`setup` registers plugin config schema in config service', async () => {
+  const configSchema = schema.string();
+  jest.spyOn(configService, 'setSchema').mockImplementation(() => Promise.resolve());
+  jest.doMock(
+    join('path-with-schema', 'server'),
+    () => ({
+      config: {
+        schema: configSchema,
+      },
+    }),
+    {
+      virtual: true,
+    }
+  );
+  mockDiscover.mockReturnValue({
+    error$: from([]),
+    plugin$: from([
+      new PluginWrapper(
+        'path-with-schema',
+        {
+          id: 'some-id',
+          version: 'some-version',
+          configPath: 'path',
+          kibanaVersion: '7.0.0',
+          requiredPlugins: [],
+          optionalPlugins: [],
+          server: true,
+          ui: true,
+        },
+        { logger } as any
+      ),
+    ]),
+  });
+  await pluginsService.setup(setupDeps);
+  expect(configService.setSchema).toBeCalledWith('path', configSchema);
+});
diff --git a/src/core/server/plugins/plugins_service.ts b/src/core/server/plugins/plugins_service.ts
@@ -130,6 +130,10 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
     await plugin$
       .pipe(
         mergeMap(async plugin => {
+          const schema = plugin.getConfigSchema();
+          if (schema) {
+            await this.coreContext.configService.setSchema(plugin.configPath, schema);
+          }
           const isEnabled = await this.coreContext.configService.isEnabledAtPath(plugin.configPath);
 
           if (pluginEnableStatuses.has(plugin.name)) {

diff --git a/src/core/server/plugins/plugins_system.test.ts b/src/core/server/plugins/plugins_system.test.ts
@@ -23,7 +23,6 @@ import {
 } from './plugins_system.test.mocks';
 
 import { BehaviorSubject } from 'rxjs';
-import { schema } from '@kbn/config-schema';
 
 import { Env } from '../config';
 import { getEnvOptions } from '../config/__mocks__/env';
@@ -119,7 +118,6 @@ test('`setupPlugins` throws if plugins have circular optional dependency', async
 test('`setupPlugins` ignores missing optional dependency', async () => {
   const plugin = createPlugin('some-id', { optional: ['missing-dep'] });
   jest.spyOn(plugin, 'setup').mockResolvedValue('test');
-  jest.spyOn(plugin, 'getConfigSchema').mockReturnValue(null);
 
   pluginsSystem.addPlugin(plugin);
 
@@ -182,7 +180,6 @@ test('correctly orders plugins and returns exposed values for "setup" and "start
   [...plugins.keys()].forEach((plugin, index) => {
     jest.spyOn(plugin, 'setup').mockResolvedValue(`added-as-${index}`);
     jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`);
-    jest.spyOn(plugin, 'getConfigSchema').mockReturnValue(null);
 
     setupContextMap.set(plugin.name, `setup-for-${plugin.name}`);
     startContextMap.set(plugin.name, `start-for-${plugin.name}`);
@@ -269,7 +266,6 @@ test('`setupPlugins` only setups plugins that have server side', async () => {
 
   [firstPluginToRun, secondPluginNotToRun, thirdPluginToRun].forEach((plugin, index) => {
     jest.spyOn(plugin, 'setup').mockResolvedValue(`added-as-${index}`);
-    jest.spyOn(plugin, 'getConfigSchema').mockReturnValue(null);
 
     pluginsSystem.addPlugin(plugin);
   });
@@ -359,7 +355,6 @@ test('`startPlugins` only starts plugins that were setup', async () => {
   [firstPluginToRun, secondPluginNotToRun, thirdPluginToRun].forEach((plugin, index) => {
     jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`);
     jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`);
-    jest.spyOn(plugin, 'getConfigSchema').mockReturnValue(null);
 
     pluginsSystem.addPlugin(plugin);
   });
@@ -378,13 +373,3 @@ Array [
 ]
 `);
 });
-
-test('`setup` calls configService.setSchema if plugin specifies config schema', async () => {
-  const pluginSchema = schema.string();
-  const plugin = createPlugin('plugin-1');
-  jest.spyOn(plugin, 'setup').mockResolvedValue('test');
-  jest.spyOn(plugin, 'getConfigSchema').mockReturnValue(pluginSchema);
-  pluginsSystem.addPlugin(plugin);
-  await pluginsSystem.setupPlugins(setupDeps);
-  await expect(configService.setSchema).toBeCalledWith('path', pluginSchema);
-});
diff --git a/src/core/server/plugins/plugins_system.ts b/src/core/server/plugins/plugins_system.ts
@@ -70,12 +70,6 @@ export class PluginsSystem {
         {} as Record<PluginName, unknown>
       );
 
-      const schema = plugin.getConfigSchema();
-
-      if (schema) {
-        await this.coreContext.configService.setSchema(plugin.configPath, schema);
-      }
-
       contracts.set(
         pluginName,
         await plugin.setup(