Skip to content

Commit 1783eda

Browse files
committed
refactor(package): replace METADATA_EXPORT_LIST and GLOBAL_EXPORT_DICT with typed registries
Migrate two flat package exporter registries to typed dataclass form: - METADATA_EXPORT_LIST (list of Resource subclasses) -> METADATA_EXPORTS (List[MetadataExportSpec]). Each spec captures both the metadata_type ('AWS::ServerlessRepo::Application') and the per-property exporter classes that handle LicenseUrl/ReadmeUrl. Template._export_metadata now dispatches via a metadata_type lookup instead of a per-class RESOURCE_TYPE filter. - GLOBAL_EXPORT_DICT (dict keyed by Fn::Transform) -> GLOBAL_TRANSFORM_EXPORTS (List[GlobalTransformExportSpec]). Each spec carries a discriminator callable (e.g. _is_aws_include for matching AWS::Include) plus the handler. _export_global_artifacts now dispatches through the discriminator so future Fn::Transform variants can register without touching the walker. The typed shape is the contract a follow-up commit will read from to process AWS::Include before language-extension expansion.
1 parent 9ffa5ed commit 1783eda

3 files changed

Lines changed: 131 additions & 21 deletions

File tree

samcli/lib/package/artifact_exporter.py

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
import copy
1818
import logging
1919
import os
20-
from typing import Dict, List, Optional
20+
from typing import Dict, List, Optional, Sequence
2121

2222
from botocore.utils import set_value_from_jmespath
2323

@@ -28,10 +28,11 @@
2828
from samcli.lib.package.code_signer import CodeSigner
2929
from samcli.lib.package.local_files_utils import get_uploaded_s3_object_name, mktempfile
3030
from samcli.lib.package.packageable_resources import (
31-
GLOBAL_EXPORT_DICT,
32-
METADATA_EXPORT_LIST,
31+
GLOBAL_TRANSFORM_EXPORTS,
32+
METADATA_EXPORTS,
3333
RESOURCES_EXPORT_LIST,
3434
ECRResource,
35+
MetadataExportSpec,
3536
ResourceZip,
3637
)
3738
from samcli.lib.package.uploaders import Destination, Uploaders
@@ -336,7 +337,7 @@ class Template:
336337
template_dict: Dict
337338
template_dir: str
338339
resources_to_export: frozenset
339-
metadata_to_export: frozenset
340+
metadata_to_export: Sequence[MetadataExportSpec]
340341
uploaders: Uploaders
341342
code_signer: CodeSigner
342343

@@ -350,7 +351,7 @@ def __init__(
350351
RESOURCES_EXPORT_LIST
351352
+ [CloudFormationStackResource, CloudFormationStackSetResource, ServerlessApplicationResource]
352353
),
353-
metadata_to_export=frozenset(METADATA_EXPORT_LIST),
354+
metadata_to_export=tuple(METADATA_EXPORTS),
354355
template_str: Optional[str] = None,
355356
normalize_template: bool = False,
356357
normalize_parameters: bool = False,
@@ -405,14 +406,21 @@ def _export_global_artifacts(self, template_dict: Dict) -> Dict:
405406
"""
406407
Template params such as AWS::Include transforms are not specific to
407408
any resource type but contain artifacts that should be exported,
408-
here we iterate through the template dict and export params with a
409-
handler defined in GLOBAL_EXPORT_DICT
409+
here we iterate through the template dict and dispatch to handlers
410+
declared in GLOBAL_TRANSFORM_EXPORTS.
410411
"""
412+
specs_by_key: Dict[str, list] = {}
413+
for spec in GLOBAL_TRANSFORM_EXPORTS:
414+
specs_by_key.setdefault(spec.template_key, []).append(spec)
415+
411416
for key, val in template_dict.items():
412-
if key in GLOBAL_EXPORT_DICT:
413-
template_dict[key] = GLOBAL_EXPORT_DICT[key](
414-
val, self.uploaders.get(ResourceZip.EXPORT_DESTINATION), self.template_dir
415-
)
417+
if key in specs_by_key:
418+
for spec in specs_by_key[key]:
419+
if spec.discriminator(val):
420+
template_dict[key] = spec.handler(
421+
val, self.uploaders.get(ResourceZip.EXPORT_DESTINATION), self.template_dir
422+
)
423+
val = template_dict[key] # subsequent specs see the rewrite
416424
elif isinstance(val, dict):
417425
self._export_global_artifacts(val)
418426
elif isinstance(val, list):
@@ -429,11 +437,13 @@ def _export_metadata(self):
429437
if "Metadata" not in self.template_dict:
430438
return
431439

