From 82148776b626cfba66d7274c9ea0295435f9a367 Mon Sep 17 00:00:00 2001 From: Benjamin Geer Date: Wed, 2 Sep 2020 18:12:37 +0200 Subject: [PATCH] fix(sipi): Improve performance of file value query (#1697) --- docs/02-knora-ontologies/knora-base.md | 7 +-- .../responders/admin/SipiResponderADM.scala | 43 ++++++++-------- .../admin/getFileValueGraphDB.scala.txt | 49 ++++++------------- .../admin/getFileValueStandard.scala.txt | 39 +++++---------- 4 files changed, 47 insertions(+), 91 deletions(-) diff --git a/docs/02-knora-ontologies/knora-base.md b/docs/02-knora-ontologies/knora-base.md index fc579086d9..909b98c896 100644 --- a/docs/02-knora-ontologies/knora-base.md +++ b/docs/02-knora-ontologies/knora-base.md @@ -72,7 +72,7 @@ property name): : The `kb:Institution` that the project belongs to. -Ontologies, resources, and valuesare associated with a project by means of the +Ontologies and resources are associated with a project by means of the `kb:attachedToProject` property, as described in [Ontologies](#ontologies) and [Properties of Resource](#properties-of-resource)). Users are associated with a project by means of the `kb:isInProject` property, as described in @@ -318,11 +318,6 @@ created by copying data from a deleted value. : The user who owns the value. -`attachedToProject` (0-1) - -: The project that the value is part of. If not specified, defaults to - the project of the containing resource. - `valueHasString` (1) : A human-readable string representation of the value's contents, diff --git a/webapi/src/main/scala/org/knora/webapi/responders/admin/SipiResponderADM.scala b/webapi/src/main/scala/org/knora/webapi/responders/admin/SipiResponderADM.scala index 1695d4be23..597a793a3b 100644 --- a/webapi/src/main/scala/org/knora/webapi/responders/admin/SipiResponderADM.scala +++ b/webapi/src/main/scala/org/knora/webapi/responders/admin/SipiResponderADM.scala @@ -22,11 +22,11 @@ package org.knora.webapi.responders.admin import akka.actor.Status import akka.http.scaladsl.util.FastFuture import akka.pattern._ -import org.knora.webapi._ import org.knora.webapi.exceptions.{InconsistentTriplestoreDataException, NotFoundException} +import org.knora.webapi.messages.SmartIri import org.knora.webapi.messages.admin.responder.projectsmessages.{ProjectIdentifierADM, ProjectRestrictedViewSettingsADM, ProjectRestrictedViewSettingsGetADM} import org.knora.webapi.messages.admin.responder.sipimessages.{SipiFileInfoGetRequestADM, SipiFileInfoGetResponseADM, SipiResponderRequestADM} -import org.knora.webapi.messages.store.triplestoremessages.{SparqlSelectRequest, SparqlSelectResponse, VariableResultsRow} +import org.knora.webapi.messages.store.triplestoremessages.{IriSubjectV2, LiteralV2, SparqlExtendedConstructRequest, SparqlExtendedConstructResponse} import org.knora.webapi.messages.util.PermissionUtilADM.EntityPermission import org.knora.webapi.messages.util.{KnoraSystemInstances, PermissionUtilADM, ResponderData} import org.knora.webapi.responders.Responder @@ -69,30 +69,25 @@ class SipiResponderADM(responderData: ResponderData) extends Responder(responder filename = request.filename ).toString()) - // _ = println(sparqlQuery) + queryResponse: SparqlExtendedConstructResponse <- (storeManager ? SparqlExtendedConstructRequest(sparqlQuery)).mapTo[SparqlExtendedConstructResponse] - queryResponse <- (storeManager ? SparqlSelectRequest(sparqlQuery)).mapTo[SparqlSelectResponse] + _ = if (queryResponse.statements.isEmpty) throw NotFoundException(s"No file value was found for filename ${request.filename}") + _ = if (queryResponse.statements.size > 1) throw InconsistentTriplestoreDataException(s"Filename ${request.filename} is used in more than one file value") - rows: Seq[VariableResultsRow] = queryResponse.results.bindings - - // check if rows were found for the given filename - _ = if (rows.isEmpty) throw NotFoundException(s"No file value was found for filename ${request.filename}") - - // check that only one file value was found (by grouping by file value IRI) - groupedByFileValue: Map[String, Seq[VariableResultsRow]] = rows.groupBy { - row: VariableResultsRow => row.rowMap("fileValue") + fileValueIriSubject: IriSubjectV2 = queryResponse.statements.keys.head match { + case iriSubject: IriSubjectV2 => iriSubject + case _ => throw InconsistentTriplestoreDataException(s"The subject of the file value with filename ${request.filename} is not an IRI") } - _ = if (groupedByFileValue.size > 1) throw InconsistentTriplestoreDataException(s"Filename ${request.filename} is used in more than one file value") - - fileValueIri: IRI = groupedByFileValue.keys.head - - assertions: Seq[(String, String)] = rows.map { - row => (row.rowMap("objPred"), row.rowMap("objObj")) + assertions: Seq[(String, String)] = queryResponse.statements(fileValueIriSubject).toSeq.flatMap { + case (predicate: SmartIri, values: Seq[LiteralV2]) => + values.map { + value => predicate.toString -> value.toString + } } maybeEntityPermission: Option[EntityPermission] = PermissionUtilADM.getUserPermissionFromAssertionsADM( - entityIri = fileValueIri, + entityIri = fileValueIriSubject.toString, assertions = assertions, requestingUser = request.requestingUser ) @@ -102,15 +97,17 @@ class SipiResponderADM(responderData: ResponderData) extends Responder(responder permissionCode: Int = maybeEntityPermission.map(_.toInt).getOrElse(0) // Sipi expects a permission code from 0 to 8 response <- permissionCode match { - case 1 => for { - maybeRVSettings <- (responderManager ? ProjectRestrictedViewSettingsGetADM(ProjectIdentifierADM(maybeShortcode = Some(request.projectID)), KnoraSystemInstances.Users.SystemUser)).mapTo[Option[ProjectRestrictedViewSettingsADM]] - + maybeRVSettings <- ( + responderManager ? ProjectRestrictedViewSettingsGetADM( + ProjectIdentifierADM(maybeShortcode = Some(request.projectID)), + requestingUser = KnoraSystemInstances.Users.SystemUser) + ).mapTo[Option[ProjectRestrictedViewSettingsADM]] } yield SipiFileInfoGetResponseADM(permissionCode = permissionCode, maybeRVSettings) + case _ => FastFuture.successful(SipiFileInfoGetResponseADM(permissionCode = permissionCode, restrictedViewSettings = None)) } - } yield response } } \ No newline at end of file diff --git a/webapi/src/main/twirl/org/knora/webapi/messages/twirl/queries/sparql/admin/getFileValueGraphDB.scala.txt b/webapi/src/main/twirl/org/knora/webapi/messages/twirl/queries/sparql/admin/getFileValueGraphDB.scala.txt index 078302d5b7..9373b220bf 100644 --- a/webapi/src/main/twirl/org/knora/webapi/messages/twirl/queries/sparql/admin/getFileValueGraphDB.scala.txt +++ b/webapi/src/main/twirl/org/knora/webapi/messages/twirl/queries/sparql/admin/getFileValueGraphDB.scala.txt @@ -24,10 +24,7 @@ * * This template is used only by getFileValue.scala.txt. * - * Since the triplestore type is GraphDB, we assume that inference is enabled, and we use it to optimise the generated - * SPARQL. Specifically, we use inference to return search results matching subproperties of Knora base properties - * such as knora-base:hasFileValue. This requires us to use GraphDB's GRAPH - * whenever we need to get explicit (non-inferred) statements. + * Since the triplestore type is GraphDB, we turn off inference in this query because it is not needed. * * @param fileValueIri the IRI of the file value. *@ @@ -38,40 +35,22 @@ PREFIX rdf: PREFIX rdfs: PREFIX knora-base: -SELECT ?fileValue ?objPred ?objObj +CONSTRUCT { + ?fileValue ?objPred ?objObj . + ?fileValue knora-base:attachedToProject ?resourceProject . + ?fileValue knora-base:hasPermissions ?currentFileValuePermissions . +} +FROM WHERE { - ?fileValue knora-base:internalFilename "@filename" . ?currentFileValue knora-base:previousValue* ?fileValue ; - knora-base:isDeleted false . - - ?resource knora-base:hasFileValue ?currentFileValue ; - knora-base:isDeleted false . - - { - GRAPH { - ?fileValue ?objPred ?objObj . - } + knora-base:hasPermissions ?currentFileValuePermissions . - FILTER(?objPred != knora-base:attachedToProject && ?objPred != knora-base:hasPermissions) - } - UNION - { - @* Return the permissions of the current version of the value. *@ + ?resource ?prop ?currentFileValue ; + knora-base:attachedToProject ?resourceProject . - ?currentFileValue knora-base:hasPermissions ?currentFileValuePermissions . - - BIND(knora-base:hasPermissions AS ?objPred) - BIND(?currentFileValuePermissions AS ?objObj) - } - UNION - { - @* Return the project of the resource that contains the value. *@ - - ?resource knora-base:attachedToProject ?resourceProject . - - BIND(knora-base:attachedToProject AS ?objPred) - BIND(?resourceProject AS ?objObj) - } -} + ?fileValue ?objPred ?objObj . + ?currentFileValue knora-base:isDeleted false . + ?resource knora-base:isDeleted false . +} \ No newline at end of file diff --git a/webapi/src/main/twirl/org/knora/webapi/messages/twirl/queries/sparql/admin/getFileValueStandard.scala.txt b/webapi/src/main/twirl/org/knora/webapi/messages/twirl/queries/sparql/admin/getFileValueStandard.scala.txt index e983f1e2b4..0ecb756dc3 100644 --- a/webapi/src/main/twirl/org/knora/webapi/messages/twirl/queries/sparql/admin/getFileValueStandard.scala.txt +++ b/webapi/src/main/twirl/org/knora/webapi/messages/twirl/queries/sparql/admin/getFileValueStandard.scala.txt @@ -36,39 +36,24 @@ PREFIX rdf: PREFIX rdfs: PREFIX knora-base: -SELECT ?fileValue ?objPred ?objObj -WHERE { - +CONSTRUCT { + ?fileValue ?objPred ?objObj . + ?fileValue knora-base:attachedToProject ?resourceProject . + ?fileValue knora-base:hasPermissions ?currentFileValuePermissions . +} WHERE { ?fileValue knora-base:internalFilename "@filename" . ?currentFileValue knora-base:previousValue* ?fileValue ; - knora-base:isDeleted false . - - ?prop rdfs:subPropertyOf* knora-base:hasFileValue . + knora-base:hasPermissions ?currentFileValuePermissions . ?resource ?prop ?currentFileValue ; - knora-base:isDeleted false . - - { - ?fileValue ?objPred ?objObj . - FILTER(?objPred != knora-base:attachedToProject && ?objPred != knora-base:hasPermissions) - } - UNION - { - @* Return the permissions of the current version of the value. *@ - - ?currentFileValue knora-base:hasPermissions ?currentFileValuePermissions . + knora-base:attachedToProject ?resourceProject . - BIND(knora-base:hasPermissions AS ?objPred) - BIND(?currentFileValuePermissions AS ?objObj) - } - UNION - { - @* Return the project of the resource that contains the value. *@ + ?fileValue ?objPred ?objObj . - ?resource knora-base:attachedToProject ?resourceProject . + @* This FILTER is unnecessary, but it makes Jena run the query faster. *@ + FILTER(?objPred != knora-base:previousValue) - BIND(knora-base:attachedToProject AS ?objPred) - BIND(?resourceProject AS ?objObj) - } + ?currentFileValue knora-base:isDeleted false . + ?resource knora-base:isDeleted false . }