diff --git a/moto/cloudformation/parsing.py b/moto/cloudformation/parsing.py index 9731008a2..cf4f06113 100644 --- a/moto/cloudformation/parsing.py +++ b/moto/cloudformation/parsing.py @@ -793,17 +793,7 @@ class ResourceMap(collections_abc.Mapping): # type: ignore[type-arg] for logical_name in resource_names_by_action["Remove"]: resource_json = self._resource_json_map[logical_name] resource = self._parsed_resources[logical_name] - # ToDo: Standardize this. - if hasattr(resource, "physical_resource_id"): - resource_name = self._parsed_resources[ - logical_name - ].physical_resource_id - else: - resource_name = None - parse_and_delete_resource( - resource_name, resource_json, self._account_id, self._region_name - ) - self._parsed_resources.pop(logical_name) + self._delete_resource(resource, resource_json) self._template = template if parameters: @@ -859,24 +849,12 @@ class ResourceMap(collections_abc.Mapping): # type: ignore[type-arg] not isinstance(parsed_resource, str) and parsed_resource is not None ): - try: - parsed_resource.delete(self._account_id, self._region_name) - except (TypeError, AttributeError): - if hasattr(parsed_resource, "physical_resource_id"): - resource_name = parsed_resource.physical_resource_id - else: - resource_name = None - resource_json = self._resource_json_map[ - parsed_resource.logical_resource_id - ] + resource_json = self._resource_json_map[ + parsed_resource.logical_resource_id + ] - parse_and_delete_resource( - resource_name, - resource_json, - self._account_id, - self._region_name, - ) + self._delete_resource(parsed_resource, resource_json) self._parsed_resources.pop(parsed_resource.logical_resource_id) except Exception as e: @@ -889,6 +867,24 @@ class ResourceMap(collections_abc.Mapping): # type: ignore[type-arg] if tries == 5: raise last_exception + def _delete_resource( + self, parsed_resource: Any, resource_json: Dict[str, Any] + ) -> None: + try: + parsed_resource.delete(self._account_id, self._region_name) + except (TypeError, AttributeError): + if hasattr(parsed_resource, "physical_resource_id"): + resource_name = parsed_resource.physical_resource_id + else: + resource_name = None + + parse_and_delete_resource( + resource_name, + resource_json, + self._account_id, + self._region_name, + ) + class OutputMap(collections_abc.Mapping): # type: ignore[type-arg] def __init__(self, resources: ResourceMap, template: Dict[str, Any], stack_id: str): diff --git a/moto/sns/models.py b/moto/sns/models.py index a3cf2afd0..fd43eb181 100644 --- a/moto/sns/models.py +++ b/moto/sns/models.py @@ -139,36 +139,20 @@ class Topic(CloudFormationModel): @classmethod def update_from_cloudformation_json( # type: ignore[misc] cls, - original_resource: Any, + original_resource: "Topic", new_resource_name: str, cloudformation_json: Any, account_id: str, region_name: str, ) -> "Topic": - cls.delete_from_cloudformation_json( - original_resource.name, cloudformation_json, account_id, region_name - ) + original_resource.delete(account_id, region_name) return cls.create_from_cloudformation_json( new_resource_name, cloudformation_json, account_id, region_name ) - @classmethod - def delete_from_cloudformation_json( # type: ignore[misc] - cls, - resource_name: str, - cloudformation_json: Any, - account_id: str, - region_name: str, - ) -> None: - sns_backend = sns_backends[account_id][region_name] - properties = cloudformation_json["Properties"] - - topic_name = properties.get(cls.cloudformation_name_type()) or resource_name - topic_arn = make_arn_for_topic(account_id, topic_name, sns_backend.region_name) - subscriptions, _ = sns_backend.list_subscriptions(topic_arn) - for subscription in subscriptions: - sns_backend.unsubscribe(subscription.arn) - sns_backend.delete_topic(topic_arn) + def delete(self, account_id: str, region_name: str) -> None: + sns_backend: SNSBackend = sns_backends[account_id][region_name] + sns_backend.delete_topic(self.arn) def _create_default_topic_policy( self, region_name: str, account_id: str, name: str diff --git a/tests/test_sns/test_sns_cloudformation.py b/tests/test_sns/test_sns_cloudformation.py index 17cd25e05..df8e1eb03 100644 --- a/tests/test_sns/test_sns_cloudformation.py +++ b/tests/test_sns/test_sns_cloudformation.py @@ -1,6 +1,7 @@ import boto3 import json import sure # noqa # pylint: disable=unused-import +import pytest from moto import mock_cloudformation, mock_sns @@ -8,24 +9,7 @@ from moto import mock_cloudformation, mock_sns @mock_cloudformation @mock_sns def test_sns_topic(): - dummy_template = { - "AWSTemplateFormatVersion": "2010-09-09", - "Resources": { - "MySNSTopic": { - "Type": "AWS::SNS::Topic", - "Properties": { - "Subscription": [ - {"Endpoint": "https://example.com", "Protocol": "https"} - ], - "TopicName": "my_topics", - }, - } - }, - "Outputs": { - "topic_name": {"Value": {"Fn::GetAtt": ["MySNSTopic", "TopicName"]}}, - "topic_arn": {"Value": {"Ref": "MySNSTopic"}}, - }, - } + dummy_template = get_template(with_properties=True) template_json = json.dumps(dummy_template) cf = boto3.client("cloudformation", region_name="us-west-1") cf.create_stack(StackName="test_stack", TemplateBody=template_json) @@ -56,24 +40,7 @@ def test_sns_topic(): @mock_cloudformation @mock_sns def test_sns_update_topic(): - dummy_template = { - "AWSTemplateFormatVersion": "2010-09-09", - "Resources": { - "MySNSTopic": { - "Type": "AWS::SNS::Topic", - "Properties": { - "Subscription": [ - {"Endpoint": "https://example.com", "Protocol": "https"} - ], - "TopicName": "my_topics", - }, - } - }, - "Outputs": { - "topic_name": {"Value": {"Fn::GetAtt": ["MySNSTopic", "TopicName"]}}, - "topic_arn": {"Value": {"Ref": "MySNSTopic"}}, - }, - } + dummy_template = get_template(with_properties=True) sns_template_json = json.dumps(dummy_template) cf = boto3.client("cloudformation", region_name="us-west-1") cf.create_stack(StackName="test_stack", TemplateBody=sns_template_json) @@ -104,25 +71,9 @@ def test_sns_update_topic(): @mock_cloudformation @mock_sns -def test_sns_update_remove_topic(): - dummy_template = { - "AWSTemplateFormatVersion": "2010-09-09", - "Resources": { - "MySNSTopic": { - "Type": "AWS::SNS::Topic", - "Properties": { - "Subscription": [ - {"Endpoint": "https://example.com", "Protocol": "https"} - ], - "TopicName": "my_topics", - }, - } - }, - "Outputs": { - "topic_name": {"Value": {"Fn::GetAtt": ["MySNSTopic", "TopicName"]}}, - "topic_arn": {"Value": {"Ref": "MySNSTopic"}}, - }, - } +@pytest.mark.parametrize("with_properties", [True, False]) +def test_sns_update_remove_topic(with_properties): + dummy_template = get_template(with_properties) sns_template_json = json.dumps(dummy_template) cf = boto3.client("cloudformation", region_name="us-west-1") cf.create_stack(StackName="test_stack", TemplateBody=sns_template_json) @@ -142,26 +93,9 @@ def test_sns_update_remove_topic(): @mock_cloudformation @mock_sns -def test_sns_delete_topic(): - dummy_template = { - "AWSTemplateFormatVersion": "2010-09-09", - "Resources": { - "MySNSTopic": { - "Type": "AWS::SNS::Topic", - "Properties": { - "Subscription": [ - {"Endpoint": "https://example.com", "Protocol": "https"} - ], - "TopicName": "my_topics", - }, - } - }, - "Outputs": { - "topic_name": {"Value": {"Fn::GetAtt": ["MySNSTopic", "TopicName"]}}, - "topic_arn": {"Value": {"Ref": "MySNSTopic"}}, - }, - } - sns_template_json = json.dumps(dummy_template) +@pytest.mark.parametrize("with_properties", [True, False]) +def test_sns_delete_topic(with_properties): + sns_template_json = json.dumps(get_template(with_properties)) cf = boto3.client("cloudformation", region_name="us-west-1") cf.create_stack(StackName="test_stack", TemplateBody=sns_template_json) @@ -173,3 +107,20 @@ def test_sns_delete_topic(): topics = sns.list_topics()["Topics"] topics.should.have.length_of(0) + + +def get_template(with_properties): + dummy_template = { + "AWSTemplateFormatVersion": "2010-09-09", + "Resources": {"MySNSTopic": {"Type": "AWS::SNS::Topic"}}, + "Outputs": { + "topic_name": {"Value": {"Fn::GetAtt": ["MySNSTopic", "TopicName"]}}, + "topic_arn": {"Value": {"Ref": "MySNSTopic"}}, + }, + } + if with_properties: + dummy_template["Resources"]["MySNSTopic"]["Properties"] = { + "Subscription": [{"Endpoint": "https://example.com", "Protocol": "https"}], + "TopicName": "my_topics", + } + return dummy_template