From 94c71056a31327d6ef121ff7bb2a2cb15675b0c4 Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Thu, 20 Jun 2019 20:11:04 +0800 Subject: [PATCH] MDL-35773 core_backup: use existing API to recover files --- backup/util/dbops/restore_dbops.class.php | 22 +++++++++++++++---- lib/filestorage/file_system.php | 4 ++-- lib/filestorage/file_system_filedir.php | 4 ++-- lib/filestorage/tests/file_storage_test.php | 8 ++----- .../tests/file_system_filedir_test.php | 8 ++----- lib/filestorage/tests/file_system_test.php | 8 ++----- lib/upgrade.txt | 2 ++ 7 files changed, 30 insertions(+), 26 deletions(-) diff --git a/backup/util/dbops/restore_dbops.class.php b/backup/util/dbops/restore_dbops.class.php index 165864ec269..74ca49b2516 100644 --- a/backup/util/dbops/restore_dbops.class.php +++ b/backup/util/dbops/restore_dbops.class.php @@ -1062,13 +1062,27 @@ abstract class restore_dbops { $foundfile = reset($foundfiles); $fs->create_file_from_storedfile($file_record, $foundfile->id); } else { - // Finally try to restore the file from trash. - $filesytem = $fs->get_file_system(); + $filesystem = $fs->get_file_system(); $restorefile = $file; $restorefile->contextid = $newcontextid; $storedfile = new stored_file($fs, $restorefile); - $trashrecovery = $filesytem->recover_file($storedfile, true); - if (!$trashrecovery) { + + // Ok, let's try recover this file. + // 1. We check if the file can be fetched locally without attempting to fetch + // from the trash. + // 2. We check if we can get the remote filepath for the specified stored file. + // 3. We check if the file can be fetched from the trash. + // 4. All failed, say we couldn't find it. + if ($filesystem->is_file_readable_locally_by_storedfile($storedfile)) { + $localpath = $filesystem->get_local_path_from_storedfile($storedfile); + $fs->create_file_from_pathname($file, $localpath); + } else if ($filesystem->is_file_readable_remotely_by_storedfile($storedfile)) { + $url = $filesystem->get_remote_path_from_storedfile($storedfile); + $fs->create_file_from_url($file, $url); + } else if ($filesystem->is_file_readable_locally_by_storedfile($storedfile, true)) { + $localpath = $filesystem->get_local_path_from_storedfile($storedfile, true); + $fs->create_file_from_pathname($file, $localpath); + } else { // A matching file was not found. $results[] = self::get_missing_file_result($file); continue; diff --git a/lib/filestorage/file_system.php b/lib/filestorage/file_system.php index ba6b8be8f6b..3f7d6f0cd7c 100644 --- a/lib/filestorage/file_system.php +++ b/lib/filestorage/file_system.php @@ -80,7 +80,7 @@ abstract class file_system { * @param bool $fetchifnotfound Whether to attempt to fetch from the remote path if not found. * @return string full path to pool file with file content */ - protected function get_local_path_from_storedfile(stored_file $file, $fetchifnotfound = false) { + public function get_local_path_from_storedfile(stored_file $file, $fetchifnotfound = false) { return $this->get_local_path_from_hash($file->get_contenthash(), $fetchifnotfound); } @@ -94,7 +94,7 @@ abstract class file_system { * @param stored_file $file The file to serve. * @return string full path to pool file with file content */ - protected function get_remote_path_from_storedfile(stored_file $file) { + public function get_remote_path_from_storedfile(stored_file $file) { return $this->get_remote_path_from_hash($file->get_contenthash(), false); } diff --git a/lib/filestorage/file_system_filedir.php b/lib/filestorage/file_system_filedir.php index f3840ca9ca0..103dcca41e5 100644 --- a/lib/filestorage/file_system_filedir.php +++ b/lib/filestorage/file_system_filedir.php @@ -119,7 +119,7 @@ class file_system_filedir extends file_system { * @param bool $fetchifnotfound Whether to attempt to fetch from the remote path if not found. * @return string The full path to the content file */ - protected function get_local_path_from_storedfile(stored_file $file, $fetchifnotfound = false) { + public function get_local_path_from_storedfile(stored_file $file, $fetchifnotfound = false) { $filepath = $this->get_local_path_from_hash($file->get_contenthash(), $fetchifnotfound); // Try content recovery. @@ -136,7 +136,7 @@ class file_system_filedir extends file_system { * @param stored_file $file The file to serve. * @return string full path to pool file with file content */ - protected function get_remote_path_from_storedfile(stored_file $file) { + public function get_remote_path_from_storedfile(stored_file $file) { return $this->get_local_path_from_storedfile($file, false); } diff --git a/lib/filestorage/tests/file_storage_test.php b/lib/filestorage/tests/file_storage_test.php index c667040cfce..30c15ad4181 100644 --- a/lib/filestorage/tests/file_storage_test.php +++ b/lib/filestorage/tests/file_storage_test.php @@ -74,10 +74,8 @@ class core_files_file_storage_testcase extends advanced_testcase { $this->assertTrue($DB->record_exists('files', array('pathnamehash'=>$pathhash))); - $method = new ReflectionMethod('file_system', 'get_local_path_from_storedfile'); - $method->setAccessible(true); $filesystem = $fs->get_file_system(); - $location = $method->invokeArgs($filesystem, array($file, true)); + $location = $filesystem->get_local_path_from_storedfile($file, true); $this->assertFileExists($location); @@ -149,10 +147,8 @@ class core_files_file_storage_testcase extends advanced_testcase { $this->assertTrue($DB->record_exists('files', array('pathnamehash'=>$pathhash))); - $method = new ReflectionMethod('file_system', 'get_local_path_from_storedfile'); - $method->setAccessible(true); $filesystem = $fs->get_file_system(); - $location = $method->invokeArgs($filesystem, array($file, true)); + $location = $filesystem->get_local_path_from_storedfile($file, true); $this->assertFileExists($location); diff --git a/lib/filestorage/tests/file_system_filedir_test.php b/lib/filestorage/tests/file_system_filedir_test.php index 76f75a294e2..14421a09a07 100644 --- a/lib/filestorage/tests/file_system_filedir_test.php +++ b/lib/filestorage/tests/file_system_filedir_test.php @@ -257,9 +257,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase { ->with($this->equalTo($file)); $file = $this->get_stored_file('example content'); - $method = new ReflectionMethod(file_system_filedir::class, 'get_local_path_from_storedfile'); - $method->setAccessible(true); - $result = $method->invokeArgs($fs, array($file, true)); + $result = $fs->get_local_path_from_storedfile($file, true); $this->assertEquals($filepath, $result); } @@ -287,9 +285,7 @@ class core_files_file_system_filedir_testcase extends advanced_testcase { ->method('recover_file'); $file = $this->get_stored_file('example content'); - $method = new ReflectionMethod(file_system_filedir::class, 'get_local_path_from_storedfile'); - $method->setAccessible(true); - $result = $method->invokeArgs($fs, array($file, false)); + $result = $fs->get_local_path_from_storedfile($file, false); $this->assertEquals($filepath, $result); } diff --git a/lib/filestorage/tests/file_system_test.php b/lib/filestorage/tests/file_system_test.php index fbcba725c88..8508a677b27 100644 --- a/lib/filestorage/tests/file_system_test.php +++ b/lib/filestorage/tests/file_system_test.php @@ -250,9 +250,7 @@ class core_files_file_system_testcase extends advanced_testcase { $file = $this->get_stored_file($filecontent); - $method = new ReflectionMethod(file_system::class, 'get_local_path_from_storedfile'); - $method->setAccessible(true); - $result = $method->invokeArgs($fs, array_merge([$file], $args)); + $result = $fs->get_local_path_from_storedfile($file, $fetch); $this->assertEquals($filepath, $result); } @@ -280,9 +278,7 @@ class core_files_file_system_testcase extends advanced_testcase { $file = $this->get_stored_file($filecontent); - $method = new ReflectionMethod(file_system::class, 'get_remote_path_from_storedfile'); - $method->setAccessible(true); - $result = $method->invokeArgs($fs, [$file]); + $result = $fs->get_remote_path_from_storedfile($file); $this->assertEquals($filepath, $result); } diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 420edb4d9fa..49c6214c69e 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -20,6 +20,8 @@ information provided here is intended especially for developers. at least a single checkbox item is selected or not. * Final deprecation (removal) of the core/modal_confirm dialogue. * Upgrade scssphp to v1.0.2, This involves renaming classes from Leafo => ScssPhp as the repo has changed. +* The methods get_local_path_from_storedfile and get_remote_path_from_storedfile in lib/filestore/file_system.php + are now public. If you are overriding these then you will need to change your methods to public in your class. === 3.7 ===