#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/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])

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}")

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)
   }
 

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}

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

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

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])
 

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
 
 }