diff --git a/CHANGELOG.md b/CHANGELOG.md index ef16734e..3823095c 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/fs/move.py b/fs/move.py index 752b5816..0d776c02 100644 --- a/fs/move.py +++ b/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( diff --git a/tests/test_move.py b/tests/test_move.py index 8eb1af75..736ade1b 100644 --- a/tests/test_move.py +++ b/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)