From 3e81c7f8bc9d6fd2a1a560ba9764d2969b997c34 Mon Sep 17 00:00:00 2001
From: Nick Partridge <nick.ryan.partridge@gmail.com>
Date: Thu, 2 Jul 2020 11:14:43 -0500
Subject: [PATCH] Add more detailed unmet deps logging (#70098) (#70599)

---
 .../server/plugins/plugins_service.test.ts    | 34 ++++++++++++--
 src/core/server/plugins/plugins_service.ts    | 45 ++++++++++++++-----
 2 files changed, 63 insertions(+), 16 deletions(-)

diff --git a/src/core/server/plugins/plugins_service.test.ts b/src/core/server/plugins/plugins_service.test.ts
index c277dc85e5e04..46fd2b00c2304 100644
--- 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,6 +227,26 @@ 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'],
+          }),
         ]),
       });
 
@@ -234,7 +254,7 @@ describe('PluginsService', () => {
       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);
 
@@ -244,14 +264,20 @@ describe('PluginsService', () => {
             "Plugin \\"explicitly-disabled-plugin\\" is disabled.",
           ],
           Array [
-            "Plugin \\"plugin-with-missing-required-deps\\" has been disabled since some of its direct or transitive dependencies are missing or disabled.",
+            "Plugin \\"plugin-with-missing-required-deps\\" has been disabled since the following direct or transitive dependencies are missing or disabled: [missing-plugin]",
           ],
           Array [
-            "Plugin \\"plugin-with-disabled-transitive-dep\\" has been disabled since some of its direct or transitive dependencies are missing or disabled.",
+            "Plugin \\"plugin-with-disabled-transitive-dep\\" has been disabled since the following direct or transitive dependencies are missing or disabled: [another-explicitly-disabled-plugin]",
           ],
           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
index 7441e753efa6a..5d1261e697bc0 100644
--- a/src/core/server/plugins/plugins_service.ts
+++ b/src/core/server/plugins/plugins_service.ts
@@ -239,11 +239,15 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
       .toPromise();
 
     for (const [pluginName, { plugin, isEnabled }] of pluginEnableStatuses) {
-      if (this.shouldEnablePlugin(pluginName, pluginEnableStatuses)) {
+      const pluginEnablement = this.shouldEnablePlugin(pluginName, pluginEnableStatuses);
+
+      if (pluginEnablement.enabled) {
         this.pluginsSystem.addPlugin(plugin);
       } else if (isEnabled) {
         this.log.info(
-          `Plugin "${pluginName}" has been disabled since some of its direct or transitive dependencies are missing or disabled.`
+          `Plugin "${pluginName}" has been disabled since the following direct or transitive dependencies are missing or disabled: [${pluginEnablement.missingDependencies.join(
+            ', '
+          )}]`
         );
       } else {
         this.log.info(`Plugin "${pluginName}" is disabled.`);
@@ -257,17 +261,34 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
     pluginName: PluginName,
     pluginEnableStatuses: Map<PluginName, { plugin: PluginWrapper; isEnabled: boolean }>,
     parents: PluginName[] = []
-  ): boolean {
+  ): { enabled: true } | { enabled: false; missingDependencies: string[] } {
     const pluginInfo = pluginEnableStatuses.get(pluginName);
-    return (
-      pluginInfo !== undefined &&
-      pluginInfo.isEnabled &&
-      pluginInfo.plugin.requiredPlugins
-        .filter((dep) => !parents.includes(dep))
-        .every((dependencyName) =>
-          this.shouldEnablePlugin(dependencyName, pluginEnableStatuses, [...parents, pluginName])
-        )
-    );
+
+    if (pluginInfo === undefined || !pluginInfo.isEnabled) {
+      return {
+        enabled: false,
+        missingDependencies: [],
+      };
+    }
+
+    const missingDependencies = pluginInfo.plugin.requiredPlugins
+      .filter((dep) => !parents.includes(dep))
+      .filter(
+        (dependencyName) =>
+          !this.shouldEnablePlugin(dependencyName, pluginEnableStatuses, [...parents, pluginName])
+            .enabled
+      );
+
+    if (missingDependencies.length === 0) {
+      return {
+        enabled: true,
+      };
+    }
+
+    return {
+      enabled: false,
+      missingDependencies,
+    };
   }
 
   private registerPluginStaticDirs(deps: PluginsServiceSetupDeps) {