diff --git a/src/itest/resources/features/retry/RetriesAndErrors.feature b/src/itest/resources/features/retry/RetriesAndErrors.feature index 9ca7cfc..a40012c 100644 --- a/src/itest/resources/features/retry/RetriesAndErrors.feature +++ b/src/itest/resources/features/retry/RetriesAndErrors.feature @@ -13,7 +13,7 @@ Feature: Retries and Errors Scenario: Process message when the data api returns 400 Given the application is running When the consumer receives a message but the data api returns a 400 - Then the message should be moved to topic disqualified-officers-delta-error + Then the message should be moved to topic disqualified-officers-delta-invalid Scenario: Process message when the data api returns 503 Given the application is running diff --git a/src/main/java/uk/gov/companieshouse/disqualifiedofficers/delta/handler/ApiResponseHandler.java b/src/main/java/uk/gov/companieshouse/disqualifiedofficers/delta/handler/ApiResponseHandler.java deleted file mode 100644 index b9c9953..0000000 --- a/src/main/java/uk/gov/companieshouse/disqualifiedofficers/delta/handler/ApiResponseHandler.java +++ /dev/null @@ -1,36 +0,0 @@ -package uk.gov.companieshouse.disqualifiedofficers.delta.handler; - -import java.util.Map; -import org.springframework.http.HttpStatus; -import uk.gov.companieshouse.disqualifiedofficers.delta.exception.NonRetryableErrorException; -import uk.gov.companieshouse.disqualifiedofficers.delta.exception.RetryableErrorException; -import uk.gov.companieshouse.logging.Logger; - -public class ApiResponseHandler { - - /** - * Handle responses by logging result and throwing any exceptions. - */ - public void handleResponse( - final HttpStatus httpStatus, - final String logContext, - final Map logMap, - Logger logger) - throws NonRetryableErrorException, RetryableErrorException { - logMap.put("status", httpStatus.toString()); - if (HttpStatus.BAD_REQUEST == httpStatus) { - // 400 BAD REQUEST status cannot be retried - String msg = "400 BAD_REQUEST response received from disqualified-officers-data-api"; - logger.errorContext(logContext, msg, null, logMap); - throw new NonRetryableErrorException(msg); - } else if (!httpStatus.is2xxSuccessful()) { - // any other client or server status can be retried - String msg = "Non-Successful response received from disqualified-officers-data-api"; - logger.errorContext(logContext, msg + ", retry", null, logMap); - throw new RetryableErrorException(msg); - } else { - logger.debugContext(logContext, - "Response received from disqualified-officers-data-api", logMap); - } - } -} diff --git a/src/main/java/uk/gov/companieshouse/disqualifiedofficers/delta/processor/DisqualifiedOfficersDeltaProcessor.java b/src/main/java/uk/gov/companieshouse/disqualifiedofficers/delta/processor/DisqualifiedOfficersDeltaProcessor.java index a27a147..27602d0 100644 --- a/src/main/java/uk/gov/companieshouse/disqualifiedofficers/delta/processor/DisqualifiedOfficersDeltaProcessor.java +++ b/src/main/java/uk/gov/companieshouse/disqualifiedofficers/delta/processor/DisqualifiedOfficersDeltaProcessor.java @@ -6,7 +6,6 @@ import java.util.Map; import java.util.Objects; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.http.HttpStatus; import org.springframework.kafka.support.KafkaHeaders; import org.springframework.messaging.Message; import org.springframework.messaging.MessageHeaders; @@ -20,7 +19,6 @@ import uk.gov.companieshouse.delta.ChsDelta; import uk.gov.companieshouse.disqualifiedofficers.delta.exception.NonRetryableErrorException; import uk.gov.companieshouse.disqualifiedofficers.delta.exception.RetryableErrorException; -import uk.gov.companieshouse.disqualifiedofficers.delta.handler.ApiResponseHandler; import uk.gov.companieshouse.disqualifiedofficers.delta.service.api.ApiClientService; import uk.gov.companieshouse.disqualifiedofficers.delta.transformer.DisqualifiedOfficersApiTransformer; import uk.gov.companieshouse.logging.Logger; @@ -109,14 +107,13 @@ private void invokeDisqualificationsDataApi(final String logContext, logContext, String.format("Process disqualification for officer with id [%s]", disqualification.getOfficerId()), - null); + logMap); final ApiResponse response = apiClientService.putDisqualification(logContext, internalDisqualificationApi.getInternalData().getOfficerId(), internalDisqualificationApi); - ApiResponseHandler apiResponseHandler = new ApiResponseHandler(); - apiResponseHandler.handleResponse(HttpStatus.valueOf(response.getStatusCode()), - logContext, logMap, logger); + logger.debugContext(logContext, + "Response received from disqualified-officers-data-api", logMap); } private void invokeDisqualificationsDataApi(final String logContext, @@ -127,13 +124,12 @@ private void invokeDisqualificationsDataApi(final String logContext, logContext, String.format("Process disqualification for officer with id [%s]", disqualification.getOfficerId()), - null); + logMap); final ApiResponse response = apiClientService.putDisqualification(logContext, internalDisqualificationApi.getInternalData().getOfficerId(), internalDisqualificationApi); - ApiResponseHandler apiResponseHandler = new ApiResponseHandler(); - apiResponseHandler.handleResponse(HttpStatus.valueOf(response.getStatusCode()), - logContext, logMap, logger); + logger.debugContext(logContext, + "Response received from disqualified-officers-data-api", logMap); } } \ No newline at end of file diff --git a/src/main/java/uk/gov/companieshouse/disqualifiedofficers/delta/service/api/BaseApiClientServiceImpl.java b/src/main/java/uk/gov/companieshouse/disqualifiedofficers/delta/service/api/BaseApiClientServiceImpl.java index b303971..314d07c 100644 --- a/src/main/java/uk/gov/companieshouse/disqualifiedofficers/delta/service/api/BaseApiClientServiceImpl.java +++ b/src/main/java/uk/gov/companieshouse/disqualifiedofficers/delta/service/api/BaseApiClientServiceImpl.java @@ -8,6 +8,8 @@ import uk.gov.companieshouse.api.handler.Executor; import uk.gov.companieshouse.api.handler.exception.URIValidationException; import uk.gov.companieshouse.api.model.ApiResponse; +import uk.gov.companieshouse.disqualifiedofficers.delta.exception.NonRetryableErrorException; +import uk.gov.companieshouse.disqualifiedofficers.delta.exception.RetryableErrorException; import uk.gov.companieshouse.logging.Logger; public abstract class BaseApiClientServiceImpl { @@ -41,15 +43,25 @@ public ApiResponse executeOp(final String logContext, return executor.execute(); } catch (URIValidationException ex) { - logger.errorContext(logContext, "SDK exception", ex, logMap); + String msg = "404 NOT_FOUND response received from disqualified-officers-data-api"; + logger.errorContext(logContext, msg, ex, logMap); - throw new ResponseStatusException(HttpStatus.NOT_FOUND, ex.getMessage(), ex); + throw new RetryableErrorException(msg, ex); } catch (ApiErrorResponseException ex) { logMap.put("status", ex.getStatusCode()); - logger.errorContext(logContext, "SDK exception", ex, logMap); - throw new ResponseStatusException(HttpStatus.valueOf(ex.getStatusCode()), - ex.getStatusMessage(), ex); + if (ex.getStatusCode() == HttpStatus.BAD_REQUEST.value()) { + // 400 BAD REQUEST status cannot be retried + String msg = + "400 BAD_REQUEST response received from disqualified-officers-data-api"; + logger.errorContext(logContext, msg, ex, logMap); + throw new NonRetryableErrorException(msg, ex); + } + + // any other client or server status is retryable + String msg = "Non-Successful response received from disqualified-officers-data-api"; + logger.errorContext(logContext, msg + ", retry", ex, logMap); + throw new RetryableErrorException(msg, ex); } } } diff --git a/src/test/java/uk/gov/companieshouse/disqualifiedofficers/delta/handler/ApiResponseHandlerTest.java b/src/test/java/uk/gov/companieshouse/disqualifiedofficers/delta/handler/ApiResponseHandlerTest.java deleted file mode 100644 index 5a1a64a..0000000 --- a/src/test/java/uk/gov/companieshouse/disqualifiedofficers/delta/handler/ApiResponseHandlerTest.java +++ /dev/null @@ -1,64 +0,0 @@ -package uk.gov.companieshouse.disqualifiedofficers.delta.handler; - -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; -import org.springframework.http.HttpStatus; -import uk.gov.companieshouse.disqualifiedofficers.delta.exception.NonRetryableErrorException; -import uk.gov.companieshouse.disqualifiedofficers.delta.exception.RetryableErrorException; -import uk.gov.companieshouse.logging.Logger; -import java.util.HashMap; -import java.util.Map; -import static org.mockito.Mockito.verify; -import static org.junit.Assert.assertThrows; - - -@ExtendWith(MockitoExtension.class) -class ApiResponseHandlerTest { - - private ApiResponseHandler apiResponseHandler = new ApiResponseHandler(); - - @Mock - private Logger logger; - - @Test - void handle200Response() throws NonRetryableErrorException, RetryableErrorException { - HttpStatus httpStatus = HttpStatus.OK; - Map logMap = new HashMap<>(); - - apiResponseHandler.handleResponse( - httpStatus,"status", logMap, logger ); - verify(logger).debugContext("status", "Response received from disqualified-officers-data-api", logMap); - } - - @Test - void handleBadResponse() throws NonRetryableErrorException, RetryableErrorException { - HttpStatus httpStatus = HttpStatus.BAD_REQUEST; - Map logMap = new HashMap<>(); - assertThrows(NonRetryableErrorException.class, () -> apiResponseHandler.handleResponse( - httpStatus, "status", logMap, logger)); - verify(logger).errorContext("status", - "400 BAD_REQUEST response received from disqualified-officers-data-api", null, logMap); - } - - @Test - void handle404Response() throws NonRetryableErrorException, RetryableErrorException { - HttpStatus httpStatus = HttpStatus.NOT_FOUND; - Map logMap = new HashMap<>(); - assertThrows(RetryableErrorException.class, () -> apiResponseHandler.handleResponse( - httpStatus, "status", logMap, logger)); - verify(logger).errorContext("status", - "Non-Successful response received from disqualified-officers-data-api, retry", null, logMap); - } - - @Test - void handle500Response() throws NonRetryableErrorException, RetryableErrorException { - HttpStatus httpStatus = HttpStatus.INTERNAL_SERVER_ERROR; - Map logMap = new HashMap<>(); - assertThrows(RetryableErrorException.class, () -> apiResponseHandler.handleResponse( - httpStatus, "status", logMap, logger)); - verify(logger).errorContext("status", - "Non-Successful response received from disqualified-officers-data-api, retry", null, logMap); - } -} diff --git a/src/test/java/uk/gov/companieshouse/disqualifiedofficers/delta/processor/DisqualifiedOfficersProcessorTest.java b/src/test/java/uk/gov/companieshouse/disqualifiedofficers/delta/processor/DisqualifiedOfficersProcessorTest.java index 2fd53e5..b4ab25a 100644 --- a/src/test/java/uk/gov/companieshouse/disqualifiedofficers/delta/processor/DisqualifiedOfficersProcessorTest.java +++ b/src/test/java/uk/gov/companieshouse/disqualifiedofficers/delta/processor/DisqualifiedOfficersProcessorTest.java @@ -77,7 +77,7 @@ void When_ApiReturns500_Expect_RetryableError() throws IOException { Message mockChsDeltaMessage = testHelper.createChsDeltaMessage(); InternalNaturalDisqualificationApi apiObject = testHelper.createDisqualificationApi(); final ApiResponse response = new ApiResponse<>(HttpStatus.INTERNAL_SERVER_ERROR.value(), null, null); - when(apiClientService.putDisqualification(any(),any(), eq(apiObject))).thenReturn(response); + when(apiClientService.putDisqualification(any(),any(), eq(apiObject))).thenThrow(new RetryableErrorException("")); when(transformer.transformNaturalDisqualification(any())).thenReturn(apiObject); assertThrows(RetryableErrorException.class, ()->deltaProcessor.processDelta(mockChsDeltaMessage)); } diff --git a/src/test/java/uk/gov/companieshouse/disqualifiedofficers/delta/service/api/BaseApiClientServiceImplTest.java b/src/test/java/uk/gov/companieshouse/disqualifiedofficers/delta/service/api/BaseApiClientServiceImplTest.java new file mode 100644 index 0000000..38acacf --- /dev/null +++ b/src/test/java/uk/gov/companieshouse/disqualifiedofficers/delta/service/api/BaseApiClientServiceImplTest.java @@ -0,0 +1,80 @@ +package uk.gov.companieshouse.disqualifiedofficers.delta.service.api; + +import com.google.api.client.http.HttpHeaders; +import com.google.api.client.http.HttpResponseException.Builder; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import uk.gov.companieshouse.api.error.ApiErrorResponseException; +import uk.gov.companieshouse.api.handler.Executor; +import uk.gov.companieshouse.api.handler.exception.URIValidationException; +import uk.gov.companieshouse.api.model.ApiResponse; +import uk.gov.companieshouse.disqualifiedofficers.delta.exception.NonRetryableErrorException; +import uk.gov.companieshouse.disqualifiedofficers.delta.exception.RetryableErrorException; +import uk.gov.companieshouse.logging.Logger; + +import java.util.HashMap; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertThrows; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +public class BaseApiClientServiceImplTest { + + private BaseApiClientServiceImpl service; + + @Mock + private Logger logger; + @Mock + private Executor executor; + + @BeforeEach + void setup() { + service = new BaseApiClientServiceImpl(logger) {}; + } + + @Test + void returnsApiResponse() throws Exception { + ApiResponse expectedResponse = new ApiResponse(200, new HashMap<>()); + when(executor.execute()).thenReturn(expectedResponse); + + ApiResponse actualResponse = service.executeOp(null, null, null, executor); + + assertThat(actualResponse).isEqualTo(expectedResponse); + } + + @Test + void throwsRetryableErrorOn404() throws Exception { + when(executor.execute()).thenThrow(new URIValidationException("Not Found")); + + RetryableErrorException thrown = assertThrows(RetryableErrorException.class, + () -> service.executeOp(null, null, null, executor)); + + assertThat(thrown.getMessage()).isEqualTo("404 NOT_FOUND response received from disqualified-officers-data-api"); + } + + @Test + void throwsRetryableErrorOn500() throws Exception { + when(executor.execute()).thenThrow( + new ApiErrorResponseException(new Builder(500, "500", new HttpHeaders()))); + + RetryableErrorException thrown = assertThrows(RetryableErrorException.class, + () -> service.executeOp(null, null, null, executor)); + + assertThat(thrown.getMessage()).isEqualTo("Non-Successful response received from disqualified-officers-data-api"); + } + + @Test + void throwsNonRetryableErrorOn400() throws Exception { + when(executor.execute()).thenThrow( + new ApiErrorResponseException(new Builder(400, "400", new HttpHeaders()))); + + NonRetryableErrorException thrown = assertThrows(NonRetryableErrorException.class, + () -> service.executeOp(null, null, null, executor)); + + assertThat(thrown.getMessage()).isEqualTo("400 BAD_REQUEST response received from disqualified-officers-data-api"); + } +}