From e6c8d8ce4c05cfdba25923b17710028f55240233 Mon Sep 17 00:00:00 2001 From: Daniel Kavan Date: Tue, 29 Mar 2022 15:17:41 +0200 Subject: [PATCH] #1693 API v3: VersionedModelControllerV3 - GET/PUT /{name}/{version}/properties - supports latest for as version-expression; get impl is unvalidated, put impl checks validity - login is now common, under /api/login for both v2 and v3 (did not work previously) --- .../rest_api/WebSecurityConfig.scala | 8 -- .../controllers/DatasetController.scala | 2 +- .../VersionedModelController.scala | 5 +- .../controllers/v3/DatasetControllerV3.scala | 39 +++++++ .../v3/VersionedModelControllerV3.scala | 8 +- .../rest_api/services/DatasetService.scala | 36 +++++- .../controllers/BaseRestApiTest.scala | 9 +- .../DatasetControllerV3IntegrationSuite.scala | 108 +++++++++++++++++- 8 files changed, 189 insertions(+), 26 deletions(-) diff --git a/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/WebSecurityConfig.scala b/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/WebSecurityConfig.scala index d7d8e1936..43ab92128 100644 --- a/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/WebSecurityConfig.scala +++ b/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/WebSecurityConfig.scala @@ -68,19 +68,11 @@ class WebSecurityConfig @Autowired()(beanFactory: BeanFactory, .anyRequest() .authenticated() .and() - // v2 login .formLogin() .loginProcessingUrl("/api/login") .successHandler(authenticationSuccessHandler) .failureHandler(authenticationFailureHandler) .permitAll() - .and() - // v3 login - .formLogin() - .loginProcessingUrl("/api-v3/login") - .successHandler(authenticationSuccessHandler) - .failureHandler(authenticationFailureHandler) - .permitAll() .and() .addFilterBefore(kerberosFilter, classOf[UsernamePasswordAuthenticationFilter]) .addFilterAfter(jwtAuthFilter, classOf[SpnegoAuthenticationProcessingFilter]) diff --git a/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/DatasetController.scala b/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/DatasetController.scala index 0f33bf7d1..b56396087 100644 --- a/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/DatasetController.scala +++ b/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/DatasetController.scala @@ -113,7 +113,7 @@ class DatasetController @Autowired()(datasetService: DatasetService) def replaceProperties(@AuthenticationPrincipal principal: UserDetails, @PathVariable datasetName: String, @RequestBody newProperties: Optional[Map[String, String]]): CompletableFuture[ResponseEntity[Option[Dataset]]] = { - datasetService.replaceProperties(principal.getUsername, datasetName, newProperties.toScalaOption).map { + datasetService.updatePropertiesV2(principal.getUsername, datasetName, newProperties.toScalaOption).map { case None => throw notFound() case Some(dataset) => val location: URI = new URI(s"/api/dataset/${dataset.name}/${dataset.version}") diff --git a/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/VersionedModelController.scala b/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/VersionedModelController.scala index dbe997478..9f6bf5756 100644 --- a/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/VersionedModelController.scala +++ b/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/VersionedModelController.scala @@ -17,7 +17,6 @@ package za.co.absa.enceladus.rest_api.controllers import java.util.Optional import java.util.concurrent.CompletableFuture - import com.mongodb.client.result.UpdateResult import org.springframework.http.HttpStatus import org.springframework.security.core.annotation.AuthenticationPrincipal @@ -29,6 +28,8 @@ import za.co.absa.enceladus.rest_api.exceptions.NotFoundException import za.co.absa.enceladus.rest_api.services.VersionedModelService import za.co.absa.enceladus.model.menas.audit._ +import scala.concurrent.Future + abstract class VersionedModelController[C <: VersionedModel with Product with Auditable[C]](versionedModelService: VersionedModelService[C]) extends BaseController { @@ -69,7 +70,7 @@ abstract class VersionedModelController[C <: VersionedModel with Product with Au @GetMapping(Array("/detail/{name}/latestVersion")) @ResponseStatus(HttpStatus.OK) - def getLatestVersionNumber(@PathVariable name: String): CompletableFuture[Int] = { + def getLatestVersionNumber(@PathVariable name: String): Future[Int] = { versionedModelService.getLatestVersionNumber(name) } diff --git a/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/v3/DatasetControllerV3.scala b/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/v3/DatasetControllerV3.scala index ac207ca02..9a0b86290 100644 --- a/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/v3/DatasetControllerV3.scala +++ b/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/v3/DatasetControllerV3.scala @@ -16,14 +16,53 @@ package za.co.absa.enceladus.rest_api.controllers.v3 import org.springframework.beans.factory.annotation.Autowired +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.rest_api.services.DatasetService +import za.co.absa.enceladus.rest_api.utils.implicits._ + +import java.net.URI +import java.util.concurrent.CompletableFuture +import javax.servlet.http.HttpServletRequest +import scala.concurrent.ExecutionContext.Implicits.global @RestController @RequestMapping(path = Array("/api-v3/datasets")) class DatasetControllerV3 @Autowired()(datasetService: DatasetService) extends VersionedModelControllerV3(datasetService) { + @GetMapping(Array("/{name}/{version}/properties")) + @ResponseStatus(HttpStatus.OK) + def getAllPropertiesForVersion(@PathVariable name: String, + @PathVariable version: String): CompletableFuture[Map[String, String]] = { + forVersionExpression(name, version)(datasetService.getVersion).map { + case Some(entity) => entity.propertiesAsMap + case None => throw notFound() + } + } + + @PutMapping(Array("/{name}/{version}/properties")) + @ResponseStatus(HttpStatus.OK) + def updateProperties(@AuthenticationPrincipal principal: UserDetails, + @PathVariable name: String, + @PathVariable version: String, + @RequestBody newProperties: java.util.Map[String, String], + request: HttpServletRequest): CompletableFuture[ResponseEntity[Nothing]] = { + forVersionExpression(name, version) { case (dsName, dsVersion) => + datasetService.updateProperties(principal.getUsername, dsName, dsVersion, newProperties.toScalaMap).map { + + case Some(entity) => + // 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") + case None => throw notFound() + } + } + } + + // todo putIntoInfoFile switch needed? + // TODO // /{datasetName}/{version}/rules // /{datasetName}/{version}/rules/{index} diff --git a/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/v3/VersionedModelControllerV3.scala b/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/v3/VersionedModelControllerV3.scala index 5c930e842..0c19739f8 100644 --- a/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/v3/VersionedModelControllerV3.scala +++ b/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/controllers/v3/VersionedModelControllerV3.scala @@ -24,12 +24,12 @@ import org.springframework.web.servlet.support.ServletUriComponentsBuilder import za.co.absa.enceladus.model.menas.audit._ import za.co.absa.enceladus.model.versionedModel._ import za.co.absa.enceladus.model.{ExportableObject, UsedIn} -import za.co.absa.enceladus.rest_api.controllers.BaseController +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.NotFoundException import za.co.absa.enceladus.rest_api.services.VersionedModelService import java.net.URI +import java.util import java.util.Optional import java.util.concurrent.CompletableFuture import javax.servlet.http.HttpServletRequest @@ -189,11 +189,11 @@ abstract class VersionedModelControllerV3[C <: VersionedModel with Product } protected def createdWithNameVersionLocation(name: String, version: Int, request: HttpServletRequest, - stripLastSegments: Int = 0): ResponseEntity[Nothing] = { + stripLastSegments: Int = 0, suffix: String = ""): ResponseEntity[Nothing] = { val strippingPrefix = Range(0, stripLastSegments).map(_ => "/..").mkString val location: URI = ServletUriComponentsBuilder.fromRequest(request) - .path(s"$strippingPrefix/{name}/{version}") + .path(s"$strippingPrefix/{name}/{version}$suffix") .buildAndExpand(name, version.toString) .normalize() // will normalize `/one/two/../three` into `/one/tree` .toUri() // will create location e.g. http:/domain.ext/api-v3/dataset/MyExampleDataset/1 diff --git a/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/DatasetService.scala b/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/DatasetService.scala index 309b53809..ef26b9396 100644 --- a/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/DatasetService.scala +++ b/rest-api/src/main/scala/za/co/absa/enceladus/rest_api/services/DatasetService.scala @@ -28,7 +28,7 @@ import za.co.absa.enceladus.model.properties.essentiality.Mandatory import za.co.absa.enceladus.model.{Dataset, Schema, UsedIn, Validation} import za.co.absa.enceladus.utils.validation.ValidationLevel import DatasetService._ -import za.co.absa.enceladus.rest_api.exceptions.NotFoundException +import za.co.absa.enceladus.rest_api.exceptions.{NotFoundException, ValidationException} import za.co.absa.enceladus.utils.validation.ValidationLevel.ValidationLevel import scala.concurrent.Future @@ -123,8 +123,24 @@ class DatasetService @Autowired()(datasetMongoRepository: DatasetMongoRepository } } - def replaceProperties(username: String, datasetName: String, - updatedProperties: Option[Map[String, String]]): Future[Option[Dataset]] = { + def updateProperties(username: String, datasetName: String, datasetVersion: Int, + updatedProperties: Map[String, String]): Future[Option[Dataset]] = { + for { + s <- 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? + } + + // updateFuture includes latest-check and version increase + update <- updateFuture(username, datasetName, datasetVersion) { latest => + Future.successful(latest.copy(properties = Some(removeBlankProperties(updatedProperties)))) + } + } yield update + } + + // kept for API v2 usage only + def updatePropertiesV2(username: String, datasetName: String, + updatedProperties: Option[Map[String, String]]): Future[Option[Dataset]] = { for { latestVersion <- getLatestVersionNumber(datasetName) update <- update(username, datasetName, latestVersion) { latest => @@ -205,7 +221,7 @@ class DatasetService @Autowired()(datasetMongoRepository: DatasetMongoRepository } } - def validateProperties(properties: Map[String, String], forRun: Boolean): Future[Validation] = { + def validateProperties(properties: Map[String, String], forRun: Boolean = false): Future[Validation] = { datasetPropertyDefinitionService.getLatestVersions().map { propDefs: Seq[PropertyDefinition] => val propDefsMap = Map(propDefs.map { propDef => (propDef.name, propDef) }: _*) // map(key, propDef) @@ -437,10 +453,20 @@ object DatasetService { */ def removeBlankProperties(properties: Option[Map[String, String]]): Option[Map[String, String]] = { properties.map { - _.filter { case (_, propValue) => propValue.nonEmpty } + removeBlankProperties } } + /** + * Removes properties having empty-string value. Effectively mapping such properties' values from Some("") to None. + * This is Backend-implementation related to DatasetService.replaceBlankProperties(dataset) on Frontend + * @param properties original properties + * @return properties without empty-string value entries + */ + def removeBlankProperties(properties: Map[String, String]): Map[String, String] = { + properties.filter { case (_, propValue) => propValue.nonEmpty } + } + private[services] def replacePrefixIfFound(fieldName: String, replacement: String, lookFor: String): Option[String] = { fieldName match { case `lookFor` => Some(replacement) // exact match diff --git a/rest-api/src/test/scala/za/co/absa/enceladus/rest_api/integration/controllers/BaseRestApiTest.scala b/rest-api/src/test/scala/za/co/absa/enceladus/rest_api/integration/controllers/BaseRestApiTest.scala index 9f0e619d7..ab2b8aa1e 100644 --- a/rest-api/src/test/scala/za/co/absa/enceladus/rest_api/integration/controllers/BaseRestApiTest.scala +++ b/rest-api/src/test/scala/za/co/absa/enceladus/rest_api/integration/controllers/BaseRestApiTest.scala @@ -32,10 +32,10 @@ import za.co.absa.enceladus.rest_api.integration.repositories.BaseRepositoryTest import scala.concurrent.Future import scala.reflect.ClassTag -abstract class BaseRestApiTestV2 extends BaseRestApiTest("/api") -abstract class BaseRestApiTestV3 extends BaseRestApiTest("/api-v3") +abstract class BaseRestApiTestV2 extends BaseRestApiTest("/api/login", "/api") +abstract class BaseRestApiTestV3 extends BaseRestApiTest("/api/login", "/api-v3") -abstract class BaseRestApiTest(apiPath: String) extends BaseRepositoryTest { +abstract class BaseRestApiTest(loginPath: String, apiPath: String) extends BaseRepositoryTest { import scala.concurrent.ExecutionContext.Implicits.global @@ -55,6 +55,7 @@ abstract class BaseRestApiTest(apiPath: String) extends BaseRepositoryTest { // expecting apiPath to be /api for v2 and /api-v3 for v3 private lazy val baseUrl = s"http://localhost:$port$apiPath" + private lazy val loginBaseUrl = s"http://localhost:$port$loginPath" private lazy val authHeaders = getAuthHeaders(user, passwd) private lazy val authHeadersAdmin = getAuthHeaders(adminUser, adminPasswd) @@ -75,7 +76,7 @@ abstract class BaseRestApiTest(apiPath: String) extends BaseRepositoryTest { } def getAuthHeaders(username: String, password: String): HttpHeaders = { - val loginUrl = s"$baseUrl/login?username=$username&password=$password&submit=Login" + val loginUrl = s"$loginBaseUrl?username=$username&password=$password&submit=Login" val response = restTemplate.postForEntity(loginUrl, HttpEntity.EMPTY, classOf[String]) diff --git a/rest-api/src/test/scala/za/co/absa/enceladus/rest_api/integration/controllers/v3/DatasetControllerV3IntegrationSuite.scala b/rest-api/src/test/scala/za/co/absa/enceladus/rest_api/integration/controllers/v3/DatasetControllerV3IntegrationSuite.scala index be79264ff..5b4e84193 100644 --- a/rest-api/src/test/scala/za/co/absa/enceladus/rest_api/integration/controllers/v3/DatasetControllerV3IntegrationSuite.scala +++ b/rest-api/src/test/scala/za/co/absa/enceladus/rest_api/integration/controllers/v3/DatasetControllerV3IntegrationSuite.scala @@ -468,7 +468,7 @@ class DatasetControllerV3IntegrationSuite extends BaseRestApiTestV3 with BeforeA } } - s"GET $apiUrl/{name}/{version}/export" should { + s"GET $apiUrl/{name}/{version}/used-in" should { "return 404" when { "when the dataset of latest version does not exist" in { val response = sendGet[String](s"$apiUrl/notFoundDataset/latest/used-in") @@ -512,6 +512,110 @@ class DatasetControllerV3IntegrationSuite extends BaseRestApiTestV3 with BeforeA } } - // todo properties test for datasets or in general for any VersionedModel + s"GET $apiUrl/{name}/{version}/properties" should { + "return 404" when { + "when the name+version does not exist" in { + val response = sendGet[String](s"$apiUrl/notFoundDataset/456/properties") + assertNotFound(response) + } + } + + "return 200" when { + "there is a specific Dataset version" should { + Seq( + ("empty1", Some(Map.empty[String, String])), + ("empty2", None), + ("non-empty", Some(Map("key1" -> "val1", "key2" -> "val2"))) + ).foreach { case (propertiesCaseName, propertiesData) => + s"return dataset properties ($propertiesCaseName)" in { + val datasetV1 = DatasetFactory.getDummyDataset(name = "datasetA", version = 1) + val datasetV2 = DatasetFactory.getDummyDataset(name = "datasetA", version = 2, properties = propertiesData) + val datasetV3 = DatasetFactory.getDummyDataset(name = "datasetA", version = 3, properties = Some(Map("other" -> "prop"))) + datasetFixture.add(datasetV1, datasetV2, datasetV3) + + val response = sendGet[Map[String, String]](s"$apiUrl/datasetA/2/properties") + assertOk(response) + + val expectedProperties = propertiesData.getOrElse(Map.empty[String, String]) + val body = response.getBody + assert(body == expectedProperties) + } + } + } + } + + "return 200" when { + "there is a latest Dataset version" should { + s"return dataset properties" in { + val datasetV1 = DatasetFactory.getDummyDataset(name = "datasetA", version = 1) + val datasetV2 = DatasetFactory.getDummyDataset(name = "datasetA", version = 2, properties = Some(Map("key1" -> "val1", "key2" -> "val2"))) + datasetFixture.add(datasetV1, datasetV2) + + val response = sendGet[Map[String, String]](s"$apiUrl/datasetA/latest/properties") + assertOk(response) + + val body = response.getBody + assert(body == Map("key1" -> "val1", "key2" -> "val2")) + } + } + } + } + + + s"PUT $apiUrl/{name}/{version}/properties" should { + "return 404" when { + "when the name+version does not exist" in { + val response = sendPut[Map[String, String], String](s"$apiUrl/notFoundDataset/456/properties", bodyOpt = Some(Map.empty)) + assertNotFound(response) + } + } + + "return 400" when { + "when version is not the latest (only last version can be updated)" in { + val datasetV1 = DatasetFactory.getDummyDataset(name = "datasetA", version = 1) + val datasetV2 = DatasetFactory.getDummyDataset(name = "datasetA", version = 2) + val datasetV3 = DatasetFactory.getDummyDataset(name = "datasetA", version = 3) + datasetFixture.add(datasetV1, datasetV2, datasetV3) + + val response = sendPut[Map[String, String], String](s"$apiUrl/datasetA/2/properties", bodyOpt = Some(Map.empty)) + assertBadRequest(response) + } + } + + // todo add update-validation failing case + // todo check validity and refuse if not valid (open: both not having proper propDef, and also not valid for a propdef?) + + + "201 Created with location" when { + Seq( + ("non-empty properties map", """{"keyA":"valA","keyB":"valB","keyC":""}""", Some(Map("keyA" -> "valA", "keyB" -> "valB"))), // empty string property would get removed (defined "" => undefined) + ("empty properties map", "{}", Some(Map.empty)) + ).foreach { case (testCaseName, payload, expectedPropertiesSet) => + s"properties are replaced with a new version ($testCaseName)" in { + val datasetV1 = DatasetFactory.getDummyDataset(name = "datasetA", version = 1) + datasetFixture.add(datasetV1) + + propertyDefinitionFixture.add( + PropertyDefinitionFactory.getDummyPropertyDefinition("keyA"), + PropertyDefinitionFactory.getDummyPropertyDefinition("keyB"), + PropertyDefinitionFactory.getDummyPropertyDefinition("keyC") + ) + + val response1 = sendPut[String, String](s"$apiUrl/datasetA/1/properties", bodyOpt = Some(payload)) + assertCreated(response1) + val headers1 = response1.getHeaders + assert(headers1.getFirst("Location").endsWith("/api-v3/datasets/datasetA/2/properties")) + + + val response2 = sendGet[Map[String, String]](s"$apiUrl/datasetA/2/properties") + assertOk(response2) + val responseBody = response2.getBody + responseBody shouldBe expectedPropertiesSet.getOrElse(Map.empty) + } + } + } + } + + // todo CR tests }