From 1773c892a8b3f0037ecebc12015be702c018403a Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Mon, 29 Sep 2025 22:02:35 +0000 Subject: [PATCH 1/6] Add configurable filesystem binary storage (fix #860) --- README.md | 15 ++++ .../uhn/fhir/jpa/starter/AppProperties.java | 26 +++++- .../common/FhirServerConfigCommon.java | 42 +++++++-- src/main/resources/application.yaml | 8 ++ ...irServerConfigCommonBinaryStorageTest.java | 87 +++++++++++++++++++ 5 files changed, 172 insertions(+), 6 deletions(-) create mode 100644 src/test/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommonBinaryStorageTest.java diff --git a/README.md b/README.md index 5c2e2207df..96b11b8c1f 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,21 @@ docker run -p 8080:8080 -e hapi.fhir.default_encoding=xml hapiproject/hapi:lates HAPI looks in the environment variables for properties in the [application.yaml](https://github.com/hapifhir/hapi-fhir-jpaserver-starter/blob/master/src/main/resources/application.yaml) file for defaults. +### Binary storage configuration + +To stream large `Binary` payloads to disk instead of the database, configure the starter with filesystem storage properties: + +``` +hapi: + fhir: + binary_storage_enabled: true + binary_storage_mode: FILESYSTEM + binary_storage_filesystem_base_directory: /binstore + # inline_resource_storage_below_size: 131072 # optional override +``` + +When `binary_storage_mode` is set to `FILESYSTEM` and `inline_resource_storage_below_size` is omitted, the starter automatically applies a 102400 byte (100 KB) inline threshold so smaller payloads remain in the database. Ensure the directory you point to is writable by the process (for Docker builds, mount it into the container with appropriate permissions). + ### Configuration via overridden application.yaml file and using Docker You can customize HAPI by telling HAPI to look for the configuration file in a different location, e.g.: diff --git a/src/main/java/ca/uhn/fhir/jpa/starter/AppProperties.java b/src/main/java/ca/uhn/fhir/jpa/starter/AppProperties.java index f10f69333b..91b3ded5e7 100644 --- a/src/main/java/ca/uhn/fhir/jpa/starter/AppProperties.java +++ b/src/main/java/ca/uhn/fhir/jpa/starter/AppProperties.java @@ -61,7 +61,15 @@ public class AppProperties { private Boolean filter_search_enabled = true; private Boolean graphql_enabled = false; private Boolean binary_storage_enabled = false; - private Integer inline_resource_storage_below_size = 0; + + public enum BinaryStorageMode { + DATABASE, + FILESYSTEM + } + + private BinaryStorageMode binary_storage_mode = BinaryStorageMode.DATABASE; + private String binary_storage_filesystem_base_directory; + private Integer inline_resource_storage_below_size; private Boolean bulk_export_enabled = false; private Boolean bulk_import_enabled = false; private Boolean default_pretty_print = true; @@ -483,6 +491,22 @@ public void setBinary_storage_enabled(Boolean binary_storage_enabled) { this.binary_storage_enabled = binary_storage_enabled; } + public BinaryStorageMode getBinary_storage_mode() { + return binary_storage_mode; + } + + public void setBinary_storage_mode(BinaryStorageMode binary_storage_mode) { + this.binary_storage_mode = binary_storage_mode; + } + + public String getBinary_storage_filesystem_base_directory() { + return binary_storage_filesystem_base_directory; + } + + public void setBinary_storage_filesystem_base_directory(String binary_storage_filesystem_base_directory) { + this.binary_storage_filesystem_base_directory = binary_storage_filesystem_base_directory; + } + public Integer getInline_resource_storage_below_size() { return inline_resource_storage_below_size; } diff --git a/src/main/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommon.java b/src/main/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommon.java index f28a2c6c36..06d11ee51f 100644 --- a/src/main/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommon.java +++ b/src/main/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommon.java @@ -3,6 +3,7 @@ import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; import ca.uhn.fhir.jpa.binary.api.IBinaryStorageSvc; import ca.uhn.fhir.jpa.binstore.DatabaseBinaryContentStorageSvcImpl; +import ca.uhn.fhir.jpa.binstore.FilesystemBinaryStorageSvcImpl; import ca.uhn.fhir.jpa.config.HibernatePropertiesProvider; import ca.uhn.fhir.jpa.model.config.PartitionSettings; import ca.uhn.fhir.jpa.model.config.PartitionSettings.CrossPartitionReferenceMode; @@ -23,6 +24,7 @@ import org.springframework.context.annotation.*; import org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean; import org.springframework.transaction.annotation.EnableTransactionManagement; +import org.springframework.util.Assert; import java.util.HashSet; import java.util.stream.Collectors; @@ -38,6 +40,7 @@ public class FhirServerConfigCommon { private static final Logger ourLog = LoggerFactory.getLogger(FhirServerConfigCommon.class); + private static final int DEFAULT_FILESYSTEM_INLINE_THRESHOLD = 102_400; public FhirServerConfigCommon(AppProperties appProperties) { ourLog.info( @@ -222,8 +225,13 @@ public JpaStorageSettings jpaStorageSettings(AppProperties appProperties) { jpaStorageSettings.setLastNEnabled(true); } - if (appProperties.getInline_resource_storage_below_size() != 0) { - jpaStorageSettings.setInlineResourceTextBelowSize(appProperties.getInline_resource_storage_below_size()); + Integer inlineResourceThreshold = appProperties.getInline_resource_storage_below_size(); + if (inlineResourceThreshold == null + && appProperties.getBinary_storage_mode() == AppProperties.BinaryStorageMode.FILESYSTEM) { + inlineResourceThreshold = resolveFilesystemMinimumBinarySize(appProperties); + } + if (inlineResourceThreshold != null && inlineResourceThreshold != 0) { + jpaStorageSettings.setInlineResourceTextBelowSize(inlineResourceThreshold); } jpaStorageSettings.setStoreResourceInHSearchIndex(appProperties.getStore_resource_in_lucene_index_enabled()); @@ -342,15 +350,39 @@ public HibernatePropertiesProvider jpaStarterDialectProvider( @Lazy @Bean public IBinaryStorageSvc binaryStorageSvc(AppProperties appProperties) { - DatabaseBinaryContentStorageSvcImpl binaryStorageSvc = new DatabaseBinaryContentStorageSvcImpl(); + IBinaryStorageSvc binaryStorageSvc; + + if (appProperties.getBinary_storage_mode() == AppProperties.BinaryStorageMode.FILESYSTEM) { + String baseDirectory = appProperties.getBinary_storage_filesystem_base_directory(); + Assert.hasText( + baseDirectory, + "binary_storage_filesystem_base_directory must be provided when binary_storage_mode=FILESYSTEM"); + + FilesystemBinaryStorageSvcImpl filesystemSvc = new FilesystemBinaryStorageSvcImpl(baseDirectory); + int minimumBinarySize = resolveFilesystemMinimumBinarySize(appProperties); + filesystemSvc.setMinimumBinarySize(minimumBinarySize); + + if (appProperties.getMax_binary_size() != null) { + filesystemSvc.setMaximumBinarySize(appProperties.getMax_binary_size().longValue()); + } - if (appProperties.getMax_binary_size() != null) { - binaryStorageSvc.setMaximumBinarySize(appProperties.getMax_binary_size()); + binaryStorageSvc = filesystemSvc; + } else { + DatabaseBinaryContentStorageSvcImpl databaseSvc = new DatabaseBinaryContentStorageSvcImpl(); + if (appProperties.getMax_binary_size() != null) { + databaseSvc.setMaximumBinarySize(appProperties.getMax_binary_size()); + } + binaryStorageSvc = databaseSvc; } return binaryStorageSvc; } + private int resolveFilesystemMinimumBinarySize(AppProperties appProperties) { + Integer inlineThreshold = appProperties.getInline_resource_storage_below_size(); + return inlineThreshold == null ? DEFAULT_FILESYSTEM_INLINE_THRESHOLD : inlineThreshold; + } + @Bean public IEmailSender emailSender(AppProperties appProperties) { if (appProperties.getSubscription() != null diff --git a/src/main/resources/application.yaml b/src/main/resources/application.yaml index 967ba37889..f68b11c323 100644 --- a/src/main/resources/application.yaml +++ b/src/main/resources/application.yaml @@ -381,6 +381,14 @@ hapi: # max_page_size: 200 # retain_cached_searches_mins: 60 # reuse_cached_search_results_millis: 60000 + # validation: + # requests_enabled: true + # responses_enabled: true + # binary_storage_enabled: true + # binary_storage_mode: FILESYSTEM + # binary_storage_filesystem_base_directory: /binstore + # When binary_storage_mode is FILESYSTEM and this value is not set, + # the starter defaults to 102400 bytes so smaller binaries stay inline. inline_resource_storage_below_size: 4000 # ------------------------------------------------------------------------------- diff --git a/src/test/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommonBinaryStorageTest.java b/src/test/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommonBinaryStorageTest.java new file mode 100644 index 0000000000..a7017af8ec --- /dev/null +++ b/src/test/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommonBinaryStorageTest.java @@ -0,0 +1,87 @@ +package ca.uhn.fhir.jpa.starter.common; + +import ca.uhn.fhir.jpa.binary.api.IBinaryStorageSvc; +import ca.uhn.fhir.jpa.binstore.DatabaseBinaryContentStorageSvcImpl; +import ca.uhn.fhir.jpa.binstore.FilesystemBinaryStorageSvcImpl; +import ca.uhn.fhir.jpa.starter.AppProperties; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class FhirServerConfigCommonBinaryStorageTest { + + @TempDir + Path tempDir; + + private FhirServerConfigCommon newConfig() { + return new FhirServerConfigCommon(new AppProperties()); + } + + @Test + void defaultsToDatabaseImplementation() { + AppProperties props = new AppProperties(); + + IBinaryStorageSvc svc = newConfig().binaryStorageSvc(props); + + assertThat(svc).isInstanceOf(DatabaseBinaryContentStorageSvcImpl.class); + } + + @Test + void filesystemModeUsesDefaultMinimumWhenUnspecified() throws Exception { + AppProperties props = new AppProperties(); + props.setBinary_storage_mode(AppProperties.BinaryStorageMode.FILESYSTEM); + Path baseDir = tempDir.resolve("fs-default"); + Files.createDirectories(baseDir); + props.setBinary_storage_filesystem_base_directory(baseDir.toString()); + + FilesystemBinaryStorageSvcImpl svc = + (FilesystemBinaryStorageSvcImpl) newConfig().binaryStorageSvc(props); + + assertThat(svc.getMinimumBinarySize()).isEqualTo(102_400); + } + + @Test + void filesystemModeHonoursExplicitMinimum() throws Exception { + AppProperties props = new AppProperties(); + props.setBinary_storage_mode(AppProperties.BinaryStorageMode.FILESYSTEM); + props.setInline_resource_storage_below_size(4096); + Path baseDir = tempDir.resolve("fs-min-explicit"); + Files.createDirectories(baseDir); + props.setBinary_storage_filesystem_base_directory(baseDir.toString()); + + FilesystemBinaryStorageSvcImpl svc = + (FilesystemBinaryStorageSvcImpl) newConfig().binaryStorageSvc(props); + + assertThat(svc.getMinimumBinarySize()).isEqualTo(4096); + } + + @Test + void filesystemModeSupportsZeroMinimumWhenExplicit() throws Exception { + AppProperties props = new AppProperties(); + props.setBinary_storage_mode(AppProperties.BinaryStorageMode.FILESYSTEM); + props.setInline_resource_storage_below_size(0); + Path baseDir = tempDir.resolve("fs-zero"); + Files.createDirectories(baseDir); + props.setBinary_storage_filesystem_base_directory(baseDir.toString()); + + FilesystemBinaryStorageSvcImpl svc = + (FilesystemBinaryStorageSvcImpl) newConfig().binaryStorageSvc(props); + + assertThat(svc.getMinimumBinarySize()).isZero(); + } + + @Test + void filesystemModeRequiresBaseDirectory() { + AppProperties props = new AppProperties(); + props.setBinary_storage_mode(AppProperties.BinaryStorageMode.FILESYSTEM); + + assertThatThrownBy(() -> newConfig().binaryStorageSvc(props)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("binary_storage_filesystem_base_directory"); + } +} From 6f18c7975f9a5cea730dcd837c3db75f4ebb29c0 Mon Sep 17 00:00:00 2001 From: Shamus Husheer Date: Wed, 1 Oct 2025 19:59:00 +0000 Subject: [PATCH 2/6] Refine binary storage configuration handling --- .../common/FhirServerConfigCommon.java | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/main/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommon.java b/src/main/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommon.java index 06d11ee51f..f6e1ade17c 100644 --- a/src/main/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommon.java +++ b/src/main/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommon.java @@ -225,11 +225,7 @@ public JpaStorageSettings jpaStorageSettings(AppProperties appProperties) { jpaStorageSettings.setLastNEnabled(true); } - Integer inlineResourceThreshold = appProperties.getInline_resource_storage_below_size(); - if (inlineResourceThreshold == null - && appProperties.getBinary_storage_mode() == AppProperties.BinaryStorageMode.FILESYSTEM) { - inlineResourceThreshold = resolveFilesystemMinimumBinarySize(appProperties); - } + Integer inlineResourceThreshold = resolveInlineResourceThreshold(appProperties); if (inlineResourceThreshold != null && inlineResourceThreshold != 0) { jpaStorageSettings.setInlineResourceTextBelowSize(inlineResourceThreshold); } @@ -352,6 +348,9 @@ public HibernatePropertiesProvider jpaStarterDialectProvider( public IBinaryStorageSvc binaryStorageSvc(AppProperties appProperties) { IBinaryStorageSvc binaryStorageSvc; + Integer maxBinarySize = appProperties.getMax_binary_size(); + Integer inlineResourceThreshold = resolveInlineResourceThreshold(appProperties); + if (appProperties.getBinary_storage_mode() == AppProperties.BinaryStorageMode.FILESYSTEM) { String baseDirectory = appProperties.getBinary_storage_filesystem_base_directory(); Assert.hasText( @@ -359,18 +358,19 @@ public IBinaryStorageSvc binaryStorageSvc(AppProperties appProperties) { "binary_storage_filesystem_base_directory must be provided when binary_storage_mode=FILESYSTEM"); FilesystemBinaryStorageSvcImpl filesystemSvc = new FilesystemBinaryStorageSvcImpl(baseDirectory); - int minimumBinarySize = resolveFilesystemMinimumBinarySize(appProperties); + int minimumBinarySize = + inlineResourceThreshold == null ? DEFAULT_FILESYSTEM_INLINE_THRESHOLD : inlineResourceThreshold; filesystemSvc.setMinimumBinarySize(minimumBinarySize); - if (appProperties.getMax_binary_size() != null) { - filesystemSvc.setMaximumBinarySize(appProperties.getMax_binary_size().longValue()); + if (maxBinarySize != null) { + filesystemSvc.setMaximumBinarySize(maxBinarySize.longValue()); } binaryStorageSvc = filesystemSvc; } else { DatabaseBinaryContentStorageSvcImpl databaseSvc = new DatabaseBinaryContentStorageSvcImpl(); - if (appProperties.getMax_binary_size() != null) { - databaseSvc.setMaximumBinarySize(appProperties.getMax_binary_size()); + if (maxBinarySize != null) { + databaseSvc.setMaximumBinarySize(maxBinarySize.longValue()); } binaryStorageSvc = databaseSvc; } @@ -378,9 +378,13 @@ public IBinaryStorageSvc binaryStorageSvc(AppProperties appProperties) { return binaryStorageSvc; } - private int resolveFilesystemMinimumBinarySize(AppProperties appProperties) { - Integer inlineThreshold = appProperties.getInline_resource_storage_below_size(); - return inlineThreshold == null ? DEFAULT_FILESYSTEM_INLINE_THRESHOLD : inlineThreshold; + private Integer resolveInlineResourceThreshold(AppProperties appProperties) { + Integer inlineResourceThreshold = appProperties.getInline_resource_storage_below_size(); + if (inlineResourceThreshold == null + && appProperties.getBinary_storage_mode() == AppProperties.BinaryStorageMode.FILESYSTEM) { + return DEFAULT_FILESYSTEM_INLINE_THRESHOLD; + } + return inlineResourceThreshold; } @Bean From d5318d2790c6628da20c3e81b474f1c998f2b546 Mon Sep 17 00:00:00 2001 From: Shamus Husheer Date: Thu, 2 Oct 2025 20:28:23 +0000 Subject: [PATCH 3/6] Refine binary storage bean wiring --- .../common/FhirServerConfigCommon.java | 64 +++++++++++-------- ...irServerConfigCommonBinaryStorageTest.java | 52 ++++++++++++--- 2 files changed, 81 insertions(+), 35 deletions(-) diff --git a/src/main/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommon.java b/src/main/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommon.java index f6e1ade17c..67b8191172 100644 --- a/src/main/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommon.java +++ b/src/main/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommon.java @@ -20,6 +20,7 @@ import org.hl7.fhir.r4.model.Bundle.BundleType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.env.YamlPropertySourceLoader; import org.springframework.context.annotation.*; import org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean; @@ -344,38 +345,49 @@ public HibernatePropertiesProvider jpaStarterDialectProvider( } @Lazy + @Primary @Bean - public IBinaryStorageSvc binaryStorageSvc(AppProperties appProperties) { - IBinaryStorageSvc binaryStorageSvc; + public IBinaryStorageSvc binaryStorageSvc( + AppProperties appProperties, + ObjectProvider databaseBinaryStorageSvcProvider, + ObjectProvider filesystemBinaryStorageSvcProvider) { + if (appProperties.getBinary_storage_mode() == AppProperties.BinaryStorageMode.FILESYSTEM) { + return filesystemBinaryStorageSvcProvider.getObject(); + } + return databaseBinaryStorageSvcProvider.getObject(); + } - Integer maxBinarySize = appProperties.getMax_binary_size(); - Integer inlineResourceThreshold = resolveInlineResourceThreshold(appProperties); + @Lazy + @Bean + public FilesystemBinaryStorageSvcImpl filesystemBinaryStorageSvc(AppProperties appProperties) { + String baseDirectory = appProperties.getBinary_storage_filesystem_base_directory(); + Assert.hasText( + baseDirectory, + "binary_storage_filesystem_base_directory must be provided when binary_storage_mode=FILESYSTEM"); - if (appProperties.getBinary_storage_mode() == AppProperties.BinaryStorageMode.FILESYSTEM) { - String baseDirectory = appProperties.getBinary_storage_filesystem_base_directory(); - Assert.hasText( - baseDirectory, - "binary_storage_filesystem_base_directory must be provided when binary_storage_mode=FILESYSTEM"); - - FilesystemBinaryStorageSvcImpl filesystemSvc = new FilesystemBinaryStorageSvcImpl(baseDirectory); - int minimumBinarySize = - inlineResourceThreshold == null ? DEFAULT_FILESYSTEM_INLINE_THRESHOLD : inlineResourceThreshold; - filesystemSvc.setMinimumBinarySize(minimumBinarySize); - - if (maxBinarySize != null) { - filesystemSvc.setMaximumBinarySize(maxBinarySize.longValue()); - } + FilesystemBinaryStorageSvcImpl filesystemSvc = new FilesystemBinaryStorageSvcImpl(baseDirectory); + Integer inlineResourceThreshold = resolveInlineResourceThreshold(appProperties); + int minimumBinarySize = + inlineResourceThreshold == null ? DEFAULT_FILESYSTEM_INLINE_THRESHOLD : inlineResourceThreshold; + filesystemSvc.setMinimumBinarySize(minimumBinarySize); - binaryStorageSvc = filesystemSvc; - } else { - DatabaseBinaryContentStorageSvcImpl databaseSvc = new DatabaseBinaryContentStorageSvcImpl(); - if (maxBinarySize != null) { - databaseSvc.setMaximumBinarySize(maxBinarySize.longValue()); - } - binaryStorageSvc = databaseSvc; + Integer maxBinarySize = appProperties.getMax_binary_size(); + if (maxBinarySize != null) { + filesystemSvc.setMaximumBinarySize(maxBinarySize.longValue()); } - return binaryStorageSvc; + return filesystemSvc; + } + + @Lazy + @Bean + public DatabaseBinaryContentStorageSvcImpl databaseBinaryStorageSvc(AppProperties appProperties) { + DatabaseBinaryContentStorageSvcImpl databaseSvc = new DatabaseBinaryContentStorageSvcImpl(); + Integer maxBinarySize = appProperties.getMax_binary_size(); + if (maxBinarySize != null) { + databaseSvc.setMaximumBinarySize(maxBinarySize.longValue()); + } + return databaseSvc; } private Integer resolveInlineResourceThreshold(AppProperties appProperties) { diff --git a/src/test/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommonBinaryStorageTest.java b/src/test/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommonBinaryStorageTest.java index a7017af8ec..e454691241 100644 --- a/src/test/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommonBinaryStorageTest.java +++ b/src/test/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommonBinaryStorageTest.java @@ -1,14 +1,15 @@ package ca.uhn.fhir.jpa.starter.common; -import ca.uhn.fhir.jpa.binary.api.IBinaryStorageSvc; import ca.uhn.fhir.jpa.binstore.DatabaseBinaryContentStorageSvcImpl; import ca.uhn.fhir.jpa.binstore.FilesystemBinaryStorageSvcImpl; import ca.uhn.fhir.jpa.starter.AppProperties; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.springframework.beans.factory.ObjectProvider; import java.nio.file.Files; import java.nio.file.Path; +import java.util.function.Supplier; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -26,7 +27,7 @@ private FhirServerConfigCommon newConfig() { void defaultsToDatabaseImplementation() { AppProperties props = new AppProperties(); - IBinaryStorageSvc svc = newConfig().binaryStorageSvc(props); + Object svc = binaryStorageSvc(props); assertThat(svc).isInstanceOf(DatabaseBinaryContentStorageSvcImpl.class); } @@ -39,8 +40,7 @@ void filesystemModeUsesDefaultMinimumWhenUnspecified() throws Exception { Files.createDirectories(baseDir); props.setBinary_storage_filesystem_base_directory(baseDir.toString()); - FilesystemBinaryStorageSvcImpl svc = - (FilesystemBinaryStorageSvcImpl) newConfig().binaryStorageSvc(props); + FilesystemBinaryStorageSvcImpl svc = filesystemBinaryStorageSvc(props); assertThat(svc.getMinimumBinarySize()).isEqualTo(102_400); } @@ -54,8 +54,7 @@ void filesystemModeHonoursExplicitMinimum() throws Exception { Files.createDirectories(baseDir); props.setBinary_storage_filesystem_base_directory(baseDir.toString()); - FilesystemBinaryStorageSvcImpl svc = - (FilesystemBinaryStorageSvcImpl) newConfig().binaryStorageSvc(props); + FilesystemBinaryStorageSvcImpl svc = filesystemBinaryStorageSvc(props); assertThat(svc.getMinimumBinarySize()).isEqualTo(4096); } @@ -69,8 +68,7 @@ void filesystemModeSupportsZeroMinimumWhenExplicit() throws Exception { Files.createDirectories(baseDir); props.setBinary_storage_filesystem_base_directory(baseDir.toString()); - FilesystemBinaryStorageSvcImpl svc = - (FilesystemBinaryStorageSvcImpl) newConfig().binaryStorageSvc(props); + FilesystemBinaryStorageSvcImpl svc = filesystemBinaryStorageSvc(props); assertThat(svc.getMinimumBinarySize()).isZero(); } @@ -80,8 +78,44 @@ void filesystemModeRequiresBaseDirectory() { AppProperties props = new AppProperties(); props.setBinary_storage_mode(AppProperties.BinaryStorageMode.FILESYSTEM); - assertThatThrownBy(() -> newConfig().binaryStorageSvc(props)) + assertThatThrownBy(() -> filesystemBinaryStorageSvc(props)) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("binary_storage_filesystem_base_directory"); } + + private Object binaryStorageSvc(AppProperties props) { + FhirServerConfigCommon config = newConfig(); + return config.binaryStorageSvc( + props, + provider(() -> config.databaseBinaryStorageSvc(props)), + provider(() -> config.filesystemBinaryStorageSvc(props))); + } + + private FilesystemBinaryStorageSvcImpl filesystemBinaryStorageSvc(AppProperties props) { + return (FilesystemBinaryStorageSvcImpl) binaryStorageSvc(props); + } + + private static ObjectProvider provider(Supplier supplier) { + return new ObjectProvider<>() { + @Override + public T getObject(Object... args) { + return supplier.get(); + } + + @Override + public T getObject() { + return supplier.get(); + } + + @Override + public T getIfAvailable() { + return supplier.get(); + } + + @Override + public T getIfUnique() { + return supplier.get(); + } + }; + } } From 9624446316ed15a2c91b2b00e278434a04e5484d Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Sat, 4 Oct 2025 21:24:56 +0000 Subject: [PATCH 4/6] Refine binary storage conditional beans --- .../common/FhirServerConfigCommon.java | 24 ++++-------- ...irServerConfigCommonBinaryStorageTest.java | 39 ++++--------------- 2 files changed, 14 insertions(+), 49 deletions(-) diff --git a/src/main/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommon.java b/src/main/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommon.java index 67b8191172..a9f82af668 100644 --- a/src/main/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommon.java +++ b/src/main/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommon.java @@ -1,7 +1,6 @@ package ca.uhn.fhir.jpa.starter.common; import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; -import ca.uhn.fhir.jpa.binary.api.IBinaryStorageSvc; import ca.uhn.fhir.jpa.binstore.DatabaseBinaryContentStorageSvcImpl; import ca.uhn.fhir.jpa.binstore.FilesystemBinaryStorageSvcImpl; import ca.uhn.fhir.jpa.config.HibernatePropertiesProvider; @@ -20,7 +19,7 @@ import org.hl7.fhir.r4.model.Bundle.BundleType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.ObjectProvider; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.env.YamlPropertySourceLoader; import org.springframework.context.annotation.*; import org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean; @@ -344,21 +343,8 @@ public HibernatePropertiesProvider jpaStarterDialectProvider( return new JpaHibernatePropertiesProvider(myEntityManagerFactory); } - @Lazy - @Primary - @Bean - public IBinaryStorageSvc binaryStorageSvc( - AppProperties appProperties, - ObjectProvider databaseBinaryStorageSvcProvider, - ObjectProvider filesystemBinaryStorageSvcProvider) { - if (appProperties.getBinary_storage_mode() == AppProperties.BinaryStorageMode.FILESYSTEM) { - return filesystemBinaryStorageSvcProvider.getObject(); - } - return databaseBinaryStorageSvcProvider.getObject(); - } - - @Lazy @Bean + @ConditionalOnProperty(prefix = "hapi.fhir", name = "binary_storage_mode", havingValue = "FILESYSTEM") public FilesystemBinaryStorageSvcImpl filesystemBinaryStorageSvc(AppProperties appProperties) { String baseDirectory = appProperties.getBinary_storage_filesystem_base_directory(); Assert.hasText( @@ -379,8 +365,12 @@ public FilesystemBinaryStorageSvcImpl filesystemBinaryStorageSvc(AppProperties a return filesystemSvc; } - @Lazy @Bean + @ConditionalOnProperty( + prefix = "hapi.fhir", + name = "binary_storage_mode", + havingValue = "DATABASE", + matchIfMissing = true) public DatabaseBinaryContentStorageSvcImpl databaseBinaryStorageSvc(AppProperties appProperties) { DatabaseBinaryContentStorageSvcImpl databaseSvc = new DatabaseBinaryContentStorageSvcImpl(); Integer maxBinarySize = appProperties.getMax_binary_size(); diff --git a/src/test/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommonBinaryStorageTest.java b/src/test/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommonBinaryStorageTest.java index e454691241..bad6417ae1 100644 --- a/src/test/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommonBinaryStorageTest.java +++ b/src/test/java/ca/uhn/fhir/jpa/starter/common/FhirServerConfigCommonBinaryStorageTest.java @@ -2,14 +2,13 @@ import ca.uhn.fhir.jpa.binstore.DatabaseBinaryContentStorageSvcImpl; import ca.uhn.fhir.jpa.binstore.FilesystemBinaryStorageSvcImpl; +import ca.uhn.fhir.jpa.binary.api.IBinaryStorageSvc; import ca.uhn.fhir.jpa.starter.AppProperties; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; -import org.springframework.beans.factory.ObjectProvider; import java.nio.file.Files; import java.nio.file.Path; -import java.util.function.Supplier; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -27,7 +26,7 @@ private FhirServerConfigCommon newConfig() { void defaultsToDatabaseImplementation() { AppProperties props = new AppProperties(); - Object svc = binaryStorageSvc(props); + IBinaryStorageSvc svc = binaryStorageSvc(props); assertThat(svc).isInstanceOf(DatabaseBinaryContentStorageSvcImpl.class); } @@ -83,39 +82,15 @@ void filesystemModeRequiresBaseDirectory() { .hasMessageContaining("binary_storage_filesystem_base_directory"); } - private Object binaryStorageSvc(AppProperties props) { + private IBinaryStorageSvc binaryStorageSvc(AppProperties props) { FhirServerConfigCommon config = newConfig(); - return config.binaryStorageSvc( - props, - provider(() -> config.databaseBinaryStorageSvc(props)), - provider(() -> config.filesystemBinaryStorageSvc(props))); + if (props.getBinary_storage_mode() == AppProperties.BinaryStorageMode.FILESYSTEM) { + return config.filesystemBinaryStorageSvc(props); + } + return config.databaseBinaryStorageSvc(props); } private FilesystemBinaryStorageSvcImpl filesystemBinaryStorageSvc(AppProperties props) { return (FilesystemBinaryStorageSvcImpl) binaryStorageSvc(props); } - - private static ObjectProvider provider(Supplier supplier) { - return new ObjectProvider<>() { - @Override - public T getObject(Object... args) { - return supplier.get(); - } - - @Override - public T getObject() { - return supplier.get(); - } - - @Override - public T getIfAvailable() { - return supplier.get(); - } - - @Override - public T getIfUnique() { - return supplier.get(); - } - }; - } } From dc29187c101d492dddb0c71e2b6c06b5ed3a3a9b Mon Sep 17 00:00:00 2001 From: shusheer Date: Sun, 5 Oct 2025 20:11:23 +0000 Subject: [PATCH 5/6] Add integration coverage for binary storage configs --- .../starter/BinaryStorageIntegrationTest.java | 280 ++++++++++++++++++ .../resources/binary-storage-test-empty.yaml | 1 + 2 files changed, 281 insertions(+) create mode 100644 src/test/java/ca/uhn/fhir/jpa/starter/BinaryStorageIntegrationTest.java create mode 100644 src/test/resources/binary-storage-test-empty.yaml diff --git a/src/test/java/ca/uhn/fhir/jpa/starter/BinaryStorageIntegrationTest.java b/src/test/java/ca/uhn/fhir/jpa/starter/BinaryStorageIntegrationTest.java new file mode 100644 index 0000000000..28b6c9d0f0 --- /dev/null +++ b/src/test/java/ca/uhn/fhir/jpa/starter/BinaryStorageIntegrationTest.java @@ -0,0 +1,280 @@ +package ca.uhn.fhir.jpa.starter; + +import ca.uhn.fhir.jpa.binary.api.IBinaryStorageSvc; +import ca.uhn.fhir.jpa.binary.api.StoredDetails; +import ca.uhn.fhir.jpa.binstore.DatabaseBinaryContentStorageSvcImpl; +import ca.uhn.fhir.jpa.binstore.FilesystemBinaryStorageSvcImpl; +import ca.uhn.fhir.jpa.dao.data.IBinaryStorageEntityDao; +import ca.uhn.fhir.jpa.model.entity.BinaryStorageEntity; +import ca.uhn.fhir.rest.api.server.SystemRequestDetails; +import org.hl7.fhir.r4.model.IdType; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.support.TransactionTemplate; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Comparator; +import java.util.concurrent.ThreadLocalRandom; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; + +abstract class BaseBinaryStorageIntegrationTest { + protected static final String COMMON_CONFIG_LOCATION = "spring.config.location=classpath:/binary-storage-test-empty.yaml"; + protected static final String COMMON_H2_USERNAME = "spring.datasource.username=sa"; + protected static final String COMMON_H2_PASSWORD = "spring.datasource.password="; + protected static final String COMMON_JPA_DDL = "spring.jpa.hibernate.ddl-auto=create-drop"; + protected static final String COMMON_HIBERNATE_DIALECT = + "spring.jpa.properties.hibernate.dialect=ca.uhn.fhir.jpa.model.dialect.HapiFhirH2Dialect"; + protected static final String COMMON_HIBERNATE_SEARCH_DISABLED = "spring.jpa.properties.hibernate.search.enabled=false"; + protected static final String COMMON_FLYWAY_DISABLED = "spring.flyway.enabled=false"; + protected static final String COMMON_FHIR_VERSION = "hapi.fhir.fhir_version=r4"; + protected static final String COMMON_REPO_VALIDATION_DISABLED = + "hapi.fhir.enable_repository_validating_interceptor=false"; + protected static final String COMMON_MDM_DISABLED = "hapi.fhir.mdm_enabled=false"; + protected static final String COMMON_CR_DISABLED = "hapi.fhir.cr_enabled=false"; + protected static final String COMMON_SUBSCRIPTION_WS_DISABLED = "hapi.fhir.subscription.websocket_enabled=false"; + protected static final String COMMON_BEAN_OVERRIDE_ALLOWED = "spring.main.allow-bean-definition-overriding=true"; + protected static final String COMMON_CIRCULAR_REFERENCES = "spring.main.allow-circular-references=true"; + protected static final String COMMON_MCP_DISABLED = "spring.ai.mcp.server.enabled=false"; + + protected StoredDetails storeBinary( + IBinaryStorageSvc binaryStorageSvc, IdType resourceId, int size, String contentType) throws IOException { + byte[] payload = randomBytes(size); + SystemRequestDetails requestDetails = new SystemRequestDetails(); + StoredDetails stored; + try (ByteArrayInputStream input = new ByteArrayInputStream(payload)) { + stored = + binaryStorageSvc.storeBinaryContent(resourceId, null, contentType, input, requestDetails); + } + assertThat(binaryStorageSvc.fetchBinaryContent(resourceId, stored.getBinaryContentId())) + .containsExactly(payload); + return stored; + } + + protected byte[] randomBytes(int size) { + byte[] payload = new byte[size]; + ThreadLocalRandom.current().nextBytes(payload); + return payload; + } + + protected void assertRegularFileCount(Path baseDir, long expectedFileCount) throws IOException { + assertThat(regularFileCount(baseDir)).isEqualTo(expectedFileCount); + } + + protected void assertRegularFileCountGreaterThan(Path baseDir, long minimumFileCount) throws IOException { + assertThat(regularFileCount(baseDir)).isGreaterThan(minimumFileCount); + } + + private long regularFileCount(Path baseDir) throws IOException { + if (Files.notExists(baseDir)) { + return 0; + } + try (Stream files = Files.walk(baseDir)) { + return files.filter(Files::isRegularFile).count(); + } + } + + protected void deleteDirectoryContents(Path baseDir) throws IOException { + if (Files.notExists(baseDir)) { + return; + } + try (Stream files = Files.walk(baseDir)) { + files.sorted(Comparator.reverseOrder()) + .filter(path -> !path.equals(baseDir)) + .forEach(path -> { + try { + Files.deleteIfExists(path); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }); + } + } +} + +@SpringBootTest( + webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, + classes = Application.class, + properties = { + BaseBinaryStorageIntegrationTest.COMMON_CONFIG_LOCATION, + "spring.datasource.url=jdbc:h2:mem:binary-storage-db;DB_CLOSE_DELAY=-1", + BaseBinaryStorageIntegrationTest.COMMON_H2_USERNAME, + BaseBinaryStorageIntegrationTest.COMMON_H2_PASSWORD, + BaseBinaryStorageIntegrationTest.COMMON_JPA_DDL, + BaseBinaryStorageIntegrationTest.COMMON_HIBERNATE_DIALECT, + BaseBinaryStorageIntegrationTest.COMMON_HIBERNATE_SEARCH_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_FLYWAY_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_FHIR_VERSION, + BaseBinaryStorageIntegrationTest.COMMON_REPO_VALIDATION_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_MDM_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_CR_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_SUBSCRIPTION_WS_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_BEAN_OVERRIDE_ALLOWED, + BaseBinaryStorageIntegrationTest.COMMON_CIRCULAR_REFERENCES, + BaseBinaryStorageIntegrationTest.COMMON_MCP_DISABLED, + "hapi.fhir.binary_storage_mode=DATABASE" + } +) +class BinaryStorageDatabaseModeIT extends BaseBinaryStorageIntegrationTest { + + @Autowired + private IBinaryStorageSvc binaryStorageSvc; + + @Autowired + private IBinaryStorageEntityDao binaryStorageEntityDao; + + @Autowired + private PlatformTransactionManager transactionManager; + + @Test + void databaseModeStoresBinaryContentInDatabase() throws IOException { + assertThat(binaryStorageSvc).isInstanceOf(DatabaseBinaryContentStorageSvcImpl.class); + + storeAndAssertDatabasePersistence(new IdType("Binary/database-small"), 24); + storeAndAssertDatabasePersistence(new IdType("Binary/database-large"), 150_000); + } + + private void storeAndAssertDatabasePersistence(IdType resourceId, int size) throws IOException { + StoredDetails stored = storeBinary(binaryStorageSvc, resourceId, size, "application/octet-stream"); + + TransactionTemplate transactionTemplate = new TransactionTemplate(transactionManager); + BinaryStorageEntity entity = + transactionTemplate.execute(status -> + binaryStorageEntityDao + .findByIdAndResourceId(stored.getBinaryContentId(), resourceId.toUnqualifiedVersionless().getValue()) + .orElseThrow()); + + assertThat(entity.hasStorageContent()).isTrue(); + assertThat(entity.getStorageContentBin()).hasSize(size); + + binaryStorageSvc.expungeBinaryContent(resourceId, stored.getBinaryContentId()); + } +} + +@SpringBootTest( + webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, + classes = Application.class, + properties = { + BaseBinaryStorageIntegrationTest.COMMON_CONFIG_LOCATION, + "spring.datasource.url=jdbc:h2:mem:binary-storage-fs-default;DB_CLOSE_DELAY=-1", + BaseBinaryStorageIntegrationTest.COMMON_H2_USERNAME, + BaseBinaryStorageIntegrationTest.COMMON_H2_PASSWORD, + BaseBinaryStorageIntegrationTest.COMMON_JPA_DDL, + BaseBinaryStorageIntegrationTest.COMMON_HIBERNATE_DIALECT, + BaseBinaryStorageIntegrationTest.COMMON_HIBERNATE_SEARCH_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_FLYWAY_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_FHIR_VERSION, + BaseBinaryStorageIntegrationTest.COMMON_REPO_VALIDATION_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_MDM_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_CR_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_SUBSCRIPTION_WS_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_BEAN_OVERRIDE_ALLOWED, + BaseBinaryStorageIntegrationTest.COMMON_CIRCULAR_REFERENCES, + BaseBinaryStorageIntegrationTest.COMMON_MCP_DISABLED, + "hapi.fhir.binary_storage_mode=FILESYSTEM", + "hapi.fhir.binary_storage_filesystem_base_directory=" + BinaryStorageFilesystemDefaultIT.BASE_DIRECTORY + } +) +class BinaryStorageFilesystemDefaultIT extends BaseBinaryStorageIntegrationTest { + static final String BASE_DIRECTORY = "target/test-binary-storage/filesystem-default"; + + @Autowired + private FilesystemBinaryStorageSvcImpl filesystemBinaryStorageSvc; + + @AfterEach + void cleanUp() throws IOException { + deleteDirectoryContents(baseDir()); + } + + @Test + void filesystemModeUsesDefaultThresholdForLargePayloads() throws Exception { + assertThat(filesystemBinaryStorageSvc.getMinimumBinarySize()).isEqualTo(102_400); + + IdType resourceId = new IdType("Binary/filesystem-default"); + String contentType = "application/octet-stream"; + + assertThat(filesystemBinaryStorageSvc.shouldStoreBinaryContent(50_000, resourceId, contentType)).isFalse(); + assertRegularFileCount(baseDir(), 0); + + StoredDetails largeStored = storeBinary(filesystemBinaryStorageSvc, resourceId, 150_000, contentType); + + assertRegularFileCountGreaterThan(baseDir(), 0); + + filesystemBinaryStorageSvc.expungeBinaryContent(resourceId, largeStored.getBinaryContentId()); + } + + private Path baseDir() throws IOException { + Path baseDir = Paths.get(BASE_DIRECTORY).toAbsolutePath(); + Files.createDirectories(baseDir); + return baseDir; + } +} + +@SpringBootTest( + webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, + classes = Application.class, + properties = { + BaseBinaryStorageIntegrationTest.COMMON_CONFIG_LOCATION, + "spring.datasource.url=jdbc:h2:mem:binary-storage-fs-custom;DB_CLOSE_DELAY=-1", + BaseBinaryStorageIntegrationTest.COMMON_H2_USERNAME, + BaseBinaryStorageIntegrationTest.COMMON_H2_PASSWORD, + BaseBinaryStorageIntegrationTest.COMMON_JPA_DDL, + BaseBinaryStorageIntegrationTest.COMMON_HIBERNATE_DIALECT, + BaseBinaryStorageIntegrationTest.COMMON_HIBERNATE_SEARCH_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_FLYWAY_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_FHIR_VERSION, + BaseBinaryStorageIntegrationTest.COMMON_REPO_VALIDATION_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_MDM_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_CR_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_SUBSCRIPTION_WS_DISABLED, + BaseBinaryStorageIntegrationTest.COMMON_BEAN_OVERRIDE_ALLOWED, + BaseBinaryStorageIntegrationTest.COMMON_CIRCULAR_REFERENCES, + BaseBinaryStorageIntegrationTest.COMMON_MCP_DISABLED, + "hapi.fhir.binary_storage_mode=FILESYSTEM", + "hapi.fhir.binary_storage_filesystem_base_directory=" + BinaryStorageFilesystemCustomThresholdIT.BASE_DIRECTORY, + "hapi.fhir.inline_resource_storage_below_size=32768" + } +) +class BinaryStorageFilesystemCustomThresholdIT extends BaseBinaryStorageIntegrationTest { + static final String BASE_DIRECTORY = "target/test-binary-storage/filesystem-custom"; + + @Autowired + private FilesystemBinaryStorageSvcImpl filesystemBinaryStorageSvc; + + @AfterEach + void cleanUp() throws IOException { + deleteDirectoryContents(baseDir()); + } + + @Test + void filesystemModeHonoursCustomThreshold() throws Exception { + assertThat(filesystemBinaryStorageSvc.getMinimumBinarySize()).isEqualTo(32_768); + + IdType resourceId = new IdType("Binary/filesystem-custom"); + String contentType = "application/octet-stream"; + + assertThat(filesystemBinaryStorageSvc.shouldStoreBinaryContent(30_000, resourceId, contentType)).isFalse(); + assertRegularFileCount(baseDir(), 0); + + StoredDetails stored = storeBinary(filesystemBinaryStorageSvc, resourceId, 40_000, contentType); + + assertRegularFileCountGreaterThan(baseDir(), 0); + + filesystemBinaryStorageSvc.expungeBinaryContent(resourceId, stored.getBinaryContentId()); + } + + private Path baseDir() throws IOException { + Path baseDir = Paths.get(BASE_DIRECTORY).toAbsolutePath(); + Files.createDirectories(baseDir); + return baseDir; + } +} diff --git a/src/test/resources/binary-storage-test-empty.yaml b/src/test/resources/binary-storage-test-empty.yaml new file mode 100644 index 0000000000..d889d02848 --- /dev/null +++ b/src/test/resources/binary-storage-test-empty.yaml @@ -0,0 +1 @@ +# Empty config to bypass default application.yaml during BinaryStorageIntegrationTest From 2caff64b5a3dbf131b3da8a6ba03e9adbf6b5462 Mon Sep 17 00:00:00 2001 From: shusheer Date: Mon, 6 Oct 2025 22:17:54 +0000 Subject: [PATCH 6/6] Exercise binary storage via REST integration tests --- .../starter/BinaryStorageIntegrationTest.java | 208 +++++++++++------- 1 file changed, 126 insertions(+), 82 deletions(-) diff --git a/src/test/java/ca/uhn/fhir/jpa/starter/BinaryStorageIntegrationTest.java b/src/test/java/ca/uhn/fhir/jpa/starter/BinaryStorageIntegrationTest.java index 28b6c9d0f0..f40368e18f 100644 --- a/src/test/java/ca/uhn/fhir/jpa/starter/BinaryStorageIntegrationTest.java +++ b/src/test/java/ca/uhn/fhir/jpa/starter/BinaryStorageIntegrationTest.java @@ -1,28 +1,35 @@ package ca.uhn.fhir.jpa.starter; -import ca.uhn.fhir.jpa.binary.api.IBinaryStorageSvc; -import ca.uhn.fhir.jpa.binary.api.StoredDetails; -import ca.uhn.fhir.jpa.binstore.DatabaseBinaryContentStorageSvcImpl; +import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.jpa.binstore.FilesystemBinaryStorageSvcImpl; import ca.uhn.fhir.jpa.dao.data.IBinaryStorageEntityDao; import ca.uhn.fhir.jpa.model.entity.BinaryStorageEntity; -import ca.uhn.fhir.rest.api.server.SystemRequestDetails; -import org.hl7.fhir.r4.model.IdType; +import ca.uhn.fhir.rest.client.api.IGenericClient; +import ca.uhn.fhir.rest.client.api.ServerValidationModeEnum; +import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r4.model.Patient; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.web.server.LocalServerPort; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.support.TransactionTemplate; -import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; import java.util.Comparator; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; +import java.util.UUID; import java.util.concurrent.ThreadLocalRandom; +import java.util.stream.Collectors; import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; @@ -45,19 +52,48 @@ abstract class BaseBinaryStorageIntegrationTest { protected static final String COMMON_BEAN_OVERRIDE_ALLOWED = "spring.main.allow-bean-definition-overriding=true"; protected static final String COMMON_CIRCULAR_REFERENCES = "spring.main.allow-circular-references=true"; protected static final String COMMON_MCP_DISABLED = "spring.ai.mcp.server.enabled=false"; + protected static final String CONTENT_TYPE = "application/octet-stream"; + + @LocalServerPort + protected int port; + + protected FhirContext fhirContext; + protected IGenericClient client; + private final List resourcesToDelete = new ArrayList<>(); + + @BeforeEach + void setUpClient() { + fhirContext = FhirContext.forR4(); + fhirContext.getRestfulClientFactory().setServerValidationMode(ServerValidationModeEnum.NEVER); + fhirContext.getRestfulClientFactory().setSocketTimeout(1200 * 1000); + String serverBase = "http://localhost:" + port + "/fhir/"; + client = fhirContext.newRestfulGenericClient(serverBase); + resourcesToDelete.clear(); + } - protected StoredDetails storeBinary( - IBinaryStorageSvc binaryStorageSvc, IdType resourceId, int size, String contentType) throws IOException { - byte[] payload = randomBytes(size); - SystemRequestDetails requestDetails = new SystemRequestDetails(); - StoredDetails stored; - try (ByteArrayInputStream input = new ByteArrayInputStream(payload)) { - stored = - binaryStorageSvc.storeBinaryContent(resourceId, null, contentType, input, requestDetails); + @AfterEach + void deleteCreatedResources() { + for (IIdType id : resourcesToDelete) { + try { + client.delete().resourceById(id).execute(); + } catch (Exception ignored) { + // Ignore cleanup failures to keep tests resilient + } } - assertThat(binaryStorageSvc.fetchBinaryContent(resourceId, stored.getBinaryContentId())) - .containsExactly(payload); - return stored; + } + + protected IIdType createPatientWithPhoto(String label, byte[] payload) { + Patient patient = new Patient(); + patient.addIdentifier().setSystem("urn:binary-storage-test").setValue(label); + patient.addName().setFamily(label); + patient.addPhoto().setContentType(CONTENT_TYPE).setData(payload); + IIdType id = client.create().resource(patient).execute().getId().toUnqualifiedVersionless(); + resourcesToDelete.add(id); + return id; + } + + protected String uniqueLabel(String prefix) { + return prefix + "-" + UUID.randomUUID(); } protected byte[] randomBytes(int size) { @@ -74,13 +110,9 @@ protected void assertRegularFileCountGreaterThan(Path baseDir, long minimumFileC assertThat(regularFileCount(baseDir)).isGreaterThan(minimumFileCount); } - private long regularFileCount(Path baseDir) throws IOException { - if (Files.notExists(baseDir)) { - return 0; - } - try (Stream files = Files.walk(baseDir)) { - return files.filter(Files::isRegularFile).count(); - } + protected Path ensureDirectory(Path directory) throws IOException { + Files.createDirectories(directory); + return directory; } protected void deleteDirectoryContents(Path baseDir) throws IOException { @@ -99,6 +131,15 @@ protected void deleteDirectoryContents(Path baseDir) throws IOException { }); } } + + private long regularFileCount(Path baseDir) throws IOException { + if (Files.notExists(baseDir)) { + return 0; + } + try (Stream files = Files.walk(baseDir)) { + return files.filter(Files::isRegularFile).count(); + } + } } @SpringBootTest( @@ -121,42 +162,53 @@ protected void deleteDirectoryContents(Path baseDir) throws IOException { BaseBinaryStorageIntegrationTest.COMMON_BEAN_OVERRIDE_ALLOWED, BaseBinaryStorageIntegrationTest.COMMON_CIRCULAR_REFERENCES, BaseBinaryStorageIntegrationTest.COMMON_MCP_DISABLED, + "hapi.fhir.binary_storage_enabled=true", "hapi.fhir.binary_storage_mode=DATABASE" } ) class BinaryStorageDatabaseModeIT extends BaseBinaryStorageIntegrationTest { - @Autowired - private IBinaryStorageSvc binaryStorageSvc; - @Autowired private IBinaryStorageEntityDao binaryStorageEntityDao; @Autowired private PlatformTransactionManager transactionManager; - @Test - void databaseModeStoresBinaryContentInDatabase() throws IOException { - assertThat(binaryStorageSvc).isInstanceOf(DatabaseBinaryContentStorageSvcImpl.class); + private TransactionTemplate transactionTemplate; - storeAndAssertDatabasePersistence(new IdType("Binary/database-small"), 24); - storeAndAssertDatabasePersistence(new IdType("Binary/database-large"), 150_000); + @BeforeEach + void initTemplate() { + transactionTemplate = new TransactionTemplate(transactionManager); } - private void storeAndAssertDatabasePersistence(IdType resourceId, int size) throws IOException { - StoredDetails stored = storeBinary(binaryStorageSvc, resourceId, size, "application/octet-stream"); + @Test + void largeAttachmentStoredInDatabase() { + Set beforeIds = captureContentIds(); + + createPatientWithPhoto(uniqueLabel("database"), randomBytes(150_000)); + + Set afterIds = captureContentIds(); + afterIds.removeAll(beforeIds); + assertThat(afterIds).hasSize(1); - TransactionTemplate transactionTemplate = new TransactionTemplate(transactionManager); - BinaryStorageEntity entity = - transactionTemplate.execute(status -> - binaryStorageEntityDao - .findByIdAndResourceId(stored.getBinaryContentId(), resourceId.toUnqualifiedVersionless().getValue()) - .orElseThrow()); + String binaryId = afterIds.iterator().next(); + BinaryStorageEntity entity = transactionTemplate.execute(status -> + binaryStorageEntityDao.findById(binaryId).orElseThrow()); assertThat(entity.hasStorageContent()).isTrue(); - assertThat(entity.getStorageContentBin()).hasSize(size); + assertThat(entity.getStorageContentBin()).hasSize(150_000); + + transactionTemplate.execute(status -> { + binaryStorageEntityDao.deleteById(binaryId); + return null; + }); + } - binaryStorageSvc.expungeBinaryContent(resourceId, stored.getBinaryContentId()); + private Set captureContentIds() { + return transactionTemplate.execute(status -> + binaryStorageEntityDao.findAll().stream() + .map(BinaryStorageEntity::getContentId) + .collect(Collectors.toCollection(LinkedHashSet::new))); } } @@ -180,42 +232,38 @@ private void storeAndAssertDatabasePersistence(IdType resourceId, int size) thro BaseBinaryStorageIntegrationTest.COMMON_BEAN_OVERRIDE_ALLOWED, BaseBinaryStorageIntegrationTest.COMMON_CIRCULAR_REFERENCES, BaseBinaryStorageIntegrationTest.COMMON_MCP_DISABLED, + "hapi.fhir.binary_storage_enabled=true", "hapi.fhir.binary_storage_mode=FILESYSTEM", - "hapi.fhir.binary_storage_filesystem_base_directory=" + BinaryStorageFilesystemDefaultIT.BASE_DIRECTORY + "hapi.fhir.binary_storage_filesystem_base_directory=target/test-binary-storage/filesystem-default" } ) class BinaryStorageFilesystemDefaultIT extends BaseBinaryStorageIntegrationTest { - static final String BASE_DIRECTORY = "target/test-binary-storage/filesystem-default"; + static final Path BASE_DIRECTORY = Paths.get("target/test-binary-storage/filesystem-default").toAbsolutePath(); @Autowired private FilesystemBinaryStorageSvcImpl filesystemBinaryStorageSvc; - @AfterEach - void cleanUp() throws IOException { - deleteDirectoryContents(baseDir()); + @BeforeEach + void prepareDirectory() throws IOException { + ensureDirectory(BASE_DIRECTORY); + deleteDirectoryContents(BASE_DIRECTORY); } @Test - void filesystemModeUsesDefaultThresholdForLargePayloads() throws Exception { + void filesystemModeUsesDefaultThreshold() throws IOException { assertThat(filesystemBinaryStorageSvc.getMinimumBinarySize()).isEqualTo(102_400); + assertRegularFileCount(BASE_DIRECTORY, 0); - IdType resourceId = new IdType("Binary/filesystem-default"); - String contentType = "application/octet-stream"; - - assertThat(filesystemBinaryStorageSvc.shouldStoreBinaryContent(50_000, resourceId, contentType)).isFalse(); - assertRegularFileCount(baseDir(), 0); - - StoredDetails largeStored = storeBinary(filesystemBinaryStorageSvc, resourceId, 150_000, contentType); + createPatientWithPhoto(uniqueLabel("fs-default-inline"), randomBytes(50_000)); + assertRegularFileCount(BASE_DIRECTORY, 0); - assertRegularFileCountGreaterThan(baseDir(), 0); - - filesystemBinaryStorageSvc.expungeBinaryContent(resourceId, largeStored.getBinaryContentId()); + createPatientWithPhoto(uniqueLabel("fs-default-offload"), randomBytes(150_000)); + assertRegularFileCountGreaterThan(BASE_DIRECTORY, 0); } - private Path baseDir() throws IOException { - Path baseDir = Paths.get(BASE_DIRECTORY).toAbsolutePath(); - Files.createDirectories(baseDir); - return baseDir; + @AfterEach + void cleanUpDirectory() throws IOException { + deleteDirectoryContents(BASE_DIRECTORY); } } @@ -239,42 +287,38 @@ private Path baseDir() throws IOException { BaseBinaryStorageIntegrationTest.COMMON_BEAN_OVERRIDE_ALLOWED, BaseBinaryStorageIntegrationTest.COMMON_CIRCULAR_REFERENCES, BaseBinaryStorageIntegrationTest.COMMON_MCP_DISABLED, + "hapi.fhir.binary_storage_enabled=true", "hapi.fhir.binary_storage_mode=FILESYSTEM", - "hapi.fhir.binary_storage_filesystem_base_directory=" + BinaryStorageFilesystemCustomThresholdIT.BASE_DIRECTORY, + "hapi.fhir.binary_storage_filesystem_base_directory=target/test-binary-storage/filesystem-custom", "hapi.fhir.inline_resource_storage_below_size=32768" } ) class BinaryStorageFilesystemCustomThresholdIT extends BaseBinaryStorageIntegrationTest { - static final String BASE_DIRECTORY = "target/test-binary-storage/filesystem-custom"; + static final Path BASE_DIRECTORY = Paths.get("target/test-binary-storage/filesystem-custom").toAbsolutePath(); @Autowired private FilesystemBinaryStorageSvcImpl filesystemBinaryStorageSvc; - @AfterEach - void cleanUp() throws IOException { - deleteDirectoryContents(baseDir()); + @BeforeEach + void prepareDirectory() throws IOException { + ensureDirectory(BASE_DIRECTORY); + deleteDirectoryContents(BASE_DIRECTORY); } @Test - void filesystemModeHonoursCustomThreshold() throws Exception { + void filesystemModeHonoursCustomThreshold() throws IOException { assertThat(filesystemBinaryStorageSvc.getMinimumBinarySize()).isEqualTo(32_768); + assertRegularFileCount(BASE_DIRECTORY, 0); - IdType resourceId = new IdType("Binary/filesystem-custom"); - String contentType = "application/octet-stream"; - - assertThat(filesystemBinaryStorageSvc.shouldStoreBinaryContent(30_000, resourceId, contentType)).isFalse(); - assertRegularFileCount(baseDir(), 0); - - StoredDetails stored = storeBinary(filesystemBinaryStorageSvc, resourceId, 40_000, contentType); + createPatientWithPhoto(uniqueLabel("fs-custom-inline"), randomBytes(30_000)); + assertRegularFileCount(BASE_DIRECTORY, 0); - assertRegularFileCountGreaterThan(baseDir(), 0); - - filesystemBinaryStorageSvc.expungeBinaryContent(resourceId, stored.getBinaryContentId()); + createPatientWithPhoto(uniqueLabel("fs-custom-offload"), randomBytes(40_000)); + assertRegularFileCountGreaterThan(BASE_DIRECTORY, 0); } - private Path baseDir() throws IOException { - Path baseDir = Paths.get(BASE_DIRECTORY).toAbsolutePath(); - Files.createDirectories(baseDir); - return baseDir; + @AfterEach + void cleanUpDirectory() throws IOException { + deleteDirectoryContents(BASE_DIRECTORY); } }