From 5c6735ea284d095937255e63aa06bf5e67349c1e Mon Sep 17 00:00:00 2001 From: Jamie Pine Date: Wed, 10 Sep 2025 13:14:30 -0400 Subject: [PATCH] refactor: rename action registration macros for clarity - Updated the action registration macros from `register_action_input!` to `register_library_action_input!` to better reflect their purpose and improve clarity in the codebase. - Adjusted related documentation and input structures to ensure consistency with the new naming convention. - Enhanced the organization of input types for file operations and library management, promoting better modularity and maintainability. These changes improve the clarity and consistency of the action registration process across the codebase. --- core/src/ops/files/copy/input.rs | 4 +- core/src/ops/files/delete/input.rs | 131 ++++++++++++------------ core/src/ops/indexing/input.rs | 4 +- core/src/ops/libraries/create/action.rs | 10 +- core/src/ops/libraries/create/input.rs | 75 +++++++------- core/src/ops/registry.rs | 10 +- docs/core/ops.md | 4 +- 7 files changed, 114 insertions(+), 124 deletions(-) diff --git a/core/src/ops/files/copy/input.rs b/core/src/ops/files/copy/input.rs index 32cc2fac0..a81d01cbf 100644 --- a/core/src/ops/files/copy/input.rs +++ b/core/src/ops/files/copy/input.rs @@ -1,7 +1,7 @@ //! Core input types for file copy operations use super::job::CopyOptions; -use crate::register_action_input; +use crate::register_library_action_input; use serde::{Deserialize, Serialize}; use std::path::PathBuf; @@ -73,7 +73,7 @@ impl crate::ops::registry::BuildLibraryActionInput for FileCopyInput { } } -register_action_input!(FileCopyInput); +register_library_action_input!(FileCopyInput); impl FileCopyInput { /// Create a new FileCopyInput with default options diff --git a/core/src/ops/files/delete/input.rs b/core/src/ops/files/delete/input.rs index fcaaa09c7..228542d28 100644 --- a/core/src/ops/files/delete/input.rs +++ b/core/src/ops/files/delete/input.rs @@ -1,96 +1,97 @@ //! Input types for file deletion operations -use crate::register_action_input; +use crate::register_library_action_input; use serde::{Deserialize, Serialize}; use std::path::PathBuf; /// Input for deleting files #[derive(Debug, Clone, Serialize, Deserialize)] pub struct FileDeleteInput { - /// The library ID where this operation takes place - pub library_id: Option, + /// The library ID where this operation takes place + pub library_id: Option, - /// Files or directories to delete - pub targets: Vec, + /// Files or directories to delete + pub targets: Vec, - /// Whether to permanently delete (true) or move to trash (false) - pub permanent: bool, + /// Whether to permanently delete (true) or move to trash (false) + pub permanent: bool, - /// Whether to delete directories recursively - pub recursive: bool, + /// Whether to delete directories recursively + pub recursive: bool, } impl crate::client::Wire for FileDeleteInput { - const METHOD: &'static str = "action:files.delete.input.v1"; + const METHOD: &'static str = "action:files.delete.input.v1"; } impl crate::ops::registry::BuildLibraryActionInput for FileDeleteInput { - type Action = crate::ops::files::delete::action::FileDeleteAction; + type Action = crate::ops::files::delete::action::FileDeleteAction; - fn build( - self, - session: &crate::infra::daemon::state::SessionState, - ) -> Result { - use crate::ops::files::delete::job::DeleteOptions; + fn build( + self, + session: &crate::infra::daemon::state::SessionState, + ) -> Result { + use crate::ops::files::delete::job::DeleteOptions; - let library_id = self.library_id - .or(session.current_library_id) - .ok_or("No library ID provided and no current library set")?; + let library_id = self + .library_id + .or(session.current_library_id) + .ok_or("No library ID provided and no current library set")?; - Ok(crate::ops::files::delete::action::FileDeleteAction { - library_id, - targets: self.targets, - options: DeleteOptions { - permanent: self.permanent, - recursive: self.recursive, - }, - }) - } + Ok(crate::ops::files::delete::action::FileDeleteAction { + library_id, + targets: self.targets, + options: DeleteOptions { + permanent: self.permanent, + recursive: self.recursive, + }, + }) + } } -register_action_input!(FileDeleteInput); +register_library_action_input!(FileDeleteInput); impl FileDeleteInput { - /// Create a new file deletion input - pub fn new(targets: Vec) -> Self { - Self { - library_id: None, - targets, - permanent: false, - recursive: true, - } - } + /// Create a new file deletion input + pub fn new(targets: Vec) -> Self { + Self { + library_id: None, + targets, + permanent: false, + recursive: true, + } + } - /// Set the library ID - pub fn with_library_id(mut self, library_id: uuid::Uuid) -> Self { - self.library_id = Some(library_id); - self - } + /// Set the library ID + pub fn with_library_id(mut self, library_id: uuid::Uuid) -> Self { + self.library_id = Some(library_id); + self + } - /// Set permanent deletion - pub fn with_permanent(mut self, permanent: bool) -> Self { - self.permanent = permanent; - self - } + /// Set permanent deletion + pub fn with_permanent(mut self, permanent: bool) -> Self { + self.permanent = permanent; + self + } - /// Set recursive deletion - pub fn with_recursive(mut self, recursive: bool) -> Self { - self.recursive = recursive; - self - } + /// Set recursive deletion + pub fn with_recursive(mut self, recursive: bool) -> Self { + self.recursive = recursive; + self + } - /// Validate the input - pub fn validate(&self) -> Result<(), Vec> { - let mut errors = Vec::new(); + /// Validate the input + pub fn validate(&self) -> Result<(), Vec> { + let mut errors = Vec::new(); - if self.targets.is_empty() { - errors.push("At least one target file must be specified".to_string()); - } + if self.targets.is_empty() { + errors.push("At least one target file must be specified".to_string()); + } - if errors.is_empty() { - Ok(()) - } else { - Err(errors) - } - } + if errors.is_empty() { + Ok(()) + } else { + Err(errors) + } + } } diff --git a/core/src/ops/indexing/input.rs b/core/src/ops/indexing/input.rs index 2b5f3b13c..1cb1a06fe 100644 --- a/core/src/ops/indexing/input.rs +++ b/core/src/ops/indexing/input.rs @@ -1,7 +1,7 @@ //! Core input types for indexing operations use super::job::{IndexMode, IndexPersistence, IndexScope}; -use crate::register_action_input; +use crate::register_library_action_input; use serde::{Deserialize, Serialize}; use std::path::PathBuf; @@ -45,7 +45,7 @@ impl crate::ops::registry::BuildLibraryActionInput for IndexInput { } } -register_action_input!(IndexInput); +register_library_action_input!(IndexInput); impl IndexInput { /// Create a new input with sane defaults diff --git a/core/src/ops/libraries/create/action.rs b/core/src/ops/libraries/create/action.rs index 10819bc9a..3704d2315 100644 --- a/core/src/ops/libraries/create/action.rs +++ b/core/src/ops/libraries/create/action.rs @@ -3,10 +3,7 @@ use super::output::LibraryCreateOutput; use crate::{ context::CoreContext, - infra::action::{ - error::ActionError, - CoreAction, - }, + infra::action::{error::ActionError, CoreAction}, }; use async_trait::async_trait; use std::path::PathBuf; @@ -19,20 +16,16 @@ pub struct LibraryCreateAction { pub path: Option, } -// LibraryCreateHandler removed - using unified ActionTrait instead - // Implement the new modular ActionType trait impl CoreAction for LibraryCreateAction { type Output = LibraryCreateOutput; async fn execute(self, context: Arc) -> Result { - // Delegate to existing business logic let library_manager = &context.library_manager; let library = library_manager .create_library(self.name.clone(), self.path.clone(), context.clone()) .await?; - // Return native output directly - no ActionOutput conversion! Ok(LibraryCreateOutput::new( library.id(), library.name().await, @@ -54,4 +47,3 @@ impl CoreAction for LibraryCreateAction { Ok(()) } } - diff --git a/core/src/ops/libraries/create/input.rs b/core/src/ops/libraries/create/input.rs index 38aa6eecc..cac7beb1e 100644 --- a/core/src/ops/libraries/create/input.rs +++ b/core/src/ops/libraries/create/input.rs @@ -7,60 +7,57 @@ use std::path::PathBuf; /// Input for creating a new library #[derive(Debug, Clone, Serialize, Deserialize)] pub struct LibraryCreateInput { - /// Name of the library - pub name: String, + /// Name of the library + pub name: String, - /// Optional path for the library (if not provided, will use default location) - pub path: Option, + /// Optional path for the library (if not provided, will use default location) + pub path: Option, } impl crate::client::Wire for LibraryCreateInput { - const METHOD: &'static str = "action:libraries.create.input.v1"; + const METHOD: &'static str = "action:libraries.create.input.v1"; } impl crate::ops::registry::BuildCoreActionInput for LibraryCreateInput { - type Action = crate::ops::libraries::create::action::LibraryCreateAction; + 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, - }) - } + 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 { - Self { - name, - path: None, - } - } + /// Create a new library creation input + pub fn new(name: String) -> Self { + Self { name, path: None } + } - /// Create with a specific path - pub fn with_path(mut self, path: PathBuf) -> Self { - self.path = Some(path); - self - } + /// Create with a specific path + pub fn with_path(mut self, path: PathBuf) -> Self { + self.path = Some(path); + self + } - /// Validate the input - pub fn validate(&self) -> Result<(), Vec> { - let mut errors = Vec::new(); + /// Validate the input + pub fn validate(&self) -> Result<(), Vec> { + let mut errors = Vec::new(); - if self.name.trim().is_empty() { - errors.push("Library name cannot be empty".to_string()); - } + if self.name.trim().is_empty() { + errors.push("Library name cannot be empty".to_string()); + } - if errors.is_empty() { - Ok(()) - } else { - Err(errors) - } - } + if errors.is_empty() { + Ok(()) + } else { + Err(errors) + } + } } diff --git a/core/src/ops/registry.rs b/core/src/ops/registry.rs index 0de8060db..f51a3bf38 100644 --- a/core/src/ops/registry.rs +++ b/core/src/ops/registry.rs @@ -7,7 +7,7 @@ //! ## Architecture //! //! The registry system works in three layers: -//! 1. **Registration**: Operations self-register using macros (`register_query!`, `register_action_input!`) +//! 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 //! @@ -25,7 +25,7 @@ //! const METHOD: &'static str = "action:my.domain.input.v1"; //! } //! impl BuildLibraryActionInput for MyActionInput { /* ... */ } -//! register_action_input!(MyActionInput); +//! register_library_action_input!(MyActionInput); //! //! // For core actions //! impl BuildCoreActionInput for MyCoreActionInput { /* ... */ } @@ -101,7 +101,7 @@ pub static QUERIES: Lazy> = Lazy::new(|| { /// 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_action_input!` +/// 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") @@ -361,7 +361,7 @@ macro_rules! register_query { /// type Action = MyAction; /// fn build(self, session: &SessionState) -> Result { /* ... */ } /// } -/// register_action_input!(MyActionInput); +/// register_library_action_input!(MyActionInput); /// ``` /// /// # What it does @@ -370,7 +370,7 @@ macro_rules! register_query { /// 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_action_input { +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> } } }; diff --git a/docs/core/ops.md b/docs/core/ops.md index 205cfea42..cec8893ff 100644 --- a/docs/core/ops.md +++ b/docs/core/ops.md @@ -154,7 +154,7 @@ impl Wire for FileCopyInput { const METHOD: &'static str = "action:files.copy.input.v1"; } -register_action_input!(FileCopyInput); +register_library_action_input!(FileCopyInput); ``` #### `output.rs` - Response Types @@ -273,7 +273,7 @@ Operations self-register using the `inventory` crate: register_query!(CoreStatusQuery); // For action inputs -register_action_input!(FileCopyInput); +register_library_action_input!(FileCopyInput); ``` ### Method Naming Convention