PR comments addressed

Author: SarahJohnsonONS

dpytools/validation/json/validation.py

@@ -6,63 +6,77 @@
 import jsonschema
 from jsonschema import ValidationError
 
-"""
-# data_dict is for passing in a dictionary to be validated
-validate_json_schema("/path/to/schema.json", data_dict=some_dictionary)
-
-# msg is to include a helpful context when debugging (i.e "what we're validating")
-validate_json_schema("/path/to/schema.json", data_dict=some_dictionary, msg="Some helpful message should this validation fail")
-
-validate_json_schema("/path/to/schema.json", data_path="/path/to/some/json", msg="Some helpful message should this validation fail")
-
-# indent should pretty print the json contents of the error to make it
-# more easily parsed by humans
-validate_json_schema("/path/to/schema.json", data_dict=some_dictionary, indent=2)
-"""
-
 
 def validate_json_schema(
-    schema_path: str,
-    data_dict_or_path: Union[Dict, str],
-    msg: str = "",
-    indent: int = 2,
-) -> Optional[ValidationError]:
+    schema_path: Union[Path, str],
+    data_dict: Optional[Dict] = None,
+    data_path: Union[Path, str, None] = None,
+    error_msg: Optional[str] = "",
+    indent: Optional[int] = 2,
+):
     """
-    Validate metadata.json files against schema provided.
+    Validate metadata.json files against schema.
+
+    Either `data_dict` or `data_path` must be provided.
 
-    `schema_path`: file path of schema to validate against
-    `data_dict_or_path`: file path or dictionary of data to be validated
-    `msg`: optional string to provide additional information about validation
-    `indent`: optional integer to be used when indenting json output
+    `msg` and `indent` are used to format the error message if validation fails.
     """
+    # Confirm that *either* `data_dict` *or* `data_path` has been provided, otherwise raise ValueError
+    if data_dict and data_path:
+        raise ValueError(
+            "Both a dictionary and file path of data have been provided - please specify either one or the other, not both."
+        )
+    if data_dict is None and data_path is None:
+        raise ValueError(
+            "Please provide either a dictionary or a file path of the data to be validated against the schema."
+        )
+
     # Load schema as dict
-    parsed_schema_path = urlparse(schema_path)
-    if parsed_schema_path.scheme == "http":
-        # TODO Load schema from URL
-        raise NotImplementedError("Validation from remote schema not yet supported")
-    else:
-        with open(Path(schema_path), "r") as f:
-            schema_from_path = json.load(f)
+    if isinstance(schema_path, str):
+        parsed_schema_path = urlparse(schema_path)
+        if parsed_schema_path.scheme == "http":
+            # TODO Load schema from URL
+            raise NotImplementedError("Validation from remote schema not yet supported")
+        schema_path = Path(schema_path).absolute()
+    if not schema_path.exists():
+        raise ValueError(f"Schema path '{schema_path}' does not exist")
+    with open(schema_path, "r") as f:
+        schema_from_path = json.load(f)
 
-    # Load data as dict
-    if isinstance(data_dict_or_path, Dict):
-        data_to_validate = data_dict_or_path
-    elif isinstance(data_dict_or_path, str):
-        with open(Path(data_dict_or_path), "r") as f:
+    # Load data to be validated as dict
+    if data_dict:
+        if not isinstance(data_dict, Dict):
+            raise ValueError("Invalid data format")
+        data_to_validate = data_dict
+
+    if data_path:
+        if isinstance(data_path, str):
+            data_path = Path(data_path).absolute()
+        if not isinstance(data_path, Path):
+            raise ValueError("Invalid data format")
+        if not data_path.exists():
+            raise ValueError(f"Data path '{data_path}' does not exist")
+        with open(data_path, "r") as f:
             data_to_validate = json.load(f)
-    else:
-        raise ValueError("Invalid data format")
 
     # Validate data against schema
-    print(msg)
     try:
         jsonschema.validate(data_to_validate, schema_from_path)
     except jsonschema.ValidationError as err:
         # TODO Handle jsonschema.SchemaError?
-        print(f"Error when validating data: {err.message}")
-        # If the error relates to a specific field, print the error's location
+        # If error is in a specific field, get the JSON path of the error location
         if err.json_path != "$":
