diff --git a/lib/classes/moodlenet/activity_packager.php b/lib/classes/moodlenet/activity_packager.php index 7e6f7ec862e..7fbc0d5c746 100644 --- a/lib/classes/moodlenet/activity_packager.php +++ b/lib/classes/moodlenet/activity_packager.php @@ -21,6 +21,7 @@ use backup_controller; use backup_root_task; use cm_info; use core\context\user; +use stored_file; defined('MOODLE_INTERNAL') || die(); @@ -66,10 +67,9 @@ class activity_packager { /** * Prepare the backup file using appropriate setting overrides and return relevant information. * - * @return array Array containing packaged file and stored_file object describing it. - * Array in the format [storedfile => stored_file object, filecontents => raw file]. + * @return stored_file */ - public function get_package(): array { + public function get_package(): stored_file { $alltasksettings = $this->get_all_task_settings(); // Override relevant settings to remove user data when packaging to share to MoodleNet. @@ -123,13 +123,12 @@ class activity_packager { } /** - * Package the activity identified by CMID. + * Package the activity identified by CMID into a new stored_file. * - * @return array Array containing packaged file and stored_file object describing it. - * Array in the format [storedfile => stored_file object, filecontents => raw file]. - * @throws \moodle_exception. + * @return stored_file + * @throws \moodle_exception */ - protected function package(): array { + protected function package(): stored_file { // Execute the backup and fetch the result. $this->controller->execute_plan(); $result = $this->controller->get_results(); @@ -147,7 +146,7 @@ class activity_packager { } // Create the location we want to copy this file to. - $fr = [ + $filerecord = [ 'contextid' => user::instance($this->userid)->id, 'userid' => $this->userid, 'component' => 'user', @@ -159,19 +158,11 @@ class activity_packager { // Create the local file based on the backup. $fs = get_file_storage(); - $packagedfiledata = [ - 'storedfile' => $fs->create_file_from_storedfile($fr, $backupfile), - ]; + $file = $fs->create_file_from_storedfile($filerecord, $backupfile); // Delete the backup now it has been created in the file area. $backupfile->delete(); - // Ensure we can handle files at the upper end of the limit supported by MoodleNet. - raise_memory_limit(activity_sender::MAX_FILESIZE); - - // Get the actual file content. - $packagedfiledata['filecontents'] = $packagedfiledata['storedfile']->get_content(); - - return $packagedfiledata; + return $file; } } diff --git a/lib/classes/moodlenet/activity_sender.php b/lib/classes/moodlenet/activity_sender.php index 2ce5b0fdb2a..ca7e4b6ba55 100644 --- a/lib/classes/moodlenet/activity_sender.php +++ b/lib/classes/moodlenet/activity_sender.php @@ -21,6 +21,7 @@ use core\event\moodlenet_resource_exported; use core\oauth2\client; use moodle_exception; use stdClass; +use stored_file; /** * API for sharing Moodle LMS activities to MoodleNet instances. @@ -108,17 +109,15 @@ class activity_sender { $filedata = $this->prepare_share_contents(); // If we have successfully prepared a file to share of permitted size, share it to MoodleNet. - if (!empty($filedata['storedfile'])) { - + if (!empty($filedata)) { // Avoid sending a file larger than the defined limit. - $filesize = $filedata['storedfile']->get_filesize(); + $filesize = $filedata->get_filesize(); if ($filesize > self::MAX_FILESIZE) { - $filedata['storedfile']->delete(); - throw new moodle_exception('moodlenet:sharefilesizelimitexceeded', 'core', '', - [ - 'filesize' => $filesize, - 'filesizelimit' => self::MAX_FILESIZE, - ]); + $filedata->delete(); + throw new moodle_exception('moodlenet:sharefilesizelimitexceeded', 'core', '', [ + 'filesize' => $filesize, + 'filesizelimit' => self::MAX_FILESIZE, + ]); } // MoodleNet only accept plaintext descriptions. @@ -130,7 +129,11 @@ class activity_sender { ['context' => $coursecontext] ); - $response = $this->moodlenetclient->create_resource_from_file($filedata, $this->cminfo->name, $resourcedescription); + $response = $this->moodlenetclient->create_resource_from_stored_file( + $filedata, + $this->cminfo->name, + $resourcedescription, + ); $responsecode = $response->getStatusCode(); $responsebody = json_decode($response->getBody()); @@ -140,7 +143,7 @@ class activity_sender { // Delete the generated file now it is no longer required. // (It has either been sent, or failed - retries not currently supported). - $filedata['storedfile']->delete(); + $filedata->delete(); } // Log every attempt to share (and whether or not it was successful). @@ -155,20 +158,17 @@ class activity_sender { /** * Prepare the data for sharing, in the format specified. * - * @return array Array of metadata about the file, as well as a stored_file object for the file. + * @return stored_file */ - protected function prepare_share_contents(): array { - + protected function prepare_share_contents(): stored_file { switch ($this->shareformat) { case self::SHARE_FORMAT_BACKUP: - default: // If sharing the activity as a backup, prepare the packaged backup. $packager = new activity_packager($this->cminfo, $this->userid); - $filedata = $packager->get_package(); - break; + return $packager->get_package(); + default: + throw new \coding_exception("Unknown share format: {$this->shareformat}'"); }; - - return $filedata; } /** diff --git a/lib/classes/moodlenet/moodlenet_client.php b/lib/classes/moodlenet/moodlenet_client.php index 7d897413e23..fabbfc074e5 100644 --- a/lib/classes/moodlenet/moodlenet_client.php +++ b/lib/classes/moodlenet/moodlenet_client.php @@ -18,6 +18,9 @@ namespace core\moodlenet; use core\http_client; use core\oauth2\client; +use stored_file; +use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\StreamInterface; /** * MoodleNet client which handles direct outbound communication with MoodleNet instances. @@ -54,22 +57,31 @@ class moodlenet_client { /** * Create a resource on MoodleNet which includes a file. * - * @param array $filedata The file data in the format [storedfile => stored_file object, filecontents => raw file]. + * @param stored_file $file The file data to send to MoodleNet. * @param string $resourcename The name of the resource being shared. * @param string $resourcedescription A description of the resource being shared. - * @return Psr\Http\Message\ResponseInterface The HTTP client response from MoodleNet. + * @return \Psr\Http\Message\ResponseInterface The HTTP client response from MoodleNet. */ - public function create_resource_from_file(array $filedata, string $resourcename, $resourcedescription) { + public function create_resource_from_stored_file( + stored_file $file, + string $resourcename, + string $resourcedescription, + ): ResponseInterface { // This may take a long time if a lot of data is being shared. \core_php_time_limit::raise(); $moodleneturl = $this->oauthclient->get_issuer()->get('baseurl'); $apiurl = rtrim($moodleneturl, '/') . self::API_CREATE_RESOURCE_URI; - $requestdata = $this->prepare_file_share_request_data($filedata, $resourcename, $resourcedescription); + $requestdata = $this->prepare_file_share_request_data( + $file->get_filename(), + $file->get_mimetype(), + $file->get_psr_stream(), + $resourcename, + $resourcedescription, + ); return $this->httpclient->request('POST', $apiurl, $requestdata); - } /** @@ -81,7 +93,13 @@ class moodlenet_client { * @param string $resourcedescription A description of the resource being shared. * @return array Data in the format required to send a file to MoodleNet using \core\httpclient. */ - protected function prepare_file_share_request_data(array $filedata, string $resourcename, $resourcedescription): array { + protected function prepare_file_share_request_data( + string $filename, + string $mimetype, + StreamInterface $stream, + string $resourcename, + $resourcedescription, + ): array { return [ 'headers' => [ 'Authorization' => 'Bearer ' . $this->oauthclient->get_accesstoken()->token, @@ -99,11 +117,10 @@ class moodlenet_client { ], [ 'name' => 'filecontents', - 'contents' => $filedata['filecontents'], + 'contents' => $stream, 'headers' => [ - 'Content-Disposition' => 'form-data; name=".resource"; filename="' . - $filedata['storedfile']->get_filename() . '"', - 'Content-Type' => $filedata['storedfile']->get_mimetype(), + 'Content-Disposition' => 'form-data; name=".resource"; filename="' . $filename . '"', + 'Content-Type' => $mimetype, 'Content-Transfer-Encoding' => 'binary', ], ], diff --git a/lib/tests/moodlenet/activity_packager_test.php b/lib/tests/moodlenet/activity_packager_test.php index 7c53b1f3455..fe89590eb0f 100644 --- a/lib/tests/moodlenet/activity_packager_test.php +++ b/lib/tests/moodlenet/activity_packager_test.php @@ -120,25 +120,18 @@ class activity_packager_test extends \advanced_testcase { $packager = new activity_packager($cminfo, $USER->id); $package = $packager->get_package(); - $this->assertEquals(2, count($package)); - - // Confirm there are backup file contents returned. - $this->assertTrue(array_key_exists('filecontents', $package)); - $this->assertNotEmpty($package['filecontents']); - // Confirm the expected stored_file object is returned. - $this->assertTrue(array_key_exists('storedfile', $package)); - $this->assertInstanceOf(\stored_file::class, $package['storedfile']); + $this->assertInstanceOf(\stored_file::class, $package); // Check some known values in the returned stored_file object to confirm they match the file we have packaged. - $this->assertNotEmpty($package['storedfile']->get_contenthash()); - $this->assertEquals(user::instance($USER->id)->id, $package['storedfile']->get_contextid()); - $this->assertEquals('user', $package['storedfile']->get_component()); - $this->assertEquals('draft', $package['storedfile']->get_filearea()); - $this->assertEquals('assign_backup.mbz', $package['storedfile']->get_filename()); - $this->assertGreaterThan(0, $package['storedfile']->get_filesize()); - $timecreated = $package['storedfile']->get_timecreated(); + $this->assertNotEmpty($package->get_contenthash()); + $this->assertEquals(user::instance($USER->id)->id, $package->get_contextid()); + $this->assertEquals('user', $package->get_component()); + $this->assertEquals('draft', $package->get_filearea()); + $this->assertEquals('assign_backup.mbz', $package->get_filename()); + $this->assertGreaterThan(0, $package->get_filesize()); + $timecreated = $package->get_timecreated(); $this->assertGreaterThanOrEqual($currenttime, $timecreated); - $this->assertEquals($timecreated, $package['storedfile']->get_timemodified()); + $this->assertEquals($timecreated, $package->get_timemodified()); } } diff --git a/lib/tests/moodlenet/activity_sender_test.php b/lib/tests/moodlenet/activity_sender_test.php index 8646f4aa373..1a2bed89af2 100644 --- a/lib/tests/moodlenet/activity_sender_test.php +++ b/lib/tests/moodlenet/activity_sender_test.php @@ -126,13 +126,9 @@ class activity_sender_test extends \advanced_testcase { activity_sender::SHARE_FORMAT_BACKUP )); $this->assertNotEmpty($package); - // Confirm there are backup file contents returned. - $this->assertTrue(array_key_exists('filecontents', $package)); - $this->assertNotEmpty($package['filecontents']); // Confirm the expected stored_file object is returned. - $this->assertTrue(array_key_exists('storedfile', $package)); - $this->assertInstanceOf(\stored_file::class, $package['storedfile']); + $this->assertInstanceOf(\stored_file::class, $package); } /** @@ -141,7 +137,7 @@ class activity_sender_test extends \advanced_testcase { * @dataProvider share_activity_provider * @covers ::share_activity * @covers ::log_event - * @covers \core\moodlenet\moodlenet_client::create_resource_from_file + * @covers \core\moodlenet\moodlenet_client::create_resource_from_stored_file * @covers \core\moodlenet\moodlenet_client::prepare_file_share_request_data * @param ResponseInterface $httpresponse * @param array $expected