Making Folder::delete() code more PHP5ish

cakephp · Dec 14, 2011 · 16d29a8 · 16d29a8
1 parent a8d0447
commit 16d29a8
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 29 deletions.
diff --git a/lib/Cake/Test/Case/Utility/FolderTest.php b/lib/Cake/Test/Case/Utility/FolderTest.php
@@ -723,10 +723,16 @@ public function testDirSize() {
  */
 	public function testDelete() {
 		$path = TMP . 'folder_delete_test';
-		$Folder = new Folder($path, true);
+		mkdir($path);
-		touch(TMP . 'folder_delete_test' . DS . 'file1');
+		touch($path . DS . 'file_1');
-		touch(TMP . 'folder_delete_test' . DS . 'file2');
+		mkdir($path . DS . 'level_1_1');
+		touch($path . DS . 'level_1_1' . DS . 'file_1_1');
+		mkdir($path . DS . 'level_1_1' . DS . 'level_2_1');
+		touch($path . DS . 'level_1_1' . DS . 'level_2_1' . DS . 'file_2_1');
+		touch($path . DS . 'level_1_1' . DS . 'level_2_1' . DS . 'file_2_2');
+		mkdir($path . DS . 'level_1_1' . DS . 'level_2_2');
 
+		$Folder = new Folder($path, true);
 		$return = $Folder->delete();
 		$this->assertTrue($return);
 
@@ -735,9 +741,13 @@ public function testDelete() {
 		$this->assertEquals($errors, array());
 
 		$expected = array(
-			$path . ' created',
+			$path . DS . 'file_1 removed',
-			$path . DS . 'file1 removed',
+			$path . DS . 'level_1_1' . DS . 'file_1_1 removed',
-			$path . DS . 'file2 removed',
+			$path . DS . 'level_1_1' . DS . 'level_2_1' . DS . 'file_2_1 removed',
+			$path . DS . 'level_1_1' . DS . 'level_2_1' . DS . 'file_2_2 removed',
+			$path . DS . 'level_1_1' . DS . 'level_2_1 removed',
+			$path . DS . 'level_1_1' . DS . 'level_2_2 removed',
+			$path . DS . 'level_1_1 removed',
 			$path . ' removed'
 		);
 		$this->assertEquals($expected, $messages);

diff --git a/lib/Cake/Utility/Folder.php b/lib/Cake/Utility/Folder.php
@@ -554,36 +554,32 @@ public function delete($path = null) {
 			return null;
 		}
 		$path = Folder::slashTerm($path);
-		if (is_dir($path) === true) {
+		if (is_dir($path)) {
-			$normalFiles = glob($path . '*');
+			$iterator = new RecursiveDirectoryIterator($path, RecursiveDirectoryIterator::SKIP_DOTS);
-			$hiddenFiles = glob($path . '\.?*');
+			foreach (new RecursiveIteratorIterator($iterator, RecursiveIteratorIterator::CHILD_FIRST) as $file) {
-
+				$filePath = $file->getPathname();
-			$normalFiles = $normalFiles ? $normalFiles : array();
+				if ($file->isFile() || $file->isLink()) {
-			$hiddenFiles = $hiddenFiles ? $hiddenFiles : array();
+					if (@unlink($filePath)) {
-
+						$this->_messages[] = __d('cake_dev', '%s removed', $filePath);
-			$files = array_merge($normalFiles, $hiddenFiles);
+					} else {
-			if (is_array($files)) {
+						$this->_errors[] = __d('cake_dev', '%s NOT removed', $filePath);
-				foreach ($files as $file) {
-					if (preg_match('/(\.|\.\.)$/', $file)) {
-						continue;
 					}
-					if (is_file($file) === true) {
+				} elseif ($file->isDir()) {
-						if (@unlink($file)) {
+					if (@rmdir($filePath)) {
-							$this->_messages[] = __d('cake_dev', '%s removed', $file);
+						$this->_messages[] = __d('cake_dev', '%s removed', $filePath);
-						} else {
+					} else {
-							$this->_errors[] = __d('cake_dev', '%s NOT removed', $file);
+						$this->_errors[] = __d('cake_dev', '%s NOT removed', $filePath);
-						}
-					} elseif (is_dir($file) === true && $this->delete($file) === false) {
 						return false;
 					}
 				}
 			}
-			$path = substr($path, 0, strlen($path) - 1);
+
-			if (rmdir($path) === false) {
+			$path = rtrim($path, DS);
+			if (@rmdir($path)) {
+				$this->_messages[] = __d('cake_dev', '%s removed', $path);
+			} else {
 				$this->_errors[] = __d('cake_dev', '%s NOT removed', $path);
 				return false;
-			} else {
-				$this->_messages[] = __d('cake_dev', '%s removed', $path);
 			}
 		}
 		return true;