Handle 204 from BIP (#2601)

204 handling to ep-merge, svc-bip-api, and mock-bip-claims-api, and catch all errors on dd submit metrics/distribution calls so jobs aren't interrupted.
department-of-veterans-affairs · Feb 15, 2024 · f72ab81 · f72ab81
1 parent 37055c4
commit f72ab81
Show file tree

Hide file tree

Showing 11 changed files with 102 additions and 14 deletions.
diff --git a/domain-ee/ee-ep-merge-app/integration/test_merge_request.py b/domain-ee/ee-ep-merge-app/integration/test_merge_request.py
@@ -7,6 +7,7 @@
 RESPONSE_DIR = './tests/responses'
 response_200 = f'{RESPONSE_DIR}/200_response.json'
 response_201 = f'{RESPONSE_DIR}/201_response.json'
+response_204 = f'{RESPONSE_DIR}/204_response.json'
 response_404 = f'{RESPONSE_DIR}/404_response.json'
 response_400 = f'{RESPONSE_DIR}/400_response.json'
 response_500 = f'{RESPONSE_DIR}/500_response.json'
@@ -92,6 +93,19 @@ async def test_completed_success_with_duplicate_contention(
             response = await submit_request_and_process(client)
             assert_successful_response(response)
 
+    @pytest.mark.asyncio(scope="session")
+    async def test_completed_no_ep400_contentions(
+            self, get_claim_endpoint: MqEndpoint, get_claim_contentions_endpoint: MqEndpoint, put_tsoj_endpoint: MqEndpoint, cancel_claim_endpoint: MqEndpoint
+    ):
+        get_claim_endpoint.set_responses([pending_claim_200])
+        get_claim_contentions_endpoint.set_responses([pending_contentions_200, response_204])
+        put_tsoj_endpoint.set_responses([response_200])
+        cancel_claim_endpoint.set_responses([response_200])
+
+        async with AsyncClient(app=app, base_url="http://test") as client:
+            response = await submit_request_and_process(client)
+            assert_successful_response(response)
+
 
 class TestErrorAtGetPendingClaim(TestMergeRequestBase):
 

diff --git a/domain-ee/ee-ep-merge-app/src/python_src/service/ep_merge_machine.py b/domain-ee/ee-ep-merge-app/src/python_src/service/ep_merge_machine.py
@@ -168,7 +168,7 @@ def on_get_pending_contentions(self, event):
     @running_get_ep400_contentions.enter
     def on_get_ep400_contentions(self, event, pending_contentions_response=None):
         request = get_contentions.Request(claim_id=self.job.ep400_claim_id)
-        response = self.make_request(request=request, hoppy_client=HOPPY.get_client(ClientName.GET_CLAIM_CONTENTIONS), response_type=get_contentions.Response)
+        response = self.make_request(request=request, hoppy_client=HOPPY.get_client(ClientName.GET_CLAIM_CONTENTIONS), response_type=get_contentions.Response, expected_statuses=[200, 204])
         self.send(event=event, pending_contentions_response=pending_contentions_response, ep400_contentions_response=response)
 
     @running_set_temp_station_of_jurisdiction.enter
@@ -191,7 +191,7 @@ def on_merge_contentions(self, event, pending_contentions_response=None, ep400_c
     def on_move_contentions_to_pending_claim(self, event, new_contentions=None, ep400_contentions_response=None):
         request = create_contentions.Request(claim_id=self.job.pending_claim_id, create_contentions=new_contentions)
         self.make_request(
-            request=request, hoppy_client=HOPPY.get_client(ClientName.CREATE_CLAIM_CONTENTIONS), response_type=create_contentions.Response, expected_status=201
+            request=request, hoppy_client=HOPPY.get_client(ClientName.CREATE_CLAIM_CONTENTIONS), response_type=create_contentions.Response, expected_statuses=201
         )
         self.send(event=event, ep400_contentions_response=ep400_contentions_response)
 
@@ -291,13 +291,15 @@ def log_metrics(self, job_duration):
                 if self.skipped_merge:
                     increment(JOB_SKIPPED_MERGE_METRIC)
 
-    def make_request(self, request: GeneralRequest, hoppy_client: AsyncHoppyClient, response_type: Type[GeneralResponse], expected_status: int = 200):
+    def make_request(self, request: GeneralRequest, hoppy_client: AsyncHoppyClient, response_type: Type[GeneralResponse], expected_statuses: list[int] | int = 200):
+        if not isinstance(expected_statuses, list):
+            expected_statuses = [expected_statuses]
         try:
             loop = asyncio.new_event_loop()
             req = hoppy_client.make_request(self.job.job_id, request.model_dump(by_alias=True))
             response = loop.run_until_complete(req)
             model = response_type.model_validate(response)
-            if model.status_code != expected_status:
+            if model.status_code not in expected_statuses:
                 self.add_error(
                     model.messages
                     if model.messages

diff --git a/domain-ee/ee-ep-merge-app/src/python_src/util/metric_logger.py b/domain-ee/ee-ep-merge-app/src/python_src/util/metric_logger.py
@@ -39,11 +39,12 @@ def increment(metric: str, value: float = 1):
     :param metric: string containing the metric name
     :param value: value to increment by
     """
+    full_metric = f'{APP_PREFIX}.{metric.strip(".").lower()}'
 
     body = MetricPayload(
         series=[
             MetricSeries(
-                metric=f'{APP_PREFIX}.{metric.strip(".").lower()}',
+                metric=full_metric,
                 type=MetricIntakeType.COUNT,
                 points=[
                     MetricPoint(
@@ -59,7 +60,9 @@ def increment(metric: str, value: float = 1):
     try:
         count_metrics_api.submit_metrics(body=body)
     except ApiException as e:
-        logging.warning(f'event=logMetricFailed status={e.status} reason={e.reason} body={e.body}')
+        logging.warning(f'event=logMetricFailed metric={full_metric} type=count value={value} status={e.status} reason={e.reason} body={e.body}')
+    except Exception as e:
+        logging.warning(f'event=logMetricFailed metric={full_metric} type=count value={value} type={type(e)} error="{e}"')
 
 
 def distribution(metric: str, value: float):
@@ -68,11 +71,12 @@ def distribution(metric: str, value: float):
     :param metric: string containing the metric name
     :param value: value to increment by
     """
+    full_metric = f'{APP_PREFIX}.{metric.strip(".").lower()}.distribution'
 
     body = DistributionPointsPayload(
         series=[
             DistributionPointsSeries(
-                metric=f'{APP_PREFIX}.{metric.strip(".").lower()}.distribution',
+                metric=full_metric,
                 points=[
                     DistributionPoint(
                         [
@@ -89,4 +93,6 @@ def distribution(metric: str, value: float):
     try:
         distribution_metrics_api.submit_distribution_points(body=body)
     except ApiException as e:
-        logging.warning(f'event=logMetricFailed status={e.status} reason={e.reason} body={e.body}')
+        logging.warning(f'event=logMetricFailed metric={full_metric} type=distribution value={value} status={e.status} reason={e.reason} body={e.body}')
+    except Exception as e:
+        logging.warning(f'event=logMetricFailed metric={full_metric} type=distribution value={value} type={type(e)} error="{e}"')
diff --git a/domain-ee/ee-ep-merge-app/tests/responses/204_response.json b/domain-ee/ee-ep-merge-app/tests/responses/204_response.json
@@ -0,0 +1,4 @@
+{
+  "statusCode": 204,
+  "statusMessage": "NO CONTENT"
+}
diff --git a/domain-ee/ee-ep-merge-app/tests/service/conftest.py b/domain-ee/ee-ep-merge-app/tests/service/conftest.py
@@ -36,6 +36,7 @@
 RESPONSE_DIR = os.path.abspath('./tests/responses')
 response_200 = f'{RESPONSE_DIR}/200_response.json'
 response_201 = f'{RESPONSE_DIR}/201_response.json'
+response_204 = f'{RESPONSE_DIR}/204_response.json'
 response_404 = f'{RESPONSE_DIR}/404_response.json'
 response_400 = f'{RESPONSE_DIR}/400_response.json'
 response_500 = f'{RESPONSE_DIR}/500_response.json'
@@ -63,6 +64,7 @@ def load_response(file, response_type):
 get_pending_contentions_increase_tinnitus_200 = load_response(pending_contentions_increase_tinnitus_200, get_contentions.Response)
 get_ep400_contentions_req = get_contentions.Request(claim_id=EP400_CLAIM_ID).model_dump(by_alias=True)
 get_ep400_contentions_200 = load_response(ep400_contentions_increase_tinnitus_200, get_contentions.Response)
+get_ep400_contentions_204 = load_response(response_204, get_contentions.Response)
 get_ep400_contentions_without_special_issues_200 = load_response(ep400_contentions_increase_tinnitus_without_special_issues_200, get_contentions.Response)
 update_temporary_station_of_jurisdiction_req = tsoj.Request(claim_id=EP400_CLAIM_ID, temp_station_of_jurisdiction="398").model_dump(by_alias=True)
 revert_temporary_station_of_jurisdiction_req = tsoj.Request(claim_id=EP400_CLAIM_ID, temp_station_of_jurisdiction="111").model_dump(by_alias=True)

diff --git a/domain-ee/ee-ep-merge-app/tests/service/test_ep_merge_process.py b/domain-ee/ee-ep-merge-app/tests/service/test_ep_merge_process.py
@@ -16,6 +16,7 @@
     ep400_contentions_increase_tinnitus_200,
     ep400_contentions_new_tinnitus_200,
     get_ep400_contentions_200,
+    get_ep400_contentions_204,
     get_ep400_contentions_req,
     get_pending_claim_200,
     get_pending_claim_req,
@@ -733,3 +734,26 @@ def test_process_succeeds_with_duplicate_contention(self, machine, mock_hoppy_as
             ]
         )
         assert_metrics_called(metric_logger_distribution, metric_logger_increment, JobState.COMPLETED_SUCCESS, None, 0, True)
+
+    def test_process_succeeds_with_no_ep400_contentions(self, machine, mock_hoppy_async_client, metric_logger_distribution, metric_logger_increment):
+        mock_async_responses(
+            mock_hoppy_async_client,
+            [
+                get_pending_claim_200,
+                get_pending_contentions_increase_tinnitus_200,
+                get_ep400_contentions_204,
+                update_temporary_station_of_jurisdiction_200,
+                cancel_claim_200,
+                add_claim_note_200,
+            ],
+        )
+        process_and_assert(machine, JobState.COMPLETED_SUCCESS)
+        mock_hoppy_async_client.make_request.assert_has_calls(
+            [
+                call(machine.job.job_id, get_pending_contentions_req),
+                call(machine.job.job_id, get_ep400_contentions_req),
+                call(machine.job.job_id, update_temporary_station_of_jurisdiction_req),
+                call(machine.job.job_id, cancel_ep400_claim_req),
+            ]
+        )
+        assert_metrics_called(metric_logger_distribution, metric_logger_increment, JobState.COMPLETED_SUCCESS, None, 0, True)
diff --git a/...mock-bip-claims-api/src/main/java/gov/va/vro/mockbipclaims/controller/BaseController.java b/...mock-bip-claims-api/src/main/java/gov/va/vro/mockbipclaims/controller/BaseController.java
@@ -17,6 +17,10 @@ protected <T extends ProviderResponse> ResponseEntity<T> create201(T response) {
     return new ResponseEntity<>(response, HttpStatus.CREATED);
   }
 
+  protected <T extends ProviderResponse> ResponseEntity<T> create204() {
+    return ResponseEntity.noContent().build();
+  }
+
   protected <T extends ProviderResponse> ResponseEntity<T> create500(T response) {
     response.addMessagesItem(createInternalServerMessage());
     return new ResponseEntity<>(response, HttpStatus.INTERNAL_SERVER_ERROR);

diff --git a/...p-claims-api/src/main/java/gov/va/vro/mockbipclaims/controller/ContentionsController.java b/...p-claims-api/src/main/java/gov/va/vro/mockbipclaims/controller/ContentionsController.java
@@ -84,6 +84,9 @@ public ResponseEntity<ContentionSummariesResponse> getContentionsForClaim(Long c
     }
 
     List<ContentionSummary> contentions = item.getContentions();
+    if (contentions.isEmpty()) {
+      return create204();
+    }
 
     response.setContentions(contentions);
 

diff --git a/mocks/mock-bip-claims-api/src/main/resources/mock-claims.json b/mocks/mock-bip-claims-api/src/main/resources/mock-claims.json
@@ -962,6 +962,17 @@
       }
     ]
   },
+  {
+    "description": [
+      "Employee Experience EP Merge End 2 End Testing - EP 400 w/ no contentions"
+    ],
+    "claimDetail": {
+      "claimId": 10006,
+      "phase": "Claim Received",
+      "endProductCode": "400",
+      "claimLifecycleStatus": "Open"
+    }
+  },
   {
     "description": [
       "Employee Experience EP Merge End 2 End Testing - Fails at get claim details"

diff --git a/svc-bip-api/src/main/java/gov/va/vro/bip/service/BipApiService.java b/svc-bip-api/src/main/java/gov/va/vro/bip/service/BipApiService.java
@@ -146,8 +146,14 @@ private <T extends BipPayloadResponse> T makeRequest(
           url,
           method,
           bipResponse.getStatusCode().value());
+      BipPayloadResponse.BipPayloadResponseBuilder<?, ?> responseBuilder;
+      if (bipResponse.hasBody()) {
+        responseBuilder = mapper.readValue(bipResponse.getBody(), expectedResponse).toBuilder();
+      } else {
+        responseBuilder = mapper.readValue("{}", expectedResponse).toBuilder();
+      }
       return (T)
-          mapper.readValue(bipResponse.getBody(), expectedResponse).toBuilder()
+          responseBuilder
               .statusCode(bipResponse.getStatusCode().value())
               .statusMessage(HttpStatus.valueOf(bipResponse.getStatusCode().value()).name())
               .build();

diff --git a/svc-bip-api/src/test/java/gov/va/vro/bip/service/BipApiServiceTest.java b/svc-bip-api/src/test/java/gov/va/vro/bip/service/BipApiServiceTest.java
@@ -204,6 +204,18 @@ public void testGetClaimContention_200() throws Exception {
     assertEquals(1, result.getContentions().size());
   }
 
+  @Test
+  public void testGetClaimContention_204() throws Exception {
+    ResponseEntity<?> resp204 = ResponseEntity.noContent().build();
+
+    mockResponseForUrl(
+        Mockito.doReturn(resp204), formatClaimUrl(CONTENTION, GOOD_CLAIM_ID), HttpMethod.GET);
+
+    GetClaimContentionsResponse result = service.getClaimContentions(GOOD_CLAIM_ID);
+    assertResponseIsSuccess(result, HttpStatus.NO_CONTENT);
+    assertNull(result.getContentions());
+  }
+
   @Test
   public void testGetClaimContentions_404() throws Exception {
     String resp404Body = getTestData(CLAIM_RESPONSE_404);
@@ -650,12 +662,12 @@ public enum TestCase {
       this.ex =
           status == HttpStatus.INTERNAL_SERVER_ERROR
               ? new HttpServerErrorException(
-                  status, status.name(), getTestData(dataFile).getBytes(), Charset.defaultCharset())
+              status, status.name(), getTestData(dataFile).getBytes(), Charset.defaultCharset())
               : new HttpClientErrorException(
-                  status,
-                  status.name(),
-                  getTestData(dataFile).getBytes(),
-                  Charset.defaultCharset());
+              status,
+              status.name(),
+              getTestData(dataFile).getBytes(),
+              Charset.defaultCharset());
     }
   }
 }