Transport: Bug fix in rename method (#6735)

`rename()` method gets two arguments as input, `old_path` and `new_path`. Previously, it was raising an `OSError` in case `new_path` wouldn't exist before renaming.

This bug is fixed in both `ssh.py` and `local.py`. Additional test has been added to `test_all_plugins.py` to verify the correct behaviour.

Author: ayushjariyal

src/aiida/transports/plugins/local.py

@@ -866,7 +866,7 @@ def rename(self, oldpath: TransportPath, newpath: TransportPath):
         :param str oldpath: existing name of the file or folder
         :param str newpath: new name for the file or folder
 
-        :raises OSError: if src/dst is not found
+        :raises OSError: if oldpath is not found or newpath already exists
         :raises ValueError: if src/dst is not a valid string
         """
         oldpath = str(oldpath)
@@ -877,8 +877,8 @@ def rename(self, oldpath: TransportPath, newpath: TransportPath):
             raise ValueError(f'Destination {newpath} is not a valid string')
         if not os.path.exists(oldpath):
             raise OSError(f'Source {oldpath} does not exist')
-        if not os.path.exists(newpath):
-            raise OSError(f'Destination {newpath} does not exist')
+        if os.path.exists(newpath):
+            raise OSError(f'Destination {newpath} already exists.')
 
         shutil.move(oldpath, newpath)
 

src/aiida/transports/plugins/ssh.py

@@ -1357,8 +1357,8 @@ def rename(self, oldpath: TransportPath, newpath: TransportPath):
         :param str oldpath: existing name of the file or folder
         :param str newpath: new name for the file or folder
 
-        :raises OSError: if oldpath/newpath is not found
-        :raises ValueError: if sroldpathc/newpath is not a valid path
+        :raises OSError: if oldpath is not found or newpath already exists
+        :raises ValueError: if oldpath/newpath is not a valid path
         """
         if not oldpath:
             raise ValueError(f'Source {oldpath} is not a valid path')
@@ -1371,13 +1371,9 @@ def rename(self, oldpath: TransportPath, newpath: TransportPath):
         if not self.isfile(oldpath):
             if not self.isdir(oldpath):
                 raise OSError(f'Source {oldpath} does not exist')
-        # TODO: this seems to be a bug (?)
-        # why to raise an OSError if the newpath does not exist?
-        # ofcourse newpath shouldn't exist, that's why we are renaming it!
-        # issue opened here: https://github.com/aiidateam/aiida-core/issues/6725
-        if not self.isfile(newpath):
-            if not self.isdir(newpath):
-                raise OSError(f'Destination {newpath} does not exist')
+
+        if self.path_exists(newpath):
+            raise OSError(f'Destination {newpath} already exist')
 
         return self.sftp.rename(oldpath, newpath)
 

tests/transports/test_all_plugins.py

@@ -1232,3 +1232,28 @@ def test_asynchronous_execution(custom_transport, tmp_path):
             except ProcessLookupError:
                 # If the process is already dead (or has never run), I just ignore the error
                 pass
+
+
+def test_rename(custom_transport, tmp_path_remote):
+    """Test the rename function of the transport plugin."""
+    with custom_transport as transport:
+        old_file = tmp_path_remote / 'oldfile.txt'
+        new_file = tmp_path_remote / 'newfile.txt'
+        another_file = tmp_path_remote / 'anotherfile.txt'
+
+        # Create a test file to rename
+        old_file.touch()
+        another_file.touch()
+        assert old_file.exists()
+        assert another_file.exists()
+
+        # Perform rename operation
+        transport.rename(old_file, new_file)
+
+        # Verify rename was successful
+        assert not old_file.exists()
+        assert new_file.exists()
+
+        # Perform rename operation if new file already exists
+        with pytest.raises(OSError, match='already exist|destination exists'):
+            transport.rename(new_file, another_file)