diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index fb473705..c120e2ad 100644 --- a/doc/changes/unreleased.md +++ b/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. 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 060385f4..52eaf6c1 100644 --- a/exasol/bucketfs/_path.py +++ b/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: 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..6f4296e0 --- /dev/null +++ b/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"