refactor: Consolidate binary detection, response building, and filter string logic

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

src/common/binary_detection.rs

@@ -0,0 +1,43 @@
+use crate::services::async_item_service::AsyncItemService;
+use crate::services::error::CoreError;
+use axum::http::StatusCode;
+use std::collections::HashMap;
+
+/// Check if content is binary when allow_binary is false
+pub async fn check_binary_content_allowed(
+    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
+pub 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) => {
+                log::warn!("Failed to get content info for binary check for item {}: {}", item_id, e);
+                Err(StatusCode::INTERNAL_SERVER_ERROR)
+            }
+        }
+    }
+}

src/modes/server/api/common.rs

@@ -0,0 +1,37 @@
+use axum::{
+    http::{header, StatusCode},
+    response::Response,
+};
+use serde::Serialize;
+use log;
+
+pub struct ResponseBuilder;
+
+impl ResponseBuilder {
+    pub fn json<T: Serialize>(data: T) -> Result<Response, StatusCode> {
+        let json = serde_json::to_vec(&data).map_err(|e| {
+            log::warn!("Failed to serialize response: {}", e);
+            StatusCode::INTERNAL_SERVER_ERROR
+        })?;
+        
+        Response::builder()
+            .header(header::CONTENT_TYPE, "application/json")
+            .header(header::CONTENT_LENGTH, json.len().to_string())
+            .body(axum::body::Body::from(json))
+            .map_err(|e| {
+                log::warn!("Failed to build response: {}", e);
+                StatusCode::INTERNAL_SERVER_ERROR
+            })
+    }
+    
+    pub fn binary(content: &[u8], mime_type: &str) -> Result<Response, StatusCode> {
+        Response::builder()
+            .header(header::CONTENT_TYPE, mime_type)
+            .header(header::CONTENT_LENGTH, content.len().to_string())
+            .body(axum::body::Body::from(content.to_vec()))
+            .map_err(|e| {
+                log::warn!("Failed to build response: {}", e);
+                StatusCode::INTERNAL_SERVER_ERROR
+            })
+    }
+}

src/modes/server/api/item.rs

@@ -5,75 +5,14 @@ use axum::{
 };
 use log::{debug, warn};
 use std::collections::HashMap;
+use crate::common::binary_detection::{check_binary_content_allowed, is_content_binary};
+use crate::filter_plugin::utils::build_filter_string;
+use crate::modes::server::api::common::ResponseBuilder;
 
 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 {
@@ -193,29 +132,11 @@ pub async fn handle_list_items(
         })
         .collect();
 
-    let response = ApiResponse {
+    ResponseBuilder::json(ApiResponse {
         success: true,
         data: Some(item_infos),
         error: None,
-    };
+    })
-
-    // Serialize to JSON to get the exact length
-    let json_response = match serde_json::to_vec(&response) {
-        Ok(data) => data,
-        Err(e) => {
-            warn!("Failed to serialize response: {}", e);
-            return Err(StatusCode::INTERNAL_SERVER_ERROR);
-        }
-    };
-    
-    // Build response with explicit Content-Length
-    let response = Response::builder()
-        .header(header::CONTENT_TYPE, "application/json")
-        .header(header::CONTENT_LENGTH, json_response.len().to_string())
-        .body(axum::body::Body::from(json_response))
-        .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
-    
-    Ok(response)
 }
 
 /// Handle as_meta=true response by returning JSON with metadata and content
@@ -255,12 +176,11 @@ async fn handle_as_meta_response_with_metadata(
             "error": "Content is binary"
         });
         
-        let response = Response::builder()
+        Response::builder()
             .header(header::CONTENT_TYPE, "application/json")
             .status(StatusCode::UNPROCESSABLE_ENTITY)
             .body(axum::body::Body::from(response_body.to_string()))
-            .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
+            .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)
-        Ok(response)
     } else {
         // Get the content as text
         match item_service.get_item_content_info(
@@ -310,11 +230,10 @@ async fn handle_as_meta_response_with_metadata(
                     "error": serde_json::Value::Null
                 });
                 
-                let response = Response::builder()
+                Response::builder()
                     .header(header::CONTENT_TYPE, "application/json")
                     .body(axum::body::Body::from(response_body.to_string()))
-                    .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
+                    .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)
-                Ok(response)
             }
             Err(e) => {
                 warn!("Failed to get content for item {}: {}", item_id, e);
@@ -524,7 +443,7 @@ async fn stream_item_content_response_with_metadata(
     let mime_type = get_mime_type(metadata);
     
     // Check if content is binary when allow_binary is false
-    check_binary_content(item_service, item_id, metadata, allow_binary).await?;
+    check_binary_content_allowed(item_service, item_id, metadata, allow_binary).await?;
     
     if stream {
         debug!("STREAMING: Using streaming approach");
@@ -561,12 +480,7 @@ async fn stream_item_content_response_with_metadata(
                 debug!("NON-STREAMING: Content length: {}, response length: {}", 
                        content.len(), response_content.len());
                 
-                let response = Response::builder()
+                ResponseBuilder::binary(response_content, &mime_type)
-                    .header(header::CONTENT_TYPE, mime_type)
-                    .header(header::CONTENT_LENGTH, response_content.len().to_string())
-                    .body(axum::body::Body::from(response_content.to_vec()))
-                    .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
-                Ok(response)
             }
             Err(e) => {
                 warn!("Failed to get content for item {}: {}", item_id, e);