Uniform "overwrite" behaviour for copy/move/copydir/movedir.

New "force" argument to removedir().


git-svn-id: http://pyfilesystem.googlecode.com/svn/trunk@119 67cdc799-7952-0410-af00-57a81ceafa0f

7 files changed, 145 insertions, 59 deletions

diff --git a/fs/base.py b/fs/base.py
index cb8100d..676e153 100644
--- a/fs/base.py
+++ b/fs/base.py
@@ -91,6 +91,7 @@ class NoSysPathError(FSError): pass
 class PathError(FSError): pass
 class ResourceLockedError(FSError): pass
 class ResourceNotFoundError(FSError): pass
+class DestinationExistsError(FSError): pass
 class SystemError(FSError): pass
 class ResourceInvalid(FSError): pass
 
@@ -222,6 +223,9 @@ class FS(object):
             self._lock = dummy_threading.RLock()
 
     def __getstate__(self):
+        #  Locks can't be pickled, so instead we just indicate the
+        #  type of lock that should be there.  None == no lock,
+        #  True == a proper lock, False == a dummy lock.
         state = self.__dict__.copy()
         lock = state.get("_lock",None)
         if lock is not None:
@@ -351,11 +355,12 @@ class FS(object):
         """
         raise UnsupportedError("UNSUPPORTED")
 
-    def removedir(self, path, recursive=False):
+    def removedir(self, path, recursive=False, force=False):
         """Remove a directory
 
         path -- Path of the directory to remove
         recursive -- If True, then blank parent directories will be removed
+        force -- If True, any directory contents will be removed
 
         """
         raise UnsupportedError("UNSUPPORTED")
@@ -560,6 +565,8 @@ class FS(object):
 
         if not self.isfile(src):
             raise ResourceInvalid("WRONG_TYPE", src, msg="Source is not a file: %(path)s")
+        if not overwrite and self.exists(dst):
+            raise DestinationExistsError("COPYFILE_FAILED", src, dst, msg="Destination file exists: %(path2)s")
 
         src_syspath = self.getsyspath(src, allow_none=True)
         dst_syspath = self.getsyspath(dst, allow_none=True)
@@ -570,9 +577,6 @@ class FS(object):
             src_file, dst_file = None, None
             try:
                 src_file = self.open(src, "rb")
-                if not overwrite:
-                    if self.exists(dst):
-                        raise OperationFailedError("COPYFILE_FAILED", src, dst, msg="Destination file exists: %(path2)s")
                 dst_file = self.open(dst, "wb")
 
                 while True:
@@ -586,11 +590,12 @@ class FS(object):
                 if dst_file is not None:
                     dst_file.close()
 
-    def move(self, src, dst, chunk_size=16384):
+    def move(self, src, dst, overwrite=False, chunk_size=16384):
         """Moves a file from one location to another.
 
         src -- Source path
         dst -- Destination path
+        overwrite -- If True, then the destination may be overwritten
 
         """
 
@@ -600,23 +605,28 @@ class FS(object):
         if src_syspath is not None and dst_syspath is not None:
             if not self.isfile(src):
                 raise ResourceInvalid("WRONG_TYPE", src, msg="Source is not a file: %(path)s")
+            if not overwrite and self.exists(dst):
+                raise DestinationExistsError("MOVE_FAILED", src, dst, msg="Destination file exists: %(path2)s")
             shutil.move(src_syspath, dst_syspath)
         else:
-            self.copy(src, dst, chunk_size=chunk_size)
+            self.copy(src, dst, overwrite=overwrite, chunk_size=chunk_size)
             self.remove(src)
 
 
-    def movedir(self, src, dst, ignore_errors=False, chunk_size=16384):
+    def movedir(self, src, dst, overwrite=False, ignore_errors=False, chunk_size=16384):
         """Moves a directory from one location to another.
 
         src -- Source directory path
         dst -- Destination directory path
