Skip to content

Commit

Permalink
#1693 API v3: VersionedModelControllerV3 - location header for put/po…
Browse files Browse the repository at this point in the history 
…st and post-import + IT to prove correct behavior
  • Loading branch information
@dk1844
dk1844 committed Mar 23, 2022
1 parent 458bfed commit a7a6cf9
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 59 deletions.
43 changes: 28 additions & 15 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 @@ -105,11 +105,17 @@ abstract class VersionedModelControllerV3[C <: VersionedModel with Product
@ResponseStatus(HttpStatus.CREATED)
def importSingleEntity(@AuthenticationPrincipal principal: UserDetails,
@PathVariable name: String,
@RequestBody importObject: ExportableObject[C]): CompletableFuture[C] = {
// todo check that the name pathVar and object conform
versionedModelService.importSingleItem(importObject.item, principal.getUsername, importObject.metadata).map {
case Some(entity) => entity // todo redo to have header Location present
case None => throw notFound()
@RequestBody importObject: ExportableObject[C],
request: HttpServletRequest): CompletableFuture[ResponseEntity[Nothing]] = {
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) =>
// 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)
case None => throw notFound()
}
}
}

Expand All @@ -125,14 +131,7 @@ abstract class VersionedModelControllerV3[C <: VersionedModel with Product
versionedModelService.create(item, principal.getUsername)
}
}.map {
case Some(entity) =>
val location: URI = ServletUriComponentsBuilder
.fromRequest(request)
.path("/{name}/{version}")
.buildAndExpand(entity.name, entity.version.toString)
.toUri() // will create location e.g. /api/dataset/MyExampleDataset/1

ResponseEntity.created(location).build()
case Some(entity) => createdWithNameVersionLocation(entity.name, entity.version, request)
case None => throw notFound()
}
}
Expand All @@ -142,15 +141,16 @@ abstract class VersionedModelControllerV3[C <: VersionedModel with Product
def edit(@AuthenticationPrincipal user: UserDetails,
@PathVariable name: String,
@PathVariable version: Int,
@RequestBody item: C): CompletableFuture[ResponseEntity[Nothing]] = {
@RequestBody item: C,
request: HttpServletRequest): CompletableFuture[ResponseEntity[Nothing]] = {

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) => ResponseEntity.noContent().build()
case Some(entity) => createdWithNameVersionLocation(entity.name, entity.version, request, stripLastSegments = 2)
case None => throw notFound()
}
}
Expand All @@ -169,4 +169,17 @@ abstract class VersionedModelControllerV3[C <: VersionedModel with Product
versionedModelService.disableVersion(name, v)
}

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

val location: URI = ServletUriComponentsBuilder.fromRequest(request)
.path(s"$strippingPrefix/{name}/{version}")
.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

ResponseEntity.created(location).build()
}

}
150 changes: 106 additions & 44 deletions ...a/enceladus/rest_api/integration/controllers/v3/DatasetControllerV3IntegrationSuite.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,11 @@ class DatasetControllerV3IntegrationSuite extends BaseRestApiTestV3 with BeforeA

val response = sendPost[Dataset, Dataset](apiUrl, bodyOpt = Some(dataset))
assertCreated(response)
val locationHeaders = response.getHeaders.get("location").asScala
locationHeaders should have size 1
val relativeLocation = stripBaseUrl(locationHeaders.head) // because locationHeader contains domain, port, etc.
val locationHeader = response.getHeaders.get("location").asScala.headOption
locationHeader shouldBe defined
locationHeader.get should endWith("/api-v3/datasets/dummyDs/1")

val relativeLocation = stripBaseUrl(locationHeader.get) // because locationHeader contains domain, port, etc.
val response2 = sendGet[Dataset](stripBaseUrl(relativeLocation))
assertOk(response2)

Expand Down Expand Up @@ -168,54 +169,21 @@ class DatasetControllerV3IntegrationSuite extends BaseRestApiTestV3 with BeforeA
)

val response = sendPut[Dataset, String](s"$apiUrl/datasetA/2", bodyOpt = Some(datasetA3))
response.getStatusCode shouldBe HttpStatus.NO_CONTENT
assertCreated(response)
val locationHeader = response.getHeaders.get("location").asScala.headOption
locationHeader shouldBe defined
locationHeader.get should endWith("/api-v3/datasets/datasetA/3")

val response2 = sendGet[Dataset](s"$apiUrl/datasetA/3") // next version
val relativeLocation = stripBaseUrl(locationHeader.get) // because locationHeader contains domain, port, etc.
val response2 = sendGet[Dataset](stripBaseUrl(relativeLocation))
assertOk(response2)

val actual = response2.getBody
val expected = toExpected(datasetA3.copy(version = 3, parent = Some(DatasetFactory.toParent(datasetA2)), properties = Some(Map("keyA" -> "valA", "keyB" -> "valB"))), actual) // blank property stripped
assert(actual == expected)
}
}
}

s"GET $apiUrl/{name}/export" should {
"return 404" when {
"when the name does not exist" in {
val response = sendGet[String](s"$apiUrl/notFoundDataset/export")
assertNotFound(response)
}
}

"return 200" when {
"there is a correct Dataset" should {
"return the exported Dataset representation for the latest version" in {
val dataset1 = DatasetFactory.getDummyDataset(name = "dataset")
val dataset2 = DatasetFactory.getDummyDataset(name = "dataset", version = 2, description = Some("Hi, I am the latest version"),
properties = Some(Map("key1" -> "val1", "key2" -> "val2")),
conformance = List(LiteralConformanceRule(0, "outputCol1", controlCheckpoint = false, "litValue1"))
)
datasetFixture.add(dataset1, dataset2)
val response = sendGet[String](s"$apiUrl/dataset/export")

assertOk(response)

val body = response.getBody
assert(body ==
"""{"metadata":{"exportVersion":1},"item":{
|"name":"dataset",
|"description":"Hi, I am the latest version",
|"hdfsPath":"/dummy/path",
|"hdfsPublishPath":"/dummy/publish/path",
|"schemaName":"dummySchema",
|"schemaVersion":1,
|"conformance":[{"_t":"LiteralConformanceRule","order":0,"outputColumn":"outputCol1","controlCheckpoint":false,"value":"litValue1"}],
|"properties":{"key2":"val2","key1":"val1"}
|}}""".stripMargin.replaceAll("[\\r\\n]", ""))
}
assert(actual == expected)
}
}

}

"return 405" when {
Expand All @@ -238,6 +206,100 @@ class DatasetControllerV3IntegrationSuite extends BaseRestApiTestV3 with BeforeA
}
}

s"GET $apiUrl/{name}/export" should {
"return 404" when {
"when the name does not exist" in {
val response = sendGet[String](s"$apiUrl/notFoundDataset/export")
assertNotFound(response)
}
}

"return 200" when {
"there is a correct Dataset" should {
"return the exported Dataset representation for the latest version" in {
val dataset1 = DatasetFactory.getDummyDataset(name = "dataset")
val dataset2 = DatasetFactory.getDummyDataset(name = "dataset", version = 2, description = Some("Hi, I am the latest version"),
properties = Some(Map("key1" -> "val1", "key2" -> "val2")),
conformance = List(LiteralConformanceRule(0, "outputCol1", controlCheckpoint = false, "litValue1"))
)
datasetFixture.add(dataset1, dataset2)
val response = sendGet[String](s"$apiUrl/dataset/export")

assertOk(response)

val body = response.getBody
assert(body ==
"""{"metadata":{"exportVersion":1},"item":{
|"name":"dataset",
|"description":"Hi, I am the latest version",
|"hdfsPath":"/dummy/path",
|"hdfsPublishPath":"/dummy/publish/path",
|"schemaName":"dummySchema",
|"schemaVersion":1,
|"conformance":[{"_t":"LiteralConformanceRule","order":0,"outputColumn":"outputCol1","controlCheckpoint":false,"value":"litValue1"}],
|"properties":{"key2":"val2","key1":"val1"}
|}}""".stripMargin.replaceAll("[\\r\\n]", ""))
}
}
}

}

s"POST $apiUrl/{name}/import" should {
val importableDs =
"""{"metadata":{"exportVersion":1},"item":{
|"name":"datasetXYZ",
|"description":"Hi, I am the latest version",
|"hdfsPath":"/dummy/path",
|"hdfsPublishPath":"/dummy/publish/path",
|"schemaName":"dummySchema",
|"schemaVersion":1,
|"conformance":[{"_t":"LiteralConformanceRule","order":0,"outputColumn":"outputCol1","controlCheckpoint":false,"value":"litValue1"}],
|"properties":{"key2":"val2","key1":"val1"}
|}}""".stripMargin.replaceAll("[\\r\\n]", "")

"return 405" when {
"a Dataset with the given name" should {
"fail when name in the URL and payload is mismatched" in {
val response = sendPost[String, String](s"$apiUrl/datasetABC/import",
bodyOpt = Some(importableDs))
response.getStatusCode shouldBe HttpStatus.BAD_REQUEST
response.getBody should include("name mismatch: 'datasetABC' != 'datasetXYZ'")
}
}
}

// "return 201" when { // todo create+update
// "there is a correct Dataset" should {
// "return the exported Dataset representation for the latest version" in {
// val dataset1 = DatasetFactory.getDummyDataset(name = "dataset")
// val dataset2 = DatasetFactory.getDummyDataset(name = "dataset", version = 2, description = Some("Hi, I am the latest version"),
// properties = Some(Map("key1" -> "val1", "key2" -> "val2")),
// conformance = List(LiteralConformanceRule(0, "outputCol1", controlCheckpoint = false, "litValue1"))
// )
// datasetFixture.add(dataset1, dataset2)
// val response = sendGet[String](s"$apiUrl/dataset/export")
//
// assertOk(response)
//
// val body = response.getBody
// assert(body ==
// """{"metadata":{"exportVersion":1},"item":{
// |"name":"dataset",
// |"description":"Hi, I am the latest version",
// |"hdfsPath":"/dummy/path",
// |"hdfsPublishPath":"/dummy/publish/path",
// |"schemaName":"dummySchema",
// |"schemaVersion":1,
// |"conformance":[{"_t":"LiteralConformanceRule","order":0,"outputColumn":"outputCol1","controlCheckpoint":false,"value":"litValue1"}],
// |"properties":{"key2":"val2","key1":"val1"}
// |}}""".stripMargin.replaceAll("[\\r\\n]", ""))
// }
// }
// }

}

s"GET $apiUrl/{name}/{version}/export" should {
"return 404" when {
"when the name+version does not exist" in {
Expand Down

0 comments on commit a7a6cf9

Please sign in to comment.