Skip to content

Commit

Permalink
Remove s3-server and dependencies (#150)
Browse files Browse the repository at this point in the history 
* fix(search) Bump up s3s-* crates as suggested in #131 (comment)
* test(search): fix tests related to improperly formatted `folder` in URLs for S3 mock filesystem
* fix(search): check that a key exists before formatting a URL, ensuring that pre-signed URLs are not generated for non-existent keys
* fix(search): use KeyNotFound error when a key is not found
* fix(search): use KeyNotFound for head and get object in AwsS3Storage
* test(search): add set of tests targeting non-existent keys
* build(search): remove unused dependencies

---------

Co-authored-by: Marko Malenic <[email protected]>
  • Loading branch information
@brainstorm @mmalenic
brainstorm and mmalenic authored Feb 6, 2023
1 parent 3ab16cf commit 766fbc0
Show file tree
Hide file tree
Showing 13 changed files with 629 additions and 233 deletions.
1 change: 0 additions & 1 deletion htsget-lambda/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ s3-storage = ["htsget-config/s3-storage", "htsget-search/s3-storage", "htsget-ht
default = ["s3-storage"]

[dependencies]
envy = "0.4"
tokio = { version = "1.24", features = ["macros", "rt-multi-thread"] }
tower-http = { version = "0.3", features = ["cors"] }
lambda_http = { version = "0.7" }
Expand Down
15 changes: 9 additions & 6 deletions htsget-search/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ noodles-vcf = { version = "0.23", features = ["async"] }

# Amazon S3
bytes = { version = "1.2", optional = true }
aws-sdk-s3 = { version = "0.22", optional = true }
aws-config = { version = "0.52", optional = true }
aws-sdk-s3 = { version = "0.24", optional = true, features = ["test-util"] }
aws-config = { version = "0.54", optional = true }

# Error control, tracing, config
thiserror = "1.0"
Expand All @@ -55,13 +55,16 @@ serde = "1.0"
htsget-test = { path = "../htsget-test", features = ["cors-tests"], default-features = false }
tempfile = "3.3"
data-url = "0.2"
once_cell = "1.17"

# Aws S3 storage dependencies.
# Aws S3 storage.
anyhow = "1.0"
s3-server = "0.2"
aws-types = { version = "0.52", features = ["hardcoded-credentials"] }
aws-credential-types = { version = "0.54", features = ["test-util"] }
s3s = { version = "0.3" }
s3s-fs = { version = "0.3" }
s3s-aws = { version = "0.3" }

# Axum server dependencies
# Axum server
reqwest = { version = "0.11", default-features = false, features = ["rustls-tls"] }

criterion = { version = "0.4", features = ["async_tokio"] }
Expand Down
168 changes: 127 additions & 41 deletions htsget-search/src/htsget/bam_search.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,16 +178,18 @@ pub(crate) mod tests {

use htsget_test::util::expected_bgzf_eof_data_url;

use crate::htsget::from_storage::tests::{
with_local_storage as with_local_storage_path,
with_local_storage_tmp as with_local_storage_tmp_path,
};
use crate::htsget::{Class::Body, Class::Header, Headers, Response, Url};
#[cfg(feature = "s3-storage")]
use crate::htsget::from_storage::tests::with_aws_storage_fn;
use crate::htsget::from_storage::tests::with_local_storage_fn;
use crate::htsget::{Class::Body, Class::Header, Headers, HtsGetError::NotFound, Response, Url};
use crate::storage::data_server::HttpTicketFormatter;
use crate::storage::local::LocalStorage;

use super::*;

const DATA_LOCATION: &str = "data/bam";
const INDEX_FILE_LOCATION: &str = "htsnexus_test_NA12878.bam.bai";

#[tokio::test]
async fn search_all_reads() {
with_local_storage(|storage| async move {
Expand Down Expand Up @@ -354,29 +356,33 @@ pub(crate) mod tests {

#[tokio::test]
async fn search_no_gzi() {
with_local_storage_tmp(|storage| async move {
let search = BamSearch::new(storage.clone());
let query = Query::new("htsnexus_test_NA12878", Format::Bam)
.with_reference_name("11")
.with_start(5015000)
.with_end(5050000);
let response = search.search(query).await;
println!("{response:#?}");

let expected_response = Ok(Response::new(
Format::Bam,
vec![
Url::new(expected_url())
.with_headers(Headers::default().with_header("Range", "bytes=0-4667"))
.with_class(Header),
Url::new(expected_url())
.with_headers(Headers::default().with_header("Range", "bytes=256721-1065951"))
.with_class(Body),
Url::new(expected_bgzf_eof_data_url()).with_class(Body),
],
));
assert_eq!(response, expected_response)
})
with_local_storage_fn(
|storage| async move {
let search = BamSearch::new(storage.clone());
let query = Query::new("htsnexus_test_NA12878", Format::Bam)
.with_reference_name("11")
.with_start(5015000)
.with_end(5050000);
let response = search.search(query).await;
println!("{response:#?}");

let expected_response = Ok(Response::new(
Format::Bam,
vec![
Url::new(expected_url())
.with_headers(Headers::default().with_header("Range", "bytes=0-4667"))
.with_class(Header),
Url::new(expected_url())
.with_headers(Headers::default().with_header("Range", "bytes=256721-1065951"))
.with_class(Body),
Url::new(expected_bgzf_eof_data_url()).with_class(Body),
],
));
assert_eq!(response, expected_response)
},
DATA_LOCATION,
&["htsnexus_test_NA12878.bam", INDEX_FILE_LOCATION],
)
.await
}

Expand All @@ -399,25 +405,105 @@ pub(crate) mod tests {
.await;
}

pub(crate) async fn with_local_storage<F, Fut>(test: F)
where
F: FnOnce(Arc<LocalStorage<HttpTicketFormatter>>) -> Fut,
Fut: Future<Output = ()>,
{
with_local_storage_path(test, "data/bam").await
#[tokio::test]
async fn search_non_existent_id_reference_name() {
with_local_storage_fn(
|storage| async move {
let search = BamSearch::new(storage.clone());
let query = Query::new("htsnexus_test_NA12878", Format::Bam);
let response = search.search(query).await;
assert!(matches!(response, Err(NotFound(_))));
},
DATA_LOCATION,
&[INDEX_FILE_LOCATION],
)
.await
}

#[tokio::test]
async fn search_non_existent_id_all_reads() {
with_local_storage_fn(
|storage| async move {
let search = BamSearch::new(storage.clone());
let query = Query::new("htsnexus_test_NA12878", Format::Bam).with_reference_name("20");
let response = search.search(query).await;
assert!(matches!(response, Err(NotFound(_))));
},
DATA_LOCATION,
&[INDEX_FILE_LOCATION],
)
.await
}

async fn with_local_storage_tmp<F, Fut>(test: F)
#[tokio::test]
async fn search_non_existent_id_header() {
with_local_storage_fn(
|storage| async move {
let search = BamSearch::new(storage.clone());
let query = Query::new("htsnexus_test_NA12878", Format::Bam).with_class(Header);
let response = search.search(query).await;
assert!(matches!(response, Err(NotFound(_))));
},
DATA_LOCATION,
&[INDEX_FILE_LOCATION],
)
.await
}

#[cfg(feature = "s3-storage")]
#[tokio::test]
async fn search_non_existent_id_reference_name_aws() {
with_aws_storage_fn(
|storage| async move {
let search = BamSearch::new(storage);
let query = Query::new("htsnexus_test_NA12878", Format::Bam);
let response = search.search(query).await;
assert!(matches!(response, Err(_)));
},
DATA_LOCATION,
&[INDEX_FILE_LOCATION],
)
.await
}

#[cfg(feature = "s3-storage")]
#[tokio::test]
async fn search_non_existent_id_all_reads_aws() {
with_aws_storage_fn(
|storage| async move {
let search = BamSearch::new(storage);
let query = Query::new("htsnexus_test_NA12878", Format::Bam).with_reference_name("20");
let response = search.search(query).await;
assert!(matches!(response, Err(_)));
},
DATA_LOCATION,
&[INDEX_FILE_LOCATION],
)
.await
}

#[cfg(feature = "s3-storage")]
#[tokio::test]
async fn search_non_existent_id_header_aws() {
with_aws_storage_fn(
|storage| async move {
let search = BamSearch::new(storage);
let query = Query::new("htsnexus_test_NA12878", Format::Bam).with_class(Header);
let response = search.search(query).await;
assert!(matches!(response, Err(_)));
},
DATA_LOCATION,
&[INDEX_FILE_LOCATION],
)
.await
}

pub(crate) async fn with_local_storage<F, Fut>(test: F)
where
F: FnOnce(Arc<LocalStorage<HttpTicketFormatter>>) -> Fut,
Fut: Future<Output = ()>,
{
with_local_storage_tmp_path(
test,
"data/bam",
&["htsnexus_test_NA12878.bam", "htsnexus_test_NA12878.bam.bai"],
)
.await
with_local_storage_fn(test, DATA_LOCATION, &[]).await
}

pub(crate) fn expected_url() -> String {
Expand Down
Loading

0 comments on commit 766fbc0

Please sign in to comment.