Merge pull request crs4#181 from kikkomep/feat/unconditional-validation-abort

feat(validation): ✨ abort validation when the file descriptor is not valid JSON

Author: kikkomep

rocrate_validator/models/requirement.py

@@ -15,6 +15,7 @@
 from __future__ import annotations
 
 import importlib
+import json
 from abc import ABC, abstractmethod
 from dataclasses import dataclass
 from functools import total_ordering
@@ -225,6 +226,11 @@ def _do_validate_(self, context: ValidationContext) -> bool:
                 logger.debug("Skipping check '%s' because: %s", check.name, e)
                 context.result._add_skipped_check(check)
                 continue
+            except json.JSONDecodeError as e:
+                # The file descriptor is not valid JSON, so this check could not run.
+                # This is a malformed-input problem (reported as an ad-hoc issue by the
+                # dedicated "File Descriptor JSON format" check), not a validator bug.
+                logger.debug("Skipping check %s: file descriptor is not valid JSON: %s", check, e)
             except Exception as e:
                 if context.maybe_warn_offline_cache_miss(e):
                     logger.debug("Offline cache miss during check %s: %s", check, e)
@@ -233,6 +239,9 @@ def _do_validate_(self, context: ValidationContext) -> bool:
                     logger.warning("Consider reporting this as a bug.")
                     if logger.isEnabledFor(logging.DEBUG):
                         logger.exception("Unhandled exception during check execution", exc_info=e)
