#322: Fixed a bug with Windows path format

doc/changes/unreleased.md

@@ -1,3 +1,7 @@
 # Unreleased
 
 ## Summary
+
+## Bugfixes
+
+* #322: Fixed `BucketPath` and `MountedBucket` using backslash separators on Windows; BucketFS paths are now always POSIX paths

doc/user_guide/basics.rst

@@ -89,6 +89,15 @@ PathLike interface
 ------------------
 A PathLike is an interface similar to the pathlib.Path and should feel familiar to most users.
 
+.. note::
+
+   BucketFS paths are always POSIX paths, regardless of the client OS.
+   A path given as a string must use ``/`` as separator. A backslash is treated
+   as a regular character of a file name, e.g. ``my_model\v1.pkl`` is a single
+   file. On Windows, either pass a ``PurePath`` or ``PureWindowsPath`` object,
+   which is converted to a POSIX path, or convert a string first, e.g.
+   ``PureWindowsPath(s).as_posix()``.
+
 Using the PathLike interface
 ++++++++++++++++++++++++++++
 

exasol/bucketfs/_buckets.py

@@ -438,7 +438,7 @@ def udf_path(self) -> str:
 
     @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 (
@@ -85,7 +86,7 @@ def relative_to(self, other: PathLike) -> PurePath:
         Example:
         a = build_path(..., path="parent")
         b = build_path(..., path="parent/child")
-        assert b.relative(a) == PurePath("child")
+        assert b.relative_to(a) == PurePath("child")
         """
 
     def as_uri(self) -> str:
@@ -215,6 +216,18 @@ def __truediv__(self, other):
         """
 
 
+def _to_posix(path):
+    """
+    Converts a PureWindowsPath to a POSIX path string, e.g.
+    'C:\\dir\\a.txt' -> 'C:/dir/a.txt'. Leaves other paths as they are,
+    including strings. Constructing a PurePosixPath directly from an absolute
+    PureWindowsPath gives this result only in Python >= 3.12.
+    """
+    if isinstance(path, PureWindowsPath):
+        return path.as_posix()
+    return path
+
+
 def _remove_archive_suffix(path: PurePath) -> PurePath:
     while path.suffix in ARCHIVE_SUFFIXES:
         path = path.with_suffix("")
@@ -277,15 +290,21 @@ 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/...'.
+
+                            BucketFS paths are always POSIX paths, regardless
+                            of the client OS. A string is interpreted as a POSIX
+                            path, i.e. only '/' separates the path segments and
+                            a backslash is a regular character of a file name,
+                            e.g. 'my_model\\v1.pkl'. A PureWindowsPath is
+                            converted to a POSIX path.
 
                             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 = PurePosixPath(_to_posix(path))
         self._bucket_api = bucket_api
 
     @property
@@ -302,8 +321,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
@@ -360,17 +377,22 @@ def relative_to(self, other: PathLike) -> PurePath:
                 f" from a foreign bucket {other_bucket_path._bucket_api}."
             )
         try:
-            return self._path.relative_to(other_bucket_path._path)
+            # Returns a path in the native format of the client OS, as before.
+            return PurePath(self._path.relative_to(other_bucket_path._path))
         except ValueError as ex:
             raise BucketFsError(repr(ex)) from ex
 
     def as_uri(self) -> str:
         return self._path.as_uri()
 
     def as_udf_path(self) -> str:
-        return str(
-            PurePath(self._bucket_api.udf_path) / _remove_archive_suffix(self._path)
-        )
+        udf_root = self._bucket_api.udf_path
+        relative_path = _remove_archive_suffix(self._path)
+        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
+        # format, e.g. 'C:\\tmp\\bucket'.
+        return str(PurePath(udf_root) / relative_path)
 
     def exists(self) -> bool:
         return self._navigate() is not None
@@ -433,7 +455,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(seg)
+            for seg in path_segments
         ]
         new_path = self._path.joinpath(*seg_paths)
         return cls(new_path, self._bucket_api)
@@ -490,9 +513,7 @@ 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)
-        return cls(new_path, self._bucket_api)
+        return self.joinpath(other)
 
     def __eq__(self, other) -> bool:
         if not isinstance(other, BucketPath):
@@ -585,6 +606,12 @@ def build_path(**kwargs) -> PathLike:
 
             path = build_path(...) / "the_desired_path"
 
+            BucketFS paths are always POSIX paths, regardless of the client OS.
+            A string path must use '/' as separator; a backslash is treated as a
+            regular character of a file name. On Windows, either pass a
+            PurePath / PureWindowsPath object, which gets converted, or convert
+            a string to POSIX format first, e.g. PureWindowsPath(s).as_posix().
+
     The rest of the arguments are backend specific.
 
     Additional On-prem arguments:

test/unit/test_bucket_path.py

@@ -2,6 +2,8 @@
 from pathlib import (
     Path,
     PurePath,
+    PurePosixPath,
+    PureWindowsPath,
 )
 
 import pytest
