maurice/snix
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor(tvix/castore/blobsvc): remove BlobService::from_url

Browse source 
Make blobservice::from_addr use the more specific constructors.

Change-Id: Id9637e279d6910ce6d92ff0086a984be5c65a8c8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10028
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
This commit is contained in:
Florian Klink 2023-11-13 13:41:16 +02:00 • committed by flokli
parent 25eca8af32
commit 840bee1e97
5 changed files with 98 additions and 241 deletions
Download patch file Download diff file Expand all files Collapse all files

102
tvix/castore/src/blobservice/sled.rs
View file

@ -31,34 +31,6 @@ impl SledBlobService {
#[async_trait]
impl BlobService for SledBlobService {
/// Constructs a [SledBlobService] 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 `/`).
fn from_url(url: &url::Url) -> Result<Self, Error> {
if url.scheme() != "sled" {
return Err(crate::Error::StorageError("invalid scheme".to_string()));
}
if url.has_host() {
return Err(crate::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().map_err(|e| Error::StorageError(e.to_string()))
} else if url.path() == "/" {
Err(crate::Error::StorageError(
"cowardly refusing to open / with sled".to_string(),
))
} else {
Self::new(url.path().into()).map_err(|e| Error::StorageError(e.to_string()))
}
}
#[instrument(skip(self), fields(blob.digest=%digest))]
async fn has(&self, digest: &B3Digest) -> Result<bool, Error> {
match self.db.contains_key(digest.as_slice()) {
@ -173,77 +145,3 @@ impl BlobWriter for SledBlobWriter {
}
}
}
#[cfg(test)]
mod tests {
use tempfile::TempDir;
use super::BlobService;
use super::SledBlobService;
/// This uses a wrong scheme.
#[test]
fn test_invalid_scheme() {
let url = url::Url::parse("http://foo.example/test").expect("must parse");
assert!(SledBlobService::from_url(&url).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!(SledBlobService::from_url(&url).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!(SledBlobService::from_url(&url).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!(SledBlobService::from_url(&url).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!(SledBlobService::from_url(&url).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!(SledBlobService::from_url(&url).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!(SledBlobService::from_url(&url).is_err());
}
}