Merge pull request crs4#170 from kikkomep/fix/shacl-issues

fix(shacl): correct property subgraph and zero-shape profile evaluation

Author: kikkomep

rocrate_validator/models.py

@@ -19,8 +19,14 @@
 
 from rocrate_validator.utils import log as logging
 from rocrate_validator.constants import VALIDATOR_NS
-from rocrate_validator.models import (Profile, Requirement, RequirementCheck,
-                                      RequirementLevel, RequirementLoader)
+from rocrate_validator.models import (
+    Profile,
+    Requirement,
+    RequirementCheck,
+    RequirementLevel,
+    RequirementLoader,
+    ValidationContext,
+)
 from rocrate_validator.requirements.shacl.checks import SHACLCheck
 from rocrate_validator.requirements.shacl.models import Shape, ShapesRegistry
 
@@ -29,16 +35,11 @@
 
 
 class SHACLRequirement(Requirement):
-
-    def __init__(self,
-                 shape: Shape,
-                 profile: Profile,
-                 path: Path):
+    def __init__(self, shape: Shape, profile: Profile, path: Path):
         self._shape = shape
-        super().__init__(profile,
-                         shape.name if shape.name else "",
-                         shape.description if shape.description else "",
-                         path)
+        super().__init__(
+            profile, shape.name if shape.name else "", shape.description if shape.description else "", path
+        )
         # init checks
         self._checks = self.__init_checks__()
         # assign check IDs
@@ -59,8 +60,15 @@ def __init_checks__(self) -> list[RequirementCheck]:
         # check if the shape has nested properties
         has_properties = hasattr(self.shape, "properties") and len(self.shape.properties) > 0
         # create a check for the shape itself, hidden if the shape has nested properties
-        checks.append(SHACLCheck(self, self.shape, name=f"Check {self.shape.name}" if has_properties else None,
-                                 hidden=has_properties, root=True))
+        checks.append(
+            SHACLCheck(
+                self,
+                self.shape,
+                name=f"Check {self.shape.name}" if has_properties else None,
+                hidden=has_properties,
+                root=True,
+            )
+        )
         # create a check for each property if the shape has nested properties
         if has_properties:
             for prop in self.shape.properties:
@@ -82,9 +90,67 @@ def hidden(self) -> bool:
                 return True
         return False
 
+    @classmethod
+    def finalize(cls, context: ValidationContext) -> None:
+        """ "
+        Finalize the SHACL requirement by ensuring that a SHACL validation run is triggered for the target profile
+        if it has no shapes of its own (e.g. an extension profile that purely inherits or only deactivates).
+
+        SHACL is normally driven by the first execute_check of a check
+        belonging to the target profile (see SHACLValidationContextManager).
+        If the target has zero SHACL checks of its own (e.g. an extension
+        profile that purely inherits or only deactivates), no pyshacl run
+        is ever triggered and inherited shapes are never evaluated.
+        Force one final run on the merged shapes graph in that case.
+        """
+
+        logger.debug("Starting %s requirement finalization for context %s", cls.__name__, context)
+
+        # extract profiles and target profile from context
+        profiles = context.profiles
+
+        from rocrate_validator.requirements.shacl.checks import SHACLCheck
+        from rocrate_validator.requirements.shacl.validator import SHACLValidationContext
+
+        target = next((p for p in profiles if p.identifier == context.settings.profile_identifier), None)
+        if target is None:
+            return
+
+        shacl_context = SHACLValidationContext.get_instance(context)
+        # If pyshacl already ran for the target during the main loop there is
+        # nothing to do.
+        if shacl_context.get_validation_result(target) is not None:
+            return
+
+        # Pick any SHACLCheck across the loaded profiles to drive the run; the
+        # check identity is only used for logging inside __do_execute_check__,
+        # the actual validation is graph-wide.
+        runner = next(
+            (c for p in profiles for r in p.requirements for c in r.get_checks() if isinstance(c, SHACLCheck)),
+            None,
+        )
+        if runner is None:
+            return
+
+        # Make sure the target's shapes (if any) are in the merged registry
+        # and switch the current profile so violations are attributed under
+        # the target profile in the report.
+        shacl_context.__set_current_validation_profile__(target)
+        shacl_context._current_validation_profile = target
+        try:
+            runner.__do_execute_check__(shacl_context)
+        except Exception as e:
+            logger.warning("Forced SHACL run for zero-shape target profile %s failed: %s", target.identifier, e)
+            if logger.isEnabledFor(logging.DEBUG):
+                logger.exception(e)
+        finally:
+            shacl_context.__unset_current_validation_profile__()
+
+        # do finalization logic here (empty for now)
+        logger.debug("Completed %s requirement finalization for context %s", cls.__name__, context)
 