432-
for metadata_type, metadata_dict in self.template_dict["Metadata"].items():
433-
for exporter_class in self.metadata_to_export:
434-
if exporter_class.RESOURCE_TYPE != metadata_type:
435-
continue
440+
specs_by_type = {spec.metadata_type: spec for spec in self.metadata_to_export}
436441

442+
for metadata_type, metadata_dict in self.template_dict["Metadata"].items():
443+
spec = specs_by_type.get(metadata_type)
444+
if spec is None:
445+
continue
446+
for exporter_class in spec.exporters:
437447
exporter = exporter_class(self.uploaders, self.code_signer)
438448
exporter.export(metadata_type, metadata_dict, self.template_dir)
439449

samcli/lib/package/packageable_resources.py

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
import logging
66
import os
77
import shutil
8-
from typing import Dict, Optional, Union, cast
8+
from dataclasses import dataclass, field
9+
from typing import Callable, Dict, List, Optional, Type, Union, cast
910

1011
import jmespath
1112
from botocore.utils import set_value_from_jmespath
@@ -717,7 +718,29 @@ def export(self, resource_id: str, resource_dict: Optional[Dict], parent_dir: st
717718
GraphQLApiCodeResource,
718719
]
719720

720-
METADATA_EXPORT_LIST = [ServerlessRepoApplicationReadme, ServerlessRepoApplicationLicense]
721+
722+
@dataclass(frozen=True)
723+
class MetadataExportSpec:
724+
"""How a top-level Metadata.<type> entry is exported and merged back.
725+
726+
Used by Template._export_metadata to route entries to their exporter
727+
classes, and by merge_language_extensions_s3_uris to copy rewritten
728+
property values back into the original (Fn::ForEach-preserving)
729+
child-stack template after LE expansion.
730+
"""
731+
732+
metadata_type: str
733+
property_names: List[str]
734+
exporters: List[Type["Resource"]] = field(default_factory=list)
735+
736+
737+
METADATA_EXPORTS: List[MetadataExportSpec] = [
738+
MetadataExportSpec(
739+
metadata_type=AWS_SERVERLESSREPO_APPLICATION,
740+
property_names=["LicenseUrl", "ReadmeUrl"],
741+
exporters=[ServerlessRepoApplicationLicense, ServerlessRepoApplicationReadme],
742+
),
743+
]
721744

722745

723746
def include_transform_export_handler(template_dict, uploader, parent_dir):
@@ -741,4 +764,29 @@ def include_transform_export_handler(template_dict, uploader, parent_dir):
741764
return template_dict
742765

743766

744-
GLOBAL_EXPORT_DICT = {"Fn::Transform": include_transform_export_handler}
767+
@dataclass(frozen=True)
768+
class GlobalTransformExportSpec:
769+
"""How a global (anywhere-in-the-tree) export hook is invoked.
770+
771+
The discriminator decides whether a given dict node matches this spec
772+
(dispatch on Fn::Transform's "Name" field today; ready for future
773+
Fn::Transform variants). The handler receives the matched node and
774+
mutates it in place to rewrite local paths to S3 URLs.
775+
"""
776+
777+
template_key: str
778+
discriminator: Callable[[object], bool]
779+
handler: Callable
780+
781+
782+
def _is_aws_include(node: object) -> bool:
783+
return isinstance(node, dict) and node.get("Name") == "AWS::Include"
784+
785+
786+
GLOBAL_TRANSFORM_EXPORTS: List[GlobalTransformExportSpec] = [
787+
GlobalTransformExportSpec(
788+
template_key="Fn::Transform",
789+
discriminator=_is_aws_include,
790+
handler=include_transform_export_handler,
791+
),
792+
]

tests/unit/lib/package/test_artifact_exporter.py

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@
4141
LambdaLayerVersionResource,
4242
copy_to_temp_dir,
4343
include_transform_export_handler,
44-
GLOBAL_EXPORT_DICT,
44+
GLOBAL_TRANSFORM_EXPORTS,
45+
GlobalTransformExportSpec,
4546
ServerlessLayerVersionResource,
4647
ServerlessRepoApplicationLicense,
4748
ServerlessRepoApplicationReadme,
@@ -1454,6 +1455,8 @@ def test_export_serverless_application_no_upload_path_is_dictionary(self):
14541455

14551456
@patch("samcli.lib.package.artifact_exporter.yaml_parse")
14561457
def test_template_export_metadata(self, yaml_parse_mock):
1458+
from samcli.lib.package.packageable_resources import MetadataExportSpec
1459+
14571460
parent_dir = os.path.sep
14581461
template_dir = os.path.join(parent_dir, "foo", "bar")
14591462
template_path = os.path.join(template_dir, "path")
@@ -1476,7 +1479,18 @@ def test_template_export_metadata(self, yaml_parse_mock):
14761479
metadata_type2_instance = Mock()
14771480
metadata_type2_class.return_value = metadata_type2_instance
14781481

