NIAD-3228: removing Jackson limit of maximum size of data that it can process (#980)

* removing Jackson limit of maximum size of data that it can process

* changelog

* covering ObjecMapper bean class with unit tests

* checkstyle

* adding more tests to confirm that objectMapper has unlimited readability

* configure objectMapper to ignore unknown properties

* adding javaTimeModule that is required by Jackson

* narrow down the scope where unknown Json property is more tolerated

* making Jackson mapper more tolerable

* removing data limit in Spring-instantiated objectMapper

* additional tests to cover setup in ObjectMapperPostProcessorConfig

* small code optimization

* removing an unnecessary dependency

* making objectMapper to be instantiated by Spring DI

* Move dependency construction into separate class

Using a separate bean construction method is less surprising than
a Bean Post Processor.

* Remove 20MB file from version control, and generate it as part of test instead.

---------

Co-authored-by: Adrian Clay <[email protected]>

Author: ORybak5

CHANGELOG.md

@@ -30,6 +30,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * When mapping `Immunizations` which contain a `NOPAT` `meta.security` tag, the resultant XML for that resource
   will contain a `NOPAT` `confidentialityCode` element.
 
+### Fixed
+* Removed the 20 MB data processing limit to enable the GP2GP Adaptor to handle larger documents.
+
 ## [2.1.3] - 2014-10-25
 
 ### Fixed

service/src/intTest/java/uk/nhs/adaptors/gp2gp/ehr/SendDocumentComponentTest.java

@@ -2,6 +2,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doThrow;
 
@@ -10,6 +11,9 @@
 import java.io.IOException;
 import java.io.InputStream;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.commons.io.IOUtils;
+import org.jetbrains.annotations.NotNull;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.runner.RunWith;
@@ -21,10 +25,12 @@
 import org.springframework.test.context.junit4.SpringRunner;
 
 import uk.nhs.adaptors.gp2gp.common.storage.LocalMockConnector;
+import uk.nhs.adaptors.gp2gp.common.storage.StorageDataWrapper;
 import uk.nhs.adaptors.gp2gp.ehr.model.EhrExtractStatus;
 import uk.nhs.adaptors.gp2gp.mhs.MhsClient;
 import uk.nhs.adaptors.gp2gp.mhs.exception.MhsConnectionException;
 import uk.nhs.adaptors.gp2gp.mhs.exception.MhsServerErrorException;
+import uk.nhs.adaptors.gp2gp.mhs.model.OutboundMessage;
 import uk.nhs.adaptors.gp2gp.testcontainers.ActiveMQExtension;
 import uk.nhs.adaptors.gp2gp.testcontainers.MongoDBExtension;
 
@@ -34,6 +40,7 @@
 @DirtiesContext
 public class SendDocumentComponentTest {
     private static final String DOCUMENT_NAME = "some-conversation-id/document-name.json";
+    public static final String OUTBOUND_MESSAGE_JSON = "src/intTest/resources/outboundMessage.json";
 
     @MockBean
     private MhsClient mhsClient;
@@ -47,6 +54,30 @@ public class SendDocumentComponentTest {
     @Autowired
     private LocalMockConnector localMockConnector;
 
+    @SuppressWarnings("checkstyle:MagicNumber")
+    @Test
+    public void When_SendDocumentTaskRunsWithOver20MbPayloadMessage_Expect_NoException() throws IOException {
+        InputStream inputStream = createInputStreamWithAttachmentOfSize(22 * 1024 * 1024);
+        localMockConnector.uploadToStorage(inputStream, inputStream.available(), DOCUMENT_NAME);
+        var ehrExtractStatus = EhrExtractStatusTestUtils.prepareEhrExtractStatus();
+        ehrExtractStatusRepository.save(ehrExtractStatus);
+
+        var sendDocumentTaskDefinition = prepareTaskDefinition(ehrExtractStatus);
+
+        assertDoesNotThrow(() -> sendDocumentTaskExecutor.execute(sendDocumentTaskDefinition));
+    }
+
+    private @NotNull InputStream createInputStreamWithAttachmentOfSize(int sizeOfAttachmentInBytes) throws IOException {
+        var inputStream = readMessageAsInputStream();
+        ObjectMapper mapper = new ObjectMapper();
+        StorageDataWrapper wrapper = mapper.readValue(inputStream, StorageDataWrapper.class);
+        OutboundMessage outboundMessage = mapper.readValue(wrapper.getData(), OutboundMessage.class);
+        outboundMessage.getAttachments().getFirst().setPayload("1".repeat(sizeOfAttachmentInBytes));
+        wrapper.setData(mapper.writeValueAsString(outboundMessage));
+
+        return IOUtils.toInputStream(mapper.writeValueAsString(wrapper), "UTF-8");
+    }
+
     @Test
     public void When_SendDocumentTaskRunsTwice_Expect_DatabaseOverwritesEhrExtractStatus() throws IOException {
         var inputStream = readMessageAsInputStream();
@@ -106,7 +137,7 @@ public void When_SendDocumentTask_WithMhsServerFailureException_Expect_Exception
     }
 
     private InputStream readMessageAsInputStream() throws IOException {
-        File file = new File("src/intTest/resources/outboundMessage.json");
+        File file = new File(OUTBOUND_MESSAGE_JSON);
         return new FileInputStream(file);
     }
 

service/src/main/java/uk/nhs/adaptors/gp2gp/common/configuration/ObjectMapperBean.java

@@ -0,0 +1,19 @@
+package uk.nhs.adaptors.gp2gp.common.configuration;
+
+import com.fasterxml.jackson.core.StreamReadConstraints;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.http.converter.json.Jackson2ObjectMapperBuilder;
+
+@Configuration
+public class ObjectMapperBean {
+    @Bean
+    public ObjectMapper objectMapper(Jackson2ObjectMapperBuilder builder) {
+        ObjectMapper objectMapper = builder.build();
+        objectMapper.getFactory().setStreamReadConstraints(
+            StreamReadConstraints.builder().maxStringLength(Integer.MAX_VALUE).build()
+        );
+        return objectMapper;
+    }
+}

service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/SendAcknowledgementTaskDefinition.java

@@ -1,7 +1,6 @@
 package uk.nhs.adaptors.gp2gp.ehr;
 
 import static uk.nhs.adaptors.gp2gp.common.task.TaskType.SEND_ACKNOWLEDGEMENT;
-
 import lombok.EqualsAndHashCode;
 import lombok.Getter;
 import lombok.experimental.SuperBuilder;

service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/SendDocumentTaskExecutor.java

@@ -12,7 +12,6 @@
 import org.springframework.stereotype.Service;
 import uk.nhs.adaptors.gp2gp.common.configuration.Gp2gpConfiguration;
 import uk.nhs.adaptors.gp2gp.common.service.RandomIdGeneratorService;
-import uk.nhs.adaptors.gp2gp.common.service.TimestampService;
 import uk.nhs.adaptors.gp2gp.common.storage.StorageConnectorService;
 import uk.nhs.adaptors.gp2gp.common.task.TaskExecutor;
 import uk.nhs.adaptors.gp2gp.ehr.model.EhrExtractStatus;
@@ -39,7 +38,6 @@ public class SendDocumentTaskExecutor implements TaskExecutor<SendDocumentTaskDe
     private final DetectDocumentsSentService detectDocumentsSentService;
     private final Gp2gpConfiguration gp2gpConfiguration;
     private final EhrDocumentMapper ehrDocumentMapper;
-    private final TimestampService timestampService;
 
     @Override
     public Class<SendDocumentTaskDefinition> getTaskType() {

service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/SendEhrExtractCoreTaskExecutor.java

@@ -48,6 +48,7 @@ public class SendEhrExtractCoreTaskExecutor implements TaskExecutor<SendEhrExtra
     private final RandomIdGeneratorService randomIdGeneratorService;
     private final TimestampService timestampService;
     private final EhrDocumentMapper ehrDocumentMapper;
+    private final ObjectMapper objectMapper;
 
     @Override
     public Class<SendEhrExtractCoreTaskDefinition> getTaskType() {
@@ -67,7 +68,7 @@ public void execute(SendEhrExtractCoreTaskDefinition sendEhrExtractCoreTaskDefin
         String outboundEhrExtract = replacePlaceholders(documentObjectNameAndSize, storageDataWrapper.getData());
 
         LOGGER.info("Checking EHR Extract size");
-        final var outboundMessage = new ObjectMapper().readValue(outboundEhrExtract, OutboundMessage.class);
+        final var outboundMessage = objectMapper.readValue(outboundEhrExtract, OutboundMessage.class);
         if (getBytesLengthOfString(outboundMessage.getPayload()) > gp2gpConfiguration.getLargeEhrExtractThreshold()) {
             LOGGER.info("EHR extract IS large");
             outboundEhrExtract = compressEhrExtractAndReplacePayloadWithSkeleton(sendEhrExtractCoreTaskDefinition, outboundMessage);
@@ -109,7 +110,7 @@ private String compressEhrExtractAndReplacePayloadWithSkeleton(
         referenceCompressedEhrExtractDocumentAsAttachmentInOutboundMessage(
                 outboundMessage, documentId, messageId, fileName, compressedEhrExtract.length());
 
-        return new ObjectMapper().writeValueAsString(outboundMessage);
+        return objectMapper.writeValueAsString(outboundMessage);
     }
 
     private void referenceCompressedEhrExtractDocumentAsAttachmentInOutboundMessage(
@@ -145,7 +146,7 @@ private void uploadCompressedEhrExtractToStorageWrapper(
             String taskId,
             String fileName) throws JsonProcessingException {
 
-        String data = new ObjectMapper().writeValueAsString(
+        String data = objectMapper.writeValueAsString(
             OutboundMessage.builder()
             .payload(
                 ehrDocumentMapper.generateMhsPayload(

service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/status/model/EhrStatusRequest.java

@@ -1,7 +1,6 @@
 package uk.nhs.adaptors.gp2gp.ehr.status.model;
 
 import java.time.Instant;
-
 import lombok.Builder;
 import lombok.Data;
 

service/src/main/java/uk/nhs/adaptors/gp2gp/gpc/DocumentToMHSTranslator.java

@@ -18,8 +18,8 @@
 @Component
 @AllArgsConstructor(onConstructor = @__(@Autowired))
 public class DocumentToMHSTranslator {
-    private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
 
+    private final ObjectMapper objectMapper;
     private final EhrDocumentMapper ehrDocumentMapper;
 
     public String translateGpcResponseToMhsOutboundRequestData(
@@ -66,6 +66,6 @@ private String prepareOutboundMessage(
             .attachments(attachments)
             .build();
 
-        return OBJECT_MAPPER.writeValueAsString(outboundMessage);
+        return objectMapper.writeValueAsString(outboundMessage);
     }
 }

service/src/test/java/uk/nhs/adaptors/gp2gp/common/configuration/ObjectMapperBeanTest.java

@@ -0,0 +1,22 @@
+package uk.nhs.adaptors.gp2gp.common.configuration;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.junit.jupiter.api.Test;
+import org.springframework.http.converter.json.Jackson2ObjectMapperBuilder;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class ObjectMapperBeanTest {
+
+    @Test
+    public void createsObjectMapperWithUnlimitedStringLengthConstraint() {
+        // The way we use JSON to store serialized XML and base64 encoded attachments means that the size of individual
+        // strings can get very large. We've set the max string size below to be 2GB.
+        // Ideal solution would be to avoid using JSON to store very large strings of data in the first place,
+        // and if we must then not use the ObjectMapper to extract that information.
+        ObjectMapperBean objectMapperBean = new ObjectMapperBean();
+        ObjectMapper objectMapper = objectMapperBean.objectMapper(new Jackson2ObjectMapperBuilder());
+
+        assertThat(objectMapper.getFactory().streamReadConstraints().getMaxStringLength()).isEqualTo(Integer.MAX_VALUE);
+    }
+}

service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/SendDocumentTaskExecutorTest.java

@@ -202,8 +202,7 @@ public void setup() {
                 new ObjectMapper(),
                 this.detectDocumentsSentService,
                 gp2gpConfiguration,
-                new EhrDocumentMapper(new TimestampService(), new RandomIdGeneratorServiceStub()),
-                new TimestampService()
+                new EhrDocumentMapper(new TimestampService(), new RandomIdGeneratorServiceStub())
         );
     }