MDL-28666 Files API and db changes

1. Fix db regression
2. Unit tests for is_external_file() and rename exception
3. Searching reference method exclude draft files

diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php
index 29af2df..c74c724 100644 (file)
--- a/lib/db/upgrade.php
+++ b/lib/db/upgrade.php
@@ -594,26 +594,29 @@ function xmldb_main_upgrade($oldversion) {
 
     if ($oldversion < 2012052100.00) {
 
-        // Define field referencefileid to be added to files
+        // Define field referencefileid to be added to files.
         $table = new xmldb_table('files');
 
-        // Define field referencefileid to be added to files
+        // Define field referencefileid to be added to files.
         $field = new xmldb_field('referencefileid', XMLDB_TYPE_INTEGER, '10', null, null, null, null, 'sortorder');
-        // Conditionally launch add field referencefileid
+
+        // Conditionally launch add field referencefileid.
         if (!$dbman->field_exists($table, $field)) {
             $dbman->add_field($table, $field);
         }
 
-        // Define field referencelastsync to be added to files
+        // Define field referencelastsync to be added to files.
         $field = new xmldb_field('referencelastsync', XMLDB_TYPE_INTEGER, '10', null, null, null, null, 'referencefileid');
-        // Conditionally launch add field referencelastsync
+
+        // Conditionally launch add field referencelastsync.
         if (!$dbman->field_exists($table, $field)) {
             $dbman->add_field($table, $field);
         }
 
-        // Define field referencelifetime to be added to files
-        $field = new xmldb_field('referencelastsync', XMLDB_TYPE_INTEGER, '10', null, null, null, null, 'referencefileid');
-        // Conditionally launch add field referencelifetime
+        // Define field referencelifetime to be added to files.
+        $field = new xmldb_field('referencelifetime', XMLDB_TYPE_INTEGER, '10', null, null, null, null, 'referencelastsync');
+
+        // Conditionally launch add field referencelifetime.
         if (!$dbman->field_exists($table, $field)) {
             $dbman->add_field($table, $field);
         }
@@ -627,7 +630,6 @@ function xmldb_main_upgrade($oldversion) {
 
         // Adding fields to table files_reference.
         $table->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null);
-        $table->add_field('fileid', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null);
         $table->add_field('repositoryid', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null);
         $table->add_field('lastsync', XMLDB_TYPE_INTEGER, '10', null, null, null, null);
         $table->add_field('lifetime', XMLDB_TYPE_INTEGER, '10', null, null, null, null);

diff --git a/lib/filestorage/file_storage.php b/lib/filestorage/file_storage.php
index 55a6307..f95827c 100644 (file)
--- a/lib/filestorage/file_storage.php
+++ b/lib/filestorage/file_storage.php
@@ -1665,9 +1665,9 @@ class file_storage {
                   FROM {files} f
              LEFT JOIN {files_reference} r
                        ON f.referencefileid = r.id
-                 WHERE r.reference = ?";
+                 WHERE r.reference = ? AND (f.component <> ? OR f.filearea <> ?)";
 
-        $rs = $DB->get_recordset_sql($sql, array($str));
+        $rs = $DB->get_recordset_sql($sql, array($str, 'user', 'draft'));
         $files = array();
         foreach ($rs as $filerecord) {
             $file = $this->get_file_instance($filerecord);
@@ -1691,9 +1691,9 @@ class file_storage {
                   FROM {files} f
              LEFT JOIN {files_reference} r
                        ON f.referencefileid = r.id
-                 WHERE r.reference = ?";
+                 WHERE r.reference = ? AND (f.component <> ? OR f.filearea <> ?)";
 
-        $count = $DB->count_records_sql($sql, array($str));
+        $count = $DB->count_records_sql($sql, array($str, 'user', 'draft'));
         return $count;
     }
 
@@ -1722,9 +1722,9 @@ class file_storage {
                   FROM {files} f
              LEFT JOIN {files_reference} r
                        ON f.referencefileid = r.id
-                 WHERE r.reference = ?";
+                 WHERE r.reference = ? AND (f.component <> ? OR f.filearea <> ?)";
 
-        $rs = $DB->get_recordset_sql($sql, array($reference));
+        $rs = $DB->get_recordset_sql($sql, array($reference, 'user', 'draft'));
         $files = array();
         foreach ($rs as $filerecord) {
             $file = $this->get_file_instance($filerecord);
@@ -1761,9 +1761,9 @@ class file_storage {
                   FROM {files} f
              LEFT JOIN {files_reference} r
                        ON f.referencefileid = r.id
-                 WHERE r.reference = ?";
+                 WHERE r.reference = ? AND (f.component <> ? OR f.filearea <> ?)";
 
-        $count = $DB->count_records_sql($sql, array($reference));
+        $count = $DB->count_records_sql($sql, array($reference, 'user', 'draft'));
         return $count;
     }
 

diff --git a/lib/filestorage/stored_file.php b/lib/filestorage/stored_file.php
index f7e1589..183ab9b 100644 (file)
--- a/lib/filestorage/stored_file.php
+++ b/lib/filestorage/stored_file.php
@@ -161,6 +161,9 @@ class stored_file {
      * @param string $filename file name
      */
     public function rename($filepath, $filename) {
+        if ($this->fs->file_exists($this->get_contextid(), $this->get_component(), $this->get_filearea(), $this->get_itemid(), $filepath, $filename)) {
+            throw new file_exception('storedfilenotcreated', '', 'file exists, cannot rename');
+        }
         $filerecord = new stdClass;
         $filerecord->filepath = $filepath;
         $filerecord->filename = $filename;

diff --git a/lib/filestorage/tests/file_storage_test.php b/lib/filestorage/tests/file_storage_test.php
index d525332..9685e3f 100644 (file)
--- a/lib/filestorage/tests/file_storage_test.php
+++ b/lib/filestorage/tests/file_storage_test.php
@@ -109,8 +109,12 @@ class filestoragelib_testcase extends advanced_testcase {
         $contenthash = $originalfile->get_contenthash();
         $newpath = '/test/';
         $newname = 'newtest.txt';
+        // try break it
+        $this->setExpectedException('file_exception');
+        // this shall throw exception
+        $originalfile->rename($filepath, $filename);
+        // this should work
         $originalfile->rename($newpath, $newname);
-
         $file = $fs->get_file($syscontext->id, $component, $filearea, $itemid, $newpath, $newname);
         $this->assertInstanceOf('stored_file', $file);
         $this->assertEquals($contenthash, $file->get_contenthash());

diff --git a/lib/tests/filelib_test.php b/lib/tests/filelib_test.php
index 667007e..9a419de 100644 (file)
--- a/lib/tests/filelib_test.php
+++ b/lib/tests/filelib_test.php
@@ -102,8 +102,16 @@ class filelib_testcase extends advanced_testcase {
         $usercontext = context_user::instance($user->id);
         $USER = $DB->get_record('user', array('id'=>$user->id));
 
+        $repositorypluginname = 'user';
+
+        $args = array();
+        $args['type'] = $repositorypluginname;
+        $repos = repository::get_instances($args);
+        $userrepository = reset($repos);
+        $this->assertInstanceOf('repository', $userrepository);
 
         $fs = get_file_storage();
+
         $syscontext = context_system::instance();
         $component = 'core';
         $filearea  = 'unittest';
@@ -122,28 +130,51 @@ class filelib_testcase extends advanced_testcase {
             'source'    => $sourcefield,
         );
         $ref = $fs->pack_reference($filerecord);
-
         $originalfile = $fs->create_file_from_string($filerecord, 'Test content');
-
         $fileid = $originalfile->get_id();
         $this->assertInstanceOf('stored_file', $originalfile);
 
+        // create a user private file
+        $userfilerecord = new stdClass;
+        $userfilerecord->contextid = $usercontext->id;
+        $userfilerecord->component = 'user';
+        $userfilerecord->filearea  = 'private';
+        $userfilerecord->itemid    = 0;
+        $userfilerecord->filepath  = '/';
+        $userfilerecord->filename  = 'userfile.txt';
+        $userfilerecord->source    = 'test';
+        $userfile = $fs->create_file_from_string($userfilerecord, 'User file content');
+        $userfileref = $fs->pack_reference($userfilerecord);
+
+        $filerefrecord = clone((object)$filerecord);
+        $filerefrecord->filename = 'testref.txt';
+        // create a file reference
+        $fileref = $fs->create_file_from_reference($filerefrecord, $userrepository->id, $userfileref);
+        $this->assertInstanceOf('stored_file', $fileref);
+        $this->assertEquals($userrepository->id, $fileref->repository->id);
+        $this->assertEquals($userfile->get_contenthash(), $fileref->get_contenthash());
+        $this->assertEquals($userfile->get_filesize(), $fileref->get_filesize());
+        $this->assertRegExp('#' . $userfile->get_filename(). '$#', $fileref->get_reference_details());
+
         $draftitemid = 0;
         file_prepare_draft_area($draftitemid, $syscontext->id, $component, $filearea, $itemid);
 
         $draftfiles = $fs->get_area_files($usercontext->id, 'user', 'draft', $draftitemid);
-        $this->assertEquals(2, count($draftfiles));
+        $this->assertEquals(3, count($draftfiles));
 
         $draftfile = $fs->get_file($usercontext->id, 'user', 'draft', $draftitemid, $filepath, $filename);
         $source = unserialize($draftfile->get_source());
         $this->assertEquals($ref, $source->original);
         $this->assertEquals($sourcefield, $source->source);
 
+        $draftfileref = $fs->get_file($usercontext->id, 'user', 'draft', $draftitemid, $filepath, $filerefrecord->filename);
+        $this->assertInstanceOf('stored_file', $draftfileref);
+        $this->assertEquals(true, $draftfileref->is_external_file());
 
         // change some information
         $author = 'Dongsheng Cai';
         $draftfile->set_author($author);
-        $newsourcefield = 'Get from flickr';
+        $newsourcefield = 'Get from Flickr';
         $license = 'GPLv3';
         $draftfile->set_license($license);
         // if you want to really just change source field, do this: