Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix(canDeleteCardinalities): canDeleteCardinalities checks too eagerly (
DEV-187) (#1941)

* make subjectClassName optional

* add test to reproduce bug

* add class to check if property is used

* Update expected-client-test-data.txt

* fix failing unit tests

* restructure and fix failing unit tests

* Resolve TODO

* Fix wrong docstring

* use camelCase for ontology names

* Use constant for freetestOntology
  • Loading branch information
irinaschubert committed Nov 22, 2021
1 parent 097494b commit 298ba47
Show file tree
Hide file tree
Showing 9 changed files with 348 additions and 34 deletions.
20 changes: 20 additions & 0 deletions test_data/all_data/freetest-data.ttl
Expand Up @@ -80,3 +80,23 @@
knora-base:valueHasString "1.5";
knora-base:hasPermissions "CR knora-admin:Creator|M knora-admin:ProjectMember|V knora-admin:KnownUser|RV knora-admin:UnknownUser";
knora-base:attachedToUser <http://rdfh.ch/users/BhkfBc3hTeS_IDo-JgXRbQ> .

<http://rdfh.ch/0001/free-test-resouce-class-instance-01> a freetest:FreeTestResourceClass ;
knora-base:attachedToUser <http://rdfh.ch/users/9XBCrDV3SRa7kS1WwynB4Q>;
knora-base:attachedToProject <http://rdfh.ch/projects/0001>;
knora-base:hasPermissions "V knora-admin:UnknownUser|M knora-admin:ProjectMember";
knora-base:creationDate "2019-11-29T10:00:00.673298Z"^^xsd:dateTime;

freetest:hasIntegerProperty <http://rdfh.ch/0001/free-test-resouce-class-instance-01/values/has-integer-property-value-01> ;
rdfs:label "a free test resource class instance";
knora-base:isDeleted false .

<http://rdfh.ch/0001/free-test-resouce-class-instance-01/values/has-integer-property-value-01> a knora-base:IntValue;
knora-base:valueHasUUID "bXMwnrHvQH2DMjOFrGmNzg"^^xsd:string;
knora-base:isDeleted false;
knora-base:valueCreationDate "2018-05-28T15:52:03.897Z"^^xsd:dateTime;
knora-base:valueHasInteger "1"^^xsd:integer;
knora-base:valueHasOrder 0;
knora-base:valueHasString "1";
knora-base:hasPermissions "CR knora-admin:Creator|M knora-admin:ProjectMember|V knora-admin:KnownUser|RV knora-admin:UnknownUser";
knora-base:attachedToUser <http://rdfh.ch/users/BhkfBc3hTeS_IDo-JgXRbQ> .
38 changes: 38 additions & 0 deletions test_data/ontologies/freetest-onto.ttl
Expand Up @@ -58,6 +58,22 @@
"max=-1" .


:hasIntegerProperty rdf:type owl:ObjectProperty ;

rdfs:subPropertyOf knora-base:hasValue ;

rdfs:label "Ganzzahl"@de ,
"Nombre entier"@fr ,
"Intero"@it ,
"Integer"@en ;

knora-base:objectClassConstraint knora-base:IntValue ;

salsah-gui:guiElement salsah-gui:Spinbox ;

salsah-gui:guiAttribute "min=0" ,
"max=-1" .


:hasDecimal rdf:type owl:ObjectProperty ;

Expand Down Expand Up @@ -121,6 +137,12 @@
owl:onProperty :hasInteger ;
owl:minCardinality "0"^^xsd:nonNegativeInteger ;
salsah-gui:guiOrder "4"^^xsd:nonNegativeInteger
] ,
[
rdf:type owl:Restriction ;
owl:onProperty :hasIntegerProperty ;
owl:minCardinality "0"^^xsd:nonNegativeInteger ;
salsah-gui:guiOrder "5"^^xsd:nonNegativeInteger
] ;

knora-base:resourceIcon "thing.png" ;
Expand All @@ -141,3 +163,19 @@
"SFT en"@en ;

rdfs:comment """A comment for SFT."""@de .


:FreeTestResourceClass rdf:type owl:Class ;
rdfs:subClassOf knora-base:Resource ,
[
rdf:type owl:Restriction ;
owl:onProperty :hasIntegerProperty ;
owl:minCardinality "0"^^xsd:nonNegativeInteger ;
salsah-gui:guiOrder "1"^^xsd:nonNegativeInteger
] ;
rdfs:label "FTRC de"@de ,
"FTRC fr"@fr ,
"FTRC it"@it ,
"FTRC en"@en ;

