Merge pull request #11969 from MaineC/feature/geo-bounding-box-refactoring

Refactoring of GeoBoundingBoxQueryBuilder and -Parser

core/src/main/java/org/elasticsearch/common/Numbers.java

@@ -178,5 +178,4 @@ public final class Numbers {
         }
         return true;
     }
-
 }

core/src/main/java/org/elasticsearch/common/geo/GeoPoint.java

@@ -22,8 +22,8 @@ package org.elasticsearch.common.geo;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
-import java.io.IOException;
 
+import java.io.IOException;
 
 import org.apache.lucene.util.BitUtil;
 import org.apache.lucene.util.XGeoHashUtils;
@@ -59,6 +59,10 @@ public final class GeoPoint implements Writeable<GeoPoint> {
         this.lon = lon;
     }
 
+    public GeoPoint(GeoPoint template) {
+        this(template.getLat(), template.getLon());
+    }
+
     public GeoPoint reset(double lat, double lon) {
         this.lat = lat;
         this.lon = lon;
@@ -175,7 +179,7 @@ public final class GeoPoint implements Writeable<GeoPoint> {
     public static GeoPoint fromIndexLong(long indexLong) {
         return new GeoPoint().resetFromIndexHash(indexLong);
     }
-    
+
     @Override
     public GeoPoint readFrom(StreamInput in) throws IOException {
         double lat = in.readDouble();

core/src/main/java/org/elasticsearch/common/geo/GeoUtils.java

@@ -23,6 +23,7 @@ import org.apache.lucene.spatial.prefix.tree.GeohashPrefixTree;
 import org.apache.lucene.spatial.prefix.tree.QuadPrefixTree;
 import org.apache.lucene.util.SloppyMath;
 import org.elasticsearch.ElasticsearchParseException;
+import org.elasticsearch.common.Numbers;
 import org.elasticsearch.common.unit.DistanceUnit;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentParser.Token;

core/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java

@@ -19,171 +19,346 @@
 
 package org.elasticsearch.index.query;
 
+import org.apache.lucene.search.Query;
+import org.elasticsearch.Version;
+import org.elasticsearch.common.Numbers;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.geo.GeoPoint;
+import org.elasticsearch.common.geo.GeoUtils;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.index.fielddata.IndexGeoPointFieldData;
+import org.elasticsearch.index.mapper.MappedFieldType;
+import org.elasticsearch.index.mapper.geo.GeoPointFieldMapper;
+import org.elasticsearch.index.search.geo.InMemoryGeoBoundingBoxQuery;
+import org.elasticsearch.index.search.geo.IndexedGeoBoundingBoxQuery;
 
 import java.io.IOException;
+import java.util.Objects;
 
+/**
+ * Creates a Lucene query that will filter for all documents that lie within the specified
+ * bounding box.
+ *
+ * This query can only operate on fields of type geo_point that have latitude and longitude
+ * enabled.
+ * */
 public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBoundingBoxQueryBuilder> {
-
+    /** Name of the query. */
     public static final String NAME = "geo_bbox";
+    /** Default for geo point coerce (as of this writing false). */
+    public static final boolean DEFAULT_COERCE = false;
+    /** Default for skipping geo point validation (as of this writing false). */
+    public static final boolean DEFAULT_IGNORE_MALFORMED = false;
+    /** Default type for executing this query (memory as of this writing). */
+    public static final GeoExecType DEFAULT_TYPE = GeoExecType.MEMORY;
+    /** Needed for serialization. */
+    static final GeoBoundingBoxQueryBuilder PROTOTYPE = new GeoBoundingBoxQueryBuilder("");
 
-    public static final String TOP_LEFT = GeoBoundingBoxQueryParser.TOP_LEFT;
-    public static final String BOTTOM_RIGHT = GeoBoundingBoxQueryParser.BOTTOM_RIGHT;
+    /** Name of field holding geo coordinates to compute the bounding box on.*/
+    private final String fieldName;
+    /** Top left corner coordinates of bounding box. */
+    private GeoPoint topLeft = new GeoPoint(Double.NaN, Double.NaN);
+    /** Bottom right corner coordinates of bounding box.*/
+    private GeoPoint bottomRight = new GeoPoint(Double.NaN, Double.NaN);
+    /** Whether or not to infer correct coordinates for wrapping bounding boxes.*/
+    private boolean coerce = DEFAULT_COERCE;
+    /** Whether or not to skip geo point validation. */
+    private boolean ignoreMalformed = DEFAULT_IGNORE_MALFORMED;
+    /** How the query should be run. */
+    private GeoExecType type = DEFAULT_TYPE;
 
-    private static final int TOP = 0;
-    private static final int LEFT = 1;
-    private static final int BOTTOM = 2;
-    private static final int RIGHT = 3;
-
-    private final String name;
-
-    private double[] box = {Double.NaN, Double.NaN, Double.NaN, Double.NaN};
-
-    private String type;
-    private Boolean coerce;
-    private Boolean ignoreMalformed;
-
-    static final GeoBoundingBoxQueryBuilder PROTOTYPE = new GeoBoundingBoxQueryBuilder(null);
-
-    public GeoBoundingBoxQueryBuilder(String name) {
-        this.name = name;
+    /**
+     * Create new bounding box query.
+     * @param fieldName name of index field containing geo coordinates to operate on.
+     * */
+    public GeoBoundingBoxQueryBuilder(String fieldName) {
+        if (fieldName == null) {
+            throw new IllegalArgumentException("Field name must not be empty.");
+        }
+        this.fieldName = fieldName;
     }
 
     /**
      * Adds top left point.
-     *
-     * @param lat The latitude
-     * @param lon The longitude
+     * @param top The top latitude
+     * @param left The left longitude
+     * @param bottom The bottom latitude
+     * @param right The right longitude
      */
-    public GeoBoundingBoxQueryBuilder topLeft(double lat, double lon) {
-        box[TOP] = lat;
-        box[LEFT] = lon;
+    public GeoBoundingBoxQueryBuilder setCorners(double top, double left, double bottom, double right) {
+        if (!ignoreMalformed) {
+            if (Numbers.isValidDouble(top) == false) {
+                throw new IllegalArgumentException("top latitude is invalid: " + top);
+            }
+            if (Numbers.isValidDouble(left) == false) {
+                throw new IllegalArgumentException("left longitude is invalid: " + left);
+            }
+            if (Numbers.isValidDouble(bottom) == false) {
+                throw new IllegalArgumentException("bottom latitude is invalid: " + bottom);
+            }
+            if (Numbers.isValidDouble(right) == false) {
+                throw new IllegalArgumentException("right longitude is invalid: " + right);
+            }
+
+            // all corners are valid after above checks - make sure they are in the right relation
+            if (top < bottom) {
+                throw new IllegalArgumentException("top is below bottom corner: " +
+                            top + " vs. " + bottom);
+                }
+
+                // we do not check longitudes as the query generation code can deal with flipped left/right values
+        }
+        
+        topLeft.reset(top, left);
+        bottomRight.reset(bottom, right);
         return this;
     }
 
-    public GeoBoundingBoxQueryBuilder topLeft(GeoPoint point) {
-        return topLeft(point.lat(), point.lon());
-    }
-
-    public GeoBoundingBoxQueryBuilder topLeft(String geohash) {
-        return topLeft(GeoPoint.fromGeohash(geohash));
-    }
-
     /**
-     * Adds bottom right corner.
-     *
-     * @param lat The latitude
-     * @param lon The longitude
-     */
-    public GeoBoundingBoxQueryBuilder bottomRight(double lat, double lon) {
-        box[BOTTOM] = lat;
-        box[RIGHT] = lon;
-        return this;
-    }
-
-    public GeoBoundingBoxQueryBuilder bottomRight(GeoPoint point) {
-        return bottomRight(point.lat(), point.lon());
-    }
-
-    public GeoBoundingBoxQueryBuilder bottomRight(String geohash) {
-        return bottomRight(GeoPoint.fromGeohash(geohash));
+     * Adds points.
+     * @param topLeft topLeft point to add.
+     * @param bottomRight bottomRight point to add.
+     * */
+    public GeoBoundingBoxQueryBuilder setCorners(GeoPoint topLeft, GeoPoint bottomRight) {
+        return setCorners(topLeft.getLat(), topLeft.getLon(), bottomRight.getLat(), bottomRight.getLon());
     }
 
     /**
-     * Adds bottom left corner.
-     *
-     * @param lat The latitude
-     * @param lon The longitude
-     */
-    public GeoBoundingBoxQueryBuilder bottomLeft(double lat, double lon) {
-        box[BOTTOM] = lat;
-        box[LEFT] = lon;
-        return this;
+     * Adds points.
+     * @param topLeft topLeft point to add as geohash.
+     * @param bottomRight bottomRight point to add as geohash.
+     * */
+    public GeoBoundingBoxQueryBuilder setCorners(String topLeft, String bottomRight) {
+        return setCorners(GeoPoint.fromGeohash(topLeft), GeoPoint.fromGeohash(bottomRight));
     }
 
-    public GeoBoundingBoxQueryBuilder bottomLeft(GeoPoint point) {
-        return bottomLeft(point.lat(), point.lon());
+    /** Returns the top left corner of the bounding box. */
+    public GeoPoint topLeft() {
+        return topLeft;
     }
-
-    public GeoBoundingBoxQueryBuilder bottomLeft(String geohash) {
-        return bottomLeft(GeoPoint.fromGeohash(geohash));
+    
+    /** Returns the bottom right corner of the bounding box. */
+    public GeoPoint bottomRight() {
+        return bottomRight;
     }
 
     /**
-     * Adds top right point.
+     * Adds corners in OGC standard bbox/ envelop format.
      *
-     * @param lat The latitude
-     * @param lon The longitude
+     * @param bottomLeft bottom left corner of bounding box.
+     * @param topRight top right corner of bounding box.
      */
-    public GeoBoundingBoxQueryBuilder topRight(double lat, double lon) {
-        box[TOP] = lat;
-        box[RIGHT] = lon;
-        return this;
+    public GeoBoundingBoxQueryBuilder setCornersOGC(GeoPoint bottomLeft, GeoPoint topRight) {
+        return setCorners(topRight.getLat(), bottomLeft.getLon(), bottomLeft.getLat(), topRight.getLon());
     }
 
-    public GeoBoundingBoxQueryBuilder topRight(GeoPoint point) {
-        return topRight(point.lat(), point.lon());
-    }
-
-    public GeoBoundingBoxQueryBuilder topRight(String geohash) {
-        return topRight(GeoPoint.fromGeohash(geohash));
+    /**
+     * Adds corners in OGC standard bbox/ envelop format.
+     *
+     * @param bottomLeft bottom left corner geohash.
+     * @param topRight top right corner geohash.
+     */
+    public GeoBoundingBoxQueryBuilder setCornersOGC(String bottomLeft, String topRight) {
+        return setCornersOGC(GeoPoint.fromGeohash(bottomLeft), GeoPoint.fromGeohash(topRight));
     }
 
+    /**
+     * Specify whether or not to try and fix broken/wrapping bounding boxes.
+     * If set to true, also enables ignoreMalformed thus disabling geo point
+     * validation altogether.
+     **/
     public GeoBoundingBoxQueryBuilder coerce(boolean coerce) {
+        if (coerce) {
+            this.ignoreMalformed = true;
+        }
         this.coerce = coerce;
         return this;
     }
 
+    /** Returns whether or not to try and fix broken/wrapping bounding boxes. */
+    public boolean coerce() {
+        return this.coerce;
+    }
+
+    /**
+     * Specify whether or not to ignore validation errors of bounding boxes.
+     * Can only be set if coerce set to false, otherwise calling this
+     * method has no effect.
+     **/
     public GeoBoundingBoxQueryBuilder ignoreMalformed(boolean ignoreMalformed) {
-        this.ignoreMalformed = ignoreMalformed;
+        if (coerce == false) {
+            this.ignoreMalformed = ignoreMalformed;
+        }
         return this;
     }
 
+    /** Returns whether or not to skip bounding box validation. */
+    public boolean ignoreMalformed() {
+        return ignoreMalformed;
+    }
+
     /**
      * Sets the type of executing of the geo bounding box. Can be either `memory` or `indexed`. Defaults
      * to `memory`.
      */
-    public GeoBoundingBoxQueryBuilder type(String type) {
+    public GeoBoundingBoxQueryBuilder type(GeoExecType type) {
+        if (type == null) {
+            throw new IllegalArgumentException("Type is not allowed to be null.");
+        }
         this.type = type;
         return this;
     }
 
+    /**
+     * For BWC: Parse type from type name.
+     * */
+    public GeoBoundingBoxQueryBuilder type(String type) {
+        this.type = GeoExecType.fromString(type);
+        return this;
+    }
+    /** Returns the execution type of the geo bounding box.*/
+    public GeoExecType type() {
+        return type;
+    }
+
+    /** Returns the name of the field to base the bounding box computation on. */
+    public String fieldName() {
+        return this.fieldName;
+    }
+
+    QueryValidationException checkLatLon(boolean indexCreatedBeforeV2_0) {
+        // validation was not available prior to 2.x, so to support bwc percolation queries we only ignore_malformed on 2.x created indexes
+        if (ignoreMalformed || indexCreatedBeforeV2_0) {
+            return null;
+        }
+
+        QueryValidationException validationException = null;
+        // For everything post 2.0 validate latitude and longitude unless validation was explicitly turned off
+        if (GeoUtils.isValidLatitude(topLeft.getLat()) == false) {
+            validationException = addValidationError("top latitude is invalid: " + topLeft.getLat(),
+                    validationException);
+        }
+        if (GeoUtils.isValidLongitude(topLeft.getLon()) == false) {
+            validationException = addValidationError("left longitude is invalid: " + topLeft.getLon(),
+                    validationException);
+        }
+        if (GeoUtils.isValidLatitude(bottomRight.getLat()) == false) {
+            validationException = addValidationError("bottom latitude is invalid: " + bottomRight.getLat(),
+                    validationException);
+        }
+        if (GeoUtils.isValidLongitude(bottomRight.getLon()) == false) {
+            validationException = addValidationError("right longitude is invalid: " + bottomRight.getLon(),
+                    validationException);
+        }
+        return validationException;
+    }
+
+    @Override
+    public Query doToQuery(QueryShardContext context) {
+        QueryValidationException exception = checkLatLon(context.indexVersionCreated().before(Version.V_2_0_0));
+        if (exception != null) {
+            throw new QueryShardException(context, "couldn't validate latitude/ longitude values", exception);
+        }
+
+        GeoPoint luceneTopLeft = new GeoPoint(topLeft);
+        GeoPoint luceneBottomRight = new GeoPoint(bottomRight);
+        if (coerce) {
+            // Special case: if the difference between the left and right is 360 and the right is greater than the left, we are asking for
+            // the complete longitude range so need to set longitude to the complete longditude range
+            double right = luceneBottomRight.getLon();
+            double left = luceneTopLeft.getLon();
+
+            boolean completeLonRange = ((right - left) % 360 == 0 && right > left);
+            GeoUtils.normalizePoint(luceneTopLeft, true, !completeLonRange);
+            GeoUtils.normalizePoint(luceneBottomRight, true, !completeLonRange);
+            if (completeLonRange) {
+                luceneTopLeft.resetLon(-180);
+                luceneBottomRight.resetLon(180);
+            }
+        }
+
+        MappedFieldType fieldType = context.fieldMapper(fieldName);
+        if (fieldType == null) {
+            throw new QueryShardException(context, "failed to find geo_point field [" + fieldName + "]");
+        }
+        if (!(fieldType instanceof GeoPointFieldMapper.GeoPointFieldType)) {
+            throw new QueryShardException(context, "field [" + fieldName + "] is not a geo_point field");
+        }
+        GeoPointFieldMapper.GeoPointFieldType geoFieldType = ((GeoPointFieldMapper.GeoPointFieldType) fieldType);
+
+        Query result;
+        switch(type) {
+            case INDEXED:
+                result = IndexedGeoBoundingBoxQuery.create(luceneTopLeft, luceneBottomRight, geoFieldType);
+                break;
+            case MEMORY:
+                IndexGeoPointFieldData indexFieldData = context.getForField(fieldType);
+                result = new InMemoryGeoBoundingBoxQuery(luceneTopLeft, luceneBottomRight, indexFieldData);
+                break;
+            default:
+                // Someone extended the type enum w/o adjusting this switch statement.
+                throw new IllegalStateException("geo bounding box type [" + type + "] not supported.");
+        }
+
+        return result;
+    }
+
     @Override
     protected void doXContent(XContentBuilder builder, Params params) throws IOException {
-        // check values
-        if(Double.isNaN(box[TOP])) {
-            throw new IllegalArgumentException("geo_bounding_box requires top latitude to be set");
-        } else if(Double.isNaN(box[BOTTOM])) {
-            throw new IllegalArgumentException("geo_bounding_box requires bottom latitude to be set");
-        } else if(Double.isNaN(box[RIGHT])) {
-            throw new IllegalArgumentException("geo_bounding_box requires right longitude to be set");
-        } else if(Double.isNaN(box[LEFT])) {
-            throw new IllegalArgumentException("geo_bounding_box requires left longitude to be set");
-        }
-
         builder.startObject(NAME);
 
-        builder.startObject(name);
-        builder.array(TOP_LEFT, box[LEFT], box[TOP]);
-        builder.array(BOTTOM_RIGHT, box[RIGHT], box[BOTTOM]);
+        builder.startObject(fieldName);
+        builder.array(GeoBoundingBoxQueryParser.TOP_LEFT, topLeft.getLon(), topLeft.getLat());
+        builder.array(GeoBoundingBoxQueryParser.BOTTOM_RIGHT, bottomRight.getLon(), bottomRight.getLat());
         builder.endObject();
-
-        if (type != null) {
-            builder.field("type", type);
-        }
-        if (coerce != null) {
-            builder.field("coerce", coerce);
-        }
-        if (ignoreMalformed != null) {
-            builder.field("ignore_malformed", ignoreMalformed);
-        }
+        builder.field("coerce", coerce);
+        builder.field("ignore_malformed", ignoreMalformed);
+        builder.field("type", type);
 
         printBoostAndQueryName(builder);
 
         builder.endObject();
     }
 
+    @Override
+    public boolean doEquals(GeoBoundingBoxQueryBuilder other) {
+        return Objects.equals(topLeft, other.topLeft) &&
+                Objects.equals(bottomRight, other.bottomRight) &&
+                Objects.equals(type, other.type) &&
+                Objects.equals(coerce, other.coerce) &&
+                Objects.equals(ignoreMalformed, other.ignoreMalformed) &&
+                Objects.equals(fieldName, other.fieldName);
+    }
+
+    @Override
+    public int doHashCode() {
+        return Objects.hash(topLeft, bottomRight, type, coerce, ignoreMalformed, fieldName);
+    }
+
+    @Override
+    public GeoBoundingBoxQueryBuilder doReadFrom(StreamInput in) throws IOException {
+        String fieldName = in.readString();
+        GeoBoundingBoxQueryBuilder geo = new GeoBoundingBoxQueryBuilder(fieldName);
+        geo.topLeft = geo.topLeft.readFrom(in);
+        geo.bottomRight = geo.bottomRight.readFrom(in);
+        geo.type = GeoExecType.readTypeFrom(in);
+        geo.coerce = in.readBoolean();
+        geo.ignoreMalformed = in.readBoolean();
+        return geo;
+    }
+
+    @Override
+    public void doWriteTo(StreamOutput out) throws IOException {
+        out.writeString(fieldName);
+        topLeft.writeTo(out);
+        bottomRight.writeTo(out);
+        type.writeTo(out);
+        out.writeBoolean(coerce);
+        out.writeBoolean(ignoreMalformed);
+    }
+
     @Override
     public String getWriteableName() {
         return NAME;

core/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryParser.java

@@ -19,43 +19,44 @@
 
 package org.elasticsearch.index.query;
 
-import org.apache.lucene.search.Query;
 import org.elasticsearch.ElasticsearchParseException;
-import org.elasticsearch.Version;
 import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeoUtils;
-import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.index.fielddata.IndexGeoPointFieldData;
-import org.elasticsearch.index.mapper.MappedFieldType;
-import org.elasticsearch.index.mapper.geo.GeoPointFieldMapper;
-import org.elasticsearch.index.search.geo.InMemoryGeoBoundingBoxQuery;
-import org.elasticsearch.index.search.geo.IndexedGeoBoundingBoxQuery;
 
 import java.io.IOException;
 
-/**
- *
- */
-public class GeoBoundingBoxQueryParser extends BaseQueryParserTemp {
+public class GeoBoundingBoxQueryParser extends BaseQueryParser<GeoBoundingBoxQueryBuilder> {
 
     public static final String NAME = "geo_bbox";
 
+    /** Key to refer to the top of the bounding box. */
     public static final String TOP = "top";
+    /** Key to refer to the left of the bounding box. */
     public static final String LEFT = "left";
+    /** Key to refer to the right of the bounding box. */
     public static final String RIGHT = "right";
+    /** Key to refer to the bottom of the bounding box. */
     public static final String BOTTOM = "bottom";
 
+    /** Key to refer to top_left corner of bounding box. */
     public static final String TOP_LEFT = TOP + "_" + LEFT;
-    public static final String TOP_RIGHT = TOP + "_" + RIGHT;
-    public static final String BOTTOM_LEFT = BOTTOM + "_" + LEFT;
+    /** Key to refer to bottom_right corner of bounding box. */
     public static final String BOTTOM_RIGHT = BOTTOM + "_" + RIGHT;
+    /** Key to refer to top_right corner of bounding box. */
+    public static final String TOP_RIGHT = TOP + "_" + RIGHT;
+    /** Key to refer to bottom left corner of bounding box.  */
+    public static final String BOTTOM_LEFT = BOTTOM + "_" + LEFT;
 
+    /** Key to refer to top_left corner of bounding box. */
     public static final String TOPLEFT = "topLeft";
-    public static final String TOPRIGHT = "topRight";
-    public static final String BOTTOMLEFT = "bottomLeft";
+    /** Key to refer to bottom_right corner of bounding box. */
     public static final String BOTTOMRIGHT = "bottomRight";
+    /** Key to refer to top_right corner of bounding box. */
+    public static final String TOPRIGHT = "topRight";
+    /** Key to refer to bottom left corner of bounding box.  */
+    public static final String BOTTOMLEFT = "bottomLeft";
 
     public static final String FIELD = "field";
 
@@ -65,8 +66,7 @@ public class GeoBoundingBoxQueryParser extends BaseQueryParserTemp {
     }
 
     @Override
-    public Query parse(QueryShardContext context) throws IOException {
-        QueryParseContext parseContext = context.parseContext();
+    public GeoBoundingBoxQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException {
         XContentParser parser = parseContext.parser();
 
         String fieldName = null;
@@ -80,9 +80,8 @@ public class GeoBoundingBoxQueryParser extends BaseQueryParserTemp {
         String queryName = null;
         String currentFieldName = null;
         XContentParser.Token token;
-        final boolean indexCreatedBeforeV2_0 = parseContext.shardContext().indexVersionCreated().before(Version.V_2_0_0);
-        boolean coerce = false;
-        boolean ignoreMalformed = false;
+        boolean coerce = GeoBoundingBoxQueryBuilder.DEFAULT_COERCE;
+        boolean ignoreMalformed = GeoBoundingBoxQueryBuilder.DEFAULT_IGNORE_MALFORMED;
 
         GeoPoint sparse = new GeoPoint();
 
@@ -140,14 +139,14 @@ public class GeoBoundingBoxQueryParser extends BaseQueryParserTemp {
                     queryName = parser.text();
                 } else if ("boost".equals(currentFieldName)) {
                     boost = parser.floatValue();
-                } else if ("coerce".equals(currentFieldName) || (indexCreatedBeforeV2_0 && "normalize".equals(currentFieldName))) {
+                } else if ("coerce".equals(currentFieldName) || ("normalize".equals(currentFieldName))) {
                     coerce = parser.booleanValue();
                     if (coerce) {
                         ignoreMalformed = true;
                     }
                 } else if ("type".equals(currentFieldName)) {
                     type = parser.text();
-                } else if ("ignore_malformed".equals(currentFieldName) && coerce == false) {
+                } else if ("ignore_malformed".equals(currentFieldName)) {
                     ignoreMalformed = parser.booleanValue();
                 } else {
                     throw new ParsingException(parseContext, "failed to parse [{}] query. unexpected field [{}]", NAME, currentFieldName);
@@ -157,60 +156,14 @@ public class GeoBoundingBoxQueryParser extends BaseQueryParserTemp {
 
         final GeoPoint topLeft = sparse.reset(top, left);  //just keep the object
         final GeoPoint bottomRight = new GeoPoint(bottom, right);
-
-        // validation was not available prior to 2.x, so to support bwc percolation queries we only ignore_malformed on 2.x created indexes
-        if (!indexCreatedBeforeV2_0 && !ignoreMalformed) {
-            if (topLeft.lat() > 90.0 || topLeft.lat() < -90.0) {
-                throw new ParsingException(parseContext, "illegal latitude value [{}] for [{}]", topLeft.lat(), NAME);
-            }
-            if (topLeft.lon() > 180.0 || topLeft.lon() < -180) {
-                throw new ParsingException(parseContext, "illegal longitude value [{}] for [{}]", topLeft.lon(), NAME);
-            }
-            if (bottomRight.lat() > 90.0 || bottomRight.lat() < -90.0) {
-                throw new ParsingException(parseContext, "illegal latitude value [{}] for [{}]", bottomRight.lat(), NAME);
-            }
-            if (bottomRight.lon() > 180.0 || bottomRight.lon() < -180) {
-                throw new ParsingException(parseContext, "illegal longitude value [{}] for [{}]", bottomRight.lon(), NAME);
-            }
-        }
-
-        if (coerce) {
-            // Special case: if the difference between the left and right is 360 and the right is greater than the left, we are asking for
-            // the complete longitude range so need to set longitude to the complete longditude range
-            boolean completeLonRange = ((right - left) % 360 == 0 && right > left);
-            GeoUtils.normalizePoint(topLeft, true, !completeLonRange);
-            GeoUtils.normalizePoint(bottomRight, true, !completeLonRange);
-            if (completeLonRange) {
-                topLeft.resetLon(-180);
-                bottomRight.resetLon(180);
-            }
-        }
-
-        MappedFieldType fieldType = context.fieldMapper(fieldName);
-        if (fieldType == null) {
-            throw new ParsingException(parseContext, "failed to parse [{}] query. could not find [{}] field [{}]", NAME, GeoPointFieldMapper.CONTENT_TYPE, fieldName);
-        }
-        if (!(fieldType instanceof GeoPointFieldMapper.GeoPointFieldType)) {
-            throw new ParsingException(parseContext, "failed to parse [{}] query. field [{}] is expected to be of type [{}], but is of [{}] type instead", NAME, fieldName, GeoPointFieldMapper.CONTENT_TYPE, fieldType.typeName());
-        }
-        GeoPointFieldMapper.GeoPointFieldType geoFieldType = ((GeoPointFieldMapper.GeoPointFieldType) fieldType);
-
-        Query filter;
-        if ("indexed".equals(type)) {
-            filter = IndexedGeoBoundingBoxQuery.create(topLeft, bottomRight, geoFieldType);
-        } else if ("memory".equals(type)) {
-            IndexGeoPointFieldData indexFieldData = context.getForField(fieldType);
-            filter = new InMemoryGeoBoundingBoxQuery(topLeft, bottomRight, indexFieldData);
-        } else {
-            throw new ParsingException(parseContext, "failed to parse [{}] query. geo bounding box type [{}] is not supported. either [indexed] or [memory] are allowed", NAME, type);
-        }
-        if (filter != null) {
-            filter.setBoost(boost);
-        }
-        if (queryName != null) {
-            context.addNamedQuery(queryName, filter);
-        }
-        return filter;
+        GeoBoundingBoxQueryBuilder builder = new GeoBoundingBoxQueryBuilder(fieldName);
+        builder.setCorners(topLeft, bottomRight);
+        builder.queryName(queryName);
+        builder.boost(boost);
+        builder.type(GeoExecType.fromString(type));
+        builder.coerce(coerce);
+        builder.ignoreMalformed(ignoreMalformed);
+        return builder;
     }
 
     @Override

core/src/main/java/org/elasticsearch/index/query/GeoExecType.java

@@ -0,0 +1,73 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.index.query;
+
+import org.elasticsearch.ElasticsearchException;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.io.stream.Writeable;
+
+import java.io.IOException;
+
+/** Specifies how a geo query should be run. */
+public enum GeoExecType implements Writeable<GeoExecType> {
+    
+    MEMORY(0), INDEXED(1);
+
+    private final int ordinal;
+
+    private static final GeoExecType PROTOTYPE = MEMORY;
+
+    GeoExecType(int ordinal) {
+        this.ordinal = ordinal;
+    }
+
+    @Override
+    public GeoExecType readFrom(StreamInput in) throws IOException {
+        int ord = in.readVInt();
+        switch(ord) {
+            case(0): return MEMORY;
+            case(1): return INDEXED;
+        }
+        throw new ElasticsearchException("unknown serialized type [" + ord + "]");
+    }
+
+    public static GeoExecType readTypeFrom(StreamInput in) throws IOException {
+        return PROTOTYPE.readFrom(in);
+    }
+
+    @Override
+    public void writeTo(StreamOutput out) throws IOException {
+        out.writeVInt(this.ordinal);
+    }
+
+    public static GeoExecType fromString(String typeName) {
+        if (typeName == null) {
+            throw new IllegalArgumentException("cannot parse type from null string");
+        }
+
+        for (GeoExecType type : GeoExecType.values()) {
+            if (type.name().equalsIgnoreCase(typeName)) {
+                return type;
+            }
+        }
+        throw new IllegalArgumentException("no type can be parsed from ordinal " + typeName);
+    }
+}

core/src/main/java/org/elasticsearch/index/query/IndexQueryParserService.java

@@ -80,7 +80,7 @@ public class IndexQueryParserService extends AbstractIndexComponent {
 
     final IndexCache indexCache;
 
-    final IndexFieldDataService fieldDataService;
+    protected IndexFieldDataService fieldDataService;
 
     final ClusterService clusterService;
 

core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java

@@ -37,6 +37,7 @@ import org.elasticsearch.common.ParseFieldMatcher;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.inject.AbstractModule;
+import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.inject.Injector;
 import org.elasticsearch.common.inject.ModulesBuilder;
 import org.elasticsearch.common.inject.multibindings.Multibinder;
@@ -58,6 +59,11 @@ import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexNameModule;
 import org.elasticsearch.index.analysis.AnalysisModule;
 import org.elasticsearch.index.cache.IndexCacheModule;
+import org.elasticsearch.index.fielddata.IndexFieldData;
+import org.elasticsearch.index.fielddata.IndexFieldDataService;
+import org.elasticsearch.index.fielddata.IndexGeoPointFieldData;
+import org.elasticsearch.index.fielddata.plain.GeoPointDoubleArrayIndexFieldData;
+import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.query.functionscore.ScoreFunctionParser;
 import org.elasticsearch.index.query.support.QueryParsers;
@@ -126,7 +132,7 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
     protected static String[] getCurrentTypes() {
         return currentTypes;
     }
-
+    
     private static NamedWriteableRegistry namedWriteableRegistry;
 
     private static String[] randomTypes;
@@ -183,6 +189,7 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
                 }
         ).createInjector();
         queryParserService = injector.getInstance(IndexQueryParserService.class);
+
         MapperService mapperService = queryParserService.mapperService;
         //create some random type with some default field, those types will stick around for all of the subclasses
         currentTypes = new String[randomIntBetween(0, 5)];
@@ -462,6 +469,7 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
         QueryShardContext queryCreationContext = new QueryShardContext(index, queryParserService);
         queryCreationContext.reset();
         queryCreationContext.parseFieldMatcher(ParseFieldMatcher.EMPTY);
+
         return queryCreationContext;
     }
 

core/src/test/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilderTests.java

@@ -0,0 +1,322 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.index.query;
+
+import com.spatial4j.core.io.GeohashUtils;
+import com.spatial4j.core.shape.Rectangle;
+
+import org.apache.lucene.search.BooleanClause;
+import org.apache.lucene.search.BooleanQuery;
+import org.apache.lucene.search.ConstantScoreQuery;
+import org.apache.lucene.search.NumericRangeQuery;
+import org.apache.lucene.search.Query;
+import org.elasticsearch.common.geo.GeoPoint;
+import org.elasticsearch.common.geo.GeoUtils;
+import org.elasticsearch.index.search.geo.InMemoryGeoBoundingBoxQuery;
+import org.elasticsearch.test.geo.RandomShapeGenerator;
+import org.junit.Test;
+
+import java.io.IOException;
+
+public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase<GeoBoundingBoxQueryBuilder> {
+    /** Randomly generate either NaN or one of the two infinity values. */
+    private static Double[] brokenDoubles = {Double.NaN, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY};
+    
+    @Override
+    protected GeoBoundingBoxQueryBuilder doCreateTestQueryBuilder() {
+        GeoBoundingBoxQueryBuilder builder = new GeoBoundingBoxQueryBuilder(GEO_POINT_FIELD_NAME);
+        Rectangle box = RandomShapeGenerator.xRandomRectangle(getRandom(), RandomShapeGenerator.xRandomPoint(getRandom()));
+
+        if (randomBoolean()) {
+            // check the top-left/bottom-right combination of setters
+            int path = randomIntBetween(0, 2);
+            switch (path) {
+            case 0:
+                builder.setCorners(
+                        new GeoPoint(box.getMaxY(), box.getMinX()), 
+                        new GeoPoint(box.getMinY(), box.getMaxX()));
+                break;
+            case 1:
+                builder.setCorners(
+                        GeohashUtils.encodeLatLon(box.getMaxY(), box.getMinX()),
+                        GeohashUtils.encodeLatLon(box.getMinY(), box.getMaxX()));
+                break;
+            default:
+                builder.setCorners(box.getMaxY(), box.getMinX(), box.getMinY(), box.getMaxX());
+            }
+        } else {
+            // check the bottom-left/ top-right combination of setters
+            if (randomBoolean()) {
+                builder.setCornersOGC(
+                        new GeoPoint(box.getMinY(), box.getMinX()),
+                        new GeoPoint(box.getMaxY(), box.getMaxX()));
+            } else {
+                builder.setCornersOGC(
+                        GeohashUtils.encodeLatLon(box.getMinY(), box.getMinX()),
+                        GeohashUtils.encodeLatLon(box.getMaxY(), box.getMaxX()));
+            }
+        }
+
+        if (randomBoolean()) {
+            builder.coerce(randomBoolean());
+        }
+        if (randomBoolean()) {
+            builder.ignoreMalformed(randomBoolean());
+        }
+
+        builder.type(randomFrom(GeoExecType.values()));
+        return builder;
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testValidationNullFieldname() {
+        new GeoBoundingBoxQueryBuilder(null);
+    }
+
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testValidationNullType() {
+        GeoBoundingBoxQueryBuilder qb = new GeoBoundingBoxQueryBuilder("teststring");
+        qb.type((GeoExecType) null);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testValidationNullTypeString() {
+        GeoBoundingBoxQueryBuilder qb = new GeoBoundingBoxQueryBuilder("teststring");
+        qb.type((String) null);
+    }
+
+    @Test
+    @Override
+    public void testToQuery() throws IOException {
+        assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0);
+        super.testToQuery();
+    }
+    
+    @Test(expected = QueryShardException.class)
+    public void testExceptionOnMissingTypes() throws IOException {
+        assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length == 0);
+        super.testToQuery();
+    }
+
+    @Test
+    public void testBrokenCoordinateCannotBeSet() {
+        PointTester[] testers = { new TopTester(), new LeftTester(), new BottomTester(), new RightTester() };
+
+        GeoBoundingBoxQueryBuilder builder = createTestQueryBuilder();
+        builder.coerce(false).ignoreMalformed(false);
+
+        for (PointTester tester : testers) {
+            try {
+                tester.invalidateCoordinate(builder, true);
+                fail("expected exception for broken " + tester.getClass().getName() + " coordinate");
+            } catch (IllegalArgumentException e) {
+                // exptected
+            }
+        }
+    }
+
+    @Test
+    public void testBrokenCoordinateCanBeSetWithIgnoreMalformed() {
+        PointTester[] testers = { new TopTester(), new LeftTester(), new BottomTester(), new RightTester() };
+
+        GeoBoundingBoxQueryBuilder builder = createTestQueryBuilder();
+        builder.ignoreMalformed(true);
+
+        for (PointTester tester : testers) {
+            tester.invalidateCoordinate(builder, true);
+        }
+    }
+
+
+    @Test
+    public void testValidation() {
+        PointTester[] testers = { new TopTester(), new LeftTester(), new BottomTester(), new RightTester() };
+
+        for (PointTester tester : testers) {
+            QueryValidationException except = null;
+
+            GeoBoundingBoxQueryBuilder builder = createTestQueryBuilder();
+            tester.invalidateCoordinate(builder.coerce(true), false);
+            except = builder.checkLatLon(true);
+            assertNull("Inner post 2.0 validation w/ coerce should ignore invalid "
+                    + tester.getClass().getName()
+                    + " coordinate: "
+                    + tester.invalidCoordinate + " ",
+                    except);
+
+            tester.invalidateCoordinate(builder.coerce(true), false);
+            except = builder.checkLatLon(false);
+            assertNull("Inner pre 2.0 validation w/ coerce should ignore invalid coordinate: "
+                    + tester.getClass().getName()
+                    + " coordinate: "
+                    + tester.invalidCoordinate + " ",
+                    except);
+
+            tester.invalidateCoordinate(builder.coerce(false).ignoreMalformed(false), false);
+            except = builder.checkLatLon(true);
+            assertNull("Inner pre 2.0 validation w/o coerce should ignore invalid coordinate for old indexes: "
+                    + tester.getClass().getName()
+                    + " coordinate: "
+                    + tester.invalidCoordinate,
+                    except);
+
+            tester.invalidateCoordinate(builder.coerce(false).ignoreMalformed(false), false);
+            except = builder.checkLatLon(false);
+            assertNotNull("Inner post 2.0 validation w/o coerce should detect invalid coordinate: "
+                    + tester.getClass().getName()
+                    + " coordinate: "
+                    + tester.invalidCoordinate,
+                    except);
+        }
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testTopBottomCannotBeFlipped() {
+        GeoBoundingBoxQueryBuilder builder = createTestQueryBuilder();
+        double top = builder.topLeft().getLat();
+        double left = builder.topLeft().getLon();
+        double bottom = builder.bottomRight().getLat();
+        double right = builder.bottomRight().getLon();
+
+        assumeTrue("top should not be equal to bottom for flip check", top != bottom);
+        System.out.println("top: " + top + " bottom: " + bottom);
+        builder.coerce(false).ignoreMalformed(false).setCorners(bottom, left, top, right);
+    }
+
+    @Test
+    public void testTopBottomCanBeFlippedOnIgnoreMalformed() {
+        GeoBoundingBoxQueryBuilder builder = createTestQueryBuilder();
+        double top = builder.topLeft().getLat();
+        double left = builder.topLeft().getLon();
+        double bottom = builder.bottomRight().getLat();
+        double right = builder.bottomRight().getLon();
+
+        assumeTrue("top should not be equal to bottom for flip check", top != bottom);
+        builder.coerce(false).ignoreMalformed(true).setCorners(bottom, left, top, right);
+    }
+
+    @Test
+    public void testLeftRightCanBeFlipped() {
+        GeoBoundingBoxQueryBuilder builder = createTestQueryBuilder();
+        double top = builder.topLeft().getLat();
+        double left = builder.topLeft().getLon();
+        double bottom = builder.bottomRight().getLat();
+        double right = builder.bottomRight().getLon();
+        
+        builder.ignoreMalformed(true).setCorners(top, right, bottom, left);
+        builder.ignoreMalformed(false).setCorners(top, right, bottom, left);
+    }
+
+    @Test
+    public void testNormalization() throws IOException {
+        assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0);
+        GeoBoundingBoxQueryBuilder qb = createTestQueryBuilder();
+        if (getCurrentTypes().length != 0 && "mapped_geo".equals(qb.fieldName())) {
+            // only execute this test if we are running on a valid geo field
+            qb.setCorners(200, 200, qb.bottomRight().getLat(), qb.bottomRight().getLon());
+            qb.coerce(true);
+            Query query = qb.toQuery(createShardContext());
+            if (query instanceof ConstantScoreQuery) {
+                ConstantScoreQuery result = (ConstantScoreQuery) query;
+                BooleanQuery bboxFilter = (BooleanQuery) result.getQuery();
+                for (BooleanClause clause : bboxFilter.clauses()) {
+                    NumericRangeQuery boundary = (NumericRangeQuery) clause.getQuery();
+                    if (boundary.getMax() != null) {
+                        assertTrue("If defined, non of the maximum range values should be larger than 180", boundary.getMax().intValue() <= 180);
+                    }
+                }
+            } else {
+                assertTrue("memory queries should result in InMemoryGeoBoundingBoxQuery", query instanceof InMemoryGeoBoundingBoxQuery);
+            }
+        }
+    }
+
+    @Override
+    protected void doAssertLuceneQuery(GeoBoundingBoxQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
+        if (queryBuilder.type() == GeoExecType.INDEXED) {
+            assertTrue("Found no indexed geo query.", query instanceof ConstantScoreQuery);
+        } else {
+            assertTrue("Found no indexed geo query.", query instanceof InMemoryGeoBoundingBoxQuery);
+        }
+    }
+
+    // Java really could do with function pointers - is there any Java8 feature that would help me here which I don't know of?
+    public abstract class PointTester {
+        private double brokenCoordinate = randomFrom(brokenDoubles);
+        private double invalidCoordinate;
+
+        public PointTester(double invalidCoodinate) {
+            this.invalidCoordinate = invalidCoodinate;
+        }
+        public void invalidateCoordinate(GeoBoundingBoxQueryBuilder qb, boolean useBrokenDouble) {
+            if (useBrokenDouble) {
+                fillIn(brokenCoordinate, qb);
+            } else {
+                fillIn(invalidCoordinate, qb);
+            }
+        }
+        protected abstract void fillIn(double fillIn, GeoBoundingBoxQueryBuilder qb);
+    }
+
+    public class TopTester extends PointTester {
+        public TopTester() {
+            super(randomDoubleBetween(GeoUtils.MAX_LAT, Double.MAX_VALUE, false));
+        }
+
+        @Override
+        public void fillIn(double coordinate, GeoBoundingBoxQueryBuilder qb) {
+            qb.setCorners(coordinate, qb.topLeft().getLon(), qb.bottomRight().getLat(), qb.bottomRight().getLon());
+        }
+    }
+
+    public class LeftTester extends PointTester {
+        public LeftTester() {
+            super(randomDoubleBetween(-Double.MAX_VALUE, GeoUtils.MIN_LON, true));
+        }
+
+        @Override
+        public void fillIn(double coordinate, GeoBoundingBoxQueryBuilder qb) {
+            qb.setCorners(qb.topLeft().getLat(), coordinate, qb.bottomRight().getLat(), qb.bottomRight().getLon());
+        }
+    }
+
+    public class BottomTester extends PointTester {
+        public BottomTester() {
+            super(randomDoubleBetween(-Double.MAX_VALUE, GeoUtils.MIN_LAT, false));
+        }
+
+        @Override
+        public void fillIn(double coordinate, GeoBoundingBoxQueryBuilder qb) {
+            qb.setCorners(qb.topLeft().getLat(), qb.topLeft().getLon(), coordinate, qb.bottomRight().getLon());
+        }
+    }
+
+    public class RightTester extends PointTester {
+        public RightTester() {
+            super(randomDoubleBetween(GeoUtils.MAX_LON, Double.MAX_VALUE, true));
+        } 
+
+        @Override
+        public void fillIn(double coordinate, GeoBoundingBoxQueryBuilder qb) {
+            qb.setCorners(qb.topLeft().getLat(), qb.topLeft().getLon(), qb.topLeft().getLat(), coordinate);
+        }
+    }
+}

core/src/test/java/org/elasticsearch/search/aggregations/bucket/GeoHashGridIT.java

@@ -212,7 +212,7 @@ public class GeoHashGridIT extends ESIntegTestCase {
     @Test
     public void filtered() throws Exception {
         GeoBoundingBoxQueryBuilder bbox = new GeoBoundingBoxQueryBuilder("location");
-        bbox.topLeft(smallestGeoHash).bottomRight(smallestGeoHash).queryName("bbox");
+        bbox.setCorners(smallestGeoHash, smallestGeoHash).queryName("bbox");
         for (int precision = 1; precision <= XGeoHashUtils.PRECISION; precision++) {
             SearchResponse response = client().prepareSearch("idx")
                     .addAggregation(

core/src/test/java/org/elasticsearch/search/geo/GeoBoundingBoxIT.java

@@ -91,7 +91,7 @@ public class GeoBoundingBoxIT extends ESIntegTestCase {
         client().admin().indices().prepareRefresh().execute().actionGet();
 
         SearchResponse searchResponse = client().prepareSearch() // from NY
-                .setQuery(geoBoundingBoxQuery("location").topLeft(40.73, -74.1).bottomRight(40.717, -73.99))
+                .setQuery(geoBoundingBoxQuery("location").setCorners(40.73, -74.1, 40.717, -73.99))
                 .execute().actionGet();
         assertThat(searchResponse.getHits().getTotalHits(), equalTo(2l));
         assertThat(searchResponse.getHits().hits().length, equalTo(2));
@@ -100,7 +100,7 @@ public class GeoBoundingBoxIT extends ESIntegTestCase {
         }
 
         searchResponse = client().prepareSearch() // from NY
-                .setQuery(geoBoundingBoxQuery("location").topLeft(40.73, -74.1).bottomRight(40.717, -73.99).type("indexed"))
+                .setQuery(geoBoundingBoxQuery("location").setCorners(40.73, -74.1, 40.717, -73.99).type("indexed"))
                 .execute().actionGet();
         assertThat(searchResponse.getHits().getTotalHits(), equalTo(2l));
         assertThat(searchResponse.getHits().hits().length, equalTo(2));
@@ -160,52 +160,52 @@ public class GeoBoundingBoxIT extends ESIntegTestCase {
         refresh();
 
         SearchResponse searchResponse = client().prepareSearch()
-                .setQuery(geoBoundingBoxQuery("location").topLeft(41, -11).bottomRight(40, 9))
+                .setQuery(geoBoundingBoxQuery("location").setCorners(41, -11, 40, 9))
                 .execute().actionGet();
         assertThat(searchResponse.getHits().getTotalHits(), equalTo(1l));
         assertThat(searchResponse.getHits().hits().length, equalTo(1));
         assertThat(searchResponse.getHits().getAt(0).id(), equalTo("2"));
         searchResponse = client().prepareSearch()
-                .setQuery(geoBoundingBoxQuery("location").topLeft(41, -11).bottomRight(40, 9).type("indexed"))
+                .setQuery(geoBoundingBoxQuery("location").setCorners(41, -11, 40, 9).type("indexed"))
                 .execute().actionGet();
         assertThat(searchResponse.getHits().getTotalHits(), equalTo(1l));
         assertThat(searchResponse.getHits().hits().length, equalTo(1));
         assertThat(searchResponse.getHits().getAt(0).id(), equalTo("2"));
 
         searchResponse = client().prepareSearch()
-                .setQuery(geoBoundingBoxQuery("location").topLeft(41, -9).bottomRight(40, 11))
+                .setQuery(geoBoundingBoxQuery("location").setCorners(41, -9, 40, 11))
                 .execute().actionGet();
         assertThat(searchResponse.getHits().getTotalHits(), equalTo(1l));
         assertThat(searchResponse.getHits().hits().length, equalTo(1));
         assertThat(searchResponse.getHits().getAt(0).id(), equalTo("3"));
         searchResponse = client().prepareSearch()
-                .setQuery(geoBoundingBoxQuery("location").topLeft(41, -9).bottomRight(40, 11).type("indexed"))
+                .setQuery(geoBoundingBoxQuery("location").setCorners(41, -9, 40, 11).type("indexed"))
                 .execute().actionGet();
         assertThat(searchResponse.getHits().getTotalHits(), equalTo(1l));
         assertThat(searchResponse.getHits().hits().length, equalTo(1));
         assertThat(searchResponse.getHits().getAt(0).id(), equalTo("3"));
 
         searchResponse = client().prepareSearch()
-                .setQuery(geoBoundingBoxQuery("location").topLeft(11, 171).bottomRight(1, -169))
+                .setQuery(geoBoundingBoxQuery("location").setCorners(11, 171, 1, -169))
                 .execute().actionGet();
         assertThat(searchResponse.getHits().getTotalHits(), equalTo(1l));
         assertThat(searchResponse.getHits().hits().length, equalTo(1));
         assertThat(searchResponse.getHits().getAt(0).id(), equalTo("5"));
         searchResponse = client().prepareSearch()
-                .setQuery(geoBoundingBoxQuery("location").topLeft(11, 171).bottomRight(1, -169).type("indexed"))
+                .setQuery(geoBoundingBoxQuery("location").setCorners(11, 171, 1, -169).type("indexed"))
                 .execute().actionGet();
         assertThat(searchResponse.getHits().getTotalHits(), equalTo(1l));
         assertThat(searchResponse.getHits().hits().length, equalTo(1));
         assertThat(searchResponse.getHits().getAt(0).id(), equalTo("5"));
 
         searchResponse = client().prepareSearch()
-                .setQuery(geoBoundingBoxQuery("location").topLeft(9, 169).bottomRight(-1, -171))
+                .setQuery(geoBoundingBoxQuery("location").setCorners(9, 169, -1, -171))
                 .execute().actionGet();
         assertThat(searchResponse.getHits().getTotalHits(), equalTo(1l));
         assertThat(searchResponse.getHits().hits().length, equalTo(1));
         assertThat(searchResponse.getHits().getAt(0).id(), equalTo("9"));
         searchResponse = client().prepareSearch()
-                .setQuery(geoBoundingBoxQuery("location").topLeft(9, 169).bottomRight(-1, -171).type("indexed"))
+                .setQuery(geoBoundingBoxQuery("location").setCorners(9, 169, -1, -171).type("indexed"))
                 .execute().actionGet();
         assertThat(searchResponse.getHits().getTotalHits(), equalTo(1l));
         assertThat(searchResponse.getHits().hits().length, equalTo(1));
@@ -239,26 +239,26 @@ public class GeoBoundingBoxIT extends ESIntegTestCase {
         SearchResponse searchResponse = client().prepareSearch()
                 .setQuery(
                         boolQuery().must(termQuery("userid", 880)).filter(
-                                geoBoundingBoxQuery("location").topLeft(74.579421999999994, 143.5).bottomRight(-66.668903999999998, 113.96875))
+                                geoBoundingBoxQuery("location").setCorners(74.579421999999994, 143.5, -66.668903999999998, 113.96875))
                 ).execute().actionGet();
         assertThat(searchResponse.getHits().totalHits(), equalTo(1l));
         searchResponse = client().prepareSearch()
                 .setQuery(
                         boolQuery().must(termQuery("userid", 880)).filter(
-                                geoBoundingBoxQuery("location").topLeft(74.579421999999994, 143.5).bottomRight(-66.668903999999998, 113.96875).type("indexed"))
+                                geoBoundingBoxQuery("location").setCorners(74.579421999999994, 143.5, -66.668903999999998, 113.96875).type("indexed"))
                 ).execute().actionGet();
         assertThat(searchResponse.getHits().totalHits(), equalTo(1l));
 
         searchResponse = client().prepareSearch()
                 .setQuery(
                         boolQuery().must(termQuery("userid", 534)).filter(
-                                geoBoundingBoxQuery("location").topLeft(74.579421999999994, 143.5).bottomRight(-66.668903999999998, 113.96875))
+                                geoBoundingBoxQuery("location").setCorners(74.579421999999994, 143.5, -66.668903999999998, 113.96875))
                 ).execute().actionGet();
         assertThat(searchResponse.getHits().totalHits(), equalTo(1l));
         searchResponse = client().prepareSearch()
                 .setQuery(
                         boolQuery().must(termQuery("userid", 534)).filter(
-                                geoBoundingBoxQuery("location").topLeft(74.579421999999994, 143.5).bottomRight(-66.668903999999998, 113.96875).type("indexed"))
+                                geoBoundingBoxQuery("location").setCorners(74.579421999999994, 143.5, -66.668903999999998, 113.96875).type("indexed"))
                 ).execute().actionGet();
         assertThat(searchResponse.getHits().totalHits(), equalTo(1l));
     }
@@ -289,43 +289,43 @@ public class GeoBoundingBoxIT extends ESIntegTestCase {
 
         SearchResponse searchResponse = client().prepareSearch()
                 .setQuery(
-                        geoBoundingBoxQuery("location").coerce(true).topLeft(50, -180).bottomRight(-50, 180)
+                        geoBoundingBoxQuery("location").coerce(true).setCorners(50, -180, -50, 180)
                 ).execute().actionGet();
         assertThat(searchResponse.getHits().totalHits(), equalTo(1l));
         searchResponse = client().prepareSearch()
                 .setQuery(
-                        geoBoundingBoxQuery("location").coerce(true).topLeft(50, -180).bottomRight(-50, 180).type("indexed")
+                        geoBoundingBoxQuery("location").coerce(true).setCorners(50, -180, -50, 180).type("indexed")
                 ).execute().actionGet();
         assertThat(searchResponse.getHits().totalHits(), equalTo(1l));
         searchResponse = client().prepareSearch()
                 .setQuery(
-                        geoBoundingBoxQuery("location").coerce(true).topLeft(90, -180).bottomRight(-90, 180)
+                        geoBoundingBoxQuery("location").coerce(true).setCorners(90, -180, -90, 180)
                 ).execute().actionGet();
         assertThat(searchResponse.getHits().totalHits(), equalTo(2l));
         searchResponse = client().prepareSearch()
                 .setQuery(
-                        geoBoundingBoxQuery("location").coerce(true).topLeft(90, -180).bottomRight(-90, 180).type("indexed")
+                        geoBoundingBoxQuery("location").coerce(true).setCorners(90, -180, -90, 180).type("indexed")
                 ).execute().actionGet();
         assertThat(searchResponse.getHits().totalHits(), equalTo(2l));
 
         searchResponse = client().prepareSearch()
                 .setQuery(
-                        geoBoundingBoxQuery("location").coerce(true).topLeft(50, 0).bottomRight(-50, 360)
+                        geoBoundingBoxQuery("location").coerce(true).setCorners(50, 0, -50, 360)
                 ).execute().actionGet();
         assertThat(searchResponse.getHits().totalHits(), equalTo(1l));
         searchResponse = client().prepareSearch()
                 .setQuery(
-                        geoBoundingBoxQuery("location").coerce(true).topLeft(50, 0).bottomRight(-50, 360).type("indexed")
+                        geoBoundingBoxQuery("location").coerce(true).setCorners(50, 0, -50, 360).type("indexed")
                 ).execute().actionGet();
         assertThat(searchResponse.getHits().totalHits(), equalTo(1l));
         searchResponse = client().prepareSearch()
                 .setQuery(
-                        geoBoundingBoxQuery("location").coerce(true).topLeft(90, 0).bottomRight(-90, 360)
+                        geoBoundingBoxQuery("location").coerce(true).setCorners(90, 0, -90, 360)
                 ).execute().actionGet();
         assertThat(searchResponse.getHits().totalHits(), equalTo(2l));
         searchResponse = client().prepareSearch()
                 .setQuery(
-                        geoBoundingBoxQuery("location").coerce(true).topLeft(90, 0).bottomRight(-90, 360).type("indexed")
+                        geoBoundingBoxQuery("location").coerce(true).setCorners(90, 0, -90, 360).type("indexed")
                 ).execute().actionGet();
         assertThat(searchResponse.getHits().totalHits(), equalTo(2l));
     }

core/src/test/java/org/elasticsearch/search/geo/GeoFilterIT.java

@@ -434,9 +434,7 @@ public class GeoFilterIT extends ESIntegTestCase {
         }
 
         SearchResponse world = client().prepareSearch().addField("pin").setQuery(
-                geoBoundingBoxQuery("pin")
-                        .topLeft(90, -179.99999)
-                        .bottomRight(-90, 179.99999)
+                geoBoundingBoxQuery("pin").setCorners(90, -179.99999, -90, 179.99999)
         ).execute().actionGet();
 
         assertHitCount(world, 53);