tobywf · Nov 21, 2019 · Nov 8, 2019 · Nov 19, 2019 · Nov 19, 2019 · Nov 20, 2019
diff --git a/python/rpdk/python/codegen.py b/python/rpdk/python/codegen.py
@@ -13,7 +13,7 @@
 from rpdk.core.jsonutils.resolver import ContainerType, resolve_models
 from rpdk.core.plugin_base import LanguagePlugin
 
-from .resolver import models_in_properties, translate_type
+from .resolver import translate_type
 
 LOG = logging.getLogger(__name__)
 
@@ -42,7 +42,6 @@ def __init__(self):
             trim_blocks=True, lstrip_blocks=True, keep_trailing_newline=True
         )
         self.env.filters["translate_type"] = translate_type
-        self.env.filters["models_in_properties"] = models_in_properties
         self.env.globals["ContainerType"] = ContainerType
         self.namespace = None
         self.package_name = None

diff --git a/python/rpdk/python/resolver.py b/python/rpdk/python/resolver.py
@@ -11,8 +11,14 @@
 
 def translate_type(resolved_type):
     if resolved_type.container == ContainerType.MODEL:
-        # we create type vars to make it easier to ref later models
-        return f"T{resolved_type.type}"
+        # quote types to ensure they can be referenced before they are declared.
+        # use alias (underscore) to avoid clashes with property names. there's
+        # an issue if a property has the same name as the type and it's used twice:
+        #   Memo: Optional["Memo"]
+        #   SecondCopyOfMemo: Optional["Memo"]  <-- mypy doesn't like this
+        # due to the schema, property names can't start with underscores, so
+        # the alias works well
+        return f'"_{resolved_type.type}"'
     if resolved_type.container == ContainerType.PRIMITIVE:
         return PRIMITIVE_TYPES[resolved_type.type]
 
@@ -27,14 +33,3 @@ def translate_type(resolved_type):
         return f"AbstractSet[{item_type}]"
 
     raise ValueError(f"Unknown container type {resolved_type.container}")
-
-
-def models_in_properties(properties):
-    # set for de-dupe, sorted for consistency
-    return sorted(
-        {
-            resolved_type.type
-            for resolved_type in properties.values()
-            if resolved_type.container == ContainerType.MODEL
-        }
-    )
diff --git a/python/rpdk/python/templates/handlers.py b/python/rpdk/python/templates/handlers.py
@@ -7,12 +7,11 @@
     OperationStatus,
     ProgressEvent,
     Resource,
-    ResourceHandlerRequest,
     SessionProxy,
     exceptions,
 )
 
-from .models import ResourceModel, TResourceModel
+from .models import ResourceHandlerRequest, ResourceModel
 
 # Use this logger to forward log messages to CloudWatch Logs.
 LOG = logging.getLogger(__name__)
@@ -24,11 +23,11 @@
 @resource.handler(Action.CREATE)
 def create_handler(
     session: Optional[SessionProxy],
-    request: ResourceHandlerRequest[TResourceModel],
+    request: ResourceHandlerRequest,
     callback_context: MutableMapping[str, Any],
-) -> ProgressEvent[TResourceModel]:
+) -> ProgressEvent:
     model = request.desiredResourceState
-    progress: ProgressEvent[TResourceModel] = ProgressEvent(
+    progress: ProgressEvent = ProgressEvent(
         status=OperationStatus.IN_PROGRESS,
         resourceModel=model,
     )
@@ -51,11 +50,11 @@ def create_handler(
 @resource.handler(Action.UPDATE)
 def update_handler(
     session: Optional[SessionProxy],
-    request: ResourceHandlerRequest[TResourceModel],
+    request: ResourceHandlerRequest,
     callback_context: MutableMapping[str, Any],
-) -> ProgressEvent[TResourceModel]:
+) -> ProgressEvent:
     model = request.desiredResourceState
-    progress: ProgressEvent[TResourceModel] = ProgressEvent(
+    progress: ProgressEvent = ProgressEvent(
         status=OperationStatus.IN_PROGRESS,
         resourceModel=model,
     )
@@ -66,11 +65,11 @@ def update_handler(
 @resource.handler(Action.DELETE)
 def delete_handler(
     session: Optional[SessionProxy],
-    request: ResourceHandlerRequest[TResourceModel],
+    request: ResourceHandlerRequest,
     callback_context: MutableMapping[str, Any],
-) -> ProgressEvent[TResourceModel]:
+) -> ProgressEvent:
     model = request.desiredResourceState
-    progress: ProgressEvent[TResourceModel] = ProgressEvent(
+    progress: ProgressEvent = ProgressEvent(
         status=OperationStatus.IN_PROGRESS,
         resourceModel=model,
     )
@@ -81,9 +80,9 @@ def delete_handler(
 @resource.handler(Action.READ)
 def read_handler(
     session: Optional[SessionProxy],
-    request: ResourceHandlerRequest[TResourceModel],
+    request: ResourceHandlerRequest,
     callback_context: MutableMapping[str, Any],
-) -> ProgressEvent[TResourceModel]:
+) -> ProgressEvent:
     model = request.desiredResourceState
     # TODO: put code here
     return ProgressEvent(
@@ -95,9 +94,9 @@ def read_handler(
 @resource.handler(Action.LIST)
 def list_handler(
     session: Optional[SessionProxy],
-    request: ResourceHandlerRequest[TResourceModel],
+    request: ResourceHandlerRequest,
     callback_context: MutableMapping[str, Any],
-) -> ProgressEvent[TResourceModel]:
+) -> ProgressEvent:
     # TODO: put code here
     return ProgressEvent(
         status=OperationStatus.SUCCESS,

diff --git a/python/rpdk/python/templates/models.py b/python/rpdk/python/templates/models.py
@@ -1,15 +1,3 @@
-{%- macro class_model_bindings(properties) -%}
-{%- set used_models = properties|models_in_properties -%}
-{%- if used_models -%}(
-{%- for name in used_models -%}
-Generic[T{{ name }}]{%- if not loop.last -%}, {%- endif -%}
-{%- endfor -%}
-){%- endif -%}
-{%- endmacro -%}
-{%- macro typevar_model_bindings(properties) -%}
-{%- set used_models = properties|models_in_properties -%}
-{%- if used_models -%}[{{ used_models|join(", ") }}]{%- endif %}
-{%- endmacro -%}
 # DO NOT modify this file by hand, changes will be overwritten
 from dataclasses import dataclass
 from typing import (
@@ -24,6 +12,11 @@
     TypeVar,
 )
 
+from cloudformation_cli_python_lib.interface import (
+    BaseResourceHandlerRequest,
+    BaseResourceModel,
+)
+
 T = TypeVar("T")
 
 
@@ -33,37 +26,42 @@ def set_or_none(value: Optional[Sequence[T]]) -> Optional[AbstractSet[T]]:
     return None
 
 
-{% for model, properties in models.items() %}
-T{{ model }} = TypeVar("T{{ model }}", bound="{{ model }}{{ typevar_model_bindings(properties) }}")
-{% endfor %}
-{% for model, properties in models.items() %}
+@dataclass
+class ResourceHandlerRequest(BaseResourceHandlerRequest):
+    # pylint: disable=invalid-name
+    desiredResourceState: Optional["ResourceModel"]
+    previousResourceState: Optional["ResourceModel"]
 
 
+{% for model, properties in models.items() %}
 @dataclass
-class {{ model }}{{ class_model_bindings(properties) }}:
+class {{ model }}{% if model == "ResourceModel" %}(BaseResourceModel){% endif %}:
     {% for name, type in properties.items() %}
     {{ name }}: Optional[{{ type|translate_type }}]
     {% endfor %}
 
-    def _serialize(self) -> Mapping[str, Any]:
-        return self.__dict__
-
     @classmethod
     def _deserialize(
-        cls: Type[T{{ model }}],
-        json: Mapping[str, Any],
-    ) -> Optional[T{{ model }}]:
-        if not json:
+        cls: Type["_{{ model }}"],
+        json_data: Optional[Mapping[str, Any]],
+    ) -> Optional["_{{ model }}"]:
+        if not json_data:
             return None
         return cls(
             {% for name, type in properties.items() %}
             {% if type.container == ContainerType.MODEL %}
-            {{ name }}={{ type.type }}._deserialize(json.get("{{ name }}")),  # type: ignore
+            {{ name }}={{ type.type }}._deserialize(json_data.get("{{ name }}")),
             {% elif type.container == ContainerType.SET %}
-            {{ name }}=set_or_none(json.get("{{ name }}")),
+            {{ name }}=set_or_none(json_data.get("{{ name }}")),
             {% else %}
-            {{ name }}=json.get("{{ name }}"),
+            {{ name }}=json_data.get("{{ name }}"),
             {% endif %}
             {% endfor %}
         )
-{% endfor %}
+
+
+# work around possible type aliasing issues when variable has same name as a model
+_{{ model }} = {{ model }}
+
+
+{% endfor -%}
diff --git a/src/cloudformation_cli_python_lib/__init__.py b/src/cloudformation_cli_python_lib/__init__.py
@@ -3,10 +3,10 @@
 from .boto3_proxy import SessionProxy  # noqa: F401
 from .interface import (  # noqa: F401
     Action,
+    BaseResourceHandlerRequest,
     HandlerErrorCode,
     OperationStatus,
     ProgressEvent,
-    ResourceHandlerRequest,
 )
 from .resource import Resource  # noqa: F401
 

diff --git a/src/cloudformation_cli_python_lib/exceptions.py b/src/cloudformation_cli_python_lib/exceptions.py
@@ -1,76 +1,76 @@
-from typing import Any, Generic
+from typing import Any
 
-from .interface import HandlerErrorCode, ProgressEvent, T
+from .interface import HandlerErrorCode, ProgressEvent
 
 
-class _HandlerError(Exception, Generic[T]):
+class _HandlerError(Exception):
     def __init__(self, *args: Any):
         self._error_code = HandlerErrorCode[type(self).__name__]
         super().__init__(*args)
 
-    def to_progress_event(self) -> ProgressEvent[T]:
+    def to_progress_event(self) -> ProgressEvent:
         return ProgressEvent.failed(self._error_code, str(self))
 
 
-class NotUpdatable(_HandlerError[T]):
+class NotUpdatable(_HandlerError):
     pass
 
 
-class InvalidRequest(_HandlerError[T]):
+class InvalidRequest(_HandlerError):
     pass
 
 
-class AccessDenied(_HandlerError[T]):
+class AccessDenied(_HandlerError):
     pass
 
 
-class InvalidCredentials(_HandlerError[T]):
+class InvalidCredentials(_HandlerError):
     pass
 
 
-class AlreadyExists(_HandlerError[T]):
+class AlreadyExists(_HandlerError):
     def __init__(self, type_name: str, identifier: str):
         super().__init__(
             f"Resource of type '{type_name}' with identifier "
             f"'{identifier}' already exists."
         )
 
 
-class NotFound(_HandlerError[T]):
+class NotFound(_HandlerError):
     def __init__(self, type_name: str, identifier: str):
         super().__init__(
             f"Resource of type '{type_name}' with identifier "
             f"'{identifier}' was not found."
         )
 
 
-class ResourceConflict(_HandlerError[T]):
+class ResourceConflict(_HandlerError):
     pass
 
 
-class Throttling(_HandlerError[T]):
+class Throttling(_HandlerError):
     pass
 
 
-class ServiceLimitExceeded(_HandlerError[T]):
+class ServiceLimitExceeded(_HandlerError):
     pass
 
 
-class NotStabilized(_HandlerError[T]):
+class NotStabilized(_HandlerError):
     pass
 
 
-class GeneralServiceException(_HandlerError[T]):
+class GeneralServiceException(_HandlerError):
     pass
 
 
-class ServiceInternalError(_HandlerError[T]):
+class ServiceInternalError(_HandlerError):
     pass
 
 
-class NetworkFailure(_HandlerError[T]):
+class NetworkFailure(_HandlerError):
     pass
 
 
-class InternalFailure(_HandlerError[T]):
+class InternalFailure(_HandlerError):
     pass
diff --git a/src/cloudformation_cli_python_lib/interface.py b/src/cloudformation_cli_python_lib/interface.py
@@ -1,12 +1,10 @@
 import logging
 from dataclasses import dataclass
 from enum import Enum, auto
-from typing import Any, Generic, List, Mapping, MutableMapping, Optional, Type, TypeVar
+from typing import Any, List, Mapping, MutableMapping, Optional, Type
 
 LOG = logging.getLogger(__name__)
 
-T = TypeVar("T")  # pylint: disable=invalid-name
-
 
 class _AutoName(Enum):
     @staticmethod
@@ -48,17 +46,28 @@ class HandlerErrorCode(str, _AutoName):
     InternalFailure = auto()
 
 
+class BaseResourceModel:
+    def _serialize(self) -> Mapping[str, Any]:
+        return self.__dict__
+
+    @classmethod
+    def _deserialize(
+        cls: Type["BaseResourceModel"], json_data: Optional[Mapping[str, Any]]
+    ) -> Optional["BaseResourceModel"]:
+        raise NotImplementedError()
+
+
 # pylint: disable=too-many-instance-attributes
 @dataclass
-class ProgressEvent(Generic[T]):
+class ProgressEvent:
     # pylint: disable=invalid-name
     status: OperationStatus
     errorCode: Optional[HandlerErrorCode] = None
     message: str = ""
     callbackContext: Optional[Mapping[str, Any]] = None
     callbackDelaySeconds: int = 0
-    resourceModel: Optional[T] = None
-    resourceModels: Optional[List[T]] = None
+    resourceModel: Optional[BaseResourceModel] = None
+    resourceModels: Optional[List[BaseResourceModel]] = None
     nextToken: Optional[str] = None
 
     def _serialize(
@@ -77,16 +86,16 @@ def _serialize(
 
     @classmethod
     def failed(
-        cls: Type["ProgressEvent[T]"], error_code: HandlerErrorCode, message: str
-    ) -> "ProgressEvent[T]":
+        cls: Type["ProgressEvent"], error_code: HandlerErrorCode, message: str
+    ) -> "ProgressEvent":
         return cls(status=OperationStatus.FAILED, errorCode=error_code, message=message)
 
 
 @dataclass
-class ResourceHandlerRequest(Generic[T]):
+class BaseResourceHandlerRequest:
     # pylint: disable=invalid-name
     clientRequestToken: str
-    desiredResourceState: Optional[T]
-    previousResourceState: Optional[T]
+    desiredResourceState: Optional[BaseResourceModel]
+    previousResourceState: Optional[BaseResourceModel]
     logicalResourceIdentifier: Optional[str]
     nextToken: Optional[str]