Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix b2 ls b2://bucketName/fileName and same for rm #294

Merged
merged 3 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
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
22 changes: 20 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 Expand Up @@ -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)
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)
"""
"""
10 changes: 3 additions & 7 deletions 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 Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions changelog.d/+hyphen_filename.fixed.md
Original file line number Diff line number Diff line change
@@ -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.
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.
8 changes: 4 additions & 4 deletions pdm.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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
3 changes: 1 addition & 2 deletions test/integration/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

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())
38 changes: 33 additions & 5 deletions test/unit/console_tool/test_upload_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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))
Expand All @@ -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"
Expand Down
Loading