Skip to content

Commit 13c5e8e

Browse files
weilai-irlweilai-irl
committed
Move the logic to clean up invalid config log entries from upgrade script to tasks
1 parent 21db71e commit 13c5e8e

File tree

6 files changed

+233
-24
lines changed

6 files changed

+233
-24
lines changed

‎local/o365/classes/task/checkinvalidconfiglog.php‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
<?php
2+
// This file is part of Moodle - http://moodle.org/
3+
//
4+
// Moodle is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU General Public License as published by
6+
// the Free Software Foundation, either version 3 of the License, or
7+
// (at your option) any later version.
8+
//
9+
// Moodle is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU General Public License for more details.
13+
//
14+
// You should have received a copy of the GNU General Public License
15+
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
16+
17+
/**
18+
* Scheduled task to check for invalid config log records and queue adhoc task if found.
19+
*
20+
* @package local_o365
21+
* @author Lai Wei <lai.wei@enovation.ie>
22+
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
23+
* @copyright (C) 2014 onwards Microsoft, Inc. (http://microsoft.com/)
24+
*/
25+
26+
namespace local_o365\task;
27+
28+
use core\task\manager;
29+
use core\task\scheduled_task;
30+
31+
/**
32+
* Scheduled task to check for invalid config log records and queue cleanup if needed.
33+
*/
34+
class checkinvalidconfiglog extends scheduled_task {
35+
/**
36+
* Get a descriptive name for this task (shown to admins).
37+
*
38+
* @return string
39+
*/
40+
public function get_name(): string {
41+
return get_string('task_checkinvalidconfiglog', 'local_o365');
42+
}
43+
44+
/**
45+
* Execute the task.
46+
*
47+
* @return bool
48+
*/
49+
public function execute(): bool {
50+
global $DB;
51+
52+
$hasinvalidrecords = false;
53+
54+
// Check if there are any invalid records to process.
55+
foreach (deleteinvalidconfiglog::CONFIG_NAMES as $configname) {
56+
$count = $DB->count_records('config_log', ['plugin' => 'local_o365', 'name' => $configname]);
57+
if ($count > 0) {
58+
mtrace("Found {$count} invalid config_log records for {$configname}");
59+
$hasinvalidrecords = true;
60+
}
61+
}
62+
63+
// If there are invalid records, check if there's already a queued or running task.
64+
if ($hasinvalidrecords) {
65+
// Check if there's already a queued adhoc task for deletion.
66+
$adhoctasks = manager::get_adhoc_tasks('\local_o365\task\deleteinvalidconfiglog');
67+
$hastask = false;
68+
foreach ($adhoctasks as $adhoctask) {
69+
if ($adhoctask->get_fail_delay() == 0) {
70+
// Task is not in a failed state.
71+
$hastask = true;
72+
break;
73+
}
74+
}
75+
76+
if (!$hastask) {
77+
mtrace("Queueing adhoc task to delete invalid config log records...");
78+
$task = new deleteinvalidconfiglog();
79+
// Set next run time to ensure it runs in the next cron cycle.
80+
$task->set_next_run_time(time() + 60);
81+
manager::queue_adhoc_task($task);
82+
} else {
83+
mtrace("Adhoc task already queued or running, skipping.");
84+
}
85+
} else {
86+
mtrace("No invalid config log records found.");
87+
}
88+
89+
return true;
90+
}
91+
}

‎local/o365/classes/task/deleteinvalidconfiglog.php‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
<?php
2+
// This file is part of Moodle - http://moodle.org/
3+
//
4+
// Moodle is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU General Public License as published by
6+
// the Free Software Foundation, either version 3 of the License, or
7+
// (at your option) any later version.
8+
//
9+
// Moodle is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU General Public License for more details.
13+
//
14+
// You should have received a copy of the GNU General Public License
15+
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
16+
17+
/**
18+
* An adhoc task to delete invalid config log records in batches.
19+
*
20+
* @package local_o365
21+
* @author Lai Wei <lai.wei@enovation.ie>
22+
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
23+
* @copyright (C) 2014 onwards Microsoft, Inc. (http://microsoft.com/)
24+
*/
25+
26+
namespace local_o365\task;
27+
28+
use core\task\adhoc_task;
29+
use core\task\manager;
30+
31+
/**
32+
* Adhoc task for deleting invalid config log records in batches.
33+
*/
34+
class deleteinvalidconfiglog extends adhoc_task {
35+
/** @var int Number of records to process per batch */
36+
const BATCH_SIZE = 10000;
37+
38+
/** @var array List of config names that should not have been logged */
39+
const CONFIG_NAMES = [
40+
'apptokens',
41+
'teamscacheupdated',
42+
'calsyncinlastrun',
43+
'task_usersync_lastdeltatoken',
44+
'task_usersync_lastdelete',
45+
'cal_site_lastsync',
46+
'cal_course_lastsync',
47+
'cal_user_lastsync',
48+
];
49+
50+
/**
51+
* Execute the task.
52+
*
53+
* @return bool
54+
*/
55+
public function execute(): bool {
56+
global $DB;
57+
58+
$hasmore = false;
59+
60+
foreach (self::CONFIG_NAMES as $configname) {
61+
mtrace("Processing config name: {$configname}");
62+
63+
// Count total records for this config name.
64+
$totalcount = $DB->count_records('config_log', ['plugin' => 'local_o365', 'name' => $configname]);
65+
66+
if ($totalcount > 0) {
67+
mtrace("... Found {$totalcount} config_log records for {$configname}");
68+
69+
// Use recordset for memory efficiency, process up to BATCH_SIZE records.
70+
$configlogids = [];
71+
$recordset = $DB->get_recordset_select(
72+
'config_log',
73+
'plugin = :plugin AND name = :name',
74+
['plugin' => 'local_o365', 'name' => $configname],
75+
'id ASC',
76+
'id'
77+
);
78+
79+
$count = 0;
80+
foreach ($recordset as $record) {
81+
$configlogids[] = $record->id;
82+
$count++;
83+
if ($count >= self::BATCH_SIZE) {
84+
break;
85+
}
86+
}
87+
$recordset->close();
88+
89+
if (!empty($configlogids)) {
90+
// Delete corresponding logstore_standard_log records.
91+
[$insql, $inparams] = $DB->get_in_or_equal($configlogids, SQL_PARAMS_NAMED);
92+
$params = array_merge(['eventname' => '\core\event\config_log_created'], $inparams);
93+
$DB->delete_records_select('logstore_standard_log', "eventname = :eventname AND objectid $insql", $params);
94+
95+
// Delete config_log records.
96+
$DB->delete_records_list('config_log', 'id', $configlogids);
97+
98+
mtrace("... Deleted {$count} records for {$configname}");
99+
100+
// Check if there are more records to process.
101+
$remaining = $DB->count_records('config_log', ['plugin' => 'local_o365', 'name' => $configname]);
102+
if ($remaining > 0) {
103+
mtrace("... {$remaining} records remaining for {$configname}");
104+
$hasmore = true;
105+
}
106+
}
107+
}
108+
}
109+
110+
// If there are more records to process, queue another ad-hoc task.
111+
if ($hasmore) {
112+
mtrace("Queueing another ad-hoc task to continue processing...");
113+
$task = new self();
114+
// Set next run time to ensure it runs in the next cron cycle, not immediately.
115+
$task->set_next_run_time(time() + 60);
116+
manager::queue_adhoc_task($task);
117+
} else {
118+
mtrace("All invalid config log records have been deleted.");
119+
}
120+
121+
return true;
122+
}
123+
}

