Skip to content

Commit 55b5d4b

Browse files
author
irinaschubert
authored
fix(candeletecardinalities): return canDoResponse of false instead of throwing an exception for inherited cardinalities (DEV-314) (#1966)
* return false instead of throwing exception * add tests * add missing dependencies after reformatting * Update doc * check for usage of inherited properties * remove code smells * Improve code after review * fix failing tests
1 parent 2cebac7 commit 55b5d4b

File tree

2 files changed

+399
-107
lines changed

2 files changed

+399
-107
lines changed

webapi/src/main/scala/org/knora/webapi/responders/v2/ontology/Cardinalities.scala

Lines changed: 75 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ object Cardinalities {
5252
)(implicit ec: ExecutionContext, stringFormatter: StringFormatter, timeout: Timeout): Future[CanDoResponseV2] = {
5353
for {
5454
cacheData: Cache.OntologyCacheData <- Cache.getCacheData
55-
ontology = cacheData.ontologies(internalOntologyIri)
5655

5756
submittedClassDefinition: ClassInfoContentV2 =
5857
deleteCardinalitiesFromClassRequest.classInfoContent.toOntologySchema(InternalSchema)
@@ -106,9 +105,20 @@ object Cardinalities {
106105
cardinalitiesToDelete: Map[SmartIri, Cardinality.KnoraCardinalityInfo] =
107106
deleteCardinalitiesFromClassRequest.classInfoContent.toOntologySchema(InternalSchema).directCardinalities
108107

109-
_ = cardinalitiesToDelete.foreach(p =>
110-
isCardinalityDefinedOnClass(cacheData, p._1, p._2, internalClassIri, internalOntologyIri)
111-
)
108+
isDefinedOnClassList: List[Boolean] <- Future
109+
.sequence(cardinalitiesToDelete.map { p =>
110+
for {
111+
isDefined: Boolean <- isCardinalityDefinedOnClass(
112+
cacheData,
113+
p._1,
114+
p._2,
115+
internalClassIri,
116+
internalOntologyIri
117+
)
118+
} yield isDefined
119+
}.toList)
120+
121+
atLeastOneCardinalityNotDefinedOnClass: Boolean = isDefinedOnClassList.contains(false)
112122

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

@@ -176,7 +186,9 @@ object Cardinalities {
176186
throw BadRequestException(msg)
177187
}
178188
)
179-
} yield CanDoResponseV2(!propertyIsUsed)
189+
190+
// response is true only when property is not used in data and cardinality is defined directly on that class
191+
} yield CanDoResponseV2(!propertyIsUsed && !atLeastOneCardinalityNotDefinedOnClass)
180192
}
181193

