Treat 1.0 and 1.1 equally

snowflakedb · Sep 12, 2024 · d4c734e · d4c734e
1 parent d273736
commit d4c734e
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 27 deletions.
diff --git a/src/snowflake/cli/api/project/definition_conversion.py b/src/snowflake/cli/api/project/definition_conversion.py
@@ -20,9 +20,12 @@
 from snowflake.cli.api.project.schemas.entities.common import (
     SqlScriptHookType,
 )
-from snowflake.cli.api.project.schemas.native_app.application import Application
+from snowflake.cli.api.project.schemas.native_app.application import (
+    Application,
+    ApplicationV11,
+)
 from snowflake.cli.api.project.schemas.native_app.native_app import NativeApp
-from snowflake.cli.api.project.schemas.native_app.package import Package
+from snowflake.cli.api.project.schemas.native_app.package import Package, PackageV11
 from snowflake.cli.api.project.schemas.project_definition import (
     ProjectDefinition,
     ProjectDefinitionV2,
@@ -232,14 +235,17 @@ def _convert_package_script_files(package_scripts: list[str]):
         return post_deploy_hooks
 
     package_entity_name = "pkg"
-    if native_app.package and native_app.package.name:
+    if (
+        native_app.package
+        and native_app.package.name
+        and native_app.package.name != PackageV11.model_fields["name"].default
+    ):
         package_identifier = native_app.package.name
     else:
-        package_identifier = f"{native_app.name}_pkg"
-        if str(definition_version) == "1":
-            # PDFv1.0 doesn't have the username append in the definition object,
-            # it's done in the NativeAppProjectModel, so we have to emulate that here
-            package_identifier += f"_{_make_template('ctx.env.USERNAME')}"
+        # Backport the PackageV11 default name template, updated for PDFv2
+        package_identifier = _make_template(
+            f"fn.concat_ids('{native_app.name}', '_pkg_', fn.sanitize_id(fn.get_username('unknown_user')) | lower)"
+        )
     package = {
         "type": "application package",
         "identifier": package_identifier,
@@ -265,14 +271,17 @@ def _convert_package_script_files(package_scripts: list[str]):
             package["meta"] = package_meta
 
     app_entity_name = "app"
-    if native_app.application and native_app.application.name:
+    if (
+        native_app.application
+        and native_app.application.name
+        and native_app.application.name != ApplicationV11.model_fields["name"].default
+    ):
         app_identifier = native_app.application.name
     else:
-        app_identifier = native_app.name
-        if str(definition_version) == "1":
-            # PDFv1.0 doesn't have the username append in the definition object,
-            # it's done in the NativeAppProjectModel, so we have to emulate that here
-            app_identifier += f"_{_make_template('ctx.env.USERNAME')}"
+        # Backport the ApplicationV11 default name template, updated for PDFv2
+        app_identifier = _make_template(
+            f"fn.concat_ids('{native_app.name}', '_', fn.sanitize_id(fn.get_username('unknown_user')) | lower)"
+        )
     app = {
         "type": "application",
         "identifier": app_identifier,

diff --git a/tests/workspace/__snapshots__/test_manager.ambr b/tests/workspace/__snapshots__/test_manager.ambr
@@ -217,7 +217,8 @@
       deploy_root: output/deploy/
       distribution: external
       generated_root: __generated/
-      identifier: myapp_pkg_<% ctx.env.USERNAME %>
+      identifier: <% fn.concat_ids('myapp', '_pkg_', fn.sanitize_id(fn.get_username('unknown_user'))
+        | lower) %>
       manifest: app/manifest.yml
       meta:
         role: pkg_role

diff --git a/tests/workspace/test_manager.py b/tests/workspace/test_manager.py
@@ -22,10 +22,6 @@
 import pytest
 import yaml
 from snowflake.cli._plugins.workspace.manager import WorkspaceManager
-from snowflake.cli.api.constants import (
-    PROJECT_TEMPLATE_VARIABLE_CLOSING,
-    PROJECT_TEMPLATE_VARIABLE_OPENING,
-)
 from snowflake.cli.api.entities.common import EntityActions
 from snowflake.cli.api.exceptions import InvalidProjectDefinitionVersionError
 from snowflake.cli.api.project.definition_manager import DefinitionManager
@@ -227,26 +223,29 @@ def test_migrating_a_file_with_duplicated_keys_raises_an_error(
     assert result.output == os_agnostic_snapshot
 
 
-def test_migrate_nativeapp_fields_with_username_v1_definition(
-    runner, project_directory
+@pytest.mark.parametrize("definition_version", [1, "1.1"])
+def test_migrate_nativeapp_fields_with_username(
+    runner, project_directory, definition_version
 ):
     with project_directory("integration") as pd:
         definition_path = pd / "snowflake.yml"
-        with definition_path.open("r") as f:
+        with definition_path.open("r+") as f:
             old_definition = yaml.safe_load(f)
-        assert old_definition["definition_version"] == 1
+            old_definition["definition_version"] = definition_version
+            f.seek(0)
+            yaml.safe_dump(old_definition, f)
+            f.truncate()
 
-        result = runner.invoke(["ws", "migrate"])
+        result = runner.invoke(["ws", "migrate", "--accept-templates"])
         assert result.exit_code == 0, result.output
 
         with definition_path.open("r") as f:
             new_definition = yaml.safe_load(f)
-        username_suffix = f"_{PROJECT_TEMPLATE_VARIABLE_OPENING} ctx.env.USERNAME {PROJECT_TEMPLATE_VARIABLE_CLOSING}"
         assert (
             new_definition["entities"]["app"]["identifier"]
-            == f"integration{username_suffix}"
+            == "<% fn.concat_ids('integration', '_', fn.sanitize_id(fn.get_username('unknown_user')) | lower) %>"
         )
         assert (
             new_definition["entities"]["pkg"]["identifier"]
-            == f"integration_pkg{username_suffix}"
+            == "<% fn.concat_ids('integration', '_pkg_', fn.sanitize_id(fn.get_username('unknown_user')) | lower) %>"
         )