Revamp Plugin Manager (#501)

Reworked the internal disabled status checking and monitoring system for plugins.
Added better support for plugin disabled status management.
Added improvements to loading (should aid in improving app bootup speed)

modules/system/classes/PluginBase.php

@@ -326,7 +326,7 @@ class PluginBase extends ServiceProviderBase
             $this->loadedYamlConfiguration = [];
         }
         else {
-            $this->loadedYamlConfiguration = Yaml::parse(file_get_contents($yamlFilePath));
+            $this->loadedYamlConfiguration = Yaml::parseFile($yamlFilePath);
             if (!is_array($this->loadedYamlConfiguration)) {
                 throw new SystemException(sprintf('Invalid format of the plugin configuration file: %s. The file should define an array.', $yamlFilePath));
             }
@@ -404,8 +404,6 @@ class PluginBase extends ServiceProviderBase
 
     /**
      * Returns the absolute path to this plugin's directory
-     *
-     * @return string
      */
     public function getPluginPath(): string
     {
@@ -414,7 +412,7 @@ class PluginBase extends ServiceProviderBase
         }
 
         $reflection = new ReflectionClass($this);
-        $this->path = dirname($reflection->getFileName());
+        $this->path = File::normalizePath(dirname($reflection->getFileName()));
 
         return $this->path;
     }
@@ -435,7 +433,7 @@ class PluginBase extends ServiceProviderBase
         if (
             !File::isFile($versionFile)
             || !($versionInfo = Yaml::withProcessor(new VersionYamlProcessor, function ($yaml) use ($versionFile) {
-                return $yaml->parse(file_get_contents($versionFile));
+                return $yaml->parseFile($versionFile);
             }))
             || !is_array($versionInfo)
         ) {
@@ -448,4 +446,25 @@ class PluginBase extends ServiceProviderBase
 
         return $this->version = trim(key(array_slice($versionInfo, -1, 1)));
     }
+
+    /**
+     * Verifies the plugin's dependencies are present and enabled
+     */
+    public function checkDependencies(PluginManager $manager): bool
+    {
+        $required = $manager->getDependencies($this);
+        if (empty($required)) {
+            return true;
+        }
+
+        foreach ($required as $require) {
+            $requiredPlugin = $manager->findByIdentifier($require);
+
+            if (!$requiredPlugin || $manager->isDisabled($requiredPlugin)) {
+                return false;
+            }
+        }
+
+        return true;
+    }
 }

modules/system/classes/UpdateManager.php

@@ -999,7 +999,8 @@ class UpdateManager
         $postData['server'] = base64_encode(serialize([
             'php'   => PHP_VERSION,
             'url'   => Url::to('/'),
-            'since' => PluginVersion::orderBy('created_at')->value('created_at')
+            // TODO: Store system boot date in `Parameter`
+            'since' => PluginVersion::orderBy('created_at')->first()->created_at
         ]));
 
         if ($projectId = Parameter::get('system::project.id')) {

modules/system/classes/VersionManager.php

@@ -346,7 +346,7 @@ class VersionManager
 
         $versionFile = $this->getVersionFile($code);
         $versionInfo = Yaml::withProcessor(new VersionYamlProcessor, function ($yaml) use ($versionFile) {
-            return $yaml->parse(file_get_contents($versionFile));
+            return $yaml->parseFile($versionFile);
         });
 
         if (!is_array($versionInfo)) {

modules/system/console/PluginDisable.php

@@ -41,10 +41,6 @@ class PluginDisable extends Command
 
         // Disable this plugin
         $pluginManager->disablePlugin($pluginName);
-        $plugin = PluginVersion::where('code', $pluginName)->first();
-        $plugin->is_disabled = true;
-        $plugin->save();
-        $pluginManager->clearDisabledCache();
 
         $this->output->writeln(sprintf('<info>%s:</info> disabled.', $pluginName));
     }

modules/system/console/PluginEnable.php

@@ -46,10 +46,6 @@ class PluginEnable extends Command
 
         // Enable this plugin
         $pluginManager->enablePlugin($pluginName);
-        $plugin = PluginVersion::where('code', $pluginName)->first();
-        $plugin->is_disabled = false;
-        $plugin->save();
-        $pluginManager->clearDisabledCache();
 
         $this->output->writeln(sprintf('<info>%s:</info> enabled.', $pluginName));
     }

