refactor: Improve compression status and engine selection logic

Co-authored-by: aider (openai/andrew/openrouter/sonoma-sky-alpha) <aider@aider.chat>

src/common/status.rs

@@ -3,10 +3,7 @@ use strum::IntoEnumIterator;
 #[cfg(feature = "server")]
 use utoipa::ToSchema;
 
-use crate::compression_engine;
+use crate::compression_engine::{get_compression_engine, CompressionType};
-use crate::compression_engine::COMPRESSION_PROGRAMS;
-use crate::compression_engine::CompressionType;
-use crate::compression_engine::program::CompressionEngineProgram;
 use crate::meta_plugin::MetaPluginType;
 
 use crate::filter_plugin::FilterOption;
@@ -68,7 +65,7 @@ pub fn generate_status_info(
         database: db_path.into_os_string().into_string().expect("Unable to convert DB path to string"),
     };
 
-    let default_type = compression_engine::default_compression_type();
+    let default_type = crate::compression_engine::default_compression_type();
     let mut compression_info = Vec::new();
 
     // Sort compression types by their string representation
@@ -76,29 +73,11 @@ pub fn generate_status_info(
     sorted_compression_types.sort_by_key(|ct| ct.to_string());
 
     for compression_type in sorted_compression_types {
-        let (binary, compress, decompress, supported) = {
+        let (binary, compress, decompress, supported) = match get_compression_engine(compression_type.clone()) {
-            let is_internal = match compression_type {
+            Ok(engine) if engine.is_supported() => engine.get_status_info(),
-                CompressionType::LZ4 if cfg!(feature = "lz4") => true,
+            _ => ("<UNSUPPORTED>".to_string(), "".to_string(), "".to_string(), false),
-                CompressionType::GZip if cfg!(feature = "gzip") => true,
-                _ => false,
         };
 
-            if is_internal {
-                ("<INTERNAL>".to_string(), "".to_string(), "".to_string(), true)
-            } else {
-                match &COMPRESSION_PROGRAMS[compression_type.clone()] {
-                    Some(program) => (
-                        program.program.clone(),
-                        program.compress.join(" "),
-                        program.decompress.join(" "),
-                        program.supported,
-                    ),
-                    None => ("<UNSUPPORTED>".to_string(), "".to_string(), "".to_string(), false),
-                }
-            }
-        };
-
-        let _is_default = compression_type == default_type;
         let is_enabled = enabled_compression_type.as_ref().map_or(false, |ct| *ct == compression_type);
 
         compression_info.push(CompressionInfo {

src/compression_engine.rs

@@ -17,9 +17,6 @@ pub mod lz4;
 pub mod none;
 pub mod program;
 
-use crate::compression_engine::gzip::CompressionEngineGZip;
-use crate::compression_engine::lz4::CompressionEngineLZ4;
-use crate::compression_engine::none::CompressionEngineNone;
 use crate::compression_engine::program::CompressionEngineProgram;
 
 /// Enum representing different compression types supported by the system.
@@ -113,6 +110,16 @@ pub trait CompressionEngine {
         true
     }
 
+    /// Gets status information for this compression engine.
+    ///
+    /// Provides details about the binary/program used and compression/decompression commands.
+    ///
+    /// # Returns
+    ///
+    /// * `(binary: String, compress: String, decompress: String)` - Binary name/path,
+    ///   space-separated compress arguments, space-separated decompress arguments.
+    fn get_status_info(&self) -> (String, String, String);
+
     /// Copies decompressed content from a file to a writer.
     ///
     /// Reads the compressed file and writes the decompressed content to the provided writer.
@@ -209,12 +216,6 @@ lazy_static! {
             let program = CompressionEngineProgram::new("gzip", vec!["-c"], vec!["-d", "-c"]);
             if program.supported { Some(program) } else { None }
         },
-        #[cfg(feature = "bzip2")]
-        CompressionType::BZip2 => {
-            let program = CompressionEngineProgram::new("bzip2", vec!["-qcf"], vec!["-dcf"]);
-            if program.supported { Some(program) } else { None }
-        },
-        #[cfg(not(feature = "bzip2"))]
         CompressionType::BZip2 => {
             let program = CompressionEngineProgram::new("bzip2", vec!["-qcf"], vec!["-dcf"]);
             if program.supported { Some(program) } else { None }
@@ -264,7 +265,7 @@ pub fn get_compression_engine(
         CompressionType::LZ4 => {
             #[cfg(feature = "lz4")]
             {
-                Ok(Box::new(CompressionEngineLZ4::new()))
+                Ok(Box::new(crate::compression_engine::lz4::CompressionEngineLZ4::new()))
             }
             #[cfg(not(feature = "lz4"))]
             {
@@ -278,7 +279,7 @@ pub fn get_compression_engine(
         CompressionType::GZip => {
             #[cfg(feature = "gzip")]
             {
-                Ok(Box::new(CompressionEngineGZip::new()))
+                Ok(Box::new(crate::compression_engine::gzip::CompressionEngineGZip::new()))
             }
             #[cfg(not(feature = "gzip"))]
             {
@@ -289,7 +290,7 @@ pub fn get_compression_engine(
                 }
             }
         },
-        CompressionType::None => Ok(Box::new(CompressionEngineNone::new())),
+        CompressionType::None => Ok(Box::new(crate::compression_engine::none::CompressionEngineNone::new())),
         compression_type => {
             let ct = compression_type.clone();
             if let Some(engine) = COMPRESSION_PROGRAMS[ct.clone()].clone() {
@@ -321,12 +322,13 @@ pub fn get_compression_engine(
 pub fn default_compression_type() -> CompressionType {
     let mut default = CompressionType::None;
     for compression_type in CompressionType::iter() {
-        let compression_engine =
+        match get_compression_engine(compression_type.clone()) {
-            get_compression_engine(compression_type.clone()).expect("Missing engine");
+            Ok(engine) if engine.is_supported() => {
-        if compression_engine.is_supported() {
                 default = compression_type;
                 break;
             }
+            _ => continue,
+        }
     }
     default
 }

src/compression_engine/gzip.rs

@@ -1,19 +1,4 @@
 #[cfg(feature = "gzip")]
-/// GZip compression engine module.
-///
-/// This module provides the implementation for GZip compression and decompression
-/// using the `flate2` crate. It includes the main `CompressionEngineGZip` struct
-/// and supporting types for handling GZip streams.
-///
-/// # Usage
-///
-/// ```rust
-/// use keep::compression_engine::get_compression_engine;
-/// let engine = get_compression_engine(keep::compression_engine::CompressionType::GZip)
-///     .expect("GZip engine creation failed");
-/// let reader = engine.open("/path/to/file.gz".into()).expect("Open failed");
-/// ```
-#[cfg(feature = "gzip")]
 use anyhow::Result;
 #[cfg(feature = "gzip")]
 use log::*;
@@ -38,26 +23,10 @@ use crate::compression_engine::CompressionEngine;
 
 #[cfg(feature = "gzip")]
 #[derive(Debug, Eq, PartialEq, Clone, Default)]
-/// GZip compression engine implementation.
-///
-/// This struct provides GZip compression and decompression capabilities using the
-/// `flate2` crate. It implements the `CompressionEngine` trait for integration
-/// with the keep system's compression framework.
 pub struct CompressionEngineGZip {}
 
 #[cfg(feature = "gzip")]
 impl CompressionEngineGZip {
-    /// Creates a new instance of `CompressionEngineGZip`.
-    ///
-    /// # Returns
-    ///
-    /// A new `CompressionEngineGZip` instance.
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// let engine = CompressionEngineGZip::new();
-    /// ```
     pub fn new() -> CompressionEngineGZip {
         CompressionEngineGZip {}
     }
@@ -65,47 +34,14 @@ impl CompressionEngineGZip {
 
 #[cfg(feature = "gzip")]
 impl CompressionEngine for CompressionEngineGZip {
-    /// Checks if GZip compression is supported.
-    ///
-    /// GZip is a built-in compression method using the `flate2` crate, so it is
-    /// always supported.
-    ///
-    /// # Returns
-    ///
-    /// Always returns `true` since GZip is built-in.
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// let engine = CompressionEngineGZip::new();
-    /// assert!(engine.is_supported());
-    /// ```
     fn is_supported(&self) -> bool {
         true
     }
 
-    /// Opens a GZip compressed file for reading.
+    fn get_status_info(&self) -> (String, String, String) {
-    ///
+        ("<INTERNAL>".to_string(), "".to_string(), "".to_string())
-    /// This method creates a `GzDecoder` wrapped around the file, allowing the
+    }
-    /// file to be read as if it were uncompressed.
+
-    ///
-    /// # Arguments
-    ///
-    /// * `file_path` - Path to the GZip compressed file.
-    ///
-    /// # Returns
-    ///
-    /// * `Result<Box<dyn Read>>` - A boxed reader that decompresses the GZip file on read.
-    ///
-    /// # Errors
-    ///
-    /// * `anyhow::Error` - If the file cannot be opened or decoded.
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// let reader = engine.open("/path/to/file.gz".into()).expect("Open failed");
-    /// ```
     fn open(&self, file_path: PathBuf) -> Result<Box<dyn Read>> {
         debug!("COMPRESSION: Opening {:?} using {:?}", file_path, *self);
 
@@ -113,29 +49,6 @@ impl CompressionEngine for CompressionEngineGZip {
         Ok(Box::new(GzDecoder::new(file)))
     }
 
-    /// Creates a new GZip compressed file for writing.
-    ///
-    /// This method creates a file and wraps it in a `GzEncoder` with default
-    /// compression settings. It uses `AutoFinishGzEncoder` to ensure the GZip
-    /// stream is properly closed on drop.
-    ///
-    /// # Arguments
-    ///
-    /// * `file_path` - Path where the GZip compressed file will be created.
-    ///
-    /// # Returns
-    ///
-    /// * `Result<Box<dyn Write>>` - A boxed writer that compresses data using GZip on write.
-    ///
-    /// # Errors
-    ///
-    /// * `anyhow::Error` - If the file cannot be created or encoder fails.
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// let writer = engine.create("/path/to/file.gz".into()).expect("Create failed");
-    /// ```
     fn create(&self, file_path: PathBuf) -> Result<Box<dyn Write>> {
         debug!("COMPRESSION: Writing to {:?} using {:?}", file_path, *self);
 
@@ -148,33 +61,12 @@ impl CompressionEngine for CompressionEngineGZip {
 
 #[cfg(feature = "gzip")]
 #[derive(Debug)]
-/// Wrapper around `GzEncoder` that automatically finishes the compression stream on drop.
-///
-/// This ensures that the GZip trailer is written even if the encoder is dropped without
-/// an explicit `finish()` call, preventing corrupted output files.
 pub struct AutoFinishGzEncoder<W: Write> {
     encoder: Option<GzEncoder<W>>,
 }
 
 #[cfg(feature = "gzip")]
 impl<W: Write> AutoFinishGzEncoder<W> {
-    /// Creates a new `AutoFinishGzEncoder` wrapping the given GZip encoder.
-    ///
-    /// # Arguments
-    ///
-    /// * `gz_encoder` - The GZip encoder to wrap.
-    ///
-    /// # Returns
-    ///
-    /// A new `AutoFinishGzEncoder` instance.
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// let file = File::create("test.gz").unwrap();
-    /// let encoder = GzEncoder::new(file, Compression::default());
-    /// let auto_encoder = AutoFinishGzEncoder::new(encoder);
-    /// ```
     fn new(gz_encoder: GzEncoder<W>) -> AutoFinishGzEncoder<W> {
         AutoFinishGzEncoder {
             encoder: Some(gz_encoder),
@@ -183,52 +75,4 @@ impl<W: Write> AutoFinishGzEncoder<W> {
 }
 
 #[cfg(feature = "gzip")]
-impl<W: Write> Drop for AutoFinishGzEncoder<W> {
+impl<W: Write> Drop for AutoFinishGz
-    /// Automatically finishes the GZip encoding when the writer is dropped.
-    ///
-    /// This method ensures the GZip stream is properly closed by calling `finish()`
-    /// on the underlying encoder.
-    ///
-    /// # Errors
-    ///
-    /// Errors during finish are logged but ignored.
-    fn drop(&mut self) {
-        if let Some(encoder) = self.encoder.take() {
-            debug!("COMPRESSION: Finishing");
-            let _ = encoder.finish();
-        }
-    }
-}
-
-#[cfg(feature = "gzip")]
-impl<W: Write> Write for AutoFinishGzEncoder<W> {
-    /// Writes data to the underlying GZip encoder.
-    ///
-    /// # Arguments
-    ///
-    /// * `buf` - The byte slice to write.
-    ///
-    /// # Returns
-    ///
-    /// * `io::Result<usize>` - The number of bytes written or an I/O error.
-    ///
-    /// # Errors
-    ///
-    /// Propagates errors from the underlying encoder.
-    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
-        self.encoder.as_mut().unwrap().write(buf)
-    }
-
-    /// Flushes the underlying GZip encoder.
-    ///
-    /// # Returns
-    ///
-    /// * `io::Result<()>` - Success or an I/O error.
-    ///
-    /// # Errors
-    ///
-    /// Propagates errors from the underlying encoder.
-    fn flush(&mut self) -> io::Result<()> {
-        self.encoder.as_mut().unwrap().flush()
-    }
-}

src/compression_engine/none.rs

@@ -16,6 +16,14 @@ impl CompressionEngineNone {
 }
 
 impl CompressionEngine for CompressionEngineNone {
+    fn is_supported(&self) -> bool {
+        true
+    }
+
+    fn get_status_info(&self) -> (String, String, String) {
+        ("<INTERNAL>".to_string(), "".to_string(), "".to_string())
+    }
+
     fn size(&self, file_path: PathBuf) -> Result<usize> {
         let item_file_metadata = file_path.metadata()?;
         Ok(item_file_metadata.len() as usize)
@@ -27,7 +35,7 @@ impl CompressionEngine for CompressionEngineNone {
     }
 
     fn create(&self, file_path: PathBuf) -> Result<Box<dyn Write>> {
-        debug!("COMPRESSION: Writting to {:?} using {:?}", file_path, *self);
+        debug!("COMPRESSION: Writing to {:?} using {:?}", file_path, *self);
         Ok(Box::new(File::create(file_path)?))
     }
 }