diff --git includes/file.inc includes/file.inc index bb1f825..34a2bd2 100644 --- includes/file.inc +++ includes/file.inc @@ -495,6 +495,7 @@ function file_load_multiple($fids = array(), $conditions = array()) { * * @param $fid * A file ID. + * * @return * A file object. * @@ -542,7 +543,134 @@ function file_save(stdClass $file) { } /** - * Copy a file to a new location and adds a file record to the database. + * Determines where a file is used. + * + * @param $file + * A file object. + * + * @return + * A nested array with usage data. The first level is keyed by module name, + * the second by object type, the third has 'id' and 'count' keys. + * + * @see file_usage_add() + * @see file_usage_delete() + */ +function file_usage_list(stdClass $file) { + $result = db_select('file_usage', 'f') + ->fields('f', array('module', 'type', 'id', 'count')) + ->condition('fid', $file->fid) + ->condition('count', 0, '>') + ->execute(); + $references = array(); + foreach ($result as $usage) { + $references[$usage->module][$usage->type] = array('id' => $usage->id, 'count' => $usage->count); + } + return $references; +} + +/** + * Records that a module is using a file. + * + * This usage information will be queried during file_delete() to ensure that + * a file is not in use before it is physically removed from disk. + * + * Examples: + * - A module that associates files with nodes, so $type would be + * 'node' and $id would be the node's nid. Files for all revisions are stored + * within a single nid. + * - The User module associates an image with a user, so $type would be 'user' + * and the $id would be the user's uid. + * + * @param $file + * A file object. + * @param $module + * The name of the module using the file. + * @param $type + * The name of the object where the file is referenced. + * @param $id + * The type of the object containing the file reference. + * @param $count + * (optional) The number of references to add to the object. Defaults to 1. + * + * @see file_usage_list() + * @see file_usage_delete() + */ +function file_usage_add(stdClass $file, $module, $type, $id, $count = 1) { + db_merge('file_usage') + ->key(array( + 'fid' => $file->fid, + 'module' => $module, + 'type' => $type, + 'id' => $id, + )) + ->fields(array('count' => $count)) + ->expression('count', 'count + :count', array(':count' => $count)) + ->execute(); +} + +/** + * Removes a record to indicate that a module is no longer using a file. + * + * If no usage records for a file remain in the database, the file will be + * deleted using file_delete(). + * + * @param $file + * A file object. + * @param $module + * The name of the module using the file. + * @param $type + * (optional) The type of the object that contains the referenced file. May + * be omitted if all module references to a file are being deleted. + * @param $id + * (optional) The unique, numeric ID of the object containing the referenced + * file. May be omitted if all module references to a file are being deleted. + * @param $count + * (optional) The number of references to delete from the object. Defaults to + * 1. 0 may be specified to delete all references to the file within a + * specific object. + * + * @return + * TRUE for success, or FALSE if no usages remain but the file still could + * not be deleted. + * + * @see file_usage_add() + * @see file_usage_list() + * @see file_delete() + */ +function file_usage_delete(stdClass $file, $module, $type = NULL, $id = NULL, $count = 1) { + // Delete rows that have a exact or less value to prevent empty rows. + $query = db_delete('file_usage') + ->condition('module', $module) + ->condition('fid', $file->fid); + if ($type && $id) { + $query + ->condition('type', $type) + ->condition('id', $id); + } + if ($count) { + $query->condition('count', $count, '<='); + } + $result = $query->execute(); + + // If the row has more than the specified count decrement it by that number. + if (!$result) { + $query = db_update('file_usage') + ->condition('module', $module) + ->condition('fid', $file->fid); + if ($type && $id) { + $query + ->condition('type', $type) + ->condition('id', $id); + } + if ($count) { + $query->expression('count', 'count - :count', array(':count' => $count)); + } + $query->execute(); + } +} + +/** + * Copies a file to a new location and adds a file record to the database. * * This function should be used when manipulating files that have records * stored in the database. This is a powerful function that in many ways @@ -609,7 +737,7 @@ function file_copy(stdClass $source, $destination = NULL, $replace = FILE_EXISTS } /** - * Copy a file to a new location without invoking the file API. + * Copies a file to a new location without invoking the file API. * * This is a powerful function that in many ways performs like an advanced * version of copy(). @@ -966,30 +1094,30 @@ function file_create_filename($basename, $directory) { /** * Delete a file and its database record. * - * If the $force parameter is not TRUE hook_file_references() will be called - * to determine if the file is being used by any modules. If the file is being - * used is the delete will be canceled. + * If the $force parameter is not TRUE, file_usage_list() will be called to + * determine if the file is being used by any modules. If the file is being + * used the delete will be canceled. * * @param $file * A file object. * @param $force - * Boolean indicating that the file should be deleted even if - * hook_file_references() reports that the file is in use. + * Boolean indicating that the file should be deleted even if the file is + * reported as in use by the file_usage table. * * @return mixed * TRUE for success, FALSE in the event of an error, or an array if the file - * is being used by another module. The array keys are the module's name and - * the values are the number of references. + * is being used by any modules. * * @see file_unmanaged_delete() - * @see hook_file_references() + * @see file_usage_list() + * @see file_usage_delete() * @see hook_file_delete() */ function file_delete(stdClass $file, $force = FALSE) { - // If any module returns a value from the reference hook, the file will not - // be deleted from Drupal, but file_delete will return a populated array that - // tests as TRUE. - if (!$force && ($references = module_invoke_all('file_references', $file))) { + // If any module still has a usage entry in the file_usage table, the file + // will not be deleted, but file_delete() will return a populated array + // that tests as TRUE. + if (!$force && ($references = file_usage_list($file))) { return $references; } @@ -1000,6 +1128,7 @@ function file_delete(stdClass $file, $force = FALSE) { // database, so UIs can still find the file in the database. if (file_unmanaged_delete($file->uri)) { db_delete('file_managed')->condition('fid', $file->fid)->execute(); + db_delete('file_usage')->condition('fid', $file->fid)->execute(); return TRUE; } return FALSE; diff --git modules/file/file.field.inc modules/file/file.field.inc index c55d369..16f206a 100644 --- modules/file/file.field.inc +++ modules/file/file.field.inc @@ -263,37 +263,65 @@ function file_field_presave($entity_type, $entity, $field, $instance, $langcode, } /** + * Implements hook_field_insert(). + */ +function file_field_insert($entity_type, $entity, $field, $instance, $langcode, &$items) { + list($id, $vid, $bundle) = entity_extract_ids($entity_type, $entity); + + // Add a new usage of each uploaded file. + foreach ($items as $item) { + $file = (object) $item; + file_usage_add($file, 'file', $entity_type, $id); + } +} + +/** * Implements hook_field_update(). * - * Check for files that have been removed from the object. + * Checks for files that have been removed from the object. */ function file_field_update($entity_type, $entity, $field, $instance, $langcode, &$items) { - // On new revisions, old files are always maintained in the previous revision. + list($id, $vid, $bundle) = entity_extract_ids($entity_type, $entity); + + // On new revisions, all files are considered to be a new usage and no + // deletion of previous file usages are necessary. if (!empty($entity->revision)) { + foreach ($items as $item) { + $file = (object) $item; + file_usage_add($file, 'file', $entity_type, $id); + } return; } // Build a display of the current FIDs. - $fids = array(); + $current_fids = array(); foreach ($items as $item) { - $fids[] = $item['fid']; + $current_fids[] = $item['fid']; } - // Get the current values in the entity, and delete files for removed items. - list($id) = entity_extract_ids($entity_type, $entity); - $original = clone $entity; + // Create a bare-bones entity so that we can load its previous values. + $original = entity_create_stub_entity($entity_type, array($id, $vid, $bundle)); field_attach_load($entity_type, array($id => $original), FIELD_LOAD_CURRENT, array('field_id' => $field['id'])); + // Compare the original field values with the ones that are being saved. + $original_fids = array(); if (!empty($original->{$field['field_name']}[$langcode])) { foreach ($original->{$field['field_name']}[$langcode] as $original_item) { - if (isset($original_item['fid']) && !in_array($original_item['fid'], $fids)) { - // For hook_file_references, remember that this is being deleted. - $original_item['file_field_name'] = $field['field_name']; - // Delete the file if possible. - file_field_delete_file($original_item, $field); + $original_fids[] = $original_item['fid']; + if (isset($original_item['fid']) && !in_array($original_item['fid'], $current_fids)) { + // Decrement the file usage count by 1 and delete the file if possible. + file_field_delete_file($original_item, $field, $entity_type, $id); } } } + + // Add new usage entries for newly added files. + foreach ($items as $item) { + if (!in_array($item['fid'], $original_fids)) { + $file = (object) $item; + file_usage_add($file, 'file', $entity_type, $id); + } + } } /** @@ -301,14 +329,10 @@ function file_field_update($entity_type, $entity, $field, $instance, $langcode, */ function file_field_delete($entity_type, $entity, $field, $instance, $langcode, &$items) { list($id, $vid, $bundle) = entity_extract_ids($entity_type, $entity); + + // Delete all file usages within this entity. foreach ($items as $delta => $item) { - // For hook_file_references(), remember that this is being deleted. - $item['file_field_name'] = $field['field_name']; - // Pass in the ID of the object that is being removed so all references can - // be counted in hook_file_references(). - $item['file_field_type'] = $entity_type; - $item['file_field_id'] = $id; - file_field_delete_file($item, $field); + file_field_delete_file($item, $field, $entity_type, $id, 0); } } @@ -316,32 +340,46 @@ function file_field_delete($entity_type, $entity, $field, $instance, $langcode, * Implements hook_field_delete_revision(). */ function file_field_delete_revision($entity_type, $entity, $field, $instance, $langcode, &$items) { + list($id, $vid, $bundle) = entity_extract_ids($entity_type, $entity); foreach ($items as $delta => $item) { - // For hook_file_references, remember that this file is being deleted. - $item['file_field_name'] = $field['field_name']; - if (file_field_delete_file($item, $field)) { + // Decrement the file usage count by 1 and delete the file if possible. + if (file_field_delete_file($item, $field, $entity_type, $id)) { $items[$delta] = NULL; } } } /** - * Check that File controls a file before attempting to delete it. + * Decrements a file usage count and attempts to delete it. + * + * This function only has an effect if the file being deleted is controlled by + * File module. + * + * @param $item + * The field item that contains a file array. + * @param $field + * The field structure for the operation. + * @param $entity_type + * The type of $entity. + * @param $id + * The entity ID which contains the file being deleted. + * @param $count + * (optional) The number of references to decrement from the object containing + * the file. Defaults to 1. + * + * @return + * Boolean TRUE if the file was deleted, or an array of remaining references + * if the file is still in use by other modules. Boolean FALSE if an error was + * encountered. */ -function file_field_delete_file($item, $field) { - // Remove the file_field_name and file_field_id properties so that references - // can be counted including the files to be deleted. - $field_name = isset($item['file_field_name']) ? $item['file_field_name'] : NULL; - $field_id = isset($item['file_field_id']) ? $item['file_field_id'] : NULL; - unset($item['file_field_name'], $item['file_field_id']); - +function file_field_delete_file($item, $field, $entity_type, $id, $count = 1) { // To prevent the file field from deleting files it doesn't know about, check // the file reference count. Temporary files can be deleted because they // are not yet associated with any content at all. $file = (object) $item; - if ($file->status == 0 || file_get_file_reference_count($file, $field) > 0) { - $file->file_field_name = $field_name; - $file->file_field_id = $field_id; + $file_usage = file_usage_list($file); + if ($file->status == 0 || !empty($file_usage['file'])) { + file_usage_delete($file, 'file', $entity_type, $id, $count); return file_delete($file); } diff --git modules/file/file.module modules/file/file.module index 4075d65..6aed23d 100644 --- modules/file/file.module +++ modules/file/file.module @@ -321,14 +321,6 @@ function file_progress_implementation() { } /** - * Implements hook_file_references(). - */ -function file_file_references($file) { - $count = file_get_file_reference_count($file, NULL, 'file'); - return $count ? array('file' => $count) : NULL; -} - -/** * Implements hook_file_delete(). */ function file_file_delete($file) { @@ -520,11 +512,8 @@ function file_managed_file_validate(&$element, &$form_state) { if ($clicked_button != 'remove_button' && !empty($element['fid']['#value'])) { if ($file = file_load($element['fid']['#value'])) { if ($file->status == FILE_STATUS_PERMANENT) { - $reference_count = 0; - foreach (module_invoke_all('file_references', $file) as $module => $references) { - $reference_count += $references; - } - if ($reference_count == 0) { + $references = file_usage_list($file); + if (empty($references)) { form_error($element, t('Referencing to the file used in the !name field is not allowed.', array('!name' => $element['#title']))); } } @@ -929,73 +918,7 @@ function file_icon_map($file) { */ /** - * Count the number of times the file is referenced. - * - * @param $file - * A file object. - * @param $field - * (optional) A CCK field array or field name as a string. If provided, - * limits the reference check to the given field. - * @param $field_type - * (optional) The name of a field type. If provided, limits the reference - * check to fields of the given type. - * @return - * An integer value. - */ -function file_get_file_reference_count($file, $field = NULL, $field_type = NULL) { - // Determine the collection of fields to check. - if (isset($field)) { - // Support $field as 'field name'. - if (is_string($field)) { - $field = field_info_field($field); - } - $fields = array($field['field_name'] => $field); - } - else { - $fields = field_info_fields(); - } - - $types = entity_get_info(); - $reference_count = 0; - - foreach ($fields as $field) { - if (empty($field_type) || $field['type'] == $field_type) { - // TODO: Use a more efficient mechanism rather than actually retrieving - // all the references themselves, such as using a COUNT() query. - $references = file_get_file_references($file, $field, FIELD_LOAD_REVISION, $field_type); - foreach ($references as $entity_type => $type_references) { - $reference_count += count($type_references); - } - - // If a field_name is present in the file object, the file is being deleted - // from this field. - if (isset($file->file_field_name) && $field['field_name'] == $file->file_field_name) { - // If deleting the entire piece of content, decrement references. - if (isset($file->file_field_type) && isset($file->file_field_id)) { - if ($file->file_field_type == $entity_type) { - $info = entity_get_info($entity_type); - $id = $types[$entity_type]['entity keys']['id']; - foreach ($type_references as $reference) { - if ($file->file_field_id == $reference->$id) { - $reference_count--; - } - } - } - } - // Otherwise we're just deleting a single reference in this field. - else { - $reference_count--; - } - } - } - } - - return $reference_count; -} - - -/** - * Get a list of references to a file. + * Gets a list of references to a file. * * @param $file * A file object. @@ -1007,8 +930,9 @@ function file_get_file_reference_count($file, $field = NULL, $field_type = NULL) * FIELD_LOAD_REVISION to retrieve all references within all revisions or * FIELD_LOAD_CURRENT to retrieve references only in the current revisions. * @param $field_type - * Optional. The name of a field type. If given, limits the reference check to - * fields of the given type. + * (optional) The name of a field type. If given, limits the reference check + * to fields of the given type. + * * @return * An integer value. */ diff --git modules/file/tests/file.test modules/file/tests/file.test index 51e3b2d..0e413bd 100644 --- modules/file/tests/file.test +++ modules/file/tests/file.test @@ -170,7 +170,7 @@ class FileFieldTestCase extends DrupalWebTestCase { * Assert that a file exists in the database. */ function assertFileEntryExists($file, $message = NULL) { - drupal_static_reset('file_load_multiple'); + entity_get_controller('file')->resetCache(); $db_file = file_load($file->fid); $message = isset($message) ? $message : t('File %file exists in database at the correct path.', array('%file' => $file->uri)); $this->assertEqual($db_file->uri, $file->uri, $message); @@ -188,7 +188,7 @@ class FileFieldTestCase extends DrupalWebTestCase { * Assert that a file does not exist in the database. */ function assertFileEntryNotExists($file, $message) { - drupal_static_reset('file_load_multiple'); + entity_get_controller('file')->resetCache(); $message = isset($message) ? $message : t('File %file exists in database at the correct path.', array('%file' => $file->uri)); $this->assertFalse(file_load($file->fid), $message); } @@ -368,7 +368,7 @@ class FileFieldRevisionTestCase extends FileFieldTestCase { // Attach the second file to a user. $user = $this->drupalCreateUser(); - $edit = array(); + $edit = (array) $user; $edit[$field_name][LANGUAGE_NONE][0] = (array) $node_file_r3; user_save($user, $edit); $this->drupalGet('user/' . $user->uid . '/edit'); diff --git modules/image/image.field.inc modules/image/image.field.inc index 87fbd31..1d59c45 100644 --- modules/image/image.field.inc +++ modules/image/image.field.inc @@ -233,6 +233,13 @@ function image_field_presave($entity_type, $entity, $field, $instance, $langcode } /** + * Implements hook_field_insert(). + */ +function image_field_insert($entity_type, $entity, $field, $instance, $langcode, &$items) { + file_field_insert($entity_type, $entity, $field, $instance, $langcode, $items); +} + +/** * Implements hook_field_update(). */ function image_field_update($entity_type, $entity, $field, $instance, $langcode, &$items) { diff --git modules/image/image.module modules/image/image.module index 50be71c..5d629c6 100644 --- modules/image/image.module +++ modules/image/image.module @@ -307,14 +307,6 @@ function image_file_delete($file) { } /** - * Implements hook_file_references(). - */ -function image_file_references($file) { - $count = file_get_file_reference_count($file, NULL, 'image'); - return $count ? array('image' => $count) : NULL; -} - -/** * Implements hook_image_default_styles(). */ function image_image_default_styles() { diff --git modules/node/node.api.php modules/node/node.api.php index 490205e..11a833d 100644 --- modules/node/node.api.php +++ modules/node/node.api.php @@ -406,13 +406,9 @@ function hook_node_delete($node) { * @ingroup node_api_hooks */ function hook_node_revision_delete($node) { - db_delete('upload')->condition('vid', $node->vid)->execute(); - if (!is_array($node->files)) { - return; - } - foreach ($node->files as $file) { - file_delete($file); - } + db_delete('mytable') + ->condition('vid', $node->vid) + ->execute(); } /** diff --git modules/simpletest/tests/file.test modules/simpletest/tests/file.test index 192674f..38fd2f4 100644 --- modules/simpletest/tests/file.test +++ modules/simpletest/tests/file.test @@ -230,13 +230,13 @@ class FileHookTestCase extends FileTestCase { $this->assertTrue(FALSE, t('Expected hooks %expected to be called but %uncalled was not called.', array('%expected' => implode(', ', $expected), '%uncalled' => implode(', ', $uncalled)))); } else { - $this->assertTrue(TRUE, t('All the expected hooks were called: %expected', array('%expected' => implode(', ', $expected)))); + $this->assertTrue(TRUE, t('All the expected hooks were called: %expected', array('%expected' => empty($expected) ? t('(none)') : implode(', ', $expected)))); } // Determine if there were any unexpected calls. $unexpected = array_diff($actual, $expected); if (count($unexpected)) { - $this->assertTrue(FALSE, t('Unexpected hooks were called: %unexpected.', array('%unexpected' => implode(', ', $unexpected)))); + $this->assertTrue(FALSE, t('Unexpected hooks were called: %unexpected.', array('%unexpected' => empty($unexpected) ? t('(none)') : implode(', ', $unexpected)))); } else { $this->assertTrue(TRUE, t('No unexpected hooks were called.')); @@ -1400,20 +1400,44 @@ class FileDeleteTest extends FileHookTestCase { } /** - * Try deleting a normal file (as opposed to a directory, symlink, etc). + * Tries deleting a normal file (as opposed to a directory, symlink, etc). */ - function testNormal() { + function testUnused() { $file = $this->createFile(); // Check that deletion removes the file and database record. - $this->assertTrue(is_file($file->uri), t("File exists.")); - $this->assertIdentical(file_delete($file), TRUE, t("Delete worked.")); - $this->assertFileHooksCalled(array('references', 'delete')); - $this->assertFalse(file_exists($file->uri), t("Test file has actually been deleted.")); + $this->assertTrue(is_file($file->uri), t('File exists.')); + $this->assertIdentical(file_delete($file), TRUE, t('Delete worked.')); + $this->assertFileHooksCalled(array('delete')); + $this->assertFalse(file_exists($file->uri), t('Test file has actually been deleted.')); $this->assertFalse(file_load($file->fid), t('File was removed from the database.')); + } + + /** + * Tries deleting a file that is in use. + */ + function testInUse() { + $file = $this->createFile(); + file_usage_add($file, 'testing', 'test', 1); + file_usage_add($file, 'testing', 'test', 1); - // TODO: implement hook_file_references() in file_test.module and report a - // file in use and test the $force parameter. + file_usage_delete($file, 'testing', 'test', 1); + file_delete($file); + $usage = file_usage_list($file); + $this->assertEqual($usage['testing']['test'], array('id' => 1, 'count' => 1), t('Test file is still in use.')); + $this->assertTrue(file_exists($file->uri), t('File still exists on the disk.')); + $this->assertTrue(file_load($file->fid), t('File still exists in the database.')); + + // Clear out the call to hook_file_load(). + file_test_reset(); + + file_usage_delete($file, 'testing', 'test', 1); + file_delete($file); + $usage = file_usage_list($file); + $this->assertFileHooksCalled(array('delete')); + $this->assertTrue(empty($usage), t('File usage data was removed.')); + $this->assertFalse(file_exists($file->uri), t('File has been deleted after its last usage was removed.')); + $this->assertFalse(file_load($file->fid), t('File was removed from the database.')); } } @@ -1515,7 +1539,7 @@ class FileMoveTest extends FileHookTestCase { $this->assertTrue($result, t('File moved sucessfully.')); // Check that the correct hooks were called. - $this->assertFileHooksCalled(array('move', 'update', 'delete', 'references', 'load')); + $this->assertFileHooksCalled(array('move', 'update', 'delete', 'load')); // Reload the file from the database and check that the changes were // actually saved. @@ -1864,6 +1888,108 @@ class FileSaveTest extends FileHookTestCase { } } +/** + * Tests file usage functions. + */ +class FileUsageTest extends FileTestCase { + function getInfo() { + return array( + 'name' => 'File usage', + 'description' => 'Tests the file usage functions.', + 'group' => 'File', + ); + } + + /** + * Tests file_usage_list(). + */ + function testGetUsage() { + $file = $this->createFile(); + db_insert('file_usage') + ->fields(array( + 'fid' => $file->fid, + 'module' => 'testing', + 'type' => 'foo', + 'id' => 1, + 'count' => 1 + )) + ->execute(); + db_insert('file_usage') + ->fields(array( + 'fid' => $file->fid, + 'module' => 'testing', + 'type' => 'bar', + 'id' => 2, + 'count' => 2 + )) + ->execute(); + + $usage = file_usage_list($file); + + $this->assertEqual(count($usage['testing']), 2, t('Returned the correct number of items.')); + $this->assertEqual($usage['testing']['foo']['id'], 1, t('Returned the correct id.')); + $this->assertEqual($usage['testing']['bar']['id'], 2, t('Returned the correct id.')); + $this->assertEqual($usage['testing']['foo']['count'], 1, t('Returned the correct count.')); + $this->assertEqual($usage['testing']['bar']['count'], 2, t('Returned the correct count.')); + } + + /** + * Tests file_usage_add(). + */ + function testAddUsage() { + $file = $this->createFile(); + file_usage_add($file, 'testing', 'foo', 1); + // Add the file twice to ensure that the count is incremented rather than + // creating additional records. + file_usage_add($file, 'testing', 'bar', 2); + file_usage_add($file, 'testing', 'bar', 2); + + $usage = db_select('file_usage', 'f') + ->fields('f') + ->condition('f.fid', $file->fid) + ->execute() + ->fetchAllAssoc('id'); + $this->assertEqual(count($usage), 2, t('Created two records')); + $this->assertEqual($usage[1]->module, 'testing', t('Correct module')); + $this->assertEqual($usage[2]->module, 'testing', t('Correct module')); + $this->assertEqual($usage[1]->type, 'foo', t('Correct type')); + $this->assertEqual($usage[2]->type, 'bar', t('Correct type')); + $this->assertEqual($usage[1]->count, 1, t('Correct count')); + $this->assertEqual($usage[2]->count, 2, t('Correct count')); + } + + /** + * Tests file_usage_delete(). + */ + function testRemoveUsage() { + $file = $this->createFile(); + db_insert('file_usage') + ->fields(array( + 'fid' => $file->fid, + 'module' => 'testing', + 'type' => 'bar', + 'id' => 2, + 'count' => 2 + )) + ->execute(); + + file_usage_delete($file, 'testing', 'bar', 2); + $count = db_select('file_usage', 'f') + ->fields('f', array('count')) + ->condition('f.fid', $file->fid) + ->execute() + ->fetchField(); + $this->assertEqual(1, $count, t('The count was decremented correctly.')); + + file_usage_delete($file, 'testing', 'bar', 2); + $count = db_select('file_usage', 'f') + ->fields('f', array('count')) + ->condition('f.fid', $file->fid) + ->execute() + ->fetchField(); + $this->assertEqual(0, $count, t('The count was decremented correctly.')); + } +} /** * Tests the file_validate() function.. diff --git modules/simpletest/tests/file_test.module modules/simpletest/tests/file_test.module index 033c43d..7180b7f 100644 --- modules/simpletest/tests/file_test.module +++ modules/simpletest/tests/file_test.module @@ -143,7 +143,6 @@ function file_test_reset() { 'load' => array(), 'validate' => array(), 'download' => array(), - 'references' => array(), 'insert' => array(), 'update' => array(), 'copy' => array(), @@ -156,7 +155,6 @@ function file_test_reset() { $return = array( 'validate' => array(), 'download' => NULL, - 'references' => NULL, ); variable_set('file_test_return', $return); } @@ -167,7 +165,7 @@ function file_test_reset() { * * @param $op * One of the hook_file_* operations: 'load', 'validate', 'download', - * 'references', 'insert', 'update', 'copy', 'move', 'delete'. + * 'insert', 'update', 'copy', 'move', 'delete'. * * @return * Array of the parameters passed to each call. @@ -184,9 +182,9 @@ function file_test_get_calls($op) { * Get an array with the calls for all hooks. * * @return - * An array keyed by hook name ('load', 'validate', 'download', - * 'references', 'insert', 'update', 'copy', 'move', 'delete') with values - * being arrays of parameters passed to each call. + * An array keyed by hook name ('load', 'validate', 'download', 'insert', + * 'update', 'copy', 'move', 'delete') with values being arrays of parameters + * passed to each call. */ function file_test_get_all_calls() { return variable_get('file_test_results', array()); @@ -197,7 +195,7 @@ function file_test_get_all_calls() { * * @param $op * One of the hook_file_* operations: 'load', 'validate', 'download', - * 'references', 'insert', 'update', 'copy', 'move', 'delete'. + * 'insert', 'update', 'copy', 'move', 'delete'. * @param $args * Values passed to hook. * @@ -214,7 +212,7 @@ function _file_test_log_call($op, $args) { * Load the appropriate return value. * * @param $op - * One of the hook_file_[validate,download,references] operations. + * One of the hook_file_[validate,download] operations. * * @return * Value set by file_test_set_return(). @@ -231,7 +229,7 @@ function _file_test_get_return($op) { * Assign a return value for a given operation. * * @param $op - * One of the hook_file_[validate,download,references] operations. + * One of the hook_file_[validate,download] operations. * @param $value * Value for the hook to return. * @@ -273,14 +271,6 @@ function file_test_file_download($uri) { } /** - * Implements hook_file_references(). - */ -function file_test_file_references($file) { - _file_test_log_call('references', array($file)); - return _file_test_get_return('references'); -} - -/** * Implements hook_file_insert(). */ function file_test_file_insert($file) { diff --git modules/system/system.api.php modules/system/system.api.php index 393f6e7..f6071de 100644 --- modules/system/system.api.php +++ modules/system/system.api.php @@ -2303,31 +2303,6 @@ function hook_file_move($file, $source) { } /** - * Report the number of times a file is referenced by a module. - * - * This hook is called to determine if a files is in use. Multiple modules may - * be referencing the same file and to prevent one from deleting a file used by - * another this hook is called. - * - * @param $file - * The file object being checked for references. - * @return - * If the module uses this file return an array with the module name as the - * key and the value the number of times the file is used. - * - * @see file_delete() - * @see upload_file_references() - */ -function hook_file_references($file) { - // If user.module is still using a file, do not let other modules delete it. - $file_used = (bool) db_query_range('SELECT 1 FROM {user} WHERE pictire = :fid', 0, 1, array(':fid' => $file->fid))->fetchField(); - if ($file_used) { - // Return the name of the module and how many references it has to the file. - return array('user' => 1); - } -} - -/** * Respond to a file being deleted. * * @param $file diff --git modules/system/system.install modules/system/system.install index 0bfb34f..7e9b123 100644 --- modules/system/system.install +++ modules/system/system.install @@ -832,6 +832,52 @@ function system_schema() { ), ); + $schema['file_usage'] = array( + 'description' => 'Track where a file is used.', + 'fields' => array( + 'fid' => array( + 'description' => 'File ID.', + 'type' => 'int', + 'unsigned' => TRUE, + 'not null' => TRUE, + ), + 'module' => array( + 'description' => 'The name of the module that is using the file.', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'type' => array( + 'description' => 'The name of the object type in which the file is used.', + 'type' => 'varchar', + 'length' => 64, + 'not null' => TRUE, + 'default' => '', + ), + 'id' => array( + 'description' => 'The primary key of the object using the file.', + 'type' => 'int', + 'unsigned' => TRUE, + 'not null' => TRUE, + 'default' => 0, + ), + 'count' => array( + 'description' => 'The number of times this file is used by this object.', + 'type' => 'int', + 'unsigned' => TRUE, + 'not null' => TRUE, + 'default' => 0, + ), + ), + 'primary key' => array('fid', 'type', 'id', 'module'), + 'indexes' => array( + 'type_id' => array('type', 'id'), + 'fid_count' => array('fid', 'count'), + 'fid_module' => array('fid', 'module'), + ), + ); + $schema['flood'] = array( 'description' => 'Flood controls the threshold of events, such as the number of contact attempts.', 'fields' => array( @@ -2769,6 +2815,10 @@ function system_update_7059(&$sandbox) { // Update the node field with the file URI. $revision['file'][LANGUAGE_NONE][$delta] = $file; + + // Add the usage entry for the file. + $file = (object) $file; + file_usage_add($file, 'file', 'node', $revision->nid); } // Insert the revision's files into the field_upload table. @@ -2792,6 +2842,58 @@ function system_update_7059(&$sandbox) { } /** + * Create the file_usage table. + */ +function system_update_7060() { + $spec = array( + 'description' => 'Track where a file is used.', + 'fields' => array( + 'fid' => array( + 'description' => 'File ID.', + 'type' => 'int', + 'unsigned' => TRUE, + 'not null' => TRUE, + ), + 'module' => array( + 'description' => 'The name of the module that is using the file.', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'type' => array( + 'description' => 'The name of the object type in which the file is used.', + 'type' => 'varchar', + 'length' => 64, + 'not null' => TRUE, + 'default' => '', + ), + 'id' => array( + 'description' => 'The primary key of the object using the file.', + 'type' => 'int', + 'unsigned' => TRUE, + 'not null' => TRUE, + 'default' => 0, + ), + 'count' => array( + 'description' => 'The number of times this file is used by this object.', + 'type' => 'int', + 'unsigned' => TRUE, + 'not null' => TRUE, + 'default' => 0, + ), + ), + 'primary key' => array('fid', 'type', 'id', 'module'), + 'indexes' => array( + 'type_id' => array('type', 'id'), + 'fid_count' => array('fid', 'count'), + 'fid_module' => array('fid', 'module'), + ), + ); + db_create_table('file_usage', $spec); +} + +/** * @} End of "defgroup updates-6.x-to-7.x" * The next series of updates should start at 8000. */ diff --git modules/system/system.module modules/system/system.module index c963cb4..5294f29 100644 --- modules/system/system.module +++ modules/system/system.module @@ -2799,8 +2799,14 @@ function system_cron() { )); foreach ($result as $row) { if ($file = file_load($row->fid)) { - if (!file_delete($file)) { - watchdog('file system', 'Could not delete temporary file "%path" during garbage collection', array('%path' => $file->uri), WATCHDOG_ERROR); + $references = file_usage_list($file); + if (empty($references)) { + if (!file_delete($file)) { + watchdog('file system', 'Could not delete temporary file "%path" during garbage collection', array('%path' => $file->uri), WATCHDOG_ERROR); + } + } + else { + watchdog('file system', 'Did not delete temporary file "%path" during garbage collection, because it is in use by the following modules: %modules.', array('%path' => $file->uri, '%modules' => implode(', ', array_keys($references))), WATCHDOG_INFO); } } } diff --git modules/user/user.install modules/user/user.install index 6ed94a7..7e13114 100644 --- modules/user/user.install +++ modules/user/user.install @@ -336,6 +336,10 @@ function user_update_dependencies() { $dependencies['user'][7006] = array( 'filter' => 7002, ); + // user_update_7012 relies on system_update_7060. + $dependencies['user'][7012] = array( + 'system' => 7060, + ); return $dependencies; } @@ -715,6 +719,34 @@ function user_update_7011() { } /** + * Add user module file usage entries. + */ +function user_update_7012(&$sandbox) { + if (!isset($sandbox['progress'])) { + // Initialize batch update information. + $sandbox['progress'] = 0; + $sandbox['last_uid_processed'] = -1; + $sandbox['max'] = db_query("SELECT COUNT(*) FROM {users} u WHERE u.picture <> 0")->fetchField(); + } + + // Add usage entries for the user picture files. + $limit = 500; + $result = db_query_range('SELECT f.*, u.uid as user_uid FROM {users} u INNER JOIN {file_managed} f ON u.picture = f.fid WHERE u.picture <> 0 AND u.uid > :uid ORDER BY u.uid', 0, $limit, array(':uid' => $sandbox['last_uid_processed']))->fetchAllAssoc('fid', PDO::FETCH_ASSOC); + foreach ($result as $row) { + $uid = $row['user_uid']; + $file = (object) $row; + file_usage_add($file, 'user', 'user', $uid); + + // Update our progress information for the batch update. + $sandbox['progress']++; + $sandbox['last_uid_processed'] = $uid; + } + + // Indicate our current progress to the batch update system. + $sandbox['#finished'] = empty($sandbox['max']) || ($sandbox['progress'] / $sandbox['max']); +} + +/** * @} End of "defgroup user-updates-6.x-to-7.x" * The next series of updates should start at 8000. */ diff --git modules/user/user.module modules/user/user.module index f57657a..73eaeb3 100644 --- modules/user/user.module +++ modules/user/user.module @@ -439,12 +439,26 @@ function user_save($account, $edit = array(), $category = 'account') { file_prepare_directory($picture_directory, FILE_CREATE_DIRECTORY); $destination = file_stream_wrapper_uri_normalize($picture_directory . '/picture-' . $account->uid . '-' . REQUEST_TIME . '.' . $info['extension']); + // Delete the previous picture. This ensures thumbnails get cleared. + // Note that we force this deletion, since the physical file is about + // to be replaced by the new file. + if ($account->picture) { + file_delete($account->picture, TRUE); + } + + // Move the temporary file into the final location. if ($picture = file_move($picture, $destination, FILE_EXISTS_RENAME)) { $picture->status |= FILE_STATUS_PERMANENT; $edit['picture'] = file_save($picture); + file_usage_add($picture, 'user', 'user', $account->uid); } } } + // If the picture existed before and was unset, remove its reference. + elseif (!empty($account->picture->fid)) { + file_delete($account->picture, TRUE); + } + $edit['picture'] = empty($edit['picture']->fid) ? 0 : $edit['picture']->fid; // Do not allow 'uid' to be changed. @@ -457,13 +471,6 @@ function user_save($account, $edit = array(), $category = 'account') { return FALSE; } - // If the picture changed or was unset, remove the old one. This step needs - // to occur after updating the {users} record so that user_file_references() - // doesn't report it in use and block the deletion. - if (!empty($account->picture->fid) && ($edit['picture'] != $account->picture->fid)) { - file_delete($account->picture); - } - // Reload user roles if provided. if (isset($edit['roles']) && is_array($edit['roles'])) { db_delete('users_roles') @@ -829,15 +836,18 @@ function user_file_download($uri) { } /** - * Implements hook_file_references(). + * Implements hook_file_move(). */ -function user_file_references($file) { - // Determine if the file is used by this module. - $file_used = (bool) db_query_range('SELECT 1 FROM {users} WHERE picture = :fid', 0, 1, array(':fid' => $file->fid))->fetchField(); - if ($file_used) { - // Return the name of the module and how many references it has to the file. - // If file is still used then 1 is enough to indicate this. - return array('user' => 1); +function user_file_move($file, $source) { + // If a user's picture is replaced with a new one, update the record in + // the users table. + if (isset($file->fid) && isset($source->fid) && $file->fid != $source->fid) { + db_update('users') + ->fields(array( + 'picture' => $file->fid, + )) + ->condition('picture', $source->fid) + ->execute(); } }