diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_10_0/5419-binaries-created-only-for-the-first-resource-entry-of-the-bundle.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_10_0/5419-binaries-created-only-for-the-first-resource-entry-of-the-bundle.yaml new file mode 100644 index 00000000000..e1394edddcc --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_10_0/5419-binaries-created-only-for-the-first-resource-entry-of-the-bundle.yaml @@ -0,0 +1,6 @@ +--- +type: fix +issue: 5419 +title: "Previously, when `AllowAutoInflateBinaries` was enabled in `JpaStorageSettings` and bundles with multiple +resources were submitted, binaries were created on the disk only for the first resource entry of the bundle. +This has been fixed." diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/BinaryStorageInterceptorR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/BinaryStorageInterceptorR4Test.java index 91d4a521037..273b382d924 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/BinaryStorageInterceptorR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/BinaryStorageInterceptorR4Test.java @@ -10,6 +10,7 @@ import ca.uhn.fhir.jpa.binary.interceptor.BinaryStorageInterceptor; import ca.uhn.fhir.jpa.binstore.MemoryBinaryStorageSvcImpl; import ca.uhn.fhir.jpa.model.entity.StorageSettings; import ca.uhn.fhir.jpa.provider.BaseResourceProviderR4Test; +import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.client.api.IClientInterceptor; @@ -18,13 +19,16 @@ import ca.uhn.fhir.rest.client.api.IHttpResponse; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.util.HapiExtensions; import org.hl7.fhir.instance.model.api.IBaseHasExtensions; -import org.hl7.fhir.instance.model.api.IBaseMetaType; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Binary; +import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.DocumentReference; import org.hl7.fhir.r4.model.Enumerations; import org.hl7.fhir.r4.model.Extension; +import org.hl7.fhir.r4.model.IdType; +import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.Resource; import org.hl7.fhir.r4.model.StringType; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -32,7 +36,6 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; -import org.junit.jupiter.params.provider.ValueSource; import org.mockito.junit.jupiter.MockitoExtension; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -62,6 +65,7 @@ public class BinaryStorageInterceptorR4Test extends BaseResourceProviderR4Test { public static final byte[] FEW_BYTES = {4, 3, 2, 1}; public static final byte[] SOME_BYTES = {1, 2, 3, 4, 5, 6, 7, 8, 7, 6, 5, 4, 3, 2, 1, 8, 9, 0, 10, 9}; public static final byte[] SOME_BYTES_2 = {6, 7, 8, 7, 6, 5, 4, 3, 2, 1, 5, 5, 5, 6}; + public static final byte[] SOME_BYTES_3 = {5, 5, 5, 6, 6, 7, 7, 7, 8, 8, 8}; private static final Logger ourLog = LoggerFactory.getLogger(BinaryStorageInterceptorR4Test.class); @Autowired @@ -381,12 +385,8 @@ public class BinaryStorageInterceptorR4Test extends BaseResourceProviderR4Test { // Create a resource with a big enough docRef DocumentReference docRef = new DocumentReference(); - DocumentReference.DocumentReferenceContentComponent content = docRef.addContent(); - content.getAttachment().setContentType("application/octet-stream"); - content.getAttachment().setData(SOME_BYTES); - DocumentReference.DocumentReferenceContentComponent content2 = docRef.addContent(); - content2.getAttachment().setContentType("application/octet-stream"); - content2.getAttachment().setData(SOME_BYTES_2); + addDocumentAttachmentData(docRef, SOME_BYTES); + addDocumentAttachmentData(docRef, SOME_BYTES_2); DaoMethodOutcome outcome = myDocumentReferenceDao.create(docRef, mySrd); // Make sure it was externalized @@ -422,18 +422,73 @@ public class BinaryStorageInterceptorR4Test extends BaseResourceProviderR4Test { } + @Test + public void testCreateBinaryAttachments_bundleWithMultipleDocumentReferences_createdAndReadBackSuccessfully() { + // Create Patient + Patient patient = new Patient(); + patient.addIdentifier().setSystem("urn:system").setValue("001"); + patient.addName().addGiven("Johnny").setFamily("Walker"); + + // Create first DocumentReference with a big enough attachments + DocumentReference docRef = new DocumentReference(); + addDocumentAttachmentData(docRef, SOME_BYTES); + addDocumentAttachmentData(docRef, SOME_BYTES_2); + + // Create second DocumentReference with a big enough attachment + DocumentReference docRef2 = new DocumentReference(); + addDocumentAttachmentData(docRef2, SOME_BYTES_3); + + // Create Bundle + Bundle bundle = new Bundle(); + bundle.setType(Bundle.BundleType.TRANSACTION); + // Patient entry component + addBundleEntry(bundle, patient, "Patient"); + // First DocumentReference entry component + addBundleEntry(bundle, docRef, "DocumentReference"); + // Second DocumentReference entry component + addBundleEntry(bundle, docRef2, "DocumentReference"); + + // Execute transaction + Bundle output = myClient.transaction().withBundle(bundle).execute(); + ourLog.debug(myFhirContext.newXmlParser().setPrettyPrint(true).encodeResourceToString(output)); + + // Verify bundle response + assertEquals(3, output.getEntry().size()); + output.getEntry().forEach(entry -> assertEquals("201 Created", entry.getResponse().getStatus())); + + // Read back and verify first DocumentReference and attachments + IIdType firstDocRef = new IdType(output.getEntry().get(1).getResponse().getLocation()); + DocumentReference firstDoc = myDocumentReferenceDao.read(firstDocRef, mySrd); + assertEquals("application/octet-stream", firstDoc.getContentFirstRep().getAttachment().getContentType()); + assertArrayEquals(SOME_BYTES, firstDoc.getContentFirstRep().getAttachment().getData()); + assertEquals("application/octet-stream", firstDoc.getContent().get(1).getAttachment().getContentType()); + assertArrayEquals(SOME_BYTES_2, firstDoc.getContent().get(1).getAttachment().getData()); + + // Read back and verify second DocumentReference and attachment + IIdType secondDocRef = new IdType(output.getEntry().get(2).getResponse().getLocation()); + DocumentReference secondDoc = myDocumentReferenceDao.read(secondDocRef, mySrd); + assertEquals("application/octet-stream", secondDoc.getContentFirstRep().getAttachment().getContentType()); + assertArrayEquals(SOME_BYTES_3, secondDoc.getContentFirstRep().getAttachment().getData()); + } + + private void addBundleEntry(Bundle theBundle, Resource theResource, String theUrl) { + Bundle.BundleEntryComponent getComponent = new Bundle.BundleEntryComponent(); + Bundle.BundleEntryRequestComponent requestComponent = new Bundle.BundleEntryRequestComponent(); + requestComponent.setMethod(Bundle.HTTPVerb.POST); + requestComponent.setUrl(theUrl); + getComponent.setRequest(requestComponent); + getComponent.setResource(theResource); + getComponent.setFullUrl(IdDt.newRandomUuid().getValue()); + theBundle.addEntry(getComponent); + } @Test public void testUpdateRejectsIncorrectBinary() { // Create a resource with a big enough docRef DocumentReference docRef = new DocumentReference(); - DocumentReference.DocumentReferenceContentComponent content = docRef.addContent(); - content.getAttachment().setContentType("application/octet-stream"); - content.getAttachment().setData(SOME_BYTES); - DocumentReference.DocumentReferenceContentComponent content2 = docRef.addContent(); - content2.getAttachment().setContentType("application/octet-stream"); - content2.getAttachment().setData(SOME_BYTES_2); + addDocumentAttachmentData(docRef, SOME_BYTES); + addDocumentAttachmentData(docRef, SOME_BYTES_2); DaoMethodOutcome outcome = myDocumentReferenceDao.create(docRef, mySrd); // Make sure it was externalized @@ -449,13 +504,13 @@ public class BinaryStorageInterceptorR4Test extends BaseResourceProviderR4Test { docRef = new DocumentReference(); docRef.setId(id.toUnqualifiedVersionless()); docRef.setStatus(Enumerations.DocumentReferenceStatus.CURRENT); - content = docRef.addContent(); + DocumentReference.DocumentReferenceContentComponent content = docRef.addContent(); content.getAttachment().setContentType("application/octet-stream"); content.getAttachment().getDataElement().addExtension( HapiExtensions.EXT_EXTERNALIZED_BINARY_ID, new StringType(binaryId) ); - content2 = docRef.addContent(); + DocumentReference.DocumentReferenceContentComponent content2 = docRef.addContent(); content2.getAttachment().setContentType("application/octet-stream"); content2.getAttachment().getDataElement().addExtension( HapiExtensions.EXT_EXTERNALIZED_BINARY_ID, @@ -497,5 +552,10 @@ public class BinaryStorageInterceptorR4Test extends BaseResourceProviderR4Test { } + private void addDocumentAttachmentData(DocumentReference theDocumentReference, byte[] theData) { + DocumentReference.DocumentReferenceContentComponent content = theDocumentReference.addContent(); + content.getAttachment().setContentType("application/octet-stream"); + content.getAttachment().setData(theData); + } } diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/binary/interceptor/BinaryStorageInterceptor.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/binary/interceptor/BinaryStorageInterceptor.java index 0ff58ec7cf3..bf32c4a758f 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/binary/interceptor/BinaryStorageInterceptor.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/binary/interceptor/BinaryStorageInterceptor.java @@ -251,7 +251,7 @@ public class BinaryStorageInterceptor> { } if (myBinaryStorageSvc.isValidBlobId(newBlobId)) { List deferredBinaryTargets = - getOrCreateDeferredBinaryStorageMap(theTransactionDetails); + getOrCreateDeferredBinaryStorageList(theResource); DeferredBinaryTarget newDeferredBinaryTarget = new DeferredBinaryTarget(newBlobId, nextTarget, data); deferredBinaryTargets.add(newDeferredBinaryTarget); @@ -289,21 +289,29 @@ public class BinaryStorageInterceptor> { } @Nonnull - private List getOrCreateDeferredBinaryStorageMap(TransactionDetails theTransactionDetails) { - return theTransactionDetails.getOrCreateUserData(getDeferredListKey(), ArrayList::new); + @SuppressWarnings("unchecked") + private List getOrCreateDeferredBinaryStorageList(IBaseResource theResource) { + Object deferredBinaryTargetList = theResource.getUserData(getDeferredListKey()); + if (deferredBinaryTargetList == null) { + deferredBinaryTargetList = new ArrayList<>(); + theResource.setUserData(getDeferredListKey(), deferredBinaryTargetList); + } + return (List) deferredBinaryTargetList; } + @SuppressWarnings("unchecked") @Hook(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED) public void storeLargeBinariesBeforeCreatePersistence( - TransactionDetails theTransactionDetails, IBaseResource theResource, Pointcut thePoincut) + TransactionDetails theTransactionDetails, IBaseResource theResource, Pointcut thePointcut) throws IOException { - if (theTransactionDetails == null) { + if (theResource == null) { return; } - List deferredBinaryTargets = theTransactionDetails.getUserData(getDeferredListKey()); - if (deferredBinaryTargets != null) { + Object deferredBinaryTargetList = theResource.getUserData(getDeferredListKey()); + + if (deferredBinaryTargetList != null) { IIdType resourceId = theResource.getIdElement(); - for (DeferredBinaryTarget next : deferredBinaryTargets) { + for (DeferredBinaryTarget next : (List) deferredBinaryTargetList) { String blobId = next.getBlobId(); IBinaryTarget target = next.getBinaryTarget(); InputStream dataStream = next.getDataStream();