jamendo: ship a default client_id so /jamendo works out of the box

The app key identifies the application, not a user, so there is no reason
to make everyone register one before they can play anything. An unset (or
blank) client_id now falls back to DEFAULT_CLIENT_ID, and init writes
whichever key is in force back into jamendo.toml, so the effective value
is always visible and replaceable.

Jamendo rate-limits per key, which the docs say plainly: a shipped
default is a shared budget, and a heavy user should register their own.
A configured key always wins.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Test User 2026-07-25 10:54:50 +02:00
parent cd3a16f95c
commit af04573b72
2 changed files with 60 additions and 9 deletions

View File

@ -22,7 +22,7 @@ use async_trait::async_trait;
use crabidy_core::proto::crabidy::{Album, LibraryNode, LibraryNodeChild, Track};
use crabidy_core::{ProviderClient, ProviderError};
use serde::{Deserialize, Serialize};
use tracing::warn;
use tracing::{debug, warn};
pub mod api;
use api::{Jam, JamAlbum, JamApi, JamTrack};
@ -42,17 +42,27 @@ pub const DEFAULT_SEARCH_RESULTS: usize = 50;
pub const DEFAULT_ALBUM_TRACKS: usize = 200;
/// Default per-request timeout in seconds.
pub const DEFAULT_CALL_TIMEOUT_SECS: u64 = 30;
/// The app key crabidy ships with, used when `jamendo.toml` configures none —
/// so `/jamendo` works out of the box. It identifies the *application*, not a
/// user, and Jamendo's rate limit is per key: a shared default means a shared
/// budget, so register your own at <https://devportal.jamendo.com> and set
/// `client_id` if you use Jamendo heavily. `init` writes whichever key is in
/// force back to the config file, so the effective value is always visible.
pub const DEFAULT_CLIENT_ID: &str = "24a50df7";
/// Default requested audio streaming format. `mp31` is Jamendo's freely
/// streamable MP3; `mp32` (higher bitrate) is *not* reliably provisioned for the
/// streaming `audio` URL and comes back empty for many tracks, so it is a poor
/// default (a Pro account can still set it explicitly).
pub const DEFAULT_AUDIOFORMAT: &str = "mp31";
/// Provider settings, persisted as `jamendo.toml`. `client_id` is **required**:
/// without it the provider does not mount (D5). The rest have defaults.
/// Provider settings, persisted as `jamendo.toml`. Every field is optional:
/// an unset `client_id` falls back to [`DEFAULT_CLIENT_ID`], so the provider
/// mounts with no configuration at all.
#[derive(Clone, Default, Deserialize, Serialize)]
pub struct Settings {
/// The registered Jamendo app key from `devportal.jamendo.com`. Required.
/// The registered Jamendo app key from `devportal.jamendo.com`. Unset
/// falls back to [`DEFAULT_CLIENT_ID`] (and `init` persists what it used).
/// **Secret**: redacted from `Debug`.
pub client_id: Option<String>,
/// Requested audio format. Default [`DEFAULT_AUDIOFORMAT`].
@ -362,14 +372,22 @@ impl ProviderClient for Client {
warn!("could not parse jamendo.toml: {err}");
ProviderError::Config("jamendo.toml is not valid TOML".to_string())
})?;
let client_id = settings
// No configured key is the normal first-run case: fall back to the
// shipped default so `/jamendo` works out of the box, and record it in
// `settings` so the write-back shows which key is in force.
let mut settings = settings;
let client_id = match settings
.client_id
.clone()
.filter(|id| !id.trim().is_empty())
.ok_or_else(|| {
warn!("jamendo provider disabled: no client_id configured");
ProviderError::Config("jamendo client_id is required".to_string())
})?;
{
Some(id) => id,
None => {
debug!("no jamendo client_id configured, using the shipped default");
settings.client_id = Some(DEFAULT_CLIENT_ID.to_string());
DEFAULT_CLIENT_ID.to_string()
}
};
let audioformat = settings
.audioformat
.clone()

View File

@ -71,6 +71,39 @@ fn settings_debug_redacts_client_id() {
assert!(dumped.contains("<redacted>"));
}
// --- The shipped default app key -----------------------------------------
/// An empty config must still mount: the shipped `DEFAULT_CLIENT_ID` stands in
/// for a missing `client_id`, and `init` records it so the write-back shows
/// which key is in force.
#[tokio::test]
async fn an_empty_config_falls_back_to_the_shipped_key() {
let client = Client::init("").await.expect("mounts with no config");
let written = client.settings();
assert!(
written.contains(DEFAULT_CLIENT_ID),
"init persists the key it used: {written}"
);
}
/// A configured key always wins over the shipped default.
#[tokio::test]
async fn a_configured_key_wins_over_the_default() {
let client = Client::init("client_id = \"my-own-key\"\n")
.await
.expect("mounts");
let written = client.settings();
assert!(written.contains("my-own-key"), "{written}");
assert!(!written.contains(DEFAULT_CLIENT_ID), "{written}");
}
/// A blank key is treated as absent, not as a key that cannot work.
#[tokio::test]
async fn a_blank_key_falls_back_too() {
let client = Client::init("client_id = \" \"\n").await.expect("mounts");
assert!(client.settings().contains(DEFAULT_CLIENT_ID));
}
// --- Tree shaping and mapping (T1, T4, T5, T7, T11) -----------------------
#[tokio::test]