From f2e12ab06c02c4a1dd76e22d2930a97ee7b9d19b Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 18 Dec 2013 13:53:21 +0100 Subject: [PATCH] Make doc lookup in geo_shape filter and query consistent with terms lookup. The `geo_shape filter and query` option in geo_shape filter and query has been replaced with the `path` option, which allows these filter and query to fetch shapes from within objects as well. Closes #4486 --- .../index/query/GeoShapeFilterBuilder.java | 18 ++--- .../index/query/GeoShapeFilterParser.java | 10 +-- .../index/query/GeoShapeQueryBuilder.java | 14 ++-- .../index/query/GeoShapeQueryParser.java | 8 +- .../index/search/shape/ShapeFetchService.java | 22 ++++-- .../search/geo/GeoShapeIntegrationTests.java | 78 ++++++++++++++++++- 6 files changed, 115 insertions(+), 35 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/query/GeoShapeFilterBuilder.java b/src/main/java/org/elasticsearch/index/query/GeoShapeFilterBuilder.java index 2b57bb6f34de6..2d44c742452e3 100644 --- a/src/main/java/org/elasticsearch/index/query/GeoShapeFilterBuilder.java +++ b/src/main/java/org/elasticsearch/index/query/GeoShapeFilterBuilder.java @@ -19,13 +19,13 @@ package org.elasticsearch.index.query; -import java.io.IOException; - import org.elasticsearch.common.geo.ShapeRelation; import org.elasticsearch.common.geo.SpatialStrategy; import org.elasticsearch.common.geo.builders.ShapeBuilder; import org.elasticsearch.common.xcontent.XContentBuilder; +import java.io.IOException; + /** * {@link FilterBuilder} that builds a GeoShape Filter */ @@ -46,7 +46,7 @@ public class GeoShapeFilterBuilder extends BaseFilterBuilder { private final String indexedShapeType; private String indexedShapeIndex; - private String indexedShapeFieldName; + private String indexedShapePath; private ShapeRelation relation = null; @@ -150,13 +150,13 @@ public GeoShapeFilterBuilder indexedShapeIndex(String indexedShapeIndex) { } /** - * Sets the name of the field in the indexed Shape document that has the Shape itself + * Sets the path of the field in the indexed Shape document that has the Shape itself * - * @param indexedShapeFieldName Name of the field where the Shape itself is defined + * @param indexedShapePath Path of the field where the Shape itself is defined * @return this */ - public GeoShapeFilterBuilder indexedShapeFieldName(String indexedShapeFieldName) { - this.indexedShapeFieldName = indexedShapeFieldName; + public GeoShapeFilterBuilder indexedShapePath(String indexedShapePath) { + this.indexedShapePath = indexedShapePath; return this; } @@ -190,8 +190,8 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep if (indexedShapeIndex != null) { builder.field("index", indexedShapeIndex); } - if (indexedShapeFieldName != null) { - builder.field("shape_field_name", indexedShapeFieldName); + if (indexedShapePath != null) { + builder.field("path", indexedShapePath); } builder.endObject(); } diff --git a/src/main/java/org/elasticsearch/index/query/GeoShapeFilterParser.java b/src/main/java/org/elasticsearch/index/query/GeoShapeFilterParser.java index 5a921329b6e74..38d9b1571d311 100644 --- a/src/main/java/org/elasticsearch/index/query/GeoShapeFilterParser.java +++ b/src/main/java/org/elasticsearch/index/query/GeoShapeFilterParser.java @@ -88,7 +88,7 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar String id = null; String type = null; String index = DEFAULTS.INDEX_NAME; - String shapeFieldName = DEFAULTS.SHAPE_FIELD_NAME; + String shapePath = DEFAULTS.SHAPE_FIELD_NAME; XContentParser.Token token; String currentFieldName = null; @@ -124,8 +124,8 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar type = parser.text(); } else if ("index".equals(currentFieldName)) { index = parser.text(); - } else if ("shape_field_name".equals(currentFieldName)) { - shapeFieldName = parser.text(); + } else if ("path".equals(currentFieldName)) { + shapePath = parser.text(); } } } @@ -134,8 +134,8 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar } else if (type == null) { throw new QueryParsingException(parseContext.index(), "Type for indexed shape not provided"); } - shape = fetchService.fetch(id, type, index, shapeFieldName); - } else { + shape = fetchService.fetch(id, type, index, shapePath); + } else { throw new QueryParsingException(parseContext.index(), "[geo_shape] filter does not support [" + currentFieldName + "]"); } } diff --git a/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java b/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java index 6511b12c9cfdc..1fc388dec3cca 100644 --- a/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java +++ b/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java @@ -42,7 +42,7 @@ public class GeoShapeQueryBuilder extends BaseQueryBuilder implements BoostableQ private final String indexedShapeType; private String indexedShapeIndex; - private String indexedShapeFieldName; + private String indexedShapePath; private String queryName; @@ -109,13 +109,13 @@ public GeoShapeQueryBuilder indexedShapeIndex(String indexedShapeIndex) { } /** - * Sets the name of the field in the indexed Shape document that has the Shape itself + * Sets the path of the field in the indexed Shape document that has the Shape itself * - * @param indexedShapeFieldName Name of the field where the Shape itself is defined + * @param indexedShapePath path of the field where the Shape itself is defined * @return this */ - public GeoShapeQueryBuilder indexedShapeFieldName(String indexedShapeFieldName) { - this.indexedShapeFieldName = indexedShapeFieldName; + public GeoShapeQueryBuilder indexedShapePath(String indexedShapePath) { + this.indexedShapePath = indexedShapePath; return this; } @@ -146,8 +146,8 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep if (indexedShapeIndex != null) { builder.field("index", indexedShapeIndex); } - if (indexedShapeFieldName != null) { - builder.field("shape_field_name", indexedShapeFieldName); + if (indexedShapePath != null) { + builder.field("path", indexedShapePath); } builder.endObject(); } diff --git a/src/main/java/org/elasticsearch/index/query/GeoShapeQueryParser.java b/src/main/java/org/elasticsearch/index/query/GeoShapeQueryParser.java index 1e3cdfbd61844..38bb63ce83b7d 100644 --- a/src/main/java/org/elasticsearch/index/query/GeoShapeQueryParser.java +++ b/src/main/java/org/elasticsearch/index/query/GeoShapeQueryParser.java @@ -65,7 +65,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars String id = null; String type = null; String index = DEFAULTS.INDEX_NAME; - String shapeFieldName = DEFAULTS.SHAPE_FIELD_NAME; + String shapePath = DEFAULTS.SHAPE_FIELD_NAME; XContentParser.Token token; String currentFieldName = null; @@ -102,8 +102,8 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars type = parser.text(); } else if ("index".equals(currentFieldName)) { index = parser.text(); - } else if ("shape_field_name".equals(currentFieldName)) { - shapeFieldName = parser.text(); + } else if ("path".equals(currentFieldName)) { + shapePath = parser.text(); } } } @@ -112,7 +112,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars } else if (type == null) { throw new QueryParsingException(parseContext.index(), "Type for indexed shape not provided"); } - shape = fetchService.fetch(id, type, index, shapeFieldName); + shape = fetchService.fetch(id, type, index, shapePath); } else { throw new QueryParsingException(parseContext.index(), "[geo_shape] query does not support [" + currentFieldName + "]"); } diff --git a/src/main/java/org/elasticsearch/index/search/shape/ShapeFetchService.java b/src/main/java/org/elasticsearch/index/search/shape/ShapeFetchService.java index e0a5993febd36..c55a23204959d 100644 --- a/src/main/java/org/elasticsearch/index/search/shape/ShapeFetchService.java +++ b/src/main/java/org/elasticsearch/index/search/shape/ShapeFetchService.java @@ -24,6 +24,7 @@ import org.elasticsearch.action.get.GetRequest; import org.elasticsearch.action.get.GetResponse; import org.elasticsearch.client.Client; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.geo.builders.ShapeBuilder; import org.elasticsearch.common.inject.Inject; @@ -49,35 +50,40 @@ public ShapeFetchService(Client client, Settings settings) { /** * Fetches the Shape with the given ID in the given type and index. * - * @param id ID of the Shape to fetch - * @param type Index type where the Shape is indexed - * @param index Index where the Shape is indexed - * @param shapeField Name of the field in the Shape Document where the Shape itself is located + * @param id ID of the Shape to fetch + * @param type Index type where the Shape is indexed + * @param index Index where the Shape is indexed + * @param path Name or path of the field in the Shape Document where the Shape itself is located * @return Shape with the given ID * @throws IOException Can be thrown while parsing the Shape Document and extracting the Shape */ - public ShapeBuilder fetch(String id, String type, String index, String shapeField) throws IOException { + public ShapeBuilder fetch(String id, String type, String index, String path) throws IOException { GetResponse response = client.get(new GetRequest(index, type, id).preference("_local").operationThreaded(false)).actionGet(); if (!response.isExists()) { throw new ElasticSearchIllegalArgumentException("Shape with ID [" + id + "] in type [" + type + "] not found"); } + String[] pathElements = Strings.splitStringToArray(path, '.'); + int currentPathSlot = 0; + XContentParser parser = null; try { parser = XContentHelper.createParser(response.getSourceAsBytesRef()); XContentParser.Token currentToken; while ((currentToken = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (currentToken == XContentParser.Token.FIELD_NAME) { - if (shapeField.equals(parser.currentName())) { + if (pathElements[currentPathSlot].equals(parser.currentName())) { parser.nextToken(); - return ShapeBuilder.parse(parser); + if (++currentPathSlot == pathElements.length) { + return ShapeBuilder.parse(parser); + } } else { parser.nextToken(); parser.skipChildren(); } } } - throw new ElasticSearchIllegalStateException("Shape with name [" + id + "] found but missing " + shapeField + " field"); + throw new ElasticSearchIllegalStateException("Shape with name [" + id + "] found but missing " + path + " field"); } finally { if (parser != null) { parser.close(); diff --git a/src/test/java/org/elasticsearch/search/geo/GeoShapeIntegrationTests.java b/src/test/java/org/elasticsearch/search/geo/GeoShapeIntegrationTests.java index dbbd9e42cdce1..41d870044e1b6 100644 --- a/src/test/java/org/elasticsearch/search/geo/GeoShapeIntegrationTests.java +++ b/src/test/java/org/elasticsearch/search/geo/GeoShapeIntegrationTests.java @@ -21,15 +21,17 @@ import org.elasticsearch.action.get.GetResponse; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.geo.ShapeRelation; import org.elasticsearch.common.geo.builders.ShapeBuilder; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.index.query.*; import org.elasticsearch.test.ElasticsearchIntegrationTest; import org.junit.Test; import java.io.IOException; import java.util.List; +import java.util.Locale; import java.util.Map; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -245,13 +247,85 @@ public void testParsingMultipleShapes() throws IOException { + "\"id\": \"1\"," + "\"type\": \"type1\"," + "\"index\": \"test\"," - + "\"shape_field_name\": \"location2\"" + + "\"path\": \"location2\"" + "}}}}"; SearchResponse result = client().prepareSearch("test").setQuery(QueryBuilders.matchAllQuery()).setPostFilter(filter).execute().actionGet(); assertHitCount(result, 1); } + @Test + public void testShapeFetching_path() throws IOException { + prepareCreate("shapes").execute().actionGet(); + prepareCreate("test").addMapping("type", "location", "type=geo_shape").execute().actionGet(); + String location = "\"location\" : {\"type\":\"polygon\", \"coordinates\":[[[-10,-10],[10,-10],[10,10],[-10,10],[-10,-10]]]}"; + client().prepareIndex("shapes", "type", "1") + .setSource( + String.format( + Locale.ROOT, "{ %s, \"1\" : { %s, \"2\" : { %s, \"3\" : { %s } }} }", location, location, location, location + ) + ).get(); + client().prepareIndex("test", "type", "1") + .setSource(jsonBuilder().startObject().startObject("location") + .field("type", "polygon") + .startArray("coordinates").startArray() + .startArray().value(-20).value(-20).endArray() + .startArray().value(20).value(-20).endArray() + .startArray().value(20).value(20).endArray() + .startArray().value(-20).value(20).endArray() + .startArray().value(-20).value(-20).endArray() + .endArray().endArray() + .endObject().endObject()).get(); + client().admin().indices().prepareRefresh("test", "shapes").execute().actionGet(); + + GeoShapeFilterBuilder filter = FilterBuilders.geoShapeFilter("location", "1", "type", ShapeRelation.INTERSECTS) + .indexedShapeIndex("shapes") + .indexedShapePath("location"); + SearchResponse result = client().prepareSearch("test").setQuery(QueryBuilders.matchAllQuery()) + .setPostFilter(filter).get(); + assertHitCount(result, 1); + filter = FilterBuilders.geoShapeFilter("location", "1", "type", ShapeRelation.INTERSECTS) + .indexedShapeIndex("shapes") + .indexedShapePath("1.location"); + result = client().prepareSearch("test").setQuery(QueryBuilders.matchAllQuery()) + .setPostFilter(filter).get(); + assertHitCount(result, 1); + filter = FilterBuilders.geoShapeFilter("location", "1", "type", ShapeRelation.INTERSECTS) + .indexedShapeIndex("shapes") + .indexedShapePath("1.2.location"); + result = client().prepareSearch("test").setQuery(QueryBuilders.matchAllQuery()) + .setPostFilter(filter).get(); + assertHitCount(result, 1); + filter = FilterBuilders.geoShapeFilter("location", "1", "type", ShapeRelation.INTERSECTS) + .indexedShapeIndex("shapes") + .indexedShapePath("1.2.3.location"); + result = client().prepareSearch("test").setQuery(QueryBuilders.matchAllQuery()) + .setPostFilter(filter).get(); + assertHitCount(result, 1); + + // now test the query variant + GeoShapeQueryBuilder query = QueryBuilders.geoShapeQuery("location", "1", "type") + .indexedShapeIndex("shapes") + .indexedShapePath("location"); + result = client().prepareSearch("test").setQuery(query).get(); + assertHitCount(result, 1); + query = QueryBuilders.geoShapeQuery("location", "1", "type") + .indexedShapeIndex("shapes") + .indexedShapePath("1.location"); + result = client().prepareSearch("test").setQuery(query).get(); + assertHitCount(result, 1); + query = QueryBuilders.geoShapeQuery("location", "1", "type") + .indexedShapeIndex("shapes") + .indexedShapePath("1.2.location"); + result = client().prepareSearch("test").setQuery(query).get(); + assertHitCount(result, 1); + query = QueryBuilders.geoShapeQuery("location", "1", "type") + .indexedShapeIndex("shapes") + .indexedShapePath("1.2.3.location"); + result = client().prepareSearch("test").setQuery(query).get(); + assertHitCount(result, 1); + } + @Test // Issue 2944 public void testThatShapeIsReturnedEvenWhenExclusionsAreSet() throws Exception { String mapping = XContentFactory.jsonBuilder().startObject().startObject("type1")