fix: Update generation logic (#5604)

* Fix generation logic * Simplify param type to list * Satisfy linter --------- Co-authored-by: Leonardo Gama <[email protected]>
aws · Jul 25, 2023 · 1e7fcdd · 1e7fcdd
1 parent 6d440eb
commit 1e7fcdd
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 39 deletions.
diff --git a/schema/schema.py → schema/make_schema.py b/schema/schema.py → schema/make_schema.py
@@ -5,7 +5,7 @@
 import json
 from dataclasses import dataclass
 from enum import Enum
-from typing import Any, Dict, List, Optional
+from typing import Any, Dict, List, Optional, Union
 
 import click
 
@@ -20,6 +20,15 @@
 PARAMS_TO_OMIT_DEFAULT_FIELD = [
     "layer_cache_basedir"  # sets default to root directory to that of machine the schema is generated on
 ]
+EXCEPTION_OPTION_CLASS_MAPPER = {  # for params without proper class names, map them uniquely
+    "parameter_overrides": ["array", "string"],
+    "image_repository": ["string"],
+    "image_repositories": ["array"],
+    "metadata": ["string"],
+    "signing_profiles": ["string"],
+    "tags": ["array"],
+    "parameter": ["array"],
+}
 CHARS_TO_CLEAN = [
     "\b",  # backspaces
     "\u001b[0m",  # ANSI start bold
@@ -43,7 +52,7 @@ class SamCliParameterSchema:
     """
 
     name: str
-    type: str
+    type: Union[str, List[str]]
     description: str = ""
     default: Optional[Any] = None
     items: Optional[str] = None
@@ -147,25 +156,27 @@ def format_param(param: click.core.Option) -> SamCliParameterSchema:
     """
     if not param:
         raise SchemaGenerationException("Expected to format a parameter that doesn't exist")
-    if not param.type.name:
-        raise SchemaGenerationException(f"Parameter {param} passed without a type")
+    if not param.type.name and param.name not in EXCEPTION_OPTION_CLASS_MAPPER.keys():
+        raise SchemaGenerationException(f"Parameter {param} passed without a type:\n{param.type}")
 
     param_type = param.type.name.lower()
-    formatted_param_type = ""
+    formatted_param_types = []
     # NOTE: Params do not have explicit "string" type; either "text" or "path".
     #       All choice options are from a set of strings.
-    if param_type in ["text", "path", "choice", "filename", "directory"]:
-        formatted_param_type = "string"
+    if param.name in EXCEPTION_OPTION_CLASS_MAPPER.keys():
+        formatted_param_types = EXCEPTION_OPTION_CLASS_MAPPER[param.name]
+    elif param_type in ["text", "path", "choice", "filename", "directory"]:
+        formatted_param_types = ["string"]
     elif param_type == "list":
-        formatted_param_type = "array"
+        formatted_param_types = ["array"]
     else:
-        formatted_param_type = param_type or "string"
+        formatted_param_types = [param_type] or ["string"]
 
     formatted_param: SamCliParameterSchema = SamCliParameterSchema(
         param.name or "",
-        formatted_param_type,
+        formatted_param_types if len(formatted_param_types) > 1 else formatted_param_types[0],
         clean_text(param.help or ""),
-        items="string" if formatted_param_type == "array" else None,
+        items="string" if "array" in formatted_param_types else None,
     )
 
     if param.default and param.name not in PARAMS_TO_OMIT_DEFAULT_FIELD:

diff --git a/schema/samcli.json b/schema/samcli.json
@@ -313,8 +313,14 @@
                 },
                 "parameter_overrides": {
                   "title": "parameter_overrides",
-                  "type": "string",
-                  "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs."
+                  "type": [
+                    "array",
+                    "string"
+                  ],
+                  "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.",
+                  "items": {
+                    "type": "string"
+                  }
                 },
                 "skip_pull_image": {
                   "title": "skip_pull_image",
@@ -396,8 +402,14 @@
                 },
                 "parameter_overrides": {
                   "title": "parameter_overrides",
-                  "type": "string",
-                  "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs."
+                  "type": [
+                    "array",
+                    "string"
+                  ],
+                  "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.",
+                  "items": {
+                    "type": "string"
+                  }
                 },
                 "debug_port": {
                   "title": "debug_port",
@@ -548,8 +560,14 @@
                 },
                 "parameter_overrides": {
                   "title": "parameter_overrides",
-                  "type": "string",
-                  "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs."
+                  "type": [
+                    "array",
+                    "string"
+                  ],
+                  "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.",
+                  "items": {
+                    "type": "string"
+                  }
                 },
                 "debug_port": {
                   "title": "debug_port",
@@ -723,8 +741,14 @@
                 },
                 "parameter_overrides": {
                   "title": "parameter_overrides",
-                  "type": "string",
-                  "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs."
+                  "type": [
+                    "array",
+                    "string"
+                  ],
+                  "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.",
+                  "items": {
+                    "type": "string"
+                  }
                 },
                 "debug_port": {
                   "title": "debug_port",
@@ -871,8 +895,11 @@
                 },
                 "image_repositories": {
                   "title": "image_repositories",
-                  "type": "string",
-                  "description": "Mapping of Function Logical ID to AWS ECR Repository URI.\n\nExample: Function_Logical_ID=ECR_Repo_Uri\nThis option can be specified multiple times."
+                  "type": "array",
+                  "description": "Mapping of Function Logical ID to AWS ECR Repository URI.\n\nExample: Function_Logical_ID=ECR_Repo_Uri\nThis option can be specified multiple times.",
+                  "items": {
+                    "type": "string"
+                  }
                 },
                 "s3_prefix": {
                   "title": "s3_prefix",
@@ -1010,8 +1037,11 @@
                 },
                 "image_repositories": {
                   "title": "image_repositories",
-                  "type": "string",
-                  "description": "Mapping of Function Logical ID to AWS ECR Repository URI.\n\nExample: Function_Logical_ID=ECR_Repo_Uri\nThis option can be specified multiple times."
+                  "type": "array",
+                  "description": "Mapping of Function Logical ID to AWS ECR Repository URI.\n\nExample: Function_Logical_ID=ECR_Repo_Uri\nThis option can be specified multiple times.",
+                  "items": {
+                    "type": "string"
+                  }
                 },
                 "force_upload": {
                   "title": "force_upload",
@@ -1063,13 +1093,22 @@
                 },
                 "tags": {
                   "title": "tags",
-                  "type": "string",
-                  "description": "List of tags to associate with the stack."
+                  "type": "array",
+                  "description": "List of tags to associate with the stack.",
+                  "items": {
+                    "type": "string"
+                  }
                 },
                 "parameter_overrides": {
                   "title": "parameter_overrides",
-                  "type": "string",
-                  "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs."
+                  "type": [
+                    "array",
+                    "string"
+                  ],
+                  "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.",
+                  "items": {
+                    "type": "string"
+                  }
                 },
                 "signing_profiles": {
                   "title": "signing_profiles",

diff --git a/tests/unit/schema/test_schema_logic.py b/tests/unit/schema/test_schema_logic.py
@@ -1,11 +1,10 @@
 from typing import List
-from unittest.mock import MagicMock, Mock, patch
-import click
+from unittest.mock import MagicMock, patch
 from parameterized import parameterized
 from unittest import TestCase
 from schema.exceptions import SchemaGenerationException
 
-from schema.schema import (
+from schema.make_schema import (
     SamCliCommandSchema,
     SamCliParameterSchema,
     SchemaKeys,
@@ -121,7 +120,7 @@ def test_param_formatted_throws_error_when_none(self):
             ("choice", SamCliParameterSchema("p_name", "string", default=["default", "value"], choices=["1", "2"])),
         ]
     )
-    @patch("schema.schema.isinstance")
+    @patch("schema.make_schema.isinstance")
     def test_param_formatted_given_type(self, param_type, expected_param, isinstance_mock):
         mock_param = MagicMock()
         mock_param.name = "p_name"
@@ -135,8 +134,8 @@ def test_param_formatted_given_type(self, param_type, expected_param, isinstance
 
         self.assertEqual(expected_param, formatted_param)
 
-    @patch("schema.schema.isinstance")
-    @patch("schema.schema.format_param")
+    @patch("schema.make_schema.isinstance")
+    @patch("schema.make_schema.format_param")
     def test_getting_params_from_cli_object(self, format_param_mock, isinstance_mock):
         mock_cli = MagicMock()
         mock_cli.params = []
@@ -154,8 +153,8 @@ def test_getting_params_from_cli_object(self, format_param_mock, isinstance_mock
         self.assertNotIn("config_file", params)
         self.assertNotIn(None, params)
 
-    @patch("schema.schema.importlib.import_module")
-    @patch("schema.schema.get_params_from_command")
+    @patch("schema.make_schema.importlib.import_module")
+    @patch("schema.make_schema.get_params_from_command")
     def test_command_structure_is_retrieved(self, get_params_mock, import_mock):
         mock_module = self._setup_mock_module()
         import_mock.side_effect = lambda _: mock_module
@@ -165,9 +164,9 @@ def test_command_structure_is_retrieved(self, get_params_mock, import_mock):
 
         self._validate_retrieved_command_structure(commands)
 
-    @patch("schema.schema.importlib.import_module")
-    @patch("schema.schema.get_params_from_command")
-    @patch("schema.schema.isinstance")
+    @patch("schema.make_schema.importlib.import_module")
+    @patch("schema.make_schema.get_params_from_command")
+    @patch("schema.make_schema.isinstance")
     def test_command_structure_is_retrieved_from_group_cli(self, isinstance_mock, get_params_mock, import_mock):
         mock_module = self._setup_mock_module()
         mock_module.cli.commands = {}
@@ -183,7 +182,7 @@ def test_command_structure_is_retrieved_from_group_cli(self, isinstance_mock, ge
 
         self._validate_retrieved_command_structure(commands)
 
-    @patch("schema.schema.retrieve_command_structure")
+    @patch("schema.make_schema.retrieve_command_structure")
     def test_schema_is_generated_properly(self, retrieve_commands_mock):
         def mock_retrieve_commands(package_name, counter=[0]):
             counter[0] += 1