Skip to content

Commit

Permalink
#1693 Validation-warnings as payload to caller in V3
Browse files Browse the repository at this point in the history 
 - v2 ignores this added information
 - needs
  • Loading branch information
@dk1844
dk1844 committed Apr 6, 2022
1 parent 51ad013 commit ba5edce
Show file tree
Hide file tree
Showing 13 changed files with 108 additions and 75 deletions.
2 changes: 1 addition & 1 deletion rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/DatasetController.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class DatasetController @Autowired()(datasetService: DatasetService)
latestVersion <- datasetService.getLatestVersionValue(datasetName)
res <- latestVersion match {
case Some(version) => datasetService.addConformanceRule(user.getUsername, datasetName, version, rule).map {
case Some(ds) => ds
case Some((ds, validation)) => ds // v2 disregarding validation
case _ => throw notFound()
}
case _ => throw notFound()
Expand Down
4 changes: 2 additions & 2 deletions ...api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/MappingTableController.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class MappingTableController @Autowired() (mappingTableService: MappingTableServ
@RequestBody upd: MenasObject[Array[DefaultValue]]): CompletableFuture[MappingTable] = {
mappingTableService.updateDefaults(user.getUsername, upd.id.name,
upd.id.version, upd.value.toList).map {
case Some(entity) => entity
case Some(entity) => entity._1 // v2 disregarding validation
case None => throw notFound()
}
}
Expand All @@ -51,7 +51,7 @@ class MappingTableController @Autowired() (mappingTableService: MappingTableServ
@RequestBody newDefault: MenasObject[DefaultValue]): CompletableFuture[MappingTable] = {
mappingTableService.addDefault(user.getUsername, newDefault.id.name,
newDefault.id.version, newDefault.value).map {
case Some(entity) => entity
case Some(entity) => entity._1 // v2 disregarding validation
case None => throw notFound()
}
}
Expand Down
2 changes: 1 addition & 1 deletion rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/SchemaController.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ class SchemaController @Autowired()(
// the parsing of sparkStruct can fail, therefore we try to save it first before saving the attachment
update <- schemaService.schemaUpload(username, menasAttachment.refName, menasAttachment.refVersion - 1, sparkStruct)
_ <- attachmentService.uploadAttachment(menasAttachment)
} yield update
} yield update.map(_._1) // v2 disregarding the validation
} catch {
case e: SchemaParsingException => throw e.copy(schemaType = schemaType) //adding schema type
}
Expand Down
6 changes: 3 additions & 3 deletions ...i/src/main/scala/za/co/absa/enceladus/rest_api/controllers/VersionedModelController.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ abstract class VersionedModelController[C <: VersionedModel with Product with Au
@ResponseStatus(HttpStatus.CREATED)
def importSingleEntity(@AuthenticationPrincipal principal: UserDetails,
@RequestBody importObject: ExportableObject[C]): CompletableFuture[C] = {
versionedModelService.importSingleItem(importObject.item, principal.getUsername, importObject.metadata).map {
versionedModelService.importSingleItemV2(importObject.item, principal.getUsername, importObject.metadata).map {
case Some(entity) => entity
case None => throw notFound()
}
Expand All @@ -131,7 +131,7 @@ abstract class VersionedModelController[C <: VersionedModel with Product with Au
versionedModelService.create(item, principal.getUsername)
}
}.map {
case Some(entity) => entity
case Some((entity, validation)) => entity // v2 does not support validation-warnings on create
case None => throw notFound()
}
}
Expand All @@ -141,7 +141,7 @@ abstract class VersionedModelController[C <: VersionedModel with Product with Au
def edit(@AuthenticationPrincipal user: UserDetails,
@RequestBody item: C): CompletableFuture[C] = {
versionedModelService.update(user.getUsername, item).map {
case Some(entity) => entity
case Some((entity, validation)) => entity // v2 disregarding validation on edit
case None => throw notFound()
}
}
Expand Down
17 changes: 9 additions & 8 deletions ...api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/v3/DatasetControllerV3.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import org.springframework.http.{HttpStatus, ResponseEntity}
import org.springframework.security.core.annotation.AuthenticationPrincipal
import org.springframework.security.core.userdetails.UserDetails
import org.springframework.web.bind.annotation._
import za.co.absa.enceladus.model.Dataset
import za.co.absa.enceladus.model.Validation
import za.co.absa.enceladus.model.conformanceRule.ConformanceRule
import za.co.absa.enceladus.rest_api.services.v3.DatasetServiceV3
import za.co.absa.enceladus.rest_api.utils.implicits._
Expand Down Expand Up @@ -50,13 +50,14 @@ class DatasetControllerV3 @Autowired()(datasetService: DatasetServiceV3)
@PathVariable name: String,
@PathVariable version: String,
@RequestBody newProperties: java.util.Map[String, String],
request: HttpServletRequest): CompletableFuture[ResponseEntity[Nothing]] = {
request: HttpServletRequest): CompletableFuture[ResponseEntity[Validation]] = {
forVersionExpression(name, version) { case (dsName, dsVersion) =>
datasetService.updateProperties(principal.getUsername, dsName, dsVersion, newProperties.toScalaMap).map {

case Some(entity) =>
case Some((entity, validation)) =>
// stripping last 3 segments (/dsName/dsVersion/properties), instead of /api-v3/dastasets/dsName/dsVersion/properties we want /api-v3/dastasets/dsName/dsVersion/properties
createdWithNameVersionLocation(entity.name, entity.version, request, stripLastSegments = 3, suffix = "/properties")
createdWithNameVersionLocationBuilder(entity.name, entity.version, request, stripLastSegments = 3, suffix = "/properties")
.body(validation) // todo include in tests
case None => throw notFound()
}
}
Expand All @@ -80,13 +81,13 @@ class DatasetControllerV3 @Autowired()(datasetService: DatasetServiceV3)
@PathVariable name: String,
@PathVariable version: String,
@RequestBody rule: ConformanceRule,
request: HttpServletRequest): CompletableFuture[ResponseEntity[Nothing]] = {
request: HttpServletRequest): CompletableFuture[ResponseEntity[Validation]] = {
forVersionExpression(name, version)(datasetService.getVersion).flatMap {
case Some(entity) => datasetService.addConformanceRule(user.getUsername, name, entity.version, rule).map {
case Some(updatedDs) =>
case Some((updatedDs, validation)) =>
val addedRuleOrder = updatedDs.conformance.last.order
createdWithNameVersionLocation(name, updatedDs.version, request, stripLastSegments = 3, // strip: /{name}/{version}/rules
suffix = s"/rules/$addedRuleOrder")
createdWithNameVersionLocationBuilder(name, updatedDs.version, request, stripLastSegments = 3, // strip: /{name}/{version}/rules
suffix = s"/rules/$addedRuleOrder").body(validation)
case _ => throw notFound()
}
case None => throw notFound()
Expand Down
23 changes: 12 additions & 11 deletions .../main/scala/za/co/absa/enceladus/rest_api/controllers/v3/VersionedModelControllerV3.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,14 @@ abstract class VersionedModelControllerV3[C <: VersionedModel with Product
def importSingleEntity(@AuthenticationPrincipal principal: UserDetails,
@PathVariable name: String,
@RequestBody importObject: ExportableObject[C],
request: HttpServletRequest): CompletableFuture[ResponseEntity[Nothing]] = {
request: HttpServletRequest): CompletableFuture[ResponseEntity[Validation]] = {
if (name != importObject.item.name) {
Future.failed(new IllegalArgumentException(s"URL and payload entity name mismatch: '$name' != '${importObject.item.name}'"))
} else {
versionedModelService.importSingleItem(importObject.item, principal.getUsername, importObject.metadata).map {
case Some(entity) =>
versionedModelService.importSingleItemV3(importObject.item, principal.getUsername, importObject.metadata).map {
case Some((entity, validation)) =>
// stripping two last segments, instead of /api-v3/dastasets/dsName/import + /dsName/dsVersion we want /api-v3/dastasets + /dsName/dsVersion
createdWithNameVersionLocation(entity.name, entity.version, request, stripLastSegments = 2)
createdWithNameVersionLocationBuilder(entity.name, entity.version, request, stripLastSegments = 2).body(validation)
case None => throw notFound()
}
}
Expand All @@ -124,15 +124,15 @@ abstract class VersionedModelControllerV3[C <: VersionedModel with Product
@ResponseStatus(HttpStatus.CREATED)
def create(@AuthenticationPrincipal principal: UserDetails,
@RequestBody item: C,
request: HttpServletRequest): CompletableFuture[ResponseEntity[Nothing]] = {
request: HttpServletRequest): CompletableFuture[ResponseEntity[Validation]] = {
versionedModelService.isDisabled(item.name).flatMap { isDisabled =>
if (isDisabled) {
versionedModelService.recreate(principal.getUsername, item)
} else {
versionedModelService.create(item, principal.getUsername)
}
}.map {
case Some(entity) => createdWithNameVersionLocation(entity.name, entity.version, request)
case Some((entity, validation)) => createdWithNameVersionLocationBuilder(entity.name, entity.version, request).body(validation)
case None => throw notFound()
}
}
Expand All @@ -143,15 +143,16 @@ abstract class VersionedModelControllerV3[C <: VersionedModel with Product
@PathVariable name: String,
@PathVariable version: Int,
@RequestBody item: C,
request: HttpServletRequest): CompletableFuture[ResponseEntity[Nothing]] = {
request: HttpServletRequest): CompletableFuture[ResponseEntity[Validation]] = {

if (name != item.name) {
Future.failed(new IllegalArgumentException(s"URL and payload entity name mismatch: '$name' != '${item.name}'"))
} else if (version != item.version) {
Future.failed(new IllegalArgumentException(s"URL and payload version mismatch: ${version} != ${item.version}"))
} else {
versionedModelService.update(user.getUsername, item).map {
case Some(entity) => createdWithNameVersionLocation(entity.name, entity.version, request, stripLastSegments = 2)
case Some((updatedEntity, validation)) =>
createdWithNameVersionLocationBuilder(updatedEntity.name, updatedEntity.version, request, stripLastSegments = 2).body(validation)
case None => throw notFound()
}
}
Expand Down Expand Up @@ -195,8 +196,8 @@ abstract class VersionedModelControllerV3[C <: VersionedModel with Product
}
}

protected def createdWithNameVersionLocation(name: String, version: Int, request: HttpServletRequest,
stripLastSegments: Int = 0, suffix: String = ""): ResponseEntity[Nothing] = {
protected def createdWithNameVersionLocationBuilder(name: String, version: Int, request: HttpServletRequest,
stripLastSegments: Int = 0, suffix: String = ""): ResponseEntity.BodyBuilder = {
val strippingPrefix = Range(0, stripLastSegments).map(_ => "/..").mkString

val location: URI = ServletUriComponentsBuilder.fromRequest(request)
Expand All @@ -207,7 +208,7 @@ abstract class VersionedModelControllerV3[C <: VersionedModel with Product

// hint on "/.." + normalize https://github.com/spring-projects/spring-framework/issues/14905#issuecomment-453400918

ResponseEntity.created(location).build()
ResponseEntity.created(location)
}

}
Expand Down
17 changes: 9 additions & 8 deletions rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/DatasetService.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class DatasetService @Autowired()(datasetMongoRepository: DatasetMongoRepository

import scala.concurrent.ExecutionContext.Implicits.global

override def update(username: String, dataset: Dataset): Future[Option[Dataset]] = {
override def update(username: String, dataset: Dataset): Future[Option[(Dataset, Validation)]] = {
super.updateFuture(username, dataset.name, dataset.version) { latest =>
updateSchedule(dataset, latest).map({ withSchedule =>
withSchedule
Expand Down Expand Up @@ -105,7 +105,7 @@ class DatasetService @Autowired()(datasetMongoRepository: DatasetMongoRepository
}
}

override def create(newDataset: Dataset, username: String): Future[Option[Dataset]] = {
override def create(newDataset: Dataset, username: String): Future[Option[(Dataset, Validation)]] = {
val dataset = Dataset(name = newDataset.name,
description = newDataset.description,
hdfsPath = newDataset.hdfsPath,
Expand All @@ -117,18 +117,19 @@ class DatasetService @Autowired()(datasetMongoRepository: DatasetMongoRepository
super.create(dataset, username)
}

def addConformanceRule(username: String, datasetName: String, datasetVersion: Int, rule: ConformanceRule): Future[Option[Dataset]] = {
def addConformanceRule(username: String, datasetName: String, datasetVersion: Int,
rule: ConformanceRule): Future[Option[(Dataset, Validation)]] = {
update(username, datasetName, datasetVersion) { dataset =>
dataset.copy(conformance = dataset.conformance :+ rule)
}
}

def updateProperties(username: String, datasetName: String, datasetVersion: Int,
updatedProperties: Map[String, String]): Future[Option[Dataset]] = {
updatedProperties: Map[String, String]): Future[Option[(Dataset, Validation)]] = {
for {
s <- validateProperties(updatedProperties).flatMap {
successfulValidation <- validateProperties(updatedProperties).flatMap {
case validation if !validation.isValid => Future.failed(ValidationException(validation)) // warnings are ok for update
case _ => Future.successful(()) // todo perhaps communicate warnings as result?
case validation => Future.successful(validation) // empty or with warnings
}

// updateFuture includes latest-check and version increase
Expand All @@ -146,7 +147,7 @@ class DatasetService @Autowired()(datasetMongoRepository: DatasetMongoRepository
update <- update(username, datasetName, latestVersion) { latest =>
latest.copy(properties = removeBlankProperties(updatedProperties))
}
} yield update
} yield update.map(_._1) // v2 does not expect validation on update
}

private def validateExistingProperty(key: String, value: String,
Expand Down Expand Up @@ -244,7 +245,7 @@ class DatasetService @Autowired()(datasetMongoRepository: DatasetMongoRepository
def getLatestVersions(missingProperty: Option[String]): Future[Seq[Dataset]] =
datasetMongoRepository.getLatestVersions(missingProperty)

override def importItem(item: Dataset, username: String): Future[Option[Dataset]] = {
override def importItem(item: Dataset, username: String): Future[Option[(Dataset, Validation)]] = {
getLatestVersionValue(item.name).flatMap {
case Some(version) => update(username, item.copy(version = version))
case None => super.create(item.copy(version = 1), username)
Expand Down
10 changes: 5 additions & 5 deletions rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/MappingTableService.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class MappingTableService @Autowired() (mappingTableMongoRepository: MappingTabl
used.map(refs => UsedIn(Some(refs), None))
}

override def create(mt: MappingTable, username: String): Future[Option[MappingTable]] = {
override def create(mt: MappingTable, username: String): Future[Option[(MappingTable, Validation)]] = {
val mappingTable = MappingTable(name = mt.name,
description = mt.description,
schemaName = mt.schemaName,
Expand All @@ -48,19 +48,19 @@ class MappingTableService @Autowired() (mappingTableMongoRepository: MappingTabl
super.create(mappingTable, username)
}

def updateDefaults(username: String, mtName: String, mtVersion: Int, defaultValues: List[DefaultValue]): Future[Option[MappingTable]] = {
def updateDefaults(username: String, mtName: String, mtVersion: Int, defaultValues: List[DefaultValue]): Future[Option[(MappingTable, Validation)]] = {
super.update(username, mtName, mtVersion) { latest =>
latest.setDefaultMappingValue(defaultValues)
}
}

def addDefault(username: String, mtName: String, mtVersion: Int, defaultValue: DefaultValue): Future[Option[MappingTable]] = {
def addDefault(username: String, mtName: String, mtVersion: Int, defaultValue: DefaultValue): Future[Option[(MappingTable, Validation)]] = {
super.update(username, mtName, mtVersion) { latest =>
latest.setDefaultMappingValue(latest.defaultMappingValue :+ defaultValue)
}
}

override def update(username: String, mt: MappingTable): Future[Option[MappingTable]] = {
override def update(username: String, mt: MappingTable): Future[Option[(MappingTable, Validation)]] = {
super.update(username, mt.name, mt.version) { latest =>
latest
.setHDFSPath(mt.hdfsPath)
Expand All @@ -71,7 +71,7 @@ class MappingTableService @Autowired() (mappingTableMongoRepository: MappingTabl
}
}

override def importItem(item: MappingTable, username: String): Future[Option[MappingTable]] = {
override def importItem(item: MappingTable, username: String): Future[Option[(MappingTable, Validation)]] = {
getLatestVersionValue(item.name).flatMap {
case Some(version) => update(username, item.copy(version = version))
case None => super.create(item.copy(version = 1), username)
Expand Down
8 changes: 4 additions & 4 deletions ...api/src/main/scala/za/co/absa/enceladus/rest_api/services/PropertyDefinitionService.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ package za.co.absa.enceladus.rest_api.services
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.stereotype.Service
import za.co.absa.enceladus.rest_api.repositories.PropertyDefinitionMongoRepository
import za.co.absa.enceladus.model.UsedIn
import za.co.absa.enceladus.model.{UsedIn, Validation}
import za.co.absa.enceladus.model.properties.PropertyDefinition

import scala.concurrent.Future
Expand All @@ -31,7 +31,7 @@ class PropertyDefinitionService @Autowired()(propertyDefMongoRepository: Propert

override def getUsedIn(name: String, version: Option[Int]): Future[UsedIn] = Future.successful(UsedIn())

override def update(username: String, propertyDef: PropertyDefinition): Future[Option[PropertyDefinition]] = {
override def update(username: String, propertyDef: PropertyDefinition): Future[Option[(PropertyDefinition, Validation)]] = {
super.update(username, propertyDef.name, propertyDef.version) { latest =>
latest
.setPropertyType(propertyDef.propertyType)
Expand All @@ -45,7 +45,7 @@ class PropertyDefinitionService @Autowired()(propertyDefMongoRepository: Propert
propertyDefMongoRepository.distinctCount()
}

override def create(newPropertyDef: PropertyDefinition, username: String): Future[Option[PropertyDefinition]] = {
override def create(newPropertyDef: PropertyDefinition, username: String): Future[Option[(PropertyDefinition, Validation)]] = {
val propertyDefBase = PropertyDefinition(
name = newPropertyDef.name,
description = newPropertyDef.description,
Expand All @@ -63,7 +63,7 @@ class PropertyDefinitionService @Autowired()(propertyDefMongoRepository: Propert
super.create(propertyDefinition, username)
}

override private[services] def importItem(item: PropertyDefinition, username: String): Future[Option[PropertyDefinition]] = {
override private[services] def importItem(item: PropertyDefinition, username: String): Future[Option[(PropertyDefinition, Validation)]] = {
getLatestVersionValue(item.name).flatMap {
case Some(version) => update(username, item.copy(version = version))
case None => super.create(item.copy(version = 1), username)
Expand Down
Loading

0 comments on commit ba5edce

Please sign in to comment.