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

Include guiElement and guiAttribute in API v2 #755

Merged
merged 28 commits into from
Mar 7, 2018

Conversation

benjamingeer
Copy link

@benjamingeer benjamingeer commented Feb 27, 2018

  • Rewrite ontology loading and caching in OntologyResponderV2 to support serving built-in ontologies like salsah-gui from the triplestore, including OWL named individuals.
  • Change salsah-gui so that GraphDB's consistency checker can check the use of guiElement and guiAttribute.
  • Serve the salsah-gui ontology in API v2 in the default schema.
  • Make existing functionality work.
  • Simplify the hasOntologies format, eliminating hasStandoffClasses and hasStandoffProperties.
  • Show salsah-gui:guiElement and salsah-gui:guiAttribute when serving ontologies in API v2 in the default schema.
  • Allow salsah-gui:guiElement and salsah-gui:guiAttribute to be included in new property definitions created via API v2.
  • Update docs.
  • Update release notes.

The existing design of ontology loading and caching in OntologyResponderV2 was made for API v1, which only needed to serve project-specific ontologies. It was based on several different SPARQL SELECT queries for different sorts of entities. These queries knew quite a lot about the structure of Knora ontologies. Only certain kinds of entities and cardinalities were loaded and cached, and the rest of the code relied on that. Now that we have to serve classes like salsah-gui:Guielement and properties like salsah-gui:guiAttribute, that’s no longer workable. Rather than add more SELECT queries and special cases, it seems better to redesign the whole thing.

The revised design in this PR is based on a CONSTRUCT query that just reads all statements from each ontology graph. This makes the code simpler and probably easier to read. All entities are then processed in the same way into Content objects. As a result, code using these objects must now assume that everything that’s in the ontologies in the triplestore is also in the ontology cache. The Content objects are then wrapped in Read objects, which have more knowledge about the different types of Knora entities and how to represent them in the API.

Also, I've restructured the cache so each ontology, including knora-base, is now stored in a ReadOntologyV2 object. This should make it easier to do #762.

As an extra bonus, IntelliJ IDEA no longer uses 100% CPU when you work on OntologyResponderV2.

Resolves #746.
Resolves #760.

@benjamingeer benjamingeer added this to the v1.3.0 milestone Feb 27, 2018
@benjamingeer benjamingeer self-assigned this Feb 27, 2018
@benjamingeer benjamingeer added enhancement improve existing code or new feature frontend (salsah) labels Mar 2, 2018
Benjamin Geer added 6 commits March 5, 2018 16:44
# Conflicts:
#	webapi/src/test/resources/test-data/ontologyR2RV2/allOntologyMetadata.json
# Conflicts:
#	webapi/src/main/scala/org/knora/webapi/messages/store/triplestoremessages/TriplestoreMessages.scala
#	webapi/src/main/scala/org/knora/webapi/store/triplestore/http/HttpTriplestoreConnector.scala
@benjamingeer
Copy link
Author

@tobiasschweizer this PR removes hasStandoffClasses and hasStandoffProperties from the ontology responses, as discussed in #760.

@tobiasschweizer
Copy link
Contributor

this PR removes hasStandoffClasses and hasStandoffProperties from the ontology responses, as discussed in #760.

Could you please check if that requires changes in the SALSAH 2 GUI?

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.

did you maybe run the Salsah 1 tests?

@tobiasschweizer
Copy link
Contributor

this PR requires dhlab-basel/Salsah#190 to merged too

@benjamingeer
Copy link
Author

did you maybe run the Salsah 1 tests?

Good catch! Had to fix a bug for that. They pass now.

@subotic
Copy link
Collaborator

subotic commented Mar 7, 2018

great, thanks :-)

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.

LGTM. thanks :-)

@subotic
Copy link
Collaborator

subotic commented Mar 7, 2018

Some general thoughts about the release notes. Maybe we should add something like a migration guide. There we could move the stuff from Required changes to existing data:. This would be one document, and then we wouldn't need to repeat things.

@benjamingeer
Copy link
Author

@subotic Thanks! A migration guide sounds like a good idea to me.

@mrivoal
Copy link

mrivoal commented Mar 7, 2018

@benjamingeer just a question: didn't you say that you would would come with a script to make existing ontologies comply with the new requirements of salsah-gui:gui-element?

@mrivoal
Copy link

mrivoal commented Mar 7, 2018

Ok, I have just been explained by @loicjaouen and @gfoo where I should look for this information. Just ignore my question/comment.

@benjamingeer benjamingeer merged commit 814fc76 into develop Mar 7, 2018
@benjamingeer benjamingeer deleted the wip/update-salsah-gui-attributes branch March 7, 2018 16:01
@benjamingeer
Copy link
Author

@mrivoal Existing ontologies should not be affected by this PR; it just affects the way ontologies are represented in API v2.

@mrivoal
Copy link

mrivoal commented Mar 7, 2018

Sorry. I mixed up this issue with #737, which requires us to update our existing ontologies according to v.1.2.0 Release Notes.

@benjamingeer
Copy link
Author

@mrivoal No worries!

@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
API/V2 enhancement improve existing code or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants