From 431ba304348f0f923c65183e5f8e3c03ac3da9b9 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov <38059171+askvortsov1@users.noreply.github.com> Date: Wed, 6 Apr 2022 11:48:52 -0400 Subject: [PATCH] fix: dont ignore optional dependencies on disabled extensions. (#3352) There is a check in the ExtensionManager::resolveExtensionOrder function that ignores optional dependencies on extensions that don't exist in the system. This is sufficient for resolution purposes. The filter removed in this PR would ignore optional dependencies on non-enabled extensions, so when such an extension was enabled, dependency resolution would run incorrectly. --- framework/core/src/Extension/Extension.php | 5 +---- framework/core/src/Extension/ExtensionManager.php | 4 +--- .../unit/Foundation/ExtensionDependencyResolutionTest.php | 2 +- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/framework/core/src/Extension/Extension.php b/framework/core/src/Extension/Extension.php index 6176f0742..d841a2f3e 100644 --- a/framework/core/src/Extension/Extension.php +++ b/framework/core/src/Extension/Extension.php @@ -219,7 +219,7 @@ class Extension implements Arrayable * * @internal */ - public function calculateDependencies($extensionSet, $enabledIds) + public function calculateDependencies($extensionSet) { $this->extensionDependencyIds = (new Collection(Arr::get($this->composerJson, 'require', []))) ->keys() @@ -235,9 +235,6 @@ class Extension implements Arrayable ->map(function ($key) { return static::nameToId($key); }) - ->filter(function ($key) use ($enabledIds) { - return array_key_exists($key, $enabledIds); - }) ->toArray(); } diff --git a/framework/core/src/Extension/ExtensionManager.php b/framework/core/src/Extension/ExtensionManager.php index d33e4808d..e69d174a6 100644 --- a/framework/core/src/Extension/ExtensionManager.php +++ b/framework/core/src/Extension/ExtensionManager.php @@ -87,9 +87,7 @@ class ExtensionManager // We calculate and store a set of composer package names for all installed Flarum extensions, // so we know what is and isn't a flarum extension in `calculateDependencies`. // Using keys of an associative array allows us to do these checks in constant time. - // We do the same for enabled extensions, for optional dependencies. $installedSet = []; - $enabledIds = array_flip($this->getEnabled()); foreach ($installed as $package) { if (Arr::get($package, 'type') != 'flarum-extension' || empty(Arr::get($package, 'name'))) { @@ -113,7 +111,7 @@ class ExtensionManager } foreach ($extensions as $extension) { - $extension->calculateDependencies($installedSet, $enabledIds); + $extension->calculateDependencies($installedSet); } $needsReset = false; diff --git a/framework/core/tests/unit/Foundation/ExtensionDependencyResolutionTest.php b/framework/core/tests/unit/Foundation/ExtensionDependencyResolutionTest.php index 832773978..90c0e924e 100644 --- a/framework/core/tests/unit/Foundation/ExtensionDependencyResolutionTest.php +++ b/framework/core/tests/unit/Foundation/ExtensionDependencyResolutionTest.php @@ -26,7 +26,7 @@ class ExtensionDependencyResolutionTest extends TestCase $this->missing = new FakeExtension('flarum-missing', ['this-does-not-exist', 'flarum-tags', 'also-not-exists']); $this->circular1 = new FakeExtension('circular1', ['circular2']); $this->circular2 = new FakeExtension('circular2', ['circular1']); - $this->optionalDependencyCategories = new FakeExtension('flarum-categories', ['flarum-tags'], ['flarum-tag-backgrounds']); + $this->optionalDependencyCategories = new FakeExtension('flarum-categories', ['flarum-tags'], ['flarum-tag-backgrounds', 'non-existent-optional-dependency']); } /** @test */