From 9dc3157b1a5bdcbfaaa2c7b584748ca161995839 Mon Sep 17 00:00:00 2001 From: jdluu Date: Sat, 29 Aug 2026 00:10:30 -0700 Subject: [PATCH] refactor: isolate OPDS download setup Move download URL validation and HTTP context construction behind a focused service while preserving the Tauri command contract and pipeline behavior.\n\nCloses #101 --- src-tauri/src/commands/opds/download.rs | 36 +---- .../src/commands/opds/download_service.rs | 138 ++++++++++++++++++ src-tauri/src/commands/opds/mod.rs | 1 + 3 files changed, 147 insertions(+), 28 deletions(-) create mode 100644 src-tauri/src/commands/opds/download_service.rs diff --git a/src-tauri/src/commands/opds/download.rs b/src-tauri/src/commands/opds/download.rs index 7fed876..10fbf99 100644 --- a/src-tauri/src/commands/opds/download.rs +++ b/src-tauri/src/commands/opds/download.rs @@ -1,15 +1,14 @@ +use super::download_service::prepare_download_setup; use super::transport::sanitize_error_message; use crate::error::AppError; use crate::opds::{ - download_file, plan_download_destination, CatalogConfig, DownloadContext, DownloadError, - ProgressCallback, Publication, + download_file, plan_download_destination, DownloadError, ProgressCallback, Publication, }; use serde::Serialize; use std::collections::HashMap; use std::sync::{LazyLock, Mutex}; use tauri::{command, AppHandle, Emitter}; use tokio_util::sync::CancellationToken; -use url::Url; #[derive(Debug, Clone, Serialize)] pub struct OpdsDownloadProgress { @@ -97,31 +96,10 @@ pub async fn download_opds_publication( content_root: String, app: AppHandle, ) -> Result { - let parsed_url = Url::parse(&catalog_url) - .map_err(|_| AppError::OpdsTransport("Invalid URL: unable to parse".to_string()))?; - - if parsed_url.scheme() != "http" && parsed_url.scheme() != "https" { - return Err(AppError::OpdsTransport( - "Invalid URL: only HTTP and HTTPS schemes are allowed".to_string(), - )); - } - - if !parsed_url.username().is_empty() { - return Err(AppError::OpdsTransport( - "Invalid URL: credentials must not be embedded in URL".to_string(), - )); - } - - let config = CatalogConfig::new("download", parsed_url.clone(), username, password)?; - - let client = reqwest::Client::builder() - .timeout(std::time::Duration::from_secs(30)) - .redirect(reqwest::redirect::Policy::none()) - .use_rustls_tls() - .build() - .map_err(|e| AppError::OpdsTransport(format!("Failed to build HTTP client: {}", e)))?; - - let context = DownloadContext::new(client, config.clone()); + let setup = prepare_download_setup(&catalog_url, &username, &password)?; + let parsed_url = setup.url; + let config = setup.config; + let context = setup.context; let content_root_path = std::path::Path::new(&content_root); let plan = plan_download_destination( @@ -201,8 +179,10 @@ pub async fn download_opds_publication( #[cfg(test)] mod tests { use super::*; + use crate::opds::{CatalogConfig, DownloadContext}; use axum::{routing::get, Router}; use axum_test::TestServer; + use url::Url; #[tokio::test] async fn cancellable_download_stops_when_token_fires() { diff --git a/src-tauri/src/commands/opds/download_service.rs b/src-tauri/src/commands/opds/download_service.rs new file mode 100644 index 0000000..c72c635 --- /dev/null +++ b/src-tauri/src/commands/opds/download_service.rs @@ -0,0 +1,138 @@ +//! Application-layer service for the OPDS download command. +//! +//! Owns download input validation and HTTP client/context construction so the +//! Tauri command stays a thin adapter over the download pipeline. The error +//! messages produced here are intentionally stable: the frontend IPC contract +//! matches on the serialized `AppError::OpdsTransport` strings, and the +//! command-derived messages must not change. + +use crate::error::AppError; +use crate::opds::{CatalogConfig, DownloadContext}; +use url::Url; + +/// A validated, ready-to-use setup for a single publication download. +pub struct DownloadSetup { + pub url: Url, + pub config: CatalogConfig, + pub context: DownloadContext, +} + +/// Prepares a download setup from raw command inputs. +/// +/// Validates the catalog URL, derives a `CatalogConfig`, and constructs the +/// HTTP client + `DownloadContext`. Failure produces the exact +/// `AppError::OpdsTransport` messages the previous inline command produced. +pub fn prepare_download_setup( + catalog_url: &str, + username: &str, + password: &str, +) -> Result { + let parsed_url = Url::parse(catalog_url) + .map_err(|_| AppError::OpdsTransport("Invalid URL: unable to parse".to_string()))?; + + if parsed_url.scheme() != "http" && parsed_url.scheme() != "https" { + return Err(AppError::OpdsTransport( + "Invalid URL: only HTTP and HTTPS schemes are allowed".to_string(), + )); + } + + if !parsed_url.username().is_empty() { + return Err(AppError::OpdsTransport( + "Invalid URL: credentials must not be embedded in URL".to_string(), + )); + } + + let config = CatalogConfig::new( + "download", + parsed_url.clone(), + username, + password.to_string(), + ) + .map_err(|e| AppError::OpdsTransport(e.to_string()))?; + + let client = reqwest::Client::builder() + .timeout(std::time::Duration::from_secs(30)) + .redirect(reqwest::redirect::Policy::none()) + .use_rustls_tls() + .build() + .map_err(|e| AppError::OpdsTransport(format!("Failed to build HTTP client: {}", e)))?; + + let context = DownloadContext::new(client, config.clone()); + + Ok(DownloadSetup { + url: parsed_url, + config, + context, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn opds_transport_message(err: &AppError) -> &str { + match err { + AppError::OpdsTransport(msg) => msg, + other => panic!("expected OpdsTransport, got {other:?}"), + } + } + + fn setup_error(catalog_url: &str) -> AppError { + match prepare_download_setup(catalog_url, "user", "pass") { + Err(e) => e, + Ok(_) => panic!("expected setup to fail for {catalog_url}"), + } + } + + #[test] + fn valid_https_url_builds_setup() { + let setup = prepare_download_setup("https://example.com/opds", "user", "pass").unwrap(); + assert_eq!(setup.url.as_str(), "https://example.com/opds"); + assert_eq!(setup.config.origin(), "https://example.com"); + } + + #[test] + fn valid_http_url_builds_setup() { + let setup = prepare_download_setup("http://example.com:8080/opds", "user", "pass").unwrap(); + assert_eq!(setup.config.origin(), "http://example.com:8080"); + } + + #[test] + fn setup_config_carries_provider_and_username() { + let setup = prepare_download_setup("https://example.com/opds", "alice", "s3cret").unwrap(); + assert_eq!(setup.config.provider, "download"); + assert_eq!(setup.config.username, "alice"); + assert_eq!(setup.config.password, "s3cret"); + } + + #[test] + fn unparseable_url_rejected() { + let err = setup_error("not a url"); + assert_eq!(opds_transport_message(&err), "Invalid URL: unable to parse"); + } + + #[test] + fn non_http_scheme_rejected() { + let err = setup_error("ftp://example.com/opds"); + assert_eq!( + opds_transport_message(&err), + "Invalid URL: only HTTP and HTTPS schemes are allowed" + ); + } + + #[test] + fn credentials_embedded_in_url_rejected() { + let err = setup_error("https://user:pass@example.com/opds"); + assert_eq!( + opds_transport_message(&err), + "Invalid URL: credentials must not be embedded in URL" + ); + } + + #[test] + fn setup_context_is_built_with_a_client() { + let setup = prepare_download_setup("https://example.com/opds", "user", "pass").unwrap(); + // The context must carry the same provider and the configured max size. + assert_eq!(setup.context.config.provider, "download"); + } +} diff --git a/src-tauri/src/commands/opds/mod.rs b/src-tauri/src/commands/opds/mod.rs index 8ed63e1..f30bcfa 100644 --- a/src-tauri/src/commands/opds/mod.rs +++ b/src-tauri/src/commands/opds/mod.rs @@ -1,5 +1,6 @@ pub mod catalog; pub mod download; +pub mod download_service; pub mod transport; pub use catalog::*;