#1693 As disabled flag cases to fail validation:

- custom isDisabledCheck is not needed anymore (V3 services use `validate` for this) - `EntityDisabledException` removed, replaced by a `Validation` with `disabled` key. - validation fixed for Schemas (did not include super-check) - V3 integTests updated to cover the `disabled` = validation fail
AbsaOSS · May 16, 2022 · 5e1b4cd · 5e1b4cd
1 parent 10c45db
commit 5e1b4cd
Show file tree

Hide file tree

Showing 8 changed files with 200 additions and 71 deletions.
diff --git a/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/RestExceptionHandler.scala b/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/RestExceptionHandler.scala
@@ -67,11 +67,6 @@ class RestExceptionHandler {
     ResponseEntity.status(HttpStatus.I_AM_A_TEAPOT).build[Any]() // Could change for LOCKED but I like this more
   }
 
-  @ExceptionHandler(value = Array(classOf[EntityDisabledException]))
-  def handleEntityDisabled(exception: EntityDisabledException): ResponseEntity[EntityDisabledException] = {
-    ResponseEntity.status(HttpStatus.BAD_REQUEST).body(exception)
-  }
-
   @ExceptionHandler(value = Array(classOf[SchemaParsingException]))
   def handleBadRequestException(exception: SchemaParsingException): ResponseEntity[Any] = {
     val response = RestResponse(exception.message, Option(SchemaParsingError.fromException(exception)))

diff --git a/.../main/scala/za/co/absa/enceladus/rest_api/controllers/v3/VersionedModelControllerV3.scala b/.../main/scala/za/co/absa/enceladus/rest_api/controllers/v3/VersionedModelControllerV3.scala
@@ -15,7 +15,6 @@
 
 package za.co.absa.enceladus.rest_api.controllers.v3
 
-import com.mongodb.client.result.UpdateResult
 import org.springframework.http.{HttpStatus, ResponseEntity}
 import org.springframework.security.core.annotation.AuthenticationPrincipal
 import org.springframework.security.core.userdetails.UserDetails
@@ -26,9 +25,8 @@ import za.co.absa.enceladus.model.versionedModel._
 import za.co.absa.enceladus.model.{ExportableObject, UsedIn, Validation}
 import za.co.absa.enceladus.rest_api.controllers.BaseController
 import za.co.absa.enceladus.rest_api.controllers.v3.VersionedModelControllerV3.LatestVersionKey
-import za.co.absa.enceladus.rest_api.exceptions.{EntityDisabledException, NotFoundException, ValidationException}
+import za.co.absa.enceladus.rest_api.exceptions.NotFoundException
 import za.co.absa.enceladus.rest_api.models.rest.DisabledPayload
-import za.co.absa.enceladus.rest_api.services.VersionedModelService
 import za.co.absa.enceladus.rest_api.services.v3.VersionedModelServiceV3
 
 import java.net.URI
@@ -132,13 +130,10 @@ abstract class VersionedModelControllerV3[C <: VersionedModel with Product
   def create(@AuthenticationPrincipal principal: UserDetails,
              @RequestBody item: C,
              request: HttpServletRequest): CompletableFuture[ResponseEntity[Validation]] = {
-    versionedModelService.isDisabled(item.name).flatMap { isDisabled =>
-      if (isDisabled) {
-        Future.failed(EntityDisabledException(s"Entity ${item.name} is disabled. Enable it first (PUT) to push new versions (PUT)."))
-      } else {
+
+    // enabled check is part of the validation
         versionedModelService.create(item, principal.getUsername)
-      }
-    }.map {
+      .map {
       case Some((entity, validation)) => createdWithNameVersionLocationBuilder(entity.name, entity.version, request).body(validation)
       case None => throw notFound()
     }
@@ -157,16 +152,11 @@ abstract class VersionedModelControllerV3[C <: VersionedModel with Product
     } else if (version != item.version) {
       Future.failed(new IllegalArgumentException(s"URL and payload version mismatch: ${version} != ${item.version}"))
     } else {
-      versionedModelService.isDisabled(item.name).flatMap { isDisabled =>
-        if (isDisabled) {
-          throw EntityDisabledException(s"Entity ${item.name} is disabled. Enable it first to create new versions.")
-        } else {
-          versionedModelService.update(user.getUsername, item).map {
-            case Some((updatedEntity, validation)) =>
-              createdWithNameVersionLocationBuilder(updatedEntity.name, updatedEntity.version, request, stripLastSegments = 2).body(validation)
-            case None => throw notFound()
-          }
-        }
+      // disable check is already part of V3 validation
+      versionedModelService.update(user.getUsername, item).map {
+        case Some((updatedEntity, validation)) =>
+          createdWithNameVersionLocationBuilder(updatedEntity.name, updatedEntity.version, request, stripLastSegments = 2).body(validation)
+        case None => throw notFound()
       }
     }
   }

diff --git a/...api/src/main/scala/za/co/absa/enceladus/rest_api/exceptions/EntityDisabledException.scala b/...api/src/main/scala/za/co/absa/enceladus/rest_api/exceptions/EntityDisabledException.scala
diff --git a/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/v3/SchemaServiceV3.scala b/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/v3/SchemaServiceV3.scala
@@ -15,15 +15,12 @@
 
 package za.co.absa.enceladus.rest_api.services.v3
 
-import org.apache.spark.sql.types.StructType
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.stereotype.Service
-import za.co.absa.enceladus.model.{Schema, UsedIn, Validation}
+import za.co.absa.enceladus.model.{Schema, SchemaField, UsedIn, Validation}
 import za.co.absa.enceladus.rest_api.repositories.{DatasetMongoRepository, MappingTableMongoRepository, SchemaMongoRepository}
-import za.co.absa.enceladus.rest_api.services.{SchemaService, VersionedModelService}
+import za.co.absa.enceladus.rest_api.services.SchemaService
 import za.co.absa.enceladus.rest_api.utils.converters.SparkMenasSchemaConvertor
-import scala.concurrent.ExecutionContext.Implicits.global
-
 
 import scala.concurrent.Future
 
@@ -38,7 +35,15 @@ class SchemaServiceV3 @Autowired()(schemaMongoRepository: SchemaMongoRepository,
   import scala.concurrent.ExecutionContext.Implicits.global
 
   override def validate(item: Schema): Future[Validation] = {
-    if (item.fields.isEmpty) {
+    for {
+      originalValidation <- super.validate(item)
+      fieldsValidation <- validateSchemaFields(item.fields)
+    } yield originalValidation.merge(fieldsValidation)
+
+  }
+
+  protected def validateSchemaFields(fields: Seq[SchemaField]): Future[Validation] = {
+    if (fields.isEmpty) {
       // V3 disallows empty schema fields - V2 allowed it at first that to get updated by an attachment upload/remote-load
       Future.successful(Validation.empty.withError("schema-fields","No fields found! There must be fields defined for actual usage."))
     } else {

diff --git a/...a/enceladus/rest_api/integration/controllers/v3/DatasetControllerV3IntegrationSuite.scala b/...a/enceladus/rest_api/integration/controllers/v3/DatasetControllerV3IntegrationSuite.scala
@@ -30,7 +30,6 @@ import za.co.absa.enceladus.model.properties.propertyType.EnumPropertyType
 import za.co.absa.enceladus.model.test.factories.{DatasetFactory, MappingTableFactory, PropertyDefinitionFactory, SchemaFactory}
 import za.co.absa.enceladus.model.versionedModel.NamedVersion
 import za.co.absa.enceladus.model.{Dataset, UsedIn, Validation}
-import za.co.absa.enceladus.rest_api.exceptions.EntityDisabledException
 import za.co.absa.enceladus.rest_api.integration.controllers.{BaseRestApiTestV3, toExpected}
 import za.co.absa.enceladus.rest_api.integration.fixtures._
 import za.co.absa.enceladus.rest_api.models.rest.DisabledPayload
@@ -111,19 +110,19 @@ class DatasetControllerV3IntegrationSuite extends BaseRestApiTestV3 with BeforeA
         val responseBody = response.getBody
         responseBody shouldBe Validation(Map("undefinedProperty1" -> List("There is no property definition for key 'undefinedProperty1'.")))
       }
-      "disabled entity with the name already exists" in {
+      "entity with the name already exists" in {
         schemaFixture.add(SchemaFactory.getDummySchema("dummySchema"))
         val dataset1 = DatasetFactory.getDummyDataset("dummyDs", disabled = true)
         datasetFixture.add(dataset1)
 
         val dataset2 = DatasetFactory.getDummyDataset("dummyDs", description = Some("a new version attempt"))
-        val response = sendPost[Dataset, EntityDisabledException](apiUrl, bodyOpt = Some(dataset2))
+        val response = sendPost[Dataset, Validation](apiUrl, bodyOpt = Some(dataset2))
 
         assertBadRequest(response)
-        response.getBody.getMessage should include("Entity dummyDs is disabled. Enable it first")
+        response.getBody shouldBe Validation.empty.withError("name", "entity with name already exists: 'dummyDs'")
+        // even if disabled, the dulicate name check has precedence and is reported first
       }
     }
-
   }
 
   s"GET $apiUrl/{name}" should {
@@ -353,10 +352,10 @@ class DatasetControllerV3IntegrationSuite extends BaseRestApiTestV3 with BeforeA
         datasetFixture.add(dataset1)
 
         val dataset2 = DatasetFactory.getDummyDataset("dummyDs", description = Some("ds update"))
-        val response = sendPut[Dataset, EntityDisabledException](s"$apiUrl/dummyDs/1", bodyOpt = Some(dataset2))
+        val response = sendPut[Dataset, Validation](s"$apiUrl/dummyDs/1", bodyOpt = Some(dataset2))
 
         assertBadRequest(response)
-        response.getBody.getMessage should include("Entity dummyDs is disabled. Enable it first")
+        response.getBody shouldBe Validation.empty.withError("disabled", "Entity dummyDs is disabled!")
       }
     }
   }
@@ -431,6 +430,21 @@ class DatasetControllerV3IntegrationSuite extends BaseRestApiTestV3 with BeforeA
         response.getStatusCode shouldBe HttpStatus.BAD_REQUEST
         response.getBody shouldBe Validation.empty.withError("key2", "There is no property definition for key 'key2'.")
       }
+      "exists and is disabled" in {
+        schemaFixture.add(SchemaFactory.getDummySchema("dummySchema")) // import feature checks schema presence
+        val dataset1 = DatasetFactory.getDummyDataset(name = "datasetXYZ", description = Some("init version"), disabled = true)
+        datasetFixture.add(dataset1)
+
+        propertyDefinitionFixture.add(
+          PropertyDefinitionFactory.getDummyPropertyDefinition("key1"),
+          PropertyDefinitionFactory.getDummyPropertyDefinition("key2")
+        )
+
+        val response = sendPost[String, Validation](s"$apiUrl/datasetXYZ/import", bodyOpt = Some(importableDs))
+        assertBadRequest(response)
+
+        response.getBody shouldBe Validation.empty.withError("disabled", "Entity datasetXYZ is disabled!")
+      }
     }
 
     "return 201" when {
@@ -684,6 +698,22 @@ class DatasetControllerV3IntegrationSuite extends BaseRestApiTestV3 with BeforeA
         assertBadRequest(response2)
         response2.getBody shouldBe Validation(Map("AorB" -> List("Value 'c' is not one of the allowed values (a, b).")))
       }
+      "when the dataset is disabled" in {
+        schemaFixture.add(SchemaFactory.getDummySchema("dummySchema"))
+        val datasetV1 = DatasetFactory.getDummyDataset(name = "datasetA", version = 1, disabled = true)
+        datasetFixture.add(datasetV1)
+
+        propertyDefinitionFixture.add(
+          PropertyDefinitionFactory.getDummyPropertyDefinition("AorB", propertyType = EnumPropertyType("a", "b"))
+        )
+
+        val response = sendPut[Map[String, String], Validation](s"$apiUrl/datasetA/1/properties",
+          bodyOpt = Some(Map("AorB" -> "a")))
+
+        assertBadRequest(response)
+        val responseBody = response.getBody
+        responseBody shouldBe Validation(Map("disabled" -> List("Entity datasetA is disabled!")))
+      }
     }
 
     "201 Created with location" when {
@@ -772,6 +802,15 @@ class DatasetControllerV3IntegrationSuite extends BaseRestApiTestV3 with BeforeA
         assertOk(response)
         response.getBody shouldBe Validation(Map("AorB" -> List("Value 'c' is not one of the allowed values (a, b).")))
       }
+      "dataset is disabled" in {
+        schemaFixture.add(SchemaFactory.getDummySchema("dummySchema"))
+        val datasetV1 = DatasetFactory.getDummyDataset(name = "datasetA", disabled = true)
+        datasetFixture.add(datasetV1)
+
+        val response = sendGet[Validation](s"$apiUrl/datasetA/1/validation")
+        assertOk(response)
+        response.getBody shouldBe Validation(Map("disabled" -> List("Entity datasetA is disabled!")))
+      }
     }
   }
 
