MDL-35773 core_backup: use existing API to recover files

diff --git a/backup/util/dbops/restore_dbops.class.php b/backup/util/dbops/restore_dbops.class.php
index 165864e..74ca49b 100644 (file)
--- 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 ba6b8be..3f7d6f0 100644 (file)
--- 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 f3840ca..103dcca 100644 (file)
--- 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 c667040..30c15ad 100644 (file)
--- 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 76f75a2..14421a0 100644 (file)
--- 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 fbcba72..8508a67 100644 (file)
--- 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 420edb4..49c6214 100644 (file)
--- 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 ===