Fix bundle auto-create by checking for IBaseReference.hasIdentifier() before validating URL-based references (#4731)

* First commit:  New tests, logs, and comments.

* Add hacky fix to run pipelines on.   Add another unit test.

* Disable baseUrl checking if hasIdentifier() is true.

* Add changelog.  Tweak unit tests.

* Remove junk.

* Add more assertions to unit tests.  Make new production fix code clearer.

* Add more clarity to unit tests.

* Another unit test fix.
This commit is contained in:
Luke deGruchy 2023-04-14 13:55:31 -04:00 committed by GitHub
parent 869b6c306a
commit 35bd1c400a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 245 additions and 4 deletions

View File

@ -0,0 +1,5 @@
---
type: fix
issue: 4736
title: "When enabling both auto_create_placeholder_reference_targets and allow_inline_match_url_references, POSTing a bundle with a conditional create URL will fail with HAPI-0507.
This has now been fixed."

View File

@ -518,7 +518,8 @@ public class SearchParamExtractorService {
return; return;
} }
String baseUrl = nextId.getBaseUrl(); final boolean hasNoIdentifier = ! nextReference.hasIdentifier();
final String baseUrl = hasNoIdentifier ? nextId.getBaseUrl() : null;
String typeString = nextId.getResourceType(); String typeString = nextId.getResourceType();
if (isBlank(typeString)) { if (isBlank(typeString)) {
String msg = "Invalid resource reference found at path[" + path + "] - Does not contain resource type - " + nextId.getValue(); String msg = "Invalid resource reference found at path[" + path + "] - Does not contain resource type - " + nextId.getValue();

View File

@ -25,8 +25,10 @@ import org.hl7.fhir.r4.model.IdType;
import org.hl7.fhir.r4.model.Identifier; import org.hl7.fhir.r4.model.Identifier;
import org.hl7.fhir.r4.model.Observation; import org.hl7.fhir.r4.model.Observation;
import org.hl7.fhir.r4.model.Observation.ObservationStatus; import org.hl7.fhir.r4.model.Observation.ObservationStatus;
import org.hl7.fhir.r4.model.Organization;
import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.r4.model.Patient;
import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.Reference;
import org.hl7.fhir.r4.model.ResourceType;
import org.hl7.fhir.r4.model.Task; import org.hl7.fhir.r4.model.Task;
import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Assertions;
@ -52,6 +54,8 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test {
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoCreatePlaceholdersR4Test.class); private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoCreatePlaceholdersR4Test.class);
private static final String TEST_IDENTIFIER_SYSTEM = "http://some-system.com";
@AfterEach @AfterEach
public final void afterResetDao() { public final void afterResetDao() {
myStorageSettings.setAutoCreatePlaceholderReferenceTargets(new JpaStorageSettings().isAutoCreatePlaceholderReferenceTargets()); myStorageSettings.setAutoCreatePlaceholderReferenceTargets(new JpaStorageSettings().isAutoCreatePlaceholderReferenceTargets());
@ -399,6 +403,75 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test {
} }
@Test
public void testCreatePlaceholderWithMatchingInlineAndSubjectReferenceIdentifiersCreatesOnlyOne_withConditionalUrl() {
myStorageSettings.setAutoCreatePlaceholderReferenceTargets(true);
myStorageSettings.setAllowInlineMatchUrlReferences(true);
myStorageSettings.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(true);
final String system = "http://bar";
final String value = "http://something.something/b";
/*
* Create an Observation that references a Patient
* Reference is populated with inline match URL and includes identifier which differs from the inlined identifier
*/
final Observation obsToCreate = new Observation();
obsToCreate.setStatus(ObservationStatus.FINAL);
obsToCreate.getSubject().setReference(String.format("%s?identifier=%s|%s", ResourceType.Patient.name(), system, value));
obsToCreate.getSubject().getIdentifier().setSystem(system).setValue(value);
final IIdType obsId = myObservationDao.create(obsToCreate, mySrd).getId();
// Read the Observation
final Observation createdObs = myObservationDao.read(obsId, new SystemRequestDetails());
//Read the Placeholder Patient
final Patient placeholderPat = myPatientDao.read(new IdType(createdObs.getSubject().getReference()), new SystemRequestDetails());
//Ensure the Obs has the right placeholder ID.
final IIdType placeholderPatId = placeholderPat.getIdElement();
assertEquals(createdObs.getSubject().getReference(), placeholderPatId.toUnqualifiedVersionless().getValueAsString());
assertTrue(placeholderPatId.isIdPartValidLong());
assertTrue(placeholderPatId.isVersionIdPartValidLong());
/*
* Should have a single identifier populated.
*/
assertEquals(1, placeholderPat.getIdentifier().size());
final List<Identifier> identifiers = placeholderPat.getIdentifier();
Identifier identifier = identifiers.get(0);
assertThat(identifier.getSystem(), is(equalTo(system)));
assertThat(identifier.getValue(), is(equalTo(value)));
}
@Test
void testAutoCreatePlaceholderReferencesAndInlineMatchWithUrlValues_conditionalCreateOrganizationAndOrganization() {
// setup
myStorageSettings.setAllowInlineMatchUrlReferences(true);
myStorageSettings.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(true);
myStorageSettings.setAutoCreatePlaceholderReferenceTargets(true);
final String identifierValue = "http://some-url-value/Organization/ID2";
// Test fails with Organization and Organization
final Patient patient = new Patient();
final Reference reference = new Reference()
.setReference(String.format("%s?identifier=%s|%s", ResourceType.Organization.name(), TEST_IDENTIFIER_SYSTEM, identifierValue))
.setIdentifier(new Identifier().setValue(identifierValue).setSystem(TEST_IDENTIFIER_SYSTEM));
patient.setManagingOrganization(reference);
myPatientDao.create(patient, mySrd);
//Read the Placeholder Observation
final IBundleProvider organizationSearch = myOrganizationDao.search(new SearchParameterMap(Organization.SP_IDENTIFIER, new TokenParam(TEST_IDENTIFIER_SYSTEM, identifierValue)), new SystemRequestDetails());
final List<IBaseResource> allResources = organizationSearch.getAllResources();
assertEquals(1, allResources.size());
assertEquals(ResourceType.Organization.name(), allResources.get(0).getIdElement().getResourceType());
assertTrue(allResources.get(0).getIdElement().toUnqualifiedVersionless().toString().startsWith(ResourceType.Organization.name()));
assertEquals(1, organizationSearch.getAllResourceIds().size());
}
// Case 4: // Case 4:
// //
// IF the inline match URL does include an identifier // IF the inline match URL does include an identifier
@ -412,6 +485,9 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test {
myStorageSettings.setAllowInlineMatchUrlReferences(true); myStorageSettings.setAllowInlineMatchUrlReferences(true);
myStorageSettings.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(true); myStorageSettings.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(true);
final String system = "http://bar";
final String value = "321";
/* /*
* Create an Observation that references a Patient * Create an Observation that references a Patient
* Reference is populated with inline match URL and includes identifier which differs from the inlined identifier * Reference is populated with inline match URL and includes identifier which differs from the inlined identifier
@ -419,7 +495,7 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test {
Observation obsToCreate = new Observation(); Observation obsToCreate = new Observation();
obsToCreate.setStatus(ObservationStatus.FINAL); obsToCreate.setStatus(ObservationStatus.FINAL);
obsToCreate.getSubject().setReference("Patient?identifier=http://foo|123"); obsToCreate.getSubject().setReference("Patient?identifier=http://foo|123");
obsToCreate.getSubject().getIdentifier().setSystem("http://bar").setValue("321"); obsToCreate.getSubject().getIdentifier().setSystem(system).setValue(value);
IIdType obsId = myObservationDao.create(obsToCreate, mySrd).getId(); IIdType obsId = myObservationDao.create(obsToCreate, mySrd).getId();
// Read the Observation // Read the Observation
@ -445,8 +521,8 @@ public class FhirResourceDaoCreatePlaceholdersR4Test extends BaseJpaR4Test {
assertThat(identifiers.get(1).getValue(), is(equalTo("123"))); assertThat(identifiers.get(1).getValue(), is(equalTo("123")));
//subject identifier //subject identifier
assertThat(identifiers.get(0).getSystem(), is(equalTo("http://bar"))); assertThat(identifiers.get(0).getSystem(), is(equalTo(system)));
assertThat(identifiers.get(0).getValue(), is(equalTo("321"))); assertThat(identifiers.get(0).getValue(), is(equalTo(value)));
// Conditionally update a Patient with the same identifier // Conditionally update a Patient with the same identifier

View File

@ -40,6 +40,7 @@ import ca.uhn.fhir.util.ClasspathUtil;
import org.apache.commons.io.IOUtils; import org.apache.commons.io.IOUtils;
import org.hamcrest.Matchers; import org.hamcrest.Matchers;
import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IAnyResource;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.AllergyIntolerance; import org.hl7.fhir.r4.model.AllergyIntolerance;
import org.hl7.fhir.r4.model.Appointment; import org.hl7.fhir.r4.model.Appointment;
@ -75,6 +76,7 @@ import org.hl7.fhir.r4.model.Practitioner;
import org.hl7.fhir.r4.model.Quantity; import org.hl7.fhir.r4.model.Quantity;
import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.Reference;
import org.hl7.fhir.r4.model.Resource; import org.hl7.fhir.r4.model.Resource;
import org.hl7.fhir.r4.model.ResourceType;
import org.hl7.fhir.r4.model.Task; import org.hl7.fhir.r4.model.Task;
import org.hl7.fhir.r4.model.ValueSet; import org.hl7.fhir.r4.model.ValueSet;
import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.AfterEach;
@ -131,6 +133,7 @@ import static org.mockito.Mockito.when;
public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest { public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest {
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirSystemDaoR4Test.class); private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirSystemDaoR4Test.class);
private static final String TEST_IDENTIFIER_SYSTEM = "http://some-system.com";
@AfterEach @AfterEach
public void after() { public void after() {
@ -4391,6 +4394,162 @@ public class FhirSystemDaoR4Test extends BaseJpaR4SystemTest {
}); });
} }
@Test
void testAutoCreatePlaceholderReferencesAndInlineMatchWithUrlValues_simpleIdentifier() {
// setup
myStorageSettings.setAllowInlineMatchUrlReferences(true);
myStorageSettings.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(true);
myStorageSettings.setAutoCreatePlaceholderReferenceTargets(true);
// Test Passes when identifier.value = ID2
final String identifierValue = "ID2";
final Patient patient = new Patient();
final Reference reference = new Reference()
.setReference(String.format("%s?identifier=%s|%s", ResourceType.Organization.name(), TEST_IDENTIFIER_SYSTEM, identifierValue))
.setIdentifier(new Identifier().setValue(identifierValue).setSystem(TEST_IDENTIFIER_SYSTEM));
patient.setManagingOrganization(reference);
final Bundle bundle = new Bundle();
bundle.setType(BundleType.TRANSACTION);
bundle.addEntry().setResource(patient)
.getRequest()
.setMethod(HTTPVerb.POST)
.setUrl(ResourceType.Patient.name());
// execute
final Bundle actual = mySystemDao.transaction(mySrd, bundle);
// validate
assertEquals("201 Created", actual.getEntry().get(0).getResponse().getStatus());
final IBundleProvider organizationSearch = myOrganizationDao.search(new SearchParameterMap(Organization.SP_IDENTIFIER, new TokenParam(TEST_IDENTIFIER_SYSTEM, identifierValue)), new SystemRequestDetails());
final List<IBaseResource> allResources = organizationSearch.getAllResources();
assertEquals(1, allResources.size());
assertEquals(ResourceType.Organization.name(), allResources.get(0).getIdElement().getResourceType());
assertTrue(allResources.get(0).getIdElement().toUnqualifiedVersionless().toString().startsWith(ResourceType.Organization.name()));
}
@Test
void testAutoCreatePlaceholderReferencesAndInlineMatchWithUrlValues_simpleUrlWithIdentifier() {
// setup
myStorageSettings.setAllowInlineMatchUrlReferences(true);
myStorageSettings.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(true);
myStorageSettings.setAutoCreatePlaceholderReferenceTargets(true);
final String identifierValue = "http://some-url-value";
final Patient patient = new Patient();
final Reference reference = new Reference()
.setReference(String.format("%s?identifier=%s|%s", ResourceType.Organization.name(), TEST_IDENTIFIER_SYSTEM, identifierValue))
.setIdentifier(new Identifier().setValue(identifierValue).setSystem(TEST_IDENTIFIER_SYSTEM));
patient.setManagingOrganization(reference);
final Bundle bundle = new Bundle();
bundle.setType(BundleType.TRANSACTION);
bundle.addEntry().setResource(patient)
.getRequest()
.setMethod(HTTPVerb.POST)
.setUrl(ResourceType.Patient.name());
// execute
final Bundle actual = mySystemDao.transaction(mySrd, bundle);
// validate
assertEquals("201 Created", actual.getEntry().get(0).getResponse().getStatus());
final IBundleProvider organizationSearch = myOrganizationDao.search(new SearchParameterMap(Organization.SP_IDENTIFIER, new TokenParam(TEST_IDENTIFIER_SYSTEM, identifierValue)), new SystemRequestDetails());
final List<IBaseResource> allResources = organizationSearch.getAllResources();
assertEquals(1, allResources.size());
assertEquals(ResourceType.Organization.name(), allResources.get(0).getIdElement().getResourceType());
assertTrue(allResources.get(0).getIdElement().toUnqualifiedVersionless().toString().startsWith(ResourceType.Organization.name()));
}
@Test
void testAutoCreatePlaceholderReferencesAndInlineMatchWithUrlValues_urlOrganizationAndObservation() {
// setup
myStorageSettings.setAllowInlineMatchUrlReferences(true);
myStorageSettings.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(true);
myStorageSettings.setAutoCreatePlaceholderReferenceTargets(true);
final String identifierValue = "http://some-url-value/Observation/ID2";
final Patient patient = new Patient();
final Reference reference = new Reference()
.setReference(String.format("%s?identifier=%s|%s", ResourceType.Organization.name(), TEST_IDENTIFIER_SYSTEM, identifierValue))
.setIdentifier(new Identifier().setValue(identifierValue).setSystem(TEST_IDENTIFIER_SYSTEM));
patient.setManagingOrganization(reference);
final Bundle bundle = new Bundle();
bundle.setType(BundleType.TRANSACTION);
bundle.addEntry().setResource(patient)
.getRequest()
.setMethod(HTTPVerb.POST)
.setUrl(ResourceType.Patient.name());
// execute
final Bundle actual = mySystemDao.transaction(mySrd, bundle);
// validate
assertEquals("201 Created", actual.getEntry().get(0).getResponse().getStatus());
final IBundleProvider organizationSearch = myOrganizationDao.search(new SearchParameterMap(Organization.SP_IDENTIFIER, new TokenParam(TEST_IDENTIFIER_SYSTEM, identifierValue)), new SystemRequestDetails());
final List<IBaseResource> allResources = organizationSearch.getAllResources();
assertEquals(1, allResources.size());
assertEquals(ResourceType.Organization.name(), allResources.get(0).getIdElement().getResourceType());
assertTrue(allResources.get(0).getIdElement().toUnqualifiedVersionless().toString().startsWith(ResourceType.Organization.name()));
}
@Test
void testAutoCreatePlaceholderReferencesAndInlineMatchWithUrlValues_OrganizationAndOrganization() {
// setup
myStorageSettings.setAllowInlineMatchUrlReferences(true);
myStorageSettings.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(true);
myStorageSettings.setAutoCreatePlaceholderReferenceTargets(true);
// Test fails with Organization and Organization
final String identifierValue = "http://some-url-value/Organization/ID2";
final Patient patient = new Patient();
final Reference reference = new Reference()
.setReference(String.format("%s?identifier=%s|%s", ResourceType.Organization.name(), TEST_IDENTIFIER_SYSTEM, identifierValue))
.setIdentifier(new Identifier().setValue(identifierValue).setSystem(TEST_IDENTIFIER_SYSTEM));
patient.setManagingOrganization(reference);
final Bundle bundle = new Bundle();
bundle.setType(BundleType.TRANSACTION);
bundle.addEntry().setResource(patient)
.getRequest()
.setMethod(HTTPVerb.POST)
.setUrl(ResourceType.Patient.name());
// execute
final Bundle actual = mySystemDao.transaction(mySrd, bundle);
// validate
assertEquals("201 Created", actual.getEntry().get(0).getResponse().getStatus());
final IBundleProvider organizationSearch = myOrganizationDao.search(new SearchParameterMap(Organization.SP_IDENTIFIER, new TokenParam(TEST_IDENTIFIER_SYSTEM, identifierValue)), new SystemRequestDetails());
final List<IBaseResource> allResources = organizationSearch.getAllResources();
assertEquals(1, allResources.size());
assertEquals(ResourceType.Organization.name(), allResources.get(0).getIdElement().getResourceType());
assertTrue(allResources.get(0).getIdElement().toUnqualifiedVersionless().toString().startsWith(ResourceType.Organization.name()));
}
@Test
public void testCreatePlaceholderWithMatchingInlineAndSubjectReferenceIdentifiersCreatesOnlyOne_withConditionalUrl() {
// setup
myStorageSettings.setAutoCreatePlaceholderReferenceTargets(true);
myStorageSettings.setAllowInlineMatchUrlReferences(true);
myStorageSettings.setPopulateIdentifierInAutoCreatedPlaceholderReferenceTargets(true);
/*
* Create an Observation that references a Patient
* Reference is populated with inline match URL and includes identifier which differs from the inlined identifier
*/
final Observation obsToCreate = new Observation();
obsToCreate.setStatus(ObservationStatus.FINAL);
String identifierValue = "http://something.something/b";
obsToCreate.getSubject()
.setReference(String.format("%s?identifier=%s|%s", ResourceType.Patient.name(), TEST_IDENTIFIER_SYSTEM, identifierValue));
obsToCreate.getSubject().getIdentifier().setSystem("http://bar").setValue("http://something.something/b");
final Bundle bundle = new Bundle();
bundle.setType(BundleType.TRANSACTION);
bundle.addEntry().setResource(obsToCreate)
.getRequest()
.setMethod(HTTPVerb.POST)
.setUrl(ResourceType.Observation.name());
// execute
final Bundle actual = mySystemDao.transaction(mySrd, bundle);
// validate
assertEquals("201 Created", actual.getEntry().get(0).getResponse().getStatus());
final IBundleProvider patientSearch = myPatientDao.search(new SearchParameterMap(Organization.SP_IDENTIFIER, new TokenParam(TEST_IDENTIFIER_SYSTEM, identifierValue)), new SystemRequestDetails());
final List<IBaseResource> allResources = patientSearch.getAllResources();
assertEquals(1, allResources.size());
assertEquals(ResourceType.Patient.name(), allResources.get(0).getIdElement().getResourceType());
assertTrue(allResources.get(0).getIdElement().toUnqualifiedVersionless().toString().startsWith(ResourceType.Patient.name()));
}
/** /**
* Per a message on the mailing list * Per a message on the mailing list