@@ -870,6 +909,17 @@ class DatasetControllerV3IntegrationSuite extends BaseRestApiTestV3 with BeforeA
 
         response.getBody shouldBe Validation.empty.withError("mapping-table", "Mapping table CurrencyMappingTable v9 not found!")
       }
+      "when dataset is disabled" in {
+        schemaFixture.add(SchemaFactory.getDummySchema("dummySchema"))
+        val datasetV1 = DatasetFactory.getDummyDataset(name = "datasetA", disabled = true, conformance = List(
+          LiteralConformanceRule(order = 0, "column1", true, "ABC"))
+        )
+        datasetFixture.add(datasetV1)
+
+        val response = sendPost[ConformanceRule, Validation](s"$apiUrl/datasetA/1/rules", bodyOpt = Some(exampleLitRule1))
+        assertBadRequest(response)
+        response.getBody shouldBe Validation.empty.withError("disabled", "Entity datasetA is disabled!")
+      }
     }
 
     "return 201" when {

diff --git a/...eladus/rest_api/integration/controllers/v3/MappingTableControllerV3IntegrationSuite.scala b/...eladus/rest_api/integration/controllers/v3/MappingTableControllerV3IntegrationSuite.scala
@@ -121,7 +121,6 @@ class MappingTableControllerV3IntegrationSuite extends BaseRestApiTestV3 with Be
           ))
         )
 
