Bugfix: Blueprints Processor always return 200 OK 75/87875/1
authorottero <rodrigo.ottero@est.tech>
Thu, 16 May 2019 09:00:07 +0000 (09:00 +0000)
committerDan Timoney <dtimoney@att.com>
Thu, 16 May 2019 13:22:34 +0000 (13:22 +0000)
Currently the Blueprints Processor mS replies with a 200 OK HTTP status
code even if an exception occurs in the server side while executing the
request.

Thus, the only way for a REST client to determine if the request was
successful or not is by analysing the response and evaluate the
content of the element status.code

This bugfix modifies the HTTP status code of the response to match the
one inside the response.

Issue-ID: CCSDK-1327
Change-Id: I05a58cb3ab9359319172f2d8f1a665fdcdc1230b
Signed-off-by: ottero <rodrigo.ottero@est.tech>
(cherry picked from commit 5801ed7abaa58f7ef728ad68c496a3780d438194)

ms/blueprintsprocessor/modules/inbounds/selfservice-api/src/main/kotlin/org/onap/ccsdk/cds/blueprintsprocessor/selfservice/api/ExecutionServiceController.kt
ms/blueprintsprocessor/modules/inbounds/selfservice-api/src/main/kotlin/org/onap/ccsdk/cds/blueprintsprocessor/selfservice/api/utils/Utils.kt
ms/blueprintsprocessor/modules/inbounds/selfservice-api/src/test/kotlin/org/onap/ccsdk/cds/blueprintsprocessor/selfservice/api/ExecutionServiceHandlerTest.kt
ms/blueprintsprocessor/modules/inbounds/selfservice-api/src/test/kotlin/org/onap/ccsdk/cds/blueprintsprocessor/selfservice/api/utils/Utils.kt [new file with mode: 0644]
ms/blueprintsprocessor/modules/inbounds/selfservice-api/src/test/resources/execution-input/faulty-input.json [new file with mode: 0644]

index 7cbc895..eff9773 100644 (file)
@@ -22,9 +22,12 @@ import kotlinx.coroutines.runBlocking
 import org.onap.ccsdk.cds.blueprintsprocessor.core.api.data.ACTION_MODE_ASYNC
 import org.onap.ccsdk.cds.blueprintsprocessor.core.api.data.ExecutionServiceInput
 import org.onap.ccsdk.cds.blueprintsprocessor.core.api.data.ExecutionServiceOutput
+import org.onap.ccsdk.cds.blueprintsprocessor.selfservice.api.utils.determineHttpStatusCode
 import org.onap.ccsdk.cds.controllerblueprints.core.BluePrintException
 import org.springframework.beans.factory.annotation.Autowired
+import org.springframework.http.HttpStatus
 import org.springframework.http.MediaType
+import org.springframework.http.ResponseEntity
 import org.springframework.http.codec.multipart.FilePart
 import org.springframework.security.access.prepost.PreAuthorize
 import org.springframework.web.bind.annotation.*
@@ -63,10 +66,11 @@ open class ExecutionServiceController {
             notes = "Takes the blueprint information and process as per the payload")
     @ResponseBody
     @PreAuthorize("hasRole('USER')")
-    fun process(@RequestBody executionServiceInput: ExecutionServiceInput): ExecutionServiceOutput = runBlocking {
+    fun process(@RequestBody executionServiceInput: ExecutionServiceInput): ResponseEntity<ExecutionServiceOutput> = runBlocking {
         if (executionServiceInput.actionIdentifiers.mode == ACTION_MODE_ASYNC) {
             throw IllegalStateException("Can't process async request through the REST endpoint. Use gRPC for async processing.")
         }
-        executionServiceHandler.doProcess(executionServiceInput)
+        val processResult = executionServiceHandler.doProcess(executionServiceInput)
+        ResponseEntity(processResult, determineHttpStatusCode(processResult.status.code))
     }
 }
index 32be3e0..2cf1c1d 100644 (file)
@@ -16,6 +16,7 @@
 package org.onap.ccsdk.cds.blueprintsprocessor.selfservice.api.utils
 
 import org.onap.ccsdk.cds.controllerblueprints.core.BluePrintException
+import org.springframework.http.HttpStatus
 import org.springframework.http.codec.multipart.FilePart
 import org.springframework.util.StringUtils
 import java.io.File
@@ -26,6 +27,7 @@ import java.time.ZoneId
 import java.time.format.DateTimeFormatter
 import java.util.*
 
