Skip to content

Commit

Permalink
Don't allow class with cardinalities on P and on a subproperty of P (#…
Browse files Browse the repository at this point in the history
…982)

* fix (OntologyResponderV2): Don't allow a class to have a cardinality on P and on a subproperty of P.

* docs (knora-ontologies): Update docs and release notes.
  • Loading branch information
Benjamin Geer authored Aug 29, 2018
1 parent b12b83e commit 098a0fe
Show file tree
Hide file tree
Showing 9 changed files with 253 additions and 47 deletions.
2 changes: 2 additions & 0 deletions docs/src/paradox/00-release-notes/v1.8.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ See the
## New features:

## Bugfixes:

- trouble with xml-checker and/or consistency-checker during bulk import (@github[#978](#978))
2 changes: 2 additions & 0 deletions docs/src/paradox/02-knora-ontologies/knora-base.md
Original file line number Diff line number Diff line change
Expand Up @@ -1475,6 +1475,8 @@ this simplified example:
- If it's a standoff class, none of its cardinalities may be on Knora
resource properties, and all its base classes with Knora IRIs must
also be standoff classes.
- A class cannot have a cardinality on property P as well as a cardinality
on a subproperty of P.

### Restrictions on properties

Expand Down
26 changes: 13 additions & 13 deletions webapi/_test_data/ontologies/anything-onto.ttl
Original file line number Diff line number Diff line change
Expand Up @@ -505,18 +505,6 @@
owl:onProperty :hasColor ;
owl:minCardinality "0"^^xsd:nonNegativeInteger ;
salsah-gui:guiOrder "10"^^xsd:nonNegativeInteger
] ,
[
rdf:type owl:Restriction ;
owl:onProperty :hasBlueThing ;
owl:minCardinality "0"^^xsd:nonNegativeInteger ;
salsah-gui:guiOrder "63"^^xsd:nonNegativeInteger
] ,
[
rdf:type owl:Restriction ;
owl:onProperty :hasBlueThingValue ;
owl:minCardinality "0"^^xsd:nonNegativeInteger ;
salsah-gui:guiOrder "16"^^xsd:nonNegativeInteger
] ;
#[
# rdf:type owl:Restriction ;
Expand Down Expand Up @@ -545,7 +533,19 @@

:BlueThing rdf:type owl:Class ;

rdfs:subClassOf :Thing ;
rdfs:subClassOf :Thing ,
[
rdf:type owl:Restriction ;
owl:onProperty :hasBlueThing ;
owl:minCardinality "0"^^xsd:nonNegativeInteger ;
salsah-gui:guiOrder "63"^^xsd:nonNegativeInteger
] ,
[
rdf:type owl:Restriction ;
owl:onProperty :hasBlueThingValue ;
owl:minCardinality "0"^^xsd:nonNegativeInteger ;
salsah-gui:guiOrder "16"^^xsd:nonNegativeInteger
] ;

rdfs:comment """Diese Resource-Klasse beschreibt ein blaues Ding"""@de ;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
@prefix xml: <http://www.w3.org/XML/1998/namespace> .
@prefix xsd: <http://www.w3.org/2001/XMLSchema#> .
@prefix rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#> .
@prefix rdfs: <http://www.w3.org/2000/01/rdf-schema#> .
@prefix owl: <http://www.w3.org/2002/07/owl#> .
@prefix knora-base: <http://www.knora.org/ontology/knora-base#> .
@prefix salsah-gui: <http://www.knora.org/ontology/salsah-gui#> .
@base <http://www.knora.org/ontology/invalid> .

# An ontology containing a class that inherits a property from a base class
# and a subproperty from another base class.

@prefix : <http://www.knora.org/ontology/invalid#> .
<http://www.knora.org/ontology/invalid> rdf:type owl:Ontology ;
rdfs:label "The invalid ontology" ;
knora-base:attachedToProject <http://rdfh.ch/projects/0001> .


:hasAuthor rdf:type owl:ObjectProperty ;

rdfs:subPropertyOf knora-base:hasValue ;

rdfs:label "has author"@en ;

knora-base:objectClassConstraint knora-base:TextValue .


:hasPoet rdf:type owl:ObjectProperty ;

rdfs:subPropertyOf :hasAuthor ;

rdfs:label "has poet"@en ;

knora-base:objectClassConstraint knora-base:TextValue .


:Text rdf:type owl:Class ;

rdfs:subClassOf knora-base:Resource ,
[
rdf:type owl:Restriction ;
owl:onProperty :hasAuthor ;
owl:cardinality "1"^^xsd:nonNegativeInteger ;
salsah-gui:guiOrder "1"^^xsd:nonNegativeInteger
] ;

knora-base:resourceIcon "thing.png" ;

rdfs:label "text"@en .


:Poem rdf:type owl:Class ;

rdfs:subClassOf knora-base:Resource ,
[
rdf:type owl:Restriction ;
owl:onProperty :hasPoet ;
owl:minCardinality "0"^^xsd:nonNegativeInteger ;
salsah-gui:guiOrder "2"^^xsd:nonNegativeInteger
] ;

knora-base:resourceIcon "thing.png" ;

rdfs:label "poem"@en .


:InvalidPoem rdf:type owl:Class ;

rdfs:subClassOf :Text, :Poem ;

knora-base:resourceIcon "thing.png" ;

rdfs:label "invalid poem"@en .
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ class OntologyResponderV2 extends Responder {
classCardinalitiesWithInheritance = classCardinalitiesWithInheritance,
directSubClassOfRelations = directSubClassOfRelations,
allSubClassOfRelations = allSubClassOfRelations,
allSubPropertyOfRelations = allSubPropertyOfRelations,
allPropertyDefs = allPropertyDefs,
allKnoraResourceProps = allKnoraResourceProps,
allLinkProps = allLinkProps,
Expand Down Expand Up @@ -497,6 +498,7 @@ class OntologyResponderV2 extends Responder {
* [[OwlCardinalityInfo]] objects.
* @param directSubClassOfRelations a map of class IRIs to their immediate base classes.
* @param allSubClassOfRelations a map of class IRIs to all their base classes.
* @param allSubPropertyOfRelations a map of property IRIs to all their base properties.
* @param allPropertyDefs a map of property IRIs to property definitions.
* @param allKnoraResourceProps a set of the IRIs of all Knora resource properties.
* @param allLinkProps a set of the IRIs of all link properties.
Expand All @@ -509,6 +511,7 @@ class OntologyResponderV2 extends Responder {
classCardinalitiesWithInheritance: Map[SmartIri, Map[SmartIri, OwlCardinalityInfo]],
directSubClassOfRelations: Map[SmartIri, Set[SmartIri]],
allSubClassOfRelations: Map[SmartIri, Set[SmartIri]],
allSubPropertyOfRelations: Map[SmartIri, Set[SmartIri]],
allPropertyDefs: Map[SmartIri, PropertyInfoContentV2],
allKnoraResourceProps: Set[SmartIri],
allLinkProps: Set[SmartIri],
Expand Down Expand Up @@ -567,6 +570,20 @@ class OntologyResponderV2 extends Responder {
throw InconsistentTriplestoreDataException(s"Class $classIri has one or more cardinalities on undefined properties: ${cardinalitiesOnMissingProps.mkString(", ")}")
}

// It cannot have cardinalities both on property P and on a subproperty of P.

val maybePropertyAndSubproperty: Option[(SmartIri, SmartIri)] = findPropertyAndSubproperty(
propertyIris = allPropertyIrisForCardinalitiesInClass,
subPropertyOfRelations = allSubPropertyOfRelations
)

maybePropertyAndSubproperty match {
case Some((basePropertyIri, propertyIri)) =>
throw InconsistentTriplestoreDataException(s"Class $classIri has a cardinality on property $basePropertyIri and on its subproperty $propertyIri")

case None => ()
}

if (isKnoraResourceClass) {
// If it's a resource class, all its directly defined cardinalities must be on Knora resource properties, not including knora-base:resourceProperty or knora-base:hasValue.

Expand Down Expand Up @@ -925,20 +942,20 @@ class OntologyResponderV2 extends Responder {
// No. Is it among the classes removed from knora-base to create the requested schema?
classIri.getOntologySchema.get match {
case apiV2Schema: ApiV2Schema =>
val internalClassIri = classIri.toOntologySchema(InternalSchema)
val internalClassIri = classIri.toOntologySchema(InternalSchema)

val knoraBaseClassesToRemove = apiV2Schema match {
case ApiV2Simple => KnoraApiV2Simple.KnoraBaseTransformationRules.KnoraBaseClassesToRemove
case ApiV2WithValueObjects => KnoraApiV2WithValueObjects.KnoraBaseTransformationRules.KnoraBaseClassesToRemove
}
val knoraBaseClassesToRemove = apiV2Schema match {
case ApiV2Simple => KnoraApiV2Simple.KnoraBaseTransformationRules.KnoraBaseClassesToRemove
case ApiV2WithValueObjects => KnoraApiV2WithValueObjects.KnoraBaseTransformationRules.KnoraBaseClassesToRemove
}

if (knoraBaseClassesToRemove.contains(internalClassIri)) {
// Yes. Include it in the set of unavailable classes.
acc + classIri
} else {
// No. It's available.
acc
}
if (knoraBaseClassesToRemove.contains(internalClassIri)) {
// Yes. Include it in the set of unavailable classes.
acc + classIri
} else {
// No. It's available.
acc
}

case InternalSchema => acc
}
Expand Down Expand Up @@ -1775,7 +1792,7 @@ class OntologyResponderV2 extends Responder {
internalClassDef.directCardinalities.keySet.foreach {
propertyIri =>
if (!isKnoraResourceProperty(propertyIri, cacheData) || propertyIri.toString == OntologyConstants.KnoraBase.ResourceProperty || propertyIri.toString == OntologyConstants.KnoraBase.HasValue) {
throw NotFoundException(s"Invalid property for cardinality: ${propertyIri.toOntologySchema(ApiV2WithValueObjects)}")
throw NotFoundException(s"Invalid property for cardinality: <${propertyIri.toOntologySchema(ApiV2WithValueObjects)}>")
}
}

Expand Down Expand Up @@ -1828,9 +1845,46 @@ class OntologyResponderV2 extends Responder {
errorFun = { msg: String => throw BadRequestException(msg) }
)

// It cannot have cardinalities both on property P and on a subproperty of P.

val maybePropertyAndSubproperty: Option[(SmartIri, SmartIri)] = findPropertyAndSubproperty(
propertyIris = cardinalitiesForClassWithInheritance.keySet,
subPropertyOfRelations = cacheData.subPropertyOfRelations
)

maybePropertyAndSubproperty match {
case Some((basePropertyIri, propertyIri)) =>
throw BadRequestException(s"Class <${internalClassDef.classIri.toOntologySchema(ApiV2WithValueObjects)}> has a cardinality on property <${basePropertyIri.toOntologySchema(ApiV2WithValueObjects)}> and on its subproperty <${propertyIri.toOntologySchema(ApiV2WithValueObjects)}>")

case None => ()
}

cardinalitiesForClassWithInheritance
}

/**
* Given a set of property IRIs, determines whether the set contains a property P and a subproperty of P.
*
* @param propertyIris the set of property IRIs.
* @param subPropertyOfRelations all the subproperty relations in the triplestore.
* @return a property and its subproperty, if found.
*/
private def findPropertyAndSubproperty(propertyIris: Set[SmartIri], subPropertyOfRelations: Map[SmartIri, Set[SmartIri]]): Option[(SmartIri, SmartIri)] = {
propertyIris.flatMap {
propertyIri =>
val maybeBasePropertyIri: Option[SmartIri] = (propertyIris - propertyIri).find {
otherPropertyIri =>
subPropertyOfRelations.get(propertyIri).exists {
baseProperties: Set[SmartIri] => baseProperties.contains(otherPropertyIri)
}
}

maybeBasePropertyIri.map {
basePropertyIri => (basePropertyIri, propertyIri)
}
}.headOption
}

/**
* Checks that a class is a subclass of all the classes that are subject class constraints of the Knora resource properties in its cardinalities.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,6 @@ class OntologyV2R2RSpec extends R2RSpec {
""".stripMargin

val expectedProperties: Set[SmartIri] = Set(
"http://0.0.0.0:3333/ontology/0001/anything/v2#hasBlueThing".toSmartIri,
"http://0.0.0.0:3333/ontology/0001/anything/v2#hasName".toSmartIri,
"http://0.0.0.0:3333/ontology/0001/anything/v2#hasInterval".toSmartIri,
"http://0.0.0.0:3333/ontology/0001/anything/v2#hasOtherThingValue".toSmartIri,
Expand All @@ -491,7 +490,6 @@ class OntologyV2R2RSpec extends R2RSpec {
"http://0.0.0.0:3333/ontology/0001/anything/v2#hasDecimal".toSmartIri,
"http://0.0.0.0:3333/ontology/0001/anything/v2#hasBoolean".toSmartIri,
"http://0.0.0.0:3333/ontology/0001/anything/v2#hasColor".toSmartIri,
"http://0.0.0.0:3333/ontology/0001/anything/v2#hasBlueThingValue".toSmartIri,
"http://0.0.0.0:3333/ontology/0001/anything/v2#hasRichtext".toSmartIri,
"http://0.0.0.0:3333/ontology/0001/anything/v2#hasUri".toSmartIri,
"http://0.0.0.0:3333/ontology/0001/anything/v2#hasInteger".toSmartIri,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1464,13 +1464,13 @@ class ResourcesResponderV1Spec extends CoreSpec(ResourcesResponderV1Spec.config)

}

"not create an anything:Thing with property anything:hasBlueThing pointing to an anything:Thing" in {
"not create an anything:BlueThing with property anything:hasBlueThing pointing to an anything:Thing" in {
val valuesToBeCreated = Map(
"http://www.knora.org/ontology/0001/anything#hasBlueThing" -> Vector(CreateValueV1WithComment(LinkUpdateV1(targetResourceIri = "http://rdfh.ch/0001/a-thing")))
)

actorUnderTest ! ResourceCreateRequestV1(
resourceTypeIri = "http://www.knora.org/ontology/0001/anything#Thing",
resourceTypeIri = "http://www.knora.org/ontology/0001/anything#BlueThing",
label = "Test Thing",
projectIri = "http://rdfh.ch/projects/0001",
values = valuesToBeCreated,
Expand Down
Loading

0 comments on commit 098a0fe

Please sign in to comment.