Merge branch 'filesystem-binary-regression' into rel_7_2

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_2_2/version.yaml

@@ -1,3 +1,3 @@
 ---
-release-date: "2024-07-15"
+release-date: "2024-07-19"
 codename: "Borealis"

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_4_0/6134-filesystem-regression.yaml

@@ -0,0 +1,6 @@
+---
+type: fix
+issue: 6134
+backport: 7.2.2
+title: "Fixed a regression in 7.2.0 which caused systems using `FILESYSTEM` binary storage mode to be unable to read metadata documents
+that had been previously stored on disk."

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/binstore/FilesystemBinaryStorageSvcImplTest.java

@@ -7,6 +7,9 @@ import ca.uhn.fhir.jpa.binary.api.StoredDetails;
 import ca.uhn.fhir.rest.server.exceptions.PayloadTooLargeException;
 import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
 import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.SerializationFeature;
 import org.apache.commons.io.FileUtils;
 import org.hl7.fhir.instance.model.api.IIdType;
 import org.hl7.fhir.r4.model.IdType;
@@ -24,6 +27,7 @@ import java.io.IOException;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
 public class FilesystemBinaryStorageSvcImplTest {
@@ -46,6 +50,30 @@ public class FilesystemBinaryStorageSvcImplTest {
 		FileUtils.deleteDirectory(myPath);
 	}
 
+	/**
+	 * See https://github.com/hapifhir/hapi-fhir/pull/6134
+	 */
+	@Test
+	public void testStoreAndRetrievePostMigration() throws IOException {
+		String blobId = "some-blob-id";
+		String oldDescriptor = "{\n" +
+			"  \"blobId\" : \"" + blobId + "\",\n" +
+			"  \"bytes\" : 80926,\n" +
+			"  \"contentType\" : \"application/fhir+json\",\n" +
+			"  \"hash\" : \"f57596cefbee4c48c8493a2a57ef5f70c52a2c5afa0e48f57cfbf4f219eb0a38\",\n" +
+			"  \"published\" : \"2024-07-20T00:12:28.187+05:30\"\n" +
+			"}";
+
+		ObjectMapper myJsonSerializer;
+		myJsonSerializer = new ObjectMapper();
+		myJsonSerializer.setSerializationInclusion(JsonInclude.Include.NON_NULL);
+		myJsonSerializer.enable(SerializationFeature.INDENT_OUTPUT);
+
+		StoredDetails storedDetails = myJsonSerializer.readValue(oldDescriptor, StoredDetails.class);
+		assertTrue(storedDetails.getBinaryContentId().equals(blobId));;
+
+	}
+
 	@Test
 	public void testStoreAndRetrieve() throws IOException {
 		IIdType id = new IdType("Patient/123");

hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/binary/api/StoredDetails.java

@@ -33,9 +33,22 @@ import java.util.Date;
 
 public class StoredDetails implements IModelJson {
 
-	@JsonProperty("binaryContentId")
+	@JsonProperty(value = "binaryContentId")
 	private String myBinaryContentId;
 
+	/**
+	 * This field exists to fix a break that changing this property name caused.
+	 * in 7.2.0 we went from blobId to binaryContentId. However this did not consider installations using filesystem
+	 * mode storage in which the data on disk was not updated, and needed to be Serialized/Deserialized at runtime.
+	 * Existing stored details used `blobId`. This causes Jackson deserialization failures which are tough to recover
+	 * from without manually modifying all those stored details
+	 * on disk.
+	 * This field is a relic to support old blobs post-upgrade to 7.2.0. It is not ever surfaced to the user, and is proxied
+	 * into `myBinaryContentId` when needed.
+	 */
+	@JsonProperty(value = "blobId")
+	private String myBlobId;
+
 	@JsonProperty("bytes")
 	private long myBytes;
 
@@ -77,7 +90,7 @@ public class StoredDetails implements IModelJson {
 	@Override
 	public String toString() {
 		return new ToStringBuilder(this)
-				.append("binaryContentId", myBinaryContentId)
+				.append("binaryContentId", getBinaryContentId())
 				.append("bytes", myBytes)
 				.append("contentType", myContentType)
 				.append("hash", myHash)
@@ -115,7 +128,11 @@ public class StoredDetails implements IModelJson {
 
 	@Nonnull
 	public String getBinaryContentId() {
-		return myBinaryContentId;
+		if (myBinaryContentId == null && myBlobId != null) {
+			return myBlobId;
+		} else {
+			return myBinaryContentId;
+		}
 	}
 
 	public StoredDetails setBinaryContentId(String theBinaryContentId) {

hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/binstore/FilesystemBinaryStorageSvcImpl.java

@@ -109,7 +109,8 @@ public class FilesystemBinaryStorageSvcImpl extends BaseBinaryStorageSvcImpl {
 
 		// Write descriptor file
 		long count = countingInputStream.getByteCount();
-		StoredDetails details = new StoredDetails(id, count, theContentType, hashingInputStream, new Date());
+		StoredDetails details = null;
+		details = new StoredDetails(id, count, theContentType, hashingInputStream, new Date());
 		File descriptorFilename = getDescriptorFilename(storagePath, theResourceId, id);
 		ourLog.info("Writing to file: {}", descriptorFilename.getAbsolutePath());
 		try (FileWriter writer = new FileWriter(descriptorFilename)) {
@@ -223,7 +224,7 @@ public class FilesystemBinaryStorageSvcImpl extends BaseBinaryStorageSvcImpl {
 		return new File(theStoragePath, filename + theExtension);
 	}
 
-	private File getStoragePath(String theId, boolean theCreate) {
+	File getStoragePath(String theId, boolean theCreate) {
 		File path = myBasePath;
 		for (int i = 0; i < 10; i++) {
 			path = new File(path, theId.substring(i, i + 1));