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

Validate ontologies loaded from the triplestore #806

Merged
merged 22 commits into from Apr 23, 2018

Conversation

benjamingeer
Copy link

@benjamingeer benjamingeer commented Apr 3, 2018

Checks on classes

  • Within the cardinalities of a class, there must be a link value property for each link property and vice versa.
  • Each class must be a subclass of all the classes that are subject class constraints of the properties in its cardinalities.

If a class is defined in a project-specific ontology:

  • It must be either a resource class or a standoff class, but not both (note that this forbids project-specific subclasses of knora-base:Value).
  • All its cardinalities must be on properties that are defined in the triplestore.
  • If it's a resource class, all its directly defined cardinalities must be on Knora resource properties (subproperties of knora-base:hasValue or knora-base:hasLinkTo), and all its base classes with Knora IRIs must also be resource classes. A cardinality on knora-base:resourceProperty or knora-base:hasValue is forbidden. It must also have an rdfs:label.
  • If it's a standoff class, none of its cardinalities may be on Knora resource properties, and all its base classes with Knora IRIs must also be standoff classes.

Checks on properties

  • The property's subject class constraint, if provided, must be a Knora class, owl:Class, or owl:Restriction, and must be a subclass of the subject class constraints of all its base properties.
  • The property's object class constraint, if provided, must be a subclass of the object class constraints of all its base properties.
  • If the property is a Knora resource property, it must have an object class constraint and an rdfs:label.

If a property is defined in a project-specific ontology:

  • Its subject class constraint, if provided, must be a Knora resource or standoff class.

If a property is defined in a project-specific ontology and is a Knora resource property:

  • It can't be a subproperty of both knora-base:hasValue and knora-base:hasLinkTo.
  • It can't be a subproperty of knora-base:hasFileValue.
  • Each of its base properties that has a Knora IRI must also be a Knora resource property.

Other checks

  • The predicate salsah-gui:guiOrder is allowed in an owl:Restriction, not in a property definition.
  • The predicates salsah-gui:guiElement and salsah-gui:guiAttribute are allowed in a property definition, not in an owl:Restriction.

Other tasks

  • Get rid of the redundant predicate knora-api:projectIri, use knora-api:attachedToProject instead.
  • Fix ontology errors found by these additional checks.
  • Add tests that load invalid ontologies from the triplestore.
  • Check that SALSAH 1 e2e tests pass.
  • Update docs.
  • Update release notes.

Resolves #778.

@benjamingeer benjamingeer added enhancement improve existing code or new feature in-progress labels Apr 3, 2018
@benjamingeer benjamingeer added this to the v1.4.0 milestone Apr 3, 2018
@benjamingeer benjamingeer self-assigned this Apr 3, 2018
Benjamin Geer added 2 commits April 3, 2018 18:34
- drawings-gods:hasPersonLinkedFileComment should have knora-base:subjectClassConstraint drawings-gods:PersonLinkedFile
- remove cardinality on undefined property: drawings-gods:ifIToldYouGodCouldYouDrawDoNotKnow
- remove cardinality on undefined property: drawings-gods:otherLanguages3
@benjamingeer
Copy link
Author

@subotic Cool, that fixed it, thanks. :)

@subotic
Copy link
Collaborator

subotic commented Apr 5, 2018

you're welcome :-)

@benjamingeer
Copy link
Author

@loicjaouen @mrivoal The ontology verification introduced in this PR found some problems in drawings-gods_ontology.ttl:

  • The class drawings-gods:PersonLinkedFile has a cardinality for drawings-gods:hasPersonLinkedFileComment, but that property had a knora-base:subjectClassConstraint of drawings-gods:Person. I've changed its subject class constraint to drawings-gods:PersonLinkedFile.
  • drawings-gods:QuestionnaireIndividual had cardinalities for two nonexistent properties: drawings-gods:ifIToldYouGodCouldYouDrawDoNotKnow and drawings-gods:otherLanguages3. I've removed both cardinalities.

Please check whether these fixes are correct.

Also, in this PR I've upgraded the RDF4J Turtle library to 2.3.0, so the output of TransformOntology (which moves salsah-gui:guiOrder to the right place) now contains anonymous blank nodes for cardinalities. I've used it to reformat drawings-gods_ontology.ttl.

@benjamingeer
Copy link
Author

@tobiasschweizer @SepidehAlassi The ontology verification introduced in this PR found a typo in biblio-onto.ttl:

https://github.com/dhlab-basel/Knora/pull/806/files#diff-d4765708f69ccf775ec8c8803fbc1a24

@tobiasschweizer
Copy link
Contributor

@tobiasschweizer @SepidehAlassi The ontology verification introduced in this PR found a typo in biblio-onto.ttl:

Great job! Thanks for fixing this!

@janCstoffregen
Copy link

