Fixed BucketPath using backslash separators on Windows

doc/changes/unreleased.md

@@ -1,3 +1,11 @@
 # Unreleased
 
 ## Summary
+
+## Bug Fixing
+
+* #322: Fixed `BucketPath` and `MountedBucket` using backslash separators on Windows. Paths in a bucket are now always POSIX paths, so `read()`, `write()`, `rm()`, `iterdir()`, `joinpath()`, `/`, `as_udf_path()`, `MountedBucket.files` and `MountedBucket.udf_path` produce `/`-separated paths on every platform. A Windows drive or UNC share in a path is dropped.
+
+## Breaking Changes
+
+* #322: On Windows, `BucketPath.relative_to()` now returns a `PurePosixPath` instead of a `PureWindowsPath`, so it no longer compares equal to a `PurePath` built from the same string on Windows.

exasol/bucketfs/_buckets.py

@@ -434,11 +434,11 @@ def name(self) -> str:
 
     @property
     def udf_path(self) -> str:
-        return str(self.root)
+        return self.root.as_posix()
 
     @property
     def files(self) -> list[str]:
-        return [str(pth.relative_to(self.root)) for pth in self.root.rglob("*.*")]
+        return [pth.relative_to(self.root).as_posix() for pth in self.root.rglob("*.*")]
 
     def delete(self, path: str) -> None:
         try:

exasol/bucketfs/_path.py