-class SHACLRequirementLoader(RequirementLoader):
 
+class SHACLRequirementLoader(RequirementLoader):
     def __init__(self, profile: Profile):
         super().__init__(profile)
         self._shape_registry = ShapesRegistry.get_instance(profile)
@@ -95,9 +161,9 @@ def __init__(self, profile: Profile):
     def shapes_registry(self) -> ShapesRegistry:
         return self._shape_registry
 
-    def load(self, profile: Profile,
-             requirement_level: RequirementLevel,
-             file_path: Path, publicID: Optional[str] = None) -> list[Requirement]:
+    def load(
+        self, profile: Profile, requirement_level: RequirementLevel, file_path: Path, publicID: Optional[str] = None
+    ) -> list[Requirement]:
         assert file_path is not None, "The file path cannot be None"
         shapes: list[Shape] = self.shapes_registry.load_shapes(file_path, publicID)
         logger.debug("Loaded %s shapes: %s", len(shapes), shapes)

rocrate_validator/requirements/shacl/requirements.py

@@ -190,20 +190,23 @@ def get_shape_graph(self, shape_node: Node) -> Graph:
 
     def get_shape_property_graph(self, shape_node: Node, shape_property: Node) -> Graph:
         """
-        Get the subgraph of the given shape node excluding the given property
+        Get the subgraph of a property shape nested inside a node shape.
+
+        Includes only triples reachable from `shape_property` (its constraints
+        and any RDF lists used by `sh:and`/`sh:or`/`sh:xone`), plus the link
+        triple `(shape_node, sh:property, shape_property)`. Nothing reachable
+        only via sibling properties is included, so subtracting this graph
+        from the merged shapes graph cannot break sibling constructs.
         """
         node_graph = self.get_shape_graph(shape_node)
         assert node_graph is not None, "The shape graph cannot be None"
 
         property_graph = Graph()
-        shacl_ns = Namespace(SHACL_NS)
-        nested_properties_to_exclude = [o for (_, _, o) in node_graph.triples(
-            (shape_node, shacl_ns.property, None)) if o != shape_property]
-        triples_to_exclude = [(s, _, o) for (s, _, o) in node_graph.triples((None, None, None))
-                              if s in nested_properties_to_exclude
-                              or o in nested_properties_to_exclude]
+        for s, p, o in __extract_related_triples__(node_graph, shape_property):
+            property_graph.add((s, p, o))
 
-        property_graph += node_graph - triples_to_exclude
+        shacl_ns = Namespace(SHACL_NS)
+        property_graph.add((shape_node, shacl_ns.property, shape_property))
 
         return property_graph
 

rocrate_validator/requirements/shacl/utils.py

@@ -0,0 +1,30 @@
+# Copyright (c) 2024-2026 CRS4
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+@prefix dct: <http://purl.org/dc/terms/> .
+@prefix prof: <http://www.w3.org/ns/dx/prof/> .
+@prefix rdfs: <http://www.w3.org/2000/01/rdf-schema#> .
+
+# A "wrapper" extension profile that inherits from c without declaring any
+# SHACL shape of its own. Used to exercise the zero-shape target profile
+# path: pyshacl must still run for inherited shapes to be evaluated.
+<https://w3id.org/c-wrapper>
+    a prof:Profile ;
+    rdfs:label "Profile C-wrapper" ;
+    rdfs:comment """Pure inheritance from Profile C, no own checks."""@en ;
+    dct:publisher <https://publisherCwrapper> ;
+    prof:isProfileOf <https://w3id.org/c> ;
+    prof:isTransitiveProfileOf <https://w3id.org/a>, <https://w3id.org/c> ;
+    prof:hasToken "c-wrapper" ;
+.