Dear dhlab,
I'm not sure how many necessary changes this will cause in our ontology. Are the error messages of those checks written in a way that we will understand what is not working in our ontology when the time comes?

As you know, we have quite a bit of ontologies already here: https://github.com/nie-ine/Ontologies/tree/master/nie-ontologies
Those ontologies work with an older version of Knora, we can import them locally in GraphDB and display them in Salsah. Is there a smart way to perform the necessary changes, maybe in a certain order? Any advice would be much appreciated since this looks like it will be a time intensive procedure for us.

Also, we wonder which goals the restrictions have, are the changes written to improve the performance or for another reason?

Thank you so much for your answer.
Best regards,
Jan

@benjamingeer
Copy link
Author

Are the error messages of those checks written in a way that we will understand what is not working in our ontology when the time comes?

Yes, that's the intention.

we wonder which goals the restrictions have

The goal is to prevent errors. None of these restrictions are new. I think all of them are already described in the knora-base ontology documentation:

http://www.knora.org/documentation/manual/rst/02-knora-ontologies/knora-base.html

Please read that document if you haven't already.

If a project-specific ontology doesn't conform to the requirements of knora-base, the Knora API server won't be able to use it correctly.

Currently it's possible to violate some of these restrictions without getting an error message, but you will get unexpected results. The purpose of this PR is to have all such violations cause error messages on startup.

@janCstoffregen
Copy link

ok, thank you for this clarification, I did not realise that there are no new restrictions in this pr.

@benjamingeer
Copy link
Author

I did not realise that there are no new restrictions in this pr.

I’ll double-check the docs and document any restrictions that weren’t documented before.

# Conflicts:
#	webapi/_test_data/other.v1.DrawingsGodsV1Spec/drawings-gods_ontology.ttl
#	webapi/src/main/scala/org/knora/webapi/KnoraService.scala
@mrivoal
Copy link

mrivoal commented Apr 10, 2018

@benjamingeer, yes, your changes to drawings-gods_ontology.ttl are correct, thanks!
Glad to see that such mistakes can now be pointed out!

@SepidehAlassi
Copy link
Contributor

@benjamingeer wow, I am impressed!

@benjamingeer
Copy link
Author

@subotic I'll be on holiday next week, so there's no need to hurry to review this. :)

@@ -264,8 +266,6 @@

rdfs:comment "A description of a user group"@en ;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'A description of an institution'

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are fast :-)


loadTestData(invalidOnto)

expectMsgPF(timeout) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a tip, you could write

expectMsgPF(timeout) {
                 case msg: akka.actor.Status.Failure => msg.cause.isInstanceOf[InconsistentTriplestoreDataException] should ===(true)
             }

as

expectMsgType[akka.actor.Status.Failure](timeout).cause should be (true)

expectMsgPF is useful when you expect more than one message type.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll do that next time. :)

Copy link
Collaborator

@subotic subotic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test, that loads an ontology with a missing knora-base:attachedToProject?

@benjamingeer
Copy link
Author

Good idea, I'll add one now.

@subotic
Copy link
Collaborator

subotic commented Apr 23, 2018

Thanks :-)

@benjamingeer
Copy link
Author

Thanks for the review! It's a relief to finally see this done.

@subotic
Copy link
Collaborator

subotic commented Apr 23, 2018

Thanks for doing it :-) It's a huge thing.

@benjamingeer benjamingeer merged commit c2320fd into develop Apr 23, 2018
@benjamingeer benjamingeer deleted the wip/validate-loaded-ontologies branch April 23, 2018 12:06
SepidehAlassi added a commit that referenced this pull request Apr 30, 2018
* develop:
  feature (webapi): make project shortcode required (#824)
  doc (webapi): jekyll / paradox based documentation website (#751)
  Validate ontologies loaded from the triplestore (#806)
  fix (webapi): add searchbox gui element to hasOtherSomething property (#828)
  feature (webapi): allow loading data into graphdb on windows (internal PR) (#814)
  build (webapi): add missing library (#822)
  feature (webapi): allow changing of user’s password by system admin (#815)
  feature (webapi): OpenAPI / Swagger API documentation generation (#812)
SepidehAlassi added a commit that referenced this pull request Apr 30, 2018
* develop:
  feature (webapi): make project shortcode required (#824)
  doc (webapi): jekyll / paradox based documentation website (#751)
  Validate ontologies loaded from the triplestore (#806)
  fix (webapi): add searchbox gui element to hasOtherSomething property (#828)
  feature (webapi): allow loading data into graphdb on windows (internal PR) (#814)
  build (webapi): add missing library (#822)
  feature (webapi): allow changing of user’s password by system admin (#815)
  feature (webapi): OpenAPI / Swagger API documentation generation (#812)
@benjamingeer benjamingeer mentioned this pull request Jul 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improve existing code or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants