Repository: RDFLib/pySHACL

Issue:
sh:or with or without --shape
First off, thanks for creating this awesome SHACL validator! 

I’m trying to define a predicate whose value can be either xsd:string or xsd:int. While I can validate the data as expected without the --shape flag, I encounter an error when using it alongside `sh:or`.

Below are the details.

SHACL file: test.shacl
-----------------------
```
@prefix ex: <http://example.org/> .
@prefix sh: <http://www.w3.org/ns/shacl#> .
@prefix xsd: <http://www.w3.org/2001/XMLSchema#> .

ex:DescriptorShape
  a sh:NodeShape ;

  sh:targetClass ex:Descriptor ;

  sh:property [
    sh:path ex:hasValue ;
    sh:or (
        [ sh:datatype xsd:string ]
        [ sh:datatype xsd:int ]
    ) ;
    sh:minCount 1 ;
    sh:maxCount 1 ;
  ] .
```

Data file: test.ttl
------------
```
@prefix xsd: <http://www.w3.org/2001/XMLSchema#> .
@prefix ex: <http://example.org/> .

ex:Example
    a ex:Descriptor ;
    ex:hasValue "1"^^xsd:decimal.
```

without `--shape`, validation worked as expected, and the data did not conform because the value 1 was of type xsd:decimal, which does not satisfy the sh:or constraint.
```
$ pyshacl -s test.shacl test.ttl 
Validation Report
Conforms: False
Results (1):
Constraint Violation in OrConstraintComponent (http://www.w3.org/ns/shacl#OrConstraintComponent):
        Severity: sh:Violation
        Source Shape: [ sh:maxCount Literal("1", datatype=xsd:integer) ; sh:minCount Literal("1", datatype=xsd:integer) ; sh:or ( [ sh:datatype xsd:string ] [ sh:datatype xsd:int ] ) ; sh:path ex:hasValue ]
        Focus Node: ex:Example
        Value Node: Literal("1", datatype=xsd:decimal)
        Result Path: ex:hasValue
        Message: Node Literal("1", datatype=xsd:decimal) must conform to one or more shapes in [ sh:datatype xsd:string ] , [ sh:datatype xsd:int ]
```

When specifying the shape explicitly using the `--shape` flag, I received the following runtime error:
```
$ pyshacl -s test.shacl test.ttl --shape ex:DescriptorShape
Validator encountered a Runtime Error:
Shape pointed to by sh:or does not exist or is not a well-formed SHACL Shape.
If you believe this is a bug in pyshacl, open an Issue on the pyshacl github page.
```

The error indicates that the source shape referenced in `sh:or` is not found (or harvested):
```
Source Shape [ sh:maxCount Literal("1", datatype=xsd:integer) ; sh:minCount Literal("1", datatype=xsd:integer) ; sh:or ( [ sh:datatype xsd:string ] [ sh:datatype xsd:int ] ) ; sh:path ex:hasValue ]
```

By digging into the code, I noticed differences in how shapes are processed with and without the `--shape` flag:

without `--shape`, all shapes are harvested as follows:
```
shapes = self.shacl_graph.shapes        # validator.py
self._build_node_shape_cache()        # shapes_graph.py
```

with `--shape`, only the specified shapes are loaded:
```
shapes = self.shacl_graph.shapes_from_uris(expanded_use_shapes)        # validator.py
self._build_node_shape_cache_from_list(shapes_uris)        # shapes_graph.py
```

It seems this might be a design choice, but it would make sense to:

1. Always harvest all shapes in the SHACL graph first.
2. Apply a filter for the shape specified with `--shape`.

This approach would ensure the dependencies (like shapes referenced in sh:or) are available for validation.

Discussion:
@ashleysommer: @need47 Thanks for opening this issue.

The `--shape` targeting feature is relatively new, not used by many PySHACL users in the wild, and not well covered by the test suite. So I'm not surprised to see a bug like this being reported.

Your assessment is correct, it was a design choice to load only the shapes that are specified, rather than filtering all shapes. This is for performance reasons. One of the goals of PySHACL was to be able to execute data validation as fast as possible. In most validator runs, it is gathering and loading all the shapes that takes the longest time in the whole execution. So being able to skip that and load only the shape URIs that are passed in is a big win for performance.

What you may have missed in your reading, is your use-case was accounted for. It is common and expected for `sh:property`->shapes to be BNodes and `sh:or`->shapes to be BNodes, and you cannot pass a BNode ID to the `--shape` argument. 
The "_build_node_shape_cache_from_list" function [_does_ follow all shape-expecting properties on a given shape, to load the referenced child shapes](https://github.com/RDFLib/pySHACL/blob/ed3667a15ccc3e51bf128bb378f0d7937bed2efd/pyshacl/shapes_graph.py#L362-L377) into the cache (including the `sh:or` constraint).

I think the problem here is due to the value of `sh:or` is a RDF List. There might be logic error in the child-shape selection code that doesn't follow far enough to load all the values in the list.
@ashleysommer: @need47  I have a proper fix for this in https://github.com/RDFLib/pySHACL/pull/285
Can you please pull that into your fork to see if it solves the reported problem?@need47: I can confirm that this commit solved my issue. Thanks a lot.@need47: Could you please publish a package for this? I want to install it via pip install directly. Thanks.@ashleysommer: Hi @need47 Yes I intended to put out a new release at the end of February, but I haven't yet had a chance to do it. You will see a new release today or tomorrow.@ashleysommer: v0.30.1 is out now
https://github.com/RDFLib/pySHACL/releases/tag/v0.30.1

Solution patch:
diff --git a/pyshacl/shapes_graph.py b/pyshacl/shapes_graph.py
index 1be67e3..d3ed90a 100644
--- a/pyshacl/shapes_graph.py
+++ b/pyshacl/shapes_graph.py
@@ -416,7 +416,12 @@ class ShapesGraph(object):
                     for _p in has_shape_expecting_p.keys():
                         property_entries = list(g.objects(s, _p))
                         for p_e in property_entries:
-                            if isinstance(p_e, rdflib.BNode):
+                            if _p in (SH_or, SH_xone, SH_and):
+                                # These are list-expecting variants of shape-expecting constraints.
+                                for item in g.items(p_e):
+                                    if isinstance(item, rdflib.BNode):
+                                        _found_child_bnodes.append(item)
+                            elif isinstance(p_e, rdflib.BNode):
                                 _found_child_bnodes.append(p_e)
                 if len(_found_child_bnodes) > 0:
                     _gather_shapes(_found_child_bnodes, recurse_depth=recurse_depth + 1)


Test patch:
diff --git a/test/test_manual_targeting.py b/test/test_manual_targeting.py
index 9b5ff32..31ffb32 100644
--- a/test/test_manual_targeting.py
+++ b/test/test_manual_targeting.py
@@ -84,10 +84,11 @@ exShape:HumanShape a sh:NodeShape ;
 
 exShape:AnimalShape a sh:NodeShape ;
     sh:property [
-        sh:datatype xsd:integer ;
         sh:path exOnt:nLegs ;
-        sh:maxInclusive 4 ;
-        sh:minInclusive 1 ;
+        sh:or (
+            [ sh:datatype xsd:integer ; sh:minInclusive 0 ; sh:maxInclusive 0 ]
+            [ sh:datatype xsd:integer ; sh:minInclusive 1 ; sh:maxInclusive 8 ]
+        ) ;
     ] ;
     sh:targetClass exOnt:Animal .
 """
@@ -195,6 +196,21 @@ def test_validate_fail_manual_targeting_shape():
     assert "Results (2)" in string
     assert not conforms
 
+def test_validate_fail_manual_targeting_shape_or():
+    res = validate(
+        data_file_text_bad,
+        shacl_graph=shacl_file_text,
+        data_graph_format='turtle',
+        shacl_graph_format='turtle',
+        ont_graph=ontology_file_text,
+        ont_graph_format="turtle",
+        inference='rdfs',
+        use_shapes=["exShape:AnimalShape"],
+        debug=True,
+    )
+    conforms, graph, string = res
+    assert "Results (2)" in string
+    assert not conforms
 
 def test_validate_fail_manual_targeting_focus_with_shape():
     res = validate(