diff --git a/ceres/src/api_service/mono_api_service.rs b/ceres/src/api_service/mono_api_service.rs index 0328ec998..9ca06e8c9 100644 --- a/ceres/src/api_service/mono_api_service.rs +++ b/ceres/src/api_service/mono_api_service.rs @@ -44,6 +44,7 @@ use async_trait::async_trait; use callisto::sea_orm_active_enums::ConvTypeEnum; use callisto::{mega_blob, mega_mr, mega_tree, raw_blob}; use common::errors::MegaError; +use common::model::Pagination; use jupiter::storage::base_storage::StorageConnector; use jupiter::storage::Storage; use jupiter::utils::converter::generate_git_keep_with_timestamp; @@ -52,11 +53,12 @@ use mercury::hash::SHA1; use mercury::internal::object::blob::Blob; use mercury::internal::object::commit::Commit; use mercury::internal::object::tree::{Tree, TreeItem, TreeItemMode}; +use neptune::model::diff_model::DiffItem; use neptune::neptune_engine::Diff; use crate::api_service::ApiHandler; use crate::model::git::CreateFileInfo; -use crate::model::mr::{MrDiff, MrDiffFile, MrPageInfo}; +use crate::model::mr::MrDiffFile; #[derive(Clone)] pub struct MonoApiService { @@ -371,6 +373,27 @@ impl MonoApiService { Ok(p_commit_id) } + pub async fn content_diff(&self, mr_link: &str) -> Result, GitError> { + let stg = self.storage.mr_storage(); + let mr = + stg.get_mr(mr_link).await.unwrap().ok_or_else(|| { + GitError::CustomError(format!("Merge request not found: {mr_link}")) + })?; + + let old_blobs = self + .get_commit_blobs(&mr.from_hash) + .await + .map_err(|e| GitError::CustomError(format!("Failed to get old commit blobs: {e}")))?; + let new_blobs = self + .get_commit_blobs(&mr.to_hash) + .await + .map_err(|e| GitError::CustomError(format!("Failed to get new commit blobs: {e}")))?; + + let diff_output = self.get_diff_by_blobs(old_blobs, new_blobs).await?; + + Ok(diff_output) + } + /// Fetches the content difference for a merge request, paginated by page_id and page_size. /// # Arguments /// * `mr_link` - The link to the merge request. @@ -378,12 +401,14 @@ impl MonoApiService { /// * `page_size` - The number of items per page. /// # Returns /// a `Result` containing `MrDiff` on success or a `GitError` on failure. - pub async fn content_diff( + pub async fn paged_content_diff( &self, mr_link: &str, - page_id: usize, - page_size: usize, - ) -> Result { + page: Pagination, + ) -> Result<(Vec, u64), GitError> { + let per_page = page.per_page as usize; + let page_id = page.page as usize; + // old and new blobs for comparison let stg = self.storage.mr_storage(); let mr = @@ -405,8 +430,8 @@ impl MonoApiService { .await?; // ensure page_id is within bounds - let start = (page_id.saturating_sub(1)) * page_size; - let end = (start + page_size).min(sorted_changed_files.len()); + let start = (page_id.saturating_sub(1)) * per_page; + let end = (start + per_page).min(sorted_changed_files.len()); let page_slice: &[MrDiffFile] = if start < sorted_changed_files.len() { let start_idx = start; @@ -421,6 +446,23 @@ impl MonoApiService { let mut page_new_blobs = Vec::new(); self.collect_page_blobs(page_slice, &mut page_old_blobs, &mut page_new_blobs); + // get diff output + let diff_output = self + .get_diff_by_blobs(page_old_blobs, page_new_blobs) + .await + .map_err(|e| GitError::CustomError(format!("Failed to get diff output: {e}")))?; + + // calculate total pages + let total = sorted_changed_files.len().div_ceil(per_page); + + Ok((diff_output, total as u64)) + } + + async fn get_diff_by_blobs( + &self, + old_blobs: Vec<(PathBuf, SHA1)>, + new_blobs: Vec<(PathBuf, SHA1)>, + ) -> Result, GitError> { let mut blob_cache: HashMap> = HashMap::new(); // Collect all unique hashes @@ -432,41 +474,55 @@ impl MonoApiService { all_hashes.insert(*hash); } - // Fetch all blobs concurrently + // Fetch all blobs with better error handling and logging + let mut failed_hashes = Vec::new(); for hash in all_hashes { match self.get_raw_blob_by_hash(&hash.to_string()).await { Ok(Some(blob)) => { blob_cache.insert(hash, blob.data.unwrap_or_default()); } - _ => { + Ok(None) => { + tracing::warn!("Blob not found for hash: {}", hash); + blob_cache.insert(hash, Vec::new()); + } + Err(e) => { + tracing::error!("Failed to fetch blob {}: {}", hash, e); + failed_hashes.push(hash); blob_cache.insert(hash, Vec::new()); } } } - // Simple synchronous closure that uses the pre-fetched cache - let read_content = |_file: &PathBuf, hash: &SHA1| -> Vec { - blob_cache.get(hash).cloned().unwrap_or_default() + if !failed_hashes.is_empty() { + tracing::warn!( + "Failed to fetch {} blob(s): {:?}", + failed_hashes.len(), + failed_hashes + ); + } + + // Enhanced content reader with better error handling + let read_content = |file: &PathBuf, hash: &SHA1| -> Vec { + match blob_cache.get(hash) { + Some(content) => content.clone(), + None => { + tracing::warn!("Missing blob content for file: {:?}, hash: {}", file, hash); + Vec::new() + } + } }; - // Use the unified diff function that returns a single string + // Use the unified diff function with configurable algorithm let diff_output = Diff::diff( - page_old_blobs, - page_new_blobs, + old_blobs, + new_blobs, "histogram".to_string(), Vec::new(), read_content, ) .await; - Ok(MrDiff { - data: diff_output, - page_info: Some(MrPageInfo { - total_pages: (sorted_changed_files.len() - 1).div_ceil(page_size), - current_page: page_id, - page_size, - }), - }) + Ok(diff_output) } fn collect_page_blobs( @@ -573,7 +629,7 @@ impl MonoApiService { #[cfg(test)] mod test { use super::*; - use crate::model::mr::{MrDiffFile, MrPageInfo}; + use crate::model::mr::MrDiffFile; use mercury::hash::SHA1; use std::path::PathBuf; use std::str::FromStr; @@ -798,20 +854,18 @@ mod test { } #[test] - fn test_paging_page_info_construction() { + fn test_paging_algorithm() { let total_files = 10usize; let current_page = 2u32; let page_size = 3u32; - let page_info = MrPageInfo { - total_pages: (total_files + page_size as usize - 1) / page_size as usize, - current_page: current_page as usize, - page_size: page_size as usize, - }; + let total_pages = (total_files + page_size as usize - 1) / page_size as usize; + let current_page = current_page as usize; + let page_size = page_size as usize; - assert_eq!(page_info.total_pages, 4); - assert_eq!(page_info.current_page, 2); - assert_eq!(page_info.page_size, 3); + assert_eq!(total_pages, 4); + assert_eq!(current_page, 2); + assert_eq!(page_size, 3); } #[test] @@ -914,4 +968,100 @@ mod test { assert_eq!(new_blobs[0].0, PathBuf::from("new.txt")); assert_eq!(new_blobs[1].0, PathBuf::from("modified.txt")); } + + #[tokio::test] + async fn test_content_diff_functionality() { + use mercury::internal::object::blob::Blob; + use std::collections::HashMap; + + // Test basic diff generation with sample data + let old_content = "Hello World\nLine 2\nLine 3"; + let new_content = "Hello Universe\nLine 2\nLine 3 modified"; + + let old_blob = Blob::from_content(old_content); + let new_blob = Blob::from_content(new_content); + + let old_blobs = vec![(PathBuf::from("test_file.txt"), old_blob.id)]; + let new_blobs = vec![(PathBuf::from("test_file.txt"), new_blob.id)]; + + // Create a blob cache for the test + let mut blob_cache: HashMap> = HashMap::new(); + blob_cache.insert(old_blob.id, old_content.as_bytes().to_vec()); + blob_cache.insert(new_blob.id, new_content.as_bytes().to_vec()); + + // Test the diff engine directly + let read_content = |_file: &PathBuf, hash: &SHA1| -> Vec { + blob_cache.get(hash).cloned().unwrap_or_default() + }; + + let diff_output = Diff::diff( + old_blobs, + new_blobs, + "histogram".to_string(), + Vec::new(), + read_content, + ) + .await; + + // Verify diff output contains expected content + assert!(!diff_output.is_empty(), "Diff output should not be empty"); + assert_eq!(diff_output.len(), 1, "Should have diff for one file"); + + let diff_item = &diff_output[0]; + assert_eq!(diff_item.path, "test_file.txt"); + assert!( + diff_item.data.contains("diff --git"), + "Should contain git diff header" + ); + assert!( + diff_item.data.contains("-Hello World"), + "Should show removed line" + ); + assert!( + diff_item.data.contains("+Hello Universe"), + "Should show added line" + ); + assert!(diff_item.data.contains("-Line 3"), "Should show old line 3"); + assert!( + diff_item.data.contains("+Line 3 modified"), + "Should show new line 3" + ); + } + + #[tokio::test] + async fn test_get_diff_by_blobs_with_empty_content() { + // Test diff generation with empty content (simulating missing blobs) + let old_hash = SHA1::from_str("1234567890123456789012345678901234567890").unwrap(); + let new_hash = SHA1::from_str("abcdefabcdefabcdefabcdefabcdefabcdefabcd").unwrap(); + + let old_blobs = vec![(PathBuf::from("empty_file.txt"), old_hash)]; + let new_blobs = vec![(PathBuf::from("empty_file.txt"), new_hash)]; + + // Create empty blob cache to simulate missing blobs + let blob_cache: HashMap> = HashMap::new(); + + let read_content = |_file: &PathBuf, hash: &SHA1| -> Vec { + blob_cache.get(hash).cloned().unwrap_or_default() + }; + + // Test the diff engine with empty content + let diff_output = Diff::diff( + old_blobs, + new_blobs, + "histogram".to_string(), + Vec::new(), + read_content, + ) + .await; + + assert!( + !diff_output.is_empty(), + "Should generate diff even with empty blobs" + ); + assert_eq!(diff_output[0].path, "empty_file.txt"); + assert!( + diff_output[0].data.contains("diff --git"), + "Should contain git diff header" + ); + } } diff --git a/ceres/src/model/mr.rs b/ceres/src/model/mr.rs index 5b6814572..2a329dbe6 100644 --- a/ceres/src/model/mr.rs +++ b/ceres/src/model/mr.rs @@ -1,7 +1,6 @@ use std::path::PathBuf; use serde::{Deserialize, Serialize}; -use utoipa::ToSchema; use mercury::hash::SHA1; @@ -31,19 +30,6 @@ impl MrDiffFile { } } -#[derive(Debug, ToSchema, Serialize, Deserialize)] -pub struct MrDiff { - pub data: String, - pub page_info: Option, -} - -#[derive(Debug, ToSchema, Serialize, Deserialize)] -pub struct MrPageInfo { - pub total_pages: usize, - pub current_page: usize, - pub page_size: usize, -} - #[derive(Serialize)] pub struct BuckFile { pub buck: SHA1, diff --git a/common/src/model.rs b/common/src/model.rs index c1eb6096f..38af77d8e 100644 --- a/common/src/model.rs +++ b/common/src/model.rs @@ -77,7 +77,6 @@ pub struct PageParams { } #[derive(PartialEq, Eq, Debug, Clone, Default, Serialize, Deserialize, ToSchema)] - pub struct CommonPage { pub total: u64, pub items: Vec, diff --git a/libra/src/command/diff.rs b/libra/src/command/diff.rs index 164af7df7..11b95efba 100644 --- a/libra/src/command/diff.rs +++ b/libra/src/command/diff.rs @@ -147,10 +147,12 @@ pub async fn execute(args: DiffArgs) { ) .await; + let results: Vec = diff_output.iter().map(|i| i.data.clone()).collect(); + // Handle output - libra processes the string according to its needs match w { Some(ref mut file) => { - file.write_all(diff_output.as_bytes()).unwrap(); + file.write_all(results.join("").as_bytes()).unwrap(); } None => { #[cfg(unix)] @@ -162,7 +164,7 @@ pub async fn execute(args: DiffArgs) { .spawn() .expect("failed to execute process"); let stdin = child.stdin.as_mut().unwrap(); - stdin.write_all(diff_output.as_bytes()).unwrap(); + stdin.write_all(results.join("").as_bytes()).unwrap(); child.wait().unwrap(); } #[cfg(not(unix))] diff --git a/mono/Cargo.toml b/mono/Cargo.toml index 9623fbb84..3010e40b2 100644 --- a/mono/Cargo.toml +++ b/mono/Cargo.toml @@ -59,6 +59,7 @@ utoipa = { workspace = true, features = ["axum_extras"] } utoipa-axum = { workspace = true } utoipa-swagger-ui = { workspace = true, features = ["axum"] } uuid = { workspace = true, features = ["v4"] } +neptune = { workspace = true } diff --git a/mono/src/api/mr/mod.rs b/mono/src/api/mr/mod.rs index 5b603b04b..4990e5081 100644 --- a/mono/src/api/mr/mod.rs +++ b/mono/src/api/mr/mod.rs @@ -1,14 +1,15 @@ use std::str::FromStr; -use ceres::model::mr::{MrDiff, MrDiffFile}; +use ceres::model::mr::MrDiffFile; use jupiter::model::mr_dto::MRDetails; use serde::{Deserialize, Serialize}; use utoipa::ToSchema; use uuid::Uuid; -use callisto::sea_orm_active_enums::MergeStatusEnum; - use crate::api::{conversation::ConversationItem, label::LabelItem}; +use callisto::sea_orm_active_enums::MergeStatusEnum; +use common::model::CommonPage; +use neptune::model::diff_model::DiffItem; pub mod mr_router; @@ -52,7 +53,13 @@ impl From for MRDetailRes { #[derive(Serialize, ToSchema)] pub struct FilesChangedList { pub mui_trees: Vec, - pub content: MrDiff, + pub content: Vec, +} + +#[derive(Serialize, ToSchema)] +pub struct FilesChangedPage { + pub mui_trees: Vec, + pub page: CommonPage, } #[derive(Serialize, Debug, ToSchema)] diff --git a/mono/src/api/mr/mr_router.rs b/mono/src/api/mr/mr_router.rs index 93b04c18d..61d16a218 100644 --- a/mono/src/api/mr/mr_router.rs +++ b/mono/src/api/mr/mr_router.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, path::PathBuf}; +use std::path::PathBuf; use axum::{ extract::{Path, State}, @@ -13,7 +13,6 @@ use common::{ model::{CommonPage, CommonResult, PageParams}, }; -use crate::api::MonoApiServiceState; use crate::api::{ api_common::{ self, @@ -25,6 +24,7 @@ use crate::api::{ mr::{FilesChangedList, MRDetailRes, MrFilesRes, MuiTreeNode}, oauth::model::LoginUser, }; +use crate::api::{mr::FilesChangedPage, MonoApiServiceState}; use crate::{api::error::ApiError, server::http_server::MR_TAG}; pub fn routers() -> OpenApiRouter { @@ -37,6 +37,7 @@ pub fn routers() -> OpenApiRouter { .routes(routes!(merge_no_auth)) .routes(routes!(close_mr)) .routes(routes!(reopen_mr)) + .routes(routes!(mr_files_changed_by_page)) .routes(routes!(mr_files_changed)) .routes(routes!(mr_files_list)) .routes(routes!(save_comment)) @@ -257,13 +258,13 @@ async fn mr_detail( Ok(Json(CommonResult::success(Some(mr_details)))) } -/// Get Merge Request file changed list +/// Get List of All Changed Files in Merge Request #[utoipa::path( get, params( ("link", description = "MR link"), ), - path = "/{link}/files-changed/{page_id}/{page_size}", + path = "/{link}/files-changed", responses( (status = 200, body = CommonResult, content_type = "application/json") ), @@ -271,20 +272,11 @@ async fn mr_detail( )] async fn mr_files_changed( Path(link): Path, - Path(page_id): Path, - Path(page_size): Path, state: State, ) -> Result>, ApiError> { - let diff_res = state - .monorepo() - .content_diff(&link, page_id, page_size) - .await?; + let diff_res = state.monorepo().content_diff(&link).await?; - let diff_files = extract_files_with_status(&diff_res.data); - let mut paths = vec![]; - for (path, _) in diff_files { - paths.push(path); - } + let paths = diff_res.iter().map(|i| i.path.clone()).collect(); let mui_trees = build_forest(paths); let res = CommonResult::success(Some(FilesChangedList { mui_trees, @@ -293,6 +285,39 @@ async fn mr_files_changed( Ok(Json(res)) } +/// Get Merge Request file changed list in Pagination +#[utoipa::path( + post, + params( + ("link", description = "MR link"), + ), + path = "/{link}/files-changed", + request_body = PageParams, + responses( + (status = 200, body = CommonResult, content_type = "application/json") + ), + tag = MR_TAG +)] +#[axum::debug_handler] +async fn mr_files_changed_by_page( + Path(link): Path, + state: State, + Json(json): Json>, +) -> Result>, ApiError> { + let (items, total) = state + .monorepo() + .paged_content_diff(&link, json.pagination) + .await?; + + let paths = items.iter().map(|i| i.path.clone()).collect(); + let mui_trees = build_forest(paths); + let res = CommonResult::success(Some(FilesChangedPage { + mui_trees, + page: CommonPage { total, items }, + })); + Ok(Json(res)) +} + /// Get Merge Request file list #[utoipa::path( get, @@ -388,26 +413,6 @@ async fn edit_title( Ok(Json(CommonResult::success(None))) } -fn extract_files_with_status(diff_output: &str) -> HashMap { - let mut files = HashMap::new(); - - let chunks: Vec<&str> = diff_output.split("diff --git ").collect(); - - for chunk in chunks.iter().skip(1) { - let lines: Vec<&str> = chunk.split_whitespace().collect(); - if lines.len() >= 2 { - let current_file = lines[0].trim_start_matches("a/").to_string(); - files.insert(current_file.clone(), "modified".to_string()); // 默认状态为修改 - if chunk.contains("new file mode") { - files.insert(current_file, "new".to_string()); - } else if chunk.contains("deleted file mode") { - files.insert(current_file, "deleted".to_string()); - } - } - } - files -} - /// Update mr related labels #[utoipa::path( post, @@ -468,11 +473,31 @@ fn build_forest(paths: Vec) -> Vec { #[cfg(test)] mod test { - use crate::api::mr::mr_router::{build_forest, extract_files_with_status}; + use crate::api::mr::mr_router::build_forest; use crate::api::mr::FilesChangedList; - use ceres::model::mr::MrDiff; + use neptune::model::diff_model::DiffItem; use std::collections::HashMap; + fn extract_files_with_status(diff_output: &str) -> HashMap { + let mut files = HashMap::new(); + + let chunks: Vec<&str> = diff_output.split("diff --git ").collect(); + + for chunk in chunks.iter().skip(1) { + let lines: Vec<&str> = chunk.split_whitespace().collect(); + if lines.len() >= 2 { + let current_file = lines[0].trim_start_matches("a/").to_string(); + files.insert(current_file.clone(), "modified".to_string()); // 默认状态为修改 + if chunk.contains("new file mode") { + files.insert(current_file, "new".to_string()); + } else if chunk.contains("deleted file mode") { + files.insert(current_file, "deleted".to_string()); + } + } + } + files + } + #[test] fn test_parse_diff_result_to_filelist() { let diff_output = r#" @@ -572,16 +597,19 @@ mod test { assert!(root_labels.contains(&"src")); assert!(root_labels.contains(&"README.md")); - let content = MrDiff { + let content = vec![DiffItem { data: sample_diff_output.to_string(), - page_info: None, - }; + path: "diff_output.txt".to_string(), + }]; // Test the complete response structure let files_changed_list = FilesChangedList { mui_trees, content }; assert!(!files_changed_list.mui_trees.is_empty()); - assert_eq!(files_changed_list.content.data, sample_diff_output); + assert_eq!( + files_changed_list.content.first().unwrap().data, + sample_diff_output + ); } #[test] diff --git a/neptune/Cargo.toml b/neptune/Cargo.toml index 2bbc27c7b..e7ebe6c58 100644 --- a/neptune/Cargo.toml +++ b/neptune/Cargo.toml @@ -7,4 +7,6 @@ edition = "2021" infer = { workspace = true } mercury = { workspace = true } path-absolutize = { workspace = true } -tracing = { workspace = true } \ No newline at end of file +tracing = { workspace = true } +utoipa = { workspace = true } +serde = { workspace = true } \ No newline at end of file diff --git a/neptune/src/lib.rs b/neptune/src/lib.rs index 66cde9ad5..14e289d86 100644 --- a/neptune/src/lib.rs +++ b/neptune/src/lib.rs @@ -1,3 +1,4 @@ +pub mod model; pub mod neptune_engine; pub use neptune_engine::Diff; diff --git a/neptune/src/model/diff_model.rs b/neptune/src/model/diff_model.rs new file mode 100644 index 000000000..4dfec434d --- /dev/null +++ b/neptune/src/model/diff_model.rs @@ -0,0 +1,8 @@ +use serde::Serialize; +use utoipa::ToSchema; + +#[derive(Serialize, ToSchema)] +pub struct DiffItem { + pub path: String, + pub data: String, +} diff --git a/neptune/src/model/mod.rs b/neptune/src/model/mod.rs new file mode 100644 index 000000000..b7235a1fe --- /dev/null +++ b/neptune/src/model/mod.rs @@ -0,0 +1 @@ +pub mod diff_model; diff --git a/neptune/src/neptune_engine.rs b/neptune/src/neptune_engine.rs index 656ddb96f..16626603d 100644 --- a/neptune/src/neptune_engine.rs +++ b/neptune/src/neptune_engine.rs @@ -1,3 +1,4 @@ +use crate::model::diff_model::DiffItem; use infer; use mercury::hash::SHA1; use path_absolutize::Absolutize; @@ -47,7 +48,7 @@ impl Diff { algorithm: String, filter: Vec, read_content: F, - ) -> String + ) -> Vec where F: Fn(&PathBuf, &SHA1) -> Vec, { @@ -59,7 +60,10 @@ impl Diff { if let Some(large_file_marker) = Self::is_large_file(&file, &old_blobs_map, &new_blobs_map, &read_content) { - diff_results.push(large_file_marker); + diff_results.push(DiffItem { + path: file.to_string_lossy().to_string(), + data: large_file_marker, + }); } else { let diff = Self::diff_for_file_string( &file, @@ -68,11 +72,14 @@ impl Diff { algorithm.as_str(), &read_content, ); - diff_results.push(diff); + diff_results.push(DiffItem { + path: file.to_string_lossy().to_string(), + data: diff, + }); } } - diff_results.join("") + diff_results } /// Checks if a file is large and returns a message if it is.