MDL-42016 files: Properly synchronise internal references
authorMarina Glancy <marina@moodle.com>
Sun, 6 Oct 2013 06:22:36 +0000 (17:22 +1100)
committerMarina Glancy <marina@moodle.com>
Sun, 6 Oct 2013 07:05:05 +0000 (18:05 +1100)
lib/filelib.php
lib/filestorage/stored_file.php
lib/filestorage/tests/file_storage_test.php
lib/upgrade.txt
mod/assign/feedback/file/importziplib.php
repository/coursefiles/lib.php
repository/local/lib.php
repository/user/lib.php

index 8127f40..bf00102 100644 (file)
@@ -928,8 +928,6 @@ function file_save_draft_area_files($draftitemid, $contextid, $component, $filea
                     $oldfile->get_referencefileid() != $newfile->get_referencefileid() ||
                     $oldfile->get_userid() != $newfile->get_userid())) {
                 $oldfile->replace_file_with($newfile);
-                // push changes to all local files that are referencing this file
-                $fs->update_references_to_storedfile($oldfile);
             }
 
             // unchanged file or directory - we keep it as is
index e937405..de04fbb 100644 (file)
@@ -97,6 +97,7 @@ class stored_file {
      */
     protected function update($dataobject) {
         global $DB;
+        $updatereferencesneeded = false;
         $keys = array_keys((array)$this->file_record);
         foreach ($dataobject as $field => $value) {
             if (in_array($field, $keys)) {
@@ -156,6 +157,10 @@ class stored_file {
                     }
                 }
 
+                if (($field == 'contenthash' || $field == 'filesize') && $this->file_record->$field != $value) {
+                    $updatereferencesneeded = true;
+                }
+
                 // adding the field
                 $this->file_record->$field = $value;
             } else {
@@ -175,6 +180,10 @@ class stored_file {
         $this->file_record->mimetype = $mimetype;
 
         $DB->update_record('files', $this->file_record);
+        if ($updatereferencesneeded) {
+            // Either filesize or contenthash of this file have changed. Update all files that reference to it.
+            $this->fs->update_references_to_storedfile($this);
+        }
     }
 
     /**
@@ -198,12 +207,21 @@ class stored_file {
     /**
      * Replace the content by providing another stored_file instance
      *
+     * @deprecated since 2.6
+     * @see stored_file::replace_file_with()
      * @param stored_file $storedfile
      */
     public function replace_content_with(stored_file $storedfile) {
+        debugging('Function stored_file::replace_content_with() is deprecated. Please use stored_file::replace_file_with()', DEBUG_DEVELOPER);
+        $filerecord = new stdClass;
         $contenthash = $storedfile->get_contenthash();
-        $this->set_contenthash($contenthash);
-        $this->set_filesize($storedfile->get_filesize());
+        if ($this->fs->content_exists($contenthash)) {
+            $filerecord->contenthash = $contenthash;
+        } else {
+            throw new file_exception('storedfileproblem', 'Invalid contenthash, content must be already in filepool', $contenthash);
+        }
+        $filerecord->filesize = $storedfile->get_filesize();
+        $this->update($filerecord);
     }
 
     /**
@@ -684,12 +702,13 @@ class stored_file {
         return $this->file_record->filesize;
     }
 
-    /**
+     /**
      * Returns the size of file in bytes.
      *
      * @param int $filesize bytes
      */
     public function set_filesize($filesize) {
+        debugging('Function stored_file::set_filesize() is deprecated. Please use stored_file::replace_file_with()', DEBUG_DEVELOPER);
         $filerecord = new stdClass;
         $filerecord->filesize = $filesize;
         $this->update($filerecord);
@@ -762,22 +781,6 @@ class stored_file {
         return $this->file_record->contenthash;
     }
 
-    /**
-     * Set contenthash
-     *
-     * @param string $contenthash
-     */
-    protected function set_contenthash($contenthash) {
-        // make sure the content exists in moodle file pool
-        if ($this->fs->content_exists($contenthash)) {
-            $filerecord = new stdClass;
-            $filerecord->contenthash = $contenthash;
-            $this->update($filerecord);
-        } else {
-            throw new file_exception('storedfileproblem', 'Invalid contenthash, content must be already in filepool', $contenthash);
-        }
-    }
-
     /**
      * Returns sha1 hash of all file path components sha1("contextid/component/filearea/itemid/dir/dir/filename.ext").
      *
index 76a4b63..e0873b2 100644 (file)
@@ -413,7 +413,7 @@ class core_files_file_storage_testcase extends advanced_testcase {
 
         $file2 = clone($file1);
         $file2->filename = '2.txt';
-        $userfile2 = $fs->create_file_from_string($file2, 'file2 content');
+        $userfile2 = $fs->create_file_from_string($file2, 'file2 content longer');
         $this->assertInstanceOf('stored_file', $userfile2);
 
         $file3 = clone($file1);
@@ -1515,6 +1515,113 @@ class core_files_file_storage_testcase extends advanced_testcase {
         $this->assertTrue($symlink2->is_external_file());
     }
 
+    /**
+     * Make sure that when internal file is updated all references to it are
+     * updated immediately. When it is deleted, the references are converted
+     * to true copies.
+     */
+    public function test_update_reference_internal() {
+        purge_all_caches();
+        $this->resetAfterTest(true);
+        $user = $this->setup_three_private_files();
+        $fs = get_file_storage();
+        $repos = repository::get_instances(array('type' => 'user'));
+        $repo = reset($repos);
+
+        // Create two aliases linking the same original.
+
+        $areafiles = array_values($fs->get_area_files($user->ctxid, 'user', 'private', false, 'filename', false));
+
+        $originalfile = $areafiles[0];
+        $this->assertInstanceOf('stored_file', $originalfile);
+        $contenthash = $originalfile->get_contenthash();
+        $filesize = $originalfile->get_filesize();
+
+        $substitutefile = $areafiles[1];
+        $this->assertInstanceOf('stored_file', $substitutefile);
+        $newcontenthash = $substitutefile->get_contenthash();
+        $newfilesize = $substitutefile->get_filesize();
+
+        $originalrecord = array(
+            'contextid' => $originalfile->get_contextid(),
+            'component' => $originalfile->get_component(),
+            'filearea'  => $originalfile->get_filearea(),
+            'itemid'    => $originalfile->get_itemid(),
+            'filepath'  => $originalfile->get_filepath(),
+            'filename'  => $originalfile->get_filename(),
+        );
+
+        $aliasrecord = $this->generate_file_record();
+        $aliasrecord->filepath = '/A/';
+        $aliasrecord->filename = 'symlink.txt';
+
+        $ref = $fs->pack_reference($originalrecord);
+        $symlink1 = $fs->create_file_from_reference($aliasrecord, $repo->id, $ref);
+        // Make sure created alias is a reference and has the same size and contenthash as source.
+        $this->assertEquals($contenthash, $symlink1->get_contenthash());
+        $this->assertEquals($filesize, $symlink1->get_filesize());
+        $this->assertEquals($repo->id, $symlink1->get_repository_id());
+        $this->assertNotEmpty($symlink1->get_referencefileid());
+        $referenceid = $symlink1->get_referencefileid();
+
+        $aliasrecord->filepath = '/B/';
+        $aliasrecord->filename = 'symlink.txt';
+        $ref = $fs->pack_reference($originalrecord);
+        $symlink2 = $fs->create_file_from_reference($aliasrecord, $repo->id, $ref);
+        // Make sure created alias is a reference and has the same size and contenthash as source.
+        $this->assertEquals($contenthash, $symlink2->get_contenthash());
+        $this->assertEquals($filesize, $symlink2->get_filesize());
+        $this->assertEquals($repo->id, $symlink2->get_repository_id());
+        // Make sure both aliases have the same reference id.
+        $this->assertEquals($referenceid, $symlink2->get_referencefileid());
+
+        // Overwrite ofiginal file.
+        $originalfile->replace_file_with($substitutefile);
+        $this->assertEquals($newcontenthash, $originalfile->get_contenthash());
+        $this->assertEquals($newfilesize, $originalfile->get_filesize());
+
+        // References to the internal files must be synchronised immediately.
+        // Refetch A/symlink.txt file.
+        $symlink1 = $fs->get_file($aliasrecord->contextid, $aliasrecord->component,
+            $aliasrecord->filearea, $aliasrecord->itemid, '/A/', 'symlink.txt');
+        $this->assertTrue($symlink1->is_external_file());
+        $this->assertEquals($newcontenthash, $symlink1->get_contenthash());
+        $this->assertEquals($newfilesize, $symlink1->get_filesize());
+        $this->assertEquals($repo->id, $symlink1->get_repository_id());
+        $this->assertEquals($referenceid, $symlink1->get_referencefileid());
+
+        // Refetch B/symlink.txt file.
+        $symlink2 = $fs->get_file($aliasrecord->contextid, $aliasrecord->component,
+            $aliasrecord->filearea, $aliasrecord->itemid, '/B/', 'symlink.txt');
+        $this->assertTrue($symlink2->is_external_file());
+        $this->assertEquals($newcontenthash, $symlink2->get_contenthash());
+        $this->assertEquals($newfilesize, $symlink2->get_filesize());
+        $this->assertEquals($repo->id, $symlink2->get_repository_id());
+        $this->assertEquals($referenceid, $symlink2->get_referencefileid());
+
+        // Remove original file.
+        $originalfile->delete();
+
+        // References must be converted to independend files.
+        // Refetch A/symlink.txt file.
+        $symlink1 = $fs->get_file($aliasrecord->contextid, $aliasrecord->component,
+            $aliasrecord->filearea, $aliasrecord->itemid, '/A/', 'symlink.txt');
+        $this->assertFalse($symlink1->is_external_file());
+        $this->assertEquals($newcontenthash, $symlink1->get_contenthash());
+        $this->assertEquals($newfilesize, $symlink1->get_filesize());
+        $this->assertNull($symlink1->get_repository_id());
+        $this->assertNull($symlink1->get_referencefileid());
+
+        // Refetch B/symlink.txt file.
+        $symlink2 = $fs->get_file($aliasrecord->contextid, $aliasrecord->component,
+            $aliasrecord->filearea, $aliasrecord->itemid, '/B/', 'symlink.txt');
+        $this->assertFalse($symlink2->is_external_file());
+        $this->assertEquals($newcontenthash, $symlink2->get_contenthash());
+        $this->assertEquals($newfilesize, $symlink2->get_filesize());
+        $this->assertNull($symlink2->get_repository_id());
+        $this->assertNull($symlink2->get_referencefileid());
+    }
+
     public function test_get_unused_filename() {
         global $USER;
         $this->resetAfterTest(true);
index 4643fc1..acb06eb 100644 (file)
@@ -100,6 +100,10 @@ Navigation:
     * settings_navigation::
           get_course_modules()              -> (no replacement)
 
+Files and repositories:
+    * stored_file::replace_content_with()   -> stored_file::replace_file_with()
+    * stored_file::set_filesize()           -> stored_file::replace_file_with()
+
 Calendar:
     * add_event()                           -> calendar_event::create()
     * update_event()                        -> calendar_event->update()
index 840f5ac..c3fd443 100644 (file)
@@ -253,7 +253,7 @@ class assignfeedback_file_zip_importer {
                                                  '/',
                                                  $filename)) {
                         // Update existing feedback file.
-                        $oldfile->replace_content_with($unzippedfile);
+                        $oldfile->replace_file_with($unzippedfile);
                         $feedbackfilesupdated++;
                     } else {
                         // Create a new feedback file.
index 30a111d..e652756 100644 (file)
@@ -211,17 +211,6 @@ class repository_coursefiles extends repository {
         return true;
     }
 
-    /**
-     * Return reference file life time
-     *
-     * @param string $ref
-     * @return int
-     */
-    public function get_reference_file_lifetime($ref) {
-        // this should be realtime
-        return 0;
-    }
-
     /**
      * Is this repository accessing private data?
      *
index b1853bf..206162a 100644 (file)
@@ -126,17 +126,6 @@ class repository_local extends repository {
         return true;
     }
 
-    /**
-     * Return reference file life time
-     *
-     * @param string $ref
-     * @return int
-     */
-    public function get_reference_file_lifetime($ref) {
-        // this should be realtime
-        return 0;
-    }
-
     /**
      * Returns all children elements that have one of the specified extensions
      *
index 4e536d5..31191d6 100644 (file)
@@ -159,17 +159,6 @@ class repository_user extends repository {
         return FILE_INTERNAL | FILE_REFERENCE;
     }
 
-    /**
-     * Return reference file life time
-     *
-     * @param string $ref
-     * @return int
-     */
-    public function get_reference_file_lifetime($ref) {
-        // this should be realtime
-        return 0;
-    }
-
     /**
      * Is this repository accessing private data?
      *