FACT-1770 adds case reference to logs (#3368)

RuthKirby · web-flow · commit ac5ee8dbac63 · 2024-11-22T14:08:33.000Z
* - adds case reference to logs

* - checkstyle fixes

* - adds case reference to handling envelope logs

* - checkstyle fixes

* - updates with new rule name to stop warning about name change

* - updates test to reflect envelope calls getCaseNumber now for logging

* - adds case reference to logs

* - cleanup log message

* - exclude jms versions of files from sonar

* - adds whitespace

* - changes to using optional so relevant message is there when case reference not present

* - fix modifiers in wrong order

* - adds space
diff --git a/build.gradle b/build.gradle
@@ -196,7 +196,7 @@ sonarqube {
     property "sonar.coverage.jacoco.xmlReportPaths", jacocoTestReport.reports.xml.outputLocation.get().asFile.absolutePath
     property "sonar.pitest.mode", "reuseReport"
     property "sonar.pitest.reportsDirectory", "build/reports/pitest"
-    property "sonar.exclusions", "**/model/out/*,**/config/**,**/LeaseAcquirer.java"
+    property "sonar.exclusions", "**/model/out/*,**/config/**,**/LeaseAcquirer.java,**/services/jms/JmsFileContentProcessor.java,**/tasks/jms/JmsBlobProcessorTask.java"
   }
 }
 
diff --git a/config/pmd/ruleset.xml b/config/pmd/ruleset.xml
@@ -67,7 +67,7 @@
     <exclude name="UncommentedEmptyMethodBody" />
   </rule>
   <rule ref="category/java/errorprone.xml">
-    <exclude name="BeanMembersShouldSerialize" />
+    <exclude name="NonSerializableClass" />
   </rule>
   <rule ref="category/java/multithreading.xml" />
   <rule ref="category/java/performance.xml" />
diff --git a/src/main/java/uk/gov/hmcts/reform/bulkscanprocessor/services/DeleteFilesService.java b/src/main/java/uk/gov/hmcts/reform/bulkscanprocessor/services/DeleteFilesService.java
@@ -14,6 +14,7 @@
 import uk.gov.hmcts.reform.bulkscanprocessor.services.storage.LeaseAcquirer;
 
 import java.util.List;
+import java.util.Optional;
 
 import static com.azure.storage.blob.models.BlobErrorCode.BLOB_NOT_FOUND;
 
