From 13df73bef01a4a5bd8efd42908ffe8a91e44f28f Mon Sep 17 00:00:00 2001 From: Jamie Pine Date: Wed, 10 Sep 2025 17:34:11 -0400 Subject: [PATCH] refactor: streamline action and query structures - Removed unnecessary whitespace in `cqrs.rs` to enhance code cleanliness. - Reorganized imports in `addressing.rs` for better clarity and consistency. - Introduced new input types for actions, including `Input` associated types in `CoreAction` and `LibraryAction`, promoting a more modular design. - Updated action implementations to utilize the new input structures, improving maintainability and reducing redundancy. - Enhanced event handling in `event.rs` to ensure proper library ID filtering. These changes improve the overall structure and maintainability of the action and query systems while ensuring a consistent API surface. --- core/src/cqrs.rs | 1 - core/src/domain/addressing.rs | 690 +++++++++--------- core/src/infra/action/mod.rs | 14 + core/src/infra/event/mod.rs | 525 +++++++------ core/src/library/manager.rs | 30 + core/src/library/mod.rs | 11 + core/src/ops/content/action.rs | 75 -- core/src/ops/content/mod.rs | 163 ----- core/src/ops/core/status/query.rs | 8 +- core/src/ops/devices/mod.rs | 3 - core/src/ops/devices/revoke/action.rs | 82 --- core/src/ops/devices/revoke/mod.rs | 4 - core/src/ops/devices/revoke/output.rs | 32 - core/src/ops/files/copy/action.rs | 150 ++-- core/src/ops/files/copy/input.rs | 89 +-- core/src/ops/files/delete/action.rs | 33 +- core/src/ops/files/delete/input.rs | 23 +- .../ops/files/duplicate_detection/action.rs | 45 +- .../ops/files/duplicate_detection/input.rs | 4 - core/src/ops/files/validation/action.rs | 37 +- core/src/ops/files/validation/input.rs | 21 - core/src/ops/indexing/action.rs | 7 + core/src/ops/indexing/input.rs | 18 - core/src/ops/libraries/create/action.rs | 24 +- core/src/ops/libraries/create/input.rs | 21 - core/src/ops/libraries/delete/action.rs | 52 +- core/src/ops/libraries/delete/input.rs | 22 +- core/src/ops/libraries/export/action.rs | 38 +- core/src/ops/libraries/export/input.rs | 14 + core/src/ops/libraries/export/mod.rs | 3 +- core/src/ops/libraries/list/query.rs | 8 +- core/src/ops/libraries/rename/action.rs | 27 +- core/src/ops/locations/add/action.rs | 40 +- core/src/ops/locations/remove/action.rs | 23 +- core/src/ops/locations/rescan/action.rs | 31 +- core/src/ops/media/thumbnail/action.rs | 39 +- core/src/ops/metadata/action.rs | 88 --- core/src/ops/metadata/mod.rs | 310 -------- core/src/ops/mod.rs | 6 +- core/src/ops/registry.rs | 647 ++-------------- core/src/ops/volumes/speed_test/action.rs | 48 +- core/src/ops/volumes/track/action.rs | 52 +- core/src/ops/volumes/track/output.rs | 8 +- core/src/ops/volumes/untrack/action.rs | 29 +- core/src/ops/volumes/untrack/output.rs | 12 +- docs/core/op_initialization_api.md | 219 ------ 46 files changed, 1188 insertions(+), 2638 deletions(-) delete mode 100644 core/src/ops/content/action.rs delete mode 100644 core/src/ops/content/mod.rs delete mode 100644 core/src/ops/devices/mod.rs delete mode 100644 core/src/ops/devices/revoke/action.rs delete mode 100644 core/src/ops/devices/revoke/mod.rs delete mode 100644 core/src/ops/devices/revoke/output.rs create mode 100644 core/src/ops/libraries/export/input.rs delete mode 100644 core/src/ops/metadata/action.rs delete mode 100644 core/src/ops/metadata/mod.rs delete mode 100644 docs/core/op_initialization_api.md diff --git a/core/src/cqrs.rs b/core/src/cqrs.rs index f015e7a02..a0243fe15 100644 --- a/core/src/cqrs.rs +++ b/core/src/cqrs.rs @@ -40,4 +40,3 @@ impl QueryManager { query.execute(self.context.clone()).await } } - diff --git a/core/src/domain/addressing.rs b/core/src/domain/addressing.rs index 3129c2200..f2017b2ba 100644 --- a/core/src/domain/addressing.rs +++ b/core/src/domain/addressing.rs @@ -4,10 +4,10 @@ //! the data structures that represent paths in Spacedrive's distributed //! file system. -use std::path::{Path, PathBuf}; +use serde::{Deserialize, Serialize}; use std::fmt; +use std::path::{Path, PathBuf}; use uuid::Uuid; -use serde::{Serialize, Deserialize}; /// A path within the Spacedrive Virtual Distributed File System /// @@ -20,410 +20,434 @@ use serde::{Serialize, Deserialize}; /// content-based paths to be resolved to optimal physical locations at runtime. #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] pub enum SdPath { - /// A direct pointer to a file at a specific path on a specific device - Physical { - /// The device where this file exists - device_id: Uuid, - /// The local path on that device - path: PathBuf, - }, - /// An abstract, location-independent handle that refers to file content - Content { - /// The unique content identifier - content_id: Uuid, - }, + /// A direct pointer to a file at a specific path on a specific device + Physical { + /// The device where this file exists + device_id: Uuid, + /// The local path on that device + path: PathBuf, + }, + /// An abstract, location-independent handle that refers to file content + Content { + /// The unique content identifier + content_id: Uuid, + }, } impl SdPath { - /// Create a new physical SdPath - pub fn new(device_id: Uuid, path: impl Into) -> Self { - Self::physical(device_id, path) - } + /// Create a new physical SdPath + pub fn new(device_id: Uuid, path: impl Into) -> Self { + Self::physical(device_id, path) + } - /// Create a physical SdPath with specific device and path - pub fn physical(device_id: Uuid, path: impl Into) -> Self { - Self::Physical { - device_id, - path: path.into(), - } - } + /// Create a physical SdPath with specific device and path + pub fn physical(device_id: Uuid, path: impl Into) -> Self { + Self::Physical { + device_id, + path: path.into(), + } + } - /// Create a content-addressed SdPath - pub fn content(content_id: Uuid) -> Self { - Self::Content { content_id } - } + /// Create a content-addressed SdPath + pub fn content(content_id: Uuid) -> Self { + Self::Content { content_id } + } - /// Create an SdPath for a local file on this device - pub fn local(path: impl Into) -> Self { - Self::Physical { - device_id: crate::common::utils::get_current_device_id(), - path: path.into(), - } - } + /// Create an SdPath for a local file on this device + pub fn local(path: impl Into) -> Self { + Self::Physical { + device_id: crate::common::utils::get_current_device_id(), + path: path.into(), + } + } - /// Check if this path is on the current device - pub fn is_local(&self) -> bool { - match self { - Self::Physical { device_id, .. } => *device_id == crate::common::utils::get_current_device_id(), - Self::Content { .. } => false, // Content paths are abstract, not inherently local - } - } + /// Check if this path is on the current device + pub fn is_local(&self) -> bool { + match self { + Self::Physical { device_id, .. } => { + *device_id == crate::common::utils::get_current_device_id() + } + Self::Content { .. } => false, // Content paths are abstract, not inherently local + } + } - /// Get the local PathBuf if this is a local path - pub fn as_local_path(&self) -> Option<&Path> { - match self { - Self::Physical { device_id, path } => { - if *device_id == crate::common::utils::get_current_device_id() { - Some(path) - } else { - None - } - } - Self::Content { .. } => None, - } - } + /// Get the local PathBuf if this is a local path + pub fn as_local_path(&self) -> Option<&Path> { + match self { + Self::Physical { device_id, path } => { + if *device_id == crate::common::utils::get_current_device_id() { + Some(path) + } else { + None + } + } + Self::Content { .. } => None, + } + } - /// Convert to a display string - pub fn display(&self) -> String { - match self { - Self::Physical { device_id, path } => { - if *device_id == crate::common::utils::get_current_device_id() { - path.display().to_string() - } else { - format!("sd://{}/{}", device_id, path.display()) - } - } - Self::Content { content_id } => { - format!("sd://content/{}", content_id) - } - } - } + /// Convert to a display string + pub fn display(&self) -> String { + match self { + Self::Physical { device_id, path } => { + if *device_id == crate::common::utils::get_current_device_id() { + path.display().to_string() + } else { + format!("sd://{}/{}", device_id, path.display()) + } + } + Self::Content { content_id } => { + format!("sd://content/{}", content_id) + } + } + } - /// Get just the file name - pub fn file_name(&self) -> Option<&str> { - match self { - Self::Physical { path, .. } => path.file_name()?.to_str(), - Self::Content { .. } => None, // Content paths don't have filenames - } - } + /// Get just the file name + pub fn file_name(&self) -> Option<&str> { + match self { + Self::Physical { path, .. } => path.file_name()?.to_str(), + Self::Content { .. } => None, // Content paths don't have filenames + } + } - /// Get the parent directory as an SdPath - pub fn parent(&self) -> Option { - match self { - Self::Physical { device_id, path } => { - path.parent().map(|p| Self::Physical { - device_id: *device_id, - path: p.to_path_buf(), - }) - } - Self::Content { .. } => None, // Content paths don't have parents - } - } + /// Get the parent directory as an SdPath + pub fn parent(&self) -> Option { + match self { + Self::Physical { device_id, path } => path.parent().map(|p| Self::Physical { + device_id: *device_id, + path: p.to_path_buf(), + }), + Self::Content { .. } => None, // Content paths don't have parents + } + } - /// Join with another path component - /// Panics if called on a Content variant - pub fn join(&self, path: impl AsRef) -> SdPath { - match self { - Self::Physical { device_id, path: base_path } => { - Self::Physical { - device_id: *device_id, - path: base_path.join(path), - } - } - Self::Content { .. } => panic!("Cannot join paths to content addresses"), - } - } + /// Join with another path component + /// Panics if called on a Content variant + pub fn join(&self, path: impl AsRef) -> SdPath { + match self { + Self::Physical { + device_id, + path: base_path, + } => Self::Physical { + device_id: *device_id, + path: base_path.join(path), + }, + Self::Content { .. } => panic!("Cannot join paths to content addresses"), + } + } - /// Get the volume that contains this path (if local and volume manager available) - pub async fn get_volume(&self, volume_manager: &crate::volume::VolumeManager) -> Option { - match self { - Self::Physical { .. } => { - if let Some(local_path) = self.as_local_path() { - volume_manager.volume_for_path(local_path).await - } else { - None - } - } - Self::Content { .. } => None, // Content paths don't have volumes until resolved - } - } + /// Get the volume that contains this path (if local and volume manager available) + pub async fn get_volume( + &self, + volume_manager: &crate::volume::VolumeManager, + ) -> Option { + match self { + Self::Physical { .. } => { + if let Some(local_path) = self.as_local_path() { + volume_manager.volume_for_path(local_path).await + } else { + None + } + } + Self::Content { .. } => None, // Content paths don't have volumes until resolved + } + } - /// Check if this path is on the same volume as another path - pub async fn same_volume(&self, other: &SdPath, volume_manager: &crate::volume::VolumeManager) -> bool { - match (self, other) { - (Self::Physical { .. }, Self::Physical { .. }) => { - if !self.is_local() || !other.is_local() { - return false; - } + /// Check if this path is on the same volume as another path + pub async fn same_volume( + &self, + other: &SdPath, + volume_manager: &crate::volume::VolumeManager, + ) -> bool { + match (self, other) { + (Self::Physical { .. }, Self::Physical { .. }) => { + if !self.is_local() || !other.is_local() { + return false; + } - if let (Some(self_path), Some(other_path)) = (self.as_local_path(), other.as_local_path()) { - volume_manager.same_volume(self_path, other_path).await - } else { - false - } - } - _ => false, // Content paths or mixed types can't be compared for volume - } - } + if let (Some(self_path), Some(other_path)) = + (self.as_local_path(), other.as_local_path()) + { + volume_manager.same_volume(self_path, other_path).await + } else { + false + } + } + _ => false, // Content paths or mixed types can't be compared for volume + } + } - /// Parse an SdPath from a URI string - /// Examples: - /// - "sd://device_id/path/to/file" -> Physical path - /// - "sd://content/content_id" -> Content path - /// - "/local/path" -> Local physical path - pub fn from_uri(uri: &str) -> Result { - if uri.starts_with("sd://") { - let uri = &uri[5..]; // Strip "sd://" + /// Parse an SdPath from a URI string + /// Examples: + /// - "sd://device_id/path/to/file" -> Physical path + /// - "sd://content/content_id" -> Content path + /// - "/local/path" -> Local physical path + pub fn from_uri(uri: &str) -> Result { + if uri.starts_with("sd://") { + let uri = &uri[5..]; // Strip "sd://" - if let Some(content_id_str) = uri.strip_prefix("content/") { - // Parse content path - let content_id = Uuid::parse_str(content_id_str) - .map_err(|_| SdPathParseError::InvalidContentId)?; - Ok(Self::Content { content_id }) - } else { - // Parse physical path - let parts: Vec<&str> = uri.splitn(2, '/').collect(); - if parts.len() != 2 { - return Err(SdPathParseError::InvalidFormat); - } + if let Some(content_id_str) = uri.strip_prefix("content/") { + // Parse content path + let content_id = Uuid::parse_str(content_id_str) + .map_err(|_| SdPathParseError::InvalidContentId)?; + Ok(Self::Content { content_id }) + } else { + // Parse physical path + let parts: Vec<&str> = uri.splitn(2, '/').collect(); + if parts.len() != 2 { + return Err(SdPathParseError::InvalidFormat); + } - let device_id = Uuid::parse_str(parts[0]) - .map_err(|_| SdPathParseError::InvalidDeviceId)?; - let path = PathBuf::from("/").join(parts[1]); + let device_id = + Uuid::parse_str(parts[0]).map_err(|_| SdPathParseError::InvalidDeviceId)?; + let path = PathBuf::from("/").join(parts[1]); - Ok(Self::Physical { device_id, path }) - } - } else { - // Assume local path - Ok(Self::local(uri)) - } - } + Ok(Self::Physical { device_id, path }) + } + } else { + // Assume local path + Ok(Self::local(uri)) + } + } - /// Convert to a URI string - pub fn to_uri(&self) -> String { - self.display() - } + /// Convert to a URI string + pub fn to_uri(&self) -> String { + self.display() + } - /// Get the device ID if this is a Physical path - pub fn device_id(&self) -> Option { - match self { - Self::Physical { device_id, .. } => Some(*device_id), - Self::Content { .. } => None, - } - } + /// Get the device ID if this is a Physical path + pub fn device_id(&self) -> Option { + match self { + Self::Physical { device_id, .. } => Some(*device_id), + Self::Content { .. } => None, + } + } - /// Get the path if this is a Physical path - pub fn path(&self) -> Option<&PathBuf> { - match self { - Self::Physical { path, .. } => Some(path), - Self::Content { .. } => None, - } - } + /// Get the path if this is a Physical path + pub fn path(&self) -> Option<&PathBuf> { + match self { + Self::Physical { path, .. } => Some(path), + Self::Content { .. } => None, + } + } - /// Get the content ID if this is a Content path - pub fn content_id(&self) -> Option { - match self { - Self::Content { content_id } => Some(*content_id), - Self::Physical { .. } => None, - } - } + /// Get the content ID if this is a Content path + pub fn content_id(&self) -> Option { + match self { + Self::Content { content_id } => Some(*content_id), + Self::Physical { .. } => None, + } + } - /// Check if this is a Physical path - pub fn is_physical(&self) -> bool { - matches!(self, Self::Physical { .. }) - } + /// Check if this is a Physical path + pub fn is_physical(&self) -> bool { + matches!(self, Self::Physical { .. }) + } - /// Check if this is a Content path - pub fn is_content(&self) -> bool { - matches!(self, Self::Content { .. }) - } + /// Check if this is a Content path + pub fn is_content(&self) -> bool { + matches!(self, Self::Content { .. }) + } - /// Try to get as a Physical path, returning device_id and path - pub fn as_physical(&self) -> Option<(Uuid, &PathBuf)> { - match self { - Self::Physical { device_id, path } => Some((*device_id, path)), - Self::Content { .. } => None, - } - } + /// Try to get as a Physical path, returning device_id and path + pub fn as_physical(&self) -> Option<(Uuid, &PathBuf)> { + match self { + Self::Physical { device_id, path } => Some((*device_id, path)), + Self::Content { .. } => None, + } + } - /// Resolve this path to an optimal physical location - /// This is the entry point for path resolution that will use the PathResolver service - pub async fn resolve( - &self, - context: &crate::context::CoreContext - ) -> Result { - let resolver = crate::ops::addressing::PathResolver; - resolver.resolve(self, context).await - } + /// Resolve this path to an optimal physical location + /// This is the entry point for path resolution that will use the PathResolver service + pub async fn resolve( + &self, + context: &crate::context::CoreContext, + ) -> Result { + let resolver = crate::ops::addressing::PathResolver; + resolver.resolve(self, context).await + } - /// Resolve this path using a JobContext - pub async fn resolve_in_job<'a>( - &self, - job_ctx: &crate::infra::job::context::JobContext<'a> - ) -> Result { - // For now, if it's already physical, just return it - // TODO: Implement proper resolution using job context's library and networking - match self { - Self::Physical { .. } => Ok(self.clone()), - Self::Content { content_id } => { - // In the future, use job_ctx.library_db() to query for content instances - Err(PathResolutionError::NoOnlineInstancesFound(*content_id)) - } - } - } + /// Resolve this path using a JobContext + pub async fn resolve_in_job<'a>( + &self, + job_ctx: &crate::infra::job::context::JobContext<'a>, + ) -> Result { + // For now, if it's already physical, just return it + // TODO: Implement proper resolution using job context's library and networking + match self { + Self::Physical { .. } => Ok(self.clone()), + Self::Content { content_id } => { + // In the future, use job_ctx.library_db() to query for content instances + Err(PathResolutionError::NoOnlineInstancesFound(*content_id)) + } + } + } } /// Error type for path resolution #[derive(Debug, Clone, PartialEq, Eq)] pub enum PathResolutionError { - NoOnlineInstancesFound(Uuid), - DeviceOffline(Uuid), - NoActiveLibrary, - DatabaseError(String), + NoOnlineInstancesFound(Uuid), + DeviceOffline(Uuid), + NoActiveLibrary, + DatabaseError(String), } impl fmt::Display for PathResolutionError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::NoOnlineInstancesFound(id) => write!(f, "No online instances found for content: {}", id), - Self::DeviceOffline(id) => write!(f, "Device is offline: {}", id), - Self::NoActiveLibrary => write!(f, "No active library"), - Self::DatabaseError(msg) => write!(f, "Database error: {}", msg), - } - } + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::NoOnlineInstancesFound(id) => { + write!(f, "No online instances found for content: {}", id) + } + Self::DeviceOffline(id) => write!(f, "Device is offline: {}", id), + Self::NoActiveLibrary => write!(f, "No active library"), + Self::DatabaseError(msg) => write!(f, "Database error: {}", msg), + } + } } impl std::error::Error for PathResolutionError {} impl From for PathResolutionError { - fn from(err: sea_orm::DbErr) -> Self { - PathResolutionError::DatabaseError(err.to_string()) - } + fn from(err: sea_orm::DbErr) -> Self { + PathResolutionError::DatabaseError(err.to_string()) + } } /// Error type for SdPath parsing #[derive(Debug, Clone, PartialEq, Eq)] pub enum SdPathParseError { - InvalidFormat, - InvalidDeviceId, - InvalidContentId, + InvalidFormat, + InvalidDeviceId, + InvalidContentId, } impl fmt::Display for SdPathParseError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::InvalidFormat => write!(f, "Invalid SdPath URI format"), - Self::InvalidDeviceId => write!(f, "Invalid device ID in SdPath URI"), - Self::InvalidContentId => write!(f, "Invalid content ID in SdPath URI"), - } - } + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::InvalidFormat => write!(f, "Invalid SdPath URI format"), + Self::InvalidDeviceId => write!(f, "Invalid device ID in SdPath URI"), + Self::InvalidContentId => write!(f, "Invalid content ID in SdPath URI"), + } + } } impl std::error::Error for SdPathParseError {} impl fmt::Display for SdPath { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.display()) - } + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.display()) + } } /// A batch of SdPaths, useful for operations on multiple files -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq, Eq)] pub struct SdPathBatch { - pub paths: Vec, + pub paths: Vec, } impl SdPathBatch { - /// Create a new batch - pub fn new(paths: Vec) -> Self { - Self { paths } - } + /// Create a new batch + pub fn new(paths: Vec) -> Self { + Self { paths } + } - /// Filter to only local paths - pub fn local_only(&self) -> Vec<&Path> { - self.paths.iter() - .filter_map(|p| p.as_local_path()) - .collect() - } + /// Filter to only local paths + pub fn local_only(&self) -> Vec<&Path> { + self.paths + .iter() + .filter_map(|p| p.as_local_path()) + .collect() + } - /// Group by device - pub fn by_device(&self) -> std::collections::HashMap> { - let mut map = std::collections::HashMap::new(); - for path in &self.paths { - if let Some(device_id) = path.device_id() { - map.entry(device_id).or_insert_with(Vec::new).push(path); - } - } - map - } + /// Group by device + pub fn by_device(&self) -> std::collections::HashMap> { + let mut map = std::collections::HashMap::new(); + for path in &self.paths { + if let Some(device_id) = path.device_id() { + map.entry(device_id).or_insert_with(Vec::new).push(path); + } + } + map + } + + /// add multiple paths + pub fn extend(&mut self, paths: Vec) { + self.paths.extend(paths); + } } #[cfg(test)] mod tests { - use super::*; + use super::*; - #[test] - fn test_sdpath_physical_creation() { - let device_id = Uuid::new_v4(); - let path = SdPath::new(device_id, "/home/user/file.txt"); + #[test] + fn test_sdpath_physical_creation() { + let device_id = Uuid::new_v4(); + let path = SdPath::new(device_id, "/home/user/file.txt"); - match path { - SdPath::Physical { device_id: did, path: p } => { - assert_eq!(did, device_id); - assert_eq!(p, PathBuf::from("/home/user/file.txt")); - } - _ => panic!("Expected Physical variant"), - } - } + match path { + SdPath::Physical { + device_id: did, + path: p, + } => { + assert_eq!(did, device_id); + assert_eq!(p, PathBuf::from("/home/user/file.txt")); + } + _ => panic!("Expected Physical variant"), + } + } - #[test] - fn test_sdpath_content_creation() { - let content_id = Uuid::new_v4(); - let path = SdPath::content(content_id); + #[test] + fn test_sdpath_content_creation() { + let content_id = Uuid::new_v4(); + let path = SdPath::content(content_id); - match path { - SdPath::Content { content_id: cid } => { - assert_eq!(cid, content_id); - } - _ => panic!("Expected Content variant"), - } - } + match path { + SdPath::Content { content_id: cid } => { + assert_eq!(cid, content_id); + } + _ => panic!("Expected Content variant"), + } + } - #[test] - fn test_sdpath_display() { - let device_id = Uuid::new_v4(); - let path = SdPath::new(device_id, "/home/user/file.txt"); + #[test] + fn test_sdpath_display() { + let device_id = Uuid::new_v4(); + let path = SdPath::new(device_id, "/home/user/file.txt"); - let display = path.display(); - assert!(display.contains(&device_id.to_string())); - assert!(display.contains("/home/user/file.txt")); - } + let display = path.display(); + assert!(display.contains(&device_id.to_string())); + assert!(display.contains("/home/user/file.txt")); + } - #[test] - fn test_sdpath_uri_parsing() { - // Test content URI - let content_id = Uuid::new_v4(); - let uri = format!("sd://content/{}", content_id); - let path = SdPath::from_uri(&uri).unwrap(); - match path { - SdPath::Content { content_id: cid } => assert_eq!(cid, content_id), - _ => panic!("Expected Content variant"), - } + #[test] + fn test_sdpath_uri_parsing() { + // Test content URI + let content_id = Uuid::new_v4(); + let uri = format!("sd://content/{}", content_id); + let path = SdPath::from_uri(&uri).unwrap(); + match path { + SdPath::Content { content_id: cid } => assert_eq!(cid, content_id), + _ => panic!("Expected Content variant"), + } - // Test physical URI - let device_id = Uuid::new_v4(); - let uri = format!("sd://{}/home/user/file.txt", device_id); - let path = SdPath::from_uri(&uri).unwrap(); - match path { - SdPath::Physical { device_id: did, path: p } => { - assert_eq!(did, device_id); - assert_eq!(p, PathBuf::from("/home/user/file.txt")); - } - _ => panic!("Expected Physical variant"), - } + // Test physical URI + let device_id = Uuid::new_v4(); + let uri = format!("sd://{}/home/user/file.txt", device_id); + let path = SdPath::from_uri(&uri).unwrap(); + match path { + SdPath::Physical { + device_id: did, + path: p, + } => { + assert_eq!(did, device_id); + assert_eq!(p, PathBuf::from("/home/user/file.txt")); + } + _ => panic!("Expected Physical variant"), + } - // Test local path - let path = SdPath::from_uri("/local/path").unwrap(); - assert!(path.is_local()); - } -} \ No newline at end of file + // Test local path + let path = SdPath::from_uri("/local/path").unwrap(); + assert!(path.is_local()); + } +} diff --git a/core/src/infra/action/mod.rs b/core/src/infra/action/mod.rs index df885093a..7a771b3e0 100644 --- a/core/src/infra/action/mod.rs +++ b/core/src/infra/action/mod.rs @@ -24,6 +24,13 @@ pub mod receipt; pub trait CoreAction: Send + Sync + 'static { /// The output type for this action - can be domain objects, job handles, etc. type Output: Send + Sync + 'static; + /// The associated input type (wire contract) for this action + type Input: Send + Sync + 'static; + + /// Build this action from its associated input + fn from_input(input: Self::Input) -> Result + where + Self: Sized; /// Execute this action with core context only async fn execute( @@ -50,6 +57,13 @@ pub trait CoreAction: Send + Sync + 'static { pub trait LibraryAction: Send + Sync + 'static { /// The output type for this action - can be domain objects, job handles, etc. type Output: Send + Sync + 'static; + /// The associated input type (wire contract) for this action + type Input: Send + Sync + 'static; + + /// Build this action from its associated input + fn from_input(input: Self::Input) -> Result + where + Self: Sized; /// Execute this action with validated library and core context async fn execute( diff --git a/core/src/infra/event/mod.rs b/core/src/infra/event/mod.rs index 60675d114..aac468f20 100644 --- a/core/src/infra/event/mod.rs +++ b/core/src/infra/event/mod.rs @@ -10,275 +10,350 @@ use uuid::Uuid; /// Core events that can be emitted throughout the system #[derive(Debug, Clone, Serialize, Deserialize)] pub enum Event { - // Core lifecycle events - CoreStarted, - CoreShutdown, + // Core lifecycle events + CoreStarted, + CoreShutdown, - // Library events - LibraryCreated { id: Uuid, name: String, path: PathBuf }, - LibraryOpened { id: Uuid, name: String, path: PathBuf }, - LibraryClosed { id: Uuid, name: String }, - LibraryDeleted { id: Uuid }, + // Library events + LibraryCreated { + id: Uuid, + name: String, + path: PathBuf, + }, + LibraryOpened { + id: Uuid, + name: String, + path: PathBuf, + }, + LibraryClosed { + id: Uuid, + name: String, + }, + LibraryDeleted { + id: Uuid, + name: String, + deleted_data: bool, + }, - // Entry events (file/directory operations) - EntryCreated { library_id: Uuid, entry_id: Uuid }, - EntryModified { library_id: Uuid, entry_id: Uuid }, - EntryDeleted { library_id: Uuid, entry_id: Uuid }, - EntryMoved { - library_id: Uuid, - entry_id: Uuid, - old_path: String, - new_path: String - }, + // Entry events (file/directory operations) + EntryCreated { + library_id: Uuid, + entry_id: Uuid, + }, + EntryModified { + library_id: Uuid, + entry_id: Uuid, + }, + EntryDeleted { + library_id: Uuid, + entry_id: Uuid, + }, + EntryMoved { + library_id: Uuid, + entry_id: Uuid, + old_path: String, + new_path: String, + }, - // Volume events - VolumeAdded(crate::volume::Volume), - VolumeRemoved { - fingerprint: crate::volume::VolumeFingerprint - }, - VolumeUpdated { - fingerprint: crate::volume::VolumeFingerprint, - old_info: crate::volume::VolumeInfo, - new_info: crate::volume::VolumeInfo, - }, - VolumeSpeedTested { - fingerprint: crate::volume::VolumeFingerprint, - read_speed_mbps: u64, - write_speed_mbps: u64, - }, - VolumeMountChanged { - fingerprint: crate::volume::VolumeFingerprint, - is_mounted: bool, - }, - VolumeError { - fingerprint: crate::volume::VolumeFingerprint, - error: String, - }, + // Volume events + VolumeAdded(crate::volume::Volume), + VolumeRemoved { + fingerprint: crate::volume::VolumeFingerprint, + }, + VolumeUpdated { + fingerprint: crate::volume::VolumeFingerprint, + old_info: crate::volume::VolumeInfo, + new_info: crate::volume::VolumeInfo, + }, + VolumeSpeedTested { + fingerprint: crate::volume::VolumeFingerprint, + read_speed_mbps: u64, + write_speed_mbps: u64, + }, + VolumeMountChanged { + fingerprint: crate::volume::VolumeFingerprint, + is_mounted: bool, + }, + VolumeError { + fingerprint: crate::volume::VolumeFingerprint, + error: String, + }, - // Job events - JobQueued { job_id: String, job_type: String }, - JobStarted { job_id: String, job_type: String }, - JobProgress { - job_id: String, - job_type: String, - progress: f64, - message: Option, - // Enhanced progress data - serialized GenericProgress - generic_progress: Option, - }, - JobCompleted { job_id: String, job_type: String, output: JobOutput }, - JobFailed { - job_id: String, - job_type: String, - error: String - }, - JobCancelled { job_id: String, job_type: String }, - JobPaused { job_id: String }, - JobResumed { job_id: String }, + // Job events + JobQueued { + job_id: String, + job_type: String, + }, + JobStarted { + job_id: String, + job_type: String, + }, + JobProgress { + job_id: String, + job_type: String, + progress: f64, + message: Option, + // Enhanced progress data - serialized GenericProgress + generic_progress: Option, + }, + JobCompleted { + job_id: String, + job_type: String, + output: JobOutput, + }, + JobFailed { + job_id: String, + job_type: String, + error: String, + }, + JobCancelled { + job_id: String, + job_type: String, + }, + JobPaused { + job_id: String, + }, + JobResumed { + job_id: String, + }, - // Indexing events - IndexingStarted { location_id: Uuid }, - IndexingProgress { - location_id: Uuid, - processed: u64, - total: Option - }, - IndexingCompleted { - location_id: Uuid, - total_files: u64, - total_dirs: u64 - }, - IndexingFailed { location_id: Uuid, error: String }, + // Indexing events + IndexingStarted { + location_id: Uuid, + }, + IndexingProgress { + location_id: Uuid, + processed: u64, + total: Option, + }, + IndexingCompleted { + location_id: Uuid, + total_files: u64, + total_dirs: u64, + }, + IndexingFailed { + location_id: Uuid, + error: String, + }, - // Device events - DeviceConnected { device_id: Uuid, device_name: String }, - DeviceDisconnected { device_id: Uuid }, + // Device events + DeviceConnected { + device_id: Uuid, + device_name: String, + }, + DeviceDisconnected { + device_id: Uuid, + }, - // Legacy events (for compatibility) - LocationAdded { - library_id: Uuid, - location_id: Uuid, - path: PathBuf, - }, - LocationRemoved { - library_id: Uuid, - location_id: Uuid, - }, - FilesIndexed { - library_id: Uuid, - location_id: Uuid, - count: usize, - }, - ThumbnailsGenerated { - library_id: Uuid, - count: usize, - }, - FileOperationCompleted { - library_id: Uuid, - operation: FileOperation, - affected_files: usize, - }, - FilesModified { - library_id: Uuid, - paths: Vec, - }, + // Legacy events (for compatibility) + LocationAdded { + library_id: Uuid, + location_id: Uuid, + path: PathBuf, + }, + LocationRemoved { + library_id: Uuid, + location_id: Uuid, + }, + FilesIndexed { + library_id: Uuid, + location_id: Uuid, + count: usize, + }, + ThumbnailsGenerated { + library_id: Uuid, + count: usize, + }, + FileOperationCompleted { + library_id: Uuid, + operation: FileOperation, + affected_files: usize, + }, + FilesModified { + library_id: Uuid, + paths: Vec, + }, - // Custom events for extensibility - Custom { - event_type: String, - data: serde_json::Value - }, + // Custom events for extensibility + Custom { + event_type: String, + data: serde_json::Value, + }, } /// Types of file operations #[derive(Debug, Clone, Serialize, Deserialize)] pub enum FileOperation { - Copy, - Move, - Delete, - Rename, + Copy, + Move, + Delete, + Rename, } /// Event bus for broadcasting events #[derive(Debug, Clone)] pub struct EventBus { - sender: broadcast::Sender, + sender: broadcast::Sender, } impl EventBus { - /// Create a new event bus with specified capacity - pub fn new(capacity: usize) -> Self { - let (sender, _) = broadcast::channel(capacity); - Self { sender } - } + /// Create a new event bus with specified capacity + pub fn new(capacity: usize) -> Self { + let (sender, _) = broadcast::channel(capacity); + Self { sender } + } - /// Emit an event to all subscribers - pub fn emit(&self, event: Event) { - match self.sender.send(event.clone()) { - Ok(subscriber_count) => { - debug!("Event emitted to {} subscribers", subscriber_count); - } - Err(_) => { - // No subscribers - this is fine, just debug log it - debug!("Event emitted but no subscribers: {:?}", event); - } - } - } + /// Emit an event to all subscribers + pub fn emit(&self, event: Event) { + match self.sender.send(event.clone()) { + Ok(subscriber_count) => { + debug!("Event emitted to {} subscribers", subscriber_count); + } + Err(_) => { + // No subscribers - this is fine, just debug log it + debug!("Event emitted but no subscribers: {:?}", event); + } + } + } - /// Subscribe to events - pub fn subscribe(&self) -> EventSubscriber { - EventSubscriber { - receiver: self.sender.subscribe(), - } - } + /// Subscribe to events + pub fn subscribe(&self) -> EventSubscriber { + EventSubscriber { + receiver: self.sender.subscribe(), + } + } - /// Get the number of active subscribers - pub fn subscriber_count(&self) -> usize { - self.sender.receiver_count() - } + /// Get the number of active subscribers + pub fn subscriber_count(&self) -> usize { + self.sender.receiver_count() + } } impl Default for EventBus { - fn default() -> Self { - Self::new(1024) - } + fn default() -> Self { + Self::new(1024) + } } /// Event subscriber for receiving events #[derive(Debug)] pub struct EventSubscriber { - receiver: broadcast::Receiver, + receiver: broadcast::Receiver, } impl EventSubscriber { - /// Receive the next event (blocking) - pub async fn recv(&mut self) -> Result { - self.receiver.recv().await - } + /// Receive the next event (blocking) + pub async fn recv(&mut self) -> Result { + self.receiver.recv().await + } - /// Try to receive an event without blocking - pub fn try_recv(&mut self) -> Result { - self.receiver.try_recv() - } + /// Try to receive an event without blocking + pub fn try_recv(&mut self) -> Result { + self.receiver.try_recv() + } - /// Filter events by type using a closure - pub async fn recv_filtered(&mut self, filter: F) -> Result - where - F: Fn(&Event) -> bool, - { - loop { - let event = self.recv().await?; - if filter(&event) { - return Ok(event); - } - } - } + /// Filter events by type using a closure + pub async fn recv_filtered( + &mut self, + filter: F, + ) -> Result + where + F: Fn(&Event) -> bool, + { + loop { + let event = self.recv().await?; + if filter(&event) { + return Ok(event); + } + } + } } /// Helper trait for event filtering pub trait EventFilter { - fn is_library_event(&self) -> bool; - fn is_volume_event(&self) -> bool; - fn is_job_event(&self) -> bool; - fn is_for_library(&self, library_id: Uuid) -> bool; + fn is_library_event(&self) -> bool; + fn is_volume_event(&self) -> bool; + fn is_job_event(&self) -> bool; + fn is_for_library(&self, library_id: Uuid) -> bool; } impl EventFilter for Event { - fn is_library_event(&self) -> bool { - matches!( - self, - Event::LibraryCreated { .. } - | Event::LibraryOpened { .. } - | Event::LibraryClosed { .. } - | Event::LibraryDeleted { .. } - | Event::EntryCreated { .. } - | Event::EntryModified { .. } - | Event::EntryDeleted { .. } - | Event::EntryMoved { .. } - ) - } + fn is_library_event(&self) -> bool { + matches!( + self, + Event::LibraryCreated { .. } + | Event::LibraryOpened { .. } + | Event::LibraryClosed { .. } + | Event::LibraryDeleted { .. } + | Event::EntryCreated { .. } + | Event::EntryModified { .. } + | Event::EntryDeleted { .. } + | Event::EntryMoved { .. } + ) + } - fn is_volume_event(&self) -> bool { - matches!( - self, - Event::VolumeAdded(_) - | Event::VolumeRemoved { .. } - | Event::VolumeUpdated { .. } - | Event::VolumeSpeedTested { .. } - | Event::VolumeMountChanged { .. } - | Event::VolumeError { .. } - ) - } + fn is_volume_event(&self) -> bool { + matches!( + self, + Event::VolumeAdded(_) + | Event::VolumeRemoved { .. } + | Event::VolumeUpdated { .. } + | Event::VolumeSpeedTested { .. } + | Event::VolumeMountChanged { .. } + | Event::VolumeError { .. } + ) + } - fn is_job_event(&self) -> bool { - matches!( - self, - Event::JobQueued { .. } - | Event::JobStarted { .. } - | Event::JobProgress { .. } - | Event::JobCompleted { .. } - | Event::JobFailed { .. } - | Event::JobCancelled { .. } - ) - } + fn is_job_event(&self) -> bool { + matches!( + self, + Event::JobQueued { .. } + | Event::JobStarted { .. } + | Event::JobProgress { .. } + | Event::JobCompleted { .. } + | Event::JobFailed { .. } + | Event::JobCancelled { .. } + ) + } - fn is_for_library(&self, library_id: Uuid) -> bool { - match self { - Event::LibraryCreated { id, .. } - | Event::LibraryOpened { id, .. } - | Event::LibraryClosed { id, .. } - | Event::LibraryDeleted { id } => *id == library_id, - Event::EntryCreated { library_id: lid, .. } - | Event::EntryModified { library_id: lid, .. } - | Event::EntryDeleted { library_id: lid, .. } - | Event::EntryMoved { library_id: lid, .. } => *lid == library_id, - Event::LocationAdded { library_id: lid, .. } - | Event::LocationRemoved { library_id: lid, .. } - | Event::FilesIndexed { library_id: lid, .. } - | Event::ThumbnailsGenerated { library_id: lid, .. } - | Event::FileOperationCompleted { library_id: lid, .. } - | Event::FilesModified { library_id: lid, .. } => *lid == library_id, - _ => false, - } - } -} \ No newline at end of file + // TODO: events should have an envelope that contains the library_id instead of this + fn is_for_library(&self, library_id: Uuid) -> bool { + match self { + Event::LibraryCreated { id, .. } + | Event::LibraryOpened { id, .. } + | Event::LibraryClosed { id, .. } + | Event::LibraryDeleted { id, .. } => *id == library_id, + Event::EntryCreated { + library_id: lid, .. + } + | Event::EntryModified { + library_id: lid, .. + } + | Event::EntryDeleted { + library_id: lid, .. + } + | Event::EntryMoved { + library_id: lid, .. + } => *lid == library_id, + Event::LocationAdded { + library_id: lid, .. + } + | Event::LocationRemoved { + library_id: lid, .. + } + | Event::FilesIndexed { + library_id: lid, .. + } + | Event::ThumbnailsGenerated { + library_id: lid, .. + } + | Event::FileOperationCompleted { + library_id: lid, .. + } + | Event::FilesModified { + library_id: lid, .. + } => *lid == library_id, + _ => false, + } + } +} diff --git a/core/src/library/manager.rs b/core/src/library/manager.rs index 04e62dd95..cd91f59ef 100644 --- a/core/src/library/manager.rs +++ b/core/src/library/manager.rs @@ -524,6 +524,36 @@ impl LibraryManager { Ok(()) } + + /// Delete a library + pub async fn delete_library(&self, id: Uuid, delete_data: bool) -> Result<()> { + let library = self + .get_library(id) + .await + .ok_or(LibraryError::NotFound(id.to_string()))?; + + //remove from library manager + let mut libraries = self.libraries.write().await; + libraries.remove(&id); + + let deleted_data_flag = if delete_data { + library.delete().await?; + true + } else { + false + }; + + // Emit event + self.event_bus.emit(Event::LibraryDeleted { + id, + name: library.name().await, + deleted_data: deleted_data_flag, + }); + + info!("Deleted library {}", id); + + Ok(()) + } } /// Check if a path is a library directory diff --git a/core/src/library/mod.rs b/core/src/library/mod.rs index 5f56ca3da..201e25de4 100644 --- a/core/src/library/mod.rs +++ b/core/src/library/mod.rs @@ -161,6 +161,17 @@ impl Library { Ok(()) } + /// Delete the library, including all data + pub async fn delete(&self) -> Result { + // Shutdown the library + self.shutdown().await?; + + // Delete the library directory + let deleted = tokio::fs::metadata(self.path()).await.is_err(); + + Ok(deleted) + } + /// Check if thumbnails exist for all specified sizes pub async fn has_all_thumbnails(&self, cas_id: &str, sizes: &[u32]) -> bool { for &size in sizes { diff --git a/core/src/ops/content/action.rs b/core/src/ops/content/action.rs deleted file mode 100644 index 08fbddd64..000000000 --- a/core/src/ops/content/action.rs +++ /dev/null @@ -1,75 +0,0 @@ -//! Content analysis action handler - -use crate::{ - context::CoreContext, - infra::{ - action::{ - error::ActionError, - LibraryAction, - }, - job::handle::JobHandle, - }, -}; -use async_trait::async_trait; -use std::sync::Arc; -use uuid::Uuid; - -#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] -pub struct ContentAction { - pub library_id: uuid::Uuid, - pub paths: Vec, - pub analyze_content: bool, - pub extract_metadata: bool, -} - -pub struct ContentHandler; - -impl ContentHandler { - pub fn new() -> Self { - Self - } -} - - - -// Add library_id to ContentAction -impl ContentAction { - /// Create a new content analysis action - pub fn new(library_id: uuid::Uuid, paths: Vec, analyze_content: bool, extract_metadata: bool) -> Self { - Self { - library_id, - paths, - analyze_content, - extract_metadata, - } - } -} - -// Implement the unified LibraryAction (replaces ActionHandler) -impl LibraryAction for ContentAction { - type Output = JobHandle; - - async fn execute(self, library: std::sync::Arc, context: Arc) -> Result { - // TODO: Implement content analysis job dispatch - Err(ActionError::Internal("ContentAnalysis action not yet implemented".to_string())) - } - - fn action_kind(&self) -> &'static str { - "content.analyze" - } - - fn library_id(&self) -> Uuid { - self.library_id - } - - async fn validate(&self, library: &std::sync::Arc, context: Arc) -> Result<(), ActionError> { - // Validate paths - if self.paths.is_empty() { - return Err(ActionError::Validation { - field: "paths".to_string(), - message: "At least one path must be specified".to_string(), - }); - } - Ok(()) - } -} \ No newline at end of file diff --git a/core/src/ops/content/mod.rs b/core/src/ops/content/mod.rs deleted file mode 100644 index 993438dd2..000000000 --- a/core/src/ops/content/mod.rs +++ /dev/null @@ -1,163 +0,0 @@ -//! Content operations for library-scoped content management - -pub mod action; - -use chrono::{DateTime, Utc}; -use sea_orm::{ColumnTrait, DatabaseConnection, EntityTrait, QueryFilter}; -use serde::{Deserialize, Serialize}; -use std::sync::Arc; -use uuid::Uuid; - -use crate::infra::db::entities::{ - content_identity::{self, Entity as ContentIdentity, Model as ContentIdentityModel}, - entry::{self, Entity as Entry, Model as EntryModel}, -}; -use crate::ops::indexing::PathResolver; - -pub use action::ContentAction; -use crate::common::errors::Result; - -#[derive(Clone, Debug, Serialize, Deserialize)] -pub struct ContentInstance { - pub entry_uuid: Option, - pub path: String, // TODO: Replace with SdPath when available - pub device_uuid: Uuid, - pub size: i64, - pub modified_at: Option>, -} - -#[derive(Clone, Debug, Serialize, Deserialize)] -pub struct LibraryContentStats { - pub entry_count: i32, - pub total_size: i64, // Size of one instance - pub combined_size: i64, // Calculated on-demand (entry_count * total_size) - pub integrity_hash: Option, - pub content_hash: String, - pub mime_type_id: Option, - pub kind_id: i32, - pub has_media_data: bool, - pub first_seen: DateTime, - pub last_verified: DateTime, -} - -pub struct ContentService { - library_db: Arc, -} - -impl ContentService { - pub fn new(library_db: Arc) -> Self { - Self { library_db } - } - - /// Find all instances of content within this library only - pub async fn find_content_instances( - &self, - content_identity_uuid: Uuid, - ) -> Result> { - // First find the content identity by UUID - let content_identity = ContentIdentity::find() - .filter(content_identity::Column::Uuid.eq(content_identity_uuid)) - .one(&*self.library_db) - .await? - .ok_or_else(|| { - crate::common::errors::CoreError::NotFound("Content identity not found".to_string()) - })?; - - // Find all entries that reference this content identity - let entries = Entry::find() - .filter(entry::Column::ContentId.eq(content_identity.id)) - .all(&*self.library_db) - .await?; - - let mut instances = Vec::new(); - for entry in entries { - // Get the full path using PathResolver - let path_buf = PathResolver::get_full_path(&*self.library_db, entry.id).await?; - let path = path_buf.to_string_lossy().to_string(); - - // TODO: Get device UUID from location when that relationship is available - let device_uuid = Uuid::new_v4(); // Placeholder - - instances.push(ContentInstance { - entry_uuid: entry.uuid, - path, - device_uuid, - size: entry.size, - modified_at: Some(entry.modified_at), - }); - } - - Ok(instances) - } - - /// Get content statistics within this library - pub async fn get_content_stats( - &self, - content_identity_uuid: Uuid, - ) -> Result { - let content_identity = ContentIdentity::find() - .filter(content_identity::Column::Uuid.eq(content_identity_uuid)) - .one(&*self.library_db) - .await? - .ok_or_else(|| { - crate::common::errors::CoreError::NotFound("Content identity not found".to_string()) - })?; - - Ok(LibraryContentStats { - entry_count: content_identity.entry_count, - total_size: content_identity.total_size, - combined_size: content_identity.combined_size(), - integrity_hash: content_identity.integrity_hash, - content_hash: content_identity.content_hash, - mime_type_id: content_identity.mime_type_id, - kind_id: content_identity.kind_id, - has_media_data: content_identity.media_data.is_some(), - first_seen: content_identity.first_seen_at, - last_verified: content_identity.last_verified_at, - }) - } - - /// Find content identity by content hash - pub async fn find_by_content_hash( - &self, - content_hash: &str, - ) -> Result> { - let content_identity = ContentIdentity::find() - .filter(content_identity::Column::ContentHash.eq(content_hash)) - .one(&*self.library_db) - .await?; - - Ok(content_identity) - } - - /// Get all content identities with entry counts above threshold - pub async fn find_duplicated_content( - &self, - min_instances: i32, - ) -> Result> { - let content_identities = ContentIdentity::find() - .filter(content_identity::Column::EntryCount.gte(min_instances)) - .all(&*self.library_db) - .await?; - - Ok(content_identities) - } - - /// Calculate total library deduplication savings - pub async fn calculate_deduplication_savings(&self) -> Result { - let content_identities = ContentIdentity::find() - .filter(content_identity::Column::EntryCount.gt(1)) // Only duplicated content - .all(&*self.library_db) - .await?; - - let total_savings: i64 = content_identities - .iter() - .map(|content| { - // Savings = (instances - 1) * size_per_instance - (content.entry_count - 1) as i64 * content.total_size - }) - .sum(); - - Ok(total_savings) - } -} diff --git a/core/src/ops/core/status/query.rs b/core/src/ops/core/status/query.rs index 7b3321ae9..ad8b36b27 100644 --- a/core/src/ops/core/status/query.rs +++ b/core/src/ops/core/status/query.rs @@ -1,7 +1,7 @@ //! Core status query (modular) use super::output::CoreStatus; -use crate::{context::CoreContext, cqrs::Query, register_query}; +use crate::{context::CoreContext, cqrs::Query}; use anyhow::Result; use std::sync::Arc; @@ -20,8 +20,4 @@ impl Query for CoreStatusQuery { } } -impl crate::client::Wire for CoreStatusQuery { - const METHOD: &'static str = "query:core.status.v1"; -} - -register_query!(CoreStatusQuery); +crate::register_query!(CoreStatusQuery, "core.status"); diff --git a/core/src/ops/devices/mod.rs b/core/src/ops/devices/mod.rs deleted file mode 100644 index e1cfd5e44..000000000 --- a/core/src/ops/devices/mod.rs +++ /dev/null @@ -1,3 +0,0 @@ -//! Device operations - -pub mod revoke; \ No newline at end of file diff --git a/core/src/ops/devices/revoke/action.rs b/core/src/ops/devices/revoke/action.rs deleted file mode 100644 index 9aa421429..000000000 --- a/core/src/ops/devices/revoke/action.rs +++ /dev/null @@ -1,82 +0,0 @@ -//! Device revoke action handler - -use crate::{ - context::CoreContext, - infra::action::{ - error::ActionError, - LibraryAction, - }, -}; -use serde::{Deserialize, Serialize}; -use std::sync::Arc; -use uuid::Uuid; - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct DeviceRevokeAction { - pub library_id: Uuid, - pub device_id: Uuid, - pub reason: Option, -} - -impl DeviceRevokeAction { - /// Create a new device revoke action - pub fn new(library_id: Uuid, device_id: Uuid, reason: Option) -> Self { - Self { - library_id, - device_id, - reason, - } - } -} - -pub struct DeviceRevokeHandler; - -impl DeviceRevokeHandler { - pub fn new() -> Self { - Self - } -} - -// Old ActionHandler implementation removed - using unified LibraryAction - -// Implement the unified LibraryAction (replaces ActionHandler) -impl LibraryAction for DeviceRevokeAction { - type Output = super::output::DeviceRevokeOutput; - - async fn execute(self, library: std::sync::Arc, context: Arc) -> Result { - // Library is pre-validated by ActionManager - - // TODO: Implement device revocation logic - let device_name = format!("Device {}", self.device_id); - - Ok(super::output::DeviceRevokeOutput { - device_id: self.device_id, - device_name, - reason: Some(self.reason.unwrap_or_else(|| "No reason provided".to_string())), - }) - } - - fn action_kind(&self) -> &'static str { - "device.revoke" - } - - fn library_id(&self) -> Uuid { - self.library_id - } - - async fn validate(&self, library: &std::sync::Arc, context: Arc) -> Result<(), ActionError> { - // Don't allow revoking self - let current_device = context.device_manager.to_device() - .map_err(|e| ActionError::Internal(format!("Failed to get current device: {}", e)))?; - - if current_device.id == self.device_id { - return Err(ActionError::Validation { - field: "device_id".to_string(), - message: "Cannot revoke current device".to_string(), - }); - } - Ok(()) - } -} - -// All old ActionHandler code removed \ No newline at end of file diff --git a/core/src/ops/devices/revoke/mod.rs b/core/src/ops/devices/revoke/mod.rs deleted file mode 100644 index 079e1753d..000000000 --- a/core/src/ops/devices/revoke/mod.rs +++ /dev/null @@ -1,4 +0,0 @@ -//! Device revoke operation - -pub mod action; -pub mod output; \ No newline at end of file diff --git a/core/src/ops/devices/revoke/output.rs b/core/src/ops/devices/revoke/output.rs deleted file mode 100644 index 9852ac56c..000000000 --- a/core/src/ops/devices/revoke/output.rs +++ /dev/null @@ -1,32 +0,0 @@ -//! Device revoke operation output - -use crate::infra::action::output::ActionOutputTrait; -use serde::{Deserialize, Serialize}; -use uuid::Uuid; - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct DeviceRevokeOutput { - pub device_id: Uuid, - pub device_name: String, - pub reason: Option, -} - -impl ActionOutputTrait for DeviceRevokeOutput { - fn to_json(&self) -> serde_json::Value { - serde_json::to_value(self).unwrap_or(serde_json::Value::Null) - } - - fn display_message(&self) -> String { - match &self.reason { - Some(reason) => format!( - "Revoked device '{}' ({}): {}", - self.device_name, self.device_id, reason - ), - None => format!("Revoked device '{}' ({})", self.device_name, self.device_id), - } - } - - fn output_type(&self) -> &'static str { - "device.revoke.output" - } -} diff --git a/core/src/ops/files/copy/action.rs b/core/src/ops/files/copy/action.rs index 360810f3c..45058bf31 100644 --- a/core/src/ops/files/copy/action.rs +++ b/core/src/ops/files/copy/action.rs @@ -3,7 +3,6 @@ use super::{ input::FileCopyInput, job::{CopyOptions, FileCopyJob}, - output::FileCopyActionOutput, }; use crate::{ context::CoreContext, @@ -11,27 +10,22 @@ use crate::{ infra::{ action::{ builder::{ActionBuildError, ActionBuilder}, - error::{ActionError, ActionResult}, + error::ActionError, LibraryAction, }, job::handle::JobHandle, }, }; -use async_trait::async_trait; -use clap::Parser; use serde::{Deserialize, Serialize}; use std::{path::PathBuf, sync::Arc}; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct FileCopyAction { - pub sources: Vec, + pub sources: SdPathBatch, pub destination: SdPath, pub options: CopyOptions, } -// Register with the inventory system -crate::op!(library_action FileCopyInput => FileCopyAction, "files.copy", via = FileCopyActionBuilder); - /// Builder for creating FileCopyAction instances with fluent API #[derive(Debug, Clone)] pub struct FileCopyActionBuilder { @@ -56,27 +50,29 @@ impl FileCopyActionBuilder { } } - /// Add multiple source files + /// Add multiple local source files pub fn sources(mut self, sources: I) -> Self where I: IntoIterator, P: Into, { - self.input - .sources - .extend(sources.into_iter().map(|p| p.into())); + let paths: Vec = sources + .into_iter() + .map(|p| SdPath::local(p.into())) + .collect(); + self.input.sources.extend(paths); self } - /// Add a single source file + /// Add a single local source file pub fn source>(mut self, source: P) -> Self { - self.input.sources.push(source.into()); + self.input.sources.paths.push(SdPath::local(source.into())); self } - /// Set the destination path + /// Set the local destination path pub fn destination>(mut self, dest: P) -> Self { - self.input.destination = dest.into(); + self.input.destination = SdPath::local(dest.into()); self } @@ -112,29 +108,35 @@ impl FileCopyActionBuilder { return; } - // Then do filesystem validation - for source in &self.input.sources { - if !source.exists() { - self.errors - .push(format!("Source file does not exist: {}", source.display())); - } else if source.is_dir() && !source.read_dir().is_ok() { - self.errors - .push(format!("Cannot read directory: {}", source.display())); - } else if source.is_file() && std::fs::metadata(source).is_err() { - self.errors - .push(format!("Cannot access file: {}", source.display())); + // Then do filesystem validation for local paths only + for source in &self.input.sources.paths { + if let Some(local_path) = source.as_local_path() { + if !local_path.exists() { + self.errors.push(format!( + "Source file does not exist: {}", + local_path.display() + )); + } else if local_path.is_dir() && std::fs::read_dir(local_path).is_err() { + self.errors + .push(format!("Cannot read directory: {}", local_path.display())); + } else if local_path.is_file() && std::fs::metadata(local_path).is_err() { + self.errors + .push(format!("Cannot access file: {}", local_path.display())); + } } } } /// Validate destination is valid fn validate_destination(&mut self) { - if let Some(parent) = self.input.destination.parent() { - if !parent.exists() { - self.errors.push(format!( - "Destination directory does not exist: {}", - parent.display() - )); + if let Some(dest_path) = self.input.destination.as_local_path() { + if let Some(parent) = dest_path.parent() { + if !parent.exists() { + self.errors.push(format!( + "Destination directory does not exist: {}", + parent.display() + )); + } } } } @@ -161,55 +163,9 @@ impl ActionBuilder for FileCopyActionBuilder { let options = self.input.to_copy_options(); - // Convert PathBuf to SdPath (local paths) - let sources = self - .input - .sources - .iter() - .map(|p| SdPath::local(p)) - .collect(); - let destination = SdPath::local(&self.input.destination); - Ok(FileCopyAction { - sources, - destination, - options, - }) - } -} - -impl FileCopyActionBuilder { - /// Create action directly from URI strings (for CLI/API use) - pub fn from_uris( - source_uris: Vec, - destination_uri: String, - options: CopyOptions, - ) -> Result { - // Parse source URIs to SdPaths - let mut sources = Vec::new(); - for uri in source_uris { - match SdPath::from_uri(&uri) { - Ok(path) => sources.push(path), - Err(e) => { - return Err(ActionBuildError::validation(format!( - "Invalid source URI '{}': {:?}", - uri, e - ))) - } - } - } - - // Parse destination URI - let destination = SdPath::from_uri(&destination_uri).map_err(|e| { - ActionBuildError::validation(format!( - "Invalid destination URI '{}': {:?}", - destination_uri, e - )) - })?; - - Ok(FileCopyAction { - sources, - destination, + sources: self.input.sources, + destination: self.input.destination, options, }) } @@ -245,24 +201,25 @@ impl FileCopyAction { } } -pub struct FileCopyHandler; - -impl FileCopyHandler { - pub fn new() -> Self { - Self - } -} +// Legacy handler removed; action is registered via the new action-centric registry impl LibraryAction for FileCopyAction { type Output = JobHandle; + type Input = FileCopyInput; + + fn from_input(input: Self::Input) -> Result { + use crate::infra::action::builder::ActionBuilder; + FileCopyActionBuilder::from_input(input) + .build() + .map_err(|e| e.to_string()) + } async fn execute( self, library: std::sync::Arc, - context: Arc, + _context: Arc, ) -> Result { - let job = FileCopyJob::new(SdPathBatch::new(self.sources), self.destination) - .with_options(self.options); + let job = FileCopyJob::new(self.sources, self.destination).with_options(self.options); let job_handle = library .jobs() @@ -274,15 +231,15 @@ impl LibraryAction for FileCopyAction { } fn action_kind(&self) -> &'static str { - "file.copy" + "files.copy" } async fn validate( &self, - library: &std::sync::Arc, - context: Arc, + _library: &std::sync::Arc, + _context: Arc, ) -> Result<(), ActionError> { - if self.sources.is_empty() { + if self.sources.paths.is_empty() { return Err(ActionError::Validation { field: "sources".to_string(), message: "At least one source file must be specified".to_string(), @@ -291,3 +248,6 @@ impl LibraryAction for FileCopyAction { Ok(()) } } + +// Register with the action-centric registry +crate::register_library_action!(FileCopyAction, "files.copy"); diff --git a/core/src/ops/files/copy/input.rs b/core/src/ops/files/copy/input.rs index 3824775be..6c7739194 100644 --- a/core/src/ops/files/copy/input.rs +++ b/core/src/ops/files/copy/input.rs @@ -2,6 +2,7 @@ use super::action::{FileCopyAction, FileCopyActionBuilder}; use super::job::CopyOptions; +use crate::domain::addressing::{SdPath, SdPathBatch}; use serde::{Deserialize, Serialize}; use std::path::PathBuf; @@ -26,11 +27,11 @@ impl Default for CopyMethod { /// This is the canonical interface that all external APIs (CLI, GraphQL, REST) convert to #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct FileCopyInput { - /// Source files or directories to copy - pub sources: Vec, + /// Source files or directories to copy (domain addressing) + pub sources: SdPathBatch, - /// Destination path - pub destination: PathBuf, + /// Destination path (domain addressing) + pub destination: SdPath, /// Whether to overwrite existing files pub overwrite: bool, @@ -49,11 +50,12 @@ pub struct FileCopyInput { } impl FileCopyInput { - /// Create a new FileCopyInput with default options + /// Create a new FileCopyInput with default options from local filesystem paths pub fn new>(sources: Vec, destination: D) -> Self { + let paths = sources.into_iter().map(|p| SdPath::local(p)).collect(); Self { - sources, - destination: destination.into(), + sources: SdPathBatch { paths }, + destination: SdPath::local(destination.into()), overwrite: false, verify_checksum: false, preserve_timestamps: true, @@ -113,22 +115,10 @@ impl FileCopyInput { pub fn validate(&self) -> Result<(), Vec> { let mut errors = Vec::new(); - if self.sources.is_empty() { + if self.sources.paths.is_empty() { errors.push("At least one source file must be specified".to_string()); } - // Validate each source path (basic validation - existence check done in builder) - for source in &self.sources { - if source.as_os_str().is_empty() { - errors.push("Source path cannot be empty".to_string()); - } - } - - // Validate destination - if self.destination.as_os_str().is_empty() { - errors.push("Destination path cannot be empty".to_string()); - } - if errors.is_empty() { Ok(()) } else { @@ -139,27 +129,22 @@ impl FileCopyInput { /// Get a summary string for logging/display pub fn summary(&self) -> String { let operation = if self.move_files { "Move" } else { "Copy" }; - let source_count = self.sources.len(); + let source_count = self.sources.paths.len(); let source_desc = if source_count == 1 { "1 source".to_string() } else { format!("{} sources", source_count) }; - format!( - "{} {} to {}", - operation, - source_desc, - self.destination.display() - ) + format!("{} {} to {:?}", operation, source_desc, self.destination,) } } impl Default for FileCopyInput { fn default() -> Self { Self { - sources: Vec::new(), - destination: PathBuf::new(), + sources: SdPathBatch { paths: Vec::new() }, + destination: SdPath::local(PathBuf::new()), overwrite: false, verify_checksum: false, preserve_timestamps: true, @@ -177,8 +162,7 @@ mod tests { fn test_new_input() { let input = FileCopyInput::new(vec!["/file1.txt".into(), "/file2.txt".into()], "/dest/"); - assert_eq!(input.sources.len(), 2); - assert_eq!(input.destination, PathBuf::from("/dest/")); + assert_eq!(input.sources.paths.len(), 2); assert!(!input.overwrite); assert!(input.preserve_timestamps); assert!(!input.move_files); @@ -188,8 +172,7 @@ mod tests { fn test_single_file() { let input = FileCopyInput::single_file("/source.txt", "/dest.txt"); - assert_eq!(input.sources, vec![PathBuf::from("/source.txt")]); - assert_eq!(input.destination, PathBuf::from("/dest.txt")); + assert_eq!(input.sources.paths.len(), 1); } #[test] @@ -216,49 +199,9 @@ mod tests { assert!(errors.iter().any(|e| e.contains("At least one source"))); } - #[test] - fn test_validation_empty_destination() { - let mut input = FileCopyInput::default(); - input.sources = vec!["/file.txt".into()]; - - let result = input.validate(); - assert!(result.is_err()); - let errors = result.unwrap_err(); - assert!(errors - .iter() - .any(|e| e.contains("Destination path cannot be empty"))); - } - #[test] fn test_validation_success() { let input = FileCopyInput::new(vec!["/file.txt".into()], "/dest/"); assert!(input.validate().is_ok()); } - - #[test] - fn test_summary() { - let input = FileCopyInput::new(vec!["/file1.txt".into(), "/file2.txt".into()], "/dest/"); - assert_eq!(input.summary(), "Copy 2 sources to /dest/"); - - let move_input = input.with_move(true); - assert_eq!(move_input.summary(), "Move 2 sources to /dest/"); - - let single_input = FileCopyInput::single_file("/file.txt", "/dest.txt"); - assert_eq!(single_input.summary(), "Copy 1 source to /dest.txt"); - } - - #[test] - fn test_to_copy_options() { - let input = FileCopyInput::single_file("/source.txt", "/dest.txt") - .with_overwrite(true) - .with_verification(true) - .with_timestamp_preservation(false) - .with_move(true); - - let options = input.to_copy_options(); - assert!(options.overwrite); - assert!(options.verify_checksum); - assert!(!options.preserve_timestamps); - assert!(options.delete_after_copy); - } } diff --git a/core/src/ops/files/delete/action.rs b/core/src/ops/files/delete/action.rs index be4f645bf..ef3b3d65d 100644 --- a/core/src/ops/files/delete/action.rs +++ b/core/src/ops/files/delete/action.rs @@ -1,7 +1,7 @@ //! File delete action handler +use super::input::FileDeleteInput; use super::job::{DeleteJob, DeleteMode, DeleteOptions}; -use super::output::FileDeleteOutput; use crate::{ context::CoreContext, domain::addressing::{SdPath, SdPathBatch}, @@ -10,50 +10,54 @@ use crate::{ job::handle::JobHandle, }, }; -use async_trait::async_trait; use serde::{Deserialize, Serialize}; use std::{path::PathBuf, sync::Arc}; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct FileDeleteAction { - pub targets: Vec, + pub targets: SdPathBatch, pub options: DeleteOptions, } impl FileDeleteAction { /// Create a new file delete action - pub fn new(targets: Vec, options: DeleteOptions) -> Self { + pub fn new(targets: SdPathBatch, options: DeleteOptions) -> Self { Self { targets, options } } /// Create a delete action with default options - pub fn with_defaults(targets: Vec) -> Self { + pub fn with_defaults(targets: SdPathBatch) -> Self { Self::new(targets, DeleteOptions::default()) } } // Implement the unified LibraryAction impl LibraryAction for FileDeleteAction { + type Input = FileDeleteInput; type Output = JobHandle; + fn from_input(input: Self::Input) -> Result { + Ok(FileDeleteAction { + targets: input.targets, + options: DeleteOptions { + permanent: input.permanent, + recursive: input.recursive, + }, + }) + } + async fn execute( self, library: std::sync::Arc, context: Arc, ) -> Result { - let targets = self - .targets - .into_iter() - .map(|path| SdPath::local(path)) - .collect(); - let mode = if self.options.permanent { DeleteMode::Permanent } else { DeleteMode::Trash }; - let job = DeleteJob::new(SdPathBatch::new(targets), mode); + let job = DeleteJob::new(self.targets, mode); let job_handle = library .jobs() @@ -74,7 +78,7 @@ impl LibraryAction for FileDeleteAction { context: Arc, ) -> Result<(), ActionError> { // Validate targets - if self.targets.is_empty() { + if self.targets.paths.is_empty() { return Err(ActionError::Validation { field: "targets".to_string(), message: "At least one target file must be specified".to_string(), @@ -84,3 +88,6 @@ impl LibraryAction for FileDeleteAction { Ok(()) } } + +// Register this action with the new registry +crate::register_library_action!(FileDeleteAction, "files.delete"); diff --git a/core/src/ops/files/delete/input.rs b/core/src/ops/files/delete/input.rs index 6f172eebd..368421f4a 100644 --- a/core/src/ops/files/delete/input.rs +++ b/core/src/ops/files/delete/input.rs @@ -1,14 +1,14 @@ //! Input types for file deletion operations use super::action::FileDeleteAction; +use crate::domain::SdPathBatch; use serde::{Deserialize, Serialize}; -use std::path::PathBuf; /// Input for deleting files #[derive(Debug, Clone, Serialize, Deserialize)] pub struct FileDeleteInput { /// Files or directories to delete - pub targets: Vec, + pub targets: SdPathBatch, /// Whether to permanently delete (true) or move to trash (false) pub permanent: bool, @@ -17,24 +17,9 @@ pub struct FileDeleteInput { pub recursive: bool, } -impl TryFrom for FileDeleteAction { - type Error = String; - fn try_from(input: FileDeleteInput) -> Result { - Ok(FileDeleteAction { - targets: input.targets, - options: crate::ops::files::delete::job::DeleteOptions { - permanent: input.permanent, - recursive: input.recursive, - }, - }) - } -} - -crate::op!(library_action FileDeleteInput => FileDeleteAction, "files.delete"); - impl FileDeleteInput { /// Create a new file deletion input - pub fn new(targets: Vec) -> Self { + pub fn new(targets: SdPathBatch) -> Self { Self { targets, permanent: false, @@ -58,7 +43,7 @@ impl FileDeleteInput { pub fn validate(&self) -> Result<(), Vec> { let mut errors = Vec::new(); - if self.targets.is_empty() { + if self.targets.paths.is_empty() { errors.push("At least one target file must be specified".to_string()); } diff --git a/core/src/ops/files/duplicate_detection/action.rs b/core/src/ops/files/duplicate_detection/action.rs index 0b250dca4..88d77e2fb 100644 --- a/core/src/ops/files/duplicate_detection/action.rs +++ b/core/src/ops/files/duplicate_detection/action.rs @@ -11,18 +11,19 @@ use crate::{ }, }; use async_trait::async_trait; +use serde::{Deserialize, Serialize}; use std::sync::Arc; #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct DuplicateDetectionAction { - pub paths: Vec, + pub paths: SdPathBatch, pub algorithm: String, pub threshold: f64, } impl DuplicateDetectionAction { /// Create a new duplicate detection action - pub fn new(paths: Vec, algorithm: String, threshold: f64) -> Self { + pub fn new(paths: SdPathBatch, algorithm: String, threshold: f64) -> Self { Self { paths, algorithm, @@ -31,12 +32,6 @@ impl DuplicateDetectionAction { } } -impl From for DuplicateDetectionAction { - fn from(i: DuplicateDetectionInput) -> Self { - Self::new(i.paths, i.algorithm, i.threshold) - } -} - pub struct DuplicateDetectionHandler; impl DuplicateDetectionHandler { @@ -47,14 +42,28 @@ impl DuplicateDetectionHandler { // Implement the unified LibraryAction (replaces ActionHandler) impl LibraryAction for DuplicateDetectionAction { + type Input = DuplicateDetectionInput; type Output = JobHandle; + fn from_input(i: Self::Input) -> Result { + let sd_paths = i + .paths + .into_iter() + .map(|p| SdPath::local(p)) + .collect::>(); + Ok(DuplicateDetectionAction { + paths: SdPathBatch { paths: sd_paths }, + algorithm: i.algorithm, + threshold: i.threshold, + }) + } + async fn execute( self, library: std::sync::Arc, context: Arc, ) -> Result { - // Library is pre-validated by ActionManager - no boilerplate! + // Library is pre-validated by ActionManager // Create duplicate detection job let mode = match self.algorithm.as_str() { @@ -64,16 +73,7 @@ impl LibraryAction for DuplicateDetectionAction { _ => DetectionMode::ContentHash, }; - let search_paths = self - .paths - .into_iter() - .map(|p| crate::domain::addressing::SdPath::local(p)) - .collect::>(); - let search_paths = crate::domain::addressing::SdPathBatch { - paths: search_paths, - }; - - let job = DuplicateDetectionJob::new(search_paths, mode); + let job = DuplicateDetectionJob::new(self.paths, mode); // Dispatch job and return handle directly let job_handle = library @@ -94,14 +94,15 @@ impl LibraryAction for DuplicateDetectionAction { library: &std::sync::Arc, context: Arc, ) -> Result<(), ActionError> { - // Validate paths - if self.paths.is_empty() { + if self.paths.paths.is_empty() { return Err(ActionError::Validation { field: "paths".to_string(), message: "At least one path must be specified".to_string(), }); } - Ok(()) } } + +// Register with the registry +crate::register_library_action!(DuplicateDetectionAction, "files.duplicate_detection"); diff --git a/core/src/ops/files/duplicate_detection/input.rs b/core/src/ops/files/duplicate_detection/input.rs index b8a03bb4b..fbab54d34 100644 --- a/core/src/ops/files/duplicate_detection/input.rs +++ b/core/src/ops/files/duplicate_detection/input.rs @@ -4,8 +4,6 @@ use super::action::DuplicateDetectionAction; use serde::{Deserialize, Serialize}; use std::path::PathBuf; -use crate::op; - /// Input for file duplicate detection operations #[derive(Debug, Clone, Serialize, Deserialize)] pub struct DuplicateDetectionInput { @@ -16,5 +14,3 @@ pub struct DuplicateDetectionInput { /// Similarity threshold (0.0 to 1.0) pub threshold: f64, } - -op!(library_action DuplicateDetectionInput => DuplicateDetectionAction, "files.duplicate_detection"); diff --git a/core/src/ops/files/validation/action.rs b/core/src/ops/files/validation/action.rs index bb521c91f..71d976961 100644 --- a/core/src/ops/files/validation/action.rs +++ b/core/src/ops/files/validation/action.rs @@ -3,25 +3,27 @@ use super::job::{ValidationJob, ValidationMode}; use crate::{ context::CoreContext, + domain::addressing::{SdPath, SdPathBatch}, infra::{ action::{error::ActionError, LibraryAction}, job::handle::JobHandle, }, + ops::files::FileValidationInput, }; use std::sync::Arc; #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct ValidationAction { - pub paths: Vec, + pub targets: SdPathBatch, pub verify_checksums: bool, pub deep_scan: bool, } impl ValidationAction { /// Create a new file validation action - pub fn new(paths: Vec, verify_checksums: bool, deep_scan: bool) -> Self { + pub fn new(targets: SdPathBatch, verify_checksums: bool, deep_scan: bool) -> Self { Self { - paths, + targets, verify_checksums, deep_scan, } @@ -30,8 +32,22 @@ impl ValidationAction { // Implement the unified LibraryAction (replaces ActionHandler) impl LibraryAction for ValidationAction { + type Input = FileValidationInput; type Output = JobHandle; + fn from_input(input: Self::Input) -> Result { + let paths = input + .paths + .into_iter() + .map(|p| SdPath::local(p)) + .collect::>(); + Ok(ValidationAction { + targets: SdPathBatch { paths }, + verify_checksums: input.verify_checksums, + deep_scan: input.deep_scan, + }) + } + async fn execute( self, library: std::sync::Arc, @@ -46,15 +62,7 @@ impl LibraryAction for ValidationAction { ValidationMode::Basic }; - // Convert paths into SdPathBatch - let targets = self - .paths - .into_iter() - .map(|p| crate::domain::addressing::SdPath::local(p)) - .collect::>(); - let targets = crate::domain::addressing::SdPathBatch { paths: targets }; - - let job = ValidationJob::new(targets, mode); + let job = ValidationJob::new(self.targets, mode); // Dispatch job and return handle directly let job_handle = library @@ -76,7 +84,7 @@ impl LibraryAction for ValidationAction { context: Arc, ) -> Result<(), ActionError> { // Validate paths - if self.paths.is_empty() { + if self.targets.paths.is_empty() { return Err(ActionError::Validation { field: "paths".to_string(), message: "At least one path must be specified".to_string(), @@ -86,3 +94,6 @@ impl LibraryAction for ValidationAction { Ok(()) } } + +// Register this action with the new registry +crate::register_library_action!(ValidationAction, "files.validation"); diff --git a/core/src/ops/files/validation/input.rs b/core/src/ops/files/validation/input.rs index 805d22062..5fce22a02 100644 --- a/core/src/ops/files/validation/input.rs +++ b/core/src/ops/files/validation/input.rs @@ -1,6 +1,5 @@ //! File validation input for external API -use crate::register_library_action_input; use serde::{Deserialize, Serialize}; use std::path::PathBuf; @@ -14,23 +13,3 @@ pub struct FileValidationInput { /// Whether to perform deep scanning pub deep_scan: bool, } - -impl crate::client::Wire for FileValidationInput { - const METHOD: &'static str = "action:files.validation.input.v1"; -} - -impl crate::ops::registry::BuildLibraryActionInput for FileValidationInput { - type Action = crate::ops::files::validation::action::ValidationAction; - - fn build(self) -> Result { - Ok( - crate::ops::files::validation::action::ValidationAction::new( - self.paths, - self.verify_checksums, - self.deep_scan, - ), - ) - } -} - -register_library_action_input!(FileValidationInput); diff --git a/core/src/ops/indexing/action.rs b/core/src/ops/indexing/action.rs index 62ff278ef..c5448ae65 100644 --- a/core/src/ops/indexing/action.rs +++ b/core/src/ops/indexing/action.rs @@ -59,8 +59,13 @@ impl IndexingHandler { } impl LibraryAction for IndexingAction { + type Input = IndexInput; type Output = JobHandle; + fn from_input(input: IndexInput) -> Result { + Ok(IndexingAction::new(input)) + } + async fn execute( self, library: std::sync::Arc, @@ -135,3 +140,5 @@ impl LibraryAction for IndexingAction { Ok(()) } } + +crate::register_library_action!(IndexingAction, "indexing.start"); diff --git a/core/src/ops/indexing/input.rs b/core/src/ops/indexing/input.rs index 3f795562b..c83fef9a3 100644 --- a/core/src/ops/indexing/input.rs +++ b/core/src/ops/indexing/input.rs @@ -1,7 +1,6 @@ //! Core input types for indexing operations use super::job::{IndexMode, IndexPersistence, IndexScope}; -use crate::register_library_action_input; use serde::{Deserialize, Serialize}; use std::path::PathBuf; @@ -27,23 +26,6 @@ pub struct IndexInput { pub persistence: IndexPersistence, } -impl crate::client::Wire for IndexInput { - const METHOD: &'static str = "action:indexing.start.input.v1"; -} - -impl crate::ops::registry::BuildLibraryActionInput for IndexInput { - type Action = crate::ops::indexing::action::IndexingAction; - - fn build(self) -> Result { - use crate::infra::action::builder::ActionBuilder; - crate::ops::indexing::action::IndexingActionBuilder::from_input(self) - .build() - .map_err(|e| e.to_string()) - } -} - -register_library_action_input!(IndexInput); - impl IndexInput { /// Create a new input with sane defaults pub fn new>(library_id: uuid::Uuid, paths: P) -> Self { diff --git a/core/src/ops/libraries/create/action.rs b/core/src/ops/libraries/create/action.rs index 3704d2315..579423c85 100644 --- a/core/src/ops/libraries/create/action.rs +++ b/core/src/ops/libraries/create/action.rs @@ -1,6 +1,6 @@ //! Library creation action handler -use super::output::LibraryCreateOutput; +use super::{input::LibraryCreateInput, output::LibraryCreateOutput}; use crate::{ context::CoreContext, infra::action::{error::ActionError, CoreAction}, @@ -12,18 +12,32 @@ use uuid::Uuid; #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct LibraryCreateAction { - pub name: String, - pub path: Option, + input: LibraryCreateInput, +} + +impl LibraryCreateAction { + pub fn new(input: LibraryCreateInput) -> Self { + Self { input } + } } // Implement the new modular ActionType trait impl CoreAction for LibraryCreateAction { + type Input = LibraryCreateInput; type Output = LibraryCreateOutput; + fn from_input(input: LibraryCreateInput) -> Result { + Ok(LibraryCreateAction::new(input)) + } + async fn execute(self, context: Arc) -> Result { let library_manager = &context.library_manager; let library = library_manager - .create_library(self.name.clone(), self.path.clone(), context.clone()) + .create_library( + self.input.name.clone(), + self.input.path.clone(), + context.clone(), + ) .await?; Ok(LibraryCreateOutput::new( @@ -38,7 +52,7 @@ impl CoreAction for LibraryCreateAction { } async fn validate(&self, _context: Arc) -> Result<(), ActionError> { - if self.name.trim().is_empty() { + if self.input.name.trim().is_empty() { return Err(ActionError::Validation { field: "name".to_string(), message: "Library name cannot be empty".to_string(), diff --git a/core/src/ops/libraries/create/input.rs b/core/src/ops/libraries/create/input.rs index cac7beb1e..17bed4f6a 100644 --- a/core/src/ops/libraries/create/input.rs +++ b/core/src/ops/libraries/create/input.rs @@ -1,6 +1,5 @@ //! Input types for library creation operations -use crate::register_core_action_input; use serde::{Deserialize, Serialize}; use std::path::PathBuf; @@ -14,26 +13,6 @@ pub struct LibraryCreateInput { pub path: Option, } -impl crate::client::Wire for LibraryCreateInput { - const METHOD: &'static str = "action:libraries.create.input.v1"; -} - -impl crate::ops::registry::BuildCoreActionInput for LibraryCreateInput { - type Action = crate::ops::libraries::create::action::LibraryCreateAction; - - fn build( - self, - _session: &crate::infra::daemon::state::SessionState, - ) -> Result { - Ok(crate::ops::libraries::create::action::LibraryCreateAction { - name: self.name, - path: self.path, - }) - } -} - -register_core_action_input!(LibraryCreateInput); - impl LibraryCreateInput { /// Create a new library creation input pub fn new(name: String) -> Self { diff --git a/core/src/ops/libraries/delete/action.rs b/core/src/ops/libraries/delete/action.rs index 1b153dd34..f1c804286 100644 --- a/core/src/ops/libraries/delete/action.rs +++ b/core/src/ops/libraries/delete/action.rs @@ -3,10 +3,8 @@ use super::output::LibraryDeleteOutput; use crate::{ context::CoreContext, - infra::action::{ - error::ActionError, - CoreAction, - }, + infra::action::{error::ActionError, CoreAction}, + ops::libraries::LibraryDeleteInput, }; use async_trait::async_trait; use serde::{Deserialize, Serialize}; @@ -15,39 +13,57 @@ use uuid::Uuid; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct LibraryDeleteAction { - pub library_id: Uuid, + input: LibraryDeleteInput, } -pub struct LibraryDeleteHandler; - -impl LibraryDeleteHandler { - pub fn new() -> Self { - Self +impl LibraryDeleteAction { + pub fn new(input: LibraryDeleteInput) -> Self { + Self { input } } } -// Note: ActionHandler implementation removed - using ActionType instead +pub struct LibraryDeleteHandler { + input: LibraryDeleteInput, +} + +impl LibraryDeleteHandler { + pub fn new(input: LibraryDeleteInput) -> Self { + Self { input } + } +} -// Implement the new modular ActionType trait impl CoreAction for LibraryDeleteAction { + type Input = LibraryDeleteInput; type Output = LibraryDeleteOutput; - async fn execute(self, context: std::sync::Arc) -> Result { + fn from_input(input: LibraryDeleteInput) -> Result { + Ok(LibraryDeleteAction::new(input)) + } + + async fn execute( + self, + context: std::sync::Arc, + ) -> Result { // Get the library to get its name before deletion let library = context .library_manager - .get_library(self.library_id) + .get_library(self.input.library_id) .await - .ok_or_else(|| ActionError::LibraryNotFound(self.library_id))?; + .ok_or_else(|| ActionError::LibraryNotFound(self.input.library_id))?; let library_name = library.name().await; // Delete the library through the library manager - // TODO: Implement actual deletion - for now just return success - // context.library_manager.delete_library(self.library_id).await?; + context + .library_manager + .delete_library(self.input.library_id, self.input.delete_data) + .await?; // Return native output directly - Ok(LibraryDeleteOutput::new(self.library_id, library_name)) + Ok(LibraryDeleteOutput::new( + self.input.library_id, + library_name, + )) } fn action_kind(&self) -> &'static str { diff --git a/core/src/ops/libraries/delete/input.rs b/core/src/ops/libraries/delete/input.rs index ef2817b59..eeac1b5c7 100644 --- a/core/src/ops/libraries/delete/input.rs +++ b/core/src/ops/libraries/delete/input.rs @@ -1,6 +1,5 @@ //! Input types for library deletion operations -use crate::register_core_action_input; use serde::{Deserialize, Serialize}; use uuid::Uuid; @@ -14,31 +13,12 @@ pub struct LibraryDeleteInput { pub delete_data: bool, } -impl crate::client::Wire for LibraryDeleteInput { - const METHOD: &'static str = "action:libraries.delete.input.v1"; -} - -impl crate::ops::registry::BuildCoreActionInput for LibraryDeleteInput { - type Action = crate::ops::libraries::delete::action::LibraryDeleteAction; - - fn build( - self, - _session: &crate::infra::daemon::state::SessionState, - ) -> Result { - Ok(crate::ops::libraries::delete::action::LibraryDeleteAction { - library_id: self.library_id, - }) - } -} - -register_core_action_input!(LibraryDeleteInput); - impl LibraryDeleteInput { /// Create a new library deletion input pub fn new(library_id: Uuid) -> Self { Self { library_id, - delete_data: false, + delete_data: true, } } diff --git a/core/src/ops/libraries/export/action.rs b/core/src/ops/libraries/export/action.rs index 0a25c6fec..d46651f1a 100644 --- a/core/src/ops/libraries/export/action.rs +++ b/core/src/ops/libraries/export/action.rs @@ -1,49 +1,41 @@ //! Library export action handler +use super::input::LibraryExportInput; use crate::{ context::CoreContext, infra::action::{error::ActionError, LibraryAction}, }; use serde::{Deserialize, Serialize}; -use std::{path::PathBuf, sync::Arc}; -use uuid::Uuid; +use std::sync::Arc; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct LibraryExportAction { - pub library_id: Uuid, - pub export_path: PathBuf, - pub include_thumbnails: bool, - pub include_previews: bool, + input: LibraryExportInput, } impl LibraryExportAction { /// Create a new library export action - pub fn new( - library_id: Uuid, - export_path: PathBuf, - include_thumbnails: bool, - include_previews: bool, - ) -> Self { - Self { - library_id, - export_path, - include_thumbnails, - include_previews, - } + pub fn new(input: LibraryExportInput) -> Self { + Self { input } } } // Implement LibraryAction impl LibraryAction for LibraryExportAction { + type Input = LibraryExportInput; type Output = super::output::LibraryExportOutput; + fn from_input(input: LibraryExportInput) -> Result { + Ok(LibraryExportAction::new(input)) + } + async fn execute( self, library: std::sync::Arc, _context: Arc, ) -> Result { // Ensure parent directory exists - if let Some(parent) = self.export_path.parent() { + if let Some(parent) = self.input.export_path.parent() { if !parent.exists() { return Err(ActionError::Validation { field: "export_path".to_string(), @@ -53,7 +45,7 @@ impl LibraryAction for LibraryExportAction { } // Create export directory - let export_dir = &self.export_path; + let export_dir = &self.input.export_path; tokio::fs::create_dir_all(&export_dir).await.map_err(|e| { ActionError::Internal(format!("Failed to create export directory: {}", e)) })?; @@ -82,7 +74,7 @@ impl LibraryAction for LibraryExportAction { ]; // Optionally export thumbnails - if self.include_thumbnails { + if self.input.include_thumbnails { let thumbnails_src = library.path().join("thumbnails"); if thumbnails_src.exists() { // TODO: Copy thumbnails directory @@ -91,7 +83,7 @@ impl LibraryAction for LibraryExportAction { } // Optionally export previews - if self.include_previews { + if self.input.include_previews { let previews_src = library.path().join("previews"); if previews_src.exists() { // TODO: Copy previews directory @@ -102,7 +94,7 @@ impl LibraryAction for LibraryExportAction { Ok(super::output::LibraryExportOutput { library_id: library.id(), library_name: config.name.clone(), - export_path: self.export_path, + export_path: self.input.export_path, exported_files, }) } diff --git a/core/src/ops/libraries/export/input.rs b/core/src/ops/libraries/export/input.rs new file mode 100644 index 000000000..ec58fb3f6 --- /dev/null +++ b/core/src/ops/libraries/export/input.rs @@ -0,0 +1,14 @@ +//! Input types for library export operations + +use serde::{Deserialize, Serialize}; +use std::path::PathBuf; +use uuid::Uuid; + +/// Input for exporting a library +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct LibraryExportInput { + pub library_id: Uuid, + pub export_path: PathBuf, + pub include_thumbnails: bool, + pub include_previews: bool, +} diff --git a/core/src/ops/libraries/export/mod.rs b/core/src/ops/libraries/export/mod.rs index 65b7c87bf..51c1c2af8 100644 --- a/core/src/ops/libraries/export/mod.rs +++ b/core/src/ops/libraries/export/mod.rs @@ -1,4 +1,5 @@ //! Library export operation pub mod action; -pub mod output; \ No newline at end of file +pub mod input; +pub mod output; diff --git a/core/src/ops/libraries/list/query.rs b/core/src/ops/libraries/list/query.rs index 4f091d1c7..2d2da4dd8 100644 --- a/core/src/ops/libraries/list/query.rs +++ b/core/src/ops/libraries/list/query.rs @@ -1,7 +1,7 @@ //! Library listing query implementation use super::output::LibraryInfo; -use crate::{context::CoreContext, cqrs::Query, register_query}; +use crate::{context::CoreContext, cqrs::Query}; use anyhow::Result; use serde::{Deserialize, Serialize}; use std::sync::Arc; @@ -64,8 +64,4 @@ impl Query for ListLibrariesQuery { } } -impl crate::client::Wire for ListLibrariesQuery { - const METHOD: &'static str = "query:libraries.list.v1"; -} - -register_query!(ListLibrariesQuery); +crate::register_query!(ListLibrariesQuery, "libraries.list"); diff --git a/core/src/ops/libraries/rename/action.rs b/core/src/ops/libraries/rename/action.rs index 1a40ddc9c..36ae2c469 100644 --- a/core/src/ops/libraries/rename/action.rs +++ b/core/src/ops/libraries/rename/action.rs @@ -12,18 +12,20 @@ use std::sync::Arc; use uuid::Uuid; #[derive(Debug, Clone, Serialize, Deserialize)] -pub struct LibraryRenameAction { +pub struct LibraryRenameInput { pub library_id: Uuid, pub new_name: String, } +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct LibraryRenameAction { + input: LibraryRenameInput, +} + impl LibraryRenameAction { /// Create a new library rename action - pub fn new(library_id: Uuid, new_name: String) -> Self { - Self { - library_id, - new_name, - } + pub fn new(input: LibraryRenameInput) -> Self { + Self { input } } } @@ -31,8 +33,13 @@ impl LibraryRenameAction { // Implement the new modular ActionType trait impl LibraryAction for LibraryRenameAction { + type Input = LibraryRenameInput; type Output = LibraryRenameOutput; + fn from_input(input: LibraryRenameInput) -> Result { + Ok(LibraryRenameAction::new(input)) + } + async fn execute( self, library: std::sync::Arc, @@ -47,16 +54,16 @@ impl LibraryAction for LibraryRenameAction { // Update the library name using update_config library .update_config(|config| { - config.name = self.new_name.clone(); + config.name = self.input.new_name.clone(); }) .await .map_err(|e| ActionError::Internal(format!("Failed to save config: {}", e)))?; // Return native output directly Ok(LibraryRenameOutput { - library_id: self.library_id, + library_id: self.input.library_id, old_name, - new_name: self.new_name, + new_name: self.input.new_name, }) } @@ -72,7 +79,7 @@ impl LibraryAction for LibraryRenameAction { // Library existence already validated by ActionManager - no boilerplate! // Validate new name - if self.new_name.trim().is_empty() { + if self.input.new_name.trim().is_empty() { return Err(ActionError::Validation { field: "new_name".to_string(), message: "Library name cannot be empty".to_string(), diff --git a/core/src/ops/locations/add/action.rs b/core/src/ops/locations/add/action.rs index f8ff95d97..83bacc895 100644 --- a/core/src/ops/locations/add/action.rs +++ b/core/src/ops/locations/add/action.rs @@ -10,7 +10,6 @@ use crate::{ infra::db::entities, location::manager::LocationManager, ops::indexing::IndexMode, - // register_action_handler removed - using unified LibraryAction dispatch }; use async_trait::async_trait; use sea_orm::{ColumnTrait, EntityTrait, QueryFilter}; @@ -19,34 +18,38 @@ use std::{path::PathBuf, sync::Arc}; use uuid::Uuid; #[derive(Debug, Clone, Serialize, Deserialize)] -pub struct LocationAddAction { +pub struct LocationAddInput { pub library_id: Uuid, pub path: PathBuf, pub name: Option, pub mode: IndexMode, } -pub struct LocationAddHandler; +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct LocationAddAction { + input: LocationAddInput, +} -impl LocationAddHandler { - pub fn new() -> Self { - Self +impl LocationAddAction { + pub fn new(input: LocationAddInput) -> Self { + Self { input } } } -// Note: ActionHandler implementation removed - using ActionType instead - // Implement the new modular ActionType trait impl LibraryAction for LocationAddAction { + type Input = LocationAddInput; type Output = LocationAddOutput; + fn from_input(input: LocationAddInput) -> Result { + Ok(LocationAddAction::new(input)) + } + async fn execute( self, library: std::sync::Arc, context: std::sync::Arc, ) -> Result { - // Library is pre-validated by ActionManager - no boilerplate! - // Get the device UUID from the device manager let device_uuid = context .device_manager @@ -65,7 +68,7 @@ impl LibraryAction for LocationAddAction { // Add the location using LocationManager let location_manager = LocationManager::new(context.events.as_ref().clone()); - let location_mode = match self.mode { + let location_mode = match self.input.mode { IndexMode::Shallow => crate::location::IndexMode::Shallow, IndexMode::Content => crate::location::IndexMode::Content, IndexMode::Deep => crate::location::IndexMode::Deep, @@ -74,8 +77,8 @@ impl LibraryAction for LocationAddAction { let (location_id, job_id_string) = location_manager .add_location( library.clone(), - self.path.clone(), - self.name.clone(), + self.input.path.clone(), + self.input.name.clone(), device_record.id, location_mode, ) @@ -92,8 +95,11 @@ impl LibraryAction for LocationAddAction { None }; - // Return native output directly - Ok(LocationAddOutput::new(location_id, self.path, self.name)) + Ok(LocationAddOutput::new( + location_id, + self.input.path, + self.input.name, + )) } fn action_kind(&self) -> &'static str { @@ -105,13 +111,13 @@ impl LibraryAction for LocationAddAction { library: &std::sync::Arc, context: std::sync::Arc, ) -> Result<(), ActionError> { - if !self.path.exists() { + if !self.input.path.exists() { return Err(ActionError::Validation { field: "path".to_string(), message: "Path does not exist".to_string(), }); } - if !self.path.is_dir() { + if !self.input.path.is_dir() { return Err(ActionError::Validation { field: "path".to_string(), message: "Path must be a directory".to_string(), diff --git a/core/src/ops/locations/remove/action.rs b/core/src/ops/locations/remove/action.rs index ad2a1ddcc..dd7142b36 100644 --- a/core/src/ops/locations/remove/action.rs +++ b/core/src/ops/locations/remove/action.rs @@ -12,25 +12,32 @@ use std::sync::Arc; use uuid::Uuid; #[derive(Debug, Clone, Serialize, Deserialize)] -pub struct LocationRemoveAction { +pub struct LocationRemoveInput { pub library_id: Uuid, pub location_id: Uuid, } +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct LocationRemoveAction { + input: LocationRemoveInput, +} + impl LocationRemoveAction { /// Create a new location remove action - pub fn new(library_id: Uuid, location_id: Uuid) -> Self { - Self { - library_id, - location_id, - } + pub fn new(input: LocationRemoveInput) -> Self { + Self { input } } } // Implement the unified LibraryAction (replaces ActionHandler) impl LibraryAction for LocationRemoveAction { + type Input = LocationRemoveInput; type Output = LocationRemoveOutput; + fn from_input(input: LocationRemoveInput) -> Result { + Ok(LocationRemoveAction::new(input)) + } + async fn execute( self, library: std::sync::Arc, @@ -39,11 +46,11 @@ impl LibraryAction for LocationRemoveAction { // Remove the location let location_manager = LocationManager::new(context.events.as_ref().clone()); location_manager - .remove_location(&library, self.location_id) + .remove_location(&library, self.input.location_id) .await .map_err(|e| ActionError::Internal(e.to_string()))?; - Ok(LocationRemoveOutput::new(self.location_id, None)) + Ok(LocationRemoveOutput::new(self.input.location_id, None)) } fn action_kind(&self) -> &'static str { diff --git a/core/src/ops/locations/rescan/action.rs b/core/src/ops/locations/rescan/action.rs index b1bb26595..d5585423c 100644 --- a/core/src/ops/locations/rescan/action.rs +++ b/core/src/ops/locations/rescan/action.rs @@ -15,16 +15,31 @@ use std::sync::Arc; use uuid::Uuid; #[derive(Debug, Clone, Serialize, Deserialize)] -pub struct LocationRescanAction { - pub library_id: Uuid, +pub struct LocationRescanInput { pub location_id: Uuid, pub full_rescan: bool, } +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct LocationRescanAction { + input: LocationRescanInput, +} + +impl LocationRescanAction { + pub fn new(input: LocationRescanInput) -> Self { + Self { input } + } +} + // Implement LibraryAction impl LibraryAction for LocationRescanAction { + type Input = LocationRescanInput; type Output = super::output::LocationRescanOutput; + fn from_input(input: LocationRescanInput) -> Result { + Ok(LocationRescanAction::new(input)) + } + async fn execute( self, library: std::sync::Arc, @@ -34,12 +49,12 @@ impl LibraryAction for LocationRescanAction { // Get location details from database let location = entities::location::Entity::find() - .filter(entities::location::Column::Uuid.eq(self.location_id)) + .filter(entities::location::Column::Uuid.eq(self.input.location_id)) .one(library.db().conn()) .await .map_err(|e| ActionError::Internal(format!("Database error: {}", e)))? .ok_or_else(|| { - ActionError::Internal(format!("Location not found: {}", self.location_id)) + ActionError::Internal(format!("Location not found: {}", self.input.location_id)) })?; // Get the location's path using PathResolver @@ -50,7 +65,7 @@ impl LibraryAction for LocationRescanAction { let location_path = SdPath::local(location_path_buf); // Determine index mode based on full_rescan flag - let mode = if self.full_rescan { + let mode = if self.input.full_rescan { IndexMode::Deep } else { match location.index_mode.as_str() { @@ -62,7 +77,7 @@ impl LibraryAction for LocationRescanAction { }; // Create indexer job for rescan - let job = IndexerJob::from_location(self.location_id, location_path, mode); + let job = IndexerJob::from_location(self.input.location_id, location_path, mode); // Dispatch the job let job_handle = library @@ -72,10 +87,10 @@ impl LibraryAction for LocationRescanAction { .map_err(ActionError::Job)?; Ok(super::output::LocationRescanOutput { - location_id: self.location_id, + location_id: self.input.location_id, location_path: location_path_str, job_id: job_handle.id().into(), - full_rescan: self.full_rescan, + full_rescan: self.input.full_rescan, }) } diff --git a/core/src/ops/media/thumbnail/action.rs b/core/src/ops/media/thumbnail/action.rs index 2a6411e78..849d62ecd 100644 --- a/core/src/ops/media/thumbnail/action.rs +++ b/core/src/ops/media/thumbnail/action.rs @@ -8,50 +8,44 @@ use crate::{ job::handle::JobHandle, }, }; -use async_trait::async_trait; use std::sync::Arc; -use uuid::Uuid; #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] -pub struct ThumbnailAction { - pub library_id: uuid::Uuid, +pub struct ThumbnailInput { pub paths: Vec, pub size: u32, pub quality: u8, } +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct ThumbnailAction { + input: ThumbnailInput, +} impl ThumbnailAction { /// Create a new thumbnail generation action - pub fn new( - library_id: uuid::Uuid, - paths: Vec, - size: u32, - quality: u8, - ) -> Self { - Self { - library_id, - paths, - size, - quality, - } + pub fn new(input: ThumbnailInput) -> Self { + Self { input } } } // Implement the unified LibraryAction (replaces ActionHandler) impl LibraryAction for ThumbnailAction { + type Input = ThumbnailInput; type Output = JobHandle; + fn from_input(input: ThumbnailInput) -> Result { + Ok(ThumbnailAction::new(input)) + } + async fn execute( self, library: std::sync::Arc, context: Arc, ) -> Result { - // Library is pre-validated by ActionManager - no boilerplate! - // Create thumbnail job config let config = ThumbnailJobConfig { - sizes: vec![self.size], - quality: self.quality, + sizes: vec![self.input.size], + quality: self.input.quality, regenerate: false, ..Default::default() }; @@ -78,10 +72,8 @@ impl LibraryAction for ThumbnailAction { library: &std::sync::Arc, context: Arc, ) -> Result<(), ActionError> { - // Library existence already validated by ActionManager - no boilerplate! - // Validate paths - if self.paths.is_empty() { + if self.input.paths.is_empty() { return Err(ActionError::Validation { field: "paths".to_string(), message: "At least one path must be specified".to_string(), @@ -91,4 +83,3 @@ impl LibraryAction for ThumbnailAction { Ok(()) } } -// Old ActionHandler implementation removed - using unified LibraryAction diff --git a/core/src/ops/metadata/action.rs b/core/src/ops/metadata/action.rs deleted file mode 100644 index 44461dfdb..000000000 --- a/core/src/ops/metadata/action.rs +++ /dev/null @@ -1,88 +0,0 @@ -//! Metadata operations action handler - -use crate::{ - context::CoreContext, - infra::{ - action::{error::ActionError, LibraryAction}, - job::handle::JobHandle, - }, -}; -use async_trait::async_trait; -use std::sync::Arc; -use uuid::Uuid; - -#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] -pub struct MetadataAction { - pub library_id: uuid::Uuid, - pub paths: Vec, - pub extract_exif: bool, - pub extract_xmp: bool, -} - -impl MetadataAction { - /// Create a new metadata extraction action - pub fn new( - library_id: uuid::Uuid, - paths: Vec, - extract_exif: bool, - extract_xmp: bool, - ) -> Self { - Self { - library_id, - paths, - extract_exif, - extract_xmp, - } - } -} - -// Old ActionHandler implementation removed - using unified LibraryAction - -// Implement the unified LibraryAction (replaces ActionHandler) -impl LibraryAction for MetadataAction { - type Output = JobHandle; - - async fn execute( - self, - library: std::sync::Arc, - context: Arc, - ) -> Result { - // Create metadata extraction job - let job_params = serde_json::json!({ - "paths": self.paths, - "extract_exif": self.extract_exif, - "extract_xmp": self.extract_xmp, - }); - - // Dispatch job and return handle - let job_handle = library - .jobs() - .dispatch_by_name("extract_metadata", job_params) - .await - .map_err(ActionError::Job)?; - - Ok(job_handle) - } - - fn action_kind(&self) -> &'static str { - "metadata.extract" - } - - async fn validate( - &self, - library: &std::sync::Arc, - context: Arc, - ) -> Result<(), ActionError> { - // Library existence already validated by ActionManager - no boilerplate! - - // Validate paths - if self.paths.is_empty() { - return Err(ActionError::Validation { - field: "paths".to_string(), - message: "At least one path must be specified".to_string(), - }); - } - - Ok(()) - } -} diff --git a/core/src/ops/metadata/mod.rs b/core/src/ops/metadata/mod.rs deleted file mode 100644 index 97302cdaf..000000000 --- a/core/src/ops/metadata/mod.rs +++ /dev/null @@ -1,310 +0,0 @@ -//! Metadata operations for hierarchical metadata management - -pub mod action; - -use chrono::{DateTime, Utc}; -use sea_orm::{ActiveModelTrait, ColumnTrait, DatabaseConnection, EntityTrait, QueryFilter, Set}; -use serde::{Deserialize, Serialize}; -use std::sync::Arc; -use uuid::Uuid; - -use crate::infra::db::entities::{ - content_identity::{self, Entity as ContentIdentity, Model as ContentIdentityModel}, - entry::{self, Entity as Entry, Model as EntryModel}, - tag::{self, Entity as Tag, Model as TagModel}, - user_metadata::{ - self, ActiveModel as UserMetadataActiveModel, Entity as UserMetadata, - Model as UserMetadataModel, - }, - user_metadata_tag::{ - self, ActiveModel as UserMetadataTagActiveModel, Entity as UserMetadataTag, - }, -}; - -pub use action::MetadataAction; -use crate::common::errors::Result; - -#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] -pub enum MetadataTarget { - /// Metadata for this specific file instance (syncs with Index domain) - Entry(Uuid), - /// Metadata for all instances of this content within library (syncs with UserMetadata domain) - Content(Uuid), -} - -#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] -pub enum MetadataScope { - Entry, // File-specific (higher priority) - Content, // Content-universal (lower priority) -} - -#[derive(Clone, Debug, Serialize, Deserialize)] -pub struct MetadataDisplay { - pub notes: Vec, // Both entry and content notes shown - pub tags: Vec, // Both entry and content tags shown - pub favorite: bool, // Entry-level overrides content-level - pub hidden: bool, // Entry-level overrides content-level - pub custom_data: Option, // Entry-level overrides content-level -} - -#[derive(Clone, Debug, Serialize, Deserialize)] -pub struct MetadataNote { - pub content: String, - pub scope: MetadataScope, - pub created_at: DateTime, -} - -#[derive(Clone, Debug, Serialize, Deserialize)] -pub struct MetadataTag { - pub tag: TagModel, - pub scope: MetadataScope, - pub created_at: DateTime, -} - -#[derive(Clone, Debug, Serialize, Deserialize)] -pub struct MetadataUpdate { - pub notes: Option, - pub favorite: Option, - pub hidden: Option, - pub custom_data: Option, - pub tag_uuids: Option>, -} - -pub struct MetadataService { - library_db: Arc, - current_device_uuid: Uuid, -} - -impl MetadataService { - pub fn new(library_db: Arc, current_device_uuid: Uuid) -> Self { - Self { - library_db, - current_device_uuid, - } - } - - /// Add metadata (notes, tags, favorites) with flexible targeting - pub async fn add_metadata( - &self, - target: MetadataTarget, - metadata_update: MetadataUpdate, - ) -> Result { - match target { - MetadataTarget::Entry(entry_uuid) => { - // File-specific metadata - create entry-scoped UserMetadata - let user_metadata = UserMetadataActiveModel { - uuid: Set(Uuid::new_v4()), - entry_uuid: Set(Some(entry_uuid)), - content_identity_uuid: Set(None), // Mutually exclusive - notes: Set(metadata_update.notes), - favorite: Set(metadata_update.favorite.unwrap_or(false)), - hidden: Set(metadata_update.hidden.unwrap_or(false)), - custom_data: Set(metadata_update.custom_data.unwrap_or_default()), - created_at: Set(Utc::now()), - updated_at: Set(Utc::now()), - ..Default::default() - } - .insert(&*self.library_db) - .await?; - - // Add tags if provided - if let Some(tag_uuids) = metadata_update.tag_uuids { - self.add_tags_to_metadata(user_metadata.id, tag_uuids) - .await?; - } - - Ok(user_metadata) - } - - MetadataTarget::Content(content_identity_uuid) => { - // Content-universal metadata - create content-scoped UserMetadata - let user_metadata = UserMetadataActiveModel { - uuid: Set(Uuid::new_v4()), - entry_uuid: Set(None), // Mutually exclusive - content_identity_uuid: Set(Some(content_identity_uuid)), - notes: Set(metadata_update.notes), - favorite: Set(metadata_update.favorite.unwrap_or(false)), - hidden: Set(metadata_update.hidden.unwrap_or(false)), - custom_data: Set(metadata_update.custom_data.unwrap_or_default()), - created_at: Set(Utc::now()), - updated_at: Set(Utc::now()), - ..Default::default() - } - .insert(&*self.library_db) - .await?; - - // Add tags if provided - if let Some(tag_uuids) = metadata_update.tag_uuids { - self.add_tags_to_metadata(user_metadata.id, tag_uuids) - .await?; - } - - Ok(user_metadata) - } - } - } - - /// Get hierarchical metadata display for an entry (both entry and content metadata shown) - pub async fn get_entry_metadata_display(&self, entry_uuid: Uuid) -> Result { - let mut display = MetadataDisplay { - notes: Vec::new(), - tags: Vec::new(), - favorite: false, - hidden: false, - custom_data: None, - }; - - // Get entry-specific metadata - let entry_metadata = UserMetadata::find() - .filter(user_metadata::Column::EntryUuid.eq(entry_uuid)) - .find_with_related(Tag) - .all(&*self.library_db) - .await?; - - for (metadata, tags) in entry_metadata { - // Notes - show both levels - if let Some(notes) = metadata.notes { - display.notes.push(MetadataNote { - content: notes, - scope: MetadataScope::Entry, - created_at: metadata.created_at, - }); - } - - // Tags - show both levels - for tag in tags { - display.tags.push(MetadataTag { - tag, - scope: MetadataScope::Entry, - created_at: metadata.created_at, - }); - } - - // Favorites/Hidden - entry overrides (higher priority) - display.favorite = metadata.favorite; - display.hidden = metadata.hidden; - display.custom_data = Some(metadata.custom_data); - } - - // Get content-level metadata if entry has content identity - if let Some(entry) = Entry::find() - .filter(entry::Column::Uuid.eq(entry_uuid)) - .one(&*self.library_db) - .await? - { - if let Some(content_id) = entry.content_id { - if let Some(content_identity) = ContentIdentity::find_by_id(content_id) - .one(&*self.library_db) - .await? - { - if let Some(content_uuid) = content_identity.uuid { - let content_metadata = UserMetadata::find() - .filter(user_metadata::Column::ContentIdentityUuid.eq(content_uuid)) - .find_with_related(Tag) - .all(&*self.library_db) - .await?; - - for (metadata, tags) in content_metadata { - // Notes - show both levels - if let Some(notes) = metadata.notes { - display.notes.push(MetadataNote { - content: notes, - scope: MetadataScope::Content, - created_at: metadata.created_at, - }); - } - - // Tags - show both levels - for tag in tags { - display.tags.push(MetadataTag { - tag, - scope: MetadataScope::Content, - created_at: metadata.created_at, - }); - } - - // Favorites/Hidden - only use if no entry-level override - if !display.favorite && metadata.favorite { - display.favorite = true; - } - if !display.hidden && metadata.hidden { - display.hidden = true; - } - if display.custom_data.is_none() { - display.custom_data = Some(metadata.custom_data); - } - } - } - } - } - } - - Ok(display) - } - - /// Promote entry-level metadata to content-level ("Apply to all instances") - pub async fn promote_to_content( - &self, - entry_metadata_id: i32, - content_identity_uuid: Uuid, - ) -> Result { - // Get existing entry-level metadata - let entry_metadata = UserMetadata::find_by_id(entry_metadata_id) - .one(&*self.library_db) - .await? - .ok_or_else(|| { - crate::common::errors::CoreError::NotFound("Metadata not found".to_string()) - })?; - - // Create new content-level metadata (entry-level remains for hierarchy) - let content_metadata = UserMetadataActiveModel { - uuid: Set(Uuid::new_v4()), - entry_uuid: Set(None), - content_identity_uuid: Set(Some(content_identity_uuid)), - notes: Set(entry_metadata.notes.clone()), - favorite: Set(entry_metadata.favorite), - hidden: Set(entry_metadata.hidden), - custom_data: Set(entry_metadata.custom_data.clone()), - created_at: Set(Utc::now()), - updated_at: Set(Utc::now()), - ..Default::default() - } - .insert(&*self.library_db) - .await?; - - // Copy tags to new content-level metadata - let entry_tags = UserMetadataTag::find() - .filter(user_metadata_tag::Column::UserMetadataId.eq(entry_metadata_id)) - .all(&*self.library_db) - .await?; - - for entry_tag in entry_tags { - UserMetadataTagActiveModel { - user_metadata_id: Set(content_metadata.id), - tag_uuid: Set(entry_tag.tag_uuid), - created_at: Set(Utc::now()), - device_uuid: Set(self.current_device_uuid), - ..Default::default() - } - .insert(&*self.library_db) - .await?; - } - - Ok(content_metadata) - } - - async fn add_tags_to_metadata(&self, metadata_id: i32, tag_uuids: Vec) -> Result<()> { - for tag_uuid in tag_uuids { - UserMetadataTagActiveModel { - user_metadata_id: Set(metadata_id), - tag_uuid: Set(tag_uuid), - created_at: Set(Utc::now()), - device_uuid: Set(self.current_device_uuid), - ..Default::default() - } - .insert(&*self.library_db) - .await?; - } - Ok(()) - } -} diff --git a/core/src/ops/mod.rs b/core/src/ops/mod.rs index bc5fb96c9..34cb88540 100644 --- a/core/src/ops/mod.rs +++ b/core/src/ops/mod.rs @@ -9,16 +9,16 @@ //! - Metadata operations (hierarchical tagging) pub mod addressing; -pub mod content; +// pub mod content; pub mod core; -pub mod devices; +// pub mod devices; pub mod entries; pub mod files; pub mod indexing; pub mod libraries; pub mod locations; pub mod media; -pub mod metadata; +// pub mod metadata; pub mod registry; pub mod sidecar; pub mod volumes; diff --git a/core/src/ops/registry.rs b/core/src/ops/registry.rs index 234ac5d5a..2b3c35710 100644 --- a/core/src/ops/registry.rs +++ b/core/src/ops/registry.rs @@ -1,55 +1,20 @@ -//! Core-side dynamic registry for actions and queries using `inventory`. +//! Minimal action/query registry (action-centric) using `inventory`. //! -//! This module provides a compile-time, self-registering system for all operations -//! in the Spacedrive core. Operations automatically register themselves using the -//! `inventory` crate, eliminating the need for manual registration boilerplate. -//! -//! ## Architecture -//! -//! The registry system works in three layers: -//! 1. **Registration**: Operations self-register using macros (`register_query!`, `register_library_action_input!`) -//! 2. **Storage**: Static HashMaps store method-to-handler mappings -//! 3. **Dispatch**: Core engine looks up handlers by method string and executes them -//! -//! ## Usage -//! -//! ```rust -//! // For queries -//! impl Wire for MyQuery { -//! const METHOD: &'static str = "query:my.domain.v1"; -//! } -//! register_query!(MyQuery); -//! -//! // For library actions -//! impl Wire for MyActionInput { -//! const METHOD: &'static str = "action:my.domain.input.v1"; -//! } -//! impl BuildLibraryActionInput for MyActionInput { /* ... */ } -//! register_library_action_input!(MyActionInput); -//! -//! // For core actions -//! impl BuildCoreActionInput for MyCoreActionInput { /* ... */ } -//! register_core_action_input!(MyCoreActionInput); -//! ``` - -use serde::{de::DeserializeOwned, Serialize}; -use std::collections::HashMap; -use std::sync::Arc; +//! Goals: +//! - Tiny, action-centric API: register Actions, decode their associated Inputs +//! - No conversion traits on inputs; Actions declare `type Input` and `from_input(..)` +//! - Single place that resolves `library_id` and dispatches use futures::future::{FutureExt, LocalBoxFuture}; use once_cell::sync::Lazy; +use serde::de::DeserializeOwned; +use std::{collections::HashMap, sync::Arc}; /// Handler function signature for queries. -/// -/// Takes a Core instance and serialized query payload, returns serialized result. -/// Uses `LocalBoxFuture` because handlers don't need to be `Send` (they run in the same thread). pub type QueryHandlerFn = fn(Arc, Vec) -> LocalBoxFuture<'static, Result, String>>; /// Handler function signature for actions. -/// -/// Takes a Core instance, session state, and serialized action payload, returns serialized result. -/// Session state includes things like current library ID and user context. pub type ActionHandlerFn = fn( Arc, crate::infra::daemon::state::SessionState, @@ -57,39 +22,20 @@ pub type ActionHandlerFn = fn( ) -> LocalBoxFuture<'static, Result, String>>; /// Registry entry for a query operation. -/// -/// Contains the method string (e.g., "query:core.status.v1") and the handler function -/// that will deserialize and execute the query. pub struct QueryEntry { - /// The method string used to identify this query pub method: &'static str, - /// The handler function that executes this query pub handler: QueryHandlerFn, } /// Registry entry for an action operation. -/// -/// Contains the method string (e.g., "action:files.copy.input.v1") and the handler function -/// that will deserialize the input, build the action, and execute it. pub struct ActionEntry { - /// The method string used to identify this action pub method: &'static str, - /// The handler function that executes this action pub handler: ActionHandlerFn, } -// Collect all registered query and action entries at compile time inventory::collect!(QueryEntry); inventory::collect!(ActionEntry); -/// Static HashMap containing all registered query handlers. -/// -/// This is lazily initialized on first access. The `inventory` crate automatically -/// collects all `QueryEntry` instances that were registered using `register_query!` -/// and builds this lookup table. -/// -/// Key: Method string (e.g., "query:core.status.v1") -/// Value: Handler function that deserializes and executes the query pub static QUERIES: Lazy> = Lazy::new(|| { let mut map = HashMap::new(); for entry in inventory::iter::() { @@ -98,14 +44,6 @@ pub static QUERIES: Lazy> = Lazy::new(|| { map }); -/// Static HashMap containing all registered action handlers. -/// -/// This is lazily initialized on first access. The `inventory` crate automatically -/// collects all `ActionEntry` instances that were registered using `register_library_action_input!` -/// or `register_core_action_input!` and builds this lookup table. -/// -/// Key: Method string (e.g., "action:files.copy.input.v1") -/// Value: Handler function that deserializes input, builds action, and executes it pub static ACTIONS: Lazy> = Lazy::new(|| { let mut map = HashMap::new(); for entry in inventory::iter::() { @@ -114,269 +52,83 @@ pub static ACTIONS: Lazy> = Lazy::new(|| map }); -/// Generic handler function for executing queries. -/// -/// This function is used by the registry to handle all query operations. It: -/// 1. Deserializes the query from the binary payload -/// 2. Executes the query using the Core engine -/// 3. Serializes the result back to binary -/// -/// # Type Parameters -/// - `Q`: The query type that implements `Query` trait -/// -/// # Arguments -/// - `core`: The Core engine instance -/// - `payload`: Serialized query data -/// -/// # Returns -/// - Serialized query result on success -/// - Error string on failure +/// Generic query handler (decode -> execute -> encode) pub fn handle_query( core: Arc, payload: Vec, ) -> LocalBoxFuture<'static, Result, String>> where - Q: crate::cqrs::Query + Serialize + DeserializeOwned + 'static, - Q::Output: Serialize + 'static, + Q: crate::cqrs::Query + serde::Serialize + DeserializeOwned + 'static, + Q::Output: serde::Serialize + 'static, { use bincode::config::standard; use bincode::serde::{decode_from_slice, encode_to_vec}; (async move { - // Deserialize the query from binary payload let q: Q = decode_from_slice(&payload, standard()) .map_err(|e| e.to_string())? .0; - - // Execute the query using the Core engine let out: Q::Output = core.execute_query(q).await.map_err(|e| e.to_string())?; - - // Serialize the result back to binary encode_to_vec(&out, standard()).map_err(|e| e.to_string()) }) .boxed_local() } -/// Trait for converting external API input types to library actions. -/// -/// This trait is implemented by input types (like `FileCopyInput`) that need to be -/// converted to actual library actions (like `FileCopyAction`) before execution. -/// -/// Library actions operate within a specific library context and require a library ID. -/// The session state provides the current library ID if not explicitly set in the input. -/// -/// # Type Parameters -/// - `Action`: The concrete action type that will be executed -pub trait BuildLibraryActionInput { - /// The action type that this input builds - type Action: crate::infra::action::LibraryAction; - - /// Convert the input to an action (pure conversion; no session/context). - /// - /// # Returns - /// - The built action on success - /// - Error string on failure - fn build(self) -> Result; -} - -/// Trait for converting external API input types to core actions. -/// -/// This trait is implemented by input types (like `LibraryCreateInput`) that need to be -/// converted to actual core actions (like `LibraryCreateAction`) before execution. -/// -/// Core actions operate at the system level and don't require a specific library context. -/// They can create/delete libraries, manage devices, etc. -/// -/// # Type Parameters -/// - `Action`: The concrete action type that will be executed -pub trait BuildCoreActionInput { - /// The action type that this input builds - type Action: crate::infra::action::CoreAction; - - /// Convert the input to an action using session state. - /// - /// # Arguments - /// - `session`: Current session state (may be used for validation or context) - /// - /// # Returns - /// - The built action on success - /// - Error string on failure - fn build( - self, - session: &crate::infra::daemon::state::SessionState, - ) -> Result; -} - -/// Generic handler function for executing library actions. -/// -/// This function is used by the registry to handle all library action operations. It: -/// 1. Deserializes the action input from the binary payload -/// 2. Converts the input to a concrete action using the session state -/// 3. Executes the action through the ActionManager -/// 4. Returns an empty result (actions typically don't return data) -/// -/// # Type Parameters -/// - `I`: The input type that implements `BuildLibraryActionInput` -/// -/// # Arguments -/// - `core`: The Core engine instance -/// - `session`: Current session state (includes library ID, user context) -/// - `payload`: Serialized action input data -/// -/// # Returns -/// - Empty vector on success (actions don't return data) -/// - Error string on failure -pub fn handle_library_action_input( +/// Generic library action handler (decode A::Input -> A::from_input -> dispatch) +pub fn handle_library_action( core: Arc, + // TODO: Move session state to core, shouldn't be in the daemon session: crate::infra::daemon::state::SessionState, payload: Vec, ) -> LocalBoxFuture<'static, Result, String>> where - I: BuildLibraryActionInput + DeserializeOwned + 'static, + A: crate::infra::action::LibraryAction + 'static, + A::Input: DeserializeOwned + 'static, { use bincode::config::standard; use bincode::serde::decode_from_slice; (async move { - // Deserialize the action input from binary payload - let input: I = decode_from_slice(&payload, standard()) + let input: A::Input = decode_from_slice(&payload, standard()) .map_err(|e| e.to_string())? .0; - - // Convert input to concrete action using session state - let action = input.build()?; - - // Execute the action through ActionManager - let action_manager = - crate::infra::action::manager::ActionManager::new(core.context.clone()); + let action = A::from_input(input)?; + let manager = crate::infra::action::manager::ActionManager::new(core.context.clone()); let library_id = session.current_library_id.ok_or("No library selected")?; - action_manager + manager .dispatch_library(library_id, action) .await .map_err(|e| e.to_string())?; - - // Actions typically don't return data, so return empty vector Ok(Vec::new()) }) .boxed_local() } -/// Generic handler function for executing core actions. -/// -/// This function is used by the registry to handle all core action operations. It: -/// 1. Deserializes the action input from the binary payload -/// 2. Converts the input to a concrete action using the session state -/// 3. Executes the action through the ActionManager -/// 4. Returns an empty result (actions typically don't return data) -/// -/// Core actions operate at the system level (library management, device management, etc.) -/// and don't require a specific library context. -/// -/// # Type Parameters -/// - `I`: The input type that implements `BuildCoreActionInput` -/// -/// # Arguments -/// - `core`: The Core engine instance -/// - `session`: Current session state (may be used for validation or context) -/// - `payload`: Serialized action input data -/// -/// # Returns -/// - Empty vector on success (actions don't return data) -/// - Error string on failure -pub fn handle_core_action_input( +/// Generic core action handler (decode A::Input -> A::from_input -> dispatch) +pub fn handle_core_action( core: Arc, session: crate::infra::daemon::state::SessionState, payload: Vec, ) -> LocalBoxFuture<'static, Result, String>> where - I: BuildCoreActionInput + DeserializeOwned + 'static, + A: crate::infra::action::CoreAction + 'static, + A::Input: DeserializeOwned + 'static, { use bincode::config::standard; use bincode::serde::decode_from_slice; (async move { - // Deserialize the action input from binary payload - let input: I = decode_from_slice(&payload, standard()) + let input: A::Input = decode_from_slice(&payload, standard()) .map_err(|e| e.to_string())? .0; - - // Convert input to concrete action using session state - let action = input.build(&session)?; - - // Execute the action through ActionManager - let action_manager = - crate::infra::action::manager::ActionManager::new(core.context.clone()); - action_manager + let action = A::from_input(input)?; + let manager = crate::infra::action::manager::ActionManager::new(core.context.clone()); + manager .dispatch_core(action) .await .map_err(|e| e.to_string())?; - - // Actions typically don't return data, so return empty vector Ok(Vec::new()) }) .boxed_local() } -/// Macro for registering query operations with the inventory system. -/// -/// This macro automatically registers a query type with the registry, eliminating -/// the need for manual registration boilerplate. The query type must implement -/// the `Wire` trait to provide its method string. -/// -/// # Usage -/// -/// ```rust -/// impl Wire for MyQuery { -/// const METHOD: &'static str = "query:my.domain.v1"; -/// } -/// register_query!(MyQuery); -/// ``` -/// -/// # What it does -/// -/// 1. Creates a `QueryEntry` with the query's method string and handler function -/// 2. Submits the entry to the `inventory` system for compile-time collection -/// 3. The entry will be automatically included in the `QUERIES` HashMap at runtime -#[macro_export] -macro_rules! register_query { - ($ty:ty) => { - inventory::submit! { $crate::ops::registry::QueryEntry { method: < $ty as $crate::client::Wire >::METHOD, handler: $crate::ops::registry::handle_query::<$ty> } } - }; -} - -/// Macro for registering library action input operations with the inventory system. -/// -/// This macro automatically registers an action input type with the registry. The -/// input type must implement both `Wire` and `BuildLibraryActionInput` traits. -/// -/// # Usage -/// -/// ```rust -/// impl Wire for MyActionInput { -/// const METHOD: &'static str = "action:my.domain.input.v1"; -/// } -/// impl BuildLibraryActionInput for MyActionInput { -/// type Action = MyAction; -/// fn build(self, session: &SessionState) -> Result { /* ... */ } -/// } -/// register_library_action_input!(MyActionInput); -/// ``` -/// -/// # What it does -/// -/// 1. Creates an `ActionEntry` with the input's method string and handler function -/// 2. Submits the entry to the `inventory` system for compile-time collection -/// 3. The entry will be automatically included in the `ACTIONS` HashMap at runtime -#[macro_export] -macro_rules! register_library_action_input { - ($ty:ty) => { - inventory::submit! { $crate::ops::registry::ActionEntry { method: < $ty as $crate::client::Wire >::METHOD, handler: $crate::ops::registry::handle_library_action_input::<$ty> } } - }; -} - -/// Optional convenience macro: declare Wire + BuildLibraryActionInput + register in one line -/// -/// Usage: -/// register_library_op!(InputType => ActionType, "action:domain.op.input.v1"); -// register_library_op! has been superseded by op!(library_action ...) - /// Helper: construct action method string from a short name like "files.copy" #[macro_export] macro_rules! action_method { @@ -393,329 +145,50 @@ macro_rules! query_method { }; } -/// Unified op! macro for registering operations with minimal boilerplate. -/// -/// Why this macro exists -/// - Standardizes how operations are declared: one line per op. -/// - Keeps inputs pure (no session / library_id) and actions context-free. -/// - Wires method (Wire), build glue, and inventory registration. -/// -/// Conversion requirement (Input -> Action) -/// - The system executes Actions, not Inputs. After decoding the Input from the wire, -/// Core must convert it to the concrete Action before dispatch. -/// - op! does NOT attempt to "guess" this conversion. Rust has no runtime reflection, -/// and declarative macros can’t infer how to build an Action without an explicit path. -/// - Therefore, you must provide ONE conversion mechanism: -/// 1) Implement `TryFrom for Action` (recommended for simple ops), or -/// 2) Provide a builder type via `via = BuilderType` (when a builder already exists) -/// -/// Precedence -/// - With `via = BuilderType`, op! generates `TryFrom for Action` by delegating to -/// `BuilderType::from_input(input).build()` and then wires the rest. -/// - Without `via`, op! expects `TryFrom for Action` to exist and uses it. -/// - (Optionally, we can add a `via_fn = path::to::convert` variant if we want to accept a -/// plain function signature `fn(Input) -> Result` in the future.) -/// -/// Variants: -/// - op!(library_action Input => Action, "domain.op"); -/// - op!(library_action Input => Action, "domain.op", via = BuilderType); -/// - op!(core_action Input => Action, "domain.op"); -/// - op!(core_action Input => Action, "domain.op", via = BuilderType); -/// - op!(query QueryType, "domain.op"); +/// Register a query type by action-style name, binding its Wire method automatically. #[macro_export] -macro_rules! op { - // Library action with builder - (library_action $input:ty => $action:ty, $name:literal, via = $builder:ty) => { - impl $crate::client::Wire for $input { - const METHOD: &'static str = $crate::action_method!($name); - } - - impl ::core::convert::TryFrom<$input> for $action { - type Error = String; - fn try_from(input: $input) -> Result { - use $crate::infra::action::builder::ActionBuilder; - <$builder>::from_input(input) - .build() - .map_err(|e| e.to_string()) - } - } - - impl $crate::ops::registry::BuildLibraryActionInput for $input { - type Action = $action; - fn build(self) -> Result { - >::try_from(self) - .map_err(|e| e.to_string()) - } - } - - $crate::register_library_action_input!($input); - }; - - // Library action using existing TryFrom - (library_action $input:ty => $action:ty, $name:literal) => { - impl $crate::client::Wire for $input { - const METHOD: &'static str = $crate::action_method!($name); - } - - // Fallback: if From for Action exists but TryFrom is not implemented, - // provide a TryFrom that delegates to From with Infallible error. This enables - // zero-boilerplate for simple 1:1 mappings by just implementing `From`. - impl ::core::convert::TryFrom<$input> for $action - where - $action: ::core::convert::From<$input>, - { - type Error = ::core::convert::Infallible; - fn try_from(input: $input) -> Result { - Ok(<$action as ::core::convert::From<$input>>::from(input)) - } - } - - impl $crate::ops::registry::BuildLibraryActionInput for $input { - type Action = $action; - fn build(self) -> Result { - >::try_from(self) - .map_err(|e| e.to_string()) - } - } - - $crate::register_library_action_input!($input); - }; - - // Core action with builder - (core_action $input:ty => $action:ty, $name:literal, via = $builder:ty) => { - impl $crate::client::Wire for $input { - const METHOD: &'static str = $crate::action_method!($name); - } - - impl ::core::convert::TryFrom<$input> for $action { - type Error = String; - fn try_from(input: $input) -> Result { - use $crate::infra::action::builder::ActionBuilder; - <$builder>::from_input(input) - .build() - .map_err(|e| e.to_string()) - } - } - - impl $crate::ops::registry::BuildCoreActionInput for $input { - type Action = $action; - fn build( - self, - _session: &$crate::infra::daemon::state::SessionState, - ) -> Result { - >::try_from(self) - .map_err(|e| e.to_string()) - } - } - - $crate::register_core_action_input!($input); - }; - - // Core action using existing TryFrom - (core_action $input:ty => $action:ty, $name:literal) => { - impl $crate::client::Wire for $input { - const METHOD: &'static str = $crate::action_method!($name); - } - - // Fallback: if From for Action exists but TryFrom is not implemented, - // provide a TryFrom that delegates to From with Infallible error. - impl ::core::convert::TryFrom<$input> for $action - where - $action: ::core::convert::From<$input>, - { - type Error = ::core::convert::Infallible; - fn try_from(input: $input) -> Result { - Ok(<$action as ::core::convert::From<$input>>::from(input)) - } - } - - impl $crate::ops::registry::BuildCoreActionInput for $input { - type Action = $action; - fn build( - self, - _session: &$crate::infra::daemon::state::SessionState, - ) -> Result { - >::try_from(self) - .map_err(|e| e.to_string()) - } - } - - $crate::register_core_action_input!($input); - }; - - // Query - (query $query:ty, $name:literal) => { +macro_rules! register_query { + ($query:ty, $name:literal) => { impl $crate::client::Wire for $query { const METHOD: &'static str = $crate::query_method!($name); } - $crate::register_query!($query); + inventory::submit! { + $crate::ops::registry::QueryEntry { + method: < $query as $crate::client::Wire >::METHOD, + handler: $crate::ops::registry::handle_query::<$query>, + } + } }; } -/// Macro for registering core action input operations with the inventory system. -/// -/// This macro automatically registers a core action input type with the registry. The -/// input type must implement both `Wire` and `BuildCoreActionInput` traits. -/// -/// Core actions operate at the system level (library management, device management, etc.) -/// and don't require a specific library context. -/// -/// # Usage -/// -/// ```rust -/// impl Wire for MyCoreActionInput { -/// const METHOD: &'static str = "action:my.domain.input.v1"; -/// } -/// impl BuildCoreActionInput for MyCoreActionInput { -/// type Action = MyCoreAction; -/// fn build(self, session: &SessionState) -> Result { /* ... */ } -/// } -/// register_core_action_input!(MyCoreActionInput); -/// ``` -/// -/// # What it does -/// -/// 1. Creates an `ActionEntry` with the input's method string and handler function -/// 2. Submits the entry to the `inventory` system for compile-time collection -/// 3. The entry will be automatically included in the `ACTIONS` HashMap at runtime +/// Register a library action `A` by short name; binds method to `A::Input` and handler to `handle_library_action::`. #[macro_export] -macro_rules! register_core_action_input { - ($ty:ty) => { - inventory::submit! { $crate::ops::registry::ActionEntry { method: < $ty as $crate::client::Wire >::METHOD, handler: $crate::ops::registry::handle_core_action_input::<$ty> } } +macro_rules! register_library_action { + ($action:ty, $name:literal) => { + impl $crate::client::Wire for < $action as $crate::infra::action::LibraryAction >::Input { + const METHOD: &'static str = $crate::action_method!($name); + } + inventory::submit! { + $crate::ops::registry::ActionEntry { + method: << $action as $crate::infra::action::LibraryAction >::Input as $crate::client::Wire >::METHOD, + handler: $crate::ops::registry::handle_library_action::<$action>, + } + } }; } -#[cfg(test)] -mod tests { - use super::*; - - /// Test function that lists all registered queries and actions. - /// - /// This is useful for debugging and verifying that operations are properly - /// registered with the inventory system. - /// - /// # Usage - /// - /// ```rust - /// #[test] - /// fn test_list_registered_operations() { - /// list_registered_operations(); - /// } - /// ``` - pub fn list_registered_operations() { - println!("=== Registered Operations ==="); - - // List all registered queries - println!("\n📋 Queries ({} total):", QUERIES.len()); - for (method, _) in QUERIES.iter() { - println!(" • {}", method); +/// Register a core action `A` similarly. +#[macro_export] +macro_rules! register_core_action { + ($action:ty, $name:literal) => { + impl $crate::client::Wire for < $action as $crate::infra::action::CoreAction >::Input { + const METHOD: &'static str = $crate::action_method!($name); } - - // List all registered actions - println!("\n⚡ Actions ({} total):", ACTIONS.len()); - for (method, _) in ACTIONS.iter() { - println!(" • {}", method); + inventory::submit! { + $crate::ops::registry::ActionEntry { + method: << $action as $crate::infra::action::CoreAction >::Input as $crate::client::Wire >::METHOD, + handler: $crate::ops::registry::handle_core_action::<$action>, + } } - - println!("\n=== End Registered Operations ==="); - } - - /// Test function that verifies all registered operations have valid method strings. - /// - /// This ensures that all registered operations follow the expected naming convention: - /// - Queries: `query:{domain}.{operation}.v{version}` - /// - Actions: `action:{domain}.{operation}.input.v{version}` - #[test] - fn test_method_naming_convention() { - // Check query naming convention - for method in QUERIES.keys() { - assert!( - method.starts_with("query:"), - "Query method '{}' should start with 'query:'", - method - ); - assert!( - method.ends_with(".v1"), - "Query method '{}' should end with '.v1'", - method - ); - } - - // Check action naming convention - for method in ACTIONS.keys() { - assert!( - method.starts_with("action:"), - "Action method '{}' should start with 'action:'", - method - ); - assert!( - method.ends_with(".input.v1"), - "Action method '{}' should end with '.input.v1'", - method - ); - } - } - - /// Test function that verifies we have at least some registered operations. - /// - /// This is a basic smoke test to ensure the inventory system is working - /// and we have some operations registered. - #[test] - fn test_has_registered_operations() { - // We should have at least the core status query - assert!( - QUERIES.contains_key("query:core.status.v1"), - "Core status query should be registered" - ); - - // We should have at least the libraries list query - assert!( - QUERIES.contains_key("query:libraries.list.v1"), - "Libraries list query should be registered" - ); - - // We should have at least one action registered - assert!( - !ACTIONS.is_empty(), - "Should have at least one action registered" - ); - - // Print the registered operations for debugging - list_registered_operations(); - } - - /// Test function that verifies no duplicate method strings are registered. - /// - /// This ensures that each operation has a unique method string. - #[test] - fn test_no_duplicate_methods() { - let mut seen_methods = std::collections::HashSet::new(); - - // Check for duplicates in queries - for method in QUERIES.keys() { - assert!( - seen_methods.insert(method), - "Duplicate query method found: {}", - method - ); - } - - // Check for duplicates in actions - for method in ACTIONS.keys() { - assert!( - seen_methods.insert(method), - "Duplicate action method found: {}", - method - ); - } - - // Check for cross-contamination between queries and actions - for method in QUERIES.keys() { - assert!( - !ACTIONS.contains_key(method), - "Method '{}' is registered as both query and action", - method - ); - } - } + }; } diff --git a/core/src/ops/volumes/speed_test/action.rs b/core/src/ops/volumes/speed_test/action.rs index fb1a3617e..c381733b7 100644 --- a/core/src/ops/volumes/speed_test/action.rs +++ b/core/src/ops/volumes/speed_test/action.rs @@ -5,42 +5,60 @@ use super::output::VolumeSpeedTestOutput; use crate::{ context::CoreContext, - infra::action::{error::ActionError, CoreAction}, + infra::action::{error::ActionError, LibraryAction}, volume::VolumeFingerprint, }; use serde::{Deserialize, Serialize}; +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct VolumeSpeedTestInput { + pub fingerprint: VolumeFingerprint, +} + /// Input for volume speed testing #[derive(Debug, Clone, Serialize, Deserialize)] + pub struct VolumeSpeedTestAction { /// The fingerprint of the volume to test - pub fingerprint: VolumeFingerprint, + input: VolumeSpeedTestInput, } impl VolumeSpeedTestAction { /// Create a new volume speed test action - pub fn new(fingerprint: VolumeFingerprint) -> Self { - Self { fingerprint } + pub fn new(input: VolumeSpeedTestInput) -> Self { + Self { input } } } // Implement the new modular ActionType trait -impl CoreAction for VolumeSpeedTestAction { +impl LibraryAction for VolumeSpeedTestAction { + type Input = VolumeSpeedTestInput; type Output = VolumeSpeedTestOutput; - async fn execute(self, context: std::sync::Arc) -> Result { + fn from_input(input: VolumeSpeedTestInput) -> Result { + Ok(VolumeSpeedTestAction::new(input)) + } + + async fn execute( + self, + library: std::sync::Arc, + context: std::sync::Arc, + ) -> Result { // Run the speed test through the volume manager - context.volume_manager - .run_speed_test(&self.fingerprint) + context + .volume_manager + .run_speed_test(&self.input.fingerprint) .await .map_err(|e| ActionError::InvalidInput(format!("Speed test failed: {}", e)))?; // Get the updated volume with speed test results let volume = context .volume_manager - .get_volume(&self.fingerprint) + .get_volume(&self.input.fingerprint) .await - .ok_or_else(|| ActionError::InvalidInput("Volume not found after speed test".to_string()))?; + .ok_or_else(|| { + ActionError::InvalidInput("Volume not found after speed test".to_string()) + })?; // Extract speeds (default to 0 if missing) let read_speed = volume.read_speed_mbps.unwrap_or(0); @@ -48,7 +66,7 @@ impl CoreAction for VolumeSpeedTestAction { // Return native output directly Ok(VolumeSpeedTestOutput::new( - self.fingerprint, + self.input.fingerprint, Some(read_speed as u32), Some(write_speed as u32), )) @@ -58,11 +76,15 @@ impl CoreAction for VolumeSpeedTestAction { "volume.speed_test" } - async fn validate(&self, context: std::sync::Arc) -> Result<(), ActionError> { + async fn validate( + &self, + library: &std::sync::Arc, + context: std::sync::Arc, + ) -> Result<(), ActionError> { // Validate volume exists let volume = context .volume_manager - .get_volume(&self.fingerprint) + .get_volume(&self.input.fingerprint) .await .ok_or_else(|| ActionError::Validation { field: "fingerprint".to_string(), diff --git a/core/src/ops/volumes/track/action.rs b/core/src/ops/volumes/track/action.rs index 92a8b574e..336e4161e 100644 --- a/core/src/ops/volumes/track/action.rs +++ b/core/src/ops/volumes/track/action.rs @@ -12,42 +12,47 @@ use crate::{ use serde::{Deserialize, Serialize}; use uuid::Uuid; -/// Input for tracking a volume #[derive(Debug, Clone, Serialize, Deserialize)] -pub struct VolumeTrackAction { - /// The fingerprint of the volume to track +pub struct VolumeTrackInput { pub fingerprint: VolumeFingerprint, - - /// The library ID to track the volume in - pub library_id: Uuid, - - /// Optional name for the tracked volume pub name: Option, } +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct VolumeTrackAction { + input: VolumeTrackInput, +} + impl VolumeTrackAction { - pub fn new(fingerprint: VolumeFingerprint, library_id: Uuid, name: Option) -> Self { - Self { - fingerprint, - library_id, - name, - } + pub fn new(input: VolumeTrackInput) -> Self { + Self { input } } /// Create a volume track action with a name - pub fn with_name(fingerprint: VolumeFingerprint, library_id: Uuid, name: String) -> Self { - Self::new(fingerprint, library_id, Some(name)) + pub fn with_name(fingerprint: VolumeFingerprint, name: String) -> Self { + Self::new(VolumeTrackInput { + fingerprint, + name: Some(name), + }) } /// Create a volume track action without a name - pub fn without_name(fingerprint: VolumeFingerprint, library_id: Uuid) -> Self { - Self::new(fingerprint, library_id, None) + pub fn without_name(fingerprint: VolumeFingerprint) -> Self { + Self::new(VolumeTrackInput { + fingerprint, + name: None, + }) } } impl LibraryAction for VolumeTrackAction { + type Input = VolumeTrackInput; type Output = VolumeTrackOutput; + fn from_input(input: VolumeTrackInput) -> Result { + Ok(VolumeTrackAction::new(input)) + } + async fn execute( self, library: std::sync::Arc, @@ -56,7 +61,7 @@ impl LibraryAction for VolumeTrackAction { // Check if volume exists let volume = context .volume_manager - .get_volume(&self.fingerprint) + .get_volume(&self.input.fingerprint) .await .ok_or_else(|| ActionError::InvalidInput("Volume not found".to_string()))?; @@ -69,13 +74,12 @@ impl LibraryAction for VolumeTrackAction { // Track the volume in the database let tracked = context .volume_manager - .track_volume(&library, &self.fingerprint, self.name.clone()) + .track_volume(&library, &self.input.fingerprint, self.input.name.clone()) .await .map_err(|e| ActionError::InvalidInput(format!("Volume tracking failed: {}", e)))?; Ok(VolumeTrackOutput::new( - self.fingerprint, - self.library_id, + self.input.fingerprint, tracked.display_name.unwrap_or(volume.name), )) } @@ -92,7 +96,7 @@ impl LibraryAction for VolumeTrackAction { // Validate volume exists let volume = context .volume_manager - .get_volume(&self.fingerprint) + .get_volume(&self.input.fingerprint) .await .ok_or_else(|| ActionError::Validation { field: "fingerprint".to_string(), @@ -108,7 +112,7 @@ impl LibraryAction for VolumeTrackAction { } // Validate name if provided - if let Some(name) = &self.name { + if let Some(name) = &self.input.name { if name.trim().is_empty() { return Err(ActionError::Validation { field: "name".to_string(), diff --git a/core/src/ops/volumes/track/output.rs b/core/src/ops/volumes/track/output.rs index 4192e8666..62d4ddfde 100644 --- a/core/src/ops/volumes/track/output.rs +++ b/core/src/ops/volumes/track/output.rs @@ -2,7 +2,6 @@ use crate::volume::VolumeFingerprint; use serde::{Deserialize, Serialize}; -use uuid::Uuid; /// Output from volume track operation #[derive(Debug, Clone, Serialize, Deserialize)] @@ -10,21 +9,16 @@ pub struct VolumeTrackOutput { /// The fingerprint of the tracked volume pub fingerprint: VolumeFingerprint, - /// The library ID where the volume was tracked - pub library_id: Uuid, - /// The display name of the tracked volume pub volume_name: String, } impl VolumeTrackOutput { /// Create new volume track output - pub fn new(fingerprint: VolumeFingerprint, library_id: Uuid, volume_name: String) -> Self { + pub fn new(fingerprint: VolumeFingerprint, volume_name: String) -> Self { Self { fingerprint, - library_id, volume_name, } } } - diff --git a/core/src/ops/volumes/untrack/action.rs b/core/src/ops/volumes/untrack/action.rs index 87cbb290a..fe1ce7f00 100644 --- a/core/src/ops/volumes/untrack/action.rs +++ b/core/src/ops/volumes/untrack/action.rs @@ -9,32 +9,35 @@ use crate::{ volume::VolumeFingerprint, }; use serde::{Deserialize, Serialize}; -use uuid::Uuid; + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct VolumeUntrackInput { + pub fingerprint: VolumeFingerprint, +} /// Input for untracking a volume #[derive(Debug, Clone, Serialize, Deserialize)] pub struct VolumeUntrackAction { /// The fingerprint of the volume to untrack - pub fingerprint: VolumeFingerprint, - - /// The library ID to untrack the volume from - pub library_id: Uuid, + input: VolumeUntrackInput, } impl VolumeUntrackAction { /// Create a new volume untrack action - pub fn new(fingerprint: VolumeFingerprint, library_id: Uuid) -> Self { - Self { - fingerprint, - library_id, - } + pub fn new(input: VolumeUntrackInput) -> Self { + Self { input } } } // Implement the unified ActionTrait (following VolumeTrackAction model) impl LibraryAction for VolumeUntrackAction { + type Input = VolumeUntrackInput; type Output = VolumeUntrackOutput; + fn from_input(input: VolumeUntrackInput) -> Result { + Ok(VolumeUntrackAction::new(input)) + } + async fn execute( self, library: std::sync::Arc, @@ -43,12 +46,12 @@ impl LibraryAction for VolumeUntrackAction { // Untrack the volume from the database context .volume_manager - .untrack_volume(&library, &self.fingerprint) + .untrack_volume(&library, &self.input.fingerprint) .await .map_err(|e| ActionError::InvalidInput(format!("Volume untracking failed: {}", e)))?; // Return native output directly - Ok(VolumeUntrackOutput::new(self.fingerprint, self.library_id)) + Ok(VolumeUntrackOutput::new(self.input.fingerprint)) } fn action_kind(&self) -> &'static str { @@ -63,7 +66,7 @@ impl LibraryAction for VolumeUntrackAction { // Validate volume exists let _volume = context .volume_manager - .get_volume(&self.fingerprint) + .get_volume(&self.input.fingerprint) .await .ok_or_else(|| ActionError::Validation { field: "fingerprint".to_string(), diff --git a/core/src/ops/volumes/untrack/output.rs b/core/src/ops/volumes/untrack/output.rs index 8ebdfa297..389774fb8 100644 --- a/core/src/ops/volumes/untrack/output.rs +++ b/core/src/ops/volumes/untrack/output.rs @@ -2,25 +2,17 @@ use crate::volume::VolumeFingerprint; use serde::{Deserialize, Serialize}; -use uuid::Uuid; /// Output from volume untrack operation #[derive(Debug, Clone, Serialize, Deserialize)] pub struct VolumeUntrackOutput { /// The fingerprint of the untracked volume pub fingerprint: VolumeFingerprint, - - /// The library ID from which the volume was untracked - pub library_id: Uuid, } impl VolumeUntrackOutput { /// Create new volume untrack output - pub fn new(fingerprint: VolumeFingerprint, library_id: Uuid) -> Self { - Self { - fingerprint, - library_id, - } + pub fn new(fingerprint: VolumeFingerprint) -> Self { + Self { fingerprint } } } - diff --git a/docs/core/op_initialization_api.md b/docs/core/op_initialization_api.md deleted file mode 100644 index 7ef46e75e..000000000 --- a/docs/core/op_initialization_api.md +++ /dev/null @@ -1,219 +0,0 @@ -### Operations Initialization API (op!) - -This document defines a simple, uniform API for declaring and registering all operations (library actions, core actions, and queries) in Core. The goals are: - -- Make operations easy to add, understand, and maintain -- Keep inputs pure (no session, no library_id) -- Keep actions free of transport concerns (no library_id in action structs) -- Provide consistent method naming and automatic inventory registration -- Reduce boilerplate and promote repeatable patterns - ---- - -### Core Concepts - -- **Input**: External API payload (CLI/GraphQL). Contains only operation-specific fields. -- **Action**: Executable business logic. Receives `Arc` (for library actions) or only `CoreContext` (for core actions) during execution. -- **Query**: Read-only operation returning data. -- **Method**: Opaque string used for routing (e.g., `action:files.copy.input.v1`, `query:core.status.v1`). -- **Registry**: Inventory-backed lookup that dispatches by method. - -Inputs are decoded, converted to actions (for actions), and executed. For library actions, the `library_id` is resolved by the dispatcher and never appears in inputs or actions. - ---- - -### The `op!` Macro (Proposed) - -One macro, three variants. Uniform API for every operation type. - -- Library actions: - -```rust -op!(library_action InputType => ActionType, "files.copy", via = BuilderType); -// or, if no builder is needed: -op!(library_action InputType => ActionType, "files.validation"); -``` - -- Core actions: - -```rust -op!(core_action InputType => ActionType, "libraries.create", via = BuilderType); -// or -op!(core_action InputType => ActionType, "libraries.delete"); -``` - -- Queries: - -```rust -op!(query QueryType, "core.status"); -``` - -#### What `op!` does - -- Generates the method string automatically: - - Library/Core actions → `action:{domain}.{op}.input.v1` - - Queries → `query:{domain}.{op}.v1` -- Implements `Wire` for the type with that method -- Implements the appropriate build/dispatch glue and registers with `inventory` -- For actions, wires Input → Action using one of: - - `TryFrom for Action` (preferred), or - - `via = BuilderType` (calls `BuilderType::from_input(input).build()`) if provided - -This keeps every operation declaration small and consistent. - ---- - -### Conversion: Input → Action - -Use `TryFrom for Action` as the single source of truth for how inputs become actions. For complex operations that already have builders, the `TryFrom` impl can delegate to the builder. - -Example (File Copy): - -```rust -impl TryFrom for FileCopyAction { - type Error = String; - fn try_from(input: FileCopyInput) -> Result { - use crate::infra::action::builder::ActionBuilder; - FileCopyActionBuilder::from_input(input).build().map_err(|e| e.to_string()) - } -} - -op!(library_action FileCopyInput => FileCopyAction, "files.copy"); -``` - -Example (Validation – direct construction): - -```rust -impl TryFrom for ValidationAction { - type Error = String; - fn try_from(input: FileValidationInput) -> Result { - Ok(ValidationAction::new(input.paths, input.verify_checksums, input.deep_scan)) - } -} - -op!(library_action FileValidationInput => ValidationAction, "files.validation"); -``` - ---- - -### Method Naming & Versioning - -- Actions: `action:{domain}.{operation}.input.v{version}` -- Queries: `query:{domain}.{operation}.v{version}` -- Default version: `v1`. Bump when the wire contract changes. -- Keep `{domain}.{operation}` short, stable, and human-readable. - -Optional helpers: - -- `action_method!("files.copy")` → `"action:files.copy.input.v1"` -- `query_method!("core.status")` → `"query:core.status.v1"` - -`op!` can call these internally so call sites only specify `"files.copy"` or `"core.status"`. - ---- - -### Dispatch Flow (Library Action) - -1. Client sends `Input` with `method` -2. Core registry decodes and builds `Action` (pure conversion) -3. Registry resolves `library_id` from session -4. `ActionManager::dispatch_library(library_id, action)` -5. Manager fetches `Arc`, validates, creates audit entry, then calls `action.execute(library, context)` - -Core actions are the same minus step 3. - ---- - -### Implementation Checklist - -- Traits/Manager (done): - - `LibraryAction` has no `library_id()` requirement - - `ActionManager::dispatch_library(library_id, action)` resolves library and logs -- Registry (done): - - `BuildLibraryActionInput::build(self) -> Action` (pure) - - Handler resolves `library_id` from session once -- Inputs/Actions: - - Inputs are pure (no session/library_id) - - Actions do not store `library_id` - - Add `TryFrom for Action` (delegate to builder when needed) -- Macro: - - Provide `op!` (three variants) and method helpers (optional) - ---- - -### Migration Plan - -1. Introduce `op!` and helpers in `ops::registry` -2. Convert existing operations: - - Files: copy, delete, validation, duplicate_detection (copy: via builder; others: direct TryFrom) - - Indexing: implement TryFrom or use builder; remove library_id from action if present - - Libraries/Core ops: create/rename/delete via `op!(core_action …)` - - Queries: swap to `op!(query …)` where appropriate -3. Delete old per-op registration boilerplate -4. Run registry tests to verify: - - All required methods registered - - Naming convention checks pass - - No duplicates across queries/actions - ---- - -### Examples (End-to-End) - -- File Copy: - -```rust -impl TryFrom for FileCopyAction { /* delegate to builder */ } -op!(library_action FileCopyInput => FileCopyAction, "files.copy"); -``` - -- Validation: - -```rust -impl TryFrom for ValidationAction { /* construct directly */ } -op!(library_action FileValidationInput => ValidationAction, "files.validation"); -``` - -- Library Create (Core Action): - -```rust -impl TryFrom for LibraryCreateAction { /* construct directly */ } -op!(core_action LibraryCreateInput => LibraryCreateAction, "libraries.create"); -``` - -- Core Status (Query): - -```rust -op!(query CoreStatusQuery, "core.status"); -``` - ---- - -### Testing & Tooling - -- Use existing registry tests: - - - `test_method_naming_convention` - - `test_has_registered_operations` - - `test_no_duplicate_methods` - - `list_registered_operations()` to debug - -- Add unit tests for `TryFrom for Action` where logic is non-trivial (builder path, validation errors). - ---- - -### Do & Don’t - -- **Do** keep inputs pure and actions context-free -- **Do** resolve `library_id` once at dispatch time -- **Do** prefer `TryFrom for Action` and reuse builders when present -- **Don’t** put `library_id` into inputs or actions -- **Don’t** pass `SessionState` into `build()` -- **Don’t** hardcode method strings inconsistently—use `op!` - ---- - -### Future Extensions - -- Derive `#[derive(LibraryOpInput("files.copy"))]` to generate `TryFrom`, `Wire`, and registration automatically for simple ops -- Add lint to enforce method naming and versioning conventions -- Method helpers `action_method!`/`query_method!` to centralize formatting