From d1376387562ccbefa9fdf74c1cf1c710855e9088 Mon Sep 17 00:00:00 2001 From: Maciej Urbanski Date: Tue, 14 May 2024 14:56:57 +0200 Subject: [PATCH 1/3] 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()) From 56a562a5ce543405d835270296bb5153df876ff2 Mon Sep 17 00:00:00 2001 From: Maciej Urbanski Date: Wed, 15 May 2024 15:19:17 +0200 Subject: [PATCH 2/3] always treat hyphen as stdin alias in b2v4 and above --- CHANGELOG.md | 1 + b2/_internal/b2v3/registry.py | 18 ++++++++++ b2/_internal/console_tool.py | 7 +--- changelog.d/+hyphen_filename.fixed.md | 2 ++ test/unit/console_tool/test_upload_file.py | 38 +++++++++++++++++++--- 5 files changed, 55 insertions(+), 11 deletions(-) create mode 100644 changelog.d/+hyphen_filename.fixed.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 69d85bfce..e97d71eaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ upcoming release can be found in [changelog.d](changelog.d). These means following breaking changes: - `b2` will no longer persists credentials and other secrets on disk if credentials were passed through `B2_*` environment variables. To explicitly persist them and keep using local cache for better performance, user can simply call `b2 account account` - `b2 ls` and `b2 rm` no longer accept two positional arguments, instead accepting only `B2 URI` (e.g. `b2://bucketName/path`) + - `-` is no longer supported as a valid filename, always being interpreted as standard input alias instead - Changed `sync` command exit status code from 0 to 1 if any warnings or errors were encountered during the operation. ### Fixed diff --git a/b2/_internal/b2v3/registry.py b/b2/_internal/b2v3/registry.py index 811e41975..400b7d33c 100644 --- a/b2/_internal/b2v3/registry.py +++ b/b2/_internal/b2v3/registry.py @@ -92,6 +92,24 @@ class Ls(B2URIMustPointToFolderMixin, B2URIBucketNFolderNameArgMixin, BaseLs): ALLOW_ALL_BUCKETS = True +class HyphenFilenameMixin: + def get_input_stream(self, filename): + if filename == '-' and os.path.exists('-'): + self._print_stderr( + "WARNING: Filename `-` won't be supported in the future and will always be treated as stdin alias." + ) + return '-' + return super().get_input_stream(filename) + + +class UploadUnboundStream(HyphenFilenameMixin, UploadUnboundStream): + __doc__ = UploadUnboundStream.__doc__ + + +class UploadFile(HyphenFilenameMixin, UploadFile): + __doc__ = UploadFile.__doc__ + + B2.register_subcommand(AuthorizeAccount) B2.register_subcommand(CancelAllUnfinishedLargeFiles) B2.register_subcommand(CancelLargeFile) diff --git a/b2/_internal/console_tool.py b/b2/_internal/console_tool.py index 25e3a9792..c7418f3ee 100644 --- a/b2/_internal/console_tool.py +++ b/b2/_internal/console_tool.py @@ -3434,12 +3434,7 @@ def upload_file_kwargs_to_unbound_upload(self, **kwargs): def get_input_stream(self, filename: str) -> str | int | io.BinaryIO: """Get input stream IF filename points to a FIFO or stdin.""" if filename == "-": - if os.path.exists('-'): - self._print_stderr( - "WARNING: Filename `-` won't be supported in the future and will always be treated as stdin alias." - ) - else: - return sys.stdin.buffer if platform.system() == "Windows" else sys.stdin.fileno() + return sys.stdin.buffer if platform.system() == "Windows" else sys.stdin.fileno() elif points_to_fifo(pathlib.Path(filename)): return filename diff --git a/changelog.d/+hyphen_filename.fixed.md b/changelog.d/+hyphen_filename.fixed.md new file mode 100644 index 000000000..2f467d3a1 --- /dev/null +++ b/changelog.d/+hyphen_filename.fixed.md @@ -0,0 +1,2 @@ +Fix `-` handling in file upload commands - even if file with `-` name exists, the stdin will be chosen over it. +This change affects `b2v4` (which is also aliased as `b2`), but not `b2v3` to keep backwards compatibility. diff --git a/test/unit/console_tool/test_upload_file.py b/test/unit/console_tool/test_upload_file.py index 6f4a65e87..44ad7750b 100644 --- a/test/unit/console_tool/test_upload_file.py +++ b/test/unit/console_tool/test_upload_file.py @@ -10,7 +10,7 @@ import os from test.helpers import skip_on_windows -import b2 +import pytest def test_upload_file__file_info_src_last_modified_millis_and_headers(b2_cli, bucket, tmpdir): @@ -77,10 +77,9 @@ def test_upload_file__named_pipe(b2_cli, bucket, tmpdir, bg_executor): writer.result(timeout=1) +@pytest.mark.apiver(to_ver=3) def test_upload_file__hyphen_file_instead_of_stdin(b2_cli, bucket, tmpdir, monkeypatch): """Test `file upload` will upload file named `-` instead of stdin by default""" - # TODO remove this in v4 - assert b2.__version__ < '4', "`-` filename should not be supported in next major version of CLI" filename = 'stdin.txt' content = "I'm very rare creature, a file named '-'" monkeypatch.chdir(str(tmpdir)) @@ -95,15 +94,44 @@ def test_upload_file__hyphen_file_instead_of_stdin(b2_cli, bucket, tmpdir, monke "size": len(content), } b2_cli.run( - ['file', 'upload', '--no-progress', 'my-bucket', '-', filename], + ['upload-file', '--no-progress', 'my-bucket', '-', filename], expected_json_in_stdout=expected_json, remove_version=True, expected_part_of_stdout=expected_stdout, - expected_stderr= + expected_stderr="WARNING: `upload-file` command is deprecated. Use `file upload` instead.\n" "WARNING: Filename `-` won't be supported in the future and will always be treated as stdin alias.\n", ) +@pytest.mark.apiver(from_ver=4) +def test_upload_file__ignore_hyphen_file(b2_cli, bucket, tmpdir, monkeypatch, mock_stdin): + """Test `file upload` will upload stdin even when file named `-` is explicitly specified""" + content = "I'm very rare creature, a file named '-'" + monkeypatch.chdir(str(tmpdir)) + source_file = tmpdir.join('-') + source_file.write(content) + + content = "stdin input" + filename = 'stdin.txt' + + expected_stdout = f'URL by file name: http://download.example.com/file/my-bucket/{filename}' + expected_json = { + "action": "upload", + "contentSha1": "2ce72aa159d1f190fddf295cc883f20c4787a751", + "fileName": filename, + "size": len(content), + } + mock_stdin.write(content) + mock_stdin.close() + + b2_cli.run( + ['file', 'upload', '--no-progress', 'my-bucket', '-', filename], + expected_json_in_stdout=expected_json, + remove_version=True, + expected_part_of_stdout=expected_stdout, + ) + + def test_upload_file__stdin(b2_cli, bucket, tmpdir, mock_stdin): """Test `file upload` stdin alias support""" content = "stdin input" From e4c22566d0cf53b0def0f4ed22536b69e4b696ff Mon Sep 17 00:00:00 2001 From: Maciej Urbanski Date: Wed, 15 May 2024 16:27:48 +0200 Subject: [PATCH 3/3] cleanup old buckets more aggresively --- test/integration/helpers.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/integration/helpers.py b/test/integration/helpers.py index da7458ae8..701329448 100755 --- a/test/integration/helpers.py +++ b/test/integration/helpers.py @@ -61,8 +61,7 @@ logger = logging.getLogger(__name__) -# A large period is set here to avoid issues related to clock skew or other time-related issues under CI -BUCKET_CLEANUP_PERIOD_MILLIS = timedelta(days=1).total_seconds() * 1000 +BUCKET_CLEANUP_PERIOD_MILLIS = timedelta(hours=3).total_seconds() * 1000 ONE_HOUR_MILLIS = 60 * 60 * 1000 ONE_DAY_MILLIS = ONE_HOUR_MILLIS * 24