Merge branch 'master' into feature/search-request-refactoring

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;
@@ -52,9 +51,19 @@ public class IdsQueryBuilder extends AbstractQueryBuilder<IdsQueryBuilder> {
     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;
     }
 
@@ -69,32 +78,13 @@ public class IdsQueryBuilder extends AbstractQueryBuilder<IdsQueryBuilder> {
      * 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<String> 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<String> ids) {
-        return addIds(ids);
-    }
-
     /**
      * Returns the ids for the query.
      */
@@ -105,13 +95,7 @@ public class IdsQueryBuilder extends AbstractQueryBuilder<IdsQueryBuilder> {
     @Override
     protected void doXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject(NAME);
-        if (types != null) {
+        builder.array("types", types);
-            if (types.length == 1) {
-                builder.field("type", types[0]);
-            } else {
-                builder.array("types", types);
-            }
-        }
         builder.startArray("values");
         for (String value : ids) {
             builder.value(value);
@@ -133,7 +117,7 @@ public class IdsQueryBuilder extends AbstractQueryBuilder<IdsQueryBuilder> {
              query = Queries.newMatchNoDocsQuery();
         } else {
             Collection<String> 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();

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

@@ -218,15 +218,6 @@ public class IndexQueryParserService extends AbstractIndexComponent {
         }
     }
 
-    @Nullable
-    public Query parseInnerQuery(QueryShardContext context) throws IOException {
-        Query query = context.parseContext().parseInnerQueryBuilder().toQuery(context);
-        if (query == null) {
-            query = Queries.newMatchNoDocsQuery();
-        }
-        return query;
-    }
-
     public QueryShardContext getShardContext() {
         return cache.get();
     }
@@ -279,16 +270,41 @@ public class IndexQueryParserService extends AbstractIndexComponent {
         context.reset(parser);
         try {
             context.parseFieldMatcher(parseFieldMatcher);
-            Query query = context.parseContext().parseInnerQueryBuilder().toQuery(context);
+            Query query = parseInnerQuery(context);
-        if (query == null) {
+            return new ParsedQuery(query, context.copyNamedQueries());
-            query = Queries.newMatchNoDocsQuery();
-        }
-        return new ParsedQuery(query, context.copyNamedQueries());
         } finally {
             context.reset(null);
         }
     }
 
+    public Query parseInnerQuery(QueryShardContext context) throws IOException {
+        return toQuery(context.parseContext().parseInnerQueryBuilder(), context);
+            }
+
+    public ParsedQuery toQuery(QueryBuilder<?> queryBuilder) {
+        QueryShardContext context = cache.get();
+        context.reset();
+        context.parseFieldMatcher(parseFieldMatcher);
+        try {
+            Query query = toQuery(queryBuilder, context);
+            return new ParsedQuery(query, context.copyNamedQueries());
+        } catch(QueryShardException | ParsingException e ) {
+            throw e;
+        } catch(Exception e) {
+            throw new QueryShardException(context, "failed to create query: {}", e, queryBuilder);
+        } finally {
+            context.reset();
+        }
+    }
+
+    private static Query toQuery(QueryBuilder<?> queryBuilder, QueryShardContext context) throws IOException {
+        Query query = queryBuilder.toQuery(context);
+        if (query == null) {
+            query = Queries.newMatchNoDocsQuery();
+        }
+        return query;
+    }
+
     public ParseFieldMatcher parseFieldMatcher() {
         return parseFieldMatcher;
     }

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);
     }
 

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

@@ -121,4 +121,20 @@ public class IdsQueryBuilderTests extends AbstractQueryTestCase<IdsQueryBuilder>
 
         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
+        }
+    }
 }

core/src/test/java/org/elasticsearch/search/child/ChildQuerySearchIT.java

@@ -209,7 +209,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));

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()));
         }

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...)`
+

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;
@@ -679,25 +680,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
@@ -1294,52 +1295,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");
@@ -1348,32 +1303,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").addIds("1", "2")).get();
-        searchResponse = client().prepareSearch().setQuery(idsQuery("type1").ids("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();
+        searchResponse = client().prepareSearch().setQuery(idsQuery("type1", "type2", "type3").addIds("1", "2", "3", "4")).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();
         assertHitCount(searchResponse, 2l);
         assertThat(searchResponse.getHits().hits().length, equalTo(2));
     }