If AutoInflateBinaries is enabled, binaries are created on the disk only for the first resource entry of the bundle (#5420)

* If AutoInflateBinaries is enabled, binaries created on disk by bundled requests are created only for the first resource entry - fix
This commit is contained in:
volodymyr-korzh 2023-11-01 11:17:22 -06:00 committed by GitHub
parent 4e295a59fb
commit 64cc704a40
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 98 additions and 24 deletions

View File

@ -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."

View File

@ -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);
}
}

View File

@ -251,7 +251,7 @@ public class BinaryStorageInterceptor<T extends IPrimitiveType<byte[]>> {
}
if (myBinaryStorageSvc.isValidBlobId(newBlobId)) {
List<DeferredBinaryTarget> deferredBinaryTargets =
getOrCreateDeferredBinaryStorageMap(theTransactionDetails);
getOrCreateDeferredBinaryStorageList(theResource);
DeferredBinaryTarget newDeferredBinaryTarget =
new DeferredBinaryTarget(newBlobId, nextTarget, data);
deferredBinaryTargets.add(newDeferredBinaryTarget);
@ -289,21 +289,29 @@ public class BinaryStorageInterceptor<T extends IPrimitiveType<byte[]>> {
}
@Nonnull
private List<DeferredBinaryTarget> getOrCreateDeferredBinaryStorageMap(TransactionDetails theTransactionDetails) {
return theTransactionDetails.getOrCreateUserData(getDeferredListKey(), ArrayList::new);
@SuppressWarnings("unchecked")
private List<DeferredBinaryTarget> getOrCreateDeferredBinaryStorageList(IBaseResource theResource) {
Object deferredBinaryTargetList = theResource.getUserData(getDeferredListKey());
if (deferredBinaryTargetList == null) {
deferredBinaryTargetList = new ArrayList<>();
theResource.setUserData(getDeferredListKey(), deferredBinaryTargetList);
}
return (List<DeferredBinaryTarget>) 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<DeferredBinaryTarget> 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<DeferredBinaryTarget>) deferredBinaryTargetList) {
String blobId = next.getBlobId();
IBinaryTarget target = next.getBinaryTarget();
InputStream dataStream = next.getDataStream();