diff --git includes/file.inc includes/file.inc index 6a28a0c..908693c 100644 --- includes/file.inc +++ 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 @@ -724,34 +821,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 +841,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 modules/node/node.api.php modules/node/node.api.php index 3be7752..4c647cb 100644 --- modules/node/node.api.php +++ modules/node/node.api.php @@ -291,7 +291,7 @@ function hook_node_delete_revision($node) { return; } foreach ($node->files as $file) { - file_delete($file); + file_remove_usage($file); } } diff --git modules/simpletest/tests/file.test modules/simpletest/tests/file.test index 18bc651..9f0ae75 100644 --- modules/simpletest/tests/file.test +++ modules/simpletest/tests/file.test @@ -223,13 +223,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.')); @@ -1245,18 +1245,42 @@ 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->assertFalse(file_exists($file->filepath), t("Test file has actually been deleted.")); + $this->assertTrue(is_file($file->filepath), t('File exists.')); + $this->assertIdentical(file_delete($file), TRUE, 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.')); + } + + /** + * 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->assertFileHooksCalled(array()); + $this->assertEqual($usage['testing']['test'], array('id' => 1, 'count' => 1), t('Test file is still in use.')); + $this->assertTrue(file_exists($file->filepath), 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(); - // TODO: implement hook_file_references() in file_test.module and report a - // file in use and test the $force parameter. + file_remove_usage($file, 'testing', 'test', 1); + $usage = file_get_usage($file); + $this->assertFileHooksCalled(array('delete')); + $this->assertTrue(empty($usage), t('File usage data was removed.')); + $this->assertFalse(file_exists($file->filepath), t('File has been deleted after its last usage was removed.')); + $this->assertFalse(file_load($file->fid), t('File was removed from the database.')); } } @@ -1358,7 +1382,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 +1744,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 modules/simpletest/tests/file_test.module modules/simpletest/tests/file_test.module index 06aa4ad..ba0a09c 100644 --- modules/simpletest/tests/file_test.module +++ modules/simpletest/tests/file_test.module @@ -77,7 +77,6 @@ function file_test_reset() { 'load' => array(), 'validate' => array(), 'download' => array(), - 'references' => array(), 'insert' => array(), 'update' => array(), 'copy' => array(), @@ -90,7 +89,6 @@ function file_test_reset() { $return = array( 'validate' => array(), 'download' => NULL, - 'references' => NULL, ); variable_set('file_test_return', $return); } @@ -101,7 +99,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() @@ -115,9 +113,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()); @@ -128,7 +126,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() @@ -143,7 +141,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(). @@ -157,7 +155,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(). @@ -197,14 +195,6 @@ function file_test_file_download($file) { } /** - * Implement hook_file_references(). - */ -function file_test_file_references($file) { - _file_test_log_call('references', array($file)); - return _file_test_get_return('references'); -} - -/** * Implement hook_file_insert(). */ function file_test_file_insert($file) { diff --git modules/system/system.api.php modules/system/system.api.php index 3f4f2c5..5dfc781 100644 --- modules/system/system.api.php +++ modules/system/system.api.php @@ -1179,6 +1179,13 @@ 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 filename of the existing target file + * is adopted for the new file object. + * + * Implement this hook if your module keeps track of files in a module-specific + * table, so that file references remain valid. + * * @param $file * The updated file object after the move. * @param $source @@ -1189,31 +1196,11 @@ function hook_file_copy($file, $source) { * @see file_move() */ 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. - $file_used = (bool) db_query_range('SELECT 1 FROM {upload} WHERE fid = :fid', array(':fid' => $file->fid), 0, 1)->fetchField(); - if ($file_used) { - // Return the name of the module and how many references it has to the file. - return array('upload' => $count); + if ($file->fid != $source->fid) { + db_update('upload') + ->fields(array('fid' => $file->fid)) + ->condition('fid', $source->fid, '=') + ->execute(); } } diff --git modules/system/system.install modules/system/system.install index 8230b78..190296e 100644 --- modules/system/system.install +++ modules/system/system.install @@ -686,6 +686,50 @@ 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 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( @@ -2131,6 +2175,59 @@ function system_update_7028() { /** + * Create the file_usage table. + */ +function system_update_7029() { + $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 modules/system/system.module modules/system/system.module index 0f31158..42dd78c 100644 --- modules/system/system.module +++ modules/system/system.module @@ -1755,8 +1755,12 @@ 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->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 modules/upload/upload.install modules/upload/upload.install index 0fa9d05..e5a5c89 100644 --- modules/upload/upload.install +++ modules/upload/upload.install @@ -97,4 +97,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 modules/upload/upload.module modules/upload/upload.module index 56ab028..904cbe0 100644 --- modules/upload/upload.module +++ modules/upload/upload.module @@ -277,14 +277,14 @@ function upload_file_load($files) { } /** - * Implement hook_file_references(). + * Implement hook_file_move(). */ -function upload_file_references($file) { - // If upload.module is still using a file, do not let other modules delete it. - $file_used = (bool) db_query_range('SELECT 1 FROM {upload} WHERE fid = :fid', array(':fid' => $file->fid), 0, 1)->fetchField(); - if ($file_used) { - // Return the name of the module and how many references it has to the file. - return array('upload' => $count); +function upload_file_move($file, $source) { + if ($file->fid != $source->fid) { + db_update('upload') + ->fields(array('fid' => $file->fid)) + ->condition('fid', $source->fid, '=') + ->execute(); } } @@ -407,7 +407,7 @@ function upload_node_delete($node) { return; } foreach ($node->files as $file) { - file_delete($file); + file_remove_usage($file); } } @@ -420,7 +420,7 @@ function upload_node_delete_revision($node) { return; } foreach ($node->files as $file) { - file_delete($file); + file_remove_usage($file); } } @@ -488,8 +488,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]); @@ -497,6 +496,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') @@ -509,6 +510,7 @@ function upload_save($node) { 'weight' => $file->weight, )) ->execute(); + $add_usage = TRUE; } // Update existing revision. else { @@ -524,6 +526,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 modules/user/user.module modules/user/user.module index bf360d6..09ca5b5 100644 --- modules/user/user.module +++ 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') @@ -828,14 +826,14 @@ function user_file_download($filepath) { } /** - * Implement hook_file_references(). + * Implement 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', array(':fid' => $file->fid), 0, 1)->fetchField(); - if ($file_used) { - // Return the name of the module and how many references it has to the file. - return array('user' => $count); +function user_file_move($file, $source) { + if ($file->fid != $source->fid) { + db_update('users') + ->fields(array('picture' => $file->fid)) + ->condition('picture', $source->fid, '=') + ->execute(); } }