tests/data/profiles/fake/c-wrapper/profile.ttl

@@ -18,11 +18,9 @@
 import pytest
 
 from rocrate_validator.constants import DEFAULT_PROFILE_IDENTIFIER
-from rocrate_validator.errors import (DuplicateRequirementCheck,
-                                      InvalidProfilePath,
-                                      ProfileSpecificationError)
-from rocrate_validator.models import (Profile, ValidationContext,
-                                      ValidationSettings, Validator)
+from rocrate_validator.errors import DuplicateRequirementCheck, InvalidProfilePath, ProfileSpecificationError
+from rocrate_validator.models import Profile, ValidationContext, ValidationSettings, Validator
+from rocrate_validator.requirements.shacl.checks import SHACLCheck
 from tests.ro_crates import InvalidFileDescriptorEntity, ValidROC
 
 # set up logging
@@ -57,7 +55,7 @@ def test_load_invalid_profile_from_validation_context(fake_profiles_path: str):
         "profiles_path": "/tmp/random_path_xxx",
         "profile_identifier": DEFAULT_PROFILE_IDENTIFIER,
         "rocrate_uri": ValidROC().wrroc_paper,
-        "enable_profile_inheritance": False
+        "enable_profile_inheritance": False,
     }
 
     settings = ValidationSettings(**settings)
@@ -79,7 +77,7 @@ def test_load_valid_profile_without_inheritance_from_validation_context(fake_pro
         "profiles_path": fake_profiles_path,
         "profile_identifier": "c",
         "rocrate_uri": ValidROC().wrroc_paper,
-        "enable_profile_inheritance": False
+        "enable_profile_inheritance": False,
     }
 
     settings = ValidationSettings(**settings)
@@ -128,7 +126,8 @@ def test_profile_spec_properties(fake_profiles_path: str):
     assert profile.version == "1.0.0", "The profile version should be 1.0.0"
     assert profile.is_profile_of == ["https://w3id.org/a"], "The profileOf property should be ['a']"
     assert profile.is_transitive_profile_of == [
-        "https://w3id.org/a"], "The transitiveProfileOf property should be ['a']"
+        "https://w3id.org/a"
+    ], "The transitiveProfileOf property should be ['a']"
 
 
 def test_profiles_loading_free_folder_structure(profiles_with_free_folder_structure_path: str):
