From 4f2a893ac673e31c99fa49060ec63ce23be45fe5 Mon Sep 17 00:00:00 2001 From: "Shamiso.Jaravaza" <33659194+ssj365@users.noreply.github.com> Date: Tue, 26 Mar 2024 00:14:49 -0600 Subject: [PATCH] MDL-80225 mod_bigbluebuttonbn: Fix recordings --- .../classes/local/proxy/recording_proxy.php | 91 +++++++++---------- mod/bigbluebuttonbn/classes/recording.php | 13 ++- .../classes/task/upgrade_recordings_task.php | 3 +- .../classes/test/testcase_helper_trait.php | 3 +- .../cli/update_dismissed_recordings.php | 3 +- .../local/proxy/recording_proxy_test.php | 6 +- 6 files changed, 62 insertions(+), 57 deletions(-) diff --git a/mod/bigbluebuttonbn/classes/local/proxy/recording_proxy.php b/mod/bigbluebuttonbn/classes/local/proxy/recording_proxy.php index 777f277badd..77baf70ad16 100644 --- a/mod/bigbluebuttonbn/classes/local/proxy/recording_proxy.php +++ b/mod/bigbluebuttonbn/classes/local/proxy/recording_proxy.php @@ -131,8 +131,8 @@ class recording_proxy extends proxy_base { * @return null|array */ public static function fetch_recording(string $recordingid): ?array { - $data = self::fetch_recordings([$recordingid]); - + $result = self::fetch_recordings([$recordingid]); + $data = $result['recordings']; if (array_key_exists($recordingid, $data)) { return $data[$recordingid]; } @@ -173,24 +173,25 @@ class recording_proxy extends proxy_base { * We use a cache to store recording indexed by keyids/recordingID. * @param array $keyids list of recordingids * @return array (associative) with recordings indexed by recordID, each recording is a non sequential array - * and sorted by {@see recording_proxy::sort_recordings} + * and sorted by {@see recording_proxy::sort_recordings} and an array containing unfetched $keyids. */ public static function fetch_recordings(array $keyids = []): array { $recordings = []; // If $ids is empty return array() to prevent a getRecordings with meetingID and recordID set to ''. if (empty($keyids)) { - return $recordings; + return ['recordings' => $recordings, 'unfetchedids' => []]; } $cache = cache::make('mod_bigbluebuttonbn', 'recordings'); $currentfetchcache = cache::make('mod_bigbluebuttonbn', 'currentfetch'); $recordings = array_filter($cache->get_many($keyids)); $missingkeys = array_diff(array_values($keyids), array_keys($recordings)); - $recordings += self::do_fetch_recordings($missingkeys); + $result = self::do_fetch_recordings($missingkeys); + $recordings += $result['recordings']; $cache->set_many($recordings); $currentfetchcache->set_many(array_flip(array_keys($recordings))); - return $recordings; + return ['recordings' => $recordings, 'unfetchedids' => $result['unfetchedids']]; } /** @@ -198,17 +199,16 @@ class recording_proxy extends proxy_base { * * @param array $keyids list of meetingids * @return array (associative) with recordings indexed by recordID, each recording is a non sequential array - * and sorted by {@see recording_proxy::sort_recordings} + * and sorted by {@see recording_proxy::sort_recordings} and an array containing unfetched $keyids. */ public static function fetch_recording_by_meeting_id(array $keyids = []): array { $recordings = []; // If $ids is empty return array() to prevent a getRecordings with meetingID and recordID set to ''. if (empty($keyids)) { - return $recordings; + return ['recordings' => $recordings, 'unfetchedids' => []]; } - $recordings = self::do_fetch_recordings($keyids, 'meetingID'); - return $recordings; + return self::do_fetch_recordings($keyids, 'meetingID'); } /** @@ -217,66 +217,63 @@ class recording_proxy extends proxy_base { * @param array $keyids list of meetingids or recordingids * @param string $key the param name used for the BBB request (|meetingID) * @return array (associative) with recordings indexed by recordID, each recording is a non sequential array. - * and sorted {@see recording_proxy::sort_recordings} + * and sorted {@see recording_proxy::sort_recordings} and an array containing unfetched $keyids */ private static function do_fetch_recordings(array $keyids = [], string $key = 'recordID'): array { $recordings = []; + $unfetchedids = []; $pagesize = 25; while ($ids = array_splice($keyids, 0, $pagesize)) { - $fetchrecordings = self::fetch_recordings_page($ids, $key); - $recordings += $fetchrecordings; + $result = self::fetch_recordings_page($ids, $key); + if (!$result['success']) { + $unfetchedids = array_merge($unfetchedids, $ids); + continue; // Skip this page as the response is not valid but we will keep record of all unfetched ids. + } + $recordings += $result['recordings']; } // Sort recordings. - return self::sort_recordings($recordings); + return ['recordings' => self::sort_recordings($recordings), 'unfetchedids' => $unfetchedids]; } /** * Helper function to fetch a page of recordings from the remote server. * * @param array $ids * @param string $key - * @return array + * @return array an associative array containing recordings list and fetch success. */ private static function fetch_recordings_page(array $ids, $key = 'recordID'): array { // The getRecordings call is executed using a method GET (supported by all versions of BBB). $xml = self::fetch_endpoint_xml('getRecordings', [$key => implode(',', $ids), 'state' => 'any']); - if (!$xml) { - return []; - } - - if ($xml->returncode != 'SUCCESS') { - return []; - } - - if (!isset($xml->recordings)) { - return []; - } - - $recordings = []; - // If there were recordings already created. - foreach ($xml->recordings->recording as $recordingxml) { - $recording = self::parse_recording($recordingxml); - $recordings[$recording['recordID']] = $recording; - // Check if there are any child. - if (isset($recordingxml->breakoutRooms->breakoutRoom)) { - $breakoutrooms = []; - foreach ($recordingxml->breakoutRooms->breakoutRoom as $breakoutroom) { - $breakoutrooms[] = trim((string) $breakoutroom); - } - if ($breakoutrooms) { - $xml = self::fetch_endpoint_xml('getRecordings', ['recordID' => implode(',', $breakoutrooms)]); - if ($xml && $xml->returncode == 'SUCCESS' && isset($xml->recordings)) { - // If there were already created meetings. - foreach ($xml->recordings->recording as $subrecordingxml) { - $recording = self::parse_recording($subrecordingxml); - $recordings[$recording['recordID']] = $recording; + // Response from the server must be valid. + if ($xml && $xml->returncode == 'SUCCESS' && isset($xml->recordings)) { + $recordings = []; + // If there were recordings already created. + foreach ($xml->recordings->recording as $recordingxml) { + $recording = self::parse_recording($recordingxml); + $recordings[$recording['recordID']] = $recording; + // Check if there are any child. + if (isset($recordingxml->breakoutRooms->breakoutRoom)) { + $breakoutrooms = []; + foreach ($recordingxml->breakoutRooms->breakoutRoom as $breakoutroom) { + $breakoutrooms[] = trim((string) $breakoutroom); + } + if ($breakoutrooms) { + $xml = self::fetch_endpoint_xml('getRecordings', ['recordID' => implode(',', $breakoutrooms)]); + if ($xml && $xml->returncode == 'SUCCESS' && isset($xml->recordings)) { + // If there were already created meetings. + foreach ($xml->recordings->recording as $subrecordingxml) { + $recording = self::parse_recording($subrecordingxml); + $recordings[$recording['recordID']] = $recording; + } } } } } + return ['recordings' => $recordings, 'success' => true]; } - - return $recordings; + debugging('getRecordings API call failed. No recording data retrieved from BBB server for ids: ' . implode(', ', $ids)); + return ['recordings' => [], 'success' => false]; } /** diff --git a/mod/bigbluebuttonbn/classes/recording.php b/mod/bigbluebuttonbn/classes/recording.php index 34bcb7556c0..9c3d77b1eb8 100644 --- a/mod/bigbluebuttonbn/classes/recording.php +++ b/mod/bigbluebuttonbn/classes/recording.php @@ -713,14 +713,16 @@ class recording extends persistent { }, $recordings)); // Fetch all metadata for these recordings. - $metadatas = recording_proxy::fetch_recordings($recordingids); + $result = recording_proxy::fetch_recordings($recordingids); + $metadatas = $result['recordings']; + $failedids = $result['unfetchedids']; // Return the instances. - return array_filter(array_map(function ($recording) use ($metadatas, $withindays) { + return array_filter(array_map(function ($recording) use ($metadatas, $withindays, $failedids) { // Filter out if no metadata was fetched. if (!array_key_exists($recording->recordingid, $metadatas)) { - // Mark it as dismissed if it is older than 30 days. - if ($withindays > $recording->timecreated) { + // If the recording was successfully fetched, mark it as dismissed if it is older than 30 days. + if (!in_array($recording->recordingid, $failedids) && $withindays > $recording->timecreated) { $recording = new self(0, $recording, null); $recording->set_status(self::RECORDING_STATUS_DISMISSED); } @@ -810,7 +812,8 @@ class recording extends persistent { // Fetch all metadata for these recordings. mtrace("=> Fetching recording metadata from server"); - $metadatas = recording_proxy::fetch_recordings($recordingids); + $result = recording_proxy::fetch_recordings($recordingids); + $metadatas = $result['recordings']; $foundcount = 0; foreach ($metadatas as $recordingid => $metadata) { diff --git a/mod/bigbluebuttonbn/classes/task/upgrade_recordings_task.php b/mod/bigbluebuttonbn/classes/task/upgrade_recordings_task.php index c2b06bf0145..e84eca42c38 100644 --- a/mod/bigbluebuttonbn/classes/task/upgrade_recordings_task.php +++ b/mod/bigbluebuttonbn/classes/task/upgrade_recordings_task.php @@ -74,7 +74,8 @@ class upgrade_recordings_task extends adhoc_task { return false; } // Retrieve recordings from the servers for this meeting. - $recordings = recording_proxy::fetch_recording_by_meeting_id([$meetingid]); + $result = recording_proxy::fetch_recording_by_meeting_id([$meetingid]); + $recordings = $result['recordings']; // Sort recordings by meetingId, then startTime. uasort($recordings, function($a, $b) { return $b['startTime'] - $a['startTime']; diff --git a/mod/bigbluebuttonbn/classes/test/testcase_helper_trait.php b/mod/bigbluebuttonbn/classes/test/testcase_helper_trait.php index df44bc74669..9db5409b981 100644 --- a/mod/bigbluebuttonbn/classes/test/testcase_helper_trait.php +++ b/mod/bigbluebuttonbn/classes/test/testcase_helper_trait.php @@ -313,7 +313,8 @@ trait testcase_helper_trait { $baselogdata['meetingid'] = $instance->get_meeting_id(); if ($importedrecordings) { // Fetch the data. - $data = recording_proxy::fetch_recordings([$recording->recordingid]); + $result = recording_proxy::fetch_recordings([$recording->recordingid]); + $data = $result['recordings']; $data = end($data); if ($data) { $metaonly = array_filter($data, function($key) { diff --git a/mod/bigbluebuttonbn/cli/update_dismissed_recordings.php b/mod/bigbluebuttonbn/cli/update_dismissed_recordings.php index 865c8109bda..329768f7c55 100644 --- a/mod/bigbluebuttonbn/cli/update_dismissed_recordings.php +++ b/mod/bigbluebuttonbn/cli/update_dismissed_recordings.php @@ -99,7 +99,8 @@ foreach ($bbcms as $bbcm) { $recordingkeys = array_map(function($rec) { return $rec->get('recordingid'); }, $recordings); - $recordingmeta = \mod_bigbluebuttonbn\local\proxy\recording_proxy::fetch_recordings($recordingkeys); + $result = \mod_bigbluebuttonbn\local\proxy\recording_proxy::fetch_recordings($recordingkeys); + $recordingmeta = $result['recordings']; if (empty($recordings)) { cli_writeln("\t->No recordings found ..."); } else { diff --git a/mod/bigbluebuttonbn/tests/local/proxy/recording_proxy_test.php b/mod/bigbluebuttonbn/tests/local/proxy/recording_proxy_test.php index e50dd4a1c0e..2d9878dbd94 100644 --- a/mod/bigbluebuttonbn/tests/local/proxy/recording_proxy_test.php +++ b/mod/bigbluebuttonbn/tests/local/proxy/recording_proxy_test.php @@ -46,7 +46,8 @@ class recording_proxy_test extends \advanced_testcase { $recordingsid = array_map(function ($r) { return $r->recordingid; }, $recordings); - $recordings = recording_proxy::fetch_recordings($recordingsid); + $result = recording_proxy::fetch_recordings($recordingsid); + $recordings = $result['recordings']; $this->assertCount(2, $recordings); } @@ -90,7 +91,8 @@ class recording_proxy_test extends \advanced_testcase { $recordingsid = array_map(function ($r) { return $r->recordingid; }, $recordings); - $recordings = recording_proxy::fetch_recordings([$recordingsid[0]]); + $result = recording_proxy::fetch_recordings([$recordingsid[0]]); + $recordings = $result['recordings']; $this->assertCount(3, $recordings); } } -- 2.35.1.windows.2