Oracle: Ensure migrated database still takes large resource text updates

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5633-oracle-hfj-res-ver-clob-migration.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 5633
+title: "Previously, migrations from 2023-02 or earlier would fail to update HFJ_RES_VER.RES_TEXT_VC to CLOB.
+        This has been fixed."

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/HibernatePropertiesProvider.java

@@ -75,4 +75,8 @@ public String getHibernateSearchBackend() {
 	public DataSource getDataSource() {
 		return myEntityManagerFactory.getDataSource();
 	}
+
+	public boolean isOracleDialect() {
+		return getDialect() instanceof org.hibernate.dialect.OracleDialect;
+	}
 }

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/JpaConfig.java

@@ -51,6 +51,7 @@
 import ca.uhn.fhir.jpa.dao.ISearchBuilder;
 import ca.uhn.fhir.jpa.dao.JpaStorageResourceParser;
 import ca.uhn.fhir.jpa.dao.MatchResourceUrlService;
+import ca.uhn.fhir.jpa.dao.ResourceHistoryCalculator;
 import ca.uhn.fhir.jpa.dao.SearchBuilderFactory;
 import ca.uhn.fhir.jpa.dao.TransactionProcessor;
 import ca.uhn.fhir.jpa.dao.data.IResourceModifiedDao;
@@ -869,4 +870,10 @@ public IResourceModifiedMessagePersistenceSvc subscriptionMessagePersistence(
 	public IMetaTagSorter metaTagSorter() {
 		return new MetaTagSorterAlphabetical();
 	}
+
+	@Bean
+	public ResourceHistoryCalculator resourceHistoryCalculator(
+			FhirContext theFhirContext, HibernatePropertiesProvider theHibernatePropertiesProvider) {
+		return new ResourceHistoryCalculator(theFhirContext, theHibernatePropertiesProvider.isOracleDialect());
+	}
 }

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java

@@ -85,7 +85,6 @@
 import ca.uhn.fhir.model.base.composite.BaseCodingDt;
 import ca.uhn.fhir.model.primitive.IdDt;
 import ca.uhn.fhir.parser.DataFormatException;
-import ca.uhn.fhir.parser.IParser;
 import ca.uhn.fhir.rest.api.Constants;
 import ca.uhn.fhir.rest.api.InterceptorInvocationTimingEnum;
 import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
@@ -105,8 +104,6 @@
 import com.google.common.base.Charsets;
 import com.google.common.collect.Sets;
 import com.google.common.hash.HashCode;
-import com.google.common.hash.HashFunction;
-import com.google.common.hash.Hashing;
 import jakarta.annotation.Nonnull;
 import jakarta.annotation.Nullable;
 import jakarta.annotation.PostConstruct;
@@ -264,6 +261,9 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> extends BaseStora
 	@Autowired
 	private PlatformTransactionManager myTransactionManager;
 
+	@Autowired
+	protected ResourceHistoryCalculator myResourceHistoryCalculator;
+
 	protected final CodingSpy myCodingSpy = new CodingSpy();
 
 	@VisibleForTesting
@@ -277,6 +277,11 @@ public void setSearchParamPresenceSvc(ISearchParamPresenceSvc theSearchParamPres
 		mySearchParamPresenceSvc = theSearchParamPresenceSvc;
 	}
 
+	@VisibleForTesting
+	public void setResourceHistoryCalculator(ResourceHistoryCalculator theResourceHistoryCalculator) {
+		myResourceHistoryCalculator = theResourceHistoryCalculator;
+	}
+
 	@Override
 	protected IInterceptorBroadcaster getInterceptorBroadcaster() {
 		return myInterceptorBroadcaster;
@@ -643,6 +648,7 @@ protected EncodedResource populateResourceIntoEntity(
 			theEntity.setResourceType(toResourceName(theResource));
 		}
 
+		byte[] resourceBinary;
 		String resourceText;
 		ResourceEncodingEnum encoding;
 		boolean changed = false;
@@ -659,6 +665,7 @@ protected EncodedResource populateResourceIntoEntity(
 				if (address != null) {
 
 					encoding = ResourceEncodingEnum.ESR;
+					resourceBinary = null;
 					resourceText = address.getProviderId() + ":" + address.getLocation();
 					changed = true;
 
@@ -675,10 +682,13 @@ protected EncodedResource populateResourceIntoEntity(
 
 					theEntity.setFhirVersion(myContext.getVersion().getVersion());
 
-					HashFunction sha256 = Hashing.sha256();
-					resourceText = encodeResource(theResource, encoding, excludeElements, myContext);
-					encoding = ResourceEncodingEnum.JSON;
-					HashCode hashCode = sha256.hashUnencodedChars(resourceText);
+					final ResourceHistoryState calculate = myResourceHistoryCalculator.calculateResourceHistoryState(
+							theResource, encoding, excludeElements);
+
+					resourceText = calculate.getResourceText();
+					resourceBinary = calculate.getResourceBinary();
+					encoding = calculate.getEncoding(); // This may be a no-op
+					final HashCode hashCode = calculate.getHashCode();
 
 					String hashSha256 = hashCode.toString();
 					if (!hashSha256.equals(theEntity.getHashSha256())) {
@@ -696,6 +706,7 @@ protected EncodedResource populateResourceIntoEntity(
 			} else {
 
 				encoding = null;
+				resourceBinary = null;
 				resourceText = null;
 			}
 
@@ -713,6 +724,7 @@ protected EncodedResource populateResourceIntoEntity(
 				changed = true;
 			}
 
+			resourceBinary = null;
 			resourceText = null;
 			encoding = ResourceEncodingEnum.DEL;
 		}
@@ -737,13 +749,15 @@ protected EncodedResource populateResourceIntoEntity(
 				if (currentHistoryVersion == null || !currentHistoryVersion.hasResource()) {
 					changed = true;
 				} else {
-					changed = !StringUtils.equals(currentHistoryVersion.getResourceTextVc(), resourceText);
+					changed = myResourceHistoryCalculator.isResourceHistoryChanged(
+							currentHistoryVersion, resourceBinary, resourceText);
 				}
 			}
 		}
 
 		EncodedResource retVal = new EncodedResource();
 		retVal.setEncoding(encoding);
+		retVal.setResourceBinary(resourceBinary);
 		retVal.setResourceText(resourceText);
 		retVal.setChanged(changed);
 
@@ -1393,8 +1407,11 @@ public IBasePersistedResource updateHistoryEntity(
 			ResourceEncodingEnum encoding = myStorageSettings.getResourceEncoding();
 			List<String> excludeElements = new ArrayList<>(8);
 			getExcludedElements(historyEntity.getResourceType(), excludeElements, theResource.getMeta());
-			String encodedResourceString = encodeResource(theResource, encoding, excludeElements, myContext);
-			boolean changed = !StringUtils.equals(historyEntity.getResourceTextVc(), encodedResourceString);
+			String encodedResourceString =
+					myResourceHistoryCalculator.encodeResource(theResource, encoding, excludeElements);
+			byte[] resourceBinary = ResourceHistoryCalculator.getResourceBinary(encoding, encodedResourceString);
+			final boolean changed = myResourceHistoryCalculator.isResourceHistoryChanged(
+					historyEntity, resourceBinary, encodedResourceString);
 
 			historyEntity.setUpdated(theTransactionDetails.getTransactionDate());
 
@@ -1406,14 +1423,15 @@ public IBasePersistedResource updateHistoryEntity(
 				return historyEntity;
 			}
 
-			populateEncodedResource(encodedResource, encodedResourceString, ResourceEncodingEnum.JSON);
+			myResourceHistoryCalculator.populateEncodedResource(
+					encodedResource, encodedResourceString, resourceBinary, encoding);
 		}
-
 		/*
 		 * Save the resource itself to the resourceHistoryTable
 		 */
 		historyEntity = myEntityManager.merge(historyEntity);
 		historyEntity.setEncoding(encodedResource.getEncoding());
+		historyEntity.setResource(encodedResource.getResourceBinary());
 		historyEntity.setResourceTextVc(encodedResource.getResourceText());
 		myResourceHistoryTableDao.save(historyEntity);
 
@@ -1423,8 +1441,12 @@ public IBasePersistedResource updateHistoryEntity(
 	}
 
 	private void populateEncodedResource(
-			EncodedResource encodedResource, String encodedResourceString, ResourceEncodingEnum theEncoding) {
+			EncodedResource encodedResource,
+			String encodedResourceString,
+			byte[] theResourceBinary,
+			ResourceEncodingEnum theEncoding) {
 		encodedResource.setResourceText(encodedResourceString);
+		encodedResource.setResourceBinary(theResourceBinary);
 		encodedResource.setEncoding(theEncoding);
 	}
 
@@ -1489,6 +1511,7 @@ private void createHistoryEntry(
 		}
 
 		historyEntry.setEncoding(theChanged.getEncoding());
+		historyEntry.setResource(theChanged.getResourceBinary());
 		historyEntry.setResourceTextVc(theChanged.getResourceText());
 
 		ourLog.debug("Saving history entry ID[{}] for RES_ID[{}]", historyEntry.getId(), historyEntry.getResourceId());
@@ -1926,16 +1949,6 @@ public static String decodeResource(byte[] theResourceBytes, ResourceEncodingEnu
 		return resourceText;
 	}
 
-	public static String encodeResource(
-			IBaseResource theResource,
-			ResourceEncodingEnum theEncoding,
-			List<String> theExcludeElements,
-			FhirContext theContext) {
-		IParser parser = theEncoding.newParser(theContext);
-		parser.setDontEncodeElements(theExcludeElements);
-		return parser.encodeResourceToString(theResource);
-	}
-
 	private static String parseNarrativeTextIntoWords(IBaseResource theResource) {
 
 		StringBuilder b = new StringBuilder();

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java

@@ -1709,17 +1709,11 @@ private void reindexOptimizeStorageHistoryEntity(ResourceTable entity, ResourceH
 		if (historyEntity.getEncoding() == ResourceEncodingEnum.JSONC
 				|| historyEntity.getEncoding() == ResourceEncodingEnum.JSON) {
 			byte[] resourceBytes = historyEntity.getResource();
-
-			// Always migrate data out of the bytes column
 			if (resourceBytes != null) {
 				String resourceText = decodeResource(resourceBytes, historyEntity.getEncoding());
-				ourLog.debug(
-						"Storing text of resource {} version {} as inline VARCHAR",
-						entity.getResourceId(),
-						historyEntity.getVersion());
-				historyEntity.setResourceTextVc(resourceText);
-				historyEntity.setEncoding(ResourceEncodingEnum.JSON);
-				changed = true;
+				if (myResourceHistoryCalculator.conditionallyAlterHistoryEntity(entity, historyEntity, resourceText)) {
+					changed = true;
+				}
 			}
 		}
 		if (isBlank(historyEntity.getSourceUri()) && isBlank(historyEntity.getRequestId())) {

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/EncodedResource.java

@@ -24,6 +24,7 @@
 class EncodedResource {
 
 	private boolean myChanged;
+	private byte[] myResource;
 	private ResourceEncodingEnum myEncoding;
 	private String myResourceText;
 
@@ -35,6 +36,14 @@ public void setEncoding(ResourceEncodingEnum theEncoding) {
 		myEncoding = theEncoding;
 	}
 
+	public byte[] getResourceBinary() {
+		return myResource;
+	}
+
+	public void setResourceBinary(byte[] theResource) {
+		myResource = theResource;
+	}
+
 	public boolean isChanged() {
 		return myChanged;
 	}

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/ResourceHistoryCalculator.java

@@ -0,0 +1,134 @@
+package ca.uhn.fhir.jpa.dao;
+
+import ca.uhn.fhir.context.FhirContext;
+import ca.uhn.fhir.jpa.model.entity.ResourceEncodingEnum;
+import ca.uhn.fhir.jpa.model.entity.ResourceHistoryTable;
+import ca.uhn.fhir.jpa.model.entity.ResourceTable;
+import ca.uhn.fhir.parser.IParser;
+import com.google.common.hash.HashCode;
+import com.google.common.hash.HashFunction;
+import com.google.common.hash.Hashing;
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import org.apache.commons.lang3.StringUtils;
+import org.hl7.fhir.instance.model.api.IBaseResource;
+
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.List;
+
+/**
+ * Responsible for various resource history-centric and {@link FhirContext} aware operations called by
+ * {@link BaseHapiFhirDao} or {@link BaseHapiFhirResourceDao} that require knowledge of whether an Oracle database is
+ * being used.
+ */
+public class ResourceHistoryCalculator {
+	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ResourceHistoryCalculator.class);
+	private static final HashFunction SHA_256 = Hashing.sha256();
+
+	private final FhirContext myFhirContext;
+	private final boolean myIsOracleDialect;
+
+	public ResourceHistoryCalculator(FhirContext theFhirContext, boolean theIsOracleDialect) {
+		myFhirContext = theFhirContext;
+		myIsOracleDialect = theIsOracleDialect;
+	}
+
+	ResourceHistoryState calculateResourceHistoryState(
+			IBaseResource theResource, ResourceEncodingEnum theEncoding, List<String> theExcludeElements) {
+		final String encodedResource = encodeResource(theResource, theEncoding, theExcludeElements);
+		final byte[] resourceBinary;
+		final String resourceText;
+		final ResourceEncodingEnum encoding;
+		final HashCode hashCode;
+
+		if (myIsOracleDialect) {
+			resourceText = null;
+			resourceBinary = getResourceBinary(theEncoding, encodedResource);
+			encoding = theEncoding;
+			hashCode = SHA_256.hashBytes(resourceBinary);
+		} else {
+			resourceText = encodedResource;
+			resourceBinary = null;
+			encoding = ResourceEncodingEnum.JSON;
+			hashCode = SHA_256.hashUnencodedChars(encodedResource);
+		}
+
+		return new ResourceHistoryState(resourceText, resourceBinary, encoding, hashCode);
+	}
+
+	boolean conditionallyAlterHistoryEntity(
+			ResourceTable theEntity, ResourceHistoryTable theHistoryEntity, String theResourceText) {
+		if (!myIsOracleDialect) {
+			ourLog.debug(
+					"Storing text of resource {} version {} as inline VARCHAR",
+					theEntity.getResourceId(),
+					theHistoryEntity.getVersion());
+			theHistoryEntity.setResourceTextVc(theResourceText);
+			theHistoryEntity.setResource(null);
+			theHistoryEntity.setEncoding(ResourceEncodingEnum.JSON);
+			return true;
+		}
+
+		return false;
+	}
+
+	boolean isResourceHistoryChanged(
+			ResourceHistoryTable theCurrentHistoryVersion,
+			@Nullable byte[] theResourceBinary,
+			@Nullable String resourceText) {
+		if (myIsOracleDialect) {
+			return !Arrays.equals(theCurrentHistoryVersion.getResource(), theResourceBinary);
+		}
+
+		return !StringUtils.equals(theCurrentHistoryVersion.getResourceTextVc(), resourceText);
+	}
+
+	String encodeResource(
+			IBaseResource theResource, ResourceEncodingEnum theEncoding, List<String> theExcludeElements) {
+		final IParser parser = theEncoding.newParser(myFhirContext);
+		parser.setDontEncodeElements(theExcludeElements);
+		return parser.encodeResourceToString(theResource);
+	}
+
+	/**
+	 * helper for returning the encoded byte array of the input resource string based on the theEncoding.
+	 *
+	 * @param theEncoding        the theEncoding to used
+	 * @param theEncodedResource the resource to encode
+	 * @return byte array of the resource
+	 */
+	@Nonnull
+	static byte[] getResourceBinary(ResourceEncodingEnum theEncoding, String theEncodedResource) {
+		switch (theEncoding) {
+			case JSON:
+				return theEncodedResource.getBytes(StandardCharsets.UTF_8);
+			case JSONC:
+				return GZipUtil.compress(theEncodedResource);
+			default:
+				return new byte[0];
+		}
+	}
+
+	void populateEncodedResource(
+			EncodedResource theEncodedResource,
+			String theEncodedResourceString,
+			@Nullable byte[] theResourceBinary,
+			ResourceEncodingEnum theEncoding) {
+		if (myIsOracleDialect) {
+			populateEncodedResourceInner(theEncodedResource, null, theResourceBinary, theEncoding);
+		} else {
+			populateEncodedResourceInner(theEncodedResource, theEncodedResourceString, null, ResourceEncodingEnum.JSON);
+		}
+	}
+
+	private void populateEncodedResourceInner(
+			EncodedResource encodedResource,
+			String encodedResourceString,
+			byte[] theResourceBinary,
+			ResourceEncodingEnum theEncoding) {
+		encodedResource.setResourceText(encodedResourceString);
+		encodedResource.setResourceBinary(theResourceBinary);
+		encodedResource.setEncoding(theEncoding);
+	}
+}