From 2ff1076dad27aeae2ec50b33773261b20399839e Mon Sep 17 00:00:00 2001 From: "Shamiso.Jaravaza" <33659194+ssj365@users.noreply.github.com> Date: Mon, 23 Jan 2023 11:44:44 -0700 Subject: [PATCH] CONTRIB-9161: Fix dismissed recordings (#604) --- classes/recording.php | 36 ++++-- classes/task/check_dismissed_recordings.php | 45 +++++++ cli/update_dismissed_recordings.php | 127 ++++++++++++++++++++ db/tasks.php | 9 ++ lang/en/bigbluebuttonbn.php | 1 + tests/task/check_recordings_task_test.php | 124 +++++++++++++++++++ 6 files changed, 330 insertions(+), 12 deletions(-) create mode 100644 classes/task/check_dismissed_recordings.php create mode 100644 cli/update_dismissed_recordings.php create mode 100644 tests/task/check_recordings_task_test.php diff --git a/classes/recording.php b/classes/recording.php index f23fc20f7..5207e3792 100644 --- a/classes/recording.php +++ b/classes/recording.php @@ -667,6 +667,11 @@ protected function metadata_get($fieldname) { return $metadata[$possiblesourcename] ?? null; } + /** + * @var string Default sort for recordings when fetching from the database. + */ + const DEFAULT_RECORDING_SORT = 'timecreated ASC'; + /** * Fetch all records which match the specified parameters, including all metadata that relates to them. * @@ -757,27 +762,34 @@ protected function refresh_metadata_if_required() { /** * Synchronise pending recordings from the server. * + * @param bool $dismissedonly * This function should be called by the check_pending_recordings scheduled task. */ - public static function sync_pending_recordings_from_server(): void { + public static function sync_pending_recordings_from_server($dismissedonly = false): void { global $DB, $CFG; - - // Sort by bigbluebuttonbn_recordings_sortorder we get the same result on different db engines. - $recordingsort = $CFG->bigbluebuttonbn_recordings_sortorder ? 'timecreated ASC' : 'timecreated DESC'; + $params = [ + 'withindays' => time() - (self::RECORDING_TIME_LIMIT_DAYS * DAYSECS), + ]; // Fetch the local data. - mtrace("=> Looking for any recording awaiting processing from the past " . self::RECORDING_TIME_LIMIT_DAYS . " days."); - $select = 'status = :status_awaiting AND timecreated > :withindays OR status = :status_reset'; - $recordings = $DB->get_records_select(static::TABLE, $select, [ - 'status_awaiting' => self::RECORDING_STATUS_AWAITING, - 'withindays' => time() - (self::RECORDING_TIME_LIMIT_DAYS * DAYSECS), - 'status_reset' => self::RECORDING_STATUS_RESET, - ], $recordingsort); + if ($dismissedonly) { + mtrace("=> Looking for any recording dismissed processing from the past " . self::RECORDING_TIME_LIMIT_DAYS . " days."); + $select = 'status = :status_dismissed AND timecreated > :withindays'; + $params['status_dismissed'] = self::RECORDING_STATUS_DISMISSED; + } else { + mtrace("=> Looking for any recording awaiting processing from the past " . self::RECORDING_TIME_LIMIT_DAYS . " days."); + $select = 'status = :status_awaiting AND timecreated > :withindays OR status = :status_reset'; + $params['status_reset'] = self::RECORDING_STATUS_RESET; + $params['status_awaiting'] = self::RECORDING_STATUS_AWAITING; + } + + $recordings = $DB->get_records_select(static::TABLE, $select, $params, self::DEFAULT_RECORDING_SORT); + // Sort by DEFAULT_RECORDING_SORT we get the same result on different db engines. $recordingcount = count($recordings); mtrace("=> Found {$recordingcount} recordings to query"); // Grab the recording IDs. - $recordingids = array_map(function ($recording) { + $recordingids = array_map(function($recording) { return $recording->recordingid; }, $recordings); diff --git a/classes/task/check_dismissed_recordings.php b/classes/task/check_dismissed_recordings.php new file mode 100644 index 000000000..7a58e89ed --- /dev/null +++ b/classes/task/check_dismissed_recordings.php @@ -0,0 +1,45 @@ +. + +namespace mod_bigbluebuttonbn\task; + +use core\task\scheduled_task; +use mod_bigbluebuttonbn\recording; + +/** + * Synchronise pending and dismissed recordings from the server. + * + * @package mod_bigbluebuttonbn + * @copyright 2022 Laurent David Blindside Networks Inc + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class check_dismissed_recordings extends scheduled_task { + /** + * Run the migration task. + */ + public function execute() { + recording::sync_pending_recordings_from_server(true); + } + + /** + * Get the name of the task for use in the interface. + * + * @return string + */ + public function get_name(): string { + return get_string('taskname:check_dismissed_recordings', 'mod_bigbluebuttonbn'); + } +} diff --git a/cli/update_dismissed_recordings.php b/cli/update_dismissed_recordings.php new file mode 100644 index 000000000..9586e3c40 --- /dev/null +++ b/cli/update_dismissed_recordings.php @@ -0,0 +1,127 @@ +. + +/** + * Recordings CLI Migration script. + * + * @package mod_bigbluebuttonbn + * @copyright 2022 onwards, Blindside Networks Inc + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @author Laurent David (laurent [at] call-learning [dt] fr) + */ + +use mod_bigbluebuttonbn\instance; +use mod_bigbluebuttonbn\logger; +use mod_bigbluebuttonbn\recording; +use mod_bigbluebuttonbn\task\upgrade_recordings_task; + +define('CLI_SCRIPT', true); + +require(__DIR__ . '/../../../config.php'); +global $CFG; +require_once($CFG->libdir . '/clilib.php'); + +// Now get cli options. +list($options, $unrecognized) = cli_get_params( + [ + 'help' => false, + 'courseid' => 0, + 'bigbluebuttonid' => 0, + 'run' => false + ], + [ + 'h' => 'help', + 'c' => 'courseid', + 'b' => 'bigbluebuttoncmid', + 'r' => 'run' + ] +); + +if ($unrecognized) { + $unrecognized = implode("\n ", $unrecognized); + cli_error(get_string('cliunknowoption', 'admin', $unrecognized)); +} + +if ($options['help']) { + $help = + "Check for dismissed recording and see if they appear on the server. +Sometimes when the remote BigBlueButton server is temporarily not accessible it has been seen that the recordings +are set to dismissed status. For now this is a workaround until we refactor slightly the recording API. + +Options: +-h, --help Print out this help +-c, --courseid Course identifier (id) in which we look for BigblueButton activities and recordings. If not specified + we check every single BigblueButton activity. +-b, --bigbluebuttoncmid Identifier for the bigbluebutton activity we would like to specifically retrieve recordings for. If not + specified we check every single BigblueButton activity + (scoped or not to a course depending on -c option). +-r,--run If false (default, just display information. By default we just display the information. +Example: +\$ sudo -u www-data /usr/bin/php mod/bigbluebuttonbn/cli/update_dismissed_recordings.php -c=4 -r=1 +"; + + echo $help; + die; +} +global $DB; + +// Note : this script is intentionally self contained (and does not depend on the code created in MDL-76339, so +// it can be simply dropped and run straightaway without upgrading. + +$bbcms = []; +if (!empty($options['courseid'])) { + $courseid = $options['courseid']; + $modinfos = get_fast_modinfo($courseid)->get_instances_of('bigbluebuttonbn'); + $bbcms = array_values($modinfos); +} else if (!empty($options['bigbluebuttoncmid'])) { + [$course, $bbcm] = get_course_and_cm_from_cmid($options['bigbluebuttoncmid']); + $bbcms = [$bbcm]; +} else { + // All bigbluebutton activities. + foreach ($DB->get_fieldset_select('bigbluebuttonbn', 'id', '') as $bbid) { + [$course, $bbcm] = get_course_and_cm_from_instance($bbid, 'bigbluebuttonbn'); + array_push($bbcms, $bbcm); + } +} +foreach ($bbcms as $bbcm) { + $instance = instance::get_from_cmid($bbcm->id); + cli_writeln("Processing BigbluebButton {$instance->get_meeting_name()}(id:{$instance->get_instance_id()})," + . " in course {$bbcm->get_course()->fullname}(id:{$bbcm->get_course()->id})...."); + $recordings = recording::get_records(['status' => recording::RECORDING_STATUS_DISMISSED, + 'bigbluebuttonbnid' => $instance->get_instance_id()]); + $recordingkeys = array_map(function($rec) { + return $rec->get('recordingid'); + }, $recordings); + $recordingmeta = \mod_bigbluebuttonbn\local\proxy\recording_proxy::fetch_recordings($recordingkeys); + if (empty($recordings)) { + cli_writeln("\t->No dimissed recordings found ..."); + } else { + foreach ($recordings as $recording) { + if (!empty($recordingmeta[$recording->get('recordingid')])) { + $recordingwithmeta = new recording(0, $recording->to_record(), $recordingmeta[$recording->get('recordingid')]); + cli_writeln("\t-> Recording data found for " . $recordingwithmeta->get('name') . ' ID:' . + $recordingwithmeta->get('recordingid')); + if ($options['run']) { + $recordingwithmeta->set('status', recording::RECORDING_STATUS_PROCESSED); + $recordingwithmeta->save(); + cli_writeln("\t\t-> Metadata and status updated..."); + } + } else { + cli_writeln("\t-> No recording data found for " . $recording->get('recordingid')); + } + } + } +} diff --git a/db/tasks.php b/db/tasks.php index 6faed032c..809ad7143 100644 --- a/db/tasks.php +++ b/db/tasks.php @@ -34,4 +34,13 @@ 'month' => '*', 'dayofweek' => '*' ], + [ + 'classname' => 'mod_bigbluebuttonbn\task\check_dismissed_recordings', + 'blocking' => 0, + 'minute' => '*', + 'hour' => '*', + 'day' => '*/10', // Every 10 days. + 'month' => '*', + 'dayofweek' => '*' + ], ]; diff --git a/lang/en/bigbluebuttonbn.php b/lang/en/bigbluebuttonbn.php index d8e18671d..7818b1c5c 100644 --- a/lang/en/bigbluebuttonbn.php +++ b/lang/en/bigbluebuttonbn.php @@ -600,6 +600,7 @@ $string['cachedef_recordings'] = 'Recording metadata'; $string['cachedef_validatedurls'] = 'Cache of validated URL checks'; $string['taskname:check_pending_recordings'] = 'Fetch pending recordings'; +$string['taskname:check_dismissed_recordings'] = 'Check for dismissed recordings'; $string['userlimitreached'] = 'The number of users allowed in a session has been reached.'; $string['waitformoderator'] = 'Waiting for a moderator to join.'; diff --git a/tests/task/check_recordings_task_test.php b/tests/task/check_recordings_task_test.php new file mode 100644 index 000000000..5122a319f --- /dev/null +++ b/tests/task/check_recordings_task_test.php @@ -0,0 +1,124 @@ +. + +namespace mod_bigbluebuttonbn\task; + +use advanced_testcase; +use core\task\manager; +use mod_bigbluebuttonbn\instance; +use mod_bigbluebuttonbn\logger; +use mod_bigbluebuttonbn\recording; +use mod_bigbluebuttonbn\test\testcase_helper_trait; + +/** + * Test class for the check_pending_recordings and check_dismissed_recordings task + * + * @package mod_bigbluebuttonbn + * @copyright 2022 onwards, Blindside Networks Inc + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \mod_bigbluebuttonbn\task\check_dismissed_recordings + * @covers \mod_bigbluebuttonbn\task\check_pending_recordings + * @covers \mod_bigbluebuttonbn\recording::sync_pending_recordings_from_server + */ +class check_recordings_task_test extends advanced_testcase { + + use testcase_helper_trait; + + /** + * @var $RECORDINGS_DATA array fake recording data. + */ + const RECORDINGS_DATA = [ + ['name' => 'Recording 1'], + ['name' => 'Recording 2'], + ['name' => 'Recording 3'], + ['name' => 'Recording 4'], + ]; + + /** + * Setup for test + */ + public function setUp(): void { + parent::setUp(); + $this->initialise_mock_server(); + $this->resetAfterTest(); + } + + /** + * Upgrade task test + */ + public function test_check_dismissed_recordings(): void { + $recordings = $this->create_meeting_and_recordings(); + $this->assertEquals(4, recording::count_records()); + foreach ($recordings as $r) { + $recording = new recording($r->id); + $recording->set('status', recording::RECORDING_STATUS_DISMISSED); + } + $this->assertEquals(0, recording::count_records(['status' => recording::RECORDING_STATUS_PROCESSED])); + $task = new check_dismissed_recordings(); + ob_start(); + $task->execute(); + ob_end_clean(); + $this->assertEquals(4, recording::count_records(['status' => recording::RECORDING_STATUS_PROCESSED])); + } + + /** + * Upgrade task test + */ + public function test_check_pending_recordings(): void { + $recordings = $this->create_meeting_and_recordings(); + $this->assertEquals(4, recording::count_records()); + foreach ($recordings as $r) { + $recording = new recording($r->id); + $recording->set('status', recording::RECORDING_STATUS_AWAITING); + } + $this->assertEquals(0, recording::count_records(['status' => recording::RECORDING_STATUS_PROCESSED])); + $task = new check_pending_recordings(); + ob_start(); + $task->execute(); + ob_end_clean(); + $this->assertEquals(4, recording::count_records(['status' => recording::RECORDING_STATUS_PROCESSED])); + } + + /** + * Create sample meeting and recording. + * + * @return array recording data (not the persistent class but plain object) + * @throws \coding_exception + */ + private function create_meeting_and_recordings(): array { + global $DB; + $generator = $this->getDataGenerator()->get_plugin_generator('mod_bigbluebuttonbn'); + $course = $this->getDataGenerator()->create_course(); + $activity = $generator->create_instance([ + 'course' => $course->id, + 'type' => instance::TYPE_ALL + ]); + $instance = instance::get_from_instanceid($activity->id); + $generator->create_meeting([ + 'instanceid' => $instance->get_instance_id(), + 'groupid' => $instance->get_group_id() + ]); + foreach (self::RECORDINGS_DATA as $rindex => $data) { + $recordings[] = $generator->create_recording( + array_merge([ + 'bigbluebuttonbnid' => $instance->get_instance_id(), + 'groupid' => $instance->get_group_id() + ], $data) + ); + } + return $recordings; + } +}