Skip to content

Commit

Permalink
Further cleanup / fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
tustvold committed Jan 3, 2024
1 parent e6b3dba commit c5c6b6e
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 41 deletions.
78 changes: 38 additions & 40 deletions object_store/src/client/get.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use std::ops::Range;

use crate::client::header::{header_meta, HeaderConfig};
use crate::path::Path;
use crate::{GetOptions, GetRange, GetResult, GetResultPayload};
use crate::{GetOptions, GetRange, GetResult, GetResultPayload, Result};
use async_trait::async_trait;
use futures::{StreamExt, TryStreamExt};
use hyper::header::CONTENT_RANGE;
Expand All @@ -28,41 +28,6 @@ use reqwest::header::ToStrError;
use reqwest::Response;
use snafu::{ensure, OptionExt, ResultExt, Snafu};

/// A specialized `Error` for get-related errors
#[derive(Debug, Snafu)]
#[allow(missing_docs)]
pub(crate) enum Error {
#[snafu(context(false))]
Header {
source: crate::client::header::Error,
},

#[snafu(context(false))]
InvalidRangeRequest {
source: crate::util::InvalidGetRange,
},

#[snafu(display("Received non-partial response when range requested"))]
NotPartial,

#[snafu(display("Content-Range header not present"))]
NoContentRange,

#[snafu(display("Failed to parse value for CONTENT_RANGE header: {value}"))]
ParseContentRange { value: String },

#[snafu(display("Content-Range header contained non UTF-8 characters"))]
InvalidContentRange { source: ToStrError },

#[snafu(display("Requested {expected:?}, got {actual:?}"))]
UnexpectedRange {
expected: Range<usize>,
actual: Range<usize>,
},
}

pub(crate) type Result<T, E = Error> = std::result::Result<T, E>;

/// A client that can perform a get request
#[async_trait]
pub trait GetClient: Send + Sync + 'static {
Expand All @@ -71,18 +36,18 @@ pub trait GetClient: Send + Sync + 'static {
/// Configure the [`HeaderConfig`] for this client
const HEADER_CONFIG: HeaderConfig;

async fn get_request(&self, path: &Path, options: GetOptions) -> crate::Result<Response>;
async fn get_request(&self, path: &Path, options: GetOptions) -> Result<Response>;
}

/// Extension trait for [`GetClient`] that adds common retrieval functionality
#[async_trait]
pub trait GetClientExt {
async fn get_opts(&self, location: &Path, options: GetOptions) -> crate::Result<GetResult>;
async fn get_opts(&self, location: &Path, options: GetOptions) -> Result<GetResult>;
}

#[async_trait]
impl<T: GetClient> GetClientExt for T {
async fn get_opts(&self, location: &Path, options: GetOptions) -> crate::Result<GetResult> {
async fn get_opts(&self, location: &Path, options: GetOptions) -> Result<GetResult> {
let range = options.range.clone();
let response = self.get_request(location, options).await?;
get_result::<T>(location, range, response).map_err(|e| crate::Error::Generic {
Expand All @@ -102,11 +67,44 @@ fn parse_content_range(s: &str) -> Option<Range<usize>> {
Some(start..(end + 1))
}

/// A specialized `Error` for get-related errors
#[derive(Debug, Snafu)]
#[allow(missing_docs)]
enum GetResultError {
#[snafu(context(false))]
Header {
source: crate::client::header::Error,
},

#[snafu(context(false))]
InvalidRangeRequest {
source: crate::util::InvalidGetRange,
},

#[snafu(display("Received non-partial response when range requested"))]
NotPartial,

#[snafu(display("Content-Range header not present"))]
NoContentRange,

#[snafu(display("Failed to parse value for CONTENT_RANGE header: {value}"))]
ParseContentRange { value: String },

#[snafu(display("Content-Range header contained non UTF-8 characters"))]
InvalidContentRange { source: ToStrError },

#[snafu(display("Requested {expected:?}, got {actual:?}"))]
UnexpectedRange {
expected: Range<usize>,
actual: Range<usize>,
},
}

fn get_result<T: GetClient>(
location: &Path,
range: Option<GetRange>,
response: Response,
) -> Result<GetResult> {
) -> Result<GetResult, GetResultError> {
let meta = header_meta(location, response.headers(), T::HEADER_CONFIG)?;

// ensure that we receive the range we asked for
Expand Down
2 changes: 1 addition & 1 deletion object_store/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ mod tests {
#[test]
fn getrange_str() {
assert_eq!(GetRange::Offset(0).to_string(), "0-");
assert_eq!(GetRange::Bounded(10..19).to_string(), "10-20");
assert_eq!(GetRange::Bounded(10..19).to_string(), "10-18");
assert_eq!(GetRange::Suffix(10).to_string(), "-10");
}

Expand Down

0 comments on commit c5c6b6e

Please sign in to comment.