From 3a4285d9d37b307454356c4e6cf42b3e3326ef14 Mon Sep 17 00:00:00 2001 From: Daniel Aaron Salwerowicz Date: Tue, 20 Aug 2024 15:33:10 +0200 Subject: [PATCH] Changed handling of duplicate orders --- .../nb/mlt/wls/order/service/OrderService.kt | 11 +++-- .../mlt/wls/order/service/SynqOrderService.kt | 15 +++++- .../mlt/wls/product/service/ProductService.kt | 9 +++- .../order/controller/OrderControllerTest.kt | 47 +++++++++++++++++-- 4 files changed, 71 insertions(+), 11 deletions(-) diff --git a/src/main/kotlin/no/nb/mlt/wls/order/service/OrderService.kt b/src/main/kotlin/no/nb/mlt/wls/order/service/OrderService.kt index cc86cee5..de06434c 100644 --- a/src/main/kotlin/no/nb/mlt/wls/order/service/OrderService.kt +++ b/src/main/kotlin/no/nb/mlt/wls/order/service/OrderService.kt @@ -39,15 +39,18 @@ 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 (!synqResponse.statusCode.isSameCodeAs(HttpStatus.CREATED)) { + // If SynQ didn't throw an error, but returned 4xx/5xx, then it is likely some error or edge-case we haven't handled + if (synqResponse.statusCode.is4xxClientError || synqResponse.statusCode.is5xxServerError) { throw ServerErrorException("Unexpected error with SynQ", null) } + // 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() + } // Return what the database saved, as it could contain changes val order = diff --git a/src/main/kotlin/no/nb/mlt/wls/order/service/SynqOrderService.kt b/src/main/kotlin/no/nb/mlt/wls/order/service/SynqOrderService.kt index 12a6a446..740e6252 100644 --- a/src/main/kotlin/no/nb/mlt/wls/order/service/SynqOrderService.kt +++ b/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) diff --git a/src/main/kotlin/no/nb/mlt/wls/product/service/ProductService.kt b/src/main/kotlin/no/nb/mlt/wls/product/service/ProductService.kt index 7bf34168..ed9085fe 100644 --- a/src/main/kotlin/no/nb/mlt/wls/product/service/ProductService.kt +++ b/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,7 +48,13 @@ 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 didn't throw an error, but returned 4xx/5xx, then it is likely some error or edge-case we haven't handled + if (synqResponse.statusCode.is4xxClientError || synqResponse.statusCode.is5xxServerError) { + throw ServerErrorException("Unexpected error with SynQ", null) + } + // 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() } diff --git a/src/test/kotlin/no/nb/mlt/wls/order/controller/OrderControllerTest.kt b/src/test/kotlin/no/nb/mlt/wls/order/controller/OrderControllerTest.kt index ca13c930..9216703e 100644 --- a/src/test/kotlin/no/nb/mlt/wls/order/controller/OrderControllerTest.kt +++ b/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() + .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() + .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