MLT-41 Change handling of duplicate orders (#29)

Changed handling of duplicate orders

src/main/kotlin/no/nb/mlt/wls/order/controller/OrderController.kt

@@ -25,14 +25,34 @@ class OrderController(val orderService: OrderService) {
         """
     )
     @ApiResponses(
+        ApiResponse(
+            responseCode = "200",
+            description = """Order with given 'hostName' and 'hostOrderId' already exists in the system.
+                No new order was created, neither was the old order updated.
+                Existing order information is returned for inspection.
+                In rare cases the response body may be empty, that can happen if Hermes WLS does not
+                have the information about the order stored in its database and is unable to retrieve
+                the existing order information from the storage system.""",
+            content = [
+                Content(
+                    mediaType = "application/json",
+                    schema = Schema(implementation = ApiOrderPayload::class)
+                )
+            ]
+        ),
         ApiResponse(
             responseCode = "201",
-            description = "Created order for specified products to appropriate systems"
+            description = "Created order for specified products to appropriate systems",
+            content = [
+                Content(
+                    mediaType = "application/json",
+                    schema = Schema(implementation = ApiOrderPayload::class)
+                )
+            ]
         ),
         ApiResponse(
             responseCode = "400",
-            description =
-                """Order payload is invalid and was not created.
+            description = """Order payload is invalid and was not created.
                 An empty error message means the order already exists with the current ID.
                 Otherwise, the error message contains information about the invalid fields.""",
             content = [Content(schema = Schema())]

src/main/kotlin/no/nb/mlt/wls/order/service/OrderService.kt

@@ -39,12 +39,15 @@ class OrderService(val db: OrderRepository, val synqService: SynqOrderService) {
                 .awaitSingleOrNull()
 
         if (existingOrder != null) {
-            return ResponseEntity.badRequest().build()
+            return ResponseEntity.ok(existingOrder.toApiOrderPayload())
         }
 
         val synqResponse = synqService.createOrder(payload.toOrder().toSynqPayload())
-        // If SynQ didn't throw an error, but returned something that wasn't a new order,
-        // then it is likely some error or edge-case
+        // If SynQ returned a 200 OK then it means it exists from before, and we can return empty response (since we don't have any order info)
+        if (synqResponse.statusCode.isSameCodeAs(HttpStatus.OK)) {
+            return ResponseEntity.ok().build()
+        }
+        // If SynQ returned anything else than 200 or 201 it's an error
         if (!synqResponse.statusCode.isSameCodeAs(HttpStatus.CREATED)) {
             throw ServerErrorException("Unexpected error with SynQ", null)
         }

src/main/kotlin/no/nb/mlt/wls/order/service/SynqOrderService.kt

@@ -12,6 +12,8 @@ import org.springframework.stereotype.Service
 import org.springframework.web.reactive.function.BodyInserters
 import org.springframework.web.reactive.function.client.WebClient
 import org.springframework.web.reactive.function.client.WebClientResponseException
+import org.springframework.web.server.ServerErrorException
+import reactor.core.publisher.Mono
 import java.net.URI
 
 @Service
@@ -33,9 +35,18 @@ class SynqOrderService(
             .body(BodyInserters.fromValue(orders))
             .retrieve()
             .toEntity(SynqError::class.java)
-            .onErrorMap(WebClientResponseException::class.java) {
-                createServerError(it)
+            .onErrorResume(WebClientResponseException::class.java) { error ->
+                val errorText = error.getResponseBodyAs(SynqError::class.java)?.errorText
+                if (errorText != null && errorText.contains("Duplicate order")) {
+                    Mono.error(DuplicateOrderException(error))
+                } else {
+                    Mono.error(error)
+                }
             }
+            .onErrorMap(WebClientResponseException::class.java) { createServerError(it) }
+            .onErrorReturn(DuplicateOrderException::class.java, ResponseEntity.ok().build())
             .awaitSingle()
     }
 }
+
+class DuplicateOrderException(override val cause: Throwable) : ServerErrorException("Order already exists in SynQ", cause)

src/main/kotlin/no/nb/mlt/wls/product/controller/ProductController.kt

@@ -20,23 +20,21 @@ import org.springframework.web.bind.annotation.RestController
 class ProductController(val productService: ProductService) {
     @Operation(
         summary = "Register a product in the storage system",
-        description =
-            "Register data about the product in Hermes WLS and appropriate storage system, " +
-                "so that the physical product can be placed in the physical storage. " +
-                "NOTE: When registering new product quantity and location are set to default values (0.0 and null). " +
-                "Hence you should not provide these values in the payload, or at least know they will be overwritten."
+        description = """Register data about the product in Hermes WLS and appropriate storage system,
+            so that the physical product can be placed in the physical storage.
+            NOTE: When registering new product quantity and location are set to default values (0.0 and null).
+            Hence you should not provide these values in the payload, or at least know they will be overwritten."""
     )
     @ApiResponses(
         value = [
             ApiResponse(
                 responseCode = "200",
-                description =
-                    "Product with given 'hostName' and 'hostId' already exists in the system. " +
-                        "No new product was created, neither was the old product updated. " +
-                        "Existing product information is returned for inspection. " +
-                        "In rare cases the response body may be empty, that can happen if Hermes WLS " +
-                        "does not have the information about product stored in its database and is " +
-                        "unable to retrieve the existing product information from the storage system.",
+                description = """Product with given 'hostName' and 'hostId' already exists in the system.
+                    No new product was created, neither was the old product updated.
+                    Existing product information is returned for inspection.
+                    In rare cases the response body may be empty, that can happen if Hermes WLS
+                    does not have the information about the product stored in its database and
+                    is unable to retrieve the existing product information from the storage system.""",
                 content = [
                     Content(
                         mediaType = "application/json",
@@ -46,10 +44,9 @@ class ProductController(val productService: ProductService) {
             ),
             ApiResponse(
                 responseCode = "201",
-                description =
-                    "Product payload is valid and the product information was registered successfully. " +
-                        "Product was created in the appropriate storage system. " +
-                        "New product information is returned for inspection.",
+                description = """Product payload is valid and the product information was registered successfully.
+                    Product was created in the appropriate storage system.
+                    New product information is returned for inspection.""",
                 content = [
                     Content(
                         mediaType = "application/json",
@@ -59,9 +56,8 @@ class ProductController(val productService: ProductService) {
             ),
             ApiResponse(
                 responseCode = "400",
-                description =
-                    "Product payload is invalid and no new product was created. " +
-                        "Error message contains information about the invalid fields.",
+                description = """Product payload is invalid and no new product was created.
+                    Error message contains information about the invalid fields.""",
                 content = [Content(schema = Schema())]
             ),
             ApiResponse(

src/main/kotlin/no/nb/mlt/wls/product/service/ProductService.kt

@@ -36,6 +36,7 @@ class ProductService(val db: ProductRepository, val synqProductService: SynqProd
                     ServerErrorException("Failed to save product in the database", it)
                 }
                 .awaitSingleOrNull()
+
         if (existingProduct != null) {
             return ResponseEntity.ok(existingProduct.toApiPayload())
         }
@@ -47,9 +48,15 @@ class ProductService(val db: ProductRepository, val synqProductService: SynqProd
         val product = payload.toProduct().copy(quantity = 0.0, location = null)
 
         // Product service should create the product in the storage system, and return error message if it fails
-        if (synqProductService.createProduct(product.toSynqPayload()).statusCode.isSameCodeAs(HttpStatus.OK)) {
+        val synqResponse = synqProductService.createProduct(product.toSynqPayload())
+        // If SynQ returned a 200 OK then it means it exists from before, and we can return empty response (since we don't have any order info)
+        if (synqResponse.statusCode.isSameCodeAs(HttpStatus.OK)) {
             return ResponseEntity.ok().build()
         }
+        // If SynQ returned anything else than 200 or 201 it's an error
+        if (!synqResponse.statusCode.isSameCodeAs(HttpStatus.CREATED)) {
+            throw ServerErrorException("Unexpected error with SynQ", null)
+        }
 
         // Product service should save the product in the database, and return 500 if it fails
         db.save(product)

src/test/kotlin/no/nb/mlt/wls/order/controller/OrderControllerTest.kt

@@ -36,6 +36,7 @@ import org.springframework.http.ResponseEntity
 import org.springframework.security.test.context.support.WithMockUser
 import org.springframework.security.test.web.reactive.server.SecurityMockServerConfigurers.csrf
 import org.springframework.test.web.reactive.server.WebTestClient
+import org.springframework.test.web.reactive.server.expectBody
 import java.net.URI
 
 @EnableTestcontainers
@@ -92,19 +93,57 @@ class OrderControllerTest(
     @Test
     @WithMockUser
     fun `createOrder with duplicate payload returns OK`() {
-        // How to handle this? OK or error?
+        webTestClient
+            .mutateWith(csrf())
+            .post()
+            .uri("/batch/create")
+            .accept(MediaType.APPLICATION_JSON)
+            .bodyValue(duplicateOrderPayload)
+            .exchange()
+            .expectStatus().isOk
+            .expectBody<ApiOrderPayload>()
+            .consumeWith { response ->
+                assertThat(response.responseBody?.hostOrderId).isEqualTo(duplicateOrderPayload.hostOrderId)
+                assertThat(response.responseBody?.hostName).isEqualTo(duplicateOrderPayload.hostName)
+                assertThat(response.responseBody?.productLine).isEqualTo(duplicateOrderPayload.productLine)
+            }
     }
 
     @Test
     @WithMockUser
     fun `createOrder payload with different data but same ID returns DB entry`() {
-        // How to handle this? OK or error?
+        webTestClient
+            .mutateWith(csrf())
+            .post()
+            .uri("/batch/create")
+            .accept(MediaType.APPLICATION_JSON)
+            .bodyValue(
+                duplicateOrderPayload.copy(productLine = listOf(ProductLine("AAAAAAAAA", OrderLineStatus.PICKED)))
+            )
+            .exchange()
+            .expectStatus().isOk
+            .expectBody<ApiOrderPayload>()
+            .consumeWith { response ->
+                assertThat(response.responseBody?.productLine).isEqualTo(duplicateOrderPayload.productLine)
+            }
     }
 
     @Test
     @WithMockUser
-    fun `createOrder where SynQ says it's a duplicate returns OK`() {
-        // How to handle this? OK or error?
+    fun `createOrder where SynQ says it's a duplicate returns OK`() { // SynqService converts an error to return OK if it finds a duplicate product
+        coEvery {
+            synqOrderService.createOrder(any())
+        } returns ResponseEntity.ok().build()
+
+        webTestClient
+            .mutateWith(csrf())
+            .post()
+            .uri("/batch/create")
+            .accept(MediaType.APPLICATION_JSON)
+            .bodyValue(testOrderPayload)
+            .exchange()
+            .expectStatus().isOk
+            .expectBody().isEmpty
     }
 
     @Test