From 23f4382ef8924deec62a1bd8ab998aef47154b1a Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Thu, 9 Jan 2025 12:02:52 +0000 Subject: [PATCH 1/4] Add new exception for markdown errors. We probably shouldn't use an exception provided by a dependency for this. --- .../probate/service/NotificationServiceIT.java | 9 +++++---- .../RequestInformationParameterException.java | 10 ++++++++++ .../handler/DefaultExceptionHandler.java | 11 ----------- .../probate/service/NotificationService.java | 7 +++---- .../handler/DefaultExceptionHandlerTest.java | 17 +++++++++++++++++ .../service/NotificationServiceTest.java | 10 ++++++---- 6 files changed, 41 insertions(+), 23 deletions(-) create mode 100644 src/main/java/uk/gov/hmcts/probate/exception/RequestInformationParameterException.java diff --git a/src/integrationTest/java/uk/gov/hmcts/probate/service/NotificationServiceIT.java b/src/integrationTest/java/uk/gov/hmcts/probate/service/NotificationServiceIT.java index a8087fe5fc..35c76bdb3f 100644 --- a/src/integrationTest/java/uk/gov/hmcts/probate/service/NotificationServiceIT.java +++ b/src/integrationTest/java/uk/gov/hmcts/probate/service/NotificationServiceIT.java @@ -16,6 +16,7 @@ import uk.gov.hmcts.probate.config.properties.registries.Registry; import uk.gov.hmcts.probate.exception.BadRequestException; import uk.gov.hmcts.probate.exception.InvalidEmailException; +import uk.gov.hmcts.probate.exception.RequestInformationParameterException; import uk.gov.hmcts.probate.insights.AppInsights; import uk.gov.hmcts.probate.model.ApplicationType; import uk.gov.hmcts.probate.model.CaseType; @@ -2124,7 +2125,7 @@ void verifySendCaveatNocEmail() @Test void throwExceptionSendEmailWhenInvalidPersonalisationExists() { - NotificationClientException expectException = assertThrows(NotificationClientException.class, + RequestInformationParameterException expectException = assertThrows(RequestInformationParameterException.class, () -> notificationService.sendEmail(CASE_STOPPED, markdownLinkCaseData)); assertEquals(MARKDOWN_ERROR_MESSAGE, expectException.getMessage()); } @@ -2140,14 +2141,14 @@ void throwExceptionSendExecutorEmailWhenInvalidPersonalisationExists() { .build()) .email("primary@probate-test.com") .notification("Yes").build(); - NotificationClientException expectException = assertThrows(NotificationClientException.class, + RequestInformationParameterException expectException = assertThrows(RequestInformationParameterException.class, () -> notificationService.sendEmail(CASE_STOPPED_REQUEST_INFORMATION, markdownLinkCaseData)); assertEquals(MARKDOWN_ERROR_MESSAGE, expectException.getMessage()); } @Test void throwExceptionSendCaveatEmailWhenInvalidPersonalisationExists() { - NotificationClientException expectException = assertThrows(NotificationClientException.class, + RequestInformationParameterException expectException = assertThrows(RequestInformationParameterException.class, () -> notificationService.sendCaveatEmail(GENERAL_CAVEAT_MESSAGE, markdownLinkCaveatData)); assertEquals(MARKDOWN_ERROR_MESSAGE, expectException.getMessage()); } @@ -2166,7 +2167,7 @@ void throwExceptionSendEmailWithDocumentAttachedWhenInvalidPersonalisationExists .build()) .email("primary@probate-test.com") .notification("Yes").build(); - NotificationClientException expectException = assertThrows(NotificationClientException.class, + RequestInformationParameterException expectException = assertThrows(RequestInformationParameterException.class, () -> notificationService.sendEmailWithDocumentAttached(markdownLinkCaseData, executorsApplyingNotification, REDECLARATION_SOT)); assertEquals(MARKDOWN_ERROR_MESSAGE, expectException.getMessage()); diff --git a/src/main/java/uk/gov/hmcts/probate/exception/RequestInformationParameterException.java b/src/main/java/uk/gov/hmcts/probate/exception/RequestInformationParameterException.java new file mode 100644 index 0000000000..aa8295d6a8 --- /dev/null +++ b/src/main/java/uk/gov/hmcts/probate/exception/RequestInformationParameterException.java @@ -0,0 +1,10 @@ +package uk.gov.hmcts.probate.exception; + +public class RequestInformationParameterException extends BusinessValidationException { + private static final String INVALID_PERSONALISATION_ERROR_MESSAGE = + "Markdown Link detected in case data, stop sending notification email."; + + public RequestInformationParameterException() { + super(INVALID_PERSONALISATION_ERROR_MESSAGE, INVALID_PERSONALISATION_ERROR_MESSAGE); + } +} diff --git a/src/main/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandler.java b/src/main/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandler.java index 3624fcdb0c..a2edb10e66 100644 --- a/src/main/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandler.java +++ b/src/main/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandler.java @@ -41,7 +41,6 @@ class DefaultExceptionHandler extends ResponseEntityExceptionHandler { public static final String CONNECTION_ERROR = "Connection error"; public static final String UNAUTHORISED_DATA_EXTRACT_ERROR = "Unauthorised access to Data-Extract error"; - @ExceptionHandler(BadRequestException.class) public ResponseEntity handle(BadRequestException exception) { @@ -93,16 +92,6 @@ public ResponseEntity handle(ConnectionException exception) { return new ResponseEntity<>(errorResponse, headers, SERVICE_UNAVAILABLE); } - @ExceptionHandler(value = NotificationClientException.class) - public ResponseEntity handle(NotificationClientException exception) { - log.warn("Notification service exception", exception); - ErrorResponse errorResponse = - new ErrorResponse(SERVICE_UNAVAILABLE.value(), CLIENT_ERROR, exception.getMessage()); - HttpHeaders headers = new HttpHeaders(); - headers.setContentType(MediaType.APPLICATION_JSON); - return new ResponseEntity<>(errorResponse, headers, SERVICE_UNAVAILABLE); - } - @ExceptionHandler(value = NotFoundException.class) public ResponseEntity handle(NotFoundException exception) { log.warn("Not found exception", exception); diff --git a/src/main/java/uk/gov/hmcts/probate/service/NotificationService.java b/src/main/java/uk/gov/hmcts/probate/service/NotificationService.java index 306c5f6d81..5d1eab61ef 100644 --- a/src/main/java/uk/gov/hmcts/probate/service/NotificationService.java +++ b/src/main/java/uk/gov/hmcts/probate/service/NotificationService.java @@ -11,6 +11,7 @@ import uk.gov.hmcts.probate.config.properties.registries.Registry; import uk.gov.hmcts.probate.exception.BadRequestException; import uk.gov.hmcts.probate.exception.InvalidEmailException; +import uk.gov.hmcts.probate.exception.RequestInformationParameterException; import uk.gov.hmcts.probate.model.ApplicationType; import uk.gov.hmcts.probate.model.CaseOrigin; import uk.gov.hmcts.probate.model.Constants; @@ -71,8 +72,6 @@ public class NotificationService { private static final String PERSONALISATION_APPLICANT_NAME = "applicant_name"; private static final String PERSONALISATION_SOT_LINK = "sot_link"; private static final DateTimeFormatter RELEASE_DATE_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd"); - private static final String INVALID_PERSONALISATION_ERROR_MESSAGE = - "Markdown Link detected in case data, stop sending notification email."; private final EmailAddresses emailAddresses; private final NotificationTemplates notificationTemplates; @@ -555,7 +554,7 @@ private String removedSolicitorNameForPersonalisation(CaseData caseData) { CommonNotificationResult doCommonNotificationServiceHandling( final Map personalisation, - final Long caseId) throws NotificationClientException { + final Long caseId) throws RequestInformationParameterException { final PersonalisationValidationRule.PersonalisationValidationResult validationResult = personalisationValidationRule.validatePersonalisation(personalisation); final Map invalidFields = validationResult.invalidFields(); @@ -564,7 +563,7 @@ CommonNotificationResult doCommonNotificationServiceHandling( if (!invalidFields.isEmpty()) { log.error("Personalisation validation failed for case: {} fields: {}", caseId, invalidFields); - throw new NotificationClientException(INVALID_PERSONALISATION_ERROR_MESSAGE); + throw new RequestInformationParameterException(); } else if (!htmlFields.isEmpty()) { log.info("Personalisation validation found HTML for case: {} fields: {}", caseId, validationResult.htmlFields()); diff --git a/src/test/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandlerTest.java b/src/test/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandlerTest.java index 152db51e78..b26000af5a 100644 --- a/src/test/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandlerTest.java +++ b/src/test/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandlerTest.java @@ -11,6 +11,7 @@ import uk.gov.hmcts.probate.exception.ConnectionException; import uk.gov.hmcts.probate.exception.NotFoundException; import uk.gov.hmcts.probate.exception.OCRMappingException; +import uk.gov.hmcts.probate.exception.RequestInformationParameterException; import uk.gov.hmcts.probate.exception.SocketException; import uk.gov.hmcts.probate.exception.model.ErrorResponse; import uk.gov.hmcts.probate.exception.model.FieldErrorResponse; @@ -21,6 +22,7 @@ import java.util.Arrays; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.openMocks; import static org.springframework.http.HttpStatus.BAD_REQUEST; @@ -178,4 +180,19 @@ void shouldReturnOCRMappingException() { assertEquals(1, response.getBody().getErrors().size()); assertEquals("Message", response.getBody().getErrors().get(0)); } + + @Test + void shouldReturnMarkdownError() { + RequestInformationParameterException ex = mock(RequestInformationParameterException.class); + when(ex.getMessage()).thenReturn(""); + when(ex.getUserMessage()).thenReturn(EXCEPTION_MESSAGE); + + ResponseEntity response = underTest.handle(ex); + + assertEquals(OK, response.getStatusCode(), + "Expected HTTP OK from RequestInformationParameterException handler"); + assertEquals(1, response.getBody().getErrors().size(), "Expected one error"); + assertEquals(EXCEPTION_MESSAGE, response.getBody().getErrors().get(0), + "Expected error to be extracted from exception"); + } } diff --git a/src/test/java/uk/gov/hmcts/probate/service/NotificationServiceTest.java b/src/test/java/uk/gov/hmcts/probate/service/NotificationServiceTest.java index 7968f427fc..6af939974d 100644 --- a/src/test/java/uk/gov/hmcts/probate/service/NotificationServiceTest.java +++ b/src/test/java/uk/gov/hmcts/probate/service/NotificationServiceTest.java @@ -9,6 +9,7 @@ import uk.gov.hmcts.probate.config.notifications.EmailAddresses; import uk.gov.hmcts.probate.config.notifications.NotificationTemplates; import uk.gov.hmcts.probate.config.properties.registries.RegistriesProperties; +import uk.gov.hmcts.probate.exception.RequestInformationParameterException; import uk.gov.hmcts.probate.service.documentmanagement.DocumentManagementService; import uk.gov.hmcts.probate.service.notification.CaveatPersonalisationService; import uk.gov.hmcts.probate.service.notification.GrantOfRepresentationPersonalisationService; @@ -22,7 +23,6 @@ import uk.gov.hmcts.probate.validator.PersonalisationValidationRule.PersonalisationValidationResult; import uk.gov.hmcts.reform.authorisation.generators.AuthTokenGenerator; import uk.gov.service.notify.NotificationClient; -import uk.gov.service.notify.NotificationClientException; import java.util.Collections; import java.util.List; @@ -98,12 +98,13 @@ void givenPersonalisationWithMarkdown_whenCommonValidation_thenThrows() { when(personalisationValidationRuleMock.validatePersonalisation(dummyPersonalisation)) .thenReturn(mockResult); - assertThrows(NotificationClientException.class, () -> + assertThrows(RequestInformationParameterException.class, () -> notificationService.doCommonNotificationServiceHandling(dummyPersonalisation, dummyCaseId)); } @Test - void givenPersonalisationWithHtml_whenCommonValidation_thenReturnsHtmlFound() throws NotificationClientException { + void givenPersonalisationWithHtml_whenCommonValidation_thenReturnsHtmlFound() + throws RequestInformationParameterException { final Map dummyPersonalisation = Collections.emptyMap(); final Long dummyCaseId = 1L; @@ -120,7 +121,8 @@ void givenPersonalisationWithHtml_whenCommonValidation_thenReturnsHtmlFound() th } @Test - void givenPersonalisationWithNoIssue_whenCommonValidation_thenReturnsAllOk() throws NotificationClientException { + void givenPersonalisationWithNoIssue_whenCommonValidation_thenReturnsAllOk() + throws RequestInformationParameterException { final Map dummyPersonalisation = Collections.emptyMap(); final Long dummyCaseId = 1L; From 04de8951b9b97accd205c0a358c92e25efc7a216 Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Thu, 9 Jan 2025 12:10:45 +0000 Subject: [PATCH 2/4] Move NotificationClientException handling out of default. --- .../handler/DefaultExceptionHandler.java | 1 - .../NotificationClientExceptionHandler.java | 81 +++++++++ .../handler/DefaultExceptionHandlerTest.java | 15 -- ...otificationClientExceptionHandlerTest.java | 156 ++++++++++++++++++ 4 files changed, 237 insertions(+), 16 deletions(-) create mode 100644 src/main/java/uk/gov/hmcts/probate/exception/handler/NotificationClientExceptionHandler.java create mode 100644 src/test/java/uk/gov/hmcts/probate/exception/handler/NotificationClientExceptionHandlerTest.java diff --git a/src/main/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandler.java b/src/main/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandler.java index a2edb10e66..1c45cbee90 100644 --- a/src/main/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandler.java +++ b/src/main/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandler.java @@ -19,7 +19,6 @@ import uk.gov.hmcts.probate.model.ccd.ocr.ValidationResponse; import uk.gov.hmcts.probate.model.ccd.ocr.ValidationResponseStatus; import uk.gov.hmcts.probate.model.ccd.raw.response.CallbackResponse; -import uk.gov.service.notify.NotificationClientException; import java.util.ArrayList; import java.util.Arrays; diff --git a/src/main/java/uk/gov/hmcts/probate/exception/handler/NotificationClientExceptionHandler.java b/src/main/java/uk/gov/hmcts/probate/exception/handler/NotificationClientExceptionHandler.java new file mode 100644 index 0000000000..51741e347f --- /dev/null +++ b/src/main/java/uk/gov/hmcts/probate/exception/handler/NotificationClientExceptionHandler.java @@ -0,0 +1,81 @@ +package uk.gov.hmcts.probate.exception.handler; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpHeaders; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.ControllerAdvice; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; +import uk.gov.hmcts.probate.model.ccd.raw.response.CallbackResponse; +import uk.gov.service.notify.NotificationClientException; + +import java.util.ArrayList; +import java.util.List; + +@Slf4j +@ControllerAdvice +public class NotificationClientExceptionHandler extends ResponseEntityExceptionHandler { + public static final String UNABLE_TO_SEND_EMAIL = "Unable to send email"; + + private final ObjectMapper objectMapper; + + @Autowired + public NotificationClientExceptionHandler(ObjectMapper objectMapper) { + this.objectMapper = objectMapper; + } + + @ExceptionHandler(value = NotificationClientException.class) + public ResponseEntity handle(NotificationClientException exception) { + log.warn("Notification service exception", exception); + final List errors = List.copyOf(getNotifyErrors(exception.getMessage())); + + final CallbackResponse errorResponse = CallbackResponse.builder() + .errors(errors) + .build(); + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(MediaType.APPLICATION_JSON); + + return ResponseEntity.ok() + .headers(headers) + .body(errorResponse); + } + + /** + * This relies on the behaviour of NotifyClient#performPostRequest passing the connection's error stream as a json + * string into the exception message, so is liable to break. + */ + private List getNotifyErrors(final String exMessage) { + final String exJson = exMessage.replaceFirst("[^{]*", ""); + + final List errors = new ArrayList<>(); + errors.add(UNABLE_TO_SEND_EMAIL); + + final JsonNode outerJson; + try { + outerJson = objectMapper.readTree(exJson); + } catch (JsonProcessingException e) { + return errors; + } + + if (!outerJson.isObject()) { + return errors; + } + if (!outerJson.has("errors") || !outerJson.get("errors").isArray()) { + return errors; + } + + final JsonNode errorsJson = outerJson.get("errors"); + for (final JsonNode errorJson : errorsJson) { + if (errorJson.isObject() && errorJson.has("message") && errorJson.get("message").isTextual()) { + final String message = errorJson.get("message").asText(); + errors.add(message); + } + } + return errors; + } +} diff --git a/src/test/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandlerTest.java b/src/test/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandlerTest.java index b26000af5a..7c3683af35 100644 --- a/src/test/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandlerTest.java +++ b/src/test/java/uk/gov/hmcts/probate/exception/handler/DefaultExceptionHandlerTest.java @@ -17,7 +17,6 @@ import uk.gov.hmcts.probate.exception.model.FieldErrorResponse; import uk.gov.hmcts.probate.model.ccd.ocr.ValidationResponse; import uk.gov.hmcts.probate.model.ccd.raw.response.CallbackResponse; -import uk.gov.service.notify.NotificationClientException; import java.util.Arrays; @@ -43,9 +42,6 @@ class DefaultExceptionHandlerTest { @Mock private BadRequestException badRequestException; - @Mock - private NotificationClientException notificationClientException; - @Mock private BusinessValidationException businessValidationException; @@ -113,17 +109,6 @@ void shouldHandleMissingPDFDataAsStatusUN() { assertEquals(bve2Mock, response.getBody().getFieldErrors().get(1)); } - @Test - void shouldReturnNotificationClientException() { - when(notificationClientException.getMessage()).thenReturn(EXCEPTION_MESSAGE); - - ResponseEntity response = underTest.handle(notificationClientException); - - assertEquals(SERVICE_UNAVAILABLE, response.getStatusCode()); - assertEquals(DefaultExceptionHandler.CLIENT_ERROR, response.getBody().getError()); - assertEquals(EXCEPTION_MESSAGE, response.getBody().getMessage()); - } - @Test void shouldReturnBusinessValidationException() { when(businessValidationException.getUserMessage()).thenReturn(EXCEPTION_MESSAGE); diff --git a/src/test/java/uk/gov/hmcts/probate/exception/handler/NotificationClientExceptionHandlerTest.java b/src/test/java/uk/gov/hmcts/probate/exception/handler/NotificationClientExceptionHandlerTest.java new file mode 100644 index 0000000000..9f37935d4c --- /dev/null +++ b/src/test/java/uk/gov/hmcts/probate/exception/handler/NotificationClientExceptionHandlerTest.java @@ -0,0 +1,156 @@ +package uk.gov.hmcts.probate.exception.handler; + +import com.fasterxml.jackson.core.JsonParseException; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.http.ResponseEntity; +import uk.gov.hmcts.probate.model.ccd.raw.response.CallbackResponse; +import uk.gov.service.notify.NotificationClientException; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; +import static org.springframework.http.HttpStatus.OK; + +public class NotificationClientExceptionHandlerTest { + NotificationClientExceptionHandler underTest; + + ObjectMapper objectMapperSpy; + + @BeforeEach + void setUp() { + objectMapperSpy = spy(ObjectMapper.class); + underTest = new NotificationClientExceptionHandler(objectMapperSpy); + } + + @Test + void shouldReturnNotificationClientException() { + final String firstError = "first_error"; + final String secondError = "second_error"; + final String exceptMsg = new StringBuilder() + .append("Status code: 400 ") + .append("{\"errors\":[") + .append("{\"message\":\"").append(firstError).append("\"},") + .append("{\"message\":\"").append(secondError).append("\"}") + .append("]}") + .toString(); + + NotificationClientException notificationClientException = mock(NotificationClientException.class); + when(notificationClientException.getMessage()).thenReturn(exceptMsg); + + ResponseEntity response = underTest.handle(notificationClientException); + + assertEquals(OK, response.getStatusCode(), "Expected HTTP OK (200) response status"); + assertEquals(3, response.getBody().getErrors().size(), "Expected three errors"); + assertEquals(NotificationClientExceptionHandler.UNABLE_TO_SEND_EMAIL, response.getBody().getErrors().get(0), + "expected first error to be from handler"); + assertEquals(firstError, response.getBody().getErrors().get(1), + "expected second error to match from exception message"); + assertEquals(secondError, response.getBody().getErrors().get(2), + "expected third error to match from exception message"); + } + + @Test + void shouldReturnNotificationClientExceptionWhenInvalidJson() throws JsonProcessingException { + final String exceptMsg = ""; + + NotificationClientException notificationClientException = mock(NotificationClientException.class); + when(notificationClientException.getMessage()).thenReturn(exceptMsg); + + when(objectMapperSpy.readTree(anyString())).thenThrow(new JsonParseException(exceptMsg)); + + ResponseEntity response = underTest.handle(notificationClientException); + + assertEquals(OK, response.getStatusCode(), "Expected HTTP OK (200) response status"); + assertEquals(1, response.getBody().getErrors().size(), + "Expected only one error on json parse failure"); + assertEquals(NotificationClientExceptionHandler.UNABLE_TO_SEND_EMAIL, response.getBody().getErrors().get(0), + "expected first error to be from handler"); + } + + @Test + void shouldReturnNotificationClientExceptionWhenJsonArray() throws JsonProcessingException { + final String exceptMsg = "[]"; + + NotificationClientException notificationClientException = mock(NotificationClientException.class); + when(notificationClientException.getMessage()).thenReturn(exceptMsg); + + ResponseEntity response = underTest.handle(notificationClientException); + + assertEquals(OK, response.getStatusCode(), "Expected HTTP OK (200) response status"); + assertEquals(1, response.getBody().getErrors().size(), + "Expected only one error on json parse failure"); + assertEquals(NotificationClientExceptionHandler.UNABLE_TO_SEND_EMAIL, response.getBody().getErrors().get(0), + "expected first error to be from handler"); + } + + @Test + void shouldReturnNotificationClientExceptionWhenJsonMissingErrors() throws JsonProcessingException { + final String firstError = "first_error"; + final String secondError = "second_error"; + final String exceptMsg = "{}"; + + NotificationClientException notificationClientException = mock(NotificationClientException.class); + when(notificationClientException.getMessage()).thenReturn(exceptMsg); + + ResponseEntity response = underTest.handle(notificationClientException); + + assertEquals(OK, response.getStatusCode(), "Expected HTTP OK (200) response status"); + assertEquals(1, response.getBody().getErrors().size(), + "Expected only one error on json parse failure"); + assertEquals(NotificationClientExceptionHandler.UNABLE_TO_SEND_EMAIL, response.getBody().getErrors().get(0), + "expected first error to be from handler"); + } + + @Test + void shouldReturnNotificationClientExceptionWhenJsonErrorsIsObject() throws JsonProcessingException { + final String firstError = "first_error"; + final String secondError = "second_error"; + final String exceptMsg = "{\"errors\":{}}"; + + NotificationClientException notificationClientException = mock(NotificationClientException.class); + when(notificationClientException.getMessage()).thenReturn(exceptMsg); + + ResponseEntity response = underTest.handle(notificationClientException); + + assertEquals(OK, response.getStatusCode(), "Expected HTTP OK (200) response status"); + assertEquals(1, response.getBody().getErrors().size(), + "Expected only one error on json parse failure"); + assertEquals(NotificationClientExceptionHandler.UNABLE_TO_SEND_EMAIL, response.getBody().getErrors().get(0), + "expected first error to be from handler"); + } + + @Test + void shouldReturnNotificationClientExceptionWhenJsonErrorsContainsUnexpected() throws JsonProcessingException { + final String firstError = "first_error"; + final String secondError = "second_error"; + final String exceptMsg = new StringBuilder() + .append("{\"errors\":[") + .append("{\"message\":\"").append(firstError).append("\"},") + .append("[],") + .append("{},") + .append("{\"message\": 0},") + .append("{\"message\":\"").append(secondError).append("\"}") + .append("]}") + .toString(); + + NotificationClientException notificationClientException = mock(NotificationClientException.class); + when(notificationClientException.getMessage()).thenReturn(exceptMsg); + + ResponseEntity response = underTest.handle(notificationClientException); + + + assertEquals(OK, response.getStatusCode(), "Expected HTTP OK (200) response status"); + assertEquals(3, response.getBody().getErrors().size(), "Expected three errors"); + assertEquals(NotificationClientExceptionHandler.UNABLE_TO_SEND_EMAIL, response.getBody().getErrors().get(0), + "expected first error to be from handler"); + assertEquals(firstError, response.getBody().getErrors().get(1), + "expected second error to match from exception message"); + assertEquals(secondError, response.getBody().getErrors().get(2), + "expected third error to match from exception message"); + } +} From 711f2ae1c03759985a8e45e3be2e0403b744acc0 Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Thu, 9 Jan 2025 16:09:01 +0000 Subject: [PATCH 3/4] Propagate Notify errors back to request for information. --- .../gov/hmcts/probate/controller/NotificationController.java | 3 ++- .../service/InformationRequestCorrespondenceService.java | 2 +- .../gov/hmcts/probate/service/InformationRequestService.java | 5 +++-- .../hmcts/probate/service/InformationRequestServiceTest.java | 4 ++-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/probate/controller/NotificationController.java b/src/main/java/uk/gov/hmcts/probate/controller/NotificationController.java index fc0e184c98..2a38392251 100644 --- a/src/main/java/uk/gov/hmcts/probate/controller/NotificationController.java +++ b/src/main/java/uk/gov/hmcts/probate/controller/NotificationController.java @@ -199,7 +199,8 @@ public ResponseEntity sendDocumentReceivedNotification( } @PostMapping(path = "/stopped-information-request") - public ResponseEntity informationRequest(@RequestBody CallbackRequest callbackRequest) { + public ResponseEntity informationRequest( + @RequestBody final CallbackRequest callbackRequest) throws NotificationClientException { Optional caseworkerInfo = userInfoService.getCaseworkerInfo(); return ResponseEntity.ok(informationRequestService.handleInformationRequest(callbackRequest, caseworkerInfo)); } diff --git a/src/main/java/uk/gov/hmcts/probate/service/InformationRequestCorrespondenceService.java b/src/main/java/uk/gov/hmcts/probate/service/InformationRequestCorrespondenceService.java index d05ba5a803..65c3cb73d0 100644 --- a/src/main/java/uk/gov/hmcts/probate/service/InformationRequestCorrespondenceService.java +++ b/src/main/java/uk/gov/hmcts/probate/service/InformationRequestCorrespondenceService.java @@ -18,7 +18,7 @@ public class InformationRequestCorrespondenceService { private final NotificationService notificationService; - public List emailInformationRequest(CaseDetails caseDetails) { + public List emailInformationRequest(CaseDetails caseDetails) throws NotificationClientException { try { final Document notification = notificationService.sendEmail(CASE_STOPPED_REQUEST_INFORMATION, caseDetails); log.info("Successful response for request for information email for case id {} ", caseDetails.getId()); diff --git a/src/main/java/uk/gov/hmcts/probate/service/InformationRequestService.java b/src/main/java/uk/gov/hmcts/probate/service/InformationRequestService.java index 1e18ba3dac..05f3a4caea 100644 --- a/src/main/java/uk/gov/hmcts/probate/service/InformationRequestService.java +++ b/src/main/java/uk/gov/hmcts/probate/service/InformationRequestService.java @@ -26,8 +26,9 @@ public class InformationRequestService { private final CallbackResponseTransformer callbackResponseTransformer; private final EmailAddressNotifyApplicantValidationRule emailAddressNotifyApplicantValidationRule; - public CallbackResponse handleInformationRequest(CallbackRequest callbackRequest, - Optional caseworkerInfo) { + public CallbackResponse handleInformationRequest( + final CallbackRequest callbackRequest, + final Optional caseworkerInfo) throws NotificationClientException { CaseData caseData = callbackRequest.getCaseDetails().getData(); CCDData dataForEmailAddress = CCDData.builder() .applicationType(caseData.getApplicationType().name()) diff --git a/src/test/java/uk/gov/hmcts/probate/service/InformationRequestServiceTest.java b/src/test/java/uk/gov/hmcts/probate/service/InformationRequestServiceTest.java index 4d7706ef49..d5200c17d8 100644 --- a/src/test/java/uk/gov/hmcts/probate/service/InformationRequestServiceTest.java +++ b/src/test/java/uk/gov/hmcts/probate/service/InformationRequestServiceTest.java @@ -73,7 +73,7 @@ public void setup() { } @Test - void testEmailRequestReturnsSentEmailDocumentSuccessfully() { + void testEmailRequestReturnsSentEmailDocumentSuccessfully() throws NotificationClientException { CollectionMember documentCollectionMember = new CollectionMember<>(Document.builder().documentType(DocumentType.SENT_EMAIL).build()); documentList = new ArrayList<>(); @@ -102,7 +102,7 @@ void testEmailRequestReturnsSentEmailDocumentSuccessfully() { } @Test - void testEmailRequestReturnsErrorWhenNoEmailProvided() { + void testEmailRequestReturnsErrorWhenNoEmailProvided() throws NotificationClientException { caseData = CaseData.builder() .applicationType(ApplicationType.PERSONAL) .build(); From 29e424687cb2517c5b412d99e5b74621698e074a Mon Sep 17 00:00:00 2001 From: Tom Saunders Date: Wed, 15 Jan 2025 15:21:34 +0000 Subject: [PATCH 4/4] Allow Notify exception to propagate from request info. --- .../InformationRequestCorrespondenceService.java | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/probate/service/InformationRequestCorrespondenceService.java b/src/main/java/uk/gov/hmcts/probate/service/InformationRequestCorrespondenceService.java index 65c3cb73d0..c526078013 100644 --- a/src/main/java/uk/gov/hmcts/probate/service/InformationRequestCorrespondenceService.java +++ b/src/main/java/uk/gov/hmcts/probate/service/InformationRequestCorrespondenceService.java @@ -19,14 +19,8 @@ public class InformationRequestCorrespondenceService { private final NotificationService notificationService; public List emailInformationRequest(CaseDetails caseDetails) throws NotificationClientException { - try { - final Document notification = notificationService.sendEmail(CASE_STOPPED_REQUEST_INFORMATION, caseDetails); - log.info("Successful response for request for information email for case id {} ", caseDetails.getId()); - return List.of(notification); - } catch (NotificationClientException e) { - log.error(e.getMessage()); - // this feels wrong - do we not want to alert the caller that the email sending has failed? - return List.of(); - } + final Document notification = notificationService.sendEmail(CASE_STOPPED_REQUEST_INFORMATION, caseDetails); + log.info("Successful response for request for information email for case id {} ", caseDetails.getId()); + return List.of(notification); } }