Checks for instantiating abstract classes is causing PyShacl checks to fail

This is causing the CI in the spdx-spec to fail.

Removing the checks will allow the CI to work.

For example:

sh:not [ sh:class ns1:ElementCollection ; ...

constraints will cause any subclass of ElementCollection to fail vaildation.

Reference spec-parser issue 936 comment.

Perhaps we should temporarily remove this check for the 3.0.1 spec release then reintroduce a working check for 3.1.

@ilans

After discussing this, removing this restriction will break the code generators. @ilans - any thoughts on this?

@JPEWdev

@JPEWdev @ilans - Can you take a look? I spent some time trying to come up with some SHACL constraints that work, but failed.

I did some digging on the internet and the conversation I found basically had the opinion you should not create such a constraint (not sure I agree) - reference this W3c email thread.

I think the reason the pyshacl is failing with the SPDX spec example is that Core:Person http://spdx.example.com/Agent/JoshuaWatt is an RDF subclass of Element which has the constraint that it can not be an Element.

Attached is the current (as of this comment) generated model.ttl file and the example file the spec repo for convenience.

package_sbom.json.txt

spdx-model.ttl.txt

Running pyshacl -e spdx-model.ttl -f spdx-model.ttl package_sbom.json will reproduce the issue. It also has some other unrelated errors.

If you would like to avoid the unrelated errors - you can use my partially hand edited model.ttl file that removes the issues except the validation error related:

spdx-model-fixed.ttl.txt

Create an IndividualElement class

I downloaded your example, and I'm not seeing the same errors you are? Maybe the files you uploaded are incorrect?

I downloaded your example, and I'm not seeing the same errors you are? Maybe the files you uploaded are incorrect?

I'll check - I'm going to re-run the tests using Alexios' latest spec-parser and on the model which includes the IndividualElement types.

Ya, I was able to reproduce it by downloading the latest model from https://spdx.org/rdf/3.0.1/spdx-model.ttl

Just finished running it - everything is fixed except the abstract class constraints.

Here's a couple snippets from the output:

Constraint Violation in NotConstraintComponent (http://www.w3.org/ns/shacl#NotConstraintComponent):
        Severity: sh:Violation
        Source Shape: ns1:ElementCollection
        Focus Node: <http://spdx.example.com/Document1>
        Value Node: <http://spdx.example.com/Document1>
        Message: Node <http://spdx.example.com/Document1> must not conform to shape [ sh:class ns1:ElementCollection ; sh:message Literal("https://spdx.org/rdf/3.0.1/terms/Core/ElementCollection is an abstract class and should not be instantiated directly. Instantiate a subclass instead.", lang=en) ]
Constraint Violation in NotConstraintComponent (http://www.w3.org/ns/shacl#NotConstraintComponent):
        Severity: sh:Violation
        Source Shape: ns1:ElementCollection
        Focus Node: <http://spdx.example.com/BOM1>
        Value Node: <http://spdx.example.com/BOM1>
        Message: Node <http://spdx.example.com/BOM1> must not conform to shape [ sh:class ns1:ElementCollection ; sh:message Literal("https://spdx.org/rdf/3.0.1/terms/Core/ElementCollection is an abstract class and should not be instantiated directly. Instantiate a subclass instead.", lang=en) ]

Note that these don't reference the individuals. From a cursory look, it to be failing on any subclass of an abstract class.

The problem is, if we remove the constraint, we loose the hints we are using in the code generators to denote the abstract classes. I spent a couple hours trying unsuccessfully to find an alternative. I'll keep looking.

I found a solution I think will work.

Adding a property constraint against the path rdf:type which restricts the type to be sh:in the list of all direct and indirect subclasses.

For example, in ElementCollection, add the property restriction:

[ sh:path rdf:type ;
    sh:in( ns1:SpdxDocument ns1:Bundle ns1:Bom ns3:Sbom ) ;
 ]

I tested this against the spec-spec example and it passes. I tested it against a JSON-LD with the following added and it failed as expected:

      {
            "type": "ElementCollection",
            "spdxId": "http://spdx.example.com/BOM1",
            "creationInfo": "_:creationinfo",
            "element": [
                "http://spdx.example.com/Package1/myprogram",
                "http://spdx.example.com/Package1"
            ]
        },

Note: adding this constraint will effectively "close" the list of subclasses that can be made of our abstract types. I'm not sure this is desirable or not.

Also note, I spent several hours trying several other alternatives and this was the only one I could find which would both pass the spec-parser example and fail when an abstract class is used. Here's a non-exhaustive list to avoid duplicative effort:

Use sh:in at the top class leve
Use sh:not abstract class with the rdf:type property shape
Various combinations of using sh:xone
Various combinations of using sh:or
Using only direct subclasses
...

BTW - I think I now understand why the current sh:not approach doesn't work. SHACL is checking agasint the shape, not the URI that defines the shape - so any rdfs subclass of a shape will always conform the the superclass shape - therefore always failing. I thought moving the constraint down to the property rdf:type would work, but based on the PyShacl validation message, it was also checking against the shape, not the value of the @type provided in the JSON-LD SPDX file.

In thinking about this, we do not want to have this constraint on the Extension class since it can not be closed.

This appears to work for me:

ns1:Element a owl:Class,
        sh:NodeShape ;
        ...
        sh:property [
            sh:path rdf:type ;
            sh:not [ sh:hasValue ns1:Element ] ;
            sh:message "https://spdx.org/rdf/3.0.1/terms/Core/Element is an abstract class and should not be instantiated directly. Instantiate a subclass instead."@en
        ] ...

Basically, instead of using sh:class it verifies that the rdf:type value specifically doesn't contain ns1:Element (which it won't for subclasses).

Try this and if it works we'll need to update shacl2code to handle this properly.

Oops - I think I made a mistake in my test ... I'll re-check

@JPEWdev

OK - it passed my tests as well.

@JPEWdev solution does not close the subclassing like my solution and is much more straightforward to implement. I vote for implementing the "not hasValue" approach.

Change implementation of abstract class restrictions

PR #171 together with the model changes in spdx/spdx-3-model#937 now fully pass validation.

Navigation Menu

Repository navigation

Checks for instantiating abstract classes is causing PyShacl checks to fail #165

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Checks for instantiating abstract classes is causing PyShacl checks to fail #165

Description

Activity

goneall commented on Dec 7, 2024

goneall commented on Dec 9, 2024

goneall commented on Dec 9, 2024

goneall commented on Dec 9, 2024

JPEWdev commented on Dec 9, 2024

goneall commented on Dec 9, 2024

JPEWdev commented on Dec 9, 2024

goneall commented on Dec 9, 2024

goneall commented on Dec 9, 2024

goneall commented on Dec 9, 2024

goneall commented on Dec 9, 2024

goneall commented on Dec 9, 2024

JPEWdev commented on Dec 9, 2024

goneall commented on Dec 9, 2024

goneall commented on Dec 10, 2024

goneall commented on Dec 10, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions