From b861ec1cc00dcecb1d00a59799a8f56830030471 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Mon, 25 Jul 2016 20:00:35 +0200 Subject: [PATCH 1/2] Allow empty json object in request body in `_count` API When the request body is missing, all documents in the target index are counted. As mentioned in #19422, the same should happen when the request body is an empty json object. This is also the behaviour for the `_search` endpoint and the two APIs should behave in the same way. --- .../index/query/QueryParseContext.java | 3 --- .../rest-api-spec/test/count/10_basic.yaml | 18 ++++++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java index 510a8049630..33382a7ab73 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java @@ -73,9 +73,6 @@ public class QueryParseContext implements ParseFieldMatcherSupplier { } } } - if (queryBuilder == null) { - throw new ParsingException(parser.getTokenLocation(), "Required query is missing"); - } return queryBuilder; } catch (ParsingException e) { throw e; diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/count/10_basic.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/count/10_basic.yaml index f3eb0a5fae6..f38d2c315eb 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/count/10_basic.yaml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/count/10_basic.yaml @@ -37,6 +37,24 @@ setup: - match: {count : 0} +--- +"count with empty body": +# empty body should default to match_all query + - do: + count: + index: test + type: test + body: { } + + - match: {count : 1} + + - do: + count: + index: test + type: test + + - match: {count : 1} + --- "count body without query element": - do: From 4bac61425c2ceb11751f42a1eb3e7e7ec2a5f89e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 26 Jul 2016 12:48:48 +0200 Subject: [PATCH 2/2] Adding unit tests for QueryParseContext --- .../index/query/QueryParseContext.java | 2 +- .../index/query/QueryParseContextTests.java | 128 ++++++++++++++++++ 2 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 core/src/test/java/org/elasticsearch/index/query/QueryParseContextTests.java diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java index 33382a7ab73..daf0f6838b5 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java @@ -110,7 +110,7 @@ public class QueryParseContext implements ParseFieldMatcherSupplier { // move to the next START_OBJECT token = parser.nextToken(); if (token != XContentParser.Token.START_OBJECT && token != XContentParser.Token.START_ARRAY) { - throw new ParsingException(parser.getTokenLocation(), "[_na] query malformed, no field after start_object"); + throw new ParsingException(parser.getTokenLocation(), "[_na] query malformed, no start_object after query name"); } @SuppressWarnings("unchecked") Optional result = (Optional) indicesQueriesRegistry.lookup(queryName, parseFieldMatcher, diff --git a/core/src/test/java/org/elasticsearch/index/query/QueryParseContextTests.java b/core/src/test/java/org/elasticsearch/index/query/QueryParseContextTests.java new file mode 100644 index 00000000000..fb88adfc95f --- /dev/null +++ b/core/src/test/java/org/elasticsearch/index/query/QueryParseContextTests.java @@ -0,0 +1,128 @@ +/* + * 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.common.ParseFieldMatcher; +import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.io.stream.NamedWriteableRegistry; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.indices.query.IndicesQueriesRegistry; +import org.elasticsearch.search.SearchModule; +import org.elasticsearch.test.ESTestCase; +import org.junit.BeforeClass; + +import java.io.IOException; +import java.util.Optional; + +import static java.util.Collections.emptyList; + +public class QueryParseContextTests extends ESTestCase { + + private static IndicesQueriesRegistry indicesQueriesRegistry; + + @BeforeClass + public static void init() { + indicesQueriesRegistry = new SearchModule(Settings.EMPTY, new NamedWriteableRegistry(), false, emptyList()) + .getQueryParserRegistry(); + } + + public void testParseTopLevelBuilder() throws IOException { + QueryBuilder query = new MatchQueryBuilder("foo", "bar"); + String requestBody = "{ \"query\" : " + query.toString() + "}"; + try (XContentParser parser = XContentFactory.xContent(requestBody).createParser(requestBody)) { + QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, parser, ParseFieldMatcher.STRICT); + QueryBuilder actual = context.parseTopLevelQueryBuilder(); + assertEquals(query, actual); + } + } + + public void testParseTopLevelBuilderEmptyObject() throws IOException { + String requestBody = "{}"; + try (XContentParser parser = XContentFactory.xContent(requestBody).createParser(requestBody)) { + QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, parser, ParseFieldMatcher.STRICT); + QueryBuilder query = context.parseTopLevelQueryBuilder(); + assertNull(query); + } + } + + public void testParseTopLevelBuilderUnknownParameter() throws IOException { + String requestBody = "{ \"foo\" : \"bar\"}"; + try (XContentParser parser = XContentFactory.xContent(requestBody).createParser(requestBody)) { + QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, parser, ParseFieldMatcher.STRICT); + ParsingException exception = expectThrows(ParsingException.class, () -> context.parseTopLevelQueryBuilder()); + assertEquals("request does not support [foo]", exception.getMessage()); + } + } + + public void testParseInnerQueryBuilder() throws IOException { + QueryBuilder query = new MatchQueryBuilder("foo", "bar"); + String source = query.toString(); + try (XContentParser parser = XContentFactory.xContent(source).createParser(source)) { + QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, parser, ParseFieldMatcher.STRICT); + Optional actual = context.parseInnerQueryBuilder(); + assertEquals(query, actual.get()); + } + } + + public void testParseInnerQueryBuilderEmptyBody() throws IOException { + String source = "{}"; + try (XContentParser parser = XContentFactory.xContent(source).createParser(source)) { + QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, parser, ParseFieldMatcher.EMPTY); + Optional emptyQuery = context.parseInnerQueryBuilder(); + assertFalse(emptyQuery.isPresent()); + } + } + + public void testParseInnerQueryBuilderExceptions() throws IOException { + String source = "{ \"foo\": \"bar\" }"; + try (XContentParser parser = JsonXContent.jsonXContent.createParser(source)) { + parser.nextToken(); + parser.nextToken(); // don't start with START_OBJECT to provoke exception + QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, parser, ParseFieldMatcher.STRICT); + ParsingException exception = expectThrows(ParsingException.class, () -> context.parseInnerQueryBuilder()); + assertEquals("[_na] query malformed, must start with start_object", exception.getMessage()); + } + + source = "{}"; + try (XContentParser parser = JsonXContent.jsonXContent.createParser(source)) { + QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, parser, ParseFieldMatcher.STRICT); + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> context.parseInnerQueryBuilder()); + assertEquals("query malformed, empty clause found at [1:2]", exception.getMessage()); + } + + source = "{ \"foo\" : \"bar\" }"; + try (XContentParser parser = JsonXContent.jsonXContent.createParser(source)) { + QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, parser, ParseFieldMatcher.STRICT); + ParsingException exception = expectThrows(ParsingException.class, () -> context.parseInnerQueryBuilder()); + assertEquals("[_na] query malformed, no start_object after query name", exception.getMessage()); + } + + source = "{ \"foo\" : {} }"; + try (XContentParser parser = JsonXContent.jsonXContent.createParser(source)) { + QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, parser, ParseFieldMatcher.STRICT); + ParsingException exception = expectThrows(ParsingException.class, () -> context.parseInnerQueryBuilder()); + assertEquals("no [query] registered for [foo]", exception.getMessage()); + } + } + +}