diff --git a/src/specify_cli/integrations/catalog.py b/src/specify_cli/integrations/catalog.py index e18d30a6fa..e93dab5185 100644 --- a/src/specify_cli/integrations/catalog.py +++ b/src/specify_cli/integrations/catalog.py @@ -674,16 +674,37 @@ def __init__(self, descriptor_path: Path) -> None: @staticmethod def _load(path: Path) -> dict: try: - with open(path, "r", encoding="utf-8") as fh: - return yaml.safe_load(fh) or {} - except yaml.YAMLError as exc: - raise IntegrationDescriptorError(f"Invalid YAML in {path}: {exc}") + text = path.read_text(encoding="utf-8") except FileNotFoundError: raise IntegrationDescriptorError(f"Descriptor not found: {path}") except (OSError, UnicodeError) as exc: raise IntegrationDescriptorError( f"Unable to read descriptor {path}: {exc}" ) + try: + # ``safe_load`` returns None for BOTH an empty document and an + # explicit null scalar (``null``, ``~``, ``Null``, ``NULL``), so it + # cannot tell them apart on its own. ``compose`` yields no node + # only for a genuinely empty document. + node = yaml.compose(text) + data = yaml.safe_load(text) + is_empty_document = node is None or ( + data is None + and isinstance(node, yaml.nodes.ScalarNode) + and node.value == "" + and node.start_mark.index == node.end_mark.index + ) + except yaml.YAMLError as exc: + raise IntegrationDescriptorError(f"Invalid YAML in {path}: {exc}") + # Only a genuinely EMPTY document becomes an empty mapping, so its + # missing-field errors are reported. Every non-mapping document -- + # including an explicit ``null``/``~`` and the falsy shapes ``[]``, + # ``false``, ``0``, ``''`` that a plain ``or {}`` would mask -- must + # reach ``_validate`` unchanged so it reports the wrong descriptor + # shape, like the truthy twins (``- a``, ``hello``) already do. + if is_empty_document: + data = {} + return data # -- Validation ------------------------------------------------------- diff --git a/tests/integrations/test_integration_catalog.py b/tests/integrations/test_integration_catalog.py index 9b02632992..87ab98a4d0 100644 --- a/tests/integrations/test_integration_catalog.py +++ b/tests/integrations/test_integration_catalog.py @@ -700,6 +700,36 @@ def test_scripts_not_a_list(self, tmp_path): with pytest.raises(IntegrationDescriptorError, match="expected a list"): IntegrationDescriptor(p) + @pytest.mark.parametrize( + "content", ["[]", "false", "0", "''", "null", "~", "NULL", "- a", "hello"] + ) + def test_falsy_non_mapping_descriptor_reports_shape_error(self, tmp_path, content): + """Every non-mapping document reports the mapping-shape error. + + `_validate` opens with an `isinstance(self.data, dict)` check, so a + truthy non-mapping (`- a`, `hello`) correctly reported "Descriptor root + must be a YAML mapping". `_load`'s plain `yaml.safe_load(fh) or {}` + masked that for the falsy shapes `[]`, `false`, `0`, `''` (coerced to + an empty mapping) and for an explicit null scalar (`null`, `~`, `NULL` + -- indistinguishable from an empty document by `safe_load` alone), so + those five reported "Missing required field: schema_version" instead. + """ + p = tmp_path / "integration.yml" + p.write_text(content) + with pytest.raises( + IntegrationDescriptorError, + match="Descriptor root must be a YAML mapping", + ): + IntegrationDescriptor(p) + + @pytest.mark.parametrize("content", ["", "---"]) + def test_empty_document_still_reports_missing_fields(self, tmp_path, content): + """Empty documents are normalized to an empty mapping, so missing fields are reported.""" + p = tmp_path / "integration.yml" + p.write_text(content) + with pytest.raises(IntegrationDescriptorError, match="Missing required field: schema_version"): + IntegrationDescriptor(p) + def test_file_not_found(self, tmp_path): with pytest.raises(IntegrationDescriptorError, match="Descriptor not found"): IntegrationDescriptor(tmp_path / "nonexistent.yml")