From adb303a53c5eabff0a41519f3b52638166decd99 Mon Sep 17 00:00:00 2001
From: Samuel Georges <sam@daftspunk.com>
Date: Sat, 21 Dec 2019 20:50:28 +1100
Subject: [PATCH] Always sort plugins by key, then dependencies

This has been benchmarked and appears to have minimal impact on performance and solves unnecessary randomness and race conditions during the app's registration and boot cycle

Fixes #4826
---
 modules/system/classes/PluginManager.php | 83 +++++++++++++++---------
 modules/system/classes/UpdateManager.php |  6 +-
 2 files changed, 56 insertions(+), 33 deletions(-)

diff --git a/modules/system/classes/PluginManager.php b/modules/system/classes/PluginManager.php
index a92ea6490..0985f2c32 100644
--- a/modules/system/classes/PluginManager.php
+++ b/modules/system/classes/PluginManager.php
@@ -11,7 +11,7 @@ use Config;
 use Schema;
 use RecursiveIteratorIterator;
 use RecursiveDirectoryIterator;
-use ApplicationException;
+use SystemException;
 
 /**
  * Plugin manager
@@ -108,6 +108,8 @@ class PluginManager
             $this->loadPlugin($namespace, $path);
         }
 
+        $this->sortDependencies();
+
         return $this->plugins;
     }
 
@@ -674,43 +676,25 @@ class PluginManager
         }
     }
 
-    /**
-     * Returns the plugin identifiers that are required by the supplied plugin.
-     * @param  string $plugin Plugin identifier, object or class
-     * @return array
-     */
-    public function getDependencies($plugin)
-    {
-        if (is_string($plugin) && (!$plugin = $this->findByIdentifier($plugin))) {
-            return false;
-        }
-
-        if (!isset($plugin->require) || !$plugin->require) {
-            return null;
-        }
-
-        return is_array($plugin->require) ? $plugin->require : [$plugin->require];
-    }
-
     /**
      * Sorts a collection of plugins, in the order that they should be actioned,
      * according to their given dependencies. Least dependent come first.
-     * @param  array $plugins Object collection to sort, or null to sort all.
      * @return array Collection of sorted plugin identifiers
      */
-    public function sortByDependencies($plugins = null)
+    protected function sortDependencies()
     {
-        if (!is_array($plugins)) {
-            $plugins = $this->getPlugins();
-        }
+        ksort($this->plugins);
 
+        /*
+         * Canvas the dependency tree
+         */
+        $checklist = $this->plugins;
         $result = [];
-        $checklist = $plugins;
 
         $loopCount = 0;
         while (count($checklist)) {
-            if (++$loopCount > 999) {
-                throw new ApplicationException('Too much recursion');
+            if (++$loopCount > 2048) {
+                throw new SystemException('Too much recursion! Check for circular dependencies in your plugins.');
             }
 
             foreach ($checklist as $code => $plugin) {
@@ -718,8 +702,8 @@ class PluginManager
                  * Get dependencies and remove any aliens
                  */
                 $depends = $this->getDependencies($plugin) ?: [];
-                $depends = array_filter($depends, function ($pluginCode) use ($plugins) {
-                    return isset($plugins[$pluginCode]);
+                $depends = array_filter($depends, function ($pluginCode) {
+                    return isset($this->plugins[$pluginCode]);
                 });
 
                 /*
@@ -746,7 +730,46 @@ class PluginManager
                 unset($checklist[$code]);
             }
         }
-        return $result;
+
+        /*
+         * Reassemble plugin map
+         */
+        $sortedPlugins = [];
+
+        foreach ($result as $code) {
+            $sortedPlugins[$code] = $this->plugins[$code];
+        }
+
+        return $this->plugins = $sortedPlugins;
+    }
+
+    /**
+     * Returns the plugin identifiers that are required by the supplied plugin.
+     * @param  string $plugin Plugin identifier, object or class
+     * @return array
+     */
+    public function getDependencies($plugin)
+    {
+        if (is_string($plugin) && (!$plugin = $this->findByIdentifier($plugin))) {
+            return false;
+        }
+
+        if (!isset($plugin->require) || !$plugin->require) {
+            return null;
+        }
+
+        return is_array($plugin->require) ? $plugin->require : [$plugin->require];
+    }
+
+    /**
+     * @deprecated Plugins are now sorted by default. See getPlugins()
+     * Remove if year >= 2022
+     */
+    public function sortByDependencies($plugins = null)
+    {
+        traceLog('PluginManager::sortByDependencies is deprecated. Plugins are now sorted by default. Use PluginManager::getPlugins()');
+
+        return array_keys($plugins ?: $this->getPlugins());
     }
 
     //
diff --git a/modules/system/classes/UpdateManager.php b/modules/system/classes/UpdateManager.php
index 0f8264199..53ba96468 100644
--- a/modules/system/classes/UpdateManager.php
+++ b/modules/system/classes/UpdateManager.php
@@ -149,9 +149,9 @@ class UpdateManager
         /*
          * Update plugins
          */
-        $plugins = $this->pluginManager->sortByDependencies();
-        foreach ($plugins as $plugin) {
-            $this->updatePlugin($plugin);
+        $plugins = $this->pluginManager->getPlugins();
+        foreach ($plugins as $code => $plugin) {
+            $this->updatePlugin($code);
         }
 
         Parameter::set('system::update.count', 0);