From b87909bd958f17d5c14dbda42106191959e53895 Mon Sep 17 00:00:00 2001 From: Nicholas McCrea Date: Thu, 27 Mar 2025 12:43:57 +0000 Subject: [PATCH 1/4] feat: Support upload of extra files --- cloudsmith_cli/cli/commands/push.py | 76 ++++++-- cloudsmith_cli/cli/tests/test_push.py | 245 ++++++++++++++++++++++++++ cloudsmith_cli/cli/validators.py | 31 ++++ 3 files changed, 333 insertions(+), 19 deletions(-) create mode 100644 cloudsmith_cli/cli/tests/test_push.py diff --git a/cloudsmith_cli/cli/commands/push.py b/cloudsmith_cli/cli/commands/push.py index 050b2e4e..e7e9474e 100644 --- a/cloudsmith_cli/cli/commands/push.py +++ b/cloudsmith_cli/cli/commands/push.py @@ -352,17 +352,33 @@ def upload_files_and_create_package( # 2. Validate file upload parameters md5_checksums = {} for k, v in kwargs.items(): - if not v or not k.endswith("_file"): + if not v: continue - md5_checksums[k] = validate_upload_file( - ctx=ctx, - opts=opts, - owner=owner, - repo=repo, - filepath=v, - skip_errors=skip_errors, - ) + # Handle a single file + if k.endswith("_file"): + md5_checksums[k] = validate_upload_file( + ctx=ctx, + opts=opts, + owner=owner, + repo=repo, + filepath=v, + skip_errors=skip_errors, + ) + + # Check if the key is "extra_files" (to handle multiple files) + if k == "extra_files" and isinstance(v, list): + md5_checksums[k] = [ + validate_upload_file( + ctx=ctx, + opts=opts, + owner=owner, + repo=repo, + filepath=file, + skip_errors=skip_errors, + ) + for file in v + ] if dry_run: click.echo() @@ -371,18 +387,35 @@ def upload_files_and_create_package( # 3. Upload any arguments that look like files for k, v in kwargs.items(): - if not v or not k.endswith("_file"): + if not v: continue - kwargs[k] = upload_file( - ctx=ctx, - opts=opts, - owner=owner, - repo=repo, - filepath=v, - skip_errors=skip_errors, - md5_checksum=md5_checksums[k], - ) + # Handle a single file + if k.endswith("_file"): + kwargs[k] = upload_file( + ctx=ctx, + opts=opts, + owner=owner, + repo=repo, + filepath=v, + skip_errors=skip_errors, + md5_checksum=md5_checksums[k], + ) + + # Check if the key is "extra_files" (to handle multiple files) + if k == "extra_files" and isinstance(v, list): + kwargs[k] = [ + upload_file( + ctx=ctx, + opts=opts, + owner=owner, + repo=repo, + filepath=file, + skip_errors=skip_errors, + md5_checksum=md5_checksums[k][idx], + ) + for idx, file in enumerate(v) + ] # 4. Create the package with package files and additional arguments _, slug = create_package( @@ -531,6 +564,11 @@ def push_handler(ctx, *args, **kwargs): option_kwargs["type"] = ExpandPath( dir_okay=False, exists=True, writable=False, resolve_path=True ) + elif k == "extra_files": + # Handle mutliple files for extra_files parameter. + option_kwargs["type"] = str + option_kwargs["multiple"] = True + option_kwargs["callback"] = validators.validate_extra_files_parameter elif info["type"] == "bool": option_name_fmt = "--%(key)s/--no-%(key)s" option_kwargs["is_flag"] = True diff --git a/cloudsmith_cli/cli/tests/test_push.py b/cloudsmith_cli/cli/tests/test_push.py new file mode 100644 index 00000000..247e62f9 --- /dev/null +++ b/cloudsmith_cli/cli/tests/test_push.py @@ -0,0 +1,245 @@ +import unittest +from unittest.mock import MagicMock, patch + +from ..commands.push import upload_files_and_create_package + + +# pylint: disable=too-many-instance-attributes +class TestPush(unittest.TestCase): + def setUp(self): + self.mock_ctx = MagicMock() + self.mock_opts = MagicMock() + self.package_type = "test_format" + self.owner = "test_owner" + self.repo = "test_repo" + self.name = "test_package" + self.version = "1.0.0" + self.dry_run = False + self.no_wait_for_sync = False + self.wait_interval = 5.0 + self.skip_errors = False + self.sync_attempts = 3 + + def test_upload_files_and_create_package(self): + # Values passed in from the command line + input_kwargs = { + "package_file": "package/file/path", + "name": "test_package", + "version": "1.0.0", + } + + # Predefine file attributes in for testing + files = { + "package_file": { + "path": "package/file/path", + "checksum": "package_file_checksum", + "id": "package_file_identifier", + }, + } + + # Kwargs for package creation in final step, contain ids returned from the AWS S3 upload + create_package_kwargs = { + "package_file": files["package_file"]["id"], + "name": self.name, + "version": self.version, + } + + with ( + patch( + "cloudsmith_cli.cli.commands.push.validate_create_package" + ) as mock_validate_create_package, + patch( + "cloudsmith_cli.cli.commands.push.validate_upload_file" + ) as mock_validate_upload_file, + patch("cloudsmith_cli.cli.commands.push.upload_file") as mock_upload_file, + patch( + "cloudsmith_cli.cli.commands.push.create_package" + ) as mock_create_package, + patch("cloudsmith_cli.cli.commands.push.wait_for_package_sync"), + ): + # Validate upload returns checksums which we use to upload the files + mock_validate_upload_file.side_effect = [ + file["checksum"] for file in files.values() + ] + # Upload files returns files ids which we use to create the package + mock_upload_file.side_effect = [file["id"] for file in files.values()] + mock_create_package.return_value = ("", "test_package_slug") + + # 1. Call upload_files_and_create_package function + upload_files_and_create_package( + self.mock_ctx, + self.mock_opts, + self.package_type, + [self.owner, self.repo], + self.dry_run, + self.no_wait_for_sync, + self.wait_interval, + self.skip_errors, + self.sync_attempts, + **input_kwargs, + ) + + # 2. Confirm that validate_create_package was called with the correct arguments + mock_validate_create_package.assert_called_once_with( + ctx=self.mock_ctx, + opts=self.mock_opts, + owner=self.owner, + repo=self.repo, + package_type=self.package_type, + skip_errors=self.skip_errors, + **input_kwargs, + ) + + # 3. For each file, confirm that validate_upload_file and upload_file were called with the correct arguments + for file_data in files.values(): + mock_validate_upload_file.assert_any_call( + ctx=self.mock_ctx, + opts=self.mock_opts, + owner=self.owner, + repo=self.repo, + filepath=file_data["path"], + skip_errors=self.skip_errors, + ) + mock_upload_file.assert_any_call( + ctx=self.mock_ctx, + opts=self.mock_opts, + owner=self.owner, + repo=self.repo, + filepath=file_data["path"], + skip_errors=self.skip_errors, + md5_checksum=file_data["checksum"], + ) + + # 4. Validate that create_package was called once with the correct arguments + mock_create_package.assert_called_once_with( + ctx=self.mock_ctx, + opts=self.mock_opts, + owner=self.owner, + repo=self.repo, + package_type=self.package_type, + skip_errors=self.skip_errors, + **create_package_kwargs, + ) + + def test_upload_files_and_create_package_extra_files(self): + # Values passed in from the command line + input_kwargs = { + "package_file": "package/file/path", + "test_file": "test/file/path", + "extra_files": ["test/extra/file/path1", "test/extra/file/path2"], + "name": "test_package", + "version": "1.0.0", + } + + # Predefine file attributes in for testing + files = { + "package_file": { + "path": "package/file/path", + "checksum": "package_file_checksum", + "id": "package_file_identifier", + }, + "test_file": { + "path": "test/file/path", + "checksum": "test_file_checksum", + "id": "test_file_identifier", + }, + "extra_file1": { + "path": "test/extra/file/path1", + "checksum": "extra_file_checksum1", + "id": "extra_file_identifier1", + }, + "extra_file2": { + "path": "test/extra/file/path2", + "checksum": "extra_file_checksum2", + "id": "extra_file_identifier2", + }, + } + + # Kwargs for package creation in final step, contain ids returned from the AWS S3 upload + create_package_kwargs = { + "package_file": files["package_file"]["id"], + "test_file": files["test_file"]["id"], + "extra_files": [ + files["extra_file1"]["id"], + files["extra_file2"]["id"], + ], + "name": self.name, + "version": self.version, + } + + with ( + patch( + "cloudsmith_cli.cli.commands.push.validate_create_package" + ) as mock_validate_create_package, + patch( + "cloudsmith_cli.cli.commands.push.validate_upload_file" + ) as mock_validate_upload_file, + patch("cloudsmith_cli.cli.commands.push.upload_file") as mock_upload_file, + patch( + "cloudsmith_cli.cli.commands.push.create_package" + ) as mock_create_package, + patch("cloudsmith_cli.cli.commands.push.wait_for_package_sync"), + ): + # Validate upload returns checksums which we use to upload the files + mock_validate_upload_file.side_effect = [ + file["checksum"] for file in files.values() + ] + # Upload files returns files ids which we use to create the package + mock_upload_file.side_effect = [file["id"] for file in files.values()] + mock_create_package.return_value = ("", "test_package_slug") + + # 1. Call upload_files_and_create_package function + upload_files_and_create_package( + self.mock_ctx, + self.mock_opts, + self.package_type, + [self.owner, self.repo], + self.dry_run, + self.no_wait_for_sync, + self.wait_interval, + self.skip_errors, + self.sync_attempts, + **input_kwargs, + ) + + # 2. Confirm that validate_create_package was called with the correct arguments + mock_validate_create_package.assert_called_once_with( + ctx=self.mock_ctx, + opts=self.mock_opts, + owner=self.owner, + repo=self.repo, + package_type=self.package_type, + skip_errors=self.skip_errors, + **input_kwargs, + ) + + # 3. For each file, confirm that validate_upload_file and upload_file were called with the correct arguments + for file_data in files.values(): + mock_validate_upload_file.assert_any_call( + ctx=self.mock_ctx, + opts=self.mock_opts, + owner=self.owner, + repo=self.repo, + filepath=file_data["path"], + skip_errors=self.skip_errors, + ) + mock_upload_file.assert_any_call( + ctx=self.mock_ctx, + opts=self.mock_opts, + owner=self.owner, + repo=self.repo, + filepath=file_data["path"], + skip_errors=self.skip_errors, + md5_checksum=file_data["checksum"], + ) + + # 4. Validate that create_package was called once with the correct arguments + mock_create_package.assert_called_once_with( + ctx=self.mock_ctx, + opts=self.mock_opts, + owner=self.owner, + repo=self.repo, + package_type=self.package_type, + skip_errors=self.skip_errors, + **create_package_kwargs, + ) diff --git a/cloudsmith_cli/cli/validators.py b/cloudsmith_cli/cli/validators.py index cc4016d9..977a50d3 100644 --- a/cloudsmith_cli/cli/validators.py +++ b/cloudsmith_cli/cli/validators.py @@ -5,6 +5,8 @@ import click +from .types import ExpandPath + CONTEXT_SETTINGS = dict(help_option_names=["-h", "--help"]) BAD_API_HEADERS = ("user-agent", "host") API_HEADER_TRANSFORMS = {} @@ -230,3 +232,32 @@ def validate_scheduled_reset_period(ctx, param, value): ) return value + + +def validate_extra_files_parameter(ctx, param, value): + """Validate and resolve paths for all extra files.""" + + if not value: + return [] + + path_obj = ExpandPath( + exists=True, + dir_okay=False, + writable=False, + resolve_path=True, + ) + + files = [] + for v in value: + for path in v.split(","): + path = path.strip() + if not path: + continue + + try: + resolved_path = path_obj.convert(path, param, ctx) + files.append(resolved_path) + except click.BadParameter as e: + raise click.BadParameter(f"Invalid file path '{path}': {e}") + + return files From 291ccc45dfc9ebde8e3a43165b8eabb834fd6f53 Mon Sep 17 00:00:00 2001 From: Nicholas McCrea Date: Fri, 28 Mar 2025 12:10:18 +0000 Subject: [PATCH 2/4] Improve help text description. --- cloudsmith_cli/cli/commands/push.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cloudsmith_cli/cli/commands/push.py b/cloudsmith_cli/cli/commands/push.py index e7e9474e..fd32ebbb 100644 --- a/cloudsmith_cli/cli/commands/push.py +++ b/cloudsmith_cli/cli/commands/push.py @@ -569,6 +569,9 @@ def push_handler(ctx, *args, **kwargs): option_kwargs["type"] = str option_kwargs["multiple"] = True option_kwargs["callback"] = validators.validate_extra_files_parameter + info["help"] = ( + info["help"] + " Accepts a comma-separated list of values." + ) elif info["type"] == "bool": option_name_fmt = "--%(key)s/--no-%(key)s" option_kwargs["is_flag"] = True From 22a19dacfccadff3757bc36e0d225e921ba2aa1d Mon Sep 17 00:00:00 2001 From: Nicholas McCrea Date: Mon, 31 Mar 2025 11:19:18 +0100 Subject: [PATCH 3/4] Bump cli version. Update changelog. Bump api version. --- .bumpversion.cfg | 2 +- CHANGELOG.md | 6 ++++++ cloudsmith_cli/data/VERSION | 2 +- setup.py | 2 +- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/.bumpversion.cfg b/.bumpversion.cfg index 970bf9c1..b1f91bb1 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 1.6.2 +current_version = 1.7.0 commit = True tag = True parse = (?P\d+)\.(?P\d+)\.(?P\d+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec1e0f98..c09f39bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +## [1.7.0] - 2025-03-31 + +### Added + +- Added `--extra-files` parameter for Maven upload command ([#190](https://github.com/cloudsmith-io/cloudsmith-cli/pull/190)) + ## [1.6.2] - 2025-03-27 - Added html templates for saml response endpoints diff --git a/cloudsmith_cli/data/VERSION b/cloudsmith_cli/data/VERSION index fdd3be6d..bd8bf882 100644 --- a/cloudsmith_cli/data/VERSION +++ b/cloudsmith_cli/data/VERSION @@ -1 +1 @@ -1.6.2 +1.7.0 diff --git a/setup.py b/setup.py index 10242128..07a7481b 100644 --- a/setup.py +++ b/setup.py @@ -53,7 +53,7 @@ def get_long_description(): "click-configfile>=0.2.3", "click-didyoumean>=0.0.3", "click-spinner>=0.1.7", - "cloudsmith-api>=2.0.16,<3.0", # Compatible upto (but excluding) 3.0+ + "cloudsmith-api>=2.0.17,<3.0", # Compatible upto (but excluding) 3.0+ "keyring>=25.4.1", "requests>=2.18.4", "requests_toolbelt>=0.8.0", From fa7cca8d262376843af7e45d4ede8f300c89b589 Mon Sep 17 00:00:00 2001 From: Nicholas McCrea Date: Mon, 31 Mar 2025 11:35:20 +0100 Subject: [PATCH 4/4] Fix typo --- cloudsmith_cli/cli/commands/push.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudsmith_cli/cli/commands/push.py b/cloudsmith_cli/cli/commands/push.py index fd32ebbb..c5e74125 100644 --- a/cloudsmith_cli/cli/commands/push.py +++ b/cloudsmith_cli/cli/commands/push.py @@ -565,7 +565,7 @@ def push_handler(ctx, *args, **kwargs): dir_okay=False, exists=True, writable=False, resolve_path=True ) elif k == "extra_files": - # Handle mutliple files for extra_files parameter. + # Handle multiple files for extra_files parameter. option_kwargs["type"] = str option_kwargs["multiple"] = True option_kwargs["callback"] = validators.validate_extra_files_parameter