182194
/**
@@ -250,9 +262,24 @@ object Cardinalities {
250262
cardinalitiesToDelete: Map[SmartIri, Cardinality.KnoraCardinalityInfo] =
251263
deleteCardinalitiesFromClassRequest.classInfoContent.toOntologySchema(InternalSchema).directCardinalities
252264

253-
_ = cardinalitiesToDelete.foreach(p =>
254-
isCardinalityDefinedOnClass(cacheData, p._1, p._2, internalClassIri, internalOntologyIri)
255-
)
265+
isDefinedOnClassList: List[Boolean] <- Future
266+
.sequence(cardinalitiesToDelete.map { p =>
267+
for {
268+
isDefined: Boolean <- isCardinalityDefinedOnClass(
269+
cacheData,
270+
p._1,
271+
p._2,
272+
internalClassIri,
273+
internalOntologyIri
274+
)
275+
} yield isDefined
276+
}.toList)
277+
278+
_ = if (isDefinedOnClassList.contains(false)) {
279+
throw BadRequestException(
280+
"The cardinality is not defined directly on the class and cannot be deleted."
281+
)
282+
}
256283

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

@@ -484,25 +511,6 @@ object Cardinalities {
484511
.entityInfoContent
485512
} yield currentClassDefinition
486513

487-
/**
488-
* Checks if the class is a subclass of `knora-base:Resource`.
489-
*
490-
* @param submittedClassInfoContentV2 the class to check
491-
* @return `true` if the class is a subclass of `knora-base:Resource`, otherwise throws an exception.
492-
*/
493-
def isKnoraResourceClass(
494-
submittedClassInfoContentV2: ClassInfoContentV2
495-
)(implicit ec: ExecutionContext, stringFormatter: StringFormatter): Future[Boolean] =
496-
if (submittedClassInfoContentV2.subClassOf.contains(KnoraBase.Resource.toSmartIri)) {
497-
FastFuture.successful(true)
498-
} else {
499-
FastFuture.failed(
500-
throw BadRequestException(
501-
s"Class ${submittedClassInfoContentV2.classIri} is not a subclass of ${KnoraBase.Resource.toSmartIri}. $submittedClassInfoContentV2"
502-
)
503-
)
504-
}
505-
506514
/**
507515
* Check if the cardinality for a property is defined on a class.
508516
*
@@ -511,7 +519,7 @@ object Cardinalities {
511519
* @param cardinalityInfo the cardinality that should be defined for the property.
512520
* @param internalClassIri the class we are checking against.
513521
* @param internalOntologyIri the ontology containing the class.
514-
* @return `true` if the cardinality is defined on the class, otherwise throws an exception.
522+
* @return `true` if the cardinality is defined on the class, `false` otherwise
515523
*/
516524
def isCardinalityDefinedOnClass(
517525
cacheData: Cache.OntologyCacheData,
@@ -521,30 +529,58 @@ object Cardinalities {
521529
internalOntologyIri: SmartIri
522530
)(implicit ec: ExecutionContext): Future[Boolean] = {
523531
val currentOntologyState: ReadOntologyV2 = cacheData.ontologies(internalOntologyIri)
524-
val currentClassState: ClassInfoContentV2 = currentOntologyState.classes
532+
533+
val readClassInfo: ReadClassInfoV2 = currentOntologyState.classes
525534
.getOrElse(
526535
internalClassIri,
527536
throw BadRequestException(
528-
s"Class ${internalClassIri} does not exist"
537+
s"Class $internalClassIri does not exist"
529538
)
530539
)
531-
.entityInfoContent
532-
val existingCardinality = currentClassState.directCardinalities
533-
.getOrElse(
534-
propertyIri,
540+
541+
// if cardinality is inherited, it's not directly defined on that class
542+
if (readClassInfo.inheritedCardinalities.keySet.contains(propertyIri)) {
543+
return FastFuture.successful(false)
544+
}
545+
546+
val currentClassState: ClassInfoContentV2 = readClassInfo.entityInfoContent
547+
val existingCardinality = currentClassState.directCardinalities.get(propertyIri)
548+
existingCardinality match {
549+
case Some(cardinality) =>
550+
if (cardinality.cardinality.equals(cardinalityInfo.cardinality)) {
551+
FastFuture.successful(true)
552+
} else {
553+
FastFuture.failed(
554+
throw BadRequestException(
555+
s"Submitted cardinality for property $propertyIri does not match existing cardinality."
556+
)
557+
)
558+
}
559+
case None =>
535560
throw BadRequestException(
536-
s"Cardinality for property ${propertyIri} is not defined."
561+
s"Submitted cardinality for property $propertyIri is not defined for class $internalClassIri."
537562
)
538-
)
539-
if (existingCardinality.cardinality.equals(cardinalityInfo.cardinality)) {
563+
}
564+
565+
}
566+
567+
/**
568+
* Checks if the class is a subclass of `knora-base:Resource`.
569+
*
570+
* @param submittedClassInfoContentV2 the class to check
571+
* @return `true` if the class is a subclass of `knora-base:Resource`, otherwise throws an exception.
572+
*/
573+
def isKnoraResourceClass(
574+
submittedClassInfoContentV2: ClassInfoContentV2
575+
)(implicit ec: ExecutionContext, stringFormatter: StringFormatter): Future[Boolean] =
576+
if (submittedClassInfoContentV2.subClassOf.contains(KnoraBase.Resource.toSmartIri)) {
540577
FastFuture.successful(true)
541578
} else {
542579
FastFuture.failed(
543580
throw BadRequestException(
544-
s"Submitted cardinality for property ${propertyIri} does not match existing cardinality."
581+
s"Class ${submittedClassInfoContentV2.classIri} is not a subclass of ${KnoraBase.Resource.toSmartIri}. $submittedClassInfoContentV2"
545582
)
546583
)
547584
}
548-
}
549585

550586
}

0 commit comments

Comments
 (0)