Skip to content
Open
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
43 changes: 39 additions & 4 deletions core/services/s3/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -721,10 +721,8 @@ impl S3Builder {
}
}

impl Builder for S3Builder {
type Config = S3Config;

fn build(self) -> Result<impl Service> {
impl S3Builder {
fn build_backend(self) -> Result<S3Backend> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed to test the constructor in a UT, but the construction was not separate, it was happening inside impl Builder, which returned Service, not S3Backend.

Doing this allowed me to test on S3Backend itself w/o duplicating much code.

debug!("backend build started: {:?}", &self);

let S3Builder {
Expand Down Expand Up @@ -1032,6 +1030,14 @@ impl Builder for S3Builder {
}
}

impl Builder for S3Builder {
type Config = S3Config;

fn build(self) -> Result<impl Service> {
self.build_backend()
}
}

/// Backend for s3 services.
#[derive(Debug, Clone)]
pub struct S3Backend {
Expand Down Expand Up @@ -1359,4 +1365,33 @@ mod tests {
"application/json"
);
}

#[test]
fn test_s3_copy_object_request_sets_content_length_zero() -> Result<()> {
let backend = S3Builder::default()
.bucket("test")
.region("us-east-1")
.skip_signature()
.disable_config_load()
.disable_ec2_metadata()
.build_backend()?;

let req = backend
.core
.s3_copy_object_request("from.txt", "to.txt", &OpCopy::default())?;

assert_eq!(req.method(), http::Method::PUT);
assert!(req.uri().to_string().ends_with("/test/to.txt"));
assert_eq!(
req.headers().get(http::header::CONTENT_LENGTH).unwrap(),
"0"
);
assert_eq!(
req.headers().get(constants::X_AMZ_COPY_SOURCE).unwrap(),
"test/from.txt"
);
assert!(req.body().is_empty());

Ok(())
}
}
19 changes: 16 additions & 3 deletions core/services/s3/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -690,13 +690,12 @@ impl S3Core {
self.send(ctx, req).await
}

pub async fn s3_copy_object(
pub fn s3_copy_object_request(
&self,
ctx: &OperationContext,
from: &str,
to: &str,
args: &OpCopy,
) -> Result<Response<Buffer>> {
) -> Result<Request<Buffer>> {
let from = build_abs_path(&self.root, from);
let to = build_abs_path(&self.root, to);

Expand Down Expand Up @@ -769,10 +768,24 @@ impl S3Core {
// Inject operation to the request.
.extension(Operation::Copy)
.extension(ServiceOperation("CopyObject"))
// AWS S3 doesn't care, but some other S3-compat providers
// require a mandatory `content-length` header to work
.header(CONTENT_LENGTH, 0)
.header(constants::X_AMZ_COPY_SOURCE, &source)
.body(Buffer::new())
.map_err(new_request_build_error)?;

Ok(req)
}

pub async fn s3_copy_object(
&self,
ctx: &OperationContext,
from: &str,
to: &str,
args: &OpCopy,
) -> Result<Response<Buffer>> {
let req = self.s3_copy_object_request(from, to, args)?;
self.send(ctx, req).await
}

Expand Down
Loading