Skip to content

Commit

Permalink
BDRSPS-1122 Make the cached ABISMapper.metadata() method return a fro…
Browse files Browse the repository at this point in the history
…zen type

This is to avoid accidental mutation of the cached return value.
Plus returning a pydantic model, rather than a dict leads to better type-checked code
  • Loading branch information
Lincoln-GR committed Jan 3, 2025
1 parent 6f177bd commit 0907eb8
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 11 deletions.
16 changes: 8 additions & 8 deletions abis_mapping/base/mapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ def template(cls) -> pathlib.Path:

# Template File is the name and filetype as extension from metadata
md = cls.metadata()
template_file = directory / f"{md['name']}.{md['file_type'].lower()}"
template_file = directory / f"{md.name}.{md.file_type.lower()}"

# Return
return template_file
Expand All @@ -347,27 +347,27 @@ def template_id(self) -> str:
Returns:
str: template id from metadata
"""
return self.metadata()["id"]
return self.metadata().id

@final
@classmethod
@functools.cache
def metadata(cls) -> dict[str, str]:
def metadata(cls) -> models.metadata.TemplateMetadata:
"""Retrieves and Caches the Template Metadata for this Template
Returns:
dict[str, Any]: Template Metadata for this Template
Template Metadata for this Template
"""
# Retrieve Metadata Filepath
directory = pathlib.Path(inspect.getfile(cls)).parent
metadata_file = directory / "metadata.json"

# Read Metadata and validate
md_dict = json.loads(metadata_file.read_text())
md_class = models.metadata.TemplateMetadata.model_validate(md_dict, strict=False)
md_content = metadata_file.read_bytes()
md_class = models.metadata.TemplateMetadata.model_validate_json(md_content)

# Return
return md_class.model_dump()
return md_class

@final
@classmethod
Expand Down Expand Up @@ -436,7 +436,7 @@ def register_mapper(mapper: type[ABISMapper]) -> None:
mapper: Mapper class to be registered.
"""
# Register the mapper with its template id
template_id = mapper.metadata()["id"]
template_id = mapper.metadata().id
_registry[template_id] = mapper


Expand Down
5 changes: 5 additions & 0 deletions abis_mapping/models/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ class TemplateMetadataLifecycleStatus(enum.StrEnum):
class TemplateMetadata(pydantic.BaseModel):
"""Model for the template `metadata.json` file."""

model_config = pydantic.ConfigDict(
# Frozen because this class is returned by the cached ABISMapper.metadata() method
frozen=True,
)

name: str
label: str
version: str
Expand Down
6 changes: 3 additions & 3 deletions tests/templates/test_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def test_get_metadata(self, test_params: conftest.TemplateTestParameters) -> Non
real_mapper = abis_mapping.base.mapper.get_mapper(test_params.template_id)
assert real_mapper is not None
metadata = real_mapper.metadata()
assert isinstance(metadata, dict)
assert isinstance(metadata, abis_mapping.models.metadata.TemplateMetadata)

def test_metadata_id_match(self, test_params: conftest.TemplateTestParameters) -> None:
"""Tests the metadata id matches the mapper id"""
Expand All @@ -77,7 +77,7 @@ def test_metadata_id_match(self, test_params: conftest.TemplateTestParameters) -

# Retrieve metadata
metadata = real_mapper.metadata()
assert metadata.get("id") == real_mapper().template_id
assert metadata.id == real_mapper().template_id

def test_get_schema(self, test_params: conftest.TemplateTestParameters) -> None:
"""Tests able to retrieve template schema."""
Expand Down Expand Up @@ -117,7 +117,7 @@ def test_metadata_sampling_type(self, test_params: conftest.TemplateTestParamete
metadata = mapper().metadata()

# Confirm field set correctly
assert metadata.get("sampling_type") == test_params.metadata_sampling_type
assert metadata.sampling_type == test_params.metadata_sampling_type

def test_schema_is_valid(self, test_params: conftest.TemplateTestParameters) -> None:
"""Tests that the schema.json is a valid frictionless schema."""
Expand Down

0 comments on commit 0907eb8

Please sign in to comment.