From e8c080bb12fae44b11c10a9c7338ff187731154a Mon Sep 17 00:00:00 2001 From: Diederik Muylwyk Date: Thu, 4 Mar 2021 13:45:09 -0500 Subject: [PATCH 01/13] Add initial FIXMEs. --- .../ca/uhn/fhir/jpa/dao/index/DaoResourceLinkResolver.java | 2 ++ .../dao/index/SearchParamWithInlineReferencesExtractor.java | 1 + .../main/java/ca/uhn/fhir/jpa/term/BaseTermReadSvcImpl.java | 1 + .../src/test/java/ca/uhn/fhir/jpa/packages/NpmR4Test.java | 3 +++ 4 files changed, 7 insertions(+) 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..f78746bbe6e 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 @@ -110,6 +110,7 @@ public class DaoResourceLinkResolver implements IResourceLinkResolver { return resolvedResource; } + // FIXME: DM 2021-03-04 - This is where placeholder reference targets are handled. /** * @param theIdToAssignToPlaceholder If specified, the placeholder resource created will be given a specific ID */ @@ -123,6 +124,7 @@ public class DaoResourceLinkResolver implements IResourceLinkResolver { @SuppressWarnings("unchecked") T newResource = (T) missingResourceDef.newInstance(); + // FIXME: DM 2021-03-04 - This is where the placeholder extension is added; should be in resource body instead of meta? IBaseMetaType meta = newResource.getMeta(); if (meta instanceof IBaseHasExtensions) { IBaseExtension extension = ((IBaseHasExtensions) meta).addExtension(); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/SearchParamWithInlineReferencesExtractor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/SearchParamWithInlineReferencesExtractor.java index 0a059bbc1ab..bdbf6ec991d 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/SearchParamWithInlineReferencesExtractor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/SearchParamWithInlineReferencesExtractor.java @@ -214,6 +214,7 @@ public class SearchParamWithInlineReferencesExtractor { } + // FIXME: DM 2021-03-04 - This is where inline match URLs are handled. /** * 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. 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..a5c579e096c 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 @@ -2123,6 +2123,7 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { } private boolean isPlaceholder(MetadataResource theResource) { + // FIXME: DM 2021-03-04 - We should probably check if this extension exists and the value is true. return theResource.getMeta().getExtensionByUrl(HapiExtensions.EXT_RESOURCE_META_PLACEHOLDER) != null; } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/NpmR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/NpmR4Test.java index eeaa4eac020..1f864c9471e 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/NpmR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/NpmR4Test.java @@ -357,6 +357,9 @@ public class NpmR4Test extends BaseJpaR4Test { } } + // FIXME: DM 2021-03-04 - This test is meant to validate that placeholder ValueSets are not created. + // FIXME: DM 2021-03-04 - This test or a new test should probably validate that placeholder ConceptMaps are also not created. + // FIXME: DM 2021-03-04 - This test does not validate that placeholder resources are created with a EXT_RESOURCE_META_PLACEHOLDER extension. // Reproduces https://github.com/hapifhir/hapi-fhir/issues/2332 @Test public void testInstallR4Package_AutoCreatePlaceholder() throws Exception { From 5a952ad9809a8b5d1335781e99361d08d3d7e6c1 Mon Sep 17 00:00:00 2001 From: Diederik Muylwyk Date: Thu, 4 Mar 2021 14:16:14 -0500 Subject: [PATCH 02/13] Add more FIXMEs as my understanding grows. --- .../jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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..3b296bb9e68 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,7 +7,6 @@ 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 com.google.common.collect.Sets; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.AuditEvent; @@ -18,16 +17,15 @@ 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.fail; @SuppressWarnings({"ConstantConditions"}) @@ -173,6 +171,7 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { assertEquals("Patient/999999999999999", outcome.getResources(0,1).get(0).getIdElement().toUnqualifiedVersionless().getValue()); } + // FIXME: DM 2021-03-04 - These ARE the tests you're looking for. @Test public void testCreatePlaceholderWithMatchUrl_IdentifierNotCopiedByDefault() { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); @@ -197,6 +196,7 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { public void testCreatePlaceholderWithMatchUrl_IdentifierCopied_NotPreExisting() { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); myDaoConfig.setAllowInlineMatchUrlReferences(true); + // FIXME: DM - 2021-03-04 - Whaaat?! This is configurable. Awesome. What's the default? myDaoConfig.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(true); Observation obsToCreate = new Observation(); From 9a7bd0e775135f8a0a8470c401d5255de8ba701d Mon Sep 17 00:00:00 2001 From: Diederik Muylwyk Date: Thu, 4 Mar 2021 16:55:16 -0500 Subject: [PATCH 03/13] Add failing tests. --- .../java/ca/uhn/fhir/util/HapiExtensions.java | 6 + .../fhir/jpa/term/BaseTermReadSvcImpl.java | 1 + ...irResourceDaoCreatePlaceholdersR4Test.java | 148 +++++++++++++++++- 3 files changed, 153 insertions(+), 2 deletions(-) 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..cd9028542ef 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,6 +109,12 @@ public class HapiExtensions { /** * URL for boolean extension added to all placeholder resources */ + public static final String EXT_RESOURCE_PLACEHOLDER = "http://hapifhir.io/fhir/StructureDefinition/resource-placeholder"; + + /** + * URL for boolean extension added to all placeholder resources + */ + // FIXME: DM 2021-03-04 - This should probably be removed, and replaced with EXT_RESOURCE_PLACEHOLDER above public static final String EXT_RESOURCE_META_PLACEHOLDER = "http://hapifhir.io/fhir/StructureDefinition/resource-meta-placeholder"; 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 a5c579e096c..7c70a2d6129 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 @@ -2124,6 +2124,7 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { private boolean isPlaceholder(MetadataResource theResource) { // FIXME: DM 2021-03-04 - We should probably check if this extension exists and the value is true. + // FIXME: DM 2021-03-04 - Don't like that getExtensionUrl returns null instead of empty collection. return theResource.getMeta().getExtensionByUrl(HapiExtensions.EXT_RESOURCE_META_PLACEHOLDER) != null; } 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 3b296bb9e68..cd7016121b7 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,9 +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.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; @@ -26,6 +29,9 @@ 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.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"}) @@ -171,7 +177,96 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { assertEquals("Patient/999999999999999", outcome.getResources(0,1).get(0).getIdElement().toUnqualifiedVersionless().getValue()); } - // FIXME: DM 2021-03-04 - These ARE the tests you're looking for. + // FIXME: DM 2021-03-04 - This test fails; extension isn't being created correctly + @Test + 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); + } + + // FIXME: DM 2021-03-04 - This test fails; extension isn't being created correctly; probably shouldn't be in meta + @Test + public void testCreatePlaceholderMetaExtension_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.getMeta().getExtensionByUrl(HapiExtensions.EXT_RESOURCE_META_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.getMeta().getExtensionByUrl(HapiExtensions.EXT_RESOURCE_META_PLACEHOLDER); + assertNull(extension); + } + @Test public void testCreatePlaceholderWithMatchUrl_IdentifierNotCopiedByDefault() { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); @@ -191,12 +286,61 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { assertEquals(0, patient.getIdentifier().size()); } + // FIXME: DM 2021-03-04 - This test fails; placeholder identifier isn't populated by default + @Test + public void testCreatePlaceholderWithMatchUrl_PopulateIdentifierSetToDefault_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_IdentifierCopied_NotPreExisting() { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); myDaoConfig.setAllowInlineMatchUrlReferences(true); - // FIXME: DM - 2021-03-04 - Whaaat?! This is configurable. Awesome. What's the default? myDaoConfig.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(true); Observation obsToCreate = new Observation(); From dfdcfa1365c398711dca0fd507dbce0866fe6ffe Mon Sep 17 00:00:00 2001 From: Diederik Muylwyk Date: Thu, 4 Mar 2021 17:01:39 -0500 Subject: [PATCH 04/13] Add failing tests. --- .../src/main/java/ca/uhn/fhir/util/HapiExtensions.java | 2 +- .../ca/uhn/fhir/jpa/dao/index/DaoResourceLinkResolver.java | 1 + .../jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java | 6 +++--- 3 files changed, 5 insertions(+), 4 deletions(-) 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 cd9028542ef..63121a0885d 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 @@ -114,7 +114,7 @@ public class HapiExtensions { /** * URL for boolean extension added to all placeholder resources */ - // FIXME: DM 2021-03-04 - This should probably be removed, and replaced with EXT_RESOURCE_PLACEHOLDER above + // FIXME: DM 2021-03-04 - This should probably be removed, and replaced with EXT_RESOURCE_PLACEHOLDER above. public static final String EXT_RESOURCE_META_PLACEHOLDER = "http://hapifhir.io/fhir/StructureDefinition/resource-meta-placeholder"; 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 f78746bbe6e..f453f3ce643 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 @@ -150,6 +150,7 @@ public class DaoResourceLinkResolver implements IResourceLinkResolver { return Optional.ofNullable(valueOf); } + // FIXME: DM 2021-03-04 - Should we throw an error if the identifier isn't available? Otherwise we get a placeholder with no identifier. private void tryToCopyIdentifierFromReferenceToTargetResource(IBaseReference theSourceReference, RuntimeResourceDefinition theTargetResourceDef, T theTargetResource) { boolean referenceHasIdentifier = theSourceReference.hasIdentifier(); if (referenceHasIdentifier) { 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 cd7016121b7..820f5afe950 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 @@ -177,7 +177,7 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { assertEquals("Patient/999999999999999", outcome.getResources(0,1).get(0).getIdElement().toUnqualifiedVersionless().getValue()); } - // FIXME: DM 2021-03-04 - This test fails; extension isn't being created correctly + // FIXME: DM 2021-03-04 - This test fails; extension isn't being created correctly. @Test public void testCreatePlaceholderExtension_WithUpdateToTarget() { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); @@ -222,7 +222,7 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { assertNull(extension); } - // FIXME: DM 2021-03-04 - This test fails; extension isn't being created correctly; probably shouldn't be in meta + // FIXME: DM 2021-03-04 - This test fails; extension isn't being created correctly; probably shouldn't be in meta. @Test public void testCreatePlaceholderMetaExtension_WithUpdateToTarget() { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); @@ -286,7 +286,7 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { assertEquals(0, patient.getIdentifier().size()); } - // FIXME: DM 2021-03-04 - This test fails; placeholder identifier isn't populated by default + // FIXME: DM 2021-03-04 - This test fails; placeholder identifier isn't populated by default. @Test public void testCreatePlaceholderWithMatchUrl_PopulateIdentifierSetToDefault_WithUpdateToTarget() { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); From 1d1d9a2f7c265ee7355114dca3a4bd99b2f88f7c Mon Sep 17 00:00:00 2001 From: Diederik Muylwyk Date: Thu, 4 Mar 2021 18:02:06 -0500 Subject: [PATCH 05/13] Fix typo in IGenericClient javadocs --- .../main/java/ca/uhn/fhir/rest/client/api/IGenericClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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(); From e3ba4bd1b61812867bbd872b84f2560a73a930f4 Mon Sep 17 00:00:00 2001 From: Diederik Muylwyk Date: Thu, 4 Mar 2021 18:54:04 -0500 Subject: [PATCH 06/13] Set default for Populate Identifier in Auto-Created Placeholder Reference Targets to true; update javadocs accordingly --- .../java/ca/uhn/fhir/util/HapiExtensions.java | 3 +- .../ca/uhn/fhir/jpa/api/config/DaoConfig.java | 78 ++++++++++++++++++- ...irResourceDaoCreatePlaceholdersR4Test.java | 10 +-- 3 files changed, 81 insertions(+), 10 deletions(-) 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 63121a0885d..3bd696d19f0 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 @@ -113,8 +113,9 @@ public class HapiExtensions { /** * URL for boolean extension added to all placeholder resources + * + * @deprecated Deprecated in 5.4.0 in favour of {@link HapiExtensions#EXT_RESOURCE_PLACEHOLDER} */ - // FIXME: DM 2021-03-04 - This should probably be removed, and replaced with EXT_RESOURCE_PLACEHOLDER above. public static final String EXT_RESOURCE_META_PLACEHOLDER = "http://hapifhir.io/fhir/StructureDefinition/resource-meta-placeholder"; 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..c791f38fde5 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 @@ -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/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 820f5afe950..ae6a807fac1 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 @@ -268,9 +268,10 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { } @Test - public void testCreatePlaceholderWithMatchUrl_IdentifierNotCopiedByDefault() { + public void testCreatePlaceholderWithMatchUrl_IdentifierNotCopied() { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); myDaoConfig.setAllowInlineMatchUrlReferences(true); + myDaoConfig.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(false); Observation obsToCreate = new Observation(); obsToCreate.setStatus(ObservationStatus.FINAL); @@ -288,7 +289,7 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { // FIXME: DM 2021-03-04 - This test fails; placeholder identifier isn't populated by default. @Test - public void testCreatePlaceholderWithMatchUrl_PopulateIdentifierSetToDefault_WithUpdateToTarget() { + public void testCreatePlaceholderWithMatchUrl_IdentifierCopiedByDefault_WithUpdateToTarget() { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); myDaoConfig.setAllowInlineMatchUrlReferences(true); @@ -338,10 +339,9 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { } @Test - public void testCreatePlaceholderWithMatchUrl_IdentifierCopied_NotPreExisting() { + public void testCreatePlaceholderWithMatchUrl_IdentifierCopiedByDefault_NotPreExisting() { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); myDaoConfig.setAllowInlineMatchUrlReferences(true); - myDaoConfig.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(true); Observation obsToCreate = new Observation(); obsToCreate.setStatus(ObservationStatus.FINAL); @@ -363,7 +363,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(); @@ -382,7 +381,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"); From 96a21515429048b305cda2677e50d572bad6a8b7 Mon Sep 17 00:00:00 2001 From: Diederik Muylwyk Date: Thu, 4 Mar 2021 19:19:34 -0500 Subject: [PATCH 07/13] Remove unnecessary FIXMEs. --- .../dao/index/SearchParamWithInlineReferencesExtractor.java | 1 - .../main/java/ca/uhn/fhir/jpa/term/BaseTermReadSvcImpl.java | 1 - .../jpa/dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java | 1 - .../src/test/java/ca/uhn/fhir/jpa/packages/NpmR4Test.java | 3 --- 4 files changed, 6 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/SearchParamWithInlineReferencesExtractor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/SearchParamWithInlineReferencesExtractor.java index bdbf6ec991d..0a059bbc1ab 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/SearchParamWithInlineReferencesExtractor.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/index/SearchParamWithInlineReferencesExtractor.java @@ -214,7 +214,6 @@ public class SearchParamWithInlineReferencesExtractor { } - // FIXME: DM 2021-03-04 - This is where inline match URLs are handled. /** * 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. 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 7c70a2d6129..a5c579e096c 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 @@ -2124,7 +2124,6 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { private boolean isPlaceholder(MetadataResource theResource) { // FIXME: DM 2021-03-04 - We should probably check if this extension exists and the value is true. - // FIXME: DM 2021-03-04 - Don't like that getExtensionUrl returns null instead of empty collection. return theResource.getMeta().getExtensionByUrl(HapiExtensions.EXT_RESOURCE_META_PLACEHOLDER) != null; } 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 ae6a807fac1..5392aad2bd4 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 @@ -287,7 +287,6 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { assertEquals(0, patient.getIdentifier().size()); } - // FIXME: DM 2021-03-04 - This test fails; placeholder identifier isn't populated by default. @Test public void testCreatePlaceholderWithMatchUrl_IdentifierCopiedByDefault_WithUpdateToTarget() { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/NpmR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/NpmR4Test.java index 1f864c9471e..eeaa4eac020 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/NpmR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/packages/NpmR4Test.java @@ -357,9 +357,6 @@ public class NpmR4Test extends BaseJpaR4Test { } } - // FIXME: DM 2021-03-04 - This test is meant to validate that placeholder ValueSets are not created. - // FIXME: DM 2021-03-04 - This test or a new test should probably validate that placeholder ConceptMaps are also not created. - // FIXME: DM 2021-03-04 - This test does not validate that placeholder resources are created with a EXT_RESOURCE_META_PLACEHOLDER extension. // Reproduces https://github.com/hapifhir/hapi-fhir/issues/2332 @Test public void testInstallR4Package_AutoCreatePlaceholder() throws Exception { From beab3522b9df8cca167836b5994ffd431d167102 Mon Sep 17 00:00:00 2001 From: Diederik Muylwyk Date: Thu, 4 Mar 2021 19:33:00 -0500 Subject: [PATCH 08/13] Enhance isPlaceholder to check value of placeholder extension. --- .../jpa/dao/index/DaoResourceLinkResolver.java | 3 +-- .../ca/uhn/fhir/jpa/term/BaseTermReadSvcImpl.java | 14 ++++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) 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 f453f3ce643..0faaa9eb160 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 @@ -110,7 +110,6 @@ public class DaoResourceLinkResolver implements IResourceLinkResolver { return resolvedResource; } - // FIXME: DM 2021-03-04 - This is where placeholder reference targets are handled. /** * @param theIdToAssignToPlaceholder If specified, the placeholder resource created will be given a specific ID */ @@ -150,7 +149,7 @@ public class DaoResourceLinkResolver implements IResourceLinkResolver { return Optional.ofNullable(valueOf); } - // FIXME: DM 2021-03-04 - Should we throw an error if the identifier isn't available? Otherwise we get a placeholder with no identifier. + // FIXME: DM 2021-03-04 - Should we issue a warning if the identifier isn't available? private void tryToCopyIdentifierFromReferenceToTargetResource(IBaseReference theSourceReference, RuntimeResourceDefinition theTargetResourceDef, T theTargetResource) { boolean referenceHasIdentifier = theSourceReference.hasIdentifier(); if (referenceHasIdentifier) { 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 a5c579e096c..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,9 +2124,13 @@ public abstract class BaseTermReadSvcImpl implements ITermReadSvc { } } - private boolean isPlaceholder(MetadataResource theResource) { - // FIXME: DM 2021-03-04 - We should probably check if this extension exists and the value is true. - 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 From bd85c0c36233469bd591d491bd8fac965ac4ef27 Mon Sep 17 00:00:00 2001 From: Diederik Muylwyk Date: Thu, 4 Mar 2021 19:38:09 -0500 Subject: [PATCH 09/13] Remove unnecessary failing test. --- ...irResourceDaoCreatePlaceholdersR4Test.java | 47 +------------------ 1 file changed, 1 insertion(+), 46 deletions(-) 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 5392aad2bd4..a81f245578a 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 @@ -201,7 +201,7 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { 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); + assertNotNull(extension); // FIXME: DM 2021-03-04 - This assertion fails for now. assertTrue(extension.hasValue()); assertTrue(((BooleanType) extension.getValue()).booleanValue()); @@ -222,51 +222,6 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { assertNull(extension); } - // FIXME: DM 2021-03-04 - This test fails; extension isn't being created correctly; probably shouldn't be in meta. - @Test - public void testCreatePlaceholderMetaExtension_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.getMeta().getExtensionByUrl(HapiExtensions.EXT_RESOURCE_META_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.getMeta().getExtensionByUrl(HapiExtensions.EXT_RESOURCE_META_PLACEHOLDER); - assertNull(extension); - } - @Test public void testCreatePlaceholderWithMatchUrl_IdentifierNotCopied() { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); From 35645999d7dac4f30ac0dc146daa8e7686211d78 Mon Sep 17 00:00:00 2001 From: Diederik Muylwyk Date: Thu, 4 Mar 2021 19:55:39 -0500 Subject: [PATCH 10/13] Add changelog entries. --- .../2332-placeholder-valuset-skips-terminology-storage.yaml | 3 +-- ...ce-targets-need-to-be-resolved-placeholder-extension.yaml | 5 +++++ ...gets-need-to-be-resolved-placeholder-identifier-true.yaml | 5 +++++ 3 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 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 create mode 100644 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 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`." From a195eebdecc1a97c85242e3ace179eef64eedbdc Mon Sep 17 00:00:00 2001 From: Diederik Muylwyk Date: Fri, 5 Mar 2021 12:09:17 -0500 Subject: [PATCH 11/13] Placeholder extension moved to resource body from meta; tests passing. --- .../main/java/ca/uhn/fhir/jpa/api/config/DaoConfig.java | 4 ++-- .../uhn/fhir/jpa/dao/index/DaoResourceLinkResolver.java | 9 +++------ .../dao/r4/FhirResourceDaoCreatePlaceholdersR4Test.java | 3 +-- 3 files changed, 6 insertions(+), 10 deletions(-) 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 c791f38fde5..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 @@ -1071,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() { @@ -1095,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) { 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 0faaa9eb160..2c0d06dfebc 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,11 +122,9 @@ public class DaoResourceLinkResolver implements IResourceLinkResolver { @SuppressWarnings("unchecked") T newResource = (T) missingResourceDef.newInstance(); - // FIXME: DM 2021-03-04 - This is where the placeholder extension is added; should be in resource body instead of meta? - 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/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 a81f245578a..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 @@ -177,7 +177,6 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { assertEquals("Patient/999999999999999", outcome.getResources(0,1).get(0).getIdElement().toUnqualifiedVersionless().getValue()); } - // FIXME: DM 2021-03-04 - This test fails; extension isn't being created correctly. @Test public void testCreatePlaceholderExtension_WithUpdateToTarget() { myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); @@ -201,7 +200,7 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test { 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); // FIXME: DM 2021-03-04 - This assertion fails for now. + assertNotNull(extension); assertTrue(extension.hasValue()); assertTrue(((BooleanType) extension.getValue()).booleanValue()); From 2945cd9f748a99a187cb0022619d286479234d1e Mon Sep 17 00:00:00 2001 From: Diederik Muylwyk Date: Fri, 5 Mar 2021 12:33:23 -0500 Subject: [PATCH 12/13] Remove FIXME --- .../java/ca/uhn/fhir/jpa/dao/index/DaoResourceLinkResolver.java | 1 - 1 file changed, 1 deletion(-) 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 2c0d06dfebc..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 @@ -146,7 +146,6 @@ public class DaoResourceLinkResolver implements IResourceLinkResolver { return Optional.ofNullable(valueOf); } - // FIXME: DM 2021-03-04 - Should we issue a warning if the identifier isn't available? private void tryToCopyIdentifierFromReferenceToTargetResource(IBaseReference theSourceReference, RuntimeResourceDefinition theTargetResourceDef, T theTargetResource) { boolean referenceHasIdentifier = theSourceReference.hasIdentifier(); if (referenceHasIdentifier) { From 214c7eb45e78d7378f93c36234c87e1672757370 Mon Sep 17 00:00:00 2001 From: Diederik Muylwyk Date: Fri, 5 Mar 2021 12:59:26 -0500 Subject: [PATCH 13/13] Remove unnecessary field. --- .../src/main/java/ca/uhn/fhir/util/HapiExtensions.java | 8 -------- 1 file changed, 8 deletions(-) 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 3bd696d19f0..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 @@ -111,14 +111,6 @@ public class HapiExtensions { */ public static final String EXT_RESOURCE_PLACEHOLDER = "http://hapifhir.io/fhir/StructureDefinition/resource-placeholder"; - /** - * URL for boolean extension added to all placeholder resources - * - * @deprecated Deprecated in 5.4.0 in favour of {@link HapiExtensions#EXT_RESOURCE_PLACEHOLDER} - */ - public static final String EXT_RESOURCE_META_PLACEHOLDER = "http://hapifhir.io/fhir/StructureDefinition/resource-meta-placeholder"; - - /** * Non instantiable */