Merge pull request #42 from companieshouse/feature/retry-integration-…

…tests Throw correct error on api status response error
companieshouse · May 26, 2022 · 279eb72 · 279eb72
2 parents 7f3af07 + 5269fcb
commit 279eb72
Show file tree

Hide file tree

Showing 7 changed files with 105 additions and 117 deletions.
diff --git 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/...ain/java/uk/gov/companieshouse/disqualifiedofficers/delta/handler/ApiResponseHandler.java b/...ain/java/uk/gov/companieshouse/disqualifiedofficers/delta/handler/ApiResponseHandler.java
diff --git a/...mpanieshouse/disqualifiedofficers/delta/processor/DisqualifiedOfficersDeltaProcessor.java b/...mpanieshouse/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<Void> 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<Void> 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);
     }
 }
diff --git a/...k/gov/companieshouse/disqualifiedofficers/delta/service/api/BaseApiClientServiceImpl.java b/...k/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 <T> ApiResponse<T> 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/...java/uk/gov/companieshouse/disqualifiedofficers/delta/handler/ApiResponseHandlerTest.java b/...java/uk/gov/companieshouse/disqualifiedofficers/delta/handler/ApiResponseHandlerTest.java
diff --git a/...ompanieshouse/disqualifiedofficers/delta/processor/DisqualifiedOfficersProcessorTest.java b/...ompanieshouse/disqualifiedofficers/delta/processor/DisqualifiedOfficersProcessorTest.java
@@ -77,7 +77,7 @@ void When_ApiReturns500_Expect_RetryableError() throws IOException {
         Message<ChsDelta> mockChsDeltaMessage = testHelper.createChsDeltaMessage();
         InternalNaturalDisqualificationApi apiObject = testHelper.createDisqualificationApi();
         final ApiResponse<Void> 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/...v/companieshouse/disqualifiedofficers/delta/service/api/BaseApiClientServiceImplTest.java b/...v/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");
+    }
+}