From 9aef3cfc8989c491db05c4d66cdbf3d89429420e Mon Sep 17 00:00:00 2001
From: Florian Klink <flokli@flokli.de>
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 <cbrewster@hey.com>
---
 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/src/pathinfoservice')

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<dyn BlobService>,
-        directory_service: Arc<dyn DirectoryService>,
-    ) -> Result<Self, Error> {
-        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