Commit d813e367 authored by Dries's avatar Dries

- Patch #348201 by catch: make it possible to load multiple files with fewer queries.

parent 82727ed8
...@@ -267,55 +267,60 @@ function file_check_location($source, $directory = '') { ...@@ -267,55 +267,60 @@ function file_check_location($source, $directory = '') {
} }
/** /**
* Load a file object from the database. * Load file objects from the database.
* *
* @param $param * @param $fids
* Either the id of a file or an array of conditions to match against in the * An array of file IDs.
* database query. * @param $conditions
* @param $reset * An array of conditions to match against the {files} table. These
* Whether to reset the internal file_load cache. * should be supplied in the form array('field_name' => 'field_value').
* @return * @return
* A file object. * An array of file objects, indexed by fid.
* *
* @see hook_file_load() * @see hook_file_load()
* @see file_load()
*/ */
function file_load($param, $reset = NULL) { function file_load_multiple($fids = array(), $conditions = array()) {
static $files = array(); $query = db_select('files', 'f')->fields('f');
if ($reset) { // If the $fids array is populated, add those to the query.
$files = array(); if ($fids) {
$query->condition('f.fid', $fids, 'IN');
} }
if (is_numeric($param)) { // If the conditions array is populated, add those to the query.
if (isset($files[(string) $param])) { if ($conditions) {
return is_object($files[$param]) ? clone $files[$param] : $files[$param]; foreach ($conditions as $field => $value) {
$query->condition('f.' . $field, $value);
} }
$result = db_query('SELECT f.* FROM {files} f WHERE f.fid = :fid', array(':fid' => $param));
} }
elseif (is_array($param)) { $files = $query->execute()->fetchAllAssoc('fid');
// Turn the conditions into a query.
$cond = array(); // Invoke hook_file_load() on the terms loaded from the database
$arguments = array(); // and add them to the static cache.
foreach ($param as $key => $value) { if (!empty($files)) {
$cond[] = 'f.' . db_escape_table($key) . " = '%s'"; foreach (module_implements('file_load') as $module) {
$arguments[] = $value; $function = $module . '_file_load';
$function($files);
} }
$result = db_query('SELECT f.* FROM {files} f WHERE ' . implode(' AND ', $cond), $arguments);
}
else {
return FALSE;
}
$file = $result->fetch(PDO::FETCH_OBJ);
if ($file && $file->fid) {
// Allow modules to add or change the file object.
module_invoke_all('file_load', $file);
// Cache the fully loaded value.
$files[(string) $file->fid] = clone $file;
} }
return $files;
}
return $file; /**
* Load a file object from the database.
*
* @param $fid
* A file ID.
* @return
* A file object.
*
* @see hook_file_load()
* @see file_load_multiple()
*/
function file_load($fid) {
$files = file_load_multiple(array($fid), array());
return reset($files);
} }
/** /**
......
...@@ -338,7 +338,7 @@ class FileSaveUploadTest extends FileHookTestCase { ...@@ -338,7 +338,7 @@ class FileSaveUploadTest extends FileHookTestCase {
* Test the file_save_upload() function. * Test the file_save_upload() function.
*/ */
function testFileSaveUpload() { function testFileSaveUpload() {
$max_fid_before = db_result(db_query('SELECT MAX(fid) AS fid FROM {files}')); $max_fid_before = db_query('SELECT MAX(fid) AS fid FROM {files}')->fetchField();
$upload_user = $this->drupalCreateUser(array('access content')); $upload_user = $this->drupalCreateUser(array('access content'));
$this->drupalLogin($upload_user); $this->drupalLogin($upload_user);
...@@ -354,7 +354,23 @@ class FileSaveUploadTest extends FileHookTestCase { ...@@ -354,7 +354,23 @@ class FileSaveUploadTest extends FileHookTestCase {
$max_fid_after = db_result(db_query('SELECT MAX(fid) AS fid FROM {files}')); $max_fid_after = db_result(db_query('SELECT MAX(fid) AS fid FROM {files}'));
$this->assertTrue($max_fid_after > $max_fid_before, t('A new file was created.')); $this->assertTrue($max_fid_after > $max_fid_before, t('A new file was created.'));
$this->assertTrue(file_load($max_fid_after), t('Loaded the file.')); $file1 = file_load($max_fid_after);
$this->assertTrue($file1, t('Loaded the file.'));
// Upload a second file.
$max_fid_before = db_query('SELECT MAX(fid) AS fid FROM {files}')->fetchField();
$image2 = current($this->drupalGetTestFiles('image'));
$edit = array('files[file_test_upload]' => realpath($image2->filename));
$this->drupalPost('file-test/upload', $edit, t('Submit'));
$max_fid_after = db_query('SELECT MAX(fid) AS fid FROM {files}')->fetchField();
$file2 = file_load($max_fid_after);
$this->assertTrue($file2);
// Load both files using file_load_multiple().
$files = file_load_multiple(array($file1->fid, $file2->fid));
$this->assertTrue(isset($files[$file1->fid]), t('File was loaded successfully'));
$this->assertTrue(isset($files[$file2->fid]), t('File was loaded successfully'));
} }
} }
...@@ -781,7 +797,7 @@ class FileDeleteTest extends FileHookTestCase { ...@@ -781,7 +797,7 @@ class FileDeleteTest extends FileHookTestCase {
$this->assertFileHookCalled('references'); $this->assertFileHookCalled('references');
$this->assertFileHookCalled('delete'); $this->assertFileHookCalled('delete');
$this->assertFalse(file_exists($file->filepath), t("Test file has actually been deleted.")); $this->assertFalse(file_exists($file->filepath), t("Test file has actually been deleted."));
$this->assertFalse(file_load(array('filepath' => $file->filepath)), t("File was removed from the database.")); $this->assertFalse(file_load($file->fid), t('File was removed from the database.'));
// TODO: implement hook_file_references() in file_test.module and report a // TODO: implement hook_file_references() in file_test.module and report a
// file in use and test the $force parameter. // file in use and test the $force parameter.
...@@ -814,7 +830,7 @@ class FileMoveTest extends FileHookTestCase { ...@@ -814,7 +830,7 @@ class FileMoveTest extends FileHookTestCase {
$this->assertFileHookCalled('update'); $this->assertFileHookCalled('update');
$this->assertEqual($file->fid, $file->fid, t("File id $file->fid is unchanged after move.")); $this->assertEqual($file->fid, $file->fid, t("File id $file->fid is unchanged after move."));
$loaded_file = file_load($file->fid, TRUE); $loaded_file = file_load($file->fid);
$this->assertTrue($loaded_file, t("File can be loaded from the database.")); $this->assertTrue($loaded_file, t("File can be loaded from the database."));
$this->assertEqual($file->filename, $loaded_file->filename, t("File name was updated correctly in the database.")); $this->assertEqual($file->filename, $loaded_file->filename, t("File name was updated correctly in the database."));
$this->assertEqual($file->filepath, $loaded_file->filepath, t("File path was updated correctly in the database.")); $this->assertEqual($file->filepath, $loaded_file->filepath, t("File path was updated correctly in the database."));
...@@ -852,7 +868,7 @@ class FileCopyTest extends FileHookTestCase { ...@@ -852,7 +868,7 @@ class FileCopyTest extends FileHookTestCase {
$this->assertTrue(file_exists($file->filepath), t('The copied file exists.')); $this->assertTrue(file_exists($file->filepath), t('The copied file exists.'));
// Check that the changes were actually saved to the database. // Check that the changes were actually saved to the database.
$loaded_file = file_load($file->fid, TRUE); $loaded_file = file_load($file->fid);
$this->assertTrue($loaded_file, t("File can be loaded from the database.")); $this->assertTrue($loaded_file, t("File can be loaded from the database."));
$this->assertEqual($file->filename, $loaded_file->filename, t("File name was updated correctly in the database.")); $this->assertEqual($file->filename, $loaded_file->filename, t("File name was updated correctly in the database."));
$this->assertEqual($file->filepath, $loaded_file->filepath, t("File path was updated correctly in the database.")); $this->assertEqual($file->filepath, $loaded_file->filepath, t("File path was updated correctly in the database."));
...@@ -884,7 +900,7 @@ class FileLoadTest extends FileHookTestCase { ...@@ -884,7 +900,7 @@ class FileLoadTest extends FileHookTestCase {
* Try to load a non-existent file by filepath. * Try to load a non-existent file by filepath.
*/ */
function testLoadMissingFilepath() { function testLoadMissingFilepath() {
$this->assertFalse(file_load(array('filepath' => 'misc/druplicon.png')), t("Try to load a file that doesn't exist in the database fails.")); $this->assertFalse(reset(file_load_multiple(array(), array('filepath' => 'misc/druplicon.png'))), t("Try to load a file that doesn't exist in the database fails."));
$this->assertFileHookCalled('load', 0); $this->assertFileHookCalled('load', 0);
} }
...@@ -892,14 +908,40 @@ class FileLoadTest extends FileHookTestCase { ...@@ -892,14 +908,40 @@ class FileLoadTest extends FileHookTestCase {
* Try to load a non-existent file by status. * Try to load a non-existent file by status.
*/ */
function testLoadInvalidStatus() { function testLoadInvalidStatus() {
$this->assertFalse(file_load(array('status' => -99)), t("Trying to load a file with an invalid status fails.")); $this->assertFalse(reset(file_load_multiple(array(), array('status' => -99))), t("Trying to load a file with an invalid status fails."));
$this->assertFileHookCalled('load', 0); $this->assertFileHookCalled('load', 0);
} }
/** /**
* This will test lading file data from the database. * Load a single file and ensure that the correct values are returned.
*/
function testSingleValues() {
// Create a new file object from scratch so we know the values.
$file = array(
'uid' => 1,
'filename' => 'druplicon.png',
'filepath' => 'misc/druplicon.png',
'filemime' => 'image/png',
'timestamp' => 1,
'status' => FILE_STATUS_PERMANENT,
);
$file = file_save($file);
$by_fid_file = file_load($file->fid);
$this->assertFileHookCalled('load', 1);
$this->assertTrue(is_object($by_fid_file), t('file_load() returned an object.'));
$this->assertEqual($by_fid_file->fid, $file->fid, t("Loading by fid got the same fid."), 'File');
$this->assertEqual($by_fid_file->filepath, $file->filepath, t("Loading by fid got the correct filepath."), 'File');
$this->assertEqual($by_fid_file->filename, $file->filename, t("Loading by fid got the correct filename."), 'File');
$this->assertEqual($by_fid_file->filemime, $file->filemime, t("Loading by fid got the correct MIME type."), 'File');
$this->assertEqual($by_fid_file->status, $file->status, t("Loading by fid got the correct status."), 'File');
$this->assertTrue($by_fid_file->file_test['loaded'], t('file_test_file_load() was able to modify the file during load.'));
}
/**
* This will test loading file data from the database.
*/ */
function testFileLoad() { function testMultiple() {
// Create a new file object. // Create a new file object.
$file = array( $file = array(
'uid' => 1, 'uid' => 1,
...@@ -913,25 +955,24 @@ class FileLoadTest extends FileHookTestCase { ...@@ -913,25 +955,24 @@ class FileLoadTest extends FileHookTestCase {
// Load by path. // Load by path.
file_test_reset(); file_test_reset();
$by_path_file = file_load(array('filepath' => $file->filepath)); $by_path_files = file_load_multiple(array(), array('filepath' => $file->filepath));
$this->assertFileHookCalled('load'); $this->assertFileHookCalled('load', 1);
$this->assertEqual(1, count($by_path_files), t('file_load_multiple() returned an array of the correct size.'));
$by_path_file = reset($by_path_files);
$this->assertTrue($by_path_file->file_test['loaded'], t('file_test_file_load() was able to modify the file during load.')); $this->assertTrue($by_path_file->file_test['loaded'], t('file_test_file_load() was able to modify the file during load.'));
$this->assertEqual($by_path_file->fid, $file->fid, t("Loading by filepath got the correct fid."), 'File'); $this->assertEqual($by_path_file->fid, $file->fid, t("Loading by filepath got the correct fid."), 'File');
// Load by fid. // Load by fid.
file_test_reset(); file_test_reset();
$by_fid_file = file_load($file->fid); $by_fid_files = file_load_multiple(array($file->fid), array());
$this->assertFileHookCalled('load', 0); $this->assertFileHookCalled('load', 1);
$this->assertEqual(1, count($by_fid_files), t('file_load_multiple() returned an array of the correct size.'));
$by_fid_file = reset($by_fid_files);
$this->assertTrue($by_fid_file->file_test['loaded'], t('file_test_file_load() was able to modify the file during load.')); $this->assertTrue($by_fid_file->file_test['loaded'], t('file_test_file_load() was able to modify the file during load.'));
$this->assertEqual($by_fid_file->filepath, $file->filepath, t("Loading by fid got the correct filepath."), 'File'); $this->assertEqual($by_fid_file->filepath, $file->filepath, t("Loading by fid got the correct filepath."), 'File');
// Load again by fid but use the reset param to reload.
file_test_reset();
$by_fid_file = file_load($file->fid, TRUE);
$this->assertFileHookCalled('load');
$this->assertEqual($by_fid_file->filepath, $file->filepath, t("Loading by fid got the correct filepath."), 'File');
} }
} }
/** /**
* Tests the file_save() function. * Tests the file_save() function.
*/ */
...@@ -1061,4 +1102,4 @@ class FileSaveDataTest extends FileHookTestCase { ...@@ -1061,4 +1102,4 @@ class FileSaveDataTest extends FileHookTestCase {
$file = file_save_data($contents, 'asdf.txt', FILE_EXISTS_ERROR); $file = file_save_data($contents, 'asdf.txt', FILE_EXISTS_ERROR);
$this->assertFalse($file, t("Overwriting a file fails when FILE_EXISTS_ERROR is specified.")); $this->assertFalse($file, t("Overwriting a file fails when FILE_EXISTS_ERROR is specified."));
} }
} }
\ No newline at end of file
...@@ -148,11 +148,13 @@ function file_test_set_return($op, $value) { ...@@ -148,11 +148,13 @@ function file_test_set_return($op, $value) {
/** /**
* Implementation of hook_file_load(). * Implementation of hook_file_load().
*/ */
function file_test_file_load($file) { function file_test_file_load($files) {
_file_test_log_call('load', array($file)); foreach ($files as $file) {
// Assign a value on the object so that we can test that the $file is passed _file_test_log_call('load', array($file));
// by reference. // Assign a value on the object so that we can test that the $file is passed
$file->file_test['loaded'] = TRUE; // by reference.
$file->file_test['loaded'] = TRUE;
}
} }
/** /**
......
...@@ -983,23 +983,24 @@ function custom_url_rewrite_inbound(&$result, $path, $path_language) { ...@@ -983,23 +983,24 @@ function custom_url_rewrite_inbound(&$result, $path, $path_language) {
} }
/** /**
* Load additional information into a file object. * Load additional information into file objects.
* *
* file_load() calls this hook to allow modules to load additional information * file_load_multiple() calls this hook to allow modules to load
* into the $file. * additional information into each file.
* *
* @param $file * @param $files
* The file object being loaded. * An array of file objects, indexed by fid.
* @return
* None.
* *
* @see file_load() * @see file_load_multiple()
* @see upload_file_load()
*/ */
function hook_file_load(&$file) { function hook_file_load($files) {
// Add the upload specific data into the file object. // Add the upload specific data into the file object.
$values = db_query('SELECT * FROM {upload} u WHERE u.fid = :fid', array(':fid' => $file->fid))->fetch(PDO::FETCH_ASSOC); $result = db_query('SELECT * FROM {upload} u WHERE u.fid IN (:fids)', array(':fids' => array_keys($files)))->fetchAll(PDO::FETCH_ASSOC);
foreach ((array)$values as $key => $value) { foreach ($result as $record) {
$file->{$key} = $value; foreach ($record as $key => $value) {
$files[$record['fid']]->$key = $value;
}
} }
} }
......
...@@ -269,11 +269,13 @@ function upload_form_alter(&$form, $form_state, $form_id) { ...@@ -269,11 +269,13 @@ function upload_form_alter(&$form, $form_state, $form_id) {
/** /**
* Implementation of hook_file_load(). * Implementation of hook_file_load().
*/ */
function upload_file_load(&$file) { function upload_file_load($files) {
// Add the upload specific data into the file object. // Add the upload specific data into the file object.
$values = db_query('SELECT * FROM {upload} u WHERE u.fid = :fid', array(':fid' => $file->fid))->fetch(PDO::FETCH_ASSOC); $result = db_query('SELECT * FROM {upload} u WHERE u.fid IN (:fids)', array(':fids' => array_keys($files)))->fetchAll(PDO::FETCH_ASSOC);
foreach ((array)$values as $key => $value) { foreach ($result as $record) {
$file->{$key} = $value; foreach ($record as $key => $value) {
$files[$record['fid']]->$key = $value;
}
} }
} }
...@@ -297,16 +299,42 @@ function upload_file_delete(&$file) { ...@@ -297,16 +299,42 @@ function upload_file_delete(&$file) {
db_delete('upload')->condition('fid', $file->fid)->execute(); db_delete('upload')->condition('fid', $file->fid)->execute();
} }
/** /**
* Implementation of hook_nodeapi_load(). * Implementation of hook_nodeapi_load().
*/ */
function upload_nodeapi_load($nodes, $types) { function upload_nodeapi_load($nodes, $types) {
// Collect all the revision ids for nodes with upload enabled.
$node_vids = array();
foreach ($nodes as $node) { foreach ($nodes as $node) {
if (variable_get("upload_$node->type", 1) == 1) { if (variable_get("upload_$node->type", 1) == 1) {
$node->files = upload_load($node); $node_vids[$node->vid] = $node->vid;
$node->files = array();
} }
} }
// If there are no vids then there's no point trying to load files.
if (empty($node_vids)) {
return;
}
// Fetch the fids associated with these node revisions.
$result = db_query('SELECT u.fid, u.nid, u.vid FROM {upload} u WHERE u.vid IN (:node_vids) ORDER BY u.weight, u.fid', array(':node_vids' => $node_vids));
// The same file may be attached to several nodes (e.g. translated nodes) so
// simply calling db_query()->fetchAllAssoc('fid') would return one node
// per file. Instead we build one array with the file ids for
// file_load_multiple() and another array with upload records so we can match
// files back to the nodes.
$fids = array();
$uploads = array();
foreach ($result as $record) {
$fids[] = $record->fid;
$uploads[] = $record;
}
$files = file_load_multiple($fids);
foreach ($uploads as $upload) {
$nodes[$upload->nid]->files[$upload->fid] = $files[$upload->fid];
}
} }
/** /**
...@@ -324,7 +352,7 @@ function upload_nodeapi_view($node, $teaser, $page) { ...@@ -324,7 +352,7 @@ function upload_nodeapi_view($node, $teaser, $page) {
); );
} }
} }
upload_nodeapi_links($node, $teaser); upload_nodeapi_links($node, $teaser);
} }
} }
...@@ -609,19 +637,6 @@ function theme_upload_form_new($form) { ...@@ -609,19 +637,6 @@ function theme_upload_form_new($form) {
return $output; return $output;
} }
function upload_load($node) {
$files = array();
if ($node->vid) {
$result = db_query('SELECT u.fid FROM {upload} u WHERE u.vid = :vid ORDER BY u.weight, u.fid', array(':vid' => $node->vid));
foreach ($result as $file) {
$files[$file->fid] = file_load($file->fid);
}
}
return $files;
}
/** /**
* Menu-callback for JavaScript-based uploads. * Menu-callback for JavaScript-based uploads.
*/ */
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment