diff options
| -rw-r--r-- | tvix/store/src/pathinfoservice/from_addr.rs | 86 | ||||
| -rw-r--r-- | tvix/store/src/pathinfoservice/sled.rs | 131 |
2 files changed, 81 insertions, 136 deletions
diff --git a/tvix/store/src/pathinfoservice/from_addr.rs b/tvix/store/src/pathinfoservice/from_addr.rs index 6cace25deaec..97e0a0f36e24 100644 --- a/tvix/store/src/pathinfoservice/from_addr.rs +++ b/tvix/store/src/pathinfoservice/from_addr.rs @@ -38,11 +38,30 @@ pub fn from_addr( } Arc::new(MemoryPathInfoService::new(blob_service, directory_service)) } else if url.scheme() == "sled" { - Arc::new(SledPathInfoService::from_url( - &url, - blob_service, - directory_service, - )?) + // sled doesn't support host, and a path can be provided (otherwise + // it'll live in memory only). + if url.has_host() { + return Err(Error::StorageError("no host allowed".to_string())); + } + + if url.path() == "/" { + return Err(Error::StorageError( + "cowardly refusing to open / with sled".to_string(), + )); + } + + // TODO: expose compression and other parameters as URL parameters? + + if url.path().is_empty() { + return Ok(Arc::new( + SledPathInfoService::new_temporary(blob_service, directory_service) + .map_err(|e| Error::StorageError(e.to_string()))?, + )); + } + return Ok(Arc::new( + SledPathInfoService::new(url.path().into(), blob_service, directory_service) + .map_err(|e| Error::StorageError(e.to_string()))?, + )); } else if url.scheme().starts_with("grpc+") { // schemes starting with grpc+ go to the GRPCPathInfoService. // That's normally grpc+unix for unix sockets, and grpc+http(s) for the HTTP counterparts. @@ -61,6 +80,7 @@ pub fn from_addr( #[cfg(test)] mod tests { + use tempfile::TempDir; use tvix_castore::utils::{gen_blob_service, gen_directory_service}; use super::from_addr; @@ -100,6 +120,62 @@ mod tests { assert!(from_addr("memory:///foo", gen_blob_service(), gen_directory_service()).is_err()) } + /// This uses the correct scheme, and doesn't specify a path (temporary sled). + #[test] + fn sled_valid_temporary() { + assert!(from_addr("sled://", gen_blob_service(), gen_directory_service()).is_ok()) + } + + /// This uses the correct scheme, and sets a tempdir as location. + #[test] + fn sled_valid_scheme_path() { + let tmpdir = TempDir::new().unwrap(); + + let mut url = url::Url::parse("sled://").expect("must parse"); + url.set_path(tmpdir.path().to_str().unwrap()); + + assert!(from_addr( + &url.to_string(), + gen_blob_service(), + gen_directory_service() + ) + .is_ok()) + } + + /// This uses the correct scheme, and specifies / as path (which should fail + // for obvious reasons) + #[test] + fn sled_fail_invalid_path_root() { + assert!(from_addr("sled:///", gen_blob_service(), gen_directory_service()).is_err()) + } + + /// This sets a host, rather than a path, which should fail. + #[test] + fn sled_invalid_host() { + assert!(from_addr( + "sled://foo.example", + gen_blob_service(), + gen_directory_service() + ) + .is_err()) + } + + /// This sets a host AND a valid path, which should fail + #[test] + fn test_invalid_host_and_path() { + let tmpdir = TempDir::new().unwrap(); + + let mut url = url::Url::parse("sled://foo.example").expect("must parse"); + url.set_path(tmpdir.path().to_str().unwrap()); + + assert!(from_addr( + &url.to_string(), + gen_blob_service(), + gen_directory_service() + ) + .is_err()) + } + #[tokio::test] /// This constructs a GRPC PathInfoService. async fn grpc_valid() { diff --git a/tvix/store/src/pathinfoservice/sled.rs b/tvix/store/src/pathinfoservice/sled.rs index b389b14b95ad..a12f918c83cf 100644 --- a/tvix/store/src/pathinfoservice/sled.rs +++ b/tvix/store/src/pathinfoservice/sled.rs @@ -49,40 +49,6 @@ impl SledPathInfoService { directory_service, }) } - - /// Constructs a [SledPathInfoService] from the passed [url::Url]: - /// - scheme has to be `sled://` - /// - there may not be a host. - /// - a path to the sled needs to be provided (which may not be `/`). - pub fn from_url( - url: &url::Url, - blob_service: Arc<dyn BlobService>, - directory_service: Arc<dyn DirectoryService>, - ) -> Result<Self, Error> { - if url.scheme() != "sled" { - return Err(Error::StorageError("invalid scheme".to_string())); - } - - if url.has_host() { - return Err(Error::StorageError(format!( - "invalid host: {}", - url.host().unwrap() - ))); - } - - // TODO: expose compression and other parameters as URL parameters, drop new and new_temporary? - if url.path().is_empty() { - Self::new_temporary(blob_service, directory_service) - .map_err(|e| Error::StorageError(e.to_string())) - } else if url.path() == "/" { - Err(Error::StorageError( - "cowardly refusing to open / with sled".to_string(), - )) - } else { - Self::new(url.path().into(), blob_service, directory_service) - .map_err(|e| Error::StorageError(e.to_string())) - } - } } #[async_trait] @@ -172,100 +138,3 @@ impl PathInfoService for SledPathInfoService { }))) } } - -#[cfg(test)] -mod tests { - use tempfile::TempDir; - - use crate::tests::utils::gen_blob_service; - use crate::tests::utils::gen_directory_service; - - use super::SledPathInfoService; - - /// This uses a wrong scheme. - #[test] - fn test_invalid_scheme() { - let url = url::Url::parse("http://foo.example/test").expect("must parse"); - - assert!( - SledPathInfoService::from_url(&url, gen_blob_service(), gen_directory_service()) - .is_err() - ); - } - - /// This uses the correct scheme, and doesn't specify a path (temporary sled). - #[test] - fn test_valid_scheme_temporary() { - let url = url::Url::parse("sled://").expect("must parse"); - - assert!( - SledPathInfoService::from_url(&url, gen_blob_service(), gen_directory_service()) - .is_ok() - ); - } - - /// This sets the path to a location that doesn't exist, which should fail (as sled doesn't mkdir -p) - #[test] - fn test_nonexistent_path() { - let tmpdir = TempDir::new().unwrap(); - - let mut url = url::Url::parse("sled://foo.example").expect("must parse"); - url.set_path(tmpdir.path().join("foo").join("bar").to_str().unwrap()); - - assert!( - SledPathInfoService::from_url(&url, gen_blob_service(), gen_directory_service()) - .is_err() - ); - } - - /// This uses the correct scheme, and specifies / as path (which should fail - // for obvious reasons) - #[test] - fn test_invalid_path_root() { - let url = url::Url::parse("sled:///").expect("must parse"); - - assert!( - SledPathInfoService::from_url(&url, gen_blob_service(), gen_directory_service()) - .is_err() - ); - } - - /// This uses the correct scheme, and sets a tempdir as location. - #[test] - fn test_valid_scheme_path() { - let tmpdir = TempDir::new().unwrap(); - - let mut url = url::Url::parse("sled://").expect("must parse"); - url.set_path(tmpdir.path().to_str().unwrap()); - - assert!( - SledPathInfoService::from_url(&url, gen_blob_service(), gen_directory_service()) - .is_ok() - ); - } - - /// This sets a host, rather than a path, which should fail. - #[test] - fn test_invalid_host() { - let url = url::Url::parse("sled://foo.example").expect("must parse"); - - assert!( - SledPathInfoService::from_url(&url, gen_blob_service(), gen_directory_service()) - .is_err() - ); - } - - /// This sets a host AND a valid path, which should fail - #[test] - fn test_invalid_host_and_path() { - let tmpdir = TempDir::new().unwrap(); - - let mut url = url::Url::parse("sled://foo.example").expect("must parse"); - url.set_path(tmpdir.path().to_str().unwrap()); - - assert!( - SledPathInfoService::from_url(&url, gen_blob_service(), gen_directory_service()) - .is_err() - ); - } -} |