Excessive Hibernate warnings when processing conditional create and d…

…elete operations on referenced resources in a transaction bundle (hapifhir#6477) * Excessive Hibernate warnings when processing conditional create and delete operations on referenced resources in a transaction bundle - failing test * Excessive Hibernate warnings when processing conditional create and delete operations on referenced resources in a transaction bundle - fix * Excessive Hibernate warnings when processing conditional create and delete operations on referenced resources in a transaction bundle - changelog
trifork · Nov 20, 2024 · e182694 · e182694
1 parent cbbba75
commit e182694
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 17 deletions.
diff --git a/...ain/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6475-fix-excessive-hibernate-warnings.yaml b/...ain/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6475-fix-excessive-hibernate-warnings.yaml
@@ -0,0 +1,7 @@
+---
+type: fix
+issue: 6475
+jira: SMILE-8490
+title: "Previously, submitting a transaction bundle containing a conditional delete, a conditional create, and a 
+resource which relied on this conditional create as a reference would lead to excessive Hibernate warnings in the logs. 
+This has been fixed."
diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceLink.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceLink.java
@@ -31,6 +31,7 @@
 import jakarta.persistence.Index;
 import jakarta.persistence.JoinColumn;
 import jakarta.persistence.ManyToOne;
+import jakarta.persistence.PostLoad;
 import jakarta.persistence.Table;
 import jakarta.persistence.Temporal;
 import jakarta.persistence.TemporalType;
@@ -97,6 +98,9 @@ public class ResourceLink extends BaseResourceIndex {
 			foreignKey = @ForeignKey(name = "FK_RESLINK_TARGET"))
 	private ResourceTable myTargetResource;
 
+	@Transient
+	private ResourceTable myTransientTargetResource;
+
 	@Column(name = "TARGET_RESOURCE_ID", insertable = true, updatable = true, nullable = true)
 	@FullTextField
 	private Long myTargetResourcePid;
@@ -141,7 +145,7 @@ public void setTargetResourceVersion(Long theTargetResourceVersion) {
 	}
 
 	public String getTargetResourceId() {
-		if (myTargetResourceId == null && myTargetResource != null) {
+		if (myTargetResourceId == null && getTargetResource() != null) {
 			myTargetResourceId = getTargetResource().getIdDt().getIdPart();
 		}
 		return myTargetResourceId;
@@ -184,11 +188,21 @@ public boolean equals(Object theObj) {
 		return b.isEquals();
 	}
 
+	/**
+	 * ResourceLink.myTargetResource field is immutable.Transient ResourceLink.myTransientTargetResource property
+	 * is used instead, allowing it to be updated via the ResourceLink#copyMutableValuesFrom method
+	 * when ResourceLink table row is reused.
+	 */
+	@PostLoad
+	public void postLoad() {
+		myTransientTargetResource = myTargetResource;
+	}
+
 	@Override
 	public <T extends BaseResourceIndex> void copyMutableValuesFrom(T theSource) {
 		ResourceLink source = (ResourceLink) theSource;
 		mySourcePath = source.getSourcePath();
-		myTargetResource = source.getTargetResource();
+		myTransientTargetResource = source.getTargetResource();
 		myTargetResourceId = source.getTargetResourceId();
 		myTargetResourcePid = source.getTargetResourcePid();
 		myTargetResourceType = source.getTargetResourceType();
@@ -331,7 +345,7 @@ public String toString() {
 	}
 
 	public ResourceTable getTargetResource() {
-		return myTargetResource;
+		return myTransientTargetResource;
 	}
 
 	/**
@@ -348,8 +362,8 @@ public ResourceLink cloneWithoutTargetPid() {
 		retVal.myTargetResourceType = myTargetResourceType;
 		if (myTargetResourceId != null) {
 			retVal.myTargetResourceId = myTargetResourceId;
-		} else if (myTargetResource != null) {
-			retVal.myTargetResourceId = myTargetResource.getIdDt().getIdPart();
+		} else if (getTargetResource() != null) {
+			retVal.myTargetResourceId = getTargetResource().getIdDt().getIdPart();
 		}
 		retVal.myTargetResourceUrl = myTargetResourceUrl;
 		retVal.myTargetResourceVersion = myTargetResourceVersion;

diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirSystemDaoR4Test.java
@@ -39,7 +39,11 @@
 import ca.uhn.fhir.rest.server.interceptor.auth.RuleBuilder;
 import ca.uhn.fhir.util.BundleBuilder;
 import ca.uhn.fhir.util.ClasspathUtil;
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.Appender;
 import org.apache.commons.io.IOUtils;
+import org.hibernate.persister.entity.AbstractEntityPersister;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.instance.model.api.IIdType;
 import org.hl7.fhir.r4.model.AllergyIntolerance;
@@ -85,6 +89,9 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.slf4j.LoggerFactory;
 import org.springframework.dao.DataIntegrityViolationException;
 import org.springframework.transaction.TransactionDefinition;
 import org.springframework.transaction.TransactionStatus;
@@ -113,13 +120,18 @@
 import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest {
 
 	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirSystemDaoR4Test.class);
 	private static final String TEST_IDENTIFIER_SYSTEM = "http://some-system.com";
 
+	@Mock
+	private Appender<ILoggingEvent> myAppender;
+
 	@AfterEach
 	public void after() {
 		JpaStorageSettings defaults = new JpaStorageSettings();
@@ -2956,7 +2968,7 @@ public void testTransactionOrdering() {
 		 * these four operations in the reverse order and verifies
 		 * that they are invoked correctly.
 		 */
-		//@formatter:off
+		//@formatter:on
 
 		int pass = 0;
 		IdType patientPlaceholderId = IdType.newRandomUuid();
@@ -3023,22 +3035,38 @@ private void testTransactionOrderingValidateResponse(int pass, Bundle resp) {
 
 	@Test
 	public void testTransactionOruBundle() throws IOException {
+		// setup
 		myStorageSettings.setAllowMultipleDelete(true);
+		Logger logger = (Logger) LoggerFactory.getLogger(AbstractEntityPersister.class);
+		logger.addAppender(myAppender);
+
+		try {
+			String input = IOUtils.toString(getClass().getResourceAsStream("/r4/oruBundle.json"), StandardCharsets.UTF_8);
+
+			Bundle inputBundle;
+			Bundle outputBundle;
 
-		String input = IOUtils.toString(getClass().getResourceAsStream("/r4/oruBundle.json"), StandardCharsets.UTF_8);
+			// execute transaction
+			inputBundle = myFhirContext.newJsonParser().parseResource(Bundle.class, input);
+			outputBundle = mySystemDao.transaction(mySrd, inputBundle);
+			ourLog.debug(myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(outputBundle));
 
-		Bundle inputBundle;
-		Bundle outputBundle;
-		inputBundle = myFhirContext.newJsonParser().parseResource(Bundle.class, input);
-		outputBundle = mySystemDao.transaction(mySrd, inputBundle);
-		ourLog.debug(myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(outputBundle));
+			// execute same transaction again
+			inputBundle = myFhirContext.newJsonParser().parseResource(Bundle.class, input);
+			outputBundle = mySystemDao.transaction(mySrd, inputBundle);
+			ourLog.debug(myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(outputBundle));
 
-		inputBundle = myFhirContext.newJsonParser().parseResource(Bundle.class, input);
-		outputBundle = mySystemDao.transaction(mySrd, inputBundle);
-		ourLog.debug(myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(outputBundle));
+			// validate
+			IBundleProvider allPatients = myPatientDao.search(new SearchParameterMap());
+			assertEquals(1, allPatients.size().intValue());
 
-		IBundleProvider allPatients = myPatientDao.search(new SearchParameterMap());
-		assertEquals(1, allPatients.size().intValue());
+			// validate that AbstractEntityPersister does not produce log messages
+			// see https://github.com/hapifhir/hapi-fhir/issues/6475 for details
+			ArgumentCaptor<ILoggingEvent> logCaptor = ArgumentCaptor.forClass(ILoggingEvent.class);
+			verify(myAppender, times(0)).doAppend(logCaptor.capture());
+		} finally {
+			logger.detachAppender(myAppender);
+		}
 	}
 
 	@Test