Required fixes before #1083982: Make file tests generic to allow them to be reused. From: Damien Tournoud --- modules/simpletest/tests/file.test | 15 +++++++++------ 1 files changed, 9 insertions(+), 6 deletions(-) diff --git a/modules/simpletest/tests/file.test b/modules/simpletest/tests/file.test index dc12b1b..430ba3e 100644 --- a/modules/simpletest/tests/file.test +++ b/modules/simpletest/tests/file.test @@ -196,10 +196,13 @@ class FileTestCase extends DrupalWebTestCase { * @return * File object. */ - function createFile($filepath = NULL, $contents = NULL, $scheme = 'public') { + function createFile($filepath = NULL, $contents = NULL, $scheme = NULL) { if (!isset($filepath)) { $filepath = $this->randomName(); } + if (!isset($scheme)) { + $scheme = file_default_scheme(); + } $filepath = $scheme . '://' . $filepath; if (!isset($contents)) { @@ -427,7 +430,7 @@ class FileValidatorTest extends DrupalWebTestCase { // Maximum size. if (image_get_toolkit()) { // Copy the image so that the original doesn't get resized. - copy(drupal_realpath('misc/druplicon.png'), 'temporary://druplicon.png'); + copy('misc/druplicon.png', 'temporary://druplicon.png'); $this->image->uri = 'temporary://druplicon.png'; $errors = file_validate_image_resolution($this->image, '10x5'); @@ -437,7 +440,7 @@ class FileValidatorTest extends DrupalWebTestCase { $this->assertTrue($info['width'] <= 10, t('Image scaled to correct width.'), 'File'); $this->assertTrue($info['height'] <= 5, t('Image scaled to correct height.'), 'File'); - drupal_unlink(drupal_realpath('temporary://druplicon.png')); + drupal_unlink('temporary://druplicon.png'); } else { // TODO: should check that the error is returned if no toolkit is available. @@ -531,13 +534,13 @@ class FileUnmanagedSaveDataTest extends FileTestCase { $filepath = file_unmanaged_save_data($contents); $this->assertTrue($filepath, t('Unnamed file saved correctly.')); $this->assertEqual(file_uri_scheme($filepath), file_default_scheme(), t("File was placed in Drupal's files directory.")); - $this->assertEqual($contents, file_get_contents(drupal_realpath($filepath)), t('Contents of the file are correct.')); + $this->assertEqual($contents, file_get_contents($filepath), t('Contents of the file are correct.')); // Provide a filename. $filepath = file_unmanaged_save_data($contents, 'public://asdf.txt', FILE_EXISTS_REPLACE); $this->assertTrue($filepath, t('Unnamed file saved correctly.')); $this->assertEqual('asdf.txt', basename($filepath), t('File was named correctly.')); - $this->assertEqual($contents, file_get_contents(drupal_realpath($filepath)), t('Contents of the file are correct.')); + $this->assertEqual($contents, file_get_contents($filepath), t('Contents of the file are correct.')); $this->assertFilePermissions($filepath, variable_get('file_chmod_file', 0664)); } } @@ -879,7 +882,7 @@ class FileDirectoryTest extends FileTestCase { */ function testFileCheckDirectoryHandling() { // A directory to operate on. - $directory = file_stream_wrapper_get_instance_by_scheme(file_default_scheme())->getDirectoryPath() . '/' . $this->randomName() . '/' . $this->randomName(); + $directory = file_default_scheme() . '://' . $this->randomName() . '/' . $this->randomName(); $this->assertFalse(is_dir($directory), t('Directory does not exist prior to testing.')); // Non-existent directory. Tests for #1083982: properly support remote filesystems. From: Damien Tournoud --- modules/simpletest/tests/file.test | 124 +++++++++++++++++++++++++++++ modules/simpletest/tests/file_test.module | 17 ++++ 2 files changed, 141 insertions(+), 0 deletions(-) diff --git a/modules/simpletest/tests/file.test b/modules/simpletest/tests/file.test index 430ba3e..9dbe546 100644 --- a/modules/simpletest/tests/file.test +++ b/modules/simpletest/tests/file.test @@ -546,6 +546,22 @@ class FileUnmanagedSaveDataTest extends FileTestCase { } /** + * Tests the file_unmanaged_save_data() function on remote filesystems. + */ +class RemoteFileUnmanagedSaveDataTest extends FileUnmanagedSaveDataTest { + public static function getInfo() { + $info = parent::getInfo(); + $info['group'] = 'File API (remote)'; + return $info; + } + + function setUp() { + parent::setUp('file_test'); + variable_set('file_default_scheme', 'dummy-remote'); + } +} + +/** * Test the file_save_upload() function. */ class FileSaveUploadTest extends FileHookTestCase { @@ -866,6 +882,22 @@ class FileSaveUploadTest extends FileHookTestCase { } /** + * Test the file_save_upload() function on remote filesystems. + */ +class RemoteFileSaveUploadTest extends FileSaveUploadTest { + public static function getInfo() { + $info = parent::getInfo(); + $info['group'] = 'File API (remote)'; + return $info; + } + + function setUp() { + parent::setUp('file_test'); + variable_set('file_default_scheme', 'dummy-remote'); + } +} + +/** * Directory related tests. */ class FileDirectoryTest extends FileTestCase { @@ -989,6 +1021,22 @@ class FileDirectoryTest extends FileTestCase { } /** + * Directory related tests. + */ +class RemoteFileDirectoryTest extends FileDirectoryTest { + public static function getInfo() { + $info = parent::getInfo(); + $info['group'] = 'File API (remote)'; + return $info; + } + + function setUp() { + parent::setUp('file_test'); + variable_set('file_default_scheme', 'dummy-remote'); + } +} + +/** * Tests the file_scan_directory() function. */ class FileScanDirectoryTest extends FileTestCase { @@ -1117,6 +1165,21 @@ class FileScanDirectoryTest extends FileTestCase { } } +/** + * Tests the file_scan_directory() function on remote filesystems. + */ +class RemoteFileScanDirectoryTest extends FileScanDirectoryTest { + public static function getInfo() { + $info = parent::getInfo(); + $info['group'] = 'File API (remote)'; + return $info; + } + + function setUp() { + parent::setUp('file_test'); + variable_set('file_default_scheme', 'dummy-remote'); + } +} /** * Deletion related tests. @@ -1163,6 +1226,21 @@ class FileUnmanagedDeleteTest extends FileTestCase { } } +/** + * Deletion related tests on remote filesystems. + */ +class RemoteFileUnmanagedDeleteTest extends FileUnmanagedDeleteTest { + public static function getInfo() { + $info = parent::getInfo(); + $info['group'] = 'File API (remote)'; + return $info; + } + + function setUp() { + parent::setUp('file_test'); + variable_set('file_default_scheme', 'dummy-remote'); + } +} /** * Deletion related tests. @@ -1240,6 +1318,21 @@ class FileUnmanagedDeleteRecursiveTest extends FileTestCase { } } +/** + * Deletion related tests on remote filesystems. + */ +class RemoteFileUnmanagedDeleteRecursiveTest extends FileUnmanagedDeleteRecursiveTest { + public static function getInfo() { + $info = parent::getInfo(); + $info['group'] = 'File API (remote)'; + return $info; + } + + function setUp() { + parent::setUp('file_test'); + variable_set('file_default_scheme', 'dummy-remote'); + } +} /** * Unmanaged move related tests. @@ -1313,6 +1406,21 @@ class FileUnmanagedMoveTest extends FileTestCase { } } +/** + * Unmanaged move related tests on remote filesystems. + */ +class RemoteFileUnmanagedMoveTest extends FileUnmanagedMoveTest { + public static function getInfo() { + $info = parent::getInfo(); + $info['group'] = 'File API (remote)'; + return $info; + } + + function setUp() { + parent::setUp('file_test'); + variable_set('file_default_scheme', 'dummy-remote'); + } +} /** * Unmanaged copy related tests. @@ -1403,6 +1511,22 @@ class FileUnmanagedCopyTest extends FileTestCase { } /** + * Unmanaged copy related tests on remote filesystems. + */ +class RemoteFileUnmanagedCopyTest extends FileUnmanagedCopyTest { + public static function getInfo() { + $info = parent::getInfo(); + $info['group'] = 'File API (remote)'; + return $info; + } + + function setUp() { + parent::setUp('file_test'); + variable_set('file_default_scheme', 'dummy-remote'); + } +} + +/** * Deletion related tests. */ class FileDeleteTest extends FileHookTestCase { diff --git a/modules/simpletest/tests/file_test.module b/modules/simpletest/tests/file_test.module index 2865a1f..b3c43e0 100644 --- a/modules/simpletest/tests/file_test.module +++ b/modules/simpletest/tests/file_test.module @@ -37,6 +37,11 @@ function file_test_stream_wrappers() { 'class' => 'DrupalDummyStreamWrapper', 'description' => t('Dummy wrapper for simpletest.'), ), + 'dummy-remote' => array( + 'name' => t('Dummy files (remote)'), + 'class' => 'DrupalDummyRemoteStreamWrapper', + 'description' => t('Dummy wrapper for simpletest (remote).'), + ), ); } @@ -442,3 +447,15 @@ class DrupalDummyStreamWrapper extends DrupalLocalStreamWrapper { } } +/** + * Helper class for testing the stream wrapper registry. + * + * Dummy remote stream wrapper implementation (dummy-remote://). + * + * Basically just the public scheme but not returning a local file for realpath. + */ +class DrupalDummyRemoteStreamWrapper extends DrupalPublicStreamWrapper { + function realpath() { + return FALSE; + } +} #1083982: properly support remote filesystems. From: Damien Tournoud --- includes/file.inc | 43 ++++++++++++++++++++++++++++++------------- 1 files changed, 30 insertions(+), 13 deletions(-) diff --git a/includes/file.inc b/includes/file.inc index 26a6c41..f79a737 100644 --- a/includes/file.inc +++ b/includes/file.inc @@ -756,7 +756,12 @@ function file_usage_delete(stdClass $file, $module, $type = NULL, $id = NULL, $c */ function file_copy(stdClass $source, $destination = NULL, $replace = FILE_EXISTS_RENAME) { if (!file_valid_uri($destination)) { - watchdog('file', 'File %file (%realpath) could not be copied, because the destination %destination is invalid. This is often caused by improper use of file_copy() or a missing stream wrapper.', array('%file' => $source->uri, '%realpath' => drupal_realpath($source->uri), '%destination' => $destination)); + if ($realpath = drupal_realpath($source->uri)) { + watchdog('file', 'File %file (%realpath) could not be copied, because the destination %destination is invalid. This is often caused by improper use of file_copy() or a missing stream wrapper.', array('%file' => $source->uri, '%realpath' => $realpath, '%destination' => $destination)); + } + else { + watchdog('file', 'File %file could not be copied, because the destination %destination is invalid. This is often caused by improper use of file_copy() or a missing stream wrapper.', array('%file' => $source->uri, '%destination' => $destination)); + } drupal_set_message(t('The specified file %file could not be copied, because the destination is invalid. More information is available in the system log.', array('%file' => $source->uri)), 'error'); return FALSE; } @@ -848,11 +853,15 @@ function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXIST $original_destination = $destination; // Assert that the source file actually exists. - $source = drupal_realpath($source); if (!file_exists($source)) { // @todo Replace drupal_set_message() calls with exceptions instead. drupal_set_message(t('The specified file %file could not be copied, because no file by that name exists. Please check that you supplied the correct filename.', array('%file' => $original_source)), 'error'); - watchdog('file', 'File %file (%realpath) could not be copied because it does not exist.', array('%file' => $original_source, '%realpath' => drupal_realpath($original_source))); + if ($realpath = drupal_realpath($source->uri)) { + watchdog('file', 'File %file (%realpath) could not be copied because it does not exist.', array('%file' => $original_source, '%realpath' => $realpath)); + } + else { + watchdog('file', 'File %file could not be copied because it does not exist.', array('%file' => $original_source)); + } return FALSE; } @@ -872,7 +881,7 @@ function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXIST $dirname = drupal_dirname($destination); if (!file_prepare_directory($dirname)) { // The destination is not valid. - watchdog('file', 'File %file could not be copied, because the destination directory %destination is not configured correctly.', array('%file' => $original_source, '%destination' => drupal_realpath($dirname))); + watchdog('file', 'File %file could not be copied, because the destination directory %destination is not configured correctly.', array('%file' => $original_source, '%destination' => $dirname)); drupal_set_message(t('The specified file %file could not be copied, because the destination directory is not properly configured. This may be caused by a problem with file or directory permissions. More information is available in the system log.', array('%file' => $original_source)), 'error'); return FALSE; } @@ -882,12 +891,14 @@ function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXIST $destination = file_destination($destination, $replace); if ($destination === FALSE) { drupal_set_message(t('The file %file could not be copied because a file by that name already exists in the destination directory.', array('%file' => $original_source)), 'error'); - watchdog('file', 'File %file could not be copied because a file by that name already exists in the destination directory (%directory)', array('%file' => $original_source, '%destination' => drupal_realpath($destination))); + watchdog('file', 'File %file could not be copied because a file by that name already exists in the destination directory (%directory)', array('%file' => $original_source, '%destination' => $destination)); return FALSE; } // Assert that the source and destination filenames are not the same. - if (drupal_realpath($source) == drupal_realpath($destination)) { + $real_source = drupal_realpath($source); + $real_destination = drupal_realpath($destination); + if ($source == $destination || ($real_source !== FALSE) && ($real_destination !== FALSE) && ($real_source == $real_destination)) { drupal_set_message(t('The specified file %file was not copied because it would overwrite itself.', array('%file' => $source)), 'error'); watchdog('file', 'File %file could not be copied because it would overwrite itself.', array('%file' => $source)); return FALSE; @@ -896,7 +907,7 @@ function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXIST file_ensure_htaccess(); // Perform the copy operation. if (!@copy($source, $destination)) { - watchdog('file', 'The specified file %file could not be copied to %destination.', array('%file' => $source, '%destination' => drupal_realpath($destination)), WATCHDOG_ERROR); + watchdog('file', 'The specified file %file could not be copied to %destination.', array('%file' => $source, '%destination' => $destination), WATCHDOG_ERROR); return FALSE; } @@ -988,7 +999,12 @@ function file_destination($destination, $replace) { */ function file_move(stdClass $source, $destination = NULL, $replace = FILE_EXISTS_RENAME) { if (!file_valid_uri($destination)) { - watchdog('file', 'File %file (%realpath) could not be moved, because the destination %destination is invalid. This may be caused by improper use of file_move() or a missing stream wrapper.', array('%file' => $source->uri, '%realpath' => drupal_realpath($source->uri), '%destination' => $destination)); + if ($realpath = drupal_realpath($source->uri)) { + watchdog('file', 'File %file (%realpath) could not be moved, because the destination %destination is invalid. This may be caused by improper use of file_move() or a missing stream wrapper.', array('%file' => $source->uri, '%realpath' => $realpath, '%destination' => $destination)); + } + else { + watchdog('file', 'File %file could not be moved, because the destination %destination is invalid. This may be caused by improper use of file_move() or a missing stream wrapper.', array('%file' => $source->uri, '%destination' => $destination)); + } drupal_set_message(t('The specified file %file could not be moved, because the destination is invalid. More information is available in the system log.', array('%file' => $source->uri)), 'error'); return FALSE; } @@ -1214,7 +1230,12 @@ function file_create_filename($basename, $directory) { */ function file_delete(stdClass $file, $force = FALSE) { if (!file_valid_uri($file->uri)) { - watchdog('file', 'File %file (%realpath) could not be deleted because it is not a valid URI. This may be caused by improper use of file_delete() or a missing stream wrapper.', array('%file' => $file->uri, '%realpath' => drupal_realpath($file->uri))); + if ($realpath = drupal_realpath($file->uri)) { + watchdog('file', 'File %file (%realpath) could not be deleted because it is not a valid URI. This may be caused by improper use of file_delete() or a missing stream wrapper.', array('%file' => $file->uri, '%realpath' => $realpath)); + } + else { + watchdog('file', 'File %file could not be deleted because it is not a valid URI. This may be caused by improper use of file_delete() or a missing stream wrapper.', array('%file' => $file->uri)); + } drupal_set_message(t('The specified file %file could not be deleted, because it is not a valid URI. More information is available in the system log.', array('%file' => $file->uri)), 'error'); return FALSE; } @@ -1258,8 +1279,6 @@ function file_delete(stdClass $file, $force = FALSE) { * @see file_unmanaged_delete_recursive() */ function file_unmanaged_delete($path) { - // Resolve streamwrapper URI to local path. - $path = drupal_realpath($path); if (is_dir($path)) { watchdog('file', '%path is a directory and cannot be removed using file_unmanaged_delete().', array('%path' => $path), WATCHDOG_ERROR); return FALSE; @@ -1301,8 +1320,6 @@ function file_unmanaged_delete($path) { * @see file_unmanaged_delete() */ function file_unmanaged_delete_recursive($path) { - // Resolve streamwrapper URI to local path. - $path = drupal_realpath($path); if (is_dir($path)) { $dir = dir($path); while (($entry = $dir->read()) !== FALSE) {