Skip to content

Commit 6992256

Browse files
committed
course sync and cohort sync task optimisation:
- Consolidate local_o365_teams_cache into local_o365_groups_cache for course sync and cohort sync tasks. - Extend local_o365_groups_cache with has_team, url, locked, team_details_last_attempted, lock_status_last_checked fields and supporting indexes. - Drop local_o365_teams_cache table. - Rewrite utils::update_groups_cache() to handle both groups and teams in one pass, with 5-minute rate limiting, lazy team-detail fetching (URL once, lock status every 7 days), and bulk DB operations. - Replace two separate cache-update calls in the course sync task with a single unified call; fix cohort sync task to proceed on rate-limit skip rather than abort. - Update all table references in tasks, admin UI, and deprecated update_teams_cache() to use the unified cache table.
1 parent 56227e9 commit 6992256

File tree

11 files changed

+544
-106
lines changed

11 files changed

+544
-106
lines changed

local/o365/classes/feature/cohortsync/main.php

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -217,22 +217,20 @@ public function fetch_cohorts(): void {
217217
/**
218218
* Update Groups cache.
219219
*
220-
* @return bool
220+
* @return bool|null True if the cache was updated, null if skipped (rate limit), false on error.
221221
*/
222-
public function update_groups_cache(): bool {
222+
public function update_groups_cache(): ?bool {
223223
global $DB;
224224

225225
try {
226-
if (utils::update_groups_cache($this->graphclient, 1)) {
226+
$result = utils::update_groups_cache($this->graphclient, 1);
227+
if ($result === true) {
227228
$sql = 'SELECT *
228229
FROM {local_o365_groups_cache}
229230
WHERE not_found_since = 0';
230231
$this->grouplist = $DB->get_records_sql($sql);
231-
232-
return true;
233-
} else {
234-
return false;
235232
}
233+
return $result;
236234
} catch (moodle_exception $e) {
237235
utils::debug('Exception in update_groups_cache: ' . $e->getMessage(), __METHOD__, $e);
238236
return false;

local/o365/classes/feature/coursesync/main.php

Lines changed: 57 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,6 +1118,10 @@ private function restore_group(int $objectrecid, string $objectid, array $object
11181118
/**
11191119
* Update Teams cache.
11201120
*
1121+
* @deprecated Use \local_o365\utils::update_groups_cache() instead, which handles both
1122+
* groups and teams in the unified cache table. This function is no longer
1123+
* called by any production code and will be removed in a future version.
1124+
*
11211125
* @return bool
11221126
*/
11231127
public function update_teams_cache(): bool {
@@ -1152,10 +1156,10 @@ public function update_teams_cache(): bool {
11521156
return false;
11531157
}
11541158

1155-
// Build existing teams records cache.
1159+
// Build existing teams records cache from unified groups cache table.
11561160
$this->mtrace('Building existing teams cache records', 1);
11571161
// Use recordset instead of get_records to reduce memory usage.
1158-
$existingcacherecordset = $DB->get_recordset('local_o365_teams_cache');
1162+
$existingcacherecordset = $DB->get_recordset('local_o365_groups_cache', ['has_team' => 1]);
11591163
$existingcachebyoid = [];
11601164
foreach ($existingcacherecordset as $existingcacherecord) {
11611165
$existingcachebyoid[$existingcacherecord->objectid] = $existingcacherecord;
@@ -1168,10 +1172,12 @@ public function update_teams_cache(): bool {
11681172
foreach ($teams as $team) {
11691173
if (array_key_exists($team['id'], $existingcachebyoid)) {
11701174
// Update existing cache record.
1175+
$lockstatuschecked = false;
11711176
if (!$existingcachebyoid[$team['id']]->locked) {
11721177
// Need to update lock status.
11731178
try {
11741179
[$rawteam, $teamurl, $lockstatus] = $this->graphclient->get_team($team['id']);
1180+
$lockstatuschecked = true;
11751181
} catch (moodle_exception $e) {
11761182
continue;
11771183
}
@@ -1183,7 +1189,10 @@ public function update_teams_cache(): bool {
11831189
$cacherecord->name = $team['displayName'];
11841190
$cacherecord->description = $team['description'];
11851191
$cacherecord->locked = $lockstatus;
1186-
$DB->update_record('local_o365_teams_cache', $cacherecord);
1192+
if ($lockstatuschecked) {
1193+
$cacherecord->lock_status_last_checked = time();
1194+
}
1195+
$DB->update_record('local_o365_groups_cache', $cacherecord);
11871196

11881197
unset($existingcachebyoid[$team['id']]);
11891198
} else {
@@ -1194,20 +1203,47 @@ public function update_teams_cache(): bool {
11941203
continue;
11951204
}
11961205

1197-
// Create new cache record.
1198-
$cacherecord = new stdClass();
1199-
$cacherecord->objectid = $team['id'];
1200-
$cacherecord->name = $team['displayName'];
1201-
$cacherecord->description = $team['description'];
1202-
$cacherecord->url = $teamurl;
1203-
$cacherecord->locked = $lockstatus;
1204-
$DB->insert_record('local_o365_teams_cache', $cacherecord);
1206+
// Upsert: a group-only row (has_team=0) may already exist for this objectid.
1207+
// Inserting a second row would create duplicates and break get_record() calls.
1208+
$cacherecord = $DB->get_record('local_o365_groups_cache', ['objectid' => $team['id']]);
1209+
if ($cacherecord) {
1210+
// Promote the existing group row to a team row.
1211+
$cacherecord->name = $team['displayName'];
1212+
$cacherecord->description = $team['description'];
1213+
$cacherecord->has_team = 1;
1214+
$cacherecord->url = $teamurl;
1215+
$cacherecord->locked = $lockstatus;
1216+
$cacherecord->not_found_since = 0;
1217+
$cacherecord->team_details_last_attempted = time();
1218+
$cacherecord->lock_status_last_checked = time();
1219+
$DB->update_record('local_o365_groups_cache', $cacherecord);
1220+
} else {
1221+
$cacherecord = new stdClass();
1222+
$cacherecord->objectid = $team['id'];
1223+
$cacherecord->name = $team['displayName'];
1224+
$cacherecord->description = $team['description'];
1225+
$cacherecord->has_team = 1;
1226+
$cacherecord->url = $teamurl;
1227+
$cacherecord->locked = $lockstatus;
1228+
$cacherecord->not_found_since = 0;
1229+
$cacherecord->team_details_last_attempted = 0;
1230+
$cacherecord->lock_status_last_checked = 0;
1231+
$DB->insert_record('local_o365_groups_cache', $cacherecord);
1232+
}
12051233
}
12061234
}
12071235

1208-
$this->mtrace('Deleting old teams cache records', 1);
1236+
// Demote groups that are no longer teams: set has_team=0 and clear team-only fields.
1237+
// Deleting the row would cause data loss if the underlying group still exists;
1238+
// this matches the demotion behaviour in utils::update_groups_cache().
1239+
$this->mtrace('Demoting old teams cache records to group-only', 1);
12091240
foreach ($existingcachebyoid as $oldcacherecord) {
1210-
$DB->delete_records('local_o365_teams_cache', ['id' => $oldcacherecord->id]);
1241+
$oldcacherecord->has_team = 0;
1242+
$oldcacherecord->url = null;
1243+
$oldcacherecord->locked = null;
1244+
$oldcacherecord->team_details_last_attempted = 0;
1245+
$oldcacherecord->lock_status_last_checked = 0;
1246+
$DB->update_record('local_o365_groups_cache', $oldcacherecord);
12111247
}
12121248

12131249
$this->mtrace('Finished updating teams cache.');
@@ -1222,15 +1258,17 @@ public function update_teams_cache(): bool {
12221258

12231259
/**
12241260
* Cleanup Teams connections records.
1225-
* This function will delete all Teams connection records with object IDs not found in the Teams cache.
1226-
* This function should only be called after Teams cache is updated - no cache update will be performed here.
1261+
* This function will delete all Teams connection records with object IDs not found in the cache.
1262+
* Teams are identified in the cache by has_team=1.
1263+
* This function should only be called after the cache is updated - no cache update will be performed here.
12271264
*
12281265
* @return void
12291266
*/
12301267
public function cleanup_teams_connections() {
12311268
global $DB;
12321269

1233-
$teamobjectids = $DB->get_fieldset_select('local_o365_teams_cache', 'objectid', '');
1270+
// Get all team object IDs from the unified cache (has_team=1).
1271+
$teamobjectids = $DB->get_fieldset_select('local_o365_groups_cache', 'objectid', 'has_team = 1');
12341272

12351273
$this->mtrace('Clean up teams connection records...');
12361274
if ($teamobjectids) {
@@ -1679,12 +1717,12 @@ public function get_all_group_ids(): array {
16791717
public function is_team_locked(string $groupobjectid) {
16801718
global $DB;
16811719

1682-
if ($teamcacherecord = $DB->get_record('local_o365_teams_cache', ['objectid' => $groupobjectid])) {
1720+
if ($teamcacherecord = $DB->get_record('local_o365_groups_cache', ['objectid' => $groupobjectid, 'has_team' => 1])) {
16831721
switch ($teamcacherecord->locked) {
16841722
case TEAM_LOCKED_STATUS_UNKNOWN:
16851723
try {
16861724
[$team, $teamurl, $lockstatus] = $this->graphclient->get_team($groupobjectid);
1687-
$DB->set_field('local_o365_teams_cache', 'locked', $lockstatus, ['objectid' => $groupobjectid]);
1725+
$DB->set_field('local_o365_groups_cache', 'locked', $lockstatus, ['objectid' => $groupobjectid]);
16881726
} catch (moodle_exception $e) {
16891727
$lockstatus = TEAM_UNLOCKED;
16901728
}
@@ -1693,7 +1731,7 @@ public function is_team_locked(string $groupobjectid) {
16931731
try {
16941732
[$team, $teamurl, $lockstatus] = $this->graphclient->get_team($groupobjectid);
16951733
if ($lockstatus == TEAM_UNLOCKED) {
1696-
$DB->set_field('local_o365_teams_cache', 'locked', $lockstatus, ['objectid' => $groupobjectid]);
1734+
$DB->set_field('local_o365_groups_cache', 'locked', $lockstatus, ['objectid' => $groupobjectid]);
16971735
}
16981736
} catch (moodle_exception $e) {
16991737
$lockstatus = TEAM_UNLOCKED;

local/o365/classes/feature/coursesync/utils.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ public static function get_matching_team_options(string $currentoid = ''): array
293293

294294
$matchedoids = $DB->get_fieldset_select('local_o365_objects', 'objectid', 'type = ? AND subtype = ?', ['group', 'course']);
295295

296-
$teamcacherecords = $DB->get_records('local_o365_teams_cache');
296+
$teamcacherecords = $DB->get_records('local_o365_groups_cache', ['has_team' => 1]);
297297
foreach ($teamcacherecords as $key => $teamcacherecord) {
298298
if ($teamcacherecord->objectid == $currentoid || !in_array($teamcacherecord->objectid, $matchedoids)) {
299299
if (!array_key_exists($teamcacherecord->name, $teamnamecache)) {

local/o365/classes/page/acp.php

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,6 +1008,8 @@ public function mode_teamconnections() {
10081008
['moodleid' => $course->id, 'type' => 'group', 'subtype' => 'course']
10091009
)
10101010
) {
1011+
$teamscachedata = ['objectid' => $grouprecord->objectid, 'has_team' => 1];
1012+
10111013
if (
10121014
$DB->record_exists(
10131015
'local_o365_objects',
@@ -1019,7 +1021,7 @@ public function mode_teamconnections() {
10191021
)
10201022
) {
10211023
// Connected to both group and team.
1022-
if ($teamscache = $DB->get_record('local_o365_teams_cache', ['objectid' => $grouprecord->objectid])) {
1024+
if ($teamscache = $DB->get_record('local_o365_groups_cache', $teamscachedata)) {
10231025
// Team record can be found in cache.
10241026
$existingconnection = html_writer::link($teamscache->url, $teamscache->name);
10251027
if (
@@ -1057,7 +1059,7 @@ public function mode_teamconnections() {
10571059
$connectlabel = get_string('acp_teamconnections_table_connect', 'local_o365');
10581060

10591061
$actions = [html_writer::link($connecturl, $connectlabel)];
1060-
} else if ($teamscache = $DB->get_record('local_o365_teams_cache', ['objectid' => $grouprecord->objectid])) {
1062+
} else if ($teamscache = $DB->get_record('local_o365_groups_cache', $teamscachedata)) {
10611063
// Connect the course with the team.
10621064
$teamobjectrecord = ['type' => 'group', 'subtype' => 'courseteam', 'objectid' => $teamscache->objectid,
10631065
'moodleid' => $course->id, 'o365name' => $teamscache->name, 'timecreated' => time(),
@@ -1176,8 +1178,9 @@ public function mode_teamconnections_update_cache() {
11761178
confirm_sesskey();
11771179

11781180
$graphclient = \local_o365\feature\coursesync\utils::get_graphclient();
1179-
$coursesync = new main($graphclient);
1180-
$coursesync->update_teams_cache();
1181+
// Pass forceupdate=true so an explicit admin request is never silently skipped
1182+
// by the 5-minute rate limit that applies to automated task runs.
1183+
\local_o365\utils::update_groups_cache($graphclient, 0, true);
11811184

11821185
$redirecturl = new moodle_url('/local/o365/acp.php', ['mode' => 'teamconnections']);
11831186
redirect($redirecturl, get_string('acp_teamconnections_teams_cache_updated', 'local_o365'));
@@ -1230,7 +1233,7 @@ public function mode_teamconnections_connect() {
12301233
redirect($redirecturl);
12311234
}
12321235

1233-
if (!$teamcacherecord = $DB->get_record('local_o365_teams_cache', ['id' => $teamid])) {
1236+
if (!$teamcacherecord = $DB->get_record('local_o365_groups_cache', ['id' => $teamid, 'has_team' => 1])) {
12341237
throw new moodle_exception('acp_teamconnections_exception_invalid_team_id', 'local_o365', $redirecturl);
12351238
} else if (
12361239
$DB->record_exists(
@@ -1377,7 +1380,7 @@ public function mode_teamconnections_update() {
13771380
redirect($redirecturl);
13781381
}
13791382

1380-
if (!$teamcacherecord = $DB->get_record('local_o365_teams_cache', ['id' => $teamid])) {
1383+
if (!$teamcacherecord = $DB->get_record('local_o365_groups_cache', ['id' => $teamid, 'has_team' => 1])) {
13811384
throw new moodle_exception('acp_teamconnections_exception_invalid_team_id', 'local_o365', $redirecturl);
13821385
} else if (
13831386
$teamobjectrecord = $DB->get_record(

local/o365/classes/task/cohortsync.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,14 @@ public function execute(): bool {
8181
*/
8282
private function execute_sync(main $cohortsync): void {
8383
// First, update the group cache, and delete any groups that no longer exist.
84+
// A null result means the update was rate-limited; proceed using the existing cache
85+
// but skip cleanup (which depends on a fresh not-found list from the API).
8486
try {
85-
if ($cohortsync->update_groups_cache()) {
87+
$cacheupdateresult = $cohortsync->update_groups_cache();
88+
if ($cacheupdateresult === true) {
8689
utils::clean_up_not_found_groups();
90+
} else if ($cacheupdateresult === null) {
91+
utils::mtrace("Groups cache update skipped (rate limit). Proceeding with existing cache.", 1);
8792
} else {
8893
utils::mtrace("Failed to update groups cache. Exiting.", 1);
8994
return;
@@ -99,7 +104,7 @@ private function execute_sync(main $cohortsync): void {
99104
utils::mtrace("Found " . count($grouplist) . " groups.", 2);
100105
$grouplistbyoid = [];
101106
foreach ($grouplist as $group) {
102-
$grouplistbyoid[$group->objectid] = $group;
107+
$grouplistbyoid[$group['id']] = $group;
103108
}
104109

105110
$mappings = $cohortsync->get_mappings();

local/o365/classes/task/coursesync.php

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -79,28 +79,21 @@ public function execute() {
7979
$coursesync = new main($graphclient, true);
8080
$coursesync->sync_courses();
8181

82-
// Wrap update_teams_cache in try-catch to handle API failures gracefully.
82+
// Update the unified groups and teams cache.
83+
// Returns true = cache updated, null = skipped (rate limit), false = error.
84+
// Cleanup is only safe to run when the cache was actually refreshed (true).
8385
try {
84-
if ($coursesync->update_teams_cache()) {
86+
if (utils::update_groups_cache($graphclient, 1) === true) {
87+
// Cleanup orphaned team connections and save not found groups.
8588
$coursesync->cleanup_teams_connections();
86-
}
87-
} catch (Exception $e) {
88-
mtrace('Error updating teams cache: ' . $e->getMessage());
89-
utils::debug('Exception in update_teams_cache: ' . $e->getMessage(), __METHOD__, $e);
90-
// Continue with other operations even if teams cache update fails.
91-
}
92-
93-
$coursesync->cleanup_course_connection_records();
94-
95-
// Update the groups cache and save any not found groups.
96-
try {
97-
if (utils::update_groups_cache($graphclient, 1)) {
9889
$coursesync->save_not_found_groups();
9990
utils::clean_up_not_found_groups();
10091
}
10192
} catch (Exception $e) {
102-
mtrace('Error updating groups cache: ' . $e->getMessage());
93+
mtrace('Error updating groups/teams cache: ' . $e->getMessage());
10394
utils::debug('Exception in update_groups_cache: ' . $e->getMessage(), __METHOD__, $e);
10495
}
96+
97+
$coursesync->cleanup_course_connection_records();
10598
}
10699
}

local/o365/classes/task/processcourserequestapproval.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,11 @@ public function execute(): bool {
9696
utils::set_course_sync_enabled($coursedata->courseid);
9797

9898
// Update teams cache record status.
99-
if ($teamcacherecord = $DB->get_record('local_o365_teams_cache', ['objectid' => $courserequestdata->teamoid])) {
99+
$teamcachefilter = ['objectid' => $courserequestdata->teamoid, 'has_team' => 1];
100+
if ($teamcacherecord = $DB->get_record('local_o365_groups_cache', $teamcachefilter)) {
100101
if ($teamcacherecord->locked != TEAM_LOCKED) {
101102
$teamcacherecord->locked = TEAM_LOCKED;
102-
$DB->update_record('local_o365_teams_cache', $teamcacherecord);
103+
$DB->update_record('local_o365_groups_cache', $teamcacherecord);
103104
}
104105
}
105106

0 commit comments

Comments
 (0)