From c8d624ca9ded9f8606331dd2d0060dc713bab3e3 Mon Sep 17 00:00:00 2001 From: Marko Malenic Date: Mon, 23 Sep 2024 13:29:14 +1000 Subject: [PATCH 1/5] refactor(config)!: remove object type and specify keys directly --- htsget-config/examples/config-files/c4gh.toml | 3 +- htsget-config/src/storage/local.rs | 1 + htsget-config/src/storage/mod.rs | 2 +- htsget-config/src/storage/object/c4gh.rs | 50 +++++++++++++++++++ htsget-config/src/storage/object/mod.rs | 19 +++---- htsget-storage/src/lib.rs | 11 ++-- 6 files changed, 68 insertions(+), 18 deletions(-) diff --git a/htsget-config/examples/config-files/c4gh.toml b/htsget-config/examples/config-files/c4gh.toml index c5553a70d..eb05b26aa 100644 --- a/htsget-config/examples/config-files/c4gh.toml +++ b/htsget-config/examples/config-files/c4gh.toml @@ -9,4 +9,5 @@ regex = ".*" substitution_string = "$0" [resolvers.storage] -object_type = { private_key = "data/c4gh/keys/bob.sec", recipient_public_key = "data/c4gh/keys/alice.pub" } # pragma: allowlist secret +private_key = "data/c4gh/keys/bob.sec" # pragma: allowlist secret +recipient_public_key = "data/c4gh/keys/alice.pub" diff --git a/htsget-config/src/storage/local.rs b/htsget-config/src/storage/local.rs index d35ceac3d..134fbb7bf 100644 --- a/htsget-config/src/storage/local.rs +++ b/htsget-config/src/storage/local.rs @@ -24,6 +24,7 @@ pub struct LocalStorage { authority: Authority, local_path: String, path_prefix: String, + #[serde(flatten)] object_type: ObjectType, } diff --git a/htsget-config/src/storage/mod.rs b/htsget-config/src/storage/mod.rs index d0c57da4a..fc1be3ad9 100644 --- a/htsget-config/src/storage/mod.rs +++ b/htsget-config/src/storage/mod.rs @@ -49,7 +49,7 @@ impl ResolvedId { /// Specify the storage backend to use as config values. #[derive(Serialize, Deserialize, Debug, Clone)] -#[serde(untagged, deny_unknown_fields)] +#[serde(untagged)] #[non_exhaustive] pub enum Storage { Tagged(TaggedStorageTypes), diff --git a/htsget-config/src/storage/object/c4gh.rs b/htsget-config/src/storage/object/c4gh.rs index a5b20a855..f7ec020cb 100644 --- a/htsget-config/src/storage/object/c4gh.rs +++ b/htsget-config/src/storage/object/c4gh.rs @@ -60,3 +60,53 @@ impl From for Error { ParseError(err.to_string()) } } + +#[cfg(test)] +mod tests { + use crate::config::tests::test_config_from_file; + use crate::storage::Storage; + use std::fs::copy; + use std::path::PathBuf; + use tempfile::TempDir; + + #[test] + fn config_storage_c4gh() { + let tmp = TempDir::new().unwrap(); + let private_key = tmp.path().join("bob.sec"); + let recipient_public_key = tmp.path().join("alice.pub"); + + let parent = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .parent() + .unwrap() + .to_path_buf(); + + copy(parent.join("data/c4gh/keys/bob.sec"), &private_key).unwrap(); + copy( + parent.join("data/c4gh/keys/alice.pub"), + &recipient_public_key, + ) + .unwrap(); + + test_config_from_file( + &format!( + r#" + [[resolvers]] + regex = "regex" + + [resolvers.storage] + private_key = "{}" + recipient_public_key = "{}" + "#, + private_key.to_string_lossy(), + recipient_public_key.to_string_lossy() + ), + |config| { + println!("{:?}", config.resolvers().first().unwrap().storage()); + assert!(matches!( + config.resolvers().first().unwrap().storage(), + Storage::Local { local_storage } if local_storage.object_type().keys().is_some() + )); + }, + ); + } +} diff --git a/htsget-config/src/storage/object/mod.rs b/htsget-config/src/storage/object/mod.rs index 409c71c6f..4fdd80485 100644 --- a/htsget-config/src/storage/object/mod.rs +++ b/htsget-config/src/storage/object/mod.rs @@ -10,14 +10,15 @@ use serde::{Deserialize, Serialize}; /// An object type, can be regular or Crypt4GH encrypted. #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq)] -#[serde(untagged, deny_unknown_fields)] -#[non_exhaustive] -pub enum ObjectType { - #[default] - Regular, +pub struct ObjectType { + #[serde(skip_serializing, flatten)] #[cfg(feature = "experimental")] - C4GH { - #[serde(flatten, skip_serializing)] - keys: C4GHKeys, - }, + keys: Option, +} + +impl ObjectType { + /// Get the C4GH keys. + pub fn keys(&self) -> Option<&C4GHKeys> { + self.keys.as_ref() + } } diff --git a/htsget-storage/src/lib.rs b/htsget-storage/src/lib.rs index 0c4d0714b..de436f403 100644 --- a/htsget-storage/src/lib.rs +++ b/htsget-storage/src/lib.rs @@ -35,7 +35,6 @@ use crate::s3::S3Storage; use crate::types::{BytesPositionOptions, DataBlock, GetOptions, HeadOptions, RangeUrlOptions}; #[cfg(feature = "url-storage")] use crate::url::UrlStorage; -use htsget_config::storage::object::ObjectType; use htsget_config::types::Scheme; #[cfg(feature = "experimental")] @@ -132,16 +131,14 @@ impl Storage { /// Create from local storage config. pub async fn from_local(config: &LocalStorageConfig) -> Result { let storage = LocalStorage::new(config.local_path(), config.clone())?; - match config.object_type() { - ObjectType::Regular => Ok(Storage::new(storage)), + + match config.object_type().keys() { + None => Ok(Storage::new(storage)), #[cfg(feature = "experimental")] - ObjectType::C4GH { keys } => Ok(Storage::new(C4GHStorage::new( + Some(keys) => Ok(Storage::new(C4GHStorage::new( keys.clone().into_inner(), storage, ))), - _ => Err(StorageError::InternalError( - "invalid object type".to_string(), - )), } } From 415d68772cd49eefb31d38fa06c9bba5217de77a Mon Sep 17 00:00:00 2001 From: Marko Malenic Date: Tue, 24 Sep 2024 07:42:00 +1000 Subject: [PATCH 2/5] refactor!: make storage config more explicit --- Cargo.lock | 1 + deploy/config/dev_umccr.toml | 16 +-- deploy/config/example_deploy.toml | 2 +- deploy/config/prod_umccr.toml | 6 +- deploy/config/public_umccr.toml | 4 +- htsget-config/README.md | 50 +++++---- htsget-config/examples/config-files/c4gh.toml | 2 + .../examples/config-files/s3_storage.toml | 3 +- .../config-files/tls_data_server.toml | 4 + .../config-files/tls_ticket_server.toml | 1 + .../examples/config-files/url_storage.toml | 1 + htsget-config/src/config/mod.rs | 6 +- htsget-config/src/resolver.rs | 21 ++-- htsget-config/src/storage/local.rs | 28 +++-- htsget-config/src/storage/mod.rs | 103 ++++++++---------- htsget-config/src/storage/object/c4gh.rs | 3 +- htsget-config/src/storage/object/mod.rs | 1 + htsget-config/src/storage/s3.rs | 9 +- htsget-config/src/storage/url.rs | 3 +- htsget-http/src/lib.rs | 1 + htsget-search/benches/search_benchmarks.rs | 1 + htsget-search/src/from_storage.rs | 3 +- htsget-storage/Cargo.toml | 1 + htsget-storage/src/c4gh/storage.rs | 7 +- htsget-storage/src/lib.rs | 64 +++++++---- htsget-storage/src/local.rs | 1 + htsget-test/src/http/mod.rs | 7 +- 27 files changed, 200 insertions(+), 149 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7feb1a5c6..fafcba168 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2370,6 +2370,7 @@ dependencies = [ "base64 0.22.1", "bincode", "bytes", + "cfg-if", "criterion", "crypt4gh", "data-url", diff --git a/deploy/config/dev_umccr.toml b/deploy/config/dev_umccr.toml index b92035245..a64c8449e 100644 --- a/deploy/config/dev_umccr.toml +++ b/deploy/config/dev_umccr.toml @@ -26,39 +26,39 @@ environment = "dev" [[resolvers]] regex = '^(org.umccr.dev.htsget-rs-test-data)/(?P.*)$' substitution_string = '$key' -storage = 'S3' +storage.type = 'S3' [[resolvers]] regex = '^(umccr-10c-data-dev)/(?P.*)$' substitution_string = '$key' -storage = 'S3' +storage.type = 'S3' [[resolvers]] regex = '^(umccr-10f-data-dev)/(?P.*)$' substitution_string = '$key' -storage = 'S3' +storage.type = 'S3' [[resolvers]] regex = '^(umccr-10g-data-dev)/(?P.*)$' substitution_string = '$key' -storage = 'S3' +storage.type = 'S3' [[resolvers]] regex = '^(umccr-agha-test-dev)/(?P.*)$' substitution_string = '$key' -storage = 'S3' +storage.type = 'S3' [[resolvers]] regex = '^(umccr-research-dev)/(?P.*)$' substitution_string = '$key' -storage = 'S3' +storage.type = 'S3' [[resolvers]] regex = '^(umccr-primary-data-dev)/(?P.*)$' substitution_string = '$key' -storage = 'S3' +storage.type = 'S3' [[resolvers]] regex = '^(umccr-validation-prod)/(?P.*)$' substitution_string = '$key' -storage = 'S3' +storage.type = 'S3' diff --git a/deploy/config/example_deploy.toml b/deploy/config/example_deploy.toml index 6a487b47c..f4cb5d377 100644 --- a/deploy/config/example_deploy.toml +++ b/deploy/config/example_deploy.toml @@ -17,4 +17,4 @@ environment = "dev" [[resolvers]] regex = '^(?P.*?)/(?P.*)$' substitution_string = '$key' -storage = 'S3' \ No newline at end of file +storage.type = 'S3' \ No newline at end of file diff --git a/deploy/config/prod_umccr.toml b/deploy/config/prod_umccr.toml index 41f686544..2a164a38f 100644 --- a/deploy/config/prod_umccr.toml +++ b/deploy/config/prod_umccr.toml @@ -22,14 +22,14 @@ environment = "prod" [[resolvers]] regex = '^(umccr-research-dev)/(?P.*)$' substitution_string = '$key' -storage = 'S3' +storage.type = 'S3' [[resolvers]] regex = '^(umccr-validation-prod)/(?P.*)$' substitution_string = '$key' -storage = 'S3' +storage.type = 'S3' [[resolvers]] regex = '^(umccr-primary-data-prod)/(?P.*)$' substitution_string = '$key' -storage = 'S3' +storage.type = 'S3' diff --git a/deploy/config/public_umccr.toml b/deploy/config/public_umccr.toml index 0ecd657d8..4f388e92a 100644 --- a/deploy/config/public_umccr.toml +++ b/deploy/config/public_umccr.toml @@ -11,9 +11,9 @@ environment = 'public' [[resolvers]] regex = '^(org.umccr.demo.sbeacon-data)/CINECA_UK1/(?P.*)$' substitution_string = 'CINECA_UK1/$key' -storage = 'S3' +storage.type = 'S3' [[resolvers]] regex = '^(org.umccr.demo.htsget-rs-data)/(?Pbam|cram|vcf|bcf|crypt4gh|mixed)/(?P.*)$' substitution_string = '$type/$key' -storage = 'S3' +storage.type = 'S3' diff --git a/htsget-config/README.md b/htsget-config/README.md index 5188eeb98..24d4e7cf1 100644 --- a/htsget-config/README.md +++ b/htsget-config/README.md @@ -151,33 +151,34 @@ For more information about regex options see the [regex crate](https://docs.rs/r Each resolver also maps to a certain storage backend. This storage backend can be used to set query IDs which are served from local storage, from S3-style bucket storage, or from HTTP URLs. To set the storage backend for a resolver, add a `[resolvers.storage]` table. Some storage backends require feature flags to be set when compiling htsget-rs. -To use `LocalStorage`, set `storage = 'Local'`. This will derive the values for the fields below from the `data_server` config: +To use `LocalStorage`, set `type = 'Local'` under `[resolvers.storage]`, and specify any additional options from below: -| Option | Description | When `storage = 'Local'` | Type | Default | -|---------------------|-------------------------------------------------------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------|------------------------------|--------------------| -| `scheme` | The scheme present on URL tickets. | Derived from `data_server_key` and `data_server_cert`. If no key and cert are present, then uses `Http`, otherwise uses `Https`. | Either `'Http'` or `'Https'` | `'Http'` | -| `authority` | The authority present on URL tickets. This should likely match the `data_server_addr`. | Same as `data_server_addr`. | URL authority | `'127.0.0.1:8081'` | -| `local_path` | The local filesystem path which the data server uses to respond to tickets. This should likely match the `data_server_local_path`. | Same as `data_server_local_path`. | Filesystem path | `'./'` | -| `path_prefix` | The path prefix which the URL tickets will have. This should likely match the `data_server_serve_at` path. | Same as `data_server_serve_at`. | URL path | `''` | +| Option | Description | Type | Default | +|--------------------------|-------------------------------------------------------------------------------------------------------------------------------------|------------------------------|--------------------| +| `scheme` | The scheme present on URL tickets. | Either `'Http'` or `'Https'` | `'Http'` | +| `authority` | The authority present on URL tickets. This should likely match the `data_server_addr`. | URL authority | `'127.0.0.1:8081'` | +| `local_path` | The local filesystem path which the data server uses to respond to tickets. This should likely match the `data_server_local_path`. | Filesystem path | `'./'` | +| `path_prefix` | The path prefix which the URL tickets will have. This should likely match the `data_server_serve_at` path. | URL path | `''` | +| `use_data_server_config` | Whether to use the data server config to fill in the above values. This overrides any other options specified from this table. | Boolean | `false` | -To use `S3Storage`, build htsget-rs with the `s3-storage` feature enabled, and set `storage = 'S3'`. This will derive the value for `bucket` from the `regex` component of the `resolvers`: +To use `S3Storage`, build htsget-rs with the `s3-storage` feature enabled, set `type = 'S3'` under `[resolvers.storage]`, and specify any additional options from below: -| Option | Description | When `storage = 'S3'` | Type | Default | -|--------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------| -| `bucket` | The AWS S3 bucket where resources can be retrieved from. | Derived from the `resolvers` `regex` property. This uses the first capture group in the `regex` as the `bucket`. | String | `''` | -| `endpoint` | A custom endpoint to override the default S3 service address. This is useful for using S3 locally or with storage backends such as MinIO. See [MinIO](#minio). | Not set, uses regular AWS S3 services. | String | Not set, uses regular AWS S3 services. | -| `path_style` | The S3 path style to request from the storage backend. If `true`, "path style" is used, e.g. `host.com/bucket/object.bam`, otherwise `bucket.host.com/object` style is used. | `false` | Boolean | `false` | +| Option | Description | Type | Default | +|--------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------|---------------------------------------------------------------------------------------------------------------------------| +| `bucket` | The AWS S3 bucket where resources can be retrieved from. | String | Derived from the `resolvers` `regex` property if empty. This uses the first capture group in the `regex` as the `bucket`. | +| `endpoint` | A custom endpoint to override the default S3 service address. This is useful for using S3 locally or with storage backends such as MinIO. See [MinIO](#minio). | String | Not set, uses regular AWS S3 services. | +| `path_style` | The S3 path style to request from the storage backend. If `true`, "path style" is used, e.g. `host.com/bucket/object.bam`, otherwise `bucket.host.com/object` style is used. | Boolean | `false` | `UrlStorage` is another storage backend which can be used to serve data from a remote HTTP URL. When using this storage backend, htsget-rs will fetch data from a `url` which is set in the config. It will also forward any headers received with the initial query, which is useful for authentication. -To use `UrlStorage`, build htsget-rs with the `url-storage` feature enabled, and set the following options under `[resolvers.storage]`: +To use `UrlStorage`, build htsget-rs with the `url-storage` feature enabled, set `type = 'Url'` under `[resolvers.storage]`, and specify any additional options from below: -| Option | Description | Type | Default | -|--------------------------------------|------------------------------------------------------------------------------------------------------------------------------|--------------------------|-----------------------------------------------------------------------------------------------------------------| -| `url` | The URL to fetch data from. | HTTP URL | `"https://127.0.0.1:8081/"` | -| `response_url` | The URL to return to the client for fetching tickets. | HTTP URL | `"https://127.0.0.1:8081/"` | -| `forward_headers` | When constructing the URL tickets, copy HTTP headers received in the initial query. | Boolean | `true` | -| `header_blacklist` | List of headers that should not be forwarded | Array of headers | `[]` | -| `tls` | Additionally enables client authentication, or sets non-native root certificates for TLS. See [TLS](#tls) for more details. | TOML table | TLS is always allowed, however the default performs no client authentication and uses native root certificates. | +| Option | Description | Type | Default | +|--------------------------------------|-----------------------------------------------------------------------------------------------------------------------------|--------------------------|-----------------------------------------------------------------------------------------------------------------| +| `url` | The URL to fetch data from. | HTTP URL | `"https://127.0.0.1:8081/"` | +| `response_url` | The URL to return to the client for fetching tickets. | HTTP URL | `"https://127.0.0.1:8081/"` | +| `forward_headers` | When constructing the URL tickets, copy HTTP headers received in the initial query. | Boolean | `true` | +| `header_blacklist` | List of headers that should not be forwarded. | Array of headers | `[]` | +| `tls` | Additionally enables client authentication, or sets non-native root certificates for TLS. See [TLS](#tls) for more details. | TOML table | TLS is always allowed, however the default performs no client authentication and uses native root certificates. | When using `UrlStorage`, the following requests will be made to the `url`. * `GET` request to fetch only the headers of the data file (e.g. `GET /data.bam`, with `Range: bytes=0-`). @@ -192,7 +193,9 @@ For example, a `resolvers` value of: [[resolvers]] regex = '^(example_bucket)/(?P.*)$' substitution_string = '$key' -storage = 'S3' +[resolvers.storage] +type = 'S3' +# Uses the first capture group in the regex as the bucket. ``` Will use "example_bucket" as the S3 bucket if that resolver matches, because this is the first capture group in the `regex`. Note, to use this feature, at least one capture group must be defined in the `regex`. @@ -206,6 +209,7 @@ regex = '.*' substitution_string = '$0' [resolvers.storage] +type = 'Local' scheme = 'Http' authority = '127.0.0.1:8081' local_path = './' @@ -220,6 +224,7 @@ regex = '.*' substitution_string = '$0' [resolvers.storage] +type = 'S3' bucket = 'bucket' ``` @@ -231,6 +236,7 @@ regex = ".*" substitution_string = "$0" [resolvers.storage] +type = 'Url' url = "http://localhost:8080" response_url = "https://example.com" forward_headers = true diff --git a/htsget-config/examples/config-files/c4gh.toml b/htsget-config/examples/config-files/c4gh.toml index eb05b26aa..ce2609318 100644 --- a/htsget-config/examples/config-files/c4gh.toml +++ b/htsget-config/examples/config-files/c4gh.toml @@ -9,5 +9,7 @@ regex = ".*" substitution_string = "$0" [resolvers.storage] +type = 'Local' + private_key = "data/c4gh/keys/bob.sec" # pragma: allowlist secret recipient_public_key = "data/c4gh/keys/alice.pub" diff --git a/htsget-config/examples/config-files/s3_storage.toml b/htsget-config/examples/config-files/s3_storage.toml index 40f2c51d8..66828a80b 100644 --- a/htsget-config/examples/config-files/s3_storage.toml +++ b/htsget-config/examples/config-files/s3_storage.toml @@ -11,8 +11,9 @@ data_server_enabled = false [[resolvers]] regex = '^(bucket)/(?P.*)$' substitution_string = '$key' -storage = 'S3' +storage.type = 'S3' # Or, set the bucket manually #[resolvers.storage] +#type = 'S3' #bucket = 'bucket' diff --git a/htsget-config/examples/config-files/tls_data_server.toml b/htsget-config/examples/config-files/tls_data_server.toml index 38b9f74d1..649710bbb 100644 --- a/htsget-config/examples/config-files/tls_data_server.toml +++ b/htsget-config/examples/config-files/tls_data_server.toml @@ -10,3 +10,7 @@ data_server_tls.key = "key.pem" [[resolvers]] regex = ".*" substitution_string = "$0" + +[resolvers.storage] +type = 'Local' +use_data_server_config = true diff --git a/htsget-config/examples/config-files/tls_ticket_server.toml b/htsget-config/examples/config-files/tls_ticket_server.toml index e80976eac..4ef72c6e7 100644 --- a/htsget-config/examples/config-files/tls_ticket_server.toml +++ b/htsget-config/examples/config-files/tls_ticket_server.toml @@ -12,4 +12,5 @@ regex = ".*" substitution_string = "$0" [resolvers.storage] +type = 'S3' bucket = "bucket" diff --git a/htsget-config/examples/config-files/url_storage.toml b/htsget-config/examples/config-files/url_storage.toml index 07dcab677..cfda737fd 100644 --- a/htsget-config/examples/config-files/url_storage.toml +++ b/htsget-config/examples/config-files/url_storage.toml @@ -16,6 +16,7 @@ regex = ".*" substitution_string = "$0" [resolvers.storage] +type = 'Url' url = "http://127.0.0.1:8081" response_url = "https://127.0.0.1:8081" forward_headers = true diff --git a/htsget-config/src/config/mod.rs b/htsget-config/src/config/mod.rs index c2223c50c..cfa443541 100644 --- a/htsget-config/src/config/mod.rs +++ b/htsget-config/src/config/mod.rs @@ -774,13 +774,15 @@ pub(crate) mod tests { data_server_serve_at = "/path" [[resolvers]] - storage = "Local" + [resolvers.storage] + type = "Local" + use_data_server_config = true "#, |config| { assert_eq!(config.resolvers.len(), 1); assert!(matches!(config.resolvers.first().unwrap().storage(), - Storage::Local { local_storage } if local_storage.local_path() == "path" && local_storage.scheme() == Http && local_storage.authority() == &Authority::from_static("127.0.0.1:8080") && local_storage.path_prefix() == "/path")); + Storage::Local(local_storage) if local_storage.local_path() == "path" && local_storage.scheme() == Http && local_storage.authority() == &Authority::from_static("127.0.0.1:8080") && local_storage.path_prefix() == "/path")); }, ); } diff --git a/htsget-config/src/resolver.rs b/htsget-config/src/resolver.rs index 1844feda9..de513c710 100644 --- a/htsget-config/src/resolver.rs +++ b/htsget-config/src/resolver.rs @@ -13,7 +13,7 @@ use crate::storage::local::LocalStorage; use crate::storage::s3::S3Storage; #[cfg(feature = "url-storage")] use crate::storage::url::UrlStorageClient; -use crate::storage::{ResolvedId, Storage, TaggedStorageTypes}; +use crate::storage::{ResolvedId, Storage}; use crate::types::Format::{Bam, Bcf, Cram, Vcf}; use crate::types::{Class, Fields, Format, Interval, Query, Response, Result, TaggedTypeAll, Tags}; @@ -293,9 +293,9 @@ impl Resolver { /// Set the local resolvers from the data server config. pub fn resolvers_from_data_server_config(&mut self, config: &DataServerConfig) { - if let Storage::Tagged(TaggedStorageTypes::Local) = self.storage() { - if let Some(local_storage) = config.into() { - self.storage = Storage::Local { local_storage }; + if let Storage::Local(local) = self.storage() { + if local.use_data_server_config() { + self.storage = Storage::Local(config.into()); } } } @@ -485,9 +485,10 @@ mod tests { "data".to_string(), "/data".to_string(), Default::default(), + false, ); let resolver = Resolver::new( - Storage::Local { local_storage }, + Storage::Local(local_storage), "id", "$0-test", AllowGuard::default(), @@ -502,7 +503,7 @@ mod tests { async fn resolver_resolve_s3_request_tagged() { let s3_storage = S3Storage::new("id".to_string(), None, false); let resolver = Resolver::new( - Storage::S3 { s3_storage }, + Storage::S3(s3_storage), "(id)-1", "$1-test", AllowGuard::default(), @@ -516,7 +517,7 @@ mod tests { #[tokio::test] async fn resolver_resolve_s3_request() { let resolver = Resolver::new( - Storage::Tagged(TaggedStorageTypes::S3), + Storage::S3(S3Storage::default()), "(id)-1", "$1-test", AllowGuard::default(), @@ -543,7 +544,7 @@ mod tests { ); let resolver = Resolver::new( - Storage::Url { url_storage }, + Storage::Url(url_storage), "(id)-1", "$1-test", AllowGuard::default(), @@ -678,7 +679,7 @@ mod tests { vec![( "HTSGET_RESOLVERS", "[{ regex=regex, substitution_string=substitution_string, \ - storage={ bucket=bucket }, \ + storage={ type=S3, bucket=bucket }, \ allow_guard={ allow_reference_names=[chr1], allow_fields=[QNAME], allow_tags=[RG], \ allow_formats=[BAM], allow_classes=[body], allow_interval_start=100, \ allow_interval_end=1000 } }]", @@ -698,7 +699,7 @@ mod tests { assert_eq!(resolver.regex().to_string(), "regex"); assert_eq!(resolver.substitution_string(), "substitution_string"); assert!( - matches!(resolver.storage(), Storage::S3 { s3_storage } if s3_storage == &expected_storage) + matches!(resolver.storage(), Storage::S3(s3_storage) if s3_storage == &expected_storage) ); assert_eq!(resolver.allow_guard(), &allow_guard); }, diff --git a/htsget-config/src/storage/local.rs b/htsget-config/src/storage/local.rs index 134fbb7bf..b6b60818f 100644 --- a/htsget-config/src/storage/local.rs +++ b/htsget-config/src/storage/local.rs @@ -26,6 +26,7 @@ pub struct LocalStorage { path_prefix: String, #[serde(flatten)] object_type: ObjectType, + use_data_server_config: bool, } impl LocalStorage { @@ -36,6 +37,7 @@ impl LocalStorage { local_path: String, path_prefix: String, object_type: ObjectType, + use_data_server_config: bool, ) -> Self { Self { scheme, @@ -43,6 +45,7 @@ impl LocalStorage { local_path, path_prefix, object_type, + use_data_server_config, } } @@ -70,6 +73,11 @@ impl LocalStorage { pub fn object_type(&self) -> &ObjectType { &self.object_type } + + /// Get whether config should be inherited from the data server config. + pub fn use_data_server_config(&self) -> bool { + self.use_data_server_config + } } impl Default for LocalStorage { @@ -80,19 +88,21 @@ impl Default for LocalStorage { local_path: default_local_path(), path_prefix: Default::default(), object_type: Default::default(), + use_data_server_config: false, } } } -impl From<&DataServerConfig> for Option { +impl From<&DataServerConfig> for LocalStorage { fn from(config: &DataServerConfig) -> Self { - Some(LocalStorage::new( + LocalStorage::new( config.tls().get_scheme(), - Authority::from_str(&config.addr().to_string()).ok()?, - config.local_path().to_str()?.to_string(), + Authority::from_str(&config.addr().to_string()).expect("expected valid authority"), + config.local_path().to_string_lossy().to_string(), config.serve_at().to_string(), Default::default(), - )) + true, + ) } } @@ -116,6 +126,7 @@ mod tests { regex = "regex" [resolvers.storage] + type = "Local" local_path = "path" scheme = "HTTPS" path_prefix = "path" @@ -124,7 +135,7 @@ mod tests { println!("{:?}", config.resolvers().first().unwrap().storage()); assert!(matches!( config.resolvers().first().unwrap().storage(), - Storage::Local { local_storage } if local_storage.local_path() == "path" && local_storage.scheme() == Scheme::Https && local_storage.path_prefix() == "path" + Storage::Local(local_storage) if local_storage.local_path() == "path" && local_storage.scheme() == Scheme::Https && local_storage.path_prefix() == "path" )); }, ); @@ -140,15 +151,16 @@ mod tests { None, CorsConfig::default(), ); - let result: Option = (&data_server_config).into(); + let result: LocalStorage = (&data_server_config).into(); let expected = LocalStorage::new( Http, Authority::from_static("127.0.0.1:8080"), "data".to_string(), "/data".to_string(), Default::default(), + true, ); - assert_eq!(result.unwrap(), expected); + assert_eq!(result, expected); } } diff --git a/htsget-config/src/storage/mod.rs b/htsget-config/src/storage/mod.rs index fc1be3ad9..fdfbedc50 100644 --- a/htsget-config/src/storage/mod.rs +++ b/htsget-config/src/storage/mod.rs @@ -1,5 +1,3 @@ -use serde::{Deserialize, Serialize}; - use crate::resolver::ResolveResponse; use crate::storage::local::LocalStorage; #[cfg(feature = "s3-storage")] @@ -7,6 +5,7 @@ use crate::storage::s3::S3Storage; #[cfg(feature = "url-storage")] use crate::storage::url::UrlStorageClient; use crate::types::{Query, Response, Result}; +use serde::{Deserialize, Serialize}; pub mod local; pub mod object; @@ -15,22 +14,6 @@ pub mod s3; #[cfg(feature = "url-storage")] pub mod url; -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] -pub enum TaggedStorageTypes { - #[serde(alias = "local", alias = "LOCAL")] - Local, - #[cfg(feature = "s3-storage")] - #[serde(alias = "s3")] - S3, -} - -/// If s3-storage is enabled, then the default is `S3`, otherwise it is `Local`. -impl Default for TaggedStorageTypes { - fn default() -> Self { - Self::Local - } -} - /// A new type representing a resolved id. #[derive(Debug)] pub struct ResolvedId(String); @@ -49,24 +32,19 @@ impl ResolvedId { /// Specify the storage backend to use as config values. #[derive(Serialize, Deserialize, Debug, Clone)] -#[serde(untagged)] +#[serde(tag = "type")] #[non_exhaustive] pub enum Storage { - Tagged(TaggedStorageTypes), - Local { - #[serde(flatten)] - local_storage: LocalStorage, - }, + #[serde(alias = "local", alias = "LOCAL")] + Local(LocalStorage), #[cfg(feature = "s3-storage")] - S3 { - #[serde(flatten)] - s3_storage: S3Storage, - }, + #[serde(alias = "s3")] + S3(S3Storage), #[cfg(feature = "url-storage")] - Url { - #[serde(flatten, skip_serializing)] - url_storage: UrlStorageClient, - }, + #[serde(alias = "url", alias = "URL")] + Url(#[serde(skip_serializing)] UrlStorageClient), + #[serde(skip)] + Unknown, } impl Storage { @@ -78,7 +56,7 @@ impl Storage { query: &Query, ) -> Option> { match self { - Storage::Local { local_storage } => Some(T::from_local(local_storage, query).await), + Storage::Local(local_storage) => Some(T::from_local(local_storage, query).await), _ => None, } } @@ -91,13 +69,14 @@ impl Storage { query: &Query, ) -> Option> { match self { - Storage::Tagged(TaggedStorageTypes::S3) => { - let bucket = first_match?.to_string(); + Storage::S3(s3_storage) => { + let mut s3_storage = s3_storage.clone(); + if s3_storage.bucket.is_empty() { + s3_storage.bucket = first_match?.to_string(); + } - let s3_storage = S3Storage::new(bucket, None, false); Some(T::from_s3(&s3_storage, query).await) } - Storage::S3 { s3_storage } => Some(T::from_s3(s3_storage, query).await), _ => None, } } @@ -109,7 +88,7 @@ impl Storage { query: &Query, ) -> Option> { match self { - Storage::Url { url_storage } => Some(T::from_url(url_storage, query).await), + Storage::Url(url_storage) => Some(T::from_url(url_storage, query).await), _ => None, } } @@ -117,7 +96,7 @@ impl Storage { impl Default for Storage { fn default() -> Self { - Self::Tagged(TaggedStorageTypes::default()) + Self::Local(Default::default()) } } @@ -132,8 +111,9 @@ pub(crate) mod tests { test_config_from_file( r#" [[resolvers]] + [resolvers.storage] + type = "Local" regex = "regex" - storage = "Local" "#, |config| { println!("{:?}", config.resolvers().first().unwrap().storage()); @@ -147,17 +127,18 @@ pub(crate) mod tests { #[test] fn config_storage_tagged_local_env() { - test_config_from_env(vec![("HTSGET_RESOLVERS", "[{storage=Local}]")], |config| { - assert!(matches!( - config.resolvers().first().unwrap().storage(), - Storage::Local { .. } - )); - }); - } - - #[test] - fn default_tagged_storage_type_local() { - assert_eq!(TaggedStorageTypes::default(), TaggedStorageTypes::Local); + test_config_from_env( + vec![( + "HTSGET_RESOLVERS", + "[{storage={ type=Local, use_data_server_config=true}}]", + )], + |config| { + assert!(matches!( + config.resolvers().first().unwrap().storage(), + Storage::Local { .. } + )); + }, + ); } #[cfg(feature = "s3-storage")] @@ -166,14 +147,15 @@ pub(crate) mod tests { test_config_from_file( r#" [[resolvers]] + [resolvers.storage] + type = "S3" regex = "regex" - storage = "S3" "#, |config| { println!("{:?}", config.resolvers().first().unwrap().storage()); assert!(matches!( config.resolvers().first().unwrap().storage(), - Storage::Tagged(TaggedStorageTypes::S3) + Storage::S3(..) )); }, ); @@ -182,11 +164,14 @@ pub(crate) mod tests { #[cfg(feature = "s3-storage")] #[test] fn config_storage_tagged_s3_env() { - test_config_from_env(vec![("HTSGET_RESOLVERS", "[{storage=S3}]")], |config| { - assert!(matches!( - config.resolvers().first().unwrap().storage(), - Storage::Tagged(TaggedStorageTypes::S3) - )); - }); + test_config_from_env( + vec![("HTSGET_RESOLVERS", "[{storage={ type=S3 }}]")], + |config| { + assert!(matches!( + config.resolvers().first().unwrap().storage(), + Storage::S3(..) + )); + }, + ); } } diff --git a/htsget-config/src/storage/object/c4gh.rs b/htsget-config/src/storage/object/c4gh.rs index f7ec020cb..93001cd6a 100644 --- a/htsget-config/src/storage/object/c4gh.rs +++ b/htsget-config/src/storage/object/c4gh.rs @@ -94,6 +94,7 @@ mod tests { regex = "regex" [resolvers.storage] + type = "Local" private_key = "{}" recipient_public_key = "{}" "#, @@ -104,7 +105,7 @@ mod tests { println!("{:?}", config.resolvers().first().unwrap().storage()); assert!(matches!( config.resolvers().first().unwrap().storage(), - Storage::Local { local_storage } if local_storage.object_type().keys().is_some() + Storage::Local(local_storage) if local_storage.object_type().keys().is_some() )); }, ); diff --git a/htsget-config/src/storage/object/mod.rs b/htsget-config/src/storage/object/mod.rs index 4fdd80485..c076d4527 100644 --- a/htsget-config/src/storage/object/mod.rs +++ b/htsget-config/src/storage/object/mod.rs @@ -17,6 +17,7 @@ pub struct ObjectType { } impl ObjectType { + #[cfg(feature = "experimental")] /// Get the C4GH keys. pub fn keys(&self) -> Option<&C4GHKeys> { self.keys.as_ref() diff --git a/htsget-config/src/storage/s3.rs b/htsget-config/src/storage/s3.rs index 70f5407c7..cea0e846e 100644 --- a/htsget-config/src/storage/s3.rs +++ b/htsget-config/src/storage/s3.rs @@ -3,9 +3,9 @@ use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize, Debug, Default, Clone, PartialEq, Eq)] #[serde(default)] pub struct S3Storage { - bucket: String, - endpoint: Option, - path_style: bool, + pub(crate) bucket: String, + pub(crate) endpoint: Option, + pub(crate) path_style: bool, } impl S3Storage { @@ -47,13 +47,14 @@ mod tests { regex = "regex" [resolvers.storage] + type = "S3" bucket = "bucket" "#, |config| { println!("{:?}", config.resolvers().first().unwrap().storage()); assert!(matches!( config.resolvers().first().unwrap().storage(), - Storage::S3 { s3_storage } if s3_storage.bucket() == "bucket" + Storage::S3(s3_storage) if s3_storage.bucket() == "bucket" )); }, ); diff --git a/htsget-config/src/storage/url.rs b/htsget-config/src/storage/url.rs index d08c2bb3e..6310142c5 100644 --- a/htsget-config/src/storage/url.rs +++ b/htsget-config/src/storage/url.rs @@ -240,6 +240,7 @@ mod tests { regex = "regex" [resolvers.storage] + type = "Url" url = "https://example.com/" response_url = "https://example.com/" forward_headers = false @@ -255,7 +256,7 @@ mod tests { println!("{:?}", config.resolvers().first().unwrap().storage()); assert!(matches!( config.resolvers().first().unwrap().storage(), - Storage::Url { url_storage } if *url_storage.url() == "https://example.com/" + Storage::Url(url_storage) if *url_storage.url() == "https://example.com/" && !url_storage.forward_headers() )); }, diff --git a/htsget-http/src/lib.rs b/htsget-http/src/lib.rs index 0ed66aed8..5000c8573 100644 --- a/htsget-http/src/lib.rs +++ b/htsget-http/src/lib.rs @@ -279,6 +279,7 @@ mod tests { "data".to_string(), "/data".to_string(), Default::default(), + false, ), ) .unwrap(), diff --git a/htsget-search/benches/search_benchmarks.rs b/htsget-search/benches/search_benchmarks.rs index 0b4e71181..b0b3c88f2 100644 --- a/htsget-search/benches/search_benchmarks.rs +++ b/htsget-search/benches/search_benchmarks.rs @@ -23,6 +23,7 @@ async fn perform_query(query: Query) -> Result<(), HtsGetError> { "../data".to_string(), "/data".to_string(), Default::default(), + false, ), &query, ) diff --git a/htsget-search/src/from_storage.rs b/htsget-search/src/from_storage.rs index 5f5607cc3..419b4a2b3 100644 --- a/htsget-search/src/from_storage.rs +++ b/htsget-search/src/from_storage.rs @@ -201,7 +201,7 @@ pub(crate) mod tests { with_config_local_storage( |_, local_storage| async { let resolvers = vec![Resolver::new( - storage::Storage::Local { local_storage }, + storage::Storage::Local(local_storage), ".*", "$0", Default::default(), @@ -272,6 +272,7 @@ pub(crate) mod tests { base_path.to_str().unwrap().to_string(), "/data".to_string(), Default::default(), + false, ), ) .await; diff --git a/htsget-storage/Cargo.toml b/htsget-storage/Cargo.toml index dfe3b28bb..df56aab28 100644 --- a/htsget-storage/Cargo.toml +++ b/htsget-storage/Cargo.toml @@ -31,6 +31,7 @@ default = [] [dependencies] url = "2" http = "1" +cfg-if = "1" # Async tokio = { version = "1", features = ["macros", "rt-multi-thread"] } diff --git a/htsget-storage/src/c4gh/storage.rs b/htsget-storage/src/c4gh/storage.rs index 8ec057e72..5ce46b3be 100644 --- a/htsget-storage/src/c4gh/storage.rs +++ b/htsget-storage/src/c4gh/storage.rs @@ -62,9 +62,14 @@ impl Debug for C4GHStorage { impl C4GHStorage { /// Create a new storage from a storage trait. pub fn new(keys: Vec, inner: impl StorageTrait + Send + Sync + 'static) -> Self { + Self::new_box(keys, Box::new(inner)) + } + + /// Create a new value from a boxed storage trait. + pub fn new_box(keys: Vec, inner: Box) -> Self { Self { keys, - inner: Box::new(inner), + inner, state: Default::default(), } } diff --git a/htsget-storage/src/lib.rs b/htsget-storage/src/lib.rs index de436f403..b1fedf69c 100644 --- a/htsget-storage/src/lib.rs +++ b/htsget-storage/src/lib.rs @@ -9,14 +9,28 @@ pub use htsget_config::types::{ Class, Format, Headers, HtsGetError, JsonResponse, Query, Response, Url, }; +#[cfg(feature = "experimental")] +use crate::c4gh::storage::C4GHStorage; +use crate::error::Result; +use crate::error::StorageError; +use crate::local::LocalStorage; +#[cfg(feature = "s3-storage")] +use crate::s3::S3Storage; +use crate::types::{BytesPositionOptions, DataBlock, GetOptions, HeadOptions, RangeUrlOptions}; +#[cfg(feature = "url-storage")] +use crate::url::UrlStorage; use async_trait::async_trait; use base64::engine::general_purpose; use base64::Engine; +use cfg_if::cfg_if; use htsget_config::storage::local::LocalStorage as LocalStorageConfig; +#[cfg(feature = "experimental")] +use htsget_config::storage::object::ObjectType; #[cfg(feature = "s3-storage")] use htsget_config::storage::s3::S3Storage as S3StorageConfig; #[cfg(feature = "url-storage")] use htsget_config::storage::url::UrlStorageClient as UrlStorageConfig; +use htsget_config::types::Scheme; use http::uri; use pin_project_lite::pin_project; use std::fmt; @@ -25,18 +39,6 @@ use std::pin::Pin; use std::task::{Context, Poll}; use tokio::io::{AsyncRead, ReadBuf}; -#[cfg(feature = "experimental")] -use crate::c4gh::storage::C4GHStorage; -use crate::error::Result; -use crate::error::StorageError; -use crate::local::LocalStorage; -#[cfg(feature = "s3-storage")] -use crate::s3::S3Storage; -use crate::types::{BytesPositionOptions, DataBlock, GetOptions, HeadOptions, RangeUrlOptions}; -#[cfg(feature = "url-storage")] -use crate::url::UrlStorage; -use htsget_config::types::Scheme; - #[cfg(feature = "experimental")] pub mod c4gh; pub mod error; @@ -79,6 +81,13 @@ pub struct Storage { inner: Box, } +impl Storage { + /// Get the inner value. + pub fn into_inner(self) -> Box { + self.inner + } +} + impl Clone for Storage { fn clone(&self) -> Self { Self { @@ -128,17 +137,28 @@ impl StorageTrait for Storage { } impl Storage { + #[cfg(feature = "experimental")] + /// Wrap an existing storage with C4GH storage. + pub fn c4gh_storage(object_type: &ObjectType, storage: Storage) -> Storage { + if let Some(keys) = object_type.keys() { + Storage::new(C4GHStorage::new_box( + keys.clone().into_inner(), + storage.into_inner(), + )) + } else { + storage + } + } + /// Create from local storage config. pub async fn from_local(config: &LocalStorageConfig) -> Result { - let storage = LocalStorage::new(config.local_path(), config.clone())?; - - match config.object_type().keys() { - None => Ok(Storage::new(storage)), - #[cfg(feature = "experimental")] - Some(keys) => Ok(Storage::new(C4GHStorage::new( - keys.clone().into_inner(), - storage, - ))), + let storage = Storage::new(LocalStorage::new(config.local_path(), config.clone())?); + cfg_if! { + if #[cfg(feature = "experimental")] { + Ok(Self::c4gh_storage(config.object_type(), storage)) + } else { + Ok(storage) + } } } @@ -285,6 +305,7 @@ mod tests { "data".to_string(), "/data".to_string(), Default::default(), + false, ); test_formatter_authority(formatter, "http"); } @@ -297,6 +318,7 @@ mod tests { "data".to_string(), "/data".to_string(), Default::default(), + false, ); test_formatter_authority(formatter, "https"); } diff --git a/htsget-storage/src/local.rs b/htsget-storage/src/local.rs index 60cb232bb..d79132a3f 100644 --- a/htsget-storage/src/local.rs +++ b/htsget-storage/src/local.rs @@ -346,6 +346,7 @@ pub(crate) mod tests { "data".to_string(), "/data".to_string(), Default::default(), + false, ), ) .unwrap() diff --git a/htsget-test/src/http/mod.rs b/htsget-test/src/http/mod.rs index 18abd7eaa..4d643f996 100644 --- a/htsget-test/src/http/mod.rs +++ b/htsget-test/src/http/mod.rs @@ -101,19 +101,18 @@ pub fn default_test_resolver(addr: SocketAddr, scheme: Scheme) -> Vec default_dir_data().to_str().unwrap().to_string(), "/data".to_string(), Default::default(), + false, ); vec![ Resolver::new( - Storage::Local { - local_storage: local_storage.clone(), - }, + Storage::Local(local_storage.clone()), "^1-(.*)$", "$1", Default::default(), ) .unwrap(), Resolver::new( - Storage::Local { local_storage }, + Storage::Local(local_storage), "^2-(.*)$", "$1", Default::default(), From 713c007f7406d025439cb43b9349cc2c184a8422 Mon Sep 17 00:00:00 2001 From: Marko Malenic Date: Tue, 24 Sep 2024 09:11:47 +1000 Subject: [PATCH 3/5] refactor(config): slight adjustment to reduce conditionally compiled code branches --- htsget-config/src/resolver.rs | 46 ++++++++++++++-------------- htsget-config/src/storage/mod.rs | 51 -------------------------------- htsget-storage/src/lib.rs | 33 ++++++++++----------- 3 files changed, 38 insertions(+), 92 deletions(-) diff --git a/htsget-config/src/resolver.rs b/htsget-config/src/resolver.rs index de513c710..28c7cb5c3 100644 --- a/htsget-config/src/resolver.rs +++ b/htsget-config/src/resolver.rs @@ -293,10 +293,16 @@ impl Resolver { /// Set the local resolvers from the data server config. pub fn resolvers_from_data_server_config(&mut self, config: &DataServerConfig) { - if let Storage::Local(local) = self.storage() { - if local.use_data_server_config() { - self.storage = Storage::Local(config.into()); + match self.storage() { + Storage::Local(local) => { + if local.use_data_server_config() { + self.storage = Storage::Local(config.into()); + } } + #[cfg(feature = "s3-storage")] + Storage::S3(_) => {} + #[cfg(feature = "url-storage")] + Storage::Url(_) => {} } } @@ -384,29 +390,21 @@ impl StorageResolver for Resolver { query.set_id(resolved_id.into_inner()); - if let Some(response) = self.storage().resolve_local_storage::(query).await { - return Some(response); - } - - #[cfg(feature = "s3-storage")] - { - let first_match = self.get_match(1, &_matched_id); - - if let Some(response) = self - .storage() - .resolve_s3_storage::(first_match, query) - .await - { - return Some(response); + match self.storage() { + Storage::Local(local_storage) => Some(T::from_local(local_storage, query).await), + #[cfg(feature = "s3-storage")] + Storage::S3(s3_storage) => { + let first_match = self.get_match(1, &_matched_id); + let mut s3_storage = s3_storage.clone(); + if s3_storage.bucket.is_empty() { + s3_storage.bucket = first_match?.to_string(); + } + + Some(T::from_s3(&s3_storage, query).await) } + #[cfg(feature = "url-storage")] + Storage::Url(url_storage) => Some(T::from_url(url_storage, query).await), } - - #[cfg(feature = "url-storage")] - if let Some(response) = self.storage().resolve_url_storage::(query).await { - return Some(response); - } - - None } } diff --git a/htsget-config/src/storage/mod.rs b/htsget-config/src/storage/mod.rs index fdfbedc50..aa0cac92a 100644 --- a/htsget-config/src/storage/mod.rs +++ b/htsget-config/src/storage/mod.rs @@ -1,10 +1,8 @@ -use crate::resolver::ResolveResponse; use crate::storage::local::LocalStorage; #[cfg(feature = "s3-storage")] use crate::storage::s3::S3Storage; #[cfg(feature = "url-storage")] use crate::storage::url::UrlStorageClient; -use crate::types::{Query, Response, Result}; use serde::{Deserialize, Serialize}; pub mod local; @@ -43,55 +41,6 @@ pub enum Storage { #[cfg(feature = "url-storage")] #[serde(alias = "url", alias = "URL")] Url(#[serde(skip_serializing)] UrlStorageClient), - #[serde(skip)] - Unknown, -} - -impl Storage { - /// Resolve the local component `Storage` into a type that implements `FromStorage`. Tagged - /// `Local` storage is not resolved because it is resolved into untagged `Local` storage when - /// `Config` is constructed. - pub async fn resolve_local_storage( - &self, - query: &Query, - ) -> Option> { - match self { - Storage::Local(local_storage) => Some(T::from_local(local_storage, query).await), - _ => None, - } - } - - /// Resolve the s3 component of `Storage` into a type that implements `FromStorage`. - #[cfg(feature = "s3-storage")] - pub async fn resolve_s3_storage( - &self, - first_match: Option<&str>, - query: &Query, - ) -> Option> { - match self { - Storage::S3(s3_storage) => { - let mut s3_storage = s3_storage.clone(); - if s3_storage.bucket.is_empty() { - s3_storage.bucket = first_match?.to_string(); - } - - Some(T::from_s3(&s3_storage, query).await) - } - _ => None, - } - } - - /// Resolve the url component of `Storage` into a type that implements `FromStorage`. - #[cfg(feature = "url-storage")] - pub async fn resolve_url_storage( - &self, - query: &Query, - ) -> Option> { - match self { - Storage::Url(url_storage) => Some(T::from_url(url_storage, query).await), - _ => None, - } - } } impl Default for Storage { diff --git a/htsget-storage/src/lib.rs b/htsget-storage/src/lib.rs index b1fedf69c..d86a93228 100644 --- a/htsget-storage/src/lib.rs +++ b/htsget-storage/src/lib.rs @@ -137,31 +137,30 @@ impl StorageTrait for Storage { } impl Storage { - #[cfg(feature = "experimental")] - /// Wrap an existing storage with C4GH storage. - pub fn c4gh_storage(object_type: &ObjectType, storage: Storage) -> Storage { - if let Some(keys) = object_type.keys() { - Storage::new(C4GHStorage::new_box( - keys.clone().into_inner(), - storage.into_inner(), - )) - } else { - storage - } - } - - /// Create from local storage config. - pub async fn from_local(config: &LocalStorageConfig) -> Result { - let storage = Storage::new(LocalStorage::new(config.local_path(), config.clone())?); + /// Wrap an existing storage with the object type storage. + pub fn from_object_type(object_type: &ObjectType, storage: Storage) -> Storage { cfg_if! { if #[cfg(feature = "experimental")] { - Ok(Self::c4gh_storage(config.object_type(), storage)) + if let Some(keys) = object_type.keys() { + Storage::new(C4GHStorage::new_box( + keys.clone().into_inner(), + storage.into_inner(), + )) + } else { + storage + } } else { Ok(storage) } } } + /// Create from local storage config. + pub async fn from_local(config: &LocalStorageConfig) -> Result { + let storage = Storage::new(LocalStorage::new(config.local_path(), config.clone())?); + Ok(Storage::from_object_type(config.object_type(), storage)) + } + /// Create from s3 config. #[cfg(feature = "s3-storage")] pub async fn from_s3(s3_storage: &S3StorageConfig) -> Storage { From 4839016dd36860fb3f34feb412281b7c5a415595 Mon Sep 17 00:00:00 2001 From: Marko Malenic Date: Tue, 24 Sep 2024 10:48:34 +1000 Subject: [PATCH 4/5] style: add newline --- deploy/config/example_deploy.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deploy/config/example_deploy.toml b/deploy/config/example_deploy.toml index f4cb5d377..abd7ec0d0 100644 --- a/deploy/config/example_deploy.toml +++ b/deploy/config/example_deploy.toml @@ -17,4 +17,4 @@ environment = "dev" [[resolvers]] regex = '^(?P.*?)/(?P.*)$' substitution_string = '$key' -storage.type = 'S3' \ No newline at end of file +storage.type = 'S3' From 1210175ac5e90a6194d5309318e03e2cab0de13f Mon Sep 17 00:00:00 2001 From: Marko Malenic Date: Tue, 24 Sep 2024 11:14:41 +1000 Subject: [PATCH 5/5] fix: build errors with conditional flags --- htsget-storage/src/lib.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/htsget-storage/src/lib.rs b/htsget-storage/src/lib.rs index d86a93228..ad2c7c52a 100644 --- a/htsget-storage/src/lib.rs +++ b/htsget-storage/src/lib.rs @@ -24,7 +24,6 @@ use base64::engine::general_purpose; use base64::Engine; use cfg_if::cfg_if; use htsget_config::storage::local::LocalStorage as LocalStorageConfig; -#[cfg(feature = "experimental")] use htsget_config::storage::object::ObjectType; #[cfg(feature = "s3-storage")] use htsget_config::storage::s3::S3Storage as S3StorageConfig; @@ -138,10 +137,10 @@ impl StorageTrait for Storage { impl Storage { /// Wrap an existing storage with the object type storage. - pub fn from_object_type(object_type: &ObjectType, storage: Storage) -> Storage { + pub fn from_object_type(_object_type: &ObjectType, storage: Storage) -> Storage { cfg_if! { if #[cfg(feature = "experimental")] { - if let Some(keys) = object_type.keys() { + if let Some(keys) = _object_type.keys() { Storage::new(C4GHStorage::new_box( keys.clone().into_inner(), storage.into_inner(), @@ -150,7 +149,7 @@ impl Storage { storage } } else { - Ok(storage) + storage } } }