Skip to content

Make http_ext::Request and http_ext::Response private #1589

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
o0Ignition0o merged 1 commit into from
Aug 24, 2022
Merged

Make http_ext::Request and http_ext::Response private #1589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions NEXT_CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ Let's remove it since it's simple to add back if later required.

By [@garypen](https://github.com/garypen) in https://github.com/apollographql/router/pull/1569

### Request and Response types from apollo_router::http_ext are private ([Issue #1589](https://github.com/apollographql/router/issues/1589))

These types were wrappers around the `Request` and `Response` types from the `http` crate.
Now the latter are used directly instead.

By [@SimonSapin](https://github.com/SimonSapin) in https://github.com/apollographql/router/pull/1589

### QueryPlan::usage_reporting and QueryPlannerContent are private ([Issue #1556](https://github.com/apollographql/router/issues/1556))

These items have been removed from the public API of `apollo_router::services::execution`.
Expand Down
51 changes: 25 additions & 26 deletions apollo-router/src/axum_http_server_factory.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,8 +417,8 @@ async fn custom_plugin_handler(
async fn handle_get(
Host(host): Host,
service: BoxService<
http_ext::Request<graphql::Request>,
http_ext::Response<BoxStream<'static, graphql::Response>>,
http::Request<graphql::Request>,
http::Response<BoxStream<'static, graphql::Response>>,
BoxError,
>,
http_request: Request<Body>,
Expand Down Expand Up @@ -455,8 +455,8 @@ async fn handle_post(
OriginalUri(uri): OriginalUri,
Json(request): Json<graphql::Request>,
service: BoxService<
http_ext::Request<graphql::Request>,
http_ext::Response<BoxStream<'static, graphql::Response>>,
http::Request<graphql::Request>,
http::Response<BoxStream<'static, graphql::Response>>,
BoxError,
>,
header_map: HeaderMap,
Expand Down Expand Up @@ -489,16 +489,16 @@ async fn run_graphql_request<RS>(
) -> impl IntoResponse
where
RS: Service<
http_ext::Request<graphql::Request>,
Response = http_ext::Response<BoxStream<'static, graphql::Response>>,
http::Request<graphql::Request>,
Response = http::Response<BoxStream<'static, graphql::Response>>,
Error = BoxError,
> + Send,
{
match service.ready_oneshot().await {
Ok(mut service) => {
let (head, body) = http_request.into_parts();

match service.call(Request::from_parts(head, body).into()).await {
match service.call(Request::from_parts(head, body)).await {
Err(e) => {
if let Some(source_err) = e.source() {
if source_err.is::<RateLimited>() {
Expand All @@ -516,7 +516,7 @@ where
.into_response()
}
Ok(response) => {
let (mut parts, mut stream) = http::Response::from(response).into_parts();
let (mut parts, mut stream) = response.into_parts();
parts.headers.insert(
"content-type",
HeaderValue::from_static("multipart/mixed;boundary=\"graphql\""),
Expand Down Expand Up @@ -731,7 +731,6 @@ mod tests {

use super::*;
use crate::configuration::Cors;
use crate::http_ext::Request;
use crate::services::new_service::NewService;
use crate::services::transport;

Expand Down Expand Up @@ -777,21 +776,21 @@ mod tests {
mock! {
#[derive(Debug)]
SupergraphService {
fn service_call(&mut self, req: Request<graphql::Request>) -> Result<http_ext::Response<BoxStream<'static, graphql::Response>>, BoxError>;
fn service_call(&mut self, req: http::Request<graphql::Request>) -> Result<http::Response<BoxStream<'static, graphql::Response>>, BoxError>;
}
}

type MockSupergraphServiceType = tower_test::mock::Mock<
http_ext::Request<graphql::Request>,
http_ext::Response<Pin<Box<dyn Stream<Item = graphql::Response> + Send>>>,
http::Request<graphql::Request>,
http::Response<Pin<Box<dyn Stream<Item = graphql::Response> + Send>>>,
>;

#[derive(Clone)]
struct TestSupergraphServiceFactory {
inner: MockSupergraphServiceType,
}

impl NewService<Request<graphql::Request>> for TestSupergraphServiceFactory {
impl NewService<http::Request<graphql::Request>> for TestSupergraphServiceFactory {
type Service = MockSupergraphServiceType;

fn new_service(&self) -> Self::Service {
Expand All @@ -803,8 +802,8 @@ mod tests {
type SupergraphService = MockSupergraphServiceType;

type Future = <<TestSupergraphServiceFactory as NewService<
http_ext::Request<graphql::Request>,
>>::Service as Service<http_ext::Request<graphql::Request>>>::Future;
http::Request<graphql::Request>,
>>::Service as Service<http::Request<graphql::Request>>>::Future;

fn custom_endpoints(&self) -> HashMap<String, Handler> {
HashMap::new()
Expand Down Expand Up @@ -980,7 +979,7 @@ mod tests {
.times(2)
.returning(move |_req| {
let example_response = example_response.clone();
Ok(http_ext::Response::from_response_to_stream(
Ok(http_ext::from_response_to_stream(
http::Response::builder()
.status(200)
.body(example_response)
Expand Down Expand Up @@ -1071,7 +1070,7 @@ mod tests {
})
.returning(move |_req| {
let example_response = example_response.clone();
Ok(http_ext::Response::from_response_to_stream(
Ok(http_ext::from_response_to_stream(
http::Response::builder()
.status(200)
.body(example_response)
Expand Down Expand Up @@ -1133,7 +1132,7 @@ mod tests {
.times(2)
.returning(move |_| {
let example_response = example_response.clone();
Ok(http_ext::Response::from_response_to_stream(
Ok(http_ext::from_response_to_stream(
http::Response::builder()
.status(200)
.body(example_response)
Expand Down Expand Up @@ -1235,7 +1234,7 @@ mod tests {
.times(2)
.returning(move |_| {
let example_response = example_response.clone();
Ok(http_ext::Response::from_response_to_stream(
Ok(http_ext::from_response_to_stream(
http::Response::builder()
.status(200)
.body(example_response)
Expand Down Expand Up @@ -1304,7 +1303,7 @@ mod tests {
.times(2)
.returning(move |_| {
let example_response = example_response.clone();
Ok(http_ext::Response::from_response_to_stream(
Ok(http_ext::from_response_to_stream(
http::Response::builder()
.status(200)
.body(example_response)
Expand Down Expand Up @@ -1373,7 +1372,7 @@ mod tests {
.times(4)
.returning(move |_| {
let example_response = example_response.clone();
Ok(http_ext::Response::from_response_to_stream(
Ok(http_ext::from_response_to_stream(
http::Response::builder()
.status(200)
.body(example_response)
Expand Down Expand Up @@ -1465,7 +1464,7 @@ mod tests {
})
.returning(move |_| {
let example_response = example_response.clone();
Ok(http_ext::Response::from_response_to_stream(
Ok(http_ext::from_response_to_stream(
http::Response::builder()
.status(200)
.body(example_response)
Expand Down Expand Up @@ -1525,7 +1524,7 @@ mod tests {
})
.returning(move |_| {
let example_response = example_response.clone();
Ok(http_ext::Response::from_response_to_stream(
Ok(http_ext::from_response_to_stream(
http::Response::builder()
.status(200)
.body(example_response)
Expand Down Expand Up @@ -1564,7 +1563,7 @@ mod tests {
reason: "Mock error".to_string(),
}
.to_response();
Ok(http_ext::Response::from_response_to_stream(
Ok(http_ext::from_response_to_stream(
http::Response::builder()
.status(200)
.body(example_response)
Expand Down Expand Up @@ -1668,7 +1667,7 @@ mod tests {
.returning(move |_| {
let example_response = example_response.clone();

Ok(http_ext::Response::from_response_to_stream(
Ok(http_ext::from_response_to_stream(
http::Response::builder()
.status(200)
.body(example_response)
Expand Down Expand Up @@ -1940,7 +1939,7 @@ Content-Type: application/json\r
.expect_service_call()
.times(2)
.returning(move |req| {
Ok(http_ext::Response::from_response_to_stream(
Ok(http_ext::from_response_to_stream(
http::Response::builder()
.status(200)
.body(
Expand Down
2 changes: 1 addition & 1 deletion apollo-router/src/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::json_ext::Value;
/// Holds [`Context`] entries.
pub(crate) type Entries = Arc<DashMap<String, Value>>;

/// Context for a [`crate::http_ext::Request`]
/// A map of arbitrary JSON values, for use by plugins.
///
/// Context makes use of [`DashMap`] under the hood which tries to handle concurrency
/// by allowing concurrency across threads without requiring locking. This is great
Expand Down
Loading