Commit 7c0825e5 authored by Ben Boeckel's avatar Ben Boeckel
Browse files

api/client: use the core http crate for the client trait

The `reqwest` crate has some unmockable behaviors, notably that a
`Response` can only be made from a `Client` implementation, so the
request needs to go somewhere magical for `reqwest` to be happy.
parent 75550a49
......@@ -15,6 +15,9 @@
* All `EnableState` fields may now be set using `bool` values.
* The `api::projects::merge_requests::EditMergeRequest` endpoint now supports
unlabeling a merge request.
* The `api::Client` trait has been changed to use the `http` crate types.
This allows for clients to not be tied to `reqwest` and for mocking and
testing of the endpoints themselves.
## Fixes
......
......@@ -23,15 +23,17 @@ derive_builder = "~0.9"
itertools = { version = "~0.8", optional = true }
log = "~0.4"
percent-encoding = { version = "^2.0", optional = true }
reqwest = { version = "~0.10", features = ["blocking", "json"], optional = true }
reqwest = { version = "~0.10.5", features = ["blocking", "json"], optional = true }
thiserror = { version = "^1.0.2", optional = true }
url = "^2.1"
bytes = "~0.5"
chrono = { version = "~0.4", features = ["serde"] }
graphql_client = { version = "~0.8", optional = true }
http = "~0.2"
serde = { version = "~1.0", features = ["derive"] }
serde_json = "^1.0"
serde_urlencoded = "~0.5"
url = "^2.1"
[dev-dependencies]
itertools = { version = "~0.8" }
......@@ -63,7 +63,7 @@ mod error;
mod ignore;
mod paged;
mod params;
mod query;
pub(crate) mod query;
mod raw;
mod sudo;
......
......@@ -6,8 +6,9 @@
use std::error::Error;
use reqwest::blocking::{RequestBuilder, Response};
use reqwest::Method;
use bytes::Bytes;
use http::request::Builder as RequestBuilder;
use http::Response;
use url::Url;
use crate::api::ApiError;
......@@ -22,9 +23,10 @@ pub trait Client {
/// This method adds the hostname for the client's target instance.
fn rest_endpoint(&self, endpoint: &str) -> Result<Url, ApiError<Self::Error>>;
/// Build a REST query from a URL and a given method.
fn build_rest(&self, method: Method, url: Url) -> RequestBuilder;
/// Send a REST query.
fn rest(&self, request: RequestBuilder) -> Result<Response, ApiError<Self::Error>>;
fn rest(
&self,
request: RequestBuilder,
body: Vec<u8>,
) -> Result<Response<Bytes>, ApiError<Self::Error>>;
}
......@@ -6,10 +6,10 @@
use std::borrow::Cow;
use reqwest::{header, Method};
use http::{self, header, Method, Request};
use serde::de::DeserializeOwned;
use crate::api::{ApiError, BodyError, Client, Query, QueryParams};
use crate::api::{query, ApiError, BodyError, Client, Query, QueryParams};
/// A trait for providing the necessary information for a single REST API endpoint.
pub trait Endpoint {
......@@ -41,15 +41,18 @@ where
let mut url = client.rest_endpoint(&self.endpoint())?;
self.parameters().add_to_url(&mut url);
let req = client.build_rest(self.method(), url);
let req = if let Some((mime, data)) = self.body()? {
req.header(header::CONTENT_TYPE, mime).body(data)
let req = Request::builder()
.method(self.method())
.uri(query::url_to_http_uri(url));
let (req, data) = if let Some((mime, data)) = self.body()? {
let req = req.header(header::CONTENT_TYPE, mime);
(req, data)
} else {
req
(req, Vec::new())
};
let rsp = client.rest(req)?;
let rsp = client.rest(req, data)?;
let status = rsp.status();
let v = serde_json::from_reader(rsp)?;
let v = serde_json::from_slice(rsp.body())?;
if !status.is_success() {
return Err(ApiError::from_gitlab(v));
}
......
......@@ -11,7 +11,7 @@
pub use std::borrow::Cow;
pub use reqwest::Method;
pub use http::Method;
pub use crate::api::BodyError;
pub use crate::api::Client;
......
......@@ -4,9 +4,9 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
use reqwest::header;
use http::{header, Request};
use crate::api::{ApiError, Client, Endpoint, Query};
use crate::api::{query, ApiError, Client, Endpoint, Query};
/// A query modifier that ignores the data returned from an endpoint.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
......@@ -30,15 +30,18 @@ where
let mut url = client.rest_endpoint(&self.endpoint.endpoint())?;
self.endpoint.parameters().add_to_url(&mut url);
let req = client.build_rest(self.endpoint.method(), url);
let req = if let Some((mime, data)) = self.endpoint.body()? {
req.header(header::CONTENT_TYPE, mime).body(data)
let req = Request::builder()
.method(self.endpoint.method())
.uri(query::url_to_http_uri(url));
let (req, data) = if let Some((mime, data)) = self.endpoint.body()? {
let req = req.header(header::CONTENT_TYPE, mime);
(req, data)
} else {
req
(req, Vec::new())
};
let rsp = client.rest(req)?;
let rsp = client.rest(req, data)?;
if !rsp.status().is_success() {
let v = serde_json::from_reader(rsp)?;
let v = serde_json::from_slice(rsp.body())?;
return Err(ApiError::from_gitlab(v));
}
......
......@@ -4,12 +4,12 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
use reqwest::header::{self, HeaderMap};
use http::{header, HeaderMap, Request};
use serde::de::DeserializeOwned;
use thiserror::Error;
use url::Url;
use crate::api::{ApiError, Client, Endpoint, Query};
use crate::api::{query, ApiError, Client, Endpoint, Query};
/// Errors which may occur with pagination.
#[derive(Debug, Error)]
......@@ -221,20 +221,23 @@ where
page_url
};
let req = client.build_rest(self.endpoint.method(), page_url);
let req = if let Some((mime, data)) = body.as_ref() {
req.header(header::CONTENT_TYPE, *mime).body(data.clone())
let req = Request::builder()
.method(self.endpoint.method())
.uri(query::url_to_http_uri(page_url));
let (req, data) = if let Some((mime, data)) = body.as_ref() {
let req = req.header(header::CONTENT_TYPE, *mime);
(req, data.clone())
} else {
req
(req, Vec::new())
};
let rsp = client.rest(req)?;
let rsp = client.rest(req, data)?;
let status = rsp.status();
if use_keyset_pagination {
next_url = next_page_from_headers(rsp.headers())?;
}
let v = serde_json::from_reader(rsp)?;
let v = serde_json::from_slice(rsp.body())?;
if !status.is_success() {
return Err(ApiError::from_gitlab(v));
}
......
......@@ -4,8 +4,17 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
use http::Uri;
use url::Url;
use crate::api::{ApiError, Client};
pub fn url_to_http_uri(url: Url) -> Uri {
url.as_str()
.parse::<Uri>()
.expect("failed to parse a url::Url as an http::Uri")
}
/// A trait which represents a query which may be made to a GitLab client.
pub trait Query<T, C>
where
......
......@@ -4,9 +4,9 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
use reqwest::header;
use http::{header, Request};
use crate::api::{ApiError, Client, Endpoint, Query};
use crate::api::{query, ApiError, Client, Endpoint, Query};
/// A query modifier that returns the raw data from the endpoint.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
......@@ -30,18 +30,21 @@ where
let mut url = client.rest_endpoint(&self.endpoint.endpoint())?;
self.endpoint.parameters().add_to_url(&mut url);
let req = client.build_rest(self.endpoint.method(), url);
let req = if let Some((mime, data)) = self.endpoint.body()? {
req.header(header::CONTENT_TYPE, mime).body(data)
let req = Request::builder()
.method(self.endpoint.method())
.uri(query::url_to_http_uri(url));
let (req, data) = if let Some((mime, data)) = self.endpoint.body()? {
let req = req.header(header::CONTENT_TYPE, mime);
(req, data)
} else {
req
(req, Vec::new())
};
let rsp = client.rest(req)?;
let rsp = client.rest(req, data)?;
if !rsp.status().is_success() {
let v = serde_json::from_reader(rsp)?;
let v = serde_json::from_slice(rsp.body())?;
return Err(ApiError::from_gitlab(v));
}
Ok(rsp.bytes().unwrap().as_ref().into())
Ok(rsp.into_body().as_ref().into())
}
}
......@@ -4,9 +4,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
use http::{HeaderMap, HeaderValue};
use log::error;
use reqwest::blocking::RequestBuilder;
use reqwest::header::{self, HeaderValue};
use thiserror::Error;
#[derive(Debug, Error)]
......@@ -15,7 +14,7 @@ pub enum AuthError {
#[error("header value error: {}", source)]
HeaderValue {
#[from]
source: header::InvalidHeaderValue,
source: http::header::InvalidHeaderValue,
},
/// This is here to force `_` matching right now.
///
......@@ -39,24 +38,30 @@ pub enum Auth {
}
impl Auth {
/// Sets the appropriate header on the request.
/// Adds the appropriate header to a set of headers.
///
/// Depending on the token type, this will be either the Private-Auth header
/// Depending on the token type, this will be either the Private-Token header
/// or the Authorization header.
///
/// Returns an error if the token string cannot be parsed as a header value.
pub fn set_header(&self, req: RequestBuilder) -> AuthResult<RequestBuilder> {
Ok(match self {
pub fn set_header<'a>(
&self,
headers: &'a mut HeaderMap<HeaderValue>,
) -> AuthResult<&'a mut HeaderMap<HeaderValue>> {
match self {
Auth::Token(token) => {
let mut token_header_value = HeaderValue::from_str(&token)?;
token_header_value.set_sensitive(true);
req.header("PRIVATE-TOKEN", token_header_value)
headers.insert("PRIVATE-TOKEN", token_header_value);
},
Auth::OAuth2(token) => {
let value = format!("Bearer {}", token);
let mut token_header_value = HeaderValue::from_str(&value)?;
token_header_value.set_sensitive(true);
req.header("Authorization", token_header_value)
headers.insert(http::header::AUTHORIZATION, token_header_value);
},
})
}
Ok(headers)
}
}
......@@ -6,19 +6,22 @@
use std::any;
use std::borrow::Borrow;
use std::convert::TryInto;
use std::fmt::{self, Debug, Display};
use bytes::Bytes;
use graphql_client::{GraphQLQuery, QueryBody, Response};
use http::{HeaderMap, Response as HttpResponse};
use itertools::Itertools;
use log::{debug, error, info, warn};
use percent_encoding::{utf8_percent_encode, AsciiSet, PercentEncode, CONTROLS};
use reqwest::blocking::{Client, RequestBuilder, Response as HttpResponse};
use reqwest::{Method, Url};
use reqwest::blocking::{Client, Response as ReqwestResponse};
use serde::de::DeserializeOwned;
use serde::de::Error as SerdeError;
use serde::ser::Serialize;
use serde::{Deserialize, Deserializer, Serializer};
use thiserror::Error;
use url::Url;
use crate::api::projects::{self, pipelines};
use crate::api::users::{CurrentUser, User, Users};
......@@ -2900,8 +2903,13 @@ impl Gitlab {
}
/// Refactored code which talks to Gitlab and transforms error messages properly.
fn send_impl(&self, req: RequestBuilder) -> GitlabResult<HttpResponse> {
let rsp = self.auth.set_header(req)?.send()?;
fn send_impl(&self, req: reqwest::blocking::RequestBuilder) -> GitlabResult<ReqwestResponse> {
let auth_headers = {
let mut headers = HeaderMap::default();
self.auth.set_header(&mut headers)?;
headers
};
let rsp = req.headers(auth_headers).send()?;
let status = rsp.status();
if status.is_server_error() {
return Err(GitlabError::http(status));
......@@ -2911,7 +2919,7 @@ impl Gitlab {
}
/// Refactored code which talks to Gitlab and transforms error messages properly.
fn send<T>(&self, req: RequestBuilder) -> GitlabResult<T>
fn send<T>(&self, req: reqwest::blocking::RequestBuilder) -> GitlabResult<T>
where
T: DeserializeOwned,
{
......@@ -2998,6 +3006,11 @@ pub enum RestError {
#[from]
source: reqwest::Error,
},
#[error("`http` error: {}", source)]
Http {
#[from]
source: http::Error,
},
/// This is here to force `_` matching right now.
///
/// **DO NOT USE**
......@@ -3014,18 +3027,27 @@ impl api::Client for Gitlab {
Ok(self.rest_url.join(endpoint)?)
}
fn build_rest(&self, method: Method, url: Url) -> RequestBuilder {
self.client.request(method, url)
}
fn rest(&self, request: RequestBuilder) -> Result<HttpResponse, api::ApiError<Self::Error>> {
self.auth
.set_header(request)
.map_err(RestError::from)
.map_err(api::ApiError::client)?
.send()
.map_err(RestError::from)
.map_err(api::ApiError::client)
fn rest(
&self,
mut request: http::request::Builder,
body: Vec<u8>,
) -> Result<HttpResponse<Bytes>, api::ApiError<Self::Error>> {
let call = || -> Result<_, RestError> {
self.auth.set_header(request.headers_mut().unwrap())?;
let http_request = request.body(body)?;
let request = http_request.try_into()?;
let rsp = self.client.execute(request)?;
let mut http_rsp = HttpResponse::builder()
.status(rsp.status())
.version(rsp.version());
let headers = http_rsp.headers_mut().unwrap();
for (key, value) in rsp.headers() {
headers.insert(key, value.clone());
}
Ok(http_rsp.body(rsp.bytes()?)?)
};
call().map_err(api::ApiError::client)
}
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment