# HG changeset patch
# User Georges Racinet <georges.racinet@octobus.net>
# Date 1690283066 -7200
#      Tue Jul 25 13:04:26 2023 +0200
# Branch oldstable
# Node ID d7b89af65ac037a9991cdeb092683cff081a7269
# Parent  ebb08cf1049a06a6421099e278ffb1986246ff8c
rhgitaly::repository: new repo_store_vfs function

This function provides the path to the store vfs, taking care
of performing all checks, in particular that the repository
exists.

One advantage is the collapsing of the various problems (missing
repo specfication, repo not found...) into a single `RepoSpecError`,
so that callers can simply use `map_err()` and we don't need to
take an error treatment argument. The check for `None` spares us
a conditional block in the service implementation (kept an artificial
block to have a readable diff by avoiding reindent).

This is a new, simpler pattern, we'll see to generalize it.

diff --git a/rust/rhgitaly/src/repository.rs b/rust/rhgitaly/src/repository.rs
--- a/rust/rhgitaly/src/repository.rs
+++ b/rust/rhgitaly/src/repository.rs
@@ -5,9 +5,10 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 use std::fmt;
 use std::marker::Send;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
+use tokio::fs;
 use tokio::sync::mpsc;
 use tokio_stream::wrappers::ReceiverStream;
 use tonic::{Response, Status};
@@ -85,6 +86,51 @@
     Ok(res)
 }
 
+/// Default gRPC error ['Status'] for repository not found.
+///
+/// To be used if repository path does not exist on disk.
+pub fn default_repo_not_found_error_status(path: &Path) -> Status {
+    Status::not_found(format!(
+        "Mercurial repository at {} not found",
+        path.display()
+    ))
+}
+
+pub async fn checked_repo_path(
+    config: &Config,
+    gl_repo: &Option<Repository>,
+) -> Result<PathBuf, RepoSpecError> {
+    let repo = gl_repo
+        .as_ref()
+        .ok_or(RepoSpecError::MissingSpecification)?;
+    let path = repo_path(config, repo)?;
+    if match fs::metadata(&path).await {
+        Ok(md) => md.is_dir(),
+        Err(_) => false,
+    } {
+        return Ok(path);
+    }
+    Err(RepoSpecError::RepoNotFound(path))
+}
+
+/// Return a path to virtual filesystem for the repository store.
+///
+/// As of this writing, this is nothing but a [`Path`], but it could become something
+/// more advanced in the future (perhaps not as much as `hg_core` `Vfs` type, though).
+///
+/// Parameter `repo` is an `Option`, so that a service method can pass directly
+/// something like `&request.repository`, with `None` giving rise to the natural
+/// `MissingSpecification` error.
+///
+/// If the repository is not found on disc, the appropriate error is also returned.
+pub async fn repo_store_vfs(
+    config: &Config,
+    repo: &Option<Repository>,
+) -> Result<PathBuf, RepoSpecError> {
+    let root = checked_repo_path(config, repo).await?;
+    Ok(root.join(".hg/store"))
+}
+
 pub fn load_repo(config: &Config, opt_repo: Option<&Repository>) -> Result<Repo, RepoLoadError> {
     let repo = opt_repo.ok_or(RepoSpecError::MissingSpecification)?;
     // TODO load core config once and for all and store it in our Config.
diff --git a/rust/rhgitaly/src/service/commit.rs b/rust/rhgitaly/src/service/commit.rs
--- a/rust/rhgitaly/src/service/commit.rs
+++ b/rust/rhgitaly/src/service/commit.rs
@@ -1,6 +1,5 @@
 use std::fmt::{Debug, Formatter};
 use std::sync::Arc;
-use tokio::fs;
 use tokio_stream::wrappers::ReceiverStream;
 use tonic::{metadata::Ascii, metadata::MetadataValue, Request, Response, Status};
 use tracing::{field, info, instrument};
@@ -18,7 +17,7 @@
 use crate::message;
 use crate::metadata::correlation_id;
 use crate::repository::{
-    default_repo_spec_error_status, load_repo_and_stream, load_repo_and_then, repo_path,
+    default_repo_spec_error_status, load_repo_and_stream, load_repo_and_then, repo_store_vfs,
     RequestWithRepo,
 };
 use crate::streaming::ResultResponseStream;
@@ -163,27 +162,19 @@
             return Err(Status::invalid_argument("empty revision"));
         }
 
-        if let Some(gl_repo) = &request.repository {
-            let config = self.config.clone();
-            let path = repo_path(&config, gl_repo).map_err(default_repo_spec_error_status)?;
-            if !match fs::metadata(&path).await {
-                Ok(md) => md.is_dir(),
-                Err(_) => false,
-            } {
-                return Err(Status::not_found(format!(
-                    "Mercurial repository at {} not found",
-                    path.display()
-                )));
-            }
+        let config = self.config.clone();
+        {
+            let store_vfs = repo_store_vfs(&config, &request.repository)
+                .await
+                .map_err(default_repo_spec_error_status)?;
 
-            let store_path = path.join(".hg/store");
-            match gitlab_revision_node_prefix(&store_path, &request.revision)
+            match gitlab_revision_node_prefix(&store_vfs, &request.revision)
                 .await
                 .map_err(|e| Status::internal(format!("Error resolving revision: {:?}", e)))?
             {
                 None => {
                     info!("Revision not resolved");
-                    return Ok(Response::new(FindCommitResponse::default()));
+                    Ok(Response::new(FindCommitResponse::default()))
                 }
                 Some(node_prefix) => {
                     info!("Revision resolved as {:x}", &node_prefix);
@@ -214,13 +205,10 @@
                     )
                     .await?;
 
-                    return Ok(Response::new(FindCommitResponse { commit }));
+                    Ok(Response::new(FindCommitResponse { commit }))
                 }
-            };
+            }
         }
-        Err(Status::invalid_argument(
-            "GetStorageByName: no such storage: \"\"",
-        ))
     }
 }