Skip to content

Commit

Permalink
fix b2 ls b2://bucketName/fileName and same for rm
Browse files Browse the repository at this point in the history
  • Loading branch information
mjurbanski-reef committed May 15, 2024
1 parent 0d0878b commit 857b513
Show file tree
Hide file tree
Showing 11 changed files with 106 additions and 20 deletions.
1 change: 1 addition & 0 deletions b2/_internal/_cli/argcompleters.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions b2/_internal/_utils/uri.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
4 changes: 2 additions & 2 deletions b2/_internal/b2v3/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -45,7 +45,7 @@ def main() -> None:
os._exit(exit_status)


class Ls(B2URIBucketNFolderNameArgMixin, BaseLs):
class Ls(B2URIMustPointToFolderMixin, B2URIBucketNFolderNameArgMixin, BaseLs):
"""
{BaseLs}
Expand Down
29 changes: 27 additions & 2 deletions b2/_internal/b2v3/rm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -57,4 +82,4 @@ class Rm(B2URIBucketNFolderNameArgMixin, BaseRm):
- **listFiles**
- **deleteFiles**
- **bypassGovernance** (if --bypass-governance is used)
"""
"""
3 changes: 2 additions & 1 deletion b2/_internal/console_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions changelog.d/+ls_file.fixed.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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'",
Expand Down
7 changes: 5 additions & 2 deletions test/integration/test_b2_command_line.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down Expand Up @@ -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'])

Expand Down
5 changes: 5 additions & 0 deletions test/unit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down
41 changes: 32 additions & 9 deletions test/unit/console_tool/test_ls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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='',
)
27 changes: 27 additions & 0 deletions test/unit/console_tool/test_rm.py
Original file line number Diff line number Diff line change
@@ -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())

0 comments on commit 857b513

Please sign in to comment.