MDL-59700 filestorage: Rework repositories to avoid add_file_to_pool
authorCameron Ball <cameron@moodle.com>
Mon, 7 Aug 2017 07:27:46 +0000 (15:27 +0800)
committerCameron Ball <cameron@moodle.com>
Mon, 7 Aug 2017 07:50:15 +0000 (15:50 +0800)
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
lib/filestorage/stored_file.php
repository/boxnet/lib.php
repository/dropbox/lib.php
repository/equella/lib.php
repository/filesystem/lib.php
repository/lib.php
repository/upgrade.txt

index 44aa02b..a9bfea5 100644 (file)
@@ -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
      *
index e1c3e73..1331246 100644 (file)
@@ -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
      *
index 195cd23..88b8b58 100644 (file)
@@ -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;
             }
         }
index 8396503..0f7f209 100644 (file)
@@ -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) {
index fbd9c84..cfffc8c 100644 (file)
@@ -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 {
index 1fa5e7c..ce8502f 100644 (file)
@@ -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.
index c6dfb5c..6f6759c 100644 (file)
@@ -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', '', '');
                     }
index 2fccc91..da9fa22 100644 (file)
@@ -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.