diff --git a/crates/pinakes-core/src/config.rs b/crates/pinakes-core/src/config.rs index cff7d4c..543ecff 100644 --- a/crates/pinakes-core/src/config.rs +++ b/crates/pinakes-core/src/config.rs @@ -2,10 +2,25 @@ use std::path::{Path, PathBuf}; use serde::{Deserialize, Serialize}; -/// Expand environment variables in a string. -/// Supports both ${VAR_NAME} and $VAR_NAME syntax. +/// Expand environment variables in a string using `std::env::var` for lookup. +/// Supports both `${VAR_NAME}` and `$VAR_NAME` syntax. /// Returns an error if a referenced variable is not set. fn expand_env_var_string(input: &str) -> crate::error::Result { + expand_env_vars(input, |name| { + std::env::var(name).map_err(|_| { + crate::error::PinakesError::Config(format!( + "environment variable not set: {name}" + )) + }) + }) +} + +/// Expand environment variables in a string using the provided lookup function. +/// Supports both `${VAR_NAME}` and `$VAR_NAME` syntax. +fn expand_env_vars( + input: &str, + lookup: impl Fn(&str) -> crate::error::Result, +) -> crate::error::Result { let mut result = String::new(); let mut chars = input.chars().peekable(); @@ -44,16 +59,7 @@ fn expand_env_var_string(input: &str) -> crate::error::Result { )); } - // Look up the environment variable - match std::env::var(&var_name) { - Ok(value) => result.push_str(&value), - Err(_) => { - return Err(crate::error::PinakesError::Config(format!( - "environment variable not set: {}", - var_name - ))); - }, - } + result.push_str(&lookup(&var_name)?); } else if ch == '\\' { // Handle escaped characters if let Some(&next_ch) = chars.peek() { @@ -769,6 +775,21 @@ pub enum StorageBackendType { Postgres, } +impl StorageBackendType { + pub fn as_str(&self) -> &'static str { + match self { + Self::Sqlite => "sqlite", + Self::Postgres => "postgres", + } + } +} + +impl std::fmt::Display for StorageBackendType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct SqliteConfig { pub path: PathBuf, @@ -1200,64 +1221,58 @@ mod tests { assert!(config.validate().is_ok()); } - // Environment variable expansion tests + // Environment variable expansion tests using expand_env_vars with a + // HashMap lookup. This avoids unsafe std::env::set_var and is + // thread-safe for parallel test execution. + fn test_lookup<'a>( + vars: &'a std::collections::HashMap<&str, &str>, + ) -> impl Fn(&str) -> crate::error::Result + 'a { + move |name| { + vars.get(name).map(|v| v.to_string()).ok_or_else(|| { + crate::error::PinakesError::Config(format!( + "environment variable not set: {name}" + )) + }) + } + } + #[test] fn test_expand_env_var_simple() { - unsafe { - std::env::set_var("TEST_VAR_SIMPLE", "test_value"); - } - let result = expand_env_var_string("$TEST_VAR_SIMPLE"); - assert!(result.is_ok()); + let vars = + std::collections::HashMap::from([("TEST_VAR_SIMPLE", "test_value")]); + let result = expand_env_vars("$TEST_VAR_SIMPLE", test_lookup(&vars)); assert_eq!(result.unwrap(), "test_value"); - unsafe { - std::env::remove_var("TEST_VAR_SIMPLE"); - } } #[test] fn test_expand_env_var_braces() { - unsafe { - std::env::set_var("TEST_VAR_BRACES", "test_value"); - } - let result = expand_env_var_string("${TEST_VAR_BRACES}"); - assert!(result.is_ok()); + let vars = + std::collections::HashMap::from([("TEST_VAR_BRACES", "test_value")]); + let result = expand_env_vars("${TEST_VAR_BRACES}", test_lookup(&vars)); assert_eq!(result.unwrap(), "test_value"); - unsafe { - std::env::remove_var("TEST_VAR_BRACES"); - } } #[test] fn test_expand_env_var_embedded() { - unsafe { - std::env::set_var("TEST_VAR_EMBEDDED", "value"); - } - let result = expand_env_var_string("prefix_${TEST_VAR_EMBEDDED}_suffix"); - assert!(result.is_ok()); + let vars = + std::collections::HashMap::from([("TEST_VAR_EMBEDDED", "value")]); + let result = + expand_env_vars("prefix_${TEST_VAR_EMBEDDED}_suffix", test_lookup(&vars)); assert_eq!(result.unwrap(), "prefix_value_suffix"); - unsafe { - std::env::remove_var("TEST_VAR_EMBEDDED"); - } } #[test] fn test_expand_env_var_multiple() { - unsafe { - std::env::set_var("VAR1", "value1"); - std::env::set_var("VAR2", "value2"); - } - let result = expand_env_var_string("${VAR1}_${VAR2}"); - assert!(result.is_ok()); + let vars = + std::collections::HashMap::from([("VAR1", "value1"), ("VAR2", "value2")]); + let result = expand_env_vars("${VAR1}_${VAR2}", test_lookup(&vars)); assert_eq!(result.unwrap(), "value1_value2"); - unsafe { - std::env::remove_var("VAR1"); - std::env::remove_var("VAR2"); - } } #[test] fn test_expand_env_var_missing() { - let result = expand_env_var_string("${NONEXISTENT_VAR}"); + let vars = std::collections::HashMap::new(); + let result = expand_env_vars("${NONEXISTENT_VAR}", test_lookup(&vars)); assert!(result.is_err()); assert!( result @@ -1269,7 +1284,8 @@ mod tests { #[test] fn test_expand_env_var_empty_name() { - let result = expand_env_var_string("${}"); + let vars = std::collections::HashMap::new(); + let result = expand_env_vars("${}", test_lookup(&vars)); assert!(result.is_err()); assert!( result @@ -1281,43 +1297,33 @@ mod tests { #[test] fn test_expand_env_var_escaped() { - let result = expand_env_var_string("\\$NOT_A_VAR"); - assert!(result.is_ok()); + let vars = std::collections::HashMap::new(); + let result = expand_env_vars("\\$NOT_A_VAR", test_lookup(&vars)); assert_eq!(result.unwrap(), "$NOT_A_VAR"); } #[test] fn test_expand_env_var_no_vars() { - let result = expand_env_var_string("plain_text"); - assert!(result.is_ok()); + let vars = std::collections::HashMap::new(); + let result = expand_env_vars("plain_text", test_lookup(&vars)); assert_eq!(result.unwrap(), "plain_text"); } #[test] fn test_expand_env_var_underscore() { - unsafe { - std::env::set_var("TEST_VAR_NAME", "value"); - } - let result = expand_env_var_string("$TEST_VAR_NAME"); - assert!(result.is_ok()); + let vars = std::collections::HashMap::from([("TEST_VAR_NAME", "value")]); + let result = expand_env_vars("$TEST_VAR_NAME", test_lookup(&vars)); assert_eq!(result.unwrap(), "value"); - unsafe { - std::env::remove_var("TEST_VAR_NAME"); - } } #[test] fn test_expand_env_var_mixed_syntax() { - unsafe { - std::env::set_var("VAR1_MIXED", "v1"); - std::env::set_var("VAR2_MIXED", "v2"); - } - let result = expand_env_var_string("$VAR1_MIXED and ${VAR2_MIXED}"); - assert!(result.is_ok()); + let vars = std::collections::HashMap::from([ + ("VAR1_MIXED", "v1"), + ("VAR2_MIXED", "v2"), + ]); + let result = + expand_env_vars("$VAR1_MIXED and ${VAR2_MIXED}", test_lookup(&vars)); assert_eq!(result.unwrap(), "v1 and v2"); - unsafe { - std::env::remove_var("VAR1_MIXED"); - std::env::remove_var("VAR2_MIXED"); - } } } diff --git a/crates/pinakes-core/src/error.rs b/crates/pinakes-core/src/error.rs index 565d643..404bd73 100644 --- a/crates/pinakes-core/src/error.rs +++ b/crates/pinakes-core/src/error.rs @@ -105,6 +105,9 @@ pub enum PinakesError { #[error("insufficient share permissions")] InsufficientSharePermissions, + + #[error("serialization error: {0}")] + Serialization(String), } impl From for PinakesError { @@ -121,8 +124,19 @@ impl From for PinakesError { impl From for PinakesError { fn from(e: serde_json::Error) -> Self { - PinakesError::Database(format!("JSON serialization error: {}", e)) + PinakesError::Serialization(e.to_string()) } } +/// Build a closure that wraps a database error with operation context. +/// +/// Usage: `stmt.execute(params).map_err(db_ctx("insert_media", media_id))?;` +pub fn db_ctx( + operation: &str, + entity: impl std::fmt::Display, +) -> impl FnOnce(E) -> PinakesError { + let context = format!("{operation} [{entity}]"); + move |e| PinakesError::Database(format!("{context}: {e}")) +} + pub type Result = std::result::Result; diff --git a/crates/pinakes-core/src/model.rs b/crates/pinakes-core/src/model.rs index 66a6212..6838c6f 100644 --- a/crates/pinakes-core/src/model.rs +++ b/crates/pinakes-core/src/model.rs @@ -181,6 +181,23 @@ pub enum CustomFieldType { Boolean, } +impl CustomFieldType { + pub fn as_str(&self) -> &'static str { + match self { + Self::Text => "text", + Self::Number => "number", + Self::Date => "date", + Self::Boolean => "boolean", + } + } +} + +impl std::fmt::Display for CustomFieldType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + /// A tag that can be applied to media items. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Tag { @@ -210,6 +227,21 @@ pub enum CollectionKind { Virtual, } +impl CollectionKind { + pub fn as_str(&self) -> &'static str { + match self { + Self::Manual => "manual", + Self::Virtual => "virtual", + } + } +} + +impl std::fmt::Display for CollectionKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + /// A member of a collection with position tracking. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct CollectionMember { diff --git a/crates/pinakes-core/src/users.rs b/crates/pinakes-core/src/users.rs index 2ce35e0..208a897 100644 --- a/crates/pinakes-core/src/users.rs +++ b/crates/pinakes-core/src/users.rs @@ -109,6 +109,20 @@ impl LibraryPermission { pub fn can_admin(&self) -> bool { matches!(self, Self::Admin) } + + pub fn as_str(&self) -> &'static str { + match self { + Self::Read => "read", + Self::Write => "write", + Self::Admin => "admin", + } + } +} + +impl std::fmt::Display for LibraryPermission { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } } /// User's access to a specific library root diff --git a/crates/pinakes-server/src/dto.rs b/crates/pinakes-server/src/dto.rs index 3cc940c..9811ffa 100644 --- a/crates/pinakes-server/src/dto.rs +++ b/crates/pinakes-server/src/dto.rs @@ -547,7 +547,7 @@ impl From for MediaResponse { .into_iter() .map(|(k, v)| { (k, CustomFieldResponse { - field_type: format!("{:?}", v.field_type).to_lowercase(), + field_type: v.field_type.to_string(), value: v.value, }) }) @@ -587,7 +587,7 @@ impl From for CollectionResponse { id: col.id.to_string(), name: col.name, description: col.description, - kind: format!("{:?}", col.kind).to_lowercase(), + kind: col.kind.to_string(), filter_query: col.filter_query, created_at: col.created_at, updated_at: col.updated_at, @@ -715,7 +715,7 @@ impl From for UserLibraryResponse { Self { user_id: access.user_id.0.to_string(), root_path: access.root_path, - permission: format!("{:?}", access.permission).to_lowercase(), + permission: access.permission.to_string(), granted_at: access.granted_at, } } diff --git a/crates/pinakes-server/src/routes/config.rs b/crates/pinakes-server/src/routes/config.rs index e8f7956..1124a89 100644 --- a/crates/pinakes-server/src/routes/config.rs +++ b/crates/pinakes-server/src/routes/config.rs @@ -33,7 +33,7 @@ pub async fn get_config( }; Ok(Json(ConfigResponse { - backend: format!("{:?}", config.storage.backend).to_lowercase(), + backend: config.storage.backend.to_string(), database_path: config .storage .sqlite @@ -146,7 +146,7 @@ pub async fn update_scanning_config( }; Ok(Json(ConfigResponse { - backend: format!("{:?}", config.storage.backend).to_lowercase(), + backend: config.storage.backend.to_string(), database_path: config .storage .sqlite