add optimization

Author: tfeldmann

CHANGELOG.md

@@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## Unreleased
 
+- Optimized moving folders between filesystems with syspaths
+  ([#550](https://github.com/PyFilesystem/pyfilesystem2/pull/550))
 
 ### Added
 

fs/move.py

@@ -5,12 +5,15 @@
 
 import typing
 
+import os
+
 from ._pathcompat import commonpath
 from .copy import copy_dir, copy_file
-from .errors import FSError
+from .error_tools import convert_os_errors
+from .errors import DirectoryExpected, FSError, IllegalDestination, ResourceNotFound
 from .opener import manage_fs
 from .osfs import OSFS
-from .path import frombase
+from .path import frombase, isbase
 
 if typing.TYPE_CHECKING:
     from typing import Text, Union
@@ -26,15 +29,13 @@ def move_fs(
 ):
     # type: (...) -> None
     """Move the contents of a filesystem to another filesystem.
-
     Arguments:
         src_fs (FS or str): Source filesystem (instance or URL).
         dst_fs (FS or str): Destination filesystem (instance or URL).
         workers (int): Use `worker` threads to copy data, or ``0`` (default) for
             a single-threaded copy.
         preserve_time (bool): If `True`, try to preserve mtime of the
             resources (defaults to `False`).
-
     """
     move_dir(src_fs, "/", dst_fs, "/", workers=workers, preserve_time=preserve_time)
 
@@ -49,7 +50,6 @@ def move_file(
 ):
     # type: (...) -> None
     """Move a file from one filesystem to another.
-
     Arguments:
         src_fs (FS or str): Source filesystem (instance or URL).
         src_path (str): Path to a file on ``src_fs``.
@@ -59,7 +59,6 @@ def move_file(
             resources (defaults to `False`).
         cleanup_dst_on_error (bool): If `True`, tries to delete the file copied to
             ``dst_fs`` if deleting the file from ``src_fs`` fails (defaults to `True`).
-
     """
     with manage_fs(src_fs, writeable=True) as _src_fs:
         with manage_fs(dst_fs, writeable=True, create=True) as _dst_fs:
@@ -123,7 +122,6 @@ def move_dir(
 ):
     # type: (...) -> None
     """Move a directory from one filesystem to another.
-
     Arguments:
         src_fs (FS or str): Source filesystem (instance or URL).
         src_path (str): Path to a directory on ``src_fs``
@@ -133,10 +131,35 @@ def move_dir(
             (default) for a single-threaded copy.
         preserve_time (bool): If `True`, try to preserve mtime of the
             resources (defaults to `False`).
-
+    Raises:
+        fs.errors.ResourceNotFound: if ``src_path`` does not exist on `src_fs`
+        fs.errors.DirectoryExpected: if ``src_path`` or one of its
+            ancestors is not a directory.
+        fs.errors.IllegalDestination: when moving a folder into itself
     """
     with manage_fs(src_fs, writeable=True) as _src_fs:
         with manage_fs(dst_fs, writeable=True, create=True) as _dst_fs:
+            if not _src_fs.exists(src_path):
+                raise ResourceNotFound(src_path)
+            if not _src_fs.isdir(src_path):
+                raise DirectoryExpected(src_path)
+
+            # if both filesystems have a syspath we use `os.rename` to move the folder
+            if _src_fs.hassyspath(src_path) and _dst_fs.hassyspath(dst_path):
+                src_syspath = _src_fs.getsyspath(src_path)
+                dst_syspath = _dst_fs.getsyspath(dst_path)
+                # recheck if the move operation is legal using the syspaths
+                if isbase(src_syspath, dst_syspath):
+                    raise IllegalDestination(dst_path)
+                with _src_fs.lock(), _dst_fs.lock():
+                    with convert_os_errors("move_dir", src_path, directory=True):
+                        os.rename(src_syspath, dst_syspath)
+                        # recreate the root dir if it has been renamed
+                        if src_path == "/":
+                            _src_fs.makedir("/")
+                    return  # optimization worked, exit early
+
+            # standard copy then delete
             with _src_fs.lock(), _dst_fs.lock():
                 _dst_fs.makedir(dst_path, recreate=True)
                 copy_dir(

tests/test_move.py

@@ -7,6 +7,7 @@
 except ImportError:
     import mock
 
+import os
 from parameterized import parameterized, parameterized_class
 
 import fs.move
@@ -167,7 +168,7 @@ def test_move_file_overwrite(self, _, fs_url):
             self.assertFalse(src.exists("target.txt"))
             self.assertFalse(dst.exists("file.txt"))
             self.assertTrue(dst.exists("target.txt"))
-            self.assertEquals(dst.readtext("target.txt"), "source content")
+            self.assertEqual(dst.readtext("target.txt"), "source content")
 
     @parameterized.expand([("temp", "temp://"), ("mem", "mem://")])
     def test_move_file_overwrite_itself(self, _, fs_url):
@@ -177,7 +178,7 @@ def test_move_file_overwrite_itself(self, _, fs_url):
             tmp.writetext("file.txt", "content")
             fs.move.move_file(tmp, "file.txt", tmp, "file.txt")
             self.assertTrue(tmp.exists("file.txt"))
-            self.assertEquals(tmp.readtext("file.txt"), "content")
+            self.assertEqual(tmp.readtext("file.txt"), "content")
 
     @parameterized.expand([("temp", "temp://"), ("mem", "mem://")])
     def test_move_file_overwrite_itself_relpath(self, _, fs_url):
@@ -188,7 +189,7 @@ def test_move_file_overwrite_itself_relpath(self, _, fs_url):
             new_dir.writetext("file.txt", "content")
             fs.move.move_file(tmp, "dir/../dir/file.txt", tmp, "dir/file.txt")
             self.assertTrue(tmp.exists("dir/file.txt"))
-            self.assertEquals(tmp.readtext("dir/file.txt"), "content")
+            self.assertEqual(tmp.readtext("dir/file.txt"), "content")
 
     @parameterized.expand([(True,), (False,)])
     def test_move_file_cleanup_on_error(self, cleanup):
@@ -206,3 +207,16 @@ def test_move_file_cleanup_on_error(self, cleanup):
                     )
             self.assertTrue(src.exists("file.txt"))
             self.assertEqual(not dst.exists("target.txt"), cleanup)
+
+    @parameterized.expand([("temp", "temp://", True), ("mem", "mem://", False)])
+    def test_move_dir_optimized(self, _, fs_url, mv_called):
+        with open_fs(fs_url) as tmp:
+            with mock.patch.object(os, "rename", wraps=os.rename) as mv:
+                dir_ = tmp.makedir("dir")
+                dir_.writetext("file.txt", "content")
+                sub = dir_.makedir("sub")
+                sub.writetext("file.txt", "sub content")
+                fs.move.move_dir(tmp, "dir", tmp, "newdir")
+                self.assertEqual(tmp.readtext("newdir/file.txt"), "content")
+                self.assertEqual(tmp.readtext("newdir/sub/file.txt"), "sub content")
+                self.assertEqual(mv.called, mv_called)