From 2553d67c7469b55741f36a07c3db8217d1463766 Mon Sep 17 00:00:00 2001 From: John Okely Date: Tue, 28 Mar 2017 11:19:59 +0800 Subject: [PATCH 1/2] MDL-58361 media: Remove media manager subclass from unit test --- lib/tests/medialib_test.php | 71 ++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 40 deletions(-) diff --git a/lib/tests/medialib_test.php b/lib/tests/medialib_test.php index 643ed07f90d..20e5d6d734e 100644 --- a/lib/tests/medialib_test.php +++ b/lib/tests/medialib_test.php @@ -133,28 +133,28 @@ class core_medialib_testcase extends advanced_testcase { */ public function test_get_players() { // All players are initially disabled (except link, which you can't). - $manager = new core_media_manager_test(); - $this->assertEmpty($manager->get_players_test()); + $manager = core_media_manager::instance(); + $this->assertEmpty($this->get_players_test($manager)); // A couple enabled, check the order. \core\plugininfo\media::set_enabled_plugins('youtube,html5audio'); - $manager = new core_media_manager_test(); - $this->assertSame('youtube, html5audio', $manager->get_players_test()); + $manager = core_media_manager::instance(); + $this->assertSame('youtube, html5audio', $this->get_players_test($manager)); // Test SWF and HTML5 media order. \core\plugininfo\media::set_enabled_plugins('html5video,html5audio,swf'); - $manager = new core_media_manager_test(); - $this->assertSame('html5video, html5audio, swf', $manager->get_players_test()); + $manager = core_media_manager::instance(); + $this->assertSame('html5video, html5audio, swf', $this->get_players_test($manager)); // Make sure that our test plugin is considered installed. \core\plugininfo\media::set_enabled_plugins('test,html5video'); - $manager = new core_media_manager_test(); - $this->assertSame('test, html5video', $manager->get_players_test()); + $manager = core_media_manager::instance(); + $this->assertSame('test, html5video', $this->get_players_test($manager)); // Make sure that non-existing plugin is NOT considered installed. \core\plugininfo\media::set_enabled_plugins('nonexistingplugin,html5video'); - $manager = new core_media_manager_test(); - $this->assertSame('html5video', $manager->get_players_test()); + $manager = core_media_manager::instance(); + $this->assertSame('html5video', $this->get_players_test($manager)); } /** @@ -474,6 +474,27 @@ class core_medialib_testcase extends advanced_testcase { $this->assertNotSame($mediamanager1, $mediamanager3); } + + + /** + * Access list of players as string, shortening it by getting rid of + * repeated text. + * @param core_media_manager $manager The core_media_manager instance + * @return string Comma-separated list of players + */ + public function get_players_test($manager) { + $method = new ReflectionMethod("core_media_manager", "get_players"); + $method->setAccessible(true); + $players = $method->invoke($manager); + $out = ''; + foreach ($players as $player) { + if ($out) { + $out .= ', '; + } + $out .= str_replace('core_media_player_', '', preg_replace('/^media_(.*)_plugin$/', '$1', get_class($player))); + } + return $out; + } } /** @@ -513,33 +534,3 @@ class media_test_plugin extends core_media_player { return 10; } } - -/** - * Media renderer override for testing purposes. - */ -class core_media_manager_test extends core_media_manager { - /** - * Access list of players as string, shortening it by getting rid of - * repeated text. - * @return string Comma-separated list of players - */ - public function get_players_test() { - $players = $this->get_players(); - $out = ''; - foreach ($players as $player) { - if ($out) { - $out .= ', '; - } - $out .= str_replace('core_media_player_', '', preg_replace('/^media_(.*)_plugin$/', '$1', get_class($player))); - } - return $out; - } - - /** - * Override the constructor to access it. - */ - public function __construct() { - global $PAGE; - parent::__construct($PAGE); - } -} From fb73f60a3674c1b4d4550a79c4ee573f1b1ee929 Mon Sep 17 00:00:00 2001 From: John Okely Date: Thu, 23 Mar 2017 10:22:00 +0800 Subject: [PATCH 2/2] MDL-58361 media: Make core_meda_manager constructor private --- media/classes/manager.php | 12 ++++++------ media/upgrade.txt | 1 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/media/classes/manager.php b/media/classes/manager.php index 1d23ecb6ab8..cab7edb7586 100644 --- a/media/classes/manager.php +++ b/media/classes/manager.php @@ -39,7 +39,7 @@ defined('MOODLE_INTERNAL') || die(); * @author 2011 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class core_media_manager { +final class core_media_manager { /** * Option: Disable text link fallback. * @@ -95,10 +95,10 @@ class core_media_manager { private $embeddablemarkers; /** @var core_media_manager caches a singleton instance */ - static protected $instance; + static private $instance; /** @var moodle_page page this instance was initialised for */ - protected $page; + private $page; /** * Returns a singleton instance of a manager @@ -125,7 +125,7 @@ class core_media_manager { * @param moodle_page $page The page we are going to add requirements to. * @see core_media_manager::instance() */ - protected function __construct($page) { + private function __construct($page) { if ($page) { $this->page = $page; $players = $this->get_players(); @@ -169,7 +169,7 @@ class core_media_manager { * * @return core_media_player[] Array of core_media_player objects in rank order */ - protected function get_players() { + private function get_players() { // Save time by only building the list once. if (!$this->players) { $sortorder = \core\plugininfo\media::get_enabled_plugins(); @@ -298,7 +298,7 @@ class core_media_manager { * @param array $options Options array * @return string HTML code for embed */ - protected function fallback_to_link($urls, $name, $options) { + private function fallback_to_link($urls, $name, $options) { // If link is turned off, return empty. if (!empty($options[self::OPTION_NO_LINK])) { return ''; diff --git a/media/upgrade.txt b/media/upgrade.txt index ef95d344622..2deef55c3c5 100644 --- a/media/upgrade.txt +++ b/media/upgrade.txt @@ -3,3 +3,4 @@ information provided here is intended especially for developers. === 3.3 === * core_media_manager setup() is now deprecated as it is now called when initialising core_media_manager::instance(). +* core_media_manager is now final. Do not extend core_media_manager, instead create a media plugin.