rdfs:comment """A comment for FTRC."""@de .
2 changes: 2 additions & 0 deletions webapi/scripts/expected-client-test-data.txt
Expand Up @@ -171,6 +171,8 @@ test-data/v2/ontologies/anything-ontology.json
test-data/v2/ontologies/can-do-response.json
test-data/v2/ontologies/candeletecardinalities-false-request.json
test-data/v2/ontologies/candeletecardinalities-false-response.json
test-data/v2/ontologies/candeletecardinalities-true-if-not-used-in-this-class-request.json
test-data/v2/ontologies/candeletecardinalities-true-if-not-used-in-this-class-response.json
test-data/v2/ontologies/candeletecardinalities-true-request.json
test-data/v2/ontologies/candeletecardinalities-true-response.json
test-data/v2/ontologies/change-class-comment-request.json
Expand Down
Expand Up @@ -1786,7 +1786,7 @@ class OntologyResponderV2(responderData: ResponderData) extends Responder(respon
* Checks if cardinalities can be removed from a class.
*
* @param canDeleteCardinalitiesFromClassRequest the request to remove cardinalities.
* @return a [[ReadOntologyV2]] in the internal schema, containing the new class definition.
* @return a [[CanDoResponseV2]] indicating whether a class's cardinalities can be deleted.
*/
private def canDeleteCardinalitiesFromClass(
canDeleteCardinalitiesFromClassRequest: CanDeleteCardinalitiesFromClassRequestV2
Expand Down
Expand Up @@ -110,10 +110,15 @@ object Cardinalities {
isCardinalityDefinedOnClass(cacheData, p._1, p._2, internalClassIri, internalOntologyIri)
)

// Check if property is used in resources
// Check if property is used in resources of this class

submittedPropertyToDelete: SmartIri = cardinalitiesToDelete.head._1
propertyIsUsed: Boolean <- isPropertyUsedInResources(settings, storeManager, submittedPropertyToDelete)
propertyIsUsed: Boolean <- isPropertyUsedInResources(
settings,
storeManager,
internalClassIri,
submittedPropertyToDelete
)

// Make an update class definition in which the cardinality to delete is removed

Expand Down Expand Up @@ -249,10 +254,15 @@ object Cardinalities {
isCardinalityDefinedOnClass(cacheData, p._1, p._2, internalClassIri, internalOntologyIri)
)

// Check if property is used in resources
// Check if property is used in resources of this class

submittedPropertyToDelete: SmartIri = cardinalitiesToDelete.head._1
propertyIsUsed: Boolean <- isPropertyUsedInResources(settings, storeManager, submittedPropertyToDelete)
propertyIsUsed: Boolean <- isPropertyUsedInResources(
settings,
storeManager,
internalClassIri,
submittedPropertyToDelete
)
_ = if (propertyIsUsed) {
throw BadRequestException("Property is used in data. The cardinality cannot be deleted.")
}
Expand Down Expand Up @@ -429,6 +439,7 @@ object Cardinalities {
def isPropertyUsedInResources(
settings: KnoraSettingsImpl,
storeManager: ActorRef,
internalClassIri: SmartIri,
internalPropertyIri: SmartIri
)(implicit ec: ExecutionContext, timeout: Timeout): Future[Boolean] =
for {
Expand All @@ -437,6 +448,7 @@ object Cardinalities {
.isPropertyUsed(
triplestore = settings.triplestoreType,
internalPropertyIri = internalPropertyIri.toString,
internalClassIri = internalClassIri.toString,
ignoreKnoraConstraints = true,
ignoreRdfSubjectAndObject = true
)
Expand Down
Expand Up @@ -17,6 +17,7 @@
*@
@(triplestore: String,
internalPropertyIri: IRI,
internalClassIri: IRI,
ignoreKnoraConstraints: Boolean = false,
ignoreRdfSubjectAndObject: Boolean = false)

Expand All @@ -33,6 +34,18 @@ ASK
}
WHERE {
BIND(IRI("@internalPropertyIri") AS ?property)
BIND(IRI("@internalClassIri") AS ?classIri)

?s ?property ?o .
{
# select all items of type classIri with property ?property.
?s ?property ?o .
?s rdf:type ?classIri .
}

UNION {
# select all items belonging to a subclass of classIri and with property ?property.
?s ?property ?o .
?s rdf:type ?class .
?class rdfs:subClassOf* ?classIri .
}
}
16 changes: 12 additions & 4 deletions webapi/src/test/scala/org/knora/webapi/e2e/v2/OntologyModels.scala
Expand Up @@ -80,13 +80,23 @@ object CreatePropertyRequest {
ontologyName: String,
lastModificationDate: Instant,
propertyName: String,
subjectClassName: String,
subjectClassName: Option[String],
propertyType: PropertyValueType,
label: LangString,
comment: LangString
): CreatePropertyRequest = {
val LocalHost_Ontology = "http://0.0.0.0:3333/ontology"
val ontologyId = LocalHost_Ontology + s"/0001/$ontologyName/v2"

val optionalSubjectClass = subjectClassName match {
case Some(subjectName) => s"""
|"knora-api:subjectType" : {
| "@id" : "$ontologyName:$subjectName"
| },
|""".stripMargin
case None => ""
}

val value = s"""{
| "@id" : "$ontologyId",
| "@type" : "owl:Ontology",
Expand All @@ -97,9 +107,7 @@ object CreatePropertyRequest {
| "@graph" : [ {
| "@id" : "$ontologyName:$propertyName",
| "@type" : "owl:ObjectProperty",
| "knora-api:subjectType" : {
| "@id" : "$ontologyName:$subjectClassName"
| },
| $optionalSubjectClass
| "knora-api:objectType" : {
| "@id" : "${propertyType.value}"
| },
Expand Down

0 comments on commit 298ba47

Please sign in to comment.