From 8ab61a6402939925d3cba23797b6f64522fa4927 Mon Sep 17 00:00:00 2001 From: Brian Pepple Date: Tue, 27 Oct 2026 18:10:09 -0400 Subject: [PATCH 1/6] Add MetronInfo v1.2 schema support Replace the bundled v1.1 schema with the stable v1.2 release and rename SchemaVersion.METRON_INFO_V1_1 to METRON_INFO_V1_2. v1.2 is a strict superset of v1.1, so v1.1 could never be the winning match in the newest-first validation order. Written MetronInfo now declares the v1.2 namespace and schemaLocation. Add end_year, status and summary to Series, read from and written to the new Series EndYear, Status and Summary elements. Status is matched case-insensitively and unknown values are dropped. An EndYear that the schema's assertions would reject (before StartYear, or on an Ongoing series) is omitted, so the write doesn't fail validation. Accept BCP 47 language tags (e.g. pt-BR, zh-Hant) in the Series and AlternativeNames language validators, which now share one implementation. This also fixes an AttributeError for languages without an ISO 639-1 code (e.g. fil, yue), which previously crashed when reading a MetronInfo AlternativeName that used one. Such languages now normalize to their ISO 639-3 code. --- darkseid/metadata/data_classes.py | 120 ++++++++++-------- darkseid/metadata/metroninfo.py | 55 ++++++-- .../MetronInfo/{v1_1 => v1_2}/MetronInfo.xsd | 21 ++- darkseid/validate.py | 8 +- tests/test_metadata.py | 39 ++++++ tests/test_metroninfo.py | 90 +++++++++++++ tests/test_validate.py | 51 ++++++-- 7 files changed, 306 insertions(+), 78 deletions(-) rename darkseid/schemas/MetronInfo/{v1_1 => v1_2}/MetronInfo.xsd (92%) diff --git a/darkseid/metadata/data_classes.py b/darkseid/metadata/data_classes.py index d3a6a76..7e69166 100644 --- a/darkseid/metadata/data_classes.py +++ b/darkseid/metadata/data_classes.py @@ -19,6 +19,7 @@ import logging +import re import pycountry @@ -28,6 +29,7 @@ MAX_UPC = 17 MAX_ISBN = 13 COUNTRY_LEN = 2 +LANGUAGE_ALPHA_3_LEN = 3 CURRENCY_LEN = 3 YEAR_LEN = 4 @@ -50,6 +52,9 @@ _COUNTRY_TO_CURRENCY: dict[str, str] = {v: k for k, v in _CURRENCY_TO_COUNTRY.items()} +# Script, region and variant subtags following the primary language subtag of a BCP 47 tag +_LANGUAGE_SUBTAGS_RE = re.compile(r"[a-zA-Z0-9]{1,8}(-[a-zA-Z0-9]{1,8})*") + def currency_to_country(currency_code: str) -> str | None: """Convert an ISO 4217 currency code to an ISO 3166-1 alpha-2 country code. @@ -77,6 +82,48 @@ def country_to_currency(country_code: str) -> str | None: return _COUNTRY_TO_CURRENCY.get(country_code) +def validate_language_tag(value: str | None) -> str | None: + """Validate and normalize a BCP 47 language tag. + + The primary language subtag is resolved with pycountry and normalized to its ISO 639-1 + code, falling back to the ISO 639-3 code for languages without one (e.g. "fil"). Any + script, region or variant subtags (e.g. "pt-BR", "zh-Hant") are kept as given. A value + without subtags may also be a language name (e.g. "English"). + + Args: + value: The language tag or name to validate. + + Returns: + The normalized language tag, or None if the value is empty. + + Raises: + ValueError: Raised when the language cannot be found or the subtags are malformed. + + """ + if not value: + return None + value = value.strip() + + primary, sep, subtags = value.partition("-") + if len(primary) == COUNTRY_LEN: + obj = pycountry.languages.get(alpha_2=primary) + elif len(primary) == LANGUAGE_ALPHA_3_LEN and sep: + obj = pycountry.languages.get(alpha_3=primary) + else: + sep = subtags = "" + try: + obj = pycountry.languages.lookup(value) + except LookupError as e: + msg = f"Couldn't find language {value}" + raise ValueError(msg) from e + if obj is None or (sep and not _LANGUAGE_SUBTAGS_RE.fullmatch(subtags)): + msg = f"Couldn't find language {value}" + raise ValueError(msg) + + code = getattr(obj, "alpha_2", None) or obj.alpha_3 + return f"{code}-{subtags}" if subtags else code + + class Validations: """A base class for data validation in dataclasses. @@ -264,7 +311,7 @@ class AlternativeNames(Basic, Validations): Attributes: name (str): The alternative name for a series. id_ (int | None): The ID associated with the alternative name, defaults to None. - language (str | None): The 2-letter ISO code of the language, defaults to None. + language (str | None): The BCP 47 language tag, defaults to None. Static Methods: validate_language(value: str, **_: any) -> str | None: Validates a language value. @@ -277,38 +324,18 @@ class AlternativeNames(Basic, Validations): def validate_language(value: str, **_: any) -> str | None: """Validate a language value. - If the value is empty, it returns None. Otherwise, it strips any leading or trailing - whitespace from the value. If the length of the value is 2, it tries to find the - language object using the alpha-2 code. Otherwise, it tries to look up the language - object using the value. If the language object is not found, it raises a ValueError. - Args: - value (str): The language value to validate. + value (str): The BCP 47 language tag or language name to validate. **_ (any): Additional keyword arguments (ignored). Returns: - Optional[str]: The validated language code, or None if the value is empty. + Optional[str]: The normalized language tag, or None if the value is empty. Raises: - ValueError: Raised when the language object cannot be found. + ValueError: Raised when the language cannot be found. """ - if not value: - return None - value = value.strip() - - if len(value) == COUNTRY_LEN: - obj = pycountry.languages.get(alpha_2=value) - else: - try: - obj = pycountry.languages.lookup(value) - except LookupError as e: - msg = f"Couldn't find language {value}" - raise ValueError(msg) from e - if obj is None: - msg = f"Couldn't find language {value}" - raise ValueError(msg) - return obj.alpha_2 + return validate_language_tag(value) @dataclass @@ -325,7 +352,10 @@ class Series(Basic, Validations): issue_count (Optional[int]): The count of issues. volume_count (int | None): The count of volumes. alternative_names: list[AlternativeNames]: A list of alternative names for series. - language (str | None): The 2-letter ISO code of the language, defaults to None. + language (str | None): The BCP 47 language tag, defaults to None. + end_year (int | None): The year that the series ended in. A 4 digit value. + status (str | None): The publication status of the series (e.g. Ongoing, Completed). + summary (str | None): A description of the series. Static Methods: validate_language(value: str, **_: any) -> str | None: Validates a language value. @@ -339,7 +369,10 @@ class Series(Basic, Validations): issue_count: int | None = None volume_count: int | None = None alternative_names: list[AlternativeNames] = field(default_factory=list) - language: str | None = None # 2-letter iso code + language: str | None = None # BCP 47 language tag + end_year: int | None = None + status: str | None = None + summary: str | None = None @staticmethod def validate_start_year(value: int, **_: any) -> int | None: @@ -368,42 +401,24 @@ def validate_start_year(value: int, **_: any) -> int | None: msg = f"Year: {value} length must be {YEAR_LEN}" raise ValueError(msg) + validate_end_year = validate_start_year + @staticmethod def validate_language(value: str, **_: any) -> str | None: """Validate a language value. - If the value is empty, it returns None. Otherwise, it strips any leading or trailing - whitespace from the value. If the length of the value is 2, it tries to find the - language object using the alpha-2 code. Otherwise, it tries to look up the language - object using the value. If the language object is not found, it raises a ValueError. - Args: - value (str): The language value to validate. + value (str): The BCP 47 language tag or language name to validate. **_ (any): Additional keyword arguments (ignored). Returns: - Optional[str]: The validated language code, or None if the value is empty. + Optional[str]: The normalized language tag, or None if the value is empty. Raises: - ValueError: Raised when the language object cannot be found. + ValueError: Raised when the language cannot be found. """ - if not value: - return None - value = value.strip() - - if len(value) == COUNTRY_LEN: - obj = pycountry.languages.get(alpha_2=value) - else: - try: - obj = pycountry.languages.lookup(value) - except LookupError as e: - msg = f"Couldn't find language {value}" - raise ValueError(msg) from e - if obj is None: - msg = f"Couldn't find language {value}" - raise ValueError(msg) - return obj.alpha_2 + return validate_language_tag(value) @dataclass @@ -1015,7 +1030,8 @@ def __str__(self: Metadata) -> str: # noqa: PLR0912 if self.series.volume: series_info += f" (v{self.series.volume})" if self.series.start_year: - series_info += f" [{self.series.start_year}]" + end_year = f"-{self.series.end_year}" if self.series.end_year else "" + series_info += f" [{self.series.start_year}{end_year}]" lines.append(f"{indent}Series: {series_info}") if self.issue: diff --git a/darkseid/metadata/metroninfo.py b/darkseid/metadata/metroninfo.py index 3f121ef..c768b66 100644 --- a/darkseid/metadata/metroninfo.py +++ b/darkseid/metadata/metroninfo.py @@ -79,6 +79,8 @@ } ) +SERIES_STATUSES = ("Cancelled", "Completed", "Hiatus", "Ongoing") + VALID_ROLES = frozenset( { "writer", @@ -162,7 +164,7 @@ class MetronInfo(BaseMetadataHandler): def __init__(self) -> None: """Initialize the MetronInfo instance.""" self._schema_path = ( - Path(__file__).parent.parent / "schemas" / "MetronInfo" / "v1_1" / "MetronInfo.xsd" + Path(__file__).parent.parent / "schemas" / "MetronInfo" / "v1_2" / "MetronInfo.xsd" ) def metadata_from_string(self, xml_string: str) -> Metadata: @@ -278,13 +280,13 @@ def _get_root(xml_bytes: bytes | None) -> ET.Element: } root.attrib["xmlns:metroninfo"] = ( - "https://metron-project.github.io/docs/metroninfo/schemas/v1.1" + "https://metron-project.github.io/docs/metroninfo/schemas/v1.2" ) root.attrib["xmlns:xsd"] = "http://www.w3.org/2001/XMLSchema" root.attrib["xmlns:xsi"] = "http://www.w3.org/2001/XMLSchema-instance" root.attrib["xsi:schemaLocation"] = ( - "https://metron-project.github.io/docs/metroninfo/schemas/v1.1 " - "https://raw.githubusercontent.com/Metron-Project/metroninfo/refs/heads/master/schema/v1.1/MetronInfo.xsd" + "https://metron-project.github.io/docs/metroninfo/schemas/v1.2 " + "https://raw.githubusercontent.com/Metron-Project/metroninfo/refs/heads/master/schema/v1.2/MetronInfo.xsd" ) return root @@ -386,6 +388,23 @@ def _normalize_series_format(format_str: str | None) -> str | None: None, ) + @staticmethod + def _normalize_series_status(status: str | None) -> str | None: + """Normalize series status to valid MetronInfo status. + + Args: + status: Status string to normalize. + + Returns: + Normalized status or None. + + """ + if not status: + return None + + lower_val = status.strip().lower() + return next((val for val in SERIES_STATUSES if val.lower() == lower_val), None) + @staticmethod def _set_date_element(root: ET.Element, tag: str, date_val: date | None = None) -> None: """Set date element in YYYY-MM-DD format. @@ -505,10 +524,18 @@ def _add_series(self, root: ET.Element, series: Series | None) -> None: # noqa: if series_format is not None: ET.SubElement(series_node, "Format").text = series_format - if series.start_year: - ET.SubElement(series_node, "StartYear").text = str(series.start_year) - elif series.volume is not None and series.volume >= VOLUME_THRESHOLD: - ET.SubElement(series_node, "StartYear").text = str(series.volume) + start_year = series.start_year + if not start_year and series.volume is not None and series.volume >= VOLUME_THRESHOLD: + start_year = series.volume + if start_year: + ET.SubElement(series_node, "StartYear").text = str(start_year) + + status = self._normalize_series_status(series.status) + # The schema rejects an EndYear before the StartYear, or on an ongoing series. + if series.end_year and status != "Ongoing" and series.end_year >= (start_year or 0): + ET.SubElement(series_node, "EndYear").text = str(series.end_year) + if status is not None: + ET.SubElement(series_node, "Status").text = status if series.issue_count: ET.SubElement(series_node, "IssueCount").text = str(series.issue_count) @@ -525,6 +552,9 @@ def _add_series(self, root: ET.Element, series: Series | None) -> None: # noqa: attrib["lang"] = alt_name.language ET.SubElement(alt_names_node, "AlternativeName", attrib=attrib).text = alt_name.name + if series.summary: + ET.SubElement(series_node, "Summary").text = series.summary + def _add_info_sources(self, root: ET.Element, info_sources: list[InfoSources]) -> None: """Add information sources to XML. @@ -827,12 +857,19 @@ def _parse_series_element(self, series_element: ET.Element | None) -> Series | N series.language = series_element.attrib.get("lang") # Define mapping for simple text elements - text_mappings = {"Name": "name", "SortName": "sort_name", "Format": "format"} + text_mappings = { + "Name": "name", + "SortName": "sort_name", + "Format": "format", + "Status": "status", + "Summary": "summary", + } # Define mapping for integer elements int_mappings = { "Volume": "volume", "StartYear": "start_year", + "EndYear": "end_year", "IssueCount": "issue_count", "VolumeCount": "volume_count", } diff --git a/darkseid/schemas/MetronInfo/v1_1/MetronInfo.xsd b/darkseid/schemas/MetronInfo/v1_2/MetronInfo.xsd similarity index 92% rename from darkseid/schemas/MetronInfo/v1_1/MetronInfo.xsd rename to darkseid/schemas/MetronInfo/v1_2/MetronInfo.xsd index 220312b..8db7cac 100644 --- a/darkseid/schemas/MetronInfo/v1_1/MetronInfo.xsd +++ b/darkseid/schemas/MetronInfo/v1_2/MetronInfo.xsd @@ -97,12 +97,21 @@ + + + + + + + @@ -255,6 +264,15 @@ + + + + + + + + + @@ -347,9 +365,10 @@ + - + diff --git a/darkseid/validate.py b/darkseid/validate.py index 5b4bca7..0d8ca9a 100644 --- a/darkseid/validate.py +++ b/darkseid/validate.py @@ -50,7 +50,7 @@ class SchemaVersion(Enum): for validation priority. """ - METRON_INFO_V1_1 = auto() + METRON_INFO_V1_2 = auto() COMIC_INFO_V2 = auto() COMIC_INFO_V1 = auto() UNKNOWN = auto() @@ -82,8 +82,8 @@ class ValidateMetadata: "file_name": "ComicInfo.xsd", "schema_class": XMLSchema10, }, - SchemaVersion.METRON_INFO_V1_1: { - "module_path": "darkseid.schemas.MetronInfo.v1_1", + SchemaVersion.METRON_INFO_V1_2: { + "module_path": "darkseid.schemas.MetronInfo.v1_2", "file_name": "MetronInfo.xsd", "schema_class": XMLSchema11, }, @@ -91,7 +91,7 @@ class ValidateMetadata: # Validation order: newest/most specific schemas first _VALIDATION_ORDER: ClassVar[list[SchemaVersion]] = [ - SchemaVersion.METRON_INFO_V1_1, + SchemaVersion.METRON_INFO_V1_2, SchemaVersion.COMIC_INFO_V2, SchemaVersion.COMIC_INFO_V1, ] diff --git a/tests/test_metadata.py b/tests/test_metadata.py index 0a19f5b..6ae06e0 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -149,6 +149,45 @@ def test_series_invalid_language(): Series("Foo Bar", language="xyz") +@pytest.mark.parametrize( + ("language", "expected"), + [ + ("en", "en"), + (" de ", "de"), + ("English", "en"), + ("eng", "en"), + ("fil", "fil"), + ("yue", "yue"), + ("pt-BR", "pt-BR"), + ("pt-br", "pt-br"), + ("zh-Hant", "zh-Hant"), + ("zh-Hant-TW", "zh-Hant-TW"), + ("fil-PH", "fil-PH"), + ("", None), + (None, None), + ], +) +def test_series_bcp47_language(language, expected): + """Test Series accepts BCP 47 language tags, normalizing the primary language subtag.""" + assert Series("Foo Bar", language=language).language == expected + + +@pytest.mark.parametrize("language", ["xyz", "zz-BR", "pt-", "pt-toolongsubtag", "pt-B_R"]) +def test_series_invalid_bcp47_language(language): + """Test Series rejects unknown languages and malformed subtags.""" + with pytest.raises(ValueError, match="Couldn't find language"): + Series("Foo Bar", language=language) + + +def test_series_end_year_validation(): + """Test Series end_year validation.""" + assert Series("Superman", start_year=1938, end_year=2011).end_year == 2011 + assert Series("Superman", end_year=None).end_year is None + + with pytest.raises(ValueError, match="length must be 4"): + Series("Superman", end_year=99) + + def test_series_start_year_validation(): """Test Series start_year validation.""" # Valid year diff --git a/tests/test_metroninfo.py b/tests/test_metroninfo.py index d2284ad..ab735f1 100644 --- a/tests/test_metroninfo.py +++ b/tests/test_metroninfo.py @@ -323,6 +323,96 @@ def test_community_rating_missing_rating_count(metron_info): assert result.rating_count is None +# Series EndYear, Status & Summary (v1.2) tests +def test_series_v1_2_fields_round_trip(metron_info, tmp_path): + """EndYear, Status and Summary validate against the schema and survive a round trip.""" + metadata = Metadata() + metadata.series = Series( + name="Justice League", + start_year=1970, + end_year=1975, + status="completed", + summary="The original run.", + ) + + filename = tmp_path / "MetronInfo.xml" + metron_info.write_xml(filename, metadata) + result = metron_info.metadata_from_string(filename.read_text()) + + assert result.series.end_year == 1975 + assert result.series.status == "Completed" + assert result.series.summary == "The original run." + + +@pytest.mark.parametrize( + ("start_year", "end_year", "status"), + [ + (1975, 1970, None), # EndYear before StartYear + (1970, 1975, "Ongoing"), # Ongoing series can't have an EndYear + ], +) +def test_series_end_year_omitted_when_invalid(metron_info, tmp_path, start_year, end_year, status): + """An EndYear the schema would reject is dropped rather than failing the write.""" + metadata = Metadata() + metadata.series = Series( + name="Test Series", start_year=start_year, end_year=end_year, status=status + ) + + filename = tmp_path / "MetronInfo.xml" + metron_info.write_xml(filename, metadata) + root = ET.fromstring(filename.read_text()) # noqa: S314 + + assert root.find("Series/EndYear") is None + assert root.find("Series/StartYear").text == str(start_year) + + +def test_series_end_year_checked_against_volume_start_year(metron_info): + """A volume used as the StartYear is also used to check the EndYear.""" + metadata = Metadata() + metadata.series = Series(name="Test Series", volume=2016, end_year=2015) + + root = metron_info._convert_metadata_to_xml(metadata).getroot() + + assert root.find("Series/StartYear").text == "2016" + assert root.find("Series/EndYear") is None + + +@pytest.mark.parametrize( + ("status", "expected"), + [ + ("Ongoing", "Ongoing"), + (" hiatus ", "Hiatus"), + ("CANCELLED", "Cancelled"), + ("Unknown", None), + ("", None), + (None, None), + ], +) +def test_normalize_series_status(metron_info, status, expected): + """Series status is matched case-insensitively and unknown values are dropped.""" + assert metron_info._normalize_series_status(status) == expected + + +def test_series_bcp47_language_round_trip(metron_info, tmp_path): + """BCP 47 language tags validate against the schema and survive a round trip.""" + metadata = Metadata() + metadata.series = Series( + name="Test Series", + language="pt-BR", + alternative_names=[ + AlternativeNames("Ang Kagila-gilalas", language="fil"), + AlternativeNames("蜘蛛人", language="zh-Hant"), + ], + ) + + filename = tmp_path / "MetronInfo.xml" + metron_info.write_xml(filename, metadata) + result = metron_info.metadata_from_string(filename.read_text()) + + assert result.series.language == "pt-BR" + assert [n.language for n in result.series.alternative_names] == ["fil", "zh-Hant"] + + # XML Conversion Tests def test_convert_complex_metadata_to_xml(metron_info, complex_metadata): """Test XML conversion with complex metadata.""" diff --git a/tests/test_validate.py b/tests/test_validate.py index 1794133..b68f1e9 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -44,7 +44,7 @@ def mock_schema_path(tmp_path): def test_schema_version_enum_values(): """Test that SchemaVersion enum has expected values.""" expected_values = [ - "METRON_INFO_V1_1", + "METRON_INFO_V1_2", "COMIC_INFO_V2", "COMIC_INFO_V1", "UNKNOWN", @@ -57,7 +57,7 @@ def test_schema_version_string_representation(): """Test SchemaVersion string representation.""" assert str(SchemaVersion.COMIC_INFO_V1) == "Comic Info V1" assert str(SchemaVersion.COMIC_INFO_V2) == "Comic Info V2" - assert str(SchemaVersion.METRON_INFO_V1_1) == "Metron Info V1.1" + assert str(SchemaVersion.METRON_INFO_V1_2) == "Metron Info V1.2" assert str(SchemaVersion.UNKNOWN) == "Unknown" @@ -128,15 +128,15 @@ def test_get_schema_path_comic_info_v1(mock_as_file, mock_files): @patch("darkseid.validate.files") @patch("darkseid.validate.as_file") -def test_get_schema_path_metron_info_v1_1(mock_as_file, mock_files): - """Test _get_schema_path for MetronInfo v1.1.""" +def test_get_schema_path_metron_info_v1_2(mock_as_file, mock_files): + """Test _get_schema_path for MetronInfo v1.2.""" mock_path = Mock() mock_files.return_value.joinpath.return_value = mock_path mock_as_file.return_value.__enter__.return_value = Path("/fake/path/MetronInfo.xsd") - result = ValidateMetadata._get_schema_path(SchemaVersion.METRON_INFO_V1_1) + result = ValidateMetadata._get_schema_path(SchemaVersion.METRON_INFO_V1_2) - mock_files.assert_called_once_with("darkseid.schemas.MetronInfo.v1_1") + mock_files.assert_called_once_with("darkseid.schemas.MetronInfo.v1_2") mock_files.return_value.joinpath.assert_called_once_with("MetronInfo.xsd") assert result == Path("/fake/path/MetronInfo.xsd") @@ -221,10 +221,10 @@ def test_validate_first_schema_valid(valid_xml_bytes): validator = ValidateMetadata(valid_xml_bytes) with patch.object(validator, "_is_valid") as mock_is_valid: - mock_is_valid.side_effect = lambda schema: schema == SchemaVersion.METRON_INFO_V1_1 + mock_is_valid.side_effect = lambda schema: schema == SchemaVersion.METRON_INFO_V1_2 result = validator.validate() - assert result == SchemaVersion.METRON_INFO_V1_1 + assert result == SchemaVersion.METRON_INFO_V1_2 def test_validate_second_schema_valid(valid_xml_bytes): @@ -253,7 +253,7 @@ def test_validate_all_returns_all_results(valid_xml_bytes): validator = ValidateMetadata(valid_xml_bytes) expected_results = { - SchemaVersion.METRON_INFO_V1_1: True, + SchemaVersion.METRON_INFO_V1_2: True, SchemaVersion.COMIC_INFO_V2: False, SchemaVersion.COMIC_INFO_V1: True, } @@ -312,7 +312,7 @@ def test_get_validation_errors_no_errors(valid_xml_bytes): [ (SchemaVersion.COMIC_INFO_V1, "darkseid.schemas.ComicInfo.v1", "ComicInfo.xsd"), (SchemaVersion.COMIC_INFO_V2, "darkseid.schemas.ComicInfo.v2", "ComicInfo.xsd"), - (SchemaVersion.METRON_INFO_V1_1, "darkseid.schemas.MetronInfo.v1_1", "MetronInfo.xsd"), + (SchemaVersion.METRON_INFO_V1_2, "darkseid.schemas.MetronInfo.v1_2", "MetronInfo.xsd"), ], ) def test_schema_config_mapping(schema_version, expected_module, expected_file): @@ -328,7 +328,7 @@ def test_schema_config_mapping(schema_version, expected_module, expected_file): [ (SchemaVersion.COMIC_INFO_V1, XMLSchema10), (SchemaVersion.COMIC_INFO_V2, XMLSchema10), - (SchemaVersion.METRON_INFO_V1_1, XMLSchema11), + (SchemaVersion.METRON_INFO_V1_2, XMLSchema11), ], ) def test_schema_class_mapping(schema_version, expected_class): @@ -341,7 +341,7 @@ def test_schema_class_mapping(schema_version, expected_class): def test_validation_order_consistency(): """Test that validation order is consistent with class definition.""" expected_order = [ - SchemaVersion.METRON_INFO_V1_1, + SchemaVersion.METRON_INFO_V1_2, SchemaVersion.COMIC_INFO_V2, SchemaVersion.COMIC_INFO_V1, ] @@ -413,3 +413,30 @@ def test_get_validation_errors_reports_entities(xml): def test_validate_accepts_documents_without_entities(xml): """Test defusing doesn't reject ordinary documents.""" assert ValidateMetadata(xml).validate() == SchemaVersion.COMIC_INFO_V2 + + +@pytest.mark.parametrize( + ("series_xml", "expected"), + [ + ( + ( + 'Foo1970' + "1975Completed" + "A series." + ), + SchemaVersion.METRON_INFO_V1_2, + ), + ( + ("Foo19751970"), + SchemaVersion.UNKNOWN, + ), + ( + "Foo1975Ongoing", + SchemaVersion.UNKNOWN, + ), + ], +) +def test_validate_metron_info_v1_2_series(series_xml, expected): + """Test validation of MetronInfo v1.2 series elements against the bundled schema.""" + xml = f"{series_xml}" + assert ValidateMetadata(xml.encode()).validate() == expected From 3688dbeaf3ec880150aaf39899ac721a46438185 Mon Sep 17 00:00:00 2001 From: Brian Pepple Date: Tue, 27 Oct 2026 18:29:21 -0400 Subject: [PATCH 2/6] Add the stable MetronInfo v1.2 schema alongside v1.1 and add SchemaVersion.METRON_INFO_V1_2. Since v1.2 is a superset of v1.1, v1.1 is validated first so documents report the oldest version they conform to. Written MetronInfo now declares the v1.2 namespace and schemaLocation. --- .../schemas/MetronInfo/v1_1/MetronInfo.xsd | 355 ++++++++++++++++++ darkseid/validate.py | 18 +- tests/test_validate.py | 32 +- 3 files changed, 398 insertions(+), 7 deletions(-) create mode 100644 darkseid/schemas/MetronInfo/v1_1/MetronInfo.xsd diff --git a/darkseid/schemas/MetronInfo/v1_1/MetronInfo.xsd b/darkseid/schemas/MetronInfo/v1_1/MetronInfo.xsd new file mode 100644 index 0000000..220312b --- /dev/null +++ b/darkseid/schemas/MetronInfo/v1_1/MetronInfo.xsd @@ -0,0 +1,355 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/darkseid/validate.py b/darkseid/validate.py index 0d8ca9a..1d2372f 100644 --- a/darkseid/validate.py +++ b/darkseid/validate.py @@ -51,6 +51,7 @@ class SchemaVersion(Enum): """ METRON_INFO_V1_2 = auto() + METRON_INFO_V1_1 = auto() COMIC_INFO_V2 = auto() COMIC_INFO_V1 = auto() UNKNOWN = auto() @@ -82,6 +83,11 @@ class ValidateMetadata: "file_name": "ComicInfo.xsd", "schema_class": XMLSchema10, }, + SchemaVersion.METRON_INFO_V1_1: { + "module_path": "darkseid.schemas.MetronInfo.v1_1", + "file_name": "MetronInfo.xsd", + "schema_class": XMLSchema11, + }, SchemaVersion.METRON_INFO_V1_2: { "module_path": "darkseid.schemas.MetronInfo.v1_2", "file_name": "MetronInfo.xsd", @@ -89,8 +95,10 @@ class ValidateMetadata: }, } - # Validation order: newest/most specific schemas first + # Validation order: MetronInfo before ComicInfo, newest ComicInfo first. MetronInfo v1.2 is a + # superset of v1.1, so v1.1 is checked first to report the oldest version a document conforms to. _VALIDATION_ORDER: ClassVar[list[SchemaVersion]] = [ + SchemaVersion.METRON_INFO_V1_1, SchemaVersion.METRON_INFO_V1_2, SchemaVersion.COMIC_INFO_V2, SchemaVersion.COMIC_INFO_V1, @@ -216,13 +224,15 @@ def _is_valid(self, schema_version: SchemaVersion) -> bool: return True def validate(self) -> SchemaVersion: - """Determine and return the highest valid schema version for the XML metadata. + """Determine and return the schema version for the XML metadata. This method checks the XML against all supported schema versions in order - of preference (newest first) and returns the first valid match. + of preference and returns the first valid match. ComicInfo is checked newest + first; MetronInfo is checked oldest first, since each version is a superset of + the previous one. Returns: - The highest valid schema version found, or SchemaVersion.UNKNOWN + The first valid schema version found, or SchemaVersion.UNKNOWN if no valid schema is found. """ diff --git a/tests/test_validate.py b/tests/test_validate.py index b68f1e9..135b5f2 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -45,6 +45,7 @@ def test_schema_version_enum_values(): """Test that SchemaVersion enum has expected values.""" expected_values = [ "METRON_INFO_V1_2", + "METRON_INFO_V1_1", "COMIC_INFO_V2", "COMIC_INFO_V1", "UNKNOWN", @@ -58,6 +59,7 @@ def test_schema_version_string_representation(): assert str(SchemaVersion.COMIC_INFO_V1) == "Comic Info V1" assert str(SchemaVersion.COMIC_INFO_V2) == "Comic Info V2" assert str(SchemaVersion.METRON_INFO_V1_2) == "Metron Info V1.2" + assert str(SchemaVersion.METRON_INFO_V1_1) == "Metron Info V1.1" assert str(SchemaVersion.UNKNOWN) == "Unknown" @@ -221,10 +223,10 @@ def test_validate_first_schema_valid(valid_xml_bytes): validator = ValidateMetadata(valid_xml_bytes) with patch.object(validator, "_is_valid") as mock_is_valid: - mock_is_valid.side_effect = lambda schema: schema == SchemaVersion.METRON_INFO_V1_2 + mock_is_valid.side_effect = lambda schema: schema == SchemaVersion.METRON_INFO_V1_1 result = validator.validate() - assert result == SchemaVersion.METRON_INFO_V1_2 + assert result == SchemaVersion.METRON_INFO_V1_1 def test_validate_second_schema_valid(valid_xml_bytes): @@ -253,6 +255,7 @@ def test_validate_all_returns_all_results(valid_xml_bytes): validator = ValidateMetadata(valid_xml_bytes) expected_results = { + SchemaVersion.METRON_INFO_V1_1: False, SchemaVersion.METRON_INFO_V1_2: True, SchemaVersion.COMIC_INFO_V2: False, SchemaVersion.COMIC_INFO_V1: True, @@ -263,7 +266,7 @@ def test_validate_all_returns_all_results(valid_xml_bytes): results = validator.validate_all() assert results == expected_results - assert len(results) == 3 + assert len(results) == 4 # get_validation_errors method tests @@ -312,6 +315,7 @@ def test_get_validation_errors_no_errors(valid_xml_bytes): [ (SchemaVersion.COMIC_INFO_V1, "darkseid.schemas.ComicInfo.v1", "ComicInfo.xsd"), (SchemaVersion.COMIC_INFO_V2, "darkseid.schemas.ComicInfo.v2", "ComicInfo.xsd"), + (SchemaVersion.METRON_INFO_V1_1, "darkseid.schemas.MetronInfo.v1_1", "MetronInfo.xsd"), (SchemaVersion.METRON_INFO_V1_2, "darkseid.schemas.MetronInfo.v1_2", "MetronInfo.xsd"), ], ) @@ -328,6 +332,7 @@ def test_schema_config_mapping(schema_version, expected_module, expected_file): [ (SchemaVersion.COMIC_INFO_V1, XMLSchema10), (SchemaVersion.COMIC_INFO_V2, XMLSchema10), + (SchemaVersion.METRON_INFO_V1_1, XMLSchema11), (SchemaVersion.METRON_INFO_V1_2, XMLSchema11), ], ) @@ -341,6 +346,7 @@ def test_schema_class_mapping(schema_version, expected_class): def test_validation_order_consistency(): """Test that validation order is consistent with class definition.""" expected_order = [ + SchemaVersion.METRON_INFO_V1_1, SchemaVersion.METRON_INFO_V1_2, SchemaVersion.COMIC_INFO_V2, SchemaVersion.COMIC_INFO_V1, @@ -440,3 +446,23 @@ def test_validate_metron_info_v1_2_series(series_xml, expected): """Test validation of MetronInfo v1.2 series elements against the bundled schema.""" xml = f"{series_xml}" assert ValidateMetadata(xml.encode()).validate() == expected + + +@pytest.mark.parametrize( + ("series_xml", "expected"), + [ + ( + 'Foo1970', + SchemaVersion.METRON_INFO_V1_1, + ), + ( + "Foo19701975", + SchemaVersion.METRON_INFO_V1_2, + ), + ('Foo', SchemaVersion.METRON_INFO_V1_2), + ], +) +def test_validate_reports_oldest_metron_info_version(series_xml, expected): + """Test MetronInfo documents report the oldest schema version they conform to.""" + xml = f"{series_xml}" + assert ValidateMetadata(xml.encode()).validate() == expected From 3fb90708a1715ff8949da16baa64aca8c65ec562 Mon Sep 17 00:00:00 2001 From: Brian Pepple Date: Tue, 27 Oct 2026 19:22:59 -0400 Subject: [PATCH 3/6] Normalize BCP 47 language tags and resolve all ISO 639 codes Look up the whole value as a language code or name before parsing it as a tag. Hyphenated language names now resolve (Ai-Cham -> aih) instead of being split into a primary subtag and bogus subtags (Na-Dene -> na-Dene). Codes are checked before name lookup, since pycountry's lookup() can match a language name equal to another language's code ("En"). Resolve the primary subtag from its ISO 639-1, ISO 639-3 or ISO 639-2/B code whether or not subtags follow, so ger-DE, fre-CA and chi-Hant are accepted like ger, fre and chi. Parse subtags with BCP 47 rules: an optional script, an optional region and any variants, in that order. Scripts and alphabetic regions must be known ISO 15924 and ISO 3166-1 codes, and each subtag is normalized to its canonical case (pt-br -> pt-BR, zh-hant-tw -> zh-Hant-TW). Extended language, extension and private use subtags are rejected. --- darkseid/metadata/data_classes.py | 82 +++++++++++++++++++++++-------- tests/test_metadata.py | 30 +++++++++-- 2 files changed, 89 insertions(+), 23 deletions(-) diff --git a/darkseid/metadata/data_classes.py b/darkseid/metadata/data_classes.py index 7e69166..7a93256 100644 --- a/darkseid/metadata/data_classes.py +++ b/darkseid/metadata/data_classes.py @@ -17,6 +17,8 @@ if TYPE_CHECKING: from datetime import date, datetime + from pycountry.db import Data + import logging import re @@ -52,8 +54,10 @@ _COUNTRY_TO_CURRENCY: dict[str, str] = {v: k for k, v in _CURRENCY_TO_COUNTRY.items()} -# Script, region and variant subtags following the primary language subtag of a BCP 47 tag -_LANGUAGE_SUBTAGS_RE = re.compile(r"[a-zA-Z0-9]{1,8}(-[a-zA-Z0-9]{1,8})*") +# BCP 47 subtags that may follow the primary language subtag +_SCRIPT_SUBTAG_RE = re.compile(r"[a-zA-Z]{4}") +_REGION_SUBTAG_RE = re.compile(r"[a-zA-Z]{2}|\d{3}") +_VARIANT_SUBTAG_RE = re.compile(r"[a-zA-Z0-9]{5,8}|\d[a-zA-Z0-9]{3}") def currency_to_country(currency_code: str) -> str | None: @@ -82,13 +86,54 @@ def country_to_currency(country_code: str) -> str | None: return _COUNTRY_TO_CURRENCY.get(country_code) +def _get_language(code: str) -> Data | None: + """Get a language by its ISO 639-1, ISO 639-3 or ISO 639-2/B code.""" + if len(code) == COUNTRY_LEN: + return pycountry.languages.get(alpha_2=code) + if len(code) == LANGUAGE_ALPHA_3_LEN: + return pycountry.languages.get(alpha_3=code) or pycountry.languages.get(bibliographic=code) + return None + + +def _normalize_language_subtags(subtags: list[str]) -> list[str] | None: + """Validate and normalize the subtags following the primary language subtag. + + Subtags must be in BCP 47 order: an optional script, an optional region, then any + variants. Scripts and alphabetic regions must be known ISO 15924 and ISO 3166-1 codes. + + Returns: + The subtags in their canonical case, or None if they are malformed. + + """ + normalized = [] + remaining = iter(subtags) + subtag = next(remaining, None) + if subtag and _SCRIPT_SUBTAG_RE.fullmatch(subtag): + if pycountry.scripts.get(alpha_4=subtag) is None: + return None + normalized.append(subtag.title()) + subtag = next(remaining, None) + if subtag and _REGION_SUBTAG_RE.fullmatch(subtag): + if subtag.isalpha() and pycountry.countries.get(alpha_2=subtag) is None: + return None + normalized.append(subtag.upper()) + subtag = next(remaining, None) + while subtag is not None: + if not _VARIANT_SUBTAG_RE.fullmatch(subtag): + return None + normalized.append(subtag.lower()) + subtag = next(remaining, None) + return normalized + + def validate_language_tag(value: str | None) -> str | None: """Validate and normalize a BCP 47 language tag. - The primary language subtag is resolved with pycountry and normalized to its ISO 639-1 - code, falling back to the ISO 639-3 code for languages without one (e.g. "fil"). Any - script, region or variant subtags (e.g. "pt-BR", "zh-Hant") are kept as given. A value - without subtags may also be a language name (e.g. "English"). + The value is first looked up as a language code or name (e.g. "ger", "English", "Ai-Cham"), + then parsed as a tag. The primary language subtag is normalized to its ISO 639-1 code, + falling back to the ISO 639-3 code for languages without one (e.g. "fil"). Script, region + and variant subtags (e.g. "pt-BR", "zh-Hant") are normalized to their canonical case. + Extended language, extension and private use subtags aren't supported. Args: value: The language tag or name to validate. @@ -104,24 +149,21 @@ def validate_language_tag(value: str | None) -> str | None: return None value = value.strip() - primary, sep, subtags = value.partition("-") - if len(primary) == COUNTRY_LEN: - obj = pycountry.languages.get(alpha_2=primary) - elif len(primary) == LANGUAGE_ALPHA_3_LEN and sep: - obj = pycountry.languages.get(alpha_3=primary) - else: - sep = subtags = "" - try: - obj = pycountry.languages.lookup(value) - except LookupError as e: - msg = f"Couldn't find language {value}" - raise ValueError(msg) from e - if obj is None or (sep and not _LANGUAGE_SUBTAGS_RE.fullmatch(subtags)): + # Codes are checked before lookup(), which can match a language name equal to another + # language's code (e.g. "En" is a language name, not just English's ISO 639-1 code). + try: + obj = _get_language(value) or pycountry.languages.lookup(value) + subtags = [] + except LookupError: + primary, *rest = value.split("-") + obj = _get_language(primary) + subtags = _normalize_language_subtags(rest) if rest else [] + if obj is None or subtags is None: msg = f"Couldn't find language {value}" raise ValueError(msg) code = getattr(obj, "alpha_2", None) or obj.alpha_3 - return f"{code}-{subtags}" if subtags else code + return "-".join([code, *subtags]) class Validations: diff --git a/tests/test_metadata.py b/tests/test_metadata.py index 6ae06e0..d2e486e 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -159,10 +159,19 @@ def test_series_invalid_language(): ("fil", "fil"), ("yue", "yue"), ("pt-BR", "pt-BR"), - ("pt-br", "pt-br"), + ("pt-br", "pt-BR"), + ("EN-us", "en-US"), ("zh-Hant", "zh-Hant"), - ("zh-Hant-TW", "zh-Hant-TW"), + ("zh-hant-tw", "zh-Hant-TW"), ("fil-PH", "fil-PH"), + ("es-419", "es-419"), + ("de-CH-1901", "de-CH-1901"), + ("sl-ROZAJ", "sl-rozaj"), + ("ger", "de"), + ("ger-DE", "de-DE"), + ("fre-CA", "fr-CA"), + ("chi-Hant", "zh-Hant"), + ("Ai-Cham", "aih"), ("", None), (None, None), ], @@ -172,7 +181,22 @@ def test_series_bcp47_language(language, expected): assert Series("Foo Bar", language=language).language == expected -@pytest.mark.parametrize("language", ["xyz", "zz-BR", "pt-", "pt-toolongsubtag", "pt-B_R"]) +@pytest.mark.parametrize( + "language", + [ + "xyz", + "zz-BR", + "pt-", + "pt-toolongsubtag", + "pt-B_R", + "Na-Dene", + "zh-min-nan", + "en-XX", + "zh-Abcd", + "pt-BR-BR", + "en-US-x-private", + ], +) def test_series_invalid_bcp47_language(language): """Test Series rejects unknown languages and malformed subtags.""" with pytest.raises(ValueError, match="Couldn't find language"): From bbee743a2023bf94df657c51355996c944168377 Mon Sep 17 00:00:00 2001 From: Brian Pepple Date: Tue, 27 Oct 2026 19:29:37 -0400 Subject: [PATCH 4/6] Share one synonym lookup across MetronInfo normalizers Add MetronInfo._match_mapping and use it for series format, series status and ComicRack age ratings, replacing three copies of the same strip/lower/lookup code. Series status now uses a STATUS_MAPPINGS table like FORMAT_MAPPINGS and RATING_MAPPINGS. ComicRack age ratings are now stripped of surrounding whitespace before matching, like format and status already were. --- darkseid/metadata/metroninfo.py | 55 ++++++++++++++++++++------------- tests/test_metroninfo.py | 2 ++ 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/darkseid/metadata/metroninfo.py b/darkseid/metadata/metroninfo.py index c768b66..e0a576c 100644 --- a/darkseid/metadata/metroninfo.py +++ b/darkseid/metadata/metroninfo.py @@ -36,6 +36,7 @@ ) if TYPE_CHECKING: + from collections.abc import Mapping from datetime import date # Constants @@ -79,8 +80,6 @@ } ) -SERIES_STATUSES = ("Cancelled", "Completed", "Hiatus", "Ongoing") - VALID_ROLES = frozenset( { "writer", @@ -153,6 +152,14 @@ "Trade Paperback": frozenset({"trade paperback", "tpb", "trade paper back"}), } +# Series status mappings +STATUS_MAPPINGS = { + "Cancelled": frozenset({"cancelled"}), + "Completed": frozenset({"completed"}), + "Hiatus": frozenset({"hiatus"}), + "Ongoing": frozenset({"ongoing"}), +} + class MetronInfo(BaseMetadataHandler): """A class to manage comic metadata and its MetronInfo XML representation. @@ -339,6 +346,27 @@ def _build_id_attrib(id_: int | str | None) -> dict[str, str]: """ return {"id": str(id_)} if id_ else {} + @staticmethod + def _match_mapping(value: str | None, mappings: Mapping[str, frozenset[str]]) -> str | None: + """Match a value case-insensitively against a mapping of canonical values to synonyms. + + Args: + value: Value to match. + mappings: Mapping of canonical values to their lowercase synonyms. + + Returns: + The canonical value whose synonyms include the value, or None. + + """ + if not value: + return None + + lower_val = value.strip().lower() + return next( + (canonical for canonical, synonyms in mappings.items() if lower_val in synonyms), + None, + ) + @staticmethod def _normalize_age_rating(age_rating: AgeRatings | None) -> str | None: """Normalize age rating to valid MetronInfo format. @@ -360,13 +388,7 @@ def _normalize_age_rating(age_rating: AgeRatings | None) -> str | None: else age_rating.metron_info ) - if age_rating.comic_rack: - lower_val = age_rating.comic_rack.lower() - for rating, synonyms in RATING_MAPPINGS.items(): - if lower_val in synonyms: - return rating - - return None + return MetronInfo._match_mapping(age_rating.comic_rack, RATING_MAPPINGS) @staticmethod def _normalize_series_format(format_str: str | None) -> str | None: @@ -379,14 +401,7 @@ def _normalize_series_format(format_str: str | None) -> str | None: Normalized format or None. """ - if not format_str: - return None - - lower_val = format_str.strip().lower() - return next( - (fmt for fmt, synonyms in FORMAT_MAPPINGS.items() if lower_val in synonyms), - None, - ) + return MetronInfo._match_mapping(format_str, FORMAT_MAPPINGS) @staticmethod def _normalize_series_status(status: str | None) -> str | None: @@ -399,11 +414,7 @@ def _normalize_series_status(status: str | None) -> str | None: Normalized status or None. """ - if not status: - return None - - lower_val = status.strip().lower() - return next((val for val in SERIES_STATUSES if val.lower() == lower_val), None) + return MetronInfo._match_mapping(status, STATUS_MAPPINGS) @staticmethod def _set_date_element(root: ET.Element, tag: str, date_val: date | None = None) -> None: diff --git a/tests/test_metroninfo.py b/tests/test_metroninfo.py index ab735f1..322ed58 100644 --- a/tests/test_metroninfo.py +++ b/tests/test_metroninfo.py @@ -253,6 +253,8 @@ def test_series_format_edge_cases(metron_info): (AgeRatings(comic_rack="PG"), "Teen", "comic_rack_pg"), (AgeRatings(comic_rack="R18+"), "Mature", "comic_rack_r18"), (AgeRatings(comic_rack="M"), "Mature", "comic_rack_m"), + (AgeRatings(comic_rack=" pg "), "Teen", "comic_rack_whitespace"), + (AgeRatings(comic_rack="NotARating"), None, "unknown_comic_rack_rating"), (AgeRatings(metron_info=""), None, "empty_metron_rating"), (AgeRatings(comic_rack=""), None, "empty_comic_rack_rating"), (AgeRatings(), None, "empty_ratings_object"), From c599ab14c9e9fa6ec03bf626ea78db6feb9aabcf Mon Sep 17 00:00:00 2001 From: Brian Pepple Date: Tue, 27 Oct 2026 19:32:07 -0400 Subject: [PATCH 5/6] Validate Series end year separately and check year order Give Series.validate_end_year its own docstring and error message instead of aliasing validate_start_year, sharing a _validate_year helper. Year errors now name the field ("End year: 99 length must be 4"). Series now rejects an end year that precedes its start year at construction. The MetronInfo writer keeps its EndYear guard, since fields can be set after construction, a volume may be written as the StartYear, and the Ongoing rule depends on the normalized status. --- darkseid/metadata/data_classes.py | 49 ++++++++++++++++++++++++++----- tests/test_metadata.py | 13 ++++++-- tests/test_metroninfo.py | 6 ++-- 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/darkseid/metadata/data_classes.py b/darkseid/metadata/data_classes.py index 7a93256..172882f 100644 --- a/darkseid/metadata/data_classes.py +++ b/darkseid/metadata/data_classes.py @@ -166,6 +166,18 @@ def validate_language_tag(value: str | None) -> str | None: return "-".join([code, *subtags]) +def _validate_year(value: int | None, label: str) -> int | None: + """Validate a four-digit year, using the label in the error message.""" + if not value: + return None + + if len(str(value)) == YEAR_LEN: + return value + + msg = f"{label}: {value} length must be {YEAR_LEN}" + raise ValueError(msg) + + class Validations: """A base class for data validation in dataclasses. @@ -434,16 +446,39 @@ def validate_start_year(value: int, **_: any) -> int | None: ValueError: Raised when the year is not exactly four digits. """ - if not value: - return None + return _validate_year(value, "Start year") - if len(str(value)) == YEAR_LEN: - return value + @staticmethod + def validate_end_year(value: int, **_: any) -> int | None: + """Validate an end year value. - msg = f"Year: {value} length must be {YEAR_LEN}" - raise ValueError(msg) + This method checks if the provided value is a valid four-digit year. If the value is not present, + it returns None. If the value is not exactly four digits, it raises a ValueError. + + Args: + value (int): The year value to validate. + **_ (any): Additional keyword arguments (ignored). + + Returns: + Optional[int]: The validated year value, or None if the value is not present. + + Raises: + ValueError: Raised when the year is not exactly four digits. - validate_end_year = validate_start_year + """ + return _validate_year(value, "End year") + + def __post_init__(self: Series) -> None: + """Validate the series fields and check the end year doesn't precede the start year. + + Raises: + ValueError: Raised when a field is invalid or the end year precedes the start year. + + """ + super().__post_init__() + if self.start_year and self.end_year and self.end_year < self.start_year: + msg = f"End year: {self.end_year} can't precede start year: {self.start_year}" + raise ValueError(msg) @staticmethod def validate_language(value: str, **_: any) -> str | None: diff --git a/tests/test_metadata.py b/tests/test_metadata.py index d2e486e..adc4f84 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -208,10 +208,19 @@ def test_series_end_year_validation(): assert Series("Superman", start_year=1938, end_year=2011).end_year == 2011 assert Series("Superman", end_year=None).end_year is None - with pytest.raises(ValueError, match="length must be 4"): + with pytest.raises(ValueError, match="End year: 99 length must be 4"): Series("Superman", end_year=99) +def test_series_end_year_before_start_year(): + """Test Series rejects an end year before the start year.""" + assert Series("Superman", start_year=1938, end_year=1938).end_year == 1938 + assert Series("Superman", end_year=1938).end_year == 1938 + + with pytest.raises(ValueError, match="End year: 1937 can't precede start year: 1938"): + Series("Superman", start_year=1938, end_year=1937) + + def test_series_start_year_validation(): """Test Series start_year validation.""" # Valid year @@ -223,7 +232,7 @@ def test_series_start_year_validation(): assert series_none.start_year is None # Invalid year length - with pytest.raises(ValueError, match=r"Year: .* length must be 4"): + with pytest.raises(ValueError, match=r"Start year: .* length must be 4"): Series("Superman", start_year=38) diff --git a/tests/test_metroninfo.py b/tests/test_metroninfo.py index 322ed58..48bedf3 100644 --- a/tests/test_metroninfo.py +++ b/tests/test_metroninfo.py @@ -356,9 +356,9 @@ def test_series_v1_2_fields_round_trip(metron_info, tmp_path): def test_series_end_year_omitted_when_invalid(metron_info, tmp_path, start_year, end_year, status): """An EndYear the schema would reject is dropped rather than failing the write.""" metadata = Metadata() - metadata.series = Series( - name="Test Series", start_year=start_year, end_year=end_year, status=status - ) + metadata.series = Series(name="Test Series", start_year=start_year, status=status) + # Set after construction, which Series doesn't validate (e.g. when parsed from XML) + metadata.series.end_year = end_year filename = tmp_path / "MetronInfo.xml" metron_info.write_xml(filename, metadata) From 9047cc93c6c7e05529f87100352bb46d42494ad9 Mon Sep 17 00:00:00 2001 From: Brian Pepple Date: Tue, 27 Oct 2026 19:41:46 -0400 Subject: [PATCH 6/6] Normalize series and alternative name languages on read Run the series language (ComicInfo LanguageISO and MetronInfo Series lang) and MetronInfo AlternativeName languages through validate_language_tag when reading, via a new BaseMetadataHandler._parse_language helper. Series languages were previously stored as read, so pt-br stayed pt-br while alternative names were normalized. An unknown language is now logged and dropped instead of raising. Previously, an AlternativeName with an unknown lang failed the whole MetronInfo read with ValueError. --- darkseid/metadata/base_handler.py | 23 +++++++++++++++++++++++ darkseid/metadata/comicinfo.py | 2 +- darkseid/metadata/metroninfo.py | 4 ++-- tests/test_comicinfo.py | 17 +++++++++++++++++ tests/test_metroninfo.py | 20 ++++++++++++++++++++ 5 files changed, 63 insertions(+), 3 deletions(-) diff --git a/darkseid/metadata/base_handler.py b/darkseid/metadata/base_handler.py index bff82e6..41c7f76 100644 --- a/darkseid/metadata/base_handler.py +++ b/darkseid/metadata/base_handler.py @@ -2,6 +2,7 @@ from __future__ import annotations +import logging import xml.etree.ElementTree as ET from abc import ABC, abstractmethod from datetime import UTC, date, datetime @@ -11,10 +12,15 @@ from defusedxml.ElementTree import parse +from darkseid.metadata.data_classes import validate_language_tag + if TYPE_CHECKING: from darkseid.metadata.data_classes import Metadata +logger = logging.getLogger(__name__) + + class XmlError(Exception): """Class for an XML error.""" @@ -214,6 +220,23 @@ def _parse_int(value: str | None) -> int | None: """ return int(value) if value and value.isdigit() else None + @staticmethod + def _parse_language(value: str | None) -> str | None: + """Safely parse a language tag or name to a normalized BCP 47 tag. + + Args: + value: Language value to parse. + + Returns: + Normalized language tag, or None if the value is empty or not a known language. + + """ + try: + return validate_language_tag(value) + except ValueError: + logger.warning("Dropping unknown language: %s", value) + return None + @staticmethod def _parse_date(value: str | None) -> date | None: """Safely parse string to a date. diff --git a/darkseid/metadata/comicinfo.py b/darkseid/metadata/comicinfo.py index 5c20262..eaced76 100644 --- a/darkseid/metadata/comicinfo.py +++ b/darkseid/metadata/comicinfo.py @@ -440,7 +440,7 @@ def _convert_xml_to_metadata(self, tree: ET.ElementTree) -> Metadata: # noqa: C md.genres = self._string_to_resource(self._get_text_content(root, "Genre")) md.web_link = self._parse_urls(self._get_text_content(root, "Web")) - md.series.language = self._get_text_content(root, "LanguageISO") + md.series.language = self._parse_language(self._get_text_content(root, "LanguageISO")) md.series.format = self._get_text_content(root, "Format") md.manga = self._get_text_content(root, "Manga") md.characters = self._string_to_resource(self._get_text_content(root, "Characters")) diff --git a/darkseid/metadata/metroninfo.py b/darkseid/metadata/metroninfo.py index e0a576c..8abdddb 100644 --- a/darkseid/metadata/metroninfo.py +++ b/darkseid/metadata/metroninfo.py @@ -865,7 +865,7 @@ def _parse_series_element(self, series_element: ET.Element | None) -> Series | N series = Series(DEFAULT_SERIES_NAME) series.id_ = self._get_id_from_attrib(series_element.attrib) - series.language = series_element.attrib.get("lang") + series.language = self._parse_language(series_element.attrib.get("lang")) # Define mapping for simple text elements text_mappings = { @@ -898,7 +898,7 @@ def _parse_series_element(self, series_element: ET.Element | None) -> Series | N alt_name = AlternativeNames( name_elem.text, self._get_id_from_attrib(name_elem.attrib), - name_elem.attrib.get("lang"), + self._parse_language(name_elem.attrib.get("lang")), ) alt_names.append(alt_name) series.alternative_names = alt_names diff --git a/tests/test_comicinfo.py b/tests/test_comicinfo.py index 429f7ac..883c9ff 100644 --- a/tests/test_comicinfo.py +++ b/tests/test_comicinfo.py @@ -395,3 +395,20 @@ def test_review_parsed_from_xml_string() -> None: ) md = ComicInfo().metadata_from_string(xml) assert md.review == "A fantastic underwater adventure." + + +@pytest.mark.parametrize( + ("language", "expected"), + [("pt-br", "pt-BR"), ("English", "en"), ("unknown", None)], +) +def test_language_iso_normalized_on_read(language: str, expected: str | None) -> None: + """Test LanguageISO is normalized on read, and an unknown language is dropped.""" + xml = ( + '' + "" + "Aquaman" + f"{language}" + "" + ) + md = ComicInfo().metadata_from_string(xml) + assert md.series.language == expected diff --git a/tests/test_metroninfo.py b/tests/test_metroninfo.py index 48bedf3..ef0d45e 100644 --- a/tests/test_metroninfo.py +++ b/tests/test_metroninfo.py @@ -415,6 +415,26 @@ def test_series_bcp47_language_round_trip(metron_info, tmp_path): assert [n.language for n in result.series.alternative_names] == ["fil", "zh-Hant"] +def test_series_languages_normalized_on_read(metron_info, caplog): + """Series and alternative name languages are normalized, and unknown ones dropped.""" + xml = """ + + + Test Series + + 蜘蛛人 + Unknown + + + """ + + result = metron_info.metadata_from_string(xml) + + assert result.series.language == "pt-BR" + assert [n.language for n in result.series.alternative_names] == ["zh-Hant", None] + assert "Dropping unknown language: zz" in caplog.text + + # XML Conversion Tests def test_convert_complex_metadata_to_xml(metron_info, complex_metadata): """Test XML conversion with complex metadata."""