From 9aef3cfc8989c491db05c4d66cdbf3d89429420e Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Mon, 13 Nov 2023 11:45:22 +0200 Subject: refactor(tvix/store/pathinfosvc): inline MemoryPathInfoSvc::from_url Change-Id: If27eb518d372f4004b7b38fc765a42957f2a6b50 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10023 Tested-by: BuildkiteCI Reviewed-by: Connor Brewster --- tvix/store/src/pathinfoservice/from_addr.rs | 52 ++++++++++++++++-- tvix/store/src/pathinfoservice/memory.rs | 83 ----------------------------- 2 files changed, 47 insertions(+), 88 deletions(-) (limited to 'tvix/store') diff --git a/tvix/store/src/pathinfoservice/from_addr.rs b/tvix/store/src/pathinfoservice/from_addr.rs index 93cb487f29b9..96a533e290fd 100644 --- a/tvix/store/src/pathinfoservice/from_addr.rs +++ b/tvix/store/src/pathinfoservice/from_addr.rs @@ -30,11 +30,11 @@ pub fn from_addr( Url::parse(uri).map_err(|e| Error::StorageError(format!("unable to parse url: {}", e)))?; Ok(if url.scheme() == "memory" { - Arc::new(MemoryPathInfoService::from_url( - &url, - blob_service, - directory_service, - )?) + // memory doesn't support host or path in the URL. + if url.has_host() || !url.path().is_empty() { + return Err(Error::StorageError("invalid url".to_string())); + } + Arc::new(MemoryPathInfoService::new(blob_service, directory_service)) } else if url.scheme() == "sled" { Arc::new(SledPathInfoService::from_url( &url, @@ -54,3 +54,45 @@ pub fn from_addr( )))? }) } + +#[cfg(test)] +mod tests { + use tvix_castore::utils::{gen_blob_service, gen_directory_service}; + + use super::from_addr; + + /// This uses a wrong scheme. + #[test] + fn invalid_scheme() { + assert!(from_addr( + "http://foo.example/test", + gen_blob_service(), + gen_directory_service() + ) + .is_err()); + } + + /// This correctly sets the scheme, and doesn't set a path. + #[test] + fn memory_valid_scheme() { + assert!(from_addr("memory://", gen_blob_service(), gen_directory_service()).is_ok()) + } + + /// This sets a memory url host to `foo` + #[test] + fn memory_invalid_host() { + assert!(from_addr("memory://foo", gen_blob_service(), gen_directory_service()).is_err()) + } + + /// This sets a memory urlp path to "/", which is invalid. + #[test] + fn memory_invalid_has_path() { + assert!(from_addr("memory:///", gen_blob_service(), gen_directory_service()).is_err()) + } + + /// This sets a memory url path "/foo", which is invalid. + #[test] + fn memory_invalid_path2() { + assert!(from_addr("memory:///foo", gen_blob_service(), gen_directory_service()).is_err()) + } +} diff --git a/tvix/store/src/pathinfoservice/memory.rs b/tvix/store/src/pathinfoservice/memory.rs index 90a151fd04e5..6f96c083476c 100644 --- a/tvix/store/src/pathinfoservice/memory.rs +++ b/tvix/store/src/pathinfoservice/memory.rs @@ -29,26 +29,6 @@ impl MemoryPathInfoService { directory_service, } } - - /// Constructs a [MemoryPathInfoService] from the passed [url::Url]: - /// - scheme has to be `memory://` - /// - there may not be a host. - /// - there may not be a path. - pub fn from_url( - url: &url::Url, - blob_service: Arc, - directory_service: Arc, - ) -> Result { - if url.scheme() != "memory" { - return Err(Error::StorageError("invalid scheme".to_string())); - } - - if url.has_host() || !url.path().is_empty() { - return Err(Error::StorageError("invalid url".to_string())); - } - - Ok(Self::new(blob_service, directory_service)) - } } #[async_trait] @@ -106,66 +86,3 @@ impl PathInfoService for MemoryPathInfoService { Box::pin(iter(items)) } } - -#[cfg(test)] -mod tests { - use crate::tests::utils::gen_blob_service; - use crate::tests::utils::gen_directory_service; - - use super::MemoryPathInfoService; - - /// This uses a wrong scheme. - #[test] - fn test_invalid_scheme() { - let url = url::Url::parse("http://foo.example/test").expect("must parse"); - - assert!( - MemoryPathInfoService::from_url(&url, gen_blob_service(), gen_directory_service()) - .is_err() - ); - } - - /// This correctly sets the scheme, and doesn't set a path. - #[test] - fn test_valid_scheme() { - let url = url::Url::parse("memory://").expect("must parse"); - - assert!( - MemoryPathInfoService::from_url(&url, gen_blob_service(), gen_directory_service()) - .is_ok() - ); - } - - /// This sets the host to `foo` - #[test] - fn test_invalid_host() { - let url = url::Url::parse("memory://foo").expect("must parse"); - - assert!( - MemoryPathInfoService::from_url(&url, gen_blob_service(), gen_directory_service()) - .is_err() - ); - } - - /// This has the path "/", which is invalid. - #[test] - fn test_invalid_has_path() { - let url = url::Url::parse("memory:///").expect("must parse"); - - assert!( - MemoryPathInfoService::from_url(&url, gen_blob_service(), gen_directory_service()) - .is_err() - ); - } - - /// This has the path "/foo", which is invalid. - #[test] - fn test_invalid_path2() { - let url = url::Url::parse("memory:///foo").expect("must parse"); - - assert!( - MemoryPathInfoService::from_url(&url, gen_blob_service(), gen_directory_service()) - .is_err() - ); - } -} -- cgit 1.4.1