Merge pull request #367 from bci-oss/365-fix-recursive-sh-node-shapes

Fix crash of ShaclValidor for recursive sh:node

Author: atextor

core/esmf-aspect-model-validator/src/main/java/org/eclipse/esmf/aspectmodel/shacl/ShaclValidator.java

@@ -22,6 +22,16 @@
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
+import org.eclipse.esmf.aspectmodel.resolver.services.VersionedModel;
+import org.eclipse.esmf.aspectmodel.shacl.constraint.Constraint;
+import org.eclipse.esmf.aspectmodel.shacl.constraint.MinCountConstraint;
+import org.eclipse.esmf.aspectmodel.shacl.constraint.SparqlConstraint;
+import org.eclipse.esmf.aspectmodel.shacl.path.PathNodeRetriever;
+import org.eclipse.esmf.aspectmodel.shacl.path.PredicatePath;
+import org.eclipse.esmf.aspectmodel.shacl.violation.EvaluationContext;
+import org.eclipse.esmf.aspectmodel.shacl.violation.Violation;
+
+import com.google.common.collect.Streams;
 import org.apache.jena.query.Query;
 import org.apache.jena.query.QueryExecution;
 import org.apache.jena.query.QueryExecutionFactory;
@@ -33,20 +43,10 @@
 import org.apache.jena.rdf.model.Resource;
 import org.apache.jena.rdf.model.Statement;
 import org.apache.jena.vocabulary.RDF;
-import org.eclipse.esmf.aspectmodel.resolver.services.VersionedModel;
-import org.eclipse.esmf.aspectmodel.shacl.constraint.Constraint;
-import org.eclipse.esmf.aspectmodel.shacl.constraint.MinCountConstraint;
-import org.eclipse.esmf.aspectmodel.shacl.constraint.SparqlConstraint;
-import org.eclipse.esmf.aspectmodel.shacl.path.PathNodeRetriever;
-import org.eclipse.esmf.aspectmodel.shacl.path.PredicatePath;
-import org.eclipse.esmf.aspectmodel.shacl.violation.EvaluationContext;
-import org.eclipse.esmf.aspectmodel.shacl.violation.Violation;
-
-import com.google.common.collect.Streams;
 
 /**
- * Implementation of a SHACL engine that allows validation on a per-element basis: {@link #validateElement(Resource)} can be used to retrieve validation
- * results only for this specific resource.
+ * Implementation of a SHACL engine that allows validation on a per-element basis: {@link #validateElement(Resource)} can be used to
+ * retrieve validation results only for this specific resource.
  */
 public class ShaclValidator {
    private final List<Shape.Node> shapes;
@@ -56,6 +56,7 @@ public class ShaclValidator {
 
    /**
     * Constructor to provide a custom RDF model containing SHACL shapes
+    *
     * @param shapesModel the shapes model
     */
    public ShaclValidator( final Model shapesModel ) {
@@ -77,7 +78,9 @@ public ShaclValidator( final Model shapesModel ) {
     * If you have more than one element to validate, prefer the method {@link #validateElements(List)} to calling this method in a loop
     * for better performance.
     * {@link Resource#getModel()} on the element must not return null, i.e., the resource may not be created using
-    * {@link org.apache.jena.rdf.model.ResourceFactory#createProperty(String)}, but instead must be created via {@link Model#createResource(String)}.
+    * {@link org.apache.jena.rdf.model.ResourceFactory#createProperty(String)}, but instead must be created via
+    * {@link Model#createResource(String)}.
+    *
     * @param element the element to be validated
     * @return the list of {@link Violation}s if there are violations
     */
@@ -86,7 +89,8 @@ public List<Violation> validateElement( final Resource element ) {
       return validateElement( element, sparqlTargets, element.getModel() );
    }
 
-   private List<Violation> validateElement( final Resource element, final Map<Resource, List<Shape.Node>> sparqlTargets, final Model resolvedModel ) {
+   private List<Violation> validateElement( final Resource element, final Map<Resource, List<Shape.Node>> sparqlTargets,
+         final Model resolvedModel ) {
       final List<Violation> violations = new ArrayList<>();
       for ( final Shape.Node shape : targetClassShapesThatApplyToElement( element, resolvedModel ) ) {
          violations.addAll( validateShapeForElement( element, shape, resolvedModel ) );
@@ -110,9 +114,11 @@ private List<Violation> validateElement( final Resource element, final Map<Resou
 
    /**
     * Validates a model using the SHACL shapes the validator was initialized with.
+    *
     * @param model the model to be validated
     * @return the list of {@link Violation}s if there are violations
     */
+   @SuppressWarnings( "UnstableApiUsage" ) // Usage of Streams.stream is deemed ok
    public List<Violation> validateModel( final VersionedModel model ) {
       final Map<Resource, List<Shape.Node>> sparqlTargetsWithShapes = findSparqlTargets( model.getRawModel() );
       return Streams.stream( model.getRawModel().listStatements( null, RDF.type, (RDFNode) null ) )
@@ -158,41 +164,46 @@ private List<Resource> querySparqlTargets( final Model model, final Query query
    }
 
    public List<Violation> validateElements( final List<Resource> elements ) {
-      final Map<Resource, List<Shape.Node>> sparqlTargets = !elements.isEmpty() ? findSparqlTargets( elements.get( 0 ).getModel() ) : Map.of();
+      final Map<Resource, List<Shape.Node>> sparqlTargets = !elements.isEmpty() ?
+            findSparqlTargets( elements.get( 0 ).getModel() ) : Map.of();
       return elements.stream().flatMap( element -> validateElement( element, sparqlTargets, element.getModel() ).stream() ).toList();
    }
 
-   public List<Violation> validateShapeForElement( final Resource element, final Shape.Node shape, final Model resolvedModel ) {
+   public List<Violation> validateShapeForElement( final Resource element, final Shape.Node nodeShape, final Model resolvedModel ) {
       final List<Violation> violations = new ArrayList<>();
-      for ( final Shape.Property property : shape.properties() ) {
-         for ( final Constraint constraint : property.attributes().constraints() ) {
-            final List<Statement> reachableNodes = property.path().accept( element, retriever );
+      for ( final Shape.Property propertyShape : nodeShape.properties() ) {
+         for ( final Constraint constraint : propertyShape.attributes().constraints() ) {
+            final List<Statement> reachableNodes = propertyShape.path().accept( element, retriever );
             // For all values that are present on the target node, check the applicable shapes and collect violations
             for ( final Statement assertion : reachableNodes ) {
-               final EvaluationContext context = new EvaluationContext( element, shape, Optional.of( assertion.getPredicate() ), reachableNodes, this,
-                     resolvedModel );
+               final EvaluationContext context = new EvaluationContext( element, nodeShape, Optional.of( propertyShape ),
+                     Optional.of( assertion.getPredicate() ), List.of( assertion ), this, resolvedModel );
                violations.addAll( constraint.apply( assertion.getObject(), context ) );
             }
 
             // important detail: Sparql constraints must run independent of whether there are any matches via the sh:path property or not
             // ( the check could be the verification whether the property exists )
             if ( reachableNodes.isEmpty() && constraint instanceof SparqlConstraint ) {
-               final EvaluationContext context = new EvaluationContext( element, shape, Optional.empty(), List.of(), this, resolvedModel );
+               final EvaluationContext context = new EvaluationContext( element, nodeShape, Optional.of( propertyShape ), Optional.empty(),
+                     List.of(), this, resolvedModel );
                violations.addAll( constraint.apply( null, context ) );
             }
 
-            // MinCount needs to be handled separately: If the property is not used at all on the target node, but a MinCount constraints >= 1
-            // exists, a violation must be emitted even though no value for the property exists
-            if ( reachableNodes.isEmpty() && constraint instanceof MinCountConstraint && property.path() instanceof final PredicatePath predicatePath ) {
+            // MinCount needs to be handled separately: If the property is not used at all on the target node, but a MinCount constraints
+            // >= 1 exists, a violation must be emitted even though no value for the property exists
+            if ( reachableNodes.isEmpty() && constraint instanceof MinCountConstraint
+                  && propertyShape.path() instanceof final PredicatePath predicatePath ) {
                final Property rdfProperty = resolvedModel.createProperty( predicatePath.predicate().getURI() );
-               final EvaluationContext context = new EvaluationContext( element, shape, Optional.of( rdfProperty ), List.of(), this, resolvedModel );
+               final EvaluationContext context = new EvaluationContext( element, nodeShape, Optional.of( propertyShape ),
+                     Optional.of( rdfProperty ), List.of(), this, resolvedModel );
                violations.addAll( constraint.apply( null, context ) );
             }
          }
       }
 
-      final EvaluationContext context = new EvaluationContext( element, shape, Optional.empty(), List.of(), this, resolvedModel );
-      for ( final Constraint constraint : shape.attributes().constraints() ) {
+      final EvaluationContext context = new EvaluationContext( element, nodeShape, Optional.empty(), Optional.empty(), List.of(), this,
+            resolvedModel );
+      for ( final Constraint constraint : nodeShape.attributes().constraints() ) {
          if ( !constraint.canBeUsedOnNodeShapes() ) {
             continue;
          }
@@ -205,6 +216,7 @@ public List<Violation> validateShapeForElement( final Resource element, final Sh
    /**
     * Returns the shapes that apply to the element because the element has a type (or the type has a transitive supertype) that
     * is given as sh:targetClass
+    *
     * @param element a model element
     * @return the stream of shapes
     */
@@ -217,6 +229,7 @@ private Set<Shape.Node> targetClassShapesThatApplyToElement( final Resource elem
 
    /**
     * Returns the shapes that apply to the element because the element uses a property which is given as sh:targetSubjectsOf
+    *
     * @param element a model element
     * @return the stream of shapes
     */
@@ -254,8 +267,4 @@ public List<Shape.Node> getShapes() {
    public Model getShapesModel() {
       return shapesModel;
    }
-
-   public PathNodeRetriever getPathNodeRetriever() {
-      return retriever;
-   }
 }

core/esmf-aspect-model-validator/src/main/java/org/eclipse/esmf/aspectmodel/shacl/ShapeLoader.java

@@ -13,7 +13,9 @@
 
 package org.eclipse.esmf.aspectmodel.shacl;
 
+import java.util.Collection;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -80,8 +82,13 @@
 public class ShapeLoader implements Function<Model, List<Shape.Node>> {
    private static final SHACL SHACL = new SHACL();
 
+   private final Map<Resource, JsLibrary> jsLibraries = new HashMap<>();
+   private final Map<Resource, Shape.Node> nodeShapes = new HashMap<>();
+   private final Set<Resource> seenNodeShapes = new HashSet<>();
+
    /**
     * When constraints are instantiated for a shape, the context is passed as input
+    *
     * @param path If the parent shape of the constraint is a property shape, the path determines what the property shape refers to
     */
    private record ShapeContext(Statement statement, Optional<Path> path) {
@@ -121,17 +128,28 @@ private record ShapeContext(Statement statement, Optional<Path> path) {
          .put( SHACL.and(), context -> new AndConstraint( nestedConstraintList( context.statement(), context.path() ) ) )
          .put( SHACL.or(), context -> new OrConstraint( nestedConstraintList( context.statement(), context.path() ) ) )
          .put( SHACL.xone(), context -> new XoneConstraint( nestedConstraintList( context.statement(), context.path() ) ) )
-         .put( SHACL.node(), context -> new NodeConstraint( nodeShape( context.statement().getObject().asResource() ), context.path() ) )
+         .put( SHACL.node(), context -> {
+            // Since sh:node can recursively refer to the same NodeShape is used in when shapes define recursive structures,
+            // the NodeConstraint is built using a Supplier for the actual NodeShape. Only if the NodeShape has not yet been
+            // seen (i.e., it could be in the process of being built right now), create it now.
+            final Resource resource = context.statement().getObject().asResource();
+            if ( !seenNodeShapes.contains( resource ) ) {
+               nodeShape( resource );
+            }
+            return new NodeConstraint( () -> nodeShapes.get( resource ), context.path() );
+         } )
          .put( SHACL.in(), context -> new AllowedValuesConstraint( context.statement().getResource().as( RDFList.class ).asJavaList() ) )
          .put( SHACL.closed(), context -> {
             boolean closed = context.statement().getBoolean();
             if ( !closed ) {
                throw new RuntimeException();
             }
-            Set<Property> ignoredProperties = context.statement().getSubject().getProperty( SHACL.ignoredProperties() ).getResource()
-                  .as( RDFList.class )
-                  .asJavaList()
+            Set<Property> ignoredProperties = Optional.ofNullable( context.statement().getSubject().getProperty( SHACL.ignoredProperties() ) )
+                  .map( Statement::getResource )
+                  .map( resource -> resource.as( RDFList.class ) )
+                  .map( RDFList::asJavaList )
                   .stream()
+                  .flatMap( Collection::stream )
                   .map( RDFNode::asResource )
                   .map( Resource::getURI )
                   .map( uri -> context.statement().getModel().createProperty( uri ) )
@@ -145,29 +163,31 @@ private record ShapeContext(Statement statement, Optional<Path> path) {
             return new SparqlConstraint( message, sparqlQuery( constraintNode ) );
          } )
          .put( SHACL.js(), context -> {
-            Resource constraintNode = context.statement().getResource();
-            JsLibrary library = jsLibrary( constraintNode.getProperty( SHACL.jsLibrary() ).getResource() );
-            String functionName = constraintNode.getProperty( SHACL.jsFunctionName() ).getString();
+            final Resource constraintNode = context.statement().getResource();
+            final JsLibrary library = jsLibrary( constraintNode.getProperty( SHACL.jsLibrary() ).getResource() );
+            final String functionName = constraintNode.getProperty( SHACL.jsFunctionName() ).getString();
             final String message = Optional.ofNullable( constraintNode.getProperty( SHACL.message() ) ).map( Statement::getString ).orElse( "" );
             return new JsConstraint( message, library, functionName );
          } )
          .build();
 
-   private final Map<Resource, JsLibrary> jsLibraries = new HashMap<>();
-
    private List<Constraint> nestedConstraintList( final Statement statement, final Optional<Path> path ) {
       return statement.getObject().as( RDFList.class ).asJavaList().stream()
             .filter( RDFNode::isResource )
             .map( RDFNode::asResource )
-            .flatMap( resource -> resource.isURIResource()
-                  ? nodeShape( resource ).attributes().constraints().stream()
-                  : shapeAttributes( resource, path ).constraints().stream() )
+            .flatMap( resource -> {
+               if ( resource.isURIResource() ) {
+                  return nodeShape( resource ).attributes().constraints().stream();
+               }
+               return shapeAttributes( resource, path ).constraints().stream();
+            } )
             .toList();
    }
 
    /**
     * Builds a {@link Pattern} from a pattern string and a flags string as specified in
     * <a href="https://www.w3.org/TR/xpath-functions/#func-matches">xpath functions</a>.
+    *
     * @param patternString the pattern string
     * @param flagsString the flags string
     * @return the pattern
@@ -190,6 +210,7 @@ private Pattern buildPattern( final String patternString, final String flagsStri
    }
 
    @Override
+   @SuppressWarnings( "UnstableApiUsage" ) // Usage of Streams.stream is deemed ok
    public List<Shape.Node> apply( final Model model ) {
       return Streams.stream( model.listStatements( null, RDF.type, SHACL.NodeShape() ) )
             .map( Statement::getSubject )
@@ -225,12 +246,16 @@ private Shape.Attributes shapeAttributes( final Resource shapeNode, final Option
             defaultValue, deactivated, message, severity, constraints );
    }
 
+   @SuppressWarnings( "UnstableApiUsage" ) // Usage of Streams.stream is deemed ok
    private Shape.Node nodeShape( final Resource shapeNode ) {
+      seenNodeShapes.add( shapeNode );
       final Shape.Attributes attributes = shapeAttributes( shapeNode, Optional.empty() );
       final List<Shape.Property> properties = Streams.stream( shapeNode.listProperties( SHACL.property() ) )
             .map( Statement::getResource )
             .map( this::propertyShape ).toList();
-      return new Shape.Node( attributes, properties );
+      final Shape.Node node = new Shape.Node( attributes, properties );
+      nodeShapes.put( shapeNode, node );
+      return node;
    }
 
    private Shape.Property propertyShape( final Resource shapeNode ) {
@@ -291,6 +316,7 @@ private boolean isRdfList( final Resource resource ) {
             || resource.hasProperty( RDF.rest ) || resource.hasProperty( RDF.first );
    }
 
+   @SuppressWarnings( "UnstableApiUsage" ) // Usage of Streams.stream is deemed ok
    private List<Constraint> constraints( final Resource valueNode, final Optional<Path> path ) {
       return constraintLoaders.entrySet().stream()
             .flatMap( entry -> {

core/esmf-aspect-model-validator/src/main/java/org/eclipse/esmf/aspectmodel/shacl/constraint/AllowedLanguagesConstraint.java

@@ -16,16 +16,18 @@
 import java.util.List;
 
 import org.apache.jena.rdf.model.RDFNode;
+
 import org.eclipse.esmf.aspectmodel.shacl.Shape;
 import org.eclipse.esmf.aspectmodel.shacl.violation.EvaluationContext;
 import org.eclipse.esmf.aspectmodel.shacl.violation.LanguageFromListViolation;
 import org.eclipse.esmf.aspectmodel.shacl.violation.Violation;
 
 /**
  * Implements <a href="https://www.w3.org/TR/shacl/#LanguageInConstraintComponent">sh:languageIn</a>
+ *
  * @param allowedLanguages the list of allowed language tags
  */
-public record AllowedLanguagesConstraint(List<String> allowedLanguages) implements Constraint {
+public record AllowedLanguagesConstraint( List<String> allowedLanguages ) implements Constraint {
    @Override
    public List<Violation> apply( final RDFNode rdfNode, final EvaluationContext context ) {
       final List<Violation> nodeKindViolations = new NodeKindConstraint( Shape.NodeKind.Literal ).apply( rdfNode, context );

core/esmf-aspect-model-validator/src/main/java/org/eclipse/esmf/aspectmodel/shacl/constraint/AllowedValuesConstraint.java

@@ -16,15 +16,17 @@
 import java.util.List;
 
 import org.apache.jena.rdf.model.RDFNode;
+
 import org.eclipse.esmf.aspectmodel.shacl.violation.EvaluationContext;
 import org.eclipse.esmf.aspectmodel.shacl.violation.ValueFromListViolation;
 import org.eclipse.esmf.aspectmodel.shacl.violation.Violation;
 
 /**
  * Implements <a href="https://www.w3.org/TR/shacl/#InConstraintComponent">sh:in</a> *
+ *
  * @param allowedValues the list of allowed values
  */
-public record AllowedValuesConstraint(List<RDFNode> allowedValues) implements Constraint {
+public record AllowedValuesConstraint( List<RDFNode> allowedValues ) implements Constraint {
    @Override
    public List<Violation> apply( final RDFNode rdfNode, final EvaluationContext context ) {
       return allowedValues.contains( rdfNode ) ?

core/esmf-aspect-model-validator/src/main/java/org/eclipse/esmf/aspectmodel/shacl/constraint/AndConstraint.java

@@ -17,13 +17,14 @@
 import java.util.stream.Collectors;
 
 import org.apache.jena.rdf.model.RDFNode;
+
 import org.eclipse.esmf.aspectmodel.shacl.violation.EvaluationContext;
 import org.eclipse.esmf.aspectmodel.shacl.violation.Violation;
 
 /**
  * Implements <a href="https://www.w3.org/TR/shacl/#AndConstraintComponent">sh:and</a>
  */
-public record AndConstraint(List<Constraint> constraints) implements Constraint {
+public record AndConstraint( List<Constraint> constraints ) implements Constraint {
    @Override
    public List<Violation> apply( final RDFNode rdfNode, final EvaluationContext context ) {
       return constraints().stream().flatMap( constraint -> constraint.apply( rdfNode, context ).stream() ).collect( Collectors.toList() );