Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SHACL validation using SHACL-SHACL #335

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

MarekSuchanek
Copy link
Contributor

@MarekSuchanek MarekSuchanek commented Sep 19, 2022

supersedes #95

@MarekSuchanek
Copy link
Contributor Author

MarekSuchanek commented Sep 19, 2022

@kburger there is some issue resulting in org.eclipse.rdf4j.sail.shacl.ast.ShaclUnsupportedException for the shacl-shacl.ttl (based on #95), any idea why that happends? 😕

@kburger
Copy link
Contributor

kburger commented Sep 19, 2022

@kburger there is some issue resulting in org.eclipse.rdf4j.sail.shacl.ast.ShaclUnsupportedException for the shacl-shacl.ttl (based on #95), any idea why that happends? 😕

I'm guessing certain features in the shacl-shacl are not supported. https://rdf4j.org/documentation/programming/shacl/#supported-shacl-features states for example:

sh:path is limited to single predicate paths, e.g. ex:age or a single inverse path. Sequence paths, alternative paths and the like are not supported.

shacl-shacl contains a lot of those, so maybe that causes the exception to be thrown instead of a shacl violation report. One option is to postpone this feature until we have a library that supports it. Alternative is to slim down the shacl-shacl shapes to the supported features from rdf4j's shacl validation.

@MarekSuchanek
Copy link
Contributor Author

shacl-shacl contains a lot of those, so maybe that causes the exception to be thrown instead of a shacl violation report. One option is to postpone this feature until we have a library that supports it. Alternative is to slim down the shacl-shacl shapes to the supported features from rdf4j's shacl validation.

Hmmm 🤔 Maybe having there an exclusion mechanism for all triples with predicates not listed in ShaclSail.getSupportedShaclPredicates() would be an actually good way to go. Even now, when someone creates a schema with unsupported predicates, the validation will always fail... What do you think?

@kburger
Copy link
Contributor

kburger commented Sep 20, 2022

shacl-shacl contains a lot of those, so maybe that causes the exception to be thrown instead of a shacl violation report. One option is to postpone this feature until we have a library that supports it. Alternative is to slim down the shacl-shacl shapes to the supported features from rdf4j's shacl validation.

Hmmm 🤔 Maybe having there an exclusion mechanism for all triples with predicates not listed in ShaclSail.getSupportedShaclPredicates() would be an actually good way to go. Even now, when someone creates a schema with unsupported predicates, the validation will always fail... What do you think?

Worth a try. Would we then warn about unsupported triples and reject the shape? Or silently ignore them? (sounds like the worst option of the two).

@MarekSuchanek
Copy link
Contributor Author

Worth a try. Would we then warn about unsupported triples and reject the shape? Or silently ignore them? (sounds like the worst option of the two).

That would require some validation reporting back what will be ignored... and some unsupported things are also a bit more complicated than just a predicate not in a list (as you indicated the one with sh:path or nested sh:property).

@MarekSuchanek
Copy link
Contributor Author

@kburger It seems that after rebasing to the newest develop (with new version of RDF4J) this passes all tests. Question is whether we want to still improve something here...

@MarekSuchanek
Copy link
Contributor Author

Next step: check error messages and possibly improve what is displayed in the client

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants