Merge pull request #2448 from hapifhir/2446-issues-with-placeholder-reference-targets-need-to-be-resolved

Issues with placeholder reference targets have been resolved.

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/api/IGenericClient.java

@@ -174,7 +174,7 @@ public interface IGenericClient extends IRestfulClient {
 	void unregisterInterceptor(Object theInterceptor);
 
 	/**
-	 * Fluent method for the "update" operation, which performs a logical delete on a server resource
+	 * Fluent method for the "update" operation, which updates a resource instance on the server
 	 */
 	IUpdate update();
 

hapi-fhir-base/src/main/java/ca/uhn/fhir/util/HapiExtensions.java

@@ -109,8 +109,7 @@ public class HapiExtensions {
 	/**
 	 * URL for boolean extension added to all placeholder resources
 	 */
-	public static final String EXT_RESOURCE_META_PLACEHOLDER = "http://hapifhir.io/fhir/StructureDefinition/resource-meta-placeholder";
+	public static final String EXT_RESOURCE_PLACEHOLDER = "http://hapifhir.io/fhir/StructureDefinition/resource-placeholder";
-
 
 	/**
 	 * Non instantiable

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2332-placeholder-valuset-skips-terminology-storage.yaml

@@ -1,5 +1,4 @@
 ---
 type: add
 issue: 2332
-title: "All created placeholder resources now have a meta extension with the url http://hapifhir.io/fhir/StructureDefinition/resource-meta-placeholder
+title: "Terminology storage is now skipped for placeholder ValueSet and ConceptMap resources."
- and the value 'true'.  Also, terminology storage is now skipped for placeholder ValueSet and ConceptMap resources."

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2446-issues-with-placeholder-reference-targets-need-to-be-resolved-placeholder-extension.yaml

@@ -0,0 +1,5 @@
+---
+type: add
+issue: 2446
+title: "Auto-created placeholder reference targets now have an extension with the URL
+   `http://hapifhir.io/fhir/StructureDefinition/resource-meta-placeholder` and a value of `true`."

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2446-issues-with-placeholder-reference-targets-need-to-be-resolved-placeholder-identifier-true.yaml

@@ -0,0 +1,5 @@
+---
+type: change
+issue: 2446
+title: "DaoConfig setting for [Populate Identifier In Auto Created Placeholder Reference Targets](https://hapifhir.io/hapi-fhir/apidocs/hapi-fhir-jpaserver-api/ca/uhn/fhir/jpa/api/config/DaoConfig.html#setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(boolean))
+   now defaults to `true`."

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

@@ -186,9 +186,11 @@ public class DaoConfig {
 	private int myPreExpandValueSetsMaxCount = 1000;
 
 	/**
+	 * Do not change default of {@code true}!
+	 *
 	 * @since 4.2.0
 	 */
-	private boolean myPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets;
+	private boolean myPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets = true;
 
 	/**
 	 * @since 5.0.0
@@ -1069,7 +1071,7 @@ public class DaoConfig {
 	 * as they are never allowed to be client supplied in HAPI FHIR JPA.
 	 *
 	 * All placeholder resources created in this way have a meta extension
-	 * with the URL {@link HapiExtensions#EXT_RESOURCE_META_PLACEHOLDER} and the value "true".
+	 * with the URL {@link HapiExtensions#EXT_RESOURCE_PLACEHOLDER} and the value "true".
 	 * </p>
 	 */
 	public boolean isAutoCreatePlaceholderReferenceTargets() {
@@ -1093,7 +1095,7 @@ public class DaoConfig {
 	 * as they are never allowed to be client supplied in HAPI FHIR JPA.
 	 *
 	 * All placeholder resources created in this way have a meta extension
-	 * with the URL {@link HapiExtensions#EXT_RESOURCE_META_PLACEHOLDER} and the value "true".
+	 * with the URL {@link HapiExtensions#EXT_RESOURCE_PLACEHOLDER} and the value "true".
 	 * </p>
 	 */
 	public void setAutoCreatePlaceholderReferenceTargets(boolean theAutoCreatePlaceholderReferenceTargets) {
@@ -1102,7 +1104,7 @@ public class DaoConfig {
 
 	/**
 	 * When {@link #setAutoCreatePlaceholderReferenceTargets(boolean)} is enabled, if this
-	 * setting is set to <code>true</code> (default is <code>false</code>) and the source
+	 * setting is set to <code>true</code> (default is <code>true</code>) and the source
 	 * reference has an identifier populated, the identifier will be copied to the target
 	 * resource.
 	 * <p>
@@ -1144,6 +1146,41 @@ public class DaoConfig {
 	 *   }
 	 * }
 	 * </pre>
+	 * <p>
+	 * Note that the default for this setting was previously <code>false</code>, and was changed to <code>true</code>
+	 * in 5.4.0 with consideration to the following:
+	 * </p>
+	 * <pre>
+	 * CP = Auto-Create Placeholder Reference Targets
+	 * PI = Populate Identifier in Auto-Created Placeholder Reference Targets
+	 *
+	 * CP | PI
+	 * -------
+	 *  F | F  <- PI=F is ignored
+	 *  F | T  <- PI=T is ignored
+	 *  T | F  <- resources may reference placeholder reference targets that are never updated : (
+	 *  T | T  <- placeholder reference targets can be updated : )
+	 * </pre>
+	 * <p>
+	 * Where CP=T and PI=F, the following could happen:
+	 * </p>
+	 * <ol>
+	 *    <li>
+	 *       Resource instance A is created with a reference to resource instance B. B is a placeholder reference target
+	 *       without an identifier.
+	 *    </li>
+	 *    <li>
+	 * 	   Resource instance C is conditionally created using a match URL. It is not matched to B although these
+	 * 	   resources represent the same entity.
+	 *    </li>
+	 *    <li>
+	 *       A continues to reference placeholder B, and does not reference populated C.
+	 *    </li>
+	 * </ol>
+	 * <p>
+	 * There may be cases where configuring this setting to <code>false</code> would be appropriate; however, these are
+	 * exceptional cases that should be opt-in.
+	 * </p>
 	 *
 	 * @since 4.2.0
 	 */
@@ -1153,7 +1190,7 @@ public class DaoConfig {
 
 	/**
 	 * When {@link #setAutoCreatePlaceholderReferenceTargets(boolean)} is enabled, if this
-	 * setting is set to <code>true</code> (default is <code>false</code>) and the source
+	 * setting is set to <code>true</code> (default is <code>true</code>) and the source
 	 * reference has an identifier populated, the identifier will be copied to the target
 	 * resource.
 	 * <p>
@@ -1195,6 +1232,41 @@ public class DaoConfig {
 	 *   }
 	 * }
 	 * </pre>
+	 * <p>
+	 * Note that the default for this setting was previously <code>false</code>, and was changed to <code>true</code>
+	 * in 5.4.0 with consideration to the following:
+	 * </p>
+	 * <pre>
+	 * CP = Auto-Create Placeholder Reference Targets
+	 * PI = Populate Identifier in Auto-Created Placeholder Reference Targets
+	 *
+	 * CP | PI
+	 * -------
+	 *  F | F  <- PI=F is ignored
+	 *  F | T  <- PI=T is ignored
+	 *  T | F  <- resources may reference placeholder reference targets that are never updated : (
+	 *  T | T  <- placeholder reference targets can be updated : )
+	 * </pre>
+	 * <p>
+	 * Where CP=T and PI=F, the following could happen:
+	 * </p>
+	 * <ol>
+	 *    <li>
+	 *       Resource instance A is created with a reference to resource instance B. B is a placeholder reference target
+	 *       without an identifier.
+	 *    </li>
+	 *    <li>
+	 * 	   Resource instance C is conditionally created using a match URL. It is not matched to B although these
+	 * 	   resources represent the same entity.
+	 *    </li>
+	 *    <li>
+	 *       A continues to reference placeholder B, and does not reference populated C.
+	 *    </li>
+	 * </ol>
+	 * <p>
+	 * There may be cases where configuring this setting to <code>false</code> would be appropriate; however, these are
+	 * exceptional cases that should be opt-in.
+	 * </p>
 	 *
 	 * @since 4.2.0
 	 */

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

@@ -41,7 +41,6 @@ import ca.uhn.fhir.util.HapiExtensions;
 import org.hl7.fhir.instance.model.api.IBase;
 import org.hl7.fhir.instance.model.api.IBaseExtension;
 import org.hl7.fhir.instance.model.api.IBaseHasExtensions;
-import org.hl7.fhir.instance.model.api.IBaseMetaType;
 import org.hl7.fhir.instance.model.api.IBaseReference;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.instance.model.api.IIdType;
@@ -123,10 +122,9 @@ public class DaoResourceLinkResolver implements IResourceLinkResolver {
 			@SuppressWarnings("unchecked")
 			T newResource = (T) missingResourceDef.newInstance();
 
-			IBaseMetaType meta = newResource.getMeta();
+			if (newResource instanceof IBaseHasExtensions) {
-			if (meta instanceof IBaseHasExtensions) {
+				IBaseExtension<?, ?> extension = ((IBaseHasExtensions) newResource).addExtension();
-				IBaseExtension<?, ?> extension = ((IBaseHasExtensions) meta).addExtension();
+				extension.setUrl(HapiExtensions.EXT_RESOURCE_PLACEHOLDER);
-				extension.setUrl(HapiExtensions.EXT_RESOURCE_META_PLACEHOLDER);
 				extension.setValue(myContext.getPrimitiveBoolean(true));
 			}
 

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseTermReadSvcImpl.java

@@ -119,15 +119,17 @@ import org.hl7.fhir.instance.model.api.IBaseDatatype;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.instance.model.api.IIdType;
 import org.hl7.fhir.instance.model.api.IPrimitiveType;
+import org.hl7.fhir.r4.model.BooleanType;
 import org.hl7.fhir.r4.model.CanonicalType;
 import org.hl7.fhir.r4.model.CodeSystem;
 import org.hl7.fhir.r4.model.CodeableConcept;
 import org.hl7.fhir.r4.model.Coding;
 import org.hl7.fhir.r4.model.ConceptMap;
+import org.hl7.fhir.r4.model.DomainResource;
 import org.hl7.fhir.r4.model.Enumerations;
+import org.hl7.fhir.r4.model.Extension;
 import org.hl7.fhir.r4.model.IdType;
 import org.hl7.fhir.r4.model.IntegerType;
-import org.hl7.fhir.r4.model.MetadataResource;
 import org.hl7.fhir.r4.model.StringType;
 import org.hl7.fhir.r4.model.ValueSet;
 import org.hl7.fhir.r4.model.codesystems.ConceptSubsumptionOutcome;
@@ -2122,8 +2124,13 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc {
 		}
 	}
 
-	private boolean isPlaceholder(MetadataResource theResource) {
+	private boolean isPlaceholder(DomainResource theResource) {
-		return theResource.getMeta().getExtensionByUrl(HapiExtensions.EXT_RESOURCE_META_PLACEHOLDER) != null;
+		boolean retVal = false;
+		Extension extension = theResource.getExtensionByUrl(HapiExtensions.EXT_RESOURCE_PLACEHOLDER);
+		if (extension != null && extension.hasValue() && extension.getValue() instanceof BooleanType) {
+			retVal = ((BooleanType) extension.getValue()).booleanValue();
+		}
+		return retVal;
 	}
 
 	@Override

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

@@ -7,10 +7,12 @@ import ca.uhn.fhir.rest.param.ReferenceParam;
 import ca.uhn.fhir.rest.param.TokenParam;
 import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
 import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
-import ca.uhn.fhir.util.TestUtil;
+import ca.uhn.fhir.util.HapiExtensions;
 import com.google.common.collect.Sets;
 import org.hl7.fhir.instance.model.api.IIdType;
 import org.hl7.fhir.r4.model.AuditEvent;
+import org.hl7.fhir.r4.model.BooleanType;
+import org.hl7.fhir.r4.model.Extension;
 import org.hl7.fhir.r4.model.IdType;
 import org.hl7.fhir.r4.model.Observation;
 import org.hl7.fhir.r4.model.Observation.ObservationStatus;
@@ -18,16 +20,18 @@ import org.hl7.fhir.r4.model.Patient;
 import org.hl7.fhir.r4.model.Reference;
 import org.hl7.fhir.r4.model.Task;
 import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Test;
 
 import java.util.List;
 
 import static org.hamcrest.CoreMatchers.startsWith;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.contains;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
 @SuppressWarnings({"ConstantConditions"})
@@ -174,9 +178,54 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test {
 	}
 
 	@Test
-	public void testCreatePlaceholderWithMatchUrl_IdentifierNotCopiedByDefault() {
+	public void testCreatePlaceholderExtension_WithUpdateToTarget() {
+		myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true);
+
+		// Create an Observation that references a Patient
+		Observation obsToCreate = new Observation();
+		obsToCreate.setStatus(ObservationStatus.FINAL);
+		obsToCreate.getSubject().setReference("Patient/AAA");
+		IIdType id = myObservationDao.create(obsToCreate, mySrd).getId();
+
+		// Read the Observation
+		Observation createdObs = myObservationDao.read(id);
+		ourLog.info("\nObservation created:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(createdObs));
+
+		/*
+		 * Read the placeholder Patient referenced by the Observation
+		 * Placeholder extension should exist and be true
+		 */
+		Patient placeholderPat = myPatientDao.read(new IdType(createdObs.getSubject().getReference()));
+		IIdType placeholderPatId = placeholderPat.getIdElement();
+		ourLog.info("\nPlaceholder Patient created:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(placeholderPat));
+		assertEquals(0, placeholderPat.getIdentifier().size());
+		Extension extension = placeholderPat.getExtensionByUrl(HapiExtensions.EXT_RESOURCE_PLACEHOLDER);
+		assertNotNull(extension);
+		assertTrue(extension.hasValue());
+		assertTrue(((BooleanType) extension.getValue()).booleanValue());
+
+		// Update the Patient
+		Patient patToUpdate = new Patient();
+		patToUpdate.setId("Patient/AAA");
+		patToUpdate.addIdentifier().setSystem("http://foo").setValue("123");
+		IIdType updatedPatId = myPatientDao.update(patToUpdate).getId();
+
+		/*
+		 * Read the updated Patient
+		 * Placeholder extension should not exist
+		 */
+		Patient updatedPat = myPatientDao.read(updatedPatId);
+		ourLog.info("\nUpdated Patient:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(updatedPat));
+		assertEquals(1, updatedPat.getIdentifier().size());
+		extension = updatedPat.getExtensionByUrl(HapiExtensions.EXT_RESOURCE_PLACEHOLDER);
+		assertNull(extension);
+	}
+
+	@Test
+	public void testCreatePlaceholderWithMatchUrl_IdentifierNotCopied() {
 		myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true);
 		myDaoConfig.setAllowInlineMatchUrlReferences(true);
+		myDaoConfig.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(false);
 
 		Observation obsToCreate = new Observation();
 		obsToCreate.setStatus(ObservationStatus.FINAL);
@@ -192,12 +241,60 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test {
 		assertEquals(0, patient.getIdentifier().size());
 	}
 
-
 	@Test
-	public void testCreatePlaceholderWithMatchUrl_IdentifierCopied_NotPreExisting() {
+	public void testCreatePlaceholderWithMatchUrl_IdentifierCopiedByDefault_WithUpdateToTarget() {
+		myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true);
+		myDaoConfig.setAllowInlineMatchUrlReferences(true);
+
+		/*
+		 * Create an Observation that references a Patient
+		 * Reference is populated with inline match URL and includes identifier
+		 */
+		Observation obsToCreate = new Observation();
+		obsToCreate.setStatus(ObservationStatus.FINAL);
+		obsToCreate.getSubject().setReference("Patient?identifier=http://foo|123");
+		obsToCreate.getSubject().getIdentifier().setSystem("http://foo").setValue("123");
+		IIdType obsId = myObservationDao.create(obsToCreate, mySrd).getId();
+
+		// Read the Observation
+		Observation createdObs = myObservationDao.read(obsId);
+		ourLog.info("\nObservation created:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(createdObs));
+
+		/*
+		 * Read the placeholder Patient referenced by the Observation
+		 * Identifier should be populated since it was provided
+		 */
+		Patient placeholderPat = myPatientDao.read(new IdType(createdObs.getSubject().getReference()));
+		IIdType placeholderPatId = placeholderPat.getIdElement();
+		ourLog.info("\nPlaceholder Patient created:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(placeholderPat));
+		assertEquals(1, placeholderPat.getIdentifier().size());
+		assertEquals(createdObs.getSubject().getReference(), placeholderPatId.toUnqualifiedVersionless().getValueAsString());
+
+		// Conditionally update a Patient with the same identifier
+		Patient patToConditionalUpdate = new Patient();
+		patToConditionalUpdate.addIdentifier().setSystem("http://foo").setValue("123");
+		patToConditionalUpdate.addName().setFamily("Simpson");
+		IIdType conditionalUpdatePatId = myPatientDao.update(patToConditionalUpdate, "Patient?identifier=http://foo|123", mySrd).getId();
+
+		// Read the conditionally updated Patient
+		Patient conditionalUpdatePat = myPatientDao.read(conditionalUpdatePatId);
+		ourLog.info("\nConditionally updated Patient:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(conditionalUpdatePat));
+		assertEquals(1, conditionalUpdatePat.getIdentifier().size());
+
+		/*
+		 * Observation should reference conditionally updated Patient
+		 * ID of placeholder Patient should match ID of conditionally updated Patient
+		 */
+		createdObs = myObservationDao.read(obsId);
+		ourLog.info("\nObservation read after Patient update:\n" + myFhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(createdObs));
+		assertEquals(createdObs.getSubject().getReference(), conditionalUpdatePatId.toUnqualifiedVersionless().getValueAsString());
+		assertEquals(placeholderPatId.toUnqualifiedVersionless().getValueAsString(), conditionalUpdatePatId.toUnqualifiedVersionless().getValueAsString());
+	}
+
+	@Test
+	public void testCreatePlaceholderWithMatchUrl_IdentifierCopiedByDefault_NotPreExisting() {
 		myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true);
 		myDaoConfig.setAllowInlineMatchUrlReferences(true);
-		myDaoConfig.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(true);
 
 		Observation obsToCreate = new Observation();
 		obsToCreate.setStatus(ObservationStatus.FINAL);
@@ -219,7 +316,6 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test {
 	public void testCreatePlaceholderWithMatchUrl_IdentifierNotCopiedBecauseNoFieldMatches() {
 		myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true);
 		myDaoConfig.setAllowInlineMatchUrlReferences(true);
-		myDaoConfig.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(true);
 		myDaoConfig.setBundleTypesAllowedForStorage(Sets.newHashSet(""));
 
 		AuditEvent eventToCreate = new AuditEvent();
@@ -238,7 +334,6 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test {
 	public void testCreatePlaceholderWithMatchUrl_PreExisting() {
 		myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true);
 		myDaoConfig.setAllowInlineMatchUrlReferences(true);
-		myDaoConfig.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(true);
 
 		Patient patient = new Patient();
 		patient.setId("ABC");