diff --git a/classes/local/proxy/recording_proxy.php b/classes/local/proxy/recording_proxy.php index 30a526a9..2f1652db 100644 --- a/classes/local/proxy/recording_proxy.php +++ b/classes/local/proxy/recording_proxy.php @@ -131,8 +131,8 @@ public static function update_recording(string $recordid, array $params): bool { * @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]; } @@ -179,17 +179,18 @@ public static function fetch_recordings(array $keyids = []): array { // 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']]; } /** @@ -203,10 +204,9 @@ public static function fetch_recording_by_meeting_id(array $keyids = []): array // 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'); } /** @@ -218,61 +218,58 @@ public static function fetch_recording_by_meeting_id(array $keyids = []): array */ 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']; } - return $recordings; + return ['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', DEBUG_DEVELOPER); + return ['recordings' => [], 'success' => false]; } /** diff --git a/classes/recording.php b/classes/recording.php index d0454831..90575efb 100644 --- a/classes/recording.php +++ b/classes/recording.php @@ -700,14 +700,16 @@ protected static function fetch_records(array $selects, array $params): array { }, $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); } @@ -795,7 +797,8 @@ public static function sync_pending_recordings_from_server($dismissedonly = fals // 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/classes/task/upgrade_recordings_task.php b/classes/task/upgrade_recordings_task.php index 65f950bf..a02432e2 100644 --- a/classes/task/upgrade_recordings_task.php +++ b/classes/task/upgrade_recordings_task.php @@ -74,7 +74,8 @@ protected function process_bigbluebuttonbn_logs(string $meetingid, bool $isimpor 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/classes/test/testcase_helper_trait.php b/classes/test/testcase_helper_trait.php index 1a6e2cb6..9dcb946b 100644 --- a/classes/test/testcase_helper_trait.php +++ b/classes/test/testcase_helper_trait.php @@ -309,7 +309,8 @@ protected function create_legacy_log_entries( $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/cli/update_dismissed_recordings.php b/cli/update_dismissed_recordings.php index ae5abf1b..60061bc2 100644 --- a/cli/update_dismissed_recordings.php +++ b/cli/update_dismissed_recordings.php @@ -105,7 +105,8 @@ $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 dimissed recordings found ..."); } else { diff --git a/tests/local/proxy/recording_proxy_test.php b/tests/local/proxy/recording_proxy_test.php index da28a1a6..ed71cca5 100644 --- a/tests/local/proxy/recording_proxy_test.php +++ b/tests/local/proxy/recording_proxy_test.php @@ -46,7 +46,8 @@ public function test_fetch_recordings() { $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 @@ public function test_fetch_recordings_breakoutroom() { $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); } }