Fix preserving modification times on same-filesystem moves

CHANGELOG.md

@@ -15,6 +15,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   Closes [#459](https://github.com/PyFilesystem/pyfilesystem2/issues/459).
 
 ### Fixed
+- Preserve modification times when moving files within the same filesystem or
+  directories within `MemoryFS` without reading the old path after the move.
+  Closes [#558](https://github.com/PyFilesystem/pyfilesystem2/issues/558).
 - 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`

CONTRIBUTORS.md

@@ -20,6 +20,7 @@ Many thanks to the following developers for contributing to this project:
 - [George Macon](https://github.com/gmacon)
 - [Giampaolo Cimino](https://github.com/gpcimino)
 - [@Hoboneer](https://github.com/Hoboneer)
+- [@Hugohong258](https://github.com/Hugohong258)
 - [Jen Hagg](https://github.com/jenhagg)
 - [Joseph Atkins-Turkish](https://github.com/Spacerat)
 - [Joshua Tauberer](https://github.com/JoshData)

fs/base.py

@@ -1192,8 +1192,7 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False):
                 except OSError:
                     pass
                 else:
-                    if preserve_time:
-                        copy_modified_time(self, _src_path, self, _dst_path)
+                    # Renaming retains the file's modification time.
                     return
         with self._lock:
             with self.open(_src_path, "rb") as read_file:

fs/memoryfs.py

@@ -15,7 +15,6 @@
 from . import errors
 from ._typing import overload
 from .base import FS
-from .copy import copy_modified_time
 from .enums import ResourceType, Seek
 from .info import Info
 from .mode import Mode
@@ -472,11 +471,9 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False):
             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)
+            # The entry retains its timestamps when moved.
             src_entry.name = dst_name
 
-            if preserve_time:
-                copy_modified_time(self, src_path, self, dst_path)
-
     def movedir(self, src_path, dst_path, create=False, preserve_time=False):
         _src_path = self.validatepath(src_path)
         _dst_path = self.validatepath(dst_path)
@@ -507,11 +504,9 @@ def movedir(self, src_path, dst_path, create=False, preserve_time=False):
             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)
+            # The entry retains its timestamps when moved.
             src_entry.name = dst_name
 
-            if preserve_time:
-                copy_modified_time(self, src_path, self, dst_path)
-
     def openbin(self, path, mode="r", buffering=-1, **options):
         # type: (Text, Text, int, **Any) -> BinaryIO
         _mode = Mode(mode)

tests/test_move.py

@@ -90,6 +90,72 @@ def test_move_dir(self):
 
 
 class TestMove(unittest.TestCase):
+    @parameterized.expand(
+        [
+            ("memory_direct", "mem://", False, False),
+            ("memory_helper", "mem://", True, False),
+            ("memory_overwrite", "mem://", True, True),
+            ("os_direct", "temp://", False, False),
+            ("os_helper", "temp://", True, False),
+            ("os_overwrite", "temp://", True, True),
+        ]
+    )
+    def test_move_file_preserves_time_on_same_fs(
+        self, _, fs_url, use_helper, destination_exists
+    ):
+        with open_fs(fs_url) as filesystem:
+            filesystem.writetext("source.txt", "Source")
+            filesystem.setinfo("source.txt", {"details": {"modified": 1000000}})
+            before = filesystem.getinfo("source.txt", namespaces=["details"]).modified
+            if destination_exists:
+                filesystem.writetext("destination.txt", "Old")
+
+            if use_helper:
+                fs.move.move_file(
+                    filesystem,
+                    "source.txt",
+                    filesystem,
+                    "destination.txt",
+                    preserve_time=True,
+                )
+            else:
+                filesystem.move(
+                    "source.txt", "destination.txt", overwrite=True, preserve_time=True
+                )
+
+            self.assertFalse(filesystem.exists("source.txt"))
+            self.assertEqual(filesystem.readtext("destination.txt"), "Source")
+            after = filesystem.getinfo(
+                "destination.txt", namespaces=["details"]
+            ).modified
+            self.assertEqual(after, before)
+
+    def test_move_memory_directory_preserves_time(self):
+        with open_fs("mem://") as filesystem:
+            filesystem.makedir("source")
+            filesystem.writetext("source/file.txt", "Source")
+            filesystem.setinfo("source", {"details": {"modified": 1000000}})
+            filesystem.setinfo("source/file.txt", {"details": {"modified": 1000001}})
+            before = filesystem.getinfo("source", namespaces=["details"]).modified
+            file_before = filesystem.getinfo(
+                "source/file.txt", namespaces=["details"]
+            ).modified
+
+            filesystem.movedir("source", "destination", create=True, preserve_time=True)
+
+            self.assertFalse(filesystem.exists("source"))
+            self.assertEqual(filesystem.readtext("destination/file.txt"), "Source")
+            self.assertEqual(
+                filesystem.getinfo("destination", namespaces=["details"]).modified,
+                before,
+            )
+            self.assertEqual(
+                filesystem.getinfo(
+                    "destination/file.txt", namespaces=["details"]
+                ).modified,
+                file_before,
+            )
+
     def test_move_file_tempfs(self):
         with open_fs("temp://") as src, open_fs("temp://") as dst:
             src_dir = src.makedir("Some subfolder")