Merge pull request #1034 from reef-technologies/fix_ls_file

fix `b2 ls b2://bucketName/fileName` and same for `rm`

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

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)

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

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}
 
@@ -92,6 +92,24 @@ class Ls(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)

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)
-        """
+    """

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
@@ -3433,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
 

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.

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.

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'",

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
 

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'])
 

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."""

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='',
+    )

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())

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"