diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index 8a330ed..c08ac69 100644 --- a/doc/changes/unreleased.md +++ b/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 diff --git a/exasol/bucketfs/_path.py b/exasol/bucketfs/_path.py index 7771852..6c00f4a 100644 --- a/exasol/bucketfs/_path.py +++ b/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 diff --git a/test/unit/test_bucket_path.py b/test/unit/test_bucket_path.py index 492a2e8..f5aa3d0 100644 --- a/test/unit/test_bucket_path.py +++ b/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