From 04554fe04d571a0f760629bff526e2d49dda34ca Mon Sep 17 00:00:00 2001
From: Andrew Phillips <andrew.phillips2@canada.ca>
Date: Mon, 25 Aug 2025 20:46:30 -0300
Subject: [PATCH] refactor: extract content info logic to item_service

Co-authored-by: aider (openai/andrew/openrouter/deepseek/deepseek-chat-v3.1) <aider@aider.chat>
---
 src/modes/server/api/item.rs       | 34 +++++++++++++++++++-----------
 src/services/async_item_service.rs | 27 +++++-------------------
 src/services/item_service.rs       | 19 +++++++++++++++++
 3 files changed, 46 insertions(+), 34 deletions(-)

diff --git a/src/modes/server/api/item.rs b/src/modes/server/api/item.rs
index f830c23..70e1f45 100644
--- a/src/modes/server/api/item.rs
+++ b/src/modes/server/api/item.rs
@@ -245,22 +245,32 @@ async fn stream_item_content_response(
     
     // Check if content is binary when allow_binary is false
     if !allow_binary {
-        let is_content_binary = if let Some(binary_val) = metadata.get("binary") {
-            binary_val == "true"
-        } else {
-            // For the binary check, we need to read a sample of the content
-            match item_service.get_item_content(item_id).await {
-                Ok(item_with_content) => {
-                    crate::common::is_binary::is_binary(&item_with_content.content)
-                }
-                Err(e) => {
-                    warn!("Failed to get content for binary check for item {}: {}", item_id, e);
-                    return Err(StatusCode::INTERNAL_SERVER_ERROR);
+        // Get the binary status using the new method
+        let is_binary = match item_service.get_item(item_id).await {
+            Ok(item_with_meta) => {
+                let metadata = item_with_meta.meta_as_map();
+                if let Some(binary_val) = metadata.get("binary") {
+                    binary_val == "true"
+                } else {
+                    // Fall back to checking the content directly
+                    match item_service.get_item_content(item_id).await {
+                        Ok(item_with_content) => {
+                            crate::common::is_binary::is_binary(&item_with_content.content)
+                        }
+                        Err(e) => {
+                            warn!("Failed to get content for binary check for item {}: {}", item_id, e);
+                            return Err(StatusCode::INTERNAL_SERVER_ERROR);
+                        }
+                    }
                 }
             }
+            Err(e) => {
+                warn!("Failed to get item {} for binary check: {}", item_id, e);
+                return Err(StatusCode::INTERNAL_SERVER_ERROR);
+            }
         };
         
-        if is_content_binary {
+        if is_binary {
             return Err(StatusCode::BAD_REQUEST);
         }
     }
diff --git a/src/services/async_item_service.rs b/src/services/async_item_service.rs
index 55a5300..e09820c 100644
--- a/src/services/async_item_service.rs
+++ b/src/services/async_item_service.rs
@@ -58,37 +58,20 @@ impl AsyncItemService {
         let db = self.db.clone();
         let item_service = self.item_service.clone();
 
-        // Get item metadata first to check binary status and get MIME type
-        let item_with_content = tokio::task::spawn_blocking(move || {
+        // Get item content, mime type, and binary status
+        let (content, mime_type, is_binary) = tokio::task::spawn_blocking(move || {
             let conn = db.blocking_lock();
-            item_service.get_item_content(&conn, item_id)
+            item_service.get_item_content_info(&conn, item_id)
         })
         .await
         .unwrap()?;
-
-        let metadata = item_with_content.item_with_meta.meta_as_map();
-        let 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 {
-            let is_content_binary = if let Some(binary_val) = metadata.get("binary") {
-                binary_val == "true"
-            } else {
-                // For the binary check, we need to read a sample of the content
-                // We'll read the first 8192 bytes to determine if it's binary
-                crate::common::is_binary::is_binary(&item_with_content.content)
-            };
-            
-            if is_content_binary {
-                return Err(CoreError::InvalidInput("Binary content not allowed".to_string()));
-            }
+        if !allow_binary && is_binary {
+            return Err(CoreError::InvalidInput("Binary content not allowed".to_string()));
         }
         
         // Create a stream that reads only the requested portion
-        let content = item_with_content.content;
         let content_len = content.len() as u64;
         
         // Apply offset and length constraints
diff --git a/src/services/item_service.rs b/src/services/item_service.rs
index 1120267..900dc48 100644
--- a/src/services/item_service.rs
+++ b/src/services/item_service.rs
@@ -66,6 +66,25 @@ impl ItemService {
         })
     }
 
+    pub fn get_item_content_info(&self, conn: &Connection, id: i64) -> Result<(Vec<u8>, String, bool), CoreError> {
+        let item_with_content = self.get_item_content(conn, id)?;
+        let metadata = item_with_content.item_with_meta.meta_as_map();
+        
+        let mime_type = metadata
+            .get("mime_type")
+            .map(|s| s.to_string())
+            .unwrap_or_else(|| "application/octet-stream".to_string());
+        
+        // Check if content is binary
+        let is_binary = if let Some(binary_val) = metadata.get("binary") {
+            binary_val == "true"
+        } else {
+            crate::common::is_binary::is_binary(&item_with_content.content)
+        };
+        
+        Ok((item_with_content.content, mime_type, is_binary))
+    }
+
     pub fn find_item(&self, conn: &Connection, ids: &[i64], tags: &[String], meta: &HashMap<String, String>) -> Result<ItemWithMeta, CoreError> {
         debug!("ITEM_SERVICE: Finding item with ids: {:?}, tags: {:?}, meta: {:?}", ids, tags, meta);
         let item_maybe = match (ids.is_empty(), tags.is_empty() && meta.is_empty()) {