Move dir optimization

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,16 @@
 
 import typing
 
+import os
+import shutil
+
 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
@@ -134,9 +138,37 @@ def move_dir(
         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 `shutil.move` 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", dst_path, directory=True):
+                        if _dst_fs.exists(dst_path):
+                            os.rmdir(dst_syspath)
+                        shutil.move(src_syspath, dst_syspath)
+                        # recreate the root dir if it has been renamed
+                        _src_fs.makedir("/", recreate=True)
+                    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 shutil
 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(shutil, "move", wraps=shutil.move) 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)