Skip to content
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

Avoid processing incoming bodies unnecessarily #527

Merged
merged 4 commits into from
Oct 10, 2024
Merged

Avoid processing incoming bodies unnecessarily #527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5e728f7
Avoid processing incoming bodies unnecessarily
IvanUkhov Oct 10, 2024
4bd92a3
Disable the default features of hyper-rustls
IvanUkhov Oct 10, 2024
dca7fb6
Revisit the features of hyper-util
IvanUkhov Oct 10, 2024
3f85ed9
Split the CI jobs further
IvanUkhov Oct 10, 2024
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
49 changes: 30 additions & 19 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,22 @@ on:
branches: [main]

jobs:
meta-check:
name: Check the generative code
common-check:
name: Check the common code
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- run: make meta-check
- run: make common-check

check:
name: Check the generated code
common-test:
name: Test the common code
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- run: make common-test

api-check:
name: Check the generated APIs
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Expand All @@ -24,21 +31,11 @@ jobs:
make gen-all-api
make cargo-api ARGS=check
make cargo-api ARGS='check --no-default-features'

make gen-all-cli
make cargo-cli ARGS=check
env:
CI: true

meta-test:
name: Test the generative code
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- run: make meta-test

test:
name: Test the generated code
api-test:
name: Test the generated APIs
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Expand All @@ -49,8 +46,8 @@ jobs:
env:
CI: true

document:
name: Document the generated code
api-document:
name: Document the generated APIs
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Expand All @@ -62,3 +59,17 @@ jobs:
env:
CI: true
RUSTDOCFLAGS: -A warnings

cli-check:
name: Check the generated CLIs
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: Swatinem/rust-cache@v2
- run: |
make gen-all-api
make gen-all-cli
make cargo-cli ARGS=check
make cargo-cli ARGS='check --no-default-features'
env:
CI: true
12 changes: 6 additions & 6 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,20 +93,20 @@ license: LICENSE.md

regen-apis: | clean-all-api clean-all-cli gen-all-api gen-all-cli license

meta-test: meta-test-python meta-test-rust
common-test: common-test-python common-test-rust

meta-test-python: $(PYTHON_BIN)
common-test-python: $(PYTHON_BIN)
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 $(PYTEST) src

meta-test-rust:
common-test-rust:
cargo test

meta-check: meta-check-python meta-check-rust
common-check: common-check-python common-check-rust

meta-check-python: $(PYTHON_BIN)
common-check-python: $(PYTHON_BIN)
$(VENV_DIR)/bin/pre-commit run --all-files --show-diff-on-failure

meta-check-rust:
common-check-rust:
cargo clippy -- -D warnings

typecheck: $(PYTHON_BIN)
Expand Down
2 changes: 1 addition & 1 deletion google-apis-common/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ chrono = { version = "0.4", default-features = false, features = ["clock", "serd
http = "1"
http-body-util = "0.1"
hyper = { version = "1", features = ["client", "http2"] }
hyper-util = "0.1"
hyper-util = { version = "0.1", features = ["client-legacy", "http2"] }
Byron marked this conversation as resolved.
Show resolved Hide resolved
itertools = "0.13"
mime = "0.3"
percent-encoding = "2"
Expand Down
23 changes: 15 additions & 8 deletions google-apis-common/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use tokio::time::sleep;
const LINE_ENDING: &str = "\r\n";

/// A body.
pub type Body = http_body_util::Full<hyper::body::Bytes>;
pub type Body = http_body_util::combinators::BoxBody<hyper::body::Bytes, hyper::Error>;

/// A response.
pub type Response = hyper::Response<Body>;
Expand Down Expand Up @@ -618,7 +618,7 @@ where
.header_value(),
)
.header(AUTHORIZATION, self.auth_header.clone())
.body(Default::default())
.body(to_body::<String>(None))
.unwrap(),
)
.await
Expand All @@ -632,7 +632,7 @@ where
}
None | Some(_) => {
let (parts, body) = r.into_parts();
let body = Body::new(to_bytes(body).await.unwrap_or_default());
let body = to_body(to_bytes(body).await);
let response = Response::from_parts(parts, body);
if let Retry::After(d) = self.delegate.http_failure(&response, None) {
sleep(d).await;
Expand Down Expand Up @@ -704,7 +704,7 @@ where
.header("Content-Range", range_header.header_value())
.header(CONTENT_TYPE, format!("{}", self.media_type))
.header(USER_AGENT, self.user_agent.to_string())
.body(to_body(bytes))
.body(to_body(bytes.into()))
.unwrap(),
)
.await
Expand All @@ -724,7 +724,7 @@ where
} else {
None
};
let response = to_response(parts, bytes);
let response = to_response(parts, bytes.into());

