core/includes/common.inc | 8 +- core/includes/file.inc | 279 ++++++++++---------- core/modules/color/color.module | 2 +- core/modules/file/file.module | 154 +++-------- .../file/lib/Drupal/file/Tests/CopyTest.php | 77 +++--- .../file/lib/Drupal/file/Tests/DownloadTest.php | 2 +- .../file/lib/Drupal/file/Tests/MoveTest.php | 150 +++++------ .../file/lib/Drupal/file/Tests/SaveDataTest.php | 52 +--- .../file/lib/Drupal/file/Tests/SaveUploadTest.php | 59 ++--- core/modules/file/tests/file_test/file_test.module | 12 +- .../lib/Drupal/image/Tests/ImageDimensionsTest.php | 2 +- .../image/Tests/ImageStylesPathAndUrlTest.php | 4 +- .../Drupal/image/Tests/ImageThemeFunctionTest.php | 4 +- .../lib/Drupal/locale/Tests/LocaleExportTest.php | 4 +- .../locale/Tests/LocaleImportFunctionalTest.php | 4 +- core/modules/locale/locale.batch.inc | 2 +- core/modules/system/image.gd.inc | 2 +- .../lib/Drupal/system/Tests/File/DirectoryTest.php | 52 +--- .../Drupal/system/Tests/File/UnmanagedCopyTest.php | 35 ++- .../Drupal/system/Tests/File/UnmanagedMoveTest.php | 32 ++- .../system/Tests/File/UnmanagedSaveDataTest.php | 2 +- .../lib/Drupal/system/Tests/Image/FileMoveTest.php | 7 +- core/modules/system/system.module | 21 +- core/modules/update/update.manager.inc | 4 +- core/modules/user/user.install | 2 +- 25 files changed, 382 insertions(+), 590 deletions(-) diff --git a/core/includes/common.inc b/core/includes/common.inc index 97b73f4..c314871 100644 --- a/core/includes/common.inc +++ b/core/includes/common.inc @@ -3144,7 +3144,7 @@ function drupal_build_css_cache($css) { $uri = $csspath . '/' . $filename; // Create the CSS file. file_prepare_directory($csspath, FILE_CREATE_DIRECTORY); - if (!file_exists($uri) && !file_unmanaged_save_data($data, $uri, FILE_EXISTS_REPLACE)) { + if (!file_exists($uri) && !file_unmanaged_save_data($data, $uri, FILE_CHECK_EXISTS_FALSE)) { return FALSE; } // If CSS gzip compression is enabled and the zlib extension is available @@ -3156,7 +3156,7 @@ function drupal_build_css_cache($css) { // aren't working can set css.gzip to FALSE in order to skip // generating a file that won't be used. if (config('system.performance')->get('css.gzip') && extension_loaded('zlib')) { - if (!file_exists($uri . '.gz') && !file_unmanaged_save_data(gzencode($data, 9, FORCE_GZIP), $uri . '.gz', FILE_EXISTS_REPLACE)) { + if (!file_exists($uri . '.gz') && !file_unmanaged_save_data(gzencode($data, 9, FORCE_GZIP), $uri . '.gz', FILE_CHECK_EXISTS_FALSE)) { return FALSE; } } @@ -4716,7 +4716,7 @@ function drupal_build_js_cache($files) { $uri = $jspath . '/' . $filename; // Create the JS file. file_prepare_directory($jspath, FILE_CREATE_DIRECTORY); - if (!file_exists($uri) && !file_unmanaged_save_data($contents, $uri, FILE_EXISTS_REPLACE)) { + if (!file_exists($uri) && !file_unmanaged_save_data($contents, $uri, FILE_CHECK_EXISTS_FALSE)) { return FALSE; } // If JS gzip compression is enabled and the zlib extension is available @@ -4728,7 +4728,7 @@ function drupal_build_js_cache($files) { // aren't working can set js.gzip to FALSE in order to skip // generating a file that won't be used. if (config('system.performance')->get('js.gzip') && extension_loaded('zlib')) { - if (!file_exists($uri . '.gz') && !file_unmanaged_save_data(gzencode($contents, 9, FORCE_GZIP), $uri . '.gz', FILE_EXISTS_REPLACE)) { + if (!file_exists($uri . '.gz') && !file_unmanaged_save_data(gzencode($contents, 9, FORCE_GZIP), $uri . '.gz', FILE_CHECK_EXISTS_FALSE)) { return FALSE; } } diff --git a/core/includes/file.inc b/core/includes/file.inc index c66a2e0..6017881 100644 --- a/core/includes/file.inc +++ b/core/includes/file.inc @@ -114,19 +114,19 @@ const FILE_MODIFY_PERMISSIONS = 2; /** - * Flag for dealing with existing files: Appends number until name is unique. + * Flag for dealing with existing files: Don't check. */ -const FILE_EXISTS_RENAME = 0; +const FILE_CHECK_EXISTS_FALSE = 0; /** - * Flag for dealing with existing files: Replace the existing file. + * Flag for dealing with existing files: Only check filesystem. */ -const FILE_EXISTS_REPLACE = 1; +const FILE_CHECK_EXISTS_FS = 1; /** - * Flag for dealing with existing files: Do nothing and return FALSE. + * Flag for dealing with existing files: Check database, then filesystem. */ -const FILE_EXISTS_ERROR = 2; +const FILE_CHECK_EXISTS_DB_FS = 2; /** * Indicates that the file is permanent and should not be deleted. @@ -599,8 +599,8 @@ function file_valid_uri($uri) { * - Checks if $source and $destination are valid and readable/writable. * - Checks that $source is not equal to $destination; if they are an error * is reported. - * - If file already exists in $destination either the call will error out, - * replace the file or rename the file based on the $replace parameter. + * - Ensures the file is copied to a "free" URI based on the $check_exists + * parameter. * - Provides a fallback using realpaths if the move fails using stream * wrappers. This can occur because PHP's copy() function does not properly * support streams if safe_mode or open_basedir are enabled. See @@ -613,31 +613,28 @@ function file_valid_uri($uri) { * URI may be a bare filepath (without a scheme) and in that case the default * scheme (file://) will be used. If this value is omitted, Drupal's default * files scheme will be used, usually "public://". - * @param $replace - * Replace behavior when the destination file already exists: - * - FILE_EXISTS_REPLACE - Replace the existing file. - * - FILE_EXISTS_RENAME - Append _{incrementing number} until the filename is - * unique. - * - FILE_EXISTS_ERROR - Do nothing and return FALSE. + * @param $check_exists + * The destination URI will be renamed if it already exists by appending + * _{incrementing number} until it is unique. + * - FILE_CHECK_EXISTS_FALSE - Don't check if it exists. + * - FILE_CHECK_EXISTS_FS - Just check the filesystem. + * - FILE_CHECK_EXISTS_DB_FS - Check the database, then the filesystem. * * @return * The path to the new file, or FALSE in the event of an error. * * @see file_copy() */ -function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXISTS_RENAME) { - $original_source = $source; - $original_destination = $destination; - +function file_unmanaged_copy($source, $destination = NULL, $check_exists = FILE_CHECK_EXISTS_FS) { // Assert that the source file actually exists. 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'); - if (($realpath = drupal_realpath($original_source)) !== FALSE) { - watchdog('file', 'File %file (%realpath) could not be copied because it does not exist.', array('%file' => $original_source, '%realpath' => $realpath)); + 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' => $source)), 'error'); + if (($realpath = drupal_realpath($source)) !== FALSE) { + watchdog('file', 'File %file (%realpath) could not be copied because it does not exist.', array('%file' => $source, '%realpath' => $realpath)); } else { - watchdog('file', 'File %file could not be copied because it does not exist.', array('%file' => $original_source)); + watchdog('file', 'File %file could not be copied because it does not exist.', array('%file' => $source)); } return FALSE; } @@ -647,31 +644,30 @@ function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXIST $destination = file_build_uri(drupal_basename($source)); } - - // Prepare the destination directory. - if (file_prepare_directory($destination)) { - // The destination is already a directory, so append the source basename. - $destination = file_stream_wrapper_uri_normalize($destination . '/' . drupal_basename($source)); + // Determine the directory name. + if (is_dir($destination)) { + $dirname = $destination; + // A URI may already have a trailing slash or look like "public://". + if (substr($destination, -1) != '/') { + $destination .= '/'; + } + $destination .= drupal_basename($source); } else { - // Perhaps $destination is a dir/file? $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' => $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; - } } - // Determine whether we can perform this operation based on overwrite rules. - $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 (%destination)', array('%file' => $original_source, '%destination' => $destination)); + // Prepare the destination directory. + 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' => $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' => $source)), 'error'); return FALSE; } + // Ensure the destination URI is suitable for a new file. + $destination = file_uri_prepare($destination, $check_exists); + // Assert that the source and destination filenames are not the same. $real_source = drupal_realpath($source); $real_destination = drupal_realpath($destination); @@ -707,65 +703,37 @@ function file_build_uri($path) { } /** - * Determines the destination path for a file. - * - * @param $destination - * A string specifying the desired final URI or filepath. - * @param $replace - * Replace behavior when the destination file already exists. - * - FILE_EXISTS_REPLACE - Replace the existing file. - * - FILE_EXISTS_RENAME - Append _{incrementing number} until the filename is - * unique. - * - FILE_EXISTS_ERROR - Do nothing and return FALSE. - * - * @return - * The destination filepath, or FALSE if the file already exists - * and FILE_EXISTS_ERROR is specified. - */ -function file_destination($destination, $replace) { - if (file_exists($destination)) { - switch ($replace) { - case FILE_EXISTS_REPLACE: - // Do nothing here, we want to overwrite the existing file. - break; - - case FILE_EXISTS_RENAME: - $basename = drupal_basename($destination); - $directory = drupal_dirname($destination); - $destination = file_create_filename($basename, $directory); - break; - - case FILE_EXISTS_ERROR: - // Error reporting handled by calling function. - return FALSE; - } - } - return $destination; -} - -/** * Moves a file to a new location without database changes or hook invocation. * + * This is a powerful function that in many ways performs like an advanced + * version of rename(). + * - Checks if $source and $destination are valid and readable/writable. + * - Checks that $source is not equal to $destination; if they are an error + * is reported. + * - Ensures the file is moved to a "free" URI based on the $check_exists + * parameter. + * * @param $source * A string specifying the filepath or URI of the original file. * @param $destination - * A string containing the destination that $source should be moved to. - * This must be a stream wrapper URI. If this value is omitted, Drupal's - * default files scheme will be used, usually "public://". - * @param $replace - * Replace behavior when the destination file already exists: - * - FILE_EXISTS_REPLACE - Replace the existing file. - * - FILE_EXISTS_RENAME - Append _{incrementing number} until the filename is - * unique. - * - FILE_EXISTS_ERROR - Do nothing and return FALSE. + * A URI containing the destination that $source should be moved to. The + * URI may be a bare filepath (without a scheme) and in that case the default + * scheme (file://) will be used. If this value is omitted, Drupal's default + * files scheme will be used, usually "public://". + * @param $check_exists + * The destination URI will be renamed if it already exists by appending + * _{incrementing number} until it is unique. + * - FILE_CHECK_EXISTS_FALSE - Don't check if it exists. + * - FILE_CHECK_EXISTS_FS - Just check the filesystem. + * - FILE_CHECK_EXISTS_DB_FS - Check the database, then the filesystem. * * @return * The URI of the moved file, or FALSE in the event of an error. * * @see file_move() */ -function file_unmanaged_move($source, $destination = NULL, $replace = FILE_EXISTS_RENAME) { - $filepath = file_unmanaged_copy($source, $destination, $replace); +function file_unmanaged_move($source, $destination = NULL, $check_exists = FILE_CHECK_EXISTS_FS) { + $filepath = file_unmanaged_copy($source, $destination, $check_exists); if ($filepath == FALSE || file_unmanaged_delete($source) == FALSE) { return FALSE; } @@ -853,28 +821,33 @@ function file_unmunge_filename($filename) { } /** - * Creates a full file path from a directory and filename. + * Ensures that a URI is suitable for a new file being created. * - * If a file with the specified name already exists, an alternative will be - * used. + * - Replaces unsuitable characters. + * - Ensures the file is copied to a "free" URI based on the $check_exists + * parameter. * - * @param $basename - * String filename - * @param $directory - * String containing the directory or parent URI. + * @param $uri + * Desired URI for a file. + * @param $check_exists + * The URI will be renamed if it already exists by appending + * _{incrementing number} until it is unique. + * - FILE_CHECK_EXISTS_FALSE - Don't check if it exists. + * - FILE_CHECK_EXISTS_FS - Just check the filesystem. + * - FILE_CHECK_EXISTS_DB_FS - Check the database, then the filesystem. * * @return - * File path consisting of $directory and a unique filename based off - * of $basename. + * Suitable URI for a new file. */ -function file_create_filename($basename, $directory) { +function file_uri_prepare($uri, $check_exists = FILE_CHECK_EXISTS_FS) { // Strip control characters (ASCII value < 32). Though these are allowed in // some filesystems, not many applications handle them well. - $basename = preg_replace('/[\x00-\x1F]/u', '_', $basename); + $basename = preg_replace('/[\x00-\x1F]/u', '_', drupal_basename($uri)); if (substr(PHP_OS, 0, 3) == 'WIN') { // These characters are not allowed in Windows filenames $basename = str_replace(array(':', '*', '?', '"', '<', '>', '|'), '_', $basename); } + $directory = drupal_dirname($uri); // A URI or path may already have a trailing slash or look like "public://". if (substr($directory, -1) == '/') { @@ -884,27 +857,61 @@ function file_create_filename($basename, $directory) { $separator = '/'; } - $destination = $directory . $separator . $basename; + // Compose the URI. + $uri = $directory . $separator . $basename; - if (file_exists($destination)) { - // Destination file already exists, generate an alternative. - $pos = strrpos($basename, '.'); - if ($pos !== FALSE) { - $name = substr($basename, 0, $pos); - $ext = substr($basename, $pos); - } - else { - $name = $basename; - $ext = ''; - } + // If required, ensure the URI is unique. + if ($check_exists) { + if (drupal_file_exists($uri, $check_exists == FILE_CHECK_EXISTS_DB_FS)) { + // URI already exists, generate an alternative. + $pos = strrpos($basename, '.'); + if ($pos !== FALSE) { + $name = substr($basename, 0, $pos); + $ext = substr($basename, $pos); + } + else { + $name = $basename; + $ext = ''; + } - $counter = 0; - do { - $destination = $directory . $separator . $name . '_' . $counter++ . $ext; - } while (file_exists($destination)); + $counter = 0; + do { + $uri = $directory . $separator . $name . '_' . $counter++ . $ext; + } while (drupal_file_exists($uri, $check_exists == FILE_CHECK_EXISTS_DB_FS)); + } } - return $destination; + return $uri; +} + +/** + * Checks whether a file exists. + * + * PHP's file_exists() only checks the filesystem, not the database. + * + * @param $uri + * The URI to check. + * @param $check_db + * Boolean TRUE if the database is to be checked. + * + * @return + * Boolean TRUE if the file exists, FALSE otherwise. + * + * @see file_exists() + * @ingroup php_wrappers + */ +function drupal_file_exists($uri, $check_db = FALSE) { + if ($check_db) { + // Check the database first because it is faster. + $result = entity_query('file') + ->condition('uri', $uri) + ->execute(); + if (count($result)) { + return TRUE; + } + } + // Does the file exist in the filesystem? + return file_exists($uri); } /** @@ -1044,12 +1051,6 @@ function file_unmanaged_delete_recursive($path, $callback = NULL) { * A string containing the URI $source should be copied to. * This must be a stream wrapper URI. If this value is omitted, Drupal's * temporary files scheme will be used ("temporary://"). - * @param $replace - * Replace behavior when the destination file already exists: - * - FILE_EXISTS_REPLACE: Replace the existing file. - * - FILE_EXISTS_RENAME: Append _{incrementing number} until the filename is - * unique. - * - FILE_EXISTS_ERROR: Do nothing and return FALSE. * * @return * An object containing the file information if the upload succeeded, FALSE @@ -1061,7 +1062,7 @@ function file_unmanaged_delete_recursive($path, $callback = NULL) { * - source: Path to the file before it is moved. * - destination: Path to the file after it is moved (same as 'uri'). */ -function file_save_upload($source, $validators = array(), $destination = FALSE, $replace = FILE_EXISTS_RENAME) { +function file_save_upload($source, $validators = array(), $destination = NULL) { global $user; static $upload_cache; @@ -1173,13 +1174,8 @@ function file_save_upload($source, $validators = array(), $destination = FALSE, if (substr($destination, -1) != '/') { $destination .= '/'; } - $file->destination = file_destination($destination . $file->filename, $replace); - // If file_destination() returns FALSE then $replace == FILE_EXISTS_ERROR and - // there's an existing file so we need to bail. - if ($file->destination === FALSE) { - drupal_set_message(t('The file %source could not be uploaded because a file by that name already exists in the destination %directory.', array('%source' => $source, '%directory' => $destination)), 'error'); - return FALSE; - } + // Ensure the destination is unique in the database and on disk. + $file->destination = file_uri_prepare($destination . $file->filename, FILE_CHECK_EXISTS_DB_FS); // Add in our check of the the file name length. $validators['file_validate_name_length'] = array(); @@ -1213,15 +1209,6 @@ function file_save_upload($source, $validators = array(), $destination = FALSE, // Set the permissions on the new file. drupal_chmod($file->uri); - // If we are replacing an existing file re-use its database record. - if ($replace == FILE_EXISTS_REPLACE) { - $existing_files = entity_load_multiple_by_properties('file', array('uri' => $file->uri)); - if (count($existing_files)) { - $existing = reset($existing_files); - $file->fid = $existing->fid; - } - } - // If we made it this far it's safe to record this file in the database. $file->save(); // Add file to the cache. @@ -1280,19 +1267,19 @@ function drupal_move_uploaded_file($filename, $uri) { * URI. If no value is provided, a randomized name will be generated and the * file will be saved using Drupal's default files scheme, usually * "public://". - * @param $replace - * Replace behavior when the destination file already exists: - * - FILE_EXISTS_REPLACE - Replace the existing file. - * - FILE_EXISTS_RENAME - Append _{incrementing number} until the filename is - * unique. - * - FILE_EXISTS_ERROR - Do nothing and return FALSE. + * @param $check_exists + * The destination URI will be renamed if it already exists by appending + * _{incrementing number} until it is unique. + * - FILE_CHECK_EXISTS_FALSE - Don't check if it exists. + * - FILE_CHECK_EXISTS_FS - Just check the filesystem. + * - FILE_CHECK_EXISTS_DB_FS - Check the database, then the filesystem. * * @return * A string with the path of the resulting file, or FALSE on error. * * @see file_save_data() */ -function file_unmanaged_save_data($data, $destination = NULL, $replace = FILE_EXISTS_RENAME) { +function file_unmanaged_save_data($data, $destination = NULL, $check_exists = FILE_CHECK_EXISTS_FS) { // Write the data to a temporary file. $temp_name = drupal_tempnam('temporary://', 'file'); if (file_put_contents($temp_name, $data) === FALSE) { @@ -1301,7 +1288,7 @@ function file_unmanaged_save_data($data, $destination = NULL, $replace = FILE_EX } // Move the file to its final destination. - return file_unmanaged_move($temp_name, $destination, $replace); + return file_unmanaged_move($temp_name, $destination, $check_exists); } /** diff --git a/core/modules/color/color.module b/core/modules/color/color.module index 87d0a37..55069da 100644 --- a/core/modules/color/color.module +++ b/core/modules/color/color.module @@ -495,7 +495,7 @@ function _color_rewrite_stylesheet($theme, &$info, &$paths, $palette, $style) { * Saves the rewritten stylesheet to disk. */ function _color_save_stylesheet($file, $style, &$paths) { - $filepath = file_unmanaged_save_data($style, $file, FILE_EXISTS_REPLACE); + $filepath = file_unmanaged_save_data($style, $file, FILE_CHECK_EXISTS_FALSE); $paths['files'][] = $filepath; // Set standard file permissions for webserver-generated files. diff --git a/core/modules/file/file.module b/core/modules/file/file.module index 913f75a..ca83372 100644 --- a/core/modules/file/file.module +++ b/core/modules/file/file.module @@ -152,8 +152,6 @@ function file_usage() { * - Checks if $source and $destination are valid and readable/writable. * - Checks that $source is not equal to $destination; if they are an error * is reported. - * - If file already exists in $destination either the call will error out, - * replace the file or rename the file based on the $replace parameter. * - Adds the new file to the files database. If the source file is a * temporary file, the resulting file will also be a temporary file. See * file_save_upload() for details on temporary files. @@ -162,15 +160,6 @@ function file_usage() { * A file entity. * @param $destination * A string containing the destination that $source should be copied to. - * This must be a stream wrapper URI. - * @param $replace - * Replace behavior when the destination file already exists: - * - FILE_EXISTS_REPLACE - Replace the existing file. If a managed file with - * the destination name exists then its database entry will be updated. If - * no database entry is found then a new one will be created. - * - FILE_EXISTS_RENAME - Append _{incrementing number} until the filename is - * unique. - * - FILE_EXISTS_ERROR - Do nothing and return FALSE. * * @return * File object if the copy is successful, or FALSE in the event of an error. @@ -178,44 +167,25 @@ function file_usage() { * @see file_unmanaged_copy() * @see hook_file_copy() */ -function file_copy(File $source, $destination = NULL, $replace = FILE_EXISTS_RENAME) { - if (!file_valid_uri($destination)) { - if (($realpath = drupal_realpath($source->uri)) !== FALSE) { - 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; - } - - if ($uri = file_unmanaged_copy($source->uri, $destination, $replace)) { - $file = clone $source; - $file->fid = NULL; - $file->uri = $uri; - $file->filename = drupal_basename($uri); - // If we are replacing an existing file re-use its database record. - if ($replace == FILE_EXISTS_REPLACE) { - $existing_files = entity_load_multiple_by_properties('file', array('uri' => $uri)); - if (count($existing_files)) { - $existing = reset($existing_files); - $file->fid = $existing->fid; - $file->filename = $existing->filename; - } - } - // If we are renaming around an existing file (rather than a directory), - // use its basename for the filename. - elseif ($replace == FILE_EXISTS_RENAME && is_file($destination)) { - $file->filename = drupal_basename($destination); +function file_copy(File $file, $destination = NULL) { + // Copy the file. + if ($uri = file_unmanaged_copy($file->uri, $destination, FILE_CHECK_EXISTS_DB_FS)) { + // Create a new file entity with the new URI. + $copied_file = clone $file; + $copied_file->fid = NULL; + $copied_file->uri = $uri; + + // If $destination is supplied and is not a directory, extract the filename. + if ($destination && !is_dir($destination)) { + $copied_file->filename = drupal_basename($destination); } - $file->save(); + $copied_file->save(); // Inform modules that the file has been copied. - module_invoke_all('file_copy', $file, $source); + module_invoke_all('file_copy', $copied_file, $file); - return $file; + return $copied_file; } return FALSE; } @@ -227,61 +197,28 @@ function file_copy(File $source, $destination = NULL, $replace = FILE_EXISTS_REN * deleting the original. * - Checks if $source and $destination are valid and readable/writable. * - Performs a file move if $source is not equal to $destination. - * - If file already exists in $destination either the call will error out, - * replace the file or rename the file based on the $replace parameter. - * - Adds the new file to the files database. * * @param Drupal\file\File $source * A file entity. * @param $destination * A string containing the destination that $source should be moved to. * This must be a stream wrapper URI. - * @param $replace - * Replace behavior when the destination file already exists: - * - FILE_EXISTS_REPLACE - Replace the existing file. If a managed file with - * the destination name exists then its database entry will be updated and - * $source->delete() called after invoking hook_file_move(). - * If no database entry is found then the source files record will be - * updated. - * - FILE_EXISTS_RENAME - Append _{incrementing number} until the filename is - * unique. - * - FILE_EXISTS_ERROR - Do nothing and return FALSE. * - * @return Drupal\file\File - * Resulting file entity for success, or FALSE in the event of an error. + * @return + * Boolean TRUE if the file move was successful, FALSE otherwise. * * @see file_unmanaged_move() * @see hook_file_move() */ -function file_move(File $source, $destination = NULL, $replace = FILE_EXISTS_RENAME) { - if (!file_valid_uri($destination)) { - if (($realpath = drupal_realpath($source->uri)) !== FALSE) { - 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; - } - - if ($uri = file_unmanaged_move($source->uri, $destination, $replace)) { - $delete_source = FALSE; - - $file = clone $source; +function file_move(File $file, $destination = NULL) { + // Move the file. + if ($uri = file_unmanaged_move($file->uri, $destination, FILE_CHECK_EXISTS_DB_FS)) { + // Update the entity with the new URI. + $source = clone $file; $file->uri = $uri; - // If we are replacing an existing file re-use its database record. - if ($replace == FILE_EXISTS_REPLACE) { - $existing_files = entity_load_multiple_by_properties('file', array('uri' => $uri)); - if (count($existing_files)) { - $existing = reset($existing_files); - $delete_source = TRUE; - $file->fid = $existing->fid; - } - } - // If we are renaming around an existing file (rather than a directory), - // use its basename for the filename. - elseif ($replace == FILE_EXISTS_RENAME && is_file($destination)) { + + // If $destination is supplied and is not a directory, extract the filename. + if ($destination && !is_dir($destination)) { $file->filename = drupal_basename($destination); } @@ -290,12 +227,7 @@ function file_move(File $source, $destination = NULL, $replace = FILE_EXISTS_REN // Inform modules that the file has been moved. module_invoke_all('file_move', $file, $source); - // Delete the original if it's not in use elsewhere. - if ($delete_source && !file_usage()->listUsage($source)) { - $source->delete(); - } - - return $file; + return TRUE; } return FALSE; } @@ -504,51 +436,25 @@ function file_validate_image_resolution(File $file, $maximum_dimensions = 0, $mi * A string containing the destination URI. This must be a stream wrapper URI. * If no value is provided, a randomized name will be generated and the file * will be saved using Drupal's default files scheme, usually "public://". - * @param $replace - * Replace behavior when the destination file already exists: - * - FILE_EXISTS_REPLACE - Replace the existing file. If a managed file with - * the destination name exists then its database entry will be updated. If - * no database entry is found then a new one will be created. - * - FILE_EXISTS_RENAME - Append _{incrementing number} until the filename is - * unique. - * - FILE_EXISTS_ERROR - Do nothing and return FALSE. * * @return Drupal\file\File * A file entity, or FALSE on error. * * @see file_unmanaged_save_data() */ -function file_save_data($data, $destination = NULL, $replace = FILE_EXISTS_RENAME) { +function file_save_data($data, $destination = NULL) { global $user; - if (empty($destination)) { - $destination = file_default_scheme() . '://'; - } - if (!file_valid_uri($destination)) { - watchdog('file', 'The data could not be saved because the destination %destination is invalid. This may be caused by improper use of file_save_data() or a missing stream wrapper.', array('%destination' => $destination)); - drupal_set_message(t('The data could not be saved because the destination is invalid. More information is available in the system log.'), 'error'); - return FALSE; - } - - if ($uri = file_unmanaged_save_data($data, $destination, $replace)) { + if ($uri = file_unmanaged_save_data($data, $destination, FILE_CHECK_EXISTS_DB_FS)) { // Create a file entity. $file = entity_create('file', array( 'uri' => $uri, 'uid' => $user->uid, 'status' => FILE_STATUS_PERMANENT, )); - // If we are replacing an existing file re-use its database record. - if ($replace == FILE_EXISTS_REPLACE) { - $existing_files = entity_load_multiple_by_properties('file', array('uri' => $uri)); - if (count($existing_files)) { - $existing = reset($existing_files); - $file->fid = $existing->fid; - $file->filename = $existing->filename; - } - } - // If we are renaming around an existing file (rather than a directory), - // use its basename for the filename. - elseif ($replace == FILE_EXISTS_RENAME && is_file($destination)) { + + // If $destination is supplied and is not a directory, extract the filename. + if ($destination && !is_dir($destination)) { $file->filename = drupal_basename($destination); } diff --git a/core/modules/file/lib/Drupal/file/Tests/CopyTest.php b/core/modules/file/lib/Drupal/file/Tests/CopyTest.php index 7f20c9f..54377f3 100644 --- a/core/modules/file/lib/Drupal/file/Tests/CopyTest.php +++ b/core/modules/file/lib/Drupal/file/Tests/CopyTest.php @@ -27,9 +27,7 @@ function testNormal() { $source = $this->createFile(NULL, $contents); $desired_uri = 'public://' . $this->randomName(); - // Clone the object so we don't have to worry about the function changing - // our reference copy. - $result = file_copy(clone $source, $desired_uri, FILE_EXISTS_ERROR); + $result = file_copy($source, $desired_uri); // Check the return status and that the contents changed. $this->assertTrue($result, t('File copied successfully.')); @@ -40,6 +38,7 @@ function testNormal() { $this->assertDifferentFile($source, $result); $this->assertEqual($result->uri, $desired_uri, t('The copied file entity has the desired filepath.')); + $this->assertEqual($result->filename, drupal_basename($desired_uri), t('The copied file entity has the correct filename.')); $this->assertTrue(file_exists($source->uri), t('The original file still exists.')); $this->assertTrue(file_exists($result->uri), t('The copied file exists.')); @@ -58,14 +57,13 @@ function testExistingRename() { $target = $this->createFile(); $this->assertDifferentFile($source, $target); - // Clone the object so we don't have to worry about the function changing - // our reference copy. - $result = file_copy(clone $source, $target->uri, FILE_EXISTS_RENAME); + $result = file_copy($source, $target->uri); // Check the return status and that the contents changed. $this->assertTrue($result, t('File copied successfully.')); $this->assertEqual($contents, file_get_contents($result->uri), t('Contents of file were copied correctly.')); $this->assertNotEqual($result->uri, $source->uri, t('Returned file path has changed from the original.')); + $this->assertEqual($result->filename, drupal_basename($target->uri), t('The copied file entity has the correct filename.')); // Check that the correct hooks were called. $this->assertFileHooksCalled(array('copy', 'insert')); @@ -88,27 +86,25 @@ function testExistingRename() { $this->assertDifferentFile($loaded_source, $loaded_result); } - /** - * Test replacement when copying over a file that already exists. - */ - function testExistingReplace() { + function testDBExistingRename() { // Setup a file to overwrite. $contents = $this->randomName(10); $source = $this->createFile(NULL, $contents); $target = $this->createFile(); + file_unmanaged_delete($target->uri); + $this->assertFalse(file_exists($target->uri), t('Target file has been deleted from filesystem.')); $this->assertDifferentFile($source, $target); - // Clone the object so we don't have to worry about the function changing - // our reference copy. - $result = file_copy(clone $source, $target->uri, FILE_EXISTS_REPLACE); + $result = file_copy($source, $target->uri); // Check the return status and that the contents changed. $this->assertTrue($result, t('File copied successfully.')); - $this->assertEqual($contents, file_get_contents($result->uri), t('Contents of file were overwritten.')); - $this->assertDifferentFile($source, $result); + $this->assertEqual($contents, file_get_contents($result->uri), t('Contents of file were copied correctly.')); + $this->assertNotEqual($result->uri, $source->uri, t('Returned file path has changed from the original.')); + $this->assertEqual($result->filename, drupal_basename($target->uri), t('The copied file entity has the correct filename.')); // Check that the correct hooks were called. - $this->assertFileHooksCalled(array('load', 'copy', 'update')); + $this->assertFileHooksCalled(array('copy', 'insert')); // Load all the affected files to check the changes that actually made it // to the database. @@ -122,32 +118,43 @@ function testExistingReplace() { // Verify that what was returned is what's in the database. $this->assertFileUnchanged($result, $loaded_result); - // Target file was reused for the result. - $this->assertFileUnchanged($loaded_target, $loaded_result); + // Make sure we end up with three distinct files afterwards. + $this->assertDifferentFile($loaded_source, $loaded_target); + $this->assertDifferentFile($loaded_target, $loaded_result); + $this->assertDifferentFile($loaded_source, $loaded_result); } - /** - * Test that copying over an existing file fails when FILE_EXISTS_ERROR is - * specified. - */ - function testExistingError() { + function testFSExistingRename() { + // Setup a file to overwrite. $contents = $this->randomName(10); - $source = $this->createFile(); - $target = $this->createFile(NULL, $contents); - $this->assertDifferentFile($source, $target); + $source = $this->createFile(NULL, $contents); + $target_uri = file_unmanaged_copy($source->uri, 'public://' . $this->randomName()); + $this->assertTrue(file_exists($target_uri), t('Target file exists on filesystem.')); - // Clone the object so we don't have to worry about the function changing - // our reference copy. - $result = file_copy(clone $source, $target->uri, FILE_EXISTS_ERROR); + $result = file_copy($source, $target_uri); - // Check the return status and that the contents were not changed. - $this->assertFalse($result, t('File copy failed.')); - $this->assertEqual($contents, file_get_contents($target->uri), t('Contents of file were not altered.')); + // Check the return status and that the contents changed. + $this->assertTrue($result, t('File copied successfully.')); + $this->assertEqual($contents, file_get_contents($result->uri), t('Contents of file were copied correctly.')); + $this->assertNotEqual($result->uri, $source->uri, t('Returned file path has changed from the original.')); + $this->assertNotEqual($result->uri, $target_uri, t('File on filesystem was not overwritten.')); + $this->assertEqual($result->filename, drupal_basename($target_uri), t('The copied file entity has the correct filename.')); // Check that the correct hooks were called. - $this->assertFileHooksCalled(array()); + $this->assertFileHooksCalled(array('copy', 'insert')); + + // Load all the affected files to check the changes that actually made it + // to the database. + $loaded_source = file_load($source->fid, TRUE); + $loaded_result = file_load($result->fid, TRUE); + + // Verify that the source file wasn't changed. + $this->assertFileUnchanged($source, $loaded_source); - $this->assertFileUnchanged($source, file_load($source->fid, TRUE)); - $this->assertFileUnchanged($target, file_load($target->fid, TRUE)); + // Verify that what was returned is what's in the database. + $this->assertFileUnchanged($result, $loaded_result); + + // Make sure we end up with distinct files afterwards. + $this->assertDifferentFile($loaded_source, $loaded_result); } } diff --git a/core/modules/file/lib/Drupal/file/Tests/DownloadTest.php b/core/modules/file/lib/Drupal/file/Tests/DownloadTest.php index 0763507..aaa5389 100644 --- a/core/modules/file/lib/Drupal/file/Tests/DownloadTest.php +++ b/core/modules/file/lib/Drupal/file/Tests/DownloadTest.php @@ -128,7 +128,7 @@ private function checkUrl($scheme, $directory, $filename, $expected_url) { // Convert $filename to a valid filename, i.e. strip characters not // supported by the filesystem, and create the file in the specified // directory. - $filepath = file_create_filename($filename, $directory); + $filepath = file_uri_prepare($directory . '/' . $filename); $directory_uri = $scheme . '://' . dirname($filepath); file_prepare_directory($directory_uri, FILE_CREATE_DIRECTORY); $file = $this->createFile($filepath, NULL, $scheme); diff --git a/core/modules/file/lib/Drupal/file/Tests/MoveTest.php b/core/modules/file/lib/Drupal/file/Tests/MoveTest.php index 15868b1..7ed4d41 100644 --- a/core/modules/file/lib/Drupal/file/Tests/MoveTest.php +++ b/core/modules/file/lib/Drupal/file/Tests/MoveTest.php @@ -24,29 +24,29 @@ public static function getInfo() { */ function testNormal() { $contents = $this->randomName(10); - $source = $this->createFile(NULL, $contents); + $file = $this->createFile(NULL, $contents); $desired_filepath = 'public://' . $this->randomName(); - // Clone the object so we don't have to worry about the function changing - // our reference copy. - $result = file_move(clone $source, $desired_filepath, FILE_EXISTS_ERROR); + $old_file = clone $file; + $result = file_move($file, $desired_filepath); // Check the return status and that the contents changed. $this->assertTrue($result, t('File moved successfully.')); - $this->assertFalse(file_exists($source->uri)); - $this->assertEqual($contents, file_get_contents($result->uri), t('Contents of file correctly written.')); + $this->assertEqual($file->filename, drupal_basename($desired_filepath), t('The moved file entity has the new filename.')); + $this->assertFalse(file_exists($old_file->uri)); + $this->assertEqual($contents, file_get_contents($file->uri), t('Contents of file correctly written.')); // Check that the correct hooks were called. $this->assertFileHooksCalled(array('move', 'load', 'update')); // Make sure we got the same file back. - $this->assertEqual($source->fid, $result->fid, t("Source file id's' %fid is unchanged after move.", array('%fid' => $source->fid))); + $this->assertEqual($file->fid, $old_file->fid, t("File id's' %fid is unchanged after move.", array('%fid' => $file->fid))); // Reload the file from the database and check that the changes were // actually saved. - $loaded_file = file_load($result->fid, TRUE); + $loaded_file = file_load($file->fid, TRUE); $this->assertTrue($loaded_file, t('File can be loaded from the database.')); - $this->assertFileUnchanged($result, $loaded_file); + $this->assertFileUnchanged($file, $loaded_file); } /** @@ -55,114 +55,98 @@ function testNormal() { function testExistingRename() { // Setup a file to overwrite. $contents = $this->randomName(10); - $source = $this->createFile(NULL, $contents); + $file = $this->createFile(NULL, $contents); $target = $this->createFile(); - $this->assertDifferentFile($source, $target); + $this->assertDifferentFile($file, $target); - // Clone the object so we don't have to worry about the function changing - // our reference copy. - $result = file_move(clone $source, $target->uri, FILE_EXISTS_RENAME); + $old_file = clone $file; + $result = file_move($file, $target->uri); // Check the return status and that the contents changed. $this->assertTrue($result, t('File moved successfully.')); - $this->assertFalse(file_exists($source->uri)); - $this->assertEqual($contents, file_get_contents($result->uri), t('Contents of file correctly written.')); + $this->assertEqual($file->filename, drupal_basename($target->uri), t('The moved file entity has the new filename.')); + $this->assertFalse(file_exists($old_file->uri)); + $this->assertEqual($contents, file_get_contents($file->uri), t('Contents of file correctly written.')); // Check that the correct hooks were called. $this->assertFileHooksCalled(array('move', 'load', 'update')); // Compare the returned value to what made it into the database. - $this->assertFileUnchanged($result, file_load($result->fid, TRUE)); - // The target file should not have been altered. - $this->assertFileUnchanged($target, file_load($target->fid, TRUE)); - // Make sure we end up with two distinct files afterwards. - $this->assertDifferentFile($target, $result); + $this->assertFileUnchanged($file, file_load($file->fid, TRUE)); // Compare the source and results. - $loaded_source = file_load($source->fid, TRUE); - $this->assertEqual($loaded_source->fid, $result->fid, t("Returned file's id matches the source.")); - $this->assertNotEqual($loaded_source->uri, $source->uri, t("Returned file path has changed from the original.")); + $this->assertEqual($old_file->fid, $file->fid, t("Returned file's id matches the source.")); + $this->assertNotEqual($old_file->uri, $file->uri, t("Returned file path has changed from the original.")); + + // Compare the target URI and result URI. + $this->assertNotEqual($target->uri, $file->uri, t("File on filesystem was not overwritten.")); } /** - * Test replacement when moving onto a file that already exists. + * Test renaming when moving onto a file that already exists. */ - function testExistingReplace() { + function testDBExistingRename() { // Setup a file to overwrite. $contents = $this->randomName(10); - $source = $this->createFile(NULL, $contents); + $file = $this->createFile(NULL, $contents); $target = $this->createFile(); - $this->assertDifferentFile($source, $target); + file_unmanaged_delete($target->uri); + $this->assertFalse(file_exists($target->uri), t('Target file has been deleted from filesystem.')); + $this->assertDifferentFile($file, $target); - // Clone the object so we don't have to worry about the function changing - // our reference copy. - $result = file_move(clone $source, $target->uri, FILE_EXISTS_REPLACE); + $old_file = clone $file; + $result = file_move($file, $target->uri); - // Look at the results. - $this->assertEqual($contents, file_get_contents($result->uri), t('Contents of file were overwritten.')); - $this->assertFalse(file_exists($source->uri)); + // Check the return status and that the contents changed. $this->assertTrue($result, t('File moved successfully.')); + $this->assertEqual($file->filename, drupal_basename($target->uri), t('The moved file entity has the new filename.')); + $this->assertFalse(file_exists($old_file->uri)); + $this->assertEqual($contents, file_get_contents($file->uri), t('Contents of file correctly written.')); // Check that the correct hooks were called. - $this->assertFileHooksCalled(array('move', 'update', 'delete', 'load')); + $this->assertFileHooksCalled(array('move', 'load', 'update')); - // Reload the file from the database and check that the changes were - // actually saved. - $loaded_result = file_load($result->fid, TRUE); - $this->assertFileUnchanged($result, $loaded_result); - // Check that target was re-used. - $this->assertSameFile($target, $loaded_result); - // Source and result should be totally different. - $this->assertDifferentFile($source, $loaded_result); + // Compare the returned value to what made it into the database. + $this->assertFileUnchanged($file, file_load($file->fid, TRUE)); + + // Compare the source and results. + $this->assertEqual($old_file->fid, $file->fid, t("Returned file's id matches the source.")); + $this->assertNotEqual($old_file->uri, $file->uri, t("Returned file path has changed from the original.")); + + // Compare the target URI and result URI. + $this->assertNotEqual($target->uri, $file->uri, t("File on filesystem was not overwritten.")); } /** - * Test replacement when moving onto itself. + * Test renaming when moving onto a file that already exists. */ - function testExistingReplaceSelf() { + function testFSExistingRename() { // Setup a file to overwrite. $contents = $this->randomName(10); - $source = $this->createFile(NULL, $contents); + $file = $this->createFile(NULL, $contents); + $target_uri = file_unmanaged_copy($file->uri, 'public://' . $this->randomName()); + $this->assertTrue(file_exists($target_uri), t('Target file exists on filesystem.')); - // Copy the file over itself. Clone the object so we don't have to worry - // about the function changing our reference copy. - $result = file_move(clone $source, $source->uri, FILE_EXISTS_REPLACE); - $this->assertFalse($result, t('File move failed.')); - $this->assertEqual($contents, file_get_contents($source->uri), t('Contents of file were not altered.')); + $old_file = clone $file; + $result = file_move($file, $target_uri); - // Check that no hooks were called while failing. - $this->assertFileHooksCalled(array()); + // Check the return status and that the contents changed. + $this->assertTrue($result, t('File moved successfully.')); + $this->assertEqual($file->filename, drupal_basename($target_uri), t('The moved file entity has the new filename.')); + $this->assertFalse(file_exists($old_file->uri)); + $this->assertEqual($contents, file_get_contents($file->uri), t('Contents of file correctly written.')); - // Load the file from the database and make sure it is identical to what - // was returned. - $this->assertFileUnchanged($source, file_load($source->fid, TRUE)); - } + // Check that the correct hooks were called. + $this->assertFileHooksCalled(array('move', 'load', 'update')); - /** - * Test that moving onto an existing file fails when FILE_EXISTS_ERROR is - * specified. - */ - function testExistingError() { - $contents = $this->randomName(10); - $source = $this->createFile(); - $target = $this->createFile(NULL, $contents); - $this->assertDifferentFile($source, $target); - - // Clone the object so we don't have to worry about the function changing - // our reference copy. - $result = file_move(clone $source, $target->uri, FILE_EXISTS_ERROR); - - // Check the return status and that the contents did not change. - $this->assertFalse($result, t('File move failed.')); - $this->assertTrue(file_exists($source->uri)); - $this->assertEqual($contents, file_get_contents($target->uri), t('Contents of file were not altered.')); - - // Check that no hooks were called while failing. - $this->assertFileHooksCalled(array()); - - // Load the file from the database and make sure it is identical to what - // was returned. - $this->assertFileUnchanged($source, file_load($source->fid, TRUE)); - $this->assertFileUnchanged($target, file_load($target->fid, TRUE)); + // Compare the returned value to what made it into the database. + $this->assertFileUnchanged($file, file_load($file->fid, TRUE)); + + // Compare the source and results. + $this->assertEqual($old_file->fid, $file->fid, t("Returned file's id matches the source.")); + $this->assertNotEqual($old_file->uri, $file->uri, t("Returned file path has changed from the original.")); + + // Compare the target URI and result URI. + $this->assertNotEqual($target_uri, $file->uri, t("File on filesystem was not overwritten.")); } } diff --git a/core/modules/file/lib/Drupal/file/Tests/SaveDataTest.php b/core/modules/file/lib/Drupal/file/Tests/SaveDataTest.php index ecdb201..b9cf120 100644 --- a/core/modules/file/lib/Drupal/file/Tests/SaveDataTest.php +++ b/core/modules/file/lib/Drupal/file/Tests/SaveDataTest.php @@ -54,7 +54,7 @@ function testWithFilename() { $this->assertTrue($result, t('Unnamed file saved correctly.')); $this->assertEqual('public', file_uri_scheme($result->uri), t("File was placed in Drupal's files directory.")); - $this->assertEqual($filename, drupal_basename($result->uri), t('File was named correctly.')); + $this->assertEqual($result->filename, $filename, t('File was named correctly.')); $this->assertEqual($contents, file_get_contents($result->uri), t('Contents of the file are correct.')); $this->assertEqual($result->filemime, 'text/plain', t('A MIME type was set.')); $this->assertEqual($result->status, FILE_STATUS_PERMANENT, t("The file's status was set to permanent.")); @@ -74,11 +74,11 @@ function testExistingRename() { $existing = $this->createFile(); $contents = $this->randomName(8); - $result = file_save_data($contents, $existing->uri, FILE_EXISTS_RENAME); + $result = file_save_data($contents, $existing->uri); $this->assertTrue($result, t("File saved successfully.")); $this->assertEqual('public', file_uri_scheme($result->uri), t("File was placed in Drupal's files directory.")); - $this->assertEqual($result->filename, $existing->filename, t("Filename was set to the basename of the source, rather than that of the renamed file.")); + $this->assertEqual($result->filename, drupal_basename($existing->uri), t("Filename was set to the basename of the desired URI, rather than that of the renamed file.")); $this->assertEqual($contents, file_get_contents($result->uri), t("Contents of the file are correct.")); $this->assertEqual($result->filemime, 'application/octet-stream', t("A MIME type was set.")); $this->assertEqual($result->status, FILE_STATUS_PERMANENT, t("The file's status was set to permanent.")); @@ -93,50 +93,4 @@ function testExistingRename() { // Verify that was returned is what's in the database. $this->assertFileUnchanged($result, file_load($result->fid, TRUE)); } - - /** - * Test file_save_data() when replacing an existing file. - */ - function testExistingReplace() { - // Setup a file to overwrite. - $existing = $this->createFile(); - $contents = $this->randomName(8); - - $result = file_save_data($contents, $existing->uri, FILE_EXISTS_REPLACE); - $this->assertTrue($result, t('File saved successfully.')); - - $this->assertEqual('public', file_uri_scheme($result->uri), t("File was placed in Drupal's files directory.")); - $this->assertEqual($result->filename, $existing->filename, t('Filename was set to the basename of the existing file, rather than preserving the original name.')); - $this->assertEqual($contents, file_get_contents($result->uri), t('Contents of the file are correct.')); - $this->assertEqual($result->filemime, 'application/octet-stream', t('A MIME type was set.')); - $this->assertEqual($result->status, FILE_STATUS_PERMANENT, t("The file's status was set to permanent.")); - - // Check that the correct hooks were called. - $this->assertFileHooksCalled(array('load', 'update')); - - // Verify that the existing file was re-used. - $this->assertSameFile($existing, $result); - - // Verify that what was returned is what's in the database. - $this->assertFileUnchanged($result, file_load($result->fid, TRUE)); - } - - /** - * Test that file_save_data() fails overwriting an existing file. - */ - function testExistingError() { - $contents = $this->randomName(8); - $existing = $this->createFile(NULL, $contents); - - // Check the overwrite error. - $result = file_save_data('asdf', $existing->uri, FILE_EXISTS_ERROR); - $this->assertFalse($result, t('Overwriting a file fails when FILE_EXISTS_ERROR is specified.')); - $this->assertEqual($contents, file_get_contents($existing->uri), t('Contents of existing file were unchanged.')); - - // Check that no hooks were called while failing. - $this->assertFileHooksCalled(array()); - - // Ensure that the existing file wasn't overwritten. - $this->assertFileUnchanged($existing, file_load($existing->fid, TRUE)); - } } diff --git a/core/modules/file/lib/Drupal/file/Tests/SaveUploadTest.php b/core/modules/file/lib/Drupal/file/Tests/SaveUploadTest.php index c2adbd6..37f9581 100644 --- a/core/modules/file/lib/Drupal/file/Tests/SaveUploadTest.php +++ b/core/modules/file/lib/Drupal/file/Tests/SaveUploadTest.php @@ -50,9 +50,7 @@ function setUp() { $this->maxFidBefore = db_query('SELECT MAX(fid) AS fid FROM {file_managed}')->fetchField(); - // Upload with replace to guarantee there's something there. $edit = array( - 'file_test_replace' => FILE_EXISTS_REPLACE, 'files[file_test_upload]' => drupal_realpath($this->image->uri), ); $this->drupalPost('file-test/upload', $edit, t('Submit')); @@ -125,7 +123,6 @@ function testHandleExtension() { // file_save_upload() to only allow ".foo". $extensions = 'foo'; $edit = array( - 'file_test_replace' => FILE_EXISTS_REPLACE, 'files[file_test_upload]' => drupal_realpath($this->image->uri), 'extensions' => $extensions, ); @@ -145,7 +142,6 @@ function testHandleExtension() { $extensions = 'foo ' . $this->image_extension; // Now tell file_save_upload() to allow the extension of our test image. $edit = array( - 'file_test_replace' => FILE_EXISTS_REPLACE, 'files[file_test_upload]' => drupal_realpath($this->image->uri), 'extensions' => $extensions, ); @@ -156,14 +152,13 @@ function testHandleExtension() { $this->assertRaw(t('You WIN!'), t('Found the success message.')); // Check that the correct hooks were called. - $this->assertFileHooksCalled(array('validate', 'load', 'update')); + $this->assertFileHooksCalled(array('validate', 'insert')); // Reset the hook counters. file_test_reset(); // Now tell file_save_upload() to allow any extension. $edit = array( - 'file_test_replace' => FILE_EXISTS_REPLACE, 'files[file_test_upload]' => drupal_realpath($this->image->uri), 'allow_all_extensions' => TRUE, ); @@ -173,7 +168,7 @@ function testHandleExtension() { $this->assertRaw(t('You WIN!'), t('Found the success message.')); // Check that the correct hooks were called. - $this->assertFileHooksCalled(array('validate', 'load', 'update')); + $this->assertFileHooksCalled(array('validate', 'insert')); } /** @@ -183,7 +178,6 @@ function testHandleDangerousFile() { // Allow the .php extension and make sure it gets renamed to .txt for // safety. Also check to make sure its MIME type was changed. $edit = array( - 'file_test_replace' => FILE_EXISTS_REPLACE, 'files[file_test_upload]' => drupal_realpath($this->phpfile->uri), 'is_image_file' => FALSE, 'extensions' => 'php', @@ -224,7 +218,7 @@ function testHandleDangerousFile() { function testHandleFileMunge() { // Ensure insecure uploads are disabled for this test. variable_set('allow_insecure_uploads', 0); - $this->image = file_move($this->image, $this->image->uri . '.foo.' . $this->image_extension); + file_move($this->image, $this->image->uri . '.foo.' . $this->image_extension); // Reset the hook counters to get rid of the 'move' we just called. file_test_reset(); @@ -235,7 +229,7 @@ function testHandleFileMunge() { 'extensions' => $extensions, ); - $munged_filename = $this->image->filename; + $munged_filename = basename($this->image->uri); $munged_filename = substr($munged_filename, 0, strrpos($munged_filename, '.')); $munged_filename .= '_.' . $this->image_extension; @@ -272,7 +266,6 @@ function testHandleFileMunge() { */ function testExistingRename() { $edit = array( - 'file_test_replace' => FILE_EXISTS_RENAME, 'files[file_test_upload]' => drupal_realpath($this->image->uri) ); $this->drupalPost('file-test/upload', $edit, t('Submit')); @@ -284,42 +277,22 @@ function testExistingRename() { } /** - * Test replacement when uploading over a file that already exists. - */ - function testExistingReplace() { - $edit = array( - 'file_test_replace' => FILE_EXISTS_REPLACE, - 'files[file_test_upload]' => drupal_realpath($this->image->uri) - ); - $this->drupalPost('file-test/upload', $edit, t('Submit')); - $this->assertResponse(200, t('Received a 200 response for posted test file.')); - $this->assertRaw(t('You WIN!'), t('Found the success message.')); - - // Check that the correct hooks were called. - $this->assertFileHooksCalled(array('validate', 'load', 'update')); - } - - /** - * Test for failure when uploading over a file that already exists. - */ - function testExistingError() { - $edit = array( - 'file_test_replace' => FILE_EXISTS_ERROR, - 'files[file_test_upload]' => drupal_realpath($this->image->uri) - ); - $this->drupalPost('file-test/upload', $edit, t('Submit')); - $this->assertResponse(200, t('Received a 200 response for posted test file.')); - $this->assertRaw(t('Epic upload FAIL!'), t('Found the failure message.')); - - // Check that the no hooks were called while failing. - $this->assertFileHooksCalled(array()); - } - - /** * Test for no failures when not uploading a file. */ function testNoUpload() { $this->drupalPost('file-test/upload', array(), t('Submit')); $this->assertNoRaw(t('Epic upload FAIL!'), t('Failure message not found.')); } + + /** + * Test renaming when uploading over a file that exists in {file_managed} but + * not on disk. + */ + function testZombieRename() { + $uri = 'temporary://' . drupal_basename($this->image->uri); + $this->assertTrue(file_exists($uri)); + file_unmanaged_delete($uri); + $this->assertFalse(file_exists($uri)); + $this->testExistingRename(); + } } diff --git a/core/modules/file/tests/file_test/file_test.module b/core/modules/file/tests/file_test/file_test.module index 1d924de..cebafaa 100644 --- a/core/modules/file/tests/file_test/file_test.module +++ b/core/modules/file/tests/file_test/file_test.module @@ -58,16 +58,6 @@ function _file_test_form($form, &$form_state) { '#type' => 'file', '#title' => t('Upload a file'), ); - $form['file_test_replace'] = array( - '#type' => 'select', - '#title' => t('Replace existing image'), - '#options' => array( - FILE_EXISTS_RENAME => t('Appends number until name is unique'), - FILE_EXISTS_REPLACE => t('Replace the existing file'), - FILE_EXISTS_ERROR => t('Fail with an error'), - ), - '#default_value' => FILE_EXISTS_RENAME, - ); $form['file_subdir'] = array( '#type' => 'textfield', '#title' => t('Subdirectory for test file'), @@ -126,7 +116,7 @@ function _file_test_form_submit(&$form, &$form_state) { $validators['file_validate_extensions'] = array($form_state['values']['extensions']); } - $file = file_save_upload('file_test_upload', $validators, $destination, $form_state['values']['file_test_replace']); + $file = file_save_upload('file_test_upload', $validators, $destination); if ($file) { $form_state['values']['file_test_upload'] = $file; drupal_set_message(t('File @filepath was uploaded.', array('@filepath' => $file->uri))); diff --git a/core/modules/image/lib/Drupal/image/Tests/ImageDimensionsTest.php b/core/modules/image/lib/Drupal/image/Tests/ImageDimensionsTest.php index e7552d2..5da7f08 100644 --- a/core/modules/image/lib/Drupal/image/Tests/ImageDimensionsTest.php +++ b/core/modules/image/lib/Drupal/image/Tests/ImageDimensionsTest.php @@ -38,7 +38,7 @@ function testImageDimensions() { // Create a working copy of the file. $files = $this->drupalGetTestFiles('image'); $file = reset($files); - $original_uri = file_unmanaged_copy($file->uri, 'public://', FILE_EXISTS_RENAME); + $original_uri = file_unmanaged_copy($file->uri); // Create a style. $style = entity_create('image_style', array('name' => 'test', 'label' => 'Test')); diff --git a/core/modules/image/lib/Drupal/image/Tests/ImageStylesPathAndUrlTest.php b/core/modules/image/lib/Drupal/image/Tests/ImageStylesPathAndUrlTest.php index ebb71ba..0ad9f77 100644 --- a/core/modules/image/lib/Drupal/image/Tests/ImageStylesPathAndUrlTest.php +++ b/core/modules/image/lib/Drupal/image/Tests/ImageStylesPathAndUrlTest.php @@ -103,7 +103,7 @@ function _testImageStyleUrlAndPath($scheme, $clean_url = TRUE) { $files = $this->drupalGetTestFiles('image'); $file = array_shift($files); $image_info = image_get_info($file->uri); - $original_uri = file_unmanaged_copy($file->uri, $scheme . '://', FILE_EXISTS_RENAME); + $original_uri = file_unmanaged_copy($file->uri, $scheme . '://'); // Let the image_module_test module know about this file, so it can claim // ownership in hook_file_download(). state()->set('image.test_file_download', $original_uri); @@ -145,7 +145,7 @@ function _testImageStyleUrlAndPath($scheme, $clean_url = TRUE) { // Repeat this with a different file that we do not have access to and // make sure that access is denied. $file_noaccess = array_shift($files); - $original_uri_noaccess = file_unmanaged_copy($file_noaccess->uri, $scheme . '://', FILE_EXISTS_RENAME); + $original_uri_noaccess = file_unmanaged_copy($file_noaccess->uri, $scheme . '://'); $generated_uri_noaccess = $scheme . '://styles/' . $this->style_name . '/' . $scheme . '/'. drupal_basename($original_uri_noaccess); $this->assertFalse(file_exists($generated_uri_noaccess), 'Generated file does not exist.'); $generate_url_noaccess = image_style_url($this->style_name, $original_uri_noaccess); diff --git a/core/modules/image/lib/Drupal/image/Tests/ImageThemeFunctionTest.php b/core/modules/image/lib/Drupal/image/Tests/ImageThemeFunctionTest.php index cd00cb1..c8363de 100644 --- a/core/modules/image/lib/Drupal/image/Tests/ImageThemeFunctionTest.php +++ b/core/modules/image/lib/Drupal/image/Tests/ImageThemeFunctionTest.php @@ -36,7 +36,7 @@ function testImageFormatterTheme() { // Create an image. $files = $this->drupalGetTestFiles('image'); $file = reset($files); - $original_uri = file_unmanaged_copy($file->uri, 'public://', FILE_EXISTS_RENAME); + $original_uri = file_unmanaged_copy($file->uri); // Create a style. $style = entity_create('image_style', array('name' => 'test', 'label' => 'Test')); @@ -79,7 +79,7 @@ function testImageStyleTheme() { // Create an image. $files = $this->drupalGetTestFiles('image'); $file = reset($files); - $original_uri = file_unmanaged_copy($file->uri, 'public://', FILE_EXISTS_RENAME); + $original_uri = file_unmanaged_copy($file->uri); // Create a style. $style = entity_create('image_style', array('name' => 'image_test', 'label' => 'Test')); diff --git a/core/modules/locale/lib/Drupal/locale/Tests/LocaleExportTest.php b/core/modules/locale/lib/Drupal/locale/Tests/LocaleExportTest.php index 7ae1bf7..c02378a 100644 --- a/core/modules/locale/lib/Drupal/locale/Tests/LocaleExportTest.php +++ b/core/modules/locale/lib/Drupal/locale/Tests/LocaleExportTest.php @@ -41,8 +41,8 @@ function setUp() { $this->drupalLogin($this->admin_user); // Copy test po files to the translations directory. - file_unmanaged_copy(drupal_get_path('module', 'locale') . '/tests/test.de.po', 'translations://', FILE_EXISTS_REPLACE); - file_unmanaged_copy(drupal_get_path('module', 'locale') . '/tests/test.xx.po', 'translations://', FILE_EXISTS_REPLACE); + file_unmanaged_copy(drupal_get_path('module', 'locale') . '/tests/test.de.po', 'translations://'); + file_unmanaged_copy(drupal_get_path('module', 'locale') . '/tests/test.xx.po', 'translations://'); } /** diff --git a/core/modules/locale/lib/Drupal/locale/Tests/LocaleImportFunctionalTest.php b/core/modules/locale/lib/Drupal/locale/Tests/LocaleImportFunctionalTest.php index fd3667c..7d7246f 100644 --- a/core/modules/locale/lib/Drupal/locale/Tests/LocaleImportFunctionalTest.php +++ b/core/modules/locale/lib/Drupal/locale/Tests/LocaleImportFunctionalTest.php @@ -38,8 +38,8 @@ function setUp() { parent::setUp(); // Copy test po files to the translations directory. - file_unmanaged_copy(drupal_get_path('module', 'locale') . '/tests/test.de.po', 'translations://', FILE_EXISTS_REPLACE); - file_unmanaged_copy(drupal_get_path('module', 'locale') . '/tests/test.xx.po', 'translations://', FILE_EXISTS_REPLACE); + file_unmanaged_copy(drupal_get_path('module', 'locale') . '/tests/test.de.po', 'translations://'); + file_unmanaged_copy(drupal_get_path('module', 'locale') . '/tests/test.xx.po', 'translations://'); $this->admin_user = $this->drupalCreateUser(array('administer languages', 'translate interface', 'access administration pages')); $this->drupalLogin($this->admin_user); diff --git a/core/modules/locale/locale.batch.inc b/core/modules/locale/locale.batch.inc index 8d87e48..46e41ce 100644 --- a/core/modules/locale/locale.batch.inc +++ b/core/modules/locale/locale.batch.inc @@ -330,7 +330,7 @@ function locale_translation_batch_fetch_import($project, $langcode, $options, &$ // translations after successfull import. Otherwise the temporary // file is deleted after being imported. if ($import_type == LOCALE_TRANSLATION_DOWNLOADED && variable_get('locale_translate_file_directory', conf_path() . '/files/translations') && isset($source_result->files[LOCALE_TRANSLATION_LOCAL])) { - if (file_unmanaged_move($file->uri, $source_result->files[LOCALE_TRANSLATION_LOCAL]->uri, FILE_EXISTS_REPLACE)) { + if (file_unmanaged_move($file->uri, $source_result->files[LOCALE_TRANSLATION_LOCAL]->uri)) { // The downloaded file is now moved to the local file location. // From this point forward we can treat it as if we imported a // local file. diff --git a/core/modules/system/image.gd.inc b/core/modules/system/image.gd.inc index 0d2f228..4c38b0a 100644 --- a/core/modules/system/image.gd.inc +++ b/core/modules/system/image.gd.inc @@ -308,7 +308,7 @@ function image_gd_save(stdClass $image, $destination) { } // Move temporary local file to remote destination. if (isset($permanent_destination) && $success) { - return (bool) file_unmanaged_move($destination, $permanent_destination, FILE_EXISTS_REPLACE); + return (bool) file_unmanaged_move($destination, $permanent_destination, FILE_CHECK_EXISTS_FALSE); } return $success; } diff --git a/core/modules/system/lib/Drupal/system/Tests/File/DirectoryTest.php b/core/modules/system/lib/Drupal/system/Tests/File/DirectoryTest.php index e6f3396..2a301a6 100644 --- a/core/modules/system/lib/Drupal/system/Tests/File/DirectoryTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/File/DirectoryTest.php @@ -64,57 +64,29 @@ function testFileCheckDirectoryHandling() { } /** - * This will take a directory and path, and find a valid filepath that is not - * taken by another file. - */ - function testFileCreateNewFilepath() { - // First we test against an imaginary file that does not exist in a - // directory. - $basename = 'xyz.txt'; - $directory = 'core/misc'; - $original = $directory . '/' . $basename; - $path = file_create_filename($basename, $directory); - $this->assertEqual($path, $original, format_string('New filepath %new equals %original.', array('%new' => $path, '%original' => $original)), 'File'); - - // Then we test against a file that already exists within that directory. - $basename = 'druplicon.png'; - $original = $directory . '/' . $basename; - $expected = $directory . '/druplicon_0.png'; - $path = file_create_filename($basename, $directory); - $this->assertEqual($path, $expected, format_string('Creating a new filepath from %original equals %new (expected %expected).', array('%new' => $path, '%original' => $original, '%expected' => $expected)), 'File'); - - // @TODO: Finally we copy a file into a directory several times, to ensure a properly iterating filename suffix. - } - - /** * This will test the filepath for a destination based on passed flags and * whether or not the file exists. * - * If a file exists, file_destination($destination, $replace) will either + * If a file exists, file_uri_prepare($destination, $check_exists) will either * return: - * - the existing filepath, if $replace is FILE_EXISTS_REPLACE - * - a new filepath if FILE_EXISTS_RENAME - * - an error (returning FALSE) if FILE_EXISTS_ERROR. + * - the existing filepath, if $check_exists is FILE_CHECK_EXISTS_FALSE + * - a new filepath if FILE_CHECK_EXISTS_FS (default) * If the file doesn't currently exist, then it will simply return the * filepath. */ - function testFileDestination() { + function testFileURIPrepare() { // First test for non-existent file. $destination = 'core/misc/xyz.txt'; - $path = file_destination($destination, FILE_EXISTS_REPLACE); - $this->assertEqual($path, $destination, 'Non-existing filepath destination is correct with FILE_EXISTS_REPLACE.', 'File'); - $path = file_destination($destination, FILE_EXISTS_RENAME); - $this->assertEqual($path, $destination, 'Non-existing filepath destination is correct with FILE_EXISTS_RENAME.', 'File'); - $path = file_destination($destination, FILE_EXISTS_ERROR); - $this->assertEqual($path, $destination, 'Non-existing filepath destination is correct with FILE_EXISTS_ERROR.', 'File'); + $path = file_uri_prepare($destination, FILE_CHECK_EXISTS_FALSE); + $this->assertEqual($path, $destination, 'Non-existing filepath destination is correct with FILE_CHECK_EXISTS_FALSE.', 'File'); + $path = file_uri_prepare($destination); + $this->assertEqual($path, $destination, 'Non-existing filepath destination is correct with FILE_CHECK_EXISTS_FS.', 'File'); $destination = 'core/misc/druplicon.png'; - $path = file_destination($destination, FILE_EXISTS_REPLACE); - $this->assertEqual($path, $destination, 'Existing filepath destination remains the same with FILE_EXISTS_REPLACE.', 'File'); - $path = file_destination($destination, FILE_EXISTS_RENAME); - $this->assertNotEqual($path, $destination, 'A new filepath destination is created when filepath destination already exists with FILE_EXISTS_RENAME.', 'File'); - $path = file_destination($destination, FILE_EXISTS_ERROR); - $this->assertEqual($path, FALSE, 'An error is returned when filepath destination already exists with FILE_EXISTS_ERROR.', 'File'); + $path = file_uri_prepare($destination, FILE_CHECK_EXISTS_FALSE); + $this->assertEqual($path, $destination, 'Existing filepath destination remains the same with FILE_CHECK_EXISTS_FALSE.', 'File'); + $path = file_uri_prepare($destination); + $this->assertNotEqual($path, $destination, 'A new filepath destination is created when filepath destination already exists with FILE_CHECK_EXISTS_FS.', 'File'); } /** diff --git a/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedCopyTest.php b/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedCopyTest.php index 7056946..16a5947 100644 --- a/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedCopyTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedCopyTest.php @@ -28,7 +28,7 @@ function testNormal() { // Copying to a new name. $desired_filepath = 'public://' . $this->randomName(); - $new_filepath = file_unmanaged_copy($uri, $desired_filepath, FILE_EXISTS_ERROR); + $new_filepath = file_unmanaged_copy($uri, $desired_filepath); $this->assertTrue($new_filepath, 'Copy was successful.'); $this->assertEqual($new_filepath, $desired_filepath, 'Returned expected filepath.'); $this->assertTrue(file_exists($uri), 'Original file remains.'); @@ -38,13 +38,22 @@ function testNormal() { // Copying with rename. $desired_filepath = 'public://' . $this->randomName(); $this->assertTrue(file_put_contents($desired_filepath, ' '), 'Created a file so a rename will have to happen.'); - $newer_filepath = file_unmanaged_copy($uri, $desired_filepath, FILE_EXISTS_RENAME); + $newer_filepath = file_unmanaged_copy($uri, $desired_filepath); $this->assertTrue($newer_filepath, 'Copy was successful.'); $this->assertNotEqual($newer_filepath, $desired_filepath, 'Returned expected filepath.'); $this->assertTrue(file_exists($uri), 'Original file remains.'); $this->assertTrue(file_exists($newer_filepath), 'New file exists.'); $this->assertFilePermissions($newer_filepath, variable_get('file_chmod_file', 0664)); + // Copying with overwrite. + $desired_filepath = 'public://' . $this->randomName(); + $this->assertTrue(file_put_contents($desired_filepath, ' '), 'Created a file that will be overwritten.'); + $newer_filepath = file_unmanaged_copy($uri, $desired_filepath, FILE_CHECK_EXISTS_FALSE); + $this->assertTrue($newer_filepath, 'Copy was successful.'); + $this->assertEqual($newer_filepath, $desired_filepath, 'Returned expected filepath.'); + $this->assertTrue(file_exists($uri), 'Original file remains.'); + $this->assertFilePermissions($newer_filepath, variable_get('file_chmod_file', 0664)); + // TODO: test copying to a directory (rather than full directory/file path) // TODO: test copying normal files using normal paths (rather than only streams) } @@ -68,29 +77,27 @@ function testOverwriteSelf() { $uri = $this->createUri(); // Copy the file onto itself with renaming works. - $new_filepath = file_unmanaged_copy($uri, $uri, FILE_EXISTS_RENAME); + $new_filepath = file_unmanaged_copy($uri, $uri); $this->assertTrue($new_filepath, 'Copying onto itself with renaming works.'); $this->assertNotEqual($new_filepath, $uri, 'Copied file has a new name.'); $this->assertTrue(file_exists($uri), 'Original file exists after copying onto itself.'); $this->assertTrue(file_exists($new_filepath), 'Copied file exists after copying onto itself.'); $this->assertFilePermissions($new_filepath, variable_get('file_chmod_file', 0664)); - // Copy the file onto itself without renaming fails. - $new_filepath = file_unmanaged_copy($uri, $uri, FILE_EXISTS_ERROR); - $this->assertFalse($new_filepath, 'Copying onto itself without renaming fails.'); - $this->assertTrue(file_exists($uri), 'File exists after copying onto itself.'); - - // Copy the file into same directory without renaming fails. - $new_filepath = file_unmanaged_copy($uri, drupal_dirname($uri), FILE_EXISTS_ERROR); - $this->assertFalse($new_filepath, 'Copying onto itself fails.'); - $this->assertTrue(file_exists($uri), 'File exists after copying onto itself.'); - // Copy the file into same directory with renaming works. - $new_filepath = file_unmanaged_copy($uri, drupal_dirname($uri), FILE_EXISTS_RENAME); + $new_filepath = file_unmanaged_copy($uri, drupal_dirname($uri)); $this->assertTrue($new_filepath, 'Copying into same directory works.'); $this->assertNotEqual($new_filepath, $uri, 'Copied file has a new name.'); $this->assertTrue(file_exists($uri), 'Original file exists after copying onto itself.'); $this->assertTrue(file_exists($new_filepath), 'Copied file exists after copying onto itself.'); $this->assertFilePermissions($new_filepath, variable_get('file_chmod_file', 0664)); + + // Copy the file onto itself without renaming doesn't work. + $new_filepath = file_unmanaged_copy($uri, $uri, FILE_CHECK_EXISTS_FALSE); + $this->assertFalse($new_filepath, 'Copying the file onto itself without renaming doesn\'t work.'); + + // Copy the file into same directory without renaming doesn't work. + $new_filepath = file_unmanaged_copy($uri, drupal_dirname($uri), FILE_CHECK_EXISTS_FALSE); + $this->assertFalse($new_filepath, 'Copy the file into same directory without renaming doesn\'t work.'); } } diff --git a/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedMoveTest.php b/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedMoveTest.php index 6e0ec14..cb2909b 100644 --- a/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedMoveTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedMoveTest.php @@ -28,7 +28,7 @@ function testNormal() { // Moving to a new name. $desired_filepath = 'public://' . $this->randomName(); - $new_filepath = file_unmanaged_move($uri, $desired_filepath, FILE_EXISTS_ERROR); + $new_filepath = file_unmanaged_move($uri, $desired_filepath); $this->assertTrue($new_filepath, 'Move was successful.'); $this->assertEqual($new_filepath, $desired_filepath, 'Returned expected filepath.'); $this->assertTrue(file_exists($new_filepath), 'File exists at the new location.'); @@ -39,13 +39,25 @@ function testNormal() { $desired_filepath = 'public://' . $this->randomName(); $this->assertTrue(file_exists($new_filepath), 'File exists before moving.'); $this->assertTrue(file_put_contents($desired_filepath, ' '), 'Created a file so a rename will have to happen.'); - $newer_filepath = file_unmanaged_move($new_filepath, $desired_filepath, FILE_EXISTS_RENAME); + $newer_filepath = file_unmanaged_move($new_filepath, $desired_filepath); $this->assertTrue($newer_filepath, 'Move was successful.'); $this->assertNotEqual($newer_filepath, $desired_filepath, 'Returned expected filepath.'); $this->assertTrue(file_exists($newer_filepath), 'File exists at the new location.'); $this->assertFalse(file_exists($new_filepath), 'No file remains at the old location.'); $this->assertFilePermissions($newer_filepath, variable_get('file_chmod_file', 0664)); + // Moving with overwrite. + $new_filepath = $newer_filepath; + $desired_filepath = 'public://' . $this->randomName(); + $this->assertTrue(file_exists($new_filepath), 'File exists before moving.'); + $this->assertTrue(file_put_contents($desired_filepath, ' '), 'Created a file that will be overwritten.'); + $newer_filepath = file_unmanaged_move($new_filepath, $desired_filepath, FILE_CHECK_EXISTS_FALSE); + $this->assertTrue($newer_filepath, 'Move was successful.'); + $this->assertEqual($newer_filepath, $desired_filepath, 'Returned expected filepath.'); + $this->assertTrue(file_exists($newer_filepath), 'File exists at the new location.'); + $this->assertFalse(file_exists($new_filepath), 'No file remains at the old location.'); + $this->assertFilePermissions($newer_filepath, variable_get('file_chmod_file', 0664)); + // TODO: test moving to a directory (rather than full directory/file path) // TODO: test creating and moving normal files (rather than streams) } @@ -55,8 +67,8 @@ function testNormal() { */ function testMissing() { // Move non-existent file. - $new_filepath = file_unmanaged_move($this->randomName(), $this->randomName()); - $this->assertFalse($new_filepath, 'Moving a missing file fails.'); + $result = file_unmanaged_move($this->randomName(), $this->randomName()); + $this->assertFalse($result, 'Moving a missing file fails.'); } /** @@ -67,14 +79,22 @@ function testOverwriteSelf() { $uri = $this->createUri(); // Move the file onto itself without renaming shouldn't make changes. - $new_filepath = file_unmanaged_move($uri, $uri, FILE_EXISTS_REPLACE); + $new_filepath = file_unmanaged_move($uri, $uri, FILE_CHECK_EXISTS_FALSE); $this->assertFalse($new_filepath, 'Moving onto itself without renaming fails.'); $this->assertTrue(file_exists($uri), 'File exists after moving onto itself.'); // Move the file onto itself with renaming will result in a new filename. - $new_filepath = file_unmanaged_move($uri, $uri, FILE_EXISTS_RENAME); + $new_filepath = file_unmanaged_move($uri, $uri); $this->assertTrue($new_filepath, 'Moving onto itself with renaming works.'); $this->assertFalse(file_exists($uri), 'Original file has been removed.'); $this->assertTrue(file_exists($new_filepath), 'File exists after moving onto itself.'); + + // Move the file onto itself without renaming doesn't work. + $new_filepath = file_unmanaged_move($uri, $uri, FILE_CHECK_EXISTS_FALSE); + $this->assertFalse($new_filepath, 'Copying the file onto itself without renaming doesn\'t work.'); + + // Copy the file into same directory without renaming doesn't work. + $new_filepath = file_unmanaged_move($uri, drupal_dirname($uri), FILE_CHECK_EXISTS_FALSE); + $this->assertFalse($new_filepath, 'Copy the file into same directory without renaming doesn\'t work.'); } } diff --git a/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedSaveDataTest.php b/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedSaveDataTest.php index cde75c7..07120da 100644 --- a/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedSaveDataTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/File/UnmanagedSaveDataTest.php @@ -32,7 +32,7 @@ function testFileSaveData() { $this->assertEqual($contents, file_get_contents($filepath), 'Contents of the file are correct.'); // Provide a filename. - $filepath = file_unmanaged_save_data($contents, 'public://asdf.txt', FILE_EXISTS_REPLACE); + $filepath = file_unmanaged_save_data($contents, 'public://asdf.txt', FILE_CHECK_EXISTS_FALSE); $this->assertTrue($filepath, 'Unnamed file saved correctly.'); $this->assertEqual('asdf.txt', drupal_basename($filepath), 'File was named correctly.'); $this->assertEqual($contents, file_get_contents($filepath), 'Contents of the file are correct.'); diff --git a/core/modules/system/lib/Drupal/system/Tests/Image/FileMoveTest.php b/core/modules/system/lib/Drupal/system/Tests/Image/FileMoveTest.php index 336888e..ee9139a 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Image/FileMoveTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Image/FileMoveTest.php @@ -45,13 +45,10 @@ function testNormal() { // Check if derivative image exists. $this->assertTrue(file_exists($derivative_uri), 'Make sure derivative image is generated successfully.'); - // Clone the object so we don't have to worry about the function changing - // our reference copy. - $desired_filepath = 'public://' . $this->randomName(); - $result = file_move(clone $file, $desired_filepath, FILE_EXISTS_ERROR); + file_move($file, 'public://' . $this->randomName()); // Check if image has been moved. - $this->assertTrue(file_exists($result->uri), 'Make sure image is moved successfully.'); + $this->assertTrue(file_exists($file->uri), 'Make sure image is moved successfully.'); // Check if derivative image has been flushed. $this->assertFalse(file_exists($derivative_uri), 'Make sure derivative image has been flushed.'); diff --git a/core/modules/system/system.module b/core/modules/system/system.module index 8ba745c..e72e212 100644 --- a/core/modules/system/system.module +++ b/core/modules/system/system.module @@ -3671,22 +3671,17 @@ function system_image_toolkits() { * be used instead. * If this value is omitted, the site's default files scheme will be used, * usually "public://". - * - * @param $managed boolean - * If this is set to TRUE, the file API hooks will be invoked and the file is - * registered in the database. - * - * @param $replace boolean - * Replace behavior when the destination file already exists: - * - FILE_EXISTS_REPLACE: Replace the existing file. - * - FILE_EXISTS_RENAME: Append _{incrementing number} until the filename is - * unique. - * - FILE_EXISTS_ERROR: Do nothing and return FALSE. + * @param $check_exists + * The destination URI will be renamed if it already exists by appending + * _{incrementing number} until it is unique. + * - FILE_CHECK_EXISTS_FALSE - Don't check if it exists. + * - FILE_CHECK_EXISTS_FS - Just check the filesystem. + * - FILE_CHECK_EXISTS_DB_FS - Check the database, then the filesystem. * * @return * On success the location the file was saved to, FALSE on failure. */ -function system_retrieve_file($url, $destination = NULL, $managed = FALSE, $replace = FILE_EXISTS_RENAME) { +function system_retrieve_file($url, $destination = NULL, $check_exists = FILE_CHECK_EXISTS_FS) { $parsed_url = parse_url($url); if (!isset($destination)) { $path = file_build_uri(drupal_basename($parsed_url['path'])); @@ -3705,7 +3700,7 @@ function system_retrieve_file($url, $destination = NULL, $managed = FALSE, $repl drupal_set_message(t('HTTP error @errorcode occurred when trying to fetch @remote.', array('@errorcode' => $result->code, '@remote' => $url)), 'error'); return FALSE; } - $local = $managed ? file_save_data($result->data, $path, $replace) : file_unmanaged_save_data($result->data, $path, $replace); + $local = file_unmanaged_save_data($result->data, $path, $check_exists); if (!$local) { drupal_set_message(t('@remote could not be saved to @path.', array('@remote' => $url, '@path' => $path)), 'error'); } diff --git a/core/modules/update/update.manager.inc b/core/modules/update/update.manager.inc index 0047a52..6823621 100644 --- a/core/modules/update/update.manager.inc +++ b/core/modules/update/update.manager.inc @@ -645,7 +645,7 @@ function update_manager_install_form_submit($form, &$form_state) { elseif ($_FILES['files']['name']['project_upload']) { $validators = array('file_validate_extensions' => array(archiver_get_extensions())); $field = 'project_upload'; - if (!($finfo = file_save_upload($field, $validators, NULL, FILE_EXISTS_REPLACE))) { + if (!($finfo = file_save_upload($field, $validators))) { // Failed to upload the file. file_save_upload() calls form_set_error() on // failure. return; @@ -836,7 +836,7 @@ function update_manager_file_get($url) { $local = $cache_directory . '/' . drupal_basename($parsed_url['path']); if (!file_exists($local) || update_delete_file_if_stale($local)) { - return system_retrieve_file($url, $local, FALSE, FILE_EXISTS_REPLACE); + return system_retrieve_file($url, $local, FILE_CHECK_EXISTS_FALSE); } else { return $local; diff --git a/core/modules/user/user.install b/core/modules/user/user.install index 9a0b663..cb8b760 100644 --- a/core/modules/user/user.install +++ b/core/modules/user/user.install @@ -669,7 +669,7 @@ function user_update_8011() { // created/saved during the upgrade path. Attempt to replicate the behavior // of file_save_data() by updating an eventually existing record for that // file. - file_unmanaged_save_data($default_image, $destination, FILE_EXISTS_REPLACE); + file_unmanaged_save_data($default_image, $destination); $uuid = new Uuid(); db_merge('file_managed') ->key(array(