@@ -83,8 +84,9 @@ public void processCompleteFiles(BlobContainerClient container) {
      */
     private boolean tryProcessCompleteEnvelope(BlobContainerClient container, Envelope envelope) {
 
-        String loggingContext = "File name: " + envelope.getZipFileName()
-            + ", Container: " + container.getBlobContainerName();
+        String loggingContext = String.format("File name: %s, Container: %s, Case reference: %s",
+                                              envelope.getZipFileName(), container.getBlobContainerName(),
+                                              Optional.ofNullable(envelope.getCaseNumber()).orElse("(Not Present)"));
 
         try {
             log.info("Deleting file. {}", loggingContext);
diff --git a/src/main/java/uk/gov/hmcts/reform/bulkscanprocessor/services/EnvelopeActionService.java b/src/main/java/uk/gov/hmcts/reform/bulkscanprocessor/services/EnvelopeActionService.java
@@ -19,6 +19,7 @@
 import uk.gov.hmcts.reform.bulkscanprocessor.model.common.Event;
 
 import java.time.Instant;
+import java.util.Optional;
 import java.util.UUID;
 
 import static java.time.Duration.between;
@@ -41,6 +42,8 @@ public class EnvelopeActionService {
     private final ProcessEventRepository processEventRepository;
     private final long notificationTimeoutHr;
 
+    private static final String CASE_REFERENCE_NOT_PRESENT = "(NOT PRESENT)";
+
     /**
      * Constructor for EnvelopeActionService.
      * @param envelopeRepository EnvelopeRepository
@@ -79,7 +82,8 @@ public void reprocessEnvelope(UUID envelopeId) {
         envelope.setStatus(UPLOADED);
         envelopeRepository.save(envelope);
 
-        log.info("Envelope {} status changed to UPLOADED", envelope.getZipFileName());
+        log.info("Envelope {} status changed to UPLOADED. Case reference: {}", envelope.getZipFileName(),
+                 Optional.ofNullable(envelope.getCaseNumber()).orElse(CASE_REFERENCE_NOT_PRESENT));
     }
 
     /**
@@ -104,7 +108,8 @@ public void moveEnvelopeToCompleted(UUID envelopeId) {
         envelope.setStatus(COMPLETED);
         envelopeRepository.save(envelope);
 
-        log.info("Envelope {} status changed to COMPLETED", envelope.getZipFileName());
+        log.info("Envelope {} status changed to COMPLETED. Case Reference: {}", envelope.getZipFileName(),
+                 Optional.ofNullable(envelope.getCaseNumber()).orElse(CASE_REFERENCE_NOT_PRESENT));
     }
 
     /**
@@ -129,7 +134,8 @@ public void moveEnvelopeToAborted(UUID envelopeId) {
         envelope.setStatus(ABORTED);
         envelopeRepository.save(envelope);
 
-        log.info("Envelope {} status changed to ABORTED", envelope.getZipFileName());
+        log.info("Envelope {} status changed to ABORTED. Case reference: {}", envelope.getZipFileName(),
+                 Optional.ofNullable(envelope.getCaseNumber()).orElse(CASE_REFERENCE_NOT_PRESENT));
     }
 
     /**
diff --git a/src/main/java/uk/gov/hmcts/reform/bulkscanprocessor/services/FileContentProcessor.java b/src/main/java/uk/gov/hmcts/reform/bulkscanprocessor/services/FileContentProcessor.java
@@ -15,6 +15,7 @@
 import uk.gov.hmcts.reform.bulkscanprocessor.tasks.processor.ZipFileContentDetail;
 import uk.gov.hmcts.reform.bulkscanprocessor.tasks.processor.ZipFileProcessor;
 
+import java.util.Optional;
 import java.util.zip.ZipInputStream;
 
 import static java.util.stream.Collectors.joining;
@@ -40,6 +41,8 @@ public class FileContentProcessor {
 
     private final FileRejector fileRejector;
 
+    private static final String CASE_REFERENCE_NOT_PRESENT = "(NOT PRESENT)";
+
     /**
      * Constructor for the FileContentProcessor.
      * @param zipFileProcessor The zip file processor
@@ -70,18 +73,20 @@ public void processZipFileContent(
         String zipFilename,
         String containerName
     ) {
+        Optional<String> caseReference = Optional.empty();
         try {
             ZipFileContentDetail zipDetail = zipFileProcessor.getZipContentDetail(zis, zipFilename);
 
             InputEnvelope inputEnvelope = envelopeProcessor.parseEnvelope(zipDetail.getMetadata(), zipFilename);
+            caseReference = Optional.ofNullable(inputEnvelope.caseNumber);
 
             log.info(
                 "Parsed envelope. File name: {}. Container: {}. Payment DCNs: {}. Document DCNs: {}, caseNumber {}",
                 zipFilename,
                 containerName,
                 inputEnvelope.payments.stream().map(payment -> payment.documentControlNumber).collect(joining(",")),
                 inputEnvelope.scannableItems.stream().map(doc -> doc.documentControlNumber).collect(joining(",")),
-                inputEnvelope.caseNumber
+                caseReference.orElse(CASE_REFERENCE_NOT_PRESENT)
             );
 
             envelopeHandler.handleEnvelope(
@@ -92,22 +97,26 @@ public void processZipFileContent(
             );
         } catch (PaymentsDisabledException ex) {
             log.error(
-                "Rejected file {} from container {} - Payments processing is disabled", zipFilename, containerName
+                "Rejected file {} from container {}, Case reference: {} - Payments processing is disabled",
+                zipFilename, containerName, caseReference.orElse(CASE_REFERENCE_NOT_PRESENT)
             );
             Long eventId = createEvent(FILE_VALIDATION_FAILURE, containerName, zipFilename, ex.getMessage());
             fileRejector.handleInvalidBlob(eventId, containerName, zipFilename, ex);
         } catch (ServiceDisabledException ex) {
             log.error(
-                "Rejected file {} from container {} - Service is disabled", zipFilename, containerName
+                "Rejected file {} from container {}, Case reference: {} - Service is disabled",
+                zipFilename, containerName, caseReference.orElse(CASE_REFERENCE_NOT_PRESENT)
             );
             Long eventId = createEvent(DISABLED_SERVICE_FAILURE, containerName, zipFilename, ex.getMessage());
             fileRejector.handleInvalidBlob(eventId, containerName, zipFilename, ex);
         } catch (EnvelopeRejectionException ex) {
-            log.warn("Rejected file {} from container {} - invalid", zipFilename, containerName, ex);
+            log.warn("Rejected file {} from container {}, Case reference: {} - invalid",
+                     zipFilename, containerName, caseReference.orElse(CASE_REFERENCE_NOT_PRESENT), ex);
             Long eventId = createEvent(FILE_VALIDATION_FAILURE, containerName, zipFilename, ex.getMessage());
             fileRejector.handleInvalidBlob(eventId, containerName, zipFilename, ex);
         } catch (PreviouslyFailedToUploadException ex) {
-            log.warn("Rejected file {} from container {} - failed previously", zipFilename, containerName, ex);
+            log.warn("Rejected file {} from container {}, Case reference: {} - failed previously",
+                     zipFilename, containerName, caseReference.orElse(CASE_REFERENCE_NOT_PRESENT), ex);
             createEvent(DOC_UPLOAD_FAILURE, containerName, zipFilename, ex.getMessage());
         } catch (Exception ex) {
             log.error("Failed to process file {} from container {}", zipFilename, containerName, ex);
diff --git a/src/main/java/uk/gov/hmcts/reform/bulkscanprocessor/services/OrchestratorNotificationService.java b/src/main/java/uk/gov/hmcts/reform/bulkscanprocessor/services/OrchestratorNotificationService.java
@@ -15,6 +15,7 @@
 import uk.gov.hmcts.reform.bulkscanprocessor.model.out.msg.EnvelopeMsg;
 import uk.gov.hmcts.reform.bulkscanprocessor.services.servicebus.ServiceBusSendHelper;
 
+import java.util.Optional;
 import java.util.concurrent.atomic.AtomicInteger;
 
 /**
@@ -94,7 +95,7 @@ private void createEvent(Envelope envelope, Event event) {
     private void updateStatus(Envelope envelope) {
         envelope.setStatus(Status.NOTIFICATION_SENT);
         envelopeRepo.saveAndFlush(envelope);
-
-        log.info("Envelope {} status changed to NOTIFICATION_SENT", envelope.getZipFileName());
+        log.info("Envelope {} status changed to NOTIFICATION_SENT. Case reference: {}", envelope.getZipFileName(),
+                 Optional.ofNullable(envelope.getCaseNumber()).orElse("(NOT PRESENT)"));
     }
 }
diff --git a/src/main/java/uk/gov/hmcts/reform/bulkscanprocessor/services/jms/JmsFileContentProcessor.java b/src/main/java/uk/gov/hmcts/reform/bulkscanprocessor/services/jms/JmsFileContentProcessor.java
@@ -16,6 +16,7 @@
 import uk.gov.hmcts.reform.bulkscanprocessor.tasks.processor.ZipFileContentDetail;
 import uk.gov.hmcts.reform.bulkscanprocessor.tasks.processor.ZipFileProcessor;
 
+import java.util.Optional;
 import java.util.zip.ZipInputStream;
 
 import static java.util.stream.Collectors.joining;
@@ -41,6 +42,8 @@ public class JmsFileContentProcessor {
 
     private final JmsFileRejector fileRejector;
 
+    private static final String CASE_REFERENCE_NOT_PRESENT = "(NOT PRESENT)";
+
     /**
      * Constructor for JmsFileContentProcessor.
      * @param zipFileProcessor The zip file processor
@@ -71,18 +74,21 @@ public void processZipFileContent(
         String zipFilename,
         String containerName
     ) {
+
+        Optional<String> caseReference = Optional.empty();
         try {
             ZipFileContentDetail zipDetail = zipFileProcessor.getZipContentDetail(zis, zipFilename);
 
             InputEnvelope inputEnvelope = envelopeProcessor.parseEnvelope(zipDetail.getMetadata(), zipFilename);
+            caseReference = Optional.ofNullable(inputEnvelope.caseNumber);
 
             log.info(
                 "Parsed envelope. File name: {}. Container: {}. Payment DCNs: {}. Document DCNs: {}, caseNumber {}",
                 zipFilename,
                 containerName,
                 inputEnvelope.payments.stream().map(payment -> payment.documentControlNumber).collect(joining(",")),
                 inputEnvelope.scannableItems.stream().map(doc -> doc.documentControlNumber).collect(joining(",")),
-                inputEnvelope.caseNumber
+                caseReference.orElse(CASE_REFERENCE_NOT_PRESENT)
             );
 
             envelopeHandler.handleEnvelope(
@@ -93,21 +99,25 @@ public void processZipFileContent(
             );
         } catch (PaymentsDisabledException ex) {
             log.error(
-                "Rejected file {} from container {} - Payments processing is disabled", zipFilename, containerName
+                "Rejected file {} from container {}, Case reference: {} - Payments processing is disabled",
+                zipFilename, containerName, caseReference.orElse(CASE_REFERENCE_NOT_PRESENT)
             );
             Long eventId = createEvent(FILE_VALIDATION_FAILURE, containerName, zipFilename, ex.getMessage());
             fileRejector.handleInvalidBlob(eventId, containerName, zipFilename, ex);
         } catch (ServiceDisabledException ex) {
             log.error(
-                "Rejected file {} from container {} - Service is disabled", zipFilename, containerName
+                "Rejected file {} from container {}, Case reference: {} - Service is disabled",
+                zipFilename, containerName, caseReference.orElse(CASE_REFERENCE_NOT_PRESENT)
             );
             Long eventId = createEvent(DISABLED_SERVICE_FAILURE, containerName, zipFilename, ex.getMessage());
             fileRejector.handleInvalidBlob(eventId, containerName, zipFilename, ex);
         }  catch (PreviouslyFailedToUploadException ex) {
-            log.warn("Rejected file {} from container {} - failed previously", zipFilename, containerName, ex);
+            log.warn("Rejected file {} from container {}, Case reference: {} - failed previously",
+                     zipFilename, containerName, caseReference.orElse(CASE_REFERENCE_NOT_PRESENT), ex);
             createEvent(DOC_UPLOAD_FAILURE, containerName, zipFilename, ex.getMessage());
         } catch (EnvelopeRejectionException ex) {
-            log.warn("Rejected file {} from container {} - invalid", zipFilename, containerName, ex);
+            log.warn("Rejected file {} from container {}, Case reference: {} - invalid",
+                     zipFilename, containerName, caseReference.orElse(CASE_REFERENCE_NOT_PRESENT), ex);
             Long eventId = createEvent(FILE_VALIDATION_FAILURE, containerName, zipFilename, ex.getMessage());
             fileRejector.handleInvalidBlob(eventId, containerName, zipFilename, ex);
         } catch (Exception ex) {
diff --git a/src/main/java/uk/gov/hmcts/reform/bulkscanprocessor/tasks/BlobProcessorTask.java b/src/main/java/uk/gov/hmcts/reform/bulkscanprocessor/tasks/BlobProcessorTask.java
@@ -18,6 +18,7 @@
 
 import java.io.IOException;
 import java.util.List;
+import java.util.Optional;
 import java.util.zip.ZipInputStream;
 
 import static uk.gov.hmcts.reform.bulkscanprocessor.model.common.Event.ZIPFILE_PROCESSING_STARTED;
@@ -131,10 +132,12 @@ private void processZipFileIfEligible(BlobContainerClient container, String zipF
 
         if (existingEnvelope != null) {
             log.warn(
-                "Envelope for zip file {} (container {}) already exists. Aborting its processing. Envelope ID: {}",
+                "Envelope for zip file {} (container {}) already exists. Aborting its processing. Envelope ID: {}. "
+                    + "Case reference: {}",
                 zipFilename,
                 container.getBlobContainerName(),
-                existingEnvelope.getId()
+                existingEnvelope.getId(),
+                Optional.ofNullable(existingEnvelope.getCaseNumber()).orElse("(NOT PRESENT)")
             );
         } else if (Boolean.FALSE.equals(blobClient.exists())) {
             logAbortedProcessingNonExistingFile(zipFilename, container.getBlobContainerName());
diff --git a/src/main/java/uk/gov/hmcts/reform/bulkscanprocessor/tasks/jms/JmsBlobProcessorTask.java b/src/main/java/uk/gov/hmcts/reform/bulkscanprocessor/tasks/jms/JmsBlobProcessorTask.java
@@ -18,6 +18,7 @@
 
 import java.io.IOException;
 import java.util.List;
+import java.util.Optional;
 import java.util.zip.ZipInputStream;
 
 import static uk.gov.hmcts.reform.bulkscanprocessor.model.common.Event.ZIPFILE_PROCESSING_STARTED;
@@ -131,10 +132,12 @@ private void processZipFileIfEligible(BlobContainerClient container, String zipF
 
         if (existingEnvelope != null) {
             log.warn(
-                "Envelope for zip file {} (container {}) already exists. Aborting its processing. Envelope ID: {}",
+                "Envelope for zip file {} (container {}) already exists. Aborting its processing. Envelope ID: {}. "
+                    + "Case reference: {}",
                 zipFilename,
                 container.getBlobContainerName(),
-                existingEnvelope.getId()
+                existingEnvelope.getId(),
+                Optional.ofNullable(existingEnvelope.getCaseNumber()).orElse("(NOT PRESENT)")
             );
         } else if (Boolean.FALSE.equals(blobClient.exists())) {
             logAbortedProcessingNonExistingFile(zipFilename, container.getBlobContainerName());
diff --git a/src/main/java/uk/gov/hmcts/reform/bulkscanprocessor/tasks/processor/EnvelopeProcessor.java b/src/main/java/uk/gov/hmcts/reform/bulkscanprocessor/tasks/processor/EnvelopeProcessor.java
@@ -25,6 +25,7 @@
 import java.io.IOException;
 import java.util.List;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.UUID;
 
 import static uk.gov.hmcts.reform.bulkscanprocessor.entity.Status.UPLOAD_FAILURE;
@@ -156,8 +157,8 @@ public void markAsUploadFailure(Envelope envelope) {
         envelope.setStatus(UPLOAD_FAILURE);
 
         envelopeRepository.saveAndFlush(envelope);
-
-        log.info("Envelope {} status changed to UPLOAD_FAILURE", envelope.getZipFileName());
+        log.info("Envelope {} status changed to UPLOAD_FAILURE. Case reference {}",
+                 envelope.getZipFileName(), Optional.ofNullable(envelope.getCaseNumber()).orElse("(NOT PRESENT)"));
     }
 
     /**
@@ -202,8 +203,8 @@ public void handleEvent(Envelope envelope, Event event) {
         Status.fromEvent(event).ifPresent(status -> {
             envelope.setStatus(status);
             envelopeRepository.saveAndFlush(envelope);
-
-            log.info("Envelope {} status changed to {}", envelope.getZipFileName(), status.name());
+            log.info("Envelope {} status changed to {}. Case reference: {}", envelope.getZipFileName(), status.name(),
+                     Optional.ofNullable(envelope.getCaseNumber()).orElse("(NOT PRESENT)"));
         });
     }
 
diff --git a/src/test/java/uk/gov/hmcts/reform/bulkscanprocessor/services/DeleteFilesServiceTest.java b/src/test/java/uk/gov/hmcts/reform/bulkscanprocessor/services/DeleteFilesServiceTest.java
@@ -132,8 +132,10 @@ void should_handle_not_deleting_existing_file() {
         final BlobClient blobClient = mock(BlobClient.class);
 
         String zipFileName = randomUUID() + ".zip";
+        String caseNumber = "12564233455665";
         Envelope envelope = mock(Envelope.class);
         given(envelope.getZipFileName()).willReturn(zipFileName);
+        given(envelope.getCaseNumber()).willReturn(caseNumber);
 
         given(envelopeRepository
             .getCompleteEnvelopesFromContainer(container1.getBlobContainerName()))
@@ -172,8 +174,10 @@ void should_not_delete_file_if_exception_thrown() {
         // given
 
         String zipFileName = randomUUID() + ".zip";
+        String caseNumber = "1714725907404444";
         Envelope envelope = mock(Envelope.class);
         given(envelope.getZipFileName()).willReturn(zipFileName);
+        given(envelope.getCaseNumber()).willReturn(caseNumber);
 
         given(envelopeRepository.getCompleteEnvelopesFromContainer(CONTAINER_NAME_1))
             .willReturn(singletonList(envelope));
@@ -196,8 +200,10 @@ void should_handle_lease_acquire_exception() {
         final BlobClient blobClient = mock(BlobClient.class);
 
         String zipFileName = randomUUID() + ".zip";
+
         Envelope envelope = mock(Envelope.class);
         given(envelope.getZipFileName()).willReturn(zipFileName);
+        given(envelope.getCaseNumber()).willReturn(null);
 
         given(envelopeRepository
             .getCompleteEnvelopesFromContainer(container1.getBlobContainerName()))

Original file line number	Diff line number	Diff line change
`@@ -196,7 +196,7 @@ sonarqube {`
`196`	`196`	`property "sonar.coverage.jacoco.xmlReportPaths", jacocoTestReport.reports.xml.outputLocation.get().asFile.absolutePath`
`197`	`197`	`property "sonar.pitest.mode", "reuseReport"`
`198`	`198`	`property "sonar.pitest.reportsDirectory", "build/reports/pitest"`
`199`		`- property "sonar.exclusions", "*/model/out/,/config/,**/LeaseAcquirer.java"`
	`199`	`+ property "sonar.exclusions", "*/model/out/,/config/,/LeaseAcquirer.java,/services/jms/JmsFileContentProcessor.java,**/tasks/jms/JmsBlobProcessorTask.java"`
`200`	`200`	`}`
`201`	`201`	`}`
`202`	`202`