From 1aa01caf9043021bcdcc8a253b4939daf56f7d2a Mon Sep 17 00:00:00 2001 From: skodak Date: Wed, 3 Jun 2009 08:10:21 +0000 Subject: [PATCH] MDL-16231 reimplemented deleting of files + cron cleanup --- admin/cron.php | 6 +- lib/db/install.xml | 18 +--- lib/db/upgrade.php | 34 +++----- lib/file/file_storage.php | 145 ++++++++++++++++++++++++++------- lib/file/stored_file.php | 24 ++++-- lib/moodlelib.php | 8 +- lib/setup.php | 5 ++ lib/simpletest/testfilelib.php | 2 +- version.php | 2 +- 9 files changed, 168 insertions(+), 76 deletions(-) diff --git a/admin/cron.php b/admin/cron.php index fcc2c003f1..2af42acc75 100644 --- a/admin/cron.php +++ b/admin/cron.php @@ -544,7 +544,11 @@ } } } - + + // cleanup file trash + $fs = get_file_storage(); + $fs->cron(); + // run any customized cronjobs, if any // looking for functions in lib/local/cron.php if (file_exists($CFG->dirroot.'/local/cron.php')) { diff --git a/lib/db/install.xml b/lib/db/install.xml index 0c12ced41c..74032e8005 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -1,5 +1,5 @@ - @@ -2021,7 +2021,7 @@ - +
@@ -2049,19 +2049,7 @@
- - - - - - - - - - - -
- +
diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 775895cb4c..3987a1b519 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -518,27 +518,6 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint($result, 2008073111); } - if ($result && $oldversion < 2008073112) { - /// Define table files_cleanup to be created - $table = new xmldb_table('files_cleanup'); - - /// Adding fields to table files_cleanup - $table->add_field('id', XMLDB_TYPE_INTEGER, '10', XMLDB_UNSIGNED, XMLDB_NOTNULL, XMLDB_SEQUENCE, null); - $table->add_field('contenthash', XMLDB_TYPE_CHAR, '40', null, XMLDB_NOTNULL, null, null); - - /// Adding keys to table files_cleanup - $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id')); - - /// Adding indexes to table files_cleanup - $table->add_index('contenthash', XMLDB_INDEX_UNIQUE, array('contenthash')); - - /// Conditionally launch create table for files_cleanup - $dbman->create_table($table); - - /// Main savepoint reached - upgrade_main_savepoint($result, 2008073112); - } - if ($result && $oldversion < 2008073113) { /// move all course, backup and other files to new filepool based storage upgrade_migrate_files_courses(); @@ -2155,6 +2134,19 @@ WHERE gradeitemid IS NOT NULL AND grademax IS NOT NULL"); upgrade_main_savepoint($result, 2009051700); } + if ($result && $oldversion < 2009060200) { + /// Define table files_cleanup to be dropped - not needed + $table = new xmldb_table('files_cleanup'); + + /// Conditionally launch drop table for files_cleanup + if ($dbman->table_exists($table)) { + $dbman->drop_table($table); + } + + /// Main savepoint reached + upgrade_main_savepoint($result, 2009060200); + } + return $result; } diff --git a/lib/file/file_storage.php b/lib/file/file_storage.php index aa03ebd076..4ca660b913 100644 --- a/lib/file/file_storage.php +++ b/lib/file/file_storage.php @@ -35,29 +35,46 @@ require_once("$CFG->libdir/file/stored_file.php"); * to use file_browser class instead. */ class file_storage { + /** Directory with file contents */ private $filedir; - + /** Contents of deleted files not needed any more */ + private $trashdir; + /** Permissions for new directories */ + private $dirpermissions; + /** Permissions for new files */ + private $filepermissions; /** * Contructor * @param string $filedir full path to pool directory */ - public function __construct($filedir) { - $this->filedir = $filedir; + public function __construct($filedir, $trashdir, $dirpermissions, $filepermissions) { + $this->filedir = $filedir; + $this->trashdir = $trashdir; + $this->dirpermissions = $dirpermissions; + $this->filepermissions = $filepermissions; // make sure the file pool directory exists if (!is_dir($this->filedir)) { - if (!check_dir_exists($this->filedir, true, true)) { + if (!mkdir($this->filedir, $this->dirpermissions, true)) { throw new file_exception('storedfilecannotcreatefiledirs'); // permission trouble } // place warning file in file pool root - file_put_contents($this->filedir.'/warning.txt', - 'This directory contains the content of uploaded files and is controlled by Moodle code. Do not manually move, change or rename any of the files and subdirectories here.'); + if (!file_exists($this->filedir.'/warning.txt')) { + file_put_contents($this->filedir.'/warning.txt', + 'This directory contains the content of uploaded files and is controlled by Moodle code. Do not manually move, change or rename any of the files and subdirectories here.'); + } + } + // make sure the file pool directory exists + if (!is_dir($this->trashdir)) { + if (!mkdir($this->trashdir, $this->dirpermissions, true)) { + throw new file_exception('storedfilecannotcreatefiledirs'); // permission trouble + } } } /** * Returns location of filedir (file pool) - * Do not use, this method is intended for stored_file instances. + * Do not use, this method is intended for stored_file instances only. * @return string pathname */ public function get_filedir() { @@ -805,8 +822,10 @@ class file_storage { $newfile = false; } else { - if (!check_dir_exists($hashpath, true, true)) { - throw new file_exception('storedfilecannotcreatefiledirs'); // permission trouble + if (!is_dir($hashpath)) { + if (!mkdir($hashpath, $this->dirpermissions, true)) { + throw new file_exception('storedfilecannotcreatefiledirs'); // permission trouble + } } $newfile = true; @@ -818,6 +837,7 @@ class file_storage { @unlink($hashfile); throw new file_pool_content_exception($contenthash); } + chmod($hashfile, $this->filepermissions); // fix permissions if needed } @@ -844,8 +864,10 @@ class file_storage { $newfile = false; } else { - if (!check_dir_exists($hashpath, true, true)) { - throw new file_exception('storedfilecannotcreatefiledirs'); // permission trouble + if (!is_dir($hashpath)) { + if (!mkdir($hashpath, $this->dirpermissions, true)) { + throw new file_exception('storedfilecannotcreatefiledirs'); // permission trouble + } } $newfile = true; @@ -855,6 +877,7 @@ class file_storage { @unlink($hashfile); throw new file_pool_content_exception($contenthash); } + chmod($hashfile, $this->filepermissions); // fix permissions if needed } return array($contenthash, $filesize, $newfile); @@ -875,38 +898,102 @@ class file_storage { return "$this->filedir/$l1/$l2/$l3"; } + /** + * Return path to file with given hash + * + * NOTE: must not be public, files in pool must not be modified + * + * @param string $contenthash + * @return string expected file location + */ + protected function trash_path_from_hash($contenthash) { + $l1 = $contenthash[0].$contenthash[1]; + $l2 = $contenthash[2].$contenthash[3]; + $l3 = $contenthash[4].$contenthash[5]; + return "$this->trashdir/$l1/$l2/$l3"; + } + + /** + * Tries to recover missing content of file from trash + * @param object $file_record + * @return bool success + */ + public function try_content_recovery($file) { + $contenthash = $file->get_contenthash(); + $trashfile = $this->trash_path_from_hash($contenthash).'/'.$contenthash; + if (!is_readable($trashfile)) { + if (!is_readable($this->trashdir.'/'.$contenthash)) { + return false; + } + // nice, at least alternative trash file in trash root exists + $trashfile = $this->trashdir.'/'.$contenthash; + } + if (filesize($trashfile) != $file->get_filesize() or sha1_file($trashfile) != $contenthash) { + //weird, better fail early + return false; + } + $contentdir = $this->path_from_hash($contenthash); + $contentfile = $contentdir.'/'.$contenthash; + if (file_exists($contentfile)) { + //strange, no need to recover anything + return true; + } + if (!is_dir($contentdir)) { + if (!mkdir($contentdir, $this->dirpermissions, true)) { + return false; + } + } + return rename($trashfile, $contentfile); + } + /** * Marks pool file as candidate for deleting + * DO NOT call directly - reserved for core! * @param string $contenthash + * @return void */ - public function mark_delete_candidate($contenthash) { + public function deleted_file_cleanup($contenthash) { global $DB; - if ($DB->record_exists('files_cleanup', array('contenthash'=>$contenthash))) { + //Note: this section is critical - in theory file could be reused at the same + // time, if this happens we can still recover the file from trash + if ($DB->record_exists('files', array('contenthash'=>$contenthash))) { + // file content is still used + return; + } + //move content file to trash + $contentfile = $this->path_from_hash($contenthash).'/'.$contenthash; + if (!file_exists($contentfile)) { + //weird, but no problem return; } - $rec = new object(); - $rec->contenthash = $contenthash; - $DB->insert_record('files_cleanup', $rec); + $trashpath = $this->trash_path_from_hash($contenthash); + $trashfile = $trashpath.'/'.$contenthash; + if (file_exists($trashfile)) { + // we already have this content in trash, no need to move it there + unlink($contentfile); + return; + } + if (!is_dir($trashpath)) { + mkdir($trashpath, $this->dirpermissions, true); + } + rename($contentfile, $trashfile); + chmod($trashfile, $this->filepermissions); // fix permissions if needed } /** * Cron cleanup job. */ public function cron() { - global $DB; - - //TODO: there is a small chance that reused files might be deleted - // if this function takes too long we should add some table locking here - - $sql = "SELECT 1 AS id, fc.contenthash - FROM {files_cleanup} fc - LEFT JOIN {files} f ON f.contenthash = fc.contenthash - WHERE f.id IS NULL"; - while ($hash = $DB->get_record_sql($sql, null, true)) { - $file = $this->path_from_hash($hash->contenthash).'/'.$hash->contenthash; - @unlink($file); - $DB->delete_records('files_cleanup', array('contenthash'=>$hash->contenthash)); + global $CFG; + // remove trash pool files once a day + // if you want to disable purging of trash put $CFG->fileslastcleanup=time(); into config.php + if (empty($CFG->fileslastcleanup) or $CFG->fileslastcleanup < time() - 60*60*24) { + require_once($CFG->libdir.'/filelib.php'); + mtrace('Deleting trash files... ', ''); + fulldelete($this->trashdir); + set_config('fileslastcleanup', time()); + mtrace('done.'); } } } diff --git a/lib/file/stored_file.php b/lib/file/stored_file.php index 542f971c66..6f2437e08d 100644 --- a/lib/file/stored_file.php +++ b/lib/file/stored_file.php @@ -54,12 +54,14 @@ class stored_file { /** * Delete file - * @return success + * @return bool success */ public function delete() { global $DB; - $this->fs->mark_delete_candidate($this->file_record->contenthash); - return $DB->delete_records('files', array('id'=>$this->file_record->id)); + $DB->delete_records('files', array('id'=>$this->file_record->id)); + // moves pool file to trash if content not needed any more + $this->fs->deleted_file_cleanup($this->file_record->contenthash); + return true; } /** @@ -93,7 +95,9 @@ class stored_file { public function get_content_file_handle() { $path = $this->get_content_file_location(); if (!is_readable($path)) { - throw new file_exception('storedfilecannotread'); + if (!$this->fs->try_content_recovery($this) or !is_readable($path)) { + throw new file_exception('storedfilecannotread'); + } } return fopen($path, 'rb'); //binary reading only!! } @@ -105,7 +109,9 @@ class stored_file { public function readfile() { $path = $this->get_content_file_location(); if (!is_readable($path)) { - throw new file_exception('storedfilecannotread'); + if (!$this->fs->try_content_recovery($this) or !is_readable($path)) { + throw new file_exception('storedfilecannotread'); + } } readfile($path); } @@ -117,7 +123,9 @@ class stored_file { public function get_content() { $path = $this->get_content_file_location(); if (!is_readable($path)) { - throw new file_exception('storedfilecannotread'); + if (!$this->fs->try_content_recovery($this) or !is_readable($path)) { + throw new file_exception('storedfilecannotread'); + } } return file_get_contents($this->get_content_file_location()); } @@ -130,7 +138,9 @@ class stored_file { public function copy_content_to($pathname) { $path = $this->get_content_file_location(); if (!is_readable($path)) { - throw new file_exception('storedfilecannotread'); + if (!$this->fs->try_content_recovery($this) or !is_readable($path)) { + throw new file_exception('storedfilecannotread'); + } } return copy($path, $pathname); } diff --git a/lib/moodlelib.php b/lib/moodlelib.php index b1e8c5f1b0..c48e438f44 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -4963,7 +4963,13 @@ function get_file_storage() { $filedir = $CFG->dataroot.'/filedir'; } - $fs = new file_storage($filedir); + if (isset($CFG->trashdir)) { + $trashdirdir = $CFG->trashdir; + } else { + $trashdirdir = $CFG->dataroot.'/trashdir'; + } + + $fs = new file_storage($filedir, $trashdirdir, $CFG->directorypermissions, $CFG->filepermissions); return $fs; } diff --git a/lib/setup.php b/lib/setup.php index e39e6744ab..c0c8b6f285 100644 --- a/lib/setup.php +++ b/lib/setup.php @@ -421,6 +421,11 @@ global $SCRIPT; if (empty($CFG->directorypermissions)) { $CFG->directorypermissions = 0777; // Must be octal (that's why it's here) } + if (empty($CFG->filepermissions)) { + $CFG->filepermissions = ($CFG->directorypermissions & 0666); // strip execute flags + } +/// better also set default umask because recursive mkdir() does not apply permissions recursively otherwise + umask(0000); /// Calculate and set $CFG->ostype to be used everywhere. Possible values are: /// - WINDOWS: for any Windows flavour. diff --git a/lib/simpletest/testfilelib.php b/lib/simpletest/testfilelib.php index b0416ffd3e..e0d1419a9c 100644 --- a/lib/simpletest/testfilelib.php +++ b/lib/simpletest/testfilelib.php @@ -51,7 +51,7 @@ class filelib_test extends UnitTestCaseUsingDatabase { parent::setUp(); $tables = array('block_instance', 'cache_flags', 'capabilities', 'context', 'context_temp', 'course', 'course_modules', 'course_categories', 'course_sections','files', - 'files_cleanup', 'grade_items', 'grade_categories', 'groups', 'groups_members', + 'grade_items', 'grade_categories', 'groups', 'groups_members', 'modules', 'role', 'role_names', 'role_context_levels', 'role_assignments', 'role_capabilities', 'user'); $this->create_test_tables($tables, 'lib'); diff --git a/version.php b/version.php index 07a30b1a31..f0b9eda66d 100644 --- a/version.php +++ b/version.php @@ -6,7 +6,7 @@ // This is compared against the values stored in the database to determine // whether upgrades should be performed (see lib/db/*.php) - $version = 2009051700; // YYYYMMDD = date of the last version bump + $version = 2009060200; // YYYYMMDD = date of the last version bump // XX = daily increments $release = '2.0 dev (Build: 20090603)'; // Human-friendly version name -- 2.39.5