From 596470394d1b30eb8a693dfe792f776b0c04493f Mon Sep 17 00:00:00 2001 From: Jamie Pine Date: Fri, 4 Jul 2025 02:52:41 -0700 Subject: [PATCH] Refactor action management and module structure for improved clarity - Moved ActionManager from `operations` to `infrastructure` to better align with its role in managing action execution. - Updated references throughout the codebase to reflect the new module structure, enhancing clarity and maintainability. - Introduced new action handling patterns in the CLI commands, ensuring actions are dispatched correctly with updated parameters. - Refactored file operations to utilize the new `files` module, improving organization and reducing complexity. These changes streamline the action management system and enhance the overall architecture of the codebase. --- core-new/src/context.rs | 2 +- .../actions/BUILDER_REFACTOR_PLAN.md | 484 ++++++++++++++++ .../actions/error.rs | 0 .../actions/handler.rs | 0 .../actions/manager.rs | 33 +- core-new/src/infrastructure/actions/mod.rs | 215 +++++++ .../actions/receipt.rs | 0 .../actions/registry.rs | 2 +- .../actions/tests.rs | 32 +- core-new/src/infrastructure/cli/commands.rs | 34 +- core-new/src/infrastructure/mod.rs | 1 + core-new/src/lib.rs | 4 +- core-new/src/library/mod.rs | 2 +- core-new/src/operations/REFACTOR_PLAN.md | 529 ++++++++++++++++-- .../operations/actions/handlers/file_copy.rs | 105 ---- .../actions/handlers/library_create.rs | 57 -- .../src/operations/actions/handlers/mod.rs | 18 - core-new/src/operations/actions/mod.rs | 205 ------- core-new/src/operations/content/action.rs | 61 ++ core-new/src/operations/content/mod.rs | 4 + core-new/src/operations/file_ops/mod.rs | 16 - core-new/src/operations/files/copy/action.rs | 110 ++++ .../{file_ops => files}/copy/job.rs | 0 .../{file_ops => files}/copy/mod.rs | 1 + .../{file_ops => files}/copy/routing.rs | 0 .../{file_ops => files}/copy/strategy.rs | 0 .../file_delete.rs => files/delete/action.rs} | 53 +- .../delete_job.rs => files/delete/job.rs} | 16 + core-new/src/operations/files/delete/mod.rs | 6 + .../files/duplicate_detection/action.rs | 103 ++++ .../duplicate_detection/job.rs} | 0 .../files/duplicate_detection/mod.rs | 7 + core-new/src/operations/files/mod.rs | 11 + .../src/operations/files/validation/action.rs | 103 ++++ .../validation/job.rs} | 0 .../src/operations/files/validation/mod.rs | 7 + core-new/src/operations/indexing/action.rs | 102 ++++ core-new/src/operations/indexing/mod.rs | 2 + .../src/operations/libraries/create/action.rs | 84 +++ .../src/operations/libraries/create/mod.rs | 3 + .../delete/action.rs} | 14 +- .../src/operations/libraries/delete/mod.rs | 3 + core-new/src/operations/libraries/mod.rs | 7 + .../add/action.rs} | 66 +-- core-new/src/operations/locations/add/mod.rs | 3 + .../index/action.rs} | 45 +- .../src/operations/locations/index/mod.rs | 3 + core-new/src/operations/locations/mod.rs | 9 + .../remove/action.rs} | 14 +- .../src/operations/locations/remove/mod.rs | 3 + .../{media_processing => media}/mod.rs | 0 .../src/operations/media/thumbnail/action.rs | 97 ++++ .../thumbnail/error.rs | 0 .../thumbnail/generator.rs | 0 .../thumbnail/job.rs | 0 .../thumbnail/mod.rs | 4 +- .../thumbnail/state.rs | 0 .../thumbnail/utils.rs | 0 core-new/src/operations/metadata/action.rs | 89 +++ core-new/src/operations/metadata/mod.rs | 4 + core-new/src/operations/mod.rs | 19 +- core-new/src/services/file_sharing.rs | 2 +- core-new/tests/job_registration_test.rs | 4 +- 63 files changed, 2199 insertions(+), 599 deletions(-) create mode 100644 core-new/src/infrastructure/actions/BUILDER_REFACTOR_PLAN.md rename core-new/src/{operations => infrastructure}/actions/error.rs (100%) rename core-new/src/{operations => infrastructure}/actions/handler.rs (100%) rename core-new/src/{operations => infrastructure}/actions/manager.rs (82%) create mode 100644 core-new/src/infrastructure/actions/mod.rs rename core-new/src/{operations => infrastructure}/actions/receipt.rs (100%) rename core-new/src/{operations => infrastructure}/actions/registry.rs (96%) rename core-new/src/{operations => infrastructure}/actions/tests.rs (65%) delete mode 100644 core-new/src/operations/actions/handlers/file_copy.rs delete mode 100644 core-new/src/operations/actions/handlers/library_create.rs delete mode 100644 core-new/src/operations/actions/handlers/mod.rs delete mode 100644 core-new/src/operations/actions/mod.rs create mode 100644 core-new/src/operations/content/action.rs delete mode 100644 core-new/src/operations/file_ops/mod.rs create mode 100644 core-new/src/operations/files/copy/action.rs rename core-new/src/operations/{file_ops => files}/copy/job.rs (100%) rename core-new/src/operations/{file_ops => files}/copy/mod.rs (91%) rename core-new/src/operations/{file_ops => files}/copy/routing.rs (100%) rename core-new/src/operations/{file_ops => files}/copy/strategy.rs (100%) rename core-new/src/operations/{actions/handlers/file_delete.rs => files/delete/action.rs} (54%) rename core-new/src/operations/{file_ops/delete_job.rs => files/delete/job.rs} (98%) create mode 100644 core-new/src/operations/files/delete/mod.rs create mode 100644 core-new/src/operations/files/duplicate_detection/action.rs rename core-new/src/operations/{file_ops/duplicate_detection_job.rs => files/duplicate_detection/job.rs} (100%) create mode 100644 core-new/src/operations/files/duplicate_detection/mod.rs create mode 100644 core-new/src/operations/files/mod.rs create mode 100644 core-new/src/operations/files/validation/action.rs rename core-new/src/operations/{file_ops/validation_job.rs => files/validation/job.rs} (100%) create mode 100644 core-new/src/operations/files/validation/mod.rs create mode 100644 core-new/src/operations/indexing/action.rs create mode 100644 core-new/src/operations/libraries/create/action.rs create mode 100644 core-new/src/operations/libraries/create/mod.rs rename core-new/src/operations/{actions/handlers/library_delete.rs => libraries/delete/action.rs} (72%) create mode 100644 core-new/src/operations/libraries/delete/mod.rs create mode 100644 core-new/src/operations/libraries/mod.rs rename core-new/src/operations/{actions/handlers/location_add.rs => locations/add/action.rs} (60%) create mode 100644 core-new/src/operations/locations/add/mod.rs rename core-new/src/operations/{actions/handlers/location_index.rs => locations/index/action.rs} (53%) create mode 100644 core-new/src/operations/locations/index/mod.rs create mode 100644 core-new/src/operations/locations/mod.rs rename core-new/src/operations/{actions/handlers/location_remove.rs => locations/remove/action.rs} (82%) create mode 100644 core-new/src/operations/locations/remove/mod.rs rename core-new/src/operations/{media_processing => media}/mod.rs (100%) create mode 100644 core-new/src/operations/media/thumbnail/action.rs rename core-new/src/operations/{media_processing => media}/thumbnail/error.rs (100%) rename core-new/src/operations/{media_processing => media}/thumbnail/generator.rs (100%) rename core-new/src/operations/{media_processing => media}/thumbnail/job.rs (100%) rename core-new/src/operations/{media_processing => media}/thumbnail/mod.rs (88%) rename core-new/src/operations/{media_processing => media}/thumbnail/state.rs (100%) rename core-new/src/operations/{media_processing => media}/thumbnail/utils.rs (100%) create mode 100644 core-new/src/operations/metadata/action.rs diff --git a/core-new/src/context.rs b/core-new/src/context.rs index 796f7975e..fd4aeb107 100644 --- a/core-new/src/context.rs +++ b/core-new/src/context.rs @@ -5,7 +5,7 @@ use crate::{ device::DeviceManager, infrastructure::events::EventBus, keys::library_key_manager::LibraryKeyManager, library::LibraryManager, - operations::actions::manager::ActionManager, + infrastructure::actions::manager::ActionManager, services::networking::NetworkingService, volume::VolumeManager, }; use std::sync::Arc; diff --git a/core-new/src/infrastructure/actions/BUILDER_REFACTOR_PLAN.md b/core-new/src/infrastructure/actions/BUILDER_REFACTOR_PLAN.md new file mode 100644 index 000000000..116e707d6 --- /dev/null +++ b/core-new/src/infrastructure/actions/BUILDER_REFACTOR_PLAN.md @@ -0,0 +1,484 @@ +# Action Builder Pattern Refactor Plan + +## Overview + +This refactor introduces a consistent builder pattern for Actions to handle CLI/API input parsing while maintaining domain ownership and type safety. This addresses the current inconsistency between Jobs (decentralized) and Actions (centralized enum) patterns. + +## Current State Problems + +1. **Input Handling Gap**: Actions need to convert raw CLI/API input to structured domain types +2. **Pattern Inconsistency**: Jobs use dynamic registration, Actions use central enum +3. **Validation Scattered**: No standardized validation approach for action construction +4. **CLI Integration Missing**: No clear path from CLI args to Action types +5. **Inefficient Job Dispatch**: Actions currently use `dispatch_by_name` with JSON serialization instead of direct job creation + +## Goals + +- Provide fluent builder API for all actions +- Standardize validation at build-time +- Enable seamless CLI/API integration +- Maintain domain ownership of input logic +- Keep serialization compatibility (ActionOutput enum needed like JobOutput) +- Eliminate inefficient `dispatch_by_name` usage in favor of direct job creation + +## Implementation Plan + +### Phase 1: Infrastructure Foundation + +#### 1.1 Create Builder Traits (`src/infrastructure/actions/builder.rs`) + +```rust +pub trait ActionBuilder { + type Action; + type Error: std::error::Error + Send + Sync + 'static; + + fn build(self) -> Result; + fn validate(&self) -> Result<(), Self::Error>; +} + +pub trait CliActionBuilder: ActionBuilder { + type Args: clap::Parser; + + fn from_cli_args(args: Self::Args) -> Self; +} + +#[derive(Debug, thiserror::Error)] +pub enum ActionBuildError { + #[error("Validation errors: {0:?}")] + Validation(Vec), + #[error("IO error: {0}")] + Io(#[from] std::io::Error), + #[error("Parse error: {0}")] + Parse(String), + #[error("Permission denied: {0}")] + Permission(String), +} +``` + +#### 1.2 Create ActionOutput Enum (`src/infrastructure/actions/output.rs`) + +```rust +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(tag = "type", content = "data")] +pub enum ActionOutput { + /// Action completed successfully with no specific output + Success, + + /// Library creation output + LibraryCreate { + library_id: Uuid, + name: String, + }, + + /// Library deletion output + LibraryDelete { + library_id: Uuid, + }, + + /// Folder creation output + FolderCreate { + folder_id: Uuid, + path: PathBuf, + }, + + /// File copy dispatch output (action just dispatches to job) + FileCopyDispatched { + job_id: Uuid, + sources_count: usize, + }, + + /// File delete dispatch output + FileDeleteDispatched { + job_id: Uuid, + targets_count: usize, + }, + + /// Location management outputs + LocationAdd { + location_id: Uuid, + path: PathBuf, + }, + + LocationRemove { + location_id: Uuid, + }, + + /// Generic output with custom data + Custom(serde_json::Value), +} + +impl ActionOutput { + pub fn custom(data: T) -> Self { + Self::Custom(serde_json::to_value(data).unwrap_or(serde_json::Value::Null)) + } +} + +impl Default for ActionOutput { + fn default() -> Self { + Self::Success + } +} +``` + +#### 1.3 Update ActionHandler trait (`src/infrastructure/actions/handler.rs`) + +```rust +#[async_trait] +pub trait ActionHandler: Send + Sync { + async fn validate( + &self, + context: Arc, + action: &Action, + ) -> ActionResult<()>; + + async fn execute( + &self, + context: Arc, + action: Action, + ) -> ActionResult; // Change from ActionReceipt to ActionOutput + + fn can_handle(&self, action: &Action) -> bool; + fn supported_actions() -> &'static [&'static str]; +} +``` + +### Phase 2: Domain Builder Implementation + +For each domain, implement the builder pattern following this template: + +#### 2.1 File Copy Action Builder (`src/operations/files/copy/action.rs`) + +```rust +pub struct FileCopyActionBuilder { + sources: Vec, + destination: Option, + options: CopyOptions, + errors: Vec, +} + +impl FileCopyActionBuilder { + pub fn new() -> Self { /* ... */ } + + // Fluent API methods + pub fn sources(mut self, sources: I) -> Self { /* ... */ } + pub fn source>(mut self, source: P) -> Self { /* ... */ } + pub fn destination>(mut self, dest: P) -> Self { /* ... */ } + pub fn overwrite(mut self, overwrite: bool) -> Self { /* ... */ } + pub fn verify_checksum(mut self, verify: bool) -> Self { /* ... */ } + pub fn preserve_timestamps(mut self, preserve: bool) -> Self { /* ... */ } + pub fn move_files(mut self) -> Self { /* ... */ } + + // Validation methods + fn validate_sources(&mut self) { /* ... */ } + fn validate_destination(&mut self) { /* ... */ } +} + +impl ActionBuilder for FileCopyActionBuilder { + type Action = FileCopyAction; + type Error = ActionBuildError; + + fn validate(&self) -> Result<(), Self::Error> { /* ... */ } + fn build(self) -> Result { /* ... */ } +} + +#[derive(clap::Parser)] +pub struct FileCopyArgs { + pub sources: Vec, + #[arg(short, long)] + pub destination: PathBuf, + #[arg(long)] + pub overwrite: bool, + #[arg(long)] + pub verify: bool, + #[arg(long, default_value = "true")] + pub preserve_timestamps: bool, + #[arg(long)] + pub move_files: bool, +} + +impl CliActionBuilder for FileCopyActionBuilder { + type Args = FileCopyArgs; + + fn from_cli_args(args: Self::Args) -> Self { /* ... */ } +} + +// Convenience methods on the action +impl FileCopyAction { + pub fn builder() -> FileCopyActionBuilder { /* ... */ } + pub fn copy_file, D: Into>(source: S, dest: D) -> FileCopyActionBuilder { /* ... */ } + pub fn copy_files(sources: I, dest: D) -> FileCopyActionBuilder { /* ... */ } +} +``` + +#### 2.2 Domain Handler Updates + +Update each action handler to return `ActionOutput` instead of `ActionReceipt` and use direct job dispatch: + +```rust +impl ActionHandler for FileCopyHandler { + async fn execute( + &self, + context: Arc, + action: Action, + ) -> ActionResult { + if let Action::FileCopy { library_id, action } = action { + // Create job instance directly (no JSON roundtrip) + let sources = action.sources + .into_iter() + .map(|path| SdPath::local(path)) + .collect(); + + let job = FileCopyJob::new( + SdPathBatch::new(sources), + SdPath::local(action.destination) + ).with_options(action.options); + + // Dispatch job directly + let job_handle = library.jobs().dispatch(job).await?; + + // Return action output instead of receipt + Ok(ActionOutput::FileCopyDispatched { + job_id: job_handle.id(), + sources_count: action.sources.len(), + }) + } else { + Err(ActionError::InvalidActionType) + } + } +} +``` + +### Phase 3: CLI Integration + +#### 3.1 Create CLI Action Router (`src/infrastructure/actions/cli.rs`) + +```rust +pub struct ActionCliRouter; + +impl ActionCliRouter { + pub fn route_and_build(command: &str, args: Vec) -> Result { + match command { + "copy" => { + let args = FileCopyArgs::try_parse_from(args)?; + let action = FileCopyActionBuilder::from_cli_args(args).build()?; + Ok(Action::FileCopy { + library_id: get_current_library_id()?, + action + }) + } + "delete" => { + let args = FileDeleteArgs::try_parse_from(args)?; + let action = FileDeleteActionBuilder::from_cli_args(args).build()?; + Ok(Action::FileDelete { + library_id: get_current_library_id()?, + action + }) + } + // ... other commands + _ => Err(ActionBuildError::Parse(format!("Unknown command: {}", command))) + } + } +} +``` + +#### 3.2 Update CLI Binary (`src/bin/cli.rs`) + +```rust +#[derive(clap::Parser)] +enum Commands { + Copy(FileCopyArgs), + Delete(FileDeleteArgs), + // ... other commands +} + +async fn main() -> Result<()> { + let cli = Cli::parse(); + + let action = match cli.command { + Commands::Copy(args) => { + let library_id = get_current_library_id()?; + let action = FileCopyActionBuilder::from_cli_args(args).build()?; + Action::FileCopy { library_id, action } + } + Commands::Delete(args) => { + let library_id = get_current_library_id()?; + let action = FileDeleteActionBuilder::from_cli_args(args).build()?; + Action::FileDelete { library_id, action } + } + // ... + }; + + let context = create_core_context().await?; + let output = context.action_manager().execute(action).await?; + + println!("{}", output); // ActionOutput implements Display + Ok(()) +} +``` + +### Phase 4: API Integration + +#### 4.1 Create API Action Parser (`src/infrastructure/actions/api.rs`) + +```rust +pub struct ActionApiParser; + +impl ActionApiParser { + pub fn parse_request( + action_type: &str, + params: serde_json::Value, + library_id: Option + ) -> Result { + match action_type { + "file.copy" => { + let mut builder = FileCopyActionBuilder::new(); + + if let Some(sources) = params.get("sources").and_then(|v| v.as_array()) { + let sources: Result, _> = sources + .iter() + .map(|v| v.as_str().ok_or("Invalid source").map(PathBuf::from)) + .collect(); + builder = builder.sources(sources?); + } + + if let Some(dest) = params.get("destination").and_then(|v| v.as_str()) { + builder = builder.destination(dest); + } + + if let Some(overwrite) = params.get("overwrite").and_then(|v| v.as_bool()) { + builder = builder.overwrite(overwrite); + } + + let action = builder.build()?; + Ok(Action::FileCopy { + library_id: library_id.ok_or_else(|| ActionBuildError::Parse("Library ID required".into()))?, + action + }) + } + // ... other action types + _ => Err(ActionBuildError::Parse(format!("Unknown action type: {}", action_type))) + } + } +} +``` + +### Phase 5: Testing Updates + +#### 5.1 Builder Tests (`src/operations/files/copy/action.rs`) + +```rust +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_builder_fluent_api() { + let action = FileCopyAction::builder() + .sources(["/src/file1.txt", "/src/file2.txt"]) + .destination("/dest/") + .overwrite(true) + .verify_checksum(true) + .build() + .unwrap(); + + assert_eq!(action.sources.len(), 2); + assert_eq!(action.destination, PathBuf::from("/dest/")); + assert!(action.options.overwrite); + assert!(action.options.verify_checksum); + } + + #[test] + fn test_builder_validation() { + let result = FileCopyAction::builder() + .sources(Vec::::new()) // Empty sources should fail + .destination("/dest/") + .build(); + + assert!(result.is_err()); + match result.unwrap_err() { + ActionBuildError::Validation(errors) => { + assert!(errors.iter().any(|e| e.contains("At least one source"))); + } + _ => panic!("Expected validation error"), + } + } + + #[test] + fn test_cli_integration() { + let args = FileCopyArgs { + sources: vec!["/src/file.txt".into()], + destination: "/dest/".into(), + overwrite: true, + verify: false, + preserve_timestamps: true, + move_files: false, + }; + + let action = FileCopyActionBuilder::from_cli_args(args).build().unwrap(); + assert_eq!(action.sources, vec![PathBuf::from("/src/file.txt")]); + assert_eq!(action.destination, PathBuf::from("/dest/")); + assert!(action.options.overwrite); + } +} +``` + +#### 5.2 Integration Tests (`tests/action_builder_test.rs`) + +```rust +#[tokio::test] +async fn test_action_execution_with_builder() { + let context = create_test_context().await; + + let action = FileCopyAction::builder() + .source("/test/source.txt") + .destination("/test/dest.txt") + .overwrite(true) + .build() + .unwrap(); + + let full_action = Action::FileCopy { + library_id: test_library_id(), + action, + }; + + let output = context.action_manager().execute(full_action).await.unwrap(); + + match output { + ActionOutput::FileCopyDispatched { job_id, sources_count } => { + assert_eq!(sources_count, 1); + assert!(!job_id.is_nil()); + } + _ => panic!("Expected FileCopyDispatched output"), + } +} +``` + +## Migration Steps + +1. **Create infrastructure** (Phase 1) +2. **Implement FileCopyActionBuilder** as proof of concept +3. **Update FileCopyHandler** to use ActionOutput +4. **Test CLI integration** with file copy +5. **Implement remaining domain builders** (FileDelete, LocationAdd, etc.) +6. **Update all handlers** to use ActionOutput +7. **Complete CLI integration** for all actions +8. **Add API integration** +9. **Update tests** throughout + +## Benefits + +- **Type Safety**: Build-time validation prevents invalid actions +- **Fluent API**: Easy to use programmatically and from CLI/API +- **Domain Ownership**: Each domain controls its input logic +- **Consistency**: Matches job pattern for serialization needs +- **Extensibility**: Easy to add new actions without infrastructure changes +- **CLI/API Ready**: Direct integration path from external inputs +- **Performance**: Eliminates JSON serialization overhead from `dispatch_by_name` +- **Direct Job Creation**: Actions create job instances directly for better type safety and efficiency + +## Backwards Compatibility + +- Existing `Action` enum structure remains unchanged +- Current action handlers work with minor output type changes +- Builders are additive - existing construction methods still work +- Migration can be done incrementally, domain by domain \ No newline at end of file diff --git a/core-new/src/operations/actions/error.rs b/core-new/src/infrastructure/actions/error.rs similarity index 100% rename from core-new/src/operations/actions/error.rs rename to core-new/src/infrastructure/actions/error.rs diff --git a/core-new/src/operations/actions/handler.rs b/core-new/src/infrastructure/actions/handler.rs similarity index 100% rename from core-new/src/operations/actions/handler.rs rename to core-new/src/infrastructure/actions/handler.rs diff --git a/core-new/src/operations/actions/manager.rs b/core-new/src/infrastructure/actions/manager.rs similarity index 82% rename from core-new/src/operations/actions/manager.rs rename to core-new/src/infrastructure/actions/manager.rs index 8bae14961..6f929c31e 100644 --- a/core-new/src/operations/actions/manager.rs +++ b/core-new/src/infrastructure/actions/manager.rs @@ -26,7 +26,6 @@ impl ActionManager { /// Dispatch an action for execution pub async fn dispatch( &self, - library_id: Uuid, action: Action, ) -> ActionResult { // 1. Find the correct handler in the registry @@ -37,14 +36,20 @@ impl ActionManager { // 2. Validate the action handler.validate(self.context.clone(), &action).await?; - // 3. Create the initial audit log entry - let audit_entry = self.create_audit_log(library_id, &action).await?; + // 3. Create the initial audit log entry (if library-scoped) + let audit_entry = if let Some(library_id) = action.library_id() { + Some(self.create_audit_log(library_id, &action).await?) + } else { + None + }; // 4. Execute the handler let result = handler.execute(self.context.clone(), action).await; - // 5. Update the audit log with the final status - self.finalize_audit_log(audit_entry, &result).await?; + // 5. Update the audit log with the final status (if we created one) + if let Some(entry) = audit_entry { + self.finalize_audit_log(entry, &result).await?; + } result } @@ -84,8 +89,14 @@ impl ActionManager { mut entry: audit_log::Model, result: &ActionResult, ) -> ActionResult<()> { - let library_id = self.determine_library_id(&entry)?; - let library = self.get_library(library_id).await?; + // We need to get the library_id to update the audit log + // For now, we'll need to store this in the audit entry or pass it through + // This is a temporary limitation that would be resolved by adding library_id to audit_log table + + // For this implementation, let's assume we can find any open library for the audit update + let libraries = self.context.library_manager.get_open_libraries().await; + let library = libraries.first() + .ok_or(ActionError::Internal("No libraries available for audit log update".to_string()))?; let db = library.db().conn(); match result { @@ -120,14 +131,6 @@ impl ActionManager { .ok_or(ActionError::LibraryNotFound(library_id)) } - /// Determine library ID from audit entry (placeholder for now) - fn determine_library_id(&self, _entry: &audit_log::Model) -> ActionResult { - // For now, we'll need to track this in the audit entry or pass it through - // This is a simplification - in practice we'd need to store library_id in audit_log - Err(ActionError::Internal( - "Library ID determination not implemented".to_string(), - )) - } /// Get action history for a library pub async fn get_action_history( diff --git a/core-new/src/infrastructure/actions/mod.rs b/core-new/src/infrastructure/actions/mod.rs new file mode 100644 index 000000000..ccc412e00 --- /dev/null +++ b/core-new/src/infrastructure/actions/mod.rs @@ -0,0 +1,215 @@ +//! Action System - User-initiated operations with audit logging +//! +//! This module provides a centralized, robust, and extensible layer for handling +//! all user-initiated operations. It serves as the primary integration point +//! for the CLI and future APIs. + +use crate::shared::types::SdPath; +use serde::{Deserialize, Serialize}; +use std::path::PathBuf; +use uuid::Uuid; + +pub mod error; +pub mod handler; +pub mod manager; +pub mod receipt; +pub mod registry; +#[cfg(test)] +mod tests; + + +/// Represents a user-initiated action within Spacedrive. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub enum Action { + // Global actions (no library context) + LibraryCreate(crate::operations::libraries::create::action::LibraryCreateAction), + LibraryDelete(crate::operations::libraries::delete::action::LibraryDeleteAction), + + // Library-scoped actions (require library_id) + FileCopy { + library_id: Uuid, + action: crate::operations::files::copy::action::FileCopyAction + }, + FileDelete { + library_id: Uuid, + action: crate::operations::files::delete::action::FileDeleteAction + }, + FileValidate { + library_id: Uuid, + action: crate::operations::files::validation::ValidationAction + }, + DetectDuplicates { + library_id: Uuid, + action: crate::operations::files::duplicate_detection::DuplicateDetectionAction + }, + + LocationAdd { + library_id: Uuid, + action: crate::operations::locations::add::action::LocationAddAction + }, + LocationRemove { + library_id: Uuid, + action: crate::operations::locations::remove::action::LocationRemoveAction + }, + LocationIndex { + library_id: Uuid, + action: crate::operations::locations::index::action::LocationIndexAction + }, + + Index { + library_id: Uuid, + action: crate::operations::indexing::action::IndexingAction + }, + + GenerateThumbnails { + library_id: Uuid, + action: crate::operations::media::thumbnail::action::ThumbnailAction + }, + + ContentAnalysis { + library_id: Uuid, + action: crate::operations::content::action::ContentAction + }, + + MetadataOperation { + library_id: Uuid, + action: crate::operations::metadata::action::MetadataAction + }, +} + +impl Action { + /// Returns the library ID for library-scoped actions + pub fn library_id(&self) -> Option { + match self { + Action::LibraryCreate(_) | Action::LibraryDelete(_) => None, + Action::FileCopy { library_id, .. } => Some(*library_id), + Action::FileDelete { library_id, .. } => Some(*library_id), + Action::FileValidate { library_id, .. } => Some(*library_id), + Action::DetectDuplicates { library_id, .. } => Some(*library_id), + Action::LocationAdd { library_id, .. } => Some(*library_id), + Action::LocationRemove { library_id, .. } => Some(*library_id), + Action::LocationIndex { library_id, .. } => Some(*library_id), + Action::Index { library_id, .. } => Some(*library_id), + Action::GenerateThumbnails { library_id, .. } => Some(*library_id), + Action::ContentAnalysis { library_id, .. } => Some(*library_id), + Action::MetadataOperation { library_id, .. } => Some(*library_id), + } + } + + /// Returns a string identifier for the action type. + pub fn kind(&self) -> &'static str { + match self { + Action::LibraryCreate(_) => "library.create", + Action::LibraryDelete(_) => "library.delete", + Action::FileCopy { .. } => "file.copy", + Action::FileDelete { .. } => "file.delete", + Action::FileValidate { .. } => "file.validate", + Action::DetectDuplicates { .. } => "file.detect_duplicates", + Action::LocationAdd { .. } => "location.add", + Action::LocationRemove { .. } => "location.remove", + Action::LocationIndex { .. } => "location.index", + Action::Index { .. } => "indexing.index", + Action::GenerateThumbnails { .. } => "media.thumbnail", + Action::ContentAnalysis { .. } => "content.analyze", + Action::MetadataOperation { .. } => "metadata.extract", + } + } + + /// Returns a human-readable description of the action + pub fn description(&self) -> String { + match self { + Action::LibraryCreate(action) => { + format!("Create library '{}'", action.name) + } + Action::LibraryDelete(_action) => { + "Delete library".to_string() + } + Action::FileCopy { action, .. } => { + format!( + "Copy {} file(s) to {}", + action.sources.len(), + action.destination.display() + ) + } + Action::FileDelete { action, .. } => { + format!("Delete {} file(s)", action.targets.len()) + } + Action::FileValidate { action, .. } => { + format!("Validate {} file(s)", action.paths.len()) + } + Action::DetectDuplicates { action, .. } => { + format!("Detect duplicates in {} path(s)", action.paths.len()) + } + Action::LocationAdd { action, .. } => match &action.name { + Some(name) => format!("Add location '{}' at {}", name, action.path.display()), + None => format!("Add location at {}", action.path.display()), + }, + Action::LocationRemove { action, .. } => { + format!("Remove location {}", action.location_id) + } + Action::LocationIndex { action, .. } => { + format!("Index location {} ({:?})", action.location_id, action.mode) + } + Action::Index { action, .. } => { + format!("Index {} path(s)", action.paths.len()) + } + Action::GenerateThumbnails { action, .. } => { + format!("Generate thumbnails for {} file(s)", action.paths.len()) + } + Action::ContentAnalysis { action, .. } => { + format!("Analyze content of {} file(s)", action.paths.len()) + } + Action::MetadataOperation { action, .. } => { + format!("Extract metadata from {} file(s)", action.paths.len()) + } + } + } + + /// Returns target summary for audit logging + pub fn targets_summary(&self) -> serde_json::Value { + match self { + Action::LibraryCreate(action) => serde_json::json!({ + "name": action.name, + "path": action.path.as_ref().map(|p| p.display().to_string()) + }), + Action::LibraryDelete(_action) => serde_json::json!({}), + Action::FileCopy { action, .. } => serde_json::json!({ + "sources": action.sources.iter().map(|s| s.display().to_string()).collect::>(), + "destination": action.destination.display().to_string() + }), + Action::FileDelete { action, .. } => serde_json::json!({ + "targets": action.targets.iter().map(|t| t.display().to_string()).collect::>() + }), + Action::FileValidate { action, .. } => serde_json::json!({ + "paths": action.paths.iter().map(|p| p.display().to_string()).collect::>() + }), + Action::DetectDuplicates { action, .. } => serde_json::json!({ + "paths": action.paths.iter().map(|p| p.display().to_string()).collect::>() + }), + Action::LocationAdd { action, .. } => serde_json::json!({ + "path": action.path.display().to_string(), + "name": action.name, + "mode": action.mode + }), + Action::LocationRemove { action, .. } => serde_json::json!({ + "location_id": action.location_id + }), + Action::LocationIndex { action, .. } => serde_json::json!({ + "location_id": action.location_id, + "mode": action.mode + }), + Action::Index { action, .. } => serde_json::json!({ + "paths": action.paths.iter().map(|p| p.display().to_string()).collect::>() + }), + Action::GenerateThumbnails { action, .. } => serde_json::json!({ + "paths": action.paths.iter().map(|p| p.display().to_string()).collect::>() + }), + Action::ContentAnalysis { action, .. } => serde_json::json!({ + "paths": action.paths.iter().map(|p| p.display().to_string()).collect::>() + }), + Action::MetadataOperation { action, .. } => serde_json::json!({ + "paths": action.paths.iter().map(|p| p.display().to_string()).collect::>() + }), + } + } +} diff --git a/core-new/src/operations/actions/receipt.rs b/core-new/src/infrastructure/actions/receipt.rs similarity index 100% rename from core-new/src/operations/actions/receipt.rs rename to core-new/src/infrastructure/actions/receipt.rs diff --git a/core-new/src/operations/actions/registry.rs b/core-new/src/infrastructure/actions/registry.rs similarity index 96% rename from core-new/src/operations/actions/registry.rs rename to core-new/src/infrastructure/actions/registry.rs index 407fa56c6..e9278085a 100644 --- a/core-new/src/operations/actions/registry.rs +++ b/core-new/src/infrastructure/actions/registry.rs @@ -60,7 +60,7 @@ pub static REGISTRY: Lazy = Lazy::new(ActionRegistry::new); macro_rules! register_action_handler { ($handler_type:ty, $action_kind:expr) => { inventory::submit! { - $crate::operations::actions::registry::ActionRegistration { + $crate::infrastructure::actions::registry::ActionRegistration { name: $action_kind, create_fn: || Box::new(<$handler_type>::new()), } diff --git a/core-new/src/operations/actions/tests.rs b/core-new/src/infrastructure/actions/tests.rs similarity index 65% rename from core-new/src/operations/actions/tests.rs rename to core-new/src/infrastructure/actions/tests.rs index bdee09ee8..e27b51759 100644 --- a/core-new/src/operations/actions/tests.rs +++ b/core-new/src/infrastructure/actions/tests.rs @@ -5,33 +5,39 @@ mod tests { use super::*; use crate::{ context::CoreContext, - operations::actions::{Action, registry::REGISTRY}, + infrastructure::actions::{Action, registry::REGISTRY}, }; #[test] fn test_action_kind() { - let action = Action::LibraryCreate { - name: "Test Library".to_string(), - path: None, - }; + let action = Action::LibraryCreate( + crate::operations::libraries::create::action::LibraryCreateAction { + name: "Test Library".to_string(), + path: None, + } + ); assert_eq!(action.kind(), "library.create"); } #[test] fn test_action_description() { - let action = Action::LibraryCreate { - name: "Test Library".to_string(), - path: None, - }; + let action = Action::LibraryCreate( + crate::operations::libraries::create::action::LibraryCreateAction { + name: "Test Library".to_string(), + path: None, + } + ); assert_eq!(action.description(), "Create library 'Test Library'"); } #[test] fn test_action_targets_summary() { - let action = Action::LibraryCreate { - name: "Test Library".to_string(), - path: Some("/path/to/library".into()), - }; + let action = Action::LibraryCreate( + crate::operations::libraries::create::action::LibraryCreateAction { + name: "Test Library".to_string(), + path: Some("/path/to/library".into()), + } + ); let summary = action.targets_summary(); assert_eq!(summary["name"], "Test Library"); assert_eq!(summary["path"], "/path/to/library"); diff --git a/core-new/src/infrastructure/cli/commands.rs b/core-new/src/infrastructure/cli/commands.rs index f2ad7e074..b5542a9d1 100644 --- a/core-new/src/infrastructure/cli/commands.rs +++ b/core-new/src/infrastructure/cli/commands.rs @@ -3,8 +3,8 @@ use crate::{ infrastructure::{database::entities, jobs::types::JobStatus}, library::Library, location::{create_location, LocationCreateArgs}, + infrastructure::actions::Action, operations::{ - actions::{Action, IndexMode as ActionIndexMode}, indexing::{IndexMode, IndexScope}, }, shared::types::SdPath, @@ -310,15 +310,15 @@ pub async fn handle_library_command( .ok_or("Action manager not available")?; // Create the action - let action = Action::LibraryCreate { - name: name.clone(), - path: path.clone(), - }; + let action = Action::LibraryCreate( + crate::operations::libraries::create::action::LibraryCreateAction { + name: name.clone(), + path: path.clone(), + } + ); // Dispatch the action - // For library creation, we don't have a library_id yet, so we'll use a placeholder - // The action manager will need to handle this case appropriately - match action_manager.dispatch(uuid::Uuid::nil(), action).await { + match action_manager.dispatch(action).await { Ok(receipt) => { if let Some(payload) = receipt.result_payload { if let (Some(lib_id), Some(lib_path)) = ( @@ -495,23 +495,25 @@ pub async fn handle_location_command( .await .ok_or("Action manager not available")?; - // Convert CliIndexMode to ActionIndexMode + // Convert CliIndexMode to IndexMode let action_mode = match mode { - CliIndexMode::Shallow => ActionIndexMode::Shallow, - CliIndexMode::Content => ActionIndexMode::Deep, // Map Content to Deep for now - CliIndexMode::Deep => ActionIndexMode::Deep, + CliIndexMode::Shallow => IndexMode::Shallow, + CliIndexMode::Content => IndexMode::Content, + CliIndexMode::Deep => IndexMode::Deep, }; // Create the action let action = Action::LocationAdd { library_id: library.id(), - path: path.clone(), - name: name.clone(), - mode: action_mode, + action: crate::operations::locations::add::action::LocationAddAction { + path: path.clone(), + name: name.clone(), + mode: action_mode, + } }; // Dispatch the action - match action_manager.dispatch(library.id(), action).await { + match action_manager.dispatch(action).await { Ok(receipt) => { if let Some(payload) = receipt.result_payload { if let Some(location_id) = diff --git a/core-new/src/infrastructure/mod.rs b/core-new/src/infrastructure/mod.rs index b72652d49..c4b9ee328 100644 --- a/core-new/src/infrastructure/mod.rs +++ b/core-new/src/infrastructure/mod.rs @@ -1,5 +1,6 @@ //! Infrastructure layer - external interfaces +pub mod actions; pub mod cli; pub mod database; pub mod events; diff --git a/core-new/src/lib.rs b/core-new/src/lib.rs index b9236d4a9..1f81f5650 100644 --- a/core-new/src/lib.rs +++ b/core-new/src/lib.rs @@ -30,7 +30,7 @@ use crate::context::CoreContext; use crate::device::DeviceManager; use crate::infrastructure::events::{Event, EventBus}; use crate::library::LibraryManager; -use crate::operations::actions::manager::ActionManager; +use crate::infrastructure::actions::manager::ActionManager; use crate::services::Services; use crate::volume::{VolumeDetectionConfig, VolumeManager}; use std::path::PathBuf; @@ -206,7 +206,7 @@ impl Core { } // 12. Initialize ActionManager and set it in context - let action_manager = Arc::new(crate::operations::actions::manager::ActionManager::new(context.clone())); + let action_manager = Arc::new(crate::infrastructure::actions::manager::ActionManager::new(context.clone())); context.set_action_manager(action_manager).await; // 13. Emit startup event diff --git a/core-new/src/library/mod.rs b/core-new/src/library/mod.rs index 19211ead5..316c20470 100644 --- a/core-new/src/library/mod.rs +++ b/core-new/src/library/mod.rs @@ -176,7 +176,7 @@ impl Library { /// Start thumbnail generation job pub async fn generate_thumbnails(&self, entry_ids: Option>) -> Result { - use crate::operations::media_processing::thumbnail::{ThumbnailJob, ThumbnailJobConfig}; + use crate::operations::media::thumbnail::{ThumbnailJob, ThumbnailJobConfig}; let config = ThumbnailJobConfig { sizes: self.config().await.settings.thumbnail_sizes.clone(), diff --git a/core-new/src/operations/REFACTOR_PLAN.md b/core-new/src/operations/REFACTOR_PLAN.md index 5868640fa..d5cc5492a 100644 --- a/core-new/src/operations/REFACTOR_PLAN.md +++ b/core-new/src/operations/REFACTOR_PLAN.md @@ -3,17 +3,20 @@ ## Current Problems ### 1. **Architectural Issues** + - Mixed abstraction levels in `/operations` (high-level actions, low-level jobs, domain logic) - Confusing naming: `file_ops` vs `media_processing` vs `indexing` - Actions are centralized and disconnected from their domains - Audit logs try to determine library context instead of having it explicit ### 2. **Library Context Issues** + - Actions operate at core level but need library-specific audit logging - Current `ActionManager.determine_library_id()` is unimplemented placeholder - No clear separation between global actions (LibraryCreate) and library-scoped actions ### 3. **Domain Modularity Issues** + - Action handlers separated from their domain logic - No clear ownership of business logic per domain - Job naming inconsistency (`delete_job.rs` vs `job.rs` in folders) @@ -21,6 +24,7 @@ ## Target Architecture ### Core Principles + 1. **Domain Modularity**: Each domain owns its complete story (actions + jobs + logic) 2. **Explicit Library Context**: Actions specify library_id when needed 3. **Consistent Structure**: Every domain follows the same pattern @@ -32,18 +36,21 @@ The current `operations/actions/` module should be moved to `infrastructure/actions/` because it provides **framework functionality**, not business logic. This aligns with the existing infrastructure pattern: **Infrastructure modules provide frameworks/systems:** + - `jobs/` - Job execution framework (traits, manager, registry, executor) - `events/` - Event system framework (dispatching, handling) - `database/` - Database access framework (entities, migrations, connections) - `actions/` - Action dispatch and audit framework (manager, registry, audit logging) ✨ **Operations modules provide business logic:** + - `files/` - File operation business logic (what to do with files) - `locations/` - Location management business logic (how to manage locations) - `indexing/` - Indexing business logic (how to index files) - `media/` - Media processing business logic (how to process media) The actions module is pure infrastructure - it doesn't care about the specific business logic of copying files or managing locations. It only provides: + - **ActionManager**: Central dispatch system - **ActionRegistry**: Auto-discovery of action handlers - **ActionHandler trait**: Interface for handling actions @@ -51,6 +58,7 @@ The actions module is pure infrastructure - it doesn't care about the specific b - **Action enum**: Central registry of all available actions This creates a clean separation where: + - **Infrastructure** provides the plumbing (how to dispatch, audit, execute) - **Operations** provides the business logic (what to do with files, locations, etc.) @@ -127,6 +135,7 @@ src/operations/ ## New Action Structure ### Core Action Enum + ```rust // src/infrastructure/actions/mod.rs #[derive(Debug, Clone, Serialize, Deserialize)] @@ -134,56 +143,56 @@ pub enum Action { // Global actions (no library context) LibraryCreate(crate::operations::libraries::create::LibraryCreateAction), LibraryDelete(crate::operations::libraries::delete::LibraryDeleteAction), - + // Library-scoped actions (require library_id) - FileCopy { - library_id: Uuid, - action: crate::operations::files::copy::FileCopyAction + FileCopy { + library_id: Uuid, + action: crate::operations::files::copy::FileCopyAction }, - FileDelete { - library_id: Uuid, - action: crate::operations::files::delete::FileDeleteAction + FileDelete { + library_id: Uuid, + action: crate::operations::files::delete::FileDeleteAction }, - FileValidate { - library_id: Uuid, - action: crate::operations::files::validation::ValidationAction + FileValidate { + library_id: Uuid, + action: crate::operations::files::validation::ValidationAction }, - DetectDuplicates { - library_id: Uuid, - action: crate::operations::files::duplicate_detection::DuplicateDetectionAction + DetectDuplicates { + library_id: Uuid, + action: crate::operations::files::duplicate_detection::DuplicateDetectionAction }, - - LocationAdd { - library_id: Uuid, - action: crate::operations::locations::add::LocationAddAction + + LocationAdd { + library_id: Uuid, + action: crate::operations::locations::add::LocationAddAction }, - LocationRemove { - library_id: Uuid, - action: crate::operations::locations::remove::LocationRemoveAction + LocationRemove { + library_id: Uuid, + action: crate::operations::locations::remove::LocationRemoveAction }, - LocationIndex { - library_id: Uuid, - action: crate::operations::locations::index::LocationIndexAction + LocationIndex { + library_id: Uuid, + action: crate::operations::locations::index::LocationIndexAction }, - - Index { - library_id: Uuid, - action: crate::operations::indexing::IndexingAction + + Index { + library_id: Uuid, + action: crate::operations::indexing::IndexingAction }, - - GenerateThumbnails { - library_id: Uuid, - action: crate::operations::media::thumbnails::ThumbnailAction + + GenerateThumbnails { + library_id: Uuid, + action: crate::operations::media::thumbnails::ThumbnailAction }, - - ContentAnalysis { - library_id: Uuid, - action: crate::operations::content::ContentAction + + ContentAnalysis { + library_id: Uuid, + action: crate::operations::content::ContentAction }, - - MetadataOperation { - library_id: Uuid, - action: crate::operations::metadata::MetadataAction + + MetadataOperation { + library_id: Uuid, + action: crate::operations::metadata::MetadataAction }, } @@ -208,6 +217,7 @@ impl Action { ``` ### Fixed ActionManager + ```rust // src/infrastructure/actions/manager.rs impl ActionManager { @@ -249,12 +259,15 @@ impl ActionManager { ## Migration Steps ### Phase 1: Move Actions to Infrastructure + 1. **Move actions module**: + ```bash mv src/operations/actions src/infrastructure/actions ``` 2. **Update infrastructure mod.rs**: + ```rust pub mod actions; pub mod cli; @@ -266,7 +279,9 @@ impl ActionManager { 3. **Update imports** throughout codebase from `crate::operations::actions` to `crate::infrastructure::actions` ### Phase 2: Restructure Domains + 1. **Create new domain folders**: + ```bash mkdir -p src/operations/files/{copy,delete,validation,duplicate_detection} mkdir -p src/operations/locations/{add,remove,index} @@ -275,6 +290,7 @@ impl ActionManager { ``` 2. **Move and rename files**: + - `file_ops/delete_job.rs` → `files/delete/job.rs` - `file_ops/validation_job.rs` → `files/validation/job.rs` - `file_ops/duplicate_detection_job.rs` → `files/duplicate_detection/job.rs` @@ -283,7 +299,9 @@ impl ActionManager { 3. **Update imports** throughout codebase ### Phase 3: Extract Domain Actions + 1. **Move action handlers to domains**: + - `infrastructure/actions/handlers/file_copy.rs` → `operations/files/copy/action.rs` - `infrastructure/actions/handlers/file_delete.rs` → `operations/files/delete/action.rs` - `infrastructure/actions/handlers/location_add.rs` → `operations/locations/add/action.rs` @@ -299,29 +317,34 @@ impl ActionManager { - `operations/metadata/action.rs` (NEW) ### Phase 4: Update Core Action System + 1. **Refactor Action enum** to use domain-specific types with explicit library_id 2. **Remove handlers directory** (empty after migration) 3. **Update ActionManager** to use explicit library_id from actions 4. **Fix audit log creation** to use correct library database ### Phase 5: Update CLI Integration + 1. **Update CLI commands** to pass library_id when creating actions: + ```rust // Before let action = Action::FileCopy { sources, destination, options }; - + // After let library_id = cli_app.get_current_library().await?.id(); - let action = Action::FileCopy { - library_id, - action: FileCopyAction { sources, destination, options } + let action = Action::FileCopy { + library_id, + action: FileCopyAction { sources, destination, options } }; ``` 2. **Update command handlers** to work with new action structure ### Phase 6: Update Job Registration + 1. **Update operations/mod.rs** to register jobs from new locations: + ```rust pub fn register_all_jobs() { // File operation jobs @@ -329,7 +352,7 @@ impl ActionManager { register_job::(); register_job::(); register_job::(); - + // Other jobs register_job::(); register_job::(); @@ -337,6 +360,7 @@ impl ActionManager { ``` ### Phase 7: Testing and Validation + 1. **Update all tests** to use new structure 2. **Run action system tests** to ensure functionality preserved 3. **Test CLI integration** with new action structure @@ -345,28 +369,33 @@ impl ActionManager { ## Benefits of This Refactor ### 1. **True Domain Modularity** + - Each domain owns its complete story (actions + jobs + logic) - Want to understand file operations? Everything is in `files/` - Want to add location features? Everything is in `locations/` ### 2. **Clear Library Context** + - Actions explicitly specify which library they operate on - No more guessing or unimplemented library ID determination - Global actions (library management) clearly separated ### 3. **Consistent Structure** + - Every domain follows the same pattern - Complex domains: `domain/operation/{job.rs, action.rs}` - Simple domains: `domain/action.rs` - No more naming inconsistencies ### 4. **Improved Maintainability** + - Related functionality grouped together - Clear boundaries between domains - Easier to test individual domains - Easier to add new domains ### 5. **Better Developer Experience** + - Intuitive navigation of codebase - Clear understanding of action vs job responsibilities - Explicit library context prevents bugs @@ -375,19 +404,429 @@ impl ActionManager { ## Potential Issues and Solutions ### 1. **Breaking Changes** + - **Issue**: This refactor breaks all existing imports - **Solution**: Update imports incrementally, test at each phase ### 2. **CLI Integration** + - **Issue**: CLI needs to pass library_id for all actions - **Solution**: Centralize library ID retrieval in CLI helper functions ### 3. **Action Enum Size** + - **Issue**: Action enum becomes quite large - **Solution**: This is acceptable for explicit typing, improves type safety ### 4. **Migration Complexity** + - **Issue**: Large number of files to move and update - **Solution**: Migrate in phases, ensure tests pass at each step -This refactor transforms the operations module from a confusing mix of concerns into a clean, domain-driven architecture where each domain owns its complete functionality and library context is explicit throughout the system. \ No newline at end of file +This refactor transforms the operations module from a confusing mix of concerns into a clean, domain-driven architecture where each domain owns its complete functionality and library context is explicit throughout the system. + +## Example: + +Here's how src/operations/libraries/create/action.rs would look following the Builder Refactor +Plan: + +```rust + //! Library creation action handler + + use crate::{ + context::CoreContext, + infrastructure::actions::{ + builder::{ActionBuilder, ActionBuildError, CliActionBuilder}, + error::{ActionError, ActionResult}, + handler::ActionHandler, + output::ActionOutput, + Action, + }, + register_action_handler, + }; + use async_trait::async_trait; + use clap::Parser; + use serde::{Deserialize, Serialize}; + use std::{path::PathBuf, sync::Arc}; + use uuid::Uuid; + + #[derive(Debug, Clone, Serialize, Deserialize)] + pub struct LibraryCreateAction { + pub name: String, + pub path: Option, + } + + // Builder implementation + pub struct LibraryCreateActionBuilder { + name: Option, + path: Option, + errors: Vec, + } + + impl LibraryCreateActionBuilder { + pub fn new() -> Self { + Self { + name: None, + path: None, + errors: Vec::new(), + } + } + + // Fluent API methods + pub fn name>(mut self, name: S) -> Self { + self.name = Some(name.into()); + self + } + + pub fn path>(mut self, path: P) -> Self { + self.path = Some(path.into()); + self + } + + pub fn auto_path(mut self) -> Self { + // Use default library path based on OS conventions + self.path = Some(Self::default_library_path()); + self + } + + // Validation methods + fn validate_name(&mut self) { + if let Some(ref name) = self.name { + if name.trim().is_empty() { + self.errors.push("Library name cannot be empty".to_string()); + } + if name.len() > 255 { + self.errors.push("Library name cannot exceed 255 characters".to_string()); + } + if name.contains(['/', '\\', ':', '*', '?', '"', '<', '>', '|']) { + self.errors.push("Library name contains invalid characters".to_string()); + } + } else { + self.errors.push("Library name is required".to_string()); + } + } + + fn validate_path(&mut self) { + if let Some(ref path) = self.path { + if let Some(parent) = path.parent() { + if !parent.exists() { + self.errors.push(format!( + "Parent directory does not exist: {}", + parent.display() + )); + } + if !parent.metadata().map_or(false, |m| m.permissions().readonly()) { + // Check if we can write to the parent directory + } + } + } + } + + fn default_library_path() -> PathBuf { + #[cfg(target_os = "macos")] + { + dirs::home_dir() + .unwrap_or_else(|| PathBuf::from("/tmp")) + .join("Library/Application Support/Spacedrive") + } + #[cfg(target_os = "windows")] + { + dirs::data_dir() + .unwrap_or_else(|| PathBuf::from("C:\\ProgramData")) + .join("Spacedrive") + } + #[cfg(target_os = "linux")] + { + dirs::data_dir() + .unwrap_or_else(|| PathBuf::from("/tmp")) + .join("spacedrive") + } + } + } + + impl ActionBuilder for LibraryCreateActionBuilder { + type Action = LibraryCreateAction; + type Error = ActionBuildError; + + fn validate(&self) -> Result<(), Self::Error> { + let mut builder = self.clone(); + builder.validate_name(); + builder.validate_path(); + + if !builder.errors.is_empty() { + return Err(ActionBuildError::Validation(builder.errors)); + } + + Ok(()) + } + + fn build(self) -> Result { + self.validate()?; + + Ok(LibraryCreateAction { + name: self.name.unwrap(), // Safe after validation + path: self.path, + }) + } + } + + // CLI Integration + #[derive(Parser)] + pub struct LibraryCreateArgs { + /// Name for the new library + pub name: String, + + /// Path where the library should be created + #[arg(short, long)] + pub path: Option, + + /// Use automatic path based on OS conventions + #[arg(long)] + pub auto_path: bool, + } + + impl CliActionBuilder for LibraryCreateActionBuilder { + type Args = LibraryCreateArgs; + + fn from_cli_args(args: Self::Args) -> Self { + let mut builder = Self::new().name(args.name); + + if args.auto_path { + builder = builder.auto_path(); + } else if let Some(path) = args.path { + builder = builder.path(path); + } + + builder + } + } + + // Convenience methods on the action + impl LibraryCreateAction { + pub fn builder() -> LibraryCreateActionBuilder { + LibraryCreateActionBuilder::new() + } + + /// Quick constructor for library with auto path + pub fn new_auto>(name: S) -> LibraryCreateActionBuilder { + Self::builder().name(name).auto_path() + } + + /// Quick constructor for library with custom path + pub fn new_at, P: Into>( + name: S, + path: P, + ) -> LibraryCreateActionBuilder { + Self::builder().name(name).path(path) + } + } + + // Handler implementation + pub struct LibraryCreateHandler; + + impl LibraryCreateHandler { + pub fn new() -> Self { + Self + } + } + + #[async_trait] + impl ActionHandler for LibraryCreateHandler { + async fn validate( + &self, + _context: Arc, + action: &Action, + ) -> ActionResult<()> { + if let Action::LibraryCreate(action) = action { + // Additional runtime validation (builder already did static validation) + if action.name.trim().is_empty() { + return Err(ActionError::Validation { + field: "name".to_string(), + message: "Library name cannot be empty".to_string(), + }); + } + + // Check if library name already exists + // TODO: Implement library name uniqueness check + + Ok(()) + } else { + Err(ActionError::InvalidActionType) + } + } + + async fn execute( + &self, + context: Arc, + action: Action, + ) -> ActionResult { + if let Action::LibraryCreate(action) = action { + let library_manager = &context.library_manager; + + // Create the library (this is an immediate operation, not a background job) + let new_library = library_manager + .create_library(action.name.clone(), action.path.clone()) + .await + .map_err(|e| ActionError::Internal(e.to_string()))?; + + // Return structured output instead of generic JSON + Ok(ActionOutput::LibraryCreate { + library_id: new_library.id(), + name: action.name, + }) + } else { + Err(ActionError::InvalidActionType) + } + } + + fn can_handle(&self, action: &Action) -> bool { + matches!(action, Action::LibraryCreate(_)) + } + + fn supported_actions() -> &'static [&'static str] { + &["library.create"] + } + } + + // Register this handler + register_action_handler!(LibraryCreateHandler, "library.create"); + + #[cfg(test)] + mod tests { + use super::*; + + #[test] + fn test_builder_fluent_api() { + let action = LibraryCreateAction::builder() + .name("My Library") + .path("/home/user/libraries/my-library") + .build() + .unwrap(); + + assert_eq!(action.name, "My Library"); + assert_eq!(action.path, Some(PathBuf::from("/home/user/libraries/my-library"))); + } + + #[test] + fn test_builder_validation() { + // Empty name should fail + let result = LibraryCreateAction::builder() + .name("") + .build(); + + assert!(result.is_err()); + match result.unwrap_err() { + ActionBuildError::Validation(errors) => { + assert!(errors.iter().any(|e| e.contains("cannot be empty"))); + } + _ => panic!("Expected validation error"), + } + + // Invalid characters should fail + let result = LibraryCreateAction::builder() + .name("Library/With*Invalid:Characters") + .build(); + + assert!(result.is_err()); + } + + #[test] + fn test_cli_integration() { + let args = LibraryCreateArgs { + name: "Test Library".to_string(), + path: Some("/custom/path".into()), + auto_path: false, + }; + + let action = LibraryCreateActionBuilder::from_cli_args(args).build().unwrap(); + assert_eq!(action.name, "Test Library"); + assert_eq!(action.path, Some(PathBuf::from("/custom/path"))); + } + + #[test] + fn test_auto_path() { + let args = LibraryCreateArgs { + name: "Test Library".to_string(), + path: None, + auto_path: true, + }; + + let action = LibraryCreateActionBuilder::from_cli_args(args).build().unwrap(); + assert_eq!(action.name, "Test Library"); + assert!(action.path.is_some()); // Should have auto-generated path + } + + #[test] + fn test_convenience_constructors() { + // Auto path constructor + let action = LibraryCreateAction::new_auto("Auto Library").build().unwrap(); + assert_eq!(action.name, "Auto Library"); + assert!(action.path.is_some()); + + // Custom path constructor + let action = LibraryCreateAction::new_at("Custom Library", "/custom/path") + .build() + .unwrap(); + assert_eq!(action.name, "Custom Library"); + assert_eq!(action.path, Some(PathBuf::from("/custom/path"))); + } + } +``` + +Key Features Added + +1. Builder Pattern + +```rust + let action = LibraryCreateAction::builder() + .name("My Library") + .path("/custom/path") + .build()?; +``` + +2. CLI Integration + +```rust + #[derive(Parser)] + pub struct LibraryCreateArgs { + pub name: String, + #[arg(short, long)] + pub path: Option, + #[arg(long)] + pub auto_path: bool, + } +``` + +3. Validation at Build Time + +- Empty name validation +- Invalid character checking +- Path existence validation +- Length limits + +4. Convenience Methods + +```rust + // Quick constructors + LibraryCreateAction::new_auto("Library Name") + LibraryCreateAction::new_at("Library Name", "/path") +``` + +5. Structured Output + +```rust + Ok(ActionOutput::LibraryCreate { + library_id: new_library.id(), + name: action.name, + }) +``` + +6. Comprehensive Tests + +- Builder validation +- CLI argument parsing +- Fluent API usage +- Convenience constructors + +This follows all the patterns from the refactor plan while being specifically tailored to +library creation needs! diff --git a/core-new/src/operations/actions/handlers/file_copy.rs b/core-new/src/operations/actions/handlers/file_copy.rs deleted file mode 100644 index ce205f600..000000000 --- a/core-new/src/operations/actions/handlers/file_copy.rs +++ /dev/null @@ -1,105 +0,0 @@ -//! File copy action handler - -use crate::{ - context::CoreContext, - operations::actions::{ - Action, error::{ActionError, ActionResult}, handler::ActionHandler, receipt::ActionReceipt, - }, - register_action_handler, -}; -use async_trait::async_trait; -use std::sync::Arc; -use uuid::Uuid; - -pub struct FileCopyHandler; - -impl FileCopyHandler { - pub fn new() -> Self { - Self - } -} - -#[async_trait] -impl ActionHandler for FileCopyHandler { - async fn validate( - &self, - _context: Arc, - action: &Action, - ) -> ActionResult<()> { - if let Action::FileCopy { sources, destination, .. } = action { - if sources.is_empty() { - return Err(ActionError::Validation { - field: "sources".to_string(), - message: "At least one source file must be specified".to_string(), - }); - } - - // Additional validation could include: - // - Check if source files exist - // - Check permissions - // - Check if destination is valid - // - Check if it would be a cross-device operation - - Ok(()) - } else { - Err(ActionError::InvalidActionType) - } - } - - async fn execute( - &self, - context: Arc, - action: Action, - ) -> ActionResult { - if let Action::FileCopy { sources, destination, options } = action { - // Get the appropriate library (we'll need to determine this from the sources) - // For now, let's assume we have a method to get the library manager - let library_manager = &context.library_manager; - - // We need to determine which library this operation should run in - // This could be determined by the source paths or passed explicitly - // For now, let's use a placeholder approach - - // Convert our action options to job options - let job_params = serde_json::json!({ - "sources": sources, - "destination": destination, - "options": { - "overwrite": options.overwrite, - "verify_checksum": options.verify_integrity, - "preserve_timestamps": options.preserve_attributes, - "delete_after_copy": false, - "move_mode": null - } - }); - - // Get a library to run the job in (this would need proper library resolution) - // For now, let's try to get the first available library or return an error - let libraries = library_manager.get_open_libraries().await; - let library = libraries.first() - .ok_or(ActionError::Internal("No libraries available".to_string()))?; - - // Dispatch the file copy job - let job_handle = library - .jobs() - .dispatch_by_name("file_copy", job_params) - .await - .map_err(ActionError::Job)?; - - Ok(ActionReceipt::job_based(Uuid::new_v4(), job_handle)) - } else { - Err(ActionError::InvalidActionType) - } - } - - fn can_handle(&self, action: &Action) -> bool { - matches!(action, Action::FileCopy { .. }) - } - - fn supported_actions() -> &'static [&'static str] { - &["file.copy"] - } -} - -// Register this handler -register_action_handler!(FileCopyHandler, "file.copy"); \ No newline at end of file diff --git a/core-new/src/operations/actions/handlers/library_create.rs b/core-new/src/operations/actions/handlers/library_create.rs deleted file mode 100644 index 9998f9dd7..000000000 --- a/core-new/src/operations/actions/handlers/library_create.rs +++ /dev/null @@ -1,57 +0,0 @@ -//! Library creation action handler - -use crate::{ - context::CoreContext, - operations::actions::{ - Action, error::ActionResult, handler::ActionHandler, receipt::ActionReceipt, - }, - register_action_handler, -}; -use async_trait::async_trait; -use std::sync::Arc; -use uuid::Uuid; - -pub struct LibraryCreateHandler; - -impl LibraryCreateHandler { - pub fn new() -> Self { - Self - } -} - -#[async_trait] -impl ActionHandler for LibraryCreateHandler { - async fn execute( - &self, - context: Arc, - action: Action, - ) -> ActionResult { - if let Action::LibraryCreate { name, path } = action { - let library_manager = &context.library_manager; - let new_library = library_manager.create_library(name, path).await?; - - let library_name = new_library.name().await; - Ok(ActionReceipt::immediate( - Uuid::new_v4(), - Some(serde_json::json!({ - "library_id": new_library.id(), - "name": library_name, - "path": new_library.path().display().to_string() - })), - )) - } else { - Err(crate::operations::actions::error::ActionError::InvalidActionType) - } - } - - fn can_handle(&self, action: &Action) -> bool { - matches!(action, Action::LibraryCreate { .. }) - } - - fn supported_actions() -> &'static [&'static str] { - &["library.create"] - } -} - -// Register this handler -register_action_handler!(LibraryCreateHandler, "library.create"); \ No newline at end of file diff --git a/core-new/src/operations/actions/handlers/mod.rs b/core-new/src/operations/actions/handlers/mod.rs deleted file mode 100644 index b74a98758..000000000 --- a/core-new/src/operations/actions/handlers/mod.rs +++ /dev/null @@ -1,18 +0,0 @@ -//! Concrete action handler implementations - -pub mod library_create; -pub mod library_delete; -pub mod file_copy; -pub mod file_delete; -pub mod location_add; -pub mod location_remove; -pub mod location_index; - -// Re-export all handlers -pub use library_create::LibraryCreateHandler; -pub use library_delete::LibraryDeleteHandler; -pub use file_copy::FileCopyHandler; -pub use file_delete::FileDeleteHandler; -pub use location_add::LocationAddHandler; -pub use location_remove::LocationRemoveHandler; -pub use location_index::LocationIndexHandler; \ No newline at end of file diff --git a/core-new/src/operations/actions/mod.rs b/core-new/src/operations/actions/mod.rs deleted file mode 100644 index 87dc22639..000000000 --- a/core-new/src/operations/actions/mod.rs +++ /dev/null @@ -1,205 +0,0 @@ -//! Action System - User-initiated operations with audit logging -//! -//! This module provides a centralized, robust, and extensible layer for handling -//! all user-initiated operations. It serves as the primary integration point -//! for the CLI and future APIs. - -use crate::shared::types::SdPath; -use serde::{Deserialize, Serialize}; -use std::path::PathBuf; -use uuid::Uuid; - -pub mod error; -pub mod handler; -pub mod handlers; -pub mod manager; -pub mod receipt; -pub mod registry; -#[cfg(test)] -mod tests; - -// Import handlers to trigger their registration -use handlers::*; - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct CopyOptions { - pub overwrite: bool, - pub preserve_attributes: bool, - pub verify_integrity: bool, -} - -impl Default for CopyOptions { - fn default() -> Self { - Self { - overwrite: false, - preserve_attributes: true, - verify_integrity: true, - } - } -} - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct DeleteOptions { - pub permanent: bool, - pub recursive: bool, -} - -impl Default for DeleteOptions { - fn default() -> Self { - Self { - permanent: false, - recursive: false, - } - } -} - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub enum IndexMode { - Shallow, - Deep, - Sync, -} - -impl Default for IndexMode { - fn default() -> Self { - Self::Deep - } -} - -/// Represents a user-initiated action within Spacedrive. -#[derive(Debug, Clone, Serialize, Deserialize)] -pub enum Action { - // Job-based file operations - FileCopy { - sources: Vec, - destination: SdPath, - options: CopyOptions, - }, - - FileDelete { - targets: Vec, - options: DeleteOptions, - }, - - // Direct (non-job) actions - LibraryCreate { - name: String, - path: Option, - }, - - LibraryDelete { - library_id: Uuid, - }, - - // Hybrid actions (direct action that dispatches a job) - LocationAdd { - library_id: Uuid, - path: PathBuf, - name: Option, - mode: IndexMode, - }, - - LocationRemove { - library_id: Uuid, - location_id: Uuid, - }, - - LocationIndex { - library_id: Uuid, - location_id: Uuid, - mode: IndexMode, - }, -} - -impl Action { - /// Returns a string identifier for the action type. - pub fn kind(&self) -> &'static str { - match self { - Action::FileCopy { .. } => "file.copy", - Action::FileDelete { .. } => "file.delete", - Action::LibraryCreate { .. } => "library.create", - Action::LibraryDelete { .. } => "library.delete", - Action::LocationAdd { .. } => "location.add", - Action::LocationRemove { .. } => "location.remove", - Action::LocationIndex { .. } => "location.index", - } - } - - /// Returns a human-readable description of the action - pub fn description(&self) -> String { - match self { - Action::FileCopy { - sources, - destination, - .. - } => { - format!( - "Copy {} file(s) to {}", - sources.len(), - destination.display() - ) - } - Action::FileDelete { targets, .. } => { - format!("Delete {} file(s)", targets.len()) - } - Action::LibraryCreate { name, .. } => { - format!("Create library '{}'", name) - } - Action::LibraryDelete { library_id } => { - format!("Delete library {}", library_id) - } - Action::LocationAdd { path, name, .. } => match name { - Some(name) => format!("Add location '{}' at {}", name, path.display()), - None => format!("Add location at {}", path.display()), - }, - Action::LocationRemove { location_id, .. } => { - format!("Remove location {}", location_id) - } - Action::LocationIndex { - location_id, mode, .. - } => { - format!("Index location {} ({:?})", location_id, mode) - } - } - } - - /// Returns target summary for audit logging - pub fn targets_summary(&self) -> serde_json::Value { - match self { - Action::FileCopy { - sources, - destination, - .. - } => serde_json::json!({ - "sources": sources.iter().map(|s| s.display()).collect::>(), - "destination": destination.display() - }), - Action::FileDelete { targets, .. } => serde_json::json!({ - "targets": targets.iter().map(|t| t.display()).collect::>() - }), - Action::LibraryCreate { name, path } => serde_json::json!({ - "name": name, - "path": path.as_ref().map(|p| p.display().to_string()) - }), - Action::LibraryDelete { library_id } => serde_json::json!({ - "library_id": library_id - }), - Action::LocationAdd { - path, name, mode, .. - } => serde_json::json!({ - "path": path.display().to_string(), - "name": name, - "mode": mode - }), - Action::LocationRemove { location_id, .. } => serde_json::json!({ - "location_id": location_id - }), - Action::LocationIndex { - location_id, mode, .. - } => serde_json::json!({ - "location_id": location_id, - "mode": mode - }), - } - } -} diff --git a/core-new/src/operations/content/action.rs b/core-new/src/operations/content/action.rs new file mode 100644 index 000000000..1ded09fa7 --- /dev/null +++ b/core-new/src/operations/content/action.rs @@ -0,0 +1,61 @@ +//! Content analysis action handler + +use crate::{ + context::CoreContext, + infrastructure::actions::{ + error::{ActionError, ActionResult}, + handler::ActionHandler, + receipt::ActionReceipt, + }, + register_action_handler, +}; +use async_trait::async_trait; +use std::sync::Arc; +use uuid::Uuid; + +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct ContentAction { + pub paths: Vec, + pub analyze_content: bool, + pub extract_metadata: bool, +} + +pub struct ContentHandler; + +impl ContentHandler { + pub fn new() -> Self { + Self + } +} + +#[async_trait] +impl ActionHandler for ContentHandler { + async fn validate( + &self, + _context: Arc, + action: &crate::infrastructure::actions::Action, + ) -> ActionResult<()> { + // TODO: Re-enable when ContentAnalysis variant is added back + Err(ActionError::Internal("ContentAnalysis action not yet implemented".to_string())) + } + + async fn execute( + &self, + context: Arc, + action: crate::infrastructure::actions::Action, + ) -> ActionResult { + // TODO: Re-enable when ContentAnalysis variant is added back + Err(ActionError::Internal("ContentAnalysis action not yet implemented".to_string())) + } + + fn can_handle(&self, action: &crate::infrastructure::actions::Action) -> bool { + // TODO: Re-enable when ContentAnalysis variant is added back + false + } + + fn supported_actions() -> &'static [&'static str] { + &["content.analyze"] + } +} + +register_action_handler!(ContentHandler, "content.analyze"); \ No newline at end of file diff --git a/core-new/src/operations/content/mod.rs b/core-new/src/operations/content/mod.rs index b350d7fd9..ecf03ce81 100644 --- a/core-new/src/operations/content/mod.rs +++ b/core-new/src/operations/content/mod.rs @@ -1,5 +1,7 @@ //! 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}; @@ -10,6 +12,8 @@ use crate::infrastructure::database::entities::{ content_identity::{self, Entity as ContentIdentity, Model as ContentIdentityModel}, entry::{self, Entity as Entry, Model as EntryModel}, }; + +pub use action::ContentAction; use crate::shared::errors::Result; #[derive(Clone, Debug, Serialize, Deserialize)] diff --git a/core-new/src/operations/file_ops/mod.rs b/core-new/src/operations/file_ops/mod.rs deleted file mode 100644 index c35750a57..000000000 --- a/core-new/src/operations/file_ops/mod.rs +++ /dev/null @@ -1,16 +0,0 @@ -//! File operations - comprehensive file management jobs -//! -//! This module contains job implementations for all file operations: -//! - Copy files and directories -//! - Move/rename files and directories -//! - Delete files (trash, permanent, secure) -//! - Duplicate detection and analysis -//! - File validation and integrity checking - -pub mod copy; -pub mod delete_job; -pub mod duplicate_detection_job; -pub mod validation_job; - -// Re-export the copy module types for easy access -pub use copy::{CopyOptions, FileCopyJob, MoveJob, MoveMode}; diff --git a/core-new/src/operations/files/copy/action.rs b/core-new/src/operations/files/copy/action.rs new file mode 100644 index 000000000..7c9ad344c --- /dev/null +++ b/core-new/src/operations/files/copy/action.rs @@ -0,0 +1,110 @@ +//! File copy action handler + +use super::job::{CopyOptions, FileCopyJob}; +use crate::{ + context::CoreContext, + infrastructure::actions::{ + error::{ActionError, ActionResult}, + handler::ActionHandler, + receipt::ActionReceipt, + Action, + }, + register_action_handler, + shared::types::{SdPath, SdPathBatch}, +}; +use async_trait::async_trait; +use serde::{Deserialize, Serialize}; +use std::{path::PathBuf, sync::Arc}; +use uuid::Uuid; + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct FileCopyAction { + pub sources: Vec, + pub destination: PathBuf, + pub options: CopyOptions, +} + +pub struct FileCopyHandler; + +impl FileCopyHandler { + pub fn new() -> Self { + Self + } +} + +#[async_trait] +impl ActionHandler for FileCopyHandler { + async fn validate(&self, _context: Arc, action: &Action) -> ActionResult<()> { + if let Action::FileCopy { + library_id: _, + action, + } = action + { + if action.sources.is_empty() { + return Err(ActionError::Validation { + field: "sources".to_string(), + message: "At least one source file must be specified".to_string(), + }); + } + + // Additional validation could include: + // - Check if source files exist + // - Check permissions + // - Check if destination is valid + // - Check if it would be a cross-device operation + + Ok(()) + } else { + Err(ActionError::InvalidActionType) + } + } + + async fn execute( + &self, + context: Arc, + action: Action, + ) -> ActionResult { + if let Action::FileCopy { library_id, action } = action { + let library_manager = &context.library_manager; + + // Get the specific library + let library = library_manager + .get_library(library_id) + .await + .ok_or(ActionError::LibraryNotFound(library_id))?; + + // Create job instance + let sources = action + .sources + .into_iter() + .map(|path| SdPath::local(path)) + .collect(); + + let job = + FileCopyJob::new(SdPathBatch::new(sources), SdPath::local(action.destination)) + .with_options(action.options); + + // Dispatch the job + let job_handle = library + .jobs() + .dispatch(job) + .await + .map_err(ActionError::Job)?; + + Ok(ActionReceipt::job_based(Uuid::new_v4(), job_handle)) + } else { + Err(ActionError::InvalidActionType) + } + } + + fn can_handle(&self, action: &Action) -> bool { + matches!(action, Action::FileCopy { .. }) + } + + fn supported_actions() -> &'static [&'static str] { + &["file.copy"] + } +} + +// Register this handler +register_action_handler!(FileCopyHandler, "file.copy"); diff --git a/core-new/src/operations/file_ops/copy/job.rs b/core-new/src/operations/files/copy/job.rs similarity index 100% rename from core-new/src/operations/file_ops/copy/job.rs rename to core-new/src/operations/files/copy/job.rs diff --git a/core-new/src/operations/file_ops/copy/mod.rs b/core-new/src/operations/files/copy/mod.rs similarity index 91% rename from core-new/src/operations/file_ops/copy/mod.rs rename to core-new/src/operations/files/copy/mod.rs index 14fa20413..cf465fd87 100644 --- a/core-new/src/operations/file_ops/copy/mod.rs +++ b/core-new/src/operations/files/copy/mod.rs @@ -1,5 +1,6 @@ //! Modular file copy operations using the Strategy Pattern +pub mod action; pub mod job; pub mod routing; pub mod strategy; diff --git a/core-new/src/operations/file_ops/copy/routing.rs b/core-new/src/operations/files/copy/routing.rs similarity index 100% rename from core-new/src/operations/file_ops/copy/routing.rs rename to core-new/src/operations/files/copy/routing.rs diff --git a/core-new/src/operations/file_ops/copy/strategy.rs b/core-new/src/operations/files/copy/strategy.rs similarity index 100% rename from core-new/src/operations/file_ops/copy/strategy.rs rename to core-new/src/operations/files/copy/strategy.rs diff --git a/core-new/src/operations/actions/handlers/file_delete.rs b/core-new/src/operations/files/delete/action.rs similarity index 54% rename from core-new/src/operations/actions/handlers/file_delete.rs rename to core-new/src/operations/files/delete/action.rs index 9272f3baa..17f32aa17 100644 --- a/core-new/src/operations/actions/handlers/file_delete.rs +++ b/core-new/src/operations/files/delete/action.rs @@ -2,15 +2,24 @@ use crate::{ context::CoreContext, - operations::actions::{ + infrastructure::actions::{ Action, error::{ActionError, ActionResult}, handler::ActionHandler, receipt::ActionReceipt, }, register_action_handler, + shared::types::{SdPath, SdPathBatch}, }; +use super::job::{DeleteOptions, DeleteJob, DeleteMode}; use async_trait::async_trait; -use std::sync::Arc; +use serde::{Deserialize, Serialize}; +use std::{path::PathBuf, sync::Arc}; use uuid::Uuid; +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct FileDeleteAction { + pub targets: Vec, + pub options: DeleteOptions, +} + pub struct FileDeleteHandler; impl FileDeleteHandler { @@ -26,8 +35,8 @@ impl ActionHandler for FileDeleteHandler { _context: Arc, action: &Action, ) -> ActionResult<()> { - if let Action::FileDelete { targets, .. } = action { - if targets.is_empty() { + if let Action::FileDelete { library_id: _, action } = action { + if action.targets.is_empty() { return Err(ActionError::Validation { field: "targets".to_string(), message: "At least one target file must be specified".to_string(), @@ -44,27 +53,33 @@ impl ActionHandler for FileDeleteHandler { context: Arc, action: Action, ) -> ActionResult { - if let Action::FileDelete { targets, options } = action { + if let Action::FileDelete { library_id, action } = action { let library_manager = &context.library_manager; - // Convert our action to job parameters - let job_params = serde_json::json!({ - "targets": targets, - "options": { - "permanent": options.permanent, - "recursive": options.recursive - } - }); + // Get the specific library + let library = library_manager + .get_library(library_id) + .await + .ok_or(ActionError::LibraryNotFound(library_id))?; + + // Create job instance directly + let targets = action.targets + .into_iter() + .map(|path| SdPath::local(path)) + .collect(); - // Get a library to run the job in - let libraries = library_manager.get_open_libraries().await; - let library = libraries.first() - .ok_or(ActionError::Internal("No libraries available".to_string()))?; + let mode = if action.options.permanent { + DeleteMode::Permanent + } else { + DeleteMode::Trash + }; - // Dispatch the file delete job + let job = DeleteJob::new(SdPathBatch::new(targets), mode); + + // Dispatch the job directly let job_handle = library .jobs() - .dispatch_by_name("delete_files", job_params) + .dispatch(job) .await .map_err(ActionError::Job)?; diff --git a/core-new/src/operations/file_ops/delete_job.rs b/core-new/src/operations/files/delete/job.rs similarity index 98% rename from core-new/src/operations/file_ops/delete_job.rs rename to core-new/src/operations/files/delete/job.rs index 60e52b381..db7dee9b9 100644 --- a/core-new/src/operations/file_ops/delete_job.rs +++ b/core-new/src/operations/files/delete/job.rs @@ -22,6 +22,22 @@ pub enum DeleteMode { Secure, } +/// Options for file delete operations +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct DeleteOptions { + pub permanent: bool, + pub recursive: bool, +} + +impl Default for DeleteOptions { + fn default() -> Self { + Self { + permanent: false, + recursive: false, + } + } +} + /// Delete job for removing files and directories #[derive(Debug, Serialize, Deserialize)] pub struct DeleteJob { diff --git a/core-new/src/operations/files/delete/mod.rs b/core-new/src/operations/files/delete/mod.rs new file mode 100644 index 000000000..baa5d1695 --- /dev/null +++ b/core-new/src/operations/files/delete/mod.rs @@ -0,0 +1,6 @@ +//! File delete operations + +pub mod action; +pub mod job; + +pub use job::*; \ No newline at end of file diff --git a/core-new/src/operations/files/duplicate_detection/action.rs b/core-new/src/operations/files/duplicate_detection/action.rs new file mode 100644 index 000000000..fdda1b62e --- /dev/null +++ b/core-new/src/operations/files/duplicate_detection/action.rs @@ -0,0 +1,103 @@ +//! File duplicate detection action handler + +use crate::{ + context::CoreContext, + infrastructure::actions::{ + error::{ActionError, ActionResult}, + handler::ActionHandler, + receipt::ActionReceipt, + }, + register_action_handler, + shared::types::{SdPath, SdPathBatch}, +}; +use super::job::{DuplicateDetectionJob, DetectionMode}; +use async_trait::async_trait; +use std::sync::Arc; +use uuid::Uuid; + +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct DuplicateDetectionAction { + pub paths: Vec, + pub algorithm: String, + pub threshold: f64, +} + +pub struct DuplicateDetectionHandler; + +impl DuplicateDetectionHandler { + pub fn new() -> Self { + Self + } +} + +#[async_trait] +impl ActionHandler for DuplicateDetectionHandler { + async fn validate( + &self, + _context: Arc, + action: &crate::infrastructure::actions::Action, + ) -> ActionResult<()> { + if let crate::infrastructure::actions::Action::DetectDuplicates { action, .. } = action { + if action.paths.is_empty() { + return Err(ActionError::Validation { + field: "paths".to_string(), + message: "At least one path must be specified".to_string(), + }); + } + Ok(()) + } else { + Err(ActionError::InvalidActionType) + } + } + + async fn execute( + &self, + context: Arc, + action: crate::infrastructure::actions::Action, + ) -> ActionResult { + if let crate::infrastructure::actions::Action::DetectDuplicates { library_id, action } = action { + let library_manager = &context.library_manager; + + let library = library_manager.get_library(library_id).await + .ok_or(ActionError::Internal(format!("Library not found: {}", library_id)))?; + + // Convert paths to SdPath and create job + let search_paths = action.paths + .into_iter() + .map(|path| SdPath::local(path)) + .collect(); + + // Parse algorithm to detection mode + let mode = match action.algorithm.as_str() { + "content_hash" => DetectionMode::ContentHash, + "size_only" => DetectionMode::SizeOnly, + "name_and_size" => DetectionMode::NameAndSize, + "deep_scan" => DetectionMode::DeepScan, + _ => DetectionMode::ContentHash, // default + }; + + let job = DuplicateDetectionJob::new(SdPathBatch::new(search_paths), mode); + + // Dispatch the job directly + let job_handle = library + .jobs() + .dispatch(job) + .await + .map_err(ActionError::Job)?; + + Ok(ActionReceipt::job_based(Uuid::new_v4(), job_handle)) + } else { + Err(ActionError::InvalidActionType) + } + } + + fn can_handle(&self, action: &crate::infrastructure::actions::Action) -> bool { + matches!(action, crate::infrastructure::actions::Action::DetectDuplicates { .. }) + } + + fn supported_actions() -> &'static [&'static str] { + &["file.detect_duplicates"] + } +} + +register_action_handler!(DuplicateDetectionHandler, "file.detect_duplicates"); \ No newline at end of file diff --git a/core-new/src/operations/file_ops/duplicate_detection_job.rs b/core-new/src/operations/files/duplicate_detection/job.rs similarity index 100% rename from core-new/src/operations/file_ops/duplicate_detection_job.rs rename to core-new/src/operations/files/duplicate_detection/job.rs diff --git a/core-new/src/operations/files/duplicate_detection/mod.rs b/core-new/src/operations/files/duplicate_detection/mod.rs new file mode 100644 index 000000000..ee6546a6c --- /dev/null +++ b/core-new/src/operations/files/duplicate_detection/mod.rs @@ -0,0 +1,7 @@ +//! File duplicate detection operations + +pub mod action; +pub mod job; + +pub use job::*; +pub use action::DuplicateDetectionAction; \ No newline at end of file diff --git a/core-new/src/operations/files/mod.rs b/core-new/src/operations/files/mod.rs new file mode 100644 index 000000000..5a10d1065 --- /dev/null +++ b/core-new/src/operations/files/mod.rs @@ -0,0 +1,11 @@ +//! File operations + +pub mod copy; +pub mod delete; +pub mod validation; +pub mod duplicate_detection; + +pub use copy::*; +pub use delete::*; +pub use validation::*; +pub use duplicate_detection::*; \ No newline at end of file diff --git a/core-new/src/operations/files/validation/action.rs b/core-new/src/operations/files/validation/action.rs new file mode 100644 index 000000000..20b43976e --- /dev/null +++ b/core-new/src/operations/files/validation/action.rs @@ -0,0 +1,103 @@ +//! File validation action handler + +use crate::{ + context::CoreContext, + infrastructure::actions::{ + error::{ActionError, ActionResult}, + handler::ActionHandler, + receipt::ActionReceipt, + }, + register_action_handler, + shared::types::{SdPath, SdPathBatch}, +}; +use super::job::{ValidationJob, ValidationMode}; +use async_trait::async_trait; +use std::sync::Arc; +use uuid::Uuid; + +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct ValidationAction { + pub paths: Vec, + pub verify_checksums: bool, + pub deep_scan: bool, +} + +pub struct ValidationHandler; + +impl ValidationHandler { + pub fn new() -> Self { + Self + } +} + +#[async_trait] +impl ActionHandler for ValidationHandler { + async fn validate( + &self, + _context: Arc, + action: &crate::infrastructure::actions::Action, + ) -> ActionResult<()> { + if let crate::infrastructure::actions::Action::FileValidate { action, .. } = action { + if action.paths.is_empty() { + return Err(ActionError::Validation { + field: "paths".to_string(), + message: "At least one path must be specified".to_string(), + }); + } + Ok(()) + } else { + Err(ActionError::InvalidActionType) + } + } + + async fn execute( + &self, + context: Arc, + action: crate::infrastructure::actions::Action, + ) -> ActionResult { + if let crate::infrastructure::actions::Action::FileValidate { library_id, action } = action { + let library_manager = &context.library_manager; + + let library = library_manager.get_library(library_id).await + .ok_or(ActionError::Internal(format!("Library not found: {}", library_id)))?; + + // Convert paths to SdPath and create job + let targets = action.paths + .into_iter() + .map(|path| SdPath::local(path)) + .collect(); + + // Determine validation mode based on action parameters + let mode = if action.deep_scan { + ValidationMode::Complete + } else if action.verify_checksums { + ValidationMode::Integrity + } else { + ValidationMode::Basic + }; + + let job = ValidationJob::new(SdPathBatch::new(targets), mode); + + // Dispatch the job directly + let job_handle = library + .jobs() + .dispatch(job) + .await + .map_err(ActionError::Job)?; + + Ok(ActionReceipt::job_based(Uuid::new_v4(), job_handle)) + } else { + Err(ActionError::InvalidActionType) + } + } + + fn can_handle(&self, action: &crate::infrastructure::actions::Action) -> bool { + matches!(action, crate::infrastructure::actions::Action::FileValidate { .. }) + } + + fn supported_actions() -> &'static [&'static str] { + &["file.validate"] + } +} + +register_action_handler!(ValidationHandler, "file.validate"); \ No newline at end of file diff --git a/core-new/src/operations/file_ops/validation_job.rs b/core-new/src/operations/files/validation/job.rs similarity index 100% rename from core-new/src/operations/file_ops/validation_job.rs rename to core-new/src/operations/files/validation/job.rs diff --git a/core-new/src/operations/files/validation/mod.rs b/core-new/src/operations/files/validation/mod.rs new file mode 100644 index 000000000..762731a6f --- /dev/null +++ b/core-new/src/operations/files/validation/mod.rs @@ -0,0 +1,7 @@ +//! File validation operations + +pub mod action; +pub mod job; + +pub use job::*; +pub use action::ValidationAction; \ No newline at end of file diff --git a/core-new/src/operations/indexing/action.rs b/core-new/src/operations/indexing/action.rs new file mode 100644 index 000000000..13bed2b7a --- /dev/null +++ b/core-new/src/operations/indexing/action.rs @@ -0,0 +1,102 @@ +//! Indexing action handler + +use crate::{ + context::CoreContext, + infrastructure::actions::{ + error::{ActionError, ActionResult}, + handler::ActionHandler, + receipt::ActionReceipt, + }, + register_action_handler, + shared::types::SdPath, +}; +use super::job::{IndexerJob, IndexMode, IndexScope}; +use async_trait::async_trait; +use std::sync::Arc; +use uuid::Uuid; + +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct IndexingAction { + pub paths: Vec, + pub recursive: bool, + pub include_hidden: bool, +} + +pub struct IndexingHandler; + +impl IndexingHandler { + pub fn new() -> Self { + Self + } +} + +#[async_trait] +impl ActionHandler for IndexingHandler { + async fn validate( + &self, + _context: Arc, + action: &crate::infrastructure::actions::Action, + ) -> ActionResult<()> { + if let crate::infrastructure::actions::Action::Index { action, .. } = action { + if action.paths.is_empty() { + return Err(ActionError::Validation { + field: "paths".to_string(), + message: "At least one path must be specified".to_string(), + }); + } + Ok(()) + } else { + Err(ActionError::InvalidActionType) + } + } + + async fn execute( + &self, + context: Arc, + action: crate::infrastructure::actions::Action, + ) -> ActionResult { + if let crate::infrastructure::actions::Action::Index { library_id, action } = action { + let library_manager = &context.library_manager; + + let library = library_manager.get_library(library_id).await + .ok_or(ActionError::Internal(format!("Library not found: {}", library_id)))?; + + // TODO: For multiple paths, we might want to create multiple jobs or handle this differently + // For now, just take the first path + let first_path = action.paths.into_iter().next() + .ok_or(ActionError::Validation { + field: "paths".to_string(), + message: "At least one path must be specified".to_string(), + })?; + + // Create indexer job directly + // TODO: Need location_id - for now using a placeholder + let job = IndexerJob::from_location( + Uuid::new_v4(), // placeholder location_id + SdPath::local(first_path), + IndexMode::Content // default mode + ); + + // Dispatch the job directly + let job_handle = library + .jobs() + .dispatch(job) + .await + .map_err(ActionError::Job)?; + + Ok(ActionReceipt::job_based(Uuid::new_v4(), job_handle)) + } else { + Err(ActionError::InvalidActionType) + } + } + + fn can_handle(&self, action: &crate::infrastructure::actions::Action) -> bool { + matches!(action, crate::infrastructure::actions::Action::Index { .. }) + } + + fn supported_actions() -> &'static [&'static str] { + &["indexing.index"] + } +} + +register_action_handler!(IndexingHandler, "indexing.index"); \ No newline at end of file diff --git a/core-new/src/operations/indexing/mod.rs b/core-new/src/operations/indexing/mod.rs index e7d245ee2..78f81ab7e 100644 --- a/core-new/src/operations/indexing/mod.rs +++ b/core-new/src/operations/indexing/mod.rs @@ -8,6 +8,7 @@ //! - Comprehensive error handling //! - Performance monitoring and metrics +pub mod action; pub mod job; pub mod state; pub mod entry; @@ -29,6 +30,7 @@ pub use entry::{EntryProcessor, EntryMetadata}; pub use filters::should_skip_path; pub use metrics::IndexerMetrics; pub use persistence::{IndexPersistence as PersistenceTrait, PersistenceFactory}; +pub use action::IndexingAction; // Rules system will be integrated here in the future // pub mod rules; \ No newline at end of file diff --git a/core-new/src/operations/libraries/create/action.rs b/core-new/src/operations/libraries/create/action.rs new file mode 100644 index 000000000..91395764f --- /dev/null +++ b/core-new/src/operations/libraries/create/action.rs @@ -0,0 +1,84 @@ +//! Library creation action handler + +use crate::{ + context::CoreContext, + infrastructure::actions::{ + error::{ActionError, ActionResult}, + handler::ActionHandler, + receipt::ActionReceipt, + }, + register_action_handler, +}; +use async_trait::async_trait; +use std::sync::Arc; +use std::path::PathBuf; +use uuid::Uuid; + +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct LibraryCreateAction { + pub name: String, + pub path: Option, +} + +pub struct LibraryCreateHandler; + +impl LibraryCreateHandler { + pub fn new() -> Self { + Self + } +} + +#[async_trait] +impl ActionHandler for LibraryCreateHandler { + async fn validate( + &self, + _context: Arc, + action: &crate::infrastructure::actions::Action, + ) -> ActionResult<()> { + if let crate::infrastructure::actions::Action::LibraryCreate(action) = action { + if action.name.trim().is_empty() { + return Err(ActionError::Validation { + field: "name".to_string(), + message: "Library name cannot be empty".to_string(), + }); + } + Ok(()) + } else { + Err(ActionError::InvalidActionType) + } + } + + async fn execute( + &self, + context: Arc, + action: crate::infrastructure::actions::Action, + ) -> ActionResult { + if let crate::infrastructure::actions::Action::LibraryCreate(action) = action { + let library_manager = &context.library_manager; + let new_library = library_manager.create_library(action.name, action.path).await?; + + let library_name = new_library.name().await; + Ok(ActionReceipt::immediate( + Uuid::new_v4(), + Some(serde_json::json!({ + "library_id": new_library.id(), + "name": library_name, + "path": new_library.path().display().to_string() + })), + )) + } else { + Err(ActionError::InvalidActionType) + } + } + + fn can_handle(&self, action: &crate::infrastructure::actions::Action) -> bool { + matches!(action, crate::infrastructure::actions::Action::LibraryCreate(_)) + } + + fn supported_actions() -> &'static [&'static str] { + &["library.create"] + } +} + +// Register this handler +register_action_handler!(LibraryCreateHandler, "library.create"); \ No newline at end of file diff --git a/core-new/src/operations/libraries/create/mod.rs b/core-new/src/operations/libraries/create/mod.rs new file mode 100644 index 000000000..fed13359e --- /dev/null +++ b/core-new/src/operations/libraries/create/mod.rs @@ -0,0 +1,3 @@ +//! Library create operations + +pub mod action; \ No newline at end of file diff --git a/core-new/src/operations/actions/handlers/library_delete.rs b/core-new/src/operations/libraries/delete/action.rs similarity index 72% rename from core-new/src/operations/actions/handlers/library_delete.rs rename to core-new/src/operations/libraries/delete/action.rs index 24cc48007..3ac312e69 100644 --- a/core-new/src/operations/actions/handlers/library_delete.rs +++ b/core-new/src/operations/libraries/delete/action.rs @@ -2,15 +2,21 @@ use crate::{ context::CoreContext, - operations::actions::{ + infrastructure::actions::{ Action, error::{ActionError, ActionResult}, handler::ActionHandler, receipt::ActionReceipt, }, register_action_handler, }; use async_trait::async_trait; +use serde::{Deserialize, Serialize}; use std::sync::Arc; use uuid::Uuid; +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct LibraryDeleteAction { + // Library deletion doesn't need additional fields beyond library_id +} + pub struct LibraryDeleteHandler; impl LibraryDeleteHandler { @@ -26,17 +32,17 @@ impl ActionHandler for LibraryDeleteHandler { context: Arc, action: Action, ) -> ActionResult { - if let Action::LibraryDelete { library_id: _library_id } = action { + if let Action::LibraryDelete(action) = action { // For now, library deletion is not implemented in the library manager // This would need to be implemented as a proper method Err(ActionError::Internal("Library deletion not yet implemented".to_string())) } else { - Err(crate::operations::actions::error::ActionError::InvalidActionType) + Err(crate::infrastructure::actions::error::ActionError::InvalidActionType) } } fn can_handle(&self, action: &Action) -> bool { - matches!(action, Action::LibraryDelete { .. }) + matches!(action, Action::LibraryDelete(_)) } fn supported_actions() -> &'static [&'static str] { diff --git a/core-new/src/operations/libraries/delete/mod.rs b/core-new/src/operations/libraries/delete/mod.rs new file mode 100644 index 000000000..dba807ff8 --- /dev/null +++ b/core-new/src/operations/libraries/delete/mod.rs @@ -0,0 +1,3 @@ +//! Library delete operations + +pub mod action; \ No newline at end of file diff --git a/core-new/src/operations/libraries/mod.rs b/core-new/src/operations/libraries/mod.rs new file mode 100644 index 000000000..7e3c3e159 --- /dev/null +++ b/core-new/src/operations/libraries/mod.rs @@ -0,0 +1,7 @@ +//! Library operations + +pub mod create; +pub mod delete; + +pub use create::*; +pub use delete::*; \ No newline at end of file diff --git a/core-new/src/operations/actions/handlers/location_add.rs b/core-new/src/operations/locations/add/action.rs similarity index 60% rename from core-new/src/operations/actions/handlers/location_add.rs rename to core-new/src/operations/locations/add/action.rs index a9879dcf3..4c940f93e 100644 --- a/core-new/src/operations/actions/handlers/location_add.rs +++ b/core-new/src/operations/locations/add/action.rs @@ -4,20 +4,30 @@ use crate::{ context::CoreContext, infrastructure::database::entities, location::manager::LocationManager, - operations::{ + infrastructure::{ actions::{ Action, error::{ActionError, ActionResult}, handler::ActionHandler, receipt::ActionReceipt, }, - indexing::{IndexMode as CoreIndexMode, IndexScope, job::{IndexerJob, IndexerJobConfig}}, + }, + operations::{ + indexing::{IndexMode, job::IndexerJob}, }, register_action_handler, shared::types::SdPath, }; use sea_orm::{ColumnTrait, EntityTrait, QueryFilter}; use async_trait::async_trait; -use std::sync::Arc; +use serde::{Deserialize, Serialize}; +use std::{path::PathBuf, sync::Arc}; use uuid::Uuid; +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct LocationAddAction { + pub path: PathBuf, + pub name: Option, + pub mode: IndexMode, +} + pub struct LocationAddHandler; impl LocationAddHandler { @@ -33,14 +43,14 @@ impl ActionHandler for LocationAddHandler { _context: Arc, action: &Action, ) -> ActionResult<()> { - if let Action::LocationAdd { path, .. } = action { - if !path.exists() { + if let Action::LocationAdd { library_id: _, action } = action { + if !action.path.exists() { return Err(ActionError::Validation { field: "path".to_string(), message: "Path does not exist".to_string(), }); } - if !path.is_dir() { + if !action.path.is_dir() { return Err(ActionError::Validation { field: "path".to_string(), message: "Path must be a directory".to_string(), @@ -57,7 +67,7 @@ impl ActionHandler for LocationAddHandler { context: Arc, action: Action, ) -> ActionResult { - if let Action::LocationAdd { library_id, path, name, mode } = action { + if let Action::LocationAdd { library_id, action } = action { let library_manager = &context.library_manager; // Get the specific library @@ -83,46 +93,28 @@ impl ActionHandler for LocationAddHandler { // Add the location using LocationManager let location_manager = LocationManager::new(context.events.as_ref().clone()); - let location_mode = match mode { - crate::operations::actions::IndexMode::Shallow => crate::location::IndexMode::Shallow, - crate::operations::actions::IndexMode::Deep => crate::location::IndexMode::Deep, - crate::operations::actions::IndexMode::Sync => crate::location::IndexMode::Deep, // Default fallback + let location_mode = match action.mode { + IndexMode::Shallow => crate::location::IndexMode::Shallow, + IndexMode::Content => crate::location::IndexMode::Content, + IndexMode::Deep => crate::location::IndexMode::Deep, }; let (location_id, location_name) = location_manager - .add_location(library.clone(), path.clone(), name, device_record.id, location_mode) + .add_location(library.clone(), action.path.clone(), action.name, device_record.id, location_mode) .await .map_err(|e| ActionError::Internal(e.to_string()))?; // Now dispatch an indexing job based on the mode - let job_handle = if matches!(mode, crate::operations::actions::IndexMode::Sync) { - // Sync mode doesn't automatically start indexing - None - } else { - // Convert action mode to core indexing mode - let core_mode = match mode { - crate::operations::actions::IndexMode::Shallow => CoreIndexMode::Shallow, - crate::operations::actions::IndexMode::Deep => CoreIndexMode::Deep, - crate::operations::actions::IndexMode::Sync => CoreIndexMode::Deep, // Fallback, but won't be used - }; + let job_handle = { + // Use the action mode directly since it's already the correct IndexMode - // Create indexer job configuration - let indexer_config = IndexerJobConfig { - location_id: Some(location_id), - path: SdPath::local(path.clone()), - mode: core_mode, - scope: IndexScope::Recursive, // Default to recursive for location indexing - persistence: crate::operations::indexing::IndexPersistence::Persistent, - max_depth: None, - }; - - // Dispatch the indexer job - let job_params = serde_json::to_value(&indexer_config) - .map_err(ActionError::JsonSerialization)?; + // Create indexer job directly + let job = IndexerJob::from_location(location_id, SdPath::local(action.path.clone()), action.mode); + // Dispatch the job directly let job_handle = library .jobs() - .dispatch_by_name("indexer", job_params) + .dispatch(job) .await .map_err(ActionError::Job)?; @@ -134,7 +126,7 @@ impl ActionHandler for LocationAddHandler { Some(serde_json::json!({ "location_id": location_id, "name": location_name, - "path": path.display().to_string() + "path": action.path.display().to_string() })), job_handle, )) diff --git a/core-new/src/operations/locations/add/mod.rs b/core-new/src/operations/locations/add/mod.rs new file mode 100644 index 000000000..e0efaf814 --- /dev/null +++ b/core-new/src/operations/locations/add/mod.rs @@ -0,0 +1,3 @@ +//! Location add operations + +pub mod action; \ No newline at end of file diff --git a/core-new/src/operations/actions/handlers/location_index.rs b/core-new/src/operations/locations/index/action.rs similarity index 53% rename from core-new/src/operations/actions/handlers/location_index.rs rename to core-new/src/operations/locations/index/action.rs index ae16da1f2..62831b5d5 100644 --- a/core-new/src/operations/actions/handlers/location_index.rs +++ b/core-new/src/operations/locations/index/action.rs @@ -2,19 +2,28 @@ use crate::{ context::CoreContext, - operations::{ + infrastructure::{ actions::{ Action, error::{ActionError, ActionResult}, handler::ActionHandler, receipt::ActionReceipt, }, - indexing::{IndexMode as CoreIndexMode, IndexScope, job::IndexerJobConfig}, + }, + operations::{ + indexing::{IndexMode, job::IndexerJob}, }, register_action_handler, shared::types::SdPath, }; use async_trait::async_trait; +use serde::{Deserialize, Serialize}; use std::sync::Arc; use uuid::Uuid; +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct LocationIndexAction { + pub location_id: Uuid, + pub mode: IndexMode, +} + pub struct LocationIndexHandler; impl LocationIndexHandler { @@ -30,7 +39,7 @@ impl ActionHandler for LocationIndexHandler { context: Arc, action: Action, ) -> ActionResult { - if let Action::LocationIndex { library_id, location_id, mode } = action { + if let Action::LocationIndex { library_id, action } = action { let library_manager = &context.library_manager; // Get the specific library @@ -39,35 +48,17 @@ impl ActionHandler for LocationIndexHandler { .await .ok_or(ActionError::LibraryNotFound(library_id))?; - // Convert action mode to core indexing mode - let core_mode = match mode { - crate::operations::actions::IndexMode::Shallow => CoreIndexMode::Shallow, - crate::operations::actions::IndexMode::Deep => CoreIndexMode::Deep, - crate::operations::actions::IndexMode::Sync => CoreIndexMode::Deep, // Treat sync as deep for indexing - }; - - // We need to get the location record to get the path - // For now, let's create a placeholder SdPath - in a real implementation, - // we'd query the database to get the location's actual path + // TODO: In a real implementation, we'd query the database to get the location's actual path + // For now, let's create a placeholder SdPath let location_path = SdPath::local("/placeholder"); // This should be the actual location path - // Create indexer job configuration - let indexer_config = IndexerJobConfig { - location_id: Some(location_id), - path: location_path, - mode: core_mode, - scope: IndexScope::Recursive, - persistence: crate::operations::indexing::IndexPersistence::Persistent, - max_depth: None, - }; - - // Dispatch an indexing job - let job_params = serde_json::to_value(&indexer_config) - .map_err(ActionError::JsonSerialization)?; + // Create indexer job directly + let job = IndexerJob::from_location(action.location_id, location_path, action.mode); + // Dispatch the job directly let job_handle = library .jobs() - .dispatch_by_name("indexer", job_params) + .dispatch(job) .await .map_err(ActionError::Job)?; diff --git a/core-new/src/operations/locations/index/mod.rs b/core-new/src/operations/locations/index/mod.rs new file mode 100644 index 000000000..b2227d203 --- /dev/null +++ b/core-new/src/operations/locations/index/mod.rs @@ -0,0 +1,3 @@ +//! Location index operations + +pub mod action; \ No newline at end of file diff --git a/core-new/src/operations/locations/mod.rs b/core-new/src/operations/locations/mod.rs new file mode 100644 index 000000000..42c7e4e93 --- /dev/null +++ b/core-new/src/operations/locations/mod.rs @@ -0,0 +1,9 @@ +//! Location operations + +pub mod add; +pub mod remove; +pub mod index; + +pub use add::*; +pub use remove::*; +pub use index::*; \ No newline at end of file diff --git a/core-new/src/operations/actions/handlers/location_remove.rs b/core-new/src/operations/locations/remove/action.rs similarity index 82% rename from core-new/src/operations/actions/handlers/location_remove.rs rename to core-new/src/operations/locations/remove/action.rs index 618bbf616..fd7b2d46d 100644 --- a/core-new/src/operations/actions/handlers/location_remove.rs +++ b/core-new/src/operations/locations/remove/action.rs @@ -3,15 +3,21 @@ use crate::{ context::CoreContext, location::manager::LocationManager, - operations::actions::{ + infrastructure::actions::{ Action, error::{ActionError, ActionResult}, handler::ActionHandler, receipt::ActionReceipt, }, register_action_handler, }; use async_trait::async_trait; +use serde::{Deserialize, Serialize}; use std::sync::Arc; use uuid::Uuid; +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct LocationRemoveAction { + pub location_id: Uuid, +} + pub struct LocationRemoveHandler; impl LocationRemoveHandler { @@ -27,7 +33,7 @@ impl ActionHandler for LocationRemoveHandler { context: Arc, action: Action, ) -> ActionResult { - if let Action::LocationRemove { library_id, location_id } = action { + if let Action::LocationRemove { library_id, action } = action { let library_manager = &context.library_manager; // Get the specific library @@ -39,14 +45,14 @@ impl ActionHandler for LocationRemoveHandler { // Remove the location let location_manager = LocationManager::new(context.events.as_ref().clone()); location_manager - .remove_location(&library, location_id) + .remove_location(&library, action.location_id) .await .map_err(|e| ActionError::Internal(e.to_string()))?; Ok(ActionReceipt::immediate( Uuid::new_v4(), Some(serde_json::json!({ - "location_id": location_id, + "location_id": action.location_id, "removed": true })), )) diff --git a/core-new/src/operations/locations/remove/mod.rs b/core-new/src/operations/locations/remove/mod.rs new file mode 100644 index 000000000..fc75387d1 --- /dev/null +++ b/core-new/src/operations/locations/remove/mod.rs @@ -0,0 +1,3 @@ +//! Location remove operations + +pub mod action; \ No newline at end of file diff --git a/core-new/src/operations/media_processing/mod.rs b/core-new/src/operations/media/mod.rs similarity index 100% rename from core-new/src/operations/media_processing/mod.rs rename to core-new/src/operations/media/mod.rs diff --git a/core-new/src/operations/media/thumbnail/action.rs b/core-new/src/operations/media/thumbnail/action.rs new file mode 100644 index 000000000..3a5d2648c --- /dev/null +++ b/core-new/src/operations/media/thumbnail/action.rs @@ -0,0 +1,97 @@ +//! Thumbnail generation action handler + +use crate::{ + context::CoreContext, + infrastructure::actions::{ + error::{ActionError, ActionResult}, + handler::ActionHandler, + receipt::ActionReceipt, + }, + register_action_handler, +}; +use super::job::{ThumbnailJob, ThumbnailJobConfig}; +use async_trait::async_trait; +use std::sync::Arc; +use uuid::Uuid; + +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct ThumbnailAction { + pub paths: Vec, + pub size: u32, + pub quality: u8, +} + +pub struct ThumbnailHandler; + +impl ThumbnailHandler { + pub fn new() -> Self { + Self + } +} + +#[async_trait] +impl ActionHandler for ThumbnailHandler { + async fn validate( + &self, + _context: Arc, + action: &crate::infrastructure::actions::Action, + ) -> ActionResult<()> { + if let crate::infrastructure::actions::Action::GenerateThumbnails { action, .. } = action { + if action.paths.is_empty() { + return Err(ActionError::Validation { + field: "paths".to_string(), + message: "At least one path must be specified".to_string(), + }); + } + Ok(()) + } else { + Err(ActionError::InvalidActionType) + } + } + + async fn execute( + &self, + context: Arc, + action: crate::infrastructure::actions::Action, + ) -> ActionResult { + if let crate::infrastructure::actions::Action::GenerateThumbnails { library_id, action } = action { + let library_manager = &context.library_manager; + + let library = library_manager.get_library(library_id).await + .ok_or(ActionError::Internal(format!("Library not found: {}", library_id)))?; + + // Create thumbnail job config + let config = ThumbnailJobConfig { + sizes: vec![action.size], + quality: action.quality, + regenerate: false, + ..Default::default() + }; + + // TODO: Convert paths to entry IDs by querying the database + // For now, create a job that processes all suitable entries + let job = ThumbnailJob::new(config); + + // Dispatch the job directly + let job_handle = library + .jobs() + .dispatch(job) + .await + .map_err(ActionError::Job)?; + + Ok(ActionReceipt::job_based(Uuid::new_v4(), job_handle)) + } else { + Err(ActionError::InvalidActionType) + } + } + + fn can_handle(&self, action: &crate::infrastructure::actions::Action) -> bool { + matches!(action, crate::infrastructure::actions::Action::GenerateThumbnails { .. }) + } + + fn supported_actions() -> &'static [&'static str] { + &["media.thumbnail"] + } +} + +register_action_handler!(ThumbnailHandler, "media.thumbnail"); \ No newline at end of file diff --git a/core-new/src/operations/media_processing/thumbnail/error.rs b/core-new/src/operations/media/thumbnail/error.rs similarity index 100% rename from core-new/src/operations/media_processing/thumbnail/error.rs rename to core-new/src/operations/media/thumbnail/error.rs diff --git a/core-new/src/operations/media_processing/thumbnail/generator.rs b/core-new/src/operations/media/thumbnail/generator.rs similarity index 100% rename from core-new/src/operations/media_processing/thumbnail/generator.rs rename to core-new/src/operations/media/thumbnail/generator.rs diff --git a/core-new/src/operations/media_processing/thumbnail/job.rs b/core-new/src/operations/media/thumbnail/job.rs similarity index 100% rename from core-new/src/operations/media_processing/thumbnail/job.rs rename to core-new/src/operations/media/thumbnail/job.rs diff --git a/core-new/src/operations/media_processing/thumbnail/mod.rs b/core-new/src/operations/media/thumbnail/mod.rs similarity index 88% rename from core-new/src/operations/media_processing/thumbnail/mod.rs rename to core-new/src/operations/media/thumbnail/mod.rs index 83f059382..2598263d1 100644 --- a/core-new/src/operations/media_processing/thumbnail/mod.rs +++ b/core-new/src/operations/media/thumbnail/mod.rs @@ -4,6 +4,7 @@ //! including images, videos, and documents. It operates as a separate job that //! can run independently or be triggered after indexing operations. +pub mod action; mod job; mod state; mod generator; @@ -14,4 +15,5 @@ pub use job::{ThumbnailJob, ThumbnailJobConfig}; pub use state::{ThumbnailState, ThumbnailPhase, ThumbnailEntry, ThumbnailStats}; pub use generator::{ThumbnailGenerator, ThumbnailInfo, ImageGenerator, VideoGenerator}; pub use error::{ThumbnailError, ThumbnailResult}; -pub use utils::ThumbnailUtils; \ No newline at end of file +pub use utils::ThumbnailUtils; +pub use action::ThumbnailAction; \ No newline at end of file diff --git a/core-new/src/operations/media_processing/thumbnail/state.rs b/core-new/src/operations/media/thumbnail/state.rs similarity index 100% rename from core-new/src/operations/media_processing/thumbnail/state.rs rename to core-new/src/operations/media/thumbnail/state.rs diff --git a/core-new/src/operations/media_processing/thumbnail/utils.rs b/core-new/src/operations/media/thumbnail/utils.rs similarity index 100% rename from core-new/src/operations/media_processing/thumbnail/utils.rs rename to core-new/src/operations/media/thumbnail/utils.rs diff --git a/core-new/src/operations/metadata/action.rs b/core-new/src/operations/metadata/action.rs new file mode 100644 index 000000000..c2d9466c7 --- /dev/null +++ b/core-new/src/operations/metadata/action.rs @@ -0,0 +1,89 @@ +//! Metadata operations action handler + +use crate::{ + context::CoreContext, + infrastructure::actions::{ + error::{ActionError, ActionResult}, + handler::ActionHandler, + receipt::ActionReceipt, + }, + register_action_handler, +}; +use async_trait::async_trait; +use std::sync::Arc; +use uuid::Uuid; + +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct MetadataAction { + pub paths: Vec, + pub extract_exif: bool, + pub extract_xmp: bool, +} + +pub struct MetadataHandler; + +impl MetadataHandler { + pub fn new() -> Self { + Self + } +} + +#[async_trait] +impl ActionHandler for MetadataHandler { + async fn validate( + &self, + _context: Arc, + action: &crate::infrastructure::actions::Action, + ) -> ActionResult<()> { + if let crate::infrastructure::actions::Action::MetadataOperation { action, .. } = action { + if action.paths.is_empty() { + return Err(ActionError::Validation { + field: "paths".to_string(), + message: "At least one path must be specified".to_string(), + }); + } + Ok(()) + } else { + Err(ActionError::InvalidActionType) + } + } + + async fn execute( + &self, + context: Arc, + action: crate::infrastructure::actions::Action, + ) -> ActionResult { + if let crate::infrastructure::actions::Action::MetadataOperation { library_id, action } = action { + let library_manager = &context.library_manager; + + let job_params = serde_json::json!({ + "paths": action.paths, + "extract_exif": action.extract_exif, + "extract_xmp": action.extract_xmp + }); + + let library = library_manager.get_library(library_id).await + .ok_or(ActionError::Internal(format!("Library not found: {}", library_id)))?; + + let job_handle = library + .jobs() + .dispatch_by_name("extract_metadata", job_params) + .await + .map_err(ActionError::Job)?; + + Ok(ActionReceipt::job_based(Uuid::new_v4(), job_handle)) + } else { + Err(ActionError::InvalidActionType) + } + } + + fn can_handle(&self, action: &crate::infrastructure::actions::Action) -> bool { + matches!(action, crate::infrastructure::actions::Action::MetadataOperation { .. }) + } + + fn supported_actions() -> &'static [&'static str] { + &["metadata.extract"] + } +} + +register_action_handler!(MetadataHandler, "metadata.extract"); \ No newline at end of file diff --git a/core-new/src/operations/metadata/mod.rs b/core-new/src/operations/metadata/mod.rs index 4f566aa67..ccd041cba 100644 --- a/core-new/src/operations/metadata/mod.rs +++ b/core-new/src/operations/metadata/mod.rs @@ -1,5 +1,7 @@ //! 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}; @@ -18,6 +20,8 @@ use crate::infrastructure::database::entities::{ self, ActiveModel as UserMetadataTagActiveModel, Entity as UserMetadataTag, }, }; + +pub use action::MetadataAction; use crate::shared::errors::Result; #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] diff --git a/core-new/src/operations/mod.rs b/core-new/src/operations/mod.rs index 5bdbd7971..2452b7a33 100644 --- a/core-new/src/operations/mod.rs +++ b/core-new/src/operations/mod.rs @@ -7,11 +7,12 @@ //! - Content operations (deduplication, statistics) //! - Metadata operations (hierarchical tagging) -pub mod actions; pub mod content; -pub mod file_ops; +pub mod files; +pub mod libraries; +pub mod locations; pub mod indexing; -pub mod media_processing; +pub mod media; pub mod metadata; /// Register all jobs with the job system @@ -19,17 +20,17 @@ pub mod metadata; /// This should be called during core initialization to register all available job types pub fn register_all_jobs() { // File operation jobs - register_job::(); - register_job::(); - register_job::(); - register_job::(); - register_job::(); + register_job::(); + register_job::(); + register_job::(); + register_job::(); + register_job::(); // Indexing jobs register_job::(); // Media processing jobs - register_job::(); + register_job::(); } /// Register a single job type with the job system diff --git a/core-new/src/services/file_sharing.rs b/core-new/src/services/file_sharing.rs index 943a3a138..31bc1aa54 100644 --- a/core-new/src/services/file_sharing.rs +++ b/core-new/src/services/file_sharing.rs @@ -2,7 +2,7 @@ use crate::{ context::CoreContext, - operations::file_ops::copy::{CopyOptions, FileCopyJob}, + operations::files::copy::{CopyOptions, FileCopyJob}, services::networking::protocols::file_transfer::FileMetadata, shared::types::SdPath, }; diff --git a/core-new/tests/job_registration_test.rs b/core-new/tests/job_registration_test.rs index e9dd244b0..aafa51ffc 100644 --- a/core-new/tests/job_registration_test.rs +++ b/core-new/tests/job_registration_test.rs @@ -2,7 +2,7 @@ use sd_core_new::{ infrastructure::jobs::{prelude::*, registry::REGISTRY}, - operations::file_ops::copy::FileCopyJob, + operations::files::copy::job::FileCopyJob, shared::types::SdPath, }; use uuid::Uuid; @@ -23,7 +23,7 @@ async fn test_job_registration() { let schema = schema.unwrap(); assert_eq!(schema.name, "file_copy"); assert_eq!(schema.resumable, true); - assert_eq!(schema.description, Some("Copy files to a destination")); + assert_eq!(schema.description, Some("Copy or move files to a destination")); } #[tokio::test]