fix delete conflict on versioned patient ref on Provenance.target (#7158)

* fix delete conflict on allowed versioned patient ref on Provenance.target

* change to use regex instead of startsWith

* addressed code review comments

Author: mrdnctrk

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_4_0/7160-delete-patient-versioned-ref.yaml

@@ -0,0 +1,6 @@
+---
+type: fix
+issue: 7160
+title: "Previously, a Patient resource could not be deleted if a `Provenance.target` has a versioned reference to the 
+Patient. The delete was prevented by the referential integrity check on delete, even though the versioned references 
+are not supposed to prevent deletion. This has now been fixed."

hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/SearchParamExtractorService.java

@@ -613,12 +613,7 @@ private void extractResourceLinks(
 			nextId = nextReference.getResource().getIdElement();
 		}
 
-		if (myContext.getParserOptions().isStripVersionsFromReferences()
-				&& !myContext
-						.getParserOptions()
-						.getDontStripVersionsFromReferencesAtPaths()
-						.contains(thePathAndRef.getPath())
-				&& nextId.hasVersionIdPart()) {
+		if (nextId.hasVersionIdPart() && shouldStripVersionFromReferenceAtPath(thePathAndRef.getPath())) {
 			nextId = nextId.toVersionless();
 		}
 
@@ -1090,6 +1085,30 @@ public void extractSearchParamComboNonUnique(ResourceTable theEntity, ResourceIn
 		populateResourceTableForComboParams(theParams.myComboTokenNonUnique, theEntity);
 	}
 
+	private boolean shouldStripVersionFromReferenceAtPath(String theSearchParamPath) {
+
+		if (!myContext.getParserOptions().isStripVersionsFromReferences()) {
+			// all references allowed to have versions globally, so don't strip
+			return false;
+		}
+
+		// global setting is to strip versions, see if there's any exceptions configured for specific paths
+		Set<String> pathsAllowedToHaveVersionedRefs =
+				myContext.getParserOptions().getDontStripVersionsFromReferencesAtPaths();
+
+		if (pathsAllowedToHaveVersionedRefs.contains(theSearchParamPath)) {
+			// path exactly matches
+			return false;
+		}
+
+		// there are some search parameters using a where clause to index the element for a specific resource type, such
+		// as "Provenance.target.where(resolve() is Patient)". We insert these in the ResourceLink table as well.
+		// Such entries in the ResourceLink table should remain versioned if the element is allowed to be versioned.
+		return pathsAllowedToHaveVersionedRefs.stream()
+				.noneMatch(pathToKeepVersioned -> theSearchParamPath.matches(
+						pathToKeepVersioned + "\\.where\\(resolve\\(\\) is [A-Z][a-zA-Z]*\\)"));
+	}
+
 	/**
 	 * This interface is used by {@link #extractSearchIndexParametersForTargetResources(RequestDetails, ResourceIndexedSearchParams, ResourceTable, Collection, IChainedSearchParameterExtractionStrategy, ISearchParamExtractor.SearchParamSet, boolean, boolean)}
 	 * in order to use that method for extracting chained search parameter indexes both

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/delete/DeleteConflictServiceR4Test.java

@@ -1,5 +1,6 @@
 package ca.uhn.fhir.jpa.delete;
 
+import ca.uhn.fhir.context.ParserOptions;
 import ca.uhn.fhir.i18n.Msg;
 import ca.uhn.fhir.interceptor.api.Hook;
 import ca.uhn.fhir.interceptor.api.Pointcut;
@@ -19,6 +20,7 @@
 import org.hl7.fhir.r4.model.OperationOutcome;
 import org.hl7.fhir.r4.model.Organization;
 import org.hl7.fhir.r4.model.Patient;
+import org.hl7.fhir.r4.model.Provenance;
 import org.hl7.fhir.r4.model.Reference;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -34,6 +36,7 @@
 import java.util.function.Function;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -375,6 +378,49 @@ public void testNoDuplicateConstraintReferences() {
 		assertThat(conflicts).hasSize(1);
 	}
 
+
+	@Test
+	void testDeletePatientReferencedInProvenanceTarget_AllowVersionsInProvenanceTarget_ShouldDeleteWithoutConflict() {
+		//we don't expect this to be called, delete should occur without any conflict
+		myDeleteInterceptor.deleteConflictFunction = x -> null;
+
+		try {
+			myFhirContext.getParserOptions().setDontStripVersionsFromReferencesAtPaths("Provenance.target");
+			Patient pat = new Patient();
+			IIdType patId = myPatientDao.create(pat, mySrd).getId();
+
+			Provenance prov = new Provenance();
+			prov.addTarget(new Reference(patId.toUnqualified()));
+			myProvenanceDao.create(prov, mySrd);
+
+			myPatientDao.delete(patId.toUnqualifiedVersionless(), mySrd);
+
+			assertThat(myDeleteInterceptor.myCallCount).isZero();
+		} finally {
+			//revert the parser option config to its default
+			myFhirContext.getParserOptions().setDontStripVersionsFromReferencesAtPaths(new ParserOptions().getDontStripVersionsFromReferencesAtPaths());
+		}
+	}
+
+	@Test
+	void testDeletePatientReferencedInProvenanceTarget_DontAllowVersionsInProvenanceTarget_ShouldFailWithConflict() {
+		myDeleteInterceptor.deleteConflictFunction = x -> null;
+
+		Patient pat = new Patient();
+		IIdType patId = myPatientDao.create(pat, mySrd).getId();
+
+		Provenance prov = new Provenance();
+		prov.addTarget(new Reference(patId.toUnqualified()));
+		myProvenanceDao.create(prov, mySrd);
+
+		IIdType versionlessPatId = patId.toUnqualifiedVersionless();
+
+		assertThatThrownBy(() -> myPatientDao.delete(versionlessPatId, mySrd))
+			.isInstanceOf(ResourceVersionConflictException.class);
+
+		assertThat(myDeleteInterceptor.myCallCount).isOne();
+	}
+
 	private DeleteConflictOutcome deleteConflicts(DeleteConflictList theList) {
 		for (DeleteConflict next : theList) {
 			IdDt source = next.getSourceId();

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatientMergeR4Test.java

@@ -46,7 +46,9 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Set;
 
+import static ca.uhn.fhir.jpa.config.r4.FhirContextR4Config.DEFAULT_PRESERVE_VERSION_REFS_R4_AND_LATER;
 import static ca.uhn.fhir.jpa.provider.ReplaceReferencesSvcImpl.RESOURCE_TYPES_SYSTEM;
 import static ca.uhn.fhir.jpa.replacereferences.ReplaceReferencesLargeTestData.RESOURCE_TYPES_EXPECTED_TO_BE_PATCHED;
 import static ca.uhn.fhir.jpa.replacereferences.ReplaceReferencesLargeTestData.TOTAL_EXPECTED_PATCHES;
@@ -82,6 +84,9 @@ public void after() throws Exception {
 
 		myStorageSettings.setDefaultTransactionEntriesForWrite(new JpaStorageSettings().getDefaultTransactionEntriesForWrite());
 		myStorageSettings.setReuseCachedSearchResultsForMillis(new JpaStorageSettings().getReuseCachedSearchResultsForMillis());
+		// For some reason, if this value is not restored to its default at the end, it interferes with other suites,
+		// so restore the default value
+		myFhirContext.getParserOptions().setDontStripVersionsFromReferencesAtPaths(DEFAULT_PRESERVE_VERSION_REFS_R4_AND_LATER);
 	}
 
 	@Override
@@ -91,13 +96,14 @@ public void before() throws Exception {
 		myStorageSettings.setReuseCachedSearchResultsForMillis(null);
 		myStorageSettings.setAllowMultipleDelete(true);
 		myFhirContext.setParserErrorHandler(new StrictErrorHandler());
-		// we need to keep the version on Provenance.target fields to
-		// verify that Provenance resources were saved with versioned target references
-		myFhirContext.getParserOptions().setStripVersionsFromReferences(false);
+		// we need to keep the version on Provenance.target fields to verify that Provenance resources were saved
+		// with versioned target references, and delete-source on merge works correctly.
+		myFhirContext.getParserOptions().setDontStripVersionsFromReferencesAtPaths("Provenance.target");
 		myTestHelper = new ReplaceReferencesTestHelper(myFhirContext, myDaoRegistry);
 		myLargeTestData = new ReplaceReferencesLargeTestData(myDaoRegistry);
 	}
 
+
 	private void waitForAsyncTaskCompletion(Parameters theOutParams) {
 		assertThat(getLastHttpStatusCode()).isEqualTo(HttpServletResponse.SC_ACCEPTED);
 		Task task = (Task) theOutParams.getParameter(OPERATION_MERGE_OUTPUT_PARAM_TASK).getResource();