Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/1693 api v3 versioned model - Dataset v3 implementation #2046

Merged
merged 34 commits into from
May 9, 2022
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
f1798b2
#1693 API v3: VersionedModel v3 init - VersionedModelControllerV3 bas…
dk1844 Mar 7, 2022
0f62851
#1693 API v3: some basic VersionedModelControllerV3 integration test
dk1844 Mar 7, 2022
c01cfda
#1693 API v3: VersionedModelControllerV3 integration test cont'd - te…
dk1844 Mar 8, 2022
4fd08e2
#1693 API v3: VersionedModelControllerV3 - post to yield location hea…
dk1844 Mar 21, 2022
5c8e005
#1693 API v3: VersionedModelControllerV3 - put to yield no content ->…
dk1844 Mar 23, 2022
458bfed
#1693 API v3: VersionedModelControllerV3 - IT for /ds/export (latest …
dk1844 Mar 23, 2022
a7a6cf9
#1693 API v3: VersionedModelControllerV3 - location header for put/po…
dk1844 Mar 23, 2022
af489b1
#1693 API v3: VersionedModelControllerV3 - location header for post-i…
dk1844 Mar 24, 2022
21f8008
#1693 API v3: VersionedModelControllerV3 -IT for GET datasets/{name},…
dk1844 Mar 24, 2022
6f63117
#1693 API v3: VersionedModelControllerV3 - IT for GET datasets/{name}…
dk1844 Mar 24, 2022
76e87ce
#1693 API v3: VersionedModelControllerV3 - GET /datasets/{name}/{vers…
dk1844 Mar 24, 2022
d62cd90
#1693 API v3: VersionedModelControllerV3 - common implementation of /…
dk1844 Mar 24, 2022
fc4c359
#1693 API v3: VersionedModelControllerV3 - /{name}/{version}/used-in …
dk1844 Mar 24, 2022
e6c8d8c
#1693 API v3: VersionedModelControllerV3 - GET/PUT /{name}/{version}/…
dk1844 Mar 29, 2022
3566df5
#1693 API v3: VersionedModelControllerV3 - GET/PUT /{name}/{version}/…
dk1844 Mar 30, 2022
219360a
#1693 Fix: (v2) VersionedModelController.getLatestVersionNumber(): Fu…
dk1844 Mar 30, 2022
112ac1e
#1693 VersionedModelControllerV3: `/{name}/{version}/validation` impl…
dk1844 Mar 30, 2022
101b37a
#1693 VersionedModelControllerV3 - IT extended for `POST /{name}/{ver…
dk1844 Mar 31, 2022
b934958
#1693 VersionedModelControllerV3 - conformance rule mgmt GET+POST dat…
dk1844 Apr 1, 2022
51ad013
#1693 PR review update - typos etc
dk1844 Apr 6, 2022
ba5edce
#1693 Validation-warnings as payload to caller in V3
dk1844 Apr 6, 2022
662119b
#1693 Validation-warnings as payload to caller in V3 - IT covered
dk1844 Apr 7, 2022
d480445
#1693 import validation check added to IT.
dk1844 Apr 7, 2022
3542a43
Merge branch 'develop-ver-3.0' into feature/1693-api-v3-versioned-model
dk1844 Apr 7, 2022
d6ef401
#1693 schema validation for DSs + IntegTest update
dk1844 Apr 11, 2022
a75ab85
#1693 Mt existence check for DS rules + reflecting in integTest
dk1844 Apr 11, 2022
9241eb1
Merge branch 'develop-ver-3.0' into feature/1693-api-v3-versioned-model
dk1844 Apr 14, 2022
09c326e
#1693 PR review update 1
dk1844 Apr 28, 2022
1c241d1
#1693 DatasetService.updateProperties[V3] -> split between DatasetSer…
dk1844 May 3, 2022
5d24819
#1693 PR review update: VersionedModelControllerV3 - GET /{name}/expo…
dk1844 May 4, 2022
7040694
#1693 VersionedMongoRepository - unused method `getAllVersionsValues`…
dk1844 May 4, 2022
e0fda73
#1693 for V3 API: VersionedSummary -> NamedLatestVersion: impl + IT
dk1844 May 4, 2022
a80d9ac
#1693 Swagger differs in paths based on `spring.profiles.active` bein…
dk1844 May 6, 2022
562da60
#1693 Swagger API: dev-profile: full v2+v3 API, non-dev: full v3 API
dk1844 May 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright 2018 ABSA Group Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package za.co.absa.enceladus.model.versionedModel

case class VersionList(_id: String, versions: Seq[Int])
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,5 @@
package za.co.absa.enceladus.model.versionedModel

case class VersionedSummary(_id: String, latestVersion: Int)
Zejnilovic marked this conversation as resolved.
Show resolved Hide resolved

benedeki marked this conversation as resolved.
Show resolved Hide resolved

Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,18 @@ class SpringFoxConfig extends ProjectMetadata {
}