modules/system/controllers/Updates.php

@@ -91,7 +91,7 @@ class Updates extends Controller
     public function manage()
     {
         $this->pageTitle = 'system::lang.plugins.manage';
-        PluginManager::instance()->clearDisabledCache();
+        PluginManager::instance()->clearFlagCache();
         return $this->asExtension('ListController')->index();
     }
 
@@ -238,6 +238,10 @@ class Updates extends Controller
         $warnings = [];
         $missingDependencies = PluginManager::instance()->findMissingDependencies();
 
+        if (!empty($missingDependencies)) {
+            PluginManager::instance()->clearFlagCache();
+        }
+
         foreach ($missingDependencies as $pluginCode => $plugin) {
             foreach ($plugin as $missingPluginCode) {
                 $warnings[] = Lang::get('system::lang.updates.update_warnings_plugin_missing', [
@@ -845,52 +849,45 @@ class Updates extends Controller
             count($checkedIds)
         ) {
             $manager = PluginManager::instance();
+            $codes = PluginVersion::lists('code', 'id');
 
-            foreach ($checkedIds as $pluginId) {
-                if (!$plugin = PluginVersion::find($pluginId)) {
+            foreach ($checkedIds as $id) {
+                $code = $codes[$id] ?? null;
+                if (!$code) {
                     continue;
                 }
 
-                $savePlugin = true;
                 switch ($bulkAction) {
                     // Enables plugin's updates.
                     case 'freeze':
-                        $plugin->is_frozen = 1;
+                        $manager->freezePlugin($code);
                         break;
 
                     // Disables plugin's updates.
                     case 'unfreeze':
-                        $plugin->is_frozen = 0;
+                        $manager->unfreezePlugin($code);
                         break;
 
                     // Disables plugin on the system.
                     case 'disable':
-                        $plugin->is_disabled = 1;
-                        $manager->disablePlugin($plugin->code, true);
+                        $manager->disablePlugin($code);
                         break;
 
                     // Enables plugin on the system.
                     case 'enable':
-                        $plugin->is_disabled = 0;
-                        $manager->enablePlugin($plugin->code, true);
+                        $manager->enablePlugin($code);
                         break;
 
                     // Rebuilds plugin database migrations.
                     case 'refresh':
-                        $savePlugin = false;
-                        $manager->refreshPlugin($plugin->code);
+                        $manager->refreshPlugin($code);
                         break;
 
                     // Rollback and remove plugins from the system.
                     case 'remove':
-                        $savePlugin = false;
-                        $manager->deletePlugin($plugin->code);
+                        $manager->deletePlugin($code);
                         break;
                 }
-
-                if ($savePlugin) {
-                    $plugin->save();
-                }
             }
         }
 

modules/system/models/PluginVersion.php

@@ -2,7 +2,6 @@
 
 use Lang;
 use Model;
-use Config;
 use System\Classes\PluginManager;
 
 /**
@@ -96,17 +95,22 @@ class PluginVersion extends Model
                 }
             }
 
-            if ($this->is_disabled) {
-                $manager->disablePlugin($this->code, true);
-            }
-            else {
-                $manager->enablePlugin($this->code, true);
-            }
+            $activeFlags = $manager->getPluginFlags($pluginObj);
+            if (!empty($activeFlags)) {
+                foreach ($activeFlags as $flag => $enabled) {
+                    if (in_array($flag, [
+                        PluginManager::DISABLED_MISSING,
+                        PluginManager::DISABLED_REPLACED,
+                        PluginManager::DISABLED_REPLACEMENT_FAILED,
+                        PluginManager::DISABLED_MISSING_DEPENDENCIES,
+                    ])) {
+                        $this->disabledBySystem = true;
+                    }
 
-            $this->disabledBySystem = $pluginObj->disabled;
-
-            if (($configDisabled = Config::get('cms.disablePlugins')) && is_array($configDisabled)) {
-                $this->disabledByConfig = in_array($this->code, $configDisabled);
+                    if ($flag === PluginManager::DISABLED_BY_CONFIG) {
+                        $this->disabledByConfig = true;
+                    }
+                }
             }
         }
         else {
@@ -118,9 +122,8 @@ class PluginVersion extends Model
 
     /**
      * Returns true if the plugin should be updated by the system.
-     * @return bool
      */
-    public function getIsUpdatableAttribute()
+    public function getIsUpdatableAttribute(): bool
     {
         return !$this->is_disabled && !$this->disabledBySystem && !$this->disabledByConfig;
     }
@@ -128,7 +131,7 @@ class PluginVersion extends Model
     /**
      * Only include enabled plugins
      * @param $query
-     * @return mixed
+     * @return QueryBuilder
      */
     public function scopeApplyEnabled($query)
     {
@@ -137,10 +140,8 @@ class PluginVersion extends Model
 
     /**
      * Returns the current version for a plugin
-     * @param  string $pluginCode Plugin code. Eg: Acme.Blog
-     * @return string
      */
-    public static function getVersion($pluginCode)
+    public static function getVersion(string $pluginCode): ?string
     {
         if (self::$versionCache === null) {
             self::$versionCache = self::lists('version', 'code');
@@ -152,7 +153,7 @@ class PluginVersion extends Model
     /**
      * Provides the slug attribute.
      */
-    public function getSlugAttribute()
+    public function getSlugAttribute(): string
     {
         return self::makeSlug($this->code);
     }
@@ -160,7 +161,7 @@ class PluginVersion extends Model
     /**
      * Generates a slug for the plugin.
      */
-    public static function makeSlug($code)
+    public static function makeSlug(string $code): string
     {
         return strtolower(str_replace('.', '-', $code));
     }

modules/system/providers.php

@@ -7,7 +7,6 @@ return [
      */
     Illuminate\Broadcasting\BroadcastServiceProvider::class,
     Illuminate\Bus\BusServiceProvider::class,
-    Illuminate\Cache\CacheServiceProvider::class,
     Illuminate\Cookie\CookieServiceProvider::class,
     Illuminate\Encryption\EncryptionServiceProvider::class,
     Illuminate\Foundation\Providers\FoundationServiceProvider::class,
@@ -22,6 +21,7 @@ return [
     /*
      * Winter Storm providers
      */
+    Winter\Storm\Cache\CacheServiceProvider::class,
     Winter\Storm\Foundation\Providers\ConsoleSupportServiceProvider::class,
     Winter\Storm\Database\DatabaseServiceProvider::class,
     Winter\Storm\Halcyon\HalcyonServiceProvider::class,

modules/system/reportwidgets/Status.php

@@ -69,7 +69,8 @@ class Status extends ReportWidgetBase
         $this->vars['requestLog']    = RequestLog::count();
         $this->vars['requestLogMsg'] = LogSetting::get('log_requests', false) ? false : true;
 
-        $this->vars['appBirthday'] = PluginVersion::orderBy('created_at')->value('created_at');
+        // TODO: Store system boot date in `Parameter`
+        $this->vars['appBirthday'] = PluginVersion::orderBy('created_at')->first()->created_at;
     }
 
     public function onLoadWarningsForm()

tests/unit/system/classes/PluginManagerTest.php

@@ -1,22 +1,118 @@
 <?php
 use System\Classes\PluginManager;
+use System\Classes\UpdateManager;
+use System\Classes\VersionManager;
+
+use Winter\Storm\Database\Model as ActiveRecord;
 
 class PluginManagerTest extends TestCase
 {
     public $manager;
+    protected $output;
 
+    /**
+     * Creates the application.
+     * @return Symfony\Component\HttpKernel\HttpKernelInterface
+     */
+    public function createApplication()
+    {
+        $app = parent::createApplication();
+
+        /*
+         * Store database in memory by default unless specified otherwise
+         */
+        if (!file_exists(base_path('config/testing/database.php'))) {
+            $app['config']->set('database.connections.testing', [
+                'driver'   => 'sqlite',
+                'database' => ':memory:',
+            ]);
+            $app['config']->set('database.default', 'testing');
+        }
+
+        return $app;
+    }
+
+    /**
+     * Perform test case set up.
+     * @return void
+     */
     public function setUp() : void
     {
+        /*
+         * Force reload of Winter singletons
+         */
+        PluginManager::forgetInstance();
+        UpdateManager::forgetInstance();
+
+        // Forces plugin migrations to be run again on every test
+        VersionManager::forgetInstance();
+
+        $this->output = new \Symfony\Component\Console\Output\BufferedOutput();
+
         parent::setUp();
 
+        /*
+         * Ensure system is up to date
+         */
+        $this->runWinterUpCommand();
+
         $manager = PluginManager::instance();
         self::callProtectedMethod($manager, 'loadDisabled');
         $manager->loadPlugins();
         self::callProtectedMethod($manager, 'loadDependencies');
-
         $this->manager = $manager;
     }
 
+    /**
+     * Flush event listeners and collect garbage.
+     * @return void
+     */
+    public function tearDown() : void
+    {
+        $this->flushModelEventListeners();
+        parent::tearDown();
+        unset($this->app);
+    }
+
+    /**
+     * Migrate database using winter:up command.
+     * @return void
+     */
+    protected function runWinterUpCommand()
+    {
+        UpdateManager::instance()
+            ->setNotesOutput($this->output)
+            ->update();
+    }
+
+    /**
+     * The models in Winter use a static property to store their events, these
+     * will need to be targeted and reset ready for a new test cycle.
+     * Pivot models are an exception since they are internally managed.
+     * @return void
+     */
+    protected function flushModelEventListeners()
+    {
+        foreach (get_declared_classes() as $class) {
+            if ($class === 'Winter\Storm\Database\Pivot' || strtolower($class) === 'october\rain\database\pivot') {
+                continue;
+            }
+
+            $reflectClass = new ReflectionClass($class);
+            if (
+                !$reflectClass->isInstantiable() ||
+                !$reflectClass->isSubclassOf('Winter\Storm\Database\Model') ||
+                $reflectClass->isSubclassOf('Winter\Storm\Database\Pivot')
+            ) {
+                continue;
+            }
+
+            $class::flushEventListeners();
+        }
+
+        ActiveRecord::flushEventListeners();
+    }
+
     //
     // Tests
     //
@@ -290,4 +386,56 @@ class PluginManagerTest extends TestCase
         $this->assertEquals('Winter.Replacement', $this->manager->getActiveReplacementMap('Winter.Original'));
         $this->assertNull($this->manager->getActiveReplacementMap('Winter.InvalidReplacement'));
     }
+
+    public function testFlagDisableStatus()
+    {
+        $plugin = $this->manager->findByIdentifier('DependencyTest.Dependency');
+        $flags = $this->manager->getPluginFlags($plugin);
+        $this->assertEmpty($flags);
+
+        $plugin = $this->manager->findByIdentifier('DependencyTest.NotFound');
+        $flags = $this->manager->getPluginFlags($plugin);
+        $this->assertCount(1, $flags);
+        $this->assertArrayHasKey(PluginManager::DISABLED_MISSING_DEPENDENCIES, $flags);
+
+        $plugin = $this->manager->findByIdentifier('Winter.InvalidReplacement');
+        $flags = $this->manager->getPluginFlags($plugin);
+        $this->assertCount(1, $flags);
+        $this->assertArrayHasKey(PluginManager::DISABLED_REPLACEMENT_FAILED, $flags);
+
+        $plugin = $this->manager->findByIdentifier('Winter.Original', true);
+        $flags = $this->manager->getPluginFlags($plugin);
+        $this->assertCount(1, $flags);
+        $this->assertArrayHasKey(PluginManager::DISABLED_REPLACED, $flags);
+    }
+
+    public function testFlagDisabling()
+    {
+        $plugin = $this->manager->findByIdentifier('Winter.Tester', true);
+
+        $flags = $this->manager->getPluginFlags($plugin);
+        $this->assertEmpty($flags);
+
+        $this->manager->disablePlugin($plugin);
+
+        $flags = $this->manager->getPluginFlags($plugin);
+        $this->assertCount(1, $flags);
+        $this->assertArrayHasKey(PluginManager::DISABLED_BY_USER, $flags);
+
+        $this->manager->enablePlugin($plugin);
+
+        $flags = $this->manager->getPluginFlags($plugin);
+        $this->assertEmpty($flags);
+
+        $this->manager->disablePlugin($plugin, PluginManager::DISABLED_BY_CONFIG);
+
+        $flags = $this->manager->getPluginFlags($plugin);
+        $this->assertCount(1, $flags);
+        $this->assertArrayHasKey(PluginManager::DISABLED_BY_CONFIG, $flags);
+
+        $this->manager->enablePlugin($plugin, PluginManager::DISABLED_BY_CONFIG);
+
+        $flags = $this->manager->getPluginFlags($plugin);
+        $this->assertEmpty($flags);
+    }
 }