@@ -281,3 +283,142 @@ 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)
+
+
+def test_relative_to_is_native(monkeypatch):
+    """
+    relative_to returns a path in the native format of the client OS, as before.
+    Windows is emulated by patching PurePath.
+    """
+    monkeypatch.setattr(bfs._path, "PurePath", PureWindowsPath)
+    api = _RecordingBucket(files=[])
+    root = bfs.path.BucketPath("root", bucket_api=api)
+    assert (root / "a/b").relative_to(root) == PureWindowsPath("a\\b")
+
+
+class _RecordingBucket:
+    """A bucket API fake recording the paths passed to it."""
+
+    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=8192):
+        self.calls.append(("download", path))
+        return []
+
+    def delete(self, path):
+        self.calls.append(("delete", path))
+
+
+@pytest.mark.parametrize(
+    "segment",
+    [
+        "dir/sub/a.txt",
+        PurePosixPath("dir/sub/a.txt"),
+        PureWindowsPath("dir\\sub\\a.txt"),
+    ],
+)
+def test_paths_are_posix(segment, monkeypatch):
+    """
+    Paths sent to BucketFS must use POSIX separators, even if the path is
+    passed as a PureWindowsPath, see issue #322. Windows is emulated by
+    patching PurePath, which BucketPath uses only where the native path format
+    of the client OS is expected.
+    """
+    monkeypatch.setattr(bfs._path, "PurePath", PureWindowsPath)
+    api = _RecordingBucket(files=["dir/sub/a.txt"])
+    root = bfs.path.BucketPath("", bucket_api=api)
+    for path in [root.joinpath(segment), root / segment]:
+        api.calls.clear()
+        path.write(b"x")
+        list(path.read())
+        path.rm()
+        assert api.calls == [
+            ("upload", "dir/sub/a.txt"),
+            ("download", "dir/sub/a.txt"),
+            ("delete", "dir/sub/a.txt"),
+        ]
+        assert str(path) == "dir/sub/a.txt"
+        assert path.as_udf_path() == "/buckets/bfsdefault/default/dir/sub/a.txt"
+    children = [str(p) for p in root.joinpath(PureWindowsPath("dir")).iterdir()]
+    assert children == ["dir/sub"]
+
+
+def test_backslash_is_part_of_file_name():
+    """
+    A string path is a POSIX path, a backslash is a regular character.
+    """
+    api = _RecordingBucket(files=[])
+    path = bfs.path.BucketPath("dir", bucket_api=api) / "my_model\\v1.pkl"
+    path.write(b"x")
+    assert path.name == "my_model\\v1.pkl"
+    assert api.calls == [("upload", "dir/my_model\\v1.pkl")]
+
+
+def test_as_udf_path_with_windows_udf_root(monkeypatch):
+    """
+    On Windows, the udf_path of a MountedBucket is a native path. The UDF path
+    stays in the native format, as before. Windows is emulated by patching PurePath.
+    """
+    monkeypatch.setattr(bfs._path, "PurePath", PureWindowsPath)
+    api = _RecordingBucket(files=[])
+    api.udf_path = "C:\\tmp\\bucket"
+    path = bfs.path.BucketPath("dir/a.txt", bucket_api=api)
+    assert path.as_udf_path() == "C:\\tmp\\bucket\\dir\\a.txt"
+
+
+def test_mounted_bucket_files_are_posix(bucket_fake):
+    assert "dir1/dir11/file110.dat" in bucket_fake.files
+
+
+def test_absolute_windows_path_is_posix():
+    """
+    An absolute PureWindowsPath is converted the same way in all Python versions.
+    """
+    api = _RecordingBucket(files=[])
+    win_path = PureWindowsPath("\\dir\\a.txt")
+    root = bfs.path.BucketPath("root", bucket_api=api)
+    assert str(bfs.path.BucketPath(win_path, bucket_api=api)) == "/dir/a.txt"
+    assert str(root / win_path) == "/dir/a.txt"
+    assert str(root.joinpath(win_path)) == "/dir/a.txt"
+
+
+@pytest.mark.parametrize(
+    "win_path, expected, expected_joined",
+    [
+        ("C:\\models\\m.pkl", "C:/models/m.pkl", "root/C:/models/m.pkl"),
+        ("C:models\\m.pkl", "C:models/m.pkl", "root/C:models/m.pkl"),
+        (
+            "\\\\server\\share\\models\\m.pkl",
+            "//server/share/models/m.pkl",
+            "//server/share/models/m.pkl",
+        ),
+    ],
+)
+def test_windows_path_with_drive(win_path, expected, expected_joined):
+    """
+    The drive is kept, the result is the same as in Python >= 3.12 without
+    the conversion, see _to_posix().
+    """
+    api = _RecordingBucket(files=[])
+    root = bfs.path.BucketPath("root", bucket_api=api)
+    win_path = PureWindowsPath(win_path)
+    assert str(bfs.path.BucketPath(win_path, bucket_api=api)) == expected
+    assert str(root.joinpath(win_path)) == expected_joined
+    assert str(root / win_path) == expected_joined
+
+
+def test_string_with_drive_is_kept():
+    """
+    A string is taken as a POSIX path as it is, even if it looks like a Windows path.
+    """
+    api = _RecordingBucket(files=[])
+    path = bfs.path.BucketPath("C:\\models\\m.pkl", bucket_api=api)
+    path.write(b"x")
+    assert api.calls == [("upload", "C:\\models\\m.pkl")]