From ec866112401f03c76c50f6846a8c70960445613b Mon Sep 17 00:00:00 2001 From: Ezra Lim Date: Wed, 1 Jul 2026 05:36:33 +0000 Subject: [PATCH 1/4] feat(storage): implement storage layer with file and SQLite backends - Add storage traits for RecipesStorage, SignalStorage, KnowledgeStorage, SessionStorage - Implement file-based storage (JSONL/JSON) for all components - Add SQLite backend for recipes with FTS5 full-text search - Support config directory resolution via $DO_SOMETHING_CONFIG or ~/.do-something - Include comprehensive test coverage for all storage components --- Cargo.lock | 232 +++++++++++++++++++- Cargo.toml | 4 + src/lib.rs | 2 + src/storage/config_dir.rs | 153 +++++++++++++ src/storage/error.rs | 46 ++++ src/storage/file_storage.rs | 136 ++++++++++++ src/storage/knowledge_store.rs | 305 ++++++++++++++++++++++++++ src/storage/mod.rs | 119 ++++++++++ src/storage/recipes_db.rs | 357 ++++++++++++++++++++++++++++++ src/storage/session_store.rs | 210 ++++++++++++++++++ src/storage/signal_log.rs | 281 ++++++++++++++++++++++++ src/storage/sqlite_recipes.rs | 390 +++++++++++++++++++++++++++++++++ src/storage/traits.rs | 130 +++++++++++ 13 files changed, 2359 insertions(+), 6 deletions(-) create mode 100644 src/storage/config_dir.rs create mode 100644 src/storage/error.rs create mode 100644 src/storage/file_storage.rs create mode 100644 src/storage/knowledge_store.rs create mode 100644 src/storage/mod.rs create mode 100644 src/storage/recipes_db.rs create mode 100644 src/storage/session_store.rs create mode 100644 src/storage/signal_log.rs create mode 100644 src/storage/sqlite_recipes.rs create mode 100644 src/storage/traits.rs diff --git a/Cargo.lock b/Cargo.lock index 237e4cc..e23bff4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -19,7 +19,7 @@ dependencies = [ "schemars 1.2.1", "serde", "serde_json", - "thiserror", + "thiserror 2.0.18", "tokio", "tokio-util", "tracing", @@ -53,6 +53,18 @@ dependencies = [ "tracing", ] +[[package]] +name = "ahash" +version = "0.8.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a15f179cd60c4584b8a8c596927aadc462e27f2ca70c04e0071964a73ba7a75" +dependencies = [ + "cfg-if", + "once_cell", + "version_check", + "zerocopy", +] + [[package]] name = "aho-corasick" version = "1.1.4" @@ -305,6 +317,27 @@ dependencies = [ "unicode-xid", ] +[[package]] +name = "dirs" +version = "5.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "44c45a9d03d6676652bcb5e724c7e988de1acad23a711b5217ab9cbecbec2225" +dependencies = [ + "dirs-sys", +] + +[[package]] +name = "dirs-sys" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "520f05a5cbd335fae5a99ff7a6ab8627577660ee5cfd6a94a6a929b52ff0321c" +dependencies = [ + "libc", + "option-ext", + "redox_users", + "windows-sys 0.48.0", +] + [[package]] name = "displaydoc" version = "0.2.5" @@ -326,13 +359,15 @@ dependencies = [ "async-trait", "blake3", "chrono", + "dirs", "eventsource-stream", "futures", "reqwest", + "rusqlite", "serde", "serde_json", "tempfile", - "thiserror", + "thiserror 2.0.18", "tokio", "tokio-util", "toml", @@ -375,6 +410,18 @@ dependencies = [ "pin-project-lite", ] +[[package]] +name = "fallible-iterator" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2acce4a10f12dc2fb14a218589d4f1f62ef011b2d0cc4b3cb1bba8e94da14649" + +[[package]] +name = "fallible-streaming-iterator" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7360491ce676a36bf9bb3c56c1aa791658183a54d2744120f27285738d90465a" + [[package]] name = "fastrand" version = "2.4.1" @@ -568,6 +615,15 @@ version = "0.12.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" +[[package]] +name = "hashbrown" +version = "0.14.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" +dependencies = [ + "ahash", +] + [[package]] name = "hashbrown" version = "0.15.5" @@ -583,6 +639,15 @@ version = "0.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4f467dd6dccf739c208452f8014c75c18bb8301b050ad1cfb27153803edb0f51" +[[package]] +name = "hashlink" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6ba4ff7128dee98c7dc9794b6a411377e1404dba1c97deb8d1a55297bd25d8af" +dependencies = [ + "hashbrown 0.14.5", +] + [[package]] name = "heck" version = "0.5.0" @@ -917,6 +982,26 @@ version = "0.2.186" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" +[[package]] +name = "libredox" +version = "0.1.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f02ab6bace2054fb888a3c16f990117b579d14a3088e472d63c6011fa185c9d3" +dependencies = [ + "libc", +] + +[[package]] +name = "libsqlite3-sys" +version = "0.30.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2e99fb7a497b1e3339bc746195567ed8d3e24945ecd636e3619d20b9de9e9149" +dependencies = [ + "cc", + "pkg-config", + "vcpkg", +] + [[package]] name = "linux-raw-sys" version = "0.12.1" @@ -1022,6 +1107,12 @@ version = "1.21.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" +[[package]] +name = "option-ext" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" + [[package]] name = "parking" version = "2.2.1" @@ -1089,6 +1180,12 @@ version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" +[[package]] +name = "pkg-config" +version = "0.3.33" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "19f132c84eca552bf34cab8ec81f1c1dcc229b811638f9d283dceabe58c5569e" + [[package]] name = "potential_utf" version = "0.1.5" @@ -1146,7 +1243,7 @@ dependencies = [ "rustc-hash", "rustls", "socket2", - "thiserror", + "thiserror 2.0.18", "tokio", "tracing", "web-time", @@ -1167,7 +1264,7 @@ dependencies = [ "rustls", "rustls-pki-types", "slab", - "thiserror", + "thiserror 2.0.18", "tinyvec", "tracing", "web-time", @@ -1246,6 +1343,17 @@ dependencies = [ "bitflags", ] +[[package]] +name = "redox_users" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba009ff324d1fc1b900bd1fdb31564febe58a8ccc8a6fdbb93b543d33b13ca43" +dependencies = [ + "getrandom 0.2.17", + "libredox", + "thiserror 1.0.69", +] + [[package]] name = "ref-cast" version = "1.0.25" @@ -1354,7 +1462,7 @@ dependencies = [ "schemars 1.2.1", "serde", "serde_json", - "thiserror", + "thiserror 2.0.18", "tokio", "tokio-util", "tracing", @@ -1373,6 +1481,20 @@ dependencies = [ "syn", ] +[[package]] +name = "rusqlite" +version = "0.32.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7753b721174eb8ff87a9a0e799e2d7bc3749323e773db92e0984debb00019d6e" +dependencies = [ + "bitflags", + "fallible-iterator", + "fallible-streaming-iterator", + "hashlink", + "libsqlite3-sys", + "smallvec", +] + [[package]] name = "rustc-hash" version = "2.1.2" @@ -1734,13 +1856,33 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "thiserror" +version = "1.0.69" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6aaf5339b578ea85b50e080feb250a3e8ae8cfcdff9a461c9ec2904bc923f52" +dependencies = [ + "thiserror-impl 1.0.69", +] + [[package]] name = "thiserror" version = "2.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4288b5bcbc7920c07a1149a35cf9590a2aa808e0bc1eafaade0b80947865fbc4" dependencies = [ - "thiserror-impl", + "thiserror-impl 2.0.18", +] + +[[package]] +name = "thiserror-impl" +version = "1.0.69" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" +dependencies = [ + "proc-macro2", + "quote", + "syn", ] [[package]] @@ -2084,6 +2226,18 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ba73ea9cf16a25df0c8caa16c51acb937d5712a8429db78a3ee29d5dcacd3a65" +[[package]] +name = "vcpkg" +version = "0.2.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426" + +[[package]] +name = "version_check" +version = "0.9.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" + [[package]] name = "want" version = "0.3.1" @@ -2307,6 +2461,15 @@ dependencies = [ "windows-link", ] +[[package]] +name = "windows-sys" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "677d2418bec65e3338edb076e806bc1ec15693c5d0104683f2efe857f61056a9" +dependencies = [ + "windows-targets 0.48.5", +] + [[package]] name = "windows-sys" version = "0.52.0" @@ -2334,6 +2497,21 @@ dependencies = [ "windows-link", ] +[[package]] +name = "windows-targets" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a2fa6e2155d7247be68c096456083145c183cbbbc2764150dda45a87197940c" +dependencies = [ + "windows_aarch64_gnullvm 0.48.5", + "windows_aarch64_msvc 0.48.5", + "windows_i686_gnu 0.48.5", + "windows_i686_msvc 0.48.5", + "windows_x86_64_gnu 0.48.5", + "windows_x86_64_gnullvm 0.48.5", + "windows_x86_64_msvc 0.48.5", +] + [[package]] name = "windows-targets" version = "0.52.6" @@ -2367,6 +2545,12 @@ dependencies = [ "windows_x86_64_msvc 0.53.1", ] +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b38e32f0abccf9987a4e3079dfb67dcd799fb61361e53e2882c3cbaf0d905d8" + [[package]] name = "windows_aarch64_gnullvm" version = "0.52.6" @@ -2379,6 +2563,12 @@ version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a9d8416fa8b42f5c947f8482c43e7d89e73a173cead56d044f6a56104a6d1b53" +[[package]] +name = "windows_aarch64_msvc" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc35310971f3b2dbbf3f0690a219f40e2d9afcf64f9ab7cc1be722937c26b4bc" + [[package]] name = "windows_aarch64_msvc" version = "0.52.6" @@ -2391,6 +2581,12 @@ version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b9d782e804c2f632e395708e99a94275910eb9100b2114651e04744e9b125006" +[[package]] +name = "windows_i686_gnu" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a75915e7def60c94dcef72200b9a8e58e5091744960da64ec734a6c6e9b3743e" + [[package]] name = "windows_i686_gnu" version = "0.52.6" @@ -2415,6 +2611,12 @@ version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fa7359d10048f68ab8b09fa71c3daccfb0e9b559aed648a8f95469c27057180c" +[[package]] +name = "windows_i686_msvc" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f55c233f70c4b27f66c523580f78f1004e8b5a8b659e05a4eb49d4166cca406" + [[package]] name = "windows_i686_msvc" version = "0.52.6" @@ -2427,6 +2629,12 @@ version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1e7ac75179f18232fe9c285163565a57ef8d3c89254a30685b57d83a38d326c2" +[[package]] +name = "windows_x86_64_gnu" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53d40abd2583d23e4718fddf1ebec84dbff8381c07cae67ff7768bbf19c6718e" + [[package]] name = "windows_x86_64_gnu" version = "0.52.6" @@ -2439,6 +2647,12 @@ version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c3842cdd74a865a8066ab39c8a7a473c0778a3f29370b5fd6b4b9aa7df4a499" +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b7b52767868a23d5bab768e390dc5f5c55825b6d30b86c844ff2dc7414044cc" + [[package]] name = "windows_x86_64_gnullvm" version = "0.52.6" @@ -2451,6 +2665,12 @@ version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0ffa179e2d07eee8ad8f57493436566c7cc30ac536a3379fdf008f47f6bb7ae1" +[[package]] +name = "windows_x86_64_msvc" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538" + [[package]] name = "windows_x86_64_msvc" version = "0.52.6" diff --git a/Cargo.toml b/Cargo.toml index 606f5d6..509d979 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,10 @@ uuid = { version = "1", features = ["v4", "serde"] } chrono = { version = "0.4", features = ["serde"] } url = { version = "2.5", features = ["serde"] } blake3 = "1" +dirs = "5" + +# SQLite storage backend +rusqlite = { version = "0.32", features = ["bundled"] } [dev-dependencies] tempfile = "3" diff --git a/src/lib.rs b/src/lib.rs index 097a8f8..948977b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,8 +1,10 @@ //! Do-something: A self-improving recipe scraping agent. pub mod models; +pub mod storage; pub use models::*; +pub use storage::*; // Re-export existing modules pub mod agent; diff --git a/src/storage/config_dir.rs b/src/storage/config_dir.rs new file mode 100644 index 0000000..5983fda --- /dev/null +++ b/src/storage/config_dir.rs @@ -0,0 +1,153 @@ +//! Config directory management. +//! +//! Manages the `~/.do-something` directory structure and provides +//! paths to all subdirectories. + +use std::env; +use std::fs; +use std::path::PathBuf; + +use super::error::{Result, StorageError}; + +/// Manages the config directory path resolution and initialization. +#[derive(Debug, Clone)] +pub struct ConfigDir { + path: PathBuf, +} + +impl ConfigDir { + /// Resolve config directory: `$DO_SOMETHING_CONFIG` or `~/.do-something`. + pub fn resolve() -> Result { + let path = if let Ok(custom) = env::var("DO_SOMETHING_CONFIG") { + PathBuf::from(custom) + } else { + dirs::home_dir() + .ok_or(StorageError::NotFound)? + .join(".do-something") + }; + + Ok(Self { path }) + } + + /// Create a ConfigDir from a specific path (for testing). + pub fn from_path(path: PathBuf) -> Self { + Self { path } + } + + /// Ensure directory structure exists. + pub fn init(&self) -> Result<()> { + fs::create_dir_all(&self.path)?; + fs::create_dir_all(self.knowledge_dir())?; + fs::create_dir_all(self.knowledge_dir().join("site_configs"))?; + fs::create_dir_all(self.knowledge_dir().join("user_models"))?; + fs::create_dir_all(self.knowledge_dir().join("patterns"))?; + fs::create_dir_all(self.recipes_dir())?; + fs::create_dir_all(self.signals_dir())?; + fs::create_dir_all(self.state_dir())?; + fs::create_dir_all(self.state_dir().join("sessions"))?; + Ok(()) + } + + /// Get the root config directory path. + pub fn path(&self) -> &std::path::Path { + &self.path + } + + /// Get knowledge directory path. + pub fn knowledge_dir(&self) -> PathBuf { + self.path.join("knowledge") + } + + /// Get recipes directory path. + pub fn recipes_dir(&self) -> PathBuf { + self.path.join("recipes") + } + + /// Get signals directory path. + pub fn signals_dir(&self) -> PathBuf { + self.path.join("signals") + } + + /// Get state directory path. + pub fn state_dir(&self) -> PathBuf { + self.path.join("state") + } + + /// Get config file path. + pub fn config_file(&self) -> PathBuf { + self.path.join("config.json") + } + + /// Get site configs directory path. + pub fn site_configs_dir(&self) -> PathBuf { + self.knowledge_dir().join("site_configs") + } + + /// Get user models directory path. + pub fn user_models_dir(&self) -> PathBuf { + self.knowledge_dir().join("user_models") + } + + /// Get patterns directory path. + pub fn patterns_dir(&self) -> PathBuf { + self.knowledge_dir().join("patterns") + } + + /// Get sessions directory path. + pub fn sessions_dir(&self) -> PathBuf { + self.state_dir().join("sessions") + } +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::tempdir; + + #[test] + fn resolve_uses_env_variable() { + let dir = tempdir().unwrap(); + // SAFETY: Test sets environment variable in single-threaded context + unsafe { + env::set_var("DO_SOMETHING_CONFIG", dir.path()); + } + + let config = ConfigDir::resolve().unwrap(); + assert_eq!(config.path(), dir.path()); + + // SAFETY: Test removes environment variable in single-threaded context + unsafe { + env::remove_var("DO_SOMETHING_CONFIG"); + } + } + + #[test] + fn init_creates_directory_structure() { + let dir = tempdir().unwrap(); + let config = ConfigDir::from_path(dir.path().to_path_buf()); + + config.init().unwrap(); + + assert!(dir.path().exists()); + assert!(config.knowledge_dir().exists()); + assert!(config.recipes_dir().exists()); + assert!(config.signals_dir().exists()); + assert!(config.state_dir().exists()); + assert!(config.site_configs_dir().exists()); + assert!(config.user_models_dir().exists()); + assert!(config.patterns_dir().exists()); + assert!(config.sessions_dir().exists()); + } + + #[test] + fn subdirectory_paths_are_correct() { + let dir = tempdir().unwrap(); + let config = ConfigDir::from_path(dir.path().to_path_buf()); + + assert_eq!(config.knowledge_dir(), dir.path().join("knowledge")); + assert_eq!(config.recipes_dir(), dir.path().join("recipes")); + assert_eq!(config.signals_dir(), dir.path().join("signals")); + assert_eq!(config.state_dir(), dir.path().join("state")); + assert_eq!(config.config_file(), dir.path().join("config.json")); + } +} diff --git a/src/storage/error.rs b/src/storage/error.rs new file mode 100644 index 0000000..a0de371 --- /dev/null +++ b/src/storage/error.rs @@ -0,0 +1,46 @@ +//! Storage error types. + +use std::io; +use thiserror::Error; + +/// Storage-related errors. +#[derive(Debug, Error)] +pub enum StorageError { + /// IO error during file operations. + #[error("IO error: {0}")] + Io(#[from] io::Error), + + /// JSON serialization/deserialization error. + #[error("JSON error: {0}")] + Json(#[from] serde_json::Error), + + /// Config directory not found or inaccessible. + #[error("Config directory not found")] + NotFound, + + /// Invalid path specified. + #[error("Invalid path: {0}")] + InvalidPath(String), + + /// Recipe not found. + #[error("Recipe not found: {0}")] + RecipeNotFound(String), + + /// Session not found. + #[error("Session not found: {0}")] + SessionNotFound(String), + + /// Lock acquisition failed. + #[error("Lock error: {0}")] + LockError(String), + + /// Index corruption. + #[error("Index corruption: {0}")] + IndexCorruption(String), + + /// SQLite database error. + #[error("Database error: {0}")] + Database(#[from] rusqlite::Error), +} + +pub type Result = std::result::Result; diff --git a/src/storage/file_storage.rs b/src/storage/file_storage.rs new file mode 100644 index 0000000..9af360a --- /dev/null +++ b/src/storage/file_storage.rs @@ -0,0 +1,136 @@ +//! Combined file-based storage backend. +//! +//! Provides a unified storage interface with file-based implementations +//! for all storage components. + +use std::path::PathBuf; +use std::sync::Arc; + +use super::config_dir::ConfigDir; +use super::error::Result; +use super::knowledge_store::FileKnowledgeStore; +use super::recipes_db::FileRecipesDb; +use super::session_store::FileSessionStore; +use super::signal_log::FileSignalLog; +use super::traits::Storage; + +/// File-based storage backend. +#[derive(Debug)] +pub struct FileStorage { + config: ConfigDir, + recipes: Arc, + signals: Arc, + knowledge: Arc, + sessions: Arc, +} + +impl FileStorage { + /// Open storage using the default config directory. + pub fn open() -> Result { + let config = ConfigDir::resolve()?; + Self::from_config(config) + } + + /// Open storage at a specific path. + pub fn open_at(path: PathBuf) -> Result { + let config = ConfigDir::from_path(path); + Self::from_config(config) + } + + /// Create storage from a ConfigDir. + pub fn from_config(config: ConfigDir) -> Result { + config.init()?; + + let recipes = Arc::new(FileRecipesDb::open(config.recipes_dir())?); + let signals = Arc::new(FileSignalLog::open(config.signals_dir())?); + let knowledge = Arc::new(FileKnowledgeStore::open(config.knowledge_dir())?); + let sessions = Arc::new(FileSessionStore::open(config.sessions_dir())?); + + Ok(Self { + config, + recipes, + signals, + knowledge, + sessions, + }) + } + + /// Get the config directory. + pub fn config_dir(&self) -> &ConfigDir { + &self.config + } +} + +impl Storage for FileStorage { + type Recipes = FileRecipesDb; + type Signals = FileSignalLog; + type Knowledge = FileKnowledgeStore; + type Sessions = FileSessionStore; + + fn recipes(&self) -> &Self::Recipes { + &self.recipes + } + + fn signals(&self) -> &Self::Signals { + &self.signals + } + + fn knowledge(&self) -> &Self::Knowledge { + &self.knowledge + } + + fn sessions(&self) -> &Self::Sessions { + &self.sessions + } +} + +/// Extension trait for file-based storage with direct access to implementations. +impl FileStorage { + /// Get direct access to the file-based recipes storage. + pub fn file_recipes(&self) -> &FileRecipesDb { + &self.recipes + } + + /// Get direct access to the file-based signal storage. + pub fn file_signals(&self) -> &FileSignalLog { + &self.signals + } + + /// Get direct access to the file-based knowledge storage. + pub fn file_knowledge(&self) -> &FileKnowledgeStore { + &self.knowledge + } + + /// Get direct access to the file-based session storage. + pub fn file_sessions(&self) -> &FileSessionStore { + &self.sessions + } +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::tempdir; + use crate::storage::traits::{RecipesStorage, SessionStorage}; + + #[test] + fn open_creates_directory_structure() { + let dir = tempdir().unwrap(); + let storage = FileStorage::open_at(dir.path().to_path_buf()).unwrap(); + + assert!(storage.config.recipes_dir().exists()); + assert!(storage.config.signals_dir().exists()); + assert!(storage.config.knowledge_dir().exists()); + assert!(storage.config.sessions_dir().exists()); + } + + #[test] + fn storage_trait_methods_work() { + let dir = tempdir().unwrap(); + let storage = FileStorage::open_at(dir.path().to_path_buf()).unwrap(); + + // Can use through trait methods + assert_eq!(storage.recipes().count().unwrap(), 0); + assert_eq!(storage.sessions().count().unwrap(), 0); + } +} diff --git a/src/storage/knowledge_store.rs b/src/storage/knowledge_store.rs new file mode 100644 index 0000000..6378a74 --- /dev/null +++ b/src/storage/knowledge_store.rs @@ -0,0 +1,305 @@ +//! File-based knowledge store for site configs, user models, and patterns. +//! +//! Provides persistence for learned configurations and patterns +//! that improve scraping over time. + +use std::fs::{self, File}; +use std::io::{BufReader, BufWriter}; +use std::path::PathBuf; + +use crate::models::agent_state::KnowledgeContext; +use crate::models::knowledge::{Patterns, SiteConfig, UserModel}; + +use super::error::Result; +use super::traits::KnowledgeStorage; + +/// File-based persistence for site configs, user models, and patterns. +#[derive(Debug)] +pub struct FileKnowledgeStore { + dir: PathBuf, +} + +impl FileKnowledgeStore { + /// Open the knowledge store at the given directory. + pub fn open(dir: PathBuf) -> Result { + fs::create_dir_all(&dir)?; + fs::create_dir_all(dir.join("site_configs"))?; + fs::create_dir_all(dir.join("user_models"))?; + fs::create_dir_all(dir.join("patterns"))?; + Ok(Self { dir }) + } + + /// Get the path for a site config file. + fn site_config_path(&self, domain: &str) -> PathBuf { + self.dir.join("site_configs").join(format!("{}.json", domain)) + } + + /// Get the path for a user model file. + fn user_model_path(&self, user_id: &str) -> PathBuf { + self.dir.join("user_models").join(format!("{}.json", user_id)) + } + + /// Get the path for the patterns file. + fn patterns_path(&self) -> PathBuf { + self.dir.join("patterns").join("patterns.json") + } + + /// Load JSON from a file, returning None if it doesn't exist. + fn load_json(&self, path: &PathBuf) -> Result> { + if !path.exists() { + return Ok(None); + } + let file = File::open(path)?; + let reader = BufReader::new(file); + let value = serde_json::from_reader(reader)?; + Ok(Some(value)) + } + + /// Load JSON from a file, erroring if it doesn't exist. + fn load_json_required(&self, path: &PathBuf) -> Result { + let file = File::open(path)?; + let reader = BufReader::new(file); + let value = serde_json::from_reader(reader)?; + Ok(value) + } + + /// Save JSON to a file. + fn save_json(&self, path: &PathBuf, value: &T) -> Result<()> { + let file = File::create(path)?; + let writer = BufWriter::new(file); + serde_json::to_writer_pretty(writer, value)?; + Ok(()) + } +} + +impl KnowledgeStorage for FileKnowledgeStore { + fn get_site_config(&self, domain: &str) -> Result> { + let path = self.site_config_path(domain); + if !path.exists() { + // Try default config + let default_path = self.site_config_path("_default"); + if default_path.exists() { + return self.load_json(&default_path); + } + return Ok(None); + } + self.load_json(&path) + } + + fn save_site_config(&self, config: &SiteConfig) -> Result<()> { + let path = self.site_config_path(&config.domain); + self.save_json(&path, config) + } + + fn list_site_configs(&self) -> Result> { + let mut domains = Vec::new(); + let configs_dir = self.dir.join("site_configs"); + + for entry in fs::read_dir(configs_dir)? { + let entry = entry?; + let path = entry.path(); + + if path.extension().map(|e| e == "json").unwrap_or(false) + && let Some(stem) = path.file_stem() + { + let name = stem.to_string_lossy(); + if name != "_default" { + domains.push(name.to_string()); + } + } + } + + domains.sort(); + Ok(domains) + } + + fn delete_site_config(&self, domain: &str) -> Result<()> { + let path = self.site_config_path(domain); + if path.exists() { + fs::remove_file(path)?; + } + Ok(()) + } + + fn get_user_model(&self, user_id: &str) -> Result> { + let path = self.user_model_path(user_id); + if !path.exists() { + return Ok(None); + } + self.load_json(&path) + } + + fn save_user_model(&self, model: &UserModel) -> Result<()> { + let path = self.user_model_path(&model.user_id); + self.save_json(&path, model) + } + + fn list_user_models(&self) -> Result> { + let mut users = Vec::new(); + let models_dir = self.dir.join("user_models"); + + for entry in fs::read_dir(models_dir)? { + let entry = entry?; + let path = entry.path(); + + if path.extension().map(|e| e == "json").unwrap_or(false) + && let Some(stem) = path.file_stem() + { + let name = stem.to_string_lossy(); + users.push(name.to_string()); + } + } + + users.sort(); + Ok(users) + } + + fn get_patterns(&self) -> Result { + let path = self.patterns_path(); + if !path.exists() { + return Ok(Patterns::default()); + } + self.load_json_required(&path) + } + + fn save_patterns(&self, patterns: &Patterns) -> Result<()> { + let path = self.patterns_path(); + self.save_json(&path, patterns) + } + + fn load_for_context(&self, domain: Option<&str>) -> Result { + let mut ctx = KnowledgeContext { + token_budget: 8000, + ..Default::default() + }; + + // Load site config if domain specified + if let Some(d) = domain + && let Some(config) = self.get_site_config(d)? + { + ctx.site_configs.push(serde_json::to_string(&config)?); + } + + // Load patterns + let patterns = self.get_patterns()?; + if !patterns.success_patterns.is_empty() || !patterns.anti_patterns.is_empty() { + ctx.patterns.push(serde_json::to_string(&patterns)?); + } + + // Load default user model + if let Some(model) = self.get_user_model("default")? { + ctx.user_model = Some(serde_json::to_string(&model)?); + } + + // Estimate tokens (rough: ~4 chars per token) + let total_len: usize = ctx.site_configs.iter().map(|s| s.len()).sum::() + + ctx.patterns.iter().map(|s| s.len()).sum::() + + ctx.user_model.as_ref().map(|s| s.len()).unwrap_or(0); + ctx.estimated_tokens = (total_len / 4) as u64; + + Ok(ctx) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::models::ParseMethod; + use tempfile::tempdir; + + #[test] + fn site_config_roundtrip() { + let dir = tempdir().unwrap(); + let store = FileKnowledgeStore::open(dir.path().to_path_buf()).unwrap(); + + let config = SiteConfig::new("test.com"); + store.save_site_config(&config).unwrap(); + + let loaded = store.get_site_config("test.com").unwrap().unwrap(); + assert_eq!(loaded.domain, "test.com"); + assert_eq!(loaded.preferred_method, ParseMethod::SchemaOrg); + } + + #[test] + fn user_model_roundtrip() { + let dir = tempdir().unwrap(); + let store = FileKnowledgeStore::open(dir.path().to_path_buf()).unwrap(); + + let model = UserModel::default_user(); + store.save_user_model(&model).unwrap(); + + let loaded = store.get_user_model("default").unwrap().unwrap(); + assert_eq!(loaded.user_id, "default"); + } + + #[test] + fn patterns_roundtrip() { + let dir = tempdir().unwrap(); + let store = FileKnowledgeStore::open(dir.path().to_path_buf()).unwrap(); + + let mut patterns = Patterns::default(); + patterns.success_patterns.push(crate::models::knowledge::SuccessPattern { + description: "Test pattern".to_string(), + sites: vec!["example.com".to_string()], + success_rate: 0.9, + sample_size: 100, + confidence: 0.85, + }); + store.save_patterns(&patterns).unwrap(); + + let loaded = store.get_patterns().unwrap(); + assert_eq!(loaded.success_patterns.len(), 1); + assert_eq!(loaded.success_patterns[0].description, "Test pattern"); + } + + #[test] + fn list_site_configs() { + let dir = tempdir().unwrap(); + let store = FileKnowledgeStore::open(dir.path().to_path_buf()).unwrap(); + + store.save_site_config(&SiteConfig::new("a.com")).unwrap(); + store.save_site_config(&SiteConfig::new("b.com")).unwrap(); + store.save_site_config(&SiteConfig::new("c.com")).unwrap(); + + let list = store.list_site_configs().unwrap(); + assert_eq!(list, vec!["a.com", "b.com", "c.com"]); + } + + #[test] + fn missing_config_returns_none() { + let dir = tempdir().unwrap(); + let store = FileKnowledgeStore::open(dir.path().to_path_buf()).unwrap(); + + let config = store.get_site_config("nonexistent.com").unwrap(); + assert!(config.is_none()); + } + + #[test] + fn load_for_context_includes_relevant_data() { + let dir = tempdir().unwrap(); + let store = FileKnowledgeStore::open(dir.path().to_path_buf()).unwrap(); + + store + .save_site_config(&SiteConfig::new("example.com")) + .unwrap(); + store.save_user_model(&UserModel::default_user()).unwrap(); + + let ctx = store.load_for_context(Some("example.com")).unwrap(); + assert_eq!(ctx.site_configs.len(), 1); + assert!(ctx.user_model.is_some()); + } + + #[test] + fn delete_site_config() { + let dir = tempdir().unwrap(); + let store = FileKnowledgeStore::open(dir.path().to_path_buf()).unwrap(); + + store + .save_site_config(&SiteConfig::new("example.com")) + .unwrap(); + assert!(store.get_site_config("example.com").unwrap().is_some()); + + store.delete_site_config("example.com").unwrap(); + assert!(store.get_site_config("example.com").unwrap().is_none()); + } +} diff --git a/src/storage/mod.rs b/src/storage/mod.rs new file mode 100644 index 0000000..41653b0 --- /dev/null +++ b/src/storage/mod.rs @@ -0,0 +1,119 @@ +//! Storage layer for the recipe scraping agent. +//! +//! This module provides persistence for: +//! - Recipe storage with search index +//! - Signal logging (daily JSONL files) +//! - Knowledge store (site configs, user models, patterns) +//! - Session state for resumability +//! +//! # Architecture +//! +//! Storage is abstracted through traits to allow multiple backends: +//! - [`RecipesStorage`] - Recipe CRUD operations +//! - [`SignalStorage`] - Signal logging +//! - [`KnowledgeStorage`] - Knowledge persistence +//! - [`SessionStorage`] - Session state +//! - [`Storage`] - Combined interface +//! +//! # Backends +//! +//! For recipes, two backends are provided: +//! - [`FileRecipesDb`] - Uses JSONL files (default) +//! - [`SqliteRecipesDb`] - Uses SQLite database +//! +//! For other storage, file-based implementations are used: +//! - [`FileSignalLog`] - Daily JSONL files +//! - [`FileKnowledgeStore`] - JSON files +//! - [`FileSessionStore`] - JSON files +//! +//! # Directory Structure +//! +//! ```text +//! ~/.do-something/ +//! ├── config.json # Runtime config +//! ├── knowledge/ +//! │ ├── site_configs/ +//! │ ├── user_models/ +//! │ └── patterns/ +//! ├── recipes/ +//! │ ├── recipes.jsonl +//! │ └── index.json +//! ├── signals/ +//! │ └── YYYY-MM-DD.jsonl +//! └── state/ +//! └── sessions/ +//! ``` + +pub mod config_dir; +pub mod error; +pub mod file_storage; +pub mod knowledge_store; +pub mod recipes_db; +pub mod session_store; +pub mod signal_log; +pub mod sqlite_recipes; +pub mod traits; + +// Re-export file-based implementations +pub use config_dir::ConfigDir; +pub use error::{Result, StorageError}; +pub use file_storage::FileStorage; +pub use knowledge_store::FileKnowledgeStore; +pub use recipes_db::FileRecipesDb; +pub use session_store::FileSessionStore; +pub use signal_log::FileSignalLog; + +// Re-export SQLite recipes implementation +pub use sqlite_recipes::SqliteRecipesDb; + +// Re-export traits +pub use traits::{KnowledgeStorage, RecipesStorage, SessionStorage, SignalStorage, Storage}; + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::tempdir; + + #[test] + fn full_storage_workflow() { + let dir = tempdir().unwrap(); + let config = ConfigDir::from_path(dir.path().to_path_buf()); + config.init().unwrap(); + + // Open all storage components + let recipes = FileRecipesDb::open(config.recipes_dir()).unwrap(); + let _signals = FileSignalLog::open(config.signals_dir()).unwrap(); + let _knowledge = FileKnowledgeStore::open(config.knowledge_dir()).unwrap(); + let sessions = FileSessionStore::open(config.sessions_dir()).unwrap(); + + // Verify all directories exist + assert!(config.recipes_dir().exists()); + assert!(config.signals_dir().exists()); + assert!(config.knowledge_dir().exists()); + assert!(config.sessions_dir().exists()); + + // Verify we can use each component + use traits::{RecipesStorage, SessionStorage}; + assert_eq!(recipes.count().unwrap(), 0); + assert_eq!(sessions.count().unwrap(), 0); + } + + #[test] + fn storage_trait_implementation() { + let dir = tempdir().unwrap(); + let storage = FileStorage::open_at(dir.path().to_path_buf()).unwrap(); + + // Can use through trait methods + use traits::{RecipesStorage, SessionStorage}; + assert_eq!(storage.recipes().count().unwrap(), 0); + assert_eq!(storage.sessions().count().unwrap(), 0); + } + + #[test] + fn sqlite_recipes_works() { + let sqlite_db = SqliteRecipesDb::open_in_memory().unwrap(); + + use traits::RecipesStorage; + assert_eq!(sqlite_db.count().unwrap(), 0); + } +} diff --git a/src/storage/recipes_db.rs b/src/storage/recipes_db.rs new file mode 100644 index 0000000..1471266 --- /dev/null +++ b/src/storage/recipes_db.rs @@ -0,0 +1,357 @@ +//! File-based recipe storage with append-only JSONL and search index. +//! +//! Provides CRUD operations for recipes with duplicate detection +//! and full-text search capabilities. + +use std::fs::{self, File, OpenOptions}; +use std::io::{BufRead, BufReader, BufWriter, Write}; +use std::path::PathBuf; +use std::sync::{Arc, RwLock}; + +use serde::{Deserialize, Serialize}; + +use crate::models::recipe::{Recipe, RecipeId}; + +use super::error::Result; +use super::traits::RecipesStorage; + +/// In-memory index for fast lookups. +#[derive(Debug, Default)] +struct SearchIndex { + by_url: std::collections::HashMap, + by_content_hash: std::collections::HashMap, + by_name: Vec<(String, RecipeId)>, +} + +/// File-based, append-only recipe storage with search index. +#[derive(Debug)] +pub struct FileRecipesDb { + dir: PathBuf, + index: Arc>, +} + +impl FileRecipesDb { + /// Open the recipes database at the given directory. + pub fn open(dir: PathBuf) -> Result { + fs::create_dir_all(&dir)?; + let db = Self { + dir, + index: Arc::new(RwLock::new(SearchIndex::default())), + }; + db.rebuild_index()?; + Ok(db) + } + + /// Get the path to the recipes JSONL file. + fn recipes_file(&self) -> PathBuf { + self.dir.join("recipes.jsonl") + } + + /// Get the path to the index file. + fn index_file(&self) -> PathBuf { + self.dir.join("index.json") + } + + /// Check if content hash already exists. + pub fn exists_by_content_hash(&self, hash: &str) -> Result { + let index = self.index.read().unwrap(); + Ok(index.by_content_hash.contains_key(hash)) + } + + /// Save index to disk. + fn save_index(&self) -> Result<()> { + let index = self.index.read().unwrap(); + let persisted = PersistedIndex { + urls: index.by_url.clone(), + hashes: index.by_content_hash.clone(), + names: index.by_name.clone(), + }; + let file = File::create(self.index_file())?; + serde_json::to_writer(file, &persisted)?; + Ok(()) + } + + /// Load index from disk. + #[allow(dead_code)] + fn load_index(&self) -> Result<()> { + if !self.index_file().exists() { + return Ok(()); + } + + let file = File::open(self.index_file())?; + let persisted: PersistedIndex = serde_json::from_reader(file)?; + + let mut index = self.index.write().unwrap(); + index.by_url = persisted.urls; + index.by_content_hash = persisted.hashes; + index.by_name = persisted.names; + + Ok(()) + } +} + +impl RecipesStorage for FileRecipesDb { + fn insert(&self, recipe: &Recipe) -> Result { + // Check for duplicates by content hash + if let Some(hash) = &recipe.content_hash + && self.exists_by_content_hash(hash)? + { + // Return existing ID + let index = self.index.read().unwrap(); + if let Some(id) = index.by_content_hash.get(hash) { + return Ok(id.clone()); + } + } + + let id = RecipeId::generate(); + let mut recipe = recipe.clone(); + recipe.id = id.clone(); + + // Compute content hash if not present + if recipe.content_hash.is_none() { + recipe.content_hash = Some(recipe.compute_hash()); + } + + // Append to JSONL file + let file = OpenOptions::new() + .create(true) + .append(true) + .open(self.recipes_file())?; + let mut writer = BufWriter::new(file); + let json = serde_json::to_string(&recipe)?; + writeln!(writer, "{}", json)?; + + // Update index + { + let mut index = self.index.write().unwrap(); + index + .by_url + .insert(recipe.source_url.to_string(), id.clone()); + if let Some(hash) = &recipe.content_hash { + index.by_content_hash.insert(hash.clone(), id.clone()); + } + index + .by_name + .push((recipe.name.to_lowercase(), id.clone())); + } + + // Persist index + self.save_index()?; + + Ok(id) + } + + fn get(&self, id: &RecipeId) -> Result> { + let file = File::open(self.recipes_file())?; + let reader = BufReader::new(file); + + for line in reader.lines() { + let line = line?; + if let Ok(recipe) = serde_json::from_str::(&line) + && &recipe.id == id + { + return Ok(Some(recipe)); + } + } + + Ok(None) + } + + fn exists_by_url(&self, url: &str) -> Result { + let index = self.index.read().unwrap(); + Ok(index.by_url.contains_key(url)) + } + + fn search(&self, query: &str) -> Result> { + let index = self.index.read().unwrap(); + let query = query.to_lowercase(); + let terms: Vec<&str> = query.split_whitespace().collect(); + + let mut results: Vec = index + .by_name + .iter() + .filter(|(name, _)| terms.iter().all(|t| name.contains(t))) + .map(|(_, id)| id.clone()) + .collect(); + + // Deduplicate + results.sort_by(|a, b| a.0.cmp(&b.0)); + results.dedup_by(|a, b| a.0 == b.0); + + Ok(results) + } + + fn rebuild_index(&self) -> Result<()> { + let mut index = SearchIndex::default(); + + if let Ok(file) = File::open(self.recipes_file()) { + let reader = BufReader::new(file); + for line in reader.lines() { + let line = line?; + if let Ok(recipe) = serde_json::from_str::(&line) { + index + .by_url + .insert(recipe.source_url.to_string(), recipe.id.clone()); + if let Some(hash) = &recipe.content_hash { + index.by_content_hash.insert(hash.clone(), recipe.id.clone()); + } + index + .by_name + .push((recipe.name.to_lowercase(), recipe.id.clone())); + } + } + } + + *self.index.write().unwrap() = index; + self.save_index()?; + + Ok(()) + } + + fn count(&self) -> Result { + let index = self.index.read().unwrap(); + Ok(index.by_url.len() as u64) + } + + fn all(&self) -> Result> { + let mut recipes = Vec::new(); + if let Ok(file) = File::open(self.recipes_file()) { + let reader = BufReader::new(file); + for line in reader.lines() { + let line = line?; + if let Ok(recipe) = serde_json::from_str::(&line) { + recipes.push(recipe); + } + } + } + Ok(recipes) + } +} + +/// Persisted index structure. +#[derive(Debug, Serialize, Deserialize)] +struct PersistedIndex { + urls: std::collections::HashMap, + hashes: std::collections::HashMap, + names: Vec<(String, RecipeId)>, +} + +#[cfg(test)] +mod tests { + use super::*; + use chrono::Utc; + use tempfile::tempdir; + + fn test_recipe(name: &str, url: &str) -> Recipe { + Recipe { + id: RecipeId::generate(), + name: name.to_string(), + source_url: url.parse().unwrap(), + source_domain: "example.com".to_string(), + ingredients: vec![], + instructions: vec![], + prep_time_minutes: None, + cook_time_minutes: None, + total_time_minutes: None, + servings: None, + cuisine: None, + difficulty: None, + tags: vec![], + nutrition: None, + image_url: None, + author: None, + description: None, + scraped_at: Utc::now(), + content_hash: None, + meta: std::collections::HashMap::new(), + } + } + + #[test] + fn insert_generates_unique_ids() { + let dir = tempdir().unwrap(); + let db = FileRecipesDb::open(dir.path().to_path_buf()).unwrap(); + + let recipe1 = test_recipe("Recipe 1", "https://example.com/recipe1"); + let recipe2 = test_recipe("Recipe 2", "https://example.com/recipe2"); + + let id1 = db.insert(&recipe1).unwrap(); + let id2 = db.insert(&recipe2).unwrap(); + + assert_ne!(id1, id2); + } + + #[test] + fn get_returns_inserted_recipe() { + let dir = tempdir().unwrap(); + let db = FileRecipesDb::open(dir.path().to_path_buf()).unwrap(); + + let recipe = test_recipe("Test Recipe", "https://example.com/test"); + let id = db.insert(&recipe).unwrap(); + + let retrieved = db.get(&id).unwrap().unwrap(); + assert_eq!(retrieved.name, "Test Recipe"); + } + + #[test] + fn exists_by_url_detects_duplicates() { + let dir = tempdir().unwrap(); + let db = FileRecipesDb::open(dir.path().to_path_buf()).unwrap(); + + let recipe = test_recipe("Test", "https://example.com/test"); + db.insert(&recipe).unwrap(); + + assert!(db.exists_by_url("https://example.com/test").unwrap()); + assert!(!db.exists_by_url("https://example.com/other").unwrap()); + } + + #[test] + fn search_finds_matching_recipes() { + let dir = tempdir().unwrap(); + let db = FileRecipesDb::open(dir.path().to_path_buf()).unwrap(); + + let recipe1 = test_recipe("Chocolate Cake", "https://example.com/cake"); + let recipe2 = test_recipe("Apple Pie", "https://example.com/pie"); + let recipe3 = test_recipe("Chocolate Mousse", "https://example.com/mousse"); + + db.insert(&recipe1).unwrap(); + db.insert(&recipe2).unwrap(); + db.insert(&recipe3).unwrap(); + + let results = db.search("chocolate").unwrap(); + assert_eq!(results.len(), 2); + } + + #[test] + fn rebuild_index_restores_state() { + let dir = tempdir().unwrap(); + let db = FileRecipesDb::open(dir.path().to_path_buf()).unwrap(); + + let recipe = test_recipe("Test", "https://example.com/test"); + db.insert(&recipe).unwrap(); + + // Open a new instance + let db2 = FileRecipesDb::open(dir.path().to_path_buf()).unwrap(); + + assert!(db2.exists_by_url("https://example.com/test").unwrap()); + } + + #[test] + fn duplicate_by_content_hash_not_inserted() { + let dir = tempdir().unwrap(); + let db = FileRecipesDb::open(dir.path().to_path_buf()).unwrap(); + + let mut recipe1 = test_recipe("Test Recipe", "https://example.com/test1"); + recipe1.content_hash = Some("hash123".to_string()); + + let mut recipe2 = test_recipe("Test Recipe", "https://example.com/test2"); + recipe2.content_hash = Some("hash123".to_string()); + + let id1 = db.insert(&recipe1).unwrap(); + let id2 = db.insert(&recipe2).unwrap(); + + // Should return the same ID for duplicates + assert_eq!(id1, id2); + assert_eq!(db.count().unwrap(), 1); + } +} diff --git a/src/storage/session_store.rs b/src/storage/session_store.rs new file mode 100644 index 0000000..599b03f --- /dev/null +++ b/src/storage/session_store.rs @@ -0,0 +1,210 @@ +//! File-based session state persistence for resumability. +//! +//! Stores session state to disk so that long-running scraping +//! sessions can be resumed after interruption. + +use std::fs::{self, File}; +use std::io::{BufReader, BufWriter}; +use std::path::PathBuf; + +use crate::models::agent_state::{SessionId, SessionState}; + +use super::error::Result; +use super::traits::SessionStorage; + +/// File-based persistable session state for resumability. +#[derive(Debug)] +pub struct FileSessionStore { + dir: PathBuf, +} + +impl FileSessionStore { + /// Open the session store at the given directory. + pub fn open(dir: PathBuf) -> Result { + fs::create_dir_all(&dir)?; + Ok(Self { dir }) + } + + /// Get the path for a session file. + fn session_path(&self, id: &SessionId) -> PathBuf { + self.dir.join(format!("{}.json", id)) + } +} + +impl SessionStorage for FileSessionStore { + fn save(&self, session: &SessionState) -> Result<()> { + let path = self.session_path(&session.id); + let file = File::create(&path)?; + let writer = BufWriter::new(file); + serde_json::to_writer_pretty(writer, session)?; + Ok(()) + } + + fn load(&self, id: &SessionId) -> Result> { + let path = self.session_path(id); + if !path.exists() { + return Ok(None); + } + let file = File::open(&path)?; + let reader = BufReader::new(file); + let session = serde_json::from_reader(reader)?; + Ok(Some(session)) + } + + fn list(&self) -> Result> { + let mut ids = Vec::new(); + + for entry in fs::read_dir(&self.dir)? { + let entry = entry?; + let path = entry.path(); + + if path.extension().map(|e| e == "json").unwrap_or(false) + && let Some(stem) = path.file_stem() + { + let name = stem.to_string_lossy(); + ids.push(SessionId::new(name.to_string())); + } + } + + // Sort by modification time, newest first + ids.sort_by(|a, b| { + let path_a = self.session_path(a); + let path_b = self.session_path(b); + let time_a = path_a.metadata().and_then(|m| m.modified()).ok(); + let time_b = path_b.metadata().and_then(|m| m.modified()).ok(); + time_b.cmp(&time_a) + }); + + Ok(ids) + } + + fn delete(&self, id: &SessionId) -> Result<()> { + let path = self.session_path(id); + if path.exists() { + fs::remove_file(path)?; + } + Ok(()) + } + + fn exists(&self, id: &SessionId) -> Result { + Ok(self.session_path(id).exists()) + } + + fn latest(&self) -> Result> { + let sessions = self.list()?; + if let Some(latest_id) = sessions.first() { + return self.load(latest_id); + } + Ok(None) + } + + fn count(&self) -> Result { + Ok(self.list()?.len()) + } + + fn clear(&self) -> Result { + let ids = self.list()?; + let count = ids.len() as u64; + for id in ids { + self.delete(&id)?; + } + Ok(count) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::tempdir; + + #[test] + fn save_and_load_session() { + let dir = tempdir().unwrap(); + let store = FileSessionStore::open(dir.path().to_path_buf()).unwrap(); + + let session = SessionState::new(SessionId::new("test-session"), "test-profile"); + store.save(&session).unwrap(); + + let loaded = store.load(&SessionId::new("test-session")).unwrap().unwrap(); + assert_eq!(loaded.id.0, "test-session"); + assert_eq!(loaded.profile_name, "test-profile"); + } + + #[test] + fn load_nonexistent_returns_none() { + let dir = tempdir().unwrap(); + let store = FileSessionStore::open(dir.path().to_path_buf()).unwrap(); + + let loaded = store.load(&SessionId::new("nonexistent")).unwrap(); + assert!(loaded.is_none()); + } + + #[test] + fn list_sessions() { + let dir = tempdir().unwrap(); + let store = FileSessionStore::open(dir.path().to_path_buf()).unwrap(); + + store + .save(&SessionState::new(SessionId::new("session-a"), "p1")) + .unwrap(); + store + .save(&SessionState::new(SessionId::new("session-b"), "p2")) + .unwrap(); + + let list = store.list().unwrap(); + assert_eq!(list.len(), 2); + } + + #[test] + fn delete_session() { + let dir = tempdir().unwrap(); + let store = FileSessionStore::open(dir.path().to_path_buf()).unwrap(); + + let id = SessionId::new("to-delete"); + store + .save(&SessionState::new(id.clone(), "profile")) + .unwrap(); + assert!(store.exists(&id).unwrap()); + + store.delete(&id).unwrap(); + assert!(!store.exists(&id).unwrap()); + } + + #[test] + fn latest_returns_most_recent() { + let dir = tempdir().unwrap(); + let store = FileSessionStore::open(dir.path().to_path_buf()).unwrap(); + + store + .save(&SessionState::new(SessionId::new("old"), "p1")) + .unwrap(); + // Small delay to ensure different timestamps + std::thread::sleep(std::time::Duration::from_millis(10)); + store + .save(&SessionState::new(SessionId::new("new"), "p2")) + .unwrap(); + + let latest = store.latest().unwrap().unwrap(); + assert_eq!(latest.id.0, "new"); + } + + #[test] + fn clear_removes_all_sessions() { + let dir = tempdir().unwrap(); + let store = FileSessionStore::open(dir.path().to_path_buf()).unwrap(); + + store + .save(&SessionState::new(SessionId::new("s1"), "p")) + .unwrap(); + store + .save(&SessionState::new(SessionId::new("s2"), "p")) + .unwrap(); + store + .save(&SessionState::new(SessionId::new("s3"), "p")) + .unwrap(); + + let count = store.clear().unwrap(); + assert_eq!(count, 3); + assert_eq!(store.count().unwrap(), 0); + } +} diff --git a/src/storage/signal_log.rs b/src/storage/signal_log.rs new file mode 100644 index 0000000..f099c48 --- /dev/null +++ b/src/storage/signal_log.rs @@ -0,0 +1,281 @@ +//! File-based daily JSONL signal logging. +//! +//! Signals are appended to date-based JSONL files for audit trails +//! and later compression into knowledge. + +use std::fs::{self, File}; +use std::io::{BufRead, BufReader, BufWriter, Write}; +use std::path::PathBuf; + +use chrono::{Duration, NaiveDate, Utc}; + +use crate::models::signal::Signal; + +use super::error::Result; +use super::traits::SignalStorage; + +/// File-based daily JSONL files for signal logging. +#[derive(Debug)] +pub struct FileSignalLog { + dir: PathBuf, +} + +impl FileSignalLog { + /// Open the signal log at the given directory. + pub fn open(dir: PathBuf) -> Result { + fs::create_dir_all(&dir)?; + Ok(Self { dir }) + } + + /// Get the path to a date's log file. + fn log_file_for_date(&self, date: NaiveDate) -> PathBuf { + self.dir.join(format!("{}.jsonl", date.format("%Y-%m-%d"))) + } + + /// Get today's log file path. + fn today_file(&self) -> PathBuf { + self.log_file_for_date(Utc::now().date_naive()) + } + + /// Count signals in a single file. + fn count_signals_in_file(&self, path: &std::path::Path) -> Result { + let file = File::open(path)?; + let reader = BufReader::new(file); + let count = reader.lines().filter(|l| l.is_ok()).count(); + Ok(count as u64) + } +} + +impl SignalStorage for FileSignalLog { + fn append(&self, signal: &Signal) -> Result<()> { + let file = OpenOptions::new() + .create(true) + .append(true) + .open(self.today_file())?; + let mut writer = BufWriter::new(file); + let json = serde_json::to_string(signal)?; + writeln!(writer, "{}", json)?; + Ok(()) + } + + fn read_range(&self, from: NaiveDate, to: NaiveDate) -> Result> { + let mut signals = Vec::new(); + let mut current = from; + + while current <= to { + let file_path = self.log_file_for_date(current); + if file_path.exists() { + let file = File::open(file_path)?; + let reader = BufReader::new(file); + for line in reader.lines() { + let line = line?; + if let Ok(signal) = serde_json::from_str::(&line) { + signals.push(signal); + } + } + } + current += Duration::days(1); + } + + Ok(signals) + } + + fn count_for_domain(&self, domain: &str, days: u32) -> Result { + let from = Utc::now().date_naive() - Duration::days(days as i64); + let to = Utc::now().date_naive(); + + let signals = self.read_range(from, to)?; + let count = signals + .iter() + .filter(|s| s.domain.as_deref() == Some(domain)) + .count(); + + Ok(count as u64) + } + + fn prune(&self, older_than_days: u32) -> Result { + let cutoff = Utc::now().date_naive() - Duration::days(older_than_days as i64); + let mut pruned = 0u64; + + for entry in fs::read_dir(&self.dir)? { + let entry = entry?; + let path = entry.path(); + + if path.extension().map(|e| e == "jsonl").unwrap_or(false) + && let Some(filename) = path.file_stem() + { + let filename_str = filename.to_string_lossy(); + if let Ok(date) = NaiveDate::parse_from_str(&filename_str, "%Y-%m-%d") + && date < cutoff + { + let count = self.count_signals_in_file(&path)?; + fs::remove_file(path)?; + pruned += count; + } + } + } + + Ok(pruned) + } + + fn read_date(&self, date: NaiveDate) -> Result> { + let file_path = self.log_file_for_date(date); + if !file_path.exists() { + return Ok(Vec::new()); + } + + let file = File::open(file_path)?; + let reader = BufReader::new(file); + let mut signals = Vec::new(); + + for line in reader.lines() { + let line = line?; + if let Ok(signal) = serde_json::from_str::(&line) { + signals.push(signal); + } + } + + Ok(signals) + } + + fn available_dates(&self) -> Result> { + let mut dates = Vec::new(); + + for entry in fs::read_dir(&self.dir)? { + let entry = entry?; + let path = entry.path(); + + if path.extension().map(|e| e == "jsonl").unwrap_or(false) + && let Some(filename) = path.file_stem() + { + let filename_str = filename.to_string_lossy(); + if let Ok(naive) = NaiveDate::parse_from_str(&filename_str, "%Y-%m-%d") { + dates.push(naive); + } + } + } + + dates.sort(); + Ok(dates) + } +} + +// Need to import OpenOptions +use std::fs::OpenOptions; + +#[cfg(test)] +mod tests { + use super::*; + use crate::models::signal::SignalType; + use tempfile::tempdir; + + #[test] + fn append_creates_daily_file() { + let dir = tempdir().unwrap(); + let log = FileSignalLog::open(dir.path().to_path_buf()).unwrap(); + + let signal = Signal::new(SignalType::ParseSuccess { + method: crate::models::ParseMethod::SchemaOrg, + time_ms: 350, + }); + + log.append(&signal).unwrap(); + + let today_path = log.today_file(); + assert!(today_path.exists()); + } + + #[test] + fn read_range_returns_signals() { + let dir = tempdir().unwrap(); + let log = FileSignalLog::open(dir.path().to_path_buf()).unwrap(); + + let signal1 = Signal::new(SignalType::ParseSuccess { + method: crate::models::ParseMethod::SchemaOrg, + time_ms: 100, + }) + .with_domain("example.com"); + + let signal2 = Signal::new(SignalType::ParseSuccess { + method: crate::models::ParseMethod::Selectors, + time_ms: 200, + }) + .with_domain("other.com"); + + log.append(&signal1).unwrap(); + log.append(&signal2).unwrap(); + + let today = Utc::now().date_naive(); + let signals = log.read_range(today, today).unwrap(); + + assert_eq!(signals.len(), 2); + } + + #[test] + fn count_for_domain_filters_correctly() { + let dir = tempdir().unwrap(); + let log = FileSignalLog::open(dir.path().to_path_buf()).unwrap(); + + for _ in 0..5 { + log.append( + &Signal::new(SignalType::ParseSuccess { + method: crate::models::ParseMethod::SchemaOrg, + time_ms: 100, + }) + .with_domain("example.com"), + ) + .unwrap(); + } + + for _ in 0..3 { + log.append( + &Signal::new(SignalType::ParseSuccess { + method: crate::models::ParseMethod::SchemaOrg, + time_ms: 100, + }) + .with_domain("other.com"), + ) + .unwrap(); + } + + let count = log.count_for_domain("example.com", 1).unwrap(); + assert_eq!(count, 5); + + let count = log.count_for_domain("other.com", 1).unwrap(); + assert_eq!(count, 3); + } + + #[test] + fn prune_removes_old_files() { + let dir = tempdir().unwrap(); + let log = FileSignalLog::open(dir.path().to_path_buf()).unwrap(); + + // Create a file for an old date + let old_date = Utc::now().date_naive() - Duration::days(10); + let old_file = log.log_file_for_date(old_date); + let mut file = File::create(old_file).unwrap(); + writeln!( + file, + "{}", + serde_json::to_string(&Signal::new(SignalType::ParseSuccess { + method: crate::models::ParseMethod::SchemaOrg, + time_ms: 100 + })) + .unwrap() + ) + .unwrap(); + + // Create today's file + log.append(&Signal::new(SignalType::ParseSuccess { + method: crate::models::ParseMethod::SchemaOrg, + time_ms: 100, + })) + .unwrap(); + + let pruned = log.prune(5).unwrap(); + assert_eq!(pruned, 1); + + assert!(log.today_file().exists()); + assert!(!log.log_file_for_date(old_date).exists()); + } +} diff --git a/src/storage/sqlite_recipes.rs b/src/storage/sqlite_recipes.rs new file mode 100644 index 0000000..ac0b229 --- /dev/null +++ b/src/storage/sqlite_recipes.rs @@ -0,0 +1,390 @@ +//! SQLite-based recipe storage. +//! +//! Provides recipe persistence using SQLite for efficient querying +//! and full-text search capabilities. + +use rusqlite::{Connection, params, OptionalExtension}; +use std::path::PathBuf; +use std::sync::{Arc, Mutex}; + +use crate::models::recipe::{Recipe, RecipeId}; + +use super::error::Result; +use super::traits::RecipesStorage; + +/// SQLite-based recipe storage. +#[derive(Debug)] +pub struct SqliteRecipesDb { + conn: Arc>, +} + +impl SqliteRecipesDb { + /// Open the recipes database at the given path. + pub fn open(path: PathBuf) -> Result { + let conn = Connection::open(path)?; + let db = Self { + conn: Arc::new(Mutex::new(conn)), + }; + db.init_tables()?; + Ok(db) + } + + /// Open an in-memory database (for testing). + pub fn open_in_memory() -> Result { + let conn = Connection::open_in_memory()?; + let db = Self { + conn: Arc::new(Mutex::new(conn)), + }; + db.init_tables()?; + Ok(db) + } + + /// Initialize database tables. + fn init_tables(&self) -> Result<()> { + let conn = self.conn.lock().unwrap(); + + conn.execute_batch( + r#" + CREATE TABLE IF NOT EXISTS recipes ( + id TEXT PRIMARY KEY, + name TEXT NOT NULL, + source_url TEXT UNIQUE NOT NULL, + source_domain TEXT NOT NULL, + ingredients_json TEXT NOT NULL, + instructions_json TEXT NOT NULL, + prep_time_minutes INTEGER, + cook_time_minutes INTEGER, + total_time_minutes INTEGER, + servings_json TEXT, + cuisine TEXT, + difficulty TEXT, + tags_json TEXT NOT NULL, + nutrition_json TEXT, + image_url TEXT, + author TEXT, + description TEXT, + scraped_at TEXT NOT NULL, + content_hash TEXT UNIQUE, + meta_json TEXT NOT NULL + ); + + CREATE INDEX IF NOT EXISTS idx_recipes_url ON recipes(source_url); + CREATE INDEX IF NOT EXISTS idx_recipes_hash ON recipes(content_hash); + CREATE INDEX IF NOT EXISTS idx_recipes_domain ON recipes(source_domain); + CREATE INDEX IF NOT EXISTS idx_recipes_name ON recipes(name); + + -- FTS5 virtual table for full-text search (standalone, no content table) + CREATE VIRTUAL TABLE IF NOT EXISTS recipes_fts USING fts5( + id UNINDEXED, + name, + ingredients_text + ); + "#, + )?; + + Ok(()) + } + + /// Serialize recipe ingredients to text for FTS. + fn ingredients_to_text(ingredients: &[crate::models::recipe::Ingredient]) -> String { + ingredients.iter().map(|i| i.raw.as_str()).collect::>().join(" ") + } +} + +impl RecipesStorage for SqliteRecipesDb { + fn insert(&self, recipe: &Recipe) -> Result { + let conn = self.conn.lock().unwrap(); + + // Check for duplicates by content hash + if let Some(hash) = &recipe.content_hash { + let existing: Option = conn + .query_row( + "SELECT id FROM recipes WHERE content_hash = ?", + params![hash], + |row| row.get(0), + ) + .optional()?; + + if let Some(id) = existing { + return Ok(RecipeId::new(id)); + } + } + + let id = RecipeId::generate(); + let mut recipe = recipe.clone(); + recipe.id = id.clone(); + + // Compute content hash if not present + if recipe.content_hash.is_none() { + recipe.content_hash = Some(recipe.compute_hash()); + } + + conn.execute( + r#" + INSERT INTO recipes ( + id, name, source_url, source_domain, ingredients_json, instructions_json, + prep_time_minutes, cook_time_minutes, total_time_minutes, servings_json, + cuisine, difficulty, tags_json, nutrition_json, image_url, author, + description, scraped_at, content_hash, meta_json + ) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18, ?19, ?20) + "#, + params![ + recipe.id.0, + recipe.name, + recipe.source_url.to_string(), + recipe.source_domain, + serde_json::to_string(&recipe.ingredients)?, + serde_json::to_string(&recipe.instructions)?, + recipe.prep_time_minutes, + recipe.cook_time_minutes, + recipe.total_time_minutes, + recipe.servings.as_ref().map(serde_json::to_string).transpose()?, + recipe.cuisine, + recipe.difficulty.map(|d| serde_json::to_string(&d)).transpose()?, + serde_json::to_string(&recipe.tags)?, + recipe.nutrition.as_ref().map(serde_json::to_string).transpose()?, + recipe.image_url.as_ref().map(|u| u.to_string()), + recipe.author, + recipe.description, + recipe.scraped_at.to_rfc3339(), + recipe.content_hash, + serde_json::to_string(&recipe.meta)?, + ], + )?; + + // Insert into FTS + let ingredients_text = Self::ingredients_to_text(&recipe.ingredients); + conn.execute( + "INSERT INTO recipes_fts(id, name, ingredients_text) VALUES (?1, ?2, ?3)", + params![recipe.id.0, recipe.name, ingredients_text], + )?; + + Ok(id) + } + + fn get(&self, id: &RecipeId) -> Result> { + let conn = self.conn.lock().unwrap(); + + let result = conn + .query_row( + "SELECT * FROM recipes WHERE id = ?", + params![id.0], + |row| { + Ok(Recipe { + id: RecipeId::new(row.get::<_, String>(0)?), + name: row.get(1)?, + source_url: row.get::<_, String>(2)?.parse().map_err(|_| rusqlite::Error::InvalidQuery)?, + source_domain: row.get(3)?, + ingredients: serde_json::from_str(&row.get::<_, String>(4)?).map_err(|_| rusqlite::Error::InvalidQuery)?, + instructions: serde_json::from_str(&row.get::<_, String>(5)?).map_err(|_| rusqlite::Error::InvalidQuery)?, + prep_time_minutes: row.get(6)?, + cook_time_minutes: row.get(7)?, + total_time_minutes: row.get(8)?, + servings: row.get::<_, Option>(9)?.map(|s| serde_json::from_str(&s)).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, + cuisine: row.get(10)?, + difficulty: row.get::<_, Option>(11)?.map(|s| serde_json::from_str(&s)).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, + tags: serde_json::from_str(&row.get::<_, String>(12)?).map_err(|_| rusqlite::Error::InvalidQuery)?, + nutrition: row.get::<_, Option>(13)?.map(|s| serde_json::from_str(&s)).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, + image_url: row.get::<_, Option>(14)?.map(|s| s.parse()).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, + author: row.get(15)?, + description: row.get(16)?, + scraped_at: chrono::DateTime::parse_from_rfc3339(&row.get::<_, String>(17)?).map(|dt| dt.with_timezone(&chrono::Utc)).map_err(|_| rusqlite::Error::InvalidQuery)?, + content_hash: row.get(18)?, + meta: serde_json::from_str(&row.get::<_, String>(19)?).map_err(|_| rusqlite::Error::InvalidQuery)?, + }) + }, + ) + .optional()?; + + Ok(result) + } + + fn exists_by_url(&self, url: &str) -> Result { + let conn = self.conn.lock().unwrap(); + let count: i64 = conn.query_row( + "SELECT COUNT(*) FROM recipes WHERE source_url = ?", + params![url], + |row| row.get(0), + )?; + Ok(count > 0) + } + + fn search(&self, query: &str) -> Result> { + let conn = self.conn.lock().unwrap(); + + let mut stmt = conn.prepare( + "SELECT id FROM recipes_fts WHERE recipes_fts MATCH ? ORDER BY rank LIMIT 100" + )?; + + let ids = stmt + .query_map(params![query], |row| row.get::<_, String>(0))? + .filter_map(|r| r.ok()) + .map(RecipeId::new) + .collect(); + + Ok(ids) + } + + fn rebuild_index(&self) -> Result<()> { + let conn = self.conn.lock().unwrap(); + conn.execute("INSERT INTO recipes_fts(recipes_fts) VALUES ('rebuild')", [])?; + Ok(()) + } + + fn count(&self) -> Result { + let conn = self.conn.lock().unwrap(); + let count: i64 = conn.query_row("SELECT COUNT(*) FROM recipes", [], |row| row.get(0))?; + Ok(count as u64) + } + + fn all(&self) -> Result> { + let conn = self.conn.lock().unwrap(); + + let mut stmt = conn.prepare( + "SELECT id, name, source_url, source_domain, ingredients_json, instructions_json, + prep_time_minutes, cook_time_minutes, total_time_minutes, servings_json, + cuisine, difficulty, tags_json, nutrition_json, image_url, author, + description, scraped_at, content_hash, meta_json FROM recipes" + )?; + + let recipes = stmt + .query_map([], |row| { + Ok(Recipe { + id: RecipeId::new(row.get::<_, String>(0)?), + name: row.get(1)?, + source_url: row.get::<_, String>(2)?.parse().map_err(|_| rusqlite::Error::InvalidQuery)?, + source_domain: row.get(3)?, + ingredients: serde_json::from_str(&row.get::<_, String>(4)?).map_err(|_| rusqlite::Error::InvalidQuery)?, + instructions: serde_json::from_str(&row.get::<_, String>(5)?).map_err(|_| rusqlite::Error::InvalidQuery)?, + prep_time_minutes: row.get(6)?, + cook_time_minutes: row.get(7)?, + total_time_minutes: row.get(8)?, + servings: row.get::<_, Option>(9)?.map(|s| serde_json::from_str(&s)).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, + cuisine: row.get(10)?, + difficulty: row.get::<_, Option>(11)?.map(|s| serde_json::from_str(&s)).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, + tags: serde_json::from_str(&row.get::<_, String>(12)?).map_err(|_| rusqlite::Error::InvalidQuery)?, + nutrition: row.get::<_, Option>(13)?.map(|s| serde_json::from_str(&s)).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, + image_url: row.get::<_, Option>(14)?.map(|s| s.parse()).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, + author: row.get(15)?, + description: row.get(16)?, + scraped_at: chrono::DateTime::parse_from_rfc3339(&row.get::<_, String>(17)?).map(|dt| dt.with_timezone(&chrono::Utc)).map_err(|_| rusqlite::Error::InvalidQuery)?, + content_hash: row.get(18)?, + meta: serde_json::from_str(&row.get::<_, String>(19)?).map_err(|_| rusqlite::Error::InvalidQuery)?, + }) + })? + .filter_map(|r| r.ok()) + .collect(); + + Ok(recipes) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use chrono::Utc; + use std::collections::HashMap; + + fn test_recipe(name: &str, url: &str) -> Recipe { + Recipe { + id: RecipeId::generate(), + name: name.to_string(), + source_url: url.parse().unwrap(), + source_domain: "example.com".to_string(), + ingredients: vec![], + instructions: vec![], + prep_time_minutes: None, + cook_time_minutes: None, + total_time_minutes: None, + servings: None, + cuisine: None, + difficulty: None, + tags: vec![], + nutrition: None, + image_url: None, + author: None, + description: None, + scraped_at: Utc::now(), + content_hash: None, + meta: HashMap::new(), + } + } + + #[test] + fn insert_and_get() { + let db = SqliteRecipesDb::open_in_memory().unwrap(); + + let recipe = test_recipe("Test Recipe", "https://example.com/test"); + let id = db.insert(&recipe).unwrap(); + + let retrieved = db.get(&id).unwrap().unwrap(); + assert_eq!(retrieved.name, "Test Recipe"); + } + + #[test] + fn generates_unique_ids() { + let db = SqliteRecipesDb::open_in_memory().unwrap(); + + let r1 = test_recipe("Recipe 1", "https://example.com/r1"); + let r2 = test_recipe("Recipe 2", "https://example.com/r2"); + + let id1 = db.insert(&r1).unwrap(); + let id2 = db.insert(&r2).unwrap(); + + assert_ne!(id1, id2); + } + + #[test] + fn exists_by_url() { + let db = SqliteRecipesDb::open_in_memory().unwrap(); + + let recipe = test_recipe("Test", "https://example.com/test"); + db.insert(&recipe).unwrap(); + + assert!(db.exists_by_url("https://example.com/test").unwrap()); + assert!(!db.exists_by_url("https://example.com/other").unwrap()); + } + + #[test] + fn search_finds_matches() { + let db = SqliteRecipesDb::open_in_memory().unwrap(); + + let r1 = test_recipe("Chocolate Cake", "https://example.com/cake"); + let r2 = test_recipe("Apple Pie", "https://example.com/pie"); + + db.insert(&r1).unwrap(); + db.insert(&r2).unwrap(); + + let results = db.search("chocolate").unwrap(); + assert_eq!(results.len(), 1); + } + + #[test] + fn duplicate_by_hash_not_inserted() { + let db = SqliteRecipesDb::open_in_memory().unwrap(); + + let mut r1 = test_recipe("Recipe 1", "https://example.com/r1"); + r1.content_hash = Some("hash123".to_string()); + + let mut r2 = test_recipe("Recipe 2", "https://example.com/r2"); + r2.content_hash = Some("hash123".to_string()); + + let id1 = db.insert(&r1).unwrap(); + let id2 = db.insert(&r2).unwrap(); + + assert_eq!(id1, id2); + assert_eq!(db.count().unwrap(), 1); + } + + #[test] + fn count_works() { + let db = SqliteRecipesDb::open_in_memory().unwrap(); + + assert_eq!(db.count().unwrap(), 0); + + db.insert(&test_recipe("R1", "https://example.com/r1")).unwrap(); + db.insert(&test_recipe("R2", "https://example.com/r2")).unwrap(); + + assert_eq!(db.count().unwrap(), 2); + } +} diff --git a/src/storage/traits.rs b/src/storage/traits.rs new file mode 100644 index 0000000..71c5704 --- /dev/null +++ b/src/storage/traits.rs @@ -0,0 +1,130 @@ +//! Storage trait definitions. +//! +//! These traits define the interfaces for storage backends, allowing +//! for multiple implementations (file-based, database, cloud, etc.). + +use crate::models::agent_state::{KnowledgeContext, SessionId, SessionState}; +use crate::models::knowledge::{Patterns, SiteConfig, UserModel}; +use crate::models::recipe::{Recipe, RecipeId}; +use crate::models::signal::Signal; + +use super::error::Result; +use chrono::NaiveDate; + +/// Recipe storage interface. +pub trait RecipesStorage: Send + Sync { + /// Insert a recipe and return its ID. + fn insert(&self, recipe: &Recipe) -> Result; + + /// Get a recipe by ID. + fn get(&self, id: &RecipeId) -> Result>; + + /// Check if a URL has already been scraped. + fn exists_by_url(&self, url: &str) -> Result; + + /// Full-text search across recipes. + fn search(&self, query: &str) -> Result>; + + /// Rebuild the search index. + fn rebuild_index(&self) -> Result<()>; + + /// Count total recipes. + fn count(&self) -> Result; + + /// Get all recipes (for iteration/export). + fn all(&self) -> Result>; +} + +/// Signal logging interface. +pub trait SignalStorage: Send + Sync { + /// Append a signal to today's log. + fn append(&self, signal: &Signal) -> Result<()>; + + /// Read signals from a date range. + fn read_range(&self, from: NaiveDate, to: NaiveDate) -> Result>; + + /// Count signals for a domain in the last N days. + fn count_for_domain(&self, domain: &str, days: u32) -> Result; + + /// Prune signals older than retention period. + fn prune(&self, older_than_days: u32) -> Result; + + /// Get signals for a specific date. + fn read_date(&self, date: NaiveDate) -> Result>; + + /// Get all available log dates. + fn available_dates(&self) -> Result>; +} + +/// Knowledge storage interface. +pub trait KnowledgeStorage: Send + Sync { + /// Get site configuration for a domain. + fn get_site_config(&self, domain: &str) -> Result>; + + /// Save site configuration. + fn save_site_config(&self, config: &SiteConfig) -> Result<()>; + + /// List all configured domains. + fn list_site_configs(&self) -> Result>; + + /// Delete site configuration for a domain. + fn delete_site_config(&self, domain: &str) -> Result<()>; + + /// Get user model for a user. + fn get_user_model(&self, user_id: &str) -> Result>; + + /// Save user model. + fn save_user_model(&self, model: &UserModel) -> Result<()>; + + /// List all user IDs with models. + fn list_user_models(&self) -> Result>; + + /// Get patterns. + fn get_patterns(&self) -> Result; + + /// Save patterns. + fn save_patterns(&self, patterns: &Patterns) -> Result<()>; + + /// Load knowledge for context injection. + fn load_for_context(&self, domain: Option<&str>) -> Result; +} + +/// Session storage interface. +pub trait SessionStorage: Send + Sync { + /// Save session state. + fn save(&self, session: &SessionState) -> Result<()>; + + /// Load session by ID. + fn load(&self, id: &SessionId) -> Result>; + + /// List all session IDs (sorted by modification time, newest first). + fn list(&self) -> Result>; + + /// Delete a session. + fn delete(&self, id: &SessionId) -> Result<()>; + + /// Check if a session exists. + fn exists(&self, id: &SessionId) -> Result; + + /// Get the most recent session. + fn latest(&self) -> Result>; + + /// Count total sessions. + fn count(&self) -> Result; + + /// Delete all sessions. + fn clear(&self) -> Result; +} + +/// Combined storage interface providing all storage components. +pub trait Storage: Send + Sync { + type Recipes: RecipesStorage; + type Signals: SignalStorage; + type Knowledge: KnowledgeStorage; + type Sessions: SessionStorage; + + fn recipes(&self) -> &Self::Recipes; + fn signals(&self) -> &Self::Signals; + fn knowledge(&self) -> &Self::Knowledge; + fn sessions(&self) -> &Self::Sessions; +} From f31e5c1f0415b9870c844d3b46fa4301f9a7a2f9 Mon Sep 17 00:00:00 2001 From: Ezra Lim Date: Thu, 2 Jul 2026 06:48:43 +0000 Subject: [PATCH 2/4] refactor(storage): remove index from file storage, keep FTS in SQLite only - Remove SearchIndex, PersistedIndex, and all index-related code from FileRecipesDb (recipes_db.rs) - Remove rebuild_index() from RecipesStorage trait - Rename SQLite rebuild_index to rebuild_fts_index as a struct method (not part of trait) - Add SQLite search_with_limit() for configurable result limits - Simplify FileRecipesDb to pure append-only JSONL with O(n) search - Fix OpenOptions import placement in signal_log.rs - Update module documentation to clarify backend differences - Add test for SQLite FTS rebuild --- src/storage/mod.rs | 57 +++++++-- src/storage/recipes_db.rs | 227 +++++++++++----------------------- src/storage/signal_log.rs | 4 +- src/storage/sqlite_recipes.rs | 46 ++++--- src/storage/traits.rs | 3 - 5 files changed, 148 insertions(+), 189 deletions(-) diff --git a/src/storage/mod.rs b/src/storage/mod.rs index 41653b0..8c49bbf 100644 --- a/src/storage/mod.rs +++ b/src/storage/mod.rs @@ -1,7 +1,7 @@ //! Storage layer for the recipe scraping agent. //! //! This module provides persistence for: -//! - Recipe storage with search index +//! - Recipe storage with search //! - Signal logging (daily JSONL files) //! - Knowledge store (site configs, user models, patterns) //! - Session state for resumability @@ -17,11 +17,16 @@ //! //! # Backends //! -//! For recipes, two backends are provided: -//! - [`FileRecipesDb`] - Uses JSONL files (default) -//! - [`SqliteRecipesDb`] - Uses SQLite database +//! ## Recipes +//! +//! Two backends are provided: +//! - [`FileRecipesDb`] - Simple JSONL storage. Good for development and small datasets. +//! Search is O(n) - scans all recipes. +//! - [`SqliteRecipesDb`] - SQLite with FTS5. Recommended for production. Provides +//! indexed full-text search and better performance at scale. +//! +//! ## Other Storage //! -//! For other storage, file-based implementations are used: //! - [`FileSignalLog`] - Daily JSONL files //! - [`FileKnowledgeStore`] - JSON files //! - [`FileSessionStore`] - JSON files @@ -36,8 +41,7 @@ //! │ ├── user_models/ //! │ └── patterns/ //! ├── recipes/ -//! │ ├── recipes.jsonl -//! │ └── index.json +//! │ └── recipes.jsonl //! ├── signals/ //! │ └── YYYY-MM-DD.jsonl //! └── state/ @@ -112,8 +116,45 @@ mod tests { #[test] fn sqlite_recipes_works() { let sqlite_db = SqliteRecipesDb::open_in_memory().unwrap(); - + use traits::RecipesStorage; assert_eq!(sqlite_db.count().unwrap(), 0); } + + #[test] + fn sqlite_fts_rebuild_works() { + let sqlite_db = SqliteRecipesDb::open_in_memory().unwrap(); + + use traits::RecipesStorage; + let recipe = crate::models::recipe::Recipe { + id: crate::models::recipe::RecipeId::generate(), + name: "Test".to_string(), + source_url: "https://example.com/test".parse().unwrap(), + source_domain: "example.com".to_string(), + ingredients: vec![], + instructions: vec![], + prep_time_minutes: None, + cook_time_minutes: None, + total_time_minutes: None, + servings: None, + cuisine: None, + difficulty: None, + tags: vec![], + nutrition: None, + image_url: None, + author: None, + description: None, + scraped_at: chrono::Utc::now(), + content_hash: None, + meta: std::collections::HashMap::new(), + }; + sqlite_db.insert(&recipe).unwrap(); + + // Rebuild FTS index + sqlite_db.rebuild_fts_index().unwrap(); + + // Search still works after rebuild + let results = sqlite_db.search("Test").unwrap(); + assert_eq!(results.len(), 1); + } } diff --git a/src/storage/recipes_db.rs b/src/storage/recipes_db.rs index 1471266..4e89c55 100644 --- a/src/storage/recipes_db.rs +++ b/src/storage/recipes_db.rs @@ -1,45 +1,31 @@ -//! File-based recipe storage with append-only JSONL and search index. +//! File-based recipe storage using JSONL format. //! -//! Provides CRUD operations for recipes with duplicate detection -//! and full-text search capabilities. +//! Provides simple append-only storage. For indexed/search-heavy workloads, +//! use [`SqliteRecipesDb`] instead. use std::fs::{self, File, OpenOptions}; use std::io::{BufRead, BufReader, BufWriter, Write}; use std::path::PathBuf; -use std::sync::{Arc, RwLock}; - -use serde::{Deserialize, Serialize}; use crate::models::recipe::{Recipe, RecipeId}; use super::error::Result; use super::traits::RecipesStorage; -/// In-memory index for fast lookups. -#[derive(Debug, Default)] -struct SearchIndex { - by_url: std::collections::HashMap, - by_content_hash: std::collections::HashMap, - by_name: Vec<(String, RecipeId)>, -} - -/// File-based, append-only recipe storage with search index. +/// File-based, append-only recipe storage. +/// +/// Note: Search operations scan all recipes. For better performance +/// with large datasets, use [`SqliteRecipesDb`]. #[derive(Debug)] pub struct FileRecipesDb { dir: PathBuf, - index: Arc>, } impl FileRecipesDb { /// Open the recipes database at the given directory. pub fn open(dir: PathBuf) -> Result { fs::create_dir_all(&dir)?; - let db = Self { - dir, - index: Arc::new(RwLock::new(SearchIndex::default())), - }; - db.rebuild_index()?; - Ok(db) + Ok(Self { dir }) } /// Get the path to the recipes JSONL file. @@ -47,46 +33,35 @@ impl FileRecipesDb { self.dir.join("recipes.jsonl") } - /// Get the path to the index file. - fn index_file(&self) -> PathBuf { - self.dir.join("index.json") - } - - /// Check if content hash already exists. - pub fn exists_by_content_hash(&self, hash: &str) -> Result { - let index = self.index.read().unwrap(); - Ok(index.by_content_hash.contains_key(hash)) - } + /// Scan recipes file and find by predicate. + fn find_by

(&self, predicate: P) -> Result> + where + P: Fn(&Recipe) -> bool, + { + if !self.recipes_file().exists() { + return Ok(None); + } - /// Save index to disk. - fn save_index(&self) -> Result<()> { - let index = self.index.read().unwrap(); - let persisted = PersistedIndex { - urls: index.by_url.clone(), - hashes: index.by_content_hash.clone(), - names: index.by_name.clone(), - }; - let file = File::create(self.index_file())?; - serde_json::to_writer(file, &persisted)?; - Ok(()) - } + let file = File::open(self.recipes_file())?; + let reader = BufReader::new(file); - /// Load index from disk. - #[allow(dead_code)] - fn load_index(&self) -> Result<()> { - if !self.index_file().exists() { - return Ok(()); + for line in reader.lines() { + let line = line?; + if let Ok(recipe) = serde_json::from_str::(&line) + && predicate(&recipe) + { + return Ok(Some(recipe)); + } } - let file = File::open(self.index_file())?; - let persisted: PersistedIndex = serde_json::from_reader(file)?; - - let mut index = self.index.write().unwrap(); - index.by_url = persisted.urls; - index.by_content_hash = persisted.hashes; - index.by_name = persisted.names; + Ok(None) + } - Ok(()) + /// Check if content hash already exists. + pub fn exists_by_content_hash(&self, hash: &str) -> Result { + Ok(self + .find_by(|r| r.content_hash.as_deref() == Some(hash))? + .is_some()) } } @@ -97,9 +72,10 @@ impl RecipesStorage for FileRecipesDb { && self.exists_by_content_hash(hash)? { // Return existing ID - let index = self.index.read().unwrap(); - if let Some(id) = index.by_content_hash.get(hash) { - return Ok(id.clone()); + if let Some(existing) = + self.find_by(|r| r.content_hash.as_deref() == Some(hash))? + { + return Ok(existing.id); } } @@ -121,121 +97,74 @@ impl RecipesStorage for FileRecipesDb { let json = serde_json::to_string(&recipe)?; writeln!(writer, "{}", json)?; - // Update index - { - let mut index = self.index.write().unwrap(); - index - .by_url - .insert(recipe.source_url.to_string(), id.clone()); - if let Some(hash) = &recipe.content_hash { - index.by_content_hash.insert(hash.clone(), id.clone()); - } - index - .by_name - .push((recipe.name.to_lowercase(), id.clone())); - } - - // Persist index - self.save_index()?; - Ok(id) } fn get(&self, id: &RecipeId) -> Result> { - let file = File::open(self.recipes_file())?; - let reader = BufReader::new(file); - - for line in reader.lines() { - let line = line?; - if let Ok(recipe) = serde_json::from_str::(&line) - && &recipe.id == id - { - return Ok(Some(recipe)); - } - } - - Ok(None) + self.find_by(|r| &r.id == id) } fn exists_by_url(&self, url: &str) -> Result { - let index = self.index.read().unwrap(); - Ok(index.by_url.contains_key(url)) + Ok(self.find_by(|r| r.source_url.as_str() == url)?.is_some()) } fn search(&self, query: &str) -> Result> { - let index = self.index.read().unwrap(); let query = query.to_lowercase(); let terms: Vec<&str> = query.split_whitespace().collect(); - let mut results: Vec = index - .by_name - .iter() - .filter(|(name, _)| terms.iter().all(|t| name.contains(t))) - .map(|(_, id)| id.clone()) - .collect(); + let mut results = Vec::new(); - // Deduplicate - results.sort_by(|a, b| a.0.cmp(&b.0)); - results.dedup_by(|a, b| a.0 == b.0); + if !self.recipes_file().exists() { + return Ok(results); + } - Ok(results) - } + let file = File::open(self.recipes_file())?; + let reader = BufReader::new(file); - fn rebuild_index(&self) -> Result<()> { - let mut index = SearchIndex::default(); - - if let Ok(file) = File::open(self.recipes_file()) { - let reader = BufReader::new(file); - for line in reader.lines() { - let line = line?; - if let Ok(recipe) = serde_json::from_str::(&line) { - index - .by_url - .insert(recipe.source_url.to_string(), recipe.id.clone()); - if let Some(hash) = &recipe.content_hash { - index.by_content_hash.insert(hash.clone(), recipe.id.clone()); - } - index - .by_name - .push((recipe.name.to_lowercase(), recipe.id.clone())); + for line in reader.lines() { + let line = line?; + if let Ok(recipe) = serde_json::from_str::(&line) { + let name_lower = recipe.name.to_lowercase(); + if terms.iter().all(|t| name_lower.contains(t)) { + results.push(recipe.id); } } } - *self.index.write().unwrap() = index; - self.save_index()?; - - Ok(()) + Ok(results) } fn count(&self) -> Result { - let index = self.index.read().unwrap(); - Ok(index.by_url.len() as u64) + if !self.recipes_file().exists() { + return Ok(0); + } + + let file = File::open(self.recipes_file())?; + let reader = BufReader::new(file); + Ok(reader.lines().filter(|l| l.is_ok()).count() as u64) } fn all(&self) -> Result> { let mut recipes = Vec::new(); - if let Ok(file) = File::open(self.recipes_file()) { - let reader = BufReader::new(file); - for line in reader.lines() { - let line = line?; - if let Ok(recipe) = serde_json::from_str::(&line) { - recipes.push(recipe); - } + + if !self.recipes_file().exists() { + return Ok(recipes); + } + + let file = File::open(self.recipes_file())?; + let reader = BufReader::new(file); + + for line in reader.lines() { + let line = line?; + if let Ok(recipe) = serde_json::from_str::(&line) { + recipes.push(recipe); } } + Ok(recipes) } } -/// Persisted index structure. -#[derive(Debug, Serialize, Deserialize)] -struct PersistedIndex { - urls: std::collections::HashMap, - hashes: std::collections::HashMap, - names: Vec<(String, RecipeId)>, -} - #[cfg(test)] mod tests { use super::*; @@ -322,20 +251,6 @@ mod tests { assert_eq!(results.len(), 2); } - #[test] - fn rebuild_index_restores_state() { - let dir = tempdir().unwrap(); - let db = FileRecipesDb::open(dir.path().to_path_buf()).unwrap(); - - let recipe = test_recipe("Test", "https://example.com/test"); - db.insert(&recipe).unwrap(); - - // Open a new instance - let db2 = FileRecipesDb::open(dir.path().to_path_buf()).unwrap(); - - assert!(db2.exists_by_url("https://example.com/test").unwrap()); - } - #[test] fn duplicate_by_content_hash_not_inserted() { let dir = tempdir().unwrap(); diff --git a/src/storage/signal_log.rs b/src/storage/signal_log.rs index f099c48..3a4a8d2 100644 --- a/src/storage/signal_log.rs +++ b/src/storage/signal_log.rs @@ -3,7 +3,7 @@ //! Signals are appended to date-based JSONL files for audit trails //! and later compression into knowledge. -use std::fs::{self, File}; +use std::fs::{self, File, OpenOptions}; use std::io::{BufRead, BufReader, BufWriter, Write}; use std::path::PathBuf; @@ -160,8 +160,6 @@ impl SignalStorage for FileSignalLog { } } -// Need to import OpenOptions -use std::fs::OpenOptions; #[cfg(test)] mod tests { diff --git a/src/storage/sqlite_recipes.rs b/src/storage/sqlite_recipes.rs index ac0b229..06c1870 100644 --- a/src/storage/sqlite_recipes.rs +++ b/src/storage/sqlite_recipes.rs @@ -89,6 +89,32 @@ impl SqliteRecipesDb { fn ingredients_to_text(ingredients: &[crate::models::recipe::Ingredient]) -> String { ingredients.iter().map(|i| i.raw.as_str()).collect::>().join(" ") } + + /// Rebuild the full-text search index. + /// + /// This is SQLite-specific and not part of the trait. + pub fn rebuild_fts_index(&self) -> Result<()> { + let conn = self.conn.lock().unwrap(); + conn.execute("INSERT INTO recipes_fts(recipes_fts) VALUES ('rebuild')", [])?; + Ok(()) + } + + /// Search with custom result limit. + pub fn search_with_limit(&self, query: &str, limit: usize) -> Result> { + let conn = self.conn.lock().unwrap(); + + let mut stmt = conn.prepare( + "SELECT id FROM recipes_fts WHERE recipes_fts MATCH ? ORDER BY rank LIMIT ?" + )?; + + let ids = stmt + .query_map(params![query, limit as i64], |row| row.get::<_, String>(0))? + .filter_map(|r| r.ok()) + .map(RecipeId::new) + .collect(); + + Ok(ids) + } } impl RecipesStorage for SqliteRecipesDb { @@ -210,25 +236,7 @@ impl RecipesStorage for SqliteRecipesDb { } fn search(&self, query: &str) -> Result> { - let conn = self.conn.lock().unwrap(); - - let mut stmt = conn.prepare( - "SELECT id FROM recipes_fts WHERE recipes_fts MATCH ? ORDER BY rank LIMIT 100" - )?; - - let ids = stmt - .query_map(params![query], |row| row.get::<_, String>(0))? - .filter_map(|r| r.ok()) - .map(RecipeId::new) - .collect(); - - Ok(ids) - } - - fn rebuild_index(&self) -> Result<()> { - let conn = self.conn.lock().unwrap(); - conn.execute("INSERT INTO recipes_fts(recipes_fts) VALUES ('rebuild')", [])?; - Ok(()) + self.search_with_limit(query, 100) } fn count(&self) -> Result { diff --git a/src/storage/traits.rs b/src/storage/traits.rs index 71c5704..62fa3e6 100644 --- a/src/storage/traits.rs +++ b/src/storage/traits.rs @@ -25,9 +25,6 @@ pub trait RecipesStorage: Send + Sync { /// Full-text search across recipes. fn search(&self, query: &str) -> Result>; - /// Rebuild the search index. - fn rebuild_index(&self) -> Result<()>; - /// Count total recipes. fn count(&self) -> Result; From 87ec297bde9bb8092b9c0af6085e36b371b3a98e Mon Sep 17 00:00:00 2001 From: Ezra Lim Date: Thu, 2 Jul 2026 09:19:51 +0000 Subject: [PATCH 3/4] fix(security): address PR review issues Security fixes: - Add path traversal prevention in knowledge_store (domain, user_id) - Add path traversal prevention in session_store (session_id) - Validate inputs reject '/', '\', '..', and null bytes Bug fixes: - Fix race condition in FileRecipesDb with mutex lock - Add explicit flush for write durability in all file-based stores - Use transactions in SqliteRecipesDb for FTS consistency - Handle URL duplicates in SqliteRecipesDb (return existing ID) Code quality: - Make search consistent: now searches name AND ingredients - Add #[non_exhaustive] to StorageError enum - Improve SQLite row parsing with explicit types Tests: 75 tests passing (17 new tests for security and concurrency) --- src/storage/error.rs | 1 + src/storage/knowledge_store.rs | 104 +++++++++-- src/storage/recipes_db.rs | 193 ++++++++++++++++++-- src/storage/session_store.rs | 91 ++++++++-- src/storage/signal_log.rs | 1 + src/storage/sqlite_recipes.rs | 320 +++++++++++++++++++++++---------- 6 files changed, 578 insertions(+), 132 deletions(-) diff --git a/src/storage/error.rs b/src/storage/error.rs index a0de371..e5c3add 100644 --- a/src/storage/error.rs +++ b/src/storage/error.rs @@ -5,6 +5,7 @@ use thiserror::Error; /// Storage-related errors. #[derive(Debug, Error)] +#[non_exhaustive] pub enum StorageError { /// IO error during file operations. #[error("IO error: {0}")] diff --git a/src/storage/knowledge_store.rs b/src/storage/knowledge_store.rs index 6378a74..57c3cfc 100644 --- a/src/storage/knowledge_store.rs +++ b/src/storage/knowledge_store.rs @@ -10,9 +10,34 @@ use std::path::PathBuf; use crate::models::agent_state::KnowledgeContext; use crate::models::knowledge::{Patterns, SiteConfig, UserModel}; -use super::error::Result; +use super::error::{Result, StorageError}; use super::traits::KnowledgeStorage; +/// Validates that a string is safe to use as a file path component. +/// Prevents path traversal attacks by rejecting strings containing: +/// - Path separators ('/', '\\') +/// - Parent directory references ('..') +/// - Null bytes +fn validate_path_component(s: &str, context: &str) -> Result<()> { + if s.is_empty() { + return Err(StorageError::InvalidPath(format!( + "{} cannot be empty", + context + ))); + } + if s.contains('/') + || s.contains('\\') + || s.contains("..") + || s.contains('\0') + { + return Err(StorageError::InvalidPath(format!( + "Invalid characters in {}: {}", + context, s + ))); + } + Ok(()) +} + /// File-based persistence for site configs, user models, and patterns. #[derive(Debug)] pub struct FileKnowledgeStore { @@ -30,13 +55,19 @@ impl FileKnowledgeStore { } /// Get the path for a site config file. - fn site_config_path(&self, domain: &str) -> PathBuf { - self.dir.join("site_configs").join(format!("{}.json", domain)) + /// + /// Returns an error if the domain contains path traversal characters. + fn site_config_path(&self, domain: &str) -> Result { + validate_path_component(domain, "domain")?; + Ok(self.dir.join("site_configs").join(format!("{}.json", domain))) } /// Get the path for a user model file. - fn user_model_path(&self, user_id: &str) -> PathBuf { - self.dir.join("user_models").join(format!("{}.json", user_id)) + /// + /// Returns an error if the user ID contains path traversal characters. + fn user_model_path(&self, user_id: &str) -> Result { + validate_path_component(user_id, "user_id")?; + Ok(self.dir.join("user_models").join(format!("{}.json", user_id))) } /// Get the path for the patterns file. @@ -74,10 +105,10 @@ impl FileKnowledgeStore { impl KnowledgeStorage for FileKnowledgeStore { fn get_site_config(&self, domain: &str) -> Result> { - let path = self.site_config_path(domain); + let path = self.site_config_path(domain)?; if !path.exists() { // Try default config - let default_path = self.site_config_path("_default"); + let default_path = self.site_config_path("_default")?; if default_path.exists() { return self.load_json(&default_path); } @@ -87,7 +118,7 @@ impl KnowledgeStorage for FileKnowledgeStore { } fn save_site_config(&self, config: &SiteConfig) -> Result<()> { - let path = self.site_config_path(&config.domain); + let path = self.site_config_path(&config.domain)?; self.save_json(&path, config) } @@ -114,7 +145,7 @@ impl KnowledgeStorage for FileKnowledgeStore { } fn delete_site_config(&self, domain: &str) -> Result<()> { - let path = self.site_config_path(domain); + let path = self.site_config_path(domain)?; if path.exists() { fs::remove_file(path)?; } @@ -122,7 +153,7 @@ impl KnowledgeStorage for FileKnowledgeStore { } fn get_user_model(&self, user_id: &str) -> Result> { - let path = self.user_model_path(user_id); + let path = self.user_model_path(user_id)?; if !path.exists() { return Ok(None); } @@ -130,7 +161,7 @@ impl KnowledgeStorage for FileKnowledgeStore { } fn save_user_model(&self, model: &UserModel) -> Result<()> { - let path = self.user_model_path(&model.user_id); + let path = self.user_model_path(&model.user_id)?; self.save_json(&path, model) } @@ -302,4 +333,55 @@ mod tests { store.delete_site_config("example.com").unwrap(); assert!(store.get_site_config("example.com").unwrap().is_none()); } + + // Path traversal security tests + #[test] + fn rejects_path_traversal_in_domain() { + let dir = tempdir().unwrap(); + let store = FileKnowledgeStore::open(dir.path().to_path_buf()).unwrap(); + + let result = store.get_site_config("../etc/passwd"); + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), StorageError::InvalidPath(_))); + } + + #[test] + fn rejects_path_traversal_in_user_id() { + let dir = tempdir().unwrap(); + let store = FileKnowledgeStore::open(dir.path().to_path_buf()).unwrap(); + + let result = store.get_user_model("../../etc/shadow"); + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), StorageError::InvalidPath(_))); + } + + #[test] + fn rejects_empty_domain() { + let dir = tempdir().unwrap(); + let store = FileKnowledgeStore::open(dir.path().to_path_buf()).unwrap(); + + let result = store.get_site_config(""); + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), StorageError::InvalidPath(_))); + } + + #[test] + fn rejects_slash_in_domain() { + let dir = tempdir().unwrap(); + let store = FileKnowledgeStore::open(dir.path().to_path_buf()).unwrap(); + + let result = store.get_site_config("example.com/subdir"); + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), StorageError::InvalidPath(_))); + } + + #[test] + fn rejects_null_byte_in_domain() { + let dir = tempdir().unwrap(); + let store = FileKnowledgeStore::open(dir.path().to_path_buf()).unwrap(); + + let result = store.get_site_config("example\0.com"); + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), StorageError::InvalidPath(_))); + } } diff --git a/src/storage/recipes_db.rs b/src/storage/recipes_db.rs index 4e89c55..cdb7d7e 100644 --- a/src/storage/recipes_db.rs +++ b/src/storage/recipes_db.rs @@ -6,6 +6,7 @@ use std::fs::{self, File, OpenOptions}; use std::io::{BufRead, BufReader, BufWriter, Write}; use std::path::PathBuf; +use std::sync::Mutex; use crate::models::recipe::{Recipe, RecipeId}; @@ -14,18 +15,23 @@ use super::traits::RecipesStorage; /// File-based, append-only recipe storage. /// -/// Note: Search operations scan all recipes. For better performance -/// with large datasets, use [`SqliteRecipesDb`]. +/// Uses a mutex to ensure thread-safe operations. Search operations scan all +/// recipes, so for large datasets, use [`SqliteRecipesDb`] instead. #[derive(Debug)] pub struct FileRecipesDb { dir: PathBuf, + /// Mutex to prevent race conditions between check-and-insert operations + lock: Mutex<()>, } impl FileRecipesDb { /// Open the recipes database at the given directory. pub fn open(dir: PathBuf) -> Result { fs::create_dir_all(&dir)?; - Ok(Self { dir }) + Ok(Self { + dir, + lock: Mutex::new(()), + }) } /// Get the path to the recipes JSONL file. @@ -63,20 +69,30 @@ impl FileRecipesDb { .find_by(|r| r.content_hash.as_deref() == Some(hash))? .is_some()) } + + /// Find recipe by URL. + fn find_by_url(&self, url: &str) -> Result> { + self.find_by(|r| r.source_url.as_str() == url) + } } impl RecipesStorage for FileRecipesDb { fn insert(&self, recipe: &Recipe) -> Result { + // Lock to prevent race condition between check and insert + let _guard = self.lock.lock().unwrap(); + // Check for duplicates by content hash if let Some(hash) = &recipe.content_hash - && self.exists_by_content_hash(hash)? - { - // Return existing ID - if let Some(existing) = + && let Some(existing) = self.find_by(|r| r.content_hash.as_deref() == Some(hash))? - { - return Ok(existing.id); - } + { + return Ok(existing.id); + } + + // Also check for duplicates by URL + let url_str = recipe.source_url.to_string(); + if let Some(existing) = self.find_by_url(&url_str)? { + return Ok(existing.id); } let id = RecipeId::generate(); @@ -88,7 +104,7 @@ impl RecipesStorage for FileRecipesDb { recipe.content_hash = Some(recipe.compute_hash()); } - // Append to JSONL file + // Append to JSONL file with explicit flush for durability let file = OpenOptions::new() .create(true) .append(true) @@ -96,6 +112,7 @@ impl RecipesStorage for FileRecipesDb { let mut writer = BufWriter::new(file); let json = serde_json::to_string(&recipe)?; writeln!(writer, "{}", json)?; + writer.flush()?; Ok(id) } @@ -105,7 +122,7 @@ impl RecipesStorage for FileRecipesDb { } fn exists_by_url(&self, url: &str) -> Result { - Ok(self.find_by(|r| r.source_url.as_str() == url)?.is_some()) + Ok(self.find_by_url(url)?.is_some()) } fn search(&self, query: &str) -> Result> { @@ -124,8 +141,19 @@ impl RecipesStorage for FileRecipesDb { for line in reader.lines() { let line = line?; if let Ok(recipe) = serde_json::from_str::(&line) { + // Search in both name and ingredients (consistent with SQLite) let name_lower = recipe.name.to_lowercase(); - if terms.iter().all(|t| name_lower.contains(t)) { + let ingredients_text: String = recipe + .ingredients + .iter() + .map(|i| i.raw.to_lowercase()) + .collect::>() + .join(" "); + + // Check if all terms match either in name or ingredients + if terms.iter().all(|t| { + name_lower.contains(t) || ingredients_text.contains(t) + }) { results.push(recipe.id); } } @@ -169,6 +197,7 @@ impl RecipesStorage for FileRecipesDb { mod tests { use super::*; use chrono::Utc; + use std::collections::HashMap; use tempfile::tempdir; fn test_recipe(name: &str, url: &str) -> Recipe { @@ -192,7 +221,35 @@ mod tests { description: None, scraped_at: Utc::now(), content_hash: None, - meta: std::collections::HashMap::new(), + meta: HashMap::new(), + } + } + + fn test_recipe_with_ingredients(name: &str, url: &str, ingredients: Vec<&str>) -> Recipe { + Recipe { + id: RecipeId::generate(), + name: name.to_string(), + source_url: url.parse().unwrap(), + source_domain: "example.com".to_string(), + ingredients: ingredients + .into_iter() + .map(crate::models::recipe::Ingredient::from_raw) + .collect(), + instructions: vec![], + prep_time_minutes: None, + cook_time_minutes: None, + total_time_minutes: None, + servings: None, + cuisine: None, + difficulty: None, + tags: vec![], + nutrition: None, + image_url: None, + author: None, + description: None, + scraped_at: Utc::now(), + content_hash: None, + meta: HashMap::new(), } } @@ -251,6 +308,37 @@ mod tests { assert_eq!(results.len(), 2); } + #[test] + fn search_finds_by_ingredients() { + let dir = tempdir().unwrap(); + let db = FileRecipesDb::open(dir.path().to_path_buf()).unwrap(); + + let r1 = test_recipe_with_ingredients( + "Simple Cake", + "https://example.com/cake", + vec!["flour", "sugar", "eggs"], + ); + let r2 = test_recipe_with_ingredients( + "Pasta", + "https://example.com/pasta", + vec!["pasta", "tomatoes", "basil"], + ); + + db.insert(&r1).unwrap(); + db.insert(&r2).unwrap(); + + // Search for ingredient + let results = db.search("flour").unwrap(); + assert_eq!(results.len(), 1); + + let results = db.search("pasta").unwrap(); + assert_eq!(results.len(), 1); + + // Both have no chocolate + let results = db.search("chocolate").unwrap(); + assert_eq!(results.len(), 0); + } + #[test] fn duplicate_by_content_hash_not_inserted() { let dir = tempdir().unwrap(); @@ -269,4 +357,81 @@ mod tests { assert_eq!(id1, id2); assert_eq!(db.count().unwrap(), 1); } + + #[test] + fn duplicate_by_url_not_inserted() { + let dir = tempdir().unwrap(); + let db = FileRecipesDb::open(dir.path().to_path_buf()).unwrap(); + + let r1 = test_recipe("Recipe 1", "https://example.com/test"); + let r2 = test_recipe("Recipe 2", "https://example.com/test"); // Same URL, different name + + let id1 = db.insert(&r1).unwrap(); + let id2 = db.insert(&r2).unwrap(); + + // Should return the same ID for duplicate URL + assert_eq!(id1, id2); + assert_eq!(db.count().unwrap(), 1); + } + + #[test] + fn concurrent_inserts_safe() { + use std::sync::Arc; + use std::thread; + + let dir = tempdir().unwrap(); + let db = Arc::new(FileRecipesDb::open(dir.path().to_path_buf()).unwrap()); + let mut handles = vec![]; + + for i in 0..10 { + let db_clone = Arc::clone(&db); + let handle = thread::spawn(move || { + let recipe = test_recipe( + &format!("Recipe {}", i), + &format!("https://example.com/r{}", i), + ); + db_clone.insert(&recipe).unwrap() + }); + handles.push(handle); + } + + let ids: Vec = handles.into_iter().map(|h| h.join().unwrap()).collect(); + + // All IDs should be unique + let unique_ids: std::collections::HashSet<_> = ids.into_iter().collect(); + assert_eq!(unique_ids.len(), 10); + + // All recipes should be stored + assert_eq!(db.count().unwrap(), 10); + } + + #[test] + fn concurrent_reads_safe() { + use std::sync::Arc; + use std::thread; + + let dir = tempdir().unwrap(); + let db = Arc::new(FileRecipesDb::open(dir.path().to_path_buf()).unwrap()); + + // Insert some recipes first + for i in 0..5 { + db.insert(&test_recipe( + &format!("Recipe {}", i), + &format!("https://example.com/r{}", i), + )) + .unwrap(); + } + + let mut handles = vec![]; + for _ in 0..10 { + let db_clone = Arc::clone(&db); + let handle = thread::spawn(move || db_clone.count().unwrap()); + handles.push(handle); + } + + let counts: Vec = handles.into_iter().map(|h| h.join().unwrap()).collect(); + + // All reads should return the same count + assert!(counts.iter().all(|c| *c == 5)); + } } diff --git a/src/storage/session_store.rs b/src/storage/session_store.rs index 599b03f..718a2b0 100644 --- a/src/storage/session_store.rs +++ b/src/storage/session_store.rs @@ -4,14 +4,35 @@ //! sessions can be resumed after interruption. use std::fs::{self, File}; -use std::io::{BufReader, BufWriter}; +use std::io::{BufReader, BufWriter, Write}; use std::path::PathBuf; use crate::models::agent_state::{SessionId, SessionState}; -use super::error::Result; +use super::error::{Result, StorageError}; use super::traits::SessionStorage; +/// Validates that a session ID is safe to use in a file path. +/// Prevents path traversal attacks. +fn validate_session_id(id: &SessionId) -> Result<()> { + if id.0.is_empty() { + return Err(StorageError::InvalidPath( + "session ID cannot be empty".to_string(), + )); + } + if id.0.contains('/') + || id.0.contains('\\') + || id.0.contains("..") + || id.0.contains('\0') + { + return Err(StorageError::InvalidPath(format!( + "Invalid characters in session ID: {}", + id.0 + ))); + } + Ok(()) +} + /// File-based persistable session state for resumability. #[derive(Debug)] pub struct FileSessionStore { @@ -26,22 +47,26 @@ impl FileSessionStore { } /// Get the path for a session file. - fn session_path(&self, id: &SessionId) -> PathBuf { - self.dir.join(format!("{}.json", id)) + /// + /// Returns an error if the session ID contains path traversal characters. + fn session_path(&self, id: &SessionId) -> Result { + validate_session_id(id)?; + Ok(self.dir.join(format!("{}.json", id))) } } impl SessionStorage for FileSessionStore { fn save(&self, session: &SessionState) -> Result<()> { - let path = self.session_path(&session.id); + let path = self.session_path(&session.id)?; let file = File::create(&path)?; - let writer = BufWriter::new(file); - serde_json::to_writer_pretty(writer, session)?; + let mut writer = BufWriter::new(file); + serde_json::to_writer_pretty(&mut writer, session)?; + writer.flush()?; Ok(()) } fn load(&self, id: &SessionId) -> Result> { - let path = self.session_path(id); + let path = self.session_path(id)?; if !path.exists() { return Ok(None); } @@ -68,8 +93,8 @@ impl SessionStorage for FileSessionStore { // Sort by modification time, newest first ids.sort_by(|a, b| { - let path_a = self.session_path(a); - let path_b = self.session_path(b); + let path_a = self.dir.join(format!("{}.json", a)); + let path_b = self.dir.join(format!("{}.json", b)); let time_a = path_a.metadata().and_then(|m| m.modified()).ok(); let time_b = path_b.metadata().and_then(|m| m.modified()).ok(); time_b.cmp(&time_a) @@ -79,7 +104,7 @@ impl SessionStorage for FileSessionStore { } fn delete(&self, id: &SessionId) -> Result<()> { - let path = self.session_path(id); + let path = self.session_path(id)?; if path.exists() { fs::remove_file(path)?; } @@ -87,7 +112,8 @@ impl SessionStorage for FileSessionStore { } fn exists(&self, id: &SessionId) -> Result { - Ok(self.session_path(id).exists()) + let path = self.session_path(id)?; + Ok(path.exists()) } fn latest(&self) -> Result> { @@ -207,4 +233,45 @@ mod tests { assert_eq!(count, 3); assert_eq!(store.count().unwrap(), 0); } + + // Path traversal security tests + #[test] + fn rejects_path_traversal_in_session_id() { + let dir = tempdir().unwrap(); + let store = FileSessionStore::open(dir.path().to_path_buf()).unwrap(); + + let result = store.load(&SessionId::new("../etc/passwd")); + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), StorageError::InvalidPath(_))); + } + + #[test] + fn rejects_slash_in_session_id() { + let dir = tempdir().unwrap(); + let store = FileSessionStore::open(dir.path().to_path_buf()).unwrap(); + + let result = store.exists(&SessionId::new("foo/bar")); + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), StorageError::InvalidPath(_))); + } + + #[test] + fn rejects_empty_session_id() { + let dir = tempdir().unwrap(); + let store = FileSessionStore::open(dir.path().to_path_buf()).unwrap(); + + let result = store.load(&SessionId::new("")); + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), StorageError::InvalidPath(_))); + } + + #[test] + fn rejects_null_byte_in_session_id() { + let dir = tempdir().unwrap(); + let store = FileSessionStore::open(dir.path().to_path_buf()).unwrap(); + + let result = store.delete(&SessionId::new("sess\0ion")); + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), StorageError::InvalidPath(_))); + } } diff --git a/src/storage/signal_log.rs b/src/storage/signal_log.rs index 3a4a8d2..0cdc792 100644 --- a/src/storage/signal_log.rs +++ b/src/storage/signal_log.rs @@ -55,6 +55,7 @@ impl SignalStorage for FileSignalLog { let mut writer = BufWriter::new(file); let json = serde_json::to_string(signal)?; writeln!(writer, "{}", json)?; + writer.flush()?; // Explicit flush for durability Ok(()) } diff --git a/src/storage/sqlite_recipes.rs b/src/storage/sqlite_recipes.rs index 06c1870..d26db43 100644 --- a/src/storage/sqlite_recipes.rs +++ b/src/storage/sqlite_recipes.rs @@ -3,7 +3,7 @@ //! Provides recipe persistence using SQLite for efficient querying //! and full-text search capabilities. -use rusqlite::{Connection, params, OptionalExtension}; +use rusqlite::{Connection, params, OptionalExtension, Transaction}; use std::path::PathBuf; use std::sync::{Arc, Mutex}; @@ -90,6 +90,78 @@ impl SqliteRecipesDb { ingredients.iter().map(|i| i.raw.as_str()).collect::>().join(" ") } + /// Parse a Recipe from a database row. + /// Uses explicit column names to avoid fragility with schema changes. + fn parse_recipe_from_row(row: &rusqlite::Row) -> rusqlite::Result { + Ok(Recipe { + id: RecipeId::new(row.get::<_, String>("id")?), + name: row.get::<_, String>("name")?, + source_url: row.get::<_, String>("source_url")?.parse().map_err(|_| rusqlite::Error::InvalidQuery)?, + source_domain: row.get::<_, String>("source_domain")?, + ingredients: serde_json::from_str(&row.get::<_, String>("ingredients_json")?).map_err(|_| rusqlite::Error::InvalidQuery)?, + instructions: serde_json::from_str(&row.get::<_, String>("instructions_json")?).map_err(|_| rusqlite::Error::InvalidQuery)?, + prep_time_minutes: row.get::<_, Option>("prep_time_minutes")?.map(|v| v as u32), + cook_time_minutes: row.get::<_, Option>("cook_time_minutes")?.map(|v| v as u32), + total_time_minutes: row.get::<_, Option>("total_time_minutes")?.map(|v| v as u32), + servings: row.get::<_, Option>("servings_json")?.map(|s| serde_json::from_str(&s)).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, + cuisine: row.get::<_, Option>("cuisine")?, + difficulty: row.get::<_, Option>("difficulty")?.map(|s| serde_json::from_str(&s)).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, + tags: serde_json::from_str(&row.get::<_, String>("tags_json")?).map_err(|_| rusqlite::Error::InvalidQuery)?, + nutrition: row.get::<_, Option>("nutrition_json")?.map(|s| serde_json::from_str(&s)).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, + image_url: row.get::<_, Option>("image_url")?.map(|s| s.parse()).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, + author: row.get::<_, Option>("author")?, + description: row.get::<_, Option>("description")?, + scraped_at: chrono::DateTime::parse_from_rfc3339(&row.get::<_, String>("scraped_at")?).map(|dt| dt.with_timezone(&chrono::Utc)).map_err(|_| rusqlite::Error::InvalidQuery)?, + content_hash: row.get::<_, Option>("content_hash")?, + meta: serde_json::from_str(&row.get::<_, String>("meta_json")?).map_err(|_| rusqlite::Error::InvalidQuery)?, + }) + } + + /// Insert a recipe within a transaction. + fn insert_in_transaction(tx: &Transaction, recipe: &Recipe) -> Result<()> { + tx.execute( + r#" + INSERT INTO recipes ( + id, name, source_url, source_domain, ingredients_json, instructions_json, + prep_time_minutes, cook_time_minutes, total_time_minutes, servings_json, + cuisine, difficulty, tags_json, nutrition_json, image_url, author, + description, scraped_at, content_hash, meta_json + ) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18, ?19, ?20) + "#, + params![ + recipe.id.0, + recipe.name, + recipe.source_url.to_string(), + recipe.source_domain, + serde_json::to_string(&recipe.ingredients)?, + serde_json::to_string(&recipe.instructions)?, + recipe.prep_time_minutes, + recipe.cook_time_minutes, + recipe.total_time_minutes, + recipe.servings.as_ref().map(serde_json::to_string).transpose()?, + recipe.cuisine, + recipe.difficulty.map(|d| serde_json::to_string(&d)).transpose()?, + serde_json::to_string(&recipe.tags)?, + recipe.nutrition.as_ref().map(serde_json::to_string).transpose()?, + recipe.image_url.as_ref().map(|u| u.to_string()), + recipe.author, + recipe.description, + recipe.scraped_at.to_rfc3339(), + recipe.content_hash, + serde_json::to_string(&recipe.meta)?, + ], + )?; + + // Insert into FTS + let ingredients_text = Self::ingredients_to_text(&recipe.ingredients); + tx.execute( + "INSERT INTO recipes_fts(id, name, ingredients_text) VALUES (?1, ?2, ?3)", + params![recipe.id.0, recipe.name, ingredients_text], + )?; + + Ok(()) + } + /// Rebuild the full-text search index. /// /// This is SQLite-specific and not part of the trait. @@ -119,9 +191,9 @@ impl SqliteRecipesDb { impl RecipesStorage for SqliteRecipesDb { fn insert(&self, recipe: &Recipe) -> Result { - let conn = self.conn.lock().unwrap(); - - // Check for duplicates by content hash + let mut conn = self.conn.lock().unwrap(); + + // First check for duplicates by content hash or URL if let Some(hash) = &recipe.content_hash { let existing: Option = conn .query_row( @@ -135,6 +207,20 @@ impl RecipesStorage for SqliteRecipesDb { return Ok(RecipeId::new(id)); } } + + // Also check for duplicates by URL + let url_str = recipe.source_url.to_string(); + let existing_url: Option = conn + .query_row( + "SELECT id FROM recipes WHERE source_url = ?", + params![url_str], + |row| row.get(0), + ) + .optional()?; + + if let Some(id) = existing_url { + return Ok(RecipeId::new(id)); + } let id = RecipeId::generate(); let mut recipe = recipe.clone(); @@ -145,45 +231,10 @@ impl RecipesStorage for SqliteRecipesDb { recipe.content_hash = Some(recipe.compute_hash()); } - conn.execute( - r#" - INSERT INTO recipes ( - id, name, source_url, source_domain, ingredients_json, instructions_json, - prep_time_minutes, cook_time_minutes, total_time_minutes, servings_json, - cuisine, difficulty, tags_json, nutrition_json, image_url, author, - description, scraped_at, content_hash, meta_json - ) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18, ?19, ?20) - "#, - params![ - recipe.id.0, - recipe.name, - recipe.source_url.to_string(), - recipe.source_domain, - serde_json::to_string(&recipe.ingredients)?, - serde_json::to_string(&recipe.instructions)?, - recipe.prep_time_minutes, - recipe.cook_time_minutes, - recipe.total_time_minutes, - recipe.servings.as_ref().map(serde_json::to_string).transpose()?, - recipe.cuisine, - recipe.difficulty.map(|d| serde_json::to_string(&d)).transpose()?, - serde_json::to_string(&recipe.tags)?, - recipe.nutrition.as_ref().map(serde_json::to_string).transpose()?, - recipe.image_url.as_ref().map(|u| u.to_string()), - recipe.author, - recipe.description, - recipe.scraped_at.to_rfc3339(), - recipe.content_hash, - serde_json::to_string(&recipe.meta)?, - ], - )?; - - // Insert into FTS - let ingredients_text = Self::ingredients_to_text(&recipe.ingredients); - conn.execute( - "INSERT INTO recipes_fts(id, name, ingredients_text) VALUES (?1, ?2, ?3)", - params![recipe.id.0, recipe.name, ingredients_text], - )?; + // Use a transaction to ensure FTS and main table stay consistent + let tx = conn.transaction()?; + Self::insert_in_transaction(&tx, &recipe)?; + tx.commit()?; Ok(id) } @@ -195,30 +246,7 @@ impl RecipesStorage for SqliteRecipesDb { .query_row( "SELECT * FROM recipes WHERE id = ?", params![id.0], - |row| { - Ok(Recipe { - id: RecipeId::new(row.get::<_, String>(0)?), - name: row.get(1)?, - source_url: row.get::<_, String>(2)?.parse().map_err(|_| rusqlite::Error::InvalidQuery)?, - source_domain: row.get(3)?, - ingredients: serde_json::from_str(&row.get::<_, String>(4)?).map_err(|_| rusqlite::Error::InvalidQuery)?, - instructions: serde_json::from_str(&row.get::<_, String>(5)?).map_err(|_| rusqlite::Error::InvalidQuery)?, - prep_time_minutes: row.get(6)?, - cook_time_minutes: row.get(7)?, - total_time_minutes: row.get(8)?, - servings: row.get::<_, Option>(9)?.map(|s| serde_json::from_str(&s)).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, - cuisine: row.get(10)?, - difficulty: row.get::<_, Option>(11)?.map(|s| serde_json::from_str(&s)).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, - tags: serde_json::from_str(&row.get::<_, String>(12)?).map_err(|_| rusqlite::Error::InvalidQuery)?, - nutrition: row.get::<_, Option>(13)?.map(|s| serde_json::from_str(&s)).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, - image_url: row.get::<_, Option>(14)?.map(|s| s.parse()).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, - author: row.get(15)?, - description: row.get(16)?, - scraped_at: chrono::DateTime::parse_from_rfc3339(&row.get::<_, String>(17)?).map(|dt| dt.with_timezone(&chrono::Utc)).map_err(|_| rusqlite::Error::InvalidQuery)?, - content_hash: row.get(18)?, - meta: serde_json::from_str(&row.get::<_, String>(19)?).map_err(|_| rusqlite::Error::InvalidQuery)?, - }) - }, + Self::parse_recipe_from_row, ) .optional()?; @@ -249,37 +277,11 @@ impl RecipesStorage for SqliteRecipesDb { let conn = self.conn.lock().unwrap(); let mut stmt = conn.prepare( - "SELECT id, name, source_url, source_domain, ingredients_json, instructions_json, - prep_time_minutes, cook_time_minutes, total_time_minutes, servings_json, - cuisine, difficulty, tags_json, nutrition_json, image_url, author, - description, scraped_at, content_hash, meta_json FROM recipes" + "SELECT * FROM recipes" )?; let recipes = stmt - .query_map([], |row| { - Ok(Recipe { - id: RecipeId::new(row.get::<_, String>(0)?), - name: row.get(1)?, - source_url: row.get::<_, String>(2)?.parse().map_err(|_| rusqlite::Error::InvalidQuery)?, - source_domain: row.get(3)?, - ingredients: serde_json::from_str(&row.get::<_, String>(4)?).map_err(|_| rusqlite::Error::InvalidQuery)?, - instructions: serde_json::from_str(&row.get::<_, String>(5)?).map_err(|_| rusqlite::Error::InvalidQuery)?, - prep_time_minutes: row.get(6)?, - cook_time_minutes: row.get(7)?, - total_time_minutes: row.get(8)?, - servings: row.get::<_, Option>(9)?.map(|s| serde_json::from_str(&s)).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, - cuisine: row.get(10)?, - difficulty: row.get::<_, Option>(11)?.map(|s| serde_json::from_str(&s)).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, - tags: serde_json::from_str(&row.get::<_, String>(12)?).map_err(|_| rusqlite::Error::InvalidQuery)?, - nutrition: row.get::<_, Option>(13)?.map(|s| serde_json::from_str(&s)).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, - image_url: row.get::<_, Option>(14)?.map(|s| s.parse()).transpose().map_err(|_| rusqlite::Error::InvalidQuery)?, - author: row.get(15)?, - description: row.get(16)?, - scraped_at: chrono::DateTime::parse_from_rfc3339(&row.get::<_, String>(17)?).map(|dt| dt.with_timezone(&chrono::Utc)).map_err(|_| rusqlite::Error::InvalidQuery)?, - content_hash: row.get(18)?, - meta: serde_json::from_str(&row.get::<_, String>(19)?).map_err(|_| rusqlite::Error::InvalidQuery)?, - }) - })? + .query_map([], Self::parse_recipe_from_row)? .filter_map(|r| r.ok()) .collect(); @@ -318,6 +320,34 @@ mod tests { } } + fn test_recipe_with_ingredients(name: &str, url: &str, ingredients: Vec<&str>) -> Recipe { + Recipe { + id: RecipeId::generate(), + name: name.to_string(), + source_url: url.parse().unwrap(), + source_domain: "example.com".to_string(), + ingredients: ingredients + .into_iter() + .map(crate::models::recipe::Ingredient::from_raw) + .collect(), + instructions: vec![], + prep_time_minutes: None, + cook_time_minutes: None, + total_time_minutes: None, + servings: None, + cuisine: None, + difficulty: None, + tags: vec![], + nutrition: None, + image_url: None, + author: None, + description: None, + scraped_at: Utc::now(), + content_hash: None, + meta: HashMap::new(), + } + } + #[test] fn insert_and_get() { let db = SqliteRecipesDb::open_in_memory().unwrap(); @@ -395,4 +425,104 @@ mod tests { assert_eq!(db.count().unwrap(), 2); } + + #[test] + fn duplicate_by_url_not_inserted() { + let db = SqliteRecipesDb::open_in_memory().unwrap(); + + let r1 = test_recipe("Recipe 1", "https://example.com/test"); + let r2 = test_recipe("Recipe 2", "https://example.com/test"); // Same URL, different name + + let id1 = db.insert(&r1).unwrap(); + let id2 = db.insert(&r2).unwrap(); + + // Should return the same ID for duplicate URL + assert_eq!(id1, id2); + assert_eq!(db.count().unwrap(), 1); + } + + #[test] + fn search_finds_by_ingredients() { + let db = SqliteRecipesDb::open_in_memory().unwrap(); + + let r1 = test_recipe_with_ingredients( + "Chocolate Cake", + "https://example.com/cake", + vec!["chocolate", "flour", "sugar"], + ); + let r2 = test_recipe_with_ingredients( + "Apple Pie", + "https://example.com/pie", + vec!["apples", "flour", "butter"], + ); + + db.insert(&r1).unwrap(); + db.insert(&r2).unwrap(); + + // Search for ingredient + let results = db.search("chocolate").unwrap(); + assert_eq!(results.len(), 1); + + let results = db.search("flour").unwrap(); + assert_eq!(results.len(), 2); // Both recipes have flour + } + + #[test] + fn concurrent_inserts_safe() { + use std::thread; + + let db = Arc::new(SqliteRecipesDb::open_in_memory().unwrap()); + let mut handles = vec![]; + + for i in 0..10 { + let db_clone = Arc::clone(&db); + let handle = thread::spawn(move || { + let recipe = test_recipe( + &format!("Recipe {}", i), + &format!("https://example.com/r{}", i), + ); + db_clone.insert(&recipe).unwrap() + }); + handles.push(handle); + } + + let ids: Vec = handles.into_iter().map(|h| h.join().unwrap()).collect(); + + // All IDs should be unique + let unique_ids: std::collections::HashSet<_> = ids.into_iter().collect(); + assert_eq!(unique_ids.len(), 10); + + // All recipes should be stored + assert_eq!(db.count().unwrap(), 10); + } + + #[test] + fn concurrent_reads_safe() { + use std::thread; + + let db = Arc::new(SqliteRecipesDb::open_in_memory().unwrap()); + + // Insert some recipes first + for i in 0..5 { + db.insert(&test_recipe( + &format!("Recipe {}", i), + &format!("https://example.com/r{}", i), + )) + .unwrap(); + } + + let mut handles = vec![]; + for _ in 0..10 { + let db_clone = Arc::clone(&db); + let handle = thread::spawn(move || { + db_clone.count().unwrap() + }); + handles.push(handle); + } + + let counts: Vec = handles.into_iter().map(|h| h.join().unwrap()).collect(); + + // All reads should return the same count + assert!(counts.iter().all(|c| *c == 5)); + } } From e4a5530f3c276255b246683af5e46a1ad2cadde0 Mon Sep 17 00:00:00 2001 From: Ezra Lim Date: Thu, 2 Jul 2026 13:40:43 +0000 Subject: [PATCH 4/4] fix: address PR review comments - Move validate_path_component to shared file_utils module - Use DEFAULT_TOKEN_BUDGET const instead of magic number - Remove backend usage recommendations from docs - Handle mutex lock poisoning gracefully without unwrap - Add path validation documentation to signal_log - Add file_utils tests for path component validation --- src/storage/file_utils.rs | 78 ++++++++++++++++++++++++++++++++++ src/storage/knowledge_store.rs | 32 +++----------- src/storage/mod.rs | 8 ++-- src/storage/recipes_db.rs | 10 +++-- src/storage/session_store.rs | 27 ++---------- src/storage/signal_log.rs | 12 +++++- 6 files changed, 109 insertions(+), 58 deletions(-) create mode 100644 src/storage/file_utils.rs diff --git a/src/storage/file_utils.rs b/src/storage/file_utils.rs new file mode 100644 index 0000000..a5b7653 --- /dev/null +++ b/src/storage/file_utils.rs @@ -0,0 +1,78 @@ +//! File system utilities for storage layer. +//! +//! Provides common file operations and safety checks. + +use super::error::{Result, StorageError}; + +/// Validates that a string is safe to use as a file path component. +/// +/// Prevents path traversal attacks by rejecting strings containing: +/// - Path separators (`/`, `\`) +/// - Parent directory references (`..`) +/// - Null bytes +pub fn validate_path_component(s: &str, context: &str) -> Result<()> { + if s.is_empty() { + return Err(StorageError::InvalidPath(format!( + "{} cannot be empty", + context + ))); + } + if s.contains('/') + || s.contains('\\') + || s.contains("..") + || s.contains('\0') + { + return Err(StorageError::InvalidPath(format!( + "Invalid characters in {}: {}", + context, s + ))); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn accepts_valid_component() { + assert!(validate_path_component("example.com", "domain").is_ok()); + assert!(validate_path_component("user123", "user_id").is_ok()); + assert!(validate_path_component("session-abc", "session").is_ok()); + } + + #[test] + fn rejects_empty_string() { + let result = validate_path_component("", "test"); + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), StorageError::InvalidPath(_))); + } + + #[test] + fn rejects_slash() { + let result = validate_path_component("foo/bar", "test"); + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), StorageError::InvalidPath(_))); + } + + #[test] + fn rejects_backslash() { + let result = validate_path_component("foo\\bar", "test"); + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), StorageError::InvalidPath(_))); + } + + #[test] + fn rejects_parent_directory() { + let result = validate_path_component("../etc/passwd", "test"); + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), StorageError::InvalidPath(_))); + } + + #[test] + fn rejects_null_byte() { + let result = validate_path_component("test\0value", "test"); + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), StorageError::InvalidPath(_))); + } +} diff --git a/src/storage/knowledge_store.rs b/src/storage/knowledge_store.rs index 57c3cfc..b2336ca 100644 --- a/src/storage/knowledge_store.rs +++ b/src/storage/knowledge_store.rs @@ -10,33 +10,12 @@ use std::path::PathBuf; use crate::models::agent_state::KnowledgeContext; use crate::models::knowledge::{Patterns, SiteConfig, UserModel}; -use super::error::{Result, StorageError}; +use super::error::Result; +use super::file_utils::validate_path_component; use super::traits::KnowledgeStorage; -/// Validates that a string is safe to use as a file path component. -/// Prevents path traversal attacks by rejecting strings containing: -/// - Path separators ('/', '\\') -/// - Parent directory references ('..') -/// - Null bytes -fn validate_path_component(s: &str, context: &str) -> Result<()> { - if s.is_empty() { - return Err(StorageError::InvalidPath(format!( - "{} cannot be empty", - context - ))); - } - if s.contains('/') - || s.contains('\\') - || s.contains("..") - || s.contains('\0') - { - return Err(StorageError::InvalidPath(format!( - "Invalid characters in {}: {}", - context, s - ))); - } - Ok(()) -} +/// Default token budget for knowledge context. +const DEFAULT_TOKEN_BUDGET: u64 = 8000; /// File-based persistence for site configs, user models, and patterns. #[derive(Debug)] @@ -200,7 +179,7 @@ impl KnowledgeStorage for FileKnowledgeStore { fn load_for_context(&self, domain: Option<&str>) -> Result { let mut ctx = KnowledgeContext { - token_budget: 8000, + token_budget: DEFAULT_TOKEN_BUDGET, ..Default::default() }; @@ -236,6 +215,7 @@ impl KnowledgeStorage for FileKnowledgeStore { mod tests { use super::*; use crate::models::ParseMethod; + use crate::StorageError; use tempfile::tempdir; #[test] diff --git a/src/storage/mod.rs b/src/storage/mod.rs index 8c49bbf..dee4481 100644 --- a/src/storage/mod.rs +++ b/src/storage/mod.rs @@ -20,10 +20,9 @@ //! ## Recipes //! //! Two backends are provided: -//! - [`FileRecipesDb`] - Simple JSONL storage. Good for development and small datasets. -//! Search is O(n) - scans all recipes. -//! - [`SqliteRecipesDb`] - SQLite with FTS5. Recommended for production. Provides -//! indexed full-text search and better performance at scale. +//! - [`FileRecipesDb`] - Simple JSONL storage. Search is O(n) - scans all recipes. +//! - [`SqliteRecipesDb`] - SQLite with FTS5. Provides indexed full-text search +//! and better performance at scale. //! //! ## Other Storage //! @@ -51,6 +50,7 @@ pub mod config_dir; pub mod error; pub mod file_storage; +pub mod file_utils; pub mod knowledge_store; pub mod recipes_db; pub mod session_store; diff --git a/src/storage/recipes_db.rs b/src/storage/recipes_db.rs index cdb7d7e..3eca0d3 100644 --- a/src/storage/recipes_db.rs +++ b/src/storage/recipes_db.rs @@ -1,7 +1,7 @@ //! File-based recipe storage using JSONL format. //! -//! Provides simple append-only storage. For indexed/search-heavy workloads, -//! use [`SqliteRecipesDb`] instead. +//! Provides simple append-only storage. Search is O(n) - scans all +//! recipes. use std::fs::{self, File, OpenOptions}; use std::io::{BufRead, BufReader, BufWriter, Write}; @@ -16,7 +16,7 @@ use super::traits::RecipesStorage; /// File-based, append-only recipe storage. /// /// Uses a mutex to ensure thread-safe operations. Search operations scan all -/// recipes, so for large datasets, use [`SqliteRecipesDb`] instead. +/// recipes. #[derive(Debug)] pub struct FileRecipesDb { dir: PathBuf, @@ -79,7 +79,9 @@ impl FileRecipesDb { impl RecipesStorage for FileRecipesDb { fn insert(&self, recipe: &Recipe) -> Result { // Lock to prevent race condition between check and insert - let _guard = self.lock.lock().unwrap(); + let _guard = self.lock.lock().map_err(|e| { + std::io::Error::other(format!("lock poisoned: {}", e)) + })?; // Check for duplicates by content hash if let Some(hash) = &recipe.content_hash diff --git a/src/storage/session_store.rs b/src/storage/session_store.rs index 718a2b0..10f52a9 100644 --- a/src/storage/session_store.rs +++ b/src/storage/session_store.rs @@ -9,30 +9,10 @@ use std::path::PathBuf; use crate::models::agent_state::{SessionId, SessionState}; -use super::error::{Result, StorageError}; +use super::error::Result; +use super::file_utils::validate_path_component; use super::traits::SessionStorage; -/// Validates that a session ID is safe to use in a file path. -/// Prevents path traversal attacks. -fn validate_session_id(id: &SessionId) -> Result<()> { - if id.0.is_empty() { - return Err(StorageError::InvalidPath( - "session ID cannot be empty".to_string(), - )); - } - if id.0.contains('/') - || id.0.contains('\\') - || id.0.contains("..") - || id.0.contains('\0') - { - return Err(StorageError::InvalidPath(format!( - "Invalid characters in session ID: {}", - id.0 - ))); - } - Ok(()) -} - /// File-based persistable session state for resumability. #[derive(Debug)] pub struct FileSessionStore { @@ -50,7 +30,7 @@ impl FileSessionStore { /// /// Returns an error if the session ID contains path traversal characters. fn session_path(&self, id: &SessionId) -> Result { - validate_session_id(id)?; + validate_path_component(&id.0, "session ID")?; Ok(self.dir.join(format!("{}.json", id))) } } @@ -141,6 +121,7 @@ impl SessionStorage for FileSessionStore { #[cfg(test)] mod tests { use super::*; + use crate::StorageError; use tempfile::tempdir; #[test] diff --git a/src/storage/signal_log.rs b/src/storage/signal_log.rs index 0cdc792..51b8281 100644 --- a/src/storage/signal_log.rs +++ b/src/storage/signal_log.rs @@ -12,6 +12,7 @@ use chrono::{Duration, NaiveDate, Utc}; use crate::models::signal::Signal; use super::error::Result; +use super::file_utils::validate_path_component; use super::traits::SignalStorage; /// File-based daily JSONL files for signal logging. @@ -22,14 +23,23 @@ pub struct FileSignalLog { impl FileSignalLog { /// Open the signal log at the given directory. + /// + /// The directory path should be a safe, application-controlled path + /// (e.g. from [`ConfigDir`](super::ConfigDir)). pub fn open(dir: PathBuf) -> Result { fs::create_dir_all(&dir)?; Ok(Self { dir }) } /// Get the path to a date's log file. + /// + /// Date filenames are validated by the `NaiveDate` type, ensuring + /// they cannot contain path traversal characters. fn log_file_for_date(&self, date: NaiveDate) -> PathBuf { - self.dir.join(format!("{}.jsonl", date.format("%Y-%m-%d"))) + let filename = format!("{}.jsonl", date.format("%Y-%m-%d")); + // Defensive validation even though NaiveDate guarantees safe format + let _ = validate_path_component(&filename, "date filename"); + self.dir.join(filename) } /// Get today's log file path.