From 857b513779a301c03076a4aec937fecd697f165c Mon Sep 17 00:00:00 2001 From: Maciej Urbanski Date: Tue, 14 May 2024 14:56:57 +0200 Subject: [PATCH] fix `b2 ls b2://bucketName/fileName` and same for `rm` --- b2/_internal/_cli/argcompleters.py | 1 + b2/_internal/_utils/uri.py | 6 ++-- b2/_internal/b2v3/registry.py | 4 +-- b2/_internal/b2v3/rm.py | 29 +++++++++++++++-- b2/_internal/console_tool.py | 3 +- changelog.d/+ls_file.fixed.md | 1 + pyproject.toml | 2 +- test/integration/test_b2_command_line.py | 7 ++-- test/unit/conftest.py | 5 +++ test/unit/console_tool/test_ls.py | 41 ++++++++++++++++++------ test/unit/console_tool/test_rm.py | 27 ++++++++++++++++ 11 files changed, 106 insertions(+), 20 deletions(-) create mode 100644 changelog.d/+ls_file.fixed.md create mode 100644 test/unit/console_tool/test_rm.py diff --git a/b2/_internal/_cli/argcompleters.py b/b2/_internal/_cli/argcompleters.py index ebf62b0b4..d59d75478 100644 --- a/b2/_internal/_cli/argcompleters.py +++ b/b2/_internal/_cli/argcompleters.py @@ -46,6 +46,7 @@ def file_name_completer(prefix, parsed_args, **kwargs): latest_only=True, recursive=False, fetch_count=LIST_FILE_NAMES_MAX_LIMIT, + folder_to_list_can_be_a_file=True, ) return [ unprintable_to_hex(folder_name or file_version.file_name) diff --git a/b2/_internal/_utils/uri.py b/b2/_internal/_utils/uri.py index fb61f088a..c5524f261 100644 --- a/b2/_internal/_utils/uri.py +++ b/b2/_internal/_utils/uri.py @@ -194,10 +194,10 @@ def _(self, uri: B2FileIdURI, *args, **kwargs) -> str: return self.get_download_url_for_fileid(uri.file_id, *args, **kwargs) @singledispatchmethod - def list_file_versions_by_uri(self, uri, *args, **kwargs): + def ls(self, uri, *args, **kwargs): raise NotImplementedError(f"Unsupported URI type: {type(uri)}") - @list_file_versions_by_uri.register + @ls.register def _(self, uri: B2URI, *args, filters: Sequence[Filter] = (), **kwargs): bucket = self.api.get_bucket_by_name(uri.bucket_name) try: @@ -207,6 +207,6 @@ def _(self, uri: B2URI, *args, filters: Sequence[Filter] = (), **kwargs): # exactly one – `with_wildcard` being passed without `recursive` option. raise B2Error(error.args[0]) - @list_file_versions_by_uri.register + @ls.register def _(self, uri: B2FileIdURI, *args, **kwargs): yield self.get_file_info_by_uri(uri), None diff --git a/b2/_internal/b2v3/registry.py b/b2/_internal/b2v3/registry.py index 7935b8404..811e41975 100644 --- a/b2/_internal/b2v3/registry.py +++ b/b2/_internal/b2v3/registry.py @@ -12,7 +12,7 @@ from b2._internal.b2v4.registry import * # noqa from b2._internal._cli.b2api import _get_b2api_for_profile from b2._internal.arg_parser import enable_camel_case_arguments -from .rm import Rm +from .rm import Rm, B2URIMustPointToFolderMixin from .sync import Sync enable_camel_case_arguments() @@ -45,7 +45,7 @@ def main() -> None: os._exit(exit_status) -class Ls(B2URIBucketNFolderNameArgMixin, BaseLs): +class Ls(B2URIMustPointToFolderMixin, B2URIBucketNFolderNameArgMixin, BaseLs): """ {BaseLs} diff --git a/b2/_internal/b2v3/rm.py b/b2/_internal/b2v3/rm.py index 4055de90e..39fe8f430 100644 --- a/b2/_internal/b2v3/rm.py +++ b/b2/_internal/b2v3/rm.py @@ -9,12 +9,37 @@ ###################################################################### from __future__ import annotations +import dataclasses +import typing + from b2._internal.b2v4.registry import B2URIBucketNFolderNameArgMixin, BaseRm +if typing.TYPE_CHECKING: + import argparse + + from b2._internal._utils.uri import B2URI + + +class B2URIMustPointToFolderMixin: + """ + Extension to B2URI*Mixins to ensure that the b2:// URIs point to a folder. + + This is directly related to how b2sdk.v3.Bucket.ls() treats paths ending with a slash as folders, where as + paths not ending with a slash are treated as potential files. + + For b2v3 we need to support old behavior which never attempted to treat the path as a file. + """ + + def get_b2_uri_from_arg(self, args: argparse.Namespace) -> B2URI: + b2_uri = super().get_b2_uri_from_arg(args) + if b2_uri.path and not args.with_wildcard and not b2_uri.path.endswith("/"): + b2_uri = dataclasses.replace(b2_uri, path=b2_uri.path + "/") + return b2_uri + # NOTE: We need to keep v3 Rm in separate file, because we need to import it in # unit tests without registering any commands. -class Rm(B2URIBucketNFolderNameArgMixin, BaseRm): +class Rm(B2URIMustPointToFolderMixin, B2URIBucketNFolderNameArgMixin, BaseRm): """ {BaseRm} @@ -57,4 +82,4 @@ class Rm(B2URIBucketNFolderNameArgMixin, BaseRm): - **listFiles** - **deleteFiles** - **bypassGovernance** (if --bypass-governance is used) - """ + """ diff --git a/b2/_internal/console_tool.py b/b2/_internal/console_tool.py index 499087a00..25e3a9792 100644 --- a/b2/_internal/console_tool.py +++ b/b2/_internal/console_tool.py @@ -2387,12 +2387,13 @@ def _print_file_version( def _get_ls_generator(self, args, b2_uri: B2URI | None = None): b2_uri = b2_uri or self.get_b2_uri_from_arg(args) try: - yield from self.api.list_file_versions_by_uri( + yield from self.api.ls( b2_uri, latest_only=not args.versions, recursive=args.recursive, with_wildcard=args.with_wildcard, filters=args.filters, + folder_to_list_can_be_a_file=True, ) except Exception as err: raise CommandError(unprintable_to_hex(str(err))) from err diff --git a/changelog.d/+ls_file.fixed.md b/changelog.d/+ls_file.fixed.md new file mode 100644 index 000000000..de7eab704 --- /dev/null +++ b/changelog.d/+ls_file.fixed.md @@ -0,0 +1 @@ +Fix `b2 ls b2://bucketName/fileName` and `b2 rm b2://bucketName/fileName` to respectively, list and remove file identified by supplied B2 URI. diff --git a/pyproject.toml b/pyproject.toml index 6f8e76bb4..ff817d2d3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ classifiers = [ dependencies = [ "argcomplete>=2,<4", "arrow>=1.0.2,<2.0.0", - "b2sdk>=2.2.1,<3", + "b2sdk>=2.3.0,<3", "docutils>=0.18.1", "idna~=3.4; platform_system == 'Java'", "importlib-metadata>=3.3; python_version < '3.8'", diff --git a/test/integration/test_b2_command_line.py b/test/integration/test_b2_command_line.py index 605e325a5..242d2485c 100755 --- a/test/integration/test_b2_command_line.py +++ b/test/integration/test_b2_command_line.py @@ -296,7 +296,7 @@ def test_download(b2_tool, bucket_name, sample_filepath, uploaded_sample_file, t assert output_b.read_text() == sample_filepath.read_text() -def test_basic(b2_tool, bucket_name, sample_file, tmp_path, b2_uri_args): +def test_basic(b2_tool, bucket_name, sample_file, tmp_path, b2_uri_args, apiver_int): file_mod_time_str = str(file_mod_time_millis(sample_file)) @@ -373,7 +373,10 @@ def test_basic(b2_tool, bucket_name, sample_file, tmp_path, b2_uri_args): list_of_files = b2_tool.should_succeed_json( ['ls', '--json', '--recursive', '--versions', *b2_uri_args(bucket_name, 'c')] ) - should_equal([], [f['fileName'] for f in list_of_files]) + if apiver_int >= 4: # b2://bucketName/c should list all c versions on v4 + should_equal(['c', 'c'], [f['fileName'] for f in list_of_files]) + else: + should_equal([], [f['fileName'] for f in list_of_files]) b2_tool.should_succeed(['file', 'copy-by-id', first_a_version['fileId'], bucket_name, 'x']) diff --git a/test/unit/conftest.py b/test/unit/conftest.py index 83131c0b8..bb7eb21bc 100644 --- a/test/unit/conftest.py +++ b/test/unit/conftest.py @@ -140,6 +140,11 @@ def bucket(bucket_info): return bucket_info['bucketName'] +@pytest.fixture +def api_bucket(bucket_info, b2_cli): + return b2_cli.b2_api.get_bucket_by_name(bucket_info['bucketName']) + + @pytest.fixture def local_file(tmp_path): """Set up a test file and return its path.""" diff --git a/test/unit/console_tool/test_ls.py b/test/unit/console_tool/test_ls.py index 360c22f62..f6239d0c4 100644 --- a/test/unit/console_tool/test_ls.py +++ b/test/unit/console_tool/test_ls.py @@ -7,15 +7,7 @@ # License https://www.backblaze.com/using_b2_code.html # ###################################################################### -###################################################################### -# -# File: test/unit/console_tool/test_download_file.py -# -# Copyright 2023 Backblaze Inc. All Rights Reserved. -# -# License https://www.backblaze.com/using_b2_code.html -# -###################################################################### +from __future__ import annotations import pytest @@ -64,3 +56,34 @@ def test_ls__without_bucket_name__option_not_supported(b2_cli, bucket_info, flag expected_stderr=f"ERROR: Cannot use {flag} option without specifying a bucket name\n", expected_status=1, ) + + +@pytest.mark.apiver(to_ver=3) +def test_ls__pre_v4__should_not_return_exact_match_filename(b2_cli, uploaded_file): + """`b2v3 ls bucketName folderName` should not return files named `folderName` even if such exist""" + b2_cli.run(["ls", uploaded_file['bucket']], expected_stdout='file1.txt\n') # sanity check + b2_cli.run( + ["ls", uploaded_file['bucket'], uploaded_file['fileName']], + expected_stdout='', + ) + + +@pytest.mark.apiver(from_ver=4) +def test_ls__b2_uri__pointing_to_bucket(b2_cli, uploaded_file): + b2_cli.run( + ["ls", f"b2://{uploaded_file['bucket']}/"], + expected_stdout='file1.txt\n', + ) + + +@pytest.mark.apiver(from_ver=4) +def test_ls__b2_uri__pointing_to_a_file(b2_cli, uploaded_file): + b2_cli.run( + ["ls", f"b2://{uploaded_file['bucket']}/{uploaded_file['fileName']}"], + expected_stdout='file1.txt\n', + ) + + b2_cli.run( + ["ls", f"b2://{uploaded_file['bucket']}/nonExistingFile"], + expected_stdout='', + ) diff --git a/test/unit/console_tool/test_rm.py b/test/unit/console_tool/test_rm.py new file mode 100644 index 000000000..61ffe3f6d --- /dev/null +++ b/test/unit/console_tool/test_rm.py @@ -0,0 +1,27 @@ +###################################################################### +# +# File: test/unit/console_tool/test_rm.py +# +# Copyright 2024 Backblaze Inc. All Rights Reserved. +# +# License https://www.backblaze.com/using_b2_code.html +# +###################################################################### +from __future__ import annotations + +import pytest + + +@pytest.mark.apiver(to_ver=3) +def test_rm__pre_v4__should_not_rm_exact_match_filename(b2_cli, api_bucket, uploaded_file): + """`b2v3 rm bucketName folderName` should not remove file named `folderName` even if such exist""" + b2_cli.run(["rm", uploaded_file['bucket'], uploaded_file['fileName']]) + assert list(api_bucket.ls()) # nothing was removed + + +@pytest.mark.apiver(from_ver=4) +def test_rm__b2_uri__pointing_to_a_file(b2_cli, api_bucket, uploaded_file): + b2_cli.run(["rm", f"b2://{uploaded_file['bucket']}/noSuchFile"]) + assert list(api_bucket.ls()) # sanity check: bucket is not empty + b2_cli.run(["rm", f"b2://{uploaded_file['bucket']}/{uploaded_file['fileName']}"]) + assert not list(api_bucket.ls())