From 60661ec9ccad6bc0552d1e4ee250b4d47e4753c8 Mon Sep 17 00:00:00 2001 From: David Pilato Date: Fri, 24 Feb 2017 13:52:36 +0100 Subject: [PATCH 001/105] Add first High level client documentation The REST Client is split into 2 parts: * Low level * High level The High level client has a main common section and the document delete API documentation as a start. --- docs/java-rest/high-level/apis.asciidoc | 10 ++ .../high-level/document/delete.asciidoc | 92 +++++++++++++++++++ .../high-level/document/index.asciidoc | 1 + docs/java-rest/high-level/index.asciidoc | 14 +++ docs/java-rest/high-level/usage.asciidoc | 83 +++++++++++++++++ docs/java-rest/index.asciidoc | 6 +- docs/java-rest/license.asciidoc | 17 ++++ .../{ => low-level}/configuration.asciidoc | 12 +-- docs/java-rest/low-level/index.asciidoc | 27 ++++++ .../{ => low-level}/sniffer.asciidoc | 14 +-- docs/java-rest/{ => low-level}/usage.asciidoc | 35 ++++--- docs/java-rest/overview.asciidoc | 45 ++------- 12 files changed, 290 insertions(+), 66 deletions(-) create mode 100644 docs/java-rest/high-level/apis.asciidoc create mode 100644 docs/java-rest/high-level/document/delete.asciidoc create mode 100644 docs/java-rest/high-level/document/index.asciidoc create mode 100644 docs/java-rest/high-level/index.asciidoc create mode 100644 docs/java-rest/high-level/usage.asciidoc create mode 100644 docs/java-rest/license.asciidoc rename docs/java-rest/{ => low-level}/configuration.asciidoc (98%) create mode 100644 docs/java-rest/low-level/index.asciidoc rename docs/java-rest/{ => low-level}/sniffer.asciidoc (97%) rename docs/java-rest/{ => low-level}/usage.asciidoc (95%) diff --git a/docs/java-rest/high-level/apis.asciidoc b/docs/java-rest/high-level/apis.asciidoc new file mode 100644 index 00000000000..f021e93b84b --- /dev/null +++ b/docs/java-rest/high-level/apis.asciidoc @@ -0,0 +1,10 @@ +* index API + +* get API + +* <> + +* bulk API + +* search API + diff --git a/docs/java-rest/high-level/document/delete.asciidoc b/docs/java-rest/high-level/document/delete.asciidoc new file mode 100644 index 00000000000..46f70b59e62 --- /dev/null +++ b/docs/java-rest/high-level/document/delete.asciidoc @@ -0,0 +1,92 @@ +[[java-rest-high-document-delete]] +=== Delete API + +[[java-rest-high-document-delete-request]] +==== Delete Request + +The most simple Delete Request needs is: + +[source,java] +-------------------------------------------------- +DeleteRequest request = new DeleteRequest( + "index", <1> + "type", <2> + "id"); <3> +-------------------------------------------------- +<1> Index name +<2> Type +<3> Document id + +You can also provide the following properties: + +[source,java] +-------------------------------------------------- +request.timeout(TimeValue.timeValueSeconds(1)); <1> +request.timeout("1s"); <2> +request.setRefreshPolicy(WriteRequest.RefreshPolicy.WAIT_UNTIL); <3> +request.setRefreshPolicy("wait_for"); <4> +request.version(2); <5> +request.versionType(VersionType.EXTERNAL); <6> +-------------------------------------------------- +<1> Timeout +<2> Timeout as String +<3> Refresh policy +<4> Refresh policy as String +<5> Version +<6> Version type + +[[java-rest-high-document-delete-sync]] +==== Execution + +[source,java] +-------------------------------------------------- +DeleteResponse response = client.delete(request); +-------------------------------------------------- + +[[java-rest-high-document-delete-async]] +==== Asynchronous Execution + +[source,java] +-------------------------------------------------- +client.deleteAsync(request, new ActionListener() { + @Override + public void onResponse(DeleteResponse deleteResponse) { + <1> + } + + @Override + public void onFailure(Exception e) { + <2> + } +}); +-------------------------------------------------- +<1> Implement if needed when execution did not throw an exception +<2> Implement if needed in case of failure + +[[java-rest-high-document-delete-response]] +==== Delete Response + +In the Delete Response object, you can check for example the result of the operation: + +[source,java] +-------------------------------------------------- +if (response.getResult().equals(DocWriteResponse.Result.NOT_FOUND)) { + <1> +} +-------------------------------------------------- +<1> Do something if we did not find the document which should have been deleted + +Note that if you have a version conflict because you defined the version within the +<>, it will raise an `ElasticsearchException` like: + +[source,java] +-------------------------------------------------- +try { + client.delete(request); +} catch (ElasticsearchException exception) { + if (exception.status().equals(RestStatus.CONFLICT) { + <1> + } +} +-------------------------------------------------- +<1> We got a version conflict diff --git a/docs/java-rest/high-level/document/index.asciidoc b/docs/java-rest/high-level/document/index.asciidoc new file mode 100644 index 00000000000..15eee483baa --- /dev/null +++ b/docs/java-rest/high-level/document/index.asciidoc @@ -0,0 +1 @@ +include::delete.asciidoc[] diff --git a/docs/java-rest/high-level/index.asciidoc b/docs/java-rest/high-level/index.asciidoc new file mode 100644 index 00000000000..8b8dbe65ffd --- /dev/null +++ b/docs/java-rest/high-level/index.asciidoc @@ -0,0 +1,14 @@ +[[java-rest-high]] +== Java High Level REST Client + +The <>'s features include: + +include::apis.asciidoc[] + +It depends on elasticsearch core project as it uses elasticsearch request and response +objects so it will simplify a migration from the transport client. + + +include::usage.asciidoc[] + +include::document/index.asciidoc[] diff --git a/docs/java-rest/high-level/usage.asciidoc b/docs/java-rest/high-level/usage.asciidoc new file mode 100644 index 00000000000..7374923d041 --- /dev/null +++ b/docs/java-rest/high-level/usage.asciidoc @@ -0,0 +1,83 @@ +[[java-rest-high-usage]] +=== Getting started + +[[java-rest-high-usage-maven]] +==== Maven Repository + +The high-level Java REST client is hosted on +http://search.maven.org/#search%7Cga%7C1%7Cg%3A%22org.elasticsearch.client%22[Maven +Central]. The minimum Java version required is `1.8`. + +The high-level REST client is subject to the same release cycle as +elasticsearch. Replace the version with the desired client version, first +released with `5.x.x`. There is no relation between the client version +and the elasticsearch version that the client can communicate with as +it depends on the <> which is compatible with all elasticsearch versions. + +[[java-rest-high-usage-maven-maven]] +===== Maven configuration + +Here is how you can configure the dependency using maven as a dependency manager. +Add the following to your `pom.xml` file: + +["source","xml",subs="attributes"] +-------------------------------------------------- + + org.elasticsearch.client + rest-high-level + {version} + +-------------------------------------------------- + +[[java-rest-high-usage-maven-gradle]] +===== Gradle configuration + +Here is how you can configure the dependency using gradle as a dependency manager. +Add the following to your `build.gradle` file: + +["source","groovy",subs="attributes"] +-------------------------------------------------- +dependencies { + compile 'org.elasticsearch.client:rest-high-level:{version}' +} +-------------------------------------------------- + +[[java-rest-high-usage-dependencies]] +==== Dependencies + +The high-level Java REST client depends on the following artifacts and their +transitive dependencies: + +- org.elasticsearch.client:rest +- org.elasticsearch:elasticsearch + + +[[java-rest-high-usage-initialization]] +==== Initialization + +A `RestHighLevelClient` instance needs a <> +to be built as follows: + +[source,java] +-------------------------------------------------- +RestHighLevelClient client = + new RestHighLevelClient(lowLevelRestClient); <1> +-------------------------------------------------- +<1> We pass the <> instance + +In the rest of this documentation about the high-level client, the `RestHighLevelClient` instance +will be referenced as `client`. + +Then you have access to the high level APIs such as: + +include::apis.asciidoc[] + +Each API comes with 2 ways of executing it: + +* Synchronously, for example method <> + +* Asynchronously which has `Async` added to the synchronous method name. Like +<>. In which case you will have to +provide a listener. + + diff --git a/docs/java-rest/index.asciidoc b/docs/java-rest/index.asciidoc index 683c495c7f8..d4447019924 100644 --- a/docs/java-rest/index.asciidoc +++ b/docs/java-rest/index.asciidoc @@ -5,8 +5,8 @@ include::../Versions.asciidoc[] include::overview.asciidoc[] -include::usage.asciidoc[] +include::low-level/index.asciidoc[] -include::configuration.asciidoc[] +include::high-level/index.asciidoc[] -include::sniffer.asciidoc[] +include::license.asciidoc[] diff --git a/docs/java-rest/license.asciidoc b/docs/java-rest/license.asciidoc new file mode 100644 index 00000000000..0fb617d2f40 --- /dev/null +++ b/docs/java-rest/license.asciidoc @@ -0,0 +1,17 @@ +[[java-rest-license]] +== License + +Copyright 2013-2017 Elasticsearch + +Licensed 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. + diff --git a/docs/java-rest/configuration.asciidoc b/docs/java-rest/low-level/configuration.asciidoc similarity index 98% rename from docs/java-rest/configuration.asciidoc rename to docs/java-rest/low-level/configuration.asciidoc index 5fc6d37daa8..a75e1620c7e 100644 --- a/docs/java-rest/configuration.asciidoc +++ b/docs/java-rest/low-level/configuration.asciidoc @@ -1,4 +1,4 @@ -== Common configuration +=== Common configuration The `RestClientBuilder` supports providing both a `RequestConfigCallback` and an `HttpClientConfigCallback` which allow for any customization that the Apache @@ -8,7 +8,7 @@ configuration that the `RestClient` is initialized with. This section describes some common scenarios that require additional configuration for the low-level Java REST Client. -=== Timeouts +==== Timeouts Configuring requests timeouts can be done by providing an instance of `RequestConfigCallback` while building the `RestClient` through its builder. @@ -34,7 +34,7 @@ RestClient restClient = RestClient.builder(new HttpHost("localhost", 9200)) .build(); -------------------------------------------------- -=== Number of threads +==== Number of threads The Apache Http Async Client starts by default one dispatcher thread, and a number of worker threads used by the connection manager, as many as the number @@ -55,7 +55,7 @@ RestClient restClient = RestClient.builder(new HttpHost("localhost", 9200)) .build(); -------------------------------------------------- -=== Basic authentication +==== Basic authentication Configuring basic authentication can be done by providing an `HttpClientConfigCallback` while building the `RestClient` through its builder. @@ -104,7 +104,7 @@ RestClient restClient = RestClient.builder(new HttpHost("localhost", 9200)) .build(); -------------------------------------------------- -=== Encrypted communication +==== Encrypted communication Encrypted communication can also be configured through the `HttpClientConfigCallback`. The @@ -130,7 +130,7 @@ RestClient restClient = RestClient.builder(new HttpHost("localhost", 9200)) .build(); -------------------------------------------------- -=== Others +==== Others For any other required configuration needed, the Apache HttpAsyncClient docs should be consulted: https://hc.apache.org/httpcomponents-asyncclient-4.1.x/ . diff --git a/docs/java-rest/low-level/index.asciidoc b/docs/java-rest/low-level/index.asciidoc new file mode 100644 index 00000000000..b5ad9d82e28 --- /dev/null +++ b/docs/java-rest/low-level/index.asciidoc @@ -0,0 +1,27 @@ +[[java-rest-low]] +== Java Low Level REST Client + +The low-level client's features include: + +* minimal dependencies + +* load balancing across all available nodes + +* failover in case of node failures and upon specific response codes + +* failed connection penalization (whether a failed node is retried depends on + how many consecutive times it failed; the more failed attempts the longer the + client will wait before trying that same node again) + +* persistent connections + +* trace logging of requests and responses + +* optional automatic <> + + +include::usage.asciidoc[] + +include::configuration.asciidoc[] + +include::sniffer.asciidoc[] diff --git a/docs/java-rest/sniffer.asciidoc b/docs/java-rest/low-level/sniffer.asciidoc similarity index 97% rename from docs/java-rest/sniffer.asciidoc rename to docs/java-rest/low-level/sniffer.asciidoc index 6c4c531306b..081ecd3dd66 100644 --- a/docs/java-rest/sniffer.asciidoc +++ b/docs/java-rest/low-level/sniffer.asciidoc @@ -1,5 +1,5 @@ [[sniffer]] -== Sniffer +=== Sniffer Minimal library that allows to automatically discover nodes from a running Elasticsearch cluster and set them to an existing `RestClient` instance. @@ -8,7 +8,7 @@ Nodes Info api and uses jackson to parse the obtained json response. Compatible with Elasticsearch 2.x and onwards. -=== Maven Repository +==== Maven Repository The low-level REST client is subject to the same release cycle as elasticsearch. Replace the version with the desired sniffer version, first @@ -17,7 +17,7 @@ and the elasticsearch version that the client can communicate with. Sniffer supports fetching the nodes list from elasticsearch 2.x and onwards. -==== Maven configuration +===== Maven configuration Here is how you can configure the dependency using maven as a dependency manager. Add the following to your `pom.xml` file: @@ -31,7 +31,7 @@ Add the following to your `pom.xml` file: -------------------------------------------------- -==== Gradle configuration +===== Gradle configuration Here is how you can configure the dependency using gradle as a dependency manager. Add the following to your `build.gradle` file: @@ -43,7 +43,7 @@ dependencies { } -------------------------------------------------- -=== Usage +==== Usage Once a `RestClient` instance has been created, a `Sniffer` can be associated to it. The `Sniffer` will make use of the provided `RestClient` to periodically @@ -133,9 +133,9 @@ Sniffer sniffer = Sniffer.builder(restClient) Note that this last configuration parameter has no effect in case sniffing on failure is not enabled like explained above. -=== License +==== License -Copyright 2013-2016 Elasticsearch +Copyright 2013-2017 Elasticsearch Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/docs/java-rest/usage.asciidoc b/docs/java-rest/low-level/usage.asciidoc similarity index 95% rename from docs/java-rest/usage.asciidoc rename to docs/java-rest/low-level/usage.asciidoc index 613b4ab7b8d..c83f9986d1f 100644 --- a/docs/java-rest/usage.asciidoc +++ b/docs/java-rest/low-level/usage.asciidoc @@ -1,6 +1,8 @@ -== Getting started +[[java-rest-low-usage]] +=== Getting started -=== Maven Repository +[[java-rest-low-usage-maven]] +==== Maven Repository The low-level Java REST client is hosted on http://search.maven.org/#search%7Cga%7C1%7Cg%3A%22org.elasticsearch.client%22[Maven @@ -8,11 +10,12 @@ Central]. The minimum Java version required is `1.7`. The low-level REST client is subject to the same release cycle as elasticsearch. Replace the version with the desired client version, first -released with `5.0.0-alpha4`. There is no relation between the client version +released with `5.0.0-alpha4`. There is no relation between the client version and the elasticsearch version that the client can communicate with. The low-level REST client is compatible with all elasticsearch versions. -==== Maven configuration +[[java-rest-low-usage-maven-maven]] +===== Maven configuration Here is how you can configure the dependency using maven as a dependency manager. Add the following to your `pom.xml` file: @@ -26,7 +29,8 @@ Add the following to your `pom.xml` file: -------------------------------------------------- -==== Gradle configuration +[[java-rest-low-usage-maven-gradle]] +===== Gradle configuration Here is how you can configure the dependency using gradle as a dependency manager. Add the following to your `build.gradle` file: @@ -38,7 +42,8 @@ dependencies { } -------------------------------------------------- -=== Dependencies +[[java-rest-low-usage-dependencies]] +==== Dependencies The low-level Java REST client internally uses the http://hc.apache.org/httpcomponents-asyncclient-dev/[Apache Http Async Client] @@ -53,7 +58,8 @@ http://hc.apache.org/httpcomponents-asyncclient-dev/[Apache Http Async Client] - commons-logging:commons-logging -=== Initialization +[[java-rest-low-usage-initialization]] +==== Initialization A `RestClient` instance can be built through the corresponding `RestClientBuilder` class, created via `RestClient#builder(HttpHost...)` @@ -101,7 +107,8 @@ http://hc.apache.org/httpcomponents-asyncclient-dev/httpasyncclient/apidocs/org/ allows to set) -=== Performing requests +[[java-rest-low-usage-requests]] +==== Performing requests Once the `RestClient` has been created, requests can be sent by calling one of the available `performRequest` or `performRequestAsync` method variants. @@ -159,7 +166,8 @@ void performRequestAsync(String method, String endpoint, Header... headers); -------------------------------------------------- -==== Request Arguments +[[java-rest-low-usage-requests-arguments]] +===== Request Arguments The following are the arguments accepted by the different methods: @@ -179,7 +187,8 @@ http://hc.apache.org/httpcomponents-core-ga/httpcore-nio/apidocs/org/apache/http request success or failure `headers`:: optional request headers -=== Reading responses +[[java-rest-low-usage-responses]] +==== Reading responses The `Response` object, either returned by the synchronous `performRequest` methods or received as an argument in `ResponseListener#onSuccess(Response)`, wraps the @@ -216,7 +225,8 @@ case the response body will not contain an error but rather the usual get api response, just without the document as it was not found. -=== Example requests +[[java-rest-low-usage-example]] +==== Example requests Here are a couple of examples: @@ -293,7 +303,8 @@ latch.await(); -------------------------------------------------- -=== Logging +[[java-rest-low-usage-logging]] +==== Logging The Java REST client uses the same logging library that the Apache Async Http Client uses: https://commons.apache.org/proper/commons-logging/[Apache Commons Logging], diff --git a/docs/java-rest/overview.asciidoc b/docs/java-rest/overview.asciidoc index 206fcb931b4..3c5ea06c1dd 100644 --- a/docs/java-rest/overview.asciidoc +++ b/docs/java-rest/overview.asciidoc @@ -1,42 +1,11 @@ +[[java-rest-overview]] == Overview -Official low-level client for Elasticsearch. Allows to communicate with an -Elasticsearch cluster through http. Compatible with all elasticsearch versions. +The Java REST Client comes with 2 flavors: -=== Features - -The low-level client's features include: - -* minimal dependencies - -* load balancing across all available nodes - -* failover in case of node failures and upon specific response codes - -* failed connection penalization (whether a failed node is retried depends on - how many consecutive times it failed; the more failed attempts the longer the - client will wait before trying that same node again) - -* persistent connections - -* trace logging of requests and responses - -* optional automatic <> - - -=== License - -Copyright 2013-2016 Elasticsearch - -Licensed 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. +* <>: which is the official low-level client for Elasticsearch. +It allows to communicate with an Elasticsearch cluster through http and is compatible +with all elasticsearch versions. +* <>: which is the official high-level client for Elasticsearch. It adds support +part of the elasticsearch document level and search API on top of the low-level client. From c373ed102f7b78e85657943816d33f55f9725d13 Mon Sep 17 00:00:00 2001 From: David Pilato Date: Fri, 24 Feb 2017 17:38:18 +0100 Subject: [PATCH 002/105] Extract documentation from test code --- .../elasticsearch/client/DocumentationIT.java | 84 +++++++++++++++++++ .../high-level/document/delete.asciidoc | 41 +++------ 2 files changed, 95 insertions(+), 30 deletions(-) create mode 100644 client/rest-high-level/src/test/java/org/elasticsearch/client/DocumentationIT.java diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/DocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/DocumentationIT.java new file mode 100644 index 00000000000..c40a9b01515 --- /dev/null +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/DocumentationIT.java @@ -0,0 +1,84 @@ +/* + * 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.client; + +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.DocWriteResponse; +import org.elasticsearch.action.delete.DeleteRequest; +import org.elasticsearch.action.delete.DeleteResponse; +import org.elasticsearch.action.support.WriteRequest; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.index.VersionType; + +import java.io.IOException; + +/** + * This class is used to generate the Java API documentation + */ +public class DocumentationIT extends ESRestHighLevelClientTestCase { + + /** + * This test documents docs/java-rest/high-level/document/delete.asciidoc + */ + public void testDelete() throws IOException { + RestHighLevelClient client = highLevelClient(); + +// tag::delete-request[] +DeleteRequest request = new DeleteRequest( + "index", // <1> + "type", // <2> + "id"); // <3> +// end::delete-request[] + +// tag::delete-request-props[] +request.timeout(TimeValue.timeValueSeconds(1)); // <1> +request.timeout("1s"); // <2> +request.setRefreshPolicy(WriteRequest.RefreshPolicy.WAIT_UNTIL); // <3> +request.setRefreshPolicy("wait_for"); // <4> +request.version(2); // <5> +request.versionType(VersionType.EXTERNAL); // <6> +// end::delete-request-props[] + +// tag::delete-execute[] +DeleteResponse response = client.delete(request); +// end::delete-execute[] + +// tag::delete-notfound[] +if (response.getResult().equals(DocWriteResponse.Result.NOT_FOUND)) { + // <1> +} +// end::delete-notfound[] + +// tag::delete-execute-async[] +client.deleteAsync(request, new ActionListener() { + @Override + public void onResponse(DeleteResponse deleteResponse) { + // <1> + } + + @Override + public void onFailure(Exception e) { + // <2> + } +}); +// end::delete-execute-async[] + + } +} diff --git a/docs/java-rest/high-level/document/delete.asciidoc b/docs/java-rest/high-level/document/delete.asciidoc index 46f70b59e62..f34061a81de 100644 --- a/docs/java-rest/high-level/document/delete.asciidoc +++ b/docs/java-rest/high-level/document/delete.asciidoc @@ -6,12 +6,9 @@ The most simple Delete Request needs is: -[source,java] +["source","java",subs="attributes,callouts"] -------------------------------------------------- -DeleteRequest request = new DeleteRequest( - "index", <1> - "type", <2> - "id"); <3> +sys2::[perl -ne 'exit if /end::delete-request/; print if $tag; $tag = $tag || /tag::delete-request/' {docdir}/../../client/rest-high-level/src/test/java/org/elasticsearch/client/DocumentationIT.java] -------------------------------------------------- <1> Index name <2> Type @@ -19,14 +16,9 @@ DeleteRequest request = new DeleteRequest( You can also provide the following properties: -[source,java] +["source","java",subs="attributes,callouts"] -------------------------------------------------- -request.timeout(TimeValue.timeValueSeconds(1)); <1> -request.timeout("1s"); <2> -request.setRefreshPolicy(WriteRequest.RefreshPolicy.WAIT_UNTIL); <3> -request.setRefreshPolicy("wait_for"); <4> -request.version(2); <5> -request.versionType(VersionType.EXTERNAL); <6> +sys2::[perl -ne 'exit if /end::delete-request-props/; print if $tag; $tag = $tag || /tag::delete-request-props/' {docdir}/../../client/rest-high-level/src/test/java/org/elasticsearch/client/DocumentationIT.java] -------------------------------------------------- <1> Timeout <2> Timeout as String @@ -38,27 +30,17 @@ request.versionType(VersionType.EXTERNAL); <6> [[java-rest-high-document-delete-sync]] ==== Execution -[source,java] +["source","java",subs="attributes,callouts"] -------------------------------------------------- -DeleteResponse response = client.delete(request); +sys2::[perl -ne 'exit if /end::delete-execute/; print if $tag; $tag = $tag || /tag::delete-execute/' {docdir}/../../client/rest-high-level/src/test/java/org/elasticsearch/client/DocumentationIT.java] -------------------------------------------------- [[java-rest-high-document-delete-async]] ==== Asynchronous Execution -[source,java] +["source","java",subs="attributes,callouts"] -------------------------------------------------- -client.deleteAsync(request, new ActionListener() { - @Override - public void onResponse(DeleteResponse deleteResponse) { - <1> - } - - @Override - public void onFailure(Exception e) { - <2> - } -}); +sys2::[perl -ne 'exit if /end::delete-execute-async/; print if $tag; $tag = $tag || /tag::delete-execute-async/' {docdir}/../../client/rest-high-level/src/test/java/org/elasticsearch/client/DocumentationIT.java] -------------------------------------------------- <1> Implement if needed when execution did not throw an exception <2> Implement if needed in case of failure @@ -68,11 +50,9 @@ client.deleteAsync(request, new ActionListener() { In the Delete Response object, you can check for example the result of the operation: -[source,java] +["source","java",subs="attributes,callouts"] -------------------------------------------------- -if (response.getResult().equals(DocWriteResponse.Result.NOT_FOUND)) { - <1> -} +sys2::[perl -ne 'exit if /end::delete-notfound/; print if $tag; $tag = $tag || /tag::delete-notfound/' {docdir}/../../client/rest-high-level/src/test/java/org/elasticsearch/client/DocumentationIT.java] -------------------------------------------------- <1> Do something if we did not find the document which should have been deleted @@ -90,3 +70,4 @@ try { } -------------------------------------------------- <1> We got a version conflict + From 3215ac11ec1b004a3d98f277321252467b3d763a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Mon, 27 Feb 2017 11:43:56 +0100 Subject: [PATCH 003/105] Add info method to High Level Rest client (#23350) This commit adds support for an info() method to the High Level Rest client that returns the cluster information usually obtained by performing a `GET hostname:9200` request. --- .../org/elasticsearch/client/Request.java | 8 +++-- .../client/RestHighLevelClient.java | 9 ++++++ .../client/ESRestHighLevelClientTestCase.java | 2 +- .../elasticsearch/client/PingAndInfoIT.java | 22 +++++++++++++- .../elasticsearch/client/RequestTests.java | 8 +++++ .../client/RestHighLevelClientTests.java | 30 ++++++++++++++++--- 6 files changed, 71 insertions(+), 8 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java index b92150386a1..d1f3f06eb21 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java @@ -29,8 +29,8 @@ import org.apache.http.entity.ByteArrayEntity; import org.apache.http.entity.ContentType; import org.apache.lucene.util.BytesRef; import org.elasticsearch.action.DocWriteRequest; -import org.elasticsearch.action.delete.DeleteRequest; import org.elasticsearch.action.bulk.BulkRequest; +import org.elasticsearch.action.delete.DeleteRequest; import org.elasticsearch.action.get.GetRequest; import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.support.ActiveShardCount; @@ -97,6 +97,10 @@ final class Request { return new Request(HttpDelete.METHOD_NAME, endpoint, parameters.getParams(), null); } + static Request info() { + return new Request(HttpGet.METHOD_NAME, "/", Collections.emptyMap(), null); + } + static Request bulk(BulkRequest bulkRequest) throws IOException { Params parameters = Params.builder(); parameters.withTimeout(bulkRequest.timeout()); @@ -264,7 +268,7 @@ final class Request { } static Request ping() { - return new Request("HEAD", "/", Collections.emptyMap(), null); + return new Request(HttpHead.METHOD_NAME, "/", Collections.emptyMap(), null); } static Request update(UpdateRequest updateRequest) throws IOException { diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java index 70e19f2ddcb..4e30311c475 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java @@ -35,6 +35,7 @@ import org.elasticsearch.action.get.GetResponse; import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.index.IndexResponse; import org.elasticsearch.action.main.MainRequest; +import org.elasticsearch.action.main.MainResponse; import org.elasticsearch.action.update.UpdateRequest; import org.elasticsearch.action.update.UpdateResponse; import org.elasticsearch.common.CheckedFunction; @@ -113,6 +114,14 @@ public class RestHighLevelClient { emptySet(), headers); } + /** + * Get the cluster info otherwise provided when sending an HTTP request to port 9200 + */ + public MainResponse info(Header... headers) throws IOException { + return performRequestAndParseEntity(new MainRequest(), (request) -> Request.info(), MainResponse::fromXContent, emptySet(), + headers); + } + /** * Retrieves a document by id using the Get API * diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/ESRestHighLevelClientTestCase.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/ESRestHighLevelClientTestCase.java index 53a99a4104d..cdd83178309 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/ESRestHighLevelClientTestCase.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/ESRestHighLevelClientTestCase.java @@ -41,7 +41,7 @@ public abstract class ESRestHighLevelClientTestCase extends ESRestTestCase { } @AfterClass - public static void cleanupClient() throws IOException { + public static void cleanupClient() { restHighLevelClient = null; } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/PingAndInfoIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/PingAndInfoIT.java index 35483a944f6..b22ded52655 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/PingAndInfoIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/PingAndInfoIT.java @@ -19,7 +19,10 @@ package org.elasticsearch.client; +import org.elasticsearch.action.main.MainResponse; + import java.io.IOException; +import java.util.Map; public class PingAndInfoIT extends ESRestHighLevelClientTestCase { @@ -27,5 +30,22 @@ public class PingAndInfoIT extends ESRestHighLevelClientTestCase { assertTrue(highLevelClient().ping()); } - //TODO add here integ tests for info api: "GET /" once we have parsing code for MainResponse + @SuppressWarnings("unchecked") + public void testInfo() throws IOException { + MainResponse info = highLevelClient().info(); + // compare with what the low level client outputs + Map infoAsMap = entityAsMap(adminClient().performRequest("GET", "/")); + assertEquals(infoAsMap.get("cluster_name"), info.getClusterName().value()); + assertEquals(infoAsMap.get("cluster_uuid"), info.getClusterUuid()); + + // only check node name existence, might be a different one from what was hit by low level client in multi-node cluster + assertNotNull(info.getNodeName()); + Map versionMap = (Map) infoAsMap.get("version"); + assertEquals(versionMap.get("build_hash"), info.getBuild().shortHash()); + assertEquals(versionMap.get("build_date"), info.getBuild().date()); + assertEquals(versionMap.get("build_snapshot"), info.getBuild().isSnapshot()); + assertEquals(versionMap.get("number"), info.getVersion().toString()); + assertEquals(versionMap.get("lucene_version"), info.getVersion().luceneVersion.toString()); + } + } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java index 62bb6b551af..d39dfaa3e75 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java @@ -66,6 +66,14 @@ public class RequestTests extends ESTestCase { assertEquals("HEAD", request.method); } + public void testInfo() { + Request request = Request.info(); + assertEquals("/", request.endpoint); + assertEquals(0, request.params.size()); + assertNull(request.entity); + assertEquals("GET", request.method); + } + public void testGet() { getAndExistsTest(Request::get, "GET"); } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java index 897b1f55466..7104ee9e39d 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.client; import com.fasterxml.jackson.core.JsonParseException; + import org.apache.http.Header; import org.apache.http.HttpEntity; import org.apache.http.HttpHost; @@ -33,15 +34,20 @@ import org.apache.http.entity.StringEntity; import org.apache.http.message.BasicHttpResponse; import org.apache.http.message.BasicRequestLine; import org.apache.http.message.BasicStatusLine; +import org.elasticsearch.Build; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.main.MainRequest; +import org.elasticsearch.action.main.MainResponse; +import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.cbor.CborXContent; import org.elasticsearch.common.xcontent.smile.SmileXContent; import org.elasticsearch.rest.RestStatus; @@ -59,6 +65,7 @@ import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import static org.elasticsearch.common.xcontent.XContentHelper.toXContent; import static org.hamcrest.CoreMatchers.instanceOf; import static org.mockito.Matchers.anyMapOf; import static org.mockito.Matchers.anyObject; @@ -79,7 +86,7 @@ public class RestHighLevelClientTests extends ESTestCase { private RestHighLevelClient restHighLevelClient; @Before - public void initClient() throws IOException { + public void initClient() { restClient = mock(RestClient.class); restHighLevelClient = new RestHighLevelClient(restClient); } @@ -115,6 +122,21 @@ public class RestHighLevelClientTests extends ESTestCase { Matchers.isNull(HttpEntity.class), argThat(new HeadersVarargMatcher(headers))); } + public void testInfo() throws IOException { + Header[] headers = RestClientTestUtil.randomHeaders(random(), "Header"); + Response response = mock(Response.class); + MainResponse testInfo = new MainResponse("nodeName", Version.CURRENT, new ClusterName("clusterName"), "clusterUuid", + Build.CURRENT, true); + when(response.getEntity()).thenReturn( + new StringEntity(toXContent(testInfo, XContentType.JSON, false).utf8ToString(), ContentType.APPLICATION_JSON)); + when(restClient.performRequest(anyString(), anyString(), anyMapOf(String.class, String.class), + anyObject(), anyVararg())).thenReturn(response); + MainResponse receivedInfo = restHighLevelClient.info(headers); + assertEquals(testInfo, receivedInfo); + verify(restClient).performRequest(eq("GET"), eq("/"), eq(Collections.emptyMap()), + Matchers.isNull(HttpEntity.class), argThat(new HeadersVarargMatcher(headers))); + } + public void testRequestValidation() { ActionRequestValidationException validationException = new ActionRequestValidationException(); validationException.addValidationError("validation error"); @@ -388,7 +410,7 @@ public class RestHighLevelClientTests extends ESTestCase { assertEquals("Elasticsearch exception [type=exception, reason=test error message]", elasticsearchException.getMessage()); } - public void testWrapResponseListenerOnSuccess() throws IOException { + public void testWrapResponseListenerOnSuccess() { { TrackingActionListener trackingActionListener = new TrackingActionListener(); ResponseListener responseListener = restHighLevelClient.wrapResponseListener( @@ -414,7 +436,7 @@ public class RestHighLevelClientTests extends ESTestCase { } } - public void testWrapResponseListenerOnException() throws IOException { + public void testWrapResponseListenerOnException() { TrackingActionListener trackingActionListener = new TrackingActionListener(); ResponseListener responseListener = restHighLevelClient.wrapResponseListener( response -> response.getStatusLine().getStatusCode(), trackingActionListener, Collections.emptySet()); @@ -543,7 +565,7 @@ public class RestHighLevelClientTests extends ESTestCase { assertEquals("Elasticsearch exception [type=exception, reason=test error message]", elasticsearchException.getMessage()); } - public void testNamedXContents() throws IOException { + public void testNamedXContents() { List namedXContents = RestHighLevelClient.getNamedXContents(); assertEquals(0, namedXContents.size()); } From b8e2d12b2319db5d989130e41abc51ac6532b607 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Mon, 27 Feb 2017 11:44:41 +0100 Subject: [PATCH 004/105] Factor out filling of TopDocs in SearchPhaseController (#23380) Previously this code was duplicated across the 3 different topdocs variants we have. It also had no real unittest (where we tested with holes in the results) which caused a sneaky bug where the comparison used `result.size()` vs `results.size()` causing several NPEs downstream. This change adds a static method to fill the top docs that is shared across all variants and adds a unittest that would have caught the issue very quickly. Closes #19356 Closes #23357 --- .../action/search/SearchPhaseController.java | 51 +++++++------------ .../search/SearchPhaseControllerTests.java | 28 ++++++++++ 2 files changed, 47 insertions(+), 32 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java b/core/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java index 52fbf952fe4..068a005204d 100644 --- a/core/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java +++ b/core/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java @@ -66,6 +66,7 @@ import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.StreamSupport; @@ -233,47 +234,19 @@ public class SearchPhaseController extends AbstractComponent { if (result.queryResult().topDocs() instanceof CollapseTopFieldDocs) { CollapseTopFieldDocs firstTopDocs = (CollapseTopFieldDocs) result.queryResult().topDocs(); final Sort sort = new Sort(firstTopDocs.fields); - final CollapseTopFieldDocs[] shardTopDocs = new CollapseTopFieldDocs[numShards]; - if (result.size() != shardTopDocs.length) { - // TopDocs#merge can't deal with null shard TopDocs - final CollapseTopFieldDocs empty = new CollapseTopFieldDocs(firstTopDocs.field, 0, new FieldDoc[0], - sort.getSort(), new Object[0], Float.NaN); - Arrays.fill(shardTopDocs, empty); - } - for (AtomicArray.Entry sortedResult : results) { - TopDocs topDocs = sortedResult.value.queryResult().topDocs(); - // the 'index' field is the position in the resultsArr atomic array - shardTopDocs[sortedResult.index] = (CollapseTopFieldDocs) topDocs; - } + fillTopDocs(shardTopDocs, results, new CollapseTopFieldDocs(firstTopDocs.field, 0, new FieldDoc[0], + sort.getSort(), new Object[0], Float.NaN)); mergedTopDocs = CollapseTopFieldDocs.merge(sort, from, topN, shardTopDocs); } else if (result.queryResult().topDocs() instanceof TopFieldDocs) { TopFieldDocs firstTopDocs = (TopFieldDocs) result.queryResult().topDocs(); final Sort sort = new Sort(firstTopDocs.fields); - final TopFieldDocs[] shardTopDocs = new TopFieldDocs[resultsArr.length()]; - if (result.size() != shardTopDocs.length) { - // TopDocs#merge can't deal with null shard TopDocs - final TopFieldDocs empty = new TopFieldDocs(0, new FieldDoc[0], sort.getSort(), Float.NaN); - Arrays.fill(shardTopDocs, empty); - } - for (AtomicArray.Entry sortedResult : results) { - TopDocs topDocs = sortedResult.value.queryResult().topDocs(); - // the 'index' field is the position in the resultsArr atomic array - shardTopDocs[sortedResult.index] = (TopFieldDocs) topDocs; - } + fillTopDocs(shardTopDocs, results, new TopFieldDocs(0, new FieldDoc[0], sort.getSort(), Float.NaN)); mergedTopDocs = TopDocs.merge(sort, from, topN, shardTopDocs); } else { final TopDocs[] shardTopDocs = new TopDocs[resultsArr.length()]; - if (result.size() != shardTopDocs.length) { - // TopDocs#merge can't deal with null shard TopDocs - Arrays.fill(shardTopDocs, Lucene.EMPTY_TOP_DOCS); - } - for (AtomicArray.Entry sortedResult : results) { - TopDocs topDocs = sortedResult.value.queryResult().topDocs(); - // the 'index' field is the position in the resultsArr atomic array - shardTopDocs[sortedResult.index] = topDocs; - } + fillTopDocs(shardTopDocs, results, Lucene.EMPTY_TOP_DOCS); mergedTopDocs = TopDocs.merge(from, topN, shardTopDocs); } @@ -314,6 +287,20 @@ public class SearchPhaseController extends AbstractComponent { return scoreDocs; } + static void fillTopDocs(T[] shardTopDocs, + List> results, + T empytTopDocs) { + if (results.size() != shardTopDocs.length) { + // TopDocs#merge can't deal with null shard TopDocs + Arrays.fill(shardTopDocs, empytTopDocs); + } + for (AtomicArray.Entry resultProvider : results) { + final T topDocs = (T) resultProvider.value.queryResult().topDocs(); + assert topDocs != null : "top docs must not be null in a valid result"; + // the 'index' field is the position in the resultsArr atomic array + shardTopDocs[resultProvider.index] = topDocs; + } + } public ScoreDoc[] getLastEmittedDocPerShard(ReducedQueryPhase reducedQueryPhase, ScoreDoc[] sortedScoreDocs, int numShards) { ScoreDoc[] lastEmittedDocPerShard = new ScoreDoc[numShards]; diff --git a/core/src/test/java/org/elasticsearch/action/search/SearchPhaseControllerTests.java b/core/src/test/java/org/elasticsearch/action/search/SearchPhaseControllerTests.java index ee68f2f1f98..36756aba946 100644 --- a/core/src/test/java/org/elasticsearch/action/search/SearchPhaseControllerTests.java +++ b/core/src/test/java/org/elasticsearch/action/search/SearchPhaseControllerTests.java @@ -21,6 +21,7 @@ package org.elasticsearch.action.search; import org.apache.lucene.search.ScoreDoc; import org.apache.lucene.search.TopDocs; +import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.text.Text; import org.elasticsearch.common.util.BigArrays; @@ -347,4 +348,31 @@ public class SearchPhaseControllerTests extends ESTestCase { } } } + + public void testFillTopDocs() { + final int maxIters = randomIntBetween(5, 15); + for (int iters = 0; iters < maxIters; iters++) { + TopDocs[] topDocs = new TopDocs[randomIntBetween(2, 100)]; + int numShards = topDocs.length; + AtomicArray resultProviderAtomicArray = generateQueryResults(numShards, Collections.emptyList(), + 2, randomBoolean()); + if (randomBoolean()) { + int maxNull = randomIntBetween(1, topDocs.length - 1); + for (int i = 0; i < maxNull; i++) { + resultProviderAtomicArray.set(randomIntBetween(0, resultProviderAtomicArray.length() - 1), null); + } + } + SearchPhaseController.fillTopDocs(topDocs, resultProviderAtomicArray.asList(), Lucene.EMPTY_TOP_DOCS); + for (int i = 0; i < topDocs.length; i++) { + assertNotNull(topDocs[i]); + if (topDocs[i] == Lucene.EMPTY_TOP_DOCS) { + assertNull(resultProviderAtomicArray.get(i)); + } else { + assertNotNull(resultProviderAtomicArray.get(i)); + assertNotNull(resultProviderAtomicArray.get(i).queryResult()); + assertSame(resultProviderAtomicArray.get(i).queryResult().topDocs(), topDocs[i]); + } + } + } + } } From 04aaedc083777fa298d786be0aeaed129eb32da3 Mon Sep 17 00:00:00 2001 From: javanna Date: Sat, 11 Feb 2017 08:28:56 +0100 Subject: [PATCH 005/105] [TEST] Remove content type auto-detection while parsing request body in REST tests --- .../elasticsearch/test/rest/yaml/section/DoSection.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java index 17292380404..482525c46b5 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java @@ -24,10 +24,9 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.xcontent.NamedXContentRegistry; -import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentLocation; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.rest.yaml.ClientYamlTestExecutionContext; import org.elasticsearch.test.rest.yaml.ClientYamlTestResponse; import org.elasticsearch.test.rest.yaml.ClientYamlTestResponseException; @@ -118,9 +117,7 @@ public class DoSection implements ExecutableSection { } else if (token.isValue()) { if ("body".equals(paramName)) { String body = parser.text(); - XContentType bodyContentType = XContentFactory.xContentType(body); - XContentParser bodyParser = XContentFactory.xContent(bodyContentType).createParser( - NamedXContentRegistry.EMPTY, body); + XContentParser bodyParser = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY, body); //multiple bodies are supported e.g. in case of bulk provided as a whole string while(bodyParser.nextToken() != null) { apiCallSection.addBody(bodyParser.mapOrdered()); From ca858befab23e43c37eea62609be1c0f9e63fdaf Mon Sep 17 00:00:00 2001 From: javanna Date: Sat, 11 Feb 2017 08:33:19 +0100 Subject: [PATCH 006/105] [TEST] create HttpEntity earlier in REST tests This allows to set content-type together with the body itself. At the moment it is always json, but this change allows makes it easier to randomize it later --- .../test/rest/yaml/ClientYamlTestClient.java | 38 +++++++++++-------- .../yaml/ClientYamlTestExecutionContext.java | 24 ++++++------ 2 files changed, 36 insertions(+), 26 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java index 0324fedd57a..68ae6fada49 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java @@ -22,15 +22,15 @@ import com.carrotsearch.randomizedtesting.RandomizedTest; import org.apache.http.Header; import org.apache.http.HttpEntity; import org.apache.http.HttpHost; +import org.apache.http.client.methods.HttpGet; import org.apache.http.entity.ContentType; -import org.apache.http.entity.StringEntity; import org.apache.http.message.BasicHeader; +import org.apache.http.util.EntityUtils; import org.apache.logging.log4j.Logger; import org.elasticsearch.Version; import org.elasticsearch.client.Response; import org.elasticsearch.client.ResponseException; import org.elasticsearch.client.RestClient; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi; import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestPath; @@ -52,6 +52,8 @@ import java.util.Objects; public class ClientYamlTestClient { private static final Logger logger = Loggers.getLogger(ClientYamlTestClient.class); + private static final ContentType YAML_CONTENT_TYPE = ContentType.create("application/yaml"); + private final ClientYamlSuiteRestSpec restSpec; private final RestClient restClient; private final Version esVersion; @@ -71,7 +73,7 @@ public class ClientYamlTestClient { /** * Calls an api with the provided parameters and body */ - public ClientYamlTestResponse callApi(String apiName, Map params, String body, Map headers) + public ClientYamlTestResponse callApi(String apiName, Map params, HttpEntity entity, Map headers) throws IOException { if ("raw".equals(apiName)) { @@ -79,10 +81,6 @@ public class ClientYamlTestClient { Map queryStringParams = new HashMap<>(params); String method = Objects.requireNonNull(queryStringParams.remove("method"), "Method must be set to use raw request"); String path = "/"+ Objects.requireNonNull(queryStringParams.remove("path"), "Path must be set to use raw request"); - HttpEntity entity = null; - if (body != null && body.length() > 0) { - entity = new StringEntity(body, ContentType.APPLICATION_JSON); - } // And everything else is a url parameter! try { Response response = restClient.performRequest(method, path, queryStringParams, entity); @@ -113,20 +111,20 @@ public class ClientYamlTestClient { List supportedMethods = restApi.getSupportedMethods(pathParts.keySet()); String requestMethod; - StringEntity requestBody = null; - if (Strings.hasLength(body)) { + if (entity != null) { if (!restApi.isBodySupported()) { throw new IllegalArgumentException("body is not supported by [" + restApi.getName() + "] api"); } + String contentType = entity.getContentType().getValue(); //randomly test the GET with source param instead of GET/POST with body - if (supportedMethods.contains("GET") && RandomizedTest.rarely()) { + if (sendBodyAsSourceParam(supportedMethods, contentType)) { logger.debug("sending the request body as source param with GET method"); - queryStringParams.put("source", body); - queryStringParams.put("source_content_type", ContentType.APPLICATION_JSON.toString()); - requestMethod = "GET"; + queryStringParams.put("source", EntityUtils.toString(entity)); + queryStringParams.put("source_content_type", contentType); + requestMethod = HttpGet.METHOD_NAME; + entity = null; } else { requestMethod = RandomizedTest.randomFrom(supportedMethods); - requestBody = new StringEntity(body, ContentType.APPLICATION_JSON); } } else { if (restApi.isBodyRequired()) { @@ -168,13 +166,23 @@ public class ClientYamlTestClient { logger.debug("calling api [{}]", apiName); try { - Response response = restClient.performRequest(requestMethod, requestPath, queryStringParams, requestBody, requestHeaders); + Response response = restClient.performRequest(requestMethod, requestPath, queryStringParams, entity, requestHeaders); return new ClientYamlTestResponse(response); } catch(ResponseException e) { throw new ClientYamlTestResponseException(e); } } + private static boolean sendBodyAsSourceParam(List supportedMethods, String contentType) { + if (supportedMethods.contains(HttpGet.METHOD_NAME)) { + if (contentType.startsWith(ContentType.APPLICATION_JSON.getMimeType()) || + contentType.startsWith(YAML_CONTENT_TYPE.getMimeType())) { + return RandomizedTest.rarely(); + } + } + return false; + } + private ClientYamlSuiteRestApi restApi(String apiName) { ClientYamlSuiteRestApi restApi = restSpec.getApi(apiName); if (restApi == null) { diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java index 27653a13f29..fe397f512c5 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java @@ -18,6 +18,9 @@ */ package org.elasticsearch.test.rest.yaml; +import org.apache.http.HttpEntity; +import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; import org.apache.logging.log4j.Logger; import org.elasticsearch.Version; import org.elasticsearch.common.logging.Loggers; @@ -62,9 +65,9 @@ public class ClientYamlTestExecutionContext { } } - String body = actualBody(bodies); + HttpEntity entity = createEntity(bodies); try { - response = callApiInternal(apiName, requestParams, body, headers); + response = callApiInternal(apiName, requestParams, entity, headers); return response; } catch(ClientYamlTestResponseException e) { response = e.getRestTestResponse(); @@ -77,29 +80,28 @@ public class ClientYamlTestExecutionContext { } } - private String actualBody(List> bodies) throws IOException { + private HttpEntity createEntity(List> bodies) throws IOException { if (bodies.isEmpty()) { - return ""; + return null; } - if (bodies.size() == 1) { - return bodyAsString(stash.replaceStashedValues(bodies.get(0))); + String bodyAsString = bodyAsString(stash.replaceStashedValues(bodies.get(0))); + return new StringEntity(bodyAsString, ContentType.APPLICATION_JSON); } - StringBuilder bodyBuilder = new StringBuilder(); for (Map body : bodies) { bodyBuilder.append(bodyAsString(stash.replaceStashedValues(body))).append("\n"); } - return bodyBuilder.toString(); + return new StringEntity(bodyBuilder.toString(), ContentType.APPLICATION_JSON); } private String bodyAsString(Map body) throws IOException { return XContentFactory.jsonBuilder().map(body).string(); } - private ClientYamlTestResponse callApiInternal(String apiName, Map params, String body, Map headers) - throws IOException { - return clientYamlTestClient.callApi(apiName, params, body, headers); + private ClientYamlTestResponse callApiInternal(String apiName, Map params, + HttpEntity entity, Map headers) throws IOException { + return clientYamlTestClient.callApi(apiName, params, entity, headers); } /** From 261f31f5b70b078bef1182ad3d933bf8cea8b1fa Mon Sep 17 00:00:00 2001 From: javanna Date: Fri, 17 Feb 2017 22:17:30 +0100 Subject: [PATCH 007/105] [TEST] move filters aggs wrapper query builder rewriting test to integ tests This test makes little sense when sent from the REST layer, as WrapperQueryBuilder is supposed to be used from the Java api. Also, providing the inner query as base64 string will work only for string formats and break for binary formats like SMILE and CBOR, whcih doesn't play well with randomizing content type in our REST tests --- .../aggregations/FiltersAggsRewriteIT.java | 67 +++++++++++++++++++ .../test/search.aggregation/60_filters.yaml | 52 -------------- 2 files changed, 67 insertions(+), 52 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/search/aggregations/FiltersAggsRewriteIT.java delete mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/60_filters.yaml diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/FiltersAggsRewriteIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/FiltersAggsRewriteIT.java new file mode 100644 index 00000000000..a3836b2fce0 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/search/aggregations/FiltersAggsRewriteIT.java @@ -0,0 +1,67 @@ +/* + * 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.search.aggregations; + +import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.index.query.WrapperQueryBuilder; +import org.elasticsearch.search.aggregations.bucket.filters.FiltersAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.filters.FiltersAggregator; +import org.elasticsearch.search.aggregations.bucket.filters.InternalFilters; +import org.elasticsearch.test.ESSingleNodeTestCase; + +import java.io.IOException; + +public class FiltersAggsRewriteIT extends ESSingleNodeTestCase { + + public void testWrapperQueryIsRewritten() throws IOException { + createIndex("test", Settings.EMPTY, "test", "title", "type=text"); + client().prepareIndex("test", "test", "1").setSource("title", "foo bar baz").get(); + client().prepareIndex("test", "test", "2").setSource("title", "foo foo foo").get(); + client().prepareIndex("test", "test", "3").setSource("title", "bar baz bax").get(); + client().admin().indices().prepareRefresh("test").get(); + + XContentType xContentType = randomFrom(XContentType.values()); + BytesReference bytesReference; + try (XContentBuilder builder = XContentFactory.contentBuilder(xContentType)) { + builder.startObject(); + { + builder.startObject("terms"); + { + builder.array("title", "foo"); + } + builder.endObject(); + } + builder.endObject(); + bytesReference = builder.bytes(); + } + FiltersAggregationBuilder builder = new FiltersAggregationBuilder("titles", new FiltersAggregator.KeyedFilter("titleterms", + new WrapperQueryBuilder(bytesReference))); + SearchResponse searchResponse = client().prepareSearch("test").setSize(0).addAggregation(builder).get(); + assertEquals(3, searchResponse.getHits().getTotalHits()); + InternalFilters filters = searchResponse.getAggregations().get("titles"); + assertEquals(1, filters.getBuckets().size()); + assertEquals(2, filters.getBuckets().get(0).getDocCount()); + } +} diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/60_filters.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/60_filters.yaml deleted file mode 100644 index 57b320970b6..00000000000 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/60_filters.yaml +++ /dev/null @@ -1,52 +0,0 @@ -setup: - - do: - indices.create: - index: test - body: - settings: - number_of_shards: 1 - number_of_replicas: 0 - mappings: - post: - properties: - title: - type: text - - - do: - index: - index: test - type: test - id: 1 - body: { "title" : "foo bar baz" } - - - do: - index: - index: test - type: test - id: 2 - body: { "title" : "foo foo foo" } - - - do: - index: - index: test - type: test - id: 3 - body: { "title" : "bar baz bax" } - - - do: - indices.refresh: {} - ---- -"Filters aggs with wrapper query": - - skip: - version: " - 5.1.1" - reason: Using filters aggs that needs rewriting, this was fixed in 5.1.2 - - - do: - search: - body: {"size": 0, "aggs": {"titles": {"filters": {"filters" : {"titleterms" : {"wrapper" : {"query": "eyJ0ZXJtcyIgOiB7ICJ0aXRsZSI6IFsiZm9vIl0gfX0="}}}}}}} - - # validate result - - match: { hits.total: 3 } - - length: { aggregations.titles.buckets: 1 } - - match: { aggregations.titles.buckets.titleterms.doc_count: 2 } From dad025a6ad1e9f66899211a1cb5a615aac7078fd Mon Sep 17 00:00:00 2001 From: javanna Date: Mon, 20 Feb 2017 12:17:30 +0100 Subject: [PATCH 008/105] [TEST] move test for binary field to specific test file that sets Content-Type header explicitly --- .../test/painless/50_script_doc_values.yaml | 24 ---------- .../painless/60_script_doc_values_binary.yaml | 47 +++++++++++++++++++ 2 files changed, 47 insertions(+), 24 deletions(-) create mode 100644 modules/lang-painless/src/test/resources/rest-api-spec/test/painless/60_script_doc_values_binary.yaml diff --git a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/50_script_doc_values.yaml b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/50_script_doc_values.yaml index cff58c01e12..3e80e5026dc 100644 --- a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/50_script_doc_values.yaml +++ b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/50_script_doc_values.yaml @@ -6,9 +6,6 @@ setup: mappings: test: properties: - binary: - type: binary - doc_values: true boolean: type: boolean date: @@ -46,7 +43,6 @@ setup: type: test id: 1 body: - binary: U29tZSBiaW5hcnkgYmxvYg== boolean: true date: 2017-01-01T12:11:12 geo_point: 41.12,-71.34 @@ -65,26 +61,6 @@ setup: - do: indices.refresh: {} ---- -"binary": - - do: - search: - body: - script_fields: - field: - script: - inline: "doc['binary'].get(0).utf8ToString()" - - match: { hits.hits.0.fields.field.0: "Some binary blob" } - - - do: - search: - body: - script_fields: - field: - script: - inline: "doc['binary'].value.utf8ToString()" - - match: { hits.hits.0.fields.field.0: "Some binary blob" } - --- "boolean": - do: diff --git a/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/60_script_doc_values_binary.yaml b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/60_script_doc_values_binary.yaml new file mode 100644 index 00000000000..9a118830808 --- /dev/null +++ b/modules/lang-painless/src/test/resources/rest-api-spec/test/painless/60_script_doc_values_binary.yaml @@ -0,0 +1,47 @@ +--- +"binary": + - skip: + features: ["headers"] + - do: + indices.create: + index: test + body: + mappings: + test: + properties: + binary: + type: binary + doc_values: true + + - do: + #set the header so we won't randomize it + headers: + Content-Type: application/json + index: + index: test + type: test + id: 1 + body: + binary: U29tZSBiaW5hcnkgYmxvYg== + + - do: + indices.refresh: {} + + - do: + search: + body: + script_fields: + field: + script: + inline: "doc['binary'].get(0).utf8ToString()" + - match: { hits.hits.0.fields.field.0: "Some binary blob" } + + - do: + search: + body: + script_fields: + field: + script: + inline: "doc['binary'].value.utf8ToString()" + - match: { hits.hits.0.fields.field.0: "Some binary blob" } + From 2f6a6090b8d0e023f0af2ee72022354df1ba621d Mon Sep 17 00:00:00 2001 From: javanna Date: Mon, 20 Feb 2017 14:19:18 +0100 Subject: [PATCH 009/105] [TEST] don't check exact size in mapper-size yaml test Rather test that the size is present and greather than zero. The actual size depends on the content-type, which is randomized. --- .../test/resources/rest-api-spec/test/mapper_size/10_basic.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/mapper-size/src/test/resources/rest-api-spec/test/mapper_size/10_basic.yaml b/plugins/mapper-size/src/test/resources/rest-api-spec/test/mapper_size/10_basic.yaml index f9a41e52150..be08c46034a 100644 --- a/plugins/mapper-size/src/test/resources/rest-api-spec/test/mapper_size/10_basic.yaml +++ b/plugins/mapper-size/src/test/resources/rest-api-spec/test/mapper_size/10_basic.yaml @@ -28,5 +28,5 @@ id: 1 stored_fields: "_size" - - match: { _size: 13 } + - gt: { _size: 0 } From 9a2dba3036dd884fec127063785992b489092c51 Mon Sep 17 00:00:00 2001 From: javanna Date: Fri, 17 Feb 2017 22:38:10 +0100 Subject: [PATCH 010/105] [TEST] add support for binary responses to REST tests infra --- .../rest/Netty4BadRequestIT.java | 9 +-- .../elasticsearch/backwards/IndexingIT.java | 20 ++----- .../rest/yaml/ClientYamlTestResponse.java | 56 +++++++++++++------ .../test/rest/yaml/ObjectPath.java | 14 ++++- .../test/rest/yaml/ObjectPathTests.java | 22 ++++---- 5 files changed, 69 insertions(+), 52 deletions(-) diff --git a/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4BadRequestIT.java b/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4BadRequestIT.java index a5ba8fe18ce..ae2449d2820 100644 --- a/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4BadRequestIT.java +++ b/modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4BadRequestIT.java @@ -19,20 +19,17 @@ package org.elasticsearch.rest; -import org.apache.http.util.EntityUtils; import org.elasticsearch.client.Response; import org.elasticsearch.client.ResponseException; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeValue; -import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.http.HttpTransportSettings; import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.test.rest.yaml.ObjectPath; import java.io.IOException; import java.nio.charset.Charset; -import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.Map; @@ -46,10 +43,7 @@ public class Netty4BadRequestIT extends ESRestTestCase { public void testBadRequest() throws IOException { final Response response = client().performRequest("GET", "/_nodes/settings", Collections.emptyMap()); - final String body = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8); - final String contentType = response.getHeader("Content-Type"); - final XContentType xContentType = XContentType.fromMediaTypeOrFormat(contentType); - final ObjectPath objectPath = ObjectPath.createFromXContent(xContentType.xContent(), body); + final ObjectPath objectPath = ObjectPath.createFromResponse(response); final Map map = objectPath.evaluate("nodes"); int maxMaxInitialLineLength = Integer.MIN_VALUE; final Setting httpMaxInitialLineLength = HttpTransportSettings.SETTING_HTTP_MAX_INITIAL_LINE_LENGTH; @@ -80,5 +74,4 @@ public class Netty4BadRequestIT extends ESRestTestCase { assertThat(e, hasToString(containsString("too_long_frame_exception"))); assertThat(e, hasToString(matches("An HTTP line is larger than \\d+ bytes"))); } - } diff --git a/qa/backwards-5.0/src/test/java/org/elasticsearch/backwards/IndexingIT.java b/qa/backwards-5.0/src/test/java/org/elasticsearch/backwards/IndexingIT.java index 5e2655ff64c..f0be7753067 100644 --- a/qa/backwards-5.0/src/test/java/org/elasticsearch/backwards/IndexingIT.java +++ b/qa/backwards-5.0/src/test/java/org/elasticsearch/backwards/IndexingIT.java @@ -21,14 +21,12 @@ package org.elasticsearch.backwards; import org.apache.http.HttpHost; import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; -import org.apache.http.util.EntityUtils; import org.elasticsearch.Version; import org.elasticsearch.client.Response; import org.elasticsearch.client.RestClient; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.seqno.SeqNoStats; import org.elasticsearch.index.seqno.SequenceNumbersService; @@ -36,7 +34,6 @@ import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.test.rest.yaml.ObjectPath; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -51,13 +48,6 @@ import static org.hamcrest.Matchers.equalTo; public class IndexingIT extends ESRestTestCase { - private ObjectPath objectPath(Response response) throws IOException { - String body = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8); - String contentType = response.getHeader("Content-Type"); - XContentType xContentType = XContentType.fromMediaTypeOrFormat(contentType); - return ObjectPath.createFromXContent(xContentType.xContent(), body); - } - private void assertOK(Response response) { assertThat(response.getStatusLine().getStatusCode(), anyOf(equalTo(200), equalTo(201))); } @@ -272,7 +262,7 @@ public class IndexingIT extends ESRestTestCase { private void assertCount(final String index, final String preference, final int expectedCount) throws IOException { final Response response = client().performRequest("GET", index + "/_count", Collections.singletonMap("preference", preference)); assertOK(response); - final int actualCount = Integer.parseInt(objectPath(response).evaluate("count").toString()); + final int actualCount = Integer.parseInt(ObjectPath.createFromResponse(response).evaluate("count").toString()); assertThat(actualCount, equalTo(expectedCount)); } @@ -280,7 +270,7 @@ public class IndexingIT extends ESRestTestCase { final Response response = client().performRequest("GET", index + "/test/" + docId, Collections.singletonMap("preference", preference)); assertOK(response); - final int actualVersion = Integer.parseInt(objectPath(response).evaluate("_version").toString()); + final int actualVersion = Integer.parseInt(ObjectPath.createFromResponse(response).evaluate("_version").toString()); assertThat("version mismatch for doc [" + docId + "] preference [" + preference + "]", actualVersion, equalTo(expectedVersion)); } @@ -323,7 +313,7 @@ public class IndexingIT extends ESRestTestCase { private List buildShards(Nodes nodes, RestClient client) throws IOException { Response response = client.performRequest("GET", "test/_stats", singletonMap("level", "shards")); - List shardStats = objectPath(response).evaluate("indices.test.shards.0"); + List shardStats = ObjectPath.createFromResponse(response).evaluate("indices.test.shards.0"); ArrayList shards = new ArrayList<>(); for (Object shard : shardStats) { final String nodeId = ObjectPath.evaluate(shard, "routing.node"); @@ -345,7 +335,7 @@ public class IndexingIT extends ESRestTestCase { private Nodes buildNodeAndVersions() throws IOException { Response response = client().performRequest("GET", "_nodes"); - ObjectPath objectPath = objectPath(response); + ObjectPath objectPath = ObjectPath.createFromResponse(response); Map nodesAsMap = objectPath.evaluate("nodes"); Nodes nodes = new Nodes(); for (String id : nodesAsMap.keySet()) { @@ -356,7 +346,7 @@ public class IndexingIT extends ESRestTestCase { HttpHost.create(objectPath.evaluate("nodes." + id + ".http.publish_address")))); } response = client().performRequest("GET", "_cluster/state"); - nodes.setMasterNodeId(objectPath(response).evaluate("master_node")); + nodes.setMasterNodeId(ObjectPath.createFromResponse(response).evaluate("master_node")); return nodes; } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestResponse.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestResponse.java index 481ae752d05..3f4d3c9dd9d 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestResponse.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestResponse.java @@ -22,9 +22,15 @@ import org.apache.http.Header; import org.apache.http.client.methods.HttpHead; import org.apache.http.util.EntityUtils; import org.elasticsearch.client.Response; +import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; @@ -36,32 +42,30 @@ import java.util.List; public class ClientYamlTestResponse { private final Response response; - private final String body; + private final byte[] body; + private final XContentType bodyContentType; private ObjectPath parsedResponse; + private String bodyAsString; ClientYamlTestResponse(Response response) throws IOException { this.response = response; if (response.getEntity() != null) { + String contentType = response.getHeader("Content-Type"); + this.bodyContentType = XContentType.fromMediaTypeOrFormat(contentType); try { - this.body = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8); + byte[] bytes = EntityUtils.toByteArray(response.getEntity()); + //skip parsing if we got text back (e.g. if we called _cat apis) + if (bodyContentType != null) { + this.parsedResponse = ObjectPath.createFromXContent(bodyContentType.xContent(), new BytesArray(bytes)); + } + this.body = bytes; } catch (IOException e) { EntityUtils.consumeQuietly(response.getEntity()); - throw new RuntimeException(e); + throw e; } } else { this.body = null; - } - parseResponseBody(); - } - - private void parseResponseBody() throws IOException { - if (body != null) { - String contentType = response.getHeader("Content-Type"); - XContentType xContentType = XContentType.fromMediaTypeOrFormat(contentType); - //skip parsing if we got text back (e.g. if we called _cat apis) - if (xContentType == XContentType.JSON || xContentType == XContentType.YAML) { - this.parsedResponse = ObjectPath.createFromXContent(xContentType.xContent(), body); - } + this.bodyContentType = null; } } @@ -94,14 +98,32 @@ public class ClientYamlTestResponse { if (parsedResponse != null) { return parsedResponse.evaluate(""); } - return body; + //we only get here if there is no response body or the body is text + assert bodyContentType == null; + return getBodyAsString(); } /** * Returns the body as a string */ public String getBodyAsString() { - return body; + if (bodyAsString == null && body != null) { + //content-type null means that text was returned + if (bodyContentType == null || bodyContentType == XContentType.JSON || bodyContentType == XContentType.YAML) { + bodyAsString = new String(body, StandardCharsets.UTF_8); + } else { + //if the body is in a binary format and gets requested as a string (e.g. to log a test failure), we convert it to json + try (XContentBuilder jsonBuilder = XContentFactory.jsonBuilder()) { + try (XContentParser parser = bodyContentType.xContent().createParser(NamedXContentRegistry.EMPTY, body)) { + jsonBuilder.copyCurrentStructure(parser); + } + bodyAsString = jsonBuilder.string(); + } catch (IOException e) { + throw new UncheckedIOException("unable to convert response body to a string format", e); + } + } + } + return bodyAsString; } public boolean isError() { diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ObjectPath.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ObjectPath.java index e2a41365032..3deb2efd92e 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ObjectPath.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ObjectPath.java @@ -18,9 +18,14 @@ */ package org.elasticsearch.test.rest.yaml; +import org.apache.http.util.EntityUtils; +import org.elasticsearch.client.Response; +import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; import java.util.ArrayList; @@ -34,7 +39,14 @@ public class ObjectPath { private final Object object; - public static ObjectPath createFromXContent(XContent xContent, String input) throws IOException { + public static ObjectPath createFromResponse(Response response) throws IOException { + byte[] bytes = EntityUtils.toByteArray(response.getEntity()); + String contentType = response.getHeader("Content-Type"); + XContentType xContentType = XContentType.fromMediaTypeOrFormat(contentType); + return ObjectPath.createFromXContent(xContentType.xContent(), new BytesArray(bytes)); + } + + public static ObjectPath createFromXContent(XContent xContent, BytesReference input) throws IOException { try (XContentParser parser = xContent.createParser(NamedXContentRegistry.EMPTY, input)) { if (parser.nextToken() == XContentParser.Token.START_ARRAY) { return new ObjectPath(parser.listOrderedMap()); diff --git a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/ObjectPathTests.java b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/ObjectPathTests.java index 1f97a6ed13b..c377b500ccc 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/ObjectPathTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/ObjectPathTests.java @@ -39,8 +39,7 @@ import static org.hamcrest.Matchers.nullValue; public class ObjectPathTests extends ESTestCase { private static XContentBuilder randomXContentBuilder() throws IOException { - //only string based formats are supported, no cbor nor smile - XContentType xContentType = randomFrom(XContentType.JSON, XContentType.YAML); + XContentType xContentType = randomFrom(XContentType.values()); return XContentBuilder.builder(XContentFactory.xContent(xContentType)); } @@ -51,7 +50,7 @@ public class ObjectPathTests extends ESTestCase { xContentBuilder.field("field2.field3", "value2"); xContentBuilder.endObject(); xContentBuilder.endObject(); - ObjectPath objectPath = ObjectPath.createFromXContent(xContentBuilder.contentType().xContent(), xContentBuilder.string()); + ObjectPath objectPath = ObjectPath.createFromXContent(xContentBuilder.contentType().xContent(), xContentBuilder.bytes()); Object object = objectPath.evaluate("field1.field2\\.field3"); assertThat(object, instanceOf(String.class)); assertThat(object, equalTo("value2")); @@ -64,7 +63,7 @@ public class ObjectPathTests extends ESTestCase { xContentBuilder.field("field2", "value2"); xContentBuilder.endObject(); xContentBuilder.endObject(); - ObjectPath objectPath = ObjectPath.createFromXContent(xContentBuilder.contentType().xContent(), xContentBuilder.string()); + ObjectPath objectPath = ObjectPath.createFromXContent(xContentBuilder.contentType().xContent(), xContentBuilder.bytes()); Object object = objectPath.evaluate("field1..field2"); assertThat(object, instanceOf(String.class)); assertThat(object, equalTo("value2")); @@ -83,7 +82,7 @@ public class ObjectPathTests extends ESTestCase { xContentBuilder.field("field2", 333); xContentBuilder.endObject(); xContentBuilder.endObject(); - ObjectPath objectPath = ObjectPath.createFromXContent(xContentBuilder.contentType().xContent(), xContentBuilder.string()); + ObjectPath objectPath = ObjectPath.createFromXContent(xContentBuilder.contentType().xContent(), xContentBuilder.bytes()); Object object = objectPath.evaluate("field1.field2"); assertThat(object, instanceOf(Integer.class)); assertThat(object, equalTo(333)); @@ -96,7 +95,7 @@ public class ObjectPathTests extends ESTestCase { xContentBuilder.field("field2", 3.55); xContentBuilder.endObject(); xContentBuilder.endObject(); - ObjectPath objectPath = ObjectPath.createFromXContent(xContentBuilder.contentType().xContent(), xContentBuilder.string()); + ObjectPath objectPath = ObjectPath.createFromXContent(xContentBuilder.contentType().xContent(), xContentBuilder.bytes()); Object object = objectPath.evaluate("field1.field2"); assertThat(object, instanceOf(Double.class)); assertThat(object, equalTo(3.55)); @@ -109,7 +108,7 @@ public class ObjectPathTests extends ESTestCase { xContentBuilder.array("array1", "value1", "value2"); xContentBuilder.endObject(); xContentBuilder.endObject(); - ObjectPath objectPath = ObjectPath.createFromXContent(xContentBuilder.contentType().xContent(), xContentBuilder.string()); + ObjectPath objectPath = ObjectPath.createFromXContent(xContentBuilder.contentType().xContent(), xContentBuilder.bytes()); Object object = objectPath.evaluate("field1.array1"); assertThat(object, instanceOf(List.class)); List list = (List) object; @@ -138,7 +137,7 @@ public class ObjectPathTests extends ESTestCase { xContentBuilder.endArray(); xContentBuilder.endObject(); xContentBuilder.endObject(); - ObjectPath objectPath = ObjectPath.createFromXContent(xContentBuilder.contentType().xContent(), xContentBuilder.string()); + ObjectPath objectPath = ObjectPath.createFromXContent(xContentBuilder.contentType().xContent(), xContentBuilder.bytes()); Object object = objectPath.evaluate("field1.array1.1.element"); assertThat(object, instanceOf(String.class)); assertThat(object, equalTo("value2")); @@ -165,7 +164,7 @@ public class ObjectPathTests extends ESTestCase { xContentBuilder.endObject(); xContentBuilder.endObject(); xContentBuilder.endObject(); - ObjectPath objectPath = ObjectPath.createFromXContent(xContentBuilder.contentType().xContent(), xContentBuilder.string()); + ObjectPath objectPath = ObjectPath.createFromXContent(xContentBuilder.contentType().xContent(), xContentBuilder.bytes()); Object object = objectPath.evaluate("metadata.templates"); assertThat(object, instanceOf(Map.class)); Map map = (Map)object; @@ -183,7 +182,7 @@ public class ObjectPathTests extends ESTestCase { xContentBuilder.endObject(); xContentBuilder.endObject(); xContentBuilder.endObject(); - ObjectPath objectPath = ObjectPath.createFromXContent(xContentBuilder.contentType().xContent(), xContentBuilder.string()); + ObjectPath objectPath = ObjectPath.createFromXContent(xContentBuilder.contentType().xContent(), xContentBuilder.bytes()); try { objectPath.evaluate("field1.$placeholder.element1"); fail("evaluate should have failed due to unresolved placeholder"); @@ -246,7 +245,8 @@ public class ObjectPathTests extends ESTestCase { xContentBuilder.field("index", "test2"); xContentBuilder.endObject(); xContentBuilder.endArray(); - ObjectPath objectPath = ObjectPath.createFromXContent(XContentFactory.xContent(XContentType.YAML), xContentBuilder.string()); + ObjectPath objectPath = ObjectPath.createFromXContent( + XContentFactory.xContent(xContentBuilder.contentType()), xContentBuilder.bytes()); Object object = objectPath.evaluate(""); assertThat(object, notNullValue()); assertThat(object, instanceOf(List.class)); From 4f487ab1b9dedd8401d8681870238de7fbfe0598 Mon Sep 17 00:00:00 2001 From: javanna Date: Fri, 17 Feb 2017 22:39:01 +0100 Subject: [PATCH 011/105] [TEST] randomize request content_type between all of the supported formats --- .../smoketest/DocsClientYamlTestSuiteIT.java | 5 ++ .../Backwards50ClientYamlTestSuiteIT.java | 5 ++ .../yaml/ClientYamlTestExecutionContext.java | 70 +++++++++++++++---- .../rest/yaml/ESClientYamlSuiteTestCase.java | 8 ++- 4 files changed, 73 insertions(+), 15 deletions(-) diff --git a/docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java b/docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java index a79730339e3..628c364d455 100644 --- a/docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java +++ b/docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java @@ -48,5 +48,10 @@ public class DocsClientYamlTestSuiteIT extends ESClientYamlSuiteTestCase { logger.error("This failing test was generated by documentation starting at {}. It may include many snippets. " + "See docs/README.asciidoc for an explanation of test generation.", name); } + + @Override + protected boolean randomizeContentType() { + return false; + } } diff --git a/qa/backwards-5.0/src/test/java/org/elasticsearch/backwards/Backwards50ClientYamlTestSuiteIT.java b/qa/backwards-5.0/src/test/java/org/elasticsearch/backwards/Backwards50ClientYamlTestSuiteIT.java index cac063fce78..f6cb67f92fb 100644 --- a/qa/backwards-5.0/src/test/java/org/elasticsearch/backwards/Backwards50ClientYamlTestSuiteIT.java +++ b/qa/backwards-5.0/src/test/java/org/elasticsearch/backwards/Backwards50ClientYamlTestSuiteIT.java @@ -39,5 +39,10 @@ public class Backwards50ClientYamlTestSuiteIT extends ESClientYamlSuiteTestCase public static Iterable parameters() throws IOException { return createParameters(); } + + @Override + protected boolean randomizeContentType() { + return false; + } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java index fe397f512c5..a2e03a063cf 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestExecutionContext.java @@ -18,15 +18,21 @@ */ package org.elasticsearch.test.rest.yaml; +import com.carrotsearch.randomizedtesting.RandomizedTest; import org.apache.http.HttpEntity; +import org.apache.http.entity.ByteArrayEntity; import org.apache.http.entity.ContentType; -import org.apache.http.entity.StringEntity; import org.apache.logging.log4j.Logger; +import org.apache.lucene.util.BytesRef; import org.elasticsearch.Version; import org.elasticsearch.common.logging.Loggers; +import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -41,13 +47,18 @@ public class ClientYamlTestExecutionContext { private static final Logger logger = Loggers.getLogger(ClientYamlTestExecutionContext.class); + private static final XContentType[] STREAMING_CONTENT_TYPES = new XContentType[]{XContentType.JSON, XContentType.SMILE}; + private final Stash stash = new Stash(); private final ClientYamlTestClient clientYamlTestClient; private ClientYamlTestResponse response; - public ClientYamlTestExecutionContext(ClientYamlTestClient clientYamlTestClient) { + private final boolean randomizeContentType; + + public ClientYamlTestExecutionContext(ClientYamlTestClient clientYamlTestClient, boolean randomizeContentType) { this.clientYamlTestClient = clientYamlTestClient; + this.randomizeContentType = randomizeContentType; } /** @@ -65,7 +76,7 @@ public class ClientYamlTestExecutionContext { } } - HttpEntity entity = createEntity(bodies); + HttpEntity entity = createEntity(bodies, headers); try { response = callApiInternal(apiName, requestParams, entity, headers); return response; @@ -80,23 +91,56 @@ public class ClientYamlTestExecutionContext { } } - private HttpEntity createEntity(List> bodies) throws IOException { + private HttpEntity createEntity(List> bodies, Map headers) throws IOException { if (bodies.isEmpty()) { return null; } if (bodies.size() == 1) { - String bodyAsString = bodyAsString(stash.replaceStashedValues(bodies.get(0))); - return new StringEntity(bodyAsString, ContentType.APPLICATION_JSON); + XContentType xContentType = getContentType(headers, XContentType.values()); + BytesRef bytesRef = bodyAsBytesRef(bodies.get(0), xContentType); + return new ByteArrayEntity(bytesRef.bytes, bytesRef.offset, bytesRef.length, + ContentType.create(xContentType.mediaTypeWithoutParameters(), StandardCharsets.UTF_8)); + } else { + XContentType xContentType = getContentType(headers, STREAMING_CONTENT_TYPES); + List bytesRefList = new ArrayList<>(); + int totalBytesLength = 0; + for (Map body : bodies) { + BytesRef bytesRef = bodyAsBytesRef(body, xContentType); + bytesRefList.add(bytesRef); + totalBytesLength += bytesRef.length - bytesRef.offset + 1; + } + byte[] bytes = new byte[totalBytesLength]; + int position = 0; + for (BytesRef bytesRef : bytesRefList) { + for (int i = bytesRef.offset; i < bytesRef.length; i++) { + bytes[position++] = bytesRef.bytes[i]; + } + bytes[position++] = xContentType.xContent().streamSeparator(); + } + return new ByteArrayEntity(bytes, ContentType.create(xContentType.mediaTypeWithoutParameters(), StandardCharsets.UTF_8)); } - StringBuilder bodyBuilder = new StringBuilder(); - for (Map body : bodies) { - bodyBuilder.append(bodyAsString(stash.replaceStashedValues(body))).append("\n"); - } - return new StringEntity(bodyBuilder.toString(), ContentType.APPLICATION_JSON); } - private String bodyAsString(Map body) throws IOException { - return XContentFactory.jsonBuilder().map(body).string(); + private XContentType getContentType(Map headers, XContentType[] supportedContentTypes) { + XContentType xContentType = null; + String contentType = headers.get("Content-Type"); + if (contentType != null) { + xContentType = XContentType.fromMediaType(contentType); + } + if (xContentType != null) { + return xContentType; + } + if (randomizeContentType) { + return RandomizedTest.randomFrom(supportedContentTypes); + } + return XContentType.JSON; + } + + private BytesRef bodyAsBytesRef(Map bodyAsMap, XContentType xContentType) throws IOException { + Map finalBodyAsMap = stash.replaceStashedValues(bodyAsMap); + try (XContentBuilder builder = XContentFactory.contentBuilder(xContentType)) { + return builder.map(finalBodyAsMap).bytes().toBytesRef(); + } } private ClientYamlTestResponse callApiInternal(String apiName, Map params, diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java index b64f12e9f02..32436515fe8 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java @@ -145,8 +145,8 @@ public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase { } ClientYamlTestClient clientYamlTestClient = new ClientYamlTestClient(restSpec, restClient, hosts, esVersion); - restTestExecutionContext = new ClientYamlTestExecutionContext(clientYamlTestClient); - adminExecutionContext = new ClientYamlTestExecutionContext(clientYamlTestClient); + restTestExecutionContext = new ClientYamlTestExecutionContext(clientYamlTestClient, randomizeContentType()); + adminExecutionContext = new ClientYamlTestExecutionContext(clientYamlTestClient, false); String[] blacklist = resolvePathsProperty(REST_TESTS_BLACKLIST, null); blacklistPathMatchers = new ArrayList<>(); for (String entry : blacklist) { @@ -381,4 +381,8 @@ public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase { private String errorMessage(ExecutableSection executableSection, Throwable t) { return "Failure at [" + testCandidate.getSuitePath() + ":" + executableSection.getLocation().lineNumber + "]: " + t.getMessage(); } + + protected boolean randomizeContentType() { + return true; + } } From 756e26cb33c68a400a3b4c46938781a134722e69 Mon Sep 17 00:00:00 2001 From: javanna Date: Wed, 22 Feb 2017 18:06:23 +0100 Subject: [PATCH 012/105] [TEST] make headers case-insensitive when running yaml tests --- .../test/rest/yaml/section/ApiCallSection.java | 4 ---- .../elasticsearch/test/rest/yaml/section/DoSection.java | 7 +++---- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/ApiCallSection.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/ApiCallSection.java index 5d097f872b4..45538454585 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/ApiCallSection.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/ApiCallSection.java @@ -61,10 +61,6 @@ public class ApiCallSection { this.headers.putAll(otherHeaders); } - public void addHeader(String key, String value) { - this.headers.put(key, value); - } - public Map getHeaders() { return unmodifiableMap(headers); } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java index 482525c46b5..744b52303b5 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java @@ -38,6 +38,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TreeMap; import static java.util.Collections.emptyList; import static java.util.Collections.unmodifiableList; @@ -77,7 +78,7 @@ public class DoSection implements ExecutableSection { DoSection doSection = new DoSection(parser.getTokenLocation()); ApiCallSection apiCallSection = null; - Map headers = new HashMap<>(); + Map headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); List expectedWarnings = new ArrayList<>(); while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { @@ -138,9 +139,7 @@ public class DoSection implements ExecutableSection { if (apiCallSection == null) { throw new IllegalArgumentException("client call section is mandatory within a do section"); } - if (headers.isEmpty() == false) { - apiCallSection.addHeaders(headers); - } + apiCallSection.addHeaders(headers); doSection.setApiCallSection(apiCallSection); doSection.setExpectedWarningHeaders(unmodifiableList(expectedWarnings)); } finally { From 1ceaef0de63b75904faa8d77d962dea364e176d0 Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Tue, 21 Feb 2017 15:20:04 +0000 Subject: [PATCH 013/105] Fixes terms error count for multiple reduce phases Previously when multiple reduces occured for the terms aggregation we would add up the errors for the aggregations but would not take into account the errors that had already been calculated for the previous reduce phases. This change corrects that by adding the previously created errors to the new error value. Closes #23286 --- .../bucket/terms/InternalTerms.java | 11 +- .../bucket/TermsDocCountErrorIT.java | 153 ++++++++++++++---- 2 files changed, 128 insertions(+), 36 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java index 938b20d9fc8..a4218da89ad 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java @@ -33,7 +33,6 @@ import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -226,7 +225,15 @@ public abstract class InternalTerms, B extends Int if (terms.getBucketsInternal().size() < getShardSize() || InternalOrder.isTermOrder(order)) { thisAggDocCountError = 0; } else if (InternalOrder.isCountDesc(this.order)) { - thisAggDocCountError = terms.getBucketsInternal().get(terms.getBucketsInternal().size() - 1).docCount; + if (terms.getDocCountError() > 0) { + // If there is an existing docCountError for this agg then + // use this as the error for this aggregation + thisAggDocCountError = terms.getDocCountError(); + } else { + // otherwise use the doc count of the last term in the + // aggregation + thisAggDocCountError = terms.getBucketsInternal().get(terms.getBucketsInternal().size() - 1).docCount; + } } else { thisAggDocCountError = -1; } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/TermsDocCountErrorIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/TermsDocCountErrorIT.java index 3f2163f25d9..9d5ca3afc54 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/TermsDocCountErrorIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/TermsDocCountErrorIT.java @@ -20,10 +20,7 @@ package org.elasticsearch.search.aggregations.bucket; import org.elasticsearch.action.index.IndexRequestBuilder; -import org.elasticsearch.action.search.SearchRequestBuilder; import org.elasticsearch.action.search.SearchResponse; -import org.elasticsearch.client.Client; -import org.elasticsearch.client.FilterClient; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentType; @@ -33,11 +30,12 @@ import org.elasticsearch.search.aggregations.bucket.terms.Terms.Bucket; import org.elasticsearch.search.aggregations.bucket.terms.Terms.Order; import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregatorFactory.ExecutionMode; import org.elasticsearch.test.ESIntegTestCase; -import org.elasticsearch.test.client.RandomizingClient; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.List; +import java.util.Map; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.search.aggregations.AggregationBuilders.sum; @@ -64,36 +62,6 @@ public class TermsDocCountErrorIT extends ESIntegTestCase { private static int numRoutingValues; - public static Client client() { - Client client = ESIntegTestCase.client(); - if (client instanceof RandomizingClient) { - return new FilterClient(client) { - /* this test doesn't work with multiple reduce phases since: - * the error for a term is the sum of the errors across all aggs that need to be reduced. - * if the term is in the aggregation, then we just use the associated error, but if it is not we need to use the - * aggregation-level error, ie. the maximum count that a doc that is not in the top list could have. - * - * the problem is that the logic we have today assumes there is a single reduce. So for instance for the agg-level error - * it takes the count of the last term. This is correct if the agg was produced on a shard: if it had a greater count - * then it would be in the top list. However if we are on an intermediate reduce, this does not work anymore. - * - * Another assumption that does not hold is that right now if a term is present in an agg, we assume its count is accurate. - * Again this is true if the agg was produced on a shard, but not if this is the result of an intermediate reduce. - * - * try with this seed and remove the setReduceUpTo below - * -Dtests.seed=B32081B1E8589AE5 -Dtests.class=org.elasticsearch.search.aggregations.bucket.TermsDocCountErrorIT - * -Dtests.method="testDoubleValueField" -Dtests.locale=lv -Dtests.timezone=WET - * This must will be addressed in a followup to #23253 - */ - @Override - public SearchRequestBuilder prepareSearch(String... indices) { - return this.in.prepareSearch(indices).setBatchedReduceSize(512); - } - }; - } - return client; - } - @Override public void setupSuiteScopeCluster() throws Exception { assertAcked(client().admin().indices().prepareCreate("idx") @@ -133,6 +101,68 @@ public class TermsDocCountErrorIT extends ESIntegTestCase { .field(DOUBLE_FIELD_NAME, 1.0 * randomInt(numUniqueTerms)) .endObject())); } + assertAcked(prepareCreate("idx_fixed_docs_0").addMapping("type", STRING_FIELD_NAME, "type=keyword") + .setSettings(Settings.builder().put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1))); + Map shard0DocsPerTerm = new HashMap<>(); + shard0DocsPerTerm.put("A", 25); + shard0DocsPerTerm.put("B", 18); + shard0DocsPerTerm.put("C", 6); + shard0DocsPerTerm.put("D", 3); + shard0DocsPerTerm.put("E", 2); + shard0DocsPerTerm.put("F", 2); + shard0DocsPerTerm.put("G", 2); + shard0DocsPerTerm.put("H", 2); + shard0DocsPerTerm.put("I", 1); + shard0DocsPerTerm.put("J", 1); + for (Map.Entry entry : shard0DocsPerTerm.entrySet()) { + for (int i = 0; i < entry.getValue(); i++) { + String term = entry.getKey(); + builders.add(client().prepareIndex("idx_fixed_docs_0", "type", term + "-" + i) + .setSource(jsonBuilder().startObject().field(STRING_FIELD_NAME, term).endObject())); + } + } + + assertAcked(prepareCreate("idx_fixed_docs_1").addMapping("type", STRING_FIELD_NAME, "type=keyword") + .setSettings(Settings.builder().put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1))); + Map shard1DocsPerTerm = new HashMap<>(); + shard1DocsPerTerm.put("A", 30); + shard1DocsPerTerm.put("B", 25); + shard1DocsPerTerm.put("F", 17); + shard1DocsPerTerm.put("Z", 16); + shard1DocsPerTerm.put("G", 15); + shard1DocsPerTerm.put("H", 14); + shard1DocsPerTerm.put("I", 10); + shard1DocsPerTerm.put("Q", 6); + shard1DocsPerTerm.put("J", 8); + shard1DocsPerTerm.put("C", 4); + for (Map.Entry entry : shard1DocsPerTerm.entrySet()) { + for (int i = 0; i < entry.getValue(); i++) { + String term = entry.getKey(); + builders.add(client().prepareIndex("idx_fixed_docs_1", "type", term + "-" + i) + .setSource(jsonBuilder().startObject().field(STRING_FIELD_NAME, term).field("shard", 1).endObject())); + } + } + + assertAcked(prepareCreate("idx_fixed_docs_2") + .addMapping("type", STRING_FIELD_NAME, "type=keyword") + .setSettings(Settings.builder().put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1))); + Map shard2DocsPerTerm = new HashMap<>(); + shard2DocsPerTerm.put("A", 45); + shard2DocsPerTerm.put("C", 44); + shard2DocsPerTerm.put("Z", 36); + shard2DocsPerTerm.put("G", 30); + shard2DocsPerTerm.put("E", 29); + shard2DocsPerTerm.put("H", 28); + shard2DocsPerTerm.put("Q", 2); + shard2DocsPerTerm.put("D", 1); + for (Map.Entry entry : shard2DocsPerTerm.entrySet()) { + for (int i = 0; i < entry.getValue(); i++) { + String term = entry.getKey(); + builders.add(client().prepareIndex("idx_fixed_docs_2", "type", term + "-" + i) + .setSource(jsonBuilder().startObject().field(STRING_FIELD_NAME, term).field("shard", 2).endObject())); + } + } + indexRandom(true, builders); ensureSearchable(); } @@ -938,5 +968,60 @@ public class TermsDocCountErrorIT extends ESIntegTestCase { assertUnboundedDocCountError(size, accurateResponse, testResponse); } + + /** + * Test a case where we know exactly how many of each term is on each shard + * so we know the exact error value for each term. To do this we search over + * 3 one-shard indices. + */ + public void testFixedDocs() throws Exception { + SearchResponse response = client().prepareSearch("idx_fixed_docs_0", "idx_fixed_docs_1", "idx_fixed_docs_2").setTypes("type") + .addAggregation(terms("terms") + .executionHint(randomExecutionHint()) + .field(STRING_FIELD_NAME) + .showTermDocCountError(true) + .size(5).shardSize(5) + .collectMode(randomFrom(SubAggCollectionMode.values()))) + .execute().actionGet(); + assertSearchResponse(response); + + Terms terms = response.getAggregations().get("terms"); + assertThat(terms, notNullValue()); + assertThat(terms.getDocCountError(), equalTo(46L)); + List buckets = terms.getBuckets(); + assertThat(buckets, notNullValue()); + assertThat(buckets.size(), equalTo(5)); + + Bucket bucket = buckets.get(0); + assertThat(bucket, notNullValue()); + assertThat(bucket.getKey(), equalTo("A")); + assertThat(bucket.getDocCount(), equalTo(100L)); + assertThat(bucket.getDocCountError(), equalTo(0L)); + + bucket = buckets.get(1); + assertThat(bucket, notNullValue()); + assertThat(bucket.getKey(), equalTo("Z")); + assertThat(bucket.getDocCount(), equalTo(52L)); + assertThat(bucket.getDocCountError(), equalTo(2L)); + + bucket = buckets.get(2); + assertThat(bucket, notNullValue()); + assertThat(bucket.getKey(), equalTo("C")); + assertThat(bucket.getDocCount(), equalTo(50L)); + assertThat(bucket.getDocCountError(), equalTo(15L)); + + + bucket = buckets.get(3); + assertThat(bucket, notNullValue()); + assertThat(bucket.getKey(), equalTo("G")); + assertThat(bucket.getDocCount(), equalTo(45L)); + assertThat(bucket.getDocCountError(), equalTo(2L)); + + bucket = buckets.get(4); + assertThat(bucket, notNullValue()); + assertThat(bucket.getKey(), equalTo("B")); + assertThat(bucket.getDocCount(), equalTo(43L)); + assertThat(bucket.getDocCountError(), equalTo(29L)); + } } From 2fb0466f669b030dfdd3cf0f876f4f4a98560781 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Mon, 27 Feb 2017 15:42:25 +0100 Subject: [PATCH 014/105] Convert suggestion response parsing to use NamedXContentRegistry (#23355) We recently added parsing code to parse suggesters responses into java api objects. This was done using a switch based on the type of the returned suggestion. We can now replace the switch with using NamedXContentRegistry, which will also be used for aggs parsing. --- .../elasticsearch/search/suggest/Suggest.java | 40 ++++++------------- .../completion/CompletionSuggestion.java | 6 +++ .../suggest/phrase/PhraseSuggestion.java | 9 +++-- .../search/suggest/term/TermSuggestion.java | 12 ++++-- .../search/suggest/SuggestTests.java | 18 +++++++++ .../search/suggest/SuggestionEntryTests.java | 15 +++---- .../search/suggest/SuggestionTests.java | 19 +++++---- 7 files changed, 68 insertions(+), 51 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/suggest/Suggest.java b/core/src/main/java/org/elasticsearch/search/suggest/Suggest.java index ba5ad712f41..f85ea18109b 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/Suggest.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/Suggest.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.suggest; import org.apache.lucene.util.CollectionUtil; +import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.io.stream.StreamInput; @@ -48,7 +49,6 @@ import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.function.Function; import java.util.stream.Collectors; import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; @@ -384,13 +384,14 @@ public class Suggest implements Iterable> fromXContent(XContentParser parser) throws IOException { ensureExpectedToken(XContentParser.Token.FIELD_NAME, parser.currentToken(), parser::getTokenLocation); String typeAndName = parser.currentName(); // we need to extract the type prefix from the name and throw error if it is not present int delimiterPos = typeAndName.indexOf(InternalAggregation.TYPED_KEYS_DELIMITER); - String type = null; - String name = null; + String type; + String name; if (delimiterPos > 0) { type = typeAndName.substring(0, delimiterPos); name = typeAndName.substring(delimiterPos + 1); @@ -399,35 +400,17 @@ public class Suggest implements Iterable entryParser = null; - // the "size" parameter and the SortBy for TermSuggestion cannot be parsed from the response, use default values - // TODO investigate if we can use NamedXContentRegistry instead of this switch - switch (type) { - case Suggestion.NAME: - suggestion = new Suggestion(name, -1); - entryParser = Suggestion.Entry::fromXContent; - break; - case PhraseSuggestion.NAME: - suggestion = new PhraseSuggestion(name, -1); - entryParser = PhraseSuggestion.Entry::fromXContent; - break; - case TermSuggestion.NAME: - suggestion = new TermSuggestion(name, -1, SortBy.SCORE); - entryParser = TermSuggestion.Entry::fromXContent; - break; - case CompletionSuggestion.NAME: - suggestion = new CompletionSuggestion(name, -1); - entryParser = CompletionSuggestion.Entry::fromXContent; - break; - default: - throw new ParsingException(parser.getTokenLocation(), "Unknown suggestion type [{}]", type); - } + + return parser.namedObject(Suggestion.class, type, name); + } + + protected static > void parseEntries(XContentParser parser, Suggestion suggestion, + CheckedFunction entryParser) + throws IOException { ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.nextToken(), parser::getTokenLocation); while ((parser.nextToken()) != XContentParser.Token.END_ARRAY) { suggestion.addTerm(entryParser.apply(parser)); } - return suggestion; } /** @@ -584,6 +567,7 @@ public class Suggest implements Iterable 0; } + public static CompletionSuggestion fromXContent(XContentParser parser, String name) throws IOException { + CompletionSuggestion suggestion = new CompletionSuggestion(name, -1); + parseEntries(parser, suggestion, CompletionSuggestion.Entry::fromXContent); + return suggestion; + } + private static final class OptionPriorityQueue extends org.apache.lucene.util.PriorityQueue { private final Comparator comparator; diff --git a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestion.java b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestion.java index fb646b03aeb..bd6c828bd42 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestion.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestion.java @@ -60,10 +60,13 @@ public class PhraseSuggestion extends Suggest.Suggestion return new Entry(); } + public static PhraseSuggestion fromXContent(XContentParser parser, String name) throws IOException { + PhraseSuggestion suggestion = new PhraseSuggestion(name, -1); + parseEntries(parser, suggestion, PhraseSuggestion.Entry::fromXContent); + return suggestion; + } + public static class Entry extends Suggestion.Entry { - static class Fields { - static final String CUTOFF_SCORE = "cutoff_score"; - } protected double cutoffScore = Double.MIN_VALUE; diff --git a/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggestion.java b/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggestion.java index 31621fb63b8..185e4c76b3c 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggestion.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggestion.java @@ -132,6 +132,13 @@ public class TermSuggestion extends Suggestion { sort.writeTo(out); } + public static TermSuggestion fromXContent(XContentParser parser, String name) throws IOException { + // the "size" parameter and the SortBy for TermSuggestion cannot be parsed from the response, use default values + TermSuggestion suggestion = new TermSuggestion(name, -1, SortBy.SCORE); + parseEntries(parser, suggestion, TermSuggestion.Entry::fromXContent); + return suggestion; + } + @Override protected Entry newEntry() { return new Entry(); @@ -140,8 +147,7 @@ public class TermSuggestion extends Suggestion { /** * Represents a part from the suggest text with suggested options. */ - public static class Entry extends - org.elasticsearch.search.suggest.Suggest.Suggestion.Entry { + public static class Entry extends org.elasticsearch.search.suggest.Suggest.Suggestion.Entry { public Entry(Text text, int offset, int length) { super(text, offset, length); @@ -235,7 +241,7 @@ public class TermSuggestion extends Suggestion { PARSER.declareFloat(constructorArg(), Suggestion.Entry.Option.SCORE); } - public static final Option fromXContent(XContentParser parser) { + public static Option fromXContent(XContentParser parser) { return PARSER.apply(parser, null); } } diff --git a/core/src/test/java/org/elasticsearch/search/suggest/SuggestTests.java b/core/src/test/java/org/elasticsearch/search/suggest/SuggestTests.java index 0948b9e2ffc..24e98899e87 100644 --- a/core/src/test/java/org/elasticsearch/search/suggest/SuggestTests.java +++ b/core/src/test/java/org/elasticsearch/search/suggest/SuggestTests.java @@ -19,8 +19,10 @@ package org.elasticsearch.search.suggest; +import org.elasticsearch.common.ParseField; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.text.Text; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; @@ -47,6 +49,22 @@ import static org.hamcrest.Matchers.equalTo; public class SuggestTests extends ESTestCase { + static NamedXContentRegistry getSuggestersRegistry() { + List namedXContents = new ArrayList<>(); + namedXContents.add(new NamedXContentRegistry.Entry(Suggest.Suggestion.class, new ParseField("term"), + (parser, context) -> TermSuggestion.fromXContent(parser, (String)context))); + namedXContents.add(new NamedXContentRegistry.Entry(Suggest.Suggestion.class, new ParseField("phrase"), + (parser, context) -> PhraseSuggestion.fromXContent(parser, (String)context))); + namedXContents.add(new NamedXContentRegistry.Entry(Suggest.Suggestion.class, new ParseField("completion"), + (parser, context) -> CompletionSuggestion.fromXContent(parser, (String)context))); + return new NamedXContentRegistry(namedXContents); + } + + @Override + protected NamedXContentRegistry xContentRegistry() { + return getSuggestersRegistry(); + } + public static Suggest createTestItem() { int numEntries = randomIntBetween(0, 5); List>> suggestions = new ArrayList<>(); diff --git a/core/src/test/java/org/elasticsearch/search/suggest/SuggestionEntryTests.java b/core/src/test/java/org/elasticsearch/search/suggest/SuggestionEntryTests.java index f1bf602ae65..07f310dfd0f 100644 --- a/core/src/test/java/org/elasticsearch/search/suggest/SuggestionEntryTests.java +++ b/core/src/test/java/org/elasticsearch/search/suggest/SuggestionEntryTests.java @@ -23,7 +23,6 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.text.Text; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.search.suggest.Suggest.Suggestion; import org.elasticsearch.search.suggest.Suggest.Suggestion.Entry; import org.elasticsearch.search.suggest.Suggest.Suggestion.Entry.Option; import org.elasticsearch.search.suggest.completion.CompletionSuggestion; @@ -44,10 +43,8 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXC public class SuggestionEntryTests extends ESTestCase { - @SuppressWarnings("rawtypes") private static final Map, Function> ENTRY_PARSERS = new HashMap<>(); static { - ENTRY_PARSERS.put(Suggestion.Entry.class, Suggestion.Entry::fromXContent); ENTRY_PARSERS.put(TermSuggestion.Entry.class, TermSuggestion.Entry::fromXContent); ENTRY_PARSERS.put(PhraseSuggestion.Entry.class, PhraseSuggestion.Entry::fromXContent); ENTRY_PARSERS.put(CompletionSuggestion.Entry.class, CompletionSuggestion.Entry::fromXContent); @@ -61,13 +58,9 @@ public class SuggestionEntryTests extends ESTestCase { Text entryText = new Text(randomAsciiOfLengthBetween(5, 15)); int offset = randomInt(); int length = randomInt(); - @SuppressWarnings("rawtypes") - Entry entry = null; - Supplier