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.

diff --git a/apps/files_external/lib/ftp.php b/apps/files_external/lib/ftp.php
index 1f3ebd14b3953662d8da7fe28785e5d9aaa9ee7b..00bf7a189cebabba35e76880e8aec7b69edf8b6c 100644 (file)
--- 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 5bff597fdca0607a9aeff09c9eae4f9575d39ed7..c5fba92ee68217b0a83e4a253dc9403d6bf95af6 100644 (file)
--- 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 7a1991d4f04fc7ef136733d0905fe2839446584c..e484325e2fb835de74cc3745f2fd7657f550b7e2 100644 (file)
--- 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 19113f52623345f57d21b1b8540a425d01dd2584..5a0581665a2ab253478f3f7988f20811227276c6 100644 (file)
--- 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'));