MDL-49329 admin: Keep reference to plugin manager in plugininfo objects
authorDavid Mudrák <david@moodle.com>
Thu, 24 Sep 2015 20:34:31 +0000 (22:34 +0200)
committerDavid Mudrák <david@moodle.com>
Thu, 8 Oct 2015 21:32:02 +0000 (23:32 +0200)
Plugin info objects are owned by the plugin manager (composition
pattern). Even if the plugin manager is a singleton, we need to keep
explicit reference to the plugin manager that owns the plugin info so
that we can mock up things in unit tests.

Therefore this patch introduces a new property of plugin info objects
that holds the reference to the instance of the plugin manager that made
(and hence owns) the given info instance.

The only trouble here is with static methods of plugin info classes such
as \core\plugininfo\base::get_enabled_plugins(). In these cases, the
code keeps using the core_plugin_manager singleton. The solution would
be to pass the plugin manager instance as a parameter but that is not
worth of change for now, IMHO.

lib/classes/plugin_manager.php
lib/classes/plugininfo/base.php
lib/classes/plugininfo/format.php
lib/classes/plugininfo/orphaned.php
lib/tests/plugin_manager_test.php

index 254e04f..757b219 100644 (file)
@@ -375,13 +375,13 @@ class core_plugin_manager {
         if (!isset($types[$type])) {
             // Orphaned subplugins!
             $plugintypeclass = static::resolve_plugininfo_class($type);
-            $this->pluginsinfo[$type] = $plugintypeclass::get_plugins($type, null, $plugintypeclass);
+            $this->pluginsinfo[$type] = $plugintypeclass::get_plugins($type, null, $plugintypeclass, $this);
             return $this->pluginsinfo[$type];
         }
 
         /** @var \core\plugininfo\base $plugintypeclass */
         $plugintypeclass = static::resolve_plugininfo_class($type);
-        $plugins = $plugintypeclass::get_plugins($type, $types[$type], $plugintypeclass);
+        $plugins = $plugintypeclass::get_plugins($type, $types[$type], $plugintypeclass, $this);
         $this->pluginsinfo[$type] = $plugins;
 
         return $this->pluginsinfo[$type];
index 71f0187..fae917a 100644 (file)
@@ -61,6 +61,8 @@ abstract class base {
     public $instances;
     /** @var int order of the plugin among other plugins of the same type - not supported yet */
     public $sortorder;
+    /** @var core_plugin_manager the plugin manager this plugin info is part of */
+    public $pluginman;
 
     /** @var array|null array of {@link \core\update\info} for this plugin */
     protected $availableupdates;
@@ -77,23 +79,26 @@ abstract class base {
      * Gathers and returns the information about all plugins of the given type,
      * either on disk or previously installed.
      *
+     * This is supposed to be used exclusively by the plugin manager when it is
+     * populating its tree of plugins.
+     *
      * @param string $type the name of the plugintype, eg. mod, auth or workshopform
      * @param string $typerootdir full path to the location of the plugin dir
      * @param string $typeclass the name of the actually called class
+     * @param core_plugin_manager $pluginman the plugin manager calling this method
      * @return array of plugintype classes, indexed by the plugin name
      */
-    public static function get_plugins($type, $typerootdir, $typeclass) {
+    public static function get_plugins($type, $typerootdir, $typeclass, $pluginman) {
         // Get the information about plugins at the disk.
         $plugins = core_component::get_plugin_list($type);
         $return = array();
         foreach ($plugins as $pluginname => $pluginrootdir) {
             $return[$pluginname] = self::make_plugin_instance($type, $typerootdir,
-                $pluginname, $pluginrootdir, $typeclass);
+                $pluginname, $pluginrootdir, $typeclass, $pluginman);
         }
 
         // Fetch missing incorrectly uninstalled plugins.
-        $manager = core_plugin_manager::instance();
-        $plugins = $manager->get_installed_plugins($type);
+        $plugins = $pluginman->get_installed_plugins($type);
 
         foreach ($plugins as $name => $version) {
             if (isset($return[$name])) {
@@ -106,6 +111,7 @@ abstract class base {
             $plugin->rootdir     = null;
             $plugin->displayname = $name;
             $plugin->versiondb   = $version;
+            $plugin->pluginman   = $pluginman;
             $plugin->init_is_standard();
 
             $return[$name] = $plugin;
@@ -122,14 +128,16 @@ abstract class base {
      * @param string $name the plugin name, eg. 'workshop'
      * @param string $namerootdir full path to the location of the plugin
      * @param string $typeclass the name of class that holds the info about the plugin
+     * @param core_plugin_manager $pluginman the plugin manager of the new instance
      * @return base the instance of $typeclass
      */
-    protected static function make_plugin_instance($type, $typerootdir, $name, $namerootdir, $typeclass) {
+    protected static function make_plugin_instance($type, $typerootdir, $name, $namerootdir, $typeclass, $pluginman) {
         $plugin              = new $typeclass();
         $plugin->type        = $type;
         $plugin->typerootdir = $typerootdir;
         $plugin->name        = $name;
         $plugin->rootdir     = $namerootdir;
+        $plugin->pluginman   = $pluginman;
 
         $plugin->init_display_name();
         $plugin->load_disk_version();
@@ -206,7 +214,7 @@ abstract class base {
      * data) or is missing from disk.
      */
     public function load_disk_version() {
-        $versions = core_plugin_manager::instance()->get_present_plugins($this->type);
+        $versions = $this->pluginman->get_present_plugins($this->type);
 
         $this->versiondisk = null;
         $this->versionrequires = null;
@@ -260,7 +268,7 @@ abstract class base {
      * @return string|bool false if not a subplugin, name of the parent otherwise
      */
     public function get_parent_plugin() {
-        return $this->get_plugin_manager()->get_parent_of_subplugin($this->type);
+        return $this->pluginman->get_parent_of_subplugin($this->type);
     }
 
     /**
@@ -272,7 +280,7 @@ abstract class base {
      * data) or has not been installed yet.
      */
     public function load_db_version() {
-        $versions = core_plugin_manager::instance()->get_installed_plugins($this->type);
+        $versions = $this->pluginman->get_installed_plugins($this->type);
 
         if (isset($versions[$this->name])) {
             $this->versiondb = $versions[$this->name];
@@ -291,14 +299,15 @@ abstract class base {
      */
     public function init_is_standard() {
 
-        $standard = core_plugin_manager::standard_plugins_list($this->type);
+        $pluginman = $this->pluginman;
+        $standard = $pluginman::standard_plugins_list($this->type);
 
         if ($standard !== false) {
             $standard = array_flip($standard);
             if (isset($standard[$this->name])) {
                 $this->source = core_plugin_manager::PLUGIN_SOURCE_STANDARD;
             } else if (!is_null($this->versiondb) and is_null($this->versiondisk)
-                and core_plugin_manager::is_deleted_standard_plugin($this->type, $this->name)) {
+                and $pluginman::is_deleted_standard_plugin($this->type, $this->name)) {
                 $this->source = core_plugin_manager::PLUGIN_SOURCE_STANDARD; // To be deleted.
             } else {
                 $this->source = core_plugin_manager::PLUGIN_SOURCE_EXTENSION;
@@ -339,6 +348,8 @@ abstract class base {
      */
     public function get_status() {
 
+        $pluginman = $this->pluginman;
+
         if (is_null($this->versiondb) and is_null($this->versiondisk)) {
             return core_plugin_manager::PLUGIN_STATUS_NODB;
 
@@ -346,7 +357,7 @@ abstract class base {
             return core_plugin_manager::PLUGIN_STATUS_NEW;
 
         } else if (!is_null($this->versiondb) and is_null($this->versiondisk)) {
-            if (core_plugin_manager::is_deleted_standard_plugin($this->type, $this->name)) {
+            if ($pluginman::is_deleted_standard_plugin($this->type, $this->name)) {
                 return core_plugin_manager::PLUGIN_STATUS_DELETE;
             } else {
                 return core_plugin_manager::PLUGIN_STATUS_MISSING;
@@ -385,7 +396,7 @@ abstract class base {
             return false;
         }
 
-        $enabled = core_plugin_manager::instance()->get_enabled_plugins($this->type);
+        $enabled = $this->pluginman->get_enabled_plugins($this->type);
 
         if (!is_array($enabled)) {
             return null;
@@ -597,13 +608,4 @@ abstract class base {
             'return' => $return,
         ));
     }
-
-    /**
-     * Provides access to the core_plugin_manager singleton.
-     *
-     * @return core_plugin_manager
-     */
-    protected function get_plugin_manager() {
-        return core_plugin_manager::instance();
-    }
 }
index d0d161d..5cbd7fb 100644 (file)
@@ -71,12 +71,14 @@ class format extends base {
      * @param string $type the name of the plugintype, eg. mod, auth or workshopform
      * @param string $typerootdir full path to the location of the plugin dir
      * @param string $typeclass the name of the actually called class
+     * @param core_plugin_manager $pluginman the plugin manager calling this method
      * @return array of plugintype classes, indexed by the plugin name
      */
-    public static function get_plugins($type, $typerootdir, $typeclass) {
+    public static function get_plugins($type, $typerootdir, $typeclass, $pluginman) {
         global $CFG;
-        $formats = parent::get_plugins($type, $typerootdir, $typeclass);
         require_once($CFG->dirroot.'/course/lib.php');
+
+        $formats = parent::get_plugins($type, $typerootdir, $typeclass, $pluginman);
         $order = get_sorted_course_formats();
         $sortedformats = array();
         foreach ($order as $formatname) {
@@ -137,7 +139,7 @@ class format extends base {
             return '';
         }
 
-        $defaultformat = $this->get_plugin_manager()->plugin_name('format_'.get_config('moodlecourse', 'format'));
+        $defaultformat = $this->pluginman->plugin_name('format_'.get_config('moodlecourse', 'format'));
         $message = get_string(
             'formatuninstallwithcourses', 'core_admin',
             (object)array('count' => $coursecount, 'format' => $this->displayname,
index ec78d39..09f12d6 100644 (file)
@@ -64,12 +64,12 @@ class orphaned extends base {
      * @param string $type the name of the plugintype, eg. mod, auth or workshopform
      * @param string $typerootdir full path to the location of the plugin dir
      * @param string $typeclass the name of the actually called class
+     * @param core_plugin_manager $pluginman the plugin manager calling this method
      * @return array of plugintype classes, indexed by the plugin name
      */
-    public static function get_plugins($type, $typerootdir, $typeclass) {
+    public static function get_plugins($type, $typerootdir, $typeclass, $pluginman) {
         $return = array();
-        $manager = \core_plugin_manager::instance();
-        $plugins = $manager->get_installed_plugins($type);
+        $plugins = $pluginman->get_installed_plugins($type);
 
         foreach ($plugins as $name => $version) {
             $plugin              = new $typeclass();
@@ -79,6 +79,7 @@ class orphaned extends base {
             $plugin->rootdir     = null;
             $plugin->displayname = $name;
             $plugin->versiondb   = $version;
+            $plugin->pluginman   = $pluginman;
             $plugin->init_is_standard();
 
             $return[$name] = $plugin;
index f9726e8..3926e3d 100644 (file)
@@ -124,6 +124,23 @@ class core_plugin_manager_testcase extends advanced_testcase {
         $this->assertSame($plugininfos2['mod']['forum'], $plugininfos4['mod']['forum']);
     }
 
+    public function test_plugininfo_back_reference_to_the_plugin_manager() {
+        $plugman1 = core_plugin_manager::instance();
+        $plugman2 = testable_core_plugin_manager::instance();
+
+        foreach ($plugman1->get_plugins() as $type => $infos) {
+            foreach ($infos as $info) {
+                $this->assertSame($info->pluginman, $plugman1);
+            }
+        }
+
+        foreach ($plugman2->get_plugins() as $type => $infos) {
+            foreach ($infos as $info) {
+                $this->assertSame($info->pluginman, $plugman2);
+            }
+        }
+    }
+
     public function test_get_plugins_of_type() {
         $plugininfos = core_plugin_manager::instance()->get_plugins();
         foreach ($plugininfos as $type => $infos) {