From d69243ee5144afeed57f513b0b50f9f2dbcdd36a Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 29 Nov 2013 12:58:57 +0100 Subject: [PATCH] Fixed FTP and SMB to use rmdir() when deleting folders Some storages need to use different calls for deleting files or folders, usually unlink() and rmdir(). Fixes #4532 (SMB dir deletion) Fixes #5941 (FTP dir deletion) Note that the extra is_dir() should be fast because it's read from the stat cache. --- apps/files_external/lib/ftp.php | 16 ++++++++++++++++ apps/files_external/lib/smb.php | 12 +++++++++--- apps/files_external/lib/streamwrapper.php | 12 ++++++++---- tests/lib/files/storage/storage.php | 14 +++++++++++++- 4 files changed, 46 insertions(+), 8 deletions(-) diff --git a/apps/files_external/lib/ftp.php b/apps/files_external/lib/ftp.php index 1f3ebd14b3..00bf7a189c 100644 --- a/apps/files_external/lib/ftp.php +++ b/apps/files_external/lib/ftp.php @@ -61,6 +61,22 @@ class FTP extends \OC\Files\Storage\StreamWrapper{ $url.='://'.$this->user.':'.$this->password.'@'.$this->host.$this->root.$path; return $url; } + + /** + * Unlinks file or directory + * @param string @path + */ + public function unlink($path) { + if ($this->is_dir($path)) { + return $this->rmdir($path); + } + else { + $url = $this->constructUrl($path); + $result = unlink($url); + clearstatcache(true, $url); + return $result; + } + } public function fopen($path,$mode) { switch($mode) { case 'r': diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php index 5bff597fdc..c5fba92ee6 100644 --- a/apps/files_external/lib/smb.php +++ b/apps/files_external/lib/smb.php @@ -82,12 +82,18 @@ class SMB extends \OC\Files\Storage\StreamWrapper{ } /** - * Unlinks file + * Unlinks file or directory * @param string @path */ public function unlink($path) { - unlink($this->constructUrl($path)); - clearstatcache(); + if ($this->is_dir($path)) { + $this->rmdir($path); + } + else { + $url = $this->constructUrl($path); + unlink($url); + clearstatcache(false, $url); + } // smb4php still returns false even on success so // check here whether file was really deleted return !file_exists($path); diff --git a/apps/files_external/lib/streamwrapper.php b/apps/files_external/lib/streamwrapper.php index 7a1991d4f0..e484325e2f 100644 --- a/apps/files_external/lib/streamwrapper.php +++ b/apps/files_external/lib/streamwrapper.php @@ -25,8 +25,9 @@ abstract class StreamWrapper extends Common { $this->unlink($path . '/' . $file); } } - $success = rmdir($this->constructUrl($path)); - clearstatcache(); + $url = $this->constructUrl($path); + $success = rmdir($url); + clearstatcache(false, $url); return $success; } else { return false; @@ -46,8 +47,11 @@ abstract class StreamWrapper extends Common { } public function unlink($path) { - $success = unlink($this->constructUrl($path)); - clearstatcache(); + $url = $this->constructUrl($path); + $success = unlink($url); + // normally unlink() is supposed to do this implicitly, + // but doing it anyway just to be sure + clearstatcache(false, $url); return $success; } diff --git a/tests/lib/files/storage/storage.php b/tests/lib/files/storage/storage.php index 19113f5262..5a0581665a 100644 --- a/tests/lib/files/storage/storage.php +++ b/tests/lib/files/storage/storage.php @@ -254,7 +254,19 @@ abstract class Storage extends \PHPUnit_Framework_TestCase { $this->instance->mkdir('folder/bar'); $this->instance->file_put_contents('folder/asd.txt', 'foobar'); $this->instance->file_put_contents('folder/bar/foo.txt', 'asd'); - $this->instance->rmdir('folder'); + $this->assertTrue($this->instance->rmdir('folder')); + $this->assertFalse($this->instance->file_exists('folder/asd.txt')); + $this->assertFalse($this->instance->file_exists('folder/bar/foo.txt')); + $this->assertFalse($this->instance->file_exists('folder/bar')); + $this->assertFalse($this->instance->file_exists('folder')); + } + + public function testRecursiveUnlink() { + $this->instance->mkdir('folder'); + $this->instance->mkdir('folder/bar'); + $this->instance->file_put_contents('folder/asd.txt', 'foobar'); + $this->instance->file_put_contents('folder/bar/foo.txt', 'asd'); + $this->assertTrue($this->instance->unlink('folder')); $this->assertFalse($this->instance->file_exists('folder/asd.txt')); $this->assertFalse($this->instance->file_exists('folder/bar/foo.txt')); $this->assertFalse($this->instance->file_exists('folder/bar'));