Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix(value): make impossible to set list root node as a value (DEV-973) (
#2088)

* add method checking if list is a root node or not

* improve ignoring trig files

* refactor HierarchicalListValueContentV2 object

* cleanup

* add list root checkiing method to ValueResponderV2 + test

* remove list root checking from ValueMessangerV2

* add new query to handle 2 checks + merge checking methods

* move throwing to responder

* fix formatting & bad merge

* fix failing test

* refactor check method to return value + move error handling to responder

* add UNIT tests for util method

* remove unused template

* review changes

* cleanup
  • Loading branch information
mpro7 committed Jul 6, 2022
1 parent 48592ad commit 94d2b46
Show file tree
Hide file tree
Showing 7 changed files with 190 additions and 36 deletions.
2 changes: 1 addition & 1 deletion .gitignore
Expand Up @@ -36,7 +36,7 @@ sipi/test
*.bak
*.rdb
.sbtrc
/*.trig
**/*.trig

dependencies.txt
/client-test-data.zip
Expand Down
Expand Up @@ -6,15 +6,16 @@
package org.knora.webapi.messages.v2.responder.valuemessages

import akka.actor.ActorRef
import com.typesafe.scalalogging.Logger
import akka.http.scaladsl.util.FastFuture
import akka.pattern._
import akka.util.Timeout
import org.knora.webapi._
import com.typesafe.scalalogging.Logger
import dsp.errors.AssertionException
import dsp.errors.BadRequestException
import dsp.errors.NotImplementedException
import dsp.errors.SipiException
import dsp.valueobjects.IriErrorMessages
import org.knora.webapi._
import org.knora.webapi.messages.IriConversions._
import org.knora.webapi.messages.OntologyConstants
import org.knora.webapi.messages.ResponderRequest.KnoraRequestV2
Expand All @@ -41,7 +42,6 @@ import java.time.Instant
import java.util.UUID
import scala.concurrent.ExecutionContext
import scala.concurrent.Future
import dsp.valueobjects.IriErrorMessages

/**
* A tagging trait for requests handled by [[org.knora.webapi.responders.v2.ValuesResponderV2]].
Expand Down
Expand Up @@ -9,17 +9,23 @@ import akka.actor.ActorRef
import akka.pattern._
import akka.util.Timeout
import com.typesafe.scalalogging.Logger
import org.knora.webapi.IRI
import dsp.errors.BadRequestException
import dsp.errors.ForbiddenException
import dsp.errors.NotFoundException
import org.knora.webapi.IRI
import org.knora.webapi.messages.OntologyConstants
import org.knora.webapi.messages.SmartIri
import org.knora.webapi.messages.StringFormatter
import org.knora.webapi.messages.admin.responder.permissionsmessages.DefaultObjectAccessPermissionsStringForPropertyGetADM
import org.knora.webapi.messages.admin.responder.permissionsmessages.DefaultObjectAccessPermissionsStringResponseADM
import org.knora.webapi.messages.admin.responder.usersmessages.UserADM
import org.knora.webapi.messages.store.sipimessages.DeleteTemporaryFileRequest
import org.knora.webapi.messages.store.sipimessages.MoveTemporaryFileToPermanentStorageRequest
import org.knora.webapi.messages.store.triplestoremessages.LiteralV2
import org.knora.webapi.messages.store.triplestoremessages.SparqlAskRequest
import org.knora.webapi.messages.store.triplestoremessages.SparqlAskResponse
import org.knora.webapi.messages.store.triplestoremessages.SparqlExtendedConstructRequest
import org.knora.webapi.messages.store.triplestoremessages.SparqlExtendedConstructResponse
import org.knora.webapi.messages.store.triplestoremessages.SubjectV2
import org.knora.webapi.messages.util.KnoraSystemInstances
import org.knora.webapi.messages.util.PermissionUtilADM
import org.knora.webapi.messages.util.PermissionUtilADM.EntityPermission
Expand All @@ -29,6 +35,7 @@ import org.knora.webapi.messages.v2.responder.resourcemessages.ReadResourceV2
import org.knora.webapi.messages.v2.responder.valuemessages.FileValueContentV2
import org.knora.webapi.messages.v2.responder.valuemessages.ReadValueV2
import org.knora.webapi.messages.v2.responder.valuemessages.ValueContentV2

import scala.concurrent.ExecutionContext
import scala.concurrent.Future
import scala.util.Failure
Expand Down Expand Up @@ -133,27 +140,49 @@ object ResourceUtilV2 {
} yield defaultObjectAccessPermissionsResponse.permissionLiteral

/**
* Checks whether a list node exists, and throws [[NotFoundException]] otherwise.
* Checks whether a list node exists and if is a root node.
*
* @param listNodeIri the IRI of the list node.
* @param nodeIri the IRI of the list node.
* @param appActor ActorRef
* @return Future of Either None for nonexistent, true for root and false for child node.
*/
def checkListNodeExists(listNodeIri: IRI, appActor: ActorRef)(implicit
def checkListNodeExistsAndIsRootNode(nodeIri: IRI, appActor: ActorRef)(implicit
timeout: Timeout,
executionContext: ExecutionContext
): Future[Unit] =
): Future[Either[Option[Nothing], Boolean]] = {
implicit val stringFormatter: StringFormatter = StringFormatter.getGeneralInstance

for {
askString <- Future(
org.knora.webapi.messages.twirl.queries.sparql.admin.txt
.checkListNodeExistsByIri(listNodeIri = listNodeIri)
.toString
)

checkListNodeExistsResponse <- appActor.ask(SparqlAskRequest(askString)).mapTo[SparqlAskResponse]

_ = if (!checkListNodeExistsResponse.result) {
throw NotFoundException(s"<$listNodeIri> does not exist or is not a ListNode")
}
} yield ()
sparqlQuery <-
Future(
org.knora.webapi.messages.twirl.queries.sparql.admin.txt
.getListNode(nodeIri = nodeIri)
.toString()
)

listNodeResponse <-
appActor
.ask(
SparqlExtendedConstructRequest(
sparql = sparqlQuery
)
)
.mapTo[SparqlExtendedConstructResponse]

statements: Map[SubjectV2, Map[SmartIri, Seq[LiteralV2]]] = listNodeResponse.statements

maybeList =
if (statements.nonEmpty) {
val propToCheck: SmartIri = stringFormatter.toSmartIri(OntologyConstants.KnoraBase.IsRootNode)
val isRootNode: Boolean = statements.map(_._2.contains(propToCheck)).head

Right(isRootNode)
} else {
Left(None)
}

} yield maybeList
}

/**
* Given a future representing an operation that was supposed to update a value in a triplestore, checks whether
Expand Down
Expand Up @@ -8,9 +8,8 @@ package org.knora.webapi.responders.v2
import akka.http.scaladsl.util.FastFuture
import akka.pattern._
import akka.stream.Materializer
import org.knora.webapi._
import dsp.errors._

import org.knora.webapi._
import org.knora.webapi.messages.IriConversions._
import org.knora.webapi.messages.OntologyConstants
import org.knora.webapi.messages.SmartIri
Expand Down Expand Up @@ -1161,9 +1160,26 @@ class ResourcesResponderV2(responderData: ResponderData) extends ResponderWithSt

Future
.sequence(
listNodesThatShouldExist
.map(listNodeIri => ResourceUtilV2.checkListNodeExists(listNodeIri, appActor))
.toSeq
listNodesThatShouldExist.map { listNodeIri =>
for {
checkNode <- ResourceUtilV2.checkListNodeExistsAndIsRootNode(listNodeIri, appActor)

_ = checkNode match {
// it doesn't have isRootNode property - it's a child node
case Right(false) => ()
// it does have isRootNode property - it's a root node
case Right(true) =>
throw BadRequestException(
s"<${listNodeIri}> is a root node. Root nodes cannot be set as values."
)
// it deosn't exists or isn't valid list
case Left(_) =>
throw NotFoundException(
s"<${listNodeIri}> does not exist, or is not a ListNode."
)
}
} yield ()
}.toSeq
)
.map(_ => ())
}
Expand Down
Expand Up @@ -7,9 +7,8 @@ package org.knora.webapi.responders.v2

import akka.http.scaladsl.util.FastFuture
import akka.pattern._
import org.knora.webapi._
import dsp.errors._

import org.knora.webapi._
import org.knora.webapi.messages.IriConversions._
import org.knora.webapi.messages.OntologyConstants
import org.knora.webapi.messages.SmartIri
Expand Down Expand Up @@ -190,11 +189,29 @@ class ValuesResponderV2(responderData: ResponderData) extends Responder(responde
requestingUser = createValueRequest.requestingUser
)

// If this is a list value, check that it points to a real list node.

// If it is a list value, check that it points to a real list node which is not a root node.
_ <- submittedInternalValueContent match {
case listValue: HierarchicalListValueContentV2 =>
ResourceUtilV2.checkListNodeExists(listValue.valueHasListNode, appActor)
for {
checkNode <-
ResourceUtilV2.checkListNodeExistsAndIsRootNode(listValue.valueHasListNode, appActor)

_ = checkNode match {
// it doesn't have isRootNode property - it's a child node
case Right(false) => ()
// it does have isRootNode property - it's a root node
case Right(true) =>
throw BadRequestException(
s"<${listValue.valueHasListNode}> is a root node. Root nodes cannot be set as values."
)
// it deosn't exists or isn't valid list
case Left(_) =>
throw NotFoundException(
s"<${listValue.valueHasListNode}> does not exist or is not a ListNode."
)
}
} yield ()

case _ => FastFuture.successful(())
}

Expand Down Expand Up @@ -325,6 +342,7 @@ class ValuesResponderV2(responderData: ResponderData) extends Responder(responde
unverifiedValue = unverifiedValue,
requestingUser = createValueRequest.requestingUser
)

} yield CreateValueResponseV2(
valueIri = verifiedValue.newValueIri,
valueType = verifiedValue.value.valueType,
Expand Down Expand Up @@ -1171,11 +1189,29 @@ class ValuesResponderV2(responderData: ResponderData) extends Responder(responde
requestingUser = updateValueRequest.requestingUser
)

// If this is a list value, check that it points to a real list node.

// If it is a list value, check that it points to a real list node which is not a root node.
_ <- submittedInternalValueContent match {
case listValue: HierarchicalListValueContentV2 =>
ResourceUtilV2.checkListNodeExists(listValue.valueHasListNode, appActor)
for {
checkNode <-
ResourceUtilV2.checkListNodeExistsAndIsRootNode(listValue.valueHasListNode, appActor)

_ = checkNode match {
// it doesn't have isRootNode property - it's a child node
case Right(false) => ()
// it does have isRootNode property - it's a root node
case Right(true) =>
throw BadRequestException(
s"<${listValue.valueHasListNode}> is a root node. Root nodes cannot be set as values."
)
// it deosn't exists or isn't valid list
case Left(_) =>
throw NotFoundException(
s"<${listValue.valueHasListNode}> does not exist or is not a ListNode."
)
}
} yield ()

case _ => FastFuture.successful(())
}

Expand Down
@@ -0,0 +1,48 @@
/*
* Copyright © 2021 - 2022 Swiss National Data and Service Center for the Humanities and/or DaSCH Service Platform contributors.
* SPDX-License-Identifier: Apache-2.0
*/

