diff --git a/core/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java index b85db4b66b1..1de8db2e801 100644 --- a/core/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java @@ -22,7 +22,6 @@ package org.elasticsearch.index.query; import org.apache.lucene.queries.TermsQuery; import org.apache.lucene.search.Query; import org.elasticsearch.cluster.metadata.MetaData; -import org.elasticsearch.common.Nullable; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.lucene.search.Queries; @@ -47,9 +46,19 @@ public class IdsQueryBuilder extends AbstractQueryBuilder { static final IdsQueryBuilder PROTOTYPE = new IdsQueryBuilder(); /** - * Creates a new IdsQueryBuilder by optionally providing the types of the documents to look for + * Creates a new IdsQueryBuilder without providing the types of the documents to look for */ - public IdsQueryBuilder(@Nullable String... types) { + public IdsQueryBuilder() { + this.types = new String[0]; + } + + /** + * Creates a new IdsQueryBuilder by providing the types of the documents to look for + */ + public IdsQueryBuilder(String... types) { + if (types == null) { + throw new IllegalArgumentException("[ids] types cannot be null"); + } this.types = types; } @@ -64,32 +73,13 @@ public class IdsQueryBuilder extends AbstractQueryBuilder { * Adds ids to the query. */ public IdsQueryBuilder addIds(String... ids) { + if (ids == null) { + throw new IllegalArgumentException("[ids] ids cannot be null"); + } Collections.addAll(this.ids, ids); return this; } - /** - * Adds ids to the query. - */ - public IdsQueryBuilder addIds(Collection ids) { - this.ids.addAll(ids); - return this; - } - - /** - * Adds ids to the filter. - */ - public IdsQueryBuilder ids(String... ids) { - return addIds(ids); - } - - /** - * Adds ids to the filter. - */ - public IdsQueryBuilder ids(Collection ids) { - return addIds(ids); - } - /** * Returns the ids for the query. */ @@ -100,13 +90,7 @@ public class IdsQueryBuilder extends AbstractQueryBuilder { @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(NAME); - if (types != null) { - if (types.length == 1) { - builder.field("type", types[0]); - } else { - builder.array("types", types); - } - } + builder.array("types", types); builder.startArray("values"); for (String value : ids) { builder.value(value); @@ -128,7 +112,7 @@ public class IdsQueryBuilder extends AbstractQueryBuilder { query = Queries.newMatchNoDocsQuery(); } else { Collection typesForQuery; - if (types == null || types.length == 0) { + if (types.length == 0) { typesForQuery = context.queryTypes(); } else if (types.length == 1 && MetaData.ALL.equals(types[0])) { typesForQuery = context.mapperService().types(); diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryBuilders.java b/core/src/main/java/org/elasticsearch/index/query/QueryBuilders.java index df823e166f7..67e654cb0d8 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryBuilders.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryBuilders.java @@ -19,7 +19,6 @@ package org.elasticsearch.index.query; -import org.elasticsearch.common.Nullable; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.geo.ShapeRelation; @@ -109,12 +108,19 @@ public abstract class QueryBuilders { return new DisMaxQueryBuilder(); } + /** + * Constructs a query that will match only specific ids within all types. + */ + public static IdsQueryBuilder idsQuery() { + return new IdsQueryBuilder(); + } + /** * Constructs a query that will match only specific ids within types. * * @param types The mapping/doc type */ - public static IdsQueryBuilder idsQuery(@Nullable String... types) { + public static IdsQueryBuilder idsQuery(String... types) { return new IdsQueryBuilder(types); } diff --git a/core/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java index 7ec3e3f8d25..177caf08711 100644 --- a/core/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java @@ -121,4 +121,20 @@ public class IdsQueryBuilderTests extends AbstractQueryTestCase return alternateVersions; } + + public void testIllegalArguments() { + try { + new IdsQueryBuilder((String[])null); + fail("must be not null"); + } catch(IllegalArgumentException e) { + //all good + } + + try { + new IdsQueryBuilder().addIds((String[])null); + fail("must be not null"); + } catch(IllegalArgumentException e) { + //all good + } + } } diff --git a/core/src/test/java/org/elasticsearch/search/child/ChildQuerySearchIT.java b/core/src/test/java/org/elasticsearch/search/child/ChildQuerySearchIT.java index 335b584e12b..09f33f60ed5 100644 --- a/core/src/test/java/org/elasticsearch/search/child/ChildQuerySearchIT.java +++ b/core/src/test/java/org/elasticsearch/search/child/ChildQuerySearchIT.java @@ -181,7 +181,7 @@ public class ChildQuerySearchIT extends ESIntegTestCase { refresh(); // TEST FETCHING _parent from child - SearchResponse searchResponse = client().prepareSearch("test").setQuery(idsQuery("child").ids("c1")).addFields("_parent").execute() + SearchResponse searchResponse = client().prepareSearch("test").setQuery(idsQuery("child").addIds("c1")).addFields("_parent").execute() .actionGet(); assertNoFailures(searchResponse); assertThat(searchResponse.getHits().totalHits(), equalTo(1l)); diff --git a/core/src/test/java/org/elasticsearch/search/compress/SearchSourceCompressTests.java b/core/src/test/java/org/elasticsearch/search/compress/SearchSourceCompressTests.java index b6d335318ce..d3b5160db4f 100644 --- a/core/src/test/java/org/elasticsearch/search/compress/SearchSourceCompressTests.java +++ b/core/src/test/java/org/elasticsearch/search/compress/SearchSourceCompressTests.java @@ -84,7 +84,7 @@ public class SearchSourceCompressTests extends ESSingleNodeTestCase { assertThat(getResponse.getSourceAsBytes(), equalTo(buildSource(10000).bytes().toBytes())); for (int i = 1; i < 100; i++) { - SearchResponse searchResponse = client().prepareSearch().setQuery(QueryBuilders.idsQuery("type1").ids(Integer.toString(i))).execute().actionGet(); + SearchResponse searchResponse = client().prepareSearch().setQuery(QueryBuilders.idsQuery("type1").addIds(Integer.toString(i))).execute().actionGet(); assertThat(searchResponse.getHits().getTotalHits(), equalTo(1l)); assertThat(searchResponse.getHits().getAt(0).source(), equalTo(buildSource(i).bytes().toBytes())); } diff --git a/docs/reference/migration/migrate_3_0.asciidoc b/docs/reference/migration/migrate_3_0.asciidoc index 6607a98f0fa..897098fdbb6 100644 --- a/docs/reference/migration/migrate_3_0.asciidoc +++ b/docs/reference/migration/migrate_3_0.asciidoc @@ -262,3 +262,7 @@ of string values: see `FilterFunctionScoreQuery.ScoreMode` and `CombineFunction` `CombineFunction.MULT` has been renamed to `MULTIPLY`. +==== IdsQueryBuilder + +For simplicity, only one way of adding the ids to the existing list (empty by default) is left: `addIds(String...)` + diff --git a/plugins/lang-groovy/src/test/java/org/elasticsearch/messy/tests/SearchQueryTests.java b/plugins/lang-groovy/src/test/java/org/elasticsearch/messy/tests/SearchQueryTests.java index 89f0778f8ce..7b8ea710a00 100644 --- a/plugins/lang-groovy/src/test/java/org/elasticsearch/messy/tests/SearchQueryTests.java +++ b/plugins/lang-groovy/src/test/java/org/elasticsearch/messy/tests/SearchQueryTests.java @@ -28,6 +28,7 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchType; import org.elasticsearch.action.search.ShardSearchFailure; import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.mapper.MapperParsingException; @@ -677,25 +678,25 @@ public class SearchQueryTests extends ESIntegTestCase { client().prepareIndex("test", "type1", "2").setSource("field1", "value2"), client().prepareIndex("test", "type1", "3").setSource("field1", "value3")); - SearchResponse searchResponse = client().prepareSearch().setQuery(constantScoreQuery(idsQuery("type1").ids("1", "3"))).get(); + SearchResponse searchResponse = client().prepareSearch().setQuery(constantScoreQuery(idsQuery("type1").addIds("1", "3"))).get(); assertHitCount(searchResponse, 2l); assertSearchHits(searchResponse, "1", "3"); // no type - searchResponse = client().prepareSearch().setQuery(constantScoreQuery(idsQuery().ids("1", "3"))).get(); + searchResponse = client().prepareSearch().setQuery(constantScoreQuery(idsQuery().addIds("1", "3"))).get(); assertHitCount(searchResponse, 2l); assertSearchHits(searchResponse, "1", "3"); - searchResponse = client().prepareSearch().setQuery(idsQuery("type1").ids("1", "3")).get(); + searchResponse = client().prepareSearch().setQuery(idsQuery("type1").addIds("1", "3")).get(); assertHitCount(searchResponse, 2l); assertSearchHits(searchResponse, "1", "3"); // no type - searchResponse = client().prepareSearch().setQuery(idsQuery().ids("1", "3")).get(); + searchResponse = client().prepareSearch().setQuery(idsQuery().addIds("1", "3")).get(); assertHitCount(searchResponse, 2l); assertSearchHits(searchResponse, "1", "3"); - searchResponse = client().prepareSearch().setQuery(idsQuery("type1").ids("7", "10")).get(); + searchResponse = client().prepareSearch().setQuery(idsQuery("type1").addIds("7", "10")).get(); assertHitCount(searchResponse, 0l); // repeat..., with terms @@ -1292,52 +1293,6 @@ public class SearchQueryTests extends ESIntegTestCase { assertHitCount(searchResponse, 0l); } - @Test - public void testBasicFilterById() throws Exception { - createIndex("test"); - - client().prepareIndex("test", "type1", "1").setSource("field1", "value1").get(); - client().prepareIndex("test", "type2", "2").setSource("field1", "value2").get(); - refresh(); - - SearchResponse searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setPostFilter(idsQuery("type1").ids("1")).get(); - assertHitCount(searchResponse, 1l); - assertThat(searchResponse.getHits().hits().length, equalTo(1)); - - searchResponse = client().prepareSearch().setQuery(constantScoreQuery(idsQuery("type1", "type2").ids("1", "2"))).get(); - assertHitCount(searchResponse, 2l); - assertThat(searchResponse.getHits().hits().length, equalTo(2)); - - searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setPostFilter(idsQuery().ids("1")).get(); - assertHitCount(searchResponse, 1l); - assertThat(searchResponse.getHits().hits().length, equalTo(1)); - - searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setPostFilter(idsQuery().ids("1", "2")).get(); - assertHitCount(searchResponse, 2l); - assertThat(searchResponse.getHits().hits().length, equalTo(2)); - - searchResponse = client().prepareSearch().setQuery(constantScoreQuery(idsQuery().ids("1", "2"))).get(); - assertHitCount(searchResponse, 2l); - assertThat(searchResponse.getHits().hits().length, equalTo(2)); - - searchResponse = client().prepareSearch().setQuery(constantScoreQuery(idsQuery("type1").ids("1", "2"))).get(); - assertHitCount(searchResponse, 1l); - assertThat(searchResponse.getHits().hits().length, equalTo(1)); - - searchResponse = client().prepareSearch().setQuery(constantScoreQuery(idsQuery().ids("1"))).get(); - assertHitCount(searchResponse, 1l); - assertThat(searchResponse.getHits().hits().length, equalTo(1)); - - // TODO: why do we even support passing null?? - searchResponse = client().prepareSearch().setQuery(constantScoreQuery(idsQuery((String[])null).ids("1"))).get(); - assertHitCount(searchResponse, 1l); - assertThat(searchResponse.getHits().hits().length, equalTo(1)); - - searchResponse = client().prepareSearch().setQuery(constantScoreQuery(idsQuery("type1", "type2", "type3").ids("1", "2", "3", "4"))).get(); - assertHitCount(searchResponse, 2l); - assertThat(searchResponse.getHits().hits().length, equalTo(2)); - } - @Test public void testBasicQueryById() throws Exception { createIndex("test"); @@ -1346,32 +1301,27 @@ public class SearchQueryTests extends ESIntegTestCase { client().prepareIndex("test", "type2", "2").setSource("field1", "value2").get(); refresh(); - SearchResponse searchResponse = client().prepareSearch().setQuery(idsQuery("type1", "type2").ids("1", "2")).get(); + SearchResponse searchResponse = client().prepareSearch().setQuery(idsQuery("type1", "type2").addIds("1", "2")).get(); assertHitCount(searchResponse, 2l); assertThat(searchResponse.getHits().hits().length, equalTo(2)); - searchResponse = client().prepareSearch().setQuery(idsQuery().ids("1")).get(); + searchResponse = client().prepareSearch().setQuery(idsQuery().addIds("1")).get(); assertHitCount(searchResponse, 1l); assertThat(searchResponse.getHits().hits().length, equalTo(1)); - searchResponse = client().prepareSearch().setQuery(idsQuery().ids("1", "2")).get(); + searchResponse = client().prepareSearch().setQuery(idsQuery().addIds("1", "2")).get(); assertHitCount(searchResponse, 2l); assertThat(searchResponse.getHits().hits().length, equalTo(2)); - - searchResponse = client().prepareSearch().setQuery(idsQuery("type1").ids("1", "2")).get(); + searchResponse = client().prepareSearch().setQuery(idsQuery("type1").addIds("1", "2")).get(); assertHitCount(searchResponse, 1l); assertThat(searchResponse.getHits().hits().length, equalTo(1)); - searchResponse = client().prepareSearch().setQuery(idsQuery().ids("1")).get(); + searchResponse = client().prepareSearch().setQuery(idsQuery(Strings.EMPTY_ARRAY).addIds("1")).get(); assertHitCount(searchResponse, 1l); assertThat(searchResponse.getHits().hits().length, equalTo(1)); - searchResponse = client().prepareSearch().setQuery(idsQuery((String[])null).ids("1")).get(); - assertHitCount(searchResponse, 1l); - assertThat(searchResponse.getHits().hits().length, equalTo(1)); - - searchResponse = client().prepareSearch().setQuery(idsQuery("type1", "type2", "type3").ids("1", "2", "3", "4")).get(); + searchResponse = client().prepareSearch().setQuery(idsQuery("type1", "type2", "type3").addIds("1", "2", "3", "4")).get(); assertHitCount(searchResponse, 2l); assertThat(searchResponse.getHits().hits().length, equalTo(2)); }