From d1376387562ccbefa9fdf74c1cf1c710855e9088 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 + pdm.lock | 8 ++--- 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 ++++++++++++++++ 12 files changed, 110 insertions(+), 24 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/pdm.lock b/pdm.lock index d255b0334..ea3954a07 100644 --- a/pdm.lock +++ b/pdm.lock @@ -5,7 +5,7 @@ groups = ["default", "bundle", "doc", "format", "full", "license", "lint", "release", "test"] strategy = ["cross_platform", "inherit_metadata"] lock_version = "4.4.1" -content_hash = "sha256:52799916d0b14eb871412bb22e75c0b382490ebd6323658408507f5f1f8c41c3" +content_hash = "sha256:ebe6f071ff22591549adda595b961246f8f9c6e6cde6fc5f63680741fc6bd810" [[package]] name = "alabaster" @@ -100,7 +100,7 @@ files = [ [[package]] name = "b2sdk" -version = "2.2.1" +version = "2.3.0" requires_python = ">=3.7" summary = "Backblaze B2 SDK" groups = ["default"] @@ -111,8 +111,8 @@ dependencies = [ "typing-extensions>=4.7.1; python_version < \"3.12\"", ] files = [ - {file = "b2sdk-2.2.1-py3-none-any.whl", hash = "sha256:f6f78450bb802b54e6b8d73f76e4be349587adfa02f1ea57abcd5a0986a63663"}, - {file = "b2sdk-2.2.1.tar.gz", hash = "sha256:fdbd9a46b94d23003f2565ef5e3882f51454c7c5f922cbbbaa5aa340f1893aea"}, + {file = "b2sdk-2.3.0-py3-none-any.whl", hash = "sha256:030696ef0ee8f958975e784c5a8248b91f222fc3c31ad24c5bb9ff6296135218"}, + {file = "b2sdk-2.3.0.tar.gz", hash = "sha256:fc292fceb5ec35c0dfd681f23e76b4449b63bb82d00423fc54edc0f4363f53e1"}, ] [[package]] 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())