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.

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

Misc small documentation and code readability fixes. #167

Merged
merged 3 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 0 additions & 5 deletions .github/workflows/cargo-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,9 @@ jobs:
rustup component add rustfmt --toolchain nightly

- name: Run rustfmt
# Delete the generated code so it doesn't get formatted.
# We'll be re-running the generator later, so this is okay.
run: |
rm -rf src/generated
echo "// empty module for rustfmt" > src/generated.rs
echo "// empty module for rustfmt" > tests/generated.rs
rustup run nightly cargo fmt --check
rm src/generated.rs
rm tests/generated.rs

- name: Set up Python
Expand Down
2 changes: 1 addition & 1 deletion src/async_client_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ impl<T: crate::client_trait::HttpClient + Sync> HttpClient for T {
pub trait NoauthClient: HttpClient {}

/// Marker trait to indicate that a HTTP client supports User authentication.
/// Team authentication works by adding a `Authorization: Bearer <TOKEN>` header.
/// User authentication works by adding a `Authorization: Bearer <TOKEN>` header.
pub trait UserAuthClient: HttpClient {}

/// Marker trait to indicate that a HTTP client supports Team authentication.
Expand Down
39 changes: 18 additions & 21 deletions src/client_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::sync::Arc;
/// When Dropbox returns an error with HTTP 409 or 429, it uses an implicit JSON object with the
/// following structure, which contains the actual error as a field.
#[derive(Debug, Deserialize)]
pub(crate) struct TopLevelError<T> {
struct TopLevelError<T> {
pub error: T,
// It also has these fields, which we don't expose anywhere:
//pub error_summary: String,
Expand All @@ -35,7 +35,7 @@ struct RateLimitedError {
}

#[allow(clippy::too_many_arguments)]
pub(crate) fn prepare_request<T: HttpClient>(
pub fn prepare_request<T: HttpClient>(
client: &T,
endpoint: Endpoint,
style: Style,
Expand Down Expand Up @@ -101,9 +101,7 @@ pub(crate) fn prepare_request<T: HttpClient>(
(req, params_body)
}

pub(crate) async fn body_to_string(
body: &mut (dyn AsyncRead + Send + Unpin),
) -> Result<String, Error> {
async fn body_to_string(body: &mut (dyn AsyncRead + Send + Unpin)) -> Result<String, Error> {
let mut s = String::new();
match body.read_to_string(&mut s).await {
Ok(_) => Ok(s),
Expand All @@ -117,10 +115,10 @@ pub(crate) async fn body_to_string(
}
}

/// Does the request and returns a two-level result. The outer result has an error if something
/// went wrong in the process of making the request (I/O errors, parse errors, server 500 errors,
/// etc.). The inner result has an error if the server returned one for the request, otherwise it
/// has the deserialized JSON response and the body stream (if any).
/// Does the request and returns a parsed result, including the response body, if any. If the
/// request is successful, the JSON response is parsed as a `TResponse`. If the result is a HTTP
/// 409 error, the response is parsed as the specified error type `TError` and returned as a
/// [`Error::Api`].
#[allow(clippy::too_many_arguments)]
pub async fn request_with_body<TResponse, TError, TParams, TClient>(
client: &TClient,
Expand Down Expand Up @@ -199,7 +197,7 @@ where

if status == 409 {
// Response should be JSON-deseraializable into the strongly-typed
// error specified by type parameter E.
// error specified by type parameter TError.
return match serde_json::from_str::<TopLevelError<TError>>(&json) {
Ok(deserialized) => {
error!("API error: {}", deserialized.error);
Expand All @@ -223,7 +221,7 @@ where
}
}

pub(crate) async fn parse_response(
pub async fn parse_response(
raw_resp: HttpRequestResultRaw,
style: Style,
) -> Result<
Expand Down Expand Up @@ -309,7 +307,7 @@ pub(crate) async fn parse_response(
}

#[derive(Debug, Clone)]
pub(crate) enum Body<'a> {
pub enum Body<'a> {
#[cfg(feature = "sync_routes")]
Borrowed(&'a [u8]),

Expand All @@ -332,6 +330,10 @@ impl<'a> From<&'a [u8]> for Body<'a> {
}
}

/// Does the request and returns a parsed result. If the request is successful, the JSON response
/// is parsed as a `TResponse`. If the result is a HTTP
/// 409 error, the response is parsed as the specified error type `TError` and returned as a
/// [`Error::Api`].
pub async fn request<TResponse, TError, TParams, TClient>(
client: &TClient,
endpoint: Endpoint,
Expand Down Expand Up @@ -364,9 +366,8 @@ mod sync_helpers {
///
/// This is ONLY safe if the result was created by a sync HttpClient, so we require it as an
/// argument just to be extra careful.
#[cfg(feature = "sync_routes")]
#[inline]
pub(crate) fn unwrap_async_result<T>(
fn unwrap_async_result<T>(
r: HttpRequestResult<T>,
_client: &impl sync::HttpClient,
) -> sync::HttpRequestResult<T> {
Expand All @@ -391,9 +392,8 @@ mod sync_helpers {
}
}

#[cfg(feature = "sync_routes")]
#[inline]
pub(crate) fn unwrap_async_body<T, E>(
pub fn unwrap_async_body<T, E>(
f: impl Future<Output = Result<HttpRequestResult<T>, Error<E>>>,
client: &impl sync::HttpClient,
) -> Result<sync::HttpRequestResult<T>, Error<E>> {
Expand All @@ -406,15 +406,12 @@ mod sync_helpers {
}
}

#[cfg(feature = "sync_routes")]
#[inline]
pub(crate) fn unwrap_async<T, E>(
f: impl Future<Output = Result<T, Error<E>>>,
) -> Result<T, Error<E>> {
pub fn unwrap_async<T, E>(f: impl Future<Output = Result<T, Error<E>>>) -> Result<T, Error<E>> {
f.now_or_never()
.expect("sync future should resolve immediately")
}
}

#[cfg(feature = "sync_routes")]
pub(crate) use sync_helpers::*;
pub use sync_helpers::*;
2 changes: 1 addition & 1 deletion src/client_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub trait HttpClient: Sync {
pub trait NoauthClient: HttpClient {}

/// Marker trait to indicate that a HTTP client supports User authentication.
/// Team authentication works by adding a `Authorization: Bearer <TOKEN>` header.
/// User authentication works by adding a `Authorization: Bearer <TOKEN>` header.
pub trait UserAuthClient: HttpClient {}

/// Marker trait to indicate that a HTTP client supports Team authentication.
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ mod client_helpers;
pub mod oauth2;

// You need to run the Stone generator to create this module.
#[rustfmt::skip]
mod generated;
pub use generated::*;

Expand Down
1 change: 1 addition & 0 deletions tests/generated_tests.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![warn(rust_2018_idioms)]

#[rustfmt::skip]
mod generated; // You need to run the Stone generator to create this module.
Loading