+        overwrite -- If True then any existing files in the destination directory will be overwritten
         ignore_errors -- If True then this method will ignore FSError exceptions when moving files
         chunk_size -- Size of chunks to use when copying, if a simple copy is required
 
         """
         if not self.isdir(src):
             raise ResourceInvalid("WRONG_TYPE", src, msg="Source is not a dst: %(path)s")
+        if not overwrite and self.exists(dst):
+            raise DestinationExistsError("MOVEDIR_FAILED", src, dst, msg="Destination exists: %(path2)s")
 
         src_syspath = self.getsyspath(src, allow_none=True)
         dst_syspath = self.getsyspath(dst, allow_none=True)
@@ -628,10 +638,9 @@ class FS(object):
             except WindowsError:
                 pass
 
-
-        def movefile_noerrors(src, dst):
+        def movefile_noerrors(src, dst, overwrite):
             try:
-                return self.move(src, dst)
+                return self.move(src, dst, overwrite)
             except FSError:
                 return
         if ignore_errors:
@@ -650,29 +659,30 @@ class FS(object):
 
                 src_filename = pathjoin(dirname, filename)
                 dst_filename = pathjoin(dst_dirpath, filename)
-                movefile(src_filename, dst_filename, chunk_size=chunk_size)
+                movefile(src_filename, dst_filename, overwrite=overwrite, chunk_size=chunk_size)
 
             self.removedir(dirname)
 
 
 
-    def copydir(self, src, dst, ignore_errors=False, chunk_size=16384):
+    def copydir(self, src, dst, overwrite=False, ignore_errors=False, chunk_size=16384):
         """Copies a directory from one location to another.
 
         src -- Source directory path
         dst -- Destination directory path
