diff --git a/src/snowflake/cli/_plugins/nativeapp/version/version_processor.py b/src/snowflake/cli/_plugins/nativeapp/version/version_processor.py index 274c82357d..159d2b6303 100644 --- a/src/snowflake/cli/_plugins/nativeapp/version/version_processor.py +++ b/src/snowflake/cli/_plugins/nativeapp/version/version_processor.py @@ -39,10 +39,9 @@ from snowflake.cli.api.console import cli_console as cc from snowflake.cli.api.exceptions import SnowflakeSQLExecutionError from snowflake.cli.api.project.schemas.native_app.native_app import NativeApp -from snowflake.cli.api.project.util import unquote_identifier +from snowflake.cli.api.project.util import to_identifier, unquote_identifier from snowflake.cli.api.utils.cursor import ( find_all_rows, - find_first_row, ) from snowflake.connector import ProgrammingError from snowflake.connector.cursor import DictCursor @@ -121,6 +120,8 @@ def add_new_version(self, version: str) -> None: """ Defines a new version in an existing application package. """ + # Make the version a valid identifier, adding quotes if necessary + version = to_identifier(version) with self.use_role(self.package_role): cc.step( f"Defining a new version {version} in application package {self.package_name}" @@ -141,6 +142,8 @@ def add_new_patch_to_version(self, version: str, patch: Optional[int] = None): """ Add a new patch, optionally a custom one, to an existing version in an application package. """ + # Make the version a valid identifier, adding quotes if necessary + version = to_identifier(version) with self.use_role(self.package_role): cc.step( f"Adding new patch to version {version} defined in application package {self.package_name}" @@ -156,11 +159,7 @@ def add_new_patch_to_version(self, version: str, patch: Optional[int] = None): add_version_query, cursor_class=DictCursor ) - show_row = find_first_row( - result_cursor, - lambda row: row[VERSION_COL] == unquote_identifier(version), - ) - + show_row = result_cursor.fetchall()[0] new_patch = show_row["patch"] cc.message( f"Patch {new_patch} created for version {version} defined in application package {self.package_name}." @@ -328,6 +327,9 @@ def process( "Manifest.yml file does not contain a value for the version field." ) + # Make the version a valid identifier, adding quotes if necessary + version = to_identifier(version) + cc.step( dedent( f"""\ diff --git a/tests/nativeapp/test_version_create_processor.py b/tests/nativeapp/test_version_create_processor.py index 79fb506b10..b418addc5d 100644 --- a/tests/nativeapp/test_version_create_processor.py +++ b/tests/nativeapp/test_version_create_processor.py @@ -105,8 +105,11 @@ def test_get_existing_release_direction_info(mock_execute, temp_dir, mock_cursor # Test add_new_version adds a new version to an app pkg correctly @mock.patch(NATIVEAPP_MANAGER_EXECUTE) -def test_add_version(mock_execute, temp_dir, mock_cursor): - version = "V1" +@pytest.mark.parametrize( + ["version", "version_identifier"], + [("V1", "V1"), ("1.0.0", '"1.0.0"'), ('"1.0.0"', '"1.0.0"')], +) +def test_add_version(mock_execute, temp_dir, mock_cursor, version, version_identifier): side_effects, expected = mock_execute_helper( [ ( @@ -120,7 +123,7 @@ def test_add_version(mock_execute, temp_dir, mock_cursor): dedent( f"""\ alter application package app_pkg - add version V1 + add version {version_identifier} using @app_pkg.app_src.stage """ ), @@ -146,8 +149,13 @@ def test_add_version(mock_execute, temp_dir, mock_cursor): # Test add_new_patch_to_version adds an "auto-increment" patch to an existing version @mock.patch(NATIVEAPP_MANAGER_EXECUTE) -def test_add_new_patch_auto(mock_execute, temp_dir, mock_cursor): - version = "V1" +@pytest.mark.parametrize( + ["version", "version_identifier"], + [("V1", "V1"), ("1.0.0", '"1.0.0"'), ('"1.0.0"', '"1.0.0"')], +) +def test_add_new_patch_auto( + mock_execute, temp_dir, mock_cursor, version, version_identifier +): side_effects, expected = mock_execute_helper( [ ( @@ -161,7 +169,7 @@ def test_add_new_patch_auto(mock_execute, temp_dir, mock_cursor): dedent( f"""\ alter application package app_pkg - add patch for version V1 + add patch for version {version_identifier} using @app_pkg.app_src.stage """ ), @@ -187,8 +195,13 @@ def test_add_new_patch_auto(mock_execute, temp_dir, mock_cursor): # Test add_new_patch_to_version adds a custom patch to an existing version @mock.patch(NATIVEAPP_MANAGER_EXECUTE) -def test_add_new_patch_custom(mock_execute, temp_dir, mock_cursor): - version = "V1" +@pytest.mark.parametrize( + ["version", "version_identifier"], + [("V1", "V1"), ("1.0.0", '"1.0.0"'), ('"1.0.0"', '"1.0.0"')], +) +def test_add_new_patch_custom( + mock_execute, temp_dir, mock_cursor, version, version_identifier +): side_effects, expected = mock_execute_helper( [ ( @@ -202,7 +215,7 @@ def test_add_new_patch_custom(mock_execute, temp_dir, mock_cursor): dedent( f"""\ alter application package app_pkg - add patch 12 for version V1 + add patch 12 for version {version_identifier} using @app_pkg.app_src.stage """ ), @@ -222,7 +235,7 @@ def test_add_new_patch_custom(mock_execute, temp_dir, mock_cursor): ) processor = _get_version_create_processor() - processor.add_new_patch_to_version(version, "12") + processor.add_new_patch_to_version(version, 12) assert mock_execute.mock_calls == expected @@ -282,7 +295,7 @@ def test_process_no_version_exists_throws_bad_option_exception_one( processor.process( bundle_map=mock_bundle_map, version="v1", - patch="12", + patch=12, policy=policy_param, git_policy=policy_param, is_interactive=False, @@ -315,7 +328,7 @@ def test_process_no_version_exists_throws_bad_option_exception_two( processor.process( bundle_map=mock_bundle_map, version="v1", - patch="12", + patch=12, policy=policy_param, git_policy=policy_param, is_interactive=False, diff --git a/tests/nativeapp/test_version_drop_processor.py b/tests/nativeapp/test_version_drop_processor.py index 61e7cb6161..9136f25d21 100644 --- a/tests/nativeapp/test_version_drop_processor.py +++ b/tests/nativeapp/test_version_drop_processor.py @@ -96,13 +96,13 @@ def test_process_has_no_existing_app_pkg(mock_get_existing, policy_param, temp_d def test_process_no_version_from_user_no_version_in_manifest( mock_version_info_in_manifest, mock_build_bundle, - mock_mismatch, + mock_distribution, mock_get_existing, policy_param, temp_dir, ): - mock_mismatch.return_Value = "internal" + mock_distribution.return_value = "internal" current_working_directory = os.getcwd() create_named_file( file_name="snowflake.yml", @@ -144,7 +144,7 @@ def test_process_drop_cannot_complete( mock_typer_confirm, mock_version_info_in_manifest, mock_build_bundle, - mock_mismatch, + mock_distribution, mock_get_existing, policy_param, is_interactive_param, @@ -152,7 +152,7 @@ def test_process_drop_cannot_complete( temp_dir, ): - mock_mismatch.return_value = "internal" + mock_distribution.return_value = "internal" current_working_directory = os.getcwd() create_named_file( file_name="snowflake.yml", @@ -190,12 +190,12 @@ def test_process_drop_cannot_complete( (ask_always_policy, True), ], ) -def test_process_drop_success( +def test_process_drop_from_manifest( mock_typer_confirm, mock_execute, mock_version_info_in_manifest, mock_build_bundle, - mock_mismatch, + mock_distribution, mock_get_existing, policy_param, is_interactive_param, @@ -203,7 +203,7 @@ def test_process_drop_success( mock_cursor, ): - mock_mismatch.return_value = "internal" + mock_distribution.return_value = "internal" side_effects, expected = mock_execute_helper( [ ( @@ -234,3 +234,72 @@ def test_process_drop_success( version=None, policy=policy_param, is_interactive=is_interactive_param ) assert mock_execute.mock_calls == expected + + +@mock.patch( + f"{VERSION_MODULE}.{DROP_PROCESSOR}.get_existing_app_pkg_info", + return_value={"owner": "package_role"}, +) +@mock_get_app_pkg_distribution_in_sf() +@mock.patch(f"{VERSION_MODULE}.{DROP_PROCESSOR}.build_bundle", return_value=None) +@mock.patch(NATIVEAPP_MANAGER_EXECUTE) +@mock.patch( + f"snowflake.cli._plugins.nativeapp.policy.{TYPER_CONFIRM}", return_value=True +) +@pytest.mark.parametrize( + "policy_param, is_interactive_param", + [ + (allow_always_policy, False), + (ask_always_policy, True), + (ask_always_policy, True), + ], +) +@pytest.mark.parametrize( + ["version", "version_identifier"], + [("V1", "V1"), ("1.0.0", '"1.0.0"'), ('"1.0.0"', '"1.0.0"')], +) +def test_process_drop_specific_version( + mock_typer_confirm, + mock_execute, + mock_build_bundle, + mock_distribution, + mock_get_existing, + policy_param, + is_interactive_param, + temp_dir, + mock_cursor, + version, + version_identifier, +): + + mock_distribution.return_value = "internal" + side_effects, expected = mock_execute_helper( + [ + ( + mock_cursor([{"CURRENT_ROLE()": "old_role"}], []), + mock.call("select current_role()", cursor_class=DictCursor), + ), + (None, mock.call("use role package_role")), + ( + None, + mock.call( + f"alter application package app_pkg drop version {version_identifier}" + ), + ), + (None, mock.call("use role old_role")), + ] + ) + mock_execute.side_effect = side_effects + + current_working_directory = os.getcwd() + create_named_file( + file_name="snowflake.yml", + dir_name=current_working_directory, + contents=[mock_snowflake_yml_file], + ) + + processor = _get_version_drop_processor() + processor.process( + version=version, policy=policy_param, is_interactive=is_interactive_param + ) + assert mock_execute.mock_calls == expected