diff --git a/CHANGES.md b/CHANGES.md index 52bb859..9bf1daa 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,7 @@ Changes ### Unreleased +* 2024-11-12 - Upgrade: Adopt changes from MDL-73703 and break the sync task user updates into chunks. * 2024-11-12 - Improvement: Change location of event observer code, resolves #26 ### v4.4-r1 diff --git a/auth.php b/auth.php index c16a375..7dbc3f8 100644 --- a/auth.php +++ b/auth.php @@ -67,16 +67,29 @@ public function auth_plugin_ldap_syncplus() { /** - * Syncronizes user fron external LDAP server to moodle user table + * Synchronise users from the external LDAP server to Moodle's user table. + * + * Calls sync_users_update_callback() with default callback if appropriate. + * + * @param bool $doupdates will do pull in data updates from LDAP if relevant + * @return bool success + */ + public function sync_users($doupdates = true) { + return $this->sync_users_update_callback($doupdates ? [$this, 'update_users'] : null); + } + + /** + * Synchronise users from the external LDAP server to Moodle's user table (callback). * * Sync is now using username attribute. * * Syncing users removes or suspends users that dont exists anymore in external LDAP. * Creates new users and updates coursecreator status of users. * - * @param bool $do_updates will do pull in data updates from LDAP if relevant + * @param callable|null $updatecallback will do pull in data updates from LDAP if relevant + * @return bool success */ - function sync_users($do_updates=true) { + public function sync_users_update_callback(?callable $updatecallback = null): bool { global $CFG, $DB; require_once($CFG->dirroot . '/user/profile/lib.php'); @@ -351,39 +364,23 @@ function sync_users($do_updates=true) { } // User Updates - time-consuming (optional). - if ($do_updates) { - // Narrow down what fields we need to update. - $updatekeys = $this->get_profile_keys(); - } else { - mtrace(get_string('noupdatestobedone', 'auth_ldap')); - } - if ($do_updates and !empty($updatekeys)) { // run updates only if relevant. + if ($updatecallback && $updatekeys = $this->get_profile_keys()) { // Run updates only if relevant. $users = $DB->get_records_sql('SELECT u.username, u.id FROM {user} u WHERE u.deleted = 0 AND u.auth = ? AND u.mnethostid = ?', array($this->authtype, $CFG->mnet_localhost_id)); if (!empty($users)) { - mtrace(get_string('userentriestoupdate', 'auth_ldap', count($users))); - - foreach ($users as $user) { - $transaction = $DB->start_delegated_transaction(); - $userinfo = $this->get_userinfo($user->username); - if (!$this->update_user_record($user->username, $updatekeys, true, - $this->is_user_suspended((object) $userinfo))) { - $skipped = ' - '.get_string('skipped'); - } - else { - $skipped = ''; + // Update users in chunks as specified in sync_updateuserchunk. + if (!empty($this->config->sync_updateuserchunk)) { + foreach (array_chunk($users, $this->config->sync_updateuserchunk) as $chunk) { + call_user_func($updatecallback, $chunk, $updatekeys); } - mtrace("\t".get_string('auth_dbupdatinguser', 'auth_db', array('name'=>$user->username, 'id'=>$user->id)).$skipped); - - // Update system roles, if needed. - $this->sync_roles($user); - $transaction->allow_commit(); + } else { + call_user_func($updatecallback, $users, $updatekeys); } - unset($users); // free mem. + unset($users); // Free mem. } - } else { // end do updates. + } else { mtrace(get_string('noupdatestobedone', 'auth_ldap')); } @@ -467,6 +464,35 @@ function sync_users($do_updates=true) { return true; } + /** + * Update users from the external LDAP server into Moodle's user table. + * + * Sync helper + * + * @param array $users chunk of users to update + * @param array $updatekeys fields to update + */ + public function update_users(array $users, array $updatekeys): void { + global $DB; + + mtrace(get_string('userentriestoupdate', 'auth_ldap', count($users))); + + foreach ($users as $user) { + $transaction = $DB->start_delegated_transaction(); + echo "\t"; + print_string('auth_dbupdatinguser', 'auth_db', ['name' => $user->username, 'id' => $user->id]); + $userinfo = $this->get_userinfo($user->username); + if (!$this->update_user_record($user->username, $updatekeys, true, + $this->is_user_suspended((object) $userinfo))) { + echo ' - '.get_string('skipped'); + } + echo "\n"; + + // Update system roles, if needed. + $this->sync_roles($user); + $transaction->allow_commit(); + } + } /** * Support login via email ($CFG->authloginviaemail) for first-time LDAP logins diff --git a/classes/task/asynchronous_sync_task.php b/classes/task/asynchronous_sync_task.php new file mode 100644 index 0000000..e665275 --- /dev/null +++ b/classes/task/asynchronous_sync_task.php @@ -0,0 +1,61 @@ +. + +/** + * Auth plugin "LDAP SyncPlus" - Ad-hoc task definition + * + * @package auth_ldap_syncplus + * @copyright 2024 Alexander Bias, ssystems GmbH + * based on code by Catalyst IT + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace auth_ldap_syncplus\task; + +use core\task\adhoc_task; + +/** + * The auth_ldap_syncplus ad-hoc task class for LDAP user update + * + * @package auth_ldap_syncplus + * @copyright 2024 Alexander Bias, ssystems GmbH + * based on code by Catalyst IT + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class asynchronous_sync_task extends adhoc_task { + /** @var string Message prefix for mtrace */ + protected const MTRACE_MSG = 'Synced LDAP (Sync Plus) users'; + + /** + * Constructor + */ + public function __construct() { + $this->set_component('auth_ldap_syncplus'); + } + + /** + * Run users sync. + */ + public function execute() { + $data = $this->get_custom_data(); + + /** @var auth_plugin_ldap $auth */ + $auth = get_auth_plugin('ldap_syncplus'); + $auth->update_users($data->users, $data->updatekeys); + + mtrace(sprintf(" %s (%d)", self::MTRACE_MSG, count($data->users))); + } +} diff --git a/classes/task/sync_task.php b/classes/task/sync_task.php index 56908e4..34b04be 100644 --- a/classes/task/sync_task.php +++ b/classes/task/sync_task.php @@ -18,7 +18,8 @@ * Auth plugin "LDAP SyncPlus" - Task definition * * @package auth_ldap_syncplus - * @copyright 2014 Alexander Bias, Ulm University + * @copyright 2024 Alexander Bias, ssystems GmbH + * based on code by Catalyst IT * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ @@ -28,10 +29,13 @@ * The auth_ldap_syncplus scheduled task class for LDAP user sync * * @package auth_ldap_syncplus - * @copyright 2014 Alexander Bias, Ulm University + * @copyright 2024 Alexander Bias, ssystems GmbH + * based on code by Catalyst IT * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class sync_task extends \core\task\scheduled_task { + /** @var string Message prefix for mtrace */ + protected const MTRACE_MSG = 'Synced LDAP (Sync Plus) users'; /** * Return localised task name. @@ -48,11 +52,22 @@ public function get_name() { * @return boolean */ public function execute() { - global $CFG; if (is_enabled_auth('ldap_syncplus')) { + /** @var auth_plugin_ldap_syncplus $auth */ $auth = get_auth_plugin('ldap_syncplus'); - $auth->sync_users(true); + $count = 0; + $auth->sync_users_update_callback(function ($users, $updatekeys) use (&$count) { + $asynctask = new asynchronous_sync_task(); + $asynctask->set_custom_data([ + 'users' => $users, + 'updatekeys' => $updatekeys, + ]); + \core\task\manager::queue_adhoc_task($asynctask); + + $count++; + mtrace(sprintf(" %s (%d)", self::MTRACE_MSG, $count)); + sleep(1); + }); } } - } diff --git a/settings.php b/settings.php index 020bab3..21c9efe 100644 --- a/settings.php +++ b/settings.php @@ -303,6 +303,11 @@ new lang_string('auth_sync_suspended_key', 'auth'), new lang_string('auth_sync_suspended', 'auth'), 0 , $yesno)); + // Sync update users chunk size. + $settings->add(new admin_setting_configtext('auth_ldap_syncplus/sync_updateuserchunk', + new lang_string('sync_updateuserchunk_key', 'auth_ldap'), + new lang_string('sync_updateuserchunk', 'auth_ldap'), 1000, PARAM_INT)); + // NTLM SSO Header. $settings->add(new admin_setting_heading('auth_ldap_syncplus/ntlm', new lang_string('auth_ntlmsso', 'auth_ldap'), '')); diff --git a/tests/behat/auth_ldap_syncplus.feature b/tests/behat/auth_ldap_syncplus.feature index 1b593f3..bbc9d18 100644 --- a/tests/behat/auth_ldap_syncplus.feature +++ b/tests/behat/auth_ldap_syncplus.feature @@ -237,6 +237,7 @@ Feature: Checking that all LDAP (Sync Plus) specific settings are working And I should not see "User 01" in the "[data-region='report-user-list-wrapper']" "css_element" And I should see "Foo Bar" in the "[data-region='report-user-list-wrapper']" "css_element" And I run the scheduled task "\auth_ldap_syncplus\task\sync_task" + And I run all adhoc tasks And I reload the page Then I should see "User 01" in the "[data-region='report-user-list-wrapper']" "css_element" And I should not see "Foo Bar" in the "[data-region='report-user-list-wrapper']" "css_element" diff --git a/version.php b/version.php index fb56dd0..f8d2606 100644 --- a/version.php +++ b/version.php @@ -25,7 +25,7 @@ defined('MOODLE_INTERNAL') || die(); $plugin->component = 'auth_ldap_syncplus'; -$plugin->version = 2024060101; +$plugin->version = 2024060102; $plugin->release = 'v4.4-r1'; $plugin->requires = 2024042200; $plugin->supported = [404, 404];