Upgrade: Adopt changes from MDL-73703 and break the sync task user up…

…dates into chunks, resolves #36
moodle-an-hochschulen · Nov 12, 2024 · 411c09b · 411c09b
1 parent 2675805
commit 411c09b
Show file tree

Hide file tree

Showing 7 changed files with 143 additions and 34 deletions.
diff --git 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, resolves #36
 * 2024-11-12 - Improvement: Change location of event observer code, resolves #26
 
 ### v4.4-r1

diff --git 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
@@ -0,0 +1,61 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Auth plugin "LDAP SyncPlus" - Ad-hoc task definition
+ *
+ * @package    auth_ldap_syncplus
+ * @copyright  2024 Alexander Bias, ssystems GmbH <[email protected]>
+ *             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 <[email protected]>
+ *             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
@@ -18,7 +18,8 @@
  * Auth plugin "LDAP SyncPlus" - Task definition
  *
  * @package    auth_ldap_syncplus
- * @copyright  2014 Alexander Bias, Ulm University <[email protected]>
+ * @copyright  2024 Alexander Bias, ssystems GmbH <[email protected]>
+ *             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 <[email protected]>
+ * @copyright  2024 Alexander Bias, ssystems GmbH <[email protected]>
+ *             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
@@ -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
@@ -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
@@ -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];