From 16a3e62ddb406a92c641c26e5e98d35984d41b57 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 7 Feb 2026 18:42:08 +0000 Subject: [PATCH] Improve UUID validation and add comprehensive tests Co-authored-by: snejus <16212750+snejus@users.noreply.github.com> --- beetsplug/_utils/musicbrainz.py | 18 +++++- test/plugins/utils/test_musicbrainz.py | 89 ++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 3 deletions(-) diff --git a/beetsplug/_utils/musicbrainz.py b/beetsplug/_utils/musicbrainz.py index 5d96581ab..74fdf15f6 100644 --- a/beetsplug/_utils/musicbrainz.py +++ b/beetsplug/_utils/musicbrainz.py @@ -197,9 +197,21 @@ class MusicBrainzAPI(RequestHandler): * Values are lowercased and stripped of whitespace. """ def format_value(value: str) -> str: - """Format a filter value, quoting it unless it's a UUID.""" - # Check if value is a UUID (MBID) - 8-4-4-4-12 format with hyphens - if len(value) == 36 and value.count("-") == 4: + """Format a filter value, quoting it unless it's a UUID. + + UUIDs are detected by checking for the 8-4-4-4-12 format: + - Total length of 36 characters + - Exactly 4 hyphens + - Hyphens at positions 8, 13, 18, and 23 + """ + if ( + len(value) == 36 + and value.count("-") == 4 + and value[8] == "-" + and value[13] == "-" + and value[18] == "-" + and value[23] == "-" + ): # Likely a UUID/MBID, don't quote it return value # Regular string value, quote it diff --git a/test/plugins/utils/test_musicbrainz.py b/test/plugins/utils/test_musicbrainz.py index 291f50eb5..81fae49af 100644 --- a/test/plugins/utils/test_musicbrainz.py +++ b/test/plugins/utils/test_musicbrainz.py @@ -1,5 +1,7 @@ from beetsplug._utils.musicbrainz import MusicBrainzAPI +import pytest + def test_group_relations(): raw_release = { @@ -80,3 +82,90 @@ def test_group_relations(): }, ], } + + +class TestSearchQueryConstruction: + """Test query construction in the search method.""" + + @pytest.fixture + def api(self): + return MusicBrainzAPI() + + @pytest.fixture + def mock_get_json(self, monkeypatch): + """Mock get_json to capture queries without making actual API calls.""" + queries = [] + + def capture_query(*args, **kwargs): + if 'params' in kwargs and 'query' in kwargs['params']: + queries.append(kwargs['params']['query']) + return {"releases": []} + + monkeypatch.setattr( + "beetsplug._utils.musicbrainz.MusicBrainzAPI.get_json", + capture_query + ) + return queries + + def test_uuid_not_quoted(self, api, mock_get_json): + """Test that valid UUIDs are not quoted in search queries.""" + uuid = "89ad4ac3-39f7-470e-963a-56509c546377" + api.search("release", {"arid": uuid}) + + assert len(mock_get_json) == 1 + query = mock_get_json[0] + assert f"arid:{uuid}" in query + assert f'arid:"{uuid}"' not in query + + def test_string_quoted(self, api, mock_get_json): + """Test that regular strings are quoted in search queries.""" + api.search("release", {"artist": "Test Artist"}) + + assert len(mock_get_json) == 1 + query = mock_get_json[0] + assert 'artist:"test artist"' in query + + def test_mixed_uuid_and_string(self, api, mock_get_json): + """Test that UUIDs and strings are formatted correctly together.""" + uuid = "89ad4ac3-39f7-470e-963a-56509c546377" + api.search("release", { + "release": "Test Album", + "arid": uuid + }) + + assert len(mock_get_json) == 1 + query = mock_get_json[0] + assert 'release:"test album"' in query + assert f"arid:{uuid}" in query + assert f'arid:"{uuid}"' not in query + + def test_invalid_uuid_quoted(self, api, mock_get_json): + """Test that invalid UUID-like strings are quoted.""" + # Wrong length + api.search("release", {"test": "not-a-uuid-123"}) + assert 'test:"not-a-uuid-123"' in mock_get_json[0] + + mock_get_json.clear() + + # Wrong hyphen positions + api.search("release", {"test": "abcd-efgh-ijkl-mnop-qrstuvwxyz123456"}) + assert 'test:"abcd-efgh-ijkl-mnop-qrstuvwxyz123456"' in mock_get_json[0] + + mock_get_json.clear() + + # Correct length and hyphen count but wrong positions + api.search("release", {"test": "12345678-123-456-7890-123456789012"}) + assert 'test:"12345678-123-456-7890-123456789012"' in mock_get_json[0] + + def test_empty_value_filtered(self, api, mock_get_json): + """Test that empty values are filtered out of the query.""" + api.search("release", { + "release": "Test Album", + "empty": "" + }) + + assert len(mock_get_json) == 1 + query = mock_get_json[0] + assert 'release:"test album"' in query + assert "empty" not in query +