From da1d49a10803a672b2ff7289892586cc389ae7c2 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Sun, 8 Mar 2015 16:55:42 -0400 Subject: [PATCH] Correctly index Observation.applies[x] in JPA server --- .../java/ca/uhn/fhir/util/FhirTerser.java | 8 +++- .../java/ca/uhn/fhir/jpa/dao/BaseFhirDao.java | 4 +- .../jpa/dao/BaseSearchParamExtractor.java | 40 +++++++++++++++++++ .../jpa/dao/SearchParamExtractorDstu1.java | 33 ++++----------- .../jpa/dao/SearchParamExtractorDstu2.java | 36 ++++------------- .../uhn/fhir/jpa/dao/FhirResourceDaoTest.java | 17 ++++++++ src/changes/changes.xml | 4 ++ 7 files changed, 84 insertions(+), 58 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseSearchParamExtractor.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/FhirTerser.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/FhirTerser.java index 8781d201c1b..095f25c28c8 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/FhirTerser.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/FhirTerser.java @@ -149,9 +149,13 @@ public class FhirTerser { if (nextDef instanceof RuntimeChildChoiceDefinition) { for (IBase next : values) { if (next != null) { - String childName = nextDef.getChildNameByDatatype(next.getClass()); - if (theSubList.get(0).equals(childName)) { + if (name.endsWith("[x]")) { retVal.add(next); + } else { + String childName = nextDef.getChildNameByDatatype(next.getClass()); + if (theSubList.get(0).equals(childName)) { + retVal.add(next); + } } } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseFhirDao.java index 498cfe17aec..6d8fbbb16b0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseFhirDao.java @@ -180,8 +180,8 @@ public abstract class BaseFhirDao implements IDao { if (nextValue.isEmpty()) { continue; } - if (nextValue.getReference().getValue().startsWith("#")) { - // This is a contained resource reference + if (nextValue.getReference().isEmpty() || nextValue.getReference().getValue().startsWith("#")) { + // This is a blank or contained resource reference continue; } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseSearchParamExtractor.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseSearchParamExtractor.java new file mode 100644 index 00000000000..1c0b3609d7e --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseSearchParamExtractor.java @@ -0,0 +1,40 @@ +package ca.uhn.fhir.jpa.dao; + +import java.util.ArrayList; +import java.util.List; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.RuntimeResourceDefinition; +import ca.uhn.fhir.model.api.IResource; +import ca.uhn.fhir.util.FhirTerser; + +public class BaseSearchParamExtractor { + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(BaseSearchParamExtractor.class); + private FhirContext myContext; + + public BaseSearchParamExtractor(FhirContext theContext) { + myContext = theContext; + } + + protected FhirContext getContext() { + return myContext; + } + + protected List extractValues(String thePaths, IResource theResource) { + List values = new ArrayList(); + String[] nextPathsSplit = thePaths.split("\\|"); + FhirTerser t = myContext.newTerser(); + for (String nextPath : nextPathsSplit) { + String nextPathTrimmed = nextPath.trim(); + try { + values.addAll(t.getValues(theResource, nextPathTrimmed)); + } catch (Exception e) { + RuntimeResourceDefinition def = myContext.getResourceDefinition(theResource); + ourLog.warn("Failed to index values from path[{}] in resource type[{}]: ", nextPathTrimmed, def.getName(), e.toString()); + } + } + return values; + } + + +} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchParamExtractorDstu1.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchParamExtractorDstu1.java index eaf7d4252a3..bdbcb83348a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchParamExtractorDstu1.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchParamExtractorDstu1.java @@ -63,21 +63,18 @@ import ca.uhn.fhir.model.primitive.BaseDateTimeDt; import ca.uhn.fhir.model.primitive.IntegerDt; import ca.uhn.fhir.model.primitive.StringDt; import ca.uhn.fhir.model.primitive.UriDt; -import ca.uhn.fhir.util.FhirTerser; -class SearchParamExtractorDstu1 implements ISearchParamExtractor { - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchParamExtractorDstu1.class); - private FhirContext myContext; +class SearchParamExtractorDstu1 extends BaseSearchParamExtractor implements ISearchParamExtractor { public SearchParamExtractorDstu1(FhirContext theContext) { - myContext = theContext; + super(theContext); } @Override public List extractSearchParamDates(ResourceTable theEntity, IResource theResource) { ArrayList retVal = new ArrayList(); - RuntimeResourceDefinition def = myContext.getResourceDefinition(theResource); + RuntimeResourceDefinition def = getContext().getResourceDefinition(theResource); for (RuntimeSearchParam nextSpDef : def.getSearchParams()) { if (nextSpDef.getParamType() != SearchParamTypeEnum.DATE) { continue; @@ -134,7 +131,7 @@ class SearchParamExtractorDstu1 implements ISearchParamExtractor { public ArrayList extractSearchParamNumber(ResourceTable theEntity, IResource theResource) { ArrayList retVal = new ArrayList(); - RuntimeResourceDefinition def = myContext.getResourceDefinition(theResource); + RuntimeResourceDefinition def = getContext().getResourceDefinition(theResource); for (RuntimeSearchParam nextSpDef : def.getSearchParams()) { if (nextSpDef.getParamType() != SearchParamTypeEnum.NUMBER) { continue; @@ -231,7 +228,7 @@ class SearchParamExtractorDstu1 implements ISearchParamExtractor { public List extractSearchParamQuantity(ResourceTable theEntity, IResource theResource) { ArrayList retVal = new ArrayList(); - RuntimeResourceDefinition def = myContext.getResourceDefinition(theResource); + RuntimeResourceDefinition def = getContext().getResourceDefinition(theResource); for (RuntimeSearchParam nextSpDef : def.getSearchParams()) { if (nextSpDef.getParamType() != SearchParamTypeEnum.QUANTITY) { continue; @@ -281,7 +278,7 @@ class SearchParamExtractorDstu1 implements ISearchParamExtractor { public List extractSearchParamStrings(ResourceTable theEntity, IResource theResource) { ArrayList retVal = new ArrayList(); - RuntimeResourceDefinition def = myContext.getResourceDefinition(theResource); + RuntimeResourceDefinition def = getContext().getResourceDefinition(theResource); for (RuntimeSearchParam nextSpDef : def.getSearchParams()) { if (nextSpDef.getParamType() != SearchParamTypeEnum.STRING) { continue; @@ -369,7 +366,7 @@ class SearchParamExtractorDstu1 implements ISearchParamExtractor { public List extractSearchParamTokens(ResourceTable theEntity, IResource theResource) { ArrayList retVal = new ArrayList(); - RuntimeResourceDefinition def = myContext.getResourceDefinition(theResource); + RuntimeResourceDefinition def = getContext().getResourceDefinition(theResource); for (RuntimeSearchParam nextSpDef : def.getSearchParams()) { if (nextSpDef.getParamType() != SearchParamTypeEnum.TOKEN) { continue; @@ -475,20 +472,4 @@ class SearchParamExtractorDstu1 implements ISearchParamExtractor { return retVal; } - private List extractValues(String thePaths, IResource theResource) { - List values = new ArrayList(); - String[] nextPathsSplit = thePaths.split("\\|"); - FhirTerser t = myContext.newTerser(); - for (String nextPath : nextPathsSplit) { - String nextPathTrimmed = nextPath.trim(); - try { - values.addAll(t.getValues(theResource, nextPathTrimmed)); - } catch (Exception e) { - RuntimeResourceDefinition def = myContext.getResourceDefinition(theResource); - ourLog.warn("Failed to index values from path[{}] in resource type[{}]: ", nextPathTrimmed, def.getName(), e.toString()); - } - } - return values; - } - } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchParamExtractorDstu2.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchParamExtractorDstu2.java index bbb488d5d90..86506a7fd46 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchParamExtractorDstu2.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchParamExtractorDstu2.java @@ -20,8 +20,7 @@ package ca.uhn.fhir.jpa.dao; * #L% */ -import static org.apache.commons.lang3.StringUtils.isBlank; -import static org.apache.commons.lang3.StringUtils.isNotBlank; +import static org.apache.commons.lang3.StringUtils.*; import java.math.BigDecimal; import java.util.ArrayList; @@ -64,14 +63,11 @@ import ca.uhn.fhir.model.primitive.BaseDateTimeDt; import ca.uhn.fhir.model.primitive.IntegerDt; import ca.uhn.fhir.model.primitive.StringDt; import ca.uhn.fhir.model.primitive.UriDt; -import ca.uhn.fhir.util.FhirTerser; -class SearchParamExtractorDstu2 implements ISearchParamExtractor { - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchParamExtractorDstu2.class); - private FhirContext myContext; +class SearchParamExtractorDstu2 extends BaseSearchParamExtractor implements ISearchParamExtractor { public SearchParamExtractorDstu2(FhirContext theContext) { - myContext = theContext; + super(theContext); } /* @@ -84,7 +80,7 @@ class SearchParamExtractorDstu2 implements ISearchParamExtractor { public List extractSearchParamDates(ResourceTable theEntity, IResource theResource) { ArrayList retVal = new ArrayList(); - RuntimeResourceDefinition def = myContext.getResourceDefinition(theResource); + RuntimeResourceDefinition def = getContext().getResourceDefinition(theResource); for (RuntimeSearchParam nextSpDef : def.getSearchParams()) { if (nextSpDef.getParamType() != SearchParamTypeEnum.DATE) { continue; @@ -147,7 +143,7 @@ class SearchParamExtractorDstu2 implements ISearchParamExtractor { public ArrayList extractSearchParamNumber(ResourceTable theEntity, IResource theResource) { ArrayList retVal = new ArrayList(); - RuntimeResourceDefinition def = myContext.getResourceDefinition(theResource); + RuntimeResourceDefinition def = getContext().getResourceDefinition(theResource); for (RuntimeSearchParam nextSpDef : def.getSearchParams()) { if (nextSpDef.getParamType() != SearchParamTypeEnum.NUMBER) { continue; @@ -250,7 +246,7 @@ class SearchParamExtractorDstu2 implements ISearchParamExtractor { public List extractSearchParamQuantity(ResourceTable theEntity, IResource theResource) { ArrayList retVal = new ArrayList(); - RuntimeResourceDefinition def = myContext.getResourceDefinition(theResource); + RuntimeResourceDefinition def = getContext().getResourceDefinition(theResource); for (RuntimeSearchParam nextSpDef : def.getSearchParams()) { if (nextSpDef.getParamType() != SearchParamTypeEnum.QUANTITY) { continue; @@ -306,7 +302,7 @@ class SearchParamExtractorDstu2 implements ISearchParamExtractor { public List extractSearchParamStrings(ResourceTable theEntity, IResource theResource) { ArrayList retVal = new ArrayList(); - RuntimeResourceDefinition def = myContext.getResourceDefinition(theResource); + RuntimeResourceDefinition def = getContext().getResourceDefinition(theResource); for (RuntimeSearchParam nextSpDef : def.getSearchParams()) { if (nextSpDef.getParamType() != SearchParamTypeEnum.STRING) { continue; @@ -400,7 +396,7 @@ class SearchParamExtractorDstu2 implements ISearchParamExtractor { public List extractSearchParamTokens(ResourceTable theEntity, IResource theResource) { ArrayList retVal = new ArrayList(); - RuntimeResourceDefinition def = myContext.getResourceDefinition(theResource); + RuntimeResourceDefinition def = getContext().getResourceDefinition(theResource); for (RuntimeSearchParam nextSpDef : def.getSearchParams()) { if (nextSpDef.getParamType() != SearchParamTypeEnum.TOKEN) { continue; @@ -506,20 +502,4 @@ class SearchParamExtractorDstu2 implements ISearchParamExtractor { return retVal; } - private List extractValues(String thePaths, IResource theResource) { - List values = new ArrayList(); - String[] nextPathsSplit = thePaths.split("\\|"); - FhirTerser t = myContext.newTerser(); - for (String nextPath : nextPathsSplit) { - String nextPathTrimmed = nextPath.trim(); - try { - values.addAll(t.getValues(theResource, nextPathTrimmed)); - } catch (Exception e) { - RuntimeResourceDefinition def = myContext.getResourceDefinition(theResource); - ourLog.warn("Failed to index values from path[{}] in resource type[{}]: ", nextPathTrimmed, def.getName(), e.toString()); - } - } - return values; - } - } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoTest.java index 601cc060ba7..b9bc93b4316 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/FhirResourceDaoTest.java @@ -294,6 +294,23 @@ public class FhirResourceDaoTest { } } + @Test + public void testChoiceParamDateAlt() { + Observation o2 = new Observation(); + o2.getCode().addCoding().setSystem("foo").setCode("testChoiceParamDateAlt02"); + o2.setApplies(new DateTimeDt("2015-03-08T11:11:11")); + IdDt id2 = ourObservationDao.create(o2).getId(); + + { + Set found = ourObservationDao.searchForIds(Observation.SP_DATE, new DateParam(">2001-01-02")); + assertThat(found, contains(id2.getIdPartAsLong())); + } + { + Set found = ourObservationDao.searchForIds(Observation.SP_DATE, new DateParam(">2016-01-02")); + assertThat(found, not(contains(id2.getIdPartAsLong()))); + } + } + @Test public void testChoiceParamQuantity() { Observation o3 = new Observation(); diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 70ee0129fc8..cf35ad9a0dd 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -135,6 +135,10 @@ specification, for the Generic Client, Annotation Client, and Server. + + Observation.applies[x] and other similar search fields with multiple allowable + value types were not being correctly indexed in the JPA server. +