quick fix - user agent + rate limit cap
This commit is contained in:
@@ -229,6 +229,21 @@ class PrimaryMusicSourceSettings(AppStruct):
|
||||
source: Literal["listenbrainz", "lastfm"] = "listenbrainz"
|
||||
|
||||
|
||||
_OFFICIAL_MB_RATE_LIMIT = 1.0
|
||||
_OFFICIAL_MB_CONCURRENT_SEARCHES = 6
|
||||
|
||||
|
||||
def is_official_musicbrainz(url: str) -> bool:
|
||||
"""Check if the URL points to the official MusicBrainz API."""
|
||||
from urllib.parse import urlparse
|
||||
try:
|
||||
parsed = urlparse(url.strip().rstrip("/"))
|
||||
hostname = (parsed.hostname or "").lower()
|
||||
return hostname in ("musicbrainz.org", "www.musicbrainz.org")
|
||||
except (ValueError, AttributeError):
|
||||
return False
|
||||
|
||||
|
||||
class MusicBrainzConnectionSettings(AppStruct):
|
||||
api_url: str = "https://musicbrainz.org/ws/2"
|
||||
rate_limit: float = 1.0
|
||||
@@ -239,6 +254,9 @@ class MusicBrainzConnectionSettings(AppStruct):
|
||||
if not self.api_url or not self.api_url.startswith(("http://", "https://")):
|
||||
self.api_url = "https://musicbrainz.org/ws/2"
|
||||
self.api_url = self.api_url.rstrip("/")
|
||||
if is_official_musicbrainz(self.api_url):
|
||||
self.rate_limit = min(self.rate_limit, _OFFICIAL_MB_RATE_LIMIT)
|
||||
self.concurrent_searches = min(self.concurrent_searches, _OFFICIAL_MB_CONCURRENT_SEARCHES)
|
||||
if self.rate_limit < 0.1 or self.rate_limit > 50.0:
|
||||
raise msgspec.ValidationError("rate_limit must be between 0.1 and 50.0")
|
||||
if self.concurrent_searches < 1 or self.concurrent_searches > 30:
|
||||
|
||||
@@ -65,6 +65,7 @@ class Settings(BaseSettings):
|
||||
config_file_path: Path = Field(default=Path("/app/config/config.json"))
|
||||
audiodb_api_key: str = Field(default="123")
|
||||
audiodb_premium: bool = Field(default=False, description="Set to true if using a premium AudioDB API key")
|
||||
instance_id: str = Field(default="", description="Auto-generated per-instance UUID for User-Agent differentiation")
|
||||
|
||||
@field_validator("log_level")
|
||||
@classmethod
|
||||
@@ -121,7 +122,8 @@ class Settings(BaseSettings):
|
||||
return self
|
||||
|
||||
def get_user_agent(self) -> str:
|
||||
return f"Musicseerr/1.0 ({self.contact_email}; https://www.musicseerr.com)"
|
||||
id_part = self.instance_id[:8] if self.instance_id else "unknown"
|
||||
return f"Musicseerr/1.0 ({id_part}; {self.contact_email}; https://www.musicseerr.com)"
|
||||
|
||||
def load_from_file(self) -> None:
|
||||
if not self.config_file_path.exists():
|
||||
|
||||
@@ -67,6 +67,7 @@ async def lifespan(app: FastAPI):
|
||||
await init_app_state(app)
|
||||
|
||||
preferences_service = get_preferences_service()
|
||||
settings.instance_id = preferences_service.get_instance_id()
|
||||
advanced_settings = preferences_service.get_advanced_settings()
|
||||
|
||||
cache = get_cache()
|
||||
|
||||
@@ -22,7 +22,14 @@ class MusicBrainzRepository(MusicBrainzArtistMixin, MusicBrainzAlbumMixin):
|
||||
self._apply_settings()
|
||||
|
||||
def _apply_settings(self) -> None:
|
||||
from api.v1.schemas.settings import (
|
||||
is_official_musicbrainz, _OFFICIAL_MB_RATE_LIMIT, _OFFICIAL_MB_CONCURRENT_SEARCHES,
|
||||
)
|
||||
|
||||
settings = self._preferences_service.get_musicbrainz_connection()
|
||||
if is_official_musicbrainz(settings.api_url):
|
||||
settings.rate_limit = min(settings.rate_limit, _OFFICIAL_MB_RATE_LIMIT)
|
||||
settings.concurrent_searches = min(settings.concurrent_searches, _OFFICIAL_MB_CONCURRENT_SEARCHES)
|
||||
set_mb_api_base(settings.api_url)
|
||||
mb_rate_limiter.update_rate(settings.rate_limit)
|
||||
if mb_rate_limiter.capacity != settings.concurrent_searches:
|
||||
|
||||
@@ -42,6 +42,23 @@ class PreferencesService:
|
||||
self._config_cache: Optional[dict] = None
|
||||
self._cache_lock = threading.Lock()
|
||||
self._migrate_musicbrainz_settings()
|
||||
self._ensure_instance_id()
|
||||
|
||||
def _ensure_instance_id(self) -> None:
|
||||
"""Generate a stable instance ID on first run."""
|
||||
config = self._load_config()
|
||||
if config.get("instance_id"):
|
||||
return
|
||||
import uuid
|
||||
instance_id = str(uuid.uuid4())
|
||||
config = self._load_config().copy()
|
||||
config["instance_id"] = instance_id
|
||||
self._save_config(config)
|
||||
logger.info("Generated new instance ID: %s", instance_id)
|
||||
|
||||
def get_instance_id(self) -> str:
|
||||
config = self._load_config()
|
||||
return config.get("instance_id", "unknown")
|
||||
|
||||
def _load_config(self) -> dict:
|
||||
with self._cache_lock:
|
||||
|
||||
@@ -784,6 +784,13 @@ class SettingsService:
|
||||
from repositories.musicbrainz_base import (
|
||||
set_mb_api_base, mb_rate_limiter, mb_circuit_breaker, mb_deduplicator,
|
||||
)
|
||||
from api.v1.schemas.settings import (
|
||||
is_official_musicbrainz, _OFFICIAL_MB_RATE_LIMIT, _OFFICIAL_MB_CONCURRENT_SEARCHES,
|
||||
)
|
||||
|
||||
if is_official_musicbrainz(settings.api_url):
|
||||
settings.rate_limit = min(settings.rate_limit, _OFFICIAL_MB_RATE_LIMIT)
|
||||
settings.concurrent_searches = min(settings.concurrent_searches, _OFFICIAL_MB_CONCURRENT_SEARCHES)
|
||||
|
||||
set_mb_api_base(settings.api_url)
|
||||
mb_rate_limiter.update_rate(settings.rate_limit)
|
||||
|
||||
@@ -0,0 +1,147 @@
|
||||
import pytest
|
||||
|
||||
from api.v1.schemas.settings import (
|
||||
is_official_musicbrainz,
|
||||
MusicBrainzConnectionSettings,
|
||||
_OFFICIAL_MB_RATE_LIMIT,
|
||||
_OFFICIAL_MB_CONCURRENT_SEARCHES,
|
||||
)
|
||||
|
||||
|
||||
class TestIsOfficialMusicBrainz:
|
||||
"""Test the URL detection helper."""
|
||||
|
||||
def test_official_https(self):
|
||||
assert is_official_musicbrainz("https://musicbrainz.org/ws/2") is True
|
||||
|
||||
def test_official_http(self):
|
||||
assert is_official_musicbrainz("http://musicbrainz.org/ws/2") is True
|
||||
|
||||
def test_official_www(self):
|
||||
assert is_official_musicbrainz("https://www.musicbrainz.org/ws/2") is True
|
||||
|
||||
def test_official_uppercase(self):
|
||||
assert is_official_musicbrainz("https://MUSICBRAINZ.ORG/ws/2") is True
|
||||
|
||||
def test_official_trailing_slash(self):
|
||||
assert is_official_musicbrainz("https://musicbrainz.org/ws/2/") is True
|
||||
|
||||
def test_official_with_spaces(self):
|
||||
assert is_official_musicbrainz(" https://musicbrainz.org/ws/2 ") is True
|
||||
|
||||
def test_custom_mirror(self):
|
||||
assert is_official_musicbrainz("https://my-mirror.example.com/ws/2") is False
|
||||
|
||||
def test_localhost(self):
|
||||
assert is_official_musicbrainz("http://localhost:5000/ws/2") is False
|
||||
|
||||
def test_empty_string(self):
|
||||
assert is_official_musicbrainz("") is False
|
||||
|
||||
def test_not_a_url(self):
|
||||
assert is_official_musicbrainz("not a url") is False
|
||||
|
||||
def test_subdomain_not_www(self):
|
||||
assert is_official_musicbrainz("https://api.musicbrainz.org/ws/2") is False
|
||||
|
||||
|
||||
class TestMusicBrainzSettingsClamping:
|
||||
"""Test that rate limits are clamped for official API."""
|
||||
|
||||
def test_official_url_clamps_rate_limit(self):
|
||||
settings = MusicBrainzConnectionSettings(
|
||||
api_url="https://musicbrainz.org/ws/2",
|
||||
rate_limit=10.0,
|
||||
concurrent_searches=6,
|
||||
)
|
||||
assert settings.rate_limit == _OFFICIAL_MB_RATE_LIMIT
|
||||
|
||||
def test_official_url_clamps_concurrent_searches(self):
|
||||
settings = MusicBrainzConnectionSettings(
|
||||
api_url="https://musicbrainz.org/ws/2",
|
||||
rate_limit=1.0,
|
||||
concurrent_searches=20,
|
||||
)
|
||||
assert settings.concurrent_searches == _OFFICIAL_MB_CONCURRENT_SEARCHES
|
||||
|
||||
def test_official_url_clamps_both(self):
|
||||
settings = MusicBrainzConnectionSettings(
|
||||
api_url="https://musicbrainz.org/ws/2",
|
||||
rate_limit=50.0,
|
||||
concurrent_searches=30,
|
||||
)
|
||||
assert settings.rate_limit == _OFFICIAL_MB_RATE_LIMIT
|
||||
assert settings.concurrent_searches == _OFFICIAL_MB_CONCURRENT_SEARCHES
|
||||
|
||||
def test_official_url_does_not_increase_low_values(self):
|
||||
settings = MusicBrainzConnectionSettings(
|
||||
api_url="https://musicbrainz.org/ws/2",
|
||||
rate_limit=0.5,
|
||||
concurrent_searches=3,
|
||||
)
|
||||
assert settings.rate_limit == 0.5
|
||||
assert settings.concurrent_searches == 3
|
||||
|
||||
def test_custom_url_allows_high_rate_limit(self):
|
||||
settings = MusicBrainzConnectionSettings(
|
||||
api_url="https://my-mirror.example.com/ws/2",
|
||||
rate_limit=25.0,
|
||||
concurrent_searches=20,
|
||||
)
|
||||
assert settings.rate_limit == 25.0
|
||||
assert settings.concurrent_searches == 20
|
||||
|
||||
def test_defaults_unchanged(self):
|
||||
settings = MusicBrainzConnectionSettings()
|
||||
assert settings.rate_limit == 1.0
|
||||
assert settings.concurrent_searches == 6
|
||||
assert settings.api_url == "https://musicbrainz.org/ws/2"
|
||||
|
||||
|
||||
class TestInstanceId:
|
||||
"""Test instance ID generation and retrieval."""
|
||||
|
||||
def test_ensure_instance_id_generates_on_first_run(self, tmp_path):
|
||||
from core.config import Settings
|
||||
from services.preferences_service import PreferencesService
|
||||
|
||||
config_path = tmp_path / "config.json"
|
||||
settings = Settings(config_file_path=config_path, root_app_dir=tmp_path)
|
||||
prefs = PreferencesService(settings)
|
||||
|
||||
instance_id = prefs.get_instance_id()
|
||||
assert instance_id != "unknown"
|
||||
assert len(instance_id) == 36 # UUID format: 8-4-4-4-12
|
||||
|
||||
def test_instance_id_is_stable_across_loads(self, tmp_path):
|
||||
from core.config import Settings
|
||||
from services.preferences_service import PreferencesService
|
||||
|
||||
config_path = tmp_path / "config.json"
|
||||
settings = Settings(config_file_path=config_path, root_app_dir=tmp_path)
|
||||
prefs1 = PreferencesService(settings)
|
||||
id1 = prefs1.get_instance_id()
|
||||
|
||||
# Create a new instance pointing to the same config
|
||||
prefs2 = PreferencesService(settings)
|
||||
id2 = prefs2.get_instance_id()
|
||||
|
||||
assert id1 == id2
|
||||
|
||||
def test_instance_id_in_user_agent(self, tmp_path):
|
||||
from core.config import Settings
|
||||
|
||||
settings = Settings(
|
||||
instance_id="a1b2c3d4-e5f6-7890-abcd-ef1234567890",
|
||||
root_app_dir=tmp_path,
|
||||
)
|
||||
ua = settings.get_user_agent()
|
||||
assert "a1b2c3d4" in ua
|
||||
assert "Musicseerr/1.0" in ua
|
||||
|
||||
def test_user_agent_unknown_when_no_instance_id(self, tmp_path):
|
||||
from core.config import Settings
|
||||
|
||||
settings = Settings(instance_id="", root_app_dir=tmp_path)
|
||||
ua = settings.get_user_agent()
|
||||
assert "unknown" in ua
|
||||
@@ -48,10 +48,28 @@
|
||||
}
|
||||
}
|
||||
|
||||
function isOfficialMusicBrainz(url: string): boolean {
|
||||
try {
|
||||
const hostname = new URL(url.trim()).hostname.toLowerCase();
|
||||
return hostname === 'musicbrainz.org' || hostname === 'www.musicbrainz.org';
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
let isOfficialApi = $derived(form.data ? isOfficialMusicBrainz(form.data.api_url) : true);
|
||||
|
||||
let hasPassedTest = $derived(
|
||||
form.testResult != null && (form.testResult as MusicBrainzTestResult).valid === true
|
||||
);
|
||||
|
||||
$effect(() => {
|
||||
if (isOfficialApi && form.data) {
|
||||
if (form.data.rate_limit > 1.0) form.data.rate_limit = 1.0;
|
||||
if (form.data.concurrent_searches > 6) form.data.concurrent_searches = 6;
|
||||
}
|
||||
});
|
||||
|
||||
$effect(() => {
|
||||
form.load();
|
||||
});
|
||||
@@ -90,6 +108,28 @@
|
||||
</p>
|
||||
</div>
|
||||
|
||||
{#if isOfficialApi}
|
||||
<div class="alert alert-info text-sm">
|
||||
<svg
|
||||
xmlns="http://www.w3.org/2000/svg"
|
||||
fill="none"
|
||||
viewBox="0 0 24 24"
|
||||
class="stroke-current shrink-0 w-5 h-5"
|
||||
>
|
||||
<path
|
||||
stroke-linecap="round"
|
||||
stroke-linejoin="round"
|
||||
stroke-width="2"
|
||||
d="M13 16h-1v-4h-1m1-4h.01M21 12a9 9 0 11-18 0 9 9 0 0118 0z"
|
||||
/>
|
||||
</svg>
|
||||
<span
|
||||
>Rate limit capped for the public MusicBrainz API. Use a self-hosted instance for
|
||||
higher limits.</span
|
||||
>
|
||||
</div>
|
||||
{/if}
|
||||
|
||||
<div class="grid grid-cols-1 sm:grid-cols-2 gap-x-6 gap-y-4">
|
||||
<div class="form-control w-full">
|
||||
<label class="label" for="mb-rate-limit">
|
||||
@@ -99,7 +139,7 @@
|
||||
id="mb-rate-limit"
|
||||
type="number"
|
||||
min="0.1"
|
||||
max="50"
|
||||
max={isOfficialApi ? 1 : 50}
|
||||
step="0.1"
|
||||
bind:value={form.data.rate_limit}
|
||||
class="input w-full"
|
||||
@@ -118,7 +158,7 @@
|
||||
id="mb-concurrent"
|
||||
type="number"
|
||||
min="1"
|
||||
max="30"
|
||||
max={isOfficialApi ? 6 : 30}
|
||||
bind:value={form.data.concurrent_searches}
|
||||
class="input w-full"
|
||||
/>
|
||||
|
||||
Reference in New Issue
Block a user