-            print(f"Error in data field: {err.json_path}")
-        # Print the data that failed validation
-        print(f"Contents of data:\n{json.dumps(data_to_validate, indent=indent)}")
-        return err
+            error_location = err.json_path
+        else:
+            error_location = "JSON data"
+        # Create formatted message to be output on ValidationError
+        if error_msg or indent:
+            formatted_msg = f"""
+Exception: {error_msg}
+Error details: {err.message}
+Error location: {error_location}
+JSON data:
+{json.dumps(data_to_validate, indent=indent)}
+"""
+            raise ValidationError(formatted_msg) from err
+        raise err

tests/test_json_validation.py

@@ -1,4 +1,5 @@
 from pathlib import Path
+from jsonschema import ValidationError
 
 import pytest
 from dpytools.validation.json.validation import validate_json_schema
@@ -12,9 +13,9 @@ def test_validate_json_schema_data_path():
     pipeline_config = "tests/test_cases/pipeline_config.json"
     assert (
         validate_json_schema(
-            pipeline_config_schema,
-            pipeline_config,
-            "Validating pipeline_config.json",
+            schema_path=pipeline_config_schema,
+            data_path=pipeline_config,
+            error_msg="Validating pipeline_config.json",
         )
         is None
     )
@@ -35,22 +36,74 @@ def test_validate_json_schema_data_dict():
     }
     assert (
         validate_json_schema(
-            pipeline_config_schema,
-            pipeline_config,
-            "Validating pipeline_config dict",
+            schema_path=pipeline_config_schema,
+            data_dict=pipeline_config,
+            error_msg="Validating pipeline_config dict",
         )
         is None
     )
 
 
-def test_validate_json_schema_invalid_data_format():
+def test_validate_json_schema_data_dict_and_data_path():
+    pipeline_config_schema = "tests/test_cases/pipeline_config_schema.json"
+    pipeline_config_path = "tests/test_cases/pipeline_config.json"
+    pipeline_config_dict = {
+        "schema": "airflow.schemas.ingress.sdmx.v1.schema.json",
+        "required_files": [{"matches": "*.sdmx", "count": 1}],
+        "supplementary_distributions": [{"matches": "*.sdmx", "count": 1}],
+        "priority": 1,
+        "contact": ["[email protected]"],
+        "pipeline": "default",
+    }
+    with pytest.raises(ValueError) as err:
+        validate_json_schema(
+            schema_path=pipeline_config_schema,
+            data_dict=pipeline_config_dict,
+            data_path=pipeline_config_path,
+        )
+    assert (
+        "Both a dictionary and file path of data have been provided - please specify either one or the other, not both."
+        in str(err.value)
+    )
+
+
+def test_validate_json_schema_no_data_dict_or_data_path():
+    pipeline_config_schema = "tests/test_cases/pipeline_config_schema.json"
+
+    with pytest.raises(ValueError) as err:
+        validate_json_schema(
+            schema_path=pipeline_config_schema,
+        )
+    assert (
+        "Please provide either a dictionary or a file path of the data to be validated against the schema."
+        in str(err.value)
+    )
+
+
+def test_validate_json_schema_invalid_data_path_format():
+    """
+    Raise ValueError if data_path is not a file path
+    """
+    pipeline_config_schema = "tests/test_cases/pipeline_config_schema.json"
+    pipeline_config = ["Invalid", "data", "format"]
+    with pytest.raises(ValueError) as err:
+        validate_json_schema(
+            schema_path=pipeline_config_schema, data_path=pipeline_config
+        )
+    assert "Invalid data format" in str(err.value)
+
+
+def test_validate_json_schema_invalid_data_dict_format():
     """
-    Raise ValueError if data is not a file path or dictionary
+    Raise ValueError if data_dict is not a dictionary
     """
     pipeline_config_schema = "tests/test_cases/pipeline_config_schema.json"
     pipeline_config = ["Invalid", "data", "format"]
-    with pytest.raises(ValueError):
-        validate_json_schema(pipeline_config_schema, pipeline_config)
+    with pytest.raises(ValueError) as err:
+        validate_json_schema(
+            schema_path=pipeline_config_schema, data_dict=pipeline_config
+        )
+    assert "Invalid data format" in str(err.value)
 
 
 def test_validate_json_schema_url():
