From e97807f7faeb4755f5a094c1430595c383cd3325 Mon Sep 17 00:00:00 2001 From: Andrew Phillips Date: Mon, 25 Aug 2025 21:47:06 -0300 Subject: [PATCH] fix: use item service for proper item handling Co-authored-by: aider (openai/andrew/openrouter/deepseek/deepseek-chat-v3.1) --- src/modes/diff.rs | 66 +++++++++++++++++++---------------------------- 1 file changed, 27 insertions(+), 39 deletions(-) diff --git a/src/modes/diff.rs b/src/modes/diff.rs index aa470a9..48a114f 100644 --- a/src/modes/diff.rs +++ b/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, tags: &Vec) { if !tags.is_empty() { @@ -22,15 +22,12 @@ fn validate_diff_args(cmd: &mut Command, ids: &Vec, tags: &Vec) { } } -use crate::services::item_service::ItemService; -use crate::services::types::ItemWithMeta; - fn fetch_and_validate_items( conn: &mut rusqlite::Connection, ids: &Vec, item_service: &ItemService, -) -> Result<(ItemWithMeta, ItemWithMeta)> { - // Fetch items, ensuring they exist. +) -> Result<(crate::services::types::ItemWithMeta, crate::services::types::ItemWithMeta)> { + // 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_a: &ItemWithMeta, - item_b: &ItemWithMeta, + item_service: &ItemService, + item_a: &crate::services::types::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 - .copy(item_path, &mut buffered_pipe_writer) - .map_err(|e| anyhow::anyhow!("Failed to copy/compress item: {}", e))?; + std::io::copy(&mut reader, &mut buffered_pipe_writer) + .map_err(|e| anyhow::anyhow!("Failed to copy item to pipe: {}", e))?; log::debug!("THREAD: Done sending item to diff"); Ok(()) } @@ -317,8 +302,11 @@ pub fn mode_diff( let item_a_tags: Vec = item_a.tags.iter().map(|t| t.name.clone()).collect(); let item_b_tags: Vec = item_b.tags.iter().map(|t| t.name.clone()).collect(); - let (item_path_a, compression_type_a, item_path_b, compression_type_b) = - setup_diff_paths_and_compression(&data_path, &item_a, &item_b)?; + let (item_path_a, item_path_b) = setup_diff_paths_and_compression(&item_service, &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);