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

5 files changed, 157 insertions, 93 deletions

diff --git a/includes/file.inc b/includes/file.inc
index 6a0ef2334..360b66fc4 100644
--- a/includes/file.inc
+++ b/includes/file.inc
@@ -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
- *   Either the id of a file or an array of conditions to match against in the
- *   database query.
- * @param $reset
- *   Whether to reset the internal file_load cache.
+ * @param $fids
+ *   An array of file IDs.
+ * @param $conditions
+ *   An array of conditions to match against the {files} table. These
+ *   should be supplied in the form array('field_name' => 'field_value').
  * @return
- *   A file object.
+ *  An array of file objects, indexed by fid.
  *
  * @see hook_file_load()
+ * @see file_load()
  */
-function file_load($param, $reset = NULL) {
-  static $files = array();
+function file_load_multiple($fids = array(), $conditions = array()) {
+  $query = db_select('files', 'f')->fields('f');
 
-  if ($reset) {
-    $files = array();
+  // If the $fids array is populated, add those to the query.
+  if ($fids) {
+    $query->condition('f.fid', $fids, 'IN');
   }
 
-  if (is_numeric($param)) {
-    if (isset($files[(string) $param])) {
-      return is_object($files[$param]) ? clone $files[$param] : $files[$param];
+  // If the conditions array is populated, add those to the query.
+  if ($conditions) {
+    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)) {
-    // Turn the conditions into a query.
-    $cond = array();
-    $arguments = array();
-    foreach ($param as $key => $value) {
-      $cond[] = 'f.' . db_escape_table($key) . " = '%s'";
-      $arguments[] = $value;
+  $files = $query->execute()->fetchAllAssoc('fid');
+
+  // Invoke hook_file_load() on the terms loaded from the database
+  // and add them to the static cache.
+  if (!empty($files)) {
+    foreach (module_implements('file_load') as $module) {
+      $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);
 }
 
 /**
diff --git a/modules/simpletest/tests/file.test b/modules/simpletest/tests/file.test
index eadafcc83..861cbdc17 100644
--- a/modules/simpletest/tests/file.test
+++ b/modules/simpletest/tests/file.test
@@ -338,7 +338,7 @@ class FileSaveUploadTest extends FileHookTestCase {
    * Test the file_save_upload() function.
    */
   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'));
     $this->drupalLogin($upload_user);
 
@@ -354,7 +354,23 @@ class FileSaveUploadTest extends FileHookTestCase {
 
     $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(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 {
     $this->assertFileHookCalled('references');
     $this->assertFileHookCalled('delete');
     $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
     // file in use and test the $force parameter.
@@ -814,7 +830,7 @@ class FileMoveTest extends FileHookTestCase {
     $this->assertFileHookCalled('update');
     $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->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."));
@@ -852,7 +868,7 @@ class FileCopyTest extends FileHookTestCase {
     $this->assertTrue(file_exists($file->filepath), t('The copied file exists.'));
 
     // 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->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."));
@@ -884,7 +900,7 @@ class FileLoadTest extends FileHookTestCase {
    * Try to load a non-existent file by filepath.
    */
   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);
   }
 
@@ -892,14 +908,40 @@ class FileLoadTest extends FileHookTestCase {
    * Try to load a non-existent file by status.
    */
   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 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.
     $file = array(
       'uid' => 1,
@@ -913,25 +955,24 @@ class FileLoadTest extends FileHookTestCase {
 
     // Load by path.
     file_test_reset();
-    $by_path_file = file_load(array('filepath' => $file->filepath));
-    $this->assertFileHookCalled('load');
+    $by_path_files = file_load_multiple(array(), array('filepath' => $file->filepath));
+    $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->assertEqual($by_path_file->fid, $file->fid, t("Loading by filepath got the correct fid."), 'File');
 
     // Load by fid.
     file_test_reset();
-    $by_fid_file = file_load($file->fid);
-    $this->assertFileHookCalled('load', 0);
+    $by_fid_files = file_load_multiple(array($file->fid), array());
+    $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->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.
  */
@@ -1061,4 +1102,4 @@ class FileSaveDataTest extends FileHookTestCase {
     $file = file_save_data($contents, 'asdf.txt', FILE_EXISTS_ERROR);
     $this->assertFalse($file, t("Overwriting a file fails when FILE_EXISTS_ERROR is specified."));
   }
-}
\ No newline at end of file
+}
diff --git a/modules/simpletest/tests/file_test.module b/modules/simpletest/tests/file_test.module
index 4e7c7e965..7a9644f0d 100644
--- a/modules/simpletest/tests/file_test.module
+++ b/modules/simpletest/tests/file_test.module
@@ -148,11 +148,13 @@ function file_test_set_return($op, $value) {
 /**
  * Implementation of hook_file_load().
  */
-function file_test_file_load($file) {
-  _file_test_log_call('load', array($file));
-  // Assign a value on the object so that we can test that the $file is passed
-  // by reference.
-  $file->file_test['loaded'] = TRUE;
+function file_test_file_load($files) {
+  foreach ($files as $file) {
+    _file_test_log_call('load', array($file));
+    // Assign a value on the object so that we can test that the $file is passed
+    // by reference.
+    $file->file_test['loaded'] = TRUE;
+  }
 }
 
 /**
diff --git a/modules/system/system.api.php b/modules/system/system.api.php
index 4738e2347..3ac517fd4 100644
--- a/modules/system/system.api.php
+++ b/modules/system/system.api.php
@@ -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
- * into the $file.
+ * file_load_multiple() calls this hook to allow modules to load
+ * additional information into each file.
  *
- * @param $file
- *   The file object being loaded.
- * @return
- *   None.
+ * @param $files
+ *   An array of file objects, indexed by fid.
  *
- * @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.
-  $values = db_query('SELECT * FROM {upload} u WHERE u.fid = :fid', array(':fid' => $file->fid))->fetch(PDO::FETCH_ASSOC);
-  foreach ((array)$values as $key => $value) {
-    $file->{$key} = $value;
+  $result = db_query('SELECT * FROM {upload} u WHERE u.fid IN (:fids)', array(':fids' => array_keys($files)))->fetchAll(PDO::FETCH_ASSOC);
+  foreach ($result as $record) {
+    foreach ($record as $key => $value) {
+      $files[$record['fid']]->$key = $value;
+    }
   }
 }
 
diff --git a/modules/upload/upload.module b/modules/upload/upload.module
index 74f95783d..3b9a87b6b 100644
--- a/modules/upload/upload.module
+++ b/modules/upload/upload.module
@@ -269,11 +269,13 @@ function upload_form_alter(&$form, $form_state, $form_id) {
 /**
  * Implementation of hook_file_load().
  */
-function upload_file_load(&$file) {
+function upload_file_load($files) {
   // 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);
-  foreach ((array)$values as $key => $value) {
-    $file->{$key} = $value;
+  $result = db_query('SELECT * FROM {upload} u WHERE u.fid IN (:fids)', array(':fids' => array_keys($files)))->fetchAll(PDO::FETCH_ASSOC);
+  foreach ($result as $record) {
+    foreach ($record as $key => $value) {
+      $files[$record['fid']]->$key = $value;
+    }
   }
 }
 
@@ -297,16 +299,42 @@ function upload_file_delete(&$file) {
   db_delete('upload')->condition('fid', $file->fid)->execute();
 }
 
-
 /**
  * Implementation of hook_nodeapi_load().
  */
 function upload_nodeapi_load($nodes, $types) {
+  // Collect all the revision ids for nodes with upload enabled.
+  $node_vids = array();
   foreach ($nodes as $node) {
     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) {
         );
       }
     }
-    
+
     upload_nodeapi_links($node, $teaser);
   }
 }
@@ -609,19 +637,6 @@ function theme_upload_form_new($form) {
   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.
  */