diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/api/IGenericClient.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/api/IGenericClient.java index 03c79700423..56581458047 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/api/IGenericClient.java +++ b/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(); diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/HapiExtensions.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/HapiExtensions.java index 71b262f72f0..3d5d052ac78 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/HapiExtensions.java +++ b/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 diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2332-placeholder-valuset-skips-terminology-storage.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2332-placeholder-valuset-skips-terminology-storage.yaml index cfde3b592df..8a63c1340a5 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_4_0/2332-placeholder-valuset-skips-terminology-storage.yaml +++ b/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 - and the value 'true'. Also, terminology storage is now skipped for placeholder ValueSet and ConceptMap resources." +title: "Terminology storage is now skipped for placeholder ValueSet and ConceptMap resources." diff --git a/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 b/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 new file mode 100644 index 00000000000..849ebfd76a1 --- /dev/null +++ b/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`." diff --git a/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 b/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 new file mode 100644 index 00000000000..47398526684 --- /dev/null +++ b/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`." diff --git a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java b/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java index 25106646a56..def5d13846f 100644 --- a/hapi-fhir-jpaserver-api/src/main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java +++ b/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". *

*/ 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". *

*/ public void setAutoCreatePlaceholderReferenceTargets(boolean theAutoCreatePlaceholderReferenceTargets) { @@ -1102,7 +1104,7 @@ public class DaoConfig { /** * When {@link #setAutoCreatePlaceholderReferenceTargets(boolean)} is enabled, if this - * setting is set to true (default is false) and the source + * setting is set to true (default is true) and the source * reference has an identifier populated, the identifier will be copied to the target * resource. *

@@ -1144,6 +1146,41 @@ public class DaoConfig { * } * } * + *

+ * Note that the default for this setting was previously false, and was changed to true + * in 5.4.0 with consideration to the following: + *

+ *
+	 * 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 : )
+	 * 
+ *

+ * Where CP=T and PI=F, the following could happen: + *

+ *
    + *
  1. + * Resource instance A is created with a reference to resource instance B. B is a placeholder reference target + * without an identifier. + *
  2. + *
  3. + * Resource instance C is conditionally created using a match URL. It is not matched to B although these + * resources represent the same entity. + *
  4. + *
  5. + * A continues to reference placeholder B, and does not reference populated C. + *
  6. + *
+ *

+ * There may be cases where configuring this setting to false would be appropriate; however, these are + * exceptional cases that should be opt-in. + *

* * @since 4.2.0 */ @@ -1153,7 +1190,7 @@ public class DaoConfig { /** * When {@link #setAutoCreatePlaceholderReferenceTargets(boolean)} is enabled, if this - * setting is set to true (default is false) and the source + * setting is set to true (default is true) and the source * reference has an identifier populated, the identifier will be copied to the target * resource. *

@@ -1195,6 +1232,41 @@ public class DaoConfig { * } * } * + *

+ * Note that the default for this setting was previously false, and was changed to true + * in 5.4.0 with consideration to the following: + *

+ *
+	 * 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 : )
+	 * 
+ *

+ * Where CP=T and PI=F, the following could happen: + *

+ *
    + *
  1. + * Resource instance A is created with a reference to resource instance B. B is a placeholder reference target + * without an identifier. + *
  2. + *
  3. + * Resource instance C is conditionally created using a match URL. It is not matched to B although these + * resources represent the same entity. + *
  4. + *
  5. + * A continues to reference placeholder B, and does not reference populated C. + *
  6. + *
+ *

+ * There may be cases where configuring this setting to false would be appropriate; however, these are + * exceptional cases that should be opt-in. + *

* * @since 4.2.0 */ diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/DaoResourceLinkResolver.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/DaoResourceLinkResolver.java index a65383748ac..fe3f0d0c6d9 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/DaoResourceLinkResolver.java +++ b/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 (meta instanceof IBaseHasExtensions) { - IBaseExtension extension = ((IBaseHasExtensions) meta).addExtension(); - extension.setUrl(HapiExtensions.EXT_RESOURCE_META_PLACEHOLDER); + if (newResource instanceof IBaseHasExtensions) { + IBaseExtension extension = ((IBaseHasExtensions) newResource).addExtension(); + extension.setUrl(HapiExtensions.EXT_RESOURCE_PLACEHOLDER); extension.setValue(myContext.getPrimitiveBoolean(true)); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseTermReadSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseTermReadSvcImpl.java index 0782e7db872..0a8d646a9bb 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/BaseTermReadSvcImpl.java +++ b/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) { - return theResource.getMeta().getExtensionByUrl(HapiExtensions.EXT_RESOURCE_META_PLACEHOLDER) != null; + private boolean isPlaceholder(DomainResource theResource) { + 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 diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java index 2278a76de7e..badb1502a49 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java +++ b/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");