Allow transaction with conditional create and patch in same bundle (#4735)

* Allow transaction with conditional create and patch in same bundle

* Add changelog

* Cleanup

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_6_0/4735-allow-conditional-create-and-patch-on-same-resource-in-transaction.yaml

@@ -0,0 +1,5 @@
+---
+type: add
+issue: 4735
+title: "It is now possible to perform a conditional create and a conditional patch on the
+  same resource (i.e. the same conditional URL) within a FHIR Transaction bundle."

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4QueryCountTest.java

@@ -50,6 +50,9 @@ import org.hl7.fhir.r4.model.Reference;
 import org.hl7.fhir.r4.model.ServiceRequest;
 import org.hl7.fhir.r4.model.StringType;
 import org.hl7.fhir.r4.model.ValueSet;
+import org.hl7.fhir.r4.model.BooleanType;
+import org.hl7.fhir.r4.model.CodeType;
+import org.hl7.fhir.r4.model.Parameters;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.MethodOrderer;
@@ -2765,8 +2768,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test
 	 * as well as a large number of updates (PUT). This means that a lot of URLs and resources
 	 * need to be resolved (ie SQL SELECT) in order to proceed with the transaction. Prior
 	 * to the optimization that introduced this test, we had 140 SELECTs, now it's 17.
-	 */
+	 *
-	/**
 	 * See the class javadoc before changing the counts in this test!
 	 */
 	@Test
@@ -2795,6 +2797,48 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test
 	}
 
 
+	/**
+	 * See the class javadoc before changing the counts in this test!
+	 */
+	@Test
+	public void testTransactionWithConditionalCreateAndConditionalPatchOnSameUrl() {
+		// Setup
+		BundleBuilder bb = new BundleBuilder(myFhirContext);
+		Patient patient = new Patient();
+		patient.setActive(false);
+		patient.addIdentifier().setSystem("http://system").setValue("value");
+		bb.addTransactionCreateEntry(patient).conditional("Patient?identifier=http://system|value");
+
+		Parameters patch = new Parameters();
+		Parameters.ParametersParameterComponent op = patch.addParameter().setName("operation");
+		op.addPart().setName("type").setValue(new CodeType("replace"));
+		op.addPart().setName("path").setValue(new CodeType("Patient.active"));
+		op.addPart().setName("value").setValue(new BooleanType(true));
+		bb.addTransactionFhirPatchEntry(patch).conditional("Patient?identifier=http://system|value");
+
+		Bundle input = bb.getBundleTyped();
+
+		// Test
+		myCaptureQueriesListener.clear();
+		Bundle output = mySystemDao.transaction(mySrd, input);
+
+		// Verify
+		assertEquals(3, myCaptureQueriesListener.countSelectQueriesForCurrentThread());
+		assertEquals(6, myCaptureQueriesListener.countInsertQueriesForCurrentThread());
+		assertEquals(1, myCaptureQueriesListener.countUpdateQueriesForCurrentThread());
+		assertEquals(0, myCaptureQueriesListener.countDeleteQueriesForCurrentThread());
+		assertEquals(1, myCaptureQueriesListener.countCommits());
+		assertEquals(0, myCaptureQueriesListener.countRollbacks());
+
+		assertEquals(input.getEntry().size(), output.getEntry().size());
+
+		runInTransaction(() -> {
+			assertEquals(1, myResourceTableDao.count());
+			assertEquals(1, myResourceHistoryTableDao.count());
+		});
+
+	}
+
 	/**
 	 * See the class javadoc before changing the counts in this test!
 	 */

hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/FhirSystemDaoTransactionR5Test.java

@@ -2,20 +2,25 @@ package ca.uhn.fhir.jpa.dao.r5;
 
 import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
 import ca.uhn.fhir.util.BundleBuilder;
+import org.hl7.fhir.r5.model.BooleanType;
 import org.hl7.fhir.r5.model.Bundle;
+import org.hl7.fhir.r5.model.CodeType;
 import org.hl7.fhir.r5.model.IdType;
 import org.hl7.fhir.r5.model.Observation;
+import org.hl7.fhir.r5.model.Parameters;
 import org.hl7.fhir.r5.model.Patient;
 import org.hl7.fhir.r5.model.Quantity;
 import org.hl7.fhir.r5.model.Reference;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import java.util.UUID;
 
 import static org.apache.commons.lang3.StringUtils.countMatches;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class FhirSystemDaoTransactionR5Test extends BaseJpaR5Test {
 
@@ -381,6 +386,56 @@ public class FhirSystemDaoTransactionR5Test extends BaseJpaR5Test {
 
 	}
 
+
+	/**
+	 * A FHIR transaction bundle containing a conditional create as well as a patch that point to the same resource
+	 */
+	@ParameterizedTest
+	@ValueSource(booleans = {true, false})
+	public void testConditionalCreateAndConditionalPatchOnSameResource(boolean thePreviouslyExisting) {
+
+		if (thePreviouslyExisting) {
+			Patient patient = new Patient();
+			patient.setActive(false);
+			patient.addIdentifier().setSystem("http://system").setValue("value");
+			myPatientDao.create(patient, mySrd);
+		}
+
+		BundleBuilder bb = new BundleBuilder(myFhirContext);
+		Patient patient = new Patient();
+		patient.setActive(false);
+		patient.addIdentifier().setSystem("http://system").setValue("value");
+		bb.addTransactionCreateEntry(patient).conditional("Patient?identifier=http://system|value");
+
+		Parameters patch = new Parameters();
+		Parameters.ParametersParameterComponent op = patch.addParameter().setName("operation");
+		op.addPart().setName("type").setValue(new CodeType("replace"));
+		op.addPart().setName("path").setValue(new CodeType("Patient.active"));
+		op.addPart().setName("value").setValue(new BooleanType(true));
+		bb.addTransactionFhirPatchEntry(patch).conditional("Patient?identifier=http://system|value");
+
+		Bundle input = bb.getBundleTyped();
+		ourLog.info("Bundle: {}", myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(input));
+
+		// Test
+		Bundle output = mySystemDao.transaction(mySrd, input);
+
+		// Verify
+		IdType createId = new IdType(output.getEntry().get(0).getResponse().getLocation());
+		IdType patchId = new IdType(output.getEntry().get(1).getResponse().getLocation());
+		assertEquals("1", createId.getVersionIdPart());
+		assertEquals("2", patchId.getVersionIdPart());
+		assertEquals(createId.getIdPart(), patchId.getIdPart());
+
+		Patient createdPatient = myPatientDao.read(patchId, mySrd);
+		assertEquals("http://system", createdPatient.getIdentifierFirstRep().getSystem());
+		assertTrue(createdPatient.getActive());
+
+		assertEquals(2, output.getEntry().size());
+	}
+
+
+
 }
 
 

hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageResourceDao.java

@@ -115,6 +115,12 @@ public abstract class BaseStorageResourceDao<T extends IBaseResource> extends Ba
 		}
 
 		IBaseResource resourceToUpdate = getStorageResourceParser().toResource(entityToUpdate, false);
+		if (resourceToUpdate == null) {
+			// If this is null, we are presumably in a FHIR transaction bundle with both a create and a patch on the same
+			// resource. This is weird but not impossible.
+			resourceToUpdate = theTransactionDetails.getResolvedResource(resourceId);
+		}
+
 		IBaseResource destination;
 		switch (thePatchType) {
 			case JSON_PATCH: