From 82d32deae088ba72fab2b69a35458ed4c524cf8e Mon Sep 17 00:00:00 2001 From: Lauro Caetano Date: Wed, 17 Feb 2021 08:30:01 +0100 Subject: [PATCH 1/3] Use a conventional struct instead of a Tuple struct The Response struct contains a reqwest::Request and a Method fields, and accessing them by position may be a bit confusing. Giving an actual name to the fields brings us the benefit of easily understanding the methods we are reading, without the need to go to the top to remember which field we are referring to. Reference: https://doc.rust-lang.org/1.9.0/book/structs.html#tuple-structs --- elasticsearch/src/http/response.rs | 32 ++++++++++++++++++------------ 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/elasticsearch/src/http/response.rs b/elasticsearch/src/http/response.rs index e9cb4a8a..66b577fe 100644 --- a/elasticsearch/src/http/response.rs +++ b/elasticsearch/src/http/response.rs @@ -31,12 +31,18 @@ use std::{collections::BTreeMap, fmt, str::FromStr}; use void::Void; /// A response from Elasticsearch -pub struct Response(reqwest::Response, Method); +pub struct Response { + response: reqwest::Response, + method: Method, +} impl Response { /// Creates a new instance of an Elasticsearch response pub fn new(response: reqwest::Response, method: Method) -> Self { - Self(response, method) + Self { + response: response, + method: method, + } } /// Get the response content-length, if known. @@ -47,12 +53,12 @@ impl Response { /// - The response is compressed and automatically decoded (thus changing /// the actual decoded length). pub fn content_length(&self) -> Option { - self.0.content_length() + self.response.content_length() } /// Gets the response content-type. pub fn content_type(&self) -> &str { - self.0 + self.response .headers() .get(crate::http::headers::CONTENT_TYPE) .and_then(|value| value.to_str().ok()) @@ -61,7 +67,7 @@ impl Response { /// Turn the response into an [Error] if Elasticsearch returned an error. pub fn error_for_status_code(self) -> Result { - match self.0.error_for_status_ref() { + match self.response.error_for_status_ref() { Ok(_) => Ok(self), Err(err) => Err(err.into()), } @@ -69,7 +75,7 @@ impl Response { /// Turn the response into an [Error] if Elasticsearch returned an error. pub fn error_for_status_code_ref(&self) -> Result<&Self, ClientError> { - match self.0.error_for_status_ref() { + match self.response.error_for_status_ref() { Ok(_) => Ok(self), Err(err) => Err(err.into()), } @@ -95,36 +101,36 @@ impl Response { where B: DeserializeOwned, { - let body = self.0.json::().await?; + let body = self.response.json::().await?; Ok(body) } /// Gets the response headers. pub fn headers(&self) -> &HeaderMap { - self.0.headers() + self.response.headers() } /// Gets the request method. pub fn method(&self) -> Method { - self.1 + self.method } /// Get the HTTP status code of the response pub fn status_code(&self) -> StatusCode { - self.0.status() + self.response.status() } /// Asynchronously reads the response body as plain text /// /// Reading the response body consumes `self` pub async fn text(self) -> Result { - let body = self.0.text().await?; + let body = self.response.text().await?; Ok(body) } /// Gets the request URL pub fn url(&self) -> &Url { - self.0.url() + self.response.url() } /// Gets the Deprecation warning response headers @@ -132,7 +138,7 @@ impl Response { /// Deprecation headers signal the use of Elasticsearch functionality /// or features that are deprecated and will be removed in a future release. pub fn warning_headers(&self) -> impl Iterator { - self.0.headers().get_all("Warning").iter().map(|w| { + self.response.headers().get_all("Warning").iter().map(|w| { let s = w.to_str().unwrap(); let first_quote = s.find('"').unwrap(); let last_quote = s.len() - 1; From f55519681d0822fb00cd9ba99ec7e41d3ab0a678 Mon Sep 17 00:00:00 2001 From: Lauro Caetano Date: Wed, 17 Feb 2021 08:36:58 +0100 Subject: [PATCH 2/3] Expose `bytes` to the Response object This patch exposes the `Response::bytes`, which is basically a wrapper on reqwest::Response::bytes(). --- elasticsearch/src/http/response.rs | 9 +++++++++ elasticsearch/tests/client.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/elasticsearch/src/http/response.rs b/elasticsearch/src/http/response.rs index 66b577fe..0828000f 100644 --- a/elasticsearch/src/http/response.rs +++ b/elasticsearch/src/http/response.rs @@ -21,6 +21,7 @@ use crate::{ error::Error as ClientError, http::{headers::HeaderMap, Method, StatusCode, Url}, }; +use bytes::Bytes; use serde::{ de, de::{DeserializeOwned, MapAccess, Visitor}, @@ -128,6 +129,14 @@ impl Response { Ok(body) } + /// Asynchronously reads the response body as bytes + /// + /// Reading the response body consumes `self` + pub async fn bytes(self) -> Result { + let bytes: Bytes = self.response.bytes().await?; + Ok(bytes) + } + /// Gets the request URL pub fn url(&self) -> &Url { self.response.url() diff --git a/elasticsearch/tests/client.rs b/elasticsearch/tests/client.rs index 23d4cdc8..671a9357 100644 --- a/elasticsearch/tests/client.rs +++ b/elasticsearch/tests/client.rs @@ -32,6 +32,7 @@ use elasticsearch::{ }; use crate::common::client::index_documents; +use bytes::Bytes; use hyper::Method; use serde_json::{json, Value}; use std::time::Duration; @@ -309,6 +310,32 @@ async fn search_with_no_body() -> Result<(), failure::Error> { Ok(()) } +#[tokio::test] +async fn read_response_as_bytes() -> Result<(), failure::Error> { + let client = client::create_default(); + let _ = index_documents(&client).await?; + let response = client + .search(SearchParts::None) + .pretty(true) + .q("title:Elasticsearch") + .send() + .await?; + + assert_eq!(response.status_code(), StatusCode::OK); + assert_eq!(response.method(), elasticsearch::http::Method::Get); + + let response: Bytes = response.bytes().await?; + let json: Value = serde_json::from_slice(&response).unwrap(); + + assert!(json["took"].as_i64().is_some()); + + for hit in json["hits"]["hits"].as_array().unwrap() { + assert!(hit["_source"]["title"].as_str().is_some()); + } + + Ok(()) +} + #[tokio::test] async fn cat_health_format_json() -> Result<(), failure::Error> { let client = client::create_default(); From 83ee450c709c7b3c7fb240a73bf7e417942547a9 Mon Sep 17 00:00:00 2001 From: Lauro Caetano Date: Sun, 21 Feb 2021 08:48:13 +0100 Subject: [PATCH 3/3] Add additional note on vm.max_map_count settings --- CONTRIBUTING.md | 47 +++++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7c90562e..e84f7706 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,7 +10,7 @@ to address it, please assign the issue to yourself, so that others know that you ## Sign the Contributor License Agreement -We do ask that you sign the [Contiributor License Agreement](https://www.elastic.co/contributor-agreement) +We do ask that you sign the [Contiributor License Agreement](https://www.elastic.co/contributor-agreement) before we can accept pull requests from you. ## Development @@ -23,21 +23,24 @@ The project makes use of the following, which should be installed - [**Docker**](https://www.docker.com/) - Docker is used to start instances of Elasticsearch by using + Docker is used to start instances of Elasticsearch by using [Elastic's Elasticsearch docker images](https://container-library.elastic.co/). For Windows, use [Docker with WSL 2 backend](https://docs.docker.com/docker-for-windows/wsl/). - + - [**Cargo make**](https://sagiegurari.github.io/cargo-make/) - Cargo make is used to define and configure a set of tasks, and run them as a flow. This helps with performing actions + Cargo make is used to define and configure a set of tasks, and run them as a flow. This helps with performing actions such as starting an Elasticsearch instance for integration tests - + Cargo make can be installed with - + ```sh cargo install --force cargo-make ``` - + + +If you are running the tests in Docker, [set `vm.max_map_count` for your platform](https://www.elastic.co/guide/en/elasticsearch/reference/current/docker.html#_set_vm_max_map_count_to_at_least_262144) to allow Elasticsearch to start. + ### Cargo make Cargo make is used to define and configure a set of tasks, and run them as a flow. To see all of the Elasticsearch @@ -64,7 +67,7 @@ The `Elasticsearch` category of steps are specifically defined for this project - Run Elasticsearch package tests - Optionally pass + Optionally pass - `STACK_VERSION`: Elasticsearch version like `7.9.0` or can be a snapshot release like `7.x-SNAPSHOT` @@ -75,12 +78,12 @@ The `Elasticsearch` category of steps are specifically defined for this project - Run YAML tests - Optionally pass + Optionally pass - `STACK_VERSION`: Elasticsearch version like `7.9.0` or can be a snapshot release like `7.x-SNAPSHOT` - `TEST_SUITE`: Elasticsearch distribution of `free` or `platinum` - + ```sh cargo make test-yaml --env STACK_VERSION=7.9.0 --env TEST_SUITE=free ``` @@ -96,9 +99,9 @@ the root client, `Elasticsearch`, or on one of the _namespaced clients_, such as are based on the grouping of APIs within the [Elasticsearch](https://github.com/elastic/elasticsearch/tree/master/rest-api-spec) and [X-Pack](https://github.com/elastic/elasticsearch/tree/master/x-pack/plugin/src/test/resources/rest-api-spec/api) REST API specs from which much of the client is generated. All API functions are `async` only, and can be `await`ed. -- #### `api_generator` +- #### `api_generator` - A small executable that downloads REST API specs from GitHub and generates much of the client package from the specs. + A small executable that downloads REST API specs from GitHub and generates much of the client package from the specs. The minimum REST API spec version compatible with the generator is `v7.4.0`. The `api_generator` package makes heavy use of the [`syn`](https://docs.rs/syn/1.0.5/syn/) and [`quote`](https://docs.rs/quote/1.0.2/quote/) crates to generate Rust code from the REST API specs. @@ -110,11 +113,11 @@ can be `to_string()`'ed and written to disk, and this is used to create much of A small executable that downloads YAML tests from GitHub and generates client tests from the YAML tests. The version of YAML tests to download are determined from the commit hash of a running Elasticsearch instance. - + The `yaml_test_runner` package can be run with `cargo make test-yaml` to run the generated client tests, passing environment variables `TEST_SUITE` and `STACK_VERSION` to control the distribution and version, respectively. - + ### Design principles 1. Generate as much of the client as feasible from the REST API specs @@ -124,8 +127,8 @@ can be `to_string()`'ed and written to disk, and this is used to create much of - accepted HTTP methods e.g. `GET`, `POST` - the URL query string parameters - whether the API accepts a body - -2. Prefer generation methods that produce ASTs and token streams over strings. + +2. Prefer generation methods that produce ASTs and token streams over strings. The `quote` and `syn` crates help 3. Get it working, then refine/refactor @@ -134,14 +137,14 @@ The `quote` and `syn` crates help - Design of the API is conducive to ease of use - Asynchronous only - Control API invariants through arguments on API function. For example - + ```no_run client.delete_script(DeleteScriptParts::Id("script_id")) .send() .await?; ``` - - An id must always be provided for a delete script API call, so the `delete_script()` function + + An id must always be provided for a delete script API call, so the `delete_script()` function must accept it as a value. ### Coding style guide @@ -172,7 +175,7 @@ cargo make clippy ### Running MSVC debugger in VS Code -From [Bryce Van Dyk's blog post](https://www.brycevandyk.com/debug-rust-on-windows-with-visual-studio-code-and-the-msvc-debugger/), +From [Bryce Van Dyk's blog post](https://www.brycevandyk.com/debug-rust-on-windows-with-visual-studio-code-and-the-msvc-debugger/), if wishing to use the MSVC debugger with Rust in VS code, which may be preferred on Windows 1. Install [C/C++ VS Code extensions](https://marketplace.visualstudio.com/items?itemName=ms-vscode.cpptools) @@ -182,7 +185,7 @@ if wishing to use the MSVC debugger with Rust in VS code, which may be preferred ```json { "version": "0.2.0", - "configurations": [ + "configurations": [ { "name": "Debug api_generator", "type": "cppvsdbg", @@ -197,5 +200,5 @@ if wishing to use the MSVC debugger with Rust in VS code, which may be preferred ] } ``` - + 3. Add `"debug.allowBreakpointsEverywhere": true` to VS code settings.json