1479-
metadata_to_export = [metadata_type1_class, metadata_type2_class]
1482+
metadata_to_export = [
1483+
MetadataExportSpec(
1484+
metadata_type="metadata_type1",
1485+
property_names=["property_1"],
1486+
exporters=[metadata_type1_class],
1487+
),
1488+
MetadataExportSpec(
1489+
metadata_type="metadata_type2",
1490+
property_names=["property_2"],
1491+
exporters=[metadata_type2_class],
1492+
),
1493+
]
14801494

14811495
template_dict = {"Metadata": {"metadata_type1": {"property_1": "abc"}, "metadata_type2": {"property_2": "def"}}}
14821496
open_mock = mock.mock_open()
@@ -1852,8 +1866,13 @@ def test_template_global_export(self, yaml_parse_mock):
18521866
}
18531867
yaml_parse_mock.return_value = template_dict
18541868

1869+
mock_spec = GlobalTransformExportSpec(
1870+
template_key="Fn::Transform",
1871+
discriminator=lambda v: isinstance(v, dict),
1872+
handler=include_transform_export_handler_mock,
1873+
)
18551874
with patch("samcli.lib.package.artifact_exporter.open", open_mock(read_data=template_str)) as open_mock:
1856-
with patch.dict(GLOBAL_EXPORT_DICT, {"Fn::Transform": include_transform_export_handler_mock}):
1875+
with patch("samcli.lib.package.artifact_exporter.GLOBAL_TRANSFORM_EXPORTS", [mock_spec]):
18571876
template_exporter = Template(template_path, parent_dir, self.uploaders_mock, resources_to_export)
18581877
exported_template = template_exporter._export_global_artifacts(template_exporter.template_dict)
18591878

@@ -1970,6 +1989,7 @@ def test_include_transform_export_handler_non_include_transform(self, is_local_f
19701989
self.s3_uploader_mock.upload_with_dedup.assert_not_called()
19711990
self.assertEqual(handler_output, {"Name": "AWS::OtherTransform", "Parameters": {"Location": "foo.yaml"}})
19721991

1992+
19731993
def test_template_export_path_be_folder(self):
19741994
template_path = "/path/foo"
19751995
# Set parent_dir to be a non-existent folder
@@ -2571,6 +2591,38 @@ def test_export_cloudformation_stack_unexpected_exception_falls_back(self, Templ
25712591
finally:
25722592
os.remove(template_path)
25732593

2594+
def test_metadata_exports_registry_shape(self):
2595+
from samcli.lib.package.packageable_resources import (
2596+
METADATA_EXPORTS,
2597+
MetadataExportSpec,
2598+
ServerlessRepoApplicationLicense,
2599+
ServerlessRepoApplicationReadme,
2600+
)
2601+
2602+
self.assertTrue(all(isinstance(s, MetadataExportSpec) for s in METADATA_EXPORTS))
2603+
2604+
by_type = {s.metadata_type: s for s in METADATA_EXPORTS}
2605+
spec = by_type["AWS::ServerlessRepo::Application"]
2606+
2607+
self.assertEqual(sorted(spec.property_names), ["LicenseUrl", "ReadmeUrl"])
2608+
self.assertIn(ServerlessRepoApplicationLicense, spec.exporters)
2609+
self.assertIn(ServerlessRepoApplicationReadme, spec.exporters)
2610+
2611+
def test_global_transform_exports_registry_shape(self):
2612+
self.assertTrue(all(isinstance(s, GlobalTransformExportSpec) for s in GLOBAL_TRANSFORM_EXPORTS))
2613+
2614+
aws_include = next(
2615+
s for s in GLOBAL_TRANSFORM_EXPORTS if s.discriminator({"Name": "AWS::Include", "Parameters": {}})
2616+
)
2617+
2618+
self.assertEqual(aws_include.template_key, "Fn::Transform")
2619+
self.assertIs(aws_include.handler, include_transform_export_handler)
2620+
2621+
# Discriminator must reject non-AWS::Include transforms and non-dict inputs
2622+
self.assertFalse(aws_include.discriminator({"Name": "AWS::OtherTransform"}))
2623+
self.assertFalse(aws_include.discriminator(None))
2624+
self.assertFalse(aws_include.discriminator("not-a-dict"))
2625+
25742626

25752627
class TestResolveNestedStackParameters(unittest.TestCase):
25762628
"""Unit tests for _resolve_nested_stack_parameters helper.

0 commit comments

Comments
 (0)