feat(cache): fix lazy PK metadata resolution and playlist track ordering
- In `track_metadata.rs`, implement fallback logic for lazy PKs: first try real_pk, then fallback to lazy_pk for metadata keys seeded before download. - In `db.rs`, add debug logging after migration to track remaining metadata under lazy_pk. - In `source.rs` (Qobuz): - Detect and repair playlists with missing cache entries by forcing refresh when `items.len() < total`. - Preserve original track order after parallel processing by attaching and sorting on original index. - Handle missing performer/cover gracefully with warnings and provider fallback. - In `metadata_cache.rs` (Radio France), simplify logging and improve test coverage for expiration logic. - In `cache.rs`, add debug/warning logs when metadata or cover seeding fails during lazy caching.
This commit is contained in:
@@ -35,6 +35,18 @@ impl AudioCacheTrackMetadata {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn read_raw(&self, key: &str) -> Result<Option<Value>, MetadataError> {
|
fn read_raw(&self, key: &str) -> Result<Option<Value>, MetadataError> {
|
||||||
|
// Si le pk est un lazy pk déjà téléchargé (pk != lazy_pk dans l'asset),
|
||||||
|
// lire d'abord sous le real_pk (métadonnées écrites par FlacCacheSink),
|
||||||
|
// puis fallback sous le lazy_pk (cover_pk, qobuz_track_id semés à l'enregistrement).
|
||||||
|
if pmocache::is_lazy_pk(&self.pk) {
|
||||||
|
if let Ok(Some(real_pk)) = self.cache.db.get_pk_by_lazy_pk(&self.pk) {
|
||||||
|
if let Ok(Some(v)) = self.cache.db.get_a_metadata(&real_pk, key) {
|
||||||
|
return Ok(Some(v));
|
||||||
|
}
|
||||||
|
// Fallback vers lazy_pk pour les clés semées avant téléchargement (cover_pk, etc.)
|
||||||
|
return self.cache.db.get_a_metadata(&self.pk, key).map_err(map_db_err);
|
||||||
|
}
|
||||||
|
}
|
||||||
self.cache
|
self.cache
|
||||||
.db
|
.db
|
||||||
.get_a_metadata(&self.pk, key)
|
.get_a_metadata(&self.pk, key)
|
||||||
|
|||||||
@@ -1137,7 +1137,23 @@ impl DB {
|
|||||||
return Err(Error::QueryReturnedNoRows);
|
return Err(Error::QueryReturnedNoRows);
|
||||||
}
|
}
|
||||||
|
|
||||||
tx.commit()
|
tx.commit()?;
|
||||||
|
|
||||||
|
// Compter les métadonnées encore sous l'ancien lazy_pk (après migration de l'asset)
|
||||||
|
let meta_under_lazy: i64 = {
|
||||||
|
let conn = self.lock_conn("update_lazy_to_downloaded_meta_check");
|
||||||
|
conn.query_row(
|
||||||
|
"SELECT COUNT(*) FROM metadata WHERE pk = ?1",
|
||||||
|
[lazy_pk],
|
||||||
|
|r| r.get(0),
|
||||||
|
).unwrap_or(0)
|
||||||
|
};
|
||||||
|
tracing::debug!(
|
||||||
|
"update_lazy_to_downloaded: {} → {} ({} metadata rows still under lazy_pk)",
|
||||||
|
lazy_pk, real_pk, meta_under_lazy
|
||||||
|
);
|
||||||
|
|
||||||
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Recherche une entry par son origin_url
|
/// Recherche une entry par son origin_url
|
||||||
|
|||||||
@@ -795,11 +795,19 @@ impl QobuzSource {
|
|||||||
.get_read_handle(&pmo_playlist_id)
|
.get_read_handle(&pmo_playlist_id)
|
||||||
.await
|
.await
|
||||||
.map_err(|e| MusicSourceError::PlaylistError(e.to_string()))?;
|
.map_err(|e| MusicSourceError::PlaylistError(e.to_string()))?;
|
||||||
let (items, _total) = reader
|
let (items, total) = reader
|
||||||
.to_items_paged(0, usize::MAX)
|
.to_items_paged(0, usize::MAX)
|
||||||
.await
|
.await
|
||||||
.map_err(|e| MusicSourceError::PlaylistError(e.to_string()))?;
|
.map_err(|e| MusicSourceError::PlaylistError(e.to_string()))?;
|
||||||
return self.adapt_items_to_qobuz(items, &parent_id).await;
|
// Si des PKs ne sont pas enregistrés dans le cache audio (e.g. playlist créée
|
||||||
|
// avec l'ancien code), on force un refresh pour les ré-enregistrer proprement.
|
||||||
|
if items.len() == total {
|
||||||
|
return self.adapt_items_to_qobuz(items, &parent_id).await;
|
||||||
|
}
|
||||||
|
info!(
|
||||||
|
"Qobuz playlist {} has {}/{} valid items, forcing refresh to repair missing entries",
|
||||||
|
pmo_playlist_id, items.len(), total
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
info!("Qobuz playlist {} creating/refreshing (version {:?})", pmo_playlist_id, qobuz_version);
|
info!("Qobuz playlist {} creating/refreshing (version {:?})", pmo_playlist_id, qobuz_version);
|
||||||
@@ -1047,7 +1055,9 @@ impl QobuzSource {
|
|||||||
// 600 tracks ne génèrent que ~N_albums_uniques téléchargements réels.
|
// 600 tracks ne génèrent que ~N_albums_uniques téléchargements réels.
|
||||||
let sem = std::sync::Arc::new(tokio::sync::Semaphore::new(16));
|
let sem = std::sync::Arc::new(tokio::sync::Semaphore::new(16));
|
||||||
|
|
||||||
let futs: Vec<_> = tracks.iter().map(|track| {
|
// On attache l'index original à chaque future pour pouvoir retrier dans l'ordre
|
||||||
|
// d'origine après complétion parallèle (JoinSet retourne dans l'ordre de fin).
|
||||||
|
let futs: Vec<_> = tracks.iter().enumerate().map(|(idx, track)| {
|
||||||
let source = self.clone();
|
let source = self.clone();
|
||||||
let sem = sem.clone();
|
let sem = sem.clone();
|
||||||
let track_id = track.id.clone();
|
let track_id = track.id.clone();
|
||||||
@@ -1084,16 +1094,31 @@ impl QobuzSource {
|
|||||||
None
|
None
|
||||||
};
|
};
|
||||||
|
|
||||||
// 2. Register lazy entry + set cover_pk + seed metadata
|
// 2. Register lazy entry + set cover_pk + seed metadata.
|
||||||
|
// Si le performer est absent (réponse API incomplète), on passe None pour
|
||||||
|
// que le provider appelle get_track et récupère les métadonnées complètes.
|
||||||
|
if metadata.artist.is_none() {
|
||||||
|
tracing::warn!(
|
||||||
|
"register_tracks_lazy: no performer for track {} (id={}), will call provider",
|
||||||
|
track_title, track_id
|
||||||
|
);
|
||||||
|
}
|
||||||
|
if cover_pk.is_none() && cover_image_url.is_some() {
|
||||||
|
tracing::warn!(
|
||||||
|
"register_tracks_lazy: cover download failed for track {} (id={}), will call provider for cover",
|
||||||
|
track_title, track_id
|
||||||
|
);
|
||||||
|
}
|
||||||
|
let meta_hint = if metadata.artist.is_some() { Some(metadata) } else { None };
|
||||||
match source.inner.cache_manager
|
match source.inner.cache_manager
|
||||||
.cache_audio_lazy_with_provider(&lazy_pk, Some(metadata), cover_pk)
|
.cache_audio_lazy_with_provider(&lazy_pk, meta_hint, cover_pk)
|
||||||
.await
|
.await
|
||||||
{
|
{
|
||||||
Ok(pk) => {
|
Ok(pk) => {
|
||||||
let _ = source.inner.cache_manager.set_audio_metadata(
|
let _ = source.inner.cache_manager.set_audio_metadata(
|
||||||
&pk, "qobuz_track_id", json!(track_id),
|
&pk, "qobuz_track_id", json!(track_id),
|
||||||
);
|
);
|
||||||
Some(pk)
|
Some((idx, pk))
|
||||||
}
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
tracing::warn!("Failed to register lazy track {}: {}", track_title, e);
|
tracing::warn!("Failed to register lazy track {}: {}", track_title, e);
|
||||||
@@ -1103,12 +1128,15 @@ impl QobuzSource {
|
|||||||
}
|
}
|
||||||
}).collect();
|
}).collect();
|
||||||
|
|
||||||
tokio::task::JoinSet::from_iter(futs)
|
// Retrier par index original pour conserver l'ordre de la playlist Qobuz
|
||||||
|
let mut results: Vec<(usize, String)> = tokio::task::JoinSet::from_iter(futs)
|
||||||
.join_all()
|
.join_all()
|
||||||
.await
|
.await
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.flatten()
|
.flatten()
|
||||||
.collect()
|
.collect();
|
||||||
|
results.sort_unstable_by_key(|(i, _)| *i);
|
||||||
|
results.into_iter().map(|(_, pk)| pk).collect()
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Cache les covers d'une liste d'artistes en parallèle.
|
/// Cache les covers d'une liste d'artistes en parallèle.
|
||||||
|
|||||||
@@ -26,9 +26,6 @@ use tokio::sync::RwLock;
|
|||||||
#[cfg(feature = "cache")]
|
#[cfg(feature = "cache")]
|
||||||
use pmocovers::Cache as CoverCache;
|
use pmocovers::Cache as CoverCache;
|
||||||
|
|
||||||
#[cfg(feature = "cache")]
|
|
||||||
use pmocache::cache_trait::FileCache;
|
|
||||||
|
|
||||||
// ============================================================================
|
// ============================================================================
|
||||||
// CachedMetadata
|
// CachedMetadata
|
||||||
// ============================================================================
|
// ============================================================================
|
||||||
@@ -583,11 +580,7 @@ impl MetadataCache {
|
|||||||
}
|
}
|
||||||
Err(discover_err) => {
|
Err(discover_err) => {
|
||||||
#[cfg(feature = "logging")]
|
#[cfg(feature = "logging")]
|
||||||
tracing::error!(
|
tracing::error!("Rediscovery failed for '{}': {}", slug, discover_err);
|
||||||
"Rediscovery failed for '{}': {}",
|
|
||||||
slug,
|
|
||||||
discover_err
|
|
||||||
);
|
|
||||||
return Err(e);
|
return Err(e);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -697,12 +690,7 @@ impl MetadataCache {
|
|||||||
match self.client.rediscover_station(slug).await {
|
match self.client.rediscover_station(slug).await {
|
||||||
Ok((id, url)) => {
|
Ok((id, url)) => {
|
||||||
#[cfg(feature = "logging")]
|
#[cfg(feature = "logging")]
|
||||||
tracing::info!(
|
tracing::info!("Re-discovered station '{}': id={}, url={}", slug, id, url);
|
||||||
"Re-discovered station '{}': id={}, url={}",
|
|
||||||
slug,
|
|
||||||
id,
|
|
||||||
url
|
|
||||||
);
|
|
||||||
self.persist_station_mapping();
|
self.persist_station_mapping();
|
||||||
// Invalider le cache mémoire pour forcer un refresh des métadonnées
|
// Invalider le cache mémoire pour forcer un refresh des métadonnées
|
||||||
let mut cache = self.cache.write().await;
|
let mut cache = self.cache.write().await;
|
||||||
@@ -801,11 +789,18 @@ mod tests {
|
|||||||
assert!(base.is_expired());
|
assert!(base.is_expired());
|
||||||
|
|
||||||
// end_time dans le futur → non expiré
|
// end_time dans le futur → non expiré
|
||||||
let not_expired = CachedMetadata { end_time: Some(now + 3600), ..base.clone() };
|
let not_expired = CachedMetadata {
|
||||||
|
end_time: Some(now + 3600),
|
||||||
|
..base.clone()
|
||||||
|
};
|
||||||
assert!(!not_expired.is_expired());
|
assert!(!not_expired.is_expired());
|
||||||
|
|
||||||
// Pas de end_time, fetched_at récent → non expiré (fallback TTL)
|
// Pas de end_time, fetched_at récent → non expiré (fallback TTL)
|
||||||
let no_end_fresh = CachedMetadata { end_time: None, fetched_at: now, ..base.clone() };
|
let no_end_fresh = CachedMetadata {
|
||||||
|
end_time: None,
|
||||||
|
fetched_at: now,
|
||||||
|
..base.clone()
|
||||||
|
};
|
||||||
assert!(!no_end_fresh.is_expired());
|
assert!(!no_end_fresh.is_expired());
|
||||||
|
|
||||||
// Pas de end_time, fetched_at ancien → expiré
|
// Pas de end_time, fetched_at ancien → expiré
|
||||||
|
|||||||
@@ -328,6 +328,7 @@ impl SourceCacheManager {
|
|||||||
None
|
None
|
||||||
};
|
};
|
||||||
|
|
||||||
|
let metadata_is_some = metadata.is_some();
|
||||||
let mut final_metadata = metadata;
|
let mut final_metadata = metadata;
|
||||||
if final_metadata.is_none() {
|
if final_metadata.is_none() {
|
||||||
if let Some(data) = provider_data.as_ref() {
|
if let Some(data) = provider_data.as_ref() {
|
||||||
@@ -352,10 +353,25 @@ impl SourceCacheManager {
|
|||||||
|
|
||||||
if let Some(meta) = final_metadata.as_ref() {
|
if let Some(meta) = final_metadata.as_ref() {
|
||||||
self.seed_audio_metadata(lazy_pk, meta);
|
self.seed_audio_metadata(lazy_pk, meta);
|
||||||
|
} else {
|
||||||
|
#[cfg(feature = "server")]
|
||||||
|
tracing::warn!(
|
||||||
|
"cache_audio_lazy_with_provider: no metadata to seed for {} \
|
||||||
|
(meta_hint={}, provider_data={})",
|
||||||
|
lazy_pk,
|
||||||
|
metadata_is_some,
|
||||||
|
provider_data.as_ref().map(|d| d.metadata.is_some()).unwrap_or(false)
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
if let Some(cover_pk) = final_cover_pk {
|
if let Some(cover_pk) = final_cover_pk {
|
||||||
let _ = self.set_audio_metadata(lazy_pk, "cover_pk", json!(cover_pk));
|
let _ = self.set_audio_metadata(lazy_pk, "cover_pk", json!(cover_pk));
|
||||||
|
} else {
|
||||||
|
#[cfg(feature = "server")]
|
||||||
|
tracing::debug!(
|
||||||
|
"cache_audio_lazy_with_provider: no cover_pk seeded for {}",
|
||||||
|
lazy_pk
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
Ok(lazy_pk.to_string())
|
Ok(lazy_pk.to_string())
|
||||||
|
|||||||
Reference in New Issue
Block a user