refactor: Reduce code duplication with helper functions

Co-authored-by: aider (openai/andrew/openrouter/deepseek/deepseek-chat-v3.1) <aider@aider.chat>

src/modes/server/api/item.rs

@@ -10,6 +10,118 @@ use crate::services::async_item_service::AsyncItemService;
 use crate::services::error::CoreError;
 use crate::modes::server::common::{AppState, ApiResponse, ItemInfo, TagsQuery, ListItemsQuery, ItemInfoListResponse, ItemInfoResponse, MetadataResponse, ItemQuery, ItemContentQuery};
 
+/// Helper function to check if content is binary and handle the check
+async fn check_binary_content(
+    item_service: &AsyncItemService,
+    item_id: i64,
+    metadata: &HashMap<String, String>,
+    allow_binary: bool,
+) -> Result<(), StatusCode> {
+    if !allow_binary {
+        let is_binary = is_content_binary(item_service, item_id, metadata).await?;
+        if is_binary {
+            return Err(StatusCode::BAD_REQUEST);
+        }
+    }
+    Ok(())
+}
+
+/// Helper function to determine if content is binary
+async fn is_content_binary(
+    item_service: &AsyncItemService,
+    item_id: i64,
+    metadata: &HashMap<String, String>,
+) -> Result<bool, StatusCode> {
+    if let Some(text_val) = metadata.get("text") {
+        Ok(text_val == "false")
+    } else {
+        // If text metadata isn't set, we need to check the content using streaming approach
+        match item_service.get_item_content_info_streaming(
+            item_id,
+            None
+        ).await {
+            Ok((_, _, is_binary)) => Ok(is_binary),
+            Err(e) => {
+                warn!("Failed to get content info for binary check for item {}: {}", item_id, e);
+                Err(StatusCode::INTERNAL_SERVER_ERROR)
+            }
+        }
+    }
+}
+
+/// Helper function to build filter string from query parameters
+fn build_filter_string(params: &ItemQuery) -> Option<String> {
+    let mut filter_parts = Vec::new();
+    if let Some(head_bytes) = params.head_bytes {
+        filter_parts.push(format!("head_bytes({})", head_bytes));
+    }
+    if let Some(head_lines) = params.head_lines {
+        filter_parts.push(format!("head_lines({})", head_lines));
+    }
+    if let Some(tail_bytes) = params.tail_bytes {
+        filter_parts.push(format!("tail_bytes({})", tail_bytes));
+    }
+    if let Some(tail_lines) = params.tail_lines {
+        filter_parts.push(format!("tail_lines({})", tail_lines));
+    }
+    if let Some(grep) = params.grep {
+        filter_parts.push(format!("grep({})", grep));
+    }
+    
+    if filter_parts.is_empty() {
+        None
+    } else {
+        Some(filter_parts.join(" | "))
+    }
+}
+
+/// Helper function to get mime type from metadata
+fn get_mime_type(metadata: &HashMap<String, String>) -> String {
+    metadata
+        .get("mime_type")
+        .map(|s| s.to_string())
+        .unwrap_or_else(|| "application/octet-stream".to_string())
+}
+
+/// Helper function to apply offset and length to content
+fn apply_offset_length(content: &[u8], offset: u64, length: u64) -> &[u8] {
+    let content_len = content.len() as u64;
+    let start = std::cmp::min(offset, content_len);
+    let end = if length > 0 {
+        std::cmp::min(start + length, content_len)
+    } else {
+        content_len
+    };
+    
+    if start < content_len {
+        &content[start as usize..end as usize]
+    } else {
+        &[]
+    }
+}
+
+/// Helper function to handle item not found errors
+fn handle_item_error(error: CoreError) -> StatusCode {
+    match error {
+        CoreError::ItemNotFound(_) | CoreError::ItemNotFoundGeneric => StatusCode::NOT_FOUND,
+        _ => {
+            warn!("Failed to get item: {}", error);
+            StatusCode::INTERNAL_SERVER_ERROR
+        }
+    }
+}
+
+/// Helper function to create AsyncItemService from AppState
+fn create_item_service(state: &AppState) -> AsyncItemService {
+    AsyncItemService::new(
+        state.data_dir.clone(), 
+        state.db.clone(), 
+        state.item_service.clone(),
+        state.cmd.clone(),
+        state.settings.clone()
+    )
+}
+
 #[utoipa::path(
     get,
     path = "/api/item/",
@@ -42,13 +154,7 @@ pub async fn handle_list_items(
         .map(|s| s.split(',').map(|t| t.trim().to_string()).collect())
         .unwrap_or_default();
 
-    let item_service = AsyncItemService::new(
+    let item_service = create_item_service(&state);
-        state.data_dir.clone(), 
-        state.db.clone(), 
-        state.item_service.clone(),
-        state.cmd.clone(),
-        state.settings.clone()
-    );
     let mut items_with_meta = item_service
         .list_items(tags, HashMap::new())
         .await
@@ -138,21 +244,7 @@ async fn handle_as_meta_response_with_metadata(
     length: u64,
 ) -> Result<Response, StatusCode> {
     // Check if content is binary
-    let is_binary = if let Some(text_val) = metadata.get("text") {
+    let is_binary = is_content_binary(item_service, item_id, metadata).await?;
-        text_val == "false"
-    } else {
-        // If text metadata isn't set, we need to check the content using streaming approach
-        match item_service.get_item_content_info_streaming(
-            item_id,
-            None
-        ).await {
-            Ok((_, _, is_binary)) => is_binary,
-            Err(e) => {
-                warn!("Failed to get content info for binary check for item {}: {}", item_id, e);
-                return Err(StatusCode::INTERNAL_SERVER_ERROR);
-            }
-        }
-    };
     
     // Get the content if it's not binary
     if is_binary {
@@ -318,13 +410,7 @@ pub async fn handle_get_item_latest_content(
         .map(|s| s.split(',').map(|t| t.trim().to_string()).collect())
         .unwrap_or_default();
 
-    let item_service = AsyncItemService::new(
+    let item_service = create_item_service(&state);
-        state.data_dir.clone(), 
-        state.db.clone(), 
-        state.item_service.clone(),
-        state.cmd.clone(),
-        state.settings.clone()
-    );
 
     // First find the item to get its ID and metadata
     let item_with_meta = item_service
@@ -392,39 +478,9 @@ pub async fn handle_get_item_content(
     debug!("ITEM_API: Getting content for item {} with stream={}, allow_binary={}, offset={}, length={}", 
            item_id, params.stream, params.allow_binary, params.offset, params.length);
 
-    // Build filter string from query parameters
+    let filter = build_filter_string(&params);
-    let mut filter_parts = Vec::new();
-    if let Some(head_bytes) = params.head_bytes {
-        filter_parts.push(format!("head_bytes({})", head_bytes));
-    }
-    if let Some(head_lines) = params.head_lines {
-        filter_parts.push(format!("head_lines({})", head_lines));
-    }
-    if let Some(tail_bytes) = params.tail_bytes {
-        filter_parts.push(format!("tail_bytes({})", tail_bytes));
-    }
-    if let Some(tail_lines) = params.tail_lines {
-        filter_parts.push(format!("tail_lines({})", tail_lines));
-    }
-    if let Some(grep) = params.grep {
-        filter_parts.push(format!("grep({})", grep));
-    }
-    // Note: head_words, tail_words, line_start, line_end are not implemented in the filter system yet
-    // You may need to add them to the filter syntax or handle them differently
 
-    let filter = if filter_parts.is_empty() {
+    let item_service = create_item_service(&state);
-        None
-    } else {
-        Some(filter_parts.join(" | "))
-    };
-
-    let item_service = AsyncItemService::new(
-        state.data_dir.clone(), 
-        state.db.clone(), 
-        state.item_service.clone(),
-        state.cmd.clone(),
-        state.settings.clone()
-    );
     // Handle as_meta parameter
     if params.as_meta {
         // Force stream=false and allow_binary=false for as_meta=true
@@ -473,33 +529,10 @@ async fn stream_item_content_response_with_metadata(
     filter: Option<String>,
 ) -> Result<Response, StatusCode> {
     debug!("STREAM_ITEM_CONTENT_RESPONSE_WITH_METADATA: stream={}", stream);
-    let mime_type = metadata
+    let mime_type = get_mime_type(metadata);
-        .get("mime_type")
-        .map(|s| s.to_string())
-        .unwrap_or_else(|| "application/octet-stream".to_string());
     
     // Check if content is binary when allow_binary is false
-    if !allow_binary {
+    check_binary_content(item_service, item_id, metadata, allow_binary).await?;
-        let is_binary = if let Some(text_val) = metadata.get("text") {
-            text_val == "false"
-        } else {
-            // If text metadata isn't set, we need to check the content using streaming approach
-            match item_service.get_item_content_info_streaming(
-                item_id,
-                None
-            ).await {
-                Ok((_, _, is_binary)) => is_binary,
-                Err(e) => {
-                    warn!("Failed to get content info for binary check for item {}: {}", item_id, e);
-                    return Err(StatusCode::INTERNAL_SERVER_ERROR);
-                }
-            }
-        };
-        
-        if is_binary {
-            return Err(StatusCode::BAD_REQUEST);
-        }
-    }
     
     if stream {
         debug!("STREAMING: Using streaming approach");
@@ -531,23 +564,10 @@ async fn stream_item_content_response_with_metadata(
             filter
         ).await {
             Ok((content, _, _)) => {
-                // Apply offset and length
+                let response_content = apply_offset_length(&content, offset, length);
-                let content_len = content.len() as u64;
-                let start = std::cmp::min(offset, content_len);
-                let end = if length > 0 {
-                    std::cmp::min(start + length, content_len)
-                } else {
-                    content_len
-                };
                 
-                let response_content = if start < content_len {
+                debug!("NON-STREAMING: Content length: {}, response length: {}", 
-                    &content[start as usize..end as usize]
+                       content.len(), response_content.len());
-                } else {
-                    &[]
-                };
-                
-                debug!("NON-STREAMING: Content length: {}, start: {}, end: {}, response length: {}", 
-                       content_len, start, end, response_content.len());
                 
                 let response = Response::builder()
                     .header(header::CONTENT_TYPE, mime_type)
@@ -596,13 +616,7 @@ pub async fn handle_get_item_latest_meta(
         .map(|s| s.split(',').map(|t| t.trim().to_string()).collect())
         .unwrap_or_default();
 
-    let item_service = AsyncItemService::new(
+    let item_service = create_item_service(&state);
-        state.data_dir.clone(), 
-        state.db.clone(), 
-        state.item_service.clone(),
-        state.cmd.clone(),
-        state.settings.clone()
-    );
 
     match item_service.find_item(vec![], tags, HashMap::new()).await {
         Ok(item_with_meta) => {
@@ -616,11 +630,7 @@ pub async fn handle_get_item_latest_meta(
 
             Ok(Json(response))
         }
-        Err(CoreError::ItemNotFoundGeneric) => Err(StatusCode::NOT_FOUND),
+        Err(e) => Err(handle_item_error(e)),
-        Err(e) => {
-            warn!("Failed to get latest item for meta: {}", e);
-            Err(StatusCode::INTERNAL_SERVER_ERROR)
-        }
     }
 }
 
@@ -649,13 +659,7 @@ pub async fn handle_get_item_meta(
     State(state): State<AppState>,
     Path(item_id): Path<i64>,
 ) -> Result<Json<ApiResponse<HashMap<String, String>>>, StatusCode> {
-    let item_service = AsyncItemService::new(
+    let item_service = create_item_service(&state);
-        state.data_dir.clone(), 
-        state.db.clone(), 
-        state.item_service.clone(),
-        state.cmd.clone(),
-        state.settings.clone()
-    );
 
     match item_service.get_item(item_id).await {
         Ok(item_with_meta) => {
@@ -669,11 +673,7 @@ pub async fn handle_get_item_meta(
 
             Ok(Json(response))
         }
-        Err(CoreError::ItemNotFound(_)) => Err(StatusCode::NOT_FOUND),
+        Err(e) => Err(handle_item_error(e)),
-        Err(e) => {
-            warn!("Failed to get item {} for meta: {}", item_id, e);
-            Err(StatusCode::INTERNAL_SERVER_ERROR)
-        }
     }
 }