mirror of
https://github.com/moodle/moodle.git
synced 2025-08-07 09:56:38 +02:00
MDL-45654 cron: fix non-empty directories deletion
Remove errors when attempting to delete non-empty directories when child directories do not get removed since the mtime was updated when files were removed from the child directory.
This commit is contained in:
Rod Norfor
parent
a929fd50f9
commit
75b70ab6cd
2 changed files with 83 additions and 56 deletions
|
|
@ -60,8 +60,11 @@ class file_temp_cleanup_task extends scheduled_task {
|
||||||
// Check if file or directory is older than the given time.
|
// Check if file or directory is older than the given time.
|
||||||
if ($iter->getMTime() < $time) {
|
if ($iter->getMTime() < $time) {
|
||||||
if ($iter->isDir() && !$iter->isDot()) {
|
if ($iter->isDir() && !$iter->isDot()) {
|
||||||
if (@rmdir($node) === false) {
|
// Don't attempt to delete the directory if it isn't empty.
|
||||||
mtrace("Failed removing directory '$node'.");
|
if (!glob($node. DIRECTORY_SEPARATOR . '*')) {
|
||||||
|
if (@rmdir($node) === false) {
|
||||||
|
mtrace("Failed removing directory '$node'.");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if ($iter->isFile()) {
|
if ($iter->isFile()) {
|
||||||
|
|
|
||||||
|
|
@ -41,73 +41,74 @@ class cronlib_testcase extends basic_testcase {
|
||||||
$tmpdir = realpath($CFG->tempdir);
|
$tmpdir = realpath($CFG->tempdir);
|
||||||
$time = time();
|
$time = time();
|
||||||
|
|
||||||
$weekstime = $time - strtotime('1 week');
|
$lastweekstime = strtotime('-1 week');
|
||||||
$beforeweekstime = $time - strtotime('1 week') - 1;
|
$beforelastweekstime = $lastweekstime - 60;
|
||||||
$afterweekstime = $time + strtotime('1 week') + 1;
|
$afterlastweekstime = $lastweekstime + 60;
|
||||||
|
|
||||||
$node1 = new stdClass();
|
$nodes = array();
|
||||||
$node1->path = '/dir1/dir1_1/dir1_2/dir1_3/';
|
// Really old directory to remove.
|
||||||
$node1->time = 1;
|
$node[] = $this->generate_test_path('/dir1/dir1_1/dir1_1_1/dir1_1_1_1/', true, 1, false);
|
||||||
$node1->isdir = true;
|
|
||||||
|
|
||||||
$node2 = new stdClass();
|
// New Directory to keep.
|
||||||
$node2->path = '/dir1/dir1_4/';
|
$node[] = $this->generate_test_path('/dir1/dir1_2/', true, $time, true);
|
||||||
$node2->time = $time;
|
|
||||||
$node2->isdir = true;
|
|
||||||
|
|
||||||
$node3 = new stdClass();
|
// Directory exactly 1 week old, keep.
|
||||||
$node3->path = '/dir2/';
|
$node[] = $this->generate_test_path('/dir2/', true, $lastweekstime, true);
|
||||||
$node3->isdir = true;
|
|
||||||
$node3->time = $time - $weekstime;
|
|
||||||
|
|
||||||
$node4 = new stdClass();
|
// Directory older than 1 week old, remove.
|
||||||
$node4->path = '/dir3/';
|
$node[] = $this->generate_test_path('/dir3/', true, $beforelastweekstime, false);
|
||||||
$node4->isdir = true;
|
|
||||||
$node4->time = $time - $afterweekstime;
|
|
||||||
|
|
||||||
$node5 = new stdClass();
|
// File older than 1 week old, remove.
|
||||||
$node5->path = '/dir1/dir1_1/dir1_2/file1';
|
$node[] = $this->generate_test_path('/dir1/dir1_1/dir1_1_1/file1_1_1_1', false, $beforelastweekstime, false);
|
||||||
$node5->isdir = false;
|
|
||||||
$node5->time = $beforeweekstime;
|
|
||||||
|
|
||||||
$node6 = new stdClass();
|
// New File to keep.
|
||||||
$node6->path = '/dir1/dir1_1/dir1_2/file2';
|
$node[] = $this->generate_test_path('/dir1/dir1_1/dir1_1_1/file1_1_1_2', false, $time, true);
|
||||||
$node6->isdir = false;
|
|
||||||
$node6->time = $time;
|
|
||||||
|
|
||||||
$node7 = new stdClass();
|
// File older than 1 week old, remove.
|
||||||
$node7->path = '/dir1/dir1_4/file1';
|
$node[] = $this->generate_test_path('/dir1/dir1_2/file1_1_2_1', false, $beforelastweekstime, false);
|
||||||
$node7->isdir = false;
|
|
||||||
$node7->time = $time - $afterweekstime;
|
|
||||||
|
|
||||||
$node8 = new stdClass();
|
// New file to keep.
|
||||||
$node8->path = '/dir1/dir1_4/file2';
|
$node[] = $this->generate_test_path('/dir1/dir1_2/file1_1_2_2', false, $time, true);
|
||||||
$node8->isdir = false;
|
|
||||||
$node8->time = $time;
|
|
||||||
|
|
||||||
$node9 = new stdClass();
|
// New file to keep.
|
||||||
$node9->path = '/file1';
|
$node[] = $this->generate_test_path('/file1', false, $time, true);
|
||||||
$node9->isdir = false;
|
|
||||||
$node9->time = $time;
|
|
||||||
|
|
||||||
$node10 = new stdClass();
|
// File older than 1 week, keep.
|
||||||
$node10->path = '/file2';
|
$node[] = $this->generate_test_path('/file2', false, $beforelastweekstime, false);
|
||||||
$node10->isdir = false;
|
|
||||||
$node10->time = $time - $afterweekstime;
|
// Directory older than 1 week to keep.
|
||||||
|
// Note: Since this directory contains a directory that contains a file that is also older than a week
|
||||||
|
// the directory won't be deleted since it's mtime will be updated when the file is deleted.
|
||||||
|
|
||||||
|
$node[] = $this->generate_test_path('/dir4/dir4_1', true, $beforelastweekstime, true);
|
||||||
|
|
||||||
|
$node[] = $this->generate_test_path('/dir4/dir4_1/dir4_1_1/', true, $beforelastweekstime, true);
|
||||||
|
|
||||||
|
// File older than 1 week to remove.
|
||||||
|
$node[] = $this->generate_test_path('/dir4/dir4_1/dir4_1_1/file4_1_1_1', false, $beforelastweekstime, false);
|
||||||
|
|
||||||
|
$expectednodes = array();
|
||||||
|
foreach ($nodes as $node) {
|
||||||
|
if ($node->keep) {
|
||||||
|
$path = $tmpdir;
|
||||||
|
$pelements = preg_split('/\//', $node->path);
|
||||||
|
foreach ($pelements as $pelement) {
|
||||||
|
if ($pelement === '') {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
$path .= DIRECTORY_SEPARATOR . $pelement;
|
||||||
|
if (!in_array($path, $expectednodes)) {
|
||||||
|
$expectednodes[] = $path;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
sort($expectednodes);
|
||||||
|
|
||||||
$data = array(
|
$data = array(
|
||||||
array(
|
array(
|
||||||
array($node1, $node2, $node3, $node4, $node5, $node6, $node7, $node8, $node9, $node10),
|
$nodes,
|
||||||
array(
|
$expectednodes
|
||||||
$tmpdir.DIRECTORY_SEPARATOR.'dir1',
|
|
||||||
$tmpdir.DIRECTORY_SEPARATOR.'dir1'.DIRECTORY_SEPARATOR.'dir1_1',
|
|
||||||
$tmpdir.DIRECTORY_SEPARATOR.'dir1'.DIRECTORY_SEPARATOR.'dir1_1'.DIRECTORY_SEPARATOR.'dir1_2',
|
|
||||||
$tmpdir.DIRECTORY_SEPARATOR.'dir1'.DIRECTORY_SEPARATOR.'dir1_1'.DIRECTORY_SEPARATOR.'dir1_2'.DIRECTORY_SEPARATOR.'file2',
|
|
||||||
$tmpdir.DIRECTORY_SEPARATOR.'dir1'.DIRECTORY_SEPARATOR.'dir1_4',
|
|
||||||
$tmpdir.DIRECTORY_SEPARATOR.'dir1'.DIRECTORY_SEPARATOR.'dir1_4'.DIRECTORY_SEPARATOR.'file2',
|
|
||||||
$tmpdir.DIRECTORY_SEPARATOR.'dir2',
|
|
||||||
$tmpdir.DIRECTORY_SEPARATOR.'file1',
|
|
||||||
)
|
|
||||||
),
|
),
|
||||||
array(
|
array(
|
||||||
array(),
|
array(),
|
||||||
|
|
@ -118,6 +119,22 @@ class cronlib_testcase extends basic_testcase {
|
||||||
return $data;
|
return $data;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Function to populate node array.
|
||||||
|
*
|
||||||
|
* @param string $path Path of directory or file
|
||||||
|
* @param bool $isdir Is the node a directory
|
||||||
|
* @param int $time modified time of the node in epoch
|
||||||
|
* @param bool $keep Should the node exist after the delete function has run
|
||||||
|
*/
|
||||||
|
private function generate_test_path($path, $isdir = false, $time = 0, $keep = false) {
|
||||||
|
$node = new stdClass();
|
||||||
|
$node->path = $path;
|
||||||
|
$node->isdir = $isdir;
|
||||||
|
$node->time = $time;
|
||||||
|
$node->keep = $keep;
|
||||||
|
return $node;
|
||||||
|
}
|
||||||
/**
|
/**
|
||||||
* Test removing files and directories from tempdir.
|
* Test removing files and directories from tempdir.
|
||||||
*
|
*
|
||||||
|
|
@ -136,6 +153,13 @@ class cronlib_testcase extends basic_testcase {
|
||||||
}
|
}
|
||||||
touch($tmpdir.$data->path, $data->time);
|
touch($tmpdir.$data->path, $data->time);
|
||||||
}
|
}
|
||||||
|
// We need to iterate through again since adding a file to a directory will
|
||||||
|
// update the modified time of the directory.
|
||||||
|
foreach ($nodes as $data) {
|
||||||
|
if ($data->isdir) {
|
||||||
|
touch($tmpdir.$data->path, $data->time);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
$task = new \core\task\file_temp_cleanup_task();
|
$task = new \core\task\file_temp_cleanup_task();
|
||||||
$task->execute();
|
$task->execute();
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue