From adb37db3b0eab1a9d578a4b04b4638bb62c0736a Mon Sep 17 00:00:00 2001 From: Maciej Urbanski Date: Fri, 11 Oct 2024 00:20:06 +0200 Subject: [PATCH] fix "b2 file hide b2://" handling and documentation --- b2/_internal/_cli/b2args.py | 57 ++++++++++++++++-------- b2/_internal/console_tool.py | 39 ++++++++++------ changelog.d/+fix_hide_file.fixed.md | 1 + test/unit/console_tool/test_file_hide.py | 44 ++++++++++++++++++ test/unit/test_console_tool.py | 24 +++++----- 5 files changed, 119 insertions(+), 46 deletions(-) create mode 100644 changelog.d/+fix_hide_file.fixed.md create mode 100644 test/unit/console_tool/test_file_hide.py diff --git a/b2/_internal/_cli/b2args.py b/b2/_internal/_cli/b2args.py index 86cfe6fe..61214e0f 100644 --- a/b2/_internal/_cli/b2args.py +++ b/b2/_internal/_cli/b2args.py @@ -55,7 +55,7 @@ def b2id_or_b2_bucket_uri(value: str) -> Union[B2URI, B2FileIdURI]: return b2_uri -def b2id_or_file_like_b2_uri(value: str) -> B2URIBase: +def b2id_or_file_like_b2_uri(value: str, *, by_id: Optional[bool] = None) -> B2URIBase: b2_uri = parse_b2_uri(value) if isinstance(b2_uri, B2URI): if b2_uri.is_dir(): @@ -63,6 +63,12 @@ def b2id_or_file_like_b2_uri(value: str) -> B2URIBase: f"B2 URI pointing to a file-like object is required, but {value} was provided" ) return b2_uri + elif isinstance(b2_uri, B2FileIdURI): + if by_id is False: + raise ValueError( + f"B2 URI pointing to file-like object by name is required (e.g. b2://bucketName/fileName)," + f" but {value} was provided" + ) return b2_uri @@ -78,12 +84,17 @@ def parse_bucket_name(value: str, allow_all_buckets: bool = False) -> str: return str(value) -def b2id_or_file_like_b2_uri_or_bucket_name(value: str) -> Union[B2URIBase, str]: - try: - bucket_name = parse_bucket_name(value) - return bucket_name - except ValueError: - return b2id_or_file_like_b2_uri(value) +def b2id_or_file_like_b2_uri_or_bucket_name(value: str, *, + by_id: Optional[bool] = None) -> Union[B2URIBase, str]: + if "://" not in value: + return value + else: + b2_uri = b2id_or_file_like_b2_uri(value, by_id=by_id) + if isinstance(b2_uri, B2FileIdURI) and by_id is False: + raise ValueError( + "This command doesn't support file id as an argument, use b2://bucketName/fileName instead" + ) + return b2_uri B2ID_URI_ARG_TYPE = wrap_with_argument_type_error(b2id_uri) @@ -94,9 +105,6 @@ def b2id_or_file_like_b2_uri_or_bucket_name(value: str) -> Union[B2URIBase, str] functools.partial(parse_b2_uri, allow_all_buckets=True) ) B2ID_OR_FILE_LIKE_B2_URI_ARG_TYPE = wrap_with_argument_type_error(b2id_or_file_like_b2_uri) -B2ID_OR_FILE_LIKE_B2_URI_OR_BUCKET_NAME_ARG_TYPE = wrap_with_argument_type_error( - b2id_or_file_like_b2_uri_or_bucket_name -) def add_bucket_name_argument( @@ -187,35 +195,46 @@ def add_b2id_or_b2_uri_argument( def add_b2id_or_b2_bucket_uri_argument(parser: argparse.ArgumentParser, name="B2_URI"): - parser.add_argument( + arg = parser.add_argument( name, type=B2ID_OR_B2_BUCKET_URI_ARG_TYPE, help="B2 URI pointing to a bucket, or a file id. e.g. b2://yourBucket, or b2id://fileId", - ).completer = b2uri_file_completer + ) + arg.completer = b2uri_file_completer + return arg def add_b2id_or_file_like_b2_uri_argument(parser: argparse.ArgumentParser, name="B2_URI"): """ Add a B2 URI pointing to a file as an argument to the parser. """ - parser.add_argument( + arg = parser.add_argument( name, type=B2ID_OR_FILE_LIKE_B2_URI_ARG_TYPE, help="B2 URI pointing to a file, e.g. b2://yourBucket/file.txt or b2id://fileId", - ).completer = b2uri_file_completer + ) + arg.completer = b2uri_file_completer + return arg def add_b2id_or_file_like_b2_uri_or_bucket_name_argument( - parser: argparse.ArgumentParser, name="B2_URI" + parser: argparse.ArgumentParser, name="B2_URI", by_id: Optional[bool] = None ): """ Add a B2 URI pointing to a file as an argument to the parser. """ - parser.add_argument( + help_ = "B2 URI pointing to a file, e.g. b2://yourBucket/file.txt" + if by_id is not False: + help_ += " or b2id://fileId" + arg = parser.add_argument( name, - type=B2ID_OR_FILE_LIKE_B2_URI_OR_BUCKET_NAME_ARG_TYPE, - help="B2 URI pointing to a file, e.g. b2://yourBucket/file.txt or b2id://fileId", - ).completer = b2uri_file_completer + type=wrap_with_argument_type_error( + functools.partial(b2id_or_file_like_b2_uri_or_bucket_name, by_id=by_id) + ), + help=help_, + ) + arg.completer = b2uri_file_completer + return arg def get_keyid_and_key_from_env_vars() -> Tuple[Optional[str], Optional[str]]: diff --git a/b2/_internal/console_tool.py b/b2/_internal/console_tool.py index 662443d2..a40c0204 100644 --- a/b2/_internal/console_tool.py +++ b/b2/_internal/console_tool.py @@ -714,18 +714,34 @@ def get_b2_uri_from_arg(self, args: argparse.Namespace) -> B2URIBase: class B2URIFileOrBucketNameFileNameArgMixin: + SUPPORTS_B2_ID: bool = True + @classmethod def _setup_parser(cls, parser): - add_b2id_or_file_like_b2_uri_or_bucket_name_argument(parser) + cls._b2_uri_arg = add_b2id_or_file_like_b2_uri_or_bucket_name_argument( + parser, by_id=cls.SUPPORTS_B2_ID + ) parser.add_argument('fileName', nargs='?', help=argparse.SUPPRESS) super()._setup_parser(parser) - def get_b2_uri_from_arg(self, args: argparse.Namespace) -> B2URIBase | str: - if isinstance(args.B2_URI, B2URI): + def get_b2_uri_from_arg(self, args: argparse.Namespace) -> B2URIBase: + if isinstance(args.B2_URI, B2URIBase): + if args.fileName: + raise argparse.ArgumentError( + self._b2_uri_arg, + "Both B2 URI and file name were provided, but only one is expected" + ) return args.B2_URI - bucket_name = args.B2_URI - return bucket_name + if not args.fileName: + raise argparse.ArgumentError( + self._b2_uri_arg, + f"Incorrect B2 URI was provided, expected `b2://bucketName/fileName`, but got {args.B2_URI!r}" + ) + self._print_stderr( + 'WARNING: "bucketName fileName" arguments syntax is deprecated, use "b2://bucketName/fileName" instead' + ) + return B2URI(args.B2_URI, args.fileName) class B2URIFileIDArgMixin: @@ -2179,15 +2195,9 @@ class FileHideBase(Command): def _run(self, args): b2_uri = self.get_b2_uri_from_arg(args) - if isinstance(b2_uri, B2URI): - bucket_name = b2_uri.bucket_name - file_name = b2_uri.path - else: - bucket_name = b2_uri - file_name = args.fileName - bucket = self.api.get_bucket_by_name(bucket_name) - file_info = bucket.hide_file(file_name) + bucket = self.api.get_bucket_by_name(b2_uri.bucket_name) + file_info = bucket.hide_file(b2_uri.path) self._print_json(file_info) return 0 @@ -5086,7 +5096,7 @@ class File(Command): {NAME} file cat b2://yourBucket/file.txt {NAME} file copy-by-id sourceFileId yourBucket file.txt {NAME} file download b2://yourBucket/file.txt localFile.txt - {NAME} file hide yourBucket file.txt + {NAME} file hide b2://yourBucket/file.txt {NAME} file info b2://yourBucket/file.txt {NAME} file update --legal-hold off b2://yourBucket/file.txt {NAME} file upload yourBucket localFile.txt file.txt @@ -5135,6 +5145,7 @@ class FileCopyById(FileCopyByIdBase): class FileHide(B2URIFileOrBucketNameFileNameArgMixin, FileHideBase): __doc__ = FileHideBase.__doc__ COMMAND_NAME = 'hide' + SUPPORTS_B2_ID = False @File.subcommands_registry.register diff --git a/changelog.d/+fix_hide_file.fixed.md b/changelog.d/+fix_hide_file.fixed.md new file mode 100644 index 00000000..e6eff1f9 --- /dev/null +++ b/changelog.d/+fix_hide_file.fixed.md @@ -0,0 +1 @@ +Fix `b2 file hide b2://bucket/file` handling and test coverage. diff --git a/test/unit/console_tool/test_file_hide.py b/test/unit/console_tool/test_file_hide.py new file mode 100644 index 00000000..e876c58b --- /dev/null +++ b/test/unit/console_tool/test_file_hide.py @@ -0,0 +1,44 @@ +###################################################################### +# +# File: test/unit/console_tool/test_file_hide.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_legacy_hide_file(b2_cli, api_bucket, uploaded_file): + b2_cli.run( + ["hide-file", uploaded_file["bucket"], uploaded_file["fileName"]], + expected_stderr='WARNING: `hide-file` command is deprecated. Use `file hide` instead.\n' + ) + assert not list(api_bucket.ls()) + + +@pytest.mark.apiver(to_ver=4) +def test_file_hide__by_bucket_and_file_name(b2_cli, api_bucket, uploaded_file): + b2_cli.run( + ["file", "hide", uploaded_file["bucket"], uploaded_file["fileName"]], + expected_stderr=( + 'WARNING: "bucketName fileName" arguments syntax is deprecated, use "b2://bucketName/fileName" instead\n' + ), + ) + assert not list(api_bucket.ls()) + + +@pytest.mark.apiver +def test_file_hide__by_b2_uri(b2_cli, api_bucket, uploaded_file): + b2_cli.run(["file", "hide", f"b2://{uploaded_file['bucket']}/{uploaded_file['fileName']}"]) + assert not list(api_bucket.ls()) + + +@pytest.mark.apiver +def test_file_hide__cannot_hide_by_b2id(b2_cli, api_bucket, uploaded_file): + b2_cli.run(["file", "hide", f"b2id://{uploaded_file['fileId']}"], expected_status=2) + assert list(api_bucket.ls()) diff --git a/test/unit/test_console_tool.py b/test/unit/test_console_tool.py index 5335cc8f..fae0a47a 100644 --- a/test/unit/test_console_tool.py +++ b/test/unit/test_console_tool.py @@ -1018,7 +1018,7 @@ def test_rm_fileid_v4(self): ) # Hide file - self._run_command(['file', 'hide', 'my-bucket', 'file1.txt'],) + self._run_command(['file', 'hide', 'b2://my-bucket/file1.txt'],) # Delete one file version self._run_command(['rm', 'b2id://9998']) @@ -1087,7 +1087,7 @@ def test_hide_file_legacy_syntax(self): } self._run_command( - ['file', 'hide', 'my-bucket', 'file1.txt'], + ['file', 'hide', 'b2://my-bucket/file1.txt'], expected_json_in_stdout=expected_json, ) @@ -1178,7 +1178,7 @@ def test_files(self): } self._run_command( - ['file', 'hide', 'my-bucket', 'file1.txt'], + ['file', 'hide', 'b2://my-bucket/file1.txt'], expected_json_in_stdout=expected_json, ) @@ -1386,7 +1386,7 @@ def test_files_encrypted(self): } self._run_command( - ['file', 'hide', 'my-bucket', 'file1.txt'], + ['file', 'hide', 'b2://my-bucket/file1.txt'], expected_json_in_stdout=expected_json, ) @@ -2144,9 +2144,9 @@ def test_get_bucket_with_hidden(self): stdout, stderr = self._get_stdouterr() console_tool = self.console_tool_class(stdout, stderr) console_tool.run_command(['b2', 'file', 'hide', 'b2://my-bucket/hidden1']) - console_tool.run_command(['b2', 'file', 'hide', 'my-bucket', 'hidden2']) - console_tool.run_command(['b2', 'file', 'hide', 'my-bucket', 'hidden3']) - console_tool.run_command(['b2', 'hide-file', 'my-bucket', 'hidden4']) + console_tool.run_command(['b2', 'file', 'hide', 'b2://my-bucket/hidden2']) + console_tool.run_command(['b2', 'file', 'hide', 'b2://my-bucket/hidden3']) + console_tool.run_command(['b2', 'file', 'hide', 'b2://my-bucket/hidden4']) # unhide one file console_tool.run_command(['b2', 'file', 'unhide', 'b2://my-bucket/hidden2']) @@ -2207,13 +2207,11 @@ def test_get_bucket_complex(self): # something has failed if the output of 'bucket get' does not match the canon. stdout, stderr = self._get_stdouterr() console_tool = self.console_tool_class(stdout, stderr) - console_tool.run_command(['b2', 'file', 'hide', 'b2://my-bucket/1/hidden1']) - console_tool.run_command(['b2', 'file', 'hide', 'my-bucket', '1/hidden1']) + for _ in range(2): + console_tool.run_command(['b2', 'file', 'hide', 'b2://my-bucket/hidden1']) console_tool.run_command(['b2', 'file', 'hide', 'b2://my-bucket/1/hidden2']) - console_tool.run_command(['b2', 'file', 'hide', 'my-bucket', '1/2/hidden3']) - console_tool.run_command(['b2', 'file', 'hide', 'my-bucket', '1/2/hidden3']) - console_tool.run_command(['b2', 'file', 'hide', 'my-bucket', '1/2/hidden3']) - console_tool.run_command(['b2', 'file', 'hide', 'my-bucket', '1/2/hidden3']) + for _ in range(4): + console_tool.run_command(['b2', 'file', 'hide', 'b2://my-bucket/1/2/hidden3']) # Unhide a file console_tool.run_command(['b2', 'file', 'unhide', 'b2://my-bucket/1/hidden2'])