From 03756f0bdb330c43b46dfeb39f63a9ff8394825f Mon Sep 17 00:00:00 2001 From: Cameron Ball Date: Mon, 7 Aug 2017 15:27:46 +0800 Subject: [PATCH] MDL-59700 filestorage: Rework repositories to avoid add_file_to_pool For historical reasons repositories need to call add_file_to_pool to sync file records. However now that a before_file_created hook has been added additional information is needed by add_file_to_pool. Ideally add_file_to_pool and friends will become private/protected, so we need to remove all uses of it in core. This patch adds some new methods to the file class to allow syncing to be managed internally by the file and the file_storage class. --- lib/filestorage/file_storage.php | 24 ++++++++++++++++++++++++ lib/filestorage/stored_file.php | 18 ++++++++++++++++++ repository/boxnet/lib.php | 4 +--- repository/dropbox/lib.php | 4 +--- repository/equella/lib.php | 4 +--- repository/filesystem/lib.php | 4 +++- repository/lib.php | 4 +--- repository/upgrade.txt | 4 ++++ 8 files changed, 53 insertions(+), 13 deletions(-) diff --git a/lib/filestorage/file_storage.php b/lib/filestorage/file_storage.php index 44aa02b1c58..a9bfea533d8 100644 --- a/lib/filestorage/file_storage.php +++ b/lib/filestorage/file_storage.php @@ -1456,6 +1456,30 @@ class file_storage { return $this->get_file_instance($newrecord); } + /** + * Synchronise stored file from file. + * + * @param stored_file $file Stored file to synchronise. + * @param string $path Path to the file to synchronise from. + * @param stdClass $filerecord The file record from the database. + */ + public function synchronise_stored_file_from_file(stored_file $file, $path, $filerecord) { + list($contenthash, $filesize) = $this->add_file_to_pool($path, null, $filerecord); + $file->set_synchronized($contenthash, $filesize); + } + + /** + * Synchronise stored file from string. + * + * @param stored_file $file Stored file to synchronise. + * @param string $content File content. + * @param stdClass $filerecord The file record from the database. + */ + public function synchronise_stored_file_from_string(stored_file $file, $content, $filerecord) { + list($contenthash, $filesize) = $this->add_string_to_pool($content, $filerecord); + $file->set_synchronized($contenthash, $filesize); + } + /** * Create a new alias/shortcut file from file reference information * diff --git a/lib/filestorage/stored_file.php b/lib/filestorage/stored_file.php index e1c3e738d09..1331246c9a6 100644 --- a/lib/filestorage/stored_file.php +++ b/lib/filestorage/stored_file.php @@ -588,6 +588,24 @@ class stored_file { return $this->fs->create_directory($this->file_record->contextid, $this->file_record->component, $this->file_record->filearea, $this->file_record->itemid, $filepath); } + /** + * Set synchronised content from file. + * + * @param string $path Path to the file. + */ + public function set_synchronised_content_from_file($path) { + $this->fs->synchronise_stored_file_from_file($this, $path, $this->file_record); + } + + /** + * Set synchronised content from content. + * + * @param string $content File content. + */ + public function set_synchronised_content_from_string($content) { + $this->fs->synchronise_stored_file_from_string($this, $content, $this->file_record); + } + /** * Synchronize file if it is a reference and needs synchronizing * diff --git a/repository/boxnet/lib.php b/repository/boxnet/lib.php index 195cd23e35c..88b8b58598f 100644 --- a/repository/boxnet/lib.php +++ b/repository/boxnet/lib.php @@ -430,9 +430,7 @@ class repository_boxnet extends repository { $result = $c->download_one($url, null, array('filepath' => $path, 'timeout' => $CFG->repositorysyncimagetimeout)); $info = $c->get_info(); if ($result === true && isset($info['http_code']) && $info['http_code'] == 200) { - $fs = get_file_storage(); - list($contenthash, $filesize, $newfile) = $fs->add_file_to_pool($path); - $file->set_synchronized($contenthash, $filesize); + $file->set_synchronised_content_from_file($path); return true; } } diff --git a/repository/dropbox/lib.php b/repository/dropbox/lib.php index 8396503c749..0f7f2096fe3 100644 --- a/repository/dropbox/lib.php +++ b/repository/dropbox/lib.php @@ -662,9 +662,7 @@ class repository_dropbox extends repository { ]); $info = $c->get_info(); if ($result === true && isset($info['http_code']) && $info['http_code'] == 200) { - $fs = get_file_storage(); - list($contenthash, $filesize, ) = $fs->add_file_to_pool($saveas); - $file->set_synchronized($contenthash, $filesize); + $file->set_synchronised_content_from_file($saveas); return true; } } catch (Exception $e) { diff --git a/repository/equella/lib.php b/repository/equella/lib.php index fbd9c8479d3..cfffc8c024f 100644 --- a/repository/equella/lib.php +++ b/repository/equella/lib.php @@ -214,9 +214,7 @@ class repository_equella extends repository { $path = $this->prepare_file(''); $result = $c->download_one($url, null, array('filepath' => $path, 'followlocation' => true, 'timeout' => $CFG->repositorysyncimagetimeout)); if ($result === true) { - $fs = get_file_storage(); - list($contenthash, $filesize, $newfile) = $fs->add_file_to_pool($path); - $file->set_synchronized($contenthash, $filesize); + $file->set_synchronised_content_from_file($path); return true; } } else { diff --git a/repository/filesystem/lib.php b/repository/filesystem/lib.php index 1fa5e7c33e8..ce8502fe9e9 100644 --- a/repository/filesystem/lib.php +++ b/repository/filesystem/lib.php @@ -577,7 +577,9 @@ class repository_filesystem extends repository { $filesize = filesize($filepath); } else { // Copy file into moodle filepool (used to generate an image thumbnail). - list($contenthash, $filesize, $newfile) = $fs->add_file_to_pool($filepath); + $file->set_timemodified(filemtime($filepath)); + $file->set_synchronised_content_from_file($filepath); + return true; } } else { // Update only file size so file will NOT be copied into moodle filepool. diff --git a/repository/lib.php b/repository/lib.php index c6dfb5c8071..6f6759c0475 100644 --- a/repository/lib.php +++ b/repository/lib.php @@ -1757,9 +1757,7 @@ abstract class repository implements cacheable_object { try { $fileinfo = $this->get_file($file->get_reference()); if (isset($fileinfo['path'])) { - list($contenthash, $filesize, $newfile) = $fs->add_file_to_pool($fileinfo['path']); - // set this file and other similar aliases synchronised - $file->set_synchronized($contenthash, $filesize); + $file->set_synchronised_content_from_file($fileinfo['path']); } else { throw new moodle_exception('errorwhiledownload', 'repository', '', ''); } diff --git a/repository/upgrade.txt b/repository/upgrade.txt index 2fccc91777e..da9fa2253d4 100644 --- a/repository/upgrade.txt +++ b/repository/upgrade.txt @@ -3,6 +3,10 @@ information provided here is intended especially for developers. Full details of the repository API are available on Moodle docs: http://docs.moodle.org/dev/Repository_API +=== 3.4 === +Repositories should no longer directly call file_system#add_file_to_pool or file_system#add_string_to_pool +instead they should call the stored_file method, set_synchronised_content_from_file or set_synchronised_content_from_string + === 3.3 === The skydrive repository is deprecated - please migrate to the newer onedrive repository.