From 702beccb9569982a251c5f03eec7cbf034902cfc Mon Sep 17 00:00:00 2001 From: Marcel Johannesmann Date: Wed, 28 Dec 2022 18:58:54 +0100 Subject: [PATCH 1/8] wrote failing test --- fs/test.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/fs/test.py b/fs/test.py index 32e6ea5c..f8c85739 100644 --- a/fs/test.py +++ b/fs/test.py @@ -1789,6 +1789,18 @@ def test_move_file_same_fs(self): self.assertEqual(self.fs.listdir("foo"), ["test2.txt"]) self.assertEqual(next(self.fs.scandir("foo")).name, "test2.txt") + def test_move_file_same_fs_preserve_time(self): + text = "Hello, World" + self.fs.makedir("foo").writetext("test.txt", text) + self.assert_text("foo/test.txt", text) + + fs.move.move_file(self.fs, "foo/test.txt", self.fs, "foo/test2.txt", preserve_time=True) + self.assert_not_exists("foo/test.txt") + self.assert_text("foo/test2.txt", text) + + self.assertEqual(self.fs.listdir("foo"), ["test2.txt"]) + self.assertEqual(next(self.fs.scandir("foo")).name, "test2.txt") + def _test_move_file(self, protocol): other_fs = open_fs(protocol) From f59837eff8fb1113714d91fbedbe459c29c889a0 Mon Sep 17 00:00:00 2001 From: Marcel Johannesmann Date: Wed, 28 Dec 2022 20:09:36 +0100 Subject: [PATCH 2/8] separated func into read and update parts --- fs/copy.py | 42 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/fs/copy.py b/fs/copy.py index 154fe715..22269803 100644 --- a/fs/copy.py +++ b/fs/copy.py @@ -14,7 +14,7 @@ from .walk import Walker if typing.TYPE_CHECKING: - from typing import Callable, Optional, Text, Union + from typing import Callable, Optional, Text, Union, Dict from .base import FS @@ -536,3 +536,43 @@ def copy_modified_time( if value in src_details: dst_details[value] = src_details[value] _dst_fs.setinfo(dst_path, {"details": dst_details}) + + +def read_modified_time( + src_fs, # type: Union[FS, Text] + src_path, # type: Text +): + # type: (...) -> Dict + """Read modified time metadata from a file. + + Arguments: + src_fs (FS or str): Source filesystem (instance or URL). + src_path (str): Path to a directory on the source filesystem. + + """ + namespaces = ("details",) + with manage_fs(src_fs, writeable=False) as _src_fs: + src_meta = _src_fs.getinfo(src_path, namespaces) + src_details = src_meta.raw.get("details", {}) + mod_details = {} + for value in ("metadata_changed", "modified"): + if value in src_details: + mod_details[value] = src_details[value] + return mod_details + + +def update_details_namespace( + dst_fs, # type: Union[FS, Text] + dst_path, # type: Text + details_dic, # type: Dict +): + # type: (...) -> None + """Update file metadata from a dict. + + Arguments: + dst_fs (FS or str): Destination filesystem (instance or URL). + dst_path (str): Path to a directory on the destination filesystem. + + """ + with manage_fs(dst_fs, create=True) as _dst_fs: + _dst_fs.setinfo(dst_path, {"details": details_dic}) From 49dfce72966cfcd788d45b4a7b1d370f571e643c Mon Sep 17 00:00:00 2001 From: Marcel Johannesmann Date: Wed, 28 Dec 2022 20:14:53 +0100 Subject: [PATCH 3/8] fixed FS.move to allow correct time preservation --- fs/base.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/fs/base.py b/fs/base.py index d42997d4..10e1bd65 100644 --- a/fs/base.py +++ b/fs/base.py @@ -22,7 +22,7 @@ from functools import partial, wraps from . import copy, errors, fsencode, glob, iotools, tools, walk, wildcard -from .copy import copy_modified_time +from .copy import copy_modified_time, read_modified_time, update_details_namespace from .glob import BoundGlobber from .mode import validate_open_mode from .path import abspath, isbase, join, normpath @@ -1187,14 +1187,27 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False): except errors.NoSysPath: # pragma: no cover pass else: + # Without preserving the modification time we can rename + # the file's path. + if not preserve_time: + try: + os.rename(src_sys_path, dst_sys_path) + except OSError: + pass + else: + return + + # read the modification before moving the file + modification_details = read_modified_time(self, _src_path) try: os.rename(src_sys_path, dst_sys_path) except OSError: pass else: - if preserve_time: - copy_modified_time(self, _src_path, self, _dst_path) + # update the meta info + update_details_namespace(self, _dst_path, modification_details) return + with self._lock: with self.open(_src_path, "rb") as read_file: # FIXME(@althonos): typing complains because open return IO From 5f1f1e6a5ed015c550ae27dc8524276f30637f5f Mon Sep 17 00:00:00 2001 From: Marcel Johannesmann Date: Wed, 28 Dec 2022 20:15:12 +0100 Subject: [PATCH 4/8] fixed MemoryFS.move to allow time preservation --- fs/memoryfs.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/fs/memoryfs.py b/fs/memoryfs.py index 0ca5ce16..4076a768 100644 --- a/fs/memoryfs.py +++ b/fs/memoryfs.py @@ -15,7 +15,7 @@ from . import errors from ._typing import overload from .base import FS -from .copy import copy_modified_time +from .copy import copy_modified_time, read_modified_time, update_details_namespace from .enums import ResourceType, Seek from .info import Info from .mode import Mode @@ -468,14 +468,19 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False): return raise errors.DestinationExists(dst_path) + # get the time info to preserve it (see #558) + if preserve_time: + modification_details = read_modified_time(self, src_path) + # move the entry from the src folder to the dst folder dst_dir_entry.set_entry(dst_name, src_entry) src_dir_entry.remove_entry(src_name) # make sure to update the entry name itself (see #509) src_entry.name = dst_name + # update the meta info, after moving if preserve_time: - copy_modified_time(self, src_path, self, dst_path) + update_details_namespace(self, dst_path, modification_details) def movedir(self, src_path, dst_path, create=False, preserve_time=False): _src_path = self.validatepath(src_path) From 50f72ba4771d4b9b14da121e7adc977112f68f62 Mon Sep 17 00:00:00 2001 From: Marcel Johannesmann Date: Thu, 29 Dec 2022 22:19:09 +0100 Subject: [PATCH 5/8] fixed MemoryFS.move to allow time preservation --- fs/memoryfs.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/fs/memoryfs.py b/fs/memoryfs.py index 0ca5ce16..4076a768 100644 --- a/fs/memoryfs.py +++ b/fs/memoryfs.py @@ -15,7 +15,7 @@ from . import errors from ._typing import overload from .base import FS -from .copy import copy_modified_time +from .copy import copy_modified_time, read_modified_time, update_details_namespace from .enums import ResourceType, Seek from .info import Info from .mode import Mode @@ -468,14 +468,19 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False): return raise errors.DestinationExists(dst_path) + # get the time info to preserve it (see #558) + if preserve_time: + modification_details = read_modified_time(self, src_path) + # move the entry from the src folder to the dst folder dst_dir_entry.set_entry(dst_name, src_entry) src_dir_entry.remove_entry(src_name) # make sure to update the entry name itself (see #509) src_entry.name = dst_name + # update the meta info, after moving if preserve_time: - copy_modified_time(self, src_path, self, dst_path) + update_details_namespace(self, dst_path, modification_details) def movedir(self, src_path, dst_path, create=False, preserve_time=False): _src_path = self.validatepath(src_path) From 22f012b3c364a58abc5a494ca029648a3ea18e61 Mon Sep 17 00:00:00 2001 From: Marcel Johannesmann Date: Thu, 29 Dec 2022 22:53:11 +0100 Subject: [PATCH 6/8] simplified FS.move, renamed update func --- fs/base.py | 24 +++++++++--------------- fs/copy.py | 2 +- fs/memoryfs.py | 4 ++-- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/fs/base.py b/fs/base.py index 10e1bd65..e0b7444e 100644 --- a/fs/base.py +++ b/fs/base.py @@ -22,7 +22,7 @@ from functools import partial, wraps from . import copy, errors, fsencode, glob, iotools, tools, walk, wildcard -from .copy import copy_modified_time, read_modified_time, update_details_namespace +from .copy import copy_modified_time, read_modified_time, update_details_info from .glob import BoundGlobber from .mode import validate_open_mode from .path import abspath, isbase, join, normpath @@ -1187,25 +1187,19 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False): except errors.NoSysPath: # pragma: no cover pass else: - # Without preserving the modification time we can rename - # the file's path. - if not preserve_time: - try: - os.rename(src_sys_path, dst_sys_path) - except OSError: - pass - else: - return - - # read the modification before moving the file - modification_details = read_modified_time(self, _src_path) + # get the time info to preserve it (see #558) + if preserve_time: + modification_details = read_modified_time(self, _src_path) + try: os.rename(src_sys_path, dst_sys_path) except OSError: pass else: - # update the meta info - update_details_namespace(self, _dst_path, modification_details) + # update the meta info, after moving + if preserve_time: + update_details_info(self, _dst_path, modification_details) + return with self._lock: diff --git a/fs/copy.py b/fs/copy.py index 22269803..d916010a 100644 --- a/fs/copy.py +++ b/fs/copy.py @@ -561,7 +561,7 @@ def read_modified_time( return mod_details -def update_details_namespace( +def update_details_info( dst_fs, # type: Union[FS, Text] dst_path, # type: Text details_dic, # type: Dict diff --git a/fs/memoryfs.py b/fs/memoryfs.py index 4076a768..16c0175f 100644 --- a/fs/memoryfs.py +++ b/fs/memoryfs.py @@ -15,7 +15,7 @@ from . import errors from ._typing import overload from .base import FS -from .copy import copy_modified_time, read_modified_time, update_details_namespace +from .copy import copy_modified_time, read_modified_time, update_details_info from .enums import ResourceType, Seek from .info import Info from .mode import Mode @@ -480,7 +480,7 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False): # update the meta info, after moving if preserve_time: - update_details_namespace(self, dst_path, modification_details) + update_details_info(self, dst_path, modification_details) def movedir(self, src_path, dst_path, create=False, preserve_time=False): _src_path = self.validatepath(src_path) From b5b2c66fd51d80264d74d43f5b7b9e0acbd7a529 Mon Sep 17 00:00:00 2001 From: Marcel Johannesmann Date: Thu, 29 Dec 2022 22:54:48 +0100 Subject: [PATCH 7/8] formatted repo with black --- fs/test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/test.py b/fs/test.py index f8c85739..4af1917c 100644 --- a/fs/test.py +++ b/fs/test.py @@ -1794,7 +1794,9 @@ def test_move_file_same_fs_preserve_time(self): self.fs.makedir("foo").writetext("test.txt", text) self.assert_text("foo/test.txt", text) - fs.move.move_file(self.fs, "foo/test.txt", self.fs, "foo/test2.txt", preserve_time=True) + fs.move.move_file( + self.fs, "foo/test.txt", self.fs, "foo/test2.txt", preserve_time=True + ) self.assert_not_exists("foo/test.txt") self.assert_text("foo/test2.txt", text) From 3579925ad64cce5025973a84d82ab824c94eb19c Mon Sep 17 00:00:00 2001 From: Marcel Johannesmann Date: Thu, 29 Dec 2022 23:40:14 +0100 Subject: [PATCH 8/8] updated changelog and contributors --- CHANGELOG.md | 1 + CONTRIBUTORS.md | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef16734e..b20ebafe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ([#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) +- Fixed a bug in `FS.move` and `MemoryFS.move`, where `peserve_time=True` resulted in an `ResourceNotFound` error. Closes [#558](https://github.com/PyFilesystem/pyfilesystem2/issues/558) ## [2.4.16] - 2022-05-02 diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 78102487..39c83669 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -25,6 +25,7 @@ Many thanks to the following developers for contributing to this project: - [Joshua Tauberer](https://github.com/JoshData) - [Justin Charlong](https://github.com/jcharlong) - [Louis Sautier](https://github.com/sbraz) +- [Marcel Johannesmann](https://github.com/mj0nez) - [Martin Durant](https://github.com/martindurant) - [Martin Larralde](https://github.com/althonos) - [Masaya Nakamura](https://github.com/mashabow)