diff --git a/includes/file.inc b/includes/file.inc index 00a9bd1..0896680 100644 --- a/includes/file.inc +++ b/includes/file.inc @@ -348,6 +348,103 @@ function file_save($file) { } /** + * Determine where a file is used. + * + * @param $file + * A file object. + * @return + * An nested array with usage data. The first level is keyed by module + * name, the second by table name, the third has 'id' and 'count' keys. + * + * @see file_add_usage() + * @see file_remove_usage() + */ +function file_get_usage(stdClass $file) { + $result = db_query('SELECT f.module, f.type, f.id, f.count FROM {file_usage} f WHERE f.fid = :fid AND count > 0', array(':fid' => $file->fid)); + $references = array(); + + foreach ($result as $usage) { + $references[$usage->module][$usage->type] = array('id' => $usage->id, 'count' => $usage->count); + } + return $references; +} + +/** + * Inform Drupal that a module is using a file. + * + * This usage information will be queried during file_delete(). + * + * Examples: + * - The upload module that associates files with node revisions so the + * @p $type would be 'node_revision' and @p $id would be the node's vid. + * - The user module associates an image with a user so the @p $type would be + * 'user' and the $id would be the user's id. + * + * @param $file + * A file object. + * @param $module + * The name of the module using the file. + * @param $type + * The name of the table where the file is referenced. + * @param $id + * The id of the row in the table. + * + * @see file_get_usage() + * @see file_remove_usage() + */ +function file_add_usage(stdClass $file, $module, $type, $id) { + db_merge('file_usage') + ->key(array( + 'fid' => $file->fid, + 'module' => $module, + 'type' => $type, + 'id' => $id, + )) + ->fields(array('count' => 1)) + ->expression('count', 'count + 1') + ->execute(); +} + +/** + * Inform Drupal that a module is no longer using a file. + * + * If no usages of the file remain in the database, the file itself will be + * deleted using file_delete(). + * + * @param $file + * A file object. + * @param $module + * The name of the module using the file. + * @param $type + * The name of the table where the file is referenced. + * @param $id + * The id of the row in the table. + * @return + * TRUE for success, or FALSE if no usages remain but the file still could + * not be deleted. + * + * @see file_add_usage() + * @see file_get_usage() + * @see file_delete() + */ +function file_remove_usage(stdClass $file, $module, $type, $id) { + db_update('file_usage')->expression('count', 'count - 1') + ->condition('fid', $file->fid) + ->condition('module', $module) + ->condition('type', $type) + ->condition('id', $id) + ->execute(); + + // Delete the file, unless it is still in use somewhere. + $references = db_result(db_query('SELECT COUNT(*) FROM {file_usage} f WHERE f.fid = :fid AND count > 0', array(':fid' => $file->fid))); + $args = func_get_args(); + if (empty($references)) { + return file_delete($file); + } + return TRUE; +} + +/** * Copy 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 @@ -585,6 +682,9 @@ function file_move($source, $destination = NULL, $replace = FILE_EXISTS_RENAME) // Inform modules that the file has been moved. module_invoke_all('file_move', $file, $source); + ///TODO: move fids in {file_usage} if the fid changed... probably needs to + /// be done by the various hook_file_move() implementations (e.g. core + /// can't update filefield's tables - or can we?) if ($delete_source) { // Try a soft delete to remove original if it's not in use elsewhere. @@ -724,34 +824,19 @@ 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. - * * @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. - * @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. + * @return + * TRUE for success, or FALSE in the event of an error. * * @see file_unmanaged_delete() - * @see hook_file_references() + * @see file_get_usage() + * @see file_remove_usage() * @see hook_file_delete() */ -function file_delete($file, $force = FALSE) { +function file_delete($file) { $file = (object)$file; - // 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))) { - return $references; - } - // Let other modules clean up any references to the deleted file. module_invoke_all('file_delete', $file); @@ -759,6 +844,7 @@ function file_delete($file, $force = FALSE) { // database, so UIs can still find the file in the database. if (file_unmanaged_delete($file->filepath)) { db_delete('files')->condition('fid', $file->fid)->execute(); + db_delete('file_usage')->condition('fid', $file->fid)->execute(); return TRUE; } return FALSE; diff --git a/modules/node/node.api.php b/modules/node/node.api.php index 25eb76b..56f1274 100644 --- a/modules/node/node.api.php +++ b/modules/node/node.api.php @@ -184,7 +184,7 @@ function hook_node_delete_revision($node) { return; } foreach ($node->files as $file) { - file_delete($file); + file_remove_usage($file); } } diff --git a/modules/simpletest/tests/file.test b/modules/simpletest/tests/file.test index df71b7b..617e5e6 100644 --- a/modules/simpletest/tests/file.test +++ b/modules/simpletest/tests/file.test @@ -1245,18 +1245,44 @@ class FileDeleteTest extends FileHookTestCase { /** * Try 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->filepath), t("File exists.")); $this->assertIdentical(file_delete($file), TRUE, t("Delete worked.")); - $this->assertFileHooksCalled(array('references', 'delete')); + $this->assertFileHooksCalled(array('delete')); $this->assertFalse(file_exists($file->filepath), t("Test file has actually been deleted.")); $this->assertFalse(file_load($file->fid), t('File was removed from the database.')); + $this->assertFalse(file_get_usage($file), t('File usage data was removed.')); + } + + /** + * Try deleting a file that is in use. + */ + function testInUse() { + $file = $this->createFile(); + file_add_usage($file, 'testing', 'test', 1); + file_add_usage($file, 'testing', 'test', 1); + + file_remove_usage($file, 'testing', 'test', 1); + $usage = file_get_usage($file); + $this->assertEqual($usage['testing']['test'], array('id' => 1, 'count' => 1), t('Delete failed, because it is known to be used somewhere still.')); + $this->assertFileHooksCalled(array()); - // TODO: implement hook_file_references() in file_test.module and report a - // file in use and test the $force parameter. + $this->assertTrue(file_exists($file->filepath), t('Test file still exists on the disk.')); + $this->assertTrue(file_load($file->fid), t('File still exists in the database.')); + $this->assertTrue(file_get_usage($file), t('File usage data still exists.')); + // Clear out the call to hook_file_load(). + file_test_reset(); + + file_remove_usage($file, 'testing', 'test', 1); + $usage = file_get_usage($file); + $this->assertTrue(empty($usage), t('Delete worked.')); + $this->assertFileHooksCalled(array('delete')); + $this->assertFalse(file_exists($file->filepath), t('Test file has actually been deleted.')); + $this->assertFalse(file_load($file->fid), t('File was removed from the database.')); + $this->assertFalse(file_get_usage($file), t('File usage data was removed.')); } } @@ -1358,7 +1384,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. @@ -1720,6 +1746,91 @@ class FileSaveTest extends FileHookTestCase { } } +/** + * Tests the file_get_usage(), file_add_usage() and file_remove_usage() + * functions. + */ +class FileUsageTest extends FileTestCase { + function getInfo() { + return array( + 'name' => t('File usage'), + 'description' => t('Tests the file usage functions.'), + 'group' => t('File'), + ); + } + + /** + * Test the file_get_usage() function. + */ + 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_get_usage($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.')); + } + + /** + * Test the file_add_usage() function. + */ + function testAddUsage() { + $file = $this->createFile(); + file_add_usage($file, 'testing', 'foo', 1); + // Add the file twice to ensure that the count is incremented rather than + // creating additional records. + file_add_usage($file, 'testing', 'bar', 2); + file_add_usage($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')); + } + + /** + * Test the file_remove_usage() function. + */ + function testRemoveUsage() { + $file = $this->createFile(); + db_insert('file_usage')->fields(array( + 'fid' => $file->fid, + 'module' => 'testing', + 'type' => 'bar', + 'id' => 2, + 'count' => 2 + ))->execute(); + + file_remove_usage($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_remove_usage($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 a/modules/simpletest/tests/file_test.module b/modules/simpletest/tests/file_test.module index c72610e..9f2d386 100644 --- a/modules/simpletest/tests/file_test.module +++ b/modules/simpletest/tests/file_test.module @@ -78,7 +78,6 @@ function file_test_reset() { 'load' => array(), 'validate' => array(), 'download' => array(), - 'references' => array(), 'insert' => array(), 'update' => array(), 'copy' => array(), @@ -91,7 +90,6 @@ function file_test_reset() { $return = array( 'validate' => array(), 'download' => NULL, - 'references' => NULL, ); variable_set('file_test_return', $return); } @@ -102,7 +100,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'. * @returns * Array of the parameters passed to each call. * @see _file_test_log_call() and file_test_reset() @@ -129,7 +127,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. * @see file_test_get_calls() and file_test_reset() @@ -144,7 +142,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(). * @see file_test_set_return() and file_test_reset(). @@ -158,7 +156,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. * @see _file_test_get_return() and file_test_reset(). @@ -198,14 +196,6 @@ function file_test_file_download($file) { } /** - * Implementation of hook_file_references(). - */ -function file_test_file_references($file) { - _file_test_log_call('references', array($file)); - return _file_test_get_return('references'); -} - -/** * Implementation of hook_file_insert(). */ function file_test_file_insert($file) { diff --git a/modules/system/system.api.php b/modules/system/system.api.php index cd5d1eb..ce2a573 100644 --- a/modules/system/system.api.php +++ b/modules/system/system.api.php @@ -1172,6 +1172,10 @@ function hook_file_copy($file, $source) { /** * Respond to a file that has been moved. * + * $file->fid stays the same except if the moved file replaced a previously + * existing one, in which case the fid and other properties of the existing + * target file is adopted for the new file object. + * * @param $file * The updated file object after the move. * @param $source @@ -1186,31 +1190,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 upload.module is still using a file, do not let other modules delete it. - $count = db_query('SELECT COUNT(*) FROM {upload} WHERE fid = :fid', array(':fid' => $file->fid))->fetchField(); - if ($count) { - // Return the name of the module and how many references it has to the file. - return array('upload' => $count); - } -} - -/** * Respond to a file being deleted. * * @param $file diff --git a/modules/system/system.install b/modules/system/system.install index c4f1013..4a5d1ca 100644 --- a/modules/system/system.install +++ b/modules/system/system.install @@ -675,6 +675,50 @@ function system_schema() { 'primary key' => array('fid'), ); + $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 table where 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'), + ), + ); + $schema['flood'] = array( 'description' => 'Flood controls the threshold of events, such as the number of contact attempts.', 'fields' => array( @@ -3454,6 +3498,59 @@ function system_update_7023() { } /** + * Create the file_usage table. + */ +function system_update_7024() { + $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 table where 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'), + ), + ); + + $ret = array(); + db_create_table($ret, 'file_usage', $schema['file_usage']); + return $ret; +} + +/** * @} End of "defgroup updates-6.x-to-7.x" * The next series of updates should start at 8000. */ diff --git a/modules/system/system.module b/modules/system/system.module index aad2016..8733597 100644 --- a/modules/system/system.module +++ b/modules/system/system.module @@ -1576,8 +1576,12 @@ function system_cron() { $result = db_query('SELECT fid FROM {files} WHERE status & :permanent1 <> :permanent2 AND timestamp < :timestamp', array(':permanent1' => FILE_STATUS_PERMANENT, ':permanent2' => FILE_STATUS_PERMANENT, ':timestamp' => REQUEST_TIME - DRUPAL_MAXIMUM_TEMP_FILE_AGE)); 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->filepath), WATCHDOG_ERROR); + $references = file_get_usage($file); + if (empty($references)) { + file_delete($file); + } + else { + watchdog('file system', 'Could not delete temporary file "%path" during garbage collection, usage references left.', array('%path' => $file->filepath), WATCHDOG_ERROR); } } } diff --git a/modules/upload/upload.install b/modules/upload/upload.install index 0d825c8..5899b02 100644 --- a/modules/upload/upload.install +++ b/modules/upload/upload.install @@ -82,4 +82,11 @@ function upload_schema() { return $schema; } - +/** + * Create file_usage records for our files. + */ +function upload_update_7000() { + $ret = array(); + $ret[] = update_sql("INSERT INTO {file_usage} (fid, module, type, id, count) SELECT fid, 'upload', 'node_revision', vid, 1 FROM {upload}"); + return $ret; +} diff --git a/modules/upload/upload.module b/modules/upload/upload.module index bbbcce1..e4e910a 100644 --- a/modules/upload/upload.module +++ b/modules/upload/upload.module @@ -278,18 +278,6 @@ function upload_file_load($files) { } /** - * Implementation of hook_file_references(). - */ -function upload_file_references($file) { - // If upload.module is still using a file, do not let other modules delete it. - $count = db_query('SELECT COUNT(*) FROM {upload} WHERE fid = :fid', array(':fid' => $file->fid))->fetchField(); - if ($count) { - // Return the name of the module and how many references it has to the file. - return array('upload' => $count); - } -} - -/** * Implementation of hook_file_delete(). */ function upload_file_delete($file) { @@ -408,7 +396,7 @@ function upload_node_delete($node) { return; } foreach($node->files as $file) { - file_delete($file); + file_remove_usage($file); } } @@ -421,7 +409,7 @@ function upload_node_delete_revision($node) { return; } foreach ($node->files as $file) { - file_delete($file); + file_remove_usage($file); } } @@ -489,8 +477,7 @@ function upload_save($node) { if (!empty($file->remove)) { // Remove the reference from this revision. db_delete('upload')->condition('fid', $file->fid)->condition('vid', $node->vid)->execute(); - // Try a soft delete, if the file isn't used elsewhere it'll be deleted. - file_delete($file); + $blah = file_remove_usage($file, 'upload', 'node_revision', $node->vid); // Remove it from the session in the case of new uploads, // that you want to disassociate before node submission. unset($node->files[$fid]); @@ -498,6 +485,8 @@ function upload_save($node) { continue; } + $add_usage = FALSE; + // Create a new revision, or associate a new file needed. if (!empty($node->old_vid) || $file->new) { db_insert('upload') @@ -510,6 +499,7 @@ function upload_save($node) { 'weight' => $file->weight, )) ->execute(); + $add_usage = TRUE; } // Update existing revision. else { @@ -525,6 +515,10 @@ function upload_save($node) { } $file->status |= FILE_STATUS_PERMANENT; $file = file_save($file); + + if ($add_usage) { + file_add_usage($file, 'upload', 'node_revision', $node->vid); + } } } diff --git a/modules/user/user.module b/modules/user/user.module index e46132f..bd04664 100644 --- a/modules/user/user.module +++ b/modules/user/user.module @@ -435,9 +435,14 @@ function user_save($account, $edit = array(), $category = 'account') { if ($picture = file_move($picture, $destination, FILE_EXISTS_REPLACE)) { $picture->status |= FILE_STATUS_PERMANENT; $edit['picture'] = file_save($picture); + file_add_usage($picture, 'user', 'user', $account->uid); } } } + // If the picture existed before and was unset, remove our reference to it. + elseif (!empty($account->picture->fid)) { + file_remove_usage($account->picture, 'user', 'user', $account->uid); + } $edit['picture'] = empty($edit['picture']->fid) ? 0 : $edit['picture']->fid; $edit['data'] = $data; @@ -455,13 +460,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') @@ -826,18 +824,6 @@ function user_file_download($filepath) { } /** - * Implementation of hook_file_references(). - */ -function user_file_references($file) { - // Determine if the file is used by this module. - $count = db_query('SELECT COUNT(*) FROM {users} WHERE picture = :fid', array(':fid' => $file->fid))->fetchField(); - if ($count) { - // Return the name of the module and how many references it has to the file. - return array('user' => $count); - } -} - -/** * Implementation of hook_file_delete(). */ function user_file_delete($file) {