#329: Fixed a bug with leading backslash

doc/changes/unreleased.md

@@ -5,3 +5,4 @@
 ## Bugfixes
 
 * #322: Fixed `BucketPath` and `MountedBucket` using backslash separators on Windows; BucketFS paths are now always POSIX paths
+* #329: Fixed absolute `BucketPath`, e.g. `'/dir/a.txt'`, being passed to the bucket with the leading `/` in data changing functions

exasol/bucketfs/_path.py

@@ -234,6 +234,18 @@ def _remove_archive_suffix(path: PurePath) -> PurePath:
     return path
 
 
+def _to_bucket_key(path: PurePosixPath) -> str:
+    """
+    Returns the path as a string in the format of a bucket file:
+    'dir/subdir/.../filename'. The path is relative to the bucket root, even if
+    the given path is absolute. The bucket root itself is an empty string.
+    """
+    path_str = str(path)[len(path.anchor) :]
+    if path_str == ".":
+        path_str = ""
+    return path_str
+
+
 class _BucketFile:
     """
     A node in a perceived file structure of a bucket.
@@ -320,10 +332,7 @@ def _get_relative_posix(self) -> str:
         Returns the pure path of this object as a string, in the format of a bucket
         file: 'dir/subdir/.../filename'.
         """
-        path_str = str(self._path)[len(self._path.anchor) :]
-        if path_str == ".":
-            path_str = ""
-        return path_str
+        return _to_bucket_key(self._path)
 
     def _navigate(self) -> _BucketFile | None:
         """
@@ -340,7 +349,9 @@ def _navigate(self) -> _BucketFile | None:
                 or file_name == path_str
                 or (not path_str)
             ):
-                path_root = path_root or _BucketFile(self._path.name, str(self.parent))
+                path_root = path_root or _BucketFile(
+                    self._path.name, _to_bucket_key(self._path.parent)
+                )
                 node = path_root
                 for part in file_name[path_len:].split("/"):
                     if part:
@@ -387,7 +398,9 @@ def as_uri(self) -> str:
 
     def as_udf_path(self) -> str:
         udf_root = self._bucket_api.udf_path
-        relative_path = _remove_archive_suffix(self._path)
+        relative_path = _remove_archive_suffix(
+            PurePosixPath(self._get_relative_posix())
+        )
         if udf_root.startswith("/"):
             return str(PurePosixPath(udf_root) / relative_path)
         # The udf_path of a MountedBucket on Windows is a local path in the native
@@ -406,14 +419,14 @@ def is_file(self) -> bool:
         return (current_node is not None) and current_node.is_file
 
     def read(self, chunk_size: int = 8192) -> Iterable[ByteString]:
-        return self._bucket_api.download(str(self._path), chunk_size)
+        return self._bucket_api.download(self._get_relative_posix(), chunk_size)
 
     def write(self, data: ByteString | BinaryIO | Sequence[ByteString]) -> None:
         if (not isinstance(data, ByteString)) and isinstance(data, Sequence):
             if not all(isinstance(chunk, ByteString) for chunk in data):
                 raise ValueError("The file chunks must be byte strings")
             data = b"".join(data)
-        self._bucket_api.upload(str(self._path), data)
+        self._bucket_api.upload(self._get_relative_posix(), data)
 
     def rm(self) -> None:
         current_node = self._navigate()
@@ -425,7 +438,7 @@ def rm(self) -> None:
             raise IsADirectoryError(
                 errno.EISDIR, os.strerror(errno.EISDIR), str(self._path)
             )
-        self._bucket_api.delete(str(self._path))
+        self._bucket_api.delete(self._get_relative_posix())
 
     def rmdir(self, recursive: bool = False) -> None:
         current_node = self._navigate()
@@ -471,13 +484,14 @@ def walk(
             )
 
         if current_node.is_dir:
-            yield from self._walk_recursive(current_node, top_down)
+            yield from self._walk_recursive(current_node, top_down, self._path)
 
     def _walk_recursive(
-        self, node: _BucketFile, top_down: bool
+        self, node: _BucketFile, top_down: bool, path: PurePosixPath
     ) -> Generator[tuple[PathLike, list[str], list[str]]]:
-
-        bucket_path = BucketPath(node.path, self._bucket_api)
+        # The node path is relative to the bucket root. The yielded path keeps
+        # the form of this path, i.e. it is absolute if this path is absolute.
+        bucket_path = BucketPath(path, self._bucket_api)
         dir_list: list[str] = []
         file_list: list[str] = []
         for child in node:
@@ -493,7 +507,7 @@ def _walk_recursive(
             yield bucket_path, dir_list, file_list
         for child in node:
             if child.is_dir:
-                yield from self._walk_recursive(child, top_down)
+                yield from self._walk_recursive(child, top_down, path / child.name)
         if not top_down:
             yield bucket_path, dir_list, file_list
 

test/unit/test_bucket_path.py

@@ -422,3 +422,89 @@ def test_string_with_drive_is_kept():
     path = bfs.path.BucketPath("C:\\models\\m.pkl", bucket_api=api)
     path.write(b"x")
     assert api.calls == [("upload", "C:\\models\\m.pkl")]
+
+
+def test_absolute_path_is_kept():
+    api = _RecordingBucket(files=[])
+    path = bfs.path.BucketPath("/dir/a.txt", bucket_api=api)
+    assert str(path) == "/dir/a.txt"
+    assert path.root == "/"
+    assert path.as_uri() == "file:///dir/a.txt"
+
+
+def test_absolute_path_is_relative_to_bucket():
+    """
+    The leading '/' of an absolute path is not passed to the bucket API.
+    """
+    api = _RecordingBucket(files=["dir/a.txt"])
+    path = bfs.path.BucketPath("/dir/a.txt", bucket_api=api)
+    path.write(b"x")
+    list(path.read())
+    path.rm()
+    assert api.calls == [
+        ("upload", "dir/a.txt"),
+        ("download", "dir/a.txt"),
+        ("delete", "dir/a.txt"),
+    ]
+
+
+@pytest.mark.parametrize(
+    "path, expected",
+    [
+        ("/dir/a.txt", "/buckets/bfsdefault/default/dir/a.txt"),
+        (
+            "/container/my_container.tar.gz",
+            "/buckets/bfsdefault/default/container/my_container",
+        ),
+        ("/", "/buckets/bfsdefault/default"),
+        ("", "/buckets/bfsdefault/default"),
+    ],
+)
+def test_absolute_as_udf_path(path, expected):
+    api = _RecordingBucket(files=[])
+    assert bfs.path.BucketPath(path, bucket_api=api).as_udf_path() == expected
+
+
+def test_absolute_path_mounted_bucket(bucket_fake):
+    path = bfs.path.BucketPath("/dir2/file_abs.dat", bucket_api=bucket_fake)
+    path.write(b"abcd")
+    assert (bucket_fake.root / "dir2/file_abs.dat").read_bytes() == b"abcd"
+    assert path.exists()
+    assert next(iter(path.read(100))) == b"abcd"
+    path.rm()
+    assert not (bucket_fake.root / "dir2/file_abs.dat").exists()
+
+
+@pytest.mark.parametrize("path", ["dir", "/dir"])
+def test_rmdir_recursive_paths(path):
+    """
+    The paths passed to the bucket API have neither a leading '/' nor './'.
+    """
+    api = _RecordingBucket(files=["dir/a.txt", "dir/sub/b.txt", "c.txt"])
+    bfs.path.BucketPath(path, bucket_api=api).rmdir(recursive=True)
+    assert sorted(api.calls) == [
+        ("delete", "dir/a.txt"),
+        ("delete", "dir/sub/b.txt"),
+    ]
+
+
+@pytest.mark.parametrize("path", ["", "/"])
+def test_rmdir_recursive_bucket_root_paths(path):
+    api = _RecordingBucket(files=["dir/a.txt", "c.txt"])
+    bfs.path.BucketPath(path, bucket_api=api).rmdir(recursive=True)
+    assert sorted(api.calls) == [("delete", "c.txt"), ("delete", "dir/a.txt")]
+
+
+@pytest.mark.parametrize(
+    "path, expected",
+    [
+        ("dir", ["dir", "dir/sub"]),
+        ("/dir", ["/dir", "/dir/sub"]),
+        ("", [".", "dir", "dir/sub"]),
+        ("/", ["/", "/dir", "/dir/sub"]),
+    ],
+)
+def test_walk_keeps_path_form(path, expected):
+    api = _RecordingBucket(files=["dir/a.txt", "dir/sub/b.txt", "c.txt"])
+    walked = [str(p) for p, _, _ in bfs.path.BucketPath(path, bucket_api=api).walk()]
+    assert walked == expected