private def filteredPaths: Predicate[String] =
or[String](regex("/api/dataset.*"), regex("/api/schema.*"),
or[String](
// api v2
regex("/api/dataset.*"), regex("/api/schema.*"),
regex("/api/mappingTable.*"), regex("/api/properties.*"),
regex("/api/monitoring.*"),regex("/api/runs.*"),
regex("/api/monitoring.*"), regex("/api/runs.*"),
regex("/api/user.*"), regex("/api/spark.*"),
regex("/api/configuration.*")
regex("/api/configuration.*"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not document v2 API as we do not want people to use it. Thoughts?

Maybe spring fox could have multiple profiles? Dev and non-dev

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with not documenting v2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I have changed the implementation to the following:

  • JVM being supplied -Dspring.profiles.active=dev will result in full V2 + V3 API as before,
  • by default (no special profile setup) will only allow Swagger to show API V3 + import/export methods of API v2 (these we consider public API in V2 already).

Hope this is satisfactory.

Showcase DEV -Dspring.profiles.active=dev

menas-api-dev

Showcase Non-DEV

menas-api-nondev

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redone due to #2046 (comment).
Menas DEV API remains unchanged, for Menas non-DEV API now only contains V3 API endpoints (i.e. V2 import/export removed)


// api v3
regex("/api-v3/datasets.*"), regex("/api-v3/schemas.*"),
regex("/api-v3/mapping-tables.*"), regex("/api-v3/property-definitions.*")

)

private def apiInfo =
Expand Down
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 Expand Up @@ -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 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the V2 and V3 suffixes are because of Spring autowiring?
(Looks ugly to me, to be honest 😄 )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this has nothing to do with spring autowiring. This is just a way to have V2 and V3 implementation in some services so that the v2 version can be removed easily when only v3 is to stay.

If we want to keep v2 and v3 long-term, this would require to create separate services, e.g. datasetService and datasetServiceV3. One of these solutions is, I believe, necessary if you want to keep the original implementation working.

It then begs the question - what is the intended lifetime of V2 API? Knowing this better, it could help to design better: copy-paste-and-change-a-few-things knowing that the V2 impl will be gone soon vs. keeping v3 extending v2 (no code duplication, but harder removal)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The v3 method moved to DatasetServiceV3, so now it is tidier. Thanks for the tip, @benedeki.

case None => throw notFound()
case Some(dataset) =>
val location: URI = new URI(s"/api/dataset/${dataset.name}/${dataset.version}")
Expand Down
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
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ class RestExceptionHandler {

private val logger = LoggerFactory.getLogger(this.getClass)

@ExceptionHandler(value = Array(classOf[IllegalArgumentException]))
def handleIllegalArgumentException(exception: IllegalArgumentException): ResponseEntity[Any] = {
ResponseEntity.badRequest().body(exception.getMessage)
}

@ExceptionHandler(value = Array(classOf[AsyncRequestTimeoutException]))
def handleAsyncRequestTimeoutException(exception: AsyncRequestTimeoutException): ResponseEntity[Any] = {
val message = Option(exception.getMessage).getOrElse("Request timeout expired.")
Expand Down
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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -29,6 +28,7 @@ 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._


abstract class VersionedModelController[C <: VersionedModel with Product with Auditable[C]](versionedModelService: VersionedModelService[C])
extends BaseController {

Expand Down Expand Up @@ -114,7 +114,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 @@ -130,7 +130,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 @@ -140,7 +140,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
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
* Copyright 2018 ABSA Group Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

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.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._

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: DatasetServiceV3)
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[Validation]] = {
forVersionExpression(name, version) { case (dsName, dsVersion) =>
datasetService.updateProperties(principal.getUsername, dsName, dsVersion, newProperties.toScalaMap).map {

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
createdWithNameVersionLocationBuilder(entity.name, entity.version, request, stripLastSegments = 3, suffix = "/properties")
.body(validation) // todo include in tests
case None => throw notFound()
}
}
}

// todo putIntoInfoFile switch needed?

@GetMapping(Array("/{name}/{version}/rules"))
@ResponseStatus(HttpStatus.OK)
def getConformanceRules(@PathVariable name: String,
@PathVariable version: String): CompletableFuture[Seq[ConformanceRule]] = {
forVersionExpression(name, version)(datasetService.getVersion).map {
case Some(entity) => entity.conformance
case None => throw notFound()
}
}

@PostMapping(Array("/{name}/{version}/rules"))
@ResponseStatus(HttpStatus.CREATED)
def addConformanceRule(@AuthenticationPrincipal user: UserDetails,
@PathVariable name: String,
@PathVariable version: String,
@RequestBody rule: ConformanceRule,
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, validation)) =>
val addedRuleOrder = updatedDs.conformance.last.order
createdWithNameVersionLocationBuilder(name, updatedDs.version, request, stripLastSegments = 3, // strip: /{name}/{version}/rules
suffix = s"/rules/$addedRuleOrder").body(validation)
case _ => throw notFound()
}
case None => throw notFound()
}
}

@GetMapping(Array("/{name}/{version}/rules/{order}"))
@ResponseStatus(HttpStatus.OK)
def getConformanceRuleByOrder(@PathVariable name: String,
@PathVariable version: String,
@PathVariable order: Int): CompletableFuture[ConformanceRule] = {
for {
optDs <- forVersionExpression(name, version)(datasetService.getVersion)
ds = optDs.getOrElse(throw notFound())
rule = ds.conformance.find(_.order == order).getOrElse(throw notFound())
} yield rule
}

}


Loading