From c79c194b0f1a1c6c8f31bdb20a04ad9266008572 Mon Sep 17 00:00:00 2001
From: Ivan Subotic <400790+subotic@users.noreply.github.com>
Date: Wed, 13 Oct 2021 10:12:31 +0200
Subject: [PATCH] fix: removing cardinality of a link property (DEV-90) (#1919)
---
.../responders/v2/OntologyResponderV2.scala | 6 +-
.../v2/ontology/Cardinalities.scala | 50 +++++++-
.../v2/ontology/OntologyHelpers.scala | 4 +-
webapi/src/test/resources/logback-test.xml | 8 +-
.../knora/webapi/e2e/v2/OntologyModels.scala | 8 ++
.../webapi/e2e/v2/OntologyV2R2RSpec.scala | 109 ++++++++++++------
.../DeleteCardinalitiesFromClassSpec.scala | 16 ++-
7 files changed, 151 insertions(+), 50 deletions(-)
diff --git a/webapi/src/main/scala/org/knora/webapi/responders/v2/OntologyResponderV2.scala b/webapi/src/main/scala/org/knora/webapi/responders/v2/OntologyResponderV2.scala
index d244878ca2..bebe3080c1 100644
--- a/webapi/src/main/scala/org/knora/webapi/responders/v2/OntologyResponderV2.scala
+++ b/webapi/src/main/scala/org/knora/webapi/responders/v2/OntologyResponderV2.scala
@@ -954,7 +954,7 @@ class OntologyResponderV2(responderData: ResponderData) extends Responder(respon
// Check that the cardinalities are valid, and add any inherited cardinalities.
(internalClassDefWithLinkValueProps, cardinalitiesForClassWithInheritance) =
OntologyHelpers
- .checkCardinalitiesBeforeAdding(
+ .checkCardinalitiesBeforeAddingAndIfNecessaryAddLinkValueProperties(
internalClassDef = internalClassDef,
allBaseClassIris = allBaseClassIris.toSet,
cacheData = cacheData
@@ -1394,7 +1394,7 @@ class OntologyResponderV2(responderData: ResponderData) extends Responder(respon
(newInternalClassDefWithLinkValueProps, cardinalitiesForClassWithInheritance) =
OntologyHelpers
- .checkCardinalitiesBeforeAdding(
+ .checkCardinalitiesBeforeAddingAndIfNecessaryAddLinkValueProperties(
internalClassDef = newInternalClassDef,
allBaseClassIris = allBaseClassIris.toSet,
cacheData = cacheData,
@@ -1654,7 +1654,7 @@ class OntologyResponderV2(responderData: ResponderData) extends Responder(respon
(newInternalClassDefWithLinkValueProps, cardinalitiesForClassWithInheritance) =
OntologyHelpers
- .checkCardinalitiesBeforeAdding(
+ .checkCardinalitiesBeforeAddingAndIfNecessaryAddLinkValueProperties(
internalClassDef = newInternalClassDef,
allBaseClassIris = allBaseClassIris.toSet,
cacheData = cacheData
diff --git a/webapi/src/main/scala/org/knora/webapi/responders/v2/ontology/Cardinalities.scala b/webapi/src/main/scala/org/knora/webapi/responders/v2/ontology/Cardinalities.scala
index 52dce75f54..f5ca9659fe 100644
--- a/webapi/src/main/scala/org/knora/webapi/responders/v2/ontology/Cardinalities.scala
+++ b/webapi/src/main/scala/org/knora/webapi/responders/v2/ontology/Cardinalities.scala
@@ -132,9 +132,19 @@ object Cardinalities {
// Make an update class definition in which the cardinality to delete is removed
+ submittedPropertyToDeleteIsLinkProperty: Boolean = cacheData
+ .ontologies(submittedPropertyToDelete.getOntologyFromEntity)
+ .properties(submittedPropertyToDelete)
+ .isLinkProp
+
newClassDefinitionWithRemovedCardinality =
currentClassDefinition.copy(
- directCardinalities = currentClassDefinition.directCardinalities - submittedPropertyToDelete
+ directCardinalities =
+ if (submittedPropertyToDeleteIsLinkProperty) {
+ // if we want to remove a link property, then we also need to remove the corresponding link property value
+ currentClassDefinition.directCardinalities - submittedPropertyToDelete - submittedPropertyToDelete.fromLinkPropToLinkValueProp
+ } else
+ currentClassDefinition.directCardinalities - submittedPropertyToDelete
)
// FIXME: Refactor. From here on is copy-paste from `changeClassCardinalities`, which I don't fully understand
@@ -153,10 +163,19 @@ object Cardinalities {
(newInternalClassDefWithLinkValueProps, cardinalitiesForClassWithInheritance) =
OntologyHelpers
- .checkCardinalitiesBeforeAdding(
+ .checkCardinalitiesBeforeAddingAndIfNecessaryAddLinkValueProperties(
internalClassDef = newClassDefinitionWithRemovedCardinality,
allBaseClassIris = allBaseClassIris.toSet,
- cacheData = cacheData
+ cacheData = cacheData,
+ // since we only want to delete (and have already removed what we want), the rest of the link properties
+ // need to be marked as wanting to keep.
+ existingLinkPropsToKeep =
+ newClassDefinitionWithRemovedCardinality.directCardinalities.keySet // gets all keys from the map as a set
+ .map(propertyIri =>
+ cacheData.ontologies(propertyIri.getOntologyFromEntity).properties(propertyIri)
+ ) // turn the propertyIri into a ReadPropertyInfoV2
+ .filter(_.isLinkProp) // we are only interested in link properties
+ .map(_.entityInfoContent.propertyIri) // turn whatever is left back to a propertyIri
)
// Check that the class definition doesn't refer to any non-shared ontologies in other projects.
@@ -255,9 +274,19 @@ object Cardinalities {
// Make an update class definition in which the cardinality to delete is removed
+ submittedPropertyToDeleteIsLinkProperty: Boolean = cacheData
+ .ontologies(submittedPropertyToDelete.getOntologyFromEntity)
+ .properties(submittedPropertyToDelete)
+ .isLinkProp
+
newClassDefinitionWithRemovedCardinality =
currentClassDefinition.copy(
- directCardinalities = currentClassDefinition.directCardinalities - submittedPropertyToDelete
+ directCardinalities =
+ if (submittedPropertyToDeleteIsLinkProperty) {
+ // if we want to remove a link property, then we also need to remove the corresponding link property value
+ currentClassDefinition.directCardinalities - submittedPropertyToDelete - submittedPropertyToDelete.fromLinkPropToLinkValueProp
+ } else
+ currentClassDefinition.directCardinalities - submittedPropertyToDelete
)
// FIXME: Refactor. From here on is copy-paste from `changeClassCardinalities`, which I don't fully understand
@@ -276,10 +305,19 @@ object Cardinalities {
(newInternalClassDefWithLinkValueProps, cardinalitiesForClassWithInheritance) =
OntologyHelpers
- .checkCardinalitiesBeforeAdding(
+ .checkCardinalitiesBeforeAddingAndIfNecessaryAddLinkValueProperties(
internalClassDef = newClassDefinitionWithRemovedCardinality,
allBaseClassIris = allBaseClassIris.toSet,
- cacheData = cacheData
+ cacheData = cacheData,
+ // since we only want to delete (and have already removed what we want), the rest of the link properties
+ // need to be marked as wanting to keep.
+ existingLinkPropsToKeep =
+ newClassDefinitionWithRemovedCardinality.directCardinalities.keySet // gets all keys from the map as a set
+ .map(propertyIri =>
+ cacheData.ontologies(propertyIri.getOntologyFromEntity).properties(propertyIri)
+ ) // turn the propertyIri into a ReadPropertyInfoV2
+ .filter(_.isLinkProp) // we are only interested in link properties
+ .map(_.entityInfoContent.propertyIri) // turn whatever is left back to a propertyIri
)
// Check that the class definition doesn't refer to any non-shared ontologies in other projects.
diff --git a/webapi/src/main/scala/org/knora/webapi/responders/v2/ontology/OntologyHelpers.scala b/webapi/src/main/scala/org/knora/webapi/responders/v2/ontology/OntologyHelpers.scala
index b449e0ff91..09fa3393a3 100644
--- a/webapi/src/main/scala/org/knora/webapi/responders/v2/ontology/OntologyHelpers.scala
+++ b/webapi/src/main/scala/org/knora/webapi/responders/v2/ontology/OntologyHelpers.scala
@@ -913,7 +913,7 @@ object OntologyHelpers {
* will be kept after the update.
* @return the updated class definition, and the cardinalities resulting from inheritance.
*/
- def checkCardinalitiesBeforeAdding(
+ def checkCardinalitiesBeforeAddingAndIfNecessaryAddLinkValueProperties(
internalClassDef: ClassInfoContentV2,
allBaseClassIris: Set[SmartIri],
cacheData: OntologyCacheData,
@@ -938,7 +938,7 @@ object OntologyHelpers {
val existingLinkValuePropsToKeep = existingLinkPropsToKeep.map(_.fromLinkPropToLinkValueProp)
val newLinkPropsInClass: Set[SmartIri] = propertyDefsForDirectCardinalities
.filter(_.isLinkProp)
- .map(_.entityInfoContent.propertyIri) -- existingLinkValuePropsToKeep
+ .map(_.entityInfoContent.propertyIri) -- existingLinkPropsToKeep
val newLinkValuePropsInClass: Set[SmartIri] = propertyDefsForDirectCardinalities
.filter(_.isLinkValueProp)
.map(_.entityInfoContent.propertyIri) -- existingLinkValuePropsToKeep
diff --git a/webapi/src/test/resources/logback-test.xml b/webapi/src/test/resources/logback-test.xml
index 11795ec826..3c70216ba8 100644
--- a/webapi/src/test/resources/logback-test.xml
+++ b/webapi/src/test/resources/logback-test.xml
@@ -146,10 +146,10 @@
-
-
-
-
+
+
+
+
diff --git a/webapi/src/test/scala/org/knora/webapi/e2e/v2/OntologyModels.scala b/webapi/src/test/scala/org/knora/webapi/e2e/v2/OntologyModels.scala
index 81f3258029..7444f66dcc 100644
--- a/webapi/src/test/scala/org/knora/webapi/e2e/v2/OntologyModels.scala
+++ b/webapi/src/test/scala/org/knora/webapi/e2e/v2/OntologyModels.scala
@@ -153,6 +153,14 @@ object CardinalityRestriction {
val cardinality = "owl:maxCardinality"
val value = 1
}
+ case object MinCardinalityOne extends CardinalityRestriction {
+ val cardinality = "owl:minCardinality"
+ val value = 1
+ }
+ case object MinCardinalityZero extends CardinalityRestriction {
+ val cardinality = "owl:minCardinality"
+ val value = 0
+ }
}
final case class Property(ontology: String, property: String)
diff --git a/webapi/src/test/scala/org/knora/webapi/e2e/v2/OntologyV2R2RSpec.scala b/webapi/src/test/scala/org/knora/webapi/e2e/v2/OntologyV2R2RSpec.scala
index addacd0559..bce2c215ee 100644
--- a/webapi/src/test/scala/org/knora/webapi/e2e/v2/OntologyV2R2RSpec.scala
+++ b/webapi/src/test/scala/org/knora/webapi/e2e/v2/OntologyV2R2RSpec.scala
@@ -71,7 +71,12 @@ class OntologyV2R2RSpec extends R2RSpec {
path = "test_data/ontologies/freetest-onto.ttl",
name = "http://www.knora.org/ontology/0001/freetest"
),
- RdfDataObject(path = "test_data/all_data/freetest-data.ttl", name = "http://www.knora.org/data/0001/freetest")
+ RdfDataObject(path = "test_data/all_data/freetest-data.ttl", name = "http://www.knora.org/data/0001/freetest"),
+ RdfDataObject(
+ path = "test_data/ontologies/anything-onto.ttl",
+ name = "http://www.knora.org/ontology/0001/anything"
+ ),
+ RdfDataObject(path = "test_data/all_data/anything-data.ttl", name = "http://www.knora.org/data/0001/anything")
)
// Directory path for generated client test data
@@ -2636,43 +2641,24 @@ class OntologyV2R2RSpec extends R2RSpec {
}
// payload to test cardinality can't be deleted
- val cardinalityCantBeDeletedPayload =
- s"""
- |{
- | "@id" : "${SharedOntologyTestDataADM.FREETEST_ONTOLOGY_IRI_LocalHost}",
- | "@type" : "owl:Ontology",
- | "knora-api:lastModificationDate" : {
- | "@type" : "xsd:dateTimeStamp",
- | "@value" : "$freetestLastModDate"
- | },
- | "@graph" : [ {
- | "@id" : "freetest:FreeTest",
- | "@type" : "owl:Class",
- | "rdfs:subClassOf" : {
- | "@type": "owl:Restriction",
- | "owl:minCardinality" : 1,
- | "owl:onProperty" : {
- | "@id" : "freetest:hasText"
- | }
- | }
- | } ],
- | "@context" : {
- | "rdf" : "http://www.w3.org/1999/02/22-rdf-syntax-ns#",
- | "knora-api" : "http://api.knora.org/ontology/knora-api/v2#",
- | "owl" : "http://www.w3.org/2002/07/owl#",
- | "rdfs" : "http://www.w3.org/2000/01/rdf-schema#",
- | "xsd" : "http://www.w3.org/2001/XMLSchema#",
- | "freetest" : "${SharedOntologyTestDataADM.FREETEST_ONTOLOGY_IRI_LocalHost}#"
- | }
- |}
- """.stripMargin
+ val cardinalityCantBeDeletedPayload = AddCardinalitiesRequest.make(
+ ontologyName = "freetest",
+ lastModificationDate = freetestLastModDate,
+ className = "FreeTest",
+ restrictions = List(
+ Restriction(
+ CardinalityRestriction.MinCardinalityOne,
+ onProperty = Property(ontology = "freetest", property = "hasText")
+ )
+ )
+ )
- CollectClientTestData("candeletecardinalities-false-request", cardinalityCantBeDeletedPayload)
+ CollectClientTestData("candeletecardinalities-false-request", cardinalityCantBeDeletedPayload.value)
// Expect cardinality can't be deleted - endpoint should return CanDo response with value false
Post(
"/v2/ontologies/candeletecardinalities",
- HttpEntity(RdfMediaTypes.`application/ld+json`, cardinalityCantBeDeletedPayload)
+ HttpEntity(RdfMediaTypes.`application/ld+json`, cardinalityCantBeDeletedPayload.value)
) ~>
addCredentials(BasicHttpCredentials(anythingUsername, password)) ~> ontologiesPath ~> check {
val responseStr = responseAs[String]
@@ -2751,7 +2737,62 @@ class OntologyV2R2RSpec extends R2RSpec {
}
}
- "determine that a class's cardinalities cannot be changed" in {
+ "verify that link-property can not be deleted" in {
+
+ // payload representing a link-property to test that cardinality can't be deleted
+ val cardinalityOnLinkPropertyWhichCantBeDeletedPayload = AddCardinalitiesRequest.make(
+ ontologyName = "anything",
+ lastModificationDate = anythingLastModDate,
+ className = "Thing",
+ restrictions = List(
+ Restriction(
+ CardinalityRestriction.MinCardinalityZero,
+ onProperty = Property(ontology = "anything", property = "isPartOfOtherThing")
+ )
+ )
+ )
+ println(cardinalityOnLinkPropertyWhichCantBeDeletedPayload)
+
+ val params =
+ s"""
+ |{
+ | "@id": "http://0.0.0.0:3333/ontology/0001/anything/v2",
+ | "@type": "http://www.w3.org/2002/07/owl#Ontology",
+ | "http://api.knora.org/ontology/knora-api/v2#lastModificationDate": {
+ | "@type": "http://www.w3.org/2001/XMLSchema#dateTimeStamp",
+ | "@value": "$anythingLastModDate"
+ | },
+ | "@graph": [{
+ | "@id": "http://0.0.0.0:3333/ontology/0001/anything/v2#Thing",
+ | "@type": "http://www.w3.org/2002/07/owl#Class",
+ | "http://www.w3.org/2000/01/rdf-schema#subClassOf": {
+ | "@type": "http://www.w3.org/2002/07/owl#Restriction",
+ | "http://www.w3.org/2002/07/owl#onProperty": {
+ | "@id": "http://0.0.0.0:3333/ontology/0001/anything/v2#isPartOfOtherThing"
+ | },
+ | "http://www.w3.org/2002/07/owl#minCardinality": 0,
+ | "http://api.knora.org/ontology/salsah-gui/v2#guiOrder": 21
+ | }
+ | }]
+ |}
+ |""".stripMargin
+ println(params)
+
+ Post(
+ "/v2/ontologies/candeletecardinalities",
+ HttpEntity(RdfMediaTypes.`application/ld+json`, cardinalityOnLinkPropertyWhichCantBeDeletedPayload.value)
+ ) ~> addCredentials(
+ BasicHttpCredentials(anythingUsername, password)
+ ) ~> ontologiesPath ~> check {
+ val responseStr = responseAs[String]
+ println(responseStr)
+ assert(status == StatusCodes.OK, response.toString)
+ val responseJsonDoc = JsonLDUtil.parseJsonLD(responseStr)
+ assert(!responseJsonDoc.body.value(OntologyConstants.KnoraApiV2Complex.CanDo).asInstanceOf[JsonLDBoolean].value)
+ }
+ }
+
+ "verify that a class's cardinalities cannot be changed" in {
val classSegment = URLEncoder.encode("http://0.0.0.0:3333/ontology/0001/anything/v2#Thing", "UTF-8")
Get(s"/v2/ontologies/canreplacecardinalities/$classSegment") ~> addCredentials(
diff --git a/webapi/src/test/scala/org/knora/webapi/responders/v2/ontology/DeleteCardinalitiesFromClassSpec.scala b/webapi/src/test/scala/org/knora/webapi/responders/v2/ontology/DeleteCardinalitiesFromClassSpec.scala
index 6847190a3f..3a80f47b7f 100644
--- a/webapi/src/test/scala/org/knora/webapi/responders/v2/ontology/DeleteCardinalitiesFromClassSpec.scala
+++ b/webapi/src/test/scala/org/knora/webapi/responders/v2/ontology/DeleteCardinalitiesFromClassSpec.scala
@@ -42,7 +42,12 @@ class DeleteCardinalitiesFromClassSpec extends IntegrationSpec(TestContainerFuse
path = "test_data/ontologies/freetest-onto.ttl",
name = "http://www.knora.org/ontology/0001/freetest"
),
- RdfDataObject(path = "test_data/all_data/freetest-data.ttl", name = "http://www.knora.org/data/0001/freetest")
+ RdfDataObject(path = "test_data/all_data/freetest-data.ttl", name = "http://www.knora.org/data/0001/freetest"),
+ RdfDataObject(
+ path = "test_data/ontologies/anything-onto.ttl",
+ name = "http://www.knora.org/ontology/0001/anything"
+ ),
+ RdfDataObject(path = "test_data/all_data/anything-data.ttl", name = "http://www.knora.org/data/0001/anything")
)
// start fuseki http connector actor
@@ -67,6 +72,15 @@ class DeleteCardinalitiesFromClassSpec extends IntegrationSpec(TestContainerFuse
resF map { res => println(res); assert(res, "property is used in resource (instance of resource class)") }
}
+ "detect that link property is in use, when used in a resource" in {
+ val FreetestOntologyIri = "http://0.0.0.0:3333/ontology/0001/anything/v2".toSmartIri
+ val internalPropertyIri = FreetestOntologyIri.makeEntityIri("isPartOfOtherThing").toOntologySchema(InternalSchema)
+ println(s"internalPropertyIri: $internalPropertyIri")
+
+ val resF = Cardinalities.isPropertyUsedInResources(settings, fusekiActor, internalPropertyIri)
+ resF map { res => println(res); assert(res, "property is used in resource (instance of resource class)") }
+ }
+
"detect that property is in use, when used in a resource of a subclass" in {
val FreetestOntologyIri = "http://0.0.0.0:3333/ontology/0001/freetest/v2".toSmartIri
val internalPropertyIri = FreetestOntologyIri.makeEntityIri("hasDecimal").toOntologySchema(InternalSchema)