update return signature, add test more cases

elastic · Jun 26, 2020 · 5d1b4e5 · 5d1b4e5
1 parent cf15a28
commit 5d1b4e5
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 10 deletions.
diff --git a/src/core/server/plugins/plugins_service.test.ts b/src/core/server/plugins/plugins_service.test.ts
@@ -51,7 +51,7 @@ const logger = loggingSystemMock.create();
 
 expect.addSnapshotSerializer(createAbsolutePathSerializer());
 
-['path-1', 'path-2', 'path-3', 'path-4', 'path-5'].forEach((path) => {
+['path-1', 'path-2', 'path-3', 'path-4', 'path-5', 'path-6', 'path-7', 'path-8'].forEach((path) => {
   jest.doMock(join(path, 'server'), () => ({}), {
     virtual: true,
   });
@@ -227,14 +227,34 @@ describe('PluginsService', () => {
             path: 'path-4',
             configPath: 'path-4-disabled',
           }),
+          createPlugin('plugin-with-disabled-optional-dep', {
+            path: 'path-5',
+            configPath: 'path-5',
+            optionalPlugins: ['explicitly-disabled-plugin'],
+          }),
+          createPlugin('plugin-with-missing-optional-dep', {
+            path: 'path-6',
+            configPath: 'path-6',
+            optionalPlugins: ['missing-plugin'],
+          }),
+          createPlugin('plugin-with-disabled-nested-transitive-dep', {
+            path: 'path-7',
+            configPath: 'path-7',
+            requiredPlugins: ['plugin-with-disabled-transitive-dep'],
+          }),
+          createPlugin('plugin-with-missing-nested-dep', {
+            path: 'path-8',
+            configPath: 'path-8',
+            requiredPlugins: ['plugin-with-missing-required-deps'],
+          }),
         ]),
       });
 
       await pluginsService.discover();
       const setup = await pluginsService.setup(setupDeps);
 
       expect(setup.contracts).toBeInstanceOf(Map);
-      expect(mockPluginSystem.addPlugin).not.toHaveBeenCalled();
+      expect(mockPluginSystem.addPlugin).toHaveBeenCalledTimes(2);
       expect(mockPluginSystem.setupPlugins).toHaveBeenCalledTimes(1);
       expect(mockPluginSystem.setupPlugins).toHaveBeenCalledWith(setupDeps);
 
@@ -252,6 +272,12 @@ describe('PluginsService', () => {
           Array [
             "Plugin \\"another-explicitly-disabled-plugin\\" is disabled.",
           ],
+          Array [
+            "Plugin \\"plugin-with-disabled-nested-transitive-dep\\" has been disabled since the following direct or transitive dependencies are missing or disabled: [\\"plugin-with-disabled-transitive-dep\\"]",
+          ],
+          Array [
+            "Plugin \\"plugin-with-missing-nested-dep\\" has been disabled since the following direct or transitive dependencies are missing or disabled: [\\"plugin-with-missing-required-deps\\"]",
+          ],
         ]
       `);
     });

diff --git a/src/core/server/plugins/plugins_service.ts b/src/core/server/plugins/plugins_service.ts
@@ -239,17 +239,14 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
       .toPromise();
 
     for (const [pluginName, { plugin, isEnabled }] of pluginEnableStatuses) {
-      const { enabled: shouldEnablePlugin, missingDependencies } = this.shouldEnablePlugin(
-        pluginName,
-        pluginEnableStatuses
-      );
+      const pluginEnablement = this.shouldEnablePlugin(pluginName, pluginEnableStatuses);
 
-      if (shouldEnablePlugin) {
+      if (pluginEnablement.enabled) {
         this.pluginsSystem.addPlugin(plugin);
       } else if (isEnabled) {
         this.log.info(
           `Plugin "${pluginName}" has been disabled since the following direct or transitive dependencies are missing or disabled: ${JSON.stringify(
-            missingDependencies
+            pluginEnablement.missingDependencies
           )}`
         );
       } else {
@@ -264,7 +261,7 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
     pluginName: PluginName,
     pluginEnableStatuses: Map<PluginName, { plugin: PluginWrapper; isEnabled: boolean }>,
     parents: PluginName[] = []
-  ): { enabled: boolean; missingDependencies: string[] } {
+  ): { enabled: true } | { enabled: false; missingDependencies: string[] } {
     const pluginInfo = pluginEnableStatuses.get(pluginName);
 
     if (pluginInfo === undefined || !pluginInfo.isEnabled) {
@@ -282,8 +279,14 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
             .enabled
       );
 
+    if (missingDependencies.length === 0) {
+      return {
+        enabled: true,
+      };
+    }
+
     return {
-      enabled: missingDependencies.length === 0,
+      enabled: false,
       missingDependencies,
     };
   }