From eb717bb034971cee4e1fdafc2ec6175252a224d4 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov <38059171+askvortsov1@users.noreply.github.com> Date: Fri, 2 Oct 2020 17:54:28 -0400 Subject: [PATCH] Basic Extension Dependency Support (#2188) - Don't enable an extension if its dependencies are not enabled - Don't disable an extension if its dependencies are not disabled --- .../js/src/admin/components/ExtensionsPage.js | 20 ++++++ .../DependentExtensionsException.php | 47 ++++++++++++++ .../DependentExtensionsExceptionHandler.php | 34 ++++++++++ .../MissingDependenciesException.php | 47 ++++++++++++++ .../MissingDependenciesExceptionHandler.php | 34 ++++++++++ framework/core/src/Extension/Extension.php | 62 ++++++++++++++++--- .../core/src/Extension/ExtensionManager.php | 42 +++++++++++-- .../src/Foundation/ErrorServiceProvider.php | 6 ++ 8 files changed, 279 insertions(+), 13 deletions(-) create mode 100644 framework/core/src/Extension/Exception/DependentExtensionsException.php create mode 100644 framework/core/src/Extension/Exception/DependentExtensionsExceptionHandler.php create mode 100644 framework/core/src/Extension/Exception/MissingDependenciesException.php create mode 100644 framework/core/src/Extension/Exception/MissingDependenciesExceptionHandler.php diff --git a/framework/core/js/src/admin/components/ExtensionsPage.js b/framework/core/js/src/admin/components/ExtensionsPage.js index 5a50fea6e..0ef4e9f9c 100644 --- a/framework/core/js/src/admin/components/ExtensionsPage.js +++ b/framework/core/js/src/admin/components/ExtensionsPage.js @@ -123,6 +123,7 @@ export default class ExtensionsPage extends Page { url: app.forum.attribute('apiUrl') + '/extensions/' + id, method: 'PATCH', body: { enabled: !enabled }, + errorHandler: this.onerror.bind(this), }) .then(() => { if (!enabled) localStorage.setItem('enabledExtension', id); @@ -131,4 +132,23 @@ export default class ExtensionsPage extends Page { app.modal.show(LoadingModal); } + + onerror(e) { + // We need to give the modal animation time to start; if we close the modal too early, + // it breaks the bootstrap modal library. + // TODO: This workaround should be removed when we move away from bootstrap JS for modals. + setTimeout(() => { + app.modal.close(); + + const error = JSON.parse(e.responseText).errors[0]; + + app.alerts.show( + { type: 'error' }, + app.translator.trans(`core.lib.error.${error.code}_message`, { + extension: error.extension, + extensions: error.extensions.join(', '), + }) + ); + }, 250); + } } diff --git a/framework/core/src/Extension/Exception/DependentExtensionsException.php b/framework/core/src/Extension/Exception/DependentExtensionsException.php new file mode 100644 index 000000000..ab2634706 --- /dev/null +++ b/framework/core/src/Extension/Exception/DependentExtensionsException.php @@ -0,0 +1,47 @@ +extension = $extension; + $this->dependent_extensions = $dependent_extensions; + + parent::__construct($extension->getId().' could not be disabled, because it is a dependency of: '.implode(', ', $this->getDependentExtensionIds())); + } + + /** + * Get array of IDs for extensions that depend on this extension. + * + * @return array + */ + public function getDependentExtensionIds() + { + return array_map(function (Extension $extension) { + return $extension->getId(); + }, $this->dependent_extensions); + } +} diff --git a/framework/core/src/Extension/Exception/DependentExtensionsExceptionHandler.php b/framework/core/src/Extension/Exception/DependentExtensionsExceptionHandler.php new file mode 100644 index 000000000..a8ad87727 --- /dev/null +++ b/framework/core/src/Extension/Exception/DependentExtensionsExceptionHandler.php @@ -0,0 +1,34 @@ +withDetails($this->errorDetails($e)); + } + + protected function errorDetails(DependentExtensionsException $e): array + { + return [ + [ + 'extension' => $e->extension->getId(), + 'extensions' => $e->getDependentExtensionIds(), + ] + ]; + } +} diff --git a/framework/core/src/Extension/Exception/MissingDependenciesException.php b/framework/core/src/Extension/Exception/MissingDependenciesException.php new file mode 100644 index 000000000..114348ca4 --- /dev/null +++ b/framework/core/src/Extension/Exception/MissingDependenciesException.php @@ -0,0 +1,47 @@ +extension = $extension; + $this->missing_dependencies = $missing_dependencies; + + parent::__construct($extension->getId().' could not be enabled, because it depends on: '.implode(', ', $this->getMissingDependencyIds())); + } + + /** + * Get array of IDs for missing (disabled) extensions that this extension depends on. + * + * @return array + */ + public function getMissingDependencyIds() + { + return array_map(function (Extension $extension) { + return $extension->getId(); + }, $this->missing_dependencies); + } +} diff --git a/framework/core/src/Extension/Exception/MissingDependenciesExceptionHandler.php b/framework/core/src/Extension/Exception/MissingDependenciesExceptionHandler.php new file mode 100644 index 000000000..39f8980cc --- /dev/null +++ b/framework/core/src/Extension/Exception/MissingDependenciesExceptionHandler.php @@ -0,0 +1,34 @@ +withDetails($this->errorDetails($e)); + } + + protected function errorDetails(MissingDependenciesException $e): array + { + return [ + [ + 'extension' => $e->extension->getId(), + 'extensions' => $e->getMissingDependencyIds(), + ] + ]; + } +} diff --git a/framework/core/src/Extension/Extension.php b/framework/core/src/Extension/Extension.php index 3707bfae0..ea7b90ed0 100644 --- a/framework/core/src/Extension/Extension.php +++ b/framework/core/src/Extension/Extension.php @@ -15,6 +15,7 @@ use Flarum\Extend\LifecycleInterface; use Illuminate\Contracts\Container\Container; use Illuminate\Contracts\Support\Arrayable; use Illuminate\Support\Arr; +use Illuminate\Support\Collection; use Illuminate\Support\Str; use League\Flysystem\Adapter\Local; use League\Flysystem\Filesystem; @@ -51,6 +52,14 @@ class Extension implements Arrayable 'jpg' => 'image/jpeg', ]; + protected static function nameToId($name) + { + list($vendor, $package) = explode('/', $name); + $package = str_replace(['flarum-ext-', 'flarum-'], '', $package); + + return "$vendor-$package"; + } + /** * Unique Id of the extension. * @@ -60,6 +69,7 @@ class Extension implements Arrayable * @var string */ protected $id; + /** * The directory of this extension. * @@ -74,6 +84,13 @@ class Extension implements Arrayable */ protected $composerJson; + /** + * The IDs of all Flarum extensions that this extension depends on. + * + * @var string[] + */ + protected $extensionDependencyIds; + /** * Whether the extension is installed. * @@ -104,9 +121,7 @@ class Extension implements Arrayable */ protected function assignId() { - list($vendor, $package) = explode('/', $this->name); - $package = str_replace(['flarum-ext-', 'flarum-'], '', $package); - $this->id = "$vendor-$package"; + $this->id = static::nameToId($this->name); } public function extend(Container $app) @@ -182,6 +197,24 @@ class Extension implements Arrayable return $this; } + /** + * Get the list of flarum extensions that this extension depends on. + * + * @param array $extensionSet: An associative array where keys are the composer package names + * of installed extensions. Used to figure out which dependencies + * are flarum extensions. + */ + public function calculateDependencies($extensionSet) + { + $this->extensionDependencyIds = (new Collection(Arr::get($this->composerJson, 'require', []))) + ->keys() + ->filter(function ($key) use ($extensionSet) { + return array_key_exists($key, $extensionSet); + })->map(function ($key) { + return static::nameToId($key); + })->toArray(); + } + /** * @return string */ @@ -253,6 +286,16 @@ class Extension implements Arrayable return $this->path; } + /** + * The IDs of all Flarum extensions that this extension depends on. + * + * @return array + */ + public function getExtensionDependencyIds() + { + return $this->extensionDependencyIds; + } + private function getExtenders(): array { $extenderFile = $this->getExtenderFile(); @@ -363,12 +406,13 @@ class Extension implements Arrayable public function toArray() { return (array) array_merge([ - 'id' => $this->getId(), - 'version' => $this->getVersion(), - 'path' => $this->path, - 'icon' => $this->getIcon(), - 'hasAssets' => $this->hasAssets(), - 'hasMigrations' => $this->hasMigrations(), + 'id' => $this->getId(), + 'version' => $this->getVersion(), + 'path' => $this->getPath(), + 'icon' => $this->getIcon(), + 'hasAssets' => $this->hasAssets(), + 'hasMigrations' => $this->hasMigrations(), + 'extensionDependencyIds' => $this->getExtensionDependencyIds(), ], $this->composerJson); } } diff --git a/framework/core/src/Extension/ExtensionManager.php b/framework/core/src/Extension/ExtensionManager.php index 40136e348..7b393dc5a 100644 --- a/framework/core/src/Extension/ExtensionManager.php +++ b/framework/core/src/Extension/ExtensionManager.php @@ -83,11 +83,18 @@ class ExtensionManager // Composer 2.0 changes the structure of the installed.json manifest $installed = $installed['packages'] ?? $installed; + // 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. + $installedSet = []; + foreach ($installed as $package) { if (Arr::get($package, 'type') != 'flarum-extension' || empty(Arr::get($package, 'name'))) { continue; } + $installedSet[Arr::get($package, 'name')] = true; + $path = isset($package['install-path']) ? $this->paths->vendor.'/composer/'.$package['install-path'] : $this->paths->vendor.'/'.Arr::get($package, 'name'); @@ -101,6 +108,11 @@ class ExtensionManager $extensions->put($extension->getId(), $extension); } + + foreach ($extensions as $extension) { + $extension->calculateDependencies($installedSet); + } + $this->extensions = $extensions->sortBy(function ($extension, $name) { return $extension->composerJsonAttribute('extra.flarum-extension.title'); }); @@ -133,17 +145,27 @@ class ExtensionManager $extension = $this->getExtension($name); + $missingDependencies = []; + $enabledIds = $this->getEnabled(); + foreach ($extension->getExtensionDependencyIds() as $dependencyId) { + if (! in_array($dependencyId, $enabledIds)) { + $missingDependencies[] = $this->getExtension($dependencyId); + } + } + + if (! empty($missingDependencies)) { + throw new Exception\MissingDependenciesException($extension, $missingDependencies); + } + $this->dispatcher->dispatch(new Enabling($extension)); - $enabled = $this->getEnabled(); - - $enabled[] = $name; + $enabledIds[] = $name; $this->migrate($extension); $this->publishAssets($extension); - $this->setEnabled($enabled); + $this->setEnabled($enabledIds); $extension->enable($this->container); @@ -165,6 +187,18 @@ class ExtensionManager $extension = $this->getExtension($name); + $dependentExtensions = []; + + foreach ($this->getEnabledExtensions() as $possibleDependent) { + if (in_array($extension->getId(), $possibleDependent->getExtensionDependencyIds())) { + $dependentExtensions[] = $possibleDependent; + } + } + + if (! empty($dependentExtensions)) { + throw new Exception\DependentExtensionsException($extension, $dependentExtensions); + } + $this->dispatcher->dispatch(new Disabling($extension)); unset($enabled[$k]); diff --git a/framework/core/src/Foundation/ErrorServiceProvider.php b/framework/core/src/Foundation/ErrorServiceProvider.php index 58c827f7f..02483841d 100644 --- a/framework/core/src/Foundation/ErrorServiceProvider.php +++ b/framework/core/src/Foundation/ErrorServiceProvider.php @@ -9,6 +9,10 @@ namespace Flarum\Foundation; +use Flarum\Extension\Exception\DependentExtensionsException; +use Flarum\Extension\Exception\DependentExtensionsExceptionHandler; +use Flarum\Extension\Exception\MissingDependenciesException; +use Flarum\Extension\Exception\MissingDependenciesExceptionHandler; use Flarum\Foundation\ErrorHandling\ExceptionHandler; use Flarum\Foundation\ErrorHandling\LogReporter; use Flarum\Foundation\ErrorHandling\Registry; @@ -57,6 +61,8 @@ class ErrorServiceProvider extends AbstractServiceProvider return [ IlluminateValidationException::class => ExceptionHandler\IlluminateValidationExceptionHandler::class, ValidationException::class => ExceptionHandler\ValidationExceptionHandler::class, + DependentExtensionsException::class => DependentExtensionsExceptionHandler::class, + MissingDependenciesException::class => MissingDependenciesExceptionHandler::class, ]; });