Skip to content

Commit

Permalink
MDL-78543 admin: refactor admin_apply_default_settings()
Browse files Browse the repository at this point in the history
  • Loading branch information
@skodak
skodak committed Jul 14, 2023
1 parent e774522 commit 9244683
Show file tree
Hide file tree
Showing 3 changed files with 145 additions and 39 deletions.
102 changes: 63 additions & 39 deletions lib/adminlib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

/**
Expand Down
79 changes: 79 additions & 0 deletions lib/tests/adminlib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', '[email protected]');
$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);
}
}
3 changes: 3 additions & 0 deletions lib/upgrade.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down

0 comments on commit 9244683

Please sign in to comment.