From 0e90867a651381929325c9e81b16e685b8cc4845 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Tue, 24 Sep 2019 14:35:34 -0400 Subject: [PATCH 1/2] fixed meta.source request append bug --- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 19 +++++++++++--- .../uhn/fhir/jpa/dao/BaseHapiFhirDaoTest.java | 16 ++++++++++++ .../dstu3/ResourceProviderDstu3Test.java | 26 +++++++++++++++++++ .../provider/r4/ResourceProviderR4Test.java | 25 ++++++++++++++++++ src/changes/changes.xml | 5 ++++ 5 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDaoTest.java diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index 64414a391ea..52d6eaf5812 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -47,12 +47,14 @@ import ca.uhn.fhir.rest.api.RestSearchParameterTypeEnum; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; -import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetails; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; -import ca.uhn.fhir.util.*; +import ca.uhn.fhir.util.CoverageIgnore; +import ca.uhn.fhir.util.MetaUtil; +import ca.uhn.fhir.util.StopWatch; +import ca.uhn.fhir.util.XmlUtil; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Charsets; import com.google.common.collect.Sets; @@ -86,7 +88,6 @@ import javax.xml.stream.events.XMLEvent; import java.util.*; import java.util.Map.Entry; -import static ca.uhn.fhir.jpa.model.util.JpaConstants.EXT_EXTERNALIZED_BINARY_ID; import static org.apache.commons.lang3.StringUtils.*; /* @@ -974,7 +975,7 @@ public abstract class BaseHapiFhirDao implements IDao, // 6. Handle source (provenance) if (isNotBlank(provenanceRequestId) || isNotBlank(provenanceSourceUri)) { - String sourceString = defaultString(provenanceSourceUri) + String sourceString = cleanProvenanceSourceUri(provenanceSourceUri) + (isNotBlank(provenanceRequestId) ? "#" : "") + defaultString(provenanceRequestId); @@ -992,6 +993,16 @@ public abstract class BaseHapiFhirDao implements IDao, return retVal; } + static String cleanProvenanceSourceUri(String theProvenanceSourceUri) { + if (isNotBlank(theProvenanceSourceUri)) { + int hashIndex = theProvenanceSourceUri.indexOf('#'); + if (hashIndex != -1) { + theProvenanceSourceUri = theProvenanceSourceUri.substring(0, hashIndex); + } + } + return defaultString(theProvenanceSourceUri); + } + public String toResourceName(Class theResourceType) { return myContext.getResourceDefinition(theResourceType).getName(); } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDaoTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDaoTest.java new file mode 100644 index 00000000000..787b3fa2ec8 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDaoTest.java @@ -0,0 +1,16 @@ +package ca.uhn.fhir.jpa.dao; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +public class BaseHapiFhirDaoTest { + + @Test + public void cleanProvenanceSourceUri() { + assertEquals("", BaseHapiFhirDao.cleanProvenanceSourceUri(null)); + assertEquals("abc", BaseHapiFhirDao.cleanProvenanceSourceUri("abc")); + assertEquals("abc", BaseHapiFhirDao.cleanProvenanceSourceUri("abc#def")); + assertEquals("abc", BaseHapiFhirDao.cleanProvenanceSourceUri("abc#def#ghi")); + } +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java index 91d6fa55695..c44d8e47ce7 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java @@ -3,6 +3,7 @@ package ca.uhn.fhir.jpa.provider.dstu3; import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.jpa.dao.data.ISearchDao; import ca.uhn.fhir.jpa.entity.Search; +import ca.uhn.fhir.jpa.model.util.JpaConstants; import ca.uhn.fhir.jpa.provider.r4.ResourceProviderR4Test; import ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl; import ca.uhn.fhir.model.api.TemporalPrecisionEnum; @@ -3913,6 +3914,31 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } + @Test + public void testUpdateWithSource() { + Patient patient = new Patient(); + patient.setActive(false); + IIdType patientid = ourClient.create().resource(patient).execute().getId().toUnqualifiedVersionless(); + + { + Patient readPatient = (Patient) ourClient.read().resource("Patient").withId(patientid).execute(); + assertThat(readPatient.getMeta().getExtensionString(JpaConstants.EXT_META_SOURCE), matchesPattern("#[a-f0-9]+")); + } + + patient.setId(patientid); + patient.setActive(true); + ourClient.update().resource(patient).execute(); + { + Patient readPatient = (Patient) ourClient.read().resource("Patient").withId(patientid).execute(); + assertThat(readPatient.getMeta().getExtensionString(JpaConstants.EXT_META_SOURCE), matchesPattern("#[a-f0-9]+")); + + readPatient.addName().setFamily("testUpdateWithSource"); + ourClient.update().resource(readPatient).execute(); + readPatient = (Patient) ourClient.read().resource("Patient").withId(patientid).execute(); + assertThat(readPatient.getMeta().getExtensionString(JpaConstants.EXT_META_SOURCE), matchesPattern("#[a-f0-9]+")); + } + } + @Test public void testUpdateWithETag() throws Exception { String methodName = "testUpdateWithETag"; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java index 2c25bc2c640..f03a95b91b0 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java @@ -5081,6 +5081,31 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test { } + @Test + public void testUpdateWithSource() { + Patient patient = new Patient(); + patient.setActive(false); + IIdType patientid = ourClient.create().resource(patient).execute().getId().toUnqualifiedVersionless(); + + { + Patient readPatient = (Patient) ourClient.read().resource("Patient").withId(patientid).execute(); + assertThat(readPatient.getMeta().getSource(), matchesPattern("#[a-f0-9]+")); + } + + patient.setId(patientid); + patient.setActive(true); + ourClient.update().resource(patient).execute(); + { + Patient readPatient = (Patient) ourClient.read().resource("Patient").withId(patientid).execute(); + assertThat(readPatient.getMeta().getSource(), matchesPattern("#[a-f0-9]+")); + + readPatient.addName().setFamily("testUpdateWithSource"); + ourClient.update().resource(readPatient).execute(); + readPatient = (Patient) ourClient.read().resource("Patient").withId(patientid).execute(); + assertThat(readPatient.getMeta().getSource(), matchesPattern("#[a-f0-9]+")); + } + } + @Test public void testUpdateWithETag() throws Exception { String methodName = "testUpdateWithETag"; diff --git a/src/changes/changes.xml b/src/changes/changes.xml index b9f335b5949..879507c2efc 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -201,6 +201,11 @@ Some resource IDs and URLs for LOINC ValueSets and ConceptMaps were inconsistently populated by the terminology uploader. This has been corrected. + + When a resource was updated with a meta.source containing a request id, the meta.source was getting appended + with the new request id, resulting in an ever growing source.meta value. E.g. after the first update, it looks + like "#9f0a901387128111#5f37835ee38a89e2" when it should only be "#5f37835ee38a89e2". This has been corrected. + From 7b363c2c5c69c0855b8377fb3d9dd9547b60b570 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Tue, 24 Sep 2019 15:28:14 -0400 Subject: [PATCH 2/2] Some test fixes --- .../ca/uhn/fhir/rest/param/ReferenceParam.java | 16 ++++++++++++---- .../uhn/fhir/rest/param/ReferenceParamTest.java | 12 ++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ReferenceParam.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ReferenceParam.java index 6ade76b3f1c..e018fb94cff 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ReferenceParam.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ReferenceParam.java @@ -22,6 +22,7 @@ package ca.uhn.fhir.rest.param; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.model.primitive.IdDt; +import ca.uhn.fhir.parser.DataFormatException; import ca.uhn.fhir.util.CoverageIgnore; import org.apache.commons.lang3.builder.ToStringBuilder; import org.apache.commons.lang3.builder.ToStringStyle; @@ -115,14 +116,21 @@ public class ReferenceParam extends BaseParam /*implements IQueryParameterType*/ if (nextIdx != -1) { myChain = q.substring(nextIdx + 1); myResourceType = q.substring(1, nextIdx); - myValue = theValue; - myIdPart = theValue; } else { myChain = null; myResourceType = q.substring(1); - myValue = theValue; - myIdPart = theValue; } + + myValue = theValue; + myIdPart = theValue; + + IdDt id = new IdDt(theValue); + if (!id.hasBaseUrl() && id.hasIdPart() && id.hasResourceType()) { + if (id.getResourceType().equals(myResourceType)) { + myIdPart = id.getIdPart(); + } + } + } else if (q.startsWith(".")) { myChain = q.substring(1); myResourceType = null; diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/ReferenceParamTest.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/ReferenceParamTest.java index 504ee2c79ef..f727d380bf5 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/ReferenceParamTest.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/param/ReferenceParamTest.java @@ -124,6 +124,18 @@ public class ReferenceParamTest { } + @Test + public void testDuplicatedTypeAndValueType() { + + ReferenceParam rp = new ReferenceParam(); + rp.setValueAsQueryToken(ourCtx, null, ":Patient", "Patient/123"); + assertEquals("Patient", rp.getResourceType()); + assertEquals("123", rp.getIdPart()); + assertEquals("Patient/123", rp.getValue()); + assertEquals(null, rp.getQueryParameterQualifier()); + + } + // TODO: verify this behavior is correct. Same case as testWithResourceTypeAsQualifier_RelativeUrl() @Test public void testWithResourceTypeAsQualifier_AbsoluteUrl() {