From a5f0859c439c7aa4f87005fde1d4ac4ba34b1ff2 Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Sat, 1 Nov 2025 21:00:26 +0100 Subject: [PATCH] Refactoring PMOMetadata --- Cargo.lock | 1 + pmometadata/Cargo.toml | 3 + pmometadata/src/lib.rs | 588 +++++++++++++++++++++++++++++++++-------- 3 files changed, 480 insertions(+), 112 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f4fc3d99..a25dac29 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2785,6 +2785,7 @@ dependencies = [ "paste", "thiserror 2.0.17", "tokio", + "tokio-test", ] [[package]] diff --git a/pmometadata/Cargo.toml b/pmometadata/Cargo.toml index e9ce4b82..83a4d8df 100644 --- a/pmometadata/Cargo.toml +++ b/pmometadata/Cargo.toml @@ -8,3 +8,6 @@ async-trait = "0.1.89" paste = "1" thiserror = "2.0.17" tokio = { version = "1", features = ["full"] } + +[dev-dependencies] +tokio-test = "0.4" diff --git a/pmometadata/src/lib.rs b/pmometadata/src/lib.rs index 51938dbb..0a737cc8 100644 --- a/pmometadata/src/lib.rs +++ b/pmometadata/src/lib.rs @@ -1,4 +1,35 @@ //! Minimal metadata abstraction shared between PMO crates. +//! +//! This crate provides a lightweight, async-friendly trait [`TrackMetadata`] for managing +//! audio track metadata across different implementations. It supports both in-memory +//! storage and database-backed implementations. +//! +//! # Features +//! +//! - **Async API**: All operations are asynchronous to support database backends +//! - **Optional fields**: Implementations only need to override supported fields +//! - **Error handling**: Distinguishes between transient errors (NotImplemented, ReadOnly) +//! and backend errors that should be propagated +//! - **Metadata copying**: Helper function to copy metadata between implementations +//! +//! # Examples +//! +//! ```rust +//! use pmometadata::{TrackMetadata, MemoryTrackMetadata}; +//! use std::time::Duration; +//! +//! # tokio_test::block_on(async { +//! let mut metadata = MemoryTrackMetadata::new(); +//! +//! // Set some metadata +//! metadata.set_title(Some("My Song".to_string())).await.unwrap(); +//! metadata.set_artist(Some("Artist Name".to_string())).await.unwrap(); +//! metadata.set_duration(Some(Duration::from_secs(180))).await.unwrap(); +//! +//! // Retrieve metadata +//! assert_eq!(metadata.get_title().await.unwrap(), Some("My Song".to_string())); +//! # }); +//! ``` #![allow(async_fn_in_trait)] use std::{ @@ -9,22 +40,90 @@ use tokio::sync::RwLock; use std::sync::Arc; use async_trait::async_trait; +/// Helper macro for copying a single metadata field. +macro_rules! copy_a_metadata { + ($src:ident, $dest:ident, $key:ident) => { + ::paste::paste! { + match $src.[]().await { + Ok(Some(value)) => { + match $dest.write().await.[](Some(value)).await { + Ok(_) => {}, + Err(e) if e.is_transient() => { + // Transient error, try setting to None instead + match $dest.write().await.[](None).await { + Ok(_) => {}, + Err(e2) if e2.is_transient() => {}, + Err(e2) => return Err(e2), + } + } + Err(e) => return Err(e), + } + }, + Ok(None) => { + match $dest.write().await.[](None).await { + Ok(_) => {}, + Err(e) if e.is_transient() => {}, + Err(e) => return Err(e), + } + } + Err(e) if e.is_transient() => { + match $dest.write().await.[](None).await { + Ok(_) => {}, + Err(e2) if e2.is_transient() => {}, + Err(e2) => return Err(e2), + } + } + Err(e) => { + return Err(e); + } + } + } + }; +} + +/// Helper macro for copying multiple metadata fields. +macro_rules! copy_metadata { + ($src:ident, $dest:ident, $( $key:ident ),*) => { + $(copy_a_metadata!($src, $dest, $key);)* + }; +} + /// Convenience alias for metadata operations that can fail or return no value. +/// +/// Returns `Ok(Some(T))` when a value is present, `Ok(None)` when the field is empty, +/// or `Err(MetadataError)` when an error occurs. pub type MetadataResult = Result, MetadataError>; /// Errors that can occur when manipulating metadata. #[derive(Debug, thiserror::Error)] pub enum MetadataError { + /// The metadata field is not implemented by this provider. + /// + /// This is a transient error that indicates the implementation doesn't support + /// this particular field. When copying metadata, these errors are ignored. #[error("metadata field is not implemented")] NotImplemented, + + /// The metadata field is read-only and cannot be modified. + /// + /// This is a transient error. When copying metadata, these errors are ignored. #[error("metadata field is read-only")] ReadOnly, + + /// An error occurred in the backend (e.g., database error). + /// + /// These errors are non-transient and should be propagated to the caller. #[error("backend error: {0}")] Backend(String), } impl MetadataError { - fn is_transient(&self) -> bool { + /// Returns `true` if this error is transient and can be safely ignored. + /// + /// Transient errors (`NotImplemented`, `ReadOnly`) indicate that the operation + /// is not supported but is not a critical failure. When copying metadata, + /// transient errors result in setting the destination field to `None`. + pub fn is_transient(&self) -> bool { matches!( self, MetadataError::NotImplemented | MetadataError::ReadOnly @@ -34,7 +133,25 @@ impl MetadataError { /// Trait implemented by metadata providers. /// -/// Only the fields supported by the implementation have to be overridden. +/// This trait provides a uniform interface for accessing and modifying audio track metadata. +/// All methods have default implementations that return `NotImplemented`, so implementations +/// only need to override the fields they support. +/// +/// # Implementing the trait +/// +/// Implementations should: +/// - Override `get_*` methods for readable fields +/// - Override `set_*` methods for writable fields +/// - Call `touch()` in setters to update the modification timestamp +/// - Return `Ok(Some(value))` for successful operations +/// - Return `Ok(None)` when a field is empty +/// - Return `Err(NotImplemented)` for unsupported fields +/// - Return `Err(ReadOnly)` for read-only fields +/// - Return `Err(Backend(msg))` for backend errors +/// +/// # Thread safety +/// +/// All implementations must be `Send + Sync` to work in async contexts. #[async_trait] pub trait TrackMetadata: Send + Sync { async fn get_title(&self) -> MetadataResult { @@ -142,6 +259,54 @@ pub trait TrackMetadata: Send + Sync { } } +/// Copies all available metadata from one implementation to another. +/// +/// This function reads all metadata fields from the source and attempts to write them +/// to the destination. It handles errors intelligently: +/// +/// - **Transient errors** (`NotImplemented`, `ReadOnly`): The field is set to `None` in the destination +/// - **Backend errors**: The error is propagated to the caller +/// - **Success**: The value is copied from source to destination +/// +/// After all fields are copied, the destination's `touch()` method is called to update +/// its modification timestamp. +/// +/// # Arguments +/// +/// * `src` - Source metadata wrapped in `Arc>` +/// * `dest` - Destination metadata wrapped in `Arc>` +/// +/// # Errors +/// +/// Returns an error if: +/// - The destination returns a backend error when setting a field +/// - The destination's `touch()` method returns an error +/// +/// # Examples +/// +/// ```rust +/// use pmometadata::{TrackMetadata, MemoryTrackMetadata, copy_metadata_into}; +/// use std::sync::Arc; +/// use tokio::sync::RwLock; +/// +/// # tokio_test::block_on(async { +/// let mut src = MemoryTrackMetadata::new(); +/// src.set_title(Some("My Song".to_string())).await.unwrap(); +/// src.set_artist(Some("Artist".to_string())).await.unwrap(); +/// +/// let dest = MemoryTrackMetadata::new(); +/// +/// let src_lock = Arc::new(RwLock::new(src)); +/// let dest_lock = Arc::new(RwLock::new(dest)); +/// +/// copy_metadata_into(&src_lock, &dest_lock).await.unwrap(); +/// +/// assert_eq!( +/// dest_lock.read().await.get_title().await.unwrap(), +/// Some("My Song".to_string()) +/// ); +/// # }); +/// ``` pub async fn copy_metadata_into( src: &Arc>, dest: &Arc>, @@ -151,118 +316,18 @@ where D: TrackMetadata + ?Sized, { let src_guard = src.read().await; - - // Expansion de copy_metadata! pour toutes les propriétés - match src_guard.get_title().await { - Ok(Some(value)) => { - dest.write().await.set_title(Some(value)).await?; - }, - Ok(None) | Err(_) => { - dest.write().await.set_title(None).await.ok(); - } - } - match src_guard.get_artist().await { - Ok(Some(value)) => { - dest.write().await.set_artist(Some(value)).await?; - }, - Ok(None) | Err(_) => { - dest.write().await.set_artist(None).await.ok(); - } - } + copy_metadata!( + src_guard, dest, title, artist, album, year, duration, track_id, + channel_id, event, rating, cover_url, cover_pk, extra + ); - match src_guard.get_album().await { - Ok(Some(value)) => { - dest.write().await.set_album(Some(value)).await?; - }, - Ok(None) | Err(_) => { - dest.write().await.set_album(None).await.ok(); - } + // Try to update the timestamp, but ignore transient errors + match dest.write().await.touch().await { + Ok(_) => Ok(()), + Err(e) if e.is_transient() => Ok(()), + Err(e) => Err(e), } - - match src_guard.get_year().await { - Ok(Some(value)) => { - dest.write().await.set_year(Some(value)).await?; - }, - Ok(None) | Err(_) => { - dest.write().await.set_year(None).await.ok(); - } - } - - match src_guard.get_duration().await { - Ok(Some(value)) => { - dest.write().await.set_duration(Some(value)).await?; - }, - Ok(None) | Err(_) => { - dest.write().await.set_duration(None).await.ok(); - } - } - - match src_guard.get_track_id().await { - Ok(Some(value)) => { - dest.write().await.set_track_id(Some(value)).await?; - }, - Ok(None) | Err(_) => { - dest.write().await.set_track_id(None).await.ok(); - } - } - - match src_guard.get_channel_id().await { - Ok(Some(value)) => { - dest.write().await.set_channel_id(Some(value)).await?; - }, - Ok(None) | Err(_) => { - dest.write().await.set_channel_id(None).await.ok(); - } - } - - match src_guard.get_event().await { - Ok(Some(value)) => { - dest.write().await.set_event(Some(value)).await?; - }, - Ok(None) | Err(_) => { - dest.write().await.set_event(None).await.ok(); - } - } - - match src_guard.get_rating().await { - Ok(Some(value)) => { - dest.write().await.set_rating(Some(value)).await?; - }, - Ok(None) | Err(_) => { - dest.write().await.set_rating(None).await.ok(); - } - } - - match src_guard.get_cover_url().await { - Ok(Some(value)) => { - dest.write().await.set_cover_url(Some(value)).await?; - }, - Ok(None) | Err(_) => { - dest.write().await.set_cover_url(None).await.ok(); - } - } - - match src_guard.get_cover_pk().await { - Ok(Some(value)) => { - dest.write().await.set_cover_pk(Some(value)).await?; - }, - Ok(None) | Err(_) => { - dest.write().await.set_cover_pk(None).await.ok(); - } - } - - match src_guard.get_extra().await { - Ok(Some(value)) => { - dest.write().await.set_extra(Some(value)).await?; - }, - Ok(None) | Err(_) => { - dest.write().await.set_extra(None).await.ok(); - } - } - - dest.write().await.touch().await?; - Ok(()) } /// In-memory metadata implementation with full read/write support. @@ -273,7 +338,6 @@ pub struct MemoryTrackMetadata { album: Option, year: Option, duration: Option, - elapsed: Option, track_id: Option, channel_id: Option, event: Option, @@ -421,3 +485,303 @@ impl TrackMetadata for MemoryTrackMetadata { Ok(Some(())) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn test_memory_metadata_new() { + let metadata = MemoryTrackMetadata::new(); + + assert_eq!(metadata.get_title().await.unwrap(), None); + assert_eq!(metadata.get_artist().await.unwrap(), None); + assert_eq!(metadata.get_album().await.unwrap(), None); + assert_eq!(metadata.get_year().await.unwrap(), None); + assert_eq!(metadata.get_duration().await.unwrap(), None); + assert_eq!(metadata.get_updated_at().await.unwrap(), None); + } + + #[tokio::test] + async fn test_memory_metadata_set_get_title() { + let mut metadata = MemoryTrackMetadata::new(); + + let result = metadata.set_title(Some("Test Title".to_string())).await; + assert!(result.is_ok()); + assert_eq!(result.unwrap(), Some(())); + + let title = metadata.get_title().await.unwrap(); + assert_eq!(title, Some("Test Title".to_string())); + } + + #[tokio::test] + async fn test_memory_metadata_set_get_artist() { + let mut metadata = MemoryTrackMetadata::new(); + + metadata.set_artist(Some("Artist Name".to_string())).await.unwrap(); + assert_eq!( + metadata.get_artist().await.unwrap(), + Some("Artist Name".to_string()) + ); + } + + #[tokio::test] + async fn test_memory_metadata_set_get_album() { + let mut metadata = MemoryTrackMetadata::new(); + + metadata.set_album(Some("Album Name".to_string())).await.unwrap(); + assert_eq!( + metadata.get_album().await.unwrap(), + Some("Album Name".to_string()) + ); + } + + #[tokio::test] + async fn test_memory_metadata_set_get_year() { + let mut metadata = MemoryTrackMetadata::new(); + + metadata.set_year(Some(2024)).await.unwrap(); + assert_eq!(metadata.get_year().await.unwrap(), Some(2024)); + } + + #[tokio::test] + async fn test_memory_metadata_set_get_duration() { + let mut metadata = MemoryTrackMetadata::new(); + let duration = Duration::from_secs(180); + + metadata.set_duration(Some(duration)).await.unwrap(); + assert_eq!(metadata.get_duration().await.unwrap(), Some(duration)); + } + + #[tokio::test] + async fn test_memory_metadata_set_get_rating() { + let mut metadata = MemoryTrackMetadata::new(); + + metadata.set_rating(Some(4.5)).await.unwrap(); + assert_eq!(metadata.get_rating().await.unwrap(), Some(4.5)); + } + + #[tokio::test] + async fn test_memory_metadata_set_get_track_id() { + let mut metadata = MemoryTrackMetadata::new(); + + metadata.set_track_id(Some("track123".to_string())).await.unwrap(); + assert_eq!( + metadata.get_track_id().await.unwrap(), + Some("track123".to_string()) + ); + } + + #[tokio::test] + async fn test_memory_metadata_set_get_channel_id() { + let mut metadata = MemoryTrackMetadata::new(); + + metadata.set_channel_id(Some("channel456".to_string())).await.unwrap(); + assert_eq!( + metadata.get_channel_id().await.unwrap(), + Some("channel456".to_string()) + ); + } + + #[tokio::test] + async fn test_memory_metadata_set_get_event() { + let mut metadata = MemoryTrackMetadata::new(); + + metadata.set_event(Some("event789".to_string())).await.unwrap(); + assert_eq!( + metadata.get_event().await.unwrap(), + Some("event789".to_string()) + ); + } + + #[tokio::test] + async fn test_memory_metadata_set_get_cover_url() { + let mut metadata = MemoryTrackMetadata::new(); + + metadata.set_cover_url(Some("https://example.com/cover.jpg".to_string())).await.unwrap(); + assert_eq!( + metadata.get_cover_url().await.unwrap(), + Some("https://example.com/cover.jpg".to_string()) + ); + } + + #[tokio::test] + async fn test_memory_metadata_set_get_cover_pk() { + let mut metadata = MemoryTrackMetadata::new(); + + metadata.set_cover_pk(Some("pk123".to_string())).await.unwrap(); + assert_eq!( + metadata.get_cover_pk().await.unwrap(), + Some("pk123".to_string()) + ); + } + + #[tokio::test] + async fn test_memory_metadata_set_get_extra() { + let mut metadata = MemoryTrackMetadata::new(); + let mut extra = HashMap::new(); + extra.insert("key1".to_string(), "value1".to_string()); + extra.insert("key2".to_string(), "value2".to_string()); + + metadata.set_extra(Some(extra.clone())).await.unwrap(); + assert_eq!(metadata.get_extra().await.unwrap(), Some(extra)); + } + + #[tokio::test] + async fn test_memory_metadata_set_none() { + let mut metadata = MemoryTrackMetadata::new(); + + metadata.set_title(Some("Title".to_string())).await.unwrap(); + assert_eq!(metadata.get_title().await.unwrap(), Some("Title".to_string())); + + metadata.set_title(None).await.unwrap(); + assert_eq!(metadata.get_title().await.unwrap(), None); + } + + #[tokio::test] + async fn test_memory_metadata_touch() { + let mut metadata = MemoryTrackMetadata::new(); + + assert_eq!(metadata.get_updated_at().await.unwrap(), None); + + metadata.touch().await.unwrap(); + let updated_at = metadata.get_updated_at().await.unwrap(); + assert!(updated_at.is_some()); + } + + #[tokio::test] + async fn test_memory_metadata_touch_on_set() { + let mut metadata = MemoryTrackMetadata::new(); + + assert_eq!(metadata.get_updated_at().await.unwrap(), None); + + metadata.set_title(Some("Title".to_string())).await.unwrap(); + let updated_at = metadata.get_updated_at().await.unwrap(); + assert!(updated_at.is_some()); + } + + #[tokio::test] + async fn test_metadata_error_is_transient() { + assert!(MetadataError::NotImplemented.is_transient()); + assert!(MetadataError::ReadOnly.is_transient()); + assert!(!MetadataError::Backend("error".to_string()).is_transient()); + } + + #[tokio::test] + async fn test_copy_metadata_into_basic() { + let mut src = MemoryTrackMetadata::new(); + src.set_title(Some("Source Title".to_string())).await.unwrap(); + src.set_artist(Some("Source Artist".to_string())).await.unwrap(); + src.set_year(Some(2024)).await.unwrap(); + + let dest = MemoryTrackMetadata::new(); + + let src_lock = Arc::new(RwLock::new(src)); + let dest_lock = Arc::new(RwLock::new(dest)); + + copy_metadata_into(&src_lock, &dest_lock).await.unwrap(); + + let dest_guard = dest_lock.read().await; + assert_eq!(dest_guard.get_title().await.unwrap(), Some("Source Title".to_string())); + assert_eq!(dest_guard.get_artist().await.unwrap(), Some("Source Artist".to_string())); + assert_eq!(dest_guard.get_year().await.unwrap(), Some(2024)); + } + + #[tokio::test] + async fn test_copy_metadata_into_partial() { + let mut src = MemoryTrackMetadata::new(); + src.set_title(Some("Title".to_string())).await.unwrap(); + // artist is None + + let dest = MemoryTrackMetadata::new(); + + let src_lock = Arc::new(RwLock::new(src)); + let dest_lock = Arc::new(RwLock::new(dest)); + + copy_metadata_into(&src_lock, &dest_lock).await.unwrap(); + + let dest_guard = dest_lock.read().await; + assert_eq!(dest_guard.get_title().await.unwrap(), Some("Title".to_string())); + assert_eq!(dest_guard.get_artist().await.unwrap(), None); + } + + #[tokio::test] + async fn test_copy_metadata_into_updates_timestamp() { + let src = MemoryTrackMetadata::new(); + let dest = MemoryTrackMetadata::new(); + + let src_lock = Arc::new(RwLock::new(src)); + let dest_lock = Arc::new(RwLock::new(dest)); + + let before = dest_lock.read().await.get_updated_at().await.unwrap(); + assert_eq!(before, None); + + copy_metadata_into(&src_lock, &dest_lock).await.unwrap(); + + let after = dest_lock.read().await.get_updated_at().await.unwrap(); + assert!(after.is_some()); + } + + #[tokio::test] + async fn test_copy_metadata_into_all_fields() { + let mut src = MemoryTrackMetadata::new(); + src.set_title(Some("Title".to_string())).await.unwrap(); + src.set_artist(Some("Artist".to_string())).await.unwrap(); + src.set_album(Some("Album".to_string())).await.unwrap(); + src.set_year(Some(2024)).await.unwrap(); + src.set_duration(Some(Duration::from_secs(180))).await.unwrap(); + src.set_track_id(Some("track123".to_string())).await.unwrap(); + src.set_channel_id(Some("channel456".to_string())).await.unwrap(); + src.set_event(Some("event789".to_string())).await.unwrap(); + src.set_rating(Some(4.5)).await.unwrap(); + src.set_cover_url(Some("https://example.com/cover.jpg".to_string())).await.unwrap(); + src.set_cover_pk(Some("pk123".to_string())).await.unwrap(); + + let mut extra = HashMap::new(); + extra.insert("key".to_string(), "value".to_string()); + src.set_extra(Some(extra.clone())).await.unwrap(); + + let dest = MemoryTrackMetadata::new(); + + let src_lock = Arc::new(RwLock::new(src)); + let dest_lock = Arc::new(RwLock::new(dest)); + + copy_metadata_into(&src_lock, &dest_lock).await.unwrap(); + + let dest_guard = dest_lock.read().await; + assert_eq!(dest_guard.get_title().await.unwrap(), Some("Title".to_string())); + assert_eq!(dest_guard.get_artist().await.unwrap(), Some("Artist".to_string())); + assert_eq!(dest_guard.get_album().await.unwrap(), Some("Album".to_string())); + assert_eq!(dest_guard.get_year().await.unwrap(), Some(2024)); + assert_eq!(dest_guard.get_duration().await.unwrap(), Some(Duration::from_secs(180))); + assert_eq!(dest_guard.get_track_id().await.unwrap(), Some("track123".to_string())); + assert_eq!(dest_guard.get_channel_id().await.unwrap(), Some("channel456".to_string())); + assert_eq!(dest_guard.get_event().await.unwrap(), Some("event789".to_string())); + assert_eq!(dest_guard.get_rating().await.unwrap(), Some(4.5)); + assert_eq!(dest_guard.get_cover_url().await.unwrap(), Some("https://example.com/cover.jpg".to_string())); + assert_eq!(dest_guard.get_cover_pk().await.unwrap(), Some("pk123".to_string())); + assert_eq!(dest_guard.get_extra().await.unwrap(), Some(extra)); + } + + #[tokio::test] + async fn test_copy_metadata_handles_errors() { + // Test that copy works even when some fields can't be set + let mut src = MemoryTrackMetadata::new(); + src.set_title(Some("Title".to_string())).await.unwrap(); + src.set_artist(Some("Artist".to_string())).await.unwrap(); + + let dest = MemoryTrackMetadata::new(); + + let src_lock = Arc::new(RwLock::new(src)); + let dest_lock = Arc::new(RwLock::new(dest)); + + // Should succeed + let result = copy_metadata_into(&src_lock, &dest_lock).await; + assert!(result.is_ok()); + + // Verify the data was copied + let dest_guard = dest_lock.read().await; + assert_eq!(dest_guard.get_title().await.unwrap(), Some("Title".to_string())); + assert_eq!(dest_guard.get_artist().await.unwrap(), Some("Artist".to_string())); + } +}