From 5e728f7ed8a477f28d59e2c8bf25044f9f37c423 Mon Sep 17 00:00:00 2001 From: Ivan Ukhov Date: Thu, 10 Oct 2024 13:32:13 +0200 Subject: [PATCH 1/4] Avoid processing incoming bodies unnecessarily --- google-apis-common/src/lib.rs | 23 +++++++++++------ src/generator/templates/api/lib/mbuild.mako | 28 ++++++++++----------- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/google-apis-common/src/lib.rs b/google-apis-common/src/lib.rs index cefc603be8..1321798d17 100644 --- a/google-apis-common/src/lib.rs +++ b/google-apis-common/src/lib.rs @@ -20,7 +20,7 @@ use tokio::time::sleep; const LINE_ENDING: &str = "\r\n"; /// A body. -pub type Body = http_body_util::Full; +pub type Body = http_body_util::combinators::BoxBody; /// A response. pub type Response = hyper::Response; @@ -618,7 +618,7 @@ where .header_value(), ) .header(AUTHORIZATION, self.auth_header.clone()) - .body(Default::default()) + .body(to_body::(None)) .unwrap(), ) .await @@ -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; @@ -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 @@ -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) = @@ -764,11 +764,18 @@ pub fn remove_json_null_values(value: &mut serde_json::value::Value) { } #[doc(hidden)] -pub fn to_body(bytes: T) -> Body +pub fn to_body(bytes: Option) -> Body where T: Into, { - 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)] @@ -786,7 +793,7 @@ pub fn to_string(bytes: &hyper::body::Bytes) -> std::borrow::Cow<'_, str> { } #[doc(hidden)] -pub fn to_response(parts: http::response::Parts, body: T) -> Response +pub fn to_response(parts: http::response::Parts, body: Option) -> Response where T: Into, { diff --git a/src/generator/templates/api/lib/mbuild.mako b/src/generator/templates/api/lib/mbuild.mako index 7b6db83f62..4c25761624 100644 --- a/src/generator/templates/api/lib/mbuild.mako +++ b/src/generator/templates/api/lib/mbuild.mako @@ -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: @@ -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::(None)) }\ % else: let request = req_builder .header(CONTENT_LENGTH, 0_u64) - .body(Default::default())\ + .body(common::to_body::(None))\ % endif % endif ; @@ -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; @@ -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))); } } } @@ -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)); @@ -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); From 4bd92a378deded72dba04838df58f0e420d2d850 Mon Sep 17 00:00:00 2001 From: Ivan Ukhov Date: Thu, 10 Oct 2024 15:53:31 +0200 Subject: [PATCH 2/4] Disable the default features of hyper-rustls --- src/generator/templates/Cargo.toml.mako | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/generator/templates/Cargo.toml.mako b/src/generator/templates/Cargo.toml.mako index 5e54d57f64..85c98c1941 100644 --- a/src/generator/templates/Cargo.toml.mako +++ b/src/generator/templates/Cargo.toml.mako @@ -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 } hyper-util = "0.1" mime = "0.3" serde = { version = "1", features = ["derive"] } From dca7fb659dc6152fd3d06cd7cbd7fe3fdd71e1c9 Mon Sep 17 00:00:00 2001 From: Ivan Ukhov Date: Thu, 10 Oct 2024 21:18:50 +0200 Subject: [PATCH 3/4] Revisit the features of hyper-util --- google-apis-common/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-apis-common/Cargo.toml b/google-apis-common/Cargo.toml index 09a3c77ae5..947a556c91 100644 --- a/google-apis-common/Cargo.toml +++ b/google-apis-common/Cargo.toml @@ -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"] } itertools = "0.13" mime = "0.3" percent-encoding = "2" From 3f85ed9301061d264972474c7602392349585724 Mon Sep 17 00:00:00 2001 From: Ivan Ukhov Date: Thu, 10 Oct 2024 21:50:23 +0200 Subject: [PATCH 4/4] Split the CI jobs further --- .github/workflows/ci.yml | 49 ++++++++++++++++++++++++---------------- Makefile | 12 +++++----- 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 26635ed76c..c646809672 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/Makefile b/Makefile index 5e092e5bae..487fca9480 100644 --- a/Makefile +++ b/Makefile @@ -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)