diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderCoords.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderCoords.java index f82ddf60c24..6a30f8fecd4 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderCoords.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderCoords.java @@ -10,6 +10,7 @@ import ca.uhn.fhir.model.dstu2.resource.Location; import ca.uhn.fhir.rest.param.QuantityParam; import ca.uhn.fhir.rest.param.SpecialParam; import ca.uhn.fhir.rest.param.TokenParam; +import com.google.common.annotations.VisibleForTesting; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.context.annotation.Scope; @@ -94,24 +95,34 @@ public class PredicateBuilderCoords extends BasePredicateBuilder implements IPre double longitudeDegrees = Double.parseDouble(longitudeValue); SearchBox box = CoordCalculator.getBox(latitudeDegrees, longitudeDegrees, distanceKm); - - // FIXME KHS - ourLog.info("Searching for {} =< latitude <= {}", box.getSouthWest().getLatitude(), box.getNorthEast().getLatitude()); - latitudePredicate = theBuilder.and( - theBuilder.greaterThanOrEqualTo(theFrom.get("myLatitude"), box.getSouthWest().getLatitude()), - theBuilder.lessThanOrEqualTo(theFrom.get("myLatitude"), box.getNorthEast().getLatitude()) - ); - // FIXME KHS - ourLog.info("Searching for {} =< longitude <= {}", box.getSouthWest().getLongitude(), box.getNorthEast().getLongitude()); - longitudePredicate = theBuilder.and( - theBuilder.greaterThanOrEqualTo(theFrom.get("myLongitude"), box.getSouthWest().getLongitude()), - theBuilder.lessThanOrEqualTo(theFrom.get("myLongitude"), box.getNorthEast().getLongitude()) - ); + latitudePredicate = latitudePredicateFromBox(theBuilder, theFrom, box); + longitudePredicate = longitudePredicateFromBox(theBuilder, theFrom, box); } Predicate singleCode = theBuilder.and(latitudePredicate, longitudePredicate); return combineParamIndexPredicateWithParamNamePredicate(theResourceName, theParamName, theFrom, singleCode); } + private Predicate latitudePredicateFromBox(CriteriaBuilder theBuilder, From theFrom, SearchBox theBox) { + return theBuilder.and( + theBuilder.greaterThanOrEqualTo(theFrom.get("myLatitude"), theBox.getSouthWest().getLatitude()), + theBuilder.lessThanOrEqualTo(theFrom.get("myLatitude"), theBox.getNorthEast().getLatitude()) + ); + } + + @VisibleForTesting + Predicate longitudePredicateFromBox(CriteriaBuilder theBuilder, From theFrom, SearchBox theBox) { + if (theBox.crossesAntiMeridian()) { + return theBuilder.or( + theBuilder.greaterThanOrEqualTo(theFrom.get("myLongitude"), theBox.getNorthEast().getLongitude()), + theBuilder.lessThanOrEqualTo(theFrom.get("myLongitude"), theBox.getSouthWest().getLongitude()) + ); + } + return theBuilder.and( + theBuilder.greaterThanOrEqualTo(theFrom.get("myLongitude"), theBox.getSouthWest().getLongitude()), + theBuilder.lessThanOrEqualTo(theFrom.get("myLongitude"), theBox.getNorthEast().getLongitude()) + ); + } + @Override public Predicate addPredicate(String theResourceName, String theParamName, diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/CoordCalculator.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/CoordCalculator.java index abcbc4649b5..093840b54f0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/CoordCalculator.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/CoordCalculator.java @@ -10,8 +10,8 @@ public class CoordCalculator { // Source: https://stackoverflow.com/questions/7222382/get-lat-long-given-current-point-distance-and-bearing static Point findTarget(double theLatitudeDegrees, double theLongitudeDegrees, double theBearingDegrees, double theDistanceKm) { - double latitudeRadians = Math.toRadians(theLatitudeDegrees); - double longitudeRadians = Math.toRadians(theLongitudeDegrees); + double latitudeRadians = Math.toRadians(Point.normalizeLatitude(theLatitudeDegrees)); + double longitudeRadians = Math.toRadians(Point.normalizeLongitude(theLongitudeDegrees)); double bearingRadians = Math.toRadians(theBearingDegrees); double distanceRadians = theDistanceKm / RADIUS_EARTH_KM; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/SearchBox.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/SearchBox.java index e55746cfaf0..833d4be0fe2 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/SearchBox.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/SearchBox.java @@ -18,4 +18,8 @@ public class SearchBox { public Point getNorthEast() { return myNorthEast; } + + public boolean crossesAntiMeridian() { + return myNorthEast.getLongitude() < mySouthWest.getLongitude(); + } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderCoordsTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderCoordsTest.java new file mode 100644 index 00000000000..2da1fb75d06 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/predicate/PredicateBuilderCoordsTest.java @@ -0,0 +1,86 @@ +package ca.uhn.fhir.jpa.dao.predicate; + +import ca.uhn.fhir.jpa.dao.SearchBuilder; +import ca.uhn.fhir.jpa.util.CoordCalculator; +import ca.uhn.fhir.jpa.util.CoordCalculatorTest; +import ca.uhn.fhir.jpa.util.SearchBox; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.junit.MockitoJUnitRunner; + +import javax.persistence.criteria.CriteriaBuilder; +import javax.persistence.criteria.From; +import javax.persistence.criteria.Predicate; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.lessThan; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.*; + +@RunWith(MockitoJUnitRunner.class) +public class PredicateBuilderCoordsTest { + PredicateBuilderCoords myPredicateBuilderCoords; + private SearchBuilder mySearchBuilder; + private CriteriaBuilder myBuilder; + private From myFrom; + + @Before + public void before() { + mySearchBuilder = mock(SearchBuilder.class); + myBuilder = mock(CriteriaBuilder.class); + myFrom = mock(From.class); + myPredicateBuilderCoords = new PredicateBuilderCoords(mySearchBuilder); + } + + @Test + public void testLongitudePredicateFromBox() { + SearchBox box = CoordCalculator.getBox(CoordCalculatorTest.LATITUDE_CHIN, CoordCalculatorTest.LONGITUDE_CHIN, CoordCalculatorTest.DISTANCE_TAVEUNI); + assertThat(box.getNorthEast().getLongitude(), greaterThan(box.getSouthWest().getLongitude())); + + ArgumentCaptor andLeft = ArgumentCaptor.forClass(Predicate.class); + ArgumentCaptor andRight = ArgumentCaptor.forClass(Predicate.class); + + ArgumentCaptor gteValue = ArgumentCaptor.forClass(Double.class); + ArgumentCaptor lteValue = ArgumentCaptor.forClass(Double.class); + + Predicate gte = mock(Predicate.class); + Predicate lte = mock(Predicate.class); + when(myBuilder.greaterThanOrEqualTo(any(), gteValue.capture())).thenReturn(gte); + when(myBuilder.lessThanOrEqualTo(any(),lteValue.capture())).thenReturn(lte); + myPredicateBuilderCoords.longitudePredicateFromBox(myBuilder, myFrom, box); + verify(myBuilder).and(andLeft.capture(), andRight.capture()); + assertEquals(andLeft.getValue(), gte); + assertEquals(andRight.getValue(), lte); + assertEquals(gteValue.getValue(), box.getSouthWest().getLongitude()); + assertEquals(lteValue.getValue(), box.getNorthEast().getLongitude()); + } + + @Test + public void testAntiMeridianLongitudePredicateFromBox() { + SearchBox box = CoordCalculator.getBox(CoordCalculatorTest.LATITUDE_TAVEUNI, CoordCalculatorTest.LONGITIDE_TAVEUNI, CoordCalculatorTest.DISTANCE_TAVEUNI); + assertThat(box.getNorthEast().getLongitude(), lessThan(box.getSouthWest().getLongitude())); + assertTrue(box.crossesAntiMeridian()); + + ArgumentCaptor orLeft = ArgumentCaptor.forClass(Predicate.class); + ArgumentCaptor orRight = ArgumentCaptor.forClass(Predicate.class); + + ArgumentCaptor gteValue = ArgumentCaptor.forClass(Double.class); + ArgumentCaptor lteValue = ArgumentCaptor.forClass(Double.class); + + Predicate gte = mock(Predicate.class); + Predicate lte = mock(Predicate.class); + when(myBuilder.greaterThanOrEqualTo(any(), gteValue.capture())).thenReturn(gte); + when(myBuilder.lessThanOrEqualTo(any(),lteValue.capture())).thenReturn(lte); + myPredicateBuilderCoords.longitudePredicateFromBox(myBuilder, myFrom, box); + verify(myBuilder).or(orLeft.capture(), orRight.capture()); + assertEquals(orLeft.getValue(), gte); + assertEquals(orRight.getValue(), lte); + assertEquals(gteValue.getValue(), box.getNorthEast().getLongitude()); + assertEquals(lteValue.getValue(), box.getSouthWest().getLongitude()); + } + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java index c45b2a38d35..a5f61eb67e8 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java @@ -4195,6 +4195,28 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test { } + @Test + public void testNearSearchApproximateNearAntiMeridian() { + Location loc = new Location(); + double latitude = CoordCalculatorTest.LATITUDE_TAVEUNI; + double longitude = CoordCalculatorTest.LONGITIDE_TAVEUNI; + Location.LocationPositionComponent position = new Location.LocationPositionComponent().setLatitude(latitude).setLongitude(longitude); + loc.setPosition(position); + String locId = myLocationDao.create(loc).getId().toUnqualifiedVersionless().getValue(); + + { // We match even when the box crosses the anti-meridian + double bigEnoughDistance = CoordCalculatorTest.DISTANCE_TAVEUNI; + SearchParameterMap map = myMatchUrlService.translateMatchUrl( + "Location?" + + Location.SP_NEAR + "=" + CoordCalculatorTest.LATITUDE_TAVEUNI + "|" + + CoordCalculatorTest.LONGITIDE_TAVEUNI + "|" + + bigEnoughDistance, myFhirCtx.getResourceDefinition("Location")); + + List ids = toUnqualifiedVersionlessIdValues(myLocationDao.search(map)); + assertThat(ids, contains(locId)); + } + } + private String toStringMultiline(List theResults) { StringBuilder b = new StringBuilder(); for (Object next : theResults) { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/CoordCalculatorTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/CoordCalculatorTest.java index a284cebfd07..893ad55795b 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/CoordCalculatorTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/CoordCalculatorTest.java @@ -18,6 +18,12 @@ public class CoordCalculatorTest { public static final double DISTANCE_KM_CHIN_TO_UHN = 1.478; public static final double BEARING_CHIN_TO_UHN = 82 + (55.0 / 60) + (46.0 / 3600); + // A Fiji island near the anti-meridian + public static final double LATITUDE_TAVEUNI = -16.8488893; + public static final double LONGITIDE_TAVEUNI = 179.889793; + // enough distance from point to cross anti-meridian + public static final double DISTANCE_TAVEUNI = 100.0; + @Test public void testCHINToUHN() { Point result = CoordCalculator.findTarget(LATITUDE_CHIN, LONGITUDE_CHIN, BEARING_CHIN_TO_UHN, DISTANCE_KM_CHIN_TO_UHN); @@ -51,15 +57,14 @@ public class CoordCalculatorTest { @Test public void testOnAntiMeridian() { - double antiMeridianLongitide = 180.0; - SearchBox box = CoordCalculator.getBox(LATITUDE_CHIN, antiMeridianLongitide, 1.0); - double expectedLatitudeDelta = 0.0090; - assertEquals(LATITUDE_CHIN - expectedLatitudeDelta, box.getSouthWest().getLatitude(), 0.0001); - assertEquals(LATITUDE_CHIN + expectedLatitudeDelta, box.getNorthEast().getLatitude(), 0.0001); - double expectedLongitudeDelta = 0.012414; - assertEquals(antiMeridianLongitide - expectedLongitudeDelta, box.getSouthWest().getLongitude(), 0.0001); + SearchBox box = CoordCalculator.getBox(LATITUDE_TAVEUNI, LONGITIDE_TAVEUNI, 100.0); + double expectedLatitudeDelta = 0.90; + assertEquals(LATITUDE_TAVEUNI - expectedLatitudeDelta, box.getSouthWest().getLatitude(), 0.01); + assertEquals(LATITUDE_TAVEUNI + expectedLatitudeDelta, box.getNorthEast().getLatitude(), 0.01); + double expectedLongitudeDelta = 0.94; + assertEquals(LONGITIDE_TAVEUNI - expectedLongitudeDelta, box.getSouthWest().getLongitude(), 0.01); // This case wraps - assertEquals(antiMeridianLongitide + expectedLongitudeDelta - 360.0, box.getNorthEast().getLongitude(), 0.0001); + assertEquals(LONGITIDE_TAVEUNI + expectedLongitudeDelta - 360.0, box.getNorthEast().getLongitude(), 0.01); } } diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java index 750b6188b65..79bf1c41f4a 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/extractor/BaseSearchParamExtractor.java @@ -29,6 +29,7 @@ import ca.uhn.fhir.model.primitive.BoundCodeDt; import ca.uhn.fhir.rest.api.RestSearchParameterTypeEnum; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import org.apache.commons.lang3.ObjectUtils; +import org.hibernate.search.spatial.impl.Point; import org.hl7.fhir.exceptions.FHIRException; import org.hl7.fhir.instance.model.api.*; import org.springframework.beans.factory.annotation.Autowired; @@ -708,7 +709,9 @@ public abstract class BaseSearchParamExtractor implements ISearchParamExtractor } // We only accept coordinates when both are present if (latitude != null && longitude != null) { - ResourceIndexedSearchParamCoords nextEntity = new ResourceIndexedSearchParamCoords(theResourceType, theSearchParam.getName(), latitude.doubleValue(), longitude.doubleValue()); + double normalizedLatitude = Point.normalizeLatitude(latitude.doubleValue()); + double normalizedLongitude = Point.normalizeLongitude(longitude.doubleValue()); + ResourceIndexedSearchParamCoords nextEntity = new ResourceIndexedSearchParamCoords(theResourceType, theSearchParam.getName(), normalizedLatitude, normalizedLongitude); theParams.add(nextEntity); } }