@@ -14,6 +14,7 @@
 )
 from pathlib import (
     PurePath,
+    PurePosixPath,
     PureWindowsPath,
 )
 from typing import (
@@ -215,6 +216,20 @@ def __truediv__(self, other):
         """
 
 
+def _to_posix_path(path: str | PurePath) -> PurePosixPath:
+    """
+    Convert a path to the POSIX form used inside a bucket. A Windows drive or UNC share
+    has no meaning in a bucket and is dropped; the separators become '/' and a leading
+    root is kept, like it is for a POSIX path.
+    """
+    if not isinstance(path, PurePath):
+        path = PurePath(path)
+    if isinstance(path, PureWindowsPath):
+        parts = path.parts[1:] if path.anchor else path.parts
+        return PurePosixPath("/" if path.root else "", *parts)
+    return PurePosixPath(path)
+
+
 def _remove_archive_suffix(path: PurePath) -> PurePath:
     while path.suffix in ARCHIVE_SUFFIXES:
         path = path.with_suffix("")
@@ -277,15 +292,17 @@ def __init__(self, path: str | PurePath, bucket_api: BucketLike):
         """
         :param path:        A pure path of a file or directory. The path is assumed to
                             be relative to the bucket. It is also permissible to have
-                            this path in an absolute form, e.g. '/dir1/...'
-                            or '\\\\abc\\...\\'.
+                            this path in an absolute form, e.g. '/dir1/...'.
+                            A Windows path object, or a string on Windows, is converted
+                            to a POSIX path: a drive or UNC share is dropped and the
+                            separators become '/'.
 
                             All Pure Path methods of the PathLike protocol will be
                             delegated to this object.
 
         :param bucket_api:  An object supporting the Bucket API protocol.
         """
-        self._path = PurePath(path)
+        self._path = _to_posix_path(path)
         self._bucket_api = bucket_api
 
     @property
@@ -302,8 +319,6 @@ def _get_relative_posix(self) -> str:
         file: 'dir/subdir/.../filename'.
         """
         path_str = str(self._path)[len(self._path.anchor) :]
-        if isinstance(self._path, PureWindowsPath):
-            path_str = path_str.replace("\\", "/")
         if path_str == ".":
             path_str = ""
         return path_str
@@ -369,7 +384,8 @@ def as_uri(self) -> str:
 
     def as_udf_path(self) -> str:
         return str(
-            PurePath(self._bucket_api.udf_path) / _remove_archive_suffix(self._path)
+            PurePosixPath(self._bucket_api.udf_path)
+            / _remove_archive_suffix(self._path)
         )
 
     def exists(self) -> bool:
@@ -433,7 +449,8 @@ def joinpath(self, *path_segments) -> PathLike:
         # The path segments can be of either this type or an os.PathLike.
         cls = type(self)
         seg_paths = [
-            seg._path if isinstance(seg, cls) else seg for seg in path_segments
+            seg._path if isinstance(seg, cls) else _to_posix_path(seg)
+            for seg in path_segments
         ]
         new_path = self._path.joinpath(*seg_paths)
         return cls(new_path, self._bucket_api)
@@ -491,7 +508,9 @@ def iterdir(self) -> Generator[PathLike]:
     def __truediv__(self, other):
         # The other object can be of either this type or an os.PathLike.
         cls = type(self)
-        new_path = self._path / (other._path if isinstance(other, cls) else other)
+        new_path = self._path / (
+            other._path if isinstance(other, cls) else _to_posix_path(other)
+        )
         return cls(new_path, self._bucket_api)
 
     def __eq__(self, other) -> bool:

test/unit/test_bucket_path.py

@@ -1,7 +1,7 @@
 from itertools import chain
 from pathlib import (
     Path,
-    PurePath,
+    PurePosixPath,
 )
 
 import pytest
@@ -280,4 +280,4 @@ def test_foreign_bucket(tmp_path, bucket_fake):
 def test_relative_to(path, expected_relative, bucket_fake):
     root = bfs.path.BucketPath("root", bucket_api=bucket_fake)
     testee = root / path
-    assert testee.relative_to(root) == PurePath(expected_relative)
+    assert testee.relative_to(root) == PurePosixPath(expected_relative)

test/unit/test_bucket_path_separators.py

@@ -0,0 +1,119 @@
+import os
+from pathlib import PureWindowsPath
+
+import pytest
+
+import exasol.bucketfs as bfs
+from exasol.bucketfs._buckets import MountedBucket
+
+
+class RecordingBucketApi:
+    udf_path = "/buckets/bfsdefault/default"
+
+    def __init__(self, files):
+        self.files = files
+        self.calls = []
+
+    def upload(self, path, data):
+        self.calls.append(("upload", path))
+
+    def download(self, path, chunk_size):
+        self.calls.append(("download", path))
+        return iter([])
+
+    def delete(self, path):
+        self.calls.append(("delete", path))
+
+
+@pytest.fixture
+def bucket_api():
+    return RecordingBucketApi(files=["dir/sub/a.txt"])
+
+
+def test_windows_path_object_is_stored_with_posix_separators(bucket_api):
+    path = bfs.path.BucketPath(PureWindowsPath("dir", "sub", "a.txt"), bucket_api)
+    assert str(path) == "dir/sub/a.txt"
+
+
+def test_read_write_and_rm_send_posix_paths(bucket_api):
+    path = bfs.path.BucketPath(PureWindowsPath("dir", "sub", "a.txt"), bucket_api)
+    path.write(b"x")
+    list(path.read())
+    path.rm()
+    assert bucket_api.calls == [
+        ("upload", "dir/sub/a.txt"),
+        ("download", "dir/sub/a.txt"),
+        ("delete", "dir/sub/a.txt"),
+    ]
+
+
+def test_iterdir_yields_posix_paths(bucket_api):
+    root = bfs.path.BucketPath(PureWindowsPath("dir"), bucket_api)
+    assert [str(child) for child in root.iterdir()] == ["dir/sub"]
+
+
+def test_as_udf_path_uses_posix_separators(bucket_api):
+    path = bfs.path.BucketPath(PureWindowsPath("dir", "sub", "a.txt"), bucket_api)
+    assert path.as_udf_path() == "/buckets/bfsdefault/default/dir/sub/a.txt"
+
+
+def test_windows_path_object_is_found_in_the_bucket_listing(bucket_api):
+    path = bfs.path.BucketPath(PureWindowsPath("dir", "sub", "a.txt"), bucket_api)
+    assert path.exists()
+
+
+def test_string_built_with_the_native_separator_is_found(bucket_api):
+    # on Windows this string contains backslashes
+    path = bfs.path.BucketPath(os.path.join("dir", "sub", "a.txt"), bucket_api)
+    assert str(path) == "dir/sub/a.txt"
+    assert path.exists()
+    assert path.name == "a.txt"
+    assert str(path.parent) == "dir/sub"
+
+
+def test_drive_letter_of_a_windows_path_is_dropped(bucket_api):
+    path = bfs.path.BucketPath(PureWindowsPath("C:/dir/sub/a.txt"), bucket_api)
+    assert str(path) == "/dir/sub/a.txt"
+    assert path.exists()
+
+
+def test_unc_share_of_a_windows_path_is_dropped(bucket_api):
+    path = bfs.path.BucketPath(PureWindowsPath("//server/share/dir/sub"), bucket_api)
+    assert str(path) == "/dir/sub"
+    assert path.exists()
+
+
+def test_joinpath_with_windows_path_objects(bucket_api):
+    root = bfs.path.BucketPath("", bucket_api)
+    path = root.joinpath(PureWindowsPath("dir", "sub"), PureWindowsPath("a.txt"))
+    assert str(path) == "dir/sub/a.txt"
+    assert path.exists()
+
+
+def test_joinpath_with_a_rooted_windows_path_object_restarts_at_the_root(bucket_api):
+    path = bfs.path.BucketPath("a", bucket_api).joinpath(PureWindowsPath("C:/x/y"))
+    assert str(path) == "/x/y"
+
+
+def test_truediv_with_windows_path_objects(bucket_api):
+    root = bfs.path.BucketPath("dir", bucket_api)
+    assert str(root / PureWindowsPath("sub", "a.txt")) == "dir/sub/a.txt"
+    assert str(root / PureWindowsPath("C:/x/y")) == "/x/y"
+
+
+def test_joinpath_with_string_uses_posix_separators(bucket_api):
+    root = bfs.path.BucketPath("", bucket_api)
+    path = root.joinpath("dir/sub/a.txt")
+    path.write(b"x")
+    assert bucket_api.calls == [("upload", "dir/sub/a.txt")]
+
+
+def test_mounted_bucket_lists_posix_files_and_udf_path(tmp_path):
+    (tmp_path / "dir" / "sub").mkdir(parents=True)
+    (tmp_path / "dir" / "sub" / "a.txt").write_bytes(b"x")
+    bucket = MountedBucket(base_path=tmp_path)
+    assert list(bucket.files) == ["dir/sub/a.txt"]
+    assert bucket.udf_path == tmp_path.as_posix()
+    path = bfs.path.BucketPath("dir/sub/a.txt", bucket)
+    assert path.exists()
+    assert path.as_udf_path() == f"{tmp_path.as_posix()}/dir/sub/a.txt"