+const val INTERNAL_SERVER_ERROR_HTTP_STATUS_CODE = 500
 
 fun currentTimestamp(): String {
     val now = LocalDateTime.now(ZoneId.systemDefault())
@@ -56,4 +58,15 @@ fun saveCBAFile(filePart: FilePart, targetDirectory: Path): Path {
     filePart.transferTo(file)
 
     return targetLocation
+}
+
+fun determineHttpStatusCode(statusCode: Int): HttpStatus {
+
+    try {
+        return HttpStatus.valueOf(statusCode)
+    } catch (exception: Exception) {
+        //if statusCode cannot be converted to a proper HttpStatus, the resource still needs to assign a HTTP status
+        // code to the response. In this case, a 500 Internal Server Error will be returned as default.
+        return HttpStatus.valueOf(INTERNAL_SERVER_ERROR_HTTP_STATUS_CODE)
+    }
 }
\ No newline at end of file
index d14761c..9cbd898 100644 (file)
@@ -38,6 +38,7 @@ import org.springframework.test.context.junit4.SpringRunner
 import org.springframework.test.web.reactive.server.WebTestClient
 import org.springframework.test.web.reactive.server.returnResult
 import org.springframework.web.reactive.function.BodyInserters
+import java.io.File
 import java.nio.file.Files
 import java.nio.file.Paths
 import java.util.*
@@ -71,11 +72,8 @@ class ExecutionServiceHandlerTest {
     @Test
     fun `test rest upload blueprint`() {
         runBlocking {
-            val file = Paths.get("./src/test/resources/test-cba.zip").toFile()
-            assertTrue(file.exists(), "couldn't get file ${file.absolutePath}")
-
             val body = MultipartBodyBuilder().apply {
-                part("file", object : ByteArrayResource(Files.readAllBytes(Paths.get("./src/test/resources/test-cba.zip"))) {
+                part("file", object : ByteArrayResource(Files.readAllBytes(loadTestCbaFile().toPath())) {
                     override fun getFilename(): String {
                         return "test-cba.zip"
                     }
@@ -98,9 +96,7 @@ class ExecutionServiceHandlerTest {
     @Test
     fun `test rest process`() {
         runBlocking {
-            val file = Paths.get("./src/test/resources/test-cba.zip").toFile()
-            assertTrue(file.exists(), "couldnt get file ${file.absolutePath}")
-            blueprintCatalog.saveToDatabase(UUID.randomUUID().toString(), file)
+            blueprintCatalog.saveToDatabase(UUID.randomUUID().toString(), loadTestCbaFile())
 
             val executionServiceInput = JacksonUtils
                     .readValueFromClassPathFile("execution-input/default-input.json",
@@ -114,4 +110,28 @@ class ExecutionServiceHandlerTest {
                     .expectStatus().isOk
         }
     }
+
+    @Test
+    fun `rest resource process should return status code 500 in case of server-side exception`() {
+        runBlocking {
+            blueprintCatalog.saveToDatabase(UUID.randomUUID().toString(), loadTestCbaFile())
+
+            val executionServiceInput = JacksonUtils
+                    .readValueFromClassPathFile("execution-input/faulty-input.json",
+                            ExecutionServiceInput::class.java)!!
+
+            webTestClient
+                    .post()
+                    .uri("/api/v1/execution-service/process")
+                    .body(BodyInserters.fromObject(executionServiceInput))
+                    .exchange()
+                    .expectStatus().is5xxServerError
+        }
+    }
+
+    private fun loadTestCbaFile(): File {
+        val testCbaFile = Paths.get("./src/test/resources/test-cba.zip").toFile()
+        assertTrue(testCbaFile.exists(), "couldn't get file ${testCbaFile.absolutePath}")
+        return testCbaFile
+    }
 }
\ No newline at end of file
diff --git a/ms/blueprintsprocessor/modules/inbounds/selfservice-api/src/test/kotlin/org/onap/ccsdk/cds/blueprintsprocessor/selfservice/api/utils/Utils.kt b/ms/blueprintsprocessor/modules/inbounds/selfservice-api/src/test/kotlin/org/onap/ccsdk/cds/blueprintsprocessor/selfservice/api/utils/Utils.kt
new file mode 100644 (file)
index 0000000..07d8ca4
--- /dev/null
@@ -0,0 +1,27 @@
+package org.onap.ccsdk.cds.blueprintsprocessor.selfservice.api.utils
+
+import org.junit.Assert.assertEquals
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.springframework.http.HttpStatus
+import org.springframework.test.context.junit4.SpringRunner
+
+@RunWith(SpringRunner::class)
+class UtilsTest {
+
+    @Test
+    fun `valid Http status codes should be produced for valid parameters`() {
+        val httpStatusCode200 = determineHttpStatusCode(200)
+        assertEquals(HttpStatus.OK, httpStatusCode200)
+
+        val httpStatusCode500 = determineHttpStatusCode(500)
+        assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, httpStatusCode500)
+    }
+
+    @Test
+    fun `Http status code 500 should be produced for any invalid parameter`() {
+        val nonExistentHttpStatusCode = determineHttpStatusCode(999999)
+        assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, nonExistentHttpStatusCode)
+    }
+
+}
\ No newline at end of file
diff --git a/ms/blueprintsprocessor/modules/inbounds/selfservice-api/src/test/resources/execution-input/faulty-input.json b/ms/blueprintsprocessor/modules/inbounds/selfservice-api/src/test/resources/execution-input/faulty-input.json
new file mode 100644 (file)
index 0000000..999039e
--- /dev/null
@@ -0,0 +1,15 @@
+{
+  "commonHeader": {
+    "originatorId": "System",
+    "requestId": "1234",
+    "subRequestId": "1234-12234"
+  },
+  "actionIdentifiers": {
+    "blueprintName": "baseconfiguration",
+    "blueprintVersion": "1.0.0",
+    "actionName": "activate",
+    "mode": "sync"
+  },
+  "payload": {
+  }
+}
\ No newline at end of file