feat(server): add tracing spans and caching to archive and blame services

- Add tracing spans with repo labels for archive and blame operations - Implement caching for archive list entries when using OID selectors - Implement caching for blame operations when using OID selectors - Add detailed
2026-06-04 15:33:16 +08:00
parent 729604f13b
commit cc202d6d1f
41 changed files with 2400 additions and 1067 deletions
@@ -3,10 +3,7 @@ mod common;
 use gitks::bare::GitBare;
 use gitks::pb::repository_service_server::RepositoryService;
 use gitks::pb::*;
-use gitks::server::GitksService;
-
 fn header(gb: &GitBare) -> RepositoryHeader {
-    let parent = gb.bare_dir.parent().unwrap().to_string_lossy().into_owned();
    let name = gb
        .bare_dir
        .file_name()
@@ -14,7 +11,6 @@ fn header(gb: &GitBare) -> RepositoryHeader {
        .to_string_lossy()
        .into_owned();
    RepositoryHeader {
-        storage_path: parent,
        relative_path: name,
        ..Default::default()
    }
@@ -26,8 +22,9 @@ fn req<T>(gb: &GitBare, f: impl FnOnce(RepositoryHeader) -> T) -> tonic::Request

 #[tokio::test]
 async fn test_get_repository() {
-    let (_dir, gb) = common::setup_bare_repo();
-    let repo = GitksService
+    let (dir, gb) = common::setup_bare_repo();
+    let svc = common::setup_service(dir.path());
+    let repo = svc
        .get_repository(req(&gb, |r| GetRepositoryRequest {
            repository: Some(r),
        }))
@@ -42,13 +39,11 @@ async fn test_get_repository() {
 #[tokio::test]
 async fn test_init_and_delete_repository() {
    let dir = tempfile::tempdir().unwrap();
-    let storage = dir.path().to_string_lossy().into_owned();
+    let svc = common::setup_service(dir.path());
    let hdr = RepositoryHeader {
-        storage_path: storage.clone(),
        relative_path: "new-repo".into(),
        ..Default::default()
    };
-    let svc = GitksService;
    svc.init_repository(tonic::Request::new(InitRepositoryRequest {
        repository: Some(hdr.clone()),
        bare: true,
@@ -83,8 +78,9 @@ async fn test_init_and_delete_repository() {

 #[tokio::test]
 async fn test_get_object_format() {
-    let (_dir, gb) = common::setup_bare_repo();
-    let resp = GitksService
+    let (dir, gb) = common::setup_bare_repo();
+    let svc = common::setup_service(dir.path());
+    let resp = svc
        .get_object_format(req(&gb, |r| RepositoryObjectFormatRequest {
            repository: Some(r),
        }))
@@ -96,53 +92,51 @@ async fn test_get_object_format() {

 #[tokio::test]
 async fn test_get_set_default_branch() {
-    let (_dir, gb) = common::setup_bare_repo();
+    let (dir, gb) = common::setup_bare_repo();
+    let svc = common::setup_service(dir.path());
    let h = header(&gb);
    assert_eq!(
-        GitksService
-            .get_default_branch(tonic::Request::new(GetDefaultBranchRequest {
-                repository: Some(h.clone())
-            }))
-            .await
-            .unwrap()
-            .into_inner()
-            .name,
-        "main"
-    );
-    GitksService
-        .set_default_branch(tonic::Request::new(SetDefaultBranchRequest {
-            repository: Some(h.clone()),
-            name: "feature".into(),
+        svc.get_default_branch(tonic::Request::new(GetDefaultBranchRequest {
+            repository: Some(h.clone())
        }))
        .await
-        .unwrap();
+        .unwrap()
+        .into_inner()
+        .name,
+        "main"
+    );
+    svc.set_default_branch(tonic::Request::new(SetDefaultBranchRequest {
+        repository: Some(h.clone()),
+        name: "feature".into(),
+    }))
+    .await
+    .unwrap();
    assert_eq!(
-        GitksService
-            .get_default_branch(tonic::Request::new(GetDefaultBranchRequest {
-                repository: Some(h)
-            }))
-            .await
-            .unwrap()
-            .into_inner()
-            .name,
+        svc.get_default_branch(tonic::Request::new(GetDefaultBranchRequest {
+            repository: Some(h)
+        }))
+        .await
+        .unwrap()
+        .into_inner()
+        .name,
        "feature"
    );
 }

 #[tokio::test]
 async fn test_get_set_repository_config() {
-    let (_dir, gb) = common::setup_bare_repo();
-    GitksService
-        .set_repository_config(tonic::Request::new(SetRepositoryConfigRequest {
-            repository: Some(header(&gb)),
-            entries: vec![RepositoryConfigEntry {
-                key: "test.key".into(),
-                values: vec!["val1".into(), "val2".into()],
-            }],
-        }))
-        .await
-        .unwrap();
-    let entry = GitksService
+    let (dir, gb) = common::setup_bare_repo();
+    let svc = common::setup_service(dir.path());
+    svc.set_repository_config(tonic::Request::new(SetRepositoryConfigRequest {
+        repository: Some(header(&gb)),
+        entries: vec![RepositoryConfigEntry {
+            key: "test.key".into(),
+            values: vec!["val1".into(), "val2".into()],
+        }],
+    }))
+    .await
+    .unwrap();
+    let entry = svc
        .get_repository_config(tonic::Request::new(GetRepositoryConfigRequest {
            repository: Some(header(&gb)),
            keys: vec!["test.key".into()],
@@ -159,8 +153,9 @@ async fn test_get_set_repository_config() {

 #[tokio::test]
 async fn test_get_repository_statistics() {
-    let (_dir, gb) = common::setup_bare_repo();
-    let s = GitksService
+    let (dir, gb) = common::setup_bare_repo();
+    let svc = common::setup_service(dir.path());
+    let s = svc
        .get_repository_statistics(req(&gb, |r| RepositoryStatisticsRequest {
            repository: Some(r),
        }))
@@ -174,8 +169,9 @@ async fn test_get_repository_statistics() {

 #[tokio::test]
 async fn test_check_repository_health() {
-    let (_dir, gb) = common::setup_bare_repo();
-    let h = GitksService
+    let (dir, gb) = common::setup_bare_repo();
+    let svc = common::setup_service(dir.path());
+    let h = svc
        .check_repository_health(tonic::Request::new(RepositoryHealthRequest {
            repository: Some(header(&gb)),
            connectivity_only: true,
@@ -188,48 +184,145 @@ async fn test_check_repository_health() {

 #[tokio::test]
 async fn test_garbage_collect() {
-    let (_dir, gb) = common::setup_bare_repo();
+    let (dir, gb) = common::setup_bare_repo();
+    let svc = common::setup_service(dir.path());
    assert!(
-        GitksService
-            .garbage_collect(tonic::Request::new(GarbageCollectRequest {
-                repository: Some(header(&gb)),
-                ..Default::default()
-            }))
-            .await
-            .unwrap()
-            .into_inner()
-            .ok
+        svc.garbage_collect(tonic::Request::new(GarbageCollectRequest {
+            repository: Some(header(&gb)),
+            ..Default::default()
+        }))
+        .await
+        .unwrap()
+        .into_inner()
+        .ok
    );
 }

 #[tokio::test]
 async fn test_repack() {
-    let (_dir, gb) = common::setup_bare_repo();
+    let (dir, gb) = common::setup_bare_repo();
+    let svc = common::setup_service(dir.path());
    assert!(
-        GitksService
-            .repack(tonic::Request::new(RepackRequest {
-                repository: Some(header(&gb)),
-                ..Default::default()
-            }))
-            .await
-            .unwrap()
-            .into_inner()
-            .ok
+        svc.repack(tonic::Request::new(RepackRequest {
+            repository: Some(header(&gb)),
+            ..Default::default()
+        }))
+        .await
+        .unwrap()
+        .into_inner()
+        .ok
    );
 }

 #[tokio::test]
 async fn test_write_commit_graph() {
-    let (_dir, gb) = common::setup_bare_repo();
+    let (dir, gb) = common::setup_bare_repo();
+    let svc = common::setup_service(dir.path());
    assert!(
-        GitksService
-            .write_commit_graph(tonic::Request::new(WriteCommitGraphRequest {
-                repository: Some(header(&gb)),
-                ..Default::default()
-            }))
-            .await
-            .unwrap()
-            .into_inner()
-            .ok
+        svc.write_commit_graph(tonic::Request::new(WriteCommitGraphRequest {
+            repository: Some(header(&gb)),
+            ..Default::default()
+        }))
+        .await
+        .unwrap()
+        .into_inner()
+        .ok
    );
 }
+
+#[tokio::test]
+async fn test_resolve_none_header() {
+    let dir = tempfile::tempdir().unwrap();
+    let svc = common::setup_service(dir.path());
+    let result = svc
+        .get_repository(tonic::Request::new(GetRepositoryRequest {
+            repository: None,
+        }))
+        .await;
+    assert!(result.is_err(), "should fail with None header");
+    let err = result.unwrap_err();
+    assert_eq!(err.code(), tonic::Code::InvalidArgument);
+}
+
+#[tokio::test]
+async fn test_resolve_empty_relative_path() {
+    let dir = tempfile::tempdir().unwrap();
+    let svc = common::setup_service(dir.path());
+    let result = svc
+        .get_repository(tonic::Request::new(GetRepositoryRequest {
+            repository: Some(RepositoryHeader {
+                relative_path: String::new(),
+                ..Default::default()
+            }),
+        }))
+        .await;
+    assert!(result.is_err(), "should fail with empty relative_path");
+}
+
+#[tokio::test]
+async fn test_resolve_nonexistent_repo() {
+    let dir = tempfile::tempdir().unwrap();
+    let svc = common::setup_service(dir.path());
+    let result = svc
+        .get_repository(tonic::Request::new(GetRepositoryRequest {
+            repository: Some(RepositoryHeader {
+                relative_path: "does-not-exist".into(),
+                ..Default::default()
+            }),
+        }))
+        .await;
+    assert!(result.is_err(), "should fail for nonexistent repo");
+}
+
+#[tokio::test]
+async fn test_init_empty_relative_path() {
+    let dir = tempfile::tempdir().unwrap();
+    let svc = common::setup_service(dir.path());
+    let result = svc
+        .init_repository(tonic::Request::new(InitRepositoryRequest {
+            repository: Some(RepositoryHeader {
+                relative_path: String::new(),
+                ..Default::default()
+            }),
+            bare: true,
+            ..Default::default()
+        }))
+        .await;
+    assert!(result.is_err(), "should fail with empty relative_path");
+}
+
+#[tokio::test]
+async fn test_delete_nonexistent_repo() {
+    let dir = tempfile::tempdir().unwrap();
+    let svc = common::setup_service(dir.path());
+    // Deleting a non-existent path should succeed (fs::remove_dir_all on non-existent is ok)
+    // or fail gracefully
+    let result = svc
+        .delete_repository(tonic::Request::new(DeleteRepositoryRequest {
+            repository: Some(RepositoryHeader {
+                relative_path: "ghost-repo".into(),
+                ..Default::default()
+            }),
+        }))
+        .await;
+    // It either succeeds (dir doesn't exist, nothing to delete) or fails
+    // Both are acceptable behaviors
+    let _ = result;
+}
+
+#[tokio::test]
+async fn test_exists_nonexistent_repo() {
+    let dir = tempfile::tempdir().unwrap();
+    let svc = common::setup_service(dir.path());
+    let result = svc
+        .repository_exists(tonic::Request::new(RepositoryExistsRequest {
+            repository: Some(RepositoryHeader {
+                relative_path: "nonexistent".into(),
+                ..Default::default()
+            }),
+        }))
+        .await
+        .unwrap()
+        .into_inner();
+    assert!(!result.exists);
+}