diff --git a/.upgradenotes/MDL-83703-2024111301454058.yml b/.upgradenotes/MDL-83703-2024111301454058.yml new file mode 100644 index 00000000000..b2f12587c63 --- /dev/null +++ b/.upgradenotes/MDL-83703-2024111301454058.yml @@ -0,0 +1,7 @@ +issueNumber: MDL-83703 +notes: + core: + - message: > + Support for `subplugins.php` files has been removed. All subplugin + metadata must be created in a `subplugins.json` file. + type: removed diff --git a/admin/tool/mfa/db/subplugins.php b/admin/tool/mfa/db/subplugins.php deleted file mode 100644 index 68a0cc3e40e..00000000000 --- a/admin/tool/mfa/db/subplugins.php +++ /dev/null @@ -1,28 +0,0 @@ -. - -/** - * Definition of MFA sub-plugins (factors). - * - * @package tool_mfa - * @author Mikhail Golenkov - * @copyright Catalyst IT - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -defined('MOODLE_INTERNAL') || die(); - -$subplugins = (array) json_decode(file_get_contents($CFG->dirroot."/admin/tool/mfa/db/subplugins.json"))->plugintypes; diff --git a/lib/classes/component.php b/lib/classes/component.php index 931e69c11e2..1b0d791b218 100644 --- a/lib/classes/component.php +++ b/lib/classes/component.php @@ -754,9 +754,10 @@ $cache = ' . var_export($cache, true) . '; error_log("$ownerdir/db/subplugins.json is invalid ($jsonerror)"); } } else if (file_exists("$ownerdir/db/subplugins.php")) { - error_log('Use of subplugins.php has been deprecated. ' . - "Please update your '$ownerdir' plugin to provide a subplugins.json file instead."); - include("$ownerdir/db/subplugins.php"); + throw new coding_exception( + 'Use of subplugins.php has been deprecated and is no longer supported. ' . + "Please update your '$ownerdir' plugin to provide a subplugins.json file instead.", + ); } foreach ($subplugins as $subtype => $dir) { diff --git a/lib/tests/component_test.php b/lib/tests/component_test.php index b8825588c32..f29ba2e10cd 100644 --- a/lib/tests/component_test.php +++ b/lib/tests/component_test.php @@ -16,6 +16,7 @@ namespace core; +use core\exception\coding_exception; use DirectoryIterator; use ReflectionClass; use ReflectionProperty; @@ -36,6 +37,7 @@ final class component_test extends \advanced_testcase { parent::tearDown(); component::reset(); + ini_set('error_log', null); } /** @@ -1382,4 +1384,147 @@ final class component_test extends \advanced_testcase { } } } + + /** + * Test that fetching of subtype data throws an exception when a subplugins.php is present without a json equivalent. + */ + public function test_fetch_subtypes_php_only(): void { + $vfileroot = \org\bovigo\vfs\vfsStream::setup('root', null, [ + 'plugintype' => [ + 'exampleplugin' => [ + 'db' => [ + 'subplugins.php' => '', + ], + ], + ], + ]); + + $this->expectException(coding_exception::class); + $this->expectExceptionMessageMatches('/Use of subplugins.php has been deprecated and is no longer supported/'); + + $pluginroot = $vfileroot->getChild('plugintype/exampleplugin'); + + $rcm = new \ReflectionMethod(\core\component::class, 'fetch_subtypes'); + $rcm->invoke(null, $pluginroot->url()); + } + + /** + * Test that fetching of subtype data does not throw an exception when a subplugins.php is present + * with a json file equivalent. + * + * Note: The content of the php file is irrelevant and we no longer use it anyway. + */ + public function test_fetch_subtypes_php_and_json(): void { + global $CFG; + + $this->resetAfterTest(); + $vfileroot = \org\bovigo\vfs\vfsStream::setup('root', null, [ + 'plugintype' => [ + 'exampleplugin' => [ + 'db' => [ + 'subplugins.json' => json_encode([ + 'plugintypes' => [ + 'exampleplugina' => 'plugintype/exampleplugin/apples', + ], + ]), + 'subplugins.php' => '', + ], + 'apples' => [], + ], + ], + ]); + + $CFG->dirroot = $vfileroot->url(); + $pluginroot = $vfileroot->getChild('plugintype/exampleplugin'); + + $rcm = new \ReflectionMethod(\core\component::class, 'fetch_subtypes'); + $subplugins = $rcm->invoke(null, $pluginroot->url()); + + $this->assertEquals([ + 'exampleplugina' => $pluginroot->getChild('apples')->url(), + ], $subplugins); + } + + /** + * Test that fetching of subtype data throws appropriate exceptions when a subplugins.php is present + * with a json file equivalent. + * + * Note: The content of the php file is irrelevant and we no longer use it anyway. + * + * @dataProvider invalid_subplugins_json_provider + * @param string[] $expectedwarnings Errors to expect in the exception message + * @param array[] $plugintypesdir The contents of the subplugins.json file + */ + public function test_fetch_subtypes_json_invalid_values( + array $expectedwarnings, + array $plugintypesdir, + ): void { + global $CFG; + + $this->resetAfterTest(); + $vfileroot = \org\bovigo\vfs\vfsStream::setup('root', null, [ + 'plugintype' => [ + 'exampleplugin' => [ + 'db' => [ + 'subplugins.json' => json_encode([ + 'plugintypes' => $plugintypesdir, + ]), + 'subplugins.php' => '', + ], + 'apples' => [], + ], + ], + ]); + + $CFG->dirroot = $vfileroot->url(); + $pluginroot = $vfileroot->getChild('plugintype/exampleplugin'); + + $logdir = make_request_directory(); + $logfile = "{$logdir}/error.log"; + ini_set('error_log', $logfile); + + $rcm = new \ReflectionMethod(\core\component::class, 'fetch_subtypes'); + $rcm->invoke(null, $pluginroot->url()); + + $warnings = file_get_contents($logfile); + foreach ($expectedwarnings as $expectedwarning) { + $this->assertMatchesRegularExpression($expectedwarning, $warnings); + } + } + + /** + * Data provider for invalid subplugins.json files. + * + * @return array + */ + public static function invalid_subplugins_json_provider(): array { + return [ + 'Invalid characters in subtype name' => [ + 'expectedwarnings' => [ + "/Invalid subtype .*APPLES.*detected.*invalid characters present/", + ], + 'plugintypesdir' => [ + 'APPLES' => 'plugintype/exampleplugin/APPLES', + ], + ], + + 'Subplugin which duplicates a core subsystem' => [ + 'expectedwarnings' => [ + "/Invalid subtype .*editor.*detected.*duplicates core subsystem/", + ], + 'plugintypesdir' => [ + 'editor' => 'plugintype/exampleplugin/apples', + ], + ], + + 'Subplugin directory does not exist' => [ + 'expectedwarnings' => [ + "/Invalid subtype directory/", + ], + 'plugintypesdir' => [ + 'exampleapples' => 'plugintype/exampleplugin/pears', + ], + ], + ]; + } }