fix(delete): unify semantics of delete-operations

This commit unifies the semantic of
`SshUrlOperations.delete` and `FileUrlOperations.delete`.
Both will now remove non-empty directories, as
well as write protected files.

They will not remove a target from a
write protected directory.

datalad_next/url_operations/file.py

@@ -5,6 +5,7 @@
 
 import logging
 import random
+import stat
 import sys
 import time
 from io import IOBase
@@ -207,8 +208,13 @@ def delete(self,
                timeout: float | None = None) -> Dict:
         """Delete the target of a file:// URL
 
-        The target can be a file or a directory. If it is a directory, it has
-        to be empty.
+        The target can be a file or a directory. `delete` will attempt to
+        delete write protected targets (by setting write permissions). If
+        the target is a directory, the complete directory and all its
+        content will be deleted. `delete` will not modify the permissions
+        of the parent of the target. That means, it will not delete a target
+        in a write protected directory, but it will empty target, if target is
+        a directory.
 
         See :meth:`datalad_next.url_operations.UrlOperations.delete`
         for parameter documentation and exception behavior.
@@ -220,12 +226,13 @@ def delete(self,
         """
         path = self._file_url_to_path(url)
         try:
+            path.chmod(stat.S_IWUSR)
             path.unlink()
         except FileNotFoundError as e:
             raise UrlOperationsResourceUnknown(url) from e
         except IsADirectoryError:
             try:
-                path.rmdir()
+                self._delete_dir(path)
             except Exception as e:
                 raise UrlOperationsRemoteError(url, message=str(e)) from e
         except Exception as e:
@@ -234,6 +241,16 @@ def delete(self,
             raise UrlOperationsRemoteError(url, message=str(e)) from e
         return {}
 
+    def _delete_dir(self, path: Path):
+        path.chmod(stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)
+        for sub_path in path.iterdir():
+            if sub_path.is_dir():
+                self._delete_dir(sub_path)
+            else:
+                sub_path.chmod(stat.S_IWUSR)
+                sub_path.unlink()
+        path.rmdir()
+
     def _copyfp(self,
                 src_fp: IOBase | BinaryIO,
                 dst_fp: IOBase | BinaryIO,

datalad_next/url_operations/ssh.py

@@ -152,7 +152,10 @@ def delete(self,
         The target can be a file or a directory. `delete` will attempt to
         delete write protected targets (by setting write permissions). If
         the target is a directory, the complete directory and all its
-        content will be deleted.
+        content will be deleted. `delete` will not modify the permissions
+        of the parent of the target. That means, it will not delete a target
+        in a write protected directory, but it will empty target, if target is
+        a directory.
 
         See :meth:`datalad_next.url_operations.UrlOperations.delete`
         for parameter documentation and exception behavior.

datalad_next/url_operations/tests/test_file.py

@@ -1,9 +1,10 @@
 import io
+import stat
+
 import pytest
 import sys
 
 from datalad_next.tests import skip_if_on_windows
-from datalad_next.utils import on_linux
 
 from ..file import (
     FileUrlOperations,
@@ -70,36 +71,44 @@ def test_file_url_upload(tmp_path, monkeypatch):
 
     # TODO test missing write permissions
 
+
 def test_file_url_delete(tmp_path):
     payload = 'payload'
-    test_path = tmp_path / 'subdir' / 'myfile'
-    test_path.parent.mkdir()
+    test_dir_path = tmp_path / 'subdir'
+    test_path = test_dir_path / 'myfile'
+
+    test_dir_path.mkdir()
     test_url = test_path.as_uri()
+    test_dir_url = test_dir_path.as_uri()
     ops = FileUrlOperations()
+
     # missing file
     with pytest.raises(UrlOperationsResourceUnknown):
         ops.delete(test_url)
 
-    # place file
+    # place file and write protect
     test_path.write_text(payload)
     assert test_path.read_text() == payload
-    # try deleting a non-empty dir
-    with pytest.raises(UrlOperationsRemoteError):
-        ops.delete(test_path.parent.as_uri())
-
+    test_path.chmod(stat.S_IRUSR)
     # file deletion works
     ops.delete(test_url)
     assert not test_path.exists()
 
-    # both windows and mac give incomprehensible AccessDenied
-    # errors on appveyor, although the directory is confirmed
-    # to be empty
-    if on_linux:
-        # empty dir deletion works too
-        # confirm it is indeed empty
-        assert not list(test_path.parent.iterdir())
-        ops.delete(test_path.parent.as_uri())
-        assert not test_path.parent.exists()
+    # place file again protect directory and file
+    test_path.write_text(payload)
+    assert test_path.read_text() == payload
+    test_dir_path.chmod(stat.S_IXUSR | stat.S_IRUSR)
+    test_path.chmod(stat.S_IRUSR)
+    # non-empty directory deletion works
+    ops.delete(test_dir_url)
+    assert not test_dir_path.exists()
+
+    # create empty, write-protected dir
+    assert not test_dir_path.exists()
+    test_dir_path.mkdir()
+    test_dir_path.chmod(stat.S_IXUSR | stat.S_IRUSR)
+    ops.delete(test_dir_url)
+    assert not test_dir_path.exists()
 
 
 @skip_if_on_windows

datalad_next/url_operations/tests/test_ssh.py

@@ -149,6 +149,7 @@ def test_ssh_stat(sshserver):
         ops.stat(test_url)
 
 
+@skip_if_on_windows
 def test_ssh_delete(sshserver):
     ssh_url, ssh_local_path = sshserver