‎local/o365/db/tasks.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,4 +107,13 @@
107107
'dayofweek' => '*',
108108
'month' => '*',
109109
],
110+
[
111+
'classname' => 'local_o365\task\checkinvalidconfiglog',
112+
'blocking' => 0,
113+
'minute' => '0',
114+
'hour' => '2',
115+
'day' => '*',
116+
'dayofweek' => '*',
117+
'month' => '*',
118+
],
110119
];

‎local/o365/db/upgrade.php‎

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1305,29 +1305,14 @@ function xmldb_local_o365_upgrade($oldversion) {
13051305
}
13061306

13071307
if ($oldversion < 2022112851) {
1308-
// Delete config log entries created by mistake.
1309-
$confignames = ['apptokens', 'teamscacheupdated', 'calsyncinlastrun', 'task_usersync_lastdeltatoken',
1310-
'task_usersync_lastdelete', 'cal_site_lastsync', 'cal_course_lastsync', 'cal_user_lastsync'];
1311-
1312-
foreach ($confignames as $configname) {
1313-
// Delete logstore_standard_log records.
1314-
// First, get the config_log IDs to avoid slow subquery in DELETE.
1315-
$configlogids = $DB->get_fieldset_select('config_log', 'id', 'plugin = :plugin AND name = :name',
1316-
['plugin' => 'local_o365', 'name' => $configname]);
1317-
1318-
if (!empty($configlogids)) {
1319-
// Process in chunks to avoid PostgreSQL parameter limits.
1320-
$chunks = array_chunk($configlogids, 10000);
1321-
foreach ($chunks as $chunk) {
1322-
list($insql, $inparams) = $DB->get_in_or_equal($chunk, SQL_PARAMS_NAMED);
1323-
$params = array_merge(['eventname' => '\core\event\config_log_created'], $inparams);
1324-
$DB->delete_records_select('logstore_standard_log', "eventname = :eventname AND objectid $insql", $params);
1325-
}
1326-
}
1327-
1328-
// Delete config_log records.
1329-
$DB->delete_records('config_log', ['plugin' => 'local_o365', 'name' => $configname]);
1330-
}
1308+
// Queue adhoc task to delete config log entries created by mistake.
1309+
// This is done via adhoc task to avoid timeout issues when there are many records to delete.
1310+
$task = new \local_o365\task\deleteinvalidconfiglog();
1311+
// Set next run time to ensure it runs in the next cron cycle, not during upgrade.
1312+
$task->set_next_run_time(time() + 60);
1313+
\core\task\manager::queue_adhoc_task($task);
1314+
1315+
mtrace('Queued adhoc task to delete invalid config log records.');
13311316

13321317
// O365 savepoint reached.
13331318
upgrade_plugin_savepoint(true, 2022112851, 'local', 'o365');

‎local/o365/lang/en/local_o365.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,6 +899,7 @@
899899
$string['task_syncusers'] = 'Sync users from Microsoft Entra ID';
900900
$string['task_processmatchqueue'] = 'Process Match Queue';
901901
$string['task_notifysecretexpiry'] = 'Notify site admin about Microsoft Entra ID app secret expiry';
902+
$string['task_checkinvalidconfiglog'] = 'Check and clean up invalid config log records';
902903
$string['task_processmatchqueue_err_museralreadymatched'] = 'Moodle user is already matched to a Microsoft 365 user.';
903904
$string['task_processmatchqueue_err_museralreadyo365'] = 'Moodle user is already connected to Microsoft 365.';
904905
$string['task_processmatchqueue_err_nomuser'] = 'No Moodle user found with this username.';

‎local/o365/version.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626

2727
defined('MOODLE_INTERNAL') || die();
2828

29-
$plugin->version = 2022112855;
29+
$plugin->version = 2022112855.1;
3030
$plugin->requires = 2022112800;
3131
$plugin->release = '4.1.12';
3232
$plugin->component = 'local_o365';

0 commit comments

Comments
 (0)