From cb126579ea6c57fc4b7897f3b0f161a63a87eb3f Mon Sep 17 00:00:00 2001 From: Ramnath0521 Date: Tue, 6 Oct 2026 21:17:53 +0530 Subject: [PATCH 1/2] Fixed BucketPath using backslash separators on Windows (#322) BucketPath stored its path as PurePath, which is a PureWindowsPath on Windows, so read(), write(), rm(), iterdir() and as_udf_path() sent backslash paths to BucketFS. Store a PurePosixPath instead, converting PurePath arguments with as_posix(). Closes #322 Co-Authored-By: Claude Sonnet 5.5 --- doc/changes/unreleased.md | 4 ++ exasol/bucketfs/_path.py | 11 ++-- test/unit/test_bucket_path.py | 4 +- test/unit/test_bucket_path_separators.py | 73 ++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 test/unit/test_bucket_path_separators.py diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index fb473705..833cf3b6 100644 --- a/doc/changes/unreleased.md +++ b/doc/changes/unreleased.md @@ -1,3 +1,7 @@ # Unreleased ## Summary + +## Bug Fixing + +* #322: Fixed `BucketPath` using backslash separators on Windows. Paths in a bucket are now always POSIX paths, so `read()`, `write()`, `rm()`, `iterdir()`, `joinpath()` and `as_udf_path()` produce `/`-separated paths on every platform. diff --git a/exasol/bucketfs/_path.py b/exasol/bucketfs/_path.py index 060385f4..4562272a 100644 --- a/exasol/bucketfs/_path.py +++ b/exasol/bucketfs/_path.py @@ -14,7 +14,7 @@ ) from pathlib import ( PurePath, - PureWindowsPath, + PurePosixPath, ) from typing import ( BinaryIO, @@ -285,7 +285,9 @@ def __init__(self, path: str | PurePath, bucket_api: BucketLike): :param bucket_api: An object supporting the Bucket API protocol. """ - self._path = PurePath(path) + self._path = PurePosixPath( + path.as_posix() if isinstance(path, PurePath) else path + ) self._bucket_api = bucket_api @property @@ -302,8 +304,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 +369,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: diff --git a/test/unit/test_bucket_path.py b/test/unit/test_bucket_path.py index 17427e03..feb41115 100644 --- a/test/unit/test_bucket_path.py +++ b/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) diff --git a/test/unit/test_bucket_path_separators.py b/test/unit/test_bucket_path_separators.py new file mode 100644 index 00000000..59992fd5 --- /dev/null +++ b/test/unit/test_bucket_path_separators.py @@ -0,0 +1,73 @@ +from pathlib import PureWindowsPath + +import pytest + +import exasol.bucketfs as bfs + + +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_joinpath_with_windows_path_object_uses_posix_separators(bucket_api): + root = bfs.path.BucketPath("", bucket_api) + path = root.joinpath(PureWindowsPath("dir", "sub"), "a.txt") + assert str(path) == "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_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")] From 764ec939e8ced62aad061f4988ec17ec88c34290 Mon Sep 17 00:00:00 2001 From: Ramnath0521 Date: Wed, 21 Oct 2026 12:55:52 +0530 Subject: [PATCH 2/2] Addressed review of the BucketPath separator fix (#322) Convert every path that enters BucketPath through one helper, so a Windows string, a Windows path object, joinpath() and "/" all end up as the same POSIX path: separators become "/", a drive or UNC share is dropped and a root is kept. Make MountedBucket.files and udf_path POSIX as well, which is the root cause the reviewer pointed out and also makes the existing unit tests pass on Windows. Document the relative_to() return type change under Breaking Changes and cover backslash strings, drive and UNC anchors, joinpath() and "/" with Windows path objects and MountedBucket in the unit tests. Co-Authored-By: Claude Sonnet 5.5 --- doc/changes/unreleased.md | 6 ++- exasol/bucketfs/_buckets.py | 4 +- exasol/bucketfs/_path.py | 32 ++++++++++--- test/unit/test_bucket_path_separators.py | 58 +++++++++++++++++++++--- 4 files changed, 84 insertions(+), 16 deletions(-) diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index 833cf3b6..c120e2ad 100644 --- a/doc/changes/unreleased.md +++ b/doc/changes/unreleased.md @@ -4,4 +4,8 @@ ## Bug Fixing -* #322: Fixed `BucketPath` using backslash separators on Windows. Paths in a bucket are now always POSIX paths, so `read()`, `write()`, `rm()`, `iterdir()`, `joinpath()` and `as_udf_path()` produce `/`-separated paths on every platform. +* #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. diff --git a/exasol/bucketfs/_buckets.py b/exasol/bucketfs/_buckets.py index 0c02c991..a06afbc3 100644 --- a/exasol/bucketfs/_buckets.py +++ b/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: diff --git a/exasol/bucketfs/_path.py b/exasol/bucketfs/_path.py index 4562272a..52eaf6c1 100644 --- a/exasol/bucketfs/_path.py +++ b/exasol/bucketfs/_path.py @@ -15,6 +15,7 @@ from pathlib import ( PurePath, PurePosixPath, + PureWindowsPath, ) from typing import ( BinaryIO, @@ -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,17 +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 = PurePosixPath( - path.as_posix() if isinstance(path, PurePath) else path - ) + self._path = _to_posix_path(path) self._bucket_api = bucket_api @property @@ -434,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) @@ -492,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: diff --git a/test/unit/test_bucket_path_separators.py b/test/unit/test_bucket_path_separators.py index 59992fd5..6f4296e0 100644 --- a/test/unit/test_bucket_path_separators.py +++ b/test/unit/test_bucket_path_separators.py @@ -1,8 +1,10 @@ +import os from pathlib import PureWindowsPath import pytest import exasol.bucketfs as bfs +from exasol.bucketfs._buckets import MountedBucket class RecordingBucketApi: @@ -45,12 +47,6 @@ def test_read_write_and_rm_send_posix_paths(bucket_api): ] -def test_joinpath_with_windows_path_object_uses_posix_separators(bucket_api): - root = bfs.path.BucketPath("", bucket_api) - path = root.joinpath(PureWindowsPath("dir", "sub"), "a.txt") - assert str(path) == "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"] @@ -66,8 +62,58 @@ def test_windows_path_object_is_found_in_the_bucket_listing(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"