From 59f6e4d51a1983ca28d0b667bd1d6b3284ab5c56 Mon Sep 17 00:00:00 2001 From: "M.Eng. Thomas Feldmann" Date: Fri, 19 Aug 2022 11:17:37 +0200 Subject: [PATCH] Fix backward incompatibility introduced in 2.4.16 (#542) * add default overwrite arg (fixes #535) * update changelog * add tests * fix deletion when moving file on itself * compare normalized pathes in early exit * check no longer needed * remove unused import --- CHANGELOG.md | 3 +++ fs/move.py | 7 ++++++- tests/test_move.py | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ffe9392..ef16734e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased + ### Added - Added `filter_glob` and `exclude_glob` parameters to `fs.walk.Walker`. @@ -16,6 +17,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Elaborated documentation of `filter_dirs` and `exclude_dirs` in `fs.walk.Walker`. Closes [#371](https://github.com/PyFilesystem/pyfilesystem2/issues/371). +- Fixes a backward incompatibility where `fs.move.move_file` raises `DestinationExists` + ([#535](https://github.com/PyFilesystem/pyfilesystem2/issues/535)). - Fixed a bug where files could be truncated or deleted when moved / copied onto itself. Closes [#546](https://github.com/PyFilesystem/pyfilesystem2/issues/546) diff --git a/fs/move.py b/fs/move.py index fdbe96fe..752b5816 100644 --- a/fs/move.py +++ b/fs/move.py @@ -82,7 +82,12 @@ def move_file( rel_dst = frombase(common, dst_syspath) with _src_fs.lock(), _dst_fs.lock(): with OSFS(common) as base: - base.move(rel_src, rel_dst, preserve_time=preserve_time) + base.move( + rel_src, + rel_dst, + overwrite=True, + preserve_time=preserve_time, + ) return # optimization worked, exit early except ValueError: # This is raised if we cannot find a common base folder. diff --git a/tests/test_move.py b/tests/test_move.py index 5401082e..8eb1af75 100644 --- a/tests/test_move.py +++ b/tests/test_move.py @@ -151,6 +151,45 @@ def test_move_file_read_only_mem_dest(self): dst_ro.exists("target.txt"), "file should not have been copied over" ) + @parameterized.expand([("temp", "temp://"), ("mem", "mem://")]) + def test_move_file_overwrite(self, _, fs_url): + # we use TempFS and MemoryFS in order to make sure the optimized code path + # behaves like the regular one (TempFS tests the optmized code path). + with open_fs(fs_url) as src, open_fs(fs_url) as dst: + src.writetext("file.txt", "source content") + dst.writetext("target.txt", "target content") + self.assertTrue(src.exists("file.txt")) + self.assertFalse(src.exists("target.txt")) + self.assertFalse(dst.exists("file.txt")) + self.assertTrue(dst.exists("target.txt")) + fs.move.move_file(src, "file.txt", dst, "target.txt") + self.assertFalse(src.exists("file.txt")) + 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") + + @parameterized.expand([("temp", "temp://"), ("mem", "mem://")]) + def test_move_file_overwrite_itself(self, _, fs_url): + # we use TempFS and MemoryFS in order to make sure the optimized code path + # 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") + self.assertTrue(tmp.exists("file.txt")) + self.assertEquals(tmp.readtext("file.txt"), "content") + + @parameterized.expand([("temp", "temp://"), ("mem", "mem://")]) + def test_move_file_overwrite_itself_relpath(self, _, fs_url): + # we use TempFS and MemoryFS in order to make sure the optimized code path + # behaves like the regular one (TempFS tests the optmized code path). + 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") + self.assertTrue(tmp.exists("dir/file.txt")) + self.assertEquals(tmp.readtext("dir/file.txt"), "content") + @parameterized.expand([(True,), (False,)]) def test_move_file_cleanup_on_error(self, cleanup): with open_fs("mem://") as src, open_fs("mem://") as dst: