Merge pull request #362 from bci-oss/361-fix-sh-node-validation

Fix evaluation of sh:node in sh:PropertyShapes

Author: atextor

core/esmf-aspect-model-validator/pom.xml

@@ -66,6 +66,10 @@
          <artifactId>lombok</artifactId>
          <scope>provided</scope>
       </dependency>
+      <dependency>
+         <groupId>org.slf4j</groupId>
+         <artifactId>slf4j-api</artifactId>
+      </dependency>
 
       <!-- Test dependencies -->
       <dependency>

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

@@ -43,12 +43,14 @@
 
 /**
  * Rust-like message formatter. Formatted messages look something like the following example:
- *
+ * <br/><br/>
+ * <pre>
  * ---> Error at line 11 column 20
  *    |
  * 11 |   :testProperty [ a :SomethingElse ]
  *    |                     ^^^^^^^^^^^^^^ Property ':testProperty' on ':Foo' has type ':SomethingElse', but only ':TestClass2' is allowed.
  *    |
+ * </pre>
  */
 public class RustLikeFormatter {
 
@@ -228,9 +230,15 @@ private boolean formatNode( final RDFNode node ) {
       final SmartToken nodePosition = extractToken( node );
       if ( null == nodePosition ) {
          // ugly special case: Jena internally replaces the RDF keyword 'a' with 'rdf:type' without position information
-         if ( node.asNode().equals( NodeConst.nodeRDFType ) ) {
+         if ( NodeConst.nodeRDFType.equals( node.asNode() ) ) {
             spacedIfPossible( "a " );
          }
+         if ( NodeConst.nodeTrue.equals( node.asNode() ) ) {
+            spacedIfPossible( "true" );
+         }
+         if ( NodeConst.nodeFalse.equals( node.asNode() ) ) {
+            spacedIfPossible( "false" );
+         }
          return true;
       }
 
@@ -254,15 +262,16 @@ private boolean isList( final Model model, final RDFNode node ) {
       return node.equals( RDF.nil ) || (node.isResource() && model.contains( node.asResource(), RDF.rest, (RDFNode) null ));
    }
 
-   private Statement findListHead( Statement listElement ) {
+   private Statement findListHead( final Statement listElement ) {
+      Statement listElementStatement = listElement;
       while ( true ) {
-         final StmtIterator iter = listElement.getModel().listStatements( null, RDF.rest, listElement.getSubject() );
+         final StmtIterator iter = listElementStatement.getModel().listStatements( null, RDF.rest, listElementStatement.getSubject() );
          if ( !iter.hasNext() ) {
             break;
          }
-         listElement = iter.nextStatement();
+         listElementStatement = iter.nextStatement();
       }
-      return listElement;
+      return listElementStatement;
    }
 
    private boolean formatList( final RDFNode listNode ) {
@@ -341,17 +350,17 @@ private void formatText( final String reconstructedText ) {
       currentColumn += reconstructedText.length();
    }
 
-   private static SmartToken extractToken( final RDFNode node ) {
-      final Node n = node.asNode();
-      if ( n instanceof AnyNode an ) {
+   private static SmartToken extractToken( final RDFNode rdfNode ) {
+      final Node node = rdfNode.asNode();
+      if ( node instanceof final AnyNode an ) {
          return an.getToken();
-      } else if ( n instanceof BlankNode bn ) {
+      } else if ( node instanceof final BlankNode bn ) {
          return bn.getToken();
-      } else if ( n instanceof LiteralNode ln ) {
+      } else if ( node instanceof final LiteralNode ln ) {
          return ln.getToken();
-      } else if ( n instanceof UriNode un ) {
+      } else if ( node instanceof final UriNode un ) {
          return un.getToken();
-      } else if ( n instanceof VariableNode vn ) {
+      } else if ( node instanceof final VariableNode vn ) {
          return vn.getToken();
       } else {
          return null;

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

@@ -22,7 +22,7 @@
  * Vocabulary for the Shapes Constraint Language (SHACL)
  */
 public class SHACL implements Namespace {
-   public final String NS = "http://www.w3.org/ns/shacl#";
+   public static final String NS = "http://www.w3.org/ns/shacl#";
 
    @Override
    public String getUri() {

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

@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) 2023 Robert Bosch Manufacturing Solutions GmbH
+ *
+ * See the AUTHORS file(s) distributed with this work for additional
+ * information regarding authorship.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * SPDX-License-Identifier: MPL-2.0
+ */
+
+package org.eclipse.esmf.aspectmodel.shacl;
+
+import java.io.Serial;
+
+/**
+ * This exception is thrown when the validation process fails
+ */
+public class ShaclValidationException extends RuntimeException {
+   @Serial
+   private static final long serialVersionUID = 7771592676699681415L;
+
+   public ShaclValidationException( final String message ) {
+      super( message );
+   }
+}

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

@@ -34,11 +34,6 @@
 import org.apache.jena.rdf.model.Statement;
 import org.apache.jena.vocabulary.RDF;
 import org.eclipse.esmf.aspectmodel.resolver.services.VersionedModel;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import com.google.common.collect.Streams;
-
 import org.eclipse.esmf.aspectmodel.shacl.constraint.Constraint;
 import org.eclipse.esmf.aspectmodel.shacl.constraint.MinCountConstraint;
 import org.eclipse.esmf.aspectmodel.shacl.constraint.SparqlConstraint;
@@ -47,15 +42,17 @@
 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.
  */
 public class ShaclValidator {
-   private static final Logger LOG = LoggerFactory.getLogger( ShaclValidator.class );
    private final List<Shape.Node> shapes;
    private final Map<Resource, List<Shape.Node>> shapesWithClassTargets;
    private final Model shapesModel;
+   private final PathNodeRetriever retriever = new PathNodeRetriever();
 
    /**
     * Constructor to provide a custom RDF model containing SHACL shapes
@@ -77,7 +74,7 @@ public ShaclValidator( final Model shapesModel ) {
 
    /**
     * Validates a model element using the SHACL shapes the validator was initialized with.
-    * If you have more than one element to validate, prefer the method {@link this.validateElements( final List<Resource>  )} to calling this method in a loop
+    * 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)}.
@@ -128,7 +125,8 @@ public List<Violation> validateModel( final VersionedModel model ) {
    private Map<Resource, List<Shape.Node>> findSparqlTargets( final Model model ) {
       final Map<Resource, List<Shape.Node>> resourceShapes = new HashMap<>();
       for ( final Shape.Node shape : targetSparqlShapes() ) {
-         final List<Resource> shapeTargets = querySparqlTargets( model, shape.attributes().targetSparql().get() );
+         final List<Resource> shapeTargets = querySparqlTargets( model, shape.attributes().targetSparql().orElseThrow( () ->
+               new ShaclValidationException( "SPARQL node shape is missing a target SPARQL expression" ) ) );
          for ( final Resource node : shapeTargets ) {
             addResourceShape( resourceShapes, node, shape );
          }
@@ -160,13 +158,12 @@ 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.size() > 0 ? 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 ) {
       final List<Violation> violations = new ArrayList<>();
-      final PathNodeRetriever retriever = new PathNodeRetriever();
       for ( final Shape.Property property : shape.properties() ) {
          for ( final Constraint constraint : property.attributes().constraints() ) {
             final List<Statement> reachableNodes = property.path().accept( element, retriever );
@@ -186,7 +183,7 @@ public List<Violation> validateShapeForElement( final Resource element, final Sh
 
             // 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 PredicatePath predicatePath ) {
+            if ( reachableNodes.isEmpty() && constraint instanceof MinCountConstraint && property.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 );
                violations.addAll( constraint.apply( null, context ) );
@@ -257,4 +254,8 @@ 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/Shape.java

@@ -79,7 +79,7 @@ record Node(
    }
 
    /**
-    * Implements <code>sh:property</code>
+    * Implements {@code sh:property}
     */
    record Property(
          Attributes attributes,