Merge branch 'MDL-59960_mimetype' of https://github.com/mrmark/moodle

lib/filestorage/file_system.php

@@ -568,14 +568,13 @@ protected static function get_file_handle_for_path($path, $type = stored_file::F
      * @return string The MIME type.
      */
     public function mimetype_from_hash($contenthash, $filename) {
-        $pathname = $this->get_remote_path_from_hash($contenthash);
+        $pathname = $this->get_local_path_from_hash($contenthash);
         $mimetype = file_storage::mimetype($pathname, $filename);
 
-        if (!$this->is_file_readable_locally_by_hash($contenthash, false) && $mimetype === 'document/unknown') {
+        if ($mimetype === 'document/unknown' && !$this->is_file_readable_locally_by_hash($contenthash)) {
             // The type is unknown, but the full checks weren't completed because the file isn't locally available.
             // Ensure we have a local copy and try again.
             $pathname = $this->get_local_path_from_hash($contenthash, true);
-
             $mimetype = file_storage::mimetype_from_file($pathname);
         }
 
@@ -593,18 +592,7 @@ public function mimetype_from_storedfile($file) {
             // Files with an empty filesize are treated as directories and have no mimetype.
             return null;
         }
-        $pathname = $this->get_remote_path_from_storedfile($file);
-        $mimetype = file_storage::mimetype($pathname, $file->get_filename());
-
-        if (!$this->is_file_readable_locally_by_storedfile($file) && $mimetype === 'document/unknown') {
-            // The type is unknown, but the full checks weren't completed because the file isn't locally available.
-            // Ensure we have a local copy and try again.
-            $pathname = $this->get_local_path_from_storedfile($file, true);
-
-            $mimetype = file_storage::mimetype_from_file($pathname);
-        }
-
-        return $mimetype;
+        return $this->mimetype_from_hash($file->get_contenthash(), $file->get_filename());
     }
 
     /**

lib/filestorage/stored_file.php

@@ -138,6 +138,7 @@ public function is_controlled_link() {
     protected function update($dataobject) {
         global $DB;
         $updatereferencesneeded = false;
+        $updatemimetype = false;
         $keys = array_keys((array)$this->file_record);
         $filepreupdate = clone($this->file_record);
         foreach ($dataobject as $field => $value) {
@@ -202,15 +203,21 @@ protected function update($dataobject) {
                     $updatereferencesneeded = true;
                 }
 
+                if ($updatereferencesneeded || ($field === 'filename' && $this->file_record->filename != $value)) {
+                    $updatemimetype = true;
+                }
+
                 // adding the field
                 $this->file_record->$field = $value;
             } else {
                 throw new coding_exception("Invalid field name, $field doesn't exist in file record");
             }
         }
         // Validate mimetype field
-        $mimetype = $this->filesystem->mimetype_from_storedfile($this);
-        $this->file_record->mimetype = $mimetype;
+        if ($updatemimetype) {
+            $mimetype = $this->filesystem->mimetype_from_storedfile($this);
+            $this->file_record->mimetype = $mimetype;
+        }
 
         $DB->update_record('files', $this->file_record);
         if ($updatereferencesneeded) {

lib/filestorage/tests/file_system_test.php

@@ -951,14 +951,13 @@ public function test_mimetype_from_hash_using_filename() {
      * a locally available file whose filename does not suggest mimetype.
      */
     public function test_mimetype_from_hash_using_file_content() {
-        $filepath = '/path/to/file/not/currently/on/disk';
         $filecontent = 'example content';
         $contenthash = file_storage::hash_from_string($filecontent);
         $filename = 'example';
 
         $filepath = __DIR__ . "/fixtures/testimage.jpg";
-        $fs = $this->get_testable_mock(['get_remote_path_from_hash']);
-        $fs->method('get_remote_path_from_hash')->willReturn($filepath);
+        $fs = $this->get_testable_mock(['get_local_path_from_hash']);
+        $fs->method('get_local_path_from_hash')->willReturn($filepath);
 
         $result = $fs->mimetype_from_hash($contenthash, $filename);
         $this->assertEquals('image/jpeg', $result);
@@ -1023,8 +1022,8 @@ public function test_mimetype_from_storedfile_using_filename() {
      */
     public function test_mimetype_from_storedfile_using_file_content() {
         $filepath = __DIR__ . "/fixtures/testimage.jpg";
-        $fs = $this->get_testable_mock(['get_remote_path_from_storedfile']);
-        $fs->method('get_remote_path_from_storedfile')->willReturn($filepath);
+        $fs = $this->get_testable_mock(['get_local_path_from_hash']);
+        $fs->method('get_local_path_from_hash')->willReturn($filepath);
 
         $file = $this->get_stored_file('example content');
 
@@ -1040,14 +1039,12 @@ public function test_mimetype_from_storedfile_using_file_content_remote() {
         $filepath = __DIR__ . "/fixtures/testimage.jpg";
 
         $fs = $this->get_testable_mock([
-            'get_remote_path_from_storedfile',
-            'is_file_readable_locally_by_storedfile',
-            'get_local_path_from_storedfile',
+            'is_file_readable_locally_by_hash',
+            'get_local_path_from_hash',
         ]);
 
-        $fs->method('get_remote_path_from_storedfile')->willReturn('/path/to/remote/file');
-        $fs->method('is_file_readable_locally_by_storedfile')->willReturn(false);
-        $fs->method('get_local_path_from_storedfile')->willReturn($filepath);
+        $fs->method('is_file_readable_locally_by_hash')->willReturn(false);
+        $fs->method('get_local_path_from_hash')->will($this->onConsecutiveCalls('/path/to/remote/file', $filepath));
 
         $file = $this->get_stored_file('example content');