From 1164a114102a2ce3d25e55f2b9bb12066177e7e6 Mon Sep 17 00:00:00 2001 From: Eric Coissac Date: Tue, 24 Mar 2026 12:02:53 +0100 Subject: [PATCH] feat(cache): centralize absolute URL generation with PMO_SERVER_URL Replace hardcoded relative URLs and manual base_url concatenation with a unified absolute URL API via pmocache::covers_absolute_url_for() and CacheTrait::absolute_url_for(). - Add pmocache as a required dependency to pmoparadise - Introduce absolute_url_for() and covers_absolute_url_for() helpers using PMO_SERVER_URL env var (default: http://localhost:8080) - Update all callers to use absolute URLs for covers and audio in streaming, playlists, Qobuz, Radio France, UPnP, and server startup - Remove redundant route_for() usage in URL construction - Add pmocache to Cargo.lock --- Cargo.lock | 1 + .../src/sinks/streaming_icyflac_sink.rs | 6 ++--- pmocache/src/cache_trait.rs | 19 ++++++++------- pmocache/src/lib.rs | 16 +++++++++++++ pmoparadise/Cargo.toml | 1 + pmoparadise/src/source.rs | 2 +- pmoplaylist/src/api.rs | 2 +- pmoplaylist/src/handle/read.rs | 4 ++-- pmoqobuz/src/api_rest.rs | 2 +- pmoradiofrance/src/metadata_cache.rs | 6 ++--- pmoserver/src/server.rs | 9 ++++++- pmoupnp/src/cache_registry.rs | 24 ++++--------------- 12 files changed, 51 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e75089a5..130a8bbb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4147,6 +4147,7 @@ dependencies = [ "pmoaudio", "pmoaudio-ext", "pmoaudiocache", + "pmocache", "pmoconfig", "pmocovers", "pmoflac", diff --git a/pmoaudio-ext/src/sinks/streaming_icyflac_sink.rs b/pmoaudio-ext/src/sinks/streaming_icyflac_sink.rs index 57e9717d..85b7a5f5 100644 --- a/pmoaudio-ext/src/sinks/streaming_icyflac_sink.rs +++ b/pmoaudio-ext/src/sinks/streaming_icyflac_sink.rs @@ -88,10 +88,8 @@ impl IcyClientStream { // Add cover URL if we have a cover_pk if let Some(pk) = &meta.cover_pk { - // Use relative URL /covers/image/{pk}/256 - // This works when streaming from the same server that serves covers - // VLC and other players will resolve relative URLs correctly - metadata_str.push_str(&format!("StreamUrl='/covers/image/{}/256';", pk)); + let cover_url = pmocache::covers_absolute_url_for(pk, None); + metadata_str.push_str(&format!("StreamUrl='{}';", cover_url)); } else if let Some(url) = &meta.cover_url { // Fallback to external cover URL if no local pk metadata_str.push_str(&format!("StreamUrl='{}';", url)); diff --git a/pmocache/src/cache_trait.rs b/pmocache/src/cache_trait.rs index 97e422c4..344a94b0 100644 --- a/pmocache/src/cache_trait.rs +++ b/pmocache/src/cache_trait.rs @@ -69,14 +69,7 @@ pub trait FileCache: Send + Sync { /// Retourne la route relative pour accéder à un item du cache /// - /// # Arguments - /// - /// * `pk` - Clé primaire de la piste - /// * `param` - Paramètre optionnel (ex: "orig", "128k", etc.) - /// - /// # Returns - /// - /// Route relative (ex: "/audio/flac/abc123" ou "/audio/tracks/abc123/orig") + /// Format: `/{cache_name}/{cache_type}/{pk}[/{param}]` fn route_for(&self, pk: &str, param: Option<&str>) -> String { if let Some(p) = param { format!("/{}/{}/{}/{}", C::cache_name(), C::cache_type(), pk, p) @@ -85,6 +78,16 @@ pub trait FileCache: Send + Sync { } } + /// Retourne l'URL absolue pour accéder à un item du cache + /// + /// Utilise la variable d'environnement `PMO_SERVER_URL` comme base, + /// avec `http://localhost:8080` comme valeur par défaut. + fn absolute_url_for(&self, pk: &str, param: Option<&str>) -> String { + let base = std::env::var("PMO_SERVER_URL") + .unwrap_or_else(|_| "http://localhost:8080".to_string()); + format!("{}{}", base.trim_end_matches('/'), self.route_for(pk, param)) + } + /// Télécharge un fichier depuis une URL et l'ajoute au cache /// /// # Arguments diff --git a/pmocache/src/lib.rs b/pmocache/src/lib.rs index f6a97974..b1833226 100644 --- a/pmocache/src/lib.rs +++ b/pmocache/src/lib.rs @@ -144,6 +144,22 @@ pub use cache::{ CacheSubscription, }; pub use cache_trait::{pk_from_content_header, FileCache}; + +/// Retourne la route relative pour une cover: `/covers/image/{pk}[/{param}]` +pub fn covers_route_for(pk: &str, param: Option<&str>) -> String { + if let Some(p) = param { + format!("/covers/image/{}/{}", pk, p) + } else { + format!("/covers/image/{}", pk) + } +} + +/// Retourne l'URL absolue pour une cover via `PMO_SERVER_URL` +pub fn covers_absolute_url_for(pk: &str, param: Option<&str>) -> String { + let base = std::env::var("PMO_SERVER_URL") + .unwrap_or_else(|_| "http://localhost:8080".to_string()); + format!("{}{}", base.trim_end_matches('/'), covers_route_for(pk, param)) +} pub use db::{CacheEntry, DB}; pub use download::{ download, download_with_transformer, ingest_with_transformer, peek_header, peek_reader_header, diff --git a/pmoparadise/Cargo.toml b/pmoparadise/Cargo.toml index 79c970cf..e5bad271 100644 --- a/pmoparadise/Cargo.toml +++ b/pmoparadise/Cargo.toml @@ -63,6 +63,7 @@ pmoplaylist = { path = "../pmoplaylist" } pmoconfig = { path = "../pmoconfig", optional = true } # Cache support (OBLIGATOIRE - architecture refactorisée) +pmocache = { path = "../pmocache" } pmocovers = { path = "../pmocovers" } pmoaudiocache = { path = "../pmoaudiocache" } diff --git a/pmoparadise/src/source.rs b/pmoparadise/src/source.rs index eef35d71..dbd4f3b3 100644 --- a/pmoparadise/src/source.rs +++ b/pmoparadise/src/source.rs @@ -213,7 +213,7 @@ impl RadioParadiseSource { let cover_pk = json["cover_pk"].as_str().map(|s| s.to_string()); let cover_url = cover_pk .as_ref() - .map(|pk| format!("{}/covers/jpeg/{}", self.base_url, pk)) + .map(|pk| pmocache::covers_absolute_url_for(pk, None)) .or_else(|| json["cover_url"].as_str().map(|s| s.to_string())) .or_else(|| Some(self.default_cover_url())); diff --git a/pmoplaylist/src/api.rs b/pmoplaylist/src/api.rs index 79953155..c8f6289c 100644 --- a/pmoplaylist/src/api.rs +++ b/pmoplaylist/src/api.rs @@ -517,7 +517,7 @@ fn playlist_track_to_response( } fn cover_url_from_pk(pk: &str) -> String { - format!("/covers/image/{}/256", pk) + pmocache::covers_absolute_url_for(pk, None) } fn normalize_cover_pk(input: Option) -> Option { diff --git a/pmoplaylist/src/handle/read.rs b/pmoplaylist/src/handle/read.rs index 085f2d39..e3d4e672 100644 --- a/pmoplaylist/src/handle/read.rs +++ b/pmoplaylist/src/handle/read.rs @@ -182,7 +182,7 @@ impl ReadHandle { let _remaining = self.remaining().await?; // Convertir cover_pk en URL si présent - let album_art = cover_pk.map(|pk| format!("/cover/{}", pk)); + let album_art = cover_pk.map(|pk| pmocache::covers_absolute_url_for(&pk, None)); Ok(Container { id: self.playlist.id.clone(), @@ -253,7 +253,7 @@ impl ReadHandle { let track_number = meta.get_track_number().await.ok().flatten(); let cover_pk = meta.get_cover_pk().await.ok().flatten(); let cover_url = if let Some(pk) = cover_pk.as_ref() { - Some(format!("/covers/jpeg/{}/256", pk)) + Some(pmocache::covers_absolute_url_for(pk, None)) } else { meta.get_cover_url().await.ok().flatten() }; diff --git a/pmoqobuz/src/api_rest.rs b/pmoqobuz/src/api_rest.rs index 70a05a18..d3f568cd 100644 --- a/pmoqobuz/src/api_rest.rs +++ b/pmoqobuz/src/api_rest.rs @@ -299,7 +299,7 @@ async fn cache_album_image(mut album: Album, cover_cache: &Arc if let Some(ref image_url) = album.image { match cover_cache.add_from_url(image_url, None).await { Ok(pk) => { - album.image_cached = Some(format!("/covers/images/{}", pk)); + album.image_cached = Some(pmocache::covers_absolute_url_for(&pk, None)); } Err(e) => { tracing::warn!("Failed to cache album image: {}", e); diff --git a/pmoradiofrance/src/metadata_cache.rs b/pmoradiofrance/src/metadata_cache.rs index f8151f4f..0da2f191 100644 --- a/pmoradiofrance/src/metadata_cache.rs +++ b/pmoradiofrance/src/metadata_cache.rs @@ -251,15 +251,13 @@ impl CachedMetadata { // Note: add_from_url() lance le téléchargement complet en arrière-plan. // L'URL est valide immédiatement — si le fichier n'est pas encore prêt, // le client web doit réessayer (retry avec backoff). - let route = cache.route_for(&pk, None); - let public_url = format!("{}{}", server_base_url.trim_end_matches('/'), route); + let public_url = pmocache::covers_absolute_url_for(&pk, None); #[cfg(feature = "logging")] tracing::debug!( - "Cached cover - UUID: {}, PK: {}, route: {}, public_url: {}", + "Cached cover - UUID: {}, PK: {}, public_url: {}", uuid, pk, - route, public_url ); diff --git a/pmoserver/src/server.rs b/pmoserver/src/server.rs index a2e6c734..bd9e9852 100644 --- a/pmoserver/src/server.rs +++ b/pmoserver/src/server.rs @@ -114,6 +114,13 @@ impl Server { pub fn new(name: impl Into, base_url: impl Into, http_port: u16) -> Self { let api_registry = Arc::new(RwLock::new(Vec::new())); + let base_url = base_url.into(); + + // Initialiser PMO_SERVER_URL pour que tous les caches puissent construire des URLs absolues + // sans avoir besoin de propager base_url manuellement. + // SAFETY: appelé une seule fois au démarrage du serveur, avant tout thread concurrent. + unsafe { std::env::set_var("PMO_SERVER_URL", &base_url) }; + // Créer le router initial avec l'endpoint de registre let registry_route = Router::new() .route("/api/registry", get(get_api_registry)) @@ -121,7 +128,7 @@ impl Server { Self { name: name.into(), - base_url: base_url.into(), + base_url, http_port, router: Arc::new(RwLock::new(registry_route)), api_router: Arc::new(RwLock::new(None)), diff --git a/pmoupnp/src/cache_registry.rs b/pmoupnp/src/cache_registry.rs index 913381ca..572fa5f7 100644 --- a/pmoupnp/src/cache_registry.rs +++ b/pmoupnp/src/cache_registry.rs @@ -54,18 +54,10 @@ pub fn get_audio_cache() -> Option> { /// // url = "http://localhost:8080/covers/images/abc123/300" /// ``` pub fn build_cover_url(pk: &str, size: Option) -> anyhow::Result { - // Récupérer l'URL de base depuis la variable d'environnement ou une config - let base_url = - std::env::var("PMO_SERVER_URL").unwrap_or_else(|_| "http://localhost:8080".to_string()); - - let cache = get_cover_cache().ok_or_else(|| anyhow::anyhow!("No registered cover cache"))?; - - let param = match size { - Some(size_) => Some(size_.to_string()), - None => None, - }; - let route = cache.route_for(pk, param.as_deref()); - Ok(format!("{}{}", base_url, route)) + Ok(pmocache::covers_absolute_url_for( + pk, + size.map(|s| s.to_string()).as_deref(), + )) } /// Construit l'URL complète pour une piste audio @@ -84,12 +76,6 @@ pub fn build_cover_url(pk: &str, size: Option) -> anyhow::Result /// // url = "http://localhost:8080/audio/tracks/abc123/stream" /// ``` pub fn build_audio_url(pk: &str, param: Option<&str>) -> anyhow::Result { - // Récupérer l'URL de base depuis la variable d'environnement ou une config - let base_url = - std::env::var("PMO_SERVER_URL").unwrap_or_else(|_| "http://localhost:8080".to_string()); - let cache = get_audio_cache().ok_or_else(|| anyhow::anyhow!("No registered audio cache"))?; - - let route = cache.route_for(pk, param); - Ok(format!("{}{}", base_url, route)) + Ok(cache.absolute_url_for(pk, param)) }