From f57fd16d6efd1027b3b3b45ca3fdb03573119da6 Mon Sep 17 00:00:00 2001
From: Florian Klink <flokli@flokli.de>
Date: Mon, 13 Nov 2023 12:10:30 +0200
Subject: refactor(tvix/store/pathinfosvc): inline GRPCPathInfoSvc::from_url

Change-Id: Ib53b5525ae13c276e61b7f564673b7c6144ffc0e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10025
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
---
 tvix/store/src/pathinfoservice/from_addr.rs | 25 +++++++++++++++++++-----
 tvix/store/src/pathinfoservice/grpc.rs      | 30 +++++++----------------------
 2 files changed, 27 insertions(+), 28 deletions(-)

(limited to 'tvix/store/src')

diff --git a/tvix/store/src/pathinfoservice/from_addr.rs b/tvix/store/src/pathinfoservice/from_addr.rs
index 96a533e290fd..6cace25deaec 100644
--- a/tvix/store/src/pathinfoservice/from_addr.rs
+++ b/tvix/store/src/pathinfoservice/from_addr.rs
@@ -1,3 +1,5 @@
+use crate::proto::path_info_service_client::PathInfoServiceClient;
+
 use super::{GRPCPathInfoService, MemoryPathInfoService, PathInfoService, SledPathInfoService};
 
 use std::sync::Arc;
@@ -42,11 +44,13 @@ pub fn from_addr(
             directory_service,
         )?)
     } else if url.scheme().starts_with("grpc+") {
-        Arc::new(GRPCPathInfoService::from_url(
-            &url,
-            blob_service,
-            directory_service,
-        )?)
+        // schemes starting with grpc+ go to the GRPCPathInfoService.
+        //   That's normally grpc+unix for unix sockets, and grpc+http(s) for the HTTP counterparts.
+        // - In the case of unix sockets, there must be a path, but may not be a host.
+        // - In the case of non-unix sockets, there must be a host, but no path.
+        // Constructing the channel is handled by tvix_castore::channel::from_url.
+        let client = PathInfoServiceClient::new(tvix_castore::channel::from_url(&url)?);
+        Arc::new(GRPCPathInfoService::from_client(client))
     } else {
         Err(Error::StorageError(format!(
             "unknown scheme: {}",
@@ -95,4 +99,15 @@ mod tests {
     fn memory_invalid_path2() {
         assert!(from_addr("memory:///foo", gen_blob_service(), gen_directory_service()).is_err())
     }
+
+    #[tokio::test]
+    /// This constructs a GRPC PathInfoService.
+    async fn grpc_valid() {
+        assert!(from_addr(
+            "grpc+http://[::1]:12345",
+            gen_blob_service(),
+            gen_directory_service()
+        )
+        .is_ok())
+    }
 }
diff --git a/tvix/store/src/pathinfoservice/grpc.rs b/tvix/store/src/pathinfoservice/grpc.rs
index a33f75c1fdc9..2e58a464aa46 100644
--- a/tvix/store/src/pathinfoservice/grpc.rs
+++ b/tvix/store/src/pathinfoservice/grpc.rs
@@ -2,11 +2,9 @@ use super::PathInfoService;
 use crate::proto::{self, ListPathInfoRequest, PathInfo};
 use async_stream::try_stream;
 use futures::Stream;
-use std::{pin::Pin, sync::Arc};
+use std::pin::Pin;
 use tonic::{async_trait, transport::Channel, Code};
-use tvix_castore::{
-    blobservice::BlobService, directoryservice::DirectoryService, proto as castorepb, Error,
-};
+use tvix_castore::{proto as castorepb, Error};
 
 /// Connects to a (remote) tvix-store PathInfoService over gRPC.
 #[derive(Clone)]
@@ -24,23 +22,6 @@ impl GRPCPathInfoService {
     ) -> Self {
         Self { grpc_client }
     }
-
-    /// Constructs a [GRPCPathInfoService] from the passed [url::Url]:
-    /// - scheme has to match `grpc+*://`.
-    ///   That's normally grpc+unix for unix sockets, and grpc+http(s) for the HTTP counterparts.
-    /// - In the case of unix sockets, there must be a path, but may not be a host.
-    /// - In the case of non-unix sockets, there must be a host, but no path.
-    /// The blob_service and directory_service arguments are ignored, because the gRPC service already provides answers to these questions.
-    pub fn from_url(
-        url: &url::Url,
-        _blob_service: Arc<dyn BlobService>,
-        _directory_service: Arc<dyn DirectoryService>,
-    ) -> Result<Self, tvix_castore::Error> {
-        let channel = tvix_castore::channel::from_url(url)?;
-        Ok(Self::from_client(
-            proto::path_info_service_client::PathInfoServiceClient::new(channel),
-        ))
-    }
 }
 
 #[async_trait]
@@ -198,8 +179,11 @@ mod tests {
         let grpc_client = {
             let url = url::Url::parse(&format!("grpc+unix://{}", socket_path.display()))
                 .expect("must parse");
-            GRPCPathInfoService::from_url(&url, gen_blob_service(), gen_directory_service())
-                .expect("must succeed")
+            let client = PathInfoServiceClient::new(
+                tvix_castore::channel::from_url(&url).expect("must succeed"),
+            );
+
+            GRPCPathInfoService::from_client(client)
         };
 
         let path_info = grpc_client
-- 
cgit 1.4.1