Add implementation and changelog, and test. (#3554)

* Add implementation and changelog, and test.

* Add daoconfig settings

* Updatae test

* Rename deexternalize -> autoinflate
This commit is contained in:
Tadgh 2022-04-20 10:47:02 -07:00 committed by GitHub
parent d20c6a7d25
commit 578d42c955
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 117 additions and 11 deletions

View File

@ -0,0 +1,4 @@
type: fix
issue: 3553
jira: SMILE-3964
title: "Added a new setting to BinaryStorageInterceptor which allows you to disable binary de-externalization during resource reads."

View File

@ -261,8 +261,11 @@ public class JpaConfig {
@Bean(name = "myBinaryStorageInterceptor") @Bean(name = "myBinaryStorageInterceptor")
@Lazy @Lazy
public BinaryStorageInterceptor binaryStorageInterceptor() { public BinaryStorageInterceptor binaryStorageInterceptor(DaoConfig theDaoConfig) {
return new BinaryStorageInterceptor(); BinaryStorageInterceptor interceptor = new BinaryStorageInterceptor();
interceptor.setAllowAutoInflateBinaries(theDaoConfig.isAllowAutoInflateBinaries());
interceptor.setAutoInflateBinariesMaximumSize(theDaoConfig.getAutoInflateBinariesMaximumBytes());
return interceptor;
} }
@Bean @Bean

View File

@ -20,6 +20,8 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.blankOrNullString; import static org.hamcrest.Matchers.blankOrNullString;
import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.containsString;
@ -47,6 +49,7 @@ public class BinaryStorageInterceptorR4Test extends BaseResourceProviderR4Test {
super.before(); super.before();
myStorageSvc.setMinimumBinarySize(10); myStorageSvc.setMinimumBinarySize(10);
myDaoConfig.setExpungeEnabled(true); myDaoConfig.setExpungeEnabled(true);
myInterceptorRegistry.registerInterceptor(myBinaryStorageInterceptor); myInterceptorRegistry.registerInterceptor(myBinaryStorageInterceptor);
} }
@ -56,7 +59,8 @@ public class BinaryStorageInterceptorR4Test extends BaseResourceProviderR4Test {
super.after(); super.after();
myStorageSvc.setMinimumBinarySize(0); myStorageSvc.setMinimumBinarySize(0);
myDaoConfig.setExpungeEnabled(new DaoConfig().isExpungeEnabled()); myDaoConfig.setExpungeEnabled(new DaoConfig().isExpungeEnabled());
myBinaryStorageInterceptor.setAutoDeExternalizeMaximumBytes(new BinaryStorageInterceptor().getAutoDeExternalizeMaximumBytes()); myBinaryStorageInterceptor.setAutoInflateBinariesMaximumSize(new BinaryStorageInterceptor().getAutoInflateBinariesMaximumSize());
myBinaryStorageInterceptor.setAllowAutoInflateBinaries(new BinaryStorageInterceptor().isAllowAutoInflateBinaries());
MemoryBinaryStorageSvcImpl binaryStorageSvc = (MemoryBinaryStorageSvcImpl) myBinaryStorageSvc; MemoryBinaryStorageSvcImpl binaryStorageSvc = (MemoryBinaryStorageSvcImpl) myBinaryStorageSvc;
binaryStorageSvc.clear(); binaryStorageSvc.clear();
@ -109,6 +113,29 @@ public class BinaryStorageInterceptorR4Test extends BaseResourceProviderR4Test {
} }
@Test
public void testCreateAndRetrieveBinary_ServerAssignedId_ExternalizedBinary_DoNotRehydrate() {
myBinaryStorageInterceptor.setAllowAutoInflateBinaries(false);
// Create a resource with a big enough binary
Binary binary = new Binary();
binary.setContentType("application/octet-stream");
binary.setData(SOME_BYTES);
DaoMethodOutcome outcome = myBinaryDao.create(binary);
// Make sure it was externalized
IIdType id = outcome.getId().toUnqualifiedVersionless();
String encoded = myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome.getResource());
ourLog.info("Encoded: {}", encoded);
assertThat(encoded, containsString(HapiExtensions.EXT_EXTERNALIZED_BINARY_ID));
assertThat(encoded, not(containsString("\"data\"")));
// Now read it back and make sure it was not de-externalized
Binary output = myBinaryDao.read(id, mySrd);
assertEquals("application/octet-stream", output.getContentType());
assertArrayEquals(null, output.getData());
assertThat(output.getDataElement().getExtensionByUrl(HapiExtensions.EXT_EXTERNALIZED_BINARY_ID), is(notNullValue()));
}
@Test @Test
public void testCreateAndRetrieveBinary_ServerAssignedId_NonExternalizedBinary() { public void testCreateAndRetrieveBinary_ServerAssignedId_NonExternalizedBinary() {
@ -294,7 +321,7 @@ public class BinaryStorageInterceptorR4Test extends BaseResourceProviderR4Test {
@Test @Test
public void testRetrieveBinaryAboveRetrievalThreshold() { public void testRetrieveBinaryAboveRetrievalThreshold() {
myBinaryStorageInterceptor.setAutoDeExternalizeMaximumBytes(5); myBinaryStorageInterceptor.setAutoInflateBinariesMaximumSize(5);
// Create a resource with a big enough binary // Create a resource with a big enough binary
Binary binary = new Binary(); Binary binary = new Binary();

View File

@ -9,6 +9,7 @@ import ca.uhn.fhir.util.HapiExtensions;
import ca.uhn.fhir.validation.FhirValidator; import ca.uhn.fhir.validation.FhirValidator;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Sets; import com.google.common.collect.Sets;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.Validate;
import org.apache.commons.lang3.time.DateUtils; import org.apache.commons.lang3.time.DateUtils;
@ -298,6 +299,15 @@ public class DaoConfig {
*/ */
private boolean myConcurrentBundleValidation; private boolean myConcurrentBundleValidation;
/**
* Since 6.0.0
*/
private boolean myAllowAutoInflateBinaries = true;
/**
* Since 6.0.0
*/
private long myAutoInflateBinariesMaximumBytes = 10 * FileUtils.ONE_MB;
/** /**
* Constructor * Constructor
*/ */
@ -2793,12 +2803,61 @@ public class DaoConfig {
* This setting indicates if a cross-partition subscription can be made. * This setting indicates if a cross-partition subscription can be made.
* *
* @see ModelConfig#setCrossPartitionSubscription(boolean) * @see ModelConfig#setCrossPartitionSubscription(boolean)
* @since 7.5.0 * @since 5.7.0
*/ */
public void setCrossPartitionSubscription(boolean theAllowCrossPartitionSubscription) { public void setCrossPartitionSubscription(boolean theAllowCrossPartitionSubscription) {
this.myModelConfig.setCrossPartitionSubscription(theAllowCrossPartitionSubscription); this.myModelConfig.setCrossPartitionSubscription(theAllowCrossPartitionSubscription);
} }
/**
*
* This setting indicates whether binaries are allowed to be automatically inflated from external storage during requests.
* Default is true.
*
* @since 6.0.0
* @return whether binaries are allowed to be automatically inflated from external storage during requests.
*/
public boolean isAllowAutoInflateBinaries() {
return myAllowAutoInflateBinaries;
}
/**
* This setting indicates whether binaries are allowed to be automatically inflated from external storage during requests.
* Default is true.
*
* @since 6.0.0
* @param theAllowAutoDeExternalizingBinaries the value to set.
*/
public void setAllowAutoInflateBinaries(boolean theAllowAutoDeExternalizingBinaries) {
myAllowAutoInflateBinaries = theAllowAutoDeExternalizingBinaries;
}
/**
* This setting controls how many bytes of binaries will be automatically inflated from external storage during requests.
* which contain binary data.
* Default is 10MB
*
* @since 6.0.0
* @param theAutoInflateBinariesMaximumBytes the maximum number of bytes to de-externalize.
*/
public void setAutoInflateBinariesMaximumBytes(long theAutoInflateBinariesMaximumBytes) {
myAutoInflateBinariesMaximumBytes = theAutoInflateBinariesMaximumBytes;
}
/**
* This setting controls how many bytes of binaries will be automatically inflated from external storage during requests.
* which contain binary data.
* Default is 10MB
*
* @since 6.0.0
* @return the number of bytes to de-externalize during requests.
*/
public long getAutoInflateBinariesMaximumBytes() {
return myAutoInflateBinariesMaximumBytes;
}
public enum StoreMetaSourceInformationEnum { public enum StoreMetaSourceInformationEnum {
NONE(false, false), NONE(false, false),
SOURCE_URI(true, false), SOURCE_URI(true, false),

View File

@ -76,22 +76,23 @@ public class BinaryStorageInterceptor {
private BinaryAccessProvider myBinaryAccessProvider; private BinaryAccessProvider myBinaryAccessProvider;
private Class<? extends IPrimitiveType<byte[]>> myBinaryType; private Class<? extends IPrimitiveType<byte[]>> myBinaryType;
private String myDeferredListKey; private String myDeferredListKey;
private long myAutoDeExternalizeMaximumBytes = 10 * FileUtils.ONE_MB; private long myAutoInflateBinariesMaximumBytes = 10 * FileUtils.ONE_MB;
private boolean myAllowAutoInflateBinaries = true;
/** /**
* Any externalized binaries will be rehydrated if their size is below this thhreshold when * Any externalized binaries will be rehydrated if their size is below this thhreshold when
* reading the resource back. Default is 10MB. * reading the resource back. Default is 10MB.
*/ */
public long getAutoDeExternalizeMaximumBytes() { public long getAutoInflateBinariesMaximumSize() {
return myAutoDeExternalizeMaximumBytes; return myAutoInflateBinariesMaximumBytes;
} }
/** /**
* Any externalized binaries will be rehydrated if their size is below this thhreshold when * Any externalized binaries will be rehydrated if their size is below this thhreshold when
* reading the resource back. Default is 10MB. * reading the resource back. Default is 10MB.
*/ */
public void setAutoDeExternalizeMaximumBytes(long theAutoDeExternalizeMaximumBytes) { public void setAutoInflateBinariesMaximumSize(long theAutoInflateBinariesMaximumBytes) {
myAutoDeExternalizeMaximumBytes = theAutoDeExternalizeMaximumBytes; myAutoInflateBinariesMaximumBytes = theAutoInflateBinariesMaximumBytes;
} }
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
@ -248,6 +249,10 @@ public class BinaryStorageInterceptor {
@Hook(Pointcut.STORAGE_PRESHOW_RESOURCES) @Hook(Pointcut.STORAGE_PRESHOW_RESOURCES)
public void preShow(IPreResourceShowDetails theDetails) throws IOException { public void preShow(IPreResourceShowDetails theDetails) throws IOException {
if (!isAllowAutoInflateBinaries()) {
return;
}
long unmarshalledByteCount = 0; long unmarshalledByteCount = 0;
for (IBaseResource nextResource : theDetails) { for (IBaseResource nextResource : theDetails) {
@ -265,7 +270,7 @@ public class BinaryStorageInterceptor {
throw new InvalidRequestException(Msg.code(1330) + msg); throw new InvalidRequestException(Msg.code(1330) + msg);
} }
if ((unmarshalledByteCount + blobDetails.getBytes()) < myAutoDeExternalizeMaximumBytes) { if ((unmarshalledByteCount + blobDetails.getBytes()) < myAutoInflateBinariesMaximumBytes) {
byte[] bytes = myBinaryStorageSvc.fetchBlob(resourceId, attachmentId.get()); byte[] bytes = myBinaryStorageSvc.fetchBlob(resourceId, attachmentId.get());
nextTarget.setData(bytes); nextTarget.setData(bytes);
@ -295,6 +300,14 @@ public class BinaryStorageInterceptor {
return binaryTargets; return binaryTargets;
} }
public void setAllowAutoInflateBinaries(boolean theAllowAutoInflateBinaries) {
myAllowAutoInflateBinaries = theAllowAutoInflateBinaries;
}
public boolean isAllowAutoInflateBinaries() {
return myAllowAutoInflateBinaries;
}
private static class DeferredBinaryTarget { private static class DeferredBinaryTarget {
private final String myBlobId; private final String myBlobId;
private final IBinaryTarget myBinaryTarget; private final IBinaryTarget myBinaryTarget;