if !success {
if let Retry::After(d) =
Expand Down Expand Up @@ -764,11 +764,18 @@ pub fn remove_json_null_values(value: &mut serde_json::value::Value) {
}

#[doc(hidden)]
pub fn to_body<T>(bytes: T) -> Body
pub fn to_body<T>(bytes: Option<T>) -> Body
where
T: Into<hyper::body::Bytes>,
{
Body::new(bytes.into())
use http_body_util::BodyExt;

fn falliable(_: std::convert::Infallible) -> hyper::Error {
unreachable!()
}

let bytes = bytes.map(Into::into).unwrap_or_default();
Body::new(http_body_util::Full::from(bytes).map_err(falliable))
}

#[doc(hidden)]
Expand All @@ -786,7 +793,7 @@ pub fn to_string(bytes: &hyper::body::Bytes) -> std::borrow::Cow<'_, str> {
}

#[doc(hidden)]
pub fn to_response<T>(parts: http::response::Parts, body: T) -> Response
pub fn to_response<T>(parts: http::response::Parts, body: Option<T>) -> Response
where
T: Into<hyper::body::Bytes>,
{
Expand Down
2 changes: 1 addition & 1 deletion src/generator/templates/Cargo.toml.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ clap = "2"
http-body-util = "0.1"
% endif
hyper = "1"
hyper-rustls = "0.27"
hyper-rustls = { version = "0.27", default-features = false }
IvanUkhov marked this conversation as resolved.
Show resolved Hide resolved
hyper-util = "0.1"
mime = "0.3"
serde = { version = "1", features = ["derive"] }
Expand Down
28 changes: 14 additions & 14 deletions src/generator/templates/api/lib/mbuild.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -739,13 +739,13 @@ else {
let request = req_builder
.header(CONTENT_TYPE, json_mime_type.to_string())
.header(CONTENT_LENGTH, request_size as u64)
.body(common::to_body(request_value_reader.get_ref().clone()))\
.body(common::to_body(request_value_reader.get_ref().clone().into()))\
% else:
let mut body_reader_bytes = vec![];
body_reader.read_to_end(&mut body_reader_bytes).unwrap();
let request = req_builder
.header(CONTENT_TYPE, content_type.to_string())
.body(common::to_body(body_reader_bytes))\
.body(common::to_body(body_reader_bytes.into()))\
% endif ## not simple_media_param
% else:
% if simple_media_param:
Expand All @@ -755,14 +755,14 @@ else {
reader.read_to_end(&mut bytes)?;
req_builder.header(CONTENT_TYPE, reader_mime_type.to_string())
.header(CONTENT_LENGTH, size)
.body(common::to_body(bytes))
.body(common::to_body(bytes.into()))
} else {
req_builder.body(Default::default())
req_builder.body(common::to_body::<String>(None))
}\
% else:
let request = req_builder
.header(CONTENT_LENGTH, 0_u64)
.body(Default::default())\
.body(common::to_body::<String>(None))\
% endif
% endif
;
Expand All @@ -783,10 +783,11 @@ else {
}
Ok(res) => {
let (mut parts, body) = res.into_parts();
let mut bytes = common::to_bytes(body).await.unwrap_or_default();
let mut body = common::Body::new(body);
if !parts.status.is_success() {
let bytes = common::to_bytes(body).await.unwrap_or_default();
let error = serde_json::from_str(&common::to_string(&bytes));
let response = common::to_response(parts, bytes);
let response = common::to_response(parts, bytes.into());

if let common::Retry::After(d) = dlg.http_failure(&response, error.as_ref().ok()) {
sleep(d).await;
Expand Down Expand Up @@ -836,14 +837,12 @@ else {
## Now the result contains the actual resource, if any ... it will be
## decoded next
Some(Ok(response)) => {
let parts_body = response.into_parts();
parts = parts_body.0;
bytes = common::to_bytes(parts_body.1).await.unwrap_or_default();
(parts, body) = response.into_parts();
if !parts.status.is_success() {
## delegate was called in upload() already - don't tell him again
dlg.store_upload_url(None);
${delegate_finish}(false);
return Err(common::Error::Failure(common::to_response(parts, bytes)));
return Err(common::Error::Failure(common::Response::from_parts(parts, body)));
}
}
}
Expand All @@ -856,9 +855,10 @@ else {
if enable_resource_parsing \
% endif
{
let bytes = common::to_bytes(body).await.unwrap_or_default();
let encoded = common::to_string(&bytes);
match serde_json::from_str(&encoded) {
Ok(decoded) => (common::to_response(parts, bytes), decoded),
Ok(decoded) => (common::to_response(parts, bytes.into()), decoded),
Err(error) => {
dlg.response_json_decode_error(&encoded, &error);
return Err(common::Error::JsonDecodeError(encoded.to_string(), error));
Expand All @@ -867,12 +867,12 @@ if enable_resource_parsing \
}\
% if supports_download:
else {
(common::to_response(parts, bytes), Default::default())
(common::Response::from_parts(parts, body), Default::default())
}\
% endif
;
% else:
let response = common::to_response(parts, bytes);
let response = common::Response::from_parts(parts, body);
% endif

${delegate_finish}(true);
Expand Down
Loading