Skip to content

Add bbox parameter #364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Photon software is open source and licensed under [Apache License, Version 2.0](
- location bias
- typo tolerance
- filter by osm tag and value
- filter by bounding box
- reverse geocode a coordinate to an address
- OSM data import (built upon [Nominatim](https://github.com/twain47/Nominatim)) inclusive continuous updates

Expand Down Expand Up @@ -132,6 +133,12 @@ http://localhost:2322/api?q=berlin&limit=2
http://localhost:2322/api?q=berlin&lang=it
```

#### Filter results by bounding box
Expected format is minLon,minLat,maxLon,maxLat.
```
http://localhost:2322/api?q=berlin&bbox=9.5,51.5,11.5,53.5
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be important to mention which order the bbox has. Maybe we should also use a different order with left,bottom and right,top points ala latSouth,lonWest,latNorth,lonEast or why did you pick this order?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, this should be documented. Concerning the order: to me this was the one I‘m used to, but Nomination and Overpass both use a different order.
Should I change it?
And before merging, I should add a verification of the lat/lon limits.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I change it?

Up, not necessary. I confused the APIs ;)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you change the order now? Because I read from the code below that you expect <minLon, maxLon, minLat, maxLat>. My preference would be as documented: <minLon,minLat,maxLon,maxLat>. (Or to be precise: <x1,y1,x2,y2>, Envelope() does not seem to care if it is min,max or max,min.)

Just to mention it, the odd boundingbox format of Nominatim is only used for output. The viewbox input parameter also expects <minLon,minLat,maxLon,maxLat>.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you have a preference then we should do x1,y1,x2,y2

```

#### Filter results by [tags and values](http://taginfo.openstreetmap.org/projects/nominatim#tags)
*Note: not all tags on [link in the title](http://taginfo.openstreetmap.org/projects/nominatim#tags) are supported. Please see [nominatim source](https://github.com/openstreetmap/osm2pgsql/blob/master/output-gazetteer.cpp#L81) for an accurate list.*
If one or many query parameters named ```osm_tag``` are present, photon will attempt to filter results by those tags. In general, here is the expected format (syntax) for the value of osm_tag request parameters.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package de.komoot.photon.query;

import com.vividsolutions.jts.geom.Envelope;

import de.komoot.photon.utils.Function;
import spark.Request;

/**
* Converts the bbox parameter into an Envelope and performs format checking.
* Created by Holger Bruch on 10/13/2018.
*/
public class BoundingBoxParamConverter implements Function<Request, Envelope, BadRequestException> {

private static final String INVALID_BBOX_ERROR_MESSAGE = "invalid search term 'bbox', expected format is: minLon,minLat,maxLon,maxLat";

@Override
public Envelope apply(Request webRequest) throws BadRequestException {
String bboxParam = webRequest.queryParams("bbox");
if (bboxParam == null) {
return null;
}
Envelope bbox = null;
try {
String[] bboxCoords = bboxParam.split(",");
if (bboxCoords.length != 4) {
throw new BadRequestException(400, INVALID_BBOX_ERROR_MESSAGE);
}
Double minLon = Double.valueOf(bboxCoords[0]);
Double minLat = Double.valueOf(bboxCoords[2]);
Double maxLon = Double.valueOf(bboxCoords[1]);
Double maxLat = Double.valueOf(bboxCoords[3]);
if (minLon > 180.0 || minLon < -180.00 || maxLon > 180.0 || maxLon < -180.00) {
throw new BadRequestException(400, INVALID_BBOX_ERROR_MESSAGE);
}
if (minLat > 90.0 || minLat < -90.00 || maxLat > 90.0 || maxLat < -90.00) {
throw new BadRequestException(400, INVALID_BBOX_ERROR_MESSAGE);
}
bbox = new Envelope(minLon, minLat, maxLon, maxLat);
} catch (NumberFormatException nfe) {
throw new BadRequestException(400, INVALID_BBOX_ERROR_MESSAGE);
}

return bbox;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package de.komoot.photon.query;

import com.vividsolutions.jts.geom.Envelope;
import com.vividsolutions.jts.geom.Point;

import java.util.HashMap;
Expand All @@ -20,8 +21,8 @@ public class FilteredPhotonRequest extends PhotonRequest {
private Map<String, Set<String>> excludeTags = new HashMap<String, Set<String>>(3);
private Map<String, Set<String>> excludeTagValues = new HashMap<String, Set<String>>(3);

FilteredPhotonRequest(String query, int limit, Point locationForBias, double locBiasScale, String language) {
super(query, limit, locationForBias, locBiasScale, language);
FilteredPhotonRequest(String query, int limit, Envelope bbox, Point locationForBias, double locBiasScale, String language) {
super(query, limit, bbox, locationForBias, locBiasScale, language);
}

public Set<String> keys() {
Expand Down
15 changes: 15 additions & 0 deletions src/main/java/de/komoot/photon/query/PhotonQueryBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@


import com.google.common.collect.ImmutableSet;
import com.vividsolutions.jts.geom.Envelope;
import com.vividsolutions.jts.geom.Point;
import org.elasticsearch.common.lucene.search.function.CombineFunction;
import org.elasticsearch.common.lucene.search.function.FiltersFunctionScoreQuery.ScoreMode;
Expand Down Expand Up @@ -51,6 +52,8 @@ public class PhotonQueryBuilder implements TagFilterQueryBuilder {

private MatchQueryBuilder languageMatchQueryBuilder;

private GeoBoundingBoxQueryBuilder bboxQueryBuilder;

private QueryBuilder m_finalQueryBuilder;

protected ArrayList<FilterFunctionBuilder> m_alFilterFunction4QueryBuilder = new ArrayList<>(1);
Expand Down Expand Up @@ -133,6 +136,15 @@ public TagFilterQueryBuilder withLocationBias(Point point, double scale) {
.boostMode(CombineFunction.MULTIPLY);
return this;
}

@Override
public TagFilterQueryBuilder withBoundingBox(Envelope bbox) {
if (bbox == null) return this;
bboxQueryBuilder = new GeoBoundingBoxQueryBuilder("coordinate");
bboxQueryBuilder.setCorners(bbox.getMaxY(), bbox.getMinX(), bbox.getMinY(), bbox.getMaxX());

return this;
}

@Override
public TagFilterQueryBuilder withTags(Map<String, Set<String>> tags) {
Expand Down Expand Up @@ -314,6 +326,9 @@ public QueryBuilder buildQuery() {
m_queryBuilderForTopLevelFilter.must(andQueryBuilderForExcludeTagFiltering);

}

if (bboxQueryBuilder != null)
m_queryBuilderForTopLevelFilter.filter(bboxQueryBuilder);

state = State.FINISHED;

Expand Down
9 changes: 8 additions & 1 deletion src/main/java/de/komoot/photon/query/PhotonRequest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package de.komoot.photon.query;

import com.vividsolutions.jts.geom.Envelope;
import com.vividsolutions.jts.geom.Point;

import java.io.Serializable;
Expand All @@ -13,13 +14,15 @@ public class PhotonRequest implements Serializable {
private Point locationForBias;
private String language;
private final double scale;
private Envelope bbox;

public PhotonRequest(String query, int limit, Point locationForBias, double scale, String language) {
public PhotonRequest(String query, int limit, Envelope bbox, Point locationForBias, double scale, String language) {
this.query = query;
this.limit = limit;
this.locationForBias = locationForBias;
this.scale = scale;
this.language = language;
this.bbox = bbox;
}

public String getQuery() {
Expand All @@ -29,6 +32,10 @@ public String getQuery() {
public Integer getLimit() {
return limit;
}

public Envelope getBbox() {
return bbox;
}

public Point getLocationForBias() {
return locationForBias;
Expand Down
14 changes: 10 additions & 4 deletions src/main/java/de/komoot/photon/query/PhotonRequestFactory.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package de.komoot.photon.query;

import com.vividsolutions.jts.geom.Coordinate;
import com.vividsolutions.jts.geom.Envelope;
import com.vividsolutions.jts.geom.GeometryFactory;
import com.vividsolutions.jts.geom.Point;
import com.vividsolutions.jts.geom.PrecisionModel;
Expand All @@ -17,13 +18,16 @@
*/
public class PhotonRequestFactory {
private final LanguageChecker languageChecker;
private final BoundingBoxParamConverter bboxParamConverter;

private final static GeometryFactory geometryFactory = new GeometryFactory(new PrecisionModel(), 4326);

protected static HashSet<String> m_hsRequestQueryParams = new HashSet<>(Arrays.asList("lang", "q", "lon", "lat",
"limit", "osm_tag", "location_bias_scale"));
"bbox", "limit", "osm_tag", "location_bias_scale"));

public PhotonRequestFactory(Set<String> supportedLanguages) {
this.languageChecker = new LanguageChecker(supportedLanguages);
this.bboxParamConverter = new BoundingBoxParamConverter();
}

public <R extends PhotonRequest> R create(Request webRequest) throws BadRequestException {
Expand Down Expand Up @@ -53,7 +57,9 @@ public <R extends PhotonRequest> R create(Request webRequest) throws BadRequestE
} catch (Exception nfe) {
//ignore
}


Envelope bbox = bboxParamConverter.apply(webRequest);

// don't use too high default value, see #306
double scale = 1.6;
String scaleStr = webRequest.queryParams("location_bias_scale");
Expand All @@ -66,9 +72,9 @@ public <R extends PhotonRequest> R create(Request webRequest) throws BadRequestE

QueryParamsMap tagFiltersQueryMap = webRequest.queryMap("osm_tag");
if (!new CheckIfFilteredRequest().execute(tagFiltersQueryMap)) {
return (R) new PhotonRequest(query, limit, locationForBias, scale, language);
return (R) new PhotonRequest(query, limit, bbox, locationForBias, scale, language);
}
FilteredPhotonRequest photonRequest = new FilteredPhotonRequest(query, limit, locationForBias, scale, language);
FilteredPhotonRequest photonRequest = new FilteredPhotonRequest(query, limit, bbox, locationForBias, scale, language);
String[] tagFilters = tagFiltersQueryMap.values();
setUpTagFilters(photonRequest, tagFilters);

Expand Down
6 changes: 6 additions & 0 deletions src/main/java/de/komoot/photon/query/ReverseQueryBuilder.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package de.komoot.photon.query;

import com.google.common.collect.ImmutableSet;
import com.vividsolutions.jts.geom.Envelope;
import com.vividsolutions.jts.geom.Point;
import org.elasticsearch.common.unit.DistanceUnit;
import org.elasticsearch.index.query.BoolQueryBuilder;
Expand Down Expand Up @@ -45,6 +46,11 @@ public TagFilterQueryBuilder withLocationBias(Point point, double scale) {
throw new RuntimeException(new NoSuchMethodException("this method is not implemented (NOOP)"));
}

@Override
public TagFilterQueryBuilder withBoundingBox(Envelope bbox) {
throw new RuntimeException(new NoSuchMethodException("this method is not implemented (NOOP)"));
}

@Override
public TagFilterQueryBuilder withTags(Map<String, Set<String>> tags) {
throw new RuntimeException(new NoSuchMethodException("this method is not implemented (NOOP)"));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package de.komoot.photon.query;

import com.vividsolutions.jts.geom.Envelope;
import com.vividsolutions.jts.geom.Point;
import org.elasticsearch.index.query.QueryBuilder;

Expand All @@ -21,6 +22,13 @@ public interface TagFilterQueryBuilder {
*/
TagFilterQueryBuilder withLimit(Integer limit);

/**
* Search results will be filtered to places contained in the bounding box (WGS84) provided in the argument.
*
* @param bbox Geographical {@link Envelope}
*/
TagFilterQueryBuilder withBoundingBox(Envelope bbox);

/**
* Location bias for query. By setting this, places found will be sorted by proximity to this point.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ public TagFilterQueryBuilder buildQuery(FilteredPhotonRequest photonRequest) {
withoutKeys(excludeKeys).
withoutValues(excludeValues).
withTagsNotValues(excludeTagValues).
withLocationBias(photonRequest.getLocationForBias(), photonRequest.getScaleForBias());
withLocationBias(photonRequest.getLocationForBias(), photonRequest.getScaleForBias()).
withBoundingBox(photonRequest.getBbox());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public SimplePhotonRequestHandler(ElasticsearchSearcher elasticsearchSearcher) {
@Override
public TagFilterQueryBuilder buildQuery(PhotonRequest photonRequest) {
return PhotonQueryBuilder.builder(photonRequest.getQuery(), photonRequest.getLanguage()).
withLocationBias(photonRequest.getLocationForBias(), photonRequest.getScaleForBias());
withLocationBias(photonRequest.getLocationForBias(), photonRequest.getScaleForBias()).
withBoundingBox(photonRequest.getBbox());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public class FilteredPhotonRequestTest {

@Test
public void testNotKey() {
FilteredPhotonRequest filteredPhotonRequest = new FilteredPhotonRequest(null, 0, null, 0, null);
FilteredPhotonRequest filteredPhotonRequest = new FilteredPhotonRequest(null, 0, null, null, 0, null);
filteredPhotonRequest.notKeys("exclude");
filteredPhotonRequest.notKeys("exclude");
filteredPhotonRequest.notKeys("anotherExclude");
Expand All @@ -27,7 +27,7 @@ public void testNotKey() {

@Test
public void testNotTag() {
FilteredPhotonRequest filteredPhotonRequest = new FilteredPhotonRequest(null, 0, null, 0, null);
FilteredPhotonRequest filteredPhotonRequest = new FilteredPhotonRequest(null, 0, null, null, 0, null);
filteredPhotonRequest.notTags("aKey", ImmutableSet.of("aValue"));
filteredPhotonRequest.notTags("anotherKey", ImmutableSet.of("anotherValue"));
Map<String, Set<String>> excludeTags = filteredPhotonRequest.notTags();
Expand All @@ -37,7 +37,7 @@ public void testNotTag() {

@Test
public void testNotValue() {
FilteredPhotonRequest filteredPhotonRequest = new FilteredPhotonRequest(null, 0, null, 0, null);
FilteredPhotonRequest filteredPhotonRequest = new FilteredPhotonRequest(null, 0, null, null, 0, null);
filteredPhotonRequest.notValues("exclude");
filteredPhotonRequest.notValues("exclude");
filteredPhotonRequest.notValues("anotherExclude");
Expand All @@ -48,7 +48,7 @@ public void testNotValue() {

@Test
public void testKey() {
FilteredPhotonRequest filteredPhotonRequest = new FilteredPhotonRequest(null, 0, null, 0, null);
FilteredPhotonRequest filteredPhotonRequest = new FilteredPhotonRequest(null, 0, null, null, 0, null);
filteredPhotonRequest.keys("keyToInclude");
filteredPhotonRequest.keys("keyToInclude");
filteredPhotonRequest.keys("anotherKeyToInclude");
Expand All @@ -57,7 +57,7 @@ public void testKey() {

@Test
public void testTag() {
FilteredPhotonRequest filteredPhotonRequest = new FilteredPhotonRequest(null, 0, null, 0, null);
FilteredPhotonRequest filteredPhotonRequest = new FilteredPhotonRequest(null, 0, null, null, 0, null);
filteredPhotonRequest.tags("aKey", ImmutableSet.of("aValue"));
filteredPhotonRequest.tags("anotherKey", ImmutableSet.of("anotherValue"));
Map<String, Set<String>> includeTags = filteredPhotonRequest.tags();
Expand All @@ -68,7 +68,7 @@ public void testTag() {

@Test
public void testValue() {
FilteredPhotonRequest filteredPhotonRequest = new FilteredPhotonRequest(null, 0, null, 0, null);
FilteredPhotonRequest filteredPhotonRequest = new FilteredPhotonRequest(null, 0, null, null, 0, null);
filteredPhotonRequest.values("keyToInclude");
filteredPhotonRequest.values("keyToInclude");
filteredPhotonRequest.values("anotherKeyToInclude");
Expand Down
37 changes: 37 additions & 0 deletions src/test/java/de/komoot/photon/query/PhotonRequestFactoryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.vividsolutions.jts.geom.Envelope;

import org.junit.Assert;
import org.junit.Test;
import org.mockito.Mockito;
Expand Down Expand Up @@ -197,4 +199,39 @@ public void testWithExcludeValueFilter() throws Exception {
Mockito.verify(mockOsmTagQueryParm, Mockito.times(2)).values();
Assert.assertEquals(ImmutableSet.of("aValue"), filteredPhotonRequest.notValues());
}

@Test
public void testWithBboxFilter() throws Exception {
Request mockRequest = Mockito.mock(Request.class);
Mockito.when(mockRequest.queryParams("q")).thenReturn("hanover");
QueryParamsMap mockQueryParamsMap = Mockito.mock(QueryParamsMap.class);
Mockito.when(mockRequest.queryMap("osm_tag")).thenReturn(mockQueryParamsMap);
Mockito.when(mockRequest.queryParams("bbox")).thenReturn("9.6,52.4,9.8,52.3");

PhotonRequestFactory photonRequestFactory = new PhotonRequestFactory(ImmutableSet.of("en"));
PhotonRequest photonRequest = photonRequestFactory.create(mockRequest);

Mockito.verify(mockRequest, Mockito.times(1)).queryParams("bbox");
Assert.assertEquals(new Envelope(9.6, 9.8, 52.3, 52.4), photonRequest.getBbox());
}

@Test
public void testWithBadBboxFilter() throws Exception {
Request mockRequest = Mockito.mock(Request.class);
Mockito.when(mockRequest.queryParams("q")).thenReturn("hanover");
QueryParamsMap mockQueryParamsMap = Mockito.mock(QueryParamsMap.class);
Mockito.when(mockRequest.queryMap("osm_tag")).thenReturn(mockQueryParamsMap);
Mockito.when(mockRequest.queryParams("bbox")).thenReturn("9.6,91,9.8,93");

try {
PhotonRequestFactory photonRequestFactory = new PhotonRequestFactory(ImmutableSet.of("en"));
photonRequestFactory.create(mockRequest);
Assert.fail();
} catch (BadRequestException e) {
Assert.assertEquals("invalid search term 'bbox', expected format is: minLon,minLat,maxLon,maxLat", e.getMessage());
}
Mockito.verify(mockRequest, Mockito.times(1)).queryParams("bbox");
}


}