@@ -59,8 +112,33 @@ def test_validate_json_schema_url():
     """
     pipeline_config_schema = "http://example.org"
     pipeline_config = "tests/test_cases/pipeline_config.json"
-    with pytest.raises(NotImplementedError):
-        validate_json_schema(pipeline_config_schema, pipeline_config)
+    with pytest.raises(NotImplementedError) as err:
+        validate_json_schema(
+            schema_path=pipeline_config_schema, data_path=pipeline_config
+        )
+    assert "Validation from remote schema not yet supported" in str(err.value)
+
+
+def test_validate_json_schema_invalid_schema_path():
+    pipeline_config_schema = "tests/test_cases/does_not_exist.json"
+    pipeline_config = "tests/test_cases/pipeline_config.json"
+    schema_path = Path(pipeline_config_schema).absolute()
+    with pytest.raises(ValueError) as err:
+        validate_json_schema(
+            schema_path=pipeline_config_schema, data_path=pipeline_config
+        )
+    assert f"Schema path '{schema_path}' does not exist" in str(err.value)
+
+
+def test_validate_json_schema_invalid_data_path():
+    pipeline_config_schema = "tests/test_cases/pipeline_config_schema.json"
+    pipeline_config = "tests/test_cases/does_not_exist.json"
+    data_path = Path(pipeline_config).absolute()
+    with pytest.raises(ValueError) as err:
+        validate_json_schema(
+            schema_path=pipeline_config_schema, data_path=pipeline_config
+        )
+    assert f"Data path '{data_path}' does not exist" in str(err.value)
 
 
 def test_validate_json_schema_data_path_required_field_missing():
@@ -69,12 +147,13 @@ def test_validate_json_schema_data_path_required_field_missing():
     """
     pipeline_config_schema = "tests/test_cases/pipeline_config_schema.json"
     pipeline_config = "tests/test_cases/pipeline_config_missing_required_field.json"
-    err = validate_json_schema(
-        pipeline_config_schema,
-        pipeline_config,
-        "Validating pipeline_config_missing_required_field.json",
-    )
-    assert err.message == "'priority' is a required property"
+    with pytest.raises(ValidationError) as err:
+        validate_json_schema(
+            schema_path=pipeline_config_schema,
+            data_path=pipeline_config,
+            error_msg="Error validating pipeline_config_missing_required_field.json",
+        )
+    assert "'priority' is a required property" in str(err.value)
 
 
 def test_validate_json_schema_data_path_invalid_data_type():
@@ -83,12 +162,13 @@ def test_validate_json_schema_data_path_invalid_data_type():
     """
     pipeline_config_schema = "tests/test_cases/pipeline_config_schema.json"
     pipeline_config = "tests/test_cases/pipeline_config_invalid_data_type.json"
-    err = validate_json_schema(
-        pipeline_config_schema,
-        pipeline_config,
-        "Validating pipeline_config_invalid_data_type.json",
-    )
-    assert err.message == "'1' is not of type 'integer'"
+    with pytest.raises(ValidationError) as err:
+        validate_json_schema(
+            schema_path=pipeline_config_schema,
+            data_path=pipeline_config,
+            error_msg="Error validating pipeline_config_invalid_data_type.json",
+        )
+    assert "'1' is not of type 'integer'" in str(err.value)
 
 
 def test_validate_json_schema_data_dict_required_field_missing():
@@ -103,12 +183,13 @@ def test_validate_json_schema_data_dict_required_field_missing():
         "contact": ["[email protected]"],
         "pipeline": "default",
     }
-    err = validate_json_schema(
-        pipeline_config_schema,
-        pipeline_config,
-        "Validating pipeline_config with required field missing",
-    )
-    assert err.message == "'priority' is a required property"
+    with pytest.raises(ValidationError) as err:
+        validate_json_schema(
+            schema_path=pipeline_config_schema,
+            data_dict=pipeline_config,
+            error_msg="Error validating pipeline_config with required field missing",
+        )
+    assert "'priority' is a required property" in str(err.value)
 
 
 def test_validate_json_schema_data_dict_invalid_data_type():
@@ -124,9 +205,10 @@ def test_validate_json_schema_data_dict_invalid_data_type():
         "contact": ["[email protected]"],
         "pipeline": "default",
     }
-    err = validate_json_schema(
-        pipeline_config_schema,
-        pipeline_config,
-        f"Validating pipeline_config dict with invalid data type",
-    )
-    assert err.message == "'1' is not of type 'integer'"
+    with pytest.raises(ValidationError) as err:
+        validate_json_schema(
+            schema_path=pipeline_config_schema,
+            data_dict=pipeline_config,
+            error_msg="Error validating pipeline_config dict with invalid data type",
+        )
+    assert "'1' is not of type 'integer'" in str(err.value)