mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-04-20 16:04:25 +02:00
Code Issues Projects Releases Wiki Activity

MDL-77958 moodlenet: Pass stored_file to improve memory usage

Browse Source 
Previously the content of the stored file was extracted and stored in a
variable, passed around, and then submitted to MoodleNet. This results
in very high memory consumption when dealing with MoodleNet.

The stored_file should be passed around as a first-level param to
discourage this, and the content should _never_ be loaded into memory.
Instead file streams and resources should be used to allow Guzzle/Curl
to buffer the file from disk/other storage straight to MoodleNet.
This commit is contained in:
Andrew Nicols 2023-04-19 09:11:26 +08:00
parent 96a1a0abce
commit 581ccb9a47
5 changed files with 67 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
lib/classes/moodlenet/activity_packager.php
View File

@ -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;
}
}

38
lib/classes/moodlenet/activity_sender.php
View File

@ -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;
}
/**

37
lib/classes/moodlenet/moodlenet_client.php
View File

@ -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',
],
],

25
lib/tests/moodlenet/activity_packager_test.php
View File

@ -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());
}
}

8
lib/tests/moodlenet/activity_sender_test.php
View File

@ -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