Skip to content

Commit

Permalink
refactor: Change presign to async for future refactor (apache#1900)
Browse files Browse the repository at this point in the history
* refactor: Change presign to async for future refactor

Signed-off-by: Xuanwo <[email protected]>

* Fix unit test

Signed-off-by: Xuanwo <[email protected]>

---------

Signed-off-by: Xuanwo <[email protected]>
  • Loading branch information
Xuanwo authored and wcy-fdu committed Apr 12, 2023
1 parent da9b42e commit d09da71
Show file tree
Hide file tree
Showing 13 changed files with 48 additions and 41 deletions.
8 changes: 5 additions & 3 deletions bindings/c/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,13 @@ use std::collections::HashMap;
use std::os::raw::c_char;
use std::str::FromStr;

use crate::types::{opendal_bytes, opendal_operator_ptr};

use ::opendal as od;
use error::opendal_code;
use result::{opendal_result_is_exist, opendal_result_read};
use result::opendal_result_is_exist;
use result::opendal_result_read;

use crate::types::opendal_bytes;
use crate::types::opendal_operator_ptr;

/// Returns a result type [`opendal_result_op`], with operator_ptr. If the construction succeeds
/// the error is nullptr, otherwise it contains the error information.
Expand Down
3 changes: 2 additions & 1 deletion bindings/c/src/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
//! "opendal_result_opendal_operator_ptr", which is unacceptable. Therefore,
//! we are defining all Result types here
use crate::{error::opendal_code, types::opendal_bytes};
use crate::error::opendal_code;
use crate::types::opendal_bytes;

/// The Rust-like Result type of opendal C binding, it contains
/// the data that the read operation returns and a error code
Expand Down
9 changes: 6 additions & 3 deletions bindings/nodejs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,10 +484,11 @@ impl Operator {
/// console.log("headers: ", req.headers);
/// ```
#[napi]
pub fn presign_read(&self, path: String, expires: u32) -> Result<PresignedRequest> {
pub async fn presign_read(&self, path: String, expires: u32) -> Result<PresignedRequest> {
let res = self
.0
.presign_read(&path, Duration::seconds(expires as i64))
.await
.map_err(format_napi_error)?;
Ok(PresignedRequest::new(res))
}
Expand All @@ -506,10 +507,11 @@ impl Operator {
/// console.log("headers: ", req.headers);
/// ```
#[napi]
pub fn presign_write(&self, path: String, expires: u32) -> Result<PresignedRequest> {
pub async fn presign_write(&self, path: String, expires: u32) -> Result<PresignedRequest> {
let res = self
.0
.presign_write(&path, Duration::seconds(expires as i64))
.await
.map_err(format_napi_error)?;
Ok(PresignedRequest::new(res))
}
Expand All @@ -528,10 +530,11 @@ impl Operator {
/// console.log("headers: ", req.headers);
/// ```
#[napi]
pub fn presign_stat(&self, path: String, expires: u32) -> Result<PresignedRequest> {
pub async fn presign_stat(&self, path: String, expires: u32) -> Result<PresignedRequest> {
let res = self
.0
.presign_stat(&path, Duration::seconds(expires as i64))
.await
.map_err(format_napi_error)?;
Ok(PresignedRequest::new(res))
}
Expand Down
4 changes: 2 additions & 2 deletions core/src/layers/error_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@ impl<A: Accessor> LayeredAccessor for ErrorContextAccessor<A> {
.await
}

fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
self.inner.presign(path, args).map_err(|err| {
async fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
self.inner.presign(path, args).await.map_err(|err| {
err.with_operation(Operation::Presign)
.with_context("service", self.meta.scheme())
.with_context("path", path)
Expand Down
3 changes: 2 additions & 1 deletion core/src/layers/logging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ impl<A: Accessor> LayeredAccessor for LoggingAccessor<A> {
.await
}

fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
async fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
debug!(
target: LOGGING_TARGET,
"service={} operation={} path={} -> started",
Expand All @@ -587,6 +587,7 @@ impl<A: Accessor> LayeredAccessor for LoggingAccessor<A> {

self.inner
.presign(path, args)
.await
.map(|v| {
debug!(
target: LOGGING_TARGET,
Expand Down
4 changes: 2 additions & 2 deletions core/src/layers/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -630,11 +630,11 @@ impl<A: Accessor> LayeredAccessor for MetricsAccessor<A> {
})
}

fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
async fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
self.handle.requests_total_presign.increment(1);

let start = Instant::now();
let result = self.inner.presign(path, args);
let result = self.inner.presign(path, args).await;
let dur = start.elapsed().as_secs_f64();

self.handle.requests_duration_seconds_presign.record(dur);
Expand Down
4 changes: 2 additions & 2 deletions core/src/layers/tracing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,8 @@ impl<A: Accessor> LayeredAccessor for TracingAccessor<A> {
}

#[tracing::instrument(level = "debug", skip(self))]
fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
self.inner.presign(path, args)
async fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
self.inner.presign(path, args).await
}

#[tracing::instrument(level = "debug", skip(self))]
Expand Down
6 changes: 3 additions & 3 deletions core/src/raw/accessor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ pub trait Accessor: Send + Sync + Debug + Unpin + 'static {
/// # Behavior
///
/// - This API is optional, return [`std::io::ErrorKind::Unsupported`] if not supported.
fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
async fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
let (_, _) = (path, args);

Err(Error::new(
Expand Down Expand Up @@ -453,8 +453,8 @@ impl<T: Accessor + ?Sized> Accessor for Arc<T> {
self.as_ref().batch(args).await
}

fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
self.as_ref().presign(path, args)
async fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
self.as_ref().presign(path, args).await
}

fn blocking_create(&self, path: &str, args: OpCreate) -> Result<RpCreate> {
Expand Down
8 changes: 4 additions & 4 deletions core/src/raw/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,8 @@ pub trait LayeredAccessor: Send + Sync + Debug + Unpin + 'static {
self.inner().batch(args).await
}

fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
self.inner().presign(path, args)
async fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
self.inner().presign(path, args).await
}

fn blocking_create(&self, path: &str, args: OpCreate) -> Result<RpCreate> {
Expand Down Expand Up @@ -269,8 +269,8 @@ impl<L: LayeredAccessor> Accessor for L {
(self as &L).batch(args).await
}

fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
(self as &L).presign(path, args)
async fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
(self as &L).presign(path, args).await
}

fn blocking_create(&self, path: &str, args: OpCreate) -> Result<RpCreate> {
Expand Down
2 changes: 1 addition & 1 deletion core/src/services/oss/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ impl Accessor for OssBackend {
))
}

fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
async fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
// We will not send this request out, just for signing.
let mut req = match args.operation() {
PresignOperation::Stat(_) => self.oss_head_object_request(path, true)?,
Expand Down
2 changes: 1 addition & 1 deletion core/src/services/s3/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1222,7 +1222,7 @@ impl Accessor for S3Backend {
))
}

fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
async fn presign(&self, path: &str, args: OpPresign) -> Result<RpPresign> {
// We will not send this request out, just for signing.
let mut req = match args.operation() {
PresignOperation::Stat(v) => self.s3_head_object_request(path, v.if_none_match())?,
Expand Down
30 changes: 15 additions & 15 deletions core/src/types/operator/operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1065,7 +1065,7 @@ impl Operator {
///
/// #[tokio::main]
/// async fn test(op: Operator) -> Result<()> {
/// let signed_req = op.presign_stat("test",Duration::hours(1))?;
/// let signed_req = op.presign_stat("test",Duration::hours(1)).await?;
/// let req = http::Request::builder()
/// .method(signed_req.method())
/// .uri(signed_req.uri())
Expand All @@ -1074,12 +1074,12 @@ impl Operator {
/// # Ok(())
/// # }
/// ```
pub fn presign_stat(&self, path: &str, expire: Duration) -> Result<PresignedRequest> {
pub async fn presign_stat(&self, path: &str, expire: Duration) -> Result<PresignedRequest> {
let path = normalize_path(path);

let op = OpPresign::new(OpStat::new(), expire);

let rp = self.inner().presign(&path, op)?;
let rp = self.inner().presign(&path, op).await?;
Ok(rp.into_presigned_request())
}

Expand All @@ -1095,7 +1095,7 @@ impl Operator {
///
/// #[tokio::main]
/// async fn test(op: Operator) -> Result<()> {
/// let signed_req = op.presign_read("test.txt", Duration::hours(1))?;
/// let signed_req = op.presign_read("test.txt", Duration::hours(1)).await?;
/// # Ok(())
/// # }
/// ```
Expand All @@ -1109,12 +1109,12 @@ impl Operator {
/// ```shell
/// curl "https://s3.amazonaws.com/examplebucket/test.txt?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=access_key_id/20130721/us-east-1/s3/aws4_request&X-Amz-Date=20130721T201207Z&X-Amz-Expires=86400&X-Amz-SignedHeaders=host&X-Amz-Signature=<signature-value>" -O /tmp/test.txt
/// ```
pub fn presign_read(&self, path: &str, expire: Duration) -> Result<PresignedRequest> {
pub async fn presign_read(&self, path: &str, expire: Duration) -> Result<PresignedRequest> {
let path = normalize_path(path);

let op = OpPresign::new(OpRead::new(), expire);

let rp = self.inner().presign(&path, op)?;
let rp = self.inner().presign(&path, op).await?;
Ok(rp.into_presigned_request())
}

Expand All @@ -1135,11 +1135,11 @@ impl Operator {
/// async fn test(op: Operator) -> Result<()> {
/// let args = OpRead::new()
/// .with_override_content_disposition("attachment; filename=\"othertext.txt\"");
/// let signed_req = op.presign_read_with("test.txt", args, Duration::hours(1))?;
/// let signed_req = op.presign_read_with("test.txt", args, Duration::hours(1)).await?;
/// # Ok(())
/// # }
/// ```
pub fn presign_read_with(
pub async fn presign_read_with(
&self,
path: &str,
op: OpRead,
Expand All @@ -1149,7 +1149,7 @@ impl Operator {

let op = OpPresign::new(op, expire);

let rp = self.inner().presign(&path, op)?;
let rp = self.inner().presign(&path, op).await?;
Ok(rp.into_presigned_request())
}

Expand All @@ -1165,7 +1165,7 @@ impl Operator {
///
/// #[tokio::main]
/// async fn test(op: Operator) -> Result<()> {
/// let signed_req = op.presign_write("test.txt", Duration::hours(1))?;
/// let signed_req = op.presign_write("test.txt", Duration::hours(1)).await?;
/// # Ok(())
/// # }
/// ```
Expand All @@ -1179,8 +1179,8 @@ impl Operator {
/// ```shell
/// curl -X PUT "https://s3.amazonaws.com/examplebucket/test.txt?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=access_key_id/20130721/us-east-1/s3/aws4_request&X-Amz-Date=20130721T201207Z&X-Amz-Expires=86400&X-Amz-SignedHeaders=host&X-Amz-Signature=<signature-value>" -d "Hello, World!"
/// ```
pub fn presign_write(&self, path: &str, expire: Duration) -> Result<PresignedRequest> {
self.presign_write_with(path, OpWrite::new(), expire)
pub async fn presign_write(&self, path: &str, expire: Duration) -> Result<PresignedRequest> {
self.presign_write_with(path, OpWrite::new(), expire).await
}

/// Presign an operation for write with option described in OpenDAL [rfc-0661](../../docs/rfcs/0661-path-in-accessor.md)
Expand All @@ -1199,7 +1199,7 @@ impl Operator {
/// #[tokio::main]
/// async fn test(op: Operator) -> Result<()> {
/// let args = OpWrite::new().with_content_type("text/csv");
/// let signed_req = op.presign_write_with("test", args, Duration::hours(1))?;
/// let signed_req = op.presign_write_with("test", args, Duration::hours(1)).await?;
/// let req = http::Request::builder()
/// .method(signed_req.method())
/// .uri(signed_req.uri())
Expand All @@ -1208,7 +1208,7 @@ impl Operator {
/// # Ok(())
/// # }
/// ```
pub fn presign_write_with(
pub async fn presign_write_with(
&self,
path: &str,
op: OpWrite,
Expand All @@ -1218,7 +1218,7 @@ impl Operator {

let op = OpPresign::new(op, expire);

let rp = self.inner().presign(&path, op)?;
let rp = self.inner().presign(&path, op).await?;
Ok(rp.into_presigned_request())
}
}
6 changes: 3 additions & 3 deletions core/tests/behavior/presign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ pub async fn test_presign_write(op: Operator) -> Result<()> {
debug!("Generate a random file: {}", &path);
let (content, size) = gen_bytes();

let signed_req = op.presign_write(&path, Duration::hours(1))?;
let signed_req = op.presign_write(&path, Duration::hours(1)).await?;
debug!("Generated request: {signed_req:?}");

let client = reqwest::Client::new();
Expand Down Expand Up @@ -118,7 +118,7 @@ pub async fn test_presign_stat(op: Operator) -> Result<()> {
op.write(&path, content.clone())
.await
.expect("write must succeed");
let signed_req = op.presign_stat(&path, Duration::hours(1))?;
let signed_req = op.presign_stat(&path, Duration::hours(1)).await?;
debug!("Generated request: {signed_req:?}");
let client = reqwest::Client::new();
let mut req = client.request(
Expand Down Expand Up @@ -150,7 +150,7 @@ pub async fn test_presign_read(op: Operator) -> Result<()> {
.await
.expect("write must succeed");

let signed_req = op.presign_read(&path, Duration::hours(1))?;
let signed_req = op.presign_read(&path, Duration::hours(1)).await?;
debug!("Generated request: {signed_req:?}");

let client = reqwest::Client::new();
Expand Down

0 comments on commit d09da71

Please sign in to comment.