4441 rel 6 4 bad references creation bug (#4519)

* adding a test

* fail in the case of ref enforce on type and on write and autocreate are all true

* update to code

* removing a line

* cleanup

* removing check on urn

* changing just to trigger a build

* adding a comment to the pom

* updating test for better information

---------

Co-authored-by: leif stawnyczy <leifstawnyczy@leifs-mbp.home>

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_4_0/4441-bad-resource-refs-should-not-create.yaml

@@ -0,0 +1,7 @@
+---
+type: fix
+issue: 4441
+title: "Creating a resource with an invalid embedded resource reference
+      would not fail. Even if IsEnforceReferentialIntegrityOnWrite was enabled.
+      This has been fixed, and invalid references will throw.
+      "

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

@@ -0,0 +1,54 @@
+package ca.uhn.fhir.jpa.dao.index;
+
+import ca.uhn.fhir.rest.api.server.RequestDetails;
+import ca.uhn.fhir.rest.api.server.storage.TransactionDetails;
+import org.hl7.fhir.instance.model.api.IBaseResource;
+
+public class ExtractInlineReferenceParams {
+	private IBaseResource myResource;
+	private TransactionDetails myTransactionDetails;
+	private RequestDetails myRequestDetails;
+	private boolean myFailOnInvalidReferences;
+
+	public ExtractInlineReferenceParams(
+		IBaseResource theResource,
+		TransactionDetails theTransactionDetails,
+		RequestDetails theRequest
+	) {
+		myResource = theResource;
+		myTransactionDetails = theTransactionDetails;
+		myRequestDetails = theRequest;
+	}
+
+	public IBaseResource getResource() {
+		return myResource;
+	}
+
+	public void setResource(IBaseResource theResource) {
+		myResource = theResource;
+	}
+
+	public TransactionDetails getTransactionDetails() {
+		return myTransactionDetails;
+	}
+
+	public void setTransactionDetails(TransactionDetails theTransactionDetails) {
+		myTransactionDetails = theTransactionDetails;
+	}
+
+	public RequestDetails getRequestDetails() {
+		return myRequestDetails;
+	}
+
+	public void setRequestDetails(RequestDetails theRequestDetails) {
+		myRequestDetails = theRequestDetails;
+	}
+
+	public boolean isFailOnInvalidReferences() {
+		return myFailOnInvalidReferences;
+	}
+
+	public void setFailOnInvalidReferences(boolean theFailOnInvalidReferences) {
+		myFailOnInvalidReferences = theFailOnInvalidReferences;
+	}
+}

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

@@ -113,7 +113,9 @@ public class SearchParamWithInlineReferencesExtractor {
 	}
 
 	public void populateFromResource(RequestPartitionId theRequestPartitionId, ResourceIndexedSearchParams theParams, TransactionDetails theTransactionDetails, ResourceTable theEntity, IBaseResource theResource, ResourceIndexedSearchParams theExistingParams, RequestDetails theRequest, boolean theFailOnInvalidReference) {
-		extractInlineReferences(theResource, theTransactionDetails, theRequest);
+		ExtractInlineReferenceParams theExtractParams = new ExtractInlineReferenceParams(theResource, theTransactionDetails, theRequest);
+		theExtractParams.setFailOnInvalidReferences(theFailOnInvalidReference);
+		extractInlineReferences(theExtractParams);
 
 		mySearchParamExtractorService.extractFromResource(theRequestPartitionId, theRequest, theParams, theExistingParams, theEntity, theResource, theTransactionDetails, theFailOnInvalidReference);
 
@@ -188,16 +190,32 @@ public class SearchParamWithInlineReferencesExtractor {
 		myContext = theContext;
 	}
 
+	@Deprecated
+	public void extractInlineReferences(
+		IBaseResource theResource,
+		TransactionDetails theTransactionDetails,
+		RequestDetails theRequest
+	) {
+		extractInlineReferences(new ExtractInlineReferenceParams(theResource, theTransactionDetails, theRequest));
+	}
+
 	/**
-	 * Handle references within the resource that are match URLs, for example references like "Patient?identifier=foo". These match URLs are resolved and replaced with the ID of the
+	 * Handle references within the resource that are match URLs, for example references like "Patient?identifier=foo".
+	 * These match URLs are resolved and replaced with the ID of the
 	 * matching resource.
+	 *
+	 * This method is *only* called from UPDATE path
 	 */
-	public void extractInlineReferences(IBaseResource theResource, TransactionDetails theTransactionDetails, RequestDetails theRequest) {
+	public void extractInlineReferences(ExtractInlineReferenceParams theParams) {
 		if (!myDaoConfig.isAllowInlineMatchUrlReferences()) {
 			return;
 		}
+		IBaseResource resource = theParams.getResource();
+		RequestDetails theRequest = theParams.getRequestDetails();
+		TransactionDetails theTransactionDetails = theParams.getTransactionDetails();
+
 		FhirTerser terser = myContext.newTerser();
-		List<IBaseReference> allRefs = terser.getAllPopulatedChildElementsOfType(theResource, IBaseReference.class);
+		List<IBaseReference> allRefs = terser.getAllPopulatedChildElementsOfType(resource, IBaseReference.class);
 		for (IBaseReference nextRef : allRefs) {
 			IIdType nextId = nextRef.getReferenceElement();
 			String nextIdText = nextId.getValue();
@@ -229,7 +247,6 @@ public class SearchParamWithInlineReferencesExtractor {
 
 				JpaPid match;
 				if (matches.isEmpty()) {
-
 					Optional<IBasePersistedResource> placeholderOpt = myDaoResourceLinkResolver.createPlaceholderTargetIfConfiguredToDoSo(matchResourceType, nextRef, null, theRequest, theTransactionDetails);
 					if (placeholderOpt.isPresent()) {
 						match = (JpaPid) placeholderOpt.get().getPersistentId();

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

@@ -1818,10 +1818,11 @@ public abstract class BaseSearchParamExtractor implements ISearchParamExtractor
 				nextId = valueRef.getResource().getIdElement();
 			}
 
-			if (nextId == null ||
+			if (
-				nextId.isEmpty() ||
+				nextId == null ||
-				nextId.getValue().startsWith("urn:")) {
+				nextId.isEmpty()
-				// Ignore placeholder references
+			) {
+				// Ignore placeholder references that are blank
 			} else if (!theWantLocalReferences && nextId.getValue().startsWith("#")) {
 				// Ignore local refs unless we specifically want them
 			} else {

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

@@ -105,8 +105,10 @@ public class SearchParamExtractorService {
 	}
 
 	/**
-	 * This method is responsible for scanning a resource for all of the search parameter instances. I.e. for all search parameters defined for
+	 * This method is responsible for scanning a resource for all of the search parameter instances.
-	 * a given resource type, it extracts the associated indexes and populates {@literal theParams}.
+	 * I.e. for all search parameters defined for
+	 * a given resource type, it extracts the associated indexes and populates
+	 * {@literal theParams}.
 	 */
 	public void extractFromResource(RequestPartitionId theRequestPartitionId, RequestDetails theRequestDetails, ResourceIndexedSearchParams theNewParams, ResourceIndexedSearchParams theExistingParams, ResourceTable theEntity, IBaseResource theResource, TransactionDetails theTransactionDetails, boolean theFailOnInvalidReference) {
 

hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkExportUseCaseTest.java

@@ -54,6 +54,7 @@ import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.concurrent.TimeUnit;
 
+import static org.apache.commons.lang3.StringUtils.isNotBlank;
 import static org.awaitility.Awaitility.await;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.notNullValue;
@@ -164,7 +165,9 @@ public class BulkExportUseCaseTest extends BaseResourceProviderR4Test {
 			HttpGet statusGet = new HttpGet(pollingLocation);
 			String expectedOriginalUrl = myClient.getServerBase() + "/$export";
 			try (CloseableHttpResponse status = ourHttpClient.execute(statusGet)) {
+				assertEquals(200, status.getStatusLine().getStatusCode());
 				String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
+				assertTrue(isNotBlank(responseContent), responseContent);
 
 				ourLog.info(responseContent);
 

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

@@ -3,6 +3,7 @@ package ca.uhn.fhir.jpa.provider.r4;
 import ca.uhn.fhir.i18n.HapiLocalizer;
 import ca.uhn.fhir.i18n.Msg;
 import ca.uhn.fhir.jpa.api.config.DaoConfig;
+import ca.uhn.fhir.jpa.api.model.DaoMethodOutcome;
 import ca.uhn.fhir.jpa.dao.data.ISearchDao;
 import ca.uhn.fhir.jpa.entity.Search;
 import ca.uhn.fhir.jpa.model.entity.ModelConfig;
@@ -12,6 +13,7 @@ import ca.uhn.fhir.jpa.model.util.JpaConstants;
 import ca.uhn.fhir.jpa.model.util.UcumServiceUtil;
 import ca.uhn.fhir.jpa.provider.BaseResourceProviderR4Test;
 import ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl;
+import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
 import ca.uhn.fhir.jpa.term.ZipCollectionBuilder;
 import ca.uhn.fhir.jpa.test.config.TestR4Config;
 import ca.uhn.fhir.jpa.util.QueryParameterUtils;
@@ -27,6 +29,7 @@ import ca.uhn.fhir.rest.api.MethodOutcome;
 import ca.uhn.fhir.rest.api.PreferReturnEnum;
 import ca.uhn.fhir.rest.api.SearchTotalModeEnum;
 import ca.uhn.fhir.rest.api.SummaryEnum;
+import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
 import ca.uhn.fhir.rest.client.apache.ResourceEntity;
 import ca.uhn.fhir.rest.client.api.IClientInterceptor;
 import ca.uhn.fhir.rest.client.api.IGenericClient;
@@ -309,12 +312,10 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 
 		assertNotNull(id);
 		assertEquals("resource-security", id.getIdPart());
-
 	}
 
 	@Test
 	public void createSearchParameter_with2Expressions_succeeds() {
-
 		SearchParameter searchParameter = new SearchParameter();
 
 		searchParameter.setStatus(Enumerations.PublicationStatus.ACTIVE);
@@ -326,7 +327,6 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 		MethodOutcome result = myClient.create().resource(searchParameter).execute();
 
 		assertEquals(true, result.getCreated());
-
 	}
 
 	@Test
@@ -454,7 +454,6 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 			assertThat(output, containsString(MSG_PREFIX_INVALID_FORMAT + "">""));
 			assertEquals(400, resp.getStatusLine().getStatusCode());
 		}
-
 	}
 
 
@@ -911,7 +910,7 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 
 	@Test
 	@Disabled
-	public void test() throws IOException {
+	public void testMakingQuery() throws IOException {
 		HttpGet get = new HttpGet(myServerBase + "/QuestionnaireResponse?_count=50&status=completed&questionnaire=ARIncenterAbsRecord&_lastUpdated=%3E" + UrlUtil.escapeUrlParam("=2018-01-01") + "&context.organization=O3435");
 		ourLog.info("*** MAKING QUERY");
 		ourHttpClient.execute(get);
@@ -7556,6 +7555,113 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 		assertTrue(resultIds.contains("Patient/" + patientId + "/_history/2"));
 	}
 
+
+	private static class CreateResourceInput {
+		boolean IsEnforceRefOnWrite;
+		boolean IsEnforceRefOnType;
+		boolean IsAutoCreatePlaceholderReferences;
+
+		public CreateResourceInput(
+			boolean theEnforceRefOnWrite,
+			boolean theEnforceRefOnType,
+			boolean theAutoCreatePlaceholders
+		) {
+			IsEnforceRefOnWrite = theEnforceRefOnWrite;
+			IsEnforceRefOnType = theEnforceRefOnType;
+			IsAutoCreatePlaceholderReferences = theAutoCreatePlaceholders;
+		}
+
+		@Override
+		public String toString() {
+			return "IsEnforceReferentialIntegrityOnWrite : "
+				+ IsEnforceRefOnWrite + "\n"
+				+ "IsEnforceReferenceTargetTypes : "
+				+ IsEnforceRefOnType + "\n"
+				+ "IsAutoCreatePlaceholderReferenceTargets : "
+				+ IsAutoCreatePlaceholderReferences + "\n";
+		}
+	}
+
+	private static List<CreateResourceInput> createResourceParameters() {
+		boolean[] bools = new boolean[] { true, false };
+		List<CreateResourceInput> input = new ArrayList<>();
+		for (boolean bool : bools) {
+			for (boolean bool2 : bools) {
+				for (boolean bool3 : bools) {
+					input.add(new CreateResourceInput(bool, bool2, bool3));
+				}
+			}
+		}
+		return input;
+	}
+
+	@ParameterizedTest
+	@MethodSource("createResourceParameters")
+	public void createResource_refIntegrityOnWriteAndRefTargetTypes_throws(CreateResourceInput theInput) {
+		ourLog.info(
+			String.format("Test case : \n%s", theInput.toString())
+		);
+
+		String patientStr = """
+			{
+			  "resourceType": "Patient",
+			  "managingOrganization": {
+			    "reference": "urn:uuid:d8080e87-1842-46b4-aea0-b65803bc2897"
+			  }
+			}
+			""";
+		IParser parser = myFhirContext.newJsonParser();
+		Patient patient = parser.parseResource(Patient.class, patientStr);
+
+		{
+			List<IBaseResource> orgs = myOrganizationDao
+				.search(new SearchParameterMap(), new SystemRequestDetails())
+				.getAllResources();
+
+			assertTrue(orgs == null || orgs.isEmpty());
+		}
+
+		boolean isEnforceRefOnWrite = myDaoConfig.isEnforceReferentialIntegrityOnWrite();
+		boolean isEnforceRefTargetTypes = myDaoConfig.isEnforceReferenceTargetTypes();
+		boolean isAutoCreatePlaceholderReferences = myDaoConfig.isAutoCreatePlaceholderReferenceTargets();
+
+		try {
+			// allows resources to be created even if they have local resources that do not exist
+			myDaoConfig.setEnforceReferentialIntegrityOnWrite(theInput.IsEnforceRefOnWrite);
+			// ensures target references are using the correct resource type
+			myDaoConfig.setEnforceReferenceTargetTypes(theInput.IsEnforceRefOnType);
+			// will create the resource if it does not already exist
+			myDaoConfig.setAutoCreatePlaceholderReferenceTargets(theInput.IsAutoCreatePlaceholderReferences);
+
+			// should fail
+			DaoMethodOutcome result = myPatientDao.create(patient, new SystemRequestDetails());
+
+			// a bad reference can never create a new resource
+			{
+				List<IBaseResource> orgs = myOrganizationDao
+					.search(new SearchParameterMap(), new SystemRequestDetails())
+					.getAllResources();
+
+				assertTrue(orgs == null || orgs.isEmpty());
+			}
+
+			// only if all 3 are true do we expect this to fail
+			assertFalse(
+				theInput.IsAutoCreatePlaceholderReferences
+				&& theInput.IsEnforceRefOnType
+				&& theInput.IsEnforceRefOnWrite
+			);
+		} catch (InvalidRequestException ex) {
+			assertTrue(ex.getMessage().contains(
+				"Invalid resource reference"
+			), ex.getMessage());
+		} finally {
+			myDaoConfig.setEnforceReferentialIntegrityOnWrite(isEnforceRefOnWrite);
+			myDaoConfig.setEnforceReferenceTargetTypes(isEnforceRefTargetTypes);
+			myDaoConfig.setAutoCreatePlaceholderReferenceTargets(isAutoCreatePlaceholderReferences);
+		}
+	}
+
 	@Test
 	public void searchResource_bySourceWithPreserveRequestIdDisabled_isSuccess() {
 		String sourceUri = "http://acme.org";
@@ -8085,5 +8191,4 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 			);
 		}
 	}
-
 }

hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java

@@ -1630,7 +1630,7 @@ public class DaoConfig {
 	 * <p>
 	 * For example, if a patient contains a reference to managing organization <code>Organization/FOO</code>
 	 * but FOO is not a valid ID for an organization on the server, the operation will be blocked unless
-	 * this propery has been set to <code>false</code>
+	 * this property has been set to <code>false</code>
 	 * </p>
 	 * <p>
 	 * This property can cause confusing results for clients of the server since searches, includes,
@@ -1648,7 +1648,7 @@ public class DaoConfig {
 	 * <p>
 	 * For example, if a patient contains a reference to managing organization <code>Organization/FOO</code>
 	 * but FOO is not a valid ID for an organization on the server, the operation will be blocked unless
-	 * this propery has been set to <code>false</code>
+	 * this property has been set to <code>false</code>
 	 * </p>
 	 * <p>
 	 * This property can cause confusing results for clients of the server since searches, includes,

pom.xml

@@ -2,6 +2,7 @@
 <project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://maven.apache.org/POM/4.0.0"
 			xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
 
+	<!-- the version info -->
 	<modelVersion>4.0.0</modelVersion>
 	<groupId>ca.uhn.hapi.fhir</groupId>
 	<artifactId>hapi-fhir</artifactId>