-
         val response = sendGet[Array[DefaultValue]](s"$apiUrl/mtA/latest/defaults")
         assertOk(response)
         response.getBody shouldBe Array(DefaultValue("columnX", "defaultXvalue"), DefaultValue("columnY", "defaultYvalue"))
@@ -141,7 +140,7 @@ class MappingTableControllerV3IntegrationSuite extends BaseRestApiTestV3 with Be
     }
 
     "return 400" when {
-      "when version is not the latest (only last version can be updated)" in {
+      "the version is not the latest (only last version can be updated)" in {
         val mtAv1 = MappingTableFactory.getDummyMappingTable("mtA", version = 1)
         val mtAv2 = MappingTableFactory.getDummyMappingTable("mtA", version = 2)
         val mtAv3 = MappingTableFactory.getDummyMappingTable("mtA", version = 3)
@@ -156,6 +155,18 @@ class MappingTableControllerV3IntegrationSuite extends BaseRestApiTestV3 with Be
           List("Version 2 of mtA is not the latest version, therefore cannot be edited")
         ))
       }
+      "the mapping table is disabled" in {
+        val mtAv1 = MappingTableFactory.getDummyMappingTable("mtA", version = 1, disabled = true)
+          .copy(defaultMappingValue = List(DefaultValue("anOldDefault", "itsValue")))
+        mappingTableFixture.add(mtAv1)
+        schemaFixture.add(SchemaFactory.getDummySchema("dummySchema")) // Schema referenced by MT must exist
+
+        val response = sendPut[Array[DefaultValue], Validation](s"$apiUrl/mtA/1/defaults",
+          bodyOpt = Some(Array.empty[DefaultValue]))
+        assertBadRequest(response)
+
+        response.getBody shouldBe Validation.empty.withError("disabled", "Entity mtA is disabled!")
+      }
     }
 
     "201 Created with location" when {
@@ -212,10 +223,20 @@ class MappingTableControllerV3IntegrationSuite extends BaseRestApiTestV3 with Be
           List("Version 2 of mtA is not the latest version, therefore cannot be edited")
         ))
       }
+      "the mapping table is disabled" in {
+        val mtAv1 = MappingTableFactory.getDummyMappingTable("mtA", version = 1, disabled = true)
+          .copy(defaultMappingValue = List(DefaultValue("anOldDefault", "itsValue")))
+        mappingTableFixture.add(mtAv1)
+        schemaFixture.add(SchemaFactory.getDummySchema("dummySchema")) // Schema referenced by MT must exist
+
+        val response = sendPost[DefaultValue, Validation](s"$apiUrl/mtA/1/defaults", bodyOpt = Some(DefaultValue("colA", "defaultA")))
+        assertBadRequest(response)
+        response.getBody shouldBe Validation.empty.withError("disabled", "Entity mtA is disabled!")
+      }
     }
 
     "201 Created with location" when {
-      s"defaults are replaced with a new version" in {
+      "defaults are replaced with a new version" in {
         val mtAv1 = MappingTableFactory.getDummyMappingTable("mtA", version = 1).copy(defaultMappingValue = List(DefaultValue("anOldDefault", "itsValue")))
         mappingTableFixture.add(mtAv1)
 
@@ -264,9 +285,9 @@ class MappingTableControllerV3IntegrationSuite extends BaseRestApiTestV3 with Be
         val mappingTable2 = MappingTableFactory.getDummyMappingTable(name = "mappingTable", version = 2)
         mappingTableFixture.add(mappingTable1, mappingTable2)
 
-        val datasetA = DatasetFactory.getDummyDataset(name = "datasetA", conformance = List(mcr("mappingTable",1)))
-        val datasetB = DatasetFactory.getDummyDataset(name = "datasetB", conformance = List(mcr("mappingTable",1)), disabled = true)
-        val datasetC = DatasetFactory.getDummyDataset(name = "datasetC", conformance = List(mcr("mappingTable",2)))
+        val datasetA = DatasetFactory.getDummyDataset(name = "datasetA", conformance = List(mcr("mappingTable", 1)))
+        val datasetB = DatasetFactory.getDummyDataset(name = "datasetB", conformance = List(mcr("mappingTable", 1)), disabled = true)
+        val datasetC = DatasetFactory.getDummyDataset(name = "datasetC", conformance = List(mcr("mappingTable", 2)))
         datasetFixture.add(datasetA, datasetB, datasetC)
 
         val response = sendGet[String](s"$apiUrl/mappingTable/used-in")
@@ -293,9 +314,9 @@ class MappingTableControllerV3IntegrationSuite extends BaseRestApiTestV3 with Be
         val mappingTable2 = MappingTableFactory.getDummyMappingTable(name = "mappingTable", version = 2)
         mappingTableFixture.add(mappingTable1, mappingTable2)
 
-        val datasetA = DatasetFactory.getDummyDataset(name = "datasetA", conformance = List(mcr("mappingTable",1)))
-        val datasetB = DatasetFactory.getDummyDataset(name = "datasetB", conformance = List(mcr("mappingTable",1)), disabled = true)
-        val datasetC = DatasetFactory.getDummyDataset(name = "datasetC", conformance = List(mcr("mappingTable",2)))
+        val datasetA = DatasetFactory.getDummyDataset(name = "datasetA", conformance = List(mcr("mappingTable", 1)))
+        val datasetB = DatasetFactory.getDummyDataset(name = "datasetB", conformance = List(mcr("mappingTable", 1)), disabled = true)
+        val datasetC = DatasetFactory.getDummyDataset(name = "datasetC", conformance = List(mcr("mappingTable", 2)))
         datasetFixture.add(datasetA, datasetB, datasetC)
 
         val response = sendGet[UsedIn](s"$apiUrl/mappingTable/1/used-in")
@@ -407,7 +428,7 @@ class MappingTableControllerV3IntegrationSuite extends BaseRestApiTestV3 with Be
 
           val dataset1 = DatasetFactory.getDummyDataset(name = "dataset1", conformance = List(mcr("mappingTable", 1)))
           val dataset2 = DatasetFactory.getDummyDataset(name = "dataset2", version = 7, conformance = List(mcr("mappingTable", 2)))
-          val dataset3 = DatasetFactory.getDummyDataset(name = "dataset3",conformance = List(mcr("anotherMappingTable", 8))) // moot
+          val dataset3 = DatasetFactory.getDummyDataset(name = "dataset3", conformance = List(mcr("anotherMappingTable", 8))) // moot
           val disabledDs = DatasetFactory.getDummyDataset(name = "disabledDs", conformance = List(mcr("mappingTable", 2)), disabled = true)
           datasetFixture.add(dataset1, dataset2, dataset3, disabledDs)