diff --git a/fs/base.py b/fs/base.py index d42997d4..d222831f 100644 --- a/fs/base.py +++ b/fs/base.py @@ -1091,6 +1091,8 @@ def movedir(self, src_path, dst_path, create=False, preserve_time=False): and ``create`` is `False`. fs.errors.DirectoryExpected: if ``src_path`` or one of its ancestors is not a directory. + fs.errors.IllegalDestination: when attempting to move the + folder at ``src_path`` to itself or one of its subfolders. """ from .move import move_dir @@ -1098,8 +1100,6 @@ def movedir(self, src_path, dst_path, create=False, preserve_time=False): with self._lock: _src_path = self.validatepath(src_path) _dst_path = self.validatepath(dst_path) - if _src_path == _dst_path: - return if isbase(_src_path, _dst_path): raise errors.IllegalDestination(dst_path) if not create and not self.exists(dst_path): @@ -1169,6 +1169,8 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False): and ``overwrite`` is `False`. fs.errors.ResourceNotFound: If a parent directory of ``dst_path`` does not exist. + fs.errors.IllegalDestination: If ``src_path`` and ``dst_path`` + refer to the same resource, and ``overwrite`` is `True`. """ _src_path = self.validatepath(src_path) @@ -1179,7 +1181,7 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False): raise errors.FileExpected(src_path) if _src_path == _dst_path: # early exit when moving a file onto itself - return + raise errors.IllegalDestination(dst_path) if self.getmeta().get("supports_rename", False): try: src_sys_path = self.getsyspath(_src_path) diff --git a/fs/memoryfs.py b/fs/memoryfs.py index 0ca5ce16..462cc485 100644 --- a/fs/memoryfs.py +++ b/fs/memoryfs.py @@ -465,8 +465,9 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False): # handle moving a file onto itself if src_dir == dst_dir and src_name == dst_name: if overwrite: - return - raise errors.DestinationExists(dst_path) + raise errors.IllegalDestination(dst_path) + else: + raise errors.DestinationExists(dst_path) # move the entry from the src folder to the dst folder dst_dir_entry.set_entry(dst_name, src_entry) @@ -483,10 +484,7 @@ def movedir(self, src_path, dst_path, create=False, preserve_time=False): dst_dir, dst_name = split(_dst_path) src_dir, src_name = split(_src_path) - # move a dir onto itself - if _src_path == _dst_path: - return - # move a dir into itself + # move a dir onto or into itself if isbase(_src_path, _dst_path): raise errors.IllegalDestination(dst_path) diff --git a/fs/test.py b/fs/test.py index 32e6ea5c..16b3b178 100644 --- a/fs/test.py +++ b/fs/test.py @@ -1813,20 +1813,20 @@ def test_move_file_temp(self): def test_move_file_onto_itself(self): self.fs.writetext("file.txt", "Hello") - self.fs.move("file.txt", "file.txt", overwrite=True) - self.assert_text("file.txt", "Hello") - + with self.assertRaises(errors.IllegalDestination): + self.fs.move("file.txt", "file.txt", overwrite=True) with self.assertRaises(errors.DestinationExists): self.fs.move("file.txt", "file.txt", overwrite=False) + self.assert_text("file.txt", "Hello") def test_move_file_onto_itself_relpath(self): subdir = self.fs.makedir("sub") subdir.writetext("file.txt", "Hello") - self.fs.move("sub/file.txt", "sub/../sub/file.txt", overwrite=True) - self.assert_text("sub/file.txt", "Hello") - + with self.assertRaises(errors.IllegalDestination): + self.fs.move("sub/file.txt", "sub/../sub/file.txt", overwrite=True) with self.assertRaises(errors.DestinationExists): self.fs.move("sub/file.txt", "sub/../sub/file.txt", overwrite=False) + self.assert_text("sub/file.txt", "Hello") def test_copy_file_onto_itself(self): self.fs.writetext("file.txt", "Hello") @@ -1911,8 +1911,8 @@ def test_movedir_onto_itself(self): folder.writetext("file1.txt", "Hello1") sub = folder.makedir("sub") sub.writetext("file2.txt", "Hello2") - - self.fs.movedir("folder", "folder") + with self.assertRaises(errors.IllegalDestination): + self.fs.movedir("folder", "folder") self.assert_text("folder/file1.txt", "Hello1") self.assert_text("folder/sub/file2.txt", "Hello2") diff --git a/tests/test_move.py b/tests/test_move.py index 8eb1af75..b2f4a389 100644 --- a/tests/test_move.py +++ b/tests/test_move.py @@ -11,7 +11,7 @@ import fs.move from fs import open_fs -from fs.errors import FSError, ResourceReadOnly +from fs.errors import FSError, ResourceReadOnly, IllegalDestination from fs.path import join from fs.wrap import read_only @@ -175,7 +175,8 @@ def test_move_file_overwrite_itself(self, _, fs_url): # behaves like the regular one (TempFS tests the optmized code path). with open_fs(fs_url) as tmp: tmp.writetext("file.txt", "content") - fs.move.move_file(tmp, "file.txt", tmp, "file.txt") + with self.assertRaises(IllegalDestination): + fs.move.move_file(tmp, "file.txt", tmp, "file.txt") self.assertTrue(tmp.exists("file.txt")) self.assertEquals(tmp.readtext("file.txt"), "content") @@ -186,7 +187,8 @@ def test_move_file_overwrite_itself_relpath(self, _, fs_url): with open_fs(fs_url) as tmp: new_dir = tmp.makedir("dir") new_dir.writetext("file.txt", "content") - fs.move.move_file(tmp, "dir/../dir/file.txt", tmp, "dir/file.txt") + with self.assertRaises(IllegalDestination): + 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")