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.

Backport of d69243e

diff --git a/apps/files_external/lib/ftp.php b/apps/files_external/lib/ftp.php
index ca6c635eb2b802812e8a3ef43d5dca6538e3760c..85d7a5a32aee7134a1f8779315939199e0c82286 100644 (file)
--- a/apps/files_external/lib/ftp.php
+++ b/apps/files_external/lib/ftp.php
@@ -58,6 +58,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 fb6259930ba75b545c7d98ba84852d4a8cfff343..0ca58f0daa777218902ec94d7e1c892119cda01d 100644 (file)
--- a/apps/files_external/lib/smb.php
+++ b/apps/files_external/lib/smb.php
@@ -72,12 +72,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 a086f411f573b0005c53d82ca9df9e1699687043..a01204d45fb408b084103a840e353f0747c16856 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;
@@ -59,8 +60,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 4587a3b2d06cc78d4cbca918ec4dfcb537224b21..ea3ca7d19e05468e049a1b11ae7bd3a279e586c0 100644 (file)
--- a/tests/lib/files/storage/storage.php
+++ b/tests/lib/files/storage/storage.php
@@ -286,4 +286,28 @@ abstract class Storage extends \PHPUnit_Framework_TestCase {
                $this->instance->touch('foo');
                $this->assertTrue($this->instance->file_exists('foo'));
        }
+
+       public function testRecursiveRmdir() {
+               $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->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'));
+               $this->assertFalse($this->instance->file_exists('folder'));
+       }
 }