diff --git a/core/Application/Kernel/PluginList.php b/core/Application/Kernel/PluginList.php
index 5d263a6ea3ac21507e6ebb09a9a236511169bfd9..70abc543d53863b41bf4ba24fbbd26cd6eaaa5c5 100644
--- a/core/Application/Kernel/PluginList.php
+++ b/core/Application/Kernel/PluginList.php
@@ -8,6 +8,8 @@
namespace Piwik\Application\Kernel;
+use Piwik\Plugin\MetadataLoader;
+
/**
* Lists the currently activated plugins. Used when setting up Piwik's environment before
* initializing the DI container.
@@ -88,7 +90,8 @@ class PluginList
}
/**
- * Sorts an array of plugins in the order they should be loaded.
+ * Sorts an array of plugins in the order they should be loaded. We cannot use DI here as DI is not initialized
+ * at this stage.
*
* @params string[] $plugins
* @return \string[]
@@ -111,10 +114,78 @@ class PluginList
$otherPluginsToLoadAfterDefaultPlugins = array_diff($plugins, $defaultPluginsLoadedFirst);
// sort by name to have a predictable order for those extra plugins
- sort($otherPluginsToLoadAfterDefaultPlugins);
+ natcasesort($otherPluginsToLoadAfterDefaultPlugins);
$sorted = array_merge($defaultPluginsLoadedFirst, $otherPluginsToLoadAfterDefaultPlugins);
return $sorted;
}
+
+ /**
+ * Sorts an array of plugins in the order they should be saved in config.ini.php. This basically influences
+ * the order of the plugin config.php and which config will be loaded first. We want to make sure to require the
+ * config or a required plugin first before loading the plugin that requires it.
+ *
+ * We do not sort using this logic on each request since it is much slower than `sortPlugins()`. The order
+ * of plugins in config.ini.php is only important for the ContainerFactory. During a regular request it is otherwise
+ * fine to load the plugins in the order of `sortPlugins()` since we will make sure that required plugins will be
+ * loaded first in plugin manager.
+ *
+ * @param string[] $plugins
+ * @param array[] $pluginJsonCache For internal testing only
+ * @return \string[]
+ */
+ public function sortPluginsAndRespectDependencies(array $plugins, $pluginJsonCache = array())
+ {
+ $global = $this->getPluginsBundledWithPiwik();
+
+ if (empty($global)) {
+ return $plugins;
+ }
+
+ // we need to make sure a possibly disabled plugin will be still loaded before any 3rd party plugin
+ $global = array_merge($global, $this->corePluginsDisabledByDefault);
+
+ $global = array_values($global);
+ $plugins = array_values($plugins);
+
+ $defaultPluginsLoadedFirst = array_intersect($global, $plugins);
+
+ $otherPluginsToLoadAfterDefaultPlugins = array_diff($plugins, $defaultPluginsLoadedFirst);
+
+ // we still want to sort alphabetically by default
+ natcasesort($otherPluginsToLoadAfterDefaultPlugins);
+
+ $sorted = array();
+ foreach ($otherPluginsToLoadAfterDefaultPlugins as $pluginName) {
+ $sorted = $this->sortRequiredPlugin($pluginName, $pluginJsonCache, $otherPluginsToLoadAfterDefaultPlugins, $sorted);
+ }
+
+ $sorted = array_merge($defaultPluginsLoadedFirst, $sorted);
+
+ return $sorted;
+ }
+
+ private function sortRequiredPlugin($pluginName, &$pluginJsonCache, $toBeSorted, $sorted)
+ {
+ if (!isset($pluginJsonCache[$pluginName])) {
+ $loader = new MetadataLoader($pluginName);
+ $pluginJsonCache[$pluginName] = $loader->loadPluginInfoJson();
+ }
+
+ if (!empty($pluginJsonCache[$pluginName]['require'])) {
+ $dependencies = $pluginJsonCache[$pluginName]['require'];
+ foreach ($dependencies as $possiblePluginName => $key) {
+ if (in_array($possiblePluginName, $toBeSorted, true) && !in_array($possiblePluginName, $sorted, true)) {
+ $sorted = $this->sortRequiredPlugin($possiblePluginName, $pluginJsonCache, $toBeSorted, $sorted);
+ }
+ }
+ }
+
+ if (!in_array($pluginName, $sorted, true)) {
+ $sorted[] = $pluginName;
+ }
+
+ return $sorted;
+ }
}
diff --git a/core/Plugin/Manager.php b/core/Plugin/Manager.php
index 9466304e27b6ae8f123e490e50cf83e94a9b6d18..ee4f658b880689bb6c2ee14a1ab688273ab9dde3 100644
--- a/core/Plugin/Manager.php
+++ b/core/Plugin/Manager.php
@@ -203,7 +203,7 @@ class Manager
*/
private function updatePluginsConfig($pluginsToLoad)
{
- $pluginsToLoad = $this->pluginList->sortPlugins($pluginsToLoad);
+ $pluginsToLoad = $this->pluginList->sortPluginsAndRespectDependencies($pluginsToLoad);
$section = PiwikConfig::getInstance()->Plugins;
$section['Plugins'] = $pluginsToLoad;
PiwikConfig::getInstance()->Plugins = $section;
diff --git a/core/Plugin/MetadataLoader.php b/core/Plugin/MetadataLoader.php
index bde849ff7eeb2387cc77596e56741d11044fd433..b3d552cf80504bab18363557be4801587cfecc46 100644
--- a/core/Plugin/MetadataLoader.php
+++ b/core/Plugin/MetadataLoader.php
@@ -11,6 +11,7 @@ namespace Piwik\Plugin;
use Exception;
use Piwik\Piwik;
use Piwik\Version;
+use Piwik\Plugin;
/**
* @see core/Version.php
@@ -90,12 +91,22 @@ class MetadataLoader
);
}
- private function loadPluginInfoJson()
+ /**
+ * It is important that this method works without using anything from DI
+ * @return array|mixed
+ */
+ public function loadPluginInfoJson()
{
- $path = $this->getPathToPluginFolder() . '/' . self::PLUGIN_JSON_FILENAME;
+ $path = $this->getPathToPluginJson();
return $this->loadJsonMetadata($path);
}
+ public function getPathToPluginJson()
+ {
+ $path = $this->getPathToPluginFolder() . '/' . self::PLUGIN_JSON_FILENAME;
+ return $path;
+ }
+
private function loadJsonMetadata($path)
{
if (!file_exists($path)) {
diff --git a/tests/PHPUnit/Framework/TestingEnvironmentManipulator.php b/tests/PHPUnit/Framework/TestingEnvironmentManipulator.php
index 1aa1a74de89b2b9a8f9965f43a8e8a6d1d9a9aab..59bac601554df09ef0670bfc3cc24f1e2e7eeb3e 100644
--- a/tests/PHPUnit/Framework/TestingEnvironmentManipulator.php
+++ b/tests/PHPUnit/Framework/TestingEnvironmentManipulator.php
@@ -227,16 +227,15 @@ class TestingEnvironmentManipulator implements EnvironmentManipulator
private function getPluginAndRequiredPlugins($pluginName, $plugins)
{
- $pluginJsonPath = $this->makePathToPluginJson($pluginName);
+ $pluginLoader = new Plugin\MetadataLoader($pluginName);
+ $pluginJson = $pluginLoader->loadPluginInfoJson();
- if (file_exists($pluginJsonPath)) {
- $pluginJson = json_decode(trim(file_get_contents($pluginJsonPath)), true);
+ if (!empty($pluginJson['require'])) {
+ foreach ($pluginJson['require'] as $possiblePluginName => $requiredVersion) {
- if (!empty($pluginJson['require'])) {
- foreach ($pluginJson['require'] as $possiblePluginName => $requiredVersion) {
- if (file_exists($this->makePathToPluginJson($possiblePluginName))) {
- $plugins = $this->getPluginAndRequiredPlugins($possiblePluginName, $plugins);
- }
+ $pluginLoader2 = new Plugin\MetadataLoader($possiblePluginName);
+ if (file_exists($pluginLoader2->getPathToPluginJson())) {
+ $plugins = $this->getPluginAndRequiredPlugins($possiblePluginName, $plugins);
}
}
}
@@ -248,11 +247,6 @@ class TestingEnvironmentManipulator implements EnvironmentManipulator
return $plugins;
}
- private function makePathToPluginJson($pluginName)
- {
- return Plugin\Manager::getPluginsDirectory() . $pluginName . '/' . Plugin\MetadataLoader::PLUGIN_JSON_FILENAME;
- }
-
private function classExists($klass)
{
if (class_exists($klass)) {
diff --git a/tests/PHPUnit/Integration/Application/Kernel/PluginListTest.php b/tests/PHPUnit/Integration/Application/Kernel/PluginListTest.php
new file mode 100644
index 0000000000000000000000000000000000000000..00f873b2eeeaa3e6d1b0c18782923967054c0f8e
--- /dev/null
+++ b/tests/PHPUnit/Integration/Application/Kernel/PluginListTest.php
@@ -0,0 +1,113 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+namespace Piwik\Tests\Integration\Application\Kernel;
+
+use Piwik\Application\Kernel\PluginList;
+use Piwik\Container\StaticContainer;
+use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
+
+/**
+ * @group PluginListTest
+ * @group Core
+ */
+class PluginListTest extends IntegrationTestCase
+{
+
+ /**
+ * @var PluginList
+ */
+ private $pluginList = array();
+
+ public function setUp()
+ {
+ parent::setUp();
+ $this->pluginList = $this->makePluginList();
+ }
+
+ public function test_sortPlugins()
+ {
+ $pluginList = $this->makePluginList();
+ $sorted = $pluginList->sortPlugins(array('UsersManager', 'CoreHome', 'MyCustomPlugin', 'ExampleCommand', 'MyCustomPlugin2', 'Abcdef'));
+ $this->assertSame(array(
+ 'CoreHome', // core plugins loaded first
+ 'UsersManager',
+ 'ExampleCommand', // a "by default disabled plugin" is loaded before custom plugins
+ 'Abcdef', // then we load custom plugins
+ 'MyCustomPlugin',
+ 'MyCustomPlugin2',
+ ), $sorted);
+ }
+
+ public function test_sortPlugins_onlyCorePlugins()
+ {
+ $pluginList = $this->makePluginList();
+ $sorted = $pluginList->sortPlugins(array('UsersManager', 'CoreHome'));
+ $this->assertSame(array('CoreHome','UsersManager'), $sorted);
+ }
+
+ public function test_sortPluginsAndRespectDependencies_sortsPluginsAlphabetically()
+ {
+ $pluginList = $this->makePluginList();
+ $sorted = $pluginList->sortPluginsAndRespectDependencies(array(
+ 'UsersManager', 'MyCustomPlugin', 'ExampleCommand', 'MyCustomPlugin2', 'CoreHome', 'Abcdef'
+ ));
+ $this->assertSame(array(
+ 'CoreHome', // core plugins loaded first
+ 'UsersManager',
+ 'ExampleCommand', // a "by default disabled plugin" is loaded before custom plugins
+ 'Abcdef', // then we load custom plugins
+ 'MyCustomPlugin',
+ 'MyCustomPlugin2',
+ ), $sorted);
+ }
+
+ public function test_sortPluginsAndRespectDependencies_makesSureToListRequiredDependencyFirst()
+ {
+ $pluginJsonInfo = array(
+ 'Abcdef' => array('require' => array('MyCustomPlugin2' => '2.2.1')),
+ 'MyCustomPlugin2' => array('require' => array('CoreHome' => '4.2.1', 'MyCustomPlugin3' => '3.0.3')),
+ 'fooBar' => array('require' => array('Ast' => '1.2.1', 'MyCustomPlugin3' => '3.0.3'))
+ );
+
+ $pluginList = $this->makePluginList();
+ $sorted = $pluginList->sortPluginsAndRespectDependencies(array(
+ 'UsersManager', 'MyCustomPlugin',
+ 'ExampleCommand', 'MyCustomPlugin2', 'Ast',
+ 'Acc', 'MyCustomPlugin3', 'CoreHome', 'Abcdef', 'fooBar',
+ ), $pluginJsonInfo);
+ $this->assertSame(array(
+ 'CoreHome', // core plugins loaded first
+ 'UsersManager',
+ 'ExampleCommand', // a "by default disabled plugin" is loaded before custom plugins
+ 'MyCustomPlugin3',
+ 'MyCustomPlugin2',
+ 'Abcdef',
+ 'Acc',
+ 'Ast',
+ 'fooBar',
+ 'MyCustomPlugin',
+ ), $sorted);
+ }
+
+ public function test_sortPluginsAndRespectDependencies_onlyCorePlugins()
+ {
+ $pluginList = $this->makePluginList();
+ $sorted = $pluginList->sortPluginsAndRespectDependencies(array('UsersManager', 'CoreHome'));
+ $this->assertSame(array('CoreHome','UsersManager'), $sorted);
+ }
+
+ private function makePluginList()
+ {
+ $globalSettingsProvider = StaticContainer::get('Piwik\Application\Kernel\GlobalSettingsProvider');
+ $section = $globalSettingsProvider->getSection('Plugins');
+ // $section['Plugins'] = $pluginsToLoad;
+ $globalSettingsProvider->setSection('Plugins', $section);
+ return new PluginList($globalSettingsProvider);
+ }
+
+}