fix: dont ignore optional dependencies on disabled extensions.

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.
flarum · Mar 25, 2022 · eebc730 · eebc730
1 parent b64003c
commit eebc730
Show file tree

Hide file tree

Showing 3 changed files with 3 additions and 8 deletions.
diff --git a/framework/core/src/Extension/Extension.php b/framework/core/src/Extension/Extension.php
@@ -219,7 +219,7 @@ public function setVersion($version)
      *
      * @internal
      */
-    public function calculateDependencies($extensionSet, $enabledIds)
+    public function calculateDependencies($extensionSet)
     {
         $this->extensionDependencyIds = (new Collection(Arr::get($this->composerJson, 'require', [])))
             ->keys()
@@ -235,9 +235,6 @@ public function calculateDependencies($extensionSet, $enabledIds)
             ->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
@@ -87,9 +87,7 @@ public function getExtensions()
             // 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 @@ public function getExtensions()
             }
 
             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
@@ -26,7 +26,7 @@ public function setUp(): void
         $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 */