From 2bc1950bc19aa9e7faba6d4e97fdedd0c3b01390 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Fri, 29 Apr 2016 09:21:12 -0400 Subject: [PATCH] JPA server transactions sometimes created an incorrect resource reference if a resource being saved contained references that had a display value but not an actual reference. Thanks to David Hay for reporting! --- .../jpa/dao/FhirResourceDaoPatientDstu2.java | 4 + .../uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java | 3 + .../jpa/dao/dstu3/FhirSystemDaoDstu3.java | 3 + .../fhir/jpa/dao/dstu2/BaseJpaDstu2Test.java | 5 +- .../jpa/dao/dstu2/FhirSystemDaoDstu2Test.java | 278 +++++++++++++----- .../fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java | 18 +- .../jpa/dao/dstu3/FhirSystemDaoDstu3Test.java | 60 ++++ src/changes/changes.xml | 5 + 8 files changed, 299 insertions(+), 77 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoPatientDstu2.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoPatientDstu2.java index c4bee106ea1..2c2a93a5682 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoPatientDstu2.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoPatientDstu2.java @@ -42,6 +42,10 @@ import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetai public class FhirResourceDaoPatientDstu2 extends FhirResourceDaoDstu2implements IFhirResourceDaoPatient { + public FhirResourceDaoPatientDstu2() { + super(); + } + private IBundleProvider doEverythingOperation(IIdType theId, IPrimitiveType theCount, DateRangeParam theLastUpdated, SortSpec theSort, StringAndListParam theContent, StringAndListParam theNarrative) { SearchParameterMap paramMap = new SearchParameterMap(); if (theCount != null) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java index 95fdb80884f..c3bb1fa05c9 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/FhirSystemDaoDstu2.java @@ -460,6 +460,9 @@ public class FhirSystemDaoDstu2 extends BaseHapiFhirSystemDao { List allRefs = terser.getAllPopulatedChildElementsOfType(nextResource, BaseResourceReferenceDt.class); for (BaseResourceReferenceDt nextRef : allRefs) { IdDt nextId = nextRef.getReference(); + if (!nextId.hasIdPart()) { + continue; + } if (idSubstitutions.containsKey(nextId)) { IdDt newId = idSubstitutions.get(nextId); ourLog.info(" * Replacing resource ref {} with {}", nextId, newId); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java index 96122c233f5..8e00bafe664 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java @@ -470,6 +470,9 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { List allRefs = terser.getAllPopulatedChildElementsOfType(nextResource, IBaseReference.class); for (IBaseReference nextRef : allRefs) { IIdType nextId = nextRef.getReferenceElement(); + if (!nextId.hasIdPart()) { + continue; + } if (idSubstitutions.containsKey(nextId)) { IdType newId = idSubstitutions.get(nextId); ourLog.info(" * Replacing resource ref {} with {}", nextId, newId); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/BaseJpaDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/BaseJpaDstu2Test.java index 60ba5fef066..12b8a989d49 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/BaseJpaDstu2Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/BaseJpaDstu2Test.java @@ -18,7 +18,6 @@ import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.ApplicationContext; -import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.transaction.PlatformTransactionManager; @@ -41,6 +40,7 @@ import ca.uhn.fhir.jpa.provider.JpaSystemProviderDstu2; import ca.uhn.fhir.model.dstu2.composite.CodeableConceptDt; import ca.uhn.fhir.model.dstu2.composite.CodingDt; import ca.uhn.fhir.model.dstu2.composite.MetaDt; +import ca.uhn.fhir.model.dstu2.resource.Appointment; import ca.uhn.fhir.model.dstu2.resource.Bundle; import ca.uhn.fhir.model.dstu2.resource.ConceptMap; import ca.uhn.fhir.model.dstu2.resource.Device; @@ -87,6 +87,9 @@ public abstract class BaseJpaDstu2Test extends BaseJpaTest { @Qualifier("myConceptMapDaoDstu2") protected IFhirResourceDao myConceptMapDao; @Autowired + @Qualifier("myAppointmentDaoDstu2") + protected IFhirResourceDao myAppointmentDao; + @Autowired @Qualifier("myBundleDaoDstu2") protected IFhirResourceDao myBundleDao; @Autowired diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirSystemDaoDstu2Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirSystemDaoDstu2Test.java index aec72821899..d6336c5cf85 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirSystemDaoDstu2Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu2/FhirSystemDaoDstu2Test.java @@ -43,6 +43,7 @@ import ca.uhn.fhir.model.base.composite.BaseCodingDt; import ca.uhn.fhir.model.dstu2.composite.CodingDt; import ca.uhn.fhir.model.dstu2.composite.MetaDt; import ca.uhn.fhir.model.dstu2.composite.ResourceReferenceDt; +import ca.uhn.fhir.model.dstu2.resource.Appointment; import ca.uhn.fhir.model.dstu2.resource.Bundle; import ca.uhn.fhir.model.dstu2.resource.Bundle.Entry; import ca.uhn.fhir.model.dstu2.resource.Bundle.EntryRequest; @@ -66,19 +67,12 @@ import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException; import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetails; -import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.util.TestUtil; public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirSystemDaoDstu2Test.class); - @AfterClass - public static void afterClassClearContext() { - TestUtil.clearAllStaticFieldsForUnitTest(); - } - - @Test public void testReindexing() { Patient p = new Patient(); @@ -151,7 +145,7 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { assertEquals(Long.valueOf(2), entity.getIndexStatus()); } - + @Test public void testSystemMetaOperation() { @@ -240,7 +234,7 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { assertEquals("http://profile/2", profiles.get(0).getValue()); } - + @Test public void testTransactionBatchWithFailingRead() { String methodName = "testTransactionBatchWithFailingRead"; @@ -287,6 +281,7 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { assertThat(respEntry.getStatus(), startsWith("404")); } + @Test public void testTransactionCreateMatchUrlWithOneMatch() { @@ -329,7 +324,7 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { assertEquals("1", o.getId().getVersionIdPart()); } - + @Test public void testTransactionCreateMatchUrlWithTwoMatch() { String methodName = "testTransactionCreateMatchUrlWithTwoMatch"; @@ -929,6 +924,25 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { } + private Bundle testTransactionOrderingCreateBundle(String methodName, int pass, IdDt patientPlaceholderId) { + Bundle req = new Bundle(); + req.addEntry().getRequest().setMethod(HTTPVerbEnum.GET).setUrl("Patient?identifier=" + methodName); + + Observation obs = new Observation(); + obs.getSubject().setReference(patientPlaceholderId); + obs.addIdentifier().setValue(methodName); + obs.getCode().setText(methodName + pass); + req.addEntry().setResource(obs).getRequest().setMethod(HTTPVerbEnum.PUT).setUrl("Observation?identifier=" + methodName); + + Patient pat = new Patient(); + pat.addIdentifier().setValue(methodName); + pat.addName().addFamily(methodName + pass); + req.addEntry().setResource(pat).setFullUrl(patientPlaceholderId.getValue()).getRequest().setMethod(HTTPVerbEnum.POST).setUrl("Patient"); + + req.addEntry().getRequest().setMethod(HTTPVerbEnum.DELETE).setUrl("Patient?identifier=" + methodName); + return req; + } + private void testTransactionOrderingValidateResponse(int pass, Bundle resp) { ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(resp)); assertEquals(4, resp.getEntry().size()); @@ -957,25 +971,6 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { assertThat(respGetBundle.getLink("self").getUrl(), endsWith("/Patient?identifier=testTransactionOrdering")); } - private Bundle testTransactionOrderingCreateBundle(String methodName, int pass, IdDt patientPlaceholderId) { - Bundle req = new Bundle(); - req.addEntry().getRequest().setMethod(HTTPVerbEnum.GET).setUrl("Patient?identifier=" + methodName); - - Observation obs = new Observation(); - obs.getSubject().setReference(patientPlaceholderId); - obs.addIdentifier().setValue(methodName); - obs.getCode().setText(methodName + pass); - req.addEntry().setResource(obs).getRequest().setMethod(HTTPVerbEnum.PUT).setUrl("Observation?identifier=" + methodName); - - Patient pat = new Patient(); - pat.addIdentifier().setValue(methodName); - pat.addName().addFamily(methodName + pass); - req.addEntry().setResource(pat).setFullUrl(patientPlaceholderId.getValue()).getRequest().setMethod(HTTPVerbEnum.POST).setUrl("Patient"); - - req.addEntry().getRequest().setMethod(HTTPVerbEnum.DELETE).setUrl("Patient?identifier=" + methodName); - return req; - } - @Test public void testTransactionReadAndSearch() { String methodName = "testTransactionReadAndSearch"; @@ -1308,6 +1303,87 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { } + /** + * From a message from David Hay + */ + @Test + public void testTransactionWithAppointments() { + Patient p = new Patient(); + p.addName().addFamily("family"); + final IIdType id = myPatientDao.create(p, mySrd).getId().toUnqualifiedVersionless(); + + //@formatter:on + String input = "{\n" + + " \"resourceType\": \"Bundle\",\n" + + " \"type\": \"transaction\",\n" + + " \"entry\": [\n" + + " {\n" + + " \"resource\": {\n" + + " \"resourceType\": \"Appointment\",\n" + + " \"status\": \"pending\",\n" + + " \"type\": {\"text\": \"Cardiology\"},\n" + + " \"description\": \"Investigate Angina\",\n" + + " \"start\": \"2016-04-30T18:48:29+12:00\",\n" + + " \"end\": \"2016-04-30T19:03:29+12:00\",\n" + + " \"minutesDuration\": 15,\n" + + " \"participant\": [\n" + + " {\n" + + " \"actor\": {\"display\": \"Clarence cardiology clinic\"},\n" + + " \"status\": \"accepted\"\n" + + " },\n" + + " {\n" + + " \"actor\": {\"reference\": \"Patient/" + id.getIdPart() + "\"},\n" + + " \"status\": \"accepted\"\n" + + " }\n" + + " ],\n" + + " \"text\": {\n" + + " \"status\": \"generated\",\n" + + " \"div\": \"
Investigate Angina<\\/div>
Clarence cardiology clinic<\\/div><\\/div>\"\n" + + " }\n" + + " },\n" + + " \"request\": {\n" + + " \"method\": \"POST\",\n" + + " \"url\": \"Appointment\"\n" + + " }\n" + + " },\n" + + " {\n" + + " \"resource\": {\n" + + " \"resourceType\": \"Appointment\",\n" + + " \"status\": \"pending\",\n" + + " \"type\": {\"text\": \"GP Visit\"},\n" + + " \"description\": \"Routine checkup\",\n" + + " \"start\": \"2016-05-03T18:48:29+12:00\",\n" + + " \"end\": \"2016-05-03T19:03:29+12:00\",\n" + + " \"minutesDuration\": 15,\n" + + " \"participant\": [\n" + + " {\n" + + " \"actor\": {\"display\": \"Dr Dave\"},\n" + + " \"status\": \"accepted\"\n" + + " },\n" + + " {\n" + + " \"actor\": {\"reference\": \"Patient/" + id.getIdPart() + "\"},\n" + + " \"status\": \"accepted\"\n" + + " }\n" + + " ],\n" + + " \"text\": {\n" + + " \"status\": \"generated\",\n" + + " \"div\": \"
Routine checkup<\\/div>
Dr Dave<\\/div><\\/div>\"\n" + + " }\n" + + " },\n" + + " \"request\": {\n" + + " \"method\": \"POST\",\n" + + " \"url\": \"Appointment\"\n" + + " }\n" + + " }\n" + + " ]\n" + + "}"; + //@formatter:on + + Bundle inputBundle = myFhirCtx.newJsonParser().parseResource(Bundle.class, input); + Bundle outputBundle = mySystemDao.transaction(mySrd, inputBundle); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(outputBundle)); + } + @Test public void testTransactionWithInvalidType() { Bundle request = new Bundle(); @@ -1324,6 +1400,65 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { } + @Test + public void testTransactionWithNullReference() { + Patient p = new Patient(); + p.addName().addFamily("family"); + final IIdType id = myPatientDao.create(p, mySrd).getId().toUnqualifiedVersionless(); + + Bundle inputBundle = new Bundle(); + + //@formatter:off + Patient app0 = new Patient(); + app0.addName().addFamily("NEW PATIENT"); + String placeholderId0 = IdDt.newRandomUuid().getValue(); + inputBundle + .addEntry() + .setResource(app0) + .setFullUrl(placeholderId0) + .getRequest() + .setMethod(HTTPVerbEnum.POST) + .setUrl("Patient"); + //@formatter:on + + //@formatter:off + Appointment app1 = new Appointment(); + app1.addParticipant().getActor().setReference(id); + inputBundle + .addEntry() + .setResource(app1) + .getRequest() + .setMethod(HTTPVerbEnum.POST) + .setUrl("Appointment"); + //@formatter:on + + //@formatter:off + Appointment app2 = new Appointment(); + app2.addParticipant().getActor().setDisplay("NO REF"); + app2.addParticipant().getActor().setDisplay("YES REF").setReference(placeholderId0); + inputBundle + .addEntry() + .setResource(app2) + .getRequest() + .setMethod(HTTPVerbEnum.POST) + .setUrl("Appointment"); + //@formatter:on + + Bundle outputBundle = mySystemDao.transaction(mySrd, inputBundle); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(outputBundle)); + + assertEquals(3, outputBundle.getEntry().size()); + IdDt id0 = new IdDt(outputBundle.getEntry().get(0).getResponse().getLocation()); + IdDt id1 = new IdDt(outputBundle.getEntry().get(1).getResponse().getLocation()); + IdDt id2 = new IdDt(outputBundle.getEntry().get(2).getResponse().getLocation()); + + app2 = myAppointmentDao.read(id2, mySrd); + assertEquals("NO REF", app2.getParticipant().get(0).getActor().getDisplay().getValue()); + assertEquals(null, app2.getParticipant().get(0).getActor().getReference().getValue()); + assertEquals("YES REF", app2.getParticipant().get(1).getActor().getDisplay().getValue()); + assertEquals(id0.toUnqualifiedVersionless().getValue(), app2.getParticipant().get(1).getActor().getReference().getValue()); + } + @Test public void testTransactionWithReferenceToCreateIfNoneExist() { Bundle bundle = new Bundle(); @@ -1377,6 +1512,46 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { assertNotEquals(medOrderId1, medOrderId2); } + @Test + public void testTransactionWithRelativeOidIds() throws Exception { + Bundle res = new Bundle(); + res.setType(BundleTypeEnum.TRANSACTION); + + Patient p1 = new Patient(); + p1.setId("urn:oid:0.1.2.3"); + p1.addIdentifier().setSystem("system").setValue("testTransactionWithRelativeOidIds01"); + res.addEntry().setResource(p1).getRequest().setMethod(HTTPVerbEnum.POST).setUrl("Patient"); + + Observation o1 = new Observation(); + o1.setId("cid:observation1"); + o1.addIdentifier().setSystem("system").setValue("testTransactionWithRelativeOidIds02"); + o1.setSubject(new ResourceReferenceDt("urn:oid:0.1.2.3")); + res.addEntry().setResource(o1).getRequest().setMethod(HTTPVerbEnum.POST).setUrl("Observation"); + + Observation o2 = new Observation(); + o2.setId("cid:observation2"); + o2.addIdentifier().setSystem("system").setValue("testTransactionWithRelativeOidIds03"); + o2.setSubject(new ResourceReferenceDt("urn:oid:0.1.2.3")); + res.addEntry().setResource(o2).getRequest().setMethod(HTTPVerbEnum.POST).setUrl("Observation"); + + Bundle resp = mySystemDao.transaction(mySrd, res); + + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(resp)); + + assertEquals(BundleTypeEnum.TRANSACTION_RESPONSE, resp.getTypeElement().getValueAsEnum()); + assertEquals(3, resp.getEntry().size()); + + assertTrue(resp.getEntry().get(0).getResponse().getLocation(), new IdDt(resp.getEntry().get(0).getResponse().getLocation()).getIdPart().matches("^[0-9]+$")); + assertTrue(resp.getEntry().get(1).getResponse().getLocation(), new IdDt(resp.getEntry().get(1).getResponse().getLocation()).getIdPart().matches("^[0-9]+$")); + assertTrue(resp.getEntry().get(2).getResponse().getLocation(), new IdDt(resp.getEntry().get(2).getResponse().getLocation()).getIdPart().matches("^[0-9]+$")); + + o1 = myObservationDao.read(new IdDt(resp.getEntry().get(1).getResponse().getLocation()), mySrd); + o2 = myObservationDao.read(new IdDt(resp.getEntry().get(2).getResponse().getLocation()), mySrd); + assertThat(o1.getSubject().getReference().getValue(), endsWith("Patient/" + p1.getId().getIdPart())); + assertThat(o2.getSubject().getReference().getValue(), endsWith("Patient/" + p1.getId().getIdPart())); + + } + // // // /** @@ -1479,46 +1654,6 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { // // } - @Test - public void testTransactionWithRelativeOidIds() throws Exception { - Bundle res = new Bundle(); - res.setType(BundleTypeEnum.TRANSACTION); - - Patient p1 = new Patient(); - p1.setId("urn:oid:0.1.2.3"); - p1.addIdentifier().setSystem("system").setValue("testTransactionWithRelativeOidIds01"); - res.addEntry().setResource(p1).getRequest().setMethod(HTTPVerbEnum.POST).setUrl("Patient"); - - Observation o1 = new Observation(); - o1.setId("cid:observation1"); - o1.addIdentifier().setSystem("system").setValue("testTransactionWithRelativeOidIds02"); - o1.setSubject(new ResourceReferenceDt("urn:oid:0.1.2.3")); - res.addEntry().setResource(o1).getRequest().setMethod(HTTPVerbEnum.POST).setUrl("Observation"); - - Observation o2 = new Observation(); - o2.setId("cid:observation2"); - o2.addIdentifier().setSystem("system").setValue("testTransactionWithRelativeOidIds03"); - o2.setSubject(new ResourceReferenceDt("urn:oid:0.1.2.3")); - res.addEntry().setResource(o2).getRequest().setMethod(HTTPVerbEnum.POST).setUrl("Observation"); - - Bundle resp = mySystemDao.transaction(mySrd, res); - - ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(resp)); - - assertEquals(BundleTypeEnum.TRANSACTION_RESPONSE, resp.getTypeElement().getValueAsEnum()); - assertEquals(3, resp.getEntry().size()); - - assertTrue(resp.getEntry().get(0).getResponse().getLocation(), new IdDt(resp.getEntry().get(0).getResponse().getLocation()).getIdPart().matches("^[0-9]+$")); - assertTrue(resp.getEntry().get(1).getResponse().getLocation(), new IdDt(resp.getEntry().get(1).getResponse().getLocation()).getIdPart().matches("^[0-9]+$")); - assertTrue(resp.getEntry().get(2).getResponse().getLocation(), new IdDt(resp.getEntry().get(2).getResponse().getLocation()).getIdPart().matches("^[0-9]+$")); - - o1 = myObservationDao.read(new IdDt(resp.getEntry().get(1).getResponse().getLocation()), mySrd); - o2 = myObservationDao.read(new IdDt(resp.getEntry().get(2).getResponse().getLocation()), mySrd); - assertThat(o1.getSubject().getReference().getValue(), endsWith("Patient/" + p1.getId().getIdPart())); - assertThat(o2.getSubject().getReference().getValue(), endsWith("Patient/" + p1.getId().getIdPart())); - - } - /** * This is not the correct way to do it, but we'll allow it to be lenient */ @@ -1562,4 +1697,9 @@ public class FhirSystemDaoDstu2Test extends BaseJpaDstu2SystemTest { } + @AfterClass + public static void afterClassClearContext() { + TestUtil.clearAllStaticFieldsForUnitTest(); + } + } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java index 5b7a8ad0165..fa5fb39f578 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/BaseJpaDstu3Test.java @@ -12,6 +12,7 @@ import org.apache.commons.io.IOUtils; import org.hibernate.search.jpa.FullTextEntityManager; import org.hibernate.search.jpa.Search; import org.hl7.fhir.dstu3.hapi.validation.IValidationSupport; +import org.hl7.fhir.dstu3.model.Appointment; import org.hl7.fhir.dstu3.model.Bundle; import org.hl7.fhir.dstu3.model.CodeSystem; import org.hl7.fhir.dstu3.model.CodeableConcept; @@ -90,12 +91,15 @@ public abstract class BaseJpaDstu3Test extends BaseJpaTest { @Autowired - @Qualifier("myCodeSystemDaoDstu3") - protected IFhirResourceDao myCodeSystemDao; + @Qualifier("myAppointmentDaoDstu3") + protected IFhirResourceDao myAppointmentDao; @Autowired @Qualifier("myBundleDaoDstu3") protected IFhirResourceDao myBundleDao; @Autowired + @Qualifier("myCodeSystemDaoDstu3") + protected IFhirResourceDao myCodeSystemDao; + @Autowired @Qualifier("myConceptMapDaoDstu3") protected IFhirResourceDao myConceptMapDao; @Autowired @@ -139,14 +143,14 @@ public abstract class BaseJpaDstu3Test extends BaseJpaTest { @Autowired @Qualifier("myNamingSystemDaoDstu3") protected IFhirResourceDao myNamingSystemDao; - @Autowired - @Qualifier("myObservationDaoDstu3") - protected IFhirResourceDao myObservationDao; @Autowired -@Qualifier("myOrganizationDaoDstu3") -protected IFhirResourceDao myOrganizationDao; +@Qualifier("myObservationDaoDstu3") +protected IFhirResourceDao myObservationDao; + @Autowired + @Qualifier("myOrganizationDaoDstu3") + protected IFhirResourceDao myOrganizationDao; @Autowired @Qualifier("myPatientDaoDstu3") protected IFhirResourceDaoPatient myPatientDao; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java index 8c910b9e48b..937a6058e70 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java @@ -22,6 +22,7 @@ import java.io.UnsupportedEncodingException; import java.util.List; import org.apache.commons.io.IOUtils; +import org.hl7.fhir.dstu3.model.Appointment; import org.hl7.fhir.dstu3.model.Bundle; import org.hl7.fhir.dstu3.model.Bundle.BundleEntryComponent; import org.hl7.fhir.dstu3.model.Bundle.BundleEntryRequestComponent; @@ -58,6 +59,7 @@ import ca.uhn.fhir.jpa.entity.ResourceTable; import ca.uhn.fhir.jpa.entity.TagTypeEnum; import ca.uhn.fhir.jpa.provider.SystemProviderDstu2Test; import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; +import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.server.Constants; import ca.uhn.fhir.rest.server.IBundleProvider; @@ -77,6 +79,64 @@ public class FhirSystemDaoDstu3Test extends BaseJpaDstu3SystemTest { TestUtil.clearAllStaticFieldsForUnitTest(); } + @Test + public void testTransactionWithNullReference() { + Patient p = new Patient(); + p.addName().addFamily("family"); + final IIdType id = myPatientDao.create(p, mySrd).getId().toUnqualifiedVersionless(); + + Bundle inputBundle = new Bundle(); + + //@formatter:off + Patient app0 = new Patient(); + app0.addName().addFamily("NEW PATIENT"); + String placeholderId0 = IdDt.newRandomUuid().getValue(); + inputBundle + .addEntry() + .setResource(app0) + .setFullUrl(placeholderId0) + .getRequest() + .setMethod(HTTPVerb.POST) + .setUrl("Patient"); + //@formatter:on + + //@formatter:off + Appointment app1 = new Appointment(); + app1.addParticipant().getActor().setReference(id.getValue()); + inputBundle + .addEntry() + .setResource(app1) + .getRequest() + .setMethod(HTTPVerb.POST) + .setUrl("Appointment"); + //@formatter:on + + //@formatter:off + Appointment app2 = new Appointment(); + app2.addParticipant().getActor().setDisplay("NO REF"); + app2.addParticipant().getActor().setDisplay("YES REF").setReference(placeholderId0); + inputBundle + .addEntry() + .setResource(app2) + .getRequest() + .setMethod(HTTPVerb.POST) + .setUrl("Appointment"); + //@formatter:on + + Bundle outputBundle = mySystemDao.transaction(mySrd, inputBundle); + ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(outputBundle)); + + assertEquals(3, outputBundle.getEntry().size()); + IdDt id0 = new IdDt(outputBundle.getEntry().get(0).getResponse().getLocation()); + IdDt id2 = new IdDt(outputBundle.getEntry().get(2).getResponse().getLocation()); + + app2 = myAppointmentDao.read(id2, mySrd); + assertEquals("NO REF", app2.getParticipant().get(0).getActor().getDisplay()); + assertEquals(null, app2.getParticipant().get(0).getActor().getReference()); + assertEquals("YES REF", app2.getParticipant().get(1).getActor().getDisplay()); + assertEquals(id0.toUnqualifiedVersionless().getValue(), app2.getParticipant().get(1).getActor().getReference()); + } + @Test public void testReindexing() { diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 57b8c6cdf3c..ee5bfa8be26 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -51,6 +51,11 @@ Note that existing databases will need to modify index "IDX_FORCEDID" as it is no longer unique, and perform a reindexing pass. + + JPA server transactions sometimes created an incorrect resource reference + if a resource being saved contained references that had a display value but + not an actual reference. Thanks to David Hay for reporting! +