improve archive config file priority

yeoleobun · yeoleobun · commit 191ac1876dee · 2026-06-05T19:49:46.000+08:00
diff --git a/src/addons/archive/README.md b/src/addons/archive/README.md
@@ -4,16 +4,27 @@ This addon provides functionality to archive old call records to compressed CSV
 
 ## Configuration
 
-Add the following section to your `config.toml` (or `rustpbx.toml`):
+Enable the addon in your main `config.toml` (or `rustpbx.toml`):
+
+```toml
+[proxy]
+addons = ["archive"]
+```
+
+Then add the following settings to `archive.toml` in the same directory as your main config file:
 
 ```toml
-[archive]
 enabled = true
 archive_time = "03:00"       # Time to run the archive job (HH:MM)
 timezone = "Asia/Shanghai"   # Timezone for the schedule
 retention_days = 30          # Keep data for 30 days
+archive_after_days = 0       # 0 archives yesterday's records
+# archive_dir = "/var/lib/rustpbx/archive"
 ```
 
+For compatibility, the addon also accepts the same settings under `[archive]` in the main config
+when `archive.toml` is not present. The console UI saves archive settings to `archive.toml`.
+
 ## Features
 
 - **Scheduled Archiving**: Automatically runs daily at the configured time.
diff --git a/src/addons/archive/handlers.rs b/src/addons/archive/handlers.rs
@@ -10,7 +10,6 @@ use chrono::NaiveDate;
 use sea_orm::{ColumnTrait, EntityTrait, PaginatorTrait, QueryFilter};
 use serde::{Deserialize, Serialize};
 use std::sync::{Arc, RwLock};
-use toml_edit::{DocumentMut, value};
 use tracing::{error, info};
 use uuid::Uuid;
 
@@ -136,11 +135,6 @@ pub async fn update_config(
     Extension(archive_state): Extension<ArchiveState>,
     Json(payload): Json<UpdateConfigPayload>,
 ) -> impl IntoResponse {
-    let config_path = state
-        .config_path
-        .clone()
-        .unwrap_or_else(|| "config.toml".to_string());
-
     // Validate timezone before touching the config file
     let tz_str = payload.timezone.trim();
     if tz_str.parse::<chrono_tz::Tz>().is_err() {
@@ -156,62 +150,38 @@ pub async fn update_config(
         ).into_response();
     }
 
-    let res = (|| -> anyhow::Result<()> {
-        let config_content = std::fs::read_to_string(&config_path)?;
-        let mut doc = config_content.parse::<DocumentMut>()?;
+    let archive_config_path = super::archive_config_path(&state.config_path);
+    let new_archive_dir = payload
+        .archive_dir
+        .as_deref()
+        .map(|s| s.trim().to_string())
+        .filter(|s| !s.is_empty());
+    let new_config = crate::addons::archive::ArchiveConfig {
+        enabled: payload.enabled,
+        archive_time: payload.archive_time.trim().to_string(),
+        timezone: Some(tz_str.to_string()),
+        retention_days: payload.retention_days,
+        archive_after_days: payload.archive_after_days,
+        archive_dir: new_archive_dir,
+    };
 
-        let needs_archive_init = doc
-            .as_table()
-            .get("archive")
-            .map(|item| !item.is_table())
-            .unwrap_or(true);
-        if needs_archive_init {
-            doc.insert("archive", toml_edit::table());
+    let res = (|| -> anyhow::Result<()> {
+        if let Some(parent) = archive_config_path.parent()
+            && !parent.as_os_str().is_empty()
+        {
+            std::fs::create_dir_all(parent)?;
         }
 
-        let archive_table = doc
-            .as_table_mut()
-            .get_mut("archive")
-            .and_then(toml_edit::Item::as_table_mut)
-            .ok_or_else(|| anyhow::anyhow!("[archive] must be a table"))?;
-
-        archive_table["enabled"] = value(payload.enabled);
-        archive_table["archive_time"] = value(payload.archive_time.trim());
-        archive_table["timezone"] = value(tz_str);
-        archive_table["retention_days"] = value(payload.retention_days as i64);
-        archive_table["archive_after_days"] = value(payload.archive_after_days as i64);
-        match payload.archive_dir.as_deref() {
-            Some(d) if !d.trim().is_empty() => {
-                archive_table["archive_dir"] = value(d.trim());
-            }
-            _ => {
-                // Remove override → fall back to derived default
-                archive_table.remove("archive_dir");
-            }
-        }
-        std::fs::write(&config_path, doc.to_string())?;
-        info!("Updated archive config in {}", config_path);
+        let config_content = toml::to_string_pretty(&new_config)?;
+        std::fs::write(&archive_config_path, config_content)?;
+        info!("Updated archive config in {}", archive_config_path.display());
         Ok(())
     })();
 
     match res {
         Ok(_) => {
             // Update in-memory config
-            let tz_str = tz_str.to_string();
-            let mut config_guard = archive_state.config.write().unwrap();
-            let new_archive_dir = payload
-                .archive_dir
-                .as_deref()
-                .map(|s| s.trim().to_string())
-                .filter(|s| !s.is_empty());
-            *config_guard = Some(crate::addons::archive::ArchiveConfig {
-                enabled: payload.enabled,
-                archive_time: payload.archive_time.trim().to_string(),
-                timezone: Some(tz_str.to_string()),
-                retention_days: payload.retention_days,
-                archive_after_days: payload.archive_after_days,
-                archive_dir: new_archive_dir,
-            });
+            *archive_state.config.write().unwrap() = Some(new_config);
             Json(serde_json::json!({"success": true})).into_response()
         }
         Err(e) => {
diff --git a/src/addons/archive/mod.rs b/src/addons/archive/mod.rs
@@ -9,6 +9,7 @@ use chrono::{DateTime, Duration, NaiveDate, NaiveTime, Utc};
 use chrono_tz::Tz;
 use sea_orm::{ColumnTrait, EntityTrait, QueryFilter, QueryOrder, QuerySelect};
 use std::collections::HashMap;
+use std::path::PathBuf;
 use std::sync::{Arc, RwLock};
 use tokio::time;
 use tracing::{error, info};
@@ -39,12 +40,95 @@ pub struct ArchiveAddon {
     state: ArchiveState,
 }
 
+fn archive_config_path(config_path: &Option<String>) -> PathBuf {
+    config_path
+        .as_deref()
+        .and_then(|path| std::path::Path::new(path).parent())
+        .map(|dir| dir.join("archive.toml"))
+        .unwrap_or_else(|| PathBuf::from("archive.toml"))
+}
+
 impl Default for ArchiveAddon {
     fn default() -> Self {
         Self::new()
     }
 }
 
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn archive_config_path_uses_main_config_directory() {
+        let path = Some("config/rustpbx.toml".to_string());
+
+        assert_eq!(
+            archive_config_path(&path),
+            std::path::PathBuf::from("config/archive.toml")
+        );
+    }
+
+    #[tokio::test]
+    async fn load_config_reads_archive_toml_next_to_main_config() {
+        let dir = tempfile::tempdir().unwrap();
+        let main_config = dir.path().join("rustpbx.toml");
+        let archive_config = dir.path().join("archive.toml");
+        std::fs::write(&main_config, "[proxy]\naddons = [\"archive\"]\n").unwrap();
+        std::fs::write(
+            &archive_config,
+            r#"enabled = true
+archive_time = "03:00"
+timezone = "Asia/Shanghai"
+retention_days = 30
+archive_after_days = 7
+archive_dir = "/tmp/rustpbx-archive"
+"#,
+        )
+        .unwrap();
+
+        let loaded =
+            ArchiveAddon::load_config(&Some(main_config.to_string_lossy().to_string())).await;
+        let loaded = loaded.expect("archive config should load");
+
+        assert!(loaded.enabled);
+        assert_eq!(loaded.archive_time, "03:00");
+        assert_eq!(loaded.timezone.as_deref(), Some("Asia/Shanghai"));
+        assert_eq!(loaded.retention_days, 30);
+        assert_eq!(loaded.archive_after_days, 7);
+        assert_eq!(loaded.archive_dir.as_deref(), Some("/tmp/rustpbx-archive"));
+    }
+
+    #[tokio::test]
+    async fn load_config_falls_back_to_archive_table_in_main_config() {
+        let dir = tempfile::tempdir().unwrap();
+        let main_config = dir.path().join("rustpbx.toml");
+        std::fs::write(
+            &main_config,
+            r#"[proxy]
+addons = ["archive"]
+
+[archive]
+enabled = true
+archive_time = "04:00"
+timezone = "UTC"
+retention_days = 14
+archive_after_days = 3
+"#,
+        )
+        .unwrap();
+
+        let loaded =
+            ArchiveAddon::load_config(&Some(main_config.to_string_lossy().to_string())).await;
+        let loaded = loaded.expect("archive config should load from main config");
+
+        assert!(loaded.enabled);
+        assert_eq!(loaded.archive_time, "04:00");
+        assert_eq!(loaded.timezone.as_deref(), Some("UTC"));
+        assert_eq!(loaded.retention_days, 14);
+        assert_eq!(loaded.archive_after_days, 3);
+    }
+}
+
 impl ArchiveAddon {
     pub fn new() -> Self {
         Self {
@@ -58,30 +142,63 @@ impl ArchiveAddon {
 
     /// Load configuration from addon-specific config file.
     pub async fn load_config(config_path: &Option<String>) -> Option<ArchiveConfig> {
+        let addon_config_path = archive_config_path(config_path);
+        if addon_config_path.exists() {
+            match tokio::fs::read_to_string(&addon_config_path).await {
+                Ok(content) => match toml::from_str::<ArchiveConfig>(&content) {
+                    Ok(config) => {
+                        tracing::info!(
+                            "Archive config loaded from {}",
+                            addon_config_path.display()
+                        );
+                        return Some(config);
+                    }
+                    Err(e) => {
+                        tracing::warn!("Failed to parse archive.toml: {}", e);
+                    }
+                },
+                Err(e) => {
+                    tracing::warn!("Failed to read archive.toml: {}", e);
+                }
+            }
+        }
+
         if let Some(path) = config_path {
-            let config_dir = std::path::Path::new(path).parent()?;
-            let addon_config_path = config_dir.join("archive.toml");
-            if addon_config_path.exists() {
-                match tokio::fs::read_to_string(&addon_config_path).await {
-                    Ok(content) => match toml::from_str::<ArchiveConfig>(&content) {
-                        Ok(config) => {
-                            tracing::info!(
-                                "Archive config loaded from {}",
-                                addon_config_path.display()
-                            );
-                            return Some(config);
-                        }
-                        Err(e) => {
-                            tracing::warn!("Failed to parse archive.toml: {}", e);
+            match tokio::fs::read_to_string(path).await {
+                Ok(content) => match toml::from_str::<toml::Value>(&content) {
+                    Ok(value) => {
+                        if let Some(archive_value) = value.get("archive") {
+                            match archive_value.clone().try_into::<ArchiveConfig>() {
+                                Ok(config) => {
+                                    tracing::info!(
+                                        "Archive config loaded from [archive] in {}",
+                                        path
+                                    );
+                                    return Some(config);
+                                }
+                                Err(e) => {
+                                    tracing::warn!(
+                                        "Failed to parse [archive] from {}: {}",
+                                        path,
+                                        e
+                                    );
+                                }
+                            }
                         }
-                    },
+                    }
                     Err(e) => {
-                        tracing::warn!("Failed to read archive.toml: {}", e);
+                        tracing::warn!("Failed to parse {} while checking [archive]: {}", path, e);
                     }
+                },
+                Err(e) => {
+                    tracing::warn!("Failed to read {} while checking [archive]: {}", path, e);
                 }
             }
         }
-        tracing::info!("Archive using default configuration (no archive.toml found)");
+
+        tracing::info!(
+            "Archive using default configuration (no archive.toml or [archive] config found)"
+        );
         None
     }
 
