Skip to content

Commit

Permalink
Add bytes method to Response struct (#164)
Browse files Browse the repository at this point in the history
* 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

* Expose `bytes` to the Response object

This patch exposes the `Response::bytes`, which is basically a wrapper
on reqwest::Response::bytes().

* Add additional note on vm.max_map_count settings

Closes #160

(cherry picked from commit 96189a5)
  • Loading branch information
laurocaetano authored and russcam committed Feb 23, 2021
1 parent c5cde88 commit c17f751
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 35 deletions.
47 changes: 25 additions & 22 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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`
Expand All @@ -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
```
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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",
Expand All @@ -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
41 changes: 28 additions & 13 deletions elasticsearch/src/http/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -31,12 +32,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.
Expand All @@ -47,12 +54,12 @@ impl Response {
/// - The response is compressed and automatically decoded (thus changing
/// the actual decoded length).
pub fn content_length(&self) -> Option<u64> {
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())
Expand All @@ -61,15 +68,15 @@ impl Response {

/// Turn the response into an [Error] if Elasticsearch returned an error.
pub fn error_for_status_code(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()),
}
}

/// 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()),
}
Expand All @@ -95,44 +102,52 @@ impl Response {
where
B: DeserializeOwned,
{
let body = self.0.json::<B>().await?;
let body = self.response.json::<B>().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<String, ClientError> {
let body = self.0.text().await?;
let body = self.response.text().await?;
Ok(body)
}

/// Asynchronously reads the response body as bytes
///
/// Reading the response body consumes `self`
pub async fn bytes(self) -> Result<Bytes, ClientError> {
let bytes: Bytes = self.response.bytes().await?;
Ok(bytes)
}

/// Gets the request URL
pub fn url(&self) -> &Url {
self.0.url()
self.response.url()
}

/// Gets the Deprecation warning response headers
///
/// 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<Item = &str> {
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;
Expand Down
27 changes: 27 additions & 0 deletions elasticsearch/tests/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -316,6 +317,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();
Expand Down

0 comments on commit c17f751

Please sign in to comment.