+            # Stop running further checks once the metadata is known to be unusable.
+            if context.aborted:
+                break
         skipped_checks = set(self._checks) - set(checks_to_perform)
         context.result.skipped_checks.update(skipped_checks)
         logger.debug(

rocrate_validator/models/validation.py

@@ -227,6 +227,12 @@ def __do_validate__(self, requirements: list[Requirement] | None = None) -> Vali
                             logger.debug("Aborting on first requirement failure")
                             terminate = True
                             break
+                    # Unconditional abort: the metadata is unusable (e.g. not valid JSON),
+                    # so stop now instead of reporting false positives from later checks.
+                    if context.aborted:
+                        logger.debug("Aborting validation: %s", context.abort_reason)
+                        terminate = True
+                        break
                 self.notify(ProfileValidationEvent(EventType.PROFILE_VALIDATION_END, profile=profile))
                 if terminate:
                     break
@@ -287,6 +293,10 @@ def __init__(self, validator: Validator, settings: ValidationSettings):
         self._properties: dict = {}
         # URLs already reported as missing from the HTTP cache during this run
         self._offline_cache_misses_warned: set[str] = set()
+        # flag set when the validation must be aborted because the metadata
+        # cannot be read (e.g. the file descriptor is not valid JSON)
+        self._aborted: bool = False
+        self._abort_reason: str | None = None
 
         # initialize the ROCrate object
         if settings.metadata_dict:
@@ -426,6 +436,43 @@ def fail_fast(self) -> bool:
         """
         return bool(self.settings.abort_on_first)
 
+    @property
+    def aborted(self) -> bool:
+        """
+        Whether the validation has been aborted because the metadata cannot be read.
+
+        Unlike :attr:`fail_fast`, this abort is unconditional: when the file descriptor
+        cannot be parsed (e.g. it is not valid JSON) no metadata can be read, so any
+        further check would only produce false positives.
+
+        :return: ``True`` if the validation has been aborted, ``False`` otherwise
+        :rtype: bool
+        """
+        return self._aborted
+
+    @property
+    def abort_reason(self) -> str | None:
+        """
+        The reason why the validation has been aborted, if any.
+        """
+        return self._abort_reason
+
+    def abort_validation(self, reason: str | None = None) -> None:
+        """
+        Signal that the validation cannot meaningfully continue because the metadata
+        is unusable (e.g. the file descriptor is not valid JSON).
+
+        The check that detects the problem is expected to record its issue first and
+        then call this method; the validation loop stops as soon as the current
+        requirement completes, avoiding false positives from checks that depend on
+        readable metadata.
+
+        :param reason: An optional human-readable description of the abort cause
+        """
+        self._aborted = True
+        self._abort_reason = reason
+        logger.debug("Validation aborted: %s", reason)
+
     @property
     def rel_fd_path(self) -> Path:
         """

rocrate_validator/profiles/ro-crate/1.1/must/0_file_descriptor_format.py

@@ -14,6 +14,7 @@
 
 # pylint: disable=invalid-name  # profile filename uses digit prefix (load-order convention)
 
+import json
 import re
 from typing import Any
 from urllib.parse import urljoin
@@ -77,6 +78,17 @@ def check(self, context: ValidationContext) -> bool:
             logger.debug("Checking validity of JSON file at %s", context.ro_crate.metadata)
             context.ro_crate.metadata.as_dict()
             return True
+        except json.JSONDecodeError as e:
+            context.result.add_issue(
+                f'RO-Crate file descriptor "{context.rel_fd_path}" is not valid JSON: '
+                f"{e.msg} at line {e.lineno}, column {e.colno} (char {e.pos})",
+                self,
+            )
+            if logger.isEnabledFor(logging.DEBUG):
+                logger.debug("RO-Crate file descriptor is not valid JSON", exc_info=True)
+            # The metadata cannot be parsed: abort to avoid false positives downstream.
+            context.abort_validation(f"file descriptor is not valid JSON: {e}")
+            return False
         except Exception:
             context.result.add_issue(
                 f'RO-Crate file descriptor "{context.rel_fd_path}" is not in the correct format', self

rocrate_validator/profiles/ro-crate/1.2/must/0_file_descriptor_format.py

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import json
 import re
 from http import HTTPStatus
 from pathlib import Path
@@ -128,6 +129,17 @@ def check(self, context: ValidationContext) -> bool:
             logger.debug("Checking validity of JSON file at %s", context.ro_crate.metadata)
             context.ro_crate.metadata.as_dict()
             return True
+        except json.JSONDecodeError as e:
+            context.result.add_issue(
+                f'RO-Crate file descriptor "{context.rel_fd_path}" is not valid JSON: '
+                f"{e.msg} at line {e.lineno}, column {e.colno} (char {e.pos})",
+                self,
+            )
+            if logger.isEnabledFor(logging.DEBUG):
+                logger.debug("RO-Crate file descriptor is not valid JSON", exc_info=True)
+            # The metadata cannot be parsed: abort to avoid false positives downstream.
+            context.abort_validation(f"file descriptor is not valid JSON: {e}")
+            return False
         except Exception:
             context.result.add_issue(
                 f'RO-Crate file descriptor "{context.rel_fd_path}" is not in the correct format', self

rocrate_validator/requirements/shacl/requirements.py

@@ -101,6 +101,12 @@ def finalize(cls, context: ValidationContext) -> None:
 
         logger.debug("Starting %s requirement finalization for context %s", cls.__name__, context)
 
+        # If the validation was aborted (e.g. the metadata is not valid JSON), the data
+        # graph cannot be parsed: skip the forced SHACL run to avoid false positives.
+        if context.aborted:
+            logger.debug("Skipping forced SHACL run: validation aborted (%s)", context.abort_reason)
+            return
+
         # extract profiles and target profile from context
         profiles = context.profiles
 

tests/integration/profiles/ro-crate-1.2/test_metadata_document.py

@@ -49,7 +49,7 @@ def test_not_json():
         False,
         profile_identifier="ro-crate-1.2",
         expected_triggered_requirements=["File Descriptor JSON format"],
-        expected_triggered_issues=['RO-Crate file descriptor "ro-crate-metadata.json" is not in the correct format'],
+        expected_triggered_issues=['RO-Crate file descriptor "ro-crate-metadata.json" is not valid JSON'],
     )
 
 

tests/integration/profiles/ro-crate/test_file_descriptor_format.py

@@ -14,7 +14,7 @@
 
 import logging
 
-from rocrate_validator import models
+from rocrate_validator import models, services
 from tests.ro_crates import InvalidFileDescriptor, ValidROC
 from tests.shared import do_entity_test
 
@@ -32,8 +32,44 @@ def test_missing_file_descriptor():
 
 
 def test_not_valid_json_format():
-    """Test a RO-Crate with an invalid JSON file descriptor format."""
-    do_entity_test(paths.invalid_json_format, models.Severity.REQUIRED, False, ["File Descriptor JSON format"], [])
+    """
+    Test a RO-Crate with an invalid JSON file descriptor format.
+
+    The validator must emit an ad-hoc issue reporting that the file descriptor is
+    not valid JSON, including the position of the parsing error.
+    """
+    do_entity_test(
+        paths.invalid_json_format,
+        models.Severity.REQUIRED,
+        False,
+        ["File Descriptor JSON format"],
+        ['RO-Crate file descriptor "ro-crate-metadata.json" is not valid JSON'],
+    )
+
+
+def test_not_valid_json_format_aborts_validation():
+    """
+    An unparsable JSON file descriptor must abort the validation (fail-fast).
+
+    Since the metadata cannot be read, no further check can run meaningfully, so the
+    only reported failure must be the JSON-format one (no false positives).
+    """
+    result = services.validate(
+        models.ValidationSettings(
+            rocrate_uri=models.URI(paths.invalid_json_format),
+            requirement_severity=models.Severity.REQUIRED,
+        )
+    )
+    assert not result.passed(), "An invalid-JSON crate must not pass validation"
+
+    failed_requirements = [r.name for r in result.failed_requirements]
+    assert failed_requirements == ["File Descriptor JSON format"], (
+        f"Only the JSON-format requirement should fail, got: {failed_requirements}"
+    )
+
+    issues = [i.message for i in result.get_issues(models.Severity.REQUIRED) if i.message]
+    assert len(issues) == 1, f"Exactly one issue expected, got: {issues}"
+    assert "is not valid JSON" in issues[0]
 
 
 def test_not_valid_jsonld_format_missing_context():