From 868ef9590d90cf484470aa950ca3dbf5a7663331 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Fri, 31 Jan 2020 17:14:37 -0500 Subject: [PATCH 01/10] started writing test --- .../dstu3/ResourceProviderDstu3Test.java | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) 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 69a03290a8a..b7534f17fc4 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 @@ -5,6 +5,7 @@ import ca.uhn.fhir.jpa.dao.data.ISearchDao; import ca.uhn.fhir.jpa.entity.Search; import ca.uhn.fhir.jpa.provider.r4.ResourceProviderR4Test; import ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl; +import ca.uhn.fhir.jpa.util.CoordCalculatorTest; import ca.uhn.fhir.model.api.TemporalPrecisionEnum; import ca.uhn.fhir.model.primitive.InstantDt; import ca.uhn.fhir.model.primitive.UriDt; @@ -4282,6 +4283,49 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } + @Test + public void testNearSearchApproximate() { + Location loc = new Location(); + double latitude = CoordCalculatorTest.LATITUDE_UHN; + double longitude = CoordCalculatorTest.LONGITUDE_UHN; + Location.LocationPositionComponent position = new Location.LocationPositionComponent().setLatitude(latitude).setLongitude(longitude); + loc.setPosition(position); + IIdType locId = ourClient.create().resource(loc).execute().getId().toUnqualifiedVersionless(); + + { // In the box + double bigEnoughDistance = CoordCalculatorTest.DISTANCE_KM_CHIN_TO_UHN * 2; + String url = "/Location/" + + Location.SP_NEAR + "=" + CoordCalculatorTest.LATITUDE_CHIN + ":" + CoordCalculatorTest.LONGITUDE_CHIN + + "&" + + Location.SP_NEAR_DISTANCE + "=" + bigEnoughDistance + "|http://unitsofmeasure.org|km"); + + Bundle actual = ourClient + .search() + .byUrl(url) + .encodedJson() + .prettyPrint() + .returnBundle(Bundle.class) + .execute(); + //@formatter:on + + assertEquals(1, actual.getEntry().size()); + assertEquals(locId.getIdPart(), actual.getEntry().get(0).getResource().getIdElement().getIdPart()); + } + { // Outside the box + double tooSmallDistance = CoordCalculatorTest.DISTANCE_KM_CHIN_TO_UHN / 2; + +// SearchParameterMap map = myMatchUrlService.translateMatchUrl( +// "Location?" + +// Location.SP_NEAR + "=" + CoordCalculatorTest.LATITUDE_CHIN + ":" + CoordCalculatorTest.LONGITUDE_CHIN + +// "&" + +// Location.SP_NEAR_DISTANCE + "=" + tooSmallDistance + "|http://unitsofmeasure.org|km", myFhirCtx.getResourceDefinition("Location")); + +// List ids = toUnqualifiedVersionlessIdValues(myLocationDao.search(map)); +// assertThat(ids.size(), is(0)); + } + + } + private String toStr(Date theDate) { return new InstantDt(theDate).getValueAsString(); } From cd617389c693ea2675e1dc30d6bc4b3647b0eeef Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Fri, 31 Jan 2020 18:07:17 -0500 Subject: [PATCH 02/10] create failing test --- .../jpa/provider/dstu3/ResourceProviderDstu3Test.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) 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 b7534f17fc4..6e395fc6fcf 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 @@ -66,6 +66,7 @@ import java.math.BigDecimal; import java.net.InetSocketAddress; import java.net.Socket; import java.net.SocketTimeoutException; +import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.util.*; @@ -4294,14 +4295,14 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { { // In the box double bigEnoughDistance = CoordCalculatorTest.DISTANCE_KM_CHIN_TO_UHN * 2; - String url = "/Location/" + - Location.SP_NEAR + "=" + CoordCalculatorTest.LATITUDE_CHIN + ":" + CoordCalculatorTest.LONGITUDE_CHIN + + String url = "/Location?" + + Location.SP_NEAR + "=" + CoordCalculatorTest.LATITUDE_CHIN + URLEncoder.encode(":") + CoordCalculatorTest.LONGITUDE_CHIN + "&" + - Location.SP_NEAR_DISTANCE + "=" + bigEnoughDistance + "|http://unitsofmeasure.org|km"); + Location.SP_NEAR_DISTANCE + "=" + bigEnoughDistance + URLEncoder.encode("|http://unitsofmeasure.org|km"); Bundle actual = ourClient .search() - .byUrl(url) + .byUrl(ourServerBase + "/" + url) .encodedJson() .prettyPrint() .returnBundle(Bundle.class) @@ -4313,6 +4314,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } { // Outside the box double tooSmallDistance = CoordCalculatorTest.DISTANCE_KM_CHIN_TO_UHN / 2; + // FIXME KHS add this part // SearchParameterMap map = myMatchUrlService.translateMatchUrl( // "Location?" + From fb1eb7ca8f858b6adfc8c4ab637aaf1fd1cd2ef2 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Sat, 1 Feb 2020 13:58:27 -0500 Subject: [PATCH 03/10] parking wip --- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 15 ++++++++++++ .../dstu3/ResourceProviderDstu3Test.java | 2 +- .../fhir/jpa/searchparam/MatchUrlService.java | 8 ++++++- .../jpa/searchparam/SearchParameterMap.java | 24 ++++--------------- 4 files changed, 28 insertions(+), 21 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java index 3ba2c792a74..1a3aca86252 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java @@ -53,6 +53,7 @@ import ca.uhn.fhir.rest.api.SortSpec; import ca.uhn.fhir.rest.api.server.IPreResourceAccessDetails; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.param.DateRangeParam; +import ca.uhn.fhir.rest.param.QuantityParam; import ca.uhn.fhir.rest.param.ReferenceParam; import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; @@ -65,6 +66,7 @@ import org.apache.commons.lang3.Validate; import org.hibernate.ScrollMode; import org.hibernate.ScrollableResults; import org.hibernate.query.Query; +import org.hl7.fhir.dstu3.model.Location; import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IBaseResource; import org.slf4j.Logger; @@ -160,6 +162,8 @@ public class SearchBuilder implements ISearchBuilder { // Remove any empty parameters theParams.clean(); + setLocationDistance(theParams); + /* * Check if there is a unique key associated with the set * of parameters passed in @@ -181,6 +185,17 @@ public class SearchBuilder implements ISearchBuilder { } + private void setLocationDistance(SearchParameterMap theParams) { + if (myResourceType == Location.class && theParams.containsKey(Location.SP_NEAR_DISTANCE)) { + List> paramList = theParams.get(Location.SP_NEAR_DISTANCE); + paramList.stream() + .flatMap(List::stream) + .findFirst() + .map(QuantityParam.class::cast) + .ifPresent(theParams::setNearDistanceParam); + } + } + @Override public Iterator createCountQuery(SearchParameterMap theParams, String theSearchUuid, RequestDetails theRequest) { 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 6e395fc6fcf..2b09ece8b0a 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 @@ -4308,7 +4308,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { .returnBundle(Bundle.class) .execute(); //@formatter:on - +// FIXME KHS hmm this test should be passing now...? assertEquals(1, actual.getEntry().size()); assertEquals(locId.getIdPart(), actual.getEntry().get(0).getResource().getIdElement().getIdPart()); } diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java index 6cd49534031..d9ed8561105 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java @@ -33,6 +33,7 @@ import ca.uhn.fhir.rest.api.RestSearchParameterTypeEnum; import ca.uhn.fhir.rest.param.DateRangeParam; import ca.uhn.fhir.rest.param.ParameterUtil; import ca.uhn.fhir.rest.param.QuantityAndListParam; +import ca.uhn.fhir.rest.param.QuantityOrListParam; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.util.ReflectionUtil; import ca.uhn.fhir.util.UrlUtil; @@ -117,7 +118,12 @@ public class MatchUrlService { paramMap.add(nextParamName, param); } else if (Location.SP_NEAR_DISTANCE.equals(nextParamName)) { QuantityAndListParam nearDistanceAndListParam = (QuantityAndListParam) ParameterUtil.parseQueryParams(myContext, RestSearchParameterTypeEnum.QUANTITY, nextParamName, paramList); - paramMap.setNearDistanceParam(nearDistanceAndListParam); + // FIXME KHS this needs to be set elsewhere + nearDistanceAndListParam.getValuesAsQueryTokens().stream() + .map(QuantityOrListParam::getValuesAsQueryTokens) + .flatMap(List::stream) + .findFirst() + .ifPresent(paramMap::setNearDistanceParam); } else if (nextParamName.startsWith("_")) { // ignore these since they aren't search params (e.g. _sort) } else { diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/SearchParameterMap.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/SearchParameterMap.java index a159ce3b6fa..a3a384936d4 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/SearchParameterMap.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/SearchParameterMap.java @@ -5,9 +5,10 @@ import ca.uhn.fhir.model.api.IQueryParameterAnd; import ca.uhn.fhir.model.api.IQueryParameterOr; import ca.uhn.fhir.model.api.IQueryParameterType; import ca.uhn.fhir.model.api.Include; -import ca.uhn.fhir.model.dstu2.resource.Location; import ca.uhn.fhir.rest.api.*; -import ca.uhn.fhir.rest.param.*; +import ca.uhn.fhir.rest.param.DateParam; +import ca.uhn.fhir.rest.param.DateRangeParam; +import ca.uhn.fhir.rest.param.QuantityParam; import ca.uhn.fhir.util.ObjectUtil; import ca.uhn.fhir.util.UrlUtil; import org.apache.commons.lang3.StringUtils; @@ -495,23 +496,8 @@ public class SearchParameterMap implements Serializable { } } - public void setNearDistanceParam(QuantityAndListParam theQuantityAndListParam) { - List orTokens = theQuantityAndListParam.getValuesAsQueryTokens(); - if (orTokens.isEmpty()) { - return; - } - if (orTokens.size() > 1) { - throw new IllegalArgumentException("Only one " + Location.SP_NEAR_DISTANCE + " parameter may be present"); - } - QuantityOrListParam quantityOrListParam = orTokens.get(0); - List tokens = quantityOrListParam.getValuesAsQueryTokens(); - if (tokens.isEmpty()) { - return; - } - if (tokens.size() > 1) { - throw new IllegalArgumentException("Only one " + Location.SP_NEAR_DISTANCE + " parameter may be present"); - } - myNearDistanceParam = tokens.get(0); + public void setNearDistanceParam(QuantityParam theQuantityParam) { + myNearDistanceParam = theQuantityParam; } public QuantityParam getNearDistanceParam() { From c375c66f156b2d5d03c8672713db2a68ae03b58a Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Sat, 1 Feb 2020 20:15:58 -0500 Subject: [PATCH 04/10] test passes --- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 2 ++ .../dstu3/ResourceProviderDstu3Test.java | 27 ++++++++++++------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java index 1a3aca86252..8c820ca77d5 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java @@ -193,6 +193,8 @@ public class SearchBuilder implements ISearchBuilder { .findFirst() .map(QuantityParam.class::cast) .ifPresent(theParams::setNearDistanceParam); + // Need to remove near-distance or it we'll get a hashcode predicate for it + theParams.remove(Location.SP_NEAR_DISTANCE); } } 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 2b09ece8b0a..cbdc24875df 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 @@ -4300,6 +4300,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { "&" + Location.SP_NEAR_DISTANCE + "=" + bigEnoughDistance + URLEncoder.encode("|http://unitsofmeasure.org|km"); + myCaptureQueriesListener.clear(); Bundle actual = ourClient .search() .byUrl(ourServerBase + "/" + url) @@ -4307,23 +4308,29 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { .prettyPrint() .returnBundle(Bundle.class) .execute(); - //@formatter:on -// FIXME KHS hmm this test should be passing now...? + myCaptureQueriesListener.logSelectQueries(); + assertEquals(1, actual.getEntry().size()); assertEquals(locId.getIdPart(), actual.getEntry().get(0).getResource().getIdElement().getIdPart()); } { // Outside the box double tooSmallDistance = CoordCalculatorTest.DISTANCE_KM_CHIN_TO_UHN / 2; - // FIXME KHS add this part + String url = "/Location?" + + Location.SP_NEAR + "=" + CoordCalculatorTest.LATITUDE_CHIN + URLEncoder.encode(":") + CoordCalculatorTest.LONGITUDE_CHIN + + "&" + + Location.SP_NEAR_DISTANCE + "=" + tooSmallDistance + URLEncoder.encode("|http://unitsofmeasure.org|km"); -// SearchParameterMap map = myMatchUrlService.translateMatchUrl( -// "Location?" + -// Location.SP_NEAR + "=" + CoordCalculatorTest.LATITUDE_CHIN + ":" + CoordCalculatorTest.LONGITUDE_CHIN + -// "&" + -// Location.SP_NEAR_DISTANCE + "=" + tooSmallDistance + "|http://unitsofmeasure.org|km", myFhirCtx.getResourceDefinition("Location")); + myCaptureQueriesListener.clear(); + Bundle actual = ourClient + .search() + .byUrl(ourServerBase + "/" + url) + .encodedJson() + .prettyPrint() + .returnBundle(Bundle.class) + .execute(); + myCaptureQueriesListener.logSelectQueries(); -// List ids = toUnqualifiedVersionlessIdValues(myLocationDao.search(map)); -// assertThat(ids.size(), is(0)); + assertEquals(0, actual.getEntry().size()); } } From 74948737a7139fbddeddeb29220849da78284d3a Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Sat, 1 Feb 2020 20:18:42 -0500 Subject: [PATCH 05/10] pre-review cleanup --- .../src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java | 1 + .../main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java index 8c820ca77d5..91146696266 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java @@ -188,6 +188,7 @@ public class SearchBuilder implements ISearchBuilder { private void setLocationDistance(SearchParameterMap theParams) { if (myResourceType == Location.class && theParams.containsKey(Location.SP_NEAR_DISTANCE)) { List> paramList = theParams.get(Location.SP_NEAR_DISTANCE); + // Set nearDistanceParam on the SearchParameterMap so it's available to the near predicate paramList.stream() .flatMap(List::stream) .findFirst() diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java index d9ed8561105..f63b682f7a0 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java @@ -118,7 +118,6 @@ public class MatchUrlService { paramMap.add(nextParamName, param); } else if (Location.SP_NEAR_DISTANCE.equals(nextParamName)) { QuantityAndListParam nearDistanceAndListParam = (QuantityAndListParam) ParameterUtil.parseQueryParams(myContext, RestSearchParameterTypeEnum.QUANTITY, nextParamName, paramList); - // FIXME KHS this needs to be set elsewhere nearDistanceAndListParam.getValuesAsQueryTokens().stream() .map(QuantityOrListParam::getValuesAsQueryTokens) .flatMap(List::stream) From ffcf617659f0ce94f804c8708764938874419c39 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Sat, 1 Feb 2020 20:27:21 -0500 Subject: [PATCH 06/10] pre-review cleanup --- .../fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 cbdc24875df..a3cd3ea38af 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 @@ -4300,7 +4300,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { "&" + Location.SP_NEAR_DISTANCE + "=" + bigEnoughDistance + URLEncoder.encode("|http://unitsofmeasure.org|km"); - myCaptureQueriesListener.clear(); +// myCaptureQueriesListener.clear(); Bundle actual = ourClient .search() .byUrl(ourServerBase + "/" + url) @@ -4308,7 +4308,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { .prettyPrint() .returnBundle(Bundle.class) .execute(); - myCaptureQueriesListener.logSelectQueries(); +// myCaptureQueriesListener.logSelectQueries(); assertEquals(1, actual.getEntry().size()); assertEquals(locId.getIdPart(), actual.getEntry().get(0).getResource().getIdElement().getIdPart()); From decb47dfc25fb21799f30e69ca840ef14e02c83e Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Sun, 2 Feb 2020 16:56:25 -0500 Subject: [PATCH 07/10] remove unused block --- .../jpa/provider/dstu3/ResourceProviderDstu3Test.java | 2 -- .../ca/uhn/fhir/jpa/searchparam/MatchUrlService.java | 10 ---------- 2 files changed, 12 deletions(-) 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 a3cd3ea38af..47dfa7e9c47 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 @@ -4300,7 +4300,6 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { "&" + Location.SP_NEAR_DISTANCE + "=" + bigEnoughDistance + URLEncoder.encode("|http://unitsofmeasure.org|km"); -// myCaptureQueriesListener.clear(); Bundle actual = ourClient .search() .byUrl(ourServerBase + "/" + url) @@ -4308,7 +4307,6 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { .prettyPrint() .returnBundle(Bundle.class) .execute(); -// myCaptureQueriesListener.logSelectQueries(); assertEquals(1, actual.getEntry().size()); assertEquals(locId.getIdPart(), actual.getEntry().get(0).getResource().getIdElement().getIdPart()); diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java index f63b682f7a0..fb77ea8af24 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/MatchUrlService.java @@ -26,14 +26,11 @@ import ca.uhn.fhir.context.RuntimeSearchParam; import ca.uhn.fhir.jpa.searchparam.registry.ISearchParamRegistry; import ca.uhn.fhir.model.api.IQueryParameterAnd; import ca.uhn.fhir.model.api.IQueryParameterType; -import ca.uhn.fhir.model.dstu2.resource.Location; import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.QualifiedParamList; import ca.uhn.fhir.rest.api.RestSearchParameterTypeEnum; import ca.uhn.fhir.rest.param.DateRangeParam; import ca.uhn.fhir.rest.param.ParameterUtil; -import ca.uhn.fhir.rest.param.QuantityAndListParam; -import ca.uhn.fhir.rest.param.QuantityOrListParam; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.util.ReflectionUtil; import ca.uhn.fhir.util.UrlUtil; @@ -116,13 +113,6 @@ public class MatchUrlService { } else if (Constants.PARAM_SOURCE.equals(nextParamName)) { IQueryParameterAnd param = ParameterUtil.parseQueryParams(myContext, RestSearchParameterTypeEnum.TOKEN, nextParamName, paramList); paramMap.add(nextParamName, param); - } else if (Location.SP_NEAR_DISTANCE.equals(nextParamName)) { - QuantityAndListParam nearDistanceAndListParam = (QuantityAndListParam) ParameterUtil.parseQueryParams(myContext, RestSearchParameterTypeEnum.QUANTITY, nextParamName, paramList); - nearDistanceAndListParam.getValuesAsQueryTokens().stream() - .map(QuantityOrListParam::getValuesAsQueryTokens) - .flatMap(List::stream) - .findFirst() - .ifPresent(paramMap::setNearDistanceParam); } else if (nextParamName.startsWith("_")) { // ignore these since they aren't search params (e.g. _sort) } else { From c10d44327cffbf15af8dd17107d48898d327e158 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Sun, 2 Feb 2020 17:52:09 -0500 Subject: [PATCH 08/10] fix in-memory matcher --- .../fhir/jpa/searchparam/matcher/InMemoryResourceMatcher.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcher.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcher.java index c93c5e14ba8..d83eb5048c9 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcher.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/matcher/InMemoryResourceMatcher.java @@ -40,6 +40,7 @@ import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.util.MetaUtil; import ca.uhn.fhir.util.UrlUtil; +import org.hl7.fhir.dstu3.model.Location; import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; @@ -88,7 +89,7 @@ public class InMemoryResourceMatcher { if (searchParameterMap.getLastUpdated() != null) { return InMemoryMatchResult.unsupportedFromParameterAndReason(Constants.PARAM_LASTUPDATED, InMemoryMatchResult.STANDARD_PARAMETER); } - if (searchParameterMap.getNearDistanceParam() != null) { + if (searchParameterMap.containsKey(Location.SP_NEAR)) { return InMemoryMatchResult.unsupportedFromReason(InMemoryMatchResult.LOCATION_NEAR); } From 63f2f3868967f09e4200876118c07a716eeeded4 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Sun, 2 Feb 2020 21:21:37 -0500 Subject: [PATCH 09/10] reinstate near param count validation --- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 20 +++------------ .../jpa/searchparam/MatchUrlServiceTest.java | 6 ++++- .../jpa/searchparam/SearchParameterMap.java | 25 +++++++++++++++++++ 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java index 91146696266..07739e125ed 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java @@ -53,7 +53,6 @@ import ca.uhn.fhir.rest.api.SortSpec; import ca.uhn.fhir.rest.api.server.IPreResourceAccessDetails; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.param.DateRangeParam; -import ca.uhn.fhir.rest.param.QuantityParam; import ca.uhn.fhir.rest.param.ReferenceParam; import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; @@ -162,7 +161,9 @@ public class SearchBuilder implements ISearchBuilder { // Remove any empty parameters theParams.clean(); - setLocationDistance(theParams); + if (myResourceType == Location.class) { + theParams.setLocationDistance(); + } /* * Check if there is a unique key associated with the set @@ -185,21 +186,6 @@ public class SearchBuilder implements ISearchBuilder { } - private void setLocationDistance(SearchParameterMap theParams) { - if (myResourceType == Location.class && theParams.containsKey(Location.SP_NEAR_DISTANCE)) { - List> paramList = theParams.get(Location.SP_NEAR_DISTANCE); - // Set nearDistanceParam on the SearchParameterMap so it's available to the near predicate - paramList.stream() - .flatMap(List::stream) - .findFirst() - .map(QuantityParam.class::cast) - .ifPresent(theParams::setNearDistanceParam); - // Need to remove near-distance or it we'll get a hashcode predicate for it - theParams.remove(Location.SP_NEAR_DISTANCE); - } - } - - @Override public Iterator createCountQuery(SearchParameterMap theParams, String theSearchUuid, RequestDetails theRequest) { init(theParams, theSearchUuid); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/searchparam/MatchUrlServiceTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/searchparam/MatchUrlServiceTest.java index c12fad6d306..e79e49fe76e 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/searchparam/MatchUrlServiceTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/searchparam/MatchUrlServiceTest.java @@ -58,6 +58,7 @@ public class MatchUrlServiceTest extends BaseJpaTest { Location.SP_NEAR + "=1000.0:2000.0" + "&" + Location.SP_NEAR_DISTANCE + "=" + kmDistance + "|http://unitsofmeasure.org|km", ourCtx.getResourceDefinition("Location")); + map.setLocationDistance(); QuantityParam nearDistanceParam = map.getNearDistanceParam(); assertEquals(1, map.size()); @@ -74,6 +75,8 @@ public class MatchUrlServiceTest extends BaseJpaTest { "&" + Location.SP_NEAR_DISTANCE + "=2|http://unitsofmeasure.org|km", ourCtx.getResourceDefinition("Location")); + map.setLocationDistance(); + fail(); } catch (IllegalArgumentException e) { assertEquals("Only one " + Location.SP_NEAR_DISTANCE + " parameter may be present", e.getMessage()); @@ -89,7 +92,8 @@ public class MatchUrlServiceTest extends BaseJpaTest { "," + "2|http://unitsofmeasure.org|km", ourCtx.getResourceDefinition("Location")); - map.setLoadSynchronous(true); + map.setLocationDistance(); + fail(); } catch (IllegalArgumentException e) { assertEquals("Only one " + Location.SP_NEAR_DISTANCE + " parameter may be present", e.getMessage()); diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/SearchParameterMap.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/SearchParameterMap.java index a3a384936d4..958e58c2f1c 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/SearchParameterMap.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/SearchParameterMap.java @@ -15,6 +15,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.builder.ToStringBuilder; import org.apache.commons.lang3.builder.ToStringStyle; +import org.hl7.fhir.dstu3.model.Location; import java.io.Serializable; import java.util.*; @@ -504,6 +505,30 @@ public class SearchParameterMap implements Serializable { return myNearDistanceParam; } + public void setLocationDistance() { + if (containsKey(Location.SP_NEAR_DISTANCE)) { + List> paramAndList = get(Location.SP_NEAR_DISTANCE); + + if (paramAndList.isEmpty()) { + return; + } + if (paramAndList.size() > 1) { + throw new IllegalArgumentException("Only one " + ca.uhn.fhir.model.dstu2.resource.Location.SP_NEAR_DISTANCE + " parameter may be present"); + } + List paramOrList = paramAndList.get(0); + if (paramOrList.isEmpty()) { + return; + } + if (paramOrList.size() > 1) { + throw new IllegalArgumentException("Only one " + ca.uhn.fhir.model.dstu2.resource.Location.SP_NEAR_DISTANCE + " parameter may be present"); + } + setNearDistanceParam((QuantityParam) paramOrList.get(0)); + + // Need to remove near-distance or it we'll get a hashcode predicate for it + remove(Location.SP_NEAR_DISTANCE); + } + } + public enum EverythingModeEnum { /* * Don't reorder! We rely on the ordinals From 709aeb4ae8f7635a9c2ff4ea3606d66549b73d28 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Mon, 3 Feb 2020 10:29:39 -0500 Subject: [PATCH 10/10] Drop unnecessary index (#1696) --- .../ITermValueSetConceptDesignationDao.java | 4 ---- .../TermValueSetConceptDesignation.java | 4 +--- .../tasks/HapiFhirJpaMigrationTasks.java | 9 +++++++- .../fhir/jpa/migrate/tasks/api/Builder.java | 22 ++++++++++++++----- 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermValueSetConceptDesignationDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermValueSetConceptDesignationDao.java index cc84b88412a..49a1f0367da 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermValueSetConceptDesignationDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ITermValueSetConceptDesignationDao.java @@ -21,8 +21,6 @@ package ca.uhn.fhir.jpa.dao.data; */ import ca.uhn.fhir.jpa.entity.TermValueSetConceptDesignation; -import org.springframework.data.domain.Pageable; -import org.springframework.data.domain.Slice; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; @@ -37,6 +35,4 @@ public interface ITermValueSetConceptDesignationDao extends JpaRepository findByTermValueSetConceptId(Pageable thePage, @Param("pid") Long theValueSetConceptId); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermValueSetConceptDesignation.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermValueSetConceptDesignation.java index ea6582e49cf..bca66a3792b 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermValueSetConceptDesignation.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/TermValueSetConceptDesignation.java @@ -33,9 +33,7 @@ import java.io.Serializable; import static org.apache.commons.lang3.StringUtils.left; import static org.apache.commons.lang3.StringUtils.length; -@Table(name = "TRM_VALUESET_C_DESIGNATION", indexes = { - @Index(name = "IDX_VALUESET_C_DSGNTN_VAL", columnList = "VAL") -}) +@Table(name = "TRM_VALUESET_C_DESIGNATION") @Entity() public class TermValueSetConceptDesignation implements Serializable { private static final long serialVersionUID = 1L; diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java index 53489f0c607..f304bf98e8e 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java @@ -186,6 +186,10 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { // TermConceptProperty version.startSectionWithMessage("Processing table: TRM_CONCEPT_PROPERTY"); version.onTable("TRM_CONCEPT_PROPERTY").addColumn("20191002.9", "PROP_VAL_LOB").nullable().type(BaseTableColumnTypeTask.ColumnTypeEnum.BLOB); + + // TermValueSetConceptDesignation + version.onTable("TRM_VALUESET_C_DESIGNATION").dropIndex("20200202.1", "IDX_VALUESET_C_DSGNTN_VAL").failureAllowed(); + } protected void init400() { // 20190401 - 20190814 @@ -324,10 +328,13 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { termValueSetConceptDesignationTable.addColumn("USE_CODE").nullable().type(BaseTableColumnTypeTask.ColumnTypeEnum.STRING, 500); termValueSetConceptDesignationTable.addColumn("USE_DISPLAY").nullable().type(BaseTableColumnTypeTask.ColumnTypeEnum.STRING, 500); termValueSetConceptDesignationTable.addColumn("VAL").nonNullable().type(BaseTableColumnTypeTask.ColumnTypeEnum.STRING, 500); + + // This index turned out not to be needed so it is disabled termValueSetConceptDesignationTable .addIndex("20190801.6", "IDX_VALUESET_C_DSGNTN_VAL") .unique(false) - .withColumns("VAL"); + .withColumns("VAL") + .doNothing(); // TermCodeSystemVersion version.startSectionWithMessage("Processing table: TRM_CODESYSTEM_VER"); diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/Builder.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/Builder.java index a7de3803e1d..ea6431e8fbb 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/Builder.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/Builder.java @@ -158,20 +158,22 @@ public class Builder { return myTableName; } - public void dropIndex(String theVersion, String theIndexName) { - dropIndexOptional(false, theVersion, theIndexName); + public BuilderCompleteTask dropIndex(String theVersion, String theIndexName) { + BaseTask task = dropIndexOptional(false, theVersion, theIndexName); + return new BuilderCompleteTask(task); } public void dropIndexStub(String theVersion, String theIndexName) { dropIndexOptional(true, theVersion, theIndexName); } - private void dropIndexOptional(boolean theDoNothing, String theVersion, String theIndexName) { + private DropIndexTask dropIndexOptional(boolean theDoNothing, String theVersion, String theIndexName) { DropIndexTask task = new DropIndexTask(myRelease, theVersion); task.setIndexName(theIndexName); task.setTableName(myTableName); task.setDoNothing(theDoNothing); addTask(task); + return task; } public void renameIndex(String theVersion, String theOldIndexName, String theNewIndexName) { @@ -286,11 +288,12 @@ public class Builder { withColumnsOptional(true, theColumnNames); } - public void withColumns(String... theColumnNames) { - withColumnsOptional(false, theColumnNames); + public BuilderCompleteTask withColumns(String... theColumnNames) { + BaseTask task = withColumnsOptional(false, theColumnNames); + return new BuilderCompleteTask(task); } - private void withColumnsOptional(boolean theDoNothing, String... theColumnNames) { + private AddIndexTask withColumnsOptional(boolean theDoNothing, String... theColumnNames) { AddIndexTask task = new AddIndexTask(myRelease, myVersion); task.setTableName(myTableName); task.setIndexName(myIndexName); @@ -298,6 +301,7 @@ public class Builder { task.setColumns(theColumnNames); task.setDoNothing(theDoNothing); addTask(task); + return task; } } } @@ -463,6 +467,12 @@ public class Builder { myTask.setFailureAllowed(true); return this; } + + public BuilderCompleteTask doNothing() { + myTask.setDoNothing(true); + return this; + } + } }