From d089e4984de4a9d93d4d9a7095721c23f30935e2 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Tue, 14 Jun 2016 15:10:59 +0100 Subject: [PATCH 1/3] MDL-54869 files: Fix file_save_draft_area_files documentation --- lib/filelib.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/filelib.php b/lib/filelib.php index c26865d54c8..8031e38feab 100644 --- a/lib/filelib.php +++ b/lib/filelib.php @@ -776,7 +776,7 @@ function file_restore_source_field_from_draft_file($storedfile) { return $storedfile; } /** - * Saves files from a draft file area to a real one (merging the list of files). + * Saves files from a draft file area to a real one (replacing and removing files in the real file area). * Can rewrite URLs in some content at the same time if desired. * * @category files From 460897052e8037736a175a54b98d10c5c1b0bb85 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Wed, 15 Jun 2016 08:50:07 +0100 Subject: [PATCH 2/3] MDL-54869 files: Create new required functions This was a complex change requiring three new functions: file_merge_files_from_draft_area_into_filearea - To just add files from a draft area into a real one (just adding or updating, not deleting) file_merge_draft_area_into_draft_area - To merge files from two draft areas, used by the latest for creating a draft area with all the original files plus the new ones. file_overwrite_existing_draftfile - Required to update existing files not losing metadata or references. The whole process is the following: User uploads a file (upload.php) Client gets a new draftid A containing the file only (return of upload.php) Client requests to merge that draftid in the user's private files (core_user_add_user_private_files) Server prepares a new UNUSED draftid B from existing area Server merges A into B Server saves the draft B into the final area --- lib/filelib.php | 141 +++++++++++++++++++++++- lib/tests/filelib_test.php | 215 +++++++++++++++++++++++++++++++++++++ webservice/upload.php | 2 +- 3 files changed, 356 insertions(+), 2 deletions(-) diff --git a/lib/filelib.php b/lib/filelib.php index 8031e38feab..fe74a65c9e3 100644 --- a/lib/filelib.php +++ b/lib/filelib.php @@ -776,7 +776,7 @@ function file_restore_source_field_from_draft_file($storedfile) { return $storedfile; } /** - * Saves files from a draft file area to a real one (replacing and removing files in the real file area). + * Saves files from a draft file area to a real one (merging the list of files). * Can rewrite URLs in some content at the same time if desired. * * @category files @@ -2688,6 +2688,145 @@ function file_is_executable($filename) { } } +/** + * Overwrite an existing file in a draft area. + * + * @param stored_file $newfile the new file with the new content and meta-data + * @param stored_file $existingfile the file that will be overwritten + * @throws moodle_exception + * @since Moodle 3.2 + */ +function file_overwrite_existing_draftfile(stored_file $newfile, stored_file $existingfile) { + if ($existingfile->get_component() != 'user' or $existingfile->get_filearea() != 'draft') { + throw new coding_exception('The file to overwrite is not in a draft area.'); + } + + $fs = get_file_storage(); + // Remember original file source field. + $source = @unserialize($existingfile->get_source()); + // Remember the original sortorder. + $sortorder = $existingfile->get_sortorder(); + if ($newfile->is_external_file()) { + // New file is a reference. Check that existing file does not have any other files referencing to it + if (isset($source->original) && $fs->search_references_count($source->original)) { + throw new moodle_exception('errordoublereference', 'repository'); + } + } + + // Delete existing file to release filename. + $newfilerecord = array( + 'contextid' => $existingfile->get_contextid(), + 'component' => 'user', + 'filearea' => 'draft', + 'itemid' => $existingfile->get_itemid(), + 'timemodified' => time() + ); + $existingfile->delete(); + + // Create new file. + $newfile = $fs->create_file_from_storedfile($newfilerecord, $newfile); + // Preserve original file location (stored in source field) for handling references. + if (isset($source->original)) { + if (!($newfilesource = @unserialize($newfile->get_source()))) { + $newfilesource = new stdClass(); + } + $newfilesource->original = $source->original; + $newfile->set_source(serialize($newfilesource)); + } + $newfile->set_sortorder($sortorder); +} + +/** + * Add files from a draft area into a final area. + * + * Most of the time you do not want to use this. It is intended to be used + * by asynchronous services which cannot direcly manipulate a final + * area through a draft area. Instead they add files to a new draft + * area and merge that new draft into the final area when ready. + * + * @param int $draftitemid the id of the draft area to use. + * @param int $contextid this parameter and the next two identify the file area to save to. + * @param string $component component name + * @param string $filearea indentifies the file area + * @param int $itemid identifies the item id or false for all items in the file area + * @param array $options area options (subdirs=false, maxfiles=-1, maxbytes=0, areamaxbytes=FILE_AREA_MAX_BYTES_UNLIMITED) + * @see file_save_draft_area_files + * @since Moodle 3.2 + */ +function file_merge_files_from_draft_area_into_filearea($draftitemid, $contextid, $component, $filearea, $itemid, + array $options = null) { + // We use 0 here so file_prepare_draft_area creates a new one, finaldraftid will be updated with the new draft id. + $finaldraftid = 0; + file_prepare_draft_area($finaldraftid, $contextid, $component, $filearea, $itemid, $options); + file_merge_draft_area_into_draft_area($draftitemid, $finaldraftid); + file_save_draft_area_files($finaldraftid, $contextid, $component, $filearea, $itemid, $options); +} + +/** + * Merge files from two draftarea areas. + * + * This does not handle conflict resolution, files in the destination area which appear + * to be more recent will be kept disregarding the intended ones. + * + * @param int $getfromdraftid the id of the draft area where are the files to merge. + * @param int $mergeintodraftid the id of the draft area where new files will be merged. + * @throws coding_exception + * @since Moodle 3.2 + */ +function file_merge_draft_area_into_draft_area($getfromdraftid, $mergeintodraftid) { + global $USER; + + $fs = get_file_storage(); + $contextid = context_user::instance($USER->id)->id; + + if (!$filestomerge = $fs->get_area_files($contextid, 'user', 'draft', $getfromdraftid)) { + throw new coding_exception('Nothing to merge or area does not belong to current user'); + } + + $currentfiles = $fs->get_area_files($contextid, 'user', 'draft', $mergeintodraftid); + + // Get hashes of the files to merge. + $newhashes = array(); + foreach ($filestomerge as $filetomerge) { + $filepath = $filetomerge->get_filepath(); + $filename = $filetomerge->get_filename(); + + $newhash = $fs->get_pathname_hash($contextid, 'user', 'draft', $mergeintodraftid, $filepath, $filename); + $newhashes[$newhash] = $filetomerge; + } + + // Calculate wich files must be added. + foreach ($currentfiles as $file) { + $filehash = $file->get_pathnamehash(); + // One file to be merged already exists. + if (isset($newhashes[$filehash])) { + $updatedfile = $newhashes[$filehash]; + + // Avoid race conditions. + if ($file->get_timemodified() > $updatedfile->get_timemodified()) { + // The existing file is more recent, do not copy the suposedly "new" one. + unset($newhashes[$filehash]); + continue; + } + // Update existing file (not only content, meta-data too). + file_overwrite_existing_draftfile($updatedfile, $file); + unset($newhashes[$filehash]); + } + } + + foreach ($newhashes as $newfile) { + $newfilerecord = array( + 'contextid' => $contextid, + 'component' => 'user', + 'filearea' => 'draft', + 'itemid' => $mergeintodraftid, + 'timemodified' => time() + ); + + $fs->create_file_from_storedfile($newfilerecord, $newfile); + } +} + /** * RESTful cURL class * diff --git a/lib/tests/filelib_test.php b/lib/tests/filelib_test.php index 78854dd887d..a4a1faf5d8c 100644 --- a/lib/tests/filelib_test.php +++ b/lib/tests/filelib_test.php @@ -971,6 +971,221 @@ EOF; // Compare the final text is the same that the original. $this->assertEquals($originaltext, $finaltext); } + + /** + * Helpter function to create draft files + * + * @param array $filedata data for the file record (to not use defaults) + * @return stored_file the stored file instance + */ + public static function create_draft_file($filedata = array()) { + global $USER; + + self::setAdminUser(); + $fs = get_file_storage(); + + $filerecord = array( + 'component' => 'user', + 'filearea' => 'draft', + 'itemid' => isset($filedata['itemid']) ? $filedata['itemid'] : file_get_unused_draft_itemid(), + 'author' => isset($filedata['author']) ? $filedata['author'] : fullname($USER), + 'filepath' => isset($filedata['filepath']) ? $filedata['filepath'] : '/', + 'filename' => isset($filedata['filename']) ? $filedata['filename'] : 'file.txt', + ); + + if (isset($filedata['contextid'])) { + $filerecord['contextid'] = $filedata['contextid']; + } else { + $usercontext = context_user::instance($USER->id); + $filerecord['contextid'] = $usercontext->id; + } + $source = isset($filedata['source']) ? $filedata['source'] : serialize((object)array('source' => 'From string')); + $content = isset($filedata['content']) ? $filedata['content'] : 'some content here'; + + $file = $fs->create_file_from_string($filerecord, $content); + $file->set_source($source); + + return $file; + } + + /** + * Test file_merge_files_from_draft_area_into_filearea + */ + public function test_file_merge_files_from_draft_area_into_filearea() { + global $USER, $CFG; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $fs = get_file_storage(); + $usercontext = context_user::instance($USER->id); + + // Create a draft file. + $filename = 'data.txt'; + $filerecord = array( + 'filename' => $filename, + ); + $file = self::create_draft_file($filerecord); + + $maxbytes = $CFG->userquota; + $maxareabytes = $CFG->userquota; + $options = array('subdirs' => 1, + 'maxbytes' => $maxbytes, + 'maxfiles' => -1, + 'areamaxbytes' => $maxareabytes); + + // Add new file. + file_merge_files_from_draft_area_into_filearea($file->get_itemid(), $usercontext->id, 'user', 'private', 0, $options); + + $files = $fs->get_area_files($usercontext->id, 'user', 'private', 0); + // Directory and file. + $this->assertCount(2, $files); + $found = false; + foreach ($files as $file) { + if (!$file->is_directory()) { + $found = true; + $this->assertEquals($filename, $file->get_filename()); + $this->assertEquals('some content here', $file->get_content()); + } + } + $this->assertTrue($found); + + // Add two more files. + $filerecord = array( + 'itemid' => $file->get_itemid(), + 'filename' => 'second.txt', + ); + self::create_draft_file($filerecord); + $filerecord = array( + 'itemid' => $file->get_itemid(), + 'filename' => 'third.txt', + ); + $file = self::create_draft_file($filerecord); + + file_merge_files_from_draft_area_into_filearea($file->get_itemid(), $usercontext->id, 'user', 'private', 0, $options); + + $files = $fs->get_area_files($usercontext->id, 'user', 'private', 0); + $this->assertCount(4, $files); + + // Update contents of one file. + $filerecord = array( + 'filename' => 'second.txt', + 'content' => 'new content', + ); + $file = self::create_draft_file($filerecord); + file_merge_files_from_draft_area_into_filearea($file->get_itemid(), $usercontext->id, 'user', 'private', 0, $options); + + $files = $fs->get_area_files($usercontext->id, 'user', 'private', 0); + $this->assertCount(4, $files); + $found = false; + foreach ($files as $file) { + if ($file->get_filename() == 'second.txt') { + $found = true; + $this->assertEquals('new content', $file->get_content()); + } + } + $this->assertTrue($found); + + // Update author. + // Set different author in the current file. + foreach ($files as $file) { + if ($file->get_filename() == 'second.txt') { + $file->set_author('Nobody'); + } + } + $filerecord = array( + 'filename' => 'second.txt', + ); + $file = self::create_draft_file($filerecord); + + file_merge_files_from_draft_area_into_filearea($file->get_itemid(), $usercontext->id, 'user', 'private', 0, $options); + + $files = $fs->get_area_files($usercontext->id, 'user', 'private', 0); + $this->assertCount(4, $files); + $found = false; + foreach ($files as $file) { + if ($file->get_filename() == 'second.txt') { + $found = true; + $this->assertEquals(fullname($USER), $file->get_author()); + } + } + $this->assertTrue($found); + + } + + /** + * Test max area bytes for file_merge_files_from_draft_area_into_filearea + */ + public function test_file_merge_files_from_draft_area_into_filearea_max_area_bytes() { + global $USER; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $fs = get_file_storage(); + + $file = self::create_draft_file(); + $options = array('subdirs' => 1, + 'maxbytes' => 5, + 'maxfiles' => -1, + 'areamaxbytes' => 10); + + // Add new file. + file_merge_files_from_draft_area_into_filearea($file->get_itemid(), $file->get_contextid(), 'user', 'private', 0, $options); + $usercontext = context_user::instance($USER->id); + $files = $fs->get_area_files($usercontext->id, 'user', 'private', 0); + $this->assertCount(0, $files); + } + + /** + * Test max file bytes for file_merge_files_from_draft_area_into_filearea + */ + public function test_file_merge_files_from_draft_area_into_filearea_max_file_bytes() { + global $USER; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $fs = get_file_storage(); + + $file = self::create_draft_file(); + $options = array('subdirs' => 1, + 'maxbytes' => 1, + 'maxfiles' => -1, + 'areamaxbytes' => 100); + + // Add new file. + file_merge_files_from_draft_area_into_filearea($file->get_itemid(), $file->get_contextid(), 'user', 'private', 0, $options); + $usercontext = context_user::instance($USER->id); + // Check we only get the base directory, not a new file. + $files = $fs->get_area_files($usercontext->id, 'user', 'private', 0); + $this->assertCount(1, $files); + $file = array_shift($files); + $this->assertTrue($file->is_directory()); + } + + /** + * Test max file number for file_merge_files_from_draft_area_into_filearea + */ + public function test_file_merge_files_from_draft_area_into_filearea_max_files() { + global $USER; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $fs = get_file_storage(); + + $file = self::create_draft_file(); + $options = array('subdirs' => 1, + 'maxbytes' => 1000, + 'maxfiles' => 0, + 'areamaxbytes' => 1000); + + // Add new file. + file_merge_files_from_draft_area_into_filearea($file->get_itemid(), $file->get_contextid(), 'user', 'private', 0, $options); + $usercontext = context_user::instance($USER->id); + // Check we only get the base directory, not a new file. + $files = $fs->get_area_files($usercontext->id, 'user', 'private', 0); + $this->assertCount(1, $files); + $file = array_shift($files); + $this->assertTrue($file->is_directory()); + } } /** diff --git a/webservice/upload.php b/webservice/upload.php index e4fb7b5fa7a..c9e28c43cc2 100644 --- a/webservice/upload.php +++ b/webservice/upload.php @@ -159,7 +159,7 @@ foreach ($files as $file) { $file_record->itemid = $itemid; $file_record->license = $CFG->sitedefaultlicense; $file_record->author = fullname($authenticationinfo['user']); - $file_record->source = ''; + $file_record->source = serialize((object)array('source' => $file->filename)); //Check if the file already exist $existingfile = $fs->file_exists($file_record->contextid, $file_record->component, $file_record->filearea, From 6e7bf3963b12c1213646c468a356787e2e94e1b5 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Wed, 29 Jun 2016 16:41:25 +0100 Subject: [PATCH 3/3] MDL-54869 webservice: Fix core_user_add_user_private_files --- user/externallib.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/user/externallib.php b/user/externallib.php index 9260620cc88..9a3828f79f7 100644 --- a/user/externallib.php +++ b/user/externallib.php @@ -977,9 +977,9 @@ class core_user_external extends external_api { * @since Moodle 2.6 */ public static function add_user_private_files($draftid) { - global $CFG, $USER, $DB; + global $CFG, $USER; + require_once($CFG->libdir . "/filelib.php"); - require_once($CFG->dirroot . "/user/lib.php"); $params = self::validate_parameters(self::add_user_private_files_parameters(), array('draftid' => $draftid)); if (isguestuser()) { @@ -999,10 +999,9 @@ class core_user_external extends external_api { $options = array('subdirs' => 1, 'maxbytes' => $maxbytes, 'maxfiles' => -1, - 'accepted_types' => '*', 'areamaxbytes' => $maxareabytes); - file_save_draft_area_files($draftid, $context->id, 'user', 'private', 0, $options); + file_merge_files_from_draft_area_into_filearea($draftid, $context->id, 'user', 'private', 0, $options); return null; }