From f862098ce6822dd0cd5ad209f02d1fa002f63f31 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Wed, 21 Sep 2022 22:15:05 -0700 Subject: [PATCH] Add test and changelog (#4039) * Add test and changelog * Fix up tests --- .../fhir/cli/UploadTerminologyCommand.java | 33 ++++++++++++++----- .../cli/UploadTerminologyCommandTest.java | 27 +++++++++++++-- ...3276-modifiable-maximum-transmit-size.yaml | 6 ++++ 3 files changed, 55 insertions(+), 11 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3276-modifiable-maximum-transmit-size.yaml diff --git a/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/UploadTerminologyCommand.java b/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/UploadTerminologyCommand.java index d590997c5e2..6bca38ccd78 100644 --- a/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/UploadTerminologyCommand.java +++ b/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/UploadTerminologyCommand.java @@ -31,7 +31,6 @@ import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; import ca.uhn.fhir.util.AttachmentUtil; import ca.uhn.fhir.util.FileUtil; import ca.uhn.fhir.util.ParametersUtil; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Charsets; import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.Options; @@ -42,6 +41,7 @@ import org.apache.commons.io.input.CountingInputStream; import org.hl7.fhir.instance.model.api.IBaseParameters; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.ICompositeType; +import org.springframework.util.unit.DataSize; import java.io.ByteArrayOutputStream; import java.io.File; @@ -58,7 +58,11 @@ public class UploadTerminologyCommand extends BaseRequestGeneratingCommand { static final String UPLOAD_TERMINOLOGY = "upload-terminology"; private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(UploadTerminologyCommand.class); private static final long DEFAULT_TRANSFER_SIZE_LIMIT = 10 * FileUtils.ONE_MB; - private static long ourTransferSizeLimit = DEFAULT_TRANSFER_SIZE_LIMIT; + private long ourTransferSizeLimit = DEFAULT_TRANSFER_SIZE_LIMIT; + + public long getTransferSizeLimit() { + return ourTransferSizeLimit; + } @Override public String getCommandDescription() { @@ -77,6 +81,7 @@ public class UploadTerminologyCommand extends BaseRequestGeneratingCommand { addRequiredOption(options, "u", "url", true, "The code system URL associated with this upload (e.g. " + ITermLoaderSvc.SCT_URI + ")"); addOptionalOption(options, "d", "data", true, "Local file to use to upload (can be a raw file or a ZIP containing the raw file)"); addOptionalOption(options, "m", "mode", true, "The upload mode: SNAPSHOT (default), ADD, REMOVE"); + addOptionalOption(options, "s", "size", true, "The maximum size of a single upload (default: 10MB). Examples: 150 kb, 3 mb, 1GB"); return options; } @@ -103,6 +108,9 @@ public class UploadTerminologyCommand extends BaseRequestGeneratingCommand { throw new ParseException(Msg.code(1540) + "No data file provided"); } + String sizeString = theCommandLine.getOptionValue("s"); + this.setTransferSizeLimitHuman(sizeString); + IGenericClient client = newClient(theCommandLine); if (theCommandLine.hasOption(VERBOSE_LOGGING_PARAM)) { @@ -237,7 +245,6 @@ public class UploadTerminologyCommand extends BaseRequestGeneratingCommand { if (bytes.length > ourTransferSizeLimit) { ourLog.info("File size is greater than {} - Going to use a local file reference instead of a direct HTTP transfer. Note that this will only work when executing this command on the same server as the FHIR server itself.", FileUtil.formatFileSize(ourTransferSizeLimit)); - String suffix = fileName.substring(fileName.lastIndexOf(".")); try { File tempFile = File.createTempFile("hapi-fhir-cli", suffix); @@ -264,12 +271,22 @@ public class UploadTerminologyCommand extends BaseRequestGeneratingCommand { SNAPSHOT, ADD, REMOVE } - @VisibleForTesting - static void setTransferSizeLimitForUnitTest(long theTransferSizeLimit) { - if (theTransferSizeLimit <= 0) { + public void setTransferSizeBytes(long theTransferSizeBytes) { + if (ourTransferSizeLimit < 0) { ourTransferSizeLimit = DEFAULT_TRANSFER_SIZE_LIMIT; - }else { - ourTransferSizeLimit = theTransferSizeLimit; + } else { + ourTransferSizeLimit = theTransferSizeBytes; + } + } + public void setTransferSizeLimitHuman(String sizeString) { + if (isBlank(sizeString)) { + setTransferSizeBytes(DEFAULT_TRANSFER_SIZE_LIMIT); + } else { + long bytes = DataSize.parse(sizeString).toBytes(); + if (bytes < 0) { + bytes = DEFAULT_TRANSFER_SIZE_LIMIT; + } + setTransferSizeBytes(bytes); } } diff --git a/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/UploadTerminologyCommandTest.java b/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/UploadTerminologyCommandTest.java index ef1250d8e8a..85e4a28b604 100644 --- a/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/UploadTerminologyCommandTest.java +++ b/hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/UploadTerminologyCommandTest.java @@ -10,11 +10,13 @@ import ca.uhn.fhir.test.utilities.BaseRestServerHelper; import ca.uhn.fhir.test.utilities.RestServerDstu3Helper; import ca.uhn.fhir.test.utilities.RestServerR4Helper; import com.google.common.base.Charsets; +import org.apache.commons.cli.ParseException; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.RegisterExtension; @@ -39,8 +41,10 @@ import java.util.zip.ZipOutputStream; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.matchesPattern; +import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.ArgumentMatchers.any; @@ -124,7 +128,6 @@ public class UploadTerminologyCommandTest { FileUtils.deleteQuietly(myTextFile); FileUtils.deleteQuietly(myArchiveFile); FileUtils.deleteQuietly(myPropertiesFile); - UploadTerminologyCommand.setTransferSizeLimitForUnitTest(-1); } @ParameterizedTest @@ -260,6 +263,23 @@ public class UploadTerminologyCommandTest { } } + @Test + public void testModifyingSizeLimitConvertsCorrectlyR4() throws ParseException { + + UploadTerminologyCommand uploadTerminologyCommand = new UploadTerminologyCommand(); + uploadTerminologyCommand.setTransferSizeLimitHuman("1GB"); + long bytes = uploadTerminologyCommand.getTransferSizeLimit(); + assertThat(bytes, is(equalTo(1024L * 1024L * 1024L))); + + uploadTerminologyCommand.setTransferSizeLimitHuman("500KB"); + bytes = uploadTerminologyCommand.getTransferSizeLimit(); + assertThat(bytes, is(equalTo(1024L * 500L))); + + uploadTerminologyCommand.setTransferSizeLimitHuman("10MB"); + bytes = uploadTerminologyCommand.getTransferSizeLimit(); + assertThat(bytes, is(equalTo(1024L * 1024L * 10L))); + } + @ParameterizedTest @MethodSource("paramsProvider") public void testDeltaAddUsingCompressedFile(String theFhirVersion, boolean theIncludeTls) throws IOException { @@ -360,6 +380,7 @@ public class UploadTerminologyCommandTest { }, "-t", theIncludeTls, myBaseRestServerHelper )); + UploadTerminologyCommand uploadTerminologyCommand = new UploadTerminologyCommand(); verify(myTermLoaderSvc, times(1)).loadCustom(any(), myDescriptorListCaptor.capture(), any()); @@ -406,7 +427,6 @@ public class UploadTerminologyCommandTest { @ParameterizedTest @MethodSource("paramsProvider") public void testSnapshotLargeFile(String theFhirVersion, boolean theIncludeTls) throws IOException { - UploadTerminologyCommand.setTransferSizeLimitForUnitTest(10); if (FHIR_VERSION_DSTU3.equals(theFhirVersion)) { when(myTermLoaderSvc.loadCustom(any(), anyList(), any())).thenReturn(new UploadStatistics(100, new org.hl7.fhir.dstu3.model.IdType("CodeSystem/101"))); @@ -423,7 +443,8 @@ public class UploadTerminologyCommandTest { "-m", "SNAPSHOT", "-u", "http://foo", "-d", myConceptsFileName, - "-d", myHierarchyFileName + "-d", myHierarchyFileName, + "-s", "10MB" }, "-t", theIncludeTls, myBaseRestServerHelper )); diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3276-modifiable-maximum-transmit-size.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3276-modifiable-maximum-transmit-size.yaml new file mode 100644 index 00000000000..900d9672d33 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3276-modifiable-maximum-transmit-size.yaml @@ -0,0 +1,6 @@ +--- +type: add +issue: 3276 +title: "Added a new optional parameter to the `upload-terminology` operation of the HAPI-FHIR CLI. you can pass the `-s` or `--size` parameter to specify +the maximum size that will be transmitted to the server, before a local file reference is used. This parameter can be filled in using human-readable format, for example: + `upload-terminology -s \"1GB\"` will permit zip files up to 1 gigabyte, and anything larger than that would default to using a local file reference."