Index: modules/simpletest/tests/file.test =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/tests/file.test,v retrieving revision 1.7 diff -u -p -r1.7 file.test --- modules/simpletest/tests/file.test 12 Oct 2008 06:37:40 -0000 1.7 +++ modules/simpletest/tests/file.test 5 Nov 2008 23:43:08 -0000 @@ -1018,14 +1018,14 @@ class FileValidateTest extends FileHookT // Use the file_test.module's test validator to ensure that passing tests // return correctly. file_test_reset(); - $GLOBALS['file_test_hook_return']['validate'] = array(); + file_test_set_return('validate', array()); $passing = array('file_test_validator' => array(array())); $this->assertEqual(file_validate($file, $passing), array(), t('Validating passes.')); $this->assertFileHookCalled('validate', 1); // Now test for failures in validators passed in and by hook_validate. file_test_reset(); - $GLOBALS['file_test_hook_return']['validate'] = array('Epic fail'); + file_test_set_return('validate', array('Epic fail')); $failing = array('file_test_validator' => array(array('Failed', 'Badly'))); $this->assertEqual(file_validate($file, $failing), array('Failed', 'Badly', 'Epic fail'), t('Validating returns errors.')); $this->assertFileHookCalled('validate', 1); Index: modules/simpletest/tests/file_test.module =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/tests/file_test.module,v retrieving revision 1.2 diff -u -p -r1.2 file_test.module --- modules/simpletest/tests/file_test.module 9 Oct 2008 00:02:29 -0000 1.2 +++ modules/simpletest/tests/file_test.module 5 Nov 2008 23:43:08 -0000 @@ -54,10 +54,12 @@ function _file_test_form_submit(&$form, /** * Reset/initialize the history of calls to the file_* hooks. + * + * @see the getter/setter functions file_test_get_calls() and file_test_reset(). */ function file_test_reset() { // Keep track of calls to these hooks - $GLOBALS['file_test_results'] = array( + $results = array( 'load' => array(), 'validate' => array(), 'download' => array(), @@ -69,30 +71,81 @@ function file_test_reset() { 'move' => array(), 'delete' => array(), ); + variable_set('file_test_results', $results); - // These hooks will return these values. - $GLOBALS['file_test_hook_return'] = array( - 'validate' => NULL, + // These hooks will return these values, @see file_test_set_return(). + $return = array( + 'validate' => array(), 'download' => NULL, 'references' => NULL, ); + variable_set('file_test_return', $return); } /** - * Get the values passed to a the hook calls for a given operation. + * Get the arguments passed to invocation of a given hook since + * file_test_reset() was last called. * - * @param $op One of the hook_file_* operations. - * @returns Array of the parameters passed to each call. + * @param $op + * One of the hook_file_* operations: "load", "validate", etc. + * @returns + * Array of the parameters passed to each call. + * @see _file_test_log_call() and file_test_reset() */ function file_test_get_calls($op) { - return $GLOBALS['file_test_results'][$op]; + $results = variable_get('file_test_results', array()); + return $results[$op]; +} + +/** + * Store the values passed to a hook invocation. + * + * @param $op + * One of the hook_file_* operations. + * @param $args + * Values passed to hook. + * @see file_test_get_calls() and file_test_reset() + */ +function _file_test_log_call($op, $args) { + $results = variable_get('file_test_results', array()); + $results[$op][] = $args; + variable_set('file_test_results', $results); +} + +/** + * Load the appropriate return value. + * + * @param $op + * One of the hook_file_[validate,download,references] operations. + * @return + * Value set by file_test_set_return(). +* @see file_test_set_return() and file_test_reset(). + */ +function _file_test_get_return($op) { + $return = variable_get('file_test_return', array($op => NULL)); + return $return[$op]; +} + +/** + * Assign a return value for a given operation. + * + * @param $op + * One of the hook_file_[validate,download,references] operations. + * @param $value + * Value for the hook to return. + * @see _file_test_get_return() and file_test_reset(). + */ +function file_test_set_return($op, $value) { + $return = variable_get('file_test_return', array()); + $return[$op] = $value; + variable_set('file_test_return', $return); } /** * Implementation of hook_file_load(). */ -function file_test_file_load(&$file) { - $GLOBALS['file_test_results']['load'][] = func_get_args(); +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; @@ -101,66 +154,65 @@ function file_test_file_load(&$file) { /** * Implementation of hook_file_validate(). */ -function file_test_file_validate(&$file) { - $GLOBALS['file_test_results']['validate'][] = func_get_args(); - return $GLOBALS['file_test_hook_return']['validate']; +function file_test_file_validate($file) { + _file_test_log_call('validate', array($file)); + return _file_test_get_return('validate'); } /** * Implementation of hook_file_status(). */ -function file_test_file_status(&$file) { - $GLOBALS['file_test_results']['status'][] = func_get_args(); +function file_test_file_status($file) { + _file_test_log_call('status', array($file)); } /** * Implementation of hook_file_download(). */ -function file_test_file_download(&$file) { - $GLOBALS['file_test_results']['download'][] = func_get_args(); - return $GLOBALS['file_test_hook_return']['download']; +function file_test_file_download($file) { + _file_test_log_call('download', array($file)); + return _file_test_get_return('download'); } /** * Implementation of hook_file_references(). */ -function file_test_file_references(&$file) { - $GLOBALS['file_test_results']['references'][] = func_get_args(); - return $GLOBALS['file_test_hook_return']['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) { - $GLOBALS['file_test_results']['insert'][] = func_get_args(); +function file_test_file_insert($file) { + _file_test_log_call('insert', array($file)); } /** * Implementation of hook_file_update(). */ -function file_test_file_update(&$file) { - $GLOBALS['file_test_results']['update'][] = func_get_args(); +function file_test_file_update($file) { + _file_test_log_call('update', array($file)); } /** * Implemenation of hook_file_copy(). */ -function file_test_file_copy(&$file, &$source) { - $GLOBALS['file_test_results']['copy'][] = func_get_args(); +function file_test_file_copy($file, $source) { + _file_test_log_call('copy', array($file, $source)); } /** * Implemenation of hook_file_move(). */ -function file_test_file_move(&$file, &$source) { - $GLOBALS['file_test_results']['move'][] = func_get_args(); +function file_test_file_move($file, $source) { + _file_test_log_call('move', array($file, $source)); } /** * Implementation of hook_file_delete(). */ -function file_test_file_delete(&$file) { - $GLOBALS['file_test_results']['delete'][] = func_get_args(); +function file_test_file_delete($file) { + _file_test_log_call('delete', array($file)); } - Index: modules/system/system.module =================================================================== RCS file: /cvs/drupal/drupal/modules/system/system.module,v retrieving revision 1.635 diff -u -p -r1.635 system.module --- modules/system/system.module 31 Oct 2008 02:18:22 -0000 1.635 +++ modules/system/system.module 5 Nov 2008 23:43:08 -0000 @@ -1426,18 +1426,15 @@ function system_cron() { db_query('DELETE FROM {batch} WHERE timestamp < %d', REQUEST_TIME - 864000); // Remove temporary files that are older than DRUPAL_MAXIMUM_TEMP_FILE_AGE. - $result = db_query('SELECT * FROM {files} WHERE status = %d and timestamp < %d', FILE_STATUS_TEMPORARY, REQUEST_TIME - DRUPAL_MAXIMUM_TEMP_FILE_AGE); - while ($file = db_fetch_object($result)) { - if (file_exists($file->filepath)) { - // If files that exist cannot be deleted, continue so the database remains - // consistent. + $result = db_query('SELECT fid FROM {files} WHERE status & :permanent != :permanent AND timestamp < :timestamp', array(':permanent' => 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); - continue; } } - db_query('DELETE FROM {files} WHERE fid = %d', $file->fid); } + $core = array('cache', 'cache_block', 'cache_filter', 'cache_page', 'cache_form', 'cache_menu'); $cache_tables = array_merge(module_invoke_all('flush_caches'), $core); foreach ($cache_tables as $table) { Index: modules/system/system.test =================================================================== RCS file: /cvs/drupal/drupal/modules/system/system.test,v retrieving revision 1.23 diff -u -p -r1.23 system.test --- modules/system/system.test 1 Nov 2008 21:21:35 -0000 1.23 +++ modules/system/system.test 5 Nov 2008 23:43:08 -0000 @@ -252,6 +252,41 @@ class CronRunTestCase extends DrupalWebT // Execute cron directly. $this->assertTrue(drupal_cron_run(), t('Cron ran successfully.')); } + + /** + * Ensure that temporary files are removed. + */ + function testTempFileCleanup() { + // Create files for all the possible combinations of age and status. We're + // using UPDATE statments rather than file_save() because it would set the + // timestamp. + + // Temporary file that's older than DRUPAL_MAXIMUM_TEMP_FILE_AGE. + $temp_old = file_save_data(''); + db_query('UPDATE {files} SET status = :status, timestamp = :timestamp WHERE fid = :fid', array(':status' => FILE_STATUS_TEMPORARY, ':timestamp' => 1, ':fid' => $temp_old->fid)); + $this->assertTrue(file_exists($temp_old->filepath), t('Old temp file was created correctly.')); + + // Temporary file that's less than DRUPAL_MAXIMUM_TEMP_FILE_AGE. + $temp_new = file_save_data(''); + db_query('UPDATE {files} SET status = :status WHERE fid = :fid', array(':status' => FILE_STATUS_TEMPORARY, ':fid' => $temp_new->fid)); + $this->assertTrue(file_exists($temp_new->filepath), t('New temp file was created correctly.')); + + // Permanent file that's older than DRUPAL_MAXIMUM_TEMP_FILE_AGE. + $perm_old = file_save_data(''); + db_query('UPDATE {files} SET timestamp = :timestamp WHERE fid = :fid', array(':timestamp' => 1, ':fid' => $perm_old->fid)); + $this->assertTrue(file_exists($perm_old->filepath), t('Old permanent file was created correctly.')); + + // Permanent file that's newer than DRUPAL_MAXIMUM_TEMP_FILE_AGE. + $perm_new = file_save_data(''); + $this->assertTrue(file_exists($perm_new->filepath), t('New permanent file was created correctly.')); + + // Run cron and then ensure that only the old, temp file was deleted. + $this->assertTrue(drupal_cron_run(), t('Cron ran successfully.')); + $this->assertFalse(file_exists($temp_old->filepath), t('Old temp file was correctly removed.')); + $this->assertTrue(file_exists($temp_new->filepath), t('New temp file was correctly ignored.')); + $this->assertTrue(file_exists($perm_old->filepath), t('Old permanent file was correctly ignored.')); + $this->assertTrue(file_exists($perm_new->filepath), t('New permanent file was correctly ignored.')); + } } class AdminOverviewTestCase extends DrupalWebTestCase {