From 7150d75f4f75dec57038740e82d7cb910959929b Mon Sep 17 00:00:00 2001
From: matthew-robinson-ons
 <113037557+matthew-robinson-ons@users.noreply.github.com>
Date: Wed, 28 Feb 2024 13:48:20 +0000
Subject: [PATCH] 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 <pricem14pc@users.noreply.github.com>

* remove redundant test

* update exception string and imports

* remove error logging

* update to format string

---------

Co-authored-by: ras-rm-pr-bot <rasrm.team@ons.gov.uk>
Co-authored-by: Mark Price <pricem14pc@users.noreply.github.com>
---
 _infra/helm/collection-exercise/Chart.yaml    |  4 +--
 .../service/SupplementaryDatasetService.java  |  9 ++++++-
 .../SupplementaryDatasetServiceTest.java      | 26 ++++++++++++++-----
 3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/_infra/helm/collection-exercise/Chart.yaml b/_infra/helm/collection-exercise/Chart.yaml
index 49bba8ad..e455e9b2 100644
--- 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/src/main/java/uk/gov/ons/ctp/response/collection/exercise/service/SupplementaryDatasetService.java b/src/main/java/uk/gov/ons/ctp/response/collection/exercise/service/SupplementaryDatasetService.java
index c1905a72..c608e8b2 100644
--- a/src/main/java/uk/gov/ons/ctp/response/collection/exercise/service/SupplementaryDatasetService.java
+++ b/src/main/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/src/test/java/uk/gov/ons/ctp/response/collection/exercise/service/SupplementaryDatasetServiceTest.java b/src/test/java/uk/gov/ons/ctp/response/collection/exercise/service/SupplementaryDatasetServiceTest.java
index 1469f691..93c101d6 100644
--- a/src/test/java/uk/gov/ons/ctp/response/collection/exercise/service/SupplementaryDatasetServiceTest.java
+++ b/src/test/java/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