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)