Removing file_directory_path() in filepath in file table http://drupal.org/node/366464 From: andrew morton --- includes/file.inc | 32 ++++++++++++++++++++++++++-- modules/simpletest/tests/file.test | 41 +++++++++++++++++++++++++----------- 2 files changed, 59 insertions(+), 14 deletions(-) diff --git includes/file.inc includes/file.inc index b9f4ed9..a6a3727 100644 --- includes/file.inc +++ includes/file.inc @@ -282,6 +282,14 @@ function file_load_multiple($fids = array(), $conditions = array()) { // If the conditions array is populated, add those to the query. if ($conditions) { + // If the condition is a filepath, it may contain the path to the + // file_directory_path, so we need to remove this to allow the fid to be + // found. + if (isset($conditions['filepath'])) { + if (strpos($conditions['filepath'], file_directory_path()) === 0) { + $conditions['filepath'] = substr($value, strlen(file_directory_path()) + 1); + } + } foreach ($conditions as $field => $value) { $query->condition('f.' . $field, $value); } @@ -291,6 +299,13 @@ function file_load_multiple($fids = array(), $conditions = array()) { // Invoke hook_file_load() on the terms loaded from the database // and add them to the static cache. if (!empty($files)) { + // Prepend file_directory_path onto filepath. + foreach ($files as $file){ + // Add file_directory_path back in for the file_load hook, and return + // value. + $file->filepath = file_directory_path() . '/' . $file->filepath; + } + foreach (module_implements('file_load') as $module) { $function = $module . '_file_load'; $function($files); @@ -334,7 +349,11 @@ function file_save($file) { $file->timestamp = REQUEST_TIME; $file->filesize = filesize($file->filepath); - if (empty($file->fid)) { + // Remove file directory path for entering into the database. + $old_filepath = $file->filepath; + $file->filepath = substr($file->filepath, strlen(file_directory_path()) + 1); + + if (empty($file->fid)) { drupal_write_record('files', $file); // Inform modules about the newly added file. module_invoke_all('file_insert', $file); @@ -345,6 +364,12 @@ function file_save($file) { module_invoke_all('file_update', $file); } + // And restore the original filepath dir for returning the file object + // TODO: Should file_save just keep the filepath relevant to the files + // directory, and code outside of file.inc add the file_directory_path as + // necessary? + $file->filepath = $old_filepath; + return $file; } @@ -368,7 +393,7 @@ function file_save($file) { * @param $destination * A string containing the destination that $source should be copied to. This * can be a complete file path, a directory path or, if this value is omitted, - * Drupal's 'files' directory will be used. + * Drupal's 'files' directory will be used. N.B. it must be relative to the files directory * @param $replace * Replace behavior when the destination file already exists: * - FILE_EXISTS_REPLACE - Replace the existing file. If a managed file with @@ -385,6 +410,9 @@ function file_save($file) { */ function file_copy($source, $destination = NULL, $replace = FILE_EXISTS_RENAME) { $source = (object)$source; + + // Ensure that the destination is inside the files directory. + $destination = file_create_path($destination); if ($filepath = file_unmanaged_copy($source->filepath, $destination, $replace)) { $file = clone $source; diff --git modules/simpletest/tests/file.test modules/simpletest/tests/file.test index bea3be3..fbe6b79 100644 --- modules/simpletest/tests/file.test +++ modules/simpletest/tests/file.test @@ -158,17 +158,19 @@ class FileTestCase extends DrupalWebTestCase { $this->assertTrue(is_file($filepath), t('The test file exists on the disk.'), 'Create test file'); $file = new stdClass(); - $file->filepath = $filepath; + $file->filepath = substr($filepath, strlen(file_directory_path()) + 1); // we don't want a file_directory_path in the value that is written to the database $file->filename = basename($file->filepath); $file->filemime = 'text/plain'; $file->uid = 1; $file->timestamp = REQUEST_TIME; - $file->filesize = filesize($file->filepath); + $file->filesize = filesize($filepath); $file->status = 0; // Write the record directly rather than calling file_save() so we don't // invoke the hooks. $this->assertNotIdentical(drupal_write_record('files', $file), FALSE, t('The file was added to the database.'), 'Create test file'); + // but we really do want the file directory path available to the tests and other functions, so we need to put it back + $file->filepath = $filepath; return $file; } } @@ -1589,11 +1591,16 @@ class FileLoadTest extends FileHookTestCase { */ function testSingleValues() { // Create a new file object from scratch so we know the values. + // as all paths are relative to the files dir, we can't use the old value of misc/druplicon.png because the path returned will include the path to the file_directory_path, where it doesn't exist + $filepath = file_directory_path() . '/' . $this->randomName(); + $contents = "file_put_contents() doesn't seem to appreciate empty strings so let's put in some data."; + file_put_contents($filepath, $contents); + $file = array( 'uid' => 1, - 'filename' => 'druplicon.png', - 'filepath' => 'misc/druplicon.png', - 'filemime' => 'image/png', + 'filename' => basename($filepath), + 'filepath' => $filepath, + 'filemime' => 'text/plain', 'timestamp' => 1, 'status' => FILE_STATUS_PERMANENT, ); @@ -1615,11 +1622,16 @@ class FileLoadTest extends FileHookTestCase { */ function testMultiple() { // Create a new file object. + // as all paths are relative to the files dir, we can't use the old value of misc/druplicon.png because the path returned will include the path to the file_directory_path + $filepath = file_directory_path() . '/' . $this->randomName(); + $contents = "file_put_contents() doesn't seem to appreciate empty strings so let's put in some data."; + file_put_contents($filepath, $contents); + $file = array( 'uid' => 1, - 'filename' => 'druplicon.png', - 'filepath' => 'misc/druplicon.png', - 'filemime' => 'image/png', + 'filename' => basename($filepath), + 'filepath' => $filepath, + 'filemime' => 'text/plain', 'timestamp' => 1, 'status' => FILE_STATUS_PERMANENT, ); @@ -1659,15 +1671,20 @@ class FileSaveTest extends FileHookTestCase { function testFileSave() { // Create a new file object. + // as all paths are relative to the files dir, we can't use the old value of misc/druplicon.png because the path returned will include the path to the file_directory_path + $filepath = file_directory_path() . '/' . $this->randomName(); + $contents = "file_put_contents() doesn't seem to appreciate empty strings so let's put in some data."; + file_put_contents($filepath, $contents); + $file = array( 'uid' => 1, - 'filename' => 'druplicon.png', - 'filepath' => 'misc/druplicon.png', - 'filemime' => 'image/png', + 'filename' => basename($filepath), + 'filepath' => $filepath, + 'filemime' => 'text/plain', 'timestamp' => 1, 'status' => FILE_STATUS_PERMANENT, ); - $file = (object) $file; + $file = file_save($file); // Save it, inserting a new record. $saved_file = file_save($file);