@@ -206,8 +205,9 @@ def __perform_test__(profile_identifier: str, expected_inherited_profiles: list[
 
         # The number of profiles should be 1
         profiles_names = [_.token for _ in profile.inherited_profiles]
-        assert profiles_names == expected_inherited_profiles, \
-            f"The number of profiles should be {expected_inherited_profiles}"
+        assert (
+            profiles_names == expected_inherited_profiles
+        ), f"The number of profiles should be {expected_inherited_profiles}"
 
     # Test the inheritance mode with 1 profile
     __perform_test__("a", [])
@@ -252,7 +252,7 @@ def test_load_invalid_profile_with_override_on_same_profile(fake_profiles_path:
         "profile_identifier": "invalid-duplicated-shapes",
         "rocrate_uri": ValidROC().wrroc_paper,
         "enable_profile_inheritance": True,
-        "allow_requirement_check_override": False
+        "allow_requirement_check_override": False,
     }
 
     settings = ValidationSettings(**settings)
@@ -275,7 +275,7 @@ def test_load_valid_profile_with_override_on_inherited_profile(fake_profiles_pat
         "profile_identifier": "c-overridden",
         "rocrate_uri": ValidROC().wrroc_paper,
         "enable_profile_inheritance": True,
-        "allow_requirement_check_override": True
+        "allow_requirement_check_override": True,
     }
 
     settings = ValidationSettings(**settings)
@@ -297,6 +297,34 @@ def test_load_valid_profile_with_override_on_inherited_profile(fake_profiles_pat
     assert len(requirements_checks) == 3, "The number of requirements should be 2"
 
 
+def test_zero_shape_target_profile_triggers_pyshacl_run(fake_profiles_path: str):
+    """Regression test for the 0-shape profile bug:
+    when the target profile has no SHACL checks of its own,
+    Validator must still drive a single pyshacl run
+    on the merged shapes graph so inherited shapes get evaluated.
+    Without the fix in `Validator.__ensure_target_shacl_run__`,
+    no SHACLCheck would be recorded as executed for the wrapper target."""
+
+    settings = ValidationSettings(
+        **{
+            "profiles_path": fake_profiles_path,
+            "profile_identifier": "c-wrapper",
+            "rocrate_uri": ValidROC().wrroc_paper,
+            "enable_profile_inheritance": True,
+            "allow_requirement_check_override": True,
+            "disable_check_for_duplicates": True,
+        }
+    )
+    result = Validator(settings).validate()
+
+    executed_shacl = [c for c in result.executed_checks if isinstance(c, SHACLCheck)]
+    assert executed_shacl, (
+        "Expected at least one inherited SHACLCheck to be executed for the "
+        "c-wrapper target. None recorded — the zero-shape pyshacl run was "
+        "skipped."
+    )
+
+
 def test_profile_parents(check_overriding_profiles_path: str):
     """Test the order of the loaded profiles."""
     logger.debug("The profiles path: %r", check_overriding_profiles_path)
@@ -366,29 +394,31 @@ def test_profile_check_overriding(check_overriding_profiles_path: str):
 
     def check_profile(profile, check, inherited_profiles, overridden_by, override):
         # Check inherited profiles
-        assert len(profile.inherited_profiles) == len(inherited_profiles), \
-            f"The number of inherited profiles should be {len(inherited_profiles)}"
+        assert len(profile.inherited_profiles) == len(
+            inherited_profiles
+        ), f"The number of inherited profiles should be {len(inherited_profiles)}"
         inherited_profiles_tokens = [_.token for _ in profile.inherited_profiles]
-        assert set(inherited_profiles_tokens) == set(inherited_profiles), \
-            f"The inherited profiles should be {inherited_profiles}"
+        assert set(inherited_profiles_tokens) == set(
+            inherited_profiles
+        ), f"The inherited profiles should be {inherited_profiles}"
 
         # Check overridden status
-        logger.debug("%r overridden by: %r", check.identifier, [
-                     _.requirement.profile.identifier for _ in check.overridden_by])
-        assert check.overridden == (len(overridden_by) > 0), \
-            f"The check overridden status should be {len(overridden_by) > 0}"
-        assert len(check.overridden_by) == len(overridden_by), \
-            f"The number of overridden checks should be {len(overridden_by)}"
+        logger.debug(
+            "%r overridden by: %r", check.identifier, [_.requirement.profile.identifier for _ in check.overridden_by]
+        )
+        assert check.overridden == (
+            len(overridden_by) > 0
+        ), f"The check overridden status should be {len(overridden_by) > 0}"
+        assert len(check.overridden_by) == len(
+            overridden_by
+        ), f"The number of overridden checks should be {len(overridden_by)}"
         overridden_by_tokens = [_.requirement.profile.identifier for _ in check.overridden_by]
-        assert set(overridden_by_tokens) == set(overridden_by), \
-            f"The overridden checks should be {overridden_by}"
+        assert set(overridden_by_tokens) == set(overridden_by), f"The overridden checks should be {overridden_by}"
 
         # Check override status
-        assert len(check.overrides) == len(override), \
-            f"The number of overridden checks should be {len(override)}"
+        assert len(check.overrides) == len(override), f"The number of overridden checks should be {len(override)}"
         override_tokens = [_.requirement.profile.identifier for _ in check.overrides]
-        assert set(override_tokens) == set(override), \
-            f"The overridden checks should be {override}"
+        assert set(override_tokens) == set(override), f"The overridden checks should be {override}"
 
     # Check the number of requirements and checks of each profile
     for profile in profiles: