Do not attempt to fetch lyrics with empty data

Modified `search_pairs` function in `lyrics.py` to:

* Firstly strip each of `artist`, `artist_sort` and `title` fields
* Only generate alternatives if both `artist` and `title` are not empty
* Ensure that `artist_sort` is not empty and not equal to artist (ignoring
  case) before appending it to the artists

Extended tests to cover the changes.
This commit is contained in:
Šarūnas Nejus 2024-10-03 14:15:56 +01:00
parent 767a83fbe6
commit 0a12d07a94
No known key found for this signature in database
GPG key ID: DD28F6704DBE3435
3 changed files with 36 additions and 12 deletions

View file

@ -152,7 +152,13 @@ def search_pairs(item):
alternatives.append(match.group(1)) alternatives.append(match.group(1))
return alternatives return alternatives
title, artist, artist_sort = item.title, item.artist, item.artist_sort title, artist, artist_sort = (
item.title.strip(),
item.artist.strip(),
item.artist_sort.strip(),
)
if not title or not artist:
return ()
patterns = [ patterns = [
# Remove any featuring artists from the artists name # Remove any featuring artists from the artists name
@ -161,7 +167,7 @@ def search_pairs(item):
artists = generate_alternatives(artist, patterns) artists = generate_alternatives(artist, patterns)
# Use the artist_sort as fallback only if it differs from artist to avoid # Use the artist_sort as fallback only if it differs from artist to avoid
# repeated remote requests with the same search terms # repeated remote requests with the same search terms
if artist != artist_sort: if artist_sort and artist.lower() != artist_sort.lower():
artists.append(artist_sort) artists.append(artist_sort)
patterns = [ patterns = [

View file

@ -45,6 +45,9 @@ Bug fixes:
configuration for each test case. This fixes the issue where some tests configuration for each test case. This fixes the issue where some tests
failed because they read developers' local lyrics configuration. failed because they read developers' local lyrics configuration.
:bug:`5133` :bug:`5133`
* :doc:`plugins/lyrics`: Do not attempt to search for lyrics if either the
artist or title is missing and ignore ``artist_sort`` value if it is empty.
:bug:`2635`
For packagers: For packagers:

View file

@ -39,21 +39,36 @@ def xfail_on_ci(msg: str) -> pytest.MarkDecorator:
class TestLyricsUtils: class TestLyricsUtils:
unexpected_empty_artist = pytest.mark.xfail( @pytest.mark.parametrize(
reason="Empty artist '' should not be present" "artist, title",
[
("Artist", ""),
("", "Title"),
(" ", ""),
("", " "),
("", ""),
],
) )
def test_search_empty(self, artist, title):
actual_pairs = lyrics.search_pairs(Item(artist=artist, title=title))
assert not list(actual_pairs)
@pytest.mark.parametrize( @pytest.mark.parametrize(
"artist, artist_sort, expected_extra_artists", "artist, artist_sort, expected_extra_artists",
[ [
_p("Alice ft. Bob", "", ["Alice"], marks=unexpected_empty_artist), ("Alice ft. Bob", "", ["Alice"]),
_p("Alice feat Bob", "", ["Alice"], marks=unexpected_empty_artist), ("Alice feat Bob", "", ["Alice"]),
_p("Alice feat. Bob", "", ["Alice"], marks=unexpected_empty_artist), ("Alice feat. Bob", "", ["Alice"]),
_p("Alice feats Bob", "", [], marks=unexpected_empty_artist), ("Alice feats Bob", "", []),
_p("Alice featuring Bob", "", ["Alice"], marks=unexpected_empty_artist), ("Alice featuring Bob", "", ["Alice"]),
_p("Alice & Bob", "", ["Alice"], marks=unexpected_empty_artist), ("Alice & Bob", "", ["Alice"]),
_p("Alice and Bob", "", ["Alice"], marks=unexpected_empty_artist), ("Alice and Bob", "", ["Alice"]),
_p("Alice", "", [], marks=unexpected_empty_artist), ("Alice", "", []),
("Alice", "Alice", []),
("Alice", "alice", []),
("Alice", "alice ", []),
("Alice", "Alice A", ["Alice A"]),
("CHVRCHΞS", "CHVRCHES", ["CHVRCHES"]), ("CHVRCHΞS", "CHVRCHES", ["CHVRCHES"]),
("横山克", "Masaru Yokoyama", ["Masaru Yokoyama"]), ("横山克", "Masaru Yokoyama", ["Masaru Yokoyama"]),
], ],