From 5a7be91e0f259c06b8d7a12163f68eae5b0e5c1d Mon Sep 17 00:00:00 2001 From: Tsiry Sandratraina Date: Sun, 28 Sep 2025 13:33:53 +0300 Subject: [PATCH] feat: Enhance MusicBrainz integration and improve data handling - Updated Release struct to include optional fields for artist credit, track count, and release group. - Introduced ReleaseGroup struct for better organization of release data. - Modified scrobble function to utilize MusicBrainzClient passed as a parameter, improving dependency management. - Implemented search_musicbrainz_recording function to streamline MusicBrainz recording searches and handle errors gracefully. - Enhanced caching mechanism for MusicBrainz responses to reduce redundant API calls. - Improved query construction for MusicBrainz searches to include status filtering. - Added tests for MusicBrainz client and release selection logic to ensure reliability. - Refactored artist credit handling in Track conversion for better safety and clarity. - Updated dependencies in Cargo.toml for improved functionality and testing capabilities. --- Cargo.lock | 61 +++- crates/scrobbler/Cargo.toml | 6 +- crates/scrobbler/src/handlers/mod.rs | 20 +- crates/scrobbler/src/handlers/scrobble.rs | 7 +- .../scrobbler/src/handlers/v1/submission.rs | 6 +- crates/scrobbler/src/lib.rs | 6 +- .../scrobbler/src/listenbrainz/core/submit.rs | 4 +- crates/scrobbler/src/listenbrainz/handlers.rs | 5 +- crates/scrobbler/src/musicbrainz/artist.rs | 4 +- crates/scrobbler/src/musicbrainz/client.rs | 334 ++++++++++++++++-- crates/scrobbler/src/musicbrainz/mod.rs | 276 +++++++++++++++ crates/scrobbler/src/musicbrainz/release.rs | 33 +- crates/scrobbler/src/scrobbler.rs | 84 +++-- crates/scrobbler/src/types.rs | 8 +- crates/webscrobbler/Cargo.toml | 4 + crates/webscrobbler/src/handlers.rs | 9 +- crates/webscrobbler/src/lib.rs | 6 +- crates/webscrobbler/src/musicbrainz/artist.rs | 4 +- crates/webscrobbler/src/musicbrainz/client.rs | 334 ++++++++++++++++-- crates/webscrobbler/src/musicbrainz/mod.rs | 275 ++++++++++++++ .../webscrobbler/src/musicbrainz/release.rs | 33 +- crates/webscrobbler/src/scrobbler.rs | 65 +++- crates/webscrobbler/src/types.rs | 5 +- 23 files changed, 1466 insertions(+), 123 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e77930a..1ce9643 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3184,6 +3184,15 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "nanoid" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ffa00dec017b5b1a8b7cf5e2c008bfda1aa7e0697ac1508b491fdf2622fb4d8" +dependencies = [ + "rand 0.8.5", +] + [[package]] name = "nkeys" version = "0.4.4" @@ -4697,9 +4706,9 @@ dependencies = [ [[package]] name = "regex" -version = "1.11.1" +version = "1.11.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b544ef1b4eac5dc2db33ea63606ae9ffcfac26c1416a2806ae0bf5f56b201191" +checksum = "8b5288124840bee7b386bc413c487869b360b2b4ec421ea56425128692f2a82c" dependencies = [ "aho-corasick", "memchr", @@ -4709,9 +4718,9 @@ dependencies = [ [[package]] name = "regex-automata" -version = "0.4.9" +version = "0.4.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "809e8dc61f6de73b46c85f4c96486310fe304c434cfa43669d7b40f711150908" +checksum = "833eb9ce86d40ef33cb1306d8accf7bc8ec2bfea4355cbdebb3df68b40925cad" dependencies = [ "aho-corasick", "memchr", @@ -4999,6 +5008,7 @@ dependencies = [ "hex", "jsonwebtoken", "md5", + "nanoid", "owo-colors", "quick-xml 0.37.5", "rand 0.9.2", @@ -5006,6 +5016,7 @@ dependencies = [ "reqwest", "serde", "serde_json", + "serial_test", "sqlx", "tokio", "tokio-stream", @@ -5089,12 +5100,14 @@ dependencies = [ "hex", "jsonwebtoken", "md5", + "nanoid", "owo-colors", "rand 0.9.2", "redis 0.29.5", "reqwest", "serde", "serde_json", + "serial_test", "sqlx", "tokio", "tokio-stream", @@ -5438,6 +5451,15 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "scc" +version = "2.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46e6f046b7fef48e2660c57ed794263155d713de679057f2d0c169bfc6e756cc" +dependencies = [ + "sdd", +] + [[package]] name = "schannel" version = "0.1.27" @@ -5463,6 +5485,12 @@ dependencies = [ "untrusted", ] +[[package]] +name = "sdd" +version = "3.0.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "490dcfcbfef26be6800d11870ff2df8774fa6e86d047e3e8c8a76b25655e41ca" + [[package]] name = "seahash" version = "4.1.0" @@ -5612,6 +5640,31 @@ dependencies = [ "serde", ] +[[package]] +name = "serial_test" +version = "3.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1b258109f244e1d6891bf1053a55d63a5cd4f8f4c30cf9a1280989f80e7a1fa9" +dependencies = [ + "futures", + "log", + "once_cell", + "parking_lot", + "scc", + "serial_test_derive", +] + +[[package]] +name = "serial_test_derive" +version = "3.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d69265a08751de7844521fd15003ae0a888e035773ba05695c5c759a6f89eef" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.101", +] + [[package]] name = "sha1" version = "0.10.6" diff --git a/crates/scrobbler/Cargo.toml b/crates/scrobbler/Cargo.toml index 15f4f8c..66e8333 100644 --- a/crates/scrobbler/Cargo.toml +++ b/crates/scrobbler/Cargo.toml @@ -22,7 +22,7 @@ owo-colors = "4.1.0" dotenv = "0.15.0" anyhow = "1.0.96" actix-web = "4.9.0" -redis = "0.29.0" +redis = { version = "0.29.0", features = ["tokio-comp"] } hex = "0.4.3" jsonwebtoken = "9.3.1" md5 = "0.7.0" @@ -41,3 +41,7 @@ actix-limitation = "0.5.1" actix-session = "0.10.1" tokio-stream = { version = "0.1.17", features = ["full"] } tracing = "0.1.41" +nanoid = "0.4.0" + +[dev-dependencies] +serial_test = "3.0.0" diff --git a/crates/scrobbler/src/handlers/mod.rs b/crates/scrobbler/src/handlers/mod.rs index 3e4ee34..54f04cf 100644 --- a/crates/scrobbler/src/handlers/mod.rs +++ b/crates/scrobbler/src/handlers/mod.rs @@ -9,6 +9,7 @@ use v1::nowplaying::nowplaying; use v1::submission::submission; use crate::cache::Cache; +use crate::musicbrainz::client::MusicbrainzClient; use crate::BANNER; pub mod scrobble; @@ -43,11 +44,17 @@ pub async fn handle_nowplaying( pub async fn handle_submission( data: web::Data>>, cache: web::Data, + mb_client: web::Data>, form: web::Form>, ) -> impl Responder { - submission(form.into_inner(), cache.get_ref(), data.get_ref()) - .await - .map_err(actix_web::error::ErrorInternalServerError) + submission( + form.into_inner(), + cache.get_ref(), + data.get_ref(), + mb_client.get_ref(), + ) + .await + .map_err(actix_web::error::ErrorInternalServerError) } #[get("/2.0")] @@ -60,12 +67,14 @@ pub async fn handle_methods( data: web::Data>>, cache: web::Data, form: web::Form>, + mb_client: web::Data>, ) -> impl Responder { let conn = data.get_ref(); let cache = cache.get_ref(); + let mb_client = mb_client.get_ref(); let method = form.get("method").unwrap_or(&"".to_string()).to_string(); - call_method(&method, conn, cache, form.into_inner()) + call_method(&method, conn, cache, mb_client, form.into_inner()) .await .map_err(actix_web::error::ErrorInternalServerError) } @@ -74,10 +83,11 @@ pub async fn call_method( method: &str, pool: &Arc>, cache: &Cache, + mb_client: &Arc, form: BTreeMap, ) -> Result { match method { - "track.scrobble" => handle_scrobble(form, pool, cache).await, + "track.scrobble" => handle_scrobble(form, pool, cache, mb_client).await, _ => Err(Error::msg(format!("Unsupported method: {}", method))), } } diff --git a/crates/scrobbler/src/handlers/scrobble.rs b/crates/scrobbler/src/handlers/scrobble.rs index 24bf342..999685a 100644 --- a/crates/scrobbler/src/handlers/scrobble.rs +++ b/crates/scrobbler/src/handlers/scrobble.rs @@ -5,14 +5,15 @@ use sqlx::Pool; use std::collections::BTreeMap; use crate::{ - auth::authenticate, cache::Cache, params::validate_scrobble_params, response::build_response, - scrobbler::scrobble, + auth::authenticate, cache::Cache, musicbrainz::client::MusicbrainzClient, + params::validate_scrobble_params, response::build_response, scrobbler::scrobble, }; pub async fn handle_scrobble( form: BTreeMap, conn: &Pool, cache: &Cache, + mb_client: &MusicbrainzClient, ) -> Result { let params = match validate_scrobble_params(&form, &["api_key", "api_sig", "sk", "method"]) { Ok(params) => params, @@ -31,7 +32,7 @@ pub async fn handle_scrobble( }))); } - match scrobble(&conn, cache, &form).await { + match scrobble(&conn, cache, mb_client, &form).await { Ok(scrobbles) => Ok(HttpResponse::Ok().json(build_response(scrobbles))), Err(e) => { if e.to_string().contains("Timestamp") { diff --git a/crates/scrobbler/src/handlers/v1/submission.rs b/crates/scrobbler/src/handlers/v1/submission.rs index dd0a87e..d3fe8fa 100644 --- a/crates/scrobbler/src/handlers/v1/submission.rs +++ b/crates/scrobbler/src/handlers/v1/submission.rs @@ -4,13 +4,15 @@ use serde_json::json; use std::{collections::BTreeMap, sync::Arc}; use crate::{ - auth::verify_session_id, cache::Cache, params::validate_required_params, scrobbler::scrobble_v1, + auth::verify_session_id, cache::Cache, musicbrainz::client::MusicbrainzClient, + params::validate_required_params, scrobbler::scrobble_v1, }; pub async fn submission( form: BTreeMap, cache: &Cache, pool: &Arc>, + mb_client: &Arc, ) -> Result { match validate_required_params(&form, &["s", "a[0]", "t[0]", "i[0]"]) { Ok(_) => { @@ -30,7 +32,7 @@ pub async fn submission( let user_id = user_id.unwrap(); tracing::info!(artist = %a, track = %t, timestamp = %i, user_id = %user_id, "Submission"); - match scrobble_v1(pool, cache, &form).await { + match scrobble_v1(pool, cache, mb_client, &form).await { Ok(_) => Ok(HttpResponse::Ok().body("OK\n")), Err(e) => Ok(HttpResponse::BadRequest().json(json!({ "error": 4, diff --git a/crates/scrobbler/src/lib.rs b/crates/scrobbler/src/lib.rs index 1b23032..523989a 100644 --- a/crates/scrobbler/src/lib.rs +++ b/crates/scrobbler/src/lib.rs @@ -27,7 +27,7 @@ use anyhow::Error; use owo_colors::OwoColorize; use sqlx::postgres::PgPoolOptions; -use crate::cache::Cache; +use crate::{cache::Cache, musicbrainz::client::MusicbrainzClient}; pub const BANNER: &str = r#" ___ ___ _____ __ __ __ @@ -71,12 +71,16 @@ pub async fn run() -> Result<(), Error> { .unwrap(), ); + let mb_client = MusicbrainzClient::new().await?; + let mb_client = Arc::new(mb_client); + HttpServer::new(move || { App::new() .wrap(RateLimiter::default()) .app_data(limiter.clone()) .app_data(Data::new(conn.clone())) .app_data(Data::new(cache.clone())) + .app_data(Data::new(mb_client.clone())) .service(handlers::handle_methods) .service(handlers::handle_nowplaying) .service(handlers::handle_submission) diff --git a/crates/scrobbler/src/listenbrainz/core/submit.rs b/crates/scrobbler/src/listenbrainz/core/submit.rs index 7e0dd59..ffe180d 100644 --- a/crates/scrobbler/src/listenbrainz/core/submit.rs +++ b/crates/scrobbler/src/listenbrainz/core/submit.rs @@ -5,6 +5,7 @@ use serde_json::json; use std::sync::Arc; use crate::auth::decode_token; +use crate::musicbrainz::client::MusicbrainzClient; use crate::repo; use crate::{cache::Cache, scrobbler::scrobble_listenbrainz}; @@ -14,6 +15,7 @@ pub async fn submit_listens( payload: SubmitListensRequest, cache: &Cache, pool: &Arc>, + mb_client: &Arc, token: &str, ) -> Result { if payload.listen_type != "playing_now" { @@ -29,7 +31,7 @@ pub async fn submit_listens( const RETRIES: usize = 15; for attempt in 1..=RETRIES { - match scrobble_listenbrainz(pool, cache, &payload, token).await { + match scrobble_listenbrainz(pool, cache, mb_client, &payload, token).await { Ok(_) => { return Ok(HttpResponse::Ok().json(json!({ "status": "ok", diff --git a/crates/scrobbler/src/listenbrainz/handlers.rs b/crates/scrobbler/src/listenbrainz/handlers.rs index 55ec2a7..a5fdba0 100644 --- a/crates/scrobbler/src/listenbrainz/handlers.rs +++ b/crates/scrobbler/src/listenbrainz/handlers.rs @@ -16,6 +16,7 @@ use crate::{ }, types::SubmitListensRequest, }, + musicbrainz::client::MusicbrainzClient, read_payload, repo, }; use tokio_stream::StreamExt; @@ -39,6 +40,7 @@ pub async fn handle_submit_listens( req: HttpRequest, data: web::Data>>, cache: web::Data, + mb_client: web::Data>, mut payload: web::Payload, ) -> impl Responder { let token = match req.headers().get("Authorization") { @@ -63,7 +65,8 @@ pub async fn handle_submit_listens( }) .map_err(actix_web::error::ErrorBadRequest)?; - submit_listens(req, cache.get_ref(), data.get_ref(), token) + let mb_client = mb_client.get_ref(); + submit_listens(req, cache.get_ref(), data.get_ref(), &mb_client, token) .await .map_err(actix_web::error::ErrorInternalServerError) } diff --git a/crates/scrobbler/src/musicbrainz/artist.rs b/crates/scrobbler/src/musicbrainz/artist.rs index ed759b3..fda5bbc 100644 --- a/crates/scrobbler/src/musicbrainz/artist.rs +++ b/crates/scrobbler/src/musicbrainz/artist.rs @@ -1,6 +1,6 @@ use serde::{Deserialize, Serialize}; -#[derive(Debug, Deserialize, Clone)] +#[derive(Debug, Deserialize, Clone, Default)] pub struct Artist { pub name: String, #[serde(rename = "sort-name")] @@ -29,7 +29,7 @@ pub struct Artist { pub aliases: Option>, } -#[derive(Debug, Deserialize, Clone)] +#[derive(Debug, Deserialize, Clone, Default)] pub struct ArtistCredit { pub joinphrase: Option, pub name: String, diff --git a/crates/scrobbler/src/musicbrainz/client.rs b/crates/scrobbler/src/musicbrainz/client.rs index f68ef51..2c6c68e 100644 --- a/crates/scrobbler/src/musicbrainz/client.rs +++ b/crates/scrobbler/src/musicbrainz/client.rs @@ -1,41 +1,327 @@ +use std::env; + use super::recording::{Recording, Recordings}; -use anyhow::Error; +use anyhow::{anyhow, Context, Error}; +use redis::aio::MultiplexedConnection; +use redis::AsyncCommands; +use serde::{Deserialize, Serialize}; +use tokio::time::{timeout, Duration, Instant}; pub const BASE_URL: &str = "https://musicbrainz.org/ws/2"; -pub const USER_AGENT: &str = "Rocksky/0.1.0"; +pub const USER_AGENT: &str = "Rocksky/0.1.0 (+https://rocksky.app)"; + +const Q_QUEUE: &str = "mb:queue:v1"; +const CACHE_SEARCH_PREFIX: &str = "mb:cache:search:"; +const CACHE_REC_PREFIX: &str = "mb:cache:rec:"; +const INFLIGHT_PREFIX: &str = "mb:inflight:"; +const RESP_PREFIX: &str = "mb:resp:"; -pub struct MusicbrainzClient {} +const CACHE_TTL_SECS: u64 = 60 * 60 * 24; +const WAIT_TIMEOUT_SECS: u64 = 20; +const INFLIGHT_TTL_SECS: i64 = 30; // de-dup window while the worker is fetching + +#[derive(Clone)] +pub struct MusicbrainzClient { + http: reqwest::Client, + redis: MultiplexedConnection, + cache_ttl: u64, +} + +#[derive(Debug, Serialize, Deserialize)] +#[serde(tag = "kind", rename_all = "snake_case")] +enum Job { + Search { id: String, query: String }, + GetRecording { id: String, mbid: String }, +} impl MusicbrainzClient { - pub fn new() -> Self { - MusicbrainzClient {} + pub async fn new() -> Result { + let client = + redis::Client::open(env::var("REDIS_URL").unwrap_or("redis://127.0.0.1".into()))?; + let redis = client.get_multiplexed_tokio_connection().await?; + let http = reqwest::Client::builder() + .user_agent(USER_AGENT) + .build() + .context("build http client")?; + let me = MusicbrainzClient { + http, + redis, + cache_ttl: CACHE_TTL_SECS, + }; + + let mut worker_conn = client.get_multiplexed_async_connection().await?; + + let http = me.http.clone(); + tokio::spawn(async move { worker_loop(http, &mut worker_conn).await }); + + Ok(me) } pub async fn search(&self, query: &str) -> Result { - let url = format!("{}/recording", BASE_URL); - let client = reqwest::Client::new(); - let response = client - .get(&url) - .header("Accept", "application/json") - .header("User-Agent", USER_AGENT) - .query(&[("query", query), ("inc", "artist-credits+releases")]) - .send() + if let Some(h) = self.get_cache(&cache_key_search(query)).await? { + return Ok(serde_json::from_str(&h).context("decode cached search")?); + } + let id = nanoid::nanoid!(); + let job = Job::Search { + id: id.clone(), + query: query.to_string(), + }; + self.enqueue_if_needed(&job, &infl_key_search(query)) .await?; - Ok(response.json().await?) + let raw = self.wait_for_response(&id).await?; + let parsed: Recordings = serde_json::from_str(&raw).context("decode search response")?; + + self.set_cache(&cache_key_search(query), &raw).await?; + Ok(parsed) } pub async fn get_recording(&self, mbid: &str) -> Result { - let url = format!("{}/recording/{}", BASE_URL, mbid); - let client = reqwest::Client::new(); - let response = client - .get(&url) - .header("Accept", "application/json") - .header("User-Agent", USER_AGENT) - .query(&[("inc", "artist-credits+releases")]) - .send() - .await?; + if let Some(h) = self.get_cache(&cache_key_rec(mbid)).await? { + return Ok(serde_json::from_str(&h).context("decode cached recording")?); + } + let id = nanoid::nanoid!(); + let job = Job::GetRecording { + id: id.clone(), + mbid: mbid.to_string(), + }; + self.enqueue_if_needed(&job, &infl_key_rec(mbid)).await?; + let raw = self.wait_for_response(&id).await?; + let parsed: Recording = serde_json::from_str(&raw).context("decode recording response")?; + self.set_cache(&cache_key_rec(mbid), &raw).await?; + Ok(parsed) + } + + // ---------- Redis helpers ---------- + + async fn get_cache(&self, key: &str) -> Result, Error> { + let mut r = self.redis.clone(); + let val: Option = r.get(key).await?; + Ok(val) + } + + async fn set_cache(&self, key: &str, json: &str) -> Result<(), Error> { + let mut r = self.redis.clone(); + let _: () = r + .set_ex(key, json, self.cache_ttl) + .await + .with_context(|| format!("cache set {key}"))?; + Ok(()) + } + + async fn enqueue_if_needed(&self, job: &Job, inflight_key: &str) -> Result<(), Error> { + let mut r = self.redis.clone(); + + // set NX to avoid duplicate work; short TTL + let set: bool = r.set_nx(inflight_key, "1").await.context("set in-flight")?; + if set { + let _: () = r + .expire(inflight_key, INFLIGHT_TTL_SECS) + .await + .context("expire inflight")?; + let payload = serde_json::to_string(job).expect("serialize job"); + let _: () = r.rpush(Q_QUEUE, payload).await.context("enqueue job")?; + } + Ok(()) + } + + async fn wait_for_response(&self, id: &str) -> Result { + let mut r = self.redis.clone(); + let resp_q = resp_key(id); + + let fut = async { + loop { + let popped: Option<(String, String)> = r.brpop(&resp_q, 2.0).await?; + if let Some((_key, json)) = popped { + return Ok::(json); + } + } + }; + + match timeout(Duration::from_secs(WAIT_TIMEOUT_SECS), fut).await { + Ok(res) => res, + Err(_) => Err(anyhow!("timed out waiting for MusicBrainz response")), + } + } +} + +async fn worker_loop( + http: reqwest::Client, + redis: &mut MultiplexedConnection, +) -> Result<(), Error> { + // pacing ticker: strictly 1 request/second + let mut next_allowed = Instant::now(); + + loop { + tokio::select! { + res = async { + + // finite timeout pop + let v: Option> = redis.blpop(Q_QUEUE, 2.0).await.ok(); + Ok::<_, Error>(v) + } => { + let Some(mut v) = res? else { + continue }; + if v.len() != 2 { + continue; } + let payload = v.pop().unwrap(); + + // 1 rps pacing + let now = Instant::now(); + if now < next_allowed { tokio::time::sleep(next_allowed - now).await; } + next_allowed = Instant::now() + Duration::from_secs(1); + + let payload: Job = match serde_json::from_str(&payload) { + Ok(j) => j, + Err(e) => { + tracing::error!(%e, "invalid job payload"); + continue; + } + }; + if let Err(e) = process_job(&http, redis, payload).await { + tracing::error!(%e, "job failed"); + } + } + } + } +} + +async fn process_job( + http: &reqwest::Client, + redis: &mut MultiplexedConnection, + job: Job, +) -> Result<(), Error> { + match job { + Job::Search { id, query } => { + let url = format!("{}/recording", BASE_URL); + let resp = http + .get(&url) + .header("Accept", "application/json") + .query(&[ + ("query", query.as_str()), + ("fmt", "json"), + ("inc", "artists+releases+isrcs"), + ]) + .send() + .await + .context("http search")?; + + if !resp.status().is_success() { + // Push an error payload so waiters don’t hang forever + let _ = push_response( + redis, + &id, + &format!(r#"{{"error":"http {}"}}"#, resp.status()), + ) + .await; + return Err(anyhow!("musicbrainz search http {}", resp.status())); + } + + let text = resp.text().await.context("read body")?; + push_response(redis, &id, &text).await?; + } + Job::GetRecording { id, mbid } => { + let url = format!("{}/recording/{}", BASE_URL, mbid); + let resp = http + .get(&url) + .header("Accept", "application/json") + .query(&[("fmt", "json"), ("inc", "artists+releases+isrcs")]) + .send() + .await + .context("http get_recording")?; + + if !resp.status().is_success() { + let _ = push_response( + redis, + &id, + &format!(r#"{{"error":"http {}"}}"#, resp.status()), + ) + .await; + return Err(anyhow!("musicbrainz get_recording http {}", resp.status())); + } + + let text = resp.text().await.context("read body")?; + push_response(redis, &id, &text).await?; + } + } + Ok(()) +} + +async fn push_response( + redis: &mut MultiplexedConnection, + id: &str, + json: &str, +) -> Result<(), Error> { + let q = resp_key(id); + // RPUSH then EXPIRE to avoid leaks if a client never BRPOPs + let _: () = redis.rpush(&q, json).await?; + let _: () = redis.expire(&q, WAIT_TIMEOUT_SECS as i64 + 5).await?; + Ok(()) +} + +fn cache_key_search(query: &str) -> String { + format!("{}{}", CACHE_SEARCH_PREFIX, fast_hash(query)) +} +fn cache_key_rec(mbid: &str) -> String { + format!("{}{}", CACHE_REC_PREFIX, mbid) +} +fn infl_key_search(query: &str) -> String { + format!("{}search:{}", INFLIGHT_PREFIX, fast_hash(query)) +} +fn infl_key_rec(mbid: &str) -> String { + format!("{}rec:{}", INFLIGHT_PREFIX, mbid) +} +fn resp_key(id: &str) -> String { + format!("{}{}", RESP_PREFIX, id) +} + +fn fast_hash(s: &str) -> u64 { + use std::hash::{Hash, Hasher}; + let mut h = std::collections::hash_map::DefaultHasher::new(); + s.hash(&mut h); + h.finish() +} + +#[cfg(test)] +mod tests { + use super::*; + use serial_test::serial; + + #[test] + fn test_fast_hash() { + let h1 = fast_hash("hello"); + let h2 = fast_hash("hello"); + let h3 = fast_hash("world"); + assert_eq!(h1, h2); + assert_ne!(h1, h3); + } + + #[test] + fn test_cache_keys() { + let q = "test query"; + let mbid = "some-mbid"; + assert!(cache_key_search(q).starts_with(CACHE_SEARCH_PREFIX)); + assert!(cache_key_rec(mbid).starts_with(CACHE_REC_PREFIX)); + assert!(infl_key_search(q).starts_with(INFLIGHT_PREFIX)); + assert!(infl_key_rec(mbid).starts_with(INFLIGHT_PREFIX)); + assert!(resp_key("id").starts_with(RESP_PREFIX)); + } + + #[tokio::test] + #[serial] + async fn test_musicbrainz_client() -> Result<(), Error> { + let client = MusicbrainzClient::new().await?; + let query = format!( + r#"recording:"{}" AND artist:"{}" AND status:Official"#, + "Come As You Are", "Nirvana" + ); + let search_res = client.search(&query).await?; - Ok(response.json().await?) + assert!(!search_res.recordings.is_empty()); + let rec = &search_res.recordings[0]; + let mbid = &rec.id; + let rec_res = client.get_recording(mbid).await?; + assert_eq!(rec_res.id, *mbid); + Ok(()) } } diff --git a/crates/scrobbler/src/musicbrainz/mod.rs b/crates/scrobbler/src/musicbrainz/mod.rs index acceccd..eee969f 100644 --- a/crates/scrobbler/src/musicbrainz/mod.rs +++ b/crates/scrobbler/src/musicbrainz/mod.rs @@ -1,5 +1,281 @@ +use crate::musicbrainz::{recording::Recordings, release::Release}; +use std::cmp::Ordering; + pub mod artist; pub mod client; pub mod label; pub mod recording; pub mod release; + +fn get_best_release(releases: &[Release]) -> Option { + if releases.is_empty() { + return None; + } + + // Remove the single filtering - this was causing the issue + let mut candidates: Vec<&Release> = releases.iter().collect(); + + if candidates.is_empty() { + return None; + } + + candidates.sort_by(|a, b| cmp_release(a, b)); + candidates.first().cloned().cloned() +} + +pub fn get_best_release_from_recordings(all: &Recordings, artist: &str) -> Option { + use std::collections::HashSet; + + let mut pool: Vec = Vec::new(); + let mut seen: HashSet = HashSet::new(); + + let all_recordings: Vec<&recording::Recording> = all + .recordings + .iter() + .filter(|rec| { + if let Some(credits) = &rec.artist_credit { + artist_credit_contains(credits, artist) + } else { + false + } + }) + .collect(); + + for rec in &all_recordings { + if let Some(rels) = &rec.releases { + for r in rels { + if seen.insert(r.id.clone()) { + pool.push(r.clone()); + } + } + } + } + + get_best_release(&pool) +} + +fn cmp_release(a: &Release, b: &Release) -> Ordering { + // First priority: prefer albums over singles + let sa = is_single_release_type(a); + let sb = is_single_release_type(b); + if sa != sb { + return bool_true_last(sa, sb); // Albums (false) come before singles (true) + } + + let ta = release_tier(a.status.as_deref()); + let tb = release_tier(b.status.as_deref()); + if ta != tb { + return ta.cmp(&tb); + } + + let pa = has_preferred_country(a, &["XW", "US"]); + let pb = has_preferred_country(b, &["XW", "US"]); + if pa != pb { + return bool_true_first(pa, pb); + } + + let la = is_live_release(a); + let lb = is_live_release(b); + if la != lb { + return bool_true_last(la, lb); + } + + let da = date_key(a.date.as_deref()); + let db = date_key(b.date.as_deref()); + if da != db { + return da.cmp(&db); + } + + match a.title.cmp(&b.title) { + Ordering::Equal => a.id.cmp(&b.id), + ord => ord, + } +} + +fn release_tier(status: Option<&str>) -> u8 { + match status.map(|s| s.to_ascii_lowercase()) { + Some(s) if s == "official" => 0, + Some(s) if s == "bootleg" => 1, + _ => 2, + } +} + +fn bool_true_first(a: bool, b: bool) -> Ordering { + match (a, b) { + (true, false) => Ordering::Less, + (false, true) => Ordering::Greater, + _ => Ordering::Equal, + } +} + +fn bool_true_last(a: bool, b: bool) -> Ordering { + match (a, b) { + (true, false) => Ordering::Greater, + (false, true) => Ordering::Less, + _ => Ordering::Equal, + } +} + +fn is_single_release_type(rel: &Release) -> bool { + if let Some(release_group) = &rel.release_group { + if let Some(primary_type) = &release_group.primary_type { + if primary_type.to_ascii_lowercase() == "single" { + return true; + } + } + } + + if rel.track_count == Some(1) { + return true; + } + if let Some(media) = &rel.media { + if media.len() == 1 && media[0].track_count == 1 { + return true; + } + let total: u32 = media.iter().map(|m| m.track_count).sum(); + if total == 1 { + return true; + } + } + false +} + +fn has_preferred_country(rel: &Release, prefs: &[&str]) -> bool { + if let Some(c) = rel.country.as_deref() { + if prefs.iter().any(|p| *p == c) { + return true; + } + } + if let Some(events) = rel.release_events.as_ref() { + for ev in events { + if let Some(area) = &ev.area { + if area + .iso_3166_1_codes + .iter() + .any(|codes| prefs.iter().any(|p| codes.contains(&p.to_string()))) + { + return true; + } + } + } + } + false +} + +/// Convert "YYYY[-MM[-DD]]" into YYYYMMDD (missing parts → 01). Unknown dates sort last. +fn date_key(d: Option<&str>) -> i32 { + if let Some(d) = d { + let mut parts = d.split('-'); + let y = parts.next().unwrap_or("9999"); + let m = parts.next().unwrap_or("01"); + let day = parts.next().unwrap_or("01"); + + let y: i32 = y.parse().unwrap_or(9999); + let m: i32 = m.parse().unwrap_or(1); + let day: i32 = day.parse().unwrap_or(1); + + return y * 10000 + m * 100 + day; + } + 9_999_01_01 +} + +fn is_live_release(rel: &Release) -> bool { + let t_live = rel.title.to_ascii_lowercase().contains("live"); + let d_live = rel + .disambiguation + .as_ref() + .map(|d| d.to_ascii_lowercase().contains("live")) + .unwrap_or(false); + t_live || d_live +} + +fn artist_credit_contains(credits: &[artist::ArtistCredit], name: &str) -> bool { + credits.iter().any(|c| c.name.eq_ignore_ascii_case(name)) +} + +#[cfg(test)] +mod tests { + use crate::musicbrainz::client::MusicbrainzClient; + use crate::musicbrainz::release::Media; + use anyhow::Error; + use serial_test::serial; + + use super::*; + + #[test] + fn test_date_key() { + assert_eq!(date_key(Some("2020-05-15")), 20200515); + assert_eq!(date_key(Some("2020-05")), 20200501); + assert_eq!(date_key(Some("2020")), 20200101); + assert_eq!(date_key(None), 99990101); + assert_eq!(date_key(Some("invalid-date")), 99990101); + } + + #[test] + fn test_is_single() { + let rel1 = Release { + track_count: Some(1), + media: None, + ..Default::default() + }; + assert!(is_single_release_type(&rel1)); + let rel2 = Release { + track_count: Some(2), + media: Some(vec![ + Media { + track_count: 1, + ..Default::default() + }, + Media { + track_count: 1, + ..Default::default() + }, + ]), + ..Default::default() + }; + assert!(!is_single_release_type(&rel2)); + } + + #[tokio::test] + #[serial] + async fn test_get_best_release_from_recordings() -> Result<(), Error> { + let client = MusicbrainzClient::new().await?; + let query = format!( + r#"recording:"{}" AND artist:"{}" AND status:Official"#, + "Smells Like Teen Spirit", "Nirvana" + ); + let recordings = client.search(&query).await?; + let best = get_best_release_from_recordings(&recordings, "Nirvana"); + assert!(best.is_some()); + let best = best.unwrap(); + assert_eq!(best.title, "Nevermind"); + assert_eq!(best.status.as_deref(), Some("Official")); + assert_eq!(best.country.as_deref(), Some("US")); + + let query = format!( + r#"recording:"{}" AND artist:"{}" AND status:Official"#, + "Medicine", "Joji" + ); + let recordings = client.search(&query).await?; + let best = get_best_release_from_recordings(&recordings, "Joji"); + assert!(best.is_some()); + let best = best.unwrap(); + assert_eq!(best.title, "Chloe Burbank Vol. 1"); + assert_eq!(best.status.as_deref(), Some("Bootleg")); + assert_eq!(best.country.as_deref(), Some("XW")); + + let query = format!( + r#"recording:"{}" AND artist:"{}" AND status:Official"#, + "Don't Stay", "Linkin Park" + ); + let recordings = client.search(&query).await?; + let best = get_best_release_from_recordings(&recordings, "Linkin Park"); + assert!(best.is_some()); + let best = best.unwrap(); + assert_eq!(best.title, "Meteora"); + assert_eq!(best.status.as_deref(), Some("Official")); + assert_eq!(best.country.as_deref(), Some("US")); + + Ok(()) + } +} diff --git a/crates/scrobbler/src/musicbrainz/release.rs b/crates/scrobbler/src/musicbrainz/release.rs index 51e5006..bffdc2c 100644 --- a/crates/scrobbler/src/musicbrainz/release.rs +++ b/crates/scrobbler/src/musicbrainz/release.rs @@ -6,7 +6,7 @@ use super::{ recording::Recording, }; -#[derive(Debug, Deserialize, Clone)] +#[derive(Debug, Deserialize, Clone, Default)] pub struct Release { #[serde(rename = "release-events")] pub release_events: Option>, @@ -24,7 +24,7 @@ pub struct Release { #[serde(rename = "cover-art-archive")] pub cover_art_archive: Option, #[serde(rename = "artist-credit")] - pub artist_credit: Vec, + pub artist_credit: Option>, #[serde(rename = "status-id")] pub status_id: Option, #[serde(rename = "label-info")] @@ -33,9 +33,13 @@ pub struct Release { pub date: Option, pub country: Option, pub asin: Option, + #[serde(rename = "track-count")] + pub track_count: Option, + #[serde(rename = "release-group")] + pub release_group: Option, } -#[derive(Debug, Deserialize, Clone)] +#[derive(Debug, Deserialize, Clone, Default)] pub struct CoverArtArchive { pub back: bool, pub artwork: bool, @@ -44,19 +48,19 @@ pub struct CoverArtArchive { pub darkened: bool, } -#[derive(Debug, Deserialize, Clone)] +#[derive(Debug, Deserialize, Clone, Default)] pub struct ReleaseEvent { pub area: Option, pub date: String, } -#[derive(Debug, Deserialize, Clone)] +#[derive(Debug, Deserialize, Clone, Default)] pub struct TextRepresentation { pub language: Option, pub script: Option, } -#[derive(Debug, Deserialize, Clone)] +#[derive(Debug, Deserialize, Clone, Default)] pub struct Media { #[serde(rename = "format-id")] pub format_id: Option, @@ -87,6 +91,21 @@ pub struct Track { pub title: String, pub recording: Recording, #[serde(rename = "artist-credit")] - pub artist_credit: Vec, + pub artist_credit: Option>, pub number: String, } + +#[derive(Debug, Deserialize, Clone, Default)] +pub struct ReleaseGroup { + pub id: String, + pub title: String, + #[serde(rename = "primary-type")] + pub primary_type: Option, + #[serde(rename = "secondary-types")] + pub secondary_types: Option>, + pub disambiguation: Option, + #[serde(rename = "first-release-date")] + pub first_release_date: Option, + #[serde(rename = "artist-credit")] + pub artist_credit: Option>, +} diff --git a/crates/scrobbler/src/scrobbler.rs b/crates/scrobbler/src/scrobbler.rs index 729586a..9e34f36 100644 --- a/crates/scrobbler/src/scrobbler.rs +++ b/crates/scrobbler/src/scrobbler.rs @@ -9,7 +9,9 @@ use crate::{ cache::Cache, crypto::decrypt_aes_256_ctr, listenbrainz::types::SubmitListensRequest, - musicbrainz::client::MusicbrainzClient, + musicbrainz::{ + client::MusicbrainzClient, get_best_release_from_recordings, recording::Recording, + }, repo, rocksky, spotify::{client::SpotifyClient, refresh_token}, types::{Scrobble, Track}, @@ -94,6 +96,7 @@ fn parse_batch(form: &BTreeMap) -> Result, Error> pub async fn scrobble( pool: &Pool, cache: &Cache, + mb_client: &MusicbrainzClient, form: &BTreeMap, ) -> Result, Error> { let mut scrobbles = parse_batch(form)?; @@ -110,8 +113,6 @@ pub async fn scrobble( return Err(Error::msg("No Spotify tokens found")); } - let mb_client = MusicbrainzClient::new(); - for scrobble in &mut scrobbles { /* 0. check if scrobble is cached @@ -224,7 +225,7 @@ pub async fn scrobble( } let query = format!( - r#"recording:"{}" AND artist:"{}""#, + r#"recording:"{}" AND artist:"{}" AND status:Official"#, scrobble.track, scrobble.artist ); let result = mb_client.search(&query).await?; @@ -248,6 +249,7 @@ pub async fn scrobble( pub async fn scrobble_v1( pool: &Pool, cache: &Cache, + mb_client: &MusicbrainzClient, form: &BTreeMap, ) -> Result<(), Error> { let session_id = form.get("s").unwrap().to_string(); @@ -269,8 +271,6 @@ pub async fn scrobble_v1( return Err(Error::msg("No Spotify tokens found")); } - let mb_client = MusicbrainzClient::new(); - let mut scrobble = Scrobble { artist: artist.trim().to_string(), track: track.trim().to_string(), @@ -399,12 +399,16 @@ pub async fn scrobble_v1( } let query = format!( - r#"recording:"{}" AND artist:"{}""#, + r#"recording:"{}" AND artist:"{}" AND status:Official"#, scrobble.track, scrobble.artist ); - let result = mb_client.search(&query).await?; - - if let Some(recording) = result.recordings.first() { + let result = search_musicbrainz_recording(&query, mb_client, &scrobble).await; + if let Err(e) = result { + tracing::warn!(artist = %scrobble.artist, track = %scrobble.track, "Musicbrainz search error: {}", e); + return Ok(()); + } + let result = result.unwrap(); + if let Some(recording) = result { let result = mb_client.get_recording(&recording.id).await?; tracing::info!(%scrobble.artist, %scrobble.track, "Musicbrainz (recording)"); scrobble.album = Some(Track::from(result.clone()).album); @@ -421,6 +425,7 @@ pub async fn scrobble_v1( pub async fn scrobble_listenbrainz( pool: &Pool, cache: &Cache, + mb_client: &MusicbrainzClient, req: &SubmitListensRequest, token: &str, ) -> Result<(), Error> { @@ -503,8 +508,6 @@ pub async fn scrobble_listenbrainz( return Err(Error::msg("No Spotify tokens found")); } - let mb_client = MusicbrainzClient::new(); - let mut scrobble = Scrobble { artist: artist.trim().to_string(), track: track.trim().to_string(), @@ -620,7 +623,7 @@ pub async fn scrobble_listenbrainz( // check if artists don't contain the scrobble artist (to avoid wrong matches) if !artists.contains(&scrobble.artist.to_lowercase()) { - tracing::warn!(artist = %artist, track = %track, "Artist mismatch, skipping"); + tracing::warn!(artist = %artist, track = ?track, "Artist mismatch, skipping"); return Ok(()); } @@ -644,27 +647,58 @@ pub async fn scrobble_listenbrainz( return Ok(()); } - // Temporary disable Musicbrainz search to reduce rate limiting issues - // and because it often returns wrong results - // we can re-enable it later with a retry mechanism - /* let query = format!( - r#"recording:"{}" AND artist:"{}""#, + r#"recording:"{}" AND artist:"{}" AND status:Official"#, scrobble.track, scrobble.artist ); - let result = mb_client.search(&query).await?; - - if let Some(recording) = result.recordings.first() { - let result = mb_client.get_recording(&recording.id).await?; - println!("{}", "Musicbrainz (recording)".yellow()); - scrobble.album = Some(Track::from(result.clone()).album); + let result = search_musicbrainz_recording(&query, mb_client, &scrobble).await; + if let Err(e) = result { + tracing::warn!(artist = %artist, track = %track, "Musicbrainz search error: {}", e); + return Ok(()); + } + let result = result.unwrap(); + if let Some(result) = result { + tracing::info!("Musicbrainz (recording)"); rocksky::scrobble(cache, &did, result.into(), scrobble.timestamp).await?; tokio::time::sleep(std::time::Duration::from_secs(1)).await; return Ok(()); } - */ tracing::warn!(artist = %artist, track = %track, "Track not found, skipping"); Ok(()) } + +async fn search_musicbrainz_recording( + query: &str, + mb_client: &MusicbrainzClient, + scrobble: &Scrobble, +) -> Result, Error> { + let result = mb_client.search(&query).await; + if let Err(e) = result { + tracing::warn!(artist = %scrobble.artist, track = %scrobble.track, "Musicbrainz search error: {}", e); + return Ok(None); + } + let result = result.unwrap(); + + let release = get_best_release_from_recordings(&result, &scrobble.artist); + + if let Some(release) = release { + let recording = result.recordings.into_iter().find(|r| { + r.releases + .as_ref() + .map(|releases| releases.iter().any(|rel| rel.id == release.id)) + .unwrap_or(false) + }); + if recording.is_none() { + tracing::warn!(artist = %scrobble.artist, track = %scrobble.track, "Recording not found in MusicBrainz result, skipping"); + return Ok(None); + } + let recording = recording.unwrap(); + let result = mb_client.get_recording(&recording.id).await?; + tracing::info!("Musicbrainz (recording)"); + return Ok(Some(result)); + } + + Ok(None) +} diff --git a/crates/scrobbler/src/types.rs b/crates/scrobbler/src/types.rs index ef15e76..78bbb00 100644 --- a/crates/scrobbler/src/types.rs +++ b/crates/scrobbler/src/types.rs @@ -69,10 +69,10 @@ impl From for Track { .map(|credit| credit.name.clone()) .unwrap_or_default(); let releases = recording.releases.unwrap_or_default(); - let album_artist = releases - .first() - .and_then(|release| release.artist_credit.first()) - .map(|credit| credit.name.clone()); + let album_artist = releases.first().and_then(|release| { + let credits = release.artist_credit.clone().unwrap_or_default(); + credits.first().map(|credit| credit.name.clone()) + }); let album = releases .first() .map(|release| release.title.clone()) diff --git a/crates/webscrobbler/Cargo.toml b/crates/webscrobbler/Cargo.toml index 9610b13..18a61cd 100644 --- a/crates/webscrobbler/Cargo.toml +++ b/crates/webscrobbler/Cargo.toml @@ -39,3 +39,7 @@ tokio-stream = { version = "0.1.17", features = ["full"] } actix-session = "0.10.1" actix-limitation = "0.5.1" tracing = "0.1.41" +nanoid = "0.4.0" + +[dev-dependencies] +serial_test = "3.0.0" diff --git a/crates/webscrobbler/src/handlers.rs b/crates/webscrobbler/src/handlers.rs index 0620e98..94067a9 100644 --- a/crates/webscrobbler/src/handlers.rs +++ b/crates/webscrobbler/src/handlers.rs @@ -1,4 +1,7 @@ -use crate::{cache::Cache, consts::BANNER, repo, scrobbler::scrobble, types::ScrobbleRequest}; +use crate::{ + cache::Cache, consts::BANNER, musicbrainz::client::MusicbrainzClient, repo, + scrobbler::scrobble, types::ScrobbleRequest, +}; use actix_web::{get, post, web, HttpRequest, HttpResponse, Responder}; use owo_colors::OwoColorize; use sqlx::{Pool, Postgres}; @@ -28,6 +31,7 @@ pub async fn index() -> impl Responder { async fn handle_scrobble( data: web::Data>>, cache: web::Data, + mb_client: web::Data>, mut payload: web::Payload, req: HttpRequest, ) -> Result { @@ -100,7 +104,8 @@ async fn handle_scrobble( } } - scrobble(&pool, &cache, params, &user.did) + let mb_client = mb_client.get_ref().as_ref(); + scrobble(&pool, &cache, mb_client, params, &user.did) .await .map_err(|err| { actix_web::error::ErrorInternalServerError(format!("Failed to scrobble: {}", err)) diff --git a/crates/webscrobbler/src/lib.rs b/crates/webscrobbler/src/lib.rs index 4127290..1a8e295 100644 --- a/crates/webscrobbler/src/lib.rs +++ b/crates/webscrobbler/src/lib.rs @@ -11,7 +11,7 @@ use anyhow::Error; use owo_colors::OwoColorize; use sqlx::postgres::PgPoolOptions; -use crate::{cache::Cache, consts::BANNER}; +use crate::{cache::Cache, consts::BANNER, musicbrainz::client::MusicbrainzClient}; pub mod auth; pub mod cache; @@ -38,6 +38,9 @@ pub async fn start_server() -> Result<(), Error> { let conn = Arc::new(pool); + let mb_client = MusicbrainzClient::new().await?; + let mb_client = Arc::new(mb_client); + let host = env::var("WEBSCROBBLER_HOST").unwrap_or_else(|_| "127.0.0.1".to_string()); let port = env::var("WEBSCROBBLER_PORT") .unwrap_or_else(|_| "7883".to_string()) @@ -65,6 +68,7 @@ pub async fn start_server() -> Result<(), Error> { .app_data(limiter.clone()) .app_data(Data::new(conn.clone())) .app_data(Data::new(cache.clone())) + .app_data(Data::new(mb_client.clone())) .service(handlers::index) .service(handlers::handle_scrobble) }) diff --git a/crates/webscrobbler/src/musicbrainz/artist.rs b/crates/webscrobbler/src/musicbrainz/artist.rs index ed759b3..fda5bbc 100644 --- a/crates/webscrobbler/src/musicbrainz/artist.rs +++ b/crates/webscrobbler/src/musicbrainz/artist.rs @@ -1,6 +1,6 @@ use serde::{Deserialize, Serialize}; -#[derive(Debug, Deserialize, Clone)] +#[derive(Debug, Deserialize, Clone, Default)] pub struct Artist { pub name: String, #[serde(rename = "sort-name")] @@ -29,7 +29,7 @@ pub struct Artist { pub aliases: Option>, } -#[derive(Debug, Deserialize, Clone)] +#[derive(Debug, Deserialize, Clone, Default)] pub struct ArtistCredit { pub joinphrase: Option, pub name: String, diff --git a/crates/webscrobbler/src/musicbrainz/client.rs b/crates/webscrobbler/src/musicbrainz/client.rs index f68ef51..2c6c68e 100644 --- a/crates/webscrobbler/src/musicbrainz/client.rs +++ b/crates/webscrobbler/src/musicbrainz/client.rs @@ -1,41 +1,327 @@ +use std::env; + use super::recording::{Recording, Recordings}; -use anyhow::Error; +use anyhow::{anyhow, Context, Error}; +use redis::aio::MultiplexedConnection; +use redis::AsyncCommands; +use serde::{Deserialize, Serialize}; +use tokio::time::{timeout, Duration, Instant}; pub const BASE_URL: &str = "https://musicbrainz.org/ws/2"; -pub const USER_AGENT: &str = "Rocksky/0.1.0"; +pub const USER_AGENT: &str = "Rocksky/0.1.0 (+https://rocksky.app)"; + +const Q_QUEUE: &str = "mb:queue:v1"; +const CACHE_SEARCH_PREFIX: &str = "mb:cache:search:"; +const CACHE_REC_PREFIX: &str = "mb:cache:rec:"; +const INFLIGHT_PREFIX: &str = "mb:inflight:"; +const RESP_PREFIX: &str = "mb:resp:"; -pub struct MusicbrainzClient {} +const CACHE_TTL_SECS: u64 = 60 * 60 * 24; +const WAIT_TIMEOUT_SECS: u64 = 20; +const INFLIGHT_TTL_SECS: i64 = 30; // de-dup window while the worker is fetching + +#[derive(Clone)] +pub struct MusicbrainzClient { + http: reqwest::Client, + redis: MultiplexedConnection, + cache_ttl: u64, +} + +#[derive(Debug, Serialize, Deserialize)] +#[serde(tag = "kind", rename_all = "snake_case")] +enum Job { + Search { id: String, query: String }, + GetRecording { id: String, mbid: String }, +} impl MusicbrainzClient { - pub fn new() -> Self { - MusicbrainzClient {} + pub async fn new() -> Result { + let client = + redis::Client::open(env::var("REDIS_URL").unwrap_or("redis://127.0.0.1".into()))?; + let redis = client.get_multiplexed_tokio_connection().await?; + let http = reqwest::Client::builder() + .user_agent(USER_AGENT) + .build() + .context("build http client")?; + let me = MusicbrainzClient { + http, + redis, + cache_ttl: CACHE_TTL_SECS, + }; + + let mut worker_conn = client.get_multiplexed_async_connection().await?; + + let http = me.http.clone(); + tokio::spawn(async move { worker_loop(http, &mut worker_conn).await }); + + Ok(me) } pub async fn search(&self, query: &str) -> Result { - let url = format!("{}/recording", BASE_URL); - let client = reqwest::Client::new(); - let response = client - .get(&url) - .header("Accept", "application/json") - .header("User-Agent", USER_AGENT) - .query(&[("query", query), ("inc", "artist-credits+releases")]) - .send() + if let Some(h) = self.get_cache(&cache_key_search(query)).await? { + return Ok(serde_json::from_str(&h).context("decode cached search")?); + } + let id = nanoid::nanoid!(); + let job = Job::Search { + id: id.clone(), + query: query.to_string(), + }; + self.enqueue_if_needed(&job, &infl_key_search(query)) .await?; - Ok(response.json().await?) + let raw = self.wait_for_response(&id).await?; + let parsed: Recordings = serde_json::from_str(&raw).context("decode search response")?; + + self.set_cache(&cache_key_search(query), &raw).await?; + Ok(parsed) } pub async fn get_recording(&self, mbid: &str) -> Result { - let url = format!("{}/recording/{}", BASE_URL, mbid); - let client = reqwest::Client::new(); - let response = client - .get(&url) - .header("Accept", "application/json") - .header("User-Agent", USER_AGENT) - .query(&[("inc", "artist-credits+releases")]) - .send() - .await?; + if let Some(h) = self.get_cache(&cache_key_rec(mbid)).await? { + return Ok(serde_json::from_str(&h).context("decode cached recording")?); + } + let id = nanoid::nanoid!(); + let job = Job::GetRecording { + id: id.clone(), + mbid: mbid.to_string(), + }; + self.enqueue_if_needed(&job, &infl_key_rec(mbid)).await?; + let raw = self.wait_for_response(&id).await?; + let parsed: Recording = serde_json::from_str(&raw).context("decode recording response")?; + self.set_cache(&cache_key_rec(mbid), &raw).await?; + Ok(parsed) + } + + // ---------- Redis helpers ---------- + + async fn get_cache(&self, key: &str) -> Result, Error> { + let mut r = self.redis.clone(); + let val: Option = r.get(key).await?; + Ok(val) + } + + async fn set_cache(&self, key: &str, json: &str) -> Result<(), Error> { + let mut r = self.redis.clone(); + let _: () = r + .set_ex(key, json, self.cache_ttl) + .await + .with_context(|| format!("cache set {key}"))?; + Ok(()) + } + + async fn enqueue_if_needed(&self, job: &Job, inflight_key: &str) -> Result<(), Error> { + let mut r = self.redis.clone(); + + // set NX to avoid duplicate work; short TTL + let set: bool = r.set_nx(inflight_key, "1").await.context("set in-flight")?; + if set { + let _: () = r + .expire(inflight_key, INFLIGHT_TTL_SECS) + .await + .context("expire inflight")?; + let payload = serde_json::to_string(job).expect("serialize job"); + let _: () = r.rpush(Q_QUEUE, payload).await.context("enqueue job")?; + } + Ok(()) + } + + async fn wait_for_response(&self, id: &str) -> Result { + let mut r = self.redis.clone(); + let resp_q = resp_key(id); + + let fut = async { + loop { + let popped: Option<(String, String)> = r.brpop(&resp_q, 2.0).await?; + if let Some((_key, json)) = popped { + return Ok::(json); + } + } + }; + + match timeout(Duration::from_secs(WAIT_TIMEOUT_SECS), fut).await { + Ok(res) => res, + Err(_) => Err(anyhow!("timed out waiting for MusicBrainz response")), + } + } +} + +async fn worker_loop( + http: reqwest::Client, + redis: &mut MultiplexedConnection, +) -> Result<(), Error> { + // pacing ticker: strictly 1 request/second + let mut next_allowed = Instant::now(); + + loop { + tokio::select! { + res = async { + + // finite timeout pop + let v: Option> = redis.blpop(Q_QUEUE, 2.0).await.ok(); + Ok::<_, Error>(v) + } => { + let Some(mut v) = res? else { + continue }; + if v.len() != 2 { + continue; } + let payload = v.pop().unwrap(); + + // 1 rps pacing + let now = Instant::now(); + if now < next_allowed { tokio::time::sleep(next_allowed - now).await; } + next_allowed = Instant::now() + Duration::from_secs(1); + + let payload: Job = match serde_json::from_str(&payload) { + Ok(j) => j, + Err(e) => { + tracing::error!(%e, "invalid job payload"); + continue; + } + }; + if let Err(e) = process_job(&http, redis, payload).await { + tracing::error!(%e, "job failed"); + } + } + } + } +} + +async fn process_job( + http: &reqwest::Client, + redis: &mut MultiplexedConnection, + job: Job, +) -> Result<(), Error> { + match job { + Job::Search { id, query } => { + let url = format!("{}/recording", BASE_URL); + let resp = http + .get(&url) + .header("Accept", "application/json") + .query(&[ + ("query", query.as_str()), + ("fmt", "json"), + ("inc", "artists+releases+isrcs"), + ]) + .send() + .await + .context("http search")?; + + if !resp.status().is_success() { + // Push an error payload so waiters don’t hang forever + let _ = push_response( + redis, + &id, + &format!(r#"{{"error":"http {}"}}"#, resp.status()), + ) + .await; + return Err(anyhow!("musicbrainz search http {}", resp.status())); + } + + let text = resp.text().await.context("read body")?; + push_response(redis, &id, &text).await?; + } + Job::GetRecording { id, mbid } => { + let url = format!("{}/recording/{}", BASE_URL, mbid); + let resp = http + .get(&url) + .header("Accept", "application/json") + .query(&[("fmt", "json"), ("inc", "artists+releases+isrcs")]) + .send() + .await + .context("http get_recording")?; + + if !resp.status().is_success() { + let _ = push_response( + redis, + &id, + &format!(r#"{{"error":"http {}"}}"#, resp.status()), + ) + .await; + return Err(anyhow!("musicbrainz get_recording http {}", resp.status())); + } + + let text = resp.text().await.context("read body")?; + push_response(redis, &id, &text).await?; + } + } + Ok(()) +} + +async fn push_response( + redis: &mut MultiplexedConnection, + id: &str, + json: &str, +) -> Result<(), Error> { + let q = resp_key(id); + // RPUSH then EXPIRE to avoid leaks if a client never BRPOPs + let _: () = redis.rpush(&q, json).await?; + let _: () = redis.expire(&q, WAIT_TIMEOUT_SECS as i64 + 5).await?; + Ok(()) +} + +fn cache_key_search(query: &str) -> String { + format!("{}{}", CACHE_SEARCH_PREFIX, fast_hash(query)) +} +fn cache_key_rec(mbid: &str) -> String { + format!("{}{}", CACHE_REC_PREFIX, mbid) +} +fn infl_key_search(query: &str) -> String { + format!("{}search:{}", INFLIGHT_PREFIX, fast_hash(query)) +} +fn infl_key_rec(mbid: &str) -> String { + format!("{}rec:{}", INFLIGHT_PREFIX, mbid) +} +fn resp_key(id: &str) -> String { + format!("{}{}", RESP_PREFIX, id) +} + +fn fast_hash(s: &str) -> u64 { + use std::hash::{Hash, Hasher}; + let mut h = std::collections::hash_map::DefaultHasher::new(); + s.hash(&mut h); + h.finish() +} + +#[cfg(test)] +mod tests { + use super::*; + use serial_test::serial; + + #[test] + fn test_fast_hash() { + let h1 = fast_hash("hello"); + let h2 = fast_hash("hello"); + let h3 = fast_hash("world"); + assert_eq!(h1, h2); + assert_ne!(h1, h3); + } + + #[test] + fn test_cache_keys() { + let q = "test query"; + let mbid = "some-mbid"; + assert!(cache_key_search(q).starts_with(CACHE_SEARCH_PREFIX)); + assert!(cache_key_rec(mbid).starts_with(CACHE_REC_PREFIX)); + assert!(infl_key_search(q).starts_with(INFLIGHT_PREFIX)); + assert!(infl_key_rec(mbid).starts_with(INFLIGHT_PREFIX)); + assert!(resp_key("id").starts_with(RESP_PREFIX)); + } + + #[tokio::test] + #[serial] + async fn test_musicbrainz_client() -> Result<(), Error> { + let client = MusicbrainzClient::new().await?; + let query = format!( + r#"recording:"{}" AND artist:"{}" AND status:Official"#, + "Come As You Are", "Nirvana" + ); + let search_res = client.search(&query).await?; - Ok(response.json().await?) + assert!(!search_res.recordings.is_empty()); + let rec = &search_res.recordings[0]; + let mbid = &rec.id; + let rec_res = client.get_recording(mbid).await?; + assert_eq!(rec_res.id, *mbid); + Ok(()) } } diff --git a/crates/webscrobbler/src/musicbrainz/mod.rs b/crates/webscrobbler/src/musicbrainz/mod.rs index acceccd..13f1ef7 100644 --- a/crates/webscrobbler/src/musicbrainz/mod.rs +++ b/crates/webscrobbler/src/musicbrainz/mod.rs @@ -1,5 +1,280 @@ +use crate::musicbrainz::{recording::Recordings, release::Release}; +use std::cmp::Ordering; + pub mod artist; pub mod client; pub mod label; pub mod recording; pub mod release; + +fn get_best_release(releases: &[Release]) -> Option { + if releases.is_empty() { + return None; + } + + let mut candidates: Vec<&Release> = releases.iter().collect(); + + if candidates.is_empty() { + return None; + } + + candidates.sort_by(|a, b| cmp_release(a, b)); + candidates.first().cloned().cloned() +} + +pub fn get_best_release_from_recordings(all: &Recordings, artist: &str) -> Option { + use std::collections::HashSet; + + let mut pool: Vec = Vec::new(); + let mut seen: HashSet = HashSet::new(); + + let all_recordings: Vec<&recording::Recording> = all + .recordings + .iter() + .filter(|rec| { + if let Some(credits) = &rec.artist_credit { + artist_credit_contains(credits, artist) + } else { + false + } + }) + .collect(); + + for rec in &all_recordings { + if let Some(rels) = &rec.releases { + for r in rels { + if seen.insert(r.id.clone()) { + pool.push(r.clone()); + } + } + } + } + + get_best_release(&pool) +} + +fn cmp_release(a: &Release, b: &Release) -> Ordering { + // First priority: prefer albums over singles + let sa = is_single_release_type(a); + let sb = is_single_release_type(b); + if sa != sb { + return bool_true_last(sa, sb); // Albums (false) come before singles (true) + } + + let ta = release_tier(a.status.as_deref()); + let tb = release_tier(b.status.as_deref()); + if ta != tb { + return ta.cmp(&tb); + } + + let pa = has_preferred_country(a, &["XW", "US"]); + let pb = has_preferred_country(b, &["XW", "US"]); + if pa != pb { + return bool_true_first(pa, pb); + } + + let la = is_live_release(a); + let lb = is_live_release(b); + if la != lb { + return bool_true_last(la, lb); + } + + let da = date_key(a.date.as_deref()); + let db = date_key(b.date.as_deref()); + if da != db { + return da.cmp(&db); + } + + match a.title.cmp(&b.title) { + Ordering::Equal => a.id.cmp(&b.id), + ord => ord, + } +} + +fn release_tier(status: Option<&str>) -> u8 { + match status.map(|s| s.to_ascii_lowercase()) { + Some(s) if s == "official" => 0, + Some(s) if s == "bootleg" => 1, + _ => 2, + } +} + +fn bool_true_first(a: bool, b: bool) -> Ordering { + match (a, b) { + (true, false) => Ordering::Less, + (false, true) => Ordering::Greater, + _ => Ordering::Equal, + } +} + +fn bool_true_last(a: bool, b: bool) -> Ordering { + match (a, b) { + (true, false) => Ordering::Greater, + (false, true) => Ordering::Less, + _ => Ordering::Equal, + } +} + +fn is_single_release_type(rel: &Release) -> bool { + if let Some(release_group) = &rel.release_group { + if let Some(primary_type) = &release_group.primary_type { + if primary_type.to_ascii_lowercase() == "single" { + return true; + } + } + } + + if rel.track_count == Some(1) { + return true; + } + if let Some(media) = &rel.media { + if media.len() == 1 && media[0].track_count == 1 { + return true; + } + let total: u32 = media.iter().map(|m| m.track_count).sum(); + if total == 1 { + return true; + } + } + false +} + +fn has_preferred_country(rel: &Release, prefs: &[&str]) -> bool { + if let Some(c) = rel.country.as_deref() { + if prefs.iter().any(|p| *p == c) { + return true; + } + } + if let Some(events) = rel.release_events.as_ref() { + for ev in events { + if let Some(area) = &ev.area { + if area + .iso_3166_1_codes + .iter() + .any(|codes| prefs.iter().any(|p| codes.contains(&p.to_string()))) + { + return true; + } + } + } + } + false +} + +/// Convert "YYYY[-MM[-DD]]" into YYYYMMDD (missing parts → 01). Unknown dates sort last. +fn date_key(d: Option<&str>) -> i32 { + if let Some(d) = d { + let mut parts = d.split('-'); + let y = parts.next().unwrap_or("9999"); + let m = parts.next().unwrap_or("01"); + let day = parts.next().unwrap_or("01"); + + let y: i32 = y.parse().unwrap_or(9999); + let m: i32 = m.parse().unwrap_or(1); + let day: i32 = day.parse().unwrap_or(1); + + return y * 10000 + m * 100 + day; + } + 9_999_01_01 +} + +fn is_live_release(rel: &Release) -> bool { + let t_live = rel.title.to_ascii_lowercase().contains("live"); + let d_live = rel + .disambiguation + .as_ref() + .map(|d| d.to_ascii_lowercase().contains("live")) + .unwrap_or(false); + t_live || d_live +} + +fn artist_credit_contains(credits: &[artist::ArtistCredit], name: &str) -> bool { + credits.iter().any(|c| c.name.eq_ignore_ascii_case(name)) +} + +#[cfg(test)] +mod tests { + use crate::musicbrainz::client::MusicbrainzClient; + use crate::musicbrainz::release::Media; + use anyhow::Error; + use serial_test::serial; + + use super::*; + + #[test] + fn test_date_key() { + assert_eq!(date_key(Some("2020-05-15")), 20200515); + assert_eq!(date_key(Some("2020-05")), 20200501); + assert_eq!(date_key(Some("2020")), 20200101); + assert_eq!(date_key(None), 99990101); + assert_eq!(date_key(Some("invalid-date")), 99990101); + } + + #[test] + fn test_is_single() { + let rel1 = Release { + track_count: Some(1), + media: None, + ..Default::default() + }; + assert!(is_single_release_type(&rel1)); + let rel2 = Release { + track_count: Some(2), + media: Some(vec![ + Media { + track_count: 1, + ..Default::default() + }, + Media { + track_count: 1, + ..Default::default() + }, + ]), + ..Default::default() + }; + assert!(!is_single_release_type(&rel2)); + } + + #[tokio::test] + #[serial] + async fn test_get_best_release_from_recordings() -> Result<(), Error> { + let client = MusicbrainzClient::new().await?; + let query = format!( + r#"recording:"{}" AND artist:"{}" AND status:Official"#, + "Smells Like Teen Spirit", "Nirvana" + ); + let recordings = client.search(&query).await?; + let best = get_best_release_from_recordings(&recordings, "Nirvana"); + assert!(best.is_some()); + let best = best.unwrap(); + assert_eq!(best.title, "Nevermind"); + assert_eq!(best.status.as_deref(), Some("Official")); + assert_eq!(best.country.as_deref(), Some("US")); + + let query = format!( + r#"recording:"{}" AND artist:"{}" AND status:Official"#, + "Medicine", "Joji" + ); + let recordings = client.search(&query).await?; + let best = get_best_release_from_recordings(&recordings, "Joji"); + assert!(best.is_some()); + let best = best.unwrap(); + assert_eq!(best.title, "Chloe Burbank Vol. 1"); + assert_eq!(best.status.as_deref(), Some("Bootleg")); + assert_eq!(best.country.as_deref(), Some("XW")); + + let query = format!( + r#"recording:"{}" AND artist:"{}" AND status:Official"#, + "Don't Stay", "Linkin Park" + ); + let recordings = client.search(&query).await?; + let best = get_best_release_from_recordings(&recordings, "Linkin Park"); + assert!(best.is_some()); + let best = best.unwrap(); + assert_eq!(best.title, "Meteora"); + assert_eq!(best.status.as_deref(), Some("Official")); + assert_eq!(best.country.as_deref(), Some("US")); + + Ok(()) + } +} diff --git a/crates/webscrobbler/src/musicbrainz/release.rs b/crates/webscrobbler/src/musicbrainz/release.rs index 51e5006..bffdc2c 100644 --- a/crates/webscrobbler/src/musicbrainz/release.rs +++ b/crates/webscrobbler/src/musicbrainz/release.rs @@ -6,7 +6,7 @@ use super::{ recording::Recording, }; -#[derive(Debug, Deserialize, Clone)] +#[derive(Debug, Deserialize, Clone, Default)] pub struct Release { #[serde(rename = "release-events")] pub release_events: Option>, @@ -24,7 +24,7 @@ pub struct Release { #[serde(rename = "cover-art-archive")] pub cover_art_archive: Option, #[serde(rename = "artist-credit")] - pub artist_credit: Vec, + pub artist_credit: Option>, #[serde(rename = "status-id")] pub status_id: Option, #[serde(rename = "label-info")] @@ -33,9 +33,13 @@ pub struct Release { pub date: Option, pub country: Option, pub asin: Option, + #[serde(rename = "track-count")] + pub track_count: Option, + #[serde(rename = "release-group")] + pub release_group: Option, } -#[derive(Debug, Deserialize, Clone)] +#[derive(Debug, Deserialize, Clone, Default)] pub struct CoverArtArchive { pub back: bool, pub artwork: bool, @@ -44,19 +48,19 @@ pub struct CoverArtArchive { pub darkened: bool, } -#[derive(Debug, Deserialize, Clone)] +#[derive(Debug, Deserialize, Clone, Default)] pub struct ReleaseEvent { pub area: Option, pub date: String, } -#[derive(Debug, Deserialize, Clone)] +#[derive(Debug, Deserialize, Clone, Default)] pub struct TextRepresentation { pub language: Option, pub script: Option, } -#[derive(Debug, Deserialize, Clone)] +#[derive(Debug, Deserialize, Clone, Default)] pub struct Media { #[serde(rename = "format-id")] pub format_id: Option, @@ -87,6 +91,21 @@ pub struct Track { pub title: String, pub recording: Recording, #[serde(rename = "artist-credit")] - pub artist_credit: Vec, + pub artist_credit: Option>, pub number: String, } + +#[derive(Debug, Deserialize, Clone, Default)] +pub struct ReleaseGroup { + pub id: String, + pub title: String, + #[serde(rename = "primary-type")] + pub primary_type: Option, + #[serde(rename = "secondary-types")] + pub secondary_types: Option>, + pub disambiguation: Option, + #[serde(rename = "first-release-date")] + pub first_release_date: Option, + #[serde(rename = "artist-credit")] + pub artist_credit: Option>, +} diff --git a/crates/webscrobbler/src/scrobbler.rs b/crates/webscrobbler/src/scrobbler.rs index 700309f..af8b43d 100644 --- a/crates/webscrobbler/src/scrobbler.rs +++ b/crates/webscrobbler/src/scrobbler.rs @@ -3,6 +3,8 @@ use std::env; use crate::cache::Cache; use crate::crypto::decrypt_aes_256_ctr; use crate::musicbrainz::client::MusicbrainzClient; +use crate::musicbrainz::get_best_release_from_recordings; +use crate::musicbrainz::recording::Recording; use crate::spotify::client::SpotifyClient; use crate::spotify::refresh_token; use crate::types::{ScrobbleRequest, Track}; @@ -15,6 +17,7 @@ use sqlx::{Pool, Postgres}; pub async fn scrobble( pool: &Pool, cache: &Cache, + mb_client: &MusicbrainzClient, scrobble: ScrobbleRequest, did: &str, ) -> Result<(), Error> { @@ -24,8 +27,6 @@ pub async fn scrobble( return Err(Error::msg("No Spotify tokens found")); } - let mb_client = MusicbrainzClient::new(); - let key = format!( "{} - {}", scrobble.data.song.parsed.artist.to_lowercase(), @@ -127,6 +128,20 @@ pub async fn scrobble( let result = spotify_client.search(&query).await?; if let Some(track) = result.tracks.items.first() { + let artists = track + .artists + .iter() + .map(|a| a.name.clone()) + .collect::>() + .join(", ") + .to_lowercase(); + let artist = scrobble.data.song.parsed.artist.trim(); + // check if artists don't contain the scrobble artist (to avoid wrong matches) + if !artists.contains(&scrobble.data.song.parsed.artist.trim().to_lowercase()) { + tracing::warn!(artist = %artist, track = ?track, "Artist mismatch, skipping"); + return Ok(()); + } + tracing::info!("Spotify (track)"); let mut track = track.clone(); @@ -147,13 +162,17 @@ pub async fn scrobble( } let query = format!( - r#"recording:"{}" AND artist:"{}""#, + r#"recording:"{}" AND artist:"{}" AND status:Official"#, scrobble.data.song.parsed.track, scrobble.data.song.parsed.artist ); - let result = mb_client.search(&query).await?; - if let Some(recording) = result.recordings.first() { - let result = mb_client.get_recording(&recording.id).await?; + let result = search_musicbrainz_recording(&query, mb_client, &scrobble).await; + if let Err(e) = result { + tracing::warn!(artist = %scrobble.data.song.parsed.artist, track = %scrobble.data.song.parsed.track, "Musicbrainz search error: {}", e); + return Ok(()); + } + let result = result.unwrap(); + if let Some(result) = result { tracing::info!("Musicbrainz (recording)"); rocksky::scrobble(cache, &did, result.into(), scrobble.time).await?; tokio::time::sleep(std::time::Duration::from_secs(1)).await; @@ -164,3 +183,37 @@ pub async fn scrobble( Ok(()) } + +async fn search_musicbrainz_recording( + query: &str, + mb_client: &MusicbrainzClient, + scrobble: &ScrobbleRequest, +) -> Result, Error> { + let result = mb_client.search(&query).await; + if let Err(e) = result { + tracing::warn!(artist = %scrobble.data.song.parsed.artist, track = %scrobble.data.song.parsed.track, "Musicbrainz search error: {}", e); + return Ok(None); + } + let result = result.unwrap(); + + let release = get_best_release_from_recordings(&result, &scrobble.data.song.parsed.artist); + + if let Some(release) = release { + let recording = result.recordings.into_iter().find(|r| { + r.releases + .as_ref() + .map(|releases| releases.iter().any(|rel| rel.id == release.id)) + .unwrap_or(false) + }); + if recording.is_none() { + tracing::warn!(artist = %scrobble.data.song.parsed.artist, track = %scrobble.data.song.parsed.track, "Recording not found in MusicBrainz result, skipping"); + return Ok(None); + } + let recording = recording.unwrap(); + let result = mb_client.get_recording(&recording.id).await?; + tracing::info!("Musicbrainz (recording)"); + return Ok(Some(result)); + } + + Ok(None) +} diff --git a/crates/webscrobbler/src/types.rs b/crates/webscrobbler/src/types.rs index cd1cb2f..cc1a459 100644 --- a/crates/webscrobbler/src/types.rs +++ b/crates/webscrobbler/src/types.rs @@ -163,7 +163,10 @@ impl From for Track { let releases = recording.releases.unwrap_or_default(); let album_artist = releases .first() - .and_then(|release| release.artist_credit.first()) + .and_then(|release| { + let credits = release.artist_credit.clone().unwrap_or_default(); + credits.first().map(|credit| credit.clone()) + }) .map(|credit| credit.name.clone()); let album = releases .first() -- 2.43.0 From 0944be3265d4b4d24a8a8a37c023396b0e853a81 Mon Sep 17 00:00:00 2001 From: Tsiry Sandratraina Date: Sun, 28 Sep 2025 14:00:40 +0300 Subject: [PATCH] fix: update MusicBrainz recording handling to include release information --- crates/scrobbler/src/musicbrainz/mod.rs | 2 ++ crates/scrobbler/src/scrobbler.rs | 3 ++- crates/scrobbler/src/types.rs | 6 +++--- crates/webscrobbler/src/musicbrainz/mod.rs | 3 +++ crates/webscrobbler/src/scrobbler.rs | 3 ++- crates/webscrobbler/src/types.rs | 6 +++--- 6 files changed, 15 insertions(+), 8 deletions(-) diff --git a/crates/scrobbler/src/musicbrainz/mod.rs b/crates/scrobbler/src/musicbrainz/mod.rs index eee969f..e9f3926 100644 --- a/crates/scrobbler/src/musicbrainz/mod.rs +++ b/crates/scrobbler/src/musicbrainz/mod.rs @@ -1,3 +1,5 @@ +use anyhow::Error; + use crate::musicbrainz::{recording::Recordings, release::Release}; use std::cmp::Ordering; diff --git a/crates/scrobbler/src/scrobbler.rs b/crates/scrobbler/src/scrobbler.rs index 9e34f36..d28d967 100644 --- a/crates/scrobbler/src/scrobbler.rs +++ b/crates/scrobbler/src/scrobbler.rs @@ -695,8 +695,9 @@ async fn search_musicbrainz_recording( return Ok(None); } let recording = recording.unwrap(); - let result = mb_client.get_recording(&recording.id).await?; + let mut result = mb_client.get_recording(&recording.id).await?; tracing::info!("Musicbrainz (recording)"); + result.releases = Some(vec![release]); return Ok(Some(result)); } diff --git a/crates/scrobbler/src/types.rs b/crates/scrobbler/src/types.rs index 78bbb00..7e7b55a 100644 --- a/crates/scrobbler/src/types.rs +++ b/crates/scrobbler/src/types.rs @@ -77,18 +77,18 @@ impl From for Track { .first() .map(|release| release.title.clone()) .unwrap_or_default(); + let release_date = releases.first().and_then(|release| release.date.clone()); Track { title: recording.title.clone(), album, artist: artist_credit, album_artist, duration: recording.length.unwrap_or_default(), - year: recording - .first_release_date + year: release_date .as_ref() .and_then(|date| date.split('-').next()) .and_then(|year| year.parse::().ok()), - release_date: recording.first_release_date.clone(), + release_date: release_date.clone(), track_number: releases .first() .and_then(|release| { diff --git a/crates/webscrobbler/src/musicbrainz/mod.rs b/crates/webscrobbler/src/musicbrainz/mod.rs index 13f1ef7..e9f3926 100644 --- a/crates/webscrobbler/src/musicbrainz/mod.rs +++ b/crates/webscrobbler/src/musicbrainz/mod.rs @@ -1,3 +1,5 @@ +use anyhow::Error; + use crate::musicbrainz::{recording::Recordings, release::Release}; use std::cmp::Ordering; @@ -12,6 +14,7 @@ fn get_best_release(releases: &[Release]) -> Option { return None; } + // Remove the single filtering - this was causing the issue let mut candidates: Vec<&Release> = releases.iter().collect(); if candidates.is_empty() { diff --git a/crates/webscrobbler/src/scrobbler.rs b/crates/webscrobbler/src/scrobbler.rs index af8b43d..3075381 100644 --- a/crates/webscrobbler/src/scrobbler.rs +++ b/crates/webscrobbler/src/scrobbler.rs @@ -210,8 +210,9 @@ async fn search_musicbrainz_recording( return Ok(None); } let recording = recording.unwrap(); - let result = mb_client.get_recording(&recording.id).await?; + let mut result = mb_client.get_recording(&recording.id).await?; tracing::info!("Musicbrainz (recording)"); + result.releases = Some(vec![release]); return Ok(Some(result)); } diff --git a/crates/webscrobbler/src/types.rs b/crates/webscrobbler/src/types.rs index cc1a459..654c810 100644 --- a/crates/webscrobbler/src/types.rs +++ b/crates/webscrobbler/src/types.rs @@ -161,6 +161,7 @@ impl From for Track { .map(|credit| credit.name.clone()) .unwrap_or_default(); let releases = recording.releases.unwrap_or_default(); + let release_date = releases.first().and_then(|release| release.date.clone()); let album_artist = releases .first() .and_then(|release| { @@ -178,12 +179,11 @@ impl From for Track { artist: artist_credit, album_artist, duration: recording.length.unwrap_or_default(), - year: recording - .first_release_date + year: release_date .as_ref() .and_then(|date| date.split('-').next()) .and_then(|year| year.parse::().ok()), - release_date: recording.first_release_date.clone(), + release_date: release_date.clone(), track_number: releases .first() .and_then(|release| { -- 2.43.0