+        overwrite -- If True then any existing files in the destination directory will be overwritten
         ignore_errors -- If True, exceptions when copying will be ignored
         chunk_size -- Size of chunks to use when copying, if a simple copy is required
 
         """
         if not self.isdir(src):
             raise ResourceInvalid("WRONG_TYPE", src, msg="Source is not a dst: %(path)s")
-        if not self.isdir(dst):
-            raise ResourceInvalid("WRONG_TYPE", dst, msg="Source is not a dst: %(path)s")
+        if not overwrite and self.exists(dst):
+            raise DestinationExistsError("COPYDIR_FAILED", dst, msg="Destination exists: %(path)s")
 
-        def copyfile_noerrors(src, dst):
+        def copyfile_noerrors(src, dst, overwrite):
             try:
-                return self.copy(src, dst)
+                return self.copy(src, dst, overwrite=overwrite)
             except FSError:
                 return
         if ignore_errors:
@@ -692,7 +702,7 @@ class FS(object):
 
                 src_filename = pathjoin(dirname, filename)
                 dst_filename = pathjoin(dst_dirpath, filename)
-                copyfile(src_filename, dst_filename, chunk_size=chunk_size)
+                copyfile(src_filename, dst_filename, overwrite=overwrite, chunk_size=chunk_size)
 
 
     def isdirempty(self, path):
@@ -790,8 +800,8 @@ class SubFS(FS):
     def remove(self, path):
         return self.parent.remove(self._delegate(path))
 
-    def removedir(self, path, recursive=False):
-        self.parent.removedir(self._delegate(path), recursive=recursive)
+    def removedir(self, path, recursive=False,force=False):
+        self.parent.removedir(self._delegate(path), recursive=recursive, force=force)
 
     def getinfo(self, path):
         return self.parent.getinfo(self._delegate(path))
diff --git a/fs/memoryfs.py b/fs/memoryfs.py
index bdadf5d..42a2c03 100644
--- a/fs/memoryfs.py
+++ b/fs/memoryfs.py
@@ -370,7 +370,7 @@ class MemoryFS(FS):
         finally:

             self._lock.release()

 

-    def removedir(self, path, recursive=False):

+    def removedir(self, path, recursive=False, force=False):

         self._lock.acquire()

         try:

             dir_entry = self._get_dir_entry(path)

@@ -382,7 +382,7 @@ class MemoryFS(FS):
             if not dir_entry.isdir():

                 raise ResourceInvalid("WRONG_TYPE", path, msg="Can't remove resource, its not a directory: %(path)s" )

 

-            if dir_entry.contents:

+            if dir_entry.contents and not force:

                 raise OperationFailedError("REMOVEDIR_FAILED", "Directory is not empty: %(path)s")

 

             if recursive:

diff --git a/fs/mountfs.py b/fs/mountfs.py
index 0d0b425..e00d2f6 100644
--- a/fs/mountfs.py
+++ b/fs/mountfs.py
@@ -198,7 +198,7 @@ class MountFS(FS):
         finally:
             self._lock.release()
 
-    def removedir(self, path, recursive=False):
+    def removedir(self, path, recursive=False, force=False):
 
         self._lock.acquire()
         try:
@@ -209,10 +209,10 @@ class MountFS(FS):
             if fs is None or fs is self:
                 raise OperationFailedError("REMOVEDIR_FAILED", path, msg="Can not removedir for an un-mounted path")
 
-            if not fs.isdirempty(delegate_path):
+            if not force and not fs.isdirempty(delegate_path):
                 raise OperationFailedError("REMOVEDIR_FAILED", "Directory is not empty: %(path)s")
 
-            return fs.removedir(delegate_path, recursive)
+            return fs.removedir(delegate_path, recursive, force)
 
         finally:
             self._lock.release()
@@ -351,4 +351,4 @@ if __name__ == "__main__":
 
     print mountfs.getinfo("1/2")
 
-    #print mountfs._delegate('1/2/Memroot/B')
\ No newline at end of file
+    #print mountfs._delegate('1/2/Memroot/B')
diff --git a/fs/osfs.py b/fs/osfs.py
index a30c40b..5ad3236 100644
--- a/fs/osfs.py
+++ b/fs/osfs.py
@@ -101,11 +101,16 @@ class OSFS(FS):
         except OSError, e:
             raise OperationFailedError("REMOVE_FAILED", path, details=e)
 
-    def removedir(self, path, recursive=False):
+    def removedir(self, path, recursive=False,force=False):
         sys_path = self.getsyspath(path)
         #  Don't remove the root directory of this FS
         if path in ("","/"):
             return
+        if force:
+            for path2 in self.listdir(path,absolute=True,files_only=True):
+                self.remove(path2)
+            for path2 in self.listdir(path,absolute=True,dirs_only=True):
+                self.removedir(path2,force=True)
         try:
             os.rmdir(sys_path)
         except OSError, e:
diff --git a/fs/rpcfs.py b/fs/rpcfs.py
index ced3b7a..6141aae 100644
--- a/fs/rpcfs.py
+++ b/fs/rpcfs.py
@@ -180,8 +180,8 @@ class RPCFS(FS):
     def remove(self,path):
         return self.proxy.remove(path)
 
-    def removedir(self,path,recursive=False):
-        return self.proxy.removedir(path,recursive)
+    def removedir(self,path,recursive=False,force=False):
+        return self.proxy.removedir(path,recursive,force)
         
     def rename(self,src,dst):
         return self.proxy.rename(src,dst)
@@ -201,14 +201,14 @@ class RPCFS(FS):
     def copy(self,src,dst,overwrite=False,chunk_size=16384):
         return self.proxy.copy(src,dst,overwrite,chunk_size)
 
-    def move(self,src,dst,chunk_size=16384):
-        return self.proxy.move(src,dst,chunk_size)
+    def move(self,src,dst,overwrite=False,chunk_size=16384):
+        return self.proxy.move(src,dst,overwrite,chunk_size)
 
-    def movedir(self,src,dst,ignore_errors=False,chunk_size=16384):
-        return self.proxy.movedir(src,dst,ignore_errors,chunk_size)
+    def movedir(self,src,dst,overwrite=False,ignore_errors=False,chunk_size=16384):
+        return self.proxy.movedir(src,dst,overwrite,ignore_errors,chunk_size)
 
-    def copydir(self,src,dst,ignore_errors=False,chunk_size=16384):
-        return self.proxy.copydir(src,dst,ignore_errors,chunk_size)
+    def copydir(self,src,dst,overwrite=False,ignore_errors=False,chunk_size=16384):
+        return self.proxy.copydir(src,dst,overwrite,ignore_errors,chunk_size)
 
 
 class RPCFSInterface(object):
@@ -246,8 +246,8 @@ class RPCFSInterface(object):
     def remove(self,path):
         return self.fs.remove(path)
 
-    def removedir(self,path,recursive=False):
-        return self.fs.removedir(path,recursive)
+    def removedir(self,path,recursive=False,force=False):
+        return self.fs.removedir(path,recursive,force)
         
     def rename(self,src,dst):
         return self.fs.rename(src,dst)
@@ -267,14 +267,14 @@ class RPCFSInterface(object):
     def copy(self,src,dst,overwrite=False,chunk_size=16384):
         return self.fs.copy(src,dst,overwrite,chunk_size)
 
-    def move(self,src,dst,chunk_size=16384):
-        return self.fs.move(src,dst,chunk_size)
+    def move(self,src,dst,overwrite=False,chunk_size=16384):
+        return self.fs.move(src,dst,overwrite,chunk_size)
 
-    def movedir(self,src,dst,ignore_errors=False,chunk_size=16384):
-        return self.fs.movedir(src,dst,ignore_errors,chunk_size)
+    def movedir(self,src,dst,overwrite=False,ignore_errors=False,chunk_size=16384):
+        return self.fs.movedir(src,dst,overwrite,ignore_errors,chunk_size)
 
-    def copydir(self,src,dst,ignore_errors=False,chunk_size=16384):
-        return self.fs.copydir(src,dst,ignore_errors,chunk_size)
+    def copydir(self,src,dst,overwrite=False,ignore_errors=False,chunk_size=16384):
+        return self.fs.copydir(src,dst,overwrite,ignore_errors,chunk_size)
 
 
 class RPCFSServer(SimpleXMLRPCServer):
diff --git a/fs/s3fs.py b/fs/s3fs.py
index cc4b762..c342228 100644
--- a/fs/s3fs.py
+++ b/fs/s3fs.py
@@ -126,7 +126,7 @@ class S3FS(FS):
         Since S3 only offers "eventual consistency" of data, it is possible
         to create a key but be unable to read it back straight away.  This
         method works around that limitation by polling the key until it reads
-        back the expected by the given key.
+        back the value expected by the given key.
 
         Note that this could easily fail if the key is modified by another
         program, meaning the content will never be as specified in the given
@@ -273,7 +273,6 @@ class S3FS(FS):
                 paths.append(nm)
         if not isDir:
             if s3path != self._prefix:
-                print "NOT A DIR:", s3path
                 raise OperationFailedError("LISTDIR_FAILED",path)
         return self._listdir_helper(path,paths,wildcard,full,absolute,hidden,dirs_only,files_only)
 
@@ -354,19 +353,26 @@ class S3FS(FS):
         while k:
             k = self._s3bukt.get_key(s3path)
 
-    def removedir(self,path,recursive=False):
+    def removedir(self,path,recursive=False,force=False):
         """Remove the directory at the given path."""
         s3path = self._s3path(path) + self._separator
-        ks = self._s3bukt.list(prefix=s3path,delimiter=self._separator)
-        # Fail if the directory is not empty
+        if force:
+            #  If we will be forcibly removing any directory contents, we 
+            #  might as well get the un-delimited list straight away.
+            ks = self._s3bukt.list(prefix=s3path)
+        else:
+            ks = self._s3bukt.list(prefix=s3path,delimiter=self._separator)
+        # Fail if the directory is not empty, or remove them if forced
         for k in ks:
             if k.name != s3path:
-                raise OperationFailedError("REMOVEDIR_FAILED",path)
+                if not force:
+                    raise OperationFailedError("REMOVEDIR_FAILED",path)
+                self._s3bukt.delete_key(k.name)
         self._s3bukt.delete_key(s3path)
         if recursive:
             pdir = dirname(path)
             try:
-                self.removedir(pdir,True)
+                self.removedir(pdir,recursive=True,force=False)
             except OperationFailedError:
                 pass
         
@@ -408,11 +414,11 @@ class S3FS(FS):
             # It exists as a regular file
             if k.name == s3path_dst:
                 if not overwrite:
-                    raise OperationFailedError("COPYFILE_FAILED",src,dst,msg="Destination file exists: %(path2)s")
+                    raise DestinationExistsError("COPYFILE_FAILED",src,dst,msg="Destination file exists: %(path2)s")
                 dstOK = True
                 break
             # Check if it refers to a directory.  If so, we copy *into* it.
-            # Since S3 lists in lexicographic order, subsequence iterations
+            # Since S3 lists in lexicographic order, subsequent iterations
             # of the loop will check for the existence of the new filename.
             if k.name == s3path_dstD:
                 nm = resourcename(src)
@@ -433,8 +439,8 @@ class S3FS(FS):
             k = self._s3bukt.get_key(s3path_dst)
             self._sync_key(k)
 
-    def move(self,src,dst,chunk_size=16384):
+    def move(self,src,dst,overwrite=False,chunk_size=16384):
         """Move a file from one location to another."""
-        self.copy(src,dst)
+        self.copy(src,dst,overwrite=overwrite)
         self._s3bukt.delete_key(self._s3path(src))
 
diff --git a/fs/tests.py b/fs/tests.py
index e86c3ad..7179cc8 100644
--- a/fs/tests.py
+++ b/fs/tests.py
@@ -196,6 +196,12 @@ class TestOSFS(unittest.TestCase):
         self.assert_(not check("foo/bar"))
         self.assert_(not check("foo"))
 
+        self.fs.makedir("frollic/waggle", recursive=True)
+        self.fs.createfile("frollic/waddle.txt","waddlewaddlewaddle")
+        self.assertRaises(fs.OperationFailedError,self.fs.removedir,"frollic")
+        self.fs.removedir("frollic",force=True)
+        self.assert_(not check("frollic"))
+
     def test_listdir(self):
 
         def makefile(fname):
@@ -295,6 +301,15 @@ class TestOSFS(unittest.TestCase):
         self.assert_(check("/c.txt"))
         self.assert_(checkcontents("/c.txt"))
 
+        makefile("foo/bar/a.txt")
+        self.assertRaises(fs.DestinationExistsError,self.fs.move,"foo/bar/a.txt","/c.txt")
+        self.assert_(check("foo/bar/a.txt"))
+        self.assert_(check("/c.txt"))
+        self.fs.move("foo/bar/a.txt","/c.txt",overwrite=True)
+        self.assert_(not check("foo/bar/a.txt"))
+        self.assert_(check("/c.txt"))
+
+
     def test_movedir(self):
         check = self.check
         contents = "If the implementation is hard to explain, it's a bad idea."
@@ -311,7 +326,6 @@ class TestOSFS(unittest.TestCase):
         self.fs.makedir("a/foo/bar", recursive=True)
         makefile("a/foo/bar/baz.txt")
 
-        self.fs.makedir("copy of a")
         self.fs.movedir("a", "copy of a")
 
         self.assert_(check("copy of a/1.txt"))
@@ -327,6 +341,51 @@ class TestOSFS(unittest.TestCase):
         self.assert_(not check("a/foo"))
         self.assert_(not check("a"))
 
+        self.fs.makedir("a")
+        self.assertRaises(fs.DestinationExistsError,self.fs.movedir,"copy of a","a")
+        self.fs.movedir("copy of a","a",overwrite=True)
+        self.assert_(not check("copy of a"))
+        self.assert_(check("a/1.txt"))
+        self.assert_(check("a/2.txt"))
+        self.assert_(check("a/3.txt"))
+        self.assert_(check("a/foo/bar/baz.txt"))
+        
+
+    def test_copyfile(self):
+        check = self.check
+        contents = "If the implementation is hard to explain, it's a bad idea."
+        def makefile(path,contents=contents):
+            f = self.fs.open(path, "wb")
+            f.write(contents)
+            f.close()
+        def checkcontents(path,contents=contents):
+            f = self.fs.open(path, "rb")
+            check_contents = f.read()
+            f.close()
+            self.assertEqual(check_contents,contents)
+            return contents == check_contents
+
+        self.fs.makedir("foo/bar", recursive=True)
+        makefile("foo/bar/a.txt")
+        self.assert_(check("foo/bar/a.txt"))
+        self.assert_(checkcontents("foo/bar/a.txt"))
+        self.fs.copy("foo/bar/a.txt", "foo/b.txt")
+        self.assert_(check("foo/bar/a.txt"))
+        self.assert_(check("foo/b.txt"))
+        self.assert_(checkcontents("foo/b.txt"))
+
+        self.fs.copy("foo/b.txt", "c.txt")
+        self.assert_(check("foo/b.txt"))
+        self.assert_(check("/c.txt"))
+        self.assert_(checkcontents("/c.txt"))
+
+        makefile("foo/bar/a.txt","different contents")
+        self.assertRaises(fs.DestinationExistsError,self.fs.copy,"foo/bar/a.txt","/c.txt")
+        self.assert_(checkcontents("/c.txt"))
+        self.fs.copy("foo/bar/a.txt","/c.txt",overwrite=True)
+        self.assert_(checkcontents("foo/bar/a.txt","different contents"))
+        self.assert_(checkcontents("/c.txt","different contents"))
+
 
     def test_copydir(self):
         check = self.check
@@ -344,7 +403,6 @@ class TestOSFS(unittest.TestCase):
         self.fs.makedir("a/foo/bar", recursive=True)
         makefile("a/foo/bar/baz.txt")
 
-        self.fs.makedir("copy of a")
         self.fs.copydir("a", "copy of a")
         self.assert_(check("copy of a/1.txt"))
         self.assert_(check("copy of a/2.txt"))
@@ -356,6 +414,14 @@ class TestOSFS(unittest.TestCase):
         self.assert_(check("a/3.txt"))
         self.assert_(check("a/foo/bar/baz.txt"))
 
+        self.assertRaises(fs.DestinationExistsError,self.fs.copydir,"a","b")
+        self.fs.copydir("a","b",overwrite=True)
+        self.assert_(check("b/1.txt"))
+        self.assert_(check("b/2.txt"))
+        self.assert_(check("b/3.txt"))
+        self.assert_(check("b/foo/bar/baz.txt"))
+       
+
     def test_copydir_with_hidden(self):
         check = self.check
         contents = "If the implementation is hard to explain, it's a bad idea."
@@ -369,7 +435,6 @@ class TestOSFS(unittest.TestCase):
         makefile("a/2.txt")
         makefile("a/.hidden.txt")
 
-        self.fs.makedir("copy of a")
         self.fs.copydir("a", "copy of a")
         self.assert_(check("copy of a/1.txt"))
         self.assert_(check("copy of a/2.txt"))
@@ -671,7 +736,7 @@ class TestS3FS(TestOSFS):
     def test_with_statement(self):
         import sys
         if sys.version_info[0] >= 2 and sys.version_info[1] >= 5:
-            #  A successful with statement
+            #  A successful 'with' statement
             contents = "testing the with statement"
             code = "from __future__ import with_statement\n"
             code += "with self.fs.open('f.txt','w-') as testfile:\n"
@@ -679,7 +744,7 @@ class TestS3FS(TestOSFS):
             code += "self.assertEquals(self.fs.getcontents('f.txt'),contents)"
             code = compile(code,"<string>",'exec')
             eval(code)
-            # A with statement raising an error
+            # A 'with' statement raising an error
             contents = "testing the with statement"
             code = "from __future__ import with_statement\n"
             code += "with self.fs.open('f.txt','w-') as testfile:\n"
@@ -688,7 +753,7 @@ class TestS3FS(TestOSFS):
             code = compile(code,"<string>",'exec')
             self.assertRaises(ValueError,eval,code,globals(),locals())
             self.assertEquals(self.fs.getcontents('f.txt'),contents)
-            
+           
 
 
 import rpcfs