fix: use item service for proper item handling

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

src/modes/diff.rs

@@ -2,8 +2,8 @@ use anyhow::{anyhow, Context, Result};
 use clap::Command;
 use std::io::Read;
 use std::os::fd::FromRawFd;
-use std::str::FromStr;
 use crate::config;
+use crate::services::item_service::ItemService;
 
 fn validate_diff_args(cmd: &mut Command, ids: &Vec<i64>, tags: &Vec<String>) {
     if !tags.is_empty() {
@@ -22,15 +22,12 @@ fn validate_diff_args(cmd: &mut Command, ids: &Vec<i64>, tags: &Vec<String>) {
     }
 }
 
-use crate::services::item_service::ItemService;
-use crate::services::types::ItemWithMeta;
-
 fn fetch_and_validate_items(
     conn: &mut rusqlite::Connection,
     ids: &Vec<i64>,
     item_service: &ItemService,
-) -> Result<(ItemWithMeta, ItemWithMeta)> {
+) -> Result<(crate::services::types::ItemWithMeta, crate::services::types::ItemWithMeta)> {
-    // Fetch items, ensuring they exist.
+    // Fetch items using the service, which handles validation
     let item_a = item_service
         .get_item(conn, ids[0])
         .with_context(|| format!("Unable to find first item (ID: {}) in database", ids[0]))?;
@@ -41,47 +38,29 @@ fn fetch_and_validate_items(
     log::debug!("MAIN: Found item A {:?}", item_a.item);
     log::debug!("MAIN: Found item B {:?}", item_b.item);
 
-    let item_a_id = item_a.item.id.ok_or_else(|| anyhow!("Item A missing ID"))?;
-    let item_b_id = item_b.item.id.ok_or_else(|| anyhow!("Item B missing ID"))?;
-
-    // Validate that item IDs are positive to prevent path traversal issues
-    if item_a_id <= 0 || item_b_id <= 0 {
-        return Err(anyhow::anyhow!(
-            "Invalid item ID: {} or {}",
-            item_a_id,
-            item_b_id
-        ));
-    }
-
     Ok((item_a, item_b))
 }
 
 fn setup_diff_paths_and_compression(
-    data_path: &std::path::PathBuf,
+    item_service: &ItemService,
-    item_a: &ItemWithMeta,
+    item_a: &crate::services::types::ItemWithMeta,
-    item_b: &ItemWithMeta,
+    item_b: &crate::services::types::ItemWithMeta,
 ) -> Result<(
     std::path::PathBuf,
-    crate::compression_engine::CompressionType,
     std::path::PathBuf,
-    crate::compression_engine::CompressionType,
 )> {
     let item_a_id = item_a.item.id.ok_or_else(|| anyhow::anyhow!("Item A missing ID"))?;
     let item_b_id = item_b.item.id.ok_or_else(|| anyhow::anyhow!("Item B missing ID"))?;
 
+    // Use the service's data path to construct proper file paths
+    let data_path = item_service.get_data_path();
     let mut item_path_a = data_path.clone();
     item_path_a.push(item_a_id.to_string());
-    let compression_type_a =
-        crate::compression_engine::CompressionType::from_str(&item_a.item.compression)?;
-    log::debug!("MAIN: Item A has compression type {:?}", compression_type_a);
     
     let mut item_path_b = data_path.clone();
     item_path_b.push(item_b_id.to_string());
-    let compression_type_b =
-        crate::compression_engine::CompressionType::from_str(&item_b.item.compression)?;
-    log::debug!("MAIN: Item B has compression type {:?}", compression_type_b);
 
-    Ok((item_path_a, compression_type_a, item_path_b, compression_type_b))
+    Ok((item_path_a, item_path_b))
 }
 
 fn setup_diff_pipes() -> Result<((libc::c_int, libc::c_int), (libc::c_int, libc::c_int)), anyhow::Error> {
@@ -167,14 +146,20 @@ fn write_item_to_pipe(
     compression_type: crate::compression_engine::CompressionType,
     pipe_writer_raw: std::fs::File,
 ) -> Result<(), anyhow::Error> {
-    use std::io::BufWriter;
+    use std::io::{BufWriter, BufReader};
+    
     let mut buffered_pipe_writer = BufWriter::new(pipe_writer_raw);
-    let engine =
+    
-        crate::compression_engine::get_compression_engine(compression_type).expect("Unable to get compression engine");
+    // Get a reader from the compression engine
+    let engine = crate::compression_engine::get_compression_engine(compression_type)
+        .map_err(|e| anyhow::anyhow!("Unable to get compression engine: {}", e))?;
+    
+    let mut reader = engine.open(item_path)
+        .map_err(|e| anyhow::anyhow!("Failed to open item: {}", e))?;
+    
     log::debug!("THREAD: Sending item to diff");
-    engine
+    std::io::copy(&mut reader, &mut buffered_pipe_writer)
-        .copy(item_path, &mut buffered_pipe_writer)
+        .map_err(|e| anyhow::anyhow!("Failed to copy item to pipe: {}", e))?;
-        .map_err(|e| anyhow::anyhow!("Failed to copy/compress item: {}", e))?;
     log::debug!("THREAD: Done sending item to diff");
     Ok(())
 }
@@ -317,8 +302,11 @@ pub fn mode_diff(
     let item_a_tags: Vec<String> = item_a.tags.iter().map(|t| t.name.clone()).collect();
     let item_b_tags: Vec<String> = item_b.tags.iter().map(|t| t.name.clone()).collect();
 
-    let (item_path_a, compression_type_a, item_path_b, compression_type_b) =
+    let (item_path_a, item_path_b) = setup_diff_paths_and_compression(&item_service, &item_a, &item_b)?;
-        setup_diff_paths_and_compression(&data_path, &item_a, &item_b)?;
+    
+    // Get compression types from the items
+    let compression_type_a = item_a.item.compression.parse()?;
+    let compression_type_b = item_b.item.compression.parse()?;
 
     let ((fd_a_read, fd_a_write), (fd_b_read, fd_b_write)) = setup_diff_pipes()?;
     let (_fd_a_read_guard, _fd_b_read_guard) = setup_fd_guards(fd_a_read, fd_b_read);