diff --git a/pyproject.toml b/pyproject.toml index 75a8f8925..ebb5e973a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,14 +38,14 @@ pytest = "^8.3.2" time-machine = "^2.15.0" [tool.poetry.group.dev.dependencies] -ruff = "^0.10.0" +ruff = "^0.9.0" datamodel-code-generator = {extras = ["http"], version = "^0.26.0"} commitizen = "^4.0.0" pre-commit = "^4.0.1" [tool.poetry.group.docs.dependencies] jupyter = "^1.1.1" -quartodoc = "^0.10.0" +quartodoc = "^0.9.0" [build-system] requires = ["poetry-core"] diff --git a/seedcase_sprout/core/sprout_checks/check_package_properties.py b/seedcase_sprout/core/sprout_checks/check_package_properties.py index 7a9f6b595..1b5f6e01f 100644 --- a/seedcase_sprout/core/sprout_checks/check_package_properties.py +++ b/seedcase_sprout/core/sprout_checks/check_package_properties.py @@ -23,7 +23,7 @@ def check_package_properties(properties: dict, check_required=True) -> dict: `properties` if all checks pass. Raises: - ExceptionGroup: A group of `CheckError`s, one for each check that failed. + ExceptionGroup: A group of `CheckError`s, one error per failed check. """ errors = checks.check_package_properties(properties) diff --git a/seedcase_sprout/core/sprout_checks/check_properties.py b/seedcase_sprout/core/sprout_checks/check_properties.py new file mode 100644 index 000000000..c2ff04cc0 --- /dev/null +++ b/seedcase_sprout/core/sprout_checks/check_properties.py @@ -0,0 +1,48 @@ +from seedcase_sprout.core import checks +from seedcase_sprout.core.sprout_checks.exclude_non_sprout_resource_errors import ( + exclude_non_sprout_resource_errors, +) +from seedcase_sprout.core.sprout_checks.get_sprout_package_errors import ( + get_sprout_package_errors, +) +from seedcase_sprout.core.sprout_checks.get_sprout_resource_errors import ( + get_sprout_resource_errors, +) + + +def check_properties(properties: dict, check_required=True) -> dict: + """Checks that `properties` matches requirements in Sprout. + + `properties` is checked against the Data Package standard and Sprout-specific + requirements. Both package and resource properties are checked. + + Args: + properties: The full package properties to check, including resource properties. + check_required: Whether the function should enforce the presence of required + fields. Defaults to True. + + Returns: + `properties`, if all checks pass. + + Raises: + ExceptionGroup: A group of `CheckError`s, one error per failed check. + """ + errors = checks.check_properties(properties) + errors = exclude_non_sprout_resource_errors(errors) + + if not check_required: + errors = [error for error in errors if error.validator != "required"] + + errors += get_sprout_package_errors(properties, check_required) + + for index, resource in enumerate(properties.get("resources", [])): + errors += get_sprout_resource_errors(resource, check_required, index) + + errors = sorted(set(errors)) + + if errors: + raise ExceptionGroup( + f"The follow checks failed on the properties:\n{properties}", errors + ) + + return properties diff --git a/seedcase_sprout/core/sprout_checks/check_resource_properties.py b/seedcase_sprout/core/sprout_checks/check_resource_properties.py index ff3825961..95820553d 100644 --- a/seedcase_sprout/core/sprout_checks/check_resource_properties.py +++ b/seedcase_sprout/core/sprout_checks/check_resource_properties.py @@ -29,7 +29,7 @@ def check_resource_properties(properties: dict, check_required: bool = True) -> `properties`, if all checks passed. Raises: - ExceptionGroup: A group of `CheckError`s, one for each check that failed. + ExceptionGroup: A group of `CheckError`s, one error per failed check. """ errors = checks.check_resource_properties(properties) errors = exclude_non_sprout_resource_errors(errors) diff --git a/tests/core/sprout_checks/test_check_properties_sprout.py b/tests/core/sprout_checks/test_check_properties_sprout.py new file mode 100644 index 000000000..b5b0163eb --- /dev/null +++ b/tests/core/sprout_checks/test_check_properties_sprout.py @@ -0,0 +1,171 @@ +from pathlib import Path + +from pytest import fixture, mark, raises + +from seedcase_sprout.core.checks.check_error import CheckError +from seedcase_sprout.core.properties import PackageProperties, ResourceProperties +from seedcase_sprout.core.sprout_checks.check_properties import check_properties +from seedcase_sprout.core.sprout_checks.get_blank_value_for_type import ( + get_blank_value_for_type, +) +from seedcase_sprout.core.sprout_checks.required_fields import ( + PACKAGE_SPROUT_REQUIRED_FIELDS, + RESOURCE_SPROUT_REQUIRED_FIELDS, +) + + +@fixture +def properties(): + return PackageProperties( + name="package-1", + id="abc1", + title="Package 1", + description="A package.", + version="1.0.0", + created="2024-05-14T05:00:01+00:00", + licenses=[{"name": "a-license"}], + contributors=[{"title": "a contributor"}], + sources=[{"title": "a source"}], + resources=[ + ResourceProperties( + name="resource-1", + path=str(Path("resources", "1", "data.parquet")), + title="Resource 1", + description="A resource.", + ), + ResourceProperties( + name="resource-2", + path=str(Path("resources", "2", "data.parquet")), + title="Resource 2", + description="A second resource.", + ), + ], + ).compact_dict + + +@mark.parametrize("check_required", [True, False]) +def test_check_passes_full_properties(properties, check_required): + """Should pass if all required fields are present and correct.""" + assert check_properties(properties, check_required=check_required) == properties + + +def test_passes_partial_properties_without_required_check(): + """Should pass properties with missing required fields when these are not + enforced.""" + assert check_properties({}, check_required=False) == {} + + +@mark.parametrize("field", [*PACKAGE_SPROUT_REQUIRED_FIELDS.keys(), "resources"]) +def test_raises_error_if_package_required_field_is_missing(properties, field): + """Should raise an error if a required field is missing among the package + properties.""" + del properties[field] + + with raises(ExceptionGroup) as error_info: + check_properties(properties, check_required=True) + + errors = error_info.value.exceptions + assert len(errors) == 1 + assert errors[0].json_path == f"$.{field}" + assert errors[0].validator == "required" + + +@mark.parametrize("field", RESOURCE_SPROUT_REQUIRED_FIELDS.keys()) +def test_raises_error_if_resource_required_field_is_missing(properties, field): + """Should raise an error if a required field is missing among the resource + properties.""" + del properties["resources"][0][field] + + with raises(ExceptionGroup) as error_info: + check_properties(properties, check_required=True) + + errors = error_info.value.exceptions + assert len(errors) == 1 + assert errors[0].json_path == f"$.resources[0].{field}" + assert errors[0].validator == "required" + + +@mark.parametrize("check_required", [True, False]) +@mark.parametrize("name,type", PACKAGE_SPROUT_REQUIRED_FIELDS.items()) +def test_raises_error_if_package_field_is_blank(properties, name, type, check_required): + """Should raise an error if there is one required package field that is present but + blank.""" + properties[name] = get_blank_value_for_type(type) + + with raises(ExceptionGroup) as error_info: + check_properties(properties, check_required=check_required) + + blank_errors = [ + error for error in error_info.value.exceptions if error.validator == "blank" + ] + + assert len(blank_errors) == 1 + assert blank_errors[0].json_path == f"$.{name}" + + +@mark.parametrize("check_required", [True, False]) +@mark.parametrize("name,type", RESOURCE_SPROUT_REQUIRED_FIELDS.items()) +def test_raises_error_if_resource_field_is_blank( + properties, name, type, check_required +): + """Should raise an error if there is one required resource field that is present + but blank.""" + properties["resources"][0][name] = get_blank_value_for_type(type) + + with raises(ExceptionGroup) as error_info: + check_properties(properties, check_required=check_required) + + blank_errors = [ + error for error in error_info.value.exceptions if error.validator == "blank" + ] + + assert len(blank_errors) == 1 + assert blank_errors[0].json_path == f"$.resources[0].{name}" + + +@mark.parametrize("check_required", [True, False]) +def test_raises_error_if_there_are_both_package_and_resource_errors( + properties, check_required +): + """Should raise `CheckError`s if there are both package and resource errors.""" + properties["name"] = "space in name" + properties["title"] = 123 + properties["resources"][0]["name"] = "space in name" + properties["resources"][1]["path"] = "/bad path" + properties["resources"][1]["data"] = "some data" + + with raises(ExceptionGroup) as error_info: + check_properties(properties, check_required=check_required) + + errors = error_info.value.exceptions + assert [error.json_path for error in errors] == [ + "$.name", + "$.resources[0].name", + "$.resources[1].data", + "$.resources[1].path", + "$.resources[1].path", + "$.title", + ] + assert [error.validator for error in errors] == [ + "pattern", + "pattern", + "inline-data", + "pattern", + "pattern", + "type", + ] + + +@mark.parametrize("check_required", [True, False]) +def test_raises_error_for_only_sprout_specific_errors(properties, check_required): + """Errors should be triggered by only those Data Package standard violations that + are relevant for Sprout.""" + properties["resources"][0]["path"] = 123 + + with raises(ExceptionGroup) as error_info: + check_properties(properties, check_required=check_required) + + errors = error_info.value.exceptions + assert errors == ( + CheckError("123 is not of type 'string'", "$.resources[0].path", "type"), + )