diff --git a/lib/adminlib.php b/lib/adminlib.php index adad417fe7749..7b0bff66356d0 100644 --- a/lib/adminlib.php +++ b/lib/adminlib.php @@ -8791,39 +8791,65 @@ function admin_get_root($reload=false, $requirefulltree=true) { /// settings utility functions /** - * This function applies default settings. + * This function applies default settings recursively. + * * Because setting the defaults of some settings can enable other settings, - * this function is called recursively until no more new settings are found. + * this function calls itself repeatedly (max 4 times) until no more new settings are saved. + * + * NOTE: previous "internal" parameters $admindefaultsettings, $settingsoutput were removed in Moodle 4.3. * - * @param object $node, NULL means complete tree, null by default - * @param bool $unconditional if true overrides all values with defaults, true by default - * @param array $admindefaultsettings default admin settings to apply. Used recursively - * @param array $settingsoutput The names and values of the changed settings. Used recursively - * @return array $settingsoutput The names and values of the changed settings + * @param part_of_admin_tree|null $node NULL means apply all settings with repeated recursion + * @param bool $unconditional if true overrides all values with defaults (true for installation, false for CLI upgrade) + * @return array The names and values of the applied setting defaults */ -function admin_apply_default_settings($node=null, $unconditional=true, $admindefaultsettings=array(), $settingsoutput=array()) { - $counter = 0; +function admin_apply_default_settings(?part_of_admin_tree $node = null, bool $unconditional = true): array { + if (is_null($node)) { + // This function relies heavily on config cache, so we need to enable in-memory caches if it + // is used during install when normal caching is disabled. + $token = new \core_cache\allow_temporary_caches(); // Value not used intentionally, see its destructor. - // This function relies heavily on config cache, so we need to enable in-memory caches if it - // is used during install when normal caching is disabled. - $token = new \core_cache\allow_temporary_caches(); + core_plugin_manager::reset_caches(); + $root = admin_get_root(true, true); + $saved = admin_apply_default_settings($root, $unconditional); + if (!$saved) { + return []; + } - if (is_null($node)) { + for ($i = 1; $i <= 3; $i++) { + core_plugin_manager::reset_caches(); + $root = admin_get_root(true, true); + // No need to force defaults in repeated runs. + $moresaved = admin_apply_default_settings($root, false); + if (!$moresaved) { + // No more setting defaults to save. + return $saved; + } + $saved += $moresaved; + } + + // We should not get here unless there are some problematic settings.php files. core_plugin_manager::reset_caches(); - $node = admin_get_root(true, true); - $counter = count($settingsoutput); + return $saved; } + // Recursive applying of defaults in admin tree. + $saved = []; if ($node instanceof admin_category) { - $entries = array_keys($node->children); - foreach ($entries as $entry) { - $settingsoutput = admin_apply_default_settings( - $node->children[$entry], $unconditional, $admindefaultsettings, $settingsoutput - ); + foreach ($node->children as $child) { + if ($child === null) { + // This should not happen, + // this is to prevent theoretical infinite loops. + continue; + } + if ($child instanceof admin_externalpage) { + continue; + } + $saved += admin_apply_default_settings($child, $unconditional); } } else if ($node instanceof admin_settingpage) { - foreach ($node->settings as $setting) { + /** @var admin_setting $setting */ + foreach ((array)$node->settings as $setting) { if ($setting->nosave) { // Not a real setting, must be a heading or description. continue; @@ -8837,30 +8863,28 @@ function admin_apply_default_settings($node=null, $unconditional=true, $admindef // No value yet - default maybe applied after admin user creation or in upgradesettings. continue; } - - $settingname = $node->name . '_' . $setting->name; // Get a unique name for the setting. - - if (!array_key_exists($settingname, $admindefaultsettings)) { // Only update a setting if not already processed. - $admindefaultsettings[$settingname] = $settingname; - $settingsoutput[$settingname] = $defaultsetting; - - // Set the default for this setting. - $setting->write_setting($defaultsetting); + // This should be unique-enough setting name that matches administration UI. + if ($setting->plugin === null) { + $settingname = $setting->name; + } else { + $settingname = $setting->plugin . '/' . $setting->name; + } + // Set the default for this setting. + $error = $setting->write_setting($defaultsetting); + if ($error === '') { $setting->write_setting_flags(null); + if (is_int($defaultsetting) || $defaultsetting instanceof lang_string + || $defaultsetting instanceof moodle_url) { + $defaultsetting = (string)$defaultsetting; + } + $saved[$settingname] = $defaultsetting; } else { - unset($admindefaultsettings[$settingname]); // Remove processed settings. + debugging("Error applying default setting '$settingname': " . $error, DEBUG_DEVELOPER); } } } - // Call this function recursively until all settings are processed. - if (($node instanceof admin_root) && ($counter != count($settingsoutput))) { - $settingsoutput = admin_apply_default_settings(null, $unconditional, $admindefaultsettings, $settingsoutput); - } - // Just in case somebody modifies the list of active plugins directly. - core_plugin_manager::reset_caches(); - - return $settingsoutput; + return $saved; } /** diff --git a/lib/tests/adminlib_test.php b/lib/tests/adminlib_test.php index cd60aced38cdb..5800dd351613d 100644 --- a/lib/tests/adminlib_test.php +++ b/lib/tests/adminlib_test.php @@ -126,4 +126,83 @@ public function test_db_should_replace_additional_skip_tables(string $table, str $actual = db_should_replace($table, $column, $additionalskiptables); $this->assertSame($actual, $expected); } + + /** + * Test method used by upgradesettings.php to make sure + * there are no missing settings in PHPUnit and Behat tests. + * + * @covers ::admin_output_new_settings_by_page + */ + public function test_admin_output_new_settings_by_page() { + $this->resetAfterTest(); + $this->setAdminUser(); + + // Add settings not set during PHPUnit init. + set_config('supportemail', 'support@example.com'); + $frontpage = new \admin_setting_special_frontpagedesc(); + $frontpage->write_setting('test test'); + + // NOTE: if this test fails then it is most likely extra setting in + // some additional plugin without default - developer needs to add + // a workaround into their db/install.php for PHPUnit and Behat. + + $root = admin_get_root(true, true); + $new = admin_output_new_settings_by_page($root); + $this->assertSame([], $new); + + unset_config('numbering', 'book'); + unset_config('supportemail'); + $root = admin_get_root(true, true); + $new = admin_output_new_settings_by_page($root); + $this->assertCount(2, $new); + } + + /** + * Test repeated recursive application of default settings. + * + * @covers ::admin_apply_default_settings + */ + public function test_admin_apply_default_settings() { + global $DB; + + $this->resetAfterTest(); + $this->setAdminUser(); + + // There should not be any pending new defaults. + $saved = admin_apply_default_settings(null, false); + $this->assertSame([], $saved); + + // Emulation of upgrades from CLI. + unset_config('logocompact', 'core_admin'); + unset_config('grade_aggregationposition'); + unset_config('numbering', 'book'); + unset_config('enabled', 'core_competency'); + unset_config('pushcourseratingstouserplans', 'core_competency'); + $saved = admin_apply_default_settings(null, false); + $expected = [ + 'core_competency/enabled' => '1', + 'grade_aggregationposition' => '1', + 'book/numbering' => '1', + 'core_admin/logocompact' => '', + 'core_competency/pushcourseratingstouserplans' => '1', + ]; + $this->assertEquals($expected, $saved); + + // Repeated application of defaults - not done usually. + $saved = admin_apply_default_settings(null, true); + $this->assertGreaterThan(500, count($saved)); + $saved = admin_apply_default_settings(); + $this->assertGreaterThan(500, count($saved)); + + // Emulate initial application of defaults. + $DB->delete_records('config', []); + $DB->delete_records('config_plugins', []); + purge_all_caches(); + $saved = admin_apply_default_settings(null, true); + $this->assertGreaterThan(500, count($saved)); + + // Make sure there were enough repetitions. + $saved = admin_apply_default_settings(null, false); + $this->assertSame([], $saved); + } } diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 2366ca5cb4b48..a6bc93bea9cd0 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -3,6 +3,9 @@ information provided here is intended especially for developers. === 4.3 === +* Unnecessary parameters of admin_apply_default_settings() function were removed; upgrade script lists + setting names in the same format as admin UI; default setting writing errors now trigger debugging messages; + duplicate setting names (with different plugin part) in one setting page do not cause problems any more. * Added a new render of caption for the table in render_caption. It can be used by set_caption($caption, $captionattributes). e.g. $caption = 'Caption for table'