diff --git a/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java index 403ed19d5cac3..31bc889cb91b8 100644 --- a/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java @@ -182,6 +182,10 @@ public String indexedShapeType() { * @return this */ public GeoShapeQueryBuilder strategy(SpatialStrategy strategy) { + if (strategy != null && strategy == SpatialStrategy.TERM && relation != ShapeRelation.INTERSECTS) { + throw new IllegalArgumentException("strategy [" + strategy.getStrategyName() + "] only supports relation [" + + ShapeRelation.INTERSECTS.getRelationName() + "] found relation [" + relation.getRelationName() + "]"); + } this.strategy = strategy; return this; } @@ -240,6 +244,10 @@ public GeoShapeQueryBuilder relation(ShapeRelation relation) { if (relation == null) { throw new IllegalArgumentException("No Shape Relation defined"); } + if (strategy != null && strategy == SpatialStrategy.TERM && relation != ShapeRelation.INTERSECTS) { + throw new IllegalArgumentException("current strategy [" + strategy.getStrategyName() + "] only supports relation [" + + ShapeRelation.INTERSECTS.getRelationName() + "] found relation [" + relation.getRelationName() + "]"); + } this.relation = relation; return this; } @@ -251,17 +259,6 @@ public ShapeRelation relation() { return relation; } - public QueryValidationException validate() { - QueryValidationException errors = null; - // TODO did we validate this before the refactoring and can we do this in setters at all? - if (strategy != null && strategy == SpatialStrategy.TERM && relation != ShapeRelation.INTERSECTS) { - errors = QueryValidationException.addValidationError(NAME, - "strategy [" + strategy.getStrategyName() + "] only supports relation [" - + ShapeRelation.INTERSECTS.getRelationName() + "] found relation [" + relation.getRelationName() + "]", errors); - } - return errors; - } - @Override protected Query doToQuery(QueryShardContext context) throws IOException { ShapeBuilder shape; diff --git a/core/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java index 66ba602417821..a4ac66c658ca3 100644 --- a/core/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java @@ -166,37 +166,31 @@ public void testNoIndexedShapeType() throws IOException { new GeoShapeQueryBuilder(GEO_SHAPE_FIELD_NAME, "id", (String) null); } - @Test + @Test(expected=IllegalArgumentException.class) public void testNoRelation() throws IOException { ShapeBuilder shape = RandomShapeGenerator.createShapeWithin(getRandom(), null); - try { - GeoShapeQueryBuilder builder = new GeoShapeQueryBuilder(GEO_SHAPE_FIELD_NAME, shape); - builder.relation(null); - fail("relation cannot be null"); - } catch (IllegalArgumentException e) { - // expected - } + GeoShapeQueryBuilder builder = new GeoShapeQueryBuilder(GEO_SHAPE_FIELD_NAME, shape); + builder.relation(null); } @Test - public void testInvalidRelation() { + public void testInvalidRelation() throws IOException { ShapeBuilder shape = RandomShapeGenerator.createShapeWithin(getRandom(), null); + GeoShapeQueryBuilder builder = new GeoShapeQueryBuilder(GEO_SHAPE_FIELD_NAME, shape); try { - GeoShapeQueryBuilder builder = new GeoShapeQueryBuilder(GEO_SHAPE_FIELD_NAME, shape); builder.strategy(SpatialStrategy.TERM); - ShapeRelation relation = randomFrom(ShapeRelation.DISJOINT, ShapeRelation.WITHIN); - builder.relation(relation); - QueryValidationException exception = builder.validate(); - assertThat(exception, notNullValue()); - assertThat(exception.validationErrors(), notNullValue()); - assertThat(exception.validationErrors().size(), equalTo(1)); - assertThat( - exception.validationErrors().get(0), - equalTo("[" + GeoShapeQueryBuilder.NAME + "] strategy [" + SpatialStrategy.TERM.getStrategyName() - + "] only supports relation [" + ShapeRelation.INTERSECTS.getRelationName() + "] found relation [" - + relation.getRelationName() + "]")); - } catch (IOException e) { - throw new RuntimeException(e); + builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.WITHIN)); + fail("Illegal combination of strategy and relation setting"); + } catch (IllegalArgumentException e) { + // okay + } + + try { + builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.WITHIN)); + builder.strategy(SpatialStrategy.TERM); + fail("Illegal combination of strategy and relation setting"); + } catch (IllegalArgumentException e) { + // okay } }