Improve exception handling and logging for SDS (#333)

* update exception handling * auto patch increment * update error message * formatting * nack correctly * try new approach * revert changes * revert * revert * update log test * formatting * rename test * remove duplicate test * catch system error as well as resource not found * formatting * Update src/test/java/uk/gov/ons/ctp/response/collection/exercise/service/SupplementaryDatasetServiceTest.java Co-authored-by: Mark Price <[email protected]> * remove redundant test * update exception string and imports * remove error logging * update to format string --------- Co-authored-by: ras-rm-pr-bot <[email protected]> Co-authored-by: Mark Price <[email protected]>
ONSdigital · Feb 28, 2024 · 7150d75 · 7150d75
1 parent eba5f3f
commit 7150d75
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 10 deletions.
diff --git a/_infra/helm/collection-exercise/Chart.yaml b/_infra/helm/collection-exercise/Chart.yaml
@@ -14,9 +14,9 @@ type: application
 
 # This is the chart version. This version number should be incremented each time you make changes
 # to the chart and its templates, including the app version.
-version: 13.0.30
+version: 13.0.31
 
 # This is the version number of the application being deployed. This version number should be
 # incremented each time you make changes to the application.
-appVersion: 13.0.30
+appVersion: 13.0.31
 
diff --git a/...java/uk/gov/ons/ctp/response/collection/exercise/service/SupplementaryDatasetService.java b/...java/uk/gov/ons/ctp/response/collection/exercise/service/SupplementaryDatasetService.java
@@ -28,7 +28,14 @@ public void addSupplementaryDatasetEntity(SupplementaryDatasetDTO supplementaryD
     CollectionExercise collectionExercise =
         collectionExerciseService.findCollectionExercise(
             supplementaryDatasetDTO.getSurveyId(), supplementaryDatasetDTO.getPeriodId());
-
+    if (collectionExercise == null) {
+      throw new CTPException(
+          CTPException.Fault.RESOURCE_NOT_FOUND,
+          "Failed to find collection exercise for supplementary dataset "
+              + "survey_id: %s, period_id: %s",
+          supplementaryDatasetDTO.getSurveyId(),
+          supplementaryDatasetDTO.getPeriodId());
+    }
     try {
       if (existsByExerciseFK(collectionExercise.getExercisePK())) {
         log.info(

diff --git a/.../uk/gov/ons/ctp/response/collection/exercise/service/SupplementaryDatasetServiceTest.java b/.../uk/gov/ons/ctp/response/collection/exercise/service/SupplementaryDatasetServiceTest.java
@@ -1,8 +1,9 @@
 package uk.gov.ons.ctp.response.collection.exercise.service;
 
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.*;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -61,13 +62,24 @@ public void testDeleteAndSaveSupplementaryDataset() throws CTPException, JsonPro
     verify(supplementaryDatasetRepository, times(1)).save(supplementaryDatasetEntity);
   }
 
-  @Test(expected = CTPException.class)
-  public void testFailedToFindCollectionExerciseForSupplementaryDataset() throws CTPException {
+  @Test
+  public void testFailedToFindCollectionExerciseThrowsResourceNotFoundException() {
     when(collectionExerciseService.findCollectionExercise(
             supplementaryDatasetDTO.getSurveyId(), supplementaryDatasetDTO.getPeriodId()))
         .thenReturn(null);
-
-    supplementaryDatasetService.addSupplementaryDatasetEntity(supplementaryDatasetDTO);
+    try {
+      supplementaryDatasetService.addSupplementaryDatasetEntity(supplementaryDatasetDTO);
+      fail("Expected CTPException was not thrown");
+    } catch (CTPException e) {
+      assertEquals(CTPException.class, e.getClass());
+      assertEquals(CTPException.Fault.RESOURCE_NOT_FOUND, e.getFault());
+      assertEquals(
+          "Failed to find collection exercise for supplementary dataset "
+              + "survey_id: 009, period_id: 2013",
+          e.getMessage());
+    }
+    verify(collectionExerciseService).findCollectionExercise(anyString(), anyString());
+    verifyNoInteractions(supplementaryDatasetRepository);
   }
 
   @Test