package org.knora.webapi.responders.v2

import akka.util.Timeout
import org.knora.webapi.CoreSpec
import org.knora.webapi.messages.store.triplestoremessages.RdfDataObject

import scala.concurrent.ExecutionContextExecutor

object ResourceUtilV2Spec {}

class ResourceUtilV2Spec extends CoreSpec() {
implicit val timeout: Timeout = settings.defaultTimeout
implicit val ec: ExecutionContextExecutor = system.dispatcher

override lazy val rdfDataObjects = List(
RdfDataObject(path = "test_data/all_data/anything-data.ttl", name = "http://www.knora.org/data/0001/anything")
)

"ResourceUtil" when {
"called `checkListNodeExistsAndIsRootNode` method" should {
"return FALSE for list child node" in {
val nodeIri = "http://rdfh.ch/lists/0001/otherTreeList01"
val checkNode = ResourceUtilV2.checkListNodeExistsAndIsRootNode(nodeIri, appActor)

checkNode.onComplete(_.get should be(Right(false)))
}

"return TRUE for list root node" in {
val nodeIri = "http://rdfh.ch/lists/0001/otherTreeList"
val checkNode = ResourceUtilV2.checkListNodeExistsAndIsRootNode(nodeIri, appActor)

checkNode.onComplete(_.get should be(Right(true)))
}

"should return NONE for nonexistent list" in {
val nodeIri = "http://rdfh.ch/lists/0001/otherTreeList77"
val checkNode = ResourceUtilV2.checkListNodeExistsAndIsRootNode(nodeIri, appActor)

checkNode.onComplete(_.get should be(Left(None)))
}
}
}
}
Expand Up @@ -6,9 +6,9 @@
package org.knora.webapi.responders.v2

import akka.testkit.ImplicitSender
import dsp.errors._
import org.knora.webapi._
import org.knora.webapi.config.AppConfig
import dsp.errors._
import org.knora.webapi.messages.IriConversions._
import org.knora.webapi.messages.OntologyConstants
import org.knora.webapi.messages.SmartIri
Expand Down Expand Up @@ -1754,6 +1754,31 @@ class ValuesResponderV2Spec extends CoreSpec() with ImplicitSender {
}
}

"not create a list value that is a root list node" in {
val resourceIri = "http://rdfh.ch/0001/a-blue-thing"
val resourceClassIri = "http://www.knora.org/ontology/0001/anything#BlueThing".toSmartIri
val propertyIri: SmartIri = "http://0.0.0.0:3333/ontology/0001/anything/v2#hasListItem".toSmartIri
val valueHasListNode = "http://rdfh.ch/lists/0001/otherTreeList"

appActor ! CreateValueRequestV2(
CreateValueV2(
resourceIri = resourceIri,
resourceClassIri = resourceClassIri,
propertyIri = propertyIri,
valueContent = HierarchicalListValueContentV2(
ontologySchema = ApiV2Complex,
valueHasListNode = valueHasListNode
)
),
requestingUser = anythingUser1,
apiRequestID = UUID.randomUUID
)

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

"create a color value" in {
// Add the value.

Expand Down

0 comments on commit 94d2b46

Please sign in to comment.