From 9800ffbc35c1bd9c4e823c3fa45b44c4ab6aaefb Mon Sep 17 00:00:00 2001 From: Jamie Pine Date: Sat, 5 Jul 2025 16:49:11 -0700 Subject: [PATCH] Refactor action handling to utilize ActionOutput - Updated action handler interfaces to return ActionOutput instead of ActionReceipt, enhancing the clarity of action results. - Refactored ActionManager and related components to align with the new output structure, improving consistency across action executions. - Introduced new output types for various actions, including file copy and location management, to provide more detailed feedback. - Enhanced CLI commands to handle the updated action outputs, ensuring user interactions reflect the new structure. These changes streamline action handling and improve the overall architecture of the codebase. --- core-new/COPY_BUILDER_USAGE.md | 198 ++++++++++ core-new/INPUT_ABSTRACTION_ARCHITECTURE.md | 276 ++++++++++++++ .../src/infrastructure/actions/builder.rs | 77 ++++ .../src/infrastructure/actions/handler.rs | 6 +- .../src/infrastructure/actions/manager.rs | 18 +- core-new/src/infrastructure/actions/mod.rs | 2 + core-new/src/infrastructure/actions/output.rs | 151 ++++++++ .../src/infrastructure/cli/adapters/copy.rs | 142 ++++++++ .../src/infrastructure/cli/adapters/mod.rs | 6 + core-new/src/infrastructure/cli/commands.rs | 171 +++++---- core-new/src/infrastructure/cli/mod.rs | 7 + core-new/src/operations/content/action.rs | 4 +- core-new/src/operations/files/copy/action.rs | 338 +++++++++++++++++- core-new/src/operations/files/copy/input.rs | 234 ++++++++++++ core-new/src/operations/files/copy/mod.rs | 1 + .../src/operations/files/delete/action.rs | 10 +- .../files/duplicate_detection/action.rs | 6 +- .../src/operations/files/validation/action.rs | 6 +- core-new/src/operations/indexing/action.rs | 6 +- .../src/operations/libraries/create/action.rs | 17 +- .../src/operations/libraries/delete/action.rs | 4 +- .../src/operations/locations/add/action.rs | 14 +- .../src/operations/locations/index/action.rs | 6 +- .../src/operations/locations/remove/action.rs | 12 +- .../src/operations/media/thumbnail/action.rs | 6 +- core-new/src/operations/metadata/action.rs | 6 +- 26 files changed, 1588 insertions(+), 136 deletions(-) create mode 100644 core-new/COPY_BUILDER_USAGE.md create mode 100644 core-new/INPUT_ABSTRACTION_ARCHITECTURE.md create mode 100644 core-new/src/infrastructure/actions/builder.rs create mode 100644 core-new/src/infrastructure/actions/output.rs create mode 100644 core-new/src/infrastructure/cli/adapters/copy.rs create mode 100644 core-new/src/infrastructure/cli/adapters/mod.rs create mode 100644 core-new/src/operations/files/copy/input.rs diff --git a/core-new/COPY_BUILDER_USAGE.md b/core-new/COPY_BUILDER_USAGE.md new file mode 100644 index 000000000..84f71f41c --- /dev/null +++ b/core-new/COPY_BUILDER_USAGE.md @@ -0,0 +1,198 @@ +# File Copy Builder Pattern Usage Examples + +This document demonstrates how the builder pattern is implemented and used for the file copy action in Spacedrive. + +## Overview + +The builder pattern provides a fluent, type-safe API for constructing file copy actions. It handles validation, CLI integration, and seamless conversion to the action system. + +## Architecture + +``` +CLI Arguments → FileCopyArgs (clap) → FileCopyActionBuilder → FileCopyAction → Action::FileCopy → Job Dispatch +``` + +## Builder API Examples + +### 1. Basic Fluent API + +```rust +use crate::operations::files::copy::action::FileCopyAction; + +// Simple single file copy +let action = FileCopyAction::builder() + .source("/path/to/source.txt") + .destination("/path/to/destination.txt") + .build()?; + +// Multiple files with options +let action = FileCopyAction::builder() + .sources(["/file1.txt", "/file2.txt", "/dir/"]) + .destination("/backup/") + .overwrite(true) + .verify_checksum(true) + .preserve_timestamps(false) + .move_files(true) // Move instead of copy + .build()?; +``` + +### 2. Convenience Methods + +```rust +// Quick single file copy +let builder = FileCopyAction::copy_file("/source.txt", "/dest.txt"); + +// Quick multiple files copy +let sources = vec!["/file1.txt", "/file2.txt"]; +let builder = FileCopyAction::copy_files(sources, "/backup/"); +``` + +### 3. CLI Integration + +The CLI automatically uses the builder pattern: + +```bash +# Basic copy +spacedrive copy file1.txt file2.txt --destination /backup/ + +# With options +spacedrive copy /photos/* --destination /backup/photos/ --overwrite --verify + +# Move operation +spacedrive copy /temp/* --destination /archive/ --move-files + +# Preserve timestamps disabled +spacedrive copy /source/ --destination /dest/ --preserve-timestamps false +``` + +### 4. Programmatic Usage + +```rust +use crate::infrastructure::actions::builder::{ActionBuilder, CliActionBuilder}; +use crate::operations::files::copy::action::{FileCopyActionBuilder, FileCopyArgs}; + +// From CLI args +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()?; + +// Direct builder usage +let action = FileCopyActionBuilder::new() + .source("/important.doc") + .destination("/backup/important.doc") + .overwrite(false) + .verify_checksum(true) + .build()?; +``` + +## Validation + +The builder provides comprehensive validation: + +```rust +// This will fail - no sources +let result = FileCopyAction::builder() + .destination("/dest/") + .build(); +assert!(result.is_err()); + +// This will fail - no destination +let result = FileCopyAction::builder() + .source("/file.txt") + .build(); +assert!(result.is_err()); + +// This will fail - source doesn't exist (if validation runs) +let result = FileCopyAction::builder() + .source("/nonexistent.txt") + .destination("/dest/") + .build(); +// Error: "Source file does not exist: /nonexistent.txt" +``` + +## CLI Command Flow + +When a user runs a copy command, here's what happens: + +1. **CLI Parsing**: `clap` parses arguments into `FileCopyArgs` +2. **Builder Creation**: `FileCopyActionBuilder::from_cli_args(args)` +3. **Validation**: Builder validates sources exist, destination is valid +4. **Action Creation**: `builder.build()` creates `FileCopyAction` +5. **Action Wrapping**: Wrapped in `Action::FileCopy { library_id, action }` +6. **Dispatch**: `action_manager.dispatch(action)` sends to handler +7. **Job Creation**: Handler creates `FileCopyJob` directly (no JSON roundtrip) +8. **Job Dispatch**: Job dispatched to job system +9. **CLI Feedback**: User sees job ID and can monitor progress + +## Error Handling + +The builder provides detailed error messages: + +```rust +match FileCopyActionBuilder::new().build() { + Err(ActionBuildError::Validation(errors)) => { + for error in errors { + println!("Validation error: {}", error); + } + // Output: + // Validation error: At least one source file must be specified + // Validation error: Destination path must be specified + } + _ => {} +} +``` + +## Benefits + +1. **Type Safety**: Compile-time validation of required fields +2. **Fluent API**: Easy to read and write +3. **Validation**: Build-time validation prevents invalid actions +4. **CLI Integration**: Seamless conversion from CLI args +5. **Performance**: Direct job creation eliminates JSON serialization +6. **Extensibility**: Easy to add new options without breaking existing code + +## Testing + +Comprehensive tests validate the builder pattern: + +```rust +#[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(); + // Validation tests... +} + +#[test] +fn test_cli_integration() { + let args = FileCopyArgs { /* ... */ }; + let action = FileCopyActionBuilder::from_cli_args(args).build().unwrap(); + // Integration tests... +} +``` + +## Future Extensions + +The builder pattern makes it easy to add new features: + +```rust +impl FileCopyActionBuilder { + // Future options + pub fn compression(mut self, level: u8) -> Self { /* ... */ } + pub fn encryption(mut self, enabled: bool) -> Self { /* ... */ } + pub fn bandwidth_limit(mut self, mbps: u32) -> Self { /* ... */ } +} +``` + +This architecture provides a solid foundation for expanding file operations while maintaining type safety and ease of use. \ No newline at end of file diff --git a/core-new/INPUT_ABSTRACTION_ARCHITECTURE.md b/core-new/INPUT_ABSTRACTION_ARCHITECTURE.md new file mode 100644 index 000000000..649574280 --- /dev/null +++ b/core-new/INPUT_ABSTRACTION_ARCHITECTURE.md @@ -0,0 +1,276 @@ +# Input Abstraction Architecture + +This document describes the refactored architecture that uses input abstraction to support multiple interfaces (CLI, GraphQL, REST, etc.) without code duplication. + +## Overview + +The input abstraction pattern separates **interface-specific argument parsing** from **core business logic** by introducing a canonical input type that all interfaces convert to. + +## Architecture Flow + +``` +┌─────────────┬─────────────────┬─────────────────┐ +│ CLI Args │ GraphQL Input │ REST Request │ +│ (clap) │ (async-graphql) │ (serde) │ +└─────────────┴─────────────────┴─────────────────┘ + │ │ │ + ▼ ▼ ▼ +┌─────────────┬─────────────────┬─────────────────┐ +│FileCopyCmd │FileCopyGQLInput │FileCopyReq │ +│Args │ │ │ +└─────────────┴─────────────────┴─────────────────┘ + │ │ │ + └─────────────────┼─────────────────┘ + ▼ + ┌─────────────────┐ + │ FileCopyInput │ ◄── Core canonical type + │ (domain) │ + └─────────────────┘ + │ + ▼ + ┌─────────────────┐ + │FileCopyAction │ + │ Builder │ + └─────────────────┘ + │ + ▼ + ┌─────────────────┐ + │ FileCopyAction │ + └─────────────────┘ + │ + ▼ + ┌─────────────────┐ + │ Job System │ + └─────────────────┘ +``` + +## File Structure + +``` +src/ +├── operations/files/copy/ +│ ├── input.rs # Core FileCopyInput type +│ ├── action.rs # FileCopyAction + Builder +│ ├── job.rs # FileCopyJob +│ └── mod.rs +├── infrastructure/ +│ ├── cli/adapters/ +│ │ ├── copy.rs # FileCopyCliArgs -> FileCopyInput +│ │ └── mod.rs +│ ├── graphql/inputs/ # Future: GraphQL inputs +│ └── rest/requests/ # Future: REST requests +``` + +## Core Components + +### 1. FileCopyInput (Domain) + +**Location**: `src/operations/files/copy/input.rs` + +The canonical input type that defines the complete interface for file copy operations: + +```rust +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FileCopyInput { + pub sources: Vec, + pub destination: PathBuf, + pub overwrite: bool, + pub verify_checksum: bool, + pub preserve_timestamps: bool, + pub move_files: bool, +} + +impl FileCopyInput { + // Fluent API for programmatic construction + pub fn new(sources: Vec, destination: PathBuf) -> Self + pub fn with_overwrite(self, overwrite: bool) -> Self + pub fn with_verification(self, verify: bool) -> Self + // ... other builder methods + + // Validation and conversion + pub fn validate(&self) -> Result<(), Vec> + pub fn to_copy_options(&self) -> CopyOptions + pub fn summary(&self) -> String +} +``` + +### 2. CLI Adapter + +**Location**: `src/infrastructure/cli/adapters/copy.rs` + +Handles CLI-specific argument parsing and converts to the core input type: + +```rust +#[derive(Debug, Clone, Parser)] +pub struct FileCopyCliArgs { + pub sources: Vec, + #[arg(short, long)] + pub destination: PathBuf, + #[arg(long)] + pub overwrite: bool, + // ... other CLI-specific options +} + +impl From for FileCopyInput { + fn from(args: FileCopyCliArgs) -> Self { + // Convert CLI args to canonical input + } +} + +impl FileCopyCliArgs { + pub fn validate_and_convert(self) -> Result +} +``` + +### 3. Updated Action Builder + +**Location**: `src/operations/files/copy/action.rs` + +The builder now uses the input abstraction as its primary interface: + +```rust +#[derive(Debug, Clone)] +pub struct FileCopyActionBuilder { + input: FileCopyInput, // Core input type + errors: Vec, +} + +impl FileCopyActionBuilder { + // Primary interface - all others convert to this + pub fn from_input(input: FileCopyInput) -> Self + + // Interface-specific convenience methods + pub fn from_cli_args(args: FileCopyCliArgs) -> Self { + Self::from_input(args.into()) + } + + // Future interfaces + pub fn from_graphql_input(input: FileCopyGraphQLInput) -> Self { + Self::from_input(input.into()) + } +} +``` + +### 4. CLI Handler + +**Location**: `src/infrastructure/cli/commands.rs` + +Simplified to use the input abstraction: + +```rust +pub async fn handle_copy_command( + args: FileCopyCliArgs, + core: &Core, + state: &mut CliState, +) -> Result<(), Box> { + // Convert and validate CLI args + let input = args.validate_and_convert()?; + + // Create action using core input + let action = FileCopyActionBuilder::from_input(input.clone()).build()?; + + // Dispatch action + let output = action_manager.dispatch(action).await?; + + // Handle output... +} +``` + +## Benefits + +### ✅ **Single Source of Truth** +- `FileCopyInput` defines the canonical interface +- All validation logic centralized in one place +- Consistent behavior across all interfaces + +### ✅ **Interface Independence** +- CLI can have CLI-specific features (help text, value parsing) +- GraphQL can use GraphQL scalars and nullable types +- REST can use different field names and structures +- Each interface optimized for its use case + +### ✅ **No Code Duplication** +- Core business logic written once +- Interface adapters are lightweight conversions +- Builder logic shared across all interfaces + +### ✅ **Easy Testing** +- Test core input types independently +- Test interface adapters separately +- Mock different interfaces easily + +### ✅ **Future Extensibility** +Adding new interfaces is straightforward: + +```rust +// Future: GraphQL support +#[derive(InputObject)] +pub struct FileCopyGraphQLInput { + pub sources: Vec, + pub destination: String, + pub options: Option, +} + +impl From for FileCopyInput { + fn from(input: FileCopyGraphQLInput) -> Self { + // Convert GraphQL input to core input + } +} + +// Update builder +impl FileCopyActionBuilder { + pub fn from_graphql_input(input: FileCopyGraphQLInput) -> Self { + Self::from_input(input.into()) + } +} +``` + +## Current CLI Usage + +The CLI interface remains exactly the same for users: + +```bash +# Basic copy +spacedrive copy file1.txt file2.txt --destination /backup/ + +# Advanced copy with options +spacedrive copy /photos/* --destination /backup/photos/ \ + --overwrite --verify --move-files + +# All existing functionality preserved +``` + +## Testing + +Comprehensive test coverage at multiple levels: + +### Core Input Tests (8 tests) +- Input validation +- Fluent API construction +- Conversion to CopyOptions +- Summary generation + +### CLI Adapter Tests (4 tests) +- CLI args → Input conversion +- Validation integration +- Default value handling + +### Action Builder Tests (7 tests) +- Builder fluent API +- Input abstraction flow +- CLI integration +- Validation scenarios + +**Total: 19 tests covering the full stack** + +## Migration Benefits + +This refactor provides: + +1. **Immediate Value**: Clean separation of concerns +2. **Future Readiness**: Easy to add GraphQL, REST APIs +3. **Maintainability**: Centralized business logic +4. **Type Safety**: Compile-time validation preserved +5. **Performance**: No runtime overhead from abstraction + +The architecture scales seamlessly as new interfaces are added, ensuring consistent behavior and preventing code duplication across the entire system. \ No newline at end of file diff --git a/core-new/src/infrastructure/actions/builder.rs b/core-new/src/infrastructure/actions/builder.rs new file mode 100644 index 000000000..877fca0d9 --- /dev/null +++ b/core-new/src/infrastructure/actions/builder.rs @@ -0,0 +1,77 @@ +//! Builder pattern traits for actions + +use std::error::Error; + +/// Core trait for action builders +pub trait ActionBuilder { + type Action; + type Error: Error + Send + Sync + 'static; + + /// Validate the current builder state + fn validate(&self) -> Result<(), Self::Error>; + + /// Build the final action instance + fn build(self) -> Result; +} + +/// Trait for builders that can be constructed from CLI arguments +pub trait CliActionBuilder: ActionBuilder { + type Args: clap::Parser; + + /// Create a builder from parsed CLI arguments + fn from_cli_args(args: Self::Args) -> Self; +} + +/// Errors that can occur during action building +#[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), + + #[error("Invalid argument: {0}")] + InvalidArgument(String), + + #[error("Required field missing: {0}")] + RequiredField(String), +} + +impl ActionBuildError { + /// Create a validation error with a single message + pub fn validation(message: impl Into) -> Self { + Self::Validation(vec![message.into()]) + } + + /// Create a validation error with multiple messages + pub fn validations(messages: Vec) -> Self { + Self::Validation(messages) + } + + /// Create a parse error + pub fn parse(message: impl Into) -> Self { + Self::Parse(message.into()) + } + + /// Create a permission error + pub fn permission(message: impl Into) -> Self { + Self::Permission(message.into()) + } + + /// Create an invalid argument error + pub fn invalid_argument(message: impl Into) -> Self { + Self::InvalidArgument(message.into()) + } + + /// Create a required field error + pub fn required_field(field: impl Into) -> Self { + Self::RequiredField(field.into()) + } +} \ No newline at end of file diff --git a/core-new/src/infrastructure/actions/handler.rs b/core-new/src/infrastructure/actions/handler.rs index 3e16b23f5..4f8ad79ff 100644 --- a/core-new/src/infrastructure/actions/handler.rs +++ b/core-new/src/infrastructure/actions/handler.rs @@ -1,6 +1,6 @@ //! Action handler trait and related types -use super::{Action, error::ActionResult, receipt::ActionReceipt}; +use super::{Action, error::ActionResult, output::ActionOutput}; use crate::context::CoreContext; use async_trait::async_trait; use std::sync::Arc; @@ -8,12 +8,12 @@ use std::sync::Arc; /// Trait that all action handlers must implement #[async_trait] pub trait ActionHandler: Send + Sync { - /// Execute the action and return a receipt + /// Execute the action and return output async fn execute( &self, context: Arc, action: Action, - ) -> ActionResult; + ) -> ActionResult; /// Validate the action before execution (optional) async fn validate( diff --git a/core-new/src/infrastructure/actions/manager.rs b/core-new/src/infrastructure/actions/manager.rs index 6f929c31e..7d0832c3d 100644 --- a/core-new/src/infrastructure/actions/manager.rs +++ b/core-new/src/infrastructure/actions/manager.rs @@ -1,7 +1,7 @@ //! Action manager - central router for all actions use super::{ - Action, error::{ActionError, ActionResult}, receipt::ActionReceipt, registry::REGISTRY, + Action, error::{ActionError, ActionResult}, output::ActionOutput, registry::REGISTRY, }; use crate::{ context::CoreContext, @@ -27,7 +27,7 @@ impl ActionManager { pub async fn dispatch( &self, action: Action, - ) -> ActionResult { + ) -> ActionResult { // 1. Find the correct handler in the registry let handler = REGISTRY .get(action.kind()) @@ -87,7 +87,7 @@ impl ActionManager { async fn finalize_audit_log( &self, mut entry: audit_log::Model, - result: &ActionResult, + result: &ActionResult, ) -> ActionResult<()> { // 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 @@ -100,11 +100,17 @@ impl ActionManager { let db = library.db().conn(); match result { - Ok(receipt) => { + Ok(output) => { entry.status = audit_log::ActionStatus::Completed; entry.completed_at = Some(chrono::Utc::now()); - entry.job_id = receipt.job_handle.as_ref().map(|h| h.id().0); - entry.result_payload = receipt.result_payload.clone().map(Into::into); + // Extract job_id from output if it contains one + entry.job_id = match output { + ActionOutput::FileCopyDispatched { job_id, .. } => Some(*job_id), + ActionOutput::FileDeleteDispatched { job_id, .. } => Some(*job_id), + ActionOutput::FileIndexDispatched { job_id, .. } => Some(*job_id), + _ => None, + }; + entry.result_payload = Some(serde_json::to_value(output).unwrap_or(serde_json::Value::Null)); } Err(error) => { entry.status = audit_log::ActionStatus::Failed; diff --git a/core-new/src/infrastructure/actions/mod.rs b/core-new/src/infrastructure/actions/mod.rs index ccc412e00..5cb9c994e 100644 --- a/core-new/src/infrastructure/actions/mod.rs +++ b/core-new/src/infrastructure/actions/mod.rs @@ -9,9 +9,11 @@ use serde::{Deserialize, Serialize}; use std::path::PathBuf; use uuid::Uuid; +pub mod builder; pub mod error; pub mod handler; pub mod manager; +pub mod output; pub mod receipt; pub mod registry; #[cfg(test)] diff --git a/core-new/src/infrastructure/actions/output.rs b/core-new/src/infrastructure/actions/output.rs new file mode 100644 index 000000000..db132663d --- /dev/null +++ b/core-new/src/infrastructure/actions/output.rs @@ -0,0 +1,151 @@ +//! Action execution output types + +use serde::{Deserialize, Serialize}; +use std::fmt; +use std::path::PathBuf; +use uuid::Uuid; + +/// Output returned from action execution +#[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, + path: PathBuf, + }, + + /// 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, + }, + + /// File indexing dispatch output + FileIndexDispatched { + job_id: Uuid, + location_id: Uuid, + }, + + /// Generic output with custom data + Custom(serde_json::Value), +} + +impl ActionOutput { + /// Create a custom output with serializable data + pub fn custom(data: T) -> Self { + Self::Custom(serde_json::to_value(data).unwrap_or(serde_json::Value::Null)) + } + + /// Create a library creation output + pub fn library_create(library_id: Uuid, name: String, path: PathBuf) -> Self { + Self::LibraryCreate { library_id, name, path } + } + + /// Create a library deletion output + pub fn library_delete(library_id: Uuid) -> Self { + Self::LibraryDelete { library_id } + } + + /// Create a folder creation output + pub fn folder_create(folder_id: Uuid, path: PathBuf) -> Self { + Self::FolderCreate { folder_id, path } + } + + /// Create a file copy dispatch output + pub fn file_copy_dispatched(job_id: Uuid, sources_count: usize) -> Self { + Self::FileCopyDispatched { job_id, sources_count } + } + + /// Create a file delete dispatch output + pub fn file_delete_dispatched(job_id: Uuid, targets_count: usize) -> Self { + Self::FileDeleteDispatched { job_id, targets_count } + } + + /// Create a location add output + pub fn location_add(location_id: Uuid, path: PathBuf) -> Self { + Self::LocationAdd { location_id, path } + } + + /// Create a location remove output + pub fn location_remove(location_id: Uuid) -> Self { + Self::LocationRemove { location_id } + } + + /// Create a file index dispatch output + pub fn file_index_dispatched(job_id: Uuid, location_id: Uuid) -> Self { + Self::FileIndexDispatched { job_id, location_id } + } +} + +impl Default for ActionOutput { + fn default() -> Self { + Self::Success + } +} + +impl fmt::Display for ActionOutput { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ActionOutput::Success => write!(f, "Action completed successfully"), + ActionOutput::LibraryCreate { library_id, name, path } => { + write!(f, "Created library '{}' (ID: {}) at path: {}", name, library_id, path.display()) + } + ActionOutput::LibraryDelete { library_id } => { + write!(f, "Deleted library with ID: {}", library_id) + } + ActionOutput::FolderCreate { folder_id, path } => { + write!(f, "Created folder (ID: {}) at path: {}", folder_id, path.display()) + } + ActionOutput::FileCopyDispatched { job_id, sources_count } => { + write!(f, "Dispatched file copy job {} for {} source(s)", job_id, sources_count) + } + ActionOutput::FileDeleteDispatched { job_id, targets_count } => { + write!(f, "Dispatched file delete job {} for {} target(s)", job_id, targets_count) + } + ActionOutput::LocationAdd { location_id, path } => { + write!(f, "Added location (ID: {}) at path: {}", location_id, path.display()) + } + ActionOutput::LocationRemove { location_id } => { + write!(f, "Removed location with ID: {}", location_id) + } + ActionOutput::FileIndexDispatched { job_id, location_id } => { + write!(f, "Dispatched file index job {} for location {}", job_id, location_id) + } + ActionOutput::Custom(value) => { + write!(f, "Custom output: {}", value) + } + } + } +} \ No newline at end of file diff --git a/core-new/src/infrastructure/cli/adapters/copy.rs b/core-new/src/infrastructure/cli/adapters/copy.rs new file mode 100644 index 000000000..d55b89b6b --- /dev/null +++ b/core-new/src/infrastructure/cli/adapters/copy.rs @@ -0,0 +1,142 @@ +//! CLI adapter for file copy operations + +use crate::operations::files::copy::input::FileCopyInput; +use clap::Parser; +use std::path::PathBuf; + +/// CLI-specific arguments for file copy command +/// This struct handles CLI parsing and converts to the core FileCopyInput type +#[derive(Debug, Clone, Parser)] +pub struct FileCopyCliArgs { + /// Source files or directories to copy + pub sources: Vec, + + /// Destination path + #[arg(short, long)] + pub destination: PathBuf, + + /// Overwrite existing files + #[arg(long)] + pub overwrite: bool, + + /// Verify checksums during copy + #[arg(long)] + pub verify: bool, + + /// Preserve file timestamps (default: true) + #[arg(long, default_value = "true")] + pub preserve_timestamps: bool, + + /// Move files instead of copying (delete source after copy) + #[arg(long)] + pub move_files: bool, +} + +impl From for FileCopyInput { + fn from(args: FileCopyCliArgs) -> Self { + Self { + sources: args.sources, + destination: args.destination, + overwrite: args.overwrite, + verify_checksum: args.verify, + preserve_timestamps: args.preserve_timestamps, + move_files: args.move_files, + } + } +} + +impl FileCopyCliArgs { + /// Convert to core input type + pub fn to_input(self) -> FileCopyInput { + self.into() + } + + /// Validate CLI arguments and convert to input + pub fn validate_and_convert(self) -> Result { + let input = self.to_input(); + + match input.validate() { + Ok(()) => Ok(input), + Err(errors) => Err(errors.join("; ")), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_cli_to_input_conversion() { + let cli_args = FileCopyCliArgs { + sources: vec!["/file1.txt".into(), "/file2.txt".into()], + destination: "/dest/".into(), + overwrite: true, + verify: false, + preserve_timestamps: true, + move_files: false, + }; + + let input: FileCopyInput = cli_args.into(); + + assert_eq!(input.sources.len(), 2); + assert_eq!(input.destination, PathBuf::from("/dest/")); + assert!(input.overwrite); + assert!(!input.verify_checksum); + assert!(input.preserve_timestamps); + assert!(!input.move_files); + } + + #[test] + fn test_validate_and_convert_success() { + let cli_args = FileCopyCliArgs { + sources: vec!["/file.txt".into()], + destination: "/dest/".into(), + overwrite: false, + verify: true, + preserve_timestamps: false, + move_files: true, + }; + + let result = cli_args.validate_and_convert(); + assert!(result.is_ok()); + + let input = result.unwrap(); + assert_eq!(input.sources, vec![PathBuf::from("/file.txt")]); + assert!(input.verify_checksum); + assert!(!input.preserve_timestamps); + assert!(input.move_files); + } + + #[test] + fn test_validate_and_convert_failure() { + let cli_args = FileCopyCliArgs { + sources: vec![], // Empty sources should fail + destination: "/dest/".into(), + overwrite: false, + verify: false, + preserve_timestamps: true, + move_files: false, + }; + + let result = cli_args.validate_and_convert(); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("At least one source")); + } + + #[test] + fn test_default_values() { + // Test that clap default values work as expected + let cli_args = FileCopyCliArgs { + sources: vec!["/file.txt".into()], + destination: "/dest/".into(), + overwrite: false, + verify: false, + preserve_timestamps: true, // Should default to true + move_files: false, + }; + + let input = cli_args.to_input(); + assert!(input.preserve_timestamps); // Default should be true + } +} \ No newline at end of file diff --git a/core-new/src/infrastructure/cli/adapters/mod.rs b/core-new/src/infrastructure/cli/adapters/mod.rs new file mode 100644 index 000000000..2af269010 --- /dev/null +++ b/core-new/src/infrastructure/cli/adapters/mod.rs @@ -0,0 +1,6 @@ +//! CLI adapters for converting CLI arguments to domain input types + +pub mod copy; + +// Re-export for convenience +pub use copy::FileCopyCliArgs; \ No newline at end of file diff --git a/core-new/src/infrastructure/cli/commands.rs b/core-new/src/infrastructure/cli/commands.rs index b5542a9d1..8d7de3eb6 100644 --- a/core-new/src/infrastructure/cli/commands.rs +++ b/core-new/src/infrastructure/cli/commands.rs @@ -3,8 +3,12 @@ use crate::{ infrastructure::{database::entities, jobs::types::JobStatus}, library::Library, location::{create_location, LocationCreateArgs}, - infrastructure::actions::Action, + infrastructure::{ + actions::{Action, builder::ActionBuilder}, + cli::adapters::FileCopyCliArgs, + }, operations::{ + files::copy::action::FileCopyActionBuilder, indexing::{IndexMode, IndexScope}, }, shared::types::SdPath, @@ -319,34 +323,21 @@ pub async fn handle_library_command( // Dispatch the action match action_manager.dispatch(action).await { - Ok(receipt) => { - if let Some(payload) = receipt.result_payload { - if let (Some(lib_id), Some(lib_path)) = ( - payload - .get("library_id") - .and_then(|v| v.as_str()) - .and_then(|s| uuid::Uuid::parse_str(s).ok()), - payload.get("path").and_then(|v| v.as_str()), - ) { - state.set_current_library(lib_id, std::path::PathBuf::from(lib_path)); + Ok(output) => { + match output { + crate::infrastructure::actions::output::ActionOutput::LibraryCreate { library_id, name, path } => { + state.set_current_library(library_id, path.clone()); println!("✅ Library created successfully!"); - println!(" ID: {}", lib_id.to_string().bright_yellow()); - println!(" Path: {}", lib_path.bright_blue()); + println!(" ID: {}", library_id.to_string().bright_yellow()); + println!(" Name: {}", name.bright_blue()); + println!(" Path: {}", path.display().to_string().bright_blue()); println!(" Status: {}", "Active".bright_green()); - } else { - println!("✅ Library created successfully!"); - println!( - " Action ID: {}", - receipt.action_id.to_string().bright_yellow() - ); } - } else { - println!("✅ Library creation initiated!"); - println!( - " Action ID: {}", - receipt.action_id.to_string().bright_yellow() - ); + _ => { + println!("✅ Library created successfully!"); + println!(" Output: {}", output); + } } } Err(e) => { @@ -514,13 +505,11 @@ pub async fn handle_location_command( // Dispatch the action match action_manager.dispatch(action).await { - Ok(receipt) => { - if let Some(payload) = receipt.result_payload { - if let Some(location_id) = - payload.get("location_id").and_then(|v| v.as_str()) - { + Ok(output) => { + match output { + crate::infrastructure::actions::output::ActionOutput::LocationAdd { location_id, path: location_path } => { println!("✅ Location added successfully!"); - println!(" ID: {}", location_id.bright_yellow()); + println!(" ID: {}", location_id.to_string().bright_yellow()); println!( " Name: {}", name.unwrap_or_else(|| path @@ -530,37 +519,13 @@ pub async fn handle_location_command( .to_string()) .bright_cyan() ); - println!(" Path: {}", path.display().to_string().bright_blue()); - - if receipt.job_handle.is_some() { - println!( - " Status: {} (job dispatched)", - "Indexing".bright_yellow() - ); - println!( - "\n💡 Tip: Monitor indexing progress with: {}", - "spacedrive job monitor".bright_cyan() - ); - } else { - println!(" Status: {}", "Ready".bright_green()); - } - } else { - println!("✅ Location addition initiated!"); - println!( - " Action ID: {}", - receipt.action_id.to_string().bright_yellow() - ); + println!(" Path: {}", location_path.display().to_string().bright_blue()); + println!(" Status: {}", "Ready".bright_green()); + } + _ => { + println!("✅ Location added successfully!"); + println!(" Output: {}", output); } - } else if receipt.job_handle.is_some() { - println!("✅ Location addition job started!"); - println!( - " Action ID: {}", - receipt.action_id.to_string().bright_yellow() - ); - println!( - "\n💡 Tip: Monitor progress with: {}", - "spacedrive job monitor".bright_cyan() - ); } } Err(e) => { @@ -1403,3 +1368,87 @@ fn format_bytes(bytes: u64) -> String { format!("{:.2} {}", size, UNITS[unit_index]) } + +/// Handle file copy command using the builder pattern with input abstraction +pub async fn handle_copy_command( + args: FileCopyCliArgs, + core: &Core, + state: &mut CliState, +) -> Result<(), Box> { + use colored::Colorize; + + // Get the current library + let library = get_current_library(core, state).await?; + let library_id = library.id(); + + // Convert CLI args to core input and validate + let input = match args.validate_and_convert() { + Ok(input) => input, + Err(e) => { + println!("❌ Invalid copy operation: {}", e); + return Ok(()); + } + }; + + println!("📁 {}", input.summary().bright_cyan()); + + // Use the builder pattern to create the action + let action = match FileCopyActionBuilder::from_input(input.clone()).build() { + Ok(action) => action, + Err(e) => { + println!("❌ Failed to build copy action: {}", e); + return Ok(()); + } + }; + + // Create the full Action enum + let full_action = Action::FileCopy { + library_id, + action, + }; + + // Get the action manager and dispatch the action + let action_manager = core + .context + .get_action_manager() + .await + .ok_or("Action manager not available")?; + + // Dispatch the action and handle the result + match action_manager.dispatch(full_action).await { + Ok(output) => { + match output { + crate::infrastructure::actions::output::ActionOutput::FileCopyDispatched { job_id, sources_count } => { + println!("✅ {} operation dispatched successfully!", + if input.move_files { "Move" } else { "Copy" }); + println!(" Job ID: {}", job_id.to_string().bright_yellow()); + println!(" Sources: {} file(s)", sources_count); + println!(" Destination: {}", input.destination.display().to_string().bright_blue()); + + if input.overwrite { + println!(" Mode: {} existing files", "Overwrite".bright_red()); + } + if input.verify_checksum { + println!(" Verification: {}", "Enabled".bright_green()); + } + if input.move_files { + println!(" Type: {} (delete source after copy)", "Move".bright_yellow()); + } + + println!("\n💡 Tip: Monitor progress with: {}", + "spacedrive job monitor".bright_cyan()); + } + _ => { + println!("✅ Copy operation completed!"); + println!(" Output: {}", output); + } + } + } + Err(e) => { + println!("❌ Failed to copy files: {}", e); + return Err(Box::new(e)); + } + } + + Ok(()) +} diff --git a/core-new/src/infrastructure/cli/mod.rs b/core-new/src/infrastructure/cli/mod.rs index c1061c4a7..8d125a878 100644 --- a/core-new/src/infrastructure/cli/mod.rs +++ b/core-new/src/infrastructure/cli/mod.rs @@ -1,3 +1,4 @@ +pub mod adapters; pub mod commands; pub mod daemon; pub mod monitor; @@ -92,6 +93,9 @@ pub enum Commands { /// Manage device networking and connections #[command(subcommand)] Network(commands::NetworkCommands), + + /// Copy files using the action system + Copy(adapters::FileCopyCliArgs), } #[derive(Subcommand, Clone)] @@ -229,6 +233,9 @@ pub async fn run() -> Result<(), Box> { Commands::Job(cmd) => commands::handle_job_command(cmd, &core, &mut state).await?, Commands::Index(cmd) => commands::handle_index_command(cmd, &core, &mut state).await?, Commands::Network(cmd) => commands::handle_network_command(cmd, &core, &mut state).await?, + Commands::Copy(args) => { + commands::handle_copy_command(args, &core, &mut state).await? + } Commands::Scan { path, mode, watch } => { commands::handle_legacy_scan_command(path, mode, watch, &core, &mut state).await? } diff --git a/core-new/src/operations/content/action.rs b/core-new/src/operations/content/action.rs index 1ded09fa7..3007796b3 100644 --- a/core-new/src/operations/content/action.rs +++ b/core-new/src/operations/content/action.rs @@ -5,7 +5,7 @@ use crate::{ infrastructure::actions::{ error::{ActionError, ActionResult}, handler::ActionHandler, - receipt::ActionReceipt, + output::ActionOutput, }, register_action_handler, }; @@ -43,7 +43,7 @@ impl ActionHandler for ContentHandler { &self, context: Arc, action: crate::infrastructure::actions::Action, - ) -> ActionResult { + ) -> ActionResult { // TODO: Re-enable when ContentAnalysis variant is added back Err(ActionError::Internal("ContentAnalysis action not yet implemented".to_string())) } diff --git a/core-new/src/operations/files/copy/action.rs b/core-new/src/operations/files/copy/action.rs index 7c9ad344c..108ee0cce 100644 --- a/core-new/src/operations/files/copy/action.rs +++ b/core-new/src/operations/files/copy/action.rs @@ -1,18 +1,23 @@ //! File copy action handler -use super::job::{CopyOptions, FileCopyJob}; +use super::{input::FileCopyInput, job::{CopyOptions, FileCopyJob}}; use crate::{ context::CoreContext, - infrastructure::actions::{ - error::{ActionError, ActionResult}, - handler::ActionHandler, - receipt::ActionReceipt, - Action, + infrastructure::{ + actions::{ + builder::{ActionBuilder, ActionBuildError}, + error::{ActionError, ActionResult}, + handler::ActionHandler, + output::ActionOutput, + Action, + }, + cli::adapters::FileCopyCliArgs, }, register_action_handler, shared::types::{SdPath, SdPathBatch}, }; use async_trait::async_trait; +use clap::Parser; use serde::{Deserialize, Serialize}; use std::{path::PathBuf, sync::Arc}; use uuid::Uuid; @@ -24,6 +29,168 @@ pub struct FileCopyAction { pub options: CopyOptions, } +/// Builder for creating FileCopyAction instances with fluent API +#[derive(Debug, Clone)] +pub struct FileCopyActionBuilder { + input: FileCopyInput, + errors: Vec, +} + +impl FileCopyActionBuilder { + /// Create a new builder + pub fn new() -> Self { + Self { + input: FileCopyInput::default(), + errors: Vec::new(), + } + } + + /// Create builder from core input type (primary interface) + pub fn from_input(input: FileCopyInput) -> Self { + Self { + input, + errors: Vec::new(), + } + } + + /// Add multiple source files + pub fn sources(mut self, sources: I) -> Self + where + I: IntoIterator, + P: Into + { + self.input.sources.extend(sources.into_iter().map(|p| p.into())); + self + } + + /// Add a single source file + pub fn source>(mut self, source: P) -> Self { + self.input.sources.push(source.into()); + self + } + + /// Set the destination path + pub fn destination>(mut self, dest: P) -> Self { + self.input.destination = dest.into(); + self + } + + /// Set whether to overwrite existing files + pub fn overwrite(mut self, overwrite: bool) -> Self { + self.input.overwrite = overwrite; + self + } + + /// Set whether to verify checksums during copy + pub fn verify_checksum(mut self, verify: bool) -> Self { + self.input.verify_checksum = verify; + self + } + + /// Set whether to preserve file timestamps + pub fn preserve_timestamps(mut self, preserve: bool) -> Self { + self.input.preserve_timestamps = preserve; + self + } + + /// Enable move mode (delete source after copy) + pub fn move_files(mut self, enable: bool) -> Self { + self.input.move_files = enable; + self + } + + /// Validate sources exist and are readable + fn validate_sources(&mut self) { + // First do basic validation from input + if let Err(basic_errors) = self.input.validate() { + self.errors.extend(basic_errors); + return; + } + + // Then do filesystem validation + for source in &self.input.sources { + if !source.exists() { + self.errors.push(format!("Source file does not exist: {}", source.display())); + } else if source.is_dir() && !source.read_dir().is_ok() { + self.errors.push(format!("Cannot read directory: {}", source.display())); + } else if source.is_file() && std::fs::metadata(source).is_err() { + self.errors.push(format!("Cannot access file: {}", source.display())); + } + } + } + + /// Validate destination is valid + fn validate_destination(&mut self) { + if let Some(parent) = self.input.destination.parent() { + if !parent.exists() { + self.errors.push(format!("Destination directory does not exist: {}", parent.display())); + } + } + } +} + +impl ActionBuilder for FileCopyActionBuilder { + type Action = FileCopyAction; + type Error = ActionBuildError; + + fn validate(&self) -> Result<(), Self::Error> { + let mut builder = self.clone(); + builder.validate_sources(); + builder.validate_destination(); + + if !builder.errors.is_empty() { + return Err(ActionBuildError::validations(builder.errors)); + } + + Ok(()) + } + + fn build(self) -> Result { + self.validate()?; + + let options = self.input.to_copy_options(); + Ok(FileCopyAction { + sources: self.input.sources, + destination: self.input.destination, + options, + }) + } +} + +impl FileCopyActionBuilder { + /// Create builder from CLI args (interface-specific convenience method) + pub fn from_cli_args(args: FileCopyCliArgs) -> Self { + Self::from_input(args.into()) + } +} + +/// Convenience methods on FileCopyAction +impl FileCopyAction { + /// Create a new builder + pub fn builder() -> FileCopyActionBuilder { + FileCopyActionBuilder::new() + } + + /// Quick builder for copying a single file + pub fn copy_file, D: Into>(source: S, dest: D) -> FileCopyActionBuilder { + FileCopyActionBuilder::new() + .source(source) + .destination(dest) + } + + /// Quick builder for copying multiple files + pub fn copy_files(sources: I, dest: D) -> FileCopyActionBuilder + where + I: IntoIterator, + P: Into, + D: Into + { + FileCopyActionBuilder::new() + .sources(sources) + .destination(dest) + } +} + pub struct FileCopyHandler; impl FileCopyHandler { @@ -63,7 +230,7 @@ impl ActionHandler for FileCopyHandler { &self, context: Arc, action: Action, - ) -> ActionResult { + ) -> ActionResult { if let Action::FileCopy { library_id, action } = action { let library_manager = &context.library_manager; @@ -73,7 +240,8 @@ impl ActionHandler for FileCopyHandler { .await .ok_or(ActionError::LibraryNotFound(library_id))?; - // Create job instance + // Create job instance directly (no JSON roundtrip) + let sources_count = action.sources.len(); let sources = action .sources .into_iter() @@ -84,14 +252,15 @@ impl ActionHandler for FileCopyHandler { FileCopyJob::new(SdPathBatch::new(sources), SdPath::local(action.destination)) .with_options(action.options); - // Dispatch the job + // Dispatch job directly let job_handle = library .jobs() .dispatch(job) .await .map_err(ActionError::Job)?; - Ok(ActionReceipt::job_based(Uuid::new_v4(), job_handle)) + // Return action output instead of receipt + Ok(ActionOutput::file_copy_dispatched(job_handle.id().into(), sources_count)) } else { Err(ActionError::InvalidActionType) } @@ -108,3 +277,152 @@ impl ActionHandler for FileCopyHandler { // Register this handler register_action_handler!(FileCopyHandler, "file.copy"); + +#[cfg(test)] +mod tests { + use super::*; + use crate::infrastructure::cli::adapters::FileCopyCliArgs; + use std::path::PathBuf; + + #[test] + fn test_builder_fluent_api() { + let action = FileCopyAction::builder() + .sources(["/src/file1.txt", "/src/file2.txt"]) + .destination("/dest/") + .overwrite(true) + .verify_checksum(true) + .preserve_timestamps(false) + .move_files(true) + .build(); + + // Note: This will fail validation because files don't exist, but it tests the API + assert!(action.is_err()); + match action.unwrap_err() { + ActionBuildError::Validation(errors) => { + assert!(!errors.is_empty()); + assert!(errors.iter().any(|e| e.contains("does not exist"))); + } + _ => panic!("Expected validation error"), + } + } + + #[test] + fn test_builder_validation_empty_sources() { + let result = FileCopyAction::builder() + .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_builder_from_input() { + let input = FileCopyInput::new( + vec!["/file1.txt".into(), "/file2.txt".into()], + "/dest/" + ) + .with_overwrite(true) + .with_verification(true) + .with_move(false); + + let builder = FileCopyActionBuilder::from_input(input.clone()); + + // Test that builder has correct values from input + assert_eq!(builder.input.sources, vec![PathBuf::from("/file1.txt"), PathBuf::from("/file2.txt")]); + assert_eq!(builder.input.destination, PathBuf::from("/dest/")); + assert!(builder.input.overwrite); + assert!(builder.input.verify_checksum); + assert!(!builder.input.move_files); + } + + #[test] + fn test_cli_integration() { + let args = FileCopyCliArgs { + sources: vec!["/src/file.txt".into()], + destination: "/dest/".into(), + overwrite: true, + verify: false, + preserve_timestamps: true, + move_files: false, + }; + + let builder = FileCopyActionBuilder::from_cli_args(args); + + // Test that builder has correct values set from CLI args + assert_eq!(builder.input.sources, vec![PathBuf::from("/src/file.txt")]); + assert_eq!(builder.input.destination, PathBuf::from("/dest/")); + assert!(builder.input.overwrite); + assert!(!builder.input.verify_checksum); + assert!(builder.input.preserve_timestamps); + assert!(!builder.input.move_files); + } + + #[test] + fn test_convenience_methods() { + // Test single file copy + let builder = FileCopyAction::copy_file("/src/file.txt", "/dest/file.txt"); + assert_eq!(builder.input.sources, vec![PathBuf::from("/src/file.txt")]); + assert_eq!(builder.input.destination, PathBuf::from("/dest/file.txt")); + + // Test multiple files copy + let sources = vec!["/src/file1.txt", "/src/file2.txt"]; + let builder = FileCopyAction::copy_files(sources.clone(), "/dest/"); + assert_eq!(builder.input.sources, sources.into_iter().map(PathBuf::from).collect::>()); + assert_eq!(builder.input.destination, PathBuf::from("/dest/")); + } + + #[test] + fn test_builder_chaining() { + let builder = FileCopyAction::builder() + .source("/file1.txt") + .source("/file2.txt") + .source("/file3.txt") + .destination("/dest/") + .overwrite(true) + .verify_checksum(false) + .preserve_timestamps(true) + .move_files(false); + + assert_eq!(builder.input.sources.len(), 3); + assert!(builder.input.overwrite); + assert!(!builder.input.verify_checksum); + assert!(builder.input.preserve_timestamps); + assert!(!builder.input.move_files); + } + + #[test] + fn test_input_abstraction_flow() { + // Test the full flow: CLI args -> Input -> Builder -> Action + let cli_args = FileCopyCliArgs { + sources: vec!["/source.txt".into()], + destination: "/dest.txt".into(), + overwrite: false, + verify: true, + preserve_timestamps: false, + move_files: true, + }; + + // Convert CLI args to input + let input: FileCopyInput = cli_args.into(); + assert_eq!(input.sources, vec![PathBuf::from("/source.txt")]); + assert!(input.verify_checksum); + assert!(!input.preserve_timestamps); + assert!(input.move_files); + + // Create builder from input + let builder = FileCopyActionBuilder::from_input(input); + + // Verify the copy options are correct + let copy_options = builder.input.to_copy_options(); + assert!(!copy_options.overwrite); + assert!(copy_options.verify_checksum); + assert!(!copy_options.preserve_timestamps); + assert!(copy_options.delete_after_copy); + } +} diff --git a/core-new/src/operations/files/copy/input.rs b/core-new/src/operations/files/copy/input.rs new file mode 100644 index 000000000..9d595be12 --- /dev/null +++ b/core-new/src/operations/files/copy/input.rs @@ -0,0 +1,234 @@ +//! Core input types for file copy operations + +use super::job::CopyOptions; +use std::path::PathBuf; + +/// Core input structure for file copy operations +/// This is the canonical interface that all external APIs (CLI, GraphQL, REST) convert to +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FileCopyInput { + /// Source files or directories to copy + pub sources: Vec, + + /// Destination path + pub destination: PathBuf, + + /// Whether to overwrite existing files + pub overwrite: bool, + + /// Whether to verify checksums during copy + pub verify_checksum: bool, + + /// Whether to preserve file timestamps + pub preserve_timestamps: bool, + + /// Whether to delete source files after copying (move operation) + pub move_files: bool, +} + +impl FileCopyInput { + /// Create a new FileCopyInput with default options + pub fn new>(sources: Vec, destination: D) -> Self { + Self { + sources, + destination: destination.into(), + overwrite: false, + verify_checksum: false, + preserve_timestamps: true, + move_files: false, + } + } + + /// Create a single file copy input + pub fn single_file, D: Into>(source: S, destination: D) -> Self { + Self::new(vec![source.into()], destination) + } + + /// Set overwrite option + pub fn with_overwrite(mut self, overwrite: bool) -> Self { + self.overwrite = overwrite; + self + } + + /// Set checksum verification option + pub fn with_verification(mut self, verify: bool) -> Self { + self.verify_checksum = verify; + self + } + + /// Set timestamp preservation option + pub fn with_timestamp_preservation(mut self, preserve: bool) -> Self { + self.preserve_timestamps = preserve; + self + } + + /// Set move files option + pub fn with_move(mut self, move_files: bool) -> Self { + self.move_files = move_files; + self + } + + /// Convert to CopyOptions for the job system + pub fn to_copy_options(&self) -> CopyOptions { + CopyOptions { + overwrite: self.overwrite, + verify_checksum: self.verify_checksum, + preserve_timestamps: self.preserve_timestamps, + delete_after_copy: self.move_files, + move_mode: None, // Will be determined by job system + } + } + + /// Validate the input + pub fn validate(&self) -> Result<(), Vec> { + let mut errors = Vec::new(); + + if self.sources.is_empty() { + errors.push("At least one source file must be specified".to_string()); + } + + // Validate each source path (basic validation - existence check done in builder) + for source in &self.sources { + if source.as_os_str().is_empty() { + errors.push("Source path cannot be empty".to_string()); + } + } + + // Validate destination + if self.destination.as_os_str().is_empty() { + errors.push("Destination path cannot be empty".to_string()); + } + + if errors.is_empty() { + Ok(()) + } else { + Err(errors) + } + } + + /// Get a summary string for logging/display + pub fn summary(&self) -> String { + let operation = if self.move_files { "Move" } else { "Copy" }; + let source_count = self.sources.len(); + let source_desc = if source_count == 1 { + "1 source".to_string() + } else { + format!("{} sources", source_count) + }; + + format!( + "{} {} to {}", + operation, + source_desc, + self.destination.display() + ) + } +} + +impl Default for FileCopyInput { + fn default() -> Self { + Self { + sources: Vec::new(), + destination: PathBuf::new(), + overwrite: false, + verify_checksum: false, + preserve_timestamps: true, + move_files: false, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_new_input() { + let input = FileCopyInput::new( + vec!["/file1.txt".into(), "/file2.txt".into()], + "/dest/" + ); + + assert_eq!(input.sources.len(), 2); + assert_eq!(input.destination, PathBuf::from("/dest/")); + assert!(!input.overwrite); + assert!(input.preserve_timestamps); + assert!(!input.move_files); + } + + #[test] + fn test_single_file() { + let input = FileCopyInput::single_file("/source.txt", "/dest.txt"); + + assert_eq!(input.sources, vec![PathBuf::from("/source.txt")]); + assert_eq!(input.destination, PathBuf::from("/dest.txt")); + } + + #[test] + fn test_fluent_api() { + let input = FileCopyInput::single_file("/source.txt", "/dest.txt") + .with_overwrite(true) + .with_verification(true) + .with_timestamp_preservation(false) + .with_move(true); + + assert!(input.overwrite); + assert!(input.verify_checksum); + assert!(!input.preserve_timestamps); + assert!(input.move_files); + } + + #[test] + fn test_validation_empty_sources() { + let input = FileCopyInput::default(); + let result = input.validate(); + + assert!(result.is_err()); + let errors = result.unwrap_err(); + assert!(errors.iter().any(|e| e.contains("At least one source"))); + } + + #[test] + fn test_validation_empty_destination() { + let mut input = FileCopyInput::default(); + input.sources = vec!["/file.txt".into()]; + + let result = input.validate(); + assert!(result.is_err()); + let errors = result.unwrap_err(); + assert!(errors.iter().any(|e| e.contains("Destination path cannot be empty"))); + } + + #[test] + fn test_validation_success() { + let input = FileCopyInput::new(vec!["/file.txt".into()], "/dest/"); + assert!(input.validate().is_ok()); + } + + #[test] + fn test_summary() { + let input = FileCopyInput::new(vec!["/file1.txt".into(), "/file2.txt".into()], "/dest/"); + assert_eq!(input.summary(), "Copy 2 sources to /dest/"); + + let move_input = input.with_move(true); + assert_eq!(move_input.summary(), "Move 2 sources to /dest/"); + + let single_input = FileCopyInput::single_file("/file.txt", "/dest.txt"); + assert_eq!(single_input.summary(), "Copy 1 source to /dest.txt"); + } + + #[test] + fn test_to_copy_options() { + let input = FileCopyInput::single_file("/source.txt", "/dest.txt") + .with_overwrite(true) + .with_verification(true) + .with_timestamp_preservation(false) + .with_move(true); + + let options = input.to_copy_options(); + assert!(options.overwrite); + assert!(options.verify_checksum); + assert!(!options.preserve_timestamps); + assert!(options.delete_after_copy); + } +} \ No newline at end of file diff --git a/core-new/src/operations/files/copy/mod.rs b/core-new/src/operations/files/copy/mod.rs index cf465fd87..d233dabe3 100644 --- a/core-new/src/operations/files/copy/mod.rs +++ b/core-new/src/operations/files/copy/mod.rs @@ -1,6 +1,7 @@ //! Modular file copy operations using the Strategy Pattern pub mod action; +pub mod input; pub mod job; pub mod routing; pub mod strategy; diff --git a/core-new/src/operations/files/delete/action.rs b/core-new/src/operations/files/delete/action.rs index 17f32aa17..da3f4ce58 100644 --- a/core-new/src/operations/files/delete/action.rs +++ b/core-new/src/operations/files/delete/action.rs @@ -3,7 +3,7 @@ use crate::{ context::CoreContext, infrastructure::actions::{ - Action, error::{ActionError, ActionResult}, handler::ActionHandler, receipt::ActionReceipt, + Action, error::{ActionError, ActionResult}, handler::ActionHandler, output::ActionOutput, }, register_action_handler, shared::types::{SdPath, SdPathBatch}, @@ -52,7 +52,7 @@ impl ActionHandler for FileDeleteHandler { &self, context: Arc, action: Action, - ) -> ActionResult { + ) -> ActionResult { if let Action::FileDelete { library_id, action } = action { let library_manager = &context.library_manager; @@ -62,7 +62,8 @@ impl ActionHandler for FileDeleteHandler { .await .ok_or(ActionError::LibraryNotFound(library_id))?; - // Create job instance directly + // Create job instance directly (no JSON roundtrip) + let targets_count = action.targets.len(); let targets = action.targets .into_iter() .map(|path| SdPath::local(path)) @@ -83,7 +84,8 @@ impl ActionHandler for FileDeleteHandler { .await .map_err(ActionError::Job)?; - Ok(ActionReceipt::job_based(Uuid::new_v4(), job_handle)) + // Return action output instead of receipt + Ok(ActionOutput::file_delete_dispatched(job_handle.id().into(), targets_count)) } else { Err(ActionError::InvalidActionType) } diff --git a/core-new/src/operations/files/duplicate_detection/action.rs b/core-new/src/operations/files/duplicate_detection/action.rs index fdda1b62e..219b538ff 100644 --- a/core-new/src/operations/files/duplicate_detection/action.rs +++ b/core-new/src/operations/files/duplicate_detection/action.rs @@ -5,7 +5,7 @@ use crate::{ infrastructure::actions::{ error::{ActionError, ActionResult}, handler::ActionHandler, - receipt::ActionReceipt, + output::ActionOutput, }, register_action_handler, shared::types::{SdPath, SdPathBatch}, @@ -54,7 +54,7 @@ impl ActionHandler for DuplicateDetectionHandler { &self, context: Arc, action: crate::infrastructure::actions::Action, - ) -> ActionResult { + ) -> ActionResult { if let crate::infrastructure::actions::Action::DetectDuplicates { library_id, action } = action { let library_manager = &context.library_manager; @@ -85,7 +85,7 @@ impl ActionHandler for DuplicateDetectionHandler { .await .map_err(ActionError::Job)?; - Ok(ActionReceipt::job_based(Uuid::new_v4(), job_handle)) + Ok(ActionOutput::Success) } else { Err(ActionError::InvalidActionType) } diff --git a/core-new/src/operations/files/validation/action.rs b/core-new/src/operations/files/validation/action.rs index 20b43976e..95919bb8e 100644 --- a/core-new/src/operations/files/validation/action.rs +++ b/core-new/src/operations/files/validation/action.rs @@ -5,7 +5,7 @@ use crate::{ infrastructure::actions::{ error::{ActionError, ActionResult}, handler::ActionHandler, - receipt::ActionReceipt, + output::ActionOutput, }, register_action_handler, shared::types::{SdPath, SdPathBatch}, @@ -54,7 +54,7 @@ impl ActionHandler for ValidationHandler { &self, context: Arc, action: crate::infrastructure::actions::Action, - ) -> ActionResult { + ) -> ActionResult { if let crate::infrastructure::actions::Action::FileValidate { library_id, action } = action { let library_manager = &context.library_manager; @@ -85,7 +85,7 @@ impl ActionHandler for ValidationHandler { .await .map_err(ActionError::Job)?; - Ok(ActionReceipt::job_based(Uuid::new_v4(), job_handle)) + Ok(ActionOutput::Success) } else { Err(ActionError::InvalidActionType) } diff --git a/core-new/src/operations/indexing/action.rs b/core-new/src/operations/indexing/action.rs index 13bed2b7a..00c4e5424 100644 --- a/core-new/src/operations/indexing/action.rs +++ b/core-new/src/operations/indexing/action.rs @@ -5,7 +5,7 @@ use crate::{ infrastructure::actions::{ error::{ActionError, ActionResult}, handler::ActionHandler, - receipt::ActionReceipt, + output::ActionOutput, }, register_action_handler, shared::types::SdPath, @@ -54,7 +54,7 @@ impl ActionHandler for IndexingHandler { &self, context: Arc, action: crate::infrastructure::actions::Action, - ) -> ActionResult { + ) -> ActionResult { if let crate::infrastructure::actions::Action::Index { library_id, action } = action { let library_manager = &context.library_manager; @@ -84,7 +84,7 @@ impl ActionHandler for IndexingHandler { .await .map_err(ActionError::Job)?; - Ok(ActionReceipt::job_based(Uuid::new_v4(), job_handle)) + Ok(ActionOutput::Success) } else { Err(ActionError::InvalidActionType) } diff --git a/core-new/src/operations/libraries/create/action.rs b/core-new/src/operations/libraries/create/action.rs index 91395764f..895fff08d 100644 --- a/core-new/src/operations/libraries/create/action.rs +++ b/core-new/src/operations/libraries/create/action.rs @@ -5,7 +5,7 @@ use crate::{ infrastructure::actions::{ error::{ActionError, ActionResult}, handler::ActionHandler, - receipt::ActionReceipt, + output::ActionOutput, }, register_action_handler, }; @@ -52,19 +52,16 @@ impl ActionHandler for LibraryCreateHandler { &self, context: Arc, action: crate::infrastructure::actions::Action, - ) -> ActionResult { + ) -> 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 new_library = library_manager.create_library(action.name.clone(), action.path.clone()).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() - })), + Ok(ActionOutput::library_create( + new_library.id(), + library_name, + new_library.path().to_path_buf(), )) } else { Err(ActionError::InvalidActionType) diff --git a/core-new/src/operations/libraries/delete/action.rs b/core-new/src/operations/libraries/delete/action.rs index 3ac312e69..3d5351270 100644 --- a/core-new/src/operations/libraries/delete/action.rs +++ b/core-new/src/operations/libraries/delete/action.rs @@ -3,7 +3,7 @@ use crate::{ context::CoreContext, infrastructure::actions::{ - Action, error::{ActionError, ActionResult}, handler::ActionHandler, receipt::ActionReceipt, + Action, error::{ActionError, ActionResult}, handler::ActionHandler, output::ActionOutput, }, register_action_handler, }; @@ -31,7 +31,7 @@ impl ActionHandler for LibraryDeleteHandler { &self, context: Arc, action: Action, - ) -> ActionResult { + ) -> ActionResult { 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 diff --git a/core-new/src/operations/locations/add/action.rs b/core-new/src/operations/locations/add/action.rs index 4c940f93e..a1996c44e 100644 --- a/core-new/src/operations/locations/add/action.rs +++ b/core-new/src/operations/locations/add/action.rs @@ -6,7 +6,7 @@ use crate::{ location::manager::LocationManager, infrastructure::{ actions::{ - Action, error::{ActionError, ActionResult}, handler::ActionHandler, receipt::ActionReceipt, + Action, error::{ActionError, ActionResult}, handler::ActionHandler, output::ActionOutput, }, }, operations::{ @@ -66,7 +66,7 @@ impl ActionHandler for LocationAddHandler { &self, context: Arc, action: Action, - ) -> ActionResult { + ) -> ActionResult { if let Action::LocationAdd { library_id, action } = action { let library_manager = &context.library_manager; @@ -121,15 +121,7 @@ impl ActionHandler for LocationAddHandler { Some(job_handle) }; - Ok(ActionReceipt::hybrid( - Uuid::new_v4(), - Some(serde_json::json!({ - "location_id": location_id, - "name": location_name, - "path": action.path.display().to_string() - })), - job_handle, - )) + Ok(ActionOutput::location_add(location_id, action.path)) } else { Err(ActionError::InvalidActionType) } diff --git a/core-new/src/operations/locations/index/action.rs b/core-new/src/operations/locations/index/action.rs index 62831b5d5..de3f2c0f8 100644 --- a/core-new/src/operations/locations/index/action.rs +++ b/core-new/src/operations/locations/index/action.rs @@ -4,7 +4,7 @@ use crate::{ context::CoreContext, infrastructure::{ actions::{ - Action, error::{ActionError, ActionResult}, handler::ActionHandler, receipt::ActionReceipt, + Action, error::{ActionError, ActionResult}, handler::ActionHandler, output::ActionOutput, }, }, operations::{ @@ -38,7 +38,7 @@ impl ActionHandler for LocationIndexHandler { &self, context: Arc, action: Action, - ) -> ActionResult { + ) -> ActionResult { if let Action::LocationIndex { library_id, action } = action { let library_manager = &context.library_manager; @@ -62,7 +62,7 @@ impl ActionHandler for LocationIndexHandler { .await .map_err(ActionError::Job)?; - Ok(ActionReceipt::job_based(Uuid::new_v4(), job_handle)) + Ok(ActionOutput::Success) } else { Err(ActionError::InvalidActionType) } diff --git a/core-new/src/operations/locations/remove/action.rs b/core-new/src/operations/locations/remove/action.rs index fd7b2d46d..7cda25faf 100644 --- a/core-new/src/operations/locations/remove/action.rs +++ b/core-new/src/operations/locations/remove/action.rs @@ -4,7 +4,7 @@ use crate::{ context::CoreContext, location::manager::LocationManager, infrastructure::actions::{ - Action, error::{ActionError, ActionResult}, handler::ActionHandler, receipt::ActionReceipt, + Action, error::{ActionError, ActionResult}, handler::ActionHandler, output::ActionOutput, }, register_action_handler, }; @@ -32,7 +32,7 @@ impl ActionHandler for LocationRemoveHandler { &self, context: Arc, action: Action, - ) -> ActionResult { + ) -> ActionResult { if let Action::LocationRemove { library_id, action } = action { let library_manager = &context.library_manager; @@ -49,13 +49,7 @@ impl ActionHandler for LocationRemoveHandler { .await .map_err(|e| ActionError::Internal(e.to_string()))?; - Ok(ActionReceipt::immediate( - Uuid::new_v4(), - Some(serde_json::json!({ - "location_id": action.location_id, - "removed": true - })), - )) + Ok(ActionOutput::location_remove(action.location_id)) } else { Err(ActionError::InvalidActionType) } diff --git a/core-new/src/operations/media/thumbnail/action.rs b/core-new/src/operations/media/thumbnail/action.rs index 3a5d2648c..aba6fd497 100644 --- a/core-new/src/operations/media/thumbnail/action.rs +++ b/core-new/src/operations/media/thumbnail/action.rs @@ -5,7 +5,7 @@ use crate::{ infrastructure::actions::{ error::{ActionError, ActionResult}, handler::ActionHandler, - receipt::ActionReceipt, + output::ActionOutput, }, register_action_handler, }; @@ -53,7 +53,7 @@ impl ActionHandler for ThumbnailHandler { &self, context: Arc, action: crate::infrastructure::actions::Action, - ) -> ActionResult { + ) -> ActionResult { if let crate::infrastructure::actions::Action::GenerateThumbnails { library_id, action } = action { let library_manager = &context.library_manager; @@ -79,7 +79,7 @@ impl ActionHandler for ThumbnailHandler { .await .map_err(ActionError::Job)?; - Ok(ActionReceipt::job_based(Uuid::new_v4(), job_handle)) + Ok(ActionOutput::Success) } else { Err(ActionError::InvalidActionType) } diff --git a/core-new/src/operations/metadata/action.rs b/core-new/src/operations/metadata/action.rs index c2d9466c7..400f9cf61 100644 --- a/core-new/src/operations/metadata/action.rs +++ b/core-new/src/operations/metadata/action.rs @@ -5,7 +5,7 @@ use crate::{ infrastructure::actions::{ error::{ActionError, ActionResult}, handler::ActionHandler, - receipt::ActionReceipt, + output::ActionOutput, }, register_action_handler, }; @@ -52,7 +52,7 @@ impl ActionHandler for MetadataHandler { &self, context: Arc, action: crate::infrastructure::actions::Action, - ) -> ActionResult { + ) -> ActionResult { if let crate::infrastructure::actions::Action::MetadataOperation { library_id, action } = action { let library_manager = &context.library_manager; @@ -71,7 +71,7 @@ impl ActionHandler for MetadataHandler { .await .map_err(ActionError::Job)?; - Ok(ActionReceipt::job_based(Uuid::new_v4(), job_handle)) + Ok(ActionOutput::Success) } else { Err(ActionError::InvalidActionType) }